From 17044876f6dbd0cba0320eda45de1c5e526983e7 Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Sun, 31 Aug 2025 07:41:28 -0400 Subject: [PATCH] Fix duplicate LOC counting due to diff_stat being passed in multiple places (#7483) --- .../clearcut-logger/clearcut-logger.ts | 22 -------- packages/core/src/telemetry/loggers.ts | 1 - packages/core/src/telemetry/metrics.test.ts | 30 +--------- packages/core/src/telemetry/metrics.ts | 8 --- packages/core/src/telemetry/types.ts | 5 +- packages/core/src/tools/edit.ts | 52 ++++++++--------- packages/core/src/tools/read-file.ts | 1 - packages/core/src/tools/read-many-files.ts | 1 - packages/core/src/tools/write-file.ts | 56 +++++++------------ 9 files changed, 52 insertions(+), 124 deletions(-) diff --git a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts index 3280d25e18..bbff163697 100644 --- a/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts +++ b/packages/core/src/telemetry/clearcut-logger/clearcut-logger.ts @@ -517,28 +517,6 @@ export class ClearcutLogger { }); } - if (event.diff_stat) { - 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.diff_stat[key as keyof typeof event.diff_stat] !== undefined - ) { - data.push({ - gemini_cli_key, - value: JSON.stringify( - event.diff_stat[key as keyof typeof event.diff_stat], - ), - }); - } - } - } - const logEvent = this.createLogEvent(EventNames.FILE_OPERATION, data); this.enqueueLogEvent(logEvent); this.flushIfNeeded(); diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index 560cbe9c11..1dc6b2a344 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -194,7 +194,6 @@ export function logFileOperation( event.lines, event.mimetype, event.extension, - event.diff_stat, event.programming_language, ); } diff --git a/packages/core/src/telemetry/metrics.test.ts b/packages/core/src/telemetry/metrics.test.ts index 444c3db8c1..c48bde221f 100644 --- a/packages/core/src/telemetry/metrics.test.ts +++ b/packages/core/src/telemetry/metrics.test.ts @@ -258,37 +258,25 @@ describe('Telemetry Metrics', () => { }); }); - it('should include diffStat when provided', () => { + it('should record file operation without diffStat', () => { 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', () => { + it('should record minimal file operation when optional parameters are undefined', () => { initializeMetricsModule(mockConfig); mockCounterAddFn.mockClear(); @@ -310,33 +298,21 @@ describe('Telemetry Metrics', () => { }); }); - it('should handle diffStat with all zero values', () => { + it('should not include diffStat attributes when diffStat is not provided', () => { 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 59ffc52947..385ee076f7 100644 --- a/packages/core/src/telemetry/metrics.ts +++ b/packages/core/src/telemetry/metrics.ts @@ -21,7 +21,6 @@ import { METRIC_CONTENT_RETRY_FAILURE_COUNT, } from './constants.js'; import type { Config } from '../config/config.js'; -import type { DiffStat } from '../tools/tools.js'; export enum FileOperation { CREATE = 'create', @@ -227,7 +226,6 @@ export function recordFileOperationMetric( lines?: number, mimetype?: string, extension?: string, - diffStat?: DiffStat, programming_language?: string, ): void { if (!fileOperationCounter || !isMetricsInitialized) return; @@ -238,12 +236,6 @@ 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; - } if (programming_language !== undefined) { attributes['programming_language'] = programming_language; } diff --git a/packages/core/src/telemetry/types.ts b/packages/core/src/telemetry/types.ts index e5bc0a5226..46ca4338e9 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -9,7 +9,7 @@ import type { Config } from '../config/config.js'; import type { ApprovalMode } from '../config/config.js'; import type { CompletedToolCall } from '../core/coreToolScheduler.js'; import { DiscoveredMCPTool } from '../tools/mcp-tool.js'; -import type { DiffStat, FileDiff } from '../tools/tools.js'; +import type { FileDiff } from '../tools/tools.js'; import { AuthType } from '../core/contentGenerator.js'; import { getDecisionFromOutcome, @@ -424,7 +424,6 @@ export class FileOperationEvent implements BaseTelemetryEvent { lines?: number; mimetype?: string; extension?: string; - diff_stat?: DiffStat; programming_language?: string; constructor( @@ -433,7 +432,6 @@ export class FileOperationEvent implements BaseTelemetryEvent { lines?: number, mimetype?: string, extension?: string, - diff_stat?: DiffStat, programming_language?: string, ) { this['event.name'] = 'file_operation'; @@ -443,7 +441,6 @@ export class FileOperationEvent implements BaseTelemetryEvent { this.lines = lines; this.mimetype = mimetype; this.extension = extension; - this.diff_stat = diff_stat; this.programming_language = programming_language; } } diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index b1b51b7450..c9c67a0ab1 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -24,16 +24,16 @@ import { ApprovalMode } from '../config/config.js'; import { ensureCorrectEdit } from '../utils/editCorrector.js'; import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js'; import { ReadFileTool } from './read-file.js'; +import { logFileOperation } from '../telemetry/loggers.js'; +import { FileOperationEvent } from '../telemetry/types.js'; +import { FileOperation } from '../telemetry/metrics.js'; +import { getSpecificMimeType } from '../utils/fileUtils.js'; +import { getLanguageFromFilePath } from '../utils/language-detection.js'; import type { ModifiableDeclarativeTool, ModifyContext, } from './modifiable-tool.js'; import { IDEConnectionStatus } from '../ide/ide-client.js'; -import { FileOperation } from '../telemetry/metrics.js'; -import { logFileOperation } from '../telemetry/loggers.js'; -import { FileOperationEvent } from '../telemetry/types.js'; -import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js'; -import { getSpecificMimeType } from '../utils/fileUtils.js'; export function applyReplacement( currentContent: string | null, @@ -395,6 +395,28 @@ class EditToolInvocation implements ToolInvocation { }; } + // Log file operation for telemetry (without diff_stat to avoid double-counting) + const mimetype = getSpecificMimeType(this.params.file_path); + const programmingLanguage = getLanguageFromFilePath( + this.params.file_path, + ); + const extension = path.extname(this.params.file_path); + const operation = editData.isNewFile + ? FileOperation.CREATE + : FileOperation.UPDATE; + + logFileOperation( + this.config, + new FileOperationEvent( + EditTool.Name, + operation, + editData.newContent.split('\n').length, + mimetype, + extension, + programmingLanguage, + ), + ); + const llmSuccessMessageParts = [ editData.isNewFile ? `Created new file: ${this.params.file_path} with provided content.` @@ -406,26 +428,6 @@ class EditToolInvocation implements ToolInvocation { ); } - const lines = editData.newContent.split('\n').length; - const mimetype = getSpecificMimeType(this.params.file_path); - const extension = path.extname(this.params.file_path); - const programming_language = getProgrammingLanguage({ - file_path: this.params.file_path, - }); - - logFileOperation( - this.config, - new FileOperationEvent( - EditTool.Name, - editData.isNewFile ? FileOperation.CREATE : FileOperation.UPDATE, - lines, - mimetype, - extension, - diffStat, - programming_language, - ), - ); - return { llmContent: llmSuccessMessageParts.join(' '), returnDisplay: displayResult, diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 413747bc6a..21c84964e0 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -117,7 +117,6 @@ ${result.llmContent}`; lines, mimetype, path.extname(this.params.absolute_path), - undefined, programming_language, ), ); diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index a115d9f425..538e0d9056 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -443,7 +443,6 @@ ${finalExclusionPatternsForDescription lines, mimetype, path.extname(filePath), - undefined, programming_language, ), ); diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index ecaad35f0d..f72bf72db9 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -35,12 +35,12 @@ import type { ModifiableDeclarativeTool, ModifyContext, } from './modifiable-tool.js'; -import { getSpecificMimeType } from '../utils/fileUtils.js'; -import { FileOperation } from '../telemetry/metrics.js'; import { IDEConnectionStatus } from '../ide/ide-client.js'; -import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js'; import { logFileOperation } from '../telemetry/loggers.js'; import { FileOperationEvent } from '../telemetry/types.js'; +import { FileOperation } from '../telemetry/metrics.js'; +import { getSpecificMimeType } from '../utils/fileUtils.js'; +import { getLanguageFromFilePath } from '../utils/language-detection.js'; /** * Parameters for the WriteFile tool @@ -309,6 +309,24 @@ class WriteFileToolInvocation extends BaseToolInvocation< ); } + // Log file operation for telemetry (without diff_stat to avoid double-counting) + const mimetype = getSpecificMimeType(file_path); + const programmingLanguage = getLanguageFromFilePath(file_path); + const extension = path.extname(file_path); + const operation = isNewFile ? FileOperation.CREATE : FileOperation.UPDATE; + + logFileOperation( + this.config, + new FileOperationEvent( + WriteFileTool.Name, + operation, + fileContent.split('\n').length, + mimetype, + extension, + programmingLanguage, + ), + ); + const displayResult: FileDiff = { fileDiff, fileName, @@ -317,38 +335,6 @@ class WriteFileToolInvocation extends BaseToolInvocation< diffStat, }; - const lines = fileContent.split('\n').length; - const mimetype = getSpecificMimeType(file_path); - const extension = path.extname(file_path); // Get extension - const programming_language = getProgrammingLanguage({ file_path }); - if (isNewFile) { - logFileOperation( - this.config, - new FileOperationEvent( - WriteFileTool.Name, - FileOperation.CREATE, - lines, - mimetype, - extension, - diffStat, - programming_language, - ), - ); - } else { - logFileOperation( - this.config, - new FileOperationEvent( - WriteFileTool.Name, - FileOperation.UPDATE, - lines, - mimetype, - extension, - diffStat, - programming_language, - ), - ); - } - return { llmContent: llmSuccessMessageParts.join(' '), returnDisplay: displayResult,