From 5b0741e986c9f28d3cb9c16d0c6953fcf1857e87 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 22 Apr 2026 12:10:21 -0500 Subject: [PATCH] =?UTF-8?q?refactor(tui):=20consolidate=20agents=20overlay?= =?UTF-8?q?=20=E2=80=94=20share=20duration/root=20helpers=20via=20lib?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull duplicated rules into ui-tui/src/lib/subagentTree so the live overlay, disk snapshot label, and diff pane all speak one dialect: - export fmtDuration(seconds) — was a private helper in subagentTree; agentsOverlay's local secLabel/fmtDur/fmtElapsedLabel now wrap the same core (with UI-only empty-string policy). - export topLevelSubagents(items) — matches buildSubagentTree's orphan semantics (no parent OR parent not in snapshot). Replaces three hand- rolled copies across createGatewayEventHandler (disk label), agentsOverlay DiffPane, and prior inline filters. Also collapse agentsOverlay boilerplate: - replace IIFE title + inner `delta` helper with straight expressions; - introduce module-level diffMetricLine for replay-diff rows; - tighten OverlayScrollbar (single thumbColor expression, vBar/thumbBody). Adds unit coverage for the new exports (fmtDuration + topLevelSubagents). No behaviour change; 221 tests pass. --- ui-tui/src/__tests__/subagentTree.test.ts | 41 +++++ ui-tui/src/app/createGatewayEventHandler.ts | 19 +- ui-tui/src/components/agentsOverlay.tsx | 194 ++++++-------------- ui-tui/src/lib/subagentTree.ts | 20 +- 4 files changed, 121 insertions(+), 153 deletions(-) diff --git a/ui-tui/src/__tests__/subagentTree.test.ts b/ui-tui/src/__tests__/subagentTree.test.ts index 649b791ce..887754ce0 100644 --- a/ui-tui/src/__tests__/subagentTree.test.ts +++ b/ui-tui/src/__tests__/subagentTree.test.ts @@ -5,11 +5,13 @@ import { descendantIds, flattenTree, fmtCost, + fmtDuration, fmtTokens, formatSummary, hotnessBucket, peakHotness, sparkline, + topLevelSubagents, treeTotals, widthByDepth } from '../lib/subagentTree.js' @@ -367,3 +369,42 @@ describe('formatSummary', () => { ).toBe('d3 · 7 agents · 124 tools · 2m 14s · ⚡2') }) }) + +describe('fmtDuration', () => { + it('formats under a minute as plain seconds', () => { + expect(fmtDuration(0)).toBe('0s') + expect(fmtDuration(42)).toBe('42s') + expect(fmtDuration(59.4)).toBe('59s') + }) + + it('formats whole minutes without trailing seconds', () => { + expect(fmtDuration(60)).toBe('1m') + expect(fmtDuration(180)).toBe('3m') + }) + + it('mixes minutes and seconds', () => { + expect(fmtDuration(134)).toBe('2m 14s') + expect(fmtDuration(605)).toBe('10m 5s') + }) +}) + +describe('topLevelSubagents', () => { + it('returns items with no parent', () => { + const items = [makeItem({ id: 'a', index: 0 }), makeItem({ id: 'b', index: 1 })] + expect(topLevelSubagents(items).map(s => s.id)).toEqual(['a', 'b']) + }) + + it('excludes children whose parent is present', () => { + const items = [ + makeItem({ id: 'p', index: 0 }), + makeItem({ depth: 1, id: 'c', index: 0, parentId: 'p' }) + ] + + expect(topLevelSubagents(items).map(s => s.id)).toEqual(['p']) + }) + + it('promotes orphans whose parent is missing', () => { + const items = [makeItem({ id: 'a', index: 0 }), makeItem({ depth: 1, id: 'orphan', index: 1, parentId: 'ghost' })] + expect(topLevelSubagents(items).map(s => s.id)).toEqual(['a', 'orphan']) + }) +}) diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 395c2d3ac..1ec123f11 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -2,6 +2,7 @@ import { STREAM_BATCH_MS } from '../config/timing.js' import { buildSetupRequiredSections, SETUP_REQUIRED_TITLE } from '../content/setup.js' import type { CommandsCatalogResponse, DelegationStatusResponse, GatewayEvent, GatewaySkin } from '../gatewayTypes.js' import { rpcErrorMessage } from '../lib/rpc.js' +import { topLevelSubagents } from '../lib/subagentTree.js' import { formatToolCall, stripAnsi } from '../lib/text.js' import { fromSkin } from '../theme.js' import type { Msg, SubagentProgress } from '../types.js' @@ -66,20 +67,12 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: return min === 0 ? s.startedAt : Math.min(min, s.startedAt) }, 0) - // Match buildSubagentTree semantics: an agent is top-level if it has - // no parent OR its parent isn't in the snapshot (orphan). Otherwise - // the disk label would fall back to `${N} subagents` for any turn - // whose roots got pruned mid-flight. - const ids = new Set(subagents.map(s => s.id)) - const top = subagents.filter(s => !s.parentId || !ids.has(s.parentId)).slice(0, 2) + const top = topLevelSubagents(subagents) + .map(s => s.goal) + .filter(Boolean) + .slice(0, 2) - const label = top.length - ? top - .map(s => s.goal) - .filter(Boolean) - .slice(0, 2) - .join(' · ') - : `${subagents.length} subagents` + const label = top.length ? top.join(' · ') : `${subagents.length} subagents` await rpc('spawn_tree.save', { finished_at: Date.now() / 1000, diff --git a/ui-tui/src/components/agentsOverlay.tsx b/ui-tui/src/components/agentsOverlay.tsx index 62b315da7..a8ad91758 100644 --- a/ui-tui/src/components/agentsOverlay.tsx +++ b/ui-tui/src/components/agentsOverlay.tsx @@ -19,11 +19,13 @@ import { descendantIds, flattenTree, fmtCost, + fmtDuration, fmtTokens, formatSummary, hotnessBucket, peakHotness, sparkline, + topLevelSubagents, treeTotals, widthByDepth } from '../lib/subagentTree.js' @@ -89,22 +91,9 @@ const heatPalette = (t: Theme) => [t.color.bronze, t.color.amber, t.color.gold, // ── Pure helpers ───────────────────────────────────────────────────── -const fmtDur = (seconds?: number): string => { - if (!seconds || seconds <= 0) { - return '' - } +const fmtDur = (seconds?: number) => (seconds == null || seconds <= 0 ? '' : fmtDuration(seconds)) +const fmtElapsedLabel = (seconds: number) => (seconds < 0 ? '' : fmtDuration(seconds)) - if (seconds < 60) { - return `${Math.round(seconds)}s` - } - - const m = Math.floor(seconds / 60) - const s = Math.round(seconds - m * 60) - - return s === 0 ? `${m}m` : `${m}m ${s}s` -} - -/** Server duration if present; else live edge from `startedAt` (running / queued). */ const displayElapsedSeconds = (item: SubagentProgress, nowMs: number): number | null => { if (item.durationSeconds != null) { return item.durationSeconds @@ -117,22 +106,6 @@ const displayElapsedSeconds = (item: SubagentProgress, nowMs: number): number | return null } -/** Like fmtDur but allows 0s for just-started / still-running rows. */ -const fmtElapsedLabel = (seconds: number): string => { - if (seconds < 0) { - return '' - } - - if (seconds < 60) { - return `${Math.max(0, Math.round(seconds))}s` - } - - const m = Math.floor(seconds / 60) - const s = Math.round(seconds - m * 60) - - return s === 0 ? `${m}m` : `${m}m ${s}s` -} - const indentFor = (depth: number): string => ' '.repeat(Math.max(0, depth)) const formatRowId = (n: number): string => String(n + 1).padStart(2, ' ') const cycle = (order: readonly T[], current: T): T => order[(order.indexOf(current) + 1) % order.length]! @@ -144,22 +117,18 @@ const statusGlyph = (item: SubagentProgress, t: Theme) => { } const prepareRows = (tree: SubagentNode[], sort: SortMode, filter: FilterMode): SubagentNode[] => - tree.length === 0 - ? [] - : [...tree] - .sort(SORT_COMPARATORS[sort]) - .flatMap(n => flattenTree([n])) - .filter(FILTER_PREDICATES[filter]) + tree.length === 0 ? [] : flattenTree([...tree].sort(SORT_COMPARATORS[sort])).filter(FILTER_PREDICATES[filter]) + +const diffMetricLine = (name: string, a: number, b: number, fmt: (n: number) => string) => { + const d = b - a + const sign = d === 0 ? '' : d > 0 ? '+' : '-' + + return `${name}: ${fmt(a)} → ${fmt(b)} (${sign}${fmt(Math.abs(d)) || '0'})` +} // ── Sub-components ─────────────────────────────────────────────────── -/** - * Detail-pane scrollbar, polled on the parent tick. `TranscriptScrollbar` - * re-renders only on scroll events — fine for the main transcript, but the - * overlay's content reflows on accordion toggle without any scroll, so the - * thumb stays stale. Ticking forces a re-read; always drawing the track - * keeps the gutter visually stable for short content too. - */ +/** Polled on parent `tick` so accordions can resize the thumb without a scroll event. */ function OverlayScrollbar({ scrollRef, t, @@ -189,13 +158,11 @@ function OverlayScrollbar({ const thumbTop = scrollable ? Math.round((pos / Math.max(1, total - vp)) * travel) : 0 const below = Math.max(0, vp - thumbTop - thumb) - const trackLines = (n: number) => (n > 0 ? `${'│\n'.repeat(Math.max(0, n - 1))}│` : '') - const thumbLines = `${'┃\n'.repeat(Math.max(0, thumb - 1))}┃` - - const thumbColor = grab !== null ? t.color.gold : hover ? t.color.amber : t.color.amber + const vBar = (n: number) => (n > 0 ? `${'│\n'.repeat(n - 1)}│` : '') + const thumbBody = `${'┃\n'.repeat(Math.max(0, thumb - 1))}┃` + const thumbColor = grab !== null ? t.color.gold : t.color.amber const trackColor = hover ? t.color.bronze : t.color.dim - // Map a local row (0..vp-1) + grab offset to a scrollTop position. const jump = (row: number, offset: number) => { if (!s || !scrollable) { return @@ -223,21 +190,21 @@ function OverlayScrollbar({ > {!scrollable ? ( - {trackLines(vp)} + {vBar(vp)} ) : ( <> {thumbTop > 0 ? ( - {trackLines(thumbTop)} + {vBar(thumbTop)} ) : null} - {thumbLines} + {thumbBody} {below > 0 ? ( - {trackLines(below)} + {vBar(below)} ) : null} @@ -246,11 +213,6 @@ function OverlayScrollbar({ ) } -/** - * Horizontal ASCII Gantt strip. One bar per subagent, anchored by row id. - * The ruler below maps screen positions to wall-clock seconds so a bar that - * "ends in the middle" reads as "finished at ~Xs". - */ function GanttStrip({ cols, cursor, @@ -305,8 +267,6 @@ function GanttStrip({ return ' '.repeat(s) + '█'.repeat(fill) + ' '.repeat(Math.max(0, barWidth - s - fill)) } - // Wall-clock axis: more ticks on short windows so the scale visibly - // “counts up” with `now` instead of a single 0/10s pair. const charStep = totalSeconds < 20 && barWidth > 20 ? 5 : 10 const ruler = Array.from({ length: barWidth }, (_, i) => { @@ -388,10 +348,6 @@ function GanttStrip({ ) } -/** - * A collapsible section. Open-state lives on a shared atom so navigating - * between agents / list ↔ detail / history doesn't reset accordions. - */ function OverlaySection({ children, count, @@ -423,7 +379,6 @@ function OverlaySection({ ) } -/** `label · value` row with the detail-pane colour hierarchy. */ function Field({ name, t, value }: { name: string; t: Theme; value: ReactNode }) { return ( @@ -461,28 +416,21 @@ function Detail({ id, node, t }: { id?: string; node: SubagentNode; t: Theme }) {glyph} {item.goal} - + + {item.model ? : null} + {item.toolsets?.length ? : null} + + + {item.durationSeconds ? : null} + {item.iteration != null ? : null} + {item.apiCalls ? : null} - {item.model ? : null} - - {item.toolsets?.length ? : null} - - - - - - {item.durationSeconds ? : null} - - {item.iteration != null ? : null} - - {item.apiCalls ? : null} - {localTokens > 0 || localCost > 0 ? ( {localTokens > 0 ? ( @@ -577,19 +525,6 @@ function Detail({ id, node, t }: { id?: string; node: SubagentNode; t: Theme }) ) } -/** Pluck the label out of `formatToolCall` output: `Read_file("…")` → `Read_file`. */ -const latestToolLabel = (tools: readonly string[]): string => { - const last = tools[tools.length - 1] - - if (!last) { - return '' - } - - const paren = last.indexOf('(') - - return (paren > 0 ? last.slice(0, paren) : last).trim() -} - function ListRow({ active, index, @@ -613,15 +548,10 @@ function ListRow({ const goal = compactPreview(node.item.goal || 'subagent', width - 28 - node.item.depth * 2) const toolsCount = node.aggregate.totalTools > 0 ? ` ·${node.aggregate.totalTools}t` : '' const kids = node.children.length ? ` ·${node.children.length}↓` : '' - - // Running rows replace the moving-number clock (timeline already has it) - // with the most recent tool label — no per-tick re-render, but changes - // as activity flows, so the list still conveys motion. - const current = node.item.status === 'running' ? latestToolLabel(node.item.tools) : '' - const trailing = current ? ` · ${compactPreview(current, 14)}` : '' - - // Selection pattern mirrors sessionPicker: inverse + amber for contrast - // across any theme, body stays cornsilk, stats dim. + const line = node.item.status === 'running' ? node.item.tools.at(-1) : undefined + const paren = line ? line.indexOf('(') : -1 + const toolShort = line ? (paren > 0 ? line.slice(0, paren) : line).trim() : '' + const trailing = toolShort ? ` · ${compactPreview(toolShort, 14)}` : '' const fg = active ? t.color.amber : t.color.cornsilk return ( @@ -670,8 +600,7 @@ function DiffPane({ - {snapshot.subagents - .filter(s => !s.parentId) + {topLevelSubagents(snapshot.subagents) .slice(0, 8) .map(s => { const { color, glyph } = statusGlyph(s, t) @@ -708,12 +637,6 @@ function DiffView({ } }) - const delta = (name: string, a: number, b: number, fmt: (n: number) => string): string => { - const sign = b - a === 0 ? '' : b > a ? '+' : '-' - - return `${name}: ${fmt(a)} → ${fmt(b)} (${sign}${fmt(Math.abs(b - a)) || '0'})` - } - const round = (n: number) => String(Math.round(n)) const sumTokens = (x: typeof aTotals) => x.inputTokens + x.outputTokens const dollars = (n: number) => fmtCost(n) || '$0.00' @@ -738,16 +661,20 @@ function DiffView({ Δ - {delta('agents', aTotals.descendantCount, bTotals.descendantCount, round)} - {delta('tools', aTotals.totalTools, bTotals.totalTools, round)} - {delta('depth', aTotals.maxDepthFromHere, bTotals.maxDepthFromHere, round)} + {diffMetricLine('agents', aTotals.descendantCount, bTotals.descendantCount, round)} + + {diffMetricLine('tools', aTotals.totalTools, bTotals.totalTools, round)} + + {diffMetricLine('depth', aTotals.maxDepthFromHere, bTotals.maxDepthFromHere, round)} - {delta('duration', aTotals.totalDuration, bTotals.totalDuration, n => `${n.toFixed(1)}s`)} + {diffMetricLine('duration', aTotals.totalDuration, bTotals.totalDuration, n => `${n.toFixed(1)}s`)} - {delta('tokens', sumTokens(aTotals), sumTokens(bTotals), fmtTokens)} - {delta('cost', aTotals.costUsd, bTotals.costUsd, dollars)} + + {diffMetricLine('tokens', sumTokens(aTotals), sumTokens(bTotals), fmtTokens)} + + {diffMetricLine('cost', aTotals.costUsd, bTotals.costUsd, dollars)} ) @@ -913,6 +840,7 @@ export function AgentsOverlay({ gw, initialHistoryIndex = 0, onClose, t }: Agent // ── Input ────────────────────────────────────────────────────────── const detailPageSize = Math.max(4, rowsH - 2) + const wheelDetailDy = 3 const scrollDetail = (dy: number) => detailScrollRef.current?.scrollBy(dy) useInput((ch, key) => { @@ -958,14 +886,12 @@ export function AgentsOverlay({ gw, initialHistoryIndex = 0, onClose, t }: Agent return scrollDetail(detailPageSize) } - // Wheel = smooth pixel scroll; arrows = 2-row nudge. Overlay's - // useInput supersedes the global wheel handler so we re-bind here. if (key.wheelUp) { - return scrollDetail(-3) + return scrollDetail(-wheelDetailDy) } if (key.wheelDown) { - return scrollDetail(3) + return scrollDetail(wheelDetailDy) } if (key.upArrow || ch === 'k') { @@ -1036,20 +962,12 @@ export function AgentsOverlay({ gw, initialHistoryIndex = 0, onClose, t }: Agent ? `caps d${delegation.maxSpawnDepth}/${delegation.maxConcurrentChildren ?? '?'}` : '' - // Single header line — title · metrics. An earlier "title + subtitle" - // variant wrapped on narrow terminals which looked like the header was - // rendering twice, and a one-line header makes it obvious at a glance - // whether the turn is live or finished. - const title = (() => { - if (!replayMode || !effectiveSnapshot) { - return `Spawn tree${delegation.paused ? ' · ⏸ paused' : ''}` - } - - const at = new Date(effectiveSnapshot.finishedAt).toLocaleTimeString() - const position = historyIndex > 0 ? `Replay ${historyIndex}/${history.length}` : 'Last turn' - - return `${position} · finished ${at}` - })() + const title = + replayMode && effectiveSnapshot + ? `${historyIndex > 0 ? `Replay ${historyIndex}/${history.length}` : 'Last turn'} · finished ${new Date( + effectiveSnapshot.finishedAt + ).toLocaleTimeString()}` + : `Spawn tree${delegation.paused ? ' · ⏸ paused' : ''}` const metaLine = [formatSummary(totals), spark, capsLabel, mix ? `· ${mix}` : ''].filter(Boolean).join(' ') diff --git a/ui-tui/src/lib/subagentTree.ts b/ui-tui/src/lib/subagentTree.ts index cad6005b7..513559b80 100644 --- a/ui-tui/src/lib/subagentTree.ts +++ b/ui-tui/src/lib/subagentTree.ts @@ -297,9 +297,13 @@ export function fmtTokens(n: number): string { return `${Math.round(n / 1000)}k` } -function fmtDuration(seconds: number): string { +/** + * `Ns` / `Nm` / `Nm Ss` formatter for seconds. Shared with the agents + * overlay so the timeline + list + summary all speak the same dialect. + */ +export function fmtDuration(seconds: number): string { if (seconds < 60) { - return `${Math.round(seconds)}s` + return `${Math.max(0, Math.round(seconds))}s` } const m = Math.floor(seconds / 60) @@ -308,6 +312,18 @@ function fmtDuration(seconds: number): string { return s === 0 ? `${m}m` : `${m}m ${s}s` } +/** + * A subagent is top-level if it has no `parentId`, or its parent isn't in + * the same snapshot (orphaned by a pruned mid-flight root). Same rule + * `buildSubagentTree` uses — keep call sites consistent across the live + * view, disk label, and diff pane. + */ +export function topLevelSubagents(items: readonly SubagentProgress[]): SubagentProgress[] { + const ids = new Set(items.map(s => s.id)) + + return items.filter(s => !s.parentId || !ids.has(s.parentId)) +} + /** * Normalize a node's hotness into a palette index 0..N-1 where N = buckets. * Higher hotness = "hotter" colour. Normalized against the tree's peak hotness