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

Improvement Plan — WealthySmart

Phased plan derived from 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.