perf(desktop): bound tool-result rendering so big /learn runs don't freeze (#52273)
ToolFallback rebuilt the `part` wrapper every render, defeating the buildToolView memo and re-running a full JSON.stringify of the result on every ~33ms stream delta. A /learn over a large directory (many ~100KB tool results) saturated the renderer main thread (hang/throttle) and spiked memory until it OOMd (crash). - Re-derive a stable `part` from the referentially-stable args/result so the view/copy memos hold across deltas. - Clamp every inline-painted payload (detail, stdout/stderr, rawResult, technical trace) to MAX_TOOL_RENDER_CHARS; the row's Copy button still reads the uncapped view.detail for the full output.
This commit is contained in:
parent
0c3f197cff
commit
cbe5c5689f
3 changed files with 83 additions and 17 deletions
|
|
@ -2,8 +2,10 @@ import { describe, expect, it } from 'vitest'
|
|||
|
||||
import {
|
||||
buildToolView,
|
||||
clampForDisplay,
|
||||
countDiffLineStats,
|
||||
inlineDiffFromResult,
|
||||
MAX_TOOL_RENDER_CHARS,
|
||||
type ToolPart
|
||||
} from './tool-fallback-model'
|
||||
|
||||
|
|
@ -110,6 +112,36 @@ describe('buildToolView file edit diffs', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('clampForDisplay', () => {
|
||||
it('passes short payloads through untouched', () => {
|
||||
expect(clampForDisplay('hello')).toBe('hello')
|
||||
expect(clampForDisplay('x'.repeat(MAX_TOOL_RENDER_CHARS))).toHaveLength(MAX_TOOL_RENDER_CHARS)
|
||||
})
|
||||
|
||||
it('truncates oversized payloads and reports the omitted count', () => {
|
||||
const oversized = 'x'.repeat(MAX_TOOL_RENDER_CHARS + 5_000)
|
||||
const clamped = clampForDisplay(oversized)
|
||||
|
||||
expect(clamped.length).toBeLessThan(oversized.length)
|
||||
expect(clamped.startsWith('x'.repeat(MAX_TOOL_RENDER_CHARS))).toBe(true)
|
||||
expect(clamped).toContain('5,000 more characters truncated')
|
||||
expect(clamped).toContain('Copy')
|
||||
})
|
||||
})
|
||||
|
||||
// A large tool result (e.g. a 100KB read_file during a `/learn` run) must not
|
||||
// be serialized into the rendered rawResult at full size — that JSON.stringify
|
||||
// payload is what floods the renderer when many rows stack up.
|
||||
describe('buildToolView caps serialized result size', () => {
|
||||
it('clamps rawResult for an oversized result', () => {
|
||||
const huge = 'y'.repeat(MAX_TOOL_RENDER_CHARS * 3)
|
||||
const view = buildToolView(part({ result: { content: huge }, toolName: 'read_file' }), '')
|
||||
|
||||
expect(view.rawResult.length).toBeLessThanOrEqual(MAX_TOOL_RENDER_CHARS + 200)
|
||||
expect(view.rawResult).toContain('truncated')
|
||||
})
|
||||
})
|
||||
|
||||
describe('countDiffLineStats', () => {
|
||||
it('counts added and removed lines', () => {
|
||||
expect(
|
||||
|
|
|
|||
|
|
@ -238,8 +238,26 @@ function contextValue(value: unknown): string {
|
|||
return typeof value === 'string' ? value : ''
|
||||
}
|
||||
|
||||
// Each tool result is server-capped (~100KB), but a turn over a big directory
|
||||
// stacks many rows; painting/serializing them all floods the renderer (freeze,
|
||||
// then OOM). Clamp every inline-painted payload to a bounded slice — the row's
|
||||
// Copy button still reads the uncapped `view.detail` for the full output.
|
||||
export const MAX_TOOL_RENDER_CHARS = 20_000
|
||||
|
||||
export function clampForDisplay(value: string, max = MAX_TOOL_RENDER_CHARS): string {
|
||||
if (value.length <= max) {
|
||||
return value
|
||||
}
|
||||
|
||||
const omitted = value.length - max
|
||||
|
||||
return `${value.slice(0, max)}\n\n… ${omitted.toLocaleString()} more characters truncated — use Copy for the full output.`
|
||||
}
|
||||
|
||||
function prettyJson(value: unknown): string {
|
||||
return typeof value === 'string' ? value : JSON.stringify(value, null, 2)
|
||||
const raw = typeof value === 'string' ? value : JSON.stringify(value, null, 2)
|
||||
|
||||
return clampForDisplay(raw ?? '')
|
||||
}
|
||||
|
||||
function parseMaybeObject(value: unknown): Record<string, unknown> {
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import { $toolDisclosureOpen, $toolViewMode, setToolDisclosureOpen } from '@/sto
|
|||
import { PendingToolApproval } from './tool-approval'
|
||||
import {
|
||||
buildToolView,
|
||||
clampForDisplay,
|
||||
cleanVisibleText,
|
||||
countDiffLineStats,
|
||||
inlineDiffFromResult,
|
||||
|
|
@ -104,7 +105,7 @@ function rawTechnicalTrace(args: unknown, result: unknown): string {
|
|||
})
|
||||
.filter(Boolean)
|
||||
|
||||
return parts.join('\n')
|
||||
return clampForDisplay(parts.join('\n'))
|
||||
}
|
||||
|
||||
function statusGlyph(status: ToolStatus, copy: ToolStatusCopy): ReactNode {
|
||||
|
|
@ -220,13 +221,25 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
const messageRunning = useAuiState(selectMessageRunning)
|
||||
const embedded = useContext(ToolEmbedContext)
|
||||
const toolViewMode = useStore($toolViewMode)
|
||||
const disclosureId = `tool-entry:${messageId}:${toolPartDisclosureId(part)}`
|
||||
|
||||
// `ToolFallback` rebuilds the `part` wrapper each render, defeating the memos
|
||||
// below and re-running buildToolView (full JSON.stringify of result) on every
|
||||
// stream delta — the freeze on big `/learn` runs. Re-derive a stable part from
|
||||
// the referentially-stable args/result so the memos hold across deltas.
|
||||
const { args, isError, result, toolCallId, toolName } = part
|
||||
|
||||
const stablePart = useMemo<ToolPart>(
|
||||
() => ({ args, isError, result, toolCallId, toolName, type: 'tool-call' }),
|
||||
[args, isError, result, toolCallId, toolName]
|
||||
)
|
||||
|
||||
const disclosureId = `tool-entry:${messageId}:${toolPartDisclosureId(stablePart)}`
|
||||
const dismissed = useStore($toolRowDismissed(disclosureId))
|
||||
const isPending = messageRunning && part.result === undefined
|
||||
const isPending = messageRunning && result === undefined
|
||||
const liveDiffs = useStore($toolInlineDiffs)
|
||||
const sideDiff = part.toolCallId ? liveDiffs[part.toolCallId] || '' : ''
|
||||
const inlineDiff = stripInlineDiffChrome(sideDiff) || inlineDiffFromResult(part.result)
|
||||
const isFileEdit = isFileEditTool(part.toolName)
|
||||
const sideDiff = toolCallId ? liveDiffs[toolCallId] || '' : ''
|
||||
const inlineDiff = stripInlineDiffChrome(sideDiff) || inlineDiffFromResult(result)
|
||||
const isFileEdit = isFileEditTool(toolName)
|
||||
const defaultOpen = Boolean(inlineDiff)
|
||||
const open = useDisclosureOpen(disclosureId, defaultOpen)
|
||||
const canDismiss = !isPending && !embedded
|
||||
|
|
@ -237,13 +250,14 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
const enterRef = useEnterAnimation(messageRunning && !embedded, `tool-entry:${disclosureId}`)
|
||||
const elapsed = useElapsedSeconds(isPending, `tool:${disclosureId}`)
|
||||
|
||||
// Stale parts (no result, but message stopped running) get a synthetic
|
||||
// empty result so buildToolView treats them as completed-no-output.
|
||||
// Stale parts (no result, but message stopped running) get a synthetic empty
|
||||
// result so buildToolView treats them as completed-no-output. Keyed on
|
||||
// stablePart so it recomputes only when this tool's data changes.
|
||||
const view = useMemo(() => {
|
||||
const p = !isPending && part.result === undefined ? { ...part, result: {} } : part
|
||||
const p = !isPending && result === undefined ? { ...stablePart, result: {} } : stablePart
|
||||
|
||||
return buildToolView(p, inlineDiff)
|
||||
}, [inlineDiff, isPending, part])
|
||||
}, [inlineDiff, isPending, result, stablePart])
|
||||
|
||||
// Surface a previewable artifact (HTML file / localhost URL) as a compact link
|
||||
// in the composer status stack rather than a bulky inline card. Uses the same
|
||||
|
|
@ -313,7 +327,9 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
view.imageUrl || view.inlineDiff || showDetail || hasSearchHits || toolViewMode === 'technical'
|
||||
)
|
||||
|
||||
const copyAction = useMemo(() => toolCopyPayload(part, view), [part, view])
|
||||
// copyAction reads the uncapped view.detail; clampForDisplay below only bounds
|
||||
// what's painted, so the row's Copy button still yields the full output.
|
||||
const copyAction = useMemo(() => toolCopyPayload(stablePart, view), [stablePart, view])
|
||||
|
||||
const diffStats = useMemo(
|
||||
() => (isFileEdit && view.inlineDiff ? countDiffLineStats(view.inlineDiff) : null),
|
||||
|
|
@ -466,7 +482,7 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
detailSections.summary && 'mt-1.5'
|
||||
)}
|
||||
>
|
||||
{detailSections.body}
|
||||
{clampForDisplay(detailSections.body)}
|
||||
</pre>
|
||||
)}
|
||||
</div>
|
||||
|
|
@ -481,7 +497,7 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
<div className="space-y-0.5">
|
||||
{view.stderr && <p className={TOOL_SECTION_LABEL_CLASS}>stdout</p>}
|
||||
<pre className={cn(TOOL_SECTION_PRE_CLASS, 'whitespace-pre-wrap wrap-anywhere')}>
|
||||
{view.rendersAnsi ? <AnsiText text={view.stdout} /> : view.stdout}
|
||||
{view.rendersAnsi ? <AnsiText text={clampForDisplay(view.stdout)} /> : clampForDisplay(view.stdout)}
|
||||
</pre>
|
||||
</div>
|
||||
)}
|
||||
|
|
@ -494,7 +510,7 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
'whitespace-pre-wrap wrap-anywhere text-(--ui-text-tertiary)'
|
||||
)}
|
||||
>
|
||||
{view.rendersAnsi ? <AnsiText text={view.stderr} /> : view.stderr}
|
||||
{view.rendersAnsi ? <AnsiText text={clampForDisplay(view.stderr)} /> : clampForDisplay(view.stderr)}
|
||||
</pre>
|
||||
</div>
|
||||
)}
|
||||
|
|
@ -504,10 +520,10 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
{view.detailLabel && <p className={TOOL_SECTION_LABEL_CLASS}>{view.detailLabel}</p>}
|
||||
{renderDetailAsCode ? (
|
||||
<pre className={cn(TOOL_SECTION_PRE_CLASS, 'whitespace-pre-wrap wrap-anywhere')}>
|
||||
{view.rendersAnsi ? <AnsiText text={view.detail} /> : view.detail}
|
||||
{view.rendersAnsi ? <AnsiText text={clampForDisplay(view.detail)} /> : clampForDisplay(view.detail)}
|
||||
</pre>
|
||||
) : (
|
||||
<CompactMarkdown className={cn(TOOL_SECTION_SURFACE_CLASS, 'wrap-anywhere')} text={view.detail} />
|
||||
<CompactMarkdown className={cn(TOOL_SECTION_SURFACE_CLASS, 'wrap-anywhere')} text={clampForDisplay(view.detail)} />
|
||||
)}
|
||||
</div>
|
||||
))}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue