🦌 Fix multi-step agent context and display merging#54
Open
masudahiroto wants to merge 13 commits intomainfrom
Open
🦌 Fix multi-step agent context and display merging#54masudahiroto wants to merge 13 commits intomainfrom
masudahiroto wants to merge 13 commits intomainfrom
Conversation
When a multi-step agent run produces multiple steps each with text AND tool calls, the conversation context was losing per-step boundaries. After page reload, all tool calls were merged into one assistant message and all text was concatenated into a separate message, causing the LLM to misunderstand previous turns. Server: Move hasEmittedTextStart/messageId to StepContext so TEXT_MESSAGE_START/END is emitted per step instead of per run. Client: Add STEP_FINISHED handler to flush per-step messages while maintaining backward compatibility with servers without step events. useServerEvents: Fix extractTurnMessages to preserve assistant content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… flash Two UI issues with per-step TEXT_MESSAGE emission: 1. Streaming text cleared at TEXT_MESSAGE_END caused flash between steps. Fix: accumulate text across steps via runTextRef, only clear at RUN_FINISHED. 2. Intermediate assistant messages with text+toolCalls displayed as separate bubbles. Fix: hide all assistant messages with toolCalls from display, combine all step text into the final saved message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Assistant messages now render with transparent background and minimal padding for a cleaner, less cluttered chat appearance. User messages retain the existing bubble style. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This reverts commit 25e5bdb.
saveAIResponse was using runTextRef (accumulated text from all steps) instead of client.currentMessageContent (last step only). Since intermediate steps' text is already preserved in turnMessages via extractTurnMessages, the final message duplicated that text. Use client.currentMessageContent for the saved message content. runTextRef continues to be used for streaming UI display only. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ntation Tests were re-implementing extractTurnMessages locally, so bugs in the real function would not be caught. Export extractTurnMessages from useServerEvents.ts and import it in tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests were re-implementing transformMessagesToClientFormat locally. Export from useChatManagement.ts and import in both persistence test files so bugs in the real function are caught by tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Per-step messages are preserved for correct LLM context, but the UI now combines consecutive assistant messages within each turn into a single bubble. Intermediate assistant messages (with toolCalls) have their text merged with the final text-only message using paragraph separators. Add mergeAssistantMessagesForDisplay utility with 8 unit tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace 5 scattered test files (~1850 lines) with 2 focused test files (~350 lines) that directly verify the core bug fix: per-step text+tool association is preserved in conversation context sent to LLM on 2nd run. - client.multiStepContext.test.ts: integration test covering message assembly, 2nd run_agent context correctness, and text deduplication - AISDKAgent.perStepEvents.test.ts: server unit test for per-step TEXT_MESSAGE_START/END emission and event ordering Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sages tests - Replace runTextRef (accumulated string never read) with hasTextFromPriorStepRef boolean for step separator logic - Add unit tests for mergeAssistantMessagesForDisplay (7 cases: simple exchange, multi-step merge, tool filtering, tool-only step, multiple turns, property preservation, ContentPart arrays) - Strengthen extractTurnMessages assertions in integration test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
still in draft. i will check this later and open it if there is no issue. |
…ests Replace Date.now() with constituent message IDs to produce stable React keys. Update stale JSDoc in extractTurnMessages to reflect STEP_FINISHED flushing. Add tests for empty input, trailing pending text, and ID determinism. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
This PR is ready for review. |
Both branches added fields to StepContext and RUN_FINISHED handler. Kept all additions from both sides: - This branch: messageId, hasEmittedTextStart, streamingText/chatId cleanup - PR #50: stepFinishReason, executingTool cleanup on truncation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mm-zacharydavison
requested changes
Apr 7, 2026
| * so the server can reconstruct valid API messages. | ||
| */ | ||
| function transformMessagesToClientFormat(persistedMessages: PersistedMessage[]): AGUIMessage[] { | ||
| export function transformMessagesToClientFormat(persistedMessages: PersistedMessage[]): AGUIMessage[] { |
Contributor
There was a problem hiding this comment.
i dont think you should export functions from hooks files, it's a leaking abstraction.
If they are common functions, they should probably be somewhere else.
Contributor
Author
There was a problem hiding this comment.
Thank you for pointing this out. I've addressed the review in 4deed3d.
I refactored the code to move these two functions (transformMessagesToClientFormat and extractTurnMessages) to packages/client/src/utils/messageConversion.ts.
| * backward compatibility when the server does not emit step events). | ||
| */ | ||
| function extractTurnMessages(messages: Message[], startIndex: number): PersistedMessage[] { | ||
| export function extractTurnMessages(messages: Message[], startIndex: number): PersistedMessage[] { |
Contributor
Author
There was a problem hiding this comment.
I've addressed this review too, as noted in this comment.
6191e6c to
4deed3d
Compare
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
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.
Summary
Fixes a bug where multi-step agent runs (producing multiple assistant steps with text+tool per step) corrupted the conversation context sent to the LLM on subsequent runs. Tool calls from all steps were being merged into a single assistant message, and all intermediate text was being concatenated into a single final message instead of being preserved per-step.
Also adds a display-layer merge (
mergeAssistantMessagesForDisplay) so the UI shows a single unified bubble per turn while keeping the correct per-step structure for LLM context.Changes
packages/client/src/client.ts— HandleSTEP_FINISHEDevent to flush per-step assistant+toolCall messages and tool results, preserving correct API message orderpackages/client/src/components/UseAIChatPanel.tsx— Replace inline filter withmergeAssistantMessagesForDisplayto combine intermediate assistant text into a single display bubblepackages/client/src/utils/mergeAssistantMessages.ts— New utility to merge consecutive assistant messages within a turn for display, using deterministic IDspackages/client/src/utils/mergeAssistantMessages.test.ts— Tests for merge utility including edge casespackages/client/src/utils/messageConversion.ts— MovedtransformMessagesToClientFormathere as a shared utility; fixedextractTurnMessagesto preserve content on intermediate assistant messagespackages/client/src/utils/messageConversion.test.ts— Tests using realtransformMessagesToClientFormatandextractTurnMessagespackages/client/src/hooks/useChatManagement.ts— ImporttransformMessagesToClientFormatfrom shared utility instead of local copypackages/client/src/hooks/useServerEvents.ts— Remove local copy ofextractTurnMessages, use shared utilitypackages/client/src/client.multiStepContext.test.ts— Integration tests validating per-step message structure and 2nd-run context correctnesspackages/server/src/agents/AISDKAgent.ts— Server-side per-step event emission fixespackages/server/src/agents/AISDKAgent.perStepEvents.test.ts— Tests for per-step event emissionpackages/client/src/providers/chatRepository/toolCallPersistence.test.ts— Updated persistence tests