diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 010ab018..e365e0eb 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -10,6 +10,7 @@ plugins { android { namespace = "com.m57.hermescontrol" + sourceSets.getByName("androidTest").assets.srcDir("$projectDir/schemas") lint { disable += "MissingTranslation" diff --git a/app/schemas/com.m57.hermescontrol.data.local.HermesDatabase/9.json b/app/schemas/com.m57.hermescontrol.data.local.HermesDatabase/9.json new file mode 100644 index 00000000..c6f9af0e --- /dev/null +++ b/app/schemas/com.m57.hermescontrol.data.local.HermesDatabase/9.json @@ -0,0 +1,136 @@ +{ + "formatVersion": 1, + "database": { + "version": 9, + "identityHash": "d3d3b8cd95987e6070d2500269bd052f", + "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, 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" + } + ], + "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, 'd3d3b8cd95987e6070d2500269bd052f')" + ] + } +} \ 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 new file mode 100644 index 00000000..795859bb --- /dev/null +++ b/app/src/androidTest/java/com/m57/hermescontrol/data/local/ChatMessagePagingDeviceTest.kt @@ -0,0 +1,379 @@ +package com.m57.hermescontrol.data.local + +import android.content.Context +import androidx.room3.Room +import androidx.sqlite.execSQL +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.m57.hermescontrol.ui.chat.ChatMessage +import com.m57.hermescontrol.ui.chat.ChatPersistenceRepository +import com.m57.hermescontrol.ui.chat.MessageRole +import kotlinx.coroutines.runBlocking +import net.zetetic.database.sqlcipher.driver.SQLCipherDriver +import org.json.JSONObject +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import java.util.UUID + +/** Executes the generated Room DAO against SQLCipher, rather than reproducing its SQL in a fake. */ +@RunWith(AndroidJUnit4::class) +class ChatMessagePagingDeviceTest { + private lateinit var context: Context + private lateinit var database: HermesDatabase + private lateinit var repository: ChatPersistenceRepository + private val databaseName = "paging-regression-${UUID.randomUUID()}.db" + + @Before + fun setUp() { + context = ApplicationProvider.getApplicationContext() + System.loadLibrary("sqlcipher") + openDatabase() + } + + private fun openDatabase() { + database = + Room + .databaseBuilder(context, HermesDatabase::class.java, databaseName) + .setDriver(driver()) + .addMigrations(HermesDatabase.MIGRATION_8_9) + .build() + repository = ChatPersistenceRepository(database.chatMessageDao()) + } + + @After + fun tearDown() { + database.close() + context.deleteDatabase(databaseName) + } + + @Test + fun unpaddedRestIdsSelectNumericLatestPageAndWalkEveryRowExactlyOnce() = + runBlocking { + val rows = (1..350).map { restRow(it, timestamp = 10L) } + repository.persistMessages(rows, "session") + repository.persistMessage(restRow(999, timestamp = 10L).copy(id = "rest-other-999"), "other") + val latest = repository.loadPage("session", null, 150) + assertEquals(rows.takeLast(150).map { it.id }, latest.messages.map { it.id }) + repository.persistMessage(restRow(351, timestamp = 1L), "session") + val middle = repository.loadPage("session", latest.cursor, 150) + val oldest = repository.loadPage("session", middle.cursor, 150) + assertEquals(rows.map { it.id }, (oldest.messages + middle.messages + latest.messages).map { it.id }) + assertEquals(true, latest.hasOlder) + assertEquals(true, middle.hasOlder) + assertEquals(false, oldest.hasOlder) + assertEquals( + "rest-session-351", + repository + .loadPage("session", null, 1) + .messages + .single() + .id, + ) + assertEquals( + "rest-other-999", + repository + .loadPage("other", null, 1) + .messages + .single() + .id, + ) + } + + @Test + fun nonMonotonicTimestampsKeepServerOrderInBothPageArrivalOrdersAfterReopen() = + runBlocking { + val rows = + listOf(9, 10, 100, 200).mapIndexed { index, id -> + restRow(id, listOf(900L, 100L, 800L, 200L)[index]) + } + for (pages in listOf(listOf(rows.take(2), rows.drop(2)), listOf(rows.drop(2), rows.take(2)))) { + repository.clearMessagesForSession("session") + pages.forEach { repository.persistMessages(it, "session") } + database.close() + openDatabase() + val latest = repository.loadPage("session", null, 2) + val older = repository.loadPage("session", latest.cursor, 2) + assertEquals(rows.drop(2).map { it.id }, latest.messages.map { it.id }) + assertEquals(rows.take(2).map { it.id }, older.messages.map { it.id }) + assertEquals(false, older.hasOlder) + } + } + + @Test + fun uuidOnlyRowsKeepInsertionOrderAfterUpdateAppendAndReopen() = + runBlocking { + val rows = + listOf("ffffffff", "00000000", "aaaaaaaa").mapIndexed { index, prefix -> + ChatMessage( + id = "$prefix-0000-4000-8000-000000000000", + role = MessageRole.USER, + content = "message-$index", + timestamp = listOf(900L, 100L, 800L)[index], + ) + } + rows.forEach { repository.persistMessage(it, "local") } + repository.persistMessage(rows.first().copy(content = "edited", timestamp = 999L), "local") + database.close() + openDatabase() + val latest = repository.loadPage("local", null, 2) + repository.persistMessage( + rows.last().copy(id = "bbbbbbbb-0000-4000-8000-000000000000", content = "new", timestamp = 1L), + "local", + ) + val older = repository.loadPage("local", latest.cursor, 2) + assertEquals(rows.drop(1).map { it.id }, latest.messages.map { it.id }) + assertEquals(listOf(rows.first().id), older.messages.map { it.id }) + assertEquals("edited", older.messages.single().content) + assertEquals(false, older.hasOlder) + assertEquals( + "new", + repository + .loadPage("local", null, 1) + .messages + .single() + .content, + ) + } + + @Test + fun migration8to9ValidatesRealSchemaAndPreservesRowsMetadataAndPaging() = + runBlocking { + val canonical = + listOf(9, 10, 100, 200).mapIndexed { index, id -> + restRow(id, listOf(900L, 100L, 800L, 200L)[index]).toEntity("session").copy( + reasoningText = "trace-$id", + toolName = "terminal", + toolCallId = "call-$id", + toolStatus = "COMPLETED", + isStreaming = true, + displayKind = "marker-$id", + tokenCount = id, + tps = 2.5, + completionId = "completion-$id", + ) + } + val local = + listOf("ffffffff", "00000000", "aaaaaaaa").mapIndexed { index, prefix -> + ChatMessageEntity( + id = "$prefix-0000-4000-8000-000000000000", + sessionId = "local", + role = "USER", + content = "local-$index", + timestamp = 100L - index, + ) + } + val malformed = + listOf( + "rest-session-", + "rest-session-10x", + "rest-other-50", + "rest-session-9223372036854775808", + ).map { + ChatMessageEntity( + id = it, + sessionId = "session", + role = "USER", + content = it, + timestamp = 1L, + ) + } + // REST pages can be inserted backwards; UUID order is physical insertion, not timestamp or text. + val seeded = canonical.asReversed() + local + malformed + createVersion8(seeded) + openDatabase() + val dao = database.chatMessageDao() + val restored = dao.getMessagesForSession("session") + dao.getMessagesForSession("local") + assertEquals(seeded.size, restored.size) + seeded.forEach { original -> + val row = restored.single { it.id == original.id } + assertEquals(original, row.copy(sortGroup = 1, sortOrder = 0)) + } + assertEquals(canonical.map { it.id }, restored.filter { it.sortGroup == 0 }.map { it.id }) + assertTrue(restored.filter { it.id in malformed.map { row -> row.id } }.all { it.sortGroup == 1 }) + assertEquals(local.map { it.id }, dao.getMessagesForSession("local").map { it.id }) + val first = repository.loadPage("session", null, 2) + var cursor = first.cursor + val pages = mutableListOf(first.messages) + var hasOlder = first.hasOlder + while (hasOlder) { + val page = repository.loadPage("session", cursor, 2) + pages += page.messages + cursor = page.cursor + hasOlder = page.hasOlder + } + assertEquals((canonical + malformed).map { it.id }, pages.asReversed().flatten().map { it.id }) + repository.persistMessage(local.first().toUiModel().copy(content = "edited", timestamp = 9999L), "local") + database.close() + openDatabase() + assertEquals(local.map { it.id }, repository.loadPage("local", null, 3).messages.map { it.id }) + assertEquals( + "edited", + repository + .loadPage("local", null, 3) + .messages + .first() + .content, + ) + assertPagingIndexUsed() + } + + @Test + fun migration8to9ValidatesEmptyDatabaseAndAllowsNewWrites() = + runBlocking { + createVersion8(emptyList()) + openDatabase() + assertTrue(repository.loadPage("session", null, 1).messages.isEmpty()) + repository.persistMessage(restRow(100, 1L), "session") + assertEquals(100L, database.chatMessageDao().getMessage("rest-session-100")?.sortOrder) + assertPagingIndexUsed() + } + + @Test + fun confirmedAliasPreservesLivePayloadAndCanonicalOrderAcrossReopen() = + runBlocking { + val live = + ChatMessage( + id = "ffffffff-0000-4000-8000-000000000000", + role = MessageRole.ASSISTANT, + content = "newer WS payload", + reasoningText = "live trace", + completionId = "completion-100", + ) + repository.persistMessage(live, "session") + repository.confirmIdentities( + listOf(live.copy(content = "stale snapshot", restId = "rest-session-100")), + "session", + ) + // A later WS write with no alias must retain the confirmed cache identity. + repository.persistMessage(live.copy(content = "final payload"), "session") + repository.persistMessage(restRow(9, 9999L), "session") + database.close() + openDatabase() + val rows = repository.loadPage("session", null, 2).messages + assertEquals(listOf("rest-session-9", live.id), rows.map { it.id }) + assertEquals("rest-session-100", rows.last().restId) + assertEquals("final payload", rows.last().content) + assertEquals("live trace", rows.last().reasoningText) + assertEquals("completion-100", rows.last().completionId) + } + + @Test + fun confirmingLastLocalRowDoesNotReuseItsCursorForALaterInsertion() = + runBlocking { + val live = + ChatMessage( + id = "ffffffff-0000-4000-8000-000000000000", + role = MessageRole.ASSISTANT, + content = "first", + ) + repository.persistMessage(live, "session") + val cursor = repository.loadPage("session", null, 1).cursor + repository.confirmIdentities(listOf(live.copy(restId = "rest-session-100")), "session") + val next = live.copy(id = "00000000-0000-4000-8000-000000000000", content = "second") + repository.persistMessage(next, "session") + assertFalse(repository.loadPage("session", cursor, 10).messages.any { it.id == next.id }) + assertEquals( + next.id, + repository + .loadPage("session", null, 1) + .messages + .single() + .id, + ) + } + + private fun driver() = SQLCipherDriver("paging-test-only".toByteArray(), null, null) + + /** Build the actual exported v8 schema, then let Room run AND validate its migration on open. */ + private fun createVersion8(rows: List) { + database.close() + context.deleteDatabase(databaseName) + val schema = + InstrumentationRegistry + .getInstrumentation() + .context.assets + .open("com.m57.hermescontrol.data.local.HermesDatabase/8.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) " + + "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) + insert.step() + insert.reset() + insert.clearBindings() + } + } + connection.execSQL("PRAGMA user_version = 8") + } + } + + private fun assertPagingIndexUsed() { + database.close() + driver().open(context.getDatabasePath(databaseName).absolutePath).use { connection -> + for (bound in listOf("", "AND (sort_group, sort_order, id) < (1, 100, 'cursor')")) { + connection + .prepare( + "EXPLAIN QUERY PLAN SELECT * FROM chat_messages WHERE session_id = 'session' $bound " + + "ORDER BY sort_group DESC, sort_order DESC, id DESC LIMIT 151", + ).use { query -> + val plan = buildList { while (query.step()) add(query.getText(3)) }.joinToString(" ") + assertTrue(plan, plan.contains("index_chat_messages_session_id_sort_group_sort_order_id")) + assertFalse(plan, plan.contains("TEMP B-TREE", ignoreCase = true)) + } + } + } + } + + private fun restRow( + id: Int, + timestamp: Long, + ) = ChatMessage( + id = "rest-session-$id", + role = MessageRole.ASSISTANT, + content = "answer-$id", + timestamp = timestamp, + ) +} diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatAnchorTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatAnchorTest.kt new file mode 100644 index 00000000..dfbf9241 --- /dev/null +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatAnchorTest.kt @@ -0,0 +1,252 @@ +package com.m57.hermescontrol.ui.chat.fullbleed + +import androidx.compose.foundation.MutatePriority +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.lazy.LazyListState +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.unit.dp +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.m57.hermescontrol.ui.chat.ChatMessage +import com.m57.hermescontrol.ui.chat.ChatSearchState +import com.m57.hermescontrol.ui.chat.ChatViewModel +import com.m57.hermescontrol.ui.chat.ClarifyUi +import com.m57.hermescontrol.ui.chat.MessageRole +import com.m57.hermescontrol.ui.chat.StreamingState +import com.m57.hermescontrol.ui.chat.ToolStatus +import com.m57.hermescontrol.ui.chat.VaultCodePromptUi +import com.m57.hermescontrol.ui.chat.VaultSaveLoginPromptUi +import com.m57.hermescontrol.ui.chat.VaultUnlockPromptUi +import com.m57.hermescontrol.ui.chat.components.rememberChatScrollController +import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.launch +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** Real lazy layout regression for the custom23 history prepend jump. */ +@RunWith(AndroidJUnit4::class) +class FullBleedChatAnchorTest { + @get:Rule + val compose = createComposeRule() + + @Test + fun loadingOnly_atExactTop_keepsContentPixel() = checkAnchor(index = 0, offset = 0, prepend = false) + + @Test + fun loadingOnly_atPartialTop_keepsContentPixel() = checkAnchor(index = 0, offset = 23, prepend = false) + + @Test + fun prepend_atExactTop_keepsContentPixel() = checkAnchor(index = 0, offset = 0, prepend = true) + + @Test + fun prepend_atPartialTop_keepsContentPixel() = checkAnchor(index = 0, offset = 23, prepend = true) + + @Test + fun prepend_atInteriorRow_keepsContentPixel() = checkAnchor(index = 8, offset = 17, prepend = true) + + @Test + fun prepend_mergingAgentTurn_keepsToolPixel() = + checkAnchor(index = 0, offset = 13, prepend = true, role = MessageRole.TOOL) + + @Test + fun loadingOnly_doesNotCancelAnActiveHistoryScroll() = + checkAnchor(index = 8, offset = 17, prepend = false, holdScroll = true) + + @Test + fun prepend_usesTheReadersPositionWhenThePageArrives() = + checkAnchor(index = 0, offset = 23, prepend = true, moveWhileLoading = true) + + @Test + fun prepend_afterReaderReachesAgentStatus_keepsTailPixel() = checkTailAnchor("agent_status") + + @Test + fun prepend_afterReaderReachesClarify_keepsTailPixel() = checkTailAnchor("clarify_bubble") + + @Test + fun prepend_afterReaderReachesVaultUnlock_keepsTailPixel() = checkTailAnchor("vault_unlock_card") + + @Test + fun prepend_afterReaderReachesVaultSave_keepsTailPixel() = checkTailAnchor("vault_save_login_card") + + @Test + fun prepend_afterReaderReachesVaultCode_keepsTailPixel() = checkTailAnchor("vault_code_card") + + @Test + fun prepend_duringActiveScroll_keepsGestureAndLiveTail_thenPublishesAtIdle() = + checkAnchor(index = 8, offset = 17, prepend = true, holdScroll = true) + + @Test + fun prepend_deferredDuringScroll_doesNotSurviveSessionSwitch() = + checkAnchor(index = 8, offset = 17, prepend = true, holdScroll = true, switchSession = true) + + private fun checkTailAnchor(key: String) = checkAnchor(index = 0, offset = 0, prepend = true, tailKey = key) + + private fun checkAnchor( + index: Int, + offset: Int, + prepend: Boolean, + role: MessageRole = MessageRole.USER, + holdScroll: Boolean = false, + moveWhileLoading: Boolean = false, + tailKey: String? = null, + switchSession: Boolean = false, + ) { + fun message(id: Int) = + ChatMessage( + id = "row-$id", + role = role, + content = "Message $id\nSecond line\nThird line", + toolName = if (role == MessageRole.TOOL) "terminal" else null, + toolStatus = if (role == MessageRole.TOOL) ToolStatus.COMPLETED else null, + ) + + val messages = mutableStateOf((150..189).map(::message)) + val loading = mutableStateOf(false) + val session = mutableStateOf("session-one") + val streaming = mutableStateOf(StreamingState()) + val tailKeys = + listOf("agent_status", "clarify_bubble", "vault_unlock_card", "vault_save_login_card", "vault_code_card") + val listState = LazyListState(index, offset) + val viewModel = mockk(relaxed = true) + lateinit var scope: CoroutineScope + compose.setContent { + scope = rememberCoroutineScope() + val controller = rememberChatScrollController(listState, scope) + Box(Modifier.size(width = 320.dp, height = if (tailKey == null) 400.dp else 24.dp)) { + FullBleedChatList( + messages = messages.value, + streamingState = streaming.value, + isAgentTyping = tailKey != null, + searchState = ChatSearchState(), + typingEffectEnabled = false, + typingEffectDelayMs = 30, + isLoading = false, + isLoadingOlder = loading.value, + listState = listState, + scrollController = controller, + lastAnimatedMessageId = null, + onLastAnimatedMessageIdChange = {}, + viewModel = viewModel, + pagingSessionId = session.value, + clarifyRequest = if (tailKey != null) ClarifyUi("Choose an option", listOf("Yes")) else null, + vaultUnlockPrompt = if (tailKey != null) VaultUnlockPromptUi(null, null) else null, + vaultSaveLoginPrompt = if (tailKey != null) VaultSaveLoginPromptUi(null, null) else null, + vaultCodePrompt = if (tailKey != null) VaultCodePromptUi(null, null) else null, + ) + } + } + var before = + compose.runOnIdle { + listState.layoutInfo.visibleItemsInfo + .first { it.index == index } + .let { it.key to it.offset } + } + + fun assertAnchor(stage: String) { + compose.runOnIdle { + val after = listState.layoutInfo.visibleItemsInfo.firstOrNull { it.key == before.first } + assertNotNull("$stage: anchor ${before.first} left viewport", after) + assertEquals("$stage: anchor ${before.first} moved (pixels)", before.second, after!!.offset) + } + } + + val scrollJob = + compose.runOnIdle { + if (holdScroll) { + scope.launch { + listState.scroll(MutatePriority.UserInput) { awaitCancellation() } + } + } else { + null + } + } + compose.runOnIdle { + if (holdScroll) assertTrue("scroll started", listState.isScrollInProgress) + loading.value = true + } + assertAnchor("loading started") + if (tailKey != null) { + compose.runOnIdle { scope.launch { listState.scrollToItem(40 + tailKeys.indexOf(tailKey)) } } + before = + compose.runOnIdle { + listState.layoutInfo.visibleItemsInfo + .first { it.index == listState.firstVisibleItemIndex } + .let { it.key to it.offset } + } + assertEquals("reader reached requested tail row", tailKey, before.first) + } + if (moveWhileLoading) { + compose.runOnIdle { scope.launch { listState.scrollToItem(index + 3, 19) } } + before = + compose.runOnIdle { + listState.layoutInfo.visibleItemsInfo + .first { it.index == listState.firstVisibleItemIndex } + .let { it.key to it.offset } + } + } + compose.runOnIdle { + if (prepend) messages.value = (0..149).map(::message) + messages.value + loading.value = false + } + assertAnchor(if (prepend) "page prepended" else "loading stopped without data") + compose.runOnIdle { + if (holdScroll) { + assertTrue("loading must not cancel the gesture", scrollJob!!.isActive) + assertTrue("scroll still in progress", listState.isScrollInProgress) + } + } + if (holdScroll && prepend) { + compose.runOnIdle { + assertEquals("older prefix waits for idle", 40, listState.layoutInfo.totalItemsCount) + messages.value = messages.value + message(190) + streaming.value = + StreamingState(streamingMessage = ChatMessage("stream", MessageRole.ASSISTANT, "Live answer")) + } + compose.runOnIdle { + assertEquals("new settled and streaming tail remain live", 42, listState.layoutInfo.totalItemsCount) + assertTrue("tail update must not cancel scrolling", scrollJob!!.isActive) + listState.dispatchRawDelta(18f) + } + before = + compose.runOnIdle { + listState.layoutInfo.visibleItemsInfo + .first { it.index == listState.firstVisibleItemIndex } + .let { it.key to it.offset } + } + if (switchSession) { + compose.runOnIdle { + messages.value = (200..239).map(::message) + streaming.value = StreamingState() + session.value = "session-two" + } + compose.runOnIdle { + assertEquals( + "session changes immediately", + 40, + listState.layoutInfo.totalItemsCount, + ) + } + } + } + compose.runOnIdle { scrollJob?.cancel() } + if (holdScroll && prepend) { + compose.runOnIdle { + assertEquals( + "no lost or stale pending rows", + if (switchSession) 40 else 192, + listState.layoutInfo.totalItemsCount, + ) + } + if (!switchSession) assertAnchor("gesture ended after reader moved; prefix published") + } + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageDao.kt b/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageDao.kt index a1040019..df2fb46f 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageDao.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageDao.kt @@ -1,23 +1,83 @@ package com.m57.hermescontrol.data.local import androidx.room3.Dao -import androidx.room3.Insert -import androidx.room3.OnConflictStrategy import androidx.room3.Query +import androidx.room3.Transaction +import androidx.room3.Upsert @Dao interface ChatMessageDao { @Query("SELECT EXISTS(SELECT 1 FROM chat_messages WHERE session_id = :sessionId)") suspend fun sessionExists(sessionId: String): Boolean - @Query("SELECT * FROM chat_messages WHERE session_id = :sessionId ORDER BY timestamp ASC") + @Query( + "SELECT * FROM chat_messages WHERE session_id = :sessionId " + + "ORDER BY sort_group, sort_order, id", + ) suspend fun getMessagesForSession(sessionId: String): List - @Insert(onConflict = OnConflictStrategy.REPLACE) - suspend fun upsert(message: ChatMessageEntity) + @Query( + "SELECT * FROM chat_messages WHERE session_id = :sessionId " + + "ORDER BY sort_group DESC, sort_order DESC, id DESC LIMIT :limit", + ) + suspend fun getLatestMessagePage( + sessionId: String, + limit: Int, + ): List - @Insert(onConflict = OnConflictStrategy.REPLACE) - suspend fun upsertAll(messages: List) + @Query( + "SELECT * FROM chat_messages WHERE session_id = :sessionId " + + "AND (sort_group, sort_order, id) < (:beforeGroup, :beforeOrder, :beforeId) " + + "ORDER BY sort_group DESC, sort_order DESC, id DESC LIMIT :limit", + ) + suspend fun getMessagePage( + sessionId: String, + beforeGroup: Int, + beforeOrder: Long, + beforeId: String, + limit: Int, + ): List + + @Query("SELECT * FROM chat_messages WHERE id = :id") + suspend fun getMessage(id: String): ChatMessageEntity? + + // Physical insertion sequence includes confirmed aliases, so confirmation cannot reuse a local cursor key. + @Query("SELECT COALESCE(MAX(rowid), 0) + 1 FROM chat_messages") + suspend fun nextLocalOrder(): Long + + @Upsert + suspend fun writeMessage(message: ChatMessageEntity) + + // Allocate once, in the same transaction as the write. Content/clock updates never move a local row. + @Transaction + suspend fun upsert(message: ChatMessageEntity) { + val existing = getMessage(message.id) + val restId = message.restId ?: existing?.restId + val canonicalOrder = canonicalMessageOrder(restId ?: message.id, message.sessionId) + writeMessage( + message.copy( + restId = restId, + completionId = message.completionId ?: existing?.completionId, + sortGroup = if (canonicalOrder != null) 0 else 1, + sortOrder = canonicalOrder ?: existing?.sortOrder ?: nextLocalOrder(), + ), + ) + } + + @Transaction + suspend fun upsertAll(messages: List) { + messages.forEach { upsert(it) } + } + + // History confirms identity, but must not overwrite a newer WS-owned payload. + @Transaction + suspend fun confirmIdentity(message: ChatMessageEntity) { + val existing = getMessage(message.id) + upsert( + existing?.copy(restId = message.restId, completionId = existing.completionId ?: message.completionId) + ?: message, + ) + } @Query("DELETE FROM chat_messages WHERE session_id = :sessionId") suspend fun deleteMessagesForSession(sessionId: String) 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 3a4d6f18..b77843d4 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 @@ -10,13 +10,13 @@ import androidx.room3.PrimaryKey * UI model but is stored independently so it survives process death. * * Messages are scoped by [sessionId] so switching sessions loads the right - * thread. The [timestamp] field preserves original ordering even if Room - * reorders internally. + * thread. [sortOrder] preserves numeric server order or local insertion order; + * [timestamp] is display metadata only. */ @Entity( tableName = "chat_messages", indices = [ - Index(value = ["session_id", "timestamp"]), + Index(value = ["session_id", "sort_group", "sort_order", "id"]), ], ) data class ChatMessageEntity( @@ -48,4 +48,21 @@ data class ChatMessageEntity( val tps: Double? = null, @ColumnInfo(name = "completion_id") val completionId: String? = null, + @ColumnInfo(name = "rest_id") + val restId: String? = null, + @ColumnInfo(name = "sort_group", defaultValue = "1") + val sortGroup: Int = 1, + @ColumnInfo(name = "sort_order", defaultValue = "0") + val sortOrder: Long = 0, ) + +/** Only the exact session prefix and a nonnegative decimal suffix identify a canonical row. */ +internal fun canonicalMessageOrder( + id: String, + sessionId: String, +): Long? { + val prefix = "rest-$sessionId-" + if (!id.startsWith(prefix)) return null + val suffix = id.removePrefix(prefix) + return suffix.takeIf { it.isNotEmpty() && it.all { char -> char in '0'..'9' } }?.toLongOrNull() +} 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 953c093d..3a246c3a 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 @@ -38,6 +38,8 @@ fun ChatMessageEntity.toUiModel(): ChatMessage = tokenCount = tokenCount, tps = tps, completionId = completionId, + restId = restId, + localOrder = sortOrder.takeIf { sortGroup == 1 && it > 0 }, ) fun ChatMessage.toEntity(sessionId: String): ChatMessageEntity = @@ -56,4 +58,5 @@ fun ChatMessage.toEntity(sessionId: String): ChatMessageEntity = tokenCount = tokenCount, tps = tps, completionId = completionId, + restId = restId, ) 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 f3525f14..c30d2891 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 = 8, + version = 9, exportSchema = true, ) abstract class HermesDatabase : RoomDatabase() { @@ -90,6 +90,31 @@ abstract class HermesDatabase : RoomDatabase() { } } + val MIGRATION_8_9: Migration = + object : Migration(8, 9) { + override suspend fun migrate(connection: SQLiteConnection) { + connection.execSQL("ALTER TABLE chat_messages ADD COLUMN rest_id TEXT") + connection.execSQL("ALTER TABLE chat_messages ADD COLUMN sort_group INTEGER NOT NULL DEFAULT 1") + connection.execSQL("ALTER TABLE chat_messages ADD COLUMN sort_order INTEGER NOT NULL DEFAULT 0") + // v8 has no durable local sequence. Preserve its surviving physical insertion order. + connection.execSQL("UPDATE chat_messages SET sort_order = rowid") + val suffix = "substr(id, length(session_id) + 7)" + val digits = "ltrim($suffix, '0')" + connection.execSQL( + "UPDATE chat_messages SET sort_group = 0, sort_order = CAST($suffix AS INTEGER) " + + "WHERE substr(id, 1, length(session_id) + 6) = 'rest-' || session_id || '-' " + + "AND $suffix != '' AND $suffix NOT GLOB '*[^0-9]*' " + + "AND (length($digits) < 19 OR " + + "(length($digits) = 19 AND $digits <= '9223372036854775807'))", + ) + connection.execSQL("DROP INDEX index_chat_messages_session_id_timestamp") + connection.execSQL( + "CREATE INDEX index_chat_messages_session_id_sort_group_sort_order_id " + + "ON chat_messages (session_id, sort_group, sort_order, id)", + ) + } + } + suspend fun get(context: Context): HermesDatabase = withContext(Dispatchers.IO) { instance?.let { return@withContext it } @@ -126,6 +151,7 @@ abstract class HermesDatabase : RoomDatabase() { MIGRATION_5_6, MIGRATION_6_7, MIGRATION_7_8, + MIGRATION_8_9, ).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 3f48a358..f65c6a6d 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 @@ -87,8 +87,16 @@ data class ChatMessage( val tps: Double? = null, /** Stable completion identity correlating with reply notifications for read tracking. */ val completionId: String? = null, + /** Confirmed transcript identity (#859); [id] remains the stable live/render key. */ + val restId: String? = null, + /** Cache insertion sequence for unconfirmed local rows; null before first persistence. */ + val localOrder: Long? = null, ) +/** 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-") } + /** * Single live transcript log entry for subagent execution. */ diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatPersistenceRepository.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatPersistenceRepository.kt index a2acc530..da1b4537 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatPersistenceRepository.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatPersistenceRepository.kt @@ -36,6 +36,50 @@ open class ChatPersistenceRepository( suspend fun loadMessages(sessionId: String): List = daoProvider().getMessagesForSession(sessionId).map { it.toUiModel() } + data class Cursor( + val group: Int, + val order: Long, + val id: String, + ) + + data class Page( + val messages: List, + val cursor: Cursor?, + val hasOlder: Boolean, + ) + + /** Read at most one page plus a lookahead row; never trim a full-session query. */ + suspend fun loadPage( + sessionId: String, + before: Cursor?, + limit: Int, + ): Page { + require(limit in 1..1_000) + val dao = daoProvider() + val rows = + if (before == null) { + dao.getLatestMessagePage(sessionId, limit + 1) + } else { + dao.getMessagePage(sessionId, before.group, before.order, before.id, limit + 1) + } + val page = rows.take(limit) + val oldest = page.lastOrNull() + return Page( + messages = page.asReversed().map { it.toUiModel() }, + cursor = oldest?.let { Cursor(it.sortGroup, it.sortOrder, it.id) } ?: before, + hasOlder = rows.size > limit, + ) + } + + /** Record confirmed UUID aliases without replacing their newer locally persisted content. */ + suspend fun confirmIdentities( + messages: List, + sessionId: String, + ) { + val dao = daoProvider() + messages.forEach { dao.confirmIdentity(it.toEntity(sessionId)) } + } + /** Clear all cached messages for a session (e.g. after /undo rewind). */ suspend fun clearMessagesForSession(sessionId: String) { daoProvider().deleteMessagesForSession(sessionId) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt index 8c935e40..49d7cfc9 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt @@ -59,7 +59,6 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue -import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -115,8 +114,6 @@ import com.m57.hermescontrol.ui.common.NavIcon import com.m57.hermescontrol.ui.model.components.ModelPickerDialog import com.m57.hermescontrol.util.ConnectorUrlValidator import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch private const val SESSION_SYNC_INTERVAL_MS = 30_000L @@ -211,7 +208,6 @@ fun ChatScreen( val listState = rememberLazyListState(prefetchStrategy = ChatTimelineNoPrefetchStrategy) val scrollScope = rememberCoroutineScope() val scrollController = rememberChatScrollController(listState, scrollScope) - var isOlderPagingArmed by remember(state.currentSessionId) { mutableStateOf(false) } var showContextSheet by remember { mutableStateOf(false) } var pendingSavePath by rememberSaveable { mutableStateOf(null) } var pendingSaveName by rememberSaveable { mutableStateOf(null) } @@ -265,38 +261,9 @@ fun ChatScreen( } } - // Arm + trigger older-message paging when the user scrolls near the top. - // Before prepending, capture the anchor (first visible message id + offset) - // so the same content stays under the reader's eye after the insert - // (issue #682). Restored in a LaunchedEffect once the page actually lands. - val pagingAnchor = remember { mutableStateOf?>(null) } - LaunchedEffect(listState, state.currentSessionId, state.hasOlderMessages, state.isLoadingOlder) { - if (!state.hasOlderMessages || state.isLoadingOlder) return@LaunchedEffect - snapshotFlow { listState.firstVisibleItemIndex } - .distinctUntilChanged() - .collectLatest { firstVisibleIndex -> - if (firstVisibleIndex > 2) { - isOlderPagingArmed = true - } else if (isOlderPagingArmed || state.messages.size <= 3) { - val anchorId = state.messages.getOrNull(firstVisibleIndex)?.id - val offset = scrollController.captureAnchorOffset() - pagingAnchor.value = if (anchorId != null) anchorId to offset else null - viewModel.loadOlderMessages() - } - } - } - - // After older history is prepended, restore the anchor message (by id, so - // streaming-token inserts during paging don't skew the index) plus offset. - LaunchedEffect(state.messages) { - val anchor = pagingAnchor.value ?: return@LaunchedEffect - val (anchorId, offset) = anchor - val index = state.messages.indexOfFirst { it.id == anchorId } - if (index >= 0) { - scrollController.scrollToItem(index, offset) - pagingAnchor.value = null - } - } + // FullBleedChatList preserves the live lazy row key and pixel offset on prepend. + // Never restore a message index captured before an asynchronous fetch: it is not + // a lazy row index and the reader may have moved in the meantime. // Continuous bottom-follow tracking from LazyListState (issue #682). LaunchedEffect(Unit) { @@ -705,6 +672,8 @@ fun ChatScreen( maxToolCallsPerTurn = state.maxToolCallsPerTurn, isLoading = state.isLoading, isLoadingOlder = state.isLoadingOlder, + hasOlderMessages = state.hasOlderMessages, + pagingSessionId = state.currentSessionId, listState = listState, scrollController = scrollController, lastAnimatedMessageId = lastAnimatedMessageId, 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 b4f63282..f4b24abc 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 @@ -10,10 +10,9 @@ import com.m57.hermescontrol.notification.ReplyNotificationTracker /** * Maps REST transcript rows ([SessionMessage]) into UI [ChatMessage]s. * - * Verbatim extraction of ChatViewModel.mapServerMessages (corral 2 of the - * 2026-08 delegate refactor). Pure with respect to the VM: it reads the live - * message list supplied by the caller and one paging-mode flag — no state - * mutation, no I/O beyond URL construction. + * Reads the supplied transcript snapshot and recovers notification identity through + * ReplyNotificationTracker's synchronized lookup. Safe on the history dispatcher; + * it never mutates ViewModel state or dismisses a notification. */ internal fun mapServerMessages( sessionId: String, @@ -24,15 +23,11 @@ internal fun mapServerMessages( isPagingOlder: Boolean = false, context: android.content.Context? = null, ): List { - val existingReasoningMap = - liveMessages - .filter { it.reasoningText.isNotBlank() } - .associateBy { it.content } - + val existingById = liveMessages.associateBy { it.canonicalRestId ?: it.id } val liveByExactId = liveMessages .filter { !it.completionId.isNullOrBlank() } - .associateBy { it.id } + .associateBy { it.canonicalRestId ?: it.id } val unmappedLiveWsAssistants = if (isPagingOlder) { @@ -40,112 +35,91 @@ internal fun mapServerMessages( } else { liveMessages .filter { - it.role == MessageRole.ASSISTANT && !it.id.startsWith("rest-") && + it.role == MessageRole.ASSISTANT && it.canonicalRestId == null && !it.completionId.isNullOrBlank() } } - val wsCompletionIdByRestIndex = mutableMapOf() - if (!isPagingOlder) { - if (unmappedLiveWsAssistants.isNotEmpty()) { - val remainingWs = unmappedLiveWsAssistants.toMutableList() - for (i in messages.indices.reversed()) { - if (remainingWs.isEmpty()) break - val m = messages[i] - val r = - when (m.role?.lowercase()) { - "user" -> MessageRole.USER - "system" -> MessageRole.SYSTEM - "tool" -> MessageRole.TOOL - else -> MessageRole.ASSISTANT - } - if (r != MessageRole.ASSISTANT) continue - val rawContent = m.contentText - // Reasoning-only or empty tool placeholder: skip, does not become visible prose - if (rawContent.isBlank() && !rawContent.contains("MEDIA:")) continue - - val restId = - if (latestPaging) { - m.id?.let { "rest-$sessionId-$it" } ?: "rest-$sessionId-${offset + i}" - } else { - "rest-$sessionId-${offset + i}" - } - if (liveByExactId.containsKey(restId)) continue - - val canonicalContent = - if (rawContent.contains("MEDIA:")) { - HostMediaExtractor.strip(rawContent).trim() - } else { - rawContent.trim() - } - - val wsIdx = remainingWs.indexOfLast { it.content.trim() == canonicalContent } - if (wsIdx >= 0) { - remainingWs.removeAt(wsIdx).completionId?.let { compId -> - wsCompletionIdByRestIndex[i] = compId - } - } - } + fun restIdAt(index: Int): String = + if (latestPaging) { + "rest-$sessionId-${requireNotNull(messages[index].id) { "Latest transcript row has no stable id" }}" + } else { + "rest-$sessionId-${offset + index}" } - // REST-only recovery is fail-closed: only a durable server row ID may - // transfer notification identity after the WebSocket copy is gone. - val activeTarget = ReplyNotificationTracker.getActiveTarget(context) - if ( - activeTarget != null && - activeTarget.sessionId == sessionId && - activeTarget.serverMessageId != null && - wsCompletionIdByRestIndex.values.none { it == activeTarget.completionId } - ) { + val wsCompletionIdByRestIndex = mutableMapOf() + if (!isPagingOlder) { + // #129: reserve durable identity before matching any repeated live prose. + // REST-only recovery remains fail-closed when the target row is absent. + val activeTarget = ReplyNotificationTracker.getActiveTarget(context)?.takeIf { it.sessionId == sessionId } + val durableTarget = activeTarget?.takeIf { it.serverMessageId != null } + if (durableTarget != null) { val exactIndex = messages.indexOfFirst { message -> - message.id == activeTarget.serverMessageId && + message.id == durableTarget.serverMessageId && message.role.equals("assistant", ignoreCase = true) } - if (exactIndex >= 0 && !liveByExactId.containsKey("rest-$sessionId-${activeTarget.serverMessageId}")) { - wsCompletionIdByRestIndex[exactIndex] = activeTarget.completionId + if (exactIndex >= 0 && !liveByExactId.containsKey(restIdAt(exactIndex))) { + wsCompletionIdByRestIndex[exactIndex] = durableTarget.completionId + } + } + val confirmedCompletions = + liveMessages.filter { it.canonicalRestId != null }.mapNotNull { it.completionId }.toSet() + val remainingWs = + unmappedLiveWsAssistants + .filter { + it.completionId !in confirmedCompletions && it.completionId != durableTarget?.completionId + }.toMutableList() + // #129: reserve known completions and live aliases. A metadata-free cached REST + // echo must remain eligible for its pending WS completion before older duplicates. + val reservedRestIds = liveByExactId.keys + liveMessages.mapNotNull { it.restId } + // Retain upstream's newest-to-newest, one-to-one WS echo matching. + for (i in messages.indices.reversed()) { + if (remainingWs.isEmpty()) break + if (i in wsCompletionIdByRestIndex || restIdAt(i) in reservedRestIds) continue + val m = messages[i] + if (m.role?.lowercase() in listOf("user", "system", "tool")) continue + val rawContent = m.contentText + if (rawContent.isBlank()) continue + val canonicalContent = + if (rawContent.contains("MEDIA:")) HostMediaExtractor.strip(rawContent).trim() else rawContent.trim() + val wsIdx = remainingWs.indexOfLast { it.content.trim() == canonicalContent } + if (wsIdx >= 0) { + remainingWs.removeAt(wsIdx).completionId?.let { wsCompletionIdByRestIndex[i] = it } } } } - // Tool rows in the REST transcript carry NO tool name — the live WS - // stream was the only source of `toolName`. Match each REST tool row - // to its WS counterpart by RESULT CONTENT (not position — pagination - // and mixed cache state make positional mapping misalign, leaving - // the newest call with a null name → generic "tool" bubble). When a - // match is found the live message is reused wholesale (same id + - // toolName + rich payload), so persistence upserts the same row - // instead of accumulating a second `rest-` copy in Room. Issue #771. - val liveToolByResult = linkedMapOf() - liveMessages - .filter { it.role == MessageRole.TOOL } - .sortedBy { it.id.startsWith("rest-") } // prefer live WS copies - .forEach { msg -> - canonicalToolResultKey(msg.content)?.let { key -> - liveToolByResult.putIfAbsent(key, msg) + // Reasoning follows the same reserved identities as completions, never a reusable text lookup. + val assistants = liveMessages.filter { it.role == MessageRole.ASSISTANT } + val assistantsByRestId = assistants.groupBy { it.canonicalRestId } + val assistantsByCompletion = assistants.groupBy { it.completionId } + val reasoningSources = arrayOfNulls(messages.size) + messages.forEachIndexed { index, row -> + if (row.role?.lowercase() in listOf("user", "system", "tool")) return@forEachIndexed + val exact = assistantsByRestId[restIdAt(index)].orEmpty() + val completion = liveByExactId[restIdAt(index)]?.completionId ?: wsCompletionIdByRestIndex[index] + val candidates = exact + completion?.let { assistantsByCompletion[it] }.orEmpty() + reasoningSources[index] = candidates.firstOrNull { it.reasoningText.isNotBlank() } ?: candidates.firstOrNull() + } + if (!isPagingOlder) { + val remaining = + assistants.filter { it.canonicalRestId == null && it.completionId == null }.toMutableList() + for (index in messages.indices.reversed()) { + val row = messages[index] + if (reasoningSources[index] != null || row.contentText.isBlank() || + row.role?.lowercase() in listOf("user", "system", "tool") || + liveByExactId[restIdAt(index)]?.completionId != null || index in wsCompletionIdByRestIndex + ) { + continue } + val content = HostMediaExtractor.strip(row.contentText).trim() + val match = remaining.indexOfLast { it.content.trim() == content } + if (match >= 0) reasoningSources[index] = remaining.removeAt(match) } - - // Issue #842: REST transcript rows carry the gateway's `tool_call_id` - // — prefer matching live bubbles by that 1:1 identity. It works for - // EVERY tool shape, including MCP/web rows whose REST copy is raw - // `` text with no JSON key to canonicalize. - val liveToolByCallId = linkedMapOf() - liveMessages - .filter { it.role == MessageRole.TOOL && it.toolCallId.isNotBlank() } - .sortedBy { it.id.startsWith("rest-") } // prefer live WS copies - .forEach { msg -> - liveToolByCallId.putIfAbsent(msg.toolCallId, msg) - } + } val mapped = mutableListOf() - // The gateway stores a reasoning-model's thinking as its OWN assistant - // row (content = "", reasoning = trace) directly before the answer row. - // Rendering that as a standalone empty assistant bubble is the - // "reasoning box in a separate bubble" artifact — fold it into the - // next assistant message with content instead. Issue #771. - var pendingReasoning: String? = null - messages.forEachIndexed { index, msg -> val role = when (msg.role?.lowercase()) { @@ -154,47 +128,32 @@ internal fun mapServerMessages( "tool" -> MessageRole.TOOL else -> MessageRole.ASSISTANT } - val globalIndex = offset + index // Issue #859: under newest-anchored paging use the server's // AUTOINCREMENT row id as the stable key — from-end positions shift // as the transcript grows and would collide across hydrations // (distinctBy would silently drop the newest copy). Legacy paging // keeps the absolute-position key its count-based sync math needs. - val restId = - if (latestPaging) { - msg.id?.let { "rest-$sessionId-$it" } ?: "rest-$sessionId-$globalIndex" - } else { - "rest-$sessionId-$globalIndex" - } + val restId = restIdAt(index) val timestamp = msg.timestampText ?.toDoubleOrNull() ?.times(1000) ?.toLong() + ?: existingById[restId]?.timestamp ?: System.currentTimeMillis() val rawContent = msg.contentText val rowReasoning = - if (msg.reasoningText.isNotBlank()) { - msg.reasoningText - } else { - existingReasoningMap[rawContent]?.reasoningText.orEmpty() + msg.reasoningText.ifBlank { + if (role == MessageRole.ASSISTANT) { + reasoningSources[index]?.reasoningText.orEmpty() + } else { + existingById[restId]?.reasoningText.orEmpty() + } } - // Empty assistant row — two cases stored by the gateway: - // 1. Reasoning-only: thinking-model split storage (content = "", - // reasoning = trace). Stash the trace and fold it into the - // next assistant message that has content (issue #771). - // 2. Tool-call placeholder: non-reasoning models emit content = "" - // with tool_calls metadata and no reasoning. These carry no - // user-visible text and must not render as empty bubbles - // (issue #956). - if (role == MessageRole.ASSISTANT && rawContent.isBlank()) { - if (rowReasoning.isNotBlank()) { - pendingReasoning = rowReasoning - } - return@forEachIndexed - } + // Retain canonical reasoning rows across page boundaries; hide only textless placeholders. + if (role == MessageRole.ASSISTANT && rawContent.isBlank() && rowReasoning.isBlank()) return@forEachIndexed // Context-reference preprocessing can persist model-facing attachment // contents after the user's text. A cold reload / second device has no @@ -232,34 +191,6 @@ internal fun mapServerMessages( } } - val finalReasoning = - if (rowReasoning.isNotBlank()) { - rowReasoning - } else if (role == MessageRole.ASSISTANT && pendingReasoning != null) { - pendingReasoning.also { pendingReasoning = null } - } else { - "" - } - - // Tool rows in the REST transcript carry no tool name. When the - // result payload matches a live WS tool message, reuse it whole — - // keeps the real name, the rich WS payload, AND the same id so - // Room upserts instead of accumulating a duplicate `rest-` row. - if (role == MessageRole.TOOL) { - // Prefer the gateway call id (1:1, works for every tool - // shape), then fall back to result-content matching. - liveToolByCallId[msg.toolCallId]?.let { live -> - mapped.add(live) - return@forEachIndexed - } - canonicalToolResultKey(rawContent)?.let { key -> - liveToolByResult[key]?.let { live -> - mapped.add(live) - return@forEachIndexed - } - } - } - val completionId = if (role == MessageRole.ASSISTANT) { liveByExactId[restId]?.completionId ?: wsCompletionIdByRestIndex[index] @@ -272,7 +203,7 @@ internal fun mapServerMessages( id = restId, role = role, content = finalContent, - reasoningText = finalReasoning, + reasoningText = rowReasoning, toolCallId = msg.toolCallId, attachments = attachments, timestamp = timestamp, @@ -284,17 +215,12 @@ internal fun mapServerMessages( ) } - // A reasoning-only row with no following answer (interrupted turn): - // don't drop the trace — attach it to the last assistant message. - if (pendingReasoning != null) { - val lastAssistantIdx = mapped.indexOfLast { it.role == MessageRole.ASSISTANT } - if (lastAssistantIdx >= 0) { - val target = mapped[lastAssistantIdx] - if (target.reasoningText.isBlank()) { - mapped[lastAssistantIdx] = target.copy(reasoningText = pendingReasoning) - } - } + // REST echoes must not reserve a match before the richer WS copy of that tool. + val liveTools = liveMessages.filter { it.role == MessageRole.TOOL && !it.id.startsWith("rest-") } + val mappedTools = mapped.filter { it.role == MessageRole.TOOL } + 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 } - - return mapped } 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 43cbdcab..6d7e6125 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 @@ -107,48 +107,120 @@ internal fun canonicalToolResultKey(content: String): String? { internal fun sameLogicalMessage( a: ChatMessage, b: ChatMessage, -): Boolean { - if (a.role != b.role) return false - if (a.role == MessageRole.TOOL) { - // Issue #842: prefer the gateway's tool call id when both sides carry - // it — the REST transcript stores `tool_call_id` and the live WS - // bubble keeps it from `tool.start`. Content canonicalization cannot - // cover MCP/web tools: the REST side stores the payload as raw - // `` text (not JSON), so it has no key at all. - if (a.toolCallId.isNotBlank() && b.toolCallId.isNotBlank()) { - return a.toolCallId == b.toolCallId +): Boolean = TranscriptComparison().same(a, b) + +/** One operation owns this cache; even invalid JSON is parsed at most once per content. */ +internal class TranscriptComparison( + private val canonicalize: (String) -> String? = ::canonicalToolResultKey, +) { + private val toolKeys = HashMap() + private val trimmed = HashMap() + private val captions = HashMap() + + fun toolKey(content: String): String? { + if (!toolKeys.containsKey(content)) toolKeys[content] = canonicalize(content) + return toolKeys[content] + } + + fun same( + a: ChatMessage, + b: ChatMessage, + ): Boolean { + if (a.role != b.role) return false + val aRestId = a.canonicalRestId + val bRestId = b.canonicalRestId + if (aRestId != null && bRestId != null) return aRestId == bRestId + // #129: completion-bearing replies need confirmed identity, not repeated prose. + // Older pages deliberately do not acquire a live reply's completion in the mapper. + if (a.role == MessageRole.ASSISTANT && (a.completionId != null || b.completionId != null)) { + return a.completionId != null && a.completionId == b.completionId } - val ka = canonicalToolResultKey(a.content) - val kb = canonicalToolResultKey(b.content) - return ka != null && ka == kb + if (a.role == MessageRole.ASSISTANT && a.content.isBlank() && b.content.isBlank() && + (a.reasoningText.isNotBlank() || b.reasoningText.isNotBlank()) + ) { + return a.reasoningText.isNotBlank() && a.reasoningText == b.reasoningText + } + if (a.role == MessageRole.TOOL) { + if (a.toolCallId.isNotBlank() && b.toolCallId.isNotBlank()) return a.toolCallId == b.toolCallId + val key = toolKey(a.content) + return key != null && key == toolKey(b.content) + } + val ta = trimmed.getOrPut(a.content) { a.content.trim() } + val tb = trimmed.getOrPut(b.content) { b.content.trim() } + if (ta == tb) return true + if (a.role == MessageRole.USER && + captions.getOrPut(ta) { stripAttachmentRefLines(ta) } == + captions.getOrPut(tb) { stripAttachmentRefLines(tb) } + ) { + return true + } + return ta.length >= 40 && tb.length >= 40 && (tb.startsWith(ta) || ta.startsWith(tb)) } - val ta = a.content.trim() - val tb = b.content.trim() - if (ta == tb) return true - // Attachment dedupe: when the user sends an image/file, the backend - // persists an ENRICHED copy of the prompt — the real caption plus - // `@image:`/`@file:` reference lines and a `[screenshot]` marker - // (tui_gateway _build_image_ref_message / run_agent flattening). The - // optimistic bubble carries the raw caption only, so the exact compare - // above fails and the REST sync renders the same message twice. Compare - // USER rows on the caption with injection lines stripped (both sides — - // stripping a clean string is a no-op). USER-only so assistant text that - // legitimately mentions these tokens is never collapsed. - if (a.role == MessageRole.USER) { - val ca = stripAttachmentRefLines(ta) - val cb = stripAttachmentRefLines(tb) - if (ca == cb) return true +} + +/** Consume each occurrence once, reserving exact IDs before considering content echoes. */ +internal fun matchTranscriptMessages( + incoming: List, + existing: List, + comparison: TranscriptComparison = TranscriptComparison(), + allowAssistantContentMatches: Boolean = true, +): List { + val byId = existing.withIndex().associate { it.value.id to it.index } + val byRestId = + existing.withIndex().mapNotNull { (index, message) -> message.canonicalRestId?.let { it to index } }.toMap() + val used = BooleanArray(existing.size) + val matches = arrayOfNulls(incoming.size) + incoming.forEachIndexed { index, message -> + (byId[message.id] ?: message.canonicalRestId?.let { byRestId[it] })?.let { match -> + if (!used[match]) { + used[match] = true + matches[index] = existing[match] + } + } } - // Issue #842: a seal race can leave the live orphan a few trailing tokens - // short of the streamed narration (the last delta was still in the - // throttled buffer when tool.start sealed the message). The backend - // persists the COMPLETE copy, so the orphan is a strict prefix of the - // REST row. Accept prefix-covering only for substantial texts (>=40 - // chars) so a short reply can never be swallowed by a longer message - // that merely starts with it. - return ta.length >= 40 && - tb.length >= 40 && - (tb.startsWith(ta) || ta.startsWith(tb)) + val byRole = existing.indices.groupBy { existing[it].role } + val byCall = + existing.indices + .filter { existing[it].toolCallId.isNotBlank() } + .groupBy { existing[it].toolCallId } + val byResult = + existing.indices + .filter { existing[it].role == MessageRole.TOOL } + .groupBy { comparison.toolKey(existing[it].content) } + val matchOrder = + incoming.indices.filter { incoming[it].role != MessageRole.ASSISTANT } + + incoming.indices.reversed().filter { incoming[it].role == MessageRole.ASSISTANT } + matchOrder.forEach { index -> + val message = incoming[index] + if (matches[index] != null) return@forEach + val candidates = + if (message.role == MessageRole.TOOL) { + val callMatches = byCall[message.toolCallId].orEmpty() + callMatches + byResult[comparison.toolKey(message.content)].orEmpty() + } else { + // Prefix matching scans only candidates with the same role. + byRole[message.role].orEmpty().let { + if (message.role == MessageRole.ASSISTANT) it.asReversed() else it + } + } + val match = + candidates.firstOrNull { candidate -> + val other = existing[candidate] + !used[candidate] && + ( + allowAssistantContentMatches || message.role != MessageRole.ASSISTANT || + message.completionId != null || other.completionId != null + ) && + // Distinct IDs within the same source are separate occurrences, not echoes. + ((message.canonicalRestId == null) != (other.canonicalRestId == null)) && + comparison.same(message, other) + } + if (match != null) { + used[match] = true + matches[index] = existing[match] + } + } + return matches.toList() } /** @@ -176,55 +248,154 @@ internal fun stripAttachmentRefLines(content: String): String = * same call twice. Drop the `rest-` copy whenever a WS copy of the same * logical message exists (issue #771). */ -internal fun dedupeCachedMessages(messages: List): List { - val rest = messages.filter { it.id.startsWith("rest-") } - if (rest.isEmpty()) return messages - val nonRest = messages.filterNot { it.id.startsWith("rest-") } - if (nonRest.isEmpty()) return messages - val keepRest = rest.filter { restMsg -> nonRest.none { sameLogicalMessage(it, restMsg) } } - return (nonRest + keepRest).sortedBy { it.timestamp } +internal fun dedupeCachedMessages( + messages: List, + confirmedOnly: Boolean = false, +): List { + val unique = messages.dedupeById() + val rest = unique.filter { it.id.startsWith("rest-") } + val live = unique.filterNot { it.id.startsWith("rest-") } + if (rest.isEmpty() || live.isEmpty()) return unique + val matches = + matchTranscriptMessages(rest, live).mapIndexed { index, match -> + match?.takeIf { + !confirmedOnly || rest[index].canonicalRestId == it.canonicalRestId || + (rest[index].completionId != null && rest[index].completionId == it.completionId) + } + } + val echoes = + rest.indices + .filter { matches[it] != null } + .map { rest[it].id } + .toSet() + // #859: retain the REST identity when keeping a rich UUID row, including across cache pages. + val aliases = + rest.indices + .mapNotNull { index -> + matches[index]?.id?.let { it to rest[index] } + }.toMap() + return unique.filterNot { it.id in echoes }.map { message -> + aliases[message.id]?.let { + message.copy(restId = it.canonicalRestId, completionId = message.completionId ?: it.completionId) + } ?: message + } } -/** - * A transcript reload must NOT yank live WS bubbles the server has not - * persisted yet. The gateway stores a tool row only once the tool - * COMPLETES server-side, so a reload that lands while a tool is running - * (app background/foreground mid-turn, reconnect re-resume, pull-refresh) - * returns a page without the tool row — replacing the list outright made - * the in-flight tool bubble vanish and left tool.complete with no RUNNING - * message to update (issue #771). - * - * Merge instead of replace: append any current message the REST page does - * not already cover (checked by id AND logical content via - * [sameLogicalMessage]) and keep chronological order. - */ +/** Cache pages enrich confirmed identities without changing keys already on screen (#859). */ +internal fun mergeCachedTranscriptPage( + page: List, + current: List, +): List { + val currentById = current.associateBy { it.id } + // Legacy cached UUIDs may lack an alias. Restore a known alias before page-local matching + // so an older identical REST row cannot claim that UUID again. + val incoming = + dedupeCachedMessages( + page.map { message -> + currentById[message.id]?.restId?.let { message.copy(restId = it) } ?: message + }, + ) + val matches = matchTranscriptMessages(incoming, current) + val replacements = + 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 + match.id to + rich.copy( + id = match.id, + restId = message.canonicalRestId ?: match.canonicalRestId, + completionId = match.completionId ?: message.completionId, + ) + }.toMap() + return ( + current.map { replacements[it.id] ?: it } + + incoming.filterIndexed { index, _ -> matches[index] == null } + ).dedupeById().inTranscriptOrder().reconcileReasoningRows() +} + +/** Merge one page with the current snapshot without consuming repeated results more than once. */ internal fun mergeTranscriptWithLive( restMessages: List, currentMessages: List, + chronological: Boolean = true, + preserveLiveIds: Boolean = false, ): List { - val dedupedRest = restMessages.dedupeById() - val dedupedCurrent = currentMessages.dedupeById() - // User rows can carry live/cache-only metadata (for example whether the - // bubble continues the active turn). Keep that richer copy when REST - // returns the same logical row. - val currentUsers = dedupedCurrent.filter { it.role == MessageRole.USER }.toMutableList() - val mergedRest = - dedupedRest.map { rest -> - if (rest.role != MessageRole.USER) return@map rest - val matchIndex = - currentUsers.indexOfFirst { it.id == rest.id }.takeIf { it >= 0 } - ?: currentUsers.indexOfFirst { sameLogicalMessage(rest, it) } - if (matchIndex >= 0) currentUsers.removeAt(matchIndex) else rest + val incoming = restMessages.dedupeById() + val current = currentMessages.dedupeById() + val matches = matchTranscriptMessages(incoming, current, allowAssistantContentMatches = chronological) + val consumed = matches.mapNotNull { it?.id }.toSet() + val merged = + incoming.mapIndexed { index, message -> + val match = matches[index] + // Keep local user metadata and stable IDs already used by the renderer. + when { + match?.role == MessageRole.USER -> { + match.copy( + restId = (message.canonicalRestId ?: match.canonicalRestId).takeUnless { it == match.id }, + ) + } + + preserveLiveIds && match != null && !match.id.startsWith("rest-") -> { + match.copy( + restId = message.canonicalRestId ?: match.canonicalRestId, + content = message.content, + timestamp = message.timestamp, + isStreaming = message.isStreaming, + reasoningText = message.reasoningText.ifBlank { match.reasoningText }, + attachments = message.attachments ?: match.attachments, + toolName = message.toolName ?: match.toolName, + toolCallId = message.toolCallId.ifBlank { match.toolCallId }, + toolStatus = message.toolStatus ?: match.toolStatus, + displayKind = message.displayKind ?: match.displayKind, + tokenCount = message.tokenCount ?: match.tokenCount, + completionId = message.completionId ?: match.completionId, + ) + } + + else -> { + message.copy(completionId = message.completionId ?: match?.completionId) + } + } } - val restIds = dedupedRest.map { it.id }.toSet() - val liveTail = - dedupedCurrent.filter { old -> - old.id !in restIds && mergedRest.none { sameLogicalMessage(it, old) } + val transcript = + if (chronological) { + current.filterNot { it.id in consumed } + merged + } else { + merged + current.filterNot { it.id in consumed } } - if (liveTail.isEmpty()) return mergedRest.dedupeById() - return (mergedRest + liveTail).dedupeById().sortedBy { it.timestamp } + // #129: a mapped completion can confirm a cached REST row while its rich UUID + // copy is also present. Fold that now-confirmed echo without changing the live key. + // The page has already consumed its content matches; consuming another would + // collapse a separate repeated occurrence. Only confirmed identities can fold here. + return dedupeCachedMessages(transcript.inTranscriptOrder(), confirmedOnly = true).reconcileReasoningRows() } +/** Numeric canonical history first, then unconfirmed local insertion order; clocks never sort a transcript. */ +private fun List.inTranscriptOrder(): List = + sortedWith( + compareBy { it.canonicalOrder == null } + .thenBy { it.canonicalOrder ?: it.localOrder ?: Long.MAX_VALUE }, + ) + +private val ChatMessage.canonicalOrder: Long? + get() = if (localOrder != null && restId == null) null else canonicalRestId?.substringAfterLast('-')?.toLongOrNull() + +/** Retain the canonical trace once, without copying it across a user/system/tool boundary. */ +private fun List.reconcileReasoningRows(): List = + mapIndexed { index, message -> + val previous = getOrNull(index - 1) + if (previous?.role == MessageRole.ASSISTANT && previous.content.isBlank() && + previous.canonicalRestId != null && previous.reasoningText.isNotBlank() && + message.role == MessageRole.ASSISTANT && message.content.isNotBlank() && + message.canonicalRestId != null && message.reasoningText == previous.reasoningText + ) { + message.copy(reasoningText = "") + } else { + message + } + } + /** Merge a REST page without matching it against already-settled transcript rows. */ internal fun mergeIncrementalTranscriptPage( restMessages: List, @@ -234,7 +405,7 @@ internal fun mergeIncrementalTranscriptPage( ): List { val settledEnd = currentMessages.indexOfLast { message -> - serverMessageIndex(message.id, sessionId)?.let { it < offset } == true + serverMessageIndex(message.canonicalRestId ?: message.id, sessionId)?.let { it < offset } == true } return ( currentMessages.take(settledEnd + 1) + 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 ccde9151..09a0faa1 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 @@ -45,6 +45,8 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject @@ -374,6 +376,7 @@ class ChatViewModel( slashUsageStore: SlashUsageStore = SlashUsageStore(application.applicationContext), searchDispatcher: kotlinx.coroutines.CoroutineDispatcher = kotlinx.coroutines.Dispatchers.Default, private val ioDispatcher: kotlinx.coroutines.CoroutineDispatcher = kotlinx.coroutines.Dispatchers.IO, + private val historyDispatcher: kotlinx.coroutines.CoroutineDispatcher = searchDispatcher, ) : AndroidViewModel(application) { constructor(application: Application) : this(application, startCleanup = true) @@ -443,6 +446,15 @@ class ChatViewModel( */ private var latestPaging = false private var isSyncingMessages = false + private var cacheCursor: ChatPersistenceRepository.Cursor? = null + private var cacheHasOlder = false + private var cacheLoaded = false + private var serverHasOlder = false + private var cacheJob: Job? = null + private var hydrationJob: Job? = null + private var olderJob: Job? = null + private var syncJob: Job? = null + private val historyFetchMutex = Mutex() // ── Session resume recovery (desktop parity) ──────────────────────── // Bounded auto-retry with exponential backoff, mirroring the desktop's @@ -642,7 +654,19 @@ class ChatViewModel( activeHydrationRequestSequence = ++hydrationRequestSequence // Preserve metadata so late results and errors are rejected as stale. sessionGeneration++ - _uiState.update { it.copy(isSessionReady = false) } + // #129: invalidate and cancel paged work together, including its busy flags. + hydrationJob?.cancel() + olderJob?.cancel() + syncJob?.cancel() + cacheJob?.cancel() + isSyncingMessages = false + cancelResumeRetry() + _uiState.update { + it.copy(isSessionReady = false, isLoadingOlder = false, isResumeRetrying = false) + } + _uiState.value.currentSessionId?.let { sessionId -> + if (!cacheLoaded) loadCachedMessages(sessionId, sessionGeneration) + } // Fail any in-flight awaited RPCs so callers don't hang // across the disconnect (delegated to HermesWsClient, issue #526). wsClient.rejectAllPending() @@ -2904,151 +2928,201 @@ class ChatViewModel( } } + /** + * CPU work never runs in StateFlow.update: it may retry. If a WS event or another + * page changes the transcript during compute, recompute against that newer snapshot. + */ + private suspend fun mergeHistoryPage( + isCurrent: () -> Boolean, + cached: Boolean = false, + prepend: Boolean = false, + mapPage: (List) -> List, + ): List? { + while (isCurrent()) { + val snapshot = _uiState.value + val computed = + withContext(historyDispatcher) { + val page = mapPage(snapshot.messages) + val merged = + if (cached) { + mergeCachedTranscriptPage(page, snapshot.messages) + } else { + mergeTranscriptWithLive( + page, + snapshot.messages, + chronological = !prepend, + preserveLiveIds = true, + ) + } + val stableMessages = if (merged == snapshot.messages) snapshot.messages else merged + Triple(page, stableMessages, hydrateTodosFromMessages(merged).ifEmpty { snapshot.todos }) + } + if (!isCurrent()) return null + var applied = false + _uiState.update { current -> + applied = isCurrent() && current.messages === snapshot.messages && current.todos === snapshot.todos + if (applied) current.copy(messages = computed.second, todos = computed.third) else current + } + if (applied) return computed.first + } + return null + } + + private fun publishHistoryAvailability() { + _uiState.update { it.copy(hasOlderMessages = cacheHasOlder || serverHasOlder) } + } + + private suspend fun readCachedPage( + sessionId: String, + generation: Long, + isCurrent: () -> Boolean = { isCurrentSessionRequest(sessionId, generation) }, + ): Boolean { + val before = cacheCursor + // Room suspend queries own their IO executor; entity mapping/dedup runs on Default. + val page = withContext(historyDispatcher) { repo.loadPage(sessionId, before, MESSAGE_PAGE_SIZE) } + val valid = { isCurrent() && cacheCursor == before } + if (!valid()) return false + mergeHistoryPage(valid, cached = true) { page.messages } ?: return false + if (!valid()) return false + cacheCursor = page.cursor + cacheHasOlder = page.hasOlder + cacheLoaded = true + publishHistoryAvailability() + return true + } + private fun loadCachedMessages( sessionId: String, generation: Long, - ): Job = - viewModelScope.launch(ioDispatcher) { - val cachedMessages = dedupeCachedMessages(repo.loadMessages(sessionId)) - _uiState.update { state -> - // Only paint if still showing this session AND no fresher server - // page has landed yet (a fast REST fetch must not be clobbered - // by a stale cache read that loses the race). - if (isCurrentSessionRequest(sessionId, generation) && - state.messages.isEmpty() && - cachedMessages.isNotEmpty() - ) { - state.copy( - messages = cachedMessages, - isLoading = false, - todos = hydrateTodosFromMessages(cachedMessages), - ) - } else { - state + ): Job { + cacheJob?.cancel() + return viewModelScope + .launch { + try { + readCachedPage(sessionId, generation) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + if (isCurrentSessionRequest(sessionId, generation)) { + // The first read can be retried by the same older-history action. + cacheHasOlder = true + publishHistoryAvailability() + _uiState.update { it.copy(errorMessage = "Failed to load cached messages: ${e.message}") } + } + } finally { + if (isCurrentSessionRequest(sessionId, generation) && _uiState.value.messages.isNotEmpty()) { + _uiState.update { it.copy(isLoading = false) } + } } - } - } + }.also { cacheJob = it } + } private fun loadSessionMessages( sessionId: String, generation: Long, ) { + if (activeHydrationRequestSequence != 0L && cacheJob?.isActive == true) { + cacheJob?.cancel() + if (!cacheLoaded) loadCachedMessages(sessionId, generation) + } val requestSequence = ++hydrationRequestSequence activeHydrationRequestSequence = requestSequence - viewModelScope.launch { - // Initial page: ask for the NEWEST page directly (order=latest — - // offset is measured back from the newest message, page returned - // chronologically; verified in hermes_state.py get_messages). - // No count-based anchor, so a stale session-list message_count can - // no longer land the page at the wrong position (issue #859). - // Legacy backends without the `order` param ignore it and echo no - // `pagination` — detect that and fall back to the count-based - // anchor so nothing regresses. - val latestResult = fetchMessagePage(sessionId, 0, MESSAGE_PAGE_SIZE, order = "latest") - if (!isCurrentHydration(sessionId, generation, requestSequence)) return@launch - val (result, requestedOffset) = - if (latestResult is NetworkResult.Success && latestResult.data.pagination?.order == "latest") { - latestPaging = true - latestResult to 0 - } else if (latestResult is NetworkResult.Success) { - latestPaging = false - val messageCount = fetchServerMessageCount(sessionId, generation, requestSequence) - if (!isCurrentHydration(sessionId, generation, requestSequence)) return@launch - val offset = (messageCount - MESSAGE_PAGE_SIZE).coerceAtLeast(0) - fetchMessagePage(sessionId, offset, MESSAGE_PAGE_SIZE) to offset - } else { - latestPaging = false - latestResult to 0 - } - if (!isCurrentHydration(sessionId, generation, requestSequence)) return@launch - when (result) { - is NetworkResult.Success -> { - // REST 200 — the gateway has the row for this session. - sessionHasServerPresence = true - val serverOffset = result.data.pagination?.offset ?: result.data.offset ?: requestedOffset - val chatMessages = - mapServerMessages( - sessionId, - result.data.messages.orEmpty(), - serverOffset, - latestPaging, - _uiState.value.messages, - context = getApplication(), - ) - loadedMessageOffset = serverOffset - withContext(ioDispatcher) { - repo.persistMessages(chatMessages, sessionId) - } - if (!isCurrentHydration(sessionId, generation, requestSequence)) return@launch - _uiState.update { state -> - if (!isCurrentHydration(sessionId, generation, requestSequence)) return@update state - // Merge, don't replace: a reload mid-turn must not - // drop live WS bubbles (running tool call, streaming - // answer) the server hasn't persisted yet. Issue #771. - val merged = mergeTranscriptWithLive(chatMessages, state.messages) - val hasOlder = - if (latestPaging) { - // Newest-anchored: older messages exist iff the - // page came back FULL (a short page means we hit - // the oldest boundary). - val returned = result.data.pagination?.returned ?: chatMessages.size - returned >= MESSAGE_PAGE_SIZE && chatMessages.isNotEmpty() + hydrationJob?.cancel() + olderJob?.cancel() + syncJob?.cancel() + isSyncingMessages = false + _uiState.update { it.copy(isLoadingOlder = false) } + val valid = { isCurrentHydration(sessionId, generation, requestSequence) } + hydrationJob = + viewModelScope.launch { + try { + val latestResult = fetchMessagePage(sessionId, 0, MESSAGE_PAGE_SIZE, order = "latest") + if (!valid()) return@launch + // Backends without stable latest IDs use the legacy absolute-position protocol. + val useLatest = + latestResult is NetworkResult.Success && + latestResult.data.pagination?.order == "latest" && + latestResult.data.messages.all { it.id != null } + val (result, requestedOffset) = + if (useLatest || latestResult is NetworkResult.Failure) { + latestResult to 0 + } else { + val count = fetchServerMessageCount(sessionId, generation, requestSequence) + if (!valid()) return@launch + val offset = (count - MESSAGE_PAGE_SIZE).coerceAtLeast(0) + fetchMessagePage(sessionId, offset, MESSAGE_PAGE_SIZE) to offset + } + if (!valid()) return@launch + when (result) { + is NetworkResult.Success -> { + val serverOffset = result.data.pagination?.offset ?: result.data.offset ?: requestedOffset + val raw = result.data.messages + val page = + mergeHistoryPage(valid) { current -> + mapServerMessages( + sessionId, + raw, + serverOffset, + useLatest, + current, + context = getApplication(), + ) + } ?: return@launch + persistHistoryPage(page, sessionId) + if (!valid()) return@launch + latestPaging = useLatest + // Cursor counts RAW rows, including hidden reasoning/placeholder rows. + loadedMessageOffset = if (useLatest) serverOffset + raw.size else serverOffset + serverHasOlder = raw.isNotEmpty() && + if (useLatest) raw.size >= MESSAGE_PAGE_SIZE else serverOffset > 0 + sessionHasServerPresence = true + publishHistoryAvailability() + hydratedGeneration = generation + finishResumeWhenHydrated(generation) + } + + is NetworkResult.Failure -> { + val error = result.error.message + if (error.contains("404", ignoreCase = true) || + error.contains("not found", ignoreCase = true) + ) { + serverHasOlder = false + publishHistoryAvailability() + hydratedGeneration = generation + finishResumeWhenHydrated(generation) } else { - serverOffset > 0 && chatMessages.isNotEmpty() + if (hydratedGeneration == generation) hydratedGeneration = -1L + handleResumeFailure(sessionId, generation, "Failed to load messages: $error") } - state.copy( - messages = merged, - todos = hydrateTodosFromMessages(merged), - isLoading = false, - hasOlderMessages = hasOlder, - isLoadingOlder = false, - ) - } - hydratedGeneration = generation - finishResumeWhenHydrated(generation) - } - - is NetworkResult.Failure -> { - val errorMsg = result.error.message - val is404 = - errorMsg.contains("404", ignoreCase = true) || - errorMsg.contains("not found", ignoreCase = true) - if (is404) { - // A 404 from GET /api/sessions/{id}/messages indicates the session has no - // persisted rows in state.db (e.g. newly created/lazy unprompted session, or empty history). - // It is NOT a fatal error: keep the session open with empty transcript, clear loading, - // and mark hydrated cleanly. - _uiState.update { - if (!isCurrentHydration(sessionId, generation, requestSequence)) return@update it - it.copy( - isLoading = false, - isLoadingOlder = false, - hasOlderMessages = false, - ) } - hydratedGeneration = generation - finishResumeWhenHydrated(generation) - return@launch } - if (hydratedGeneration == generation) hydratedGeneration = -1L - _uiState.update { - if (!isCurrentHydration(sessionId, generation, requestSequence)) return@update it - it.copy( - isLoading = false, - isLoadingOlder = false, - ) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + if (valid()) { + hydratedGeneration = -1L + handleResumeFailure(sessionId, generation, "Failed to load messages: ${e.message}") } - // Route the transcript failure through the bounded resume - // retry (desktop parity) instead of a one-shot snackbar — - // a transient backend/network blip recovers on its own, - // and a persistent failure ends in an explicit Retry. - handleResumeFailure( - sessionId, - generation, - "Failed to load messages: ${result.error.message}", - ) + } finally { + if (valid()) _uiState.update { it.copy(isLoading = false) } } } + } + + private suspend fun persistHistoryPage( + page: List, + sessionId: String, + ) { + // A mapped page can reuse a live WS message. Never overwrite its newer persisted + // version with the snapshot used for mapping; WS owns persistence of those IDs. + val pageIds = page.mapNotNull { it.canonicalRestId }.toSet() + val aliases = _uiState.value.messages.filter { it.restId in pageIds && !it.id.startsWith("rest-") } + withContext(historyDispatcher) { + repo.persistMessages( + page.mapNotNull { message -> message.canonicalRestId?.let { message.copy(id = it, restId = null) } }, + sessionId, + ) + repo.confirmIdentities(aliases, sessionId) } } @@ -3096,6 +3170,15 @@ class ChatViewModel( runtimeSessionId = null pendingInitialPrompt = null ActiveSessionHolder.clear() + cacheJob?.cancel() + hydrationJob?.cancel() + olderJob?.cancel() + syncJob?.cancel() + activeHydrationRequestSequence = 0L + cacheCursor = null + cacheHasOlder = false + cacheLoaded = false + serverHasOlder = false loadedMessageOffset = 0 latestPaging = false isSyncingMessages = false @@ -3372,222 +3455,150 @@ class ChatViewModel( fun loadOlderMessages() { val state = _uiState.value val sessionId = state.currentSessionId ?: return + if (!state.hasOlderMessages || state.isLoadingOlder || olderJob?.isActive == true) return + if (cacheJob?.isActive == true) return + val fromCache = cacheHasOlder || !cacheLoaded + if (!fromCache && (hydrationJob?.isActive == true || isSyncingMessages || !serverHasOlder)) return val generation = sessionGeneration - if (!state.hasOlderMessages || state.isLoadingOlder) return - if (!latestPaging && loadedMessageOffset <= 0) return - val oldOffset = loadedMessageOffset - // latest: offsets count BACK from the newest message, so older pages go - // UP; legacy: absolute offsets go DOWN toward 0 (issue #859). - val newOffset = - if (latestPaging) { - oldOffset + MESSAGE_PAGE_SIZE - } else { - (oldOffset - MESSAGE_PAGE_SIZE).coerceAtLeast(0) - } - // Legacy: the final page can be short; requesting a full page at a - // clamped offset OVERLAPS already-loaded rows (duplicate stable keys - // crash LazyColumn), so size the request to the real gap. Latest: - // pages are disjoint from-end ranges, so a full page never overlaps. - val limit = if (latestPaging) MESSAGE_PAGE_SIZE else oldOffset - newOffset + val requestSequence = activeHydrationRequestSequence + val valid = { isCurrentHydration(sessionId, generation, requestSequence) } _uiState.update { it.copy(isLoadingOlder = true) } - viewModelScope.launch { - val result = - fetchMessagePage( - sessionId, - newOffset, - limit, - order = if (latestPaging) "latest" else null, - ) - when (result) { - is NetworkResult.Success -> { - if (!isCurrentSessionRequest(sessionId, generation)) return@launch - val returnedOffset = result.data.pagination?.offset ?: result.data.offset ?: newOffset - val older = - mapServerMessages( - sessionId, - result.data.messages.orEmpty(), - returnedOffset, - latestPaging, - _uiState.value.messages, - isPagingOlder = true, - ) - loadedMessageOffset = returnedOffset - withContext(ioDispatcher) { repo.persistMessages(older, sessionId) } - _uiState.update { current -> - if (!isCurrentSessionRequest(sessionId, generation)) return@update current - val hasOlder = - if (latestPaging) { - // Full page = more older messages behind it; a - // short (or empty) page is the oldest boundary. - val returned = result.data.pagination?.returned ?: older.size - returned >= limit && older.isNotEmpty() - } else { - returnedOffset < oldOffset && older.isNotEmpty() && returnedOffset > 0 + olderJob = + viewModelScope.launch { + try { + while (cacheHasOlder || !cacheLoaded) { + val beforeMessages = _uiState.value.messages + if (!readCachedPage(sessionId, generation, valid)) return@launch + val afterMessages = _uiState.value.messages + val addedRows = + withContext(historyDispatcher) { + val previousIds = beforeMessages.mapTo(HashSet()) { it.id } + afterMessages.any { it.id !in previousIds } } - current.copy( - messages = (older + current.messages).distinctBy { it.id }, - isLoadingOlder = false, - hasOlderMessages = hasOlder, - ) + if (!valid()) return@launch + if (addedRows) return@launch + // Hydration persists canonical rows behind the local cache cursor. Consume + // those already-visible echoes without spending another older-history action. } - } + if (hydrationJob?.isActive == true || isSyncingMessages || !serverHasOlder) return@launch + // Cache reads may suspend through hydration/sync; use the current server cursor. + val useLatest = latestPaging + val oldOffset = loadedMessageOffset + val offset = if (useLatest) oldOffset else (oldOffset - MESSAGE_PAGE_SIZE).coerceAtLeast(0) + val limit = if (useLatest) MESSAGE_PAGE_SIZE else oldOffset - offset + if (limit <= 0) return@launch + val result = fetchMessagePage(sessionId, offset, limit, order = if (useLatest) "latest" else null) + if (!valid()) return@launch + when (result) { + is NetworkResult.Success -> { + val returnedOffset = result.data.pagination?.offset ?: result.data.offset ?: offset + val raw = result.data.messages + val page = + mergeHistoryPage(valid, prepend = true) { current -> + mapServerMessages( + sessionId, + raw, + returnedOffset, + useLatest, + current, + isPagingOlder = true, + context = getApplication(), + ) + } ?: return@launch + persistHistoryPage(page, sessionId) + if (!valid()) return@launch + loadedMessageOffset = if (useLatest) returnedOffset + raw.size else returnedOffset + serverHasOlder = raw.isNotEmpty() && + if (useLatest) { + loadedMessageOffset > oldOffset && raw.size >= limit + } else { + returnedOffset < oldOffset && returnedOffset > 0 + } + publishHistoryAvailability() + } - is NetworkResult.Failure -> { - _uiState.update { - if (isCurrentSessionRequest(sessionId, generation)) { - it.copy(isLoadingOlder = false) - } else { - it + is NetworkResult.Failure -> { + _uiState.update { + it.copy( + errorMessage = "Failed to load older messages: ${result.error.message}", + ) + } } } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + if (valid()) { + _uiState.update { + it.copy( + errorMessage = "Failed to load older messages: ${e.message}", + ) + } + } + } finally { + if (valid()) _uiState.update { it.copy(isLoadingOlder = false) } } } - } } fun syncCurrentSession() { - // Issue #840: a session created but never prompted has no server row - // yet — the REST transcript 404s and the resume-retry machinery spins - // forever (visible in logcat as repeated /messages 404s). MessageStart - // flips the flag once the first prompt persists the row. if (!sessionHasServerPresence) return val state = _uiState.value val sessionId = state.currentSessionId ?: return - val generation = sessionGeneration - if (isSyncingMessages || state.isLoading || state.isLoadingOlder || state.isAgentTyping || - _streamingState.value.streamingMessage != null + if (isSyncingMessages || hydrationJob?.isActive == true || state.isLoading || state.isLoadingOlder || + state.isAgentTyping || _streamingState.value.streamingMessage != null ) { return } + val generation = sessionGeneration + val requestSequence = activeHydrationRequestSequence + val valid = { isCurrentHydration(sessionId, generation, requestSequence) } + val useLatest = latestPaging val nextOffset = - if (latestPaging) { - // Newest-anchored paging can't compute an absolute - // "after my last row" offset (from-end offsets shift as the - // transcript grows), so refetch the newest page; the logical - // merge below keeps existing copies and only adds new rows - // (issue #859). + if (useLatest) { 0 } else { state.messages - .mapNotNull { serverMessageIndex(it.id, sessionId) } + .mapNotNull { serverMessageIndex(it.canonicalRestId ?: it.id, sessionId) } .maxOrNull() ?.plus(1) ?: loadedMessageOffset } isSyncingMessages = true - viewModelScope.launch { - try { - val result = - fetchMessagePage( - sessionId, - nextOffset, - MESSAGE_PAGE_SIZE, - order = if (latestPaging) "latest" else null, - ) - when (result) { - is NetworkResult.Success -> { - if (!isCurrentSessionRequest(sessionId, generation)) return@launch - val incoming = - mapServerMessages( - sessionId, - result.data.messages.orEmpty(), - nextOffset, - latestPaging, - _uiState.value.messages, - isPagingOlder = true, - ) - if (incoming.isEmpty()) return@launch - withContext(ioDispatcher) { repo.persistMessages(incoming, sessionId) } - _uiState.update { current -> - if (!isCurrentSessionRequest(sessionId, generation)) return@update current - // Issue #771: the sync merge was dropping the - // newest tool bubble — the incoming REST page - // didn't include it yet (server persists tool rows - // at completion, but the sync offset may predate - // that), and the fragile toolName/content match - // consumed the WRONG incoming tool for an existing - // one, leaving the newest WS tool with no match - // → dropped. Use sameLogicalMessage (canonical - // result-key match) and always preserve any WS - // message that has no REST counterpart. - val unmatchedIncoming: MutableList = incoming.toMutableList() - val incomingById = HashMap(unmatchedIncoming.size) - for (i in unmatchedIncoming.indices) { - val id = unmatchedIncoming[i]?.id - if (id != null && !incomingById.containsKey(id)) { - incomingById[id] = i - } - } - - val mergedList = mutableListOf() - - for (existing in current.messages) { - val existingServerIndex = serverMessageIndex(existing.id, sessionId) - if (existingServerIndex != null) { - val matchIdx = incomingById[existing.id] - if (matchIdx != null && unmatchedIncoming[matchIdx] != null) { - mergedList.add(unmatchedIncoming[matchIdx]!!) - unmatchedIncoming[matchIdx] = null - } else if (latestPaging) { - // Newest-anchored paging re-keys rows by - // from-end position, so a transcript that - // grew since the last fetch shifted ids — - // match by logical content and keep the - // existing copy (issue #859). - val logicalIdx = - unmatchedIncoming.indexOfFirst { inc -> - inc != null && sameLogicalMessage(inc, existing) - } - if (logicalIdx >= 0) { - mergedList.add(existing) - unmatchedIncoming[logicalIdx] = null - } else { - mergedList.add(existing) - } - } else { - mergedList.add(existing) - } - } else { - // WS message (UUID id, no server index): - // match by canonical content, not fragile - // toolName/content equality. - val matchIdx = - unmatchedIncoming.indexOfFirst { inc -> - inc != null && sameLogicalMessage(inc, existing) - } - if (matchIdx >= 0) { - // Prefer the WS copy (richer payload, - // real tool name) when available. - mergedList.add(existing) - unmatchedIncoming[matchIdx] = null - } else { - // No REST counterpart (server hasn't - // persisted yet) — KEEP the WS message. - mergedList.add(existing) - } - } - } - - for (inc in unmatchedIncoming) { - if (inc != null) { - mergedList.add(inc) - } - } - val merged = mergedList.distinctBy { it.id } - if (sameMessages(current.messages, merged)) { - current - } else { - current.copy(messages = merged) - } - } + syncJob = + viewModelScope.launch { + try { + val result = + fetchMessagePage(sessionId, nextOffset, MESSAGE_PAGE_SIZE, if (useLatest) "latest" else null) + if (!valid()) return@launch + if (result is NetworkResult.Success) { + val offset = result.data.pagination?.offset ?: result.data.offset ?: nextOffset + val page = + mergeHistoryPage(valid) { current -> + mapServerMessages( + sessionId, + result.data.messages, + offset, + useLatest, + current, + // Sync fetches recent replies, so confirm live completion identities. + isPagingOlder = false, + context = getApplication(), + ) + } ?: return@launch + persistHistoryPage(page, sessionId) + // Never derive the older cursor from displayed rows or reset it to this latest page. + // Append-only growth shifts from-end offsets toward newer rows: the next older + // request may overlap, but stable IDs remove echoes without skipping any history. } - - is NetworkResult.Failure -> {} + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + if (valid()) _uiState.update { it.copy(errorMessage = "Failed to sync messages: ${e.message}") } + } finally { + if (valid()) isSyncingMessages = false } - } finally { - if (generation == sessionGeneration) isSyncingMessages = false } - } } internal fun onModelSwitchInitiated() { @@ -3926,15 +3937,17 @@ class ChatViewModel( offset: Int, limit: Int, order: String? = null, - ) = withContext(ioDispatcher) { - safeApiCall { - ApiClient.hermesApi.getSessionMessages( - sessionId = sessionId, - limit = limit, - offset = offset, - includeCompacted = true, - order = order, - ) + ) = historyFetchMutex.withLock { + withContext(ioDispatcher) { + safeApiCall { + ApiClient.hermesApi.getSessionMessages( + sessionId = sessionId, + limit = limit, + offset = offset, + includeCompacted = true, + order = order, + ) + } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatHistoryPrefetch.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatHistoryPrefetch.kt new file mode 100644 index 00000000..eed574ec --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatHistoryPrefetch.kt @@ -0,0 +1,29 @@ +package com.m57.hermescontrol.ui.chat.components + +/** One older-page request per deliberate upward gesture, including an overscroll at index zero. */ +internal class ChatHistoryPrefetch { + private var upwardPixels = 0f + private var requested = false + + fun onScroll( + firstVisibleIndex: Int, + deltaY: Float, + userInput: Boolean, + canLoad: Boolean, + ): Boolean { + if (userInput && deltaY < 0f) upwardPixels = 0f + if (deltaY <= 0f || !canLoad || requested) return false + // A fling retains the drag's intent until scrolling stops. Programmatic + // scrolling without an upward user gesture must never load history. + if (!userInput && upwardPixels == 0f) return false + upwardPixels += deltaY + if (firstVisibleIndex > 25 || upwardPixels < 24f) return false + requested = true + return true + } + + fun endGesture() { + upwardPixels = 0f + requested = false + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatScrollController.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatScrollController.kt index faafe0d7..20e76b3d 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatScrollController.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatScrollController.kt @@ -33,9 +33,9 @@ import kotlinx.coroutines.yield * 5. **Unread indicator** — when follow is paused, [pendingCount] accumulates the * number of tail updates; shows on the FAB. Tapping the FAB resumes following * and clears the count. - * 6. **Paging anchor preservation** — capture the first visible item + offset - * with [captureAnchor] before prepending older history, then - * [restoreAnchor] after insertion so the same content stays under the eye. + * 6. **Paging anchors** — the full-bleed renderer preserves the live lazy row + * key and offset when older history is inserted; this controller owns no + * fetch-time anchor or animated compensation. * 7. **Serialized scroll commands** — every scroll (send, FAB, session switch, * search navigation, auto-follow) is launched from [scope] so animations * don't compete. @@ -189,30 +189,17 @@ class ChatScrollController( } } + /** An explicit history gesture must not be undone by a short list's bottom-follow. */ + fun pauseFollowing() { + isFollowingBottom = false + } + /** FAB tap: resume following + clear unread. */ fun resumeFollowing() = jumpToBottom(animated = true) /** True when the FAB should be visible (not following bottom + content exists). */ fun showFab(contentPresent: Boolean): Boolean = !isFollowingBottom && contentPresent - /** - * Current pixel scroll offset of the first visible item — captured before - * prepending older history so it can be restored after insertion. - */ - fun captureAnchorOffset(): Int = listState.firstVisibleItemScrollOffset - - /** - * Restore the viewport to a specific item (resolved by id in the caller, so - * the restore is robust even if streaming tokens arrive during the page - * insert). Keeps the reader's eye on the same content after prepend. - */ - fun scrollToItem( - index: Int, - offset: Int, - ) { - scope.launch { listState.scrollToItem(index, offset) } - } - /** * Navigate to a search match (serialized through [scope]). * @@ -258,7 +245,7 @@ fun tailContentKey( clarifyRequest: Any?, ): Any = listOf( - messages.size, + messages.lastOrNull(), streamingMessage?.hashCode() ?: 0, isThinking, subagentIndicators.size, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt index cd861d78..33d24c21 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt @@ -4,7 +4,6 @@ import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn @@ -12,9 +11,17 @@ import androidx.compose.material3.CircularProgressIndicator import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.SideEffect +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.input.nestedscroll.NestedScrollConnection +import androidx.compose.ui.input.nestedscroll.NestedScrollSource +import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.Density @@ -32,6 +39,7 @@ import com.m57.hermescontrol.ui.chat.ToolCallDivider import com.m57.hermescontrol.ui.chat.VaultCodePromptUi import com.m57.hermescontrol.ui.chat.VaultSaveLoginPromptUi import com.m57.hermescontrol.ui.chat.VaultUnlockPromptUi +import com.m57.hermescontrol.ui.chat.components.ChatHistoryPrefetch import com.m57.hermescontrol.ui.chat.components.ChatScrollController import com.m57.hermescontrol.ui.chat.components.ClarifyBubble import com.m57.hermescontrol.ui.chat.components.ReasoningCard @@ -93,6 +101,9 @@ fun FullBleedChatList( savingAttachmentPath: String? = null, openingAttachmentPath: String? = null, onImageClick: (ImageViewerModel) -> Unit = {}, + hasOlderMessages: Boolean = false, + pagingSessionId: String? = null, + onLoadOlder: () -> Unit = viewModel::loadOlderMessages, ) { if (messages.isEmpty() && !isLoading && !isAgentTyping) { Box( @@ -105,19 +116,133 @@ fun FullBleedChatList( ) } } else { - val toolMilestones = remember(messages) { toolCallMilestones(messages) } + // Keep only the incoming older prefix out of layout during a drag/fling. + // The input remains authoritative: overlap updates, new messages and streaming + // stay live, and a session change resets the boundary immediately. + val previousFirstId = remember(listState, pagingSessionId) { mutableStateOf(messages.firstOrNull()?.id) } + val firstRenderedId = previousFirstId.value + val olderPrefixSize = + remember(messages, firstRenderedId) { + if (firstRenderedId != null && firstRenderedId != messages.firstOrNull()?.id) { + messages.indexOfFirst { it.id == firstRenderedId }.coerceAtLeast(0) + } else { + 0 + } + } + val hiddenPrefixSize = if (olderPrefixSize > 0 && listState.isScrollInProgress) olderPrefixSize else 0 + val renderedMessages = + remember(messages, hiddenPrefixSize) { + if (hiddenPrefixSize > 0) messages.subList(hiddenPrefixSize, messages.size) else messages + } + val toolMilestones = remember(renderedMessages) { toolCallMilestones(renderedMessages) } + val settledTurns = remember(renderedMessages) { groupIntoTurns(renderedMessages) } + val settledIds = remember(renderedMessages) { renderedMessages.mapTo(HashSet()) { it.id } } val turns = - remember(messages, streamingState.streamingMessage) { - groupIntoTurnsWithStreaming(messages, streamingState.streamingMessage) + remember(settledTurns, streamingState.streamingMessage) { + appendStreamingTurn(settledTurns, settledIds, streamingState.streamingMessage) } - val agentStatus = deriveAgentStatus(isAgentTyping, streamingState, messages) - // messageId → LazyColumn item index. The lazy list has EXTRA items - // vs the message list (reasoning hoists, tool rows), so search-match - // scrolling must resolve the lazy index, not use the message index. - val lazyIndexById = - remember(turns, isLoadingOlder) { - messageIdToLazyIndex(turns, leadingItems = if (isLoadingOlder) 1 else 0) + val agentStatus = deriveAgentStatus(isAgentTyping, streamingState, renderedMessages) + // A single ordered definition supplies both emitted tail rows and anchor keys. + val tailItems = + buildMap Unit> { + agentStatus?.let { status -> + put("agent_status") { + AgentStatusIndicator(status = status) + } + } + + // Clarify bubble — rendered at the very bottom + if (clarifyRequest != null) { + put("clarify_bubble") { + ClarifyBubble( + clarifyRequest = clarifyRequest, + onRespondSingle = { option -> onRespondClarify?.invoke(option) }, + onRespondBatch = { answers -> onRespondClarifyBatch?.invoke(answers) }, + onDismiss = { onDismissClarify?.invoke() }, + ) + } + } + + if (vaultUnlockPrompt != null) { + put("vault_unlock_card") { + VaultUnlockCard( + prompt = vaultUnlockPrompt, + onConfirm = { password -> onRespondVaultUnlock?.invoke(password) }, + onDismiss = { onDismissVaultUnlock?.invoke() }, + ) + } + } + + if (vaultSaveLoginPrompt != null) { + put("vault_save_login_card") { + VaultSaveLoginCard( + prompt = vaultSaveLoginPrompt, + onConfirm = { identifier, password -> + onRespondVaultSaveLogin?.invoke(identifier, password) + }, + onDismiss = { onDismissVaultSaveLogin?.invoke() }, + ) + } + } + + if (vaultCodePrompt != null) { + put("vault_code_card") { + VaultCodeCard( + prompt = vaultCodePrompt, + onConfirm = { code -> onRespondVaultCode?.invoke(code) }, + onDismiss = { onDismissVaultCode?.invoke() }, + ) + } + } } + // custom23 prepend regression: a 150-row page can move the anchor beyond + // Compose's nearest-key lookup window. Capture the LIVE layout at insertion, + // not when fetching starts, and request its new index before the next measure. + // A staged prefix only reaches here at idle, so this cannot stop its drag/fling. + SideEffect { + val oldFirstId = previousFirstId.value + val firstId = renderedMessages.firstOrNull()?.id + if (oldFirstId != null && oldFirstId != firstId && renderedMessages.any { it.id == oldFirstId }) { + val firstVisibleIndex = listState.firstVisibleItemIndex + val anchor = listState.layoutInfo.visibleItemsInfo.firstOrNull { it.index == firstVisibleIndex } + if (anchor != null) { + val newIndex = (fullBleedItemKeys(turns) + tailItems.keys).indexOf(anchor.key) + if (newIndex >= 0 && newIndex != anchor.index) { + listState.requestScrollToItem(newIndex, listState.firstVisibleItemScrollOffset) + } + } + } + previousFirstId.value = firstId + } + val prefetch = remember(pagingSessionId) { ChatHistoryPrefetch() } + val canLoad = rememberUpdatedState(hasOlderMessages && !isLoadingOlder && !isLoading) + val loadOlder = rememberUpdatedState(onLoadOlder) + val prefetchConnection = + remember(listState, prefetch, scrollController) { + object : NestedScrollConnection { + override fun onPreScroll( + available: Offset, + source: NestedScrollSource, + ): Offset { + if (prefetch.onScroll( + firstVisibleIndex = listState.firstVisibleItemIndex, + deltaY = available.y, + userInput = source == NestedScrollSource.UserInput, + canLoad = canLoad.value, + ) + ) { + scrollController.pauseFollowing() + loadOlder.value() + } + return Offset.Zero + } + } + } + LaunchedEffect(listState, prefetch) { + snapshotFlow { listState.isScrollInProgress }.collect { scrolling -> + if (!scrolling) prefetch.endGesture() + } + } // Scroll the current search match into view, word-focused. Lives here // (not in ChatLifecycleEffects) because only this composable knows @@ -128,6 +253,7 @@ fun FullBleedChatList( searchState.currentIndex, searchState.matchIndices, searchState.matchOffsets, + renderedMessages.firstOrNull()?.id, ) { if (searchState.isActive && searchState.currentIndex >= 0 && @@ -135,6 +261,9 @@ fun FullBleedChatList( ) { val messageIndex = searchState.matchIndices[searchState.currentIndex] if (messageIndex < 0 || messageIndex >= messages.size) return@LaunchedEffect + // Search indices refer to incoming messages; resolve by id into the + // rendered rows. A hit in a staged prefix is retried when it becomes visible. + val lazyIndexById = messageIdToLazyIndex(turns) val lazyIndex = lazyIndexById[messages[messageIndex].id] ?: return@LaunchedEffect val contentOffset = searchState.matchOffsets.getOrElse(searchState.currentIndex) { 0 } val contentLength = messages[messageIndex].content.length @@ -153,202 +282,163 @@ fun FullBleedChatList( } CompositionLocalProvider(LocalDensity provides chatDensity) { - LazyColumn( - state = listState, - modifier = Modifier.fillMaxSize(), - contentPadding = PaddingValues(vertical = 8.dp), - ) { - if (isLoadingOlder) { - item(key = "loading-older") { - Box( - modifier = Modifier.fillMaxWidth().padding(12.dp), - contentAlignment = Alignment.Center, - ) { - CircularProgressIndicator(modifier = Modifier.size(24.dp)) - } - } - } - - turns.forEach { turn -> - when (turn) { - is ChatTurn.User -> { - // Eager captures: item lambda reads these at - // composition time (lazy), so capture now. - val userMessage = turn.message - item(key = "user-${userMessage.id}") { - Column(modifier = Modifier.padding(bottom = 12.dp)) { - renderChatBubble( - message = userMessage, - searchQuery = if (searchState.isActive) searchState.query else "", - isCurrentMatch = - searchState.currentMatchId != null && - searchState.currentMatchId == userMessage.id, - onOpenAttachment = viewModel::openAttachment, - onSaveAttachment = onSaveAttachment, - savingAttachmentPath = savingAttachmentPath, - openingAttachmentPath = openingAttachmentPath, - onImageClick = onImageClick, - messageStatsEnabled = messageStatsEnabled, - showUserMessageTokens = showUserMessageTokens, - ) - } - } - } - - is ChatTurn.Agent -> { - // Reasoning hoist: the turn's reasoning renders at the - // TOP of the turn — above tool rows — so thinking - // leads, then the tool work, then the answer. The - // matching prose entry renders without its own card. - val turnReasoning = - turn.entries - .filterIsInstance() - .firstOrNull { it.message.reasoningText.isNotBlank() } - if (turnReasoning != null) { - val reasoning = turnReasoning.message - item(key = "reasoning-${reasoning.id}") { - Column(modifier = Modifier.padding(bottom = 6.dp)) { - ReasoningCard( - reasoningText = reasoning.reasoningText, - isStreaming = reasoning.isStreaming, + Box(Modifier.fillMaxSize()) { + LazyColumn( + state = listState, + modifier = Modifier.fillMaxSize().nestedScroll(prefetchConnection), + contentPadding = PaddingValues(vertical = 8.dp), + ) { + turns.forEach { turn -> + when (turn) { + is ChatTurn.User -> { + // Eager captures: item lambda reads these at + // composition time (lazy), so capture now. + val userMessage = turn.message + item(key = "user-${userMessage.id}") { + Column(modifier = Modifier.padding(bottom = 12.dp)) { + renderChatBubble( + message = userMessage, + searchQuery = if (searchState.isActive) searchState.query else "", + isCurrentMatch = + searchState.currentMatchId != null && + searchState.currentMatchId == userMessage.id, + onOpenAttachment = viewModel::openAttachment, + onSaveAttachment = onSaveAttachment, + savingAttachmentPath = savingAttachmentPath, + openingAttachmentPath = openingAttachmentPath, + onImageClick = onImageClick, + messageStatsEnabled = messageStatsEnabled, + showUserMessageTokens = showUserMessageTokens, ) } } } - turn.entries.forEach { entry -> - when (entry) { - is AgentEntry.Prose -> { - val proseMessage = entry.message - val hoistedReasoning = - turnReasoning != null && - proseMessage.id == turnReasoning.message.id - if (proseMessage.hasVisibleAgentContent()) { - item(key = "prose-${proseMessage.id}") { - Column(modifier = Modifier.padding(bottom = 12.dp)) { - if (proseMessage.isStreaming && typingEffectEnabled) { - StreamingFullBleedWithTypingEffect( - streaming = proseMessage, - typingDelayMs = typingEffectDelayMs, - showReasoning = !hoistedReasoning, - ) + + is ChatTurn.Agent -> { + // Reasoning hoist: the turn's reasoning renders at the + // TOP of the turn — above tool rows — so thinking + // leads, then the tool work, then the answer. The + // matching prose entry renders without its own card. + val turnReasoning = + turn.entries + .filterIsInstance() + .firstOrNull { it.message.reasoningText.isNotBlank() } + if (turnReasoning != null) { + val reasoning = turnReasoning.message + item(key = "reasoning-${reasoning.id}") { + Column(modifier = Modifier.padding(bottom = 6.dp)) { + ReasoningCard( + reasoningText = reasoning.reasoningText, + isStreaming = reasoning.isStreaming, + ) + } + } + } + turn.entries.forEach { entry -> + when (entry) { + is AgentEntry.Prose -> { + val proseMessage = entry.message + val hoistedReasoning = + turnReasoning != null && + proseMessage.id == turnReasoning.message.id + if (!proseMessage.hasVisibleAgentContent() && !hoistedReasoning && + proseMessage.reasoningText.isNotBlank() + ) { + item(key = "reasoning-${proseMessage.id}") { + ReasoningCard( + reasoningText = proseMessage.reasoningText, + isStreaming = proseMessage.isStreaming, + ) + } + } + if (proseMessage.hasVisibleAgentContent()) { + item(key = "prose-${proseMessage.id}") { + Column(modifier = Modifier.padding(bottom = 12.dp)) { + if (proseMessage.isStreaming && typingEffectEnabled) { + StreamingFullBleedWithTypingEffect( + streaming = proseMessage, + typingDelayMs = typingEffectDelayMs, + showReasoning = !hoistedReasoning, + ) + } else { + FullBleedAgentMessage( + message = proseMessage, + // Highlight only bubbles that actually contain a match — + // the rest skip the highlight scan entirely. + searchQuery = + if (searchState.isActive && + proseMessage.id in searchState.matchedIds + ) { + searchState.query + } else { + "" + }, + isCurrentMatch = + searchState.currentMatchId != null && + searchState.currentMatchId == proseMessage.id, + showReasoning = !hoistedReasoning, + onOpenAttachment = viewModel::openAttachment, + onSaveAttachment = onSaveAttachment, + savingAttachmentPath = savingAttachmentPath, + openingAttachmentPath = openingAttachmentPath, + canSaveAttachment = savingAttachmentPath == null, + onImageClick = onImageClick, + messageStatsEnabled = messageStatsEnabled, + showAssistantMessageTokens = showAssistantMessageTokens, + showTokensPerSecond = showTokensPerSecond, + ) + } + } + } + } + } + + is AgentEntry.ToolRow -> { + val toolMessage = entry.message + val milestone = toolMilestones[toolMessage.id] + item(key = "tool-${toolMessage.id}") { + Column(modifier = Modifier.padding(bottom = 6.dp)) { + FullBleedToolRow(toolMessage) + milestone?.let { count -> + ToolCallDivider(count = count, maxPerTurn = maxToolCallsPerTurn) + } + } + } + } + + is AgentEntry.SystemEvent -> { + val sysMessage = entry.message + item(key = "sys-${sysMessage.id}") { + Column(modifier = Modifier.padding(bottom = 6.dp)) { + if (sysMessage.displayKind != null) { + // Timeline marker (issue #904): + // model/personality switches and + // auto-continues render as a chip. + TimelineMarkerChip(message = sysMessage) } else { - FullBleedAgentMessage( - message = proseMessage, - // Highlight only bubbles that actually contain a match — - // the rest skip the highlight scan entirely. - searchQuery = - if (searchState.isActive && - proseMessage.id in searchState.matchedIds - ) { - searchState.query - } else { - "" - }, - isCurrentMatch = - searchState.currentMatchId != null && - searchState.currentMatchId == proseMessage.id, - showReasoning = !hoistedReasoning, - onOpenAttachment = viewModel::openAttachment, - onSaveAttachment = onSaveAttachment, - savingAttachmentPath = savingAttachmentPath, - openingAttachmentPath = openingAttachmentPath, - canSaveAttachment = savingAttachmentPath == null, - onImageClick = onImageClick, - messageStatsEnabled = messageStatsEnabled, - showAssistantMessageTokens = showAssistantMessageTokens, - showTokensPerSecond = showTokensPerSecond, + FullBleedSystemEvent( + message = sysMessage, + onRespondApproval = viewModel::respondToApproval, ) } } } } } - - is AgentEntry.ToolRow -> { - val toolMessage = entry.message - val milestone = toolMilestones[toolMessage.id] - item(key = "tool-${toolMessage.id}") { - Column(modifier = Modifier.padding(bottom = 6.dp)) { - FullBleedToolRow(toolMessage) - milestone?.let { count -> - ToolCallDivider(count = count, maxPerTurn = maxToolCallsPerTurn) - } - } - } - } - - is AgentEntry.SystemEvent -> { - val sysMessage = entry.message - item(key = "sys-${sysMessage.id}") { - Column(modifier = Modifier.padding(bottom = 6.dp)) { - if (sysMessage.displayKind != null) { - // Timeline marker (issue #904): - // model/personality switches and - // auto-continues render as a chip. - TimelineMarkerChip(message = sysMessage) - } else { - FullBleedSystemEvent( - message = sysMessage, - onRespondApproval = viewModel::respondToApproval, - ) - } - } - } - } } } } } - } - agentStatus?.let { status -> - item(key = "agent_status") { - AgentStatusIndicator(status = status) + tailItems.forEach { (key, content) -> + item(key = key) { content() } } } - - // Clarify bubble — rendered at the very bottom - if (clarifyRequest != null) { - item(key = "clarify_bubble") { - ClarifyBubble( - clarifyRequest = clarifyRequest, - onRespondSingle = { option -> onRespondClarify?.invoke(option) }, - onRespondBatch = { answers -> onRespondClarifyBatch?.invoke(answers) }, - onDismiss = { onDismissClarify?.invoke() }, - ) - } - } - - if (vaultUnlockPrompt != null) { - item(key = "vault_unlock_card") { - VaultUnlockCard( - prompt = vaultUnlockPrompt, - onConfirm = { password -> onRespondVaultUnlock?.invoke(password) }, - onDismiss = { onDismissVaultUnlock?.invoke() }, - ) - } - } - - if (vaultSaveLoginPrompt != null) { - item(key = "vault_save_login_card") { - VaultSaveLoginCard( - prompt = vaultSaveLoginPrompt, - onConfirm = { identifier, password -> - onRespondVaultSaveLogin?.invoke(identifier, password) - }, - onDismiss = { onDismissVaultSaveLogin?.invoke() }, - ) - } - } - - if (vaultCodePrompt != null) { - item(key = "vault_code_card") { - VaultCodeCard( - prompt = vaultCodePrompt, - onConfirm = { code -> onRespondVaultCode?.invoke(code) }, - onDismiss = { onDismissVaultCode?.invoke() }, - ) - } + // Loading history must never become the list's first visible anchor. + if (isLoadingOlder) { + CircularProgressIndicator( + modifier = Modifier.align(Alignment.TopCenter).padding(12.dp).size(24.dp), + ) } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt index bfa614b8..cc83a539 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt @@ -128,19 +128,22 @@ fun groupIntoTurns(messages: List): List { fun groupIntoTurnsWithStreaming( messages: List, streamingMessage: ChatMessage?, +): List = appendStreamingTurn(groupIntoTurns(messages), messages.mapTo(HashSet()) { it.id }, streamingMessage) + +/** Reuse every settled turn; only the last agent turn needs a changing streaming entry. */ +internal fun appendStreamingTurn( + settled: List, + settledIds: Set, + streamingMessage: ChatMessage?, ): List { - if (streamingMessage == null || messages.any { it.id == streamingMessage.id }) { - return groupIntoTurns(messages) - } - val turns = groupIntoTurns(messages).toMutableList() + if (streamingMessage == null || streamingMessage.id in settledIds) return settled val prose = AgentEntry.Prose(streamingMessage) - val last = turns.lastOrNull() - if (last is ChatTurn.Agent) { - turns[turns.lastIndex] = ChatTurn.Agent(last.entries + prose) + val last = settled.lastOrNull() + return if (last is ChatTurn.Agent) { + settled.dropLast(1) + ChatTurn.Agent(last.entries + prose) } else { - turns += ChatTurn.Agent(listOf(prose)) + settled + ChatTurn.Agent(listOf(prose)) } - return turns } /** @@ -156,8 +159,9 @@ fun groupIntoTurnsWithStreaming( * * Mirrors the item emission order in [FullBleedChatList] exactly: * user turn → 1 item; agent turn → optional reasoning item, then one item per - * visible entry. [leadingItems] accounts for fixed items emitted before the - * turns (e.g. the `loading-older` spinner when paging). + * visible entry, including later reasoning-only rows at their own positions. + * [leadingItems] accounts for fixed items emitted before the + * turns. The history spinner is an overlay and consumes no index. * * @return messageId → LazyColumn item index of the message's content item * (prose items for agent messages; user items for user messages). @@ -165,48 +169,54 @@ fun groupIntoTurnsWithStreaming( fun messageIdToLazyIndex( turns: List, leadingItems: Int = 0, -): Map { - val map = mutableMapOf() - var itemIndex = leadingItems - turns.forEach { turn -> - when (turn) { - is ChatTurn.User -> { - map[turn.message.id] = itemIndex - itemIndex++ +): Map = + fullBleedItemKeys(turns) + .mapIndexedNotNull { index, key -> + if (key.startsWith("user-") || key.startsWith("prose-")) { + key.substringAfter('-') to index + leadingItems + } else { + null } + }.toMap() - is ChatTurn.Agent -> { - val hasReasoning = - turn.entries - .filterIsInstance() - .any { it.message.reasoningText.isNotBlank() } - if (hasReasoning) { - // The reasoning- hoist item occupies one slot. - itemIndex++ +/** Lazy row identities, including hoisted reasoning and grouped tool/system entries. */ +internal fun fullBleedItemKeys(turns: List): List = + buildList { + turns.forEach { turn -> + when (turn) { + is ChatTurn.User -> { + add("user-${turn.message.id}") } - turn.entries.forEach { entry -> - when (entry) { - is AgentEntry.Prose -> { - if (entry.message.hasVisibleAgentContent()) { - map[entry.message.id] = itemIndex - itemIndex++ + + is ChatTurn.Agent -> { + val hoisted = + turn.entries + .filterIsInstance() + .firstOrNull { it.message.reasoningText.isNotBlank() } + hoisted?.let { add("reasoning-${it.message.id}") } + turn.entries.forEach { entry -> + when (entry) { + is AgentEntry.Prose -> { + if (entry.message.hasVisibleAgentContent()) { + add("prose-${entry.message.id}") + } else if (entry != hoisted && entry.message.reasoningText.isNotBlank()) { + add("reasoning-${entry.message.id}") + } } - } - is AgentEntry.ToolRow -> { - itemIndex++ - } + is AgentEntry.ToolRow -> { + add("tool-${entry.message.id}") + } - is AgentEntry.SystemEvent -> { - itemIndex++ + is AgentEntry.SystemEvent -> { + add("sys-${entry.message.id}") + } } } } } } } - return map -} /** * Resolve the message id of the CURRENT search match once, so per-item 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 new file mode 100644 index 00000000..dcf91260 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt @@ -0,0 +1,617 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.model.Attachment +import com.m57.hermescontrol.data.model.SessionMessage +import com.m57.hermescontrol.ui.chat.fakes.FakeChatMessageDao +import com.m57.hermescontrol.ui.chat.fullbleed.fullBleedItemKeys +import com.m57.hermescontrol.ui.chat.fullbleed.groupIntoTurns +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.JsonPrimitive +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class ChatPagingMergeTest { + @Test + fun confirmedLiveOccurrenceCannotConsumeEarlierIdenticalPage() { + for (role in listOf(MessageRole.USER, MessageRole.ASSISTANT, MessageRole.TOOL)) { + val content = if (role == MessageRole.TOOL) "{\"output\":\"ok\"}" else "continue" + val live = ChatMessage(id = "uuid-new", role = role, content = content, timestamp = 2L) + val newest = live.copy(id = "rest-s-20") + val earlier = live.copy(id = "rest-s-10", timestamp = 1L) + val hydrated = mergeTranscriptWithLive(listOf(newest), listOf(live), preserveLiveIds = true) + val paged = + mergeTranscriptWithLive( + listOf(earlier), + hydrated, + chronological = false, + preserveLiveIds = true, + ) + assertEquals("Distinct $role occurrences must survive across pages", 2, paged.size) + assertEquals(listOf("rest-s-10", "uuid-new"), paged.map { it.id }) + val refreshed = mergeTranscriptWithLive(listOf(newest), paged, preserveLiveIds = true) + assertEquals(2, refreshed.size) + assertEquals("uuid-new", refreshed.last().id) + } + } + + @Test + fun mapperAndMergeKeepConfirmedOccurrencesMetadataAndKeysAcrossPages() { + for (role in listOf(MessageRole.USER, MessageRole.ASSISTANT, MessageRole.TOOL)) { + val content = if (role == MessageRole.TOOL) """{"output":"ok"}""" else "continue" + val live = + ChatMessage( + id = "uuid-new", + role = role, + content = content, + timestamp = 100L, + reasoningText = "live reasoning", + toolName = "terminal", + toolStatus = ToolStatus.COMPLETED, + attachments = + listOf( + Attachment(uri = "content://image", name = "image", mimeType = "image/png", size = 1), + ), + finishTimestamp = 200L, + tps = 5.0, + ) + + fun page( + id: Int, + current: List, + ): List = + mapServerMessages( + "s", + listOf(SessionMessage(id = id, role = role.name.lowercase(), content = JsonPrimitive(content))), + 0, + true, + current, + ) + + val hydrated = mergeTranscriptWithLive(page(20, listOf(live)), listOf(live), preserveLiveIds = true) + val confirmed = hydrated.single() + assertEquals("rest-s-20", confirmed.restId) + assertEquals(live.id, confirmed.id) + assertEquals(live.reasoningText, confirmed.reasoningText) + assertEquals(live.attachments, confirmed.attachments) + assertEquals(live.finishTimestamp, confirmed.finishTimestamp) + assertEquals(live.tps, confirmed.tps) + assertEquals(live.toolStatus, confirmed.toolStatus) + val refreshedPage = page(20, hydrated) + assertEquals(confirmed.timestamp, refreshedPage.single().timestamp) + + // Missing server timestamps must not put this older occurrence after the UUID alias. + val paged = + mergeTranscriptWithLive(page(10, hydrated), hydrated, chronological = false, preserveLiveIds = true) + assertEquals(listOf("rest-s-10", "uuid-new"), paged.map { it.id }) + assertEquals("", paged.first().reasoningText) + val refreshed = + mergeTranscriptWithLive(page(10, paged), paged, chronological = false, preserveLiveIds = true) + assertEquals(paged, refreshed) + assertEquals("rest-s-20", refreshed.last().restId) + } + } + + @Test + fun cacheAliasesRejectEarlierOccurrencesInEitherPageOrder() { + for (role in listOf(MessageRole.USER, MessageRole.ASSISTANT, MessageRole.TOOL)) { + val content = if (role == MessageRole.TOOL) """{"output":"ok"}""" else "continue" + val live = ChatMessage(id = "uuid", role = role, content = content, timestamp = 20L) + val echo = live.copy(id = "rest-s-20") + val earlier = live.copy(id = "rest-s-10", timestamp = 10L) + for (firstPage in listOf(listOf(live, echo), listOf(echo, live))) { + val cached = dedupeCachedMessages(firstPage) + assertEquals(listOf(live.copy(restId = echo.id)), cached) + val nextPage = dedupeCachedMessages(listOf(earlier) + cached) + assertEquals(listOf(earlier, live.copy(restId = echo.id)), nextPage) + assertEquals(nextPage, dedupeCachedMessages(listOf(echo) + nextPage)) + } + } + } + + @Test + fun cachedPagesPreserveDisplayedKeyAndEnrichAliasInEitherArrivalOrder() { + val live = tool("uuid", timestamp = 20L).copy(toolName = "terminal") + val echo = tool("rest-s-20", timestamp = 20L) + val earlier = tool("rest-s-10", timestamp = 10L) + for ((first, second) in listOf(live to echo, echo to live)) { + val merged = mergeCachedTranscriptPage(listOf(second), listOf(first)) + assertEquals(listOf(live.copy(id = first.id, restId = echo.id)), merged) + val older = mergeCachedTranscriptPage(listOf(earlier), merged) + assertEquals(listOf(earlier.id, first.id), older.map { it.id }) + assertEquals(older, mergeCachedTranscriptPage(listOf(echo, live), older)) + } + assertEquals( + listOf(earlier, live.copy(restId = echo.id)), + mergeCachedTranscriptPage(listOf(earlier, live), listOf(live.copy(restId = echo.id))), + ) + // A page-local dedup must also transfer its alias to an already displayed UUID. + assertEquals( + listOf(live.copy(restId = echo.id)), + mergeCachedTranscriptPage(listOf(echo, live), listOf(live)), + ) + } + + @Test + fun exactAliasIsReservedBeforeUnconfirmedContentAndSurvivesContentChanges() { + for (role in listOf(MessageRole.USER, MessageRole.ASSISTANT, MessageRole.TOOL)) { + val content = if (role == MessageRole.TOOL) """{"output":"ok"}""" else "continue" + val live = ChatMessage(id = "uuid", role = role, content = content, restId = "rest-s-20") + val unconfirmed = live.copy(id = "uuid-next", restId = null) + val older = live.copy(id = "rest-s-10", restId = null) + val exact = live.copy(id = "rest-s-20", restId = null, content = "updated by server") + val matches = matchTranscriptMessages(listOf(older, exact), listOf(live, unconfirmed)) + assertEquals(listOf(unconfirmed, live), matches) + assertTrue(!sameLogicalMessage(older, live)) + } + } + + @Test + fun olderRowsUseAliasBoundariesEvenWhenLocalTimestampsDisagree() { + val first = tool("uuid-first", timestamp = 100L).copy(restId = "rest-s-4") + val last = tool("uuid-last", timestamp = 1L).copy(restId = "rest-s-10") + val page = listOf(1, 5, 7).map { tool("rest-s-$it", timestamp = 50L) } + + val merged = mergeTranscriptWithLive(page, listOf(first, last), chronological = false, preserveLiveIds = true) + + assertEquals(listOf("rest-s-1", "uuid-first", "rest-s-5", "rest-s-7", "uuid-last"), merged.map { it.id }) + } + + private fun tool( + id: String, + callId: String = "", + timestamp: Long = 1L, + ) = ChatMessage( + id = id, + role = MessageRole.TOOL, + content = """{"output":"ok","exit_code":0}""", + toolCallId = callId, + toolStatus = ToolStatus.COMPLETED, + timestamp = timestamp, + ) + + @Test + fun merge_oneRestEchoCannotConsumeTwoIdenticalLiveResults() { + val first = tool("ws-first", timestamp = 1L) + val second = tool("ws-second", timestamp = 2L) + val echo = tool("rest-session-1", timestamp = 1L) + + val merged = mergeTranscriptWithLive(listOf(echo), listOf(first, second)) + + assertEquals("A result echo may consume at most one live occurrence", 2, merged.size) + assertTrue(merged.any { it.id == second.id }) + } + + @Test + fun cache_oneLiveEchoCannotConsumeTwoPersistedResults() { + val first = tool("rest-session-1", timestamp = 1L) + val second = tool("rest-session-2", timestamp = 2L) + val echo = tool("ws-first", timestamp = 1L) + + val cached = dedupeCachedMessages(listOf(first, echo, second)) + + assertEquals(2, cached.size) + assertTrue(cached.any { it.id == second.id }) + } + + @Test + fun merge_identicalPayloadsWithDifferentCallIdsStayDistinct() { + val first = tool("rest-session-1", callId = "call-first") + val second = tool("ws-second", callId = "call-second") + + val merged = mergeTranscriptWithLive(listOf(first), listOf(second)) + + assertEquals(setOf(first.id, second.id), merged.map { it.id }.toSet()) + } + + @Test + fun merge_largeSettledHistoryRetainsEveryStableId() { + val settled = (1..2_000).map { index -> tool("rest-session-$index", callId = "call-$index") } + val overlappingPage = settled.takeLast(150) + val live = tool("ws-new", callId = "call-new", timestamp = 2L) + + val merged = mergeTranscriptWithLive(overlappingPage, settled + live) + + assertEquals(2_001, merged.size) + assertEquals((settled + live).map { it.id }.toSet(), merged.map { it.id }.toSet()) + } + + @Test + fun olderOverlapUpdatesInPlaceWithoutMovingExistingRows() { + val current = (1..10).map { tool("rest-s-$it", callId = "call-$it", timestamp = 1L) } + val page = current.slice(4..5).map { it.copy(toolName = "terminal") } + + val merged = mergeTranscriptWithLive(page, current, chronological = false) + + assertEquals(current.map { it.id }, merged.map { it.id }) + assertEquals("terminal", merged[4].toolName) + assertEquals("terminal", merged[5].toolName) + } + + @Test + fun olderDisjointAndGapRowsUseServerOrderWhenTimestampsAreEqual() { + val current = listOf(4, 6, 10).map { tool("rest-s-$it", callId = "call-$it", timestamp = 1L) } + val page = listOf(1, 3, 5, 6, 7).map { tool("rest-s-$it", callId = "call-$it", timestamp = 1L) } + + val merged = mergeTranscriptWithLive(page, current, chronological = false) + + assertEquals(listOf(1, 3, 4, 5, 6, 7, 10).map { "rest-s-$it" }, merged.map { it.id }) + } + + private fun serverTool( + id: Int, + callId: String = "", + ) = com.m57.hermescontrol.data.model.SessionMessage( + id = id, + role = "tool", + content = kotlinx.serialization.json.JsonPrimitive("""{"output":"ok","exit_code":0}"""), + tool_call_id = callId, + ) + + @Test + fun mapper_mixedRoomEchoPrefersRichWsToolOverExactRestId() { + val live = + tool("uuid-1", callId = "call-42").copy( + toolName = "terminal", + content = """{"name":"terminal","result":{"output":"ok","exit_code":0}}""", + ) + val echo = tool("rest-s-42", callId = "call-42") + + val mapped = mapServerMessages("s", listOf(serverTool(42, "call-42")), 0, true, listOf(live, echo)) + + assertEquals(listOf(live.copy(restId = "rest-s-42")), mapped) + } + + @Test + fun mapper_mixedEchoesConsumeIdenticalResultsOnlyOncePerOccurrence() { + val first = tool("uuid-1").copy(toolName = "terminal") + val second = tool("uuid-2").copy(toolName = "terminal") + val cache = listOf(first, second, tool("rest-s-42"), tool("rest-s-43")) + + val mapped = mapServerMessages("s", listOf(serverTool(42), serverTool(43)), 0, true, cache) + + assertEquals( + listOf(first.copy(restId = "rest-s-42"), second.copy(restId = "rest-s-43")), + mapped, + ) + assertEquals(2, mapped.map { it.id }.toSet().size) + } + + @Test + fun mapper_differentCallIdsCannotReuseIdenticalLiveResult() { + val live = tool("uuid-1", callId = "call-41").copy(toolName = "terminal") + val echo = tool("rest-s-42", callId = "call-42") + + val mapped = mapServerMessages("s", listOf(serverTool(42, "call-42")), 0, true, listOf(live, echo)) + + assertEquals("rest-s-42", mapped.single().id) + assertEquals("call-42", mapped.single().toolCallId) + assertEquals(null, mapped.single().toolName) + } + + @Test + fun cacheAndRestMergesKeepNumericOrderDespiteNonMonotonicTimestamps() { + val rows = + listOf(9, 10, 100, 200).mapIndexed { index, id -> + ChatMessage( + id = "rest-session-$id", + role = MessageRole.ASSISTANT, + content = "answer-$id", + timestamp = listOf(900L, 100L, 800L, 200L)[index], + ) + } + for (cache in listOf(false, true)) { + for (newestFirst in listOf(false, true)) { + val first = if (newestFirst) rows.drop(2) else rows.take(2) + val second = if (newestFirst) rows.take(2) else rows.drop(2) + val merged = + if (cache) { + mergeCachedTranscriptPage(second, mergeCachedTranscriptPage(first, emptyList())) + } else { + mergeTranscriptWithLive( + second, + mergeTranscriptWithLive(first, emptyList(), preserveLiveIds = true), + chronological = !newestFirst, + preserveLiveIds = true, + ) + } + assertEquals("cache=$cache newestFirst=$newestFirst", rows.map { it.id }, merged.map { it.id }) + } + } + } + + @Test + fun mapperAssignsOneLiveTraceOnlyToNewestEqualContentRestRow() { + for (completion in listOf(null, "completion-200")) { + val live = + ChatMessage( + id = "uuid", + role = MessageRole.ASSISTANT, + content = "Done", + reasoningText = "newest trace", + completionId = completion, + ) + val mapped = + mapServerMessages( + "session", + listOf(assistantRow(100), assistantRow(200)), + 0, + true, + listOf(live), + ) + assertEquals(listOf("", "newest trace"), mapped.map { it.reasoningText }) + assertEquals(listOf(null, completion), mapped.map { it.completionId }) + val merged = mergeTranscriptWithLive(mapped, listOf(live), preserveLiveIds = true) + assertEquals(listOf("rest-session-100", "uuid"), merged.map { it.id }) + assertEquals(listOf("", "newest trace"), merged.map { it.reasoningText }) + assertEquals("rest-session-200", merged.last().canonicalRestId) + } + } + + @Test + fun mapperReservesCanonicalAndCompletionReasoningBeforeContentFallback() { + val first = + ChatMessage( + id = "uuid-first", + role = MessageRole.ASSISTANT, + content = "Done", + reasoningText = "first trace", + completionId = "completion-100", + ) + val second = first.copy(id = "uuid-second", reasoningText = "second trace", completionId = "completion-200") + for (canonicalFirst in listOf(false, true)) { + val live = listOf(if (canonicalFirst) first.copy(restId = "rest-session-100") else first, second) + val mapped = + mapServerMessages( + "session", + listOf(assistantRow(100), assistantRow(200)), + 0, + true, + live, + ) + assertEquals(listOf("first trace", "second trace"), mapped.map { it.reasoningText }) + assertEquals(listOf("completion-100", "completion-200"), mapped.map { it.completionId }) + } + } + + @Test + fun mapperUsesNewestReasoningCandidateWhenNoCompletionIdentityExists() { + val first = + ChatMessage(id = "uuid-first", role = MessageRole.ASSISTANT, content = "Done", reasoningText = "old") + val second = first.copy(id = "uuid-second", reasoningText = "new") + val mapped = mapServerMessages("session", listOf(assistantRow(200)), 0, true, listOf(first, second)) + assertEquals("new", mapped.single().reasoningText) + } + + @Test + fun olderPagesCannotStealLiveReasoningInEitherArrivalOrder() { + val live = + ChatMessage( + id = "uuid", + role = MessageRole.ASSISTANT, + content = "Done", + reasoningText = "newest trace", + completionId = "completion-200", + timestamp = 200_000L, + ) + for (newestFirst in listOf(false, true)) { + val order = if (newestFirst) listOf(200, 100) else listOf(100, 200) + var current = listOf(live) + order.forEach { id -> + current = applyServerPage(current, listOf(assistantRow(id)), older = id == 100) + assertEquals("A trace may appear only once after page $id", 1, current.count { it.reasoningText != "" }) + if (id == + 100 + ) { + assertEquals("", current.single { it.canonicalRestId == "rest-session-100" }.reasoningText) + } + } + assertEquals(listOf("rest-session-100", "uuid"), current.map { it.id }) + assertEquals(listOf("", "newest trace"), current.map { it.reasoningText }) + assertEquals(listOf(null, "completion-200"), current.map { it.completionId }) + assertEquals("rest-session-200", current.last().canonicalRestId) + } + // Older paging also forbids text-only reasoning fallback without a completion id. + val older = + mapServerMessages( + "session", + listOf(assistantRow(100)), + 150, + true, + listOf(live.copy(completionId = null)), + isPagingOlder = true, + ) + assertEquals("", older.single().reasoningText) + } + + @Test + fun reasoningOnlyBoundaryRetainsCanonicalRowBeforeAnswerArrives() { + val previous = assistantRow(90, content = "Previous answer") + val reasoning = assistantRow(100, content = "", reasoning = "boundary trace") + val mapped = mapServerMessages("session", listOf(previous, reasoning), 0, true, emptyList()) + assertEquals(listOf("rest-session-90", "rest-session-100"), mapped.map { it.canonicalRestId }) + assertEquals("", mapped.first().reasoningText) + assertEquals("boundary trace", mapped.last().reasoningText) + assertEquals("", mapped.last().content) + } + + @Test + fun splitReasoningAndAnswerSurviveBothArrivalOrdersRefreshAndPagedCacheReload() = + runTest { + val reasoning = assistantRow(100, content = "", reasoning = "boundary trace") + val answer = assistantRow(200) + for (answerFirst in listOf(false, true)) { + val dao = FakeChatMessageDao() + val repository = ChatPersistenceRepository(dao) + var current = emptyList() + val pages = if (answerFirst) listOf(answer, reasoning) else listOf(reasoning, answer) + pages.forEach { row -> + val mapped = + mapServerMessages( + "session", + listOf(row), + 0, + true, + current, + isPagingOlder = answerFirst && row.id == 100, + ) + current = + mergeTranscriptWithLive( + mapped, + current, + chronological = !(answerFirst && row.id == 100), + preserveLiveIds = true, + ) + // Same input that persistHistoryPage writes, including the boundary page before its neighbor. + repository.persistMessages( + mapped.map { it.copy(id = requireNotNull(it.canonicalRestId)) }, + "session", + ) + } + assertBoundaryTranscript(current) + val refreshed = applyServerPage(current, listOf(reasoning, answer)) + assertEquals(current, refreshed) + val latest = repository.loadPage("session", null, 1) + val older = repository.loadPage("session", latest.cursor, 1) + assertEquals(true, latest.hasOlder) + assertEquals(false, older.hasOlder) + for ((first, second) in listOf(latest.messages to older.messages, older.messages to latest.messages)) { + assertBoundaryTranscript( + mergeCachedTranscriptPage(second, mergeCachedTranscriptPage(first, emptyList())), + ) + } + assertEquals(0, dao.fullSessionReads) + assertEquals(listOf(2, 2), dao.pageLimits) + } + } + + @Test + fun reasoningDoesNotCrossUserOrToolBoundaryInEitherArrivalOrder() { + for (role in listOf("user", "tool")) { + val reasoning = assistantRow(100, content = "", reasoning = "before $role") + val boundary = + SessionMessage( + id = 150, + role = role, + content = JsonPrimitive("boundary"), + timestamp = JsonPrimitive(150), + tool_call_id = if (role == "tool") "call-150" else null, + ) + val answer = assistantRow(200) + for (newestFirst in listOf(false, true)) { + val first = if (newestFirst) listOf(boundary, answer) else listOf(reasoning) + val second = if (newestFirst) listOf(reasoning) else listOf(boundary, answer) + val merged = applyServerPage(applyServerPage(emptyList(), first), second, older = newestFirst) + assertEquals( + listOf(100, 150, 200).map { "rest-session-$it" }, + merged.map { it.canonicalRestId }, + ) + assertEquals(listOf("before $role", "", ""), merged.map { it.reasoningText }) + // Mapping all rows together must have the same ownership as separate pages. + assertEquals(merged, applyServerPage(merged, listOf(reasoning, boundary, answer))) + } + } + } + + @Test + fun multipleReasoningOnlyRowsRemainVisibleAcrossToolSteps() { + val rows = + listOf( + assistantRow(100, content = "", reasoning = "first trace"), + SessionMessage( + id = 150, + role = "tool", + content = JsonPrimitive("result"), + timestamp = JsonPrimitive(150), + ), + assistantRow(200, content = "", reasoning = "second trace"), + assistantRow(300), + ) + val mapped = applyServerPage(emptyList(), rows) + assertEquals(listOf("first trace", "second trace"), mapped.map { it.reasoningText }.filter { it.isNotBlank() }) + assertEquals( + listOf("reasoning-rest-session-100", "reasoning-rest-session-200"), + fullBleedItemKeys(groupIntoTurns(mapped)).filter { it.startsWith("reasoning-") }, + ) + assertEquals(1, fullBleedItemKeys(groupIntoTurns(mapped)).count { it.startsWith("prose-") }) + } + + @Test + fun canonicalReasoningAndMatchingLiveAnswerRenderTraceOnlyOnceInEitherArrivalOrder() { + val live = + ChatMessage( + id = "uuid-answer", + role = MessageRole.ASSISTANT, + content = "Done", + reasoningText = "boundary trace", + completionId = "completion-200", + timestamp = 200_000L, + ) + val reasoning = assistantRow(100, content = "", reasoning = "boundary trace") + val answer = assistantRow(200) + for (answerFirst in listOf(false, true)) { + val pages = if (answerFirst) listOf(answer, reasoning) else listOf(reasoning, answer) + var current = listOf(live) + pages.forEach { row -> + current = applyServerPage(current, listOf(row), older = row.id == 100) + } + assertEquals(listOf("rest-session-100", "uuid-answer"), current.map { it.id }) + assertEquals(listOf("boundary trace", ""), current.map { it.reasoningText }) + assertEquals("completion-200", current.last().completionId) + assertEquals("rest-session-200", current.last().canonicalRestId) + assertEquals( + listOf("reasoning-rest-session-100", "prose-uuid-answer"), + fullBleedItemKeys(groupIntoTurns(current)), + ) + } + } + + @Test + fun differentReasoningOnlyRowsCannotMatchByEmptyContent() { + val live = + ChatMessage( + id = "uuid-thinking", + role = MessageRole.ASSISTANT, + content = "", + reasoningText = "new trace", + ) + val canonical = live.copy(id = "rest-session-100", reasoningText = "old trace") + assertTrue(!sameLogicalMessage(canonical, live)) + assertEquals(2, mergeTranscriptWithLive(listOf(canonical), listOf(live), preserveLiveIds = true).size) + assertTrue(sameLogicalMessage(canonical.copy(reasoningText = "new trace"), live)) + } + + private fun assertBoundaryTranscript(messages: List) { + assertEquals(listOf("rest-session-100", "rest-session-200"), messages.map { it.canonicalRestId }) + assertEquals(listOf("boundary trace", ""), messages.map { it.reasoningText }) + assertEquals(listOf("", "Done"), messages.map { it.content }) + assertEquals( + listOf("reasoning-rest-session-100", "prose-rest-session-200"), + fullBleedItemKeys(groupIntoTurns(messages)), + ) + } + + private fun assistantRow( + id: Int, + content: String = "Done", + reasoning: String = "", + ) = SessionMessage( + id = id, + role = "assistant", + content = JsonPrimitive(content), + reasoning = JsonPrimitive(reasoning), + timestamp = JsonPrimitive(id), + ) + + private fun applyServerPage( + current: List, + rows: List, + older: Boolean = false, + ): List = + mergeTranscriptWithLive( + mapServerMessages("session", rows, 0, true, current, isPagingOlder = older), + current, + chronological = !older, + preserveLiveIds = true, + ) +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPersistenceRepositoryTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPersistenceRepositoryTest.kt index 780e2751..bb26ccd7 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPersistenceRepositoryTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPersistenceRepositoryTest.kt @@ -46,6 +46,36 @@ class ChatPersistenceRepositoryTest { org.junit.Assert.assertSame(failure, runCatching { failing.loadMessages("s") }.exceptionOrNull()) } + @Test + fun pagedReadWaitsForLazyProviderAndRetainsCompletionIdentity() = + runTest { + val ready = kotlinx.coroutines.CompletableDeferred() + var calls = 0 + val lazyRepository = + ChatPersistenceRepository { + calls++ + ready.await() + } + assertEquals(0, calls) + val pending = async { lazyRepository.loadPage("s", null, 150) } + kotlinx.coroutines.yield() + assertEquals(1, calls) + org.junit.Assert.assertFalse(pending.isCompleted) + repository.persistMessage( + ChatMessage(id = "rest-s-1", role = MessageRole.ASSISTANT, content = "Done", completionId = "comp"), + "s", + ) + ready.complete(dao) + val row = pending.await().messages.single() + assertEquals("rest-s-1", row.canonicalRestId) + assertEquals("comp", row.completionId) + assertEquals(0, dao.fullSessionReads) + assertEquals(listOf(151), dao.pageLimits) + val failure = IllegalStateException("database unavailable") + val failing = ChatPersistenceRepository { throw failure } + org.junit.Assert.assertSame(failure, runCatching { failing.loadPage("s", null, 150) }.exceptionOrNull()) + } + @Before fun setup() { dao = FakeChatMessageDao() @@ -146,4 +176,123 @@ class ChatPersistenceRepositoryTest { assertEquals("msg-B1", loadedB[0].id) assertEquals("msg-B2", loadedB[1].id) } + + @Test + fun loadPage_usesSeparateInitialAndCursorQueries() = + runTest { + val tracedDao = io.mockk.spyk(dao) + val pagedRepository = ChatPersistenceRepository(tracedDao) + (1..3).forEach { index -> + pagedRepository.persistMessage( + ChatMessage(id = "row-$index", role = MessageRole.USER, content = "$index", timestamp = 10L), + "session", + ) + } + + val first = pagedRepository.loadPage("session", before = null, limit = 2) + val second = pagedRepository.loadPage("session", before = first.cursor, limit = 2) + + assertEquals(listOf("row-2", "row-3"), first.messages.map { it.id }) + assertEquals(listOf("row-1"), second.messages.map { it.id }) + assertEquals(true, first.hasOlder) + assertEquals(false, second.hasOlder) + io.mockk.coVerify(exactly = 1) { tracedDao.getLatestMessagePage("session", 3) } + io.mockk.coVerify(exactly = 1) { tracedDao.getMessagePage("session", 1, 2L, "row-2", 3) } + io.mockk.coVerify(exactly = 0) { tracedDao.getMessagesForSession(any()) } + } + + @Test + fun loadPage_equalTimestampsAndLiveInsertKeepCursorDeterministicAndSessionsIsolated() = + runTest { + val ids = (1..350).map { "rest-session-$it" } + repository.persistMessages( + ids.map { ChatMessage(id = it, role = MessageRole.USER, content = it, timestamp = 10L) }, + "session", + ) + val first = repository.loadPage("session", before = null, limit = 150) + repository.persistMessage( + ChatMessage(id = "new", role = MessageRole.USER, content = "new", timestamp = 20L), + "session", + ) + repository.persistMessage( + ChatMessage(id = "other", role = MessageRole.USER, content = "other", timestamp = 10L), + "other-session", + ) + val second = repository.loadPage("session", before = first.cursor, limit = 150) + val third = repository.loadPage("session", before = second.cursor, limit = 150) + + assertEquals(ids.takeLast(150), first.messages.map { it.id }) + assertEquals(ids, (third.messages + second.messages + first.messages).map { it.id }) + assertEquals(false, third.hasOlder) + assertEquals(listOf(151, 151, 151), dao.pageLimits) + assertEquals(0, dao.fullSessionReads) + assertEquals(ids.toSet() + "new", dao.idsForSession("session")) + assertEquals(setOf("other"), dao.idsForSession("other-session")) + } + + @Test + fun loadPage_nonMonotonicTimestampsFollowServerIdsInEitherArrivalOrder() = + runTest { + val rows = + listOf(9, 10, 100, 200).mapIndexed { index, id -> + ChatMessage( + id = "rest-session-$id", + role = MessageRole.ASSISTANT, + content = "answer-$id", + timestamp = listOf(900L, 100L, 800L, 200L)[index], + ) + } + for (pages in listOf(listOf(rows.take(2), rows.drop(2)), listOf(rows.drop(2), rows.take(2)))) { + repository.clearMessagesForSession("session") + pages.forEach { repository.persistMessages(it, "session") } + val latest = repository.loadPage("session", null, 2) + val older = repository.loadPage("session", latest.cursor, 2) + assertEquals(rows.drop(2).map { it.id }, latest.messages.map { it.id }) + assertEquals(rows.take(2).map { it.id }, older.messages.map { it.id }) + assertEquals(true, latest.hasOlder) + assertEquals(false, older.hasOlder) + } + assertEquals(0, dao.fullSessionReads) + assertEquals(listOf(3, 3, 3, 3), dao.pageLimits) + } + + @Test + fun loadPage_uuidOnlyRowsKeepInsertionOrderAcrossUpdatesAndLiveAppend() = + runTest { + val rows = + listOf("ffffffff", "00000000", "aaaaaaaa").mapIndexed { index, prefix -> + ChatMessage( + id = "$prefix-0000-4000-8000-000000000000", + role = MessageRole.USER, + content = "message-$index", + timestamp = listOf(900L, 100L, 800L)[index], + ) + } + rows.forEach { repository.persistMessage(it, "local") } + repository.persistMessage(rows.first().copy(content = "edited", timestamp = 999L), "local") + val latest = repository.loadPage("local", null, 2) + repository.persistMessage( + ChatMessage( + id = "bbbbbbbb-0000-4000-8000-000000000000", + role = MessageRole.USER, + content = "appended after cursor", + timestamp = 1L, + ), + "local", + ) + val older = repository.loadPage("local", latest.cursor, 2) + assertEquals(rows.drop(1).map { it.id }, latest.messages.map { it.id }) + assertEquals(listOf(rows.first().id), older.messages.map { it.id }) + assertEquals("edited", older.messages.single().content) + assertEquals(false, older.hasOlder) + assertEquals( + "appended after cursor", + repository + .loadPage("local", null, 1) + .messages + .single() + .content, + ) + assertEquals(0, dao.fullSessionReads) + } } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatToolDedupeTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatToolDedupeTest.kt index eb0c1403..a3a8f5d9 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatToolDedupeTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatToolDedupeTest.kt @@ -4,7 +4,6 @@ import com.m57.hermescontrol.data.ws.WsEvent import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull -import org.junit.Assert.assertSame import org.junit.Assert.assertTrue import org.junit.Test @@ -193,7 +192,7 @@ class ChatToolDedupeTest { assertEquals(3, deduped.size) // WS copies win; rest- copies of the same logical message are dropped - assertTrue(deduped.contains(wsTool)) + assertTrue(deduped.contains(wsTool.copy(restId = restTool.id))) assertTrue(deduped.none { it.id == "rest-s-1" }) assertTrue(deduped.none { it.id == "rest-s-2" }) // rest- user kept when no WS copy exists @@ -279,7 +278,7 @@ class ChatToolDedupeTest { val merged = mergeTranscriptWithLive(listOf(rest), listOf(live)) // Preserves cache/live-only user flags such as continuesActiveTurn. - assertSame(live, merged.single()) + assertEquals(live.copy(restId = rest.id), merged.single()) } @Test @@ -551,7 +550,7 @@ class ChatToolDedupeTest { val merged = mergeIncrementalTranscriptPage(incoming, current, "s", 1) assertEquals(listOf("retry", "retry"), merged.map { it.content }) - assertSame(live, merged.last()) + assertEquals(live.copy(restId = "rest-s-1"), merged.last()) } // ── Issue #842: MCP/web tool rows (raw `` text) ── @@ -654,6 +653,6 @@ The following content was retrieved from an external source. ) val deduped = dedupeCachedMessages(listOf(wsTool, restTool)) assertEquals(1, deduped.size) - assertEquals(wsTool, deduped.single()) + assertEquals(wsTool.copy(restId = restTool.id), deduped.single()) } } 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 84918f52..0180dfce 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 @@ -335,10 +335,21 @@ class ChatViewModelTest { // ── Helpers ────────────────────────────────────────────────────────────── /** Create a ViewModel with the fake repo injected directly. */ - private fun createViewModel(startCleanup: Boolean = false): ChatViewModel = + private fun createViewModel( + startCleanup: Boolean = false, + historyDispatcher: kotlinx.coroutines.CoroutineDispatcher = testDispatcher, + ): ChatViewModel = // Both dispatchers injected — a real ioDispatcher would race the // test scheduler (repo writes hop it) and shuffle RPC ordering. - ChatViewModel(app, startCleanup, fakeRepo, fakeSlashUsageStore, testDispatcher, testDispatcher) + ChatViewModel( + app, + startCleanup, + fakeRepo, + fakeSlashUsageStore, + testDispatcher, + testDispatcher, + historyDispatcher, + ) /** * Create ViewModel, simulate GatewayReady, feed SESSION_CREATE result, @@ -349,8 +360,9 @@ class ChatViewModelTest { */ private suspend fun TestScope.createViewModelWithSession( startCleanup: Boolean = false, + historyDispatcher: kotlinx.coroutines.CoroutineDispatcher = testDispatcher, ): Pair { - val viewModel = createViewModel(startCleanup) + val viewModel = createViewModel(startCleanup, historyDispatcher) advanceUntilIdle() mockConnectionStatus.value = ConnectionStatus.CONNECTED @@ -5198,6 +5210,824 @@ class ChatViewModelTest { ) } + // Chat paging: deterministic regressions, with no real network or wall-clock timing. + private fun seedPagingCache( + count: Int, + sameTimestamp: Boolean = false, + ): List = + (1..count).map { index -> + val id = "cached-${index.toString().padStart(5, '0')}" + fakeRepo.dao.addMessageDirect( + com.m57.hermescontrol.data.local.ChatMessageEntity( + id = id, + sessionId = "session-456", + role = "user", + content = "Cached message $index", + timestamp = if (sameTimestamp) 1L else index.toLong(), + ), + ) + id + } + + private fun pagingResponse( + rows: IntRange, + offset: Int = 0, + ): retrofit2.Response = + retrofit2.Response.success( + com.m57.hermescontrol.data.model.SessionMessagesResponse( + messages = + rows.map { index -> + com.m57.hermescontrol.data.model.SessionMessage( + id = index, + role = "assistant", + content = JsonPrimitive("Server message $index"), + timestamp = JsonPrimitive(index), + ) + }, + pagination = + com.m57.hermescontrol.data.model.PaginationInfo( + limit = 150, + offset = offset, + order = "latest", + returned = rows.count(), + ), + ), + ) + + @Test + fun paging_syncNewestRestEcho_preservesLiveIdAndCompletionInOneBubble() = + runTest { + val api = ApiClient.hermesApi + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(IntRange.EMPTY) + viewModel.switchSession("session-456") + advanceUntilIdle() + + // A completed live reply has no confirmed REST identity until its echo arrives. + val live = + ChatMessage( + id = "live-reply", + role = MessageRole.ASSISTANT, + content = "Server message 42", + timestamp = 42_000L, + completionId = "comp", + restId = null, + ) + + @Suppress("UNCHECKED_CAST") + val state = + ChatViewModel::class.java + .getDeclaredField("_uiState") + .apply { isAccessible = true } + .get(viewModel) as MutableStateFlow + state.value = viewModel.uiState.value.copy(messages = listOf(live)) + advanceUntilIdle() + assertNull( + viewModel.uiState.value.messages + .single() + .restId, + ) + assertEquals( + "comp", + viewModel.uiState.value.messages + .single() + .completionId, + ) + + coEvery { + api.getSessionMessages("session-456", 150, 0, "latest", any()) + } returns pagingResponse(42..42) + + viewModel.syncCurrentSession() + advanceUntilIdle() + + coVerify(exactly = 2) { + api.getSessionMessages("session-456", 150, 0, "latest", any()) + } + val messages = viewModel.uiState.value.messages + assertEquals("The newest REST echo must confirm the live reply without adding a bubble", 1, messages.size) + val message = messages.single() + assertEquals("Server message 42", message.content) + assertEquals("live-reply", message.id) + assertEquals("rest-session-456-42", message.restId) + assertEquals("comp", message.completionId) + } + + @Test + fun paging_largeCache_initialReadIsBoundedAtDao() = + runTest { + val expectedIds = seedPagingCache(10_000) + stubSession456Rests(success = false) + val (viewModel, _) = createViewModelWithSession() + + viewModel.switchSession("session-456") + advanceUntilIdle() + + assertEquals( + expectedIds.takeLast(150), + viewModel.uiState.value.messages + .map { it.id }, + ) + assertEquals("Loading all rows then taking a suffix is not paging", 0, fakeRepo.dao.fullSessionReads) + assertEquals(expectedIds.toSet(), fakeRepo.dao.idsForSession("session-456")) + assertTrue(viewModel.uiState.value.hasOlderMessages) + assertFalse(viewModel.uiState.value.isLoading) + } + + @Test + fun paging_offlinePagesRetainEveryRowWithEqualTimestamps() = + runTest { + val expectedIds = seedPagingCache(350, sameTimestamp = true) + stubSession456Rests(success = false) + val (viewModel, _) = createViewModelWithSession() + viewModel.switchSession("session-456") + advanceUntilIdle() + assertEquals( + expectedIds.takeLast(150), + viewModel.uiState.value.messages + .map { it.id }, + ) + + viewModel.loadOlderMessages() + advanceUntilIdle() + assertEquals( + expectedIds.takeLast(300), + viewModel.uiState.value.messages + .map { it.id }, + ) + assertTrue(viewModel.uiState.value.hasOlderMessages) + assertFalse(viewModel.uiState.value.isLoadingOlder) + + viewModel.loadOlderMessages() + advanceUntilIdle() + assertEquals( + expectedIds, + viewModel.uiState.value.messages + .map { it.id }, + ) + assertFalse(viewModel.uiState.value.hasOlderMessages) + assertFalse(viewModel.uiState.value.isLoadingOlder) + assertEquals(expectedIds.toSet(), fakeRepo.dao.idsForSession("session-456")) + assertEquals(0, fakeRepo.dao.fullSessionReads) + } + + @Test + fun paging_cacheEchoEnrichesExistingUuidBeforeEarlierRepeatedOccurrencesArrive() = + runTest { + val roles = listOf(MessageRole.USER, MessageRole.ASSISTANT, MessageRole.TOOL) + for (index in 1..303) { + val occurrence = + when (index) { + in 1..3 -> index - 1 + in 151..153 -> index - 151 + in 301..303 -> index - 301 + else -> null + } + val role = occurrence?.let { roles[it] } ?: MessageRole.SYSTEM + val id = + when (index) { + in 1..3 -> "rest-session-456-${10 + requireNotNull(occurrence)}" + in 151..153 -> "rest-session-456-${20 + requireNotNull(occurrence)}" + in 301..303 -> "uuid-${requireNotNull(occurrence)}" + else -> "filler-$index" + } + fakeRepo.dao.addMessageDirect( + com.m57.hermescontrol.data.local.ChatMessageEntity( + id = id, + sessionId = "session-456", + role = role.name, + content = + when (role) { + MessageRole.TOOL -> """{"output":"ok"}""" + MessageRole.SYSTEM -> "filler $index" + else -> "continue" + }, + timestamp = index.toLong(), + toolName = if (role == MessageRole.TOOL && index > 300) "terminal" else null, + ), + ) + } + val storedIds = fakeRepo.dao.idsForSession("session-456") + stubSession456Rests(success = false) + val (viewModel, _) = createViewModelWithSession() + viewModel.switchSession("session-456") + advanceUntilIdle() + assertEquals(150, viewModel.uiState.value.messages.size) + + viewModel.loadOlderMessages() + advanceUntilIdle() + for (index in roles.indices) { + val retained = + viewModel.uiState.value.messages + .single { it.id == "uuid-$index" } + assertEquals("rest-session-456-${20 + index}", retained.restId) + } + assertEquals(297, viewModel.uiState.value.messages.size) + + viewModel.loadOlderMessages() + advanceUntilIdle() + val messages = viewModel.uiState.value.messages + assertEquals(300, messages.size) + for (index in roles.indices) { + assertEquals(2, messages.count { it.role == roles[index] }) + assertTrue(messages.any { it.id == "rest-session-456-${10 + index}" }) + assertTrue(messages.any { it.id == "uuid-$index" }) + } + assertEquals("terminal", messages.single { it.id == "uuid-2" }.toolName) + assertEquals(storedIds, fakeRepo.dao.idsForSession("session-456")) + assertFalse(viewModel.uiState.value.hasOlderMessages) + } + + @Test + fun paging_reusedToolPersistsCanonicalKeyWithoutReplacingLiveKey() = + runTest { + fakeRepo.dao.addMessageDirect( + com.m57.hermescontrol.data.local.ChatMessageEntity( + id = "uuid-tool", + sessionId = "session-456", + role = "TOOL", + content = """{"output":"ok"}""", + timestamp = 1L, + toolName = "terminal", + ), + ) + val api = ApiClient.hermesApi + val response = pagingResponse(42..42).body()!! + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns + retrofit2.Response.success( + response.copy( + messages = + response.messages.map { + it.copy( + role = "tool", + content = JsonPrimitive("""{"output":"ok"}"""), + ) + }, + ), + ) + val (viewModel, _) = createViewModelWithSession() + viewModel.switchSession("session-456") + advanceUntilIdle() + + val message = + viewModel.uiState.value.messages + .single() + assertEquals("uuid-tool", message.id) + assertEquals("rest-session-456-42", message.restId) + assertEquals("terminal", message.toolName) + assertEquals(setOf("uuid-tool", "rest-session-456-42"), fakeRepo.dao.idsForSession("session-456")) + } + + @Test + fun paging_emptyServerPageDoesNotHideOlderOfflineHistory() = + runTest { + val api = ApiClient.hermesApi + val expectedIds = seedPagingCache(350) + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(IntRange.EMPTY) + + viewModel.switchSession("session-456") + advanceUntilIdle() + + assertEquals(150, viewModel.uiState.value.messages.size) + assertTrue(viewModel.uiState.value.hasOlderMessages) + viewModel.loadOlderMessages() + advanceUntilIdle() + assertEquals(300, viewModel.uiState.value.messages.size) + assertEquals(expectedIds.toSet(), fakeRepo.dao.idsForSession("session-456")) + } + + @Test + fun paging_cacheFirst_serverHydrationDoesNotInflateInitialWindow() = + runTest { + val api = ApiClient.hermesApi + seedPagingCache(10_000) + val response = + CompletableDeferred>() + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } coAnswers { response.await() } + + viewModel.switchSession("session-456") + runCurrent() + val cacheSize = viewModel.uiState.value.messages.size + response.complete(pagingResponse(1..150)) + advanceUntilIdle() + + assertEquals(150, cacheSize) + assertEquals(300, viewModel.uiState.value.messages.size) + assertFalse(viewModel.uiState.value.isLoading) + assertTrue(viewModel.uiState.value.hasOlderMessages) + assertEquals(0, fakeRepo.dao.fullSessionReads) + } + + @Test + fun paging_serverFirst_cacheCursorStillAllowsOfflineHistory() = + runTest { + val api = ApiClient.hermesApi + seedPagingCache(350) + val cacheRead = CompletableDeferred() + fakeRepo.dao.beforeRead = { cacheRead.await() } + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(IntRange.EMPTY) + + viewModel.switchSession("session-456") + runCurrent() + cacheRead.complete(Unit) + advanceUntilIdle() + + assertEquals(150, viewModel.uiState.value.messages.size) + assertTrue(viewModel.uiState.value.hasOlderMessages) + viewModel.loadOlderMessages() + advanceUntilIdle() + assertEquals(300, viewModel.uiState.value.messages.size) + assertFalse(viewModel.uiState.value.isLoadingOlder) + } + + @Test + fun paging_pendingOlderPageIsSuppressedAndFailureAllowsRetry() = + runTest { + val api = ApiClient.hermesApi + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(151..300) + viewModel.switchSession("session-456") + advanceUntilIdle() + val pending = + CompletableDeferred>() + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } coAnswers { pending.await() } + + viewModel.loadOlderMessages() + runCurrent() + repeat(5) { viewModel.loadOlderMessages() } + runCurrent() + coVerify(exactly = 1) { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } + assertTrue(viewModel.uiState.value.isLoadingOlder) + pending.complete(retrofit2.Response.error(500, okhttp3.ResponseBody.create(null, "test failure"))) + advanceUntilIdle() + assertFalse(viewModel.uiState.value.isLoadingOlder) + assertTrue(viewModel.uiState.value.hasOlderMessages) + + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } returns pagingResponse(1..150, offset = 150) + viewModel.loadOlderMessages() + advanceUntilIdle() + assertEquals(300, viewModel.uiState.value.messages.size) + assertFalse(viewModel.uiState.value.isLoadingOlder) + } + + @Test + fun paging_oldSessionResponseCannotReplaceNewSession() = + runTest { + val api = ApiClient.hermesApi + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(151..300) + coEvery { + api.getSessionMessages("session-other", any(), any(), any(), any()) + } returns pagingResponse(900..900) + viewModel.switchSession("session-456") + advanceUntilIdle() + val pending = + CompletableDeferred>() + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } coAnswers { pending.await() } + + viewModel.loadOlderMessages() + runCurrent() + viewModel.switchSession("session-other") + runCurrent() + pending.complete(pagingResponse(1..150, offset = 150)) + advanceUntilIdle() + + assertEquals("session-other", viewModel.uiState.value.currentSessionId) + assertEquals( + listOf("rest-session-other-900"), + viewModel.uiState.value.messages + .map { it.id }, + ) + assertFalse(viewModel.uiState.value.isLoadingOlder) + } + + /** Holds CPU continuations without sleeping or starting a real thread. */ + private class QueuedHistoryDispatcher : kotlinx.coroutines.CoroutineDispatcher() { + private val queue = ArrayDeque() + val pending: Boolean get() = queue.isNotEmpty() + + override fun dispatch( + context: kotlin.coroutines.CoroutineContext, + block: Runnable, + ) { + queue.addLast(block) + } + + fun runNext() = queue.removeFirst().run() + } + + private fun TestScope.drainHistory(dispatcher: QueuedHistoryDispatcher) { + repeat(100) { + runCurrent() + if (!dispatcher.pending) return + dispatcher.runNext() + } + error("History did not settle after 100 CPU continuations") + } + + @Test + fun paging_liveMessageArrivingBeforeComputedSnapshotCommitSurvives() = + runTest { + val api = ApiClient.hermesApi + val cpu = QueuedHistoryDispatcher() + val (viewModel, _) = createViewModelWithSession(historyDispatcher = cpu) + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(1..150) + viewModel.switchSession("session-456") + runCurrent() + // Cache query runs first; hydration has already captured an empty snapshot. + cpu.runNext() + runCurrent() + mockEventsFlow.emit(WsEvent.MessageComplete("Live during merge", "session-456")) + cpu.runNext() + // The queued WS event runs before the computed result can commit on Main. + runCurrent() + drainHistory(cpu) + + val messages = viewModel.uiState.value.messages + assertEquals(151, messages.size) + assertEquals(1, messages.count { it.content == "Live during merge" }) + assertFalse(viewModel.uiState.value.isLoading) + } + + @Test + fun paging_sessionSwitchDiscardsQueuedCpuResult() = + runTest { + val api = ApiClient.hermesApi + val cpu = QueuedHistoryDispatcher() + val (viewModel, _) = createViewModelWithSession(historyDispatcher = cpu) + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(1..150) + coEvery { + api.getSessionMessages("session-other", any(), any(), any(), any()) + } returns pagingResponse(900..900) + viewModel.switchSession("session-456") + runCurrent() + cpu.runNext() + runCurrent() + cpu.runNext() + viewModel.switchSession("session-other") + drainHistory(cpu) + + assertEquals("session-other", viewModel.uiState.value.currentSessionId) + assertEquals( + listOf("rest-session-other-900"), + viewModel.uiState.value.messages + .map { it.id }, + ) + assertFalse(viewModel.uiState.value.isLoading) + } + + @Test + fun paging_reconnectInvalidatesPendingOlderRequestAndItsCursor() = + runTest { + val api = ApiClient.hermesApi + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(301..450) + viewModel.switchSession("session-456") + advanceUntilIdle() + val oldResumeId = sentRequestMethods.last { it.first == WsMethods.SESSION_RESUME }.second + mockEventsFlow.emit(WsEvent.RpcResult(oldResumeId, mapOf("session_id" to "runtime-456"))) + advanceUntilIdle() + assertTrue(viewModel.uiState.value.isSessionReady) + val pending = + CompletableDeferred>() + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } coAnswers { pending.await() } + viewModel.loadOlderMessages() + runCurrent() + + coEvery { + api.getSessionMessages("session-456", 150, 0, "latest", any()) + } returns pagingResponse(501..650) + mockConnectionStatus.value = ConnectionStatus.RECONNECTING + runCurrent() + assertFalse(viewModel.uiState.value.isSessionReady) + assertFalse(viewModel.uiState.value.isLoadingOlder) + assertFalse(viewModel.sendMessage("preserved draft")) + mockConnectionStatus.value = ConnectionStatus.CONNECTED + mockEventsFlow.emit(WsEvent.GatewayReady(null)) + runCurrent() + pending.complete(pagingResponse(1..150, offset = 150)) + advanceUntilIdle() + assertFalse(viewModel.uiState.value.isLoadingOlder) + assertFalse( + viewModel.uiState.value.messages + .any { it.id == "rest-session-456-1" }, + ) + + mockEventsFlow.emit(WsEvent.RpcResult(oldResumeId, mapOf("session_id" to "stale-runtime"))) + runCurrent() + assertFalse(viewModel.uiState.value.isSessionReady) + val freshResumeId = sentRequestMethods.last { it.first == WsMethods.SESSION_RESUME }.second + mockEventsFlow.emit(WsEvent.RpcResult(freshResumeId, mapOf("session_id" to "fresh-runtime"))) + advanceUntilIdle() + assertTrue(viewModel.uiState.value.isSessionReady) + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } returns pagingResponse(351..500, offset = 150) + viewModel.loadOlderMessages() + advanceUntilIdle() + assertTrue( + viewModel.uiState.value.messages + .any { it.id == "rest-session-456-500" }, + ) + coVerify(exactly = 0) { + api.getSessionMessages("session-456", 150, 300, "latest", any()) + } + } + + @Test + fun paging_offlineCacheRemainsReadableAndFreshResumeRestoresSendReadiness() = + runTest { + val ids = seedPagingCache(350) + stubSession456Rests(success = false) + val (viewModel, _) = createViewModelWithSession() + viewModel.switchSession("session-456") + advanceUntilIdle() + assertFalse(viewModel.uiState.value.isSessionReady) + assertFalse(viewModel.sendMessage("draft")) + mockConnectionStatus.value = ConnectionStatus.NO_NETWORK + runCurrent() + viewModel.loadOlderMessages() + advanceUntilIdle() + assertEquals( + ids.takeLast(300), + viewModel.uiState.value.messages + .map { it.id }, + ) + assertFalse(viewModel.uiState.value.isLoadingOlder) + val api = ApiClient.hermesApi + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(IntRange.EMPTY) + mockConnectionStatus.value = ConnectionStatus.CONNECTED + mockEventsFlow.emit(WsEvent.GatewayReady(null)) + advanceUntilIdle() + assertFalse(viewModel.uiState.value.isSessionReady) + val resumeId = sentRequestMethods.last { it.first == WsMethods.SESSION_RESUME }.second + mockEventsFlow.emit(WsEvent.RpcResult(resumeId, mapOf("session_id" to "runtime-456"))) + advanceUntilIdle() + assertTrue(viewModel.uiState.value.isSessionReady) + assertTrue(viewModel.sendMessage("draft")) + advanceUntilIdle() + assertEquals( + 1, + viewModel.uiState.value.messages + .count { it.content == "draft" }, + ) + } + + @Test + fun paging_cacheAndServerCursorsAdvanceIndependently() = + runTest { + val api = ApiClient.hermesApi + val cachedIds = seedPagingCache(350) + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(151..300) + viewModel.switchSession("session-456") + advanceUntilIdle() + repeat(2) { + viewModel.loadOlderMessages() + advanceUntilIdle() + } + coVerify(exactly = 1) { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } + assertEquals( + cachedIds.toSet() + (151..300).map { "rest-session-456-$it" }, + viewModel.uiState.value.messages + .map { it.id } + .toSet(), + ) + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } returns pagingResponse(1..150, offset = 150) + viewModel.loadOlderMessages() + advanceUntilIdle() + + // The remaining cache page contains only the latest REST rows already on screen. + // This same action must reach the independent server cursor, not stop on those echoes. + coVerify(exactly = 1) { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } + val expectedIds = cachedIds.toSet() + (1..300).map { "rest-session-456-$it" } + assertEquals(650, viewModel.uiState.value.messages.size) + assertEquals( + expectedIds, + viewModel.uiState.value.messages + .map { it.id } + .toSet(), + ) + assertEquals(expectedIds, fakeRepo.dao.idsForSession("session-456")) + assertFalse(viewModel.uiState.value.isLoadingOlder) + assertEquals(0, fakeRepo.dao.fullSessionReads) + assertTrue(fakeRepo.dao.pageLimits.all { it == 151 }) + } + + @Test + fun paging_latestGrowthBetweenPagesAndSyncNeverSkipsOlderRows() = + runTest { + val api = ApiClient.hermesApi + val (viewModel, _) = createViewModelWithSession() + var total = 450 + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } coAnswers { + val offset = arg(2) + val end = (total - offset).coerceAtLeast(0) + val start = (end - 150).coerceAtLeast(0) + 1 + pagingResponse(start..end, offset) + } + viewModel.switchSession("session-456") + advanceUntilIdle() + viewModel.loadOlderMessages() + advanceUntilIdle() + total = 460 + viewModel.syncCurrentSession() + advanceUntilIdle() + repeat(2) { + viewModel.loadOlderMessages() + advanceUntilIdle() + } + + assertEquals( + (1..460).map { "rest-session-456-$it" }.toSet(), + viewModel.uiState.value.messages + .map { it.id } + .toSet(), + ) + assertFalse(viewModel.uiState.value.hasOlderMessages) + } + + @Test + fun paging_rawHiddenRowsStillAdvanceServerCursor() = + runTest { + val api = ApiClient.hermesApi + val (viewModel, _) = createViewModelWithSession() + val response = pagingResponse(1..150).body()!! + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns + retrofit2.Response.success( + response.copy(messages = response.messages.map { it.copy(content = JsonPrimitive("")) }), + ) + viewModel.switchSession("session-456") + advanceUntilIdle() + assertTrue(viewModel.uiState.value.hasOlderMessages) + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } returns pagingResponse(IntRange.EMPTY, offset = 150) + viewModel.loadOlderMessages() + advanceUntilIdle() + assertFalse(viewModel.uiState.value.hasOlderMessages) + coVerify(exactly = 1) { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } + } + + @Test + fun paging_cacheReadExceptionSettlesAndAllowsRetry() = + runTest { + val ids = seedPagingCache(350) + stubSession456Rests(success = false) + fakeRepo.dao.beforeRead = { error("Synthetic Room read failure") } + val (viewModel, _) = createViewModelWithSession() + viewModel.switchSession("session-456") + advanceUntilIdle() + assertFalse(viewModel.uiState.value.isLoading) + assertTrue(viewModel.uiState.value.hasOlderMessages) + fakeRepo.dao.beforeRead = {} + viewModel.loadOlderMessages() + advanceUntilIdle() + + assertEquals( + ids.takeLast(150), + viewModel.uiState.value.messages + .map { it.id }, + ) + assertFalse(viewModel.uiState.value.isLoadingOlder) + } + + @Test + fun paging_mapperExceptionSettlesAndRetriesSameServerOffset() = + runTest { + val api = ApiClient.hermesApi + val (viewModel, _) = createViewModelWithSession() + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(151..300) + viewModel.switchSession("session-456") + advanceUntilIdle() + val invalid = pagingResponse(1..150, 150).body()!! + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } returns retrofit2.Response.success(invalid.copy(messages = invalid.messages.map { it.copy(id = null) })) + viewModel.loadOlderMessages() + advanceUntilIdle() + assertFalse(viewModel.uiState.value.isLoadingOlder) + assertTrue(viewModel.uiState.value.hasOlderMessages) + coEvery { + api.getSessionMessages("session-456", 150, 150, "latest", any()) + } returns pagingResponse(1..150, 150) + viewModel.loadOlderMessages() + advanceUntilIdle() + assertEquals(300, viewModel.uiState.value.messages.size) + } + + private suspend fun TestScope.assertOverlappingHistoryPageKeepsOrder(equalTimestamps: Boolean) { + val api = ApiClient.hermesApi + val (viewModel, _) = createViewModelWithSession() + (1..350).forEach { index -> + fakeRepo.dao.addMessageDirect( + com.m57.hermescontrol.data.local.ChatMessageEntity( + id = "rest-session-456-$index", + sessionId = "session-456", + role = "ASSISTANT", + content = "Server message $index", + timestamp = if (equalTimestamps) 1_000L else index * 1_000L, + ), + ) + } + coEvery { api.getSessionMessages("session-456", any(), any(), any(), any()) } coAnswers { + val offset = arg(2) + val end = 350 - offset + val response = pagingResponse((end - 149).coerceAtLeast(1)..end, offset).body()!! + retrofit2.Response.success( + if (equalTimestamps) { + response.copy(messages = response.messages.map { it.copy(timestamp = JsonPrimitive(1)) }) + } else { + response + }, + ) + } + viewModel.switchSession("session-456") + advanceUntilIdle() + repeat(2) { + viewModel.loadOlderMessages() + advanceUntilIdle() + } + val before = + viewModel.uiState.value.messages + .map { it.id } + assertEquals((1..350).map { "rest-session-456-$it" }.toSet(), before.toSet()) + assertEquals(350, before.size) + coVerify(exactly = 1) { api.getSessionMessages("session-456", any(), any(), any(), any()) } + // Local history is exhausted. Both subsequent server pages overlap it completely. + repeat(2) { + viewModel.loadOlderMessages() + advanceUntilIdle() + assertEquals( + before, + viewModel.uiState.value.messages + .map { it.id }, + ) + } + assertFalse(viewModel.uiState.value.hasOlderMessages) + coVerify(exactly = 1) { api.getSessionMessages("session-456", 150, 150, "latest", any()) } + coVerify(exactly = 1) { api.getSessionMessages("session-456", 150, 300, "latest", any()) } + } + + @Test + fun paging_cacheExhaustedOverlappingServerPageKeepsExistingOrder() = + runTest { assertOverlappingHistoryPageKeepsOrder(equalTimestamps = false) } + + @Test + fun paging_cacheExhaustedOverlapWithEqualTimestampsKeepsExistingOrder() = + runTest { assertOverlappingHistoryPageKeepsOrder(equalTimestamps = true) } + // ── Attachment open (issue #724) ───────────────────────────────────── @Test diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedItemKeysTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedItemKeysTest.kt new file mode 100644 index 00000000..0df28900 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedItemKeysTest.kt @@ -0,0 +1,39 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.ui.chat.fullbleed.fullBleedItemKeys +import com.m57.hermescontrol.ui.chat.fullbleed.groupIntoTurns +import com.m57.hermescontrol.ui.chat.fullbleed.messageIdToLazyIndex +import org.junit.Assert.assertEquals +import org.junit.Test + +class FullBleedItemKeysTest { + @Test + fun keysIncludeReasoningToolsAndSystemButSkipEmptyProse() { + val turns = + groupIntoTurns( + listOf( + ChatMessage("u", MessageRole.USER, "question"), + ChatMessage("empty", MessageRole.ASSISTANT, ""), + ChatMessage("t", MessageRole.TOOL, "output"), + ChatMessage("a", MessageRole.ASSISTANT, "answer", reasoningText = "thinking"), + ChatMessage("s", MessageRole.SYSTEM, "event"), + ), + ) + assertEquals(listOf("user-u", "reasoning-a", "tool-t", "prose-a", "sys-s"), fullBleedItemKeys(turns)) + assertEquals(mapOf("u" to 0, "a" to 3), messageIdToLazyIndex(turns)) + assertEquals(mapOf("u" to 2, "a" to 5), messageIdToLazyIndex(turns, leadingItems = 2)) + } + + @Test + fun toolAnchorSurvivesPrependThatExtendsItsAgentTurn() { + val tool = ChatMessage("live-tool", MessageRole.TOOL, "output") + val older = + (0..149).map { + ChatMessage("old-$it", MessageRole.ASSISTANT, "answer", reasoningText = "thinking") + } + assertEquals(listOf("tool-live-tool"), fullBleedItemKeys(groupIntoTurns(listOf(tool)))) + val keys = fullBleedItemKeys(groupIntoTurns(older + tool)) + assertEquals("reasoning-old-0", keys.first()) + assertEquals(151, keys.indexOf("tool-live-tool")) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ReadNotificationReviewRegressionTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ReadNotificationReviewRegressionTest.kt index 12fe98ca..b06cbaba 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ReadNotificationReviewRegressionTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ReadNotificationReviewRegressionTest.kt @@ -104,6 +104,172 @@ class ReadNotificationReviewRegressionTest { assertEquals(listOf("old-completion", "new-completion"), mapped.map { it.completionId }) } + @Test + fun pagingMergePreservesBothIdentitiesAndDoesNotConsumeOlderRepeatedReply() { + val live = ChatMessage(id = "ws", role = MessageRole.ASSISTANT, content = "Done", completionId = "comp") + val newest = listOf(SessionMessage(id = 200, role = "assistant", content = JsonPrimitive("Done"))) + val mapped = mapServerMessages("session", newest, 0, true, listOf(live)) + val hydrated = mergeTranscriptWithLive(mapped, listOf(live), preserveLiveIds = true) + assertEquals("ws", hydrated.single().id) + assertEquals("rest-session-200", hydrated.single().restId) + assertEquals("comp", hydrated.single().completionId) + val older = listOf(SessionMessage(id = 100, role = "assistant", content = JsonPrimitive("Done"))) + val page = mapServerMessages("session", older, 150, true, hydrated, isPagingOlder = true) + val merged = mergeTranscriptWithLive(page, hydrated, chronological = false, preserveLiveIds = true) + assertEquals(listOf("rest-session-100", "ws"), merged.map { it.id }) + assertEquals(listOf(null, "comp"), merged.map { it.completionId }) + assertEquals("rest-session-200", merged.last().restId) + // A later refresh of ONLY the old row must not reassign the confirmed UUID. + assertNull(mapServerMessages("session", older, 0, true, hydrated).single().completionId) + assertEquals("comp", mapServerMessages("session", newest, 0, true, hydrated).single().completionId) + } + + @Test + fun olderPageAndCacheMergeCannotClaimUnconfirmedNotificationByText() { + val live = ChatMessage(id = "ws", role = MessageRole.ASSISTANT, content = "Done", completionId = "comp") + val older = listOf(SessionMessage(id = 100, role = "assistant", content = JsonPrimitive("Done"))) + val page = mapServerMessages("session", older, 150, true, listOf(live), isPagingOlder = true) + val merged = mergeTranscriptWithLive(page, listOf(live), chronological = false, preserveLiveIds = true) + assertEquals(2, merged.size) + assertNull(merged.single { it.id == "rest-session-100" }.completionId) + assertNull(merged.single { it.id == "ws" }.restId) + assertEquals(2, mergeCachedTranscriptPage(page, listOf(live)).size) + } + + @Test + fun repeatedCompletionMatchesStayOnTheSameLiveKeysAfterMerge() { + val live = + listOf( + ChatMessage(id = "old-ws", role = MessageRole.ASSISTANT, content = "Done", completionId = "old"), + ChatMessage(id = "new-ws", role = MessageRole.ASSISTANT, content = "Done", completionId = "new"), + ) + val page = + mapServerMessages( + "session", + listOf(SessionMessage(id = 200, role = "assistant", content = JsonPrimitive("Done"))), + 0, + true, + live, + ) + val merged = mergeTranscriptWithLive(page, live, preserveLiveIds = true) + assertEquals("rest-session-200", merged.single { it.id == "new-ws" }.restId) + assertNull(merged.single { it.id == "old-ws" }.restId) + assertEquals("new", merged.single { it.id == "new-ws" }.completionId) + } + + @Test + fun durableTargetWinsOverRepeatedLiveTextEvenWithLegacyPositions() { + ReplyNotificationTracker.setTargetForTest( + ReplyNotificationTarget("default", "session", "comp", 1L, serverMessageId = 100), + ) + val live = ChatMessage(id = "ws", role = MessageRole.ASSISTANT, content = "Done", completionId = "comp") + val rows = + listOf( + SessionMessage(id = 100, role = "assistant", content = JsonPrimitive("Done")), + SessionMessage(id = 200, role = "assistant", content = JsonPrimitive("Done")), + ) + for (latest in listOf(true, false)) { + val mapped = mapServerMessages("session", rows, 10, latest, listOf(live), context = context) + assertEquals(listOf("comp", null), mapped.map { it.completionId }) + val merged = mergeTranscriptWithLive(mapped, listOf(live), preserveLiveIds = true) + assertEquals(if (latest) "rest-session-100" else "rest-session-10", merged.single { it.id == "ws" }.restId) + assertNull(mapServerMessages("session", rows.takeLast(1), 11, latest, listOf(live)).single().completionId) + } + } + + @Test + fun exactCacheAndRestOverlapsRetainCompletionWhenPreferredCopyHasNone() { + val live = + ChatMessage( + id = "ws", + role = MessageRole.ASSISTANT, + content = "Done", + restId = "rest-session-200", + completionId = "comp", + ) + val rest = live.copy(id = "rest-session-200", restId = null, completionId = null) + assertEquals("comp", dedupeCachedMessages(listOf(live, rest)).single().completionId) + assertEquals("comp", mergeCachedTranscriptPage(listOf(rest), listOf(live)).single().completionId) + assertEquals("comp", mergeTranscriptWithLive(listOf(rest), listOf(live)).single().completionId) + val restored = rest.copy(completionId = "comp").toEntity("session").toUiModel() + val richWithoutCompletion = live.copy(completionId = null) + val merged = mergeCachedTranscriptPage(listOf(restored), listOf(richWithoutCompletion)).single() + assertEquals("ws", merged.id) + assertEquals("rest-session-200", merged.restId) + assertEquals("comp", merged.completionId) + } + + @Test + fun durableHydrationFoldsMixedCacheEchoIntoRichLiveReply() { + ReplyNotificationTracker.setTargetForTest( + ReplyNotificationTarget("default", "session", "comp", 1L, serverMessageId = 200), + ) + val live = ChatMessage(id = "ws", role = MessageRole.ASSISTANT, content = "Done", completionId = "comp") + val echo = live.copy(id = "rest-session-200", completionId = null) + val current = listOf(echo, live) + val mapped = + mapServerMessages( + "session", + listOf(SessionMessage(id = 200, role = "assistant", content = JsonPrimitive("Done"))), + 0, + true, + current, + ) + val merged = mergeTranscriptWithLive(mapped, current, preserveLiveIds = true).single() + assertEquals("ws", merged.id) + assertEquals("rest-session-200", merged.restId) + assertEquals("comp", merged.completionId) + } + + @Test + fun cachedNewestDuplicateWithoutDurableIdKeepsCompletionOnNewestLiveReply() { + // #129 review: a metadata-free cached echo must not redirect the newest WS completion. + ReplyNotificationTracker.setTargetForTest( + ReplyNotificationTarget("default", "session", "comp", 1L, serverMessageId = null), + ) + val cachedNewest = + ChatMessage( + id = "rest-session-200", + role = MessageRole.ASSISTANT, + content = "Done", + completionId = null, + ) + val live = + ChatMessage( + id = "ws", + role = MessageRole.ASSISTANT, + content = "Done", + completionId = "comp", + restId = null, + ) + val current = listOf(cachedNewest, live) + val history = + listOf( + SessionMessage(id = 100, role = "assistant", content = JsonPrimitive("Done")), + SessionMessage(id = 200, role = "assistant", content = JsonPrimitive("Done")), + ) + val mapped = + mapServerMessages( + sessionId = "session", + messages = history, + offset = 0, + latestPaging = true, + liveMessages = current, + isPagingOlder = false, + context = context, + ) + val merged = mergeTranscriptWithLive(mapped, current, preserveLiveIds = true) + + assertNull(mapped.single { it.id == "rest-session-100" }.completionId) + assertEquals("comp", mapped.single { it.id == "rest-session-200" }.completionId) + assertEquals(2, merged.size) + assertEquals(setOf("rest-session-100", "ws"), merged.map { it.id }.toSet()) + assertEquals(2, merged.count { it.content == "Done" }) + assertNull(merged.single { it.id == "rest-session-100" }.completionId) + assertEquals("rest-session-200", merged.single { it.id == "ws" }.restId) + assertEquals("comp", merged.single { it.id == "ws" }.completionId) + } + @Test fun viewingOldCachedTextMustNotClearNewReplyNotification() { postTarget("Done") @@ -383,7 +549,11 @@ class ReadNotificationReviewRegressionTest { SessionMessage(id = 2, role = "assistant", content = JsonPrimitive("The result MEDIA:/opt/pic.png")), ) val mapped = mapServerMessages("session", history, 0, true, live, isPagingOlder = false) - assertEquals("comp-reasoning-media", mapped.single().completionId) + assertEquals(listOf("rest-session-1", "rest-session-2"), mapped.map { it.canonicalRestId }) + assertEquals("thinking step", mapped.first().reasoningText) + assertEquals("", mapped.first().content) + assertNull(mapped.first().completionId) + assertEquals("comp-reasoning-media", mapped.last().completionId) } @Test diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/components/ChatHistoryPrefetchTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/components/ChatHistoryPrefetchTest.kt new file mode 100644 index 00000000..69afacde --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/components/ChatHistoryPrefetchTest.kt @@ -0,0 +1,71 @@ +package com.m57.hermescontrol.ui.chat.components + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ChatHistoryPrefetchTest { + @Test + fun shortUpwardDragCanReachMovementThresholdDuringFling() { + val policy = ChatHistoryPrefetch() + assertFalse(policy.onScroll(30, 5f, userInput = true, canLoad = true)) + assertTrue(policy.onScroll(25, 30f, userInput = false, canLoad = true)) + assertFalse(policy.onScroll(10, 30f, userInput = false, canLoad = true)) + } + + @Test + fun upwardFlingCanReachPrefetchThresholdAfterDragEnds() { + val policy = ChatHistoryPrefetch() + assertFalse(policy.onScroll(70, 30f, userInput = true, canLoad = true)) + assertFalse(policy.onScroll(40, 40f, userInput = false, canLoad = true)) + assertTrue(policy.onScroll(25, 40f, userInput = false, canLoad = true)) + assertFalse(policy.onScroll(10, 40f, userInput = false, canLoad = true)) + policy.endGesture() + assertFalse(policy.onScroll(5, 40f, userInput = false, canLoad = true)) + } + + @Test + fun reversingDirectionDisarmsPreviousUpwardIntent() { + val policy = ChatHistoryPrefetch() + assertFalse(policy.onScroll(70, 30f, userInput = true, canLoad = true)) + assertFalse(policy.onScroll(50, -40f, userInput = true, canLoad = true)) + assertFalse(policy.onScroll(25, 40f, userInput = false, canLoad = true)) + } + + @Test + fun initialTopAndProgrammaticMovementDoNotFetch() { + val policy = ChatHistoryPrefetch() + assertFalse(policy.onScroll(0, 0f, userInput = true, canLoad = true)) + assertFalse(policy.onScroll(0, 100f, userInput = false, canLoad = true)) + assertFalse(policy.onScroll(0, -100f, userInput = true, canLoad = true)) + } + + @Test + fun prefetchStartsTwentyFiveRowsBeforeTopAfterMeaningfulMovement() { + val policy = ChatHistoryPrefetch() + assertFalse(policy.onScroll(30, 30f, userInput = true, canLoad = true)) + assertTrue(policy.onScroll(25, 1f, userInput = true, canLoad = true)) + } + + @Test + fun initialTopNeedsDeliberateGestureAndRequestsOnlyOnce() { + val policy = ChatHistoryPrefetch() + assertFalse(policy.onScroll(0, 10f, userInput = true, canLoad = true)) + assertTrue(policy.onScroll(0, 15f, userInput = true, canLoad = true)) + repeat(100) { + assertFalse(policy.onScroll(0, 50f, userInput = true, canLoad = true)) + } + policy.endGesture() + assertTrue(policy.onScroll(0, 25f, userInput = true, canLoad = true)) + } + + @Test + fun loadingAndExhaustionSuppressRequestsAndNextGestureCanRetry() { + val policy = ChatHistoryPrefetch() + assertFalse(policy.onScroll(20, 50f, userInput = true, canLoad = false)) + policy.endGesture() + assertTrue(policy.onScroll(20, 25f, userInput = true, canLoad = true)) + policy.endGesture() + assertTrue(policy.onScroll(20, 25f, userInput = true, canLoad = true)) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/fakes/FakeChatMessageDao.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/fakes/FakeChatMessageDao.kt index 36be9b7f..bc49b346 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/fakes/FakeChatMessageDao.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/fakes/FakeChatMessageDao.kt @@ -2,29 +2,80 @@ package com.m57.hermescontrol.ui.chat.fakes import com.m57.hermescontrol.data.local.ChatMessageDao import com.m57.hermescontrol.data.local.ChatMessageEntity +import com.m57.hermescontrol.data.local.canonicalMessageOrder import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ConcurrentMap /** * In-memory [ChatMessageDao] for use in tests. - * Uses [ConcurrentHashMap] for thread safety — no manual synchronization needed. + * Paging mirrors the DAO's numeric cursor; writes use its default order allocation methods. */ class FakeChatMessageDao : ChatMessageDao { private val messages: ConcurrentMap = ConcurrentHashMap() + private val ordering = compareBy { it.sortGroup }.thenBy { it.sortOrder }.thenBy { it.id } + + private var insertionSequence = 0L + + var fullSessionReads = 0 + private set + + var beforeRead: suspend () -> Unit = {} + override suspend fun sessionExists(sessionId: String): Boolean = messages.values.any { it.sessionId == sessionId } - override suspend fun getMessagesForSession(sessionId: String): List = - messages.values + override suspend fun getMessagesForSession(sessionId: String): List { + fullSessionReads++ + beforeRead() + return messages.values .filter { it.sessionId == sessionId } - .sortedBy { it.timestamp } - - override suspend fun upsert(message: ChatMessageEntity) { - messages[message.id] = message + .sortedWith(ordering) } - override suspend fun upsertAll(messageList: List) { - messageList.forEach { messages[it.id] = it } + val pageLimits = mutableListOf() + + override suspend fun getLatestMessagePage( + sessionId: String, + limit: Int, + ): List { + pageLimits += limit + beforeRead() + return messages.values + .filter { it.sessionId == sessionId } + .sortedWith(ordering.reversed()) + .take(limit) + } + + override suspend fun getMessagePage( + sessionId: String, + beforeGroup: Int, + beforeOrder: Long, + beforeId: String, + limit: Int, + ): List { + pageLimits += limit + beforeRead() + return messages.values + .filter { + it.sessionId == sessionId && + ( + it.sortGroup < beforeGroup || + ( + it.sortGroup == beforeGroup && + (it.sortOrder < beforeOrder || (it.sortOrder == beforeOrder && it.id < beforeId)) + ) + ) + }.sortedWith(ordering.reversed()) + .take(limit) + } + + override suspend fun getMessage(id: String): ChatMessageEntity? = messages[id] + + override suspend fun nextLocalOrder(): Long = insertionSequence + 1 + + override suspend fun writeMessage(message: ChatMessageEntity) { + if (message.id !in messages) insertionSequence++ + messages[message.id] = message } override suspend fun deleteMessagesForSession(sessionId: String) { @@ -33,14 +84,25 @@ class FakeChatMessageDao : ChatMessageDao { /** Direct access for test setup — bypasses the suspend modifier. */ fun addMessageDirect(message: ChatMessageEntity) { - messages[message.id] = message + val existing = messages[message.id] + if (existing == null) insertionSequence++ + val order = canonicalMessageOrder(message.restId ?: message.id, message.sessionId) + messages[message.id] = + message.copy( + sortGroup = if (order != null) 0 else 1, + sortOrder = order ?: existing?.sortOrder ?: insertionSequence, + ) } /** Reset all stored messages. */ fun clear() { messages.clear() + insertionSequence = 0L } + fun idsForSession(sessionId: String): Set = + messages.values.filter { it.sessionId == sessionId }.mapTo(mutableSetOf()) { it.id } + /** Returns the number of stored messages. */ fun count(): Int = messages.size }