fix: Price display for high disparity pools #4199
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReworks price formatting in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Formatter as formatter.ts
Caller->>Formatter: getPriceExtendedFormatOptions(value: Dec)
alt value < 100
Formatter->>Formatter: stringify value, locate decimal part
Formatter->>Formatter: leadingZeros = leadingZerosCount(decimalPart)
Formatter->>Formatter: actualDecimalCount = decimalPart.length
Formatter->>Formatter: compute maximumSignificantDigits (fixed 4 or based on leadingZeros)
Formatter->>Formatter: compute maxDecimals (consider leading zeros and decimal count)
else
Formatter->>Formatter: integerLen = length(integer part)
Formatter->>Formatter: maximumSignificantDigits = integerLen + 2
Formatter->>Formatter: set maxDecimals (based on integerLen)
end
Formatter-->>Caller: { maxDecimals, notation, maximumSignificantDigits }
sequenceDiagram
autonumber
actor Caller
participant Formatter as formatter.ts
Caller->>Formatter: compressZeros(formattedValue, hasCurrencySymbol?, zerosThreshold?)
Formatter->>Formatter: strip sign and currency if present
alt has decimal
Formatter->>Formatter: count consecutive zeros after decimal (using leadingZerosCount)
alt zeros >= threshold
Formatter->>Formatter: preserve necessary leading zeros and keep significant digits
else
Formatter->>Formatter: trim redundant zeros
end
else
Formatter->>Formatter: return integer representation unchanged
end
Formatter-->>Caller: compressed string
sequenceDiagram
autonumber
actor Caller
participant Formatter as formatter.ts
Caller->>Formatter: formatFiatPrice(value, maxDecimals)
Formatter->>Formatter: build safe maxDecimalStr (handle missing decimals)
Formatter->>Formatter: format value, then pad/truncate decimal part to maxDecimals
Formatter-->>Caller: formatted fiat string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/web/utils/formatter.ts (2)
309-378: Inconsistent return shape when no decimal; always includezerosanddecimalDigits.Callers will expect stable keys. Also, assume multi-char currency signs and negatives; current
hasCurrencySymbol+[0]will misparse cases like “-¥123” or “CA$”.Apply minimal shape fix:
if (!punctuationSymbol) { return { currencySign, - significantDigits: formattedValue.substring(significantDigitsSubStart), + significantDigits: formattedValue.substring(significantDigitsSubStart), + zeros: 0, + decimalDigits: "", }; }And simplify unused boolean:
- const otherDigits = charsAfterPunct.substring(zerosCount); - - const canDisplayZeros = zerosCount !== 0 || otherDigits.length !== 0; - + const otherDigits = charsAfterPunct.substring(zerosCount); return { currencySign, significantDigits: formattedValue.substring( significantDigitsSubStart, punctIdx ), - zeros: canDisplayZeros ? zerosCount : 0, + zeros: zerosCount, decimalDigits: otherDigits, };Follow-ups (optional):
- Accept
currencySign: string | undefined(or infer viaIntl.NumberFormat.formatToParts) rather thanhasCurrencySymbol: boolean.- Use
formatToPartsto robustly locate decimal vs thousands separators for all locales.
396-411: Not locale-safe and drops trailing currency symbols; useIntl.formatToParts.Splitting on "." and rejoining with "." breaks locales using “,” and loses suffix currency symbols (e.g., “12 345,67 €”).
Apply:
- const splitPretty = formatPretty(maxDecimalPrice, { - ...getPriceExtendedFormatOptions(maxDecimalPrice.toDec()), - }).split("."); - - const decimalPart = (splitPretty[1] || "00").slice(0, maxDecimals); - const paddedDecimalPart = decimalPart.padEnd(maxDecimals, "0"); - - return splitPretty[0] + "." + paddedDecimalPart; + const fmtOpts = getPriceExtendedFormatOptions(maxDecimalPrice.toDec()); + const numForIntl = Number( + new IntPretty(maxDecimalPrice).maxDecimals(fmtOpts.maxDecimals).locale(false).toString() + ); + const intl = new Intl.NumberFormat(price.fiatCurrency.locale, { + style: "currency", + currency: price.fiatCurrency.currency, + ...fmtOpts, + }); + const parts = intl.formatToParts(numForIntl); + let fractionSeen = false; + const updated = parts.map((p) => { + if (p.type === "fraction") { + fractionSeen = true; + return { ...p, value: p.value.slice(0, maxDecimals).padEnd(maxDecimals, "0") }; + } + return p; + }); + if (!fractionSeen) { + const decimalSymbol = + parts.find((p) => p.type === "decimal")?.value ?? + (1.1).toLocaleString(price.fiatCurrency.locale).slice(1, 2); + // Insert decimal + zero-padded fraction before currency (or at end). + const insertAt = + updated.findIndex((p) => p.type === "currency") ?? updated.length; + updated.splice(insertAt, 0, { type: "decimal", value: decimalSymbol }, { type: "fraction", value: "".padEnd(maxDecimals, "0") } as Intl.NumberFormatPart); + } + return updated.map((p) => p.value).join("");
🧹 Nitpick comments (3)
packages/web/utils/formatter.ts (3)
244-269: Simplify: branch always assigns4for values < 100.Both inner branches set
maximumSignificantDigits = 4. Collapse to a single conditional for clarity.Apply:
- let maximumSignificantDigits: number; - - if (value.lt(new Dec(100))) { - // For prices < 100, check if we have many leading zeros after decimal point - const valueStr = value.toString(); - - // Only count leading zeros if the value has a decimal part - if (valueStr.includes(".")) { - const leadingZeros = leadingZerosCount(valueStr); - - // If we have 4+ leading zeros, we need more significant digits to show meaningful precision - // e.g., 0.0000005 needs at least 4 sig figs after the leading zeros to show price movements - if (leadingZeros >= 4) { - // Show 4 significant digits after leading zeros for smooth price tracking - // e.g., 0.000000330738247 → 7 leading zeros + 4 sig figs = 0.0000003307 - maximumSignificantDigits = 4; - } else { - maximumSignificantDigits = 4; - } - } else { - // No decimal point, use default - maximumSignificantDigits = 4; - } - } else { - maximumSignificantDigits = integerPartLength + 2; - } + let maximumSignificantDigits: number = + value.lt(new Dec(100)) ? 4 : integerPartLength + 2;
271-286: AvoidparseFloatto compute decimal count; preserve string precision.Parsing to float can switch to exponential and lose precision. Derive counts from the string.
Apply:
- const valueStr = value.toString(); - const actualDecimalCount = getDecimalCount(parseFloat(valueStr)); + const valueStr = value.toString(); + const actualDecimalCount = valueStr.includes(".") + ? valueStr.split(".")[1].length + : 0;Optional: cache
leadingZerosCount(valueStr)once and reuse for both branches.
296-307: Duplicate zero-counter; reuse importedleadingZerosCount.Remove
countLeadingZerosand use the existingleadingZerosCountto keep one source of truth.Apply:
-const countLeadingZeros = (decimalDigits: string) => { - let zeroCount = 0; - for (let i = 0; i < decimalDigits.length; i++) { - if (decimalDigits[i] === "0") { - zeroCount++; - } else { - break; - } - } - - return zeroCount; -};And at usage (see Line 352):
- const zerosCount = countLeadingZeros(charsAfterPunct); + const zerosCount = leadingZerosCount(`0.${charsAfterPunct}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web/utils/formatter.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/web/utils/formatter.ts (1)
packages/web/utils/number.ts (2)
leadingZerosCount(24-32)getDecimalCount(8-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: Summary
🔇 Additional comments (2)
packages/web/utils/formatter.ts (2)
14-16: LGTM: importingleadingZerosCountGood reuse of existing numeric utils for leading-zero handling.
238-294: Return-shape change is safe. No callers depend on the removedminimumSignificantDigits,minimumFractionDigits,maximumFractionDigits, ordisabledTrimZerosfields.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| const decimalPart = (splitPretty[1] || "00").slice(0, maxDecimals); | ||
| const paddedDecimalPart = decimalPart.padEnd(maxDecimals, "0"); | ||
|
|
||
| return splitPretty[0] + "." + paddedDecimalPart; |
There was a problem hiding this comment.
Bug: Decimal Separator Error in Price Formatting
The formatPrice function incorrectly appends a trailing decimal point (e.g., "123.") when maxDecimals is 0. This happens because the decimal separator is unconditionally added, even when no decimal places are intended, potentially creating invalid number strings for Dec construction.
There was a problem hiding this comment.
Couldn't find any examples where this was the case, all whole numbers had 0s appended
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/web/utils/formatter.ts (1)
274-288: Consider documenting the+ 2buffer in the maxDecimals formula.The formula
leadingZeros + maximumSignificantDigits + 2includes a+ 2buffer. While this safety margin ensures precision is preserved throughIntPretty, the reason for specifically adding 2 isn't explained in the comments.Consider adding a brief inline comment:
const maxDecimals = leadingZeros >= 4 ? Math.max( actualDecimalCount, - leadingZeros + maximumSignificantDigits + 2 + leadingZeros + maximumSignificantDigits + 2 // +2 buffer for rounding safety ) : Math.max(actualDecimalCount, 2);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web/utils/__tests__/formatter.spec.ts(1 hunks)packages/web/utils/formatter.ts(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web/utils/tests/formatter.spec.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/web/utils/formatter.ts (2)
packages/web/components/chart/light-weight-charts/utils.ts (1)
priceFormatter(7-30)packages/web/utils/number.ts (2)
getDecimalCount(8-22)leadingZerosCount(24-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: preview-swap-usdc-tests
- GitHub Check: Cursor Bugbot
- GitHub Check: Summary
🔇 Additional comments (4)
packages/web/utils/formatter.ts (4)
14-14: LGTM!The
leadingZerosCountimport is correctly added and used ingetPriceExtendedFormatOptions.
57-72: LGTM!The padding logic ensures consistent decimal places for
PricePrettyvalues when using standard notation with significant digits. This addresses the UX concern of inconsistent trailing zeros (e.g., $123,456.1 vs $123,456.10).
245-248: LGTM!The comment has been updated to correctly state "4 significant digits" which now matches the implementation. This addresses the previous concern about documentation contradicting behavior.
399-413: LGTM!The changes correctly ensure consistent decimal formatting by:
- Safely constructing
maxDecimalStrwith proper fallbacks for missing decimal parts- Padding the final decimal portion to exactly
maxDecimalsplacesThis guarantees consistent trailing zeros (e.g., $5.00 instead of $5.0 or $5).
2 participants
What is the purpose of the change:
Positions with base/quote that have high variation in value but are still within the bounds of the CL pool have functional charts but non-functional price displays.
Linear Task
https://linear.app/osmosis/issue/FE-1447/incorrect-price-display-for-babybtc
Brief Changelog
Changes use of minimumDecimals to be minimumSignificantDigits to allow prices such as 0.0000005215 to be displayed fully.
Testing and Verifying
This change has been tested locally by rebuilding the website and verified content and links are expected