Skip to content

fix(billing-core): round decimal money exactly - #2883

Merged
kang-heewon merged 1 commit into
trunkfrom
fix/2472-money-decimal-rounding
Sep 23, 2026
Merged

kang-heewon merged 1 commit into
trunkfrom
fix/2472-money-decimal-rounding

Conversation

@kang-heewon

@kang-heewon kang-heewon commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Convert decimal money values to minor units with exact integer intermediates before applying the configured rounding mode, so binary floating-point multiplication cannot shift half-up boundaries.
  • Preserve calculated decimals, repeating fractions, tiny values, and large amounts whenever the final minor-unit result remains a safe integer.
  • Keep the public Money API unchanged and publish the behavior correction as a patch changeset for @croco/billing-core.

Closes #2472

Validation

  • Validation profile: Standard
  • Negative control: the focused regression failed on the prior implementation because 1.005 USD produced 100 instead of 101.
  • Focused Money suite: 16/16 passed; full @croco/billing-core suite: 184/184 passed.
  • @croco/billing-core lint, typecheck, and build passed.
  • pnpm pr-review-companion -- --base origin/trunk --head HEAD --run-required-checks passed, including architecture policy, public API drift, and pnpm check.
  • Normal pre-push gates passed with 243/243 full-repository test tasks and 243/243 guarded typecheck tasks.
  • Independent exact-head code review passed with no findings after 50,352 oracle comparisons across currencies, rounding modes, IEEE-754 edge cases, subnormal values, signed zero, and safe-integer boundaries.

Integration

  • Head: dd2f28b51133d77e6b27ecd3632c4dab5a1702d8
  • Validated base: 3ae15edb806855391cb0a5698f1ff0a1d7ff4c01
  • Current base: 00fe49487a3a5edfc736a58096fcdd6de1c20276
  • Base drift is limited to audit-drizzle source, tests, documentation, and its changeset; it does not interact with the billing-core decimal conversion contract, and GitHub reports the PR conflict-free.
  • No frontend-visible or UX-visible behavior changed.

Copilot AI lite review requested due to automatic review settings September 22, 2026 17:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 38 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 21864b61-ee5a-4833-9dbb-871bb9084c3c

📥 Commits

Reviewing files that changed from the base of the PR and between 00fe494 and dd2f28b.

📒 Files selected for processing (3)
  • .changeset/exact-money-decimal-rounding.md
  • packages/billing-core/src/libs/Money.ts
  • packages/billing-core/src/tests/Money.spec.ts

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.

❤️ Share

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

@github-actions

Copy link
Copy Markdown

📊 Benchmark Results

✅ All benchmarks passed

Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 36.3μs 30.0ms 8.2μs +343.5% ❌ -
CrocoApp lambdaHandler (10 controllers) 1.7ms 50.0ms 258.4μs +574.3% ❌ -
Lambda cold-start simulation 1.5ms 80.0ms 418.1μs +251.4% ❌ -
Lambda cold-start with headers 1.2ms 80.0ms 369.7μs +224.7% ❌ -
Lambda cold-start with binary body 1.2ms 80.0ms 339.1μs +253.2% ❌ -
Lambda cold-start with query params 1.2ms 80.0ms 301.3μs +299.6% ❌ -
Lambda cold-start with authorizer context 1.2ms 80.0ms 299.8μs +286.0% ❌ -
Lambda cold-start realistic scenario 1.1ms 80.0ms 299.2μs +281.2% ❌ -
EventBusConfig.start (10 handlers) 1.7μs 10.0ms 1.4μs +16.1% ✅ -
EventPublisher.publishNow single event 1.7μs 2.0ms 1.7μs -0.7% ✅ -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.1μs -25.0% ✅ -
Container.get singleton (cold) 52.5μs 5.0ms 70.3μs -25.2% ✅ -
Container.register × 50 components 2.7ms 10.0ms 3.2ms -15.9% ✅ -
Container.validate (50 components) 3.1ms 20.0ms 3.4ms -7.5% ✅ -
Container.get singleton (warm) 1.2μs 500.0μs 1.6μs -27.6% ✅ -
TelemetryRuntime.init (lambda preset) 9.9μs 200.0ms 1.1ms -99.1% ✅ -
lambdaPreset config creation 1.1μs 2.0ms 1.4μs -21.2% ✅ -

Updated: 2026-09-22T17:50:15.353Z · Commit: dd2f28b

@kang-heewon
kang-heewon merged commit c74e71d into trunk Sep 23, 2026
21 checks passed
@kang-heewon
kang-heewon deleted the fix/2472-money-decimal-rounding branch September 23, 2026 00:13
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.

[billing-core] Money.fromDecimal이 부동소수점 곱셈을 사용해 1.005 USD가 1.00으로 반올림된다

2 participants