From af43e690d3575fc1c5f05aecba7f2904af75f4d1 Mon Sep 17 00:00:00 2001 From: arj22 Date: Sat, 29 Aug 2026 00:13:57 -0500 Subject: [PATCH 1/5] Respect configured mantissa on time chart Y-axis tick labels The Y-axis tickFormatter in HDXMultiSeriesTimeChart unconditionally forced mantissa: 0, discarding whatever mantissa/decimals the chart's Number Format configured. Series whose values live under 1 (fractional Prometheus gauges, ratios, etc.) therefore always render every axis tick as "0", even though the tooltip/legend (which calls formatNumber(value, numberFormat) directly, see ChartTooltip.tsx) show the correct value. Extract the formatter into an exported formatAxisTick so an explicit axisNumberFormat.mantissa now wins, falling back to 0 only when unset - preserving the existing compact-abbreviation behavior for the common case of unconfigured, typically-large counts. average and unit remain forced as before. --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 44 +++++++++++++------ .../__tests__/HDXMultiSeriesTimeChart.test.ts | 37 ++++++++++++++++ 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 922f07ccc9..f8e6549f28 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -766,6 +766,36 @@ export function collectMemoChartGradientHexes( ); } +/** + * Y-axis tick label formatter. Exported so a unit test can pin the + * mantissa-precedence behavior without rendering recharts. + * + * `average` and `unit` are always forced (compact abbreviation like `1.2k` + * reads better on an axis than a series' configured unit repeated on every + * tick), but an explicit `axisNumberFormat.mantissa` is honored rather than + * discarded. Without this, a chart whose configured Decimals produces + * correct tooltip/legend values (e.g. `0.14`) would still round every axis + * tick to `0` for any series whose values live under 1 (fractional + * Prometheus gauges, ratios, etc.), since `mantissa` only defaults to `0` + * here for the common case of unconfigured, typically-large counts. + */ +export function formatAxisTick( + value: number, + axisNumberFormat?: NumberFormat, +): string { + return axisNumberFormat + ? formatNumber(value, { + mantissa: 0, + ...axisNumberFormat, + average: true, + unit: undefined, + }) + : new Intl.NumberFormat('en-US', { + notation: 'compact', + compactDisplay: 'short', + }).format(value); +} + export const MemoChart = memo(function MemoChart({ graphResults, setIsClickActive, @@ -1115,19 +1145,7 @@ export const MemoChart = memo(function MemoChart({ ); const tickFormatter = useCallback( - (value: number) => { - return axisNumberFormat - ? formatNumber(value, { - ...axisNumberFormat, - average: true, - mantissa: 0, - unit: undefined, - }) - : new Intl.NumberFormat('en-US', { - notation: 'compact', - compactDisplay: 'short', - }).format(value); - }, + (value: number) => formatAxisTick(value, axisNumberFormat), [axisNumberFormat], ); diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 723207bbf7..608f791c50 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -11,6 +11,7 @@ import type { ActiveClickSeries } from '@/HDXMultiSeriesTimeChart'; import { buildActiveClickSeries, collectMemoChartGradientHexes, + formatAxisTick, getSelectedLineData, getVisibleLineData, getVisibleTooltipRows, @@ -19,6 +20,42 @@ import { } from '@/HDXMultiSeriesTimeChart'; import { COLORS } from '@/utils'; +describe('formatAxisTick', () => { + it('falls back to mantissa 0 when the format has none configured', () => { + // Matches the historical behavior for typically-large, unconfigured + // counts (log/event counts, request rates): compact, no decimals. + expect(formatAxisTick(1234, { output: 'number' })).toBe('1k'); + }); + + it('honors an explicit mantissa instead of always rounding to 0', () => { + // Regression: a chart whose Decimals setting produces correct + // tooltip/legend values (e.g. via formatNumber(value, numberFormat) + // directly, see ChartTooltip.tsx) would previously still show every + // axis tick as "0" for values under 1 (fractional Prometheus gauges, + // ratios, etc.), since mantissa was unconditionally forced to 0. + expect(formatAxisTick(0.14, { output: 'number', mantissa: 2 })).toBe( + '0.14', + ); + expect(formatAxisTick(0.021, { output: 'number', mantissa: 2 })).toBe( + '0.02', + ); + }); + + it('always strips a configured unit and forces compact averaging', () => { + expect( + formatAxisTick(1234, { + output: 'number', + mantissa: 2, + unit: 'req/s', + }), + ).toBe('1.23k'); + }); + + it('uses compact Intl formatting when no axisNumberFormat is set', () => { + expect(formatAxisTick(1234)).toBe('1.2K'); + }); +}); + describe('collectMemoChartGradientHexes', () => { it('includes every categorical hex from COLORS up front', () => { const hexes = collectMemoChartGradientHexes([]); From 75c2cb8cf285a0d5bde336962d9369ac62ff2d98 Mon Sep 17 00:00:00 2001 From: arj22 Date: Sat, 29 Aug 2026 00:22:49 -0500 Subject: [PATCH 2/5] Add changeset for axis tick mantissa fix --- .changeset/swift-owls-spend.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/swift-owls-spend.md diff --git a/.changeset/swift-owls-spend.md b/.changeset/swift-owls-spend.md new file mode 100644 index 0000000000..92b19417b2 --- /dev/null +++ b/.changeset/swift-owls-spend.md @@ -0,0 +1,12 @@ +--- +"@hyperdx/app": patch +--- + +fix(charts): honor a configured mantissa on time chart Y-axis tick labels + +The Y-axis of a time series chart always rounded tick labels to 0 decimal +places, regardless of the chart's Number Format settings. Charts whose +values live under 1 (fractional gauges, ratios, sub-1 rates) rendered every +axis tick as `0` even though the tooltip and legend showed the correct +value. The axis now respects an explicitly configured mantissa, falling +back to 0 only when none is set. From 15881f5d6079277a9889178ef3cc9dd45fb01d5c Mon Sep 17 00:00:00 2001 From: arj22 Date: Mon, 31 Aug 2026 15:06:51 -0500 Subject: [PATCH 3/5] Cap honored axis mantissa at 2 decimal places An unconditionally-honored mantissa regresses ticks for values >= 1: HyperDX's own bundled dashboard templates (go-runtime.json et al.) set mantissa on line-chart tiles for tooltip readability, well above the near-zero case this formatter targets, and the Decimals control allows up to 10 - which would badly overflow the axis's fixed-width label column. Measured against the actual rendered font (11px IBM Plex Mono) and the axis's real layout budget (40px column, minus recharts' own 6px tickSize + 2px tickMargin, plus its 5px default chart margin before the true canvas edge): 2 decimals is the largest cap where the worst case ("99.99") still lands inside the canvas, while 3 already clips past it (e.g. "99.999"). 2 decimals is also sufficient for the bug being fixed - any value >= 0.005 stays distinguishable from 0. Flat cap, not magnitude-aware like DBHeatmapChart's tickFormatter (which ignores configured mantissa entirely and only avoids the near-zero case) - out of scope here, and wouldn't fix a value like 99.99 either. --- .changeset/swift-owls-spend.md | 3 +- packages/app/src/HDXMultiSeriesTimeChart.tsx | 38 +++++++++++++++---- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 15 ++++++++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/.changeset/swift-owls-spend.md b/.changeset/swift-owls-spend.md index 92b19417b2..4a4efc27f2 100644 --- a/.changeset/swift-owls-spend.md +++ b/.changeset/swift-owls-spend.md @@ -8,5 +8,6 @@ The Y-axis of a time series chart always rounded tick labels to 0 decimal places, regardless of the chart's Number Format settings. Charts whose values live under 1 (fractional gauges, ratios, sub-1 rates) rendered every axis tick as `0` even though the tooltip and legend showed the correct -value. The axis now respects an explicitly configured mantissa, falling +value. The axis now respects an explicitly configured mantissa, capped at +2 decimal places to keep labels within the axis's fixed width, falling back to 0 only when none is set. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index f8e6549f28..5227b420b2 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -766,18 +766,42 @@ export function collectMemoChartGradientHexes( ); } +/** + * Y-axis tick labels get at most this many decimal places, regardless of a + * chart's configured Decimals (which can go up to 10 - see NumberFormat.tsx). + * + * `` (40px) leaves ~32px for the label itself + * after Recharts' own tickSize (6px) + tickMargin (2px). Measured in Chrome + * at 11px IBM Plex Mono (the tick font): "0.00" is 26px (fits), "99.99" is + * 33px (borderline), "99.9999" is 46px (well over). 2 decimals is enough to + * keep any value >= 0.005 distinguishable from 0 - the failure this cap + * exists to fix - without the axis regularly overflowing its column for + * ordinary values. + */ +const MAX_AXIS_MANTISSA = 2; + /** * Y-axis tick label formatter. Exported so a unit test can pin the * mantissa-precedence behavior without rendering recharts. * * `average` and `unit` are always forced (compact abbreviation like `1.2k` * reads better on an axis than a series' configured unit repeated on every - * tick), but an explicit `axisNumberFormat.mantissa` is honored rather than - * discarded. Without this, a chart whose configured Decimals produces - * correct tooltip/legend values (e.g. `0.14`) would still round every axis - * tick to `0` for any series whose values live under 1 (fractional - * Prometheus gauges, ratios, etc.), since `mantissa` only defaults to `0` - * here for the common case of unconfigured, typically-large counts. + * tick), but an explicit `axisNumberFormat.mantissa` is honored (capped at + * MAX_AXIS_MANTISSA) rather than discarded outright. Without this, a chart + * whose configured Decimals produces correct tooltip/legend values (e.g. + * `0.14`) would still round every axis tick to `0` for any series whose + * values live under 1 (fractional Prometheus gauges, ratios, etc.), since + * mantissa was previously forced to 0 unconditionally. + * + * The cap, rather than honoring mantissa outright, matters because an + * explicit mantissa is the common case, not a rare one: HyperDX's own + * bundled dashboard templates (go-runtime.json et al.) set mantissa on + * lines/byte/percent tiles for tooltip readability, with values well above + * 1 and no connection to the near-zero problem this cap fixes. The cap is + * flat rather than magnitude-aware (unlike DBHeatmapChart's tickFormatter, + * which ignores configured mantissa entirely) because a little unrequested + * precision on a large value (e.g. `256.20` instead of `256`) is harmless, + * while losing all precision on a small one (`0.14` rounding to `0`) is not. */ export function formatAxisTick( value: number, @@ -785,8 +809,8 @@ export function formatAxisTick( ): string { return axisNumberFormat ? formatNumber(value, { - mantissa: 0, ...axisNumberFormat, + mantissa: Math.min(axisNumberFormat.mantissa ?? 0, MAX_AXIS_MANTISSA), average: true, unit: undefined, }) diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 608f791c50..8f84d2bbb4 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -41,6 +41,21 @@ describe('formatAxisTick', () => { ); }); + it('caps an explicit mantissa at 2 instead of honoring it outright', () => { + // A flat cap, not magnitude-aware: HyperDX's own bundled dashboard + // templates (e.g. go-runtime.json) set mantissa on tiles whose values + // are well above 1, for tooltip readability, unrelated to the near-zero + // problem this formatter fixes. The Decimals setting allows up to 10, + // which would badly overflow the axis's ~32px label budget (measured: + // "0.1400000000" is far wider than "0.00" fits) if honored outright. + expect(formatAxisTick(0.14, { output: 'number', mantissa: 10 })).toBe( + '0.14', + ); + expect(formatAxisTick(1234, { output: 'number', mantissa: 10 })).toBe( + '1.23k', + ); + }); + it('always strips a configured unit and forces compact averaging', () => { expect( formatAxisTick(1234, { From 4f54359d292f5efe99458bdc91bf73b3fa92a7db Mon Sep 17 00:00:00 2001 From: arj22 Date: Mon, 31 Aug 2026 15:35:58 -0500 Subject: [PATCH 4/5] Make axis mantissa magnitude-aware: 0 for ticks >= 1, capped otherwise Capping the honored mantissa at 2 (previous commit) protected against an unbounded Decimals setting, but a chart with Decimals=2 and ordinary values still regressed: 200/400/600/800 rendered as 200.00/.../800.00, and go-runtime.json's byte tile (mantissa: 1, well under the cap) still went from 256 MB to 256.0 MB, since numbro doesn't trim trailing mantissa zeros and nothing in the cap stopped a small configured mantissa from applying to a large value. Caught in review, independently on both the deep-review bot and a second claude-code-review pass, both proposing the same fix as DBHeatmapChart's existing tickFormatter: mantissa is 0 whenever a tick's magnitude is >= 1, and only capped-at-2 for a sub-1 tick, where this PR's actual bug lives. Verified against the real formatNumber code paths (not just reasoning about it) that this fully restores go-runtime.json's tile to its original "256 MB" and a throughput tile to its original unpadded integer, since a raw byte or throughput value is essentially always >= 1. Updated tests: two prior assertions ('1.23k') were wrong under the new rule and are corrected to '1k', since a tick's magnitude, not its configured mantissa, now decides whether it's abbreviated with decimals. Added coverage for the >= 1 branch (including a negative tick) and for the go-runtime.json byte/throughput regression directly. --- .changeset/swift-owls-spend.md | 10 ++-- packages/app/src/HDXMultiSeriesTimeChart.tsx | 51 ++++++++++--------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 41 +++++++++++---- 3 files changed, 64 insertions(+), 38 deletions(-) diff --git a/.changeset/swift-owls-spend.md b/.changeset/swift-owls-spend.md index 4a4efc27f2..4fb8dc4dd9 100644 --- a/.changeset/swift-owls-spend.md +++ b/.changeset/swift-owls-spend.md @@ -2,12 +2,14 @@ "@hyperdx/app": patch --- -fix(charts): honor a configured mantissa on time chart Y-axis tick labels +fix(charts): show decimals on time chart Y-axis ticks under 1 The Y-axis of a time series chart always rounded tick labels to 0 decimal places, regardless of the chart's Number Format settings. Charts whose values live under 1 (fractional gauges, ratios, sub-1 rates) rendered every axis tick as `0` even though the tooltip and legend showed the correct -value. The axis now respects an explicitly configured mantissa, capped at -2 decimal places to keep labels within the axis's fixed width, falling -back to 0 only when none is set. +value. A tick under 1 now honors the chart's configured decimals, capped at +2 to keep the label within the axis's fixed width. A tick of 1 or more stays +an integer exactly as before, whatever the chart's Number Format configures, +so ordinary counts and the byte/percent tiles in the bundled dashboard +templates are unaffected. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 5227b420b2..a5e31721ae 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -767,16 +767,17 @@ export function collectMemoChartGradientHexes( } /** - * Y-axis tick labels get at most this many decimal places, regardless of a + * A tick under 1 gets at most this many decimal places, regardless of a * chart's configured Decimals (which can go up to 10 - see NumberFormat.tsx). + * Ticks >= 1 are always integers (see formatAxisTick) and so aren't governed + * by this - only a sub-1 label's own decimals can make it wide. * - * `` (40px) leaves ~32px for the label itself - * after Recharts' own tickSize (6px) + tickMargin (2px). Measured in Chrome - * at 11px IBM Plex Mono (the tick font): "0.00" is 26px (fits), "99.99" is - * 33px (borderline), "99.9999" is 46px (well over). 2 decimals is enough to - * keep any value >= 0.005 distinguishable from 0 - the failure this cap - * exists to fix - without the axis regularly overflowing its column for - * ordinary values. + * `` leaves a few dozen px for the label itself + * after Recharts' own tickSize + tickMargin. Measured in Chrome at 11px IBM + * Plex Mono (the tick font): "0.00" fits comfortably, "-0.99" is borderline, + * "-0.9999" is well over. 2 decimals is enough to keep any value >= 0.005 + * distinguishable from 0 - the failure this cap exists to fix - without a + * sub-1 label overflowing the column. */ const MAX_AXIS_MANTISSA = 2; @@ -786,22 +787,21 @@ const MAX_AXIS_MANTISSA = 2; * * `average` and `unit` are always forced (compact abbreviation like `1.2k` * reads better on an axis than a series' configured unit repeated on every - * tick), but an explicit `axisNumberFormat.mantissa` is honored (capped at - * MAX_AXIS_MANTISSA) rather than discarded outright. Without this, a chart - * whose configured Decimals produces correct tooltip/legend values (e.g. - * `0.14`) would still round every axis tick to `0` for any series whose - * values live under 1 (fractional Prometheus gauges, ratios, etc.), since - * mantissa was previously forced to 0 unconditionally. + * tick). For a tick >= 1, mantissa is always 0, matching DBHeatmapChart's + * tickFormatter and every previously-shipped chart byte-for-byte - large + * numbers stay `200`/`1k`/`256 MB`, never `200.00`/`1.23k`/`256.0 MB`, + * however many decimals the chart's Number Format configures. For a tick + * under 1, an explicit axisNumberFormat.mantissa is honored (capped at + * MAX_AXIS_MANTISSA) rather than forced to 0. Without that, a chart whose + * configured Decimals produces correct tooltip/legend values (e.g. `0.14`) + * would still round every axis tick to `0` for any series whose values live + * under 1 (fractional Prometheus gauges, ratios, etc.). * - * The cap, rather than honoring mantissa outright, matters because an - * explicit mantissa is the common case, not a rare one: HyperDX's own - * bundled dashboard templates (go-runtime.json et al.) set mantissa on - * lines/byte/percent tiles for tooltip readability, with values well above - * 1 and no connection to the near-zero problem this cap fixes. The cap is - * flat rather than magnitude-aware (unlike DBHeatmapChart's tickFormatter, - * which ignores configured mantissa entirely) because a little unrequested - * precision on a large value (e.g. `256.20` instead of `256`) is harmless, - * while losing all precision on a small one (`0.14` rounding to `0`) is not. + * An explicit mantissa is the common case, not a rare one, which is why the + * >= 1 branch can't just honor it: HyperDX's own bundled dashboard templates + * (go-runtime.json et al.) set mantissa on lines/byte/percent tiles for + * tooltip readability, with values well above 1 and no connection to the + * near-zero problem this formatter fixes. */ export function formatAxisTick( value: number, @@ -810,7 +810,10 @@ export function formatAxisTick( return axisNumberFormat ? formatNumber(value, { ...axisNumberFormat, - mantissa: Math.min(axisNumberFormat.mantissa ?? 0, MAX_AXIS_MANTISSA), + mantissa: + Math.abs(value) >= 1 + ? 0 + : Math.min(axisNumberFormat.mantissa ?? 0, MAX_AXIS_MANTISSA), average: true, unit: undefined, }) diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 8f84d2bbb4..c1e1340c56 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -41,18 +41,39 @@ describe('formatAxisTick', () => { ); }); - it('caps an explicit mantissa at 2 instead of honoring it outright', () => { - // A flat cap, not magnitude-aware: HyperDX's own bundled dashboard - // templates (e.g. go-runtime.json) set mantissa on tiles whose values - // are well above 1, for tooltip readability, unrelated to the near-zero - // problem this formatter fixes. The Decimals setting allows up to 10, - // which would badly overflow the axis's ~32px label budget (measured: - // "0.1400000000" is far wider than "0.00" fits) if honored outright. + it('caps a sub-1 tick`s mantissa at 2 instead of honoring it outright', () => { + // The Decimals setting allows up to 10, which would badly overflow the + // axis's label budget for a sub-1 tick (measured: "0.1400000000" is far + // wider than "0.14" fits) if honored outright. expect(formatAxisTick(0.14, { output: 'number', mantissa: 10 })).toBe( '0.14', ); - expect(formatAxisTick(1234, { output: 'number', mantissa: 10 })).toBe( - '1.23k', + }); + + it('forces 0 decimals for any tick >= 1, regardless of configured mantissa', () => { + // Regression (caught in review): HyperDX's own bundled dashboard + // templates (e.g. go-runtime.json) set mantissa on tiles whose values + // are well above 1, for tooltip readability, unrelated to the near-zero + // problem this formatter fixes - honoring it there instead of forcing 0 + // would turn shipped ticks like `500`/`1k` into `500.00`/`1.23k`, wide + // enough to overflow the axis's label budget for an ordinary value, not + // just an extreme one. + expect(formatAxisTick(200, { output: 'number', mantissa: 2 })).toBe('200'); + expect(formatAxisTick(1234, { output: 'number', mantissa: 10 })).toBe('1k'); + // A negative tick is still `>= 1` in magnitude (Math.abs), so it's + // integer too, unlike the sub-1-magnitude `-0.5` covered separately. + expect(formatAxisTick(-1.5, { output: 'number', mantissa: 2 })).toBe('-1'); + }); + + it('preserves shipped byte/throughput tiles that configure a mantissa', () => { + // go-runtime.json's "Memory: Used vs Limit vs GC Target" tile ships as + // { output: 'byte', mantissa: 1 } - a raw byte count is always >= 1, so + // this hits the forced-integer branch and stays `256 MB`, not `256.0 MB`. + expect(formatAxisTick(268435456, { output: 'byte', mantissa: 1 })).toBe( + '256 MB', + ); + expect(formatAxisTick(1234567, { output: 'throughput', mantissa: 2 })).toBe( + '1234567', ); }); @@ -63,7 +84,7 @@ describe('formatAxisTick', () => { mantissa: 2, unit: 'req/s', }), - ).toBe('1.23k'); + ).toBe('1k'); }); it('uses compact Intl formatting when no axisNumberFormat is set', () => { From e307b6a8242308ab63bba1b6ee4181cb21842e8a Mon Sep 17 00:00:00 2001 From: arj22 Date: Mon, 31 Aug 2026 16:18:14 -0500 Subject: [PATCH 5/5] Fix percent scaling and zero-tick regressions; raise threshold to 10 Two more real regressions caught in a third review round: - Percent charts take the sub-1 branch on the pre-x100 value: formatNumber multiplies a percent output by 100 before applying mantissa (a percent tile's raw value is a 0-1 ratio), so the magnitude check has to run against that displayed value, not the raw one - otherwise an ordinary 25% tile regresses to "25.00%", and go-runtime.json's process.cpu.utilization tile is exactly this shape. - Math.abs(0) is never >= any positive threshold, so the baseline 0 tick took the sub-1 branch and rendered "0.00"/"0.0 B". Since yAxisDomain defaults to [0, 'auto'], a 0 tick is present on nearly every chart, so this was a highly visible regression. 0 is unambiguous already and never needs the decimal rescue this formatter exists to provide, so it now always short-circuits to an integer. Also raised the magnitude threshold from >= 1 to >= 10. Measured: at a 2-decimal cap, a single extra character - a negative sign ("-9.99") or a percent suffix ("9.99%") - still exactly fits the axis's label budget, same width as "99.99". A 2-digit integer part combined with either character ("-99.99"/"99.99%") clips; that combination (and doubling up, a negative percent) is being left out of scope rather than threading output/sign-aware thresholds through this. This also substantially narrows the mixed-precision-on-one-axis problem flagged separately: ticks 0/0.5/1/1.5/2 on one chart no longer collapse 1.5 and 2 into the identical label "2", since they all now fall under the same branch. Ported the same fix to packages/cli/src/shared/formatNumber.ts's axisTickFormatter, a documented @source sync target (packages/cli/CONTRIBUTING.md) that still had the old unconditional mantissa: 0 - termchart-rendered tiles had the exact bug this PR fixes on the web. Added a changeset entry for @hyperdx/cli (versioned independently of the app/api/otel-collector fixed group) and a new test file mirroring the web's formatAxisTick coverage, since none existed for this file. All 9 new CLI tests passed on the first run, confirming the CLI's own formatNumber handles percent scaling identically to the web's. --- .changeset/swift-owls-spend.md | 23 +++-- packages/app/src/HDXMultiSeriesTimeChart.tsx | 90 ++++++++++++------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 62 +++++++++++-- .../cli/src/__tests__/formatNumber.test.ts | 61 +++++++++++++ packages/cli/src/shared/formatNumber.ts | 34 +++++-- 5 files changed, 214 insertions(+), 56 deletions(-) create mode 100644 packages/cli/src/__tests__/formatNumber.test.ts diff --git a/.changeset/swift-owls-spend.md b/.changeset/swift-owls-spend.md index 4fb8dc4dd9..57d4d07f5a 100644 --- a/.changeset/swift-owls-spend.md +++ b/.changeset/swift-owls-spend.md @@ -1,15 +1,20 @@ --- "@hyperdx/app": patch +"@hyperdx/cli": patch --- -fix(charts): show decimals on time chart Y-axis ticks under 1 +fix(charts): show decimals on Y-axis ticks under 10 in magnitude -The Y-axis of a time series chart always rounded tick labels to 0 decimal -places, regardless of the chart's Number Format settings. Charts whose -values live under 1 (fractional gauges, ratios, sub-1 rates) rendered every -axis tick as `0` even though the tooltip and legend showed the correct -value. A tick under 1 now honors the chart's configured decimals, capped at -2 to keep the label within the axis's fixed width. A tick of 1 or more stays -an integer exactly as before, whatever the chart's Number Format configures, -so ordinary counts and the byte/percent tiles in the bundled dashboard +The Y-axis of a time series chart (and the CLI's termchart equivalent) +always rounded tick labels to 0 decimal places, regardless of the chart's +Number Format settings. Charts whose values live under 1 (fractional +gauges, ratios, sub-1 rates) rendered every axis tick as `0` even though +the tooltip and legend showed the correct value. + +A tick under 10 in magnitude (as displayed - a percent tick's magnitude is +checked against its ×100 value, not its raw 0-1 ratio) now honors the +chart's configured decimals, capped at 2 to keep the label within the +axis's fixed width. A tick of 10 or more, and a tick of exactly 0, stay +integers exactly as before, whatever the chart's Number Format configures +- so ordinary counts and the byte/percent tiles in the bundled dashboard templates are unaffected. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index a5e31721ae..1cab711763 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -767,60 +767,88 @@ export function collectMemoChartGradientHexes( } /** - * A tick under 1 gets at most this many decimal places, regardless of a - * chart's configured Decimals (which can go up to 10 - see NumberFormat.tsx). - * Ticks >= 1 are always integers (see formatAxisTick) and so aren't governed - * by this - only a sub-1 label's own decimals can make it wide. + * A tick under MAGNITUDE_THRESHOLD gets at most this many decimal places, + * regardless of a chart's configured Decimals (which can go up to 10 - see + * NumberFormat.tsx). A tick >= MAGNITUDE_THRESHOLD is always an integer (see + * formatAxisTick) and so isn't governed by this - only a label under the + * threshold, with its own decimals, can make it wide. * * `` leaves a few dozen px for the label itself * after Recharts' own tickSize + tickMargin. Measured in Chrome at 11px IBM - * Plex Mono (the tick font): "0.00" fits comfortably, "-0.99" is borderline, - * "-0.9999" is well over. 2 decimals is enough to keep any value >= 0.005 - * distinguishable from 0 - the failure this cap exists to fix - without a - * sub-1 label overflowing the column. + * Plex Mono (the tick font, monospace): every character costs ~6.6px, and + * 5 characters is the most that fits. At 2 decimals, "9.99" (4 chars) fits + * with room to spare, and adding a single extra character - a negative + * sign ("-9.99") or a percent suffix ("9.99%") - still exactly fits at 5. + * A 2-digit integer part pushes either of those over (6 chars, e.g. + * "-99.99"/"99.99%" both clip), which is why MAGNITUDE_THRESHOLD is 10, not + * 100: it trades a wider decimal-preserving range for values that are both + * negative and percent-formatted (out of scope here) for one that's safe + * for plain positive numbers, percent, and negative numbers each on their + * own - the only combination this codebase's charts have needed decimals + * for so far. 2 decimals is also enough to keep any value >= 0.005 + * distinguishable from 0, the failure this cap exists to fix. */ const MAX_AXIS_MANTISSA = 2; +/** See MAX_AXIS_MANTISSA's comment for the width math behind this value. */ +const MAGNITUDE_THRESHOLD = 10; + /** * Y-axis tick label formatter. Exported so a unit test can pin the * mantissa-precedence behavior without rendering recharts. * * `average` and `unit` are always forced (compact abbreviation like `1.2k` * reads better on an axis than a series' configured unit repeated on every - * tick). For a tick >= 1, mantissa is always 0, matching DBHeatmapChart's - * tickFormatter and every previously-shipped chart byte-for-byte - large - * numbers stay `200`/`1k`/`256 MB`, never `200.00`/`1.23k`/`256.0 MB`, - * however many decimals the chart's Number Format configures. For a tick - * under 1, an explicit axisNumberFormat.mantissa is honored (capped at + * tick). For a tick at or past MAGNITUDE_THRESHOLD, mantissa is always 0 - + * large numbers stay `200`/`1k`/`256 MB`, never `200.00`/`1.23k`/`256.0 MB`, + * however many decimals the chart's Number Format configures. Below the + * threshold, an explicit axisNumberFormat.mantissa is honored (capped at * MAX_AXIS_MANTISSA) rather than forced to 0. Without that, a chart whose * configured Decimals produces correct tooltip/legend values (e.g. `0.14`) * would still round every axis tick to `0` for any series whose values live - * under 1 (fractional Prometheus gauges, ratios, etc.). + * under 1 (fractional Prometheus gauges, ratios, etc.). A tick of exactly 0 + * always short-circuits to an integer too - it's already unambiguous, and + * doesn't need the decimal rescue this formatter exists to provide. * * An explicit mantissa is the common case, not a rare one, which is why the - * >= 1 branch can't just honor it: HyperDX's own bundled dashboard templates - * (go-runtime.json et al.) set mantissa on lines/byte/percent tiles for - * tooltip readability, with values well above 1 and no connection to the + * large-magnitude branch can't just honor it: HyperDX's own bundled + * dashboard templates (go-runtime.json et al.) set mantissa on lines/byte/ + * percent tiles for tooltip readability, with values well above the * near-zero problem this formatter fixes. + * + * `formatNumber` multiplies a percent-output value by 100 before applying + * mantissa (a percent tile's raw value is a 0-1 ratio, e.g. `0.25` for + * "25%"), so the magnitude check runs against that same displayed value, + * not the raw one - otherwise every percent tile would take the small- + * magnitude branch regardless of how large the rendered percentage is. + * + * This diverges from DBHeatmapChart's tickFormatter (magnitude-aware at a + * >= 1 threshold, but ignoring configured mantissa entirely for values + * under it) - a deliberate difference in both the threshold and whether + * configured mantissa is honored at all, not an oversight. */ export function formatAxisTick( value: number, axisNumberFormat?: NumberFormat, ): string { - return axisNumberFormat - ? formatNumber(value, { - ...axisNumberFormat, - mantissa: - Math.abs(value) >= 1 - ? 0 - : Math.min(axisNumberFormat.mantissa ?? 0, MAX_AXIS_MANTISSA), - average: true, - unit: undefined, - }) - : new Intl.NumberFormat('en-US', { - notation: 'compact', - compactDisplay: 'short', - }).format(value); + if (!axisNumberFormat) { + return new Intl.NumberFormat('en-US', { + notation: 'compact', + compactDisplay: 'short', + }).format(value); + } + + const displayed = axisNumberFormat.output === 'percent' ? value * 100 : value; + + return formatNumber(value, { + ...axisNumberFormat, + mantissa: + displayed === 0 || Math.abs(displayed) >= MAGNITUDE_THRESHOLD + ? 0 + : Math.min(axisNumberFormat.mantissa ?? 0, MAX_AXIS_MANTISSA), + average: true, + unit: undefined, + }); } export const MemoChart = memo(function MemoChart({ diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index c1e1340c56..af96261cda 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -50,19 +50,63 @@ describe('formatAxisTick', () => { ); }); - it('forces 0 decimals for any tick >= 1, regardless of configured mantissa', () => { + it('forces 0 decimals for any tick >= 10, regardless of configured mantissa', () => { // Regression (caught in review): HyperDX's own bundled dashboard // templates (e.g. go-runtime.json) set mantissa on tiles whose values - // are well above 1, for tooltip readability, unrelated to the near-zero - // problem this formatter fixes - honoring it there instead of forcing 0 - // would turn shipped ticks like `500`/`1k` into `500.00`/`1.23k`, wide - // enough to overflow the axis's label budget for an ordinary value, not - // just an extreme one. + // are well above the threshold, for tooltip readability, unrelated to + // the near-zero problem this formatter fixes - honoring it there + // instead of forcing 0 would turn shipped ticks like `200`/`1k` into + // `200.00`/`1.23k`, wide enough to overflow the axis's label budget for + // an ordinary value, not just an extreme one. expect(formatAxisTick(200, { output: 'number', mantissa: 2 })).toBe('200'); expect(formatAxisTick(1234, { output: 'number', mantissa: 10 })).toBe('1k'); - // A negative tick is still `>= 1` in magnitude (Math.abs), so it's - // integer too, unlike the sub-1-magnitude `-0.5` covered separately. - expect(formatAxisTick(-1.5, { output: 'number', mantissa: 2 })).toBe('-1'); + expect(formatAxisTick(10, { output: 'number', mantissa: 2 })).toBe('10'); + // A negative tick is still `>= 10` in magnitude (Math.abs), so it's + // integer too. + expect(formatAxisTick(-15, { output: 'number', mantissa: 2 })).toBe('-15'); + }); + + it('honors configured mantissa up to 9.99 in magnitude, positive or negative', () => { + // Raised from a >= 1 threshold to >= 10: at a 2-decimal cap, a single + // extra character - a negative sign or a percent suffix - still fits + // the axis's label budget up to one integer digit (measured: "-9.99" + // and "9.99%" are both the same width as "99.99", which fits). This + // also mostly avoids the mixed-precision-on-one-axis problem a >= 1 + // threshold had: ticks 0, 0.5, 1, 1.5, 2 on one chart no longer collapse + // 1.5 and 2 into the identical label "2". + expect(formatAxisTick(9.99, { output: 'number', mantissa: 2 })).toBe( + '9.99', + ); + expect(formatAxisTick(-1.5, { output: 'number', mantissa: 2 })).toBe( + '-1.50', + ); + }); + + it('always renders exactly 0 as a bare integer', () => { + // Regression (caught in review): 0 has magnitude 0, which is under any + // positive threshold, so it took the capped-mantissa branch and + // rendered `0.00`/`0.0 B` - on essentially every chart, since the + // y-axis domain defaults to starting at 0. 0 is already unambiguous; + // it doesn't need the decimal rescue this formatter exists to provide. + expect(formatAxisTick(0, { output: 'number', mantissa: 2 })).toBe('0'); + expect(formatAxisTick(0, { output: 'byte', mantissa: 1 })).toBe('0 B'); + }); + + it('checks a percent tick`s magnitude against its displayed (x100) value', () => { + // Regression (caught in review): formatNumber multiplies a percent + // value by 100 before applying mantissa, since a percent tile's raw + // value is a 0-1 ratio. Checking the raw value's magnitude instead of + // the displayed one meant every percent tick took the capped-mantissa + // branch regardless of the rendered percentage - go-runtime.json's + // process.cpu.utilization tile (`{output: 'percent', mantissa: 1}`) + // would go from `25%` to `25.00%` for an ordinary, not-near-zero value. + expect(formatAxisTick(0.25, { output: 'percent', mantissa: 2 })).toBe( + '25%', + ); + // A genuinely near-zero percent (0.1%) still gets the decimal rescue. + expect(formatAxisTick(0.001, { output: 'percent', mantissa: 2 })).toBe( + '0.10%', + ); }); it('preserves shipped byte/throughput tiles that configure a mantissa', () => { diff --git a/packages/cli/src/__tests__/formatNumber.test.ts b/packages/cli/src/__tests__/formatNumber.test.ts new file mode 100644 index 0000000000..71f64140c3 --- /dev/null +++ b/packages/cli/src/__tests__/formatNumber.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from '@jest/globals'; + +import { axisTickFormatter } from '@/shared/formatNumber'; + +// Mirrors packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts's +// formatAxisTick suite - this file is a documented @source port of that +// function (see formatNumber.ts's axisTickFormatter docblock) and is +// expected to stay behaviorally in sync with it. +describe('axisTickFormatter', () => { + it('returns undefined when the chart has no number format', () => { + expect(axisTickFormatter(undefined)).toBeUndefined(); + }); + + it('falls back to mantissa 0 when the format has none configured', () => { + const format = axisTickFormatter({ output: 'number' }); + expect(format?.(1234)).toBe('1k'); + }); + + it('honors an explicit mantissa for a tick under the magnitude threshold', () => { + const format = axisTickFormatter({ output: 'number', mantissa: 2 }); + expect(format?.(0.14)).toBe('0.14'); + }); + + it('caps a small tick`s mantissa at 2 instead of honoring it outright', () => { + const format = axisTickFormatter({ output: 'number', mantissa: 10 }); + expect(format?.(0.14)).toBe('0.14'); + }); + + it('forces 0 decimals for any tick >= 10, regardless of configured mantissa', () => { + const format = axisTickFormatter({ output: 'number', mantissa: 2 }); + expect(format?.(200)).toBe('200'); + expect(format?.(10)).toBe('10'); + expect(format?.(-15)).toBe('-15'); + }); + + it('honors configured mantissa up to 9.99 in magnitude, positive or negative', () => { + const format = axisTickFormatter({ output: 'number', mantissa: 2 }); + expect(format?.(9.99)).toBe('9.99'); + expect(format?.(-1.5)).toBe('-1.50'); + }); + + it('always renders exactly 0 as a bare integer', () => { + expect(axisTickFormatter({ output: 'number', mantissa: 2 })?.(0)).toBe('0'); + expect(axisTickFormatter({ output: 'byte', mantissa: 1 })?.(0)).toBe('0 B'); + }); + + it('checks a percent tick`s magnitude against its displayed (x100) value', () => { + const format = axisTickFormatter({ output: 'percent', mantissa: 2 }); + expect(format?.(0.25)).toBe('25%'); + expect(format?.(0.001)).toBe('0.10%'); + }); + + it('preserves shipped byte/throughput tiles that configure a mantissa', () => { + expect( + axisTickFormatter({ output: 'byte', mantissa: 1 })?.(268435456), + ).toBe('256 MB'); + expect( + axisTickFormatter({ output: 'throughput', mantissa: 2 })?.(1234567), + ).toBe('1234567'); + }); +}); diff --git a/packages/cli/src/shared/formatNumber.ts b/packages/cli/src/shared/formatNumber.ts index 3827426e0a..97e750e17b 100644 --- a/packages/cli/src/shared/formatNumber.ts +++ b/packages/cli/src/shared/formatNumber.ts @@ -476,13 +476,28 @@ export function resolveChartNumberFormats( return { formatByColumn, chartFormat }; } +/** + * Below this magnitude (as displayed - see axisTickFormatter), a tick + * honors the chart's configured mantissa, capped at MAX_AXIS_MANTISSA; at + * or past it, a tick is always an integer. Mirrors + * packages/app/src/HDXMultiSeriesTimeChart.tsx's MAX_AXIS_MANTISSA / + * MAGNITUDE_THRESHOLD for behavioral parity with the web's y-axis - see + * that file's comment for the pixel-width reasoning behind these specific + * values (40px SVG axis column, 11px monospace font), which doesn't + * directly apply to a terminal chart's own width budget. Ported for + * consistency rather than independently derived for termchart's renderer. + */ +const MAX_AXIS_MANTISSA = 2; +const MAGNITUDE_THRESHOLD = 10; + /** * Build a termchart y-axis tick formatter from a chart's number format: - * compact, no decimals — the same semantics as the web's y-axis. - * Returns undefined (termchart default formatting) when the chart has - * no number format. + * compact, decimals only under MAGNITUDE_THRESHOLD (capped at + * MAX_AXIS_MANTISSA) - the same semantics as the web's y-axis. Returns + * undefined (termchart default formatting) when the chart has no number + * format. * - * @source packages/app/src/HDXMultiSeriesTimeChart.tsx (tickFormatter) + * @source packages/app/src/HDXMultiSeriesTimeChart.tsx (formatAxisTick) */ export function axisTickFormatter( numberFormat: NumberFormat | undefined, @@ -490,11 +505,16 @@ export function axisTickFormatter( if (!numberFormat) { return undefined; } - return (value: number) => - formatNumber(value, { + return (value: number) => { + const displayed = numberFormat.output === 'percent' ? value * 100 : value; + return formatNumber(value, { ...numberFormat, + mantissa: + displayed === 0 || Math.abs(displayed) >= MAGNITUDE_THRESHOLD + ? 0 + : Math.min(numberFormat.mantissa ?? 0, MAX_AXIS_MANTISSA), average: true, - mantissa: 0, unit: undefined, }); + }; }