[HDX-4997] Record alert evaluation errors in AlertHistory and add alert detail page

[wrn14897]

Summary

⚠️ Rollout status: not enabled in production. The alert detail page is gated behind NEXT_PUBLIC_ENABLE_ALERT_DETAILS (default off) — currently set only in dev (.env.development) and CI (the e2e webserver). With the flag off, the alerts page renders no Details links and /alerts/:id redirects back to /alerts. Self-hosted deployments can opt in via the commented docker-compose.yml entry. Additionally, managed-cloud production runs the EE/CHC alert provider, which doesn't persist evaluation errors/analytics yet (planned EE follow-up), so evaluation-error history wouldn't populate there regardless.

When an alert evaluation fails (ClickHouse query error/timeout, webhook failure), the only persisted signal was alert.executionErrors — a latest-only snapshot wiped by the next successful run. Users couldn't see when evaluations failed, how often, or why. This is a prerequisite for ever tightening the alert query evaluation timeout (HDX-4977): before that value can change, users need to be able to see which alerts hit query errors/timeouts and when.

This PR makes alert failures first-class, Datadog-monitor-style, and gives every alert a status page. Three logical commits:

1. Evaluations read model (feat(alerts): evaluations read model)

• Types for evaluation errors (AlertError/AlertErrorType incl. QUERY_TIMEOUT), per-window evaluations with per-group breakdown, and evaluation analytics; AlertHistory gains optional errors/analytics fields and AlertState.ERROR (only ever used on history rows).
• GET /alerts/:id/evaluations: per-window evaluation history scoped to a startTime/endTime range with layered hard caps — span clamped to the 31d retention window, limit fixed at 200, each request scans a hard-bounded slice of at most ~(limit+1) × interval of history (group-by alerts can have many rows per window and the $group stage processes every matched row, so the scan is bounded, not just the returned page), and cursor-based paging via a server-provided nextBefore that always advances past the scanned slice so pagination keeps progressing across gaps instead of stalling.
• Windows containing ERROR rows surface deduped error details and rank as ERROR; firing-transition chart annotations exclude ERROR rows.

2. Alert detail page (feat(app): alert detail page) — gated behind NEXT_PUBLIC_ENABLE_ALERT_DETAILS

New /alerts/:id page reachable via a Details link on each alerts-page row (the alert name keeps linking to the saved search / dashboard tile):

• Header with state badge, silence/ack, source link, and a time picker; the alert's underlying query charted over the selected range with threshold reference lines and firing/recovery annotations. Chart, timeline strip (60 windows), and event stream all follow the picker's exact range.
• Evaluation event stream: one parent row per window labeled with the evaluated bucket start (matching the chart's x-axis, full evaluated span in a tooltip), state, latest value, breaches, backfilled buckets, query/webhook durations, and error labels; per-group child rows for group-by alerts (k/n groups firing, firing-first, capped server-side at 50 with an explicit cap message); older windows load via an infinite-scroll sentinel. A failed page fetch unmounts the sentinel (whose effect would otherwise refire forever against a failing endpoint) and renders an explicit retry affordance.
• Alerts-page history strip renders errored windows as striped-red segments with full error details in a modal on click (ungated — it simply never populates until the write side runs).

3. Error + analytics persistence (feat(alerts): persist evaluation errors and analytics)

• Failed evaluations are recorded as ERROR-state AlertHistory rows carrying error type/message/timestamp, upserted per evaluation window so per-tick retries collapse into a single row (a permanently failing 1d alert produces 1 row/day, not 1/minute; rows expire with the existing 30d TTL). Webhook failures also produce an ERROR row alongside the normal rows; a stale ERROR row from a failed earlier tick is removed when a clean same-window retry succeeds.
• Retry/backfill semantics are untouched: ERROR rows are excluded from the due-ness gate, the retry date-range computation, and consecutive-window counting — recording an error never marks the window as evaluated, so the failed window is still retried every tick and backfilled on recovery (covered by integration tests).
• Query timeouts are classified as QUERY_TIMEOUT (client request timeout/abort, server-side TIMEOUT_EXCEEDED/159, socket timeouts — walking the error cause chain, since BaseClickhouseClient.query wraps failures and keeps the identifying metadata only on cause) with an actionable message that includes the configured evaluation timeout.
• Evaluation analytics (queryDurationMs — time-to-failure on query-error rows, webhookDurationMs — total delivery wall time incl. retries, backfilledBuckets — missed ticks caught up in this run) are recorded on every history row the evaluation writes.

Screenshots or video

Before	After
Failed evaluations left no trace on the alerts page except a latest-only ! icon	Errored windows render as striped-red segments in the history strip; clicking opens the error details; each alert has a Details page with the full evaluation event stream (flag-gated)

How to test on Vercel preview

N/A — alert evaluation history requires the alerting job + MongoDB-backed alerts, which don't exist in LOCAL_MODE previews (and the detail page is flag-gated off there). Covered by API integration tests and full-stack Playwright tests instead.

How this was tested

• tsc --noEmit (app + api); app unit tests (evaluations table incl. retry affordance, history cards, error classification — 18 classifier tests incl. wrapped/nested/self-referential cause chains)
• Integration: all 8 alert-related suites, 426/426 pass — ERROR row content, same-window upsert dedupe + stale-row cleanup, cross-window ERROR rows preserved, QUERY_TIMEOUT classification, failed-window-still-retried/backfilled, webhook-failure ERROR rows (incl. grouped alerts), evaluations endpoint (range clamping, bounded scan, nextBefore cursor, per-group cap), analytics fields
• E2E (make dev-e2e FILE=alerts): the 3 new specs (errored strip segment, error details modal, detail-page navigation + evaluation history) pass with the flag threaded through the e2e webserver; remaining Alert Creation/Lifecycle/Notes failures reproduce identically on pristine origin/main (21 suite-wide failures there) and are unrelated
• eslint exactly at the 663-warning budget (zero net new warnings); knip clean (stryker findings pre-existing on main)

References

• Linear Issue: HDX-4997
• Related: HDX-4977 (alert query timeout rollout — this PR is its visibility prerequisite)
• [HDX-4997] Persist alert evaluation errors and analytics in AlertHistory #2797 (the write-side split) was fast-forwarded into this branch and shows as merged; [HDX-4997] Add alert detail page with evaluation history #2798 was folded in earlier.

Note for EE: AlertProvider.recordAlertErrors gained optional evaluationWindowStart and analytics params (backward compatible — the CHC provider keeps compiling and can adopt ERROR-row persistence in a follow-up).

Screenshot

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview	Aug 6, 2026 11:02pm
hyperdx-storybook	Ready	Preview	Aug 6, 2026 11:02pm

[changeset-bot]

🦋 Changeset detected

Latest commit: bd81e59

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/common-utils	Minor
@hyperdx/api	Minor
@hyperdx/app	Minor
@hyperdx/otel-collector	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

• Critical-path files (1) — tenancy, public API, or shipped database config:
  • packages/api/src/routers/external-api/v2/alerts.ts
• Background tasks or delivery pipeline substantially modified — 297 lines (bar: 30):
  • packages/api/src/tasks/checkAlerts/errors.ts
  • packages/api/src/tasks/checkAlerts/index.ts
  • packages/api/src/tasks/checkAlerts/providers/default.ts
  • packages/api/src/tasks/checkAlerts/providers/index.ts
• Cross-layer change: touches frontend (packages/app) + backend (packages/api) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats

• Production files changed: 26
• Production lines changed: 2212 (+ 1921 in test files, excluded from tier calculation)
• Critical-path lines changed: 299
• Branch: warren/HDX-4997-alert-error-history
• Author: wrn14897

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[greptile-apps]

Greptile Summary

This PR persists alert evaluation failures and introduces an alert-detail experience.

• Adds ERROR-state history records with error classification and evaluation analytics.
• Adds bounded, cursor-paginated evaluation-history APIs with grouped-alert breakdowns.
• Adds alert chart, timeline, event-stream, retry, and error-detail UI.

Confidence Score: 4/5

The PR is not yet safe to merge because valid same-minute ranges can still suppress the evaluation-history request while the chart renders that range.

The evaluations hook floors both picker bounds to minute boundaries and then requires the normalized start to precede the normalized end, so an exact range contained within one minute remains disabled rather than loading its evaluation history.

Files Needing Attention: packages/app/src/api.ts

Important Files Changed

Filename	Overview
packages/api/src/controllers/alertHistory.ts	Adds grouped evaluation reads, bounded cursor pagination, error aggregation, analytics resolution, and ERROR-aware transition handling.
packages/api/src/routers/api/alerts.ts	Exposes the team-scoped evaluations endpoint with validated and retention-clamped range parameters.
packages/api/src/tasks/checkAlerts/index.ts	Records evaluation failures and analytics while preserving retry and backfill behavior.
packages/api/src/tasks/checkAlerts/providers/default.ts	Persists and clears per-window ERROR history records alongside normal alert-state updates.
packages/app/src/AlertDetailPage.tsx	Composes the alert detail chart, exact-range controls, history strip, and paginated evaluation stream.
packages/app/src/api.ts	Adds alert evaluation fetching and pagination, but still collapses valid same-minute ranges before deciding whether to issue the request.
packages/app/src/components/alerts/AlertEvaluationsTable.tsx	Renders evaluation windows, grouped rows, analytics, failures, infinite loading, and explicit retry states.

Sequence Diagram

sequenceDiagram
  participant Job as Alert evaluation job
  participant CH as ClickHouse
  participant Mongo as AlertHistory
  participant API as Evaluations API
  participant UI as Alert detail page
  Job->>CH: Evaluate alert query
  alt Evaluation succeeds
    Job->>Mongo: Persist state rows and analytics
  else Query or notification fails
    Job->>Mongo: Upsert ERROR row and error details
  end
  UI->>API: Request bounded time-range page
  API->>Mongo: Group history by evaluation window
  Mongo-->>API: States, groups, errors, analytics
  API-->>UI: Page plus nextBefore cursor

Loading

_{Reviews (23): Last reviewed commit: "feat(alerts): persist evaluation errors ..." | Re-trigger Greptile}

[github-actions]

E2E Test Results

✅ All tests passed • 276 passed • 1 skipped • 1114s

Status	Count
✅ Passed	276
❌ Failed	0
⚠️ Flaky	1
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

<!-- deep-review -->

Deep Review

🔴 P0/P1 — must fix

• packages/api/src/tasks/checkAlerts/index.ts:1553 — The new state: { $ne: AlertState.ERROR } filters in getPreviousAlertHistories and getConsecutiveWindowHistories are the only thing keeping ERROR rows from counting as evaluated windows, so a code rollback leaves those rows in place while removing the filters: shouldSkipAlertCheck then matches on createdAt alone and getAlertEvaluationDateRange advances previousCreatedAt past the failed window, which is never re-queried, and any ERROR row inside the numConsecutiveWindows lookback breaks the every(state === ALERT || PENDING) check so multi-window alerts silently stop firing.
  • Fix: Gate the ERROR-row writes behind a flag that can be disabled independently of the read-side filters, or state in the deploy runbook that rollback is unsafe once ERROR rows exist and require draining them first.
• packages/api/src/tasks/checkAlerts/errors.ts:67 — isQueryTimeoutError inspects e.type/e.code on the thrown value directly, but BaseClickhouseClient.query() re-throws every failure as new ClickHouseQueryError(message, debugSql) with the original attached as cause, so isClickHouseError fails both its instanceof and constructor-name checks and the server-side TIMEOUT_EXCEEDED/159 and ETIMEDOUT branches never match in production — the timeout is persisted as QUERY_ERROR and the counter is labelled error_type: 'error'.
  • Fix: Walk the cause chain in isQueryTimeoutError and isClickHouseError, and add a case to errors.test.ts built from the wrapped shape BaseClickhouseClient.query() actually throws rather than a bare ClickHouseError.
• packages/api/src/tasks/checkAlerts/index.ts:1038 — The recordAlertErrors call in the query-failure catch is unguarded, and DefaultAlertProvider.recordAlertErrors commits Alert.updateOne before awaiting upsertErrorHistory, so a throw from the second write escapes into processAlert's outer catch, which reclassifies the failure as AlertErrorType.UNKNOWN and overwrites the just-persisted QUERY_TIMEOUT/QUERY_ERROR with a hardcoded generic message; the same unguarded upsertErrorHistory inside updateAlertState additionally flips evalOutcome to error for an evaluation that succeeded.
  • Fix: Wrap the upsertErrorHistory calls in providers/default.ts in their own try/catch that logs and returns, so a history-write failure cannot reach the outer catch and clobber the authoritative executionErrors.
  • _{reliability, adversarial}
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:139 — The sentinel effect's only guard is !isFetchingNextPage, and hasNextPage is derived from the last successful page, so a failing /alerts/:id/evaluations request re-fires onLoadMore() on every settle with no error surfaced; the same unbounded loop runs on the success path because the server returns hasMore: true with empty data when a scan slice has no rows, so a 1m-interval alert over a 30-day picker range walks the range in roughly 215 back-to-back aggregation requests from a single page view.
  • Fix: Thread the infinite query's error state into LoadMoreSentinel and skip onLoadMore() once a page fetch has failed, and cap auto-pagination at N pages behind an explicit "load older" click.
  • _{julik-frontend-races, reliability, correctness, adversarial, api-contract}

🟡 P2 — recommended

• packages/api/src/controllers/alertHistory.ts:301 — getAlertTransitionsInRange has no $limit or scan-count bound, unlike the sibling evaluations pipeline this PR explicitly bounds, and the new detail chart calls it unconditionally with a user-selectable range up to MAX_HISTORY_SPAN_MS, so a 1m-interval group-by alert over 30 days materializes roughly 43k windows times per-window group count into one $group.
  • Fix: Apply the same (limit + 1) * intervalMs scan bound used by getAlertEvaluations, or add a document-count cap before the $group stage.
• packages/api/src/routers/api/alerts.ts:107 — GET /alerts fetches 20 history windows per alert and formatAlertResponse passes errors through verbatim, while QUERY_ERROR/QUERY_TIMEOUT are absent from the hardcoded-message map so raw ClickHouse text is persisted at up to 10,000 chars per entry — one unreachable connection failing every alert in a team turns the alerts landing page into a multi-megabyte response.
  • Fix: Truncate persisted messages far more aggressively in makeAlertError, and return only errors[].type plus a short preview from the list endpoint, keeping full text behind GET /alerts/:id/evaluations.
• packages/api/src/tasks/checkAlerts/providers/default.ts:461 — upsertErrorHistory uses $set: { errors }, so a later tick in the same evaluation window replaces rather than merges the stored array: a 1d alert that times out at 00:01 and then fails its webhook at 00:02 keeps only WEBHOOK_ERROR and loses the timeout permanently, even though the read path's dedupeErrors exists to merge distinct errors per window.
  • Fix: Accumulate with $addToSet/$push capped by $slice and let dedupeErrors collapse duplicates on read, or document that only the latest tick's errors are retained per window.
• packages/api/src/tasks/checkAlerts/providers/default.ts:454 — The upsert filter { alert, createdAt, state: ERROR } is not backed by a unique index (models/alertHistory.ts declares only the createdAt TTL plus two non-unique compound indexes), so updateOne(..., { upsert: true }) is not atomic against a concurrent insert and two overlapping evaluations of the same window can each insert a row, contradicting the function's own one-row-per-window docstring.
  • Fix: Add a partial unique index on { alert: 1, createdAt: 1, state: 1 } filtered to state: 'ERROR' and retry an E11000 from the insert race as an update.
  • _{data-migrations, reliability, adversarial}
• packages/app/src/components/alerts/AlertHistoryCards.tsx:128 — groupStateToOverallState ranks ALERT above ERROR, so a window that fired and whose webhook failed is grouped as ALERT with errors populated, but the click target and error modal are gated on history.state === AlertState.ERROR — the "fired but never notified" case renders as an ordinary solid segment with no way to reach the stored error.
  • Fix: Gate the clickable error affordance on (history.errors?.length ?? 0) > 0 while leaving the striped styling driven by state.
  • _{correctness, adversarial}
• packages/app/src/AlertDetailPage.tsx:129 — AlertDetailBody never destructures isError from api.useAlertEvaluations, so a failed first page leaves isLoading false and data undefined, and AlertEvaluationsTable's evaluations.length === 0 && !hasNextPage branch renders "No evaluations in the selected time range" — a backend failure is presented as healthy-but-empty on the page whose purpose is diagnosing failures.
  • Fix: Pass isError/error through to AlertEvaluationsTable and render a distinct error state with a retry affordance.
  • _{reliability, julik-frontend-races}
• packages/api/src/controllers/alertHistory.ts:92 — fetchGroupedWindows bounds the $match by interval count but places $group before $limit, so document volume scales with group-by cardinality: a 200-window page on a 1m-interval alert with hundreds of groups pulls every matched row into $group on each page, and the code's own comment acknowledges the cost without mitigating it.
  • Fix: Add a document-count cap inside the pipeline or an early exit once limit distinct createdAt values have been seen, independent of the time bound.
• packages/api/src/tasks/checkAlerts/providers/default.ts:411 — executionErrors accumulates one entry per failing group and is written unbounded via $set, mirrored onto the ERROR history row; only per-message text is truncated, so a high-cardinality group-by alert with a persistently failing webhook can grow the document toward the BSON limit and fail an Alert.updateOne that also carries state: finalState.
  • Fix: Dedupe by type and message and cap the array to a fixed entry count before persisting to either Alert.executionErrors or the ERROR row.
• packages/api/src/models/alert.ts:149 — state is declared enum: AlertState, which now includes ERROR, so the "AlertHistory rows only" invariant rests entirely on a comment and current call-site discipline; an ERROR value on Alert.state would be schema-valid and would match none of AlertsPage.tsx's ALERT/PENDING/OK buckets, silently dropping the alert from the list, and is absent from the v2 OpenAPI AlertState enum.
  • Fix: Split the enum so Alert.state uses a variant without ERROR and only AlertHistory.state/AlertTransition.state accept the widened set.
  • _{maintainability, data-migrations}
• packages/api/src/controllers/alertHistory.ts:47 — dedupeErrors here and dedupeAlertErrors in AlertHistoryCards.tsx implement the same ${type}||${message} key and same keep-newest rule twice and have already diverged, with only the backend copy sorting newest-first, so error ordering differs between the alerts-list and evaluations paths.
  • Fix: Move one implementation into packages/common-utils and import it from both the controller and the component.
• packages/app/src/components/alerts/AlertDetailChart.tsx:135 — TileAlertChart's config useMemo rebuilds the dashboard tile's ChartConfig assembly field-for-field from DBDashboardPage.tsx, so any change to the real tile's config (a new source field, a change to getMetricTableName) leaves this preview charting something different from what the tile and the alert task actually query.
  • Fix: Extract the builder-tile and raw-SQL-tile config assembly into a shared helper and call it from both Tile and TileAlertChart.
• packages/app/src/utils/alerts.ts:50 — extendDateRangeToInterval is eight near-identical branches of threshold-and-unit boundary math with no test coverage; utils/__tests__/alerts.test.ts only exercises normalizeNoOpAlertScheduleFields, so a wrong magnitude or comparator in any branch silently mis-scopes the detail chart and evaluations range.
  • Fix: Add unit tests covering each interval's threshold, an already-wide range that must pass through unchanged, the exact-threshold boundary, and an unmatched interval.
• packages/app/src/api.ts:233 — useAlertEvaluations is the cursor engine behind the entire load-more UX and has no test coverage at all, and getNextPageParam trusts lastPage.nextBefore whenever hasMore is true even though nextBefore is only .optional() in the schema.
  • Fix: Add a test with a mocked server asserting getNextPageParam returns the cursor when hasMore is true, undefined when false, and terminates rather than looping when hasMore is true with nextBefore absent.
• packages/api/src/routers/api/alerts.ts:174 — The new evaluation-history capability is mounted only on the session-authenticated internal router; the API-key-authenticated external-api/v2/alerts.ts registers no equivalent route and the MCP clickstack_get_alert tool is fixed at getRecentAlertHistories({ limit: 20 }), so an agent cannot reproduce the time-ranged failure investigation a human can now perform.
  • Fix: Add a v2 route that calls getAlertEvaluations under validateUserAccessKey, and extend the MCP tool with startTime/endTime/before parameters.

🔵 P3 nitpicks (7)

• packages/common-utils/src/types.ts:2185 — AlertEvaluationsApiResponseSchema declares nextBefore optional independently of hasMore, so the "cursor present whenever more pages exist" contract is enforced by comment rather than by a discriminated union.
  • Fix: Model the response as a union of { hasMore: true, nextBefore: Date } and { hasMore: false }.
• packages/common-utils/src/types.ts:621 — AlertErrorSchema.timestamp is z.union([z.string(), z.date()]), but the value is always a string once serialized, so every client consumer must handle a Date branch that cannot occur.
  • Fix: Type timestamp as z.string() and rely on the existing server-side pre-serialization wrapper for the Date form.
• packages/app/src/components/alerts/AlertHistoryCards.tsx:221 — showErrorIndicator and history are independent optional props with only one valid pairing, so a caller can pass an explicit window array alongside an indicator summarizing alert.executionErrors, which describes a different set of windows.
  • Fix: Split out a presentational strip component and keep the executionErrors indicator wired up only in the alerts-list caller.
• packages/api/src/tasks/checkAlerts/index.ts:228 — makeAlertError, HARDCODED_ALERT_ERROR_MESSAGES, and makeQueryAlertError were added to the already-1860-line index.ts even though errors.ts is the dedicated home for the classification predicates they call, so adding an AlertErrorType now requires edits in two files.
  • Fix: Move the three error-construction helpers into errors.ts alongside isQueryTimeoutError.
• packages/app/src/components/alerts/AlertHistoryCards.tsx:199 — AlertErrorsIndicator hardcodes color: 'var(--mantine-color-red-6)', which agent_docs/code_style.md prohibits in favour of the semantic danger token.
  • Fix: Replace the raw Mantine color variable with the documented semantic danger token.
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:95 — EvaluationRow uses <Text c="red"> where agent_docs/code_style.md specifies <Text variant="danger"> for inline status text.
  • Fix: Switch to variant="danger".
• packages/app/src/components/alerts/__tests__/AlertEvaluationsTable.test.tsx:85 — Both sentinel tests replace useInViewport with a hardcoded boolean, so they only re-assert the component's own if (inViewport && !isFetchingNextPage) guard and would still pass if the ref were never attached to the sentinel element.
  • Fix: Keep the guard tests and add one that renders the sentinel against a polyfilled IntersectionObserver to prove the ref is wired.

---

Reviewers (13): correctness, adversarial, security, reliability, api-contract, performance, testing, maintainability, kieran-typescript, data-migrations, julik-frontend-races, project-standards, agent-native.

Testing gaps:

• No fixture places an AlertState.ERROR row where removing the $ne: AlertState.ERROR filter from getPreviousAlertHistories or getConsecutiveWindowHistories would change the result, so a regression on the PR's load-bearing exclusion invariant would not fail a test.
• No test drives the error path with an error shaped like what BaseClickhouseClient.query() actually throws, which is why the QUERY_TIMEOUT classification gap passes CI.
• No test forces recordAlertErrors to reject and asserts the specific classification survives instead of being overwritten with UNKNOWN.
• No test covers concurrent upsertErrorHistory calls for the same { alert, createdAt, state } key — the existing dedupe test only exercises sequential ticks and passes with or without a unique index.
• getAlertEvaluations pagination has no test isolating truncatedByCount from truncatedByScanBound, no tie-on-identical-createdAt cursor test, and no test for an empty page returned with hasMore: true.
• AlertHistoryCards truncation and padding math is never exercised above maxItems, though the detail page passes 60 windows.
• AlertDetailPage, AlertDetailChart, and useAlertAnnotations have no tests; the recordAlertErrors branch where evaluationWindowStart is undefined is uncovered.

Coverage limitations: Bash, Grep, and Glob were unavailable in this environment (bwrap failed on every invocation, including with the sandbox disabled), so no git diff was computed — reviewers read the checked-out head and separated feature code from pre-existing code by inspection, which means added-vs-existing attribution on the large modified files is best-effort. ce-learnings-researcher was not run (docs/solutions/ could not be enumerated without Glob), and the root AGENTS.md changeset requirement could not be verified for the same reason — confirm a changeset entry exists before merge.

[github-actions]

<!-- deep-review -->

Deep Review

🔴 P0/P1 — must fix

• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:209 — The scroll sentinel re-arms on every isFetchingNextPage false-transition, and because a scan-bound-truncated page legitimately returns {data: [], hasMore: true, nextBefore}, an empty page appends no rows, never scrolls the sentinel out of view, and fires the next fetch immediately — a wide picked range or an alert with sparse history turns one page view into hundreds of back-to-back aggregation requests with no user interaction.
  • Fix: Track consecutive pages that appended zero rows and stop auto-firing onLoadMore after a small threshold, falling back to an explicit "Load more" button.
  • _{julik-frontend-races, adversarial, correctness, testing}

🟡 P2 — recommended

• packages/api/src/controllers/alertHistory.ts:290 — fetchStructuredWindows $pushes a full sub-document for every matched row into one array per window and applies $limit only after $group, while ALERT_EVALUATION_GROUPS_LIMIT is applied in JS after materialization, so the scan is bounded in time but not in rows and a high-cardinality group-by alert can exceed the 100MB $group limit with no allowDiskUse fallback.
  • Fix: Cap rows per window inside the pipeline before the final $group and pass allowDiskUse: true on the aggregate.
  • _{performance, adversarial, correctness}
• packages/api/src/controllers/alertHistory.ts:455 — getAlertTransitionsInRange matches the full caller-supplied range (clamped only to 31 days) with no per-window scan bound and no allowDiskUse, unlike the sibling getAlertEvaluations that was deliberately bounded.
  • Fix: Apply the same (limit + 1) * interval scan bound (or a window-count cap) to this aggregation and set allowDiskUse: true.
• packages/api/src/tasks/checkAlerts/index.ts:1038 — The recordAlertErrors call in the query-failure branch is unwrapped inside the outer try, so a Mongo write failure propagates to the catch at line 1461, which reclassifies the error as UNKNOWN and overwrites the QUERY_TIMEOUT/QUERY_ERROR diagnostic with a hardcoded message — the identical call at line 1479 already has its own guard.
  • Fix: Wrap the line 1038 call in its own try/catch that logs and returns, mirroring the guard at line 1479.
  • _{adversarial, reliability}
• packages/api/src/tasks/checkAlerts/index.ts:827 — evaluationWindowStart is declared at line 827 but only assigned at line 869, after ms(alert.interval), normalizeScheduleStartAt, normalizeScheduleOffsetMinutes, and getScheduledWindowStart; a throw anywhere in that span reaches the catch with the value undefined, and recordAlertErrors then skips upsertErrorHistory entirely, silently dropping exactly the configuration-level failures from the evaluation history.
  • Fix: Derive a best-effort window start in the catch block when the hoisted value is still undefined so every failure is attributable to a window.
  • _{reliability, correctness, testing, adversarial}
• packages/api/src/tasks/checkAlerts/providers/default.ts:461 — upsertErrorHistory uses $set: { errors }, replacing the whole array, so when a window fails twice with different error classes across ticks (a query timeout, then a webhook failure on a later successful evaluation) the first diagnostic is destroyed even though the read path deduplicates by type and message specifically to display several.
  • Fix: Merge with $addToSet: { errors: { $each: errors } } plus a length cap, and add a test covering two distinct error types within one window.
  • _{correctness, testing}
• packages/api/src/tasks/checkAlerts/index.ts:920 — The chartConfig == null early return and the !meta early return around line 1200 only log and return, writing no AlertHistory row of any state, so a permanently broken alert renders as "No evaluations in the selected time range." on the new detail page — the page actively conceals the failure it was added to surface.
  • Fix: Call recordAlertErrors with an INVALID_ALERT error and the current window before returning from both branches, and cover each with a test.
  • _{testing, adversarial, correctness}
• packages/app/src/AlertDetailPage.tsx:129 — isError and error are never destructured from useAlertEvaluations and AlertEvaluationsTable accepts no error prop, so a failed page fetch leaves hasNextPage true from the last successful page, keeps the sentinel mounted, and re-arms indefinitely behind a permanent "Loading older evaluations…" spinner; a first-page failure instead renders the empty-state message, presenting a server error as no data.
  • Fix: Thread isError into the table, stop auto-fetching in the error state, and render a retry affordance instead of the spinner.
  • _{julik-frontend-races, adversarial, testing}
• packages/api/src/tasks/checkAlerts/providers/default.ts:422 — executionErrors accumulates one entry per failing group per evaluation, and that whole array is now copied into a 30-day-retained per-window AlertHistory row with no dedupe or element cap before the write, while the read path collapses every byte-identical WEBHOOK_ERROR entry to a single line.
  • Fix: Dedupe by type and message and cap the array length before persisting, rather than only on read.
  • _{adversarial, security}
• packages/api/src/controllers/alertHistory.ts:410 — getRecentAlertHistoriesBatch wraps each per-alert aggregation in Promise.all with no per-item catch, so one alert's query failure rejects the whole batch and the alerts list route returns 500 instead of degrading to an empty history for that one alert; the equivalent prefetch helpers in the alert task share the pattern.
  • Fix: Catch inside the queue.add callback, log, and return an empty history for the failing alert.
• packages/app/src/utils/alerts.ts:50 — extendDateRangeToInterval is new, drives the detail page's chart and evaluation range through eight interval-specific branches plus a fallback, and has no tests; the colocated test file only covers normalizeNoOpAlertScheduleFields.
  • Fix: Add unit tests for each interval branch, the already-wide no-op case, and the exact-boundary case.
  • _{testing, maintainability}
• packages/api/src/models/alert.ts:12 — AlertState is redeclared verbatim, doc comment included, alongside the definition in packages/common-utils/src/types.ts; this change had to add ERROR to both copies by hand, and the same file already re-exports AlertThresholdType from common-utils, so the correct pattern was available.
  • Fix: Import and re-export AlertState from @hyperdx/common-utils/dist/types instead of maintaining a second copy.
  • _{project-standards, maintainability, kieran-typescript}
• packages/api/src/controllers/alertHistory.ts:94 — fetchGroupedWindows/mapGroupedHistories and fetchStructuredWindows/mapStructuredWindow are near-identical $match/$sort/$group/$sort/$limit pipelines differing only in whether group identity survives the $push, and getAlertTransitionsInRange adds a third, so the ERROR-exclusion rule is now hand-written in three places that must stay consistent while the names give a reader no way to tell them apart.
  • Fix: Derive the alerts-page shape from fetchStructuredWindows, or rename to encode the real distinction such as fetchCollapsedWindows versus fetchPerGroupWindows.
• packages/api/src/tasks/checkAlerts/providers/index.ts:106 — evaluationWindowStart is optional on recordAlertErrors, so an out-of-tree AlertProvider keeps satisfying the interface with a two-argument method while silently recording zero ERROR rows, and the new detail page presents that provider's evaluation history as complete rather than incomplete.
  • Fix: Document at the interface that omitting the parameter opts out of evaluation-error history, and add a contract test asserting registered providers produce an ERROR row.
  • _{api-contract, adversarial}

🔵 P3 nitpicks (10)

• packages/api/src/models/alertHistory.ts:84 — The one-ERROR-row-per-window invariant rests only on the upsert filter; the collection has no unique index on {alert, createdAt, state}, so two concurrent ticks or replicas can both miss and both insert.
  • Fix: Add a partial unique index filtered to state: 'ERROR', or document that duplicates are expected and rely on read-side dedupe.
• packages/api/src/controllers/alertHistory.ts:491 — When the alert was firing before startTime and the first in-range window lands exactly on startTime in a non-firing state, pinCarryInIfFiring pushes an ALERT marker and the next branch immediately pushes an OK marker at the identical timestamp; minute-quantized bounds make this alignment routine.
  • Fix: Only pin the carry-in marker when the first in-range window is strictly after startTime or is itself firing.
• packages/app/src/api.ts:238 — useAlertEvaluations sets no staleTime or refetchOnWindowFocus, so a focus refetch replays each page at its original cursor and a boundary shift can yield two entries sharing a createdAt, which is used directly as the React key for rows holding local expand state.
  • Fix: Disable focus refetching for this historical query, or dedupe by createdAt when flattening pages.
• packages/api/src/controllers/alertHistory.ts:362 — The Mongo date filter is typed Record<string, Date>, so a mistyped operator key compiles and fails silently at the database layer instead of at the type checker.
  • Fix: Type it as { $gte?: Date; $lt?: Date; $lte?: Date } or FilterQuery<IAlertHistory>['createdAt'].
• packages/api/src/controllers/alertHistory.ts:243 — r.group as string papers over a non-narrowing .filter, so editing the predicate would keep compiling after the runtime guarantee the cast depends on is gone.
  • Fix: Make the filter a type predicate returning r is EvaluationWindowRow & { group: string } and drop the cast.
• packages/app/src/components/alerts/AlertHistoryCards.tsx:75 — dedupeAlertErrors reimplements the server's dedupeErrors but omits the newest-first sort, so the two have already diverged on ordering while re-deduplicating data the server already deduplicated.
  • Fix: Share one implementation from common-utils, or at minimum match the server's sort order.
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:32 — stateBadge is a third hand-written AlertState-to-presentation mapping alongside stateToBgColorClass and the inline badge switch in AlertsPage.tsx, with no shared source of truth.
  • Fix: Centralize label and color in one ALERT_STATE_PRESENTATION map that all three render paths read.
• packages/api/src/routers/api/alerts.ts:217 — The MAX_HISTORY_SPAN_MS clamp is re-derived independently in the /evaluations and /history routes with different null handling for startTime.
  • Fix: Extract a single clampStartTime(startTime, endTime, maxSpanMs) helper used by both routes.
• packages/app/src/utils/alerts.ts:50 — The interval-to-window-size ladder is written out a second time here, duplicating the pairs already encoded in intervalToDateRange a few lines above.
  • Fix: Extract one Record<AlertInterval, Duration> map and have both functions read from it.
• packages/common-utils/src/types.ts:606 — AlertState.ERROR now appears in history[].state on the pre-existing /alerts and /alerts/:id responses, widening the realized value set for consumers written against the old three states with no changelog or schema note.
  • Fix: Note the new value in the alert API docs and confirm external consumers have an unknown-state fallback.

---

Reviewers (11): correctness, adversarial, security, reliability, api-contract, performance, testing, maintainability, kieran-typescript, julik-frontend-races, project-standards.

Testing gaps:

• No test fails if state: { $ne: ERROR } is dropped from getPreviousAlertHistories or getConsecutiveWindowHistories — the retry/backfill invariant this change rests on would break silently.
• No coverage of getAlertEvaluations over a range that is entirely a gap (the empty-page-with-hasMore chain) or of a before cursor at or below startTime.
• AlertEvaluationsTable accepts no error input, so failed-page behavior is untestable without a signature change.
• No coverage of a group-by alert exceeding ALERT_EVALUATION_GROUPS_LIMIT, or of high group cardinality against the $group stage.
• AlertHistoryCardList's new history, maxItems, and showErrorIndicator props — the exact usage the detail page introduces — are exercised by no test.

Reviewer note: Bash, Grep, and Glob were unavailable in this environment, so reviewers worked from the checked-out files rather than a computed diff; findings were confirmed by reading the cited code, but new-versus-pre-existing attribution is weaker than usual, and the repo's changeset requirement could not be verified. Security review found no exploitable issues and confirmed the new endpoint resolves the alert via getAlertById(id, teamId) before reading history.

[github-actions]

Scope note: Bash was unavailable in this environment (every invocation failed at sandbox setup, with and without the override), so no git diff could be produced. Reviewers worked from the files at HEAD with the changed surface identified by symbol. Findings were verified against the actual code, but new-vs-pre-existing attribution is best-effort; where a finding sits in code that predates this work but is newly reached by it, that is stated inline.

🔴 P0/P1 — must fix

• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:276 — The load-more sentinel's effect depends on isFetchingNextPage, so it re-fires every time a fetch settles, including a failed one, while hasNextPage stays true and no rows are appended to push the sentinel out of view.
  • Fix: Thread isError into the table and gate the effect on !isError, render a terminal error row with an explicit retry instead of the perpetual spinner, and re-arm auto-fetch only after the loaded page count increases.
  • _{adversarial, performance, reliability, correctness}
• packages/api/src/controllers/alertHistory.ts:323 — $group pushes one subdocument per matched AlertHistory row before $limit runs, and the 50-group cap is applied in Node at line 298, so a group-by alert's cost scales with group cardinality; allowDiskUse is unset, so the post-$group blocking sort can exceed the 100 MB stage limit and a single fat window can exceed the 16 MB BSON limit, 500-ing the new page.
  • Fix: Bound the accumulator inside the pipeline — narrow to the target createdAt values with $group/$sort/$limit first, then use $topN capped at ALERT_EVALUATION_GROUPS_LIMIT with a separate $sum for the total — and pass allowDiskUse on both aggregate calls.
  • _{security, adversarial, performance, correctness}

🟡 P2 — recommended

• packages/api/src/controllers/alertHistory.ts:255 — Each window's lastValues is the uncapped union across every group row and is returned verbatim, while the client reads only the first and last entry and skips latestValue() entirely for grouped rows.
  • Fix: Return only the earliest and latest bucket entries per window, computing the distinct-bucket count needed by resolveWindowAnalytics during the flatten instead of from the retained array.
  • _{security, adversarial, performance}
• packages/api/src/controllers/alertHistory.ts:412 — hasMore is set from the scan bound alone, so every empty scan slice returns data: [] with an advancing cursor, making the client walk a wide range one slice at a time (about 220 round trips for a 1-minute alert over the clamped span).
  • Fix: Loop the scan slice server-side, advancing pageEndMs to scanFloorMs until limit windows are collected or startTime is reached, capped by a fixed slice budget per request.
  • _{adversarial, performance, reliability, correctness, api-contract}
• packages/api/src/tasks/checkAlerts/providers/default.ts:477 — $set: { errors } replaces the whole array on the window's single ERROR row, so a query timeout recorded on one tick is erased when a later tick for the same window records a webhook failure.
  • Fix: Merge into the existing array with $addToSet/$each plus a bounded $slice, keeping one entry per distinct error type.
  • _{correctness, adversarial, reliability}
• packages/api/src/tasks/checkAlerts/index.ts:1051 — This recordAlertErrors call is not individually guarded, unlike the identical call at line 1515, so a failed Mongo write propagates to the outer catch, which reclassifies the failure as UNKNOWN and overwrites the already-persisted timeout diagnostic with the hardcoded generic message.
  • Fix: Wrap this call in its own try/catch that logs and swallows, mirroring the guarded call in the outer catch.
  • _{reliability, adversarial}
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:145 — firingGroups counts the server-capped groups array while groupsTotal is the uncapped count, so a window with 500 groups of which 300 are firing renders as 50/500 groups firing.
  • Fix: Return a server-computed firing-group total alongside groupsTotal and render that instead of counting the truncated array.
• packages/api/src/tasks/checkAlerts/index.ts:1136 — The webhookDurationMs bracket wraps all of fireChannelEvent, which for saved-search alerts runs a ClickHouse query for sample log lines before any delivery attempt, so the value surfaced as the Webhook Duration column attributes ClickHouse latency to the webhook destination.
  • Fix: Move the measurement inside the delivery path so it covers only notification I/O, or rename the field, column, and doc comment to reflect total notification time.
• packages/api/src/tasks/checkAlerts/index.ts:978 — tryOptimizeConfigWithMaterializedView performs ClickHouse I/O outside the try block that starts at line 1001, so on materialized-view-backed sources a ClickHouse outage lands in the outer catch as UNKNOWN with no queryDurationMs, bypassing the new timeout classification entirely.
  • Fix: Wrap the call so it either degrades to the unoptimized config like the adjacent alias-clause block or routes through makeQueryAlertError.
• packages/api/src/tasks/checkAlerts/index.ts:1221 — The meta == null early return, and the chartConfig == null return at line 927, set the error outcome and log but persist nothing, leaving those failed windows with no error record and no row in the new evaluation history.
  • Fix: Call recordAlertErrors with an appropriate error type and the evaluation window start on both early-return paths.
• packages/api/src/models/alertHistory.ts:114 — Nothing backs the {alert, createdAt, state} upsert key with a unique constraint, and in production the task runs as a one-shot process with no lock, so two overlapping ticks can each insert an ERROR row for the same window and inflate the per-window counts and groupsTotal read back by the endpoint.
  • Fix: Add a partial unique index on {alert, createdAt, state} filtered to state: 'ERROR' and treat a duplicate-key error in the upsert as success.
  • _{adversarial, reliability, performance, learnings-researcher}
• packages/app/src/api.ts:238 — useAlertEvaluations sets no staleTime, refetchOnWindowFocus, or maxPages, and the QueryClient has no defaultOptions, so a tab blur/focus makes React Query re-fetch every accumulated page sequentially against the heavy per-window aggregation.
  • Fix: Set a non-zero staleTime, disable refetchOnWindowFocus/refetchOnMount for this query, and cap retained pages with maxPages.
• packages/common-utils/src/types.ts:832 — analytics is declared on AlertHistorySchema, which types AlertsPageItemSchema.history, but the producer for GET /alerts and GET /alerts/:id never projects or emits it, so the same declared type carries different real field sets depending on the endpoint.
  • Fix: Move analytics onto AlertEvaluationSchema, the only response shape whose producer populates it.
• packages/api/src/controllers/alertHistory.ts:173 — AlertEvaluationEntry is hand-written alongside the zod AlertEvaluationSchema for the same payload, and because the router assigns data: page.data from a variable rather than an object literal, excess-property checking never fires, so a field added on one side is silently dropped on the other.
  • Fix: Derive the controller types from the zod-inferred types, or add a compile-time assignability assertion pinning the two together.
  • _{kieran-typescript, maintainability}
• packages/api/src/controllers/alertHistory.ts:489 — This aggregation has no $limit and pushes every matched row's state into a per-window array, bounded only by the 31-day span clamp; the new detail page fires it unconditionally for the picked range, so a 1-minute alert on a wide range scans tens of thousands of windows times group count.
  • Fix: Replace states: { $push: '$state' } with boolean $max accumulators for the firing and pending cases and bound the returned window count. (Pre-existing aggregation, newly reached at user-controlled scale by the detail page.)
• packages/api/src/tasks/checkAlerts/providers/__tests__/default.int.test.ts:1 — The provider's dedicated test file covers only getAlertTasks and the link builders, so the new updateAlertState, recordAlertErrors, and upsertErrorHistory write paths have no direct coverage; the read-side tests insert ERROR rows manually and never exercise the upsert.
  • Fix: Add tests asserting first-tick ERROR-row creation, same-window upsert into a single row across two ticks, and that $setOnInsert defaults apply only on insert.

🔵 P3 nitpicks (15)

• packages/api/src/controllers/alertHistory.ts:396 — Record<string, Date> for the Mongo filter accepts any key, so a mistyped range operator compiles and silently changes query semantics.
  • Fix: Introduce a named type with explicit $gte/$lt/$lte fields and use it for both fetch helpers.
• packages/api/src/controllers/alertHistory.ts:275 — r.group as string relies on a preceding .filter() that TypeScript cannot narrow through, so the cast keeps compiling if that condition is ever edited.
  • Fix: Make the filter a type predicate narrowing group to string and drop the cast.
  • _{kieran-typescript, project-standards}
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:223 — The error label uses a raw palette color where the repo's documented style guide calls for the semantic variant.
  • Fix: Replace c="red" with variant="danger".
• packages/api/src/routers/api/alerts.ts:188 — endTime and before are unbounded positive integers, so a value past the maximum representable date yields Invalid Date bounds that reach the aggregation, and the .refine only compares the two when both are supplied.
  • Fix: Add a max bound to the epoch-ms fields and extend the refinement to compare against the default end time when endTime is omitted.
  • _{adversarial, security}
• packages/api/src/tasks/checkAlerts/providers/default.ts:426 — Nothing removes a window's ERROR row when a later attempt succeeds cleanly, and groupStateToOverallState ranks ERROR above OK, so a recovered window reads as an error for the full retention period and its history-strip card renders as a modal button instead of the source-search link.
  • Fix: Either delete the ERROR row when a later attempt for the window records no errors, or rank ERROR below OK so the row contributes only error detail.
• packages/api/src/tasks/checkAlerts/errors.ts:34 — The constructor-name fallback asserts the full ClickHouseError shape while checking only a name string, so any class with that name is treated as a classified ClickHouse error.
  • Fix: Narrow the predicate's return type to what is actually verified, or add a structural check on type/code.
• packages/api/src/tasks/checkAlerts/providers/index.ts:108 — The ERROR-row upsert lives entirely in the default provider behind two optional trailing params, so an out-of-tree provider registered through the extension point keeps type-checking while silently producing no evaluation-error history.
  • Fix: State the persistence obligation in the interface docs, or move the upsert into shared code every provider path calls.
  • _{api-contract, reliability, maintainability}
• packages/api/src/routers/external-api/v2/alerts.ts:353 — The new per-window evaluation history, error records, and analytics have no API-key-authenticated route, and the MCP single-alert tool still reads the pipeline that never projects analytics or per-group rows, so the diagnostics are browser-session-only.
  • Fix: Add an external-API evaluations route reusing getAlertEvaluations, and point the MCP tool at the structured-window path.
• packages/api/src/models/alert.ts:151 — The Alert document's state enum now accepts ERROR, which the published v2 OpenAPI AlertState enum does not list, leaving a comment as the only guard on the history-only invariant.
  • Fix: Narrow the document's enum to the non-ERROR members via a shared constant used by the OpenAPI schema too.
• packages/api/src/controllers/alertHistory.ts:262 — Group rows are fully mapped and sorted, with a localeCompare tiebreak, before being sliced to 50, so per-request CPU scales with group cardinality on the single event-loop thread.
  • Fix: Do a bounded top-K selection instead of a full sort and use relational string comparison for the tiebreak.
• packages/api/src/controllers/alertHistory.ts:95 — fetchGroupedWindows/mapGroupedHistories and fetchStructuredWindows/mapStructuredWindow independently reimplement the same pipeline shape, lastValues comparator, and error dedupe.
  • Fix: Extract the shared comparator and dedupe, and comment why the two pipelines cannot be unified.
• packages/api/src/controllers/alertHistory.ts:1 — At 548 lines the file is well past the 300-line ceiling in the repo's documented code-style guide, with most of the growth from the new evaluation-window logic.
  • Fix: Split the structured-window and pagination functions into their own module.
• packages/api/src/tasks/checkAlerts/providers/default.ts:508 — A configured timeout of 0 passes the non-negative schema check, is not replaced by the ?? default, and is accepted by the client, so the timeout message renders as a 0-second limit while the client treats it as unlimited.
  • Fix: Require a positive value in the task-args schema, or coerce falsy values to the default in getClickHouseClient.
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:345 — Rows are neither memoized nor virtualized and pages accumulate without bound, with a tooltip instance mounted per row.
  • Fix: Wrap the row component in React.memo and cap retained pages or virtualize the table body.
• packages/api/src/controllers/alertHistory.ts:321 — The $sort immediately before a $group on the same key imposes no order any consumer relies on, since every downstream array is re-sorted, but it makes index selection load-bearing for avoiding a blocking sort.
  • Fix: Drop the pre-$group sort, or comment that it exists only to steer the query plan.

---

Reviewers (12): correctness, adversarial, security, reliability, performance, api-contract, kieran-typescript, testing, maintainability, project-standards, agent-native, learnings-researcher.

Verified as sound (no findings): team scoping on the new endpoint via getAlertById; pagination cursor progress in every branch; ERROR-row exclusion from the due-ness gate, retry range, consecutive-window counting, and transition annotations; per-alert isolation in the evaluation loop; no NoSQL-operator injection or raw-HTML sink.

Testing gaps:

• No coverage of the same-window error sequence (query failure then webhook failure) asserting what survives on the single ERROR row.
• No concurrency test asserting exactly one ERROR row exists after two parallel upserts for the same window.
• No high-cardinality group-by fixture bounding the evaluations aggregation's document count or serialized response size.
• No client test that the load-more sentinel stops after a failed page fetch, or that empty-but-hasMore pages terminate in a bounded number of fetches.
• useAlertAnnotations/alertTransitionsToAnnotations and the detail page itself have no test files.
• No test for a ClickHouse failure raised from the materialized-view optimization path, where classification currently falls through to UNKNOWN.

[github-actions]

Deep Review

⚠️ Partial coverage. git/bash are non-functional in this environment (bwrap: Can't create file at /home/.mcp.json: Permission denied), so no unified diff could be produced and reviewers worked from a reconstructed file inventory against the checked-out head. 13 reviewers were dispatched; 2 returned before output was required. The findings below are ones verified by direct reading of the source, plus the one returned persona report. Treat this as a partial pass, not a clean bill of health.

✅ No critical issues found.

🟡 P2 -- recommended

• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:277 -- The sentinel effect refires every time isFetchingNextPage settles back to false, and the server sets hasMore whenever the picked range exceeds one (limit + 1) × interval scan slice, so a wide time picker range auto-fires many sequential aggregation requests with no user scrolling.
  • Fix: Gate onLoadMore behind an explicit user action (or a per-mount fetch budget) once the sentinel has already auto-triggered a page, so a wide range does not chain-fetch the whole retention window.
• packages/api/src/controllers/alertHistory.ts:383 -- scanMs = (limit + 1) * intervalMs is ~3.35h for a 1-minute-interval alert, so truncatedByScanBound stays true for ~215 pages across a 30-day picker range, and each page runs a full $match/$group aggregation.
  • Fix: Scale the scan slice with the requested span (or cap total pages per range) so page count does not grow linearly with range ÷ interval.
• packages/api/src/tasks/checkAlerts/providers/default.ts:484 -- upsertErrorHistory relies on {alert, createdAt, state: ERROR} being unique to collapse per-tick retries into one row, but packages/api/src/models/alertHistory.ts:114 declares no unique index on that key, so concurrent evaluations of the same alert and window can both miss the match and insert duplicate ERROR rows.
  • Fix: Add a unique partial index on {alert: 1, createdAt: 1, state: 1} filtered to state: 'ERROR' so the upsert's dedupe guarantee is enforced by the database.
• packages/api/src/mcp/tools/alerts/getAlert.ts:129 -- The MCP alert tool still calls getRecentAlertHistories (fixed 20 windows, no per-group breakdown, no analytics, no paging) while the UI consumes getAlertEvaluations, and no external v2 route exposes the new evaluation history, so an agent cannot retrieve what a human now sees.
  • Fix: Point the MCP getAlert tool at getAlertEvaluations and add a corresponding external-API v2 evaluations route.
  • _agent-native

🔵 P3 nitpicks (2)

• packages/api/src/routers/api/alerts.ts:163 -- MAX_HISTORY_SPAN_MS clamps the queryable span to 31 days while the TTL index at packages/api/src/models/alertHistory.ts:114 expires rows after 30 days, so the oldest day of the advertised range is always empty.
  • Fix: Derive the clamp from the same TTL constant so the two cannot drift.
• packages/api/src/tasks/checkAlerts/index.ts:1518 -- Failures thrown before evaluationWindowStart is assigned at line 876 pass undefined, and packages/api/src/tasks/checkAlerts/providers/default.ts:461 then skips the history upsert entirely, so those evaluations leave no ERROR row and stay invisible in the new evaluation history.
  • Fix: Fall back to a rounded-down current window start when the scheduled window start has not been computed yet.

---

Reviewers (3): correctness (orchestrator direct verification), agent-native, learnings-researcher.

Testing gaps:

• No verification was possible that the concurrent/duplicate ERROR-row upsert path is covered, since the test suites could not be executed or searched in this environment.
• Front-end auto-pagination behavior over a wide time range is not exercised by any test that could be confirmed present.
• 11 of 13 dispatched reviewers (correctness persona, testing, maintainability, project-standards, security, reliability, performance, api-contract, adversarial, kieran-typescript, julik-frontend-races) did not return before output was required; their coverage areas are unreviewed.