Merge pull request #3525 from vitorpamplona/claude/dm-unread-own-messages-jl18pt

Fix DM read state: mark rooms as read when sending messages
2026-08-09 08:04:45 +00:00 · 2026-07-10 20:55:14 -04:00
parent 57aaf69073 139659bbda
commit 0eb2beda22
7 changed files with 249 additions and 17 deletions
@@ -187,6 +187,7 @@ import com.vitorpamplona.quartz.nip10Notes.threadRootIdOrSelf
 import com.vitorpamplona.quartz.nip17Dm.NIP17Factory
 import com.vitorpamplona.quartz.nip17Dm.base.BaseDMGroupEvent
 import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey
+import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKeyable
 import com.vitorpamplona.quartz.nip17Dm.base.NIP17Group
 import com.vitorpamplona.quartz.nip17Dm.files.ChatMessageEncryptedFileHeaderEvent
 import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent
@@ -2526,6 +2527,8 @@ class Account(
        // broadcastPrivately) instead of waiting for the newEventBundles
        // batcher; the later batched re-delivery is deduped by the chatroom.
        cache.getNoteIfExists(newEvent.id)?.let { newNotesPreProcessor.consume(it) }
+
+        markDmRoomAsRead(newEvent)
    }

    override suspend fun sendNip17EncryptedFile(template: EventTemplate<ChatMessageEncryptedFileHeaderEvent>) {
@@ -2586,6 +2589,28 @@ class Account(
        // batcher re-delivers this note later; the processor's replay path and
        // the chatroom add are both idempotent.
        mineNote?.let { newNotesPreProcessor.consume(it) }
+
+        markDmRoomAsRead(signedEvents.msg)
+    }
+
+    /**
+     * Sending a message into a DM room means the user has caught up with what the room
+     * showed when they replied: advance the local read marker to the newest known message —
+     * not just the sent one, whose local clock may lag behind a skew-ahead peer's — so the
+     * unread indicators clear without requiring the conversation to be reopened
+     * (#1286, #1287). No-op for private events that don't belong to a room (private notes,
+     * reactions, deletions).
+     */
+    private fun markDmRoomAsRead(event: Event) {
+        if (event is ChatroomKeyable) {
+            val room = event.chatroomKey(signer.pubKey)
+            val newestInRoom =
+                chatroomList.rooms
+                    .get(room)
+                    ?.newestMessage
+                    ?.createdAt() ?: 0L
+            markAsRead(privateChatLastReadRoute(room), maxOf(event.createdAt, newestInRoom))
+        }
    }

    // --- Marmot Group Messaging ---
@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.model
+
+import com.vitorpamplona.quartz.nip01Core.core.Event
+import com.vitorpamplona.quartz.nip01Core.core.HexKey
+import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey
+import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKeyable
+
+/**
+ * Route key under which a private chat room's last-read time is stored in AccountSettings.
+ * Every marker writer (send paths, ingestion, room view, hidden-room sweep) and reader
+ * (Messages-tab dot, room-row bubble) must build the key through this function: a format
+ * drift between a writer and a reader silently splits read state (#1286).
+ */
+fun privateChatLastReadRoute(room: ChatroomKey) = "Room/${room.hashCode()}"
+
+/**
+ * True when [message] marks [room] as read up to its timestamp: the logged-in user authored
+ * it, so sending it — from this device, or from another one arriving via the self-addressed
+ * gift wrap — means they had caught up with the conversation (#1286, #1287). Notes-to-self
+ * rooms are exempt: there the user's own messages ARE the content still to be seen.
+ */
+fun chatMessageMarksRoomAsRead(
+    message: Event,
+    room: ChatroomKey,
+    loggedInUser: HexKey,
+): Boolean = message.pubKey == loggedInUser && room.users.singleOrNull() != loggedInUser
+
+/**
+ * Read-marker route + timestamp for the newest message of a private chat room, or null when
+ * the room cannot be unread: no chat event, a newest message that counts as read (see
+ * [chatMessageMarksRoomAsRead]), or every participant hidden.
+ */
+fun unreadPrivateChatRoute(
+    newestMessage: Event?,
+    loggedInUser: HexKey,
+    isAllHidden: (Set<HexKey>) -> Boolean,
+): Pair<String, Long>? {
+    if (newestMessage !is ChatroomKeyable) return null
+    val room = newestMessage.chatroomKey(loggedInUser)
+    if (chatMessageMarksRoomAsRead(newestMessage, room, loggedInUser)) return null
+    if (isAllHidden(room.users)) return null
+    return privateChatLastReadRoute(room) to newestMessage.createdAt
+}
@@ -68,6 +68,8 @@ import com.vitorpamplona.amethyst.model.User
 import com.vitorpamplona.amethyst.model.privacyOptions.EmptyRoleBasedHttpClientBuilder
 import com.vitorpamplona.amethyst.model.privacyOptions.IRoleBasedHttpClientBuilder
 import com.vitorpamplona.amethyst.model.privacyOptions.RoleBasedHttpClientBuilder
+import com.vitorpamplona.amethyst.model.privateChatLastReadRoute
+import com.vitorpamplona.amethyst.model.unreadPrivateChatRoute
 import com.vitorpamplona.amethyst.service.ClinkDebitPayer
 import com.vitorpamplona.amethyst.service.OnlineChecker
 import com.vitorpamplona.amethyst.service.V4VPaymentHandler
@@ -1886,7 +1888,7 @@ class AccountViewModel(
                    }

                    noteEvent is ChatroomKeyable -> {
-                        account.markAsRead("Room/${noteEvent.chatroomKey(account.signer.pubKey).hashCode()}", noteEvent.createdAt)
+                        account.markAsRead(privateChatLastReadRoute(noteEvent.chatroomKey(account.signer.pubKey)), noteEvent.createdAt)
                    }

                    noteEvent is DraftWrapEvent -> {
@@ -1894,7 +1896,7 @@ class AccountViewModel(
                        if (innerEvent is IsInPublicChatChannel) {
                            account.markAsRead("Channel/${innerEvent.channelId()}", noteEvent.createdAt)
                        } else if (innerEvent is ChatroomKeyable) {
-                            account.markAsRead("Room/${innerEvent.chatroomKey(account.signer.pubKey).hashCode()}", noteEvent.createdAt)
+                            account.markAsRead(privateChatLastReadRoute(innerEvent.chatroomKey(account.signer.pubKey)), noteEvent.createdAt)
                        }
                    }
                }
@@ -1904,25 +1906,18 @@ class AccountViewModel(
        }
    }

-    private fun unreadPrivateChatRoute(chat: Note): Pair<String, Long>? {
-        val noteEvent = chat.event ?: return null
-        val room = (noteEvent as? ChatroomKeyable)?.chatroomKey(account.signer.pubKey) ?: return null
-        if (account.isAllHidden(room.users)) return null
-        return privateChatRoute(room) to noteEvent.createdAt
-    }
+    private fun unreadPrivateChatRoute(chat: Note): Pair<String, Long>? = unreadPrivateChatRoute(chat.event, account.signer.pubKey, account::isAllHidden)

    private fun markHiddenChatroomsAsRead() {
        account.chatroomList.rooms.forEach { roomKey, chatroom ->
            if (account.isAllHidden(roomKey.users)) {
                chatroom.newestMessage?.createdAt()?.let {
-                    account.markAsRead(privateChatRoute(roomKey), it)
+                    account.markAsRead(privateChatLastReadRoute(roomKey), it)
                }
            }
        }
    }

-    private fun privateChatRoute(room: ChatroomKey) = "Room/${room.hashCode()}"
-
    class Factory(
        val account: Account,
        val settings: UiSettingsState,
@@ -26,6 +26,8 @@ import com.vitorpamplona.amethyst.commons.nipACWebRtcCalls.CallManager
 import com.vitorpamplona.amethyst.model.Account
 import com.vitorpamplona.amethyst.model.LocalCache
 import com.vitorpamplona.amethyst.model.Note
+import com.vitorpamplona.amethyst.model.chatMessageMarksRoomAsRead
+import com.vitorpamplona.amethyst.model.privateChatLastReadRoute
 import com.vitorpamplona.quartz.experimental.ephemChat.chat.EphemeralChatEvent
 import com.vitorpamplona.quartz.marmot.GroupEventResult
 import com.vitorpamplona.quartz.marmot.MarmotInboundProcessor
@@ -96,7 +98,10 @@ class EventProcessor(
            is CallRenegotiateEvent,
            -> callManager?.onSignalingEvent(event)

-            is ChatroomKeyable -> chatHandler.add(event, eventNote, publicNote)
+            is ChatroomKeyable -> {
+                chatHandler.add(event, eventNote, publicNote)
+                markOwnChatMessageAsRead(event)
+            }

            is DraftWrapEvent -> draftHandler.add(event, eventNote, publicNote)

@@ -110,6 +115,23 @@ class EventProcessor(
        }
    }

+    /**
+     * A chat message authored by this account — sent from this device through any code
+     * path, or arriving via the self-addressed gift wrap from another device — means the
+     * user was caught up with the room when they sent it, so advance the room's read
+     * marker here at the single ingestion choke point rather than at each send site
+     * (#1286, #1287). markAsRead is monotonic, so out-of-order history sync cannot move
+     * the marker backwards. Unsent drafts never reach this branch (DraftEventHandler
+     * indexes their rumors directly into the chatroom).
+     */
+    private fun <T> markOwnChatMessageAsRead(event: T) where T : Event, T : ChatroomKeyable {
+        val me = account.signer.pubKey
+        val room = event.chatroomKey(me)
+        if (chatMessageMarksRoomAsRead(event, room, me)) {
+            account.markAsRead(privateChatLastReadRoute(room), event.createdAt)
+        }
+    }
+
    suspend fun delete(note: Note) {
        note.event?.let { event ->
            try {
@@ -48,6 +48,7 @@ import com.vitorpamplona.amethyst.commons.ui.feeds.RelayReachDetailDialog
 import com.vitorpamplona.amethyst.commons.ui.feeds.RelayReachMarkers
 import com.vitorpamplona.amethyst.commons.ui.feeds.RelayReachSentinels
 import com.vitorpamplona.amethyst.commons.ui.feeds.RelayReachState
+import com.vitorpamplona.amethyst.model.privateChatLastReadRoute
 import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.EventFinderFilterAssemblerSubscription
 import com.vitorpamplona.amethyst.ui.actions.uploads.resolveSharedMedia
 import com.vitorpamplona.amethyst.ui.feeds.WatchLifecycleAndUpdateModel
@@ -268,7 +269,7 @@ fun ChatroomViewUI(
                feedContentState = feedViewModel.feedState,
                accountViewModel = accountViewModel,
                nav = nav,
-                routeForLastRead = "Room/${room.hashCode()}",
+                routeForLastRead = privateChatLastReadRoute(room),
                avoidDraft = newPostModel.draftTag,
                onWantsToReply = newPostModel::reply,
                onWantsToEditDraft = newPostModel::editFromDraft,
@@ -61,7 +61,9 @@ import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupChann
 import com.vitorpamplona.amethyst.model.LocalCache
 import com.vitorpamplona.amethyst.model.Note
 import com.vitorpamplona.amethyst.model.User
+import com.vitorpamplona.amethyst.model.chatMessageMarksRoomAsRead
 import com.vitorpamplona.amethyst.model.nip11RelayInfo.loadRelayInfo
+import com.vitorpamplona.amethyst.model.privateChatLastReadRoute
 import com.vitorpamplona.amethyst.service.relayClient.reqCommand.channel.observeChannel
 import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNoteHasEvent
 import com.vitorpamplona.amethyst.service.relayClient.reqCommand.user.UserFinderByParentFilterAssemblerSubscription
@@ -143,7 +145,7 @@ fun ChatroomComposeChannelOrUser(
    val baseNoteEvent = baseNote.event
    if (baseNoteEvent is DraftWrapEvent) {
        ObserveDraftEvent(baseNote, accountViewModel) { innerNote ->
-            ChatroomEntry(innerNote, accountViewModel, nav)
+            ChatroomEntry(innerNote, accountViewModel, nav, isDraft = true)
        }
    } else {
        ChatroomEntry(baseNote, accountViewModel, nav)
@@ -155,6 +157,7 @@ private fun ChatroomEntry(
    lastMessage: Note,
    accountViewModel: AccountViewModel,
    nav: INav,
+    isDraft: Boolean = false,
 ) {
    if (lastMessage is RelayGroupServerRoomNote) {
        RelayGroupServerRoomCompose(lastMessage, accountViewModel, nav)
@@ -213,7 +216,7 @@ private fun ChatroomEntry(

        is ChatroomKeyable -> {
            val room = baseNoteEvent.chatroomKey(accountViewModel.userProfile().pubkeyHex)
-            UserRoomCompose(room, lastMessage, accountViewModel, nav)
+            UserRoomCompose(room, lastMessage, isDraft, accountViewModel, nav)
        }

        is EphemeralChatEvent -> {
@@ -521,6 +524,7 @@ private fun ChannelTitleWithLabelInfo(
 private fun UserRoomCompose(
    room: ChatroomKey,
    lastMessage: Note,
+    isDraft: Boolean,
    accountViewModel: AccountViewModel,
    nav: INav,
 ) {
@@ -572,8 +576,15 @@ private fun UserRoomCompose(
                }
            }

-            val lastReadTime by accountViewModel.account.loadLastReadFlow("Room/${room.hashCode()}").collectAsStateWithLifecycle()
-            if ((lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime) {
+            // A sent message I authored counts as read (#1286, #1287); an unsent draft still needs my attention.
+            val newestEvent = lastMessage.event
+            val countsAsRead =
+                !isDraft &&
+                    newestEvent != null &&
+                    chatMessageMarksRoomAsRead(newestEvent, room, accountViewModel.account.signer.pubKey)
+
+            val lastReadTime by accountViewModel.account.loadLastReadFlow(privateChatLastReadRoute(room)).collectAsStateWithLifecycle()
+            if (!countsAsRead && (lastMessage.createdAt() ?: Long.MIN_VALUE) > lastReadTime) {
                Spacer(modifier = Height4dpModifier)
                NewItemsBubble()
            }
@@ -0,0 +1,115 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.model
+
+import com.vitorpamplona.quartz.nip01Core.core.HexKey
+import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey
+import com.vitorpamplona.quartz.nip17Dm.messages.ChatMessageEvent
+import com.vitorpamplona.quartz.nip25Reactions.ReactionEvent
+import kotlinx.collections.immutable.persistentSetOf
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+
+/**
+ * The unread predicate behind the Messages tab dot and the room-row bubble: a room whose
+ * newest message was authored by the logged-in user counts as read (#1286, #1287), except
+ * notes-to-self rooms, where the user's own messages are the content still to be seen.
+ */
+class PrivateChatroomReadStateTest {
+    private val me: HexKey = "a".repeat(64)
+    private val peer: HexKey = "b".repeat(64)
+
+    private val roomWithPeer = ChatroomKey(persistentSetOf(peer))
+    private val selfRoom = ChatroomKey(persistentSetOf(me))
+
+    private fun message(
+        from: HexKey,
+        to: HexKey,
+        createdAt: Long,
+    ) = ChatMessageEvent(
+        id = "0".repeat(64),
+        pubKey = from,
+        createdAt = createdAt,
+        tags = arrayOf(arrayOf("p", to)),
+        content = "hello",
+        sig = "",
+    )
+
+    @Test
+    fun newestMessageFromPeerReturnsTheRoomRoute() {
+        val route = unreadPrivateChatRoute(message(from = peer, to = me, createdAt = 100), me, isAllHidden = { false })
+
+        assertEquals(privateChatLastReadRoute(roomWithPeer) to 100L, route)
+    }
+
+    @Test
+    fun newestMessageAuthoredByMeCountsAsRead() {
+        assertNull(unreadPrivateChatRoute(message(from = me, to = peer, createdAt = 100), me, isAllHidden = { false }))
+    }
+
+    @Test
+    fun notesToSelfRoomsCanStillBeUnread() {
+        val route = unreadPrivateChatRoute(message(from = me, to = me, createdAt = 100), me, isAllHidden = { false })
+
+        assertEquals(privateChatLastReadRoute(selfRoom) to 100L, route)
+    }
+
+    @Test
+    fun hiddenRoomsAreNeverUnread() {
+        assertNull(unreadPrivateChatRoute(message(from = peer, to = me, createdAt = 100), me, isAllHidden = { true }))
+    }
+
+    @Test
+    fun missingEventIsNotUnread() {
+        assertNull(unreadPrivateChatRoute(null, me, isAllHidden = { false }))
+    }
+
+    @Test
+    fun nonChatEventsAreNotUnread() {
+        val reaction =
+            ReactionEvent(
+                id = "0".repeat(64),
+                pubKey = peer,
+                createdAt = 100,
+                tags = arrayOf(arrayOf("p", me)),
+                content = "+",
+                sig = "",
+            )
+
+        assertNull(unreadPrivateChatRoute(reaction, me, isAllHidden = { false }))
+    }
+
+    @Test
+    fun myMessageMarksAPeerRoomAsRead() {
+        assertEquals(true, chatMessageMarksRoomAsRead(message(from = me, to = peer, createdAt = 100), roomWithPeer, me))
+    }
+
+    @Test
+    fun aPeerMessageNeverMarksTheRoomAsRead() {
+        assertEquals(false, chatMessageMarksRoomAsRead(message(from = peer, to = me, createdAt = 100), roomWithPeer, me))
+    }
+
+    @Test
+    fun myMessageDoesNotMarkTheSelfRoomAsRead() {
+        assertEquals(false, chatMessageMarksRoomAsRead(message(from = me, to = me, createdAt = 100), selfRoom, me))
+    }
+}