fix(tui): adjusted unstable overflow handling for streamed boxes
- Updated the TUI commit-path guard so unstable overflow falls back to normal commits unless rewrites reach above the viewport top. - Added a regression test that streams a growing box while scrollback risk is enabled and checks all logical rows stay reachable in history or view.
This commit is contained in:
+18
-1
@@ -1826,7 +1826,24 @@ export class TUI extends Container {
|
||||
}
|
||||
return { kind: "stablePrefixCommit", target: stableScrollbackTarget };
|
||||
}
|
||||
if (stableScrollbackBoundary && hasUnstableOverflow && contentGrew && !isMultiplexerSession()) {
|
||||
// Unstable content (a live block) overflows the viewport. Withholding its
|
||||
// scrollback commit is only safe while the rows bound for offscreen are
|
||||
// still volatile — committing a row we can never repaint would strand a
|
||||
// stale duplicate above the live region on ED3-risk terminals. That holds
|
||||
// only when this frame actually rewrites a row at or above the new viewport
|
||||
// top (`diff.firstChanged < overflowRows`, e.g. a streaming markdown block
|
||||
// re-wrapping). When the frame only grows at or below the viewport top the
|
||||
// rows scrolling off are immutable, so dropping them here instead of
|
||||
// committing would lose them entirely (a box taller than the viewport whose
|
||||
// top is neither in scrollback nor on screen). Fall through to the normal
|
||||
// commit paths in that case so nothing scrolled off becomes unreachable.
|
||||
if (
|
||||
stableScrollbackBoundary &&
|
||||
hasUnstableOverflow &&
|
||||
contentGrew &&
|
||||
diff.firstChanged < overflowRows &&
|
||||
!isMultiplexerSession()
|
||||
) {
|
||||
return { kind: "viewportRepaint" };
|
||||
}
|
||||
if (pureAppend && contentGrew && this.#previousLines.length > height && !isMultiplexerSession()) {
|
||||
|
||||
@@ -2387,6 +2387,75 @@ describe("TUI terminal-state regressions", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps a live block's scrolled-off top reachable when it overflows the viewport", async () => {
|
||||
// Regression (cbdff129b): the stable-prefix scrollback commit capped native
|
||||
// history growth at the stable prefix. A single live block taller than the
|
||||
// viewport — e.g. a streaming tool-preview box — then had its top rows
|
||||
// neither committed to scrollback nor on screen: they vanished ("box top
|
||||
// cut"). Unlike a re-wrapping markdown block, the box grows append-only at
|
||||
// its bottom, so the rows scrolling above the viewport are immutable and
|
||||
// MUST reach native scrollback rather than being dropped.
|
||||
await withTerminalRisk(true, async () => {
|
||||
const height = 8;
|
||||
const term = new UnknownViewportTerminal(40, height, 500);
|
||||
const writes = captureWrites(term);
|
||||
const tui = new TUI(term);
|
||||
const prefix = ["intro-0", "intro-1", "intro-2"];
|
||||
const transcript = new StablePrefixLinesComponent(prefix);
|
||||
const footer = new MutableLinesComponent(["status", "prompt>"]);
|
||||
transcript.setStableLineCount(prefix.length); // only the prefix is stable; the box is live
|
||||
tui.setNativeScrollbackStableComponent(transcript);
|
||||
tui.addChild(transcript);
|
||||
tui.addChild(footer);
|
||||
|
||||
try {
|
||||
tui.start();
|
||||
await settle(term);
|
||||
|
||||
// Grow the box well past the viewport, appending at its bottom edge.
|
||||
for (let body = 1; body <= 14; body++) {
|
||||
transcript.setLines([...prefix, "box-top", ...rows("box-", body)]);
|
||||
tui.requestRender();
|
||||
await settle(term);
|
||||
|
||||
// Every row that has scrolled above the bottom-anchored viewport must
|
||||
// stay reachable through native scrollback (history ++ active grid).
|
||||
const reachable = term.getScrollBuffer().map(line => line.trimEnd());
|
||||
for (const row of [...prefix, "box-top", ...rows("box-", body)]) {
|
||||
expect(
|
||||
countMatches(reachable, new RegExp(`\\b${row}\\b`)),
|
||||
`${row} must stay reachable at body=${body}`,
|
||||
).toBeGreaterThanOrEqual(1);
|
||||
}
|
||||
}
|
||||
|
||||
// Viewport stays bottom-anchored on the live tail (box bottom + footer).
|
||||
expect(visible(term).map(line => line.trim())).toEqual([
|
||||
"box-8",
|
||||
"box-9",
|
||||
"box-10",
|
||||
"box-11",
|
||||
"box-12",
|
||||
"box-13",
|
||||
"status",
|
||||
"prompt>",
|
||||
]);
|
||||
// The box top scrolled into committed history rather than being dropped.
|
||||
const baseY = term.getBufferPosition().baseY;
|
||||
expect(
|
||||
term
|
||||
.getScrollBuffer()
|
||||
.slice(0, baseY)
|
||||
.map(line => line.trimEnd()),
|
||||
).toContain("box-top");
|
||||
// Anti-yank guarantee preserved: no destructive saved-lines erase.
|
||||
expect(writes.join("")).not.toContain("\x1b[3J");
|
||||
} finally {
|
||||
tui.stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps a scrolled-up reader anchored while streaming inserts arrive on POSIX (unknown viewport)", async () => {
|
||||
// POSIX terminals cannot report scrollback position, so isNativeViewportAtBottom()
|
||||
// is undefined. Before the fix the planner optimistically treated "unknown" as
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
import { describe, expect, it, vi } from "bun:test";
|
||||
import { type Component, TERMINAL, TUI } from "@oh-my-pi/pi-tui";
|
||||
import { VirtualTerminal } from "./virtual-terminal";
|
||||
|
||||
class UnknownViewportTerminal extends VirtualTerminal {
|
||||
isNativeViewportAtBottom(): undefined {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
class MutableLinesComponent 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.map(line => line.slice(0, width));
|
||||
}
|
||||
}
|
||||
|
||||
class StablePrefixLinesComponent extends MutableLinesComponent {
|
||||
#stableLineCount = 0;
|
||||
setStableLineCount(count: number): void {
|
||||
this.#stableLineCount = count;
|
||||
}
|
||||
getStableLineCount(): number {
|
||||
return this.#stableLineCount;
|
||||
}
|
||||
}
|
||||
|
||||
type MutableTerminalInfo = { eagerEraseScrollbackRisk: boolean };
|
||||
const mutableTerminalInfo = TERMINAL as unknown as MutableTerminalInfo;
|
||||
|
||||
async function settle(term: VirtualTerminal): Promise<void> {
|
||||
const nextTick = Promise.withResolvers<void>();
|
||||
process.nextTick(nextTick.resolve);
|
||||
await nextTick.promise;
|
||||
await Bun.sleep(1);
|
||||
await term.flush();
|
||||
}
|
||||
|
||||
function visible(term: VirtualTerminal): string[] {
|
||||
return term.getViewport().map(line => line.trimEnd());
|
||||
}
|
||||
|
||||
describe("box-top repro", () => {
|
||||
it("keeps the box top reachable while it streams past the viewport", async () => {
|
||||
const saved = TERMINAL.eagerEraseScrollbackRisk;
|
||||
mutableTerminalInfo.eagerEraseScrollbackRisk = true;
|
||||
let monotonicNow = 0;
|
||||
const now = vi.spyOn(performance, "now").mockImplementation(() => {
|
||||
monotonicNow += 20;
|
||||
return monotonicNow;
|
||||
});
|
||||
try {
|
||||
const H = 8;
|
||||
const term = new UnknownViewportTerminal(40, H, 500);
|
||||
const tui = new TUI(term);
|
||||
const transcript = new StablePrefixLinesComponent(["T0", "T1", "T2", "T3"]);
|
||||
const footer = new MutableLinesComponent(["status", "prompt>"]);
|
||||
if (Bun.env.REPRO_STABLE !== "0") tui.setNativeScrollbackStableComponent(transcript);
|
||||
tui.addChild(transcript);
|
||||
tui.addChild(footer);
|
||||
try {
|
||||
tui.start();
|
||||
await settle(term);
|
||||
|
||||
const prefix = ["T0", "T1", "T2", "T3"];
|
||||
// Box top border is the FIRST box row "B-top".
|
||||
for (let k = 1; k <= 12; k++) {
|
||||
const box = ["B-top", ...Array.from({ length: k }, (_v, i) => `Bmid${i}`), "B-bot"];
|
||||
transcript.setLines([...prefix, ...box]);
|
||||
transcript.setStableLineCount(prefix.length); // box stays unstable while streaming
|
||||
tui.requestRender();
|
||||
await settle(term);
|
||||
|
||||
const hist = term.getScrollBuffer().map(l => l.trimEnd());
|
||||
const view = visible(term);
|
||||
const all = [...hist, ...view];
|
||||
const total = prefix.length + box.length + 2;
|
||||
// Each logical row must be reachable somewhere (history or viewport).
|
||||
const missing = [...prefix, ...box].filter(row => !all.some(l => l.includes(row)));
|
||||
console.log(
|
||||
`k=${k} total=${total} viewTop=${JSON.stringify(view[0])} baseY=${term.getBufferPosition().baseY} missing=${JSON.stringify(missing)}`,
|
||||
);
|
||||
}
|
||||
expect(true).toBe(true);
|
||||
} finally {
|
||||
tui.stop();
|
||||
}
|
||||
} finally {
|
||||
now.mockRestore();
|
||||
mutableTerminalInfo.eagerEraseScrollbackRisk = saved;
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user