mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): stop assistant reply rendering twice after a tool-call turn (#70232)
The renderer showed two assistant bubbles for one turn — a partial streamed copy plus the clean final copy (#63679). A reload fixed it, so the persisted transcript was correct; this was live reconciliation. Root cause is in completeAssistantMessage (use-message-stream/index.ts). message.interim fires for BOTH verify-on-stop candidates AND ordinary tool-call turns (tui_gateway _load_interim_assistant_messages), and the interim seal clears streamId + sets interimBoundaryPending. So at message.complete the streamId fast-path is skipped and it enters the fallback. There the settle-onto-interim branch was gated on responsePreviewed — true ONLY for verify-on-stop. A normal tool-call turn whose final text matched its sealed interim satisfied neither settle branch and fell through to append a brand-new bubble. Two rows, distinct ids, id-based dedup cannot collapse them → renders twice. Fix: settle onto the sealed interim whenever the final CONTINUES it — final == interim, final starts with interim (streamed + trailing delta), or interim starts with final (streaming dropped characters). This is gated on existing.interim so it only ever collapses a genuine sealed interim, and a genuinely DIFFERENT final still appends as its own bubble. responsePreviewed is retained as an OR so the verify-on-stop continuation-budget case (final text rewritten, no shared prefix) still settles as before. The prior test that asserted a non-previewed identical interim+final produced TWO bubbles was encoding the bug; rewritten to assert one. Added coverage: prefix-extended non-previewed final collapses; a genuinely different final still appends (no over-collapse). Community analysis on the issue (seedSeenBubbleKeys / kind-guard) was against a pre-refactor v0.7.0 bundle — those symbols no longer exist; this is the current-code root cause. Co-authored-by: SHL0MS <SHL0MS@users.noreply.github.com>
This commit is contained in:
parent
75afaf46da
commit
53bdcacf17
2 changed files with 66 additions and 22 deletions
|
|
@ -501,27 +501,38 @@ export function useMessageStream({
|
|||
const existing = prev[index]
|
||||
const existingText = chatMessageText(existing).trim()
|
||||
|
||||
// The last assistant row is a sealed interim (a tool-call turn or a
|
||||
// verify-on-stop candidate — `message.interim` fires for BOTH, see
|
||||
// tui_gateway `_load_interim_assistant_messages`). When the final
|
||||
// completion is the SAME turn's reply, settle it onto that interim
|
||||
// instead of appending a second bubble. Continuity, not exact
|
||||
// equality: streaming can drop characters and the final may add a
|
||||
// trailing delta, so treat prefix-either-way as the same message.
|
||||
// (mergeFinalAssistantText, via completeMessage, does the real
|
||||
// text merge — replaces the interim's text with the full final.)
|
||||
const finalContinuesInterim = Boolean(
|
||||
existing.interim &&
|
||||
finalText &&
|
||||
existingText &&
|
||||
(finalText === existingText ||
|
||||
finalText.startsWith(existingText) ||
|
||||
existingText.startsWith(finalText))
|
||||
)
|
||||
|
||||
if (existing.pending || (!interimBoundaryPending && finalText && existingText === finalText)) {
|
||||
nextMessages = prev.map((message, messageIndex) =>
|
||||
messageIndex === index ? completeMessage(message) : message
|
||||
)
|
||||
} else if (
|
||||
interimBoundaryPending &&
|
||||
responsePreviewed &&
|
||||
finalText &&
|
||||
existingText &&
|
||||
finalText.startsWith(existingText)
|
||||
) {
|
||||
// The verification candidate was published provisionally as an
|
||||
// interim message and then reused as the terminal response
|
||||
// (continuation-budget fallback). Settle the interim in place
|
||||
// instead of creating a duplicate — the DB has one row, so the
|
||||
// live UI must agree. (#65919 review: duplicate-message blocker)
|
||||
//
|
||||
// Prefix match (not exact equality): the final response may be
|
||||
// the streamed text plus a trailing delta. mergeFinalAssistantText
|
||||
// (called via completeMessage) handles the actual merge — it
|
||||
// strips the old text parts and appends the full final text.
|
||||
} else if (interimBoundaryPending && (responsePreviewed || finalContinuesInterim)) {
|
||||
// Settle the interim in place instead of creating a duplicate —
|
||||
// the DB has one row, so the live UI must agree. Previously this
|
||||
// was gated on `responsePreviewed` alone, so a NON-previewed
|
||||
// tool-call turn whose final matched its sealed interim appended a
|
||||
// second bubble (the "renders twice: partial first copy + clean
|
||||
// final" bug, #63679). `finalContinuesInterim` closes that gap
|
||||
// for ordinary tool-call turns while `responsePreviewed` still
|
||||
// covers the verify-on-stop continuation-budget case even when the
|
||||
// final text was rewritten and no longer shares a prefix.
|
||||
nextMessages = prev.map((message, messageIndex) =>
|
||||
messageIndex === index ? completeMessage(message) : message
|
||||
)
|
||||
|
|
|
|||
|
|
@ -186,18 +186,51 @@ describe('useMessageStream interim text sealing', () => {
|
|||
expect(getState().interimBoundaryPending).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps an identical final completion distinct from an interim reply without response_previewed', async () => {
|
||||
it('settles an identical final onto a non-previewed interim (tool-call turn) instead of duplicating (#63679)', async () => {
|
||||
await mountStream()
|
||||
await start()
|
||||
|
||||
// A plain tool-call turn: the streamed text is sealed as an interim at the
|
||||
// tool boundary (no response_previewed — that flag is only for verify-on-
|
||||
// stop). The final completion is the SAME turn's reply. It must settle onto
|
||||
// the interim, not append a second bubble — the DB has one row. This is the
|
||||
// "renders twice" bug: partial streamed copy + clean final copy side by side.
|
||||
await interim('same reply')
|
||||
await complete('same reply')
|
||||
|
||||
// Without response_previewed, the interim and terminal replies are
|
||||
// distinct messages — the gateway didn't signal that the final reuses
|
||||
// the provisional candidate.
|
||||
const texts = assistantMessages()
|
||||
expect(texts.filter(t => t === 'same reply')).toHaveLength(2)
|
||||
expect(texts.filter(t => t === 'same reply')).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('settles a prefix-extended final onto a non-previewed interim (streamed + trailing delta)', async () => {
|
||||
await mountStream()
|
||||
await start()
|
||||
|
||||
// The stream dropped/settled early at the tool boundary; the final adds a
|
||||
// trailing delta. Same turn — one bubble with the full final text.
|
||||
await delta('partial')
|
||||
await interim('partial')
|
||||
await complete('partial answer continued')
|
||||
|
||||
const texts = assistantMessages()
|
||||
expect(texts.filter(t => t.includes('partial'))).toHaveLength(1)
|
||||
expect(texts[0]).toBe('partial answer continued')
|
||||
})
|
||||
|
||||
it('appends a genuinely different final as its own bubble (two real assistant segments)', async () => {
|
||||
await mountStream()
|
||||
await start()
|
||||
|
||||
// The interim is one segment (pre-tool commentary); the final is different
|
||||
// content, not a continuation of it. These are two real messages and must
|
||||
// both render — the fix must not over-collapse distinct replies.
|
||||
await interim('let me check the files')
|
||||
await complete('the answer is 42')
|
||||
|
||||
const texts = assistantMessages()
|
||||
expect(texts).toContain('let me check the files')
|
||||
expect(texts).toContain('the answer is 42')
|
||||
expect(texts).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('settles an identical final completion onto the interim when response_previewed', async () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue