Switch to precise rounding when removing included taxes - #923
Open
samlown wants to merge 1 commit into
Open
Conversation
Removing taxes included in prices from a document using the currency rounding rule now switches it to precise, so the resulting tax bases and amounts match those of the original document. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #923 +/- ##
=======================================
Coverage 94.95% 94.95%
=======================================
Files 327 327
Lines 17668 17673 +5
=======================================
+ Hits 16777 16782 +5
Misses 549 549
Partials 342 342 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
@cavalle Claude came up with this solution for the calculation of prices that include tax in formats that don't support it. I think its reasonable to switch to precise calculations. |
cavalle
approved these changes
Aug 17, 2026
cavalle
left a comment
Contributor
There was a problem hiding this comment.
Oh! This makes sense! LGTM!!
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
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.
When an invoice combines
tax.prices_include: VATwithtax.rounding: currency,RemoveIncludedTaxesproduced a tax breakdown that no longer matched the original document.The problem
In currency mode,
extractIncludedTaxesderives the tax from the running sum of the tax-inclusive line totals and shares the rounding back over the lines, so the ex-tax base it reports is not the sum of independently-rounded per-line ex-tax amounts.The convertor discarded that: it divides each unit price by the rate (+2 decimal places), and
calculateLinethen re-rounds each line total down to the currency's precision. For identical lines the per-line loss has the same sign every time, so it accumulates:totals.roundingPayablestayed correct because the residual landed inTotals.Rounding(BT-144), but the tax breakdown itself — the part that gets reported — was wrong, and the error grew with the number of lines rather than staying within the "cent or two" the method documents.The fix
removeIncludedTaxesswitches acurrency-rounded document toprecise, resolving the regime default first so the rule is explicit in the output.Currency rounding's contract is that every amount is rounded to the currency before summing. Once included taxes are removed the unit price carries extra decimal places, so that contract is already broken and cannot be restored. Precise mode keeps the line total at the price's precision and rounds once at the base, which is arithmetically the same operation as
Percent.From(inclusive_sum). All four cases above then reproduce the original base and tax exactly, with no rounding adjustment at all.Alternative considered
Making currency mode respect the item price's precision when it exceeds the currency's also fixes the totals, but
examples/de/invoice-de-rounding-2.jsonpins the opposite requirement: 4-decimal input prices under currency rounding must yield 2-decimal line totals, as XRechnung/ZUGFeRD require. That approach was dropped.Trade-off
Converted documents now carry line totals with more decimal places than the currency. If a downstream format requires 2-decimal line amounts this matters, though the alternative was an incorrect tax breakdown plus a rounding line, which those formats also reject. The switch is a single condition and easy to gate at the addon or caller level if that turns out to be preferable.
Test
TestRemoveIncludedTaxes/with currency rounding rulecovers the 12 × 125.00 @ 6% case; verified failing without the fix (base 1415.04 vs 1415.09, tax 84.90 vs 84.91). Full suite passes.🤖 Generated with Claude Code