Skip to content

Respect configured mantissa on time chart Y-axis tick labels - #3030

Open
arj22 wants to merge 2 commits into
hyperdxio:mainfrom
arj22:fix/axis-tick-mantissa
Open

Respect configured mantissa on time chart Y-axis tick labels#3030
arj22 wants to merge 2 commits into
hyperdxio:mainfrom
arj22:fix/axis-tick-mantissa

Conversation

@arj22

@arj22 arj22 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • HDXMultiSeriesTimeChart's Y-axis tickFormatter unconditionally set mantissa: 0, discarding whatever mantissa/decimals the chart's Number Format configured. Any series whose values live under 1 (fractional Prometheus gauges, ratios, sub-1 rates, etc.) therefore renders every axis tick as 0, even though the tooltip/legend show the correct value (ChartTooltip.tsx calls formatNumber(value, numberFormat) directly, with no such override).
  • Extracted the formatter into an exported formatAxisTick and reordered the spread so an explicit axisNumberFormat.mantissa wins, falling back to 0 only when unset. average: true and unit: undefined remain forced as before, so the existing compact-abbreviation behavior for typically-large, unconfigured counts (log/event counts, request rates) is unchanged.
image

Test plan

  • Added unit tests in HDXMultiSeriesTimeChart.test.ts (formatAxisTick) covering: no mantissa configured (unchanged behavior), explicit mantissa honored for sub-1 values, unit stripped/average forced regardless, and the no-axisNumberFormat Intl fallback.
  • yarn jest src/__tests__/HDXMultiSeriesTimeChart.test.ts — 33/33 passing.
  • tsc --noEmit clean.
  • eslint clean on both touched files (pre-existing warnings elsewhere in the file are unrelated to this change).

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.
@changeset-bot

changeset-bot Bot commented Aug 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 75c2cb8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Aug 29, 2026

Copy link
Copy Markdown

@arj22 is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added external needs-vouch Author needs a maintainer to vouch for them labels Aug 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Hi @arj22, thanks for the pull request!

Before we review code from a first-time contributor we ask that a maintainer vouches for you, and you're not on our list yet. This PR stays open — it just isn't in the review queue until someone vouches.

To get vouched, open an issue saying hello and what you're working on:

https://github.com/hyperdxio/hyperdx/issues/new?template=introduce-yourself.md

A maintainer will usually reply within a day or two, and then this PR gets picked up as normal. More detail in our contributing guide.

@greptile-apps

greptile-apps Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR updates time-chart Y-axis labels to honor an explicitly configured mantissa while preserving the existing zero-decimal fallback and compact formatting.

  • Extracts the axis formatter into the testable formatAxisTick helper.
  • Adds regression coverage for mantissa precedence, forced axis options, and fallback formatting.
  • Adds the required patch changeset for @hyperdx/app.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously missing app changeset is now present, correctly targets @hyperdx/app, and matches the repository’s release format.

Important Files Changed

Filename Overview
.changeset/swift-owls-spend.md Adds a correctly targeted patch changeset, resolving the previously reported release-record omission.
packages/app/src/HDXMultiSeriesTimeChart.tsx Reorders number-format precedence so explicit mantissa settings reach Y-axis tick labels while axis-specific overrides remain forced.
packages/app/src/tests/HDXMultiSeriesTimeChart.test.ts Adds focused coverage for configured and default mantissa behavior and the formatter fallback.

Reviews (2): Last reviewed commit: "Add changeset for axis tick mantissa fix" | Re-trigger Greptile

Comment thread packages/app/src/HDXMultiSeriesTimeChart.tsx
@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

The core change is sound. The Y-axis tickFormatter bug was a spread-order defect: the old { ...axisNumberFormat, average: true, mantissa: 0, unit: undefined } placed mantissa: 0 after the spread, unconditionally clobbering any configured mantissa. The new { mantissa: 0, ...axisNumberFormat, average: true, unit: undefined } makes 0 a default the spread overrides while still forcing average/unit. Precedence is correct, the no-axisNumberFormat fallback branch is unchanged, and the added tests pin the regression (0.14'0.14', 0.021'0.02') plus the fallback, unit-stripping, and Intl paths. Types are clean (no any/casts), and unit: undefined on the NumberFormat spread target compiles because exactOptionalPropertyTypes is off.

The prior "missing app changeset" review comment is resolved — .changeset/swift-owls-spend.md is present with a correct @hyperdx/app patch entry.

🔵 P3 nitpicks (2)
  • packages/app/src/HDXMultiSeriesTimeChart.tsx:788 — the literal mantissa: 0 before the spread is a no-op, since formatNumber already applies options.mantissa ?? 0 internally (utils.ts:997); the reorder plus that internal default is what actually establishes precedence.

    • Fix: drop the redundant mantissa: 0, or add an inline note that it only mirrors formatNumber's own default so it is not mistaken for load-bearing precedence logic.
  • .changeset/swift-owls-spend.md:2 — the frontmatter uses double quotes ("@hyperdx/app") while all existing changesets and yarn changeset output use single quotes; valid YAML either way, cosmetic only.

    • Fix: change "@hyperdx/app" to '@hyperdx/app' to match existing entries.

Reviewers (6): correctness, testing, maintainability, project-standards, kieran-typescript, previous-comments.

Testing gaps: (non-blocking, behavior unchanged from pre-fix code)

  • No test asserts average: true overrides an explicit axisNumberFormat.average: false (the doc comment's "always forces compact averaging" claim is only proven for the default case).
  • No coverage for NaN/negative or an explicit mantissa: undefined key; the last case is correct only because formatNumber applies ?? 0, so a regression removing that coalescing would not be caught here.

@arj22

arj22 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

@pulpdrew mind vouching + taking a look when you have a moment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external needs-vouch Author needs a maintainer to vouch for them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant