From e662dd32f8b04380196b0d06da00b3913af8e616 Mon Sep 17 00:00:00 2001 From: Martin Kalema <130189437+MartinKalema@users.noreply.github.com> Date: Tue, 22 Jul 2025 16:32:28 +0300 Subject: [PATCH] fix: improve diff generation and multi-line string replacement for large files (#23) - rewrote generateDiff() to show only relevant context (3 lines before/after changes) - fixed incorrect change counts (was showing 272 additions/removals for 1-line change) - added fuzzy matching for multi-line string replacements to handle formatting differences - added replaceAll parameter to replace all occurrences of a string - added replaceLines() method for line-based editing when string matching isn't suitable - improved hunk generation to merge nearby changes for better readability this significantly improves the user experience when editing large files by showing only the relevant changes instead of the entire file content in confirmation dialogs. --- src/agent/grok-agent.ts | 3 +- src/grok/tools.ts | 6 +- src/tools/text-editor.ts | 406 ++++++++++++++++++++++++++++++++------- src/types/index.ts | 1 + 4 files changed, 348 insertions(+), 68 deletions(-) diff --git a/src/agent/grok-agent.ts b/src/agent/grok-agent.ts index 77261e6..679866d 100644 --- a/src/agent/grok-agent.ts +++ b/src/agent/grok-agent.ts @@ -537,7 +537,8 @@ Current working directory: ${process.cwd()}`, return await this.textEditor.strReplace( args.path, args.old_str, - args.new_str + args.new_str, + args.replace_all ); case "bash": diff --git a/src/grok/tools.ts b/src/grok/tools.ts index 975fcd4..c0e6811 100644 --- a/src/grok/tools.ts +++ b/src/grok/tools.ts @@ -62,12 +62,16 @@ export const GROK_TOOLS: GrokTool[] = [ }, old_str: { type: "string", - description: "Text to replace (must match exactly)", + description: "Text to replace (must match exactly, or will use fuzzy matching for multi-line strings)", }, new_str: { type: "string", description: "Text to replace with", }, + replace_all: { + type: "boolean", + description: "Replace all occurrences (default: false, only replaces first occurrence)", + }, }, required: ["path", "old_str", "new_str"], }, diff --git a/src/tools/text-editor.ts b/src/tools/text-editor.ts index f0ce303..df57e49 100644 --- a/src/tools/text-editor.ts +++ b/src/tools/text-editor.ts @@ -70,7 +70,8 @@ export class TextEditorTool { async strReplace( filePath: string, oldStr: string, - newStr: string + newStr: string, + replaceAll: boolean = false ): Promise { try { const resolvedPath = path.resolve(filePath); @@ -85,25 +86,39 @@ export class TextEditorTool { const content = await fs.readFile(resolvedPath, "utf-8"); if (!content.includes(oldStr)) { - return { - success: false, - error: `String not found in file: "${oldStr}"`, - }; + if (oldStr.includes('\n')) { + const fuzzyResult = this.findFuzzyMatch(content, oldStr); + if (fuzzyResult) { + oldStr = fuzzyResult; + } else { + return { + success: false, + error: `String not found in file. For multi-line replacements, consider using line-based editing.`, + }; + } + } else { + return { + success: false, + error: `String not found in file: "${oldStr}"`, + }; + } } - // Check if user has already accepted file operations for this session + const occurrences = (content.match(new RegExp(oldStr.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length; + const sessionFlags = this.confirmationService.getSessionFlags(); if (!sessionFlags.fileOperations && !sessionFlags.allOperations) { - // Create a proper diff preview showing the change - const newContent = content.replace(oldStr, newStr); + const previewContent = replaceAll + ? content.split(oldStr).join(newStr) + : content.replace(oldStr, newStr); const oldLines = content.split("\n"); - const newLines = newContent.split("\n"); + const newLines = previewContent.split("\n"); const diffContent = this.generateDiff(oldLines, newLines, filePath); const confirmationResult = await this.confirmationService.requestConfirmation( { - operation: "Edit file", + operation: `Edit file${replaceAll && occurrences > 1 ? ` (${occurrences} occurrences)` : ''}`, filename: filePath, showVSCodeOpen: false, content: diffContent, @@ -119,7 +134,9 @@ export class TextEditorTool { } } - const newContent = content.replace(oldStr, newStr); + const newContent = replaceAll + ? content.split(oldStr).join(newStr) + : content.replace(oldStr, newStr); await fs.writeFile(resolvedPath, newContent, "utf-8"); this.editHistory.push({ @@ -129,7 +146,6 @@ export class TextEditorTool { new_str: newStr, }); - // Generate diff output const oldLines = content.split("\n"); const newLines = newContent.split("\n"); const diff = this.generateDiff(oldLines, newLines, filePath); @@ -210,6 +226,94 @@ export class TextEditorTool { } } + async replaceLines( + filePath: string, + startLine: number, + endLine: number, + newContent: string + ): Promise { + try { + const resolvedPath = path.resolve(filePath); + + if (!(await fs.pathExists(resolvedPath))) { + return { + success: false, + error: `File not found: ${filePath}`, + }; + } + + const fileContent = await fs.readFile(resolvedPath, "utf-8"); + const lines = fileContent.split("\n"); + + if (startLine < 1 || startLine > lines.length) { + return { + success: false, + error: `Invalid start line: ${startLine}. File has ${lines.length} lines.`, + }; + } + + if (endLine < startLine || endLine > lines.length) { + return { + success: false, + error: `Invalid end line: ${endLine}. Must be between ${startLine} and ${lines.length}.`, + }; + } + + const sessionFlags = this.confirmationService.getSessionFlags(); + if (!sessionFlags.fileOperations && !sessionFlags.allOperations) { + const newLines = [...lines]; + const replacementLines = newContent.split("\n"); + newLines.splice(startLine - 1, endLine - startLine + 1, ...replacementLines); + + const diffContent = this.generateDiff(lines, newLines, filePath); + + const confirmationResult = + await this.confirmationService.requestConfirmation( + { + operation: `Replace lines ${startLine}-${endLine}`, + filename: filePath, + showVSCodeOpen: false, + content: diffContent, + }, + "file" + ); + + if (!confirmationResult.confirmed) { + return { + success: false, + error: confirmationResult.feedback || "Line replacement cancelled by user", + }; + } + } + + const replacementLines = newContent.split("\n"); + lines.splice(startLine - 1, endLine - startLine + 1, ...replacementLines); + const newFileContent = lines.join("\n"); + + await fs.writeFile(resolvedPath, newFileContent, "utf-8"); + + this.editHistory.push({ + command: "str_replace", + path: filePath, + old_str: `lines ${startLine}-${endLine}`, + new_str: newContent, + }); + + const oldLines = fileContent.split("\n"); + const diff = this.generateDiff(oldLines, lines, filePath); + + return { + success: true, + output: diff, + }; + } catch (error: any) { + return { + success: false, + error: `Error replacing lines in ${filePath}: ${error.message}`, + }; + } + } + async insert( filePath: string, insertLine: number, @@ -303,38 +407,230 @@ export class TextEditorTool { } } + private findFuzzyMatch(content: string, searchStr: string): string | null { + const functionMatch = searchStr.match(/function\s+(\w+)/); + if (!functionMatch) return null; + + const functionName = functionMatch[1]; + const contentLines = content.split('\n'); + + let functionStart = -1; + for (let i = 0; i < contentLines.length; i++) { + if (contentLines[i].includes(`function ${functionName}`) && contentLines[i].includes('{')) { + functionStart = i; + break; + } + } + + if (functionStart === -1) return null; + + let braceCount = 0; + let functionEnd = functionStart; + + for (let i = functionStart; i < contentLines.length; i++) { + const line = contentLines[i]; + for (const char of line) { + if (char === '{') braceCount++; + if (char === '}') braceCount--; + } + + if (braceCount === 0 && i > functionStart) { + functionEnd = i; + break; + } + } + + const actualFunction = contentLines.slice(functionStart, functionEnd + 1).join('\n'); + + const searchNormalized = this.normalizeForComparison(searchStr); + const actualNormalized = this.normalizeForComparison(actualFunction); + + if (this.isSimilarStructure(searchNormalized, actualNormalized)) { + return actualFunction; + } + + return null; + } + + private normalizeForComparison(str: string): string { + return str + .replace(/["'`]/g, '"') + .replace(/\s+/g, ' ') + .replace(/{\s+/g, '{ ') + .replace(/\s+}/g, ' }') + .replace(/;\s*/g, ';') + .trim(); + } + + private isSimilarStructure(search: string, actual: string): boolean { + const extractTokens = (str: string) => { + const tokens = str.match(/\b(function|console\.log|return|if|else|for|while)\b/g) || []; + return tokens; + }; + + const searchTokens = extractTokens(search); + const actualTokens = extractTokens(actual); + + if (searchTokens.length !== actualTokens.length) return false; + + for (let i = 0; i < searchTokens.length; i++) { + if (searchTokens[i] !== actualTokens[i]) return false; + } + + return true; + } + private generateDiff( oldLines: string[], newLines: string[], filePath: string ): string { - // Count actual changes - let addedLines = 0; - let removedLines = 0; - let i = 0, - j = 0; - - // Simple algorithm to detect changes + const CONTEXT_LINES = 3; + + const changes: Array<{ + oldStart: number; + oldEnd: number; + newStart: number; + newEnd: number; + }> = []; + + let i = 0, j = 0; + while (i < oldLines.length || j < newLines.length) { - if ( - i < oldLines.length && - j < newLines.length && - oldLines[i] === newLines[j] - ) { + while (i < oldLines.length && j < newLines.length && oldLines[i] === newLines[j]) { i++; j++; - } else if ( - i < oldLines.length && - (j >= newLines.length || oldLines[i] !== newLines[j]) - ) { - removedLines++; - i++; - } else if (j < newLines.length) { - addedLines++; - j++; + } + + if (i < oldLines.length || j < newLines.length) { + const changeStart = { old: i, new: j }; + + let oldEnd = i; + let newEnd = j; + + while (oldEnd < oldLines.length || newEnd < newLines.length) { + let matchFound = false; + let matchLength = 0; + + for (let k = 0; k < Math.min(2, oldLines.length - oldEnd, newLines.length - newEnd); k++) { + if (oldEnd + k < oldLines.length && + newEnd + k < newLines.length && + oldLines[oldEnd + k] === newLines[newEnd + k]) { + matchLength++; + } else { + break; + } + } + + if (matchLength >= 2 || (oldEnd >= oldLines.length && newEnd >= newLines.length)) { + matchFound = true; + } + + if (matchFound) { + break; + } + + if (oldEnd < oldLines.length) oldEnd++; + if (newEnd < newLines.length) newEnd++; + } + + changes.push({ + oldStart: changeStart.old, + oldEnd: oldEnd, + newStart: changeStart.new, + newEnd: newEnd + }); + + i = oldEnd; + j = newEnd; } } - + + const hunks: Array<{ + oldStart: number; + oldCount: number; + newStart: number; + newCount: number; + lines: Array<{ type: '+' | '-' | ' '; content: string }>; + }> = []; + + let accumulatedOffset = 0; + + for (let changeIdx = 0; changeIdx < changes.length; changeIdx++) { + const change = changes[changeIdx]; + + let contextStart = Math.max(0, change.oldStart - CONTEXT_LINES); + let contextEnd = Math.min(oldLines.length, change.oldEnd + CONTEXT_LINES); + + if (hunks.length > 0) { + const lastHunk = hunks[hunks.length - 1]; + const lastHunkEnd = lastHunk.oldStart + lastHunk.oldCount; + + if (lastHunkEnd >= contextStart) { + const oldHunkEnd = lastHunk.oldStart + lastHunk.oldCount; + const newContextEnd = Math.min(oldLines.length, change.oldEnd + CONTEXT_LINES); + + for (let idx = oldHunkEnd; idx < change.oldStart; idx++) { + lastHunk.lines.push({ type: ' ', content: oldLines[idx] }); + } + + for (let idx = change.oldStart; idx < change.oldEnd; idx++) { + lastHunk.lines.push({ type: '-', content: oldLines[idx] }); + } + for (let idx = change.newStart; idx < change.newEnd; idx++) { + lastHunk.lines.push({ type: '+', content: newLines[idx] }); + } + + for (let idx = change.oldEnd; idx < newContextEnd && idx < oldLines.length; idx++) { + lastHunk.lines.push({ type: ' ', content: oldLines[idx] }); + } + + lastHunk.oldCount = newContextEnd - lastHunk.oldStart; + lastHunk.newCount = lastHunk.oldCount + (change.newEnd - change.newStart) - (change.oldEnd - change.oldStart); + + continue; + } + } + + const hunk: typeof hunks[0] = { + oldStart: contextStart + 1, + oldCount: contextEnd - contextStart, + newStart: contextStart + 1 + accumulatedOffset, + newCount: contextEnd - contextStart + (change.newEnd - change.newStart) - (change.oldEnd - change.oldStart), + lines: [] + }; + + for (let idx = contextStart; idx < change.oldStart; idx++) { + hunk.lines.push({ type: ' ', content: oldLines[idx] }); + } + + for (let idx = change.oldStart; idx < change.oldEnd; idx++) { + hunk.lines.push({ type: '-', content: oldLines[idx] }); + } + + for (let idx = change.newStart; idx < change.newEnd; idx++) { + hunk.lines.push({ type: '+', content: newLines[idx] }); + } + + for (let idx = change.oldEnd; idx < contextEnd && idx < oldLines.length; idx++) { + hunk.lines.push({ type: ' ', content: oldLines[idx] }); + } + + hunks.push(hunk); + + accumulatedOffset += (change.newEnd - change.newStart) - (change.oldEnd - change.oldStart); + } + + let addedLines = 0; + let removedLines = 0; + + for (const hunk of hunks) { + for (const line of hunk.lines) { + if (line.type === '+') addedLines++; + if (line.type === '-') removedLines++; + } + } + let summary = `Updated ${filePath}`; if (addedLines > 0 && removedLines > 0) { summary += ` with ${addedLines} addition${ @@ -346,44 +642,22 @@ export class TextEditorTool { summary += ` with ${removedLines} removal${ removedLines !== 1 ? "s" : "" }`; - } else { - summary += " with changes"; + } else if (changes.length === 0) { + return `No changes in ${filePath}`; } - - // Generate proper git-style diff format + let diff = summary + "\n"; diff += `--- a/${filePath}\n`; diff += `+++ b/${filePath}\n`; - diff += `@@ -1,${oldLines.length} +1,${newLines.length} @@\n`; - - // Generate unified diff - i = 0; - j = 0; - const CONTEXT_LINES = 3; - - while (i < oldLines.length || j < newLines.length) { - const oldLine = i < oldLines.length ? oldLines[i] : null; - const newLine = j < newLines.length ? newLines[j] : null; - - if (oldLine === newLine && oldLine !== null) { - // Context line - diff += ` ${oldLine}\n`; - i++; - j++; - } else { - // Show removed lines - if (oldLine !== null) { - diff += `-${oldLine}\n`; - i++; - } - // Show added lines - if (newLine !== null) { - diff += `+${newLine}\n`; - j++; - } + + for (const hunk of hunks) { + diff += `@@ -${hunk.oldStart},${hunk.oldCount} +${hunk.newStart},${hunk.newCount} @@\n`; + + for (const line of hunk.lines) { + diff += `${line.type}${line.content}\n`; } } - + return diff.trim(); } diff --git a/src/types/index.ts b/src/types/index.ts index 46dfdb1..f565e25 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -19,6 +19,7 @@ export interface EditorCommand { content?: string; insert_line?: number; view_range?: [number, number]; + replace_all?: boolean; } export interface AgentState {