amadan
Per-line tax codes: the second code review answered (the resumed finalize validates the invoice it read; an empty split asks for no weights; the glue's guards and the apportionment review tested; the exact wrong-code recovery outcome and the in-window first-line replay; the one-line record frozen properly; wording)
Opus P3-1: validateDraftLinesOf takes the invoice stepFinalizing read, so a finalize landing between reads cannot look like one made outside; the design and the merge's ENABLEMENT text state that the tightening applies to every month. Opus P3-2: bodySplitByGrossUsage returns before asking for weights when there are no lines (a house month without Days is waived under the split). Opus P3-3: the body's shape is the two proposals', said so in the design and comments. Opus P3-4: the one-emitted-code record is frozen through freezeBillingMonth. Opus P3-5 and Astra: the glue refuses a one-code calculation and the transfer weights refuse overflow, tested; the apportionment review is tested through the preparer seam. Astra 2: the wrong-code recovery asserts the unrecovered review alone, the first operation unacknowledged and no sibling created; the first line's loss inside the window asserts the replay's arrival. 32 mutants (taxcodes-mutants-round3.txt): all die but M14, equivalent. Design revision 6; dispositions and both reports in the tree; decision 16 reworded on the launch branch (68b16c52).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8 files changed,
+382
−32
docs/superpowers/specs/2026-10-03-per-line-tax-codes-design.md+8 −8docs/superpowers/specs/CODEX-FINDINGS-per-line-tax-codes-review2.md+106 −0docs/superpowers/specs/DISPOSITIONS-per-line-tax-codes.md+16 −1docs/superpowers/specs/OPUS-REVIEW-per-line-tax-codes-review2.md+119 −0internal/console/billinginvoice.go+10 −2internal/console/billingtaxlifecycle_test.go+60 −13internal/console/billingtaxsplit.go+21 −8internal/console/billingtaxsplit_test.go+42 −0
CI passed — run details
diff --git a/docs/superpowers/specs/2026-10-03-per-line-tax-codes-design.md b/docs/superpowers/specs/2026-10-03-per-line-tax-codes-design.md| index 64f10befa..c62646a3a 100644 |
| --- a/docs/superpowers/specs/2026-10-03-per-line-tax-codes-design.md |
| +++ b/docs/superpowers/specs/2026-10-03-per-line-tax-codes-design.md |
| @@ -1,6 +1,6 @@ |
| # Per-line tax codes for a month in which the plan changed |
| -Date: 2026-10-03. Status: design, revision 5, as built (the first code review, Opus and Astra both accept with fixes, corrected three claims here: activation is more than one line, the glue now exists uncalled; maps are not all iterated in sorted order, and need not be; a waived calculation has no lines. Revision 4 was built from (revision 3 accepted with fixes by Astra's confirming review: no P1; one P2, the month-end answer's daily-data exemption contradicted partial-month pricing, and two qualifications, the waiver sentence and the older worker's nonterminal records; corrected here. Revision 2 accepted with fixes by Astra: no P1; three P2, a daily-data requirement that would regress one-code months, the line identity not checked on every acceptance path, two readings of the transfer arithmetic; two P3, references not yet written, "the next pass" promised without its conditions; each answered below. Revision 1 rejected by Astra on two P1s, four P2s and a P3: the invoice engine's per-meter line identity, held months that cannot resume, an older worker's durable review, the day-eligibility rule, policy stated as mechanics, imprecise apportionment, the zero-usage statement; each answered below). Answers `OUTSTANDING.md` item 17 and prepares decision 16 (`DECISIONS-FOR-PAUL.md`), which is Paul's; nothing here chooses it. Branch `design-per-line-tax-codes`, off the launch branch at `29ea6676`. |
| +Date: 2026-10-03. Status: design, revision 6, as built (the second code review, both accept with fixes, no P1 or P2, corrected: the body's shape covers the two proposals and not every conceivable answer; the shape rule is `taxShapeOK` beside `wellFormed`; the apportionment review is tested through the preparer seam; a resumed finalize validates the invoice it read and that tightening applies to every month. Revision 5 was as built after the first code review (the first code review, Opus and Astra both accept with fixes, corrected three claims here: activation is more than one line, the glue now exists uncalled; maps are not all iterated in sorted order, and need not be; a waived calculation has no lines. Revision 4 was built from (revision 3 accepted with fixes by Astra's confirming review: no P1; one P2, the month-end answer's daily-data exemption contradicted partial-month pricing, and two qualifications, the waiver sentence and the older worker's nonterminal records; corrected here. Revision 2 accepted with fixes by Astra: no P1; three P2, a daily-data requirement that would regress one-code months, the line identity not checked on every acceptance path, two readings of the transfer arithmetic; two P3, references not yet written, "the next pass" promised without its conditions; each answered below. Revision 1 rejected by Astra on two P1s, four P2s and a P3: the invoice engine's per-meter line identity, held months that cannot resume, an older worker's durable review, the day-eligibility rule, policy stated as mechanics, imprecise apportionment, the zero-usage statement; each answered below). Answers `OUTSTANDING.md` item 17 and prepares decision 16 (`DECISIONS-FOR-PAUL.md`), which is Paul's; nothing here chooses it. Branch `design-per-line-tax-codes`, off the launch branch at `29ea6676`. |
| ## What happens today |
| @@ -15,7 +15,7 @@ So the only thing missing is which tax code each overage line carries. That is d |
| ## What is the owner's, stated as such |
| -Decision 16 says "split by span, with the usage of each UTC day attributed to the plan in force on that day" or "the month-end plan's code". Neither sentence is precise enough to build from, and the refinements are choices, not mechanics. They are listed in `DECISIONS-FOR-PAUL.md` under 16 as 16a to 16d and this design chooses none of them; it builds so that any answer is one body of one function: |
| +Decision 16 says "split by span, with the usage of each UTC day attributed to the plan in force on that day" or "the month-end plan's code". Neither sentence is precise enough to build from, and the refinements are choices, not mechanics. They are listed in `DECISIONS-FOR-PAUL.md` under 16 as 16a to 16d and this design chooses none of them; it builds the two proposals as bodies of one shape (the lines, a way to the per-code weights, the last paid span's code), and the glue holds what another answer would need (the spans, the month) without pretending the shape already expresses 16b's alternatives or a 16c hold: |
| - **16a, the change day.** A UTC day is the finest grain the frozen billing statement provides (the collection underneath has finer readings; the statement does not). Proposed approximation: a day's usage follows the plan in force at the day's start (00:00 UTC). It can attribute nearly a whole day to the earlier plan. |
| - **16b, how a meter's unchanged amount is divided between codes.** Proposed: in proportion to the usage attributed to each code (gross usage, after the eligibility rule below). The alternatives differ: by each span's own excess over its own share of the allowance (a span whose usage stayed under its allowance gets nothing), or by consuming the pooled allowance chronologically and attributing only the excess after it. With span allowances of 100 and 100 and usage of 50 and 250 the pooled overage is 100: gross weighting gives the first span a sixth of it, the other two give it nothing. Pricing spans independently would charge 150, not 100, and is not an option here: amounts do not change. |
| @@ -28,7 +28,7 @@ What the design asserts without a decision: pre-tax amounts (each resource axis' |
| It builds the mechanics every answer needs, keeps the hold exactly where it is until the answer, and keeps every calculation that has one tax code frozen byte for byte as today (its digest, request hashes and operation names unchanged). |
| -1. **A line carries its own tax code.** `plans.OverageLine` gains `TaxCode string \`json:"tax_code,omitempty"\``; its existing fields keep their untagged names (`Meter`, `AmountCents`), so no old byte moves. Two shapes: a **one-code calculation** has `tax_code` set and no line carrying one (today's shape, every existing record, every month whose counted spans map to one code, including Personal to Community changes, quantity changes and partial coverage); a **two-code calculation** has `tax_code` empty and every line carrying a code. `wellFormed` gains the tax shape beside the money: exactly one of the two; a code of the form `txcd_` plus at least one character wherever one appears; a non-waived calculation with no lines refused; and an explicit waived branch, no lines, carrying the one-code shape when the spans map to one code and the two-code shape (empty `tax_code`, no lines) otherwise. Two distinct codes among the emitted lines are not required: a two-code history can have chargeable usage under one code only, and the month-end answer puts one code on every line. Version 1 calculations stay version 1 and are read as today. The entry guard in `billingInvoiceRun` keeps its account and month checks and `wellFormed`, and drops only the separate top-level `txcd_` predicate, which the shape rule now covers; `lineParams` takes the line's code or, for the one-code shape, the calculation's, and refuses a line with a code at both levels or at neither. |
| +1. **A line carries its own tax code.** `plans.OverageLine` gains `TaxCode string \`json:"tax_code,omitempty"\``; its existing fields keep their untagged names (`Meter`, `AmountCents`), so no old byte moves. Two shapes: a **one-code calculation** has `tax_code` set and no line carrying one (today's shape, every existing record, every month whose counted spans map to one code, including Personal to Community changes, quantity changes and partial coverage); a **two-code calculation** has `tax_code` empty and every line carrying a code. `taxShapeOK`, asked beside `wellFormed` at the entry guard and in `lineParams`, is the tax shape (the money shape stays `wellFormed`'s): exactly one of the two; a code of the form `txcd_` plus at least one character wherever one appears; a non-waived calculation with no lines refused; and an explicit waived branch, no lines, carrying the one-code shape when the spans map to one code and the two-code shape (empty `tax_code`, no lines) otherwise. Two distinct codes among the emitted lines are not required: a two-code history can have chargeable usage under one code only, and the month-end answer puts one code on every line. Version 1 calculations stay version 1 and are read as today. The entry guard in `billingInvoiceRun` keeps its account and month checks and `wellFormed`, and drops only the separate top-level `txcd_` predicate, which the shape rule now covers; `lineParams` takes the line's code or, for the one-code shape, the calculation's, and refuses a line with a code at both levels or at neither. |
| 2. **A line has a stable identity, checked on every acceptance path.** The invoice engine journals each provider operation under a kind (`line-<meter>`), replays by request hash (a hash of the `InvoiceItemParams` JSON), checks the provider's answer to a POST against the frozen line, recovers a lost answer by matching the provider's lines on `metadata[meter]` and `metadata[calculation_digest]`, and validates the draft by the same kinds and keys. One-code calculations keep all of that unchanged: the new `InvoiceItemParams` field for the line's metadata tax code is `omitempty`, so an old request's JSON, and its hash, are byte-identical. A two-code line's kind is `line-<meter>-<tax code>`; its request carries `tax_code` and `metadata[tax_code]`, both from the frozen line; the POST answer is accepted only if the item's `metadata[tax_code]` is the frozen line's; recovery matches meter, digest and tax code; the draft validation expects the operation of each line's own kind and requires each listed line's `metadata[tax_code]` to be the frozen line's, equal-amount lines included. The old kinds and the new cannot collide (a tax code is never empty); a calculation is frozen before any operation and its shape decides every kind, and an old worker refuses the new shape, so no path acknowledges an old-kind and a new-kind operation for the same axis of one calculation. |
| @@ -38,12 +38,12 @@ It builds the mechanics every answer needs, keeps the hold exactly where it is u |
| 5. **A meter's amount is apportioned, never re-priced.** The four axes' minor-unit amounts and the subtotal are computed exactly as today (`overageLines`: floor, floor test, invoice-wide largest-remainder hand-out between axes). Then, under 16b's gross-usage proposal, each axis's amount is divided among that axis's codes: weights are the per-code base units; `share_c = floor(amount * weight_c / sum_weights)` in minor units, with the minor units still owed to reach the axis's amount handed out one at a time to the codes with the largest remainder, ties broken by the code string's byte order; a code's share of zero is omitted; an axis with a positive amount and a zero weight sum holds (`billing_invoice_tax_code_mismatch`: the usage that produced the charge is not attributed). Lines are ordered by axis order then code byte order. The invoice-wide rounding is never rerun over the expanded lines. Under 16c's last-span answer, every line carries one code and nothing is divided. Where order matters (the days, the codes of an axis, the lines) it is fixed by date, byte order and axis order; the maps that are ranged without sorting feed sums whose result does not depend on order, and two runs over the same inputs produce identical bytes (tested). |
| -6. **The hold stays until the decision.** `freeze` keeps returning `errBillingTaxCodeMixed` for a two-code month, through `taxCodesForLines`. Behind it, uncalled, is everything activation needs: `twoCodeCalculation`, the glue that takes a month priced as today, asks the allowance for its spans again, attributes the eligible days (only if the body divides by them) and hands the lines to the chosen body (`bodySplitByGrossUsage`, `bodyLastSpansCode`), returning the two-code shape with its spans kept; and the review for a charge with no attributed usage (`errBillingTaxApportion` is `billing_invoice_tax_code_mismatch`). Activation is still a reviewed change of its own: the switch in `freeze` from the error to the glue with the decided body, its tests through `freeze` (among them one that drives `freeze` to `errBillingTaxApportion` and sees the `billing_invoice_tax_code_mismatch` review, which no test can reach today), and the roll rule; it is not one line, and nothing here pretends it is. |
| +6. **The hold stays until the decision.** `freeze` keeps returning `errBillingTaxCodeMixed` for a two-code month, through `taxCodesForLines`. Behind it, uncalled, is everything activation needs: `twoCodeCalculation`, the glue that takes a month priced as today, asks the allowance for its spans again, attributes the eligible days (only if the body divides by them) and hands the lines to the chosen body (`bodySplitByGrossUsage`, `bodyLastSpansCode`), returning the two-code shape with its spans kept; and the review for a charge with no attributed usage (`errBillingTaxApportion` is `billing_invoice_tax_code_mismatch`). Activation is still a reviewed change of its own: the switch in `freeze` from the error to the glue with the decided body, its tests through `freeze` (the attribution that produces `errBillingTaxApportion` reached from a real statement and history; the review it files is already tested through the preparer seam), and the roll rule; it is not one line, and nothing here pretends it is. |
| ## Compatibility and the roll rule |
| - **Roll order is not free.** An older invoice worker that reads a nonterminal two-code calculation (terminal records are skipped before the guard) passes today's money-only `wellFormed`, fails the top-level code check, and files `billing_invoice_record_mismatch` for the month: a durable, account-wide hold that then blocks the new worker too (`checkBillingHoldTx`), and that it can file again after an operator resolves it while it still runs. It does not invoice and does not rewrite the calculation, but it is not harmless. So: every console that can run the invoice pass is on a build with this change, and stopped if older, before `taxCodesForLines` stops returning the error; no older build is rolled back to afterwards; and the generic rollback note in `ENABLEMENT.md` ("an older binary runs against the migrated schema") does not apply past activation. Those sentences are a required edit to `ENABLEMENT.md` in the plan's last task, beside the retirement protocol's roll rule, before activation; until activation no two-code calculation exists, so the change itself rolls in any order. |
| -- A finalize that was journaled and then lost is not replayed or renewed over a draft whose lines would not pass today: the resumed step reads the invoice and, while it is a draft, validates its lines (the metadata code among them) first. |
| +- A finalize that was journaled and then lost is not replayed or renewed over a draft whose lines would not pass today: the resumed step reads the invoice once and, while it is a draft, validates that reading's lines (the metadata code among them) first; a finalize landing between reads cannot look like one made outside. This applies to every month from the merge, one-code months included: a one-code draft that changed under a journaled finalize now holds (`billing_invoice_lines_mismatch`) instead of being finalized. `ENABLEMENT.md` says so. |
| - A calculation frozen before this change is read exactly as before. The barrier trigger on `billing_months` reads `$.v` and is untouched; the version of a price-book calculation stays 2, a legacy one stays 1. |
| - Months already held: 16d, with its conditions (no other open review, the backoff, the twelve-month lookback). Nothing resolves a review but the operator door, and this design adds no automatic resolution and no handling for a held month outside the lookback. |
| - `DECISIONS-FOR-PAUL.md` carries 16a to 16d (launch branch `27419dcf`); the design's own text calls each a proposal and nothing in it reads as decided. |
| @@ -53,9 +53,9 @@ It builds the mechanics every answer needs, keeps the hold exactly where it is u |
| - Byte and digest fixtures, pinned as literal bytes recorded before the change (not regenerated by it): a legacy version 1 month; a price-book version 2 month with one plan; a one-code month with two plans (Personal to Community); a quantity change; a partly covered month; a waived month; a fully covered one-code month with non-zero usage and no `Days` (priced today, priced after). Each byte-identical, with its request hashes and operation kinds. |
| - A two-code month still holds, with the same review, and nothing is frozen; its review resolved by the operator door and the month priced on the next pass once a body is enabled in the test. |
| - The attribution: a change at 00:00 UTC; a continuous change at 15:00 (the day eligible, attributed under 16a to the earlier plan); a lapse at 15:00 (the day excluded as today); an expiry mid-month; a change exactly at the month's first instant; a fully covered month (nil `Covered`) attributed over the whole month; repeated spans with one code aggregated; the no-paid-plan month end; both transfer inputs under different codes; malformed `Days` (a date outside the month, a negative value, sums disagreeing) holding; `Days` absent with zero usage waived; absent with non-zero usage on a fully covered two-code month holding under the split answer and priced under the month-end one; absent on a partly covered two-code month holding under both. |
| -- The apportionment: each axis's shares sum to its amount and the subtotal is unchanged; a code with all of an axis's usage gives one line; equal remainders broken by byte order; a tiny share rounding to zero omitted; a zero weight sum with a positive amount holding; overflow refused; shuffled map insertion giving identical bytes; a fractional boundary in one transfer meter (N + 0.75 and 0.75 bytes-equivalent under two codes) with the exact expected shares, which conservation alone would not catch. |
| -- The invoice lifecycle for a two-code month, behind a test switch: two lines for one axis with equal and with unequal amounts; for each split line an in-window replay and an expired-window recovery, with a paginated draft listing; recovery by the three metadata keys; the draft validated against both kinds; the POST answer and the draft listing each refused when `metadata[tax_code]` is missing or wrong on one line (the fake provider stores and returns the tax code and the metadata, which today it does not); the transmitted `tax_code` and `metadata[tax_code]` asserted to be the frozen line's; a one-code month's journal, request bytes and hashes unchanged; a mixed history that is waived, and one that emits one code only, through both reader generations. |
| -- The older reader, with the real old control flow (the old `wellFormed` and entry guard kept in a test file): a two-code record makes it file `billing_invoice_record_mismatch`, write nothing to the provider, leave the calculation bytes unchanged, and block a subsequent new-reader run until the review is resolved; and with a second, unrelated review open on the account, the resolution of the tax-code review alone does not free the month. |
| +- The apportionment: each axis's shares sum to its amount and the subtotal is unchanged; a code with all of an axis's usage gives one line; equal remainders broken by byte order; a tiny share rounding to zero omitted; a zero weight sum with a positive amount holding; overflow refused; shuffled map insertion giving identical bytes; a fractional boundary in one transfer meter (two fractions of a byte under one code, a whole gigabyte under the other) asserted on the weights (floor each meter, then add), which conservation alone would not catch. |
| +- The invoice lifecycle for a two-code month, through a record frozen by hand: two lines for one axis with equal and with unequal amounts; for each split line an in-window replay (one more arrival under the same key) and an expired-window recovery (no arrival), with a paginated draft listing; recovery by the three metadata keys; the draft validated against both kinds; the POST answer and the draft listing each refused when `metadata[tax_code]` is missing or wrong on one line (the fake provider stores and returns the tax code and the metadata, which today it does not); the transmitted `tax_code` and `metadata[tax_code]` asserted to be the frozen line's; a one-code month's journal, request bytes and hashes unchanged; a mixed history that is waived, and one that emits one code only, through both reader generations. |
| +- The older reader, as a harness copying the entry of `29ea6676` (the terminal skip, the decode, the account, month, money and top-level code checks, the review filed by the run's own method; `wellFormed` is the shared function, unchanged since): a two-code record makes it file `billing_invoice_record_mismatch`, write nothing to the provider, leave the calculation bytes unchanged, and block a subsequent new-reader run until the review is resolved; and with a second, unrelated review open on the account, the resolution of the tax-code review alone does not free the month. |
| - `wellFormed` and `lineParams`: a code at both levels; at neither; a two-code calculation with one line lacking a code; a malformed code; a non-waived calculation with no lines. |
| - Mutants in a tree copy once the assertions above exist, each with its killing test named. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-per-line-tax-codes-review2.md b/docs/superpowers/specs/CODEX-FINDINGS-per-line-tax-codes-review2.md| new file mode 100644 |
| index 000000000..1e9154a14 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-per-line-tax-codes-review2.md |
| @@ -0,0 +1,106 @@ |
| +The design dispositions hold in the production code, with remaining qualifications about test coverage and rollout documentation. I found no P1 or P2 defect at `8715e4cd`. Two P3 verification fixes remain. |
| + |
| +This was source-only review: nothing modified, no tests or network commands run. References below are at `8715e4cd` unless another commit is named. Test counterfactuals are assessed by inspection. |
| + |
| +**Design dispositions, finding by finding** |
| + |
| +| Design finding | Assessment | |
| +|---|---| |
| +| Round 1 P1: identity keyed only by meter | Holds. Kinds, requests, POST checks, recovery and draft validation distinguish codes (`internal/console/billinginvoice.go:990`, `:1036`, `:1048`, `:1138`). | |
| +| Round 1 P1: held months cannot resume automatically | Holds. Nothing automatically resolves reviews. Resolution, other account reviews, backoff and lookback remain separate conditions. | |
| +| Round 1 P2: older workers create durable account-wide reviews | Holds. The older entry predicate rejects nonterminal per-line records. The new harness files through `r.review` (`billingtaxlifecycle_test.go:248`). Task 5’s operational documentation remains due at merge. | |
| +| Round 1 P2: eligibility before attribution | Holds. Whole-day eligibility precedes span selection (`billingtaxsplit.go:87`); full coverage becomes a whole-month interval (`:51`). The new glue composes these helpers. | |
| +| Round 1 P2: attribution choices are policy | Holds. Midnight attribution, gross weighting and last-paid-span attribution remain dormant proposals. | |
| +| Round 1 P2: precise apportionment | Holds. Chronological accumulation, separate meter floors, checked transfer addition, largest remainders, byte-order ties and omission of zero shares are implemented. | |
| +| Round 1 P3: zero usage without `Days` | Holds wherever daily data is required. Full one-code months retain their existing exemption. | |
| +| Round 2 P2: universal daily requirements would regress one-code months | Holds. The refactor preserves existing pricing behavior; the full-month, nonzero-usage, no-`Days` fixture covers it. | |
| +| Round 2 P2: POST and draft identity checks | Holds in code. Missing and wrong codes are tested. The recovery-specific assertion needs strengthening; finding 2 below. | |
| +| Round 2 P2: ambiguous transfer arithmetic | Holds. Each meter is floored before transfer integers are added (`billingtaxsplit.go:126`). The fractional test checks weights, not resulting shares. | |
| +| Round 2 P3: decision and rollout references | Decision refinements exist, and the substantive corrections are present at launch commit `02e3538a`. The rollout edit is explicitly deferred to Task 5. | |
| +| Round 2 P3: unconditional “next pass” | Corrected in the design and decision 16d at `02e3538a`: other reviews, backoff and twelve-month lookback remain conditions. | |
| +| Round 3 P2: month-end exemption versus partial-month pricing | Holds. Partial-month pricing requires usable days before either body runs; the month-end body adds no daily-data requirement (`billingtaxsplit_test.go:289`). | |
| +| Round 3 qualifications: waiver and terminal records | Hold. Waivers require no lines; terminal records are skipped before decoding and tax-shape checks. | |
| + |
| +**First code review, finding by finding** |
| + |
| +| Finding | Closed? | Would the relevant test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1: missing activation glue and review mapping** | Implementation closed; verification partly open. `twoCodeCalculation` exists, remains uncalled, and the error maps to a review. | Glue assertions detect missing spans, incorrect attribution and eager daily-data requirements. Removing the review mapping still passes the current suite’s new tests: M27 remains uncovered. See finding 1. | |
| +| **Opus P3-1: hand-built fixture requests** | Closed. The second table uses engine functions and literal expected hashes. | These compatibility tests should pass against the previous production behavior; this was a test-only deficiency. They now detect drift in the engine functions that the original table bypassed. | |
| +| **Opus P3-2: predicate standing in for an older reader** | Substantively closed. The harness includes terminal skip, decoding, account/month checks, money validation and the old top-level predicate, then calls `r.review`. | The new tests should pass with the previously correct production behavior. Removing rejection from the harness fails the expected-review assertions. `wellFormed` remains shared, but its implementation is unchanged from the base. | |
| +| **Opus P3-3: waived shape** | Closed as an implementation issue; intentional compatibility differences remain. | Removing the no-lines waiver requirement fails the new shape-table case at `billingtaxshape_test.go:29`. Engine tests alone would not isolate it because `stepFrozen` also rejects waived records with lines. | |
| +| **Opus P3-4: stale decision text** | Substantive corrections verified at `02e3538a:implementation/carloku-paid-launch/DECISIONS-FOR-PAUL.md:54`. | No behavioral test applies. Minor stale wording remains, listed below. | |
| +| **Opus P3-5: mutant evidence and negative amount** | Negative-amount coverage closed. Reported mutant reruns cannot be independently verified from this commit. | Removing the negative-amount check fails the case at `billingtaxsplit_test.go:150`. M3/M15’s rewritten mutations and execution results are absent from the reviewed tree. | |
| +| **Opus P3-6: lookup ordering before mixed-code detection** | Correctly dispositioned as unreachable under production configuration, not changed. | No regression test establishes otherwise. `validBillingTaxCodes` requires all three paid plans (`billingservice.go:136`); pricing rejects unknown plans earlier. | |
| +| **Astra 1: resumed finalization bypass** | Closed. | **No.** Without `stepFinalizing`’s validation at `billinginvoice.go:1214`, the four cases at `billingtaxlifecycle_test.go:175` finalize the corrupted draft and fail. | |
| +| **Astra 2: wrong compatibility fixtures** | Closed for the requested frozen shape and engine hashes. | The new tests pass against the previous correct production behavior, intentionally. They now exercise `lineParams`, `lineKind`, `draftParams` and `requestHash`. | |
| +| **Astra 3: lifecycle and reader coverage** | Mostly closed. Wrong-code cases, first-line expired recovery, the older-reader harness, waivers and one-emitted-code records are present. Two narrow gaps remain in finding 2. | The additional coverage largely passes against the first-review production code. Wrong POST and draft equality assertions detect their respective regressions; the wrong-code recovery case does **not** independently establish recovery rejection. | |
| +| **Astra 4: invalid mutants reported as kills** | Reported corrected, but execution evidence is unavailable in `8715e4cd`. | Cannot assess the actual M3/M15 runs without their mutation definitions and logs. Compilation failure would still not count as an assertion kill. | |
| + |
| +**Remaining findings, most severe first** |
| + |
| +1. **P3 — The apportionment error-to-review mapping is testable now, but remains untested.** |
| + |
| + **Location:** `internal/console/billinginvoice.go:319`; claimed limitation at `docs/superpowers/specs/2026-10-03-per-line-tax-codes-design.md:41` and `DISPOSITIONS-per-line-tax-codes.md:55`. |
| + |
| + **Concrete scenario:** Delete the `errBillingTaxApportion` mapping. The current new tests remain green because none returns that error through `invoiceBillingMonth`. Once activation reaches this error, the engine returns it without filing the promised durable review. |
| + |
| + The production path is dormant, but testing the mapping does not require activation or decision 16. The existing `billingInvoiceState.prepare` seam (`billinginvoice.go:85`, `:124`) can return the error; `freeze` propagates it at `:459`. |
| + |
| + **Smallest acceptable fix:** Add a fixture test whose preparer returns `errBillingTaxApportion`. Assert `errBillingInvoiceHeld`, the exact `billing_invoice_tax_code_mismatch:<month>` review, no frozen record and no provider mutation. Correct the claim that no test can reach the mapping today. Keep the real attribution-to-review integration test as an activation obligation. |
| + |
| +2. **P3 — The wrong-code recovery test can pass without recovery rejecting the wrong code; first-line in-window replay is still missing.** |
| + |
| + **Location:** `internal/console/billingtaxlifecycle_test.go:143`, `:157`; replay cases at `:104`. |
| + |
| + **Concrete scenario:** Remove the recovery tax-code predicate at `billinginvoice.go:1048`. In the wrong-code recovery subtest, only the lost first item exists. Recovery accepts that item despite its wrong metadata, creates the second item, and later draft validation rejects the listing. The test still passes because it only requires an error, an unfinished month and no finalize/payment. |
| + |
| + Separately, the added first-line loss case runs only after 25 hours. The promised in-window replay of **each** split line remains incomplete. |
| + |
| + **Smallest acceptable fix:** For wrong-code recovery, assert the exact unrecovered review, that the first operation remains unacknowledged, and that no second item is created. Add the first-line loss at two hours, asserting replay arrival count, the unchanged key and one provider mutation. |
| + |
| +**Checked claims A–H** |
| + |
| +- **A — Byte identity.** I found no valid production one-code input whose calculation JSON, digest, request JSON/hash or operation kind changes. Existing field names and order remain; the three new fields use `omitempty`; `priceBillingCalculation` clears spans at `billingcalculation.go:174`; metadata is omitted from one-code provider forms; JSON map encoding remains deterministic. The daily-validation extraction and scale helper preserve the previous arithmetic. |
| + |
| + The original table contains seven pricing outputs. The second contains six one-code frozen-shape fixtures, correctly excluding `legacy_two_codes`. Their expectations are literals; history shows the original table introduced in `5429b690` and the second in `8715e4cd`. The second table pins engine hashes, including the legacy currency adapter. It reconstructs freeze’s code-setting step rather than running evidence preparation and database freeze; its `StatementObject` is empty. That is useful compatibility coverage, but not a complete production-freeze fixture. |
| + |
| +- **B — Shape and guard differences.** The new guard intentionally accepts per-line shapes and empty-code, empty-line waivers that the old top-level predicate rejected. It rejects nonwaived empty-line records, waived records containing lines, and top-level-code records also carrying line codes. The latter fields would have been ignored by an older decoder. These are not identical acceptance sets. |
| + |
| + `validTaxCode` preserves the old syntactic rule: `txcd_` plus at least one byte. It does not certify that a provider code exists. `lineParams` independently refuses both levels, neither level and malformed codes; `lineKind` preserves old names for untagged lines. |
| + |
| +- **C — Identity through the engine.** Pending operations compare request hashes before replay. Split POST checks, recovery matching and draft validation use the frozen code; equal amounts do not collapse identity. Draft validation binds listed items to acknowledged operation IDs as well as metadata. The resumed-finalize fix closes the previously identified draft bypass. |
| + |
| + A mismatching POST response with an object ID is deliberately **acknowledged as an existing object and then held** (`billinginvoice.go:834`); it is not accepted as permission to finalize. Recovery of an already-finalized outcome remains distinct from validating a still-draft invoice. |
| + |
| + I found no ordinary engine path producing both old-kind and new-kind operations for the same axis of one immutable calculation. Pagination follows `has_more`; the lifecycle fake exercises an abbreviated embedded listing followed by a complete `/lines` response, while actual endpoint pagination belongs to the client tests. |
| + |
| +- **D — Attribution.** Existing partial-month pricing still determines billed days and `CountedDays`. Attribution repeats that eligibility rule before assigning a day to the half-open span containing midnight. Continuous changes retain the day; lapses exclude partial days. Full coverage receives a whole-month interval despite nil `Covered`. |
| + |
| + Repeated codes aggregate; an eligible day with no containing span returns `errBillingDaysMismatch`; an unmapped plan returns `errBillingUnavailable`. Missing days with zero meters are usable; nonzero usage without days fails where attribution requires them. Malformed daily data retains the existing validation. None of these dormant allocation helpers replaces the month’s billed usage or prices. |
| + |
| +- **E — Arithmetic.** Each `(code, meter)` accumulates in date order and is floored once. Transfer integers are then added with an overflow check. `apportion` uses exact integer multiplication/division, rejects negative amounts/weights and overflowing weight totals, orders ties by code bytes, and omits zero shares. Positive amounts with no weight fail. Successful shares conserve each axis’s amount, hence the subtotal. No expanded-line invoice-wide rounding occurs. |
| + |
| + Unsorted map iteration does not affect successful output: chronological floating accumulation and final code ordering are fixed; remaining sums are checked nonnegative integers. |
| + |
| +- **F — Policy and hold.** Nothing activates 16 or 16a–16d. `twoCodeCalculation` and both bodies remain uncalled from production freeze. The last-span body implements one proposed answer to 16c; it does not establish that answer as policy. Under valid production configuration, `taxCodesForLines` preserves today’s one-code result and mixed-code hold, including mixed histories that would otherwise waive. |
| + |
| +- **G — Older reader and rollout.** The harness’s entry predicate matches the base behavior; money-only `wellFormed` is unchanged. Nonterminal per-line records fail the old top-level check and file `billing_invoice_record_mismatch` before provider work. Terminal records are skipped first. An older worker can reopen the review after resolution; unresolved reviews block authorizing steps account-wide. |
| + |
| + At merge, Task 5 must update `ENABLEMENT.md` to require upgrading or stopping every older invoice worker before activation, prohibit rollback to an older reader afterwards, and explicitly override the generic older-binary rollback sentence at `ENABLEMENT.md:206`. It should describe resolving accidental older-reader reviews and the separate conditions for releasing previously mixed-code months. `OUTSTANDING.md` and `READINESS.md` also need their planned updates. |
| + |
| +- **H — Tests and mutants.** M14’s stated equivalence is valid **under ordered spans and coverage produced together by `AllowanceForMonth`**: eligibility guarantees a containing span, and earlier spans have already ended. It is not equivalent for inconsistent helper inputs. |
| + |
| + M27 remains a surviving, testable omission. The negative-amount omission is now caught. Missing/wrong POST and draft metadata checks have meaningful assertions; the recovery-specific limitation is finding 2. The cited `logs/taxcodes-mutants-round2.txt` does not exist in `8715e4cd`, so “27 killed, two surviving” remains reported evidence rather than independently verified evidence in this review. |
| + |
| +**Claims still overstated or contradicted** |
| + |
| +- Design `:41` says no test can reach the apportionment review today. The existing prepare seam can. |
| +- Design `:57` promises in-window replay and expired recovery for each split line. The first line has only expired recovery coverage. |
| +- Design `:58` and plan `:36` describe a copied old `wellFormed`. The harness calls the shared current function, which is presently unchanged. |
| +- Design `:56` and plan `:31` promise exact fractional-transfer **shares**. The test asserts weights; dispositions `:51` accurately acknowledges this. |
| +- At launch commit `02e3538a`, decision context `:52` still says “Nothing is built for it,” while `:56` says the mechanics are built. Its revision reference also remains 4 although the reviewed design identifies itself as revision 5. |
| + |
| +**Before merge:** require the two small verification fixes, the corresponding claim corrections, Task 5’s launch records, and the stipulated reviews, exact-tip isolated gate and CI. Decision 16 is not required to merge these inactive mechanics. |
| + |
| +**Verdict: accept with fixes.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/DISPOSITIONS-per-line-tax-codes.md b/docs/superpowers/specs/DISPOSITIONS-per-line-tax-codes.md| index 406bfccb6..4ddc5d8d3 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-per-line-tax-codes.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-per-line-tax-codes.md |
| @@ -50,8 +50,23 @@ Reports: `OPUS-REVIEW-per-line-tax-codes-review1.md`, `CODEX-FINDINGS-per-line-t |
| Stated and not changed: the fractional-boundary test asserts the weights, not shares; the lifecycle fake's paged listing is an abbreviated embedded page then one complete listing, the client's pagination being tested on its own. |
| +## Code review two (`8715e4cd`; Opus: accept with fixes, five P3; Astra: accept with fixes, two P3) |
| + |
| +Reports: `OPUS-REVIEW-per-line-tax-codes-review2.md`, `CODEX-FINDINGS-per-line-tax-codes-review2.md`. No P1 or P2; every first-review finding closed. The gate on `8715e4cd` passed alone and CI passed. |
| + |
| +| Finding | Disposition | Where | Test | |
| +| --- | --- | --- | --- | |
| +| Opus P3-1: the resumed finalize read the invoice twice, so a finalize landing between the reads would file `billing_invoice_finalized_externally` for a correct one; and the tightening applies to every month, unstated. | `validateDraftLinesOf` takes the invoice already read; the design and `ENABLEMENT.md` (Task 5) state the tightening for every month. | `billinginvoice.go` | `TestAResumedFinalizeValidatesTheDraftFirst` | |
| +| Opus P3-2: the split body asked for weights with nothing to divide (a waived two-code month without `Days` would retry forever after activation). | No lines, no weights asked. | `billingtaxsplit.go` | `TestTheGluesGuards` (a house month without `Days` waived under the split) | |
| +| Opus P3-3: "any answer is one body of one function" overstated; `bodyLastSpansCode` quietly took 16c's first option. | Reworded in the design and the comments: the shape is the two proposals'; the glue holds the spans and the month for what another answer would need; a 16c hold is written when 16c is. | design, `billingtaxsplit.go` | | |
| +| Opus P3-4: the one-emitted-code test rewrote the record by SQL and left the digest stale. | The one-line record is frozen through `freezeBillingMonth`. | `billingtaxlifecycle_test.go` | `TestWaivedAndOneEmittedCodeRecordsThroughBothReaders` | |
| +| Opus P3-5: two guards untested (a one-code calculation handed to the glue; the transfer overflow). | Both tested. | | `TestTheGluesGuards` | |
| +| Astra 1: the apportionment review's mapping was testable through the preparer seam. | Tested there; the design no longer says no test can reach it; the attribution that produces the error through `freeze` remains the activation change's test. | | `TestAnUnattributedChargeHoldsTheMonthForAPerson`; mutant M27 now dies | |
| +| Astra 2: the wrong-code recovery test accepted any error; the first line had no in-window replay case. | The recovery case asserts the `billing_invoice_unrecovered` review alone, the first operation unacknowledged and no sibling created; the first line's loss inside the window asserts one more arrival under its key. | | `TestAWrongMetadataCodeHoldsTheMonth`, `TestATwoCodeLinesLostAnswerIsRecoveredByItsCode` | |
| +| Both: claims (`wellFormed` gains the shape; copied old `wellFormed`; exact shares; "the switch in `taxCodesForLines`"; decision 16's "nothing is built", revision 4). | Reworded in the design, this file, and decision 16 on the launch branch (`68b16c52`). | | | |
| + |
| ## The build |
| -Tasks 0 to 4 and the mutants of Task 6 are committed; Task 5 (the records on the launch branch) comes at the merge. `wellFormed` kept its money shape and the tax shape is `taxShapeOK`, asked beside it at the entry guard, as the second review's note on "`wellFormed` alone" asked; the design's "gains the tax shape" is met by the guard, not by one function. Mutants: twenty-three in the first batch and twenty-nine in the second, in git-less tree copies, each with its killing tests logged (`taxcodes-mutants-round{1,2}.txt`); all die but two: M14, equivalent under the production invariant (spans and coverage from one `AllowanceForMonth`; not for arbitrary inconsistent helper inputs), and M27, the review mapping of `errBillingTaxApportion`, which nothing reaches until decision 16 switches `freeze` to the glue; the test that kills it is an obligation of the activation change, named in the design. Every one-code month's bytes, digest, request hashes and operation kinds are the ones pinned in Task 0 from the unchanged code. |
| +Tasks 0 to 4 and the mutants of Task 6 are committed; Task 5 (the records on the launch branch) comes at the merge. `wellFormed` kept its money shape and the tax shape is `taxShapeOK`, asked beside it at the entry guard, as the second review's note on "`wellFormed` alone" asked; the design's "gains the tax shape" is met by the guard, not by one function. Mutants: twenty-three in the first batch and twenty-nine in the second, in git-less tree copies, each with its killing tests logged (`taxcodes-mutants-round{1,2}.txt`); all die but M14, equivalent under the production invariant (spans and coverage from one `AllowanceForMonth`; not for arbitrary inconsistent helper inputs); M27 survived the second batch and dies in the third (`taxcodes-mutants-round3.txt`) on the preparer-seam test. Every one-code month's bytes, digest, request hashes and operation kinds are the ones pinned in Task 0 from the unchanged code. |
| What a reader of the invoice should know: a two-code month does not exist until decision 16 is taken and `taxCodesForLines` is switched, after the roll rule; nothing on this branch changes a one-code month, an amount, the floor, or the price book. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-per-line-tax-codes-review2.md b/docs/superpowers/specs/OPUS-REVIEW-per-line-tax-codes-review2.md| new file mode 100644 |
| index 000000000..45ce3d233 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-per-line-tax-codes-review2.md |
| @@ -0,0 +1,119 @@ |
| +# Second code review: `design-per-line-tax-codes` at 8715e4cd (base 29ea6676) |
| + |
| +I found no P1 or P2. All eleven first-review findings are closed. Every code fix has a test that fails without it, except M27, which no test can reach until activation. The design's dispositions hold, with four wording exceptions listed under "Claims the code contradicts". By reading the code, nothing changes for a one-code month: its bytes, digest, request hashes, operation kinds and amounts are all as before. Nothing decides 16 or 16a to 16d, and the hold is today's. |
| + |
| +## First review: is each finding closed at 8715e4cd? |
| + |
| +| Finding | Closed? | Fails without its fix? | |
| +|---|---|---| |
| +| Opus P2-1: activation glue, and `errBillingTaxApportion` retried quietly | Yes. `twoCodeCalculation` and the two bodies exist and nothing calls them (billingtaxsplit.go:295–347). The error now files a review (billinginvoice.go:319). | The glue test fails without the glue (M25, M26, M19, M23 die). The review mapping (M27) survives, and only the activation change can test it. That is stated correctly. | |
| +| Opus P3-1 / Astra 2: Task 0 pinned the wrong things | Yes. `TestTodaysFrozenRecordsAndRequestsArePinned` pins the frozen bytes after the code is set. It goes through `lineParams`, `lineKind`, `draftParams` and `requestHash`, which strips the currency of a version 1 month. It asserts no metadata code. | Yes. M11, M12 and M13 die. If `lineParams` always set the metadata code, the test would now catch it. | |
| +| Opus P3-2 / Astra 3: the lifecycle tests were too narrow | Yes, all of it. The first line's loss is covered, and wrong codes on the POST answer, the recovery listing and the draft, with equal amounts. The older-reader harness copies 29ea6676:336 exactly, files through `r.review`, and skips terminal records. Waived and one-emitted-code records go through both readers. | Yes (M6–M10 die). | |
| +| Opus P3-3: the waived branch of the shape | Yes. A waived calculation must have no lines (billingcalculation.go:104). | Yes (M4, M29). | |
| +| Opus P3-4: stale sentences in decision 16 | Mostly. 02e3538a fixes the box-side claim, the 16d conditions and the revision number. One wording slip remains (claim 2 below). | n/a | |
| +| Opus P3-5: M3/M15 didn't compile; no negative amount | Yes. Both now compile, and a negative-amount case exists (M28). | Yes. | |
| +| Opus P3-6: an unconfigured plan is reported before the mix | Noted and unchanged. It is unreachable because the configuration requires all three paid plans. | n/a | |
| +| Astra 1: a lost finalize was replayed over a draft without validating it again | Yes. `stepFinalizing` validates a draft first (billinginvoice.go:1210–1218). | Yes (M24 dies in all four cases). | |
| +| Astra 4: two mutants did not compile | Yes. | n/a | |
| +| Astra's checked claims (iteration order, M14 label, one-line activation, 16d citation, partial month with no `Days`) | All corrected in design revision 5, the dispositions and 02e3538a. | n/a | |
| + |
| +## The three design rounds against the code |
| + |
| +Every disposition holds. |
| +- **Round 1:** the line identity is built; there is no automatic resolution; the roll rule is waiting for Task 5; eligibility comes before attribution; 16a–16d are stated as the owner's; the apportionment is exact; zero usage with no `Days` behaves as stated. |
| +- **Round 2:** one-code months need no new daily data; the POST answer and the draft both check the metadata code; transfer meters are floored first, then added (M16 dies); the references and 16d conditions are written. |
| +- **Round 3:** the month-end answer needs no `Days` on a whole month, and a partly covered month without `Days` still can't be priced (tested in the glue test). |
| + |
| +## Findings, most severe first |
| + |
| +**P3-1. The resumed finalize can file a false review (billinginvoice.go:1210–1218).** `stepFinalizing` reads the invoice once, sees `draft`, then `validateDraftLines` reads it again. |
| +- **Scenario:** a finalize from the dead process lands between the two reads. The second read sees `open`, and `validateDraftLines` files `billing_invoice_finalized_externally` for a correctly finalized invoice. |
| +- **Scope:** this applies to every month, one-code included. The window is tiny. |
| +- **Related, also every month:** a one-code draft that changed under a journaled finalize now gets a lines-mismatch review instead of being finalized. That is a deliberate tightening. The design doesn't say it applies to every month. |
| +- **Smallest fix:** pass the invoice already read into the validation, or let an open/paid invoice on that path fall through to `perform`. State the tightening in the design and in ENABLEMENT. |
| + |
| +**P3-2. The split body asks for weights when it has nothing to divide (billingtaxsplit.go:337–343).** `bodySplitByGrossUsage` calls `weights()` even when there are no lines. |
| +- **Scenario (only after activation):** a waived two-code whole month (house account, or below the floor) whose statement has usage but no `Days`. It gets the quiet `errBillingDays` and retries forever instead of being waived. |
| +- **Contradicts:** the design's "attributes the eligible days (only if the body divides by them)". |
| +- **Smallest fix:** if there are no lines, return them before calling `weights()`; add a test case. Alternatively, name it as an activation obligation. |
| + |
| +**P3-3. The body's signature can only express the proposals (billingtaxsplit.go:282, 313, 345).** `twoCodeBody` receives per-code usage and the last span's code only. |
| +- It cannot express 16b's alternatives (span excess, chronological consumption), which need per-span allowances. |
| +- It cannot express 16c's "hold", which needs to know whether the last span reaches the month's end. |
| +- `bodyLastSpansCode` quietly applies 16c's first option to a cancelled month. |
| +- **Nothing is decided** (none of it is called), but the design's "any answer is one body of one function" is not true. |
| +- **Smallest fix:** reword the design and the comments (e.g. "as 16c would settle it" becomes "if 16c takes the last paid span's code"), or pass the spans and the month's end to the body. |
| + |
| +**P3-4. The one-emitted-code test stores an inconsistent record (billingtaxlifecycle_test.go:371–379).** It rewrites `billing_months.calculation` by SQL and leaves the digest of the two-line record. |
| +- **Scenario:** the record's digest isn't the hash of its bytes. The test passes only because nothing re-hashes the stored record. |
| +- **Smallest fix:** build the one-line calculation first and freeze it through `freezeBillingMonth`. |
| + |
| +**P3-5. Two guards no test kills.** |
| +- `twoCodeCalculation`'s `calc.TaxCode != ""` check (:296): the "split twice" case passes a calculation whose own code is empty, so the line check catches it first. A one-code calculation fed to the glue would be silently converted. |
| +- `usageByCode`'s transfer overflow check (:131). |
| +- **Smallest fix:** a case each: a frozen one-code calculation passed to the glue, and two transfer meters near the int64 maximum. |
| + |
| +## A–I |
| + |
| +**A. Byte identity.** I found no input that changes a one-code month. |
| +- `OverageLine.TaxCode`, `MonthAllowance.Spans` and `InvoiceItemParams.TaxCodeMetadata` are all `omitempty`. Field order and names are untouched. |
| +- `priceBillingCalculation` drops the spans before every return (:174). `AllowanceForMonth`'s only serialized caller is that function. |
| +- JSON sorts map keys. `metadata[tax_code]` is sent only when the field is set. |
| +- Extracting `checkedStatementDays` and `billingMeterScale` changes no check, order or error. |
| +- Both fixture tables are literal and are the right ones: legacy, one plan, two plans with one code, a quantity change, partial, waived, and no `Days`. A one-code partial month with two spans would be a useful extra, but M13 covers the risk. |
| + |
| +**B. Shape and entry guard.** |
| +- Records the new guard refuses and the old accepted: a non-waived record with no lines, and a waived record with lines. `stepFrozen` already refused both with the same review. |
| +- Records the new guard accepts and the old refused: the two-code shapes, including waived with no code. This is intended, and the older reader refuses all of them. |
| +- A code at both levels, at neither, or malformed is refused by both `taxShapeOK` and `lineParams`. |
| +- `lineKind` can't collide with an old kind, because a code is never empty. |
| + |
| +**C. Line identity.** The calculation is fixed once frozen (`reload` checks the digest), and its shape is all one or all the other. Every kind therefore comes from one shape, so an old-kind and a new-kind operation for one axis can't both be acknowledged. |
| +- The POST answer, recovery (meter, digest, code), paged listing and draft validation all require the code. |
| +- Equal amounts are told apart by the code. |
| +- A line without metadata is caught at the POST answer, the draft and, now, the resumed finalize. |
| +- For untagged lines the new clauses are equivalent to before. |
| + |
| +**D. Attribution.** |
| +- Eligibility uses the same predicate as `billingUsageOnPaidDays`. A fully covered month (`Covered` nil) takes the whole month. |
| +- Spans are half-open. The first-instant rule is labelled as 16a's proposal. |
| +- Two spans of one code aggregate. |
| +- An eligible day no span holds can't happen with spans from one `AllowanceForMonth` (they are ordered, disjoint, and their union is the coverage); the code returns an error anyway. A plan with no code returns `errBillingUnavailable`. |
| +- No `Days` with zero usage gives no weights; no `Days` with usage gives `errBillingDays`. Malformed `Days` fail exactly as today. |
| +- Billed days, usage and amounts are untouched. |
| + |
| +**E. Arithmetic.** |
| +- Per (code, meter), days are summed in date order and floored once; transfer is added per code with the overflow check. |
| +- `apportion` multiplies in `big.Int`, floors, hands out the largest remainders with a stable sort over byte-sorted codes, and omits zero shares. |
| +- Zero weight with a positive amount is an error. A wrapped sum is caught on its first overflow. |
| +- The result doesn't depend on map order. Shares sum to the axis amount, so the subtotal can't change. Nothing rounds across axes. |
| + |
| +**F. Policy.** `taxCodesForLines` is today's loop, and every two-code month still holds with `billing_invoice_tax_code_mixed`. The only difference is P3-6 of review 1 (unreachable). Wording is the only issue (P3-3). |
| + |
| +**G. Older reader and roll rule.** |
| +- An older binary files `billing_invoice_record_mismatch` for any nonterminal two-code record, waived included, before touching the provider. The harness's predicate is 29ea6676:336 verbatim. A terminal record is skipped. |
| +- That review holds the whole account, and the older binary can file it again after it is resolved, for as long as it runs. The test proves this. |
| +- Until activation no two-code record can exist, so this merge rolls in any order. |
| + |
| +ENABLEMENT.md (Task 5) must say: |
| +1. Before any build whose `freeze` can write a two-code calculation (the switch is in `freeze`, not in `taxCodesForLines`), every console that can run the invoice pass is on a build containing this merge, and older ones are stopped. |
| +2. No rollback below it after that. |
| +3. "An older binary runs against the migrated schema" does not apply past activation. |
| +4. The resumed finalize now validates the draft for every month. |
| + |
| +**H. Tests and mutants.** Each code change has a killing test (M1–M29), except: |
| +- **M14:** truly equivalent. Spans are ordered and disjoint, eligibility is decided first, and the span holding the day's first instant is always the first span that overlaps the day. |
| +- **M27:** reachable only after activation; correctly named as that change's obligation. |
| + |
| +Untested and worth having are P3-5's two guards. Cosmetic: the mutant log has a Python traceback from a stale M4 pattern after M3, then the real M4. |
| + |
| +**I. Before the merge.** Nothing is required beyond the gate alone on the exact tip, CI, and Task 5's records worded as in G (keyed to `freeze`, not `taxCodesForLines`). P3-1 to P3-5 are cheap and I'd take them now. If they wait, P3-2 and P3-3 must be listed as obligations of the activation change. |
| + |
| +## Claims the code contradicts |
| + |
| +1. Design, "What is the owner's": "it builds so that any answer is one body of one function". The body's signature can't express 16b's alternatives or 16c's hold (P3-3). |
| +2. Decision 16 (02e3538a) and plan Task 5: "the switch in `taxCodesForLines`". It only returns one string; activation switches `freeze` to `twoCodeCalculation`, which is what the design says. |
| +3. Design §1, revision 5: "`wellFormed` gains the tax shape". The shape is a separate `taxShapeOK`, checked beside `wellFormed` at the guard. The dispositions say this; the design body doesn't. |
| +4. Design §6 and the glue's comment: weights are asked for "only if the body divides by them". The split body asks even with zero lines (P3-2). |
| + |
| +**Verdict: accept with fixes.** There is no P1 or P2. The P3s are wording and test fixes that can land with Task 5, or else be named as obligations of the activation change. |
diff --git a/internal/console/billinginvoice.go b/internal/console/billinginvoice.go| index 2d5743533..4ef5db72e 100644 |
| --- a/internal/console/billinginvoice.go |
| +++ b/internal/console/billinginvoice.go |
| @@ -1120,7 +1120,13 @@ func (r *billingInvoiceRun) validateDraftLines(ctx context.Context) error { |
| if err != nil { |
| return err |
| } |
| - if err := r.verifyInvoice(inv, "draft"); err != nil { |
| + return r.validateDraftLinesOf(ctx, inv) |
| +} |
| + |
| +// validateDraftLinesOf is validateDraftLines over an invoice already read. |
| +func (r *billingInvoiceRun) validateDraftLinesOf(ctx context.Context, inv *billingstripe.Invoice) error { |
| + var err error |
| + if err = r.verifyInvoice(inv, "draft"); err != nil { |
| if inv != nil && (inv.Status == "open" || inv.Status == "paid") { |
| // Finalized outside this engine: record the fact and hold. |
| return r.review("billing_invoice_finalized_externally") |
| @@ -1212,7 +1218,9 @@ func (r *billingInvoiceRun) stepFinalizing(ctx context.Context) error { |
| return err |
| } |
| if inv.Status == "draft" { |
| - if err := r.validateDraftLines(ctx); err != nil { |
| + // The invoice as just read, not read again: a finalize landing |
| + // between two reads must not look like one made outside. |
| + if err := r.validateDraftLinesOf(ctx, inv); err != nil { |
| return err |
| } |
| } |
diff --git a/internal/console/billingtaxlifecycle_test.go b/internal/console/billingtaxlifecycle_test.go| index 642344a95..23476406e 100644 |
| --- a/internal/console/billingtaxlifecycle_test.go |
| +++ b/internal/console/billingtaxlifecycle_test.go |
| @@ -1,6 +1,8 @@ |
| package console |
| import ( |
| + "context" |
| + "database/sql" |
| "encoding/json" |
| "errors" |
| "strings" |
| @@ -8,6 +10,8 @@ import ( |
| "time" |
| "github.com/carlosframework/platform/internal/plans" |
| + "github.com/carlosframework/platform/internal/release" |
| + "github.com/carlosframework/platform/internal/usage" |
| ) |
| // storeTwoCodeMonth freezes, by hand, the calculation a two-code month |
| @@ -101,10 +105,11 @@ func TestATwoCodeLinesLostAnswerIsRecoveredByItsCode(t *testing.T) { |
| later time.Duration |
| lost string |
| }{ |
| - "inside the window, one page": {false, 2 * time.Hour, "line-awake_seconds-txcd_personal"}, |
| - "past the window, one page": {false, 25 * time.Hour, "line-awake_seconds-txcd_personal"}, |
| - "past the window, paged listing": {true, 25 * time.Hour, "line-awake_seconds-txcd_personal"}, |
| - "past the window, the first line, no sibling yet": {false, 25 * time.Hour, "line-awake_seconds-txcd_business"}, |
| + "inside the window, one page": {false, 2 * time.Hour, "line-awake_seconds-txcd_personal"}, |
| + "past the window, one page": {false, 25 * time.Hour, "line-awake_seconds-txcd_personal"}, |
| + "past the window, paged listing": {true, 25 * time.Hour, "line-awake_seconds-txcd_personal"}, |
| + "past the window, the first line, no sibling yet": {false, 25 * time.Hour, "line-awake_seconds-txcd_business"}, |
| + "inside the window, the first line, no sibling yet": {false, 2 * time.Hour, "line-awake_seconds-txcd_business"}, |
| } { |
| t.Run(name, func(t *testing.T) { |
| ix := taxLifecycleFixture(t) |
| @@ -124,6 +129,9 @@ func TestATwoCodeLinesLostAnswerIsRecoveredByItsCode(t *testing.T) { |
| if tc.later > billingIdempotencyWindow && arrivalsAfter[lost] != arrivalsBefore[lost] { |
| t.Fatalf("%s resent past the window: %d -> %d", lost, arrivalsBefore[lost], arrivalsAfter[lost]) |
| } |
| + if tc.later < billingIdempotencyWindow && arrivalsAfter[lost] != arrivalsBefore[lost]+1 { |
| + t.Fatalf("%s not replayed under its key inside the window: %d -> %d", lost, arrivalsBefore[lost], arrivalsAfter[lost]) |
| + } |
| if rec := ix.month(t); rec.State != "complete" { |
| t.Fatalf("month: %+v", rec) |
| } |
| @@ -154,8 +162,8 @@ func TestAWrongMetadataCodeHoldsTheMonth(t *testing.T) { |
| ix.now = ix.now.Add(25 * time.Hour) |
| err = ix.run(t) |
| } |
| - if err == nil { |
| - t.Fatal("the month completed over a wrong code") |
| + if !errors.Is(err, errBillingInvoiceHeld) { |
| + t.Fatalf("over a wrong code: %v", err) |
| } |
| if rec := ix.month(t); rec.State == "complete" || rec.State == "open" { |
| t.Fatalf("month: %+v", rec) |
| @@ -164,6 +172,20 @@ func TestAWrongMetadataCodeHoldsTheMonth(t *testing.T) { |
| if mutations["finalize"] != 0 || mutations["pay"] != 0 { |
| t.Fatalf("finalized or paid over a wrong code: %v", mutations) |
| } |
| + if name == "on the listing a recovery reads" { |
| + // The recovery did not take the wrong-coded item as its own: |
| + // the first operation stays unacknowledged, the sibling was |
| + // never created, and the review names the mismatch. |
| + if op := ix.ops(t)["line-awake_seconds-txcd_business"]; op.State == billingOpAcked { |
| + t.Fatalf("the lost line was recovered over a wrong code: %+v", op) |
| + } |
| + if mutations["line-awake_seconds-txcd_personal"] != 0 { |
| + t.Fatalf("the sibling was created after a wrong-coded recovery: %v", mutations) |
| + } |
| + if reviews := ix.reviews(t); len(reviews) != 1 || reviews[0] != "billing_invoice_unrecovered:"+invoiceFixtureMonth { |
| + t.Fatalf("reviews: %v; want the unrecovered review alone", reviews) |
| + } |
| + } |
| }) |
| } |
| } |
| @@ -368,15 +390,16 @@ func TestWaivedAndOneEmittedCodeRecordsThroughBothReaders(t *testing.T) { |
| if err := ix.st.SetOperator("paul@keymail.dev", true); err != nil { |
| t.Fatal(err) |
| } |
| - calc := storeTwoCodeMonth(t, ix, func(total int64) int64 { return 0 }) |
| - if len(calc.Lines) != 2 { |
| - t.Fatal("two lines were stored") |
| + // A zero share is omitted by apportion: the record the glue would |
| + // write has the one line, frozen as any record is. |
| + calc := ix.expectedCalc(t) |
| + calc.TaxCode = "" |
| + calc.Lines = []plans.OverageLine{{Meter: "awake_seconds", AmountCents: calc.Lines[0].AmountCents, TaxCode: "txcd_business"}} |
| + if !calc.taxShapeOK() { |
| + t.Fatal("the one-emitted-code shape") |
| } |
| - // A zero share is omitted by apportion; store the record as the |
| - // glue would, with the one line. |
| - calc.Lines = calc.Lines[:1] |
| raw, _ := json.Marshal(calc) |
| - if _, err := ix.st.db.Exec(`UPDATE billing_months SET calculation=? WHERE account_id=? AND month=?`, raw, ix.acct.ID, invoiceFixtureMonth); err != nil { |
| + if _, err := ix.st.freezeBillingMonth(ix.acct.ID, invoiceFixtureMonth, raw); err != nil { |
| t.Fatal(err) |
| } |
| if err := olderReaderStep(ix); !errors.Is(err, errBillingInvoiceHeld) { |
| @@ -394,3 +417,27 @@ func TestWaivedAndOneEmittedCodeRecordsThroughBothReaders(t *testing.T) { |
| ix.assertMutations(t, map[string]int{"draft": 1, "line-awake_seconds-txcd_business": 1, "finalize": 1, "pay": 1}) |
| }) |
| } |
| + |
| +// The review for a charge with no attributed usage is filed by the run, |
| +// through the preparer seam, without activation: the month is not frozen, |
| +// nothing reaches the provider, and the hold names the mismatch. (The |
| +// attribution that produces the error is the activation change's to test |
| +// through freeze.) |
| +func TestAnUnattributedChargeHoldsTheMonthForAPerson(t *testing.T) { |
| + ix := taxLifecycleFixture(t) |
| + ix.srv.billing.invoice.prepare = func(context.Context, release.Store, usage.Statement, []plans.Assignment, time.Time) (*billingCalculation, error) { |
| + return nil, errBillingTaxApportion |
| + } |
| + if err := ix.run(t); !errors.Is(err, errBillingInvoiceHeld) { |
| + t.Fatalf("run: %v", err) |
| + } |
| + if reviews := ix.reviews(t); len(reviews) != 1 || reviews[0] != "billing_invoice_tax_code_mismatch:"+invoiceFixtureMonth { |
| + t.Fatalf("reviews: %v", reviews) |
| + } |
| + if _, err := ix.st.billingMonth(ix.acct.ID, invoiceFixtureMonth); !errors.Is(err, sql.ErrNoRows) { |
| + t.Fatalf("a record was frozen: %v", err) |
| + } |
| + if _, mutations, _ := ix.fake.counts(); len(mutations) != 0 { |
| + t.Fatalf("the provider was written: %v", mutations) |
| + } |
| +} |
diff --git a/internal/console/billingtaxsplit.go b/internal/console/billingtaxsplit.go| index b27a0a3c5..90080214b 100644 |
| --- a/internal/console/billingtaxsplit.go |
| +++ b/internal/console/billingtaxsplit.go |
| @@ -257,9 +257,12 @@ func splitLinesByGrossUsage(lines []plans.OverageLine, weights map[string]plans. |
| return out, nil |
| } |
| -// lastSpansCodeLines is the body for the month-end answer as 16c would |
| -// settle it: every line carries the one code given, nothing is divided. |
| -// Not called from freeze until decision 16. |
| +// lastSpansCodeLines is the body for the month-end answer if 16c takes the |
| +// last paid span's code: every line carries the one code given, nothing is |
| +// divided. (If 16c holds a month whose last paid span ends before the |
| +// month does, the glue, which has the spans and the month, refuses before |
| +// calling a body; that branch is written when 16c is.) Not called from |
| +// freeze until decision 16. |
| func lastSpansCodeLines(lines []plans.OverageLine, code string) ([]plans.OverageLine, error) { |
| if !validTaxCode(code) { |
| return nil, errBillingTaxApportion |
| @@ -274,11 +277,16 @@ func lastSpansCodeLines(lines []plans.OverageLine, code string) ([]plans.Overage |
| return out, nil |
| } |
| -// twoCodeBody is one answer to decision 16: given a calculation's lines |
| -// as priced today and the per-code weights of the month, the lines each |
| -// carrying its code. splitLinesByGrossUsage is 16b's proposal; |
| -// lastSpansCodeLines, bound to the last span's code, is the month-end |
| -// answer as 16c would settle it. |
| +// twoCodeBody is one answer to decision 16 in the shape of the two |
| +// proposals: given a calculation's lines as priced today, a way to the |
| +// per-code weights of the month, and the last paid span's code, the lines |
| +// each carrying its code. splitLinesByGrossUsage is 16b's proposal; |
| +// lastSpansCodeLines is the month-end answer if 16c takes the last paid |
| +// span's code. 16b's alternatives (a span's own excess; the pooled |
| +// allowance consumed in time order) would need each span's share of the |
| +// allowance as well, which the glue can compute from the spans it holds; |
| +// the shape grows when the answer needs it, and nothing here is that |
| +// answer. |
| type twoCodeBody func(lines []plans.OverageLine, weights func() (map[string]plans.Resources, error), lastCode string) ([]plans.OverageLine, error) |
| // twoCodeCalculation is the glue activation needs and nothing calls: it |
| @@ -335,6 +343,11 @@ func twoCodeCalculation(calc *billingCalculation, history []plans.Assignment, co |
| // The two bodies in twoCodeBody's shape. |
| func bodySplitByGrossUsage(lines []plans.OverageLine, weights func() (map[string]plans.Resources, error), _ string) ([]plans.OverageLine, error) { |
| + if len(lines) == 0 { |
| + // Nothing to divide (a waived month): the weights, and the Days |
| + // they need, are not asked for. |
| + return nil, nil |
| + } |
| w, err := weights() |
| if err != nil { |
| return nil, err |
diff --git a/internal/console/billingtaxsplit_test.go b/internal/console/billingtaxsplit_test.go| index 854308801..f930fc7ca 100644 |
| --- a/internal/console/billingtaxsplit_test.go |
| +++ b/internal/console/billingtaxsplit_test.go |
| @@ -318,3 +318,45 @@ func TestTheActivationGlueProducesATwoCodeMonth(t *testing.T) { |
| t.Fatalf("split twice: %v", err) |
| } |
| } |
| + |
| +// Two guards of the glue and the weights: a one-code calculation handed |
| +// to the glue is refused, not converted; two transfer meters whose base |
| +// units together pass the integer's reach are refused as overflow; and a |
| +// waived two-code month with usage but no Days is waived under the split |
| +// answer too, since nothing is divided and the Days are not asked for. |
| +func TestTheGluesGuards(t *testing.T) { |
| + terms := plans.PriceTerms{TermsVersion: plans.CurrentTermsVersion, PriceBook: plans.CurrentPriceBook, Currency: plans.USD} |
| + history := monthHistory(terms, false) |
| + later := history[0] |
| + later.Revision, later.EffectiveAt, later.Plan, later.Quantity = 2, sep(16, 0), plans.PlanBusiness, 10 |
| + history = append(history, later) |
| + priced, err := priceBillingCalculation(dailyStatement(), history) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + oneCode := *priced |
| + oneCode.TaxCode = "txcd_10102001" |
| + if _, err := twoCodeCalculation(&oneCode, history, taxSplitCodeOf, bodySplitByGrossUsage); !errors.Is(err, errBillingTaxApportion) { |
| + t.Fatalf("a one-code calculation through the glue: %v", err) |
| + } |
| + oct1 := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC) |
| + huge := usage.Statement{V: 1, Account: "bab", Month: "2026-09", Meters: map[string]float64{usage.MeterEgressGB: 5e9, usage.MeterStoreEgressGB: 5e9}, DaysInMonth: 30, Closed: true, |
| + Days: map[string]map[string]float64{"2026-09-02": {usage.MeterEgressGB: 5e9, usage.MeterStoreEgressGB: 5e9}}} |
| + if _, err := usageByCode(huge, []plans.CoveredInterval{{From: sep(1, 0), To: oct1}}, []plans.PlanSpan{{From: sep(1, 0), To: oct1, Plan: plans.PlanBusiness}}, taxSplitCodeOf); !errors.Is(err, plans.ErrOverflow) { |
| + t.Fatalf("transfer weights past the integer's reach: %v", err) |
| + } |
| + // Waived, with usage on the statement but no Days: a house account's |
| + // month, say. Nothing to divide, so nothing asked. |
| + house := monthHistory(terms, true) |
| + h2 := house[0] |
| + h2.Revision, h2.EffectiveAt, h2.Plan, h2.Quantity = 2, sep(16, 0), plans.PlanBusiness, 10 |
| + house = append(house, h2) |
| + noDays := usage.Statement{V: 1, Account: "bab", Month: "2026-09", Meters: map[string]float64{usage.MeterInstanceHours: 3000}, DaysInMonth: 30, Closed: true} |
| + waived, err := priceBillingCalculation(noDays, house) |
| + if err != nil || !waived.waived() { |
| + t.Fatalf("a house month: %+v %v", waived, err) |
| + } |
| + if calc, err := twoCodeCalculation(waived, house, taxSplitCodeOf, bodySplitByGrossUsage); err != nil || len(calc.Lines) != 0 || !calc.taxShapeOK() { |
| + t.Fatalf("a waived two-code month without days under the split: %+v %v", calc, err) |
| + } |
| +} |