From 8d92e2d3310552adbb563f92c4d6fb443cc6b3f9 Mon Sep 17 00:00:00 2001 From: Zero Liu Date: Wed, 1 Jul 2026 00:15:14 +0800 Subject: [PATCH] fix(agent-mode): coalesce streamed prose split by an invisible trail part MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A streamed agent message renders each `text` part as its own block-level markdown div, so when a top-level-invisible part — a hidden ToolSearch, an empty plan, or a concurrent background sub-agent's event — lands between two prose deltas, one sentence splits into two stacked blocks and shows a spurious mid-sentence line break in the live UI. The saved markdown, built from the flat `displayText` (deltas concatenated with no separator), has no break, so the artifact is render-only and self-heals on reload. Add a shared `isTopLevelInvisible` predicate and apply it across all three consumers of the parts stream: - `foldNodes` skips empty plans and coalesces adjacent text nodes, so the live trail matches the saved `displayText`. - `agentResponseText` derives from the folded trail, so Copy/Insert no longer injects a blank line into a split sentence (a visible tool card / reasoning / non-empty plan between prose still yields a real `\n\n` break). - `splitTrailingText` treats invisible parts as transparent and the collapsed "Worked for X" view renders the final run as one block, so the collapse can't strand half a sentence in the hidden research. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentMode/ui/AgentTrailView.tsx | 9 +- src/agentMode/ui/agentTrail.test.ts | 126 +++++++++++++++++++++++++++- src/agentMode/ui/agentTrail.ts | 107 +++++++++++++++++------ 3 files changed, 212 insertions(+), 30 deletions(-) diff --git a/src/agentMode/ui/AgentTrailView.tsx b/src/agentMode/ui/AgentTrailView.tsx index 04c46de7..8c663365 100644 --- a/src/agentMode/ui/AgentTrailView.tsx +++ b/src/agentMode/ui/AgentTrailView.tsx @@ -78,10 +78,11 @@ export const AgentTrail: React.FC = ({ return (
- {final.map((p, i) => ( - // eslint-disable-next-line @eslint-react/no-array-index-key -- text parts are append-only and may contain duplicate text - - ))} + {/* `final` may be several text parts only because a top-level-invisible + part (a background sub-agent event, a dropped ToolSearch, an empty + plan) split one prose run; join with "" to render the original + sentence as one block, not a broken-apart paragraph. */} + p.text).join("")} app={app} /> {actions}
); diff --git a/src/agentMode/ui/agentTrail.test.ts b/src/agentMode/ui/agentTrail.test.ts index abdd410d..b962402a 100644 --- a/src/agentMode/ui/agentTrail.test.ts +++ b/src/agentMode/ui/agentTrail.test.ts @@ -27,6 +27,17 @@ function text(value: string): AgentMessagePart { return { kind: "text", text: value }; } +function plan(contents: string[]): AgentMessagePart { + return { + kind: "plan", + entries: contents.map((content) => ({ + content, + priority: "medium" as const, + status: "pending" as const, + })), + }; +} + /** * Wrap a list of tool_call parts as children of a sub-agent (Task) so the * trail builder treats them as depth-1 peers — the level where compaction @@ -301,6 +312,65 @@ describe("buildAgentTrail", () => { const tree = buildAgentTrail(parts); expect(tree.map((n) => n.type)).toEqual(["reasoning", "plan"]); }); + + describe("coalesces a prose run split by a top-level-invisible part", () => { + it("merges text around a hidden ToolSearch into one block", () => { + const parts: AgentMessagePart[] = [ + text("let the "), + tool("ts", { vendorToolName: "ToolSearch" }), + text("kid feel the change"), + ]; + const tree = buildAgentTrail(parts); + expect(tree).toHaveLength(1); + expect(tree[0].type).toBe("text"); + if (tree[0].type === "text") { + expect(tree[0].part.text).toBe("let the kid feel the change"); + } + }); + + it("merges text around an empty plan into one block", () => { + const parts: AgentMessagePart[] = [text("let the "), plan([]), text("kid feel the change")]; + const tree = buildAgentTrail(parts); + expect(tree).toHaveLength(1); + expect(tree[0].type).toBe("text"); + if (tree[0].type === "text") { + expect(tree[0].part.text).toBe("let the kid feel the change"); + } + }); + + it("merges text around a sub-agent child, keeping the sub-agent card", () => { + // A backgrounded sub-agent's child event lands between two prose deltas; + // it renders nested under the parent card (not a top-level peer), so the + // prose must stay one block — this is the real-world repro. + const parts = withSubagent("task1", [ + text("let the "), + tool("c1", { vendorToolName: "Read" }), + text("kid feel the change"), + ]); + const tree = buildAgentTrail(parts); + expect(tree.map((n) => n.type)).toEqual(["subagent", "text"]); + const node = tree[1]; + if (node.type === "text") { + expect(node.part.text).toBe("let the kid feel the change"); + } + }); + + it("keeps two blocks when a VISIBLE tool splits the prose", () => { + const parts: AgentMessagePart[] = [ + text("before "), + tool("x", { vendorToolName: "Read" }), + text("after"), + ]; + const tree = buildAgentTrail(parts); + expect(tree.map((n) => n.type)).toEqual(["text", "action", "text"]); + }); + + it("keeps two blocks when a non-empty plan splits the prose", () => { + const parts: AgentMessagePart[] = [text("before "), plan(["step 1"]), text("after")]; + const tree = buildAgentTrail(parts); + expect(tree.map((n) => n.type)).toEqual(["text", "plan", "text"]); + }); + }); }); describe("splitTrailingText", () => { @@ -344,6 +414,33 @@ describe("splitTrailingText", () => { expect(research).toHaveLength(2); expect(final).toEqual([]); }); + + it("keeps a sentence whole in final when an empty plan splits it", () => { + const parts: AgentMessagePart[] = [ + tool("a"), + text("intro"), + tool("b"), + text("let the "), + plan([]), + text("kid feel the change"), + ]; + const { research, final } = splitTrailingText(parts); + expect(final.map((p) => p.text)).toEqual(["let the ", "kid feel the change"]); + // The empty plan is invisible but stays in research; the visible `tool b` + // is the real boundary. + expect(research.map((p) => p.kind)).toEqual(["tool_call", "text", "tool_call", "plan"]); + }); + + it("a sub-agent child in the trailing run stays transparent", () => { + const parts = withSubagent("task1", [ + text("let the "), + tool("c1"), + text("kid feel the change"), + ]); + const { research, final } = splitTrailingText(parts); + expect(final.map((p) => p.text)).toEqual(["let the ", "kid feel the change"]); + expect(research.map((p) => p.kind)).toEqual(["tool_call", "tool_call"]); + }); }); describe("agentResponseText", () => { @@ -372,9 +469,34 @@ describe("agentResponseText", () => { expect(agentResponseText(parts)).toBe("A\n\nB"); }); - it("drops a whitespace-only text part without leaving a stray blank line", () => { + it("coalesces fragments separated only by a dropped whitespace part", () => { const parts: AgentMessagePart[] = [text("A"), text(" "), text("B")]; - expect(agentResponseText(parts)).toBe("A\n\nB"); + // The whitespace part renders nothing, so the fragments are one prose run — + // joined with no separator, not a blank-line paragraph break. + expect(agentResponseText(parts)).toBe("AB"); + }); + + it("joins prose split by a hidden ToolSearch with no break", () => { + const parts: AgentMessagePart[] = [ + text("let the "), + tool("ts", { vendorToolName: "ToolSearch" }), + text("kid feel the change"), + ]; + expect(agentResponseText(parts)).toBe("let the kid feel the change"); + }); + + it("joins prose split by an empty plan with no break", () => { + const parts: AgentMessagePart[] = [text("let the "), plan([]), text("kid feel the change")]; + expect(agentResponseText(parts)).toBe("let the kid feel the change"); + }); + + it("joins prose split by a concurrent sub-agent child with no break", () => { + const parts = withSubagent("task1", [ + text("let the "), + tool("c1"), + text("kid feel the change"), + ]); + expect(agentResponseText(parts)).toBe("let the kid feel the change"); }); it("sanitizes the text the same way legacy chat copy does", () => { diff --git a/src/agentMode/ui/agentTrail.ts b/src/agentMode/ui/agentTrail.ts index 8f7cbe06..91eeabe1 100644 --- a/src/agentMode/ui/agentTrail.ts +++ b/src/agentMode/ui/agentTrail.ts @@ -27,9 +27,13 @@ export interface BuildAgentTrailOptions { /** * Split a turn's parts into the trailing user-visible answer and everything - * that came before (the "research"). The boundary is the last contiguous run - * of `text` parts: any `tool_call`, `thought`, or `plan` part after a text - * run reclassifies that run as research. + * that came before (the "research"). The boundary is the last run of `text` + * parts; a *visible* `tool_call`, `thought`, or non-empty `plan` after a text + * run reclassifies that run as research. Top-level-invisible parts (hidden + * ToolSearch, sub-agent children, an empty plan) are transparent: they stay in + * `research` but do not break the trailing run, so a sentence split by one (e.g. + * a background sub-agent event arriving mid-prose) stays whole in `final`. The + * caller renders `final` as a single coalesced block to match. * * Used by the trail UI to fold the research portion into a single * "Worked for X" block once a turn has cleanly ended (`stopReason: end_turn`). @@ -40,39 +44,46 @@ export function splitTrailingText(parts: AgentMessagePart[]): { research: AgentMessagePart[]; final: TextPart[]; } { - const final: TextPart[] = []; - let boundary = parts.length; + const turnToolIds = toolCallIds(parts); + const finalIdx = new Set(); for (let i = parts.length - 1; i >= 0; i--) { const p = parts[i]; if (p.kind === "text") { - final.unshift(p); - boundary = i; + finalIdx.add(i); continue; } + if (isTopLevelInvisible(p, turnToolIds)) continue; break; } - return { research: parts.slice(0, boundary), final }; + const research: AgentMessagePart[] = []; + const final: TextPart[] = []; + parts.forEach((p, i) => { + if (finalIdx.has(i)) final.push(p as TextPart); + else research.push(p); + }); + return { research, final }; } /** * The agent's full textual response across the turn, ready for the clipboard - * or the editor: every `text` part in stream order (not just the trailing run), - * joined and run through the same sanitization legacy chat applies - * (`cleanMessageForCopy`), so tool-call cards, reasoning, plans, and chat-only - * artifacts never leak in. Interleaving research (a `thought` or `tool_call` - * between two prose chunks) must not drop the earlier prose, so we collect all - * text parts rather than only the trailing run. Empty/whitespace-only parts are - * skipped so they don't leave stray blank lines. + * or the editor. Derived from the folded trail so Copy/Insert matches what the + * user sees: each top-level `text` node is one prose block — the trail coalesces + * a run split only by a top-level-invisible part (hidden ToolSearch, sub-agent + * child, empty plan) back into one — and distinct blocks (separated by a + * *visible* tool card, reasoning, or non-empty plan) join with a blank line. + * Interleaving research must not drop earlier prose, so every block is kept, not + * just the trailing run. Run through the same sanitization legacy chat applies + * (`cleanMessageForCopy`) so tool cards, reasoning, and chat-only artifacts + * never leak in. * Returns `""` when the turn produced no prose (a tool-only turn, or one * cancelled mid-tool) — the trail UI uses that to gate the Copy / Insert * affordances off so they never sit under an empty bubble. */ export function agentResponseText(parts: AgentMessagePart[]): string { - const text = parts - .filter((p): p is TextPart => p.kind === "text" && p.text.trim().length > 0) - .map((p) => p.text) - .join("\n\n"); - return cleanMessageForCopy(text); + const blocks = buildAgentTrail(parts) + .filter((n): n is Extract => n.type === "text") + .map((n) => n.part.text); + return cleanMessageForCopy(blocks.join("\n\n")); } /** @@ -99,6 +110,36 @@ function isHiddenTool(part: AgentMessagePart): boolean { return part.kind === "tool_call" && part.vendorToolName === "ToolSearch"; } +/** Every `tool_call` id present in a turn — used to recognize sub-agent children + * (a `tool_call` whose `parentToolCallId` names one of these). */ +function toolCallIds(parts: AgentMessagePart[]): Set { + const ids = new Set(); + for (const p of parts) if (p.kind === "tool_call") ids.add(p.id); + return ids; +} + +/** + * Parts that render to nothing at the *top level* of the trail: the hidden + * `ToolSearch` loader, an empty `plan` (`PlanPill` returns null), and sub-agent + * children (shown nested under their parent's card, never as a top-level peer). + * + * A prose run split only by these is one block: the store concatenates the + * streamed deltas into `displayText` with no separator, so a part that leaves no + * visible peer between two prose chunks must not turn them into two paragraphs. + * This is the difference between a real `\n\n` break (a visible tool card / + * reasoning / non-empty plan sits between the chunks) and a spurious one (e.g. a + * background sub-agent event arriving mid-sentence). + */ +function isTopLevelInvisible(part: AgentMessagePart, turnToolIds: Set): boolean { + if (isHiddenTool(part)) return true; + if (part.kind === "plan") return part.entries.length === 0; + return ( + part.kind === "tool_call" && + part.parentToolCallId !== undefined && + turnToolIds.has(part.parentToolCallId) + ); +} + /** * Fold a flat `AgentMessagePart[]` into a render tree. Compaction folds * runs of `N >= 2` consecutive same-`toolKey` peers into one aggregate @@ -169,16 +210,34 @@ function foldNodes( } if (p.kind === "text") { // Streamed prose breaks compaction (design doc §"Compaction"): a text - // part between two same-tool calls disqualifies grouping. Pushing - // straight to `out` here naturally enforces that — the next tool_call - // can't see a prior aggregate/action of the same key as `prev`. + // node between two same-tool calls disqualifies grouping. Keeping the + // text in `out` enforces that — the next tool_call can't see a prior + // aggregate/action of the same key as `prev`. // Skip empty/whitespace-only text parts so they don't become a flex // child contributing `gap-1` plus their own padding to the trail. if (p.text.trim().length === 0) continue; - out.push({ type: "text", part: p }); + // Coalesce with the previous text node when one is adjacent. Two text + // parts only become adjacent after a top-level-invisible part (hidden + // ToolSearch, sub-agent child, empty plan) was dropped between them, so + // merging reconstructs the original prose run — byte-identical to the + // flat `displayText` the store saved — instead of two stacked blocks that + // read as a spurious mid-sentence line break. + const prevNode = out[out.length - 1]; + if (prevNode && prevNode.type === "text") { + out[out.length - 1] = { + type: "text", + part: { kind: "text", text: prevNode.part.text + p.text }, + }; + } else { + out.push({ type: "text", part: p }); + } continue; } if (p.kind === "plan") { + // An empty plan renders nothing (`PlanPill` returns null); emitting it as + // a node would wedge between two prose parts and block the coalescing + // above, leaving the spurious break. Drop it so the prose stays one block. + if (p.entries.length === 0) continue; out.push({ type: "plan", part: p }); continue; }