Skip to content

fix: make start_line and end_line parsing more lenient to accept brackets #11159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
roomote wants to merge 1 commit into
base: main
Choose a base branch
from
+133 −5
Draft

fix: make start_line and end_line parsing more lenient to accept brackets #11159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions .changeset/lenient-start-line-parsing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"roo-cline": patch
---

fix: make start_line and end_line parsing more lenient to accept brackets

Some models send `:start_line:[245]` or `:start_line: [245]` instead of `:start_line:245`. This change updates the regex to accept optional brackets around the line number, improving compatibility with models like MiniMax-2.1 and Gemini.

Supported formats:

- `:start_line:253`
- `:start_line: 253`
- `:start_line:[253]`
- `:start_line: [253]`

Fixes #11087
110 changes: 110 additions & 0 deletions src/core/diff/strategies/__tests__/multi-search-replace.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,30 @@ describe("MultiSearchReplaceDiffStrategy", () => {
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
})

it("validates line numbers with brackets (model variation)", () => {
const diff =
"<<<<<<< SEARCH\n" +
":start_line:[10]\n" +
"-------\n" +
"content1\n" +
"=======\n" +
"new1\n" +
">>>>>>> REPLACE"
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
})

it("validates line numbers with space and brackets (model variation)", () => {
const diff =
"<<<<<<< SEARCH\n" +
":start_line: [10]\n" +
"-------\n" +
"content1\n" +
"=======\n" +
"new1\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)
Expand Down Expand Up @@ -826,6 +850,92 @@ function five() {
}
})
})

describe("bracket variations in line numbers", () => {
let strategy: MultiSearchReplaceDiffStrategy

beforeEach(() => {
strategy = new MultiSearchReplaceDiffStrategy(1.0, 5)
})

it("should accept start_line with brackets around the number", async () => {
const originalContent = 'function hello() {\n console.log("hello")\n}\n'
const diffContent =
"test.ts\n" +
"<<<<<<< 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("should accept start_line with space and brackets around the number", async () => {
const originalContent = 'function hello() {\n console.log("hello")\n}\n'
const diffContent =
"test.ts\n" +
"<<<<<<< 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("should accept end_line with brackets around the number", async () => {
const originalContent = 'function hello() {\n console.log("hello")\n}\n'
const diffContent =
"test.ts\n" +
"<<<<<<< SEARCH\n" +
":start_line:[1]\n" +
":end_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("should accept end_line with space and brackets around the number", async () => {
const originalContent = 'function hello() {\n console.log("hello")\n}\n'
const diffContent =
"test.ts\n" +
"<<<<<<< SEARCH\n" +
":start_line: [1]\n" +
":end_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')
}
})
})
})

describe("fuzzy matching", () => {
Expand Down
12 changes: 7 additions & 5 deletions src/core/diff/strategies/multi-search-replace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,13 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
2. (?<!\\)<<<<<<< SEARCH\s*\n
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.
3. ((?:\:start_line:\s*\[?(\d+)\]?\s*\n))?
Optionally matches a ":start_line:" line with optional brackets around the number. The outer capturing group is group 1 and the inner (\d+) is group 2.
Accepts formats: :start_line:5, :start_line: 5, :start_line:[5], :start_line: [5]

4. ((?:\:end_line:\s*(\d+)\s*\n))?
Optionally matches a ":end_line:" line. Group 3 is the whole match and group 4 is the digits.
4. ((?:\:end_line:\s*\[?(\d+)\]?\s*\n))?
Optionally matches a ":end_line:" line with optional brackets around the number. Group 3 is the whole match and group 4 is the digits.
Accepts formats: :end_line:5, :end_line: 5, :end_line:[5], :end_line: [5]

5. ((?<!\\)-------\s*\n)?
Optionally matches the "-------" marker line (group 5).
Expand All @@ -289,7 +291,7 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {

let matches = [
...diffContent.matchAll(
/(?:^|\n)(?<!\\)<<<<<<< SEARCH>?\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?:\:end_line:\s*(\d+)\s*\n))?((?<!\\)-------\s*\n)?([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)=======\s*\n)([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)>>>>>>> REPLACE)(?=\n|$)/g,
/(?:^|\n)(?<!\\)<<<<<<< SEARCH>?\s*\n((?:\:start_line:\s*\[?(\d+)\]?\s*\n))?((?:\:end_line:\s*\[?(\d+)\]?\s*\n))?((?<!\\)-------\s*\n)?([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)=======\s*\n)([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)>>>>>>> REPLACE)(?=\n|$)/g,
),
]

Expand Down
Loading