Skip to content

Commit 8c39118

Browse files
committed
Derive chrome budgets from an explicit fixed-row list
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.
1 parent 1e617b1 commit 8c39118

5 files changed

Lines changed: 88 additions & 7 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Generic chrome-budget reducer shared by every screen that must reserve
2+
// space for fixed rows before handing the remainder to a scrollable region.
3+
//
4+
// The value is not the arithmetic — summing is trivial — it is that the
5+
// budget can only be as complete as its explicit row list. A row that is
6+
// mounted but never added to the list is a gap in that list, not a runtime
7+
// guess that only shows up as garbled output on a short terminal.
8+
9+
/** One named fixed row (or block of rows) outside a screen's scrollable region. */
10+
export type ChromeRow = {
11+
readonly id: string;
12+
readonly rows: number;
13+
};
14+
15+
/** Space every listed row reserves, summed. */
16+
export function chromeBudget(rows: readonly ChromeRow[]): number {
17+
return rows.reduce((total, row) => total + row.rows, 0);
18+
}

‎src/tui-opentui/geometry/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ export {
1717
type ZoneId,
1818
} from "./zones.js";
1919

20+
export { chromeBudget, type ChromeRow } from "./chrome-budget.js";
21+
2022
export {
2123
BOTTOM_MARGIN_MIN_ROWS,
2224
BOTTOM_MARGIN_ROWS,

‎src/tui-opentui/geometry/resolve.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Pure geometry resolver: terminal size + zone visibility + overlay mode → rects.
22
// Caller passes { columns, rows }; this module never reads process.stdout.
33

4+
import { chromeBudget, type ChromeRow } from "./chrome-budget.js";
45
import { resolveContentWidth, resolveSideMargin } from "./margins.js";
56
import {
67
COLLAPSE_ORDER,
@@ -161,12 +162,10 @@ export function desiredHeights(input: GeometryInput): MutableHeights {
161162
}
162163

163164
function sumChrome(heights: MutableHeights): number {
164-
let total = 0;
165-
for (const id of PAINT_ORDER) {
166-
if (id === "transcript" || id === "overlay_host") continue;
167-
total += heights[id];
168-
}
169-
return total;
165+
const rows: ChromeRow[] = PAINT_ORDER.filter(
166+
(id) => id !== "transcript" && id !== "overlay_host",
167+
).map((id) => ({ id, rows: heights[id] }));
168+
return chromeBudget(rows);
170169
}
171170

172171
function transcriptFloorFor(mode: OverlayMode, terminalRows: number): number {

‎src/tui-opentui/provider-setup.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { describe, expect, test } from "bun:test"
22

33
import { createHarness, type Harness } from "./harness.js"
44
import {
5+
ALTERNATE_ROW_IDS,
6+
CHROME_ROWS,
57
CUSTOM_CHOICE_ID,
68
failureGuidance,
79
LOGIN_CANCELLED_MESSAGE,
@@ -172,6 +174,32 @@ async function mountSetup(
172174
return { done, harness }
173175
}
174176

177+
describe("root's fixed-row chrome budget", () => {
178+
test("mounts exactly the rows CHROME_ROWS and ALTERNATE_ROW_IDS name", async () => {
179+
const { harness, done } = await mountSetup()
180+
try {
181+
const surface = harness.root
182+
.getChildren()
183+
.find((child) => child.id === "provider-setup")
184+
expect(surface).toBeDefined()
185+
// rootPadding is root's own paddingTop, not a child — every other
186+
// CHROME_ROWS entry plus every alternate-step row is one child each.
187+
// A row mounted without a matching entry in either list throws this
188+
// off, so the bug class this guards against (a row added to `root`
189+
// without being named anywhere) fails here rather than only showing
190+
// up as garbled text on a short terminal.
191+
const expectedChildCount =
192+
CHROME_ROWS.filter((row) => row.id !== "rootPadding").length +
193+
ALTERNATE_ROW_IDS.length
194+
expect(surface?.getChildren().length).toBe(expectedChildCount)
195+
} finally {
196+
harness.pressKey("Ctrl+C")
197+
await done
198+
harness.destroy()
199+
}
200+
})
201+
})
202+
175203
function type(harness: Harness, text: string): void {
176204
for (const ch of text) harness.pressKey(ch)
177205
}

‎src/tui-opentui/provider-setup.ts‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { codexProviderName } from "../config/codex-providers.js"
3333
import { xaiProviderName } from "../config/xai-providers.js"
3434
import { TELEMETRY_NOTICE } from "../telemetry/index.js"
3535
import { wrapLines } from "../tui/view/height.js"
36+
import { chromeBudget, type ChromeRow } from "./geometry/chrome-budget.js"
3637
import { resolveSideMargin } from "./geometry/margins.js"
3738
import {
3839
createListViewport,
@@ -565,6 +566,36 @@ const LOGIN_ROWS = 4
565566
const LIST_ROWS_MAX = 10
566567
const LIST_ROWS_MIN = 3
567568
const TELEMETRY_ROWS = 3
569+
570+
/**
571+
* Every row `root` reserves outside the list step's scrollable region,
572+
* named 1:1 with the `root.add(...)` calls below (`rootPadding` stands for
573+
* `root`'s own `paddingTop`, which is not a child but still costs a row).
574+
* `listHeight()` derives its budget by summing this list instead of
575+
* carrying a hand-counted integer, so a row added to `root` without a
576+
* matching entry here is a length mismatch caught by the test that checks
577+
* `root`'s children against `CHROME_ROWS` + `ALTERNATE_ROW_IDS`, not a
578+
* guess that only shows up as garbled text on a short terminal.
579+
*
580+
* `loginBox`, `inputFrame`, and `telemetry` are deliberately excluded: the
581+
* step machine only ever shows one of them (or the list) at a time, so they
582+
* never compete with the list for the same rows.
583+
*/
584+
export const CHROME_ROWS: readonly ChromeRow[] = [
585+
{ id: "rootPadding", rows: 1 },
586+
{ id: "header", rows: 1 },
587+
{ id: "intro", rows: 1 },
588+
{ id: "step", rows: 1 },
589+
{ id: "instruction", rows: 1 },
590+
{ id: "summary", rows: 1 + SUMMARY_SLOTS },
591+
{ id: "listBoxPadding", rows: 1 },
592+
{ id: "statusLine", rows: 1 },
593+
{ id: "guidance", rows: 1 },
594+
{ id: "footer", rows: 1 },
595+
]
596+
597+
/** `root`'s other direct children — never on screen at the same time as the list. */
598+
export const ALTERNATE_ROW_IDS = ["loginBox", "inputFrame", "telemetry"] as const
568599
/**
569600
* Input capacity. The renderable defaults to 1000 characters and truncates a
570601
* longer paste silently, which a first run would read as "paste is broken";
@@ -646,7 +677,10 @@ export async function runProviderSetup(
646677

647678
function listHeight(): number {
648679
const rows = renderer.height || 24
649-
return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - 14))
680+
return Math.max(
681+
LIST_ROWS_MIN,
682+
Math.min(LIST_ROWS_MAX, rows - chromeBudget(CHROME_ROWS)),
683+
)
650684
}
651685

652686
const steps = (): readonly SetupStep[] => stepsFor(choice)

0 commit comments

Comments
 (0)