fix(chat-list): mark read moves READ to the highest stored message id

apps/flipcash/shared/chat/src/commonMain/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt

@@ -675,9 +675,10 @@ class MessagingDelegate @Inject constructor(
     }
 
     override suspend fun markAsRead(chatId: ChatId): Result<Unit> {
-        // The stored newest id, tombstones included — the feed's own `lastMessage` skips them so
+        // The highest stored id, tombstones included — the feed's own `lastMessage` skips them so
         // the list can preview the last message with content, and reading it here would park the
-        // pointer below a deleted message and leave the chat permanently unread.
+        // pointer below a deleted message and leave the chat permanently unread. Highest by id, not
+        // newest by timestamp: the unread count compares ids, so a timestamp pick lands short.
         val messageId = messageDataSource.getLatestMessageId(chatId)
             ?: stateHolder.current.feed
                 ?.firstOrNull { it.chatId == chatId }

apps/flipcash/shared/persistence/db/src/commonMain/kotlin/com/flipcash/app/persistence/dao/ChatMessageDao.kt

@@ -59,6 +59,18 @@ interface ChatMessageDao {
     @Query("SELECT * FROM chat_messages WHERE chat_id_hex = :chatIdHex ORDER BY timestamp_epoch_ms DESC LIMIT 1")
     suspend fun getLatest(chatIdHex: String): ChatMessageEntity?
 
+    /**
+     * The highest server-assigned message id stored for [chatIdHex], tombstones and empty rows
+     * included, or `null` when there is none. Mark as Read moves READ here.
+     *
+     * By id, not by timestamp: the unread count compares the READ pointer by id against the newest
+     * visible message, so an anchor picked by timestamp lands short whenever the two orders disagree
+     * (two messages in one millisecond, or an unsent placeholder stamped with a device clock that
+     * runs ahead). Placeholders carry negative ids and are excluded; the server never saw them.
+     */
+    @Query("SELECT MAX(message_id) FROM chat_messages WHERE chat_id_hex = :chatIdHex AND message_id > 0")
+    suspend fun getNewestMessageId(chatIdHex: String): Long?
+
     /** The lowest message id stored for [chatIdHex], where paging back toward older history resumes. */
     @Query("SELECT MIN(message_id) FROM chat_messages WHERE chat_id_hex = :chatIdHex")
     suspend fun getOldestMessageId(chatIdHex: String): Long?
@@ -70,9 +82,9 @@ interface ChatMessageDao {
      *
      * This is what the conversation list previews and what its unread check reads, so deleting the
      * newest message falls the row back to the one before it instead of reading "Message deleted".
-     * Deliberately separate from [getLatest]: identity-keyed anchors (mark-read, receive buzz) need
-     * the newest id including tombstones, or a delete would regress the read pointer and leave the
-     * chat unread forever.
+     * Deliberately separate from the identity-keyed anchors ([getNewestMessageId] for mark-read,
+     * [getLatest] for the receive buzz): they need the newest id including tombstones, or a delete
+     * would regress the read pointer and leave the chat unread forever.
      */
     @Query("SELECT * FROM chat_messages WHERE chat_id_hex = :chatIdHex AND is_deleted = 0 AND content_json IS NOT NULL AND content_json != '[]' AND (encryption_state IS NULL OR encryption_state != 'KEY_PENDING') " + NOT_REQUEST_SHARE + "ORDER BY timestamp_epoch_ms DESC, message_id DESC LIMIT 1")
     suspend fun getLatestVisible(chatIdHex: String): ChatMessageEntity?

apps/flipcash/shared/persistence/sources/src/androidHostTest/kotlin/com/flipcash/app/persistence/sources/MarkReadAnchorTest.kt

@@ -0,0 +1,108 @@
+package com.flipcash.app.persistence.sources
+
+import android.content.Context
+import androidx.test.core.app.ApplicationProvider
+import com.flipcash.app.persistence.FlipcashDatabase
+import com.flipcash.app.persistence.converters.MessageContentSerialized
+import com.flipcash.app.persistence.entities.ChatMessageEntity
+import com.flipcash.app.persistence.init
+import com.flipcash.app.persistence.sources.mapper.chat.ChatEntityMapper
+import com.flipcash.services.models.chat.ChatId
+import com.getcode.utils.hexEncodedString
+import io.mockk.mockk
+import kotlinx.coroutines.test.runTest
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import kotlin.test.assertEquals
+
+/**
+ * Mark as Read from the chat list moves READ to [ChatMessageDataSource.getLatestMessageId]. The
+ * list's unread count compares the pointer by message id against the newest visible message, so
+ * the anchor has to be the highest stored id. Picking the newest row by timestamp landed short
+ * whenever timestamps and ids disagreed, and the count dropped instead of clearing.
+ */
+@RunWith(RobolectricTestRunner::class)
+class MarkReadAnchorTest {
+
+    private val mapper = ChatEntityMapper()
+    private val messages = ChatMessageDataSource(mapper, mockk(relaxed = true), mockk(relaxed = true))
+
+    private val chatId = ChatId(listOf(0xAB.toByte()))
+    private val otherId = listOf<Byte>(4, 5, 6)
+
+    @Before
+    fun setUp() {
+        FlipcashDatabase.init(ApplicationProvider.getApplicationContext<Context>(), "entropy-a")
+    }
+
+    @After
+    fun tearDown() {
+        FlipcashDatabase.closeDb()
+    }
+
+    private fun inbound(messageId: Long, timestampEpochMs: Long) = ChatMessageEntity(
+        chatIdHex = mapper.chatIdHex(chatId),
+        messageId = messageId,
+        senderIdHex = otherId.hexEncodedString(),
+        contentJson = listOf(MessageContentSerialized.Text("m$messageId")),
+        timestampEpochMs = timestampEpochMs,
+        unreadSeq = messageId,
+    )
+
+    private suspend fun store(vararg rows: ChatMessageEntity) {
+        FlipcashDatabase.requireInstance().chatMessageDao().upsert(rows.toList())
+    }
+
+    @Test
+    fun `the anchor is the newest message when it shares a millisecond with an older one`() = runTest {
+        // The feed writes the chat's newest message first; the history load fills in older ones
+        // afterwards, so the older row is the later insert.
+        store(inbound(messageId = 5, timestampEpochMs = 4_000))
+        store(
+            inbound(messageId = 1, timestampEpochMs = 1_000),
+            inbound(messageId = 2, timestampEpochMs = 2_000),
+            inbound(messageId = 3, timestampEpochMs = 3_000),
+            inbound(messageId = 4, timestampEpochMs = 4_000),
+        )
+
+        assertEquals(5L, messages.getLatestVisible(mapper.chatIdHex(chatId))?.messageId)
+        assertEquals(5L, messages.getLatestMessageId(chatId))
+    }
+
+    @Test
+    fun `the anchor is the highest id when an older message carries a later timestamp`() = runTest {
+        store(
+            inbound(messageId = 1, timestampEpochMs = 1_000),
+            inbound(messageId = 2, timestampEpochMs = 9_000).copy(isDeleted = true),
+            inbound(messageId = 3, timestampEpochMs = 3_000),
+        )
+
+        assertEquals(3L, messages.getLatestVisible(mapper.chatIdHex(chatId))?.messageId)
+        assertEquals(3L, messages.getLatestMessageId(chatId))
+    }
+
+    @Test
+    fun `an unsent placeholder is not an anchor`() = runTest {
+        store(
+            inbound(messageId = 1, timestampEpochMs = 1_000),
+            inbound(messageId = 2, timestampEpochMs = 2_000),
+            // A pending send is stamped with the device clock, which can run ahead of the server's.
+            inbound(messageId = -3_000, timestampEpochMs = 3_000).copy(senderIdHex = "010203"),
+        )
+
+        assertEquals(2L, messages.getLatestMessageId(chatId))
+    }
+
+    @Test
+    fun `a newest tombstone is still the anchor`() = runTest {
+        store(
+            inbound(messageId = 1, timestampEpochMs = 1_000),
+            inbound(messageId = 2, timestampEpochMs = 2_000).copy(isDeleted = true),
+        )
+
+        assertEquals(2L, messages.getLatestMessageId(chatId))
+    }
+}

apps/flipcash/shared/persistence/sources/src/commonMain/kotlin/com/flipcash/app/persistence/sources/ChatMessageDataSource.kt

@@ -158,8 +158,9 @@ class ChatMessageDataSource @Inject constructor(
     suspend fun hasMessages(chatId: ChatId): Boolean =
         db?.chatMessageDao()?.getLatest(mapper.chatIdHex(chatId)) != null
 
+    /** The highest stored server message id in [chatId]; see [ChatMessageDao.getNewestMessageId]. */
     suspend fun getLatestMessageId(chatId: ChatId): Long? =
-        db?.chatMessageDao()?.getLatest(mapper.chatIdHex(chatId))?.messageId
+        db?.chatMessageDao()?.getNewestMessageId(mapper.chatIdHex(chatId))
 
     /** The locally-stored copy of a single message, or `null` if this device has never seen it. */
     suspend fun getMessage(chatId: ChatId, messageId: Long): ChatMessage? =