admin/WealthySmart
1
0
Fork 0
mirror of https://github.com/escalante29/WealthySmart.git synced 2026-07-17 11:08:47 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity

Add five-hat codebase review and phased improvement plan

Browse Source 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Carlos Escalante
2026-06-09 19:13:19 -06:00
parent 20b4ad102d
commit 5c265129e8
8 changed files with 1184 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
codebase-review/README.md Normal file
View File

@@ -0,0 +1,45 @@
# 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.