diff --git a/packages/coding-agent/src/edit/renderer.ts b/packages/coding-agent/src/edit/renderer.ts index f6c914cbf..cf959c001 100644 --- a/packages/coding-agent/src/edit/renderer.ts +++ b/packages/coding-agent/src/edit/renderer.ts @@ -696,11 +696,6 @@ function wrapEditRendererLine(line: string, width: number): string[] { export const editToolRenderer = { mergeCallAndResult: true, - // Pending preview is a TAIL window of the streamed diff ("… N more lines - // above" + last rows); the result render re-anchors the block top-first, so - // committing the preview's settled head would strand a stale call-box - // fragment in native scrollback. - provisionalPendingPreview: true, renderCall( args: EditRenderArgs, diff --git a/packages/coding-agent/src/modes/components/bash-execution.ts b/packages/coding-agent/src/modes/components/bash-execution.ts index 2d5ac236a..b17300cb2 100644 --- a/packages/coding-agent/src/modes/components/bash-execution.ts +++ b/packages/coding-agent/src/modes/components/bash-execution.ts @@ -64,6 +64,15 @@ export class BashExecutionComponent extends Container { this.#contentContainer.addChild(this.#loader); } + /** + * Transcript finalization contract (see `FinalizableBlock`): the collapsed + * streaming preview rewrites its tail window every chunk, so the block must + * stay out of native scrollback until the command completes. + */ + isTranscriptBlockFinalized(): boolean { + return this.#status !== "running"; + } + /** * Set whether the output is expanded (shows full output) or collapsed (preview only). */ diff --git a/packages/coding-agent/src/modes/components/eval-execution.ts b/packages/coding-agent/src/modes/components/eval-execution.ts index 5b82dfe17..fd6084a9e 100644 --- a/packages/coding-agent/src/modes/components/eval-execution.ts +++ b/packages/coding-agent/src/modes/components/eval-execution.ts @@ -61,6 +61,15 @@ export class EvalExecutionComponent extends Container { this.#contentContainer.addChild(this.#loader); } + /** + * Transcript finalization contract (see `FinalizableBlock`): the collapsed + * streaming preview rewrites its tail window every chunk, so the block must + * stay out of native scrollback until the cell completes. + */ + isTranscriptBlockFinalized(): boolean { + return this.#status !== "running"; + } + setExpanded(expanded: boolean): void { this.#expanded = expanded; this.#updateDisplay(); diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index 3b5c4b699..09b5ad79e 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -697,12 +697,12 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac /** * Standalone harnesses may mount a tool component directly under `TUI` * instead of inside `TranscriptContainer`. In that shape the component must - * report its own live-region seam for provisional previews, or the core - * renderer treats it like shell output and commits tail-window edit/eval/bash - * previews to immutable native scrollback before the result replaces them. + * report its own live-region seam while unfinalized, or the core renderer + * treats it like shell output and commits still-mutating preview rows to + * immutable native scrollback before the result replaces them. */ getNativeScrollbackLiveRegionStart(): number | undefined { - return !this.isTranscriptBlockFinalized() && !this.isTranscriptBlockCommitStable() ? 0 : undefined; + return this.isTranscriptBlockFinalized() ? undefined : 0; } /** @@ -726,52 +726,6 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac return (this.#result.details as { async?: { state?: string } } | undefined)?.async?.state === "running"; } - /** - * Whether this still-live block's settled rows may enter native scrollback - * (see `FinalizableBlock.isTranscriptBlockCommitStable`). Renderers classify - * pending views by durability instead of by tool name: a provisional view is - * allowed to be useful on screen, but finalization may replace or re-anchor - * it wholesale, so committing any of its rows would strand stale preview - * bytes in immutable scrollback. Non-provisional views stream rows whose - * committed prefix survives the remaining transitions. - */ - isTranscriptBlockCommitStable(): boolean { - if (this.#displaceableByToolName) return false; - if (this.isTranscriptBlockFinalized()) return true; - // `provisionalPendingPreview` describes only the PENDING call preview - // (`renderCall`, before any result): the result render may re-anchor it - // wholesale, so its rows must never commit. Once a (streaming partial) - // result exists the result renderer is usually the live shape — its body - // is top-anchored and grows append-only, and `deriveLiveCommitState` - // gates per-row durability — so the block is commit-stable like any - // settled stream. Gating the flag on the pending phase is what keeps a - // collapsed streaming eval/bash/ssh whose box outgrows the viewport from - // stranding its head: while commit-unstable its scrolled-off top - // committed nowhere and repainted nowhere, so it read as truncated until - // ctrl+o (expanded) flipped it stable. - // - // Renderers whose partial-result chrome (header glyph, frame state) - // differs from the final result render set `provisionalPartialResult` - // to opt out of stream-commit while `isPartial` holds: the ratchet - // would otherwise promote the stable partial chrome to native scrollback - // after `STABLE_PREFIX_COMMIT_FRAMES` and leave it stranded above the - // final frame once the chrome flips. Once the result settles - // (`isPartial === false`) the block is commit-stable again. - if (this.#result !== undefined) { - if (this.#isPartial) { - const tool = this.#tool as { provisionalPartialResult?: boolean } | undefined; - const provisionalPartialResult = - tool?.provisionalPartialResult ?? toolRenderers[this.#toolName]?.provisionalPartialResult; - if (provisionalPartialResult) return false; - } - return true; - } - const tool = this.#tool as { provisionalPendingPreview?: boolean | "collapsed" } | undefined; - const provisionalPendingPreview = - tool?.provisionalPendingPreview ?? toolRenderers[this.#toolName]?.provisionalPendingPreview; - return provisionalPendingPreview !== true && (provisionalPendingPreview !== "collapsed" || this.#expanded); - } - /** * Mark the tool terminal even though no result arrived (the turn aborted or * abandoned it) and stop animating, so it can freeze and stops pinning the diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 291206920..a349c0a34 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -1409,10 +1409,6 @@ export function createShellRenderer(config: ShellRendererConfig) { }, mergeCallAndResult: true, inline: true, - // Collapsed pending preview caps the command to a viewport-sized tail - // window that shifts while args stream. Expanded output is top-anchored - // enough for the transcript to commit its settled prefix. - provisionalPendingPreview: "collapsed", }; } diff --git a/packages/coding-agent/src/tools/eval-render.ts b/packages/coding-agent/src/tools/eval-render.ts index bcae5bac3..5e7f73090 100644 --- a/packages/coding-agent/src/tools/eval-render.ts +++ b/packages/coding-agent/src/tools/eval-render.ts @@ -773,24 +773,4 @@ export const evalToolRenderer = { mergeCallAndResult: true, inline: true, - // Collapsed pending preview shows tail-window code cells; the result render - // interleaves each cell's output under its code, re-laying-out every row - // below the first cell. Expanded output is top-anchored enough for the - // transcript to commit its settled prefix. - provisionalPendingPreview: "collapsed", - // Partial-result chrome is NOT byte-stable: `renderAgentProgressEvents` - // inserts/removes each subagent's current-tool line as it starts/stops a - // tool, and ticks status icon/stats/duration on already-rendered rows, - // while `options.isPartial` holds for the whole eval() cell (agent - // progress ticks never carry an `async` completed/failed state, so - // `event-controller.ts` keeps `isPartial: true` throughout). If this - // block were commit-stable during that churn, `deriveLiveCommitState`'s - // stable-prefix ratchet would promote agent rows that keep mutating (a - // "slow ticker", see transcript-container.ts) into native scrollback, - // and the tui resync's "duplication, never loss" contract would - // repeatedly re-show the frame tail under a heavy concurrent - // `agent()`/`parallel()` fan-out — the overlapping/duplicated tree rows - // seen when many subagents run at once. Once the cell settles - // (`isPartial === false`) the block is commit-stable again. - provisionalPartialResult: true, }; diff --git a/packages/coding-agent/src/tools/renderers.ts b/packages/coding-agent/src/tools/renderers.ts index 6234e7566..bc5043f33 100644 --- a/packages/coding-agent/src/tools/renderers.ts +++ b/packages/coding-agent/src/tools/renderers.ts @@ -43,26 +43,6 @@ export type ToolRenderer = { mergeCallAndResult?: boolean; /** Render without background box, inline in the response flow */ inline?: boolean; - /** - * Whether pending-call rows are provisional: useful on screen while a tool is - * streaming, but not durable transcript history. `true` means every pending - * shape is provisional. `"collapsed"` means only the collapsed pending shape - * is provisional; expanded rendering is top-anchored/append-shaped enough to - * let the transcript commit its settled prefix. Absent = the pending preview - * streams rows the result render preserves. - */ - provisionalPendingPreview?: boolean | "collapsed"; - /** - * Whether the partial-result render is provisional: chrome rows (header - * glyph, frame state) that change between `options.isPartial === true` and - * the final result render. When `true`, the block is treated as - * commit-unstable while a partial result is in flight, so the - * stable-prefix ratchet in `deriveLiveCommitState` cannot promote the - * partial chrome to native scrollback only to have the final render strand - * it above the settled frame. Absent = the partial render is byte-stable - * with the final render and may commit like any settled stream. - */ - provisionalPartialResult?: boolean; /** * Whether the renderer's pending-call path visibly consumes * `options.spinnerFrame`. Used to avoid scheduling repaint ticks for live diff --git a/packages/coding-agent/src/tools/ssh.ts b/packages/coding-agent/src/tools/ssh.ts index ca746fc30..a59f0da71 100644 --- a/packages/coding-agent/src/tools/ssh.ts +++ b/packages/coding-agent/src/tools/ssh.ts @@ -386,22 +386,6 @@ export const sshToolRenderer = { }); }, mergeCallAndResult: true, - // Pending call preview can re-anchor wholesale when the final result inserts - // the `Output` section, so no pending SSH rows may commit to native - // scrollback — even when expanded. The expanded pending shape was previously - // allowed to commit, which left two visible shapes in native scrollback once - // the result settled: a stale `⏳ SSH: [host]` header above the final frame, - // and the pending `╰──╯` footer reused in-place as the new `├── Output ──┤` - // separator with a fresh footer pushed below it. - provisionalPendingPreview: true, - // Partial-result chrome (pending icon and frame state) differs from the - // final SSH glyph/state, so the block stays commit-unstable while - // `options.isPartial` holds. Without this, a long-running SSH command's - // stable pending header would be promoted by the stable-prefix ratchet and - // committed to native scrollback, then the final render's SSH glyph would - // land below and strand a duplicate pending header above the final frame - // ([#3177](https://github.com/can1357/oh-my-pi/issues/3177)). - provisionalPartialResult: true, // Streamed args can initially render the SSH placeholder (`⏳ SSH: […]` / // `$ …`), then the first partial result inserts the `Output` section and // re-anchors the frame. Force a full repaint at that seam so placeholder rows diff --git a/packages/coding-agent/test/modes/components/assistant-message-mermaid.test.ts b/packages/coding-agent/test/modes/components/assistant-message-mermaid.test.ts index 95e65d7f5..6a5c1b27d 100644 --- a/packages/coding-agent/test/modes/components/assistant-message-mermaid.test.ts +++ b/packages/coding-agent/test/modes/components/assistant-message-mermaid.test.ts @@ -98,87 +98,44 @@ describe("AssistantMessageComponent mermaid markdown", () => { }); }); -describe("AssistantMessageComponent reflowing-markdown commit stability", () => { - // A streaming reply is built empty then fed via updateContent (the live path); - // passing a message to the constructor would mark it finalized. - it("is commit-unstable while a streaming reply still carries a mermaid fence", () => { +describe("AssistantMessageComponent settled-row commit boundary", () => { + function renderStreamingMarkdown(markdown: string): AssistantMessageComponent { const component = new AssistantMessageComponent(); - component.updateContent(createAssistantMessage("Here is the flow:\n\n```mermaid\nflowchart TD\n A-->B")); - expect(component.isTranscriptBlockCommitStable()).toBe(false); - }); + component.updateContent(createAssistantMessage(markdown), { transient: true }); + component.render(80); + return component; + } - it("becomes commit-stable once the mermaid reply finalizes", () => { - const component = new AssistantMessageComponent(); - component.updateContent(createAssistantMessage("```mermaid\nflowchart TD\n A-->B\n```")); - expect(component.isTranscriptBlockCommitStable()).toBe(false); - component.markTranscriptBlockFinalized(); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("stays commit-stable for a streaming reply without a mermaid fence", () => { - const component = new AssistantMessageComponent(); - component.updateContent(createAssistantMessage("A long normal reply.\n\n- one\n- two\n\nMore prose follows.")); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("does not trip on prose that mentions a mermaid fence inline", () => { - const component = new AssistantMessageComponent(); - component.updateContent(createAssistantMessage("Wrap the diagram in a ```mermaid block to render it.")); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("is commit-unstable for intro prose followed by a streaming mermaid tail", () => { - const component = new AssistantMessageComponent(); - component.updateContent( - createAssistantMessage("Intro prose above the diagram.\n\n```mermaid\nflowchart TD\n A-->B"), + it("exposes frozen paragraph rows for streaming prose", () => { + const component = renderStreamingMarkdown( + "First paragraph is already byte-stable.\n\nSecond paragraph is still streaming tokens", ); - expect(component.isTranscriptBlockCommitStable()).toBe(false); + + expect(component.getTranscriptBlockSettledRows()).toBeGreaterThan(0); }); - it("is commit-unstable while a streaming reply still renders a GFM table", () => { - const component = new AssistantMessageComponent(); - component.updateContent(createAssistantMessage("Results:\n\n| Name | Score |\n| --- | --- |\n| a | 1 |")); - expect(component.isTranscriptBlockCommitStable()).toBe(false); + it("exposes zero settled rows for reflowing markdown while streaming", () => { + for (const markdown of [ + "Here is the flow:\n\n```mermaid\nflowchart TD\n A-->B", + "```mermaid\nflowchart TD\n A-->B\n```", + "Results:\n\n| Name | Score |\n| --- | --- |\n| a | 1 |", + ]) { + const component = renderStreamingMarkdown(markdown); + + expect(component.getTranscriptBlockSettledRows()).toBe(0); + } }); - it("becomes commit-stable once a table reply finalizes", () => { - const component = new AssistantMessageComponent(); - component.updateContent(createAssistantMessage("| Name | Score |\n| --- | --- |\n| a | 1 |\n| b | 2 |")); - expect(component.isTranscriptBlockCommitStable()).toBe(false); - component.markTranscriptBlockFinalized(); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); + it("exposes zero settled rows after a reflowing block finalizes", () => { + for (const markdown of [ + "```mermaid\nflowchart TD\n A-->B\n```", + "| Name | Score |\n| --- | --- |\n| a | 1 |\n| b | 2 |", + ]) { + const component = renderStreamingMarkdown(markdown); + component.markTranscriptBlockFinalized(); - it("stays commit-stable for pipe-heavy prose with no table delimiter row", () => { - const component = new AssistantMessageComponent(); - component.updateContent( - createAssistantMessage("Weigh cost | benefit | risk before deciding, and note `a || b` short-circuits."), - ); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("stays commit-stable for a streaming table header before its delimiter row arrives", () => { - // Header alone is just a paragraph with pipes — Markdown lays out no table, - // and nothing re-flows, until the delimiter row streams in. - const component = new AssistantMessageComponent(); - component.updateContent(createAssistantMessage("| Name | Score |")); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("stays commit-stable for a fenced code block containing a table delimiter", () => { - // A shell snippet with pipes and dashes inside a code fence is literal text, - // not a reflowing table, so a long code-heavy reply still commits normally. - const component = new AssistantMessageComponent(); - component.updateContent(createAssistantMessage("Run it:\n\n```sh\necho '| --- | --- |'\ncat data | sort\n```")); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("stays commit-stable for a mermaid fence shown as example content inside a code block", () => { - const component = new AssistantMessageComponent(); - component.updateContent( - createAssistantMessage("Example:\n\n````md\n```mermaid\nflowchart TD\n A-->B\n```\n````"), - ); - expect(component.isTranscriptBlockCommitStable()).toBe(true); + expect(component.getTranscriptBlockSettledRows()).toBe(0); + } }); }); diff --git a/packages/coding-agent/test/modes/components/transcript-container.test.ts b/packages/coding-agent/test/modes/components/transcript-container.test.ts index 021bddbc3..6fb342557 100644 --- a/packages/coding-agent/test/modes/components/transcript-container.test.ts +++ b/packages/coding-agent/test/modes/components/transcript-container.test.ts @@ -53,12 +53,22 @@ class StreamingBlock implements Component { } } -// A still-live block whose render is provisional (a tool call's tail-window -// streaming preview): the result render replaces it wholesale, so its settled -// rows must never be offered for native-scrollback commit. -class ProvisionalStreamingBlock extends StreamingBlock { - isTranscriptBlockCommitStable(): boolean { - return false; +// A still-live block that can declare a byte-stable rendered prefix. The +// transcript container may commit only those declared rows before finalization. +class DeclaredSettledStreamingBlock extends StreamingBlock { + #settledRows: number; + + constructor(lines: string[], settledRows: number) { + super(lines); + this.#settledRows = settledRows; + } + + setSettledRows(rows: number): void { + this.#settledRows = rows; + } + + getTranscriptBlockSettledRows(): number { + return this.#settledRows; } } @@ -181,7 +191,7 @@ describe("TranscriptContainer", () => { expect(container.render(80)).toEqual(["a-reflowed", "", "b2"]); }); - it("reports the live block start that gates native scrollback commits", () => { + it("reports undefined as the native scrollback boundary when every block is finalized", () => { const container = new TranscriptContainer(); const a = new MutableBlock(["a1", "a2"]); const b = new MutableBlock(["b1"]); @@ -189,11 +199,11 @@ describe("TranscriptContainer", () => { container.addChild(b); expect(container.render(40)).toEqual(["a1", "a2", "", "b1"]); - expect(container.getNativeScrollbackLiveRegionStart()).toBe(3); + expect(container.getNativeScrollbackLiveRegionStart()).toBeUndefined(); b.set(["b1", "b2"]); expect(container.render(40)).toEqual(["a1", "a2", "", "b1", "b2"]); - expect(container.getNativeScrollbackLiveRegionStart()).toBe(3); + expect(container.getNativeScrollbackLiveRegionStart()).toBeUndefined(); }); it("keeps an unfinalized block below the seam when a finalized block is appended below it", () => { @@ -214,8 +224,8 @@ describe("TranscriptContainer", () => { // The tool's result lands after the card is already below it. tool.finalize(["✔ write: 4 lines"]); expect(container.render(40)).toEqual(["✔ write: 4 lines", "", "rule card"]); - // The seam moves past the now-finalized tool. - expect(container.getNativeScrollbackLiveRegionStart()).toBe(2); + // All blocks are finalized; the whole rendered frame is committable. + expect(container.getNativeScrollbackLiveRegionStart()).toBeUndefined(); // Even after finalizing, a late re-layout still repaints in the window. tool.set(["collapsed"]); @@ -251,9 +261,8 @@ describe("TranscriptContainer", () => { const rendered = plain(container.render(80)); expect(rendered).toContain("The config file write went through despite the interruption."); - expect(rendered).not.toContain(USER_INTERRUPT_LABEL); expect(rendered).toContain("Copied raw SSE stream"); - expect(container.getNativeScrollbackLiveRegionStart()).not.toBe(0); + expect(container.getNativeScrollbackLiveRegionStart()).toBeUndefined(); }); it("starts the live region at the earliest of several unfinalized blocks", () => { @@ -278,113 +287,66 @@ describe("TranscriptContainer", () => { // The pending block updates freely while live. pending.finalize(["pending-final"]); expect(container.render(40)).toEqual(["done-collapsed", "", "pending-final", "", "card"]); + expect(container.getNativeScrollbackLiveRegionStart()).toBeUndefined(); + }); + + it("stops the boundary at the first unfinalized block's first content row when no rows are settled", () => { + const container = new TranscriptContainer(); + container.addChild(new MutableBlock(["history"])); + const live = new StreamingBlock(["live-0", "live-1"]); + container.addChild(live); + container.addChild(new MutableBlock(["below"])); + + expect(container.render(40)).toEqual(["history", "", "live-0", "live-1", "", "below"]); + expect(container.getNativeScrollbackLiveRegionStart()).toBe(2); + + live.set(["live-0 updated", "live-1"]); + expect(container.render(40)).toEqual(["history", "", "live-0 updated", "live-1", "", "below"]); + expect(container.getNativeScrollbackLiveRegionStart()).toBe(2); + }); + + it("extends the boundary through declared settled rows after stripping leading blank padding", () => { + const container = new TranscriptContainer(); + container.addChild(new MutableBlock(["history"])); + const live = new DeclaredSettledStreamingBlock(["", "settled-a", "settled-b", "live-tail", ""], 3); + container.addChild(live); + + expect(container.render(40)).toEqual(["history", "", "settled-a", "settled-b", "live-tail"]); expect(container.getNativeScrollbackLiveRegionStart()).toBe(4); }); - it("never offers a commit-unstable live block's settled rows for native scrollback", () => { + it("returns undefined after the first unfinalized block finalizes", () => { const container = new TranscriptContainer(); container.addChild(new MutableBlock(["history"])); - // A pending collapsed tool preview: byte-static while the tool executes - // (the spinner stops once args complete), but replaced wholesale by the - // result render — committing any of it would strand a stale call-box - // fragment in terminal history above the final block. - const preview = new ProvisionalStreamingBlock([ - "┌ Edit: foo.ts", - "… (2 more hunks above)", - "-old-a", - "+new-a", - "-old-b", - "+new-b", - "└ (streaming)", - ]); - container.addChild(preview); + const live = new StreamingBlock(["live"]); + container.addChild(live); - // Far past STABLE_PREFIX_COMMIT_FRAMES: a durable block's settled head - // would have been promoted long ago. - for (let frame = 0; frame < 40; frame++) container.render(40); + expect(container.render(40)).toEqual(["history", "", "live"]); expect(container.getNativeScrollbackLiveRegionStart()).toBe(2); - expect(container.getNativeScrollbackCommitSafeEnd()).toBeUndefined(); - // The result render re-anchors the block top-first; nothing of the stale - // preview was committed, so nothing can be duplicated. Finalizing makes - // the full body commit-safe like any settled block. - preview.finalize(["✔ Edit: foo.ts (+2/-2)", "-old-a", "+new-a", "context"]); - container.render(40); - expect(container.getNativeScrollbackLiveRegionStart()).toBe(2); - expect(container.getNativeScrollbackCommitSafeEnd()).toBe(6); + live.finalize(["done"]); + expect(container.render(40)).toEqual(["history", "", "done"]); + expect(container.getNativeScrollbackLiveRegionStart()).toBeUndefined(); }); - it("still promotes a durable live block's settled head after the stability window", () => { + it("pins the boundary at an empty unfinalized block's row position", () => { const container = new TranscriptContainer(); container.addChild(new MutableBlock(["history"])); - // Default contract (no isTranscriptBlockCommitStable): settled leading - // rows are durable — a streaming assistant message, a top-anchored - // expanded tool stream — and promote once they sit visibly unchanged for - // the whole stability window, holding back only the volatile tail. - const streaming = new StreamingBlock(["head-0", "head-1", "head-2", "head-3", "head-4", "head-5", "tail"]); - container.addChild(streaming); + container.addChild(new StreamingBlock([])); + container.addChild(new MutableBlock(["below"])); - for (let frame = 0; frame < 40; frame++) container.render(40); + expect(container.render(40)).toEqual(["history", "", "below"]); + expect(container.getNativeScrollbackLiveRegionStart()).toBe(1); + }); + + it("does not let finalized blocks below the first unfinalized block extend the boundary", () => { + const container = new TranscriptContainer(); + container.addChild(new MutableBlock(["history"])); + container.addChild(new StreamingBlock(["live"])); + container.addChild(new StreamingBlock(["finalized-below-0", "finalized-below-1"], true)); + + expect(container.render(40)).toEqual(["history", "", "live", "", "finalized-below-0", "finalized-below-1"]); expect(container.getNativeScrollbackLiveRegionStart()).toBe(2); - // blockStart 2 + (7 rows - TAIL_VOLATILITY_ROWS holdback of 4) = 5. - expect(container.getNativeScrollbackCommitSafeEnd()).toBe(5); - }); - - it("withholds the durable snapshot commit for a streaming mermaid reply, then promotes it on finalize", () => { - // A fenced mermaid diagram re-lays-out its whole body every frame as the - // reply streams. If its scrolled-off rows were committed to immutable - // native scrollback (the durable-snapshot path) the later re-layout would - // strand a stale diagram fragment in history that only Ctrl+L clears, so - // the live mermaid reply must advertise no durable snapshot end. - const container = new TranscriptContainer(); - container.addChild(new StreamingBlock(["earlier turn"], true)); - const assistant = new AssistantMessageComponent(); - assistant.updateContent( - makeAssistantMessage({ content: [{ type: "text", text: "```mermaid\nflowchart TD\n A-->B\n```" }] }), - ); - container.addChild(assistant); - - for (let frame = 0; frame < 4; frame++) container.render(60); - expect(container.getNativeScrollbackSnapshotSafeEnd()).toBeUndefined(); - - // Finalized: the final layout is permanent and commits like any block. - assistant.markTranscriptBlockFinalized(); - container.render(60); - expect(container.getNativeScrollbackSnapshotSafeEnd()).toBeGreaterThan(0); - }); - - it("withholds the durable snapshot commit for a streaming GFM table, then promotes it on finalize", () => { - // A streaming table re-aligns its columns as rows arrive; its scrolled-off - // rows must not commit an intermediate alignment to immutable scrollback. - const container = new TranscriptContainer(); - container.addChild(new StreamingBlock(["earlier turn"], true)); - const assistant = new AssistantMessageComponent(); - assistant.updateContent( - makeAssistantMessage({ content: [{ type: "text", text: "| Name | Score |\n| --- | --- |\n| a | 1 |" }] }), - ); - container.addChild(assistant); - - for (let frame = 0; frame < 4; frame++) container.render(60); - expect(container.getNativeScrollbackSnapshotSafeEnd()).toBeUndefined(); - - assistant.markTranscriptBlockFinalized(); - container.render(60); - expect(container.getNativeScrollbackSnapshotSafeEnd()).toBeGreaterThan(0); - }); - - it("still commits the durable snapshot of a streaming reply without a mermaid fence", () => { - // Guard the narrow scope: an ordinary streaming reply stays commit-stable, - // so its settled rows still reach native scrollback while it streams. - const container = new TranscriptContainer(); - container.addChild(new StreamingBlock(["earlier turn"], true)); - const assistant = new AssistantMessageComponent(); - assistant.updateContent( - makeAssistantMessage({ content: [{ type: "text", text: "A normal streamed answer with **bold** text." }] }), - ); - container.addChild(assistant); - - for (let frame = 0; frame < 4; frame++) container.render(60); - expect(container.getNativeScrollbackSnapshotSafeEnd()).toBeGreaterThan(0); }); it("does not re-render finalized rows already committed to native scrollback", () => { const container = new TranscriptContainer(); diff --git a/packages/coding-agent/test/streaming-output-scrollback.test.ts b/packages/coding-agent/test/streaming-output-scrollback.test.ts index 6415655ce..a292086b6 100644 --- a/packages/coding-agent/test/streaming-output-scrollback.test.ts +++ b/packages/coding-agent/test/streaming-output-scrollback.test.ts @@ -67,16 +67,13 @@ class StaticBlock implements Component { } } -// A still-live predecessor (e.g. a parallel tool that is still running): being -// non-finalized closes the transcript's commit-safe run, so the streaming tool -// below it commits as forced-overflow — the path that sprayed. +// A still-live predecessor (e.g. a parallel tool that is still running) pins the +// transcript commit boundary, so rows below it stay repaintable until the +// predecessor finalizes. class LiveBarrier extends StaticBlock { isTranscriptBlockFinalized(): boolean { return false; } - isTranscriptBlockCommitStable(): boolean { - return true; - } } // Stand-in for the input editor + status drawn below the transcript. diff --git a/packages/coding-agent/test/tools/eval-commit-stability.test.ts b/packages/coding-agent/test/tools/eval-commit-stability.test.ts index d9002a4e4..33d7caef0 100644 --- a/packages/coding-agent/test/tools/eval-commit-stability.test.ts +++ b/packages/coding-agent/test/tools/eval-commit-stability.test.ts @@ -1,27 +1,4 @@ -/** - * Regression: under heavy concurrent `agent()`/`parallel()` fan-out inside an - * eval cell, `renderAgentProgressEvents` mutates on almost every progress tick - * — a per-subagent "current tool" line is inserted/removed as each subagent - * starts/stops a tool call, and status icon/stats/duration tick on already- - * rendered rows — while `options.isPartial` holds `true` for the WHOLE eval - * cell (agent progress ticks never carry an `async` completed/failed state, so - * `event-controller.ts`'s `#handleToolExecutionUpdate` keeps passing - * `isPartial: true` to `ToolExecutionComponent.updateResult` throughout). - * - * Without `evalToolRenderer.provisionalPartialResult: true`, - * `ToolExecutionComponent.isTranscriptBlockCommitStable()` reports the eval - * block as commit-stable while partial. That lets the transcript's stable-prefix - * ratchet (`deriveLiveCommitState`) promote agent-progress rows that keep - * mutating (a "slow ticker") into native scrollback, and `packages/tui`'s - * committed-prefix resync then repeatedly re-shows the frame tail under its - * "duplication, never loss" contract — producing the reported - * overlapping/duplicated status-tree rows. Contract: while a partial eval - * result is in flight the block reports commit-unstable so the ratchet keeps - * its rows in the live region; once the cell settles (`isPartial === false`) - * it is commit-stable again. The opt-in is renderer-scoped (matches the - * `sshToolRenderer` precedent for issue #3177). - */ -import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { beforeAll, describe, expect, it } from "bun:test"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import type { EvalStatusEvent, EvalToolDetails } from "@oh-my-pi/pi-coding-agent/eval/types"; import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; @@ -34,10 +11,6 @@ function makeEvalComponent() { return new ToolExecutionComponent("eval", { code: "parallel([...])", language: "python" }, {}, undefined, uiStub); } -function partialResult(text: string) { - return { content: [{ type: "text" as const, text }] }; -} - /** Build an eval result whose `details.cells` carry agent-fan-out progress. */ function evalAgentResult(events: EvalStatusEvent[], text = "") { const details: EvalToolDetails = { @@ -58,63 +31,50 @@ function evalAgentResult(events: EvalStatusEvent[], text = "") { return { content: [{ type: "text" as const, text }], details }; } -describe("eval tool block commit stability", () => { +function expectLive(component: ToolExecutionComponent): void { + expect(component.isTranscriptBlockFinalized()).toBe(false); + expect(component.getNativeScrollbackLiveRegionStart()).toBe(0); +} + +function expectFinal(component: ToolExecutionComponent): void { + expect(component.isTranscriptBlockFinalized()).toBe(true); + expect(component.getNativeScrollbackLiveRegionStart()).toBeUndefined(); +} + +describe("eval tool transcript finalization", () => { beforeAll(async () => { resetSettingsForTest(); await Settings.init({ inMemory: true }); await initTheme(); }); - afterEach(() => { - vi.restoreAllMocks(); - }); - - it("reports commit-unstable while an eval result is partial", () => { + it("keeps partial eval results in the native-scrollback live region", () => { const component = makeEvalComponent(); + component.updateResult(evalAgentResult([{ op: "agent", id: "a1", status: "running" }]), true); - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(false); + expectLive(component); }); - it("flips commit-stable as soon as the eval result settles", () => { + it("moves the block out of the live region as soon as the eval result settles", () => { const component = makeEvalComponent(); component.updateResult(evalAgentResult([{ op: "agent", id: "a1", status: "running" }]), true); - expect(component.isTranscriptBlockCommitStable()).toBe(false); + expectLive(component); - component.updateResult(partialResult("done\n"), false); - expect(component.isTranscriptBlockFinalized()).toBe(true); - expect(component.isTranscriptBlockCommitStable()).toBe(true); + component.updateResult({ content: [{ type: "text" as const, text: "done\n" }] }, false); + + expectFinal(component); }); - it("does not opt other foreground tools out of partial-result stream commits", () => { - // Sanity: bash and friends still get the existing `isPartial` - // commit-stable behaviour — the eval opt-in must be renderer-scoped. - const component = new ToolExecutionComponent("bash", { command: "ls" }, {}, undefined, uiStub); - component.updateResult(partialResult("a\nb\n"), true); - - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("stays commit-unstable across agent-progress churn while partial", () => { - // Defends the fix regardless of which specific row mutated: a subagent - // starting a tool inserts a `currentTool` line; stopping it removes one. - // Both shapes must read commit-unstable while `isPartial` holds, so the - // ratchet never promotes either into native scrollback mid-flight. + it("stays in the live region across agent-progress shape churn while partial", () => { const component = makeEvalComponent(); - // First tick: one running subagent with a current-tool line present. component.updateResult( evalAgentResult([{ op: "agent", id: "a1", status: "running", currentTool: "read" }]), true, ); - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(false); + expectLive(component); - // Second tick: the current-tool line is gone (subagent between tools), - // and a second subagent has joined — row count and per-row content both - // changed. Still partial, still commit-unstable. component.updateResult( evalAgentResult([ { op: "agent", id: "a1", status: "running" }, @@ -122,7 +82,7 @@ describe("eval tool block commit stability", () => { ]), true, ); - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(false); + + expectLive(component); }); }); diff --git a/packages/coding-agent/test/tools/ssh-commit-stability.test.ts b/packages/coding-agent/test/tools/ssh-commit-stability.test.ts index daefe9930..7519f9f13 100644 --- a/packages/coding-agent/test/tools/ssh-commit-stability.test.ts +++ b/packages/coding-agent/test/tools/ssh-commit-stability.test.ts @@ -1,17 +1,4 @@ -/** - * Issue #3177: `sshToolRenderer.renderResult` swaps the pending icon/frame - * state for the SSH glyph + success state when `options.isPartial` flips - * false. Without the renderer's `provisionalPartialResult: true` opt-out, a - * long-running SSH command keeps the same partial header bytes for the whole - * `STABLE_PREFIX_COMMIT_FRAMES` window, the transcript's stable-prefix - * ratchet promotes them to native scrollback, and the final render strands a - * pending `⏳ SSH: [host]` header above the final `⇄ SSH: [host]` header - * (the bug the user reported). Contract: while a partial SSH result is in - * flight, the block reports commit-unstable so `deriveLiveCommitState` keeps - * its rows in the live region; once the result settles it is commit-stable - * again. - */ -import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { beforeAll, describe, expect, it } from "bun:test"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; @@ -23,88 +10,52 @@ function makeSshComponent() { return new ToolExecutionComponent("ssh", { host: "sccpu", command: "uptime" }, {}, undefined, uiStub); } -function partialResult(text: string) { +function result(text: string) { return { content: [{ type: "text" as const, text }] }; } -describe("ssh tool block commit stability", () => { +function expectLive(component: ToolExecutionComponent): void { + expect(component.isTranscriptBlockFinalized()).toBe(false); + expect(component.getNativeScrollbackLiveRegionStart()).toBe(0); +} + +function expectFinal(component: ToolExecutionComponent): void { + expect(component.isTranscriptBlockFinalized()).toBe(true); + expect(component.getNativeScrollbackLiveRegionStart()).toBeUndefined(); +} + +describe("ssh tool transcript finalization", () => { beforeAll(async () => { resetSettingsForTest(); await Settings.init({ inMemory: true }); await initTheme(); }); - afterEach(() => { - vi.restoreAllMocks(); - }); - - it("reports commit-unstable while an SSH result is partial", () => { + it("keeps partial SSH results in the native-scrollback live region", () => { const component = makeSshComponent(); - component.updateResult(partialResult("connecting…"), true); - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(false); + component.updateResult(result("connecting…"), true); + + expectLive(component); }); - it("keeps the collapsed pending SSH preview commit-unstable until a result arrives", () => { - // Issue #3714: with `provisionalPendingPreview: true` the pending call - // preview is commit-unstable regardless of expansion, so neither the - // `⏳ SSH: [host]` header nor the framed bottom border can leak into - // native scrollback before the result render inserts `Output`. + it("keeps pending SSH previews in the live region before a result arrives", () => { + for (const expanded of [false, true]) { + const component = makeSshComponent(); + component.setExpanded(expanded); + component.setArgsComplete(); + + expectLive(component); + } + }); + + it("moves the block out of the live region as soon as the SSH result settles", () => { const component = makeSshComponent(); - component.setArgsComplete(); + component.updateResult(result("connecting…"), true); + expectLive(component); - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(false); - }); + component.updateResult(result("done\n"), false); - it("keeps the expanded pending SSH preview commit-unstable until a result arrives", () => { - // Issue #3714: the previous `"collapsed"` opt-out left expanded pending - // rows commit-stable. Once the box outgrew the viewport the stale - // `⏳ SSH: [host]` header and the pending `╰──╯` footer reached native - // scrollback, then the final result re-anchored the frame and stranded - // the pending rows above (header variant) or reused the footer row in - // place as `├── Output ──┤` (footer variant). Expanded MUST also be - // commit-unstable until the result render replaces the pending shape. - const component = makeSshComponent(); - component.setExpanded(true); - component.setArgsComplete(); - - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(false); - }); - - it("flips commit-stable as soon as the SSH result settles", () => { - const component = makeSshComponent(); - component.updateResult(partialResult("connecting…"), true); - expect(component.isTranscriptBlockCommitStable()).toBe(false); - - component.updateResult(partialResult("done\n"), false); - expect(component.isTranscriptBlockFinalized()).toBe(true); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("does not opt other foreground tools out of partial-result stream commits", () => { - // Sanity: bash and friends still get the existing `isPartial` - // commit-stable behaviour — the SSH opt-in must be renderer-scoped. - const component = new ToolExecutionComponent("bash", { command: "ls" }, {}, undefined, uiStub); - component.updateResult(partialResult("a\nb\n"), true); - - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(true); - }); - - it("does not opt other foreground tools out of expanded pending-preview commits", () => { - // Sanity: bash/eval still use `provisionalPendingPreview: "collapsed"`, - // so once expanded their pending preview is commit-stable. The SSH - // `true` opt-in MUST remain renderer-scoped — flipping the default - // here would block long top-anchored streams (e.g. a task call's - // context/assignment markdown) from reaching native scrollback. - const component = new ToolExecutionComponent("bash", { command: "ls" }, {}, undefined, uiStub); - component.setExpanded(true); - component.setArgsComplete(); - - expect(component.isTranscriptBlockFinalized()).toBe(false); - expect(component.isTranscriptBlockCommitStable()).toBe(true); + expectFinal(component); }); }); diff --git a/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts b/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts index 25532f3f2..943d5e0c9 100644 --- a/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts +++ b/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts @@ -4,8 +4,11 @@ import type { Component } from "@oh-my-pi/pi-tui"; class MutableLiveBlock implements Component { #lines: string[]; - constructor(lines: string[]) { + #settledRows: number; + + constructor(lines: string[], settledRows: number) { this.#lines = [...lines]; + this.#settledRows = settledRows; } render(width: number): string[] { return this.#lines.map(line => line.slice(0, width)); @@ -16,22 +19,26 @@ class MutableLiveBlock implements Component { isTranscriptBlockFinalized(): boolean { return false; } + getTranscriptBlockSettledRows(): number { + return this.#settledRows; + } } describe("transcript streaming commit (assistant text)", () => { - it("treats in-place growth of the trailing line as append-only", () => { + it("commits only the declared settled head while the trailing line grows", () => { const chat = new TranscriptContainer(); // Models a streaming assistant reply: stable head rows plus a current - // line that grows token-by-token without adding a new row. - const block = new MutableLiveBlock(["para one", "para two", "the quick brown"]); + // line that grows token-by-token without adding a new row. The head is + // committable only because the block explicitly declares those rows settled. + const block = new MutableLiveBlock(["para one", "para two", "the quick brown"], 2); chat.addChild(block); chat.render(80); + expect(chat.getNativeScrollbackLiveRegionStart()).toBe(2); block.setLines(["para one", "para two", "the quick brown fox"]); chat.render(80); - // The head rows never changed; only the trailing line grew. Its scrolled- - // off head must be committable to native scrollback (tmux pane history). - expect(chat.getNativeScrollbackCommitSafeEnd()).toBe(3); + + expect(chat.getNativeScrollbackLiveRegionStart()).toBe(2); }); });