From 3a523c3c8ac1bf9094dbe65a6f53b71085438c0c Mon Sep 17 00:00:00 2001 From: main Date: Fri, 20 Mar 2026 23:31:29 -0400 Subject: Open the metric unit vocabulary --- crates/fidget-spinner-cli/src/main.rs | 27 ++--------- crates/fidget-spinner-cli/src/mcp/catalog.rs | 5 +- crates/fidget-spinner-cli/src/ui.rs | 38 ++++++++++------ crates/fidget-spinner-cli/tests/mcp_hardening.rs | 58 ++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 41 deletions(-) (limited to 'crates/fidget-spinner-cli') diff --git a/crates/fidget-spinner-cli/src/main.rs b/crates/fidget-spinner-cli/src/main.rs index 2c026d1..7482794 100644 --- a/crates/fidget-spinner-cli/src/main.rs +++ b/crates/fidget-spinner-cli/src/main.rs @@ -493,8 +493,8 @@ struct MetricDefineArgs { project: ProjectArg, #[arg(long)] key: String, - #[arg(long, value_enum)] - unit: CliMetricUnit, + #[arg(long)] + unit: String, #[arg(long, value_enum)] objective: CliOptimizationObjective, #[arg(long, value_enum, default_value_t = CliMetricVisibility::Canonical)] @@ -581,15 +581,6 @@ struct SkillShowArgs { name: Option, } -#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] -enum CliMetricUnit { - Seconds, - Bytes, - Count, - Ratio, - Custom, -} - #[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)] enum CliOptimizationObjective { Minimize, @@ -995,7 +986,7 @@ fn run_metric_define(args: MetricDefineArgs) -> Result<(), StoreError> { let mut store = open_store(&args.project.project)?; print_json(&store.define_metric(DefineMetricRequest { key: NonEmptyText::new(args.key)?, - unit: args.unit.into(), + unit: MetricUnit::new(args.unit)?, objective: args.objective.into(), visibility: args.visibility.into(), description: args.description.map(NonEmptyText::new).transpose()?, @@ -1398,18 +1389,6 @@ fn print_json(value: &impl Serialize) -> Result<(), StoreError> { Ok(()) } -impl From for MetricUnit { - fn from(value: CliMetricUnit) -> Self { - match value { - CliMetricUnit::Seconds => Self::Seconds, - CliMetricUnit::Bytes => Self::Bytes, - CliMetricUnit::Count => Self::Count, - CliMetricUnit::Ratio => Self::Ratio, - CliMetricUnit::Custom => Self::Custom, - } - } -} - impl From for OptimizationObjective { fn from(value: CliOptimizationObjective) -> Self { match value { diff --git a/crates/fidget-spinner-cli/src/mcp/catalog.rs b/crates/fidget-spinner-cli/src/mcp/catalog.rs index 9b486bc..d6c8171 100644 --- a/crates/fidget-spinner-cli/src/mcp/catalog.rs +++ b/crates/fidget-spinner-cli/src/mcp/catalog.rs @@ -601,9 +601,8 @@ fn tool_input_schema(name: &str) -> Value { ("key", string_schema("Metric key.")), ( "unit", - enum_string_schema( - &["seconds", "bytes", "count", "ratio", "custom"], - "Metric unit.", + string_schema( + "Metric unit token. Builtins include `scalar`, `count`, `ratio`, `percent`, `bytes`, `nanoseconds`, `microseconds`, `milliseconds`, and `seconds`; custom lowercase ascii tokens are also allowed.", ), ), ( diff --git a/crates/fidget-spinner-cli/src/ui.rs b/crates/fidget-spinner-cli/src/ui.rs index 9a42411..0b05b29 100644 --- a/crates/fidget-spinner-cli/src/ui.rs +++ b/crates/fidget-spinner-cli/src/ui.rs @@ -10,7 +10,7 @@ use axum::routing::get; use camino::Utf8PathBuf; use fidget_spinner_core::{ AttachmentTargetRef, ExperimentAnalysis, ExperimentOutcome, ExperimentStatus, FrontierRecord, - FrontierVerdict, MetricUnit, NonEmptyText, RunDimensionValue, Slug, VertexRef, + FrontierVerdict, KnownMetricUnit, MetricUnit, NonEmptyText, RunDimensionValue, Slug, VertexRef, }; use fidget_spinner_store_sqlite::{ ExperimentDetail, ExperimentSummary, FrontierMetricSeries, FrontierOpenProjection, @@ -640,7 +640,7 @@ fn render_metric_series_section( (point.verdict.as_str()) } } - td.nowrap { (format_metric_value(point.value, series.metric.unit)) } + td.nowrap { (format_metric_value(point.value, &series.metric.unit)) } } } } @@ -1295,7 +1295,7 @@ fn render_metric_panel( @for metric in metrics { tr { td { (metric.key) } - td { (format_metric_value(metric.value, metric_unit_for(metric, outcome))) } + td { (format_metric_value(metric.value, &metric_unit_for(metric, outcome))) } } } } @@ -1310,9 +1310,9 @@ fn metric_unit_for( outcome: &ExperimentOutcome, ) -> MetricUnit { if metric.key == outcome.primary_metric.key { - return MetricUnit::Custom; + return MetricUnit::scalar(); } - MetricUnit::Custom + MetricUnit::scalar() } fn render_vertex_relation_sections( @@ -1424,7 +1424,7 @@ fn render_experiment_card(experiment: &ExperimentSummary) -> Markup { div.meta-row { span.metric-pill { (metric.key) ": " - (format_metric_value(metric.value, metric.unit)) + (format_metric_value(metric.value, &metric.unit)) } } } @@ -1449,7 +1449,7 @@ fn render_experiment_summary_line(experiment: &ExperimentSummary) -> Markup { @if let Some(metric) = experiment.primary_metric.as_ref() { span.metric-pill { (metric.key) ": " - (format_metric_value(metric.value, metric.unit)) + (format_metric_value(metric.value, &metric.unit)) } } } @@ -1627,13 +1627,23 @@ fn render_dimension_value(value: &RunDimensionValue) -> String { } } -fn format_metric_value(value: f64, unit: MetricUnit) -> String { - match unit { - MetricUnit::Bytes => format!("{} B", format_integerish(value)), - MetricUnit::Seconds => format!("{value:.3} s"), - MetricUnit::Count => format_integerish(value), - MetricUnit::Ratio => format!("{value:.4}"), - MetricUnit::Custom => format_float(value), +fn format_metric_value(value: f64, unit: &MetricUnit) -> String { + match unit.known_kind() { + Some(KnownMetricUnit::Bytes) => format!("{} B", format_integerish(value)), + Some(KnownMetricUnit::Seconds) => format!("{value:.3} s"), + Some(KnownMetricUnit::Milliseconds) => format!("{value:.3} ms"), + Some(KnownMetricUnit::Microseconds) => format!("{} us", format_integerish(value)), + Some(KnownMetricUnit::Nanoseconds) => format!("{} ns", format_integerish(value)), + Some(KnownMetricUnit::Count) => format_integerish(value), + Some(KnownMetricUnit::Ratio) => format!("{value:.4}"), + Some(KnownMetricUnit::Percent) => format!("{value:.2}%"), + Some(KnownMetricUnit::Scalar) | None => { + if unit.as_str() == "scalar" { + format_float(value) + } else { + format!("{} {}", format_float(value), unit.as_str()) + } + } } } diff --git a/crates/fidget-spinner-cli/tests/mcp_hardening.rs b/crates/fidget-spinner-cli/tests/mcp_hardening.rs index c086a45..86b6719 100644 --- a/crates/fidget-spinner-cli/tests/mcp_hardening.rs +++ b/crates/fidget-spinner-cli/tests/mcp_hardening.rs @@ -453,6 +453,64 @@ fn registry_and_history_surfaces_render_timestamps_as_strings() -> TestResult { Ok(()) } +#[test] +fn metric_define_accepts_builtin_and_custom_unit_tokens() -> TestResult { + let project_root = temp_project_root("metric_units")?; + init_project(&project_root)?; + + let mut harness = McpHarness::spawn(Some(&project_root))?; + let _ = harness.initialize()?; + harness.notify_initialized()?; + + let microseconds = harness.call_tool_full( + 23, + "metric.define", + json!({ + "key": "oracle_solve_wallclock_micros", + "unit": "micros", + "objective": "minimize", + "visibility": "canonical", + }), + )?; + assert_tool_ok(µseconds); + assert_eq!( + tool_content(µseconds)["record"]["unit"].as_str(), + Some("microseconds") + ); + + let custom = harness.call_tool_full( + 24, + "metric.define", + json!({ + "key": "root_lp_objective_last", + "unit": "objective", + "objective": "minimize", + "visibility": "canonical", + }), + )?; + assert_tool_ok(&custom); + assert_eq!( + tool_content(&custom)["record"]["unit"].as_str(), + Some("objective") + ); + + let placeholder = harness.call_tool( + 25, + "metric.define", + json!({ + "key": "bad_custom_placeholder", + "unit": "custom", + "objective": "minimize", + }), + )?; + assert_tool_error(&placeholder); + assert!( + must_some(tool_error_message(&placeholder), "metric unit error")?.contains("metric unit") + ); + + Ok(()) +} + #[test] fn hypothesis_body_discipline_is_enforced_over_mcp() -> TestResult { let project_root = temp_project_root("single_paragraph")?; -- cgit v1.2.3