WealthySmart/codebase-review/01-architect.md

Architect Review — WealthySmart

Hat: Staff-level Software Architect. Focus: structure, layering, data model, migration story, API consistency, deploy topology.

Editorial note: ARCH-01 was corrected after verification — see the finding itself and README.md.

Executive Summary

WealthySmart is a single-user personal finance management application with a modern full-stack architecture: FastAPI + SQLModel backend (PostgreSQL) paired with a React 19 + Vite frontend served through a Hono proxy, plus an LLM-powered AI agent interface. The system handles multi-currency transactions (CRC/USD/EUR/BTC/XMR), budget forecasting with complex billing-cycle logic, pension/municipal bill tracking, and push notifications. The architecture is sound overall — no fundamental redesign is needed — but there are weak defaults in configuration, zero test coverage, an ad-hoc migration story, and consistency gaps in validation and response schemas that should be addressed incrementally.

How the System Fits Together

┌─────────────────────────────────────────────────────────────────┐
│                    DEPLOYMENT (Docker Compose)                  │
├─────────────────────────────────────────────────────────────────┤
│                                                                 │
│  ┌──────────────────────┐  ┌─────────────────────┐             │
│  │   Frontend (Node)    │  │  Backend (FastAPI)  │             │
│  │  ┌─────────────────┐ │  │  ┌───────────────┐  │             │
│  │  │ Vite (port 3000)│ │  │  │ Uvicorn       │  │             │
│  │  │ React 19 + TS   │ │  │  │ (port 8000)   │  │             │
│  │  └─────────────────┘ │  │  └───────────────┘  │             │
│  │  ┌─────────────────┐ │  │  ┌───────────────┐  │             │
│  │  │ Hono Server     │ │  │  │ 15 endpoints  │  │             │
│  │  │ (port 3001)     │ │  │  │ + LLM agent   │  │             │
│  │  │ • SPA serving   │ │  │  └───────────────┘  │             │
│  │  │ • CopilotKit    │ │  │                     │             │
│  │  │   runtime       │ │  │                     │             │
│  │  │ • /api proxy    │ │  │                     │             │
│  │  └─────────────────┘ │  │                     │             │
│  └──────────────────────┘  └─────────────────────┘             │
│           │                            │                        │
│           └────────────────────────────┘                        │
│                      │                                          │
│  ┌──────────────────────────────────┐                          │
│  │   PostgreSQL 16 (port 5432)      │                          │
│  │   • 15 tables + enums            │                          │
│  │   • No Alembic migrations        │                          │
│  └──────────────────────────────────┘                          │
│                                                                 │
│  Production: nginx-proxy + Let's Encrypt on single VPS         │
│  Ingestion: n8n flows POST to /api/v1 with API tokens          │
└─────────────────────────────────────────────────────────────────┘

Key data flows:

• Frontend requests /api/v1/... (proxied by Hono to FastAPI)
• Agent requests go to /api/v1/agent/agui (Microsoft Agent Framework + OpenAI)
• Exchange rates refreshed every 6 hours from multiple fallback APIs
• Auth via httpOnly cookies (SPA) or Bearer tokens (API clients / n8n)

Strengths

1. Modular endpoint structure — 15 focused routers (accounts, transactions, budget, analytics, etc.) with clear single responsibilities.
2. Sophisticated budget projection logic — budget_projection.py correctly handles billing cycles (18th–18th for credit cards vs. 1st–1st calendar month), deferred transactions, recurring items with override amounts, and cumulative balance tracking across years, with a clean API (compute_monthly_projection, compute_yearly_projection_with_cumulative).
3. Resilient exchange-rate handling — fallback chain (BCCR → ExchangeRate-API → currency-api → FloatRates → CoinGecko) with in-memory and database caching; uses a _last_known_* pattern to retain previous values during API outages.
4. Clean auth model — dual-mode authentication (httpOnly cookie for SPA, Bearer token for API clients) with JWT + API-token support. Token revocation marks rows inactive rather than deleting them.
5. Seed data for development — comprehensive default categories, accounts, and recurring items; single-user design appropriate for the app.
6. Type-safe models — SQLModel schemas enforce some constraints (unique categories, year/month combinations) at the DB level.
7. REST conventions mostly respected — POSTs return 201, pagination via offset/limit is consistently applied.
8. Consistent error surface — HTTPException with clear status codes and detail messages throughout.

Findings

[ARCH-01] Weak secret hygiene around .env files and config defaults

Severity: High (downgraded from Critical after verification) Location: .env, .env.prod (working tree), backend/app/config.py:6–9, docker-compose.yml Evidence (corrected): .env and .env.prod exist in the repo root and contain real secrets (OpenAI API key, DB password, admin credentials, VAPID keys), but they are gitignored and were never committed — git ls-files and git log --all --diff-filter=A confirm no history. The genuine issues are: (a) config.py ships working fallback credentials (SECRET_KEY="change-me-in-production", ADMIN_USERNAME/PASSWORD = "admin"), (b) docker-compose.yml hardcodes dev DB credentials, and (c) plaintext secrets on the dev machine with no rotation discipline. Why it matters: If a deploy ever runs without its env file (typo in compose, CI regression — note reference_gitea_deploy: .env.prod is regenerated each deploy), the app silently boots with admin/admin and a known JWT signing key. Recommendation: Remove the defaults and fail fast at startup (see SEC-01). Keep .env.example as the documented template. Rotate any key that has been pasted into chats/logs.

[ARCH-02] No Formal Database Migration Strategy

Severity: High Location: backend/app/db.py:13–76; no alembic/ directory Evidence: Migrations are ad-hoc SQL strings in run_migrations() with IF NOT EXISTS clauses. Adding a column requires hand-written ALTER TABLE code. Schema evolution is not version-controlled or reversible. Why it matters: Manual migrations are error-prone and have no rollback. Dev and prod can diverge silently. The project rule "never reset prod DB; use migrations" makes this the single most important infrastructure gap. Recommendation: Introduce Alembic: alembic init, autogenerate a baseline from current models, convert the existing manual statements, run alembic upgrade head at container startup. Effort: M (1–2 days).

[ARCH-03] Validation & Error-Handling Inconsistencies

Severity: Medium Location: endpoints/transactions.py:71–90, endpoints/budget.py:108–109, endpoints/analytics.py:56 Evidence: list_transactions() mixes cycle_year/cycle_month and start_date/end_date filters with or_ in a nested query — unclear precedence. budget.py uses positional HTTPException(400, ...) while others use keyword args. analytics.py:56 filters with the string literal "COMPRA" instead of TransactionType.COMPRA. Why it matters: Mixing filter modes risks undefined behavior; hardcoded enum strings break refactoring safety; inconsistent error style hampers debugging. Recommendation: Extract a build_transaction_query() helper with explicit precedence; standardize on status_code= kwargs; replace string literals with enum members. Effort: S.

[ARCH-04] Agent Tools Duplicate Service Logic

Severity: Medium Location: backend/app/agent/tools.py, backend/app/services/budget_projection.py Evidence: tools.py re-implements queries (get_accounts(), get_net_worth(), get_cycle_summary()) that overlap with the service layer instead of delegating to it. Why it matters: Budget logic changes must be applied in two places or the assistant and the UI disagree about the user's money. Recommendation: Make agent tools thin wrappers over service functions. Effort: M (1 day).

[ARCH-05] No Input Limits on Bulk Operations

Severity: Medium Location: endpoints/import_transactions.py:96–149 Evidence: paste_import() accepts arbitrary text with no size cap; a 10 MB paste is parsed entirely in memory before any validation. Why it matters: Memory-exhaustion DoS vector; bulk operations should validate request size upfront. Recommendation: Field(..., max_length=1_000_000) on the request model; pre-validate before opening the write loop. Effort: S.

[ARCH-06] Missing Test Coverage

Severity: High Location: No tests/ directory anywhere; requirements.txt lacks pytest Evidence: Zero test files in the repository. Why it matters: Budget projection is the most intricate logic in the app (billing-cycle edges, deferred carryover, cumulative balances with overrides) and is entirely unguarded against regression. Recommendation: backend/tests/ with test_budget_projection.py (Feb 28/29, year boundaries, overrides), test_auth.py, test_exchange_rate.py; Vitest for useBudget. Effort: L (3–5 days for meaningful coverage).

[ARCH-07] Single Hono Instance Is a Single Point of Failure

Severity: Low (revised for a single-user app) Location: frontend/server.ts:1–20, frontend/Dockerfile, docker-compose.prod.yml Evidence: One Hono container serves SPA + CopilotKit runtime + proxy. The Dockerfile healthcheck probes the backend's /api/health, not Hono's own readiness. Why it matters: If Hono crashes the whole app is down even with a healthy backend. For a single-user app, replicas are over-engineering; a correct healthcheck + restart policy is the right-sized fix. Recommendation: Healthcheck Hono itself; restart: unless-stopped; add 5xx retry in the frontend API client. Skip load balancing.

[ARCH-08] Exchange-Rate Refresh Loop Swallows Cancellation

Severity: Low Location: backend/app/services/exchange_rate.py:348–366, backend/app/main.py:66 Evidence: refresh_rates_periodically() runs while True with a broad except that logs and continues; cancellation during shutdown can be silently swallowed. Recommendation: Catch asyncio.CancelledError explicitly and break; keep the broad handler for everything else. Effort: S.

[ARCH-09] No Foreign-Key Delete Rules

Severity: Medium Location: backend/app/models/models.py:144, 258, 427 Evidence: Transaction.category_id and similar FKs declare foreign_key= with no ondelete behavior; deleting a category leaves dangling references. Why it matters: Silent referential inconsistency — budget rows pointing at categories that no longer exist. Recommendation: Decide per-FK between SET NULL (transactions should outlive categories) and CASCADE (water readings die with their receipt); apply via Alembic migration. Effort: S.

[ARCH-10] Fragile Cookie Parsing in Agent Middleware

Severity: Medium Location: backend/app/main.py:88–142 Evidence: Cookie extraction uses regex r"(?:^|;\s*)ws_token=([^;]+)", which mishandles edge cases (multiple cookies of the same name, unusual encodings). Recommendation: Use stdlib http.cookies.SimpleCookie. Effort: S.

[ARCH-11] deferred_to_next_cycle Implemented in Backend but Not Exposed in UI

Severity: Medium Location: backend/app/models/models.py:145, endpoints/transactions.py:219–235 Evidence: The field is settable via PATCH and respected by budget projection; the deferral toggle exists in row actions but TransactionModal.tsx has no field for it, and the affordance is undiscoverable (see UX-05). Recommendation: Add the checkbox to TransactionModal.tsx and make the row-action state visible. Effort: S.

[ARCH-12] Pagination Lacks a Deterministic Tiebreaker

Severity: Low Location: endpoints/transactions.py:96, endpoints/salarios.py:35–36 Evidence: Paginated queries order by date DESC only; equal dates yield non-deterministic page boundaries. Recommendation: .order_by(col(...).date.desc(), col(...).id.desc()) everywhere paginated. Effort: S.

[ARCH-13] Mutation → Refetch-Everything Pattern

Severity: Low Location: frontend/src/hooks/useBudget.ts:61–74 Evidence: Every mutation triggers Promise.all([fetchProjection(), fetchMonthDetail(), fetchRecurringItems()]) regardless of what changed. Why it matters: Wasteful, and the pattern will multiply as modules grow. Acceptable as a simplicity tradeoff today; the durable fix is a query library (see FE-10 / Plan Phase 3).

[ARCH-14] Hardcoded Dev DB Credentials in Compose

Severity: Medium Location: docker-compose.yml (db service environment: block) Evidence: POSTGRES_PASSWORD: wealthy_pass hardcoded in source control (prod compose correctly uses ${POSTGRES_PASSWORD}). Recommendation: Move dev credentials to the gitignored .env; reference via ${...} in both compose files. Effort: S.

[ARCH-15] N+1 Query in Category Aggregation

Severity: Medium Location: backend/app/services/budget_projection.py:370–383 Evidence: compute_cc_by_category() calls session.get(Category, cat_id) inside a loop — one query per category with spending. Recommendation: Prefetch {c.id: c for c in session.exec(select(Category)).all()} and look up in the dict. Effort: S (minutes).

[ARCH-16] API Response Schema Inconsistency

Severity: Low Location: endpoints/transactions.py:28–33 (Pydantic models) vs endpoints/budget.py:100–157 (raw dicts) vs endpoints/analytics.py:18–23 Evidence: Some endpoints return typed models, others plain dicts; OpenAPI schema generation is incomplete and frontend types are maintained by hand. Recommendation: Shared models in app/api/schemas.py + response_model= on every route; this also unlocks frontend type codegen (see FE-14). Effort: M.

[ARCH-17] Agent Session via ContextVar Couples Tools to HTTP Middleware

Severity: Low Location: backend/app/agent/tools.py:40–52, backend/app/main.py:131–141 Evidence: Tools fetch the DB session from a contextvars.ContextVar set by middleware; calling a tool outside a request raises LookupError. Recommendation: Document the constraint and add a clear error message (see BE-12); full refactor only if tools ever need to run outside HTTP. Effort: S for the guard.

[ARCH-18] No Data Export / Archival Path

Severity: Low Location: All tables are append-only with no export endpoint Why it matters: Acceptable growth for a personal app, but there is no way to get historical data out (taxes, accountant, backup verification). Recommendation: CSV export endpoint per major table (also UX wishlist #5). Effort: M.

[ARCH-19] Hono Proxy Trusts Upstream Blindly

Severity: Low Location: frontend/server.ts proxy handlers Evidence: Responses are piped through without shape validation; combined with FE-14 (no runtime validation client-side), schema drift fails confusingly. Recommendation: Don't validate in the proxy (wrong layer); fix via shared response schemas (ARCH-16) and optional Zod at the API-client boundary.

[ARCH-20] Lazy Imports Papering Over a Circular Dependency

Severity: Low Location: backend/app/auth.py:39–43 Evidence: _validate_token() imports get_session and APIToken inside the function body to dodge a cycle with db.py. Recommendation: Split token utilities into their own module to break the cycle properly. Effort: S.

Recommended Refactors (prioritized)

#	Item	Effort	Phase
1	ARCH-01/SEC-01: fail-fast on default secrets, fix dev compose creds	S	0
2	ARCH-02: Alembic migrations	M	1
3	ARCH-06: test suite (budget projection first)	L	1
4	ARCH-15: N+1 fix in compute_cc_by_category	S	2
5	ARCH-03: standardize validation/error handling/enums	S	2
6	ARCH-09: FK delete rules via migration	S	2
7	ARCH-04: agent tools delegate to services	M	2
8	ARCH-16: shared response schemas (response_model= everywhere)	M	3
9	ARCH-13/FE-10: query library on the frontend	M	3
10	ARCH-11: expose deferred-transaction UI	S	4
11	ARCH-18: CSV export	M	4
12	ARCH-05/08/10/12/14/17/20: small hardening fixes	S each	0–2

Overall assessment: Production-ready for a single-user personal app, with the caveat that configuration defaults must fail fast before anything else. The architecture needs no redesign — invest in migrations, tests, and consistency.