From 8d631b063f10556f55c99abf650606cb9d9714cc Mon Sep 17 00:00:00 2001 From: liruifengv Date: Sat, 4 Jul 2026 16:52:40 +0800 Subject: [PATCH] fix(pi-tui): make the viewport anchor follow above-viewport content shifts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The anchor pins a buffer row index, but an above-viewport length change shifts the content living at every index below it. The pinned window then suddenly showed content further along (visible upward creep), and the rows that slid above the window top were lost: never committed to scrollback, which holds older bytes at those indices. During streaming, every above-viewport net shrink (a finished agent row collapsing, a merged step, a spinner line disappearing) permanently swallowed that many rows, and the blank area under the input box kept growing. doRender now scores two hypotheses for such frames — window stayed put vs window content shifted by the length delta — and when the shift explains the frame strictly better, moves the anchor with the content: a pure shift re-anchors with no painting at all (the screen already shows exactly that content), and a shift with local in-window changes (spinner/timer rows) repaints the window at the shifted anchor. Commit order stays continuous, so the exactly-once scrollback invariant is preserved: no loss, no duplication. Covered by e2e case07 (above-viewport shift), which fails on the previous revision; the stale-content unit test now asserts the follow-the-content window instead of the old swallowed-row behavior. --- .changeset/cli-above-viewport-shift.md | 5 + .changeset/pi-tui-above-viewport-shift.md | 5 + packages/pi-tui/e2e/README.md | 1 + .../e2e/case07-above-viewport-shift.test.ts | 96 +++++++++++++++++++ packages/pi-tui/src/tui.ts | 59 ++++++++++++ packages/pi-tui/test/tui-render.test.ts | 15 +-- 6 files changed, 174 insertions(+), 7 deletions(-) create mode 100644 .changeset/cli-above-viewport-shift.md create mode 100644 .changeset/pi-tui-above-viewport-shift.md create mode 100644 packages/pi-tui/e2e/case07-above-viewport-shift.test.ts diff --git a/.changeset/cli-above-viewport-shift.md b/.changeset/cli-above-viewport-shift.md new file mode 100644 index 0000000000..016bed54f7 --- /dev/null +++ b/.changeset/cli-above-viewport-shift.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix transcript rows vanishing and content creeping upward during streaming, which left a growing blank area under the input box. diff --git a/.changeset/pi-tui-above-viewport-shift.md b/.changeset/pi-tui-above-viewport-shift.md new file mode 100644 index 0000000000..9286e293b9 --- /dev/null +++ b/.changeset/pi-tui-above-viewport-shift.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/pi-tui": patch +--- + +Make the viewport anchor follow content when lines above the viewport are added or removed, instead of pinning a buffer row index that let the visible window drift and lose rows. diff --git a/packages/pi-tui/e2e/README.md b/packages/pi-tui/e2e/README.md index 3c07a24403..22dc23474a 100644 --- a/packages/pi-tui/e2e/README.md +++ b/packages/pi-tui/e2e/README.md @@ -52,6 +52,7 @@ Accepted trade-offs (by design, not bugs): | [case04](./case04-oscillation-duplication.test.ts) | Transcript spans duplicated in scrollback during streaming (shrink/grow oscillation) | Shrink re-anchor rewound the viewport anchor; the next grow re-committed rows scrollback already held | Fixed (#1353) | | [case05](./case05-growth-past-anchor-row-loss.test.ts) | Transcript rows missing from scrollback after streaming | Growth past the anchor with an above-viewport change repainted in place without scrolling, so the skipped rows never got committed (invariant 2) | Fixed (#1353, review) | | [case06](./case06-cursor-above-pinned-window.test.ts) | Rows written to the wrong screen position after a pinned shrink | `repaintViewport()` positioned the cursor to a logical row above the painted window; `hardwareCursorRow` desynced from the real cursor (invariant 3) | Fixed (#1353, review) | +| [case07](./case07-above-viewport-shift.test.ts) | Rows vanish and content creeps upward while streaming; blank area grows under the input box | The anchor pins a buffer row index; an above-viewport shrink shifts all content below it, so the pinned window jumped ahead and the rows sliding past its top were never committed | Fixed | A case for a not-yet-fixed bug is marked **Open** in the table and is expected to fail — it is the executable reproduction. Flip it to Fixed diff --git a/packages/pi-tui/e2e/case07-above-viewport-shift.test.ts b/packages/pi-tui/e2e/case07-above-viewport-shift.test.ts new file mode 100644 index 0000000000..7130e4638b --- /dev/null +++ b/packages/pi-tui/e2e/case07-above-viewport-shift.test.ts @@ -0,0 +1,96 @@ +import assert from "node:assert"; +import { describe, it } from "node:test"; +import { countInBuffer, createHarness } from "./harness.ts"; + +// Case 07 — rows vanish and content creeps upward while streaming. +// +// Symptom: during a long streaming turn, visible transcript rows kept +// disappearing and the remaining content crept upward, leaving an +// ever-growing blank area under the input box. Scrolling back showed +// gaps: the vanished rows were in neither the screen nor scrollback. +// +// Root cause: the viewport anchor pins a *buffer row index*, not +// content. When content ABOVE the viewport shrinks by k lines (a +// finished agent-group row collapsing, a merged step, a spinner line +// disappearing), every line below the shrink point shifts up by k, so +// the pinned window suddenly shows content k lines further down: the +// screen appears to jump up, and the k rows that slid above the window +// top are lost — scrollback at those indices holds older bytes, and the +// rows were never committed. Each above-viewport net shrink permanently +// swallows k rows. +// +// The fix makes the anchor follow content instead of row indices: when +// the frame is best explained as an above-viewport shift, the anchor +// moves with the shift (screen content stays put), and later growth +// commits rows in content order — no loss, no duplication. +// +// Fixed: the anchor follows the shift, so the screen stays put and later +// growth commits rows in content order. + +describe("e2e case07: above-viewport shrink must not shift or lose rows", () => { + it("keeps the visible window unchanged when lines above it are removed", async () => { + // 60 lines at height 10 -> anchor 50; screen shows row-50..row-58 + input. + const initial = [...Array.from({ length: 59 }, (_, i) => `row-${i}`), "[INPUT-BOX]"]; + const h = await createHarness(initial, { rows: 10 }); + const before = h.terminal.getViewport(); + assert.ok(before[0]!.includes("row-50"), "sanity: window starts at row-50"); + + // Remove 3 lines far above the viewport (row-10..row-12); everything + // below shifts up by 3, but the visible content is unchanged. + const shrunk = initial.filter((_, i) => i < 10 || i > 12); + await h.frame(shrunk); + + assert.deepStrictEqual( + h.terminal.getViewport(), + before, + "the visible window must not move when only above-viewport lines are removed", + ); + + // Growth afterwards must commit rows in content order: nothing lost, + // nothing duplicated. + const grown = [...shrunk.slice(0, -1), "tail-0", "tail-1", "tail-2", "[INPUT-BOX]"]; + await h.frame(grown); + + const buffer = h.terminal.getScrollBuffer(); + for (const marker of ["row-49", "row-50", "row-51", "row-52", "row-53"]) { + const count = countInBuffer(buffer, marker); + assert.strictEqual(count, 1, `"${marker}" must appear exactly once, got ${count}`); + } + const viewport = h.terminal.getViewport(); + assert.ok(viewport.some((line) => line.includes("tail-2")), "appended rows must be visible"); + assert.ok(viewport[9]!.includes("[INPUT-BOX]"), "input box must sit on the bottom row"); + + h.stop(); + }); + + it("applies the shift even when a live row inside the window also changed", async () => { + // Same shape, but the window contains a status line that changes in + // the same frame as the above-viewport shrink (spinner/timer tick). + const content = (status: string): string[] => [ + ...Array.from({ length: 58 }, (_, i) => `row-${i}`), + status, + "[INPUT-BOX]", + ]; + const h = await createHarness(content("status-A"), { rows: 10 }); + const before = h.terminal.getViewport(); + assert.ok(before.some((line) => line.includes("status-A")), "sanity: status visible"); + + // Remove 3 above-viewport lines AND tick the status line. + const shrunk = content("status-B").filter((_, i) => i < 10 || i > 12); + await h.frame(shrunk); + + const after = h.terminal.getViewport(); + assert.ok(after.some((line) => line.includes("status-B")), "status line must show the new value"); + // Every other visible row stays put (no upward creep). + for (let r = 0; r < 10; r++) { + if (before[r]!.includes("status-A")) continue; + assert.strictEqual( + after[r], + before[r], + `row ${r} must not move on an above-viewport shrink`, + ); + } + + h.stop(); + }); +}); diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index 8d6f168df1..ad92670394 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -1580,6 +1580,65 @@ export class TUI extends Container { // and skip above-viewport changes: scrollback keeps stale bytes, but the // user's scroll position is preserved. if (firstChanged < prevViewportTop) { + // The anchor pins a buffer row index, but an above-viewport length + // change shifts the content living at every index below it: the + // pinned window would suddenly show content |delta| rows further + // along (visible upward creep), and the rows that slid above the + // window top would be lost — never committed, and scrollback holds + // older bytes at those indices. Decide which hypothesis explains + // the frame better: "window stayed put" vs "window content shifted + // by the length delta". If the shift wins, move the anchor with + // the content: the screen stays put and later growth commits rows + // in content order (no loss, no duplication — scrollback already + // ends exactly where the shifted anchor begins). + const lengthDelta = newLines.length - this.previousLines.length; + const shiftedTop = prevViewportTop + lengthDelta; + if (lengthDelta !== 0 && shiftedTop >= 0) { + let pinnedDiffs = 0; + let shiftedDiffs = 0; + for (let r = 0; r < height; r++) { + const prevIdx = prevViewportTop + r; + const prevLine = prevIdx < this.previousLines.length ? this.previousLines[prevIdx]! : ""; + const pinnedLine = prevIdx < newLines.length ? newLines[prevIdx]! : ""; + if (prevLine !== pinnedLine) pinnedDiffs++; + const shiftedIdx = shiftedTop + r; + const shiftedLine = shiftedIdx < newLines.length ? newLines[shiftedIdx]! : ""; + if (prevLine !== shiftedLine) shiftedDiffs++; + } + if (shiftedDiffs < pinnedDiffs) { + if (shiftedDiffs === 0) { + // Pure shift: the visible window already shows exactly + // this content — re-anchor without painting anything. + logRedraw( + `anchor follows above-viewport shift (${this.previousLines.length} -> ${newLines.length}, viewportTop ${prevViewportTop} -> ${shiftedTop}, no repaint)`, + ); + this.hardwareCursorRow += lengthDelta; + this.cursorRow = Math.max(0, newLines.length - 1); + this.previousViewportTop = shiftedTop; + this.positionHardwareCursor(cursorPos, newLines.length, shiftedTop, height); + this.previousLines = newLines; + this.previousKittyImageIds = this.collectKittyImageIds(newLines); + this.previousWidth = width; + this.previousHeight = height; + return; + } + // Shift plus local in-window changes (spinner/timer rows): + // move the anchor with the content, then repaint the window + // there. Both anchor and cursor bookkeeping shift together, + // so the physical screen rows they map to are unchanged. + // (repaintViewport's kitty-image delete range still scans + // from the shifted index — a straddling image combined with + // a shift may be missed, but any image inside the paint + // range falls back to a destructive full render anyway.) + hardwareCursorRow += lengthDelta; + prevViewportTop = shiftedTop; + repaintViewport( + Math.max(desiredViewportTop, shiftedTop), + `anchor follows above-viewport shift (${this.previousLines.length} -> ${newLines.length}, viewportTop ${shiftedTop - lengthDelta} -> ${shiftedTop}, ${shiftedDiffs} in-window changes)`, + ); + return; + } + } let visibleFirstChanged = -1; for (let i = prevViewportTop; i <= lastChanged; i++) { const oldLine = i < this.previousLines.length ? this.previousLines[i] : ""; diff --git a/packages/pi-tui/test/tui-render.test.ts b/packages/pi-tui/test/tui-render.test.ts index d33bca4d89..7f5acc4188 100644 --- a/packages/pi-tui/test/tui-render.test.ts +++ b/packages/pi-tui/test/tui-render.test.ts @@ -755,11 +755,15 @@ describe("TUI differential rendering", () => { assert.ok(!line.includes("Chat 14"), `Stale "Chat 14" at viewport row ${i}`); } - // Partial shrinks keep the viewport anchor pinned (rewinding would - // duplicate scrollback rows), so the remaining content sits at the - // top of the window with blank rows below. The stale "Chat 12/13/14" - // rows remain in scrollback but are not visible. + // The chat shrink is an above-viewport length change: the anchor + // follows the shifted content, so the window keeps showing the same + // rows (editor intact, nothing swallowed) with blank rows below. + // The stale "Chat 12/13/14" bytes remain in scrollback but are not + // visible. assert.deepStrictEqual(viewport, [ + "Chat 10", + "Chat 11", + "Editor 0", "Editor 1", "Editor 2", "", @@ -767,9 +771,6 @@ describe("TUI differential rendering", () => { "", "", "", - "", - "", - "", ]); tui.stop();