From d9d96e47006786acbb05d6ef0ee7a927f91f8f12 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Fri, 31 Jul 2026 15:28:49 -0700 Subject: [PATCH] fix(apply-diff): report all failed diff blocks and diagnose mistyped start_line markers Previously only the last failure reached the model, so multi-block apply_diff errors were effectively invisible and the model retried blind. - Report every failed block with '--- Diff block N of M ---' labels and bounded per-block detail instead of surfacing only the final failure. - Emit a 'LIKELY CAUSE' hint when a failed block's first line looks like a mistyped start-line marker. - Partial success now reads 'Applied X of Y diff blocks' and instructs resending only the failed blocks. --- .changeset/apply-diff-failure-reporting.md | 5 + .../multi-search-replace-marker-hint.spec.ts | 81 +++++ .../diff/strategies/multi-search-replace.ts | 31 +- src/core/tools/ApplyDiffTool.ts | 103 +++++-- .../applyDiffTool.failureReporting.spec.ts | 291 ++++++++++++++++++ 5 files changed, 479 insertions(+), 32 deletions(-) create mode 100644 .changeset/apply-diff-failure-reporting.md create mode 100644 src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts create mode 100644 src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts diff --git a/.changeset/apply-diff-failure-reporting.md b/.changeset/apply-diff-failure-reporting.md new file mode 100644 index 00000000000..f852656c613 --- /dev/null +++ b/.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. diff --git a/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts new file mode 100644 index 00000000000..6eeadbb9b63 --- /dev/null +++ b/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>): 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') + }) +}) diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index 98cca973db6..b23f13a4d07 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/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:" 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, diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index b73334833a0..2c931610a88 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/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 = `\n${ - failPart.error - }${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n` - } + 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 = `\n${summary}${this.formatFailedParts(failedParts)}\n` } 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` + + `\n${this.formatFailedParts(unappliedParts)}\n\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 ? "\nMaking 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." @@ -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[]): 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 { const relPath: string | undefined = block.params.path const diffContent: string | undefined = block.params.diff diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts new file mode 100644 index 00000000000..5fc8475830b --- /dev/null +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -0,0 +1,291 @@ +import fs from "fs/promises" + +import type { DiffResult, ToolResponse } from "../../../shared/tools" +import { fileExistsAtPath } from "../../../utils/fs" +import { applyDiffTool } from "../ApplyDiffTool" + +vi.mock("fs/promises", () => ({ + default: { + readFile: vi.fn().mockResolvedValue(""), + }, +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(true), +})) + +vi.mock("../../../utils/path", () => ({ + getReadablePath: vi.fn().mockReturnValue("test/file.txt"), +})) + +vi.mock("../../prompts/responses", () => ({ + formatResponse: { + toolError: vi.fn((message: string) => `Error: ${message}`), + rooIgnoreError: vi.fn((filePath: string) => `Access denied: ${filePath}`), + createPrettyPatch: vi.fn(() => "mock-diff"), + }, +})) + +vi.mock("../../diff/stats", () => ({ + sanitizeUnifiedDiff: vi.fn((diff: string) => diff), + computeDiffStats: vi.fn(() => ({ additions: 1, deletions: 1 })), +})) + +vi.mock("vscode", () => ({ + window: { showWarningMessage: vi.fn() }, + env: { openExternal: vi.fn() }, + Uri: { parse: vi.fn() }, +})) + +type FailPart = Extract + +describe("ApplyDiffTool failure reporting", () => { + const testFilePath = "test/file.txt" + + let mockTask: any + let mockAskApproval: ReturnType + let mockHandleError: ReturnType + let pushedResult: ToolResponse | undefined + + beforeEach(() => { + vi.clearAllMocks() + pushedResult = undefined + ;(fileExistsAtPath as any).mockResolvedValue(true) + ;(fs.readFile as any).mockResolvedValue("line one\nline two\nline three\n") + + mockAskApproval = vi.fn().mockResolvedValue(true) + mockHandleError = vi.fn().mockResolvedValue(undefined) + + mockTask = { + cwd: "/workspace", + consecutiveMistakeCount: 0, + consecutiveMistakeCountForApplyDiff: new Map(), + didEditFile: false, + api: { getModel: () => ({ id: "claude-test" }) }, + rooIgnoreController: { validateAccess: vi.fn().mockReturnValue(true) }, + rooProtectedController: { isWriteProtected: vi.fn().mockReturnValue(false) }, + providerRef: { + deref: vi.fn().mockReturnValue({ + getState: vi.fn().mockResolvedValue({ + diagnosticsEnabled: true, + writeDelayMs: 0, + experiments: {}, + }), + }), + }, + diffViewProvider: { + editType: undefined, + originalContent: "", + open: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + reset: vi.fn().mockResolvedValue(undefined), + revertChanges: vi.fn().mockResolvedValue(undefined), + saveChanges: vi.fn().mockResolvedValue(undefined), + saveDirectly: vi.fn().mockResolvedValue(undefined), + scrollToFirstDiff: vi.fn(), + pushToolWriteResult: vi.fn().mockResolvedValue("Changes successfully applied."), + }, + fileContextTracker: { trackFileContext: vi.fn().mockResolvedValue(undefined) }, + say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue(undefined), + recordToolError: vi.fn(), + recordToolUsage: vi.fn(), + processQueuedMessages: vi.fn(), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing param error"), + diffStrategy: undefined, + } + }) + + function makeFailure(errorText: string | undefined, details?: unknown): FailPart { + return { success: false, error: errorText, details } as unknown as FailPart + } + + function buildDiffContent(blockCount: number): string { + const blocks: string[] = [] + for (let blockIndex = 0; blockIndex < blockCount; blockIndex++) { + blocks.push(`<<<<<<< SEARCH\nsearch-${blockIndex}\n=======\nreplace-${blockIndex}\n>>>>>>> REPLACE`) + } + return blocks.join("\n") + } + + async function runApplyDiff(diffResult: DiffResult, blockCount: number): Promise { + mockTask.diffStrategy = { + applyDiff: vi.fn().mockResolvedValue(diffResult), + getProgressStatus: vi.fn().mockReturnValue({ icon: "diff-multiple", text: "x" }), + } + + await applyDiffTool.execute({ path: testFilePath, diff: buildDiffContent(blockCount) }, mockTask, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: vi.fn((result: ToolResponse) => { + pushedResult = result + }), + }) + + return String(pushedResult ?? "") + } + + function makeFailureSet(count: number): FailPart[] { + const failures: FailPart[] = [] + for (let failureIndex = 1; failureIndex <= count; failureIndex++) { + failures.push(makeFailure(`FAILURE-MARKER-${failureIndex}\nsecond line of failure ${failureIndex}`)) + } + return failures + } + + describe("failure accumulation and rendering", () => { + it("reports every failed block, not just the last one", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(2) }, 2) + + expect(output).toContain("FAILURE-MARKER-1") + expect(output).toContain("FAILURE-MARKER-2") + expect(output).toContain("--- Diff block 1 of 2 ---") + expect(output).toContain("--- Diff block 2 of 2 ---") + }) + + it("renders exactly five failures in full detail with no abbreviation trailer", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(5) }, 5) + + for (let failureIndex = 1; failureIndex <= 5; failureIndex++) { + expect(output).toContain(`FAILURE-MARKER-${failureIndex}`) + expect(output).toContain(`second line of failure ${failureIndex}`) + } + expect(output).not.toContain("are shown as a single summary line each") + }) + + it("abbreviates the sixth failure and emits a trailer naming the range", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(6) }, 6) + + expect(output).toContain("FAILURE-MARKER-6") + expect(output).not.toContain("second line of failure 6") + expect(output).toContain("second line of failure 5") + expect(output).toContain("(Failures 6-6 are shown as a single summary line each") + }) + + it("omits the diff block label when there is a single failure", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(1) }, 1) + + expect(output).toContain("FAILURE-MARKER-1") + expect(output).not.toContain("--- Diff block") + }) + + it("produces no stray separators for an empty failures array", async () => { + const output = await runApplyDiff({ success: false, failParts: [], error: "nothing applied" }, 1) + + expect(output).not.toContain("--- Diff block") + expect(output).not.toContain("are shown as a single summary line each") + }) + + it("substitutes a placeholder when a failure has no error text", async () => { + const output = await runApplyDiff({ success: false, failParts: [makeFailure(undefined)] }, 1) + + expect(output).toContain("Unknown apply_diff failure") + }) + + it("appends Details for detailed failures and omits them for abbreviated ones", async () => { + const failures = makeFailureSet(6) + failures[0] = makeFailure("FAILURE-MARKER-1\nsecond line of failure 1", { similarity: "DETAIL-FIRST" }) + failures[5] = makeFailure("FAILURE-MARKER-6\nsecond line of failure 6", { similarity: "DETAIL-SIXTH" }) + + const output = await runApplyDiff({ success: false, failParts: failures }, 6) + + expect(output).toContain("Details:") + expect(output).toContain("DETAIL-FIRST") + expect(output).not.toContain("DETAIL-SIXTH") + }) + + it("truncates a detailed failure that exceeds the character cap", async () => { + const oversizedError = `FAILURE-MARKER-1\n${"x".repeat(6000)}` + const output = await runApplyDiff({ success: false, failParts: [makeFailure(oversizedError)] }, 1) + + expect(output).toContain("characters omitted — retry this block alone for full detail") + expect(output).not.toContain("x".repeat(5000)) + }) + + it("leaves a detailed failure just under the character cap untruncated", async () => { + const nearCapError = `FAILURE-MARKER-1\n${"x".repeat(3900)}` + const output = await runApplyDiff({ success: false, failParts: [makeFailure(nearCapError)] }, 1) + + expect(output).not.toContain("[truncated") + expect(output).toContain("x".repeat(3900)) + }) + + it("truncates an abbreviated one-line summary that exceeds the summary cap", async () => { + const failures = makeFailureSet(6) + failures[5] = makeFailure(`FAILURE-MARKER-6 ${"y".repeat(500)}\nsecond line of failure 6`) + + const output = await runApplyDiff({ success: false, failParts: failures }, 6) + + expect(output).not.toContain("y".repeat(400)) + expect(output).toContain("characters omitted — retry this block alone for full detail") + }) + + it("emits a real trailer range when eight failures are reported", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(8) }, 8) + + expect(output).toContain("(Failures 6-8 are shown as a single summary line each") + expect(output).toContain("FAILURE-MARKER-8") + expect(output).not.toContain("second line of failure 8") + }) + + it("caps the number of abbreviated failures and reports the remainder as a count", async () => { + const output = await runApplyDiff({ success: false, failParts: makeFailureSet(60) }, 60) + + expect(output).toContain("(Failures 6-50 are shown as a single summary line each") + expect(output).toContain("(+10 more failures not shown") + expect(output).not.toContain("FAILURE-MARKER-51") + }) + }) + + describe("partial apply path", () => { + function partialResult(failures: FailPart[]): DiffResult { + return { success: true, content: "updated content", failParts: failures } as DiffResult + } + + it("reports how many blocks applied and asks for only the failed blocks to be resent", async () => { + const output = await runApplyDiff(partialResult(makeFailureSet(1)), 2) + + expect(output).toContain("Applied 1 of 2") + expect(output).toContain("FAILURE-MARKER-1") + expect(output).toContain("resend ONLY the blocks that failed") + }) + + it("does not tell the model to use read_file to re-apply the diffs", async () => { + const output = await runApplyDiff(partialResult(makeFailureSet(1)), 2) + + expect(output).not.toContain("re-apply") + }) + + it("applies the abbreviation cap on the partial path as well", async () => { + const output = await runApplyDiff(partialResult(makeFailureSet(6)), 8) + + expect(output).toContain("Applied 2 of 8") + expect(output).toContain("(Failures 6-6 are shown as a single summary line each") + expect(output).not.toContain("second line of failure 6") + }) + + it("emits no partial-apply hint when every block applies", async () => { + const output = await runApplyDiff({ success: true, content: "updated content", failParts: [] }, 1) + + expect(output).not.toContain("Applied") + expect(output).toContain("Changes successfully applied.") + expect(output).toContain("Making multiple related changes in a single apply_diff is more efficient") + }) + + it("counts and renders only the unsuccessful entries in failParts", async () => { + const mixedParts = [ + { success: true } as unknown as FailPart, + makeFailure("FAILURE-MARKER-1\nsecond line of failure 1"), + { success: true } as unknown as FailPart, + ] + const output = await runApplyDiff( + { success: true, content: "updated content", failParts: mixedParts } as DiffResult, + 3, + ) + + expect(output).toContain("Applied 2 of 3") + expect(output).toContain("FAILURE-MARKER-1") + expect(output).not.toContain("--- Diff block") + }) + }) +})