Fix misreported number of lines being removed by model (#12076)

This commit is contained in:
Adib234
2025-10-30 07:29:39 -07:00
committed by GitHub
parent 2a3244b1bb
commit b8330b626e
3 changed files with 174 additions and 3 deletions

View File

@@ -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);
});
});
});

View File

@@ -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);
});
});
});

View File

@@ -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 = {