Fix/tool calls default metric

frontend/src/components/protocol/MetricsEditor.autosave.test.tsx

@@ -25,14 +25,17 @@ describe('MetricsEditor autosave', () => {
     })
   })
 
-  it('selects every built-in by default and disables contextual metrics without eligible nodes', async () => {
+  it('selects every producible built-in by default and disables contextual metrics without eligible nodes', async () => {
     const user = userEvent.setup()
     renderEditor(<MetricsEditor experimentId="experiment-1" metrics={[]} measurementPlan={null} graph={EMPTY_GRAPH} onChange={vi.fn()} />)
 
     await user.click(screen.getByRole('button', { name: 'Add metrics' }))
     const dialog = await screen.findByRole('dialog', { name: 'Manage metrics' })
+    const contextual = new Set(['tool_calls', 'tool_error_rate', 'critic_approvals', 'critic_rejections'])
     for (const entry of METRIC_CATALOG.filter((candidate) => candidate.kind === 'runtime')) {
-      expect(within(dialog).getByRole('checkbox', { name: new RegExp(`^${entry.name}`) })).toBeChecked()
+      const checkbox = within(dialog).getByRole('checkbox', { name: new RegExp(`^${entry.name}`) })
+      if (contextual.has(entry.key)) expect(checkbox).not.toBeChecked()
+      else expect(checkbox).toBeChecked()
     }
     expect(within(dialog).getByRole('checkbox', { name: /^Tool calls/ })).toHaveAttribute('aria-disabled', 'true')
     expect(within(dialog).getByRole('checkbox', { name: /^Tool error rate/ })).toHaveAttribute('aria-disabled', 'true')

frontend/src/components/protocol/MetricsEditor.tsx

@@ -144,12 +144,13 @@ function MetricsDialog({
   const canvasMetricKeys = new Set(contextualMetricSuggestions(graph).map((suggestion) => suggestion.key))
   const hasValidTool = canvasMetricKeys.has('tool_error_rate')
   const hasCriticGate = graph?.nodes.some((node) => node.type === 'critic_gate') ?? false
+  const canvasCannotProduce = (key: string) =>
+    ((key === 'tool_calls' || key === 'tool_error_rate') && !hasValidTool)
+    || ((key === 'critic_approvals' || key === 'critic_rejections') && !hasCriticGate)
   const unavailableBuiltInKeys = capabilitiesLoading || capabilitiesUnavailable
     ? []
     : builtInEntries.flatMap((entry) => {
-      const unavailable = !supportedBuiltInKeys.has(entry.key)
-        || ((entry.key === 'tool_calls' || entry.key === 'tool_error_rate') && !hasValidTool)
-        || ((entry.key === 'critic_approvals' || entry.key === 'critic_rejections') && !hasCriticGate)
+      const unavailable = !supportedBuiltInKeys.has(entry.key) || canvasCannotProduce(entry.key)
       return unavailable ? [entry.key] : []
     })
   const unavailableBuiltInKeySignature = unavailableBuiltInKeys.join('\u0000')
@@ -207,14 +208,22 @@ function MetricsDialog({
     }
     if (wasOpenRef.current) return
     wasOpenRef.current = true
-    setDraftKeys(new Set(initialDraftKeySet))
+    // A default selection (nothing saved yet) must not pre-check a metric this
+    // canvas can't produce -- it would save a metric that can never report.
+    // A saved selection is shown as-is, so it can still be unchecked.
+    setDraftKeys(new Set(initialDraftSignature === undefined
+      ? initialDraftKeySet
+      : [...initialDraftKeySet].filter((key) => !canvasCannotProduce(key))))
     setSaveError(undefined)
     setCustomChanges([])
     setCustomMetricIds(initialCustomMetricIds)
     setStagedOrderAnnouncement('')
     setCustomMetricDirty(false)
     setCustomMetricPendingDelete(undefined)
     setCustomMetricDraft(undefined)
+    // Seeds once per open (wasOpenRef); canvas availability changing while the
+    // dialog is open is handled by the newly-unavailable effect below.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [open, initialDraftKeySet, metrics, initialCustomMetricIds, initialCustomMetricSignature])
 
   useEffect(() => {
@@ -317,7 +326,7 @@ function MetricsDialog({
                     const unavailableReasonId = `metric-unavailable-${entry.key}`
                     return <div key={entry.key} className={`rounded-md border p-2.5 ${unavailableReason ? 'opacity-70' : 'hover:bg-muted/40'}`}>
                       <label className={unavailableReason ? 'flex cursor-not-allowed items-start gap-3' : 'flex cursor-pointer items-start gap-3'}>
-                      <Checkbox aria-label={entry.name} aria-describedby={unavailableReason ? unavailableReasonId : undefined} checked={selected} disabled={disabled || Boolean(unavailableReason)} onCheckedChange={(checked) => toggle(entry.key, checked === true)} />
+                      <Checkbox aria-label={entry.name} aria-describedby={unavailableReason ? unavailableReasonId : undefined} checked={selected} disabled={disabled || (Boolean(unavailableReason) && !selected)} onCheckedChange={(checked) => toggle(entry.key, checked === true)} />
                       <span className="min-w-0 flex-1">
                         <span className="text-sm font-medium">{entry.name}</span>
                         <span className="mt-0.5 block text-xs text-muted-foreground">{entry.shortDescription}</span>
@@ -466,13 +475,29 @@ export function MetricsEditor({
 
   function builtInDraft(selectedKeys: Set<string>) {
     const currentByKey = new Map(runtimeBuiltIns.map((metric) => [metric.catalogKey!, metric]))
+    // The plan's runtime outputs are what the dialog shows as selected, so they
+    // are also what a deselect must remove -- even when the design's own metric
+    // list has lost the matching declaration, which would otherwise leave the
+    // plan naming a metric nothing declares and every save 422ing.
+    const plannedIdByKey = new Map(
+      (measurementPlan?.producers ?? [])
+        .filter((producer) => producer.producer_id === 'asaree.runtime')
+        .flatMap((producer) => Object.entries(producer.outputs)),
+    )
     const preserved = normalized.filter((metric) => !(metric.kind === 'runtime' && metric.catalogKey && catalogBuiltIns.some((entry) => entry.key === metric.catalogKey)))
     const selected = catalogBuiltIns
       .filter((entry) => selectedKeys.has(entry.key))
-      .map((entry) => currentByKey.get(entry.key) ?? makeCatalogMetric(entry, false))
+      .map((entry) => {
+        const current = currentByKey.get(entry.key)
+        if (current) return current
+        const plannedId = plannedIdByKey.get(entry.key)
+        return plannedId ? { ...makeCatalogMetric(entry, false), id: plannedId } : makeCatalogMetric(entry, false)
+      })
     let nextPlan = measurementPlan
-    for (const metric of runtimeBuiltIns.filter((metric) => catalogBuiltIns.some((entry) => entry.key === metric.catalogKey) && !selectedKeys.has(metric.catalogKey!))) {
-      nextPlan = removeMetricFromMeasurementPlan(nextPlan, metric.id)
+    for (const entry of catalogBuiltIns.filter((entry) => !selectedKeys.has(entry.key))) {
+      for (const metricId of new Set([currentByKey.get(entry.key)?.id, plannedIdByKey.get(entry.key)])) {
+        nextPlan = removeMetricFromMeasurementPlan(nextPlan, metricId)
+      }
     }
     for (const metric of selected) nextPlan = upsertRuntimeMetric(nextPlan, metric)
     const nextMetrics = withoutRanking([...preserved, ...selected])

frontend/src/components/protocol/ProtocolCanvas.tsx

@@ -796,13 +796,11 @@ export const ProtocolCanvas = forwardRef<ProtocolCanvasHandle, {
   }, [nodes, edges])
 
   // The most recent thing this canvas actually did, whichever kind it was.
-  // The node badges read `runQuery` alone, because that's the run the canvas
-  // is *watching*; a Test Run reports itself in its own results panel instead
-  // (list_protocol_runs excludes test runs, so it can never seed runQuery).
-  // Config findings can't follow that split: a Test Run is how you iterate on
-  // the canvas, so "your cap is too low" learned from one has to reach the
-  // node you'd fix. Newest wins, so raising the cap and running for real
-  // clears a finding the earlier Test Run left behind.
+  // Node badges, inspectors' Input/Output and config findings all read this,
+  // not `runQuery` alone: list_protocol_runs excludes test runs, so after a
+  // page reload runQuery can never be seeded with one, and a Test Run's node
+  // output vanished from the inspectors while its results panel still showed
+  // it. Newest wins, so running for real supersedes an earlier Test Run.
   const latestNodeRuns = useMemo(() => {
     const run = runQuery.data
     const test = testRunQuery.data
@@ -985,8 +983,8 @@ export const ProtocolCanvas = forwardRef<ProtocolCanvasHandle, {
         deletable: !nonDeletablePatternNodeIds.has(n.id),
         data: {
           ...n.data,
-          runStatus: runQuery.data?.node_runs[n.id]?.status,
-          runTruncated: Boolean(runQuery.data?.node_runs[n.id]?.truncation),
+          runStatus: latestNodeRuns?.[n.id]?.status,
+          runTruncated: Boolean(latestNodeRuns?.[n.id]?.truncation),
           missingLlm: n.type === 'agent' && !agentIdsWithLlm.has(n.id),
           // "Require specific output format" is on, but nothing says what the
           // format is. Unlike missingLlm this doesn't stop the run -- the agent
@@ -1043,7 +1041,7 @@ export const ProtocolCanvas = forwardRef<ProtocolCanvasHandle, {
     })
   }, [
     nodes,
-    runQuery.data,
+    latestNodeRuns,
     nonDeletablePatternNodeIds,
     agentIdsWithLlm,
     agentIdsWithParser,
@@ -2129,7 +2127,7 @@ export const ProtocolCanvas = forwardRef<ProtocolCanvasHandle, {
           <CriticGateNodeInspector
             node={{ id: selectedNode.id, type: 'critic_gate', position: selectedNode.position, data: selectedNode.data as CriticGateNodeData }}
             experimentId={experimentId}
-            nodeRun={runQuery.data?.node_runs[selectedNode.id]}
+            nodeRun={latestNodeRuns?.[selectedNode.id]}
             onChange={updateNodeData}
             onDelete={requestDeleteNode}
             onClose={() => setSelectedNodeId(null)}
@@ -2244,7 +2242,7 @@ export const ProtocolCanvas = forwardRef<ProtocolCanvasHandle, {
               handoffPeers={selectedHandoffPeers}
               wiredOutputParserLabel={selectedOutputParserLabel}
               fetchPromptPreview={fetchPromptPreview}
-              nodeRun={runQuery.data?.node_runs[selectedNode.id]}
+              nodeRun={latestNodeRuns?.[selectedNode.id]}
               onChange={updateNodeData}
               onDelete={requestDeleteNode}
               onClose={() => setSelectedNodeId(null)}

frontend/src/lib/metricCatalog.ts

@@ -36,7 +36,7 @@ export const METRIC_CATALOG: readonly MetricCatalogEntry[] = [
   { key: 'input_tokens', name: 'Input tokens', shortDescription: 'Tokens sent to the model during the run.', kind: 'runtime', valueType: 'number', defaultDirection: 'minimize', aggregation: 'sum', unit: 'tokens', category: 'Tokens', source: 'Run summary' },
   { key: 'output_tokens', name: 'Output tokens', shortDescription: 'Tokens generated by the model during the run.', kind: 'runtime', valueType: 'number', defaultDirection: 'minimize', aggregation: 'sum', unit: 'tokens', category: 'Tokens', source: 'Run summary' },
   { key: 'total_tokens', name: 'Total tokens', shortDescription: 'Combined input and output tokens for the run.', kind: 'runtime', valueType: 'number', defaultDirection: 'minimize', aggregation: 'sum', unit: 'tokens', category: 'Tokens', source: 'Run summary', recommended: true },
-  { key: 'tool_calls', name: 'Tool calls', shortDescription: 'Recorded tool-call attempts across attributed Agent runs.', kind: 'runtime', valueType: 'number', defaultDirection: 'minimize', aggregation: 'sum', unit: 'calls', category: 'Tools', source: 'Protocol activity', recommended: true },
+  { key: 'tool_calls', name: 'Tool calls', shortDescription: 'Recorded tool-call attempts across attributed Agent runs.', kind: 'runtime', valueType: 'number', defaultDirection: 'minimize', aggregation: 'sum', unit: 'calls', category: 'Tools', source: 'Protocol activity' },
   { key: 'tool_error_rate', name: 'Tool error rate', shortDescription: 'Failed tool calls divided by all tool-call attempts.', kind: 'runtime', valueType: 'number', defaultDirection: 'minimize', aggregation: 'mean', unit: 'rate', category: 'Tools', source: 'Protocol activity' },
   { key: 'agent_loop_iterations', name: 'Agent-loop iterations', shortDescription: 'Distinct Agent reasoning-loop iterations in the run.', kind: 'runtime', valueType: 'number', defaultDirection: 'minimize', aggregation: 'sum', unit: 'iterations', category: 'Agents', source: 'Protocol activity' },
   { key: 'critic_rejections', name: 'Critic rejections', shortDescription: 'Rejected critic-gate reviews during the run.', kind: 'runtime', valueType: 'number', defaultDirection: 'minimize', aggregation: 'sum', unit: 'reviews', category: 'Critics', source: 'Protocol activity' },

src/asaree/services/metrics.py

@@ -116,12 +116,14 @@
     },
 )
 _CATALOG_BY_KEY = {str(entry["key"]): entry for entry in METRIC_CATALOG}
-RECOMMENDED_RUNTIME_METRIC_KEYS = ("cost_usd", "duration_seconds", "total_tokens", "tool_calls")
+# Only metrics every run can produce. Tool calls is deliberately absent: an
+# experiment is created before its canvas exists, and on a canvas with no tool
+# wired it can never measure anything -- the metrics editor offers it once one is.
+RECOMMENDED_RUNTIME_METRIC_KEYS = ("cost_usd", "duration_seconds", "total_tokens")
 _RECOMMENDED_RUNTIME_METRIC_IDS = {
     "cost_usd": "runtime-cost",
     "duration_seconds": "runtime-duration",
     "total_tokens": "runtime-total-tokens",
-    "tool_calls": "runtime-tool-calls",
 }
 _KINDS = {"runtime", "custom"}
 _VALUE_TYPES = {"number", "boolean", "opaque"}

tests/test_experiments.py

@@ -132,18 +132,6 @@ async def test_create_experiment_returns_a_persisted_recommended_measurement_pla
             "primary": False,
             "unit": "tokens",
         },
-        {
-            "id": "runtime-tool-calls",
-            "catalogKey": "tool_calls",
-            "name": "Tool calls",
-            "description": "Recorded tool-call attempts across attributed Agent runs.",
-            "kind": "runtime",
-            "valueType": "number",
-            "direction": "minimize",
-            "aggregation": "sum",
-            "primary": False,
-            "unit": "calls",
-        },
     ]
     expected_plan = {
         "metrics": [
@@ -177,16 +165,6 @@ async def test_create_experiment_returns_a_persisted_recommended_measurement_pla
                 "description": "Combined input and output tokens for the run.",
                 "unit": "tokens",
             },
-            {
-                "id": "runtime-tool-calls",
-                "name": "Tool calls",
-                "value_type": "number",
-                "direction": "minimize",
-                "aggregation": "sum",
-                "primary": False,
-                "description": "Recorded tool-call attempts across attributed Agent runs.",
-                "unit": "calls",
-            },
         ],
         "producers": [
             {
@@ -197,7 +175,6 @@ async def test_create_experiment_returns_a_persisted_recommended_measurement_pla
                     "cost_usd": "runtime-cost",
                     "duration_seconds": "runtime-duration",
                     "total_tokens": "runtime-total-tokens",
-                    "tool_calls": "runtime-tool-calls",
                 },
                 "artifacts": [],
                 "config": {},

tests/test_metrics.py

@@ -117,15 +117,13 @@ def test_recommended_runtime_measurements_form_one_normalized_non_primary_plan()
         "cost_usd",
         "duration_seconds",
         "total_tokens",
-        "tool_calls",
     ]
-    assert [metric["primary"] for metric in declarations] == [False, False, False, False]
+    assert [metric["primary"] for metric in declarations] == [False, False, False]
     assert normalize_measurement_plan(plan) == plan
     assert plan["producers"][0]["outputs"] == {
         "cost_usd": "runtime-cost",
         "duration_seconds": "runtime-duration",
         "total_tokens": "runtime-total-tokens",
-        "tool_calls": "runtime-tool-calls",
     }