Skip to content

Commit ea30c9a

Browse files
committed
Address greybeard review: accurate docs, seeded task visibility, drop the string task shape
- docs/TUI.md: the subscribeChrome paragraph claimed making it required fixed an observed break; the production caller always passed it, so restate this as closing a shape that could have type-checked while omitted, not a fix for something that broke. - docs/TUI.md: the collapse-order paragraph conflated two mechanisms. COLLAPSE_ORDER/collapseOnce governs what collapse takes from zones ahead of prompt; PROMPT_CAP_FRACTION independently bounds the prompt's own requested height before collapse ever runs. Name both and what each guarantees. - shell.ts defaultVisibility: seed task: 0 alongside agents: 0, so the adjacent comment about avoiding a needless first relayout is true for both row-count fields it now describes. - chrome-state.ts: delete the string member of ChromeLiveState['task'] and formatTasksPanel's string branch. chromeFromSession never produces a string, so the only callers left were its own tests — a back-compat surface for callers this repo owns, which AGENTS.md forbids. Updated demo.ts and the tests that exercised the string shape accordingly. - Reworded 'session-persisted' to 'shell-lifetime, in memory, nothing written to storage' everywhere it appeared (docs, comments, a test title) — that is the actual design, the phrasing just claimed durability it doesn't have. - geometry.test.ts: corrected a test comment that attributed the short- terminal invariant solely to collapse order; across most of the tested range it actually holds via PROMPT_CAP_FRACTION capping the requested prompt before collapse runs at all.
1 parent 970b0f5 commit ea30c9a

7 files changed

Lines changed: 59 additions & 45 deletions

File tree

docs/TUI.md

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,26 +116,39 @@ bounded to `TASKS_PANEL_MAX_VISIBLE` rows, same shape as the agents panel: a
116116
longer list degrades to a trailing `+N more` row rather than growing the zone
117117
without limit, and it shrinks one row at a time under space pressure
118118
(`COLLAPSE_ORDER` in `geometry/zones.ts`) rather than vanishing in one step.
119-
`task` sits ahead of `agents` in `COLLAPSE_ORDER`, so on a short terminal the
120-
task panel is always fully collapsed before the prompt box is ever touched —
121-
the prompt is never pushed off screen by a competing chrome zone.
119+
120+
Two independent mechanisms keep the task panel from ever costing the prompt
121+
box a row on a short terminal, and they guarantee different things.
122+
`COLLAPSE_ORDER` places `task` ahead of `prompt`, so `collapseOnce`
123+
(`geometry/resolve.ts`) always drains `task` to zero before it ever reduces
124+
`prompt` — that loop only runs when the transcript floor is not yet met, and
125+
it never touches a zone later in the order while an earlier one still has
126+
rows to give up. Separately, `PROMPT_CAP_FRACTION` in `desiredHeights` caps
127+
how tall a *requested* prompt is allowed to start at (`PROMPT_CAP_FRACTION *
128+
terminal.rows`), independent of collapse and before it ever runs. Neither
129+
mechanism substitutes for the other: the cap bounds the prompt's own growth
130+
on any terminal, tall or short; the collapse order bounds what other zones
131+
are allowed to take from it once the transcript floor is at risk.
122132

123133
The panel is toggleable independent of its live data: `toggleTasksPanel`
124-
(bound to the `toggle_task` palette action) flips a hidden flag that persists
125-
on the shell for the life of the session, while the live task list keeps
126-
updating underneath it — un-hiding shows the current list, not a stale
127-
snapshot from before the hide. Hidden or empty, the zone costs zero rows.
134+
(bound to the `toggle_task` palette action) flips a hidden flag held on the
135+
shell for its lifetime — in memory only, nothing written to storage — while
136+
the live task list keeps updating underneath it. Un-hiding shows the current
137+
list, not a stale snapshot from before the hide. Hidden or empty, the zone
138+
costs zero rows.
128139

129140
The task tool writes state through `ChatDirectorImpl` (`src/agent/director.ts`),
130141
which calls `onTasksChange` on every `manage_tasks` tool call and on session
131142
resume (`restoreTasks`). The runner forwards that into the OpenTUI host via
132143
`RunnerHostDeps.chrome`/`subscribeChrome` (`src/tui-opentui/runner-host.ts`):
133-
`subscribeChrome` is a required dependency, not optional, because an omitted
134-
subscription used to type-check cleanly while silently leaving the panel
135-
frozen at its mount-time snapshot — a mechanism built and never wired, hidden
136-
behind an optional callback. `runner-host.test.ts` drives a live
137-
`subscribeChrome` notify end to end and asserts the panel actually repaints,
138-
so that class of regression fails a test again if it recurs.
144+
`subscribeChrome` is a required dependency, not optional. The production
145+
caller has always passed a real subscription, so this did not fix an
146+
observed break; it closes a shape that could have been omitted and would
147+
still have type-checked — the same "callback that types fine when absent"
148+
hazard this feature's own callback (`onTasksChange`) is named after in the
149+
tracking issue. `runner-host.test.ts` drives a live `subscribeChrome` notify
150+
end to end and asserts the panel actually repaints, so an omission would now
151+
fail a test as well as the type checker.
139152

140153
## The live agents panel
141154

src/tui-opentui/chrome-state.test.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ describe("formatChromeZones", () => {
2828
})
2929
})
3030

31-
test("partial: task string only", () => {
32-
const out = formatChromeZones({ task: "cutover readiness" })
33-
expect(out.task).toEqual([{ label: "cutover readiness", status: null }])
31+
test("partial: task rows only", () => {
32+
const out = formatChromeZones({
33+
task: [{ title: "cutover readiness", status: "doing" }],
34+
})
35+
expect(out.task).toEqual([{ label: "cutover readiness", status: "doing" }])
3436
expect(out.agents).toBeNull()
3537
})
3638

@@ -100,13 +102,9 @@ describe("formatChromeZones", () => {
100102
})
101103

102104
describe("formatTasksPanel", () => {
103-
test("string / empty", () => {
105+
test("null / undefined hide the zone", () => {
104106
expect(formatTasksPanel(null)).toBeNull()
105-
expect(formatTasksPanel("")).toBeNull()
106-
expect(formatTasksPanel(" ")).toBeNull()
107-
expect(formatTasksPanel("wire host")).toEqual([
108-
{ label: "wire host", status: null },
109-
])
107+
expect(formatTasksPanel(undefined)).toBeNull()
110108
})
111109

112110
test("each row carries its own status", () => {

src/tui-opentui/chrome-state.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,10 @@ export type TaskPanelRow = {
6161
*/
6262
export type ChromeLiveState = {
6363
/**
64-
* Task list: string shorthand (rendered as a single unstyled row) or the
65-
* structured rows the task tool writes. Distinct from `agents` — a task is
66-
* a unit of work with a status, not an executor.
64+
* Task list: the structured rows the task tool writes. Distinct from
65+
* `agents` — a task is a unit of work with a status, not an executor.
6766
*/
68-
readonly task?: readonly ChromeTaskRow[] | string | null
67+
readonly task?: readonly ChromeTaskRow[] | null
6968
/** Subagent sessions for the strip summary (running preferred). */
7069
readonly agents?: readonly ChromeAgentSession[] | null
7170
/**
@@ -131,20 +130,14 @@ export function chromeZonesContent(state: ChromeLiveState): ChromeZoneContent {
131130
*
132131
* Terminal tasks (done/cancelled) still render — the panel is a live list of
133132
* work, not just what remains — so an operator watching it sees a task move
134-
* to "done" rather than silently vanish. A bare string input renders as one
135-
* row with no status marker: it is free-form summary text, not a task record.
133+
* to "done" rather than silently vanish.
136134
*/
137135
export function formatTasksPanel(
138-
task: readonly ChromeTaskRow[] | string | null | undefined,
136+
task: readonly ChromeTaskRow[] | null | undefined,
139137
maxVisible: number = TASKS_PANEL_MAX_VISIBLE,
140138
): readonly TaskPanelRow[] | null {
141139
if (task === null || task === undefined) return null
142140

143-
if (typeof task === "string") {
144-
const t = task.trim()
145-
return t.length === 0 ? null : [{ label: t, status: null }]
146-
}
147-
148141
const rows: TaskPanelRow[] = task
149142
.map((t) => ({ label: t.title.trim(), status: t.status }))
150143
.filter((r) => r.label.length > 0)

src/tui-opentui/demo.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,9 @@ renderer.keyInput.on("keypress", (key: KeyEvent) => {
277277
setChromeZones(shell, {
278278
task: on
279279
? null
280-
: formatChromeZones({ task: "cutover readiness" }).task,
280+
: formatChromeZones({
281+
task: [{ title: "cutover readiness", status: "doing" }],
282+
}).task,
281283
})
282284
return
283285
}

src/tui-opentui/geometry.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,16 @@ describe("resolveGeometry — task panel", () => {
190190
});
191191

192192
test("on a short terminal the task panel is fully collapsed before the prompt is ever shrunk below its idle rows", () => {
193-
// Shrink the terminal until something has to give. The task panel sits
194-
// ahead of the prompt in COLLAPSE_ORDER, so collapseOnce always drains it
195-
// to zero before touching the prompt — the prompt degrades last, never
196-
// first, so it is never pushed off screen by a competing chrome zone.
193+
// Shrink the terminal until something has to give. Two mechanisms can
194+
// land the prompt below its idle rows here: PROMPT_CAP_FRACTION caps the
195+
// *requested* prompt before collapse ever runs (the one that actually
196+
// fires across most of this range, since a short terminal caps prompt
197+
// rows well before a 6-row task panel could account for the deficit on
198+
// its own), and collapseOnce would additionally shrink prompt only after
199+
// draining every zone ahead of it in COLLAPSE_ORDER — task included.
200+
// Either way the invariant holds: whenever prompt is below its idle
201+
// rows, task is already at zero, so the task panel never survives at
202+
// the prompt's expense.
197203
for (let rows = 24; rows >= 10; rows--) {
198204
const layout = resolveGeometry({
199205
terminal: { columns: 80, rows },

src/tui-opentui/shell.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -761,9 +761,10 @@ function defaultVisibility(visibility?: ZoneVisibility): ZoneVisibility {
761761
notice: false,
762762
progress: false,
763763
progressDivider: false,
764-
// Explicit 0 rather than left undefined: the agents field is now a row
765-
// count, and setChromeZones compares it by ===, so an undefined start
766-
// forces one needless relayout the first time it is ever compared.
764+
// Explicit 0 rather than left undefined: task and agents are row
765+
// counts, and setChromeZones compares them by ===, so an undefined
766+
// start forces one needless relayout the first time either is compared.
767+
task: 0,
767768
agents: 0,
768769
...visibility,
769770
}
@@ -1895,7 +1896,7 @@ type ShellInternals = {
18951896
/** Agents panel rows (empty array = zone off), one row per rendered line. */
18961897
agents: readonly AgentPanelRow[]
18971898
}
1898-
/** Operator toggle for the task panel; persists for the life of the shell (session). */
1899+
/** Operator toggle for the task panel; in-memory, held for the life of the shell. */
18991900
tasksPanelHidden: boolean
19001901
}
19011902

@@ -4341,7 +4342,8 @@ export function setChromeZones(
43414342
* Toggle the task-list panel visible/hidden without touching the live task
43424343
* data underneath it — un-hiding shows whatever the task tool last wrote,
43434344
* not a stale snapshot from before the hide. The flag lives on the shell's
4344-
* internals for the life of the process, i.e. persists for the session.
4345+
* internals in memory for the shell's lifetime; nothing is written to
4346+
* storage, so it does not survive a restart.
43454347
*/
43464348
export function toggleTasksPanel(shell: AppShell): void {
43474349
const bag = internals.get(shell)

src/tui-opentui/wave6.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ describe("CL-5731: task list panel", () => {
566566
)
567567
})
568568

569-
test("the toggle persists across further chrome pushes for the life of the shell (the session)", async () => {
569+
test("the toggle persists across further chrome pushes for the life of the shell", async () => {
570570
await withTestRenderer(
571571
async (h) => {
572572
const shell = createAppShell(h.renderer, {

0 commit comments

Comments
 (0)