Skip to content

feat(alerts): incident-first alerts hub, timeline incident page, rebuilt rule editor - #1502

Merged
Makisuo merged 8 commits into
mainfrom
feat/alerting-rethink
Oct 15, 2026
Merged

feat(alerts): incident-first alerts hub, timeline incident page, rebuilt rule editor#1502
Makisuo merged 8 commits into
mainfrom
feat/alerting-rethink

fix(alerts): review fixes for mode switching, rule summary, quiet count

8dc0980
Select commit
Loading
Failed to load commit list.
Maple Review Bot / Maple / review completed Oct 15, 2026 in 51s

Confidence 6/10 · 1 issue to address

🟡 Confidence 6/10 · needs attention
The three earlier findings are verifiably fixed and tested; the incident route's missing failure branch is the one unguarded path, and its other hunks are already reviewed.
quality 88/100 · 1 warning · 1 note · tests partial · risk medium

The follow-up commit fixes the mode-switch, rule-summary and quiet-count findings, and keeps the revised incident page. One path in the incident route still reports a failed fetch as a missing incident.

  • Switching a builder rule to Built-in now reports the split-by tokens it drops
  • ruleConditionSummary prints builder thresholds in the display unit
  • The quiet-rule footer counts enabled rules with no lane, capped lanes included
  • The incident route starts an investigation only on click, through useAsyncAction

Findings

🟠 Warning · F5 · A failed incident-list fetch renders "Incident not found"

correctness · apps/web/src/routes/alerts/incidents/$incidentId.tsx:77-91

When useAlertIncidentsList returns a failure (a transient API or auth error), .orElse(() => []) leaves incidents empty and incident null while Result.isInitial is false, so the page claims the incident "may have been pruned, or the link is stale" instead of showing the failure. The delivery query in the same component does branch on Result.isFailure ($incidentId.tsx:253); the incidents and rules results get no such branch, so a fetch error is presented as a missing record with no retry.

Add a `Result.isFailure(incidentsResult)` branch before this one that renders `ErrorState` (or `ResourceNotFound` with a retry) so an API failure is not reported as a deleted incident.

Still open from earlier reviews

Fixed since the last review

  • ✅ F1 · Switching a builder rule to Built-in silently drops its split-by tokens
  • ✅ F2 · ruleConditionSummary prints builder thresholds in the stored unit, not the display unit
  • ✅ F3 · The quiet-rule footer counts fired and capped lanes as quiet
What was checked
  • queryDraftToBuiltin now names a lost split in dropped (rule-modes.ts:115) and the test pins it
  • The quiet footer counts every lane's rule, enabled rules only (inbox-tab.tsx:254)
  • ruleConditionSummary formats through displaySignalType and the builder-aggregation test asserts 5.0%

8dc0980 · 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 91 in apps/web/src/routes/alerts/incidents/$incidentId.tsx

See this annotation in the file changed.

@maple-review-bot maple-review-bot / Maple / review

correctness: A failed incident-list fetch renders "Incident not found" 

When `useAlertIncidentsList` returns a failure (a transient API or auth error), `.orElse(() => [])` leaves `incidents` empty and `incident` null while `Result.isInitial` is false, so the page claims the incident "may have been pruned, or the link is stale" instead of showing the failure. The delivery query in the same component does branch on `Result.isFailure` (`$incidentId.tsx:253`); the incidents and rules results get no such branch, so a fetch error is presented as a missing record with no retry.
View more details on Maple Review Bot