From ec21078bf8c78ab98227407131a70b88fbc822a6 Mon Sep 17 00:00:00 2001 From: EislM0203 Date: Wed, 30 Sep 2026 09:10:45 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20balance=20adjustments=20counted=20as=20s?= =?UTF-8?q?pending=20and=20income=20=E2=80=94=20add=20'adjustment'=20type?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/accounts/{id}/adjust-balance wrote its delta as 'expense' (down) or 'income' (up), so every downward correction surfaced as uncategorized spending in get_monthly_spending, and every upward one as income. Opening balances were written as 'income' too, kept out of the income total only by a description != 'Opening balance' filter. Backend: - adjust_balance (routers/accounts.py) and create_account (crud.py) now write type='adjustment' with the signed amount. Balances are unaffected: SUM(amount - fee) ignores type. - get_monthly_spending drops the description filter; spending and income select purely by type. - TransactionBase gains 'adjustment' for reads; TransactionCreate does not, so only the server can write one. - Migration j0e1f2a3b4c5 retypes historical rows: unlinked income/expense rows with the exact generated descriptions. Downgrade restores the typing from the sign of the amount. Tests: cli/tests/test_adjustment.py covers spending/income exclusion, the balance landing on target, and the migration + downgrade on a scratch DB. test_cli's opening-balance assertion updated to the new type. 65 passed. Docs: CLAUDE.md type list and invariant 6; spec in docs/superpowers/specs/2026-09-30-adjustment-transaction-type.md. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 6 +- ...j0e1f2a3b4c5_retype_balance_adjustments.py | 46 +++++++ api/app/crud.py | 5 +- api/app/models.py | 2 +- api/app/routers/accounts.py | 33 ++--- api/app/schemas.py | 2 +- cli/tests/test_adjustment.py | 119 ++++++++++++++++++ cli/tests/test_cli.py | 2 +- .../2026-09-30-adjustment-transaction-type.md | 31 +++++ 9 files changed, 217 insertions(+), 29 deletions(-) create mode 100644 api/alembic/versions/j0e1f2a3b4c5_retype_balance_adjustments.py create mode 100644 cli/tests/test_adjustment.py create mode 100644 docs/superpowers/specs/2026-09-30-adjustment-transaction-type.md diff --git a/CLAUDE.md b/CLAUDE.md index e11bf74..b96b12a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -58,7 +58,7 @@ cli/ - **Name resolution**: `find_account` / `find_category` map a name to the integer id every other tool takes (exact match wins, else substring). Accounts and categories are also published as the resources `fin://accounts` and `fin://categories`. There is no `get_openapi_spec` tool — the tools already cover the API and the spec is too large to be worth a tool slot. - **Transport**: `FIN_MCP_TRANSPORT=stdio` (default, for clients that spawn the process) or `streamable-http`, which binds `FIN_MCP_HOST:FIN_MCP_PORT` and serves MCP at `FIN_MCP_PATH` (default `/mcp`). HTTP mode sets `stateless_http` so no session affinity is required. - **Deployment**: its own image (`cli/Dockerfile`) and its own Deployment + ClusterIP Service in k8s — it is not mounted into the API app. Reference manifest: `deploy/k8s/fin-mcp.yaml`. In-cluster endpoint is `http://fin-mcp.apps.svc.cluster.local:8000/mcp`. -- **Tool params are typed**: enum-valued arguments use `Literal`, so the MCP client rejects bad values before the call. In particular `create_transaction` accepts only `expense`/`income` — the API itself accepts all six transaction types, and writing a `transfer` row through it would create an orphan leg with no `linked_account_id` (invariant #1). Two-leg writes must go through `create_transfer` / `create_settlement` / `create_split_expense` / `create_splitwise_i_paid`. +- **Tool params are typed**: enum-valued arguments use `Literal`, so the MCP client rejects bad values before the call. In particular `create_transaction` accepts only `expense`/`income` — `POST /api/transactions` also accepts `splitwise_pay`/`splitwise_receive`, and rejects `transfer`/`settlement`, which would create an orphan leg with no `linked_account_id` (invariant #1). Two-leg writes must go through `create_transfer` / `create_settlement` / `create_split_expense` / `create_splitwise_i_paid`. **Security**: `fin-mcp` has no auth of its own — it inherits whatever `FinClient` uses. With writes enabled it must stay unreachable from outside the cluster (no HTTPRoute) unless `FIN_API_TOKEN` is set on both the API and the MCP server. @@ -81,7 +81,7 @@ cli/ `Account` is the top-level entity with a `type` field: `bank`, `investment`, `rsu`, `splitwise`, or `receivable`. Bank accounts track cash via `Transaction` rows. Investment and RSU accounts track equity via `Holding` rows (one per ticker, unique on `account_id + ticker`). A Splitwise account records shared-expense balances. -`Transaction` belongs to one account and has a `type`: `expense`, `income`, `transfer`, `splitwise_pay`, `splitwise_receive`, or `settlement`. Transfers and settlements always produce two linked rows — a debit on one account and a credit on another — connected via `linked_account_id`. The `fee` column is included in balance calculations (`amount - fee`). For Splitwise pay transactions, `splitwise_partner_share` records the partner's portion (used in spending calculations to show only the user's net share). +`Transaction` belongs to one account and has a `type`: `expense`, `income`, `transfer`, `splitwise_pay`, `splitwise_receive`, `settlement`, or `adjustment`. `adjustment` is a single-leg correction written only by the server — opening balances on account creation and `POST /api/accounts/{id}/adjust-balance`. It moves the balance but is excluded from spending and income, because it is neither consumption nor earnings. Transfers and settlements always produce two linked rows — a debit on one account and a credit on another — connected via `linked_account_id`. The `fee` column is included in balance calculations (`amount - fee`). For Splitwise pay transactions, `splitwise_partner_share` records the partner's portion (used in spending calculations to show only the user's net share). **Idempotency**: every write that moves money accepts an optional `client_ref` — `POST /api/transactions`, `/batch`, `/transfer` and `/settlement`. Every row of one write carries the same key, and the creators check `crud.find_by_client_ref` before writing, so retrying a timed-out request replays the original rows instead of double-booking. The index is deliberately **not** unique (a transfer writes two rows under one key); duplicate suppression is a check-then-write, which is sound because SQLite serialises writers and the scheduler never writes transactions. A NULL `client_ref` means "no idempotency requested" and never collides. @@ -98,7 +98,7 @@ cli/ 3. **WAL mode**: `PRAGMA journal_mode=WAL` is applied on every new connection (both the main engine and the scheduler's private engine). The scheduler runs in a background thread and must not share the request-scoped session. 4. **Non-root Docker user**: The API Dockerfile creates user `app` (uid 1000) and runs as that user. Do not revert to root. 5. **Balance formula**: For bank/splitwise/receivable accounts, balance = `SUM(amount - fee)` over all transactions. Do not sum `amount` alone. -6. **Spending logic**: `splitwise_pay` expenses are counted at `amount + splitwise_partner_share` (the user's net share), not the full `amount`. This is intentional — see `get_monthly_spending` in crud.py. +6. **Spending logic**: `splitwise_pay` expenses are counted at `amount + splitwise_partner_share` (the user's net share), not the full `amount`. This is intentional — see `get_monthly_spending` in crud.py. Spending and income select rows by `type`, never by description: anything that is not real consumption or earnings (transfers, settlements, adjustments) gets its own type so those queries skip it. ## What NOT to do diff --git a/api/alembic/versions/j0e1f2a3b4c5_retype_balance_adjustments.py b/api/alembic/versions/j0e1f2a3b4c5_retype_balance_adjustments.py new file mode 100644 index 0000000..49a9467 --- /dev/null +++ b/api/alembic/versions/j0e1f2a3b4c5_retype_balance_adjustments.py @@ -0,0 +1,46 @@ +"""retype balance adjustments and opening balances as 'adjustment' + +Revision ID: j0e1f2a3b4c5 +Revises: i9d0e1f2a3b4 +Create Date: 2026-09-30 + +POST /api/accounts/{id}/adjust-balance wrote its correction row as 'income' +(delta > 0) or 'expense' (delta < 0), so every downward correction showed up +as uncategorized spending and every upward one as income. Account creation +wrote the opening balance as 'income' too, kept out of the income total only +by a description filter. Neither is earnings or consumption — both are +corrections to the ledger. New rows are written as 'adjustment'; this +migration retypes the existing rows the same way. + +Matched on the exact generated descriptions, only on unlinked income/expense +rows. Balances are unaffected: SUM(amount - fee) ignores type. + +Downgrade restores the old typing from the sign of the amount, which is exactly +how the rows were originally typed. +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +revision: str = "j0e1f2a3b4c5" +down_revision: Union[str, None] = "i9d0e1f2a3b4" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.get_bind().execute(sa.text( + "UPDATE transactions SET type = 'adjustment' " + "WHERE type IN ('income', 'expense') " + "AND linked_account_id IS NULL " + "AND description IN ('Balance adjustment', 'Opening balance')" + )) + + +def downgrade() -> None: + op.get_bind().execute(sa.text( + "UPDATE transactions " + "SET type = CASE WHEN amount < 0 THEN 'expense' ELSE 'income' END " + "WHERE type = 'adjustment'" + )) diff --git a/api/app/crud.py b/api/app/crud.py index 5702190..69c3223 100644 --- a/api/app/crud.py +++ b/api/app/crud.py @@ -31,9 +31,11 @@ def create_account(db: Session, account: schemas.AccountCreate) -> models.Accoun db.add(db_account) db.flush() # assign the id for the opening transaction if initial and initial > 0: + # 'adjustment', not 'income': an opening balance is money that was + # already there, not money earned this month. opening = models.Transaction( account_id=db_account.id, - type="income", + type="adjustment", amount=initial, fee=0, currency=db_account.currency, @@ -799,7 +801,6 @@ def get_monthly_spending( models.Transaction.date < _end, models.Transaction.type == 'income', models.Account.type.notin_(["payable", "receivable"]), - models.Transaction.description != 'Opening balance', ) if account_id is not None: income_q = income_q.filter(models.Transaction.account_id == account_id) diff --git a/api/app/models.py b/api/app/models.py index 4e24e56..2d65da9 100644 --- a/api/app/models.py +++ b/api/app/models.py @@ -60,7 +60,7 @@ class Transaction(Base): account_id = Column(Integer, ForeignKey("accounts.id"), nullable=False) amount = Column(Float, nullable=False) currency = Column(Text, nullable=False, default="EUR") - type = Column(Text, nullable=False) # expense|income|transfer|splitwise_pay|splitwise_receive|settlement + type = Column(Text, nullable=False) # expense|income|transfer|splitwise_pay|splitwise_receive|settlement|adjustment category_id = Column(Integer, ForeignKey("categories.id"), nullable=True) description = Column(Text, nullable=True) fee = Column(Float, nullable=False, default=0) diff --git a/api/app/routers/accounts.py b/api/app/routers/accounts.py index 8dbfe0a..e4e7098 100644 --- a/api/app/routers/accounts.py +++ b/api/app/routers/accounts.py @@ -62,27 +62,18 @@ def adjust_balance(account_id: int, req: BalanceAdjustRequest, db: Session = Dep if abs(delta) < 0.001: raise HTTPException(status_code=422, detail="Balance already matches target") - if delta > 0: - txn = models.Transaction( - account_id=account_id, - type="income", - amount=delta, - fee=0, - currency=account.currency, - description="Balance adjustment", - date=_date.today(), - ) - else: - # Expense: store as negative amount so SUM(amount - fee) reduces the balance - txn = models.Transaction( - account_id=account_id, - type="expense", - amount=delta, # negative, matches the sign convention used throughout crud.py - fee=0, - currency=account.currency, - description="Balance adjustment", - date=_date.today(), - ) + # Typed 'adjustment', not income/expense: a correction to the ledger is + # neither earnings nor consumption, so spending and income must not see it. + # The signed delta still moves SUM(amount - fee) onto the target. + txn = models.Transaction( + account_id=account_id, + type="adjustment", + amount=delta, + fee=0, + currency=account.currency, + description="Balance adjustment", + date=_date.today(), + ) db.add(txn) db.commit() diff --git a/api/app/schemas.py b/api/app/schemas.py index 311e4cd..1698d14 100644 --- a/api/app/schemas.py +++ b/api/app/schemas.py @@ -74,7 +74,7 @@ class TransactionBase(BaseModel): account_id: int amount: float currency: str = "EUR" - type: Literal["expense", "income", "transfer", "splitwise_pay", "splitwise_receive", "settlement"] + type: Literal["expense", "income", "transfer", "splitwise_pay", "splitwise_receive", "settlement", "adjustment"] category_id: Optional[int] = None description: Optional[str] = Field(default=None, max_length=500) fee: float = 0 diff --git a/cli/tests/test_adjustment.py b/cli/tests/test_adjustment.py new file mode 100644 index 0000000..9294ab6 --- /dev/null +++ b/cli/tests/test_adjustment.py @@ -0,0 +1,119 @@ +"""Balance corrections — the 'adjustment' transaction type. + +A balance adjustment or opening balance moves the account balance but is +neither earnings nor consumption, so it must stay out of the spending and +income totals. Before this type existed, a downward adjustment was written as +an 'expense' and surfaced as uncategorized spending. +""" + +import os +import sqlite3 +import subprocess +import sys +from datetime import date +from pathlib import Path + +API_DIR = Path(__file__).resolve().parents[2] / "api" + + +def _bank(client, **extra): + return client.create_account({"name": "Checking", "type": "bank", "currency": "EUR", **extra}) + + +def _balance(client, account_id): + return sum(t["amount"] - t["fee"] for t in client.list_transactions(account_id=account_id)) + + +def _month(): + return date.today().strftime("%Y-%m") + + +class TestAdjustBalance: + def test_downward_adjustment_is_not_spending(self, client): + acct = _bank(client, initial_balance=1000) + + txn = client.adjust_account_balance(acct["id"], 700) + + assert txn["type"] == "adjustment" + assert txn["amount"] == -300 + assert _balance(client, acct["id"]) == 700 + spending = client.get_spending_month(_month()) + assert spending["total"] == 0 + assert spending["categories"] == [] + + def test_upward_adjustment_is_not_income(self, client): + acct = _bank(client) + + txn = client.adjust_account_balance(acct["id"], 250) + + assert txn["type"] == "adjustment" + assert _balance(client, acct["id"]) == 250 + assert client.get_spending_month(_month())["income_total"] == 0 + + def test_real_spending_still_counts_next_to_an_adjustment(self, client): + acct = _bank(client, initial_balance=1000) + client.create_transaction({ + "account_id": acct["id"], "date": date.today().isoformat(), + "amount": -40, "type": "expense", "description": "Rewe", + }) + + client.adjust_account_balance(acct["id"], 900) + + assert client.get_spending_month(_month())["total"] == 40 + assert _balance(client, acct["id"]) == 900 + + +class TestOpeningBalance: + def test_opening_balance_is_an_adjustment_and_not_income(self, client): + acct = _bank(client, initial_balance=500) + + [opening] = client.list_transactions(account_id=acct["id"]) + assert opening["type"] == "adjustment" + assert opening["description"] == "Opening balance" + assert client.get_spending_month(_month())["income_total"] == 0 + + +def _alembic(db_path, *args): + env = {**os.environ, "DATABASE_URL": f"sqlite:///{db_path}"} + subprocess.run( + [sys.executable, "-m", "alembic", *args], + cwd=API_DIR, env=env, check=True, capture_output=True, + ) + + +class TestMigration: + def test_retypes_generated_rows_only_and_downgrades_by_sign(self, tmp_path): + db = tmp_path / "mig.db" + _alembic(db, "upgrade", "i9d0e1f2a3b4") + conn = sqlite3.connect(db) + conn.execute( + "INSERT INTO accounts (id, name, type, currency, is_active) " + "VALUES (1, 'Checking', 'bank', 'EUR', 1), (2, 'Savings', 'bank', 'EUR', 1)" + ) + rows = [ + # id, type, amount, description, linked_account_id + (1, "income", 500, "Opening balance", None), + (2, "expense", -300, "Balance adjustment", None), + (3, "income", 20, "Balance adjustment", None), + (4, "expense", -40, "Rewe", None), + (5, "income", 2000, "Salary", None), + (6, "transfer", -100, "Balance adjustment", 2), + ] + conn.executemany( + "INSERT INTO transactions (id, account_id, type, amount, fee, currency, description, " + "linked_account_id, date) VALUES (?, 1, ?, ?, 0, 'EUR', ?, ?, '2026-09-01')", + rows, + ) + conn.commit() + conn.close() + + _alembic(db, "upgrade", "head") + types = dict(sqlite3.connect(db).execute("SELECT id, type FROM transactions")) + assert types == { + 1: "adjustment", 2: "adjustment", 3: "adjustment", + 4: "expense", 5: "income", 6: "transfer", + } + + _alembic(db, "downgrade", "i9d0e1f2a3b4") + types = dict(sqlite3.connect(db).execute("SELECT id, type FROM transactions")) + assert types == {r[0]: r[1] for r in rows} diff --git a/cli/tests/test_cli.py b/cli/tests/test_cli.py index d933242..c6fadaa 100644 --- a/cli/tests/test_cli.py +++ b/cli/tests/test_cli.py @@ -75,7 +75,7 @@ class TestAccounts: txns = client.list_transactions(account_id=acct["id"]) assert len(txns) == 1 assert txns[0]["amount"] == 250.0 - assert txns[0]["type"] == "income" + assert txns[0]["type"] == "adjustment" class TestTransactions: diff --git a/docs/superpowers/specs/2026-09-30-adjustment-transaction-type.md b/docs/superpowers/specs/2026-09-30-adjustment-transaction-type.md new file mode 100644 index 0000000..988b6b8 --- /dev/null +++ b/docs/superpowers/specs/2026-09-30-adjustment-transaction-type.md @@ -0,0 +1,31 @@ +# Design: `adjustment` transaction type + +Date: 2026-09-30 + +## Overview + +1. Stop balance corrections from counting as spending or income. +2. Replace the description-string filter that kept opening balances out of income with a type. + +## 1. Balance adjustments + +`POST /api/accounts/{id}/adjust-balance` wrote its delta as `income` (delta > 0) or `expense` (delta < 0). `get_monthly_spending` counts every negative `expense` row, so a downward correction appeared as uncategorized spending, and an upward one as income. + +**Change**: the row is written as `type = 'adjustment'` with the signed delta. Balances are unaffected — `SUM(amount - fee)` in `net_worth.py` and in the adjust endpoint ignores `type`. Spending (`type IN ('expense','splitwise_receive')` / `'splitwise_pay'`) and income (`type = 'income'`) select by type, so they skip it with no query change. + +## 2. Opening balances + +`crud.create_account` wrote the opening balance as `income`, and the income query excluded it with `description != 'Opening balance'`. It is now written as `adjustment` and that filter is removed. + +## Schema + +No DDL — `transactions.type` is free `Text`. `TransactionBase.type` (the read schema) gains `adjustment`. `TransactionCreate` does **not**: adjustments are written only by the server, so `POST /api/transactions` cannot create one. + +## Migration `j0e1f2a3b4c5` + +Data-only. Retypes rows where `type IN ('income','expense')`, `linked_account_id IS NULL`, and `description IN ('Balance adjustment','Opening balance')` — the exact generated descriptions. Must not touch linked legs or user-entered rows. Downgrade restores the typing from the sign of `amount`, which is how the rows were originally typed. + +## Out of scope + +- Moving the adjust-balance write from the router into `crud.py` (architecture-contract weak point #2). +- Frontend: `Transaction.type` is a plain `string` in `client.ts`, and adjustment rows stay editable like any unlinked row.