mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-24 00:51:34 +00:00
Merge 1efc0cb4c3 into e921f9d21e
This commit is contained in:
commit
921da4db3d
3 changed files with 146 additions and 1 deletions
5
.changeset/fix-apply-diff-malformed-separator.md
Normal file
5
.changeset/fix-apply-diff-malformed-separator.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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 === "") {
|
||||
|
|
@ -263,7 +302,7 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
|
|||
Ensures the first marker starts at the beginning of the file or right after a newline.
|
||||
|
||||
2. (?<!\\)<<<<<<< SEARCH\s*\n
|
||||
Matches the line "<<<<<<< SEARCH" (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped.
|
||||
Matches the line "<<<<<<< SEARCH" (ignoring any trailing spaces), the negative lookbehind makes sure it isn't escaped.
|
||||
|
||||
3. ((?:\:start_line:\s*(\d+)\s*\n))?
|
||||
Optionally matches a ":start_line:" line. The outer capturing group is group 1 and the inner (\d+) is group 2.
|
||||
|
|
@ -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)) ||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue