Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-apply-diff-malformed-separator.md
Original file line number Diff line number Diff line change
@@ -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.
76 changes: 76 additions & 0 deletions src/core/diff/strategies/__tests__/multi-search-replace.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
64 changes: 64 additions & 0 deletions src/core/diff/strategies/multi-search-replace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 === "") {
Expand Down Expand Up @@ -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` +
` <search content here>\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)) ||
Expand Down
Loading