From 375aa38ef86d3a992e0d467c190bac3e01b9f76d Mon Sep 17 00:00:00 2001
From: Hugh Kaznowski <hughkaznowski@Hughs-MacBook-Pro.local>
Date: Sat, 5 Sep 2026 22:50:46 +0100
Subject: [PATCH] Validate metric[param] filters against declared label params
 (ticket 039)

compile_metric_comparison used to look up `param` by reading whatever key
happened to be present in a bucket's `labels` map at query time, so a typo
or a param never declared as a label on that metric silently matched
nothing instead of being rejected. Now it looks up the target metric's
MetricDeclaration (ticket 037's unified label_params/value_param shape) and
rejects the filter up front with an error naming the metric and its actual
declared label params when `param` isn't among them.

Metric names with no declaration at all are left unvalidated here, matching
prior behavior (bucket_values simply yields nothing for them).

Added tests: metric[nonexistent_param] is rejected with an error naming the
metric and its real label params; existing metric[real_param] filters still
match as before.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
---
 src/engine.rs | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/src/engine.rs b/src/engine.rs
index 802d909..f30eac6 100644
--- a/src/engine.rs
+++ b/src/engine.rs
@@ -746,6 +746,23 @@ fn compile_metric_comparison(
         )
     })?;
 
+    // Validate `param` against the metric's actual declared `label_params` (ticket 037's unified
+    // `MetricDeclaration`) rather than just reading whatever key happens to be present in a
+    // bucket's `labels` map — a typo or a param that was never declared as a label on this metric
+    // would otherwise silently match nothing instead of being rejected up front. A metric name
+    // with no declaration at all is left alone here (`bucket_values` below simply yields nothing
+    // for it, matching prior behavior) since there's no label_params list to validate against.
+    if let Some(DataValue::MetricDeclaration { label_params, .. }) =
+        index.get(&IndexKey::MetricDeclaration { name: metric.to_string() })
+    {
+        if !label_params.iter().any(|p| p == param) {
+            return Err(format!(
+                "param '{param}' is not a label param of metric '{metric}'; declared label params: [{}]",
+                label_params.join(", ")
+            ));
+        }
+    }
+
     let mut matching = HashSet::new();
     for (labels, (sequence, _, _)) in bucket_values(index, metric, None) {
         let Some(primitive) = labels.get(param) else {
@@ -2941,4 +2958,52 @@ mod tests {
             _ => panic!("expected an Error response for a metric comparison without [param]"),
         }
     }
+
+    #[test]
+    fn metric_comparison_with_undeclared_param_is_rejected_naming_real_label_params() {
+        let mut index = BTreeMap::new();
+        let pattern = r"level=(?P<level>\w+) duration=(?P<duration>\d+)";
+        declare_test_param(&mut index, "level", pattern, "level", MetricPrimitiveKind::String);
+        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
+        declare_metric(&mut index, metric_request("requests", pattern, vec!["level", "duration"])).unwrap();
+        ingest_log(&mut index, 0, "level=error duration=5");
+
+        match handle_query(&index, logs_query(Some("requests[nonexistent_param]==\"x\"")), 0) {
+            EngineResponse::Error(message) => {
+                assert!(
+                    message.contains("nonexistent_param"),
+                    "error should name the offending param, got: {message}"
+                );
+                assert!(
+                    message.contains("requests"),
+                    "error should name the metric, got: {message}"
+                );
+                assert!(
+                    message.contains("level") && message.contains("duration"),
+                    "error should list the metric's real declared label params, got: {message}"
+                );
+            }
+            _ => panic!("expected an Error response for an undeclared param"),
+        }
+    }
+
+    #[test]
+    fn metric_comparison_with_valid_label_param_still_matches() {
+        let mut index = BTreeMap::new();
+        let pattern = r"level=(?P<level>\w+) duration=(?P<duration>\d+)";
+        declare_test_param(&mut index, "level", pattern, "level", MetricPrimitiveKind::String);
+        declare_test_param(&mut index, "duration", pattern, "duration", MetricPrimitiveKind::Number);
+        declare_metric(&mut index, metric_request("requests", pattern, vec!["level", "duration"])).unwrap();
+
+        ingest_log(&mut index, 0, "level=error duration=5");
+        ingest_log(&mut index, 1, "level=info duration=2");
+
+        match handle_query(&index, logs_query(Some("requests[level]==error")), 0) {
+            EngineResponse::Logs(entries) => {
+                let sequences: Vec<u64> = entries.iter().map(|e| e.sequence).collect();
+                assert_eq!(sequences, vec![0]);
+            }
+            _ => panic!("expected Logs response"),
+        }
+    }
 }