WealthySmart/codebase-review/PROGRESS.md

# Implementation Progress

Tracking execution of [07-improvement-plan.md](07-improvement-plan.md). Newest entries first within each phase.

---

## Phase 0 — Security Hardening Sprint ✅ DONE

**Completed and deployed to production 2026-06-09** (deploy task 81, verified live).

| Plan item | Status | Commit |
|---|---|---|
| 0.1 Fail-fast configuration (SEC-01, BE-15) | ✅ | `15ea1ef` |
| 0.2 Login hardening: compare_digest + 5/min/IP rate limit (SEC-02, SEC-03) | ✅ | `b412370` |
| 0.3 CORS pinned to prod domain + localhost dev (SEC-04) | ✅ | `b412370` |
| 0.4 7-day tokens (was 30) + COOKIE_SECURE setting (SEC-05, SEC-08) | ✅ | `15ea1ef`, `b412370` |
| 0.5 Security headers via Hono middleware (SEC-07, FE-08) | ✅ | `bebd7b5` |
| 0.6 Paste-import 1 MB cap / proxy 502 / 401 loop guard (ARCH-05, FE-23, FE-22) | ✅ | `3a357e4`, `bebd7b5`, `ad29cce` |
| 0.7 Housekeeping: dead AgentHomeClient removed, dev compose creds → .env, .env.example | ✅ | `4a758b2`, `15ea1ef` |

**Production verification (2026-06-09):**
- Health 200 after deploy; frontend+backend containers recreated, healthy
- `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy` live on prod responses
- Login rate limit live: 6th rapid attempt → **429 with `Retry-After: 58`**
- CORS live: `evil.example` origin gets no allow header; app origin allowed
- Fail-fast config: behaviorally tested (rejects missing/default/weak secrets in 4/4 cases); prod booted with real Gitea-injected secrets

**Incidents & discoveries during rollout:**
- Deploy task 80 **failed** on a latent CI bug unrelated to Phase 0: unpinned corepack resolved pnpm 11, which hard-errors on unreviewed dependency build scripts (`ERR_PNPM_IGNORED_BUILDS`). Fixed in `d874cf5` by pinning `packageManager: pnpm@10.33.1` and completing the build-script review in `pnpm-workspace.yaml` (approve `@swc/core`, `esbuild`; ignore `@scarf/scarf`). A failed build never touches running containers — prod was unaffected.
- The Gitea repo is a **pull-mirror** (no direct pushes). Deploys are triggered by pushing to GitHub and then running a mirror-sync via the Gitea API from the VPS (token via `gitea admin user generate-access-token`, POST `/repos/admin/wealthysmart/mirror-sync` from inside the container — port 3000 is not published on the host).
- **Dev credential change:** dev no longer accepts `admin/admin`. The generated dev password lives in the gitignored `.env` (old file backed up as `.env.bak-phase0`). Prod credentials unchanged.
- Two temporary `phase0-sync-*` Gitea tokens should be revoked in Gitea UI → Settings → Applications.

**Corrections to the review docs found during implementation:** none beyond those already noted in [README.md](README.md).

---

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

Completed 2026-06-09.

| Plan item | Status | Commit |
|---|---|---|
| 1.1 pytest harness, 55-test suite (no network, SQLite fixtures) | ✅ | `c725c14` |
| 1.1 CI gate: test job (pytest + typecheck) blocks deploy | ✅ | (workflow) |
| 1.2 Cycle-math characterization + month validation (BE-07) | ✅ | `0a4c3e9`, `c725c14` |
| 1.3 Alembic adoption: autogen baseline + stamp-or-upgrade startup | ✅ | `9a25c4b` |
| 1.4 Datetime sweep: 29 utcnow sites → `app.timeutil.utcnow()` | ✅ | `996bd43` |
| 1.5 Decimal-equivalence baseline (float behavior pinned by tests) | ✅ | `c725c14` |
| Pre-req: all 20 frontend type errors fixed, typecheck green | ✅ | `49b96ed` |

**Key findings & decisions:**
- **BE-06 is a FALSE POSITIVE.** `get_cycle_range(Y,M)` returns the cycle *starting* M/18; budget month M composes `get_previous_cycle` → `[(M-1)/18, M/18)`, exactly as the code comment and the frontend convention state. Empirically validated against prod: May 2026 budget's credit-card figure reproduced by independent SQL over the cycle window (95 transactions exactly, refunds to the colón, 0.13% residual from a single EUR transaction the simplified SQL can't convert). Convention now pinned by `tests/test_cycle_math.py` and the docstring.
- **Datetime strategy:** all DB columns are naive `TIMESTAMP`, so the sweep deliberately keeps naive-UTC semantics via a shared `app/timeutil.py::utcnow()` (non-deprecated API, zero behavior change). Going tz-aware would have poisoned naive/aware comparisons. TIMESTAMPTZ migration deferred to Phase 2 — now a one-line change.
- **Alembic baseline** (`c3da001a0eb3`) autogenerated against empty Postgres, then schema-diffed against the live schema until column-identical. Two model fixes fell out: `deferred_to_next_cycle` now declares its `server_default` (live DBs have it from the legacy ALTER), `usersettings.data` is JSONB on Postgres (matching live) via a dialect variant. Startup stamps pre-Alembic DBs, upgrades fresh ones, under a pg advisory lock (prod uvicorn runs 2 workers → lifespan runs twice).
- Verified locally: adoption + idempotent re-run on dev DB, fresh 13-table build on empty DB, full dev-container boot through the Alembic path, 55/55 tests, typecheck 0 errors, production vite build.

**Dev environment note:** backend dev tooling lives in `backend/.venv` (gitignored); `pytest` and `alembic` run from there. See CLAUDE.md for the migration workflow.

---

## Phase 2 — Backend Correctness & Robustness ✅ DONE

Completed 2026-06-10.

| Plan item | Status | Commit |
|---|---|---|
| 2.1 Money → Decimal/NUMERIC (29 columns, BE-02) | ✅ | `74886fb` |
| 2.2 Upload hardening: 10 MB cap, %PDF magic, to_thread (SEC-06, BE-14) | ✅ | (uploads) |
| 2.3 FK delete rules, grouped queries (10→3), N+1 fixes, tiebreakers | ✅ | `3cdd7d9` |
| 2.4 Error visibility: per-source rate logging, freshness, clear agent errors | ✅ | `82f10a5` |
| 2.5 Agent cleanup: real multipliers, year bounds, offsets, read-only policy | ✅ | `82f10a5` |
| 2.6 Consistency: enums, keyword status codes, SimpleCookie, import hygiene | ✅ | `440da3e` |

**Key decisions:**
- **Decimal strategy:** `Money = Annotated[Decimal, PlainSerializer(float, when_used="json")]`
  — storage is exact `NUMERIC(15,2)` (rates 15,6) while JSON keeps emitting plain
  numbers, so the frontend contract is unchanged (pinned by
  `test_models_serialization.py`). Services coerce to float at their computation
  boundaries. Migration `7f567c8bafdb` verified on dev: per-row values and table
  sums byte-identical across the type change.
- **FK rules** (`7884505b16b3`): transactions/recurring items `SET NULL` on category
  delete — this also fixes the 500 that `DELETE /categories` raised for in-use
  categories; water readings `CASCADE`. Verified with a rolled-back probe on dev.
- **Agent net worth** no longer guesses (hardcoded 600 CRC / 1.08 EUR removed): it
  converts via `get_crc_multipliers` and reports unconvertible accounts explicitly.
- **Read-only tool policy** documented in `agent/tools.py` (SEC-09): ingested email
  text is a prompt-injection surface; any future write tool needs UI confirmation.

**Review findings closed as overstated/false during implementation:**
- BE-09 (temp-file leak): parsers already use context-managed temp files.
- BE-10 (per-file commits): the pension batch already commits atomically.
- ARCH-08 (swallowed cancellation): `CancelledError` is a `BaseException` since
  Python 3.8 — `except Exception` never caught it.
- ARCH-03 (filter precedence): `if cycle / elif dates` is already explicit.
- BE-22 (upsert race): accepted as-is — single writer (daily n8n cron), batch-
  scoped commit; ON CONFLICT complexity not warranted.

Tests: 55 → **69** (new: model serialization, agent tools, upload validation).

**Production verification (2026-06-10, deploy after CI test gate):** boot logs show
both migrations applied in order; `alembic_version = 7884505b16b3`;
`transaction.amount` is `numeric(15,2)`; FK rules live (`SET NULL`×2, `CASCADE`);
data intact through the type change (238 transactions, sum unchanged); live API
emits amounts as JSON numbers.

---

## Phase 3 — Frontend Data Layer & Feedback ✅ DONE (3.4 deferred)

Completed 2026-06-10.

| Plan item | Status | Commit |
|---|---|---|
| 3.1 TanStack Query: provider, api timeout/signal, useBudget, Budget | ✅ | `65dffda` |
| 3.1 All remaining pages on queries with ErrorState+retry | ✅ | `7a1733b` |
| 3.2 Sonner toasts on every mutation; confirm on override clear | ✅ | (toasts) |
| 3.3 Auth probe: network failure ≠ logged out (retry once) | ✅ | (auth) |
| 3.5 Lazy context init, CK repair logging, upload-result cap | ✅ | (auth/pages) |
| 3.4 OpenAPI type generation | ⏸ **deferred** | — |

**What changed for the user:** every page now shows an explicit error panel with
a Reintentar button when the API fails (previously: silent empty charts/tables);
every mutation (transaction save/delete, deferred toggle, recurring items,
balance overrides) confirms success or failure with a toast; month navigation
keeps the previous month visible while loading; rapid filter/month changes can
no longer paint stale results (request cancellation); all requests time out at
30s instead of hanging; clearing a balance override asks for confirmation; the
privacy mask and theme no longer flash wrong on reload; a transient network
blip at app start no longer kicks an authenticated user to the login page.

**Architecture:** TanStack Query with `staleTime` 30s and one retry;
`['budget']`, `['transactions']`, `['analytics']`, `['salarios']`,
`['pensions']`, `['municipal']` key families; mutations invalidate their family.
`useBudget` keeps its original return shape so consumers didn't churn.

**3.4 (OpenAPI type generation) deferred to its own pass:** it needs the backend
`response_model` sweep (ARCH-16) first and touches every endpoint plus the
api.ts type exports — a separate, mechanical change with its own verification.
The hand-written types are currently accurate (typecheck is green against real
responses). Revisit after Phase 4's backend endpoints land.

**Closed as already-handled during implementation:** FE-04 (the Pensions ROI
fallback already guarded `len >= 2`).

## Phase 3 — Frontend Data Layer & Feedback ⏳ NOT STARTED

## Phase 4 — UX, Trust & Features 🔄 ITERATION 1 DONE

Iteration 1 completed 2026-06-10. The plan marks 4.x items independent; this
iteration delivered the highest-impact slice.

| Plan item | Status | Commit |
|---|---|---|
| 4.1 Sync Status page + nav badge (UX-17, the #1 trust feature) | ✅ | `4ceb675` |
| 4.3 Localization: shared es-CR dates module, stray labels | ✅ | (l10n) |
| 4.5 Undo grace (5s + Deshacer) for transaction deletes | ✅ | `629d118` |
| 4.8 CSV export endpoint + Budget/Salarios buttons | ✅ | `571428f` |
| 4.7 picks: aria-pressed, deferred row styling, override hint | ✅ | `b3a7f51` |

**Sync Status design:** per-source health derived entirely from existing data
(`max(created_at)` per source + cadence thresholds: BAC 7d, salary 35d,
municipal/pensions 40d, exchange rate 1d) — no n8n integration required for v1.
Endpoint tested; verified live (it immediately flagged the dev DB's stale data).

**Deferred to iteration 2** (each independent): 4.2 import review & dedup
transparency (L — includes the reference-hash tightening, which must migrate
existing hashes to avoid breaking dedup), 4.4 transaction search-everywhere is
**not needed** (search already applies to all sources — UX-04 was a false
positive) but bulk operations remain (L), 4.6 mobile card layout (M), 4.7
remainder: shared period navigator, editable pension assumptions, cycle filter
on all Analytics charts, empty-state actions, dirty-form guard, breadcrumb,
meter labels. 4.9 wishlist unscheduled.