Fix Collector Cost Regression alert re-firing on an unrefreshed collector_cost row

Darling/Darling.Tests/DarlingSelfAlertTests.cs

@@ -2463,9 +2463,12 @@ public async Task JobOverCadence_IsAStandingCondition_ReFiresOnlyOnCooldown_AndW
     }
     /* ---------------- #2674 collector-cost regression self-alert ---------------- */
 
+    private static readonly DateTime DefaultRegressionMetricTime = new(2026, 7, 1, 11, 0, 0, DateTimeKind.Utc);
+
     private static PerformanceMonitor.Darling.Service.Mcp.DarlingCollectorCostReader.CostRegression Regression(
-        long latestMs = 8000, double baselineMs = 2000.0, int serverId = 7, string collector = "query_store") =>
-        new(serverId, "prod-multi-19", collector, latestMs, baselineMs);
+        long latestMs = 8000, double baselineMs = 2000.0, int serverId = 7, string collector = "query_store",
+        DateTime? latestMetricTime = null) =>
+        new(serverId, "prod-multi-19", collector, latestMs, baselineMs, latestMetricTime ?? DefaultRegressionMetricTime);
 
     [Fact]
     public async Task CollectorCostRegression_FiresOnEntry_SuppressedWithinCooldown_ResolvesWhenGone()
@@ -2504,4 +2507,31 @@ await e.ApplyCostRegressionsAsync(new[]
 
         Assert.Equal(2, h.Deliverer.Outcomes.Count);
     }
+
+    /// <summary>#2707: a cooldown-elapsed re-ask against the SAME collect.collector_cost row (unchanged
+    /// LatestMetricTime) must not re-fire — the exact shape of #2704's Poison Wait bug, here caused by the
+    /// hourly flush lagging the evaluator's own cooldown instead of a collector lagging an alert loop. A
+    /// genuinely new hourly row (LatestMetricTime advanced), still regressed, must still fire.</summary>
+    [Fact]
+    public async Task CollectorCostRegression_DoesNotRefire_OnTheSameMetricTime_EvenAfterCooldownElapses()
+    {
+        var h = new Harness();
+        var e = h.Build();
+
+        /* 1) fires once on entry. */
+        await e.ApplyCostRegressionsAsync(new[] { Regression() }, Ct);
+        Assert.Single(h.Deliverer.Outcomes);
+
+        /* 2) cooldown elapses, but the reader hands back the SAME underlying hourly row (LatestMetricTime
+           unchanged) — the flush hasn't landed a new one yet. Must not re-fire. */
+        h.Now = h.Now.AddMinutes(10);
+        await e.ApplyCostRegressionsAsync(new[] { Regression() }, Ct);
+        Assert.Single(h.Deliverer.Outcomes);
+
+        /* 3) a genuinely new hourly row lands (LatestMetricTime advanced), still regressed -> fires again. */
+        h.Now = h.Now.AddMinutes(10);
+        await e.ApplyCostRegressionsAsync(
+            new[] { Regression(latestMetricTime: DefaultRegressionMetricTime.AddHours(1)) }, Ct);
+        Assert.Equal(2, h.Deliverer.Outcomes.Count);
+    }
 }

Darling/PerformanceMonitor.Darling.Service/DarlingSelfAlertEvaluator.cs

@@ -169,6 +169,15 @@ value is the server name so a resolution has it without a second read. Same acti
        alerts — so a cheap collector, or a new one, cannot trip it. */
     private readonly ConcurrentDictionary<string, string> _activeCostRegression = new();
     private readonly ConcurrentDictionary<string, DateTime> _lastCostRegressionAlert = new();
+
+    /// <summary>#2707: the <c>collect.collector_cost</c> row this key last fired on, keyed the same as
+    /// <see cref="_lastCostRegressionAlert"/>. This evaluator is meant to run once per hourly store-metrics
+    /// tick, but if that tick's cadence ever outpaces the cooldown — or the hourly flush itself lags a tick —
+    /// re-asking <see cref="Mcp.DarlingCollectorCostReader.GetCostRegressionsAsync"/> hands back the exact
+    /// same <c>latest_ms</c> computed from the exact same underlying hourly rows, and a cooldown-elapsed check
+    /// alone cannot tell that answer from a genuinely new one. Mirrors #2704's
+    /// <c>PoisonWaitDelta.CollectionTime</c> fix for the identical shape of bug.</summary>
+    private readonly ConcurrentDictionary<string, DateTime> _lastCostRegressionDataPoint = new();
     private const double CostRegressionFactor = 2.0;
     private const long CostRegressionBaselineFloorMs = 1000;
     private static readonly TimeSpan CostRegressionBaselineWindow = TimeSpan.FromDays(14);
@@ -626,9 +635,17 @@ internal async Task ApplyCostRegressionsAsync(
             current.Add(key);
             _activeCostRegression[key] = regression.ServerName;
 
-            if (CooldownElapsed(_lastCostRegressionAlert, key, now))
+            /* #2707: the reader's own latest_metric_time is the newest collect.collector_cost row folded
+               into LatestMs — a cooldown-elapsed re-ask against a hourly flush that hasn't landed a new row
+               yet must wait for that row rather than re-fire on a total it already reported. Same shape as
+               #2704's collection-time gate on Poison Wait. */
+            bool hasFreshDataPoint = !_lastCostRegressionDataPoint.TryGetValue(key, out var lastDataPoint)
+                || regression.LatestMetricTime > lastDataPoint;
+
+            if (hasFreshDataPoint && CooldownElapsed(_lastCostRegressionAlert, key, now))
             {
                 _lastCostRegressionAlert[key] = now;
+                _lastCostRegressionDataPoint[key] = regression.LatestMetricTime;
                 var ratio = regression.BaselineMs > 0 ? regression.LatestMs / regression.BaselineMs : 0;
                 await FireAsync(
                     key, regression.ServerName, "Collector Cost Regression",
@@ -652,6 +669,7 @@ await FireAsync(
             if (!current.Contains(key) && _activeCostRegression.TryRemove(key, out var serverName))
             {
                 _lastCostRegressionAlert.TryRemove(key, out _);
+                _lastCostRegressionDataPoint.TryRemove(key, out _);
                 var parts = key.Split(':', 3);
                 var collector = parts.Length == 3 ? parts[2] : key;
                 await RecordResolutionAsync(new AlertResolution(

Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingCollectorCostReader.cs

@@ -122,31 +122,40 @@ public static async Task<List<CollectorCostDailyPoint>> GetTrendAsync(
     /// (#2674) — the self-alert's detection query. Per (server, collector): latest day's summed sql_ms vs
     /// the AVERAGE of the prior days in the window, returned only when the baseline is meaningful (>= floor,
     /// and at least 3 prior days so a new collector cannot trip it) and the latest exceeds it by the factor.
-    /// $1 = baseline window start (naive UTC), $2 = baseline floor ms, $3 = factor.</summary>
+    /// $1 = baseline window start (naive UTC), $2 = baseline floor ms, $3 = factor.
+    ///
+    /// <para><c>latest_metric_time</c> (#2707) is the newest raw hourly row folded into <c>latest_ms</c> —
+    /// the freshness anchor the self-alert needs to tell "this regression got worse" from "the hourly flush
+    /// hasn't landed a new row since I last looked", the same distinction #2704 draws with wait_stats'
+    /// collection_time. Without it, re-asking this query on a cooldown that outpaces the flush hands back the
+    /// exact same latest_ms twice, and the evaluator has no way to know the second answer isn't new.</para></summary>
     public const string RegressionSql = @"
 WITH daily AS
 (
-    SELECT cc.server_id, cc.collector_name, date_trunc('day', cc.metric_time) AS day, sum(cc.total_sql_ms) AS sql_ms
+    SELECT cc.server_id, cc.collector_name, date_trunc('day', cc.metric_time) AS day,
+           sum(cc.total_sql_ms) AS sql_ms, max(cc.metric_time) AS latest_metric_time_in_day
     FROM collect.collector_cost AS cc
     WHERE cc.metric_time >= $1
     GROUP BY cc.server_id, cc.collector_name, date_trunc('day', cc.metric_time)
 ),
 ranked AS
 (
-    SELECT server_id, collector_name, day, sql_ms,
+    SELECT server_id, collector_name, day, sql_ms, latest_metric_time_in_day,
            max(day) OVER (PARTITION BY server_id, collector_name) AS latest_day
     FROM daily
 ),
 agg AS
 (
     SELECT server_id, collector_name,
-           max(sql_ms) FILTER (WHERE day = latest_day)  AS latest_ms,
-           avg(sql_ms) FILTER (WHERE day < latest_day)  AS baseline_ms,
-           count(*)    FILTER (WHERE day < latest_day)  AS baseline_days
+           max(sql_ms)                     FILTER (WHERE day = latest_day) AS latest_ms,
+           avg(sql_ms)                     FILTER (WHERE day < latest_day) AS baseline_ms,
+           count(*)                        FILTER (WHERE day < latest_day) AS baseline_days,
+           max(latest_metric_time_in_day)  FILTER (WHERE day = latest_day) AS latest_metric_time
     FROM ranked
     GROUP BY server_id, collector_name
 )
-SELECT a.server_id, COALESCE(s.display_name, s.server_name) AS server_name, a.collector_name, a.latest_ms, a.baseline_ms
+SELECT a.server_id, COALESCE(s.display_name, s.server_name) AS server_name, a.collector_name,
+       a.latest_ms, a.baseline_ms, a.latest_metric_time
 FROM agg AS a
 JOIN collect.servers AS s ON s.server_id = a.server_id
 WHERE a.baseline_days >= 3
@@ -159,7 +168,8 @@ public sealed record CostRegression(
         string ServerName,
         string CollectorName,
         long LatestMs,
-        double BaselineMs);
+        double BaselineMs,
+        DateTime LatestMetricTime);
 
     public static async Task<List<CostRegression>> GetCostRegressionsAsync(
         NpgsqlDataSource postgres, DateTime baselineSinceUtc, long baselineFloorMs, double factor,
@@ -179,7 +189,8 @@ public static async Task<List<CostRegression>> GetCostRegressionsAsync(
                 reader.GetString(1),
                 reader.GetString(2),
                 reader.GetInt64(3),
-                reader.GetDouble(4)));
+                reader.GetDouble(4),
+                reader.GetDateTime(5)));
         }
 
         return rows;