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.
This commit is contained in:
can1357
@@ -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.
|
||||
|
||||
@@ -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<TArgs>(config: ShellRendererConfig<TArgs>) {
|
||||
.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(
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user