-
Notifications
You must be signed in to change notification settings - Fork 1.8k
applyPatchTool: original content leaks through when trailing newline counts differ #4667
Description
Bug
generateUpdateTextDocumentEdit in applyPatchTool.tsx uses lines.length (from the new content) as the replace range end instead of covering the entire original document. When the AI shortens a file and the trailing newline count differs between old and new content, original lines in the gap between the replace end and the delete start are neither replaced nor deleted, leaking into the result.
Root cause
The original code applies three separate TextEdits (all on original positions, applied atomically):
// 1. Replace [0, lines.length) with newContent
workspaceEdit.replace(path, new Range(
new Position(0, 0),
new Position(lines.length, 0) // <-- new content's line count, not original's
), newContent);
// 2. Insert trailing newlines
for (let i = newTrailing; i < originalTrailing; i++) {
workspaceEdit.insert(path, new Position(lines.length + i, 0), '\n');
}
// 3. Delete remaining lines
if (lines.length < textDocument.lineCount) {
const newLineCount = lines.length + Math.max(originalTrailing - newTrailing, 0);
workspaceEdit.delete(path, new Range(
new Position(newLineCount, 0),
new Position(textDocument.lineCount, 0)
));
}When originalTrailing > newTrailing, a gap of (originalTrailing - newTrailing) original lines exists between the replace end (lines.length) and the delete start (newLineCount). These lines are neither replaced nor deleted.
Reproduction
Original file (4 lines, 1 trailing empty):
ALPHA
BETA
GAMMA
(content: "ALPHA\nBETA\nGAMMA\n", lineCount=4, originalTrailing=1)
AI removes BETA and GAMMA, producing newContent = "ALPHA" (no trailing newline):
lines = ["ALPHA"],lines.length = 1,newTrailing = 0
TextEdits generated (on original positions)
| Edit | Range (original) | Replacement | Offset |
|---|---|---|---|
| replace | (0,0)-(1,0) | "ALPHA" | [0, 6) |
| insert | (1,0)-(1,0) | "\n" | [6, 6) |
| delete | (2,0)-(4,0) | "" | [12, 18) |
Atomic application on "ALPHA\nBETA\nGAMMA\n"
| Offset | Original | Edit | Running output |
|---|---|---|---|
| [0, 6) | "ALPHA\n" | replace with "ALPHA" | "ALPHA" |
| 6 | -- | insert "\n" | "ALPHA\n" |
| [6, 12) | "BETA\n" | no edit | "ALPHA\nBETA\n" |
| [12, 18) | "GAMMA\n" | delete | "ALPHA\nBETA\n" |
Result: "ALPHA\nBETA\n" -- original "BETA" leaked through.
Expected: "ALPHA\n"
The bug triggers when an AI shortens a file (common during refactoring) and the trailing newline counts differ (common when the LLM omits the trailing newline).
Additional impact
turnEditedDocuments stores the expected content (changes.newContent), creating a cascading divergence between the AI's view of the file and the actual file state. Subsequent edits operate on wrong assumptions.
Fix (implemented in PR #4668)
Build adjustedContent with trailing newlines appended, then replace the entire original document in a single operation:
const originalTrailing = this.getTrailingDocumentEmptyLineCount(textDocument);
const newTrailing = this.getTrailingArrayEmptyLineCount(lines);
const extraTrailing = Math.max(originalTrailing - newTrailing, 0);
const adjustedContent = extraTrailing > 0 ? newContent + '\n'.repeat(extraTrailing) : newContent;
workspaceEdit.replace(path, new Range(
new Position(0, 0),
new Position(textDocument.lineCount, 0) // covers entire original
), adjustedContent);This replaces the prior three-operation approach (replace + insert loop + conditional delete) with a single replace that covers the full document. No gap is possible.
This approach matches the correct pattern already used in codeMapper.ts.
A regression test is included that reproduces this exact scenario and asserts the result is "ALPHA\n" (not "ALPHA\nBETA\n").