fix(chat): stop duplicating interim commentary before tool calls (issue #842) (#854)

* fix(chat): stop duplicating interim commentary before tool calls (issue #842)

Interim commentary the agent narrates before each tool call rendered
TWICE. Two mechanisms, both now covered:

1. The old tool.start orphan seal + message.complete's full text: when the
   complete payload repeats the commentary, the final bubble duplicated
   it. onMessageComplete now strips the sealed-orphan prefix (tracked in
   StreamingState) from the final text.

2. On-device capture + DB forensics (2026-08-08) exposed the real
   repeat offender: the app seals the RAW streamed text — which carries
   leading blank lines the model emits before its narration — while the
   backend persists a CLEANED copy (leading whitespace stripped). A
   transcript reload ~10s later merged the REST page, exact-content
   matching failed, and the cleaned REST copy rendered on top of the
   live bubble. sameLogicalMessage now compares trimmed content, so the
   sealed live bubble is covered by the REST row.

Also:
- session.interrupt seals the in-flight streaming message before
  clearing streaming state, so interrupting a commentary turn keeps the
  partial text
- regression tests cover both turn shapes (commentary in the final row →
  stripped; absent → kept) and the whitespace-drift reload case

* fix(chat): match tool rows by gateway call id, drop reasoning-echo dupes (#842)

REST transcript rows carry tool_call_id; live WS bubbles keep tool_id from
tool.start. Match tool rows on that 1:1 identity so MCP/web rows (stored as
raw <untrusted_tool_result> text with no JSON key) dedupe during the
post-turn transcript merge instead of ghosting after the final message.

reasoning.available echo (narration text == message body, sent ~40ms before
each tool.start) no longer overwrites the real reasoning trace, so narration
bubbles don't render their own text twice (body + Reasoning card).

Room v5 migration adds tool_call_id; regression tests use captured
on-device payload shapes.

* fix(chat): flush throttled tokens before orphan seal + prefix-safe dedupe (#842)

The tool.start seal raced the 33ms throttled token flush: deltas landing just
before the transition stayed in the buffer, so the sealed narration orphan came
out truncated (on-device capture: orphan ended '...chickpea', backend persisted
'...chickpea goodness:') and failed the logical-match against the clean REST
row — the commentary ghosted on the next transcript sync.

- ChatStreamingController.flushPendingTokens(): force-flushes the throttled
  TOKEN buffer on MessageStart/MessageComplete/MessageDone/ToolStart so the
  reducer seals the COMPLETE narration.
- sameLogicalMessage: accept prefix-covering (both sides >=40 chars) so a
  residual seal drift can never re-add a substantial narration.
- Regression tests: controller flush + truncated-orphan prefix match.

---------

Co-authored-by: M57 <hy4ri@users.noreply.github.com>

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt

@@ -34,6 +34,15 @@ data class ChatMessage(
     val isStreaming: Boolean = false,
     val toolName: String? = null,
     val toolStatus: ToolStatus? = null,
+    /**
+     * Gateway tool call id (`call_00_...`, from `tool.start` payload `tool_id`
+     * / REST `tool_call_id`). The REST transcript and the WS stream carry the
+     * SAME id for one tool call, so it is the robust 1:1 identity for
+     * deduplicating tool rows (issue #842) — result-content canonicalization
+     * fails for MCP/web tools because the REST side stores the payload as raw
+     * `<untrusted_tool_result>` text, not JSON.
+     */
+    val toolCallId: String = "",
     val approvalInfo: ApprovalInfo? = null,
     /** Transient clarify request data — when present, renders [ClarifyBubble] inline. */
     val clarifyInfo: ClarifyUi? = null,

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatStreamingController.kt

@@ -52,7 +52,14 @@ class ChatStreamingController(
         // Issue #755: the shared streaming state must not carry the previous
         // message's reasoning into the next one. The finalized message keeps
         // its own copy (persisted to Room), so clearing here is safe.
-        streamingState.update { it.copy(isReasoning = false, reasoningText = "") }
+        streamingState.update {
+            it.copy(
+                isReasoning = false,
+                reasoningText = "",
+                // Issue #842: sealed-orphan tracking belongs to one turn only.
+                sealedOrphanIds = emptyList(),
+            )
+        }
     }
 
     /**
@@ -90,6 +97,35 @@ class ChatStreamingController(
         flushReasoning()
     }
 
+    /**
+     * Force-flushes any throttled TOKEN buffer onto the streaming message
+     * before a state-transition event (ToolStart / MessageComplete /
+     * MessageDone). `handleMessageToken` only flushes when >=33ms have passed
+     * since the last flush, so a delta that lands just before the transition
+     * can still be sitting in the buffer. The reducer seals the streaming
+     * message into a finalized orphan at tool.start (issue #842) — sealing a
+     * stale copy truncated the narration's tail (observed on-device: the
+     * orphan ended "...chickpea" while the REST row ended "...chickpea
+     * goodness:"), which then failed the logical-match against the clean
+     * REST row and re-added the commentary as a ghost bubble. Flushing the
+     * tokens synchronously first makes the sealed orphan complete.
+     */
+    fun flushPendingTokens() {
+        val currentContent = streamingBuffer.toString()
+        if (currentContent.isEmpty()) return
+        streamingState.update { state ->
+            val current = state.streamingMessage ?: return@update state
+            state.copy(
+                streamingMessage =
+                    current.copy(
+                        content = currentContent,
+                        reasoningText =
+                            if (current.reasoningText.isNotBlank()) current.reasoningText else state.reasoningText,
+                    ),
+            )
+        }
+    }
+
     fun handleMessageToken(event: WsEvent.MessageToken) {
         if (!isCurrentSession(event.sessionId)) return
 

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt

@@ -107,7 +107,15 @@ internal fun canonicalToolResultKey(content: String): String? {
  * True when [a] and [b] are the same logical message (the WS-persisted and
  * REST-persisted copies of one row — they carry different ids, see #771).
  * Tool messages match on their normalized result payload; other roles on
- * exact content.
+ * exact content, IGNORING leading/trailing whitespace.
+ *
+ * Issue #842: the app seals the RAW streamed text (which can carry leading
+ * blank lines the model emits before its narration), while the backend
+ * persists a CLEANED copy (leading whitespace stripped). An exact-content
+ * compare made the reload merge treat them as different messages and add
+ * the REST copy on top — the commentary duplicated ~10s after the stream
+ * ended. Trim closes the drift: the sealed live bubble is covered by the
+ * REST row and the duplicate never renders.
  */
 internal fun sameLogicalMessage(
     a: ChatMessage,
@@ -115,11 +123,31 @@ internal fun sameLogicalMessage(
 ): 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
+        // `<untrusted_tool_result>` text (not JSON), so it has no key at all.
+        if (a.toolCallId.isNotBlank() && b.toolCallId.isNotBlank()) {
+            return a.toolCallId == b.toolCallId
+        }
         val ka = canonicalToolResultKey(a.content)
         val kb = canonicalToolResultKey(b.content)
         return ka != null && ka == kb
     }
-    return a.content == b.content
+    val ta = a.content.trim()
+    val tb = b.content.trim()
+    if (ta == tb) return true
+    // 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))
 }
 
 /**
@@ -620,7 +648,15 @@ class ChatViewModel(
             is WsEvent.MessageComplete,
             is WsEvent.MessageDone,
             is WsEvent.ToolStart,
-            -> streamingController.flushPendingReasoning()
+            -> {
+                streamingController.flushPendingReasoning()
+                // Issue #842: the token buffer can hold deltas that landed
+                // <33ms before the transition. The reducer seals the
+                // streaming message into the orphan at tool.start — flush
+                // first so the seal carries the COMPLETE narration (a
+                // truncated seal fails the later REST dedupe and ghosts).
+                streamingController.flushPendingTokens()
+            }
 
             else -> Unit
         }
@@ -987,6 +1023,12 @@ class ChatViewModel(
             }
 
             WsMethods.SESSION_INTERRUPT -> {
+                // Issue #842 follow-up: seal whatever the agent streamed so far
+                // (interim commentary + partial answer) BEFORE clearing the
+                // streaming state. The old tool.start orphan seal used to leave
+                // pre-tool text behind on interrupt; with that seal gone, the
+                // partial would otherwise vanish entirely.
+                sealStreamingMessageIfAny()
                 _uiState.update {
                     it.copy(
                         isAgentTyping = false,
@@ -1907,6 +1949,27 @@ class ChatViewModel(
 
     // ── Session resume recovery (desktop parity) ─────────────────────────
 
+    /**
+     * Persists the in-flight streaming message as-is (isStreaming=false) so an
+     * interrupted turn keeps the text the user already saw on screen. No-op
+     * when there is no streaming content/reasoning to save. (Issue #842
+     * follow-up: replaces the old tool.start orphan seal — the streaming
+     * message now survives tool calls, so interrupts are the only path that
+     * would otherwise drop the partial text.)
+     */
+    private fun sealStreamingMessageIfAny() {
+        val streaming = _streamingState.value.streamingMessage ?: return
+        if (streaming.content.isBlank() && streaming.reasoningText.isBlank()) return
+        val finalized = streaming.copy(isStreaming = false)
+        _uiState.update { it.copy(messages = (it.messages + finalized).dedupeById()) }
+        val sid = _uiState.value.currentSessionId
+        if (sid != null) {
+            viewModelScope.launch(ioDispatcher) {
+                repo.persistMessage(finalized, sid)
+            }
+        }
+    }
+
     private fun resetSessionState(
         sessionId: String?,
         title: String,
@@ -2281,7 +2344,11 @@ class ChatViewModel(
                                 }
                             }
                             val merged = mergedList.distinctBy { it.id }
-                            if (sameMessages(current.messages, merged)) current else current.copy(messages = merged)
+                            if (sameMessages(current.messages, merged)) {
+                                current
+                            } else {
+                                current.copy(messages = merged)
+                            }
                         }
                     }
 
@@ -2510,6 +2577,18 @@ class ChatViewModel(
                 }
             }
 
+        // 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
+        // `<untrusted_tool_result>` text with no JSON key to canonicalize.
+        val liveToolByCallId = linkedMapOf<String, ChatMessage>()
+        _uiState.value.messages
+            .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<ChatMessage>()
         // The gateway stores a reasoning-model's thinking as its OWN assistant
         // row (content = "", reasoning = trace) directly before the answer row.
@@ -2593,6 +2672,12 @@ class ChatViewModel(
             // 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)
@@ -2607,6 +2692,7 @@ class ChatViewModel(
                     role = role,
                     content = finalContent,
                     reasoningText = finalReasoning,
+                    toolCallId = msg.toolCallId,
                     attachments = attachments,
                     timestamp = timestamp,
                     isStreaming = false,

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt

@@ -12,6 +12,33 @@ private fun List<ChatMessage>.upsertById(message: ChatMessage): List<ChatMessage
     return (this + message).dedupeById()
 }
 
+/**
+ * Strips this turn's sealed-orphan prefix from the final message text.
+ *
+ * Issue #842: `message.complete` carries the FULL final assistant text, which
+ * often repeats the interim commentary that was already sealed into its own
+ * bubble(s) at each `tool.start`. When the complete text begins with the
+ * concatenation of those sealed contents, the prefix is removed so the final
+ * bubble shows only the not-yet-rendered part — every narration line appears
+ * exactly once (sealed orphan bubbles + stripped answer), matching the
+ * persisted transcript. Returns the text unchanged when there is no overlap
+ * (commentary that is NOT carried in the final row stays in its orphan
+ * bubbles and the final bubble keeps its full text).
+ */
+private fun stripSealedOrphanPrefix(
+    text: String,
+    messages: List<ChatMessage>,
+    sealedOrphanIds: List<String>,
+): String {
+    if (sealedOrphanIds.isEmpty()) return text
+    val prefix =
+        sealedOrphanIds
+            .mapNotNull { id -> messages.firstOrNull { it.id == id }?.content }
+            .joinToString("")
+    if (prefix.isEmpty() || !text.startsWith(prefix)) return text
+    return text.removePrefix(prefix)
+}
+
 /**
  * Pure state reducer for WebSocket events.
  *
@@ -263,15 +290,31 @@ object ChatWsEventReducer {
         // message.complete carries it into the finalized bubble.
         val text = event.text
         return if (!text.isNullOrBlank() && streamingState.streamingMessage != null) {
-            ReducerResult(
-                state = state,
-                streamingState =
-                    streamingState.copy(
-                        isReasoning = true,
-                        reasoningText = text,
-                        streamingMessage = streamingState.streamingMessage.copy(reasoningText = text),
-                    ),
-            )
+            // Issue #842: the gateway ALSO re-sends the agent's interim
+            // NARRATION as reasoning.available ~40ms before each tool.start
+            // (the commentary already rendered as the message body, echoed
+            // verbatim). Attaching the echo as reasoning would make every
+            // narration bubble render its own text twice (body + Reasoning
+            // card). Real reasoning never equals the message content, so the
+            // exact-content echo is skipped and the reasoning.delta trace
+            // (already in streamingState) stays authoritative.
+            val content = streamingState.streamingMessage.content
+            if (text.trim() == content.trim()) {
+                ReducerResult(
+                    state = state,
+                    streamingState = streamingState.copy(isReasoning = true),
+                )
+            } else {
+                ReducerResult(
+                    state = state,
+                    streamingState =
+                        streamingState.copy(
+                            isReasoning = true,
+                            reasoningText = text,
+                            streamingMessage = streamingState.streamingMessage.copy(reasoningText = text),
+                        ),
+                )
+            }
         } else {
             ReducerResult(
                 state = state,
@@ -297,14 +340,31 @@ object ChatWsEventReducer {
             event.reasoning
                 ?.takeIf { it.isNotBlank() }
                 ?: streamingState.reasoningText.ifBlank { streaming?.reasoningText ?: "" }
+        // Issue #842: the complete payload carries the FULL final text, which
+        // often repeats commentary already sealed into its own bubble(s) at
+        // each tool.start. Strip the sealed prefix so the final bubble shows
+        // only the part not already rendered — every narration line appears
+        // exactly once, matching the persisted transcript (sealed orphans +
+        // stripped answer). Turns whose final text does NOT contain the
+        // commentary are left untouched (orphans stay, final keeps its text).
+        val text = stripSealedOrphanPrefix(event.text, state.messages, streamingState.sealedOrphanIds)
+        // Edge: the complete payload was entirely covered by sealed orphans
+        // (the interim text already rendered as its own bubble(s)) — nothing
+        // new to show, so don't add an empty final bubble.
+        if (text.isBlank()) {
+            return ReducerResult(
+                state = state.copy(isAgentTyping = false),
+                streamingState = streamingState.copy(streamingMessage = null),
+            )
+        }
         val msg =
             streaming?.copy(
-                content = event.text,
+                content = text,
                 isStreaming = false,
                 reasoningText = reasoning,
             ) ?: ChatMessage(
                 role = MessageRole.ASSISTANT,
-                content = event.text,
+                content = text,
                 reasoningText = reasoning,
             )
         val effects = mutableListOf<ReducerEffect>()
@@ -389,9 +449,17 @@ object ChatWsEventReducer {
                 content = contentJson,
                 toolName = event.name,
                 toolStatus = ToolStatus.RUNNING,
+                // Issue #842: keep the gateway's call id so REST transcript
+                // rows (which carry the same `tool_call_id`) can be matched
+                // 1:1 against this live bubble during merges.
+                toolCallId = event.data?.get("tool_id") as? String ?: "",
             )
 
-        // Finalize any orphan streaming message
+        // Finalize any orphan streaming message (issue #771 + #842): interim
+        // text streamed BEFORE the tool is sealed as its own bubble (desktop
+        // parity). The sealed id is tracked in StreamingState so
+        // onMessageComplete can strip the repeated prefix — the complete
+        // payload often re-carries the same commentary text.
         var orphanToPersist: ChatMessage? = null
         val newState =
             if (streamingState.streamingMessage?.content?.isNotEmpty() == true) {
@@ -422,19 +490,19 @@ object ChatWsEventReducer {
             effects.add(ReducerEffect.PersistMessage(toolMessage, sid))
         }
 
-        // Issue #771: KEEP the streaming state across a tool call instead of
-        // resetting it. A reasoning-model turn streams its thinking BEFORE the
-        // tool starts (content is still empty here) — wiping the streaming
-        // message at tool.start made the reasoning bubble vanish mid-turn and
-        // left the finalized answer with no reasoning card. The stream stays
-        // alive; post-tool message.delta continues the SAME message and
-        // message.complete finalizes it with the reasoning intact.
-        // When an orphan WAS sealed (interim text before the tool), the
-        // streaming message is done — null it, but keep the reasoning text in
-        // the shared state so the post-tool fallback message still picks it up.
+        // Issue #771: KEEP the streaming state across a tool call when the
+        // stream has nothing to seal yet (a reasoning-model turn streams its
+        // thinking BEFORE the tool starts) — wiping it made the reasoning
+        // bubble vanish mid-turn. When an orphan WAS sealed (interim text
+        // before the tool), the streaming message is done — null it, keep the
+        // reasoning text in shared state, and remember the sealed orphan so
+        // message.complete can strip the repeated prefix.
         val finalStreamingState =
             if (orphanToPersist != null) {
-                streamingState.copy(streamingMessage = null)
+                streamingState.copy(
+                    streamingMessage = null,
+                    sealedOrphanIds = streamingState.sealedOrphanIds + orphanToPersist.id,
+                )
             } else {
                 streamingState
             }

app/src/main/java/com/m57/hermescontrol/ui/chat/StreamingState.kt

@@ -6,4 +6,12 @@ data class StreamingState(
     val thinkingText: String = "",
     val isReasoning: Boolean = false,
     val reasoningText: String = "",
+    /**
+     * Ids of assistant messages sealed at tool.start during the CURRENT turn
+     * (interim commentary before a tool call). Consumed by
+     * [com.m57.hermescontrol.ui.chat.ChatWsEventReducer.onMessageComplete] to
+     * strip the repeated prefix from the final text (issue #842). Reset on
+     * every fresh StreamingState (message.start / interrupt / session switch).
+     */
+    val sealedOrphanIds: List<String> = emptyList(),
 )