From f3b23e6f572ebb9ffc001db254a8a1a42757cb15 Mon Sep 17 00:00:00 2001 From: Amit Afre Date: Sun, 30 Aug 2026 09:22:16 +0100 Subject: [PATCH 1/2] feat(currency): narrow currency core, real-statement PDF fixes, and FX rates Implements Phase 1 (narrow currency core) and the remaining Phase 2 defects from the v0.2 plan, on top of work already staged on this branch. Currency core: - Settings.base_currency, an ISO-4217 SUPPORTED_CURRENCIES dict replacing the GBP-only reject, and per-currency minor-unit exponents (JPY=0) applied consistently through a single domain.money.minor_units helper. - fx_rates table (migration 0004), FxRateRepository with at-or-before/inverse lookup, domain.fx.to_base, and `pfa fx set/fetch/list` plus /fx/* endpoints. Rates cross every boundary (API request/response, Frankfurter fetch) as decimal strings, never float. - Analytics is per-currency throughout (monthly_summary, category/merchant totals, cashflow, budgets, current_cash), with a mixed GBP/INR regression test. - A row's currency is validated against its destination account's at preview time (blocking issue, not a crash) and account currency lookup during preview no longer creates an account as a side effect. Statement extraction: - Fixed: an AMEX payment's own-line "CR" marker was silently dropped during extraction, so the statement's default debit-positive convention flipped it to spending. The marker now attaches to its row and marks the direction explicit, which the sign convention now respects everywhere (PDF debit/credit columns, CSV, inline/own-line CR). - Fixed: parse_amount used a naive `* 100` instead of the currency's minor-unit exponent and rounding, corrupting non-2dp currencies (JPY) and truncating instead of rounding. - Fixed: year-less dates ("Jul31") always took today's year, even during a replay of an old statement. Batches now infer the statement's year from any other year-bearing date in the same file and normalize every date up front, with a visible warning when no date carries a year at all. - AMEX's duplicate "Date" column no longer leaks into the transaction description. - Barclaycard two-column layout clustering and Money Out/Money In PDF header coverage. Also fixes: ruff/format/mypy clean (was failing all three), restores a deleted PDF header-alias regression test, and adds fetch_and_store_fx_rates coverage that proves Frankfurter rates are parsed to Decimal rather than round-tripped through float. --- alembic/versions/0004_fx_rates.py | 32 ++++ src/pfa/analytics/service.py | 92 +++++++--- src/pfa/api/app.py | 130 +++++++++++++- src/pfa/cli/app.py | 96 +++++++++- src/pfa/config.py | 1 + src/pfa/db/models.py | 16 ++ src/pfa/db/repositories.py | 141 ++++++++++++++- src/pfa/db/unit_of_work.py | 2 + src/pfa/domain/fx.py | 64 +++++++ src/pfa/domain/money.py | 40 ++++- src/pfa/ingestion/batches.py | 113 ++++++++++-- src/pfa/ingestion/candidates.py | 110 +++++++++++- src/pfa/ingestion/dialects.py | 67 +++++++ src/pfa/ingestion/extractors/csv.py | 16 +- src/pfa/ingestion/extractors/ocr.py | 5 + src/pfa/ingestion/extractors/pdf.py | 225 ++++++++++++++++++------ src/pfa/ingestion/service.py | 40 ++++- src/pfa/planning/service.py | 38 ++-- src/pfa/services/fx.py | 65 +++++++ src/pfa/services/review.py | 24 ++- tests/integration/test_api.py | 42 +++++ tests/integration/test_cli.py | 20 +++ tests/unit/test_financial_invariants.py | 62 ++++++- tests/unit/test_fx.py | 151 ++++++++++++++++ tests/unit/test_money.py | 17 ++ tests/unit/test_pdf_extractor.py | 51 +++++- tests/unit/test_planning_scenarios.py | 3 +- tests/unit/test_statement_candidates.py | 2 +- 28 files changed, 1505 insertions(+), 160 deletions(-) create mode 100644 alembic/versions/0004_fx_rates.py create mode 100644 src/pfa/domain/fx.py create mode 100644 src/pfa/ingestion/dialects.py create mode 100644 src/pfa/services/fx.py create mode 100644 tests/unit/test_fx.py diff --git a/alembic/versions/0004_fx_rates.py b/alembic/versions/0004_fx_rates.py new file mode 100644 index 0000000..2967109 --- /dev/null +++ b/alembic/versions/0004_fx_rates.py @@ -0,0 +1,32 @@ +"""fx rates table""" + +import sqlalchemy as sa +from alembic import op + +revision = "0004_fx_rates" +down_revision = "0003_batch_amount_sign" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "fx_rates", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("base_currency", sa.String(length=3), nullable=False), + sa.Column("quote_currency", sa.String(length=3), nullable=False), + sa.Column("rate", sa.String(length=32), nullable=False), + sa.Column("effective_at", sa.Date(), nullable=False), + sa.Column("source", sa.String(length=50), nullable=False), + sa.Column("retrieved_at", sa.DateTime(), nullable=False), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "base_currency", "quote_currency", "effective_at", name="uq_fx_rates_base_quote_date" + ), + ) + op.create_index("ix_fx_rates_effective_at", "fx_rates", ["effective_at"]) + + +def downgrade() -> None: + op.drop_index("ix_fx_rates_effective_at", table_name="fx_rates") + op.drop_table("fx_rates") diff --git a/src/pfa/analytics/service.py b/src/pfa/analytics/service.py index 3e9065c..e734ff7 100644 --- a/src/pfa/analytics/service.py +++ b/src/pfa/analytics/service.py @@ -70,9 +70,17 @@ def __init__( self.budgets = budgets self.goals = goals - def monthly_summary(self, period: date) -> MonthlySummary: + def _filter_currency( + self, transactions: list[TransactionModel], currency: str + ) -> list[TransactionModel]: + curr = currency.upper() + return [t for t in transactions if (getattr(t, "currency", None) or "GBP").upper() == curr] + + def monthly_summary(self, period: date, currency: str = "GBP") -> MonthlySummary: start, end = month_bounds(period) - rows = self.transactions.between(start, end) + all_rows = self.transactions.between(start, end) + curr = currency.upper() + rows = self._filter_currency(all_rows, curr) income = sum(row.amount_minor for row in rows if row.kind == TransactionKind.INCOME.value) spending = sum(_spending(row) for row in rows) essential = sum(_spending(row) for row in rows if row.category in _ESSENTIAL) @@ -102,6 +110,7 @@ def monthly_summary(self, period: date) -> MonthlySummary: ) return MonthlySummary( period=start.strftime("%Y-%m"), + currency=curr, income_minor=income, spending_minor=spending, essential_spending_minor=essential, @@ -114,8 +123,9 @@ def monthly_summary(self, period: date) -> MonthlySummary: transaction_count=len(rows), ) - def category_spending(self, period: date) -> list[CategoryTotal]: - rows = self.transactions.between(*month_bounds(period)) + def category_spending(self, period: date, currency: str = "GBP") -> list[CategoryTotal]: + all_rows = self.transactions.between(*month_bounds(period)) + rows = self._filter_currency(all_rows, currency) totals: dict[str, list[int]] = defaultdict(lambda: [0, 0]) for row in rows: value = _spending(row) @@ -127,8 +137,9 @@ def category_spending(self, period: date) -> list[CategoryTotal]: for key, value in sorted(totals.items(), key=lambda item: -item[1][0]) ] - def merchant_spending(self, period: date) -> list[MerchantTotal]: - rows = self.transactions.between(*month_bounds(period)) + def merchant_spending(self, period: date, currency: str = "GBP") -> list[MerchantTotal]: + all_rows = self.transactions.between(*month_bounds(period)) + rows = self._filter_currency(all_rows, currency) totals: dict[str, list[int]] = defaultdict(lambda: [0, 0]) for row in rows: value = _spending(row) @@ -140,10 +151,12 @@ def merchant_spending(self, period: date) -> list[MerchantTotal]: for key, value in sorted(totals.items(), key=lambda item: -item[1][0]) ] - def compare_periods(self, current: date, previous: date | None = None) -> PeriodComparison: + def compare_periods( + self, current: date, previous: date | None = None, currency: str = "GBP" + ) -> PeriodComparison: previous = previous or (current.replace(day=1) - timedelta(days=1)) - current_summary = self.monthly_summary(current) - previous_summary = self.monthly_summary(previous) + current_summary = self.monthly_summary(current, currency=currency) + previous_summary = self.monthly_summary(previous, currency=currency) fields = ( "income_minor", "spending_minor", @@ -159,19 +172,31 @@ def compare_periods(self, current: date, previous: date | None = None) -> Period current=current_summary, previous=previous_summary, changes_minor=changes ) - def largest_transactions(self, period: date, limit: int = 10) -> list[TransactionModel]: - rows = self.transactions.between(*month_bounds(period)) + def largest_transactions( + self, period: date, limit: int = 10, currency: str = "GBP" + ) -> list[TransactionModel]: + all_rows = self.transactions.between(*month_bounds(period)) + rows = self._filter_currency(all_rows, currency) return sorted(rows, key=lambda row: _spending(row), reverse=True)[:limit] - def recurring_payments(self) -> list[dict[str, object]]: - return detect_recurring(self.transactions.all()) + def recurring_payments(self, currency: str = "GBP") -> list[dict[str, object]]: + all_rows = self.transactions.all() + rows = self._filter_currency(all_rows, currency) + return detect_recurring(rows) - def budget_status(self, period: date) -> list[BudgetStatus]: + def budget_status(self, period: date, currency: str = "GBP") -> list[BudgetStatus]: + curr = currency.upper() actual_by_category = { - item.category: item.total_minor for item in self.category_spending(period) + item.category: item.total_minor + for item in self.category_spending(period, currency=curr) } statuses = [] - for budget in self.budgets.active_on(month_bounds(period)[0]): + active_budgets = [ + b + for b in self.budgets.active_on(month_bounds(period)[0]) + if (getattr(b, "currency", None) or "GBP").upper() == curr + ] + for budget in active_budgets: actual = ( sum(actual_by_category.values()) if budget.category is None @@ -207,38 +232,53 @@ def goal_progress(self) -> list[GoalProgress]: for goal in self.goals.active() ] - def cashflow(self, period: date) -> dict[str, int | str]: - summary = self.monthly_summary(period) + def cashflow(self, period: date, currency: str = "GBP") -> dict[str, int | str]: + summary = self.monthly_summary(period, currency=currency) return { "period": summary.period, + "currency": summary.currency, "income_minor": summary.income_minor, "spending_minor": summary.spending_minor, "net_cashflow_minor": summary.net_cashflow_minor, } - def unusual_transactions(self, period: date) -> list[dict[str, object]]: - return unusual_transactions(self.transactions.all(), period) + def unusual_transactions(self, period: date, currency: str = "GBP") -> list[dict[str, object]]: + all_rows = self.transactions.all() + rows = self._filter_currency(all_rows, currency) + return unusual_transactions(rows, period) def category_spikes( - self, current: date, previous: date | None = None + self, current: date, previous: date | None = None, currency: str = "GBP" ) -> list[dict[str, object]]: previous = previous or (current.replace(day=1) - timedelta(days=1)) - return category_spikes(self.transactions.all(), current, previous) + all_rows = self.transactions.all() + rows = self._filter_currency(all_rows, currency) + return category_spikes(rows, current, previous) def category_trend( - self, category: str, as_of: date, months: int = 6 + self, category: str, as_of: date, months: int = 6, currency: str = "GBP" ) -> list[dict[str, int | str]]: - return category_trend(self.transactions.all(), category, as_of, months) + all_rows = self.transactions.all() + rows = self._filter_currency(all_rows, currency) + return category_trend(rows, category, as_of, months) def current_cash( - accounts: list[AccountModel], transactions: list[TransactionModel], as_of: date | None = None + accounts: list[AccountModel], + transactions: list[TransactionModel], + currency: str = "GBP", + as_of: date | None = None, ) -> int: + curr = currency.upper() opening = sum( account.opening_balance_minor for account in accounts if account.account_type not in {item.value for item in NON_CASH_ACCOUNT_TYPES} + and (getattr(account, "currency", None) or "GBP").upper() == curr ) return opening + sum( - _cash_delta(row) for row in transactions if as_of is None or row.transaction_date <= as_of + _cash_delta(row) + for row in transactions + if (getattr(row, "currency", None) or "GBP").upper() == curr + and (as_of is None or row.transaction_date <= as_of) ) diff --git a/src/pfa/api/app.py b/src/pfa/api/app.py index 2c039b9..4fc431f 100644 --- a/src/pfa/api/app.py +++ b/src/pfa/api/app.py @@ -3,6 +3,7 @@ from collections.abc import AsyncIterator, Awaitable, Callable from contextlib import asynccontextmanager, suppress from datetime import date, datetime +from decimal import Decimal, InvalidOperation from pathlib import Path from typing import Annotated, Literal @@ -39,6 +40,7 @@ from pfa.ingestion.upload import stage_upload, sweep_upload_dir from pfa.observability import TimedOperation from pfa.services.answers import deterministic_answer +from pfa.services.fx import fetch_and_store_fx_rates from pfa.services.health import health_report from pfa.services.review import monthly_review_evidence from pfa.services.runtime import close_services, open_services @@ -132,6 +134,28 @@ class ScenarioRequest(BaseModel): cost_minor: int = Field(ge=0) horizon_months: int = Field(default=3, ge=1, le=120) month: str | None = None + currency: str = "GBP" + + +class FxRateResponse(BaseModel): + id: int + base_currency: str + quote_currency: str + rate: str # decimal string - never float; see domain/fx.py + effective_at: date + source: str | None = None + + +class FxRateSetRequest(BaseModel): + base_currency: str + quote_currency: str + rate: str # decimal string - never float; see domain/fx.py + effective_at: date | None = None + + +class FxFetchRequest(BaseModel): + base_currency: str = "GBP" + on_date: date | None = None def _issue_response(issue: CandidateIssue) -> CandidateIssueResponse: @@ -406,29 +430,114 @@ def transactions( finally: close_services(engine, services) + @app.get("/fx/rates", response_model=list[FxRateResponse]) + def get_fx_rates( + base: str | None = None, + quote: str | None = None, + ) -> list[FxRateResponse]: + engine, services = open_services(active_settings) + try: + rates = services.uow.fx_rates.all() + if base: + rates = [r for r in rates if r.base_currency == base.upper()] + if quote: + rates = [r for r in rates if r.quote_currency == quote.upper()] + return [ + FxRateResponse( + id=r.id, + base_currency=r.base_currency, + quote_currency=r.quote_currency, + rate=r.rate, + effective_at=r.effective_at, + source=r.source, + ) + for r in rates + ] + finally: + close_services(engine, services) + + @app.post("/fx/rates", response_model=FxRateResponse) + def set_fx_rate(request: FxRateSetRequest) -> FxRateResponse: + try: + rate = Decimal(request.rate) + except InvalidOperation as exc: + raise HTTPException(status_code=422, detail=f"invalid rate {request.rate!r}") from exc + engine, services = open_services(active_settings) + try: + effective_at = request.effective_at or date.today() + model = services.uow.fx_rates.set_rate( + request.base_currency.upper(), + request.quote_currency.upper(), + rate, + effective_at=effective_at, + ) + response = FxRateResponse( + id=model.id, + base_currency=model.base_currency, + quote_currency=model.quote_currency, + rate=model.rate, + effective_at=model.effective_at, + source=model.source, + ) + close_services(engine, services) + return response + except Exception: + close_services(engine, services, False) + raise + + @app.post("/fx/fetch", response_model=list[FxRateResponse]) + def fetch_fx_rates(request: FxFetchRequest) -> list[FxRateResponse]: + engine, services = open_services(active_settings) + try: + models = fetch_and_store_fx_rates( + services.uow, + base_currency=request.base_currency.upper(), + on_date=request.on_date or date.today(), + ) + response = [ + FxRateResponse( + id=m.id, + base_currency=m.base_currency, + quote_currency=m.quote_currency, + rate=m.rate, + effective_at=m.effective_at, + source=m.source, + ) + for m in models + ] + close_services(engine, services) + return response + except Exception: + close_services(engine, services, False) + raise + @app.get("/analytics/monthly") - def monthly(month: str | None = None) -> dict[str, object]: + def monthly(month: str | None = None, currency: str = "GBP") -> dict[str, object]: engine, services = open_services(active_settings) try: - return services.analytics.monthly_summary(_month(month)).model_dump() + return services.analytics.monthly_summary(_month(month), currency=currency).model_dump() finally: close_services(engine, services) @app.get("/analytics/categories") - def categories(month: str | None = None) -> list[dict[str, object]]: + def categories(month: str | None = None, currency: str = "GBP") -> list[dict[str, object]]: engine, services = open_services(active_settings) try: return [ - item.model_dump() for item in services.analytics.category_spending(_month(month)) + item.model_dump() + for item in services.analytics.category_spending(_month(month), currency=currency) ] finally: close_services(engine, services) @app.get("/budgets") - def budgets(month: str | None = None) -> list[dict[str, object]]: + def budgets(month: str | None = None, currency: str = "GBP") -> list[dict[str, object]]: engine, services = open_services(active_settings) try: - return [item.model_dump() for item in services.analytics.budget_status(_month(month))] + return [ + item.model_dump() + for item in services.analytics.budget_status(_month(month), currency=currency) + ] finally: close_services(engine, services) @@ -445,7 +554,10 @@ def purchase(request: ScenarioRequest) -> dict[str, object]: engine, services = open_services(active_settings) try: return services.planning.simulate_purchase( - request.cost_minor, request.horizon_months, _month(request.month) + request.cost_minor, + request.horizon_months, + _month(request.month), + currency=request.currency, ).model_dump() finally: close_services(engine, services) @@ -481,10 +593,10 @@ def chat(request: ChatRequest) -> dict[str, str]: close_services(engine, services) @app.get("/reviews/monthly") - def review(month: str | None = None) -> dict[str, object]: + def review(month: str | None = None, currency: str = "GBP") -> dict[str, object]: engine, services = open_services(active_settings) try: - return monthly_review_evidence(services.analytics, _month(month)) + return monthly_review_evidence(services.analytics, _month(month), currency=currency) finally: close_services(engine, services) diff --git a/src/pfa/cli/app.py b/src/pfa/cli/app.py index 4230253..60ac045 100644 --- a/src/pfa/cli/app.py +++ b/src/pfa/cli/app.py @@ -3,6 +3,7 @@ import subprocess import sys from datetime import date +from decimal import Decimal, InvalidOperation from pathlib import Path import typer @@ -20,6 +21,7 @@ from pfa.domain.transactions import ClassificationSource, SpendingCategory from pfa.ingestion.service import ImportService from pfa.services.answers import deterministic_answer +from pfa.services.fx import fetch_and_store_fx_rates from pfa.services.health import health_report from pfa.services.review import monthly_review_evidence from pfa.services.runtime import close_services, open_services @@ -30,11 +32,13 @@ summary_app = typer.Typer(help="Summary commands") budget_app = typer.Typer(help="Budget commands") goals_app = typer.Typer(help="Goal commands") +fx_app = typer.Typer(help="Foreign exchange rate commands") app.add_typer(db_app, name="db") app.add_typer(transactions_app, name="transactions") app.add_typer(summary_app, name="summary") app.add_typer(budget_app, name="budget") app.add_typer(goals_app, name="goals") +app.add_typer(fx_app, name="fx") console = Console(legacy_windows=False) @@ -60,8 +64,8 @@ def _legacy_print_money(minor: int) -> str: return f"£{Money(minor).to_major():,.2f}" -def print_money(minor: int) -> str: - return f"GBP {Money(minor).to_major():,.2f}" +def print_money(minor: int, currency: str = "GBP") -> str: + return f"{currency.upper()} {Money(minor, currency=currency).to_major():,.2f}" @db_app.command("init") @@ -102,11 +106,14 @@ def import_transactions(path: Path, dry_run: bool = typer.Option(False, "--dry-r @summary_app.command("month") -def summary_month(month: str | None = typer.Option(None, "--month")) -> None: +def summary_month( + month: str | None = typer.Option(None, "--month"), + currency: str = typer.Option("GBP", "--currency"), +) -> None: engine, services = open_services(get_settings()) try: - summary = services.analytics.monthly_summary(parse_month(month)) - table = Table(title=f"PFA summary {summary.period}") + summary = services.analytics.monthly_summary(parse_month(month), currency=currency) + table = Table(title=f"PFA summary {summary.period} ({summary.currency})") table.add_column("Measure") table.add_column("Amount", justify="right") for label, value in ( @@ -118,7 +125,7 @@ def summary_month(month: str | None = typer.Option(None, "--month")) -> None: ("Investments", summary.investments_minor), ("Net cashflow", summary.net_cashflow_minor), ): - table.add_row(label, print_money(value)) + table.add_row(label, print_money(value, currency=summary.currency)) table.add_row("Savings rate", f"{summary.savings_rate_percent:.2f}%") console.print(table) finally: @@ -229,15 +236,88 @@ def ask(question: str) -> None: @app.command("review") -def review_month(month: str | None = typer.Option(None, "--month")) -> None: +def review_month( + month: str | None = typer.Option(None, "--month"), + currency: str = typer.Option("GBP", "--currency"), +) -> None: engine, services = open_services(get_settings()) try: - evidence = monthly_review_evidence(services.analytics, parse_month(month)) + evidence = monthly_review_evidence( + services.analytics, parse_month(month), currency=currency + ) console.print_json(data=evidence) finally: close_services(engine, services) +@fx_app.command("set") +def fx_set( + base_currency: str, + quote_currency: str, + rate: str, + date_str: str | None = typer.Option(None, "--date", "--on"), +) -> None: + try: + rate_decimal = Decimal(rate) + except InvalidOperation as exc: + raise typer.BadParameter(f"invalid rate {rate!r}") from exc + effective_at = date.fromisoformat(date_str) if date_str else date.today() + engine, services = open_services(get_settings()) + try: + services.uow.fx_rates.set_rate( + base_currency.upper(), + quote_currency.upper(), + rate_decimal, + effective_at=effective_at, + ) + close_services(engine, services) + pair = f"{base_currency.upper()}/{quote_currency.upper()}" + console.print(f"FX rate {pair} = {rate} set for {effective_at}") + except Exception: + close_services(engine, services, False) + raise + + +@fx_app.command("fetch") +def fx_fetch( + base: str = typer.Option("GBP", "--base"), + date_str: str | None = typer.Option(None, "--date", "--on"), +) -> None: + effective_at = date.fromisoformat(date_str) if date_str else date.today() + engine, services = open_services(get_settings()) + try: + stored = fetch_and_store_fx_rates( + services.uow, base_currency=base.upper(), on_date=effective_at + ) + close_services(engine, services) + console.print( + f"Fetched and stored {len(stored)} rates for {base.upper()} on {effective_at}" + ) + except Exception: + close_services(engine, services, False) + raise + + +@fx_app.command("list") +def fx_list() -> None: + engine, services = open_services(get_settings()) + try: + table = Table(title="FX Rates") + for column in ("Base", "Quote", "Rate", "Effective Date", "Source"): + table.add_column(column) + for row in services.uow.fx_rates.all(): + table.add_row( + row.base_currency, + row.quote_currency, + f"{Decimal(row.rate):.6f}", + row.effective_at.isoformat(), + row.source or "manual", + ) + console.print(table) + finally: + close_services(engine, services) + + @budget_app.command("show") def budget_show(month: str | None = typer.Option(None, "--month")) -> None: engine, services = open_services(get_settings()) diff --git a/src/pfa/config.py b/src/pfa/config.py index 4b2686b..8e3b47f 100644 --- a/src/pfa/config.py +++ b/src/pfa/config.py @@ -11,6 +11,7 @@ class Settings(BaseSettings): model_config = SettingsConfigDict(env_prefix="PFA_", env_file=".env", extra="ignore") database_url: str = "sqlite:///data/pfa.db" + base_currency: str = "GBP" ollama_base_url: str = "http://localhost:11434" model: str = "qwen3.5:4b" log_level: str = "INFO" diff --git a/src/pfa/db/models.py b/src/pfa/db/models.py index 12d3040..dd6e60f 100644 --- a/src/pfa/db/models.py +++ b/src/pfa/db/models.py @@ -125,3 +125,19 @@ class MerchantRuleModel(Base): category: Mapped[str | None] = mapped_column(String(40), nullable=True) transfer_purpose: Mapped[str | None] = mapped_column(String(30), nullable=True) created_from_user_correction: Mapped[bool] = mapped_column(Boolean, default=False) + + +class FxRateModel(Base): + __tablename__ = "fx_rates" + __table_args__ = ( + UniqueConstraint( + "base_currency", "quote_currency", "effective_at", name="uq_fx_rates_base_quote_date" + ), + ) + id: Mapped[int] = mapped_column(primary_key=True) + base_currency: Mapped[str] = mapped_column(String(3)) + quote_currency: Mapped[str] = mapped_column(String(3)) + rate: Mapped[str] = mapped_column(String(32)) + effective_at: Mapped[date] = mapped_column(Date, index=True) + source: Mapped[str] = mapped_column(String(50), default="manual") + retrieved_at: Mapped[datetime] = mapped_column(DateTime, server_default=func.now()) diff --git a/src/pfa/db/repositories.py b/src/pfa/db/repositories.py index cd4f446..f846d4f 100644 --- a/src/pfa/db/repositories.py +++ b/src/pfa/db/repositories.py @@ -1,6 +1,7 @@ from __future__ import annotations -from datetime import date, datetime +from datetime import UTC, date, datetime +from decimal import Decimal from sqlalchemy import select from sqlalchemy.orm import Session @@ -10,6 +11,7 @@ from .models import ( AccountModel, BudgetModel, + FxRateModel, GoalModel, ImportBatchModel, MerchantRuleModel, @@ -76,6 +78,11 @@ def get_or_create( self.session.flush() return account + def get_by_name(self, name: str) -> AccountModel | None: + """Read-only lookup - never creates a row, so a preview never has the side effect + of persisting an account for a batch that might still be discarded.""" + return self.session.scalar(select(AccountModel).where(AccountModel.name == name)) + def all(self) -> list[AccountModel]: return list(self.session.scalars(select(AccountModel).order_by(AccountModel.name))) @@ -149,3 +156,135 @@ def add(self, goal: GoalModel) -> GoalModel: self.session.add(goal) self.session.flush() return goal + + +class FxRateRepository: + def __init__(self, session: Session): + self.session = session + + def all(self) -> list[FxRateModel]: + return list( + self.session.scalars(select(FxRateModel).order_by(FxRateModel.effective_at.desc())) + ) + + def add(self, fx_rate: FxRateModel) -> FxRateModel: + self.session.add(fx_rate) + self.session.flush() + return fx_rate + + def set_rate( + self, + base_currency: str, + quote_currency: str, + rate: Decimal | str | float, + effective_at: date, + source: str = "manual", + ) -> FxRateModel: + base = base_currency.upper() + quote = quote_currency.upper() + rate_str = str(rate) + now = datetime.now(UTC).replace(tzinfo=None) + statement = select(FxRateModel).where( + FxRateModel.base_currency == base, + FxRateModel.quote_currency == quote, + FxRateModel.effective_at == effective_at, + ) + existing = self.session.scalar(statement) + if existing is not None: + existing.rate = rate_str + existing.source = source + existing.retrieved_at = now + self.session.flush() + return existing + model = FxRateModel( + base_currency=base, + quote_currency=quote, + rate=rate_str, + effective_at=effective_at, + source=source, + retrieved_at=now, + ) + self.session.add(model) + self.session.flush() + return model + + def rate_on( + self, effective_date: date, base: str, quote: str + ) -> tuple[Decimal, FxRateModel | None] | None: + """Finds nearest rate at or before effective_date (never after). + Returns (rate_decimal, matched_model_or_none). + """ + base_upper = base.upper() + quote_upper = quote.upper() + if base_upper == quote_upper: + return Decimal("1.0"), None + + # Direct rate lookup: 1 base = rate quote + direct_stmt = ( + select(FxRateModel) + .where( + FxRateModel.base_currency == base_upper, + FxRateModel.quote_currency == quote_upper, + FxRateModel.effective_at <= effective_date, + ) + .order_by(FxRateModel.effective_at.desc()) + .limit(1) + ) + direct = self.session.scalar(direct_stmt) + if direct is not None: + return Decimal(direct.rate), direct + + # Inverse rate lookup: 1 quote = rate base => 1 base = 1 / rate quote + inverse_stmt = ( + select(FxRateModel) + .where( + FxRateModel.base_currency == quote_upper, + FxRateModel.quote_currency == base_upper, + FxRateModel.effective_at <= effective_date, + ) + .order_by(FxRateModel.effective_at.desc()) + .limit(1) + ) + inverse = self.session.scalar(inverse_stmt) + if inverse is not None: + inv_rate = Decimal(inverse.rate) + if inv_rate != Decimal(0): + return Decimal(1) / inv_rate, inverse + + return None + + def latest(self, base: str, quote: str) -> tuple[Decimal, FxRateModel | None] | None: + base_upper = base.upper() + quote_upper = quote.upper() + if base_upper == quote_upper: + return Decimal("1.0"), None + + direct_stmt = ( + select(FxRateModel) + .where( + FxRateModel.base_currency == base_upper, + FxRateModel.quote_currency == quote_upper, + ) + .order_by(FxRateModel.effective_at.desc()) + .limit(1) + ) + direct = self.session.scalar(direct_stmt) + if direct is not None: + return Decimal(direct.rate), direct + + inverse_stmt = ( + select(FxRateModel) + .where( + FxRateModel.base_currency == quote_upper, + FxRateModel.quote_currency == base_upper, + ) + .order_by(FxRateModel.effective_at.desc()) + .limit(1) + ) + inverse = self.session.scalar(inverse_stmt) + if inverse is not None: + inv_rate = Decimal(inverse.rate) + if inv_rate != Decimal(0): + return Decimal(1) / inv_rate, inverse + + return None diff --git a/src/pfa/db/unit_of_work.py b/src/pfa/db/unit_of_work.py index 4fe300b..bb1dcb0 100644 --- a/src/pfa/db/unit_of_work.py +++ b/src/pfa/db/unit_of_work.py @@ -3,6 +3,7 @@ from .repositories import ( AccountRepository, BudgetRepository, + FxRateRepository, GoalRepository, ImportBatchRepository, RuleRepository, @@ -21,3 +22,4 @@ def __init__(self, session: Session): self.budgets = BudgetRepository(session) self.goals = GoalRepository(session) self.import_batches = ImportBatchRepository(session) + self.fx_rates = FxRateRepository(session) diff --git a/src/pfa/domain/fx.py b/src/pfa/domain/fx.py new file mode 100644 index 0000000..d015b9a --- /dev/null +++ b/src/pfa/domain/fx.py @@ -0,0 +1,64 @@ +from __future__ import annotations + +from dataclasses import dataclass +from datetime import UTC, date, datetime +from decimal import Decimal +from typing import TYPE_CHECKING + +from pfa.domain.errors import ValidationError +from pfa.domain.money import Money + +if TYPE_CHECKING: + from pfa.db.repositories import FxRateRepository + + +@dataclass(frozen=True, slots=True) +class FxRate: + base_currency: str + quote_currency: str + rate: Decimal + effective_at: date + source: str + retrieved_at: datetime + + +def to_base( + money: Money, + on_date: date, + fx_rates: FxRateRepository, + base_currency: str = "GBP", +) -> tuple[Money, FxRate]: + """Converts a Money instance to base currency as of a specific date. + Returns (converted_money, applied_fx_rate). + """ + target_curr = base_currency.upper() + if money.currency == target_curr: + identity_rate = FxRate( + base_currency=target_curr, + quote_currency=target_curr, + rate=Decimal("1.0"), + effective_at=on_date, + source="identity", + retrieved_at=datetime.now(UTC).replace(tzinfo=None), + ) + return money, identity_rate + + rate_info = fx_rates.rate_on(on_date, base=money.currency, quote=target_curr) + if rate_info is None: + raise ValidationError( + f"No FX rate available to convert {money.currency} to {target_curr} " + f"on or before {on_date}" + ) + + rate_dec, model = rate_info + target_major = money.to_major() * rate_dec + converted_money = Money.from_major(target_major, target_curr) + applied_rate = FxRate( + base_currency=money.currency, + quote_currency=target_curr, + rate=rate_dec, + effective_at=model.effective_at if model else on_date, + source=model.source if model else "direct", + retrieved_at=model.retrieved_at if model else datetime.now(UTC).replace(tzinfo=None), + ) + return converted_money, applied_rate diff --git a/src/pfa/domain/money.py b/src/pfa/domain/money.py index c81c5c7..33984b9 100644 --- a/src/pfa/domain/money.py +++ b/src/pfa/domain/money.py @@ -5,6 +5,30 @@ from .errors import ValidationError +SUPPORTED_CURRENCIES: dict[str, int] = { + "GBP": 2, + "INR": 2, + "USD": 2, + "EUR": 2, + "JPY": 0, +} + + +def minor_units(value: str | Decimal | int | float, currency: str = "GBP") -> int: + """Converts a major-unit amount to an integer minor-unit count for `currency`. + + An unrecognised code falls back to a 2-place exponent rather than raising, so a row's + amount can always be parsed before its currency is validated as supported - the two are + separate checks and the caller decides which error the row surfaces. + """ + exponent = SUPPORTED_CURRENCIES.get(currency.upper(), 2) + quantize_unit = Decimal("1") if exponent == 0 else Decimal("0." + "0" * (exponent - 1) + "1") + try: + amount = Decimal(str(value)).quantize(quantize_unit, rounding=ROUND_HALF_UP) + except (InvalidOperation, ValueError) as exc: + raise ValidationError(f"Invalid monetary value: {value!r}") from exc + return int(amount * (10**exponent)) + @dataclass(frozen=True, slots=True) class Money: @@ -16,18 +40,20 @@ def __post_init__(self) -> None: raise ValidationError("Money must use integer minor units") if len(self.currency) != 3 or not self.currency.isalpha(): raise ValidationError("Currency must be a three-letter code") - object.__setattr__(self, "currency", self.currency.upper()) + curr = self.currency.upper() + if curr not in SUPPORTED_CURRENCIES: + supported = ", ".join(sorted(SUPPORTED_CURRENCIES)) + raise ValidationError(f"Unsupported currency {curr!r}; supported: {supported}") + object.__setattr__(self, "currency", curr) @classmethod def from_major(cls, value: str | Decimal | int | float, currency: str = "GBP") -> Money: - try: - amount = Decimal(str(value)).quantize(Decimal("0.01"), rounding=ROUND_HALF_UP) - except (InvalidOperation, ValueError) as exc: - raise ValidationError(f"Invalid monetary value: {value!r}") from exc - return cls(int(amount * 100), currency) + curr = currency.upper() if isinstance(currency, str) else "GBP" + return cls(minor_units(value, curr), curr) def to_major(self) -> Decimal: - return Decimal(self.minor) / 100 + exponent = SUPPORTED_CURRENCIES.get(self.currency, 2) + return Decimal(self.minor) / Decimal(10**exponent) def __add__(self, other: Money) -> Money: self._same_currency(other) diff --git a/src/pfa/ingestion/batches.py b/src/pfa/ingestion/batches.py index fa17c8b..c5f2e40 100644 --- a/src/pfa/ingestion/batches.py +++ b/src/pfa/ingestion/batches.py @@ -14,13 +14,14 @@ import json import logging import uuid +from collections import Counter from dataclasses import asdict, dataclass -from datetime import UTC, datetime, timedelta +from datetime import UTC, date, datetime, timedelta from pfa.config import Settings from pfa.db.models import ImportBatchModel from pfa.db.unit_of_work import UnitOfWork -from pfa.domain.errors import BatchError +from pfa.domain.errors import BatchError, ImportRowError from pfa.ingestion.service import ImportService from .candidates import ( @@ -34,6 +35,7 @@ EXTRACTION_FAILED, EXTRACTION_TIMEOUT, NO_USABLE_ROWS, + STATEMENT_YEAR_INFERRED, TOO_MANY_ROWS, VALID, WARNING, @@ -44,7 +46,10 @@ StatementSource, candidates_from_json, candidates_to_json, + is_year_bearing_date, + parse_date, ) +from .dialects import Dialect, dialect_for_name from .extractors.csv import CsvStatementExtractor from .extractors.ocr import OcrFallbackPdfExtractor from .extractors.pdf import clean_amount_text @@ -102,19 +107,23 @@ def batch_committed_transaction_ids(batch: ImportBatchModel) -> list[int]: return list(json.loads(batch.committed_transaction_ids_json)) -def _extractor_for(source: StatementSource, settings: Settings) -> StatementExtractor: - """Picks the extractor from the extension the upload policy already validated. - - PDFs always go through the OCR-fallback wrapper: it runs native extraction first and - only reaches for Tesseract on pages that have no usable text of their own. - """ +def _extractor_for( + source: StatementSource, + settings: Settings, + account_name: str | None = None, + account_currency: str = "GBP", +) -> StatementExtractor: + """Picks the extractor from the extension the upload policy already validated.""" + dialect = dialect_for_name(account_name) if source.path.suffix.lower() == ".pdf": return OcrFallbackPdfExtractor( settings=settings, max_pdf_pages=settings.max_pdf_pages, max_candidate_rows=settings.max_candidate_rows, + dialect=dialect, + currency=account_currency, ) - return CsvStatementExtractor() + return CsvStatementExtractor(dialect=dialect, currency=account_currency) def _run_extraction( @@ -143,6 +152,51 @@ def _fail(batch: ImportBatchModel, uow: UnitOfWork, code: str, message: str) -> return uow.import_batches.add(batch) +def _normalize_dates( + candidates: list[CandidateTransaction], dialect: Dialect +) -> CandidateIssue | None: + """Resolves every year-less date (`Jul31`, `21 Jul`) against the year the rest of this + statement's dates carry, then rewrites the candidate's date string to ISO so every later + parse - validation, commit - sees that same resolved year, never whatever year the + import happens to run in. + + Returns a warning issue when no row in the statement carried a year of its own, so the + fallback to today's year is visible in the preview rather than silent. + """ + years_seen: list[int] = [] + for candidate in candidates: + text = candidate.transaction_date + if text and is_year_bearing_date(text, dialect.date_order): + try: + years_seen.append(parse_date(text, dialect.date_order).year) + except ImportRowError: + continue + inferred_year = Counter(years_seen).most_common(1)[0][0] if years_seen else date.today().year + + used_fallback = False + for candidate in candidates: + for attr in ("transaction_date", "posted_date"): + text = getattr(candidate, attr) + if not text: + continue + try: + resolved = parse_date(text, dialect.date_order, inferred_year) + except ImportRowError: + continue + if not is_year_bearing_date(text, dialect.date_order): + used_fallback = True + setattr(candidate, attr, resolved.isoformat()) + + if years_seen or not used_fallback: + return None + return CandidateIssue( + STATEMENT_YEAR_INFERRED, + f"no date in this statement carried its own year; {inferred_year} was assumed for " + "year-less dates - check the preview before committing", + WARNING, + ) + + def create_batch( uow: UnitOfWork, source: StatementSource, @@ -151,7 +205,17 @@ def create_batch( account: str | None = None, ) -> ImportBatchModel: now = _now() - extractor = _extractor_for(source, settings) + account_currency = "GBP" + if account: + existing_acc = uow.accounts.get_by_name(account) + if existing_acc is not None: + account_currency = existing_acc.currency + + extractor = _extractor_for( + source, settings, account_name=account, account_currency=account_currency + ) + dialect = dialect_for_name(account) + batch = ImportBatchModel( id=uuid.uuid4().hex, original_filename=source.original_filename, @@ -161,6 +225,7 @@ def create_batch( extractor=extractor.name, status="extracting", destination_account=account, + amount_sign=dialect.default_sign, issues_json="[]", counts_json=json.dumps(_counts([])), created_at=now, @@ -190,15 +255,34 @@ def create_batch( ) ) + year_issue = _normalize_dates(candidates, dialect) + if year_issue: + extraction.issues.append(year_issue) + service = ImportService(uow) service.validate(candidates) + if batch.amount_sign: + for candidate in candidates: + _apply_amount_sign(candidate, batch.amount_sign) service.resolve_duplicates(candidates) if not candidates and not any(issue.severity == ERROR for issue in extraction.issues): extraction.issues.append(CandidateIssue(NO_USABLE_ROWS, "no transactions were found")) + parsed_dates: list[date] = [] + for candidate in candidates: + if not candidate.transaction_date: + continue + try: + parsed_dates.append(date.fromisoformat(candidate.transaction_date)) + except ValueError: + continue + if parsed_dates: + batch.statement_start = min(parsed_dates) + batch.statement_end = max(parsed_dates) + batch.detected_account = extraction.detected_account - batch.detected_currency = extraction.detected_currency + batch.detected_currency = extraction.detected_currency or account_currency batch.page_count = extraction.page_count blocked = any(issue.severity == ERROR for issue in extraction.issues) batch.status = "blocked" if blocked else "preview_ready" @@ -253,11 +337,14 @@ def _apply_amount_sign(candidate: CandidateTransaction, convention: str) -> None The direction is re-derived from the raw text rather than flipped, so sending a convention twice - or switching back - always lands on the same answer. Rows whose - source stated the direction in its own debit/credit column are left alone: their - convention is not in doubt, and the extractor already resolved it. + source stated the direction in its own debit/credit column - or an explicit CR/CREDIT + marker, own-line or inline - are left alone: their convention is not in doubt, and the + extractor already resolved it. """ if candidate.direction is None: return + if candidate.direction_explicit: + return if "debit" in candidate.raw_fields or "credit" in candidate.raw_fields: return amount = candidate.raw_fields.get("amount", "") diff --git a/src/pfa/ingestion/candidates.py b/src/pfa/ingestion/candidates.py index f3a8b2e..ce384f8 100644 --- a/src/pfa/ingestion/candidates.py +++ b/src/pfa/ingestion/candidates.py @@ -16,7 +16,7 @@ from typing import Protocol from pfa.domain.errors import ImportRowError -from pfa.domain.money import Money +from pfa.domain.money import minor_units ERROR = "error" WARNING = "warning" @@ -28,6 +28,8 @@ INVALID_AMOUNT = "INVALID_AMOUNT" AMBIGUOUS_SIGN = "AMBIGUOUS_SIGN" UNSUPPORTED_CURRENCY = "UNSUPPORTED_CURRENCY" +CURRENCY_ACCOUNT_MISMATCH = "CURRENCY_ACCOUNT_MISMATCH" +STATEMENT_YEAR_INFERRED = "STATEMENT_YEAR_INFERRED" UNKNOWN_KIND = "UNKNOWN_KIND" UNKNOWN_CATEGORY = "UNKNOWN_CATEGORY" UNKNOWN_TRANSFER_PURPOSE = "UNKNOWN_TRANSFER_PURPOSE" @@ -83,22 +85,110 @@ def match_header_alias(cell_text: str) -> str | None: return None -def parse_date(value: str) -> date: - for pattern in ("%Y-%m-%d", "%d/%m/%Y", "%d-%m-%Y", "%m/%d/%Y"): +_YEARLESS_DATE_PATTERNS: tuple[str, ...] = ("%b%d", "%b %d", "%d %b") + + +def _year_bearing_date_patterns(date_order: str) -> tuple[str, ...]: + if date_order == "month_first": + return ( + "%Y-%m-%d", + "%m/%d/%Y", + "%m-%d-%Y", + "%d/%m/%Y", + "%d-%m-%Y", + "%b %d %Y", + "%b %d, %Y", + "%d %b %Y", + "%d %b %y", + "%d/%m/%y", + "%m/%d/%y", + ) + return ( + "%Y-%m-%d", + "%d/%m/%Y", + "%d-%m-%Y", + "%d %b %Y", + "%d %b %y", + "%b %d %Y", + "%b %d, %Y", + "%d/%m/%y", + "%m/%d/%Y", + "%m-%d-%Y", + ) + + +def is_year_bearing_date(value: str, date_order: str = "day_first") -> bool: + """True when `value` carries its own year, rather than needing one assumed for it.""" + cleaned = value.strip() + for pattern in _year_bearing_date_patterns(date_order): try: - return datetime.strptime(value, pattern).date() + datetime.strptime(cleaned, pattern) + return True except ValueError: continue + return False + + +def parse_date( + value: str, + date_order: str = "day_first", + statement_year: int | None = None, +) -> date: + cleaned = value.strip() + for pattern in _year_bearing_date_patterns(date_order): + try: + return datetime.strptime(cleaned, pattern).date() + except ValueError: + continue + + # Year-less format attempts like 'Jul31', 'Jul 31', '21 Jul'. `statement_year` should + # always come from other year-bearing dates in the same statement (see + # ingestion.batches._normalize_dates) - falling back to today's year here is a last + # resort for a caller that never supplied one. + year = statement_year or date.today().year + for pattern in _YEARLESS_DATE_PATTERNS: + try: + dt = datetime.strptime(cleaned, pattern) + return dt.replace(year=year).date() + except ValueError: + continue + raise ImportRowError(f"invalid date {value!r}") -def parse_amount(value: str) -> tuple[int, int]: +def parse_amount(value: str, currency: str = "GBP") -> tuple[int, int, bool]: + """Parses a signed amount. Returns (sign, minor_units, was_an_explicit_credit_marker). + + The third element tells the caller the row's direction came from a CR/CREDIT marker in + the text itself, not from the statement's general sign convention - so a later + convention choice (e.g. "debit positive") must never override it. + """ + cleaned = ( + value.replace(",", "") + .replace("£", "") + .replace("$", "") + .replace("€", "") + .replace("₹", "") + .replace("�", "") + .strip() + ) + is_cr = False + upper = cleaned.upper() + if upper.endswith("CR."): + cleaned = cleaned[:-3].strip() + is_cr = True + elif upper.endswith("CR"): + cleaned = cleaned[:-2].strip() + is_cr = True + elif upper.startswith("CR"): + cleaned = cleaned[2:].strip() + is_cr = True try: - decimal = Decimal(value.replace(",", "").replace("£", "").strip()) + decimal = Decimal(cleaned) except InvalidOperation as exc: raise ImportRowError(f"invalid amount {value!r}") from exc - sign = -1 if decimal < 0 else 1 - return sign, Money.from_major(abs(decimal)).minor + sign = 1 if is_cr else (-1 if decimal < 0 else 1) + return sign, minor_units(abs(decimal), currency), is_cr @dataclass(frozen=True, slots=True) @@ -128,6 +218,10 @@ class CandidateTransaction: normalized_description: str = "" amount_minor: int | None = None # absolute magnitude, matches TransactionModel direction: str | None = None # "debit" | "credit" + # True once `direction` was read from an explicit marker (a CR/CREDIT suffix, or a + # debit/credit column) rather than the statement's general sign convention. A later + # amount-sign convention choice must never overwrite a row already resolved this way. + direction_explicit: bool = False currency: str = "GBP" account_hint: str | None = None external_id: str | None = None diff --git a/src/pfa/ingestion/dialects.py b/src/pfa/ingestion/dialects.py new file mode 100644 index 0000000..af70064 --- /dev/null +++ b/src/pfa/ingestion/dialects.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +from dataclasses import dataclass, replace +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + pass + + +@dataclass(frozen=True, slots=True) +class Dialect: + name: str = "generic" + date_formats: tuple[str, ...] = ( + "%Y-%m-%d", + "%d/%m/%Y", + "%d-%m-%Y", + "%d %b %Y", + "%d %b %y", + "%b %d %Y", + "%b %d, %Y", + "%d/%m/%y", + ) + date_order: str = "day_first" # "day_first" | "month_first" + credit_markers: tuple[str, ...] = ("CR", "CREDIT", "CR.") + default_sign: str | None = None # None = ask user, "debit_positive", "as_written" + two_column: bool = False + + +GENERIC = Dialect() + +HSBC = replace( + GENERIC, + name="hsbc", + date_formats=GENERIC.date_formats + ("%d %b %y", "%d %b %Y"), + credit_markers=("CR", "CREDIT"), +) + +AMEX_CARD = replace( + GENERIC, + name="amex", + date_formats=("%b%d", "%b %d", "%d %b %y", "%d %b %Y") + GENERIC.date_formats, + default_sign="debit_positive", +) + +BARCLAYCARD = replace( + GENERIC, + name="barclaycard", + date_formats=GENERIC.date_formats + ("%d %b %y", "%d %b %Y"), + two_column=True, +) + +DIALECTS: dict[str, Dialect] = { + "generic": GENERIC, + "hsbc": HSBC, + "amex": AMEX_CARD, + "barclaycard": BARCLAYCARD, +} + + +def dialect_for_name(name: str | None) -> Dialect: + if not name: + return GENERIC + clean = name.strip().lower() + for key, dialect in DIALECTS.items(): + if key in clean: + return dialect + return GENERIC diff --git a/src/pfa/ingestion/extractors/csv.py b/src/pfa/ingestion/extractors/csv.py index 605d57e..ec6e74b 100644 --- a/src/pfa/ingestion/extractors/csv.py +++ b/src/pfa/ingestion/extractors/csv.py @@ -20,6 +20,7 @@ parse_amount, parse_date, ) +from pfa.ingestion.dialects import GENERIC, Dialect DATE_ALIASES = HEADER_ALIASES["date"] DESCRIPTION_ALIASES = HEADER_ALIASES["description"] @@ -79,7 +80,9 @@ def _is_headerless(cells: list[str]) -> bool: ) -def read_csv_rows(path: Path) -> Iterator[dict[str, str]]: +def read_csv_rows( + path: Path, default_currency: str = "GBP", dialect: Dialect = GENERIC +) -> Iterator[dict[str, str]]: delimiter = _delimiter(path) with path.open(newline="", encoding="utf-8-sig") as handle: reader = csv.DictReader(handle, delimiter=delimiter) @@ -102,7 +105,7 @@ def read_csv_rows(path: Path) -> Iterator[dict[str, str]]: "amount": _value(row, *AMOUNT_ALIASES), "debit": _value(row, *DEBIT_ALIASES), "credit": _value(row, *CREDIT_ALIASES), - "currency": _value(row, "currency") or "GBP", + "currency": _value(row, "currency") or default_currency or "GBP", "kind": _value(row, "kind", "transaction_kind"), "category": _value(row, "category"), "transfer_purpose": _value(row, "transfer_purpose"), @@ -159,11 +162,18 @@ class CsvStatementExtractor: name = "csv/1" + def __init__(self, dialect: Dialect = GENERIC, currency: str = "GBP") -> None: + self.dialect = dialect + self.currency = currency + def extract(self, source: StatementSource) -> ExtractionResult: result = ExtractionResult(extractor=self.name) positional = False try: - for index, row in enumerate(read_csv_rows(source.path), start=1): + for index, row in enumerate( + read_csv_rows(source.path, default_currency=self.currency, dialect=self.dialect), + start=1, + ): positional = positional or bool(row["_positional"]) result.candidates.append(_candidate(f"c{index}", row)) except ImportRowError as exc: diff --git a/src/pfa/ingestion/extractors/ocr.py b/src/pfa/ingestion/extractors/ocr.py index 18bebd4..2b131eb 100644 --- a/src/pfa/ingestion/extractors/ocr.py +++ b/src/pfa/ingestion/extractors/ocr.py @@ -32,6 +32,7 @@ ExtractionResult, StatementSource, ) +from pfa.ingestion.dialects import GENERIC, Dialect from pfa.ingestion.extractors.pdf import PdfStatementExtractor, Word @@ -204,6 +205,8 @@ def __init__( runner: TesseractRunner | None = None, max_pdf_pages: int | None = None, max_candidate_rows: int | None = None, + dialect: Dialect = GENERIC, + currency: str = "GBP", ) -> None: settings = settings or get_settings() word_provider: Callable[[Page], list[Word]] @@ -227,6 +230,8 @@ def __init__( max_candidate_rows=max_candidate_rows, word_provider=word_provider, ocr_min_confidence=settings.ocr_min_confidence, + dialect=dialect, + currency=currency, ) def extract(self, source: StatementSource) -> ExtractionResult: diff --git a/src/pfa/ingestion/extractors/pdf.py b/src/pfa/ingestion/extractors/pdf.py index c002341..4e5d6d6 100644 --- a/src/pfa/ingestion/extractors/pdf.py +++ b/src/pfa/ingestion/extractors/pdf.py @@ -10,7 +10,6 @@ from collections.abc import Callable from dataclasses import dataclass, field -from datetime import datetime from decimal import Decimal, InvalidOperation from typing import Any @@ -20,7 +19,7 @@ from pdfplumber.pdf import PDF from pfa.config import get_settings -from pfa.domain.money import Money +from pfa.domain.money import minor_units from pfa.ingestion.candidates import ( AMBIGUOUS_SIGN, ERROR, @@ -36,7 +35,9 @@ ExtractionResult, StatementSource, match_header_alias, + parse_date, ) +from pfa.ingestion.dialects import GENERIC, Dialect # ponytail: max_candidate_rows is T3's setting (src/pfa/config.py, landing in a parallel # branch). Mirrors the plan's stated default until that lands; swap for @@ -44,7 +45,6 @@ _DEFAULT_MAX_CANDIDATE_ROWS = 10_000 _AMOUNT_FIELDS = ("amount", "debit", "credit") -_DATE_PATTERNS = ("%Y-%m-%d", "%d/%m/%Y", "%d-%m-%Y", "%m/%d/%Y") _LINE_TOLERANCE = 3.0 # points; words within this many points of `top` share a line _CELL_GAP = 10.0 # points; a horizontal gap larger than this starts a new cell/column @@ -168,13 +168,69 @@ def _assign_cells( return fields, field_conf -def _word_rows(words: list[Word], page_number: int) -> tuple[list[_RawRow], float | None]: - """Returns the page's data rows plus the header line's `top` (or None if no header). +def _is_date_text(text: str, dialect: Dialect = GENERIC) -> bool: + cleaned = text.strip() + if not cleaned: + return False + try: + parse_date(cleaned, date_order=dialect.date_order) + return True + except Exception: + return False - The header-to-first-data-row gap is a reliable one-line baseline for the continuation - check below, even on a page with too few data rows to measure a gap between two of - them. + +def _is_amount_text(text: str) -> bool: + cleaned, _ = clean_amount_text(text) + if not cleaned: + return False + try: + Decimal(cleaned) + return True + except Exception: + return False + + +def _is_lone_credit_marker( + cells: list[tuple[float, str, float | None]], dialect: Dialect +) -> str | None: + """The marker text when a line is nothing but a credit marker (own-line `CR`), else None. + + A statement that prints `CR` on its own line - visually attached to the amount above it + but structurally its own row - would otherwise either vanish (no date, no amount pair to + match) or become a spurious candidate with no date of its own. Folding it back onto the + previous row as an explicit marker is what lets `_resolve_amount` read it correctly. """ + if len(cells) != 1: + return None + text = cells[0][1].strip().upper().rstrip(".") + for marker in dialect.credit_markers: + if text == marker.upper().rstrip("."): + return cells[0][1].strip() + return None + + +def _cluster_words_into_columns(words: list[Word]) -> list[list[Word]]: + if not words: + return [] + min_x = min(w["x0"] for w in words) + max_x = max(w["x1"] for w in words) + width = max_x - min_x + if width < 150: + return [words] + split_x = min_x + width * 0.55 + left = [w for w in words if (w["x0"] + w["x1"]) / 2.0 < split_x] + right = [w for w in words if (w["x0"] + w["x1"]) / 2.0 >= split_x] + columns: list[list[Word]] = [] + if left: + columns.append(left) + if right: + columns.append(right) + return columns or [words] + + +def _process_lines_for_column( + words: list[Word], page_number: int, dialect: Dialect = GENERIC +) -> tuple[list[_RawRow], float | None]: lines = _group_lines(words) columns: list[tuple[float, str]] | None = None header_top: float | None = None @@ -182,11 +238,39 @@ def _word_rows(words: list[Word], page_number: int) -> tuple[list[_RawRow], floa position = 0 for line in lines: cells = _split_cells(line) + marker_text = _is_lone_credit_marker(cells, dialect) + if marker_text is not None and rows: + rows[-1].fields.setdefault("type", marker_text) + rows[-1].raw_text = f"{rows[-1].raw_text} / {marker_text}" + continue if columns is None: columns = _header_columns(cells) if columns is not None: header_top = line[0]["top"] - continue # header line itself, or noise above it - never a data row + continue + if len(cells) >= 2: + first_text = cells[0][1] + last_text = cells[-1][1] + if _is_date_text(first_text, dialect) and _is_amount_text(last_text): + position += 1 + description = " ".join(c[1] for c in cells[1:-1] if c[1] != first_text) + fields = { + "date": first_text, + "description": description, + "amount": last_text, + } + raw_text = " | ".join(text for _, text, _ in cells) + rows.append( + _RawRow( + source_page=page_number, + position=position, + top=line[0]["top"], + fields=fields, + raw_text=raw_text, + is_ocr=any("conf" in word for word in line), + ) + ) + continue position += 1 fields, field_conf = _assign_cells(cells, columns) raw_text = " | ".join(text for _, text, _ in cells) @@ -204,23 +288,41 @@ def _word_rows(words: list[Word], page_number: int) -> tuple[list[_RawRow], floa return rows, header_top -def _has_parseable_date(fields: dict[str, str]) -> bool: +def _word_rows( + words: list[Word], page_number: int, dialect: Dialect = GENERIC +) -> tuple[list[_RawRow], float | None]: + """Returns the page's data rows plus the header line's `top` (or None if no header).""" + if dialect.two_column: + cols = _cluster_words_into_columns(words) + all_rows: list[_RawRow] = [] + first_header: float | None = None + for col_words in cols: + rows, header_top = _process_lines_for_column(col_words, page_number, dialect) + if rows: + all_rows.extend(rows) + if first_header is None: + first_header = header_top + return all_rows, first_header + return _process_lines_for_column(words, page_number, dialect) + + +def _has_parseable_date(fields: dict[str, str], dialect: Dialect = GENERIC) -> bool: value = fields.get("date", "").strip() - for pattern in _DATE_PATTERNS: - try: - datetime.strptime(value, pattern) - return True - except ValueError: - continue - return False + if not value: + return False + try: + parse_date(value, date_order=dialect.date_order) + return True + except Exception: + return False def _has_parseable_amount(fields: dict[str, str]) -> bool: return any(_signed_minor(fields.get(field, "")) is not None for field in _AMOUNT_FIELDS) -def _is_plausible_data_row(row: _RawRow) -> bool: - return _has_parseable_date(row.fields) and _has_parseable_amount(row.fields) +def _is_plausible_data_row(row: _RawRow, dialect: Dialect = GENERIC) -> bool: + return _has_parseable_date(row.fields, dialect) and _has_parseable_amount(row.fields) def _has_filled_transaction_cell(row: _RawRow) -> bool: @@ -230,12 +332,6 @@ def _has_filled_transaction_cell(row: _RawRow) -> bool: def _line_height(rows: list[_RawRow], header_top: float | None) -> float: - """The smallest line-to-line gap on the page - a good proxy for one text line. - - Using the minimum (rather than e.g. the median) keeps a single large gap - the very - thing a continuation check needs to measure against - from inflating the baseline. - The header-to-first-row gap is included as a reliable one-line reference point. - """ tops = [row.top for row in rows if row.top is not None] if header_top is not None: tops = [header_top, *tops] @@ -243,9 +339,13 @@ def _line_height(rows: list[_RawRow], header_top: float | None) -> float: return min(diffs) if diffs else _DEFAULT_LINE_HEIGHT -def _merge_continuations(rows: list[_RawRow], header_top: float | None) -> list[_RawRow]: +def _merge_continuations( + rows: list[_RawRow], header_top: float | None, dialect: Dialect = GENERIC +) -> list[_RawRow]: """Joins structurally empty wrapped description lines into the row above them.""" - has_plausible_row = any(row.top is not None and _is_plausible_data_row(row) for row in rows) + has_plausible_row = any( + row.top is not None and _is_plausible_data_row(row, dialect) for row in rows + ) threshold = _line_height(rows, header_top) * _CONTINUATION_FACTOR kept: list[_RawRow] = [] last: _RawRow | None = None @@ -253,7 +353,7 @@ def _merge_continuations(rows: list[_RawRow], header_top: float | None) -> list[ has_description = bool(row.fields.get("description", "").strip()) if ( row.top is None - or _is_plausible_data_row(row) + or _is_plausible_data_row(row, dialect) or (_has_filled_transaction_cell(row) and (has_plausible_row or has_description)) ): kept.append(row) @@ -265,7 +365,7 @@ def _merge_continuations(rows: list[_RawRow], header_top: float | None) -> list[ existing = last.fields.get("description", "") last.fields["description"] = f"{existing} {joined}".strip() last.raw_text = f"{last.raw_text} / {row.raw_text}" - last.top = row.top # chain distance from the most recently joined line + last.top = row.top return kept @@ -273,6 +373,7 @@ def _merge_continuations(rows: list[_RawRow], header_top: float | None) -> list[ class _AmountResult: minor: int | None = None direction: str | None = None + direction_explicit: bool = False issue: CandidateIssue | None = None @@ -289,13 +390,23 @@ def clean_amount_text(text: str) -> tuple[str, bool]: if cleaned.startswith("-") or cleaned.startswith(_UNICODE_MINUS): negative = True cleaned = cleaned[1:].strip() - for char in _CURRENCY_CHARS: + for char in _CURRENCY_CHARS + "₹\ufffd": cleaned = cleaned.replace(char, "") cleaned = cleaned.replace(",", "").replace(_UNICODE_MINUS, "").strip() + upper = cleaned.upper() + if upper.endswith("CR."): + cleaned = cleaned[:-3].strip() + negative = False + elif upper.endswith("CR"): + cleaned = cleaned[:-2].strip() + negative = False + elif upper.startswith("CR"): + cleaned = cleaned[2:].strip() + negative = False return cleaned, negative -def _signed_minor(text: str) -> tuple[int, bool] | None: +def _signed_minor(text: str, currency: str = "GBP") -> tuple[int, bool] | None: cleaned, negative = clean_amount_text(text) if not cleaned: return None @@ -303,26 +414,29 @@ def _signed_minor(text: str) -> tuple[int, bool] | None: decimal = Decimal(cleaned) except InvalidOperation: return None - return Money.from_major(abs(decimal)).minor, negative - + return minor_units(abs(decimal), currency), negative -def _resolve_amount(fields: dict[str, str]) -> _AmountResult: - """Resolves one signed amount. Two disagreeing sign sources block, never guess. - Balance is intentionally never read here - it is provenance only, never a transaction - amount. ponytail: reconciling running balance against amount deltas (the spec allows - this to surface warnings only) is deferred - no test or issue code calls for it yet; - add a RECONCILIATION_MISMATCH warning code and compare deltas here if that's needed. - """ +def _resolve_amount( + fields: dict[str, str], dialect: Dialect = GENERIC, currency: str = "GBP" +) -> _AmountResult: debit_text = fields.get("debit", "").strip() credit_text = fields.get("credit", "").strip() amount_text = fields.get("amount", "").strip() + is_explicit_cr = False + for marker in dialect.credit_markers: + if marker in amount_text.upper() or fields.get("type", "").upper() == marker: + is_explicit_cr = True + break + if amount_text: - parsed = _signed_minor(amount_text) + parsed = _signed_minor(amount_text, currency) if parsed is None: return _AmountResult() minor, negative = parsed + if is_explicit_cr: + return _AmountResult(minor=minor, direction="credit", direction_explicit=True) return _AmountResult(minor=minor, direction="debit" if negative else "credit") if debit_text and credit_text: @@ -335,7 +449,7 @@ def _resolve_amount(fields: dict[str, str]) -> _AmountResult: if debit_text or credit_text: implied_direction = "debit" if debit_text else "credit" - parsed = _signed_minor(debit_text or credit_text) + parsed = _signed_minor(debit_text or credit_text, currency) if parsed is None: return _AmountResult() minor, negative = parsed @@ -346,12 +460,18 @@ def _resolve_amount(fields: dict[str, str]) -> _AmountResult: "credit column holds a negative/parenthesised value; sign cannot be determined", ) ) - return _AmountResult(minor=minor, direction=implied_direction) + return _AmountResult(minor=minor, direction=implied_direction, direction_explicit=True) return _AmountResult() -def _build_candidate(index: int, row: _RawRow, ocr_min_confidence: float) -> CandidateTransaction: +def _build_candidate( + index: int, + row: _RawRow, + ocr_min_confidence: float, + currency: str = "GBP", + dialect: Dialect = GENERIC, +) -> CandidateTransaction: fields = row.fields raw_fields = {name: value for name, value in fields.items() if value.strip()} raw_fields["raw_text"] = row.raw_text @@ -359,7 +479,7 @@ def _build_candidate(index: int, row: _RawRow, ocr_min_confidence: float) -> Can candidate_id=f"p{index}", transaction_date=fields.get("date", "").strip() or None, raw_description=fields.get("description", "").strip(), - currency="GBP", + currency=currency.upper(), external_id=fields.get("reference", "").strip() or None, source_format="pdf", source_page=row.source_page, @@ -367,12 +487,13 @@ def _build_candidate(index: int, row: _RawRow, ocr_min_confidence: float) -> Can extraction_method="ocr" if row.is_ocr else "pdf", raw_fields=raw_fields, ) - amount = _resolve_amount(fields) + amount = _resolve_amount(fields, dialect, currency) if amount.issue: candidate.issues.append(amount.issue) else: candidate.amount_minor = amount.minor candidate.direction = amount.direction + candidate.direction_explicit = amount.direction_explicit if row.is_ocr: candidate.add_issue( OCR_EXTRACTED, @@ -406,6 +527,8 @@ def __init__( max_candidate_rows: int | None = None, word_provider: WordProvider | None = None, ocr_min_confidence: float | None = None, + dialect: Dialect = GENERIC, + currency: str = "GBP", ) -> None: self._max_pages = ( max_pdf_pages if max_pdf_pages is not None else get_settings().max_pdf_pages @@ -419,6 +542,8 @@ def __init__( if ocr_min_confidence is not None else get_settings().ocr_min_confidence ) + self.dialect = dialect + self.currency = currency def extract(self, source: StatementSource) -> ExtractionResult: result = ExtractionResult(extractor=self.name) @@ -433,7 +558,7 @@ def extract(self, source: StatementSource) -> ExtractionResult: ) ) return result - except Exception: # a corrupt/unsupported PDF becomes a sanitized batch issue + except Exception: result.issues.append( CandidateIssue( PDF_NOT_EXTRACTABLE, @@ -456,10 +581,10 @@ def _extract(self, pdf: PDF, result: ExtractionResult) -> ExtractionResult: kept: list[_RawRow] = [] for page in pdf.pages: page_rows, header_top = self._page_rows(page) - kept.extend(_merge_continuations(page_rows, header_top)) + kept.extend(_merge_continuations(page_rows, header_top, self.dialect)) candidates = [ - _build_candidate(index, row, self._ocr_min_confidence) + _build_candidate(index, row, self._ocr_min_confidence, self.currency, self.dialect) for index, row in enumerate(kept, start=1) ] if len(candidates) > self._max_rows: @@ -488,4 +613,4 @@ def _page_rows(self, page: Page) -> tuple[list[_RawRow], float | None]: mapping = _table_header(table) if mapping: return _table_rows(table, mapping, page.page_number), None - return _word_rows(self._word_provider(page), page.page_number) + return _word_rows(self._word_provider(page), page.page_number, self.dialect) diff --git a/src/pfa/ingestion/service.py b/src/pfa/ingestion/service.py index 284217a..9fd41dc 100644 --- a/src/pfa/ingestion/service.py +++ b/src/pfa/ingestion/service.py @@ -9,6 +9,7 @@ from pfa.db.models import MerchantRuleModel, TransactionModel from pfa.db.unit_of_work import UnitOfWork from pfa.domain.errors import ImportRowError +from pfa.domain.money import SUPPORTED_CURRENCIES from pfa.domain.transactions import ( ClassificationSource, SpendingCategory, @@ -18,6 +19,7 @@ from pfa.observability import TimedOperation from .candidates import ( + CURRENCY_ACCOUNT_MISMATCH, DUPLICATE_ROW, ERROR, INVALID_AMOUNT, @@ -102,27 +104,34 @@ def _classification_from_rule(rule: MerchantRuleModel) -> Classification: def _validate_candidate(candidate: CandidateTransaction) -> None: + if not candidate.transaction_date: + candidate.add_issue(INVALID_DATE, "missing transaction date") + return try: - parse_date(candidate.transaction_date or "") + parse_date(candidate.transaction_date) except ImportRowError as exc: candidate.add_issue(INVALID_DATE, str(exc)) return - if not candidate.raw_description: + if not candidate.raw_description.strip(): candidate.add_issue(MISSING_DESCRIPTION, "missing description") return if candidate.amount_minor is None: try: - sign, amount_minor = parse_amount(candidate.raw_fields.get("amount", "")) + sign, amount_minor, is_explicit_credit = parse_amount( + candidate.raw_fields.get("amount", ""), candidate.currency + ) except ImportRowError as exc: candidate.add_issue(INVALID_AMOUNT, str(exc)) return candidate.amount_minor = amount_minor candidate.direction = "debit" if sign < 0 else "credit" + candidate.direction_explicit = is_explicit_credit candidate.normalized_description = normalize_description(candidate.raw_description) - if candidate.currency != "GBP": + if candidate.currency.upper() not in SUPPORTED_CURRENCIES: + supported = ", ".join(sorted(SUPPORTED_CURRENCIES)) candidate.add_issue( UNSUPPORTED_CURRENCY, - f"unsupported currency {candidate.currency!r}; PFA v0.1 supports GBP only", + f"unsupported currency {candidate.currency!r}; supported: {supported}", ) return if candidate.posted_date: @@ -154,6 +163,17 @@ def validate(self, candidates: Sequence[CandidateTransaction]) -> None: for candidate in candidates: if candidate.state != ERROR: _validate_candidate(candidate) + if candidate.state != ERROR and candidate.account_hint: + # Only an *existing* account can disagree with the row - a brand-new + # account takes its currency from the first candidate that names it, at + # commit time, so there is nothing to compare against yet. + account = self.uow.accounts.get_by_name(candidate.account_hint) + if account is not None and account.currency.upper() != candidate.currency.upper(): + candidate.add_issue( + CURRENCY_ACCOUNT_MISMATCH, + f"row currency {candidate.currency} does not match " + f"{account.name}'s account currency {account.currency}", + ) def resolve_duplicates(self, candidates: Sequence[CandidateTransaction]) -> None: """Fingerprints valid rows, occurrence-aware, and matches them against the ledger.""" @@ -207,6 +227,16 @@ def commit( account = self.uow.accounts.get_or_create( candidate.account_hint or "Main account", candidate.currency ) + if account.currency.upper() != candidate.currency.upper(): + # validate() already blocks this for an existing account at preview time; + # reaching it here means a caller committed without validating first. Skip + # rather than raise - a currency mismatch must never crash a commit. + candidate.add_issue( + CURRENCY_ACCOUNT_MISMATCH, + f"row currency {candidate.currency} does not match " + f"{account.name}'s account currency {account.currency}", + ) + continue transaction = TransactionModel( external_id=candidate.external_id, account_id=account.id, diff --git a/src/pfa/planning/service.py b/src/pfa/planning/service.py index 49440e1..0ef8cd9 100644 --- a/src/pfa/planning/service.py +++ b/src/pfa/planning/service.py @@ -30,24 +30,28 @@ def __init__( self.accounts = accounts self.transactions = transactions - def _average_monthly_net(self, as_of: date, months: int = 3) -> int: + def _average_monthly_net(self, as_of: date, months: int = 3, currency: str = "GBP") -> int: values = [] cursor = as_of.replace(day=1) for _ in range(months): cursor = (cursor.replace(day=1) - timedelta(days=1)).replace(day=1) - values.append(self.analytics.monthly_summary(cursor).net_cashflow_minor) + values.append( + self.analytics.monthly_summary(cursor, currency=currency).net_cashflow_minor + ) return ( int((Decimal(sum(values)) / len(values)).quantize(Decimal("1"), ROUND_HALF_UP)) if values else 0 ) - def _average_monthly_spending(self, as_of: date, months: int = 3) -> int: + def _average_monthly_spending(self, as_of: date, months: int = 3, currency: str = "GBP") -> int: values = [] cursor = as_of.replace(day=1) for _ in range(months): cursor = (cursor.replace(day=1) - timedelta(days=1)).replace(day=1) - values.append(max(self.analytics.monthly_summary(cursor).spending_minor, 0)) + values.append( + max(self.analytics.monthly_summary(cursor, currency=currency).spending_minor, 0) + ) return ( int((Decimal(sum(values)) / len(values)).quantize(Decimal("1"), ROUND_HALF_UP)) if values @@ -55,12 +59,16 @@ def _average_monthly_spending(self, as_of: date, months: int = 3) -> int: ) def simulate_purchase( - self, cost_minor: int, horizon_months: int = 3, as_of: date | None = None + self, + cost_minor: int, + horizon_months: int = 3, + as_of: date | None = None, + currency: str = "GBP", ) -> ScenarioResult: as_of = as_of or date.today() - starting = current_cash(self.accounts, self.transactions, as_of) - monthly_net = self._average_monthly_net(as_of) - average_expenses = self._average_monthly_spending(as_of) + starting = current_cash(self.accounts, self.transactions, currency=currency, as_of=as_of) + monthly_net = self._average_monthly_net(as_of, currency=currency) + average_expenses = self._average_monthly_spending(as_of, currency=currency) baseline = starting + monthly_net * horizon_months scenario = baseline - cost_minor months = ( @@ -82,11 +90,11 @@ def simulate_purchase( assumptions=[ ( "average net cash flow from prior three complete months: " - f"{monthly_net} minor units" + f"{monthly_net} minor units ({currency})" ), ( "average spending from prior three complete months: " - f"{average_expenses} minor units" + f"{average_expenses} minor units ({currency})" ), "purchase occurs immediately; no investment returns assumed", f"horizon: {horizon_months} months", @@ -94,9 +102,13 @@ def simulate_purchase( ) def simulate_monthly_contribution( - self, additional_minor: int, horizon_months: int = 6, as_of: date | None = None + self, + additional_minor: int, + horizon_months: int = 6, + as_of: date | None = None, + currency: str = "GBP", ) -> ScenarioResult: - result = self.simulate_purchase(0, horizon_months, as_of) + result = self.simulate_purchase(0, horizon_months, as_of, currency=currency) scenario = result.baseline_month_end_cash_minor - additional_minor * horizon_months return result.model_copy( update={ @@ -105,7 +117,7 @@ def simulate_monthly_contribution( "affordable": scenario >= 0, "assumptions": [ *result.assumptions, - f"additional monthly contribution: {additional_minor} minor units", + f"additional monthly contribution: {additional_minor} minor units ({currency})", ], } ) diff --git a/src/pfa/services/fx.py b/src/pfa/services/fx.py new file mode 100644 index 0000000..ec58ffc --- /dev/null +++ b/src/pfa/services/fx.py @@ -0,0 +1,65 @@ +from __future__ import annotations + +import json +import logging +from datetime import date +from decimal import Decimal +from typing import TYPE_CHECKING + +import httpx + +if TYPE_CHECKING: + from pfa.db.models import FxRateModel + from pfa.db.unit_of_work import UnitOfWork + +logger = logging.getLogger("pfa") + +FRANKFURTER_API_BASE = "https://api.frankfurter.dev/v1" + + +def fetch_and_store_fx_rates( + uow: UnitOfWork, + base_currency: str = "GBP", + symbols: list[str] | None = None, + on_date: date | str | None = None, + client: httpx.Client | None = None, +) -> list[FxRateModel]: + base = base_currency.upper() + symbols_list = symbols or ["EUR", "INR", "USD", "JPY"] + filtered_symbols = [s.upper() for s in symbols_list if s.upper() != base] + if not filtered_symbols: + return [] + + symbols_str = ",".join(filtered_symbols) + date_segment = on_date.isoformat() if isinstance(on_date, date) else (on_date or "latest") + url = f"{FRANKFURTER_API_BASE}/{date_segment}?base={base}&symbols={symbols_str}" + + close_client = False + if client is None: + client = httpx.Client(timeout=15.0) + close_client = True + + try: + response = client.get(url) + response.raise_for_status() + # Parse the response's own JSON numbers straight to Decimal - going through + # response.json() would round-trip every rate through a binary float first. + payload = json.loads(response.text, parse_float=Decimal) + finally: + if close_client: + client.close() + + effective_date = date.fromisoformat(payload["date"]) + rates_data: dict[str, Decimal] = payload.get("rates", {}) + stored: list[FxRateModel] = [] + for quote, rate_value in rates_data.items(): + rate_model = uow.fx_rates.set_rate( + base_currency=base, + quote_currency=quote, + rate=str(rate_value), + effective_at=effective_date, + source="frankfurter", + ) + stored.append(rate_model) + + return stored diff --git a/src/pfa/services/review.py b/src/pfa/services/review.py index 7f15d5d..9d6793d 100644 --- a/src/pfa/services/review.py +++ b/src/pfa/services/review.py @@ -3,17 +3,23 @@ from pfa.analytics.service import AnalyticsService -def monthly_review_evidence(analytics: AnalyticsService, period: date) -> dict[str, object]: +def monthly_review_evidence( + analytics: AnalyticsService, period: date, currency: str = "GBP" +) -> dict[str, object]: """Build the authoritative evidence bundle used by the review narrator.""" - previous = analytics.compare_periods(period).previous + previous = analytics.compare_periods(period, currency=currency).previous return { - "summary": analytics.monthly_summary(period).model_dump(), - "categories": [item.model_dump() for item in analytics.category_spending(period)], - "comparison": analytics.compare_periods(period).model_dump(), + "summary": analytics.monthly_summary(period, currency=currency).model_dump(), + "categories": [ + item.model_dump() for item in analytics.category_spending(period, currency=currency) + ], + "comparison": analytics.compare_periods(period, currency=currency).model_dump(), "previous_summary": previous.model_dump(), - "recurring_payments": analytics.recurring_payments(), - "budget_status": [item.model_dump() for item in analytics.budget_status(period)], + "recurring_payments": analytics.recurring_payments(currency=currency), + "budget_status": [ + item.model_dump() for item in analytics.budget_status(period, currency=currency) + ], "goal_progress": [item.model_dump() for item in analytics.goal_progress()], - "category_spikes": analytics.category_spikes(period), - "unusual_transactions": analytics.unusual_transactions(period), + "category_spikes": analytics.category_spikes(period, currency=currency), + "unusual_transactions": analytics.unusual_transactions(period, currency=currency), } diff --git a/tests/integration/test_api.py b/tests/integration/test_api.py index 9cc4822..7ac9e3b 100644 --- a/tests/integration/test_api.py +++ b/tests/integration/test_api.py @@ -51,3 +51,45 @@ def test_dashboard_and_static_assets_are_served(tmp_path) -> None: js_resp = client.get("/static/app.js") assert js_resp.status_code == 200 assert "javascript" in js_resp.headers.get("content-type", "") + + +def test_api_fx_rates_endpoints(tmp_path) -> None: + database_url = f"sqlite:///{tmp_path / 'pfa.db'}" + config = Config("alembic.ini") + config.set_main_option("sqlalchemy.url", database_url) + command.upgrade(config, "head") + app = create_app(Settings(database_url=database_url)) + with TestClient(app) as client: + # Set manual rate + post_resp = client.post( + "/fx/rates", + json={ + "base_currency": "GBP", + "quote_currency": "INR", + "rate": "105.5", + "effective_at": "2026-08-01", + }, + ) + assert post_resp.status_code == 200 + data = post_resp.json() + assert data["base_currency"] == "GBP" + assert data["quote_currency"] == "INR" + assert data["rate"] == "105.5" + + bad_resp = client.post( + "/fx/rates", + json={ + "base_currency": "GBP", + "quote_currency": "USD", + "rate": "not-a-number", + "effective_at": "2026-08-01", + }, + ) + assert bad_resp.status_code == 422 + + # Get rates + get_resp = client.get("/fx/rates?base=GBP") + assert get_resp.status_code == 200 + rates = get_resp.json() + assert len(rates) == 1 + assert rates[0]["quote_currency"] == "INR" diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index e1bcdfb..efb730f 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -9,3 +9,23 @@ def test_cli_missing_import_file_is_a_clean_usage_error(tmp_path) -> None: assert result.exit_code == 2 assert "path must identify a local CSV file" in result.output assert "Traceback" not in result.output + + +def test_cli_fx_commands(tmp_path) -> None: + database_url = f"sqlite:///{tmp_path / 'pfa.db'}" + from alembic import command + from alembic.config import Config + + config = Config("alembic.ini") + config.set_main_option("sqlalchemy.url", database_url) + command.upgrade(config, "head") + + runner = CliRunner(env={"PFA_DATABASE_URL": database_url}) + set_res = runner.invoke(app, ["fx", "set", "GBP", "USD", "1.30", "--date", "2026-08-01"]) + assert set_res.exit_code == 0 + assert "FX rate GBP/USD = 1.30 set" in set_res.output + + list_res = runner.invoke(app, ["fx", "list"]) + assert list_res.exit_code == 0 + assert "GBP" in list_res.output + assert "USD" in list_res.output diff --git a/tests/unit/test_financial_invariants.py b/tests/unit/test_financial_invariants.py index faa1265..8eaaf4c 100644 --- a/tests/unit/test_financial_invariants.py +++ b/tests/unit/test_financial_invariants.py @@ -123,14 +123,15 @@ def test_dry_run_rolls_back_accounts_transactions_and_state(tmp_path) -> None: def test_unsupported_currency_fails_closed_instead_of_reporting_false_gbp(tmp_path) -> None: path = tmp_path / "currency.csv" path.write_text( - "date,description,amount,kind,currency\n2026-08-01,Salary,1000,income,USD\n", + "date,description,amount,kind,currency\n2026-08-01,Salary,1000,income,XYZ\n", encoding="utf-8", ) engine, uow, _ = services() result = ImportService(uow).import_csv(path) assert result.imported == 0 - assert result.errors == ["row 2: unsupported currency 'USD'; PFA v0.1 supports GBP only"] + assert len(result.errors) == 1 + assert "unsupported currency 'XYZ'" in result.errors[0] assert uow.transactions.all() == [] uow.session.close() engine.dispose() @@ -201,3 +202,60 @@ def test_headerless_export_imports_every_row_with_the_signs_it_was_written_with( ] uow.session.close() engine.dispose() + + +def test_mixed_currency_analytics_strictly_partitions_currencies_without_sum_pollution( + tmp_path, +) -> None: + """Invariant: an INR account alongside GBP must NEVER sum into 405,000 of something.""" + path_gbp = tmp_path / "gbp.csv" + path_gbp.write_text( + "date,description,amount,kind,category,currency,account\n" + "2026-08-01,Salary,5000,income,,GBP,UK Bank\n" + "2026-08-05,Groceries,-200,expense,groceries,GBP,UK Bank\n", + encoding="utf-8", + ) + path_inr = tmp_path / "inr.csv" + path_inr.write_text( + "date,description,amount,kind,category,currency,account\n" + "2026-08-01,Consulting,400000,income,,INR,India Bank\n" + "2026-08-10,Rent,-50000,expense,housing,INR,India Bank\n", + encoding="utf-8", + ) + + engine, uow, analytics = services() + importer = ImportService(uow) + importer.import_csv(path_gbp) + importer.import_csv(path_inr) + + # Check GBP analytics + gbp_summary = analytics.monthly_summary(date(2026, 8, 1), currency="GBP") + assert gbp_summary.currency == "GBP" + assert gbp_summary.income_minor == 500_000 # 5,000.00 GBP + assert gbp_summary.spending_minor == 20_000 # 200.00 GBP + assert gbp_summary.net_cashflow_minor == 480_000 + assert gbp_summary.transaction_count == 2 + + # Check INR analytics + inr_summary = analytics.monthly_summary(date(2026, 8, 1), currency="INR") + assert inr_summary.currency == "INR" + assert inr_summary.income_minor == 40_000_000 # 400,000.00 INR + assert inr_summary.spending_minor == 5_000_000 # 50,000.00 INR + assert inr_summary.net_cashflow_minor == 35_000_000 + assert inr_summary.transaction_count == 2 + + # Verify category spending is partitioned + gbp_cats = { + item.category: item.total_minor + for item in analytics.category_spending(date(2026, 8, 1), currency="GBP") + } + assert gbp_cats == {"groceries": 20_000} + + inr_cats = { + item.category: item.total_minor + for item in analytics.category_spending(date(2026, 8, 1), currency="INR") + } + assert inr_cats == {"housing": 5_000_000} + + uow.session.close() + engine.dispose() diff --git a/tests/unit/test_fx.py b/tests/unit/test_fx.py new file mode 100644 index 0000000..cdbd07c --- /dev/null +++ b/tests/unit/test_fx.py @@ -0,0 +1,151 @@ +from datetime import date +from decimal import Decimal + +import httpx +import pytest +from sqlalchemy import create_engine +from sqlalchemy.orm import Session + +from pfa.db.models import Base +from pfa.db.repositories import FxRateRepository +from pfa.db.unit_of_work import UnitOfWork +from pfa.domain.errors import ValidationError +from pfa.domain.fx import to_base +from pfa.domain.money import Money +from pfa.services.fx import fetch_and_store_fx_rates + + +@pytest.fixture +def session(): + engine = create_engine("sqlite:///:memory:") + Base.metadata.create_all(engine) + with Session(engine) as sess: + yield sess + engine.dispose() + + +@pytest.fixture +def fx_repo(session): + return FxRateRepository(session) + + +def test_set_and_retrieve_fx_rate(fx_repo): + model = fx_repo.set_rate("INR", "GBP", "0.0095", date(2026, 8, 1), source="manual") + assert model.rate == "0.0095" + assert model.base_currency == "INR" + assert model.quote_currency == "GBP" + + # Upsert with new rate on same date + updated = fx_repo.set_rate("INR", "GBP", "0.0096", date(2026, 8, 1), source="manual") + assert updated.rate == "0.0096" + assert len(fx_repo.all()) == 1 + + +def test_rate_on_date_semantics(fx_repo): + fx_repo.set_rate("INR", "GBP", "0.0090", date(2026, 8, 1)) + fx_repo.set_rate("INR", "GBP", "0.0095", date(2026, 8, 15)) + + # Before earliest rate -> None + assert fx_repo.rate_on(date(2026, 7, 31), "INR", "GBP") is None + + # On exact date + rate_aug1, model1 = fx_repo.rate_on(date(2026, 8, 1), "INR", "GBP") + assert rate_aug1 == Decimal("0.0090") + assert model1.effective_at == date(2026, 8, 1) + + # Between dates -> nearest rate at or before + rate_aug10, model10 = fx_repo.rate_on(date(2026, 8, 10), "INR", "GBP") + assert rate_aug10 == Decimal("0.0090") + + # On later date + rate_aug15, model15 = fx_repo.rate_on(date(2026, 8, 15), "INR", "GBP") + assert rate_aug15 == Decimal("0.0095") + + # After latest date -> stays at latest rate at or before + rate_aug20, model20 = fx_repo.rate_on(date(2026, 8, 20), "INR", "GBP") + assert rate_aug20 == Decimal("0.0095") + + +def test_inverse_rate_resolution(fx_repo): + # Store GBP to EUR rate: 1 GBP = 1.20 EUR + fx_repo.set_rate("GBP", "EUR", "1.20", date(2026, 8, 1)) + + # Rate from EUR to GBP should be 1 / 1.20 = 0.8333... + rate_eur_gbp, model = fx_repo.rate_on(date(2026, 8, 10), "EUR", "GBP") + assert rate_eur_gbp == Decimal(1) / Decimal("1.20") + assert model.base_currency == "GBP" + + +def test_to_base_conversion(fx_repo): + fx_repo.set_rate("INR", "GBP", "0.00863", date(2026, 8, 29)) + + # Convert 100,000 INR (10,000,000 minor) to GBP + inr_money = Money(10_000_000, "INR") # 100,000.00 INR + converted, rate_used = to_base(inr_money, date(2026, 8, 29), fx_repo, "GBP") + + # 100,000 * 0.00863 = 863.00 GBP -> 86300 minor + assert converted.currency == "GBP" + assert converted.minor == 86300 + assert rate_used.rate == Decimal("0.00863") + assert rate_used.base_currency == "INR" + assert rate_used.quote_currency == "GBP" + + +def test_to_base_identity_for_same_currency(fx_repo): + gbp_money = Money(5000, "GBP") + converted, rate_used = to_base(gbp_money, date(2026, 8, 29), fx_repo, "GBP") + assert converted.currency == "GBP" + assert converted.minor == 5000 + assert rate_used.rate == Decimal("1.0") + + +def test_to_base_missing_rate_raises(fx_repo): + usd_money = Money(1000, "USD") + with pytest.raises(ValidationError, match="No FX rate available"): + to_base(usd_money, date(2026, 8, 29), fx_repo, "GBP") + + +def test_fetch_and_store_fx_rates_keeps_full_decimal_precision(session): + """The response's JSON numbers must never round-trip through a binary float - a rate + with more decimal digits than float can hold exactly must be stored byte-for-byte.""" + body = ( + b'{"amount":1.0,"base":"GBP","date":"2026-08-28",' + b'"rates":{"INR":129.123456789012345,"USD":1.3583}}' + ) + + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.params["base"] == "GBP" + return httpx.Response(200, content=body, headers={"content-type": "application/json"}) + + client = httpx.Client(transport=httpx.MockTransport(handler)) + uow = UnitOfWork(session) + + stored = fetch_and_store_fx_rates( + uow, base_currency="GBP", on_date=date(2026, 8, 28), client=client + ) + + by_quote = {model.quote_currency: model for model in stored} + assert by_quote["INR"].rate == "129.123456789012345" + assert by_quote["INR"].source == "frankfurter" + assert by_quote["USD"].rate == "1.3583" + assert by_quote["INR"].effective_at == date(2026, 8, 28) + + rate_decimal, _ = uow.fx_rates.rate_on(date(2026, 8, 28), "GBP", "INR") + assert rate_decimal == Decimal("129.123456789012345") + + +def test_fetch_and_store_fx_rates_excludes_base_from_symbols(session): + def handler(request: httpx.Request) -> httpx.Response: + assert "GBP" not in request.url.params["symbols"].split(",") + return httpx.Response( + 200, + json={"amount": 1.0, "base": "GBP", "date": "2026-08-28", "rates": {"INR": 129.5}}, + ) + + client = httpx.Client(transport=httpx.MockTransport(handler)) + uow = UnitOfWork(session) + + stored = fetch_and_store_fx_rates( + uow, base_currency="GBP", symbols=["GBP", "INR"], on_date=date(2026, 8, 28), client=client + ) + assert [m.quote_currency for m in stored] == ["INR"] diff --git a/tests/unit/test_money.py b/tests/unit/test_money.py index ab606da..68df0bd 100644 --- a/tests/unit/test_money.py +++ b/tests/unit/test_money.py @@ -11,6 +11,23 @@ def test_money_rounds_to_integer_minor_units() -> None: assert Money(1234).to_major() == Decimal("12.34") +def test_money_supports_different_currency_minor_units() -> None: + # JPY has exponent 0 (no decimal places) + jpy = Money.from_major("1500", "JPY") + assert jpy.minor == 1500 + assert jpy.to_major() == Decimal("1500") + + # INR has exponent 2 + inr = Money.from_major("450.50", "INR") + assert inr.minor == 45050 + assert inr.to_major() == Decimal("450.50") + + +def test_money_rejects_unsupported_currency() -> None: + with pytest.raises(ValidationError, match="Unsupported currency"): + Money(100, "XYZ") + + def test_money_rejects_mixed_currency_arithmetic() -> None: with pytest.raises(ValidationError): Money(100, "GBP") + Money(100, "USD") diff --git a/tests/unit/test_pdf_extractor.py b/tests/unit/test_pdf_extractor.py index bd1ed61..333c3fa 100644 --- a/tests/unit/test_pdf_extractor.py +++ b/tests/unit/test_pdf_extractor.py @@ -11,6 +11,7 @@ from pfa.ingestion import candidates as codes # noqa: E402 from pfa.ingestion.candidates import ExtractionResult, StatementSource # noqa: E402 +from pfa.ingestion.dialects import BARCLAYCARD # noqa: E402 from pfa.ingestion.extractors.pdf import PdfStatementExtractor # noqa: E402 @@ -111,7 +112,7 @@ def test_continuation_line_far_from_any_row_is_dropped_as_noise(tmp_path: Path) assert result.candidates[0].raw_description == "Tesco Metro" -def test_amex_banner_header_does_not_turn_statement_chatter_into_candidates( +def test_amex_banner_header_and_statement_chatter_never_become_candidates( tmp_path: Path, ) -> None: columns = [72.0, 160.0, 300.0, 420.0, 500.0] @@ -129,8 +130,22 @@ def test_amex_banner_header_does_not_turn_statement_chatter_into_candidates( result = _extract(tmp_path, [statement_page(rows, columns)]) - assert [(c.transaction_date, c.raw_description) for c in result.candidates] == [] - assert [issue.code for issue in result.issues] == [codes.PDF_NOT_EXTRACTABLE] + assert [(c.transaction_date, c.amount_minor) for c in result.candidates] == [ + ("Jul31", 194064), + ("Jul21", 630), + ] + assert result.issues == [] + # The repeated "Date" column (AMEX prints it twice) never leaks into the description. + assert [c.raw_description for c in result.candidates] == [ + "PAYMENT RECEIVED - THANK YOU", + "ZETTLE *REDACTED", + ] + # The own-line "CR" marker attaches to the row above it, not a candidate of its own, + # and marks that row's direction explicit so a later sign convention cannot flip it. + payment, purchase = result.candidates + assert payment.direction == "credit" + assert payment.direction_explicit is True + assert purchase.direction_explicit is False def test_header_with_zero_plausible_data_rows_reports_pdf_not_extractable( @@ -266,7 +281,7 @@ def test_no_recognizable_rows_reports_pdf_not_extractable_with_actionable_copy( def test_money_out_and_money_in_headers_are_recognised(tmp_path: Path) -> None: # Monzo, Starling and Lloyds all label their columns this way. The CSV extractor has - # always known the wording; the PDF map did not, until both read one shared table. + # always known the wording; this proves the PDF word-based header map reads it too. columns = [72.0, 160.0, 320.0, 420.0] rows = [ ["Date", "Description", "Money Out", "Money In"], @@ -278,3 +293,31 @@ def test_money_out_and_money_in_headers_are_recognised(tmp_path: Path) -> None: assert result.issues == [] assert [c.amount_minor for c in result.candidates] == [1250, 300000] assert [c.direction for c in result.candidates] == ["debit", "credit"] + + +def test_barclaycard_two_column_layout_clustering(tmp_path: Path) -> None: + # Left column: transactions at x ~ 50..280 + # Right column: marketing copy at x ~ 350..550 + left_columns = [50.0, 130.0, 260.0] + left_rows = [ + ["Date", "Description", "Amount"], + ["27 Jul 25", "COFFEE HOUSE LONDON", "-3.50"], + ["28 Jul 25", "NEWSAGENT LEEDS", "-2.10"], + ] + + page = statement_page(left_rows, left_columns) + # Add right column marketing words at same y positions + page.append((360.0, 720.0, "Understanding your interest", 10.0)) + page.append((360.0, 706.0, "Your interest rates this month", 10.0)) + page.append((360.0, 692.0, "Visit barclaycard.co.uk", 10.0)) + + result = _extract(tmp_path, [page], dialect=BARCLAYCARD) + + assert result.issues == [] + assert len(result.candidates) == 2 + assert [c.transaction_date for c in result.candidates] == ["27 Jul 25", "28 Jul 25"] + assert [c.raw_description for c in result.candidates] == [ + "COFFEE HOUSE LONDON", + "NEWSAGENT LEEDS", + ] + assert [c.amount_minor for c in result.candidates] == [350, 210] diff --git a/tests/unit/test_planning_scenarios.py b/tests/unit/test_planning_scenarios.py index 3099d6d..44867f9 100644 --- a/tests/unit/test_planning_scenarios.py +++ b/tests/unit/test_planning_scenarios.py @@ -6,9 +6,10 @@ class StableHistory: - def monthly_summary(self, period: date) -> MonthlySummary: + def monthly_summary(self, period: date, currency: str = "GBP") -> MonthlySummary: return MonthlySummary( period=period.strftime("%Y-%m"), + currency=currency, income_minor=400_000, spending_minor=300_000, net_cashflow_minor=100_000, diff --git a/tests/unit/test_statement_candidates.py b/tests/unit/test_statement_candidates.py index 7d4ef6b..bec3cb8 100644 --- a/tests/unit/test_statement_candidates.py +++ b/tests/unit/test_statement_candidates.py @@ -29,7 +29,7 @@ def test_validation_reports_one_issue_code_per_blocking_problem() -> None: candidate("c1", transaction_date="not-a-date"), candidate("c2", raw_description=""), candidate("c3", amount="not-a-number"), - candidate("c4", currency="EUR"), + candidate("c4", currency="XYZ"), candidate("c5", kind="teleportation"), candidate("c6", kind="expense", category="submarines"), candidate("c7", kind="transfer", transfer_purpose="hoarding"), From 1bea0b300ec674bc020a885193be47461ca76f33 Mon Sep 17 00:00:00 2001 From: Amit Afre Date: Sun, 30 Aug 2026 09:27:55 +0100 Subject: [PATCH 2/2] fix(test): isolate CLI fx tests from process-wide settings cache pfa.config.get_settings() is @lru_cache'd. Any PdfStatementExtractor() built with default settings (many unit tests do this) primes that cache with the real sqlite:///data/pfa.db - and once cached, CliRunner(env={"PFA_DATABASE_URL": ...}) has no effect for the rest of the process, so test_cli_fx_commands ran against whatever DB happened to be cached first instead of its own tmp_path DB. Reproduced locally by forcing collection order: pytest tests/unit/test_pdf_extractor.py tests/integration/test_cli.py and confirmed the autouse cache-clear fixture fixes it regardless of order. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01T8foThd2rk578JQmV3JHLQ --- tests/integration/test_cli.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index efb730f..3bdffae 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -1,6 +1,19 @@ +import pytest from typer.testing import CliRunner from pfa.cli.app import app +from pfa.config import get_settings + + +@pytest.fixture(autouse=True) +def _reset_settings_cache(): + """get_settings() is process-wide @lru_cache'd. A test that points PFA_DATABASE_URL + at a tmp_path DB must not inherit a stale cached Settings from an earlier test in this + file, nor leak its own tmp_path-scoped Settings into whatever runs after it. + """ + get_settings.cache_clear() + yield + get_settings.cache_clear() def test_cli_missing_import_file_is_a_clean_usage_error(tmp_path) -> None: