fix: PR D — decimal.js monetary math migration (round 4, bug 11)#81
Merged
Conversation
Replace float arithmetic on monetary values with decimal.js operating on Drizzle's string numeric representation. Round only at persist/display boundary. Primary fix (Bug 11): tax engine - calculateTaxes compound base: runningBase.plus(rounded) exact each step - backCalculateFromInclusive: gross / (1 + rate) now bit-exact - Percentage rate application Decimal-safe Full sweep — 11 business-logic files: - folio.service (reversal/negation, tax charge reversal) - payment.service (gateway amount, partial-refund idempotency key stability) - reservation.service (120% deposit, express-checkout balance guard) - reports.service (daily/financial/trend aggregation, ADR, RevPAR) - night-audit.service (totalRoom/totalTax, ADR, RevPAR) - connect-booking/search/insights.service (rate shopping, nightly breakdown, cancellation first-night penalty sums to totalAmount exactly) - channel/ari.service + rate-parity.service (override percentage math exact) parseFloat instances: 44 → 19 (25 removed). Remaining 19 intentionally kept: - tests (6), statistical/ML agent inputs (9), OTA XML boundary parsers (2), confidence probabilities (2). Documented by decision rule. 562/562 tests pass unchanged — existing tax assertions lived in float-safe zone; Decimal output matched at 2-decimal precision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Final PR from round-4 codex review. All monetary arithmetic now runs on `decimal.js` over Drizzle's string numeric representation.
Rule: Drizzle returns `numeric` as strings. Keep them as strings. Math via `new Decimal(str).plus/times/div(...).toFixed(2)`. Round only at the persist/display boundary.
Scope
Primary (Bug 11): tax engine — compound base, percentage rates, inclusive back-calc.
Full sweep across 11 business-logic files: folio, payment, reservation, reports, night-audit, connect-booking/search/insights, channel/ari + rate-parity.
`parseFloat` on money: 44 → 19 (25 removed). Remaining 19 intentionally kept (tests, statistical/ML agent inputs, OTA XML boundary parsers, probability values) — documented by decision rule.
Silent correctness wins
Test plan
Round 4 complete
PRs A (#78), B (#79), C (#80), D (this) address all 12 new bugs from the fourth codex review.
🤖 Generated with Claude Code