diff --git a/src/ai/conversation.ts b/src/ai/conversation.ts index f66febf..c5b4997 100644 --- a/src/ai/conversation.ts +++ b/src/ai/conversation.ts @@ -7,6 +7,12 @@ export interface ToolExecution { wasSuccessful: boolean; } +export function toToolExecution(toolName: string, input: any, rawOutput: any): ToolExecution { + let output = rawOutput; + if (rawOutput?.type === 'json' && rawOutput?.value) output = rawOutput.value; + return { toolName, input, output, wasSuccessful: output?.success !== false }; +} + export function toolExecutionLabel(input: Record | undefined): string { return input?.explanation || input?.assertion || input?.reason || input?.request || ''; } @@ -18,15 +24,13 @@ export class Conversation { id: string; messages: ModelMessage[]; model: any; - telemetryFunctionId?: string; protectedPrefixCount = 0; private autoTrimRules: Map; - constructor(messages: ModelMessage[] = [], model?: any, telemetryFunctionId?: string) { + constructor(messages: ModelMessage[] = [], model?: any) { this.id = this.generateId(); this.messages = messages; this.model = model || ''; - this.telemetryFunctionId = telemetryFunctionId; this.autoTrimRules = new Map(); } @@ -83,7 +87,7 @@ export class Conversation { } clone(): Conversation { - return new Conversation([...this.messages], this.model, this.telemetryFunctionId); + return new Conversation([...this.messages], this.model); } cleanupTag(tagName: string, replacement: string, keepLast = 0): void { @@ -243,14 +247,7 @@ export class Conversation { if (!Array.isArray(message.content)) continue; for (const part of message.content) { if (part.type !== 'tool-result') continue; - const rawOutput = part.output as Record; - const output = rawOutput?.type === 'json' && rawOutput?.value ? rawOutput.value : rawOutput; - executions.push({ - toolName: part.toolName, - input: toolCalls.get(part.toolCallId) || {}, - output, - wasSuccessful: output?.success !== false, - }); + executions.push(toToolExecution(part.toolName, toolCalls.get(part.toolCallId) || {}, part.output)); } } diff --git a/src/ai/provider.ts b/src/ai/provider.ts index 50dc477..7b31064 100644 --- a/src/ai/provider.ts +++ b/src/ai/provider.ts @@ -11,7 +11,7 @@ import { Stats } from '../stats.ts'; import { createDebug, tag } from '../utils/logger.js'; import { type RetryOptions, withRetry } from '../utils/retry.js'; import { RulesLoader } from '../utils/rules-loader.ts'; -import { Conversation } from './conversation.js'; +import { Conversation, toToolExecution } from './conversation.js'; const debugLog = createDebug('explorbot:provider'); const promptLog = createDebug('explorbot:provider:out'); @@ -213,20 +213,25 @@ export class Provider { const runTelemetry = Observability.getTelemetry(); - if (!options.telemetry) { + let optionTelemetry = options.telemetry; + if (options.telemetryFunctionId && !optionTelemetry?.functionId) { + optionTelemetry = { ...optionTelemetry, functionId: options.telemetryFunctionId }; + } + + if (!optionTelemetry) { return runTelemetry; } if (!runTelemetry) { - return options.telemetry; + return optionTelemetry; } return { ...runTelemetry, - ...options.telemetry, + ...optionTelemetry, metadata: { ...runTelemetry.metadata, - ...options.telemetry.metadata, + ...optionTelemetry.metadata, }, }; } @@ -260,12 +265,8 @@ export class Provider { const toolCalls = response.toolCalls || []; const toolResults = response.toolResults || []; - const toolExecutions = toolCalls.map((call: any, index: number) => ({ - toolName: call.toolName || '', - input: call.input, - output: toolResults[index]?.output, - wasSuccessful: toolResults[index]?.output?.success || false, - })); + const resultsById = new Map(toolResults.map((r: any) => [r.toolCallId, r])); + const toolExecutions = toolCalls.map((call: any) => toToolExecution(call.toolName || '', call.input, resultsById.get(call.toolCallId)?.output)); return { conversation, response, toolExecutions }; } diff --git a/tests/unit/conversation.test.ts b/tests/unit/conversation.test.ts index d4e403b..321c016 100644 --- a/tests/unit/conversation.test.ts +++ b/tests/unit/conversation.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'bun:test'; -import { Conversation } from '../../src/ai/conversation'; +import { Conversation, toToolExecution } from '../../src/ai/conversation'; describe('Conversation', () => { describe('cleanupTag', () => { @@ -405,4 +405,28 @@ describe('Conversation', () => { expect(output.pageDiff.htmlParts).toBeDefined(); }); }); + + describe('toToolExecution', () => { + it('unwraps the {type:json, value} envelope', () => { + const exec = toToolExecution('click', { locator: 'Save' }, { type: 'json', value: { success: true, url: '/x' } }); + expect(exec.output).toEqual({ success: true, url: '/x' }); + expect(exec.wasSuccessful).toBe(true); + }); + + it('defaults missing success to true', () => { + const exec = toToolExecution('getVisitedStates', {}, { states: ['/a', '/b'] }); + expect(exec.wasSuccessful).toBe(true); + }); + + it('respects explicit success: false', () => { + const exec = toToolExecution('click', { locator: 'Save' }, { success: false, message: 'not found' }); + expect(exec.wasSuccessful).toBe(false); + }); + + it('handles a missing output', () => { + const exec = toToolExecution('click', { locator: 'Save' }, undefined); + expect(exec.wasSuccessful).toBe(true); + expect(exec.output).toBeUndefined(); + }); + }); }); diff --git a/tests/unit/provider.test.ts b/tests/unit/provider.test.ts index 0bb2a0e..fa090d4 100644 --- a/tests/unit/provider.test.ts +++ b/tests/unit/provider.test.ts @@ -368,6 +368,20 @@ describe('Provider', () => { }); }); + describe('getTelemetry', () => { + it('honors the telemetryFunctionId option shorthand', () => { + (provider as any).telemetryEnabled = true; + const telemetry = (provider as any).getTelemetry({ telemetryFunctionId: 'researcher.textContent' }); + expect(telemetry?.functionId).toBe('researcher.textContent'); + }); + + it('lets an explicit telemetry.functionId win over the shorthand', () => { + (provider as any).telemetryEnabled = true; + const telemetry = (provider as any).getTelemetry({ telemetry: { functionId: 'explicit' }, telemetryFunctionId: 'shorthand' }); + expect(telemetry?.functionId).toBe('explicit'); + }); + }); + describe('abort on idle timeout', () => { it('aborts the in-flight request when the idle timeout fires', async () => { const capture: { signal?: AbortSignal } = {};