CERBERUS/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2026-10-24 03:09:26 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

refactor(client): build edit previews structurally instead of round-tripping diff text

Browse source 
The args preview was formatted into unified-diff text purely so parseUnifiedDiff
could read it back, and the separators that round trip needed between batched
edits are byte-identical to a real changed line: deleting a source line
`-- old_text` prefixes to exactly `--- old_text`. Neither the marker names nor
their position could tell the two apart, so the parser either inflated the
statistics or dropped a genuine edit.

buildEditPreviewDiff now constructs the preview's lines from the structured
edits, so edit boundaries are structural and no separator is ever emitted.
parseUnifiedDiff is back to parsing only real tool output, where `--- ` and
`+++ ` are genuine file headers, and it no longer special-cases synthetic
markers at all. Copy text comes from a one-way serializer.

Also include agent identity in the web-search snapshot key: handoff agents
repeat provider tool-call ids, so without it one agent's search results
replaced another's.
This commit is contained in:
Marco Beretta 2026-08-28 23:58:58 +02:00
parent 8aab624308
commit c4e815c6b4
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
4 changed files with 134 additions and 102 deletions
Download patch file Download diff file Expand all files Collapse all files

93
client/src/components/Chat/Messages/Content/Parts/DiffView.tsx
View file

@ -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<DiffLine['type'], string> = {
add: '+',
del: '-',

77
client/src/components/Chat/Messages/Content/Parts/FileAuthoringCall.tsx
View file

@ -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<string, unknown> | 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,

54
client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts
View file

@ -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);

12
client/src/hooks/SSE/useAttachmentHandler.ts
View file

@ -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) {