From 6fff6d94835b7d7ffc1ef2236013059cabd02769 Mon Sep 17 00:00:00 2001 From: SAY-5 Date: Tue, 28 Apr 2026 04:54:03 -0700 Subject: [PATCH 1/2] fix(apply_diff): detect malformed `-------` separator and surface helpful error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #12210. When a smaller model emits an apply_diff payload where the `-------` separator is missing the trailing newline, the outer regex's optional separator group quietly absorbs the run-on line into the search content. Match then fails with a confusing "63% similar (needs 100%)" error and the model loops. Add `detectMalformedSeparator` that runs against each replacement's search content right after marker unescaping. It flags first lines that start with seven or more dashes followed by non-dash, non-whitespace content on the same line. Markdown HRs and bare separators are NOT flagged. When detected, return an actionable error naming the exact problem and showing the correct shape so the model can self-correct instead of looping. Three regression tests under `malformed \`-------\` separator detection`: - exact reproduction from the issue → "Malformed separator" error surfaced; "63% similar" message no longer appears - well-formed separator still applies cleanly - search content with a long row of dashes inside the file is NOT misclassified --- .../__tests__/multi-search-replace.spec.ts | 76 +++++++++++++++++++ .../diff/strategies/multi-search-replace.ts | 64 ++++++++++++++++ 2 files changed, 140 insertions(+) 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)) || From c28ec2b5260ab60918dc55d2ebc8b950251ecf36 Mon Sep 17 00:00:00 2001 From: SAY-5 Date: Tue, 28 Apr 2026 04:56:18 -0700 Subject: [PATCH 2/2] chore: add changeset for #12210 fix --- .changeset/fix-apply-diff-malformed-separator.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix-apply-diff-malformed-separator.md 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.