diff --git a/frontend/src/components/protocol/MetricsEditor.autosave.test.tsx b/frontend/src/components/protocol/MetricsEditor.autosave.test.tsx index 5b818e9..98f7e25 100644 --- a/frontend/src/components/protocol/MetricsEditor.autosave.test.tsx +++ b/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() 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') diff --git a/frontend/src/components/protocol/MetricsEditor.tsx b/frontend/src/components/protocol/MetricsEditor.tsx index efe6ec4..667d81a 100644 --- a/frontend/src/components/protocol/MetricsEditor.tsx +++ b/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,7 +208,12 @@ 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) @@ -215,6 +221,9 @@ function MetricsDialog({ 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