fix: align alert chart markers with the evaluated bucket start

[wrn14897]

On the alert details page, firing/recovery markers were drawn one bucket to the right of the data they belong to: the latest OK marker sat at 7:30 while both the plotted data point and the history table's Evaluation Window showed 7:29. Markers now land on the start of the newest evaluated bucket, lining up with the chart and the table.

The transitions endpoint now emits bucketStart — the newest lastValues.startTime across the window's rows (including group-by rows), falling back to createdAt − interval for windows without lastValues, the same fallback the table uses. The client draws at bucketStart ?? createdAt, so older API responses keep working. Deriving the time server-side from lastValues (rather than shifting by the alert interval client-side) keeps markers correct when evaluations bucket finer than the interval. Dashboard tile alert annotations use the same endpoint, so they shift consistently.

Validated with new integration tests (newest-bucket derivation, empty-lastValues fallback, group-by max, carry-in pin at range start) and unit tests for the annotation mapping; full unit suites and the alertHistory integration suite pass.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview	Aug 17, 2026 5:41pm
hyperdx-storybook	Ready	Preview	Aug 17, 2026 5:41pm

[changeset-bot]

🦋 Changeset detected

Latest commit: d66c936

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	Patch
@hyperdx/api	Patch
@hyperdx/app	Patch
@hyperdx/otel-collector	Patch

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 3 — Standard

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

Why this tier:

• Cross-layer change: touches frontend (packages/app) + backend (packages/api) + shared utils (packages/common-utils)

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

Stats

• Production files changed: 3
• Production lines changed: 110 (+ 255 in test files, excluded from tier calculation)
• Branch: warren/fix-markers-in-alert-details-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]

E2E Test Results

✅ All tests passed • 296 passed • 1 skipped • 969s

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

Tests ran across 4 shards in parallel.

View full report →

[greptile-apps]

Greptile Summary

The PR aligns alert transition annotations with the newest evaluated bucket while retaining compatibility with older transition payloads.

• Derives and returns bucketStart from alert-history evaluation data, with fallback and range-edge handling.
• Uses bucketStart for chart annotation placement and keys.
• Adds integration and unit coverage for grouped histories, recoveries, missing bucket values, carry-in behavior, and client fallback.
• Replaces untyped test-helper alert identifiers with the alert model’s Mongoose object-ID type.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/api/src/controllers/alertHistory.ts	Derives transition marker times from evaluated bucket starts, with empty-history fallback and range-start flooring.
packages/api/src/controllers/tests/alertHistory.int.test.ts	Adds broad integration coverage and correctly types alert identifiers as Mongoose object IDs.
packages/app/src/hooks/useAlertAnnotations.tsx	Maps annotations to bucketStart while preserving createdAt compatibility for older responses.
packages/app/src/hooks/tests/useAlertAnnotations.test.tsx	Covers bucket-based marker placement, fallback behavior, and key uniqueness.
packages/common-utils/src/types.ts	Extends the shared transition schema with validated timestamps and an optional bucketStart field.

Sequence Diagram

sequenceDiagram
  participant History as AlertHistory
  participant API as Transitions endpoint
  participant App as Annotation mapping
  participant Chart as Time chart
  History->>API: Evaluation rows and lastValues.startTime
  API->>API: Select newest bucket start
  API-->>App: createdAt, state, bucketStart
  App->>App: "time = bucketStart ?? createdAt"
  App-->>Chart: Firing/recovery annotation

Loading

_{Reviews (7): Last reviewed commit: "style: format newestBucketStart signatur..." | Re-trigger Greptile}

[github-actions]

Deep Review

✅ No critical issues found. No data loss, auth, injection, or happy-path crash is introduced; the z.string().datetime() tightening is inert at runtime (no .parse()/.safeParse() call site exists and the sole producer emits toISOString()). One correctness risk and several nits are worth a look before merge.

🟡 P2 -- recommended

• packages/app/src/hooks/useAlertAnnotations.tsx:26 -- markers are now plotted at bucketStart ?? createdAt, but transitions are only ordered by createdAt, so a later recovery whose derived bucketStart predates an earlier firing's bucketStart (non-monotonic buckets from backfilled/irregular evaluations or uneven group-by coverage) renders the green OK marker left of the red firing marker; the floor at range start guards only the carry-in edge, not ordering between in-range transitions.
  • Fix: clamp each emitted transition's bucketStart to be >= the previously emitted transition's bucketStart so drawn markers stay monotonic.
  • _{adversarial, correctness}

🔵 P3 nitpicks (5)

• packages/api/src/controllers/alertHistory.ts:544 -- when firing on entry, the carry-in ALERT pin and a first-in-range OK recovery both floor to startTime, stacking two opposite-state markers at the identical left-edge x (keys differ, so no drop -- purely visual).
  • Fix: nudge a floored in-range crossing to strictly after the carry-in pin so the firing→recovery pair stays visually distinct.
• packages/api/src/controllers/alertHistory.ts:565 -- the newest lastValues.startTime, else createdAt − interval fallback is reimplemented here independently of the evaluation-table path (AlertEvaluationRow.tsx), so the two can silently diverge and desync the chart marker from the table.
  • Fix: extract the shared derivation into a common helper and call it from both the transitions controller and the evaluation-row component.
  • _{maintainability, testing}
• packages/api/src/controllers/alertHistory.ts:542 -- the comment states the synthetic carry-in marker "carries no bucketStart of its own," but pinCarryInIfFiring unconditionally sets bucketStart: startTime.toISOString(), misleading a future reader auditing when the field is absent.
  • Fix: reword the comment to say bucketStart is pinned to startTime rather than omitted.
• packages/common-utils/src/types.ts:885 -- tightening createdAt from z.string() to z.string().datetime() is a behavior-adjacent schema change bundled into a marker fix; it is inert today but would reject any future non-Z ISO producer if a .parse() is later added.
  • Fix: confirm no other AlertTransition producer emits an offset-form timestamp, or scope the tightening to its own change.
  • _{correctness, api-contract, kieran-typescript, adversarial, project-standards}
• packages/api/src/controllers/alertHistory.ts:493 -- the bucketStarts: Date[][] aggregate generic asserts more than Mongo guarantees (dates-as-strings under a divergent engine would break startedAt > max), extending the file's pre-existing unchecked-cast pattern to data that now drives marker placement.
  • Fix: add an instanceof Date guard at the boundary before feeding values into newestBucketStart.

---

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

Testing gaps:

• No test for a single row's lastValues mixing valid and null/missing startTime entries (the claimed DocumentDB defensiveness path).
• No test for heterogeneous group-by windows where some rows omit lastValues entirely versus an empty array.
• No test asserting marker x-ordering when bucket starts are non-monotonic across in-range windows (the P2 above).
• No round-trip test parsing controller output through AlertTransitionSchema to confirm the tightened createdAt and optional bucketStart accept real payloads.

[wrn14897]

Re: Deep Review findings —

P2 — the new lastBucketStart: { $max: { $max: '$lastValues.startTime' } } uses a nested $max array-expression inside a $group accumulator … exercised only by the standard Mongo integration suite.

Addressed in 35e44a1: dropped the nested $max expression entirely and switched to this file's established DocumentDB-safe pattern — $push the whole lastValues arrays in the $group and derive the newest bucket start in JS (same approach as mapGroupedHistories, which exists for exactly this engine-compat reason). The aggregation now uses only $push, which the rest of this controller already exercises against DocumentDB in production.

P3 — extract the shared "newest bucket start, else createdAt − interval" derivation into a single common helper consumed by both the controller and the table row.

Declined: the two sites operate on different shapes in different contexts — the controller derives from grouped aggregation rows (Date objects, interval already resolved to ms) while AlertEvaluationRow derives from JSON-serialized API objects (string timestamps, interval via ALERT_INTERVAL_TO_MINUTES). The genuinely shareable portion is a one-line fallback, and a cross-package helper for that would be premature abstraction. Both sites carry comments pointing at each other's semantics instead.

P3 — add a case with an in-range fire and recovery that carry different bucketStart values and assert each annotation lands on its own bucket start.

Addressed in 35e44a1: added maps each transition to its own bucketStart to useAlertAnnotations.test.tsx.

[wrn14897]

Re: Deep Review round 2 — all four items addressed in 782ba43 (note: that round's prose reviewed the pre-35e44a11b head; the nested $max it discusses was already replaced with the $push-and-derive-in-JS pattern).

P2 — an in-range crossing … derives bucketStart = createdAt − interval, which lands before the visible range start; whether that marker renders (clamped vs dropped) is not verified.

Clamping was already verified: chartAnnotations.test.tsx covers "clamps a marker before the domain to the left edge" (positionAnnotations clamps into the visible domain by design). Added the integration case may emit a bucketStart before the range start for an edge crossing documenting the API contract, plus a note on the schema field.

P2 — never assert a recovery (OK) transition's bucketStart is derived from its window's lastValues.

Added derives a recovery's bucketStart from the recovering window's lastValues.

P3 — use z.string().datetime() for bucketStart (and consider aligning createdAt).

Done for both fields — the only producer emits toISOString().

P3 — assert that two same-bucketStart opposite-state transitions still produce distinct annotation keys.

Added keeps keys distinct for opposite-state transitions sharing a time (keys include the state).

[wrn14897]

Re: Deep Review round 3 —

P2 — Clamp the in-range bucketStart to max(bucketStart, startTime) so real-transition markers never precede the range start, matching the carry-in pin's behavior.

Declined, deliberately: bucketStart is a fact about the data (when the newest evaluated bucket started), and clamping belongs to presentation. The chart already clamps every marker into its actual rendered x-domain (positionAnnotations, covered by "clamps a marker before the domain to the left edge") — which is the correct clamp boundary. The API's startTime is not: the client bucket-floors the requested range before calling, so a server-side clamp would target a quantized approximation of the domain and bake a rendering decision into the data contract. The carry-in pin is not an asymmetry — that marker is synthetic ("already firing when the window opens"), so startTime is its definition rather than a clamp. This contract is documented on the schema field and pinned by the may emit a bucketStart before the range start for an edge crossing integration test.

(Also pushed badaee5 fixing the lint check failure — prettier formatting on the new helper.)

[wrn14897]

Re: Deep Review round 4 — both P2s addressed in d66c936.

P2 (adversarial) — a carried-in firing marker is pinned at startTime, but an immediate in-range recovery derives bucketStart = createdAt − interval, which falls before startTime … re-introducing the mis-ordering the pin exists to prevent.

Accepted — this supersedes my round-3 decline, which missed the carry-in interaction: response-internal ordering (recovery never before the pin it follows) is an API invariant, not chart presentation. bucketStart is now floored at startTime for all in-range transitions (only edge crossings can derive below it; consecutive real transitions are ≥ one interval apart, so they were already ordered). Covered by two new tests: floors bucketStart at the range start for an edge crossing and keeps a recovery after the carry-in pin it follows. Schema doc updated.

P2 (performance) — the transitions aggregation … now $pushes every row's lastValues array … multiplying buffered payload and heap.

Addressed by pushing only '$lastValues.startTime' (a plain field path — dates only, still no expression operators inside accumulators, so the DocumentDB-safe discipline holds) instead of whole lastValues rows. The scan itself stays bounded by the router's MAX_HISTORY_SPAN_MS (31d) clamp; unlike getAlertEvaluations, annotations must cover the whole visible range in one response, so pagination-style scan floors don't apply.