From f3dc1a941a1fc77bea2240e633231c3d4b236b46 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 4 Aug 2026 21:19:26 -0700 Subject: [PATCH 1/2] Extract noisy JSX/statement IIFEs into helpers (CL-5351 slice C) Behavior-preserving refactors only: hoist work/accept ordering and copy-mode windowing in app.tsx, PromptActionBar + renderInputLines in chat-input, re-auth hint boolean in agent-modal, plain manage_tasks parse block in use-stream, and shared persistWithMergeBase for the two provider-manager disk writes. --- src/tui/app.tsx | 102 ++++++----- src/tui/components/agent-modal.tsx | 21 +-- src/tui/components/chat-input.tsx | 239 +++++++++++++++----------- src/tui/hooks/use-provider-manager.ts | 110 +++++++----- src/tui/use-stream.ts | 12 +- 5 files changed, 274 insertions(+), 210 deletions(-) diff --git a/src/tui/app.tsx b/src/tui/app.tsx index 281ab00de..8f84ce02a 100644 --- a/src/tui/app.tsx +++ b/src/tui/app.tsx @@ -235,6 +235,21 @@ export type AppProps = { onFirstUserMessage?: () => void; }; +// Center a selection in a fixed-height window over a copy-target list. +// Returns the visible slice and the absolute index of its first item so +// the caller can mark the selected row without re-scanning the full list. +function windowedCopyTargets( + items: readonly CopyTarget[], + selectedIndex: number, + windowSize = 6, +): { window: readonly CopyTarget[]; start: number } { + const start = Math.max( + 0, + Math.min(selectedIndex - Math.floor(windowSize / 2), Math.max(0, items.length - windowSize)), + ); + return { window: items.slice(start, start + windowSize), start }; +} + export function App({ eventEmitter, agent, @@ -1074,6 +1089,40 @@ export function App({ ); } + // Work / Acceptance chrome: order flips by goal phase (implementing = Work on top). + const workBlock = hasActiveTasks(state.tasks) ? ( + + + + ) : null; + const acceptBlock = + goalActive && goalSnapshot !== null ? ( + + + + ) : null; + const workAcceptBlocks = workPrimary ? ( + <> + {workBlock} + {acceptBlock} + + ) : ( + <> + {acceptBlock} + {workBlock} + + ); + + const copyModeSelection = copyModeIndex ?? 0; + const { window: copyModeWindow, start: copyModeWindowStart } = windowedCopyTargets( + copyTargetList, + copyModeSelection, + ); + return ( @@ -1256,35 +1305,7 @@ export function App({ )} {!taskFullScreenOpen && ( - {(() => { - const workBlock = hasActiveTasks(state.tasks) ? ( - - - - ) : null; - const acceptBlock = - goalActive && goalSnapshot !== null ? ( - - - - ) : null; - // implementing: Work on top; planning/reviewing/completed: Acceptance on top - return workPrimary ? ( - <> - {workBlock} - {acceptBlock} - - ) : ( - <> - {acceptBlock} - {workBlock} - - ); - })()} + {workAcceptBlocks} {agentsStripVisible ? ( Copy — ↑/↓ select · y/⏎ copy · a copy all · esc cancel - {(() => { - const windowSize = 6; - const sel = copyModeIndex ?? 0; - const start = Math.max(0, Math.min(sel - Math.floor(windowSize / 2), Math.max(0, copyTargetList.length - windowSize))); - return copyTargetList.slice(start, start + windowSize).map((target, i) => { - const idx = start + i; - const selected = idx === sel; - return ( - - {selected ? "› " : " "}{target.label}: {target.preview} - - ); - }); - })()} + {copyModeWindow.map((target, i) => { + const idx = copyModeWindowStart + i; + const selected = idx === copyModeSelection; + return ( + + {selected ? "› " : " "}{target.label}: {target.preview} + + ); + })} )} {exitConfirmOpen ? ( diff --git a/src/tui/components/agent-modal.tsx b/src/tui/components/agent-modal.tsx index ef38ea86d..5fce4c04f 100644 --- a/src/tui/components/agent-modal.tsx +++ b/src/tui/components/agent-modal.tsx @@ -789,7 +789,7 @@ export function AgentModal({ setFormError(null); }); - const helpText = ((): string | null => { +const helpText = ((): string | null => { switch (step) { case "provider": return "Up/Down navigate · Enter models · a add · e edit · x remove · t tiers · p profiles · Esc close"; @@ -814,6 +814,11 @@ export function AgentModal({ } })(); const helpLines = helpText !== null ? wrapHelpSegments(helpText.split(" · "), contentWidth) : []; + const selectedProviderRow = providers[providerIndex]; + const showReauthHint = + selectedProviderRow !== undefined && + (selectedProviderRow.codexProfile !== undefined || selectedProviderRow.xaiProfile !== undefined) && + unauthedProviders?.has(selectedProviderRow.name) === true; return ( ); })} - {(() => { - const p = providers[providerIndex]; - const isUnauthed = p !== undefined && (p.codexProfile !== undefined || p.xaiProfile !== undefined) && unauthedProviders?.has(p.name) === true; - return isUnauthed ? ( - - Enter to re-authenticate - - ) : null; - })()} + {showReauthHint ? ( + + Enter to re-authenticate + + ) : null} )} diff --git a/src/tui/components/chat-input.tsx b/src/tui/components/chat-input.tsx index 745ff6565..e3b21145f 100644 --- a/src/tui/components/chat-input.tsx +++ b/src/tui/components/chat-input.tsx @@ -66,6 +66,64 @@ export type ChatInputProps = { canSubmitEmpty?: boolean; }; +// Action bar above the prompt: revolving verb + interrupt/queue hint on the +// left, profile · model · effort right-aligned. Null when nothing to show. +function PromptActionBar(props: { + showSteerHint: boolean; + value: string; + steerOnEnter: boolean; + queuedCount: number; + verb?: string; + profile?: string; + model?: string; + effort?: string; + attachmentSummary?: string; +}): ReactNode { + const { + showSteerHint, + value, + steerOnEnter, + queuedCount, + verb, + profile, + model, + effort, + attachmentSummary, + } = props; + // Enter and Alt+Enter are no-ops on an empty field, so with nothing typed + // the hint advertises the interrupt chord instead. + const hasPromptText = value.trim().length > 0; + const actionsText = !hasPromptText + ? "Esc Esc interrupt" + : !steerOnEnter + ? "Enter queues for orchestrator" + : "Enter steer · Alt+Enter queue"; + const steerText = queuedCount > 0 ? `${queuedCount} queued · ${actionsText}` : actionsText; + const modelText = composePromptActionBarModelLabel({ + ...(profile !== undefined ? { profile } : {}), + ...(model !== undefined ? { model } : {}), + ...(effort !== undefined ? { effort } : {}), + }); + const showAttachments = attachmentSummary !== undefined && attachmentSummary.length > 0; + if (!showSteerHint && modelText === undefined && !showAttachments) return null; + return ( + + {showAttachments && ( + {attachmentSummary} + )} + {showSteerHint && ( + + {verb !== undefined && verb.length > 0 ? `${verb} · ` : ""}{steerText} + + )} + + {modelText !== undefined && ( + {modelText} + )} + + ); +} + // The subset of Ink's Key type that applyKey needs. Keeping only what we use // prevents coupling to Ink's full Key shape in test code. export type InputKey = { @@ -685,6 +743,70 @@ export function ChatInput({ // discoverable immediately. const showSteerHint = isProcessing; + const renderInputLines = (): ReactNode[] => { + const out: ReactNode[] = []; + if (atTopEdge) { + out.push( + {" ↑"}, + ); + } + for (let i = windowStart; i < windowEnd; i++) { + const line = lines[i]!; + const prefix = i === 0 ? "> " : " "; + if (i !== cursorLine) { + out.push( + + {prefix} + {line} + , + ); + continue; + } + const head = line.slice(0, cursorCol); + const atChar = line.slice(cursorCol, cursorCol + cursorCharLength); + const tail = line.slice(cursorCol + cursorCharLength); + out.push( + + {prefix} + {head} + {atChar.length > 0 ? ( + <> + {atChar} + {tail} + + ) : ( + ▏ + )} + , + ); + } + if (atBottomEdge) { + out.push( + {" ↓"}, + ); + } + return out; + }; + + // When slash/@ pickers are open the input renders plainly so the + // suggestion list sits flush above it without a competing border. + const inputBody = showSlash || showAt ? ( + + {renderInputLines()} + + ) : ( + + + {renderInputLines()} + + + ); + return ( {showSlash && ( @@ -749,111 +871,18 @@ export function ChatInput({ {showAt && ( )} - {(() => { - // Action bar: the row directly above the prompt box. While processing, - // the revolving verb + action hint sit on the left; the - // profile · model · effort is always right-aligned on the same baseline. - // Enter and Alt+Enter are no-ops on an empty field, so with nothing - // typed the hint advertises the interrupt chord instead. - const hasPromptText = value.trim().length > 0; - const actionsText = !hasPromptText - ? "Esc Esc interrupt" - : !steerOnEnter - ? "Enter queues for orchestrator" - : "Enter steer · Alt+Enter queue"; - const steerText = queuedCount > 0 ? `${queuedCount} queued · ${actionsText}` : actionsText; - const modelText = composePromptActionBarModelLabel({ - ...(profile !== undefined ? { profile } : {}), - ...(model !== undefined ? { model } : {}), - ...(effort !== undefined ? { effort } : {}), - }); - const showAttachments = attachmentSummary !== undefined && attachmentSummary.length > 0; - if (!showSteerHint && modelText === undefined && !showAttachments) return null; - return ( - - {showAttachments && ( - {attachmentSummary} - )} - {showSteerHint && ( - - {verb !== undefined && verb.length > 0 ? `${verb} · ` : ""}{steerText} - - )} - - {modelText !== undefined && ( - {modelText} - )} - - ); - })()} - {(() => { - const renderInputLines = () => { - const out: ReactNode[] = []; - if (atTopEdge) { - out.push( - {" ↑"}, - ); - } - for (let i = windowStart; i < windowEnd; i++) { - const line = lines[i]!; - const prefix = i === 0 ? "> " : " "; - if (i !== cursorLine) { - out.push( - - {prefix} - {line} - , - ); - continue; - } - const head = line.slice(0, cursorCol); - const atChar = line.slice(cursorCol, cursorCol + cursorCharLength); - const tail = line.slice(cursorCol + cursorCharLength); - out.push( - - {prefix} - {head} - {atChar.length > 0 ? ( - <> - {atChar} - {tail} - - ) : ( - ▏ - )} - , - ); - } - if (atBottomEdge) { - out.push( - {" ↓"}, - ); - } - return out; - }; - - // When slash/@ pickers are open the input renders plainly so the - // suggestion list sits flush above it without a competing border. - if (showSlash || showAt) { - return ( - - {renderInputLines()} - - ); - } - return ( - - - {renderInputLines()} - - - ); - })()} + + {inputBody} ); } diff --git a/src/tui/hooks/use-provider-manager.ts b/src/tui/hooks/use-provider-manager.ts index ed7731ee7..1c1dbfadd 100644 --- a/src/tui/hooks/use-provider-manager.ts +++ b/src/tui/hooks/use-provider-manager.ts @@ -160,6 +160,43 @@ function persistGlobalSettings( ); } +// Shared disk-first persist path for catalog and tier writes. Loads a fresh +// merge base, builds settings, optionally mutates in-memory state, then saves. +// Fire-and-forget: callers `void` the promise so UI stays non-blocking. +async function persistWithMergeBase(args: { + globalSettingsPath: string; + initialSettings: Settings | undefined; + buildSettings: (base: Settings | undefined) => Settings; + onMessage: (msg: string) => void; + successMessage: string; + failPrefix: string; + /** Runs only after load+build succeed, before the disk write. */ + onBeforeSave?: () => void; +}): Promise { + let base: Settings | undefined; + try { + base = await loadMergeBase(args.globalSettingsPath, args.initialSettings); + } catch (err) { + args.onMessage(`${args.failPrefix}: ${err instanceof Error ? err.message : String(err)}`); + return; + } + let settings: Settings; + try { + settings = args.buildSettings(base); + } catch (err) { + args.onMessage(`${args.failPrefix}: ${err instanceof Error ? err.message : String(err)}`); + return; + } + args.onBeforeSave?.(); + persistGlobalSettings( + args.globalSettingsPath, + settings, + args.onMessage, + args.successMessage, + args.failPrefix, + ); +} + export function useProviderManager({ initialProvider, initialModel, @@ -316,37 +353,22 @@ export function useProviderManager({ successMessage: string, ): void => { // Disk-first merge base: mid-session /plugins writes must survive a later - // provider save. Fail closed if settings cannot be re-read. - void (async () => { - let base: Settings | undefined; - try { - base = await loadMergeBase(globalSettingsPath, initialSettings); - } catch (err) { - onMessage( - `Provider settings changed locally, but saving failed: ${err instanceof Error ? err.message : String(err)}`, - ); - return; - } - let settings: Settings; - try { - settings = persistSettingsWithTiers(catalog, defaultProvider, base, tiers); - } catch (err) { - onMessage( - `Provider settings changed locally, but saving failed: ${err instanceof Error ? err.message : String(err)}`, - ); - return; - } - setProviderCatalog(catalog); - setGlobalDefaultProvider(defaultProvider); - publishRuntimeResolution(catalog, tiers, defaultProvider); - persistGlobalSettings( - globalSettingsPath, - settings, - onMessage, - successMessage, - "Provider settings changed locally, but saving failed", - ); - })(); + // provider save. Fail closed if settings cannot be re-read. Catalog state + // only updates after load+build succeed so a failed re-read never leaves + // the UI ahead of disk. + void persistWithMergeBase({ + globalSettingsPath, + initialSettings, + buildSettings: (base) => persistSettingsWithTiers(catalog, defaultProvider, base, tiers), + onMessage, + successMessage, + failPrefix: "Provider settings changed locally, but saving failed", + onBeforeSave: () => { + setProviderCatalog(catalog); + setGlobalDefaultProvider(defaultProvider); + publishRuntimeResolution(catalog, tiers, defaultProvider); + }, + }); }; const upsertProvider = (submission: ProviderSubmission): { ok: true } | { ok: false; error: string } => { @@ -391,26 +413,20 @@ export function useProviderManager({ successMessage: string, failPrefix: string, ): void => { + // Tiers update optimistically so the modal reflects the edit immediately; + // disk write is best-effort via the shared merge-base path. setTiers(nextTiers); pushLiveSources(nextTiers); publishRuntimeResolution(providerCatalog, nextTiers); - void (async () => { - let base: Settings | undefined; - try { - base = await loadMergeBase(globalSettingsPath, initialSettings); - } catch (err) { - onMessage(`${failPrefix}: ${err instanceof Error ? err.message : String(err)}`); - return; - } - persistGlobalSettings( - globalSettingsPath, + void persistWithMergeBase({ + globalSettingsPath, + initialSettings, + buildSettings: (base) => persistSettingsWithTiers(providerCatalog, globalDefaultProvider, base, nextTiers), - - onMessage, - successMessage, - failPrefix, - ); - })(); + onMessage, + successMessage, + failPrefix, + }); }; const saveTierAssignment = ( diff --git a/src/tui/use-stream.ts b/src/tui/use-stream.ts index af71b4402..1fc009b45 100644 --- a/src/tui/use-stream.ts +++ b/src/tui/use-stream.ts @@ -1052,12 +1052,14 @@ export function createAgentStreamState( const taskArgs = callIdToArguments.get(result.callId) ?? ""; callIdToName.delete(result.callId); callIdToArguments.delete(result.callId); - const newTasks = (() => { - let raw: unknown; - try { raw = JSON.parse(taskArgs as string); } catch { return tasks; } + let newTasks = tasks; + try { + const raw: unknown = JSON.parse(taskArgs as string); const parsed = parseManageTasksArgs(raw); - return parsed !== null ? applyManageTasks(tasks, parsed) : tasks; - })(); + if (parsed !== null) newTasks = applyManageTasks(tasks, parsed); + } catch { + // Malformed manage_tasks args: leave the checklist unchanged. + } if (taskCallIndex >= 0) { spliceBlocks(taskCallIndex, 1); } From c326c4dc6f78eecbd30cb7240900b3d2eff7d824 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 4 Aug 2026 21:55:13 -0700 Subject: [PATCH 2/2] Polish IIFE helpers: tighten persistWithMergeBase and PromptActionBar Collapse dual try/catch in persistWithMergeBase into one path, destructure PromptActionBar props directly, and compute input lines once before the bordered/plain body branch. --- src/tui/components/chat-input.tsx | 29 +++++++++++----------- src/tui/hooks/use-provider-manager.ts | 35 ++++++++++----------------- 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/src/tui/components/chat-input.tsx b/src/tui/components/chat-input.tsx index e3b21145f..79548b547 100644 --- a/src/tui/components/chat-input.tsx +++ b/src/tui/components/chat-input.tsx @@ -68,7 +68,17 @@ export type ChatInputProps = { // Action bar above the prompt: revolving verb + interrupt/queue hint on the // left, profile · model · effort right-aligned. Null when nothing to show. -function PromptActionBar(props: { +function PromptActionBar({ + showSteerHint, + value, + steerOnEnter, + queuedCount, + verb, + profile, + model, + effort, + attachmentSummary, +}: { showSteerHint: boolean; value: string; steerOnEnter: boolean; @@ -79,17 +89,6 @@ function PromptActionBar(props: { effort?: string; attachmentSummary?: string; }): ReactNode { - const { - showSteerHint, - value, - steerOnEnter, - queuedCount, - verb, - profile, - model, - effort, - attachmentSummary, - } = props; // Enter and Alt+Enter are no-ops on an empty field, so with nothing typed // the hint advertises the interrupt chord instead. const hasPromptText = value.trim().length > 0; @@ -99,6 +98,7 @@ function PromptActionBar(props: { ? "Enter queues for orchestrator" : "Enter steer · Alt+Enter queue"; const steerText = queuedCount > 0 ? `${queuedCount} queued · ${actionsText}` : actionsText; + // exactOptionalPropertyTypes: omit undefined keys rather than pass them. const modelText = composePromptActionBarModelLabel({ ...(profile !== undefined ? { profile } : {}), ...(model !== undefined ? { model } : {}), @@ -790,9 +790,10 @@ export function ChatInput({ // When slash/@ pickers are open the input renders plainly so the // suggestion list sits flush above it without a competing border. + const inputLines = renderInputLines(); const inputBody = showSlash || showAt ? ( - {renderInputLines()} + {inputLines} ) : ( @@ -802,7 +803,7 @@ export function ChatInput({ flexDirection="column" paddingX={1} > - {renderInputLines()} + {inputLines} ); diff --git a/src/tui/hooks/use-provider-manager.ts b/src/tui/hooks/use-provider-manager.ts index 1c1dbfadd..26acbcbef 100644 --- a/src/tui/hooks/use-provider-manager.ts +++ b/src/tui/hooks/use-provider-manager.ts @@ -163,6 +163,8 @@ function persistGlobalSettings( // Shared disk-first persist path for catalog and tier writes. Loads a fresh // merge base, builds settings, optionally mutates in-memory state, then saves. // Fire-and-forget: callers `void` the promise so UI stays non-blocking. +// onBeforeSave runs only after load+build succeed so a failed re-read never +// leaves UI state ahead of disk. async function persistWithMergeBase(args: { globalSettingsPath: string; initialSettings: Settings | undefined; @@ -170,31 +172,22 @@ async function persistWithMergeBase(args: { onMessage: (msg: string) => void; successMessage: string; failPrefix: string; - /** Runs only after load+build succeed, before the disk write. */ onBeforeSave?: () => void; }): Promise { - let base: Settings | undefined; try { - base = await loadMergeBase(args.globalSettingsPath, args.initialSettings); - } catch (err) { - args.onMessage(`${args.failPrefix}: ${err instanceof Error ? err.message : String(err)}`); - return; - } - let settings: Settings; - try { - settings = args.buildSettings(base); + const base = await loadMergeBase(args.globalSettingsPath, args.initialSettings); + const settings = args.buildSettings(base); + args.onBeforeSave?.(); + persistGlobalSettings( + args.globalSettingsPath, + settings, + args.onMessage, + args.successMessage, + args.failPrefix, + ); } catch (err) { args.onMessage(`${args.failPrefix}: ${err instanceof Error ? err.message : String(err)}`); - return; } - args.onBeforeSave?.(); - persistGlobalSettings( - args.globalSettingsPath, - settings, - args.onMessage, - args.successMessage, - args.failPrefix, - ); } export function useProviderManager({ @@ -353,9 +346,7 @@ export function useProviderManager({ successMessage: string, ): void => { // Disk-first merge base: mid-session /plugins writes must survive a later - // provider save. Fail closed if settings cannot be re-read. Catalog state - // only updates after load+build succeed so a failed re-read never leaves - // the UI ahead of disk. + // provider save. Fail closed if settings cannot be re-read. void persistWithMergeBase({ globalSettingsPath, initialSettings,