diff --git a/Review/handoff-ui-audit.md b/Review/handoff-ui-audit.md index 490e0d2..18323e6 100644 --- a/Review/handoff-ui-audit.md +++ b/Review/handoff-ui-audit.md @@ -1,25 +1,29 @@ # 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. +You are taking over a 5-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.** +**Date of handoff: 2026-06-04.** --- ## TL;DR -Four commits land all 14 audit findings + 2 of the §Future items: +Nine commits land all 14 audit findings + 4 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 | +| 2 | `ccc70aa` + `f5fb755` | 6 P1s + bonus S3.3 | ✅ green | ⚠️ blocked — see S2 + 0015 fix below | +| 3 | `e90a9d6` | 3 P2s + a11y sweep | ✅ green | ⚠️ blocked — depends on S2 deploy | +| 4 | `d71b67a` | F7 global error handler + F6 plan-status aria-label | ✅ green | ⚠️ not yet deployed (frontend-only) | +| 5 | `d78bd18` | F5 URL week selector + **CRITICAL 0015 cast fix** | ✅ green | ⚠️ not yet deployed | +| 5 | `f740f40` | F2 keyboard shortcuts + ShortcutHelpBanner | ✅ 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. +All work is on `main` ahead of `origin/main` (pre-existing WIP also present). All five sprints compile. **Sprint 1 is live. Sprints 2, 3, 4, 5 are not yet live on `100.108.208.56:8082/`.** -**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). +**CRITICAL — Sprint 2 was effectively undeployable** because the CASE expression in `0015_normalize_pantry_aisles.py` failed with `text = boolean` on the `varchar(100) aisle` column. The bug is fixed in `d78bd18` (Sprint 5). Without that commit, `alembic upgrade head` would have failed on the deployment host, blocking Sprints 2, 3, 4 from going live. **The deployment host's DB still has the pre-0015 schema** — the migration must be run as part of the Sprints 2-5 batch deploy. + +**Next action:** the user runs the deploy commands in `Review/sprint2-verification.md`, `Review/sprint3-verification.md`, `Review/sprint4-verification.md`, and `Review/sprint5-verification.md` on the deployment host, then smoke-checks per the checklists. The Sprints 2-5 deploy is a single batch (one `git pull`, one migration, one rebuild). After verification, any remaining items move to the §Future backlog in `fix-ui-audit.md` (F1 onboarding, F3 bulk add, F4 plan-whole-week, F8 Spoonacular enrichment, F9 Ollama matcher). --- @@ -32,17 +36,18 @@ All work is on `main` ahead of `origin/main` (pre-existing WIP also present). Al │ ├── 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). +│ ├── sprint5-verification.md # Deploy + smoke checks for Sprint 5 (F5+F2+0015 fix; backend+frontend). │ ├── 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. +├── fix-ui-audit.md # The plan. Implementation notes per task + commit refs. Sprints 4-5 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. +**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 four `sprintN-verification.md` files. --- @@ -139,12 +144,12 @@ The user's flow is: read the smoke check, run the deploy, click around, mark eac ## 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: +F2, F5, F6, and F7 are done in Sprints 4-5. 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`) +- ~~F2. Keyboard shortcuts (`/`, `g p`, `g s`, `n m`)~~ — done in Sprint 5 - F3. Bulk add on Pantry/Shopping List (H7) - F4. Plan-the-whole-week button (H7) -- F5. Persistent week selector in URL +- ~~F5. Persistent week selector in URL~~ — done in Sprint 5 - ~~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/`) @@ -183,6 +188,19 @@ F6 and F7 are done in Sprint 4. The audit intentionally deferred these. If the u | 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 | +| 5 | `backend/app/api/meals.py` | F5: added ?week_start= query param | +| 5 | `backend/app/api/shopping_list.py` | F5: added ?week_start= query param | +| 5 | `backend/alembic/versions/0015_normalize_pantry_aisles.py` | **CRITICAL cast fix** — was blocking Sprint 2 deploy | +| 5 | `frontend/src/lib/utils.ts` | F5: isoMonday/parseIsoDate/shiftIsoDate/formatIsoDate helpers | +| 5 | `frontend/src/api/index.ts` | F5: getPlanned/get take weekStart? | +| 5 | `frontend/src/pages/Dashboard.tsx` | F5: useSearchParams sync, week nav, queryKey includes weekStart | +| 5 | `frontend/src/pages/ShoppingList.tsx` | F5: same URL sync as Dashboard | +| 5 | `frontend/src/hooks/useKeyboardShortcuts.ts` | **NEW** — F2 global handler with vim-style sequences | +| 5 | `frontend/src/hooks/useFocusSearch.ts` | **NEW** — F2 focus-search CustomEvent bus | +| 5 | `frontend/src/components/ShortcutHelpBanner.tsx` | **NEW** — F2 help dialog | +| 5 | `frontend/src/App.tsx` | F2: mount GlobalShortcuts + ShortcutHelpBanner | +| 5 | `frontend/src/pages/Pantry.tsx` | F2: useFocusSearchOnShortcut on search input | +| 5 | `frontend/src/pages/Recipes.tsx` | F2: useFocusSearchOnShortcut on search input | --- @@ -203,13 +221,12 @@ git log --oneline -10 git status git log origin/main..HEAD --oneline -# 3. If the user wants to deploy pending work: +# 3. If the user wants to deploy pending work (Sprints 2-5 as a single batch): # - 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. +# - Sprints 3-5: docker compose -f docker-compose.yml up -d --build backend frontend +# See Review/sprint{2,3,4,5}-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. diff --git a/Review/sprint5-verification.md b/Review/sprint5-verification.md new file mode 100644 index 0000000..34d9c7b --- /dev/null +++ b/Review/sprint5-verification.md @@ -0,0 +1,184 @@ +# Sprint 5 — Deploy & smoke-check (F5 + F2) + +**Goal:** verify the URL week selector (F5) and keyboard shortcuts (F2) work end-to-end on `http://100.108.208.56:8082/`. + +**Commits:** +- `d78bd18` — F5 URL week selector + 0015 migration cast fix +- `f740f40` — F2 keyboard shortcuts + +**Sprint scope:** F5 (URL week selector) + F2 (keyboard shortcuts) + 0015 cast fix +**Backend changes:** `meals.py` (week_start param), `shopping_list.py` (week_start param), `0015_normalize_pantry_aisles.py` (cast fix) +**Frontend changes:** `App.tsx`, `lib/utils.ts`, `api/index.ts`, `pages/Dashboard.tsx`, `pages/ShoppingList.tsx`, `pages/Pantry.tsx`, `pages/Recipes.tsx`; new `hooks/useKeyboardShortcuts.ts`, `hooks/useFocusSearch.ts`, `components/ShortcutHelpBanner.tsx` + +## 1. Deploy + +This sprint includes a **backend migration** and a **frontend bundle** change. + +```bash +# On deployment host (100.108.224.12) +cd /path/to/MealPlanner +git pull + +# 1. Persistent backup of aisle values BEFORE the migration (recommended) +docker compose exec -T db psql -U mealplanner -d mealplanner \ + -f /dev/stdin < backend/scripts/persist_aisle_backup.sql + +# 2. Apply the migration (now includes the cast fix from Sprint 5) +docker compose exec backend alembic upgrade head +# Expected: "Running upgrade 0014 -> 0015, Normalize ingredient.aisle and grocery_item.aisle..." + +# 3. Rebuild & restart both backend and frontend +docker compose -f docker-compose.yml up -d --build backend frontend +``` + +**CRITICAL:** the migration fix in `d78bd18` is what makes the Sprint 2 migration actually runnable. The deployment host will hit the same `text = boolean` error without it. + +## 2. Build verification (already green locally) + +``` +$ cd frontend && npm run build +> tsc && vite build +✓ 1894 modules transformed. +dist/index.html 0.54 kB │ gzip: 0.32 kB +dist/assets/index-BeQbgL42.css 37.83 kB │ gzip: 6.79 kB +dist/assets/index-6M0ldwxq.js 474.37 kB │ gzip: 146.54 kB +✓ built in 6.71s +``` + +tsc 0 errors, vite 0 errors. + +## 3. Smoke checks + +### S5.A — F5 backend: `?week_start=` is honoured + +```bash +# 1. Without param — should return the latest plan +curl -s "http://100.108.208.56:8082/api/meals" | python3 -c "import sys,json; d=json.load(sys.stdin); print('week_start_date:', d.get('week_start_date') if d else 'NULL')" + +# 2. With a known plan's week — should return that plan +curl -s "http://100.108.208.56:8082/api/meals?week_start=2026-05-15" | python3 -c "import sys,json; d=json.load(sys.stdin); print('week_start_date:', d.get('week_start_date') if d else 'NULL', 'items:', len(d.get('items',[])) if d else 0)" + +# 3. With a week that has no plan — should return null +curl -s "http://100.108.208.56:8082/api/meals?week_start=2099-01-01" +# Expected: null (or a JSON "null") + +# 4. Same for shopping-list +curl -s "http://100.108.208.56:8082/api/shopping-list?week_start=2026-05-15" | python3 -c "import sys,json; d=json.load(sys.stdin); print('week_start_date:', d.get('week_start_date'), 'items:', len(d.get('items',[])))" +``` + +The local dev DB returns `week_start_date: 2026-05-15 items: 4` for the known plan and `null` for 2099-01-01. The deployment host should match (modulo different seed data). + +### S5.B — F5 frontend: URL week navigation on Dashboard + +1. Open `http://100.108.208.56:8082/`. +2. Confirm the header shows the current week ("This week" button is highlighted, primary-50 background). +3. Click the **left chevron** — title should change to the previous week. URL should update to `?week=YYYY-MM-DD` (Monday's date). +4. Click the left chevron again — should be 2 weeks back. URL still has `?week=...`. +5. Click the **"This week" / "Current"** button — URL should drop `?week=...`, title should be the current week. +6. Click the **right chevron** — title should be next week. (If no plan for that week, you should see the "No meal plan yet" empty state.) +7. Manually type `http://100.108.208.56:8082/?week=2026-05-15` in the URL bar. Page should load with that week's plan. + +### S5.C — F5 frontend: URL week navigation on Shopping List + +Same as S5.B but on `/shopping-list`. The empty state should say "No plan for that week" (not "No shopping list yet") when `?week=` is set to a week that has no plan. + +### S5.D — F5 frontend: query-key isolation + +1. Open `http://100.108.208.56:8082/?week=2026-05-15`. +2. Open DevTools → Network tab. +3. Drag a meal to a different slot. The Network tab should show: + - `PUT /api/meals/items/{id}/move?new_day_of_week=...&new_meal_type=...` + - `GET /api/meals?week_start=2026-05-15` (the refetch, not `/api/meals` without the param) +4. Verify the change persists on refresh. + +### S5.E — F2 keyboard shortcuts + +Open `http://100.108.208.56:8082/` in a fresh tab (so no input is focused). + +| Action | Expected | +|---|---| +| Press `?` | Help banner appears under the nav; auto-dismisses after 6s | +| Press `?` then `Escape` | Banner appears then immediately closes | +| Press `g` then `d` | Navigate to `/` (Dashboard) | +| Press `g` then `r` | Navigate to `/recipes` | +| Press `g` then `p` | Navigate to `/pantry` | +| Press `g` then `s` | Navigate to `/shopping-list` | +| On Pantry, press `/` | Search input gains focus, text is selected | +| On Recipes, press `/` | Search input gains focus, text is selected | +| On Pantry, focus the search, type `g d` | "g d" is typed in the search box; no navigation | +| Press `g` and wait 2 seconds | Nothing happens (sequence timeout) | + +### S5.F — Migration 0015 cast fix verification + +After `alembic upgrade head` returns successfully: + +```bash +# Pick a sample of rows to see they normalized correctly +docker compose exec -T db psql -U mealplanner -d mealplanner -c " + SELECT aisle, COUNT(*) AS n + FROM ingredient + GROUP BY aisle + ORDER BY n DESC + LIMIT 20; +" +# Expected: aisle values are sentence-case canonical labels only: +# 'Pantry', 'Produce', 'Meat & Seafood', 'Dairy & Eggs', 'Frozen', +# 'Bakery', 'Beverages', 'Spices', 'Other', or NULL. +# If you see 'pantry', 'meat_seafood', 'canned_goods' etc., the +# migration did NOT run correctly — re-check the cast fix. +``` + +### S5.G — Regression check: Sprints 1-4 still work + +- [ ] `/recommended` redirects to `/recipes/recommended` (Sprint 1 B4) +- [ ] `/this-does-not-exist` renders the NotFound page (Sprint 1 B4) +- [ ] Recipe detail shows ingredients with proper spacing (Sprint 1 B1) +- [ ] Meal detail shows `$X.XX per serving` (Sprint 1 B3) +- [ ] Pantry aisle `