diff --git a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx index 4065549b01..ee364de1a3 100644 --- a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx @@ -2,12 +2,6 @@ import { cn } from '~/utils'; const HUNK_HEADER = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/; -/** The separator pair `formatEditPreview` writes ahead of each batched edit, - * numbered once a batch holds more than one. Only ever skipped as an adjacent - * pair sitting immediately before a hunk header; see `parseUnifiedDiff`. */ -const OLD_TEXT_MARKER = /^--- old_text(?: \d+)?$/; -const NEW_TEXT_MARKER = /^\+\+\+ new_text(?: \d+)?$/; - export interface DiffLine { type: 'add' | 'del' | 'context' | 'hunk'; text: string; @@ -23,11 +17,18 @@ export interface ParsedDiff { } /** - * Parses unified-diff text into typed lines with old/new line numbers when the - * hunk headers carry them. Also understands the argless `@@` separators and - * `--- old_text` / `+++ new_text` markers the streaming args preview emits. - * file headers and `\ No newline` markers are dropped (the window header - * already names the file). + * Parses REAL unified-diff text (a tool's own output) into typed lines with + * old/new line numbers when the hunk headers carry them. File headers and + * `\ No newline` markers are dropped, since the window header already names + * the file. + * + * Deliberately knows nothing about the args preview. That preview is built + * from structured edits by `buildEditPreviewDiff`, so it never round-trips + * through diff text: there are no synthetic separators to recognize, and + * therefore no byte sequence a real changed line could be mistaken for. An + * earlier version emitted `--- old_text` / `+++ new_text` separators and tried + * to spot them here, which could not be made safe, because replacing a source + * line `-- old_text` with `++ new_text` produces exactly those strings. */ export function parseUnifiedDiff(diff: string): ParsedDiff { const lines: DiffLine[] = []; @@ -38,25 +39,7 @@ export function parseUnifiedDiff(diff: string): ParsedDiff { let newLine: number | undefined; let inHunk = false; - const rawLines = diff.replace(/\n$/, '').split('\n'); - for (let index = 0; index < rawLines.length; index += 1) { - const raw = rawLines[index]; - /** `formatEditPreview` writes an `--- old_text N` / `+++ new_text N` pair - * ahead of EACH batched edit's `@@`, and `inHunk` never resets, so a - * `!inHunk` gate recognized only the first pair and counted every later - * one as a deletion plus an addition. Matching the marker names alone is - * not enough either: deleting a real `-- old_text` comment line (SQL, - * Lua) is prefixed to exactly `--- old_text`. So require separator - * POSITION: the pair adjacent and immediately followed by a hunk header, - * which no run of genuine content satisfies. */ - if ( - OLD_TEXT_MARKER.test(raw) && - NEW_TEXT_MARKER.test(rawLines[index + 1] ?? '') && - (rawLines[index + 2] ?? '').startsWith('@@') - ) { - index += 1; - continue; - } + for (const raw of diff.replace(/\n$/, '').split('\n')) { if (!inHunk && (raw.startsWith('--- ') || raw.startsWith('+++ '))) { continue; } @@ -104,6 +87,56 @@ export function parseUnifiedDiff(diff: string): ParsedDiff { return { lines, additions, deletions, hasLineNumbers }; } +export interface TextEditPreview { + oldText: string; + newText: string; +} + +/** + * Builds the diff for an `edit_file` args preview straight from the structured + * edits, instead of formatting them into diff text for `parseUnifiedDiff` to + * read back. + * + * That round trip is what made batched previews ambiguous: the separators it + * needed between edits were byte-identical to a real changed line, since + * deleting a source line `-- old_text` prefixes to exactly `--- old_text`. No + * parser can tell those apart from position or bytes alone. Built here, each + * edit boundary is structural and no separator is ever emitted. + * + * A boundary between edits becomes an empty `hunk` line, which `DiffView` + * renders as its divider rule. There are no line numbers: args previews carry + * the replacement text, never its position in the file. + */ +export function buildEditPreviewDiff(edits: TextEditPreview[]): ParsedDiff { + const lines: DiffLine[] = []; + let additions = 0; + let deletions = 0; + + edits.forEach((edit, index) => { + if (index > 0) { + lines.push({ type: 'hunk', text: '' }); + } + for (const text of edit.oldText.split('\n')) { + deletions += 1; + lines.push({ type: 'del', text }); + } + for (const text of edit.newText.split('\n')) { + additions += 1; + lines.push({ type: 'add', text }); + } + }); + + return { lines, additions, deletions, hasLineNumbers: false }; +} + +/** One-way serialization for the copy button. Never parsed back, so it is free + * to be lossy about hunk boundaries. */ +export function diffPreviewText(parsed: ParsedDiff): string { + return parsed.lines + .map((line) => (line.type === 'hunk' ? '' : `${LINE_MARKERS[line.type]}${line.text}`)) + .join('\n'); +} + const LINE_MARKERS: Record = { add: '+', del: '-', diff --git a/client/src/components/Chat/Messages/Content/Parts/FileAuthoringCall.tsx b/client/src/components/Chat/Messages/Content/Parts/FileAuthoringCall.tsx index a37c467846..e821dad9c1 100644 --- a/client/src/components/Chat/Messages/Content/Parts/FileAuthoringCall.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/FileAuthoringCall.tsx @@ -1,10 +1,15 @@ import { useMemo } from 'react'; import { FilePenLine, FilePlus2 } from 'lucide-react'; import type { TAttachment, PartMetadata } from 'librechat-data-provider'; +import DiffView, { + parseUnifiedDiff, + buildEditPreviewDiff, + diffPreviewText, + type TextEditPreview, +} from './DiffView'; import parseJsonField, { parseJsonFieldOccurrences } from './parseJsonField'; import ProgressText from '~/components/Chat/Messages/Content/ProgressText'; import { toolPanelSpacingClassName } from '../disclosure'; -import DiffView, { parseUnifiedDiff } from './DiffView'; import useToolCallState from './useToolCallState'; import useLazyHighlight from './useLazyHighlight'; import CodeWindowHeader from './CodeWindowHeader'; @@ -20,11 +25,6 @@ type FileAuthoringToolName = 'create_file' | 'edit_file'; type ToolCallArgs = string | Record | undefined; -interface TextEditPreview { - oldText: string; - newText: string; -} - function hasDiff(output: string): boolean { return /\n@@\s/.test(output) || output.includes('\n--- ') || output.includes('\n+++ '); } @@ -51,32 +51,13 @@ function textValue(value: unknown): string { return typeof value === 'string' ? value : ''; } -function editPreviewLines(prefix: '-' | '+', text: string): string { - return text - .split('\n') - .map((line) => `${prefix}${line}`) - .join('\n'); -} - -function formatEditPreview(edits: TextEditPreview[]): string { - return edits - .map((edit, index) => { - const suffix = edits.length > 1 ? ` ${index + 1}` : ''; - return [ - `--- old_text${suffix}`, - `+++ new_text${suffix}`, - '@@', - editPreviewLines('-', edit.oldText), - editPreviewLines('+', edit.newText), - ].join('\n'); - }) - .join('\n\n'); -} - -function buildEditArgsPreview(args: ToolCallArgs): string { +/** Structured edits for the args preview. Never formatted into diff text: + * `buildEditPreviewDiff` renders them directly, so no separator has to be + * invented and later told apart from real content. */ +function buildEditArgsPreview(args: ToolCallArgs): TextEditPreview[] { const parsed = parseArgsObject(args); if (Array.isArray(parsed?.edits) && parsed.edits.length > 0) { - const edits = parsed.edits + return parsed.edits .map((edit): TextEditPreview | undefined => { if (typeof edit !== 'object' || edit === null || Array.isArray(edit)) { return undefined; @@ -87,24 +68,22 @@ function buildEditArgsPreview(args: ToolCallArgs): string { return oldText || newText ? { oldText, newText } : undefined; }) .filter((edit): edit is TextEditPreview => !!edit); - return formatEditPreview(edits); } if (parsed) { const oldText = textValue(parsed.old_text); const newText = textValue(parsed.new_text); - return oldText || newText ? formatEditPreview([{ oldText, newText }]) : ''; + return oldText || newText ? [{ oldText, newText }] : []; } /** Partial JSON during streaming: pair up field occurrences in document order, covering both single-replacement and batched `edits` args */ const oldTexts = parseJsonFieldOccurrences(args, 'old_text'); const newTexts = parseJsonFieldOccurrences(args, 'new_text'); const editCount = Math.max(oldTexts.length, newTexts.length); - const edits = Array.from({ length: editCount }, (_, index) => ({ + return Array.from({ length: editCount }, (_, index) => ({ oldText: oldTexts[index] ?? '', newText: newTexts[index] ?? '', })).filter((edit) => edit.oldText || edit.newText); - return formatEditPreview(edits); } export default function FileAuthoringCall({ @@ -142,23 +121,35 @@ export default function FileAuthoringCall({ ); const intent = useToolCallIntent(args); const authoredContent = useMemo(() => parseJsonField(args, 'content'), [args]); - const editArgsPreview = useMemo(() => buildEditArgsPreview(args), [args]); + const editArgs = useMemo(() => buildEditArgsPreview(args), [args]); const fileName = filePath.split('/').pop() || filePath || localize('com_ui_file').toLowerCase(); const fileLang = useMemo(() => langFromPath(filePath), [filePath]); - const argsPreview = isCreate ? authoredContent : editArgsPreview; const outputIsDiff = hasDiff(output); - /** A diff in the output supersedes the args preview — it carries the input with real file context */ - const preview = outputIsDiff ? output : argsPreview || output; + /** A diff in the output supersedes the args preview: it carries the input + * with real file context. Only the output is ever PARSED; the args preview + * is built from structured edits, so it never round-trips through diff text + * where a separator could be confused with a real changed line. */ + const parsedDiff = useMemo(() => { + if (outputIsDiff) { + return parseUnifiedDiff(output); + } + return !isCreate && editArgs.length > 0 ? buildEditPreviewDiff(editArgs) : null; + }, [outputIsDiff, output, isCreate, editArgs]); + /** Plain text for the non-diff pane, and the copy payload either way. */ + const preview = useMemo(() => { + if (isCreate) { + return authoredContent || output; + } + if (outputIsDiff) { + return output; + } + return parsedDiff ? diffPreviewText(parsedDiff) : output; + }, [isCreate, authoredContent, outputIsDiff, output, parsedDiff]); const showOutputSection = !!output && preview !== output; - const previewIsDiff = outputIsDiff || (!isCreate && !!editArgsPreview && preview !== output); let previewLang = 'plaintext'; if (isCreate && authoredContent && preview === authoredContent) { previewLang = fileLang; } - const parsedDiff = useMemo( - () => (previewIsDiff && preview ? parseUnifiedDiff(preview) : null), - [previewIsDiff, preview], - ); const { showCode, toggleCode, expandStyle, expandRef, phase } = useToolCallState({ initialProgress, diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts b/client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts index 38d305a84b..9aa3f6980d 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts @@ -1,4 +1,4 @@ -import { parseUnifiedDiff } from '../DiffView'; +import { parseUnifiedDiff, buildEditPreviewDiff } from '../DiffView'; describe('parseUnifiedDiff', () => { it('drops file headers but keeps changed lines that begin with header markers', () => { @@ -40,36 +40,40 @@ describe('parseUnifiedDiff', () => { expect(parsed.deletions).toBe(1); }); - it('skips the separator pair before every batched edit', () => { - const parsed = parseUnifiedDiff( - [ - '--- old_text 1', - '+++ new_text 1', - '@@', - '-first old', - '+first new', - '', - '--- old_text 2', - '+++ new_text 2', - '@@', - '-second old', - '+second new', - ].join('\n'), - ); + it('counts a batched edit preview from its structured edits', () => { + /** Built directly rather than formatted into diff text and parsed back: + * the separators that round trip needed were byte-identical to a real + * changed line, so no parser could tell them apart. */ + const parsed = buildEditPreviewDiff([ + { oldText: 'first old', newText: 'first new' }, + { oldText: 'second old', newText: 'second new' }, + ]); - /** The second edit's synthetic pair used to be counted as a real deletion - * and addition, inflating both statistics and rendering marker rows. */ expect(parsed.deletions).toBe(2); expect(parsed.additions).toBe(2); - expect(parsed.lines.some((line) => line.text.includes('old_text'))).toBe(false); - expect(parsed.lines.some((line) => line.text.includes('new_text'))).toBe(false); + expect(parsed.hasLineNumbers).toBe(false); + expect(parsed.lines).toEqual([ + { type: 'del', text: 'first old' }, + { type: 'add', text: 'first new' }, + { type: 'hunk', text: '' }, + { type: 'del', text: 'second old' }, + { type: 'add', text: 'second new' }, + ]); }); - it('keeps changed lines that collide with the synthetic header names', () => { + it('keeps multi-line replacement text as separate rows', () => { + const parsed = buildEditPreviewDiff([{ oldText: 'a\nb', newText: 'c' }]); + + expect(parsed.deletions).toBe(2); + expect(parsed.additions).toBe(1); + expect(parsed.lines.map((line) => line.text)).toEqual(['a', 'b', 'c']); + }); + + it('keeps changed lines that look like the old synthetic header names', () => { /** Deleting a real `-- old_text` comment (SQL, Lua) and adding - * `++ new_text` prefixes to exactly the synthetic marker text. Only the - * separator POSITION, an adjacent pair before a hunk header, separates - * the two, so this pair inside a hunk must survive as content. */ + * `++ new_text` prefixes to exactly the text the removed separators used. + * Nothing emits those separators any more, so these are unambiguously + * content and must survive. */ const parsed = parseUnifiedDiff(['@@ -1,2 +1,2 @@', '--- old_text', '+++ new_text'].join('\n')); expect(parsed.deletions).toBe(1); diff --git a/client/src/hooks/SSE/useAttachmentHandler.ts b/client/src/hooks/SSE/useAttachmentHandler.ts index 4b735ec325..896ae5e87b 100644 --- a/client/src/hooks/SSE/useAttachmentHandler.ts +++ b/client/src/hooks/SSE/useAttachmentHandler.ts @@ -10,9 +10,12 @@ import type { QueryClient } from '@tanstack/react-query'; import { handleMemoryArtifact } from '~/utils/memory'; import store from '~/store'; -/** Identity of a re-emitted web-search snapshot: its tool call plus the turn - * inside that call. Scoped to `web_search` because that is the flow whose - * server helper rebuilds and rewrites the whole payload on each highlight. */ +/** Identity of a re-emitted web-search snapshot: its tool call, the turn + * inside that call, and the agent that ran it. Scoped to `web_search` because + * that is the flow whose server helper rebuilds and rewrites the whole + * payload on each highlight. The agent qualifies the key for the same reason + * the rendering path filters by it: handoff agents repeat provider tool-call + * ids (`call_0`), so without it one agent's results would replace another's. */ const searchSnapshotKeyOf = (attachment: TAttachment): string | undefined => { if (attachment.type !== Tools.web_search) { return undefined; @@ -21,8 +24,9 @@ const searchSnapshotKeyOf = (attachment: TAttachment): string | undefined => { if (!toolCallId) { return undefined; } + const agentId = (attachment as { agentId?: string }).agentId ?? ''; const turn = attachment[Tools.web_search]?.turn; - return `${Tools.web_search}:${toolCallId}:${turn ?? 0}`; + return `${Tools.web_search}:${agentId}:${toolCallId}:${turn ?? 0}`; }; export default function useAttachmentHandler(queryClient?: QueryClient) {