fix: sum all non-total entries per the spec formula in assert_totals_consistent - #76
Open
vishkaty wants to merge 1 commit into
Open
Conversation
…ries Observed: assert_totals_consistent picked only the first fulfillment, tax and fee entry via next(), ignored every type outside the well known set, then compared subtotal + fulfillment + tax + fee minus the absolute discount against the total entry. Expected: checkout.md (2026-04-08) Verification defines the formula directly: sum(e.amount for e in totals if e.type != "total") == total.amount checkout.md Repeating Types states that all types except subtotal and total MAY appear multiple times, for example multi jurisdiction tax lines or itemized fees, and the type field is an open string with custom entries such as the account_credit example in the same section. The old arithmetic misfired in both directions: * a conformant server returning two tax lines, two fee lines, or a custom entry (the split tax and account_credit examples are taken verbatim from checkout.md) failed with a false Total math mismatch; * a non conformant server whose total genuinely drops a repeated or custom entry passed, because the check dropped those same entries. The fix sums the signed amount of every non total entry per the spec formula, asserts exactly one total entry (checkout.md totals invariants), and keeps the configured subtotal and expected discount equality checks unchanged. Verified against both the Python and the Node reference servers (full suite green on both) and against spec example driven positive and negative fixtures: every conformant variation now passes and every genuinely broken totals array still fails.
damaz91
added
the
status:needs-triage
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
2 participants
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.
Observed
assert_totals_consistent(business_logic_test.py) computes the expected total assubtotal + fulfillment + tax + fee - abs(discount), taking each well-known type withnext()— i.e. the FIRST entry only — and ignoring any other total type. Per the 2026-04-08 spec this false-fails conformant servers:subtotalandtotalMAY appear multiple times — for example, multi-jurisdiction tax lines or itemized fees", and "thetypefield is an open string". A server with two tax lines, itemized fees, or a custom entry (e.g.duty,account_credit) fails "Total math mismatch" even when its totals are correct — including checkout.md's own Split-tax and account_credit examples.The check is also unsound in the other direction: because it sums only the first entry per type, a server whose
totalgenuinely omits a repeated tax line PASSES, and duplicatetotalentries pass.Fix
Use the spec's own Verification formula (checkout.md):
sum(e.amount for e in totals if e.type != "total") == total_entry.amount— a signed sum over all non-totalentries (amounts are signed; discounts are negative, so no special-casing). Add the spec's exactly-one-totalinvariant. The configured-subtotal and expected-discount equality checks are unchanged.This both relaxes (accepts spec-legal repeating/custom types) and tightens (now catches a total that drops a repeated entry, and duplicate totals).
Verification
duty); the new check accepts all and still fails a genuinely wrong total, wrong subtotal, wrong discount, a dropped repeated entry, and duplicate totals.