Skip to content

fix(math): fmtCompact promotes the suffix when rounding reaches 1000 - #109

Open
hey-research-lab wants to merge 1 commit into
Twigpine:mainfrom
hey-research-lab:fix/compact-suffix-promotion
Open

hey-research-lab wants to merge 1 commit into
Twigpine:mainfrom
hey-research-lab:fix/compact-suffix-promotion

Conversation

@hey-research-lab

@hey-research-lab hey-research-lab commented Oct 19, 2026 •

Copy link
Copy Markdown

What was wrong

fmtCompact (app/src/lib/launchpad/math.ts) chooses K/M/B/T from the raw value and only then rounds inside that unit. A value just under a unit boundary therefore rounds up to "1000" of the smaller unit:

call before after
fmtCompact(999_999) 1000.00K 1.00M
fmtCompact(999_996_000) 1000.00M 1.00B
fmtUsd(999_960, { compact: true }) $1000.0K $1.0M

fmtQuoteUnits was already fixed for this ("no 1000.00K" in math.test.ts) by switching to Intl's compact notation, but fmtCompact itself is still used by fmtUsd({ compact: true }) (OG image volume, dashboard "Fees earned" / "Holdings value"), fmtTokens, the trade panel (amount out, minimum received, balances), the holders panel, the token page supply label and the trades table.

How it was verified

New test in math.test.ts; against the old math.ts it fails:

$ node --experimental-strip-types --test src/lib/launchpad/math.test.ts
not ok 8 - fmtCompact promotes the suffix when rounding reaches 1000 of a unit
  expected: '1.00M'
  actual: '1000.00K'
# pass 17
# fail 1

With the fix:

# tests 18
# pass 18
# fail 0

The full app suite (npm test), tsc --noEmit -p . and eslint were clean on main before the change (1369 pass, 11 skipped live tests); after the change math.ts typechecks under --strict and the math suite passes.

What changed

fmtCompact walks a [size, suffix] table; when the rounded mantissa in the chosen unit reaches 1000 it formats in the next unit up. Values below 1,000 and the — / 0.00 cases are unchanged.

Checks run (CONTRIBUTING)

cd app && npm run lint && npx tsc --noEmit -p . && npm test && npm run build

Lint and tsc clean; npm test: 1,370 passed, 0 failed, 11 skipped; the build completes. No contract, CI or dependency file is touched.

Summary by CodeRabbit

  • Bug Fixes
    • Compact number formatting now promotes values to the next suffix when rounding reaches 1,000, so values such as 999,999 display as 1.00M instead of 1000.00K.

fmtCompact picked its unit from the raw value and then rounded inside it, so 999,999 printed as "1000.00K" and a $999,960 compact USD figure (fmtUsd with compact, the OG image and the dashboard stats) as "$1000.0K". fmtQuoteUnits already avoids this by using Intl's compact notation; fmtCompact, which the trade panel, holders panel, token page and fmtTokens use, still did not. A value whose rounded mantissa reaches 1000 now takes the next unit.
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b90182d6-9c57-4e5c-b917-01273ff59ef5
📥 Commits

Reviewing files that changed from the base of the PR and between 1e006c3 and a79ba1b.

📒 Files selected for processing: 2
  • app/src/lib/launchpad/math.test.ts
  • app/src/lib/launchpad/math.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

fmtCompact now selects suffixes from a descending unit table and promotes values that round to 1000 of the selected unit. Tests cover suffix boundaries, negative values, configurable precision, trillion-scale values, and values below 1000.

Changes: Compact number formatting

Layer / File(s) Summary
Suffix selection and formatting tests
app/src/lib/launchpad/math.ts, app/src/lib/launchpad/math.test.ts
fmtCompact uses a descending suffix table and promotes values that round to 1000 when a larger suffix exists. Tests cover boundary promotion, negative values, precision options, trillion-scale values, and values below 1000.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: kevincodex1

Merge Risk: ⚪ Minimal · up to a79ba

Rounded compact values now use the next suffix when available, and the inspected tests and affected display paths match that behavior. No material merge risk was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: promoting the compact suffix when rounding reaches 1000.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@Vasanthdev2004 Vasanthdev2004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This fixes a real formatting edge case and the change stays nicely scoped. I checked the signed boundaries, the existing precision settings and the fmtUsd/fmtTokens callers. The 18 math tests and additional boundary checks pass; I didn't find a blocker in the two-file diff.

Full CI hasn't run yet: run 39199114992 is waiting for fork-run approval. Let's get that running and check the app/build results before merging. The green guard and CodeRabbit contexts aren't the full CI result.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants