Merge pull request #1461 from corbitsdev/cl-10642-tui-adopt-scrollbox-full-api-scrollbar-delete-scroll-hacks

feat(tui): scroll the transcript and overlay through ScrollBox

Author: TheGreatAxios

src/tui/allow-once-reprompt.test.ts

@@ -484,7 +484,7 @@ describe("CL-8792 overlay host: suspend preserves the surface instead of dismiss
       openSlash(shell);
       const list = defined(shell.overlayList, "slash list");
       expect(list.select.isDestroyed).toBe(false);
-      expect(list.select.parent).toBe(shell.overlayView.body);
+      expect(list.viewport.parent).toBe(shell.overlayView.body);
 
       let resolved: unknown;
       emitter.emit("permission.gate", {
@@ -501,8 +501,8 @@ describe("CL-8792 overlay host: suspend preserves the surface instead of dismiss
       expect(shell.overlayKind).toBe("help");
       expect(shell.overlayList).toBe(list);
       expect(list.select.isDestroyed).toBe(false);
-      expect(list.select.parent).toBe(shell.overlayView.body);
-      expect(shell.overlayView.body.getChildren()).toContain(list.select);
+      expect(list.viewport.parent).toBe(shell.overlayView.body);
+      expect(shell.overlayView.body.getChildren()).toContain(list.viewport);
     });
   });
 

src/tui/decision-truncation.test.ts

@@ -41,8 +41,12 @@ const hintRequest: PermissionRequest = {
 function bodySelect(
   view: ReturnType<typeof createOverlayView>,
 ): SelectRenderable {
+  // The list lives in the host ScrollBox, not as a direct body child.
   const found = view.body
     .getChildren()
+    .flatMap((row) =>
+      row instanceof SelectRenderable ? [row] : row.getChildren(),
+    )
     .find((row) => row instanceof SelectRenderable);
   if (!(found instanceof SelectRenderable))
     throw new Error("expected the overlay list");

src/tui/overlay-paint.test.ts

@@ -83,15 +83,18 @@ async function paintOverlay(
 /**
  * Every interior row must be blank or exactly one expected overlay row; two
  * renderables sharing cells produces a hybrid string that matches nothing.
+ * A native ScrollBar thumb occupies the last column when the list overflows;
+ * that glyph is not a second copy of the row.
  */
 function expectCleanInterior(
   interior: readonly string[],
   expected: readonly string[],
 ): void {
   const allowed = new Set(expected.map((e) => e.trimEnd()));
   for (const row of interior) {
-    if (row.trim().length === 0) continue;
-    expect(allowed.has(row)).toBe(true);
+    const withoutBar = row.replace(/[▀▄█▌▐░▒▓]+$/u, "").trimEnd();
+    if (withoutBar.length === 0) continue;
+    expect(allowed.has(withoutBar)).toBe(true);
   }
 }
 
@@ -125,7 +128,8 @@ describe("overlay host never shares cells with the prompt border", () => {
       expectCleanInterior(interior.slice(1), expected);
 
       // The selected row must be intact, not overwritten by the model label.
-      expect(interior).toContain(` ${ITEMS[0]}`);
+      // A ScrollBar thumb may sit on the same line, so match the label prefix.
+      expect(interior.some((row) => row.startsWith(` ${ITEMS[0]}`))).toBe(true);
       for (const row of interior) {
         expect(row.includes(MODEL_LABEL)).toBe(false);
         expect(row.includes("alice")).toBe(false);

src/tui/overlay-view.test.ts

@@ -33,8 +33,12 @@ function bodyRows(view: ReturnType<typeof createOverlayView>): string[] {
 function bodySelect(
   view: ReturnType<typeof createOverlayView>,
 ): SelectRenderable {
+  // The list lives in the host ScrollBox, not as a direct body child.
   const found = view.body
     .getChildren()
+    .flatMap((row) =>
+      row instanceof SelectRenderable ? [row] : row.getChildren(),
+    )
     .find((row) => row instanceof SelectRenderable);
   if (!(found instanceof SelectRenderable))
     throw new Error("expected the overlay list");

src/tui/overlay-view.ts

@@ -373,7 +373,8 @@ export function createOverlayView(ctx: RenderContext) {
       name: line,
       description: "",
     }));
-    body.add(list.select);
+    body.add(list.viewport);
+    list.scrollActiveIntoView();
   }
 
   /** Rule + two-line description zone, painted when `describe` is set. */
@@ -414,11 +415,11 @@ export function createOverlayView(ctx: RenderContext) {
   }
 
   /**
-   * Detach the select before `clearBody` destroys the body's children — the
-   * list owns it across paints.
+   * Detach the list viewport before `clearBody` destroys the body's children —
+   * the list owns the select inside that ScrollBox across paints.
    */
   function detachList(list: OverlayList): void {
-    if (list.select.parent === body) body.remove(list.select);
+    if (list.viewport.parent) body.remove(list.viewport);
   }
 
   function paintList(
@@ -455,10 +456,11 @@ export function createOverlayView(ctx: RenderContext) {
     });
     // An empty list renders nothing: the renderable would claim a background
     // row the chooser did not reserve.
-    if (presentation.items.length > 0) body.add(list.select);
+    if (presentation.items.length > 0) body.add(list.viewport);
     else addOverlayRow(` ${OVERLAY_EMPTY_STATE}`, UI.textDim);
     paintAnswerRow(presentation.answer, contentWidth);
     paintDescriptionZone(presentation.describe, contentWidth);
+    list.scrollActiveIntoView();
   }
 
   function paintTitle(

src/tui/shell.test.ts

@@ -125,6 +125,70 @@ describe("createAppShell", () => {
     );
   });
 
+  test("one wheel detent survives a same-height content size pass", async () => {
+    await withTestRenderer(
+      async (h) => {
+        const shell = createAppShell(h.renderer, {
+          terminal: { columns: 80, rows: 24 },
+          wireKeys: false,
+        });
+        try {
+          for (let i = 0; i < 50; i++) {
+            appendTranscript(shell, `seed-${i}`);
+          }
+          await h.renderOnce();
+          const max = Math.max(
+            0,
+            shell.transcript.scrollHeight - shell.transcript.height,
+          );
+          expect(max).toBeGreaterThan(1);
+          const scrollHeight = shell.transcript.scrollHeight;
+          shell.transcript.scrollTop = max - 1;
+          shell.transcript.content.onSizeChange?.();
+          expect(shell.transcript.scrollHeight).toBe(scrollHeight);
+          expect(shell.transcript.scrollTop).toBe(max - 1);
+          expect(isTranscriptFollowing(shell)).toBe(false);
+        } finally {
+          shell.dispose();
+        }
+      },
+      { width: 80, height: 24 },
+    );
+  });
+
+  test("one wheel detent up from a followed tail stays off the bottom", async () => {
+    await withTestRenderer(
+      async (h) => {
+        const shell = createAppShell(h.renderer, {
+          terminal: { columns: 80, rows: 24 },
+          wireKeys: false,
+        });
+        try {
+          for (let i = 0; i < 50; i++) {
+            appendTranscript(shell, `seed-${i}`);
+          }
+          await h.renderOnce();
+          const max = Math.max(
+            0,
+            shell.transcript.scrollHeight - shell.transcript.height,
+          );
+          expect(max).toBeGreaterThan(1);
+          expect(isTranscriptFollowing(shell)).toBe(true);
+
+          // One detent, not a multi-tick fling. The next frame must not
+          // treat max - 1 as the tail and scroll it back.
+          shell.transcript.scrollTop = max - 1;
+          await h.renderOnce();
+          expect(shell.transcript.scrollTop).toBe(max - 1);
+          expect(isTranscriptFollowing(shell)).toBe(false);
+        } finally {
+          shell.dispose();
+        }
+      },
+      { width: 80, height: 24 },
+    );
+  });
+
   test("the notice says pinned only while the tail is off screen", async () => {
     await withTestRenderer(
       async (h) => {

src/tui/shell/chrome.ts

@@ -1047,23 +1047,33 @@ export function syncPromptRows(shell: AppShell): void {
 }
 
 /**
- * Resize the leading filler to soak leftover viewport space (`scrollHeight`
- * minus filler). Not at row-mutation time: mid-layout rows read short for a
- * frame, so a filler grown then would bury one. The render-frame hook calls
- * it after that pass.
+ * Pin a short transcript to the prompt. ScrollBox content is already at least
+ * the viewport tall (`minHeight: "100%"`); justify-end packs the rows against
+ * the bottom. The landing positions its own mark, so flex-end there would
+ * shove it twice. The index-0 spacer stays at height 0 — growing it was the
+ * old anchor.
  */
+const transcriptJustify = new WeakMap<AppShell, "flex-start" | "flex-end">();
+
 export function syncTranscriptSpacer(shell: AppShell): void {
   const spacer = transcriptSpacers.get(shell);
   if (spacer === undefined) return;
-  // The landing already bottom-anchors its mark via the above/below split; a
-  // filler would double-count and squeeze it.
-  if (isLanding(shell)) {
-    if (spacer.height !== 0) spacer.height = 0;
-    return;
-  }
-  const rowsHeight = Math.max(0, shell.transcript.scrollHeight - spacer.height);
-  const nextHeight = Math.max(0, shell.transcript.height - rowsHeight);
-  if (spacer.height !== nextHeight) spacer.height = nextHeight;
+  if (spacer.height !== 0) spacer.height = 0;
+  const justify = isLanding(shell) ? "flex-start" : "flex-end";
+  if (transcriptJustify.get(shell) === justify) return;
+  shell.transcript.contentOptions = { justifyContent: justify };
+  transcriptJustify.set(shell, justify);
+}
+
+/** Keep the newest row in view while the tail is followed. */
+export function followTranscriptTail(shell: AppShell): void {
+  if (isLanding(shell) || !isTranscriptFollowing(shell)) return;
+  const last = shell.transcript.getChildren().at(-1);
+  if (last === undefined || last.id === "shell-transcript-spacer") return;
+  if (last.height <= 0) return;
+  const viewportY = shell.transcript.viewport.y;
+  if (last.y === 0 && viewportY !== 0) return;
+  shell.transcript.scrollChildIntoView(last.id);
 }
 
 export function relayout(shell: AppShell, opts?: RelayoutOpts): GeometryLayout {
@@ -1381,7 +1391,7 @@ export function replaceStreamRowAt(
 export function repaintTranscriptWindow(shell: AppShell): void {
   clearLandingMark(shell);
   shell.agentVoices = new Set(agentVoicesIn(shell.streamLog));
-  // The bottom-anchor spacer (index 0) stays; the eviction notice (if any)
+  // Index 0 is the zero-height spacer and 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);

src/tui/shell/index.ts

@@ -34,6 +34,7 @@ import { flushStreamRowUpdates } from "../runtime-bridge.js";
 import {
   type AppShell,
   type AppShellOptions,
+  bindTranscriptTailSticky,
   EMPTY_PRIMARY_BINDINGS,
   flashTimers,
   initShellInternals,
@@ -66,6 +67,7 @@ import {
   syncNoticeAfterLayout,
   syncPromptRows,
   syncTranscriptSpacer,
+  followTranscriptTail,
 } from "./chrome.js";
 import {
   clearPendingAttachments,
@@ -202,6 +204,7 @@ export function createAppShell(
     stickyScroll: true,
     stickyStart: "bottom",
     scrollY: true,
+    viewportCulling: true,
     focusable: true,
     rootOptions: { backgroundColor: UI.ground },
     contentOptions: { backgroundColor: UI.ground },
@@ -211,12 +214,14 @@ export function createAppShell(
   // row restating it.
   transcript.verticalScrollBar.visible = false;
   transcript.horizontalScrollBar.visible = false;
+  bindTranscriptTailSticky(transcript);
   // Markdown links live on nodes we do not own; this bubbling handler arms
   // them (armed rows stop propagation, so a click opens exactly once).
   armMarkdownLinks(transcript, ctx);
 
-  // Leading filler that bottom-anchors a short transcript; see
-  // `syncTranscriptSpacer`. Zero height until the first sync call.
+  // Index 0 stays a zero-height spacer so row offsets do not move. Short
+  // transcripts bottom-align through the scroll content (see
+  // `syncTranscriptSpacer`), not by growing this filler.
   const transcriptSpacer = new BoxRenderable(ctx, {
     id: "shell-transcript-spacer",
     width: "100%",
@@ -388,6 +393,7 @@ export function createAppShell(
     // After a natural render, not at mutation time: the row's box needs a
     // layout pass to size itself, and claiming the padding first starves it.
     syncTranscriptSpacer(shell);
+    followTranscriptTail(shell);
     syncNoticeAfterLayout(shell);
   };
 

src/tui/shell/internals.ts

@@ -498,11 +498,39 @@ export type PrimaryOverlayKind =
   | "mcp"
   | "plugin_credentials";
 
-/** Whether the transcript viewport is stuck to the bottom (FOLLOW vs PINNED). */
+/** True when the viewport is on the tail (`scrollTop >= max`). */
 export function isTranscriptFollowing(shell: AppShell): boolean {
   const { transcript } = shell;
   const max = Math.max(0, transcript.scrollHeight - transcript.height);
-  return transcript.scrollTop >= max - 1;
+  return transcript.scrollTop >= max;
+}
+
+/** Bottom sticky stays armed only on the tail, so one detent up stays pinned. */
+export function bindTranscriptTailSticky(
+  transcript: ScrollBoxRenderable,
+): void {
+  const desc = Object.getOwnPropertyDescriptor(
+    Object.getPrototypeOf(transcript),
+    "scrollTop",
+  );
+  const get = desc?.get;
+  const set = desc?.set;
+  if (get === undefined || set === undefined) {
+    throw new Error("ScrollBox scrollTop accessor is missing");
+  }
+  const sync = (): void => {
+    const max = Math.max(0, transcript.scrollHeight - transcript.height);
+    transcript.stickyScroll = transcript.scrollTop >= max;
+  };
+  Object.defineProperty(transcript, "scrollTop", {
+    configurable: true,
+    enumerable: true,
+    get: () => get.call(transcript),
+    set: (value: number) => {
+      set.call(transcript, value);
+      sync();
+    },
+  });
 }
 
 /** Sticky-scroll mode label (surfaced on the notice row only when PINNED). */
@@ -546,6 +574,8 @@ export interface OverlayListRange {
  * rebuilds on geometry change. */
 export interface OverlayList {
   readonly select: SelectRenderable;
+  /** ScrollBox that clips the select and owns the host ScrollBar. */
+  readonly viewport: ScrollBoxRenderable;
   readonly activeIndex: number;
   /** Item-row capacity reserved by layout (not the renderable's row height). */
   readonly height: number;
@@ -557,6 +587,8 @@ export interface OverlayList {
   jump(index: number): void;
   setCount(count: number): void;
   setHeight(items: number, rowsPerItem?: number): void;
+  /** Move the host ScrollBox so the active row is inside the viewport. */
+  scrollActiveIntoView(): void;
   visibleRange(): OverlayListRange;
 }
 
@@ -723,11 +755,9 @@ interface ShellInternals {
 
 export const internals = new WeakMap<AppShell, ShellInternals>();
 
-/** Filler row sized to leftover viewport space so a short transcript
- * bottom-anchors against the prompt box; settles at zero once rows fill it.
- * A real child, not padding: `minHeight: "100%"` counts padding as viewport
- * height, so `scrollHeight - spacer.height` isolates the rows.
- * Index 0 is always the spacer. */
+/** Index-0 child of the transcript. Height stays 0: short logs bottom-align
+ * via the scroll content's justify-end, not a filler resized every frame.
+ * The node remains so row children keep a stable offset. */
 export const transcriptSpacers = new WeakMap<AppShell, BoxRenderable>();
 
 /** True while the landing composition is still mounted. */