From 3fa7421abb1d6ca5583119b8722dff8e89efe178 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 5 May 2026 04:37:16 +0000 Subject: [PATCH 1/9] fix(heatmap): persist drag-select rectangle across re-renders The dashed selection rectangle on the Event Deltas heatmap collapses to a 2x2 px box the instant the user releases the mouse. The filter is applied correctly (URL params, comparison legend, bar charts all update), but visually you can no longer see what region you picked. Root cause: uplot-react's dataMatch deep-compares the data prop and calls chart.setData(data, true) when refs differ. setData resets uPlot's internal u.select rectangle. HeatmapContainer rebuilt the [time, bucket, count] arrays in its render body, so on every parent re-render those entries had new references with identical values, and dataMatch returned false. Memoize the data array with stable refs so dataMatch returns true, setData is skipped, and u.select survives the re-render that follows a drag (the URL param write triggers). Also memoize generatedTsBuckets so its fresh-Date-array each render doesn't propagate into the heatmapData useMemo deps. HDX-4147 --- .../app/src/components/DBHeatmapChart.tsx | 88 +++++++++++++------ 1 file changed, 60 insertions(+), 28 deletions(-) diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index e69e5ed673..5ddfc148bc 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -600,39 +600,59 @@ function HeatmapContainer({ enabled: !!minMaxData && bucketConfig != null && max > effectiveMin, }); - const generatedTsBuckets = timeBucketByGranularity( - dateRange[0], - dateRange[1], - granularity, + // Memoize so the array refs are stable across re-renders that don't + // change the underlying time range or granularity. Without this, + // timeBucketByGranularity returns a fresh Date[] each render, which + // would propagate into the heatmapData useMemo deps and defeat its + // own memoization. + const fromMs = dateRange[0]?.getTime() ?? 0; + const toMs = dateRange[1]?.getTime() ?? 0; + const generatedTsBuckets = useMemo( + () => timeBucketByGranularity(dateRange[0], dateRange[1], granularity), + // dateRange itself may be a fresh array each render; depend on the + // primitive timestamps + granularity instead. + // eslint-disable-next-line react-hooks/exhaustive-deps + [fromMs, toMs, granularity], ); const timestampColumn = inferTimestampColumn(data?.meta ?? []); - // Compute the y-axis value for a given bucket index. - // For log scale we store values in log space so that bins are uniformly - // spaced on the linear uPlot y-axis. The heatmapPaths renderer assumes - // uniform increments (yBinIncr = ys[1] - ys[0]) to compute tile height; - // with actual log-spaced values the first increment is tiny relative to the - // full range and tiles render at ~0px height (invisible). - const bucketToYValue = (j: number) => { - if (scaleType === 'log' && effectiveMin > 0 && max > effectiveMin) { - // Return the natural-log of the actual bucket boundary so that the - // y-values are uniformly spaced. Tick labels are exponentiated back - // via the tickFormatter below. - const actualValue = - effectiveMin * Math.pow(max / effectiveMin, j / nBuckets); - return Math.log(actualValue); + // Build the [time, bucket, count] arrays that feed uPlot. Memoized so the + // inner references stay stable when only the URL filter params change + // (drag-select writes xMin/xMax/yMin/yMax). uplot-react's dataMatch + // compares each series entry with === ; if the inner refs change, it calls + // setData(data, true), which resets uPlot's u.select rectangle. Stable + // refs here keep dataMatch happy and the dashed selection rectangle + // visible after the user releases the mouse. + const heatmapData = useMemo(() => { + const time: number[] = []; // x values + const bucket: number[] = []; // y value series 1 + const count: number[] = []; // y value series 2 + + if (data == null || timestampColumn == null) { + return [time, bucket, count]; } - // Linear: min + j * step - return effectiveMin + j * ((max - effectiveMin) / nBuckets); - }; - const time: number[] = []; // x values - const bucket: number[] = []; // y value series 1 - const count: number[] = []; // y value series 2 - if (data != null && timestampColumn != null) { - let dataIndex = 0; + // Compute the y-axis value for a given bucket index. + // For log scale we store values in log space so that bins are uniformly + // spaced on the linear uPlot y-axis. The heatmapPaths renderer assumes + // uniform increments (yBinIncr = ys[1] - ys[0]) to compute tile height; + // with actual log-spaced values the first increment is tiny relative to + // the full range and tiles render at ~0px height (invisible). + const bucketToYValue = (j: number) => { + if (scaleType === 'log' && effectiveMin > 0 && max > effectiveMin) { + // Return the natural-log of the actual bucket boundary so that the + // y-values are uniformly spaced. Tick labels are exponentiated back + // via the tickFormatter below. + const actualValue = + effectiveMin * Math.pow(max / effectiveMin, j / nBuckets); + return Math.log(actualValue); + } + // Linear: min + j * step + return effectiveMin + j * ((max - effectiveMin) / nBuckets); + }; + let dataIndex = 0; for (let i = 0; i < generatedTsBuckets.length; i++) { const generatedTs = generatedTsBuckets[i].getTime(); @@ -657,7 +677,19 @@ function HeatmapContainer({ } } } - } + + return [time, bucket, count]; + }, [ + data, + timestampColumn?.name, + generatedTsBuckets, + scaleType, + effectiveMin, + max, + nBuckets, + ]); + + const [time, bucket, count] = heatmapData; const toolbarItemsMemo = useMemo(() => { const allToolbarItems: React.ReactNode[] = []; @@ -743,7 +775,7 @@ function HeatmapContainer({ ) : ( Date: Tue, 5 May 2026 04:46:21 +0000 Subject: [PATCH 2/9] fix(heatmap): satisfy exhaustive-deps and add changeset Lint feedback from CI: - timestampColumn?.name dep is wrong since the closure uses the full timestampColumn object; pass the object instead. The ref is stable when data is stable, so this doesn't reintroduce the original bug. - Stop destructuring bucket/count when only time is needed for the not-enough-data short-circuit. Add changeset (patch on @hyperdx/app). --- .changeset/heatmap-select-persist.md | 5 +++++ packages/app/src/components/DBHeatmapChart.tsx | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 .changeset/heatmap-select-persist.md diff --git a/.changeset/heatmap-select-persist.md b/.changeset/heatmap-select-persist.md new file mode 100644 index 0000000000..bf29e58b2c --- /dev/null +++ b/.changeset/heatmap-select-persist.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/app": patch +--- + +fix: Persist heatmap drag-select rectangle on Event Deltas and Search heatmaps so the dashed selection stays visible after mouseup diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index 5ddfc148bc..7668ce8a53 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -681,7 +681,7 @@ function HeatmapContainer({ return [time, bucket, count]; }, [ data, - timestampColumn?.name, + timestampColumn, generatedTsBuckets, scaleType, effectiveMin, @@ -689,7 +689,7 @@ function HeatmapContainer({ nBuckets, ]); - const [time, bucket, count] = heatmapData; + const time = heatmapData[0]; const toolbarItemsMemo = useMemo(() => { const allToolbarItems: React.ReactNode[] = []; From 41fd036bf04968c5ce313d100d7662cc3d533382 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 5 May 2026 04:51:15 +0000 Subject: [PATCH 3/9] fix(heatmap): inline timestampColumn into useMemo, drop trivial labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot review feedback: timestampColumn was a non-primitive useMemo dep derived entirely from data?.meta. Move the call inside the useMemo so data is the only relevant dep, which is the cleanest way to satisfy both exhaustive-deps and the ref-stability the fix relies on. Also drop the // x values / // y value series 1 / // y value series 2 labels — they describe what the line is rather than why. --- .../app/src/components/DBHeatmapChart.tsx | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index 7668ce8a53..a239b1199b 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -615,8 +615,6 @@ function HeatmapContainer({ [fromMs, toMs, granularity], ); - const timestampColumn = inferTimestampColumn(data?.meta ?? []); - // Build the [time, bucket, count] arrays that feed uPlot. Memoized so the // inner references stay stable when only the URL filter params change // (drag-select writes xMin/xMax/yMin/yMax). uplot-react's dataMatch @@ -625,9 +623,13 @@ function HeatmapContainer({ // refs here keep dataMatch happy and the dashed selection rectangle // visible after the user releases the mouse. const heatmapData = useMemo(() => { - const time: number[] = []; // x values - const bucket: number[] = []; // y value series 1 - const count: number[] = []; // y value series 2 + const time: number[] = []; + const bucket: number[] = []; + const count: number[] = []; + + // timestampColumn is fully derived from data.meta, so it doesn't need + // to be a separate hook dep — having data as a dep is sufficient. + const timestampColumn = inferTimestampColumn(data?.meta ?? []); if (data == null || timestampColumn == null) { return [time, bucket, count]; @@ -679,15 +681,7 @@ function HeatmapContainer({ } return [time, bucket, count]; - }, [ - data, - timestampColumn, - generatedTsBuckets, - scaleType, - effectiveMin, - max, - nBuckets, - ]); + }, [data, generatedTsBuckets, scaleType, effectiveMin, max, nBuckets]); const time = heatmapData[0]; From c9785a2d4f6f8fe48b9de163ec57daefca6dc875 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 5 May 2026 21:34:17 +0000 Subject: [PATCH 4/9] fix(heatmap): drop em-dash from timestampColumn comment prose-lint flags U+2014 anywhere; tighten the comment instead. --- packages/app/src/components/DBHeatmapChart.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index a239b1199b..ffd73696d4 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -627,8 +627,7 @@ function HeatmapContainer({ const bucket: number[] = []; const count: number[] = []; - // timestampColumn is fully derived from data.meta, so it doesn't need - // to be a separate hook dep — having data as a dep is sufficient. + // timestampColumn is derived from data.meta, so data covers the dep. const timestampColumn = inferTimestampColumn(data?.meta ?? []); if (data == null || timestampColumn == null) { From efc91c8711c815c2b64158af0728379c68e218cf Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 5 May 2026 22:12:16 +0000 Subject: [PATCH 5/9] fix(heatmap): tighten memoization comments Drop the dataMatch description (it was element-wise === per series, not top-level === as written) and consolidate to the actionable mechanism: fresh data ref drives uplot-react to call setData(data, true) which wipes u.select. Add HDX-4147 ticket reference. --- .../app/src/components/DBHeatmapChart.tsx | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index ffd73696d4..2555dfe6d9 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -600,28 +600,21 @@ function HeatmapContainer({ enabled: !!minMaxData && bucketConfig != null && max > effectiveMin, }); - // Memoize so the array refs are stable across re-renders that don't - // change the underlying time range or granularity. Without this, - // timeBucketByGranularity returns a fresh Date[] each render, which - // would propagate into the heatmapData useMemo deps and defeat its - // own memoization. + // Memoize so timeBucketByGranularity's fresh Date[] doesn't defeat + // the heatmapData memoization downstream. dateRange itself may be a + // fresh array each render, so depend on primitive ms + granularity. const fromMs = dateRange[0]?.getTime() ?? 0; const toMs = dateRange[1]?.getTime() ?? 0; const generatedTsBuckets = useMemo( () => timeBucketByGranularity(dateRange[0], dateRange[1], granularity), - // dateRange itself may be a fresh array each render; depend on the - // primitive timestamps + granularity instead. // eslint-disable-next-line react-hooks/exhaustive-deps [fromMs, toMs, granularity], ); - // Build the [time, bucket, count] arrays that feed uPlot. Memoized so the - // inner references stay stable when only the URL filter params change - // (drag-select writes xMin/xMax/yMin/yMax). uplot-react's dataMatch - // compares each series entry with === ; if the inner refs change, it calls - // setData(data, true), which resets uPlot's u.select rectangle. Stable - // refs here keep dataMatch happy and the dashed selection rectangle - // visible after the user releases the mouse. + // Memoize: URL-filter-only re-renders (drag-select writes xMin/xMax/ + // yMin/yMax to the URL) must not change the data ref, or uplot-react + // calls setData(data, true), which wipes uPlot's u.select rectangle. + // Stable refs keep the dashed selection visible after mouseup. (HDX-4147) const heatmapData = useMemo(() => { const time: number[] = []; const bucket: number[] = []; From bd09e26653689bf49c35600622984e2309d49a66 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 6 May 2026 14:56:07 +0000 Subject: [PATCH 6/9] fix(heatmap): mirror selection from URL on every uPlot create The previous memoization fix was insufficient. The actual cause is that uplot-react destroys and recreates the chart whenever its `options` prop ref changes. `tickFormatter` was rebuilt on every render because `numberFormat` is a fresh object reference from callers (e.g. DBSearchHeatmapChart constructs `{output: 'duration', factor: 0.001}` inline). Rebuilt tickFormatter cascaded into the `options` useMemo, optionsUpdateState detected new top-level keys (series, axes, cursor, plugins are new array/object literals on each recompute), classified the change as 'create', and the chart was destroyed and recreated, wiping `u.select`. Two layers: 1. Stabilize `tickFormatter`'s dep on `numberFormat` content via JSON.stringify, so callers passing a fresh-each-render `numberFormat` no longer cause chart recreation. 2. Pass the URL-backed selection into `Heatmap` as a `selectionBounds` prop and reapply via `setSelect` on chart create + on the prop changing. This makes the URL the source of truth and survives any future cause of chart recreation (theme switch, resize bounce, etc.). --- .../app/src/components/DBHeatmapChart.tsx | 129 +++++++++++++++++- .../Search/DBSearchHeatmapChart.tsx | 24 ++++ 2 files changed, 147 insertions(+), 6 deletions(-) diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index 2555dfe6d9..5dc77a09e0 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -35,6 +35,81 @@ import { SQLPreview } from './ChartSQLPreview'; type Mode2DataArray = [number[], number[], number[]]; +/** + * Drag-select bounds in data space: x in seconds (URL convention), y in + * the y-axis's natural unit (NOT log-space; callers pass the actual value + * users would expect to see, e.g. ms latency). yMin may be 0 when the + * selection touched the bottom bucket; the renderer clamps to the chart's + * visible y-axis floor. + */ +export type SelectionBounds = { + xMin: number; + xMax: number; + yMin: number; + yMax: number; +}; + +/** + * Reapply a persisted selection rectangle to a uPlot instance. Called on + * chart create and whenever the bounds prop changes; uPlot's `u.select` + * is owned by the chart, so it gets wiped on any chart recreation. We use + * the URL-backed bounds as the source of truth and mirror them onto the + * chart imperatively. fireHook=false avoids re-entering the setSelect hook. + */ +function applySelectionToChart( + u: uPlot, + bounds: SelectionBounds | null | undefined, + scaleType: HeatmapScaleType, +) { + if (bounds == null) { + u.setSelect({ left: 0, top: 0, width: 0, height: 0 }, false); + return; + } + + const { xMin, xMax, yMin, yMax } = bounds; + + // x is in seconds in the URL; uPlot's x-axis is configured ms:1 so + // values are stored as ms. + const xMinPx = u.valToPos(xMin * 1000, 'x'); + const xMaxPx = u.valToPos(xMax * 1000, 'x'); + + const yScaleMin = u.scales.y?.min; + const yScaleMax = u.scales.y?.max; + if (yScaleMin == null || yScaleMax == null) { + return; + } + + // For log scale, y-axis values are stored in log-space. Convert and + // clamp to the visible axis: yMin may be 0 (bottom-bucket adjustment in + // HeatmapContainer's onFilter wrapper); yMax may exceed the axis. + let yLowPlot: number; + let yHighPlot: number; + if (scaleType === 'log') { + yHighPlot = yMax > 0 ? Math.min(Math.log(yMax), yScaleMax) : yScaleMax; + yLowPlot = yMin > 0 ? Math.max(Math.log(yMin), yScaleMin) : yScaleMin; + } else { + yHighPlot = Math.min(yMax, yScaleMax); + yLowPlot = Math.max(yMin, yScaleMin); + } + + // uPlot's y-axis: high data values map to small pixel y (top of chart). + const yHighPx = u.valToPos(yHighPlot, 'y'); + const yLowPx = u.valToPos(yLowPlot, 'y'); + + const left = Math.min(xMinPx, xMaxPx); + const right = Math.max(xMinPx, xMaxPx); + + u.setSelect( + { + left, + top: yHighPx, + width: right - left, + height: Math.max(0, yLowPx - yHighPx), + }, + false, + ); +} + // From: https://github.com/leeoniya/uPlot/blob/a4edb297a9b80baf781f4d05a40fb52fae737bff/demos/latency-heatmap.html#L436 function heatmapPaths(opts: { disp: { fill: { lookup: string[]; values: any } }; @@ -534,6 +609,7 @@ function HeatmapContainer({ enabled = true, onFilter, onClearFilter, + selectionBounds, title, toolbarPrefix, toolbarSuffix, @@ -544,6 +620,14 @@ function HeatmapContainer({ enabled?: boolean; onFilter?: (xMin: number, xMax: number, yMin: number, yMax: number) => void; onClearFilter?: () => void; + /** + * The currently-applied drag-select bounds. When provided, the heatmap + * draws the dashed selection rectangle and reapplies it after any uPlot + * recreation (theme switch, prop change, resize) so the user always sees + * which slice they filtered. Caller owns the URL/query-state plumbing; + * this is purely a visual mirror of that state. + */ + selectionBounds?: SelectionBounds | null; title?: React.ReactNode; toolbarPrefix?: React.ReactNode[]; toolbarSuffix?: React.ReactNode[]; @@ -611,10 +695,11 @@ function HeatmapContainer({ [fromMs, toMs, granularity], ); - // Memoize: URL-filter-only re-renders (drag-select writes xMin/xMax/ - // yMin/yMax to the URL) must not change the data ref, or uplot-react - // calls setData(data, true), which wipes uPlot's u.select rectangle. - // Stable refs keep the dashed selection visible after mouseup. (HDX-4147) + // Stable [time, bucket, count] arrays let uplot-react skip its setData + // path when only URL-state (xMin/xMax/yMin/yMax) changed. Pairs with the + // selectionBounds prop on Heatmap below: the prop reapplies u.select on + // any chart recreation, this memo prevents the recreation in the + // common case. const heatmapData = useMemo(() => { const time: number[] = []; const bucket: number[] = []; @@ -770,7 +855,7 @@ function HeatmapContainer({ // clamped by greatest(value, effectiveMin). If the // selection touches that bucket, widen yMin to 0 so // the downstream SQL filter captures all those spans. - // The 1.1× threshold adds 10% headroom to account for + // The 1.1x threshold adds 10% headroom to account for // floating-point rounding in the bucket boundary. const adjustedYMin = scaleType === 'log' && yMin <= effectiveMin * 1.1 @@ -783,6 +868,7 @@ function HeatmapContainer({ onClearFilter={onClearFilter} scaleType={scaleType} palette={palette} + selectionBounds={selectionBounds} /> )} @@ -893,6 +979,7 @@ function Heatmap({ onClearFilter, scaleType = 'linear', palette, + selectionBounds, }: { data: Mode2DataArray; numberFormat?: NumberFormat; @@ -900,6 +987,7 @@ function Heatmap({ onClearFilter?: () => void; scaleType?: HeatmapScaleType; palette: string[]; + selectionBounds?: SelectionBounds | null; }) { const [highlightedPoint, setHighlightedPoint] = useState< | { @@ -937,6 +1025,17 @@ function Heatmap({ // persisted u.select rectangle (which is owned by uPlot, not React). const uplotRef = useRef(null); + // Reapply the URL-backed selection whenever the bounds prop changes + // (e.g. a fresh drag-select arrives via the round-trip through the + // parent's URL state, or the parent clears the filter). This complements + // the onCreate path: that handles chart creation, this handles bounds + // changes against an existing chart. + useEffect(() => { + if (uplotRef.current) { + applySelectionToChart(uplotRef.current, selectionBounds, scaleType); + } + }, [selectionBounds, scaleType]); + // Timestamp of the most recent drag-end. Guards the container's onClick // handler from clearing the selection when the synthetic click event // that fires on mouseup-after-drag arrives. @@ -944,6 +1043,17 @@ function Heatmap({ const { ref, width, height } = useElementSize(); + // Stabilize on numberFormat content, not reference. Callers (e.g. + // DBSearchHeatmapChart) build a fresh `numberFormat` object on every + // render; depending on its identity would rebuild tickFormatter, then + // the options memo, then uplot-react would see new top-level keys via + // optionsUpdateState and treat the change as 'create', destroying the + // chart and wiping u.select. Hashing the contents lets the memo skip + // when the actual format is unchanged. (HDX-4147) + const numberFormatKey = useMemo( + () => (numberFormat ? JSON.stringify(numberFormat) : ''), + [numberFormat], + ); const tickFormatter = useCallback( (value: number) => { // y-values are stored in log space for log scale; exponentiate back @@ -969,7 +1079,8 @@ function Heatmap({ compactDisplay: 'short', }).format(actualValue); }, - [numberFormat, scaleType], + // eslint-disable-next-line react-hooks/exhaustive-deps + [numberFormatKey, scaleType], ); const options: uPlot.Options = useMemo(() => { @@ -1161,6 +1272,12 @@ function Heatmap({ resetScales={true} onCreate={chart => { uplotRef.current = chart; + // Reapply the persisted selection on every uPlot construction. + // uplot-react destroys+recreates the chart whenever options + // identity changes (and various deps make options unstable in + // practice), wiping u.select. Re-mirroring from the URL-backed + // bounds is the source-of-truth fix. (HDX-4147) + applySelectionToChart(chart, selectionBounds, scaleType); }} onDelete={() => { uplotRef.current = null; diff --git a/packages/app/src/components/Search/DBSearchHeatmapChart.tsx b/packages/app/src/components/Search/DBSearchHeatmapChart.tsx index e618526abb..e6d2ea5813 100644 --- a/packages/app/src/components/Search/DBSearchHeatmapChart.tsx +++ b/packages/app/src/components/Search/DBSearchHeatmapChart.tsx @@ -26,6 +26,7 @@ import DBHeatmapChart, { darkPalette, type HeatmapScaleType, lightPalette, + type SelectionBounds, toHeatmapChartConfig, } from '../DBHeatmapChart'; @@ -93,6 +94,28 @@ export function DBSearchHeatmapChart({ prevDateRangeRef.current = key; }, [fromMs, toMs, setFields]); + // Mirror the URL's selection coordinates onto the heatmap so the dashed + // rectangle stays in sync after any uPlot recreation. Memoize on the + // primitive coords so the prop's identity is stable when the URL hasn't + // changed; otherwise the downstream useEffect would re-fire on every + // render. (HDX-4147) + const selectionBounds: SelectionBounds | null = useMemo(() => { + if ( + fields.xMin == null || + fields.xMax == null || + fields.yMin == null || + fields.yMax == null + ) { + return null; + } + return { + xMin: fields.xMin, + xMax: fields.xMax, + yMin: fields.yMin, + yMax: fields.yMax, + }; + }, [fields.xMin, fields.xMax, fields.yMin, fields.yMax]); + return ( { setFields({ xMin, xMax, yMin, yMax }); }} From ffad4b563315267d06740e776d617a1d399c4bb3 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 6 May 2026 15:05:25 +0000 Subject: [PATCH 7/9] fix(heatmap): apply selection from uPlot's ready hook Page reload with a URL-encoded selection wasn't drawing the rectangle. Root cause: at onCreate time, uPlot has constructed but not yet completed its initial layout for mode-2 facet data. u.scales.y.min/max can still be undefined, so applySelectionToChart's early-out triggered and setSelect never got called. Move the apply call to uPlot's `ready` hook, which fires once after the initial draw completes. Hold selectionBounds and scaleType in refs so the hook (captured inside the options useMemo) reads the latest values without requiring memo dep churn. --- .../app/src/components/DBHeatmapChart.tsx | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index 5dc77a09e0..b42f6ed893 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -1025,11 +1025,24 @@ function Heatmap({ // persisted u.select rectangle (which is owned by uPlot, not React). const uplotRef = useRef(null); + // Hold selectionBounds and scaleType in refs so the uPlot `ready` hook + // (captured inside the options useMemo via closure) always sees the + // latest values without needing them in the memo's dep array. Mutating + // a ref doesn't trigger re-renders, so the options identity stays + // stable across selection/scale changes. + const selectionBoundsRef = useRef(selectionBounds); + const scaleTypeRef = useRef(scaleType); + useEffect(() => { + selectionBoundsRef.current = selectionBounds; + scaleTypeRef.current = scaleType; + }); + // Reapply the URL-backed selection whenever the bounds prop changes // (e.g. a fresh drag-select arrives via the round-trip through the // parent's URL state, or the parent clears the filter). This complements - // the onCreate path: that handles chart creation, this handles bounds - // changes against an existing chart. + // the uPlot `ready` hook below: that handles initial chart creation + // (and any recreation), this handles bounds changes against an existing + // chart. useEffect(() => { if (uplotRef.current) { applySelectionToChart(uplotRef.current, selectionBounds, scaleType); @@ -1209,6 +1222,17 @@ function Heatmap({ }), { hooks: { + // Fires once after uPlot finishes initial layout/draw. At + // onCreate time scales aren't reliably populated for mode-2 + // facet data, so reapplying the URL-backed selection from + // here ensures valToPos has the bounds it needs. (HDX-4147) + ready: u => { + applySelectionToChart( + u, + selectionBoundsRef.current, + scaleTypeRef.current, + ); + }, setSelect: u => { // Ignore zero-size selections (e.g. single-click) if (u.select.width <= 0 || u.select.height <= 0) { @@ -1272,12 +1296,6 @@ function Heatmap({ resetScales={true} onCreate={chart => { uplotRef.current = chart; - // Reapply the persisted selection on every uPlot construction. - // uplot-react destroys+recreates the chart whenever options - // identity changes (and various deps make options unstable in - // practice), wiping u.select. Re-mirroring from the URL-backed - // bounds is the source-of-truth fix. (HDX-4147) - applySelectionToChart(chart, selectionBounds, scaleType); }} onDelete={() => { uplotRef.current = null; From 41b87cc654c2b7b32177f9ed31c169df8cec8f63 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev Date: Wed, 6 May 2026 22:17:20 +0000 Subject: [PATCH 8/9] docs(heatmap): clarify subtle invariants per compound-review nits Three small comment additions in DBHeatmapChart.tsx (compound review on #2189 flagged these for the next reader): - `applySelectionToChart`: 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. - The ref-mirroring `useEffect` deliberately has no deps array; add a trailing comment so it doesn't read as "missing deps" at a glance. - `numberFormatKey`: document the dependency on NumberFormat being JSON-serializable, with a switching note in case the type ever grows a function-valued field. --- packages/app/src/components/DBHeatmapChart.tsx | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index b42f6ed893..ef5b485213 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -61,6 +61,10 @@ function applySelectionToChart( bounds: SelectionBounds | null | undefined, scaleType: HeatmapScaleType, ) { + // The clear path runs BEFORE the scale-not-populated guard below so a + // null bounds always clears the rectangle, even on first paint when + // u.scales.y is not yet populated. Keep this ordering when refactoring; + // swapping it would suppress clears while scales are loading. if (bounds == null) { u.setSelect({ left: 0, top: 0, width: 0, height: 0 }, false); return; @@ -1032,6 +1036,8 @@ function Heatmap({ // stable across selection/scale changes. const selectionBoundsRef = useRef(selectionBounds); const scaleTypeRef = useRef(scaleType); + // Runs every commit; mirrors latest props into refs (no deps array on + // purpose). useEffect(() => { selectionBoundsRef.current = selectionBounds; scaleTypeRef.current = scaleType; @@ -1063,6 +1069,13 @@ function Heatmap({ // optionsUpdateState and treat the change as 'create', destroying the // chart and wiping u.select. Hashing the contents lets the memo skip // when the actual format is unchanged. (HDX-4147) + // + // Relies on NumberFormat being JSON-serializable: today it is plain + // config (string + number fields), so JSON.stringify is a faithful + // fingerprint. If the type ever grows a function-valued field + // (e.g. a custom `formatter` callback), switch to a shallow-equal + // helper keyed on the known fields, because functions stringify to + // undefined and would silently skip rebuilds. const numberFormatKey = useMemo( () => (numberFormat ? JSON.stringify(numberFormat) : ''), [numberFormat], From 13c627aebd3c68712df2910ec84a152bf1c7aa97 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Sat, 9 May 2026 03:09:58 +0000 Subject: [PATCH 9/9] fix: address deep review findings --- .../src/components/DBHeatmapChart.test.tsx | 148 ++++++++++++++++++ .../app/src/components/DBHeatmapChart.tsx | 20 +-- 2 files changed, 159 insertions(+), 9 deletions(-) create mode 100644 packages/app/src/components/DBHeatmapChart.test.tsx diff --git a/packages/app/src/components/DBHeatmapChart.test.tsx b/packages/app/src/components/DBHeatmapChart.test.tsx new file mode 100644 index 0000000000..4bf406c9cd --- /dev/null +++ b/packages/app/src/components/DBHeatmapChart.test.tsx @@ -0,0 +1,148 @@ +import type uPlot from 'uplot'; + +import { applySelectionToChart, SelectionBounds } from './DBHeatmapChart'; + +type StubU = { + scales: { + x?: { min?: number; max?: number }; + y?: { min?: number; max?: number }; + }; + valToPos: jest.Mock; + setSelect: jest.Mock< + void, + [{ left: number; top: number; width: number; height: number }, boolean] + >; +}; + +function makeStubU(opts?: { + yScale?: { min?: number; max?: number }; + xScale?: { min?: number; max?: number }; + valToPos?: (val: number, axis: 'x' | 'y') => number; +}): StubU { + const yScale = opts?.yScale === undefined ? { min: 0, max: 10 } : opts.yScale; + const xScale = + opts?.xScale === undefined ? { min: 0, max: 1_000_000 } : opts.xScale; + // Default: identity passthrough so callers can verify exact arguments. + const valToPos = jest.fn( + opts?.valToPos ?? ((val: number, _axis: 'x' | 'y') => val), + ); + const setSelect = jest.fn(); + return { + scales: { x: xScale, y: yScale }, + valToPos, + setSelect, + }; +} + +describe('applySelectionToChart', () => { + it('clears the selection when bounds is null', () => { + const u = makeStubU(); + applySelectionToChart(u as unknown as uPlot, null, 'linear'); + expect(u.setSelect).toHaveBeenCalledTimes(1); + expect(u.setSelect).toHaveBeenCalledWith( + { left: 0, top: 0, width: 0, height: 0 }, + false, + ); + expect(u.valToPos).not.toHaveBeenCalled(); + }); + + it('clears the selection when bounds is undefined', () => { + const u = makeStubU(); + applySelectionToChart(u as unknown as uPlot, undefined, 'linear'); + expect(u.setSelect).toHaveBeenCalledWith( + { left: 0, top: 0, width: 0, height: 0 }, + false, + ); + }); + + it('returns early without calling setSelect when y scale is unpopulated', () => { + const u = makeStubU({ yScale: { min: undefined, max: undefined } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 1, yMax: 5 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + expect(u.setSelect).not.toHaveBeenCalled(); + }); + + it('converts xMin/xMax from seconds to ms before calling valToPos', () => { + const u = makeStubU(); + const bounds: SelectionBounds = { xMin: 5, xMax: 10, yMin: 1, yMax: 9 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + expect(u.valToPos).toHaveBeenCalledWith(5_000, 'x'); + expect(u.valToPos).toHaveBeenCalledWith(10_000, 'x'); + }); + + it('passes raw y values through under linear scale', () => { + const u = makeStubU({ yScale: { min: 0, max: 100 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 5, yMax: 50 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + // y values must NOT be log-transformed under linear scale. + expect(u.valToPos).toHaveBeenCalledWith(50, 'y'); + expect(u.valToPos).toHaveBeenCalledWith(5, 'y'); + }); + + it('applies Math.log to y values under log scale', () => { + const u = makeStubU({ yScale: { min: 0, max: 100 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 1, yMax: Math.E }; + applySelectionToChart(u as unknown as uPlot, bounds, 'log'); + // Math.log(1) === 0, Math.log(Math.E) === 1 + expect(u.valToPos).toHaveBeenCalledWith(0, 'y'); + expect(u.valToPos).toHaveBeenCalledWith(1, 'y'); + }); + + it('clamps yMin === 0 to the chart-floor (yScaleMin) under linear scale', () => { + const u = makeStubU({ yScale: { min: 2, max: 100 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 0, yMax: 50 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + // yMin=0 should clamp UP to yScale.min (2), not pass through. + expect(u.valToPos).toHaveBeenCalledWith(2, 'y'); + expect(u.valToPos).not.toHaveBeenCalledWith(0, 'y'); + }); + + it('clamps yMin === 0 to the chart-floor (yScaleMin) under log scale', () => { + const u = makeStubU({ yScale: { min: -1, max: 5 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 0, yMax: Math.E }; + applySelectionToChart(u as unknown as uPlot, bounds, 'log'); + // yMin=0 (or any non-positive) bypasses Math.log and clamps to yScale.min. + expect(u.valToPos).toHaveBeenCalledWith(-1, 'y'); + }); + + it('clamps yMax above the chart ceiling to yScaleMax under linear', () => { + const u = makeStubU({ yScale: { min: 0, max: 10 } }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 1, yMax: 999 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + expect(u.valToPos).toHaveBeenCalledWith(10, 'y'); + }); + + it('handles reversed xMin > xMax via Math.min/max for left/right', () => { + const u = makeStubU(); + const bounds: SelectionBounds = { xMin: 10, xMax: 5, yMin: 1, yMax: 9 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + // valToPos identity: xMin*1000=10000, xMax*1000=5000 + // left should be Math.min(10000, 5000) = 5000, width = 10000-5000 = 5000 + const call = u.setSelect.mock.calls[0][0]; + expect(call.left).toBe(5_000); + expect(call.width).toBe(5_000); + }); + + it('produces non-negative height when yLowPx < yHighPx', () => { + // valToPos returns inverse: smaller y-data = larger pixel-y (uPlot semantics). + const u = makeStubU({ + yScale: { min: 0, max: 10 }, + // Force yLowPx < yHighPx by inverting the mapping + valToPos: (val, axis) => (axis === 'y' ? 100 - val : val), + }); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 2, yMax: 8 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + const call = u.setSelect.mock.calls[0][0]; + // yHighPx = 100-8 = 92, yLowPx = 100-2 = 98, height = max(0, 98-92) = 6 + expect(call.top).toBe(92); + expect(call.height).toBe(6); + }); + + it('passes fireHook=false to setSelect to avoid re-entering the setSelect hook', () => { + const u = makeStubU(); + const bounds: SelectionBounds = { xMin: 0, xMax: 1, yMin: 1, yMax: 9 }; + applySelectionToChart(u as unknown as uPlot, bounds, 'linear'); + const fireHook = u.setSelect.mock.calls[0][1]; + expect(fireHook).toBe(false); + }); +}); diff --git a/packages/app/src/components/DBHeatmapChart.tsx b/packages/app/src/components/DBHeatmapChart.tsx index ef5b485213..4deac54ea1 100644 --- a/packages/app/src/components/DBHeatmapChart.tsx +++ b/packages/app/src/components/DBHeatmapChart.tsx @@ -56,7 +56,7 @@ export type SelectionBounds = { * the URL-backed bounds as the source of truth and mirror them onto the * chart imperatively. fireHook=false avoids re-entering the setSelect hook. */ -function applySelectionToChart( +export function applySelectionToChart( u: uPlot, bounds: SelectionBounds | null | undefined, scaleType: HeatmapScaleType, @@ -1043,25 +1043,27 @@ function Heatmap({ scaleTypeRef.current = scaleType; }); + const { ref, width, height } = useElementSize(); + // Reapply the URL-backed selection whenever the bounds prop changes // (e.g. a fresh drag-select arrives via the round-trip through the - // parent's URL state, or the parent clears the filter). This complements - // the uPlot `ready` hook below: that handles initial chart creation - // (and any recreation), this handles bounds changes against an existing - // chart. + // parent's URL state, or the parent clears the filter), or when the + // container resizes (uPlot calls setSize, not a full recreate, so the + // pixel-space u.select would otherwise be stale). This complements the + // uPlot `ready` hook below: that handles initial chart creation (and + // any recreation), this handles bounds and size changes against an + // existing chart. useEffect(() => { if (uplotRef.current) { applySelectionToChart(uplotRef.current, selectionBounds, scaleType); } - }, [selectionBounds, scaleType]); + }, [selectionBounds, scaleType, width, height]); // Timestamp of the most recent drag-end. Guards the container's onClick // handler from clearing the selection when the synthetic click event // that fires on mouseup-after-drag arrives. const justDraggedAtRef = useRef(0); - const { ref, width, height } = useElementSize(); - // Stabilize on numberFormat content, not reference. Callers (e.g. // DBSearchHeatmapChart) build a fresh `numberFormat` object on every // render; depending on its identity would rebuild tickFormatter, then @@ -1203,7 +1205,7 @@ function Heatmap({ }, plugins: [ // legendAsTooltipPlugin() - // eslint-disable-next-line react-hooks/refs -- mouseInsideRef is read at event time, not during render + highlightDataPlugin({ proximity: 20, onPointHighlight: ({