fix(apply-diff): report all failed diff blocks and diagnose mistyped start_line markers

.changeset/apply-diff-failure-reporting.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+Fix `apply_diff` error reporting so failed diffs are fixable in a single retry. When several diff blocks fail, all of them are now reported together (with per-block labels, the first few in full detail and the rest summarized) instead of only the last one. A mistyped start-line marker such as `:3` in place of `:start_line:3` is now called out as the likely cause, replacing the misleading "the file content may have changed" advice. When only some blocks apply, the response now says how many applied and asks for only the failed blocks to be resent, and includes the actual errors instead of an instruction that could not work.

src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts

@@ -0,0 +1,81 @@
+import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace"
+
+// Guards the POST-FAILURE "LIKELY CAUSE" hint for mistyped start-line markers.
+// The heuristic must never act as an upfront rejection gate.
+describe("MultiSearchReplaceDiffStrategy malformed start-line marker hint", () => {
+	let strategy: MultiSearchReplaceDiffStrategy
+
+	beforeEach(() => {
+		strategy = new MultiSearchReplaceDiffStrategy()
+	})
+
+	const nonMatchingFile = ["alpha", "beta", "gamma", "delta", "epsilon"].join("\n")
+
+	function buildDiff(searchBody: string, replaceBody: string): string {
+		return `<<<<<<< SEARCH\n${searchBody}\n=======\n${replaceBody}\n>>>>>>> REPLACE`
+	}
+
+	function firstFailureError(result: Awaited<ReturnType<MultiSearchReplaceDiffStrategy["applyDiff"]>>): string {
+		expect(result.success).toBe(false)
+		const failures = (result as any).failParts ?? []
+		expect(failures.length).toBeGreaterThan(0)
+		return failures[0].error as string
+	}
+
+	it("adds LIKELY CAUSE when the first search line is a bare ':3' marker and the block fails", async () => {
+		const diffContent = buildDiff(":3\nnot-in-the-file", "replacement")
+		const result = await strategy.applyDiff(nonMatchingFile, diffContent)
+
+		const errorText = firstFailureError(result)
+		expect(errorText).toContain("LIKELY CAUSE")
+		expect(errorText).toContain(":start_line:")
+	})
+
+	const markerVariants = ["start_line:3", ":start-line 3", ": start_line : 3", ":START_LINE:3", "Start_Line: 3"]
+
+	it.each(markerVariants)("adds LIKELY CAUSE for the malformed marker variant %j", async (markerVariant) => {
+		const diffContent = buildDiff(`${markerVariant}\nnot-in-the-file`, "replacement")
+		const result = await strategy.applyDiff(nonMatchingFile, diffContent)
+
+		expect(firstFailureError(result)).toContain("LIKELY CAUSE")
+	})
+
+	it("still applies cleanly when the file genuinely contains a ':3' line (hint is not a gate)", async () => {
+		const fileWithMarkerLikeLine = ["alpha", ":3", "beta", "gamma"].join("\n")
+		const diffContent = buildDiff(":3\nbeta", ":3\nBETA")
+
+		const result = await strategy.applyDiff(fileWithMarkerLikeLine, diffContent)
+
+		expect(result.success).toBe(true)
+		const appliedContent = (result as any).content as string
+		expect(appliedContent).toContain("BETA")
+		expect(appliedContent).not.toContain("LIKELY CAUSE")
+		expect((result as any).failParts ?? []).toHaveLength(0)
+	})
+
+	it("omits the hint for ordinary non-matching content but keeps the file-changed tip", async () => {
+		const diffContent = buildDiff("completely unrelated content\nsecond line", "replacement")
+		const result = await strategy.applyDiff(nonMatchingFile, diffContent)
+
+		const errorText = firstFailureError(result)
+		expect(errorText).not.toContain("LIKELY CAUSE")
+		expect(errorText).toContain("the file content may have changed")
+	})
+
+	it("does not flag the correct ':start_line:3' marker form", async () => {
+		const diffContent = `<<<<<<< SEARCH\n:start_line:3\n-------\nnot-in-the-file\n=======\nreplacement\n>>>>>>> REPLACE`
+		const result = await strategy.applyDiff(nonMatchingFile, diffContent)
+
+		expect(firstFailureError(result)).not.toContain("LIKELY CAUSE")
+	})
+
+	it("adds LIKELY CAUSE when a well-formed ':start_line:N' line goes unparsed and swallows its separator", async () => {
+		// Indenting the marker makes the parser miss it, so both it and the "-------" become search text.
+		const diffContent = buildDiff("  :start_line:3\n-------\nnot-in-the-file", "replacement")
+		const result = await strategy.applyDiff(nonMatchingFile, diffContent)
+
+		const errorText = firstFailureError(result)
+		expect(errorText).toContain("LIKELY CAUSE")
+		expect(errorText).toContain('together with the "-------" line after it')
+	})
+})

src/core/diff/strategies/multi-search-replace.ts

@@ -457,9 +457,31 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
 
 					const lineRange = startLine ? ` at line: ${startLine}` : ""
 
+					// A mistyped start-line marker (":3", "start_line:3") is not recognized by
+					// the parser, so it becomes the first line of the search text - and the
+					// following "-------" is swallowed too - making an exact match impossible.
+					const hintLines = searchChunk.split("\n")
+					const firstSearchLine = hintLines[0]?.trim() ?? ""
+					const separatorSwallowed = hintLines[1]?.trim() === "-------"
+					const malformedMarkerHint = /^(?::\s*\d{1,9}:?|:?\s*start[_-]?line\s*:?\s*\d{1,9}:?)$/i.test(
+						firstSearchLine,
+					)
+						? `\n- LIKELY CAUSE: the first line of your search content is "${firstSearchLine}", which looks like a malformed start-line marker and was therefore searched for as literal file text${separatorSwallowed ? ', together with the "-------" line after it' : ""}. The only recognized form is ":start_line:<number>" on the line immediately after "<<<<<<< SEARCH", optionally followed by a "-------" line. Remove or correct that line and retry.`
+						: ""
+
 					diffResults.push({
 						success: false,
-						error: `No sufficiently similar match found${lineRange} (${Math.floor(bestMatchScore * 100)}% similar, needs ${Math.floor(this.fuzzyThreshold * 100)}%)\n\nDebug Info:\n- Similarity Score: ${Math.floor(bestMatchScore * 100)}%\n- Required Threshold: ${Math.floor(this.fuzzyThreshold * 100)}%\n- Search Range: ${startLine ? `starting at line ${startLine}` : "start to end"}\n- Tried both standard and aggressive line number stripping\n- Tip: Use the read_file tool to get the latest content of the file before attempting to use the apply_diff tool again, as the file content may have changed\n\nSearch Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`,
+						error:
+							`No sufficiently similar match found${lineRange} ` +
+							`(${Math.floor(bestMatchScore * 100)}% similar, needs ${Math.floor(this.fuzzyThreshold * 100)}%)\n\n` +
+							`Debug Info:\n` +
+							`- Similarity Score: ${Math.floor(bestMatchScore * 100)}%\n` +
+							`- Required Threshold: ${Math.floor(this.fuzzyThreshold * 100)}%\n` +
+							`- Search Range: ${startLine ? `starting at line ${startLine}` : "start to end"}\n` +
+							`- Tried both standard and aggressive line number stripping${malformedMarkerHint}\n` +
+							`- Tip: Use the read_file tool to get the latest content of the file before attempting ` +
+							`to use the apply_diff tool again, as the file content may have changed\n\n` +
+							`Search Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`,
 					})
 					continue
 				}
@@ -513,13 +535,6 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
 			appliedCount++
 		}
 		const finalContent = resultLines.join(lineEnding)
-		// [apply_diff diagnostics] TEMP: confirm appliedCount/failParts at the all-or-nothing gate.
-		console.warn(
-			`[apply_diff diagnostics] applyDiff summary` +
-				` matches=${matches.length}` +
-				` appliedCount=${appliedCount}` +
-				` failParts=${diffResults.length}`,
-		)
 		if (appliedCount === 0) {
 			return {
 				success: false,

src/core/tools/ApplyDiffTool.ts

@@ -11,7 +11,7 @@ import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
 import { unescapeHtmlEntities } from "../../utils/text-normalization"
 import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
 import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
-import type { ToolUse } from "../../shared/tools"
+import type { DiffResult, ToolUse } from "../../shared/tools"
 
 import { BaseTool, ToolCallbacks } from "./BaseTool"
 
@@ -20,6 +20,19 @@ interface ApplyDiffParams {
 	diff: string
 }
 
+// Each failed block embeds a slice of the file, so only the first N are shown in
+// full; the rest are abbreviated to keep the error payload bounded.
+const DETAILED_APPLY_DIFF_FAILURES = 5
+
+// A single detailed failure can embed the whole file, so cap its rendered size.
+const MAX_DETAILED_FAILURE_CHARACTERS = 4000
+
+// Abbreviated failures are one line each, but a single line can still be very long.
+const MAX_SUMMARY_FAILURE_CHARACTERS = 200
+
+// Beyond this many failures even one-line summaries dominate the payload.
+const MAX_ABBREVIATED_FAILURES = 50
+
 export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 	readonly name = "apply_diff" as const
 
@@ -86,26 +99,13 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 				let formattedError = ""
 
 				if (diffResult.failParts && diffResult.failParts.length > 0) {
-					// [apply_diff diagnostics] TEMP: how many failParts exist, and how many are surfaced.
-					const failedParts = diffResult.failParts.filter((p) => !p.success)
-					console.warn(
-						`[apply_diff diagnostics] surfacing failParts` +
-							` total=${diffResult.failParts.length}` +
-							` failed=${failedParts.length}` +
-							` (only the LAST failed message is shown to the model)`,
-					)
-
-					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>`
-					}
+					const failedParts = diffResult.failParts.filter((part) => !part.success)
+					const summary =
+						failedParts.length > 1
+							? `${failedParts.length} diff blocks failed to apply. All of them are reported below and must be fixed before retrying.\n\n`
+							: ""
+
+					formattedError = `<error_details>\n${summary}${this.formatFailedParts(failedParts)}\n</error_details>`
 				} else {
 					const errorDetails = diffResult.details ? JSON.stringify(diffResult.details, null, 2) : ""
 
@@ -238,17 +238,25 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 
 			// Used to determine if we should wait for busy terminal to update before sending api request
 			task.didEditFile = true
+
+			// Counted by regex, so a literal `<<<<<<< SEARCH` inside block content inflates it: the `X of Y` count is advisory.
+			const searchBlocks = (diffContent.match(/<<<<<<< SEARCH/g) || []).length
+			const unappliedParts = diffResult.failParts?.filter((part) => !part.success) ?? []
 			let partFailHint = ""
 
-			if (diffResult.failParts && diffResult.failParts.length > 0) {
-				partFailHint = `But unable to apply all diff parts to file: ${absolutePath}. Use the read_file tool to check the newest file version and re-apply diffs.\n`
+			if (unappliedParts.length > 0) {
+				const appliedBlocks = Math.max(0, searchBlocks - unappliedParts.length)
+				partFailHint =
+					`Applied ${appliedBlocks} of ${searchBlocks} diff blocks to ${absolutePath}. ` +
+					`The other ${unappliedParts.length} did NOT apply, so those changes are NOT in the file. ` +
+					`Fix the failures below and resend ONLY the blocks that failed; the blocks that already applied will no longer match.\n\n` +
+					`<error_details>\n${this.formatFailedParts(unappliedParts)}\n</error_details>\n\n`
 			}
 
 			// Get the formatted response message
 			const message = await task.diffViewProvider.pushToolWriteResult(task, task.cwd, !fileExists)
 
 			// Check for single SEARCH/REPLACE block warning
-			const searchBlocks = (diffContent.match(/<<<<<<< SEARCH/g) || []).length
 			const singleBlockNotice =
 				searchBlocks === 1
 					? "\n<notice>Making multiple related changes in a single apply_diff is more efficient. If other changes are needed in this file, please include them as additional SEARCH/REPLACE blocks.</notice>"
@@ -276,6 +284,53 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 		}
 	}
 
+	// Reporting only some failures makes the model rediscover the rest one request at a
+	// time, exhausting the consecutive-mistake limit before the diff can converge.
+	private formatFailedParts(failedParts: Extract<DiffResult, { success: false }>[]): string {
+		const renderedCount = Math.min(failedParts.length, MAX_ABBREVIATED_FAILURES)
+		const sections = failedParts.slice(0, renderedCount).map((failPart, index) => {
+			const label = failedParts.length > 1 ? `--- Diff block ${index + 1} of ${failedParts.length} ---\n` : ""
+			const error = failPart.error ?? "Unknown apply_diff failure"
+
+			if (index >= DETAILED_APPLY_DIFF_FAILURES) {
+				const summaryLine = error.split("\n")[0]
+				return `${label}${this.truncate(summaryLine, MAX_SUMMARY_FAILURE_CHARACTERS)}`
+			}
+
+			const errorDetails = failPart.details ? JSON.stringify(failPart.details, null, 2) : ""
+			const detailedSection = `${label}${error}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}`
+
+			return this.truncate(detailedSection, MAX_DETAILED_FAILURE_CHARACTERS)
+		})
+
+		const trailers: string[] = []
+		const abbreviatedRangeStart = DETAILED_APPLY_DIFF_FAILURES + 1
+
+		if (renderedCount > DETAILED_APPLY_DIFF_FAILURES) {
+			trailers.push(
+				`(Failures ${abbreviatedRangeStart}-${renderedCount} are shown as a single summary line each; retry those blocks individually for full detail.)`,
+			)
+		}
+
+		const omitted = failedParts.length - renderedCount
+
+		if (omitted > 0) {
+			trailers.push(`(+${omitted} more failures not shown; retry those blocks individually for full detail.)`)
+		}
+
+		return [sections.join("\n\n"), ...trailers].join("\n\n")
+	}
+
+	private truncate(text: string, maxCharacters: number): string {
+		if (text.length <= maxCharacters) {
+			return text
+		}
+
+		const omitted = text.length - maxCharacters
+
+		return `${text.slice(0, maxCharacters)}\n... [truncated; ${omitted} characters omitted — retry this block alone for full detail]`
+	}
+
 	override async handlePartial(task: Task, block: ToolUse<"apply_diff">): Promise<void> {
 		const relPath: string | undefined = block.params.path
 		const diffContent: string | undefined = block.params.diff