[HDX-4997] Add alert detail page with evaluation history

[wrn14897]

Summary

Stack (2/3): #2797 (persist evaluation errors) ← this PR ← #2786 (evaluation event stream). Review/merge bottom-up.

Datadog-style alert status page at /alerts/:id, reachable via a new Details link on each alerts-page row (the alert name keeps linking to its saved search / dashboard tile). Gives the error history persisted in #2797 a home where users can actually investigate an alert:

• 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 — via AlertPreviewChart for saved-search alerts and the tile's chart config for dashboard-tile alerts.
• A widened evaluation-history strip (60 windows) reusing the alerts-page segments, including errored evaluations.
• An evaluation event stream: one row per window with state, latest value, breach count, and expandable error details, paginated via the /alerts/:id/evaluations endpoint from [HDX-4997] Persist alert evaluation errors and analytics in AlertHistory #2797.

Refinements to the event stream (time-range-driven pagination, per-group breakdown, analytics columns) land in #2786 on top of this.

Screenshots or video

Each alert now has a Details page with the query chart, history strip, and the full evaluation event stream.

How to test on Vercel preview

N/A — needs the alerting job + seeded alert data. Covered by full-stack Playwright tests instead.

How this was tested

• tsc --noEmit (app) and AlertEvaluationsTable unit tests green at this commit
• E2E: detail-page navigation + evaluation history spec, seeded through the fullstack global setup (make dev-e2e FILE=alerts)

References

• Linear Issue: HDX-4997

[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 12:33am
hyperdx-storybook	Ready	Preview	Aug 6, 2026 12:33am

[changeset-bot]

⚠️ No Changeset found

Latest commit: c4cc636

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[greptile-apps]

Greptile Summary

The PR adds a dedicated alert-detail route for inspecting alert state, source data, evaluation history, and persisted evaluation errors.

• Adds source-aware charts with alert annotations and threshold reference lines.
• Adds paginated evaluation history with expandable error details.
• Links alert-list rows to the new detail page and extends full-stack and component coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/app/src/AlertDetailPage.tsx	Adds the alert-detail page shell, source actions, time-range state, chart, history strip, and paginated evaluation table.
packages/app/src/components/alerts/AlertDetailChart.tsx	Builds source-specific chart configurations for saved-search and dashboard-tile alerts, with graceful fallbacks for unsupported sources.
packages/app/src/api.ts	Adds single-alert retrieval and cursor-based infinite querying for evaluation history.
packages/app/src/components/alerts/AlertEvaluationsTable.tsx	Adds the evaluation event-stream table with state badges, values, breach counts, pagination, and expandable errors.
packages/app/src/AlertsPage.tsx	Adds detail-page navigation while preserving links to each alert's underlying saved search or dashboard tile.
packages/app/tests/e2e/features/alerts.spec.ts	Extends full-stack coverage for errored history segments and alert-detail navigation.

Sequence Diagram

sequenceDiagram
  participant User
  participant Alerts as Alerts Page
  participant Detail as Alert Detail Page
  participant API as Alert API
  participant Chart as Chart Query
  User->>Alerts: Click Details
  Alerts->>Detail: Navigate to /alerts/:id
  Detail->>API: Fetch alert
  Detail->>API: Fetch paginated evaluations
  Detail->>API: Fetch alert-history annotations
  Detail->>Chart: Query source over selected range
  API-->>Detail: Alert and evaluation history
  Chart-->>Detail: Time-series data
  Detail-->>User: Chart, history strip, and event stream

Loading

_{Reviews (7): Last reviewed commit: "style(app): suppress new unsafe-type-ass..." | Re-trigger Greptile}

[github-actions]

<!-- deep-review -->

Deep Review

✅ No critical issues found.

_{Note: git, grep, and glob were unavailable in this environment (sandbox failure), so the diff could not be computed directly. Scope was reconstructed from the two (app)-scoped commits on this branch, meaning review focused on packages/app; packages/api and packages/common-utils were read as base context only. Findings citing base files are flagged as such.}

🟡 P2 -- recommended

• packages/app/src/AlertDetailPage.tsx:250 -- The not-found branch and the body branch are independent conditions, so a failed background refetch that retains cached data renders the "Alert not found" card stacked directly above the fully-populated page.
  • Fix: Gate the empty state on !alert alone so it cannot render simultaneously with AlertDetailBody.
  • _adversarial
• packages/app/src/AlertDetailPage.tsx:126 -- isError is never read from api.useAlertEvaluations, so a 500 or 401 on the evaluations endpoint leaves evaluations empty and the table reports "No evaluations recorded yet" for an alert that has evaluated many times.
  • Fix: Destructure isError and pass it to AlertEvaluationsTable so fetch failures render an error state instead of the empty state.
  • _correctness
• packages/app/src/AlertDetailPage.tsx:224 -- router.query.alertId is empty on the first render of this statically-optimized dynamic route, and useAlert(undefined) is disabled so isLoading is false, making "Alert not found" the first paint on every direct load or refresh of a valid alert URL.
  • Fix: Include router.isReady in the readiness gate before choosing between skeleton, not-found, and body, matching the pattern in DBDashboardPage.tsx.
  • _{correctness, adversarial}
• packages/app/src/api.ts:243 -- getNextPageParam stops paging the moment the server reports hasMore: false, but the server derives that flag from a row count inside a lookback window of (limit + 1) intervals, so any history gap wider than that window ends the stream early and hides retained evaluations.
  • Fix: Derive hasMore from the existence of older rows for the alert rather than from the row count inside the fixed lookback window, or widen the lookback when a page returns short.
  • _{adversarial, correctness}
• packages/app/src/api.ts:231 -- The evaluations useInfiniteQuery sets neither staleTime nor maxPages while the app's QueryClient declares no defaults, so every window-focus event re-runs one aggregation per loaded page and the accumulated row array grows without bound across "Load older evaluations" clicks.
  • Fix: Set a staleTime around the alert interval and pass maxPages to bound both refetch cost and accumulated rows.
  • _{performance, adversarial}
• packages/app/src/AlertDetailPage.tsx:39 -- The detail page imports AlertNote, getAlertDisplayName, and getAlertSourceUrl from ./AlertsPage, pulling that page's entire module graph including nuqs and react-markdown into this route's bundle and coupling the page to an unrelated page's refactors.
  • Fix: Move the three shared helpers into utils/alerts.ts or a dedicated components/alerts module and import from there in both pages.
  • _{maintainability}
• packages/app/src/utils/alerts.ts:50 -- The new extendDateRangeToInterval has eight interval-specific branches feeding the detail-page chart range and zero unit coverage, so a wrong date-fns unit in any branch would ship silently.
  • Fix: Add unit tests covering the pass-through case and at least one widened case for representative intervals.
  • _testing
• packages/app/src/components/alerts/AlertHistoryCards.tsx:221 -- The new history, maxItems, and showErrorIndicator props are exactly the configuration the detail page uses, yet both existing tests render the component with only the alert prop, leaving the padding arithmetic and the slice/reverse ordering at the maxItems boundary untested.
  • Fix: Add cases rendering with an explicit history array, a maxItems override, and showErrorIndicator={false}, asserting padding count and card order.
  • _testing

🔵 P3 nitpicks (11)

• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:47 -- The state switch covers only ALERT, PENDING, ERROR, and OK, so DISABLED and INSUFFICIENT_DATA fall through to a default that renders the raw enum string in the header badge.
  • Fix: Add explicit cases for AlertState.DISABLED and AlertState.INSUFFICIENT_DATA with human-readable labels.
• packages/app/src/components/alerts/AlertHistoryCards.tsx:143 -- The strip's error affordance keys off state === ERROR while the table keys off errors.length > 0, so an ERROR window with no persisted errors renders a card promising "Click for details" that opens an empty modal labelled "Multiple Errors" by errorTypeSummary([]).
  • Fix: Key both surfaces off the same predicate and render an explicit "no error details recorded" message instead of an empty modal body.
• packages/app/src/AlertDetailPage.tsx:121 -- extendDateRangeToInterval silently replaces any selection narrower than the interval floor while both charts pass showDateRangeIndicator={false}, so selecting "Past 1h" on a 1h-interval alert charts 16 hours with nothing on screen revealing it.
  • Fix: Apply the floor only to the initial default range, or re-enable the chart's date-range indicator so the effective window is visible.
• packages/app/src/AlertDetailPage.tsx:174 -- useAlertEvaluations receives no date range, so the history strip and event stream always show the newest windows while the chart above them honours the picker, letting a past-incident view pair yesterday's spike with today's all-green windows.
  • Fix: Pass the selected range into the evaluations query, or label those two sections as showing the most recent windows.
• packages/app/src/components/alerts/AlertDetailChart.tsx:87 -- SavedSearchAlertChart never receives alertUrl so its fallback has no "Open source" link, and a deleted dashboard, missing tile, or deleted source all funnel into the tile fallback's "This tile type can't be previewed here" message.
  • Fix: Thread alertUrl into SavedSearchAlertChart and distinguish a missing dashboard/tile/source from an unsupported tile type before choosing the message.
  • _{correctness, maintainability}
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:56 -- latestValue returns the last element of a lastValues array that the server flattens across every group and sorts only by startTime, so for a group-by alert the "Latest Value" column shows whichever group happened to sort last while "Breaches" is a cross-group sum.
  • Fix: Render per-group values or a summary for alerts with groupBy, or relabel the column so one group's value is not presented as the window's value.
• packages/app/src/AlertDetailPage.tsx:51 -- defaultTimeRange is evaluated once at module scope, so a long-lived tab opens the page on a chart window that ended when the JS chunk first loaded.
  • Fix: Compute the default range in a mount-time initializer inside the component.
• packages/app/src/components/alerts/AlertHistoryCards.tsx:240 -- Both the items and paddingItems memos guard with a condition whose two branches are provably equivalent, doubling the code a reader must verify for identical output.
  • Fix: Replace them with unconditional history.slice(0, maxItems) and new Array(Math.max(0, maxItems - history.length)).fill(null).
• packages/app/src/AlertDetailPage.tsx:60 -- AlertProperties re-implements the threshold-label, notify-via, and created-by rendering that AlertDetails in AlertsPage.tsx already performs, including the identical label-map lookup and webhook-icon call.
  • Fix: Extract the shared alert-metadata rendering into one component used by both pages.
• packages/app/src/utils/alerts.ts:50 -- extendDateRangeToInterval hand-repeats the same eight interval-to-duration pairs already encoded in intervalToDateRange, with nothing enforcing that the two stay consistent.
  • Fix: Extract a single interval-to-minimum-window table and derive both functions from it.
• packages/api/src/mcp/tools/alerts/getAlert.ts:129 -- Humans can now page through evaluation windows in the UI, while the only agent-reachable path hardcodes a limit of 20 with no cursor, so an agent investigating a failure further back has no equivalent read path.
  • Fix: Add pagination parameters to the alert MCP tool backed by the same history query the evaluations endpoint uses.
  • _agent-native

---

Reviewers (8): correctness, testing, maintainability, project-standards, adversarial, kieran-typescript, performance, agent-native.

Testing gaps:

• AlertDetailPage.tsx and AlertDetailChart.tsx have no unit tests; the tile-config assembly's raw-SQL, builder, missing-source, and Number→Line branches are entirely uncovered.
• The e2e assertion that alert-detail-page is visible cannot fail for the right reason, because that wrapper div renders in the loading and "Alert not found" states too — which is why the first-paint regression above is invisible to CI.
• No test covers the evaluations error path, the isLoading skeleton branch, the multi-error-type label join, or an ERROR window whose errors field is absent.
• No test exercises fetchNextPage pagination, a short first page with hasMore: false, or a focus-triggered refetch after several "Load older" clicks.

_{Two reviewers disagreed on whether TanStack Query v5 replays stale infinite-query cursors on refetch; v5 recomputes page params from freshly fetched pages, so the proposed dropped-window scenario was dropped rather than reported.}

[github-actions]

E2E Test Results

✅ All tests passed • 272 passed • 1 skipped • 1033s

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

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

• Diff size: 833 production lines changed (Tier 2 max: < 250)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats

• Production files changed: 9
• Production lines changed: 833 (+ 211 in test files, excluded from tier calculation)
• Branch: warren/HDX-4997-alert-detail-page
• Author: wrn14897

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

[github-actions]

<!-- deep-review -->

Deep Review

⚠️ Partial run. bash is non-functional in this environment (bwrap: Can't create file at /home/.mcp.json: Permission denied, with and without the sandbox disabled), so git diff was unavailable and the review scope had to be reconstructed by reading the tree directly. Twelve reviewers were dispatched; only the security reviewer had returned by the reporting deadline. The findings below are limited to what was actually verified. Correctness, testing, maintainability, project-standards, kieran-typescript, adversarial, frontend-races, performance, reliability, agent-native, and learnings results are not included — treat the absence of findings in those dimensions as unmeasured, not clean.

✅ No critical issues found.

🟡 P2 -- recommended

• packages/app/src/AlertDetailPage.tsx:250 -- The detail route's unauthorized/not-found path renders an "Alert not found" empty state, but no test asserts that a 403 or 404 from GET /alerts/:id reaches it without leaking alert attributes, the chart, or evaluation rows.
  • Fix: Add a component test that mocks the alert fetch as 403 and as 404 and asserts the empty state renders while alert-evaluations-table and the chart are absent.
• packages/app/src/components/alerts/AlertHistoryCards.tsx:59 -- Persisted evaluation error text is rendered verbatim in a Code block and can carry fragments of the alert's SQL plus database and table names; the new 60-window strip and paginated table make that content substantially more discoverable to every team member than the previous inline cards did, without any role gate.
  • Fix: Confirm with product that no role is expected to be blind to query internals, and redact or gate the raw error body if that assumption does not hold.

🔵 P3 nitpicks (3)

• packages/app/src/components/alerts/__tests__/AlertEvaluationsTable.test.tsx:18 -- The error-expansion case uses only a benign message, so nothing pins evaluation errors as inert text; AlertNote on the sibling alerts page does run a markdown renderer, making the distinction between the two surfaces worth locking down.
  • Fix: Add a case whose error.message contains markup and a javascript: URL and assert it renders as escaped text with no element or anchor created.
• packages/app/src/AlertsPage.tsx:68 -- getAlertSourceUrl is now exported and feeds the detail page's source-link button, but no test constrains its return value to relative same-origin paths.
  • Fix: Add a unit test asserting the returned URL is relative for every AlertSource variant.
• packages/app/src/AlertDetailPage.tsx:224 -- router.query.alertId is interpolated into the request path unencoded; the server still validates it via objectIdSchema and there is no privilege gain, but the raw interpolation is needless.
  • Fix: Wrap the id in encodeURIComponent at both call sites in packages/app/src/api.ts.

---

Reviewers (1 of 12 returned): security. Dispatched but not returned before reporting: correctness, testing, maintainability, project-standards, kieran-typescript, adversarial, julik-frontend-races, performance, reliability, agent-native, learnings-researcher.

Testing gaps:

• No coverage of the detail route's 403/404 states.
• No hostile-content test for the evaluation error rendering path.
• No assertion that getAlertSourceUrl yields same-origin relative paths.
• Cursor pagination (useAlertEvaluations getNextPageParam), the AlertSource branch in AlertDetailChart, and the AlertHistoryCardList extraction were not assessed for coverage — those reviewers did not report.

[github-actions]

<!-- deep-review -->

Deep Review

_{Note: git was unavailable in this environment (sandbox failure on every shell invocation), so the change surface was reconstructed by import-following from pages/alerts/[alertId].tsx and verified file-by-file against the working tree rather than from a textual diff. Findings on modified files are scoped to the alert-detail feature code.}

🔴 P0/P1 -- must fix

• packages/app/src/AlertDetailPage.tsx:127 -- The evaluations query's error state is never read, so any 4xx/5xx/network failure collapses to [] and the page affirmatively reports "No evaluations recorded yet. Evaluations appear here after the alert runs." while also blanking the history strip.
  • Fix: Destructure isError/error from api.useAlertEvaluations, pass an error prop into AlertEvaluationsTable, and render a distinct load-failure state with a retry action instead of the empty-state copy.
  • _{correctness, adversarial, reliability, api-contract}
• packages/app/src/AlertDetailPage.tsx:226 -- pages/alerts/[alertId].tsx has no getStaticProps/getServerSideProps and _app.tsx has no getInitialProps, so the route is statically optimized and router.query is {} on the prerender and first client render, making alertId undefined, disabling useAlert (isLoading === false for a disabled v5 query), and painting the "Alert not found" card before any skeleton.
  • Fix: Treat "router not ready" as loading — render the skeleton when !router.isReady || isLoading, and gate the not-found EmptyState on router.isReady && !isLoading && (isError || !alert).
  • _{adversarial, correctness}

🟡 P2 -- recommended

• packages/app/src/AlertDetailPage.tsx:251 -- The not-found branch and the body branch are independent conditions, and React Query keeps data while setting status: 'error', so a failed background refetch renders "Alert not found / Back to alerts" stacked directly above the fully populated chart and evaluation stream.
  • Fix: Gate the EmptyState on !alert alone and surface isError with cached data as a non-destructive inline banner.
• packages/api/src/controllers/alertHistory.ts:100 -- getRecentAlertHistories bounds the scan with createdAt >= endTime - (limit + 1) * interval and hasMore is then derived from histories.length > limit inside that same window, so any evaluation gap wider than one page of wall-clock time makes hasMore false and permanently strands older history that still exists under the 30-day TTL.
  • Fix: Drop the $gte lookback when a before cursor is supplied and rely on $sort + $limit over the {alert, createdAt} index, or compute hasMore with a separate existence probe below the page tail.
  • _{correctness, adversarial, api-contract}
• packages/app/src/api.ts:231 -- useAlertEvaluations sets no staleTime, refetchOnWindowFocus, or maxPages and the app-level QueryClient defines no defaultOptions, so every tab refocus re-requests each loaded 100-row page sequentially and re-anchors page 1 to a new server-side new Date() while later pages keep their frozen before cursors.
  • Fix: Set staleTime and refetchOnWindowFocus: false on this hook, and pin the first page's upper bound with an explicit per-mount before anchor so refetching cannot shift which windows belong to which page.
  • _{reliability, adversarial}
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:169 -- Every fetched page is flattened into one array and rendered as an unvirtualized Table.Tbody with no maxPages cap, so repeated "Load older evaluations" clicks grow live Table.Tr rows — each holding its own useState — without bound.
  • Fix: Cap retained pages via maxPages on the infinite query, or virtualize the table body.
  • _{reliability, performance}
• packages/app/src/AlertDetailPage.tsx:187 -- AlertDetailChart, AlertHistoryCardList, and AlertEvaluationsTable are mounted with no error boundary, and neither withAppNav's PageWrapper nor _app.tsx provides one, so a render throw in any panel unmounts the whole page — unlike AckAlert in the same header, which self-wraps in @/components/Error/ErrorBoundary.
  • Fix: Wrap each of the three new panels in the existing ErrorBoundary, matching the pattern AckAlert already uses.
• packages/app/src/AlertDetailPage.tsx:52 -- The new no-unsafe-type-assertion suppression justifies casting away [Date | null, Date | null] on the claim that the label always resolves, but parseTimeQuery delegates to a chrono natural-language parse and every other relative label in the codebase uses the spelled-out Last 12 hours form rather than Past 12h, with no test pinning this string.
  • Fix: Use the non-nullable parseRelativeTimeQuery(ms('12h')) already exported from ./timeQuery, or add a runtime fallback instead of asserting away the nulls.
• packages/app/src/utils/alerts.ts:50 -- extendDateRangeToInterval is new logic with eight near-duplicate interval branches living in src/utils/, a directory carrying a jest coverageThreshold floor, and src/utils/__tests__/alerts.test.ts covers only normalizeNoOpAlertScheduleFields.
  • Fix: Add cases covering each interval's extend-vs-passthrough boundary at, just under, and just over the threshold, plus a range already wider than the floor.
  • _{testing, maintainability, kieran-typescript, correctness}
• packages/app/src/components/alerts/AlertHistoryCards.tsx:221 -- The new history/maxItems/showErrorIndicator props are the exact combination the detail page uses for its 60-window strip, but AlertHistoryCards.test.tsx never passes any of them, leaving the history override, the slice/pad math, and the indicator-suppression branch uncovered.
  • Fix: Add cases asserting the history prop overrides alert.history, that maxItems both truncates and pads, and that showErrorIndicator={false} hides the indicator despite non-empty executionErrors.
• packages/app/src/api.ts:242 -- getNextPageParam is the only thing preventing either an infinite refetch loop or a silently repeated page, and neither it nor the before cursor derivation has any test.
  • Fix: Add a hook-level test covering hasMore: true with data, hasMore: true with an empty data array, and hasMore: false.
  • _{testing, api-contract, kieran-typescript, correctness, adversarial}
• packages/app/src/AlertDetailPage.tsx:39 -- The new page imports AlertNote, getAlertDisplayName, and getAlertSourceUrl from ./AlertsPage, making one page module depend on a sibling page module's internals purely to share display helpers.
  • Fix: Move the three helpers into a shared module such as src/utils/alerts.ts and have both pages import from there.
  • _{maintainability, kieran-typescript}
• packages/app/src/AlertsPage.tsx:216 -- The branch adds a second state-to-badge mapping in AlertEvaluationsTable.tsx rather than consolidating with the inline switch here, and the two already disagree: this one handles DISABLED but not ERROR, the new one handles ERROR but not DISABLED.
  • Fix: Delete the inline switch and render the new AlertStateBadge here, extracting it to a shared location so there is one state-to-badge mapping.

🔵 P3 nitpicks (12)

• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:47 -- The default branch renders {state} verbatim, so a disabled alert's header badge on the new page reads DISABLED where the alerts list reads Disabled, and INSUFFICIENT_DATA leaks likewise.
  • Fix: Add explicit AlertState.DISABLED and AlertState.INSUFFICIENT_DATA cases and make the default render a neutral Unknown.
• packages/app/src/components/alerts/AlertDetailChart.tsx:224 -- config is undefined both for genuinely unsupported tile configs and for a deleted dashboard or tile, so a dangling alert reference is reported as "This tile type can't be previewed here."
  • Fix: Branch on !dashboard || !tile first with a message naming the missing tile, and keep the unsupported-type copy for the config case.
• packages/app/src/AlertDetailPage.tsx:114 -- defaultTimeRange is evaluated once at module import so a long-lived session charts a window anchored to first import, and showRelativeInterval is not passed to useNewTimeQuery, so its mount effect immediately replaces the Past 12h label with an absolute range string.
  • Fix: Compute the initial range per mount inside AlertDetailBody and pass showRelativeInterval so the relative label survives.
• packages/app/src/api.ts:234 -- alertId comes straight from router.query with no shape validation and is interpolated unencoded into the request path, so /alerts/..%2Fme yields /api/alerts/../me, which URL resolution normalizes to a different endpoint before the response is fed to the page.
  • Fix: Wrap the id in encodeURIComponent at both interpolation sites and validate the route segment against /^[a-f0-9]{24}$/ before enabling the queries.
• packages/app/src/AlertsPage.tsx:70 -- getAlertSourceUrl interpolates dashboardId and tileId into the href without encoding, and tile ids are only constrained by z.string().min(1), so an id containing # or & produces a link that highlights the wrong tile.
  • Fix: Wrap both interpolated ids in encodeURIComponent.
• packages/app/src/components/alerts/AlertDetailChart.tsx:89 -- The saved-search failure path renders ChartFallback without alertUrl, so it loses the "Open source" escape hatch that the tile and unknown-source fallbacks both provide.
  • Fix: Pass alertUrl through to this ChartFallback call for consistency with the other two sites.
• packages/app/src/components/alerts/__tests__/AlertHistoryCards.test.tsx:13 -- The suppression is justified as a "partial test fixture", but every required field of AlertsPageItem is present and the only type error is the raw string 'above' against the AlertThresholdType enum, so as unknown as disables checking across the whole fixture.
  • Fix: Use AlertThresholdType.ABOVE and drop both the double cast and the eslint-disable comment.
• packages/app/tests/e2e/features/alerts.spec.ts:752 -- The spec builds [data-testid="alert-evaluation-row"] inline instead of going through a page-object accessor, unlike the neighbouring getErrorHistorySegments usage.
  • Fix: Add a getEvaluationRows() accessor to tests/e2e/page-objects/AlertsPage.ts and call it from the spec.
• packages/app/src/components/alerts/AlertHistoryCards.tsx:303 -- The file is 303 lines after this branch's prop additions, past the 300-line ceiling stated in AGENTS.md Key Principles.
  • Fix: Move AlertErrorsIndicator and the dedupeAlertErrors/errorTypeSummary helpers into a sibling module.
• packages/app/src/components/alerts/AlertHistoryCards.tsx:156 -- styles.historyCardLink is referenced on both the errored-segment button and the segment anchor, but styles/AlertsPage.module.scss defines no such class, so the className resolves to undefined.
  • Fix: Add the missing .historyCardLink rule or drop the dead className references.
• packages/app/src/components/alerts/__tests__/AlertEvaluationsTable.test.tsx:11 -- Both fixtures set counts: 0 and every render passes isLoading={false}, so the counts > 0 breach-count branch and the Skeleton early return are never exercised.
  • Fix: Add a fixture with counts > 0 asserting the count renders, and a case with isLoading true asserting the skeleton.
• packages/api/src/routers/api/alerts.ts:168 -- The evaluation-history capability this page is built on lives only on the cookie-authenticated internal router; openapi.json documents no /evaluations path, so the External API still exposes only the single most-recent executionErrors.
  • Fix: File a follow-up to add an API-key-authenticated GET /api/v2/alerts/{id}/evaluations reusing getRecentAlertHistories and document it in openapi.json.

---

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

Testing gaps:

• No unit test drives AlertDetailPage at all — the router-not-ready render, the isError-with-cached-data render, and the loading-to-content transition are all uncovered, which is precisely where both P1 findings live.
• No test covers useAlertEvaluations pagination or the evaluations error path; the e2e spec asserts exactly two seeded rows and never clicks "Load older evaluations".
• AlertDetailChart's branch selection (saved-search vs tile vs neither) and its missing-savedSearch / missing-source / missing-tile paths are untested, and the e2e spec only exercises a saved-search alert.
• The static-export build was checked and is not at risk: two pre-existing dynamic pages-router routes already ship without getStaticPaths through the green clickhouse-static-build CI job.

[github-actions]

Reviewer note on tooling: Bash was unavailable for this entire run (sandbox init failure), so no git diff could be produced. Scope was reconstructed by reading files directly; findings in modified files were checked against surrounding code to avoid flagging untouched lines.

🔴 P0/P1 — must fix

• packages/api/src/controllers/alertHistory.ts:100 — The evaluations query derives its $gte floor from (limit + 1) × current interval, so with the default limit=100 the span is 102 intervals but hasMore needs 101 distinct windows inside it — a tolerance of exactly one missing evaluation.
  • Fix: When before is set, drop the $gte floor (or widen it to the 30-day TTL horizon) and derive hasMore from a separate existence probe for a window older than the last returned row.
  • _{correctness, adversarial, api-contract}
• packages/api/src/controllers/alertHistory.ts:102 — Because the floor is anchored to new Date() and the alert's current interval, an alert that stopped evaluating recently, is DISABLED, or had its interval edited 1d→5m via PUT /alerts/:id returns an empty first page, and the page then asserts "No evaluations recorded yet" for an alert with weeks of retained history.
  • Fix: Page the evaluations endpoint by createdAt alone with the TTL horizon as the only floor, so rows written under a previous cadence remain reachable.
  • _{adversarial, correctness, api-contract}
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:145 — AlertDetailPage.tsx:127-133 never destructures isError, and in react-query v5 isLoading is isPending && isFetching, so a failed evaluations request leaves isLoading=false with evaluations=[] and renders "No evaluations recorded yet. Evaluations appear here after the alert runs." for a 500 or network failure.
  • Fix: Thread isError/error from useAlertEvaluations into AlertEvaluationsTable and render a distinct error state with a retry instead of falling through to the empty-state branch.

🟡 P2 — recommended

• packages/app/src/AlertDetailPage.tsx:251 — router.query.alertId is undefined until the pages-router hydrates, which disables useAlert and makes isLoading false with alert undefined, so the "Alert not found" empty state renders on first paint of every deep link or refresh.
  • Fix: Gate the not-found branch on the isReady flag useNewTimeQuery already returns (or useRouter().isReady) and treat alertId == null as loading.
  • _{correctness, adversarial}
• packages/app/src/AlertDetailPage.tsx:251 — isError is collapsed with !alert, so a 403 from a stale session, a 500 from a Mongo outage, and a genuinely deleted alert all render the same permanent "Alert not found" card with no retry affordance.
  • Fix: Branch on the ky HTTPError's response.status to distinguish 404 from 403 and 5xx, and offer a retry for transient failures.
  • _{reliability, api-contract}
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:56 — latestValue returns lastValues[lastValues.length - 1].count, but the controller flattens lastValues across every group-by group and sorts only by startTime, which is identical for all groups in a window, so "Latest Value" is an arbitrary group's number.
  • Fix: For alerts with groupBy set, show the maximum value among the newest startTime entries or render – rather than an arbitrary group's count.
• packages/app/src/AlertDetailPage.tsx:122 — extendDateRangeToInterval is applied unconditionally to whatever the TimePicker produced, so a 6h-interval alert charts 4 days for any selection under 4 days while showDateRangeIndicator={false} hides the substitution and the input keeps showing the user's chosen range.
  • Fix: Widen only the untouched default range, or surface the effective range via the chart's date-range indicator.
• packages/app/src/utils/alerts.ts:50 — extendDateRangeToInterval adds eight untested interval branches in src/utils/, the one directory carrying an enforced Jest branch-coverage floor in packages/app/jest.config.js.
  • Fix: Add a describe('extendDateRangeToInterval') block in packages/app/src/utils/__tests__/alerts.test.ts covering each interval both narrower than and wider than its minimum window.
• packages/api/src/controllers/alertHistory.ts:110 — $group runs before $limit and $pushes every group's lastValues and errors, so per-request cost scales with windows × groups × buckets while limit is caller-controlled to 200 with no span clamp — unlike the sibling /:id/history handler, which deliberately clamps to 31 days.
  • Fix: Project only the fields the UI reads before grouping, $slice the pushed arrays, and add an explicit span clamp matching MAX_HISTORY_SPAN_MS.
  • _{adversarial, performance}
• packages/app/src/api.ts:231 — useAlertEvaluations sets neither maxPages nor staleTime, and the app's QueryClient is built with no defaultOptions, so after N "Load older evaluations" clicks every window focus replays N sequential aggregations.
  • Fix: Pass maxPages and a non-zero staleTime to useInfiniteQuery, and consider refetchOnWindowFocus: false for this query.
• packages/app/src/AlertDetailPage.tsx:212 — onLoadMore={() => fetchNextPage()} discards the result and the app wires query errors only to console.error, so a failed page fetch silently re-enables the button with no user-visible feedback while hasNextPage stays true.
  • Fix: Surface the evaluations query's error state as an inline message or notification when a page fetch fails.
• packages/app/src/components/alerts/AlertDetailChart.tsx:142 — TileAlertChart's tile-to-ChartConfigWithDateRange assembly is a field-for-field copy of the dashboard tile's config wiring, including the pick(source, [...]) list and the isLogSource/isTraceSource conditionals, with only a comment noting it "mirrors" the original.
  • Fix: Extract the shared field wiring into one exported buildTileChartConfig(tileConfig, source, dateRange, granularity, options) used by both call sites, passing the two intentional differences as options.
• .changeset/ — AGENTS.md item 5 requires a changeset for any user-facing change to a published package, and .changeset/config.json lists @hyperdx/app and @hyperdx/api in its fixed group; this PR adds a page and two endpoints, and no changeset could be confirmed for it.
  • Fix: Run yarn changeset and commit an entry describing the new alert detail page and the two new endpoints.

🔵 P3 nitpicks (11)

• packages/app/src/components/alerts/AlertHistoryCards.tsx:235 — const history = historyProp ?? alert.history never falls back because [] ?? x is [], so the strip renders nothing while evaluations load even though GET /:id already returned 20 windows.
  • Fix: Use historyProp?.length ? historyProp : alert.history.
• packages/app/src/AlertDetailPage.tsx:114 — showRelativeInterval is not passed, so the hook's effect overwrites the Past 12h label with an absolute date-range string on mount, making initialDisplayValue dead.
  • Fix: Pass showRelativeInterval: true, or drop the unused initialDisplayValue.
• packages/app/src/AlertDetailPage.tsx:52 — defaultTimeRange is resolved once at module evaluation, so in a long-lived tab the chart plots a 12h window that ended when the bundle loaded.
  • Fix: Compute the default inside the component with useMemo so the relative range resolves at mount.
• packages/app/src/components/alerts/AlertDetailChart.tsx:224 — A failed useDashboards() leaves isDashboardsLoading false with dashboards undefined, so a fetch failure and a deleted tile both render "This tile type can't be previewed here."
  • Fix: Branch on useDashboards().isError and on tile == null to emit distinct messages.
  • _{correctness, reliability}
• packages/app/src/components/alerts/AlertDetailChart.tsx:87 — The saved-search fallback omits alertUrl that the tile branch passes, and cannot distinguish a deleted saved search from a failed fetch.
  • Fix: Pass alertUrl through and special-case alert.savedSearchId == null.
• packages/app/src/AlertDetailPage.tsx:133 — The header TimePicker scopes only the chart; useAlertEvaluations takes no range, so the table and strip always show the newest N windows.
  • Fix: Scope the TimePicker visually to the chart until range-driven pagination lands.
  • _{correctness, adversarial}
• packages/app/src/components/alerts/AlertEvaluationsTable.tsx:25 — This stateBadge handles ERROR but falls through to a gray badge rendering the raw enum text for DISABLED and INSUFFICIENT_DATA, while the alerts list's inline badges handle DISABLED but not ERROR.
  • Fix: Export one AlertStateBadge covering every AlertState and use it from both call sites.
• packages/app/src/AlertDetailPage.tsx:52 — The new as [Date, Date] assertion and its lint suppression are avoidable; packages/app/src/timeQuery.ts already has a parseValidTimeRange helper returning [Date, Date] | undefined, and agent_docs/code_style.md says to avoid as casts.
  • Fix: Export and use parseValidTimeRange instead of asserting.
• packages/app/src/AlertDetailPage.tsx:38 — getAlertDisplayName, getAlertSourceUrl, and AlertNote are newly exported from a page module purely so the detail page can import them, despite utils/alerts.ts and components/alerts/ already existing for this.
  • Fix: Move the two pure helpers to utils/alerts.ts and AlertNote to components/alerts/.
• packages/app/src/utils/alerts.ts:20 — intervalToGranularity, intervalToDateRange, and the new extendDateRangeToInterval each re-enumerate the same eight intervals with a silent default, so a ninth interval compiles cleanly and misbehaves three different ways.
  • Fix: Drive all three from one Record<AlertInterval, …> table so a missing key is a compile error.
• packages/api/src/routers/api/alerts.ts:145 — GET /:id computes and serializes a 20-window history aggregation that the detail page always discards, since it passes history={evaluations} instead.
  • Fix: Make the inline history optional on GET /:id so the detail page does not pay for it.

---

Reviewers (11): correctness, adversarial, testing, maintainability, project-standards, api-contract, performance, security, reliability, agent-native, learnings-researcher. The TypeScript and frontend-races personas did not return before synthesis; their highest-value items (the avoidable as assertion, router readiness, and referential stability of searchedTimeRange) were verified directly instead.

Verified clean — no findings: Team scoping on both new endpoints is enforced in the Mongo filter itself (getAlertEnhanced and getAlertById both query {_id, team}), so there is no IDOR; objectIdSchema and z.coerce.number() reject object- and array-valued params before they reach Mongo; alert notes render through react-markdown v10 with no rehype-raw, and error messages render as escaped React children; webhook error text is replaced with fixed strings server-side. Express route ordering, duplicate React keys across pages, infinite pagination loops, and cross-alert cache leakage were each investigated and refuted.

Testing gaps:

• No API-side test exists for GET /:id or GET /:id/evaluations — the limit/before/hasMore contract and the cross-team 404 are entirely unverified, and the test files an earlier pass cited do not exist in the tree.
• No test covers getRecentAlertHistories with before set, in particular that a series with two or more missing windows still reports hasMore: true.
• AlertHistoryCards.test.tsx exercises only the default alert.history path; the new history, maxItems, and showErrorIndicator props the detail page actually uses are untested.
• AlertEvaluationsTable.test.tsx never covers isLoading, isFetchingNextPage, multiple error types on one row, or two appended pages.
• Component-level tests for AlertDetailPage and AlertDetailChart are a real gap but explicitly outside the project's stated coverage policy in packages/app/jest.config.js, so they are noted rather than filed as findings.

[wrn14897]

Closing: this PR's content has been folded into #2786 together with the evaluation event stream work, per the restructure to two independent PRs (#2786 merges first with the detail page + read side; #2797 follows with the alert-task write side). No review comments existed here.