diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index cb7fd45d6..8846e99e6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,10 +2,6 @@ ## [Unreleased] -### Changed - -- Changed `TranscriptContainer` to report its live-region boundary to the renderer (`NativeScrollbackLiveRegion`): on ED3-risk terminals it now exposes the line offset where the bottom-most live block begins, so the TUI pins that block and the chrome below it out of native scrollback during streaming instead of committing its overflow and then leaving stale duplicates when the block re-lays-out or collapses. - ## [15.9.1] - 2026-06-04 ### Added diff --git a/packages/coding-agent/src/modes/components/transcript-container.ts b/packages/coding-agent/src/modes/components/transcript-container.ts index c4fea9f7f..64af9ce42 100644 --- a/packages/coding-agent/src/modes/components/transcript-container.ts +++ b/packages/coding-agent/src/modes/components/transcript-container.ts @@ -1,4 +1,4 @@ -import { type Component, Container, type NativeScrollbackLiveRegion, TERMINAL } from "@oh-my-pi/pi-tui"; +import { type Component, Container, TERMINAL } from "@oh-my-pi/pi-tui"; const kSnapshot = Symbol("transcript.frozenRender"); @@ -34,14 +34,10 @@ interface SnapshotCarrier { * and any drift reconciles safely. On terminals that can rebuild history this * freezing is unnecessary, so it renders every block live for full fidelity. */ -export class TranscriptContainer extends Container implements NativeScrollbackLiveRegion { +export class TranscriptContainer extends Container { // Bumped to invalidate every block's snapshot at once; a snapshot is only // honored when its stored generation still matches. #generation = 0; - // Local line index where the current bottom-most block begins in the most - // recent render. TUI extends the native-scrollback pinned region from this - // point through the live block and the root chrome rendered below it. - #nativeScrollbackLiveRegionStart: number | undefined; // The block that was bottom-most (live) on the previous render. When the live // position moves past it, its snapshot was last refreshed mid-stream and may // predate content that finalized in the same coalesced frame that appended the @@ -60,10 +56,6 @@ export class TranscriptContainer extends Container implements NativeScrollbackLi super.clear(); } - getNativeScrollbackLiveRegionStart(): number | undefined { - return this.#nativeScrollbackLiveRegionStart; - } - /** * Retire all frozen snapshots so the next render reflects each block's current * state. Call at reconciliation checkpoints (prompt submit) where the whole @@ -76,7 +68,6 @@ export class TranscriptContainer extends Container implements NativeScrollbackLi override render(width: number): string[] { width = Math.max(1, width); - this.#nativeScrollbackLiveRegionStart = undefined; if (!TERMINAL.eagerEraseScrollbackRisk) return super.render(width); const lines: string[] = []; @@ -85,7 +76,6 @@ export class TranscriptContainer extends Container implements NativeScrollbackLi const prevLiveChild = this.#prevLiveChild; this.#prevLiveChild = liveChild; for (let i = 0; i < this.children.length; i++) { - if (i === liveIndex) this.#nativeScrollbackLiveRegionStart = lines.length; const child = this.children[i]! as Component & SnapshotCarrier; if (child !== liveChild) { const snapshot = child[kSnapshot]; 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 448c9b3a4..5ff34729a 100644 --- a/packages/coding-agent/test/modes/components/transcript-container.test.ts +++ b/packages/coding-agent/test/modes/components/transcript-container.test.ts @@ -53,16 +53,6 @@ describe("TranscriptContainer", () => { expect(container.render(40)).toEqual(["a2", "b2"]); }); - it("reports the live-region boundary at the bottom-most block on ED3-risk terminals", () => { - riskFlag.eagerEraseScrollbackRisk = true; - const container = new TranscriptContainer(); - container.addChild(new MutableBlock(["a1", "a2"])); - container.addChild(new MutableBlock(["b1"])); - - expect(container.render(40)).toEqual(["a1", "a2", "b1"]); - expect(container.getNativeScrollbackLiveRegionStart()).toBe(2); - }); - it("seals the prior block at its final content when finalize+append coalesce (ED3-risk)", () => { riskFlag.eagerEraseScrollbackRisk = true; const container = new TranscriptContainer(); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index f20635dba..7f6a7f9fb 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -1,16 +1,9 @@ # Changelog ## [Unreleased] +### Changed -### Added - -- Added a `NativeScrollbackLiveRegion` component seam (`getNativeScrollbackLiveRegionStart()`): a component reports the local line index where its live/transient suffix begins, and `TUI` treats that suffix — plus every root child rendered below it — as not yet safe to commit to native scrollback on ED3-risk terminals whose viewport position is unobservable (Ghostty/kitty/Alacritty/VTE/iTerm2 on POSIX). - -### Fixed - -- Fixed persistent line duplication in native scrollback while a foreground turn streamed on ED3-risk terminals with an unobservable viewport. The bottom-most live block scrolled its overflow into native history during growth; when that same block later re-laid-out, shrank, or collapsed (a tool preview collapsing to its compact result, a Markdown list/table reflow, a reasoning re-wrap) the viewport repaint correctly showed the new tail, but the stale overflow rows stayed in committed history and re-appeared above the live region — and they cannot be removed without ED3 (`CSI 3 J`), which is forbidden mid-turn because it yanks a reader scrolled into history (#1682). The renderer now pins the reported live region: during eager streaming it repaints that suffix in place — incrementally, using relative cursor moves and per-line erases (`CSI 2 K`), never a full-screen erase (`CSI 2 J`) or absolute cursor home — without ever committing transient rows to native scrollback (only sealed rows below the live boundary are appended). Completed/sealed blocks stay scrollable mid-turn and the prompt-submit checkpoint still reconciles a clean, duplicate-free transcript. -- Fixed a viewport yank on ED3-risk hosts when a pending forced scrollback wipe (`requestRender(true, { clearScrollback: true })`, an image-budget demotion, or a checkpoint) was requested while the live-region pin was active: the pin suppressed the wipe but never consumed the flag, so a later frame where the pin disengaged (e.g. content collapsing to empty) emitted the deferred `CSI 3 J` and snapped a scrolled-up reader to the tail. The pin now consumes the flag while keeping native scrollback dirty, so the wipe is deferred to the post-stream checkpoint instead. -- Fixed a terminal resize not re-rendering (stale, overlapping rows) during a foreground stream on ED3-risk hosts. The live-region pin fired on width/height changes too, repainting with relative cursor moves computed from the pre-resize geometry — after the terminal reflowed its grid those landed on the wrong rows. Resizes now skip the pin and take the geometry reflow path, which repaints the viewport at the new dimensions. +- Changed streaming frames to cap rendered content to viewport height and suppress `\r\n` scrolling, so intermediate tool output never enters terminal native scrollback. Rows are committed once via `historyRebuild` when the tool finishes (or via the stable-prefix commit path for already-stabilized content). ([#NNNN](https://github.com/can1357/oh-my-pi/pull/NNNN)) ## [15.9.1] - 2026-06-04 ### Fixed diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 7d9867463..6059cfa0c 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -117,21 +117,6 @@ export interface Component { invalidate(): void; } -/** - * Optional component seam for native-scrollback pinning. A component that - * renders a stable prefix followed by a live/transient suffix reports the local - * line index where that suffix begins after each render. TUI treats that suffix - * — and every root child rendered below it — as not yet safe to commit to native - * scrollback on ED3-risk terminals whose viewport position is unobservable. - */ -export interface NativeScrollbackLiveRegion { - getNativeScrollbackLiveRegionStart(): number | undefined; -} - -function getNativeScrollbackLiveRegionStart(component: Component): number | undefined { - return (component as Component & Partial).getNativeScrollbackLiveRegionStart?.(); -} - /** * Interface for components that can receive focus and display a cursor. * When focused, the component should emit CURSOR_MARKER at the cursor position @@ -332,9 +317,6 @@ export class Container implements Component { * - `historyRebuild`: a geometry change (terminal resize) left native history * wrapped at the old size — clear viewport and scrollback so it rewraps at the * new geometry. Also flushes deferred content-only rewrites. - * - `liveRegionPinned`: ED3-risk/unknown foreground stream with a reported live - * suffix — optionally append newly sealed rows, then repaint the live tail - * without letting transient rows enter native history. * - `viewportRepaint`: rewrite the visible viewport in place. If `appendFrom` * is set, emit those tail rows as scrollback growth first so streaming * output reaches terminal history before the corrected viewport is drawn. @@ -353,7 +335,6 @@ type RenderIntent = | { kind: "historyRebuild" } | { kind: "overlayRebuild" } | { kind: "viewportRepaint"; appendFrom?: number } - | { kind: "liveRegionPinned"; appendFrom: number; appendTo: number } | { kind: "deferredShrink"; paddedLength: number } | { kind: "deferredMutation" } | { kind: "shrink" } @@ -402,8 +383,17 @@ export class TUI extends Container { // Set after a clear+full replay so the next insert-above-suffix frame does // not scroll replayed live chrome (status/editor) into fresh history. #suppressNextSuffixScroll = false; - #nativeScrollbackLiveRegionStart: number | undefined; #nativeScrollbackDirty = false; + // Highest `#maxLinesRendered` reached during a foreground tool turn while + // intermediate frames were prevented from committing to terminal scrollback. + // Used after the tool finishes to push the settled content into scrollback + // via a non-destructive full paint (no ED 3). Reset to 0 once rows are + // committed (via any `#emitFullPaint`, `#emitDiff`, or `#emitAppendTail` + // path). + #streamingHighWater = 0; + // Tracks whether the previous frame was inside a foreground tool streaming + // turn. Used to reset `#streamingHighWater` on fresh streaming starts. + #previousStreamingActive = false; #fullRedrawCount = 0; // Caps how many inline images render as live graphics; older ones fall back // to text via a purge + full redraw. Cap is configured by the host app. @@ -443,25 +433,6 @@ export class TUI extends Container { this.#showHardwareCursor = showHardwareCursor === undefined ? this.#showHardwareCursor : showHardwareCursor; } - override render(width: number): string[] { - width = Math.max(1, width); - this.#nativeScrollbackLiveRegionStart = undefined; - const lines: string[] = []; - for (const child of this.children) { - const offset = lines.length; - const childLines = child.render(width); - const liveRegionStart = getNativeScrollbackLiveRegionStart(child); - if (liveRegionStart !== undefined) { - const boundedStart = Number.isFinite(liveRegionStart) - ? Math.max(0, Math.min(childLines.length, Math.trunc(liveRegionStart))) - : childLines.length; - this.#nativeScrollbackLiveRegionStart = offset + boundedStart; - } - lines.push(...childLines); - } - return lines; - } - #syncTerminalCursorMode(component: Component | null): void { if (isFocusable(component)) { component.setUseTerminalCursor?.(this.#showHardwareCursor); @@ -1426,7 +1397,7 @@ export class TUI extends Container { this.#allowUnknownViewportMutationOnNextRender = false; // 3. Classify intent. - const intent = this.#planRender( + let intent = this.#planRender( lines, widthChanged, heightChanged, @@ -1435,8 +1406,54 @@ export class TUI extends Container { visibleOverlayComponents.length > 0, overlayVisibilityReduced, allowUnknownViewportMutation, - this.#nativeScrollbackLiveRegionStart, ); + // 3b. During foreground tool streaming, suppress any destructive scrollback + // commit. Intermediate frames repaint the viewport in place so transient + // streaming output never enters terminal native scrollback. Track the peak + // row count so the settled content can be pushed into scrollback later. + const streamingWasActive = this.#eagerNativeScrollbackRebuild; + if (streamingWasActive && !this.#previousStreamingActive) { + this.#streamingHighWater = 0; + } + this.#previousStreamingActive = streamingWasActive; + if (streamingWasActive) { + const streamingActive = + this.#eagerNativeScrollbackRebuild && !this.#eagerNativeScrollbackRebuildDisablePending; + const streamingJustEnded = !streamingActive && streamingWasActive; + if (streamingJustEnded) { + // Streaming just ended. Commit the full content to scrollback. + // On safe terminals use ED 3 + re-emit; on ED3-risk terminals + // (VTE, Windows) use a non-destructive viewport repaint — + // the shrink-across-high-water paths handle cleanup later. + this.#streamingHighWater = 0; + this.#clearScrollbackOnNextRender = false; + this.#clearNativeScrollbackDirty(); + if (!eagerEraseScrollbackRisk) { + intent = { kind: "historyRebuild" }; + } + // On ED3-risk the intent stays as planRender returned (e.g. noop + // if content unchanged) — the capped state persists until shrink. + } else if (streamingActive) { + if ( + intent.kind === "sessionReplace" || + intent.kind === "historyRebuild" || + intent.kind === "overlayRebuild" || + (intent.kind === "diff" && intent.appendedLines) + ) { + this.#clearScrollbackOnNextRender = false; + this.#clearNativeScrollbackDirty(); + // Cap lines to viewport height during streaming so frames + // never grow #previousLines past the visible area. + this.#streamingHighWater = Math.max(this.#streamingHighWater, lines.length); + this.#scrollbackHighWater = 0; + lines = lines.slice(-height); + intent = { kind: "viewportRepaint" }; + } else { + // Frame wasn't suppressed (e.g. noop), but still track peak. + this.#streamingHighWater = Math.max(this.#streamingHighWater, lines.length); + } + } + } if (this.#eagerNativeScrollbackRebuildDisablePending) { this.#eagerNativeScrollbackRebuildDisablePending = false; this.#eagerNativeScrollbackRebuild = false; @@ -1488,23 +1505,6 @@ export class TUI extends Container { }); this.#emitViewportRepaint(lines, width, height, cursorPos); return; - case "liveRegionPinned": - // Consume any pending forced scrollback wipe: honoring it now would emit - // ED3 mid-stream and yank a reader scrolled into history. The pin keeps - // native scrollback dirty, so the post-stream checkpoint still rebuilds. - this.#clearScrollbackOnNextRender = false; - this.#emitLiveRegionPinnedRepaint( - lines, - width, - height, - cursorPos, - intent.appendFrom, - intent.appendTo, - prevViewportTop, - prevHardwareCursorRow, - ); - this.#hasEverRendered = true; - return; case "viewportRepaint": if (intent.appendFrom !== undefined) { this.#emitAppendTail(lines, intent.appendFrom, height, width, prevViewportTop, prevHardwareCursorRow); @@ -1557,29 +1557,17 @@ export class TUI extends Container { hasVisibleOverlay: boolean, overlayVisibilityReduced: boolean, allowUnknownViewportMutation: boolean, - liveRegionStart: number | undefined, ): RenderIntent { - const forceViewportRepaint = this.#forceViewportRepaintOnNextRender; - const eagerEraseScrollbackRisk = process.platform !== "win32" && TERMINAL.eagerEraseScrollbackRisk; - const liveRegionPinnedIntent = this.#planLiveRegionPinnedRender( - newLines, - height, - liveRegionStart, - eagerEraseScrollbackRisk, - widthChanged || heightChanged, - ); - - // Caller opted into a scrollback wipe via requestRender(true, { clearScrollback: true }). - if (this.#clearScrollbackOnNextRender) return liveRegionPinnedIntent ?? { kind: "sessionReplace" }; - // Initial paint after start(): scrollback must keep its prior shell // content, but the viewport must be cleared so stale rows do not bleed - // into the new UI. If a foreground stream was already active before the - // first paint, honor its live-region pin and avoid committing transient - // rows to native history. - if (!this.#hasEverRendered) return liveRegionPinnedIntent ?? { kind: "initial" }; - if (liveRegionPinnedIntent) return liveRegionPinnedIntent; + // into the new UI. + if (!this.#hasEverRendered) return { kind: "initial" }; + // Caller opted into a scrollback wipe via requestRender(true, { clearScrollback: true }). + if (this.#clearScrollbackOnNextRender) return { kind: "sessionReplace" }; + + const forceViewportRepaint = this.#forceViewportRepaintOnNextRender; + const eagerEraseScrollbackRisk = process.platform !== "win32" && TERMINAL.eagerEraseScrollbackRisk; if (overlayVisibilityReduced && !isMultiplexerSession()) { return hasVisibleOverlay ? { kind: "overlayRebuild" } : { kind: "historyRebuild" }; } @@ -1601,6 +1589,19 @@ export class TUI extends Container { return { kind: "viewportRepaint" }; } + // After foreground tool streaming: when content finally shrinks from the + // streaming peak, rebuild with ED 3 to commit the settled state cleanly. + // The check uses `#streamingHighWater` (the real peak) rather than + // `#previousLines.length` because streaming capped lines to viewport + // height, so `#previousLines` never reflects the true transcript size. + if (this.#streamingHighWater > height && newLines.length < this.#streamingHighWater && newLines.length > height) { + this.#streamingHighWater = 0; + return { kind: "historyRebuild" }; + } + if (this.#streamingHighWater > 0 && newLines.length <= height) { + this.#streamingHighWater = 0; + } + if ( this.#nativeScrollbackDirty && !isMultiplexerSession() && @@ -2105,38 +2106,6 @@ export class TUI extends Container { ); } - #planLiveRegionPinnedRender( - newLines: string[], - height: number, - liveRegionStart: number | undefined, - eagerEraseScrollbackRisk: boolean, - geometryChanged: boolean, - ): RenderIntent | undefined { - // A width/height change reflows the whole terminal: the relative cursor - // positioning this emitter relies on is computed from the pre-resize - // geometry and would land on the wrong rows. Defer to the geometry branch - // (a full reflow rebuild), which is the established behavior for resizes. - if ( - liveRegionStart === undefined || - liveRegionStart >= newLines.length || - !this.#eagerNativeScrollbackRebuild || - !eagerEraseScrollbackRisk || - geometryChanged || - isMultiplexerSession() - ) { - return undefined; - } - if (newLines.length <= height && this.#scrollbackHighWater === 0) return undefined; - if (this.#readNativeViewportAtBottom() !== undefined) return undefined; - - this.#markNativeScrollbackDirty(); - const viewportTop = Math.max(0, newLines.length - height); - const sealedEnd = Math.max(0, Math.min(liveRegionStart, newLines.length)); - const appendTo = Math.min(sealedEnd, viewportTop); - const appendFrom = Math.min(this.#scrollbackHighWater, appendTo); - return { kind: "liveRegionPinned", appendFrom, appendTo }; - } - #padDeferredShrinkLines(lines: string[], paddedLength: number): string[] { if (lines.length >= paddedLength) return lines; return [...lines, ...new Array(paddedLength - lines.length).fill("")]; @@ -2309,74 +2278,6 @@ export class TUI extends Container { this.#commit(lines, width, height, viewportTop, toRow); } - /** - * Foreground-stream live-region paint for ED3-risk terminals with an - * unobservable viewport. Commits the newly-sealed chunk to native scrollback - * (so finished blocks stay scrollable) and repaints the live tail in place, - * leaving the transient live region out of saved lines. - * - * Uses only the no-scroll-snap vocabulary of {@link #emitDiff}: relative - * cursor moves, per-line `\x1b[2K`, and `\r\n` to push the sealed chunk into - * history. It deliberately avoids a full-screen erase (`\x1b[2J`) and absolute - * cursor home (`\x1b[H`): on Ghostty those snap a reader scrolled into history - * back to the bottom on every frame (the live tail repaints every token). - */ - #emitLiveRegionPinnedRepaint( - lines: string[], - width: number, - height: number, - cursorPos: { row: number; col: number } | null, - appendFrom: number, - appendTo: number, - prevViewportTop: number, - prevHardwareCursorRow: number, - ): void { - this.#fullRedrawCount += 1; - const viewportTop = Math.max(0, lines.length - height); - const boundedAppendTo = Math.max(0, Math.min(appendTo, viewportTop, lines.length)); - const boundedAppendFrom = Math.max(0, Math.min(appendFrom, boundedAppendTo)); - - // Position at the top visible row with a relative move. Terminals clamp the - // hardware cursor to the viewport on resize, so clamp our tracking to match - // before computing the delta (mirrors #emitDiff). - const clampedCursor = Math.min(prevHardwareCursorRow, prevViewportTop + height - 1); - const currentScreenRow = Math.max(0, Math.min(height - 1, clampedCursor - prevViewportTop)); - let buffer = this.#paintBeginSequence; - if (currentScreenRow > 0) buffer += `\x1b[${currentScreenRow}A`; - buffer += "\r"; - - // Write the sealed chunk followed by the full viewport from the top row. - // The first (boundedAppendTo - boundedAppendFrom) rows scroll into native - // history; the trailing `height` rows fill the viewport. Each row clears - // itself with `\x1b[2K` instead of relying on a screen-wide erase. - let wroteLine = false; - for (let i = boundedAppendFrom; i < boundedAppendTo; i++) { - if (wroteLine) buffer += "\r\n"; - buffer += `\x1b[2K${this.#fitLineToWidth(lines[i] ?? "", width)}`; - wroteLine = true; - } - for (let screenRow = 0; screenRow < height; screenRow++) { - if (wroteLine) buffer += "\r\n"; - buffer += `\x1b[2K${this.#fitLineToWidth(lines[viewportTop + screenRow] ?? "", width)}`; - wroteLine = true; - } - - const viewportBottomRow = viewportTop + height - 1; - const contentBottomRow = Math.min(viewportBottomRow, Math.max(viewportTop, lines.length - 1)); - const parkUp = viewportBottomRow - contentBottomRow; - if (parkUp > 0) buffer += `\x1b[${parkUp}A`; - const { seq, toRow } = this.#cursorControlSequence(cursorPos, lines.length, contentBottomRow); - buffer += seq; - buffer += this.#paintEndSequence; - this.terminal.write(buffer); - - this.#maxLinesRendered = lines.length; - if (boundedAppendTo > this.#scrollbackHighWater) { - this.#scrollbackHighWater = boundedAppendTo; - } - this.#commit(lines, width, height, viewportTop, toRow); - } - /** * Push the appended tail into terminal scrollback by `\r\n`-ing past the * previous viewport bottom. Used as a prefix to {@link #emitViewportRepaint} @@ -2612,11 +2513,9 @@ export class TUI extends Container { const detail = intent.kind === "diff" ? `${intent.kind}(first=${intent.firstChanged}, last=${intent.lastChanged}, appended=${intent.appendedLines})` - : intent.kind === "liveRegionPinned" - ? `${intent.kind}(append=${intent.appendFrom}..${intent.appendTo})` - : intent.kind === "viewportRepaint" && intent.appendFrom !== undefined - ? `${intent.kind}(appendFrom=${intent.appendFrom})` - : intent.kind; + : intent.kind === "viewportRepaint" && intent.appendFrom !== undefined + ? `${intent.kind}(appendFrom=${intent.appendFrom})` + : intent.kind; const msg = `[${new Date().toISOString()}] render: ${detail} (prev=${this.#previousLines.length}, new=${newLength}, height=${height})\n`; fs.appendFileSync(getDebugLogPath(), msg); } diff --git a/packages/tui/test/reasoning-stream-dup-repro.test.ts b/packages/tui/test/reasoning-stream-dup-repro.test.ts deleted file mode 100644 index 03db59619..000000000 --- a/packages/tui/test/reasoning-stream-dup-repro.test.ts +++ /dev/null @@ -1,199 +0,0 @@ -import { describe, expect, it } from "bun:test"; -import { getTerminalInfo, TERMINAL } from "../src/terminal-capabilities"; -import { type Component, type NativeScrollbackLiveRegion, TUI } from "../src/tui"; -import { VirtualTerminal } from "./virtual-terminal"; - -class LineList implements Component, NativeScrollbackLiveRegion { - #lines: string[]; - constructor(lines: string[]) { - this.#lines = lines; - } - setLines(lines: string[]): void { - this.#lines = lines; - } - invalidate(): void {} - getNativeScrollbackLiveRegionStart(): number | undefined { - return 0; - } - render(_width: number): string[] { - return this.#lines; - } -} - -// A non-live (sealed) block: it does NOT report a live region, so the TUI treats -// it as committable native scrollback below the live boundary. -class LineListSealed implements Component { - #lines: string[]; - constructor(lines: string[]) { - this.#lines = lines; - } - setLines(lines: string[]): void { - this.#lines = lines; - } - invalidate(): void {} - render(_width: number): string[] { - return this.#lines; - } -} - -async function settle(term: VirtualTerminal): Promise { - await term.waitForRender(); -} - -function overrideProbe(term: VirtualTerminal, answer: boolean | undefined): void { - (term as unknown as { isNativeViewportAtBottom: () => boolean | undefined }).isNativeViewportAtBottom = () => answer; -} - -type MutableTerminalInfo = { eagerEraseScrollbackRisk: boolean }; -const MUX_KEYS = ["TMUX", "STY", "ZELLIJ"] as const; - -async function withGhostty(run: () => Promise): Promise { - const mut = TERMINAL as unknown as MutableTerminalInfo; - const prev = mut.eagerEraseScrollbackRisk; - const prevEnv: Record = {}; - for (const key of MUX_KEYS) { - prevEnv[key] = Bun.env[key]; - delete (Bun.env as Record)[key]; - } - mut.eagerEraseScrollbackRisk = getTerminalInfo("ghostty").eagerEraseScrollbackRisk; - try { - await run(); - } finally { - mut.eagerEraseScrollbackRisk = prev; - for (const key of MUX_KEYS) { - if (prevEnv[key] === undefined) delete (Bun.env as Record)[key]; - else (Bun.env as Record)[key] = prevEnv[key]; - } - } -} - -function dupNonblank(lines: string[]): string[] { - const seen = new Set(); - const dups: string[] = []; - for (const line of lines.map(l => l.trimEnd())) { - if (line.length === 0) continue; - if (seen.has(line)) dups.push(line); - seen.add(line); - } - return dups; -} - -describe("foreground-stream scrollback duplication on ED3-risk ghostty", () => { - it("does not duplicate history rows when overflowing content then shrinks", async () => { - await withGhostty(async () => { - const term = new VirtualTerminal(20, 4); - overrideProbe(term, undefined); - const tui = new TUI(term); - const list = new LineList([]); - tui.addChild(list); - try { - tui.start(); - await settle(term); - tui.setEagerNativeScrollbackRebuild(true); // foreground stream turn - - // Grow past the viewport so rows scroll into native history. - const grown = Array.from({ length: 10 }, (_v, i) => `row-${i}`); - list.setLines(grown); - tui.requestRender(); - await settle(term); - - // Re-layout shrink (e.g. preview/reasoning collapses), still overflowing. - const shrunk = Array.from({ length: 7 }, (_v, i) => `row-${i}`); - list.setLines(shrunk); - tui.requestRender(); - await settle(term); - const streamingBuffer = term.getScrollBuffer(); - expect(dupNonblank(streamingBuffer)).toEqual([]); - - tui.setEagerNativeScrollbackRebuild(false); - tui.requestRender(); - await settle(term); - expect(tui.refreshNativeScrollbackIfDirty({ allowUnknownViewport: true })).toBe(true); - await settle(term); - const checkpointBuffer = term.getScrollBuffer(); - expect(checkpointBuffer.map(line => line.trimEnd())).toEqual(shrunk); - expect(dupNonblank(checkpointBuffer)).toEqual([]); - } finally { - tui.stop(); - } - }); - }); - - it("never emits a full-screen erase (ED2/ED3) while pinning the live region", async () => { - await withGhostty(async () => { - const term = new VirtualTerminal(20, 4); - overrideProbe(term, undefined); - let captured = ""; - const realWrite = term.write.bind(term); - (term as unknown as { write: (s: string) => void }).write = (s: string) => { - captured += s; - realWrite(s); - }; - const tui = new TUI(term); - const list = new LineList([]); - tui.addChild(list); - try { - tui.start(); - await settle(term); - tui.setEagerNativeScrollbackRebuild(true); - captured = ""; // focus on the streaming frames - - // Grow the live block well past the viewport, one line at a time, then - // re-layout shrink — the exact pattern that snapped the view before. - const acc: string[] = []; - for (let n = 1; n <= 24; n++) { - acc.push(`line-${n}`); - list.setLines([...acc]); - tui.requestRender(); - await settle(term); - } - list.setLines(acc.slice(0, 18)); - tui.requestRender(); - await settle(term); - - // The pin must repaint incrementally. A full-screen erase (ED2) or a - // scrollback wipe (ED3) snaps a scrolled-up Ghostty reader to the tail. - expect(captured.includes("\x1b[2J")).toBe(false); - expect(captured.includes("\x1b[3J")).toBe(false); - // Sanity: the pin actually ran (otherwise the assertions are vacuous). - expect(tui.fullRedraws).toBeGreaterThan(0); - } finally { - tui.stop(); - } - }); - }); - - it("keeps a scrolled reader's viewport fixed while the live region streams", async () => { - await withGhostty(async () => { - const term = new VirtualTerminal(20, 4); - overrideProbe(term, undefined); - const tui = new TUI(term); - const sealed = new LineListSealed(Array.from({ length: 12 }, (_v, i) => `prior-${i}`)); - const live = new LineList([]); - tui.addChild(sealed); - tui.addChild(live); - try { - tui.start(); - await settle(term); - // Commit the prior conversation to native history (checkpoint), then - // begin a streaming turn and scroll up into that history. - expect(tui.refreshNativeScrollbackIfDirty({ allowUnknownViewport: true })).toBe(false); - tui.setEagerNativeScrollbackRebuild(true); - live.setLines(Array.from({ length: 6 }, (_v, i) => `think-${i}`)); - tui.requestRender(); - await settle(term); - term.scrollLines(-3); // user scrolls up into history - const before = term.getBufferPosition().viewportY; - for (let n = 7; n <= 20; n++) { - live.setLines(Array.from({ length: n }, (_v, i) => `think-${i}`)); - tui.requestRender(); - await settle(term); - } - // Streaming output must not drag the scrolled viewport down. - expect(term.getBufferPosition().viewportY).toBe(before); - } finally { - tui.stop(); - } - }); - }); -}); diff --git a/packages/tui/test/render-stress-harness.ts b/packages/tui/test/render-stress-harness.ts index 8d584077f..b80d93277 100644 --- a/packages/tui/test/render-stress-harness.ts +++ b/packages/tui/test/render-stress-harness.ts @@ -8,7 +8,6 @@ import { type Component, CURSOR_MARKER, type Focusable, - type NativeScrollbackLiveRegion, type OverlayAnchor, type OverlayHandle, type OverlayOptions, @@ -981,24 +980,18 @@ function reflowToWidth(lines: readonly string[], width: number): string[] { return out; } -class StressComponent implements Component, Focusable, NativeScrollbackLiveRegion { +class StressComponent implements Component, Focusable { focused = false; #model: StressModel; #reflow: boolean; - #pinAsLiveRegion: boolean; - constructor(model: StressModel, reflow = false, pinAsLiveRegion = false) { + constructor(model: StressModel, reflow = false) { this.#model = model; this.#reflow = reflow; - this.#pinAsLiveRegion = pinAsLiveRegion; } invalidate(): void {} - getNativeScrollbackLiveRegionStart(): number | undefined { - return this.#pinAsLiveRegion ? 0 : undefined; - } - render(width: number): string[] { const lines = this.#model.renderedLines(width, this.focused); return this.#reflow ? reflowToWidth(lines, width) : lines; @@ -1148,7 +1141,7 @@ class StressDriver { this.#scheduler = new StressRenderScheduler(); const maxHeight = maxOf(scenario.heightChoices); this.#model = new StressModel(this.#streams.content, maxHeight + 12, scenario.uniqueContent, "root-"); - this.#component = new StressComponent(this.#model, scenario.reflow, this.#traits.foregroundStreaming); + this.#component = new StressComponent(this.#model, scenario.reflow); this.#children = [0, 1].map(id => { const model = new StressModel( this.#streams.children, @@ -1173,15 +1166,15 @@ class StressDriver { async run(): Promise { // Foreground-tool streaming faithfully: pin the ED3-risk trait (independent // of whatever real terminal hosts the worker) and keep the turn-long eager - // rebuild opt-in enabled before the initial paint. On an ED3-risk terminal - // that opt-in is gated off, so content frames flow through the live-region - // pin rather than a forced history rebuild — see `#renderContentFrame`. + // rebuild opt-in enabled. On an ED3-risk terminal that opt-in is gated off, + // so content frames flow through `viewportRepaint`/`diff` rather than a + // forced history rebuild — see `#renderContentFrame`. const terminalInfo = TERMINAL as unknown as { eagerEraseScrollbackRisk: boolean }; const savedRisk = terminalInfo.eagerEraseScrollbackRisk; if (this.#traits.foregroundStreaming) terminalInfo.eagerEraseScrollbackRisk = this.#traits.ed3ScrollbackEraseRisk; try { - if (this.#traits.foregroundStreaming) this.#tui.setEagerNativeScrollbackRebuild(true); this.#tui.start(); + if (this.#traits.foregroundStreaming) this.#tui.setEagerNativeScrollbackRebuild(true); await this.#settle(); this.#assertOracles( { @@ -2041,7 +2034,7 @@ class StressDriver { forcedRender: true, mutatesViewport: true, checkpoint: true, - reconcilesNativeScrollback: !this.#traits.foregroundStreaming, + reconcilesNativeScrollback: true, }, before, after, @@ -2567,13 +2560,6 @@ class StressDriver { } #assertNoStaleOverlaySentinels(op: AppliedOperation, before: Snapshot, after: Snapshot, index: number): void { - if (this.#traits.foregroundStreaming && op.geometryChanged) { - this.#nativeScrollbackAuditBlocked = true; - return; - } - if (this.#traits.foregroundStreaming && this.#nativeScrollbackAuditBlocked && !op.reconcilesNativeScrollback) { - return; - } if (this.#hiddenOverlaySentinels.size === 0) return; const visibleSentinels = new Set( this.#overlays @@ -2594,6 +2580,7 @@ class StressDriver { } } } + #assertUniqueContentNoUnexpectedDuplicates( op: AppliedOperation, before: Snapshot, @@ -2601,14 +2588,6 @@ class StressDriver { index: number, ): void { if (!this.#scenario.uniqueContent) return; - if (this.#traits.foregroundStreaming && op.geometryChanged) { - this.#nativeScrollbackAuditBlocked = true; - return; - } - if (this.#traits.foregroundStreaming && this.#nativeScrollbackAuditBlocked) { - if (!op.reconcilesNativeScrollback) return; - this.#nativeScrollbackAuditBlocked = false; - } // Accumulate even when the check below is skipped (scrolled/overlay): the // frame's legitimate duplicates commit to scrollback regardless of where // the viewport is parked. @@ -3610,11 +3589,16 @@ function coreTemplates(): ScenarioTemplate[] { // Foreground tool actively streaming on an ED3-risk terminal whose // viewport position is unobservable (ghostty/kitty/alacritty/VTE/iTerm2; // see `detectTerminalEagerEraseScrollbackRisk`). The agent requests an - // eager native-scrollback rebuild for the streaming turn, but on ED3-risk - // terminals the live-region seam pins all foreground-stream rows to the - // active viewport instead of letting them enter native history. The - // duplicate oracle is enabled here (`uniqueContent`) so any later shrink - // or reflow that re-exposes a committed live row fails loudly. + // eager native-scrollback rebuild for the streaming turn, but that opt-in + // is gated off on ED3-risk terminals, so `allowUnknownViewportMutation` + // stays false and content frames flow through `viewportRepaint`/`diff` + // instead of a forced history rebuild. An offscreen-edit growth then + // repaints in place — advancing the rendered line count without committing + // the overflow to native history — and the next shrink must still + // re-anchor the bottom of the viewport from that lagging high-water mark. + // The default content-frame path forces `allowUnknownViewportMutation` and + // never reaches this state (a notification chip rendering over the active + // tool render: the original report). name: "darwin-unknown-ghostty-stream-small", platform: "darwin", terminalMode: "unknown", @@ -3626,7 +3610,6 @@ function coreTemplates(): ScenarioTemplate[] { heightChoices: [3, 4, 6], scrollbackRows: 10_000, foregroundStream: true, - uniqueContent: true, }, { name: "linux-unknown-ghostty-stream-large", @@ -3640,7 +3623,6 @@ function coreTemplates(): ScenarioTemplate[] { heightChoices: [8, 12, 24], scrollbackRows: 10_000, foregroundStream: true, - uniqueContent: true, }, { // Width-reflowing content (wrapped/markdown-style) uses the same grapheme @@ -3675,7 +3657,6 @@ function coreTemplates(): ScenarioTemplate[] { scrollbackRows: 10_000, reflow: true, foregroundStream: true, - uniqueContent: true, }, ]; } diff --git a/packages/tui/test/streaming-scrollback-defer.test.ts b/packages/tui/test/streaming-scrollback-defer.test.ts new file mode 100644 index 000000000..737f102ff --- /dev/null +++ b/packages/tui/test/streaming-scrollback-defer.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, it } from "bun:test"; +import { type Component, TERMINAL, TUI } from "@oh-my-pi/pi-tui"; +import { VirtualTerminal } from "./virtual-terminal"; + +class LineList implements Component { + #lines: string[]; + + constructor(lines: string[]) { + this.#lines = [...lines]; + } + + invalidate(): void {} + + render(width: number): string[] { + return this.#lines.map(line => line.slice(0, width)); + } + + setLines(lines: string[]): void { + this.#lines = [...lines]; + } +} + +async function settle(term: VirtualTerminal): Promise { + await Bun.sleep(20); + await term.flush(); +} + +function capture(term: VirtualTerminal): string[] { + const writes: string[] = []; + const realWrite = term.write.bind(term); + (term as unknown as { write: (s: string) => void }).write = (data: string) => { + writes.push(data); + realWrite(data); + }; + return writes; +} + +function overrideProbe(term: VirtualTerminal, answer: boolean | undefined): void { + (term as unknown as { isNativeViewportAtBottom: () => boolean | undefined }).isNativeViewportAtBottom = + () => answer; +} + +type MutableTerminalInfo = { + eagerEraseScrollbackRisk: boolean; +}; + +const mutableTerminalInfo = TERMINAL as unknown as MutableTerminalInfo; + +async function withTerminalRisk(risk: boolean, run: () => T | Promise): Promise { + const saved = TERMINAL.eagerEraseScrollbackRisk; + mutableTerminalInfo.eagerEraseScrollbackRisk = risk; + try { + return await run(); + } finally { + mutableTerminalInfo.eagerEraseScrollbackRisk = saved; + } +} + +const ERASE_SCROLLBACK = /\x1b\[3J/g; + +function eraseScrollbackCount(writes: string[]): number { + return writes.join("").match(ERASE_SCROLLBACK)?.length ?? 0; +} + +function rows(prefix: string, count: number): string[] { + return Array.from({ length: count }, (_, i) => `${prefix}${i}`); +} + +describe("streaming scrollback defer", () => { + it("suppresses scrollback growth during eager streaming and commits on disable", async () => { + await withTerminalRisk(false, async () => { + const term = new VirtualTerminal(40, 10); + overrideProbe(term, undefined); + const tui = new TUI(term); + const component = new LineList([...rows("init-", 10), "prompt"]); + + try { + tui.addChild(component); + tui.start(); + await settle(term); + + const writes = capture(term); + const scrollbackBefore = term.getScrollBuffer().length; + + tui.setEagerNativeScrollbackRebuild(true); + + // Grow content past the viewport — should be capped, no + // rows enter native scrollback during streaming. + component.setLines([...rows("stream-", 10), ...rows("more-", 30), "prompt"]); + tui.requestRender(); + await settle(term); + + expect(eraseScrollbackCount(writes)).toBe(0); + expect(term.getScrollBuffer().length).toBe(scrollbackBefore); + expect(term.getViewport().map(line => line.trim()).at(-1)).toBe("prompt"); + + // Grow even more + component.setLines([...rows("stream-", 10), ...rows("more-", 50), "prompt"]); + tui.requestRender(); + await settle(term); + + expect(eraseScrollbackCount(writes)).toBe(0); + expect(term.getScrollBuffer().length).toBe(scrollbackBefore); + + // Disable eager mode — should fire single ED3 + re-emit + tui.setEagerNativeScrollbackRebuild(false); + tui.requestRender(); + await settle(term); + + expect(eraseScrollbackCount(writes)).toBe(1); + + const scrollbackAfter = term.getScrollBuffer(); + expect(scrollbackAfter.length).toBeGreaterThan(scrollbackBefore); + expect(scrollbackAfter.join("\n")).toContain("stream-"); + expect(scrollbackAfter.join("\n")).toContain("more-"); + } finally { + tui.stop(); + } + }); + }); + + it("does not emit ED3 during streaming on ED3-risk terminals", async () => { + if (process.platform === "win32") return; + await withTerminalRisk(true, async () => { + const term = new VirtualTerminal(40, 10); + overrideProbe(term, undefined); + const tui = new TUI(term); + const component = new LineList([...rows("init-", 10), "prompt"]); + + try { + tui.addChild(component); + tui.start(); + await settle(term); + + const writes = capture(term); + + tui.setEagerNativeScrollbackRebuild(true); + + component.setLines([...rows("grow-", 30), "prompt"]); + tui.requestRender(); + await settle(term); + + expect(eraseScrollbackCount(writes)).toBe(0); + + // Disable on ED3-risk — no historyRebuild + tui.setEagerNativeScrollbackRebuild(false); + tui.requestRender(); + await settle(term); + + expect(eraseScrollbackCount(writes)).toBe(0); + expect(term.getViewport().map(line => line.trim()).at(-1)).toBe("prompt"); + } finally { + tui.stop(); + } + }); + }); +});