# 03 — Code Review (Δ since 2026-08-20 baseline + current-generation review)

Method: live tree synced from `/var/www/unified` (byte-diff vs repo `app/` = zero src/server drift), git archaeology (`reflog`, `-S` string history, branch/stash audit), container + cron-log probing, DB probes via `supabase-db`. Focus = the delta since the Aug-20 audit and the Sep-15→18 wave.

## Review verdicts — current generation (Sep 15–18 wave)

### ✅ Sound (verified, keep)
1. **Admin Reporting rebuild** (`admin/reporting/*`, migrations 030/031 applied live 2026-09-17): `/reimb/reporting` + `/export` both behind `requireAdmin`; CSV escaper present; saved views table live; 45/45 QA noted at ship. Clean.
2. **Redesign ports (Sep-16/17)** left almost no token bypass: only **3** literal-gray utility classes remain across `src/pages/admin/**` (0 in CoverageCenter). The Sep-16 redesign wave genuinely cleaned the kit — this is the strongest part of the current tree.
3. **Reminder fix** (HEAD `60c3d56`): `case_date` normalization + 24h dedup; container logs show **0 eval-notify failures** in the 10 h after the 00:55 redeploy (was ~1000 failures since Sep 16).
4. **Public routers** (`replay.ts` 59 L, `public-redesign-decisions.ts` 221 L) — explicit documented access policy, verified-token gate on writes, per-route rate-limit. Deliberate design, not accidental exposure.
5. **Zero TODO/FIXME/HACK/XXX** across `src/` + `server/` (grep, excluding .bak). Rare for a codebase this size.
6. **Migrations tracked + fail-loud** (`unified._migrations`); all 31 applied; boot-gated via `/api/health` 503 on failure.

### ⚠️ Problems (this run's findings, chronologically evidenced)

**CR-1 · P0 (governance/correctness) — The Reversion Incident: 19+ Aug remediation fixes are gone from live AND git.**
- Evidence chain: `app/package.json` never contained `typecheck`/`engines` in ANY commit (`git log --all -S` empty); container runs express **4.22.2** (the Aug-21 log claims 5.2.1 live); `Page 1 of 1` back at `CallSchedulePrint.tsx:296`; `host.docker.internal` hardcoded in 3 pools (fix kept only in orPool); no `voiceSessionPurge.ts`; no `ipKeyGenerator`; no PDF route/pdfkit; `/api/ready|live` back to 308 (aliases gone, real probes `/api/health/ready|live` still 200).
- Root cause: all remediation was applied **directly on the live tree / in the container**, never committed. `deploy.sh` rsyncs `server/ src/ package.json …` from the repo on every run → silently overwrites live-tree edits. Sep-7 `_clobbered-20260907-1526` event + Sep-17 `snapshot: pre-port baseline` commit then froze the reverted state into git ("wip: partial ports on stale baseline — _do not deploy_").
- Recovery audit: **no backup contains the fixes** (Sep-9 rename tarball + Sep-17 pre-port tarball both verified absent) → recovery = re-implement from the remediation log (which is detailed) + commit through git this time.
- Also: repo is **37 commits ahead of `origin/master`** (last push Sep 4) → `.github/workflows/ci.yml` has never executed; git is not functioning as the safety net.

**CR-2 · P1 — Role source-of-truth regression = live user-facing defects.** (B-02 class; see 34 + DB evidence.)
- `server/middleware/auth.ts` derives `category` from CRM (`public.contacts`) again; the beneficiary_type fix is gone.
- Live data: active reimb residents — **Danzig = CRM 'Faculty'** (roster loss patched manually via app_access, but eval-management flags + role display still wrong; EZ id zero-padded mismatch `000517772` vs `517772`); **Karki + Kassam = 'Archived'** (login hard-blocked at middleware; reimb rows still `is_active` → still counted in "active residents"); **Allen = 'Fellow'** (mild); **5 residents with empty email/EZ ghost rows** (Cheng, Nauheim, Loloi, Wallace, Dave — created 2026-08-12, matched to an empty-email app_access row); **Sabin Sajin duplicated ×2, no email**; 1 junk `app_access` row + 2 contacts with empty email.
- Faculty: 7 rows typed `faculty` carry CRM 'Staff' (likely intentional program-staff); 24 of 27 faculty reimb rows have no category match by EZ id (format variance) — recon further in S3 role lens.

**CR-3 · P2 (performance regression) — Admin lazy-split lost.** `Admin.tsx` has zero `lazy()`; `Admin` chunk = 349 KB raw / 54 KB gz (Aug-21 dock: 83 KB post-split). All admin tabs eager again.

**CR-4 · P2 — Sep-17 /schedule critique partially reverted.** `ScheduleViewTab.tsx:193` still reads `Live · updates on load` (fix #3 claims "Updates on load" shipped); `sessions` / `sites covered` strings unguarded at :209/:213 (fix #2 partially lost). Fixes #4 ("Every edit saves straight to the live schedules") and #1 (chronological clinic sort) survived. Fix #5 verified in S4.

**CR-5 · P2 (hygiene/sensitive) — Live tree carries non-app scratch material:** amazon_*/ebay_*/fanatics scrapers + captures, `cookies.txt` (!), `check_*.py`, `.q*.cjs`, `qa_*.cjs`, `verify_build.sh`, 1.1 GB total incl `.venv/`, `dist.old`, `curl_imp/`. Deploy dir doubles as workstation — enlarges blast radius and audit surface. (Repo `app/` mirrors most of it.)

**CR-6 · P3 — Debris and stale copies:** 15 live `*.bak-*` files (incl. `server/index.ts.bak-*`), `.bak` siblings in `src/pages`, 5 files where live ≠ repo are stale non-deployed docs (`README.md`, `docs/ops/dr.md` say "QGenda" while code/README-repo say "roster"; `e2e/api.spec.ts` live copy hits `/api/qgenda/shifts` — an endpoint that no longer exists; repo copy correct).

**CR-7 · P3 — Runtime warnings live in container logs:** express-rate-limit IPv6 ValidationError (bypass-wise relevant on IPv6); pg `client.query() already executing` deprecation (will break at pg@9); npm SIGTERM noise on restart (benign).

**CR-8 · P3 — Migration prefix collisions:** two `026_*`, two `027_*` files (all applied, tracked by full name) — rename hygiene before the numbering matters.

## Positives carried from baseline (still true)
- Atomic write paths for swaps/reimb; fail-closed auth; zod on inputs; pino+Sentry observability; scheduler idempotent + try/catch wrapped; health endpoint genuinely deep (5 pools + migrations).
- Sep-wave commit messages are receipt-grade (per-commit verification notes) — the discipline exists at commit time; it just doesn't extend to keeping git/deploy the source of truth.