diff --git a/.changeset/fix-apply-diff-malformed-separator.md b/.changeset/fix-apply-diff-malformed-separator.md new file mode 100644 index 00000000000..29e3c704937 --- /dev/null +++ b/.changeset/fix-apply-diff-malformed-separator.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +apply_diff: detect a malformed `-------` separator in the SEARCH section (e.g. `-------import { ... }` with no trailing newline) and surface a clear "Malformed separator" error instead of the generic "63% similar" mismatch. Smaller models that occasionally emit this shape can now self-correct on the next attempt instead of looping. Closes #12210. diff --git a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts index f06f3f406fb..23ef70f4d5a 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts @@ -1204,4 +1204,80 @@ function sum(a, b) { expect(result.error).toContain(":start_line:5 <-- Invalid location") }) }) + + // Regression for https://github.com/RooCodeInc/Roo-Code/issues/12210. + describe("malformed `-------` separator detection", () => { + let strategy: MultiSearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new MultiSearchReplaceDiffStrategy(1.0, 5) + }) + + it("returns a 'malformed separator' error when LLM omits the newline after -------", async () => { + const originalContent = + "import { useTranslate } from '../../i18n/I18nContext';\n" + + "\n" + + "type MouseMode = 'draw' | 'erase';\n" + const diffContent = + "<<<<<<< SEARCH\n" + + ":start_line:1\n" + + "-------import { useTranslate } from '../../i18n/I18nContext';\n" + + "\n" + + "type MouseMode\n" + + "=======\n" + + "import { useTranslate } from '../../i18n/I18nContext';\n" + + "import { MaskEditorProvider, useMaskEditor } from './MaskEditorContext';\n" + + "\n" + + "type MouseMode\n" + + ">>>>>>> REPLACE" + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(false) + if (!result.success) { + const parts = result.failParts ?? [] + const errors = parts + .filter((part) => part.success === false) + .map((part) => ("error" in part ? (part.error ?? "") : "")) + .concat("error" in result ? (result.error ?? "") : "") + .join(" ") + expect(errors).toContain("Malformed separator") + expect(errors).toContain("must be on its own line") + expect(errors).not.toContain("63%") + expect(errors).not.toContain("similar") + } + }) + + it("does not flag a well-formed -------\\n separator", async () => { + const originalContent = "function hello() {\n console.log('hello')\n}\n" + const diffContent = + "<<<<<<< SEARCH\n" + + ":start_line:1\n" + + "-------\n" + + "function hello() {\n" + + "=======\n" + + "function helloWorld() {\n" + + ">>>>>>> REPLACE" + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe("function helloWorld() {\n console.log('hello')\n}\n") + } + }) + + it("does not flag a search line that legitimately contains many dashes", async () => { + const originalContent = "/* ---------- header ---------- */\nconst x = 1;\n" + const diffContent = + "<<<<<<< SEARCH\n" + + ":start_line:1\n" + + "-------\n" + + "/* ---------- header ---------- */\n" + + "=======\n" + + "/* === header === */\n" + + ">>>>>>> REPLACE" + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + }) + }) }) diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index f43bbee0dc9..a1e69e80538 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -8,6 +8,45 @@ import { normalizeString } from "../../../utils/text-normalization" const BUFFER_LINES = 40 // Number of extra context lines to show before and after matches +/** + * Detect a malformed `-------` separator in the SEARCH section content. + * + * If the LLM forgets the newline after the separator, the run-on line + * (e.g. `-------import { ... }`) ends up as the first line of the + * captured search content. The outer regex doesn't reject this — it + * just falls through, and the file-content match fails with a + * confusing "63% similar" error. By detecting the malformed prefix + * here we can return an actionable error instead. + * + * Returns the offending line when malformed, or `null` otherwise. + * + * Regression test for https://github.com/RooCodeInc/Roo-Code/issues/12210. + */ +function detectMalformedSeparator(searchContent: string): string | null { + if (!searchContent) { + return null + } + const firstLine = searchContent.split(/\r?\n/, 1)[0] ?? "" + const trimmed = firstLine.trim() + // Must start with at least seven dashes (the separator) AND have + // non-dash, non-whitespace content immediately after them on the + // same line. A bare `-------` (separator on its own line that + // happened to land in the search content for unrelated reasons) is + // not flagged. + const match = trimmed.match(/^-{7,}(.+)$/) + if (!match) { + return null + } + const tail = match[1].trim() + // Allow lines that start with a literal `-` continuation (e.g. + // markdown bullets that begin with extra dashes) — they wouldn't + // look like the separator-then-code shape that confuses callers. + if (tail === "" || /^-+$/.test(tail)) { + return null + } + return firstLine +} + function getSimilarity(original: string, search: string): number { // Empty searches are no longer supported if (search === "") { @@ -321,6 +360,31 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy { searchContent = this.unescapeMarkers(searchContent) replaceContent = this.unescapeMarkers(replaceContent) + // Detect the specific malformation flagged in #12210: an + // unescaped `-------` separator that lacks the trailing + // newline (e.g. `-------import { ... }`). The outer regex + // makes the separator group optional, so the search content + // silently absorbs the run-on line and matching fails with + // a confusing "63% similar" error. Surface the actual root + // cause so the model can self-correct. + const malformedSeparator = detectMalformedSeparator(searchContent) + if (malformedSeparator) { + diffResults.push({ + success: false, + error: + `Malformed separator in SEARCH section\n\n` + + `Debug Info:\n` + + `- The "-------" separator that follows :start_line:/:end_line: must be on its own line, followed by a newline before the search content begins.\n` + + `- Got: ${JSON.stringify(malformedSeparator)}\n` + + `- Expected:\n` + + ` :start_line:7\n` + + ` -------\n` + + ` \n` + + `- Tip: insert a newline immediately after "-------". Do not put the first line of search content on the same line as the separator.`, + }) + continue + } + // Strip line numbers from search and replace content if every line starts with a line number const hasAllLineNumbers = (everyLineHasLineNumbers(searchContent) && everyLineHasLineNumbers(replaceContent)) ||