fix(diffstats): Fix diff stats to correctly capture the edits (#7446)

Co-authored-by: Shnatu <snatu@google.com>
Co-authored-by: Gaurav <39389231+gsquared94@users.noreply.github.com>
This commit is contained in:
Shardul Natu
2025-08-30 13:56:10 -07:00
committed by GitHub
parent 96707b588e
commit a167f28ead
2 changed files with 30 additions and 19 deletions
+19 -5
View File
@@ -507,7 +507,7 @@ describe('EditTool', () => {
}); });
it('should successfully replace multiple occurrences when expected_replacements specified', async () => { it('should successfully replace multiple occurrences when expected_replacements specified', async () => {
fs.writeFileSync(filePath, 'old text old text old text', 'utf8'); fs.writeFileSync(filePath, 'old text\nold text\nold text', 'utf8');
const params: EditToolParams = { const params: EditToolParams = {
file_path: filePath, file_path: filePath,
old_string: 'old', old_string: 'old',
@@ -520,12 +520,19 @@ describe('EditTool', () => {
expect(result.llmContent).toMatch(/Successfully modified file/); expect(result.llmContent).toMatch(/Successfully modified file/);
expect(fs.readFileSync(filePath, 'utf8')).toBe( expect(fs.readFileSync(filePath, 'utf8')).toBe(
'new text new text new text', 'new text\nnew text\nnew text',
); );
const display = result.returnDisplay as FileDiff; const display = result.returnDisplay as FileDiff;
expect(display.fileDiff).toMatch(/old text old text old text/);
expect(display.fileDiff).toMatch(/new text new text new text/); expect(display.fileDiff).toMatch(/-old text\n-old text\n-old text/);
expect(display.fileDiff).toMatch(/\+new text\n\+new text\n\+new text/);
expect(display.fileName).toBe(testFile); expect(display.fileName).toBe(testFile);
expect((result.returnDisplay as FileDiff).diffStat).toStrictEqual({
ai_added_lines: 3,
ai_removed_lines: 3,
user_added_lines: 0,
user_removed_lines: 0,
});
}); });
it('should return error if expected_replacements does not match actual occurrences', async () => { it('should return error if expected_replacements does not match actual occurrences', async () => {
@@ -562,13 +569,14 @@ describe('EditTool', () => {
}); });
it('should include modification message when proposed content is modified', async () => { it('should include modification message when proposed content is modified', async () => {
const initialContent = 'This is some old text.'; const initialContent = 'Line 1\nold line\nLine 3\nLine 4\nLine 5\n';
fs.writeFileSync(filePath, initialContent, 'utf8'); fs.writeFileSync(filePath, initialContent, 'utf8');
const params: EditToolParams = { const params: EditToolParams = {
file_path: filePath, file_path: filePath,
old_string: 'old', old_string: 'old',
new_string: 'new', new_string: 'new',
modified_by_user: true, modified_by_user: true,
ai_proposed_content: 'Line 1\nAI line\nLine 3\nLine 4\nLine 5\n',
}; };
(mockConfig.getApprovalMode as Mock).mockReturnValueOnce( (mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
@@ -580,6 +588,12 @@ describe('EditTool', () => {
expect(result.llmContent).toMatch( expect(result.llmContent).toMatch(
/User modified the `new_string` content/, /User modified the `new_string` content/,
); );
expect((result.returnDisplay as FileDiff).diffStat).toStrictEqual({
ai_added_lines: 1,
ai_removed_lines: 1,
user_added_lines: 1,
user_removed_lines: 1,
});
}); });
it('should not include modification message when proposed content is not modified', async () => { it('should not include modification message when proposed content is not modified', async () => {
+11 -14
View File
@@ -86,9 +86,9 @@ export interface EditToolParams {
modified_by_user?: boolean; modified_by_user?: boolean;
/** /**
* Initially proposed string. * Initially proposed content.
*/ */
ai_proposed_string?: string; ai_proposed_content?: string;
} }
interface CalculatedEdit { interface CalculatedEdit {
@@ -365,12 +365,12 @@ class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> {
let displayResult: ToolResultDisplay; let displayResult: ToolResultDisplay;
const fileName = path.basename(this.params.file_path); const fileName = path.basename(this.params.file_path);
const originallyProposedContent = const originallyProposedContent =
this.params.ai_proposed_string || this.params.new_string; this.params.ai_proposed_content || editData.newContent;
const diffStat = getDiffStat( const diffStat = getDiffStat(
fileName, fileName,
editData.currentContent ?? '', editData.currentContent ?? '',
originallyProposedContent, originallyProposedContent,
this.params.new_string, editData.newContent,
); );
if (editData.isNewFile) { if (editData.isNewFile) {
@@ -572,16 +572,13 @@ Expectation for required parameters:
oldContent: string, oldContent: string,
modifiedProposedContent: string, modifiedProposedContent: string,
originalParams: EditToolParams, originalParams: EditToolParams,
): EditToolParams => { ): EditToolParams => ({
const content = originalParams.new_string; ...originalParams,
return { ai_proposed_content: oldContent,
...originalParams, old_string: oldContent,
ai_proposed_string: content, new_string: modifiedProposedContent,
old_string: oldContent, modified_by_user: true,
new_string: modifiedProposedContent, }),
modified_by_user: true,
};
},
}; };
} }
} }