Repository navigation
feat(alerts): alert when a rule's window has no data - #1208
Confidence 3/5 · 1 issue to address
🟡 Confidence 3/5 · needs attention
The scheduler/preview logic and grouped rejection check out and are well tested; the one unmitigated risk is the new alert value on the v2 wire, which older generated clients cannot decode.
quality 90/100 · 1 warning · tests covered · risk high · 1/1 new units observable
Adds an opt-in alert-on-no-data mode that treats an empty window as a breach, wired through the scheduler, preview, v2 API, MCP tools and the rule form, with grouped rules rejected. The evaluation logic and tests hold up; the new alert value on the v2 wire breaks older generated clients.
- Rules gain
alertOnNoData, compiled tonoDataBehavior: "alert"(AlertRuleModel.ts:263) - An empty window breaches with a null value (
alerting-core/src/index.ts:99) - Grouped plans reject the flag; changing it resolves open incidents
- v2, MCP, alchemy and the app form expose
alert_on_no_data
Findings
🟠 Warning · F1 · no_data_behavior: "alert" on the v2 wire breaks older generated clients
correctness · packages/domain/src/query-engine.ts:711
no_data_behavior is serialized from this literal, so the first rule saved with alert_on_no_data returns "alert" — a value the checked-in spec has no enum case for in clients generated before this PR. HomeModel.swift:201 and IncidentsListView.swift:31 decode the rule page with try? await rules.items, so one such rule fails the whole page and every incident card loses its rule name until the app is rebuilt from the new spec. Keep the wire value in {skip, zero} for this field (map alert to skip in toV2Rule at apps/api/src/routes/v2/alert-rules.http.ts:89) and let alert_on_no_data carry the new state, or expose the new mode behind a versioned field.
Decouple the wire contract from the stored mode: emit `skip`/`zero` in `no_data_behavior` and report alert-on-no-data through `alert_on_no_data` only, so shipped clients keep decoding rules.
What was checked
- Grouped rejection goes through
planGroupingTokens, so a builder-query rule grouped by its draft is still rejected (AlertRuleModel.ts:798) - The breach that comes from an empty window never enters the hold path:
planAlertLifecyclegates onstatus === "healthy"(alerting-core/src/index.ts:436) - The preview's synthetic empty-result series uses
toStorageGroupKey(plan, "all"), the same storage key the scheduler opens under (AlertRuleModel.ts:193)
Observability coverage: 1 of 1 changes observable
| Change | Kind | Observable | Evidence |
|---|---|---|---|
no-data breach in runSchedulerTick |
background | yes | runs inside the existing scheduler span and incident/delivery metrics; no new call, log or entrypoint added |
cd80cc0 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
Annotations
Check warning on line 711 in packages/domain/src/query-engine.ts
maple-review-bot / Maple / review
correctness: `no_data_behavior: "alert"` on the v2 wire breaks older generated clients
`no_data_behavior` is serialized from this literal, so the first rule saved with `alert_on_no_data` returns `"alert"` — a value the checked-in spec has no enum case for in clients generated before this PR. `HomeModel.swift:201` and `IncidentsListView.swift:31` decode the rule page with `try? await rules.items`, so one such rule fails the whole page and every incident card loses its rule name until the app is rebuilt from the new spec. Keep the wire value in `{skip, zero}` for this field (map `alert` to `skip` in `toV2Rule` at `apps/api/src/routes/v2/alert-rules.http.ts:89`) and let `alert_on_no_data` carry the new state, or expose the new mode behind a versioned field.