fix(review): bound agent metric labels

Author: j15z

apps/sim/lib/copilot/request/metrics.test.ts

@@ -2,7 +2,7 @@
  * @vitest-environment node
  */
 
-import { describe, expect, it, vi } from 'vitest'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const { toolCallsAdd, toolDurationRecord } = vi.hoisted(() => ({
   toolCallsAdd: vi.fn(),
@@ -24,8 +24,30 @@ import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
 import { recordSimToolMetric } from '@/lib/copilot/request/metrics'
 
 describe('recordSimToolMetric', () => {
-  it('attributes call counts to the agent without adding it to duration', () => {
-    recordSimToolMetric('read', 'workflow', 'success', 125)
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it.each(['main', 'workflow'])(
+    'attributes call counts to the registered %s agent without adding it to duration',
+    (agentId) => {
+      recordSimToolMetric('read', agentId, 'success', 125)
+
+      const baseAttributes = {
+        [TraceAttr.ToolName]: 'read',
+        [TraceAttr.ToolExecutor]: 'sim',
+        [TraceAttr.ToolOutcome]: 'success',
+      }
+      expect(toolCallsAdd).toHaveBeenCalledWith(1, {
+        ...baseAttributes,
+        [TraceAttr.GenAiAgentName]: agentId,
+      })
+      expect(toolDurationRecord).toHaveBeenCalledWith(125, baseAttributes)
+    }
+  )
+
+  it('collapses unknown agent IDs to the bounded fallback', () => {
+    recordSimToolMetric('read', 'tenant-defined-agent', 'success', 125)
 
     const baseAttributes = {
       [TraceAttr.ToolName]: 'read',
@@ -34,7 +56,7 @@ describe('recordSimToolMetric', () => {
     }
     expect(toolCallsAdd).toHaveBeenCalledWith(1, {
       ...baseAttributes,
-      [TraceAttr.GenAiAgentName]: 'workflow',
+      [TraceAttr.GenAiAgentName]: 'other',
     })
     expect(toolDurationRecord).toHaveBeenCalledWith(125, baseAttributes)
   })

apps/sim/lib/copilot/request/metrics.ts

@@ -68,6 +68,15 @@ function cappedToolName(name: string): string {
   return TOOL_CATALOG[name] ? name : 'other'
 }
 
+const REGISTERED_AGENT_IDS = new Set([
+  'main',
+  ...Object.values(TOOL_CATALOG).flatMap(({ subagentId }) => (subagentId ? [subagentId] : [])),
+])
+
+function cappedAgentId(agentId: string): string {
+  return REGISTERED_AGENT_IDS.has(agentId) ? agentId : 'other'
+}
+
 // recordSimToolMetric emits copilot.tool.calls (+1) and copilot.tool.duration
 // for one server-side Sim tool dispatch (executor=sim). outcome is the bounded
 // tool outcome (success/error/…). Pure telemetry.
@@ -85,7 +94,7 @@ export function recordSimToolMetric(
   }
   toolCalls.add(1, {
     ...baseAttrs,
-    [TraceAttr.GenAiAgentName]: agentId,
+    [TraceAttr.GenAiAgentName]: cappedAgentId(agentId),
   })
   if (durationMs >= 0) toolDuration.record(durationMs, baseAttrs)
 }

apps/sim/lib/copilot/request/tools/executor.test.ts

@@ -112,25 +112,32 @@ describe('executeToolAndReport metrics', () => {
     )
   })
 
-  it('falls back to main when forwarding an unexpected throw', async () => {
-    const toolCall: ToolCallState = {
-      id: 'call-2',
-      name: 'read',
-      status: MothershipStreamV1ToolOutcome.error,
-      endTime: Date.now(),
+  it.each([
+    { agentId: 'workflow', expectedAgentId: 'workflow' },
+    { agentId: undefined, expectedAgentId: 'main' },
+  ])(
+    'forwards $expectedAgentId when an unexpected error occurs',
+    async ({ agentId, expectedAgentId }) => {
+      const toolCall: ToolCallState = {
+        id: 'call-2',
+        name: 'read',
+        status: MothershipStreamV1ToolOutcome.error,
+        agentId,
+        endTime: Date.now(),
+      }
+      const context = createStreamingContext({
+        toolCalls: new Map([[toolCall.id, toolCall]]),
+      })
+
+      await expect(executeToolAndReport(toolCall.id, context, executionContext)).rejects.toThrow(
+        'missing a canonical error'
+      )
+      expect(recordSimToolMetric).toHaveBeenCalledWith(
+        'read',
+        expectedAgentId,
+        MothershipStreamV1ToolOutcome.error,
+        expect.any(Number)
+      )
     }
-    const context = createStreamingContext({
-      toolCalls: new Map([[toolCall.id, toolCall]]),
-    })
-
-    await expect(executeToolAndReport(toolCall.id, context, executionContext)).rejects.toThrow(
-      'missing a canonical error'
-    )
-    expect(recordSimToolMetric).toHaveBeenCalledWith(
-      'read',
-      'main',
-      MothershipStreamV1ToolOutcome.error,
-      expect.any(Number)
-    )
-  })
+  )
 })