Phase 2.6: Rule coverage, reconciliation tests, docs close-out

backend/app/collections/README.md

@@ -5,17 +5,22 @@ service built on `fastapi/full-stack-fastapi-template`. This file is the
 single source of truth for what the service does and how to run it; see
 `docs/` for internals (owned per file, see below).
 
-Status: **Phase 1 (Domain core)**. This README will be filled in as each
-phase lands; see `../../../docs/CHANGELOG.md` for what has shipped so far.
+Status: **Phase 2 (Validation and exceptions) complete**. This README
+will be filled in as each phase lands; see `../../../docs/CHANGELOG.md`
+for what has shipped so far.
 
 ## What this does
 
 Computes an overdue collections position — invoice-level and by
 region/customer — from an ERP export (Customers, Invoices, Payments,
-Region_Mapping sheets), as of a fixed report date. Phase 1 covers the pure
-calculation core: loading the workbook into typed records and computing
-outstanding/overdue/ageing/region figures. No API, database, or exception
-reporting yet — see `docs/CHANGELOG.md` for what each phase adds.
+Region_Mapping sheets), as of a fixed report date, and flags every data
+quality or business-rule issue it finds along the way rather than
+silently dropping the affected row. Phase 1 built the pure calculation
+core (loading the workbook into typed records, computing
+outstanding/overdue/ageing/region figures); Phase 2 added the 14-rule
+exception catalogue (`docs/RULES.md`) that explains every exclusion. No
+API or database persistence yet — see `docs/CHANGELOG.md` for what each
+phase adds.
 
 ## How to run
 
@@ -70,9 +75,30 @@ guard, and the agent-side build tooling (ponytail, caveman, anydoc)._
 
 ## Validation performed
 
-_Filled in Phase 2 and 10: rule coverage test, reconciliation test,
-independent SQL recompute — see `docs/DEVELOPMENT.md` for how to run the
-test suite._
+- **107 tests** (`uv run pytest app/collections/tests -v`): loader/resolver
+  tests, boundary tests on the calculators, a positive and a negative case
+  per exception rule against hand-built records, and the tests below.
+- **Rule coverage test** (`tests/validate/test_coverage.py`): asserts every
+  one of the 14 rules fires at least once against `dataset_a_original.xlsx`,
+  and that the registry covers exactly E001-E014 — a rule that never
+  triggers is treated as a bug, not good news.
+- **Reconciliation tests** (`tests/test_reconcile.py`): two identities
+  proving nothing silently vanishes. Every invoice's
+  `outstanding + valid_paid == invoice_amount + overpaid`, summed across
+  the full 36-invoice portfolio; and every payment rupee is either applied
+  to reduce some invoice's outstanding or explicitly unapplied, with every
+  unapplied payment traceable to an exception row. These are
+  internal-consistency checks — verifying the calculator's own arithmetic
+  never creates or destroys a rupee — not an independently-derived second
+  method; that's the SQL crosscheck deferred to Phase 3+, once persistence
+  exists to recompute from. Verifying the pandas-free result with a second
+  method there will be the most convincing possible answer to "what
+  validation did you perform."
+- **Exact-ID assertions throughout**, not just "fires at least once":
+  every rule test and the reference-number regression test assert the
+  precise set of invoice/payment/customer IDs a rule or calculation
+  should produce, so a rule silently over- or under-firing is caught, not
+  just a rule going fully silent.
 
 ## Owning-doc map
 

backend/app/collections/docs/DEVELOPMENT.md

@@ -3,7 +3,7 @@
 Owns: local setup, commands, git workflow, commit format, testing,
 release process. Does not own: business logic (see `README.md`).
 
-Status: **Phase 0 skeleton.**
+Status: **Phase 2 complete.**
 
 ## Local setup
 
@@ -17,7 +17,8 @@ changes that flow yet.
 
 ```
 cd backend
-uv run pytest app/collections/tests -v                       # Phase 1 test suite
+uv run pytest app/collections/tests -v                       # full test suite (107 tests)
+uv run pytest app/collections/tests/validate -v               # exception rule tests only
 uv run python -m app.collections.scripts.reference_summary   # reference numbers against dataset A
 ```
 
@@ -31,9 +32,14 @@ the repo root for the full standing instructions this build follows.
 Phase 1 covers boundary tests (due date exactly on the report date; a
 payment landing exactly on the report date) and a reference-number
 regression test against `fixtures/dataset_a_original.xlsx`
-(`tests/test_reference_numbers.py`). Rule-unit/coverage/gate/guard/
-failure/reconcile layers are added from Phase 2 on — see MASTER_PLAN.md
-section 10 for the full test layer breakdown.
+(`tests/test_reference_numbers.py`). Phase 2 added one test file per
+exception rule under `tests/validate/` (positive case against dataset A
+plus hand-built negative/boundary cases), a rule-coverage test
+(`tests/validate/test_coverage.py`), and two reconciliation identities
+(`tests/test_reconcile.py`). Gate/guard/failure layers are added from
+Phase 3 on; the independently-derived SQL crosscheck needs persistence
+and is deferred there too — see MASTER_PLAN.md section 10 for the full
+test layer breakdown.
 
 ## Release process
 

backend/app/collections/docs/RULES.md

@@ -3,11 +3,13 @@
 Owns: the exception rule catalogue, single source of truth for E001-E014.
 Does not own: anything that isn't a rule.
 
-Status: **14/14 rules implemented** in `validate/rules.py`, registered in
-`validate/engine.py`. Coverage asserted in
-`tests/validate/test_coverage.py` (lands in the next PR, along with the
-reconciliation tests). This file is the single source of truth for the
-catalogue and must match the code exactly.
+Status: **14/14 rules implemented and complete.** `validate/rules.py`
+defines each rule; `validate/engine.py`'s `RULE_REGISTRY` registers all
+14; `tests/validate/test_coverage.py` asserts every rule fires on
+dataset A and the registry covers exactly E001-E014;
+`tests/test_reconcile.py` proves no invoice or payment rupee is lost.
+This file is the single source of truth for the catalogue and must match
+the code exactly.
 
 ## Severity
 
@@ -22,20 +24,24 @@ which describes pipeline-stage health, not business weight.
 | Code | Category | Severity | Dataset A count |
 |---|---|---|---|
 | E001 | Missing due date | error | 1 |
+| E002 | Unknown customer reference (invoice or payment) | error | 2 |
+| E003 | Unknown invoice reference | error | 1 |
 | E004 | Non-positive invoice amount | error | 1 |
-| E011 | Cancelled invoice | warning | 1 |
-| E012 | Credit Note invoice | warning | 1 |
-| E013 | Duplicate source system reference | warning | 2 |
+| E005 | Non-positive payment amount | error | 2 |
 | E006 | Invalid GSTIN format | warning | 1 |
+| E007 | Payment-to-invoice customer mismatch | error | 1 |
 | E008 | Missing GSTIN | warning | 1 |
-| E005 | Non-positive payment amount | error | 2 |
 | E009 | Payment after report date | warning | 1 |
-| E002 | Unknown customer reference (invoice or payment) | error | 2 |
-| E003 | Unknown invoice reference | error | 1 |
-| E007 | Payment-to-invoice customer mismatch | error | 1 |
 | E010 | Payment before invoice date | warning | 1 |
+| E011 | Cancelled invoice | warning | 1 |
+| E012 | Credit Note invoice | warning | 1 |
+| E013 | Duplicate source system reference | warning | 2 |
 | E014 | Overpayment (payments exceed invoice amount) | warning | 1 |
 
+17 exception rows total against 36 invoice records (~47%) — consistent
+with QA_PREP.md Q7's "roughly 16 to 19, about 45%" as an independent
+sanity check that this catalogue is calibrated correctly.
+
 ## E001 — Missing due date
 
 **Condition:** `Invoice.due_date is None`.
@@ -46,6 +52,31 @@ date).
 **Why:** an invoice with no due date cannot be classified overdue or
 current; excluding it and flagging it beats guessing a date.
 
+## E002 — Unknown customer reference
+
+**Condition:** `customer_id` on an invoice *or* a payment is not present
+in the Customers sheet. One rule, checked against both sheets, since
+either kind of record can carry a dangling customer reference.
+**Fires on:** INV-1026 (Customer_ID C999), PAY-2025 (Customer_ID C999).
+PAY-2025 also fires E003 independently (its Invoice_ID is unknown too) —
+deliberately two separate rows rather than one combined flag, so each
+broken reference is visible regardless of whether the other were fixed.
+**Excludes:** the invoice from the overdue report; a payment referencing
+an unknown customer is also, incidentally, unresolvable (see E003).
+**Why:** cannot attribute a Region or a credit position to a customer
+that doesn't exist in the master data.
+
+## E003 — Unknown invoice reference
+
+**Condition:** `Payment.invoice_id` is not present in the Invoices sheet.
+**Fires on:** PAY-2025 (Invoice_ID INV-9999). PAY-2025 also fires E002
+independently (its Customer_ID is C999) — see E002 above.
+**Excludes:** the payment is never applied to anything —
+`calculate/outstanding.py` indexes payments by real invoice IDs, so there
+is nothing to index it under.
+**Why:** cash received against a reference that doesn't exist needs a
+human to say what it was actually for.
+
 ## E004 — Non-positive invoice amount
 
 **Condition:** `Invoice.invoice_amount <= 0`.
@@ -55,33 +86,15 @@ current; excluding it and flagging it beats guessing a date.
 or a credit that hasn't been reclassified as a Credit Note; either way
 it should not silently net into the overdue total.
 
-## E011 — Cancelled invoice
-
-**Condition:** `Invoice.status == "Cancelled"`.
-**Fires on:** INV-1029.
-**Excludes:** the overdue report (`calculate/overdue.py`'s
-`ELIGIBLE_STATUS`).
-**Why:** correct-by-design exclusion, not defective data — shown here so
-nothing silently disappears from the picture.
-
-## E012 — Credit Note invoice
-
-**Condition:** `Invoice.status == "Credit Note"`.
-**Fires on:** INV-1030.
-Same treatment and rationale as E011.
-
-## E013 — Duplicate source system reference
+## E005 — Non-positive payment amount
 
-**Condition:** two or more invoices share a non-blank
-`Source_System_Ref`. Blank references are not compared against each
-other — a shared "no reference recorded" is not a duplicate.
-**Fires on:** INV-1011 and INV-1031, both `EMP-SO-0011`. Emitted as one
-row per affected invoice (two rows total), each naming the sibling(s).
-**Excludes:** nothing — both invoices remain in the overdue report
-independently.
-**Why:** a shared ERP reference across two different invoice records can
-mean a duplicate entry or a legitimately split sales order; either way it
-needs investigation, not an automatic guess.
+**Condition:** `Payment.payment_amount <= 0`.
+**Fires on:** PAY-2018 (0), PAY-2029 (-5,000).
+**Excludes:** the payment from the valid-payments total for its invoice
+(`calculate/outstanding.py`'s `is_valid_payment` requires `> 0`).
+**Why:** a zero or negative payment amount cannot reduce an outstanding
+balance; it is either a reversal that needs its own record or a
+recording error.
 
 ## E006 — Invalid GSTIN format
 
@@ -96,6 +109,20 @@ math.
 **Why:** flags master-data quality for whoever owns Customer records;
 doesn't block collection reporting.
 
+## E007 — Payment-to-invoice customer mismatch
+
+**Condition:** `Payment.customer_id != Invoice.customer_id` for the
+invoice the payment names (only evaluated when that invoice exists — see
+E003).
+**Fires on:** PAY-2026 — recorded against customer C003, but INV-1002
+belongs to C002.
+**Excludes:** the payment from C002's valid paid total, so C002 correctly
+still shows overdue rather than being credited with someone else's cash.
+**Why (worked example, matches MASTER_PLAN.md section 8 and QA_PREP.md
+Q11):** the payment is flagged rather than reassigned, since reassignment
+would be a silent data correction moving cash between ledgers on a guess.
+Owner: Accounts Receivable.
+
 ## E008 — Missing GSTIN
 
 **Condition:** `Customer.gstin is None`.
@@ -105,16 +132,6 @@ doesn't block collection reporting.
 because "missing" and "malformed" call for different remediation (get
 the value vs. fix the value).
 
-## E005 — Non-positive payment amount
-
-**Condition:** `Payment.payment_amount <= 0`.
-**Fires on:** PAY-2018 (0), PAY-2029 (-5,000).
-**Excludes:** the payment from the valid-payments total for its invoice
-(`calculate/outstanding.py`'s `is_valid_payment` requires `> 0`).
-**Why:** a zero or negative payment amount cannot reduce an outstanding
-balance; it is either a reversal that needs its own record or a
-recording error.
-
 ## E009 — Payment after report date
 
 **Condition:** `Payment.payment_date > report_date`.
@@ -128,45 +145,6 @@ business activity, not bad data — hence `warning`, not `error` — but a
 reviewer looking at why a position didn't move needs to see it, not
 infer it.
 
-## E002 — Unknown customer reference
-
-**Condition:** `customer_id` on an invoice *or* a payment is not present
-in the Customers sheet. One rule, checked against both sheets, since
-either kind of record can carry a dangling customer reference.
-**Fires on:** INV-1026 (Customer_ID C999), PAY-2025 (Customer_ID C999).
-PAY-2025 also fires E003 independently (its Invoice_ID is unknown too) —
-deliberately two separate rows rather than one combined flag, so each
-broken reference is visible regardless of whether the other were fixed.
-**Excludes:** the invoice from the overdue report; a payment referencing
-an unknown customer is also, incidentally, unresolvable (see E003).
-**Why:** cannot attribute a Region or a credit position to a customer
-that doesn't exist in the master data.
-
-## E003 — Unknown invoice reference
-
-**Condition:** `Payment.invoice_id` is not present in the Invoices sheet.
-**Fires on:** PAY-2025 (Invoice_ID INV-9999). PAY-2025 also fires E002
-independently (its Customer_ID is C999) — see E002 above.
-**Excludes:** the payment is never applied to anything —
-`calculate/outstanding.py` indexes payments by real invoice IDs, so there
-is nothing to index it under.
-**Why:** cash received against a reference that doesn't exist needs a
-human to say what it was actually for.
-
-## E007 — Payment-to-invoice customer mismatch
-
-**Condition:** `Payment.customer_id != Invoice.customer_id` for the
-invoice the payment names (only evaluated when that invoice exists — see
-E003).
-**Fires on:** PAY-2026 — recorded against customer C003, but INV-1002
-belongs to C002.
-**Excludes:** the payment from C002's valid paid total, so C002 correctly
-still shows overdue rather than being credited with someone else's cash.
-**Why (worked example, matches MASTER_PLAN.md section 8 and QA_PREP.md
-Q11):** the payment is flagged rather than reassigned, since reassignment
-would be a silent data correction moving cash between ledgers on a guess.
-Owner: Accounts Receivable.
-
 ## E010 — Payment before invoice date
 
 **Condition:** `Payment.payment_date < Invoice.invoice_date` (only when
@@ -179,6 +157,34 @@ README.md Assumptions; this is a data-quality-only flag.
 pre-payment against a purchase order), but the workbook doesn't
 authorize excluding it, so the calculation still counts it in full.
 
+## E011 — Cancelled invoice
+
+**Condition:** `Invoice.status == "Cancelled"`.
+**Fires on:** INV-1029.
+**Excludes:** the overdue report (`calculate/overdue.py`'s
+`ELIGIBLE_STATUS`).
+**Why:** correct-by-design exclusion, not defective data — shown here so
+nothing silently disappears from the picture.
+
+## E012 — Credit Note invoice
+
+**Condition:** `Invoice.status == "Credit Note"`.
+**Fires on:** INV-1030.
+Same treatment and rationale as E011.
+
+## E013 — Duplicate source system reference
+
+**Condition:** two or more invoices share a non-blank
+`Source_System_Ref`. Blank references are not compared against each
+other — a shared "no reference recorded" is not a duplicate.
+**Fires on:** INV-1011 and INV-1031, both `EMP-SO-0011`. Emitted as one
+row per affected invoice (two rows total), each naming the sibling(s).
+**Excludes:** nothing — both invoices remain in the overdue report
+independently.
+**Why:** a shared ERP reference across two different invoice records can
+mean a duplicate entry or a legitimately split sales order; either way it
+needs investigation, not an automatic guess.
+
 ## E014 — Overpayment (payments exceeding invoice amount)
 
 **Condition:** sum of valid payments for an invoice (the same
@@ -205,4 +211,7 @@ business decision the workbook doesn't define — surfaced, not assumed.
   nothing.
 - E014 only evaluates invoices with `invoice_amount > 0`, to avoid a
   spurious fire against an already-E004-flagged negative-amount invoice
-  with zero payments against it.
+  with zero payments against it. It also doesn't distinguish "genuine
+  overpayment" from "a floored negative amount" in the same way as the
+  reconciliation identity in `tests/test_reconcile.py` — see that file's
+  docstring for the subtlety this uncovered.