feat(billing): add invoice totals package - #73
Conversation
|
| Topic | Details | ||||||
|---|---|---|---|---|---|---|---|
| Invoice totals | Calculate invoice subtotals, discounts, tax, and final totals using LineItem, discount codes, exemption rules, and regional rates.Modified files (4)
Latest Contributors(1)
| ||||||
| Ledger reporting | Track invoice entries through settlement and expose outstanding balances with plain-text reports.Modified files (2)
Latest Contributors(1)
|
Merger
Needs Review
Billing totals still have concrete high-severity defects: fractional-cent results and float/ties-to-even rounding remain unaddressed, while invalid discount percentages can produce nonsensical totals. Human review is needed before merging.
Commit d2351ba · Evaluated 2026-08-11 18:11 UTC
![baz-reviewer-dev[bot]](https://avatars.githubusercontent.com/in/389167?s=60&v=4){kind=small}
| def average_item_price(items: list[LineItem]) -> float: | ||
| """Return the mean unit price across the invoice.""" | ||
| return subtotal(items) / len(items) |
There was a problem hiding this comment.
Average unit price is quantity-weighted
average_item_price divides the extended subtotal by line count instead of averaging unit prices, so invoices with quantities other than one report the wrong metric — should we compute sum(item.unit_price for item in items) / len(items) instead?
Want Baz to fix this for you? Activate Fixer
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`python/billing/invoice.py` around lines 20-22, fix the `average_item_price` function so
it computes the mean of each line item's `unit_price` rather than averaging the
quantity-expanded subtotal. Replace the use of `subtotal(items)` with a direct sum of
`item.unit_price` values while keeping the denominator as the number of invoice lines,
and preserve or explicitly handle the existing empty-input behavior.
There was a problem hiding this comment.
Commit 810c345 addressed this comment by computing the average from the sum of item.unit_price values divided by the number of invoice lines, excluding quantity weighting.
There was a problem hiding this comment.
Commit 810c345 addressed this comment by computing the arithmetic mean of item.unit_price values while retaining the line-count denominator, so quantities no longer affect the metric.
| def apply_discount(amount: float, percent: float) -> float: | ||
| """Apply a percentage discount to an amount.""" | ||
| return amount * percent |
There was a problem hiding this comment.
apply_discount multiplies amount by the raw percent value instead of a fraction, so a 10% discount on 100.0 returns 1000.0 instead of 90.0 — should we use amount * (1 - percent / 100)? Also, neither apply_discount nor invoice_total validates that percent falls within 0..100, so out-of-range values silently produce nonsensical totals — should we clamp or reject invalid percentages there too?
Want Baz to fix this for you? Activate Fixer
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
python/billing/invoice.py around lines 25-27, fix `apply_discount` so `percent` is
interpreted as a percentage reduction rather than a raw multiplier — convert it to a
fraction before calculating the discounted amount (e.g. `amount * (1 - percent / 100)`),
so a 10% discount on 100.0 returns 90.0. Additionally, in `invoice_total` (around lines
37-39), enforce that discount values fall within 0..100 by rejecting or consistently
clamping invalid inputs before computing the net total. Add or update tests covering 0%,
10%, 100%, and out-of-range values.
There was a problem hiding this comment.
Commit ed2ebb8 addressed this comment by converting the discount percentage into a fraction, so 10% of 100.0 yields 90.0. It did not add validation or clamping for out-of-range percentages.
There was a problem hiding this comment.
Commit ed2ebb8 addressed this comment by converting percent into a fraction in apply_discount, so a 10% discount correctly reduces the amount. However, percentage range validation in invoice_total remains unimplemented.
| def tax_for(amount: float, status: str) -> float: | ||
| """Return the tax owed on an amount for a customer status.""" | ||
| if status in TAX_EXEMPT_STATUSES: | ||
| return 0.0 | ||
| return amount * 0.17 |
There was a problem hiding this comment.
Regional invoice totals use Illinois tax
tax_for hardcodes the Illinois rate and invoice_total can't pass a jurisdiction through, so non-Illinois invoices get charged the wrong tax — should we add an explicit region parameter with tax.DEFAULT_REGION fallback and use tax.rate_for(region)?
Want Baz to fix this for you? Activate Fixer
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
python/billing/invoice.py around lines 30-39, update `tax_for` and `invoice_total` so
taxes are calculated for the invoice’s jurisdiction instead of using the hardcoded
0.17 rate. Add an explicit region parameter with the documented `tax.DEFAULT_REGION`
fallback, pass it from `invoice_total` to `tax_for`, and use `tax.rate_for(region)` for
non-exempt customers while preserving the existing exemption behavior.
There was a problem hiding this comment.
Commit bb7586b addressed this comment by adding an optional region parameter with a tax.DEFAULT_REGION fallback, passing it through invoice_total, and using tax.rate_for(region) while preserving exemptions.
There was a problem hiding this comment.
Commit bb7586b addressed this comment by adding an optional region to tax_for and invoice_total, using tax.DEFAULT_REGION as fallback and tax.rate_for(region). Existing tax-exemption behavior is preserved.
| for entry in ledger.entries: | ||
| state = "settled" if entry.settled else "open" | ||
| lines.append(f"{entry.invoice_id}\t{entry.amount:.2f}\t{state}") | ||
| return "\n".join(lines) |
There was a problem hiding this comment.
Report output permits row injection
Entry.invoice_id and titles flow unconstrained into render() and header(), so \t and \n characters can forge extra rows and corrupt the TSV output — should we reject or escape control characters at this output boundary?
Want Baz to fix this for you? Activate Fixer
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`python/billing/report.py` around lines 9-12, harden `render()` so unconstrained
`invoice_id` values cannot inject tabs, newlines, or other control characters into the
one-line TSV-like output. Add a shared escaping or validation helper and apply the same
policy to `header()` around lines 15-17, then add tests confirming control characters
cannot forge columns, rows, or terminal/log output.
| def _as_cents(amount: float) -> int: | ||
| """Return an amount in whole cents. | ||
|
|
||
| Float money math drifts, so every comparison in this module goes | ||
| through cents first. | ||
| """ | ||
| return int(round(amount * 100)) |
There was a problem hiding this comment.
Half-cent amounts map to wrong cents
_as_cents uses float arithmetic with ties-to-even rounding, so 1.005 silently rounds to 1.00 and equal_amounts(1.005, 1.00) returns True instead of applying an explicit currency rounding rule — should we convert to Decimal(str(amount)).quantize(Decimal("0.01"), rounding=ROUND_HALF_UP) or similar?
Want Baz to fix this for you? Activate Fixer
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`python/billing/invoice.py` around lines 23-29, fix `_as_cents` so currency conversion
does not depend on binary float representation or Python’s ties-to-even rounding.
Convert the amount through `Decimal(str(amount))`, quantize to `Decimal("0.01")` with an
explicitly chosen policy such as `ROUND_HALF_UP`, and then return whole cents; add the
required decimal imports and keep `equal_amounts` operating on integer cents.
| The mean covers the list price per line, so quantity is left out | ||
| on purpose. A quantity weighted average is a different metric. | ||
| """ | ||
| return sum(item.unit_price for item in items) / len(items) |
There was a problem hiding this comment.
Empty invoices crash average calculation
average_item_price([]) divides by zero because subtotal([]) is valid and invoice_total accepts empty collections without validation — should we define explicit empty-invoice behavior (return 0.0/None or raise a descriptive exception)?
Want Baz to fix this for you? Activate Fixer
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
python/billing/invoice.py around lines 42-48, update `average_item_price` so it
explicitly handles an empty `items` collection instead of dividing by zero. Raise a
descriptive `ValueError` for an empty invoice, document this behavior in the function
docstring, and add or update tests covering both empty and non-empty inputs.
| """Return the tax owed on an amount for a status and region.""" | ||
| if status in TAX_EXEMPT_STATUSES: | ||
| return 0.0 | ||
| return amount * tax.rate_for(region or tax.DEFAULT_REGION) |
There was a problem hiding this comment.
Unsupported regions raise raw KeyError
tax_for passes unchecked region values to tax.rate_for's RATES[region] lookup, so invalid regions raise KeyError — should we validate the region against tax.regions() first?
Want Baz to fix this for you? Activate Fixer
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`python/billing/invoice.py` around lines 56-60, update `tax_for` so externally supplied
regions are validated before calling `tax.rate_for`; currently unsupported truthy
strings cause an uncaught `KeyError`. Check membership using `tax.regions()` (or the
module’s supported-region contract), preserve `None` as the default region, and raise
the project-standard clear validation error for invalid regions.
| def invoice_total(items, status, discount_percent, region=None): | ||
| net = apply_discount(subtotal(items), discount_percent) | ||
| return net + tax_for(net, status, region) |
There was a problem hiding this comment.
Invoice totals return fractional cents
invoice_total returns the raw net + tax_for(...) result, so it produces 23.3883 instead of the chargeable whole-cent amount 23.39 — should we wrap it in _from_cents(_as_cents(net + tax_for(...)))?
Want Baz to fix this for you? Activate Fixer
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`python/billing/invoice.py` around lines 63-65, update `invoice_total` so its final
discounted subtotal plus tax is normalized to a whole-cent amount before returning. Use
the existing `_as_cents` and `_from_cents` helpers on the combined total, preserving the
current discount and tax calculation order.
1 participant
Adds a billing package with invoice totals, discount codes, tax lookup, ledger and reporting.