Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pi-tui-above-viewport-inplace.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/tui-usage-flicker.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 3 additions & 1 deletion packages/pi-tui/AGENTS.md
Original file line number Diff line number Diff line change
@@ -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)

Expand All @@ -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. 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

- `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.
Expand Down
79 changes: 75 additions & 4 deletions packages/pi-tui/src/tui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1171,6 +1172,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<ReadonlyArray<number>>,
): 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 "";

Expand Down Expand Up @@ -1274,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;
Expand Down Expand Up @@ -1333,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);
Expand Down Expand Up @@ -1515,11 +1548,49 @@ 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;
}
// 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
// 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;
Comment on lines +1583 to +1584

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Repaint cached rows when the Termux viewport expands

When TERMUX_VERSION is set and the terminal grows after this path skips an above-viewport update, the newly exposed rows can show stale content. This branch records the unpainted lines in previousLines, while the existing Termux height-change path deliberately avoids fullRender; the next render therefore sees no diff and does not repaint rows that have just become visible. Retain the unpainted range or force it to be repainted when a height increase exposes it.

AGENTS.md reference: packages/pi-tui/AGENTS.md:L15-L15

Useful? React with 👍 / 👎.

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
Expand Down
131 changes: 131 additions & 0 deletions packages/pi-tui/test/tui-render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -990,3 +990,134 @@ 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("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);
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();
});
});