Meal-Planner/Review/handoff-ui-audit.md

# UI/UX Audit & Fix — Agent Handoff

You are taking over a 4-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

Four commits land all 14 audit findings + 2 of the §Future items:

| 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 |
| 4 | `d71b67a` | F7 global error handler + F6 plan-status aria-label | ✅ green | ⚠️ not yet deployed |

All work is on `main` ahead of `origin/main` (pre-existing WIP also present). All four sprints compile. **Sprint 1 is live. Sprints 2, 3, and 4 are not yet live on `100.108.208.56:8082/`.** Sprint 4 is frontend-only (no backend changes); Sprint 2 includes a backend migration; Sprints 3 and 4 are pure frontend.

**Next action:** the user runs the deploy commands in `Review/sprint2-verification.md`, `Review/sprint3-verification.md`, and `Review/sprint4-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` (F1 onboarding, F2 keyboard shortcuts, F3 bulk add, F4 plan-whole-week, F5 URL week selector, F8 Spoonacular enrichment, F9 Ollama matcher — F6 and F7 now done in Sprint 4).

---

## 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).
│   ├── sprint4-verification.md      # Deploy + smoke checks for Sprint 4 (F7+F6, 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. Implementation notes per task + commit refs. Sprint 4 added.
├── 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 three `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)

F6 and F7 are done in Sprint 4. 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)~~ — done in Sprint 4
- ~~F7. Global `react-query` `onError` toast handler~~ — done in Sprint 4
- 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 |
| 4 | `frontend/src/lib/toast.tsx` | F7: extractErrorMessage + showApiError |
| 4 | `frontend/src/App.tsx` | F7: QueryCache/MutationCache onError + queries defaultOptions |
| 4 | `frontend/src/pages/Dashboard.tsx` | F7: removed 6 local onError; F6: plan-status aria-label |
| 4 | `frontend/src/pages/Pantry.tsx` | F7: removed 3 local onError; undo now uses showApiError |
| 4 | `frontend/src/pages/MealDetail.tsx` | F7: removed submitMutation.onError |

---

## 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
#    - Sprint 3: docker compose -f docker-compose.yml up -d --build frontend
#    - Sprint 4: docker compose -f docker-compose.yml up -d --build frontend
#    See Review/sprint{2,3,4}-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.