Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/core/src/code_assist/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ describe('CodeAssistServer', () => {
role: 'model',
parts: [
{ text: 'response' },
{ functionCall: { name: 'test', args: {} } },
{ functionCall: { name: 'replace', args: {} } },
],
},
finishReason: FinishReason.SAFETY,
Expand Down Expand Up @@ -160,7 +160,7 @@ describe('CodeAssistServer', () => {
role: 'model',
parts: [
{ text: 'response' },
{ functionCall: { name: 'test', args: {} } },
{ functionCall: { name: 'replace', args: {} } },
],
},
finishReason: FinishReason.STOP,
Expand Down Expand Up @@ -233,7 +233,7 @@ describe('CodeAssistServer', () => {
content: {
parts: [
{ text: 'chunk' },
{ functionCall: { name: 'test', args: {} } },
{ functionCall: { name: 'replace', args: {} } },
],
},
},
Expand Down
119 changes: 93 additions & 26 deletions packages/core/src/code_assist/telemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('telemetry', () => {
},
],
true,
[{ name: 'someTool', args: {} }],
[{ name: 'replace', args: {} }],
);
const traceId = 'test-trace-id';
const streamingLatency: StreamingLatency = { totalLatency: '1s' };
Expand Down Expand Up @@ -130,7 +130,7 @@ describe('telemetry', () => {

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

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

const result = createConversationOffered(
Expand All @@ -169,7 +169,7 @@ describe('telemetry', () => {
},
],
true,
[{ name: 'tool', args: {} }],
[{ name: 'replace', args: {} }],
);

const result = createConversationOffered(
Expand All @@ -186,7 +186,7 @@ describe('telemetry', () => {
// 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: {} },
{ name: 'replace', args: {} },
]);

const result = createConversationOffered(
Expand All @@ -212,7 +212,7 @@ describe('telemetry', () => {
},
],
true,
[{ name: 'tool', args: {} }],
[{ name: 'replace', args: {} }],
);
const result = createConversationOffered(response, 'id', undefined, {});
expect(result?.includedCode).toBe(true);
Expand All @@ -229,7 +229,7 @@ describe('telemetry', () => {
},
],
true,
[{ name: 'tool', args: {} }],
[{ name: 'replace', args: {} }],
);
const result = createConversationOffered(response, 'id', undefined, {});
expect(result?.includedCode).toBe(false);
Expand All @@ -250,7 +250,7 @@ describe('telemetry', () => {
} as unknown as CodeAssistServer;

const response = createMockResponse([], true, [
{ name: 'tool', args: {} },
{ name: 'replace', args: {} },
]);
const streamingLatency = {};

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

await recordConversationOffered(
Expand Down Expand Up @@ -331,17 +331,89 @@ describe('telemetry', () => {

await recordToolCallInteractions({} as Config, toolCalls);

expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith({
traceId: 'trace-1',
status: ActionStatus.ACTION_STATUS_NO_ERROR,
interaction: ConversationInteractionInteraction.ACCEPT_FILE,
acceptedLines: '5',
removedLines: '3',
isAgentic: true,
});
expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith(
expect.objectContaining({
traceId: 'trace-1',
status: ActionStatus.ACTION_STATUS_NO_ERROR,
interaction: ConversationInteractionInteraction.ACCEPT_FILE,
acceptedLines: '8',
removedLines: '3',
isAgentic: true,
}),
);
});

it('should record UNKNOWN interaction for other accepted tools', async () => {
it('should include language in interaction if file_path is present', async () => {
const toolCalls: CompletedToolCall[] = [
{
request: {
name: 'replace',
args: {
file_path: 'test.ts',
old_string: 'old',
new_string: 'new',
},
callId: 'call-1',
isClientInitiated: false,
prompt_id: 'p1',
traceId: 'trace-1',
},
response: {
resultDisplay: {
diffStat: {
model_added_lines: 5,
model_removed_lines: 3,
},
},
},
outcome: ToolConfirmationOutcome.ProceedOnce,
status: 'success',
} as unknown as CompletedToolCall,
];

await recordToolCallInteractions({} as Config, toolCalls);

expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith(
expect.objectContaining({
language: 'TypeScript',
}),
);
});

it('should include language in interaction if write_file is used', async () => {
const toolCalls: CompletedToolCall[] = [
{
request: {
name: 'write_file',
args: { file_path: 'test.py', content: 'test' },
callId: 'call-1',
isClientInitiated: false,
prompt_id: 'p1',
traceId: 'trace-1',
},
response: {
resultDisplay: {
diffStat: {
model_added_lines: 5,
model_removed_lines: 3,
},
},
},
outcome: ToolConfirmationOutcome.ProceedOnce,
status: 'success',
} as unknown as CompletedToolCall,
];

await recordToolCallInteractions({} as Config, toolCalls);

expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith(
expect.objectContaining({
language: 'Python',
}),
);
});

it('should not record interaction for other accepted tools', async () => {
const toolCalls: CompletedToolCall[] = [
{
request: {
Expand All @@ -359,19 +431,14 @@ describe('telemetry', () => {

await recordToolCallInteractions({} as Config, toolCalls);

expect(mockServer.recordConversationInteraction).toHaveBeenCalledWith({
traceId: 'trace-2',
status: ActionStatus.ACTION_STATUS_NO_ERROR,
interaction: ConversationInteractionInteraction.UNKNOWN,
isAgentic: true,
});
expect(mockServer.recordConversationInteraction).not.toHaveBeenCalled();
});

it('should not record interaction for cancelled status', async () => {
const toolCalls: CompletedToolCall[] = [
{
request: {
name: 'tool',
name: 'replace',
args: {},
callId: 'call-3',
isClientInitiated: false,
Expand All @@ -394,7 +461,7 @@ describe('telemetry', () => {
const toolCalls: CompletedToolCall[] = [
{
request: {
name: 'tool',
name: 'replace',
args: {},
callId: 'call-4',
isClientInitiated: false,
Expand Down
43 changes: 31 additions & 12 deletions packages/core/src/code_assist/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ 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';
import { getLanguageFromFilePath } from '../utils/language-detection.js';
import {
computeModelAddedAndRemovedLines,
getFileDiffFromResultDisplay,
} from '../utils/fileDiffUtils.js';
import { isEditToolParams } from '../tools/edit.js';
import { isWriteFileToolParams } from '../tools/write-file.js';

export async function recordConversationOffered(
server: CodeAssistServer,
Expand Down Expand Up @@ -85,10 +88,12 @@ export function createConversationOffered(
signal: AbortSignal | undefined,
streamingLatency: StreamingLatency,
): 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) {
// Only send conversation offered events for responses that contain edit
// function calls. Non-edit function calls don't represent file modifications.
if (
!response.functionCalls ||
!response.functionCalls.some((call) => EDIT_TOOL_NAMES.has(call.name || ''))
) {
return;
}

Expand Down Expand Up @@ -116,6 +121,7 @@ function summarizeToolCalls(
let isEdit = false;
let acceptedLines = 0;
let removedLines = 0;
let language = undefined;

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

if (
!language &&
(isEditToolParams(toolCall.request.args) ||
isWriteFileToolParams(toolCall.request.args))
) {
language = getLanguageFromFilePath(toolCall.request.args.file_path);
}

if (toolCall.status === 'success') {
const fileDiff = getFileDiffFromResultDisplay(
toolCall.response.resultDisplay,
);
if (fileDiff?.diffStat) {
const lines = computeModelAddedAndRemovedLines(fileDiff.diffStat);
acceptedLines += lines.addedLines;

// The API expects acceptedLines to be addedLines + removedLines.
acceptedLines += lines.addedLines + lines.removedLines;
removedLines += lines.removedLines;
}
}
}
}
}

// Only file interaction telemetry if 100% of the tool calls were accepted.
return traceId && acceptedToolCalls / toolCalls.length >= 1
// Only file interaction telemetry if 100% of the tool calls were accepted
// and at least one of them was an edit.
return traceId && acceptedToolCalls / toolCalls.length >= 1 && isEdit
? createConversationInteraction(
traceId,
actionStatus || ActionStatus.ACTION_STATUS_NO_ERROR,
isEdit
? ConversationInteractionInteraction.ACCEPT_FILE
: ConversationInteractionInteraction.UNKNOWN,
isEdit ? String(acceptedLines) : undefined,
isEdit ? String(removedLines) : undefined,
ConversationInteractionInteraction.ACCEPT_FILE,
String(acceptedLines),
String(removedLines),
language,
)
: undefined;
}
Expand All @@ -178,16 +194,19 @@ function createConversationInteraction(
interaction: ConversationInteractionInteraction,
acceptedLines?: string,
removedLines?: string,
language?: string,
): ConversationInteraction {
return {
traceId,
status,
interaction,
acceptedLines,
removedLines,
language,
isAgentic: true,
};
}

function includesCode(resp: GenerateContentResponse): boolean {
if (!resp.candidates) {
return false;
Expand Down
14 changes: 14 additions & 0 deletions packages/core/src/tools/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,20 @@ export interface EditToolParams {
ai_proposed_content?: string;
}

export function isEditToolParams(args: unknown): args is EditToolParams {
if (typeof args !== 'object' || args === null) {
return false;
}
return (
'file_path' in args &&
typeof args.file_path === 'string' &&
'old_string' in args &&
typeof args.old_string === 'string' &&
'new_string' in args &&
typeof args.new_string === 'string'
);
}

interface CalculatedEdit {
currentContent: string | null;
newContent: string;
Expand Down
14 changes: 14 additions & 0 deletions packages/core/src/tools/write-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@ export interface WriteFileToolParams {
ai_proposed_content?: string;
}

export function isWriteFileToolParams(
args: unknown,
): args is WriteFileToolParams {
if (typeof args !== 'object' || args === null) {
return false;
}
return (
'file_path' in args &&
typeof args.file_path === 'string' &&
'content' in args &&
typeof args.content === 'string'
);
}

interface GetCorrectedFileContentResult {
originalContent: string;
correctedContent: string;
Expand Down
Loading