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 34d6f29b..4a33fb46 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 @@ -101,6 +101,12 @@ data class ChatMessage( val messageProvenance: MessageProvenance = MessageProvenance.UNKNOWN, /** Read from history, not observed live in this view. Never persisted as delivery state. */ val isHistoricalCache: Boolean = false, + /** + * Legacy USER restored without a canonical identity or a current send receipt. + * Placement only: keep it outside the live tail without asserting delivery. + * Transient; UNKNOWN provenance and the persisted row remain unchanged. + */ + val isRestoredUnconfirmed: Boolean = false, ) /** Cached REST rows already carry their canonical identity in the persisted primary key. */ 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 fcb7dc9b..a04c7951 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 @@ -335,6 +335,7 @@ internal fun dedupeCachedMessages( restId = it.canonicalRestId, completionId = message.completionId ?: it.completionId, displayKind = message.displayKind ?: it.displayKind, + isRestoredUnconfirmed = false, ) } ?: message } @@ -403,6 +404,9 @@ internal fun mergeCachedTranscriptPage( restId = match.canonicalRestId ?: message.canonicalRestId, completionId = match.completionId ?: message.completionId, displayKind = match.displayKind ?: message.displayKind, + isRestoredUnconfirmed = + match.isRestoredUnconfirmed && message.isRestoredUnconfirmed && + match.canonicalRestId == null && message.canonicalRestId == null, ) }.toMap() val resolvedOrders = @@ -437,6 +441,7 @@ internal fun mergeTranscriptWithLive( restId = (message.canonicalRestId ?: match.canonicalRestId).takeUnless { it == match.id }, displayKind = message.displayKind ?: match.displayKind, isHistoricalCache = false, + isRestoredUnconfirmed = false, ) } @@ -464,6 +469,7 @@ internal fun mergeTranscriptWithLive( tokenCount = message.tokenCount ?: match.tokenCount, completionId = message.completionId ?: match.completionId, isHistoricalCache = false, + isRestoredUnconfirmed = false, ) } @@ -482,6 +488,7 @@ internal fun mergeTranscriptWithLive( content = mergedContent, completionId = message.completionId ?: match?.completionId, isHistoricalCache = false, + isRestoredUnconfirmed = false, ) } } @@ -533,9 +540,10 @@ private fun List.inTranscriptOrder( ?: latestCanonical } if (hasPendingPredecessor) pendingOrderByLocal[message.id] = pendingLocalOrder ?: Long.MAX_VALUE - } else if (message.isHistoricalCache) { + } else if (message.isHistoricalCache || message.isRestoredUnconfirmed) { // Room groups UUID-only rows after all confirmed rows; that predecessor is - // not a chronological anchor. Keep unresolved history before the server window. + // not a chronological anchor. Restored legacy rows stay before the server + // window too, without promoting their uncertain delivery to confirmed history. localAnchors[message.id] = beforeCanonical } else { hasPendingPredecessor = true 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 05f5101a..5d6ddc93 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 @@ -3334,6 +3334,14 @@ class ChatViewModel( role = MessageRole.USER, content = displayContent, tokenCount = TokenEstimator.estimate(displayContent).takeIf { it > 0 }, + // Follow-up to #1253: stripped /queue text is an unconfirmed prompt, + // not a permanently-local command or an ambiguous legacy cache row. + messageProvenance = + if (result is SlashResult.QueuePrompt && displayContent != command) { + MessageProvenance.LOCAL_PENDING + } else { + MessageProvenance.UNKNOWN + }, ) val sessionId = _uiState.value.currentSessionId @@ -4417,15 +4425,24 @@ class ChatViewModel( // Receipt bubbles can be absent after rejection/reconnect. Offer their identities // to the canonical matcher without displaying unconfirmed messages again. val visibleIds = snapshot.messages.map { it.id }.toSet() + val currentSendScope = sendScope() + val receiptRows = + sendStore + .all() + .filter { it.scope == currentSendScope && it.sessionId == snapshot.currentSessionId } + val receiptBackedIds = + receiptRows + .filter { + it.state in + setOf(PendingSendState.SENDING, PendingSendState.ACCEPTED, PendingSendState.UNKNOWN) + }.mapTo(mutableSetOf()) { it.id } val receiptCandidates = if (cached) { emptyList() } else { - sendStore - .all() + receiptRows .filter { - it.scope == sendScope() && it.sessionId == snapshot.currentSessionId && - it.id !in visibleIds && + it.id !in visibleIds && it.state in setOf(PendingSendState.SENDING, PendingSendState.ACCEPTED, PendingSendState.UNKNOWN) }.map { @@ -4444,8 +4461,8 @@ class ChatViewModel( val mapped = mapPage(snapshot.messages) val currentById = snapshot.messages.associateBy { it.id } // Durable provenance distinguishes new unsent prompts after process death. - // Legacy UUID-only USER rows remain conservatively unconfirmed: UNKNOWN is - // not evidence that the server delivered them. + // Legacy UUID-only USER rows remain unconfirmed, but restored placement is + // independent of delivery: UNKNOWN must not pin old prompts after fresh replies. val page = if (cached) { mapped.map { message -> @@ -4459,6 +4476,24 @@ class ChatViewModel( message.role == MessageRole.USER -> false else -> true }, + isRestoredUnconfirmed = + when { + message.role != MessageRole.USER || + message.canonicalRestId != null || + message.isPermanentlyLocal() || + message.messageProvenance == MessageProvenance.LOCAL_PENDING || + message.id in receiptBackedIds -> { + false + } + + currentById[message.id] != null -> { + currentById.getValue(message.id).isRestoredUnconfirmed + } + + else -> { + true + } + }, ) } } else { diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCachedHistoryTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCachedHistoryTest.kt index 6d7ba108..1e446aec 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCachedHistoryTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatCachedHistoryTest.kt @@ -130,6 +130,38 @@ class ChatCachedHistoryTest { assertEquals(listOf(start.id, historicalUser.id, canonical.id), merged.map { it.id }) } + @Test + fun restoredRepeatedUsersKeepBothOccurrencesUntilEachCanonicalPageConfirmsOne() { + val first = + ChatMessage( + id = "restored-first", + role = MessageRole.USER, + content = "continue", + localOrder = 1, + isRestoredUnconfirmed = true, + ) + val second = first.copy(id = "restored-second", localOrder = 2) + val latest = ChatMessage(id = "rest-s-100", role = MessageRole.ASSISTANT, content = "Latest answer") + val restored = mergeCachedTranscriptPage(listOf(first, second), listOf(latest)) + assertEquals(listOf(first.id, second.id, latest.id), restored.map { it.id }) + assertTrue(restored.take(2).all { it.canonicalRestId == null }) + + val older = ChatMessage(id = "rest-s-20", role = MessageRole.USER, content = "continue") + val once = mergeTranscriptWithLive(listOf(older), restored, chronological = false) + assertEquals(2, once.count { it.role == MessageRole.USER }) + assertEquals(1, once.count { it.canonicalRestId == older.id }) + val confirmed = once.single { it.canonicalRestId == older.id } + assertFalse(confirmed.isRestoredUnconfirmed) + val remaining = once.single { it.role == MessageRole.USER && it.canonicalRestId == null } + assertTrue(remaining.isRestoredUnconfirmed) + assertEquals(MessageProvenance.UNKNOWN, remaining.messageProvenance) + + val twice = mergeTranscriptWithLive(listOf(older.copy(id = "rest-s-21")), once, chronological = false) + assertEquals(listOf(first.id, second.id, latest.id), twice.map { it.id }) + assertEquals(listOf("rest-s-20", "rest-s-21"), twice.take(2).map { it.canonicalRestId }) + assertTrue(twice.none { it.isRestoredUnconfirmed }) + } + @Test fun historicalRowsPrecedeCanonicalWindowWithoutChangingLiveTailOrAttachments() { val attachment = Attachment("content://test/file", "file.txt", "text/plain") 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 57f46ef5..f750132f 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 @@ -7564,6 +7564,204 @@ class ChatViewModelTest { ) } + @Test + fun paging_restoredLegacyUserBlockBeforeRestDoesNotFollowNewestReply() = + runTest { verifyRestoredLegacyUserPlacement(cacheFirst = true) } + + @Test + fun paging_restoredLegacyUserBlockAfterRestDoesNotFollowNewestReply() = + runTest { verifyRestoredLegacyUserPlacement(cacheFirst = false) } + + private suspend fun TestScope.verifyRestoredLegacyUserPlacement(cacheFirst: Boolean) { + val restored = + (1..3).map { index -> + ChatMessage( + id = "restored-legacy-$index", + role = MessageRole.USER, + content = "Earlier prompt $index", + // Placement must not infer delivery or chronology from wall-clock age. + timestamp = Long.MAX_VALUE - index, + ) + } + val pending = + ChatMessage( + id = "real-pending", + role = MessageRole.USER, + content = "Unsent current prompt", + timestamp = 1L, + messageProvenance = MessageProvenance.LOCAL_PENDING, + ) + (restored + pending).forEach { fakeRepo.persistMessage(it, "session-456") } + val cacheRead = CompletableDeferred() + if (!cacheFirst) fakeRepo.dao.beforeRead = { cacheRead.await() } + val response = CompletableDeferred>() + val (viewModel, _) = createViewModelWithSession() + val api = ApiClient.hermesApi + coEvery { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } coAnswers { response.await() } + + viewModel.switchSession("session-456") + runCurrent() + response.complete(pagingResponse(100..102)) + runCurrent() + cacheRead.complete(Unit) + advanceUntilIdle() + + val expected = restored.map { it.id } + (100..102).map { "rest-session-456-$it" } + pending.id + assertEquals( + "Restored legacy prompts must not form a stale block after the newest reply", + expected, + viewModel.uiState.value.messages + .map { it.id }, + ) + restored.forEach { original -> + val retained = + viewModel.uiState.value.messages + .single { it.id == original.id } + assertEquals(original.content, retained.content) + assertEquals(original.timestamp, retained.timestamp) + assertEquals(MessageProvenance.UNKNOWN, retained.messageProvenance) + assertNull("Moving a row must not invent delivery confirmation", retained.canonicalRestId) + assertFalse("Restored placement is not proven historical delivery", retained.isHistoricalCache) + } + assertTrue(fakeRepo.dao.idsForSession("session-456").containsAll(restored.map { it.id } + pending.id)) + assertEquals(0, fakeRepo.dao.fullSessionReads) + coVerify(exactly = 1) { + api.getSessionMessages("session-456", any(), any(), any(), any()) + } + verify(exactly = 0) { HermesWsClient.sendMessage(any(), any(), any(), any()) } + + viewModel.syncCurrentSession() + advanceUntilIdle() + assertEquals( + expected, + viewModel.uiState.value.messages + .map { it.id }, + ) + verify(exactly = 0) { HermesWsClient.sendMessage(any(), any(), any(), any()) } + } + + @Test + fun paging_queuePromptsPersistPendingProvenanceAfterRestart() = runTest { verifyQueuedPromptRestoration("/queue") } + + @Test + fun paging_queueAliasPromptsPersistPendingProvenanceAfterRestart() = runTest { verifyQueuedPromptRestoration("/q") } + + private suspend fun TestScope.verifyQueuedPromptRestoration(command: String) { + val (viewModel, sessionId) = createViewModelWithSession() + assertTrue(viewModel.sendMessage("$command pending queued prompt")) + advanceUntilIdle() + val queued = + viewModel.uiState.value.messages + .single { it.role == MessageRole.USER } + assertEquals("pending queued prompt", queued.content) + assertEquals( + "Queued prompts must retain durable pending provenance", + MessageProvenance.LOCAL_PENDING.name, + fakeRepo.dao.getMessage(queued.id)?.messageProvenance, + ) + + val api = ApiClient.hermesApi + coEvery { + api.getSessionMessages(sessionId, any(), any(), any(), any()) + } returns pagingResponse(100..102) + val recreated = createViewModel() + advanceUntilIdle() + recreated.switchSession(sessionId) + advanceUntilIdle() + + val restored = recreated.uiState.value.messages + assertEquals(queued.id, restored.last().id) + assertTrue(restored.indexOfFirst { it.id == "rest-$sessionId-102" } in 0 until restored.lastIndex) + assertEquals(MessageProvenance.LOCAL_PENDING, restored.last().messageProvenance) + assertNull(restored.last().canonicalRestId) + assertFalse(restored.last().isHistoricalCache) + verify(exactly = 1) { HermesWsClient.sendMessage(any(), any(), any(), any()) } + } + + @Test + fun paging_coldRestartReceiptBackedUnknownUserRemainsLiveTail() = + runTest { + val receiptBacked = + ChatMessage( + id = "receipt-backed-unknown-user", + role = MessageRole.USER, + content = "possibly accepted before restart", + messageProvenance = MessageProvenance.UNKNOWN, + ) + val rejected = + ChatMessage( + id = "receipt-backed-rejected-user", + role = MessageRole.USER, + content = "rejected before restart", + messageProvenance = MessageProvenance.UNKNOWN, + ) + fakeRepo.persistMessage(receiptBacked, "session-456") + fakeRepo.persistMessage(rejected, "session-456") + val store = ChatSendStore() + val scope = + listOf( + AuthManager.getBaseUrl(), + AuthManager.getSelectedProfileId() ?: AuthManager.DEFAULT_PROFILE_ID, + AuthManager.activeProfileId.value ?: AuthManager.DEFAULT_PROFILE_ID, + ).joinToString("\u001f") + store.put( + PendingSend( + id = receiptBacked.id, + scope = scope, + sessionId = "session-456", + text = receiptBacked.content, + mode = BusySendMode.QUEUE, + state = PendingSendState.UNKNOWN, + ), + ) + store.put( + PendingSend( + id = rejected.id, + scope = scope, + sessionId = "session-456", + text = rejected.content, + mode = BusySendMode.QUEUE, + state = PendingSendState.REJECTED, + ), + ) + val cacheRead = CompletableDeferred() + fakeRepo.dao.beforeRead = { cacheRead.await() } + val (viewModel, _) = createViewModelWithSession(sendStore = store) + coEvery { + ApiClient.hermesApi.getSessionMessages("session-456", any(), any(), any(), any()) + } returns pagingResponse(100..102) + + viewModel.switchSession("session-456") + runCurrent() + cacheRead.complete(Unit) + advanceUntilIdle() + + assertEquals( + listOf(rejected.id) + (100..102).map { "rest-session-456-$it" } + receiptBacked.id, + viewModel.uiState.value.messages + .map { it.id }, + ) + val rejectedRetained = + viewModel.uiState.value.messages + .single { it.id == rejected.id } + assertTrue(rejectedRetained.isRestoredUnconfirmed) + assertEquals(MessageProvenance.UNKNOWN, rejectedRetained.messageProvenance) + val retained = + viewModel.uiState.value.messages + .single { it.id == receiptBacked.id } + assertFalse(retained.isHistoricalCache) + assertFalse(retained.isRestoredUnconfirmed) + assertEquals(MessageProvenance.UNKNOWN, retained.messageProvenance) + assertNull(retained.canonicalRestId) + assertEquals(PendingSendState.UNKNOWN, store.all().single { it.id == receiptBacked.id }.state) + assertEquals(PendingSendState.REJECTED, store.all().single { it.id == rejected.id }.state) + verify(exactly = 0) { + HermesWsClient.sendMessage(any(), receiptBacked.content, any(), any()) + } + } + @Test fun paging_coldRestartLegacyUnknownUserRemainsConservativelyUnconfirmed() = runTest { @@ -7585,15 +7783,18 @@ class ChatViewModelTest { cacheRead.complete(Unit) advanceUntilIdle() assertEquals( - (100..102).map { "rest-session-456-$it" } + ambiguous.id, + listOf(ambiguous.id) + (100..102).map { "rest-session-456-$it" }, viewModel.uiState.value.messages .map { it.id }, ) - assertFalse( + val retained = viewModel.uiState.value.messages - .last() - .isHistoricalCache, - ) + .single { it.id == ambiguous.id } + assertFalse(retained.isHistoricalCache) + assertTrue(retained.isRestoredUnconfirmed) + assertEquals(MessageProvenance.UNKNOWN, retained.messageProvenance) + assertNull(retained.canonicalRestId) + assertTrue(fakeRepo.dao.idsForSession("session-456").contains(ambiguous.id)) } @Test