From 55ebb40524614db15fe0380e0fc3acfc07802131 Mon Sep 17 00:00:00 2001 From: Muhammad Zakir Ramadhan <61570975+zakirkun@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:43:13 +0700 Subject: [PATCH] Show tool detail and outcomes in the transcript Each tool line carries the arguments that identify the call - the paths a batch read is about to pull in, the files a patch touches - and its result line carries a one-line outcome. Worker approvals are labelled as subagent asks, and a subagent result attaches to its step in the panel. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/ui/App.tsx | 209 +++++++++++++++++++++++++++++++++++++--- src/ui/Panels.tsx | 64 ++++++++---- test/ui-panels.test.tsx | 26 ++++- 3 files changed, 269 insertions(+), 30 deletions(-) diff --git a/src/ui/App.tsx b/src/ui/App.tsx index e458cce..8fbbe7a 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -21,7 +21,7 @@ import { PromptInput } from './PromptInput'; type Line = | { key: string; kind: 'user'; text: string } | { key: string; kind: 'assistant'; text: string } - | { key: string; kind: 'tool'; name: string; summary: string; ok: boolean } + | { key: string; kind: 'tool'; name: string; detail: string[]; result?: string; ok: boolean } | { key: string; kind: 'info'; text: string } | { key: string; kind: 'error'; text: string }; @@ -110,6 +110,18 @@ export function applySubagentEvent(current: SubagentView[], event: SubagentEvent return current.map((a) => a.id === event.id ? { ...a, steps: [...a.steps, { tool: event.tool, summary: event.summary }] } : a, ); + case 'result': + // Attaches to the step it answers rather than appending, so a subagent's + // step count stays the number of calls it made. + return current.map((a) => { + if (a.id !== event.id) return a; + const last = a.steps.at(-1); + if (!last || last.tool !== event.tool || last.outcome !== undefined) return a; + return { + ...a, + steps: [...a.steps.slice(0, -1), { ...last, outcome: event.summary, ok: event.ok }], + }; + }); case 'end': return current.map((a) => (a.id === event.id ? { ...a, status: event.ok ? 'done' : 'failed' } : a)); case 'error': @@ -160,7 +172,7 @@ const nextKey = () => `l${seq++}`; function preview(input: unknown): string { if (input === null || typeof input !== 'object') return String(input); const o = input as Record; - const first = o['command'] ?? o['path'] ?? o['pattern'] ?? o['description'] ?? o['question'] ?? o['name']; + const first = o['command'] ?? o['path'] ?? o['pattern'] ?? o['url'] ?? o['description'] ?? o['question'] ?? o['name']; if (typeof first === 'string') return first.length > 90 ? `${first.slice(0, 90)}...` : first; // A tool with no obvious label, e.g. todo_write, gets a shape rather than a @@ -171,6 +183,157 @@ function preview(input: unknown): string { return keys.length === 0 ? '' : keys.slice(0, 3).join(', '); } +const clip = (s: string, n = 68) => (s.length > n ? `${s.slice(0, n)}...` : s); + +/** + * The arguments that matter for one call, one per line. + * + * `preview` picks a single field, which loses exactly the information a reader + * wants: a `read_file` with an offset, a `grep` scoped by `include`, the twenty + * paths a batch read is about to pull in. This is what goes under the tool line in + * the transcript and beside the spinner while a call is in flight. + */ +export function toolDetail(name: string, input: unknown): string[] { + if (input === null || typeof input !== 'object') return []; + const o = input as Record; + const str = (k: string) => (typeof o[k] === 'string' ? (o[k] as string) : undefined); + const num = (k: string) => (typeof o[k] === 'number' ? (o[k] as number) : undefined); + const bool = (k: string) => o[k] === true; + + switch (name) { + case 'read_file': { + const range = num('offset') ? `lines ${num('offset')}${num('limit') ? `-${num('offset')! + num('limit')! - 1}` : '+'}` : undefined; + return [clip(str('path') ?? ''), ...(range ? [range] : [])]; + } + case 'read_many_files': { + const files = Array.isArray(o['files']) ? (o['files'] as { path?: unknown }[]) : []; + const paths = files.map((f) => (typeof f.path === 'string' ? f.path : '?')); + // Every path, not a count: the point of showing this is knowing what is + // about to enter the context. + return paths.slice(0, 8).map(clip).concat(paths.length > 8 ? [`... ${paths.length - 8} more`] : []); + } + case 'write_file': { + const content = str('content') ?? ''; + return [clip(str('path') ?? ''), `${content.split('\n').length} lines, ${content.length} chars`]; + } + case 'edit_file': { + const old = str('oldString') ?? ''; + return [ + clip(str('path') ?? ''), + `- ${clip(old.split('\n')[0] ?? '', 60)}${old.includes('\n') ? ` (+${old.split('\n').length - 1} lines)` : ''}`, + ...(bool('replaceAll') ? ['every occurrence'] : []), + ]; + } + case 'multi_edit': { + const edits = Array.isArray(o['edits']) ? (o['edits'] as { oldString?: unknown }[]) : []; + return [ + clip(str('path') ?? ''), + ...edits.slice(0, 5).map((e, i) => { + const old = typeof e.oldString === 'string' ? e.oldString : ''; + return `${i + 1}. - ${clip(old.split('\n')[0] ?? '', 58)}`; + }), + ...(edits.length > 5 ? [`... ${edits.length - 5} more edits`] : []), + ]; + } + case 'apply_patch': { + const patch = str('patch') ?? ''; + const ops = [...patch.matchAll(/^\*\*\* (Add|Update|Delete) File: (.+)$/gm)].map( + (m) => `${m[1]!.toLowerCase()} ${m[2]!.trim()}`, + ); + const moves = [...patch.matchAll(/^\*\*\* Move to: (.+)$/gm)].map((m) => `move to ${m[1]!.trim()}`); + return [...ops, ...moves].slice(0, 10).map(clip); + } + case 'bash': { + const timeout = num('timeout'); + return [ + ...(str('command') ?? '').split('\n').slice(0, 4).map((l) => clip(l)), + ...(timeout ? [`timeout ${Math.round(timeout / 1000)}s`] : []), + ]; + } + case 'grep': { + const parts = [`/${str('pattern') ?? ''}/`]; + if (str('include')) parts.push(`in ${str('include')}`); + if (bool('ignoreCase')) parts.push('case-insensitive'); + if (bool('includeIgnored')) parts.push('including ignored files'); + return [clip(parts.join(' '), 90)]; + } + case 'glob': + return [clip(str('pattern') ?? ''), ...(bool('includeIgnored') ? ['including ignored files'] : [])]; + case 'list_dir': + return [clip(str('path') ?? '.'), `depth ${num('depth') ?? 2}`]; + case 'web_fetch': + return [clip(str('url') ?? '', 90)]; + case 'task': { + const kind = str('kind') ?? 'explore'; + return [`${kind}${kind === 'worker' ? ' (writes)' : ''}: ${clip(str('description') ?? '')}`]; + } + case 'todo_write': { + const todos = Array.isArray(o['todos']) ? (o['todos'] as { content?: unknown; status?: unknown }[]) : []; + return todos.slice(0, 6).map((t) => `${String(t.status ?? '')}: ${clip(String(t.content ?? ''), 56)}`); + } + case 'git_show': + return [str('ref') ?? '', ...(str('path') ? [clip(str('path')!)] : [])]; + case 'git_log': + return [`${num('limit') ?? 15} commits`, ...(str('path') ? [clip(str('path')!)] : [])]; + case 'git_diff': + return [bool('staged') ? 'staged' : 'working tree', ...(str('path') ? [clip(str('path')!)] : [])]; + case 'git_blame': { + const from = num('startLine'); + return [clip(str('path') ?? ''), ...(from ? [`lines ${from}-${num('endLine') ?? from + 40}`] : [])]; + } + case 'remember': + return [`${str('kind') ?? 'fact'}: ${clip(str('text') ?? '', 60)}`]; + case 'recall': + case 'forget': + return [clip(str('query') ?? str('text') ?? '')]; + case 'skill': + return [str('name') ?? '']; + default: { + const label = preview(input); + return label ? [clip(label, 90)] : []; + } + } +} + +/** First line of a tool result, so the transcript shows an outcome not just a call. */ +export function resultSummary(name: string, output: unknown): string { + const text = typeof output === 'string' ? output : JSON.stringify(output ?? ''); + if (!text) return ''; + + const lines = text.split('\n').filter((l) => l.trim().length > 0); + const first = lines[0] ?? ''; + + // grep and glob return one hit per line, so the count is the useful summary. + if (name === 'grep' || name === 'glob') { + if (/^No (matches|files matched)/.test(first)) return first; + return `${lines.length} ${name === 'grep' ? 'hit' : 'path'}${lines.length === 1 ? '' : 's'}`; + } + if (name === 'read_file' || name === 'read_many_files') return `${lines.length} lines`; + if (name === 'bash') { + const exit = /^exit: (\d+)/.exec(first); + return exit ? `exit ${exit[1]}${lines.length > 1 ? `, ${lines.length - 1} lines out` : ''}` : clip(first); + } + return clip(first, 78); +} + +/** + * Attaches a result to the most recent unanswered call of that tool. + * + * Matched on name rather than call id because the transcript is a flat list of + * committed lines, and a parallel pair of calls to the same tool is rare enough + * that "the newest one still waiting" is right in practice and cheap. + */ +function withResult(lines: Line[], name: string, result: string, ok: boolean): Line[] { + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i]!; + if (line.kind !== 'tool' || line.name !== name || line.result !== undefined) continue; + const next = [...lines]; + next[i] = { ...line, result, ok }; + return next; + } + return lines; +} + function ApprovalDetail({ name, input }: { name: string; input: unknown }) { const o = (input ?? {}) as Record; if (name === 'bash') return {String(o['command'] ?? '')}; @@ -197,13 +360,18 @@ function Approval({ pending }: { pending: Pending }) { {pending.req.repeated ? `${pending.req.toolName} is repeating the same call` - : `${pending.req.toolName} wants to run`} + : pending.req.subagent + ? `a worker subagent wants to run ${pending.req.toolName}` + : `${pending.req.toolName} wants to run`} {pending.req.repeated && ( allowed by the rules, but this is the third identical call this turn )} + {pending.req.subagent && !pending.req.repeated && ( + delegated work, gated by your rules exactly as a direct call is + )} {!pending.req.repeated && pending.req.matchedPattern && pending.req.matchedPattern !== '*' && ( {`matched ${pending.req.toolName}: "${pending.req.matchedPattern}"`} )} @@ -293,7 +461,7 @@ export function App({ const [inputGeneration, setInputGeneration] = useState(0); const [inputCursor, setInputCursor] = useState(0); const [toolOutput, setToolOutput] = useState(''); - const [active, setActive] = useState<{ name: string; summary?: string } | undefined>(); + const [active, setActive] = useState<{ name: string; detail?: string[] } | undefined>(); const [thinking, setThinking] = useState(''); const [thinkingOpen, setThinkingOpen] = useState(false); const [queue, setQueue] = useState([]); @@ -509,19 +677,22 @@ export function App({ setActive({ name: ev.name }); break; case 'tool-call': - setActive({ name: ev.name, summary: preview(ev.input) }); - push({ kind: 'tool', name: ev.name, summary: preview(ev.input), ok: true }); + setActive({ name: ev.name, detail: toolDetail(ev.name, ev.input) }); + push({ kind: 'tool', name: ev.name, detail: toolDetail(ev.name, ev.input), ok: true }); break; case 'tool-output': setToolOutput((s) => `${s}${ev.chunk}`.slice(-2000)); break; case 'tool-error': setActive(undefined); - push({ kind: 'tool', name: ev.name, summary: String(ev.error), ok: false }); + // Attaches to the call rather than pushing a second line, so the + // transcript reads as one entry per call with its outcome. + setHistory((h) => withResult(h, ev.name, String(ev.error), false)); break; case 'tool-result': setActive(undefined); setToolOutput(''); + setHistory((h) => withResult(h, ev.name, resultSummary(ev.name, ev.output), true)); setNotebook(session.notebook.state()); break; case 'tool-denied': @@ -867,9 +1038,25 @@ export function App({ {line.kind === 'user' && {`> ${line.text}`}} {line.kind === 'assistant' && } {line.kind === 'tool' && ( - - {line.ok ? '*' : 'x'} {line.name}({line.summary}) - + + + {line.ok ? '*' : 'x'} + + {line.name} + + {line.detail[0] !== undefined && {` ${line.detail[0]}`}} + + {line.detail.slice(1).map((d, i) => ( + + {` ${d}`} + + ))} + {line.result !== undefined && line.result.length > 0 && ( + + {` ${line.ok ? '->' : 'x'} ${line.result}`} + + )} + )} {line.kind === 'info' && {line.text}} {line.kind === 'error' && error: {line.text}} @@ -1019,7 +1206,7 @@ export function App({ {busy && !modal && ( - {active && } + {active && } working... esc to interrupt diff --git a/src/ui/Panels.tsx b/src/ui/Panels.tsx index 3410a40..b088aef 100644 --- a/src/ui/Panels.tsx +++ b/src/ui/Panels.tsx @@ -40,24 +40,37 @@ export function TodoPanel({ todos, width = 40 }: { todos: Todo[]; width?: number ); } +export type SubagentStep = { + tool: string; + summary: string; + /** First line of the result, once it arrives. */ + outcome?: string; + ok?: boolean; +}; + export type SubagentView = { id: string; kind: SubagentKind; description: string; - steps: { tool: string; summary: string }[]; + steps: SubagentStep[]; status: 'running' | 'done' | 'failed'; error?: string; }; -const KIND_LABEL: Record = { explore: 'explore', review: 'review' }; +const KIND_LABEL: Record = { explore: 'explore', review: 'review', worker: 'worker' }; + +/** A worker can write, so its panel entry has to be distinguishable at a glance. */ +const KIND_COLOUR: Record = { explore: 'cyan', review: 'blue', worker: 'yellow' }; /** * Live view of delegated work. * * A subagent can run for a minute over many files; without this the parent's spinner - * is the only feedback and the user cannot tell progress from a hang. + * is the only feedback and the user cannot tell progress from a hang. A `worker` + * additionally changes the workspace, so its calls and their outcomes are shown + * rather than just a step count. */ -export function SubagentPanel({ agents }: { agents: SubagentView[] }) { +export function SubagentPanel({ agents, steps = 4 }: { agents: SubagentView[]; steps?: number }) { if (agents.length === 0) return null; return ( @@ -72,14 +85,20 @@ export function SubagentPanel({ agents }: { agents: SubagentView[] }) { ) : ( {a.status === 'done' ? '*' : 'x'} )} - {` ${KIND_LABEL[a.kind]}`} + {` ${KIND_LABEL[a.kind]}`} + {a.kind === 'worker' && {' (writes)'}} {`: ${a.description}`} {` ${a.steps.length} step${a.steps.length === 1 ? '' : 's'}`} - {a.steps.slice(-3).map((s, i) => ( - - {` ${s.tool}(${s.summary.slice(0, 60)})`} - + {a.steps.slice(-steps).map((s, i) => ( + + {` ${s.tool}(${s.summary.slice(0, 58)})`} + {s.outcome !== undefined && ( + + {` ${s.ok === false ? 'x' : '->'} ${s.outcome}`} + + )} + ))} {a.error && {` ${a.error}`}} @@ -109,17 +128,26 @@ export function OutputPanel({ text, lines = 8 }: { text: string; lines?: number * The tool call in flight, from tool-start until its result arrives. * * A read of a large file or a two-minute test run is otherwise indistinguishable - * from a hang, and the name arrives before the arguments finish streaming, so the - * summary is filled in a moment later. + * from a hang. The name arrives before the arguments finish streaming, so `detail` + * is filled in a moment later; it is a list because one line rarely says enough — + * a batch read is about to pull in twenty paths, and which twenty is the point. */ -export function ActiveTool({ name, summary }: { name: string; summary?: string }) { +export function ActiveTool({ name, detail = [] }: { name: string; detail?: readonly string[] }) { return ( - - - - - {` ${name}`} - {summary ? {` ${summary}`} : null} + + + + + + {` ${name}`} + {detail[0] !== undefined && {` ${detail[0]}`}} + + {detail.slice(1, 6).map((d, i) => ( + + {` ${d}`} + + ))} + {detail.length > 6 && {` ... ${detail.length - 6} more`}} ); } diff --git a/test/ui-panels.test.tsx b/test/ui-panels.test.tsx index f8b03eb..37ffe3f 100644 --- a/test/ui-panels.test.tsx +++ b/test/ui-panels.test.tsx @@ -151,7 +151,7 @@ test('the info panel renders a markdown body', () => { }); test('the active tool line names the tool and the file it is touching', () => { - const app = render(); + const app = render(); const frame = app.lastFrame() ?? ''; expect(frame).toContain('read_file'); expect(frame).toContain('src/session.ts'); @@ -164,6 +164,18 @@ test('the active tool line renders before the arguments have arrived', () => { app.unmount(); }); +test('the active tool line shows several detail lines and caps the rest', () => { + const detail = Array.from({ length: 9 }, (_, i) => `src/file${i}.ts`); + const app = render(); + const frame = app.lastFrame() ?? ''; + // One line is never enough for a batch read: which paths are about to enter the + // context is the whole point of showing it. + expect(frame).toContain('src/file0.ts'); + expect(frame).toContain('src/file5.ts'); + expect(frame).toContain('3 more'); + app.unmount(); +}); + test('thinking collapses to a token count, and expands on request', () => { const text = 'x'.repeat(1648); const collapsed = render(); @@ -214,6 +226,18 @@ test('subagent events fold into the panel view', () => { expect(view[1]).toMatchObject({ id: 'b', status: 'failed', error: 'exploded' }); }); +test('a subagent result attaches to its step without clearing the panel state', () => { + const started = applySubagentEvent([], { type: 'start', id: 'a', kind: 'explore', description: 'find auth' }); + const stepped = applySubagentEvent(started, { type: 'step', id: 'a', tool: 'grep', summary: 'login' }); + const view = applySubagentEvent(stepped, { type: 'result', id: 'a', tool: 'grep', summary: '2 hits', ok: true }); + + expect(view).toHaveLength(1); + expect(view[0]).toMatchObject({ + id: 'a', + steps: [{ tool: 'grep', summary: 'login', outcome: '2 hits', ok: true }], + }); +}); + test('an event for an unknown id is ignored rather than throwing', () => { const view = applySubagentEvent([], { type: 'step', id: 'ghost', tool: 'grep', summary: 'x' }); expect(view).toEqual([]);