From 7eb461d693cfb5940190adb30279cf26845e6d22 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 6 Aug 2026 21:19:44 -0500 Subject: [PATCH] refactor(desktop): share how a tool row renders The transcript decides what a tool call draws and the render budget has to price it. Both sides need the same answer, so the classification moves out of the tool renderer into its own module rather than the budget importing the formatting and i18n weight of fallback-model to ask one question. Adds isSilentTool for the rows that render nothing at all: todo is hoisted to its own panel, and a reaction's UI is the emoji on the bubble. --- .../assistant-ui/tool/fallback-model/index.ts | 10 ++--- .../assistant-ui/tool/fallback.test.ts | 14 +++++- .../components/assistant-ui/tool/fallback.tsx | 20 ++------- apps/desktop/src/lib/tool-render-class.ts | 44 +++++++++++++++++++ 4 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 apps/desktop/src/lib/tool-render-class.ts diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts index f1e2675002363..2a21657f68e63 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts @@ -2,6 +2,7 @@ import { type ToolTitleKey, translateNow } from '@/i18n' import { normalizeExternalUrl } from '@/lib/external-link' import { summarizeShellCommand } from '@/lib/summarize-command' import { capitalize, normalize } from '@/lib/text' +import { isCardTool, isFileEditTool, isSilentTool } from '@/lib/tool-render-class' import { extractToolErrorMessage, formatToolResultSummary } from '@/lib/tool-result-summary' import { @@ -31,11 +32,10 @@ export * from './format' export * from './targets' export * from './types' -const FILE_EDIT_TOOL_NAMES = new Set(['edit_file', 'patch', 'write_file']) - -export function isFileEditTool(toolName: string): boolean { - return FILE_EDIT_TOOL_NAMES.has(toolName) -} +// The transcript's render budget prices a turn by the same classification, so +// it lives in `@/lib/tool-render-class` where both sides can reach it without +// pulling this module's formatting/i18n weight into the cost path. +export { isCardTool, isFileEditTool, isSilentTool } export interface DiffLineStats { added: number diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.test.ts b/apps/desktop/src/components/assistant-ui/tool/fallback.test.ts index abe38c26f7dd6..b9847bff08357 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.test.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from 'vitest' -import { isCardTool, splitRunItems, technicalTrace } from './fallback' +import { isCardTool, isSilentTool } from '@/lib/tool-render-class' + +import { splitRunItems, technicalTrace } from './fallback' describe('isCardTool', () => { it('keeps what the user has to look at out of a summary', () => { @@ -19,6 +21,16 @@ describe('isCardTool', () => { }) }) +describe('isSilentTool', () => { + it('names the rows that render nothing in the transcript', () => { + // `todo` is hoisted to its own panel; a reaction's UI is the emoji on the + // bubble. The render budget must not charge for either. + expect(isSilentTool('todo')).toBe(true) + expect(isSilentTool('react_to_message')).toBe(true) + expect(isSilentTool('terminal')).toBe(false) + }) +}) + describe('splitRunItems', () => { it('collapses a stretch of activity into one run', () => { expect(splitRunItems(['read_file', 'search_files', 'terminal'])).toEqual([{ end: 2, kind: 'run', start: 0 }]) diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx index 0ea7f4760fd31..6e56d0918c194 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -53,6 +53,7 @@ import { cleanVisibleText, countDiffLineStats, inlineDiffFromResult, + isCardTool, isFileEditTool, isPreviewableTarget, looksRedundant, @@ -738,22 +739,9 @@ function TerminalTranscript({ command, exitCode }: TerminalTranscriptProps) { } // Tools that draw their own surface and must never be folded into a run's -// summary. Two kinds, for the same reason — the thing on screen IS the point: -// -// - File edits are the deliverable, not scaffolding. The diff is what the -// user reviews, so it stays visible at its place in the turn, live and -// settled, the way a PR shows its changes. -// - `clarify`, `image_generate` and `delegate_task` bypass ToolEntry to -// render their own markup: a question the user has to answer, an image -// they asked for, the several agents a fan-out is running. -// -// Everything else is ephemeral activity — reads, searches, commands — which is -// what a run summarizes and what the live ticker cycles through. -const CARD_TOOLS = new Set(['clarify', 'delegate_task', 'image_generate']) - -export function isCardTool(toolName: string): boolean { - return CARD_TOOLS.has(toolName) || isFileEditTool(toolName) -} +// summary live in `fallback-model` (`isCardTool`) — the DOM render budget +// prices a turn by the same rule, so both sides have to agree on which rows +// collapse into a summary line and which mount their own markup. export type RunItem = { end: number; kind: 'run'; start: number } | { index: number; kind: 'card' } diff --git a/apps/desktop/src/lib/tool-render-class.ts b/apps/desktop/src/lib/tool-render-class.ts new file mode 100644 index 0000000000000..810b35fe57613 --- /dev/null +++ b/apps/desktop/src/lib/tool-render-class.ts @@ -0,0 +1,44 @@ +/** + * Which surface a tool call renders as. + * + * Two consumers have to agree on this and they sit on opposite sides of the + * app: the transcript decides what to draw, and the DOM render budget decides + * how much of the transcript to mount. Pricing a turn correctly means pricing + * what the grouping actually renders, so the classification lives on its own + * rather than inside either one. + */ + +const FILE_EDIT_TOOL_NAMES = new Set(['edit_file', 'patch', 'write_file']) + +/** Renders a diff — the deliverable of the turn, and the one card whose cost scales. */ +export function isFileEditTool(toolName: string): boolean { + return FILE_EDIT_TOOL_NAMES.has(toolName) +} + +// Tools that draw their own surface and must never be folded into a run's +// summary. Two kinds, for the same reason — the thing on screen IS the point: +// +// - File edits are the deliverable, not scaffolding. The diff is what the +// user reviews, so it stays visible at its place in the turn, live and +// settled, the way a PR shows its changes. +// - `clarify`, `image_generate` and `delegate_task` bypass ToolEntry to +// render their own markup: a question the user has to answer, an image +// they asked for, the several agents a fan-out is running. +// +// Everything else is ephemeral activity — reads, searches, commands — which is +// what a run summarizes and what the live ticker cycles through. +const CARD_TOOL_NAMES = new Set(['clarify', 'delegate_task', 'image_generate']) + +export function isCardTool(toolName: string): boolean { + return CARD_TOOL_NAMES.has(toolName) || isFileEditTool(toolName) +} + +// Activity tools that render nothing at all: `todo` parts are hoisted to a +// dedicated panel above the message content, and a reaction's UI is the emoji +// landing on the bubble. Both still render when they FAIL, which is a bounded +// error row either way. +const SILENT_TOOL_NAMES = new Set(['react_to_message', 'todo']) + +export function isSilentTool(toolName: string): boolean { + return SILENT_TOOL_NAMES.has(toolName) +}