scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-09-10 22:41:14 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: Improve error message when apply_diff structure is invalid (#2260)

Browse source 
Currently all errors in apply_diff diff structure are assumed to be
related to merge conflicts. This is unfortunate because it is way more
likely that LLM simply made an error structuring the diff. The LLM is
then directed to fix escaping in the diff from which it doesn't recover.

It is safe to assume that missing or extra "<<<<<<< SEARCH" or ">>>>>>>
REPLACE" strings are likely bad diff structure, because while "<<<<<<<"
and ">>>>>>>" are common merge conflict markers, no tools add "SEARCH"
and "REPLACE" strings after them. This is likely output of the LLM
itself.

LLM not recovering has been observed in Claude 3.5 Sonnet and Claude 3.7
Sonnet models.

To address the issue, error message now mentions merge conflict markers
and their escaping only if it's clear that errors may come from this
source. In the rest of cases the error message repeats the expected diff
structure back to the LLM.
This commit is contained in:
Povilas Kanapickas 2025-04-03 23:43:57 +03:00 • committed by GitHub
parent d2f9970654
commit 065c5ab222
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 84 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

41
src/core/diff/strategies/__tests__/multi-search-replace.test.ts
View file

@ -28,18 +28,57 @@ describe("MultiSearchReplaceDiffStrategy", () => {
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
})
it("validates multiple correct marker sequences with line numbers", () => {
const diff =
"<<<<<<< SEARCH\n" +
":start_line:10\n" +
":end_line:11\n" +
"-------\n" +
"content1\n" +
"=======\n" +
"new1\n" +
">>>>>>> REPLACE\n\n" +
"<<<<<<< SEARCH\n" +
":start_line:10\n" +
":end_line:11\n" +
"-------\n" +
"content2\n" +
"=======\n" +
"new2\n" +
">>>>>>> REPLACE"
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
})
it("detects separator before search", () => {
const diff = "=======\n" + "content\n" + ">>>>>>> REPLACE"
const result = strategy["validateMarkerSequencing"](diff)
expect(result.success).toBe(false)
expect(result.error).toContain("'=======' found in your diff content")
expect(result.error).toContain("Diff block is malformed")
})
it("detects replace before separator", () => {
it("detects missing separator", () => {
const diff = "<<<<<<< SEARCH\n" + "content\n" + ">>>>>>> REPLACE"
const result = strategy["validateMarkerSequencing"](diff)
expect(result.success).toBe(false)
expect(result.error).toContain("'>>>>>>> REPLACE' found in your diff content")
expect(result.error).toContain("Diff block is malformed")
})
it("detects two separators", () => {
const diff = "<<<<<<< SEARCH\n" + "content\n" + "=======\n" + "=======\n" + ">>>>>>> REPLACE"
const result = strategy["validateMarkerSequencing"](diff)
expect(result.success).toBe(false)
expect(result.error).toContain("'=======' found in your diff content")
expect(result.error).toContain("When removing merge conflict markers")
})
it("detects replace before separator (merge conflict message)", () => {
const diff = "<<<<<<< SEARCH\n" + "content\n" + ">>>>>>>"
const result = strategy["validateMarkerSequencing"](diff)
expect(result.success).toBe(false)
expect(result.error).toContain("'>>>>>>>' found in your diff content")
expect(result.error).toContain("When removing merge conflict markers")
})
it("detects incomplete sequence", () => {

51
src/core/diff/strategies/multi-search-replace.ts
View file

@ -162,8 +162,10 @@ Only use a single line of '=======' between search and replacement content, beca
const SEARCH = "<<<<<<< SEARCH"
const SEP = "======="
const REPLACE = ">>>>>>> REPLACE"
const SEARCH_PREFIX = "<<<<<<<"
const REPLACE_PREFIX = ">>>>>>>"
const reportError = (found: string, expected: string) => ({
const reportMergeConflictError = (found: string, expected: string) => ({
success: false,
error:
`ERROR: Special marker '${found}' found in your diff content at line ${state.line}:\n` +
@ -187,27 +189,62 @@ Only use a single line of '=======' between search and replacement content, beca
`\\${REPLACE}\n`,
})
const reportInvalidDiffError = (found: string, expected: string) => ({
success: false,
error:
`ERROR: Diff block is malformed: marker '${found}' found in your diff content at line ${state.line}. Expected: ${expected}\n` +
"\n" +
"CORRECT FORMAT:\n\n" +
"<<<<<<< SEARCH\n" +
":start_line: (required) The line number of original content where the search block starts.\n" +
":end_line: (required) The line number of original content where the search block ends.\n" +
"-------\n" +
"[exact content to find including whitespace]\n" +
"=======\n" +
"[new content to replace with]\n" +
">>>>>>> REPLACE\n",
})
const lines = diffContent.split("\n")
const searchCount = lines.filter((l) => l.trim() === SEARCH).length
const sepCount = lines.filter((l) => l.trim() === SEP).length
const replaceCount = lines.filter((l) => l.trim() === REPLACE).length
const likelyBadStructure = searchCount !== replaceCount || sepCount < searchCount
for (const line of diffContent.split("\n")) {
state.line++
const marker = line.trim()
switch (state.current) {
case State.START:
if (marker === SEP) return reportError(SEP, SEARCH)
if (marker === REPLACE) return reportError(REPLACE, SEARCH)
if (marker === SEP)
return likelyBadStructure
? reportInvalidDiffError(SEP, SEARCH)
: reportMergeConflictError(SEP, SEARCH)
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEARCH)
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
if (marker === SEARCH) state.current = State.AFTER_SEARCH
else if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
break
case State.AFTER_SEARCH:
if (marker === SEARCH) return reportError(SEARCH, SEP)
if (marker === REPLACE) return reportError(REPLACE, SEP)
if (marker === SEARCH) return reportInvalidDiffError(SEARCH, SEP)
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP)
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
if (marker === SEP) state.current = State.AFTER_SEPARATOR
break
case State.AFTER_SEPARATOR:
if (marker === SEARCH) return reportError(SEARCH, REPLACE)
if (marker === SEP) return reportError(SEP, REPLACE)
if (marker === SEARCH) return reportInvalidDiffError(SEARCH, REPLACE)
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE)
if (marker === SEP)
return likelyBadStructure
? reportInvalidDiffError(SEP, REPLACE)
: reportMergeConflictError(SEP, REPLACE)
if (marker === REPLACE) state.current = State.START
else if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, REPLACE)
break
}
}