Merge pull request #115 from RooVetGit/search_replace_line_numbers

Improvements to search/replace diff

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Roo Cline Changelog
 
+## [2.2.7]
+
+-   More fixes to search/replace diffs
+
 ## [2.2.6]
 
 -   Add a fuzzy match tolerance when applying diffs

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "roo-cline",
-  "version": "2.2.6",
+  "version": "2.2.7",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "roo-cline",
-      "version": "2.2.6",
+      "version": "2.2.7",
       "dependencies": {
         "@anthropic-ai/bedrock-sdk": "^0.10.2",
         "@anthropic-ai/sdk": "^0.26.0",

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.6",
+  "version": "2.2.7",
   "icon": "assets/icons/rocket.png",
   "galleryBanner": {
     "color": "#617A91",

src/core/diff/strategies/__tests__/search-replace.test.ts

@@ -291,6 +291,329 @@ function sum(a, b) {
         })
     })
 
+    describe('line-constrained search', () => {
+        let strategy: SearchReplaceDiffStrategy
+
+        beforeEach(() => {
+            strategy = new SearchReplaceDiffStrategy()
+        })
+
+        it('should find and replace within specified line range', () => {
+            const originalContent = `
+function one() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+
+function three() {
+    return 3;
+}
+`.trim()
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function two() {
+    return 2;
+}
+=======
+function two() {
+    return "two";
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
+            expect(result).toBe(`function one() {
+    return 1;
+}
+
+function two() {
+    return "two";
+}
+
+function three() {
+    return 3;
+}`)
+        })
+
+        it('should find and replace within buffer zone (5 lines before/after)', () => {
+            const originalContent = `
+function one() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+
+function three() {
+    return 3;
+}
+`.trim()
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function three() {
+    return 3;
+}
+=======
+function three() {
+    return "three";
+}
+>>>>>>> REPLACE`
+
+            // 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() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+
+function three() {
+    return "three";
+}`)
+        })
+
+        it('should not find matches outside search range and buffer zone', () => {
+            const originalContent = `
+function one() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+
+function three() {
+    return 3;
+}
+
+function four() {
+    return 4;
+}
+
+function five() {
+    return 5;
+}
+`.trim()
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function five() {
+    return 5;
+}
+=======
+function five() {
+    return "five";
+}
+>>>>>>> REPLACE`
+
+            // 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)
+        })
+
+        it('should handle search range at start of file', () => {
+            const originalContent = `
+function one() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+`.trim()
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function one() {
+    return 1;
+}
+=======
+function one() {
+    return "one";
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent, 1, 3)
+            expect(result).toBe(`function one() {
+    return "one";
+}
+
+function two() {
+    return 2;
+}`)
+        })
+
+        it('should handle search range at end of file', () => {
+            const originalContent = `
+function one() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+`.trim()
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function two() {
+    return 2;
+}
+=======
+function two() {
+    return "two";
+}
+>>>>>>> REPLACE`
+
+            const result = strategy.applyDiff(originalContent, diffContent, 5, 7)
+            expect(result).toBe(`function one() {
+    return 1;
+}
+
+function two() {
+    return "two";
+}`)
+        })
+
+        it('should match specific instance of duplicate code using line numbers', () => {
+            const originalContent = `
+function processData(data) {
+    return data.map(x => x * 2);
+}
+
+function unrelatedStuff() {
+    console.log("hello");
+}
+
+// Another data processor
+function processData(data) {
+    return data.map(x => x * 2);
+}
+
+function moreStuff() {
+    console.log("world");
+}
+`.trim()
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function processData(data) {
+    return data.map(x => x * 2);
+}
+=======
+function processData(data) {
+    // Add logging
+    console.log("Processing data...");
+    return data.map(x => x * 2);
+}
+>>>>>>> REPLACE`
+
+            // Target the second instance of processData
+            const result = strategy.applyDiff(originalContent, diffContent, 10, 12)
+            expect(result).toBe(`function processData(data) {
+    return data.map(x => x * 2);
+}
+
+function unrelatedStuff() {
+    console.log("hello");
+}
+
+// Another data processor
+function processData(data) {
+    // Add logging
+    console.log("Processing data...");
+    return data.map(x => x * 2);
+}
+
+function moreStuff() {
+    console.log("world");
+}`)
+        })
+
+        it('should search from start line to end of file when only start_line is provided', () => {
+            const originalContent = `
+function one() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+
+function three() {
+    return 3;
+}
+`.trim()
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function three() {
+    return 3;
+}
+=======
+function three() {
+    return "three";
+}
+>>>>>>> REPLACE`
+
+            // Only provide start_line, should search from there to end of file
+            const result = strategy.applyDiff(originalContent, diffContent, 8)
+            expect(result).toBe(`function one() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+
+function three() {
+    return "three";
+}`)
+        })
+
+        it('should search from start of file to end line when only end_line is provided', () => {
+            const originalContent = `
+function one() {
+    return 1;
+}
+
+function two() {
+    return 2;
+}
+
+function three() {
+    return 3;
+}
+`.trim()
+            const diffContent = `test.ts
+<<<<<<< SEARCH
+function one() {
+    return 1;
+}
+=======
+function one() {
+    return "one";
+}
+>>>>>>> REPLACE`
+
+            // 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() {
+    return "one";
+}
+
+function two() {
+    return 2;
+}
+
+function three() {
+    return 3;
+}`)
+        })
+    })
+
     describe('getToolDescription', () => {
         let strategy: SearchReplaceDiffStrategy
 
@@ -312,5 +635,11 @@ function sum(a, b) {
             expect(description).toContain('<apply_diff>')
             expect(description).toContain('</apply_diff>')
         })
+
+        it('should document start_line and end_line parameters', () => {
+            const description = strategy.getToolDescription('/test')
+            expect(description).toContain('start_line: (required) The line number where the search block starts.')
+            expect(description).toContain('end_line: (required) The line number where the search block ends.')
+        })
     })
 })

src/core/diff/strategies/search-replace.ts

@@ -56,7 +56,7 @@ export class SearchReplaceDiffStrategy implements DiffStrategy {
 
     getToolDescription(cwd: string): string {
         return `## apply_diff
-Description: Request to replace existing code using search and replace blocks.
+Description: Request to replace existing code using a search and replace block.
 This tool allows for precise, surgical replaces to files by specifying exactly what content to search for and what to replace it with.
 The tool will maintain proper indentation and formatting while making changes.
 Only a single operation is allowed per tool use.
@@ -65,11 +65,11 @@ If you're not confident in the exact content to search for, use the read_file to
 
 Parameters:
 - path: (required) The path of the file to modify (relative to the current working directory ${cwd})
-- diff: (required) The search/replace blocks defining the changes.
+- diff: (required) The search/replace block defining the changes.
+- start_line: (required) The line number where the search block starts.
+- end_line: (required) The line number where the search block ends.
 
-Format:
-1. First line must be the file path
-2. Followed by search/replace blocks:
+Diff format:
 \`\`\`
 <<<<<<< SEARCH
 [exact content to find including whitespace]
@@ -82,16 +82,15 @@ Example:
 
 Original file:
 \`\`\`
-def calculate_total(items):
-    total = 0
-    for item in items:
-        total += item
-    return total
+1 | def calculate_total(items):
+2 |     total = 0
+3 |     for item in items:
+4 |         total += item
+5 |     return total
 \`\`\`
 
 Search/Replace content:
 \`\`\`
-main.py
 <<<<<<< SEARCH
 def calculate_total(items):
     total = 0
@@ -111,10 +110,12 @@ Usage:
 <diff>
 Your search/replace content here
 </diff>
+<start_line>1</start_line>
+<end_line>5</end_line>
 </apply_diff>`
     }
 
-    applyDiff(originalContent: string, diffContent: string): string | false {
+    applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): string | false {
         // Extract the search and replace blocks
         const match = diffContent.match(/<<<<<<< SEARCH\n([\s\S]*?)\n=======\n([\s\S]*?)\n>>>>>>> REPLACE/);
         if (!match) {
@@ -131,11 +132,25 @@ Your search/replace content here
         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
         let matchIndex = -1;
         let bestMatchScore = 0;
         
-        for (let i = 0; i <= originalLines.length - searchLines.length; i++) {
+        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');
@@ -169,40 +184,19 @@ Your search/replace content here
         
         // Apply the replacement while preserving exact indentation
         const indentedReplaceLines = replaceLines.map((line, i) => {
-            // Get the corresponding original and search indentations
-            const originalIndent = originalIndents[Math.min(i, originalIndents.length - 1)];
-            const searchIndent = searchIndents[Math.min(i, searchIndents.length - 1)];
+            // Get the matched line's exact indentation
+            const matchedIndent = originalIndents[0];
             
-            // Get the current line's indentation
+            // Get the current line's indentation relative to the search content
             const currentIndentMatch = line.match(/^[\t ]*/);
             const currentIndent = currentIndentMatch ? currentIndentMatch[0] : '';
+            const searchBaseIndent = searchIndents[0] || '';
             
-            // Get the corresponding search line's indentation
-            const searchLineIndex = Math.min(i, searchLines.length - 1);
-            const searchLineIndent = searchIndents[searchLineIndex];
+            // Calculate the relative indentation from the search content
+            const relativeIndent = currentIndent.slice(searchBaseIndent.length);
             
-            // Get the corresponding original line's indentation
-            const originalLineIndex = Math.min(i, originalIndents.length - 1);
-            const originalLineIndent = originalIndents[originalLineIndex];
-
-            // If this line has the same indentation as its corresponding search line,
-            // use the original indentation
-            if (currentIndent === searchLineIndent) {
-                return originalLineIndent + line.trim();
-            }
-
-            // Otherwise, preserve the original indentation structure
-            const indentChar = originalLineIndent.charAt(0) || '\t';
-            const indentLevel = Math.floor(originalLineIndent.length / indentChar.length);
-
-            // Calculate the relative indentation from the search line
-            const searchLevel = Math.floor(searchLineIndent.length / indentChar.length);
-            const currentLevel = Math.floor(currentIndent.length / indentChar.length);
-            const relativeLevel = currentLevel - searchLevel;
-
-            // Apply the relative indentation to the original level
-            const targetLevel = Math.max(0, indentLevel + relativeLevel);
-            return indentChar.repeat(targetLevel) + line.trim();
+            // Apply the matched indentation plus any relative indentation
+            return matchedIndent + relativeIndent + line.trim();
         });
         
         // Construct the final content

src/core/diff/types.ts

@@ -13,7 +13,9 @@ export interface DiffStrategy {
      * Apply a diff to the original content
      * @param originalContent The original file content
      * @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
      */
-    applyDiff(originalContent: string, diffContent: string): string | false
+    applyDiff(originalContent: string, diffContent: string, startLine?: number, endLine?: number): string | false
 }