From c465742635f9f0f78372d42a62d79c10a9fc26b8 Mon Sep 17 00:00:00 2001 From: siddharthd Date: Sun, 26 Jul 2026 16:19:02 +1000 Subject: [PATCH] docs: capture this session's learnings for a future pickup CLAUDE.md gains the traps a new session would otherwise re-discover: - Rules: a zero-condition rule matches everything (rule 43 would split all ~3,700 transactions); preview-then-apply-by-id is the safe pattern and why it beats auto-applying on ingestion; how run provenance works. - Shared expenses: transaction_splits.settled is dead data; getParticipantBalances is correct and must not be 'fixed'; settlement cannot be attributed per trip. - The shared loan: separate ledger, fixed 50% with a tracked receivable, why the share must not be derived from actual payments, and why interest stays as spend. - Extraction: balance assertions are the check that works, do not derive opening_balance or add a totals assertion (both would be tautological), Gemini invents summary fields it was not given, empty statements must not throw, FX is per-date, and CSV comparisons need millisecond ordering. The design doc records Phase 0 as done - including that the original Phase 0 plan was wrong, since reading the code first is what prevented breaking a working balance page. Known Gaps lists what is open: the unbuilt phases, 11 failing assertions, the uncategorised Up rows, and the CSVs sitting in 030490e's history. --- CLAUDE.md | 138 +++++++++++++++++++++++++++++++++ docs/shared-expenses-design.md | 63 ++++++++++++--- 2 files changed, 191 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7e015f9..f18d2cb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,6 +109,49 @@ it silently deletes the unlogged remainder from spend totals. - `participants` — people; `id=1` is "Me" (the primary user) - `account_owner_mappings` — persists bank+account → owner assignments +### Shared expenses and settlement — read before touching + +The model is under active redesign. See `docs/shared-expenses-design.md` for the +proposal and what is already decided. Three traps: + +**`transaction_splits.settled` is dead data.** It is `false` on every row. Its +only writer was `/api/splits/settle`, removed in `3f04cbd` because nothing called +it and one request could mark all of a participant's splits settled. Do not build +on this flag until settlement contexts exist. + +**`getParticipantBalances` computes `splits − payments` and is correct.** Do not +"fix" it to exclude settled splits — the payments that settled them are still +subtracted, so you would double-count. The two settlement models (running tab vs +per-split flag) must not be mixed. + +**Settlement cannot be attributed per trip.** `split_payments` records only +from/to/amount/date. Any per-trip settled/unsettled figure is fabricated; the +trip view used to show one and always reported 100% unsettled. Trips show share +only, and point at `/shared` for real balances. + +Also: settlements already exist twice. Four of eight `split_payments` match an +offset-account credit exactly on amount and date, with `linked_transaction_id` +populated on only one. And Sonu's loan contributions (`…emi` in the offset +account, 39 rows, $37,980.24) are categorised `transfers`, indistinguishable from +ordinary internal transfers. + +### The shared loan + +The loan is a **separate ledger**, not a shared expense and not a settlement +context — a contribution must never be able to settle a dinner. Sonu's obligation +is a fixed 50% of the repayment; actual contributions vary, and the difference is +a tracked receivable ($4,000.00 over 2025-07 → 2026-06). + +Do not derive the share from actual payments. During her leave the obligation did +not change, only the payment did — a percentage-of-actual model would silently +redefine her share as 30% and make the shortfall vanish. + +Loan interest reconciles exactly: `repayments − interest − fees = balance +reduction`. It stays categorised `loan_interest` and counts as spend — over 12 +months $63,500 of cash left and debt fell $44,127.36, and the $16,523.64 +difference bought nothing. Excluding it would leave the balance sheet unable to +reconcile cash out against equity gained. + ### Import Date (`created_at`) `transactions.created_at` is the import timestamp (DB default `now()`). In the transactions and shared views, the "Imported" column shows: @@ -123,6 +166,80 @@ Conditions are AND-evaluated. Fields: `merchant_normalized`, `description`, `cat `contains` and `equals` operators are case-insensitive (both sides `.toLowerCase()`). +**A rule with zero conditions matches every transaction.** Both apply paths use +`conditions.length === 0 || conditions.every(...)`. Rule 43 "Home 50/50 Sonu" has +no conditions and a 50/50 split action — applying it blindly would split all +~3,700 transactions with another participant. That is what `manual_only` is for: +those rules are excluded from bulk runs and fire from the transactions page +against a hand-picked selection. + +### Previewing a rule before applying it + +`GET /api/rules/[id]/matches` is a dry run — it writes nothing and returns only +the transactions a rule would actually *change*, with already-correct rows +summarised as a count. The Preview button on the rules page uses it. + +Apply then goes through `POST /api/transactions/bulk` with `action: "apply_rule"` +and **explicit transaction ids**, not the conditions. That is the safety +property: a rule whose conditions are too broad cannot reach further than what +the preview showed and the user ticked. + +Prefer this over auto-applying rules on ingestion. It fails safe, works +retroactively, and tells you which rules are consistent enough to automate later. + +### Rule apply history + +`rule_apply_runs` snapshots the before-state so a run can be reverted, and since +migration 0017 also records `rule_id`, `rule_name` and `source` +(`all` | `rule` | `selection`). `rule_name` is denormalised deliberately and +there is no FK to `rules` — history must stay readable after a rule is renamed or +deleted, and deleting a rule must not cascade away the audit trail. + +`GET /api/rules/runs/[id]` diffs that snapshot against current values. Rows +changed by something else since the run are flagged, because reverting restores +the pre-run value and discards the later edit. + +### Trusting extracted statement data + +**Balance assertions are the check that works.** `getStatements` computes +`opening + movement − closing`; the statements page flags any statement that does +not reconcile. Sign depends on what the balance means — on a credit card or loan +it is what you owe, so spending increases it; on a transaction or offset account +it is what you hold. 11 pre-existing statements currently fail, ~$4,177 +unexplained, including two adjacent ANZ statements off by exactly ±$230.38 (a +transaction filed against the wrong one). + +**Do not derive `opening_balance` from `closing − movement`.** It is an accounting +identity, so every statement would reconcile and the check would go permanently +green. A null that reads "unverified" is worth more than a number that is right +by construction. For the same reason, do not add a totals assertion comparing +`total_debits` to the summed rows — those totals are now *computed* from the rows +(see the N8N `Parse Gemini Result` node), so that check can never fail. + +**Gemini invents summary fields the statement does not print.** Wise PDFs show +only a closing balance; asked for an opening balance anyway, the model produced +11,277.08 against a truth of 0.00, and on another statement read the running +balance of the oldest row. Every transaction was extracted perfectly in both +cases — verified row for row against the CSV exports. When a balance assertion +fails, suspect the summary before the transactions. + +**Gemini drops rows silently on long tables.** `finishReason` was `STOP`, not +`MAX_TOKENS`, so raising `maxOutputTokens` does not help. This did *not* actually +occur on the Wise imports (that was the summary bug above), but it is why an +empty statement must not throw: a document that errors never gets tagged, so it +is re-fetched every poll forever and blocks everything behind it in the queue +(`ordering=-created`, `page_size=1`). + +**FX is per transaction date**, via Frankfurter (ECB daily, free, no key), with +weekends resolving to the prior publication. A single spot rate across a 15-month +statement is wrong by up to 20%. Wise's own rates are more accurate in principle +but differ by only 0.05% and exist on 44 of 194 rows, so mixing bases is not +worth it. + +**When comparing CSV exports to extracted data, order by full timestamp +including milliseconds.** Two of one statement's rows are 1ms apart; dropping the +fraction reversed them and produced a bogus opening balance. + ## Development Patterns ### Adding a new API route @@ -226,3 +343,24 @@ break or go stale. And the views are **not** owner-scoped and do **not** merge See `README.md` → **Known Gaps / TODOs** for full details. **Payment provider tracking**: `merchant_normalized` currently conflates payment provider (PayPal, Afterpay, Zip) with the actual merchant. Plan: add `payment_provider` column, update Gemini prompt to extract it separately, backfill from `merchant_name` patterns, surface in UI filters. + +### Open as of 2026-07-26 + +- **Shared expenses redesign** — `docs/shared-expenses-design.md`. Phase 0 done; + Phases 1–4 unbuilt. Deliberately paused to live with the current behaviour + before committing to a model designed in one session. +- **11 statements fail the balance assertion**, ~$4,177 unexplained. Predates + this work. One ANZ statement is off by exactly $0.50, traced to a misread digit + in fee rows ($5.00 vs $5.50). +- **28 Up Bank debits are categorised `other`** ($3,760.74). Up only categorised + 16 of 88 rows. The `Payee` field is populated throughout, so merchant rules plus + the rule preview should clear most of it. +- **Up item sales are categorised `income`** ($4,238.04 across 19 credits — iPad, + drone, camera). Correct in that they are excluded from spend, but it mixes + asset disposals into the income line alongside salary. +- **`payment_method` is not shown in the transactions list** — settable on create + and edit only. Worth a column or filter if cash becomes routine. +- **Raw statement exports live in `dump/`**, gitignored since `31a8177`. They were + committed by accident in `030490e` and remain in that commit's history; the repo + has no GitHub remote, so exposure is limited to the local Gitea. Purging history + was offered and not actioned. diff --git a/docs/shared-expenses-design.md b/docs/shared-expenses-design.md index 9aaed01..2fde17e 100644 --- a/docs/shared-expenses-design.md +++ b/docs/shared-expenses-design.md @@ -144,16 +144,59 @@ payment did. A percentage-of-actual model would silently redefine her share as ## Migration path -1. **Fix the trip settlement bug first** — make `getTripAnalytics` and - `getParticipantBalances` agree. Low risk: all splits are currently unsettled, - so honouring the flag changes nothing today. -2. Add contexts; put every existing split in "Household"; every payment likewise. -3. Backfill `linked_transaction_id` for the four exact matches; flag the other - four for manual linking. -4. Create the "Pre-2026" closed context. Apply household split rules to - pre-cutoff transactions into it — fixes ~$97,627 of the trailing 12 months - currently shown as 100% yours. -5. Loan contributions and equity — last, and only after the questions below. +### Phase 0 — DONE (2026-07-26, commit `3f04cbd`) + +Stop the trip view reporting a settlement breakdown it cannot compute. + +The original plan was "make `getTripAnalytics` and `getParticipantBalances` +agree". **That plan was wrong** and reading the code before building is what +caught it: + +- `getParticipantBalances` is *not* buggy. It computes `splits − payments`, + which is coherent. Excluding settled splits there while still subtracting the + payments that settled them would have double-counted and broken a working page. +- The real defect was narrower: the trip view showed Settled/Unsettled from + `transaction_splits.settled`, which nothing sets. A correct per-trip figure is + not computable at all, because `split_payments` has no trip attribution. + +So the fix was **subtractive**: the trip view now shows each participant's share +and points at Shared for what is actually owed. + +Also removed `/api/splits/settle` — unreachable from the UI but live on its URL, +where one call with `participant_id` would mark every one of that person's splits +settled, writing a flag nothing reads. + +`transaction_splits.settled` / `settled_at` still exist and are now pure dead +data. Phase 1 either repurposes them ("is my context closed?") or drops them. + +### Phase 1 — settlement contexts + +Add contexts; put every existing split in "Household"; every payment likewise. +Balance queries group by context. Touches `queries.ts` (both balance CTEs), +`shared/page.tsx`, `trips/[id]/page.tsx`, `split-payments/route.ts`. ~1 day. + +### Phase 2 — link payments to transactions + +Backfill `linked_transaction_id` for the four exact matches; flag the other four +for manual linking. On ingestion, propose a matching credit as a settlement +rather than silently categorising it `transfers`. ~half a day. + +### Phase 3 — retroactive pre-2026 split + +Create the "Pre-2026" closed context. Apply household split rules into it via the +rule preview (`/api/rules/[id]/matches`, built 2026-07-26) — fixes ~$97,627 of +the trailing 12 months currently shown as 100% yours. Then delete the `splitFrom` +cutoff entirely. + +**Validate the ratio first.** This assumes today's 50/50 held through 2025. The +SplitMyExpenses CSVs should be used to *check* that assumption — not to +reconcile, since transactions were sometimes combined and exact matching is +impossible. + +### Phase 4 — loan ledger + +Contribution schedule, contributions recognised from `emi` credits, running +receivable. Independent of contexts — the loan is a separate ledger. ~1–2 days. ---