Add between/outside range predicate operators for custom alert rules (#3351)

[erikdarlingdata]

What

Implements the between / outside range predicate operators for custom alert rules (#3351) — the range comparisons deferred from #3285.

A rule's predicate can now test a metric against a two-sided band [lowerBound, upperBound] in addition to the existing scalar bars (gt/ge/lt/le):

• outside breaches when value < lowerBound OR value > upperBound
• between breaches when lowerBound <= value <= upperBound (inclusive bounds)

Design

• The band lives in new predicate fields lowerBound / upperBound (nullable), never by overloading warnThreshold / criticalThreshold — overloading would break the "crossed the warning threshold" notification wording and the two-tier severity model.
• Mutually exclusive shapes, enforced in the validator (CustomAlertRuleDefinition.TryParse): a scalar op requires warnThreshold (+ an optional more-extreme criticalThreshold) and rejects bounds; a range op requires both numeric bounds with lowerBound < upperBound and rejects warn/critical.
• Warning-only in v1 for range ops: a two-tier band would need a warn-band and a crit-band (four bounds). SeverityFor returns Warning for a breaching range op; a code comment marks the escalation follow-up.
• Compare / SeverityFor / IsBreaching remain the single breach authority. The delivered-alert body renders the band as outside 10 - 100 (ASCII, with a null numeric threshold twin) via a new FiredThreshold helper the evaluator calls.

Predicate JSON (range rule)

{
  "metric": { "source": "wait_stats", "measure": "wait_time_ms", "aggregate": "sum", "hours": 1 },
  "predicate": { "op": "outside", "lowerBound": 10, "upperBound": 100 }
}

Frontend

• Editor: outside / between added to the operator dropdown; the Warning/Critical inputs (and the help text) swap for Lower/Upper-bound inputs when a range op is chosen, mapped to lowerBound/upperBound in modelToDefinition/definitionToModel. The live /api/alerts/test preview evaluates the serialized definition, so it works unchanged.
• Rule card chip renders range rules as outside 10–100 / between 10–100.

Tests

Darling.Tests.exe for CustomAlertRuleDefinitionTests + CustomAlertEvaluateNowTests + CustomAlertSeverityEscalationTests + CustomAlertTemplatesTests → 65 passing, plus DarlingMcpCustomAlertToolsSurfaceTests → 18 passing. Added: parser accepts between/outside; bounds required + numeric; lower < upper; scalar-vs-range mutual exclusion (both directions); breach logic at/inside/outside the band for both ops; SeverityFor = Warning; FiredThreshold round-trip (band text + null numeric twin, and the scalar bar); and ClassifyTestValue would-fire for range ops.

Service + tests build 0 Warning(s) / 0 Error(s).

Deferred

• Range-op Critical tier (escalation over a warn-band + crit-band, four bounds) — Warning-only in v1, code comment left in SeverityFor. Recommend a tracked issue.
• Count-with-< trap for range ops — the scalar </<=-on-a-count guard (zero events vs stalled collector) is not mirrored to range ops; whether a given band is ambiguous depends on the specific bounds, so it needs a design decision rather than a mechanical mirror.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WejwpgWF5Xfm3fAmoEbFz4

[claude]

Minor cosmetic ambiguity: bounds are rendered with an en dash and no surrounding spaces, so a negative bound reads badly — e.g. lowerBound: -20, upperBound: -5 renders as outside -20–-5, which is easy to misread as a single number or a different range. CustomAlertRuleDefinition.FiredThreshold deliberately uses a spaced " - " for the same band text in the delivered alert body; this chip could do the same (or add spaces around the en dash) for consistency and readability with negative bounds. Not a functional bug since this is a text: node (safe from injection either way), just a display nit worth a follow-up.

[claude]

Review

Reviewed the between/outside range-predicate addition (backend parser/evaluator, MCP tool description, and the alert editor + rule-card UI). Overall this is careful, well-scoped work:

• Mutual exclusivity between scalar (warnThreshold/criticalThreshold) and range (lowerBound/upperBound) shapes is enforced in both directions in CustomAlertRuleDefinition.TryParse, with tests for both directions.
• Single authority preserved: IsBreaching/SeverityFor/FiredThreshold remain the one place breach/severity/rendering logic lives; CustomAlertEvaluator.BuildFireOutcome and the evaluate-now tool both route through them, so the sweep and the "would fire" preview can't diverge.
• Constructor site for the record was the only place adding the two new fields (LowerBound/UpperBound) touched — no other code constructs CustomAlertRuleDefinition directly, and nothing else pattern-matches exhaustively on CustomAlertOp, so the new enum members don't leave a dangling switch elsewhere.
• Lite/Darling parity: custom alert rules with a predicate/op DSL are Darling-only — Lite has no equivalent rule engine (only the shared render builders that already take opaque threshold-text strings), so there's no parity drift to flag here.
• Frontend: modelToDefinition/definitionToModel map lowerBound/upperBound correctly, the save-blocker and readyToCheck mirror the backend's required-bounds/lower < upper checks, and the editor only redraws the threshold inputs when crossing the scalar↔range boundary (preserving in-progress values otherwise).
• Left one inline nit on alert-rules.js: the rule-card chip renders a range band with a bare en dash and no spaces (outside -20–-5), which is ambiguous for negative bounds — the delivered-alert-body renderer (FiredThreshold) already uses a spaced " - " for the same reason; worth the same treatment for consistency.

No correctness, security, or performance issues found beyond that cosmetic one.