Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/duration-axis-tick-width.md
Original file line number Diff line number Diff line change
@@ -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.
15 changes: 14 additions & 1 deletion packages/app/src/HDXMultiSeriesTimeChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -838,6 +843,14 @@ export function formatAxisTick(
}).format(value);
}

// 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') {
Comment thread
arj22 marked this conversation as resolved.
Resolved
Comment thread
arj22 marked this conversation as resolved.
Resolved
Comment thread
arj22 marked this conversation as resolved.
Resolved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 major — CLI's documented @source port of formatAxisTick not updated, so duration ticks still render 13.33min there

packages/cli/src/shared/formatNumber.ts:502 (axisTickFormatter) carries @source packages/app/src/HDXMultiSeriesTimeChart.tsx (formatAxisTick) and its test file states it "is expected to stay behaviorally in sync" (packages/cli/src/__tests__/formatNumber.test.ts:5-8), but it still falls straight through to formatNumber → formatDurationMs (packages/cli/src/shared/formatNumber.ts:218-221, 254) for output: 'duration'. A CLI dashboard tile on a trace-duration series (tileRender.ts:96 passes the resolved chartFormat) therefore keeps the wide "13.33min" label this PR is fixing, in an even tighter terminal gutter. Port formatDurationMsCompact into the CLI's formatNumber.ts and add the same duration branch to axisTickFormatter, plus the mirrored test cases.

Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond

const factor = axisNumberFormat.factor ?? 1;
Comment thread
arj22 marked this conversation as resolved.
Resolved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — Third copy of the duration value * factor * 1000 → compact-label conversion

The same two lines already exist in packages/app/src/components/DBHeatmapChart.tsx:1104-1108 (actualValue * (numberFormat?.factor ?? 1) * 1000 then formatDurationMsCompact) and the raw conversion in packages/app/src/utils.ts:1026-1030. Per the REQUIRED DRY rule, export one helper next to formatDurationMs in utils.ts (e.g. formatDurationAxisTick(value, numberFormat) or at minimum durationValueToMs(value, factor)) and call it from all three sites, so the factor semantics have a single source of truth.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

return formatDurationMsCompact(value * factor * 1000);
Comment thread
arj22 marked this conversation as resolved.
Resolved
Comment thread
arj22 marked this conversation as resolved.
Resolved
}

const displayed = axisNumberFormat.output === 'percent' ? value * 100 : value;

return formatNumber(value, {
Expand Down
24 changes: 24 additions & 0 deletions packages/app/src/__tests__/HDXMultiSeriesTimeChart.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,30 @@ describe('formatAxisTick', () => {
it('uses compact Intl formatting when no axisNumberFormat is set', () => {
expect(formatAxisTick(1234)).toBe('1.2K');
});

describe('duration output', () => {
// 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"', () => {
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', () => {
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', () => {
Expand Down
Loading