[teeohhem]

Summary

Tile alerts can only express a static threshold, and the chart editor hard-codes that in three places: the alert's condition row, the threshold lines on the preview chart, and saving. This adds an extension point for other detection strategies, the UI counterpart to the backend's AlertEvaluator (#3576). With no extension registered, the editor renders and saves exactly as before.

AlertUIExtension (components/alerts/alertUIExtension.tsx) is read from a new @/extensions module, which supplies none here. Every part is optional:

• useConditionEditor (used by TileAlertEditor): a header, the condition fields, the labels, the interval options and a footer for the condition row. The threshold controls move, unchanged, into ThresholdConditionFields, which renders when no condition fields are supplied.
• useTimeChartPreview (used by ChartPreviewPanel): replaces the previewed config, adds DBTimeChart props, hides the threshold reference lines, and adds content below the chart.
• useChartEditor (used by EditTimeChartForm): beforeSave, which can stop a save, and normalizeAlert, which is applied to a builder chart's saved alert. Callbacks must keep their identity across renders.
• Provider: wraps EditTimeChartForm, applied in the folder's index.ts, so the hooks above can share state.

How to test locally or on Vercel

1. cd packages/app && yarn ci:unit src/components/DBEditTimeChartForm src/components/ChartEditor src/components/alerts. The new *.extension.test.tsx files check:

   • that the condition row and preview use what an extension supplies
   • that beforeSave blocks a save and normalizeAlert shapes the saved alert
   • that the provider wraps the editor

   The existing suites cover the unchanged no-extension path.

2. Manual: open a dashboard tile, add an alert, and check that the threshold row, interval and preview reference lines look and behave as on main.

Screenshots

No visual change is intended. I haven't captured before/after screenshots, because I had no browser available; please add them if you can.

🤖 Generated with Claude Code

[vercel]

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

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Nov 11, 2026 6:43am UTC
hyperdx-storybook	Ready	Preview	Nov 11, 2026 6:43am UTC

[changeset-bot]

🦋 Changeset detected

Latest commit: 66565f3

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

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/app	Patch
@hyperdx/api	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:

• Diff size: 348 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: 7
• Production lines changed: 348 (+ 577 in test files, excluded from tier calculation)
• Branch: claude/alert-condition-ui
• Author: teeohhem

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]

Duplicate code analysis

No new duplicate code introduced by this PR.

---

jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.20%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

5 finding(s): 🔴 0 critical · 🟠 1 major · 🔵 4 minor

1 posted as inline comment(s) on the changed lines. 4 listed below.

Findings outside the changed lines

• 🟠 packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx:727 — beforeSave is skipped on the inline-alert save path (handleSaveAlert) → handleSave calls beforeSave before saving, but handleSaveAlert does not. handleSaveAlert is the panel's save action whenever onSaveAlert is set, which includes EditInlineAlertModal (components/alerts/EditInlineAlertModal.tsx). An extension that blocks bad alert configurations can therefore be bypassed there. Fix: put the beforeSave(form) check at the top of handleSaveAlert as well, add beforeSave to its deps, and add a test for the inline path to DBEditTimeChartForm.extension.test.tsx. Alternatively, move the check into validateAndNormalize so every save path gets it.

3 minor

• 🔵 packages/app/src/components/alerts/alertUIExtension.tsx:1023 — timeChartProps overlaps with config and hideThresholdLines, so there are two ways to set the same props → timeChartProps is typed Partial<ComponentProps<typeof DBTimeChart>>, so it can also carry config, referenceLines and referenceLineValues. Because ChartPreviewPanel.tsx:337 spreads it last, an extension can silently override config or bring back the lines that hideThresholdLines hid. Fix: type it as Omit<..., 'config' | 'referenceLines' | 'referenceLineValues'>, or drop the dedicated fields and keep only timeChartProps.
• 🔵 packages/app/src/components/DBEditTimeChartForm/TileAlertEditor.tsx:263 — Threshold-only equality note still shows when an extension replaces the threshold fields → The 'Floating-point query results are not rounded during equality comparison' note still depends only on alert.thresholdType. If an extension supplies conditionFields for a non-threshold mode and the form still holds thresholdType EQUAL or NOT_EQUAL (for example, from before the mode was switched), the note appears for a mode that has no equality comparison. Fix: render the note only when conditionUI.conditionFields == null, or move it into ThresholdConditionFields.
• 🔵 packages/app/src/components/alerts/alertUIExtension.tsx:1034 — The normalizeAlert generic signature <T>(alert: T) => T forces implementations to cast → An unconstrained generic identity signature means any implementation that actually changes the alert (like the test's raiseThreshold) must cast to T or return the input unchanged. This goes against the repo's 'Avoid as casts' rule. Fix: type it on the real value, e.g. (alert: SavedChartConfig['alert']) => SavedChartConfig['alert'], which is the type of what normalizeNoOpAlertScheduleFields returns at the call site in EditTimeChartForm.tsx.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

[github-actions]

Deep Review

✅ No critical issues found. With no extension registered (this build's default — @/extensions supplies undefined), the editor renders and saves exactly as on main. I verified the two highest-risk paths against the base SHA: ChartPreviewPanel's referenceLines/referenceLineValues gating reproduces the prior render when hideThresholdLines is undefined, and EditTimeChartForm's beforeSave/normalizeAlert defaults (allowSave → true, keepAlert → identity) leave the save path unchanged. The threshold-control extraction into ThresholdConditionFields is a faithful move with all prior imports still in use.

🟡 P2 — recommended

• packages/app/src/components/alerts/alertUIExtension.tsx:62 — The extension hooks receive react-hook-form internals (Control<ChartEditorFormState>, UseFormSetValue/Watch/GetValues) and ComponentProps<typeof DBTimeChart>, binding the @public contract to internal form-state and component-prop shapes that any future refactor will silently break for implementers.
  • Fix: Narrow the public surface to a stable, intention-revealing shape (e.g. expose only the alert fields and setters an extension needs) rather than passing raw form/component internals.

🔵 P3 nitpicks (3)

• packages/app/src/components/alerts/alertUIExtension.tsx:51 — normalizeAlert?: <T>(alert: T) => T advertises an identity transform for any T, but it is only ever applied to the saved alert object, so the generic over-promises and misleads implementers about what may be returned.
  • Fix: Type the parameter and return as the concrete saved-alert type instead of a free generic T.
• packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx:337 — {...alertPreview.timeChartProps} is spread after the explicit props, so a supplied extension can silently override sourceId, config, and referenceLines; harmless for a trusted in-tree seam but an easy future footgun.
  • Fix: Spread extension props before the invariant props, or omit the keys the editor must own.
• packages/app/src/components/alerts/alertUIExtension.tsx:45 — The "callbacks must keep their identity across renders" requirement for beforeSave/normalizeAlert is only documented in a comment, yet both feed useCallback/useMemo dependency arrays; an extension returning fresh callbacks each render would churn the memoized save path with no guardrail.
  • Fix: Note the constraint at the hook-return site and consider wrapping the resolved callbacks in a stable ref so a non-conforming extension cannot cause re-render loops.

---

Reviewers: maintainability (completed), plus orchestrator-level correctness/adversarial verification of the no-extension path. The remaining dispatched reviewers (correctness, testing, project-standards, kieran-typescript, julik-frontend-races, adversarial, api-contract, agent-native, learnings) did not return before output was required; the findings above reflect the completed maintainability sub-review and the orchestrator's own diff analysis.

Testing gaps: New *.extension.test.tsx suites exercise the extension-injected path (condition row, preview, beforeSave/normalizeAlert, Provider wrap); the no-extension equivalence path relies on the pre-existing suites rather than an added assertion — acceptable given the default build ships no extension, but worth a quick confirmation that those suites still green.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[github-actions]

E2E Test Results

✅ All tests passed • 466 passed • 1 skipped • 1136s

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

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🔵 minor — Narrowed intervalOptions are not reconciled with the form's current interval

If the extension passes intervalOptions that leave out the alert's current interval (the default is '5m'; an existing alert might be '1m'), the NativeSelect shows a different option than the one stored, and the hidden value is what gets saved. Fix: when the current alert.interval is not a key of conditionUI.intervalOptions, reset it to the first allowed key with setValue, or document that the extension must do this and enforce it in the hook wrapper.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[jordan-simonovski]

potentially blocking here, seems handleSaveAlert doesn't call beforeSave

This check only runs in handleSave. The edit-alert modal saves through handleSaveAlert a little further down, so an extension that blocks a save there would still see the alert saved. Would something like this make sense?

const handleSaveAlert = useCallback(
  (form: ChartEditorFormState) => {
    if (!beforeSave(form)) return;