From 128330e36f277452f6d07ae749643ee77f4ca4e8 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 19 Jun 2026 20:16:05 +0200 Subject: [PATCH] fix(coding-agent): preserve untouched lines in fuzzy edit closes #5899 --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/core/tools/edit-diff.ts | 157 +++++++++++++++--- packages/coding-agent/test/tools.test.ts | 51 ++++++ 3 files changed, 184 insertions(+), 25 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c5aae0f9..fdc99e61 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- Fixed fuzzy `edit` matches to preserve untouched line blocks instead of rewriting the whole file through normalized content ([#5899](https://github.com/earendil-works/pi/issues/5899)). - Fixed bash commands through legacy WSL `bash.exe` to pass scripts over stdin so shell variables expand in the target bash ([#5893](https://github.com/earendil-works/pi/issues/5893)). - Fixed `/model` to hide GitHub Copilot models that are unavailable to the authenticated account ([#5897](https://github.com/earendil-works/pi/issues/5897)). - Fixed `/model` selector search to rank exact provider-prefixed matches before proxy-provider model ID matches ([#5892](https://github.com/earendil-works/pi/issues/5892)). diff --git a/packages/coding-agent/src/core/tools/edit-diff.ts b/packages/coding-agent/src/core/tools/edit-diff.ts index cc94c202..5a4d966b 100644 --- a/packages/coding-agent/src/core/tools/edit-diff.ts +++ b/packages/coding-agent/src/core/tools/edit-diff.ts @@ -53,6 +53,124 @@ export function normalizeForFuzzyMatch(text: string): string { ); } +function splitLinesWithEndings(content: string): string[] { + return content.match(/[^\n]*\n|[^\n]+/g) ?? []; +} + +interface LineSpan { + start: number; + end: number; +} + +interface MatchedEdit { + editIndex: number; + matchIndex: number; + matchLength: number; + newText: string; +} + +type TextReplacement = Pick; + +function getLineSpans(content: string): LineSpan[] { + let offset = 0; + return splitLinesWithEndings(content).map((line) => { + const span = { start: offset, end: offset + line.length }; + offset = span.end; + return span; + }); +} + +function getReplacementLineRange(lines: LineSpan[], replacement: TextReplacement) { + const replacementStart = replacement.matchIndex; + const replacementEnd = replacement.matchIndex + replacement.matchLength; + + let startLine = -1; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (replacementStart >= line.start && replacementStart < line.end) { + startLine = i; + break; + } + } + if (startLine === -1) { + throw new Error("Replacement range is outside the base content."); + } + + let endLine = startLine; + while (endLine < lines.length && lines[endLine].end < replacementEnd) { + endLine++; + } + if (endLine >= lines.length) { + throw new Error("Replacement range is outside the base content."); + } + + return { startLine, endLine: endLine + 1 }; +} + +function applyReplacements(content: string, replacements: TextReplacement[], offset = 0): string { + let result = content; + for (let i = replacements.length - 1; i >= 0; i--) { + const replacement = replacements[i]; + const matchIndex = replacement.matchIndex - offset; + result = + result.substring(0, matchIndex) + replacement.newText + result.substring(matchIndex + replacement.matchLength); + } + return result; +} + +/** + * Apply replacements matched against `baseContent` to `originalContent` while + * preserving unchanged line blocks from the original. + * + * This is useful when `baseContent` is a normalized view of the original. Each + * replacement is widened to the lines it actually touches, those touched lines + * are rewritten from the normalized base, and all other lines are copied back + * from `originalContent`. The actual replacement ranges drive preservation so + * duplicate normalized lines cannot be aligned to the wrong occurrence. + */ +export function applyReplacementsPreservingUnchangedLines( + originalContent: string, + baseContent: string, + replacements: TextReplacement[], +): string { + const originalLines = splitLinesWithEndings(originalContent); + const baseLines = getLineSpans(baseContent); + if (originalLines.length !== baseLines.length) { + throw new Error("Cannot preserve unchanged lines because the base content has a different line count."); + } + + const groups: Array<{ startLine: number; endLine: number; replacements: TextReplacement[] }> = []; + const sortedReplacements = [...replacements].sort((a, b) => a.matchIndex - b.matchIndex); + for (const replacement of sortedReplacements) { + const range = getReplacementLineRange(baseLines, replacement); + const current = groups[groups.length - 1]; + if (current && range.startLine < current.endLine) { + current.endLine = Math.max(current.endLine, range.endLine); + current.replacements.push(replacement); + continue; + } + groups.push({ ...range, replacements: [replacement] }); + } + + let originalLineIndex = 0; + let result = ""; + for (const group of groups) { + result += originalLines.slice(originalLineIndex, group.startLine).join(""); + + const groupStartOffset = baseLines[group.startLine].start; + const groupEndOffset = baseLines[group.endLine - 1].end; + result += applyReplacements( + baseContent.slice(groupStartOffset, groupEndOffset), + group.replacements, + groupStartOffset, + ); + originalLineIndex = group.endLine; + } + result += originalLines.slice(originalLineIndex).join(""); + + return result; +} + export interface FuzzyMatchResult { /** Whether a match was found */ found: boolean; @@ -74,13 +192,6 @@ export interface Edit { newText: string; } -interface MatchedEdit { - editIndex: number; - matchIndex: number; - matchLength: number; - newText: string; -} - export interface AppliedEditsResult { baseContent: string; newContent: string; @@ -120,9 +231,9 @@ export function fuzzyFindText(content: string, oldText: string): FuzzyMatchResul }; } - // When fuzzy matching, we work in the normalized space for replacement. - // This means the output will have normalized whitespace/quotes/dashes, - // which is acceptable since we're fixing minor formatting differences anyway. + // When fuzzy matching, return offsets in normalized space. Callers can use + // the normalized content to compute replacements, then decide how much of + // that normalized output should be written back. return { found: true, index: fuzzyIndex, @@ -186,8 +297,9 @@ function getNoChangeError(path: string, totalEdits: number): Error { * * All edits are matched against the same original content. Replacements are * then applied in reverse order so offsets remain stable. If any edit needs - * fuzzy matching, the operation runs in fuzzy-normalized content space to - * preserve current single-edit behavior. + * fuzzy matching, the operation runs in fuzzy-normalized content space and then + * overlays those line-level changes onto the original content so unchanged line + * blocks keep their original bytes. */ export function applyEditsToNormalizedContent( normalizedContent: string, @@ -206,19 +318,18 @@ export function applyEditsToNormalizedContent( } const initialMatches = normalizedEdits.map((edit) => fuzzyFindText(normalizedContent, edit.oldText)); - const baseContent = initialMatches.some((match) => match.usedFuzzyMatch) - ? normalizeForFuzzyMatch(normalizedContent) - : normalizedContent; + const usedFuzzyMatch = initialMatches.some((match) => match.usedFuzzyMatch); + const replacementBaseContent = usedFuzzyMatch ? normalizeForFuzzyMatch(normalizedContent) : normalizedContent; const matchedEdits: MatchedEdit[] = []; for (let i = 0; i < normalizedEdits.length; i++) { const edit = normalizedEdits[i]; - const matchResult = fuzzyFindText(baseContent, edit.oldText); + const matchResult = fuzzyFindText(replacementBaseContent, edit.oldText); if (!matchResult.found) { throw getNotFoundError(path, i, normalizedEdits.length); } - const occurrences = countOccurrences(baseContent, edit.oldText); + const occurrences = countOccurrences(replacementBaseContent, edit.oldText); if (occurrences > 1) { throw getDuplicateError(path, i, normalizedEdits.length, occurrences); } @@ -242,14 +353,10 @@ export function applyEditsToNormalizedContent( } } - let newContent = baseContent; - for (let i = matchedEdits.length - 1; i >= 0; i--) { - const edit = matchedEdits[i]; - newContent = - newContent.substring(0, edit.matchIndex) + - edit.newText + - newContent.substring(edit.matchIndex + edit.matchLength); - } + const baseContent = normalizedContent; + const newContent = usedFuzzyMatch + ? applyReplacementsPreservingUnchangedLines(normalizedContent, replacementBaseContent, matchedEdits) + : applyReplacements(replacementBaseContent, matchedEdits); if (baseContent === newContent) { throw getNoChangeError(path, normalizedEdits.length); diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index b06590f7..f8b09c90 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -1031,6 +1031,57 @@ describe("edit tool fuzzy matching", () => { expect(readFileSync(testFile, "utf-8")).toBe("console.log('world');\nhello universe\n"); }); + + it("should preserve the correct occurrence when fuzzy replacement equals a nearby line", async () => { + const testFile = join(testDir, "fuzzy-preserve-duplicate-line.txt"); + const originalContent = ["replace me\u0020\u0020\u0020", "after\u0020\u0020\u0020", ""].join("\n"); + writeFileSync(testFile, originalContent); + + const result = await editTool.execute("test-fuzzy-preserve-duplicate-line", { + path: testFile, + edits: [{ oldText: "replace me\n", newText: "after\n" }], + }); + + const expectedContent = ["after", "after\u0020\u0020\u0020", ""].join("\n"); + expect(readFileSync(testFile, "utf-8")).toBe(expectedContent); + expect(applyPatch(originalContent, result.details?.patch ?? "")).toBe(expectedContent); + }); + + it("should preserve untouched lines and produce an applicable patch for fuzzy multi-edits", async () => { + const testFile = join(testDir, "fuzzy-preserve-multi.txt"); + const originalContent = [ + "keep before\u0020\u0020", + "first target\u0020\u0020", + "first after", + "keep middle\u0020\u0020\u0020", + "second target\u0020\u0020", + "second after", + "keep after\u0020\u0020", + "", + ].join("\n"); + writeFileSync(testFile, originalContent); + + const result = await editTool.execute("test-fuzzy-preserve-multi", { + path: testFile, + edits: [ + { oldText: "first target\nfirst after", newText: "FIRST\nFIRST2" }, + { oldText: "second target\nsecond after", newText: "SECOND\nSECOND2" }, + ], + }); + + const expectedContent = [ + "keep before\u0020\u0020", + "FIRST", + "FIRST2", + "keep middle\u0020\u0020\u0020", + "SECOND", + "SECOND2", + "keep after\u0020\u0020", + "", + ].join("\n"); + expect(readFileSync(testFile, "utf-8")).toBe(expectedContent); + expect(applyPatch(originalContent, result.details?.patch ?? "")).toBe(expectedContent); + }); }); describe("edit tool CRLF handling", () => {