fix(chat): keep historical cache out of the live transcript tail (#1253)

2026-09-24 21:27:39 +03:00
1 parent c581fd23d1
commit 94ec95a33c
@@ -1,6 +1,7 @@
 package com.m57.hermescontrol.data.local

 import com.m57.hermescontrol.ui.chat.ChatMessage
+import com.m57.hermescontrol.ui.chat.MessageProvenance
 import com.m57.hermescontrol.ui.chat.MessageRole
 import com.m57.hermescontrol.ui.chat.ToolStatus
 import org.junit.Assert.assertEquals
@@ -187,6 +188,32 @@ class ChatMessageMapperTest {
        assertEquals(MessageRole.USER, roundTripped.role)
    }

+    @Test
+    fun roundTripPreservesLocalPendingProvenance() {
+        val roundTripped =
+            ChatMessage(
+                id = "pending",
+                role = MessageRole.USER,
+                content = "not submitted",
+                messageProvenance = MessageProvenance.LOCAL_PENDING,
+            ).toEntity("s").toUiModel()
+        assertEquals(MessageProvenance.LOCAL_PENDING, roundTripped.messageProvenance)
+    }
+
+    @Test
+    fun unknownPersistedProvenanceDoesNotInventDeliveryEvidence() {
+        val entity =
+            ChatMessageEntity(
+                id = "legacy",
+                sessionId = "s",
+                role = "USER",
+                content = "ambiguous",
+                timestamp = 1L,
+                messageProvenance = "future-value",
+            )
+        assertEquals(MessageProvenance.UNKNOWN, entity.toUiModel().messageProvenance)
+    }
+
    @Test
    fun roundTripWithoutDisplayKindStaysNull() {
        val ui =
@@ -97,4 +97,16 @@ class HermesDatabaseMigrationsTest {
                )
            }
        }
+
+    @Test
+    fun migration9to10_addsDurableProvenanceWithoutInventingDeliveryState() =
+        runBlocking {
+            val connection = mockk<SQLiteConnection>(relaxed = true)
+            HermesDatabase.MIGRATION_9_10.migrate(connection)
+            verify(exactly = 1) {
+                connection.execSQL(
+                    "ALTER TABLE chat_messages ADD COLUMN message_provenance TEXT NOT NULL DEFAULT 'UNKNOWN'",
+                )
+            }
+        }
 }
@@ -0,0 +1,155 @@
+package com.m57.hermescontrol.ui.chat
+
+import com.m57.hermescontrol.data.model.Attachment
+import com.m57.hermescontrol.data.ws.WsEvent
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class ChatCachedHistoryTest {
+    private val historicalTool =
+        ChatMessage(
+            id = "cached-tool",
+            role = MessageRole.TOOL,
+            content = "{}",
+            toolName = "terminal",
+            toolCallId = "call-old",
+            toolStatus = ToolStatus.RUNNING,
+            isHistoricalCache = true,
+        )
+
+    @Test
+    fun exactToolIdPromotesOnlyItsHistoricalIdentity() {
+        val other = historicalTool.copy(id = "other", toolCallId = "call-other")
+        val state = ChatUiState(currentSessionId = "s", messages = listOf(historicalTool, other))
+        val events =
+            listOf(
+                WsEvent.ToolProgress(name = "terminal", preview = "working", sessionId = "s", toolId = "call-old"),
+                WsEvent.ToolGenerating(name = "terminal", sessionId = "s", toolId = "call-old"),
+                WsEvent.ToolComplete(name = "terminal", data = mapOf("tool_id" to "call-old"), sessionId = "s"),
+            )
+        events.forEach { event ->
+            val result = ChatWsEventReducer.reduce(state, StreamingState(), event, "s")
+            assertFalse(
+                "$event must supersede historical provenance",
+                result.state.messages
+                    .first()
+                    .isHistoricalCache,
+            )
+            assertTrue(
+                result.state.messages
+                    .last()
+                    .isHistoricalCache,
+            )
+        }
+    }
+
+    @Test
+    fun toolProgressWithNonmatchingExplicitIdDoesNotPromoteHistoricalSameNameCall() {
+        val state = ChatUiState(currentSessionId = "s", messages = listOf(historicalTool))
+
+        val result =
+            ChatWsEventReducer.reduce(
+                state,
+                StreamingState(),
+                WsEvent.ToolProgress(name = "terminal", preview = "new call", sessionId = "s", toolId = "call-new"),
+                "s",
+            )
+
+        assertEquals(listOf(historicalTool), result.state.messages)
+    }
+
+    @Test
+    fun toolCompleteWithNonmatchingExplicitIdDoesNotOverwriteHistoricalSameNameCall() {
+        val state = ChatUiState(currentSessionId = "s", messages = listOf(historicalTool))
+
+        val result =
+            ChatWsEventReducer.reduce(
+                state,
+                StreamingState(),
+                WsEvent.ToolComplete(
+                    name = "terminal",
+                    data = mapOf("tool_id" to "call-new", "output" to "new call"),
+                    sessionId = "s",
+                ),
+                "s",
+            )
+
+        assertEquals(listOf(historicalTool), result.state.messages)
+        assertTrue(result.effects.isEmpty())
+    }
+
+    @Test
+    fun legacyIdlessToolEventsStillPromoteHistoricalSameNameCall() {
+        val state = ChatUiState(currentSessionId = "s", messages = listOf(historicalTool))
+        val events =
+            listOf(
+                WsEvent.ToolProgress(name = "terminal", preview = "legacy", sessionId = "s"),
+                WsEvent.ToolComplete(name = "terminal", data = mapOf("output" to "legacy"), sessionId = "s"),
+            )
+
+        events.forEach { event ->
+            val result = ChatWsEventReducer.reduce(state, StreamingState(), event, "s")
+            assertFalse(
+                "$event must retain the legacy name fallback",
+                result.state.messages
+                    .single()
+                    .isHistoricalCache,
+            )
+        }
+    }
+
+    @Test
+    fun lateCacheCannotDemoteAnObservedLiveIdentity() {
+        val live = historicalTool.copy(isHistoricalCache = false, progressPreview = "new progress")
+        val merged = mergeCachedTranscriptPage(listOf(historicalTool), listOf(live))
+        assertEquals(listOf(live), merged)
+    }
+
+    @Test
+    fun cachedSessionStartMarkerStaysFirstBeforeHistoricalUuidAndCanonicalRestWindow() {
+        val start =
+            ChatMessage(
+                id = "cached-session-start",
+                role = MessageRole.SYSTEM,
+                content = "Session created",
+                isHistoricalCache = true,
+            )
+        val historicalUser =
+            ChatMessage(
+                id = "5df43b6f-98ea-46db-9fc4-d1e4cf11d43d",
+                role = MessageRole.USER,
+                content = "older cached prompt",
+                isHistoricalCache = true,
+            )
+        val canonical = ChatMessage(id = "rest-s-0", role = MessageRole.ASSISTANT, content = "canonical reply")
+
+        val merged = mergeTranscriptWithLive(listOf(canonical), listOf(start, historicalUser))
+
+        assertEquals(listOf(start.id, historicalUser.id, canonical.id), merged.map { it.id })
+    }
+
+    @Test
+    fun historicalRowsPrecedeCanonicalWindowWithoutChangingLiveTailOrAttachments() {
+        val attachment = Attachment("content://test/file", "file.txt", "text/plain")
+        val old =
+            ChatMessage(
+                id = "cached-user",
+                role = MessageRole.USER,
+                content = "old",
+                timestamp = Long.MAX_VALUE,
+                attachments = listOf(attachment),
+                isHistoricalCache = true,
+            )
+        val pending = old.copy(id = "pending", content = "pending", isHistoricalCache = false)
+        val recent = ChatMessage(id = "rest-s-100", role = MessageRole.ASSISTANT, content = "recent", timestamp = 1)
+        val live = historicalTool.copy(id = "live-tool", toolCallId = "call-live", isHistoricalCache = false)
+        val resume = ChatMessage(id = "resume", role = MessageRole.SYSTEM, content = "Session resumed")
+        val merged = mergeTranscriptWithLive(listOf(recent), listOf(old, historicalTool, pending, live, resume))
+        assertEquals(listOf(old.id, historicalTool.id, recent.id, pending.id, live.id, resume.id), merged.map { it.id })
+        assertEquals(listOf(attachment), merged.first().attachments)
+        assertEquals(listOf(attachment), merged.single { it.id == pending.id }.attachments)
+        assertEquals(merged, mergeTranscriptWithLive(listOf(recent), merged))
+    }
+}
@@ -12,6 +12,63 @@ import org.junit.Assert.assertTrue
 import org.junit.Test

 class ChatPagingMergeTest {
+    @Test
+    fun lateCachedRunningToolCannotReplaceCanonicalServerResult() {
+        val canonical =
+            ChatMessage(
+                id = "rest-s-42",
+                role = MessageRole.TOOL,
+                content = """{"output":"server result","exit_code":0}""",
+                toolCallId = "call-42",
+                toolStatus = ToolStatus.COMPLETED,
+            )
+        val cached =
+            ChatMessage(
+                id = "cached-tool",
+                role = MessageRole.TOOL,
+                content = """{"name":"terminal","args":{"command":"pwd"}}""",
+                toolName = "terminal",
+                toolCallId = "call-42",
+                toolStatus = ToolStatus.RUNNING,
+                isHistoricalCache = true,
+                tokenCount = 7,
+                tps = 2.5,
+                finishTimestamp = 123L,
+            )
+
+        val merged = mergeCachedTranscriptPage(listOf(cached), listOf(canonical)).single()
+
+        assertEquals(canonical.id, merged.id)
+        assertEquals(canonical.content, merged.content)
+        assertEquals(ToolStatus.COMPLETED, merged.toolStatus)
+        assertEquals("terminal", merged.toolName)
+        assertEquals("call-42", merged.toolCallId)
+        assertEquals(cached.tokenCount, merged.tokenCount)
+        assertEquals(cached.tps, merged.tps)
+        assertEquals(cached.finishTimestamp, merged.finishTimestamp)
+    }
+
+    @Test
+    fun canonicalResultSettlesHistoricalRunningToolWithServerOutput() {
+        val cached =
+            ChatMessage(
+                id = "old-tool",
+                role = MessageRole.TOOL,
+                content = """{"name":"terminal","args":{"command":"pwd"}}""",
+                toolName = "terminal",
+                toolCallId = "call-42",
+                toolStatus = ToolStatus.RUNNING,
+                isHistoricalCache = true,
+            )
+        val page = mapServerMessages("s", listOf(serverTool(42, "call-42")), 0, true, listOf(cached))
+        val merged = mergeTranscriptWithLive(page, listOf(cached), preserveLiveIds = true).single()
+        assertEquals(cached.id, merged.id)
+        assertEquals("rest-s-42", merged.canonicalRestId)
+        assertEquals(ToolStatus.COMPLETED, merged.toolStatus)
+        assertTrue(merged.content.contains("exit_code"))
+        assertTrue(!merged.isHistoricalCache)
+    }
+
    @Test
    fun confirmedLiveOccurrenceCannotConsumeEarlierIdenticalPage() {
        for (role in listOf(MessageRole.USER, MessageRole.ASSISTANT, MessageRole.TOOL)) {
@@ -5389,6 +5389,200 @@ class ChatViewModelTest {
            )
        }

+    @Test
+    fun paging_staleCacheBeforeRestDoesNotBecomeTheLatestTail() =
+        runTest {
+            val cachedIds = seedPagingCache(3)
+            val response = CompletableDeferred<retrofit2.Response<SessionMessagesResponse>>()
+            val (viewModel, _) = createViewModelWithSession()
+            coEvery {
+                ApiClient.hermesApi.getSessionMessages("session-456", any(), any(), any(), any())
+            } coAnswers { response.await() }
+
+            viewModel.switchSession("session-456")
+            runCurrent()
+            assertEquals(
+                cachedIds,
+                viewModel.uiState.value.messages
+                    .map { it.id },
+            )
+            response.complete(pagingResponse(100..102))
+            advanceUntilIdle()
+
+            val expected = cachedIds + (100..102).map { "rest-session-456-$it" }
+            assertEquals(
+                expected,
+                viewModel.uiState.value.messages
+                    .map { it.id },
+            )
+            viewModel.syncCurrentSession()
+            advanceUntilIdle()
+            assertEquals(
+                expected,
+                viewModel.uiState.value.messages
+                    .map { it.id },
+            )
+            assertTrue(fakeRepo.dao.idsForSession("session-456").containsAll(cachedIds))
+        }
+
+    @Test
+    fun paging_staleCacheAfterRestDoesNotBecomeTheLatestTail() =
+        runTest {
+            val cachedIds = seedPagingCache(3)
+            val cacheRead = CompletableDeferred<Unit>()
+            fakeRepo.dao.beforeRead = { cacheRead.await() }
+            val (viewModel, _) = createViewModelWithSession()
+            coEvery {
+                ApiClient.hermesApi.getSessionMessages("session-456", any(), any(), any(), any())
+            } returns pagingResponse(100..102)
+
+            viewModel.switchSession("session-456")
+            runCurrent()
+            cacheRead.complete(Unit)
+            advanceUntilIdle()
+
+            assertEquals(
+                cachedIds + (100..102).map { "rest-session-456-$it" },
+                viewModel.uiState.value.messages
+                    .map { it.id },
+            )
+        }
+
+    @Test
+    fun paging_coldRestartPendingLocalRemainsAfterCanonicalWindow() =
+        runTest {
+            val pending =
+                ChatMessage(
+                    id = "pending-local",
+                    role = MessageRole.USER,
+                    content = "not delivered yet",
+                    messageProvenance = MessageProvenance.LOCAL_PENDING,
+                )
+            fakeRepo.persistMessage(pending, "session-456")
+            val cacheRead = CompletableDeferred<Unit>()
+            fakeRepo.dao.beforeRead = { cacheRead.await() }
+            val (viewModel, _) = createViewModelWithSession()
+            coEvery {
+                ApiClient.hermesApi.getSessionMessages("session-456", any(), any(), any(), any())
+            } returns pagingResponse(100..102)
+
+            viewModel.switchSession("session-456")
+            runCurrent()
+            cacheRead.complete(Unit)
+            advanceUntilIdle()
+
+            assertEquals(
+                (100..102).map { "rest-session-456-$it" } + pending.id,
+                viewModel.uiState.value.messages
+                    .map { it.id },
+            )
+        }
+
+    @Test
+    fun paging_coldRestartLegacyUnknownUserRemainsConservativelyUnconfirmed() =
+        runTest {
+            val ambiguous =
+                ChatMessage(
+                    id = "legacy-unknown-user",
+                    role = MessageRole.USER,
+                    content = "possibly unsent before migration",
+                )
+            fakeRepo.persistMessage(ambiguous, "session-456")
+            val cacheRead = CompletableDeferred<Unit>()
+            fakeRepo.dao.beforeRead = { cacheRead.await() }
+            val (viewModel, _) = createViewModelWithSession()
+            coEvery {
+                ApiClient.hermesApi.getSessionMessages("session-456", any(), any(), any(), any())
+            } returns pagingResponse(100..102)
+            viewModel.switchSession("session-456")
+            runCurrent()
+            cacheRead.complete(Unit)
+            advanceUntilIdle()
+            assertEquals(
+                (100..102).map { "rest-session-456-$it" } + ambiguous.id,
+                viewModel.uiState.value.messages
+                    .map { it.id },
+            )
+            assertFalse(
+                viewModel.uiState.value.messages
+                    .last()
+                    .isHistoricalCache,
+            )
+        }
+
+    @Test
+    fun paging_coldRestartPermanentLocalKeepsLocalOrdering() =
+        runTest {
+            val localCommand =
+                ChatMessage(
+                    id = "local-command",
+                    role = MessageRole.USER,
+                    content = "/help",
+                )
+            fakeRepo.persistMessage(localCommand, "session-456")
+            val cacheRead = CompletableDeferred<Unit>()
+            fakeRepo.dao.beforeRead = { cacheRead.await() }
+            val (viewModel, _) = createViewModelWithSession()
+            coEvery {
+                ApiClient.hermesApi.getSessionMessages("session-456", any(), any(), any(), any())
+            } returns pagingResponse(100..102)
+
+            viewModel.switchSession("session-456")
+            runCurrent()
+            cacheRead.complete(Unit)
+            advanceUntilIdle()
+
+            assertEquals(
+                (100..102).map { "rest-session-456-$it" } + localCommand.id,
+                viewModel.uiState.value.messages
+                    .map { it.id },
+            )
+        }
+
+    @Test
+    fun paging_staleCacheDoesNotDemoteOptimisticIdentity() =
+        runTest {
+            val cachedIds = seedPagingCache(3)
+            val cacheRead = CompletableDeferred<Unit>()
+            fakeRepo.dao.beforeRead = { cacheRead.await() }
+            val (viewModel, _) = createViewModelWithSession()
+            coEvery {
+                ApiClient.hermesApi.getSessionMessages("session-456", any(), any(), any(), any())
+            } returns pagingResponse(100..102)
+
+            viewModel.switchSession("session-456")
+            runCurrent()
+            val attachment = Attachment("content://test/file", "file.txt", "text/plain")
+            val optimistic =
+                ChatMessage(
+                    id = cachedIds[1],
+                    role = MessageRole.USER,
+                    content = "Cached message 2",
+                    attachments = listOf(attachment),
+                )
+
+            @Suppress("UNCHECKED_CAST")
+            val state =
+                ChatViewModel::class.java
+                    .getDeclaredField("_uiState")
+                    .apply { isAccessible = true }
+                    .get(viewModel) as MutableStateFlow<Any>
+            state.value = viewModel.uiState.value.copy(messages = viewModel.uiState.value.messages + optimistic)
+            cacheRead.complete(Unit)
+            advanceUntilIdle()
+
+            assertEquals(
+                listOf(cachedIds[0], cachedIds[2]) + (100..102).map { "rest-session-456-$it" } + cachedIds[1],
+                viewModel.uiState.value.messages
+                    .map { it.id },
+            )
+            val retained =
+                viewModel.uiState.value.messages
+                    .last()
+            assertFalse(retained.isHistoricalCache)
+            assertEquals(listOf(attachment), retained.attachments)
+        }
+
    // Chat paging: deterministic regressions, with no real network or wall-clock timing.
    private fun seedPagingCache(
        count: Int,
@@ -26,6 +26,21 @@ class FullBleedTurnsTest {

    private fun entries(vararg e: AgentEntry) = ChatTurn.Agent(e.toList())

+    @Test
+    fun historicalRunningToolDoesNotClaimLiveActivity() {
+        val messages =
+            listOf(
+                msg("user", MessageRole.USER),
+                msg("old-tool", MessageRole.TOOL, toolStatus = ToolStatus.RUNNING)
+                    .copy(toolName = "terminal", isHistoricalCache = true),
+            )
+        assertEquals(AgentStatus.Typing, deriveAgentStatus(true, StreamingState(), messages))
+        assertEquals(
+            AgentStatus.Tool("terminal"),
+            deriveAgentStatus(true, StreamingState(), messages.map { it.copy(isHistoricalCache = false) }),
+        )
+    }
+
    @Test
    fun `empty list produces no turns`() {
        assertEquals(emptyList<ChatTurn>(), groupIntoTurns(emptyList()))