Open the metric unit vocabulary

4 files changed, 87 insertions, 41 deletions

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)]
@@ -582,15 +582,6 @@ struct SkillShowArgs {
 }
 
 #[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
-enum CliMetricUnit {
-    Seconds,
-    Bytes,
-    Count,
-    Ratio,
-    Custom,
-}
-
-#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
 enum CliOptimizationObjective {
     Minimize,
     Maximize,
@@ -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<CliMetricUnit> 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<CliOptimizationObjective> 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
@@ -454,6 +454,64 @@ fn registry_and_history_surfaces_render_timestamps_as_strings() -> TestResult {
 }
 
 #[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(&microseconds);
+    assert_eq!(
+        tool_content(&microseconds)["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")?;
     init_project(&project_root)?;