diff --git a/.changeset/cli-above-viewport-shift.md b/.changeset/cli-above-viewport-shift.md new file mode 100644 index 000000000..016bed54f --- /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 000000000..9286e293b --- /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 3c07a2440..22dc23474 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 000000000..7130e4638 --- /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 8d6f168df..ad9267039 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 d33bca4d8..7f5acc418 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();