From 8c39118ab109b28e23719c7df2d9c9ccc28d6174 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 08:15:29 -0700 Subject: [PATCH] Derive chrome budgets from an explicit fixed-row list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit provider-setup.ts's listHeight() subtracted a flat 14 from the terminal height, justified only by a comment naming roughly nine rows plus slack — a guess that had to be re-guessed by hand whenever a row was added or removed. shell.ts already avoids this by summing its ZoneId budget from PAINT_ORDER, so factor that reducer into a shared geometry/chrome-budget.ts and have both call sites use it. provider-setup.ts now names each fixed row it reserves in a CHROME_ROWS list and derives its budget by summing it. A test mounts the surface and checks root's child count against CHROME_ROWS plus the alternate-step rows, so a row added to root without a matching entry fails there instead of only showing up as garbled text on a short terminal. landing.ts mounts its boxes into zones shell.ts already sizes via PAINT_ORDER, so it carries no fixed-row budget of its own. stream.ts has no renderable tree at all. Neither needed changes. --- src/tui-opentui/geometry/chrome-budget.ts | 18 ++++++++++++ src/tui-opentui/geometry/index.ts | 2 ++ src/tui-opentui/geometry/resolve.ts | 11 ++++--- src/tui-opentui/provider-setup.test.ts | 28 ++++++++++++++++++ src/tui-opentui/provider-setup.ts | 36 ++++++++++++++++++++++- 5 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 src/tui-opentui/geometry/chrome-budget.ts diff --git a/src/tui-opentui/geometry/chrome-budget.ts b/src/tui-opentui/geometry/chrome-budget.ts new file mode 100644 index 000000000..3c2a84c33 --- /dev/null +++ b/src/tui-opentui/geometry/chrome-budget.ts @@ -0,0 +1,18 @@ +// Generic chrome-budget reducer shared by every screen that must reserve +// space for fixed rows before handing the remainder to a scrollable region. +// +// The value is not the arithmetic — summing is trivial — it is that the +// budget can only be as complete as its explicit row list. A row that is +// mounted but never added to the list is a gap in that list, not a runtime +// guess that only shows up as garbled output on a short terminal. + +/** One named fixed row (or block of rows) outside a screen's scrollable region. */ +export type ChromeRow = { + readonly id: string; + readonly rows: number; +}; + +/** Space every listed row reserves, summed. */ +export function chromeBudget(rows: readonly ChromeRow[]): number { + return rows.reduce((total, row) => total + row.rows, 0); +} diff --git a/src/tui-opentui/geometry/index.ts b/src/tui-opentui/geometry/index.ts index d0e75000f..48079ebba 100644 --- a/src/tui-opentui/geometry/index.ts +++ b/src/tui-opentui/geometry/index.ts @@ -17,6 +17,8 @@ export { type ZoneId, } from "./zones.js"; +export { chromeBudget, type ChromeRow } from "./chrome-budget.js"; + export { BOTTOM_MARGIN_MIN_ROWS, BOTTOM_MARGIN_ROWS, diff --git a/src/tui-opentui/geometry/resolve.ts b/src/tui-opentui/geometry/resolve.ts index b0bcb0233..c6a043fda 100644 --- a/src/tui-opentui/geometry/resolve.ts +++ b/src/tui-opentui/geometry/resolve.ts @@ -1,6 +1,7 @@ // Pure geometry resolver: terminal size + zone visibility + overlay mode → rects. // Caller passes { columns, rows }; this module never reads process.stdout. +import { chromeBudget, type ChromeRow } from "./chrome-budget.js"; import { resolveContentWidth, resolveSideMargin } from "./margins.js"; import { COLLAPSE_ORDER, @@ -161,12 +162,10 @@ export function desiredHeights(input: GeometryInput): MutableHeights { } function sumChrome(heights: MutableHeights): number { - let total = 0; - for (const id of PAINT_ORDER) { - if (id === "transcript" || id === "overlay_host") continue; - total += heights[id]; - } - return total; + const rows: ChromeRow[] = PAINT_ORDER.filter( + (id) => id !== "transcript" && id !== "overlay_host", + ).map((id) => ({ id, rows: heights[id] })); + return chromeBudget(rows); } function transcriptFloorFor(mode: OverlayMode, terminalRows: number): number { diff --git a/src/tui-opentui/provider-setup.test.ts b/src/tui-opentui/provider-setup.test.ts index d8dad1ee9..b9318f5f9 100644 --- a/src/tui-opentui/provider-setup.test.ts +++ b/src/tui-opentui/provider-setup.test.ts @@ -2,6 +2,8 @@ import { describe, expect, test } from "bun:test" import { createHarness, type Harness } from "./harness.js" import { + ALTERNATE_ROW_IDS, + CHROME_ROWS, CUSTOM_CHOICE_ID, failureGuidance, LOGIN_CANCELLED_MESSAGE, @@ -172,6 +174,32 @@ async function mountSetup( return { done, harness } } +describe("root's fixed-row chrome budget", () => { + test("mounts exactly the rows CHROME_ROWS and ALTERNATE_ROW_IDS name", async () => { + const { harness, done } = await mountSetup() + try { + const surface = harness.root + .getChildren() + .find((child) => child.id === "provider-setup") + expect(surface).toBeDefined() + // rootPadding is root's own paddingTop, not a child — every other + // CHROME_ROWS entry plus every alternate-step row is one child each. + // A row mounted without a matching entry in either list throws this + // off, so the bug class this guards against (a row added to `root` + // without being named anywhere) fails here rather than only showing + // up as garbled text on a short terminal. + const expectedChildCount = + CHROME_ROWS.filter((row) => row.id !== "rootPadding").length + + ALTERNATE_ROW_IDS.length + expect(surface?.getChildren().length).toBe(expectedChildCount) + } finally { + harness.pressKey("Ctrl+C") + await done + harness.destroy() + } + }) +}) + function type(harness: Harness, text: string): void { for (const ch of text) harness.pressKey(ch) } diff --git a/src/tui-opentui/provider-setup.ts b/src/tui-opentui/provider-setup.ts index e82cc3081..01ba3e78e 100644 --- a/src/tui-opentui/provider-setup.ts +++ b/src/tui-opentui/provider-setup.ts @@ -33,6 +33,7 @@ import { codexProviderName } from "../config/codex-providers.js" import { xaiProviderName } from "../config/xai-providers.js" import { TELEMETRY_NOTICE } from "../telemetry/index.js" import { wrapLines } from "../tui/view/height.js" +import { chromeBudget, type ChromeRow } from "./geometry/chrome-budget.js" import { resolveSideMargin } from "./geometry/margins.js" import { createListViewport, @@ -565,6 +566,36 @@ const LOGIN_ROWS = 4 const LIST_ROWS_MAX = 10 const LIST_ROWS_MIN = 3 const TELEMETRY_ROWS = 3 + +/** + * Every row `root` reserves outside the list step's scrollable region, + * named 1:1 with the `root.add(...)` calls below (`rootPadding` stands for + * `root`'s own `paddingTop`, which is not a child but still costs a row). + * `listHeight()` derives its budget by summing this list instead of + * carrying a hand-counted integer, so a row added to `root` without a + * matching entry here is a length mismatch caught by the test that checks + * `root`'s children against `CHROME_ROWS` + `ALTERNATE_ROW_IDS`, not a + * guess that only shows up as garbled text on a short terminal. + * + * `loginBox`, `inputFrame`, and `telemetry` are deliberately excluded: the + * step machine only ever shows one of them (or the list) at a time, so they + * never compete with the list for the same rows. + */ +export const CHROME_ROWS: readonly ChromeRow[] = [ + { id: "rootPadding", rows: 1 }, + { id: "header", rows: 1 }, + { id: "intro", rows: 1 }, + { id: "step", rows: 1 }, + { id: "instruction", rows: 1 }, + { id: "summary", rows: 1 + SUMMARY_SLOTS }, + { id: "listBoxPadding", rows: 1 }, + { id: "statusLine", rows: 1 }, + { id: "guidance", rows: 1 }, + { id: "footer", rows: 1 }, +] + +/** `root`'s other direct children — never on screen at the same time as the list. */ +export const ALTERNATE_ROW_IDS = ["loginBox", "inputFrame", "telemetry"] as const /** * Input capacity. The renderable defaults to 1000 characters and truncates a * longer paste silently, which a first run would read as "paste is broken"; @@ -646,7 +677,10 @@ export async function runProviderSetup( function listHeight(): number { const rows = renderer.height || 24 - return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - 14)) + return Math.max( + LIST_ROWS_MIN, + Math.min(LIST_ROWS_MAX, rows - chromeBudget(CHROME_ROWS)), + ) } const steps = (): readonly SetupStep[] => stepsFor(choice)