From b8330b626ef9a134bee5089669751289a3c025c4 Mon Sep 17 00:00:00 2001 From: Adib234 <30782825+Adib234@users.noreply.github.com> Date: Thu, 30 Oct 2025 07:29:39 -0700 Subject: [PATCH] Fix misreported number of lines being removed by model (#12076) --- packages/core/src/tools/edit.test.ts | 85 +++++++++++++++++++++ packages/core/src/tools/smart-edit.test.ts | 87 ++++++++++++++++++++++ packages/core/src/tools/smart-edit.ts | 5 +- 3 files changed, 174 insertions(+), 3 deletions(-) diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index ab021cd161..5f1a164be4 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -1114,4 +1114,89 @@ describe('EditTool', () => { expect(params.new_string).toBe(modifiedContent); }); }); + + describe('multiple file edits', () => { + it('should perform multiple removals and report correct diff stats', async () => { + const numFiles = 10; + const files: Array<{ + path: string; + initialContent: string; + toRemove: string; + }> = []; + const expectedLinesRemoved: number[] = []; + const actualLinesRemoved: number[] = []; + + // 1. Create 10 files with 5-10 lines each + for (let i = 0; i < numFiles; i++) { + const fileName = `test-file-${i}.txt`; + const filePath = path.join(rootDir, fileName); + const numLines = Math.floor(Math.random() * 6) + 5; // 5 to 10 lines + const lines = Array.from( + { length: numLines }, + (_, j) => `File ${i}, Line ${j + 1}`, + ); + const content = lines.join('\n') + '\n'; + + // Determine which lines to remove (2 or 3 lines) + const numLinesToRemove = Math.floor(Math.random() * 2) + 2; // 2 or 3 + expectedLinesRemoved.push(numLinesToRemove); + const startLineToRemove = 1; // Start removing from the second line + const linesToRemove = lines.slice( + startLineToRemove, + startLineToRemove + numLinesToRemove, + ); + const toRemove = linesToRemove.join('\n') + '\n'; + + fs.writeFileSync(filePath, content, 'utf8'); + files.push({ + path: filePath, + initialContent: content, + toRemove, + }); + } + + // 2. Create and execute 10 tool calls for removal + for (const file of files) { + const params: EditToolParams = { + file_path: file.path, + old_string: file.toRemove, + new_string: '', // Removing the content + }; + const invocation = tool.build(params); + const result = await invocation.execute(new AbortController().signal); + + if ( + result.returnDisplay && + typeof result.returnDisplay === 'object' && + 'diffStat' in result.returnDisplay && + result.returnDisplay.diffStat + ) { + actualLinesRemoved.push( + result.returnDisplay.diffStat?.model_removed_lines, + ); + } else if (result.error) { + console.error(`Edit failed for ${file.path}:`, result.error); + } + } + + // 3. Assert that the content was removed from each file + for (const file of files) { + const finalContent = fs.readFileSync(file.path, 'utf8'); + const expectedContent = file.initialContent.replace(file.toRemove, ''); + expect(finalContent).toBe(expectedContent); + expect(finalContent).not.toContain(file.toRemove); + } + + // 4. Assert that the total number of removed lines matches the diffStat total + const totalExpectedRemoved = expectedLinesRemoved.reduce( + (sum, current) => sum + current, + 0, + ); + const totalActualRemoved = actualLinesRemoved.reduce( + (sum, current) => sum + current, + 0, + ); + expect(totalActualRemoved).toBe(totalExpectedRemoved); + }); + }); }); diff --git a/packages/core/src/tools/smart-edit.test.ts b/packages/core/src/tools/smart-edit.test.ts index d6ec11acb6..8e81309fad 100644 --- a/packages/core/src/tools/smart-edit.test.ts +++ b/packages/core/src/tools/smart-edit.test.ts @@ -660,4 +660,91 @@ describe('SmartEditTool', () => { calculateSpy.mockRestore(); }); }); + + describe('multiple file edits', () => { + it('should perform multiple removals and report correct diff stats', async () => { + const numFiles = 10; + const files: Array<{ + path: string; + initialContent: string; + toRemove: string; + }> = []; + const expectedLinesRemoved: number[] = []; + const actualLinesRemoved: number[] = []; + + // 1. Create 10 files with 5-10 lines each + for (let i = 0; i < numFiles; i++) { + const fileName = `test-file-${i}.txt`; + const filePath = path.join(rootDir, fileName); + const numLines = Math.floor(Math.random() * 6) + 5; // 5 to 10 lines + const lines = Array.from( + { length: numLines }, + (_, j) => `File ${i}, Line ${j + 1}`, + ); + const content = lines.join('\n') + '\n'; + + // Determine which lines to remove (2 or 3 lines) + const numLinesToRemove = Math.floor(Math.random() * 2) + 2; // 2 or 3 + expectedLinesRemoved.push(numLinesToRemove); + const startLineToRemove = 1; // Start removing from the second line + const linesToRemove = lines.slice( + startLineToRemove, + startLineToRemove + numLinesToRemove, + ); + const toRemove = linesToRemove.join('\n') + '\n'; + + fs.writeFileSync(filePath, content, 'utf8'); + files.push({ + path: filePath, + initialContent: content, + toRemove, + }); + } + + // 2. Create and execute 10 tool calls for removal + for (const file of files) { + const params: EditToolParams = { + file_path: file.path, + instruction: `Remove lines from the file`, + old_string: file.toRemove, + new_string: '', // Removing the content + ai_proposed_string: '', + }; + const invocation = tool.build(params); + const result = await invocation.execute(new AbortController().signal); + + if ( + result.returnDisplay && + typeof result.returnDisplay === 'object' && + 'diffStat' in result.returnDisplay && + result.returnDisplay.diffStat + ) { + actualLinesRemoved.push( + result.returnDisplay.diffStat?.model_removed_lines, + ); + } else if (result.error) { + console.error(`Edit failed for ${file.path}:`, result.error); + } + } + + // 3. Assert that the content was removed from each file + for (const file of files) { + const finalContent = fs.readFileSync(file.path, 'utf8'); + const expectedContent = file.initialContent.replace(file.toRemove, ''); + expect(finalContent).toBe(expectedContent); + expect(finalContent).not.toContain(file.toRemove); + } + + // 4. Assert that the total number of removed lines matches the diffStat total + const totalExpectedRemoved = expectedLinesRemoved.reduce( + (sum, current) => sum + current, + 0, + ); + const totalActualRemoved = actualLinesRemoved.reduce( + (sum, current) => sum + current, + 0, + ); + expect(totalActualRemoved).toBe(totalExpectedRemoved); + }); + }); }); diff --git a/packages/core/src/tools/smart-edit.ts b/packages/core/src/tools/smart-edit.ts index 8c826292a8..9c6ebdbc43 100644 --- a/packages/core/src/tools/smart-edit.ts +++ b/packages/core/src/tools/smart-edit.ts @@ -763,12 +763,11 @@ class EditToolInvocation 'Proposed', DEFAULT_DIFF_OPTIONS, ); - const originallyProposedContent = - this.params.ai_proposed_string || this.params.new_string; + const diffStat = getDiffStat( fileName, editData.currentContent ?? '', - originallyProposedContent, + editData.newContent, this.params.new_string, ); displayResult = {