fix: break the transcript's same-second tie on write order, so a burst reads forwards
`ChatMessageDao`'s two transcript queries ordered `createdAt DESC` and nothing
else. `MantraConverters` stores an `Instant` as epoch seconds, so lines written
inside one second tie -- a ceremony puts a dozen into a room faster than that,
and a request with the answer it triggers routinely lands inside one -- and with
no second key SQLite hands them back in scan order, which is rowid *ascending*.
Under a descending query drawn bottom-up by the feed's `reverseLayout`, that
draws a same-second burst backwards. Three lines written in one second, read
back through the old query:
expected:<[third, second, first]> but was:<[first, second, third]>
**The list and the room it opens already disagreed.** `ChatRoomDao` picks each
room's preview with `ORDER BY createdAt DESC, id DESC`, and 50feb6f justified
that key as "the order the transcript shows them in, so the list and the room it
opens agree about what was said last". The transcript never had the key, so that
was not true of it: under a tie the chat list quoted the last-written line while
the room put the first-written one above the composer.
`id DESC` on both reads settles it, and one constant now carries them both --
the snapshot a screen loads with and the flow it then follows are the same list,
and must not order it differently.
**The chronicle test was asserting the ascending order.** `an answered catch-up
leaves one line, whatever it took to deliver` expected [requested, received] out
of a newest-first query. It passed only by riding the tie's rowid order, and
failed whenever the two writes straddled a second boundary -- one run in eight on
a clean tree. Its expectation is now the newest-first order the DAO promises,
which the tie-break makes deterministic rather than a coin flip.
One new DAO test, covering the same-second burst through both the snapshot and
the flow the open room actually reads.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -8,14 +8,35 @@ import press.mantra.compose.database.model.ChatMessage
|
||||
import com.vitorpamplona.quartz.nip01Core.core.HexKey
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
|
||||
/**
|
||||
* The room's transcript, newest first -- the order the feed's `reverseLayout` draws from the
|
||||
* bottom up, so the first row is the line sitting above the composer.
|
||||
*
|
||||
* `id DESC` is not decoration. [press.mantra.compose.database.converters.MantraConverters]
|
||||
* stores an `Instant` as epoch seconds, so lines written inside one second tie: a ceremony
|
||||
* puts a dozen into a room faster than that, and a request with the answer it triggers
|
||||
* routinely lands inside one. `createdAt DESC` alone left those to whatever order the sorter
|
||||
* produced -- the rows as scanned, which under a descending query drew a same-second burst
|
||||
* backwards, and is free to change under a new index or a different plan, swapping two lines
|
||||
* in a transcript the user has already read.
|
||||
*
|
||||
* The key [ChatRoomDao] already breaks its last-message tie on, which is what makes the chat
|
||||
* list's preview and the bottom of the room it opens name the same line.
|
||||
*
|
||||
* One constant for both reads because they are the same list: the snapshot a screen loads
|
||||
* with and the flow it then follows must not order it differently.
|
||||
*/
|
||||
private const val TRANSCRIPT =
|
||||
"SELECT * FROM ChatMessage WHERE chatRoomId = :chatRoomId ORDER BY createdAt DESC, id DESC"
|
||||
|
||||
@Dao
|
||||
interface ChatMessageDao {
|
||||
@Transaction
|
||||
@Query("SELECT * FROM ChatMessage WHERE chatRoomId = :chatRoomId ORDER BY createdAt DESC")
|
||||
@Query(TRANSCRIPT)
|
||||
fun observeChatMessagesByChatRoomId(chatRoomId: String): Flow<List<press.mantra.compose.database.model.intermdiate.LocalChatMessage>>
|
||||
|
||||
@Transaction
|
||||
@Query("SELECT * FROM ChatMessage WHERE chatRoomId = :chatRoomId ORDER BY createdAt DESC")
|
||||
@Query(TRANSCRIPT)
|
||||
suspend fun getChatMessagesByChatRoomId(chatRoomId: String): List<press.mantra.compose.database.model.intermdiate.LocalChatMessage>
|
||||
|
||||
@Query("SELECT * FROM ChatMessage WHERE giftWrapPayloadId = :giftWrapPayloadId ORDER BY createdAt DESC")
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package press.mantra.compose.database.dao
|
||||
|
||||
import androidx.room3.Room
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import press.mantra.compose.database.MantraDatabase
|
||||
import press.mantra.compose.database.builder.getRoomDatabase
|
||||
@@ -226,6 +227,39 @@ class ChatMessageDaoJvmTest {
|
||||
assertEquals("newer", db.chatMessageDao().getChatMessagesByMarmotGroupEventId(eventId)?.content)
|
||||
}
|
||||
|
||||
/**
|
||||
* Timestamps are stored to the second, so a burst written in one second ties -- a ceremony
|
||||
* puts a dozen lines into a room faster than that, and a request with the answer it triggers
|
||||
* routinely lands inside one. With nothing to break the tie the transcript came back in
|
||||
* whatever order SQLite's sorter produced, which under a descending query drew a same-second
|
||||
* burst backwards and was free to change between two reads of the same room.
|
||||
*
|
||||
* `id DESC` is the key `ChatRoomDao` already breaks its last-message tie on, so the chat
|
||||
* list's preview and the bottom of the room it opens name the same line.
|
||||
*/
|
||||
@Test
|
||||
fun `lines written in the same second come back in write order`() = runBlocking {
|
||||
seedRooms()
|
||||
val burst = Instant.fromEpochSeconds(1_000)
|
||||
line(groupEventId = null, content = "first", createdAt = burst)
|
||||
line(groupEventId = null, content = "second", createdAt = burst)
|
||||
line(groupEventId = null, content = "third", createdAt = burst)
|
||||
|
||||
// Newest first, which is the order `reverseLayout` draws from the bottom up.
|
||||
assertEquals(
|
||||
listOf("third", "second", "first"),
|
||||
db.chatMessageDao().getChatMessagesByChatRoomId(roomOne).map { it.chatMessage.content }
|
||||
)
|
||||
|
||||
// The flow is what the open room actually reads, and it must not order differently
|
||||
// from the snapshot the same screen loads with.
|
||||
assertEquals(
|
||||
listOf("third", "second", "first"),
|
||||
db.chatMessageDao().observeChatMessagesByChatRoomId(roomOne).first()
|
||||
.map { it.chatMessage.content }
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a senders lines are counted per room`() = runBlocking {
|
||||
seedRooms()
|
||||
|
||||
@@ -963,8 +963,12 @@ class ChronicleApplyJvmTest {
|
||||
)
|
||||
}
|
||||
|
||||
// Newest first, which is the transcript's order: the answer is the line above
|
||||
// the composer and the ask sits over it. Both land in the same second, so what
|
||||
// decides between them is `ChatMessageDao`'s `id DESC` rather than the sorter --
|
||||
// without it this read whichever way SQLite happened to hand the tie back.
|
||||
assertEquals(
|
||||
listOf(ChatMessage.TYPE_CHRONICLE_REQUESTED, ChatMessage.TYPE_CHRONICLE_RECEIVED),
|
||||
listOf(ChatMessage.TYPE_CHRONICLE_RECEIVED, ChatMessage.TYPE_CHRONICLE_REQUESTED),
|
||||
receiver.chatMessageDao().getChatMessagesByChatRoomId(chatRoomId)
|
||||
.map { it.chatMessage.messageType }
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user