edits: fix conflicting edits in multi-replace-string

src/extension/tools/node/abstractReplaceStringTool.tsx

@@ -25,6 +25,7 @@ import { removeLeadingFilepathComment } from '../../../util/common/markdown';
 import { timeout } from '../../../util/vs/base/common/async';
 import { Iterable } from '../../../util/vs/base/common/iterator';
 import { ResourceMap, ResourceSet } from '../../../util/vs/base/common/map';
+import { extUriBiasedIgnorePathCase } from '../../../util/vs/base/common/resources';
 import { isDefined } from '../../../util/vs/base/common/types';
 import { URI } from '../../../util/vs/base/common/uri';
 import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation';
@@ -102,13 +103,54 @@ export abstract class AbstractReplaceStringTool<T extends { explanation: string
 		if (this.lastOperation?.inputKey !== cacheKey) {
 			this.lastOperation = {
 				inputKey: cacheKey,
-				operation: Promise.all(input.map(i => this._prepareEditsForFile(options, i, token))),
+				operation: this._prepareEdits(options, input, token)
 			};
 		}
 
 		return this.lastOperation.operation;
 	}
 
+	private async _prepareEdits(options: vscode.LanguageModelToolInvocationOptions<T> | vscode.LanguageModelToolInvocationPrepareOptions<T>, input: IAbstractReplaceStringInput[], token: vscode.CancellationToken) {
+		const results = await Promise.all(input.map(i => this._prepareEditsForFile(options, i, token)));
+		this._errorConflictingEdits(results);
+		return results;
+	}
+
+	private _errorConflictingEdits(results: IPrepareEdit[]) {
+		for (let i = 1; i < results.length; i++) {
+			const current = results[i];
+			if (!current.generatedEdit.success) {
+				continue;
+			}
+
+			for (let k = 0; k < i; k++) {
+				const other = results[k];
+				if (!other.generatedEdit.success || !extUriBiasedIgnorePathCase.isEqual(current.uri, other.uri)) {
+					continue;
+				}
+
+				const allEdits = [
+					...current.generatedEdit.textEdits,
+					...other.generatedEdit.textEdits,
+				].sort((a, b) => a.range.start.compareTo(b.range.start));
+
+				const hasOverlap = allEdits.some((e2, i) => {
+					if (i === 0) { return false; }
+					const e1 = allEdits[i - 1];
+					return !e1.range.end.isBeforeOrEqual(e2.range.start);
+				});
+
+				if (hasOverlap) {
+					current.generatedEdit = {
+						success: false,
+						errorMessage: `Edit at index ${i} conflicts with another replacement in ${this.promptPathRepresentationService.getFilePath(current.uri)}. You can make another call to try again.`
+					};
+					break;
+				}
+			}
+		}
+	}
+
 	private async _prepareEditsForFile(options: vscode.LanguageModelToolInvocationOptions<T> | vscode.LanguageModelToolInvocationPrepareOptions<T>, input: IAbstractReplaceStringInput, token: vscode.CancellationToken): Promise<IPrepareEdit> {
 		const uri = resolveToolInputPath(input.filePath, this.promptPathRepresentationService);
 		try {

src/extension/tools/node/editFileToolUtils.tsx

@@ -191,10 +191,10 @@ function calculateSimilarity(str1: string, str2: string): number {
 
 interface MatchResultCommon {
 	type: string;
-	/** Replacement text */
+	/** Resulting document text */
 	text: string;
 	/** Array of [startIndex, endIndex] to replace in the file content */
-	editPosition: [number, number][];
+	editPosition: { start: number; end: number; text: string }[];
 	/** Model suggestion to correct a fialing match */
 	suggestion?: string;
 }
@@ -274,12 +274,19 @@ function tryExactMatch(text: string, oldStr: string, newStr: string): MatchResul
 		return { text, editPosition: [], type: 'none' };
 	}
 
+	const identical = getIdenticalChars(oldStr, newStr);
+	const editPosition = matchPositions.map(idx => ({
+		start: idx + identical.leading,
+		end: idx + oldStr.length - identical.trailing,
+		text: newStr.slice(identical.leading, newStr.length - identical.trailing)
+	}));
+
 	// Check for multiple exact occurrences.
 	if (matchPositions.length > 1) {
 		return {
 			text,
 			type: 'multiple',
-			editPosition: matchPositions.map(idx => [idx, idx + oldStr.length]),
+			editPosition,
 			strategy: 'exact',
 			matchPositions,
 			suggestion: "Multiple exact matches found. Make your search string more specific."
@@ -291,7 +298,7 @@ function tryExactMatch(text: string, oldStr: string, newStr: string): MatchResul
 	return {
 		text: replaced,
 		type: 'exact',
-		editPosition: [[firstExactIdx, firstExactIdx + oldStr.length]],
+		editPosition,
 	};
 }
 
@@ -300,7 +307,8 @@ function tryExactMatch(text: string, oldStr: string, newStr: string): MatchResul
  */
 function tryWhitespaceFlexibleMatch(text: string, oldStr: string, newStr: string, eol: string): MatchResult {
 	const haystack = text.split(eol).map(line => line.trim());
-	const needle = oldStr.trim().split(eol).map(line => line.trim());
+	const oldLines = oldStr.trim().split(eol);
+	const needle = oldLines.map(line => line.trim());
 	needle.push(''); // trailing newline to match until the end of a line
 
 	const convert = new OffsetLineColumnConverter(text);
@@ -320,26 +328,35 @@ function tryWhitespaceFlexibleMatch(text: string, oldStr: string, newStr: string
 		};
 	}
 
-	const positions = matchedLines.map(match => convert.positionToOffset(new EditorPosition(match + 1, 1)));
+
+	const newLines = newStr.trim().split(eol);
+	const identical = getIndenticalLines(oldLines, newLines);
+	const positions = matchedLines.map(match => {
+		const start = new EditorPosition(match + identical.leading + 1, 1);
+		const end = start.delta(oldLines.length - identical.trailing);
+		return { start, end };
+	});
 
 	if (matchedLines.length > 1) {
 		return {
 			text,
 			type: 'multiple',
 			editPosition: [],
-			matchPositions: positions,
+			matchPositions: positions.map(p => convert.positionToOffset(p.start)),
 			suggestion: "Multiple matches found with flexible whitespace. Make your search string more unique.",
 			strategy: 'whitespace',
 		};
 	}
 
-	// Exactly one whitespace-flexible match found
-	const startIdx = positions[0];
-	const endIdx = convert.positionToOffset(new EditorPosition(matchedLines[0] + 1 + needle.length, 1));
-	const replaced = text.slice(0, startIdx) + newStr + eol + text.slice(endIdx);
+	const { start, end } = positions[0];
+	const startIdx = convert.positionToOffset(start);
+	const endIdx = convert.positionToOffset(end) - 1; // -1 to include the last EOL
+
+	const minimizedNewStr = newLines.slice(identical.leading, newLines.length - identical.trailing).join(eol);
+	const replaced = text.slice(0, startIdx) + minimizedNewStr + text.slice(endIdx);
 	return {
 		text: replaced,
-		editPosition: [[startIdx, endIdx]],
+		editPosition: [{ start: startIdx, end: endIdx, text: minimizedNewStr }],
 		type: 'whitespace',
 	};
 }
@@ -356,11 +373,11 @@ function tryFuzzyMatch(text: string, oldStr: string, newStr: string, eol: string
 
 	// Build a regex pattern where each line is matched exactly
 	// but allows for trailing spaces/tabs and flexible newline formats
-	const lines = oldStr.split(eol);
-	const pattern = lines
+	const oldLines = oldStr.split(eol);
+	const pattern = oldLines
 		.map((line, i) => {
 			const escaped = escapeRegex(line);
-			return i < lines.length - 1 || hasTrailingLF
+			return i < oldLines.length - 1 || hasTrailingLF
 				? `${escaped}[ \\t]*\\r?\\n`
 				: `${escaped}[ \\t]*`;
 		})
@@ -395,15 +412,23 @@ function tryFuzzyMatch(text: string, oldStr: string, newStr: string, eol: string
 	return {
 		text: replaced,
 		type: 'fuzzy',
-		editPosition: [[startIdx, endIdx]],
+		editPosition: [{ start: startIdx, end: endIdx, text: newStr }],
 	};
 }
 
+let defaultSimilaryMatchThreshold = 0.95;
+
+export function setSimilarityMatchThresholdForTests(threshold: number) {
+	const old = defaultSimilaryMatchThreshold;
+	defaultSimilaryMatchThreshold = threshold;
+	return old;
+}
+
 /**
  * Tries to match based on overall string similarity as a last resort.
  * Only works for relatively small strings to avoid performance issues.
  */
-function trySimilarityMatch(text: string, oldStr: string, newStr: string, eol: string, threshold: number = 0.95): MatchResult {
+function trySimilarityMatch(text: string, oldStr: string, newStr: string, eol: string, threshold: number = defaultSimilaryMatchThreshold): MatchResult {
 	// Skip similarity matching for very large strings or too many lines
 	if (oldStr.length > 1000 || oldStr.split(eol).length > 20) {
 		return { text, editPosition: [], type: 'none' };
@@ -417,6 +442,9 @@ function trySimilarityMatch(text: string, oldStr: string, newStr: string, eol: s
 		return { text, editPosition: [], type: 'none' };
 	}
 
+	const newLines = newStr.split(eol);
+	const identical = getIndenticalLines(oldLines, newLines);
+
 	let bestMatch = { startLine: -1, startOffset: 0, oldLength: 0, similarity: 0 };
 	let startOffset = 0;
 
@@ -426,15 +454,29 @@ function trySimilarityMatch(text: string, oldStr: string, newStr: string, eol: s
 		let oldLength = 0;
 
 		// Calculate similarity for each line in the window
+		let startOffsetIdenticalIncr = 0;
+		let endOffsetIdenticalIncr = 0;
 		for (let j = 0; j < oldLines.length; j++) {
 			const similarity = calculateSimilarity(oldLines[j], lines[i + j]);
 			totalSimilarity += similarity;
 			oldLength += lines[i + j].length;
+
+			if (j < identical.leading) {
+				startOffsetIdenticalIncr += lines[i + j].length + eol.length;
+			}
+			if (j >= oldLines.length - identical.trailing) {
+				endOffsetIdenticalIncr += lines[i + j].length + eol.length;
+			}
 		}
 
 		const avgSimilarity = totalSimilarity / oldLines.length;
 		if (avgSimilarity > threshold && avgSimilarity > bestMatch.similarity) {
-			bestMatch = { startLine: i, startOffset, similarity: avgSimilarity, oldLength: oldLength + (oldLines.length - 1) * eol.length };
+			bestMatch = {
+				startLine: i + identical.leading,
+				startOffset: startOffset + startOffsetIdenticalIncr,
+				similarity: avgSimilarity,
+				oldLength: oldLength + (oldLines.length - 1) * eol.length - startOffsetIdenticalIncr - endOffsetIdenticalIncr,
+			};
 		}
 
 		startOffset += lines[i].length + eol.length;
@@ -445,16 +487,24 @@ function trySimilarityMatch(text: string, oldStr: string, newStr: string, eol: s
 	}
 
 	// Replace the matched section
-	const newLines = [
+	const newStrMinimized = newLines.slice(identical.leading, newLines.length - identical.trailing).join(eol);
+	const matchStart = bestMatch.startLine - identical.leading;
+	const afterIdx = matchStart + oldLines.length - identical.trailing;
+
+	const newText = [
 		...lines.slice(0, bestMatch.startLine),
-		...newStr.split(eol),
-		...lines.slice(bestMatch.startLine + oldLines.length)
-	];
+		...newLines.slice(identical.leading, newLines.length - identical.trailing),
+		...lines.slice(afterIdx),
+	].join(eol);
 
 	return {
-		text: newLines.join(eol),
+		text: newText,
 		type: 'similarity',
-		editPosition: [[bestMatch.startOffset, bestMatch.startOffset + bestMatch.oldLength]],
+		editPosition: [{
+			start: bestMatch.startOffset,
+			end: bestMatch.startOffset + bestMatch.oldLength,
+			text: newStrMinimized,
+		}],
 		similarity: bestMatch.similarity,
 		suggestion: `Used similarity matching (${(bestMatch.similarity * 100).toFixed(1)}% similar). Verify the replacement.`
 	};
@@ -472,6 +522,39 @@ function getPatch({ fileContents, oldStr, newStr }: { fileContents: string; oldS
 	}];
 }
 
+/** Gets the number of identical leading and trailing lines between two arrays of strings */
+function getIndenticalLines(a: string[], b: string[]) {
+	let leading = 0;
+	let trailing = 0;
+	while (leading < a.length &&
+		leading < b.length &&
+		a[leading] === b[leading]) {
+		leading++;
+	}
+	while (trailing + leading < a.length &&
+		trailing + leading < b.length &&
+		a[a.length - 1 - trailing] === b[b.length - 1 - trailing]) {
+		trailing++;
+	}
+
+	return { leading, trailing };
+}
+
+/** Gets the number of identical leading and trailing characters between two strings */
+function getIdenticalChars(oldString: string, newString: string) {
+	let leading = 0;
+	let trailing = 0;
+
+	while (leading < oldString.length && leading < newString.length && oldString[leading] === newString[leading]) {
+		leading++;
+	}
+	while (trailing + leading < oldString.length && trailing + leading < newString.length &&
+		oldString[oldString.length - trailing - 1] === newString[newString.length - trailing - 1]) {
+		trailing++;
+	}
+	return { leading, trailing };
+}
+
 // Apply string edit function
 export async function applyEdit(
 	uri: URI,
@@ -518,7 +601,7 @@ export async function applyEdit(
 						updatedFile = originalFile.replace(old_string + eol, new_string);
 
 						if (result.editPosition.length) {
-							const [start, end] = result.editPosition[0];
+							const { start, end } = result.editPosition[0];
 							const range = new Range(document.positionAt(start), document.positionAt(end));
 							edits.push(TextEdit.delete(range));
 						}
@@ -539,7 +622,7 @@ export async function applyEdit(
 					updatedFile = result.text;
 
 					if (result.editPosition.length) {
-						const [start, end] = result.editPosition[0];
+						const { start, end } = result.editPosition[0];
 						const range = new Range(document.positionAt(start), document.positionAt(end));
 						edits.push(TextEdit.delete(range));
 					}
@@ -564,9 +647,9 @@ export async function applyEdit(
 					updatedFile = result.text;
 
 					if (result.editPosition.length) {
-						const [start, end] = result.editPosition[0];
+						const { start, end, text } = result.editPosition[0];
 						const range = new Range(document.positionAt(start), document.positionAt(end));
-						edits.push(TextEdit.replace(range, new_string));
+						edits.push(TextEdit.replace(range, text));
 					}
 
 					// If we used similarity matching, add a warning