fix: unify tool-success mapping and honor telemetryFunctionId (plan 007)

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<string, any> | 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<string, number>;
 
-  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<string, any>;
-        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));
       }
     }
 

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 };
   }

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();
+    });
+  });
 });

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 } = {};