Skip to content
Merged
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
6 changes: 3 additions & 3 deletions src/tui/allow-once-reprompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand All @@ -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);
});
});

Expand Down
4 changes: 4 additions & 0 deletions src/tui/decision-truncation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
10 changes: 7 additions & 3 deletions src/tui/overlay-paint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/tui/overlay-view.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
12 changes: 7 additions & 5 deletions src/tui/overlay-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
64 changes: 64 additions & 0 deletions src/tui/shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
38 changes: 24 additions & 14 deletions src/tui/shell/chrome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions src/tui/shell/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { flushStreamRowUpdates } from "../runtime-bridge.js";
import {
type AppShell,
type AppShellOptions,
bindTranscriptTailSticky,
EMPTY_PRIMARY_BINDINGS,
flashTimers,
initShellInternals,
Expand Down Expand Up @@ -66,6 +67,7 @@ import {
syncNoticeAfterLayout,
syncPromptRows,
syncTranscriptSpacer,
followTranscriptTail,
} from "./chrome.js";
import {
clearPendingAttachments,
Expand Down Expand Up @@ -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 },
Expand All @@ -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%",
Expand Down Expand Up @@ -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);
};

Expand Down
44 changes: 37 additions & 7 deletions src/tui/shell/internals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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. */
Expand Down
Loading
Loading