fix: balance adjustments counted as spending and income — add 'adjustment' type
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
548d640231
commit
ec21078bf8
9 files changed
+217
-29
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -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'"
|
||||
))
|
||||
+3
-2
@@ -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)
|
||||
|
||||
+1
-1
@@ -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)
|
||||
|
||||
+12
-21
@@ -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()
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
@@ -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}
|
||||
@@ -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:
|
||||
|
||||
@@ -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.
|
||||
Reference in new issue
Block a user