diff --git a/src/tui-opentui/long-log.test.ts b/src/tui-opentui/long-log.test.ts deleted file mode 100644 index 410b6bde4..000000000 --- a/src/tui-opentui/long-log.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { describe, expect, test } from "bun:test" -import type { StreamRow } from "./stream" -import { - LONG_LOG_COLLAPSE_THRESHOLD, - LONG_LOG_WINDOW, - collapseMarker, - mustWindow, - windowSlice, -} from "./long-log" - -function rows(n: number): StreamRow[] { - return Array.from({ length: n }, (_, i) => ({ - role: "assistant" as const, - text: `line-${i}`, - })) -} - -describe("windowSlice", () => { - test("empty log", () => { - const w = windowSlice([]) - expect(w).toEqual({ - start: 0, - end: 0, - rows: [], - truncatedAbove: false, - truncatedBelow: false, - total: 0, - }) - }) - - test("follow-tail keeps last windowSize rows", () => { - const log = rows(50) - const w = windowSlice(log, { windowSize: 10 }) - expect(w.start).toBe(40) - expect(w.end).toBe(50) - expect(w.rows).toHaveLength(10) - expect(w.rows[0]?.text).toBe("line-40") - expect(w.rows[9]?.text).toBe("line-49") - expect(w.truncatedAbove).toBe(true) - expect(w.truncatedBelow).toBe(false) - expect(w.total).toBe(50) - }) - - test("short log is fully visible", () => { - const log = rows(5) - const w = windowSlice(log, { windowSize: 10 }) - expect(w.start).toBe(0) - expect(w.end).toBe(5) - expect(w.truncatedAbove).toBe(false) - expect(w.truncatedBelow).toBe(false) - }) - - test("pinIndex keeps historical row in window", () => { - const log = rows(100) - const w = windowSlice(log, { windowSize: 10, pinIndex: 5 }) - expect(w.start).toBeLessThanOrEqual(5) - expect(w.end).toBeGreaterThan(5) - expect(w.rows.some((r) => r.text === "line-5")).toBe(true) - expect(w.truncatedBelow).toBe(true) - }) - - test("pin at head", () => { - const log = rows(100) - const w = windowSlice(log, { windowSize: 10, pinIndex: 0 }) - expect(w.start).toBe(0) - expect(w.rows[0]?.text).toBe("line-0") - expect(w.truncatedAbove).toBe(false) - expect(w.truncatedBelow).toBe(true) - }) - - test("pin at tail", () => { - const log = rows(100) - const w = windowSlice(log, { windowSize: 10, pinIndex: 99 }) - expect(w.end).toBe(100) - expect(w.truncatedBelow).toBe(false) - }) -}) - -describe("mustWindow / budgets", () => { - test("threshold and default window are positive", () => { - expect(LONG_LOG_WINDOW).toBeGreaterThan(0) - expect(LONG_LOG_COLLAPSE_THRESHOLD).toBeGreaterThan(LONG_LOG_WINDOW) - }) - - test("mustWindow flips at collapse threshold", () => { - expect(mustWindow(LONG_LOG_COLLAPSE_THRESHOLD)).toBe(false) - expect(mustWindow(LONG_LOG_COLLAPSE_THRESHOLD + 1)).toBe(true) - }) -}) - -describe("collapseMarker", () => { - test("formats count", () => { - expect(collapseMarker(0)).toBe("") - expect(collapseMarker(1)).toBe("… 1 earlier line collapsed") - expect(collapseMarker(42)).toBe("… 42 earlier lines collapsed") - }) -}) - -describe("long-log smoke scale", () => { - test("multi-thousand slice is O(window) not O(total)", () => { - const log = rows(5000) - const t0 = performance.now() - const w = windowSlice(log, { windowSize: LONG_LOG_WINDOW }) - const ms = performance.now() - t0 - expect(w.rows).toHaveLength(LONG_LOG_WINDOW) - expect(w.truncatedAbove).toBe(true) - expect(w.total).toBe(5000) - // Pure slice should be well under a millisecond class budget on CI. - expect(ms).toBeLessThan(50) - }) -}) diff --git a/src/tui-opentui/long-log.ts b/src/tui-opentui/long-log.ts index 450bb9183..14856627e 100644 --- a/src/tui-opentui/long-log.ts +++ b/src/tui-opentui/long-log.ts @@ -1,29 +1,14 @@ /** - * Long-log window strategy — keep multi-thousand-line sessions interactive. - * Pure slice math; shell paints only the window, not the full history. - * - * Budget (Wave 6 defaults; CL-5399 may refine): - * - Painted window: last N rows (or pin around offset) - * - Collapse threshold: when history exceeds this, older rows stay in the - * model but drop from the render tree until scrolled into the window + * Retention budget for a long-running transcript. The paint tree tracks + * `streamLog` 1:1 (see shell.ts's repaintTranscriptWindow/paintAppendStreamRow) + * so every retained row stays reachable by scrolling; this cap is what keeps + * that array — and so the paint tree — bounded over a long session. */ -import type { StreamRow } from "./stream.js" - -/** Rows kept in the paint tree under normal follow-tail. */ -export const LONG_LOG_WINDOW = 200 - -/** - * When total rows exceed this, append/scroll paths must use windowSlice - * (never re-paint the full history). - */ -export const LONG_LOG_COLLAPSE_THRESHOLD = 500 - /** * Retained tail of a stream log. Display-only state — the agent's own context * is kept separately — but an unbounded array still costs memory and O(n) - * snapshot/diff work on every append over a long, tool-heavy session. Set - * above the collapse threshold so eviction never fights the paint window. + * snapshot/diff work on every append over a long, tool-heavy session. */ export const MAX_RETAINED_STREAM_ROWS = 600 @@ -31,90 +16,3 @@ export const MAX_RETAINED_STREAM_ROWS = 600 export function retentionOverflow(length: number): number { return Math.max(0, length - MAX_RETAINED_STREAM_ROWS) } - -export type LongLogWindow = { - /** Inclusive start index into the full row log. */ - readonly start: number - /** Exclusive end index. */ - readonly end: number - /** Slice of rows to paint. */ - readonly rows: readonly StreamRow[] - /** True when older rows exist above the window. */ - readonly truncatedAbove: boolean - /** True when newer rows exist below the window (pinned). */ - readonly truncatedBelow: boolean - /** Full log length. */ - readonly total: number -} - -export type WindowSliceOpts = { - /** Max rows to include (default LONG_LOG_WINDOW). */ - readonly windowSize?: number - /** - * Pin the window so this index is visible (keep-active-visible style). - * When omitted, follow the tail (last windowSize rows). - */ - readonly pinIndex?: number -} - -/** - * Compute which rows to paint for a long log. - * Follow-tail by default; pinIndex keeps a historical row in view. - */ -export function windowSlice( - log: readonly StreamRow[], - opts?: WindowSliceOpts, -): LongLogWindow { - const total = log.length - const windowSize = Math.max(1, Math.floor(opts?.windowSize ?? LONG_LOG_WINDOW)) - - if (total === 0) { - return { - start: 0, - end: 0, - rows: [], - truncatedAbove: false, - truncatedBelow: false, - total: 0, - } - } - - let end: number - let start: number - - if (opts?.pinIndex !== undefined) { - const pin = Math.max(0, Math.min(total - 1, Math.floor(opts.pinIndex))) - // Center-ish: keep pin in window; prefer showing context after pin when possible. - start = Math.max(0, pin - Math.floor(windowSize / 2)) - end = Math.min(total, start + windowSize) - start = Math.max(0, end - windowSize) - } else { - // Follow tail - end = total - start = Math.max(0, total - windowSize) - } - - return { - start, - end, - rows: log.slice(start, end), - truncatedAbove: start > 0, - truncatedBelow: end < total, - total, - } -} - -/** Whether the log is large enough that windowing is mandatory. */ -export function mustWindow(totalRows: number): boolean { - return totalRows > LONG_LOG_COLLAPSE_THRESHOLD -} - -/** - * Collapse marker line for the paint tree when truncatedAbove. - * Pure string — shell styles it as system chrome. - */ -export function collapseMarker(above: number): string { - const n = Math.max(0, Math.floor(above)) - if (n <= 0) return "" - return `… ${n} earlier line${n === 1 ? "" : "s"} collapsed` -} diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 75b570e5d..83312f485 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -110,13 +110,7 @@ import { visibleSlice, type ListViewportState, } from "./list-viewport.js" -import { - LONG_LOG_WINDOW, - collapseMarker, - mustWindow, - retentionOverflow, - windowSlice, -} from "./long-log.js" +import { retentionOverflow } from "./long-log.js" import { DEFAULT_PALETTE_COMMANDS, filterPaletteCommands, @@ -1989,34 +1983,67 @@ function labelBefore(shell: AppShell, index: number): string | null { return blockLabel(rowBefore(shell, index), row, transcriptRowLayout(shell)) } -/** Paint + push onto the visible streamLog (child while observing, parent otherwise). */ +/** + * Notice painted above the oldest retained row once the cap has evicted + * anything. Unlike the pre-CL-5551 collapse marker it replaces, scrolling + * never reveals more — these rows are gone, not merely out of the window. + */ +function evictedRowsNotice(evicted: number): string { + return ` … ${evicted} earlier row${evicted === 1 ? "" : "s"} dropped (past the retention limit)` +} + +/** + * Paint + push onto the visible streamLog (child while observing, parent + * otherwise). The paint tree stays 1:1 with the (retention-capped) log — + * CL-5551 already bounds `streamLog` to `MAX_RETAINED_STREAM_ROWS`, so there + * is no separate, smaller window to maintain on top of it: every retained + * row gets a node, which is also what makes all of it reachable by + * scrolling (CL-5553). A trim past the cap costs one node removal here, not + * a rebuild. + */ function paintAppendStreamRow(shell: AppShell, row: StreamRow): void { clearLandingMark(shell) const gainedVoice = noteAgentVoice(shell, row) shell.streamLog.push(row) + const baseBefore = shell.streamLogBase shell.streamLogBase = trimRetainedLog(shell.streamLog, shell.streamLogBase) shell.lineCount = shell.streamLog.length - // Under collapse threshold: append one paint node (cheap). - // Over threshold: rebuild the windowed paint tree only. The retention cap - // sits above the collapse threshold, so a trim never lands here — by the - // time eviction starts, appends are already windowed. - if (!gainedVoice && !mustWindow(shell.streamLog.length)) { - const index = shell.streamLog.length - 1 - shell.transcript.add( - createStreamRowRenderable( - shell, - row, - gapBefore(shell, index), - labelBefore(shell, index), - shell.streamLogBase + index, - ), - ) + if (gainedVoice) { + repaintTranscriptWindow(shell) paintChrome(shell) return } - repaintTranscriptWindow(shell) + const dropped = shell.streamLogBase - baseBefore + if (dropped > 0) { + for (const evicted of transcriptRowChildren(shell).slice(0, dropped)) { + shell.transcript.remove(evicted) + destroySubtree(evicted) + } + const marker = transcriptMarker(shell) + if (marker instanceof TextRenderable) { + marker.content = evictedRowsNotice(shell.streamLogBase) + } else { + const node = new TextRenderable(shell.renderer as CliRenderer, { + content: evictedRowsNotice(shell.streamLogBase), + fg: UI.textDim, + }) + evictionMarkers.add(node) + shell.transcript.add(node, 1) + } + } + + const index = shell.streamLog.length - 1 + shell.transcript.add( + createStreamRowRenderable( + shell, + row, + gapBefore(shell, index), + labelBefore(shell, index), + shell.streamLogBase + index, + ), + ) paintChrome(shell) } @@ -2069,14 +2096,39 @@ export function truncateStreamRows(shell: AppShell, length: number): void { paintChrome(shell) } +/** + * Identifies a transcript child as the eviction notice rather than a row. + * Identity, not position or state, is the source of truth: `streamLogBase` + * flips to nonzero the instant a trim happens, one step before the notice + * node itself exists in the paint tree, so deriving "is there a marker" + * from state would misalign row indices for exactly that transitional call. + */ +const evictionMarkers = new WeakSet() + /** * Row-index code paths (below, and the two windowed-rebuild callers) treat * `getChildren()` as a 1:1 array with `streamLog`. The leading bottom-anchor - * spacer (see `transcriptSpacers`) breaks that at index 0, so every consumer - * that needs the row-only view goes through here rather than the raw call. + * spacer (see `transcriptSpacers`) and, once retention has evicted anything, + * the eviction notice above the oldest retained row both break that — every + * consumer that needs the row-only view goes through here rather than the + * raw call. */ function transcriptRowChildren(shell: AppShell): readonly BaseRenderable[] { - return shell.transcript.getChildren().slice(1) + const children = shell.transcript.getChildren().slice(1) + return children.length > 0 && evictionMarkers.has(children[0]!) + ? children.slice(1) + : children +} + +/** The eviction-notice node, if the retention cap has dropped anything. */ +function transcriptMarker(shell: AppShell): BaseRenderable | undefined { + const children = shell.transcript.getChildren().slice(1) + return children.length > 0 && evictionMarkers.has(children[0]!) ? children[0] : undefined +} + +/** Raw child-list offset before the first row: the spacer, plus the notice if present. */ +function transcriptRowOffset(shell: AppShell): number { + return transcriptMarker(shell) === undefined ? 1 : 2 } /** @@ -2108,8 +2160,8 @@ export function replaceStreamRowAt( const children = transcriptRowChildren(shell) // A raw appendTranscript line breaks the 1:1 node↔row mapping; fall back to - // the windowed rebuild, which derives every node from the log. - if (mustWindow(shell.streamLog.length) || children.length !== shell.streamLog.length) { + // a full repaint, which derives every node from the log. + if (children.length !== shell.streamLog.length) { repaintTranscriptWindow(shell) paintChrome(shell) return @@ -2124,11 +2176,12 @@ export function replaceStreamRowAt( shell.transcript.remove(stale) destroySubtree(stale) } - // +1: index 0 in the transcript's own child list is the bottom-anchor - // spacer, not a row (see `transcriptRowChildren`). + // Raw child list is spacer (+ eviction notice, if any) then rows; see + // `transcriptRowOffset` (see `transcriptRowChildren` for why row 0 is not + // simply index 1). shell.transcript.add( createStreamRowRenderable(shell, row, gapBefore(shell, local), labelBefore(shell, local), index), - local + 1, + local + transcriptRowOffset(shell), ) paintChrome(shell) } @@ -2218,28 +2271,34 @@ function retextStreamRowBody( return true } -/** Rebuild transcript paint tree from the long-log window (O(window), not O(total)). */ +/** + * Rebuild the transcript paint tree from `streamLog` — every retained row, + * not a smaller window of it. `streamLog` is already capped at + * `MAX_RETAINED_STREAM_ROWS`, so this is O(cap), and painting all of it is + * what makes the full retained history reachable by scrolling. + */ export function repaintTranscriptWindow(shell: AppShell): void { clearLandingMark(shell) shell.agentVoices = new Set(agentVoicesIn(shell.streamLog)) - // The bottom-anchor spacer (index 0) stays; only row nodes get torn down. - const children = transcriptRowChildren(shell) - for (const child of [...children]) { + // The bottom-anchor spacer (index 0) stays; the eviction notice (if any) + // and every row get torn down and rebuilt from the log. + for (const child of shell.transcript.getChildren().slice(1)) { shell.transcript.remove(child) destroySubtree(child) } - const win = windowSlice(shell.streamLog, { windowSize: LONG_LOG_WINDOW }) - if (win.truncatedAbove) { - shell.transcript.add( - new TextRenderable(shell.renderer as CliRenderer, { - content: ` ${collapseMarker(win.start)}`, - fg: UI.textDim, - }), - ) + // Rows evicted by the retention cap are gone for good, not just scrolled + // past — say so, or the boundary reads as the true start of history. + if (shell.streamLogBase > 0) { + const marker = new TextRenderable(shell.renderer as CliRenderer, { + content: evictedRowsNotice(shell.streamLogBase), + fg: UI.textDim, + }) + evictionMarkers.add(marker) + shell.transcript.add(marker) } - win.rows.forEach((row, offset) => { - const local = win.start + offset + + shell.streamLog.forEach((row, local) => { shell.transcript.add( createStreamRowRenderable( shell, diff --git a/src/tui-opentui/transcript-long-log-scroll.test.ts b/src/tui-opentui/transcript-long-log-scroll.test.ts new file mode 100644 index 000000000..126bf9543 --- /dev/null +++ b/src/tui-opentui/transcript-long-log-scroll.test.ts @@ -0,0 +1,158 @@ +/** + * CL-5553: the full retained transcript (bounded by CL-5551's + * MAX_RETAINED_STREAM_ROWS) has to stay reachable by scrolling, and + * appending a new row must not rebuild the whole paint tree to do it. + */ +import { describe, expect, test } from "bun:test" +import { withTestRenderer } from "./harness" +import { appendStreamRow, createAppShell, replaceStreamRowAt, streamRowCount } from "./shell" +import { MAX_RETAINED_STREAM_ROWS } from "./long-log" + +async function settle(h: { renderOnce: () => Promise }): Promise { + // Markdown highlighting and viewport culling both settle a frame or two + // after the triggering mutation, not within it. + for (let i = 0; i < 5; i++) { + await new Promise((resolve) => setTimeout(resolve, 10)) + await h.renderOnce() + } +} + +describe("long-log transcript scrolling", () => { + test("scrolling to the top reaches the oldest retained row, not just the last 200", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + }) + try { + const total = MAX_RETAINED_STREAM_ROWS + 50 + for (let i = 0; i < total; i++) { + appendStreamRow(shell, { role: "assistant", text: `row-${i}` }) + } + await settle(h) + + shell.transcript.scrollTop = 0 + await settle(h) + const frame = h.captureCharFrame() + + // Rows 0-49 were evicted by the retention cap — gone by design, not + // a scrolling bug. Row 50 is the oldest still-retained row and sits + // 450 rows above the old 200-row paint window; it must be reachable + // in one scroll rather than staying stranded behind a collapsed marker. + expect(frame).toContain("row-50") + expect(frame).not.toContain("row-49") + // The boundary says rows were dropped rather than reading as the + // true start of history — eviction is permanent, unlike the old + // collapse marker, so scrolling further will never reveal row 0. + expect(frame).toContain("50 earlier rows dropped") + } finally { + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) + + test("appending past the retention cap evicts at most one painted node, not the whole window", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + }) + try { + for (let i = 0; i < MAX_RETAINED_STREAM_ROWS; i++) { + appendStreamRow(shell, { role: "assistant", text: `row-${i}` }) + } + await settle(h) + + let removed = 0 + const originalRemove = shell.transcript.remove.bind(shell.transcript) + shell.transcript.remove = (child) => { + removed += 1 + return originalRemove(child) + } + + // Now at the cap: this append evicts exactly one row from the front. + appendStreamRow(shell, { role: "assistant", text: "one-more" }) + await settle(h) + + // Before this fix, repaintTranscriptWindow ran on every append once + // the log passed LONG_LOG_COLLAPSE_THRESHOLD (500): it tore down every + // existing child and repainted windowSlice's LONG_LOG_WINDOW (200) rows + // from scratch — 200 removals for this one append, every append after. + expect(removed).toBe(1) + } finally { + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) + + test("the eviction notice updates in place across repeated evictions, not by teardown", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + }) + try { + for (let i = 0; i < MAX_RETAINED_STREAM_ROWS + 3; i++) { + appendStreamRow(shell, { role: "assistant", text: `row-${i}` }) + } + await settle(h) + shell.transcript.scrollTop = 0 + await settle(h) + expect(h.captureCharFrame()).toContain("3 earlier rows dropped") + + appendStreamRow(shell, { role: "assistant", text: "one-more" }) + await settle(h) + shell.transcript.scrollTop = 0 + await settle(h) + expect(h.captureCharFrame()).toContain("4 earlier rows dropped") + } finally { + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) + + test("replaceStreamRowAt stays a single-node retext after eviction has started", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + }) + try { + for (let i = 0; i < MAX_RETAINED_STREAM_ROWS + 5; i++) { + appendStreamRow(shell, { role: "assistant", text: `row-${i}` }) + } + await settle(h) + + let removed = 0 + const originalRemove = shell.transcript.remove.bind(shell.transcript) + shell.transcript.remove = (child) => { + removed += 1 + return originalRemove(child) + } + + // Streaming token updates hit this path on every delta; past the + // retention cap it must still touch one node, not the eviction + // notice's presence forcing a full repaintTranscriptWindow. + const lastIndex = streamRowCount(shell) - 1 + replaceStreamRowAt(shell, lastIndex, { role: "assistant", text: "edited" }) + await settle(h) + + expect(removed).toBeLessThanOrEqual(1) + } finally { + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) +}) diff --git a/src/tui-opentui/wave6.test.ts b/src/tui-opentui/wave6.test.ts index dc7ff3cbf..06fb12387 100644 --- a/src/tui-opentui/wave6.test.ts +++ b/src/tui-opentui/wave6.test.ts @@ -5,12 +5,7 @@ import { describe, expect, test } from "bun:test" import { IDLE_TRANSCRIPT_FLOOR } from "./geometry/index" import { focusOwner, scrollLease } from "./focus/index" import { withTestRenderer } from "./harness" -import { - LONG_LOG_COLLAPSE_THRESHOLD, - LONG_LOG_WINDOW, - MAX_RETAINED_STREAM_ROWS, - mustWindow, -} from "./long-log" +import { MAX_RETAINED_STREAM_ROWS } from "./long-log" import { openPermissionsOverlay } from "./overlays" import { acceptOverlaySelection, @@ -146,7 +141,7 @@ describe("Wave 6: command palette", () => { }) describe("Wave 6: long-log windowing", () => { - test("multi-thousand append stays interactive (windowed paint)", async () => { + test("multi-thousand append stays interactive (full-retained-log paint)", async () => { await withTestRenderer( async (h) => { const shell = createAppShell(h.renderer, { @@ -154,8 +149,8 @@ describe("Wave 6: long-log windowing", () => { wireKeys: false, }) try { - const n = LONG_LOG_COLLAPSE_THRESHOLD + 50 - expect(mustWindow(n)).toBe(true) + // Below MAX_RETAINED_STREAM_ROWS: no eviction, so painted == n + 1 below holds. + const n = MAX_RETAINED_STREAM_ROWS - 50 const t0 = performance.now() for (let i = 0; i < n; i++) { @@ -178,11 +173,12 @@ describe("Wave 6: long-log windowing", () => { expect(shell.streamLog.length).toBe(n) expect(shell.lineCount).toBe(n) - // Paint tree is windowed, not full history. + // Paint tree tracks the full retained log 1:1 (CL-5553) — capped at + // MAX_RETAINED_STREAM_ROWS by CL-5551, not a smaller paint window, + // so every retained row stays reachable by scrolling. const painted = shell.transcript.getChildren().length - // collapse marker + window rows - expect(painted).toBeLessThanOrEqual(LONG_LOG_WINDOW + 2) - expect(painted).toBeGreaterThan(0) + expect(painted).toBeLessThanOrEqual(MAX_RETAINED_STREAM_ROWS + 1) + expect(painted).toBe(n + 1) // +1: bottom-anchor spacer // Smoke: no multi-second peg on append storm expect(elapsed).toBeLessThan(5_000)