From 03b71424008ab2f0615baa0b4962b883731cbf61 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Mon, 13 Apr 2026 19:58:11 -0500 Subject: [PATCH 1/2] fix(tui): reset segment state on claude-code sub-turn shrink Commit c8c416802 (#4144) introduced module-level renderedSegments state to track interleaved text/tool components per assistant turn, but never reset it when an adapter shrinks streamingMessage.content[] back to 0/1 at a provider sub-turn boundary within one assistant lifecycle (the claude-code adapter does this). Consequence chain: the segment walker finds the stale text-run entry at startIndex=0, calls updateContent on it with the new (shrunk) message, and the in-place edit destroys the prior sub-turn's visible text. New tool blocks at contentIndex=1 then collide with stale registrations, causing visual ordering corruption. hasToolsInTurn stays sticky-true and lastPinnedText never clears, so the pinned "Working - Latest Output" mirror freezes on the pre-shrink snapshot. Track lastContentLength explicitly. On shrink, clear renderedSegments, reset lastPinnedText, and reset lastProcessedContentIndex so the walker treats the new sub-turn as fresh segments that append after prior sub-turn children. Prior history stays rendered as frozen components; pendingTools and the spinner border are untouched. Adds two regression tests in chat-controller-ordering.test.ts: one verifies prior sub-turn components are not overwritten and new tools append in content[] order after a shrink, the other verifies the pinned markdown updates from the first sub-turn's text to the second sub-turn's text across a shrink boundary. Co-Authored-By: Claude Opus 4.6 --- .../src/core/chat-controller-ordering.test.ts | 151 ++++++++++++++++++ .../controllers/chat-controller.ts | 25 ++- 2 files changed, 172 insertions(+), 4 deletions(-) diff --git a/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts b/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts index 383580724..c03db913d 100644 --- a/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts +++ b/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts @@ -656,3 +656,154 @@ test("chat-controller does not duplicate text when content is [text, tool, text] assert.deepEqual((secondText as any).range, { startIndex: 2, endIndex: 2 }, "range must not be cleared on message_end (would cause duplication)"); }); + +// Regression for the claude-code sub-turn bug that followed #4144: +// an adapter can reset content[] back to 0/1 mid-lifecycle when a new provider +// sub-turn begins. The segment walker must NOT update prior-sub-turn text-run +// components in place (which would destroy earlier history) and must NOT reuse +// stale tool registrations for a new tool at the same contentIndex. Prior +// sub-turn children must stay frozen; new sub-turn segments must append after +// them, and the pinned "Latest Output" mirror must re-evaluate for the new sub-turn. +test("chat-controller freezes prior sub-turn and appends new segments when content shrinks", async () => { + (globalThis as any)[Symbol.for("@gsd/pi-coding-agent:theme")] = { + fg: (_key: string, text: string) => text, + bg: (_key: string, text: string) => text, + bold: (text: string) => text, + italic: (text: string) => text, + truncate: (text: string) => text, + }; + + const host = createHost(); + host.getMarkdownThemeWithSettings = () => ({}); + + const t1 = { type: "toolCall", id: "t1", name: "tool_one", arguments: {} }; + const t2 = { type: "toolCall", id: "t2", name: "tool_two", arguments: {} }; + + await handleAgentEvent(host, { type: "message_start", message: makeAssistant([]) } as any); + + // Sub-turn 1: grow to [A, T1, B] + await handleAgentEvent(host, { + type: "message_update", + message: makeAssistant([{ type: "text", text: "A" }]), + assistantMessageEvent: { + type: "text_delta", contentIndex: 0, delta: "A", + partial: makeAssistant([{ type: "text", text: "A" }]), + }, + } as any); + await handleAgentEvent(host, { + type: "message_update", + message: makeAssistant([{ type: "text", text: "A" }, t1]), + assistantMessageEvent: { + type: "toolcall_end", contentIndex: 1, + toolCall: { ...t1, externalResult: { content: [{ type: "text", text: "r1" }], details: {}, isError: false } }, + partial: makeAssistant([{ type: "text", text: "A" }, t1]), + }, + } as any); + await handleAgentEvent(host, { + type: "message_update", + message: makeAssistant([{ type: "text", text: "A" }, t1, { type: "text", text: "B" }]), + assistantMessageEvent: { + type: "text_delta", contentIndex: 2, delta: "B", + partial: makeAssistant([{ type: "text", text: "A" }, t1, { type: "text", text: "B" }]), + }, + } as any); + + assert.equal(host.chatContainer.children.length, 3, "sub-turn 1 renders 3 children"); + const priorA = host.chatContainer.children[0]; + const priorT1 = host.chatContainer.children[1]; + const priorB = host.chatContainer.children[2]; + + // Sub-turn boundary: adapter resets content[] to [C] + await handleAgentEvent(host, { + type: "message_update", + message: makeAssistant([{ type: "text", text: "C" }]), + assistantMessageEvent: { + type: "text_delta", contentIndex: 0, delta: "C", + partial: makeAssistant([{ type: "text", text: "C" }]), + }, + } as any); + + // Prior 3 children must still exist in DOM — and a NEW text-run for "C" appended after them. + assert.equal(host.chatContainer.children.length, 4, "shrink must append new segment, not replace prior history"); + assert.equal(host.chatContainer.children[0], priorA, "prior A component stays at index 0"); + assert.equal(host.chatContainer.children[1], priorT1, "prior T1 component stays at index 1"); + assert.equal(host.chatContainer.children[2], priorB, "prior B component stays at index 2"); + assert.notEqual(host.chatContainer.children[3], priorA, "new C text-run must be a different component from prior A"); + assert.equal(host.chatContainer.children[3]?.constructor?.name, "AssistantMessageComponent"); + + // Prior A component must still render "A", not be overwritten with "C". + function getRenderedTexts(comp: any): string[] { + const contentContainer = comp.children?.[0]; + if (!contentContainer) return []; + return (contentContainer.children ?? []) + .filter((c: any) => c.constructor?.name === "Markdown") + .map((c: any) => (c as any).text as string); + } + assert.deepEqual(getRenderedTexts(priorA), ["A"], "prior A text-run must still contain 'A' after shrink"); + assert.deepEqual(getRenderedTexts(priorB), ["B"], "prior B text-run must still contain 'B' after shrink"); + assert.deepEqual(getRenderedTexts(host.chatContainer.children[3]), ["C"], "new text-run must contain only 'C'"); + + // Sub-turn 2 grows with a new tool T2 at contentIndex=1. + await handleAgentEvent(host, { + type: "message_update", + message: makeAssistant([{ type: "text", text: "C" }, t2]), + assistantMessageEvent: { + type: "toolcall_end", contentIndex: 1, + toolCall: { ...t2, externalResult: { content: [{ type: "text", text: "r2" }], details: {}, isError: false } }, + partial: makeAssistant([{ type: "text", text: "C" }, t2]), + }, + } as any); + + // T2 must be appended after the new C text-run, not conflated with the stale T1 registration. + assert.equal(host.chatContainer.children.length, 5, "new tool appends after new text-run"); + assert.equal(host.chatContainer.children[4]?.constructor?.name, "ToolExecutionComponent"); + assert.notEqual(host.chatContainer.children[4], priorT1, "new T2 must be a different component from prior T1"); +}); + +// Regression: after a sub-turn shrink, lastPinnedText must be cleared so the +// pinned "Latest Output" mirror can display text from the new sub-turn instead +// of staying frozen on a stale snapshot (the "bottom green stays" symptom). +test("chat-controller updates pinned zone after sub-turn shrink", async () => { + (globalThis as any)[Symbol.for("@gsd/pi-coding-agent:theme")] = { + fg: (_key: string, text: string) => text, + bg: (_key: string, text: string) => text, + bold: (text: string) => text, + italic: (text: string) => text, + truncate: (text: string) => text, + }; + + const host = createHost(); + host.getMarkdownThemeWithSettings = () => ({}); + + const t1 = { type: "toolCall", id: "t1", name: "tool_one", arguments: {} }; + const t2 = { type: "toolCall", id: "t2", name: "tool_two", arguments: {} }; + + await handleAgentEvent(host, { type: "message_start", message: makeAssistant([]) } as any); + + // Sub-turn 1 with pinnable text before a tool → populates pinned zone with "first". + await handleAgentEvent(host, { + type: "message_update", + message: makeAssistant([{ type: "text", text: "first" }, t1]), + assistantMessageEvent: { + type: "toolcall_end", contentIndex: 1, + toolCall: { ...t1, externalResult: { content: [{ type: "text", text: "r1" }], details: {}, isError: false } }, + partial: makeAssistant([{ type: "text", text: "first" }, t1]), + }, + } as any); + const pinnedMarkdown = host.pinnedMessageContainer.children[1]; + assert.equal((pinnedMarkdown as any)?.text, "first", "pinned zone seeded with sub-turn 1 text"); + + // Sub-turn boundary: content resets to [second, t2]. + await handleAgentEvent(host, { + type: "message_update", + message: makeAssistant([{ type: "text", text: "second" }, t2]), + assistantMessageEvent: { + type: "toolcall_end", contentIndex: 1, + toolCall: { ...t2, externalResult: { content: [{ type: "text", text: "r2" }], details: {}, isError: false } }, + partial: makeAssistant([{ type: "text", text: "second" }, t2]), + }, + } as any); + + // Pinned markdown must now reflect the new sub-turn's text, not stay frozen on "first". + assert.equal((pinnedMarkdown as any)?.text, "second", "pinned zone must update after sub-turn shrink (#4144 regression)"); +}); diff --git a/packages/pi-coding-agent/src/modes/interactive/controllers/chat-controller.ts b/packages/pi-coding-agent/src/modes/interactive/controllers/chat-controller.ts index c230e37b7..59d780223 100644 --- a/packages/pi-coding-agent/src/modes/interactive/controllers/chat-controller.ts +++ b/packages/pi-coding-agent/src/modes/interactive/controllers/chat-controller.ts @@ -10,6 +10,10 @@ import { appKey } from "../components/keybinding-hints.js"; // Tracks the last processed content index to avoid re-scanning all blocks on every message_update let lastProcessedContentIndex = 0; +// Tracks the previous content[] length so we can detect when an adapter resets +// the assistant content array for a new provider sub-turn within one lifecycle. +let lastContentLength = 0; + // --- Segment walker state (per streaming assistant turn) --- type RenderedSegment = | { kind: "text-run"; startIndex: number; endIndex: number; component: AssistantMessageComponent } @@ -85,6 +89,7 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & { // Reset content index tracker and pinned state when a new assistant message starts if (event.type === "message_start" && event.message.role === "assistant") { lastProcessedContentIndex = 0; + lastContentLength = 0; lastPinnedText = ""; hasToolsInTurn = false; renderedSegments = []; @@ -108,6 +113,7 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & { lastPinnedText = ""; hasToolsInTurn = false; renderedSegments = []; + lastContentLength = 0; if (pinnedBorder) pinnedBorder.stopSpinner(); pinnedBorder = undefined; pinnedTextComponent = undefined; @@ -211,12 +217,22 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & { } const contentBlocks = host.streamingMessage.content; - // Some adapters reuse a single assistant lifecycle while internally - // spanning multiple provider turns. When a new turn starts, content - // length can shrink back to 0/1; reset scan index to avoid skipping. - if (lastProcessedContentIndex >= contentBlocks.length) { + // Some adapters (notably claude-code) reuse a single assistant + // lifecycle while internally spanning multiple provider sub-turns. + // When a new sub-turn starts, content[] length shrinks back to 0/1. + // The scan loop needs its index reset, AND the segment walker's + // renderedSegments map must be cleared so existing text-run + // components don't get overwritten in place with new sub-turn + // content (#4144 regression). Prior sub-turn children stay in + // chatContainer as frozen history; new segments append after them. + if (contentBlocks.length < lastContentLength) { + renderedSegments = []; + lastPinnedText = ""; + lastProcessedContentIndex = 0; + } else if (lastProcessedContentIndex >= contentBlocks.length) { lastProcessedContentIndex = 0; } + lastContentLength = contentBlocks.length; for (let i = lastProcessedContentIndex; i < contentBlocks.length; i++) { const content = contentBlocks[i]; if (content.type === "toolCall") { @@ -474,6 +490,7 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & { host.streamingComponent = undefined; host.streamingMessage = undefined; renderedSegments = []; + lastContentLength = 0; // Clear pinned output once the message is finalized in the chat // container — prevents duplicate display when the agent continues // (e.g. form elicitation) after the assistant message ends. From 2bf23133952d5083d59f9788ac75125b84ab4024 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Mon, 13 Apr 2026 20:36:52 -0500 Subject: [PATCH 2/2] test(tui): finalize sub-turn regression tests to stop pinned spinner The two new sub-turn shrink regression tests created a pinned DynamicBorder (via message_update with pinnable text + tool) but never emitted message_end, so the spinner's setInterval kept the test process alive until CI timed out after 15 minutes. Append a message_end to each test so the module-level pinnedBorder is torn down. --- .../src/core/chat-controller-ordering.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts b/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts index c03db913d..ae24d455d 100644 --- a/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts +++ b/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts @@ -758,6 +758,9 @@ test("chat-controller freezes prior sub-turn and appends new segments when conte assert.equal(host.chatContainer.children.length, 5, "new tool appends after new text-run"); assert.equal(host.chatContainer.children[4]?.constructor?.name, "ToolExecutionComponent"); assert.notEqual(host.chatContainer.children[4], priorT1, "new T2 must be a different component from prior T1"); + + // Finalize so the module-level pinned spinner (setInterval) is torn down and the test process can exit. + await handleAgentEvent(host, { type: "message_end", message: makeAssistant([{ type: "text", text: "C" }, t2]) } as any); }); // Regression: after a sub-turn shrink, lastPinnedText must be cleared so the @@ -806,4 +809,7 @@ test("chat-controller updates pinned zone after sub-turn shrink", async () => { // Pinned markdown must now reflect the new sub-turn's text, not stay frozen on "first". assert.equal((pinnedMarkdown as any)?.text, "second", "pinned zone must update after sub-turn shrink (#4144 regression)"); + + // Finalize so the module-level pinned spinner (setInterval) is torn down and the test process can exit. + await handleAgentEvent(host, { type: "message_end", message: makeAssistant([{ type: "text", text: "second" }, t2]) } as any); });