From 578b1c2f86f5a13859e30697d2ba23c320f30805 Mon Sep 17 00:00:00 2001 From: Andrea V <1577639+karimodm@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:14:13 +0200 Subject: [PATCH] fix(chat): bound oversized tool diff previews (#3293) Closes #3292. Keep #3286 open for the separate context-panel DiffView path. Thanks for fixing oversized tool-card previews. --- .../chat/message/parts/DOCUMENTATION.md | 2 +- .../chat/message/parts/PlainDiffFallback.tsx | 28 +++++---- .../chat/message/parts/ToolPart.tsx | 15 +++-- .../chat/message/parts/toolDiffPreview.ts | 29 +++++++++ .../chat/message/parts/toolDiffUtils.test.ts | 61 +++++++++++++++++++ .../chat/message/parts/toolDiffUtils.ts | 11 ++++ 6 files changed, 129 insertions(+), 17 deletions(-) create mode 100644 packages/ui/src/components/chat/message/parts/toolDiffPreview.ts diff --git a/packages/ui/src/components/chat/message/parts/DOCUMENTATION.md b/packages/ui/src/components/chat/message/parts/DOCUMENTATION.md index 2f48b596..2efb7c7a 100644 --- a/packages/ui/src/components/chat/message/parts/DOCUMENTATION.md +++ b/packages/ui/src/components/chat/message/parts/DOCUMENTATION.md @@ -86,7 +86,7 @@ Use this doc when you ask an agent to change tool/header/description behavior. - Every other tool, including search/fetch, OpenCode built-ins, custom tools, plugins, and MCP tools, is **expandable** and renders through `ToolPart`. - The managed `openchamber` plugin tool uses the expandable path and hides its broad protocol input. The plugin supplies the selected action's human description as the native tool title; the UI renders that metadata without owning an action map. The full versioned result envelope renders through the same neutral JSON summary/tree/raw views as other tools, without a tool-specific output card. - `ToolPart` defers expanded content after a user toggle, preventing large tool input/output payloads from mounting during the initial chat render. -- The rich tool diff preview lives in `ToolPartDiffPreview.tsx` and is lazy-loaded from `ToolPart`. It is the only tool-card piece that imports the `@pierre/diffs` + Shiki rendering stack, keeping that stack out of the eager chat startup graph. While its chunk loads (first rendered diff only) the plain-text patch from `PlainDiffFallback.tsx` renders as the Suspense fallback, mirroring the preview's error fallback. `ToolPart` itself must not statically import `@pierre/diffs` runtime modules or `@/lib/shiki/appThemeRegistry`. +- The rich tool diff preview lives in `ToolPartDiffPreview.tsx` and is lazy-loaded from `ToolPart`. It is the only tool-card piece that imports the `@pierre/diffs` + Shiki rendering stack, keeping that stack out of the eager chat startup graph. While its chunk loads (first rendered diff only) the plain-text patch from `PlainDiffFallback.tsx` renders as the Suspense fallback, mirroring the preview's error fallback. Patches over 256 KiB or 2,000 lines skip rich parsing and use a bounded plain-text preview; navigation keeps the original patch. `ToolPart` itself must not statically import `@pierre/diffs` runtime modules or `@/lib/shiki/appThemeRegistry`. - The `@pierre/diffs` stack is knowingly unprotected against the JS/TS `template-call` backtracking that OOM'd the renderer in openchamber/openchamber#2587. Our own markdown Shiki worker sanitizes every grammar it loads (`@/lib/shiki/sanitizeTemplateCallGrammar`), but the diff worker pool runs `preferredHighlighter: 'shiki-wasm'` (`DiffWorkerProvider.tsx`) and resolves its languages by id through `@pierre/diffs`' own registry — `langs` accepts `SupportedLanguages` strings only, so there is no seam to hand it a pre-sanitized `LanguageRegistration`. A pathological template literal inside a rendered diff can therefore still hang that pool's Oniguruma engine. The available levers are upstream (a `langs` overload accepting grammar objects) or switching that pool to the JS regex engine; neither is done. - Running bash output falls back to `state.metadata.output` until canonical `state.output` arrives. Its output viewport grows with the content up to `46vh`, then scrolls and follows new output until the user scrolls up; following resumes when the user returns to the bottom. Live output appends or replaces rewritten snapshots as plain text without worker highlighting; finalized output normalizes ANSI terminal controls with a bounded synthetic-cell budget, bypasses the throttle, and receives the normal one-time highlighted rendering. - Thinking/Justification duration is hidden in `sorted` mode (handled in `ReasoningPart.tsx` + `JustificationBlock.tsx`). diff --git a/packages/ui/src/components/chat/message/parts/PlainDiffFallback.tsx b/packages/ui/src/components/chat/message/parts/PlainDiffFallback.tsx index 6e6e91e8..ae78cac2 100644 --- a/packages/ui/src/components/chat/message/parts/PlainDiffFallback.tsx +++ b/packages/ui/src/components/chat/message/parts/PlainDiffFallback.tsx @@ -1,19 +1,25 @@ import React from 'react'; +import { getToolDiffPreviewText } from './toolDiffPreview'; + /** * Plain-text patch rendering used when the rich `@pierre/diffs` preview is * unavailable: non-diff render modes, preview errors, and while the lazily * loaded diff preview chunk is still downloading. Lives in its own module so * `ToolPart` can render it without importing the @pierre/diffs stack. */ -export const PlainDiffFallback: React.FC<{ diff: string }> = ({ diff }) => ( - 
-        {diff}
-
-); +export const PlainDiffFallback: React.FC<{ diff: string }> = ({ diff }) => { + const preview = React.useMemo(() => getToolDiffPreviewText(diff), [diff]); + + return ( + 
+            {preview}
+
+ ); +}; diff --git a/packages/ui/src/components/chat/message/parts/ToolPart.tsx b/packages/ui/src/components/chat/message/parts/ToolPart.tsx index 29425e2e..64dad914 100644 --- a/packages/ui/src/components/chat/message/parts/ToolPart.tsx +++ b/packages/ui/src/components/chat/message/parts/ToolPart.tsx @@ -22,6 +22,7 @@ import { FileTypeIcon } from '@/components/icons/FileTypeIcon'; import { copyTextToClipboard } from '@/lib/clipboard'; import type { ToolPopupContent } from '../types'; import { PlainDiffFallback } from './PlainDiffFallback'; +import { isToolDiffPreviewOversized } from './toolDiffPreview'; import { lazyWithChunkRecovery } from '@/lib/chunkLoadRecovery'; import { @@ -1200,11 +1201,15 @@ const renderAnimatedPathWithIcon = (path: string, animate = true, grow = true, s // Suspense fallback, mirroring the preview's own error fallback. const LazyToolPartDiffPreview = lazyWithChunkRecovery(() => import('./ToolPartDiffPreview')); -const DiffPreview: React.FC<{ diff: string; diffViewMode: DiffViewMode }> = ({ diff, diffViewMode }) => ( - }> - - -); +const DiffPreview: React.FC<{ diff: string; diffViewMode: DiffViewMode }> = ({ diff, diffViewMode }) => { + if (isToolDiffPreviewOversized(diff)) return ; + + return ( + }> + + + ); +}; interface ToolExpandedContentProps { part: ToolPartType; diff --git a/packages/ui/src/components/chat/message/parts/toolDiffPreview.ts b/packages/ui/src/components/chat/message/parts/toolDiffPreview.ts new file mode 100644 index 00000000..1267e1f1 --- /dev/null +++ b/packages/ui/src/components/chat/message/parts/toolDiffPreview.ts @@ -0,0 +1,29 @@ +export const TOOL_DIFF_PREVIEW_MAX_CHARS = 256 * 1024; +export const TOOL_DIFF_PREVIEW_MAX_LINES = 2_000; + +const getPreviewEnd = (diff: string): number | null => { + const scanEnd = Math.min(diff.length, TOOL_DIFF_PREVIEW_MAX_CHARS); + let lineCount = 1; + + for (let index = 0; index < scanEnd; index += 1) { + const character = diff.charCodeAt(index); + if (character !== 10 && character !== 13) continue; + + const separatorLength = character === 13 && diff.charCodeAt(index + 1) === 10 ? 2 : 1; + if (index + separatorLength < diff.length) { + lineCount += 1; + if (lineCount > TOOL_DIFF_PREVIEW_MAX_LINES) return index; + } + if (separatorLength === 2) index += 1; + } + + return scanEnd < diff.length ? scanEnd : null; +}; + +export const isToolDiffPreviewOversized = (diff: string): boolean => getPreviewEnd(diff) !== null; + +export const getToolDiffPreviewText = (diff: string): string => { + const previewEnd = getPreviewEnd(diff); + if (previewEnd === null) return diff; + return `${diff.slice(0, previewEnd).trimEnd()}\n…`; +}; diff --git a/packages/ui/src/components/chat/message/parts/toolDiffUtils.test.ts b/packages/ui/src/components/chat/message/parts/toolDiffUtils.test.ts index e0352fed..13fae957 100644 --- a/packages/ui/src/components/chat/message/parts/toolDiffUtils.test.ts +++ b/packages/ui/src/components/chat/message/parts/toolDiffUtils.test.ts @@ -10,6 +10,12 @@ import { getRenderablePatchInfo, resolveToolQuickOpenTarget, } from './toolDiffUtils'; +import { + getToolDiffPreviewText, + isToolDiffPreviewOversized, + TOOL_DIFF_PREVIEW_MAX_CHARS, + TOOL_DIFF_PREVIEW_MAX_LINES, +} from './toolDiffPreview'; const identity = (path: string) => path; @@ -182,6 +188,61 @@ describe('toolDiffUtils', () => { expect(entries[0]?.renderMode).toBe('text'); expect(entries[0]?.patch).toContain('@@'); }); + + test('keeps oversized metadata patches out of the rich diff renderer', () => { + const patch = [ + '--- a/src/generated.ts', + '+++ b/src/generated.ts', + `@@ -1,${TOOL_DIFF_PREVIEW_MAX_LINES + 1} +0,0 @@`, + ...Array.from( + { length: TOOL_DIFF_PREVIEW_MAX_LINES + 1 }, + (_, index) => `-${String(index).padStart(40, '0')}`, + ), + ].join('\n'); + const metadata = { + files: [{ relativePath: 'src/generated.ts', patch }], + }; + const entries = getDiffPatchEntries(metadata, undefined, identity); + + expect(entries).toHaveLength(1); + expect(entries[0]?.renderMode).toBe('text'); + expect(entries[0]?.patch).toBe(patch); + expect(resolveToolQuickOpenTarget('apply_patch', undefined, metadata)?.patch).toBe(patch); + + const preview = getToolDiffPreviewText(entries[0]?.patch ?? ''); + expect(preview.endsWith('\n…')).toBe(true); + expect(preview.split('\n')).toHaveLength(TOOL_DIFF_PREVIEW_MAX_LINES + 1); + expect(preview.length).toBeLessThan(patch.length); + }); + + test('bounds oversized single-line diff previews by character count', () => { + const patch = `+${'x'.repeat(TOOL_DIFF_PREVIEW_MAX_CHARS + 1_000)}`; + const preview = getToolDiffPreviewText(patch); + + expect(preview.endsWith('\n…')).toBe(true); + expect(preview.length).toBe(TOOL_DIFF_PREVIEW_MAX_CHARS + 2); + }); + + test('preserves diff previews at the character and line limits', () => { + const characterLimit = 'x'.repeat(TOOL_DIFF_PREVIEW_MAX_CHARS); + const lineLimit = Array.from({ length: TOOL_DIFF_PREVIEW_MAX_LINES }, () => '+line').join('\n'); + const terminatedLineLimit = `${lineLimit}\n`; + + expect(getToolDiffPreviewText(characterLimit)).toBe(characterLimit); + expect(getToolDiffPreviewText(lineLimit)).toBe(lineLimit); + expect(getToolDiffPreviewText(terminatedLineLimit)).toBe(terminatedLineLimit); + }); + + test('counts bare carriage returns as line separators', () => { + const patch = Array.from( + { length: TOOL_DIFF_PREVIEW_MAX_LINES + 1 }, + (_, index) => `+${index}`, + ).join('\r'); + + expect(isToolDiffPreviewOversized(patch)).toBe(true); + expect(getToolDiffPreviewText(patch).split('\r')).toHaveLength(TOOL_DIFF_PREVIEW_MAX_LINES); + }); + test('resolves the quick-open target from the same entry the expanded card renders', () => { const patch = [ '--- a/src/file.ts', diff --git a/packages/ui/src/components/chat/message/parts/toolDiffUtils.ts b/packages/ui/src/components/chat/message/parts/toolDiffUtils.ts index cfe1796e..6ecb7778 100644 --- a/packages/ui/src/components/chat/message/parts/toolDiffUtils.ts +++ b/packages/ui/src/components/chat/message/parts/toolDiffUtils.ts @@ -1,5 +1,7 @@ import { parsePatchFiles } from '@pierre/diffs'; +import { isToolDiffPreviewOversized } from './toolDiffPreview'; + export type DiffPatchEntry = { id: string; title: string; @@ -431,6 +433,15 @@ const getPatchEntriesFromText = ( idPrefix: string, resolveTitle: (path: string) => string, ): DiffPatchEntry[] => { + if (isToolDiffPreviewOversized(patch)) { + return [{ + id: `${idPrefix}-0`, + title: resolveTitle(fallbackTitle), + patch, + renderMode: 'text', + }]; + } + const normalized = normalizeLooseUnifiedPatch(patch); if (!normalized) { return [];