fix(server): cap query_metric_series window at retention, not a flat 90 days (JEF-593)

server/src/api.rs

@@ -513,17 +513,42 @@ pub async fn metric_series(
     Ok(Json(query_metric_series(&pool, q).await.map_err(internal)?))
 }
 
+/// Hard ceiling (hours) on `query_metric_series`'s window, sourced from
+/// `WATCHER_RETENTION_DAYS` (JEF-593) — the same knob `retention::prune_once`
+/// prunes `metric_series_rollups` against — rather than a hard-wired constant,
+/// so raising retention widens the queryable window without a code change.
+/// Falls back to the sibling facet/histogram/hist_facet queries' fixed
+/// `24 * 7` ceiling both when unset/unparsable and when retention is disabled
+/// (`<= 0`), since a disabled sweep prunes nothing and shouldn't be read as an
+/// unbounded scan window.
+fn metric_series_max_hours() -> i32 {
+    parse_retention_max_hours(std::env::var("WATCHER_RETENTION_DAYS").ok().as_deref())
+}
+
+/// Pure helper behind `metric_series_max_hours()`, extracted so it can be
+/// unit-tested without mutating process-global env state.
+fn parse_retention_max_hours(raw: Option<&str>) -> i32 {
+    match raw.and_then(|s| s.parse::<i32>().ok()) {
+        Some(days) if days > 0 => days * 24,
+        _ => 24 * 7,
+    }
+}
+
 /// One metric's collapsed time series, shared by the HTTP handler and the MCP
 /// `metric_series` tool. Applies the same hours clamp, and honors an absolute
-/// `from`/`to` window when given (see `SeriesQuery`). The 90-day ceiling here is
-/// deliberate (JEF-561): the covering index (JEF-548) makes the read itself
-/// cheap, and the adaptive output bucket below keeps the *result* bounded
-/// regardless of window width, so there's no remaining cost to widening it.
+/// `from`/`to` window when given (see `SeriesQuery`). The ceiling used to be a
+/// flat 90 days on the theory that the covering index (JEF-548) makes the read
+/// cheap and the adaptive output bucket below keeps the *result* bounded
+/// regardless of window width — but `metric_series_rollups` is itself pruned at
+/// `WATCHER_RETENTION_DAYS` (default 7), so anything past that structurally
+/// holds no rows: widening the scan range there was pure waste. Capped to
+/// `metric_series_max_hours()` instead (JEF-593), which tracks retention.
 pub async fn query_metric_series(
     pool: &PgPool,
     q: SeriesQuery,
 ) -> Result<Vec<SeriesPoint>, sqlx::Error> {
-    let (lo, hi, width) = resolve_window_and_width(q.hours, 24, 24 * 90, q.from, q.to);
+    let (lo, hi, width) =
+        resolve_window_and_width(q.hours, 24, metric_series_max_hours(), q.from, q.to);
     sqlx::query_as::<_, SeriesPoint>(
         "SELECT metric_bucket(bucket, $5) AS t, sum(sum) / nullif(sum(count), 0) AS v
          FROM metric_series_rollups
@@ -1466,6 +1491,56 @@ mod tests {
         assert_eq!(parse_max_lookback_hours(Some("48")), 48);
     }
 
+    // JEF-593: at the default 7-day retention, the series ceiling must match
+    // the sibling facet/histogram/hist_facet queries' fixed `24 * 7` — the old
+    // 90-day ceiling scanned 83 days of range that retention had already
+    // pruned to nothing.
+    #[test]
+    fn parse_retention_max_hours_matches_sibling_ceiling_at_default_retention() {
+        assert_eq!(parse_retention_max_hours(None), 24 * 7);
+    }
+
+    #[test]
+    fn parse_retention_max_hours_tracks_a_non_default_retention() {
+        assert_eq!(parse_retention_max_hours(Some("3")), 24 * 3);
+        assert_eq!(parse_retention_max_hours(Some("30")), 24 * 30);
+    }
+
+    // A disabled sweep (`<= 0`) prunes nothing, but must not be read as license
+    // to scan an unbounded range — fall back to the sibling ceiling instead.
+    #[test]
+    fn parse_retention_max_hours_falls_back_to_sibling_ceiling_when_retention_disabled() {
+        assert_eq!(parse_retention_max_hours(Some("0")), 24 * 7);
+        assert_eq!(parse_retention_max_hours(Some("-1")), 24 * 7);
+    }
+
+    #[test]
+    fn parse_retention_max_hours_falls_back_to_sibling_ceiling_when_invalid() {
+        assert_eq!(parse_retention_max_hours(Some("not a number")), 24 * 7);
+    }
+
+    // Acceptance criterion: a >retention window request is clamped to the
+    // retention bound, exactly like JEF-532's absolute-window clamp test above
+    // — but driven by the retention-derived ceiling instead of a literal.
+    #[test]
+    fn resolve_window_clamps_series_window_to_retention_ceiling() {
+        let hi = Utc::now();
+        let from = hi - Duration::days(90);
+        let (lo, resolved_hi) = resolve_window(
+            None,
+            24,
+            parse_retention_max_hours(None),
+            Some(from),
+            Some(hi),
+        );
+        assert_eq!(resolved_hi, hi);
+        assert_eq!(
+            lo,
+            hi - Duration::hours(24 * 7),
+            "a 90-day span should be clamped down to the 7-day retention ceiling"
+        );
+    }
+
     // JEF-561: a narrow window (well under TARGET_POINTS * base) must collapse
     // to exactly `base` — unwidened, full rollup resolution — so a short-range
     // chart's points are unaffected.

server/tests/smoke.rs

@@ -740,16 +740,17 @@ async fn metric_series_honors_absolute_from_to_window() {
     assert_eq!(buckets[0]["counts"], serde_json::json!([0, 5, 0]));
 }
 
-/// The series endpoint's window can be up to 90 days wide (`resolve_window`'s
-/// `max_hours` for `/api/metrics/series`, JEF-548) — this is the query whose
-/// wide-window rollup scan was traced holding a pool connection for tens of
-/// seconds (the fix: a covering index on `metric_series_rollups`, migration
-/// 0017). This doesn't benchmark the index directly (no way to force a plan
-/// choice through the HTTP API), but it does pin the behavior the index must
-/// not change: a request at the 90-day ceiling returns exactly the points
-/// inside that window — none dropped, none leaked in from just outside it —
-/// so a future change to the index or the query can't silently narrow or
-/// widen what's returned.
+/// The series endpoint's window is capped at the retention-derived ceiling
+/// (`resolve_window`'s `max_hours` for `/api/metrics/series` — `24 * 7` at the
+/// default `WATCHER_RETENTION_DAYS`, JEF-593; previously a flat 90 days,
+/// JEF-548). Originally the query whose wide-window rollup scan was traced
+/// holding a pool connection for tens of seconds (the fix: a covering index on
+/// `metric_series_rollups`, migration 0017). This doesn't benchmark the index
+/// directly (no way to force a plan choice through the HTTP API), but it does
+/// pin the behavior the index must not change: a request at (or past) the
+/// 7-day ceiling returns exactly the points inside that window — none
+/// dropped, none leaked in from just outside it — so a future change to the
+/// index or the query can't silently narrow or widen what's returned.
 #[tokio::test]
 #[serial]
 async fn metric_series_wide_window_returns_bounded_correct_points() {
@@ -758,52 +759,54 @@ async fn metric_series_wide_window_returns_bounded_correct_points() {
         return;
     };
     let day = 86_400.0;
-    insert_rollup_at(&pool, "wide.metric", 89.0 * day).await;
-    insert_rollup_at(&pool, "wide.metric", 45.0 * day).await;
-    insert_rollup_at(&pool, "wide.metric", 10.0 * day).await;
-    // Just past the 90-day ceiling: must be excluded, not silently included.
-    insert_rollup_at(&pool, "wide.metric", 91.0 * day).await;
+    insert_rollup_at(&pool, "wide.metric", 6.0 * day).await;
+    insert_rollup_at(&pool, "wide.metric", 3.0 * day).await;
+    insert_rollup_at(&pool, "wide.metric", 1.0 * day).await;
+    // Just past the 7-day ceiling: must be excluded, not silently included.
+    insert_rollup_at(&pool, "wide.metric", 8.0 * day).await;
 
     let router = app(pool);
+    // Requests far past the ceiling (2160h = 90d) must still clamp to it.
     let (status, series) =
         get_json(&router, "/api/metrics/series?name=wide.metric&hours=2160").await;
     assert_eq!(status, StatusCode::OK);
     let arr = series.as_array().expect("array");
     assert_eq!(
         arr.len(),
         3,
-        "the 91-day-old point is outside the 90-day window, series = {arr:?}"
+        "the 8-day-old point is outside the 7-day window, series = {arr:?}"
     );
     for point in arr {
         assert_eq!(point["v"], 1.0);
     }
-    // Ascending by time — oldest (89d) first, newest (10d) last.
+    // Ascending by time — oldest (6d) first, newest (1d) last.
     let times: Vec<&str> = arr.iter().map(|p| p["t"].as_str().unwrap()).collect();
     let mut sorted = times.clone();
     sorted.sort();
     assert_eq!(times, sorted, "points must be ordered t ASC");
 }
 
-/// JEF-561: at the 90-day ceiling the adaptive output bucket widens from the
-/// base 300s rollup bucket to `300 * ceil((2160h in secs / 300) / 1000) = 7800s`
-/// (see `output_bucket_secs`'s unit tests for that math) — a chart gets ~1000
-/// points instead of ~26k. This pins that the widening re-aggregates rollup
-/// rows *correctly*: two rows placed well inside the same 7800s output bucket
-/// must collapse into one point whose value is the count-weighted average of
-/// both (not a plain average), while a row a full bucket-width away stays its
-/// own point, unmerged.
+/// JEF-561/JEF-593: at the (now retention-derived) 7-day ceiling the adaptive
+/// output bucket widens from the base 300s rollup bucket to
+/// `300 * ceil((168h in secs / 300) / 1000) = 900s` (see `output_bucket_secs`'s
+/// unit tests for that math) — a chart gets bounded output instead of one
+/// point per raw rollup bucket. This pins that the widening re-aggregates
+/// rollup rows *correctly*: two rows placed well inside the same 900s output
+/// bucket must collapse into one point whose value is the count-weighted
+/// average of both (not a plain average), while a row a full bucket-width
+/// away stays its own point, unmerged.
 #[tokio::test]
 #[serial]
 async fn metric_series_wide_window_reaggregates_merged_buckets_correctly() {
     let Some(pool) = pool_or_skip().await else {
         eprintln!("skipping: DATABASE_URL not set");
         return;
     };
-    const WIDTH: f64 = 7800.0;
+    const WIDTH: f64 = 900.0;
     let now_epoch = chrono::Utc::now().timestamp() as f64;
-    // A bucket safely inside the 90-day window (well clear of "now" and of the
-    // 91-day exclusion edge exercised by the sibling test above).
-    let bucket_start = ((now_epoch - 5.0 * 86_400.0) / WIDTH).floor() * WIDTH;
+    // A bucket safely inside the 7-day window (well clear of "now" and of the
+    // ceiling edge exercised by the sibling test above).
+    let bucket_start = ((now_epoch - 3.5 * 86_400.0) / WIDTH).floor() * WIDTH;
     let secs_ago_at = |frac: f64| now_epoch - (bucket_start + WIDTH * frac);
 
     // Two rows well inside the same output bucket (30%/60% across it — a wide