mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-12 12:54:07 -07:00
Fix duplicate LOC counting due to diff_stat being passed in multiple places (#7483)
This commit is contained in:
@@ -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);
|
const logEvent = this.createLogEvent(EventNames.FILE_OPERATION, data);
|
||||||
this.enqueueLogEvent(logEvent);
|
this.enqueueLogEvent(logEvent);
|
||||||
this.flushIfNeeded();
|
this.flushIfNeeded();
|
||||||
|
|||||||
@@ -194,7 +194,6 @@ export function logFileOperation(
|
|||||||
event.lines,
|
event.lines,
|
||||||
event.mimetype,
|
event.mimetype,
|
||||||
event.extension,
|
event.extension,
|
||||||
event.diff_stat,
|
|
||||||
event.programming_language,
|
event.programming_language,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -258,37 +258,25 @@ describe('Telemetry Metrics', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should include diffStat when provided', () => {
|
it('should record file operation without diffStat', () => {
|
||||||
initializeMetricsModule(mockConfig);
|
initializeMetricsModule(mockConfig);
|
||||||
mockCounterAddFn.mockClear();
|
mockCounterAddFn.mockClear();
|
||||||
|
|
||||||
const diffStat = {
|
|
||||||
ai_added_lines: 5,
|
|
||||||
ai_removed_lines: 2,
|
|
||||||
user_added_lines: 3,
|
|
||||||
user_removed_lines: 1,
|
|
||||||
};
|
|
||||||
|
|
||||||
recordFileOperationMetricModule(
|
recordFileOperationMetricModule(
|
||||||
mockConfig,
|
mockConfig,
|
||||||
FileOperation.UPDATE,
|
FileOperation.UPDATE,
|
||||||
undefined,
|
undefined,
|
||||||
undefined,
|
undefined,
|
||||||
undefined,
|
undefined,
|
||||||
diffStat,
|
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(mockCounterAddFn).toHaveBeenCalledWith(1, {
|
expect(mockCounterAddFn).toHaveBeenCalledWith(1, {
|
||||||
'session.id': 'test-session-id',
|
'session.id': 'test-session-id',
|
||||||
operation: FileOperation.UPDATE,
|
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);
|
initializeMetricsModule(mockConfig);
|
||||||
mockCounterAddFn.mockClear();
|
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);
|
initializeMetricsModule(mockConfig);
|
||||||
mockCounterAddFn.mockClear();
|
mockCounterAddFn.mockClear();
|
||||||
|
|
||||||
const diffStat = {
|
|
||||||
ai_added_lines: 0,
|
|
||||||
ai_removed_lines: 0,
|
|
||||||
user_added_lines: 0,
|
|
||||||
user_removed_lines: 0,
|
|
||||||
};
|
|
||||||
|
|
||||||
recordFileOperationMetricModule(
|
recordFileOperationMetricModule(
|
||||||
mockConfig,
|
mockConfig,
|
||||||
FileOperation.UPDATE,
|
FileOperation.UPDATE,
|
||||||
undefined,
|
undefined,
|
||||||
undefined,
|
undefined,
|
||||||
undefined,
|
undefined,
|
||||||
diffStat,
|
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(mockCounterAddFn).toHaveBeenCalledWith(1, {
|
expect(mockCounterAddFn).toHaveBeenCalledWith(1, {
|
||||||
'session.id': 'test-session-id',
|
'session.id': 'test-session-id',
|
||||||
operation: FileOperation.UPDATE,
|
operation: FileOperation.UPDATE,
|
||||||
ai_added_lines: 0,
|
|
||||||
ai_removed_lines: 0,
|
|
||||||
user_added_lines: 0,
|
|
||||||
user_removed_lines: 0,
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ import {
|
|||||||
METRIC_CONTENT_RETRY_FAILURE_COUNT,
|
METRIC_CONTENT_RETRY_FAILURE_COUNT,
|
||||||
} from './constants.js';
|
} from './constants.js';
|
||||||
import type { Config } from '../config/config.js';
|
import type { Config } from '../config/config.js';
|
||||||
import type { DiffStat } from '../tools/tools.js';
|
|
||||||
|
|
||||||
export enum FileOperation {
|
export enum FileOperation {
|
||||||
CREATE = 'create',
|
CREATE = 'create',
|
||||||
@@ -227,7 +226,6 @@ export function recordFileOperationMetric(
|
|||||||
lines?: number,
|
lines?: number,
|
||||||
mimetype?: string,
|
mimetype?: string,
|
||||||
extension?: string,
|
extension?: string,
|
||||||
diffStat?: DiffStat,
|
|
||||||
programming_language?: string,
|
programming_language?: string,
|
||||||
): void {
|
): void {
|
||||||
if (!fileOperationCounter || !isMetricsInitialized) return;
|
if (!fileOperationCounter || !isMetricsInitialized) return;
|
||||||
@@ -238,12 +236,6 @@ export function recordFileOperationMetric(
|
|||||||
if (lines !== undefined) attributes['lines'] = lines;
|
if (lines !== undefined) attributes['lines'] = lines;
|
||||||
if (mimetype !== undefined) attributes['mimetype'] = mimetype;
|
if (mimetype !== undefined) attributes['mimetype'] = mimetype;
|
||||||
if (extension !== undefined) attributes['extension'] = extension;
|
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) {
|
if (programming_language !== undefined) {
|
||||||
attributes['programming_language'] = programming_language;
|
attributes['programming_language'] = programming_language;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ import type { Config } from '../config/config.js';
|
|||||||
import type { ApprovalMode } from '../config/config.js';
|
import type { ApprovalMode } from '../config/config.js';
|
||||||
import type { CompletedToolCall } from '../core/coreToolScheduler.js';
|
import type { CompletedToolCall } from '../core/coreToolScheduler.js';
|
||||||
import { DiscoveredMCPTool } from '../tools/mcp-tool.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 { AuthType } from '../core/contentGenerator.js';
|
||||||
import {
|
import {
|
||||||
getDecisionFromOutcome,
|
getDecisionFromOutcome,
|
||||||
@@ -424,7 +424,6 @@ export class FileOperationEvent implements BaseTelemetryEvent {
|
|||||||
lines?: number;
|
lines?: number;
|
||||||
mimetype?: string;
|
mimetype?: string;
|
||||||
extension?: string;
|
extension?: string;
|
||||||
diff_stat?: DiffStat;
|
|
||||||
programming_language?: string;
|
programming_language?: string;
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
@@ -433,7 +432,6 @@ export class FileOperationEvent implements BaseTelemetryEvent {
|
|||||||
lines?: number,
|
lines?: number,
|
||||||
mimetype?: string,
|
mimetype?: string,
|
||||||
extension?: string,
|
extension?: string,
|
||||||
diff_stat?: DiffStat,
|
|
||||||
programming_language?: string,
|
programming_language?: string,
|
||||||
) {
|
) {
|
||||||
this['event.name'] = 'file_operation';
|
this['event.name'] = 'file_operation';
|
||||||
@@ -443,7 +441,6 @@ export class FileOperationEvent implements BaseTelemetryEvent {
|
|||||||
this.lines = lines;
|
this.lines = lines;
|
||||||
this.mimetype = mimetype;
|
this.mimetype = mimetype;
|
||||||
this.extension = extension;
|
this.extension = extension;
|
||||||
this.diff_stat = diff_stat;
|
|
||||||
this.programming_language = programming_language;
|
this.programming_language = programming_language;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -24,16 +24,16 @@ import { ApprovalMode } from '../config/config.js';
|
|||||||
import { ensureCorrectEdit } from '../utils/editCorrector.js';
|
import { ensureCorrectEdit } from '../utils/editCorrector.js';
|
||||||
import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js';
|
import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js';
|
||||||
import { ReadFileTool } from './read-file.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 {
|
import type {
|
||||||
ModifiableDeclarativeTool,
|
ModifiableDeclarativeTool,
|
||||||
ModifyContext,
|
ModifyContext,
|
||||||
} from './modifiable-tool.js';
|
} from './modifiable-tool.js';
|
||||||
import { IDEConnectionStatus } from '../ide/ide-client.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(
|
export function applyReplacement(
|
||||||
currentContent: string | null,
|
currentContent: string | null,
|
||||||
@@ -395,6 +395,28 @@ class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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 = [
|
const llmSuccessMessageParts = [
|
||||||
editData.isNewFile
|
editData.isNewFile
|
||||||
? `Created new file: ${this.params.file_path} with provided content.`
|
? `Created new file: ${this.params.file_path} with provided content.`
|
||||||
@@ -406,26 +428,6 @@ class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
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 {
|
return {
|
||||||
llmContent: llmSuccessMessageParts.join(' '),
|
llmContent: llmSuccessMessageParts.join(' '),
|
||||||
returnDisplay: displayResult,
|
returnDisplay: displayResult,
|
||||||
|
|||||||
@@ -117,7 +117,6 @@ ${result.llmContent}`;
|
|||||||
lines,
|
lines,
|
||||||
mimetype,
|
mimetype,
|
||||||
path.extname(this.params.absolute_path),
|
path.extname(this.params.absolute_path),
|
||||||
undefined,
|
|
||||||
programming_language,
|
programming_language,
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -443,7 +443,6 @@ ${finalExclusionPatternsForDescription
|
|||||||
lines,
|
lines,
|
||||||
mimetype,
|
mimetype,
|
||||||
path.extname(filePath),
|
path.extname(filePath),
|
||||||
undefined,
|
|
||||||
programming_language,
|
programming_language,
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -35,12 +35,12 @@ import type {
|
|||||||
ModifiableDeclarativeTool,
|
ModifiableDeclarativeTool,
|
||||||
ModifyContext,
|
ModifyContext,
|
||||||
} from './modifiable-tool.js';
|
} from './modifiable-tool.js';
|
||||||
import { getSpecificMimeType } from '../utils/fileUtils.js';
|
|
||||||
import { FileOperation } from '../telemetry/metrics.js';
|
|
||||||
import { IDEConnectionStatus } from '../ide/ide-client.js';
|
import { IDEConnectionStatus } from '../ide/ide-client.js';
|
||||||
import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js';
|
|
||||||
import { logFileOperation } from '../telemetry/loggers.js';
|
import { logFileOperation } from '../telemetry/loggers.js';
|
||||||
import { FileOperationEvent } from '../telemetry/types.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
|
* 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 = {
|
const displayResult: FileDiff = {
|
||||||
fileDiff,
|
fileDiff,
|
||||||
fileName,
|
fileName,
|
||||||
@@ -317,38 +335,6 @@ class WriteFileToolInvocation extends BaseToolInvocation<
|
|||||||
diffStat,
|
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 {
|
return {
|
||||||
llmContent: llmSuccessMessageParts.join(' '),
|
llmContent: llmSuccessMessageParts.join(' '),
|
||||||
returnDisplay: displayResult,
|
returnDisplay: displayResult,
|
||||||
|
|||||||
Reference in New Issue
Block a user