Files
WealthySmart/codebase-review/PROGRESS.md
Carlos Escalante 248417dbab
All checks were successful
Deploy to VPS / test (push) Successful in 1m43s
Deploy to VPS / deploy (push) Successful in 2m20s
PROGRESS: deferred items and wishlist complete — nothing remains
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-06-12 18:07:31 -06:00

13 KiB
Raw Blame History

Implementation Progress

Tracking execution of 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.


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 DONE (iterations 1 + 2)

Iteration 2 (2026-06-11)

Plan item Status
4.2 Import review: skipped duplicates listed with matched row + import-anyway override (re-imports only skipped lines)
4.4 Bulk operations: POST /transactions/bulk + selection UI (assign category, defer/restore, delete)
4.7 Editable pension assumptions (UserSettings-backed)
4.7 Dirty-form guard (UX-06), empty-state CTA (UX-14)
BE-08 hash tightening superseded — the override solves the practical harm; rehashing would orphan every existing reference
UX-11 mobile layout false positive — a card layout already existed (two real bugs in it fixed: undo-pending rows showed, en-US dates)
UX-13 cycle filter mostly false positive — daily-spending already accepts cycle params; the trend is multi-cycle by design
UX-24 meter labels, UX-22 breadcrumb, shared period navigator (UX-03) not done — needs dedicated UI; lowest-value remainder

The improvement plan is complete — and as of 2026-06-12, so is everything that had been deferred:

Final batch (2026-06-12)

Item Status
3.4 OpenAPI codegen: response_model sweep + generated api-types.gen.ts + CI drift gate (caught 2 real drifts: missing SAVINGS enum value, untyped raw_charges)
UX-03 shared PeriodNavigator · UX-22 breadcrumb · UX-24 meter labels
Wishlist: rate-history chart (Analytics), contribution calculator (Pensiones), what-if Planificador page, PWA installability
Bonus fix: sw.js was a self-unregistering stub — serviceWorker.ready never resolved, so web push had been silently dead; replaced with a working worker

Tests: 82. Final false-positive tally across the whole plan: BE-06, BE-09, BE-10, ARCH-03, ARCH-08, BE-22 (accepted), FE-04, UX-04, UX-11, UX-13 — the characterization-tests-first approach caught all of them before any "fix" landed. Nothing from the review or wishlist remains.

Iteration 1 (2026-06-10)

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.