From 6e45c5c6991e01db81918034a39a8ad5880ed1bd Mon Sep 17 00:00:00 2001 From: arj22 Date: Thu, 17 Sep 2026 20:09:35 -0500 Subject: [PATCH 1/2] fix(app): route duration axis ticks through the compact formatter formatAxisTick's width-safety logic (MAX_AXIS_MANTISSA/MAGNITUDE_THRESHOLD, from #3030) never applied to output: 'duration' - formatNumber's own duration branch returns early with formatDurationMs before any of the mantissa/average/unit overrides are consulted, and formatDurationMs has no width budget of its own (e.g. "13.33min", 8 chars, well past what the axis's label area fits). Routes duration through formatDurationMsCompact instead, already used for the same reason by DBHeatmapChart's own axis. Co-Authored-By: Claude Sonnet 5 --- packages/app/src/HDXMultiSeriesTimeChart.tsx | 22 ++++++++++- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 38 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 1cab711763..0cf465749d 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -32,7 +32,12 @@ import { DisplayType } from '@hyperdx/common-utils/dist/types'; import { Popover } from '@mantine/core'; import type { NumberFormat } from '@/types'; -import { COLORS, formatNumber, truncateMiddle } from '@/utils'; +import { + COLORS, + formatDurationMsCompact, + formatNumber, + truncateMiddle, +} from '@/utils'; import { AnnotationHitLayer, @@ -838,6 +843,21 @@ export function formatAxisTick( }).format(value); } + // 'duration' is handled separately from the numbro-based formatting below: + // formatNumber's own 'duration' branch returns early with formatDurationMs, + // before ever looking at the mantissa/average/unit overrides this function + // passes in - so none of MAX_AXIS_MANTISSA/MAGNITUDE_THRESHOLD's width + // safety applies to it, and formatDurationMs has no width budget of its own + // (e.g. "13.33min", 8 chars, well past what Y_AXIS_WIDTH fits). Route + // through formatDurationMsCompact instead - already used for this exact + // reason by DBHeatmapChart's axis - which uses a single-character unit for + // minutes ("m" vs "min") and significant-figure precision that shrinks as + // the value grows, keeping labels short at any magnitude. + if (axisNumberFormat.output === 'duration') { + const factor = axisNumberFormat.factor ?? 1; + return formatDurationMsCompact(value * factor * 1000); + } + const displayed = axisNumberFormat.output === 'percent' ? value * 100 : value; return formatNumber(value, { diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index af96261cda..9642673580 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -134,6 +134,44 @@ describe('formatAxisTick', () => { it('uses compact Intl formatting when no axisNumberFormat is set', () => { expect(formatAxisTick(1234)).toBe('1.2K'); }); + + describe('duration output', () => { + // Regression: 'duration' hits formatNumber's own early-return branch + // (`if (options.output === 'duration') return formatDurationMs(ms)`), + // before the mantissa/average/unit overrides above are ever consulted - + // so none of this formatter's width safety (MAX_AXIS_MANTISSA, + // MAGNITUDE_THRESHOLD) applied to it, and formatDurationMs has no width + // budget of its own: a value like 799.8s rendered as "13.33min" (8 + // chars), well past what the axis's label budget fits (measured + // elsewhere in this file at up to ~5 chars). Fixed by routing 'duration' + // through formatDurationMsCompact instead - already used for the same + // reason by DBHeatmapChart's own axis - which uses a single-character + // minute unit ("m", not "min") and significant-figure precision that + // shrinks as the value grows. + it('renders minutes with a single-character unit, not "min"', () => { + // 799.8s = 13.33min old-formatter would render "13.33min" (8 chars). + expect(formatAxisTick(799.8, { output: 'duration' })).toBe('13m'); + }); + + it('renders seconds compactly', () => { + expect(formatAxisTick(39.51, { output: 'duration' })).toBe('39.5s'); + }); + + it('respects a configured factor, matching formatNumber`s own duration math', () => { + // A raw value already in milliseconds (e.g. this dashboard's ClickHouse + // latency columns) is passed with factor: 0.001 so formatNumber's own + // `value * factor * 1000` nets out to the raw ms - same convention + // this formatter's duration branch has to honor. + expect( + formatAxisTick(442_800, { output: 'duration', factor: 0.001 }), + ).toBe('7.4m'); + }); + + it('renders sub-millisecond and hour-scale values compactly too', () => { + expect(formatAxisTick(0.0000005, { output: 'duration' })).toBe('500ns'); + expect(formatAxisTick(7500, { output: 'duration' })).toBe('2.1h'); + }); + }); }); describe('collectMemoChartGradientHexes', () => { From f16ea7f5ac365073a8308a0832e475dbb20340b2 Mon Sep 17 00:00:00 2001 From: arj22 Date: Thu, 17 Sep 2026 20:26:16 -0500 Subject: [PATCH 2/2] address review: add changeset, trim duplicate comments - Add missing changeset (@hyperdx/app patch) per AGENTS.md's release process. - Condense the formatAxisTick comment and the test file's duration-output block down to the non-obvious reason, dropping restated examples/numbers the tests and diff already show. Co-Authored-By: Claude Sonnet 5 --- .changeset/duration-axis-tick-width.md | 7 +++++++ packages/app/src/HDXMultiSeriesTimeChart.tsx | 13 +++--------- .../__tests__/HDXMultiSeriesTimeChart.test.ts | 20 +++---------------- 3 files changed, 13 insertions(+), 27 deletions(-) create mode 100644 .changeset/duration-axis-tick-width.md diff --git a/.changeset/duration-axis-tick-width.md b/.changeset/duration-axis-tick-width.md new file mode 100644 index 0000000000..4b3192989f --- /dev/null +++ b/.changeset/duration-axis-tick-width.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/app': patch +--- + +fix: keep duration axis-tick labels within the chart's width budget + +Duration-formatted Y-axis labels (e.g. "13.33min") could render wider than the axis area allows and get clipped. They now use the same compact formatter already used by the heatmap chart's axis. diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 0cf465749d..2e5c6cb004 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -843,16 +843,9 @@ export function formatAxisTick( }).format(value); } - // 'duration' is handled separately from the numbro-based formatting below: - // formatNumber's own 'duration' branch returns early with formatDurationMs, - // before ever looking at the mantissa/average/unit overrides this function - // passes in - so none of MAX_AXIS_MANTISSA/MAGNITUDE_THRESHOLD's width - // safety applies to it, and formatDurationMs has no width budget of its own - // (e.g. "13.33min", 8 chars, well past what Y_AXIS_WIDTH fits). Route - // through formatDurationMsCompact instead - already used for this exact - // reason by DBHeatmapChart's axis - which uses a single-character unit for - // minutes ("m" vs "min") and significant-figure precision that shrinks as - // the value grows, keeping labels short at any magnitude. + // formatNumber returns early for 'duration', before the mantissa/width + // safety below ever runs, and formatDurationMs has no width budget of its + // own - use the compact formatter instead, as DBHeatmapChart's axis does. if (axisNumberFormat.output === 'duration') { const factor = axisNumberFormat.factor ?? 1; return formatDurationMsCompact(value * factor * 1000); diff --git a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts index 9642673580..2f8ac5df38 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts +++ b/packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts @@ -136,20 +136,10 @@ describe('formatAxisTick', () => { }); describe('duration output', () => { - // Regression: 'duration' hits formatNumber's own early-return branch - // (`if (options.output === 'duration') return formatDurationMs(ms)`), - // before the mantissa/average/unit overrides above are ever consulted - - // so none of this formatter's width safety (MAX_AXIS_MANTISSA, - // MAGNITUDE_THRESHOLD) applied to it, and formatDurationMs has no width - // budget of its own: a value like 799.8s rendered as "13.33min" (8 - // chars), well past what the axis's label budget fits (measured - // elsewhere in this file at up to ~5 chars). Fixed by routing 'duration' - // through formatDurationMsCompact instead - already used for the same - // reason by DBHeatmapChart's own axis - which uses a single-character - // minute unit ("m", not "min") and significant-figure precision that - // shrinks as the value grows. + // Regression: 'duration' bypasses this formatter's width safety entirely + // (formatNumber returns early for it), and formatDurationMs has no width + // budget of its own - e.g. "13.33min" is 8 characters. it('renders minutes with a single-character unit, not "min"', () => { - // 799.8s = 13.33min old-formatter would render "13.33min" (8 chars). expect(formatAxisTick(799.8, { output: 'duration' })).toBe('13m'); }); @@ -158,10 +148,6 @@ describe('formatAxisTick', () => { }); it('respects a configured factor, matching formatNumber`s own duration math', () => { - // A raw value already in milliseconds (e.g. this dashboard's ClickHouse - // latency columns) is passed with factor: 0.001 so formatNumber's own - // `value * factor * 1000` nets out to the raw ms - same convention - // this formatter's duration branch has to honor. expect( formatAxisTick(442_800, { output: 'duration', factor: 0.001 }), ).toBe('7.4m');