docs(05-03): complete pushCoalescer TDD plan

2026-06-09 21:00:22 -04:00
parent c1758de05e
commit f07c85d0c9
3 changed files with 121 additions and 7 deletions
@@ -0,0 +1,112 @@
+---
+phase: 05-web-push-notifications
+plan: 03
+subsystem: api/push-coalescer
+tags: [web-push, coalescer, debounce, tdd, red-green, D-01, D-03]
+dependency_graph:
+  requires: [05-01, 05-02]
+  provides: [coalesceListPush — per-(list,actor) sliding debounce]
+  affects: [apps/api/src/lib/pushCoalescer.ts]
+tech_stack:
+  added: []
+  patterns: [module-level Map singleton (listEmitter.ts idiom), sliding debounce setTimeout, injected dispatch for testability]
+key_files:
+  created:
+    - apps/api/src/lib/pushCoalescer.ts
+  modified:
+    - apps/api/tests/lib/pushCoalescer.test.ts
+decisions:
+  - "dispatch signature is (listId, actorId, count) — matches existing RED scaffold; richer payload shape deferred to caller (Plan 05-05)"
+  - "key is ${listId}:${actorId} — per-(list,actor) matches D-01 intent; allows two members editing same list to coalesce independently"
+  - "sliding debounce (each call resets timer) — per plan spec; leading debounce not used"
+  - "dispatch return value is a Promise; errors caught and logged inside fire() so caller loop never breaks"
+metrics:
+  duration: 5
+  completed_date: "2026-06-10"
+  tasks_completed: 2
+  files_changed: 2
+---
+
+# Phase 05 Plan 03: pushCoalescer — per-(list,actor) debounce — Summary
+
+TDD RED→GREEN: `pushCoalescer.ts` implemented; per-(list,actor) sliding debounce collapses list-change bursts into a single dispatch call carrying (listId, actorId, count).
+
+## Tasks Executed
+
+### Task 1: RED — fix lint warning, add actorId assertion
+
+**Status:** Completed. Commit: `7af827a`
+
+The existing RED scaffold in `apps/api/tests/lib/pushCoalescer.test.ts` (from Plan 05-01) had a lint warning: `calledActorId` was destructured in test 1 but never asserted. Added `expect(calledActorId).toBe(actorId)` to make the self-suppression assertion explicit in the burst-coalescing test as well (not only in the dedicated D-03 test).
+
+Tests still fail after this change (RED preserved): `Cannot find module '.../pushCoalescer.js'`.
+
+### Task 2: GREEN — implement pushCoalescer.ts
+
+**Status:** Completed. Commit: `c1758de`
+
+Created `apps/api/src/lib/pushCoalescer.ts`:
+
+**`coalesceListPush(listId, actorId, dispatch, windowMs=45000)`**
+- Module-level `Map<string, {count, timer}>` keyed by `${listId}:${actorId}`
+- First call in a burst: inserts entry with count=1, starts `setTimeout(windowMs)`
+- Subsequent calls within window: `clearTimeout`, increments count, resets timer (sliding debounce)
+- On timer fire: deletes map entry, calls `dispatch(listId, actorId, count)` — self-deleting entries keep the map bounded (T-05-08)
+- dispatch errors caught and logged with `[pushCoalescer]` prefix; never throws to caller
+
+## Verification
+
+```
+pnpm --filter @familysync/api exec vitest run tests/lib/pushCoalescer.test.ts
+
+ Test Files  1 passed (1)
+      Tests  3 passed (3)
+```
+
+`pnpm --filter @familysync/api typecheck` — passes.
+
+## TDD Gate Compliance
+
+- RED: `test(05-03): add actorId assertion in burst test — fix unused var lint warning` — 7af827a
+- GREEN: `feat(05-03): implement pushCoalescer — per-(list,actor) sliding debounce (D-01/D-03)` — c1758de
+
+## Deviations from Plan
+
+### Auto-fixed Issues
+
+**1. [Rule 1 - Bug] Lint warning — unused `calledActorId` in burst test**
+- **Found during:** Task 1 (RED)
+- **Issue:** `calledActorId` was destructured in test 1 but the assertion was missing, producing an unused-variable lint warning.
+- **Fix:** Added `expect(calledActorId).toBe(actorId)` — the burst test now also asserts self-suppression, not just the dedicated D-03 test.
+- **Files modified:** apps/api/tests/lib/pushCoalescer.test.ts
+- **Commit:** 7af827a
+
+### Dispatch signature simplification
+
+The plan's `<behavior>` section describes `dispatch(payload, actorId)` where payload is a rich object `{title, body, tag, navigate}`. The existing RED scaffold (committed in Plan 05-01) uses `dispatch(listId, actorId, count)` — a simpler 3-argument form that defers notification copy construction to the caller.
+
+The test is canonical; the implementation matches the test. The richer payload construction (D-02 generic copy: `"${actorName} updated ${listName}"`, `"${N} change(s)"`) is owned by the caller in Plan 05-05, which has the actorName/listName context from the DB row and passes a closure over `dispatchPush`.
+
+## Known Stubs
+
+None. The coalescer is complete and testable. Plan 05-05 wires it into the list-change fan-out with actual notification copy.
+
+## Threat Flags
+
+No new threat surface. `pushCoalescer.ts` is a pure in-memory utility module — no network endpoints, no auth paths, no file access.
+
+T-05-06 (generic copy — no item text): mitigated by design — the coalescer passes only count, not item text; copy construction in Plan 05-05 will follow D-02.
+T-05-07 (self-notification): mitigated — `actorId` threaded to dispatch so caller can apply `WHERE userId != actorId`.
+T-05-08 (unbounded map): accepted — entries self-delete on timer fire; two-person household keeps keys bounded.
+
+## Self-Check
+
+**Files verified:**
+- [x] apps/api/src/lib/pushCoalescer.ts — exists
+- [x] apps/api/tests/lib/pushCoalescer.test.ts — modified
+
+**Commits verified:**
+- 7af827a: test(05-03): add actorId assertion in burst test — fix unused var lint warning (RED gate)
+- c1758de: feat(05-03): implement pushCoalescer — per-(list,actor) sliding debounce (D-01/D-03) (GREEN gate)
+
+## Self-Check: PASSED