Merge pull request #1234 from qdaxb/support_edit_multi_locations_in_apply_diff

Supports updating multiple locations of a file in one call of the apply_diff tool

Author: mrubens

src/core/Cline.ts

@@ -184,7 +184,10 @@ export class Cline {
 		this.checkpointStorage = checkpointStorage
 
 		// Initialize diffStrategy based on current state
-		this.updateDiffStrategy(Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.DIFF_STRATEGY))
+		this.updateDiffStrategy(
+			Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.DIFF_STRATEGY),
+			Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE),
+		)
 
 		if (startTask) {
 			if (task || images) {
@@ -254,13 +257,23 @@ export class Cline {
 	}
 
 	// Add method to update diffStrategy
-	async updateDiffStrategy(experimentalDiffStrategy?: boolean) {
+	async updateDiffStrategy(experimentalDiffStrategy?: boolean, multiSearchReplaceDiffStrategy?: boolean) {
 		// If not provided, get from current state
-		if (experimentalDiffStrategy === undefined) {
+		if (experimentalDiffStrategy === undefined || multiSearchReplaceDiffStrategy === undefined) {
 			const { experiments: stateExperimental } = (await this.providerRef.deref()?.getState()) ?? {}
-			experimentalDiffStrategy = stateExperimental?.[EXPERIMENT_IDS.DIFF_STRATEGY] ?? false
+			if (experimentalDiffStrategy === undefined) {
+				experimentalDiffStrategy = stateExperimental?.[EXPERIMENT_IDS.DIFF_STRATEGY] ?? false
+			}
+			if (multiSearchReplaceDiffStrategy === undefined) {
+				multiSearchReplaceDiffStrategy = stateExperimental?.[EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE] ?? false
+			}
 		}
-		this.diffStrategy = getDiffStrategy(this.api.getModel().id, this.fuzzyMatchThreshold, experimentalDiffStrategy)
+		this.diffStrategy = getDiffStrategy(
+			this.api.getModel().id,
+			this.fuzzyMatchThreshold,
+			experimentalDiffStrategy,
+			multiSearchReplaceDiffStrategy,
+		)
 	}
 
 	// Storing task to disk for history
@@ -1667,17 +1680,36 @@ export class Cline {
 									success: false,
 									error: "No diff strategy available",
 								}
+								let partResults = ""
+
 								if (!diffResult.success) {
 									this.consecutiveMistakeCount++
 									const currentCount =
 										(this.consecutiveMistakeCountForApplyDiff.get(relPath) || 0) + 1
 									this.consecutiveMistakeCountForApplyDiff.set(relPath, currentCount)
-									const errorDetails = diffResult.details
-										? JSON.stringify(diffResult.details, null, 2)
-										: ""
-									const formattedError = `Unable to apply diff to file: ${absolutePath}\n\n<error_details>\n${
-										diffResult.error
-									}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n</error_details>`
+									let formattedError = ""
+									if (diffResult.failParts && diffResult.failParts.length > 0) {
+										for (const failPart of diffResult.failParts) {
+											if (failPart.success) {
+												continue
+											}
+											const errorDetails = failPart.details
+												? JSON.stringify(failPart.details, null, 2)
+												: ""
+											formattedError = `<error_details>\n${
+												failPart.error
+											}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n</error_details>`
+											partResults += formattedError
+										}
+									} else {
+										const errorDetails = diffResult.details
+											? JSON.stringify(diffResult.details, null, 2)
+											: ""
+										formattedError = `Unable to apply diff to file: ${absolutePath}\n\n<error_details>\n${
+											diffResult.error
+										}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n</error_details>`
+									}
+
 									if (currentCount >= 2) {
 										await this.say("error", formattedError)
 									}
@@ -1707,6 +1739,10 @@ export class Cline {
 								const { newProblemsMessage, userEdits, finalContent } =
 									await this.diffViewProvider.saveChanges()
 								this.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request
+								let partFailHint = ""
+								if (diffResult.failParts && diffResult.failParts.length > 0) {
+									partFailHint = `But unable to apply all diff parts to file: ${absolutePath}, silently use <read_file> tool to check newest file version and re-apply diffs\n`
+								}
 								if (userEdits) {
 									await this.say(
 										"user_feedback_diff",
@@ -1718,6 +1754,7 @@ export class Cline {
 									)
 									pushToolResult(
 										`The user made the following updates to your content:\n\n${userEdits}\n\n` +
+											partFailHint +
 											`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` +
 											`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(
 												finalContent || "",
@@ -1730,7 +1767,8 @@ export class Cline {
 									)
 								} else {
 									pushToolResult(
-										`Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}`,
+										`Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}\n` +
+											partFailHint,
 									)
 								}
 								await this.diffViewProvider.reset()

src/core/__tests__/Cline.test.ts

@@ -375,7 +375,7 @@ describe("Cline", () => {
 
 			expect(cline.diffEnabled).toBe(true)
 			expect(cline.diffStrategy).toBeDefined()
-			expect(getDiffStrategySpy).toHaveBeenCalledWith("claude-3-5-sonnet-20241022", 0.9, false)
+			expect(getDiffStrategySpy).toHaveBeenCalledWith("claude-3-5-sonnet-20241022", 0.9, false, false)
 
 			getDiffStrategySpy.mockRestore()
 
@@ -396,7 +396,7 @@ describe("Cline", () => {
 
 			expect(cline.diffEnabled).toBe(true)
 			expect(cline.diffStrategy).toBeDefined()
-			expect(getDiffStrategySpy).toHaveBeenCalledWith("claude-3-5-sonnet-20241022", 1.0, false)
+			expect(getDiffStrategySpy).toHaveBeenCalledWith("claude-3-5-sonnet-20241022", 1.0, false, false)
 
 			getDiffStrategySpy.mockRestore()
 

src/core/diff/DiffStrategy.ts

@@ -2,6 +2,7 @@ import type { DiffStrategy } from "./types"
 import { UnifiedDiffStrategy } from "./strategies/unified"
 import { SearchReplaceDiffStrategy } from "./strategies/search-replace"
 import { NewUnifiedDiffStrategy } from "./strategies/new-unified"
+import { MultiSearchReplaceDiffStrategy } from "./strategies/multi-search-replace"
 /**
  * Get the appropriate diff strategy for the given model
  * @param model The name of the model being used (e.g., 'gpt-4', 'claude-3-opus')
@@ -11,11 +12,17 @@ export function getDiffStrategy(
 	model: string,
 	fuzzyMatchThreshold?: number,
 	experimentalDiffStrategy: boolean = false,
+	multiSearchReplaceDiffStrategy: boolean = false,
 ): DiffStrategy {
 	if (experimentalDiffStrategy) {
 		return new NewUnifiedDiffStrategy(fuzzyMatchThreshold)
 	}
-	return new SearchReplaceDiffStrategy(fuzzyMatchThreshold)
+
+	if (multiSearchReplaceDiffStrategy) {
+		return new MultiSearchReplaceDiffStrategy(fuzzyMatchThreshold)
+	} else {
+		return new SearchReplaceDiffStrategy(fuzzyMatchThreshold)
+	}
 }
 
 export type { DiffStrategy }