fix(diffstats): Always return diff stats from EditTool (#7489)

Co-authored-by: Shnatu <snatu@google.com>
This commit is contained in:
Shardul Natu
2025-09-04 10:20:40 -07:00
committed by GitHub
parent cb43bb9ca4
commit cda4280d74
2 changed files with 29 additions and 24 deletions

View File

@@ -476,7 +476,20 @@ describe('EditTool', () => {
expect(result.llmContent).toMatch(/Created new file/);
expect(fs.existsSync(newFilePath)).toBe(true);
expect(fs.readFileSync(newFilePath, 'utf8')).toBe(fileContent);
expect(result.returnDisplay).toBe(`Created ${newFileName}`);
const display = result.returnDisplay as FileDiff;
expect(display.fileDiff).toMatch(/\+Content for the new file\./);
expect(display.fileName).toBe(newFileName);
expect((result.returnDisplay as FileDiff).diffStat).toStrictEqual({
model_added_lines: 1,
model_removed_lines: 0,
model_added_chars: 25,
model_removed_chars: 0,
user_added_lines: 0,
user_removed_lines: 0,
user_added_chars: 0,
user_removed_chars: 0,
});
});
it('should return error if old_string is not found in file', async () => {

View File

@@ -13,7 +13,6 @@ import type {
ToolInvocation,
ToolLocation,
ToolResult,
ToolResultDisplay,
} from './tools.js';
import { BaseDeclarativeTool, Kind, ToolConfirmationOutcome } from './tools.js';
import { ToolErrorType } from './tool-error.js';
@@ -362,7 +361,6 @@ class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> {
.getFileSystemService()
.writeTextFile(this.params.file_path, editData.newContent);
let displayResult: ToolResultDisplay;
const fileName = path.basename(this.params.file_path);
const originallyProposedContent =
this.params.ai_proposed_content || editData.newContent;
@@ -373,27 +371,21 @@ class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> {
editData.newContent,
);
if (editData.isNewFile) {
displayResult = `Created ${shortenPath(makeRelative(this.params.file_path, this.config.getTargetDir()))}`;
} else {
// Generate diff for display, even though core logic doesn't technically need it
// The CLI wrapper will use this part of the ToolResult
const fileDiff = Diff.createPatch(
fileName,
editData.currentContent ?? '', // Should not be null here if not isNewFile
editData.newContent,
'Current',
'Proposed',
DEFAULT_DIFF_OPTIONS,
);
displayResult = {
fileDiff,
fileName,
originalContent: editData.currentContent,
newContent: editData.newContent,
diffStat,
};
}
const fileDiff = Diff.createPatch(
fileName,
editData.currentContent ?? '', // Should not be null here if not isNewFile
editData.newContent,
'Current',
'Proposed',
DEFAULT_DIFF_OPTIONS,
);
const displayResult = {
fileDiff,
fileName,
originalContent: editData.currentContent,
newContent: editData.newContent,
diffStat,
};
// Log file operation for telemetry (without diff_stat to avoid double-counting)
const mimetype = getSpecificMimeType(this.params.file_path);