Skip to content

WIP: share removed included taxes out between the rows - #910

Draft
cavalle wants to merge 1 commit into
mainfrom
pt-rounding
Draft

WIP: share removed included taxes out between the rows#910
cavalle wants to merge 1 commit into
mainfrom
pt-rounding

Conversation

@cavalle

@cavalle cavalle commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #895, which fixed the tax-inclusive totals under rounding: currency but not what happens when those prices are subsequently removed — which is mandatory for SAF-T/AT, as the format cannot express VAT-inclusive prices. Portuguese payment receipts were still being rejected as a result (Pylon #5267, customer Cloudbeds).

The currency calculator extracts the included tax from the sum of each rate's tax-inclusive line totals and shares it back over the lines, so a rate's base is not the sum of the tax removed from each line on its own. RemoveIncludedTaxes did the latter, rounding every line down: 12 nights at a tax-inclusive 125.00 with 6% VAT came out with a base of 1415.04 instead of the 1415.09 the invoice was issued with, plus a totals.rounding of 0.06 that formats unable to express it silently drop.

  • tax: new exported TotalCalculator.ExtractIncludedTaxes, returning the total of each taxable line with the included tax taken out. This is the same per-line distribution calculateCurrencyTotals already performed internally and threw away, so there is no second implementation to keep in step.
  • bill: RemoveIncludedTaxes under rounding: currency now snapshots the net total every row should end up with before touching the prices, then nudges the net line prices (and fixed document discount/charge amounts) until they add up to it, recalculating in between so line discounts and charges settle. Each nudged price takes the least accuracy that still reproduces its line's sum, so the sums stay recalculable from the prices presented; large quantities pick up extra decimals as needed (117.92453 for a quantity of 1000).
  • Documents using rounding: precise are untouched — every pre-existing removal test passes unchanged.

Result for the invoice above: base 1415.09, tax 84.91, total_with_tax 1500.00, no totals.rounding, with the lines carrying the remainder as 7 × 117.9245 + 5 × 117.93. Checked end-to-end against at-pt: SAF-T now renders NetTotal 1415.09 / TaxPayable 84.91 / GrossTotal 1500.00, and a receipt with base 1415.09 passes both validatePaymentLine and AvailToPay.ApplyPayment — the two checks that previously demanded different roundings of the same economics and so could not both be satisfied.

Multi-rate documents now get the correct base for every rate. A ±0.01 residual can still remain where a rate's tax cannot be recalculated from its own 2-decimal base at all (13% of 53.81 gives 7.00, but the tax-inclusive 60.80 implies 6.99); that is irreducible and still lands in totals.rounding, which is exactly what the field is for.

Pre-Review Checklist

  • Opened this PR as a draft
  • Read the CONTRIBUTING.md guide.
  • Performed a self-review of my code.
  • Added thorough tests with at least 90% code coverage.
  • Modified or created example GOBL documents to show my changes in use, if appropriate. — n/a: the example runner only calculates, it never calls RemoveIncludedTaxes, so no example can exercise this path. The pt and es prices-include examples added in Extract included taxes from rate group sums with currency rounding #895 already cover the calculation side.
  • Added links to the source of the changes in tax regimes or addons, either structured or in the comments.
  • Run go generate . to ensure that the Schemas and Regime data are up to date.
  • Reviewed and fixed all linter warnings.
  • Been obsessive with pointer nil checks to avoid panics.
  • Updated the CHANGELOG.md with an overview of my changes.
  • Marked this PR as ready for review.

And if you are part of the org:

  • Requested a review from Copilot and fixed or dismissed (with a reason) all the feedback raised.
  • Requested a review from @samlown.

🤖 Generated with Claude Code

With the currency rounding rule the tax included in the prices is
extracted from the sum of each rate's tax-inclusive line totals and
shared back over the lines, so the rate's base is not the sum of the tax
removed from every line on its own. RemoveIncludedTaxes did the latter,
rounding every line down: a hotel invoice of 12 nights at a tax-inclusive
125.00 with 6% VAT came out with a base of 1415.04 instead of the 1415.09
it was issued with, and a totals.rounding of 0.06 that formats unable to
express it silently drop.

The net totals every row should end up with are now taken from the same
distribution the tax totals apply, and the net line prices nudged until
they add up to them, gaining only the accuracy needed for each line's sum
to still be recalculable from the price presented. Documents using the
precise rounding rule are unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cavalle
cavalle requested a review from samlown July 27, 2026 14:29
@cavalle cavalle changed the title bill: share removed included taxes out between the rows WIP: share removed included taxes out between the rows Jul 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.31783% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.69%. Comparing base (43ca367) to head (9eb02f6).

Files with missing lines Patch % Lines
bill/tax_removal.go 69.90% 17 Missing and 14 partials ⚠️
bill/calculator.go 66.66% 2 Missing and 2 partials ⚠️
tax/totals_calculator_currency.go 85.71% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #910      +/-   ##
==========================================
- Coverage   94.87%   94.69%   -0.18%     
==========================================
  Files         320      321       +1     
  Lines       17431    17552     +121     
==========================================
+ Hits        16537    16621      +84     
- Misses        553      573      +20     
- Partials      341      358      +17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant