fix(heatmap): persist drag-select rectangle across re-renders

[alex-fedotyev]

Summary

The dashed selection rectangle on the Event Deltas heatmap collapses to a 2x2 px box the instant the user releases the mouse. The filter itself is applied correctly (URL gets xMin/xMax/yMin/yMax, the comparison legend shows "Selection" vs "Background", and the bar charts below split into two colors), but visually the user has no idea what slice of the heatmap they picked.

Repro on play-clickstack: open the Demo Traces source, switch to the Event Deltas tab, drag any region. Inspect the .u-select element after mouse-up: width=2 px, height=2 px, position pinned to top-left of the canvas.

Same component is used in the Search heatmap (DBSearchHeatmapChart), so this fix applies there too. Dashboard tile heatmaps don't pass onFilter, so they're not affected.

Linked: HDX-4147.

Root cause

uplot-react destroys and recreates the chart whenever its options prop reference changes and optionsUpdateState classifies the change as 'create' (any non-width/height top-level key is not Object.is-equal). In HeatmapContainer's Heatmap:

1. tickFormatter was a useCallback keyed on numberFormat reference. Callers (e.g. DBSearchHeatmapChart) build {output: 'duration', factor: 0.001} inline on every render, so numberFormat is a fresh ref each time.
2. New tickFormatter ref makes the options useMemo recompute, returning a fresh object whose series, axes, cursor, and plugins are all new array/object literals.
3. uplot-react sees a new options ref, walks the keys, classifies as 'create', destroys the old chart, builds a new one. uPlot's u.select lives on the chart instance, so it gets wiped along with the canvas.
4. The 2x2 px residue is uPlot's default zero-size selection element with its 1 px border on each side.

The earlier [time, bucket, count] memoization stabilized the data prop but left the options recreation cycle untouched, so the symptom persisted.

Fix

Two layers, one targeted at the recreation cycle, one accepting that recreation can happen for any reason and recovering from it.

1. Stabilize tickFormatter by hashing numberFormat with JSON.stringify instead of depending on its reference. Same content -> same hash -> same tickFormatter -> same options -> no recreate. The hashed-content pattern is correct here because the formatter only reads top-level scalar fields off numberFormat.

2. Treat the URL as the source of truth for the selection rectangle. DBSearchHeatmapChart already writes xMin/xMax/yMin/yMax to the URL via nuqs. I plumb those back down through a new selectionBounds prop on DBHeatmapChart -> Heatmap, and reapply via setSelect from uPlot's ready hook on every chart construction plus a useEffect for in-place bounds changes. Layer 2 is what actually keeps the rectangle alive on page reload, theme switch, resize bounce, or any future cause of recreation. The ready hook is needed (rather than onCreate) because mode-2 facet data leaves u.scales.y.min/max unpopulated until the first draw lands; reading them earlier returns undefined and the apply silently no-ops.

The bottom-bucket adjustment (yMin = 0 when the drag touched the floor of a log axis) round-trips correctly: applySelectionToChart clamps Math.log(yMin) to the chart's u.scales.y.min when yMin <= 0.

Test plan

• Bug reproduced on the Vercel preview build for this branch (drag-select on Demo Traces > Event Deltas, observed .u-select collapse to 2x2 px at the canvas origin).
• After the fix on a fresh Vercel build:
  • Drag-select renders a persistent dashed red rectangle: inline style left: 65px; top: 69px; width: 183px; height: 133px after mouseup, and stays visible.
  • Reloading the page with xMin/xMax/yMin/yMax in the URL restores the rectangle to the same coordinates the user dragged. uPlot's inline style is set via the ready hook.
  • Clicking on the chart away from the rectangle clears both the URL state and the rectangle (inline style goes to left: 0; top: 0; width: 0; height: 0).
• yarn workspace @hyperdx/app jest --testPathPatterns heatmap passes (1549 tests).
• yarn workspace @hyperdx/app run tsc --noEmit introduces no new errors in this diff (the 8 pre-existing errors in KubernetesDashboardPage, ServicesDashboardPage, SessionsPage, SourceForm, SourcesList, SourceSelect are also present on main).

[changeset-bot]

🦋 Changeset detected

Latest commit: 4132b5f

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

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	May 11, 2026 10:23pm

[github-actions]

🟡 Tier 3 — Standard

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

Why this tier:

• Diff size: 262 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: 2
• Production lines changed: 262 (+ 148 in test files, excluded from tier calculation)
• Branch: alex/HDX-4147-heatmap-select-persist
• Author: alex-fedotyev

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]

PR Review

✅ No critical issues found.

The fix is well-scoped and well-tested. Two-layer approach (stabilize tickFormatter via content hash + reapply selection from ready hook on every chart creation) addresses the root cause cleanly. Bounds round-trip correctly: seconds-to-ms x-conversion, Math.log for log-scale y, yMin=0 clamping to chart floor.

Minor observations (non-blocking):

• ⚠️ applySelectionToChart line 92: yMax > 0 ? ... : yScaleMax — when yMax is non-positive in log scale, fallback to yScaleMax (chart top) is the opposite of what you'd intuitively want, but this state shouldn't occur in practice (drag never produces yMax≤0). Not blocking; comment documents the intent.
• ⚠️ Tick-formatter stabilization assumes NumberFormat is JSON-serializable. The inline comment already calls out the failure mode if a function-valued field is added later — good documentation, but worth a follow-up test if anyone extends NumberFormat.
• ⚠️ useEffect at DBHeatmapChart.tsx:1041 runs every commit (no deps array) to mirror props into refs. Intentional and documented, but slightly unusual — the existing onFilterRef pattern just above uses a dep array. Consistency would be nicer.
• ℹ️ Test stub's default identity valToPos doesn't match real uPlot y-inversion semantics; tests work because they assert call arguments, not resulting heights. Fine for unit coverage.

Looks ready to merge.

[github-actions]

E2E Test Results

✅ All tests passed • 175 passed • 3 skipped • 1141s

Status	Count
✅ Passed	175
❌ Failed	0
⚠️ Flaky	3
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Compound Engineering Review

✅ No critical issues found. This is a well-engineered fix for the heatmap drag-select rectangle persistence bug. The imperative applySelectionToChart escape hatch is well-bounded, types are tight, and the commentary explains the load-bearing invariants (the numberFormatKey fingerprint, the clear-before-scale-guard ordering in applySelectionToChart, the seconds → ms x-axis convention). Suggestions below are optional polish.

P2 packages/app/src/components/DBHeatmapChart.tsx:80-83 — when bounds != null but u.scales.y.{min,max} are undefined, applySelectionToChart early-returns without clearing, so a stale rectangle from a prior render can persist while scales settle. Asymmetric with the null-bounds path that always clears. → either clear before the guard, or add a one-line comment that the stale-during-settle behavior is intentional and the ready hook will catch up.

P2 packages/app/src/components/DBHeatmapChart.tsx:1037-1041 — the dep-less useEffect that mirrors selectionBounds/scaleType into refs every commit lands after paint, so on a chart recreation the ready hook (which reads the refs) can briefly see stale values during the same commit. Bounds change is recovered one frame later by the bounds-effect, but it's avoidable. → assign refs inline during render (selectionBoundsRef.current = selectionBounds; scaleTypeRef.current = scaleType;), drop the effect entirely. Same semantics, no ordering question, no ESLint suppression needed.

P2 packages/app/src/components/DBHeatmapChart.tsx:1029 — uplotRef is set by uplot-react but never nulled on unmount. With key={JSON.stringify(config)} recreations, theoretically the bounds effect could fire against a torn-down instance during a recreate seam. → add useEffect(() => () => { uplotRef.current = null; }, []) and an if (!u.root?.isConnected) return; guard inside applySelectionToChart. Belt-and-braces; harmless today.

P2 packages/app/src/components/DBHeatmapChart.tsx:1102-1119 — numberFormatKey = JSON.stringify(numberFormat) works but the 13-line comment justifying it is itself a smell, and key-order divergence between callers could miss cache hits. The comment already flags the function-field risk. → consider a [output, mantissa, unit, ...].join('|') keyed on the known NumberFormat fields (confirmed plain primitives in packages/common-utils/src/types.ts). Cheaper, more robust, lets the comment shrink.

P2 packages/app/src/components/DBHeatmapChart.tsx JSDoc on applySelectionToChart (around line 38-43) — comment implies "renderer clamps to chart's visible y-axis floor" applies generally, but the floor-clamp is only the log-scale path; linear silently passes yMin through Math.max(yMin, yScaleMin) which can shrink the rectangle if yScaleMin > 0. Behavior is correct, doc is misleading. → tighten to "log scale clamps yMin=0 to axis floor; linear passes yMin literally".

P2 packages/app/src/components/DBHeatmapChart.tsx (bounds useEffect + ready hook) — on chart create with non-null bounds, both paths apply (ready then the bounds effect). Idempotent with fireHook=false, but each recreation costs two valToPos reads. → harmless; if you trim, prefer keeping ready (handles recreations) and dropping the bounds effect's mount-time apply via a useRef guard, or just leave it and add a one-line comment so the next maintainer doesn't "fix" one path.

Skipped as not actionable: the simplicity reviewer's suggestion to delete the memos on generatedTsBuckets / heatmapData — those are load-bearing for the fix (preventing uplot-react's recreate path that wipes u.select), and the author's comments correctly call this out.

[alex-fedotyev]

P3 nits addressed in 287183d (clarifying comments only, no behavioural change):

• Note the bounds-null clear path runs BEFORE the y-scale-not-populated guard so a future refactor doesn't swap the ordering and silently suppress clears on first paint
• Trailing comment on the deps-less ref-mirroring useEffect so it doesn't read as "missing deps" at a glance
• numberFormatKey: documented the JSON-serializability assumption with a note to switch to a shallow-equal helper if NumberFormat ever grows a function-valued field

[github-actions]

Deep Review

✅ No critical issues found. The clamp/conversion math in applySelectionToChart is sound, the ref-mirror invariant survives strict-mode and the lifecycle ordering (uPlot's ready reads refs that are either populated by useRef's initial value on first mount or by the previous commit's mirror effect on recreation), and the pure helper is exercised in isolation. Remaining items are test-coverage gaps for the React wiring and a few hygiene cleanups.

🟡 P2 — recommended

• packages/app/src/components/DBHeatmapChart.test.tsx:1 — Test sits beside its source while AGENTS.md:67 mandates __tests__/ directories and the sibling packages/app/src/components/__tests__/ already holds every other component test (DBTimeChart.test.tsx, DBHistogramChart.test.tsx, heatmapBuckets.test.ts, etc.).
  • Fix: Move the file to packages/app/src/components/__tests__/DBHeatmapChart.test.tsx and update the import to ../DBHeatmapChart.
  • _{project-standards}
• packages/app/src/components/DBHeatmapChart.tsx:1037-1060,1244-1250 — The pure helper is covered, but the React wiring that actually prevents the regression — options-memo stability under fresh-but-equal numberFormat, the ready-hook reapply on chart recreation, the [selectionBounds, scaleType, width, height] reapply useEffect, and the no-deps ref-mirror — has zero render-level coverage, so reverting any of those pieces would silently pass the existing 12 unit tests.
  • Fix: Add an RTL or render-harness test that mounts Heatmap, renders twice with structurally-equal-but-fresh numberFormat references, asserts the options reference is stable, then triggers a palette-driven recreation and asserts the dashed rectangle is re-applied.
  • _{testing, julik-frontend-races}
• packages/app/src/components/Search/DBSearchHeatmapChart.tsx:97-118 — The selectionBounds useMemo encodes a load-bearing "null when ANY of the four fields is null" contract with no test file for this component; a future refactor that swaps && for || or stops short-circuiting on a single null field would silently regress.
  • Fix: Add DBSearchHeatmapChart.test.tsx covering the all-null, all-set, and each-single-field-null branches of the selectionBounds memo.
  • _testing

🔵 P3 nitpicks (5)

• packages/app/src/components/DBHeatmapChart.tsx:1081-1084 — numberFormatKey collapses NaN, Infinity, undefined, and any function-valued field to the same hash, so a future addition to NumberFormat could conflate distinct configs and reopen the stale-tickFormatter failure mode the inline comment already warns about.
  • Fix: Hash the known scalar fields by template-string (e.g., ${unit}|${factor}|${output}|${mantissa}) or extract a typed shallow-equal helper so the contract is enforced by code rather than by comment.
  • _{correctness, maintainability, adversarial, kieran-typescript}
• packages/app/src/components/DBHeatmapChart.tsx:127-130 — Reversed xMin > xMax is covered, but reversed yMin > yMax is not, so the Math.max(0, yLowPx - yHighPx) clamp's behavior on inverted y bounds is unverified.
  • Fix: Add a test for yMin > yMax to lock either the swap-and-render or collapse-to-zero-height contract.
  • _{testing, kieran-typescript}
• packages/app/src/components/DBHeatmapChart.tsx:1037-1044,1247-1248 — scaleTypeRef mirrors a value that is already a dep of the options useMemo, so the ready hook's closure could read scaleType directly (the way the sibling setSelect hook at line 1263 already does); only selectionBoundsRef is structurally required because selectionBounds is intentionally not a memo dep.
  • Fix: Drop scaleTypeRef, read scaleType from the options-memo closure in the ready hook, and shrink the mirror useEffect to one assignment.
  • _{maintainability}
• packages/app/src/components/DBHeatmapChart.tsx:92 — Under log scale, yMax <= 0 short-circuits to yScaleMax (full chart top), which is asymmetric with the yMin <= 0 clamp-to-floor on the next line; the producer side cannot emit yMax <= 0 today but the branch exists and is untested.
  • Fix: Either treat yMax <= 0 as an invalid bounds (early return) or clamp it the same way as yMin; add a test case either way.
  • _{correctness, testing, adversarial}
• packages/app/src/components/DBHeatmapChart.tsx:1207 — The pre-existing // eslint-disable-next-line react-hooks/refs -- mouseInsideRef is read at event time directive above highlightDataPlugin was deleted while new sibling .current reads were added inside the same options useMemo's ready (line 1247-1248) and setSelect (line 1270-1271) hooks.
  • Fix: Run yarn lint packages/app/src/components/DBHeatmapChart.tsx; if react-hooks/refs still flags any of the in-closure reads, restore the disable on each callback rather than leaving the file inconsistent.
  • _{maintainability}

---

Reviewers (9): correctness, testing, maintainability, project-standards, ce-agent-native-reviewer, ce-learnings-researcher, adversarial, kieran-typescript, julik-frontend-races.

Testing gaps: the three P2 items above are the substantive testing gaps; agent-native parity and past-learnings searches returned clean.

[alex-fedotyev]

/just-fix-it

[github-actions]

Fixed:

• DBHeatmapChart.tsx:59 (P0) — exported applySelectionToChart; added DBHeatmapChart.test.tsx with 12 unit tests covering null/undefined clear, scale-not-populated early return, log/linear branching, yMin === 0 clamp to yScaleMin (both scales), yMax > yScaleMax clamp, xMin * 1000 ms conversion, reversed xMin > xMax, non-negative height, and fireHook=false.
• DBHeatmapChart.tsx:1054-1058 (P2) — added width, height to the reapply effect's deps so the persisted rectangle is reprojected on container resize. Moved useElementSize above the effect to bring the values into scope.

Skipped:

• P2 ref-mirror race (lines 1037-1056 / 1242-1248) — fix has two non-equivalent options (render-time ref assignment vs. onCreate-driven apply) with different React-strict-mode tradeoffs; ambiguous per rules. Recommend follow-up.
• All P3 nitpicks per rules.