From 73af634763c87e7195ff74e4cf4474118d4da5ab Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:08:57 +0200 Subject: [PATCH] fix(client): stop counting empty edit sides as changed lines A pure deletion (`new_text: ""`) rendered a blank green row and reported one addition, because `''.split('\n')` yields one element. A trailing newline did the same at the end of either side. Both sides now go through one helper that treats an empty side as no rows and a trailing newline as a terminator. The defect predates the structural builder; it was carried over from the string preview it replaced. --- .../Chat/Messages/Content/Parts/DiffView.tsx | 13 +++++++++++-- .../Content/Parts/__tests__/DiffView.test.ts | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx index ee364de1a3..1d785550a4 100644 --- a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx @@ -92,6 +92,15 @@ export interface TextEditPreview { newText: string; } +/** + * Rows one side of an edit contributes. An empty side means "nothing here", + * not one blank line, and a trailing newline terminates the last line rather + * than starting an empty one: plain `split('\n')` would render a blank row and + * count it as a change, so a pure deletion (`new_text: ""`) claimed one + * addition. + */ +const previewRows = (text: string): string[] => (text ? text.replace(/\n$/, '').split('\n') : []); + /** * Builds the diff for an `edit_file` args preview straight from the structured * edits, instead of formatting them into diff text for `parseUnifiedDiff` to @@ -116,11 +125,11 @@ export function buildEditPreviewDiff(edits: TextEditPreview[]): ParsedDiff { if (index > 0) { lines.push({ type: 'hunk', text: '' }); } - for (const text of edit.oldText.split('\n')) { + for (const text of previewRows(edit.oldText)) { deletions += 1; lines.push({ type: 'del', text }); } - for (const text of edit.newText.split('\n')) { + for (const text of previewRows(edit.newText)) { additions += 1; lines.push({ type: 'add', text }); } 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 9aa3f6980d..c8c031dcd6 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 @@ -69,6 +69,24 @@ describe('parseUnifiedDiff', () => { expect(parsed.lines.map((line) => line.text)).toEqual(['a', 'b', 'c']); }); + it('does not count an empty replacement as an added line', () => { + /** A pure deletion (`new_text: ""`) must not render a blank green row or + * report an addition; `''.split('\n')` yields one element. */ + const parsed = buildEditPreviewDiff([{ oldText: 'gone', newText: '' }]); + + expect(parsed.deletions).toBe(1); + expect(parsed.additions).toBe(0); + expect(parsed.lines).toEqual([{ type: 'del', text: 'gone' }]); + }); + + it('treats a trailing newline as a terminator, not an empty final line', () => { + const parsed = buildEditPreviewDiff([{ oldText: 'a\n', newText: 'b\n' }]); + + expect(parsed.deletions).toBe(1); + expect(parsed.additions).toBe(1); + expect(parsed.lines.map((line) => line.text)).toEqual(['a', 'b']); + }); + 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 text the removed separators used.