diff --git a/.changeset/swift-owls-spend.md b/.changeset/swift-owls-spend.md new file mode 100644 index 0000000000..57d4d07f5a --- /dev/null +++ b/.changeset/swift-owls-spend.md @@ -0,0 +1,20 @@ +--- +"@hyperdx/app": patch +"@hyperdx/cli": patch +--- + +fix(charts): show decimals on Y-axis ticks under 10 in magnitude + +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 922f07ccc9..1cab711763 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -766,6 +766,91 @@ export function collectMemoChartGradientHexes( ); } +/** + * 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, 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 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.). 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 + * 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 { + 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({ graphResults, setIsClickActive, @@ -1115,19 +1200,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..af96261cda 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,122 @@ 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('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', + ); + }); + + 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 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'); + 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', () => { + // 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', + ); + }); + + it('always strips a configured unit and forces compact averaging', () => { + expect( + formatAxisTick(1234, { + output: 'number', + mantissa: 2, + unit: 'req/s', + }), + ).toBe('1k'); + }); + + 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([]); 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, }); + }; }