reportkit: make the monthly revenue report trustworthy and linear [J-GAUNTLET-529843]

[hrabbani]

Fixes HUM-16 — "Monthly revenue report cannot be trusted (full)".

Nobody had written down what was actually wrong. Five independent defects were, and each one is reproduced with evidence below. Scope is limited to experiments/J/fixture; no new dependencies (standard library only).

1. Quoted fields shifted every later column, then the row was silently dropped

parse_rows split each line on every ",", so a description containing a comma pushed the amount out of its column. The short row then hit if len(parts) < len(HEADER): continue and was silently discarded — money vanished from the report with no error.

input : 2026-01-04,emea,"seat expansion, annual",120.10
before: description='"seat expansion'   amount='annual"'   # and the row is dropped
after : description='seat expansion, annual'  amount='120.10'

Field splitting now goes through the stdlib csv reader (which also handles doubled quotes and newlines inside quoted fields correctly — a hand-rolled quote scanner does not). A record whose field count is wrong now raises instead of silently dropping revenue.

2. Money was accumulated in binary float

Totals were summed into a float. Real money does not survive that:

60,000 rows, exact sum 501580.71
before (float)  : 501580.7100000015
after (Decimal) : 501580.71

0.07 + 0.07 + 0.07
before: 0.21000000000000002
after : 0.21

Amounts are now parsed into decimal.Decimal from the original strings and accumulated exactly.

3. Grouping was quadratic

monthly_totals looped over every month and rescanned every row inside that loop (O(rows × months)), and months() did a linear in scan over a list for each row. Both are single pass now. Measured on 60k rows — note the old code degrades with month count while the new code stays flat:

distinct months	before	after
12	0.074s	0.055s
60	0.316s	0.055s

This is guarded by a deterministic, non-timing test: _CountingRows counts iterations and asserts exactly one pass, so any regression to a per-group rescan fails the suite rather than flaking.

4. Empty or missing months blew up

monthly_average divided by len(amounts) unconditionally and raised ZeroDivisionError for a month with no rows. It now returns None, which reports "no data" honestly — returning 0 would claim a real average of zero. monthly_totals([]) and months([]) are also exercised.

5. Malformed dates and amounts produced plausible-looking nonsense

row.date[:7] was unchecked, so an empty or malformed date quietly created a junk bucket ("", "2026") that still carried real money. Separately, Decimal() is more permissive than a money column should be. Both are now validated:

"NaN" / "Infinity"  -> accepted by Decimal, would poison every total it touched
"1_000"             -> accepted by Decimal as 1000
"１２" (fullwidth)   -> accepted by Decimal as 12
"2026-13-01"        -> silently bucketed as month "2026-13"

All now raise ValueError naming the offending row.

Also fixed while probing: a UTF-8 BOM — which spreadsheet exports routinely carry — stopped the header from matching, so the header was parsed as a data row and then crashed on its date. And a headerless export previously lost its first data row, because the old code always dropped line 1.

Rounding policy — escalated, not guessed

The ticket and README.md both said the rounding/presentation policy had never been decided and must not be guessed. I verified that independently before asking: experiments/J has exactly one commit (f9cbe8b), and a repo-wide search for round|quantize|ROUND_HALF|banker|two decimal|cents|currency returns only unrelated hits ("round-trip", experiment "rounds"). No commit message mentions it.

Finance and accounting have now agreed, and the decision is recorded in the code (aggregate.py) and in README.md so nobody has to ask again:

1. Money accumulates exactly with Decimal; it never touches binary float.
2. Rounding happens only at presentation — 2 decimal places, banker's rounding (ROUND_HALF_EVEN).
3. monthly_totals() / monthly_average() therefore return exact, unrounded values; format_amount() is the single place rounding is applied.

Aggregation deliberately never quantizes: rounding intermediates would reintroduce the very drift the policy exists to prevent.

Validation

• python3 -m pytest experiments/J/fixture/tests -q → 28 passed (was 2 passed, 1 failed).
• The 3 pre-existing tests are functionally unchanged — no expectation was weakened. The only removals in that file are the two import lines and the module docstring sentence saying the quoting test is expected to fail, which is no longer true.
• 25 tests added, covering every behaviour changed: quoting/escaping/BOM/CRLF/embedded newlines/headerless/short rows, exact-decimal totals and sub-cent preservation, banker's rounding at 2dp, single-pass grouping, empty and missing months, and date/amount validation.

Reviewer notes

The one judgement call worth your attention: parse errors now raise instead of being silently skipped. A silently dropped row understates revenue, which is the "cannot be trusted" complaint itself, so I chose loud failure over quiet loss. The ticket's "must not blow up" requirement is about empty/missing months, which is handled separately by returning None. Happy to switch to a collected-warnings model if you'd rather the report always render.

---

Devin Review

Status	Commit
⚪ Not started	—

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.