From 427d8ac35288f62715929807f861ebf0ff676ee3 Mon Sep 17 00:00:00 2001 From: Peter Woolery Date: Wed, 3 Jun 2026 18:58:52 -0700 Subject: [PATCH] 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. --- Review/handoff-ui-audit.md | 230 +++++++++++++++++++++++++++++++++++++ 1 file changed, 230 insertions(+) create mode 100644 Review/handoff-ui-audit.md diff --git a/Review/handoff-ui-audit.md b/Review/handoff-ui-audit.md new file mode 100644 index 0000000..07ca78a --- /dev/null +++ b/Review/handoff-ui-audit.md @@ -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 ``, `aria-current="page"`, `
`, `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.