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 0d844d302212..75bacb338a5d 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.test.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from 'vitest' import { isUnboundableTool, shouldBoundToolGroup, technicalTrace } from './fallback' -import { isFileEditTool } from './fallback-model' describe('shouldBoundToolGroup', () => { it('bounds long runs of ordinary tool calls', () => { @@ -23,25 +22,18 @@ describe('isUnboundableTool', () => { expect(isUnboundableTool('image_generate')).toBe(true) }) - it('exempts tools whose body is a code block the user reads', () => { - expect(isUnboundableTool('execute_code')).toBe(true) - expect(isUnboundableTool('read_file')).toBe(true) - }) - - // The window clips a diff to a ~2-row viewport, so every tool that renders - // one has to be exempt. Derived from isFileEditTool rather than re-listed, - // so a newly supported edit tool can't be exempt in one place and clipped - // in the other. - it('exempts every file-edit tool, so diffs are never clipped', () => { - for (const toolName of ['edit_file', 'patch', 'write_file']) { - expect(isFileEditTool(toolName)).toBe(true) - expect(isUnboundableTool(toolName)).toBe(true) + // Everything ToolEntry renders carries `data-tool-row`, so the + // `:has([data-tool-row][data-tool-open])` rule in styles.css lifts the cap + // on its own. A diff row mounts open and frees the group immediately; a + // collapsed row has no body in the DOM to clip. Exempting these in JS + // instead vetoed grouping for the whole run — and since reads and edits are + // most of a coding session, runs of 19 calls never collapsed at all. + it('bounds the rows the CSS break-out already covers', () => { + for (const toolName of ['read_file', 'execute_code', 'edit_file', 'patch', 'write_file']) { + expect(isUnboundableTool(toolName)).toBe(false) } }) - // Console output is a log tail: the last lines are the ones that matter, - // which is exactly what the bounded window pins. Keeping it boundable is - // what stops the exemption from swallowing the feature whole. it('still bounds console output and other ordinary rows', () => { expect(isUnboundableTool('terminal')).toBe(false) expect(isUnboundableTool('web_search')).toBe(false) diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx index 1ba2fd25a465..7c004e07c1fa 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -700,29 +700,24 @@ function TerminalTranscript({ command, exitCode }: TerminalTranscriptProps) { // auto-scrolling window; fewer than this stays a plain inline stack. const TOOL_GROUP_SCROLL_THRESHOLD = 3 -// Tools whose body (an interactive form, a full-size image, a syntax- -// highlighted code/diff block) must never be trapped behind the window's -// max-height + fade mask. A run holding any of them stays a plain, fully- -// visible stack no matter how long it is. +// Tools whose body must never be trapped behind the window's max-height + +// fade mask. A run holding any of them stays a plain, fully-visible stack no +// matter how long it is. // -// A row rendered by ToolEntry carries `data-tool-row`, so once the user -// expands it the `:has([data-tool-row][data-tool-open])` rule in styles.css -// lifts the cap on its own. That escape hatch is why most tools are safe to -// bound. These are the ones it cannot reach: +// This list is deliberately tiny. A row rendered by ToolEntry carries +// `data-tool-row`, and the `:has([data-tool-row][data-tool-open])` rule in +// styles.css lifts the cap whenever one is open — so anything ToolEntry +// renders takes care of itself. A code/diff row mounts open (see +// `defaultOpen`), lifting the cap the moment it appears; a collapsed row is a +// one-line status with no body in the DOM at all, so there is nothing to clip. // -// - `clarify` / `image_generate` render their own components and never emit -// `data-tool-row`, so no amount of expanding frees them. -// - the code tools *do* emit it, but their body is a code block the user -// reads rather than a one-line status — peering at a diff through a -// ~2-row viewport until you think to expand it is the bug. Console output -// (`terminal`) stays boundable: it's a log tail, and the last lines are -// the ones that matter, which is exactly what the window pins. -const CODE_BODY_TOOLS = ['execute_code', 'read_file'] - -const UNBOUNDABLE_TOOLS = new Set(['clarify', 'image_generate', ...CODE_BODY_TOOLS]) +// Only components that bypass ToolEntry need this opt-out: `clarify` and +// `image_generate` render their own markup, never emit `data-tool-row`, and so +// the CSS escape hatch can never reach them. +const UNBOUNDABLE_TOOLS = new Set(['clarify', 'image_generate']) export function isUnboundableTool(toolName: string): boolean { - return UNBOUNDABLE_TOOLS.has(toolName) || isFileEditTool(toolName) + return UNBOUNDABLE_TOOLS.has(toolName) } export function shouldBoundToolGroup(childCount: number, hasUnboundable: boolean) {