fix(acp): group streamed text by messageId so a tool call cannot split a message - #22
Open
pythonlearner1025 wants to merge 1 commit into
Open
fix(acp): group streamed text by messageId so a tool call cannot split a message#22pythonlearner1025 wants to merge 1 commit into
pythonlearner1025 wants to merge 1 commit into
Conversation
…split a message
Assistant messages were rendered split mid-sentence with a tool card
wedged in between the halves. In stored history the second half is a
separate text item that literally starts with a space:
[21] text "Three"
[22] tool_call toolu_0166kpDv... (grep ...)
[23] text " characterization agents are running in parallel, ..."
The adapter already publishes the grouping key. ACP defines `messageId`
as "all chunks belonging to the same message share the same messageId",
the Claude adapter stamps the Anthropic assistant message id on every
agent_message_chunk, Codex stamps the response item id, and
`zContentChunk` in acp/schema.ts parses it. history-apply.ts then threw
it away: chunks became `{type,text}` and a delta merged only when the
LAST item was text, so any tool_call appended between two deltas of the
same message ended that block permanently. Tool calls are reported
asynchronously, so parallel and background tools land between deltas
routinely — 13 of 271 stored text blocks on one box, about 5%.
Carry `messageId` onto the text/thought item and resolve the merge
target with it: scan back over the items a message can be interrupted by
(tool_call, subagent_task, images) and continue the block the id points
at. The scan stops at the first text-like item, so two different
messages are never joined, and no content heuristic is involved — the
discriminator is the id and nothing else.
Deltas without a `messageId` keep the previous last-item-only behavior
exactly, so adapters that publish none are unaffected. Adjacent blocks
still compact regardless of their ids; a block that ends up spanning two
messages simply stops claiming one. `parseAssistantTextTags` propagates
the id into the parts it splits out, so a <thinking> split does not lose
the grouping.
Model: claude-opus-5[1m]
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Symptom
An assistant message renders split mid-sentence with a tool card wedged between the halves. In stored history the second half is a separate text item that literally starts with a space:
Measured on one box: 13 of 271 stored assistant text blocks (~5%) begin that way. It reads to the user as a garbled or lost message.
Root cause
The adapter already publishes the grouping key, and
history-apply.tsthrows it away.messageIdas the grouping key for streamed chunks — all chunks belonging to the same message share it.agent_message_chunk; Codex stamps the response item id.zContentChunkinacp/schema.tsalready parses it.history-apply.tsmapped a chunk to{type, text}, dropping the id, and merged a delta only when the last item was text.tool_callnotifications are reported asynchronously, so a parallel or background tool routinely reports itself between two deltas of the same message. When that happened the text block ended at the tool call and the rest of the sentence became a second block below the card — permanently, in stored history.The fix
Carry
messageIdonto the text/thought item, and resolve the merge target with it: scan back over the items a message can be interrupted by (tool_call,subagent_task, images) and continue the block the id points at.The scan stops at the first text-like item, so two different messages are never joined. No content heuristic is involved — the discriminator is the id and nothing else. ("Continuation starts with a space/lowercase" was explicitly rejected: it corrupts legitimate content.)
Compatibility
messageIdkeep the previous last-item-only behavior exactly, so adapters that publish none are unaffected.parseAssistantTextTagspropagates the id into the parts it splits out, so a<thinking>split does not lose the grouping.Tests
10 new cases in
packages/shared/tests/acp-history-apply.test.ts, covering the bug, both appliers, and the negative controls (different messages stay separate; thought and text never cross; absent id behaves as before).corepack pnpm vitest run tests/acp-history-apply.test.tsinpackages/shared: 47 passed.🤖 Generated with Claude Code