From a8a0cc8a40fdfef403b727c554439283228f51fa Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 22 Jun 2026 21:43:19 +0200 Subject: [PATCH] fix(coding-agent): prevented streaming output duplication in scrollback - Clamped tool output preview height to the available viewport rows to stop redundant banner commits. - Added `outputBlockContentWidth` helper to accurately measure visual lines for scrollback budget calculations. - Updated `bash` and `eval-render` output wrapping to account for block padding and inner content width. - Added regression test confirming streaming tool output maintains a stable line count without duplicating headers. --- packages/coding-agent/CHANGELOG.md | 3 + packages/coding-agent/src/tools/bash.ts | 11 +- .../coding-agent/src/tools/eval-render.ts | 42 +++-- packages/coding-agent/src/tui/output-block.ts | 11 ++ .../test/streaming-output-scrollback.test.ts | 165 ++++++++++++++++++ 5 files changed, 217 insertions(+), 15 deletions(-) create mode 100644 packages/coding-agent/test/streaming-output-scrollback.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c52eaf0ee..6674d348f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,8 +1,11 @@ # Changelog ## [Unreleased] + ### Fixed +- Fixed streaming output blocks incorrectly calculating preview height, preventing flickering banners +- Fixed streaming `bash`/`eval` tool output duplicating its `… (N earlier lines, showing 10 of M) (ctrl+o to expand)` preview into native scrollback. The collapsed output is a sliding tail window fixed at 10 lines, so when the box outgrew the live viewport (a tall command/output under a still-live predecessor such as a parallel tool) its mutating tail scrolled above the commit window and the renderer re-committed a fresh snapshot every frame, stacking dozens of stale preview banners and chunks. The output preview is now clamped to the viewport tail (`Math.min(10, previewWindowRows())`) and measured in visual rows at the box's inner content width (via the new `outputBlockContentWidth` helper), so on short terminals the volatile tail shrinks to stay on-screen and is never committed. Fixes the duplication introduced when scroll-off commits were made loss-free. - Prevented `/handoff` from executing while a response is streaming to avoid session corruption - Fixed `/handoff` cold-missing the provider prompt cache. Handoff generation now builds its request through the same pipeline a live turn uses (`convertMessagesToLlm` + `Agent.buildSideRequestContext` + `prepareSimpleStreamOptions`, via the new `generateHandoffFromContext`), so it reuses the live system prompt, normalized tools, transformed/obfuscated message history, and — critically — a stable `promptCacheKey` with a unique side `sessionId`. Previously the oneshot sent no cache-routing key and skipped the `transformContext`/`transformProviderContext` and tool/message normalization the loop applies, so its prefix never matched what the turn populated and every handoff re-read the whole context uncached. Mirrors the cache-preserving path already used by `/btw` and `/omfg`. - Fixed `/handoff` (and the RPC `handoff` command) resetting the agent while a response was still streaming, which let the live turn keep emitting into the torn-down session. Manual handoff now refuses while a prompt is in flight (matching `/fork` and `/move`); the auto-handoff path is unaffected. diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 5605de27e..b67ebb6cf 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -19,7 +19,7 @@ import bashDescription from "../prompts/tools/bash.md" with { type: "text" }; import type { ClientBridgeTerminalExitStatus, ClientBridgeTerminalOutput } from "../session/client-bridge"; import { DEFAULT_MAX_BYTES, enforceInlineByteCap, streamTailUpdates, TailBuffer } from "../session/streaming-output"; import { renderStatusLine } from "../tui"; -import { CachedOutputBlock, markFramedBlockComponent } from "../tui/output-block"; +import { CachedOutputBlock, markFramedBlockComponent, outputBlockContentWidth } from "../tui/output-block"; import { getSixelLineMask } from "../utils/sixel"; import type { ToolSession } from "."; import { truncateForPrompt } from "./approval"; @@ -1334,7 +1334,14 @@ export function createShellRenderer(config: ShellRendererConfig) { .map(line => uiTheme.fg("toolOutput", replaceTabs(line))) .join("\n"); const textContent = styledOutput; - const result = truncateToVisualLines(textContent, previewLines, width); + // Cap the collapsed/streaming output to a viewport-sized tail and + // measure it at the box's INNER width. Otherwise a growing tail + // window scrolls its (mutating) rows above the live-region window + // and the engine re-commits a fresh snapshot every frame — + // spraying duplicate "… ctrl+o to expand" banners into native + // scrollback (the box never overflows the viewport now). + const previewBudget = Math.min(previewLines, previewWindow); + const result = truncateToVisualLines(textContent, previewBudget, outputBlockContentWidth(width)); if (result.skippedCount > 0) { outputLines.push( uiTheme.fg( diff --git a/packages/coding-agent/src/tools/eval-render.ts b/packages/coding-agent/src/tools/eval-render.ts index 293b2cd7f..ae0341fcc 100644 --- a/packages/coding-agent/src/tools/eval-render.ts +++ b/packages/coding-agent/src/tools/eval-render.ts @@ -18,7 +18,7 @@ import type { RenderResultOptions } from "../extensibility/custom-tools/types"; import { formatContextUsage } from "../modes/components/status-line/context-thresholds"; import { truncateToVisualLines } from "../modes/components/visual-truncate"; import { getMarkdownTheme, type Theme } from "../modes/theme/theme"; -import { markFramedBlockComponent, renderCodeCell } from "../tui"; +import { markFramedBlockComponent, outputBlockContentWidth, renderCodeCell } from "../tui"; import { JSON_TREE_MAX_DEPTH_COLLAPSED, JSON_TREE_MAX_DEPTH_EXPANDED, @@ -468,23 +468,33 @@ function formatCellOutputLines( return { lines: [], hiddenCount: 0 }; } + // Cell output lands in renderCodeCell → renderOutputBlock, which re-wraps it + // at the box's inner content width. Bound the collapsed tail by VISUAL rows + // at that width so a long-line tail can't wrap into more rows than budgeted + // and scroll its mutating preview above the live-region window — the + // duplicate "ctrl+o to expand" scrollback spray. + const innerWidth = outputBlockContentWidth(width); + if (cell.hasMarkdown && cell.status !== "error") { const md = new Markdown(cell.output, 0, 0, getMarkdownTheme()); - const allLines = md.render(width); + const allLines = md.render(innerWidth); const displayLines = expanded ? allLines : allLines.slice(-previewLines); const hiddenCount = allLines.length - displayLines.length; return { lines: displayLines, hiddenCount }; } - const rawLines = cell.output.split("\n"); - const displayLines = expanded ? rawLines : rawLines.slice(-previewLines); - const hiddenCount = rawLines.length - displayLines.length; - const outputLines = displayLines.map(line => { - const cleaned = replaceTabs(line); - return cell.status === "error" ? theme.fg("error", cleaned) : theme.fg("toolOutput", cleaned); - }); - - return { lines: outputLines, hiddenCount }; + const styledOutput = cell.output + .split("\n") + .map(line => { + const cleaned = replaceTabs(line); + return cell.status === "error" ? theme.fg("error", cleaned) : theme.fg("toolOutput", cleaned); + }) + .join("\n"); + if (expanded) { + return { lines: styledOutput.split("\n"), hiddenCount: 0 }; + } + const { visualLines, skippedCount } = truncateToVisualLines(styledOutput, previewLines, innerWidth); + return { lines: visualLines, hiddenCount: skippedCount }; } export const evalToolRenderer = { @@ -586,7 +596,10 @@ export const evalToolRenderer = { return markFramedBlockComponent({ render: (width: number): readonly string[] => { const expanded = options.renderContext?.expanded ?? options.expanded; - const previewLines = options.renderContext?.previewLines ?? EVAL_DEFAULT_PREVIEW_LINES; + const previewLines = Math.min( + options.renderContext?.previewLines ?? EVAL_DEFAULT_PREVIEW_LINES, + previewWindowRows(), + ); const key = `${expanded}|${previewLines}|${options.spinnerFrame}|${previewWindowRows()}`; if (cached && cached.key === key && cached.width === width) { return cached.result; @@ -717,7 +730,10 @@ export const evalToolRenderer = { return { render: (width: number): readonly string[] => { - const previewLines = options.renderContext?.previewLines ?? EVAL_DEFAULT_PREVIEW_LINES; + const previewLines = Math.min( + options.renderContext?.previewLines ?? EVAL_DEFAULT_PREVIEW_LINES, + previewWindowRows(), + ); if (cachedLines === undefined || cachedWidth !== width || cachedPreviewLines !== previewLines) { const result = truncateToVisualLines(textContent, previewLines, width); cachedLines = result.visualLines; diff --git a/packages/coding-agent/src/tui/output-block.ts b/packages/coding-agent/src/tui/output-block.ts index 970a96f14..3d5ad0851 100644 --- a/packages/coding-agent/src/tui/output-block.ts +++ b/packages/coding-agent/src/tui/output-block.ts @@ -46,6 +46,17 @@ function normalizeContentPaddingLeft(value: number | undefined): number { return Math.max(0, Math.floor(value)); } +/** + * Inner content width that {@link renderOutputBlock} wraps its body to, for a + * given outer `width`: both vertical borders (1 cell each) plus the left + * content padding. Renderers that size a tail window MUST budget visual rows + * against this, not the outer width — otherwise the block re-wraps their lines + * into more rows than they counted and the box overflows its intended height. + */ +export function outputBlockContentWidth(width: number, contentPaddingLeft?: number): number { + return Math.max(1, width - 2 - normalizeContentPaddingLeft(contentPaddingLeft)); +} + export function renderOutputBlock(options: OutputBlockOptions, theme: Theme): string[] { const { header, headerMeta, state, sections = [], width, applyBg = true } = options; const h = theme.boxRound.horizontal; diff --git a/packages/coding-agent/test/streaming-output-scrollback.test.ts b/packages/coding-agent/test/streaming-output-scrollback.test.ts new file mode 100644 index 000000000..6415655ce --- /dev/null +++ b/packages/coding-agent/test/streaming-output-scrollback.test.ts @@ -0,0 +1,165 @@ +import { afterEach, beforeAll, describe, expect, test } from "bun:test"; +import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; +import { TranscriptContainer } from "@oh-my-pi/pi-coding-agent/modes/components/transcript-container"; +import { theme as activeTheme, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { evalToolRenderer } from "@oh-my-pi/pi-coding-agent/tools/eval-render"; +import { previewWindowRows } from "@oh-my-pi/pi-coding-agent/tools/render-utils"; +import { type Component, TUI } from "@oh-my-pi/pi-tui"; +import { VirtualTerminal } from "../../tui/test/virtual-terminal"; + +// Long, path-like output that wraps at the box's inner width — the case that +// made a fixed 10-line preview overflow the viewport once committed. +function longLines(count: number): string { + return Array.from( + { length: count }, + (_, i) => `out-line-${i} ${"=".repeat(60)} https://example.com/very/long/path/segment/${i}`, + ).join("\n"); +} + +type DrainableScheduler = { + now(): number; + scheduleImmediate(cb: () => void): void; + scheduleRender(cb: () => void, delayMs: number): { cancel(): void }; + flush(): void; +}; +function makeDrainableScheduler(): DrainableScheduler { + let clock = 0; + const queue: Array<{ run: () => void; cancelled: boolean }> = []; + const enqueue = (cb: () => void) => { + const item = { run: cb, cancelled: false }; + queue.push(item); + return item; + }; + return { + now: () => clock, + scheduleImmediate(cb) { + enqueue(cb); + }, + scheduleRender(cb) { + const item = enqueue(cb); + return { + cancel() { + item.cancelled = true; + }, + }; + }, + flush() { + let guard = 0; + while (queue.length > 0) { + if (++guard > 100_000) throw new Error("scheduler did not settle"); + const item = queue.shift()!; + clock += 1; + if (!item.cancelled) item.run(); + } + }, + }; +} + +// Plain Component → finalized by default: a settled block above the live region. +class StaticBlock implements Component { + #lines: string[]; + constructor(lines: string[]) { + this.#lines = lines; + } + invalidate(): void {} + render(_width: number): string[] { + return this.#lines; + } +} + +// 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. +class LiveBarrier extends StaticBlock { + isTranscriptBlockFinalized(): boolean { + return false; + } + isTranscriptBlockCommitStable(): boolean { + return true; + } +} + +// Stand-in for the input editor + status drawn below the transcript. +class Footer implements Component { + #rows: number; + constructor(rows: number) { + this.#rows = rows; + } + invalidate(): void {} + render(_width: number): string[] { + return Array.from({ length: this.#rows }, (_, i) => `editor-${i}`); + } +} + +const ORIGINAL_ROWS = Object.getOwnPropertyDescriptor(process.stdout, "rows"); +function stubStdoutRows(rows: number): void { + Object.defineProperty(process.stdout, "rows", { configurable: true, value: rows }); +} + +describe("streaming tool output never sprays duplicate scrollback banners", () => { + beforeAll(async () => { + await initTheme(); + }); + afterEach(() => { + if (ORIGINAL_ROWS) Object.defineProperty(process.stdout, "rows", ORIGINAL_ROWS); + else Reflect.deleteProperty(process.stdout, "rows"); + }); + + test("bash: growing partial output under a live predecessor does not duplicate banners", async () => { + if (process.platform === "win32") return; + const rows = 14; + stubStdoutRows(rows); + const term = new VirtualTerminal(80, rows); + const scheduler = makeDrainableScheduler(); + const tui = new TUI(term, undefined, { renderScheduler: scheduler }); + const transcript = new TranscriptContainer(); + transcript.addChild(new StaticBlock(["user: run the build"])); + transcript.addChild(new LiveBarrier(["assistant: still working in a parallel tool…"])); + const bash = new ToolExecutionComponent("bash", { command: "build.sh" }, {}, undefined, tui, process.cwd()); + transcript.addChild(bash); + tui.addChild(transcript); + tui.addChild(new Footer(6)); + + try { + tui.start(); + scheduler.flush(); + await term.flush(); + for (let n = 1; n <= 40; n++) { + bash.updateResult({ content: [{ type: "text", text: longLines(n) }], isError: false }, true); + term.scrollLines(1000); + tui.requestRender(); + scheduler.flush(); + await term.flush(); + } + const buffer = term.getScrollBuffer().map(row => Bun.stripANSI(row).trimEnd()); + const banners = buffer.filter(row => row.includes("ctrl+o")).length; + // Pre-fix this re-committed a fresh snapshot per streamed frame (~30+). + expect(banners).toBeLessThanOrEqual(1); + } finally { + bash.stopAnimation(); + tui.stop(); + await term.flush(); + } + }, 30_000); + + test("eval: collapsed cell output stays within the viewport budget", () => { + const rows = 18; + stubStdoutRows(rows); + const result = { + content: [{ type: "text", text: "" }], + details: { + cells: [ + { index: 0, code: "run()", language: "js" as const, output: longLines(60), status: "running" as const }, + ], + }, + isError: false, + }; + const component = evalToolRenderer.renderResult(result, { expanded: false, isPartial: true }, activeTheme); + const lines = component.render(80); + // The collapsed cell box fits the viewport budget: code + output tails are + // each capped at previewWindowRows() VISUAL rows. Pre-fix the long output + // wrapped into ~2x its line count and blew past this. + expect(lines.length).toBeLessThanOrEqual(previewWindowRows() + 10); + expect(lines.map(line => Bun.stripANSI(line)).join("\n")).toContain("ctrl+o"); + }); +});