From 88af3a4723a79564c5f405f01ba7d435d5024f20 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Mon, 2 Feb 2026 11:39:42 +0800 Subject: [PATCH] fix(core): Preserve trailing whitespace in newString during edits Remove unconditional trimming of trailing whitespace from newString in normalizeEditStrings(). This fixes cases where intentional trailing whitespace (e.g., in multi-line strings, heredocs) was being stripped. The oldString fuzzy matching still works correctly, and newString is now preserved exactly as the LLM intended. Fixes #1618 Co-authored-by: Qwen-Coder --- packages/core/src/utils/editHelper.test.ts | 109 ++++++++++++++++----- packages/core/src/utils/editHelper.ts | 43 +++----- 2 files changed, 94 insertions(+), 58 deletions(-) diff --git a/packages/core/src/utils/editHelper.test.ts b/packages/core/src/utils/editHelper.test.ts index 79fe78f6e80..467a426ff41 100644 --- a/packages/core/src/utils/editHelper.test.ts +++ b/packages/core/src/utils/editHelper.test.ts @@ -16,11 +16,11 @@ describe('normalizeEditStrings', () => { const two = 2; `; - it('returns literal matches unchanged and trims new_string trailing whitespace', () => { + it('returns literal matches unchanged', () => { const result = normalizeEditStrings( file, 'const two = 2;', - ' const two = 42; ', + ' const two = 42;', ); expect(result).toEqual({ oldString: 'const two = 2;', @@ -32,11 +32,11 @@ const two = 2; const result = normalizeEditStrings( "const greeting = 'Don't';\n", 'const greeting = ‘Don’t’;', - 'const greeting = “Hello”; ', + 'const greeting = "Hello";', ); expect(result).toEqual({ oldString: "const greeting = 'Don't';", - newString: 'const greeting = “Hello”;', + newString: 'const greeting = "Hello";', }); }); @@ -48,15 +48,7 @@ const two = 2; }); }); - it('still trims new_string when editing a brand-new file', () => { - const result = normalizeEditStrings(null, '', 'new file contents '); - expect(result).toEqual({ - oldString: '', - newString: 'new file contents', - }); - }); - - it('matches unicode dash variants', () => { + it('matches unicode dash variants and preserves newString', () => { const result = normalizeEditStrings( 'const range = "1-2";\n', 'const range = "1\u20132";', @@ -64,19 +56,7 @@ const two = 2; ); expect(result).toEqual({ oldString: 'const range = "1-2";', - newString: 'const range = "3\u20135";', - }); - }); - - it('matches when trailing whitespace differs only at line ends', () => { - const result = normalizeEditStrings( - 'value = 1;\n', - 'value = 1; \n', - 'value = 2; \n', - ); - expect(result).toEqual({ - oldString: 'value = 1;\n', - newString: 'value = 2;\n', + newString: 'const range = "3\u20135"; ', }); }); @@ -103,6 +83,83 @@ const two = 2; newString: 'console.log("bye")', }); }); + + // Tests for issue #1618: Preserve trailing whitespace in newString + describe('trailing whitespace preservation in newString', () => { + it('preserves trailing whitespace when intentionally adding to end of line', () => { + // Test with tab + const result1 = normalizeEditStrings( + 'value = 1;\n', + 'value = 1;\n', + 'value = 1;\t\n', + ); + expect(result1.newString).toBe('value = 1;\t\n'); + + // Test with spaces (same behavior, just different whitespace char) + const result2 = normalizeEditStrings('text\n', 'text\n', 'text \n'); + expect(result2.newString).toBe('text \n'); + }); + + it('preserves newString trailing whitespace even when oldString is fuzzy matched', () => { + const result = normalizeEditStrings( + 'value = 1;\n', // File has no trailing spaces + 'value = 1; \n', // LLM copied with extra spaces (will be fuzzy matched) + 'value = 2; \n', // LLM replacement also has spaces + ); + expect(result).toEqual({ + oldString: 'value = 1;\n', // Canonical from file + newString: 'value = 2; \n', // Preserved as LLM intended + }); + }); + + it('preserves trailing whitespace in multi-line template literals', () => { + const file = 'const s = "";\n'; + const result = normalizeEditStrings( + file, + 'const s = "";', + 'const s = `line1 \nline2`;', // Trailing spaces after line1 are significant + ); + expect(result.newString).toBe('const s = `line1 \nline2`;'); + }); + + it('preserves trailing whitespace when creating new file', () => { + const result = normalizeEditStrings( + null, + '', + 'content with trailing tab\t\n', + ); + expect(result).toEqual({ + oldString: '', + newString: 'content with trailing tab\t\n', + }); + }); + + it('still supports fuzzy matching after trailing whitespace was added in previous edit', () => { + // Round 1: Add trailing spaces to a line + let fileContent = 'value = 1;\n'; + const round1 = normalizeEditStrings( + fileContent, + 'value = 1;\n', + 'value = 1; \n', // Adding trailing spaces + ); + expect(round1.newString).toBe('value = 1; \n'); + + // Simulate the edit being applied + fileContent = fileContent.replace(round1.oldString, round1.newString); + expect(fileContent).toBe('value = 1; \n'); // File now has trailing spaces + + // Round 2: LLM tries to edit again, but its oldString doesn't have trailing spaces + // (because LLM context may not preserve exact whitespace) + const round2 = normalizeEditStrings( + fileContent, + 'value = 1;\n', // LLM thinks there's no trailing spaces + 'value = 2;\n', + ); + // Fuzzy matching should still find the line and return canonical slice WITH trailing spaces + expect(round2.oldString).toBe('value = 1; \n'); + expect(round2.newString).toBe('value = 2;\n'); + }); + }); }); describe('countOccurrences', () => { diff --git a/packages/core/src/utils/editHelper.ts b/packages/core/src/utils/editHelper.ts index 6b4a388db63..797655a2f2a 100644 --- a/packages/core/src/utils/editHelper.ts +++ b/packages/core/src/utils/editHelper.ts @@ -64,30 +64,6 @@ function normalizeBasicCharacters(text: string): string { return normalized; } -/** - * Removes trailing whitespace from each line while keeping the original newline - * separators intact. - */ -function stripTrailingWhitespacePreserveNewlines(text: string): string { - const pieces = text.split(/(\r\n|\n|\r)/); - let result = ''; - - for (let i = 0; i < pieces.length; i++) { - const segment = pieces[i]; - if (segment === undefined) { - continue; - } - - if (i % 2 === 0) { - result += segment.trimEnd(); - } else { - result += segment; - } - } - - return result; -} - /* -------------------------------------------------------------------------- */ /* Line-based search helpers */ /* -------------------------------------------------------------------------- */ @@ -323,23 +299,26 @@ export interface NormalizedEditStrings { /** * Runs the core normalization pipeline: - * 1. Strip trailing whitespace copied from numbered output. - * 2. Attempt to find the literal text inside {@link fileContent}. - * 3. If found through a relaxed match (smart quotes, line trims, etc.), + * 1. Attempt to find the literal text inside {@link fileContent}. + * 2. If found through a relaxed match (smart quotes, line trims, etc.), * return the canonical slice from disk so later replacements operate on * exact bytes. + * 3. Preserve newString as-is (it represents the LLM's intent). + * + * Note: Trailing whitespace in newString is intentionally NOT stripped. + * While LLMs may sometimes accidentally add trailing whitespace, stripping it + * unconditionally breaks legitimate use cases where trailing whitespace is + * intentional (e.g., multi-line strings, heredocs). See issue #1618. */ export function normalizeEditStrings( fileContent: string | null, oldString: string, newString: string, ): NormalizedEditStrings { - const trimmedNewString = stripTrailingWhitespacePreserveNewlines(newString); - if (fileContent === null || oldString === '') { return { oldString, - newString: trimmedNewString, + newString, }; } @@ -348,7 +327,7 @@ export function normalizeEditStrings( return { oldString: canonicalOriginal.slice, newString: adjustNewStringForTrailingLine( - trimmedNewString, + newString, canonicalOriginal.removedTrailingFinalEmptyLine, ), }; @@ -356,7 +335,7 @@ export function normalizeEditStrings( return { oldString, - newString: trimmedNewString, + newString, }; }