From f3921957b5017df5e2bd639e129a6ddb2619b0e9 Mon Sep 17 00:00:00 2001 From: m57 <41645758+Hy4ri@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:17:10 +0300 Subject: [PATCH] fix(chat): reconcile live assistant reply carrying verifier footer with REST echo (#1241) (#1256) --- .../ui/chat/ChatServerMessageMapper.kt | 37 ++++++++ .../ui/chat/ChatTranscriptMerge.kt | 62 ++++++++++++- .../ui/chat/ChatVerifierFooter.kt | 80 +++++++++++++++++ .../ui/chat/ChatPagingMergeTest.kt | 44 +++++++++ .../ui/chat/ChatVerifierFooterTest.kt | 90 +++++++++++++++++++ 5 files changed, 310 insertions(+), 3 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatVerifierFooter.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatVerifierFooterTest.kt 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 394dc888..79a9972b 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 @@ -49,6 +49,7 @@ internal fun mapServerMessages( } val wsCompletionIdByRestIndex = mutableMapOf() + val verifierContentByRestIndex = 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. @@ -89,6 +90,27 @@ internal fun mapServerMessages( remainingWs.removeAt(wsIdx).completionId?.let { wsCompletionIdByRestIndex[i] = it } } } + // Fallback: match live assistant containing a file-mutation verifier footer against + // its footer-free REST counterpart (issue #1241). + 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.displayContentText ?: m.contentText + if (rawContent.isBlank()) continue + val canonicalContent = + if (rawContent.contains("MEDIA:")) HostMediaExtractor.strip(rawContent).trim() else rawContent.trim() + val wsIdx = remainingWs.indexOfLast { ChatVerifierFooter.matchesBase(it.content, canonicalContent) } + if (wsIdx >= 0) { + val matchedWs = remainingWs.removeAt(wsIdx) + matchedWs.completionId?.let { wsCompletionIdByRestIndex[i] = it } + // Propagate the richer live footer content to the REST row so canonical mapping keeps it + if (ChatVerifierFooter.split(matchedWs.content) != null) { + verifierContentByRestIndex[i] = matchedWs.content + } + } + } } // Reasoning follows the same reserved identities as completions, never a reusable text lookup. @@ -119,6 +141,19 @@ internal fun mapServerMessages( val match = remaining.indexOfLast { it.content.trim() == content } if (match >= 0) reasoningSources[index] = remaining.removeAt(match) } + for (index in messages.indices.reversed()) { + val row = messages[index] + val displayContent = row.displayContentText ?: row.contentText + if (reasoningSources[index] != null || displayContent.isBlank() || + row.role?.lowercase() in listOf("user", "system", "tool") || + liveByExactId[restIdAt(index)]?.completionId != null || index in wsCompletionIdByRestIndex + ) { + continue + } + val content = HostMediaExtractor.strip(displayContent).trim() + val match = remaining.indexOfLast { ChatVerifierFooter.matchesBase(it.content, content) } + if (match >= 0) reasoningSources[index] = remaining.removeAt(match) + } } val mapped = mutableListOf() @@ -166,6 +201,8 @@ internal fun mapServerMessages( var finalContent = if (role == MessageRole.USER) { stripGatewayAttachedContext(stripGatewaySteerWrapper(rawContent)) + } else if (role == MessageRole.ASSISTANT && verifierContentByRestIndex.containsKey(index)) { + verifierContentByRestIndex[index] ?: rawContent } else { rawContent } 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 1eb48730..fcb7dc9b 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 @@ -185,6 +185,14 @@ internal class TranscriptComparison( val tb = trimmed.getOrPut(b.content) { b.content.trim() } if (a.role == MessageRole.USER && (ta.startsWith("/") || tb.startsWith("/"))) return false if (ta == tb) return true + if (a.role == MessageRole.ASSISTANT && + (a.completionId != null || b.completionId != null) && + a.completionId == b.completionId && + ta.isNotBlank() && tb.isNotBlank() && + ChatVerifierFooter.matchesBase(ta, tb) + ) { + return true + } if (a.role == MessageRole.USER && captions.getOrPut(ta) { stripAttachmentRefLines(ta) } == captions.getOrPut(tb) { stripAttachmentRefLines(tb) } @@ -250,7 +258,17 @@ internal fun matchTranscriptMessages( ) && // Distinct IDs within the same source are separate occurrences, not echoes. ((message.canonicalRestId == null) != (other.canonicalRestId == null)) && - comparison.same(message, other) + ( + comparison.same(message, other) || + ( + allowAssistantContentMatches && + message.role == MessageRole.ASSISTANT && + message.content.isNotBlank() && other.content.isNotBlank() && + message.completionId != null && + message.completionId == other.completionId && + ChatVerifierFooter.matchesBase(message.content, other.content) + ) + ) } if (match != null) { used[match] = true @@ -341,6 +359,20 @@ internal fun mergeCachedTranscriptPage( incoming .mapIndexedNotNull { index, message -> val match = matches[index] ?: return@mapIndexedNotNull null + val preservedContent = + if (match.role == MessageRole.ASSISTANT && + ChatVerifierFooter.split(match.content) != null && + ChatVerifierFooter.matchesBase(match.content, message.content) + ) { + match.content + } else if (message.role == MessageRole.ASSISTANT && + ChatVerifierFooter.split(message.content) != null && + ChatVerifierFooter.matchesBase(message.content, match.content) + ) { + message.content + } else { + null + } val rich = when { // A late historical snapshot may contribute safe metadata, but it must @@ -367,6 +399,7 @@ internal fun mergeCachedTranscriptPage( match.id to rich.copy( id = match.id, + content = preservedContent ?: rich.content, restId = match.canonicalRestId ?: message.canonicalRestId, completionId = match.completionId ?: message.completionId, displayKind = match.displayKind ?: message.displayKind, @@ -408,9 +441,18 @@ internal fun mergeTranscriptWithLive( } preserveLiveIds && match != null && !match.id.startsWith("rest-") -> { + val mergedContent = + if (match.role == MessageRole.ASSISTANT && + ChatVerifierFooter.split(match.content) != null && + ChatVerifierFooter.matchesBase(match.content, message.content) + ) { + match.content + } else { + message.content + } match.copy( restId = message.canonicalRestId ?: match.canonicalRestId, - content = message.content, + content = mergedContent, timestamp = message.timestamp, isStreaming = message.isStreaming, reasoningText = message.reasoningText.ifBlank { match.reasoningText }, @@ -426,7 +468,21 @@ internal fun mergeTranscriptWithLive( } else -> { - message.copy(completionId = message.completionId ?: match?.completionId, isHistoricalCache = false) + val mergedContent = + if (message.role == MessageRole.ASSISTANT && + match != null && + ChatVerifierFooter.split(match.content) != null && + ChatVerifierFooter.matchesBase(match.content, message.content) + ) { + match.content + } else { + message.content + } + message.copy( + content = mergedContent, + completionId = message.completionId ?: match?.completionId, + isHistoricalCache = false, + ) } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatVerifierFooter.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatVerifierFooter.kt new file mode 100644 index 00000000..3651c7b1 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatVerifierFooter.kt @@ -0,0 +1,80 @@ +package com.m57.hermescontrol.ui.chat + +/** + * Utility to recognize and extract File-mutation verifier footers appended to + * assistant responses on live channels. + * + * Pattern emitted by backend `turn_explainers._format_file_mutation_failure_footer`: + * ⚠️ File-mutation verifier: N file edit(s) FAILED this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm what actually landed. + * • `path` — [tool] error_preview + * • … and N more + */ +internal object ChatVerifierFooter { + private const val HEADER_PREFIX = "⚠️ File-mutation verifier:" + private const val HEADER_NO_EMOJI_PREFIX = "File-mutation verifier:" + + data class SplitResult( + val body: String, + val footer: String, + ) + + /** + * Splits [content] into (body, footer) if it cleanly ends with a recognized + * File-mutation verifier block. If not, returns null. + */ + fun split(content: String): SplitResult? { + val trimmed = content.trimEnd() + if (trimmed.isBlank()) return null + + val headerIdx = + trimmed.lastIndexOf(HEADER_PREFIX).let { idx -> + if (idx >= 0) idx else trimmed.lastIndexOf(HEADER_NO_EMOJI_PREFIX) + } + if (headerIdx <= 0) return null + + // Must be preceded by a newline (standalone block) + val charBefore = trimmed[headerIdx - 1] + if (charBefore != '\n') return null + + val headerAndRest = trimmed.substring(headerIdx) + val lines = headerAndRest.lines() + val firstLine = lines.first().trim() + + // Verify first line structure: "⚠️ File-mutation verifier: ... FAILED this turn..." + if (!firstLine.contains("file edit(s) FAILED this turn") && + !firstLine.contains("file(s) were NOT modified this turn") + ) { + return null + } + + // Remaining lines must be bullet items or continuation lines + for (i in 1 until lines.size) { + val line = lines[i] + if (line.isBlank()) continue + val trimmedLine = line.trimStart() + // Bullets typically start with • + if (!trimmedLine.startsWith("•") && !trimmedLine.startsWith("-") && !line.startsWith(" ")) { + return null + } + } + + val body = trimmed.substring(0, headerIdx).trimEnd() + if (body.isBlank()) return null + + val footer = trimmed.substring(headerIdx) + return SplitResult(body = body, footer = footer) + } + + /** + * Returns true if [a] and [b] have the same underlying text once any + * trailing file-mutation verifier footer is removed. + */ + fun matchesBase( + a: String, + b: String, + ): Boolean { + val cleanA = split(a)?.body ?: a.trim() + val cleanB = split(b)?.body ?: b.trim() + return cleanA == cleanB + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt index 6a3297a9..815a315f 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatPagingMergeTest.kt @@ -655,6 +655,50 @@ class ChatPagingMergeTest { assertTrue(sameLogicalMessage(canonical, live)) } + @Test + fun verifierFooterReplyMatchesCanonicalCounterpartWithoutFooter() { + val footer = + """ + ⚠️ File-mutation verifier: 1 file edit(s) FAILED this turn despite any wording above that may suggest otherwise. Run git status or read_file to confirm what actually landed. + • /tmp/skill_lang_audit.py — [write_file] Write denied: '/tmp/skill_lang_audit.py' is outside HERMES_WRITE_SAFE_ROOT (/opt/data). Unset the variable or add this path's directory prefix. + """.trimIndent() + val body = "I completed the audit. Here are the findings." + val liveAssistant = + ChatMessage( + id = "uuid-123", + role = MessageRole.ASSISTANT, + content = "$body\n\n$footer", + completionId = "comp-1", + ) + val serverRow = + SessionMessage( + id = 42, + role = "assistant", + content = JsonPrimitive(body), + ) + + // 1. mapServerMessages correctly matches liveAssistant and acquires comp-1 and rich footer content + val mapped = mapServerMessages("session", listOf(serverRow), 0, true, listOf(liveAssistant)) + assertEquals(1, mapped.size) + val canonical = mapped.single() + assertEquals("rest-session-42", canonical.id) + assertEquals("comp-1", canonical.completionId) + assertTrue("Canonical mapped row should retain verifier footer", canonical.content.contains(footer)) + + // 2. mergeTranscriptWithLive merges into a single message preserving footer and acquiring restId + val merged = + mergeTranscriptWithLive( + restMessages = mapped, + currentMessages = listOf(liveAssistant), + preserveLiveIds = true, + ) + assertEquals(1, merged.size) + val single = merged.single() + assertEquals("uuid-123", single.id) + assertEquals("rest-session-42", single.restId) + assertTrue("Merged message must keep the verifier warning footer", single.content.contains(footer)) + } + @Test fun permanentlyLocalRowsDoNotMatchCanonicalRowsByContent() { val localFeedback = diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatVerifierFooterTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatVerifierFooterTest.kt new file mode 100644 index 00000000..7e2fcc63 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatVerifierFooterTest.kt @@ -0,0 +1,90 @@ +package com.m57.hermescontrol.ui.chat + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class ChatVerifierFooterTest { + private val sampleFooter = + """ + ⚠️ File-mutation verifier: 1 file edit(s) FAILED this turn despite any wording above that may suggest otherwise. Run git status or read_file to confirm what actually landed. + • /tmp/skill_lang_audit.py — [write_file] Write denied: '/tmp/skill_lang_audit.py' is outside HERMES_WRITE_SAFE_ROOT (/opt/data). Unset the variable or add this path's directory prefix. + """.trimIndent() + + private val backtickedFooter = + """ + ⚠️ File-mutation verifier: 1 file edit(s) FAILED this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm what actually landed. + • `/tmp/skill_lang_audit.py` — [write_file] Write denied: '/tmp/skill_lang_audit.py' is outside HERMES_WRITE_SAFE_ROOT (/opt/data). Unset the variable or add this path's directory prefix. + """.trimIndent() + + @Test + fun `split extracts body and footer for exact reporter format`() { + val body = "I completed the audit. Here are the findings." + val full = "$body\n\n$sampleFooter" + + val result = ChatVerifierFooter.split(full) + assertNotNull(result) + assertEquals(body, result?.body) + assertEquals(sampleFooter, result?.footer?.trim()) + } + + @Test + fun `split extracts body and footer for backend backticked format`() { + val body = "Audit summary:\n- item 1\n- item 2" + val full = "$body\n\n$backtickedFooter" + + val result = ChatVerifierFooter.split(full) + assertNotNull(result) + assertEquals(body, result?.body) + assertEquals(backtickedFooter, result?.footer?.trim()) + } + + @Test + fun `split handles multiple bullets and overflow note`() { + val footer = + """ + ⚠️ File-mutation verifier: 3 file edit(s) FAILED this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm what actually landed. + • `/a.py` — [patch] failed + • `/b.py` — [patch] failed + • … and 1 more + """.trimIndent() + val body = "Done" + val result = ChatVerifierFooter.split("$body\n\n$footer") + assertNotNull(result) + assertEquals("Done", result?.body) + } + + @Test + fun `split rejects footer when body is empty`() { + val result = ChatVerifierFooter.split(sampleFooter) + assertNull(result) + } + + @Test + fun `split rejects footer when inside a line without newline before it`() { + val text = "Look at this: ⚠️ File-mutation verifier: 1 file edit(s) FAILED this turn despite any wording..." + val result = ChatVerifierFooter.split(text) + assertNull(result) + } + + @Test + fun `split rejects text with unrelated trailing prose`() { + val full = "Body text\n\n$sampleFooter\nAnd here is more stuff after the footer." + val result = ChatVerifierFooter.split(full) + assertNull(result) + } + + @Test + fun `matchesBase compares body ignoring verifier footer`() { + val body = "Here is the response without errors." + val withFooter = "$body\n\n$sampleFooter" + + assertTrue(ChatVerifierFooter.matchesBase(body, withFooter)) + assertTrue(ChatVerifierFooter.matchesBase(withFooter, body)) + assertTrue(ChatVerifierFooter.matchesBase(withFooter, withFooter)) + assertFalse(ChatVerifierFooter.matchesBase("Different body", withFooter)) + } +}