fix(desktop): honor display.interim_assistant_messages — stop collapsing mid-turn narration#61447
Conversation
…ing mid-turn narration Desktop collapsed a multi-step turn to only the final message on message.complete, dropping every interim assistant text segment streamed between tool calls. The text is persisted and reappears on reload, and every other surface (messaging gateway, Ink TUI, Desktop's own reload path) keeps it — Desktop's live-completion path was the sole outlier and never consulted the existing display.interim_assistant_messages setting. Wire the setting through to completion (default true) and extract the merge into a pure, tested mergeFinalAssistantText() that keeps interim narration and upgrades the trailing segment in place, matching the TUI/reload contract. Also clarify the interim_assistant_messages comments in config.py and cli-config.yaml.example (comment-only; no default change). Fixes NousResearch#54905.
|
This PR also fixes #61297 (same root cause — interim narration collapsing). Mentioning here for cross-reference. |
|
Thanks for the cross-reference — confirmed, #61297 is the same root cause (Desktop's live-completion path dropping every interim segment and keeping only Worth noting the report also ticks CLI (interactive chat) as a component, but the Ink TUI's |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: LGTM
What the PR Does
Honor display.interim_assistant_messages configuration, stopping mid-turn narration from being collapsed.
Assessment
- Small config honoring fix.
Reviewed by Hermes Agent
Review note / issue linkageThis also fixes #61822 (same Desktop root cause). Why #61822 maps here
Please add Sanity check done against current main
Scope caveat (not a blocker)
I closed the accidental salvage duplicate #61997 in favor of this original PR. |
What does this PR do?
On the Desktop app, a multi-step turn (
text → tool → text → tool → …)streams all its mid-turn narration live, then collapses to only the final
message the instant
message.completelands. Any substantive point the modelmakes between tool calls that the final message doesn't restate disappears from
the transcript. The text is persisted to
state.dband reappears on sessionreload — the loss is render-only — but Desktop is the only surface that does
this.
Root cause:
completeAssistantMessageinapps/desktop/src/app/session/hooks/use-message-stream/index.tsran areplaceTextPartthat unconditionally dropped everytextpart oncompletion and re-appended the gateway's
final_response(message.complete.text)as one trailing part.
final_responseis only the agent's last assistantsegment (
agent/conversation_loop.py:final_response = assistant_message.contentin the no-tool-calls branch), so every earlier narration segment was discarded.
git blameshows this filter is original to the desktop app's first commit —incidental, never a deliberate design decision.
This is a real divergence: the setting that governs interim commentary already
exists product-wide —
display.interim_assistant_messages(defaulttrue,documented as "signal, not noise") — and every other surface already keeps
interim text: the messaging gateway gates sending on it; the Ink TUI keeps
interleaved segments and appends only the final tail
(
ui-tui/.../turnController.ts::recordMessageComplete); and Desktop's ownreload path (
toChatMessages) reconstructs the interleaving. Desktop'slive-completion path was the sole outlier and consulted the setting nowhere.
The fix wires Desktop to honor the existing
display.interim_assistant_messagessetting (default
true, so the improved behavior is on by default; an explicitfalsepreserves the old lean collapse) and corrects the merge so keep-modematches what the TUI and reload path already produce — no new config key, no new
IPC (the renderer already fetches
config.display).Why this content is worth keeping (not filler)
Because
final_responseis only the agent's last segment, the dropped text isspecifically everything the agent worked out along the way — root-cause
findings, decisions, caveats, partial results — that the closing line doesn't
restate. The screenshot below is a debugging turn: the agent diagnoses that a
bare
catch {}is silently swallowing Stripe errors, then finishes with a terse"Fixed." BEFORE, the user is told it's fixed but never sees why it broke —
the diagnosis is gone. AFTER, the root-cause finding is preserved. That's the
case for interim being signal: it frequently carries the single most valuable
sentence in the turn.
Screenshot
Both panels are rendered by the real shipping
Threadcomponent, and the twomessage states are produced by running the actual
mergeFinalAssistantTextfunction this PR adds over one shared streamed input (built with the real
appendAssistantTextPart/upsertToolPartbuilders): left =keepInterim:false(today's collapse), right =
keepInterim:true(this PR's default). It is afaithful render of the function's output, not a hand-drawn mockup — though not a
live end-to-end gateway capture.
Related Issue
Fixes #54905. Also fixes #61297 (same Desktop root cause — the streamed analysis
collapses to only the brief final conclusion). Prior report #40903 was closed
completedwithout a fix — same bug.Type of Change
Changes Made
apps/desktop/src/lib/chat-messages.ts— extract the completion-merge into apure, exported
mergeFinalAssistantText(parts, finalText, keepInterim).keepInterim: truekeeps interim narration and upgrades the trailing streamedtext segment in place (the one
final_responserestates), or appends when thefinal text is genuinely new;
keepInterim: falseis byte-identical to the oldcollapse. Reasoning-dedup behavior is unchanged.
apps/desktop/src/app/session/hooks/use-message-stream/index.ts— replace theinline
replaceTextPartwith a call tomergeFinalAssistantText, reading thenew setting atom.
apps/desktop/src/store/session.ts— add$keepInterimAssistantMessagesatom(default
true) + setter, mirroring the backend config.apps/desktop/src/app/session/hooks/use-hermes-config.ts— set the atom fromconfig.display.interim_assistant_messageson config refresh (defaulttrue;only an explicit
falseopts into lean).apps/desktop/src/types/hermes.ts— addinterim_assistant_messages?: booleanto the
HermesConfig.displaytype.apps/desktop/src/lib/chat-messages.test.ts— 7 unit tests formergeFinalAssistantText(keep + lean modes).apps/desktop/src/app/session/hooks/use-message-stream/interim-narration.test.tsx— integration test driving the real hook through
message.start → delta → tool.start → tool.complete → delta → message.complete,asserting narration survives when on and collapses when off.
cli-config.yaml.example+hermes_cli/config.py— update theinterim_assistant_messagescomments (were "Gateway-only") to document that itnow also governs Desktop transcript behavior. Comment-only; no default change.
How to Test
the model narrates between tool calls (e.g. "check the repo layout, then
summarize", where it says something substantive before the final answer).
mid-turn narration vanishes until you reload the session.
After (default): the mid-turn narration stays in the transcript alongside
the tool calls and the final message.
display.interim_assistant_messages: falsein~/.hermes/config.yamlandrepeat — the old lean collapse (final message only) is preserved.
npm run --prefix apps/desktop typecheck(clean),npm run --prefix apps/desktop build(clean),and
cd apps/desktop && ../../node_modules/.bin/vitest run --environment jsdom src/lib/chat-messages.test.ts src/app/session/hooks/use-message-stream/interim-narration.test.tsx(37 passing).Checklist
Code
fix(desktop): …)tests/hermes_cli/test_config.py145,tests/gateway/test_display_config.py52, desktop vitest 37) — all pass.Note: desktop CI is typecheck + production build (both pass here); vitest
is included as fix evidence. The Python change is comment-only.
Documentation & Housekeeping
cli-config.yaml.example+hermes_cli/config.pycomments forinterim_assistant_messagescli-config.yaml.example(comment clarified; the key alreadyexisted, no new key added)
CONTRIBUTING.md/AGENTS.md— N/A (no architecture change)platform-specific code paths
Open question for maintainers
Desktop ignored
display.interim_assistant_messageswhile the TUI, messaginggateway, and reload path all respect/implement interim text. This PR assumes that
was an oversight to bring in line, not a deliberate Desktop-only lean default. If
it was deliberate, what's the rationale — so the default can be revisited rather
than flipped. Happy to add a Settings → Display toggle that writes the key if a
GUI control is wanted; kept out of this PR to keep the diff minimal since the
config key already drives it.