fix: harden number formatting utilities — negative zero, precision, and consistency#833
Open
fix: harden number formatting utilities — negative zero, precision, and consistency#833
Conversation
- Fix negative zero display ("-$0") in formatOtherPrice (GSW-2501)
- Fix formatRate producing "+0%" instead of "0%" when showSign is `true`
- Add null guard to `formatTokenAmount` `NaN` branch
- Use nullish coalescing (`??`) for `minLimit` to honor explicit 0
- Unify `minLimit` comparison and display threshold in
`formatOtherPrice`
- Add `isGreaterThan(0)` guard to minLimit checks across functions
- Replace Number() conversion with removeTrailingZeros to prevent
precision loss in `formatPoolPairAmount` and `formatOtherPrice`
- Fix removeTrailingZeros to preserve integer trailing zero (like
`"100"`)
- Extract parseToBigNumber and resolveMinLimit shared helpers
- Unify NumericInput type and BigNumber() call style
floating-point precision loss Replace `1 / Math.pow(10, decimals)` with `BigNumber(1).div(BigNumber(10).pow(decimals))` to eliminate IEEE 754 rounding errors at large decimal exponents (e.g. decimals=18). Return a `ResolvedMinLimit` struct that separates comparison (`value: BigNumber`) from display (`display: string` via `toFixed()`) to prevent exponential notation.
All formatting functions now follow a consistent pattern: 1. Parse input 2. Format abs 3. Derive sign via `resolveSign` 4. Assemble derived values and return it
internal re-parsing `toKMBFormat` previously accepted `BigNumber | string | number` and redundantly stripped commas, parsed to `BigNumber`, compute `abs()` and derived the sign. those are already handled via `parseToBigNumber` and `resolveSign`.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughA comprehensive numeric utility refactoring introducing centralized parsing and validation via Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/web/src/utils/number-utils.ts (1)
118-118: Minor inconsistency: Upper bound gate missing USD prefix.When
usd=trueand value exceeds 999.99B, the return value">999.99B"doesn't include the$prefix, unlike other tiers. Consider:♻️ Suggested fix
- if (value.isGreaterThan(999.99 * 1e9)) return ">999.99B"; + if (value.isGreaterThan(999.99 * 1e9)) return prefix + ">999.99B";
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/web/src/utils/new-number-utils.spec.tspackages/web/src/utils/new-number-utils.tspackages/web/src/utils/number-utils.tspackages/web/src/utils/stake-position-utils.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes Daily Earnings displaying as
-$0on the position detail page (GSW-2501) and addresses additional issues discovered in thenew-number-utils.tsformatting functions.Problem
1. Negative zero display (
-$0) — GSW-2501When the backend returned
accuReward1D: -6, multiplying by a very small token price produced a tiny negative USD value.formatOtherPricerounded this to "0" at 2 decimal places but preserved the negative sign, resulting in-$0.2.
formatRateproduced+0%When
showSign=trueand the value was zero, the sign was computed as "+" before the zero check, outputting+0%instead of0$.3. Inconsistent
minLimithandlingminLimit=0was treated as falsy by||and fell back to a decimals-derived limitformatOtherPriceto show as<$0.001(contradictory since0.005 > 0.001)1 / Math.pow(1o, decimals)used JS floating point arithmetic, losing precision at large exponents (e.g.decimal=18)4. Precision loss via
Number()conversionformatPoolPairAmountandformatOtherPriceusedNumber(\0.${fraction}`)` to strip trailing zeros, silently truncating decimals beyond IEEE 754 precision (~15 digits).5. Inconsistent trailing zero removal
Three different approaches were used:
removeTrailingZerosregex informatPrice,Number()conversion informatOtherPrice, and manual split/parse informatPoolPairAmount.6. Inconsistent sign handling
Each function determined the sign differently:
formatRate: computed sign upfront, then re-createdBigNumber(amount).abs()formatOtherPrice: computednegativeSignearly, checkedformatted === "0"at the endformatPrice: computednegativeSignearly, mixed signed/unsigned formatting paths7. Mixed input types in
toKMBFormatAccepted
BigNumber | string | numberand redundantly stripped commas, re-parsed toBigNumber, computedabs(), and derived the sign which are all already handled by callers.Changes
Shared helpers extracted
parseToBigNumber(value): Consolidates null/empty/NaN checks, comma stripping, and BigNumber parsing. Returns{ bigNum, abs, raw }.resolveMinLimit(minLimit, decimals): UsesBigNumber(1).div(BigNumber(10).pow(decimals))for arbitrary-precision threshold computation. Returns{ value: BigNumber, display: string }to separate comparison from display (avoids exponential notation like"1e-18").resolveSign(bigNum, formattedAbs, showSign?): Single source of truth for sign determination. Suppresses sign whenformattedAbs === "0", eliminating negative zero by design.NumericInput: Unified input type alias forstring | number | BigNumber | null | undefined.Summary by CodeRabbit
Tests
Refactor