docs(phase-04): verification — 3/4 verified, LIST-03 rank-collation gap deferred to gap-closure

2026-06-09 13:49:58 -04:00
parent 690f0b95c0
commit be2078e21f
1 changed files with 163 additions and 0 deletions
@@ -0,0 +1,163 @@
+---
+phase: 04-shared-lists-live-sync
+verified: 2026-06-09T14:00:00Z
+status: gaps_found
+score: 3/4 must-haves verified
+overrides_applied: 0
+gaps:
+  - truth: "A member can drag an active item to a new position and the order persists (reorder via drag-to-top)"
+    status: partial
+    reason: "rank column uses utf8mb4_uca1400_ai_ci (case-insensitive) collation. fractional-indexing generates uppercase-prefixed keys (e.g. 'Zz') when inserting before the first active item (drag-to-top). In the DB, 'Zz' < 'a0' evaluates FALSE (measured: db_less=0), but in JS it is TRUE. After a drag-to-top the PATCH writes a rank the DB ORDER BY rank sorts to the bottom — the dragged item lands at the bottom on next fetch, contradicting the user's action. Tests at line 931 of lists.test.ts explicitly work around this by restricting to lowercase ranks (a0-a5) and noting 'avoids collation issues with uppercase ranks'. No regression test for an uppercase-prefixed rank exists."
+    artifacts:
+      - path: "apps/api/src/db/schema.ts"
+        issue: "listItems.rank column defined as varchar(255) with no explicit collation — inherits table/DB default utf8mb4_uca1400_ai_ci instead of utf8mb4_bin"
+      - path: "apps/api/src/db/migrations/0001_lists_schema.sql"
+        issue: "rank column DDL does not specify COLLATE utf8mb4_bin — column gets case-insensitive collation from DB default"
+      - path: "apps/api/tests/routes/lists.test.ts"
+        issue: "reorder test at line 931 explicitly avoids uppercase ranks to sidestep the bug; no regression test that inserts an uppercase-prefixed rank and asserts ORDER BY returns JS string order"
+    missing:
+      - "Migrate rank column to COLLATE utf8mb4_bin via drizzle-kit generate+migrate (NEVER db:push on populated MariaDB)"
+      - "Add a regression test that seeds an uppercase-prefixed rank (e.g. 'Zz') as first item, PATCHes it to top, and asserts GET returns items in JS string order"
+---
+
+# Phase 4: Shared Lists + Live Sync Verification Report
+
+**Phase Goal:** Both members can create and manage shared named lists with real-time co-edit sync — edits by one member appear for the other without any manual refresh
+**Verified:** 2026-06-09T14:00:00Z
+**Status:** gaps_found
+**Re-verification:** No — initial verification
+
+---
+
+## Goal Achievement
+
+### Observable Truths (Roadmap Success Criteria)
+
+| # | Truth | Status | Evidence |
+|---|-------|--------|----------|
+| 1 | Either member can create a named list and delete a list they no longer need | VERIFIED | `POST /api/lists` with auto-share wired in `lists.ts:249`; `DELETE /api/lists/:id` owner-only at `lists.ts:403`; `CreateListSheet.tsx` (336 lines, real form); `ListDeleteDialog.tsx` (196 lines); 181 API tests pass including scoped-access and cascade-delete tests |
+| 2 | Either member can add items to a list, check items off, reorder them by drag-and-drop, and delete individual items | VERIFIED | `POST /:id/items` + `PATCH /list-items/:itemId` (exact-one-field LWW) + `DELETE /list-items/:itemId`; `ListDetail.tsx` (501 lines) with `DndContext`/`SortableContext`; `ItemRow.tsx` (273 lines) with `useSortable`; fractional rank assigned on create; optimistic mutations wired for all four operations; 181 API tests pass |
+| 3 | When one member adds or checks off an item, the other member sees the change appear without refreshing — even after a brief network gap | VERIFIED | `publishListEvent` called after every write in `lists.ts`; scoped `GET /api/sse/lists` in `sse.ts:85` subscribes per accessible list via `getAccessibleListIds`; `useListSSE.ts` (114 lines) implements bounded-backoff EventSource (D-11); `refetchInterval:30000` polling fallback active (D-12); D-04 no-leak invariant tested in `lists.test.ts:856` |
+
+**Score (roadmap truths):** 3/3 met, but Truth 2 has a partial defect at drag-to-top ordering — see gap below.
+
+### Partial Defect: LIST-03 Drag-to-Top (Rank Collation)
+
+LIST-03 reorder works correctly for moves within the lowercase rank space (a0–a5 range) but fails for drag-to-top because `fractional-indexing` generates an uppercase-prefixed key (e.g. `Zz`) when prepending before the first item.
+
+**Measured divergence (live DB query):**
+```
+SELECT ('Zz' < 'a0') AS db_less, ('Zz' < 'a0' COLLATE utf8mb4_bin) AS bin_less
+→ db_less: 0   bin_less: 1
+```
+
+The DB returns `FALSE` for `'Zz' < 'a0'` under `utf8mb4_uca1400_ai_ci` (case-insensitive), but JS string comparison returns `true`. So:
+- User drags item to the top.
+- Client computes `generateKeyBetween(null, firstActiveRank)` → `'Zz'` (uppercase prefix).
+- PATCH writes `rank = 'Zz'` to DB.
+- On next refetch, DB `ORDER BY rank` sorts `'Zz'` AFTER all `'a...'` ranks → the dragged item surfaces at the bottom.
+
+The test suite was written to avoid this: `lists.test.ts:931` comment reads _"This stays within the normal rank space and avoids collation issues with uppercase ranks."_ No test covers the upstream failure path.
+
+**Requirement mapping:** LIST-03 is PARTIAL (moves within lowercase range work; drag-to-top breaks on refetch).
+
+---
+
+### Required Artifacts
+
+| Artifact | Expected | Status | Details |
+|----------|----------|--------|---------|
+| `apps/api/src/db/schema.ts` | lists, listShares, listItems Drizzle tables | VERIFIED | All three tables present at lines 174–241; correct FK refs, indexes, D-01/D-02/D-13 column shapes |
+| `apps/api/src/db/migrations/0001_lists_schema.sql` | Additive CREATE TABLE migration | VERIFIED | File exists; CREATE TABLE for lists/list_items/list_shares; no DROP/TRUNCATE of existing tables; FKs correct |
+| `apps/pwa/src/components/BottomTabBar.tsx` | Calendar/Lists bottom tab navigation | VERIFIED | 88 lines; NavLink to `/calendar` and `/lists`; 44px+ touch targets; active-state accent via isActive callback |
+| `apps/pwa/src/routes/ListsIndex.tsx` | Lists surface with empty state | VERIFIED | Full implementation; useQuery(['lists']); ListsEmptyState; ListCard; CreateListSheet; ListDeleteDialog wired |
+| `apps/api/src/routes/lists.ts` | POST/GET/PATCH/DELETE /api/lists with scoped access | VERIFIED | 693 lines; all four verbs; scoped GET (owner + shares); auto-share on create; publishListEvent fan-out on every write |
+| `apps/pwa/src/components/CreateListSheet.tsx` | New-list form with shared/private toggle | VERIFIED | 336 lines; default shared=true; form validation; useMutation wired |
+| `apps/pwa/src/components/ListCard.tsx` | List summary card navigating to /lists/:id | VERIFIED | 176 lines; item counts; navigate to /lists/:id |
+| `apps/pwa/src/components/ListDeleteDialog.tsx` | List-delete confirmation (D-06) | VERIFIED | 196 lines; reuses dialog pattern; owner-only delete path |
+| `apps/api/src/lib/rank.ts` | fractional rank helpers (rankForAppend, rankBetween) | VERIFIED | 43 lines; wraps `generateKeyBetween`; pure functions; no DB access |
+| `apps/pwa/src/routes/ListDetail.tsx` | List detail with active/completed split + add/check/delete | VERIFIED | 501 lines; DndContext/SortableContext; D-05 active/completed split; all four mutations; D-09 delete-wins |
+| `apps/pwa/src/components/ItemRow.tsx` | dnd-kit sortable item with drag handle | VERIFIED | 273 lines; `useSortable`; handle-scoped listeners; CSS.Transform animation (D-14) |
+| `apps/pwa/src/components/AddItemInput.tsx` | Sticky add-item input | VERIFIED | 105 lines; onAdd callback; isPending state |
+| `apps/api/src/routes/sse.ts` | GET /api/sse/lists scoped SSE stream | VERIFIED | 122 lines; `/lists` route present; `subscribeListEvents` + `getAccessibleListIds`; 30s heartbeat |
+| `apps/pwa/src/hooks/useListSSE.ts` | Bounded-backoff EventSource wrapper invalidating React Query | VERIFIED | 114 lines; 6-step backoff (250ms→8s); `onStateChange` to 'connected'/'reconnecting'/'disconnected'; D-10 full refetch on open |
+| `apps/pwa/src/components/LiveSyncIndicator.tsx` | Connected/reconnecting/disconnected indicator | VERIFIED | 119 lines; three distinct render branches; role="status"/"alert" |
+| `apps/api/src/lib/listEmitter.ts` | In-memory scoped event emitter | VERIFIED | 55 lines; module-level singleton; per-list channels `list:${listId}`; publish/subscribe/unsubscribe |
+| `apps/api/src/lib/listAccess.ts` | getAccessibleListIds access-scope query | VERIFIED | 43 lines; owned UNION shared; deduplicated; used by SSE endpoint |
+
+### Key Link Verification
+
+| From | To | Via | Status | Details |
+|------|----|-----|--------|---------|
+| `apps/pwa/src/App.tsx` | `/lists` route | `BrowserRouter` + `BottomTabBar` NavLink | WIRED | `App.tsx:31`; `<Route path="/lists" element={<ListsIndex />} />`; `<Route path="/lists/:listId" element={<ListDetail />} />` |
+| `apps/api/src/index.ts` | `listsRouter` + `listItemsRouter` | `app.route('/api/lists', listsRouter)` + `app.route('/api/list-items', listItemsRouter)` | WIRED | `index.ts:65-66`; both routers mounted |
+| `apps/api/src/routes/lists.ts` | `list_shares` | Auto-insert shares on create + scoped GET | WIRED | `lists.ts:264-277` (auto-share POST); `lists.ts:169-183` (scoped GET via owned+shared IDs) |
+| `apps/pwa/src/routes/ListsIndex.tsx` | `/api/lists` | `useQuery + useMutation` in `listsClient` | WIRED | `ListsIndex.tsx:43` (`useQuery(['lists'], fetchLists)`); `useMutation(deleteList)` wired |
+| `apps/pwa/src/routes/ListDetail.tsx` | `/api/lists/:id/items` + `/api/list-items/:id` | `useQuery + optimistic mutations` | WIRED | `ListDetail.tsx:138-186` (fetch + mutations using `fetchListItems`/`addItem`/`patchListItem`/`deleteItem`) |
+| `apps/api/src/routes/lists.ts` | `publishListEvent` | Fan-out trigger after every successful write | WIRED | Calls present after POST list (`lists.ts:287`), PATCH list (`lists.ts:379`), DELETE list (`lists.ts:425`), POST item (`lists.ts:491`), PATCH item (`lists.ts:634`), DELETE item (`lists.ts:685`) |
+| `apps/api/src/routes/sse.ts` | `subscribeListEvents + getAccessibleListIds` | Scoped per-list subscription inside `streamSSE` | WIRED | `sse.ts:89` (`getAccessibleListIds`); `sse.ts:96` (`subscribeListEvents` per listId in loop) |
+| `apps/pwa/src/hooks/useListSSE.ts` | `/api/sse/lists` | `new EventSource(withCredentials) → invalidateQueries` | WIRED | `useListSSE.ts:65` (`new EventSource('/api/sse/lists', { withCredentials: true })`); event listeners call `handleListChange` which invalidates `['list', listId]` |
+| `apps/pwa/src/routes/ListDetail.tsx` | `useListSSE` | Mounted in `ListDetail` render; `setSyncState` passed to hook | WIRED | `ListDetail.tsx:116` (`useListSSE({ listId: parsedListId, onStateChange: setSyncState })`); `<LiveSyncIndicator state={syncState} />` at line 385 |
+
+### Data-Flow Trace (Level 4)
+
+| Artifact | Data Variable | Source | Produces Real Data | Status |
+|----------|---------------|--------|--------------------|--------|
+| `ListsIndex.tsx` | `data?.lists` | `useQuery(['lists'], fetchLists)` → `GET /api/lists` → DB query (`lists` + `listShares` + `listItems` COUNT) | Yes — DB query with scoped WHERE clause | FLOWING |
+| `ListDetail.tsx` | `data?.items` | `useQuery(['list', listId], fetchListItems)` → `GET /api/lists/:id/items` → DB `SELECT ... ORDER BY rank ASC` | Yes — DB query returning real items | FLOWING |
+| `sse.ts /lists` | SSE events | `subscribeListEvents` ← `publishListEvent` triggered by route writes → real DB mutations | Yes — events fire only after confirmed DB writes | FLOWING |
+
+### Behavioral Spot-Checks
+
+| Behavior | Command | Result | Status |
+|----------|---------|--------|--------|
+| API test suite (181 tests) | `pnpm --filter @familysync/api exec vitest run` | 181 passed, 0 failed (17 test files) | PASS |
+| PWA test suite (160 tests) | `pnpm --filter @familysync/pwa test` | 160 passed, 0 failed (14 test files) | PASS |
+| Production build | `pnpm build` | Built in 413ms; SW generated; no TypeScript errors | PASS |
+| Rank column collation in DB | `SELECT ('Zz' < 'a0') AS db_less` | `db_less: 0` (FALSE) vs JS `'Zz' < 'a0'` = true | FAIL — confirms rank gap |
+
+### Probe Execution
+
+No `scripts/*/tests/probe-*.sh` probes declared or found for this phase.
+
+### Requirements Coverage
+
+| Requirement | Source Plan | Description | Status | Evidence |
+|-------------|------------|-------------|--------|----------|
+| LIST-01 | 04-03-PLAN.md | User can create and delete named lists | SATISFIED | `POST /DELETE /api/lists`; auto-share; cascade delete; API tests pass |
+| LIST-02 | 04-04-PLAN.md | User can add items, check off, delete | SATISFIED | `POST/PATCH/DELETE /api/list-items`; D-05 checked-sink; D-08 single-field PATCH; API tests pass |
+| LIST-03 | 04-05-PLAN.md | User can reorder items within a list | PARTIAL | dnd-kit + fractional rank wired and works for lowercase rank space; drag-to-top broken due to rank column collation (utf8mb4_uca1400_ai_ci vs required utf8mb4_bin) |
+| LIST-04 | 04-06-PLAN.md | Both members' list edits appear live without manual refresh | SATISFIED | scoped SSE endpoint; publishListEvent on every write; useListSSE bounded-backoff hook; D-04 no-leak tested; polling fallback active |
+
+### Anti-Patterns Found
+
+| File | Line | Pattern | Severity | Impact |
+|------|------|---------|----------|--------|
+| `apps/pwa/src/routes/ListsIndex.tsx` | 66 | `TODO: surface "Couldn't delete. Try again." toast (Plan 06 / notification layer)` | Info | Error feedback on delete failure absent; rollback still happens (cache restored); functional correctness unaffected |
+| `apps/pwa/src/routes/ListDetail.tsx` | 379-381 | List detail header shows literal "List" instead of the list name | Info | UX limitation (no extra fetch for name in detail view); noted as future improvement; all item operations work correctly |
+| `apps/api/src/db/schema.ts` | 231 | `rank: varchar(255)` with no COLLATE override — inherits utf8mb4_uca1400_ai_ci | BLOCKER | Drag-to-top produces uppercase-prefixed rank that DB sorts incorrectly vs JS; see gap |
+
+**Debt markers:** Zero `TBD`, `FIXME`, or `XXX` markers found in any Phase 4 source file.
+
+### Human Verification Required
+
+None — all functional behaviors are verifiable via automated tests or direct code inspection. The drag-to-top ordering failure is confirmed by live DB query rather than requiring visual testing.
+
+---
+
+### Gaps Summary
+
+**One open gap** blocks full LIST-03 goal achievement:
+
+**Root cause:** The `list_items.rank` column was created without an explicit collation and inherited the database/table default (`utf8mb4_uca1400_ai_ci`), which is case-insensitive. The `fractional-indexing` library uses a base-62 character set that generates uppercase-prefixed keys (e.g. `Zz`) when inserting before the first item. Under case-insensitive collation, `'Zz' < 'a0'` is FALSE in MariaDB (uppercase letters sort after lowercase), so `ORDER BY rank ASC` returns rows in the wrong order after a drag-to-top.
+
+**Impact:** Drag-to-top is silently broken for the end user — the item appears to move but snaps back to the bottom on the next refetch. Moves within the lowercase range (e.g. `a0`→`a5`) are unaffected because both JS and MariaDB order them correctly.
+
+**Required fix:** Migrate the `rank` column to `COLLATE utf8mb4_bin` via `drizzle-kit generate+migrate`. Never use `db:push` on this populated DB. Add a regression test that seeds an uppercase-prefixed rank, reads items back, and asserts the returned order matches JS string order.
+
+**All other LIST-01, LIST-02, and LIST-04 behaviors are fully implemented and tested.** 181 API tests and 160 PWA tests pass. The production build is clean.
+
+---
+
+_Verified: 2026-06-09T14:00:00Z_
+_Verifier: Claude (gsd-verifier)_