Add income tracking Phase 2: link reimbursements to expenses - #45
Merged
Conversation
Adds a reimburses_expense_id FK on income rows, a link_income_to_expense tool for the model to set/correct that link after the fact, and shows the link in both the Income view (repays X) and ExpenseTable (Reimbursed badge). SYSTEM prompt updated to pass the linked expense id when save_income finds a confident match, and to use the new tool for corrections. NOTE: uv run pytest tests/ was not run — no local Postgres or Docker daemon available in this environment. Needs verification before merge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The prior commit (d7bdc4d) added reimbursement linking but was never run against a real Postgres instance. Running the suite here surfaced three real bugs, now fixed: - link_income_to_expense and save_income raised an uncaught ForeignKeyViolation on a nonexistent or soft-deleted expense id instead of returning a graceful error - a real crash risk since the model supplies these ids from its own (fallible) lookups. - The CSV export crashed on every request: get_expenses() now returns a reimbursed field that wasn't in the export's fixed column list. - The desktop table nested the "Reimbursed"/"repays X" text inside the same element as the description, breaking exact-text Playwright selectors other specs rely on to find a row by its description. Added backend tests for the two FK-validation paths, a seeded reimbursed expense/income pair, and reimbursement.spec.js covering the badge and "repays" note on both viewports. Full suite verified: 141 backend tests, 66 e2e tests, lint, and build all green.
Use add_income's own return value for income_id instead of a throwaway assignment followed by a separate get_income() lookup.
4 tasks
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
Phase 2 of income tracking (Phase 1, merged in #43, shipped add-via-chat + read-only list). This adds the piece deliberately deferred from Phase 1: linking a Reimbursement income row to the specific expense it repays.
reimburses_expense_idFK onincomerows, alink_income_to_expensetool for the model to set/correct that link after the fact, and areimbursedflag onget_expenses()rows (via a correlatedEXISTSsubquery).save_incomenow acceptsreimburses_expense_iddirectly when a confident match is found.SYSTEMnow tells the model to searchget_expensesfor a matching expense when it sees a reimbursement-shaped income line and pass that expense's id onsave_income, and to uselink_income_to_expenseto fix a wrong or missing link after the fact — income and expense ids are called out explicitly as separate, collidable sequences (same caution added in Phase 1 for edit/delete).Review
This branch's two commits were built in two different sessions. The first (
d7bdc4d) implemented the feature but — per its own commit message — was never run against a real Postgres instance (no local DB available in that environment). The second commit (1c8ede0, this session) ran the full suite for the first time and found three real bugs that testing would have caught immediately:link_income_to_expenseandsave_incomeraised an uncaughtForeignKeyViolationon a nonexistent or soft-deleted expense id instead of failing gracefully — a real risk since the model supplies these ids from its own lookups, which can be wrong. Fixed with an existence check that returns{"status": "error", ...}.get_expenses()now returns areimbursedfield that wasn't in the export's fixed column list.Also ran an independent adversarial review pass (fresh-context agent, no sight of the fixing session's own reasoning) against the fix commit specifically. It confirmed: no exploitable race in the new existence-check-then-mutate pattern (this app never hard-deletes expense rows, so the check is effectively atomic in practice), the CSV boolean rendering matches existing precedent (
flaggedwas already handled the same way), both desktop-table spots got the span-wrapping fix (mobile was already fine), the three new tests fail if the fix is reverted (not false-positive), and the new seed data doesn't destabilize any other spec's assumptions. It flagged one thing as worth noting but out of scope: reimbursement linking isn't scoped per-user — consistent with (not a regression from) this app's existing shared-household design, where all expense/income data is already visible and editable by any household member.Test plan
uv run pytest tests/— 141 passed (includes 3 new tests for the FK-validation fix)npm run lint/npm run build— cleannpx playwright test e2e/reimbursement.spec.js— 4/4 (badge + repays note, both viewports)npm run test:e2e— 66/66, no regressions from the new seeded reimbursement pair or the markup fixRoadmap / future work
Tracked as a follow-up (Phase 3), deliberately out of scope here:
update_income/delete_income— income entries still can't be edited or deleted, only relinked.Generated by Claude Code