From 5ab184fcaf40d4e7dec9ba6a0526cac39b602ee2 Mon Sep 17 00:00:00 2001 From: agarwalravikant Date: Fri, 8 Aug 2025 10:08:07 +0530 Subject: [PATCH] Fix for git issue 5657 to add lines of code added/removed telemetry (#5823) Co-authored-by: Ravikant Agarwal --- docs/telemetry.md | 7 +- .../src/core/nonInteractiveToolExecutor.ts | 20 +++ .../clearcut-logger/clearcut-logger.ts | 18 +++ .../clearcut-logger/event-metadata-key.ts | 12 ++ packages/core/src/telemetry/metrics.test.ts | 82 +++++++++++ packages/core/src/telemetry/metrics.ts | 8 ++ packages/core/src/telemetry/types.ts | 20 +++ packages/core/src/tools/diffOptions.test.ts | 129 ++++++++++++++++++ packages/core/src/tools/diffOptions.ts | 53 +++++++ packages/core/src/tools/edit.ts | 32 ++++- packages/core/src/tools/tools.ts | 8 ++ packages/core/src/tools/write-file.ts | 33 ++++- 12 files changed, 408 insertions(+), 14 deletions(-) create mode 100644 packages/core/src/tools/diffOptions.test.ts diff --git a/docs/telemetry.md b/docs/telemetry.md index 16bff27b..68c3aed2 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -183,9 +183,10 @@ Logs are timestamped records of specific events. The following events are logged - `function_args` - `duration_ms` - `success` (boolean) - - `decision` (string: "accept", "reject", or "modify", if applicable) + - `decision` (string: "accept", "reject", "auto_accept", or "modify", if applicable) - `error` (if applicable) - `error_type` (if applicable) + - `metadata` (if applicable, dictionary of string -> any) - `gemini_cli.api_request`: This event occurs when making a request to Gemini API. - **Attributes**: @@ -262,3 +263,7 @@ Metrics are numerical measurements of behavior over time. The following metrics - `lines` (Int, if applicable): Number of lines in the file. - `mimetype` (string, if applicable): Mimetype of the file. - `extension` (string, if applicable): File extension of the file. + - `ai_added_lines` (Int, if applicable): Number of lines added/changed by AI. + - `ai_removed_lines` (Int, if applicable): Number of lines removed/changed by AI. + - `user_added_lines` (Int, if applicable): Number of lines added/changed by user in AI proposed changes. + - `user_removed_lines` (Int, if applicable): Number of lines removed/changed by user in AI proposed changes. diff --git a/packages/core/src/core/nonInteractiveToolExecutor.ts b/packages/core/src/core/nonInteractiveToolExecutor.ts index 29d79152..ab38d830 100644 --- a/packages/core/src/core/nonInteractiveToolExecutor.ts +++ b/packages/core/src/core/nonInteractiveToolExecutor.ts @@ -5,6 +5,7 @@ */ import { + FileDiff, logToolCall, ToolCallRequestInfo, ToolCallResponseInfo, @@ -75,6 +76,24 @@ export async function executeToolCall( const tool_display = toolResult.returnDisplay; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let metadata: { [key: string]: any } = {}; + if ( + toolResult.error === undefined && + typeof tool_display === 'object' && + tool_display !== null && + 'diffStat' in tool_display + ) { + const diffStat = (tool_display as FileDiff).diffStat; + if (diffStat) { + metadata = { + ai_added_lines: diffStat.ai_added_lines, + ai_removed_lines: diffStat.ai_removed_lines, + user_added_lines: diffStat.user_added_lines, + user_removed_lines: diffStat.user_removed_lines, + }; + } + } const durationMs = Date.now() - startTime; logToolCall(config, { 'event.name': 'tool_call', @@ -88,6 +107,7 @@ export async function executeToolCall( error_type: toolResult.error === undefined ? undefined : toolResult.error.type, prompt_id: toolCallRequest.prompt_id, + metadata, decision: ToolCallDecision.AUTO_ACCEPT, }); diff --git a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts index bff9f28e..09ae6741 100644 --- a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts +++ b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts @@ -476,6 +476,24 @@ export class ClearcutLogger { }, ]; + if (event.metadata) { + const metadataMapping: { [key: string]: EventMetadataKey } = { + ai_added_lines: EventMetadataKey.GEMINI_CLI_AI_ADDED_LINES, + ai_removed_lines: EventMetadataKey.GEMINI_CLI_AI_REMOVED_LINES, + user_added_lines: EventMetadataKey.GEMINI_CLI_USER_ADDED_LINES, + user_removed_lines: EventMetadataKey.GEMINI_CLI_USER_REMOVED_LINES, + }; + + for (const [key, gemini_cli_key] of Object.entries(metadataMapping)) { + if (event.metadata[key] !== undefined) { + data.push({ + gemini_cli_key, + value: JSON.stringify(event.metadata[key]), + }); + } + } + } + const logEvent = this.createLogEvent(tool_call_event_name, data); this.enqueueLogEvent(logEvent); this.flushIfNeeded(); diff --git a/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts b/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts index 54f570f1..81f41603 100644 --- a/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts +++ b/packages/core/src/telemetry/clearcut-logger/event-metadata-key.ts @@ -197,6 +197,18 @@ export enum EventMetadataKey { // Logs the type of the IDE connection. GEMINI_CLI_IDE_CONNECTION_TYPE = 46, + + // Logs AI added lines in edit/write tool response. + GEMINI_CLI_AI_ADDED_LINES = 47, + + // Logs AI removed lines in edit/write tool response. + GEMINI_CLI_AI_REMOVED_LINES = 48, + + // Logs user added lines in edit/write tool response. + GEMINI_CLI_USER_ADDED_LINES = 49, + + // Logs user removed lines in edit/write tool response. + GEMINI_CLI_USER_REMOVED_LINES = 50, } export function getEventMetadataKey( diff --git a/packages/core/src/telemetry/metrics.test.ts b/packages/core/src/telemetry/metrics.test.ts index 4fcdd9e1..7b430884 100644 --- a/packages/core/src/telemetry/metrics.test.ts +++ b/packages/core/src/telemetry/metrics.test.ts @@ -221,5 +221,87 @@ describe('Telemetry Metrics', () => { mimetype: 'application/javascript', }); }); + + it('should include diffStat when provided', () => { + initializeMetricsModule(mockConfig); + mockCounterAddFn.mockClear(); + + const diffStat = { + ai_added_lines: 5, + ai_removed_lines: 2, + user_added_lines: 3, + user_removed_lines: 1, + }; + + recordFileOperationMetricModule( + mockConfig, + FileOperation.UPDATE, + undefined, + undefined, + undefined, + diffStat, + ); + + expect(mockCounterAddFn).toHaveBeenCalledWith(1, { + 'session.id': 'test-session-id', + operation: FileOperation.UPDATE, + ai_added_lines: 5, + ai_removed_lines: 2, + user_added_lines: 3, + user_removed_lines: 1, + }); + }); + + it('should not include diffStat attributes when diffStat is not provided', () => { + initializeMetricsModule(mockConfig); + mockCounterAddFn.mockClear(); + + recordFileOperationMetricModule( + mockConfig, + FileOperation.UPDATE, + 10, + 'text/plain', + 'txt', + undefined, + ); + + expect(mockCounterAddFn).toHaveBeenCalledWith(1, { + 'session.id': 'test-session-id', + operation: FileOperation.UPDATE, + lines: 10, + mimetype: 'text/plain', + extension: 'txt', + }); + }); + + it('should handle diffStat with all zero values', () => { + initializeMetricsModule(mockConfig); + mockCounterAddFn.mockClear(); + + const diffStat = { + ai_added_lines: 0, + ai_removed_lines: 0, + user_added_lines: 0, + user_removed_lines: 0, + }; + + recordFileOperationMetricModule( + mockConfig, + FileOperation.UPDATE, + undefined, + undefined, + undefined, + diffStat, + ); + + expect(mockCounterAddFn).toHaveBeenCalledWith(1, { + 'session.id': 'test-session-id', + operation: FileOperation.UPDATE, + ai_added_lines: 0, + ai_removed_lines: 0, + user_added_lines: 0, + user_removed_lines: 0, + }); + }); }); }); diff --git a/packages/core/src/telemetry/metrics.ts b/packages/core/src/telemetry/metrics.ts index 103f7f71..1e4509da 100644 --- a/packages/core/src/telemetry/metrics.ts +++ b/packages/core/src/telemetry/metrics.ts @@ -23,6 +23,7 @@ import { METRIC_FILE_OPERATION_COUNT, } from './constants.js'; import { Config } from '../config/config.js'; +import { DiffStat } from '../tools/tools.js'; export enum FileOperation { CREATE = 'create', @@ -189,6 +190,7 @@ export function recordFileOperationMetric( lines?: number, mimetype?: string, extension?: string, + diffStat?: DiffStat, ): void { if (!fileOperationCounter || !isMetricsInitialized) return; const attributes: Attributes = { @@ -198,5 +200,11 @@ export function recordFileOperationMetric( if (lines !== undefined) attributes.lines = lines; if (mimetype !== undefined) attributes.mimetype = mimetype; if (extension !== undefined) attributes.extension = extension; + if (diffStat !== undefined) { + attributes.ai_added_lines = diffStat.ai_added_lines; + attributes.ai_removed_lines = diffStat.ai_removed_lines; + attributes.user_added_lines = diffStat.user_added_lines; + attributes.user_removed_lines = diffStat.user_removed_lines; + } fileOperationCounter.add(1, attributes); } diff --git a/packages/core/src/telemetry/types.ts b/packages/core/src/telemetry/types.ts index 766d5f47..edcd8b1b 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -7,6 +7,7 @@ import { GenerateContentResponseUsageMetadata } from '@google/genai'; import { Config } from '../config/config.js'; import { CompletedToolCall } from '../core/coreToolScheduler.js'; +import { FileDiff } from '../tools/tools.js'; import { AuthType } from '../core/contentGenerator.js'; import { getDecisionFromOutcome, @@ -105,6 +106,8 @@ export class ToolCallEvent { error?: string; error_type?: string; prompt_id: string; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + metadata?: { [key: string]: any }; constructor(call: CompletedToolCall) { this['event.name'] = 'tool_call'; @@ -119,6 +122,23 @@ export class ToolCallEvent { this.error = call.response.error?.message; this.error_type = call.response.errorType; this.prompt_id = call.request.prompt_id; + + if ( + call.status === 'success' && + typeof call.response.resultDisplay === 'object' && + call.response.resultDisplay !== null && + 'diffStat' in call.response.resultDisplay + ) { + const diffStat = (call.response.resultDisplay as FileDiff).diffStat; + if (diffStat) { + this.metadata = { + ai_added_lines: diffStat.ai_added_lines, + ai_removed_lines: diffStat.ai_removed_lines, + user_added_lines: diffStat.user_added_lines, + user_removed_lines: diffStat.user_removed_lines, + }; + } + } } } diff --git a/packages/core/src/tools/diffOptions.test.ts b/packages/core/src/tools/diffOptions.test.ts new file mode 100644 index 00000000..95c3beb4 --- /dev/null +++ b/packages/core/src/tools/diffOptions.test.ts @@ -0,0 +1,129 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { getDiffStat } from './diffOptions.js'; + +describe('getDiffStat', () => { + const fileName = 'test.txt'; + + it('should return 0 for all stats when there are no changes', () => { + const oldStr = 'line1\nline2\n'; + const aiStr = 'line1\nline2\n'; + const userStr = 'line1\nline2\n'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 0, + ai_removed_lines: 0, + user_added_lines: 0, + user_removed_lines: 0, + }); + }); + + it('should correctly report AI additions', () => { + const oldStr = 'line1\nline2\n'; + const aiStr = 'line1\nline2\nline3\n'; + const userStr = 'line1\nline2\nline3\n'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 1, + ai_removed_lines: 0, + user_added_lines: 0, + user_removed_lines: 0, + }); + }); + + it('should correctly report AI removals', () => { + const oldStr = 'line1\nline2\nline3\n'; + const aiStr = 'line1\nline3\n'; + const userStr = 'line1\nline3\n'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 0, + ai_removed_lines: 1, + user_added_lines: 0, + user_removed_lines: 0, + }); + }); + + it('should correctly report AI modifications', () => { + const oldStr = 'line1\nline2\nline3\n'; + const aiStr = 'line1\nline_two\nline3\n'; + const userStr = 'line1\nline_two\nline3\n'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 1, + ai_removed_lines: 1, + user_added_lines: 0, + user_removed_lines: 0, + }); + }); + + it('should correctly report user additions', () => { + const oldStr = 'line1\nline2\n'; + const aiStr = 'line1\nline2\nline3\n'; + const userStr = 'line1\nline2\nline3\nline4\n'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 1, + ai_removed_lines: 0, + user_added_lines: 1, + user_removed_lines: 0, + }); + }); + + it('should correctly report user removals', () => { + const oldStr = 'line1\nline2\n'; + const aiStr = 'line1\nline2\nline3\n'; + const userStr = 'line1\nline2\n'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 1, + ai_removed_lines: 0, + user_added_lines: 0, + user_removed_lines: 1, + }); + }); + + it('should correctly report user modifications', () => { + const oldStr = 'line1\nline2\n'; + const aiStr = 'line1\nline2\nline3\n'; + const userStr = 'line1\nline2\nline_three\n'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 1, + ai_removed_lines: 0, + user_added_lines: 1, + user_removed_lines: 1, + }); + }); + + it('should handle complex changes from both AI and user', () => { + const oldStr = 'line1\nline2\nline3\nline4\n'; + const aiStr = 'line_one\nline2\nline_three\nline4\n'; + const userStr = 'line_one\nline_two\nline_three\nline4\nline5\n'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 2, + ai_removed_lines: 2, + user_added_lines: 2, + user_removed_lines: 1, + }); + }); + + it('should report a single line modification as one addition and one removal', () => { + const oldStr = 'hello world'; + const aiStr = 'hello universe'; + const userStr = 'hello universe'; + const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr); + expect(diffStat).toEqual({ + ai_added_lines: 1, + ai_removed_lines: 1, + user_added_lines: 0, + user_removed_lines: 0, + }); + }); +}); diff --git a/packages/core/src/tools/diffOptions.ts b/packages/core/src/tools/diffOptions.ts index 598b46f1..50574226 100644 --- a/packages/core/src/tools/diffOptions.ts +++ b/packages/core/src/tools/diffOptions.ts @@ -5,8 +5,61 @@ */ import * as Diff from 'diff'; +import { DiffStat } from './tools.js'; export const DEFAULT_DIFF_OPTIONS: Diff.PatchOptions = { context: 3, ignoreWhitespace: true, }; + +export function getDiffStat( + fileName: string, + oldStr: string, + aiStr: string, + userStr: string, +): DiffStat { + const countLines = (patch: Diff.ParsedDiff) => { + let added = 0; + let removed = 0; + patch.hunks.forEach((hunk: Diff.Hunk) => { + hunk.lines.forEach((line: string) => { + if (line.startsWith('+')) { + added++; + } else if (line.startsWith('-')) { + removed++; + } + }); + }); + return { added, removed }; + }; + + const patch = Diff.structuredPatch( + fileName, + fileName, + oldStr, + aiStr, + 'Current', + 'Proposed', + DEFAULT_DIFF_OPTIONS, + ); + const { added: aiAddedLines, removed: aiRemovedLines } = countLines(patch); + + const userPatch = Diff.structuredPatch( + fileName, + fileName, + aiStr, + userStr, + 'Proposed', + 'User', + DEFAULT_DIFF_OPTIONS, + ); + const { added: userAddedLines, removed: userRemovedLines } = + countLines(userPatch); + + return { + ai_added_lines: aiAddedLines, + ai_removed_lines: aiRemovedLines, + user_added_lines: userAddedLines, + user_removed_lines: userRemovedLines, + }; +} diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index f1d0498a..33323203 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -25,7 +25,7 @@ import { makeRelative, shortenPath } from '../utils/paths.js'; import { isNodeError } from '../utils/errors.js'; import { Config, ApprovalMode } from '../config/config.js'; import { ensureCorrectEdit } from '../utils/editCorrector.js'; -import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js'; +import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js'; import { ReadFileTool } from './read-file.js'; import { ModifiableDeclarativeTool, ModifyContext } from './modifiable-tool.js'; import { IDEConnectionStatus } from '../ide/ide-client.js'; @@ -79,6 +79,11 @@ export interface EditToolParams { * Whether the edit was modified manually by the user. */ modified_by_user?: boolean; + + /** + * Initially proposed string. + */ + ai_proposed_string?: string; } interface CalculatedEdit { @@ -353,11 +358,20 @@ class EditToolInvocation implements ToolInvocation { 'Proposed', DEFAULT_DIFF_OPTIONS, ); + const originallyProposedContent = + this.params.ai_proposed_string || this.params.new_string; + const diffStat = getDiffStat( + fileName, + editData.currentContent ?? '', + originallyProposedContent, + this.params.new_string, + ); displayResult = { fileDiff, fileName, originalContent: editData.currentContent, newContent: editData.newContent, + diffStat, }; } @@ -513,12 +527,16 @@ Expectation for required parameters: oldContent: string, modifiedProposedContent: string, originalParams: EditToolParams, - ): EditToolParams => ({ - ...originalParams, - old_string: oldContent, - new_string: modifiedProposedContent, - modified_by_user: true, - }), + ): EditToolParams => { + const content = originalParams.new_string; + return { + ...originalParams, + ai_proposed_string: content, + old_string: oldContent, + new_string: modifiedProposedContent, + modified_by_user: true, + }; + }, }; } } diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index ceacd6ca..8e064973 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -498,6 +498,14 @@ export interface FileDiff { fileName: string; originalContent: string | null; newContent: string; + diffStat?: DiffStat; +} + +export interface DiffStat { + ai_removed_lines: number; + ai_added_lines: number; + user_added_lines: number; + user_removed_lines: number; } export interface ToolEditConfirmationDetails { diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 9e7e3813..4a9f8d35 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -25,7 +25,7 @@ import { ensureCorrectEdit, ensureCorrectFileContent, } from '../utils/editCorrector.js'; -import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js'; +import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js'; import { ModifiableDeclarativeTool, ModifyContext } from './modifiable-tool.js'; import { getSpecificMimeType } from '../utils/fileUtils.js'; import { @@ -52,6 +52,11 @@ export interface WriteFileToolParams { * Whether the proposed content was modified by the user. */ modified_by_user?: boolean; + + /** + * Initially proposed content. + */ + ai_proposed_content?: string; } interface GetCorrectedFileContentResult { @@ -283,6 +288,15 @@ export class WriteFileTool DEFAULT_DIFF_OPTIONS, ); + const originallyProposedContent = + params.ai_proposed_content || params.content; + const diffStat = getDiffStat( + fileName, + currentContentForDiff, + originallyProposedContent, + params.content, + ); + const llmSuccessMessageParts = [ isNewFile ? `Successfully created and wrote to new file: ${params.file_path}.` @@ -299,6 +313,7 @@ export class WriteFileTool fileName, originalContent: correctedContentResult.originalContent, newContent: correctedContentResult.correctedContent, + diffStat, }; const lines = fileContent.split('\n').length; @@ -311,6 +326,7 @@ export class WriteFileTool lines, mimetype, extension, + diffStat, ); } else { recordFileOperationMetric( @@ -319,6 +335,7 @@ export class WriteFileTool lines, mimetype, extension, + diffStat, ); } @@ -418,11 +435,15 @@ export class WriteFileTool _oldContent: string, modifiedProposedContent: string, originalParams: WriteFileToolParams, - ) => ({ - ...originalParams, - content: modifiedProposedContent, - modified_by_user: true, - }), + ) => { + const content = originalParams.content; + return { + ...originalParams, + ai_proposed_content: content, + content: modifiedProposedContent, + modified_by_user: true, + }; + }, }; } }