feat(loans): mortgage tracking that splits payments into interest and capital - #8
Merged
Merged
Conversation
Models a mortgage as a first-class Loan so each payment lands in the ledger as deductible interest (SA105 box 44) plus non-deductible capital repayment, instead of one lump the user splits by hand. Prevents a real overclaim: a £750/month direct debit against a £71k interest-only mortgage at 2.7% has only ~£160/month of deductible interest. No new tax rate logic — the finance/capital category kinds, the Section 24 reducer and the 2027/28 22% surcharge are all already wired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
17 tasks, 74 TDD steps. Phase order puts the schema, pure split maths and the box-44 regression test first, so the overclaim is provably impossible at the engine level before any UI exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the database layer for first-class mortgage/loan tracking: Loan (the mortgage/loan definition with an interval-based repayment schedule, mirroring RecurringRule) and LoanPayment (one row per scheduled payment, storing computed vs actual interest so later tasks can split capital from deductible interest). Transaction gains an optional loanPaymentId FK (ON DELETE SET NULL) so ledger rows survive if a loan payment is deleted. Names the Transaction.loanPaymentId FK (Transaction_loanPaymentId_fkey) to match every other FK in the schema, using SQLite's column-level CONSTRAINT ... REFERENCES form since a table-level named FK clause is not valid inside ADD COLUMN. Realigns the Transaction model's fields (loanPaymentId is the longest field name in that block, so the existing column widths had to widen) and notes on LoanPayment that closingBalancePence must only be written by derivePayment().
- resetDb must delete LoanPayment/Loan between Transaction and Property (Loan->Property is ON DELETE RESTRICT, so order matters); without this loan rows leak across test files. - Task 7's test was a pure function mirroring the generation loop. This repo has a seeded test.db harness and existing data-layer tests hit Prisma, so replaced it with a real integration test - the mirror would have verified a duplicate of the logic and missed txn pairing, idempotency and category wiring. - categories.ts now pins the expected CategoryKind alongside each name, with a test asserting the seeded kinds. A rename would otherwise mis-file money silently rather than failing the build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Also fixes src/lib/data/categories.test.ts, whose hardcoded seeded-category count (11) was invalidated by the new seed row (now 12).
- prisma/seed.ts now imports the shared constants instead of re-hardcoding the literals, so the drift-prevention comment in categories.ts is actually true instead of just detected by a test. - categories.test.ts: replace the weak literal-vs-constant assertion with a check that capital repayment and capital improvements are genuinely distinct rows (different ids) -- the failure mode that could actually happen is the seed collapsing them onto one category. - resetDb.ts: document the FK ordering invariant so the next person extending the delete list doesn't have to re-derive it by hand.
The constants file claimed it prevented seed/code drift "by construction", but Task 2 as written told the seed to hardcode the same literals - so drift was only detected by a test, not prevented. Verified viable: scripts/ already import from src/ under tsx, and tax/types.ts is import-free. Also pinned the final gate to expect exactly one pre-existing failure (scripts/migrate-akaunting/apply.test.ts, missing akaunting-migration/ dir) so a real regression cannot hide behind it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of Tasks 3-4 flagged the positional signature as the one thing to fix
before Task 7 wires it into a loop: three adjacent number params, two of them
pence of similar magnitude. TypeScript cannot catch a transposition between
same-typed positional args, and the failure mode is a silently wrong SA105
figure - the exact bug this feature exists to prevent.
Also: LoanKind moves to src/lib/tax/types.ts beside the existing hand-rolled
CategoryKind; thrown messages drop raw pence because server actions surface
Error.message to users verbatim via ?error= ("16281p" not "£162.81"); and the
negative-balance case now throws instead of being clamped to zero by Math.max,
which would have hidden a desynced balance chain.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ivePayment Address review feedback on Tasks 3-4: switch derivePayment to a single options object (positional same-typed pence args were transposition-prone), throw on a negative opening balance instead of silently clamping it, strip raw pence from the thrown message (server actions surface Error.message verbatim to users), move LoanKind next to CategoryKind in tax/types.ts, document paidPence vs scheduledPaymentPence, and add tests for the negative guards, the override-triggered redemption clamp, and the repayment-loan override.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…inated union Review found the "excludes capital from profit" test proved nothing: computeProfit ignores capital rows for two independent reasons (allowable: false, and no "capital" arm in its categoryKind switch), so deleting those rows entirely would give byte-identical results. Added a guard that the rows are present and material, plus a mutation test that recategorises capital as a deductible expense and asserts profit moves by exactly that amount - which is what actually proves the categorisation is load-bearing. LoanAdvice is now discriminated on reliefBasis. It previously always populated effectiveRateBps, holding the *nominal* rate for company-owned property with a comment telling the UI not to label it "after relief". A comment is too weak for a financial disclosure surface; narrowing now enforces the branch. Also documented why the term-end warning is deliberately suppressed for repayment loans rather than that being an artefact of the kind gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…vice on reliefBasis - schedule.test.ts: the old "excludes capital repayment" test passed even with capital rows deleted entirely (computeProfit ignores them for two independent reasons). Replaced with a guarded version plus a mutation test that recategorises capital as an expense and asserts profit moves by exactly that amount. - advice.ts: LoanAdvice is now a discriminated union on reliefBasis so a caller destructuring effectiveRateBps for a company-owned loan gets null, not a plausible-looking nominal rate a comment merely warned against relying on. - advice.ts: documented why the term-end warning is gated to interestOnly loans (repayment balances amortise to zero by design; detecting underpayment there needs a term-end projection, out of scope). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…k the model - Regression assertions become exact (182,900p box 44; 717,100p overclaim). Money here is deterministic integer pence with no floating point, so a range bought no robustness and would let a real rate drift pass - in the one test named for pinning box 44. - The "20% + surcharge" reducer rule is now duplicated between advice.ts and summary.ts:37. Extract propertyReducerBps(taxYear, region) into bands.ts beside propertySurchargeBps and call it from both. - Task 7 gains a cross-check test: yearOfSplitPayments() reimplements the balance-carry-forward loop in memory, so without it the regression suite could keep passing against its own copy while the real generator broke. - The SA105 regression suite moves to its own file for discoverability. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…box-44 pins; split SA105 regression suite - tax/bands.ts: add propertyReducerBps(taxYear, region) = 20% + propertySurchargeBps, wrapping the surcharge lookup so a future change to the 20% base lands in one place. - tax/summary.ts, loans/advice.ts: both call propertyReducerBps instead of duplicating the "2000 + propertySurchargeBps(...)" literal. Pure refactor — no behaviour change. - loans/advice.ts: trim the duplicated capitalNoReliefPence doc comment on LoanAdviceCommon down to a pointer at the canonical one on LoanAdviceInput. - loans/sa105Regression.test.ts (new): the SA105 regression suite (yearOfSplitPayments, RENT fixture, and its 4 tests) moved out of schedule.test.ts, which was mixing unit tests for daysBetween/computeInterestPence/derivePayment with an integration-style suite that imports the tax engine. Box-44 and overclaim assertions are now exact pence figures (182_900 / 717_100) rather than ranges, since this path is fully deterministic integer arithmetic and a range would let a real rate drift pass silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Task 7's specified code and its own test fixtures contradicted each other, and the implementer correctly escalated instead of picking a side. recurringOccurrences() emits startDate itself when the schedule's day-of-month matches it. That is right for a RecurringRule - rent starting on the 1st should pay on the 1st - but a Loan's startDate is the accrual ORIGIN, since openingBalancePence is defined as the balance as at that date. Passing it through unchanged produced a phantom payment with zero elapsed days, zero interest and the entire amount booked as capital, one extra payment before every expected one. Fixed by seeding lastGeneratedDate with startDate, which reuses that function's existing "skip anything on or before" rule to mean "first payment strictly after the origin". No change to the shared occurrences.ts. Verified: null gives [04-01, 05-01, 06-01]; seeded gives [05-01, 06-01], and exactly 12 payments over a year. Also removed a doc comment that described the phantom-payment behaviour as if it were intended. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second escalation from the Task 7 implementer, also a genuine plan defect. The test asserted interest falls every payment. Under daily accrual it does not: a 31-day period adds ~3.3% of accrual, outweighing the ~0.8% the balance fell, so 1 Jun costs MORE than 1 May (£161.46 vs £157.56) even though £592 of capital was repaid in between. Verified the real sequence zigzags with month length: 1 May 30d £157.56 | 1 Jun 31d £161.46 | 1 Jul 30d £154.94 | 1 Aug 31d £158.74 Now asserts what is actually invariant - the balance falls every period and the chain is unbroken - and compares two equal-length periods (1 May and 1 Jul, both 30 days) for the interest trend, with the three figures pinned exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Seeds toLoanOccurrenceRule's lastGeneratedDate with the loan's own startDate so recurringOccurrences() treats startDate as the accrual origin rather than a payment date itself — otherwise a loan whose schedule day matches its startDate (the common case) produces a phantom zero-day, zero-interest, all-capital first payment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds setActualInterest to substitute the lender's real interest figure for a payment (rewriting its ledger rows), reconcileYear/ applyYearVariance to true up a tax year's box 44 total against the lender's annual statement by adjusting only the year's final payment, and capitalNoReliefPence for the non-deductible total. Deliberate limitation: overriding a mid-year payment does not recalculate later payments' opening balances. applyYearVariance targets the year's final payment specifically to keep the staleness contained rather than attempting cascading recalculation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review found setActualInterest, reconcileYear, applyYearVariance and capitalNoReliefPence shipped with zero coverage - a plan defect, on the code path that puts the lender's actual figure into SA105 box 44. Added 12 tests covering: the computed-snapshot surviving an override (the trap where derivePayment returns computedInterestPence: 0 for daysInPeriod: 0 and could overwrite the audit trail), the ledger rewrite scoping to one payment, the year total preferring actual over computed, applyYearVariance landing the whole delta on the final payment and no-opping on zero, the guard surfacing when a variance exceeds one payment, the half-open tax-year boundary so a 6 April payment counts in one year only, and the documented staleness limitation pinned explicitly so it stays deliberate. All figures computed independently first: 15,756 / 16,146 / 15,494 interest, 47,396 total, 177,604 capital, 18,098 after a variance to 50,000. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e, capitalNoReliefPence Task 8 shipped with its four functions untested. Adds the 12 tests from the plan's Task 8 Step 2 (now 13, per the actual code block — a harmless miscount in the summary), against the real Prisma-backed £71k/2.7%/£750pm fixture generated to three payments (1 May/1 Jun/1 Jul 2026). Notably verifies the trap where setActualInterest calls derivePayment with daysInPeriod: 0 (irrelevant, since the override supplies the interest directly): the update payload must omit computedInterestPence so the rate-derived audit-trail snapshot survives the override untouched. Also pins the documented staleness limitation (overriding one payment does not recalculate later opening balances) and the half-open tax-year boundary around a 6 April payment, so neither regresses silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tion.ts loans.ts reached 300 lines against a 122-line max everywhere else in src/lib/data/, holding three separable concerns. Splitting before the UI imports it, so downstream tasks import only what they need. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
loans.ts held CRUD, payment generation, and year-end reconcile in one 300-line file, against a 122-line max everywhere else in src/lib/data/. Move generateLoanPayments, setActualInterest, reconcileYear, applyYearVariance, capitalNoReliefPence and their private helpers into a new loanGeneration.ts, leaving loans.ts as CRUD-only. Also tighten toLoanOccurrenceRule/termsOf to accept Prisma's Loan model type instead of bare-string structural params with `as` casts, so a future enum rename or addition fails at compile time. Reword the loan category lookup error so it no longer tells a non-technical self-hosted user to run a CLI command. Pure refactor — all 23 tests in loans.test.ts pass unmodified (only their imports changed to pull from both modules). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the loans overview page (active-property scoped, pause/resume/delete actions) and the new-loan form page, so landlords can register a mortgage and start splitting payments into deductible interest vs non-deductible capital. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the per-loan detail page: balance, rate (branched on relief basis so company-owned properties aren't mislabelled with a Section 24 "effective rate"), this year's non-deductible capital, an interest-only term-end warning, and the payment schedule with per-row actual-interest override. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the loan edit form and the "check against my statement" page, which lets a landlord match Quidly's recorded interest to their lender's annual statement by adjusting the final payment of the tax year — keeping the SA105 box 44 figure tied to the number HMRC would check against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The original targeted src/app/(app)/import/review/page.tsx and assumed a per-row category select. Neither exists - I flagged this as the one task with an unread dependency when writing the plan, and it was wrong in two ways: - There is no import/review/ route. The flow is one page with three search-param-driven steps. The review/ dir that misled me is scan/review, for receipt scanning. - Import applies ONE category to every row, so there is no per-row control. That makes the warning more important: a £750 mortgage debit would be filed whole under whatever single category the user picked. Rewritten against PreviewRow and the step-3 preview table's Status column, with a Banner summary so the warning is visible without scanning every row. confirmImportAction stays unchanged - the flag is advisory, since silently dropping rows the user asked to import would be worse than warning them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Caught by the Task 15-16 implementer: the self-review section still described import/review/page.tsx as merely unread, after the Task 16 section had been rewritten because that route does not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add the loans feature bullet to the README, refresh the stale test-count badge and local-dev test count, and add a manual verification checklist for the live browser walkthrough deferred from Task 17. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every per-task review passed; these are integration gaps only a holistic pass could see. The critical one: the CSV import warning is cosmetic. confirmImportAction never consults the matcher, so a flagged mortgage payment imports as one fully-deductible line. The feature's central goal was one click from being defeated, via a path the feature itself added. The spec promised the review step would OFFER a split. "A suggestion, never automatic" meant don't split without asking - not provide no way to avoid the mistake. Fixing by making the safe path default with an explicit opt-in. Also: the promised guard on deleting loan-generated transactions was never implemented; deleting a property with a zero-payment loan leaks a raw Prisma FK error because getPropertyCounts doesn't count loans; and the README still claims single-year 2025-26 support. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
confirmImportAction re-parsed rows for import but never re-ran matchLoanPayment, so a row the preview flagged as a probable mortgage payment was imported anyway as one fully-deductible line -- the exact box 44 overclaim this feature exists to prevent. Factor the decision into a pure selectRowsToImport() helper (skip duplicates always, skip loan matches unless includeLoanMatches=true) and wire it into the action. The step-3 form now offers an explicit opt-in checkbox and the submit button's count never overstates what will actually be imported.
deleteTransactionAction deleted unconditionally, so removing a transaction that came from a loan payment desynced the ledger from the LoanPayment audit trail and broke the loan detail page, reconcileYear and capitalNoReliefPence. Add loanDeleteBlockMessage() and have the action refuse and redirect with a message naming the loan when loanPaymentId is set.
getPropertyCounts only checked transactions and recurring rules, so a property with a loan that had zero recorded payments passed deletePropertyIfEmpty's friendly guard and then hit Loan.propertyId's ON DELETE RESTRICT, surfacing a raw Prisma FK-violation string to the user. Count loans too and include them in the guard and its message.
The caveat claimed Quidly was configured only for 2025-26 with other years falling back to it. CONFIGURED_TAX_YEARS actually covers 2025-26 through 2027-28, and bands.ts carries the Finance Act 2026 property-rate surcharge that raises the Section 24 reducer to 22% for England/Wales/NI from 2027-28 (Scotland's 2027-28 figures are provisional pending the Scottish Budget). State the real range and the rate change, since the feature now surfaces it.
WarlaxZ
force-pushed
the
feat/mortgage-loan-tracking
branch
from
August 1, 2026 14:34
12476c4 to
e4ec361
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Models a mortgage as a first-class
Loanso every payment is recorded as its two correct parts — deductible interest (SA105 box 44) and non-deductible capital repayment — instead of one lump the user has to split by hand.The bug this prevents. A landlord with a single £750/month direct debit against a £71,000 interest-only mortgage at 2.7% has only £1,829.00/year of deductible interest out of £9,000 paid. The other £7,171.00 is capital repayment and attracts no relief whatsoever. Nothing previously stopped that whole £9,000 being filed in box 44 — roughly a £7,000/year overclaim on one property.
No new tax-rate logic was needed: the
finance/capitalcategory kinds, the Section 24 reducer, and the Finance Act 2026 rise from 20% to 22% for 2027-28 were already in the engine. This adds the bookkeeping structure that feeds it correct inputs.What's included
Loan/LoanPaymentmodels with a schedule that reuses the existingrecurringOccurrences()walker.days/365, notrate/12) — so February is genuinely cheaper, and a 31-day period correctly costs more than a preceding 30-day one even as the balance falls.Deliberate non-goals
No BIM45700 capital-account tracking (HMRC rewrote that guidance on 2026-07-01 and it is contested), no overpayment-vs-pension comparison (edges into regulated advice), no rate-history table (per-payment snapshots handle rate changes), no repayment-pot tracking, no portfolio loans spanning properties.
Test Plan
tsc --noEmitclean;npm run buildsucceeds with all five/loansroutes generated.docs/loans-manual-verification.md. Needs a running instance; not yet exercised in a browser.Rebased onto main
Rebased after #7 landed. Two README conflicts, both because #7 had independently superseded my
changes: it replaced the test-count badge with a CI badge, and had already corrected the same
stale tax-year caveat. Resolved by keeping main's version in both cases and grafting on only
the genuinely additive part — a note that the Section 24 reducer rises to 22% for
England/Wales/NI in 2027-28 while Scotland stays at 20% pending Holyrood.
Also worth noting: #7's
fix(test): create the akaunting-migration dirresolved thepre-existing failure this PR originally had to work around. The suite is now fully green —
61 files, 385 tests, zero failures and zero skips.
🤖 Generated with Claude Code