From ecfbc045b38472b4a9276bf83fb1475cbb2fc21d Mon Sep 17 00:00:00 2001 From: KO Ho Tin Date: Sun, 26 Jul 2026 17:16:16 +0800 Subject: [PATCH 1/3] fix(tui): skip destructive redraw for in-place changes above the viewport Any changed line above the previous viewport top forced fullRender(true), which clears the screen and scrollback (ESC[2J/ESC[3J) and reprints the whole transcript. While subagents run, the agents status header ticks several times per second; once /usage (or any appended content) pushes it above the viewport, every tick becomes a destructive full redraw -- sustained flicker and scroll-position yanks. Rows above the viewport are already committed to scrollback, so an in-place mutation (same line count, no kitty images in the above-viewport range) cannot change what the terminal shows. Commit the caches and skip the paint when the change stays above the viewport, or clamp the repaint range to the visible part when it spans the boundary. Layout shifts, size changes, and image lines keep the full-redraw baseline restored in #1367, so none of the artifact classes that reverted #1315/#1353 can reappear on this path. Fixes #2039 --- .changeset/pi-tui-above-viewport-inplace.md | 5 + .changeset/tui-usage-flicker.md | 5 + packages/pi-tui/src/tui.ts | 53 ++++++- .../test/above-viewport-inplace.vitest.ts | 132 ++++++++++++++++++ packages/pi-tui/vitest.config.ts | 2 +- 5 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 .changeset/pi-tui-above-viewport-inplace.md create mode 100644 .changeset/tui-usage-flicker.md create mode 100644 packages/pi-tui/test/above-viewport-inplace.vitest.ts diff --git a/.changeset/pi-tui-above-viewport-inplace.md b/.changeset/pi-tui-above-viewport-inplace.md new file mode 100644 index 0000000000..011962fc47 --- /dev/null +++ b/.changeset/pi-tui-above-viewport-inplace.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/pi-tui": patch +--- + +Skip the destructive full redraw when lines above the viewport change in place, so scrollback and scroll position survive above-viewport status ticks. diff --git a/.changeset/tui-usage-flicker.md b/.changeset/tui-usage-flicker.md new file mode 100644 index 0000000000..8b9433a388 --- /dev/null +++ b/.changeset/tui-usage-flicker.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix the TUI flickering and losing scroll position when live agent status updates continue above the visible viewport, such as after opening /usage during a running task. diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index e4556a8f47..0b94b8933a 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -1171,6 +1171,19 @@ export class TUI extends Container { return { firstChanged: expandedFirstChanged, lastChanged: expandedLastChanged }; } + /** Whether either frame carries kitty image ids inside [start, end]. */ + private rangeHasKittyImages( + start: number, + end: number, + newLineImageIds: ReadonlyArray>, + ): boolean { + for (let i = start; i <= end; i++) { + if ((newLineImageIds[i] ?? EMPTY_IMAGE_IDS).length > 0) return true; + if ((this.previousLineImageIds[i] ?? EMPTY_IMAGE_IDS).length > 0) return true; + } + return false; + } + private deleteChangedKittyImages(firstChanged: number, lastChanged: number): string { if (firstChanged < 0 || lastChanged < firstChanged) return ""; @@ -1515,11 +1528,43 @@ export class TUI extends Container { } // Differential rendering can only touch what was actually visible. - // If the first changed line is above the previous viewport, we need a full redraw. + // If the first changed line is above the previous viewport, a full + // redraw is normally needed — but an in-place mutation (same line + // count, no kitty images in the above-viewport range) of rows already + // committed to scrollback cannot change what the terminal shows, so + // repainting the whole buffer would only produce a destructive + // ESC[2J/ESC[3J clear per tick (e.g. an agent-status spinner pushed + // above the viewport, #2039). Skip the paint, or clamp the range to + // the viewport when it spans the boundary; any layout shift or image + // involvement keeps the full redraw (the post-#1367 baseline). if (firstChanged < prevViewportTop) { - logRedraw(`firstChanged < viewportTop (${firstChanged} < ${prevViewportTop})`); - fullRender(true); - return; + const aboveEnd = Math.min(lastChanged, prevViewportTop - 1); + const inPlaceAboveViewport = + newLines.length === this.previousLines.length && + !this.rangeHasKittyImages(firstChanged, aboveEnd, lineImageIds); + if (!inPlaceAboveViewport) { + logRedraw(`firstChanged < viewportTop (${firstChanged} < ${prevViewportTop})`); + fullRender(true); + return; + } + if (lastChanged < prevViewportTop) { + // Entirely above the viewport: scrollback keeps a stale frame + // of those rows (invisible from the live viewport), so only + // commit the caches and reposition the hardware cursor, + // mirroring the no-change path above. + this.positionHardwareCursor(cursorPos, newLines.length); + this.previousViewportTop = prevViewportTop; + this.previousLines = newLines; + this.previousRawLines = rawLines; + this.previousLineImageIds = lineImageIds; + this.previousKittyImageIds = this.unionKittyImageIds(lineImageIds); + this.previousWidth = width; + this.previousHeight = height; + return; + } + // The change spans the viewport boundary: repaint only the + // visible part on the regular differential path. + firstChanged = prevViewportTop; } // Render from first changed line to end diff --git a/packages/pi-tui/test/above-viewport-inplace.vitest.ts b/packages/pi-tui/test/above-viewport-inplace.vitest.ts new file mode 100644 index 0000000000..48537a8416 --- /dev/null +++ b/packages/pi-tui/test/above-viewport-inplace.vitest.ts @@ -0,0 +1,132 @@ +/** + * In-place changes above the viewport must not trigger destructive full + * redraws (ESC[2J / ESC[3J): those clear screen + scrollback on every tick + * of an above-viewport spinner, producing sustained flicker and scroll- + * position yanks (#2039). Layout shifts keep the post-#1367 full-redraw + * baseline. + * + * Written with the vitest API and opted into vitest.config.ts — the rest of + * this package's suite runs under node:test. + */ +import { describe, expect, it } from "vitest"; +import { type Component, TUI } from "../src/tui.ts"; +import { VirtualTerminal } from "./virtual-terminal.ts"; + +class TestComponent implements Component { + lines: string[] = []; + render(_width: number): string[] { + return this.lines; + } + invalidate(): void {} +} + +class LoggingVirtualTerminal extends VirtualTerminal { + private writes: string[] = []; + + override write(data: string): void { + this.writes.push(data); + super.write(data); + } + + getWrites(): string { + return this.writes.join(""); + } + + clearWrites(): void { + this.writes = []; + } +} + +function makeLines(count: number): string[] { + return Array.from({ length: count }, (_, i) => `line ${i}`); +} + +describe("above-viewport in-place changes", () => { + it("skips the destructive full redraw when a line above the viewport changes in place", async () => { + const terminal = new LoggingVirtualTerminal(40, 10); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + // 30 lines on a 10-row terminal -> viewport top sits at row 20. + component.lines = makeLines(30); + tui.start(); + await terminal.waitForRender(); + const initialRedraws = tui.fullRedraws; + const viewportBefore = await terminal.flushAndGetViewport(); + + // Repeated ticks mirror a spinner line that /usage pushed above the + // viewport: same line count, one changed row per tick. + for (let tick = 0; tick < 4; tick++) { + const lines = makeLines(30); + lines[3] = `spinner frame ${tick}`; + component.lines = lines; + terminal.clearWrites(); + tui.requestRender(); + await terminal.waitForRender(); + + expect(tui.fullRedraws).toBe(initialRedraws); + const writes = terminal.getWrites(); + expect(writes).not.toContain("\x1b[2J"); + expect(writes).not.toContain("\x1b[3J"); + } + + // The visible viewport stays untouched by the skipped repaints. + expect(await terminal.flushAndGetViewport()).toEqual(viewportBefore); + tui.stop(); + }); + + it("repaints only the visible part when an in-place change spans the viewport boundary", async () => { + const terminal = new LoggingVirtualTerminal(40, 10); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + component.lines = makeLines(30); + tui.start(); + await terminal.waitForRender(); + const initialRedraws = tui.fullRedraws; + + const lines = makeLines(30); + for (let i = 18; i <= 22; i++) { + lines[i] = `changed ${i}`; + } + component.lines = lines; + terminal.clearWrites(); + tui.requestRender(); + await terminal.waitForRender(); + + expect(tui.fullRedraws).toBe(initialRedraws); + const writes = terminal.getWrites(); + expect(writes).not.toContain("\x1b[2J"); + expect(writes).not.toContain("\x1b[3J"); + + // The visible rows (20-22 -> screen rows 0-2) show the new content. + const viewport = await terminal.flushAndGetViewport(); + expect(viewport[0]).toContain("changed 20"); + expect(viewport[1]).toContain("changed 21"); + expect(viewport[2]).toContain("changed 22"); + expect(viewport[3]).toContain("line 23"); + tui.stop(); + }); + + it("keeps the full redraw when the line count changes above the viewport", async () => { + const terminal = new LoggingVirtualTerminal(40, 10); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + component.lines = makeLines(30); + tui.start(); + await terminal.waitForRender(); + const initialRedraws = tui.fullRedraws; + + // Inserting a row above the viewport shifts every following line — + // that layout change must keep the post-#1367 destructive redraw. + const lines = makeLines(30); + lines.splice(3, 0, "inserted above viewport"); + component.lines = lines; + tui.requestRender(); + await terminal.waitForRender(); + + expect(tui.fullRedraws).toBeGreaterThan(initialRedraws); + tui.stop(); + }); +}); diff --git a/packages/pi-tui/vitest.config.ts b/packages/pi-tui/vitest.config.ts index 8dbb6fd118..477d70dd32 100644 --- a/packages/pi-tui/vitest.config.ts +++ b/packages/pi-tui/vitest.config.ts @@ -7,7 +7,7 @@ import { defineConfig } from "vitest/config"; // file to vitest's API and add it to `include` to opt it in. export default defineConfig({ test: { - include: [], + include: ["test/above-viewport-inplace.vitest.ts"], passWithNoTests: true, }, }); From 6a3eac69fbfd0289b8187304e1abae7d5c090017 Mon Sep 17 00:00:00 2001 From: KO Ho Tin Date: Sun, 26 Jul 2026 17:34:54 +0800 Subject: [PATCH 2/3] test(tui): pin above-viewport in-place rendering in the node:test suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the guarding tests into tui-render.test.ts so the package test gate (node --test) exercises them — a re-vendor that clobbered the fix would otherwise pass acceptance — and record the behavior as divergence 6 in the vendored-fork ledger. --- packages/pi-tui/AGENTS.md | 4 +- .../test/above-viewport-inplace.vitest.ts | 132 ------------------ packages/pi-tui/test/tui-render.test.ts | 96 +++++++++++++ packages/pi-tui/vitest.config.ts | 2 +- 4 files changed, 100 insertions(+), 134 deletions(-) delete mode 100644 packages/pi-tui/test/above-viewport-inplace.vitest.ts diff --git a/packages/pi-tui/AGENTS.md b/packages/pi-tui/AGENTS.md index c25bd9efe3..e0101db82a 100644 --- a/packages/pi-tui/AGENTS.md +++ b/packages/pi-tui/AGENTS.md @@ -1,6 +1,6 @@ # pi-tui Agent Guide -`packages/pi-tui` is a vendored copy of pi-tui from the upstream pi-mono project (baseline: upstream 0.80.2, see commit `7859b0af`). It is no longer patched via pnpm patches — all local fixes are applied directly to the source. The differential-rendering behavior in `src/tui.ts` matches upstream: the fork's viewport/scrollback rendering patches were reverted; the only remaining divergences are listed below. +`packages/pi-tui` is a vendored copy of pi-tui from the upstream pi-mono project (baseline: upstream 0.80.2, see commit `7859b0af`). It is no longer patched via pnpm patches — all local fixes are applied directly to the source. The fork's broad viewport/scrollback rendering patches were reverted back to upstream's destructive-redraw baseline; the only remaining divergences are listed below. ## Local divergences from upstream (must be preserved on every re-vendor) @@ -12,6 +12,8 @@ Never overwrite this directory wholesale when syncing from upstream. Each of the 4. **`src/components/text.ts` / `markdown.ts` / `truncated-text.ts` / `editor.ts` — negative-width `repeat` guards**: the `repeat` counts for blank lines, horizontal rules, and the editor's top/bottom borders are clamped to ≥ 0 (two editor border sites; markdown's emptyLine and hr — the hr site is currently unreachable from the render entry and is purely defensive). Guarding tests: the "negative width safety" cases — Text's lives in `test/tui-render.test.ts` (Text has no dedicated test file), Markdown's and TruncatedText's live in their own test files; the editor's is "does not throw at zero or negative widths" inside the "Editor narrow width rendering" group in `test/editor.test.ts`. 5. **`src/tui.ts` — per-frame processed-line reuse**: `doRender` keeps the previous frame's raw lines (`previousRawLines`), their processed output (`previousLines`), and per-line kitty image ids (`previousLineImageIds`). A line whose raw string is reference-identical to the previous frame's reuses its processed output (truncation + `normalizeTerminalOutput` + trailing `SEGMENT_RESET`) verbatim, so a steady-state frame costs O(total lines) pointer comparisons plus O(changed lines) real work instead of re-normalizing every line. Image-id consumers (`expandChangedRangeForKittyImages`, `deleteChangedKittyImages`, the frame-end id union) read the cached per-line ids rather than re-scanning line text; upstream has no such cache and re-processes every line every frame (upstream also has `applyLineResets`, which this divergence inlines into the processed-line build). Reuse is only valid when the terminal width is unchanged; width changes re-render everything at the new width so references never match. Guarding tests: "TUI steady-frame processed-line reuse" in `test/tui-render.test.ts`. +6. **`src/tui.ts` — skip destructive redraw for in-place changes above the viewport**: when every changed line sits at or above the previous viewport top, the line count is unchanged, and neither frame has kitty image ids in the above-viewport part of the changed range, `doRender` must NOT fall back to `fullRender(true)` (upstream clears screen + scrollback via ESC[2J/ESC[3J for any above-viewport change, which flickers and yanks scroll position on every above-viewport status tick). Entirely-above changes commit the line caches without painting (scrollback keeps a stale frame); changes spanning the boundary clamp the repaint range to the viewport top and continue on the differential path. Any layout shift or image involvement keeps upstream's destructive redraw. Guarding tests: "TUI above-viewport in-place changes" in `test/tui-render.test.ts`. + ## Acceptance after syncing from upstream - `pnpm --filter @moonshot-ai/pi-tui test` must pass in full; any failure among the guarding tests above means a local divergence was overwritten and lost. diff --git a/packages/pi-tui/test/above-viewport-inplace.vitest.ts b/packages/pi-tui/test/above-viewport-inplace.vitest.ts deleted file mode 100644 index 48537a8416..0000000000 --- a/packages/pi-tui/test/above-viewport-inplace.vitest.ts +++ /dev/null @@ -1,132 +0,0 @@ -/** - * In-place changes above the viewport must not trigger destructive full - * redraws (ESC[2J / ESC[3J): those clear screen + scrollback on every tick - * of an above-viewport spinner, producing sustained flicker and scroll- - * position yanks (#2039). Layout shifts keep the post-#1367 full-redraw - * baseline. - * - * Written with the vitest API and opted into vitest.config.ts — the rest of - * this package's suite runs under node:test. - */ -import { describe, expect, it } from "vitest"; -import { type Component, TUI } from "../src/tui.ts"; -import { VirtualTerminal } from "./virtual-terminal.ts"; - -class TestComponent implements Component { - lines: string[] = []; - render(_width: number): string[] { - return this.lines; - } - invalidate(): void {} -} - -class LoggingVirtualTerminal extends VirtualTerminal { - private writes: string[] = []; - - override write(data: string): void { - this.writes.push(data); - super.write(data); - } - - getWrites(): string { - return this.writes.join(""); - } - - clearWrites(): void { - this.writes = []; - } -} - -function makeLines(count: number): string[] { - return Array.from({ length: count }, (_, i) => `line ${i}`); -} - -describe("above-viewport in-place changes", () => { - it("skips the destructive full redraw when a line above the viewport changes in place", async () => { - const terminal = new LoggingVirtualTerminal(40, 10); - const tui = new TUI(terminal); - const component = new TestComponent(); - tui.addChild(component); - // 30 lines on a 10-row terminal -> viewport top sits at row 20. - component.lines = makeLines(30); - tui.start(); - await terminal.waitForRender(); - const initialRedraws = tui.fullRedraws; - const viewportBefore = await terminal.flushAndGetViewport(); - - // Repeated ticks mirror a spinner line that /usage pushed above the - // viewport: same line count, one changed row per tick. - for (let tick = 0; tick < 4; tick++) { - const lines = makeLines(30); - lines[3] = `spinner frame ${tick}`; - component.lines = lines; - terminal.clearWrites(); - tui.requestRender(); - await terminal.waitForRender(); - - expect(tui.fullRedraws).toBe(initialRedraws); - const writes = terminal.getWrites(); - expect(writes).not.toContain("\x1b[2J"); - expect(writes).not.toContain("\x1b[3J"); - } - - // The visible viewport stays untouched by the skipped repaints. - expect(await terminal.flushAndGetViewport()).toEqual(viewportBefore); - tui.stop(); - }); - - it("repaints only the visible part when an in-place change spans the viewport boundary", async () => { - const terminal = new LoggingVirtualTerminal(40, 10); - const tui = new TUI(terminal); - const component = new TestComponent(); - tui.addChild(component); - component.lines = makeLines(30); - tui.start(); - await terminal.waitForRender(); - const initialRedraws = tui.fullRedraws; - - const lines = makeLines(30); - for (let i = 18; i <= 22; i++) { - lines[i] = `changed ${i}`; - } - component.lines = lines; - terminal.clearWrites(); - tui.requestRender(); - await terminal.waitForRender(); - - expect(tui.fullRedraws).toBe(initialRedraws); - const writes = terminal.getWrites(); - expect(writes).not.toContain("\x1b[2J"); - expect(writes).not.toContain("\x1b[3J"); - - // The visible rows (20-22 -> screen rows 0-2) show the new content. - const viewport = await terminal.flushAndGetViewport(); - expect(viewport[0]).toContain("changed 20"); - expect(viewport[1]).toContain("changed 21"); - expect(viewport[2]).toContain("changed 22"); - expect(viewport[3]).toContain("line 23"); - tui.stop(); - }); - - it("keeps the full redraw when the line count changes above the viewport", async () => { - const terminal = new LoggingVirtualTerminal(40, 10); - const tui = new TUI(terminal); - const component = new TestComponent(); - tui.addChild(component); - component.lines = makeLines(30); - tui.start(); - await terminal.waitForRender(); - const initialRedraws = tui.fullRedraws; - - // Inserting a row above the viewport shifts every following line — - // that layout change must keep the post-#1367 destructive redraw. - const lines = makeLines(30); - lines.splice(3, 0, "inserted above viewport"); - component.lines = lines; - tui.requestRender(); - await terminal.waitForRender(); - - expect(tui.fullRedraws).toBeGreaterThan(initialRedraws); - tui.stop(); - }); -}); diff --git a/packages/pi-tui/test/tui-render.test.ts b/packages/pi-tui/test/tui-render.test.ts index d40811d0e8..6ebd5e7cb1 100644 --- a/packages/pi-tui/test/tui-render.test.ts +++ b/packages/pi-tui/test/tui-render.test.ts @@ -990,3 +990,99 @@ describe("TUI steady-frame processed-line reuse", () => { } }); }); + +describe("TUI above-viewport in-place changes", () => { + // Divergence 6 in AGENTS.md: in-place mutations of rows already committed + // to scrollback must not trigger the destructive ESC[2J/ESC[3J full + // redraw (#2039). Layout shifts keep upstream's destructive baseline. + function makeLines(count: number): string[] { + return Array.from({ length: count }, (_, i) => `line ${i}`); + } + + it("skips the destructive full redraw when a line above the viewport changes in place", async () => { + const terminal = new LoggingVirtualTerminal(40, 10); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + // 30 lines on a 10-row terminal -> viewport top sits at row 20. + component.lines = makeLines(30); + tui.start(); + await terminal.waitForRender(); + const initialRedraws = tui.fullRedraws; + const viewportBefore = await terminal.flushAndGetViewport(); + + // Repeated ticks mirror a spinner line pushed above the viewport: + // same line count, one changed row per tick. + for (let tick = 0; tick < 4; tick++) { + const lines = makeLines(30); + lines[3] = `spinner frame ${tick}`; + component.lines = lines; + terminal.clearWrites(); + tui.requestRender(); + await terminal.waitForRender(); + + assert.strictEqual(tui.fullRedraws, initialRedraws, "in-place above-viewport change must not full-redraw"); + const writes = terminal.getWrites(); + assert.ok(!writes.includes("\x1b[2J"), "must not clear the screen"); + assert.ok(!writes.includes("\x1b[3J"), "must not clear scrollback"); + } + + assert.deepStrictEqual(await terminal.flushAndGetViewport(), viewportBefore, "viewport must stay untouched"); + tui.stop(); + }); + + it("repaints only the visible part when an in-place change spans the viewport boundary", async () => { + const terminal = new LoggingVirtualTerminal(40, 10); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + component.lines = makeLines(30); + tui.start(); + await terminal.waitForRender(); + const initialRedraws = tui.fullRedraws; + + const lines = makeLines(30); + for (let i = 18; i <= 22; i++) { + lines[i] = `changed ${i}`; + } + component.lines = lines; + terminal.clearWrites(); + tui.requestRender(); + await terminal.waitForRender(); + + assert.strictEqual(tui.fullRedraws, initialRedraws, "boundary-spanning in-place change must not full-redraw"); + const writes = terminal.getWrites(); + assert.ok(!writes.includes("\x1b[2J"), "must not clear the screen"); + assert.ok(!writes.includes("\x1b[3J"), "must not clear scrollback"); + + // The visible rows (20-22 -> screen rows 0-2) show the new content. + const viewport = await terminal.flushAndGetViewport(); + assert.ok(viewport[0]?.includes("changed 20"), "row 20 repainted"); + assert.ok(viewport[1]?.includes("changed 21"), "row 21 repainted"); + assert.ok(viewport[2]?.includes("changed 22"), "row 22 repainted"); + assert.ok(viewport[3]?.includes("line 23"), "unchanged rows preserved"); + tui.stop(); + }); + + it("keeps the full redraw when the line count changes above the viewport", async () => { + const terminal = new LoggingVirtualTerminal(40, 10); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + component.lines = makeLines(30); + tui.start(); + await terminal.waitForRender(); + const initialRedraws = tui.fullRedraws; + + // Inserting a row above the viewport shifts every following line — + // that layout change must keep the destructive redraw baseline. + const lines = makeLines(30); + lines.splice(3, 0, "inserted above viewport"); + component.lines = lines; + tui.requestRender(); + await terminal.waitForRender(); + + assert.ok(tui.fullRedraws > initialRedraws, "layout shift above the viewport must full-redraw"); + tui.stop(); + }); +}); diff --git a/packages/pi-tui/vitest.config.ts b/packages/pi-tui/vitest.config.ts index 477d70dd32..8dbb6fd118 100644 --- a/packages/pi-tui/vitest.config.ts +++ b/packages/pi-tui/vitest.config.ts @@ -7,7 +7,7 @@ import { defineConfig } from "vitest/config"; // file to vitest's API and add it to `include` to opt it in. export default defineConfig({ test: { - include: ["test/above-viewport-inplace.vitest.ts"], + include: [], passWithNoTests: true, }, }); From 2a6000a20775db600c50181e13532cc0c154674b Mon Sep 17 00:00:00 2001 From: KO Ho Tin Date: Sun, 26 Jul 2026 17:58:02 +0800 Subject: [PATCH 3/3] fix(tui): repaint skipped rows re-exposed by a Termux height increase The above-viewport skip path leaves the painted scrollback row stale by design. Termux height changes bypass the destructive redraw, so growing the terminal could pull such rows back into the viewport without a repaint. Track the lowest skipped row and invalidate the re-exposed cache entries so the differential path repaints them; every other height change full-renders anyway. --- packages/pi-tui/AGENTS.md | 2 +- packages/pi-tui/src/tui.ts | 26 ++++++++++++++++++ packages/pi-tui/test/tui-render.test.ts | 35 +++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/packages/pi-tui/AGENTS.md b/packages/pi-tui/AGENTS.md index e0101db82a..fe2e704c95 100644 --- a/packages/pi-tui/AGENTS.md +++ b/packages/pi-tui/AGENTS.md @@ -12,7 +12,7 @@ Never overwrite this directory wholesale when syncing from upstream. Each of the 4. **`src/components/text.ts` / `markdown.ts` / `truncated-text.ts` / `editor.ts` — negative-width `repeat` guards**: the `repeat` counts for blank lines, horizontal rules, and the editor's top/bottom borders are clamped to ≥ 0 (two editor border sites; markdown's emptyLine and hr — the hr site is currently unreachable from the render entry and is purely defensive). Guarding tests: the "negative width safety" cases — Text's lives in `test/tui-render.test.ts` (Text has no dedicated test file), Markdown's and TruncatedText's live in their own test files; the editor's is "does not throw at zero or negative widths" inside the "Editor narrow width rendering" group in `test/editor.test.ts`. 5. **`src/tui.ts` — per-frame processed-line reuse**: `doRender` keeps the previous frame's raw lines (`previousRawLines`), their processed output (`previousLines`), and per-line kitty image ids (`previousLineImageIds`). A line whose raw string is reference-identical to the previous frame's reuses its processed output (truncation + `normalizeTerminalOutput` + trailing `SEGMENT_RESET`) verbatim, so a steady-state frame costs O(total lines) pointer comparisons plus O(changed lines) real work instead of re-normalizing every line. Image-id consumers (`expandChangedRangeForKittyImages`, `deleteChangedKittyImages`, the frame-end id union) read the cached per-line ids rather than re-scanning line text; upstream has no such cache and re-processes every line every frame (upstream also has `applyLineResets`, which this divergence inlines into the processed-line build). Reuse is only valid when the terminal width is unchanged; width changes re-render everything at the new width so references never match. Guarding tests: "TUI steady-frame processed-line reuse" in `test/tui-render.test.ts`. -6. **`src/tui.ts` — skip destructive redraw for in-place changes above the viewport**: when every changed line sits at or above the previous viewport top, the line count is unchanged, and neither frame has kitty image ids in the above-viewport part of the changed range, `doRender` must NOT fall back to `fullRender(true)` (upstream clears screen + scrollback via ESC[2J/ESC[3J for any above-viewport change, which flickers and yanks scroll position on every above-viewport status tick). Entirely-above changes commit the line caches without painting (scrollback keeps a stale frame); changes spanning the boundary clamp the repaint range to the viewport top and continue on the differential path. Any layout shift or image involvement keeps upstream's destructive redraw. Guarding tests: "TUI above-viewport in-place changes" in `test/tui-render.test.ts`. +6. **`src/tui.ts` — skip destructive redraw for in-place changes above the viewport**: when every changed line sits at or above the previous viewport top, the line count is unchanged, and neither frame has kitty image ids in the above-viewport part of the changed range, `doRender` must NOT fall back to `fullRender(true)` (upstream clears screen + scrollback via ESC[2J/ESC[3J for any above-viewport change, which flickers and yanks scroll position on every above-viewport status tick). Entirely-above changes commit the line caches without painting (scrollback keeps a stale frame); changes spanning the boundary clamp the repaint range to the viewport top and continue on the differential path. Any layout shift or image involvement keeps upstream's destructive redraw. The lowest skipped row is tracked in `aboveViewportStaleTop`; when a Termux height increase re-exposes skipped rows (the one height-change path that avoids `fullRender`), the exposed cache entries are invalidated so the differential path repaints them. Guarding tests: "TUI above-viewport in-place changes" in `test/tui-render.test.ts`. ## Acceptance after syncing from upstream diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index 0b94b8933a..ae6dc8c93f 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -336,6 +336,7 @@ export class TUI extends Container { private clearOnShrink = process.env['PI_CLEAR_ON_SHRINK'] === "1"; // Clear empty rows when content shrinks (default: off) private maxLinesRendered = 0; // Track terminal's working area (max lines ever rendered) private previousViewportTop = 0; // Track previous viewport top for resize-aware cursor moves + private aboveViewportStaleTop: number | null = null; // Lowest row whose repaint was skipped above the viewport private fullRedrawCount = 0; private stopped = false; private pendingOsc11BackgroundReplies = 0; @@ -1287,6 +1288,24 @@ export class TUI extends Container { let prevViewportTop = heightChanged ? Math.max(0, previousBufferLength - height) : this.previousViewportTop; let viewportTop = prevViewportTop; let hardwareCursorRow = this.hardwareCursorRow; + + // A height increase can pull rows whose repaint was skipped (the + // above-viewport in-place path below) back into the viewport. On the + // Termux path, which deliberately avoids the destructive height-change + // redraw, invalidate the exposed rows in the caches so the diff + // repaints them; every other height change full-renders anyway. + if (heightChanged && this.aboveViewportStaleTop !== null && prevViewportTop < this.previousViewportTop) { + const exposedEnd = Math.min(this.previousViewportTop, this.previousLines.length); + for (let i = prevViewportTop; i < exposedEnd; i++) { + // NUL can never equal a rendered raw line or processed output, so + // both the reuse fast path and the diff see these rows as changed. + this.previousLines[i] = "\u0000"; + this.previousRawLines[i] = "\u0000"; + } + if (prevViewportTop <= this.aboveViewportStaleTop) { + this.aboveViewportStaleTop = null; + } + } const computeLineDiff = (targetRow: number): number => { const currentScreenRow = hardwareCursorRow - prevViewportTop; const targetScreenRow = targetRow - viewportTop; @@ -1346,6 +1365,7 @@ export class TUI extends Container { // Helper to clear scrollback and viewport and render all new lines const fullRender = (clear: boolean): void => { this.fullRedrawCount += 1; + this.aboveViewportStaleTop = null; let buffer = "\x1b[?2026h"; // Begin synchronized output if (clear) { buffer += this.deleteKittyImages(this.previousKittyImageIds); @@ -1547,6 +1567,12 @@ export class TUI extends Container { fullRender(true); return; } + // Remember the lowest skipped row: a later Termux height increase + // re-exposes such rows and must invalidate them (see doRender top). + this.aboveViewportStaleTop = + this.aboveViewportStaleTop === null + ? firstChanged + : Math.min(this.aboveViewportStaleTop, firstChanged); if (lastChanged < prevViewportTop) { // Entirely above the viewport: scrollback keeps a stale frame // of those rows (invisible from the live viewport), so only diff --git a/packages/pi-tui/test/tui-render.test.ts b/packages/pi-tui/test/tui-render.test.ts index 6ebd5e7cb1..e80b4e8436 100644 --- a/packages/pi-tui/test/tui-render.test.ts +++ b/packages/pi-tui/test/tui-render.test.ts @@ -1064,6 +1064,41 @@ describe("TUI above-viewport in-place changes", () => { tui.stop(); }); + it("repaints skipped rows that a Termux height increase re-exposes", async () => { + await withEnv({ TERMUX_VERSION: "1" }, async () => { + const terminal = new LoggingVirtualTerminal(40, 10); + const tui = new TUI(terminal); + const component = new TestComponent(); + tui.addChild(component); + component.lines = makeLines(30); + tui.start(); + await terminal.waitForRender(); + const initialRedraws = tui.fullRedraws; + + // An in-place tick above the viewport takes the skip path, leaving + // the on-screen scrollback row stale. + const lines = makeLines(30); + lines[15] = "spinner frame 0"; + component.lines = lines; + tui.requestRender(); + await terminal.waitForRender(); + terminal.clearWrites(); + + // Growing the terminal re-exposes rows 6-19. Termux height changes + // skip the destructive redraw, so the stale row must be repainted + // by the differential path instead. + terminal.resize(40, 24); + await terminal.waitForRender(); + + assert.strictEqual(tui.fullRedraws, initialRedraws, "Termux height change must not full-redraw"); + assert.ok(!terminal.getWrites().includes("\x1b[2J"), "must not clear the screen"); + assert.ok(!terminal.getWrites().includes("\x1b[3J"), "must not clear scrollback"); + const viewport = terminal.getViewport(); + assert.ok(viewport.join("\n").includes("spinner frame 0"), "re-exposed row shows the current content"); + tui.stop(); + }); + }); + it("keeps the full redraw when the line count changes above the viewport", async () => { const terminal = new LoggingVirtualTerminal(40, 10); const tui = new TUI(terminal);