fix(desktop): preserve pre-tool-call text parts on message completion - #64492
Closed
isukennedy wants to merge 2 commits into
Closed
fix(desktop): preserve pre-tool-call text parts on message completion#64492isukennedy wants to merge 2 commits into
isukennedy wants to merge 2 commits into
Conversation
The replaceTextPart function in completeAssistantMessage was dropping ALL text parts and replacing them with the final_response from message.complete. But final_response only contains the last API call's content — text streamed before tool calls (from earlier API calls in the tool-calling loop) was silently discarded. Fix: find the last tool-call part index and preserve text parts that appear before it. Only replace text parts after the last tool-call (which come from the final API call and are covered by finalText).
alt-glitch
added
type/bug
teknium1
reviewed
Jul 16, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for isolating this to the completion reconciliation path. The premise is confirmed on current main: replaceTextPart() removes every text part at apps/desktop/src/app/session/hooks/use-message-stream/index.ts:359-377, while tool events flush preceding deltas before inserting the tool row at :294-328.
Problems
- The PR changes only
apps/desktop/src/app/session/hooks/use-message-stream/index.tsand adds no regression coverage. Existing hook-level event harnesses inapps/desktop/src/app/session/hooks/use-message-stream/todo-cleanup.test.tsx:17-52provide the appropriate integration boundary.
Suggested changes
- Add a test for
message.deltabefore a tool,tool.start/tool.complete, another delta, andmessage.complete. Assert the pre-tool narration and tool row survive while the trailing streamed text is reconciled to the completion payload.
The implementation’s last-tool-call boundary is otherwise aligned with the current stream ordering. This is an automated hermes-sweeper review.
| const dedupeReference = normalize(visibleFinalText) | ||
|
|
||
| const kept = parts.filter(part => { | ||
| // Find the last tool-call part index. Text parts before it came from |
Contributor
There was a problem hiding this comment.
Please add a hook-level regression test for pre-tool message.delta text followed by a tool event and message.complete; this boundary is the behavior the change must preserve.
teknium1
added
sweeper:risk-session-state
Collaborator
|
closed by #65919 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the model streams text, calls tools, then streams more text, the desktop app drops the text that was streamed before the tool calls. Only the final API response's content survives.
Example: Model says "Here's what I found:" -> calls
search_files-> says "The results show X."What the user sees after completion: just "The results show X." — the pre-tool-call narration is gone.
Root cause
replaceTextPart()incompleteAssistantMessageunconditionally filters out ALLtextparts and replaces them with a singletextpart containingfinalTextfrommessage.complete. ButfinalTextonly contains the last API call's content — text from earlier API calls in the tool-calling loop was streamed viamessage.deltaand flushed totextparts, but those get nuked on completion.Fix
Find the last
tool-callpart index. Text parts before it are preserved (from earlier API calls). Only text parts after the last tool-call are replaced (they're from the final API call, covered byfinalText).Files changed
apps/desktop/src/app/session/hooks/use-message-stream/index.ts—replaceTextPart()incompleteAssistantMessage()