[HDX-4997] Persist alert evaluation errors and analytics in AlertHistory

[wrn14897]

Summary

Depends on #2786 (the read model: types, schema fields, evaluations endpoint, UI). Based on its branch — GitHub retargets this PR to main automatically when #2786 merges. The two PRs share zero files.

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).

This PR is the write side: the alert task records what #2786 displays.

• 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/notification failures also produce an ERROR row alongside the normal evaluation rows; a stale ERROR row from a failed earlier tick is removed when a clean same-window retry succeeds (otherwise the window would render as ERROR forever).
• 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.

How to test on Vercel preview

N/A — the alerting job doesn't run in LOCAL_MODE previews. Covered by integration tests.

How this was tested

• api tsc --noEmit; errors.test unit suite (18 tests: classification 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 on successful retry, cross-window ERROR rows preserved, QUERY_TIMEOUT classification, failed-window-still-retried/backfilled, webhook-failure ERROR rows (incl. grouped alerts), analytics fields (steady state + backfill)

References

• Linear Issue: HDX-4997
• Related: HDX-4977 (alert query timeout rollout — this PR is its visibility prerequisite)
• Read side (merges first): [HDX-4997] Record alert evaluation errors in AlertHistory and add alert detail page #2786

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).

[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 10:41pm
hyperdx-storybook	Ready	Preview	Aug 6, 2026 10:41pm

[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 — 230 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: 14
• Production lines changed: 642 (+ 955 in test files, excluded from tier calculation)
• Critical-path lines changed: 232
• Branch: warren/HDX-4997-alert-error-backend
• 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 adds the alert-evaluation write path needed to preserve evaluation failures and diagnostics in AlertHistory.

• Upserts query, timeout, invalid-alert, and webhook failures as window-scoped ERROR histories.
• Keeps ERROR histories out of scheduling, backfill, and consecutive-window calculations so failed evaluations remain retryable.
• Records query duration, webhook duration, and backfilled-bucket analytics on generated histories.
• Classifies wrapped ClickHouse, client, and socket timeouts as QUERY_TIMEOUT and documents the new API enum.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the wrapped-timeout issue is fixed by traversing the production wrapper’s cause, and the separate pagination-gap thread is resolved by a cursor that advances across empty scan slices.

Important Files Changed

Filename	Overview
packages/api/src/tasks/checkAlerts/errors.ts	Walks bounded, cycle-safe error cause chains to classify wrapped ClickHouse and socket timeouts.
packages/api/src/tasks/checkAlerts/index.ts	Attributes failures and evaluation analytics to the scheduled window while excluding ERROR histories from retry and consecutive-window state.
packages/api/src/tasks/checkAlerts/providers/default.ts	Upserts one ERROR history per alert window and removes a stale same-window error after a clean retry.
packages/api/src/tasks/checkAlerts/providers/index.ts	Extends the provider contract with optional evaluation-window and analytics inputs while retaining compatibility.
packages/common-utils/src/clickhouse/index.ts	Exposes the configured request timeout for actionable alert timeout messages.
packages/api/src/tasks/checkAlerts/tests/errors.test.ts	Covers direct, wrapped, nested, non-timeout, and cyclic timeout-classification inputs.
packages/api/src/tasks/checkAlerts/tests/checkAlerts.int.test.ts	Exercises ERROR persistence, retry cleanup, deduplication, backfill behavior, webhook failures, and analytics.

Sequence Diagram

sequenceDiagram
  participant Task as Alert task
  participant CH as ClickHouse
  participant Provider as AlertProvider
  participant Mongo as AlertHistory
  participant Hook as Webhook

  Task->>CH: Evaluate alert window
  alt Query fails
    CH-->>Task: Error / timeout
    Task->>Provider: recordAlertErrors(window, analytics)
    Provider->>Mongo: Upsert ERROR history
  else Query succeeds
    CH-->>Task: Query results
    Task->>Hook: Deliver notification when required
    Hook-->>Task: Success or failure
    Task->>Provider: updateAlertState(histories, errors)
    Provider->>Mongo: Persist normal histories
    alt Notification failed
      Provider->>Mongo: Upsert same-window ERROR history
    else Clean same-window retry
      Provider->>Mongo: Delete stale ERROR history
    end
  end

Loading

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

[github-actions]

E2E Test Results

✅ All tests passed • 268 passed • 1 skipped • 939s

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

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

<!-- deep-review -->

Deep Review

Scope caveat: Bash, Grep, and Glob were unavailable in this environment (bwrap init failure), so no git diff against the base SHA could be computed and the plugin's persona fan-out could not run as designed. Findings below were derived by reading the checked-out head state of the feature's files directly, then adversarially verifying the two highest-impact claims with independent refutation agents. Attribution of a given line to this PR vs. pre-existing code is stated per finding where it matters.

✅ No critical issues found.

🟡 P2 -- recommended

• packages/api/src/controllers/alertHistory.ts:100 -- The new /evaluations pagination bounds each page by a time span derived from limit ((limit + 1) * intervalMinutes), so any hole in evaluation windows wider than that span returns zero rows and reports hasMore: false, permanently hiding older history that is still within the 30-day TTL.
  • Fix: Page by row count — walk back to the TTL floor rather than a limit-derived lookback — and derive hasMore from whether an older row actually exists.
  • _{correctness, adversarial-verifier}
• packages/app/src/components/alerts/AlertHistoryCards.tsx:128 -- A window that both fired and recorded a notification failure aggregates to ALERT (ALERT outranks ERROR in groupStateToOverallState), and every error affordance — striped-red class, error tooltip, UnstyledButton, onShowErrors — is gated on history.state === AlertState.ERROR, so the persisted per-window errors array is delivered to the client and never read.
  • Fix: Gate the error affordance on history.errors?.length instead of history.state, so fired-and-failed windows expose their errors.
  • _{correctness, adversarial-verifier}
• packages/api/src/tasks/checkAlerts/index.ts:920 -- Two evaluation failure paths return after only a logger.error without calling recordAlertErrors — chartConfig == null (line 920) and meta == null (line 1200) — so an alert misconfigured into either state fails on every tick while producing no executionErrors and no ERROR row, leaving exactly the blind spot this change sets out to remove.
  • Fix: Record an INVALID_ALERT error via recordAlertErrors with the current window start on both early-return paths before returning.
  • _{correctness, adversarial-verifier}

🔵 P3 nitpicks (5)

• packages/api/src/tasks/checkAlerts/providers/default.ts:468 -- The ERROR-row upsert is keyed on {alert, createdAt, state} but no unique index backs that key, so two concurrent evaluations of the same alert in the same window can both miss and both insert, yielding duplicate rows.
  • Fix: Add a partial unique index on {alert: 1, createdAt: 1} filtered to state: 'ERROR' so the upsert cannot race into duplicates.
• packages/api/src/tasks/checkAlerts/providers/default.ts:421 -- updateAlertState infers the evaluation window from histories[0]?.createdAt rather than receiving it explicitly, unlike the sibling recordAlertErrors; if a future change ever writes history rows with differing createdAt, both the ERROR-row upsert and the stale-row delete would silently target the wrong window.
  • Fix: Pass the evaluation window start into updateAlertState explicitly instead of reading it off the first history record.
• packages/api/src/tasks/checkAlerts/providers/default.ts:433 -- The stale-ERROR-row deleteOne fires on every successful evaluation, adding a write per alert per window even in the overwhelmingly common case where no ERROR row exists.
  • Fix: Fold the stale-ERROR cleanup into the same bulkWrite as the history inserts so a clean evaluation costs no extra round trip.
• packages/api/src/models/alertHistory.ts:67 -- The new errors[].type field declares enum: AlertErrorType (the enum object) while the sibling state field two declarations above uses Object.values(AlertState), so the two adjacent enum validators rely on different Mongoose coercion behaviour.
  • Fix: Use Object.values(AlertErrorType) to match the adjacent state field's declaration.
• packages/api/src/tasks/checkAlerts/providers/index.ts:106 -- evaluationWindowStart is optional on recordAlertErrors, so an AlertProvider implementation that ignores it still compiles and silently persists no ERROR rows at all, with no type-level signal that window attribution is now required behaviour.
  • Fix: Make evaluationWindowStart a required parameter on recordAlertErrors.

---

Reviewers (6): correctness/orchestrator analysis, adversarial-verifier (pagination), adversarial-verifier (history-strip UI), test-inventory reader, shared-types + frontend reader, ClickHouse client type reader.

Testing gaps:

• No test seeds an AlertState.ERROR row directly into getPreviousAlertHistories or getConsecutiveWindowHistories; the due-ness/backfill exclusion those two rely on is only proven indirectly through a full processAlert integration run.
• AlertHistoryCards.test.tsx fixtures only a pure ERROR window and a pure OK window — no ALERT-state-with-errors fixture, which is why the fired-and-failed rendering gap is uncaught.
• The /evaluations pagination tests build windows at exact interval cadence only; no case exercises a gap wider than the limit-derived lookback, so early hasMore: false termination is untested.
• isQueryTimeoutError classification is well covered at the unit level (wrapped cause chains, TIMEOUT_EXCEEDED/159, ETIMEDOUT, self-referential chains) — no gap found there.

[wrn14897]

Pushed three fixes:

• Stale ERROR row on successful same-window retry (1773c32): updateAlertState now deletes the {alert, createdAt, state: ERROR} row when a clean evaluation (no errors) writes normal history rows for that window — previously the row survived forever and the evaluations view ranks ERROR above OK/ALERT. ERROR rows from other windows are untouched (a tick that failed and was backfilled later keeps its record), and webhook-failure ERROR rows still coexist with the same window's normal rows (errors.length > 0 takes the upsert branch). Updated the same-window retry integration test and added a cross-window guard test (160/160 pass).
• Wrapped timeout classification (c2a6889): isQueryTimeoutError walks the cause chain — see inline reply.
• Knip (dab076b): unexported ALERT_ERROR_TYPE_LABELS, AlertErrorsContent, and makeQueryAlertError (all internal-only at this layer; the export is re-added in [HDX-4997] Add alert detail page with evaluation history #2798 where the evaluations table consumes them).

The pagination-gap comment is addressed by the nextBefore cursor in stack layer 3 (#2786) — see inline reply.

[changeset-bot]

🦋 Changeset detected

Latest commit: 78111b0

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]

Deep Review

⚠️ Review could not run — environment failure, not a clean bill of health.

The multi-agent review was invoked (mode:report-only base:d09db065f5…) but aborted at Stage 1 (scope detection). Every route to this PR's diff is blocked in this runner, so zero reviewer sub-agents were dispatched and no findings — positive or negative — should be inferred from this comment.

What failed

Every Bash invocation fails during sandbox setup, before the command itself executes:

bwrap: Can't create file at /home/.mcp.json: Permission denied

This reproduced on:

• git merge-base, git diff, git rev-parse, and bare echo
• both sandboxed and with the sandbox explicitly disabled
• the orchestrator session and inside dispatched sub-agents (so the failure is environmental, not a per-session permission gate)

The fallback routes were also unavailable:

Route	Result
git diff / gh pr diff	blocked — bwrap failure above
Grep / Glob	not present in this session, and not resolvable via ToolSearch
WebFetch of …/pull/2797.diff	permission not granted

Read is the only functioning code-access tool, and with no search or git it cannot enumerate changed files. Blind path-probing confirmed a handful of files exist (packages/api/src/tasks/checkAlerts/errors.ts, packages/api/src/models/alert.ts, packages/api/src/routers/api/alerts.ts, packages/app/src/AlertsPage.tsx) but cannot recover the change set, the removed lines, or base-vs-head attribution.

Why no findings were emitted instead

A review assembled from guessed file paths could not distinguish code introduced by this PR from pre-existing code, and could not verify removed lines at all. Since the wrapper's rubric grades on breakage introduced by this diff, and downstream automation treats Fix: lines as authoritative, speculative findings would be worse than none. Notably, one probe (checkAlerts/errors.test.ts absent) is indistinguishable from a wrong path guess rather than a real missing-test gap — exactly the class of false positive that would have shipped.

Suggested remediation for the workflow

• Fix the runner sandbox. The bwrap mount of /home/.mcp.json fails because the path is outside the writable allowlist; scoping MCP config discovery to the workspace (or pre-creating that path) should restore Bash.
• Grant Grep/Glob to the deep-review job as a degraded-mode fallback so file discovery survives a Bash outage.
• Consider passing the diff in as an input (e.g. a pre-computed patch artifact from an earlier step that runs before sandboxing), so scope detection does not depend on shell access at review time.

Re-run once Bash is restored; the review is expected to complete normally at that point.

---

Reviewers (0): none dispatched — aborted at Stage 1 scope detection.

Testing gaps: not assessed — requires the diff.

[github-actions]

<!-- deep-review -->

Deep Review

🔴 P0/P1 — must fix

• packages/api/src/tasks/checkAlerts/providers/default.ts:415 — The post-create writes in updateAlertState (Alert.updateOne, upsertErrorHistory, deleteOne) are unguarded, so a transient Mongo rejection escapes into processAlert's outer catch, which upserts a permanent ERROR row onto a window that already persisted its normal history rows and cannot be re-evaluated.

  • Fix: Wrap the three post-create writes in updateAlertState in their own try/catch that logs and swallows, or track a "history already written for this window" flag so the outer catch skips the ERROR-row upsert and only refreshes executionErrors.
  • _{correctness, reliability, adversarial}

  Chain, all verifiable from the diff: AlertHistory.create is wrapped in Promise.allSettled (default.ts:379) so the rows are durably written and cannot throw; the following Alert.updateOne (:415) and cleanup deleteOne (:442) are bare awaits. A rejection propagates through index.ts:1491 into the catch at index.ts:1496, which calls recordAlertErrors(..., evaluationWindowStart) at index.ts:1515 and upserts an ERROR row keyed to the same window. On the next tick shouldSkipAlertCheck (index.ts:548-551) sees the successfully-created non-ERROR rows for that window and returns true, so updateAlertState — the only caller of the stale-row cleanup — never runs for that window again. The window renders ERROR for the full 30d TTL despite a fully successful evaluation and notification. Before this PR the same catch only overwrote executionErrors, which the next tick corrected.

🟡 P2 — recommended

• packages/api/src/tasks/checkAlerts/index.ts:1051 — The recordAlertErrors call on the query-failure path is not wrapped in try/catch (unlike the identical call at index.ts:1514), so a failure of the ERROR-row upsert escalates into the outer catch and overwrites the actionable QUERY_TIMEOUT message with the hardcoded UNKNOWN text.

  • Fix: Wrap the index.ts:1051 call in the same try/catch used at index.ts:1514, log the persistence failure, and return without falling through to the generic handler.
  • _{adversarial, reliability}

  Alert.updateOne inside recordAlertErrors (default.ts:456) commits executionErrors: [QUERY_TIMEOUT] before the AlertHistory upsert at default.ts:484 runs. If that upsert rejects, the outer catch re-invokes recordAlertErrors with makeAlertError(UNKNOWN, ...), which resolves to HARDCODED_ALERT_ERROR_MESSAGES[UNKNOWN] (index.ts:203) and destroys the timeout diagnosis. It also double-counts the same failure across alertQueryFailuresCounter and alertProcessFailuresCounter.

• packages/api/src/tasks/checkAlerts/index.ts:927 — Two evaluation-failure exits set evalOutcome = 'error' and return without calling recordAlertErrors, so a permanently broken alert produces no ERROR row and leaves a stale executionErrors value from an unrelated earlier tick pinned indefinitely.

  • Fix: Call recordAlertErrors with an INVALID_ALERT error and evaluationWindowStart before returning at both index.ts:936 and index.ts:1224, matching the query-failure path.
  • _{correctness, adversarial, testing, maintainability}

  chartConfig == null (index.ts:927) fires for a PromQL tile (:658), a builder tile whose displayType is unsupported, or a tile whose Source document was deleted (:662-669) — a permanent misconfiguration that recurs every tick forever. meta == null (index.ts:1221) fires when getResponseMetadata finds no numeric or no Date column. alert.executionErrors is written only at default.ts:417 and default.ts:458, neither of which is reached, so the alert detail page shows either nothing or a stale error from a prior failure mode. evaluationWindowStart is already assigned at index.ts:876, before both sites.

• packages/api/src/tasks/checkAlerts/providers/default.ts:442 — The stale-row cleanup uses deleteOne against an upsert key {alert, createdAt, state} that has no unique index backing it, so concurrent evaluations can insert duplicate ERROR rows of which only one is ever removed.

  • Fix: Change deleteOne to deleteMany and add a partial unique index on {alert: 1, createdAt: 1, state: 1} restricted to state: 'ERROR'.
  • _{reliability, adversarial, correctness}

  AlertHistorySchema declares only a TTL index and two non-unique compound indexes (models/alertHistory.ts:114-123). MongoDB's upsert is race-safe against duplicate insertion only when a unique index backs the filter. Overlapping runs are reachable because a query can hang for the full requestTimeout (default.ts:522) while the next tick starts, and ERROR rows are deliberately excluded from shouldSkipAlertCheck so the failing window is retried every tick. The surviving duplicate makes a cleanly-evaluated window render ERROR permanently.

• packages/api/src/tasks/checkAlerts/providers/index.ts:108 — The two new optional trailing params on recordAlertErrors keep out-of-tree AlertProvider implementations compiling while silently disabling all ERROR-row persistence for them, with no compile error, runtime warning, or log.

  • Fix: Replace the trailing positional optionals with a single required options object, or split ERROR-row persistence into a distinctly named method so its absence is an explicit opt-in.
  • _{api-contract, maintainability}

  TypeScript structural typing accepts a 2-param implementation against the 4-param interface, and the call sites at index.ts:1051 and index.ts:1515 pass the window and analytics unconditionally. A provider registered through the ADDITIONAL_PROVIDERS extension point (providers/index.ts:125) therefore degrades to pre-PR behavior — executionErrors only, no evaluation history — which is the entire feature.

• packages/api/src/tasks/checkAlerts/errors.ts:38 — The constructor-name fallback in isClickHouseError, which exists specifically to handle two installed copies of @clickhouse/client-common, has no test coverage because every fixture in errors.test.ts constructs the real imported class so instanceof always short-circuits first.

  • Fix: Add a case in errors.test.ts using a locally declared class named ClickHouseError carrying type/code, and assert isQueryTimeoutError still classifies it as a timeout.
  • _{testing, kieran-typescript}

🔵 P3 nitpicks (6)

• packages/api/src/tasks/checkAlerts/providers/default.ts:491 — $set: { errors } fully replaces the row's error array, so a QUERY_TIMEOUT recorded on one tick is erased when a later tick in the same window records a webhook failure.

  • Fix: Merge into the existing array or document on upsertErrorHistory that the row holds only the most recent tick's errors.

• packages/api/src/tasks/checkAlerts/providers/default.ts:487 — The window key is exact Date equality derived from the alert's current interval/scheduleStartAt/scheduleOffsetMinutes, so editing any of those strands every existing ERROR row at a timestamp no future evaluation can produce.

  • Fix: Purge outstanding ERROR-state rows for an alert when its interval or schedule fields change.

• packages/api/src/tasks/checkAlerts/index.ts:1022 — The query-failure path populates only queryDurationMs, so every ERROR window renders backfilledBuckets: 0 even when dozens of windows are pending backfill.

  • Fix: Either omit backfilledBuckets on ERROR rows so the reader shows it as unknown rather than zero, or compute pending backfill from the last successful window.

• packages/api/src/tasks/checkAlerts/index.ts:1489 — evaluationAnalytics is a single mutable object aliased onto every history record in the batch, so any future per-record analytics field would silently write through to all siblings and to the ERROR row.

  • Fix: Assign a shallow copy per record with record.analytics = { ...evaluationAnalytics }.
  • _{maintainability, kieran-typescript}

• packages/api/src/tasks/checkAlerts/index.ts:1487 — The analytics-stamping loop plus updateAlertState call is duplicated verbatim in the single-value path at index.ts:1266 and the time-series path, so the two copies must be kept in sync by hand.

  • Fix: Extract a shared finalizeAlertHistories helper called from both paths.

• packages/api/src/tasks/checkAlerts/providers/default.ts:433 — The stale-row cleanup issues an extra AlertHistory.deleteOne on every successful evaluation of every alert on every tick, even though the overwhelming majority of ticks have no stale row.

  • Fix: Gate the delete on a signal already loaded this tick, such as a non-empty alert.executionErrors.
  • _{performance, reliability}

---

Reviewers (9): correctness, reliability, adversarial, testing, maintainability, project-standards, api-contract, kieran-typescript, performance.

Testing gaps:

• No test covers updateAlertState throwing after AlertHistory.create succeeded, which is the P1 above and would fail today.
• No test covers recordAlertErrors rejecting inside the query-failure path and asserting the QUERY_TIMEOUT message survives.
• No test exercises the chartConfig == null or meta == null branches to pin down whether persisting nothing is the intended contract.
• The QUERY_TIMEOUT integration assertion matches /\d+s/ rather than an explicitly configured requestTimeout, so a wrong interpolated value would pass.
• webhookDurationMs is asserted only as expect.any(Number), leaving the cross-notification accumulation at index.ts:1169 unverified.
• The state: {$ne: ERROR} filters have no query-level test; they are covered only indirectly through full processAlert integration runs.
• No test seeds several consecutive ERROR rows ahead of a valid row for one (alert, group) to exercise the $group/$first path under the new filter.

Environment note: Bash, Grep, and Glob all failed in this runner (bwrap: Can't create file at /home/.mcp.json), so git diff could not be computed. Scope was reconstructed by reading the files directly; the PR is a single commit and findings are anchored to lines verified by reading. Two items could not be checked as a result: whether a .changeset/ entry exists (AGENTS.md requires one for user-facing behavior changes to @hyperdx/api), and whether the out-of-tree enterprise AlertProvider has been updated for the recordAlertErrors signature change.

Suppressed below the confidence bar: a claim that $ne: AlertState.ERROR defeats the index-backed $group/$first short-circuit during outages, a claim that isClickHouseError's predicate lies about the narrowed shape, and an unchecked NodeJS.ErrnoException cast at errors.ts:62. Separately, clickhouseClient.requestTimeoutMs was flagged as a possible NaN source and then independently cleared by two reviewers — it is a real public getter with a 3600000 default (common-utils/src/clickhouse/index.ts:675), so the timeout message cannot render as NaNs in production.

[wrn14897]

Closing: per the decision to keep a single PR, this branch was fast-forwarded into #2786's branch (warren/HDX-4997-alert-error-history), so #2786 now contains the entire feature — detail page, evaluations read model, and this PR's error/analytics persistence. The diff here is now empty. All review-thread fixes (cause-chain timeout classification, stale ERROR-row cleanup, gap-tolerant pagination) are included in #2786.