WealthySmart/codebase-review/07-improvement-plan.md

# Improvement Plan — WealthySmart

Phased plan derived from [06-weak-spots-inventory.md](06-weak-spots-inventory.md). Phases are ordered by dependency and risk: **harden → build foundations → fix correctness → rebuild the frontend data layer → polish and grow**. Each phase is independently shippable; stop-anywhere is safe.

Conventions respected throughout: small logical commits, push to GitHub origin (Gitea mirrors), `.env.prod` vars must be added in **both** Gitea secrets and the workflow (see `reference_gitea_deploy`), never reset the prod DB.

---

## Phase 0 — Security Hardening Sprint

**Goal:** close every cheap hole in the public-facing surface. **Effort: ~1 day.** No schema changes, no behavior changes for the legitimate user (except needing real env values — verify Gitea secrets are complete *before* deploying).

### 0.1 Fail-fast configuration (SEC-01, BE-15)
- `backend/app/config.py`: remove defaults for `SECRET_KEY`, `ADMIN_USERNAME`, `ADMIN_PASSWORD`; add a pydantic `model_validator` rejecting empty/known-default values.
- Keep a documented `.env.example` (no real values) as the template.
- **Pre-deploy check:** confirm all three exist in Gitea secrets and the workflow env-file generation step.
- *Accept:* app refuses to boot with missing/default secrets; prod deploy still succeeds.

### 0.2 Login hardening (SEC-02, SEC-03)
- Consolidate to a single login implementation (the cookie login in `main.py` is the one the SPA uses; have `endpoints/auth.py` delegate or remove it from the router if unused).
- `hmac.compare_digest` on username and password.
- Rate limit: `slowapi` 5/min/IP on login (in-memory is fine, single process). Return 429 with a Retry-After.
- *Accept:* 6th attempt within a minute → 429; valid login unaffected.

### 0.3 CORS pinning (SEC-04)
- `allow_origins=["https://wealth.cescalante.dev", "http://localhost:3000", "http://localhost:3001"]`; explicit methods/headers.
- *Accept:* prod SPA, dev SPA, and n8n flows (server-to-server — CORS-exempt) all still work.

### 0.4 Token & cookie tightening (SEC-05, SEC-08)
- `ACCESS_TOKEN_EXPIRE_MINUTES = 60 * 24 * 7` (7 days — balance for a personal app).
- New `COOKIE_SECURE: bool = True` setting (False in dev env); use it in `set_cookie`. Add to Gitea secrets/workflow if env-driven.
- *Accept:* prod cookie has `Secure`; dev login still works over http.

### 0.5 Security headers (SEC-07, FE-08)
- Hono middleware in `frontend/server.ts`: `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy: strict-origin-when-cross-origin`. CSP deferred until inline-script usage is audited (CopilotKit may need allowances).
- HSTS at nginx-proxy level (VPS config, not repo).

### 0.6 Small backend/proxy guards (ARCH-05, FE-23, FE-22)
- `max_length` on paste-import request text.
- try/catch → 502 JSON in `proxyToBackend`.
- Skip 401 redirect when already on `/login` in `api.ts`.

### 0.7 Housekeeping
- Delete dead `AgentHomeClient.tsx` (FE-20).
- Move dev DB credentials from `docker-compose.yml` into `.env` references (ARCH-14).
- Rotate any secret previously pasted into logs/chats as a precaution.

---

## Phase 1 — Foundations: Tests, Migrations, Time

**Goal:** make every later change safe. **Effort: ~1 week.** This phase has the highest leverage in the plan.

### 1.1 Test harness (ARCH-06, BE-25)
- Add `pytest`, `httpx` (test client) to a new `requirements-dev.txt`; create `backend/tests/` with a session fixture (SQLite in-memory or dockerized Postgres — prefer Postgres for `DISTINCT ON`/`NUMERIC` fidelity).
- CI: extend the Gitea workflow with a test job that gates deploy; add `pnpm typecheck` for the frontend.
- *Accept:* `pytest` green locally and in CI; deploy blocked on red.

### 1.2 Characterization tests for cycle math — **before touching the code** (BE-06, BE-07, T4)
- `test_budget_projection.py`: pin current behavior of `get_cycle_range`/`get_month_range` for: mid-month dates, the 18th itself, month/year boundaries, Feb 28/29.
- Validate against **known-real data**: pick 2–3 historical months where the correct BAC statement total is known and assert the projection matches.
- Then resolve the code/comment contradiction — fix whichever is wrong; add input validation (`1 <= month <= 12`, MIN/MAX_YEAR).
- *Accept:* documented, test-pinned answer to "which cycle does month M's budget use," verified against a real statement.

### 1.3 Alembic (ARCH-02, BE-19, T3)
- `alembic init`; baseline autogenerated from current models; `alembic stamp head` against the existing prod schema (no destructive ops).
- Port the `run_migrations()` SQL into history as already-applied; reduce `db.py` startup to `alembic upgrade head`.
- Document the workflow in CLAUDE.md (`alembic revision --autogenerate` → review → commit).
- *Accept:* fresh dev DB builds from migrations alone; prod stamps cleanly with zero data change.

### 1.4 Datetime sweep (BE-01, T10)
- Replace `datetime.utcnow()` → `datetime.now(timezone.utc)`; audit each comparison for naive/aware mixing (token expiry in `auth.py`, rate cache, budget date filters).
- Add a test for token expiry comparison.
- *Accept:* zero `utcnow` references; tests green.

### 1.5 Decimal migration — staged start (BE-02, T5)
- This phase only: write equivalence tests for projection math (current float behavior) so Phase 2's migration has a baseline. The actual column migration happens in Phase 2 after Alembic is proven in prod once.

---

## Phase 2 — Backend Correctness & Robustness

**Goal:** the backend tells the truth and fails loudly. **Effort: ~1 week.**

### 2.1 Money → Decimal (BE-02)
- Alembic migration: monetary columns → `NUMERIC(15,2)` (rates → `NUMERIC(15,6)`); models → `Decimal`; sweep `budget_projection.py` / `exchange_rate.py` / agent tools for float arithmetic; ensure JSON serialization keeps current shape (FastAPI serializes Decimal → number/string; pick and pin one — test it).
- *Accept:* Phase 1.5 equivalence tests pass within tolerance; API responses unchanged in shape; prod migration applied without reset.

### 2.2 Upload pipeline hardening (SEC-06, BE-09, BE-10, BE-14 — T9)
- Both upload endpoints: 10 MB bounded read, `%PDF` magic check, `asyncio.to_thread()` around parsing, `TemporaryDirectory()` for cleanup.
- Keep per-file commits but return a precise per-file manifest (filename → imported/updated/skipped/error) — pairs with UX-09 in Phase 3.
- *Accept:* oversized/non-PDF rejected with clear errors; event loop unblocked during parse (verify with a concurrent request); no temp files left after a forced timeout.

### 2.3 Query & constraint pass (T11)
- N+1 fixes: category prefetch (ARCH-15), `DISTINCT ON` pensions (BE-04), eager-loaded water readings (BE-05); collapse projection aggregates into grouped queries (BE-21).
- Alembic migration: FK delete rules — `SET NULL` for `Transaction.category_id` / `RecurringItem.category_id`, `CASCADE` for `WaterMeterReading.receipt_id` (ARCH-09, BE-11); pagination tiebreaker `, id DESC` (ARCH-12); `ON CONFLICT DO UPDATE` for pension upsert (BE-22).
- Atomic savings accrual: raise early on missing accounts (BE-03).
- *Accept:* projection endpoint query count measured before/after; constraint behavior covered by tests.

### 2.4 Error-visibility pass (BE-18, BE-12, ARCH-08)
- `exchange_rate.py`: specific exceptions + `logger.warning` per source; timestamp on `_last_known` caches with freshness in responses (BE-20).
- Clear `RuntimeError` for unbound agent session; explicit `CancelledError` handling in the refresh loop.

### 2.5 Agent tool cleanup (T12 backend half)
- Tools delegate to service functions (ARCH-04); year bounds (BE-16); remove `600.0`/`1.08` magic fallbacks — use last persisted DB rate or state unavailability (BE-24); optional offset params (BE-17).
- Document the invariant: **agent tools are read-only**; any future write tool requires a UI confirmation step (SEC-09).
- *Accept:* assistant's cycle summary numerically matches the Budget page for the same month (test).

### 2.6 Consistency sweep (ARCH-03, ARCH-10, ARCH-20)
- Enum members over string literals; keyword `status_code=`; `SimpleCookie` parsing; break the auth/db lazy-import cycle.

---

## Phase 3 — Frontend Data Layer & Feedback

**Goal:** kill the silent-failure class wholesale. **Effort: ~1 week.**

### 3.1 Adopt TanStack Query (FE-10 → retires FE-01/03/09/18/24, ARCH-13 — T7)
- `QueryClientProvider` at the root; wrap the existing `api.ts` request helper (keep `ApiError`, 401 handling); default `AbortSignal.timeout(30_000)`, `staleTime` ~30s, one retry.
- Migrate page by page: **Budget first** (worst race conditions), then Analytics, Pensions, Salarios, ServiciosMunicipales. Replace `useBudget`'s manual fetch trio with queries + targeted mutation invalidation.
- *Accept:* rapid month/source/search changes never render stale data; switching pages back within 30s renders instantly from cache.

### 3.2 Toast + error-state standard (T6: UX-01, UX-02, FE-02, FE-06, UX-09, UX-15)
- Add Sonner; success/error toast on every mutation; shared `<QueryBoundary>` (or per-page pattern) rendering skeleton → error-with-retry → data/empty.
- Render upload `errors[]` from the Phase 2.2 manifest in an Alert list.
- ConfirmDialog on *all* destructive actions including balance-override clear.
- *Accept:* with the backend stopped, every page shows an explicit error with retry; every mutation produces visible confirmation.

### 3.3 Auth-flow fixes (FE-15, FE-22)
- Auth probe distinguishes network failure (retry once / unknown state) from 401; redirect guard from 0.6 verified.

### 3.4 Type alignment (ARCH-16 + FE-14)
- Backend: shared Pydantic response models + `response_model=` on every route.
- Frontend: generate types from OpenAPI (`openapi-typescript`) replacing hand-written interfaces in `api.ts`; CI check that generated types are current.
- *Accept:* a backend schema change breaks `pnpm typecheck` instead of production rendering.

### 3.5 Small fixes batch
- Lazy initializers for theme/privacy (FE-07); guard Pensions ROI indexing (FE-04); timeout post-upload refetch (FE-05); cap upload-result list (FE-19); log CopilotKit middleware errors (FE-11); month-detail existence check (FE-12).

---

## Phase 4 — UX, Trust & Features

**Goal:** the daily-use payoff. Ordered by impact; each item independent. **Effort: 2–3 weeks spread.**

### 4.1 Sync Status page (UX-17 — the #1 trust feature)
- v1 (M): backend endpoint deriving per-source health from existing data — `max(created_at)` per `TransactionSource` + pension/municipal uploads; cadence thresholds (BAC ≈ daily-weekly, salary biweekly/monthly, municipal/pension monthly) → OK/warning per source. Frontend page + sidebar badge on warning.
- v2 (L): n8n execution-status integration (n8n DB is already queryable per CLAUDE.md) for actual failure detail.
- *Accept:* stop an n8n flow for N days → visible warning in-app.

### 4.2 Import review & dedup transparency (UX-20, BE-08)
- Import responses list skipped duplicates (incoming row + matched existing row); UI renders the comparison with "import anyway" override; tighten the reference-hash inputs (include `card_last4`).
- *Accept:* paste-import of a file with near-duplicates shows exactly what was skipped and why; override works.

### 4.3 Localization & formatting unification (UX-19, FE-17, UX-07 — T13)
- Single `src/lib/dates.ts` using `Intl` with `es-CR`; delete the three per-page month-name arrays; sweep `en-US` usages; translate stray English labels.
- *Accept:* `grep -r "en-US" src/` returns nothing; one source of truth for month names.

### 4.4 Transactions power features (Wishlist #1, #2; UX-04)
- Search across all sources (fix the inert cash-tab search first — S); backend `q` param searching merchant/notes/reference; filter bar (date range, amount range, category multiselect); multi-select with bulk re-categorize / defer / delete-with-undo.

### 4.5 Undo & destructive-action safety (UX-10)
- Toast-with-Undo (5s delayed API call) for transaction and recurring-item deletes — no schema change needed.

### 4.6 Mobile table experience (UX-11)
- Card-stack rendering under 640px for the transaction DataTable (merchant/amount primary; category/bank secondary).

### 4.7 Module polish batch (each S–M)
- Shared month/period navigator across Budget/Proyecciones/Analytics; Pensions history navigation (UX-03)
- Deferred-transaction visibility: modal field + row styling + toast (ARCH-11, UX-05)
- Editable pension assumptions persisted server-side (UX-12)
- Cycle filter on all three Analytics charts (UX-13)
- Empty states with "add first item" actions (UX-14); dirty-form guard (UX-06); balance-override helper text (UX-21); breadcrumb from Proyecciones (UX-22); meter labels (UX-24); a11y `aria-pressed` sweep (FE-16); scroll-container fix (UX-25)

### 4.8 Export & assistant value (ARCH-18, UX-16, UX-23)
- CSV export endpoint + buttons for transactions/salarios/pensions; copy/export on `SpendingSummaryCard`.

### 4.9 Wishlist backlog (unscheduled)
- What-if scenario planner · exchange-rate history & per-date override · PWA polish · pension contribution calculator · per-month recurring-item overrides.

---

## Sequencing & Dependencies

```
Phase 0 (1 day) ──► Phase 1 (1 wk) ──► Phase 2 (1 wk) ──► Phase 3 (1 wk) ──► Phase 4 (2–3 wks)
 security            tests ◄─────────── needs tests        needs 2.2 manifest   needs 3.x for
 (no deps)           alembic ◄────────── needs alembic      (for UX-09)          error patterns
                     cycle truth         decimal, uploads,                       4.1/4.2 need
                     datetime            constraints, agent                      backend endpoints
```

Hard dependencies: **1.2 before any budget-math edit** · **1.3 before any schema change (2.1, 2.3, 4.2)** · **1.1 before 2.1** · **3.1 before 3.2** (error states come from the query layer) · **2.2 before the UX-09 part of 3.2**.

Parallelizable: Phase 0 anytime; 4.3 (localization) anytime; 3.x frontend work can overlap 2.x backend work except where noted.

## Definition of Done (whole plan)

- App refuses to boot with default secrets; login rate-limited; CORS pinned; uploads validated.
- `pytest` + `pnpm typecheck` gate deploys; cycle math is test-pinned against real statements.
- Schema changes ship as Alembic migrations only; money is `NUMERIC`/`Decimal` end-to-end.
- No silent failures: every page has error+retry; every mutation has visible feedback; ingestion health is observable in-app.
- One locale, one date formatter, one month-navigation pattern.