From a882e9744bd2176d2b82eb7ee394df13af90e01d Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 31 May 2026 17:07:41 +0000 Subject: [PATCH] fix(tui): rebuilt blank deferred shrink from padded top Codex review on #1599: the blank-viewport guard must compare the new transcript length against the viewport top used by the padded repaint, not #scrollbackHighWater. Prior unknown-POSIX viewport repaints can commit a long logical frame without advancing the high-water mark, so the stale high-water mark still lets all-blank deferred shrinks through.\n\nCompute paddedViewportTop from #previousLines.length - height and rebuild when the new tail cannot reach it. Add a regression for an offscreen POSIX viewport repaint that grows the committed frame to 120 rows while high-water remains at the original 20-row overflow, then shrinks to 15 rows. --- packages/tui/src/tui.ts | 17 +++--- packages/tui/test/render-regressions.test.ts | 55 ++++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index e3887b664..477673c21 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -1342,12 +1342,17 @@ export class TUI extends Container { // previous row count so no committed row is re-emitted, and the next checkpoint // rebuild (e.g. prompt submit -> `refreshNativeScrollbackIfDirty`) cleans up. // - // That deferral only carries real content when `newLines.length > scrollbackHighWater` - // — otherwise the padded viewport rows fall entirely past the end of `newLines` - // and render as all blanks, hiding the prompt until the next checkpoint. For - // shrinks that large, yanking a scrolled reader (historyRebuild) is the lesser - // evil; do it unconditionally. - if (newLines.length <= this.#scrollbackHighWater) { + // That deferral only carries real content when `newLines.length` reaches the + // padded viewport top (`previousLines.length - height`) — otherwise every + // row the padded repaint draws is past the end of `newLines` and renders as + // blank, hiding the prompt until the next checkpoint. This can happen even + // when `scrollbackHighWater` is much lower than `previousLines.length - height`, + // because prior unknown-POSIX viewport repaints commit longer logical frames + // without moving the native scrollback boundary. For shrinks that large, + // yanking a scrolled reader (historyRebuild) is the lesser evil; do it + // unconditionally. + const paddedViewportTop = Math.max(0, this.#previousLines.length - height); + if (newLines.length <= paddedViewportTop) { return { kind: "historyRebuild" }; } this.#markNativeScrollbackDirty(); diff --git a/packages/tui/test/render-regressions.test.ts b/packages/tui/test/render-regressions.test.ts index c4769e8b8..bbd2e635a 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -1625,6 +1625,61 @@ describe("TUI terminal-state regressions", () => { tui.stop(); } }); + it("rebuilds history when prior POSIX repaint left the padded viewport past the new tail", async () => { + const term = new UnknownViewportTerminal(40, 10); + const tui = new TUI(term); + const initial = rows("line-", 19); + const component = new MutableLinesComponent([...initial, "prompt-row"]); + tui.addChild(component); + + try { + tui.start(); + await settle(term); + + // Unknown-POSIX offscreen mutation: repainting the viewport commits the + // 120-row logical frame, but `#emitViewportRepaint` intentionally does not + // advance `#scrollbackHighWater` (it remains at the original 20-row frame's + // 10-row overflow). The later shrink must compare against the padded viewport + // top (`120 - height`) rather than the stale high-water mark. + const expanded = ["edited-line", ...rows("line-", 118), "prompt-row"]; + component.setLines(expanded); + tui.requestRender(); + await settle(term); + expect(visible(term).map(line => line.trim())).toEqual([ + "line-109", + "line-110", + "line-111", + "line-112", + "line-113", + "line-114", + "line-115", + "line-116", + "line-117", + "prompt-row", + ]); + + const short = [...rows("short-", 14), "prompt-row"]; + component.setLines(short); + tui.requestRender(); + await settle(term); + + expect(visible(term).map(line => line.trim())).toEqual([ + "short-5", + "short-6", + "short-7", + "short-8", + "short-9", + "short-10", + "short-11", + "short-12", + "short-13", + "prompt-row", + ]); + expect(term.getScrollBuffer().join("\n")).not.toContain("line-"); + } finally { + tui.stop(); + } + }); it("renders streaming row inserts on WSL Windows Terminal even when viewport probe is unavailable", async () => { const originalPlatform = process.platform;