Guard COUNT range-op alerts against a stalled-collector no-data 0 (#3373)

[erikdarlingdata]

What

Extends the custom-alert no-data (stalled-collector) guard from the scalar lt/le COUNT case to the between/outside range ops (#3371), completing #3373 (part of #3285).

A COUNT metric compiles to COUNT(*), which reads 0 over an empty window -- never the NULL the evaluator's no-data freeze catches -- so a stalled collector is indistinguishable from "zero events happened". A range band that treats that 0 as a breach therefore false-fires on a dead collector, exactly the ambiguity the scalar path already rejects.

The rule

For a COUNT metric + range op, reject at parse time when the band fires on 0:

band	fires on 0?	verdict
outside [1, N]	yes (0 < 1)	rejected
between [0, N]	yes (0 in band)	rejected
outside [0, N]	no (0 is the inclusive edge)	allowed
between [1, N]	no (0 < 1)	allowed

Non-COUNT metrics are unaffected: SUM/AVG return NULL on an empty window, which the no-data freeze already handles.

How it reuses the scalar guard (no parallel path)

• The parse-time rejection lives in CustomAlertRuleDefinition.TryParse, right beside the scalar lt/le count trap, keyed on the same plan.Aggregate == ComposeAggregate.Count archetype check.
• The band-membership math is factored into one private RangeBreaches; IsBreaching, the new public BreachesOnZero() predicate, and the guard all route through it -- IsBreaching stays the single evaluation authority (BreachesOnZero() == IsBreaching(0) for a range op).
• The rejection is mirrored in the two other places the scalar trap lives: the alert-editor.js save-blocker (which previously said range ops had "no count trap") and the validate_custom_alert_rule MCP tool description.

Tests

CustomAlertRuleDefinitionTests: the four band/op combinations on a COUNT (two rejected, two allowed), a non-COUNT (SUM) band that fires on 0 staying allowed, BreachesOnZero == IsBreaching(0) across range ops and false for a scalar op, plus the previously-missing scalar COUNT lt/le rejection test alongside them.

Verification

• dotnet build Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj -c Debug -> 0 Warning(s) / 0 Error(s); test project builds clean too.
• CustomAlertRuleDefinitionTests 55/55 pass; the related non-live evaluate-now / severity-escalation / templates / rule-health classes 36/36 pass (the IsBreaching refactor is behavior-preserving).

Scope

No change to the range predicate JSON shape, the scalar path, or the severity model (no Critical tier -- that's #3372). CHANGELOG untouched (team lead folds it post-wave).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y5xoN5PzhzYfHHrfutyeEe

[claude]

Minor: BreachesOnZero() isn't actually called anywhere in production code — the parse-time guard at line 305 calls the private static RangeBreaches(op, lower, upper, 0) directly (it has to, since there's no CustomAlertRuleDefinition instance yet at parse time), not this method. The only callers are the unit tests.

The doc comment above (lines 127-135) says "This is the predicate the parse-time count guard uses to reject such a rule" — that's not quite accurate; the guard uses the same underlying logic via RangeBreaches, but not this method itself.

Since nothing outside the test suite consumes it, consider either wiring it into an actual call site (e.g. a rule-health/UI affordance that flags "this band would fire on a stalled-collector zero") or dropping the public method and testing RangeBreaches's zero-case behavior through IsBreaching directly, to avoid shipping unused public API surface.

[claude]

Review summary

Reviewed the range-op extension of the stalled-collector count guard (#3373). This is a Darling-only change (custom alerting has no Lite counterpart), so there's no Lite/Darling parity concern here, and no T-SQL is touched.

Correctness — Verified the core logic by hand-tracing all four band/op combinations:

• RangeBreaches in CustomAlertRuleDefinition.cs is a faithful, unchanged-semantics factoring of the prior inline switch (Between: value >= lo && value <= hi; Outside: value < lo || value > hi).
• The parse-time guard (plan.Aggregate == ComposeAggregate.Count && RangeBreaches(op, lower, upper, 0)) correctly rejects exactly the bands that fire on a stalled-collector 0 (outside [1,100], between [0,100]) and allows the bands that don't (outside [0,100], between [1,100]) — matches the table in the PR description.
• Confirmed ComposeAggregate.Count compiles to CAST(COUNT(*) AS double precision) (a plain per-event row count) and serializes to the wire string "count", so the JS mirror's metric.aggregate === "count" check in alert-editor.js lines up correctly with the backend guard, and the boolean logic there (0 < lower || 0 > upper for outside, lower <= 0 && 0 <= upper for between) is algebraically identical to the C# RangeBreaches.
• IsBreaching's refactor to route both scalar and range through one expression preserves behavior (scalar still uses Compare, range now uses RangeBreaches with identical conditions).

Tests — the new theory cases cover the four band/op combinations, the non-COUNT (SUM) exemption, and BreachesOnZero() == IsBreaching(0) equivalence. Logic checks out; wasn't able to execute dotnet build/dotnet test in this sandboxed review environment (command approval unavailable), so the "0 Warnings/55/55 pass" claim in the PR description is unverified but the code reads correctly.

One minor nit — left inline on CustomAlertRuleDefinition.cs: the new public BreachesOnZero() method isn't actually called by any production code path (the parse-time guard calls the private RangeBreaches directly, since no instance exists yet at parse time); only the test suite exercises it, and its doc comment slightly overstates that the guard "uses" it.

No security, injection, or missing-index concerns — this PR only touches C# validation/JS mirror logic, no SQL.