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

[erikdarlingdata]

Fixes #2707. Sibling defect to #2703, fixed for Poison Wait in #2704 — same category of bug, different code path (DarlingSelfAlertEvaluator, not the shared AlertEngine; collect.collector_cost hourly aggregate, not wait_stats deltas).

Problem

ApplyCostRegressionsAsync gated re-firing on CooldownElapsed(_lastCostRegressionAlert, key, now) alone. If the hourly store-metrics tick's actual cadence ever outpaces the cooldown, or the flush itself lags a tick, re-asking GetCostRegressionsAsync hands back the exact same latest_ms computed from the exact same underlying collect.collector_cost rows — observed live today as byte-identical repeat "Collector Cost Regression" alerts an hour apart on upsilon-01/plan_correction (207,313 ms/day at both 09:07:15 and 10:03:58 UTC) and multi-24/ag_replica_states (16,916 ms/day, same two timestamps).

Fix

Added LatestMetricTime to CostRegression — the newest collect.collector_cost row actually folded into latest_ms, computed the same FILTER-aggregation way latest_ms/baseline_ms already are. ApplyCostRegressionsAsync now requires that anchor to have advanced past what was last alerted on (tracked in a new _lastCostRegressionDataPoint dictionary, mirroring _lastCostRegressionAlert's existing lifecycle including the resolve-cleanup), in addition to cooldown-elapsed, before re-firing. Same shape as #2704's PoisonWaitDelta.CollectionTime gate.

Verification

• dotnet build (service + tests) clean, 0 errors.
• Verified the SQL change against a real local Postgres 17, running the actual shipped RegressionSql string (spliced out of the source file, not a retyped copy): with only an early hourly row landed, latest_metric_time reports that row's timestamp; after a second hourly row lands for the same day, latest_metric_time advances to it and latest_ms grows accordingly. Confirms the freshness anchor tracks real data movement rather than being a coincidental proxy.
• Added CollectorCostRegression_DoesNotRefire_OnTheSameMetricTime_EvenAfterCooldownElapses to DarlingSelfAlertTests.cs: fires once, cooldown elapses with the SAME LatestMetricTime → no re-fire; a genuinely new LatestMetricTime (still regressed) → fires again. Couldn't run Darling.Tests locally (macOS lacks the WindowsDesktop runtime the net10.0-windows TFM needs) — hand-traced against the two-condition gate (hasFreshDataPoint && CooldownElapsed(...)), same limitation noted on Fix Poison Wait alert re-firing on an unrefreshed wait_stats row #2704 and Fix QUERY_HIGH_DOP firing on a stale lifetime max_dop #2706; CI's Darling PostgreSQL tests/build jobs are the arbiter.
• The two pre-existing cost-regression tests (FiresOnEntry_SuppressedWithinCooldown_ResolvesWhenGone, DistinctCollectors_FireIndependently) needed no behavioral changes, just an updated Regression() test-helper signature for the new required record field.

🤖 Generated with Claude Code

[claude]

Nit: the new latest_metric_time column is wider than the padding used to align the other three AS aliases, so this block no longer lines up (AS sits at a different column on line 153 than on 150-152). Not a functional issue, just breaks the visual-alignment convention this file otherwise follows for multi-line SELECT/aggregate lists.

[claude]

Reviewed. This is a clean, well-scoped fix that mirrors the #2704 (Poison Wait) pattern correctly:

• Logic: hasFreshDataPoint correctly gates re-firing on LatestMetricTime advancing past the last-alerted anchor, in addition to (not instead of) CooldownElapsed. First-time entry (no dictionary entry yet) correctly defaults to "fresh." The resolve-path cleanup of _lastCostRegressionDataPoint mirrors _lastCostRegressionAlert's existing lifecycle, so a resolved-then-recurring regression fires immediately on re-entry rather than being stuck waiting for a newer timestamp than one that was already cleared.
• SQL: latest_metric_time is computed with the same FILTER (WHERE day = latest_day) pattern already used for latest_ms/baseline_ms, so it can't be NULL under any condition those two aren't already NULL under — no new null-handling risk. Table aliases and schema-qualification (collect.collector_cost AS cc, collect.servers AS s) follow existing convention.
• Column ordinals: GetDateTime(5) lines up correctly with the new a.latest_metric_time as the 6th projected column.
• Tests: the new test exercises exactly the three states that matter (same-metric-time-within-cooldown → no refire, cooldown-elapsed-but-stale → no refire, genuinely-newer-metric-time → refire), and the two pre-existing cost-regression tests still hold under the new required constructor arg.
• Parity: no Lite counterpart exists for this self-alert — "Collector Cost Regression" is Darling-specific telemetry about its own Postgres-store collection cost, which Lite (no persistent store) has no equivalent for. The shared PoisonWaitDelta/AlertEngine fix from Fix Poison Wait alert re-firing on an unrefreshed wait_stats row #2704 already covers Lite via the common PerformanceMonitor.Alerting library, so this is not parity drift — it's a separate code path with no shared counterpart, as the PR description states.

Left one minor inline nit (broken AS-column alignment introduced by the wider new column name in the agg CTE — cosmetic only). No correctness, security, or performance issues found.

[claude]

Reviewed. This is a clean, well-scoped fix — no blocking issues found.

Correctness

• The hasFreshDataPoint gate mirrors Fix Poison Wait alert re-firing on an unrefreshed wait_stats row #2704's PoisonWaitDelta.CollectionTime pattern exactly (ConcurrentDictionary<string, DateTime>, TryGetValue + strict > comparison, same resolve-cleanup lifecycle via TryRemove). First-fire-on-entry, re-fire-only-on-new-data-point-plus-cooldown, and resolve-then-refire-immediately semantics all check out.
• RegressionSql: max(latest_metric_time_in_day) FILTER (WHERE day = latest_day) correctly rides the same ranked/agg CTE shape as latest_ms/baseline_ms, so the freshness anchor is guaranteed non-null whenever a row survives the baseline_days >= 3 filter — no null-handling gap on the GetDateTime(5) read.
• Column ordinals in GetCostRegressionsAsync (0..5) line up with the SELECT list and the CostRegression record's positional order. Test helper's updated Regression(...) signature matches too.
• New test (CollectorCostRegression_DoesNotRefire_OnTheSameMetricTime_EvenAfterCooldownElapses) exercises exactly the scenario in the bug report: cooldown elapses (10 min vs. the harness's 5 min default) with an unchanged LatestMetricTime → correctly asserts no re-fire, then a genuinely advanced LatestMetricTime → fires. This test would have failed before the fix and passes after, which is the right shape of regression test.

Lite/Darling parity

• Confirmed collect.collector_cost / CostRegression / DarlingSelfAlertEvaluator are Darling-only (Lite's only reference is the cross-app MCP tool-inventory pin test, which just checks tool naming, not behavior). No parity gap — this bug's home, the self-alert loop that isn't part of the shared AlertEngine, doesn't exist on the Lite side, matching the PR description.

Security / performance

• Fully parameterized query, no string concatenation into SQL. The added max(...) aggregate rides the existing GROUP BY, no new scan or meaningfully different cost.

No further changes requested.