Skip to content

box: stop a tool card splitting one assistant message into two text blocks - #174

Open
pythonlearner1025 wants to merge 2 commits into
mainfrom
fix/lody-message-split-daemon-patch
Open

box: stop a tool card splitting one assistant message into two text blocks#174
pythonlearner1025 wants to merge 2 commits into
mainfrom
fix/lody-message-split-daemon-patch

Conversation

@pythonlearner1025

Copy link
Copy Markdown
Member

Symptom

A lody 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:

[21] text       "Three"
[22] tool_call  toolu_0166kpDv... (grep ...)
[23] text       " characterization agents are running in parallel, plus the full suite on step 1."

Measured on a canary box 2026-09-02: 23 of 375 stored assistant text blocks (~6%) begin that way. It reads to the user as a garbled or lost message.

Root cause

The daemon already knows which message a delta belongs to, and throws it away.

  • claude-acp.js computes messageIdForGrouping(message) — the Anthropic assistant message id — and applyMessageId stamps it on every agent_message_chunk / agent_thought_chunk. It is emitted explicitly for grouping.
  • The ACP schema keeps it: zContentChunk.messageId.
  • The history applier maps the chunk to {type, text} without the id, and merges a delta only into items[items.length - 1]. So anything appended between two deltas of one message — a tool call, a subagent task — ends that text block permanently.

Tool calls are reported asynchronously, so parallel and background tools land between deltas routinely.

What was ruled out first

The applier is the right layer only because the transport is faithful, and that was measured, not assumed:

  • enqueueACPUpdate and the whole dispatchEvent → emit path are synchronous — no reordering on the way into the buffer.
  • Flush grouping is a single group across ~4,600 flushes in the daemon logs.
  • acp.flush_updates_now nests acp.flush_updates_batch rather than racing it (an apparent span overlap is nesting, not concurrency).
  • No duplicate toolCallIds; Loro replay shows only monotonic appends.

The patch

Six hunks against the published bundle, guarded on the installed package version plus each anchor at exactly one occurrence (a whole-file sha256 can only ever pin the first patch in a chain).

The discriminator is the id and nothing else. No text heuristic — "starts with a space" and "starts with a lowercase letter" both corrupt legitimate content, and a genuine text → tool → text across two API messages carries two different ids and must still render as two blocks.

Adjacency still wins first, so the patch can only ever join what today splits — it can never split what today joins. With no id the emitted object is byte-identical to today's, so Codex, Grok and DeepSeek are untouched.

The two emitted helpers are a behavioural transcription of the upstream fix, blitzdotdev/Lody#22, verified equal over a 69-case matrix covering both the merge target and the id the merged item keeps. Delete this patch when that merges and the daemon pin moves.

Verification

  • Applies to a copy of the real installed bundle (6 hunks); idempotent (second run exits 0).
  • Guards refuse correctly on a version bump and on a moved anchor.
  • Applies cleanly fourth in the Dockerfile chain, after the sha-guarded patch; patched bundle passes node --check.
  • Decision table over the shipped helper: 10/10, including the real "Three" case and every negative control.
  • Differential test vs the upstream TypeScript fix: 69 comparisons, 0 divergences.

Gates

  • npm run typecheck — pass
  • npm run lint:gate — pass (74 anti-slop, baseline; 0 blitz-house)
  • npm test — 723 tests, 1 failure: webapp-websocket-origin.test.ts timed out at 5000ms under parallel load and passes 4/4 in isolation. Unrelated to this diff, which touches only packages/box/patches/, the Dockerfile and docs.

🤖 Generated with Claude Code

pythonlearner1025 and others added 2 commits September 2, 2026 02:05
…locks

A lody 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:

  [21] text       "Three"
  [22] tool_call  toolu_0166kpDv... (grep ...)
  [23] text       " characterization agents are running in parallel, plus ..."

Measured on this box 2026-09-02: 23 of 375 stored assistant text blocks (~6%)
begin that way. It reads to the user as a garbled or lost message.

The daemon already knows which message a delta belongs to and throws it away.
`claude-acp.js` computes `messageIdForGrouping` -- the Anthropic assistant
message id -- and `applyMessageId` stamps it on every agent_message_chunk;
`zContentChunk.messageId` parses it. The history applier maps the chunk to
`{type, text}` without the id and merges a delta only into
`items[items.length - 1]`, so anything appended between two deltas of ONE
message ends that block permanently.

Verified the ordering is faithful before blaming the applier: enqueueACPUpdate
and the whole dispatchEvent -> emit path are synchronous, flush grouping is
always a single group across ~4,600 flushes, and flush_updates_now NESTS
flush_updates_batch rather than racing it. The interleaving is real in the ACP
stream -- parallel and background tools report themselves between deltas -- so
the applier is the right layer.

Six hunks, guarded on the installed package version plus each anchor at exactly
one occurrence. The discriminator is the id and nothing else: no text heuristic,
since "starts with a space" corrupts legitimate content and a genuine
text -> tool -> text across two messages must still render as two blocks.
Adjacency still wins first, so the patch can only join what today splits, never
split what today joins. With no id the emitted object is byte-identical to
today's, so every other adapter is untouched.

The two emitted helpers are a behavioural transcription of the upstream fix
(blitzdotdev/Lody#22), verified equal over a 69-case matrix covering both the
merge target and the id the merged item keeps. Delete this patch when that
merges and the daemon pin moves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`PATCH_SCRIPTS` says it is "every patch the image build applies to the
published bundle, IN THE ORDER the Dockerfile applies them", and it listed
three. Without this the harness boots a daemon that differs from the one a box
ships, so the message-split patch would never be exercised against a real
daemon — and the next patch to land would inherit the same gap.

Verified with it registered: test/lody-session-workdir.test.ts 26/26 and
test/lody-worktree-session.test.ts 8/8 against a real daemon carrying all four
patches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copy link
Copy Markdown
Member Author

Follow-up: harness registration + full gate results

Two corrections to the gate section above, both now resolved in b6f1481.

lody-daemon-harness.ts was not applying this patch. Its PATCH_SCRIPTS is documented as "every patch the image build applies to the published bundle, IN THE ORDER the Dockerfile applies them" and listed three. So the harness was booting a daemon that differs from the one a box ships, and this patch would never have been exercised against a real daemon. Registered as the fourth entry.

The full-suite failures were load flakes, not regressions. All three pass on re-run:

Suite Under full parallel load In isolation
control-plane/test/webapp-websocket-origin.test.ts timed out at 5000ms 4/4 pass
webapp/test/lody-session-workdir.test.ts beforeAll timed out at 120000ms 26/26 pass (14s)
webapp/test/lody-worktree-session.test.ts beforeAll timed out at 120000ms 8/8 pass (24s)

The two webapp suites start a real daemon, so they are the ones that actually prove this patch does not break daemon boot — node --check only proves the patched bundle parses. Both now boot and pass a daemon carrying all four patches.

Gates: typecheck pass, lint:gate pass (74 anti-slop = baseline, 0 blitz-house).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant