From 7a2219ffe1c8b87cb73e8d70ab6291f06ba902fc Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 00:04:37 -0700 Subject: [PATCH 1/2] Speak dialog choices in plain English, not renderable names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accepting a settings or approval dialog echoed its internal overlay kind and label straight to the transcript: "overlay chose (settings): compaction summarize ‹ drop ›". The word "overlay" named a renderable no operator knows, the "chose (kind): label" shape read as a state transition, and a cycled field's own selection markers survived into the line. Build the echo from the field's id and the value that actually won, worded as a sentence a person would say, and use the overlay's kind as its meta column instead of the literal word "overlay". The same path serves settings, permissions, and operator dialogs. Also stop labelling the goal/task/agents visibility toggles with the internal "chrome" zone name in their transcript text and meta. --- src/tui-opentui/command-surfaces.test.ts | 21 ++++++++++++++ src/tui-opentui/overlays.test.ts | 3 +- src/tui-opentui/shell.ts | 35 ++++++++++++++++++------ 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/tui-opentui/command-surfaces.test.ts b/src/tui-opentui/command-surfaces.test.ts index 13dca494a..57fd6e72b 100644 --- a/src/tui-opentui/command-surfaces.test.ts +++ b/src/tui-opentui/command-surfaces.test.ts @@ -166,6 +166,27 @@ describe("settings surface", () => { }) }) + test("choosing a cycled row writes a plain-English transcript line, not the internal echo", async () => { + await withShell(async (shell) => { + const { deps } = settingsDeps() + openCommandSurface(shell, "settings", deps) + await Promise.resolve() + await Promise.resolve() + + cycleOverlaySelection(shell, 1) + await Promise.resolve() + await Promise.resolve() + acceptOverlaySelection(shell) + + const row = shell.streamLog.at(-1) + expect(row?.text).toBe("Set compaction to drop.") + expect(row?.meta).not.toBe("overlay") + expect(row?.text).not.toContain("‹") + expect(row?.text).not.toContain("›") + expect(row?.text).not.toContain("overlay") + }) + }) + test("session mode scope switch honours a local write", async () => { await withShell(async (shell) => { const { deps, calls } = settingsDeps() diff --git a/src/tui-opentui/overlays.test.ts b/src/tui-opentui/overlays.test.ts index 7a0410c34..41c0d386e 100644 --- a/src/tui-opentui/overlays.test.ts +++ b/src/tui-opentui/overlays.test.ts @@ -223,7 +223,8 @@ describe("model / provider picker", () => { await h.renderOnce() frame = h.captureCharFrame() - expect(frame).toContain("chose (model_picker)") + expect(frame).toContain("model picker") + expect(frame).toMatch(/Chose /) } finally { shell.dispose() } diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 520c0b2e5..dd587c1c7 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -3347,7 +3347,7 @@ export function handleOverlayAnswerKey( appendStreamRow(shell, { role: "system", text: `answered: ${text}`, - meta: "overlay", + meta: overlayKindWord(shell.overlayKind ?? "operator"), }) // Deliberate submit, not a dismiss — closeInsetOverlay must not also fire // the Esc/cancel path. @@ -3735,14 +3735,33 @@ export function acceptOverlaySelection(shell: AppShell): void { if (bag?.overlayEchoChoice !== false) { appendStreamRow(shell, { role: "system", - text: `chose (${kind}): ${label}`, - meta: "overlay", + text: overlayChoiceText(label, id), + meta: overlayKindWord(kind), }) } closeInsetOverlay(shell) dispatchOverlayAccept(shell, selection, perOpen) } +/** + * Plain-English echo of an accepted choice. A cycled settings field's label + * carries every option with `‹ ›` around the active one (list-painting detail, + * not something an operator asked for) — name the field by its id and report + * only the value that won. A plain list item has no such markers, so it is + * quoted as-is. + */ +function overlayChoiceText(label: string, id: string | undefined): string { + const active = label.match(/‹\s*(.+?)\s*›/) + if (active === null) return `Chose ${label.trim()}.` + const field = id === undefined ? "setting" : id.replace(/[-_]/g, " ") + return `Set ${field} to ${active[1]}.` +} + +/** Internal overlay kinds read as words in the transcript, not identifiers. */ +function overlayKindWord(kind: PrimaryOverlayKind): string { + return kind.replace(/_/g, " ") +} + /** * Dispatch a selected palette item after the palette has closed. * - residual → `runPaletteAction` (overlays / chrome) @@ -3843,8 +3862,8 @@ export function runPaletteAction( }) appendStreamRow(shell, { role: "system", - text: on ? "goal chrome off" : "goal chrome on", - meta: "chrome", + text: on ? "goal banner off" : "goal banner on", + meta: "goal", }) return } @@ -3856,8 +3875,8 @@ export function runPaletteAction( }) appendStreamRow(shell, { role: "system", - text: on ? "task chrome off" : "task chrome on", - meta: "chrome", + text: on ? "task banner off" : "task banner on", + meta: "task", }) return } @@ -3870,7 +3889,7 @@ export function runPaletteAction( appendStreamRow(shell, { role: "system", text: on ? "agents strip off" : "agents strip on", - meta: "chrome", + meta: "agents", }) return } From 86eec83ed7b23206c04b6ce872a30c0341fc794c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 00:28:56 -0700 Subject: [PATCH 2/2] Thread the chosen settings value instead of parsing it back out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit overlayChoiceText recovered a cycled field's winning value by regex-matching the ‹ › markers back out of its rendered display label. That is scraping a paint artifact for data the caller already had: cycleField builds the label from a known activeId and option list, so the marker glyphs, spacing, or a label that legitimately contains ‹/› could silently fall through to the raw-label branch and reprint the exact marker-laden string this fix exists to remove. Add itemValues alongside itemIds on the overlay open path, threaded through OverlaySelection as an optional value, and have the settings surface supply each cycled row's active option label directly. The echo now reads that value; no parsing of the painted string remains. --- src/tui-opentui/command-surfaces.ts | 23 +++++++++++++++ src/tui-opentui/overlays.test.ts | 33 +++++++++++++++++++++ src/tui-opentui/shell.ts | 46 ++++++++++++++++++++++++----- 3 files changed, 94 insertions(+), 8 deletions(-) diff --git a/src/tui-opentui/command-surfaces.ts b/src/tui-opentui/command-surfaces.ts index bc8b47254..dbe73fea8 100644 --- a/src/tui-opentui/command-surfaces.ts +++ b/src/tui-opentui/command-surfaces.ts @@ -251,6 +251,14 @@ function cycleValue(options: readonly T[], current: T, direction: -1 | 1): T return next ?? current } +/** The active option's plain label — the value an accept echo should report, not the row's painted display string. */ +function activeOptionLabel( + options: readonly CycleOption[], + activeId: T, +): string { + return options.find((o) => o.id === activeId)?.label ?? activeId +} + const COMPACTION_OPTIONS: readonly CycleOption[] = [ { id: "llm", label: "summarize" }, { id: "pruning", label: "drop" }, @@ -273,6 +281,8 @@ const SETTINGS_NAME_WIDTH = 16 type SettingsCycleRow = { readonly id: string readonly value: string + /** Plain value the row currently holds, for the accept echo — not the painted `value` string. */ + readonly chosenLabel: string readonly describe: ItemDescription readonly cycle: (direction: -1 | 1) => void } @@ -286,6 +296,7 @@ function settingsCycleRows( { id: "compaction", value: `${"compaction".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(COMPACTION_OPTIONS, snapshot.compactionMode)}`, + chosenLabel: activeOptionLabel(COMPACTION_OPTIONS, snapshot.compactionMode), describe: { what: "how the transcript is trimmed once the context fills.", impact: @@ -299,6 +310,7 @@ function settingsCycleRows( { id: "session-mode", value: `${"session mode".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(SESSION_MODE_OPTIONS, snapshot.sessionMode)}`, + chosenLabel: activeOptionLabel(SESSION_MODE_OPTIONS, snapshot.sessionMode), describe: { what: "single agent works in-session; orchestrator delegates through a worker fleet.", impact: "orchestrator can run sub-agents concurrently and costs more per turn.", @@ -313,6 +325,7 @@ function settingsCycleRows( { id: "session-scope", value: `${" scope".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(SESSION_SCOPE_OPTIONS, snapshot.sessionModeScope)}`, + chosenLabel: activeOptionLabel(SESSION_SCOPE_OPTIONS, snapshot.sessionModeScope), describe: { what: "whether the session mode above applies to every repo or just this one.", impact: "this repo writes a local override that takes precedence over the global default.", @@ -326,6 +339,7 @@ function settingsCycleRows( { id: "wait-for-approval", value: `${"approval wait".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(ON_OFF_OPTIONS, snapshot.waitForApproval ? "on" : "off")}`, + chosenLabel: activeOptionLabel(ON_OFF_OPTIONS, snapshot.waitForApproval ? "on" : "off"), describe: { what: "whether a tool's time budget pauses while waiting on your approval.", impact: "off counts the wait against the tool's timeout, so a slow approval can time it out.", @@ -336,6 +350,7 @@ function settingsCycleRows( { id: "telemetry", value: `${"telemetry".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(ON_OFF_OPTIONS, snapshot.telemetryEnabled ? "on" : "off")}`, + chosenLabel: activeOptionLabel(ON_OFF_OPTIONS, snapshot.telemetryEnabled ? "on" : "off"), describe: { what: "anonymous usage data shared to help improve corbits.", impact: "off stops all telemetry from this session.", @@ -346,6 +361,7 @@ function settingsCycleRows( { id: "prompt-cost", value: `${"show cost".padEnd(SETTINGS_NAME_WIDTH)}${cycleField(ON_OFF_OPTIONS, snapshot.showPromptCost ? "on" : "off")}`, + chosenLabel: activeOptionLabel(ON_OFF_OPTIONS, snapshot.showPromptCost ? "on" : "off"), describe: { what: "shows the session's spend in the prompt border, next to the context percentage.", impact: "it's a running total that draws the eye every time it changes — off by default; /cost still gives the full breakdown on demand.", @@ -434,10 +450,17 @@ function renderSettingsMenu( ]) const ids = [...cycleRows.map((r) => r.id), ...navRows.map((r) => r.id)] const items = [...cycleRows.map((r) => r.value), ...navRows.map((r) => r.value)] + // Nav rows (permissions, plugins, hooks) open a sub-surface rather than + // holding a value of their own, so they carry no echo value. + const values: readonly (string | undefined)[] = [ + ...cycleRows.map((r) => r.chosenLabel), + ...navRows.map(() => undefined), + ] openSettingsOverlay(shell, { items, itemIds: ids, + itemValues: values, activeIndex: Math.min(activeIndex, Math.max(0, items.length - 1)), describe: (id) => descById.get(id) ?? null, onCycle: (id, direction) => { diff --git a/src/tui-opentui/overlays.test.ts b/src/tui-opentui/overlays.test.ts index 41c0d386e..4f486a801 100644 --- a/src/tui-opentui/overlays.test.ts +++ b/src/tui-opentui/overlays.test.ts @@ -18,6 +18,7 @@ import { closeInsetOverlay, createAppShell, moveOverlaySelection, + openListOverlay, pageOverlaySelection, relayout, setShellOverlayHooks, @@ -445,3 +446,35 @@ describe("resize mid-overlay", () => { ) }) }) + +describe("accept echo reads the chosen value structurally", () => { + test("a label containing its own ‹ › text does not corrupt the echo", async () => { + // A row whose display label happens to contain marker glyphs for reasons + // that have nothing to do with the cycled-field convention — the echo + // must still report the caller-supplied value, not something scraped + // back out of the label. + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + }) + try { + openListOverlay(shell, { + kind: "settings", + items: ["server name ‹ prod › staging"], + itemIds: ["server"], + itemValues: ["prod"], + }) + acceptOverlaySelection(shell) + + const row = shell.streamLog.at(-1) + expect(row?.text).toBe("Set server to prod.") + } finally { + shell.dispose() + } + }, + { width: 80, height: 24 }, + ) + }) +}) diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index dd587c1c7..6449ccc79 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -267,6 +267,8 @@ export type OverlaySelection = { readonly label: string /** Stable id when the host provided `itemIds`; otherwise omitted. */ readonly id?: string + /** Plain chosen value when the host provided `itemValues`; otherwise omitted. */ + readonly value?: string } /** @@ -1701,6 +1703,7 @@ type PriorOverlaySnapshot = { readonly title: string readonly paletteCommands: readonly PaletteCommand[] readonly itemIds: readonly string[] + readonly itemValues: readonly (string | undefined)[] readonly onAccept: ((selection: OverlaySelection) => void) | null readonly onToggleExpand: (() => void) | null readonly onCycle: ((itemId: string, direction: -1 | 1) => void) | null @@ -1720,6 +1723,8 @@ type ShellInternals = { priorOverlay: PriorOverlaySnapshot | null /** Optional stable ids aligned with overlayItems for the open primary. */ overlayItemIds: readonly string[] + /** Optional plain chosen values aligned with overlayItems for the open primary. */ + overlayItemValues: readonly (string | undefined)[] /** Per-open accept callback; cleared on close without invoke (Esc path). */ overlayOnAccept: ((selection: OverlaySelection) => void) | null /** False while an overlay that reports its own outcome is open. */ @@ -2963,6 +2968,13 @@ export type OpenListOverlayOpts = { readonly items?: readonly string[] /** Optional stable ids aligned with `items` (permission scope ids, model ids). */ readonly itemIds?: readonly string[] + /** + * Optional plain chosen-value aligned with `items`, for rows whose display + * label carries more than the value itself (a cycled field's name, padding, + * and `‹ ›` markers around the active option). The accept echo reads this + * instead of recovering the value by parsing the label back apart. + */ + readonly itemValues?: readonly (string | undefined)[] readonly body?: string readonly activeIndex?: number readonly frameId?: string @@ -3055,6 +3067,7 @@ export function openListOverlay( title: String(shell.overlayTitle.content), paletteCommands: shell.paletteCommands, itemIds: bag.overlayItemIds, + itemValues: bag.overlayItemValues, onAccept: bag.overlayOnAccept, onToggleExpand: bag.overlayOnToggleExpand, onCycle: bag.overlayOnCycle, @@ -3085,6 +3098,7 @@ export function openListOverlay( // Palette open does not own primary accept; leave prior snapshot's callback. if (!isPalette) { bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : [] + bag.overlayItemValues = opts?.itemValues ? [...opts.itemValues] : [] bag.overlayOnAccept = opts?.onAccept ?? null bag.overlayEchoChoice = opts?.echoChoice ?? true bag.overlayOnToggleExpand = opts?.onToggleExpand ?? null @@ -3095,6 +3109,7 @@ export function openListOverlay( } else if (!bag.priorOverlay) { // Bare palette (no primary under it): no accept payload. bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : [] + bag.overlayItemValues = opts?.itemValues ? [...opts.itemValues] : [] bag.overlayOnAccept = opts?.onAccept ?? null bag.overlayEchoChoice = opts?.echoChoice ?? true bag.overlayOnToggleExpand = opts?.onToggleExpand ?? null @@ -3457,6 +3472,7 @@ export function closeInsetOverlay(shell: AppShell): void { // captured before this clears, and is invoked separately once state settles). if (bag && !prior) { bag.overlayItemIds = [] + bag.overlayItemValues = [] bag.overlayOnAccept = null bag.overlayOnToggleExpand = null bag.overlayOnCycle = null @@ -3485,6 +3501,7 @@ export function closeInsetOverlay(shell: AppShell): void { shell.paletteCommands = prior.paletteCommands shell.overlayTitle.content = prior.title bag.overlayItemIds = prior.itemIds + bag.overlayItemValues = prior.itemValues bag.overlayOnAccept = prior.onAccept bag.overlayOnToggleExpand = prior.onToggleExpand bag.overlayOnCycle = prior.onCycle @@ -3629,11 +3646,13 @@ export function setOverlayItems( shell: AppShell, items: readonly string[], itemIds?: readonly string[], + itemValues?: readonly (string | undefined)[], ): void { if (!shell.overlayList) return shell.overlayItems = items const bag = internals.get(shell) if (bag && itemIds) bag.overlayItemIds = [...itemIds] + if (bag && itemValues) bag.overlayItemValues = [...itemValues] shell.overlayList = setListCount(shell.overlayList, items.length) paintOverlayList(shell) } @@ -3720,11 +3739,13 @@ export function acceptOverlaySelection(shell: AppShell): void { const bag = internals.get(shell) const id = bag?.overlayItemIds[idx] + const value = bag?.overlayItemValues[idx] const selection: OverlaySelection = { kind, index: idx, label, ...(id !== undefined ? { id } : {}), + ...(value !== undefined ? { value } : {}), } // Capture before close clears per-open state. const perOpen = bag?.overlayOnAccept ?? null @@ -3735,7 +3756,7 @@ export function acceptOverlaySelection(shell: AppShell): void { if (bag?.overlayEchoChoice !== false) { appendStreamRow(shell, { role: "system", - text: overlayChoiceText(label, id), + text: overlayChoiceText(label, id, value), meta: overlayKindWord(kind), }) } @@ -3746,15 +3767,20 @@ export function acceptOverlaySelection(shell: AppShell): void { /** * Plain-English echo of an accepted choice. A cycled settings field's label * carries every option with `‹ ›` around the active one (list-painting detail, - * not something an operator asked for) — name the field by its id and report - * only the value that won. A plain list item has no such markers, so it is - * quoted as-is. + * not something an operator asked for), so the caller passes the value that + * actually won structurally via `itemValues` rather than leaving it to be + * recovered from the rendered label — a marker or spacing change, or a label + * that legitimately contains `‹`/`›`, would otherwise corrupt the echo + * silently. A plain list item has no separate value, so it is quoted as-is. */ -function overlayChoiceText(label: string, id: string | undefined): string { - const active = label.match(/‹\s*(.+?)\s*›/) - if (active === null) return `Chose ${label.trim()}.` +function overlayChoiceText( + label: string, + id: string | undefined, + value: string | undefined, +): string { + if (value === undefined) return `Chose ${label.trim()}.` const field = id === undefined ? "setting" : id.replace(/[-_]/g, " ") - return `Set ${field} to ${active[1]}.` + return `Set ${field} to ${value}.` } /** Internal overlay kinds read as words in the transcript, not identifiers. */ @@ -4193,6 +4219,8 @@ export type OpenResidualListOpts = { readonly items?: readonly string[] /** Stable ids aligned with `items` (setting keys, session ids, paths). */ readonly itemIds?: readonly string[] + /** Plain chosen value aligned with `items`, for the accept echo (see `OpenListOverlayOpts.itemValues`). */ + readonly itemValues?: readonly (string | undefined)[] readonly activeIndex?: number /** Per-open accept; host binds toggle / resume / mention insert. */ readonly onAccept?: (selection: OverlaySelection) => void @@ -4213,6 +4241,7 @@ export function openSettingsOverlay( activeIndex: opts?.activeIndex ?? 0, frameId: "overlay-settings", ...(opts?.itemIds !== undefined ? { itemIds: opts.itemIds } : {}), + ...(opts?.itemValues !== undefined ? { itemValues: opts.itemValues } : {}), ...(opts?.onAccept !== undefined ? { onAccept: opts.onAccept } : {}), ...(opts?.onCycle !== undefined ? { onCycle: opts.onCycle } : {}), ...(opts?.describe !== undefined ? { describe: opts.describe } : {}), @@ -5416,6 +5445,7 @@ export function createAppShell( overlayBodyRows: undefined, priorOverlay: null, overlayItemIds: [], + overlayItemValues: [], overlayOnAccept: null, overlayEchoChoice: true, overlayOnToggleExpand: null,