mirror of
https://github.com/escalante29/WealthySmart.git
synced 2026-07-17 11:08:47 +02:00
46 lines
4.0 KiB
Markdown
46 lines
4.0 KiB
Markdown
# WealthySmart — Five-Hat Codebase Review
|
||
|
||
**Date:** 2026-06-09
|
||
**Scope:** Full repository (`backend/`, `frontend/`, compose files, CI workflow). `tech_docs/`, `node_modules/`, and `.claude/` excluded.
|
||
**Method:** Five independent senior/staff-level review agents, each with a distinct lens, followed by cross-verification of the highest-severity claims against the actual repo state and a consolidated synthesis.
|
||
|
||
## The Five Hats
|
||
|
||
| Hat | Report | Focus |
|
||
|---|---|---|
|
||
| 🏛 Architect | [01-architect.md](01-architect.md) | System structure, layering, data model, migrations, deploy topology |
|
||
| ⚙️ Backend | [02-backend.md](02-backend.md) | FastAPI/SQLModel correctness, money/date math, queries, PDF parsing, agent tools |
|
||
| 🎨 Frontend | [03-frontend.md](03-frontend.md) | React data fetching, state, type safety, a11y, Hono server, CopilotKit |
|
||
| 🔐 Cybersec | [04-security.md](04-security.md) | AuthN/Z, secrets, CORS, rate limiting, uploads, injection surface |
|
||
| 👤 Poweruser | [05-poweruser.md](05-poweruser.md) | Daily-use UX, feedback loops, data trust, consistency, feature gaps |
|
||
|
||
## Synthesis Documents
|
||
|
||
- **[06-weak-spots-inventory.md](06-weak-spots-inventory.md)** — All 90+ findings deduplicated into 13 cross-cutting themes with a severity matrix.
|
||
- **[07-improvement-plan.md](07-improvement-plan.md)** — Phased implementation plan (Phase 0–4) with concrete steps, acceptance criteria, and effort estimates.
|
||
|
||
## Editorial Corrections (verified against the repo)
|
||
|
||
Agent findings were spot-checked. One material correction:
|
||
|
||
- **`.env` / `.env.prod` are NOT committed to git.** Both are covered by `.gitignore` (`.env`, `.env.*`) and `git log --all --diff-filter=A -- .env .env.prod` returns nothing. The Architect's ARCH-01 ("secrets in version control", Critical) is **downgraded**: the files exist only in the working tree, which is expected. The *real* residual risks are (a) hardcoded fallback credentials in `backend/app/config.py` (verified: `SECRET_KEY = "change-me-in-production"`, `ADMIN_PASSWORD = "admin"`) and (b) hardcoded dev DB credentials in `docker-compose.yml`. These are tracked as SEC-01 / BE-15.
|
||
|
||
Claims that were verified as accurate: CORS `allow_origins=["*"]` with `allow_credentials=True` (`backend/app/main.py:81`), 30-day token expiry (`backend/app/config.py:7`), default admin credentials (`backend/app/config.py:9`), zero test files in the repo, and no Alembic directory.
|
||
|
||
## Top 10 — If You Only Fix Ten Things
|
||
|
||
1. **Fail fast on default secrets** — startup must refuse to run with `SECRET_KEY="change-me-in-production"` or `admin/admin` (SEC-01, BE-15). *S*
|
||
2. **Rate-limit + constant-time login** — both login endpoints have unlimited attempts and `!=` comparison (SEC-02, SEC-03). *S*
|
||
3. **Restrict CORS** to `https://wealth.cescalante.dev` (SEC-04). *S*
|
||
4. **Add a test suite** — zero tests guard the most complex logic in the app (billing-cycle math, deferred carryover, PDF parsing) (ARCH-06, BE-25). *L*
|
||
5. **Adopt Alembic** — schema evolution is ad-hoc `ALTER TABLE` strings in `db.py`; memory says "never reset prod DB," which makes real migrations essential (ARCH-02, BE-19). *M*
|
||
6. **Verify billing-cycle boundary math** — possible off-by-one in `get_cycle_range` would misalign every monthly budget (BE-06). *S to verify, with tests*
|
||
7. **Migrate money fields from `float` to `Decimal`/`Numeric`** (BE-02). *M*
|
||
8. **Validate uploads** — size cap, `%PDF` magic-byte check, per-file transactional rollback (SEC-06, BE-10). *S*
|
||
9. **Frontend error/feedback layer** — surfaced errors, success toasts, request cancellation; today failures are silent `console.error` (FE-01/02/03, UX-01/02). *M*
|
||
10. **n8n sync visibility** — silent parser failures mean missing transactions go unnoticed; add a sync-status/reconciliation view (UX-17, UX-20). *L*
|
||
|
||
## Reading Order
|
||
|
||
For a quick pass: this README → [06-weak-spots-inventory.md](06-weak-spots-inventory.md) → [07-improvement-plan.md](07-improvement-plan.md). The five hat reports hold the file:line evidence behind every consolidated item.
|