From 50ebd284fc09e1d571f5ce17d523f2e1574b9019 Mon Sep 17 00:00:00 2001 From: arj22 Date: Mon, 21 Sep 2026 22:02:59 -0500 Subject: [PATCH 01/25] fix: keep Y-axis ticks evenly spaced when Recharts thins them for overlap Recharts' own "nice" tick candidates aren't always uniformly spaced, and its minTickGap-based overlap thinning drops individual candidates without re-spacing the survivors - producing axes like 0/300/1k (gaps of 300 then 700) instead of 0/250/500/750/1k. Feed it a uniform candidate set instead: thinning a uniformly-spaced sequence for overlap always keeps every Nth candidate, so whatever survives stays evenly spaced. Tick count behavior is unchanged - this only changes which values are chosen, not how many survive. Co-Authored-By: Claude Sonnet 5 --- .changeset/y-axis-even-tick-spacing.md | 9 +++++ packages/app/src/HDXMultiSeriesTimeChart.tsx | 37 +++++++++++++++++-- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 20 ++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 .changeset/y-axis-even-tick-spacing.md diff --git a/.changeset/y-axis-even-tick-spacing.md b/.changeset/y-axis-even-tick-spacing.md new file mode 100644 index 0000000000..1ff6d3aacf --- /dev/null +++ b/.changeset/y-axis-even-tick-spacing.md @@ -0,0 +1,9 @@ +--- +'@hyperdx/app': patch +--- + +fix: keep Y-axis ticks evenly spaced when Recharts thins them for overlap + +A chart's Y-axis could render unevenly spaced ticks, e.g. `0`, `300`, `1k` (gaps of 300 then 700) instead of `0`, `250`, `500`, `750`, `1k` — even though the underlying scale is linear. Recharts generates its own "nice" candidate tick values and then thins them via `minTickGap` to avoid overlapping labels, but that thinning drops individual candidates without re-spacing the survivors, so an already slightly uneven candidate set could come out badly uneven. + +The Y-axis now feeds Recharts a genuinely uniform set of candidate ticks (evenly dividing the domain into quarters) instead. Thinning a uniform sequence for overlap always keeps every Nth candidate, so however many ticks survive stay evenly spaced — this doesn't change how many ticks are shown, only where they land. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 2e5c6cb004..62473ed37c 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -864,6 +864,17 @@ export function formatAxisTick( }); } +const Y_AXIS_TICK_CANDIDATES = 5; + +// Recharts' minTickGap thinning drops ticks without re-spacing survivors; +// feeding it a uniform set keeps thinning's tick count but even spacing. +export function getEvenlySpacedTicks(min: number, max: number): number[] { + return Array.from( + { length: Y_AXIS_TICK_CANDIDATES }, + (_, i) => min + ((max - min) * i) / (Y_AXIS_TICK_CANDIDATES - 1), + ); +} + export const MemoChart = memo(function MemoChart({ graphResults, setIsClickActive, @@ -1051,11 +1062,19 @@ export const MemoChart = memo(function MemoChart({ const shouldFitYAxis = fitYAxisToData && displayType !== DisplayType.StackedBar; - // The data min/max is only needed to either zoom into a selection or to - // fit the lower bound to the data. When neither applies, let Recharts - // auto-calculate the upper bound while pinning the lower bound to zero. + // The lower bound stays pinned at zero unless selecting/fitting to + // data (below), so only the max is needed here, with the same headroom. if (!hasSelection && !shouldFitYAxis) { - return [0, 'auto']; + let maxValue = -Infinity; + graphResults.forEach(dataPoint => { + lineData.forEach(ld => { + const value = dataPoint[ld.dataKey]; + if (typeof value === 'number' && !isNaN(value)) { + maxValue = Math.max(maxValue, value); + } + }); + }); + return maxValue === -Infinity ? [0, 'auto'] : [0, maxValue * 1.05]; } // Calculate domain based on visible series (all series when there's no @@ -1217,6 +1236,15 @@ export const MemoChart = memo(function MemoChart({ [axisNumberFormat], ); + // Only overrides Recharts' own candidates for a concrete domain - a bare + // 'auto' bound has no fixed range to divide evenly. + const yAxisTicks = useMemo(() => { + const [min, max] = yAxisDomain; + return typeof min === 'number' && typeof max === 'number' + ? getEvenlySpacedTicks(min, max) + : undefined; + }, [yAxisDomain]); + const [highlightStart, setHighlightStart] = useState(); const [highlightEnd, setHighlightEnd] = useState(); const mouseDownPosRef = useRef(null); @@ -1670,6 +1698,7 @@ export const MemoChart = memo(function MemoChart({ tickFormatter={tickFormatter} tick={{ fontSize: 11, fontFamily: 'IBM Plex Mono, monospace' }} domain={yAxisDomain} + ticks={yAxisTicks} /> {lines} {/* HOVER tooltip (also drives cross-chart shadow tooltips via syncId). diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 2f8ac5df38..1ab5b20c2f 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -12,6 +12,7 @@ import { buildActiveClickSeries, collectMemoChartGradientHexes, formatAxisTick, + getEvenlySpacedTicks, getSelectedLineData, getVisibleLineData, getVisibleTooltipRows, @@ -160,6 +161,25 @@ describe('formatAxisTick', () => { }); }); +describe('getEvenlySpacedTicks', () => { + it('divides the domain into 5 uniform steps', () => { + expect(getEvenlySpacedTicks(0, 1000)).toEqual([0, 250, 500, 750, 1000]); + }); + + it('works for a non-zero lower bound', () => { + expect(getEvenlySpacedTicks(10, 30)).toEqual([10, 15, 20, 25, 30]); + }); + + it('reproduces the reported uneven-spacing case with a uniform set', () => { + // Regression: Recharts' own "nice" ticks for this domain rendered as + // 0/300/1k (gaps of 300 then 700) after minTickGap thinning. + const ticks = getEvenlySpacedTicks(0, 950); + for (let i = 1; i < ticks.length; i++) { + expect(ticks[i] - ticks[i - 1]).toBeCloseTo(ticks[1] - ticks[0]); + } + }); +}); + describe('collectMemoChartGradientHexes', () => { it('includes every categorical hex from COLORS up front', () => { const hexes = collectMemoChartGradientHexes([]); From c93fc2c2c633904371310c6b68ddd046530b73a0 Mon Sep 17 00:00:00 2001 From: arj22 Date: Mon, 21 Sep 2026 22:23:28 -0500 Subject: [PATCH 02/25] Round the Y-axis to nice steps; fix stacked-bar max; dedupe the scan Three review findings addressed: Critical: the new default-domain max was computed from the largest individual series value, but stacked bars sum at each timestamp (all share stackId="1") - a 60+40 stack reached 100 but got a domain top of 63. Stacked bars now defer to Recharts entirely, as before this PR. Major: dividing an unrounded domain (max * 1.05, or max + padding) into even quarters produced fractional labels (341/683/1k) or, worse, duplicate rounded ones (a peak of 3 gave 0/1/2/2/3). Ported the Graphics Gems "nice number" rounding already used by the CLI's termchart (packages/cli/src/termchart/scale.ts's niceTicks) so the domain rounds to a clean step before ticks are chosen from it. Minor: consolidated the two near-identical min/max scans (default vs. selection/fit-to-data) into one scanYAxisValueRange, parameterized by a visibility predicate, and replaced a tautological test (asserting an arithmetic sequence has equal gaps, which can't fail) with real domain/tick assertions for the reported peaks. Co-Authored-By: Claude Sonnet 5 --- .changeset/y-axis-even-tick-spacing.md | 6 +- packages/app/src/HDXMultiSeriesTimeChart.tsx | 203 +++++++++++------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 123 +++++++++-- 3 files changed, 235 insertions(+), 97 deletions(-) diff --git a/.changeset/y-axis-even-tick-spacing.md b/.changeset/y-axis-even-tick-spacing.md index 1ff6d3aacf..de33eb4a97 100644 --- a/.changeset/y-axis-even-tick-spacing.md +++ b/.changeset/y-axis-even-tick-spacing.md @@ -2,8 +2,8 @@ '@hyperdx/app': patch --- -fix: keep Y-axis ticks evenly spaced when Recharts thins them for overlap +fix: keep Y-axis ticks evenly spaced and cleanly rounded -A chart's Y-axis could render unevenly spaced ticks, e.g. `0`, `300`, `1k` (gaps of 300 then 700) instead of `0`, `250`, `500`, `750`, `1k` — even though the underlying scale is linear. Recharts generates its own "nice" candidate tick values and then thins them via `minTickGap` to avoid overlapping labels, but that thinning drops individual candidates without re-spacing the survivors, so an already slightly uneven candidate set could come out badly uneven. +A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k`, or `0, 341, 683, 1k`) instead of clean, even ones like `0, 250, 500, 750, 1k`. The axis now rounds its range to a "nice" step (1/2/2.5/5 × a power of 10, the same idea most charting libraries use) before choosing ticks, so labels land on round numbers and stay evenly spaced. Tick count is unaffected — this only changes which values are chosen. -The Y-axis now feeds Recharts a genuinely uniform set of candidate ticks (evenly dividing the domain into quarters) instead. Thinning a uniform sequence for overlap always keeps every Nth candidate, so however many ticks survive stay evenly spaced — this doesn't change how many ticks are shown, only where they land. +Also fixed: a stacked bar chart's Y-axis was computing its max from a single series instead of the summed stack height, which could clip bars past the top of the plot. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 62473ed37c..87e85fa740 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -864,15 +864,115 @@ export function formatAxisTick( }); } -const Y_AXIS_TICK_CANDIDATES = 5; +/** Round `x` to the nearest "nice" step: 1/2/2.5/5 x10^n. */ +function niceAxisStep(x: number): number { + const exp = Math.floor(Math.log10(x)); + const frac = x / 10 ** exp; + const nice = + frac < 1.5 ? 1 : frac < 2.25 ? 2 : frac < 3.75 ? 2.5 : frac < 7.5 ? 5 : 10; + return nice * 10 ** exp; +} -// Recharts' minTickGap thinning drops ticks without re-spacing survivors; -// feeding it a uniform set keeps thinning's tick count but even spacing. -export function getEvenlySpacedTicks(min: number, max: number): number[] { - return Array.from( - { length: Y_AXIS_TICK_CANDIDATES }, - (_, i) => min + ((max - min) * i) / (Y_AXIS_TICK_CANDIDATES - 1), - ); +export interface NiceYAxisBounds { + min: number; + max: number; + ticks: number[]; +} + +// Ported from packages/cli/src/termchart/scale.ts's niceTicks (Graphics +// Gems nice numbers) - rounds to a clean step, not a raw range fraction. +export function getNiceYAxisBounds( + min: number, + max: number, + maxTicks = 5, +): NiceYAxisBounds { + if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { + return { min, max, ticks: [] }; + } + const step = niceAxisStep((max - min) / (maxTicks - 1)); + const niceMin = Math.floor(min / step) * step; + const niceMax = Math.ceil(max / step) * step; + const count = Math.round((niceMax - niceMin) / step); + return { + min: niceMin, + max: niceMax, + ticks: Array.from({ length: count + 1 }, (_, i) => niceMin + i * step), + }; +} + +// Shared by every yAxisDomain branch - callers decide which series count +// and how the resulting range becomes a domain. +export function scanYAxisValueRange( + graphResults: any[], + lineData: LineData[], + isVisible: (seriesName: string) => boolean, +): { min: number; max: number } { + let min = Infinity; + let max = -Infinity; + graphResults.forEach(dataPoint => { + lineData.forEach(ld => { + const seriesName = ld.displayName || ld.dataKey; + if (!isVisible(seriesName)) return; + const value = dataPoint[ld.dataKey]; + if (typeof value === 'number' && !isNaN(value)) { + min = Math.min(min, value); + max = Math.max(max, value); + } + }); + }); + return { min, max }; +} + +export interface YAxisBounds { + domain: AxisDomain; + ticks: number[] | undefined; +} + +// An empty ticks array (flat data) must become undefined, not [] - an +// empty array renders no ticks at all; undefined lets Recharts decide. +function toYAxisBounds(bounds: NiceYAxisBounds): YAxisBounds { + return { + domain: [bounds.min, bounds.max], + ticks: bounds.ticks.length > 0 ? bounds.ticks : undefined, + }; +} + +// A stacked bar's rendered height sums its series at each timestamp, not +// any single one - leave that case entirely to Recharts, as before. +export function computeYAxisBounds( + graphResults: any[], + lineData: LineData[], + selectedSeriesNames: Set | undefined, + fitYAxisToData: boolean, + displayType: DisplayType, +): YAxisBounds { + const hasSelection = hasSeriesSelection(selectedSeriesNames); + const shouldFitYAxis = + fitYAxisToData && displayType !== DisplayType.StackedBar; + const isVisible = (seriesName: string): boolean => + !hasSelection || selectedSeriesNames.has(seriesName); + + if (!hasSelection && !shouldFitYAxis) { + if (displayType === DisplayType.StackedBar) { + return { domain: [0, 'auto'], ticks: undefined }; + } + const { max } = scanYAxisValueRange(graphResults, lineData, () => true); + if (max === -Infinity) { + return { domain: [0, 'auto'], ticks: undefined }; + } + return toYAxisBounds(getNiceYAxisBounds(0, max * 1.05)); + } + + const { min, max } = scanYAxisValueRange(graphResults, lineData, isVisible); + if (min === Infinity || max === -Infinity) { + return { domain: ['auto', 'auto'], ticks: undefined }; + } + const padding = (max - min) * 0.05; + // When fitting to data, allow the lower bound to follow the data + // minimum below zero; otherwise keep it pinned at zero either way. + const lowerBound = + shouldFitYAxis && min < 0 ? min - padding : Math.max(0, min - padding); + return toYAxisBounds(getNiceYAxisBounds(lowerBound, max + padding)); } export const MemoChart = memo(function MemoChart({ @@ -1053,72 +1153,18 @@ export const MemoChart = memo(function MemoChart({ ); }, [nearestSeriesKey, visibleLineData.length, id]); - const yAxisDomain: AxisDomain = useMemo(() => { - const hasSelection = hasSeriesSelection(selectedSeriesNames); - - // Fitting the y-axis lower bound to the data only applies to line charts. - // Bar charts are always anchored at zero so the bar lengths stay - // proportional to their values. - const shouldFitYAxis = - fitYAxisToData && displayType !== DisplayType.StackedBar; - - // The lower bound stays pinned at zero unless selecting/fitting to - // data (below), so only the max is needed here, with the same headroom. - if (!hasSelection && !shouldFitYAxis) { - let maxValue = -Infinity; - graphResults.forEach(dataPoint => { - lineData.forEach(ld => { - const value = dataPoint[ld.dataKey]; - if (typeof value === 'number' && !isNaN(value)) { - maxValue = Math.max(maxValue, value); - } - }); - }); - return maxValue === -Infinity ? [0, 'auto'] : [0, maxValue * 1.05]; - } - - // Calculate domain based on visible series (all series when there's no - // explicit selection). - let minValue = Infinity; - let maxValue = -Infinity; - - graphResults.forEach(dataPoint => { - lineData.forEach(ld => { - const seriesName = ld.displayName || ld.dataKey; - // Only consider visible series - if (!hasSelection || selectedSeriesNames.has(seriesName)) { - const value = dataPoint[ld.dataKey]; - if (typeof value === 'number' && !isNaN(value)) { - minValue = Math.min(minValue, value); - maxValue = Math.max(maxValue, value); - } - } - }); - }); - - // If we found valid values, return them with some padding - if (minValue !== Infinity && maxValue !== -Infinity) { - const padding = (maxValue - minValue) * 0.05; // 5% padding - // When fitting to data, allow the lower bound to follow the data - // minimum; otherwise keep it pinned at zero. The 5% padding must not - // drag the axis below zero unless the data itself is negative, so - // clamp at zero whenever the minimum is non-negative. - const lowerBound = - shouldFitYAxis && minValue < 0 - ? minValue - padding - : Math.max(0, minValue - padding); - const upperBound = maxValue + padding; - return [lowerBound, upperBound]; - } - - return ['auto', 'auto']; - }, [ - graphResults, - lineData, - selectedSeriesNames, - fitYAxisToData, - displayType, - ]); + const yAxisBounds = useMemo( + () => + computeYAxisBounds( + graphResults, + lineData, + selectedSeriesNames, + fitYAxisToData, + displayType, + ), + [graphResults, lineData, selectedSeriesNames, fitYAxisToData, displayType], + ); + const yAxisDomain = yAxisBounds.domain; const [containerWidth, setContainerWidth] = useState(0); @@ -1236,14 +1282,7 @@ export const MemoChart = memo(function MemoChart({ [axisNumberFormat], ); - // Only overrides Recharts' own candidates for a concrete domain - a bare - // 'auto' bound has no fixed range to divide evenly. - const yAxisTicks = useMemo(() => { - const [min, max] = yAxisDomain; - return typeof min === 'number' && typeof max === 'number' - ? getEvenlySpacedTicks(min, max) - : undefined; - }, [yAxisDomain]); + const yAxisTicks = yAxisBounds.ticks; const [highlightStart, setHighlightStart] = useState(); const [highlightEnd, setHighlightEnd] = useState(); diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 1ab5b20c2f..c7cc405bb2 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -6,18 +6,22 @@ // #2362 where a semantic-hex `lineData[].color` (e.g. the output of // `getChartColorInfo()` on HyperDX) would not have a matching gradient // def after the `COLORS` palette was unified to Observable 10. +import { DisplayType } from '@hyperdx/common-utils/dist/types'; + import type { LineData } from '@/ChartUtils'; import type { ActiveClickSeries } from '@/HDXMultiSeriesTimeChart'; import { buildActiveClickSeries, collectMemoChartGradientHexes, + computeYAxisBounds, formatAxisTick, - getEvenlySpacedTicks, + getNiceYAxisBounds, getSelectedLineData, getVisibleLineData, getVisibleTooltipRows, HARD_LINES_LIMIT, sameActiveClickSeries, + scanYAxisValueRange, } from '@/HDXMultiSeriesTimeChart'; import { COLORS } from '@/utils'; @@ -161,22 +165,117 @@ describe('formatAxisTick', () => { }); }); -describe('getEvenlySpacedTicks', () => { - it('divides the domain into 5 uniform steps', () => { - expect(getEvenlySpacedTicks(0, 1000)).toEqual([0, 250, 500, 750, 1000]); +describe('getNiceYAxisBounds', () => { + it('rounds to a clean step instead of dividing the raw range', () => { + // Regression: evenly dividing an unrounded range into quarters gave + // fractional labels (341/683/1k) or, worse, duplicate rounded ones. + expect(getNiceYAxisBounds(0, 1000)).toEqual({ + min: 0, + max: 1000, + ticks: [0, 250, 500, 750, 1000], + }); }); it('works for a non-zero lower bound', () => { - expect(getEvenlySpacedTicks(10, 30)).toEqual([10, 15, 20, 25, 30]); + expect(getNiceYAxisBounds(10, 30)).toEqual({ + min: 10, + max: 30, + ticks: [10, 15, 20, 25, 30], + }); }); - it('reproduces the reported uneven-spacing case with a uniform set', () => { - // Regression: Recharts' own "nice" ticks for this domain rendered as - // 0/300/1k (gaps of 300 then 700) after minTickGap thinning. - const ticks = getEvenlySpacedTicks(0, 950); - for (let i = 1; i < ticks.length; i++) { - expect(ticks[i] - ticks[i - 1]).toBeCloseTo(ticks[1] - ticks[0]); - } + it('returns no ticks for a degenerate (flat) range', () => { + expect(getNiceYAxisBounds(5, 5)).toEqual({ min: 5, max: 5, ticks: [] }); + }); +}); + +describe('scanYAxisValueRange', () => { + const series = (dataKey: string): LineData => ({ + dataKey, + currentPeriodKey: dataKey, + previousPeriodKey: `${dataKey}.prev`, + displayName: dataKey, + valueColumnName: dataKey, + color: '#a', + }); + + it('finds the min/max across every series when all are visible', () => { + const lineData = [series('a'), series('b')]; + const graphResults = [{ a: 10, b: 40 }, { a: -5 }]; + expect(scanYAxisValueRange(graphResults, lineData, () => true)).toEqual({ + min: -5, + max: 40, + }); + }); + + it('ignores a series the visibility predicate excludes', () => { + const lineData = [series('a'), series('b')]; + const graphResults = [{ a: 10, b: 40 }]; + expect( + scanYAxisValueRange(graphResults, lineData, name => name === 'a'), + ).toEqual({ min: 10, max: 10 }); + }); +}); + +describe('computeYAxisBounds', () => { + const series = (dataKey: string): LineData => ({ + dataKey, + currentPeriodKey: dataKey, + previousPeriodKey: `${dataKey}.prev`, + displayName: dataKey, + valueColumnName: dataKey, + color: '#a', + }); + + it('rounds the default domain to nice, non-duplicating ticks', () => { + // A peak of 3 divided into raw quarters rounds (mantissa 0) to + // 0/1/2/2/3 - two ticks reading "2" at different heights. + const bounds = computeYAxisBounds( + [{ a: 3 }], + [series('a')], + undefined, + false, + DisplayType.Line, + ); + expect(bounds).toEqual({ + domain: [0, 4], + ticks: [0, 1, 2, 3, 4], + }); + }); + + it('leaves a stacked bar`s max to Recharts instead of one series`', () => { + // Regression: bars sharing a stackId sum at each timestamp, so 60 and + // 40 individually reach a stack height of 100, not 63. + const bounds = computeYAxisBounds( + [{ a: 60, b: 40 }], + [series('a'), series('b')], + undefined, + false, + DisplayType.StackedBar, + ); + expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); + }); + + it('falls back to auto when there is no numeric data', () => { + const bounds = computeYAxisBounds( + [], + [], + undefined, + false, + DisplayType.Line, + ); + expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); + }); + + it('lets a fitted axis follow a negative minimum, still nicely rounded', () => { + const bounds = computeYAxisBounds( + [{ a: -50 }, { a: 200 }], + [series('a')], + undefined, + true, + DisplayType.Line, + ); + expect(bounds.domain).toEqual([-100, 250]); }); }); From 1415fb42068f242dc14fd2d7679b67fa755f7ca4 Mon Sep 17 00:00:00 2001 From: arj22 Date: Mon, 21 Sep 2026 22:46:50 -0500 Subject: [PATCH 03/25] Fix stacked-bar-with-selection, degenerate domains, undrawn-series scaling, and the 2.5 step Four issues from this round's review: Critical/P1 (duplicate reports): the stacked-bar guard only sat inside the no-selection branch, so a stacked bar WITH an active legend selection still fell through to per-series-max scanning. Moved the guard to the top of computeYAxisBounds so it applies unconditionally. Critical: getNiceYAxisBounds(0, 0) (all-zero data) or a negative max past its max<=min guard produced a literal [0, 0] or inverted [0, -5] domain instead of falling back - toYAxisBounds now falls back to each branch's own no-data sentinel ([0,'auto'] default, ['auto','auto'] fit-to-data) whenever the guard fires. Major: the max scan used the full lineData (up to 250 materialized series) instead of visibleLineData (selection- and HARD_LINES_LIMIT-filtered, i.e. what's actually drawn), so an undrawn series could scale the axis and squash the real lines to the bottom of the plot. Also let scanYAxisValueRange drop its now-unneeded isVisible predicate, since visibleLineData is already filtered. Major: the nice-step algorithm's 2.5 bucket produced ticks (e.g. 12.5) that formatAxisTick rounds to integers past its magnitude threshold ("13"), breaking the even progression this PR exists to fix. Dropped 2.5, matching the existing 1/2/5-only precedent in components/TimelineChart/utils.ts's calculateInterval. Co-Authored-By: Claude Sonnet 5 --- .changeset/y-axis-even-tick-spacing.md | 4 +- packages/app/src/HDXMultiSeriesTimeChart.tsx | 88 +++++++++++-------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 85 ++++++++++++------ 3 files changed, 112 insertions(+), 65 deletions(-) diff --git a/.changeset/y-axis-even-tick-spacing.md b/.changeset/y-axis-even-tick-spacing.md index de33eb4a97..03df59585d 100644 --- a/.changeset/y-axis-even-tick-spacing.md +++ b/.changeset/y-axis-even-tick-spacing.md @@ -4,6 +4,6 @@ fix: keep Y-axis ticks evenly spaced and cleanly rounded -A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k`, or `0, 341, 683, 1k`) instead of clean, even ones like `0, 250, 500, 750, 1k`. The axis now rounds its range to a "nice" step (1/2/2.5/5 × a power of 10, the same idea most charting libraries use) before choosing ticks, so labels land on round numbers and stay evenly spaced. Tick count is unaffected — this only changes which values are chosen. +A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k`, or `0, 341, 683, 1k`) instead of clean, even ones like `0, 250, 500, 750, 1k`. The axis now rounds its range to a "nice" step (1/2/5 × a power of 10, the same idea most charting libraries use) before choosing ticks, so labels land on round numbers and stay evenly spaced. Tick count is unaffected — this only changes which values are chosen. -Also fixed: a stacked bar chart's Y-axis was computing its max from a single series instead of the summed stack height, which could clip bars past the top of the plot. +Also fixed: a stacked bar chart's Y-axis was computing its max from a single series instead of the summed stack height (clipping bars), an all-zero or all-negative chart could get a collapsed or inverted domain, and the axis could scale to a series that isn't even drawn on the chart. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 87e85fa740..6e8652632c 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -864,12 +864,12 @@ export function formatAxisTick( }); } -/** Round `x` to the nearest "nice" step: 1/2/2.5/5 x10^n. */ +// Round `x` up to a "nice" step: 1/2/5 x10^n - no 2.5, since formatAxisTick +// forces integers past its magnitude threshold, rounding "12.5" to "13". function niceAxisStep(x: number): number { const exp = Math.floor(Math.log10(x)); const frac = x / 10 ** exp; - const nice = - frac < 1.5 ? 1 : frac < 2.25 ? 2 : frac < 3.75 ? 2.5 : frac < 7.5 ? 5 : 10; + const nice = frac <= 1 ? 1 : frac <= 2 ? 2 : frac <= 5 ? 5 : 10; return nice * 10 ** exp; } @@ -900,19 +900,16 @@ export function getNiceYAxisBounds( }; } -// Shared by every yAxisDomain branch - callers decide which series count -// and how the resulting range becomes a domain. +// Shared by every yAxisDomain branch below. Callers pass only the series +// actually drawn (already selection- and HARD_LINES_LIMIT-filtered). export function scanYAxisValueRange( graphResults: any[], lineData: LineData[], - isVisible: (seriesName: string) => boolean, ): { min: number; max: number } { let min = Infinity; let max = -Infinity; graphResults.forEach(dataPoint => { lineData.forEach(ld => { - const seriesName = ld.displayName || ld.dataKey; - if (!isVisible(seriesName)) return; const value = dataPoint[ld.dataKey]; if (typeof value === 'number' && !isNaN(value)) { min = Math.min(min, value); @@ -928,51 +925,60 @@ export interface YAxisBounds { ticks: number[] | undefined; } -// An empty ticks array (flat data) must become undefined, not [] - an -// empty array renders no ticks at all; undefined lets Recharts decide. -function toYAxisBounds(bounds: NiceYAxisBounds): YAxisBounds { - return { - domain: [bounds.min, bounds.max], - ticks: bounds.ticks.length > 0 ? bounds.ticks : undefined, - }; +const DEFAULT_Y_AXIS_BOUNDS: YAxisBounds = { + domain: [0, 'auto'], + ticks: undefined, +}; +const FIT_Y_AXIS_BOUNDS: YAxisBounds = { + domain: ['auto', 'auto'], + ticks: undefined, +}; + +// An empty ticks array (flat data) must fall back, not become a +// degenerate/inverted domain - use this branch's own no-data sentinel. +function toYAxisBounds( + bounds: NiceYAxisBounds, + fallback: YAxisBounds, +): YAxisBounds { + return bounds.ticks.length > 0 + ? { domain: [bounds.min, bounds.max], ticks: bounds.ticks } + : fallback; } -// A stacked bar's rendered height sums its series at each timestamp, not -// any single one - leave that case entirely to Recharts, as before. +// A stacked bar's rendered height sums its series at each timestamp - leave +// that entirely to Recharts, regardless of selection/fit-to-data state. export function computeYAxisBounds( graphResults: any[], - lineData: LineData[], - selectedSeriesNames: Set | undefined, + visibleLineData: LineData[], + hasSelection: boolean, fitYAxisToData: boolean, displayType: DisplayType, ): YAxisBounds { - const hasSelection = hasSeriesSelection(selectedSeriesNames); - const shouldFitYAxis = - fitYAxisToData && displayType !== DisplayType.StackedBar; - const isVisible = (seriesName: string): boolean => - !hasSelection || selectedSeriesNames.has(seriesName); + if (displayType === DisplayType.StackedBar) { + return DEFAULT_Y_AXIS_BOUNDS; + } + const shouldFitYAxis = fitYAxisToData; if (!hasSelection && !shouldFitYAxis) { - if (displayType === DisplayType.StackedBar) { - return { domain: [0, 'auto'], ticks: undefined }; - } - const { max } = scanYAxisValueRange(graphResults, lineData, () => true); - if (max === -Infinity) { - return { domain: [0, 'auto'], ticks: undefined }; - } - return toYAxisBounds(getNiceYAxisBounds(0, max * 1.05)); + const { max } = scanYAxisValueRange(graphResults, visibleLineData); + return max === -Infinity + ? DEFAULT_Y_AXIS_BOUNDS + : toYAxisBounds(getNiceYAxisBounds(0, max * 1.05), DEFAULT_Y_AXIS_BOUNDS); } - const { min, max } = scanYAxisValueRange(graphResults, lineData, isVisible); + const { min, max } = scanYAxisValueRange(graphResults, visibleLineData); if (min === Infinity || max === -Infinity) { - return { domain: ['auto', 'auto'], ticks: undefined }; + return FIT_Y_AXIS_BOUNDS; } const padding = (max - min) * 0.05; // When fitting to data, allow the lower bound to follow the data // minimum below zero; otherwise keep it pinned at zero either way. const lowerBound = shouldFitYAxis && min < 0 ? min - padding : Math.max(0, min - padding); - return toYAxisBounds(getNiceYAxisBounds(lowerBound, max + padding)); + return toYAxisBounds( + getNiceYAxisBounds(lowerBound, max + padding), + FIT_Y_AXIS_BOUNDS, + ); } export const MemoChart = memo(function MemoChart({ @@ -1157,12 +1163,18 @@ export const MemoChart = memo(function MemoChart({ () => computeYAxisBounds( graphResults, - lineData, - selectedSeriesNames, + visibleLineData, + hasSeriesSelection(selectedSeriesNames), fitYAxisToData, displayType, ), - [graphResults, lineData, selectedSeriesNames, fitYAxisToData, displayType], + [ + graphResults, + visibleLineData, + selectedSeriesNames, + fitYAxisToData, + displayType, + ], ); const yAxisDomain = yAxisBounds.domain; diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index c7cc405bb2..337dcdd34e 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -172,7 +172,7 @@ describe('getNiceYAxisBounds', () => { expect(getNiceYAxisBounds(0, 1000)).toEqual({ min: 0, max: 1000, - ticks: [0, 250, 500, 750, 1000], + ticks: [0, 500, 1000], }); }); @@ -199,22 +199,16 @@ describe('scanYAxisValueRange', () => { color: '#a', }); - it('finds the min/max across every series when all are visible', () => { + it('finds the min/max across every series it is given', () => { + // Callers pass only the drawn series (already selection- and + // HARD_LINES_LIMIT-filtered), so no visibility filtering happens here. const lineData = [series('a'), series('b')]; const graphResults = [{ a: 10, b: 40 }, { a: -5 }]; - expect(scanYAxisValueRange(graphResults, lineData, () => true)).toEqual({ + expect(scanYAxisValueRange(graphResults, lineData)).toEqual({ min: -5, max: 40, }); }); - - it('ignores a series the visibility predicate excludes', () => { - const lineData = [series('a'), series('b')]; - const graphResults = [{ a: 10, b: 40 }]; - expect( - scanYAxisValueRange(graphResults, lineData, name => name === 'a'), - ).toEqual({ min: 10, max: 10 }); - }); }); describe('computeYAxisBounds', () => { @@ -233,7 +227,7 @@ describe('computeYAxisBounds', () => { const bounds = computeYAxisBounds( [{ a: 3 }], [series('a')], - undefined, + false, false, DisplayType.Line, ); @@ -243,39 +237,80 @@ describe('computeYAxisBounds', () => { }); }); - it('leaves a stacked bar`s max to Recharts instead of one series`', () => { - // Regression: bars sharing a stackId sum at each timestamp, so 60 and - // 40 individually reach a stack height of 100, not 63. + it('never picks a step that formatAxisTick would round unevenly', () => { + // Regression: a 2.5 step gives "12.5", which formatAxisTick rounds to + // "13" past its magnitude threshold - an uneven-looking progression. const bounds = computeYAxisBounds( - [{ a: 60, b: 40 }], - [series('a'), series('b')], - undefined, + [{ a: 12 }], + [series('a')], false, - DisplayType.StackedBar, + false, + DisplayType.Line, ); - expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); + expect(bounds).toEqual({ + domain: [0, 15], + ticks: [0, 5, 10, 15], + }); + }); + + it('leaves a stacked bar`s max to Recharts, selection or not', () => { + // Regression: 60 and 40 individually reach a stack height of 100, not + // 63 - the guard must apply regardless of hasSelection. + const lineData = [series('a'), series('b')]; + const graphResults = [{ a: 60, b: 40 }]; + for (const hasSelection of [false, true]) { + expect( + computeYAxisBounds( + graphResults, + lineData, + hasSelection, + false, + DisplayType.StackedBar, + ), + ).toEqual({ domain: [0, 'auto'], ticks: undefined }); + } }); it('falls back to auto when there is no numeric data', () => { + const bounds = computeYAxisBounds([], [], false, false, DisplayType.Line); + expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); + }); + + it('falls back to auto for flat data instead of a degenerate domain', () => { + // Regression: getNiceYAxisBounds(0, 0) hit its max<=min guard and + // returned a literal [0, 0] domain, collapsing the axis to one point. const bounds = computeYAxisBounds( - [], - [], - undefined, + [{ a: 0 }, { a: 0 }], + [series('a')], + false, false, DisplayType.Line, ); expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); }); + it('falls back to auto for flat negative data on a fitted axis', () => { + // Regression: the same guard could also invert the domain (e.g. + // [0, -5]) for all-negative data padded past zero. + const bounds = computeYAxisBounds( + [{ a: -50 }, { a: -50 }], + [series('a')], + false, + true, + DisplayType.Line, + ); + expect(bounds).toEqual({ domain: ['auto', 'auto'], ticks: undefined }); + }); + it('lets a fitted axis follow a negative minimum, still nicely rounded', () => { const bounds = computeYAxisBounds( [{ a: -50 }, { a: 200 }], [series('a')], - undefined, + false, true, DisplayType.Line, ); - expect(bounds.domain).toEqual([-100, 250]); + expect(bounds.domain).toEqual([-100, 300]); }); }); From 867edaf3b8cba267b57838b9963145f7c6867b83 Mon Sep 17 00:00:00 2001 From: arj22 Date: Mon, 21 Sep 2026 22:48:30 -0500 Subject: [PATCH 04/25] Round off float dust in getNiceYAxisBounds Missed the toPrecision(12) cleanup the CLI's niceTicks (which this was ported from) already does - a 0.1-ish step could emit a tick like 0.6000000000000001 or a domain endpoint with the same dust. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 10 +++++++--- .../app/src/__tests__/HDXMultiSeriesTimeChart.test.ts | 9 +++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 6e8652632c..2c4002d736 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -890,13 +890,17 @@ export function getNiceYAxisBounds( return { min, max, ticks: [] }; } const step = niceAxisStep((max - min) / (maxTicks - 1)); - const niceMin = Math.floor(min / step) * step; - const niceMax = Math.ceil(max / step) * step; + // Rounds off float dust (e.g. a 0.1 step giving 0.6000000000000001). + const clean = (v: number) => Number(v.toPrecision(12)); + const niceMin = clean(Math.floor(min / step) * step); + const niceMax = clean(Math.ceil(max / step) * step); const count = Math.round((niceMax - niceMin) / step); return { min: niceMin, max: niceMax, - ticks: Array.from({ length: count + 1 }, (_, i) => niceMin + i * step), + ticks: Array.from({ length: count + 1 }, (_, i) => + clean(niceMin + i * step), + ), }; } diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 337dcdd34e..b1b058ad00 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -187,6 +187,15 @@ describe('getNiceYAxisBounds', () => { it('returns no ticks for a degenerate (flat) range', () => { expect(getNiceYAxisBounds(5, 5)).toEqual({ min: 5, max: 5, ticks: [] }); }); + + it('cleans up float dust from a fractional step', () => { + // Regression: without rounding, a 0.1-ish step gave 0.6000000000000001. + expect(getNiceYAxisBounds(0, 0.42)).toEqual({ + min: 0, + max: 0.6, + ticks: [0, 0.2, 0.4, 0.6], + }); + }); }); describe('scanYAxisValueRange', () => { From a812caa4c8da0859fad8fdf5c03715dc6dec9a5e Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 00:07:39 -0500 Subject: [PATCH 05/25] fix: keep Y-axis ticks nicely rounded without expanding the domain Nice-rounding ticks by expanding the domain regressed Fit-Y-Axis-to-Data (halved its effective zoom), added dead space/lost ticks on the default axis, and went stale once an alert reference line silently extends the domain. Ticks are now chosen within the existing (unexpanded) domain instead, and are skipped in favor of Recharts' own when a reference line is present. Also restores 2.5 as a valid step everywhere except the one magnitude where it produces non-integer ticks. Co-Authored-By: Claude Sonnet 5 --- .changeset/y-axis-even-tick-spacing.md | 2 +- packages/app/src/HDXMultiSeriesTimeChart.tsx | 105 ++++++++++-------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 88 ++++++++++----- 3 files changed, 119 insertions(+), 76 deletions(-) diff --git a/.changeset/y-axis-even-tick-spacing.md b/.changeset/y-axis-even-tick-spacing.md index 03df59585d..8ce577f8df 100644 --- a/.changeset/y-axis-even-tick-spacing.md +++ b/.changeset/y-axis-even-tick-spacing.md @@ -4,6 +4,6 @@ fix: keep Y-axis ticks evenly spaced and cleanly rounded -A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k`, or `0, 341, 683, 1k`) instead of clean, even ones like `0, 250, 500, 750, 1k`. The axis now rounds its range to a "nice" step (1/2/5 × a power of 10, the same idea most charting libraries use) before choosing ticks, so labels land on round numbers and stay evenly spaced. Tick count is unaffected — this only changes which values are chosen. +A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k`, or `0, 341, 683, 1k`) instead of clean, even ones like `0, 250, 500, 750, 1k`. Ticks are now chosen from a "nice" step (1/2/2.5/5 × a power of 10) within the chart's existing range, so labels land on round, evenly-spaced numbers without changing the axis's zoom level or count. Also fixed: a stacked bar chart's Y-axis was computing its max from a single series instead of the summed stack height (clipping bars), an all-zero or all-negative chart could get a collapsed or inverted domain, and the axis could scale to a series that isn't even drawn on the chart. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 2c4002d736..24c37b1238 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -864,44 +864,51 @@ export function formatAxisTick( }); } -// Round `x` up to a "nice" step: 1/2/5 x10^n - no 2.5, since formatAxisTick -// forces integers past its magnitude threshold, rounding "12.5" to "13". +// Rounds off float dust (e.g. 3 * 1.05 giving 3.1500000000000004). +const cleanNumber = (v: number) => Number(v.toPrecision(12)); + +// Round `x` up to a "nice" step: 1/2/(2.5)/5 x10^n. Only a step of exactly +// 2.5 is excluded - it's non-integer and formatAxisTick rounds "12.5" to "13". function niceAxisStep(x: number): number { const exp = Math.floor(Math.log10(x)); const frac = x / 10 ** exp; - const nice = frac <= 1 ? 1 : frac <= 2 ? 2 : frac <= 5 ? 5 : 10; + if (exp === 0) { + const nice = frac <= 1 ? 1 : frac <= 2 ? 2 : frac <= 5 ? 5 : 10; + return nice * 10 ** exp; + } + const nice = + frac <= 1 + ? 1 + : frac <= 2.25 + ? 2 + : frac <= 3.75 + ? 2.5 + : frac <= 7.5 + ? 5 + : 10; return nice * 10 ** exp; } -export interface NiceYAxisBounds { - min: number; - max: number; - ticks: number[]; -} - -// Ported from packages/cli/src/termchart/scale.ts's niceTicks (Graphics -// Gems nice numbers) - rounds to a clean step, not a raw range fraction. -export function getNiceYAxisBounds( +// Ported from packages/cli/src/termchart/scale.ts's niceTicks. Ticks stay +// within the given [min, max] rather than expanding it to the step boundary. +export function getNiceYAxisTicks( min: number, max: number, maxTicks = 5, -): NiceYAxisBounds { +): number[] { if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { - return { min, max, ticks: [] }; + return []; } const step = niceAxisStep((max - min) / (maxTicks - 1)); - // Rounds off float dust (e.g. a 0.1 step giving 0.6000000000000001). - const clean = (v: number) => Number(v.toPrecision(12)); - const niceMin = clean(Math.floor(min / step) * step); - const niceMax = clean(Math.ceil(max / step) * step); - const count = Math.round((niceMax - niceMin) / step); - return { - min: niceMin, - max: niceMax, - ticks: Array.from({ length: count + 1 }, (_, i) => - clean(niceMin + i * step), - ), - }; + const ticks: number[] = []; + for ( + let t = cleanNumber(Math.ceil(min / step) * step); + t <= max + step * 1e-9; + t = cleanNumber(t + step) + ) { + ticks.push(t); + } + return ticks; } // Shared by every yAxisDomain branch below. Callers pass only the series @@ -938,17 +945,6 @@ const FIT_Y_AXIS_BOUNDS: YAxisBounds = { ticks: undefined, }; -// An empty ticks array (flat data) must fall back, not become a -// degenerate/inverted domain - use this branch's own no-data sentinel. -function toYAxisBounds( - bounds: NiceYAxisBounds, - fallback: YAxisBounds, -): YAxisBounds { - return bounds.ticks.length > 0 - ? { domain: [bounds.min, bounds.max], ticks: bounds.ticks } - : fallback; -} - // A stacked bar's rendered height sums its series at each timestamp - leave // that entirely to Recharts, regardless of selection/fit-to-data state. export function computeYAxisBounds( @@ -957,6 +953,7 @@ export function computeYAxisBounds( hasSelection: boolean, fitYAxisToData: boolean, displayType: DisplayType, + hasReferenceLines: boolean, ): YAxisBounds { if (displayType === DisplayType.StackedBar) { return DEFAULT_Y_AXIS_BOUNDS; @@ -965,9 +962,17 @@ export function computeYAxisBounds( if (!hasSelection && !shouldFitYAxis) { const { max } = scanYAxisValueRange(graphResults, visibleLineData); - return max === -Infinity - ? DEFAULT_Y_AXIS_BOUNDS - : toYAxisBounds(getNiceYAxisBounds(0, max * 1.05), DEFAULT_Y_AXIS_BOUNDS); + const upperBound = cleanNumber(max * 1.05); + if (max === -Infinity || upperBound <= 0) { + return DEFAULT_Y_AXIS_BOUNDS; + } + const ticks = getNiceYAxisTicks(0, upperBound); + return { + domain: [0, upperBound], + // A reference line's ReferenceArea can silently extend this domain + // (extendDomain) at render time, making static ticks go stale. + ticks: hasReferenceLines || ticks.length === 0 ? undefined : ticks, + }; } const { min, max } = scanYAxisValueRange(graphResults, visibleLineData); @@ -977,12 +982,18 @@ export function computeYAxisBounds( const padding = (max - min) * 0.05; // When fitting to data, allow the lower bound to follow the data // minimum below zero; otherwise keep it pinned at zero either way. - const lowerBound = - shouldFitYAxis && min < 0 ? min - padding : Math.max(0, min - padding); - return toYAxisBounds( - getNiceYAxisBounds(lowerBound, max + padding), - FIT_Y_AXIS_BOUNDS, + const lowerBound = cleanNumber( + shouldFitYAxis && min < 0 ? min - padding : Math.max(0, min - padding), ); + const upperBound = cleanNumber(max + padding); + if (upperBound <= lowerBound) { + return FIT_Y_AXIS_BOUNDS; + } + const ticks = getNiceYAxisTicks(lowerBound, upperBound); + return { + domain: [lowerBound, upperBound], + ticks: hasReferenceLines || ticks.length === 0 ? undefined : ticks, + }; } export const MemoChart = memo(function MemoChart({ @@ -1171,6 +1182,9 @@ export const MemoChart = memo(function MemoChart({ hasSeriesSelection(selectedSeriesNames), fitYAxisToData, displayType, + Array.isArray(referenceLines) + ? referenceLines.length > 0 + : referenceLines != null, ), [ graphResults, @@ -1178,6 +1192,7 @@ export const MemoChart = memo(function MemoChart({ selectedSeriesNames, fitYAxisToData, displayType, + referenceLines, ], ); const yAxisDomain = yAxisBounds.domain; diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index b1b058ad00..e4d5743062 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -15,7 +15,7 @@ import { collectMemoChartGradientHexes, computeYAxisBounds, formatAxisTick, - getNiceYAxisBounds, + getNiceYAxisTicks, getSelectedLineData, getVisibleLineData, getVisibleTooltipRows, @@ -165,36 +165,35 @@ describe('formatAxisTick', () => { }); }); -describe('getNiceYAxisBounds', () => { +describe('getNiceYAxisTicks', () => { it('rounds to a clean step instead of dividing the raw range', () => { // Regression: evenly dividing an unrounded range into quarters gave // fractional labels (341/683/1k) or, worse, duplicate rounded ones. - expect(getNiceYAxisBounds(0, 1000)).toEqual({ - min: 0, - max: 1000, - ticks: [0, 500, 1000], - }); + expect(getNiceYAxisTicks(0, 1000)).toEqual([0, 250, 500, 750, 1000]); }); it('works for a non-zero lower bound', () => { - expect(getNiceYAxisBounds(10, 30)).toEqual({ - min: 10, - max: 30, - ticks: [10, 15, 20, 25, 30], - }); + expect(getNiceYAxisTicks(10, 30)).toEqual([10, 15, 20, 25, 30]); + }); + + it('never expands past the given range, even if it clips ticks short', () => { + // A tight/fitted range shouldn't gain extra ticks beyond its own bounds. + expect(getNiceYAxisTicks(95, 205)).toEqual([100, 125, 150, 175, 200]); }); it('returns no ticks for a degenerate (flat) range', () => { - expect(getNiceYAxisBounds(5, 5)).toEqual({ min: 5, max: 5, ticks: [] }); + expect(getNiceYAxisTicks(5, 5)).toEqual([]); }); it('cleans up float dust from a fractional step', () => { // Regression: without rounding, a 0.1-ish step gave 0.6000000000000001. - expect(getNiceYAxisBounds(0, 0.42)).toEqual({ - min: 0, - max: 0.6, - ticks: [0, 0.2, 0.4, 0.6], - }); + expect(getNiceYAxisTicks(0, 0.42)).toEqual([0, 0.2, 0.4]); + }); + + it('avoids a step that formatAxisTick would round unevenly (12.5 -> "13")', () => { + // At this magnitude (exp === 0), a 2.5 step yields non-integer ticks + // that straddle formatAxisTick's forced-integer threshold of 10. + expect(getNiceYAxisTicks(0, 12.6)).toEqual([0, 5, 10]); }); }); @@ -230,7 +229,7 @@ describe('computeYAxisBounds', () => { color: '#a', }); - it('rounds the default domain to nice, non-duplicating ticks', () => { + it('rounds ticks to clean, non-duplicating values without expanding the domain', () => { // A peak of 3 divided into raw quarters rounds (mantissa 0) to // 0/1/2/2/3 - two ticks reading "2" at different heights. const bounds = computeYAxisBounds( @@ -239,29 +238,45 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, + false, ); expect(bounds).toEqual({ - domain: [0, 4], - ticks: [0, 1, 2, 3, 4], + domain: [0, 3.15], + ticks: [0, 1, 2, 3], }); }); it('never picks a step that formatAxisTick would round unevenly', () => { - // Regression: a 2.5 step gives "12.5", which formatAxisTick rounds to - // "13" past its magnitude threshold - an uneven-looking progression. + // Regression: a 2.5 step at this magnitude gives "12.5", which + // formatAxisTick rounds to "13" - an uneven-looking progression. const bounds = computeYAxisBounds( [{ a: 12 }], [series('a')], false, false, DisplayType.Line, + false, ); expect(bounds).toEqual({ - domain: [0, 15], - ticks: [0, 5, 10, 15], + domain: [0, 12.6], + ticks: [0, 5, 10], }); }); + it('omits explicit ticks when a reference line can extend the domain', () => { + // A ReferenceArea's ifOverflow="extendDomain" can silently widen the + // domain past what these ticks were computed from - defer to Recharts. + const bounds = computeYAxisBounds( + [{ a: 1000 }], + [series('a')], + false, + false, + DisplayType.Line, + true, + ); + expect(bounds).toEqual({ domain: [0, 1050], ticks: undefined }); + }); + it('leaves a stacked bar`s max to Recharts, selection or not', () => { // Regression: 60 and 40 individually reach a stack height of 100, not // 63 - the guard must apply regardless of hasSelection. @@ -275,25 +290,33 @@ describe('computeYAxisBounds', () => { hasSelection, false, DisplayType.StackedBar, + false, ), ).toEqual({ domain: [0, 'auto'], ticks: undefined }); } }); it('falls back to auto when there is no numeric data', () => { - const bounds = computeYAxisBounds([], [], false, false, DisplayType.Line); + const bounds = computeYAxisBounds( + [], + [], + false, + false, + DisplayType.Line, + false, + ); expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); }); it('falls back to auto for flat data instead of a degenerate domain', () => { - // Regression: getNiceYAxisBounds(0, 0) hit its max<=min guard and - // returned a literal [0, 0] domain, collapsing the axis to one point. + // Regression: an all-zero domain ([0, 0]) collapsed the axis to one point. const bounds = computeYAxisBounds( [{ a: 0 }, { a: 0 }], [series('a')], false, false, DisplayType.Line, + false, ); expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); }); @@ -307,19 +330,24 @@ describe('computeYAxisBounds', () => { false, true, DisplayType.Line, + false, ); expect(bounds).toEqual({ domain: ['auto', 'auto'], ticks: undefined }); }); - it('lets a fitted axis follow a negative minimum, still nicely rounded', () => { + it('lets a fitted axis follow a negative minimum without expanding the domain', () => { const bounds = computeYAxisBounds( [{ a: -50 }, { a: 200 }], [series('a')], false, true, DisplayType.Line, + false, ); - expect(bounds.domain).toEqual([-100, 300]); + expect(bounds).toEqual({ + domain: [-62.5, 212.5], + ticks: [-50, 0, 50, 100, 150, 200], + }); }); }); From cde1d172d93c32e540266db34bf43ffede916636 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 00:09:37 -0500 Subject: [PATCH 06/25] fix: keep the zero-pinned fallback for flat, selected, non-fitted data A selection with fit-to-data off degenerated to the unpinned fit fallback (['auto','auto']) instead of the zero-pinned default, since only fitting itself should opt out of the zero baseline. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 9 +++++++-- .../src/__tests__/HDXMultiSeriesTimeChart.test.ts | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 24c37b1238..2d938e5cfd 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -975,9 +975,14 @@ export function computeYAxisBounds( }; } + // A selection with fit-to-data off still keeps the zero-pinned fallback, + // not the unpinned fit fallback - only fitting itself opts out of it. + const degenerateFallback = shouldFitYAxis + ? FIT_Y_AXIS_BOUNDS + : DEFAULT_Y_AXIS_BOUNDS; const { min, max } = scanYAxisValueRange(graphResults, visibleLineData); if (min === Infinity || max === -Infinity) { - return FIT_Y_AXIS_BOUNDS; + return degenerateFallback; } const padding = (max - min) * 0.05; // When fitting to data, allow the lower bound to follow the data @@ -987,7 +992,7 @@ export function computeYAxisBounds( ); const upperBound = cleanNumber(max + padding); if (upperBound <= lowerBound) { - return FIT_Y_AXIS_BOUNDS; + return degenerateFallback; } const ticks = getNiceYAxisTicks(lowerBound, upperBound); return { diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index e4d5743062..26183875c0 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -335,6 +335,20 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: ['auto', 'auto'], ticks: undefined }); }); + it('keeps the zero-pinned fallback for flat selected data when not fitting', () => { + // Regression: this case used the fit fallback (`['auto','auto']`) + // even though only fit-to-data, not selection, should opt out of it. + const bounds = computeYAxisBounds( + [{ a: 100 }, { a: 100 }], + [series('a')], + true, + false, + DisplayType.Line, + false, + ); + expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); + }); + it('lets a fitted axis follow a negative minimum without expanding the domain', () => { const bounds = computeYAxisBounds( [{ a: -50 }, { a: 200 }], From 91e8d08dcee583a1434db7a6c9c4f059d82695d2 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 00:16:36 -0500 Subject: [PATCH 07/25] fix: never let a nice tick step exceed maxTicks Rounding to the nearest 1/2/2.5/5 step could pick one smaller than the raw target, overflowing past maxTicks (e.g. 6 ticks for a 0-1480 range). Now searches nearby nice steps and picks the smallest one whose actual generated ticks still fit the limit. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 58 +++++++++---------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 12 +++- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 2d938e5cfd..3a3c3f29c5 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -867,30 +867,30 @@ export function formatAxisTick( // Rounds off float dust (e.g. 3 * 1.05 giving 3.1500000000000004). const cleanNumber = (v: number) => Number(v.toPrecision(12)); -// Round `x` up to a "nice" step: 1/2/(2.5)/5 x10^n. Only a step of exactly -// 2.5 is excluded - it's non-integer and formatAxisTick rounds "12.5" to "13". -function niceAxisStep(x: number): number { - const exp = Math.floor(Math.log10(x)); - const frac = x / 10 ** exp; - if (exp === 0) { - const nice = frac <= 1 ? 1 : frac <= 2 ? 2 : frac <= 5 ? 5 : 10; - return nice * 10 ** exp; +// Every 1/2/5 x10^n step, plus 2.5 x10^n (excluding the bare value 2.5, +// which is non-integer and formatAxisTick rounds "12.5" to "13"). +function niceStepsNear(target: number): number[] { + const exp = Math.floor(Math.log10(target)); + return [exp - 1, exp, exp + 1] + .flatMap(e => [1, 2, 2.5, 5, 10].map(m => m * 10 ** e)) + .filter(step => step > 0 && step !== 2.5) + .sort((a, b) => a - b); +} + +function ticksWithinRange(step: number, min: number, max: number): number[] { + const ticks: number[] = []; + for ( + let t = cleanNumber(Math.ceil(min / step) * step); + t <= max + step * 1e-9; + t = cleanNumber(t + step) + ) { + ticks.push(t); } - const nice = - frac <= 1 - ? 1 - : frac <= 2.25 - ? 2 - : frac <= 3.75 - ? 2.5 - : frac <= 7.5 - ? 5 - : 10; - return nice * 10 ** exp; + return ticks; } -// Ported from packages/cli/src/termchart/scale.ts's niceTicks. Ticks stay -// within the given [min, max] rather than expanding it to the step boundary. +// Ported from packages/cli/src/termchart/scale.ts's niceTicks, but picks the +// smallest nice step that still fits within [min, max] and maxTicks. export function getNiceYAxisTicks( min: number, max: number, @@ -899,16 +899,14 @@ export function getNiceYAxisTicks( if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { return []; } - const step = niceAxisStep((max - min) / (maxTicks - 1)); - const ticks: number[] = []; - for ( - let t = cleanNumber(Math.ceil(min / step) * step); - t <= max + step * 1e-9; - t = cleanNumber(t + step) - ) { - ticks.push(t); + const steps = niceStepsNear((max - min) / (maxTicks - 1)); + for (const step of steps) { + const ticks = ticksWithinRange(step, min, max); + if (ticks.length <= maxTicks) { + return ticks; + } } - return ticks; + return ticksWithinRange(steps[steps.length - 1], min, max); } // Shared by every yAxisDomain branch below. Callers pass only the series diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 26183875c0..0cf044cd03 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -186,8 +186,14 @@ describe('getNiceYAxisTicks', () => { }); it('cleans up float dust from a fractional step', () => { - // Regression: without rounding, a 0.1-ish step gave 0.6000000000000001. - expect(getNiceYAxisTicks(0, 0.42)).toEqual([0, 0.2, 0.4]); + // Regression: without rounding, a 0.1 step gave 0.30000000000000004. + expect(getNiceYAxisTicks(0, 0.42)).toEqual([0, 0.1, 0.2, 0.3, 0.4]); + }); + + it('never returns more than maxTicks ticks', () => { + // Regression: rounding to the nearest step could pick one below the + // raw target, overflowing to 6 ticks here instead of capping at 5. + expect(getNiceYAxisTicks(0, 1480).length).toBeLessThanOrEqual(5); }); it('avoids a step that formatAxisTick would round unevenly (12.5 -> "13")', () => { @@ -360,7 +366,7 @@ describe('computeYAxisBounds', () => { ); expect(bounds).toEqual({ domain: [-62.5, 212.5], - ticks: [-50, 0, 50, 100, 150, 200], + ticks: [0, 100, 200], }); }); }); From 0f9ae7bed9952ad056b9088277d374c158466564 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 00:34:55 -0500 Subject: [PATCH 08/25] fix: extend the zero-pinned Y-axis domain to cover negative data Recharts widens an explicit domain to fit out-of-range data (allowDataOverflow defaults to false), so a domain pinned at [0, upperBound] with negative data actually renders wider than that, stranding a domain-only tick list in a sliver at the top of the axis. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 14 ++++++++++---- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 3a3c3f29c5..b4aa0b2d29 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -959,14 +959,20 @@ export function computeYAxisBounds( const shouldFitYAxis = fitYAxisToData; if (!hasSelection && !shouldFitYAxis) { - const { max } = scanYAxisValueRange(graphResults, visibleLineData); + const { min, max } = scanYAxisValueRange(graphResults, visibleLineData); + if (max === -Infinity) { + return DEFAULT_Y_AXIS_BOUNDS; + } + // Recharts widens an explicit domain to fit out-of-range data, so + // negative data must be reflected here, not just pinned at zero. + const lowerBound = cleanNumber(Math.min(0, min)); const upperBound = cleanNumber(max * 1.05); - if (max === -Infinity || upperBound <= 0) { + if (upperBound <= lowerBound) { return DEFAULT_Y_AXIS_BOUNDS; } - const ticks = getNiceYAxisTicks(0, upperBound); + const ticks = getNiceYAxisTicks(lowerBound, upperBound); return { - domain: [0, upperBound], + domain: [lowerBound, upperBound], // A reference line's ReferenceArea can silently extend this domain // (extendDomain) at render time, making static ticks go stale. ticks: hasReferenceLines || ticks.length === 0 ? undefined : ticks, diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 0cf044cd03..13218795a9 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -269,6 +269,23 @@ describe('computeYAxisBounds', () => { }); }); + it('extends the zero-pinned domain to cover negative data too', () => { + // Regression: Recharts widens a [0, upperBound] domain to fit negative + // data, stranding a domain-only tick list in a sliver at the top. + const bounds = computeYAxisBounds( + [{ a: -100 }, { a: 1 }], + [series('a')], + false, + false, + DisplayType.Line, + false, + ); + expect(bounds).toEqual({ + domain: [-100, 1.05], + ticks: [-100, -75, -50, -25, 0], + }); + }); + it('omits explicit ticks when a reference line can extend the domain', () => { // A ReferenceArea's ifOverflow="extendDomain" can silently widen the // domain past what these ticks were computed from - defer to Recharts. From e4ddaaff88ad1c03617c747df6bffcc0f1cee6bd Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 00:57:49 -0500 Subject: [PATCH 09/25] fix: pad an all-negative series' upper bound away from zero max * 1.05 made the upper bound more negative than max itself for an all-negative series (-900 * 1.05 = -945), excluding the data's own maximum from the domain and stranding the precomputed ticks once Recharts widened the rendered domain back up to it. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 4 +++- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index b4aa0b2d29..75310414cf 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -966,7 +966,9 @@ export function computeYAxisBounds( // Recharts widens an explicit domain to fit out-of-range data, so // negative data must be reflected here, not just pinned at zero. const lowerBound = cleanNumber(Math.min(0, min)); - const upperBound = cleanNumber(max * 1.05); + // max * 1.05 would shrink the upper bound below max for negative data; + // padding away from zero keeps headroom regardless of max's sign. + const upperBound = cleanNumber(max + Math.abs(max) * 0.05); if (upperBound <= lowerBound) { return DEFAULT_Y_AXIS_BOUNDS; } diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 13218795a9..2389f29628 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -286,6 +286,23 @@ describe('computeYAxisBounds', () => { }); }); + it('pads an all-negative series away from zero, not toward it', () => { + // Regression: max * 1.05 made the upper bound *more* negative than max + // itself (-900 * 1.05 = -945), excluding the data's own maximum. + const bounds = computeYAxisBounds( + [{ a: -1000 }, { a: -900 }], + [series('a')], + false, + false, + DisplayType.Line, + false, + ); + expect(bounds).toEqual({ + domain: [-1000, -855], + ticks: [-1000, -950, -900], + }); + }); + it('omits explicit ticks when a reference line can extend the domain', () => { // A ReferenceArea's ifOverflow="extendDomain" can silently widen the // domain past what these ticks were computed from - defer to Recharts. From 1dff27228ac02454dbebb836a54ac8661469aa71 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 07:54:18 -0500 Subject: [PATCH 10/25] fix: three remaining major Y-axis regressions - Reference lines: an explicit numeric domain skips Recharts' own nice rounding even with ticks suppressed, reproducing the exact uneven spacing this PR fixes on every alert chart. Fall back to auto entirely when a reference line is present. - Selection branch: mirror the default branch's negative-data fix - the lower bound must follow negative data regardless of fit mode, since Recharts widens the domain to it either way. - Reject any candidate step whose formatted labels collide (e.g. 1500 and 2000 both rendering "2k"), not just the literal 2.5 step. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 46 +++++++++++----- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 55 +++++++++++++++++-- 2 files changed, 84 insertions(+), 17 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 75310414cf..b12688e47a 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -889,12 +889,22 @@ function ticksWithinRange(step: number, min: number, max: number): number[] { return ticks; } -// Ported from packages/cli/src/termchart/scale.ts's niceTicks, but picks the -// smallest nice step that still fits within [min, max] and maxTicks. +// A coarser step can still format two ticks to the same label (e.g. 1500 +// and 2000 both averaging to "2k") even though the raw numbers differ. +function hasDistinctLabels( + ticks: number[], + formatTick?: (value: number) => string, +): boolean { + return !formatTick || new Set(ticks.map(formatTick)).size === ticks.length; +} + +// Ported from packages/cli/src/termchart/scale.ts's niceTicks: the smallest +// step that fits within [min, max]/maxTicks and formats to distinct labels. export function getNiceYAxisTicks( min: number, max: number, maxTicks = 5, + formatTick?: (value: number) => string, ): number[] { if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { return []; @@ -902,11 +912,11 @@ export function getNiceYAxisTicks( const steps = niceStepsNear((max - min) / (maxTicks - 1)); for (const step of steps) { const ticks = ticksWithinRange(step, min, max); - if (ticks.length <= maxTicks) { + if (ticks.length <= maxTicks && hasDistinctLabels(ticks, formatTick)) { return ticks; } } - return ticksWithinRange(steps[steps.length - 1], min, max); + return []; } // Shared by every yAxisDomain branch below. Callers pass only the series @@ -952,13 +962,20 @@ export function computeYAxisBounds( fitYAxisToData: boolean, displayType: DisplayType, hasReferenceLines: boolean, + axisNumberFormat?: NumberFormat, ): YAxisBounds { if (displayType === DisplayType.StackedBar) { return DEFAULT_Y_AXIS_BOUNDS; } const shouldFitYAxis = fitYAxisToData; + const formatTick = (value: number) => formatAxisTick(value, axisNumberFormat); if (!hasSelection && !shouldFitYAxis) { + // A fully numeric domain skips Recharts' own nice rounding, and a + // reference line can extend it further - defer to Recharts entirely. + if (hasReferenceLines) { + return DEFAULT_Y_AXIS_BOUNDS; + } const { min, max } = scanYAxisValueRange(graphResults, visibleLineData); if (max === -Infinity) { return DEFAULT_Y_AXIS_BOUNDS; @@ -972,12 +989,10 @@ export function computeYAxisBounds( if (upperBound <= lowerBound) { return DEFAULT_Y_AXIS_BOUNDS; } - const ticks = getNiceYAxisTicks(lowerBound, upperBound); + const ticks = getNiceYAxisTicks(lowerBound, upperBound, 5, formatTick); return { domain: [lowerBound, upperBound], - // A reference line's ReferenceArea can silently extend this domain - // (extendDomain) at render time, making static ticks go stale. - ticks: hasReferenceLines || ticks.length === 0 ? undefined : ticks, + ticks: ticks.length === 0 ? undefined : ticks, }; } @@ -986,24 +1001,27 @@ export function computeYAxisBounds( const degenerateFallback = shouldFitYAxis ? FIT_Y_AXIS_BOUNDS : DEFAULT_Y_AXIS_BOUNDS; + if (hasReferenceLines) { + return degenerateFallback; + } const { min, max } = scanYAxisValueRange(graphResults, visibleLineData); if (min === Infinity || max === -Infinity) { return degenerateFallback; } const padding = (max - min) * 0.05; - // When fitting to data, allow the lower bound to follow the data - // minimum below zero; otherwise keep it pinned at zero either way. + // Recharts widens the domain to actual negative data regardless of fit + // mode, so the lower bound must follow it whenever min itself is negative. const lowerBound = cleanNumber( - shouldFitYAxis && min < 0 ? min - padding : Math.max(0, min - padding), + min < 0 ? min - padding : Math.max(0, min - padding), ); const upperBound = cleanNumber(max + padding); if (upperBound <= lowerBound) { return degenerateFallback; } - const ticks = getNiceYAxisTicks(lowerBound, upperBound); + const ticks = getNiceYAxisTicks(lowerBound, upperBound, 5, formatTick); return { domain: [lowerBound, upperBound], - ticks: hasReferenceLines || ticks.length === 0 ? undefined : ticks, + ticks: ticks.length === 0 ? undefined : ticks, }; } @@ -1196,6 +1214,7 @@ export const MemoChart = memo(function MemoChart({ Array.isArray(referenceLines) ? referenceLines.length > 0 : referenceLines != null, + axisNumberFormat, ), [ graphResults, @@ -1204,6 +1223,7 @@ export const MemoChart = memo(function MemoChart({ fitYAxisToData, displayType, referenceLines, + axisNumberFormat, ], ); const yAxisDomain = yAxisBounds.domain; diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 2389f29628..f543d14084 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -303,9 +303,9 @@ describe('computeYAxisBounds', () => { }); }); - it('omits explicit ticks when a reference line can extend the domain', () => { - // A ReferenceArea's ifOverflow="extendDomain" can silently widen the - // domain past what these ticks were computed from - defer to Recharts. + it('defers entirely to Recharts when a reference line is present', () => { + // A fully numeric domain reproduces the exact uneven spacing this PR + // fixes, and a reference line can extend it further - fall back to auto. const bounds = computeYAxisBounds( [{ a: 1000 }], [series('a')], @@ -314,7 +314,7 @@ describe('computeYAxisBounds', () => { DisplayType.Line, true, ); - expect(bounds).toEqual({ domain: [0, 1050], ticks: undefined }); + expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); }); it('leaves a stacked bar`s max to Recharts, selection or not', () => { @@ -403,6 +403,53 @@ describe('computeYAxisBounds', () => { ticks: [0, 100, 200], }); }); + + it('extends a selection`s lower bound to negative data even without fitting', () => { + // Regression: `Math.max(0, ...)` pinned this at 0 even though min < 0, + // but Recharts widens the domain to the real minimum regardless. + const bounds = computeYAxisBounds( + [{ a: -100 }, { a: 100 }], + [series('a')], + true, + false, + DisplayType.Line, + false, + ); + expect(bounds).toEqual({ + domain: [-110, 110], + ticks: [-100, -50, 0, 50, 100], + }); + }); + + it('defers entirely to Recharts when a reference line is present, fitted or selected', () => { + const bounds = computeYAxisBounds( + [{ a: 100 }, { a: 200 }], + [series('a')], + false, + true, + DisplayType.Line, + true, + ); + expect(bounds).toEqual({ domain: ['auto', 'auto'], ticks: undefined }); + }); + + it('rejects a step whose formatted labels collide, even if the count fits', () => { + // Regression: a step of 500 gives ticks 0/500/1000/1500/2000, which + // formatAxisTick's compact averaging renders as "0,500,1k,2k,2k". + const bounds = computeYAxisBounds( + [{ a: 2000 }], + [series('a')], + false, + false, + DisplayType.Line, + false, + { output: 'number' }, + ); + const labels = (bounds.ticks ?? []).map(t => + formatAxisTick(t, { output: 'number' }), + ); + expect(new Set(labels).size).toBe(labels.length); + }); }); describe('collectMemoChartGradientHexes', () => { From a0efd453c1e876db177257dbd638212bebd6e7d3 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 08:19:47 -0500 Subject: [PATCH 11/25] fix: stop ticksWithinRange from looping forever at extreme magnitudes cleanNumber's 12-significant-digit rounding can make t + step round back to t once |t| is large enough relative to step, so the loop's increment silently stopped advancing while still pushing to the ticks array - a frozen tab followed by OOM, reachable from the selection/fit branch's flat-high-baseline case. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 14 +++++++++----- .../src/__tests__/HDXMultiSeriesTimeChart.test.ts | 9 +++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index b12688e47a..bbaa35b9d5 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -879,12 +879,16 @@ function niceStepsNear(target: number): number[] { function ticksWithinRange(step: number, min: number, max: number): number[] { const ticks: number[] = []; - for ( - let t = cleanNumber(Math.ceil(min / step) * step); - t <= max + step * 1e-9; - t = cleanNumber(t + step) - ) { + let t = cleanNumber(Math.ceil(min / step) * step); + while (t <= max + step * 1e-9) { ticks.push(t); + const next = cleanNumber(t + step); + // At extreme magnitudes, float precision can make this step a no-op - + // stop rather than push the same value forever. + if (next <= t) { + break; + } + t = next; } return ticks; } diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index f543d14084..259b2ef799 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -196,6 +196,15 @@ describe('getNiceYAxisTicks', () => { expect(getNiceYAxisTicks(0, 1480).length).toBeLessThanOrEqual(5); }); + it('terminates instead of looping forever at extreme magnitudes', () => { + // Regression: cleanNumber's 12-significant-digit rounding can make + // t + step round back to t, so the generation loop never advanced. + expect(() => getNiceYAxisTicks(999999999999, 1000000000030)).not.toThrow(); + expect( + getNiceYAxisTicks(999999999999, 1000000000030).length, + ).toBeLessThanOrEqual(5); + }); + it('avoids a step that formatAxisTick would round unevenly (12.5 -> "13")', () => { // At this magnitude (exp === 0), a 2.5 step yields non-integer ticks // that straddle formatAxisTick's forced-integer threshold of 10. From d8a2c4547a39377f5e1306950a1bc31bc738b9e0 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 08:33:49 -0500 Subject: [PATCH 12/25] fix: reject a precision-collapsed step instead of accepting its truncation ticksWithinRange's non-advancing-step guard returned whatever ticks it had collected so far, which getNiceYAxisTicks then accepted as a legitimately coarse (often single-tick) result instead of trying a larger, representable step. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 18 ++++++++++++++---- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 13 ++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index bbaa35b9d5..3abf3225a4 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -877,16 +877,22 @@ function niceStepsNear(target: number): number[] { .sort((a, b) => a - b); } -function ticksWithinRange(step: number, min: number, max: number): number[] { +// null means the step is unusable at this magnitude (see below), which +// getNiceYAxisTicks must treat as a rejection, not as a short tick list. +function ticksWithinRange( + step: number, + min: number, + max: number, +): number[] | null { const ticks: number[] = []; let t = cleanNumber(Math.ceil(min / step) * step); while (t <= max + step * 1e-9) { ticks.push(t); const next = cleanNumber(t + step); // At extreme magnitudes, float precision can make this step a no-op - - // stop rather than push the same value forever. + // reject it rather than accept a truncated, collapsed tick list. if (next <= t) { - break; + return null; } t = next; } @@ -916,7 +922,11 @@ export function getNiceYAxisTicks( const steps = niceStepsNear((max - min) / (maxTicks - 1)); for (const step of steps) { const ticks = ticksWithinRange(step, min, max); - if (ticks.length <= maxTicks && hasDistinctLabels(ticks, formatTick)) { + if ( + ticks && + ticks.length <= maxTicks && + hasDistinctLabels(ticks, formatTick) + ) { return ticks; } } diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 259b2ef799..1ec3aaeb6e 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -196,13 +196,12 @@ describe('getNiceYAxisTicks', () => { expect(getNiceYAxisTicks(0, 1480).length).toBeLessThanOrEqual(5); }); - it('terminates instead of looping forever at extreme magnitudes', () => { - // Regression: cleanNumber's 12-significant-digit rounding can make - // t + step round back to t, so the generation loop never advanced. - expect(() => getNiceYAxisTicks(999999999999, 1000000000030)).not.toThrow(); - expect( - getNiceYAxisTicks(999999999999, 1000000000030).length, - ).toBeLessThanOrEqual(5); + it('rejects a precision-collapsed step instead of looping or truncating', () => { + // Regression: accepting the truncated result of a precision-collapsed + // step gave a one-tick axis instead of trying a coarser step. + expect(getNiceYAxisTicks(999999999999, 1000000000030)).toEqual([ + 1000000000000, 1000000000010, 1000000000020, 1000000000030, + ]); }); it('avoids a step that formatAxisTick would round unevenly (12.5 -> "13")', () => { From edfae8a641f590e31a0a3425eabce03bb3169442 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 09:50:55 -0500 Subject: [PATCH 13/25] fix: expand the default axis's upper bound to fill tick-count slack The tight-domain search could pick a step with unused capacity (e.g. 3 of 5 allowed ticks) while leaving a step's worth of dead space between the last tick and the domain edge - a max of 1200 gave ticks [0,500,1000] over domain [0,1260], 20% unlabeled at the top. Now rounds the upper bound up to that step's next boundary whenever doing so still fits within maxTicks, closing the gap without ever exceeding it (a tight step with no slack, like max=1000's 5-tick result, is untouched). Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 51 +++++++++++++++++-- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 31 ++++++++--- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 3abf3225a4..73bcafef8c 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -933,6 +933,46 @@ export function getNiceYAxisTicks( return []; } +export interface ExpandableYAxisTicks { + max: number; + ticks: number[]; +} + +// For the plain default branch (domain may expand, as [0,'auto'] did +// pre-PR): rounds the upper bound up to fill slack under maxTicks. +export function getExpandableYAxisTicks( + min: number, + max: number, + maxTicks = 5, + formatTick?: (value: number) => string, +): ExpandableYAxisTicks { + if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { + return { max, ticks: [] }; + } + const steps = niceStepsNear((max - min) / (maxTicks - 1)); + for (const step of steps) { + const tightTicks = ticksWithinRange(step, min, max); + if ( + !tightTicks || + tightTicks.length > maxTicks || + !hasDistinctLabels(tightTicks, formatTick) + ) { + continue; + } + const expandedMax = cleanNumber(Math.ceil(max / step) * step); + const expandedTicks = ticksWithinRange(step, min, expandedMax); + if ( + expandedTicks && + expandedTicks.length <= maxTicks && + hasDistinctLabels(expandedTicks, formatTick) + ) { + return { max: expandedMax, ticks: expandedTicks }; + } + return { max, ticks: tightTicks }; + } + return { max, ticks: [] }; +} + // Shared by every yAxisDomain branch below. Callers pass only the series // actually drawn (already selection- and HARD_LINES_LIMIT-filtered). export function scanYAxisValueRange( @@ -1003,10 +1043,15 @@ export function computeYAxisBounds( if (upperBound <= lowerBound) { return DEFAULT_Y_AXIS_BOUNDS; } - const ticks = getNiceYAxisTicks(lowerBound, upperBound, 5, formatTick); + const expanded = getExpandableYAxisTicks( + lowerBound, + upperBound, + 5, + formatTick, + ); return { - domain: [lowerBound, upperBound], - ticks: ticks.length === 0 ? undefined : ticks, + domain: [lowerBound, expanded.max], + ticks: expanded.ticks.length === 0 ? undefined : expanded.ticks, }; } diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 1ec3aaeb6e..ee118ad80b 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -243,7 +243,7 @@ describe('computeYAxisBounds', () => { color: '#a', }); - it('rounds ticks to clean, non-duplicating values without expanding the domain', () => { + it('rounds ticks to clean, non-duplicating values, expanding for slack', () => { // A peak of 3 divided into raw quarters rounds (mantissa 0) to // 0/1/2/2/3 - two ticks reading "2" at different heights. const bounds = computeYAxisBounds( @@ -255,8 +255,8 @@ describe('computeYAxisBounds', () => { false, ); expect(bounds).toEqual({ - domain: [0, 3.15], - ticks: [0, 1, 2, 3], + domain: [0, 4], + ticks: [0, 1, 2, 3, 4], }); }); @@ -272,8 +272,25 @@ describe('computeYAxisBounds', () => { false, ); expect(bounds).toEqual({ - domain: [0, 12.6], - ticks: [0, 5, 10], + domain: [0, 15], + ticks: [0, 5, 10, 15], + }); + }); + + it('expands past padding to avoid a sparse, dead-space-heavy axis', () => { + // Regression: a step landing between two step levels left only 3 + // ticks (0/500/1000) over domain [0,1260], 20% dead space at the top. + const bounds = computeYAxisBounds( + [{ a: 1200 }], + [series('a')], + false, + false, + DisplayType.Line, + false, + ); + expect(bounds).toEqual({ + domain: [0, 1500], + ticks: [0, 500, 1000, 1500], }); }); @@ -306,8 +323,8 @@ describe('computeYAxisBounds', () => { false, ); expect(bounds).toEqual({ - domain: [-1000, -855], - ticks: [-1000, -950, -900], + domain: [-1000, -850], + ticks: [-1000, -950, -900, -850], }); }); From 941ab1d1da5dbd8caef65e614b72a00b0917dc92 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 10:01:23 -0500 Subject: [PATCH 14/25] chore: tighten changeset to a short, current summary Per maintainer feedback on a sibling PR: changesets should stay to a single file with a 1-3 sentence description. This one had drifted stale relative to everything since fixed in this PR. Co-Authored-By: Claude Sonnet 5 --- .changeset/y-axis-even-tick-spacing.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.changeset/y-axis-even-tick-spacing.md b/.changeset/y-axis-even-tick-spacing.md index 8ce577f8df..fea71e3485 100644 --- a/.changeset/y-axis-even-tick-spacing.md +++ b/.changeset/y-axis-even-tick-spacing.md @@ -4,6 +4,4 @@ fix: keep Y-axis ticks evenly spaced and cleanly rounded -A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k`, or `0, 341, 683, 1k`) instead of clean, even ones like `0, 250, 500, 750, 1k`. Ticks are now chosen from a "nice" step (1/2/2.5/5 × a power of 10) within the chart's existing range, so labels land on round, evenly-spaced numbers without changing the axis's zoom level or count. - -Also fixed: a stacked bar chart's Y-axis was computing its max from a single series instead of the summed stack height (clipping bars), an all-zero or all-negative chart could get a collapsed or inverted domain, and the axis could scale to a series that isn't even drawn on the chart. +A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k` instead of `0, 250, 500, 750, 1k`). Ticks now round to clean, evenly-spaced values, and several related edge cases (stacked bars, negative or degenerate data, alert reference lines) that could produce a wrong or unlabeled axis are fixed too. From 18b2b3a87feb68d202ef2d78c88ef46574ccf934 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 10:15:51 -0500 Subject: [PATCH 15/25] fix: two remaining major Y-axis regressions, tighten changeset - Reference lines: only omit ticks, keep the computed tight domain. Bailing out to the full auto fallback silently disabled Fit-to-Data and legend-isolate zoom on every alert chart, since those always pass a reference line. - Cap domain expansion at 25% extra headroom over the padded range, so a rejected step (label collision) falling back to a coarser one can't push the axis top far past the data (max=2000 was expanding to domain 3000, a third of the plot empty). Also trimmed the changeset: it credited this PR with fixing several edge cases that only ever existed mid-development on this same branch, never in a shipped release. Co-Authored-By: Claude Sonnet 5 --- .changeset/y-axis-even-tick-spacing.md | 2 +- packages/app/src/HDXMultiSeriesTimeChart.tsx | 14 +++++++---- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 24 ++++++++++--------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/.changeset/y-axis-even-tick-spacing.md b/.changeset/y-axis-even-tick-spacing.md index fea71e3485..adf150f8d6 100644 --- a/.changeset/y-axis-even-tick-spacing.md +++ b/.changeset/y-axis-even-tick-spacing.md @@ -4,4 +4,4 @@ fix: keep Y-axis ticks evenly spaced and cleanly rounded -A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k` instead of `0, 250, 500, 750, 1k`). Ticks now round to clean, evenly-spaced values, and several related edge cases (stacked bars, negative or degenerate data, alert reference lines) that could produce a wrong or unlabeled axis are fixed too. +A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k` instead of `0, 250, 500, 750, 1k`). Ticks now round to clean, evenly-spaced values instead. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 73bcafef8c..6cc2602dd4 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -960,6 +960,11 @@ export function getExpandableYAxisTicks( continue; } const expandedMax = cleanNumber(Math.ceil(max / step) * step); + // Filling a step's worth of dead space is fine, but not at the cost + // of a large fraction of the range - keep the tight result instead. + if (expandedMax - max > (max - min) * 0.25) { + return { max, ticks: tightTicks }; + } const expandedTicks = ticksWithinRange(step, min, expandedMax); if ( expandedTicks && @@ -1060,9 +1065,6 @@ export function computeYAxisBounds( const degenerateFallback = shouldFitYAxis ? FIT_Y_AXIS_BOUNDS : DEFAULT_Y_AXIS_BOUNDS; - if (hasReferenceLines) { - return degenerateFallback; - } const { min, max } = scanYAxisValueRange(graphResults, visibleLineData); if (min === Infinity || max === -Infinity) { return degenerateFallback; @@ -1077,7 +1079,11 @@ export function computeYAxisBounds( if (upperBound <= lowerBound) { return degenerateFallback; } - const ticks = getNiceYAxisTicks(lowerBound, upperBound, 5, formatTick); + // A reference line can extend this domain further (stale ticks), but + // Fit-to-Data/selection still need the tight domain itself to work. + const ticks = hasReferenceLines + ? [] + : getNiceYAxisTicks(lowerBound, upperBound, 5, formatTick); return { domain: [lowerBound, upperBound], ticks: ticks.length === 0 ? undefined : ticks, diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index ee118ad80b..f3a93c03d5 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -243,7 +243,7 @@ describe('computeYAxisBounds', () => { color: '#a', }); - it('rounds ticks to clean, non-duplicating values, expanding for slack', () => { + it('rounds ticks to clean, non-duplicating values', () => { // A peak of 3 divided into raw quarters rounds (mantissa 0) to // 0/1/2/2/3 - two ticks reading "2" at different heights. const bounds = computeYAxisBounds( @@ -255,8 +255,8 @@ describe('computeYAxisBounds', () => { false, ); expect(bounds).toEqual({ - domain: [0, 4], - ticks: [0, 1, 2, 3, 4], + domain: [0, 3.15], + ticks: [0, 1, 2, 3], }); }); @@ -446,7 +446,9 @@ describe('computeYAxisBounds', () => { }); }); - it('defers entirely to Recharts when a reference line is present, fitted or selected', () => { + it('keeps the fitted/selected domain when a reference line is present, only dropping ticks', () => { + // Regression: bailing out to the auto fallback (instead of just + // omitting ticks) disabled Fit-to-Data/legend-isolate on alert charts. const bounds = computeYAxisBounds( [{ a: 100 }, { a: 200 }], [series('a')], @@ -455,12 +457,12 @@ describe('computeYAxisBounds', () => { DisplayType.Line, true, ); - expect(bounds).toEqual({ domain: ['auto', 'auto'], ticks: undefined }); + expect(bounds).toEqual({ domain: [95, 205], ticks: undefined }); }); it('rejects a step whose formatted labels collide, even if the count fits', () => { - // Regression: a step of 500 gives ticks 0/500/1000/1500/2000, which - // formatAxisTick's compact averaging renders as "0,500,1k,2k,2k". + // Regression: step 500's "0,500,1k,2k,2k" labels collide; step 1000 + // was then wrongly expanded to domain 3000, a third of it empty. const bounds = computeYAxisBounds( [{ a: 2000 }], [series('a')], @@ -470,10 +472,10 @@ describe('computeYAxisBounds', () => { false, { output: 'number' }, ); - const labels = (bounds.ticks ?? []).map(t => - formatAxisTick(t, { output: 'number' }), - ); - expect(new Set(labels).size).toBe(labels.length); + expect(bounds).toEqual({ + domain: [0, 2100], + ticks: [0, 1000, 2000], + }); }); }); From 712623ed1323bbdfdf155832ac00077c8a39de5b Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 11:15:37 -0500 Subject: [PATCH 16/25] fix: never show duplicate Y-axis tick labels, escalating precision Grafana/Chronosphere never render two ticks with the same label - a narrow range at the configured decimal precision (e.g. a 2.9-3.4GB memory gauge at 0 decimals) collapsed every tick to "3 GB", or worse, a single tick with no scale at all. Tick selection now escalates past the configured mantissa (bypassing formatAxisTick's own forced-0 rule) when that's the only way to keep labels distinct, applied uniformly to the default and Fit-to-Data/ selection branches alike. A candidate step or expansion is now also rejected outright if it would leave fewer than 2 ticks - a one-tick axis is never an acceptable outcome, even though its label is trivially "distinct" from nothing. Co-Authored-By: Claude Sonnet 5 --- .changeset/y-axis-even-tick-spacing.md | 2 +- packages/app/src/HDXMultiSeriesTimeChart.tsx | 150 +++++++++++++----- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 84 ++++++++-- 3 files changed, 182 insertions(+), 54 deletions(-) diff --git a/.changeset/y-axis-even-tick-spacing.md b/.changeset/y-axis-even-tick-spacing.md index adf150f8d6..7e6d8bfa1f 100644 --- a/.changeset/y-axis-even-tick-spacing.md +++ b/.changeset/y-axis-even-tick-spacing.md @@ -4,4 +4,4 @@ fix: keep Y-axis ticks evenly spaced and cleanly rounded -A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k` instead of `0, 250, 500, 750, 1k`). Ticks now round to clean, evenly-spaced values instead. +A chart's Y-axis could render unevenly spaced or fractional ticks (e.g. `0, 300, 1k` instead of `0, 250, 500, 750, 1k`), or even show two ticks with the identical label. Ticks now round to clean, evenly-spaced, always-distinct values instead. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 6cc2602dd4..e57dc282bd 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -899,13 +899,69 @@ function ticksWithinRange( return ticks; } -// A coarser step can still format two ticks to the same label (e.g. 1500 -// and 2000 both averaging to "2k") even though the raw numbers differ. -function hasDistinctLabels( +// Escalates precision past the configured mantissa, bypassing +// formatAxisTick's own selection (which can force 0 regardless). +function formatTickAtMantissa( + value: number, + axisNumberFormat: NumberFormat, + mantissa: number, +): string { + return formatNumber(value, { + ...axisNumberFormat, + mantissa, + average: true, + unit: undefined, + }); +} + +// Ticks must never carry duplicate labels (Grafana/Chronosphere never do). +const MAX_TICK_MANTISSA_ESCALATION = 4; + +function isDistinct( ticks: number[], - formatTick?: (value: number) => string, + formatTick: (value: number) => string, ): boolean { - return !formatTick || new Set(ticks.map(formatTick)).size === ticks.length; + return new Set(ticks.map(formatTick)).size === ticks.length; +} + +// Prefers formatAxisTick's normal output, but escalates precision past the +// configured mantissa when that's the only way to keep labels distinct. +function resolveDistinctTickLabels( + ticks: number[], + axisNumberFormat: NumberFormat | undefined, +): ((value: number) => string) | null { + const base = (value: number) => formatAxisTick(value, axisNumberFormat); + if (isDistinct(ticks, base)) { + return base; + } + if (!axisNumberFormat) { + // No configured mantissa to escalate - fall back to full, non-compact + // precision, which always distinguishes any two different numbers. + const fullPrecision = (value: number) => + new Intl.NumberFormat('en-US').format(value); + return isDistinct(ticks, fullPrecision) ? fullPrecision : null; + } + if (axisNumberFormat.output === 'duration') { + return null; + } + const baseMantissa = Math.max(0, axisNumberFormat.mantissa ?? 0); + for ( + let m = baseMantissa + 1; + m <= baseMantissa + MAX_TICK_MANTISSA_ESCALATION; + m++ + ) { + const escalated = (value: number) => + formatTickAtMantissa(value, axisNumberFormat, m); + if (isDistinct(ticks, escalated)) { + return escalated; + } + } + return null; +} + +export interface NiceYAxisTicks { + ticks: number[]; + tickFormatter?: (value: number) => string; } // Ported from packages/cli/src/termchart/scale.ts's niceTicks: the smallest @@ -914,28 +970,31 @@ export function getNiceYAxisTicks( min: number, max: number, maxTicks = 5, - formatTick?: (value: number) => string, -): number[] { + axisNumberFormat?: NumberFormat, +): NiceYAxisTicks { if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { - return []; + return { ticks: [] }; } const steps = niceStepsNear((max - min) / (maxTicks - 1)); for (const step of steps) { const ticks = ticksWithinRange(step, min, max); - if ( - ticks && - ticks.length <= maxTicks && - hasDistinctLabels(ticks, formatTick) - ) { - return ticks; + // A single tick conveys no scale at all - never accept it, even + // though its "labels" are trivially distinct from one another. + if (!ticks || ticks.length < 2 || ticks.length > maxTicks) { + continue; + } + const tickFormatter = resolveDistinctTickLabels(ticks, axisNumberFormat); + if (tickFormatter) { + return { ticks, tickFormatter }; } } - return []; + return { ticks: [] }; } export interface ExpandableYAxisTicks { max: number; ticks: number[]; + tickFormatter?: (value: number) => string; } // For the plain default branch (domain may expand, as [0,'auto'] did @@ -944,7 +1003,7 @@ export function getExpandableYAxisTicks( min: number, max: number, maxTicks = 5, - formatTick?: (value: number) => string, + axisNumberFormat?: NumberFormat, ): ExpandableYAxisTicks { if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { return { max, ticks: [] }; @@ -952,28 +1011,38 @@ export function getExpandableYAxisTicks( const steps = niceStepsNear((max - min) / (maxTicks - 1)); for (const step of steps) { const tightTicks = ticksWithinRange(step, min, max); - if ( - !tightTicks || - tightTicks.length > maxTicks || - !hasDistinctLabels(tightTicks, formatTick) - ) { + // A single tick conveys no scale at all - never accept it, even + // though its "labels" are trivially distinct from one another. + if (!tightTicks || tightTicks.length < 2 || tightTicks.length > maxTicks) { + continue; + } + const tightFormatter = resolveDistinctTickLabels( + tightTicks, + axisNumberFormat, + ); + if (!tightFormatter) { continue; } const expandedMax = cleanNumber(Math.ceil(max / step) * step); // Filling a step's worth of dead space is fine, but not at the cost // of a large fraction of the range - keep the tight result instead. - if (expandedMax - max > (max - min) * 0.25) { - return { max, ticks: tightTicks }; - } - const expandedTicks = ticksWithinRange(step, min, expandedMax); - if ( - expandedTicks && - expandedTicks.length <= maxTicks && - hasDistinctLabels(expandedTicks, formatTick) - ) { - return { max: expandedMax, ticks: expandedTicks }; + if (expandedMax - max <= (max - min) * 0.25) { + const expandedTicks = ticksWithinRange(step, min, expandedMax); + if (expandedTicks && expandedTicks.length <= maxTicks) { + const expandedFormatter = resolveDistinctTickLabels( + expandedTicks, + axisNumberFormat, + ); + if (expandedFormatter) { + return { + max: expandedMax, + ticks: expandedTicks, + tickFormatter: expandedFormatter, + }; + } + } } - return { max, ticks: tightTicks }; + return { max, ticks: tightTicks, tickFormatter: tightFormatter }; } return { max, ticks: [] }; } @@ -1001,6 +1070,7 @@ export function scanYAxisValueRange( export interface YAxisBounds { domain: AxisDomain; ticks: number[] | undefined; + tickFormatter?: (value: number) => string; } const DEFAULT_Y_AXIS_BOUNDS: YAxisBounds = { @@ -1027,7 +1097,6 @@ export function computeYAxisBounds( return DEFAULT_Y_AXIS_BOUNDS; } const shouldFitYAxis = fitYAxisToData; - const formatTick = (value: number) => formatAxisTick(value, axisNumberFormat); if (!hasSelection && !shouldFitYAxis) { // A fully numeric domain skips Recharts' own nice rounding, and a @@ -1052,11 +1121,12 @@ export function computeYAxisBounds( lowerBound, upperBound, 5, - formatTick, + axisNumberFormat, ); return { domain: [lowerBound, expanded.max], ticks: expanded.ticks.length === 0 ? undefined : expanded.ticks, + tickFormatter: expanded.tickFormatter, }; } @@ -1081,12 +1151,13 @@ export function computeYAxisBounds( } // A reference line can extend this domain further (stale ticks), but // Fit-to-Data/selection still need the tight domain itself to work. - const ticks = hasReferenceLines - ? [] - : getNiceYAxisTicks(lowerBound, upperBound, 5, formatTick); + const { ticks, tickFormatter } = hasReferenceLines + ? { ticks: [], tickFormatter: undefined } + : getNiceYAxisTicks(lowerBound, upperBound, 5, axisNumberFormat); return { domain: [lowerBound, upperBound], ticks: ticks.length === 0 ? undefined : ticks, + tickFormatter, }; } @@ -1410,6 +1481,9 @@ export const MemoChart = memo(function MemoChart({ ); const yAxisTicks = yAxisBounds.ticks; + // Explicit ticks may need a precision-escalated formatter to stay + // distinct - see resolveDistinctTickLabels - so prefer it when present. + const yAxisTickFormatter = yAxisBounds.tickFormatter ?? tickFormatter; const [highlightStart, setHighlightStart] = useState(); const [highlightEnd, setHighlightEnd] = useState(); @@ -1861,7 +1935,7 @@ export const MemoChart = memo(function MemoChart({ { it('rounds to a clean step instead of dividing the raw range', () => { // Regression: evenly dividing an unrounded range into quarters gave // fractional labels (341/683/1k) or, worse, duplicate rounded ones. - expect(getNiceYAxisTicks(0, 1000)).toEqual([0, 250, 500, 750, 1000]); + expect(getNiceYAxisTicks(0, 1000).ticks).toEqual([0, 250, 500, 750, 1000]); }); it('works for a non-zero lower bound', () => { - expect(getNiceYAxisTicks(10, 30)).toEqual([10, 15, 20, 25, 30]); + expect(getNiceYAxisTicks(10, 30).ticks).toEqual([10, 15, 20, 25, 30]); }); it('never expands past the given range, even if it clips ticks short', () => { // A tight/fitted range shouldn't gain extra ticks beyond its own bounds. - expect(getNiceYAxisTicks(95, 205)).toEqual([100, 125, 150, 175, 200]); + expect(getNiceYAxisTicks(95, 205).ticks).toEqual([100, 125, 150, 175, 200]); }); it('returns no ticks for a degenerate (flat) range', () => { - expect(getNiceYAxisTicks(5, 5)).toEqual([]); + expect(getNiceYAxisTicks(5, 5).ticks).toEqual([]); }); it('cleans up float dust from a fractional step', () => { // Regression: without rounding, a 0.1 step gave 0.30000000000000004. - expect(getNiceYAxisTicks(0, 0.42)).toEqual([0, 0.1, 0.2, 0.3, 0.4]); + expect(getNiceYAxisTicks(0, 0.42).ticks).toEqual([0, 0.1, 0.2, 0.3, 0.4]); }); it('never returns more than maxTicks ticks', () => { // Regression: rounding to the nearest step could pick one below the // raw target, overflowing to 6 ticks here instead of capping at 5. - expect(getNiceYAxisTicks(0, 1480).length).toBeLessThanOrEqual(5); + expect(getNiceYAxisTicks(0, 1480).ticks.length).toBeLessThanOrEqual(5); }); it('rejects a precision-collapsed step instead of looping or truncating', () => { // Regression: accepting the truncated result of a precision-collapsed // step gave a one-tick axis instead of trying a coarser step. - expect(getNiceYAxisTicks(999999999999, 1000000000030)).toEqual([ + expect(getNiceYAxisTicks(999999999999, 1000000000030).ticks).toEqual([ 1000000000000, 1000000000010, 1000000000020, 1000000000030, ]); }); @@ -207,7 +207,35 @@ describe('getNiceYAxisTicks', () => { it('avoids a step that formatAxisTick would round unevenly (12.5 -> "13")', () => { // At this magnitude (exp === 0), a 2.5 step yields non-integer ticks // that straddle formatAxisTick's forced-integer threshold of 10. - expect(getNiceYAxisTicks(0, 12.6)).toEqual([0, 5, 10]); + expect(getNiceYAxisTicks(0, 12.6).ticks).toEqual([0, 5, 10]); + }); + + it('never accepts a single tick, even though it is trivially "distinct"', () => { + // Regression: a lone survivor's label can't collide with anything, + // so a naive distinctness check accepted a one-tick, no-scale axis. + const result = getNiceYAxisTicks(999999999999, 1000000000000.001); + expect(result.ticks.length === 0 || result.ticks.length >= 2).toBe(true); + }); + + it('escalates precision past the configured mantissa to keep labels distinct', () => { + // Regression: Grafana/Chronosphere never show two ticks with the same + // label - 0 decimals collapses a narrow byte range entirely to "3 GB". + const GB = 1024 ** 3; + const result = getNiceYAxisTicks(2.825 * GB, 3.45 * GB, 5, { + output: 'byte', + mantissa: 0, + }); + const labels = result.ticks.map(t => result.tickFormatter!(t)); + expect(result.ticks.length).toBeGreaterThanOrEqual(2); + expect(new Set(labels).size).toBe(labels.length); + }); + + it('falls back to full, non-compact precision when no format is configured', () => { + // Regression: with no mantissa to escalate, close-together magnitudes + // collapsed under Intl's compact notation (e.g. "1T") instead. + const result = getNiceYAxisTicks(999999999999, 1000000000030); + const labels = result.ticks.map(t => result.tickFormatter!(t)); + expect(new Set(labels).size).toBe(labels.length); }); }); @@ -257,6 +285,7 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: [0, 3.15], ticks: [0, 1, 2, 3], + tickFormatter: expect.any(Function), }); }); @@ -274,6 +303,7 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: [0, 15], ticks: [0, 5, 10, 15], + tickFormatter: expect.any(Function), }); }); @@ -291,6 +321,7 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: [0, 1500], ticks: [0, 500, 1000, 1500], + tickFormatter: expect.any(Function), }); }); @@ -308,6 +339,7 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: [-100, 1.05], ticks: [-100, -75, -50, -25, 0], + tickFormatter: expect.any(Function), }); }); @@ -325,6 +357,7 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: [-1000, -850], ticks: [-1000, -950, -900, -850], + tickFormatter: expect.any(Function), }); }); @@ -426,6 +459,7 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: [-62.5, 212.5], ticks: [0, 100, 200], + tickFormatter: expect.any(Function), }); }); @@ -443,6 +477,7 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: [-110, 110], ticks: [-100, -50, 0, 50, 100], + tickFormatter: expect.any(Function), }); }); @@ -460,9 +495,9 @@ describe('computeYAxisBounds', () => { expect(bounds).toEqual({ domain: [95, 205], ticks: undefined }); }); - it('rejects a step whose formatted labels collide, even if the count fits', () => { - // Regression: step 500's "0,500,1k,2k,2k" labels collide; step 1000 - // was then wrongly expanded to domain 3000, a third of it empty. + it('escalates precision instead of dropping a step whose labels collide', () => { + // Regression: step 500's "0,500,1k,2k,2k" labels collide - escalating + // precision keeps the denser step instead of falling back to a coarser one. const bounds = computeYAxisBounds( [{ a: 2000 }], [series('a')], @@ -472,10 +507,29 @@ describe('computeYAxisBounds', () => { false, { output: 'number' }, ); - expect(bounds).toEqual({ - domain: [0, 2100], - ticks: [0, 1000, 2000], - }); + expect(bounds.domain).toEqual([0, 2100]); + expect(bounds.ticks).toEqual([0, 500, 1000, 1500, 2000]); + const labels = bounds.ticks!.map(t => bounds.tickFormatter!(t)); + expect(new Set(labels).size).toBe(labels.length); + }); + + it('never collapses to a single tick, escalating precision or a wider step instead', () => { + // Regression: a narrow byte range at 0 decimals (2.9-3.4GB) all rounds + // to "3 GB" - never show duplicate ticks or a no-scale single tick. + const GB = 1024 ** 3; + const bounds = computeYAxisBounds( + [{ a: 2.9 * GB }, { a: 3.4 * GB }], + [series('a')], + false, + true, + DisplayType.Line, + false, + { output: 'byte', mantissa: 0 }, + ); + expect(bounds.ticks).toBeDefined(); + expect(bounds.ticks!.length).toBeGreaterThanOrEqual(2); + const labels = bounds.ticks!.map(t => bounds.tickFormatter!(t)); + expect(new Set(labels).size).toBe(labels.length); }); }); From a2d080f23bc9ea76e7820ea0aa0d0722bbec65d5 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 11:53:52 -0500 Subject: [PATCH 17/25] fix: escalate duration precision too, start numeric escalation minimally - Duration formats had no escalation path at all - a colliding range (e.g. 3600-3700s all rendering "1h") fell back to null/undefined ticks unconditionally. Now escalates formatDurationMsCompact's fixed 2-3 significant digits the same way numeric formats do. - Numeric escalation started from "configured mantissa + 1", but formatAxisTick already forces mantissa to 0 above its magnitude threshold regardless of what's configured - so the collision was at 0, and escalating from e.g. 3 (mantissa 2 + 1) skipped a sufficient 1, producing needlessly wide labels that can overflow the axis width budget. Escalation now always starts at 1. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 46 ++++++++++++++++--- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 26 +++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index e57dc282bd..2d700ac541 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -914,6 +914,33 @@ function formatTickAtMantissa( }); } +// Mirrors formatDurationMsCompact's unit selection, but with an escalatable +// significant-digit count instead of a fixed 2-3 per unit. +function formatDurationAtPrecision(ms: number, precision: number): string { + if (ms < 0) { + return `-${formatDurationAtPrecision(-ms, precision)}`; + } + if (ms === 0) { + return '0'; + } + if (ms < 0.001) { + return `${+(ms * 1e6).toPrecision(precision)}ns`; + } + if (ms < 1) { + return `${+(ms * 1000).toPrecision(precision)}µs`; + } + if (ms < 1000) { + return `${+ms.toPrecision(precision)}ms`; + } + if (ms < 120_000) { + return `${+(ms / 1000).toPrecision(precision)}s`; + } + if (ms < 3_600_000) { + return `${+(ms / 60_000).toPrecision(precision)}m`; + } + return `${+(ms / 3_600_000).toPrecision(precision)}h`; +} + // Ticks must never carry duplicate labels (Grafana/Chronosphere never do). const MAX_TICK_MANTISSA_ESCALATION = 4; @@ -942,14 +969,21 @@ function resolveDistinctTickLabels( return isDistinct(ticks, fullPrecision) ? fullPrecision : null; } if (axisNumberFormat.output === 'duration') { + // formatDurationMsCompact has no mantissa - escalate its own fixed + // 2-3 significant digits instead, past whichever unit it picks. + const factor = axisNumberFormat.factor ?? 1; + for (let p = 3; p <= 3 + MAX_TICK_MANTISSA_ESCALATION; p++) { + const escalated = (value: number) => + formatDurationAtPrecision(value * factor * 1000, p); + if (isDistinct(ticks, escalated)) { + return escalated; + } + } return null; } - const baseMantissa = Math.max(0, axisNumberFormat.mantissa ?? 0); - for ( - let m = baseMantissa + 1; - m <= baseMantissa + MAX_TICK_MANTISSA_ESCALATION; - m++ - ) { + // formatAxisTick can force mantissa down to 0 regardless of what's + // configured, so escalation must start from 1, not the configured value. + for (let m = 1; m <= MAX_TICK_MANTISSA_ESCALATION; m++) { const escalated = (value: number) => formatTickAtMantissa(value, axisNumberFormat, m); if (isDistinct(ticks, escalated)) { diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index aabacae8b9..87baf72bd6 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -15,6 +15,7 @@ import { collectMemoChartGradientHexes, computeYAxisBounds, formatAxisTick, + getExpandableYAxisTicks, getNiceYAxisTicks, getSelectedLineData, getVisibleLineData, @@ -237,6 +238,31 @@ describe('getNiceYAxisTicks', () => { const labels = result.ticks.map(t => result.tickFormatter!(t)); expect(new Set(labels).size).toBe(labels.length); }); + + it('escalates from the minimal precision, not past the configured mantissa', () => { + // Regression: formatAxisTick forces mantissa to 0 regardless of what's + // configured, so escalation from "configured+1" skipped a sufficient 1. + const result = getExpandableYAxisTicks(0, 2100, 5, { + output: 'number', + mantissa: 2, + }); + expect(result.ticks.map(t => result.tickFormatter!(t))).toEqual([ + '0.0', + '0.5k', + '1.0k', + '1.5k', + '2.0k', + ]); + }); + + it('escalates a duration format past its fixed significant-digit count', () => { + // Regression: duration collisions returned null unconditionally, + // discarding the tick list instead of trying more precision. + const result = getNiceYAxisTicks(3600, 3700, 5, { output: 'duration' }); + expect(result.tickFormatter).toBeDefined(); + const labels = result.ticks.map(t => result.tickFormatter!(t)); + expect(new Set(labels).size).toBe(labels.length); + }); }); describe('scanYAxisValueRange', () => { From 3a45f5454a81aa14a9c83900c95945c3f07faa32 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 18:24:50 -0500 Subject: [PATCH 18/25] fix: reject escalated mantissa that overflows the axis label budget Escalating precision to keep ticks distinct could render a label wider than the axis leaves room for; now rejected in favor of a coarser step, scoped to number/percent since byte/duration were already exempt. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 20 ++++++++++++++++++- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 11 ++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 2d700ac541..54959ede01 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -951,6 +951,18 @@ function isDistinct( return new Set(ticks.map(formatTick)).size === ticks.length; } +// MAX_AXIS_MANTISSA's width budget only covers plain numbers/percent (see +// its docblock) - byte/currency add a suffix it never accounted for. +const WIDTH_BUDGETED_OUTPUTS = new Set(['number', 'percent']); +const MAX_TICK_LABEL_LENGTH = 5; + +function fitsLabelBudget( + ticks: number[], + formatTick: (value: number) => string, +): boolean { + return ticks.every(t => formatTick(t).length <= MAX_TICK_LABEL_LENGTH); +} + // Prefers formatAxisTick's normal output, but escalates precision past the // configured mantissa when that's the only way to keep labels distinct. function resolveDistinctTickLabels( @@ -983,10 +995,16 @@ function resolveDistinctTickLabels( } // formatAxisTick can force mantissa down to 0 regardless of what's // configured, so escalation must start from 1, not the configured value. + const isWidthBudgeted = WIDTH_BUDGETED_OUTPUTS.has( + axisNumberFormat.output ?? 'number', + ); for (let m = 1; m <= MAX_TICK_MANTISSA_ESCALATION; m++) { const escalated = (value: number) => formatTickAtMantissa(value, axisNumberFormat, m); - if (isDistinct(ticks, escalated)) { + if ( + isDistinct(ticks, escalated) && + (!isWidthBudgeted || fitsLabelBudget(ticks, escalated)) + ) { return escalated; } } diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 87baf72bd6..a2add63c94 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -263,6 +263,17 @@ describe('getNiceYAxisTicks', () => { const labels = result.ticks.map(t => result.tickFormatter!(t)); expect(new Set(labels).size).toBe(labels.length); }); + + it('rejects a plain-number escalation that would overflow the axis label budget', () => { + // Regression: mantissa 3 was needed for distinctness but rendered + // "1.0500k" (7 chars) - must be rejected in favor of a coarser step. + const result = getExpandableYAxisTicks(1050, 1050.315, 5, { + output: 'number', + mantissa: 3, + }); + const labels = result.ticks.map(t => result.tickFormatter!(t)); + expect(labels.every(l => l.length <= 5)).toBe(true); + }); }); describe('scanYAxisValueRange', () => { From d5fe5412644f820c5018dd8d9e5fa4b505549243 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 19:07:53 -0500 Subject: [PATCH 19/25] fix: fixed-unit escalation, duration/base trimming, memo, budget checks Fixed-unit escalation no longer reintroduces the suffix formatAxisTick drops (P1); escalated numeric labels now trim trailing zeros like the base formatter; duration escalation now respects the label budget too, reusing a parameterized formatDurationMsCompact instead of a duplicate; the default-axis branch defers to Recharts fully (not a raw domain) when no step fits; yAxisBounds memoizes on a boolean, not the referenceLines node identity. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 67 ++++++++----------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 20 +++++- packages/app/src/utils.ts | 23 +++++-- 3 files changed, 61 insertions(+), 49 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 50bfcc916d..29e1d9719d 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -915,39 +915,19 @@ function formatTickAtMantissa( axisNumberFormat: NumberFormat, mantissa: number, ): string { - return formatNumber(value, { - ...axisNumberFormat, - mantissa, - average: true, - unit: undefined, - }); -} - -// Mirrors formatDurationMsCompact's unit selection, but with an escalatable -// significant-digit count instead of a fixed 2-3 per unit. -function formatDurationAtPrecision(ms: number, precision: number): string { - if (ms < 0) { - return `-${formatDurationAtPrecision(-ms, precision)}`; - } - if (ms === 0) { - return '0'; - } - if (ms < 0.001) { - return `${+(ms * 1e6).toPrecision(precision)}ns`; - } - if (ms < 1) { - return `${+(ms * 1000).toPrecision(precision)}µs`; - } - if (ms < 1000) { - return `${+ms.toPrecision(precision)}ms`; + // Mirrors formatAxisTick's fixed-unit handling - re-adding the suffix it + // drops would make budget checks reject a label that never actually renders that wide. + if (isFixedNumericUnit(axisNumberFormat.numericUnit)) { + return trimTrailingZeros(value.toFixed(mantissa)); } - if (ms < 120_000) { - return `${+(ms / 1000).toPrecision(precision)}s`; - } - if (ms < 3_600_000) { - return `${+(ms / 60_000).toPrecision(precision)}m`; - } - return `${+(ms / 3_600_000).toPrecision(precision)}h`; + return trimTrailingZeros( + formatNumber(value, { + ...axisNumberFormat, + mantissa, + average: true, + unit: undefined, + }), + ); } // Ticks must never carry duplicate labels (Grafana/Chronosphere never do). @@ -995,8 +975,8 @@ function resolveDistinctTickLabels( const factor = axisNumberFormat.factor ?? 1; for (let p = 3; p <= 3 + MAX_TICK_MANTISSA_ESCALATION; p++) { const escalated = (value: number) => - formatDurationAtPrecision(value * factor * 1000, p); - if (isDistinct(ticks, escalated)) { + formatDurationMsCompact(value * factor * 1000, p); + if (isDistinct(ticks, escalated) && fitsLabelBudget(ticks, escalated)) { return escalated; } } @@ -1178,9 +1158,14 @@ export function computeYAxisBounds( 5, axisNumberFormat, ); + // No step at this domain could be nicely ticked - a raw numeric domain + // is worse than deferring to Recharts entirely (same reasoning as above). + if (expanded.ticks.length === 0) { + return DEFAULT_Y_AXIS_BOUNDS; + } return { domain: [lowerBound, expanded.max], - ticks: expanded.ticks.length === 0 ? undefined : expanded.ticks, + ticks: expanded.ticks, tickFormatter: expanded.tickFormatter, }; } @@ -1394,6 +1379,12 @@ export const MemoChart = memo(function MemoChart({ ); }, [nearestSeriesKey, visibleLineData.length, id]); + // A boolean, not the ReactNode itself - some callers rebuild that node's + // identity every render, which would otherwise bust this memo each time. + const hasReferenceLines = Array.isArray(referenceLines) + ? referenceLines.length > 0 + : referenceLines != null; + const yAxisBounds = useMemo( () => computeYAxisBounds( @@ -1402,9 +1393,7 @@ export const MemoChart = memo(function MemoChart({ hasSeriesSelection(selectedSeriesNames), fitYAxisToData, displayType, - Array.isArray(referenceLines) - ? referenceLines.length > 0 - : referenceLines != null, + hasReferenceLines, axisNumberFormat, ), [ @@ -1413,7 +1402,7 @@ export const MemoChart = memo(function MemoChart({ selectedSeriesNames, fitYAxisToData, displayType, - referenceLines, + hasReferenceLines, axisNumberFormat, ], ); diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 9fa710ec6a..1dbbc1450c 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -320,11 +320,11 @@ describe('getNiceYAxisTicks', () => { // skipped a sufficient mantissa, producing needlessly wide labels. const result = getExpandableYAxisTicks(0, 2100, 5, { output: 'number' }); expect(result.ticks.map(t => result.tickFormatter!(t))).toEqual([ - '0.0', + '0', '0.5k', - '1.0k', + '1k', '1.5k', - '2.0k', + '2k', ]); }); @@ -347,6 +347,20 @@ describe('getNiceYAxisTicks', () => { const labels = result.ticks.map(t => result.tickFormatter!(t)); expect(labels.every(l => l.length <= 5)).toBe(true); }); + + it('escalates a fixed-unit axis without reintroducing the suffix it drops', () => { + // Regression: escalation called formatNumber directly, restoring the + // "cps" suffix formatAxisTick strips - overflowing the label budget. + const result = getExpandableYAxisTicks(1.15, 1.150115, 5, { + output: 'throughput', + numericUnit: NumericUnit.Cps, + mantissa: 2, + }); + expect(result.ticks.map(t => result.tickFormatter!(t))).toEqual([ + '1.1', + '1.2', + ]); + }); }); describe('scanYAxisValueRange', () => { diff --git a/packages/app/src/utils.ts b/packages/app/src/utils.ts index 6315d561b2..a9f6e71cd8 100644 --- a/packages/app/src/utils.ts +++ b/packages/app/src/utils.ts @@ -1104,21 +1104,30 @@ export function formatDurationMs(ms: number): string { return `${parseFloat((ms / 3_600_000).toFixed(2))}h`; } -/** Compact duration labels for axis ticks — fewer decimals, shorter units. */ -export function formatDurationMsCompact(ms: number): string { - if (ms < 0) return `-${formatDurationMsCompact(-ms)}`; +/** + * Compact duration labels for axis ticks — fewer decimals, shorter units. + * An explicit `precision` overrides each branch's own default significant- + * digit count, e.g. to escalate past a collision on the default precision. + */ +export function formatDurationMsCompact( + ms: number, + precision?: number, +): string { + if (ms < 0) return `-${formatDurationMsCompact(-ms, precision)}`; if (ms === 0) return '0'; - if (ms < 0.001) return `${+(ms * 1e6).toPrecision(2)}ns`; + if (ms < 0.001) return `${+(ms * 1e6).toPrecision(precision ?? 2)}ns`; if (ms < 1) { const µs = ms * 1000; + if (precision != null) return `${+µs.toPrecision(precision)}µs`; return µs < 10 ? `${+µs.toPrecision(2)}µs` : `${Math.round(µs)}µs`; } if (ms < 1000) { + if (precision != null) return `${+ms.toPrecision(precision)}ms`; return ms < 10 ? `${+ms.toPrecision(2)}ms` : `${Math.round(ms)}ms`; } - if (ms < 120_000) return `${+(ms / 1000).toPrecision(3)}s`; - if (ms < 3_600_000) return `${+(ms / 60_000).toPrecision(2)}m`; - return `${+(ms / 3_600_000).toPrecision(2)}h`; + if (ms < 120_000) return `${+(ms / 1000).toPrecision(precision ?? 3)}s`; + if (ms < 3_600_000) return `${+(ms / 60_000).toPrecision(precision ?? 2)}m`; + return `${+(ms / 3_600_000).toPrecision(precision ?? 2)}h`; } // format uptime as days, hours, minutes or seconds From f32ee39053773a50209f9298c52bf2d2f5dfae3f Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 20:58:07 -0500 Subject: [PATCH 20/25] modify comments Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 2 +- packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 29e1d9719d..8520e436ce 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -930,7 +930,7 @@ function formatTickAtMantissa( ); } -// Ticks must never carry duplicate labels (Grafana/Chronosphere never do). +// Ticks must never carry duplicate labels. const MAX_TICK_MANTISSA_ESCALATION = 4; function isDistinct( diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 1dbbc1450c..7d45701ff4 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -295,8 +295,8 @@ describe('getNiceYAxisTicks', () => { }); it('escalates precision past the configured mantissa to keep labels distinct', () => { - // Regression: Grafana/Chronosphere never show two ticks with the same - // label - 0 decimals collapses a narrow byte range entirely to "3 GB". + // Regression: two ticks must never show the same label - 0 decimals + // collapses a narrow byte range entirely to "3 GB". const GB = 1024 ** 3; const result = getNiceYAxisTicks(2.825 * GB, 3.45 * GB, 5, { output: 'byte', From 4d3a66aa6a97c9fec2eafe7a486f410ba5453169 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 21:29:17 -0500 Subject: [PATCH 21/25] fix: fall back to getYAxisTicks when no nice step fits a domain computeYAxisBounds was leaving ticks undefined when its nice-step search came up empty, deferring to Recharts' raw domain-division default - which has no distinctness guard and can render duplicate labels. Falls back to getYAxisTicks's reduce-tick-count strategy instead, restoring behavior #3162 already shipped for this case before the merge replaced its wiring. Also trims a few comments that crept past 2 lines. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 41 ++++++++++++++----- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 19 +++++++++ packages/app/src/utils.ts | 7 +--- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 8520e436ce..43722751d5 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -1158,10 +1158,16 @@ export function computeYAxisBounds( 5, axisNumberFormat, ); - // No step at this domain could be nicely ticked - a raw numeric domain - // is worse than deferring to Recharts entirely (same reasoning as above). + // No nice step fits - fall back to getYAxisTicks' reduce-tick-count + // dedup instead of Recharts' raw, collision-prone default. if (expanded.ticks.length === 0) { - return DEFAULT_Y_AXIS_BOUNDS; + const baseFormat = (value: number) => + formatAxisTick(value, axisNumberFormat); + return { + domain: [lowerBound, upperBound], + ticks: getYAxisTicks(lowerBound, upperBound, baseFormat), + tickFormatter: baseFormat, + }; } return { domain: [lowerBound, expanded.max], @@ -1191,12 +1197,29 @@ export function computeYAxisBounds( } // A reference line can extend this domain further (stale ticks), but // Fit-to-Data/selection still need the tight domain itself to work. - const { ticks, tickFormatter } = hasReferenceLines - ? { ticks: [], tickFormatter: undefined } - : getNiceYAxisTicks(lowerBound, upperBound, 5, axisNumberFormat); + if (hasReferenceLines) { + return { domain: [lowerBound, upperBound], ticks: undefined }; + } + const { ticks, tickFormatter } = getNiceYAxisTicks( + lowerBound, + upperBound, + 5, + axisNumberFormat, + ); + // Same fallback as the default branch above - reduce tick count via + // getYAxisTicks rather than leaving this to Recharts' raw default. + if (ticks.length === 0) { + const baseFormat = (value: number) => + formatAxisTick(value, axisNumberFormat); + return { + domain: [lowerBound, upperBound], + ticks: getYAxisTicks(lowerBound, upperBound, baseFormat), + tickFormatter: baseFormat, + }; + } return { domain: [lowerBound, upperBound], - ticks: ticks.length === 0 ? undefined : ticks, + ticks, tickFormatter, }; } @@ -1525,9 +1548,7 @@ export const MemoChart = memo(function MemoChart({ ); const yAxisTicks = yAxisBounds.ticks; - // computeYAxisBounds already resolves ticks+formatter together (nice - // steps, escalated for distinctness) for every numeric domain, so this - // takes precedence over the plain getYAxisTicks pass used elsewhere. + // computeYAxisBounds already resolves ticks+formatter together per domain. const yAxisTickFormatter = yAxisBounds.tickFormatter ?? tickFormatter; const [highlightStart, setHighlightStart] = useState(); diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 7d45701ff4..63221628ae 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -655,6 +655,25 @@ describe('computeYAxisBounds', () => { const labels = bounds.ticks!.map(t => bounds.tickFormatter!(t)); expect(new Set(labels).size).toBe(labels.length); }); + + it('falls back to getYAxisTicks when no nice step fits, instead of a raw duplicate-prone domain', () => { + // Regression: no nice step fit this range, leaving ticks undefined for + // Recharts' raw domain-division default - which collides. + const GB = 1024 ** 3; + const bounds = computeYAxisBounds( + [{ a: 3.269 * GB }, { a: 3.511 * GB }], + [series('a')], + false, + true, + DisplayType.Line, + false, + { output: 'byte', mantissa: 1 }, + ); + expect(bounds.ticks).toBeDefined(); + expect(bounds.ticks!.length).toBeGreaterThanOrEqual(2); + const labels = bounds.ticks!.map(t => bounds.tickFormatter!(t)); + expect(new Set(labels).size).toBe(labels.length); + }); }); describe('getYAxisTicks', () => { diff --git a/packages/app/src/utils.ts b/packages/app/src/utils.ts index a9f6e71cd8..35386b87e2 100644 --- a/packages/app/src/utils.ts +++ b/packages/app/src/utils.ts @@ -1104,11 +1104,8 @@ export function formatDurationMs(ms: number): string { return `${parseFloat((ms / 3_600_000).toFixed(2))}h`; } -/** - * Compact duration labels for axis ticks — fewer decimals, shorter units. - * An explicit `precision` overrides each branch's own default significant- - * digit count, e.g. to escalate past a collision on the default precision. - */ +// Compact duration labels for axis ticks. An explicit `precision` overrides +// each branch's own default significant-digit count. export function formatDurationMsCompact( ms: number, precision?: number, From 0bf58fc347f2c90bf79cc7c054c525d6bf85b232 Mon Sep 17 00:00:00 2001 From: arj22 Date: Tue, 22 Sep 2026 22:04:55 -0500 Subject: [PATCH 22/25] fix: budget-check full precision, dedup reference-line ticks, fix 2.5 step Full-precision fallback now respects the label budget like every other escalation path, falling through to a coarser step instead of rendering overflowing grouped numbers. Reference-line domains now dedup ticks via getYAxisTicks instead of dropping tick management entirely, restoring behavior alert charts had before this branch existed. The 2.5x10^n step exclusion now applies at every magnitude below 1 (0.25, 0.025, ...), not just the bare value - those round unevenly the same way. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 28 +++++++++---- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 40 ++++++++++++++----- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 43722751d5..eab21a85dd 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -876,13 +876,16 @@ export function getYAxisTicks( // Rounds off float dust (e.g. 3 * 1.05 giving 3.1500000000000004). const cleanNumber = (v: number) => Number(v.toPrecision(12)); -// Every 1/2/5 x10^n step, plus 2.5 x10^n (excluding the bare value 2.5, -// which is non-integer and formatAxisTick rounds "12.5" to "13"). +// Every 1/2/5 x10^n step, plus 2.5 x10^n only for n > 0 (25, 250, ... are +// exact integers; 2.5, 0.25, 0.025, ... round unevenly at any precision). function niceStepsNear(target: number): number[] { const exp = Math.floor(Math.log10(target)); return [exp - 1, exp, exp + 1] - .flatMap(e => [1, 2, 2.5, 5, 10].map(m => m * 10 ** e)) - .filter(step => step > 0 && step !== 2.5) + .flatMap(e => { + const multipliers = e > 0 ? [1, 2, 2.5, 5, 10] : [1, 2, 5, 10]; + return multipliers.map(m => m * 10 ** e); + }) + .filter(step => step > 0) .sort((a, b) => a - b); } @@ -967,7 +970,10 @@ function resolveDistinctTickLabels( // precision, which always distinguishes any two different numbers. const fullPrecision = (value: number) => new Intl.NumberFormat('en-US').format(value); - return isDistinct(ticks, fullPrecision) ? fullPrecision : null; + return isDistinct(ticks, fullPrecision) && + fitsLabelBudget(ticks, fullPrecision) + ? fullPrecision + : null; } if (axisNumberFormat.output === 'duration') { // formatDurationMsCompact has no mantissa - escalate its own fixed @@ -1195,10 +1201,16 @@ export function computeYAxisBounds( if (upperBound <= lowerBound) { return degenerateFallback; } - // A reference line can extend this domain further (stale ticks), but - // Fit-to-Data/selection still need the tight domain itself to work. + // A reference line can widen the rendered domain, so precomputed nice-step + // ticks could go stale - use getYAxisTicks' dedup instead of skipping it. if (hasReferenceLines) { - return { domain: [lowerBound, upperBound], ticks: undefined }; + const baseFormat = (value: number) => + formatAxisTick(value, axisNumberFormat); + return { + domain: [lowerBound, upperBound], + ticks: getYAxisTicks(lowerBound, upperBound, baseFormat), + tickFormatter: baseFormat, + }; } const { ticks, tickFormatter } = getNiceYAxisTicks( lowerBound, diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 63221628ae..21b22c3295 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -273,12 +273,10 @@ describe('getNiceYAxisTicks', () => { expect(getNiceYAxisTicks(0, 1480).ticks.length).toBeLessThanOrEqual(5); }); - it('rejects a precision-collapsed step instead of looping or truncating', () => { - // Regression: accepting the truncated result of a precision-collapsed - // step gave a one-tick axis instead of trying a coarser step. - expect(getNiceYAxisTicks(999999999999, 1000000000030).ticks).toEqual([ - 1000000000000, 1000000000010, 1000000000020, 1000000000030, - ]); + it('rejects a step whose only distinct formatting overflows the label budget', () => { + // This domain needs 17-char labels to distinguish at all - no step + // fits the budget and stays distinct, so this returns no ticks. + expect(getNiceYAxisTicks(999999999999, 1000000000030).ticks).toEqual([]); }); it('avoids a step that formatAxisTick would round unevenly (12.5 -> "13")', () => { @@ -361,6 +359,25 @@ describe('getNiceYAxisTicks', () => { '1.2', ]); }); + + it('rejects a full-precision fallback that overflows the label budget', () => { + // Regression: no-format fallback had no budget check, rendering + // 9-char grouped numbers instead of falling through to a coarser step. + const result = getNiceYAxisTicks(1200000, 1250000); + const labels = result.ticks.map(t => result.tickFormatter!(t)); + expect(labels.every(l => l.length <= 5)).toBe(true); + expect(new Set(labels).size).toBe(labels.length); + }); + + it('excludes a 2.5-scaled step below 1, not just the bare value', () => { + // Regression: step 0.25 (2.5x10^-1) survived the bare-2.5 filter, + // rounding to an uneven 0.3/0.2/0.3/0.2 progression at mantissa 0. + const result = getNiceYAxisTicks(0, 1.1, 5, { + output: 'number', + mantissa: 0, + }); + expect(result.ticks).toEqual([0, 0.5, 1]); + }); }); describe('scanYAxisValueRange', () => { @@ -605,9 +622,9 @@ describe('computeYAxisBounds', () => { }); }); - it('keeps the fitted/selected domain when a reference line is present, only dropping ticks', () => { - // Regression: bailing out to the auto fallback (instead of just - // omitting ticks) disabled Fit-to-Data/legend-isolate on alert charts. + it('keeps ticks deduped via getYAxisTicks when a reference line is present', () => { + // Regression: bailing to ticks: undefined dropped the duplicate-label + // dedup an alert chart (always has a reference line) used to get. const bounds = computeYAxisBounds( [{ a: 100 }, { a: 200 }], [series('a')], @@ -616,7 +633,10 @@ describe('computeYAxisBounds', () => { DisplayType.Line, true, ); - expect(bounds).toEqual({ domain: [95, 205], ticks: undefined }); + expect(bounds.domain).toEqual([95, 205]); + expect(bounds.ticks).toBeDefined(); + const labels = bounds.ticks!.map(t => bounds.tickFormatter!(t)); + expect(new Set(labels).size).toBe(labels.length); }); it('escalates precision instead of dropping a step whose labels collide', () => { From 5fe68fd2dd6412a0a4d3318e79e85cceb4fb7bfd Mon Sep 17 00:00:00 2001 From: arj22 Date: Wed, 23 Sep 2026 04:41:20 -0500 Subject: [PATCH 23/25] fix: restore 2.5-step density with decimals, extend domain for reference lines Mantissa-aware 2.5x10^n exclusion: only excluded below 1 when the format forces integers, restoring tick density for decimal gauges/ratios that have room to represent it exactly (e.g. mantissa 2 keeps a clean 0.25 step instead of falling back to a sparser 0.5 one). Reference lines: threaded real threshold value(s) from Alerts.tsx down through DBTimeChart/MemoChart into computeYAxisBounds (previously just a boolean), so a threshold that would widen the domain gets folded into the nice-step calculation up front instead of ticking a domain that goes stale once Recharts' ifOverflow="extendDomain" moves it. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 78 ++++++++++++++----- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 74 ++++++++++++------ .../app/src/components/AlertPreviewChart.tsx | 5 ++ .../DBEditTimeChartForm/ChartPreviewPanel.tsx | 11 +++ packages/app/src/components/DBTimeChart.tsx | 4 + .../components/alerts/AlertDetailChart.tsx | 16 +++- 6 files changed, 140 insertions(+), 48 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index eab21a85dd..d173b0a5b4 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -876,13 +876,28 @@ export function getYAxisTicks( // Rounds off float dust (e.g. 3 * 1.05 giving 3.1500000000000004). const cleanNumber = (v: number) => Number(v.toPrecision(12)); -// Every 1/2/5 x10^n step, plus 2.5 x10^n only for n > 0 (25, 250, ... are -// exact integers; 2.5, 0.25, 0.025, ... round unevenly at any precision). -function niceStepsNear(target: number): number[] { +// A format with no decimals to spend forces every tick to an integer, +// where a 2.5x10^n step (n <= 0) rounds unevenly - see niceStepsNear. +function forcesIntegerTicks(axisNumberFormat?: NumberFormat): boolean { + if (!axisNumberFormat) return true; + if (axisNumberFormat.output === 'duration') return false; + return (axisNumberFormat.mantissa ?? 0) === 0; +} + +// Every 1/2/5 x10^n step, plus 2.5x10^n for n > 0 (integers like 250 are +// always safe; 2.5, 0.25, ... round unevenly with no decimals to spare). +function niceStepsNear( + target: number, + axisNumberFormat?: NumberFormat, +): number[] { const exp = Math.floor(Math.log10(target)); + const excludeSmall2_5 = forcesIntegerTicks(axisNumberFormat); return [exp - 1, exp, exp + 1] .flatMap(e => { - const multipliers = e > 0 ? [1, 2, 2.5, 5, 10] : [1, 2, 5, 10]; + const includeQuarterStep = e > 0 || !excludeSmall2_5; + const multipliers = includeQuarterStep + ? [1, 2, 2.5, 5, 10] + : [1, 2, 5, 10]; return multipliers.map(m => m * 10 ** e); }) .filter(step => step > 0) @@ -1016,7 +1031,7 @@ export function getNiceYAxisTicks( if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { return { ticks: [] }; } - const steps = niceStepsNear((max - min) / (maxTicks - 1)); + const steps = niceStepsNear((max - min) / (maxTicks - 1), axisNumberFormat); for (const step of steps) { const ticks = ticksWithinRange(step, min, max); // A single tick conveys no scale at all - never accept it, even @@ -1049,7 +1064,7 @@ export function getExpandableYAxisTicks( if (!Number.isFinite(min) || !Number.isFinite(max) || max <= min) { return { max, ticks: [] }; } - const steps = niceStepsNear((max - min) / (maxTicks - 1)); + const steps = niceStepsNear((max - min) / (maxTicks - 1), axisNumberFormat); for (const step of steps) { const tightTicks = ticksWithinRange(step, min, max); // A single tick conveys no scale at all - never accept it, even @@ -1114,6 +1129,10 @@ export interface YAxisBounds { tickFormatter?: (value: number) => string; } +// Stable identity for the referenceLineValues default - a fresh `[]` literal +// as a default prop value defeats memoization and can loop React's compiler. +const EMPTY_REFERENCE_LINE_VALUES: number[] = []; + const DEFAULT_Y_AXIS_BOUNDS: YAxisBounds = { domain: [0, 'auto'], ticks: undefined, @@ -1131,7 +1150,7 @@ export function computeYAxisBounds( hasSelection: boolean, fitYAxisToData: boolean, displayType: DisplayType, - hasReferenceLines: boolean, + referenceLineValues: number[], axisNumberFormat?: NumberFormat, ): YAxisBounds { if (displayType === DisplayType.StackedBar) { @@ -1142,7 +1161,7 @@ export function computeYAxisBounds( if (!hasSelection && !shouldFitYAxis) { // A fully numeric domain skips Recharts' own nice rounding, and a // reference line can extend it further - defer to Recharts entirely. - if (hasReferenceLines) { + if (referenceLineValues.length > 0) { return DEFAULT_Y_AXIS_BOUNDS; } const { min, max } = scanYAxisValueRange(graphResults, visibleLineData); @@ -1201,14 +1220,33 @@ export function computeYAxisBounds( if (upperBound <= lowerBound) { return degenerateFallback; } - // A reference line can widen the rendered domain, so precomputed nice-step - // ticks could go stale - use getYAxisTicks' dedup instead of skipping it. - if (hasReferenceLines) { + // A reference line can widen the domain (extendDomain) - extend it up + // front and nice-step the result, instead of ticking a stale domain. + if (referenceLineValues.length > 0) { + const extendedLower = cleanNumber( + Math.min(lowerBound, ...referenceLineValues), + ); + const extendedUpper = cleanNumber( + Math.max(upperBound, ...referenceLineValues), + ); + const expanded = getExpandableYAxisTicks( + extendedLower, + extendedUpper, + 5, + axisNumberFormat, + ); + if (expanded.ticks.length > 0) { + return { + domain: [extendedLower, expanded.max], + ticks: expanded.ticks, + tickFormatter: expanded.tickFormatter, + }; + } const baseFormat = (value: number) => formatAxisTick(value, axisNumberFormat); return { - domain: [lowerBound, upperBound], - ticks: getYAxisTicks(lowerBound, upperBound, baseFormat), + domain: [extendedLower, extendedUpper], + ticks: getYAxisTicks(extendedLower, extendedUpper, baseFormat), tickFormatter: baseFormat, }; } @@ -1244,6 +1282,7 @@ export const MemoChart = memo(function MemoChart({ dateRange, lineData, referenceLines, + referenceLineValues = EMPTY_REFERENCE_LINE_VALUES, annotations, logReferenceTimestamp, displayType = DisplayType.Line, @@ -1274,6 +1313,9 @@ export const MemoChart = memo(function MemoChart({ dateRange: [Date, Date] | Readonly<[Date, Date]>; lineData: LineData[]; referenceLines?: React.ReactNode; + // Raw numeric value(s) backing referenceLines (pre-rendered JSX the axis + // math can't read), used to size the Y-axis domain around them. + referenceLineValues?: number[]; /** * Event markers (alerts, releases, …) drawn as dashed vertical lines with a * label above. Passed as data rather than pre-rendered elements so the chart @@ -1414,12 +1456,6 @@ export const MemoChart = memo(function MemoChart({ ); }, [nearestSeriesKey, visibleLineData.length, id]); - // A boolean, not the ReactNode itself - some callers rebuild that node's - // identity every render, which would otherwise bust this memo each time. - const hasReferenceLines = Array.isArray(referenceLines) - ? referenceLines.length > 0 - : referenceLines != null; - const yAxisBounds = useMemo( () => computeYAxisBounds( @@ -1428,7 +1464,7 @@ export const MemoChart = memo(function MemoChart({ hasSeriesSelection(selectedSeriesNames), fitYAxisToData, displayType, - hasReferenceLines, + referenceLineValues, axisNumberFormat, ), [ @@ -1437,7 +1473,7 @@ export const MemoChart = memo(function MemoChart({ selectedSeriesNames, fitYAxisToData, displayType, - hasReferenceLines, + referenceLineValues, axisNumberFormat, ], ); diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 21b22c3295..32db9d90f2 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -369,14 +369,21 @@ describe('getNiceYAxisTicks', () => { expect(new Set(labels).size).toBe(labels.length); }); - it('excludes a 2.5-scaled step below 1, not just the bare value', () => { + it('excludes a 2.5-scaled step below 1 only when the format forces integers', () => { // Regression: step 0.25 (2.5x10^-1) survived the bare-2.5 filter, // rounding to an uneven 0.3/0.2/0.3/0.2 progression at mantissa 0. - const result = getNiceYAxisTicks(0, 1.1, 5, { + const forcedInteger = getNiceYAxisTicks(0, 1.1, 5, { output: 'number', mantissa: 0, }); - expect(result.ticks).toEqual([0, 0.5, 1]); + expect(forcedInteger.ticks).toEqual([0, 0.5, 1]); + + // With decimals to spend, 0.25 renders exactly - keep the denser step. + const withDecimals = getNiceYAxisTicks(0, 1.1, 5, { + output: 'number', + mantissa: 2, + }); + expect(withDecimals.ticks).toEqual([0, 0.25, 0.5, 0.75, 1]); }); }); @@ -421,7 +428,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [0, 3.15], @@ -439,7 +446,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [0, 15], @@ -457,7 +464,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [0, 1500], @@ -475,7 +482,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [-100, 1.05], @@ -493,7 +500,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [-1000, -850], @@ -511,7 +518,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - true, + [1500], ); expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); }); @@ -529,7 +536,7 @@ describe('computeYAxisBounds', () => { hasSelection, false, DisplayType.StackedBar, - false, + [], ), ).toEqual({ domain: [0, 'auto'], ticks: undefined }); } @@ -542,7 +549,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); }); @@ -555,7 +562,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); }); @@ -569,7 +576,7 @@ describe('computeYAxisBounds', () => { false, true, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: ['auto', 'auto'], ticks: undefined }); }); @@ -583,7 +590,7 @@ describe('computeYAxisBounds', () => { true, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [0, 'auto'], ticks: undefined }); }); @@ -595,7 +602,7 @@ describe('computeYAxisBounds', () => { false, true, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [-62.5, 212.5], @@ -613,7 +620,7 @@ describe('computeYAxisBounds', () => { true, false, DisplayType.Line, - false, + [], ); expect(bounds).toEqual({ domain: [-110, 110], @@ -622,7 +629,7 @@ describe('computeYAxisBounds', () => { }); }); - it('keeps ticks deduped via getYAxisTicks when a reference line is present', () => { + it('nice-steps normally when a reference line already sits inside the range', () => { // Regression: bailing to ticks: undefined dropped the duplicate-label // dedup an alert chart (always has a reference line) used to get. const bounds = computeYAxisBounds( @@ -631,12 +638,31 @@ describe('computeYAxisBounds', () => { false, true, DisplayType.Line, + [150], + ); + expect(bounds).toEqual({ + domain: [95, 205], + ticks: [100, 125, 150, 175, 200], + tickFormatter: expect.any(Function), + }); + }); + + it('extends the domain to include an out-of-range reference line, then nice-steps it', () => { + // A threshold beyond the data (e.g. Alerts.tsx's ifOverflow="extendDomain") + // widens the rendered domain - ticking it up front avoids stale ticks. + const bounds = computeYAxisBounds( + [{ a: 100 }, { a: 200 }], + [series('a')], + false, true, + DisplayType.Line, + [300], ); - expect(bounds.domain).toEqual([95, 205]); - expect(bounds.ticks).toBeDefined(); - const labels = bounds.ticks!.map(t => bounds.tickFormatter!(t)); - expect(new Set(labels).size).toBe(labels.length); + expect(bounds).toEqual({ + domain: [95, 300], + ticks: [100, 150, 200, 250, 300], + tickFormatter: expect.any(Function), + }); }); it('escalates precision instead of dropping a step whose labels collide', () => { @@ -648,7 +674,7 @@ describe('computeYAxisBounds', () => { false, false, DisplayType.Line, - false, + [], { output: 'number' }, ); expect(bounds.domain).toEqual([0, 2100]); @@ -667,7 +693,7 @@ describe('computeYAxisBounds', () => { false, true, DisplayType.Line, - false, + [], { output: 'byte', mantissa: 0 }, ); expect(bounds.ticks).toBeDefined(); @@ -686,7 +712,7 @@ describe('computeYAxisBounds', () => { false, true, DisplayType.Line, - false, + [], { output: 'byte', mantissa: 1 }, ); expect(bounds.ticks).toBeDefined(); diff --git a/packages/app/src/components/AlertPreviewChart.tsx b/packages/app/src/components/AlertPreviewChart.tsx index 03a96990f2..29801ba74e 100644 --- a/packages/app/src/components/AlertPreviewChart.tsx +++ b/packages/app/src/components/AlertPreviewChart.tsx @@ -119,6 +119,10 @@ export const AlertPreviewChart = ({ }), [threshold, thresholdMax, thresholdType], ); + const referenceLineValues = useMemo( + () => [threshold, thresholdMax].filter((v): v is number => v != null), + [threshold, thresholdMax], + ); return ( @@ -128,6 +132,7 @@ export const AlertPreviewChart = ({ showMVOptimizationIndicator={false} showDateRangeIndicator={false} referenceLines={referenceLines} + referenceLineValues={referenceLineValues} annotations={annotations} config={config} /> diff --git a/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx b/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx index 60e7a7b5fd..c4e9fbd939 100644 --- a/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx @@ -209,6 +209,16 @@ export function ChartPreviewPanel({ [queriedConfig, tableSource, dateRange, queryReady], ); + const referenceLineValues = useMemo( + () => + alert + ? [alert.threshold, alert.thresholdMax].filter( + (v): v is number => v != null, + ) + : undefined, + [alert], + ); + return ( <> {isBlockedByRequiredFilters ? ( @@ -263,6 +273,7 @@ export function ChartPreviewPanel({ thresholdType: alert.thresholdType, }) } + referenceLineValues={referenceLineValues} errorVariant="inline" showMVOptimizationIndicator={false} // Preview doesn't need the MV indicators; disabling both lets diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart.tsx index 1ae67bcbb8..6cd84278b9 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart.tsx @@ -304,6 +304,8 @@ type DBTimeChartComponentProps = { onTimeRangeSelect?: (start: Date, end: Date) => void; queryKeyPrefix?: string; referenceLines?: React.ReactNode; + /** Raw numeric value(s) backing referenceLines, for Y-axis domain sizing. */ + referenceLineValues?: number[]; /** Event markers (e.g. alert firing/recovery) drawn as dashed lines with labels. */ annotations?: ChartAnnotation[]; setDisplayType?: (type: DisplayType) => void; @@ -339,6 +341,7 @@ function DBTimeChartComponent({ onTimeRangeSelect, queryKeyPrefix, referenceLines, + referenceLineValues, annotations, setDisplayType, showDisplaySwitcher = true, @@ -1035,6 +1038,7 @@ function DBTimeChartComponent({ tooltipNumberFormatsByKey={formatByColumn} onTimeRangeSelect={onTimeRangeSelect} referenceLines={referenceLines} + referenceLineValues={referenceLineValues} annotations={annotations} setIsClickActive={setPinnedPayload} refreshClickActive={refreshPinnedPayload} diff --git a/packages/app/src/components/alerts/AlertDetailChart.tsx b/packages/app/src/components/alerts/AlertDetailChart.tsx index 31e9ef48ce..6f284a1ae4 100644 --- a/packages/app/src/components/alerts/AlertDetailChart.tsx +++ b/packages/app/src/components/alerts/AlertDetailChart.tsx @@ -244,7 +244,7 @@ const SINGLE_VALUE_RAW_SQL_MESSAGE = 'This alert runs a raw SQL query that returns one value per window, so it has no chart over time.'; function useAlertReferenceLines(alert: AlertsPageItem) { - return React.useMemo( + const referenceLines = React.useMemo( () => getAlertReferenceLines({ threshold: alert.threshold, @@ -253,6 +253,14 @@ function useAlertReferenceLines(alert: AlertsPageItem) { }), [alert.threshold, alert.thresholdMax, alert.thresholdType], ); + const referenceLineValues = React.useMemo( + () => + [alert.threshold, alert.thresholdMax].filter( + (v): v is number => v != null, + ), + [alert.threshold, alert.thresholdMax], + ); + return { referenceLines, referenceLineValues }; } function TileAlertChart({ @@ -294,7 +302,7 @@ function TileAlertChart({ [tile, source, dashboard?.filters, dateRange, granularity], ); - const referenceLines = useAlertReferenceLines(alert); + const { referenceLines, referenceLineValues } = useAlertReferenceLines(alert); if (isDashboardsLoading || (tileSourceId != null && isSourceLoading)) { return ; @@ -321,6 +329,7 @@ function TileAlertChart({ showMVOptimizationIndicator={false} showDateRangeIndicator={false} referenceLines={referenceLines} + referenceLineValues={referenceLineValues} annotations={annotations} config={config} /> @@ -364,7 +373,7 @@ function InlineAlertChart({ [chartConfig, source, dateRange, granularity], ); - const referenceLines = useAlertReferenceLines(alert); + const { referenceLines, referenceLineValues } = useAlertReferenceLines(alert); if (configSourceId != null && isSourceLoading) { return ; @@ -391,6 +400,7 @@ function InlineAlertChart({ showMVOptimizationIndicator={false} showDateRangeIndicator={false} referenceLines={referenceLines} + referenceLineValues={referenceLineValues} annotations={annotations} config={config} /> From e63a5e5e84913904d879bec93e45e8b1ffde73f6 Mon Sep 17 00:00:00 2001 From: arj22 Date: Wed, 23 Sep 2026 05:17:06 -0500 Subject: [PATCH 24/25] fix: stop a stale thresholdMax from widening the axis on non-range alerts getAlertReferenceLineValues mirrors getAlertReferenceLines' own branching so the values used to size the Y-axis domain can never include a thresholdMax that isn't actually rendered - previously any non-null thresholdMax was grabbed unconditionally, so a leftover value from an alert that used to be BETWEEN/NOT_BETWEEN (but was since changed to a simple type) could silently stretch the domain around an invisible line. Co-Authored-By: Claude Sonnet 5 --- .../app/src/components/AlertPreviewChart.tsx | 7 ++- packages/app/src/components/Alerts.tsx | 24 ++++++++ .../DBEditTimeChartForm/ChartPreviewPanel.tsx | 13 +++-- .../src/components/__tests__/Alerts.test.ts | 58 +++++++++++++++++++ .../components/alerts/AlertDetailChart.tsx | 15 +++-- 5 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 packages/app/src/components/__tests__/Alerts.test.ts diff --git a/packages/app/src/components/AlertPreviewChart.tsx b/packages/app/src/components/AlertPreviewChart.tsx index 29801ba74e..1040c6f295 100644 --- a/packages/app/src/components/AlertPreviewChart.tsx +++ b/packages/app/src/components/AlertPreviewChart.tsx @@ -23,7 +23,7 @@ import { DBTimeChart } from '@/components/DBTimeChart'; import { useAliasMapFromChartConfig } from '@/hooks/useChartConfig'; import { intervalToDateRange, intervalToGranularity } from '@/utils/alerts'; -import { getAlertReferenceLines } from './Alerts'; +import { getAlertReferenceLines, getAlertReferenceLineValues } from './Alerts'; type AlertPreviewChartProps = { source: TSource; @@ -120,8 +120,9 @@ export const AlertPreviewChart = ({ [threshold, thresholdMax, thresholdType], ); const referenceLineValues = useMemo( - () => [threshold, thresholdMax].filter((v): v is number => v != null), - [threshold, thresholdMax], + () => + getAlertReferenceLineValues({ threshold, thresholdMax, thresholdType }), + [threshold, thresholdMax, thresholdType], ); return ( diff --git a/packages/app/src/components/Alerts.tsx b/packages/app/src/components/Alerts.tsx index e0e1a82aa7..ac472bbc2d 100644 --- a/packages/app/src/components/Alerts.tsx +++ b/packages/app/src/components/Alerts.tsx @@ -256,3 +256,27 @@ export const getAlertReferenceLines = ({ /> ); }; + +// Mirrors getAlertReferenceLines' own branching, so the values used to size +// the Y-axis domain can never drift from what's actually drawn as a line. +export const getAlertReferenceLineValues = ({ + thresholdType, + threshold, + thresholdMax, +}: { + thresholdType: AlertThresholdType; + threshold: number; + thresholdMax?: number; +}): number[] => { + if (threshold == null) { + return []; + } + if ( + (thresholdType === AlertThresholdType.BETWEEN || + thresholdType === AlertThresholdType.NOT_BETWEEN) && + thresholdMax != null + ) { + return [threshold, thresholdMax]; + } + return [threshold]; +}; diff --git a/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx b/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx index c4e9fbd939..2f352a138a 100644 --- a/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/ChartPreviewPanel.tsx @@ -15,7 +15,10 @@ import { IconList } from '@tabler/icons-react'; import { SortingState } from '@tanstack/react-table'; import { buildTableRowSearchUrl } from '@/ChartUtils'; -import { getAlertReferenceLines } from '@/components/Alerts'; +import { + getAlertReferenceLines, + getAlertReferenceLineValues, +} from '@/components/Alerts'; import { ChartEditorFormState } from '@/components/ChartEditor/types'; import ChartSQLPreview from '@/components/ChartSQLPreview'; import { DBBarChart } from '@/components/DBBarChart'; @@ -212,9 +215,11 @@ export function ChartPreviewPanel({ const referenceLineValues = useMemo( () => alert - ? [alert.threshold, alert.thresholdMax].filter( - (v): v is number => v != null, - ) + ? getAlertReferenceLineValues({ + threshold: alert.threshold, + thresholdMax: alert.thresholdMax, + thresholdType: alert.thresholdType, + }) : undefined, [alert], ); diff --git a/packages/app/src/components/__tests__/Alerts.test.ts b/packages/app/src/components/__tests__/Alerts.test.ts new file mode 100644 index 0000000000..9009336054 --- /dev/null +++ b/packages/app/src/components/__tests__/Alerts.test.ts @@ -0,0 +1,58 @@ +import { AlertThresholdType } from '@hyperdx/common-utils/dist/types'; + +import { getAlertReferenceLineValues } from '@/components/Alerts'; + +describe('getAlertReferenceLineValues', () => { + it('returns only threshold for a simple (non-range) alert type', () => { + expect( + getAlertReferenceLineValues({ + threshold: 50, + thresholdMax: undefined, + thresholdType: AlertThresholdType.ABOVE, + }), + ).toEqual([50]); + }); + + it('ignores a stale thresholdMax left over from a since-changed alert type', () => { + // Regression: getAlertReferenceLines only draws thresholdMax for + // BETWEEN/NOT_BETWEEN - a leftover value here would silently widen the + // Y-axis domain around a line that's never actually rendered. + expect( + getAlertReferenceLineValues({ + threshold: 50, + thresholdMax: 500, + thresholdType: AlertThresholdType.ABOVE, + }), + ).toEqual([50]); + }); + + it('includes both values for BETWEEN, matching the rendered reference area', () => { + expect( + getAlertReferenceLineValues({ + threshold: 10, + thresholdMax: 20, + thresholdType: AlertThresholdType.BETWEEN, + }), + ).toEqual([10, 20]); + }); + + it('includes both values for NOT_BETWEEN, matching the rendered reference areas', () => { + expect( + getAlertReferenceLineValues({ + threshold: 10, + thresholdMax: 20, + thresholdType: AlertThresholdType.NOT_BETWEEN, + }), + ).toEqual([10, 20]); + }); + + it('drops thresholdMax for BETWEEN when it is unset', () => { + expect( + getAlertReferenceLineValues({ + threshold: 10, + thresholdMax: undefined, + thresholdType: AlertThresholdType.BETWEEN, + }), + ).toEqual([10]); + }); +}); diff --git a/packages/app/src/components/alerts/AlertDetailChart.tsx b/packages/app/src/components/alerts/AlertDetailChart.tsx index 6f284a1ae4..60608d154e 100644 --- a/packages/app/src/components/alerts/AlertDetailChart.tsx +++ b/packages/app/src/components/alerts/AlertDetailChart.tsx @@ -25,7 +25,10 @@ import { import { Anchor, Center, Paper, Skeleton, Text } from '@mantine/core'; import { AlertPreviewChart } from '@/components/AlertPreviewChart'; -import { getAlertReferenceLines } from '@/components/Alerts'; +import { + getAlertReferenceLines, + getAlertReferenceLineValues, +} from '@/components/Alerts'; import { DBTimeChart } from '@/components/DBTimeChart'; import { useDashboards } from '@/dashboard'; import { useAlertAnnotations } from '@/hooks/useAlertAnnotations'; @@ -255,10 +258,12 @@ function useAlertReferenceLines(alert: AlertsPageItem) { ); const referenceLineValues = React.useMemo( () => - [alert.threshold, alert.thresholdMax].filter( - (v): v is number => v != null, - ), - [alert.threshold, alert.thresholdMax], + getAlertReferenceLineValues({ + threshold: alert.threshold, + thresholdMax: alert.thresholdMax, + thresholdType: alert.thresholdType, + }), + [alert.threshold, alert.thresholdMax, alert.thresholdType], ); return { referenceLines, referenceLineValues }; } From 3e7dc0e4d39640ce4c9175a3adc4759446eb5cf5 Mon Sep 17 00:00:00 2001 From: arj22 Date: Wed, 23 Sep 2026 05:38:15 -0500 Subject: [PATCH 25/25] Include the fixed 0 edge for BELOW alert reference line values getAlertReferenceLineValues omitted the implicit y1=0 edge that getAlertReferenceLines draws for BELOW/BELOW_OR_EQUAL, letting the Y-axis domain get computed too narrow and then silently widened. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/components/Alerts.tsx | 6 +++++ .../src/components/__tests__/Alerts.test.ts | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/packages/app/src/components/Alerts.tsx b/packages/app/src/components/Alerts.tsx index ac472bbc2d..360d797fb8 100644 --- a/packages/app/src/components/Alerts.tsx +++ b/packages/app/src/components/Alerts.tsx @@ -278,5 +278,11 @@ export const getAlertReferenceLineValues = ({ ) { return [threshold, thresholdMax]; } + if ( + thresholdType === AlertThresholdType.BELOW || + thresholdType === AlertThresholdType.BELOW_OR_EQUAL + ) { + return [0, threshold]; + } return [threshold]; }; diff --git a/packages/app/src/components/__tests__/Alerts.test.ts b/packages/app/src/components/__tests__/Alerts.test.ts index 9009336054..5ce1772ce5 100644 --- a/packages/app/src/components/__tests__/Alerts.test.ts +++ b/packages/app/src/components/__tests__/Alerts.test.ts @@ -55,4 +55,26 @@ describe('getAlertReferenceLineValues', () => { }), ).toEqual([10]); }); + + it("includes 0 for BELOW, matching the reference area's fixed bottom edge", () => { + // Regression: getAlertReferenceLines renders BELOW/BELOW_OR_EQUAL with + // y1={0} - omitting 0 here let Recharts silently widen the domain later. + expect( + getAlertReferenceLineValues({ + threshold: 550, + thresholdMax: undefined, + thresholdType: AlertThresholdType.BELOW, + }), + ).toEqual([0, 550]); + }); + + it("includes 0 for BELOW_OR_EQUAL, matching the reference area's fixed bottom edge", () => { + expect( + getAlertReferenceLineValues({ + threshold: 550, + thresholdMax: undefined, + thresholdType: AlertThresholdType.BELOW_OR_EQUAL, + }), + ).toEqual([0, 550]); + }); });