diff --git a/.github/workflows/code-qa.yml b/.github/workflows/code-qa.yml index 0a5f3c1..de3f054 100644 --- a/.github/workflows/code-qa.yml +++ b/.github/workflows/code-qa.yml @@ -42,4 +42,4 @@ jobs: run: npm run install:all - name: Run unit tests - run: npx jest \ No newline at end of file + run: npm test \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 76fdaf7..85023ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,9 @@ # Roo Cline Changelog -## [2.2.7] +## [2.2.6 - 2.2.10] - More fixes to search/replace diffs -## [2.2.6] - -- Add a fuzzy match tolerance when applying diffs - ## [2.2.5] - Allow MCP servers to be enabled/disabled diff --git a/README.md b/README.md index e7e0b78..63a5006 100644 --- a/README.md +++ b/README.md @@ -1,18 +1,19 @@ # Roo-Cline -A fork of Cline, an autonomous coding agent, optimized for speed and flexibility. -- Auto-approval capabilities for commands, write, and browser operations -- Support for .clinerules per-project custom instructions -- Ability to run side-by-side with Cline -- Unit test coverage (written almost entirely by Roo Cline!) -- Support for playing sound effects -- Support for OpenRouter compression -- Support for copying prompts from the history screen -- Support for editing through diffs / handling truncated full-file edits -- Support for newer Gemini models (gemini-exp-1206 and gemini-2.0-flash-exp) -- Support for dragging and dropping images into chats -- Support for auto-approving MCP tools -- Support for enabling/disabling MCP servers +A fork of Cline, an autonomous coding agent, tweaked for more speed and flexibility. It’s been mainly writing itself recently, with a light touch of human guidance here and there. + +## Features + +- Automatically approve commands, browsing, file writing, and MCP tools +- Faster, more targeted edits via diffs (even on big files) +- Detects and fixes missing code chunks +- `.clinerules` for project-specific instructions +- Drag and drop images into chats +- Sound effects for feedback +- Quick prompt copying from history +- OpenRouter compression support +- Support for newer Gemini models (gemini-exp-1206, gemini-2.0-flash-exp) +- Runs alongside the original Cline ## Disclaimer diff --git a/package-lock.json b/package-lock.json index 6711dce..a07ea9c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.2.7", + "version": "2.2.10", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.2.7", + "version": "2.2.10", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index bd0c639..e05bb84 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.2.7", + "version": "2.2.10", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", @@ -157,7 +157,7 @@ "package": "npm run build:webview && npm run check-types && npm run lint && node esbuild.js --production", "pretest": "npm run compile-tests && npm run compile && npm run lint", "start:webview": "cd webview-ui && npm run start", - "test": "jest", + "test": "jest && npm run test:webview", "test:webview": "cd webview-ui && npm run test", "prepare": "husky", "publish:marketplace": "vsce publish", diff --git a/src/core/Cline.ts b/src/core/Cline.ts index cb27fe1..5b190d7 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -1237,20 +1237,25 @@ export class Cline { const originalContent = await fs.readFile(absolutePath, "utf-8") // Apply the diff to the original content - let newContent = this.diffStrategy?.applyDiff(originalContent, diffContent) ?? false - if (newContent === false) { + const diffResult = this.diffStrategy?.applyDiff(originalContent, diffContent) ?? { + success: false, + error: "No diff strategy available" + } + if (!diffResult.success) { this.consecutiveMistakeCount++ - await this.say("error", `Unable to apply diff to file - contents are out of sync: ${absolutePath}`) - pushToolResult(`Error applying diff to file: ${absolutePath} - contents are out of sync. Try re-reading the relevant lines of the file and applying the diff again.`) + const errorDetails = diffResult.details ? `\n\nDetails:\n${JSON.stringify(diffResult.details, null, 2)}` : '' + await this.say("error", `Unable to apply diff to file: ${absolutePath}\n${diffResult.error}${errorDetails}`) + pushToolResult(`Error applying diff to file: ${absolutePath}\n${diffResult.error}${errorDetails}`) break } + const newContent = diffResult.content this.consecutiveMistakeCount = 0 // Show diff view before asking for approval this.diffViewProvider.editType = "modify" await this.diffViewProvider.open(relPath); - await this.diffViewProvider.update(newContent, true); + await this.diffViewProvider.update(diffResult.content, true); await this.diffViewProvider.scrollToFirstDiff(); const completeMessage = JSON.stringify({ diff --git a/src/core/diff/strategies/__tests__/search-replace.test.ts b/src/core/diff/strategies/__tests__/search-replace.test.ts index ee2174a..8e48680 100644 --- a/src/core/diff/strategies/__tests__/search-replace.test.ts +++ b/src/core/diff/strategies/__tests__/search-replace.test.ts @@ -22,7 +22,10 @@ function hello() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe('function hello() {\n console.log("hello world")\n}\n') + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('function hello() {\n console.log("hello world")\n}\n') + } }) it('should match content with different surrounding whitespace', () => { @@ -39,7 +42,10 @@ function example() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe('\nfunction example() {\n return 43;\n}\n\n') + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('\nfunction example() {\n return 43;\n}\n\n') + } }) it('should match content with different indentation in search block', () => { @@ -56,7 +62,10 @@ function test() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(' function test() {\n return false;\n }\n') + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(' function test() {\n return false;\n }\n') + } }) it('should handle tab-based indentation', () => { @@ -73,7 +82,10 @@ function test() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe("function test() {\n\treturn false;\n}\n") + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe("function test() {\n\treturn false;\n}\n") + } }) it('should preserve mixed tabs and spaces', () => { @@ -94,7 +106,10 @@ function test() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe("\tclass Example {\n\t constructor() {\n\t\tthis.value = 1;\n\t }\n\t}") + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe("\tclass Example {\n\t constructor() {\n\t\tthis.value = 1;\n\t }\n\t}") + } }) it('should handle additional indentation with tabs', () => { @@ -112,7 +127,10 @@ function test() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe("\tfunction test() {\n\t\t// Add comment\n\t\treturn false;\n\t}") + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe("\tfunction test() {\n\t\t// Add comment\n\t\treturn false;\n\t}") + } }) it('should preserve exact indentation characters when adding lines', () => { @@ -131,7 +149,10 @@ function test() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe("\tfunction test() {\n\t\t// First comment\n\t\t// Second comment\n\t\treturn true;\n\t}") + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe("\tfunction test() {\n\t\t// First comment\n\t\t// Second comment\n\t\treturn true;\n\t}") + } }) it('should handle Windows-style CRLF line endings', () => { @@ -148,7 +169,10 @@ function test() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe("function test() {\r\n return false;\r\n}\r\n") + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe("function test() {\r\n return false;\r\n}\r\n") + } }) it('should return false if search content does not match', () => { @@ -165,7 +189,7 @@ function hello() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(false) + expect(result.success).toBe(false) }) it('should return false if diff format is invalid', () => { @@ -173,7 +197,7 @@ function hello() { const diffContent = `test.ts\nInvalid diff format` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(false) + expect(result.success).toBe(false) }) it('should handle multiple lines with proper indentation', () => { @@ -192,7 +216,10 @@ function hello() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe('class Example {\n constructor() {\n this.value = 0\n }\n\n getValue() {\n // Add logging\n console.log("Getting value")\n return this.value\n }\n}\n') + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('class Example {\n constructor() {\n this.value = 0\n }\n\n getValue() {\n // Add logging\n console.log("Getting value")\n return this.value\n }\n}\n') + } }) it('should preserve whitespace exactly in the output', () => { @@ -209,7 +236,10 @@ function hello() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(" modified\n still indented\n end\n") + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(" modified\n still indented\n end\n") + } }) it('should preserve indentation when adding new lines after existing content', () => { @@ -226,10 +256,461 @@ function hello() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(' onScroll={() => updateHighlights()}\n onDragOver={(e) => {\n e.preventDefault()\n e.stopPropagation()\n }}') + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(' onScroll={() => updateHighlights()}\n onDragOver={(e) => {\n e.preventDefault()\n e.stopPropagation()\n }}') + } }) + + it('should handle varying indentation levels correctly', () => { + const originalContent = ` +class Example { + constructor() { + this.value = 0; + if (true) { + this.init(); + } + } +}`.trim(); + + const diffContent = `test.ts +<<<<<<< SEARCH + class Example { + constructor() { + this.value = 0; + if (true) { + this.init(); + } + } + } +======= + class Example { + constructor() { + this.value = 1; + if (true) { + this.init(); + this.setup(); + this.validate(); + } + } + } +>>>>>>> REPLACE`.trim(); + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(` +class Example { + constructor() { + this.value = 1; + if (true) { + this.init(); + this.setup(); + this.validate(); + } + } +}`.trim()); + } + }) + + it('should handle mixed indentation styles in the same file', () => { + const originalContent = `class Example { + constructor() { + this.value = 0; + if (true) { + this.init(); + } + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + constructor() { + this.value = 0; + if (true) { + this.init(); + } + } +======= + constructor() { + this.value = 1; + if (true) { + this.init(); + this.validate(); + } + } +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`class Example { + constructor() { + this.value = 1; + if (true) { + this.init(); + this.validate(); + } + } +}`); + } + }) + + it('should handle Python-style significant whitespace', () => { + const originalContent = `def example(): + if condition: + do_something() + for item in items: + process(item) + return True`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + if condition: + do_something() + for item in items: + process(item) +======= + if condition: + do_something() + while items: + item = items.pop() + process(item) +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`def example(): + if condition: + do_something() + while items: + item = items.pop() + process(item) + return True`); + } + }); + + it('should preserve empty lines with indentation', () => { + const originalContent = `function test() { + const x = 1; + + if (x) { + return true; + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + const x = 1; + + if (x) { +======= + const x = 1; + + // Check x + if (x) { +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function test() { + const x = 1; + + // Check x + if (x) { + return true; + } +}`); + } + }); + + it('should handle indentation when replacing entire blocks', () => { + const originalContent = `class Test { + method() { + if (true) { + console.log("test"); + } + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + method() { + if (true) { + console.log("test"); + } + } +======= + method() { + try { + if (true) { + console.log("test"); + } + } catch (e) { + console.error(e); + } + } +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`class Test { + method() { + try { + if (true) { + console.log("test"); + } + } catch (e) { + console.error(e); + } + } +}`); + } + }); + + it('should handle negative indentation relative to search content', () => { + const originalContent = `class Example { + constructor() { + if (true) { + this.init(); + this.setup(); + } + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + this.init(); + this.setup(); +======= + this.init(); + this.setup(); +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`class Example { + constructor() { + if (true) { + this.init(); + this.setup(); + } + } +}`); + } + }); + + it('should handle extreme negative indentation (no indent)', () => { + const originalContent = `class Example { + constructor() { + if (true) { + this.init(); + } + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + this.init(); +======= +this.init(); +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`class Example { + constructor() { + if (true) { +this.init(); + } + } +}`); + } + }); + + it('should handle mixed indentation changes in replace block', () => { + const originalContent = `class Example { + constructor() { + if (true) { + this.init(); + this.setup(); + this.validate(); + } + } +}`.trim(); + const diffContent = `test.ts +<<<<<<< SEARCH + this.init(); + this.setup(); + this.validate(); +======= + this.init(); + this.setup(); + this.validate(); +>>>>>>> REPLACE`; + + const result = strategy.applyDiff(originalContent, diffContent); + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`class Example { + constructor() { + if (true) { + this.init(); + this.setup(); + this.validate(); + } + } +}`); + } + }); }) + describe('line number stripping', () => { + describe('line number stripping', () => { + let strategy: SearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new SearchReplaceDiffStrategy() + }) + + it('should strip line numbers from both search and replace sections', () => { + const originalContent = 'function test() {\n return true;\n}\n' + const diffContent = `test.ts +<<<<<<< SEARCH +1 | function test() { +2 | return true; +3 | } +======= +1 | function test() { +2 | return false; +3 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('function test() {\n return false;\n}\n') + } + }) + + it('should not strip when not all lines have numbers in either section', () => { + const originalContent = 'function test() {\n return true;\n}\n' + const diffContent = `test.ts +<<<<<<< SEARCH +1 | function test() { +2 | return true; +3 | } +======= +1 | function test() { + return false; +3 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(false) + }) + + it('should preserve content that naturally starts with pipe', () => { + const originalContent = '|header|another|\n|---|---|\n|data|more|\n' + const diffContent = `test.ts +<<<<<<< SEARCH +1 | |header|another| +2 | |---|---| +3 | |data|more| +======= +1 | |header|another| +2 | |---|---| +3 | |data|updated| +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('|header|another|\n|---|---|\n|data|updated|\n') + } + }) + + it('should preserve indentation when stripping line numbers', () => { + const originalContent = ' function test() {\n return true;\n }\n' + const diffContent = `test.ts +<<<<<<< SEARCH +1 | function test() { +2 | return true; +3 | } +======= +1 | function test() { +2 | return false; +3 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(' function test() {\n return false;\n }\n') + } + }) + + it('should handle different line numbers between sections', () => { + const originalContent = 'function test() {\n return true;\n}\n' + const diffContent = `test.ts +<<<<<<< SEARCH +10 | function test() { +11 | return true; +12 | } +======= +20 | function test() { +21 | return false; +22 | } +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('function test() {\n return false;\n}\n') + } + }) + + it('should not strip content that starts with pipe but no line number', () => { + const originalContent = '| Pipe\n|---|\n| Data\n' + const diffContent = `test.ts +<<<<<<< SEARCH +| Pipe +|---| +| Data +======= +| Pipe +|---| +| Updated +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('| Pipe\n|---|\n| Updated\n') + } + }) + + it('should handle mix of line-numbered and pipe-only content', () => { + const originalContent = '| Pipe\n|---|\n| Data\n' + const diffContent = `test.ts +<<<<<<< SEARCH +| Pipe +|---| +| Data +======= +1 | | Pipe +2 | |---| +3 | | NewData +>>>>>>> REPLACE` + + const result = strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('1 | | Pipe\n2 | |---|\n3 | | NewData\n') + } + }) + }) + }); + describe('fuzzy matching', () => { let strategy: SearchReplaceDiffStrategy @@ -253,7 +734,10 @@ function getData() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe('function getData() {\n const data = fetchData();\n return data.filter(Boolean);\n}\n') + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('function getData() {\n const data = fetchData();\n return data.filter(Boolean);\n}\n') + } }) it('should not match when content is too different (<90% similar)', () => { @@ -270,7 +754,7 @@ function processData(data) { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(false) + expect(result.success).toBe(false) }) it('should match content with extra whitespace', () => { @@ -287,7 +771,10 @@ function sum(a, b) { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe('function sum(a, b) {\n return a + b + 1;\n}') + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe('function sum(a, b) {\n return a + b + 1;\n}') + } }) }) @@ -324,7 +811,9 @@ function two() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent, 5, 7) - expect(result).toBe(`function one() { + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function one() { return 1; } @@ -335,6 +824,7 @@ function two() { function three() { return 3; }`) + } }) it('should find and replace within buffer zone (5 lines before/after)', () => { @@ -365,7 +855,9 @@ function three() { // Even though we specify lines 5-7, it should still find the match at lines 9-11 // because it's within the 5-line buffer zone const result = strategy.applyDiff(originalContent, diffContent, 5, 7) - expect(result).toBe(`function one() { + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function one() { return 1; } @@ -376,6 +868,7 @@ function two() { function three() { return "three"; }`) + } }) it('should not find matches outside search range and buffer zone', () => { @@ -414,7 +907,7 @@ function five() { // Searching around function two() (lines 5-7) // function five() is more than 5 lines away, so it shouldn't match const result = strategy.applyDiff(originalContent, diffContent, 5, 7) - expect(result).toBe(false) + expect(result.success).toBe(false) }) it('should handle search range at start of file', () => { @@ -439,13 +932,16 @@ function one() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent, 1, 3) - expect(result).toBe(`function one() { + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function one() { return "one"; } function two() { return 2; }`) + } }) it('should handle search range at end of file', () => { @@ -470,13 +966,16 @@ function two() { >>>>>>> REPLACE` const result = strategy.applyDiff(originalContent, diffContent, 5, 7) - expect(result).toBe(`function one() { + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function one() { return 1; } function two() { return "two"; }`) + } }) it('should match specific instance of duplicate code using line numbers', () => { @@ -513,7 +1012,9 @@ function processData(data) { // Target the second instance of processData const result = strategy.applyDiff(originalContent, diffContent, 10, 12) - expect(result).toBe(`function processData(data) { + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function processData(data) { return data.map(x => x * 2); } @@ -531,6 +1032,7 @@ function processData(data) { function moreStuff() { console.log("world"); }`) + } }) it('should search from start line to end of file when only start_line is provided', () => { @@ -560,7 +1062,9 @@ function three() { // Only provide start_line, should search from there to end of file const result = strategy.applyDiff(originalContent, diffContent, 8) - expect(result).toBe(`function one() { + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function one() { return 1; } @@ -571,6 +1075,7 @@ function two() { function three() { return "three"; }`) + } }) it('should search from start of file to end line when only end_line is provided', () => { @@ -600,7 +1105,9 @@ function one() { // Only provide end_line, should search from start of file to there const result = strategy.applyDiff(originalContent, diffContent, undefined, 4) - expect(result).toBe(`function one() { + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function one() { return "one"; } @@ -611,6 +1118,102 @@ function two() { function three() { return 3; }`) + } + }) + + it('should prioritize exact line match over expanded search', () => { + const originalContent = ` +function one() { + return 1; +} + +function process() { + return "old"; +} + +function process() { + return "old"; +} + +function two() { + return 2; +}` + const diffContent = `test.ts +<<<<<<< SEARCH +function process() { + return "old"; +} +======= +function process() { + return "new"; +} +>>>>>>> REPLACE` + + // Should match the second instance exactly at lines 10-12 + // even though the first instance at 6-8 is within the expanded search range + const result = strategy.applyDiff(originalContent, diffContent, 10, 12) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(` +function one() { + return 1; +} + +function process() { + return "old"; +} + +function process() { + return "new"; +} + +function two() { + return 2; +}`) + } + }) + + it('should fall back to expanded search only if exact match fails', () => { + const originalContent = ` +function one() { + return 1; +} + +function process() { + return "target"; +} + +function two() { + return 2; +}`.trim() + const diffContent = `test.ts +<<<<<<< SEARCH +function process() { + return "target"; +} +======= +function process() { + return "updated"; +} +>>>>>>> REPLACE` + + // Specify wrong line numbers (3-5), but content exists at 6-8 + // Should still find and replace it since it's within the expanded range + const result = strategy.applyDiff(originalContent, diffContent, 3, 5) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(`function one() { + return 1; +} + +function process() { + return "updated"; +} + +function two() { + return 2; +}`) + } }) }) diff --git a/src/core/diff/strategies/__tests__/unified.test.ts b/src/core/diff/strategies/__tests__/unified.test.ts index 4e6c449..83a53b2 100644 --- a/src/core/diff/strategies/__tests__/unified.test.ts +++ b/src/core/diff/strategies/__tests__/unified.test.ts @@ -59,7 +59,10 @@ function calculateTotal(items: number[]): number { export { calculateTotal };` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(expected) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(expected) + } }) it('should successfully apply a diff adding a new method', () => { @@ -93,7 +96,10 @@ export { calculateTotal };` }` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(expected) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(expected) + } }) it('should successfully apply a diff modifying imports', () => { @@ -128,7 +134,10 @@ function App() { }` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(expected) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(expected) + } }) it('should successfully apply a diff with multiple hunks', () => { @@ -190,7 +199,10 @@ async function processFile(path: string) { export { processFile };` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(expected) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(expected) + } }) it('should handle empty original content', () => { @@ -207,7 +219,10 @@ export { processFile };` }\n` const result = strategy.applyDiff(originalContent, diffContent) - expect(result).toBe(expected) + expect(result.success).toBe(true) + if (result.success) { + expect(result.content).toBe(expected) + } }) }) }) diff --git a/src/core/diff/strategies/search-replace.ts b/src/core/diff/strategies/search-replace.ts index ee0272e..a950b08 100644 --- a/src/core/diff/strategies/search-replace.ts +++ b/src/core/diff/strategies/search-replace.ts @@ -1,4 +1,4 @@ -import { DiffStrategy } from "../types" +import { DiffStrategy, DiffResult } from "../types" function levenshteinDistance(a: string, b: string): number { const matrix: number[][] = []; @@ -115,58 +115,129 @@ Your search/replace content here ` } - applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): string | false { + applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): DiffResult { // Extract the search and replace blocks const match = diffContent.match(/<<<<<<< SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n>>>>>>> REPLACE/); if (!match) { - return false; + // Log detailed format information + console.log('Invalid Diff Format Debug:', { + expectedFormat: "<<<<<<< SEARCH\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE", + tip: "Make sure to include both SEARCH and REPLACE sections with correct markers" + }); + + return { + success: false, + error: "Invalid diff format - missing required SEARCH/REPLACE sections" + }; } - const [_, searchContent, replaceContent] = match; + let [_, searchContent, replaceContent] = match; // Detect line ending from original content const lineEnding = originalContent.includes('\r\n') ? '\r\n' : '\n'; + + // Strip line numbers from search and replace content if every line starts with a line number + const hasLineNumbers = (content: string) => { + const lines = content.split(/\r?\n/); + return lines.length > 0 && lines.every(line => /^\d+\s+\|(?!\|)/.test(line)); + }; + + if (hasLineNumbers(searchContent) && hasLineNumbers(replaceContent)) { + const stripLineNumbers = (content: string) => { + return content.replace(/^\d+\s+\|(?!\|)/gm, '') + }; + + searchContent = stripLineNumbers(searchContent); + replaceContent = stripLineNumbers(replaceContent); + } // Split content into lines, handling both \n and \r\n const searchLines = searchContent.split(/\r?\n/); const replaceLines = replaceContent.split(/\r?\n/); const originalLines = originalContent.split(/\r?\n/); - // Determine search range based on provided line numbers - let searchStartIndex = 0; - let searchEndIndex = originalLines.length; - - if (startLine !== undefined || endLine !== undefined) { - // Convert to 0-based index and add buffer - if (startLine !== undefined) { - searchStartIndex = Math.max(0, startLine - 6); - } - if (endLine !== undefined) { - searchEndIndex = Math.min(originalLines.length, endLine + 5); - } - } - - // Find the search content in the original using fuzzy matching + // First try exact line range if provided let matchIndex = -1; let bestMatchScore = 0; + let bestMatchContent = ""; - for (let i = searchStartIndex; i <= searchEndIndex - searchLines.length; i++) { - // Join the lines and calculate overall similarity - const originalChunk = originalLines.slice(i, i + searchLines.length).join('\n'); + if (startLine !== undefined && endLine !== undefined) { + // Convert to 0-based index + const exactStartIndex = startLine - 1; + const exactEndIndex = endLine - 1; + + if (exactStartIndex < 0 || exactEndIndex >= originalLines.length) { + // Log detailed debug information + console.log('Invalid Line Range Debug:', { + requestedRange: { start: startLine, end: endLine }, + fileBounds: { start: 1, end: originalLines.length } + }); + + return { + success: false, + error: `Line range ${startLine}-${endLine} is invalid (file has ${originalLines.length} lines)`, + }; + } + + // Check exact range first + const originalChunk = originalLines.slice(exactStartIndex, exactEndIndex + 1).join('\n'); const searchChunk = searchLines.join('\n'); const similarity = getSimilarity(originalChunk, searchChunk); - if (similarity > bestMatchScore) { + if (similarity >= this.fuzzyThreshold) { + matchIndex = exactStartIndex; bestMatchScore = similarity; - matchIndex = i; + bestMatchContent = originalChunk; } } - + + // If no match found in exact range, try expanded range + if (matchIndex === -1) { + let searchStartIndex = 0; + let searchEndIndex = originalLines.length; + + if (startLine !== undefined || endLine !== undefined) { + // Convert to 0-based index and add buffer + if (startLine !== undefined) { + searchStartIndex = Math.max(0, startLine - 6); + } + if (endLine !== undefined) { + searchEndIndex = Math.min(originalLines.length, endLine + 5); + } + } + + // Find the search content in the expanded range using fuzzy matching + for (let i = searchStartIndex; i <= searchEndIndex - searchLines.length; i++) { + // Join the lines and calculate overall similarity + const originalChunk = originalLines.slice(i, i + searchLines.length).join('\n'); + const searchChunk = searchLines.join('\n'); + + const similarity = getSimilarity(originalChunk, searchChunk); + if (similarity > bestMatchScore) { + bestMatchScore = similarity; + matchIndex = i; + bestMatchContent = originalChunk; + } + } + } + // Require similarity to meet threshold if (matchIndex === -1 || bestMatchScore < this.fuzzyThreshold) { - return false; + const searchChunk = searchLines.join('\n'); + // Log detailed debug information to console + console.log('Search/Replace Debug Info:', { + similarity: bestMatchScore, + threshold: this.fuzzyThreshold, + searchContent: searchChunk, + bestMatch: bestMatchContent || undefined + }); + + return { + success: false, + error: `No sufficiently similar match found${startLine !== undefined ? ` near lines ${startLine}-${endLine}` : ''} (${Math.round(bestMatchScore * 100)}% similar, needs ${Math.round(this.fuzzyThreshold * 100)}%)` + }; } - + // Get the matched lines from the original content const matchedLines = originalLines.slice(matchIndex, matchIndex + searchLines.length); @@ -175,13 +246,13 @@ Your search/replace content here const match = line.match(/^[\t ]*/); return match ? match[0] : ''; }); - + // Get the exact indentation of each line in the search block const searchIndents = searchLines.map(line => { const match = line.match(/^[\t ]*/); return match ? match[0] : ''; }); - + // Apply the replacement while preserving exact indentation const indentedReplaceLines = replaceLines.map((line, i) => { // Get the matched line's exact indentation @@ -192,17 +263,28 @@ Your search/replace content here const currentIndent = currentIndentMatch ? currentIndentMatch[0] : ''; const searchBaseIndent = searchIndents[0] || ''; - // Calculate the relative indentation from the search content - const relativeIndent = currentIndent.slice(searchBaseIndent.length); + // Calculate the relative indentation level + const searchBaseLevel = searchBaseIndent.length; + const currentLevel = currentIndent.length; + const relativeLevel = currentLevel - searchBaseLevel; - // Apply the matched indentation plus any relative indentation - return matchedIndent + relativeIndent + line.trim(); + // If relative level is negative, remove indentation from matched indent + // If positive, add to matched indent + const finalIndent = relativeLevel < 0 + ? matchedIndent.slice(0, Math.max(0, matchedIndent.length + relativeLevel)) + : matchedIndent + currentIndent.slice(searchBaseLevel); + + return finalIndent + line.trim(); }); - + // Construct the final content const beforeMatch = originalLines.slice(0, matchIndex); const afterMatch = originalLines.slice(matchIndex + searchLines.length); - return [...beforeMatch, ...indentedReplaceLines, ...afterMatch].join(lineEnding); + const finalContent = [...beforeMatch, ...indentedReplaceLines, ...afterMatch].join(lineEnding); + return { + success: true, + content: finalContent + }; } -} +} \ No newline at end of file diff --git a/src/core/diff/strategies/unified.ts b/src/core/diff/strategies/unified.ts index 9e95e09..2f80a61 100644 --- a/src/core/diff/strategies/unified.ts +++ b/src/core/diff/strategies/unified.ts @@ -1,5 +1,5 @@ import { applyPatch } from "diff" -import { DiffStrategy } from "../types" +import { DiffStrategy, DiffResult } from "../types" export class UnifiedDiffStrategy implements DiffStrategy { getToolDescription(cwd: string): string { @@ -108,7 +108,30 @@ Your diff here ` } - applyDiff(originalContent: string, diffContent: string): string | false { - return applyPatch(originalContent, diffContent) as string | false + applyDiff(originalContent: string, diffContent: string): DiffResult { + try { + const result = applyPatch(originalContent, diffContent) + if (result === false) { + return { + success: false, + error: "Failed to apply unified diff - patch rejected", + details: { + searchContent: diffContent + } + } + } + return { + success: true, + content: result + } + } catch (error) { + return { + success: false, + error: `Error applying unified diff: ${error.message}`, + details: { + searchContent: diffContent + } + } + } } } diff --git a/src/core/diff/types.ts b/src/core/diff/types.ts index e5d4478..3957a1f 100644 --- a/src/core/diff/types.ts +++ b/src/core/diff/types.ts @@ -1,6 +1,17 @@ /** * Interface for implementing different diff strategies */ + +export type DiffResult = + | { success: true; content: string } + | { success: false; error: string; details?: { + similarity?: number; + threshold?: number; + matchedRange?: { start: number; end: number }; + searchContent?: string; + bestMatch?: string; + }}; + export interface DiffStrategy { /** * Get the tool description for this diff strategy @@ -15,7 +26,7 @@ export interface DiffStrategy { * @param diffContent The diff content in the strategy's format * @param startLine Optional line number where the search block starts. If not provided, searches the entire file. * @param endLine Optional line number where the search block ends. If not provided, searches the entire file. - * @returns The new content after applying the diff, or false if the diff could not be applied + * @returns A DiffResult object containing either the successful result or error details */ - applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): string | false + applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): DiffResult } diff --git a/src/integrations/editor/__tests__/detect-omission.test.ts b/src/integrations/editor/__tests__/detect-omission.test.ts new file mode 100644 index 0000000..4740b1f --- /dev/null +++ b/src/integrations/editor/__tests__/detect-omission.test.ts @@ -0,0 +1,66 @@ +import { detectCodeOmission } from '../detect-omission' + +describe('detectCodeOmission', () => { + const originalContent = `function example() { + // Some code + const x = 1; + const y = 2; + return x + y; +}` + + it('should detect square bracket line range omission', () => { + const newContent = `[Previous content from line 1-305 remains exactly the same] +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect single-line comment omission', () => { + const newContent = `// Lines 1-50 remain unchanged +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect multi-line comment omission', () => { + const newContent = `/* Previous content remains the same */ +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect HTML-style comment omission', () => { + const newContent = ` +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect JSX-style comment omission', () => { + const newContent = `{/* Rest of the code remains the same */} +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should detect Python-style comment omission', () => { + const newContent = `# Previous content remains unchanged +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(true) + }) + + it('should not detect regular comments without omission keywords', () => { + const newContent = `// Adding new functionality +const z = 3;` + expect(detectCodeOmission(originalContent, newContent)).toBe(false) + }) + + it('should not detect when comment is part of original content', () => { + const originalWithComment = `// Content remains unchanged +${originalContent}` + const newContent = `// Content remains unchanged +const z = 3;` + expect(detectCodeOmission(originalWithComment, newContent)).toBe(false) + }) + + it('should not detect code that happens to contain omission keywords', () => { + const newContent = `const remains = 'some value'; +const unchanged = true;` + expect(detectCodeOmission(originalContent, newContent)).toBe(false) + }) +}) \ No newline at end of file diff --git a/src/integrations/editor/detect-omission.ts b/src/integrations/editor/detect-omission.ts index 565ebd3..5cb0f8e 100644 --- a/src/integrations/editor/detect-omission.ts +++ b/src/integrations/editor/detect-omission.ts @@ -7,7 +7,7 @@ export function detectCodeOmission(originalFileContent: string, newFileContent: string): boolean { const originalLines = originalFileContent.split("\n") const newLines = newFileContent.split("\n") - const omissionKeywords = ["remain", "remains", "unchanged", "rest", "previous", "existing", "..."] + const omissionKeywords = ["remain", "remains", "unchanged", "rest", "previous", "existing", "content", "same", "..."] const commentPatterns = [ /^\s*\/\//, // Single-line comment for most languages @@ -15,6 +15,7 @@ export function detectCodeOmission(originalFileContent: string, newFileContent: /^\s*\/\*/, // Multi-line comment opening /^\s*{\s*\/\*/, // JSX comment opening /^\s*