From 8627798737441982547fbab4d786df86111488f4 Mon Sep 17 00:00:00 2001 From: Daniel Roberts <29379642+danielroberts20@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:44:32 +0000 Subject: [PATCH 1/3] feat(transactions): parse and ingest CommBank PDF statements CommBank only exports PDF statements (date, details, signed amount, running balance; no currency, transaction ID or time of day). commbank.py parses the transaction table with pdfplumber, never storing the statement header (name, address, BSB, account number), and inserts rows labelled 'commbank' in AUD with an FX-converted GBP amount and a place resolved from the nearest location fix within +/-12 h (get_nearest_lat_lon_within_hours, using indexed range lookups). Adds the pdfplumber dependency. Co-Authored-By: Claude Sonnet 5.5 --- .../database/transaction/ingest/commbank.py | 287 ++++++++++++++++++ .../app/database/transaction/ingest/util.py | 38 +++ server/requirements.txt | 1 + server/tests/test_ingest_util_db.py | 20 ++ .../tests/transaction/test_commbank_insert.py | 166 ++++++++++ .../tests/transaction/test_commbank_parse.py | 118 +++++++ 6 files changed, 630 insertions(+) create mode 100644 server/app/database/transaction/ingest/commbank.py create mode 100644 server/tests/transaction/test_commbank_insert.py create mode 100644 server/tests/transaction/test_commbank_parse.py diff --git a/server/app/database/transaction/ingest/commbank.py b/server/app/database/transaction/ingest/commbank.py new file mode 100644 index 0000000..3b2693e --- /dev/null +++ b/server/app/database/transaction/ingest/commbank.py @@ -0,0 +1,287 @@ +""" +ingest/commbank.py — Parse and ingest a CommBank "Transaction Summary" PDF. + +CommBank only exports PDF statements. Each transaction starts with a line of the form + + 11 Sep 2026 Fast Transfer From Daniel James Roberts $1.87 $1.87 + +(date, details, signed amount, running balance) and may be followed by detail lines +(`Card xx0488`, `Value Date: 13/09/2026`, `PayID ...`, free-text memo). + +The statement has no currency (always AUD), no transaction ID and no time of day. +The source label is always 'commbank'. + +Privacy: the statement header (name, address, BSB, account number) is never parsed +into a row or stored in `raw` — parsing only starts at the transaction table header. +""" + +import hashlib +import io +import json +import logging +import re +from datetime import date, datetime +from decimal import Decimal + +from database.connection import get_conn, to_iso_str +from database.cost_of_living.queries import get_col_entry, get_uk_col_index +from database.exchange.fx import convert_to_gbp +from database.location.geocoding import get_place_id +from database.transaction.ingest.util import get_nearest_lat_lon_within_hours, maybe_mark_internal +from notifications import send_notification # required for tests + +logger = logging.getLogger(__name__) + +CURRENCY = "AUD" + +_MONEY = r"-?\$[\d,]+\.\d{2}" +_TX_LINE = re.compile(rf"^(\d{{2}} [A-Za-z]{{3}} \d{{4}}) (.+) ({_MONEY}) ({_MONEY})$") +_CARD_LINE = re.compile(r"^Card xx(\d{4})$") +_VALUE_DATE_LINE = re.compile(r"^Value Date: (\d{2}/\d{2}/\d{4})$") + +_TABLE_HEADER = "Date Transaction details Amount Balance" +_TABLE_END = "Any pending transactions" +# Repeated on every page — never part of a transaction +_FURNITURE_PREFIXES = ("Account Number", "Page ", "Created ", "While this letter", "we’re not responsible") +_NOISE_LINES = {"CREDIT TO ACCOUNT"} + +INTEREST_DESCRIPTION_KEYWORDS = ["interest"] + + +class CommbankParseError(ValueError): + """The PDF is not a CommBank transaction summary, or its layout has changed.""" + + +def _money_to_cents(s: str) -> int: + """'-$1,234.56' -> -123456. Integer cents keep balance checks exact.""" + negative = s.startswith("-") + return (-1 if negative else 1) * int(Decimal(s.lstrip("-").replace("$", "").replace(",", "")) * 100) + + +def _map_detail_type(description: str, has_card: bool, amount_cents: int) -> str: + """Map a CommBank description to a normalised transaction_detail value.""" + desc = description.lower() + if desc.startswith("wdl atm"): + return "ATM" + if "transfer" in desc: + return "TRANSFER" + if "interest" in desc: + return "INTEREST" + if has_card or amount_cents < 0: + return "CARD_PAYMENT" + return "DEPOSIT" + + +def _transfer_party(description: str) -> tuple[str | None, str | None]: + """Return (payer, payee) for 'Fast Transfer From X' / 'Transfer To X' descriptions.""" + m = re.match(r"^(?:Fast )?Transfer (From|To) (.+)$", description, re.IGNORECASE) + if not m: + return None, None + name = m.group(2).strip() + return (name, None) if m.group(1).lower() == "from" else (None, name) + + +def parse_commbank_text(text: str) -> list[dict]: + """Parse extracted statement text into raw transaction dicts (oldest first). + + Raises CommbankParseError if no transactions are found or the running + balances do not reconcile — a mismatch means a row was mis-parsed or the + PDF layout changed, and nothing should be ingested. + """ + txs: list[dict] = [] + in_table = False + + for line in (l.strip() for l in text.splitlines()): + if not line: + continue + if line == _TABLE_HEADER: + in_table = True + continue + if not in_table: + continue + if line.startswith(_TABLE_END): + break # closing paragraph; nothing after this is a transaction + if line.startswith(_FURNITURE_PREFIXES): + continue + + m = _TX_LINE.match(line) + if m: + posted, details, amount, balance = m.groups() + txs.append({ + "posted_date": datetime.strptime(posted, "%d %b %Y").date(), + "description": details.strip(), + "amount_cents": _money_to_cents(amount), + "balance_cents": _money_to_cents(balance), + "card_suffix": None, + "value_date": None, + "notes": [], + }) + continue + + if not txs or line in _NOISE_LINES: + continue + cur = txs[-1] + if (cm := _CARD_LINE.match(line)): + cur["card_suffix"] = cm.group(1) + elif (vm := _VALUE_DATE_LINE.match(line)): + cur["value_date"] = datetime.strptime(vm.group(1), "%d/%m/%Y").date() + else: + cur["notes"].append(line) + + if not txs: + raise CommbankParseError("No transactions found — is this a CommBank Transaction Summary PDF?") + + for prev, cur in zip(txs, txs[1:]): + if prev["balance_cents"] + cur["amount_cents"] != cur["balance_cents"]: + raise CommbankParseError( + f"Running balance does not reconcile at {cur['posted_date']} " + f"'{cur['description'][:40]}' — PDF layout may have changed" + ) + return txs + + +def extract_pdf_text(pdf_bytes: bytes) -> str: + """Extract text from every page of the PDF.""" + import pdfplumber # lazy: only needed when a real PDF is parsed + + try: + with pdfplumber.open(io.BytesIO(pdf_bytes)) as pdf: + return "\n".join(page.extract_text() or "" for page in pdf.pages) + except Exception as e: + raise CommbankParseError(f"Could not read PDF: {e}") from e + + +def _assign_ids(txs: list[dict]) -> None: + """Add a stable `id` to each tx. + + The statement has no transaction ID, and identical same-day charges are common, + so the ID hashes (posted date, description, amount) plus the occurrence number + of that combination within the statement. Unlike hashing the running balance, + this is unaffected by other rows posting in between on overlapping uploads. + """ + seen: dict[tuple, int] = {} + for tx in txs: + key = (tx["posted_date"].isoformat(), tx["description"], tx["amount_cents"]) + n = seen.get(key, 0) + seen[key] = n + 1 + tx["id"] = "CBA-" + hashlib.sha256(f"{key}-{n}".encode()).hexdigest()[:16] + + +def parse_commbank_pdf(pdf_bytes: bytes) -> list[dict]: + """PDF bytes -> list of parsed transaction dicts with ids. Raises CommbankParseError.""" + txs = parse_commbank_text(extract_pdf_text(pdf_bytes)) + _assign_ids(txs) + return txs + + +def _to_row(tx: dict, source: str) -> dict: + """Build a transactions-table row from a parsed tx.""" + amount_cents = tx["amount_cents"] + amount = amount_cents / 100 + description = tx["description"] + payer, payee = _transfer_party(description) + + # The value date is when the card was actually used; the posted date can lag by days. + # No time of day exists, so use noon UTC to keep the calendar date stable. + effective_date: date = tx["value_date"] or tx["posted_date"] + timestamp = to_iso_str(datetime.combine(effective_date, datetime.min.time()).replace(hour=12)) + + raw = { + "posted_date": tx["posted_date"].isoformat(), + "value_date": tx["value_date"].isoformat() if tx["value_date"] else None, + "description": description, + "amount": amount, + "balance": tx["balance_cents"] / 100, + "card_suffix": tx["card_suffix"], + "notes": tx["notes"], + } + row = { + "id": tx["id"], + "source": source, + "bank": "CommBank", + "timestamp": timestamp, + "amount": amount, + "currency": CURRENCY, + "amount_gbp": convert_to_gbp(amount, CURRENCY, effective_date), + "description": description, + "payment_reference": " | ".join(tx["notes"]) or None, + "payer": payer, + "payee": payee, + "merchant": None, + "fees": 0.0, + "transaction_type": "CREDIT" if amount_cents >= 0 else "DEBIT", + "transaction_detail": _map_detail_type(description, tx["card_suffix"] is not None, amount_cents), + "state": "COMPLETED", # CommBank excludes pending transactions from statements + "is_internal": 0, + "is_interest": 1 if any(kw in description.lower() for kw in INTEREST_DESCRIPTION_KEYWORDS) else 0, + "running_balance": tx["balance_cents"] / 100, + "raw": json.dumps(raw, ensure_ascii=False), + } + return maybe_mark_internal(row) + + +def insert(txs: list[dict], source: str = "commbank"): + """Insert parsed CommBank transactions (from parse_commbank_pdf). + + Re-uploading the same statement is safe — duplicates are silently skipped. + + :returns: (inserted, skipped, errors) counts. + """ + inserted = 0 + skipped = 0 + errors = 0 + + logger.info(f"Inserting {len(txs)} transactions from {source}...") + + with get_conn() as conn: + cursor = conn.cursor() + for tx in txs: + try: + row = _to_row(tx, source) + + # Date-only timestamp: use the nearest fix that day rather than a 15-minute look-back + lat, lon = get_nearest_lat_lon_within_hours(cursor, row["timestamp"]) + place_id = get_place_id(lat, lon, conn=conn) + + cost_of_living = get_col_entry(lat=lat, lon=lon, conn=conn) + col_id = cost_of_living["id"] if cost_of_living else None + amount_normalised = ( + row["amount_gbp"] * (get_uk_col_index(conn) / cost_of_living["col_index"]) + if cost_of_living and row["amount_gbp"] is not None else None + ) + + cursor.execute( + """ + INSERT OR IGNORE INTO transactions ( + id, source, bank, timestamp, amount, currency, amount_gbp, + description, payment_reference, payer, payee, merchant, + fees, transaction_type, transaction_detail, state, + is_internal, is_interest, running_balance, raw, place_id, + col_id, amount_normalised + ) VALUES (:id, :source, :bank, :timestamp, + :amount, :currency, :amount_gbp, :description, + :payment_reference, :payer, :payee, :merchant, + :fees, :transaction_type, :transaction_detail, + :state, :is_internal, :is_interest, :running_balance, + :raw, :place_id, :col_id, :amount_normalised) + """, + row | {"place_id": place_id, "col_id": col_id, "amount_normalised": amount_normalised}, + ) + if cursor.rowcount == 1: + inserted += 1 + else: + skipped += 1 + logger.info(f"Duplicate transaction. Skipping... {row['id']} ({tx['description'][:40]})") + except Exception as e: + logger.error(f"Error when processing CommBank transaction {tx.get('id')}: {str(e)}") + errors += 1 + + conn.commit() + + send_notification( + title="CommBank", + body=f"💵 {len(txs)} rows received | {inserted} inserted | {skipped} skipped", + time_sensitive=False, + ) + + return inserted, skipped, errors diff --git a/server/app/database/transaction/ingest/util.py b/server/app/database/transaction/ingest/util.py index 7b6f4b2..f498910 100644 --- a/server/app/database/transaction/ingest/util.py +++ b/server/app/database/transaction/ingest/util.py @@ -4,6 +4,7 @@ Shared helpers used by both the Revolut and Wise ingest modules. """ +from datetime import datetime, timedelta from typing import Optional from config.general import SELF_NAMES @@ -32,6 +33,43 @@ def get_closest_lat_lon_by_timestamp(cursor, timestamp: str) -> tuple[Optional[f return lat, lon +def get_nearest_lat_lon_within_hours(cursor, timestamp: str, hours: float = 12) -> tuple[Optional[float], Optional[float]]: + """Nearest location fix (before or after) within +/- `hours` of timestamp. + + For sources with no time of day (e.g. CommBank statements, which carry only a + date), where get_closest_lat_lon_by_timestamp's 15-minute look-back would + almost never match. + + Compares the raw timestamp column against precomputed bounds (one indexed + lookup each side) — wrapping the column in julianday() defeats the index and + scans the whole location_unified view per call. + """ + target = datetime.fromisoformat(timestamp.replace("Z", "+00:00")).replace(tzinfo=None) + fmt = "%Y-%m-%dT%H:%M:%SZ" + target_s = target.strftime(fmt) + window = timedelta(hours=hours) + + before = cursor.execute(""" + SELECT timestamp, latitude, longitude FROM location_unified + WHERE timestamp <= ? AND timestamp >= ? + ORDER BY timestamp DESC LIMIT 1 + """, (target_s, (target - window).strftime(fmt))).fetchone() + after = cursor.execute(""" + SELECT timestamp, latitude, longitude FROM location_unified + WHERE timestamp > ? AND timestamp <= ? + ORDER BY timestamp ASC LIMIT 1 + """, (target_s, (target + window).strftime(fmt))).fetchone() + + def gap(row): + ts = datetime.fromisoformat(row["timestamp"].replace("Z", "+00:00")).replace(tzinfo=None) + return abs(ts - target) + + candidates = [r for r in (before, after) if r] + if not candidates: + return None, None + best = min(candidates, key=gap) + return best["latitude"], best["longitude"] + def maybe_mark_internal(row: dict) -> dict: desc = (row.get('description') or '').lower() payee = (row.get('payee') or '').lower() diff --git a/server/requirements.txt b/server/requirements.txt index f53d0c3..459f757 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -21,3 +21,4 @@ airportsdata prefect==3.6.25 prefect-shell>=0.3.5 openai==2.30.0 +pdfplumber diff --git a/server/tests/test_ingest_util_db.py b/server/tests/test_ingest_util_db.py index 1d2f4ad..2e7a08d 100644 --- a/server/tests/test_ingest_util_db.py +++ b/server/tests/test_ingest_util_db.py @@ -103,3 +103,23 @@ def test_future_point_excluded(self, db): lat, lon = get_closest_lat_lon_by_timestamp(cur, "2024-06-15 09:00:00") assert lat is None assert lon is None + + +class TestGetNearestLatLonWithinHours: + + def test_finds_fix_hours_away_before_or_after(self, db): + from database.transaction.ingest.util import get_nearest_lat_lon_within_hours + _insert(db, "2026-09-13T08:00:00Z", 1.0, 1.0) # 4h before + _insert(db, "2026-09-13T15:00:00Z", 2.0, 2.0) # 3h after — nearer + lat, lon = get_nearest_lat_lon_within_hours(db.cursor(), "2026-09-13T12:00:00Z") + assert (lat, lon) == (2.0, 2.0) + + def test_none_when_outside_window(self, db): + from database.transaction.ingest.util import get_nearest_lat_lon_within_hours + _insert(db, "2026-09-14T01:00:00Z", 1.0, 1.0) # 13h after + assert get_nearest_lat_lon_within_hours(db.cursor(), "2026-09-13T12:00:00Z") == (None, None) + + def test_window_boundary_included(self, db): + from database.transaction.ingest.util import get_nearest_lat_lon_within_hours + _insert(db, "2026-09-14T00:00:00Z", 1.0, 1.0) # exactly +12h + assert get_nearest_lat_lon_within_hours(db.cursor(), "2026-09-13T12:00:00Z") == (1.0, 1.0) diff --git a/server/tests/transaction/test_commbank_insert.py b/server/tests/transaction/test_commbank_insert.py new file mode 100644 index 0000000..8d3591c --- /dev/null +++ b/server/tests/transaction/test_commbank_insert.py @@ -0,0 +1,166 @@ +""" +test_commbank_insert.py — Tests for CommBank insert() logic and the upload endpoint. +""" + +import json +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from auth import require_upload_token +from conftest import app, db, row_count, upload_log_rows +from database.transaction.ingest.commbank import CommbankParseError, _assign_ids, parse_commbank_text +from test_commbank_parse import SAMPLE, make_text + + +def parsed(text=SAMPLE): + txs = parse_commbank_text(text) + _assign_ids(txs) + return txs + + +def run_insert(db, txs): + with patch("database.transaction.ingest.commbank.get_conn", return_value=db), \ + patch("database.transaction.ingest.commbank.convert_to_gbp", return_value=-4.0), \ + patch("database.transaction.ingest.commbank.get_nearest_lat_lon_within_hours", return_value=(None, None)), \ + patch("database.transaction.ingest.commbank.get_place_id", return_value=1), \ + patch("database.transaction.ingest.commbank.get_col_entry", return_value=None): + from database.transaction.ingest.commbank import insert + return insert(txs) + + +def test_inserts_all_rows(db): + assert run_insert(db, parsed()) == (4, 0, 0) + assert row_count(db) == 4 + + +def test_reupload_is_idempotent(db): + run_insert(db, parsed()) + assert run_insert(db, parsed()) == (0, 4, 0) + assert row_count(db) == 4 + + +def test_card_row_uses_value_date_for_timestamp(db): + run_insert(db, parsed()) + row = db.execute("SELECT * FROM transactions WHERE description LIKE 'KFC%'").fetchone() + assert row["timestamp"] == "2026-09-13T12:00:00Z" + raw = json.loads(row["raw"]) + assert raw["posted_date"] == "2026-09-15" + assert raw["value_date"] == "2026-09-13" + assert raw["card_suffix"] == "0488" + + +def test_row_without_value_date_uses_posted_date(db): + run_insert(db, parsed()) + row = db.execute("SELECT * FROM transactions WHERE description LIKE 'SOME CAFE%'").fetchone() + assert row["timestamp"] == "2026-09-13T12:00:00Z" + + +def test_fields_mapped(db): + run_insert(db, parsed()) + row = db.execute("SELECT * FROM transactions WHERE description LIKE 'KFC%'").fetchone() + assert row["bank"] == "CommBank" + assert row["source"] == "commbank" + assert row["currency"] == "AUD" + assert row["amount"] == pytest.approx(-12.95) + assert row["running_balance"] == pytest.approx(79.93) + assert row["transaction_type"] == "DEBIT" + assert row["transaction_detail"] == "CARD_PAYMENT" + assert row["state"] == "COMPLETED" + assert row["is_internal"] == 0 + + +def test_self_transfers_marked_internal(db): + # Names in config.general.SELF_NAMES + text = make_text( + "11 Sep 2026 Fast Transfer From Daniel James Roberts $50.00 $50.00\n" + "12 Sep 2026 Transfer To Daniel James Roberts -$10.00 $40.00\n" + ) + run_insert(db, parsed(text)) + rows = db.execute("SELECT * FROM transactions ORDER BY timestamp").fetchall() + assert [r["is_internal"] for r in rows] == [1, 1] + assert [r["transaction_detail"] for r in rows] == ["TRANSFER", "TRANSFER"] + assert rows[0]["payer"] == "Daniel James Roberts" + assert rows[1]["payee"] == "Daniel James Roberts" + + +def test_atm_detail(db): + text = make_text("14 Sep 2026 Wdl ATM CBA ATM ST KILDA B VIC 316902 AUS -$20.00 $80.00\n") + run_insert(db, parsed(text)) + assert db.execute("SELECT transaction_detail FROM transactions").fetchone()[0] == "ATM" + + +def test_bad_row_counted_as_error_not_fatal(db): + txs = parsed() + del txs[1]["posted_date"] + inserted, skipped, errors = run_insert(db, txs) + assert (inserted, errors) == (3, 1) + + +# --------------------------------------------------------------------------- +# Upload endpoint +# --------------------------------------------------------------------------- + +@pytest.fixture +def commbank_client(db, tmp_path): + app.dependency_overrides[require_upload_token] = lambda: None + with patch("upload.transaction.router.parse_commbank_pdf") as mock_parse, \ + patch("upload.transaction.router.insert_commbank") as mock_insert, \ + patch("upload.transaction.router.COMMBANK_BACKUP_DIR", tmp_path), \ + patch("database.transaction.upload_log.get_conn", return_value=db): + mock_parse.return_value = [{"id": "x"}] + mock_insert.return_value = (1, 0, 0) # inserted, skipped, errors + with TestClient(app) as c: + yield c, mock_parse, mock_insert, tmp_path + app.dependency_overrides.clear() + + +def _post(c, name="TransactionSummary.pdf", period="2026-09"): + return c.post( + "/upload/transaction/commbank", + files={"file": (name, b"%PDF-fake", "application/pdf")}, + data={"period": period}, + headers={"authorization": "Bearer testtoken"}, + ) + + +def test_endpoint_queues_and_backs_up(commbank_client): + c, _, mock_insert, tmp_path = commbank_client + resp = _post(c) + assert resp.status_code == 200 + assert resp.json()["status"] == "queued" + assert len(list(tmp_path.glob("*.pdf"))) == 1 + mock_insert.assert_called_once() + + +def test_endpoint_records_upload_log(commbank_client, db): + c, *_ = commbank_client + _post(c) + rows = upload_log_rows(db) + assert len(rows) == 1 + assert rows[0]["source"] == "commbank" + assert rows[0]["period_start"] == "2026-09-01" + assert rows[0]["period_end"] == "2026-09-30" + assert rows[0]["row_count"] == 1 + + +def test_endpoint_rejects_non_pdf(commbank_client): + c, mock_parse, *_ = commbank_client + assert _post(c, name="statement.csv").status_code == 400 + mock_parse.assert_not_called() + + +def test_endpoint_rejects_bad_period(commbank_client): + c, *_ = commbank_client + assert _post(c, period="September").status_code == 400 + + +def test_endpoint_rejects_unparseable_pdf(commbank_client): + c, mock_parse, mock_insert, tmp_path = commbank_client + mock_parse.side_effect = CommbankParseError("No transactions found") + resp = _post(c) + assert resp.status_code == 400 + assert "No transactions" in resp.json()["detail"] + assert list(tmp_path.glob("*.pdf")) == [] + mock_insert.assert_not_called() diff --git a/server/tests/transaction/test_commbank_parse.py b/server/tests/transaction/test_commbank_parse.py new file mode 100644 index 0000000..5174b56 --- /dev/null +++ b/server/tests/transaction/test_commbank_parse.py @@ -0,0 +1,118 @@ +""" +test_commbank_parse.py — Tests for CommBank statement text parsing (no PDF needed). +""" + +from datetime import date + +import pytest + +from database.transaction.ingest.commbank import ( + CommbankParseError, _assign_ids, parse_commbank_text, +) + +HEADER = """Account Number 000000 12345678 +Page 1 of 2 +JANE CITIZEN +1 EXAMPLE ST +Account name JANE CITIZEN +Date Transaction details Amount Balance +""" + +FOOTER = """Any pending transactions haven’t been included in this list. Proceeds of cheques aren’t available until +cleared. +Kind regards, +The CommBank Team. +""" + + +def make_text(body: str) -> str: + return HEADER + body + FOOTER + + +SAMPLE = make_text("""11 Sep 2026 Fast Transfer From Jane Citizen $100.00 $100.00 +CREDIT TO ACCOUNT +13 Sep 2026 SOME CAFE MELBOURNE AU -$7.12 $92.88 +15 Sep 2026 KFC St Kilda AUS -$12.95 $79.93 +Card xx0488 +Value Date: 13/09/2026 +Created 01/10/26 02:37pm (Sydney/Melbourne time) +While this letter is accurate at the time it’s produced, +Account Number 000000 12345678 +Page 2 of 2 +Date Transaction details Amount Balance +16 Sep 2026 Transfer To Jane Citizen -$1.00 $78.93 +PayID Email from CommBank App +Test +""") + + +def test_parses_all_rows(): + txs = parse_commbank_text(SAMPLE) + assert len(txs) == 4 + assert [t["amount_cents"] for t in txs] == [10000, -712, -1295, -100] + assert txs[0]["posted_date"] == date(2026, 9, 11) + + +def test_card_and_value_date_attached(): + tx = parse_commbank_text(SAMPLE)[2] + assert tx["card_suffix"] == "0488" + assert tx["value_date"] == date(2026, 9, 13) + assert tx["notes"] == [] + + +def test_notes_captured_noise_dropped(): + txs = parse_commbank_text(SAMPLE) + assert txs[0]["notes"] == [] # CREDIT TO ACCOUNT is noise + assert txs[3]["notes"] == ["PayID Email from CommBank App", "Test"] + + +def test_header_not_parsed_into_rows(): + txs = parse_commbank_text(SAMPLE) + assert all("JANE" not in t["description"] and "EXAMPLE" not in " ".join(t["notes"]) for t in txs) + + +def test_page_furniture_not_in_notes(): + for t in parse_commbank_text(SAMPLE): + assert not any(n.startswith(("Created", "While", "Page", "Account Number")) for n in t["notes"]) + + +def test_footer_not_in_notes(): + assert all("pending" not in " ".join(t["notes"]) for t in parse_commbank_text(SAMPLE)) + + +def test_thousands_separator_and_negative_balance(): + text = make_text( + "01 Sep 2026 Pay $1,234.56 $1,234.56\n" + "02 Sep 2026 Rent -$1,300.00 -$65.44\n" + ) + txs = parse_commbank_text(text) + assert txs[0]["amount_cents"] == 123456 + assert txs[1]["balance_cents"] == -6544 + + +def test_balance_mismatch_raises(): + text = make_text( + "01 Sep 2026 A -$1.00 $9.00\n" + "02 Sep 2026 B -$1.00 $5.00\n" + ) + with pytest.raises(CommbankParseError, match="reconcile"): + parse_commbank_text(text) + + +def test_no_transactions_raises(): + with pytest.raises(CommbankParseError): + parse_commbank_text("not a statement") + + +def test_identical_same_day_rows_get_distinct_stable_ids(): + text = make_text( + "16 Sep 2026 Base St Kilda VI AUS -$9.25 $90.75\n" + "16 Sep 2026 Base St Kilda VI AUS -$9.25 $81.50\n" + ) + a = parse_commbank_text(text) + _assign_ids(a) + b = parse_commbank_text(text) + _assign_ids(b) + assert a[0]["id"] != a[1]["id"] + assert [t["id"] for t in a] == [t["id"] for t in b] + assert a[0]["id"].startswith("CBA-") From ad379631f3b39b9deb1491bd6371aa56939f6cf0 Mon Sep 17 00:00:00 2001 From: Daniel Roberts <29379642+danielroberts20@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:44:33 +0000 Subject: [PATCH 2/3] feat(upload): add POST /upload/commbank and a CommBank backup directory Accepts a CommBank Transaction Summary PDF plus the YYYY-MM period it covers. The PDF is parsed up front (an unreadable statement is rejected with a 400), a backup copy is saved, and ingestion is queued in the background with the period recorded in upload_log. COMMBANK_BACKUP_DIR is created with the other backup dirs and shown in the local-backups metadata. Co-Authored-By: Claude Sonnet 5.5 --- server/app/config/general.py | 2 ++ server/app/metadata/backups.py | 3 +- server/app/upload/transaction/router.py | 46 ++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/server/app/config/general.py b/server/app/config/general.py index 4550cb8..dc05ac5 100644 --- a/server/app/config/general.py +++ b/server/app/config/general.py @@ -37,6 +37,7 @@ LOCATION_OVERLAND_BACKUP_DIR = LOCATION_BACKUP_DIR / "overland" REVOLUT_BACKUP_DIR = UPLOADS_BACKUP_DIR / "revolut" WISE_BACKUP_DIR = UPLOADS_BACKUP_DIR / "wise" +COMMBANK_BACKUP_DIR = UPLOADS_BACKUP_DIR / "commbank" JOURNAL_BACKUP_DIR = UPLOADS_BACKUP_DIR / "journal" JOURNAL_DIR = DATA_DIR / "journal" @@ -53,6 +54,7 @@ LOCATION_OVERLAND_BACKUP_DIR, REVOLUT_BACKUP_DIR, WISE_BACKUP_DIR, + COMMBANK_BACKUP_DIR, JOURNAL_BACKUP_DIR, ] diff --git a/server/app/metadata/backups.py b/server/app/metadata/backups.py index 78a44e3..4779db5 100644 --- a/server/app/metadata/backups.py +++ b/server/app/metadata/backups.py @@ -15,7 +15,7 @@ from config.general import ( DATABASE_BACKUP_DIR, FX_BACKUP_DIR, HEALTH_BACKUP_DIR, LOCATION_OVERLAND_BACKUP_DIR, LOCATION_SHORTCUTS_BACKUP_DIR, - REVOLUT_BACKUP_DIR, STALE_DAYS, WISE_BACKUP_DIR, WORKOUT_BACKUP_DIR, + COMMBANK_BACKUP_DIR, REVOLUT_BACKUP_DIR, STALE_DAYS, WISE_BACKUP_DIR, WORKOUT_BACKUP_DIR, ) @@ -68,6 +68,7 @@ def get_local_backups() -> dict: }, "revolut": _latest_in_dir(REVOLUT_BACKUP_DIR, "*.csv"), "wise": _latest_in_dir(WISE_BACKUP_DIR, "*.zip"), + "commbank": _latest_in_dir(COMMBANK_BACKUP_DIR, "*.pdf"), } diff --git a/server/app/upload/transaction/router.py b/server/app/upload/transaction/router.py index ed9f375..a827f85 100644 --- a/server/app/upload/transaction/router.py +++ b/server/app/upload/transaction/router.py @@ -8,10 +8,12 @@ from babel import numbers from typing import Optional from notifications import send_notification -from config.general import REVOLUT_BACKUP_DIR, WISE_BACKUP_DIR +from config.general import COMMBANK_BACKUP_DIR, REVOLUT_BACKUP_DIR, WISE_BACKUP_DIR +from fastapi.concurrency import run_in_threadpool from fastapi import APIRouter, UploadFile, File, Form, HTTPException, BackgroundTasks, Depends # type: ignore from pydantic import BaseModel, Field, field_validator # type: ignore from database.transaction.ingest.revolut import insert as insert_revolut +from database.transaction.ingest.commbank import CommbankParseError, insert as insert_commbank, parse_commbank_pdf from auth import require_upload_token from database.exchange.fx import convert_to_gbp @@ -68,6 +70,12 @@ def _revolut_upload_task(decoded: str, period_start: str, period_end: str) -> No _record_upload("revolut", period_start, period_end, row_count) +def _commbank_upload_task(txs: list[dict], period_start: str, period_end: str) -> None: + inserted, skipped, _errors = insert_commbank(txs) + row_count = inserted + skipped + _record_upload("commbank", period_start, period_end, row_count) + + @router.post("/wise", dependencies=[Depends(require_upload_token)]) async def upload_wise( background_tasks: BackgroundTasks, @@ -134,6 +142,42 @@ async def upload_revolut( return {"status": "queued"} +@router.post("/commbank", dependencies=[Depends(require_upload_token)]) +async def upload_commbank( + background_tasks: BackgroundTasks, + file: UploadFile = File(...), + period: str = Form(..., description="Calendar month this export covers, 'YYYY-MM'"), +): + """Accept a CommBank Transaction Summary PDF, validate it, save a backup, and queue ingestion. + + The PDF is parsed up front so an unreadable or unrecognised statement is + rejected with a 400 rather than failing silently in the background. + + `period` (form field, required) is the "YYYY-MM" calendar month this + statement covers — used to record upload_log coverage so daily_summary + can determine spend_complete for dates in that month. + """ + if not file.filename.lower().endswith(".pdf"): + raise HTTPException(status_code=400, detail="File must be a PDF") + + period_start, period_end = _parse_period(period) + + contents = await file.read() + + try: + txs = await run_in_threadpool(parse_commbank_pdf, contents) + except CommbankParseError as e: + raise HTTPException(status_code=400, detail=str(e)) + + now = datetime.now() + backup_path = COMMBANK_BACKUP_DIR / f"{now.strftime('%Y-%m-%d_%H-%M-%S')}.pdf" + with open(backup_path, "wb") as f: + f.write(contents) + + background_tasks.add_task(_commbank_upload_task, txs, period_start, period_end) + return {"status": "queued", "transactions": len(txs)} + + class CashTransactionRequest(BaseModel): """Request body for manually logging a cash transaction.""" From 0fd0518cf7004674e046ed30bd8e5b4b3e12ac6b Mon Sep 17 00:00:00 2001 From: Daniel Roberts <29379642+danielroberts20@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:44:33 +0000 Subject: [PATCH 3/3] feat(daily-summary): require CommBank uploads for spend-complete from account opening spend_complete now needs a covering upload_log entry from CommBank as well as Revolut and Wise, but only for dates on or after the account existed (UPLOAD_SOURCE_START_DATES), so earlier dates can still complete. Co-Authored-By: Claude Sonnet 5.5 --- .../daily_summary/transactions.py | 20 ++++++++++++--- .../test_transactions_completeness.py | 25 ++++++++++++++++--- ...st_backfill_upload_log_and_completeness.py | 5 ++-- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/server/app/scheduled_tasks/daily_summary/transactions.py b/server/app/scheduled_tasks/daily_summary/transactions.py index b628a90..5acd2ae 100644 --- a/server/app/scheduled_tasks/daily_summary/transactions.py +++ b/server/app/scheduled_tasks/daily_summary/transactions.py @@ -25,7 +25,21 @@ # Sources that must each confirm coverage of a date's calendar month in # upload_log before that date can be considered spend-complete. -REQUIRED_UPLOAD_SOURCES = ("revolut", "wise") +REQUIRED_UPLOAD_SOURCES = ("revolut", "wise", "commbank") + +# First date (local, ISO) a source can have data. A source is only required for +# dates on/after its start — otherwise every date before the account existed +# could never be spend-complete. Sources not listed apply to all dates. +UPLOAD_SOURCE_START_DATES = { + "commbank": "2026-08-30", # CommBank account opened +} + + +def _required_sources_for(local_date: str) -> tuple[str, ...]: + return tuple( + s for s in REQUIRED_UPLOAD_SOURCES + if UPLOAD_SOURCE_START_DATES.get(s, "") <= local_date + ) # --------------------------------------------------------------------------- @@ -117,7 +131,7 @@ def _transactions(conn, ctx: dict) -> dict: def _upload_coverage_predicate(local_date: str, data: dict, conn) -> bool: """ A date is spend-complete once every source in REQUIRED_UPLOAD_SOURCES - has an upload_log entry whose period covers it — i.e. that source's + that existed on that date (see UPLOAD_SOURCE_START_DATES) has an upload_log entry whose period covers it — i.e. that source's statement for this calendar month has actually been uploaded. Previously this was never auto-closed and the monthly backfill flow set @@ -131,7 +145,7 @@ def _upload_coverage_predicate(local_date: str, data: dict, conn) -> bool: WHERE period_start <= ? AND period_end >= ? """, (local_date, local_date)).fetchall() covered = {r["source"] for r in rows} - return all(source in covered for source in REQUIRED_UPLOAD_SOURCES) + return all(source in covered for source in _required_sources_for(local_date)) TRANSACTIONS_DOMAIN = Domain( diff --git a/server/tests/daily_summary/test_transactions_completeness.py b/server/tests/daily_summary/test_transactions_completeness.py index 5d87843..45152bb 100644 --- a/server/tests/daily_summary/test_transactions_completeness.py +++ b/server/tests/daily_summary/test_transactions_completeness.py @@ -47,8 +47,8 @@ def _add_upload(conn, source, period_start, period_end, row_count=1): ) -def test_required_sources_are_revolut_and_wise(): - assert set(REQUIRED_UPLOAD_SOURCES) == {"revolut", "wise"} +def test_required_sources_are_revolut_wise_and_commbank(): + assert set(REQUIRED_UPLOAD_SOURCES) == {"revolut", "wise", "commbank"} def test_incomplete_with_no_uploads(conn): @@ -60,15 +60,17 @@ def test_incomplete_when_only_one_source_covers(conn): assert _upload_coverage_predicate("2026-03-15", {}, conn) is False -def test_complete_when_both_sources_cover(conn): +def test_complete_when_all_sources_cover(conn): _add_upload(conn, "revolut", "2026-03-01", "2026-03-31") _add_upload(conn, "wise", "2026-03-01", "2026-03-31") + _add_upload(conn, "commbank", "2026-03-01", "2026-03-31") assert _upload_coverage_predicate("2026-03-15", {}, conn) is True def test_date_outside_any_uploaded_period_is_incomplete(conn): _add_upload(conn, "revolut", "2026-03-01", "2026-03-31") _add_upload(conn, "wise", "2026-03-01", "2026-03-31") + _add_upload(conn, "commbank", "2026-03-01", "2026-03-31") assert _upload_coverage_predicate("2026-04-01", {}, conn) is False @@ -78,11 +80,28 @@ def test_zero_row_count_upload_still_counts_as_coverage(conn): uploaded yet.""" _add_upload(conn, "revolut", "2026-03-01", "2026-03-31", row_count=0) _add_upload(conn, "wise", "2026-03-01", "2026-03-31", row_count=0) + _add_upload(conn, "commbank", "2026-03-01", "2026-03-31", row_count=0) assert _upload_coverage_predicate("2026-03-15", {}, conn) is True def test_boundary_dates_are_covered(conn): _add_upload(conn, "revolut", "2026-03-01", "2026-03-31") _add_upload(conn, "wise", "2026-03-01", "2026-03-31") + _add_upload(conn, "commbank", "2026-03-01", "2026-03-31") assert _upload_coverage_predicate("2026-03-01", {}, conn) is True assert _upload_coverage_predicate("2026-03-31", {}, conn) is True + + +def test_commbank_not_required_before_account_opened(conn): + """Dates before the CommBank account existed only need revolut + wise.""" + _add_upload(conn, "revolut", "2026-03-01", "2026-03-31") + _add_upload(conn, "wise", "2026-03-01", "2026-03-31") + assert _upload_coverage_predicate("2026-03-15", {}, conn) is True + + +def test_commbank_required_from_account_opened(conn): + _add_upload(conn, "revolut", "2026-09-01", "2026-09-30") + _add_upload(conn, "wise", "2026-09-01", "2026-09-30") + assert _upload_coverage_predicate("2026-09-15", {}, conn) is False + _add_upload(conn, "commbank", "2026-09-01", "2026-09-30") + assert _upload_coverage_predicate("2026-09-15", {}, conn) is True diff --git a/server/tests/test_backfill_upload_log_and_completeness.py b/server/tests/test_backfill_upload_log_and_completeness.py index aa31802..8e398e7 100644 --- a/server/tests/test_backfill_upload_log_and_completeness.py +++ b/server/tests/test_backfill_upload_log_and_completeness.py @@ -146,8 +146,9 @@ def test_recompute_spend_complete_flips_0_to_1_when_both_sources_covered(db_path conn.execute("INSERT INTO daily_summary (date, spend_complete) VALUES ('2026-06-15', 0)") conn.execute(""" INSERT INTO upload_log (source, period_start, period_end, row_count, inferred) - VALUES ('revolut', '2026-06-01', '2026-06-30', 5, 0), - ('wise', '2026-06-01', '2026-06-30', 3, 0) + VALUES ('revolut', '2026-06-01', '2026-06-30', 5, 0), + ('wise', '2026-06-01', '2026-06-30', 3, 0), + ('commbank', '2026-06-01', '2026-06-30', 2, 0) """) conn.commit()