test(tax): property-based invariants for the tax engine - #117
gfargo-horizon-agent[bot] merged 2 commits into
Conversation
Six invariants covering the engine and exporters: 1. Basis conservation — Σ lotsConsumed[].costBasis == disposal.costBasis for every DisposalResult produced by computeTax (FIFO + HIFO). 2. Fee conservation — under subtract-from-proceeds, net proceeds are reduced by exactly the fee leg's USD value (guards bug B5). 3. Term partitioning — disposal.term matches classifyTerm(acquiredAt, disposedAt) for all disposals (guards bug B4, commit ee9e5c8). 4. Method ordering — HIFO total gain <= FIFO total gain for pure-gain histories (HIFO minimises gain by selling highest-cost lots first). 5. Determinism — computeTax called twice on identical input is deep-equal across all disposal fields. 6. Cross-format agreement — CSV footer, Form 8949 page totals, Schedule D line7/line15, and TXF proceeds-minus-costBasis all reconcile to the same short/long-term gain totals. Also adds arbLedgerHistory (funded, strictly-ordered LedgerEntry sequence over BTC/ETH/SOL) and arbPureGainHistory (all-gains scenario) to test-helpers.ts, and exports HISTORY_TAX_YEAR for use in tests. 10 new property tests; 150 runs each for engine tests, 100 for formatter tests.
![gfargo-horizon-agent[bot]](https://avatars.githubusercontent.com/in/4083186?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 Agent review (kiro · sonnet→opus) — CONCERNS
REVIEW: CONCERNS
RESOLVES: partial
All six invariants compile and the generator design is sound, but two invariants are weaker than they claim: the term test is tautological (self-referential against classifyTerm), and the fee test uses a single out-leg and so cannot actually catch B5. The proposal's explicit permutation-invariance guard for B30 is also absent from the determinism invariant.
5 concerns · 1 nit — 6 inline on the diff
There was a problem hiding this comment.
🔎 Agent re-review (kiro · sonnet, delta) — CONCERNS
Try running kiro-cli login to re-authenticate, or kiro-cli profile to select a profile.
All tools are now trusted (!). Kiro will execute tools without asking for confirmation.
Agents can sometimes do unexpected things so understand the risks.
Learn more at https://kiro.dev/docs/cli/chat/security/#using-tools-trust-all-safely
Monthly request limit reached
You can enable overages to continue making requests.
The limits reset on 09/01.
Addresses PR #117 review feedback: - Term invariant now compares against an independently implemented reference classifier instead of calling classifyTerm on its own output, so a regression inside classifyTerm is actually caught. - Fee invariant adds a multi-out-leg trade case (two assets sold in one trade with one fee leg) — a single-out-leg trade can't distinguish "fee once per trade" from "fee once per leg" (B5). - Determinism invariant adds permutation-invariance cases: computeTax on a shuffled copy of the same entries must match the original, guarding B30. - TXF cross-format test now runs across all three checkbox categories (A, B, C) instead of only 'C', exercising the full tax-line mapping. - CSV footer parser now uses csv-parse instead of hand-rolled quote stripping, matching the library formatCsv writes with. - Removed an unused alias in arbPureGainHistory.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The revise directly fixes all six flagged issues with correct, independent implementations (non-tautological term reference, genuine multi-out-leg fee test, real permutation invariance, A/B/C TXF coverage, library-based CSV parsing, dead code removed), and all 13 tests pass with no regressions.
What
Add six property-based invariants that test the tax engine (
computeTax) and exporters end-to-end. Usesfast-check(already a dependency) with newarbLedgerHistoryandarbPureGainHistoryarbitraries that generate fully-funded, deterministically-orderedLedgerEntrysequences.New file:
packages/tax/src/engine-invariants.test.ts— 10 property tests across 6 invariants.Edited file:
packages/tax/src/test-helpers.ts— addsarbLedgerHistory,arbPureGainHistory, andHISTORY_TAX_YEARexports.Why
Plane: OSS-1382
How
Σ lotsConsumed[].costBasis == disposal.costBasisfor everyDisposalResultfromcomputeTax(FIFO + HIFO, 150 runs each).subtract-from-proceeds, net proceeds are reduced by exactly the fee leg's USD value — guards bug B5 (fee allocated once per trade vs once per out-leg).disposal.term === classifyTerm(acquiredAt, disposedAt)for all disposals — guards bug B4 (365-day vs calendar-anniversary), commit ee9e5c8.computeTaxcalled twice on identical input produces deep-equal disposal arrays, income totals, and warning lists.arbTaxResult.Arbitraries are designed to be safe:
Testing
tsc -bpasses (typecheck)pricing/chain.test.tsfails — pre-existing better-sqlite3 native binding issue in this environment, fails identically onmain)test-helpers.tslints clean (test files are excluded from lint by project config)🤖 Generated by the harbor agent loop. Reviewed by a human before merge.
Closes #85