Skip to content

Commit 5a95706

Browse files
gundermanckunal-10-cloud
authored andcommitted
feat(telemetry): include language in telemetry and fix accepted lines computation (google-gemini#21126)
1 parent d71ea3f commit 5a95706

File tree

5 files changed

+155
-41
lines changed

5 files changed

+155
-41
lines changed

packages/core/src/code_assist/server.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ describe('CodeAssistServer', () => {
116116
role: 'model',
117117
parts: [
118118
{ text: 'response' },
119-
{ functionCall: { name: 'test', args: {} } },
119+
{ functionCall: { name: 'replace', args: {} } },
120120
],
121121
},
122122
finishReason: FinishReason.SAFETY,
@@ -160,7 +160,7 @@ describe('CodeAssistServer', () => {
160160
role: 'model',
161161
parts: [
162162
{ text: 'response' },
163-
{ functionCall: { name: 'test', args: {} } },
163+
{ functionCall: { name: 'replace', args: {} } },
164164
],
165165
},
166166
finishReason: FinishReason.STOP,
@@ -233,7 +233,7 @@ describe('CodeAssistServer', () => {
233233
content: {
234234
parts: [
235235
{ text: 'chunk' },
236-
{ functionCall: { name: 'test', args: {} } },
236+
{ functionCall: { name: 'replace', args: {} } },
237237
],
238238
},
239239
},

packages/core/src/code_assist/telemetry.test.ts

Lines changed: 93 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ describe('telemetry', () => {
8282
},
8383
],
8484
true,
85-
[{ name: 'someTool', args: {} }],
85+
[{ name: 'replace', args: {} }],
8686
);
8787
const traceId = 'test-trace-id';
8888
const streamingLatency: StreamingLatency = { totalLatency: '1s' };
@@ -130,7 +130,7 @@ describe('telemetry', () => {
130130

131131
it('should set status to CANCELLED if signal is aborted', () => {
132132
const response = createMockResponse([], true, [
133-
{ name: 'tool', args: {} },
133+
{ name: 'replace', args: {} },
134134
]);
135135
const signal = new AbortController().signal;
136136
vi.spyOn(signal, 'aborted', 'get').mockReturnValue(true);
@@ -147,7 +147,7 @@ describe('telemetry', () => {
147147

148148
it('should set status to ERROR_UNKNOWN if response has error (non-OK SDK response)', () => {
149149
const response = createMockResponse([], false, [
150-
{ name: 'tool', args: {} },
150+
{ name: 'replace', args: {} },
151151
]);
152152

153153
const result = createConversationOffered(
@@ -169,7 +169,7 @@ describe('telemetry', () => {
169169
},
170170
],
171171
true,
172-
[{ name: 'tool', args: {} }],
172+
[{ name: 'replace', args: {} }],
173173
);
174174

175175
const result = createConversationOffered(
@@ -186,7 +186,7 @@ describe('telemetry', () => {
186186
// We force functionCalls to be present to bypass the guard,
187187
// simulating a state where we want to test the candidates check.
188188
const response = createMockResponse([], true, [
189-
{ name: 'tool', args: {} },
189+
{ name: 'replace', args: {} },
190190
]);
191191

192192
const result = createConversationOffered(
@@ -212,7 +212,7 @@ describe('telemetry', () => {
212212
},
213213
],
214214
true,
215-
[{ name: 'tool', args: {} }],
215+
[{ name: 'replace', args: {} }],
216216
);
217217
const result = createConversationOffered(response, 'id', undefined, {});
218218
expect(result?.includedCode).toBe(true);
@@ -229,7 +229,7 @@ describe('telemetry', () => {
229229
},
230230
],
231231
true,
232-
[{ name: 'tool', args: {} }],
232+
[{ name: 'replace', args: {} }],
233233
);
234234
const result = createConversationOffered(response, 'id', undefined, {});
235235
expect(result?.includedCode).toBe(false);
@@ -250,7 +250,7 @@ describe('telemetry', () => {
250250
} as unknown as CodeAssistServer;
251251

252252
const response = createMockResponse([], true, [
253-
{ name: 'tool', args: {} },
253+
{ name: 'replace', args: {} },
254254
]);
255255
const streamingLatency = {};
256256

@@ -274,7 +274,7 @@ describe('telemetry', () => {
274274
recordConversationOffered: vi.fn(),
275275
} as unknown as CodeAssistServer;
276276
const response = createMockResponse([], true, [
277-
{ name: 'tool', args: {} },
277+
{ name: 'replace', args: {} },
278278
]);
279279

280280
await recordConversationOffered(
@@ -331,17 +331,89 @@ describe('telemetry', () => {
331331

332332
await recordToolCallInteractions({} as Config, toolCalls);
333333

334-
expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith({
335-
traceId: 'trace-1',
336-
status: ActionStatus.ACTION_STATUS_NO_ERROR,
337-
interaction: ConversationInteractionInteraction.ACCEPT_FILE,
338-
acceptedLines: '5',
339-
removedLines: '3',
340-
isAgentic: true,
341-
});
334+
expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith(
335+
expect.objectContaining({
336+
traceId: 'trace-1',
337+
status: ActionStatus.ACTION_STATUS_NO_ERROR,
338+
interaction: ConversationInteractionInteraction.ACCEPT_FILE,
339+
acceptedLines: '8',
340+
removedLines: '3',
341+
isAgentic: true,
342+
}),
343+
);
342344
});
343345

344-
it('should record UNKNOWN interaction for other accepted tools', async () => {
346+
it('should include language in interaction if file_path is present', async () => {
347+
const toolCalls: CompletedToolCall[] = [
348+
{
349+
request: {
350+
name: 'replace',
351+
args: {
352+
file_path: 'test.ts',
353+
old_string: 'old',
354+
new_string: 'new',
355+
},
356+
callId: 'call-1',
357+
isClientInitiated: false,
358+
prompt_id: 'p1',
359+
traceId: 'trace-1',
360+
},
361+
response: {
362+
resultDisplay: {
363+
diffStat: {
364+
model_added_lines: 5,
365+
model_removed_lines: 3,
366+
},
367+
},
368+
},
369+
outcome: ToolConfirmationOutcome.ProceedOnce,
370+
status: 'success',
371+
} as unknown as CompletedToolCall,
372+
];
373+
374+
await recordToolCallInteractions({} as Config, toolCalls);
375+
376+
expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith(
377+
expect.objectContaining({
378+
language: 'TypeScript',
379+
}),
380+
);
381+
});
382+
383+
it('should include language in interaction if write_file is used', async () => {
384+
const toolCalls: CompletedToolCall[] = [
385+
{
386+
request: {
387+
name: 'write_file',
388+
args: { file_path: 'test.py', content: 'test' },
389+
callId: 'call-1',
390+
isClientInitiated: false,
391+
prompt_id: 'p1',
392+
traceId: 'trace-1',
393+
},
394+
response: {
395+
resultDisplay: {
396+
diffStat: {
397+
model_added_lines: 5,
398+
model_removed_lines: 3,
399+
},
400+
},
401+
},
402+
outcome: ToolConfirmationOutcome.ProceedOnce,
403+
status: 'success',
404+
} as unknown as CompletedToolCall,
405+
];
406+
407+
await recordToolCallInteractions({} as Config, toolCalls);
408+
409+
expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith(
410+
expect.objectContaining({
411+
language: 'Python',
412+
}),
413+
);
414+
});
415+
416+
it('should not record interaction for other accepted tools', async () => {
345417
const toolCalls: CompletedToolCall[] = [
346418
{
347419
request: {
@@ -359,19 +431,14 @@ describe('telemetry', () => {
359431

360432
await recordToolCallInteractions({} as Config, toolCalls);
361433

362-
expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith({
363-
traceId: 'trace-2',
364-
status: ActionStatus.ACTION_STATUS_NO_ERROR,
365-
interaction: ConversationInteractionInteraction.UNKNOWN,
366-
isAgentic: true,
367-
});
434+
expect(mockServer.recordConversationInteraction).not.toHaveBeenCalled();
368435
});
369436

370437
it('should not record interaction for cancelled status', async () => {
371438
const toolCalls: CompletedToolCall[] = [
372439
{
373440
request: {
374-
name: 'tool',
441+
name: 'replace',
375442
args: {},
376443
callId: 'call-3',
377444
isClientInitiated: false,
@@ -394,7 +461,7 @@ describe('telemetry', () => {
394461
const toolCalls: CompletedToolCall[] = [
395462
{
396463
request: {
397-
name: 'tool',
464+
name: 'replace',
398465
args: {},
399466
callId: 'call-4',
400467
isClientInitiated: false,

packages/core/src/code_assist/telemetry.ts

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,13 @@ import { EDIT_TOOL_NAMES } from '../tools/tool-names.js';
2222
import { getErrorMessage } from '../utils/errors.js';
2323
import type { CodeAssistServer } from './server.js';
2424
import { ToolConfirmationOutcome } from '../tools/tools.js';
25+
import { getLanguageFromFilePath } from '../utils/language-detection.js';
2526
import {
2627
computeModelAddedAndRemovedLines,
2728
getFileDiffFromResultDisplay,
2829
} from '../utils/fileDiffUtils.js';
30+
import { isEditToolParams } from '../tools/edit.js';
31+
import { isWriteFileToolParams } from '../tools/write-file.js';
2932

3033
export async function recordConversationOffered(
3134
server: CodeAssistServer,
@@ -85,10 +88,12 @@ export function createConversationOffered(
8588
signal: AbortSignal | undefined,
8689
streamingLatency: StreamingLatency,
8790
): ConversationOffered | undefined {
88-
// Only send conversation offered events for responses that contain function
89-
// calls. Non-function call events don't represent user actionable
90-
// 'suggestions'.
91-
if ((response.functionCalls?.length || 0) === 0) {
91+
// Only send conversation offered events for responses that contain edit
92+
// function calls. Non-edit function calls don't represent file modifications.
93+
if (
94+
!response.functionCalls ||
95+
!response.functionCalls.some((call) => EDIT_TOOL_NAMES.has(call.name || ''))
96+
) {
9297
return;
9398
}
9499

@@ -116,6 +121,7 @@ function summarizeToolCalls(
116121
let isEdit = false;
117122
let acceptedLines = 0;
118123
let removedLines = 0;
124+
let language = undefined;
119125

120126
// Iterate the tool calls and summarize them into a single conversation
121127
// interaction so that the ConversationOffered and ConversationInteraction
@@ -144,30 +150,40 @@ function summarizeToolCalls(
144150
if (EDIT_TOOL_NAMES.has(toolCall.request.name)) {
145151
isEdit = true;
146152

153+
if (
154+
!language &&
155+
(isEditToolParams(toolCall.request.args) ||
156+
isWriteFileToolParams(toolCall.request.args))
157+
) {
158+
language = getLanguageFromFilePath(toolCall.request.args.file_path);
159+
}
160+
147161
if (toolCall.status === 'success') {
148162
const fileDiff = getFileDiffFromResultDisplay(
149163
toolCall.response.resultDisplay,
150164
);
151165
if (fileDiff?.diffStat) {
152166
const lines = computeModelAddedAndRemovedLines(fileDiff.diffStat);
153-
acceptedLines += lines.addedLines;
167+
168+
// The API expects acceptedLines to be addedLines + removedLines.
169+
acceptedLines += lines.addedLines + lines.removedLines;
154170
removedLines += lines.removedLines;
155171
}
156172
}
157173
}
158174
}
159175
}
160176

161-
// Only file interaction telemetry if 100% of the tool calls were accepted.
162-
return traceId && acceptedToolCalls / toolCalls.length >= 1
177+
// Only file interaction telemetry if 100% of the tool calls were accepted
178+
// and at least one of them was an edit.
179+
return traceId && acceptedToolCalls / toolCalls.length >= 1 && isEdit
163180
? createConversationInteraction(
164181
traceId,
165182
actionStatus || ActionStatus.ACTION_STATUS_NO_ERROR,
166-
isEdit
167-
? ConversationInteractionInteraction.ACCEPT_FILE
168-
: ConversationInteractionInteraction.UNKNOWN,
169-
isEdit ? String(acceptedLines) : undefined,
170-
isEdit ? String(removedLines) : undefined,
183+
ConversationInteractionInteraction.ACCEPT_FILE,
184+
String(acceptedLines),
185+
String(removedLines),
186+
language,
171187
)
172188
: undefined;
173189
}
@@ -178,16 +194,19 @@ function createConversationInteraction(
178194
interaction: ConversationInteractionInteraction,
179195
acceptedLines?: string,
180196
removedLines?: string,
197+
language?: string,
181198
): ConversationInteraction {
182199
return {
183200
traceId,
184201
status,
185202
interaction,
186203
acceptedLines,
187204
removedLines,
205+
language,
188206
isAgentic: true,
189207
};
190208
}
209+
191210
function includesCode(resp: GenerateContentResponse): boolean {
192211
if (!resp.candidates) {
193212
return false;

packages/core/src/tools/edit.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,20 @@ export interface EditToolParams {
413413
ai_proposed_content?: string;
414414
}
415415

416+
export function isEditToolParams(args: unknown): args is EditToolParams {
417+
if (typeof args !== 'object' || args === null) {
418+
return false;
419+
}
420+
return (
421+
'file_path' in args &&
422+
typeof args.file_path === 'string' &&
423+
'old_string' in args &&
424+
typeof args.old_string === 'string' &&
425+
'new_string' in args &&
426+
typeof args.new_string === 'string'
427+
);
428+
}
429+
416430
interface CalculatedEdit {
417431
currentContent: string | null;
418432
newContent: string;

packages/core/src/tools/write-file.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,20 @@ export interface WriteFileToolParams {
7474
ai_proposed_content?: string;
7575
}
7676

77+
export function isWriteFileToolParams(
78+
args: unknown,
79+
): args is WriteFileToolParams {
80+
if (typeof args !== 'object' || args === null) {
81+
return false;
82+
}
83+
return (
84+
'file_path' in args &&
85+
typeof args.file_path === 'string' &&
86+
'content' in args &&
87+
typeof args.content === 'string'
88+
);
89+
}
90+
7791
interface GetCorrectedFileContentResult {
7892
originalContent: string;
7993
correctedContent: string;

0 commit comments

Comments
 (0)