diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 08ea421ab3..d76503822e 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -27,6 +27,7 @@ import { coreEvents, CoreEvent, createWorkingStdio, + recordToolCallInteractions, } from '@google/gemini-cli-core'; import type { Content, Part } from '@google/genai'; @@ -407,6 +408,8 @@ export async function runNonInteractive({ geminiClient .getChat() .recordCompletedToolCalls(currentModel, completedToolCalls); + + await recordToolCallInteractions(config, completedToolCalls); } catch (error) { debugLogger.error( `Error recording completed tool call information: ${error}`, diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 81922684e3..3ab2cbce5f 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -38,6 +38,7 @@ import { runInDevTraceSpan, EDIT_TOOL_NAMES, processRestorableToolCalls, + recordToolCallInteractions, } from '@google/gemini-cli-core'; import { type Part, type PartListUnion, FinishReason } from '@google/genai'; import type { @@ -163,6 +164,11 @@ export const useGeminiStream = ( currentModel, completedToolCallsFromScheduler, ); + + await recordToolCallInteractions( + config, + completedToolCallsFromScheduler, + ); } catch (error) { debugLogger.warn( `Error recording completed tool call information: ${error}`, diff --git a/packages/core/src/code_assist/server.test.ts b/packages/core/src/code_assist/server.test.ts index 321239df96..91873a796e 100644 --- a/packages/core/src/code_assist/server.test.ts +++ b/packages/core/src/code_assist/server.test.ts @@ -102,7 +102,10 @@ describe('CodeAssistServer', () => { index: 0, content: { role: 'model', - parts: [{ text: 'response' }], + parts: [ + { text: 'response' }, + { functionCall: { name: 'test', args: {} } }, + ], }, finishReason: FinishReason.SAFETY, safetyRatings: [], @@ -142,7 +145,10 @@ describe('CodeAssistServer', () => { index: 0, content: { role: 'model', - parts: [{ text: 'response' }], + parts: [ + { text: 'response' }, + { functionCall: { name: 'test', args: {} } }, + ], }, finishReason: FinishReason.STOP, safetyRatings: [], @@ -204,7 +210,16 @@ describe('CodeAssistServer', () => { const mockResponseData = { traceId: 'stream-trace-id', response: { - candidates: [{ content: { parts: [{ text: 'chunk' }] } }], + candidates: [ + { + content: { + parts: [ + { text: 'chunk' }, + { functionCall: { name: 'test', args: {} } }, + ], + }, + }, + ], sdkHttpResponse: { responseInternal: { ok: true, diff --git a/packages/core/src/code_assist/server.ts b/packages/core/src/code_assist/server.ts index e6f7009f7b..ccebc80977 100644 --- a/packages/core/src/code_assist/server.ts +++ b/packages/core/src/code_assist/server.ts @@ -47,8 +47,8 @@ import { toGenerateContentRequest, } from './converter.js'; import { - createConversationOffered, formatProtoJsonDuration, + recordConversationOffered, } from './telemetry.js'; import { getClientMetadata } from './experiments/client_metadata.js'; @@ -107,15 +107,13 @@ export class CodeAssistServer implements ContentGenerator { const translatedResponse = fromGenerateContentResponse(response); - if (response.traceId) { - const offered = createConversationOffered( - translatedResponse, - response.traceId, - req.config?.abortSignal, - streamingLatency, - ); - await server.recordConversationOffered(offered); - } + await recordConversationOffered( + server, + response.traceId, + translatedResponse, + streamingLatency, + req.config?.abortSignal, + ); yield translatedResponse; } @@ -145,15 +143,13 @@ export class CodeAssistServer implements ContentGenerator { const translatedResponse = fromGenerateContentResponse(response); - if (response.traceId) { - const offered = createConversationOffered( - translatedResponse, - response.traceId, - req.config?.abortSignal, - streamingLatency, - ); - await this.recordConversationOffered(offered); - } + await recordConversationOffered( + this, + response.traceId, + translatedResponse, + streamingLatency, + req.config?.abortSignal, + ); return translatedResponse; } diff --git a/packages/core/src/code_assist/telemetry.test.ts b/packages/core/src/code_assist/telemetry.test.ts index 1f6eaff152..d6a821a0b0 100644 --- a/packages/core/src/code_assist/telemetry.test.ts +++ b/packages/core/src/code_assist/telemetry.test.ts @@ -4,17 +4,38 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { createConversationOffered, formatProtoJsonDuration, + recordConversationOffered, + recordToolCallInteractions, } from './telemetry.js'; -import { ActionStatus, type StreamingLatency } from './types.js'; -import { FinishReason, GenerateContentResponse } from '@google/genai'; +import { + ActionStatus, + ConversationInteractionInteraction, + type StreamingLatency, +} from './types.js'; +import { + FinishReason, + GenerateContentResponse, + type FunctionCall, +} from '@google/genai'; +import * as codeAssist from './codeAssist.js'; +import type { CodeAssistServer } from './server.js'; +import type { CompletedToolCall } from '../core/coreToolScheduler.js'; +import { + ToolConfirmationOutcome, + type AnyDeclarativeTool, + type AnyToolInvocation, +} from '../tools/tools.js'; +import type { Config } from '../config/config.js'; +import type { ToolCallResponseInfo } from '../core/turn.js'; function createMockResponse( candidates: GenerateContentResponse['candidates'] = [], ok = true, + functionCalls: FunctionCall[] | undefined = undefined, ) { const response = new GenerateContentResponse(); response.candidates = candidates; @@ -24,27 +45,44 @@ function createMockResponse( } as unknown as Response, json: async () => ({}), }; + + // If functionCalls is explicitly provided, mock the getter. + // Otherwise, let the default behavior (if any) or undefined prevail. + // In the real SDK, functionCalls is a getter derived from candidates. + // For testing `createConversationOffered` which guards on functionCalls, + // we often need to force it to be present. + if (functionCalls !== undefined) { + Object.defineProperty(response, 'functionCalls', { + get: () => functionCalls, + configurable: true, + }); + } + return response; } describe('telemetry', () => { describe('createConversationOffered', () => { it('should create a ConversationOffered object with correct values', () => { - const response = createMockResponse([ - { - index: 0, - content: { - role: 'model', - parts: [{ text: 'response with ```code```' }], + const response = createMockResponse( + [ + { + index: 0, + content: { + role: 'model', + parts: [{ text: 'response with ```code```' }], + }, + citationMetadata: { + citations: [ + { uri: 'https://example.com', startIndex: 0, endIndex: 10 }, + ], + }, + finishReason: FinishReason.STOP, }, - citationMetadata: { - citations: [ - { uri: 'https://example.com', startIndex: 0, endIndex: 10 }, - ], - }, - finishReason: FinishReason.STOP, - }, - ]); + ], + true, + [{ name: 'someTool', args: {} }], + ); const traceId = 'test-trace-id'; const streamingLatency: StreamingLatency = { totalLatency: '1s' }; @@ -65,8 +103,33 @@ describe('telemetry', () => { }); }); + it('should return undefined if no function calls', () => { + const response = createMockResponse( + [ + { + index: 0, + content: { + role: 'model', + parts: [{ text: 'response without function calls' }], + }, + }, + ], + true, + [], // Empty function calls + ); + const result = createConversationOffered( + response, + 'trace-id', + undefined, + {}, + ); + expect(result).toBeUndefined(); + }); + it('should set status to CANCELLED if signal is aborted', () => { - const response = createMockResponse(); + const response = createMockResponse([], true, [ + { name: 'tool', args: {} }, + ]); const signal = new AbortController().signal; vi.spyOn(signal, 'aborted', 'get').mockReturnValue(true); @@ -77,11 +140,13 @@ describe('telemetry', () => { {}, ); - expect(result.status).toBe(ActionStatus.ACTION_STATUS_CANCELLED); + expect(result?.status).toBe(ActionStatus.ACTION_STATUS_CANCELLED); }); it('should set status to ERROR_UNKNOWN if response has error (non-OK SDK response)', () => { - const response = createMockResponse([], false); + const response = createMockResponse([], false, [ + { name: 'tool', args: {} }, + ]); const result = createConversationOffered( response, @@ -90,16 +155,20 @@ describe('telemetry', () => { {}, ); - expect(result.status).toBe(ActionStatus.ACTION_STATUS_ERROR_UNKNOWN); + expect(result?.status).toBe(ActionStatus.ACTION_STATUS_ERROR_UNKNOWN); }); it('should set status to ERROR_UNKNOWN if finishReason is not STOP or MAX_TOKENS', () => { - const response = createMockResponse([ - { - index: 0, - finishReason: FinishReason.SAFETY, - }, - ]); + const response = createMockResponse( + [ + { + index: 0, + finishReason: FinishReason.SAFETY, + }, + ], + true, + [{ name: 'tool', args: {} }], + ); const result = createConversationOffered( response, @@ -108,11 +177,15 @@ describe('telemetry', () => { {}, ); - expect(result.status).toBe(ActionStatus.ACTION_STATUS_ERROR_UNKNOWN); + expect(result?.status).toBe(ActionStatus.ACTION_STATUS_ERROR_UNKNOWN); }); it('should set status to EMPTY if candidates is empty', () => { - const response = createMockResponse(); + // We force functionCalls to be present to bypass the guard, + // simulating a state where we want to test the candidates check. + const response = createMockResponse([], true, [ + { name: 'tool', args: {} }, + ]); const result = createConversationOffered( response, @@ -121,35 +194,43 @@ describe('telemetry', () => { {}, ); - expect(result.status).toBe(ActionStatus.ACTION_STATUS_EMPTY); + expect(result?.status).toBe(ActionStatus.ACTION_STATUS_EMPTY); }); it('should detect code in response', () => { - const response = createMockResponse([ - { - index: 0, - content: { - parts: [ - { text: 'Here is some code:\n```js\nconsole.log("hi")\n```' }, - ], + const response = createMockResponse( + [ + { + index: 0, + content: { + parts: [ + { text: 'Here is some code:\n```js\nconsole.log("hi")\n```' }, + ], + }, }, - }, - ]); + ], + true, + [{ name: 'tool', args: {} }], + ); const result = createConversationOffered(response, 'id', undefined, {}); - expect(result.includedCode).toBe(true); + expect(result?.includedCode).toBe(true); }); it('should not detect code if no backticks', () => { - const response = createMockResponse([ - { - index: 0, - content: { - parts: [{ text: 'Here is some text.' }], + const response = createMockResponse( + [ + { + index: 0, + content: { + parts: [{ text: 'Here is some text.' }], + }, }, - }, - ]); + ], + true, + [{ name: 'tool', args: {} }], + ); const result = createConversationOffered(response, 'id', undefined, {}); - expect(result.includedCode).toBe(false); + expect(result?.includedCode).toBe(false); }); }); @@ -159,4 +240,199 @@ describe('telemetry', () => { expect(formatProtoJsonDuration(100)).toBe('0.1s'); }); }); + + describe('recordConversationOffered', () => { + it('should call server.recordConversationOffered if traceId is present', async () => { + const serverMock = { + recordConversationOffered: vi.fn(), + } as unknown as CodeAssistServer; + + const response = createMockResponse([], true, [ + { name: 'tool', args: {} }, + ]); + const streamingLatency = {}; + + await recordConversationOffered( + serverMock, + 'trace-id', + response, + streamingLatency, + undefined, + ); + + expect(serverMock.recordConversationOffered).toHaveBeenCalledWith( + expect.objectContaining({ + traceId: 'trace-id', + }), + ); + }); + + it('should not call server.recordConversationOffered if traceId is undefined', async () => { + const serverMock = { + recordConversationOffered: vi.fn(), + } as unknown as CodeAssistServer; + const response = createMockResponse([], true, [ + { name: 'tool', args: {} }, + ]); + + await recordConversationOffered( + serverMock, + undefined, + response, + {}, + undefined, + ); + + expect(serverMock.recordConversationOffered).not.toHaveBeenCalled(); + }); + }); + + describe('recordToolCallInteractions', () => { + let mockServer: { recordConversationInteraction: ReturnType }; + + beforeEach(() => { + mockServer = { + recordConversationInteraction: vi.fn(), + }; + vi.spyOn(codeAssist, 'getCodeAssistServer').mockReturnValue( + mockServer as unknown as CodeAssistServer, + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should record ACCEPT_FILE interaction for accepted edit tools', async () => { + const toolCalls: CompletedToolCall[] = [ + { + request: { + name: 'replace', // in EDIT_TOOL_NAMES + args: {}, + callId: 'call-1', + isClientInitiated: false, + prompt_id: 'p1', + traceId: 'trace-1', + }, + outcome: ToolConfirmationOutcome.ProceedOnce, + status: 'success', + } as unknown as CompletedToolCall, + ]; + + await recordToolCallInteractions({} as Config, toolCalls); + + expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith({ + traceId: 'trace-1', + status: ActionStatus.ACTION_STATUS_NO_ERROR, + interaction: ConversationInteractionInteraction.ACCEPT_FILE, + isAgentic: true, + }); + }); + + it('should record UNKNOWN interaction for other accepted tools', async () => { + const toolCalls: CompletedToolCall[] = [ + { + request: { + name: 'read_file', // NOT in EDIT_TOOL_NAMES + args: {}, + callId: 'call-2', + isClientInitiated: false, + prompt_id: 'p2', + traceId: 'trace-2', + }, + outcome: ToolConfirmationOutcome.ProceedOnce, + status: 'success', + } as unknown as CompletedToolCall, + ]; + + await recordToolCallInteractions({} as Config, toolCalls); + + expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith({ + traceId: 'trace-2', + status: ActionStatus.ACTION_STATUS_NO_ERROR, + interaction: ConversationInteractionInteraction.UNKNOWN, + isAgentic: true, + }); + }); + + it('should not record interaction for cancelled status', async () => { + const toolCalls: CompletedToolCall[] = [ + { + request: { + name: 'tool', + args: {}, + callId: 'call-3', + isClientInitiated: false, + prompt_id: 'p3', + traceId: 'trace-3', + }, + status: 'cancelled', + response: {} as unknown as ToolCallResponseInfo, + tool: {} as unknown as AnyDeclarativeTool, + invocation: {} as unknown as AnyToolInvocation, + } as CompletedToolCall, + ]; + + await recordToolCallInteractions({} as Config, toolCalls); + + expect(mockServer.recordConversationInteraction).not.toHaveBeenCalled(); + }); + + it('should not record interaction for error status', async () => { + const toolCalls: CompletedToolCall[] = [ + { + request: { + name: 'tool', + args: {}, + callId: 'call-4', + isClientInitiated: false, + prompt_id: 'p4', + traceId: 'trace-4', + }, + status: 'error', + response: { + error: new Error('fail'), + } as unknown as ToolCallResponseInfo, + } as CompletedToolCall, + ]; + + await recordToolCallInteractions({} as Config, toolCalls); + + expect(mockServer.recordConversationInteraction).not.toHaveBeenCalled(); + }); + + it('should not record interaction if tool calls are mixed or not 100% accepted', async () => { + // Logic: traceId && acceptedToolCalls / toolCalls.length >= 1 + const toolCalls: CompletedToolCall[] = [ + { + request: { + name: 't1', + args: {}, + callId: 'c1', + isClientInitiated: false, + prompt_id: 'p1', + traceId: 't1', + }, + outcome: ToolConfirmationOutcome.ProceedOnce, + status: 'success', + }, + { + request: { + name: 't2', + args: {}, + callId: 'c2', + isClientInitiated: false, + prompt_id: 'p1', + traceId: 't1', + }, + outcome: ToolConfirmationOutcome.Cancel, // Rejected + status: 'success', + }, + ] as unknown as CompletedToolCall[]; + + await recordToolCallInteractions({} as Config, toolCalls); + + expect(mockServer.recordConversationInteraction).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/core/src/code_assist/telemetry.ts b/packages/core/src/code_assist/telemetry.ts index bda72a4da1..e2184598c0 100644 --- a/packages/core/src/code_assist/telemetry.ts +++ b/packages/core/src/code_assist/telemetry.ts @@ -8,17 +8,86 @@ import { FinishReason, type GenerateContentResponse } from '@google/genai'; import { getCitations } from '../utils/generateContentResponseUtilities.js'; import { ActionStatus, + ConversationInteractionInteraction, + type ConversationInteraction, type ConversationOffered, type StreamingLatency, } from './types.js'; +import type { CompletedToolCall } from '../core/coreToolScheduler.js'; +import type { Config } from '../config/config.js'; +import { debugLogger } from '../utils/debugLogger.js'; +import { getCodeAssistServer } from './codeAssist.js'; +import { EDIT_TOOL_NAMES } from '../tools/tool-names.js'; +import { getErrorMessage } from '../utils/errors.js'; +import type { CodeAssistServer } from './server.js'; +import { ToolConfirmationOutcome } from '../tools/tools.js'; + +export async function recordConversationOffered( + server: CodeAssistServer, + traceId: string | undefined, + response: GenerateContentResponse, + streamingLatency: StreamingLatency, + abortSignal: AbortSignal | undefined, +): Promise { + try { + if (traceId) { + const offered = createConversationOffered( + response, + traceId, + abortSignal, + streamingLatency, + ); + if (offered) { + await server.recordConversationOffered(offered); + } + } + } catch (error: unknown) { + debugLogger.warn( + `Error recording tool call interactions: ${getErrorMessage(error)}`, + ); + } +} + +export async function recordToolCallInteractions( + config: Config, + toolCalls: CompletedToolCall[], +): Promise { + // Only send interaction events for responses that contain function calls. + if (toolCalls.length === 0) { + return; + } + + try { + const server = getCodeAssistServer(config); + if (!server) { + return; + } + + const interaction = summarizeToolCalls(toolCalls); + if (interaction) { + await server.recordConversationInteraction(interaction); + } + } catch (error: unknown) { + debugLogger.warn( + `Error recording tool call interactions: ${getErrorMessage(error)}`, + ); + } +} export function createConversationOffered( response: GenerateContentResponse, traceId: string, signal: AbortSignal | undefined, streamingLatency: StreamingLatency, -): ConversationOffered { - const actionStatus = getStatus(response, signal); +): ConversationOffered | undefined { + // Only send conversation offered events for responses that contain function + // calls. Non-function call events don't represent user actionable + // 'suggestions'. + if ((response.functionCalls?.length || 0) === 0) { + return; + } + + const actionStatus = getStatusFromResponse(response, signal); return { citationCount: String(getCitations(response).length), @@ -30,6 +99,71 @@ export function createConversationOffered( }; } +function summarizeToolCalls( + toolCalls: CompletedToolCall[], +): ConversationInteraction | undefined { + let acceptedToolCalls = 0; + let actionStatus = undefined; + let traceId = undefined; + + // Treat file edits as ACCEPT_FILE and everything else as unknown. + let isEdit = false; + + // Iterate the tool calls and summarize them into a single conversation + // interaction so that the ConversationOffered and ConversationInteraction + // events are 1:1 in telemetry. + for (const toolCall of toolCalls) { + traceId ||= toolCall.request.traceId; + + // If any tool call is canceled, we treat the entire interaction as canceled. + if (toolCall.status === 'cancelled') { + actionStatus = ActionStatus.ACTION_STATUS_CANCELLED; + break; + } + + // If any tool call encounters an error, we treat the entire interaction as + // having errored. + if (toolCall.status === 'error') { + actionStatus = ActionStatus.ACTION_STATUS_ERROR_UNKNOWN; + break; + } + + // Record if the tool call was accepted. + if (toolCall.outcome !== ToolConfirmationOutcome.Cancel) { + acceptedToolCalls++; + + // Edits are ACCEPT_FILE, everything else is UNKNOWN. + if (EDIT_TOOL_NAMES.has(toolCall.request.name)) { + isEdit ||= true; + } + } + } + + // Only file interaction telemetry if 100% of the tool calls were accepted. + return traceId && acceptedToolCalls / toolCalls.length >= 1 + ? createConversationInteraction( + traceId, + actionStatus || ActionStatus.ACTION_STATUS_NO_ERROR, + isEdit + ? ConversationInteractionInteraction.ACCEPT_FILE + : ConversationInteractionInteraction.UNKNOWN, + ) + : undefined; +} + +function createConversationInteraction( + traceId: string, + status: ActionStatus, + interaction: ConversationInteractionInteraction, +): ConversationInteraction { + return { + traceId, + status, + interaction, + isAgentic: true, + }; +} + function includesCode(resp: GenerateContentResponse): boolean { if (!resp.candidates) { return false; @@ -47,7 +181,7 @@ function includesCode(resp: GenerateContentResponse): boolean { return false; } -function getStatus( +function getStatusFromResponse( response: GenerateContentResponse, signal: AbortSignal | undefined, ): ActionStatus { diff --git a/packages/core/src/core/turn.ts b/packages/core/src/core/turn.ts index e7ba0d8bb7..86c7fdf49c 100644 --- a/packages/core/src/core/turn.ts +++ b/packages/core/src/core/turn.ts @@ -109,6 +109,7 @@ export interface ToolCallRequestInfo { isClientInitiated: boolean; prompt_id: string; checkpoint?: string; + traceId?: string; } export interface ToolCallResponseInfo { @@ -291,7 +292,7 @@ export class Turn { // Handle function calls (requesting tool execution) const functionCalls = resp.functionCalls ?? []; for (const fnCall of functionCalls) { - const event = this.handlePendingFunctionCall(fnCall); + const event = this.handlePendingFunctionCall(fnCall, traceId); if (event) { yield event; } @@ -370,6 +371,7 @@ export class Turn { private handlePendingFunctionCall( fnCall: FunctionCall, + traceId?: string, ): ServerGeminiStreamEvent | null { const callId = fnCall.id ?? @@ -383,6 +385,7 @@ export class Turn { args, isClientInitiated: false, prompt_id: this.prompt_id, + traceId, }; this.pendingToolCalls.push(toolCallRequest); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 876a7bf731..c73b5d7b95 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -44,6 +44,7 @@ export * from './code_assist/codeAssist.js'; export * from './code_assist/oauth2.js'; export * from './code_assist/server.js'; export * from './code_assist/types.js'; +export * from './code_assist/telemetry.js'; export * from './core/apiKeyCredentialStorage.js'; // Export utilities