Jan Novak
8734089223
All checks were successful
Deploy to K8s / deploy (push) Successful in 9s
Replace proportional split with a fill-first loop that allocates min(remaining, deficit) to each matched month in user-supplied order, where deficit = expected - already_paid. Prior transactions' contributions are now properly accounted for, so a second payment on overlapping months fills only what's still owed instead of splitting proportionally by total expected. Surplus after all deficits are covered goes to the credit bucket. Fixes: Matyáš Thér 200+550 showing 566/183 instead of 500/250. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
185 lines
10 KiB
Markdown
185 lines
10 KiB
Markdown
# Fill-first multi-month payment allocation
|
||
|
||
## Context
|
||
|
||
Matyáš Thér paid in two transactions:
|
||
|
||
| # | Amount | Purpose |
|
||
|---|--------|---------------------|
|
||
| 1 | 200 | `2026-02` |
|
||
| 2 | 550 | `2026-02, 2026-03` |
|
||
|
||
Total 750 = his expected fee for the two months (likely 2026-02 = 500, 2026-03 = 250 — junior tier or exception-adjusted). The app currently shows 2026-02 = **566** paid, 2026-03 = **183** paid. The user wants:
|
||
|
||
> First use the second payment for the rest of 2026-02 (no more, no less), then put the remainder toward 2026-03.
|
||
|
||
Both Python ([scripts/match_payments.py](scripts/match_payments.py)) and Go ([go/internal/domain/reconcile/reconcile.go](go/internal/domain/reconcile/reconcile.go)) have the same bug: when a single transaction's `in_window_share` is less than the sum of in-window expected fees, both fall into a **proportional** branch that splits the new transaction across months in proportion to each month's *total* expected fee — never consulting what prior transactions already paid into earlier months.
|
||
|
||
Trace for txn 2:
|
||
|
||
- `in_window = [(2026-02, 500), (2026-03, 250)]`, `total_expected = 750`, `in_window_share = 550`.
|
||
- `550 < 750` → proportional:
|
||
- 02 alloc = `550 × 500 / 750 = 366.67`
|
||
- 03 alloc = `550 − 366.67 = 183.33`
|
||
- Combined with txn 1's 200 → 02 = 566.67, 03 = 183.33 → display `566 / 183`. ✓ matches reported numbers.
|
||
|
||
The fix: replace the greedy + proportional branches with a single **fill-first** loop that iterates `in_window` in user-supplied order (already chronological by convention from [scripts/infer_payments.py:151](scripts/infer_payments.py#L151)) and allocates `min(remaining, max(0, expected − paid_so_far))` to each month, with any final surplus going to the credit bucket. This collapses three cases (greedy / proportional / pure overflow) into one and naturally consults the ledger's `paid` field which is already updated by prior transactions in the same reconcile pass.
|
||
|
||
The even-split branch (`total_expected == 0`, prepayment before fees known) stays untouched — different semantic, folding it in would silently change behavior.
|
||
|
||
## Changes
|
||
|
||
### Python — [scripts/match_payments.py:471-498](scripts/match_payments.py#L471-L498)
|
||
|
||
Replace both `total_expected > 0` branches (current lines 471–498) with a single unified loop. Keep lines 466–469 above and the even-split fallback below (lines 499–510) as-is.
|
||
|
||
```python
|
||
if total_expected > 0:
|
||
# Fill-first: iterate in_window in matched_months order (chronological by
|
||
# convention), allocate min(remaining, deficit) to each month, where
|
||
# deficit accounts for prior transactions already credited to that month.
|
||
# Any surplus after all in-window deficits are covered → credit bucket.
|
||
remaining = in_window_share
|
||
for m, exp in in_window:
|
||
paid_so_far = ledger[member_name][m]["paid"]
|
||
deficit = max(0.0, float(exp) - paid_so_far)
|
||
alloc = min(remaining, deficit)
|
||
if alloc <= 0:
|
||
continue
|
||
ledger[member_name][m]["paid"] += alloc
|
||
ledger[member_name][m]["transactions"].append({
|
||
"amount": alloc,
|
||
"date": tx["date"],
|
||
"sender": tx["sender"],
|
||
"message": tx["message"],
|
||
"confidence": confidence,
|
||
})
|
||
remaining -= alloc
|
||
if remaining > 0:
|
||
credits[member_name] = credits.get(member_name, 0) + int(remaining)
|
||
else:
|
||
# … existing even-split branch (lines 499–510) unchanged …
|
||
```
|
||
|
||
Note: skipping the `transactions.append` when `alloc <= 0` (e.g. month already fully paid by a prior txn) avoids zero-amount ghost rows in the per-month transaction list. This is a small UI-visible side effect; before committing, grep tests for assertions on `len(transactions)` per month to confirm nothing relies on the current "one row per (txn, month) regardless of alloc" behavior.
|
||
|
||
### Go — [go/internal/domain/reconcile/reconcile.go:320-357](go/internal/domain/reconcile/reconcile.go#L320-L357)
|
||
|
||
Same shape — replace both `totalExpected > 0` branches. Even-split branch (lines 358–372) stays.
|
||
|
||
```go
|
||
if totalExpected > 0 {
|
||
// Fill-first; see Python reconcile() for rationale.
|
||
remaining := inWindowShare
|
||
for _, mw := range inWindow {
|
||
md := ledger[memberName][mw.month]
|
||
deficit := float64(mw.expected) - md.Paid
|
||
if deficit < 0 {
|
||
deficit = 0
|
||
}
|
||
alloc := remaining
|
||
if deficit < alloc {
|
||
alloc = deficit
|
||
}
|
||
if alloc <= 0 {
|
||
continue
|
||
}
|
||
md.Paid += alloc
|
||
md.Transactions = append(md.Transactions, TxEntry{
|
||
Amount: alloc,
|
||
Date: tx.Date,
|
||
Sender: tx.Sender,
|
||
Message: tx.Message,
|
||
Confidence: string(m.Confidence),
|
||
})
|
||
ledger[memberName][mw.month] = md
|
||
remaining -= alloc
|
||
}
|
||
if remaining > 0 {
|
||
credits[memberName] += int(remaining)
|
||
}
|
||
} else {
|
||
// … existing even-split branch (lines 358–372) unchanged …
|
||
}
|
||
```
|
||
|
||
### Tests
|
||
|
||
#### Python — [tests/test_reconcile_exceptions.py](tests/test_reconcile_exceptions.py)
|
||
|
||
1. **Rewrite `test_proportional_underpayment`** ([line 96](tests/test_reconcile_exceptions.py#L96)) — its current assertions (`paid_02 < 750`, `paid_03 < 350`, `paid_04 < 750`, and 02/04 equal allocation) are incompatible with fill-first. Under fill-first with the same fixture (1250 across `02:750, 03:350, 04:750`):
|
||
- 02: `min(1250, 750) = 750` (full) → remaining 500
|
||
- 03: `min(500, 350) = 350` (full) → remaining 150
|
||
- 04: `min(150, 750) = 150` (partial) → remaining 0
|
||
|
||
Replace assertions with these exact expected values, rename to `test_underpayment_fills_earliest_first`.
|
||
|
||
2. **Add `test_fill_first_across_two_transactions`** — the Matyáš regression:
|
||
|
||
```python
|
||
def test_fill_first_across_two_transactions(self):
|
||
"""Prior txn fills 02 partially; later txn finishes 02 then spills to 03."""
|
||
members = [('Matyáš', 'A', {'2026-02': (500, 2), '2026-03': (250, 1)})]
|
||
sorted_months = ['2026-02', '2026-03']
|
||
tx1 = _tx('Matyáš', '2026-02', 200)
|
||
tx2 = _tx('Matyáš', '2026-02, 2026-03', 550)
|
||
|
||
result = reconcile(members, sorted_months, [tx1, tx2])
|
||
months = result['members']['Matyáš']['months']
|
||
|
||
self.assertAlmostEqual(months['2026-02']['paid'], 500, places=2)
|
||
self.assertAlmostEqual(months['2026-03']['paid'], 250, places=2)
|
||
```
|
||
|
||
3. `test_greedy_exact_match`, `test_greedy_overpayment_goes_to_credit`, `test_single_month_unchanged`, `test_two_members_multi_month` should pass unchanged — fill-first agrees with greedy when payment ≥ total expected.
|
||
|
||
#### Go — [go/internal/domain/reconcile/reconcile_test.go](go/internal/domain/reconcile/reconcile_test.go)
|
||
|
||
- Add `TestUnderpaymentFillsEarliestFirst` mirroring the rewritten Python test.
|
||
- Add `TestFillFirstAcrossTwoTransactions` mirroring the Matyáš scenario.
|
||
|
||
#### Parity — [go/tests/parity/reconcile/reconcile_parity_test.go](go/tests/parity/reconcile/reconcile_parity_test.go)
|
||
|
||
Add a fixture for the Matyáš two-transaction case. Since both implementations change together and Python remains canonical, existing parity fixtures should continue to pass; verify after edits.
|
||
|
||
### Changelog — [CHANGELOG.md](CHANGELOG.md)
|
||
|
||
Append top entry (use `date "+%Y-%m-%d %H:%M %Z"` at commit time):
|
||
|
||
```markdown
|
||
## 2026-05-11 23:55 CET — fill-first multi-month payment allocation
|
||
|
||
- Multi-month payment allocation now fills the earliest in-window deficit first
|
||
and spills the remainder to later months, accounting for prior transactions'
|
||
contributions. Previously a single transaction was split proportionally to
|
||
each month's total expected fee, ignoring earlier payments — surfaced by
|
||
Matyáš Thér's two-payment 200+550 case showing 566/183 instead of 500/250.
|
||
- scripts/match_payments.py, go/internal/domain/reconcile/reconcile.go, tests.
|
||
```
|
||
|
||
## Critical files
|
||
|
||
- [scripts/match_payments.py](scripts/match_payments.py) — Python reconcile (canonical)
|
||
- [go/internal/domain/reconcile/reconcile.go](go/internal/domain/reconcile/reconcile.go) — Go reconcile (mirrors Python)
|
||
- [tests/test_reconcile_exceptions.py](tests/test_reconcile_exceptions.py) — rewrite `test_proportional_underpayment` + add Matyáš test
|
||
- [go/internal/domain/reconcile/reconcile_test.go](go/internal/domain/reconcile/reconcile_test.go) — add Go tests
|
||
- [go/tests/parity/reconcile/reconcile_parity_test.go](go/tests/parity/reconcile/reconcile_parity_test.go) — add parity fixture
|
||
- [CHANGELOG.md](CHANGELOG.md) — top entry
|
||
|
||
## Verification
|
||
|
||
1. `make test` — Python unit tests including the rewritten + new fill-first tests.
|
||
2. `cd go && go test ./internal/domain/reconcile/... ./tests/parity/reconcile/...` — Go unit + parity tests.
|
||
3. `make web` → load `/adults` or `/juniors` (whichever lists Matyáš Thér) → his 2026-02 row should be fully paid (no shortfall), 2026-03 fully paid, no leftover credit.
|
||
4. Spot-check one other multi-month-purpose member to make sure fully-covered cases still look right.
|
||
|
||
## Branch & MR (per CLAUDE.md)
|
||
|
||
1. `git checkout -b fix/fill-first-multi-month-allocation`
|
||
2. Apply edits + tests + CHANGELOG entry.
|
||
3. `make test && (cd go && go test ./...)` — both green.
|
||
4. Commit: `fix(reconcile): fill earliest month deficit first in multi-month allocations` with `Co-Authored-By` trailer.
|
||
5. `git push -u origin fix/fill-first-multi-month-allocation`
|
||
6. `tea pr create --title "fix(reconcile): fill earliest month deficit first" --description "<body>" --base main --head fix/fill-first-multi-month-allocation`
|
||
7. Print MR URL. Do not merge from CLI.
|