[HDX-5077] Alert-task coverage for multi-series metric tiles

[wrn14897]

Why

Follow-up to #2859 (composed multi-series metric query). That PR verified the existing 268 checkAlerts int tests pass unchanged, but the alert suite had zero multi-series metric tile coverage, and the refactor intentionally changed two null/NaN edge cases in how alert evaluation sees multi-series results. This PR pins those semantics with tests written against the shipped implementation.

Behavior deltas pinned (introduced by #2859, both fixes over the old node-side merge)

1. Gap in the last series at a bucket: the old mergeResultSets produced a row with the missing series' key simply absent, so parseAlertData's "last value column wins" silently fell back to an earlier series' value — an arbitrary evaluation. The composed query pivots the gap to an explicit NULL, which the null guard in processAlert skips.
2. Ratio with a zero denominator: the old node-side computeRatio produced a JS NaN that slipped past the null guard (NaN != null) and pushed a NaN count into alert history. The composed query divides to SQL NULL, which the guard skips — exactly what its own comment said it was for.

What

• New int tests (first metric multi-series coverage in checkAlerts.int.test.ts):
  • TILE alert (metrics, multi-series) - threshold compares the last series value — fires on the last select item's value (6), not the first series' (100), and resolves next window
  • TILE alert (metrics, multi-series) - a gap in the last series is skipped, not backfilled from an earlier series — no ALERT from the first series' above-threshold value; single default OK history with no evaluated values
  • TILE alert (metrics, ratio) - a zero denominator yields NULL and is skipped without NaN history — no NaN ever lands in lastValues
• Comment-only production change: retire the stale TODO: we might want to fix the null value from the upstream (check if this is still needed) on the null guard — the NULL is now intentional composed-query semantics (gap / div-by-zero), documented as the contract. No logic change.

Test plan

• make dev-int FILE=checkAlerts.int — 164/164 pass (incl. the 3 new tests) against docker CH
• npx nx run @hyperdx/api:ci:lint green (at the 311-warning ratchet; new tests add zero warnings)
• tsc --noEmit clean

No changeset: tests + comment only.

Issues

Linear: HDX-5077

[vercel]

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

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

[changeset-bot]

⚠️ No Changeset found

Latest commit: f236c2c

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

[github-actions]

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

• Standard feature/fix — introduces new logic or modifies core functionality

Additional context: touches background tasks or the delivery pipeline lightly (5 lines, under the 30-line bar for Tier 4)

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats

• Production files changed: 1
• Production lines changed: 5 (+ 279 in test files, excluded from tier calculation)
• Branch: warren/hdx-5077-checkalerts-metric-tile-tests
• Author: wrn14897

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

[greptile-apps]

Greptile Summary

This PR adds integration coverage for multi-series metric-tile alert evaluation while documenting the existing intentional NULL-handling contract.

• Verifies that alert thresholds use the last selected series.
• Verifies that missing final-series values are skipped rather than backfilled.
• Verifies that zero-denominator ratios do not create NaN alert history.
• Replaces a stale TODO with an explanation of the existing null guard.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/api/src/tasks/checkAlerts/tests/checkAlerts.int.test.ts	Adds isolated integration tests covering last-series selection, missing-series gaps, and zero-denominator ratios.
packages/api/src/tasks/checkAlerts/index.ts	Updates only the explanatory comment around the unchanged null-value guard.

_{Reviews (3): Last reviewed commit: "Merge branch 'main' into warren/hdx-5077..." | Re-trigger Greptile}

[github-actions]

E2E Test Results

✅ All tests passed • 293 passed • 1 skipped • 1150s

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

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Deep Review

Scope: packages/api/src/tasks/checkAlerts/index.ts (comment-only) and packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts (3 new integration tests + gaugePoint/setupMetricTileAlert helpers), diffed against 69a89aa9.

Intent: Add the first multi-series metric-tile coverage to the alert suite and document the null-guard's contract. The three tests trace cleanly against processAlert — last-series value drives evaluation, a last-series gap pivots to SQL NULL and is skipped, and a zero-denominator ratio yields NULL (never NaN) in history. No production behavior changes.

✅ No critical issues found.

🟡 P2 -- recommended

• packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts:6725 -- setupMetricTileAlert installs a persistent jest.spyOn(slack, 'postMessageToWebhook').mockResolvedValue(...) that is never restored, and the suite's clearAllMocks clears call records but not implementations, so this becomes the default postMessageToWebhook behavior for every later test in the file.
  • Fix: Restore the spy in a local afterEach (mockRestore()), or scope the mock so it does not persist past these three tests.
• packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts:6867 -- the suite pins that a gap in the last series is skipped but never covers the complementary case where an earlier series is NULL/absent and the last series has a value, leaving the PR's core claim ("only the last series' value is evaluated") unproven.
  • Fix: Add a test where the first series has no in-window row and the last series does, asserting the alert still evaluates off the last series' value.
  • _{correctness, testing, kieran-typescript}

🔵 P3 nitpicks (4)

• packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts:6716 -- select is typed as Array<Record<string, string>>, a permissive shape that lets a typo'd metricType/aggFn type-check silently.
  • Fix: Annotate with the real select-item type (e.g. DerivedColumn[] from @hyperdx/common-utils) instead of a local Record<string, string>.
  • _{maintainability, kieran-typescript}
• packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts:6921 -- ratio coverage exists only for the zero-denominator edge case; no happy-path ratio test verifies the computed value or threshold comparison.
  • Fix: Add a non-zero-denominator ratio test asserting the evaluated lastValues[0].count and ABOVE/BELOW behavior.
• packages/api/src/tasks/checkAlerts/index.ts:1369 -- the comment describes NULL as "a metric series with no row at this bucket" generally, but the skip only fires when the evaluated (last) series' value is NULL.
  • Fix: Note that value reflects the last value column, so the skip applies to a gap or zero-denominator in that series specifically.
• packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.int.test.ts:6842 -- const [history] = await AlertHistory.find(...) accesses history.state with no guard, surfacing an opaque "cannot read properties of undefined" if the record is absent.
  • Fix: Add expect(history).toBeDefined() (or a throw) before dereferencing for a clearer failure signal.

---

Reviewers (4): correctness, testing, maintainability, kieran-typescript.

Testing gaps:

• Multi-series metric tile combined with groupBy is untested (only single-series+groupBy and multi-series+empty-groupBy are covered).
• No coverage for sum/histogram metric types or aggFns other than max in the multi-series alert path.
• The "both series simultaneously NULL" case is untested (likely equivalent to the single-gap case, low priority).