docs(review): add handoff document for UI audit work

Review/handoff-ui-audit.md is a focused handoff for a fresh agent taking over the UI/UX audit and fix cycle (Sprints 1, 2, 3). It complements docs/HANDOFF.md (project-wide) rather than duplicating it. Covers: - Commit table (f3e4a44 / ccc70aa / f5fb755 / e90a9d6) and deploy status (Sprint 1 deployed, Sprints 2-3 awaiting deploy). - Per-sprint file-level diff summary so the next agent can audit the changes without re-reading the audit doc. - Environment quirks: deployment host is not this machine; psql lives in the db container; frontend/src/lib/ is force-added because of a pre-existing .gitignore bug; pre-existing WIP in git status; ESLint not configured. - Active risks: backend migration not yet run; Dashboard Undo rebuilds not restores; S3.5 a11y caveats. - The 9 explicit follow-up items in audit \xa7Future. - Quick-start for the next agent.
2026-06-03 18:58:52 -07:00
parent e90a9d6683
commit 427d8ac352
1 changed files with 230 additions and 0 deletions
@@ -0,0 +1,230 @@
+# UI/UX Audit & Fix — Agent Handoff
+
+You are taking over a 3-sprint UI/UX audit and fix cycle. All code changes are committed and build green. The user's deployment host (Tailscale `100.108.224.12`) is the only environment you should touch for verification — the local repo on this machine (`/home/peter/Projects/MealPlanner`) was the editing host; the running app lives elsewhere.
+
+**Date of handoff: 2026-06-03.**
+
+---
+
+## TL;DR
+
+Three commits land all 14 audit findings from `Review/ui-nielsen-audit.md`:
+
+| Sprint | Commit | Scope | Build | Deploy |
+|---|---|---|---|---|
+| 1 | `f3e4a44` | 5 P0 blockers | ✅ green | ✅ deployed 2026-06-02 by user |
+| 2 | `ccc70aa` + `f5fb755` | 6 P1s + bonus S3.3 | ✅ green | ⚠️ not yet deployed |
+| 3 | `e90a9d6` | 3 P2s + a11y sweep | ✅ green | ⚠️ not yet deployed |
+
+All work is on `main` ahead of `origin/main` (pre-existing WIP also present). All three sprints compile. **Sprint 1 is live. Sprints 2 and 3 are not yet live on `100.108.208.56:8082/`.**
+
+**Next action:** the user runs the deploy commands in `Review/sprint2-verification.md` and `Review/sprint3-verification.md` on the deployment host, then smoke-checks per the checklists. After verification, any remaining items move to the §Future backlog in `fix-ui-audit.md`.
+
+---
+
+## Where the work lives
+
+```
+/home/peter/Projects/MealPlanner/
+├── Review/
+│   ├── ui-nielsen-audit.md          # The audit (276 → ~340 lines). Status blocks per sprint at the top.
+│   ├── sprint2-verification.md      # Deploy + smoke checks for Sprint 2 (BACKEND MIGRATION INCLUDED).
+│   ├── sprint3-verification.md      # Deploy + smoke checks for Sprint 3 (frontend only).
+│   ├── docs-claude.md               # Prior docs adversarial review (not yours).
+│   ├── docs-gpt5.5.md               # Prior docs adversarial review.
+│   ├── repo-gpt5.5.md               # Prior repo adversarial review.
+│   ├── reviewconcensus.md           # Prior review consensus.
+│   └── synthesis.md                 # Prior review synthesis.
+├── fix-ui-audit.md                  # The plan (232 lines). Implementation notes per task + commit refs.
+├── docs/HANDOFF.md                  # The project-wide handoff (don't duplicate — read for context).
+└── docs/ORIENTATION.md              # Project orientation.
+```
+
+**Read first**, in this order: `docs/ORIENTATION.md` → `docs/HANDOFF.md` → `Review/ui-nielsen-audit.md` (the audit) → `fix-ui-audit.md` (the plan) → the two `sprintN-verification.md` files.
+
+---
+
+## Audit findings — all closed
+
+The audit has 14 findings + 3 a11y items. They map 1:1 to the plan's task list. All have code committed. Some have known caveats — read the audit's "Sprint 1/2/3 status" blocks at the top of `Review/ui-nielsen-audit.md` for the per-finding deploy status.
+
+**P0 blockers (Sprint 1, `f3e4a44`):**
+- B1 Recipe ingredients fused unit+name (`RecipeDetail.tsx:161`) — `.trim()` removed.
+- B2 Meal ingredients missing quantities (`MealDetail.tsx:248-252`) — `qty` field; also `ing.ingredient?.name` fallback.
+- B3 `$N/A per serving` (`MealDetail.tsx:191`) — conditional on `cost != null`.
+- B4 `/recommended` blank page — `*` NotFound catch-all + `/recommended` alias to `/recipes/recommended`. New `pages/NotFound.tsx`. `EmptyState` extended with optional `to` prop.
+- B5 Mobile empty meal slots hidden (`Dashboard.tsx:164,219`) — `hidden md:*` removed; `min-h-11` (44 px) on the Generate button.
+
+**P1 (Sprint 2, `ccc70aa` + `f5fb755`):**
+- B6 Meal-card title 1-line truncate — `line-clamp-2`; image shrinks 56→40 on `<md`.
+- B7 MealDetail hero overlap + spoonacular SEO copy — hero reworked, `cleanDescription()` in `lib/utils.ts` strips 14 boilerplate patterns, raw text → "Notes from source" disclosure.
+- B8 Pantry aisle free-text → canonical select — `PANTRY_AISLES` enum, `Select` components, ingredient name marked `*`. **Backend migration `0015_normalize_pantry_aisles.py` normalizes `ingredient.aisle` + `grocery_item.aisle` in a single transaction; dry-run at `backend/scripts/dry_run_aisle_migration.sql`; persistent backup at `backend/scripts/persist_aisle_backup.sql`.**
+- B9 ShoppingList snake_case aisle labels — `AISLE_LABEL` map + `aisleDisplay()`.
+- B10 Mobile pantry table — `role="region"` + right-edge white-to-transparent gradient hint.
+- B11 Recipes filters — `applied`/`pending` state split, Apply/Reset buttons, active-count chip on Filters button.
+
+**P2 + a11y (Sprint 3, `e90a9d6`):**
+- B12 Undo-toast replaces `confirm()` — `lib/toast.tsx` (renamed from `.ts` for JSX) gains `showToast.undo(message, onUndo, ms=5000)`. Dashboard undo re-fires `generateItem` to refill the slot (recipe may differ — documented in plan §R4). Pantry undo re-fires `pantry.add` with original fields (fully reversible).
+- B13 Mobile nav wrap — `whitespace-nowrap` + `px-2 sm:px-3`.
+- S3.3 Mobile shopping-list stat grid (3-col on all viewports) — done in Sprint 2.
+- S3.4 ErrorBoundary already mounted at `App.tsx:42` — verified, no code change.
+- S3.5 A11y sweep — `<nav aria-label="Primary">`, `aria-current="page"`, `<main id="main-content">`, `Badge` extended with `icon` + `aria-label` props, approval-status Badge uses `aria-label="Approval status: …"`.
+
+---
+
+## Environment quirks you MUST know
+
+### 1. The deployment host is not this machine
+
+This repo lives on a development host (Tailscale `100.108.146.47`). The live deployment is on the user's home server at Tailscale `100.108.224.12`, served at `http://100.108.208.56:8082/`. The deployment host's Docker stack is the source of truth for the running app.
+
+When you `docker compose up -d --build` *here*, you rebuild images on this host's Docker daemon, not the deployment host's. The frontend I built in Sprint 1 was verified by the user *manually* deploying from this repo on the deployment host.
+
+**Don't try to push to the deployment host.** The user's workflow is: commit locally, then `git pull` on the deployment host and rebuild there.
+
+### 2. The db is in a container; no host psql
+
+On the deployment host (`docker-ubuntu`), `psql` is not installed. All DB commands must run via `docker compose exec db psql -U mealplanner -d mealplanner -f /dev/stdin < script.sql`. The `Sprint 2` verification doc has the exact incantations.
+
+**On the local dev host** (this one), `psql` IS available because the same `db` service runs locally for the user's dev work. But the DB used in dev is *different* from the one the user is migrating (Tailscale host). The dry-run I ran on this host showed 21,196 rows would change; the deployment-host DB will have a different count.
+
+### 3. `.gitignore` blocks `frontend/src/lib/`
+
+There's a pre-existing repo bug: `.gitignore` line 17 is `lib/` (the Python `lib/` ignore), and it catches `frontend/src/lib/`. New files in `frontend/src/lib/` need `git add -f`. Existing tracked files (`utils.ts`) work; the new `toast.tsx` was force-added in commit `e90a9d6`. Be aware when adding new files there.
+
+### 4. The `.env` file is at the project root
+
+It contains `SESSION_PASSWORD=test-family-password`, `APP_BASE_URL=http://100.108.224.12:8082` (Tailscale host), real SendGrid keys, real Ollama creds. Don't commit it (`.env` is in `.gitignore`). Don't `cat` it casually.
+
+### 5. ESLint is not configured
+
+`npm run lint` errors with "ESLint couldn't find a configuration file." That's a pre-existing repo gap, not introduced by this work. The `build` script (`tsc && vite build`) is the typecheck.
+
+### 6. Pre-existing WIP is in `git status`
+
+When you `git status`, you'll see modifications to `backend/app/api/meals.py`, `backend/app/api/recipes.py`, `backend/app/schemas/recipe.py`, `frontend/src/api/index.ts`, `nginx/nginx.conf`. These are NOT mine — they were already in the working tree from prior commits. Don't commit them as part of UI audit work. If you need to amend them, do it on a separate commit with a separate message.
+
+---
+
+## What "verification" actually looks like
+
+This work has *no automated test suite in CI* (per the audit, only `npm run build` enforces typecheck). The verification gate is **screenshot + manual smoke** on the deployment host.
+
+For each sprint, a `sprintN-verification.md` lists:
+- A **deploy command** block (the user runs on the deployment host).
+- A **smoke-check** list of manual interactions to perform in the browser.
+- A list of "Things to look for" — the observable behavior change for each finding.
+
+The user's flow is: read the smoke check, run the deploy, click around, mark each item ✅ or report an issue. The verification log is a record, not a script.
+
+**If you need to re-verify locally**, the original screenshots are at `/tmp/opencode/mp-review/screenshots/` (20 PNGs, 1366 px and 390 px viewports). Post-fix screenshots are in `/tmp/opencode/mp-review/screenshots/fix-sprint1/`. The playwright harness script is at `/tmp/opencode/mp-review/shoot-sprint1.js`. Useful templates for re-screenshotting Sprints 2 and 3 are in there — adapt the URL list.
+
+---
+
+## Active risks & open questions
+
+1. **S2.3 / B8 backend migration NOT YET RUN on deployment host.** The user must run `alembic upgrade head` after `git pull`. Dry-run shows 21k rows change on the dev DB; the prod DB may differ. The persistent backup (`persist_aisle_backup.sql`) creates two permanent tables the operator can `DROP` after confidence is established.
+
+2. **S3.1 / B12 Dashboard Undo is "rebuild, not restore".** `meals.generateItem` re-fills the slot with a *new* recipe; the original recipe is gone. The plan documents this trade-off (§R4). If the user pushes back, the alternative is a backend endpoint to restore from a snapshot (out of scope, would need a separate task).
+
+3. **`frontend/src/lib/toast.tsx` is force-added because of the `lib/` gitignore bug.** Future agents should be aware: any new file in `frontend/src/lib/` needs `git add -f`. Consider fixing the `.gitignore` (use `/lib/` or specific Python paths instead of `lib/`) as a small follow-up.
+
+4. **S2.3 may need a follow-up constraint** — the migration docstring says no `CHECK (aisle IN (...))` constraint is added. The next migration (`0016`) could add it, but that's an optional tightening. The frontend already constrains the form to canonical values, so DB constraint is belt-and-suspenders.
+
+5. **`scripts/dry_run_aisle_migration.sql` is read-only but reads from prod.** If the user runs it via `docker exec ... psql -f /dev/stdin < script.sql`, no writes happen. But they should know it's a *query*, not a script. The comment at the top of the file says so.
+
+---
+
+## What's NOT in scope (per audit §Future)
+
+The audit intentionally deferred these. If the user asks, they become a new plan:
+- F1. Onboarding hints / tour (H10)
+- F2. Keyboard shortcuts (`/`, `g p`, `g s`, `n m`)
+- F3. Bulk add on Pantry/Shopping List (H7)
+- F4. Plan-the-whole-week button (H7)
+- F5. Persistent week selector in URL
+- F6. Badge a11y for color-only signals (generalized — the current badges are text+color)
+- F7. Global `react-query` `onError` toast handler
+- F8. Backend Spoonacular enrichment (separate proposal in `docs/proposals/`)
+- F9. Ollama LLM matcher (separate proposal in `docs/proposals/`)
+
+---
+
+## File-level diff summary (audit work only)
+
+| Sprint | File | What |
+|---|---|---|
+| 1 | `frontend/src/pages/RecipeDetail.tsx` | B1: drop `.trim()` |
+| 1 | `frontend/src/pages/MealDetail.tsx` | B2: `qty` + name fallback; B3: cost conditional |
+| 1 | `frontend/src/App.tsx` | B4: `/recommended` alias + `*` NotFound |
+| 1 | `frontend/src/pages/Dashboard.tsx` | B5: remove `hidden md:*` on empty slots |
+| 1 | `frontend/src/pages/NotFound.tsx` | **NEW** — uses EmptyState |
+| 1 | `frontend/src/components/ui/EmptyState.tsx` | optional `to` prop |
+| 1 | `frontend/src/types/index.ts` | `RecipeIngredient.notes` + `ingredient?` |
+| 2 | `frontend/src/pages/Dashboard.tsx` | B6: line-clamp-2 + smaller image |
+| 2 | `frontend/src/pages/MealDetail.tsx` | B7: hero rework + cleanDescription + disclosure |
+| 2 | `frontend/src/lib/utils.ts` | B7: cleanDescription() |
+| 2 | `frontend/src/pages/Pantry.tsx` | B8: select for aisle/unit + required marker; B10: scroll hint |
+| 2 | `frontend/src/types/index.ts` | B8: PANTRY_AISLES + PantryAisle type |
+| 2 | `backend/alembic/versions/0015_normalize_pantry_aisles.py` | **NEW** — migration |
+| 2 | `backend/scripts/dry_run_aisle_migration.sql` | **NEW** — read-only preview |
+| 2 | `backend/scripts/persist_aisle_backup.sql` | **NEW** — persistent backup |
+| 2 | `frontend/src/pages/ShoppingList.tsx` | B9 + S3.3: aisle label + 3-col grid |
+| 2 | `frontend/src/pages/Recipes.tsx` | B11: applied/pending filter + Apply/Reset + active count |
+| 3 | `frontend/src/lib/toast.ts` → `.tsx` | renamed + undo helper |
+| 3 | `frontend/src/pages/Dashboard.tsx` | B12: undo on delete; S3.5: aria-label on status badge |
+| 3 | `frontend/src/pages/Pantry.tsx` | B12: reversible undo on remove |
+| 3 | `frontend/src/App.tsx` | B13 + S3.5: nav nowrap, aria-current, main id |
+| 3 | `frontend/src/components/ui/Badge.tsx` | S3.5: icon + aria-label props |
+
+---
+
+## Quick-start for the next agent
+
+If you are continuing this work:
+
+```bash
+# 1. Get oriented
+cd /home/peter/Projects/MealPlanner
+cat docs/ORIENTATION.md
+cat docs/HANDOFF.md
+cat Review/ui-nielsen-audit.md
+cat fix-ui-audit.md
+
+# 2. See what's still in flight
+git log --oneline -10
+git status
+git log origin/main..HEAD --oneline
+
+# 3. If the user wants to deploy pending work:
+#    - Sprint 2: docker compose exec db psql -U mealplanner -d mealplanner \
+#        -f /dev/stdin < backend/scripts/persist_aisle_backup.sql
+#    - docker compose exec backend alembic upgrade head
+#    - docker compose -f docker-compose.yml up -d --build frontend
+#    - Sprint 3: docker compose -f docker-compose.yml up -d --build frontend
+#    See Review/sprint{2,3}-verification.md for the full checklist.
+
+# 4. If you need to take new screenshots after changes
+#    /tmp/opencode/mp-review/shoot-sprint1.js is a working playwright harness.
+#    Adapt the URL list. Output: /tmp/opencode/mp-review/screenshots/.
+
+# 5. Build check before any commit
+cd frontend && npm run build
+```
+
+**When you commit**, follow the existing style:
+- `fix(ui): …` for bug fixes
+- `feat(ui): …` for new behavior
+- `refactor(frontend): …` for restructuring without behavior change
+- `docs(review): …` for doc-only commits
+- Conventional commits, imperative mood, body explaining the why
+
+**When you push**, only push UI-audit commits. Pre-existing WIP is the user's to manage.
+
+---
+
+## Final words
+
+Trust the build output. Trust the smoke checklist. Don't trust the deployment host's UI until the user confirms. The verification model is "I shipped, you verified, you reported, I fixed" — the agent in this role never sees the live UI directly.
+
+**Last updated: 2026-06-03** — Sprints 1, 2, 3 all committed; Sprint 1 deployed; Sprints 2 and 3 awaiting deploy.