Finance-Accounts/ar-aging-app/docs/audit-2026-07-31-code-revie...

201 lines
13 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# Audit Report — Amazon A/R Aging Dashboard
**Scope:** correctness of every receivable figure the dashboard publishes.
**Performed:** 31 July 2026 · **Basis:** full source-code review of the calculation engine plus
re-performance against the January 2026 production dataset.
**Dataset used:** closing "Jan 2026 Test" (session 1) — 16 Amazon Custom Unified Transaction
files, 13 marketplaces with activity plus Turkey, **3,399,517 transaction rows**, reporting month
2026-01, month-end 2026-01-30, clearing-lag 2 days.
---
## 1. Conclusion
The receivable **calculation** is sound. Re-performance reproduces the Finance team's manual
workbook: **USA January 2026 = 11,110,433** (`Detail!D11`, with the 125.44 reserve applied) and the
column-completeness test ties to the penny on every one of the 16 source files.
The **controls around it were not.** Three defects allowed a wrong or missing number to reach the
ledger without any warning, one of which was actively misstating the group receivable by
**USD 444,658.44** on the January close. The status the dashboard displayed as assurance —
"Reconciled" — was arithmetically incapable of failing.
All confirmed defects are fixed, and six independent controls now run on every close. **No
receivable value changed as a result of the fixes** (see §5).
---
## 2. Findings
Severity is ledger impact. "Active" = present in the January 2026 data. "Latent" = the code path
exists and will misstate when the triggering condition occurs, but did not occur in this dataset.
| # | Finding | Status | Measured exposure |
|---|---|---|---|
| **F1** | **Reconciliation Control added different currencies together** — each marketplace's closing was summed in its own local currency (USD + EUR + GBP + PLN + SEK + CAD + AUD as one figure). This was the figure gating Finance sign-off. | **Active** | **USD 444,658.44 understated** (2,278,406.86 vs 2,723,065.30) |
| **F3** | **The "Reconciled" status could not fail.** `uploaded_total` was accumulated from the same record stream that filled the three buckets it was compared against — one sum written twice. | **Active** | Reported "Reconciled" while F1 was live |
| **F8** | **Payout rows were never classified.** The post-classification `UPDATE` matched on `account_type`, but the aggregator bucketed blank account types as `(unspecified)` while the database stored `""`. | **Active** | **25 rows carrying 4,782,085.25** left with a NULL status |
| **F2** | **Stale FX applied silently to any month.** `DEFAULT_FX_USD` is a hardcoded January-2026 snapshot, merged in for every closing. A July close would have valued EUR at January's rate and said nothing. | **Active (latent misstatement)** | 12 of 13 marketplaces on unconfirmed defaults |
| **F13** | **A partial `PUT /fx` deleted every rate not named in it** (delete-all-then-insert). Discovered during verification: a one-marketplace update reduced 14 stored rates to 1, after which the close silently fell back to hardcoded defaults. | **Active** | 13 of 14 rates destroyed by one call |
| **F4** | Order rows with an unrecognized `account type` are excluded from the receivable with no warning. | **Latent** | **0 rows** — all 3,399,517 rows resolved cleanly |
| **F5** | Order rows with a blank/non-numeric settlement id sort to 1 and are silently classified "paid". | **Latent** | **0 rows** |
| **F6** | **The two parsers were not interchangeable** — different sheet-selection tie-break, and different row-emptiness rules (one tested the converted value, where empty amount cells become `0.0`, so every row qualified). | **Latent** | Readers agreed on all 16 real files; divergence reproduced on a synthetic fixture (4 rows vs 3) |
| **F7** | The journal pass re-read the files independently of the settlement pass, with a different field set and ignoring per-file marketplace overrides. | **Latent** | No override in use on this close |
| **F9** | Two headers mapping to the same field: the second column was silently discarded. | **Latent** | No collisions in these files |
| **F10** | Sign-off did not invalidate when the numbers changed — a closing could read "verified by X" against figures X never saw. | **Active (process risk)** | — |
| **F11** | The Aging tab was not an aging: 100% was forced into "Current" unconditionally, so a settlement Amazon was holding could never surface. Now banded by **days past due** (see below). | **Active (presentation)** | Bands unchanged for Jan-2026 (all Current); a held settlement now ages |
| **F12** | Currency fell back to a bare `"USD"` when a marketplace had no result row — EUR amounts could render labelled USD. | **Latent** | — |
### What was already correct
Worth stating plainly, because it bounds the exposure:
- **Column completeness ties exactly.** Σ(GL lines) Σ(source `total` column) = **0.00** across all
16 files. No amount column is unmapped, double-mapped, or dropped.
- **Bucket completeness is clean.** Every one of the 3,399,517 rows resolves to a recognized
account type and a numeric settlement id.
- **The two readers agreed** on all 16 real files, so no past close depended on which parser ran.
- **The receivable calculation itself matches the manual workbook** to the penny.
### Turkey — an empty file is indistinguishable from a failed one
`Turkey Amazon Transactions January, 2026.xlsx` is 10.7 KB with worksheet extent `A1:T7`: six
preamble rows, one header row, **no data rows**. Turkey genuinely had no January activity, so this
is *not* a misstatement.
It is, however, the clearest illustration of the core problem. Eleven Turkish amount headers
(`ürün satışları`, `satış ücretleri`, `Amazon Lojistik ücretleri` …) are unmapped, and the
`unmapped_amounts` safeguard stayed silent — because it only sums over rows that were read, and no
rows were read. A file that fails to parse produces exactly the same silent zero as a file with
nothing in it. Control **C1** now separates the two by checking the worksheet's own declared extent.
---
## 3. Controls now in place
Six controls run automatically at the end of every close and on demand. Each compares the engine's
output against **something the engine did not produce** — that independence is what allows them to
fail. A failure at error severity puts the closing in `blocked`: `/summary`, `/finance-summary`,
`/aging` and both Excel exports withhold the figure and return the reason instead.
| Control | Checks against | Fails when |
|---|---|---|
| **C1 Source row count** | the worksheet's own `<dimension>` | rows read ≠ rows the file declares |
| **C2 Column completeness** | Amazon's own `total` column | Σ(GL lines) ≠ Σ(`total`) — a column unmapped, double-mapped, or newly added |
| **C3 Bucket completeness** | the receivable filter itself | a money-carrying order row has no recognized account type or settlement id |
| **C4 Dual-method agreement** | the settlement method vs the roll-forward | the two closing methods diverge (warning — a first close legitimately differs) |
| **C5 FX confirmed** | a human, for this reporting month | any non-USD rate is a seeded default or was confirmed for a different month |
| **C6 Currency integrity** | the All-Markets roll-up | the two group totals disagree — i.e. someone added currencies without converting |
| **C7 Reader equivalence** | the other parser | the two readers disagree on any fixture (CI-time, `tests/test_reader_equivalence.py`) |
**Design rule:** a number that cannot be trusted is never displayed. A missing number cannot be
posted to the ledger; a wrong one can.
---
## 4. Result on the January 2026 close
Re-run after the fixes, the closing **blocked** on C5 — 12 marketplaces valued at unconfirmed
January-2026 default rates. After Finance confirmed the rates for the reporting month, it released:
```
C1 Source row count PASS 16 file(s) fully accounted for
C2 Column completeness PASS GL lines tie to the source `total` column (difference 0.00)
C3 Bucket completeness PASS every order row resolves to a receivable bucket
C4 Dual-method agreement REVIEW 13 marketplaces disagree (see below) — warning, non-blocking
C5 FX rates confirmed PASS confirmed for 2026-01
C6 Currency integrity PASS both group roll-ups agree at 2,723,065.30 USD
```
### C4 needs your attention
The two closing methods disagree on every marketplace, most starkly on USA:
| | Settlement method | Roll-forward | Difference |
|---|---|---|---|
| USA | 11,110,308.00 | 241,594.30 | 10,868,713.70 |
| Canada | 1,103,090.00 CAD | 398,604.11 CAD | 704,485.89 |
| Group (USD) | **14,518,331.37** | **2,723,065.30** | 11,795,266.07 |
**Cause: every opening AR balance on this closing is zero** (`opening_mode = "zero"`). The
roll-forward is `opening + net revenue payouts received`, so with no opening balance it measures
only January's movement, not the receivable actually outstanding. The settlement figure of
14,518,331.37 is the correct one; the roll-forward — and therefore the AR Ledger and Finance
Summary tabs — is not usable on this closing until opening balances are entered.
This was previously invisible: the tabs showed both numbers with no indication that one was
unusable. **Action:** enter December 2025 closing balances as the opening, or carry forward from a
prior processed closing, then re-run the controls.
---
### A note on the aging basis (F11)
An A/R aging must measure **days past due**, not days since the transaction. An Amazon
receivable is not due when the order posts — it is due when its settlement disburses, roughly 14
days after the settlement's last activity plus the clearing lag.
This matters more than it sounds. Banding by transaction date instead would have filed
**9,556,111.11 of USA's 11,110,308 as "1-30 days overdue"** when it was simply a normal biweekly
settlement not yet due — contradicting the Finance workbook and making the report meaningless.
With the due-date basis, January 2026 is **100% Current across all 13 marketplaces**, exactly as
the manual workbook shows, while a genuinely stuck settlement now ages out of Current.
Covered by `tests/test_aging.py`.
---
## 5. No receivable value changed
Every fix was verified not to move a number. Before and after the full set of changes, on the same
source files:
| | Before | After |
|---|---|---|
| USA settlement receivable | 11,110,308 | **11,110,308** |
| Group closing receivable (USD) | 14,518,331.37 | **14,518,331.37** |
| Transaction rows | 3,399,517 | **3,399,517** |
| Σ all rows | 7,277,434.83 | **7,277,434.83** |
| Rows with NULL settlement status | 25 (4,782,085.25) | **0** |
| Reconciliation Control closing | 2,278,406.86 *(currencies mixed)* | **2,723,065.30** *(converted)* |
The two changed lines are the two defects: the payout rows are now classified, and the group total
is now converted rather than summed across currencies.
---
## 6. How to re-perform this audit
1. **Benchmark**`python -m pytest tests -m integration` (12 tests). Asserts USA January 2026 =
11,110,433 against the manual workbook and that all 13 marketplaces reconcile.
2. **Controls and equivalence**`python -m pytest tests -m "not integration"` (99 tests),
including `tests/test_controls.py`, which introduces each defect and asserts the close blocks.
3. **Parse-level re-performance** — for each source file, assert
`Σ(component amount columns) == Σ(total column)` per row, and that rows read equal the
worksheet's declared extent. Both are now controls C2 and C1.
4. **Currency check** — compare `/api/sessions/{id}/reconciliation-control` against
`/api/sessions/{id}/all-markets`. They must agree to the cent; that is control C6.
5. **Negative testing** — the controls are only worth their green light if they can go red. Each
test in `tests/test_controls.py` constructs the real defect (a half-parsed file, an unmapped
amount column, an unrecognized account type, a January rate on a July close) and asserts the
close is blocked.
### Known limitations
- **C4 is a warning, not a block.** A first-ever close legitimately has zero openings, so blocking
on it would be unusable. It must be read, not dismissed.
- **The reserve is a manual input.** USA reconciles to 11,110,433 only when the 125.44 reserve is
entered; without it the engine returns 11,110,308. No control validates the reserve against an
external source.
- **FX rates are confirmed, not sourced.** C5 proves a person accepted each rate for the reporting
month; it cannot prove the rate is right. Per the project's security constraint, no rate is ever
fetched from an external service.
- **Latent findings are fixed but unexercised by production data.** F4, F5, F6, F7, F9 and F12 are
covered by unit tests against synthetic fixtures, not by January 2026 data.
---
*Prepared as part of the A/R Aging dashboard audit. Source references: `app/core/controls.py`
(control definitions and the reasoning behind each), `app/core/reconciliation.py` (why the old
identity was not a control), `app/core/money.py` (currency-safe aggregation).*