Fix regex help icon click-toggle and add PromQL by()/sum(rate()) grouping

RegexHelpIcon previously used a single `open` boolean driven by both hover
and click, so clicking to pin the popover open was undone by the next
mouseleave. Split into separate `pinned` (click) and `hovering` (mouse-over)
states so a click-toggle actually persists until clicked again.

The seeded `log_category` dashboard's line panel plotted the bare metric
name, since `sum()`/`rate()` always collapsed all label combinations into
one series and PromQL syntax had no way to preserve a label dimension
through an aggregation. Added `by (label, ...)` grouping and nested
`sum(rate(metric[range]))`/`avg(rate(...))` parsing, and made `rate()`
compute a real windowed time series instead of a single trailing point, so
a query like `sum(rate(log_category[1m])) by (category)` now renders one
line per category.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o

src/engine.rs

@@ -1147,7 +1147,82 @@ fn matches_label_matchers(
     })
 }
 
-/// Evaluates a parsed PromQL query against `index`, producing chart-ready series: baseline is one series per label combination; `sum()`/`avg()` merge series by timestamp into one label-less series; `rate()` is simplified to a single point over `(last_ts - range, last_ts]` (an occurrence-count-per-second reading, not PromQL's true sliding-window-per-step) since bucket values are occurrence markers, not cumulative counters.
+/// Projects a full label map down to just the names in `by_labels`; an empty `by_labels` collapses
+/// everything to the empty map, i.e. "one group total" (the historical `sum()`/`avg()` behavior).
+fn project_labels(
+    labels: &BTreeMap<String, String>,
+    by_labels: &[String],
+) -> BTreeMap<String, String> {
+    by_labels
+        .iter()
+        .filter_map(|name| labels.get(name).map(|value| (name.clone(), value.clone())))
+        .collect()
+}
+
+/// Turns one series' occurrence points into a windowed rate: at each of the series' own timestamps,
+/// counts how many points fall in the trailing `(ts - range_ms, ts]` window and divides by the window
+/// width in seconds. Points are occurrence markers rather than cumulative counters, so this reads as
+/// "occurrences per second over the trailing window", evaluated at every point instead of just the last.
+fn windowed_rate_series(points: &[(i64, f64)], range_ms: i64) -> Vec<(i64, f64)> {
+    let range_seconds = range_ms as f64 / 1000.0;
+    points
+        .iter()
+        .map(|(ts, _)| {
+            let window_start = ts - range_ms;
+            let count_in_window =
+                points.iter().filter(|(t, _)| *t > window_start && *t <= *ts).count();
+            (*ts, count_in_window as f64 / range_seconds)
+        })
+        .collect()
+}
+
+/// Merges several series by exact timestamp match: values sharing a timestamp are summed (or averaged).
+/// This is the plain `sum()`/`avg()` behavior — appropriate when the underlying points are occurrence
+/// markers rather than a continuous reading, so there's nothing to carry forward between timestamps.
+fn merge_series_exact(sub_series: &[Vec<(i64, f64)>], is_avg: bool) -> Vec<(i64, f64)> {
+    let mut grouped: BTreeMap<i64, Vec<f64>> = BTreeMap::new();
+    for points in sub_series {
+        for (timestamp_ms, value) in points {
+            grouped.entry(*timestamp_ms).or_default().push(*value);
+        }
+    }
+    grouped
+        .into_iter()
+        .map(|(timestamp_ms, values)| {
+            let sum: f64 = values.iter().sum();
+            let value = if is_avg { sum / values.len() as f64 } else { sum };
+            (timestamp_ms, value)
+        })
+        .collect()
+}
+
+/// Merges several already-windowed rate series by sweeping the union of their timestamps, carrying each
+/// sub-series' last-known value forward (0.0 before its first point) so every step reflects every
+/// sub-series' most recent reading — unlike `merge_series_exact`, which only combines exact-timestamp matches.
+fn merge_series_locf(sub_series: &[Vec<(i64, f64)>], is_avg: bool) -> Vec<(i64, f64)> {
+    let mut all_timestamps: std::collections::BTreeSet<i64> = std::collections::BTreeSet::new();
+    for points in sub_series {
+        all_timestamps.extend(points.iter().map(|(ts, _)| *ts));
+    }
+    let mut cursors = vec![0usize; sub_series.len()];
+    let mut last_values = vec![0.0f64; sub_series.len()];
+    all_timestamps
+        .into_iter()
+        .map(|ts| {
+            for (i, points) in sub_series.iter().enumerate() {
+                while cursors[i] < points.len() && points[cursors[i]].0 <= ts {
+                    last_values[i] = points[cursors[i]].1;
+                    cursors[i] += 1;
+                }
+            }
+            let sum: f64 = last_values.iter().sum();
+            let value = if is_avg { sum / last_values.len() as f64 } else { sum };
+            (ts, value)
+        })
+        .collect()
+}
+
+/// Evaluates a parsed PromQL query against `index`, producing chart-ready series: baseline is one series per label combination; `sum()`/`avg()` merge series into one series per `by_labels` group (the whole result collapses to a single label-less series when `by_labels` is empty); `rate()` — bare or wrapped in `sum()`/`avg()` — turns each series into a windowed occurrences-per-second reading via `windowed_rate_series`, so it plots as a real line rather than one trailing point.
 fn evaluate_promql(
     index: &BTreeMap<IndexKey, DataValue>,
     parsed: &PromqlQuery,
@@ -1179,36 +1254,42 @@ fn evaluate_promql(
             .map(|(labels, points)| PromqlSeriesDto { labels, points })
             .collect(),
         Some(AggOp::Sum) | Some(AggOp::Avg) => {
-            let mut grouped: BTreeMap<i64, Vec<f64>> = BTreeMap::new();
-            for points in series_points.values() {
-                for (timestamp_ms, value) in points {
-                    grouped.entry(*timestamp_ms).or_default().push(*value);
-                }
-            }
             let is_avg = parsed.agg == Some(AggOp::Avg);
-            let points = grouped
+            // Validated by `promql::parse_query`: an inner rate() always carries a range.
+            let rate_range_ms = parsed
+                .inner_rate
+                .then(|| parsed.range.expect("inner rate() always carries a range").as_millis() as i64);
+
+            let mut groups: BTreeMap<BTreeMap<String, String>, Vec<Vec<(i64, f64)>>> = BTreeMap::new();
+            for (labels, points) in series_points {
+                let points = match rate_range_ms {
+                    Some(range_ms) => windowed_rate_series(&points, range_ms),
+                    None => points,
+                };
+                let group_key = project_labels(&labels, &parsed.by_labels);
+                groups.entry(group_key).or_default().push(points);
+            }
+
+            groups
                 .into_iter()
-                .map(|(timestamp_ms, values)| {
-                    let sum: f64 = values.iter().sum();
-                    let value = if is_avg { sum / values.len() as f64 } else { sum };
-                    (timestamp_ms, value)
+                .map(|(group_labels, sub_series)| {
+                    let points = if rate_range_ms.is_some() {
+                        merge_series_locf(&sub_series, is_avg)
+                    } else {
+                        merge_series_exact(&sub_series, is_avg)
+                    };
+                    PromqlSeriesDto { labels: group_labels, points }
                 })
-                .collect();
-            vec![PromqlSeriesDto { labels: BTreeMap::new(), points }]
+                .collect()
         }
         Some(AggOp::Rate) => {
             // Validated by `promql::parse_query`: `rate()` never parses without one.
             let range_ms = parsed.range.expect("rate() always carries a range").as_millis() as i64;
-            let range_seconds = range_ms as f64 / 1000.0;
             series_points
                 .into_iter()
-                .filter_map(|(labels, points)| {
-                    let last_ts = points.last()?.0;
-                    let window_start = last_ts - range_ms;
-                    let count_in_window =
-                        points.iter().filter(|(ts, _)| *ts > window_start && *ts <= last_ts).count();
-                    let rate = count_in_window as f64 / range_seconds;
-                    Some(PromqlSeriesDto { labels, points: vec![(last_ts, rate)] })
+                .map(|(labels, points)| PromqlSeriesDto {
+                    labels,
+                    points: windowed_rate_series(&points, range_ms),
                 })
                 .collect()
         }
@@ -1925,6 +2006,73 @@ mod tests {
         assert_eq!(series[0].points, vec![(0, 2.0)]);
     }
 
+    #[test]
+    fn promql_sum_by_preserves_the_named_label_dimension() {
+        let mut index = BTreeMap::new();
+        declare_requests_metric(&mut index);
+        ingest_log(&mut index, 0, "request path=/a");
+        ingest_log(&mut index, 0, "request path=/b");
+        ingest_log(&mut index, 1, "request path=/a");
+
+        let parsed = promql::parse_query("sum(requests) by (path)").unwrap();
+        let mut series = evaluate_promql(&index, &parsed, None, None);
+        series.sort_by(|a, b| a.labels.get("path").cmp(&b.labels.get("path")));
+
+        assert_eq!(series.len(), 2);
+        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
+        assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0)]);
+        assert_eq!(series[1].labels.get("path").map(String::as_str), Some("/b"));
+        assert_eq!(series[1].points, vec![(0, 1.0)]);
+    }
+
+    #[test]
+    fn bare_rate_produces_a_windowed_point_per_timestamp_not_a_single_point() {
+        let mut index = BTreeMap::new();
+        declare_requests_metric(&mut index);
+        ingest_log(&mut index, 0, "request path=/a");
+        ingest_log(&mut index, 1000, "request path=/a");
+
+        let parsed = promql::parse_query("rate(requests[2s])").unwrap();
+        let series = evaluate_promql(&index, &parsed, None, None);
+
+        assert_eq!(series.len(), 1);
+        assert_eq!(series[0].points, vec![(0, 0.5), (1000, 1.0)]);
+    }
+
+    #[test]
+    fn sum_of_rate_by_label_gives_each_label_value_its_own_line() {
+        let mut index = BTreeMap::new();
+        declare_requests_metric(&mut index);
+        ingest_log(&mut index, 0, "request path=/a");
+        ingest_log(&mut index, 1000, "request path=/a");
+        ingest_log(&mut index, 0, "request path=/b");
+
+        let parsed = promql::parse_query("sum(rate(requests[2s])) by (path)").unwrap();
+        let mut series = evaluate_promql(&index, &parsed, None, None);
+        series.sort_by(|a, b| a.labels.get("path").cmp(&b.labels.get("path")));
+
+        assert_eq!(series.len(), 2);
+        assert_eq!(series[0].labels.get("path").map(String::as_str), Some("/a"));
+        assert_eq!(series[0].points, vec![(0, 0.5), (1000, 1.0)]);
+        assert_eq!(series[1].labels.get("path").map(String::as_str), Some("/b"));
+        assert_eq!(series[1].points, vec![(0, 0.5)]);
+    }
+
+    #[test]
+    fn sum_of_rate_without_by_clause_still_collapses_to_one_series() {
+        let mut index = BTreeMap::new();
+        declare_requests_metric(&mut index);
+        ingest_log(&mut index, 0, "request path=/a");
+        ingest_log(&mut index, 0, "request path=/b");
+
+        let parsed = promql::parse_query("sum(rate(requests[2s]))").unwrap();
+        let series = evaluate_promql(&index, &parsed, None, None);
+
+        assert_eq!(series.len(), 1);
+        assert!(series[0].labels.is_empty());
+        assert_eq!(series[0].points, vec![(0, 1.0)]);
+    }
+
     #[test]
     fn promql_range_bounds_filter_points_via_start_end_ms() {
         let mut index = BTreeMap::new();

src/profile.rs

@@ -236,7 +236,7 @@ fn seeded_default_dashboard() -> Dashboard {
         Panel {
             id: generate_id("error-volume-line"),
             title: "Error vs. non-error volume".to_string(),
-            promql: SEEDED_METRIC_NAME.to_string(),
+            promql: format!("sum(rate({SEEDED_METRIC_NAME}[1m])) by ({SEEDED_METRIC_LABEL})"),
             chart_type: ChartType::Line,
         },
         Panel {
@@ -793,9 +793,11 @@ mod tests {
         assert_eq!(default_profile.dashboards.len(), 1);
         let dashboard = &default_profile.dashboards[0];
         assert_eq!(dashboard.panels.len(), 2);
-        assert!(dashboard.panels.iter().all(|p| p.promql == SEEDED_METRIC_NAME));
-        assert!(dashboard.panels.iter().any(|p| p.chart_type == ChartType::Line));
-        assert!(dashboard.panels.iter().any(|p| p.chart_type == ChartType::Pie));
+        assert!(dashboard
+            .panels
+            .iter()
+            .any(|p| p.chart_type == ChartType::Line && p.promql.contains(SEEDED_METRIC_NAME)));
+        assert!(dashboard.panels.iter().any(|p| p.chart_type == ChartType::Pie && p.promql == SEEDED_METRIC_NAME));
 
         // Reload (simulating a restart) and ensure_default again: must not re-seed or duplicate.
         let mut store = ProfileStore::load(&store_path);

src/promql.rs

@@ -1,5 +1,6 @@
 //! Hand-rolled parser for a small PromQL subset: metric name + label matchers, an optional `[5m]` range, and a top-level `sum()`/`avg()`/`rate()`/`histogram()`/`gauge()` wrapper.
 //! Only exact-match (`=`) label matchers are supported; `rate()` requires a duration range selector, while `histogram(metric[width])` reuses that `[...]` slot for a bare bucket width instead.
+//! `sum()`/`avg()` may additionally wrap a `rate(...)` call directly (e.g. `sum(rate(requests[5m]))`), and either may be followed by a `by (label, ...)` clause to preserve one series per distinct value of the named labels instead of collapsing to a single series.
 
 use std::time::Duration;
 
@@ -29,6 +30,12 @@ pub struct PromqlQuery {
     pub range: Option<Duration>,
     /// Bare-number range selector, e.g. `[10]`; only meaningful for `histogram()`, where it's the bucket width.
     pub bucket_width: Option<f64>,
+    /// Set when `agg` is `Sum`/`Avg` and it wraps an inner `rate(metric[range])` rather than a bare vector
+    /// selector, e.g. `sum(rate(requests[5m]))`. `range` above carries the inner `rate()`'s duration in that case.
+    pub inner_rate: bool,
+    /// Labels named in a trailing `by (...)` clause; empty means "collapse to a single series" (the historical
+    /// behavior of `sum()`/`avg()`). When non-empty, one series is kept per distinct combination of these labels.
+    pub by_labels: Vec<String>,
 }
 
 /// What a `[...]` range selector turned out to hold: a duration, or a bare width.
@@ -217,7 +224,51 @@ impl<'a> Parser<'a> {
             Some(RangeSelector::Duration(d)) => (Some(d), None),
             Some(RangeSelector::BareWidth(w)) => (None, Some(w)),
         };
-        Ok(PromqlQuery { agg: None, metric_name, matchers, range, bucket_width })
+        Ok(PromqlQuery {
+            agg: None,
+            metric_name,
+            matchers,
+            range,
+            bucket_width,
+            inner_rate: false,
+            by_labels: Vec::new(),
+        })
+    }
+
+    /// Parses a trailing `by (label, ...)` clause, if present; returns an empty vec otherwise.
+    fn parse_by_clause(&mut self) -> Result<Vec<String>, String> {
+        self.skip_whitespace();
+        let checkpoint = self.pos;
+        let Ok(ident) = self.parse_identifier() else {
+            self.pos = checkpoint;
+            return Ok(Vec::new());
+        };
+        if ident != "by" {
+            self.pos = checkpoint;
+            return Ok(Vec::new());
+        }
+        self.skip_whitespace();
+        self.expect_char('(')?;
+        let mut labels = Vec::new();
+        self.skip_whitespace();
+        if self.peek() == Some(')') {
+            self.expect_char(')')?;
+            return Err("by () clause must name at least one label".to_string());
+        }
+        loop {
+            labels.push(self.parse_identifier()?);
+            self.skip_whitespace();
+            match self.peek() {
+                Some(',') => {
+                    self.pos += 1;
+                }
+                Some(')') => break,
+                Some(c) => return Err(format!("expected ',' or ')', found '{c}'")),
+                None => return Err("unterminated by clause (missing ')')".to_string()),
+            }
+        }
+        self.expect_char(')')?;
+        Ok(labels)
     }
 
     fn parse_query(&mut self) -> Result<PromqlQuery, String> {
@@ -235,9 +286,39 @@ impl<'a> Parser<'a> {
         self.skip_whitespace();
         if agg.is_some() && self.peek() == Some('(') {
             self.expect_char('(')?;
-            let mut inner = self.parse_vector_selector()?;
+            self.skip_whitespace();
+            // `sum()`/`avg()` may wrap `rate(metric[range])` directly instead of a bare vector selector.
+            let mut inner = if matches!(agg, Some(AggOp::Sum) | Some(AggOp::Avg)) {
+                let inner_checkpoint = self.pos;
+                let took_inner_rate = self.parse_identifier().is_ok_and(|id| id == "rate");
+                if took_inner_rate {
+                    self.skip_whitespace();
+                    if self.peek() == Some('(') {
+                        self.expect_char('(')?;
+                        let mut vector = self.parse_vector_selector()?;
+                        self.expect_char(')')?;
+                        if vector.range.is_none() {
+                            return Err(
+                                "rate() requires a range selector, e.g. rate(my_metric[5m])"
+                                    .to_string(),
+                            );
+                        }
+                        vector.inner_rate = true;
+                        vector
+                    } else {
+                        self.pos = inner_checkpoint;
+                        self.parse_vector_selector()?
+                    }
+                } else {
+                    self.pos = inner_checkpoint;
+                    self.parse_vector_selector()?
+                }
+            } else {
+                self.parse_vector_selector()?
+            };
             self.expect_char(')')?;
             inner.agg = agg;
+            inner.by_labels = self.parse_by_clause()?;
             Ok(inner)
         } else {
             // Rewind and parse as a plain vector selector, covering metric names that collide with an agg_op keyword.
@@ -390,4 +471,60 @@ mod tests {
         assert_eq!(query.range, Some(Duration::from_secs(300)));
         assert_eq!(query.bucket_width, None);
     }
+
+    #[test]
+    fn parses_sum_of_rate() {
+        let query = parse_query("sum(rate(requests[1m]))").unwrap();
+        assert_eq!(query.agg, Some(AggOp::Sum));
+        assert!(query.inner_rate);
+        assert_eq!(query.metric_name, "requests");
+        assert_eq!(query.range, Some(Duration::from_secs(60)));
+        assert!(query.by_labels.is_empty());
+    }
+
+    #[test]
+    fn parses_avg_of_rate_with_label_matchers() {
+        let query = parse_query(r#"avg(rate(requests{path="/a"}[5m]))"#).unwrap();
+        assert_eq!(query.agg, Some(AggOp::Avg));
+        assert!(query.inner_rate);
+        assert_eq!(query.matchers, vec![LabelMatcher { name: "path".to_string(), value: "/a".to_string() }]);
+    }
+
+    #[test]
+    fn rejects_sum_of_rate_without_range() {
+        let err = parse_query("sum(rate(requests))").unwrap_err();
+        assert!(err.contains("range selector"));
+    }
+
+    #[test]
+    fn parses_sum_with_by_clause() {
+        let query = parse_query("sum(requests) by (path)").unwrap();
+        assert_eq!(query.agg, Some(AggOp::Sum));
+        assert!(!query.inner_rate);
+        assert_eq!(query.by_labels, vec!["path".to_string()]);
+    }
+
+    #[test]
+    fn parses_sum_of_rate_with_by_clause_multiple_labels() {
+        let query = parse_query("sum(rate(requests[1m])) by (path, method)").unwrap();
+        assert!(query.inner_rate);
+        assert_eq!(query.by_labels, vec!["path".to_string(), "method".to_string()]);
+    }
+
+    #[test]
+    fn sum_without_by_clause_has_empty_by_labels() {
+        let query = parse_query("sum(requests)").unwrap();
+        assert!(query.by_labels.is_empty());
+    }
+
+    #[test]
+    fn rejects_empty_by_clause() {
+        let err = parse_query("sum(requests) by ()").unwrap_err();
+        assert!(err.contains("at least one label"));
+    }
+
+    #[test]
+    fn rejects_unterminated_by_clause() {
+        assert!(parse_query("sum(requests) by (path").is_err());
+    }
 }

webapp/src/components/RegexHelpIcon.tsx

@@ -23,23 +23,32 @@ const CHEAT_SHEET: { tokens: string; meaning: string }[] = [
 // "@" icon next to regex inputs; shows a compact cheat sheet for the regex flavor the server actually
 // uses (Rust `regex` crate — no lookaround/backreferences), modeled on HelpIcon's button styling.
 export function RegexHelpIcon({ label }: { label?: string }) {
-  const [open, setOpen] = useState(false)
+  // Click pins the popover open until clicked again; hovering shows it only while the mouse
+  // stays over the icon/box, independent of the pinned state.
+  const [pinned, setPinned] = useState(false)
+  const [hovering, setHovering] = useState(false)
+  const open = pinned || hovering
   const ref = useRef<HTMLDivElement>(null)
 
   useEffect(() => {
-    if (!open) return
+    if (!pinned) return
     function onClickOutside(e: MouseEvent) {
-      if (ref.current && !ref.current.contains(e.target as Node)) setOpen(false)
+      if (ref.current && !ref.current.contains(e.target as Node)) setPinned(false)
     }
     document.addEventListener('mousedown', onClickOutside)
     return () => document.removeEventListener('mousedown', onClickOutside)
-  }, [open])
+  }, [pinned])
 
   return (
-    <div ref={ref} className="relative inline-flex" onMouseEnter={() => setOpen(true)} onMouseLeave={() => setOpen(false)}>
+    <div
+      ref={ref}
+      className="relative inline-flex"
+      onMouseEnter={() => setHovering(true)}
+      onMouseLeave={() => setHovering(false)}
+    >
       <button
         type="button"
-        onClick={() => setOpen((v) => !v)}
+        onClick={() => setPinned((v) => !v)}
         title="Regex syntax cheat sheet"
         aria-label={label ?? 'Regex syntax cheat sheet'}
         className="flex h-5 w-5 shrink-0 items-center justify-center rounded-full border border-border-soft bg-surface-2 text-xs leading-none text-text-muted hover:border-border-strong hover:text-text-secondary"