From 94ec95a33cdeb9432ad8dd9351c1eefcfbed219b Mon Sep 17 00:00:00 2001 From: AlexDev_ <56083016+alexdev03@users.noreply.github.com> Date: Thu, 24 Sep 2026 20:27:39 +0200 Subject: [PATCH] fix(chat): keep historical cache out of the live transcript tail (#1253) --- .../10.json | 143 +++++++++++++ .../data/local/ChatMessagePagingDeviceTest.kt | 82 +++++++- .../data/local/ChatMessageEntity.kt | 2 + .../data/local/ChatMessageMapper.kt | 5 + .../data/local/HermesDatabase.kt | 14 +- .../m57/hermescontrol/ui/chat/ChatMessage.kt | 14 ++ .../ui/chat/ChatServerMessageMapper.kt | 13 +- .../ui/chat/ChatTranscriptMerge.kt | 38 +++- .../hermescontrol/ui/chat/ChatViewModel.kt | 26 ++- .../ui/chat/ChatWsEventReducer.kt | 14 +- .../m57/hermescontrol/ui/chat/ToolBubble.kt | 8 +- .../ui/chat/fullbleed/AgentStatusIndicator.kt | 4 +- .../data/local/ChatMessageMapperTest.kt | 27 +++ .../local/HermesDatabaseMigrationsTest.kt | 12 ++ .../ui/chat/ChatCachedHistoryTest.kt | 155 ++++++++++++++ .../ui/chat/ChatPagingMergeTest.kt | 57 +++++ .../ui/chat/ChatViewModelTest.kt | 194 ++++++++++++++++++ .../ui/chat/FullBleedTurnsTest.kt | 15 ++ 18 files changed, 805 insertions(+), 18 deletions(-) create mode 100644 app/schemas/com.m57.hermescontrol.data.local.HermesDatabase/10.json create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCachedHistoryTest.kt diff --git a/app/schemas/com.m57.hermescontrol.data.local.HermesDatabase/10.json b/app/schemas/com.m57.hermescontrol.data.local.HermesDatabase/10.json new file mode 100644 index 00000000..8c91d72b --- /dev/null +++ b/app/schemas/com.m57.hermescontrol.data.local.HermesDatabase/10.json @@ -0,0 +1,143 @@ +{ + "formatVersion": 1, + "database": { + "version": 10, + "identityHash": "1b554126b4f6033e34af91670253c1b9", + "entities": [ + { + "tableName": "chat_messages", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `session_id` TEXT NOT NULL, `role` TEXT NOT NULL, `content` TEXT NOT NULL, `reasoning_text` TEXT NOT NULL, `timestamp` INTEGER NOT NULL, `tool_name` TEXT, `tool_call_id` TEXT NOT NULL, `tool_status` TEXT, `is_streaming` INTEGER NOT NULL, `display_kind` TEXT, `token_count` INTEGER, `tps` REAL, `completion_id` TEXT, `rest_id` TEXT, `sort_group` INTEGER NOT NULL DEFAULT 1, `sort_order` INTEGER NOT NULL DEFAULT 0, `message_provenance` TEXT NOT NULL DEFAULT 'UNKNOWN', PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sessionId", + "columnName": "session_id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "role", + "columnName": "role", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "content", + "columnName": "content", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "reasoningText", + "columnName": "reasoning_text", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "timestamp", + "columnName": "timestamp", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "toolName", + "columnName": "tool_name", + "affinity": "TEXT" + }, + { + "fieldPath": "toolCallId", + "columnName": "tool_call_id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "toolStatus", + "columnName": "tool_status", + "affinity": "TEXT" + }, + { + "fieldPath": "isStreaming", + "columnName": "is_streaming", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "displayKind", + "columnName": "display_kind", + "affinity": "TEXT" + }, + { + "fieldPath": "tokenCount", + "columnName": "token_count", + "affinity": "INTEGER" + }, + { + "fieldPath": "tps", + "columnName": "tps", + "affinity": "REAL" + }, + { + "fieldPath": "completionId", + "columnName": "completion_id", + "affinity": "TEXT" + }, + { + "fieldPath": "restId", + "columnName": "rest_id", + "affinity": "TEXT" + }, + { + "fieldPath": "sortGroup", + "columnName": "sort_group", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "1" + }, + { + "fieldPath": "sortOrder", + "columnName": "sort_order", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "messageProvenance", + "columnName": "message_provenance", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'UNKNOWN'" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_chat_messages_session_id_sort_group_sort_order_id", + "unique": false, + "columnNames": [ + "session_id", + "sort_group", + "sort_order", + "id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_chat_messages_session_id_sort_group_sort_order_id` ON `${TABLE_NAME}` (`session_id`, `sort_group`, `sort_order`, `id`)" + } + ] + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '1b554126b4f6033e34af91670253c1b9')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/java/com/m57/hermescontrol/data/local/ChatMessagePagingDeviceTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/data/local/ChatMessagePagingDeviceTest.kt index 795859bb..2340f16f 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/data/local/ChatMessagePagingDeviceTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/data/local/ChatMessagePagingDeviceTest.kt @@ -41,7 +41,7 @@ class ChatMessagePagingDeviceTest { Room .databaseBuilder(context, HermesDatabase::class.java, databaseName) .setDriver(driver()) - .addMigrations(HermesDatabase.MIGRATION_8_9) + .addMigrations(HermesDatabase.MIGRATION_8_9, HermesDatabase.MIGRATION_9_10) .build() repository = ChatPersistenceRepository(database.chatMessageDao()) } @@ -234,6 +234,29 @@ class ChatMessagePagingDeviceTest { assertPagingIndexUsed() } + @Test + fun migration9to10PreservesAmbiguousUnsentUuidUserAsUnknown() = + runBlocking { + val unsent = + ChatMessageEntity( + id = "pending-before-upgrade", + sessionId = "session", + role = "USER", + content = "may not have reached the server", + timestamp = 1L, + sortOrder = 1L, + ) + createVersion9(listOf(unsent)) + openDatabase() + val restored = database.chatMessageDao().getMessage(unsent.id)!! + assertEquals("UNKNOWN", restored.messageProvenance) + assertEquals( + com.m57.hermescontrol.ui.chat.MessageProvenance.UNKNOWN, + restored.toUiModel().messageProvenance, + ) + assertEquals(unsent.content, restored.content) + } + @Test fun confirmedAliasPreservesLivePayloadAndCanonicalOrderAcrossReopen() = runBlocking { @@ -350,6 +373,63 @@ class ChatMessagePagingDeviceTest { } } + /** Build the exported v9 schema so Room validates the real non-destructive v9 -> v10 path. */ + private fun createVersion9(rows: List) { + database.close() + context.deleteDatabase(databaseName) + val schema = + InstrumentationRegistry + .getInstrumentation() + .context.assets + .open("com.m57.hermescontrol.data.local.HermesDatabase/9.json") + .bufferedReader() + .use { JSONObject(it.readText()).getJSONObject("database") } + val file = context.getDatabasePath(databaseName) + file.parentFile?.mkdirs() + driver().open(file.absolutePath).use { connection -> + val entity = schema.getJSONArray("entities").getJSONObject(0) + connection.execSQL(entity.getString("createSql").replace("\${TABLE_NAME}", "chat_messages")) + val indices = entity.getJSONArray("indices") + for (index in 0 until indices.length()) { + connection.execSQL( + indices.getJSONObject(index).getString("createSql").replace("\${TABLE_NAME}", "chat_messages"), + ) + } + val setup = schema.getJSONArray("setupQueries") + for (index in 0 until setup.length()) connection.execSQL(setup.getString(index)) + connection + .prepare( + "INSERT INTO chat_messages (id, session_id, role, content, reasoning_text, timestamp, " + + "tool_name, tool_call_id, tool_status, is_streaming, display_kind, token_count, tps, " + + "completion_id, rest_id, sort_group, sort_order) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + ).use { insert -> + rows.forEach { row -> + insert.bindText(1, row.id) + insert.bindText(2, row.sessionId) + insert.bindText(3, row.role) + insert.bindText(4, row.content) + insert.bindText(5, row.reasoningText) + insert.bindLong(6, row.timestamp) + row.toolName?.let { insert.bindText(7, it) } ?: insert.bindNull(7) + insert.bindText(8, row.toolCallId) + row.toolStatus?.let { insert.bindText(9, it) } ?: insert.bindNull(9) + insert.bindLong(10, if (row.isStreaming) 1L else 0L) + row.displayKind?.let { insert.bindText(11, it) } ?: insert.bindNull(11) + row.tokenCount?.let { insert.bindLong(12, it.toLong()) } ?: insert.bindNull(12) + row.tps?.let { insert.bindDouble(13, it) } ?: insert.bindNull(13) + row.completionId?.let { insert.bindText(14, it) } ?: insert.bindNull(14) + row.restId?.let { insert.bindText(15, it) } ?: insert.bindNull(15) + insert.bindLong(16, row.sortGroup.toLong()) + insert.bindLong(17, row.sortOrder) + insert.step() + insert.reset() + insert.clearBindings() + } + } + connection.execSQL("PRAGMA user_version = 9") + } + } + private fun assertPagingIndexUsed() { database.close() driver().open(context.getDatabasePath(databaseName).absolutePath).use { connection -> diff --git a/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageEntity.kt b/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageEntity.kt index 0695140d..9b645e7a 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageEntity.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageEntity.kt @@ -54,6 +54,8 @@ data class ChatMessageEntity( val sortGroup: Int = 1, @ColumnInfo(name = "sort_order", defaultValue = "0") val sortOrder: Long = 0, + @ColumnInfo(name = "message_provenance", defaultValue = "'UNKNOWN'") + val messageProvenance: String = "UNKNOWN", ) internal fun ChatMessageEntity.isSessionStartMarker(): Boolean = diff --git a/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageMapper.kt b/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageMapper.kt index 97c39b5b..5be0dd2d 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageMapper.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageMapper.kt @@ -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 @@ -40,6 +41,9 @@ fun ChatMessageEntity.toUiModel(): ChatMessage = completionId = completionId, restId = restId, localOrder = sortOrder.takeIf { sortGroup == 1 && it > 0 && !isSessionStartMarker() }, + messageProvenance = + MessageProvenance.entries.firstOrNull { it.name == messageProvenance } + ?: MessageProvenance.UNKNOWN, ) fun ChatMessage.toEntity(sessionId: String): ChatMessageEntity = @@ -59,4 +63,5 @@ fun ChatMessage.toEntity(sessionId: String): ChatMessageEntity = tps = tps, completionId = completionId, restId = restId, + messageProvenance = messageProvenance.name, ) diff --git a/app/src/main/java/com/m57/hermescontrol/data/local/HermesDatabase.kt b/app/src/main/java/com/m57/hermescontrol/data/local/HermesDatabase.kt index c30d2891..6464a569 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/local/HermesDatabase.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/local/HermesDatabase.kt @@ -14,7 +14,7 @@ import java.io.File @Database( entities = [ChatMessageEntity::class], - version = 9, + version = 10, exportSchema = true, ) abstract class HermesDatabase : RoomDatabase() { @@ -115,6 +115,17 @@ abstract class HermesDatabase : RoomDatabase() { } } + val MIGRATION_9_10: Migration = + object : Migration(9, 10) { + override suspend fun migrate(connection: SQLiteConnection) { + // v9 cannot prove whether a UUID-only row reached the server. Preserve that + // ambiguity; only new outgoing prompts record LOCAL_PENDING before submit. + connection.execSQL( + "ALTER TABLE chat_messages ADD COLUMN message_provenance TEXT NOT NULL DEFAULT 'UNKNOWN'", + ) + } + } + suspend fun get(context: Context): HermesDatabase = withContext(Dispatchers.IO) { instance?.let { return@withContext it } @@ -152,6 +163,7 @@ abstract class HermesDatabase : RoomDatabase() { MIGRATION_6_7, MIGRATION_7_8, MIGRATION_8_9, + MIGRATION_9_10, ).fallbackToDestructiveMigration(false) .build() .also { instance = it } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt index f65c6a6d..34d6f29b 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt @@ -35,6 +35,12 @@ data class ToolOutputRiskData( val redacted: Boolean, ) +/** Durable evidence about a locally-created row; UNKNOWN must not be treated as proof of delivery. */ +enum class MessageProvenance { + UNKNOWN, + LOCAL_PENDING, +} + data class ChatMessage( val id: String = UUID.randomUUID().toString(), val role: MessageRole, @@ -91,12 +97,20 @@ data class ChatMessage( val restId: String? = null, /** Cache insertion sequence for unconfirmed local rows; null before first persistence. */ val localOrder: Long? = null, + /** Persisted before prompt submission so process death cannot turn an unsent prompt into old history. */ + val messageProvenance: MessageProvenance = MessageProvenance.UNKNOWN, + /** Read from history, not observed live in this view. Never persisted as delivery state. */ + val isHistoricalCache: Boolean = false, ) /** Cached REST rows already carry their canonical identity in the persisted primary key. */ internal val ChatMessage.canonicalRestId: String? get() = restId ?: id.takeIf { it.startsWith("rest-") } +/** A persisted RUNNING snapshot is not evidence of current tool activity. */ +internal val ChatMessage.isToolRunning: Boolean + get() = toolStatus == ToolStatus.RUNNING && !isHistoricalCache + /** * Single live transcript log entry for subagent execution. */ diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt index 59327ce1..394dc888 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt @@ -227,6 +227,17 @@ internal fun mapServerMessages( val matches = matchTranscriptMessages(mappedTools, liveTools) val toolsById = mappedTools.indices.associate { index -> mappedTools[index].id to matches[index] } return mapped.map { message -> - toolsById[message.id]?.copy(restId = message.canonicalRestId) ?: message + val local = toolsById[message.id] + if (local?.isHistoricalCache == true && local.toolStatus == ToolStatus.RUNNING) { + // A canonical tool-result row settles a cached tool.start, not the reverse. + local.copy( + restId = message.canonicalRestId, + content = message.content, + toolStatus = ToolStatus.COMPLETED, + isHistoricalCache = false, + ) + } else { + local?.copy(restId = message.canonicalRestId) ?: message + } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt index cede6a9c..1eb48730 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt @@ -341,13 +341,35 @@ internal fun mergeCachedTranscriptPage( incoming .mapIndexedNotNull { index, message -> val match = matches[index] ?: return@mapIndexedNotNull null - val rich = if (match.id.startsWith("rest-") && !message.id.startsWith("rest-")) message else match + val rich = + when { + // A late historical snapshot may contribute safe metadata, but it must + // never replace a canonical server payload/status already on screen. + message.isHistoricalCache && match.canonicalRestId != null -> { + match.copy( + toolName = match.toolName ?: message.toolName, + toolCallId = match.toolCallId.ifBlank { message.toolCallId }, + attachments = match.attachments ?: message.attachments, + finishTimestamp = match.finishTimestamp ?: message.finishTimestamp, + tokenCount = match.tokenCount ?: message.tokenCount, + tps = match.tps ?: message.tps, + ) + } + + match.id.startsWith("rest-") && !message.id.startsWith("rest-") -> { + message + } + + else -> { + match + } + } match.id to rich.copy( id = match.id, - restId = message.canonicalRestId ?: match.canonicalRestId, + restId = match.canonicalRestId ?: message.canonicalRestId, completionId = match.completionId ?: message.completionId, - displayKind = message.displayKind ?: match.displayKind, + displayKind = match.displayKind ?: message.displayKind, ) }.toMap() val resolvedOrders = @@ -381,6 +403,7 @@ internal fun mergeTranscriptWithLive( match.copy( restId = (message.canonicalRestId ?: match.canonicalRestId).takeUnless { it == match.id }, displayKind = message.displayKind ?: match.displayKind, + isHistoricalCache = false, ) } @@ -398,11 +421,12 @@ internal fun mergeTranscriptWithLive( displayKind = message.displayKind ?: match.displayKind, tokenCount = message.tokenCount ?: match.tokenCount, completionId = message.completionId ?: match.completionId, + isHistoricalCache = false, ) } else -> { - message.copy(completionId = message.completionId ?: match?.completionId) + message.copy(completionId = message.completionId ?: match?.completionId, isHistoricalCache = false) } } } @@ -431,6 +455,8 @@ private fun List.inTranscriptOrder( resolvedOrders: Map, ): List { val latestCanonical = mapNotNull { it.canonicalOrder }.maxOrNull() ?: -1L + // Session-start markers use -1 and must remain before unresolved cached history. + val beforeCanonical = (mapNotNull { it.canonicalOrder }.filter { it >= 0L }.minOrNull() ?: 0L) - 1L var precedingCanonical: Long? = null var hasPendingPredecessor = false var pendingLocalOrder: Long? = null @@ -451,6 +477,10 @@ private fun List.inTranscriptOrder( ?: latestCanonical } if (hasPendingPredecessor) pendingOrderByLocal[message.id] = pendingLocalOrder ?: Long.MAX_VALUE + } else if (message.isHistoricalCache) { + // Room groups UUID-only rows after all confirmed rows; that predecessor is + // not a chronological anchor. Keep unresolved history before the server window. + localAnchors[message.id] = beforeCanonical } else { hasPendingPredecessor = true pendingLocalOrder = message.localOrder diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt index 8c8ad8a5..f1a32bfe 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt @@ -1832,6 +1832,7 @@ class ChatViewModel( content = text, attachments = if (attachments.isNotEmpty()) attachments else null, tokenCount = TokenEstimator.estimate(text).takeIf { it > 0 }, + messageProvenance = MessageProvenance.LOCAL_PENDING, ) // Update UI immediately @@ -1894,6 +1895,7 @@ class ChatViewModel( content = text, attachments = if (attachments.isNotEmpty()) attachments else null, tokenCount = TokenEstimator.estimate(text).takeIf { it > 0 }, + messageProvenance = MessageProvenance.LOCAL_PENDING, ) // Upload attachments then submit prompt @@ -3256,7 +3258,29 @@ class ChatViewModel( val snapshot = _uiState.value val computed = withContext(historyDispatcher) { - val page = mapPage(snapshot.messages) + val mapped = mapPage(snapshot.messages) + val currentById = snapshot.messages.associateBy { it.id } + // Durable provenance distinguishes new unsent prompts after process death. + // Legacy UUID-only USER rows remain conservatively unconfirmed: UNKNOWN is + // not evidence that the server delivered them. + val page = + if (cached) { + mapped.map { message -> + message.copy( + isHistoricalCache = + when { + currentById[message.id]?.isHistoricalCache == false -> false + message.isPermanentlyLocal() -> false + message.messageProvenance == MessageProvenance.LOCAL_PENDING -> false + message.canonicalRestId != null -> true + message.role == MessageRole.USER -> false + else -> true + }, + ) + } + } else { + mapped + } val merged = if (cached) { mergeCachedTranscriptPage(page, snapshot.messages) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt index 4f9088cd..7260e860 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt @@ -681,6 +681,7 @@ object ChatWsEventReducer { messages[toolIdx].copy( toolStatus = ToolStatus.COMPLETED, content = contentJson, + isHistoricalCache = false, ) messages[toolIdx] = updated @@ -928,7 +929,11 @@ object ChatWsEventReducer { val toolIdx = findToolIndex(messages, event.name, event.toolId, ToolStatus.RUNNING) if (toolIdx < 0) return ReducerResult(state = state, streamingState = streamingState) - messages[toolIdx] = messages[toolIdx].copy(progressPreview = event.preview ?: "") + messages[toolIdx] = + messages[toolIdx].copy( + progressPreview = event.preview ?: "", + isHistoricalCache = false, + ) return ReducerResult( state = state.copy(messages = messages), streamingState = streamingState, @@ -944,14 +949,14 @@ object ChatWsEventReducer { val toolIdx = findToolIndex(messages, event.name, event.toolId, ToolStatus.RUNNING) if (toolIdx < 0) return ReducerResult(state = state, streamingState = streamingState) - messages[toolIdx] = messages[toolIdx].copy(progressPreview = "") + messages[toolIdx] = messages[toolIdx].copy(progressPreview = "", isHistoricalCache = false) return ReducerResult( state = state.copy(messages = messages), streamingState = streamingState, ) } - /** Match stable tool IDs first, with name matching for legacy events. */ + /** Match an explicit stable ID exclusively; use name matching only when legacy events omit the ID. */ private fun findToolIndex( messages: List, name: String?, @@ -965,7 +970,8 @@ object ChatWsEventReducer { it.toolCallId == toolId && it.toolStatus == status } - if (byId >= 0) return byId + // An explicit identity must never claim a different historical invocation. + return byId } return messages.indexOfLast { diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ToolBubble.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ToolBubble.kt index 54a8b4d7..3eb0c605 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ToolBubble.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ToolBubble.kt @@ -143,8 +143,8 @@ internal fun ToolBubble( val statusColors = LocalHermesStatusColors.current val view = - remember(message.content, message.toolName, message.toolStatus) { - parseToolOutput(message.content, message.toolName, message.toolStatus == ToolStatus.RUNNING) + remember(message.content, message.toolName, message.isToolRunning) { + parseToolOutput(message.content, message.toolName, message.isToolRunning) } val config = ToolSchemaRegistry.getDisplayConfig(message.toolName) @@ -182,7 +182,7 @@ internal fun ToolBubble( HeaderRow(message, config, contentColor, statusColors) // ── Tool progress preview (tool.progress) ── - if (message.toolStatus == ToolStatus.RUNNING && !message.progressPreview.isNullOrEmpty()) { + if (message.isToolRunning && !message.progressPreview.isNullOrEmpty()) { Text( text = message.progressPreview, style = @@ -534,7 +534,7 @@ private fun HeaderRow( horizontalArrangement = Arrangement.spacedBy(8.dp), ) { // Status icon or spinner - if (message.toolStatus == ToolStatus.RUNNING) { + if (message.isToolRunning) { CircularProgressIndicator( modifier = Modifier.size(14.dp), strokeWidth = 2.dp, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/AgentStatusIndicator.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/AgentStatusIndicator.kt index 1f6e1cc2..f9460c6a 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/AgentStatusIndicator.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/AgentStatusIndicator.kt @@ -20,8 +20,8 @@ import com.m57.hermescontrol.ui.chat.ChatMessage import com.m57.hermescontrol.ui.chat.MessageRole import com.m57.hermescontrol.ui.chat.StreamingState import com.m57.hermescontrol.ui.chat.ToolSchemaRegistry -import com.m57.hermescontrol.ui.chat.ToolStatus import com.m57.hermescontrol.ui.chat.components.TypingIndicator +import com.m57.hermescontrol.ui.chat.isToolRunning import com.m57.hermescontrol.ui.chat.isUserTurnBoundary /** Live status for an agent turn before visible assistant prose is available. */ @@ -61,7 +61,7 @@ internal fun deriveAgentStatus( .asReversed() .firstOrNull { it.role == MessageRole.TOOL && - it.toolStatus == ToolStatus.RUNNING + it.isToolRunning } val hasVisibleStreamingContent = streamingState.streamingMessage?.content?.isNotBlank() == true diff --git a/app/src/test/java/com/m57/hermescontrol/data/local/ChatMessageMapperTest.kt b/app/src/test/java/com/m57/hermescontrol/data/local/ChatMessageMapperTest.kt index 5ce02d57..5c9dcc21 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/local/ChatMessageMapperTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/local/ChatMessageMapperTest.kt @@ -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 = diff --git a/app/src/test/java/com/m57/hermescontrol/data/local/HermesDatabaseMigrationsTest.kt b/app/src/test/java/com/m57/hermescontrol/data/local/HermesDatabaseMigrationsTest.kt index f57f20ac..c4a9e72e 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/local/HermesDatabaseMigrationsTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/local/HermesDatabaseMigrationsTest.kt @@ -97,4 +97,16 @@ class HermesDatabaseMigrationsTest { ) } } + + @Test + fun migration9to10_addsDurableProvenanceWithoutInventingDeliveryState() = + runBlocking { + val connection = mockk(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'", + ) + } + } } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCachedHistoryTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCachedHistoryTest.kt new file mode 100644 index 00000000..6d7ba108 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCachedHistoryTest.kt @@ -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)) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt index def65f96..6a3297a9 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt @@ -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)) { diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt index 3e77cf3e..0bdb03a7 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt @@ -5389,6 +5389,200 @@ class ChatViewModelTest { ) } + @Test + fun paging_staleCacheBeforeRestDoesNotBecomeTheLatestTail() = + runTest { + val cachedIds = seedPagingCache(3) + val response = CompletableDeferred>() + 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() + 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() + 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() + 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() + 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() + 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 + 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, diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt index aff84307..1b2ae77b 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt @@ -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(), groupIntoTurns(emptyList()))