Phase 11: Per-Event Reminders (CAL-13/14, NOTIF-04/05/06) #19

Merged
luckberg merged 41 commits from gsd/phase-11-per-event-reminders into main 2026-06-14 14:06:45 -04:00
Owner

Phase 11: Per-Event Reminders

Implements per-event reminder lead times (CAL-13/CAL-14, NOTIF-04/05/06), promoted from backlog 999.4.

What shipped

  • VALARM layer (vevent.ts): buildTimedValarm, buildAllDayValarm, classifyValarms, extractValarms, computeAlertInstantUtc; DURATION trigger (no VALUE=TEXT).
  • Variable-lead scheduler (reminderScheduler.ts): per-event lead from reminder_lead_minutes, uid:dtstartMs dedup, dropped fixed-15-min/shared-only restriction, all-day 9 AM-local fire, humanized push body.
  • End-to-end plumbing: reminderLeadMinutes through route/outbox schemas, preserve-on-no-change (D-08), buildVeventString wiring, sync-derives-from-VALARM, surfaced on CalendarOccurrence.
  • Reminder picker (EventForm.tsx): allDay-aware preset swap, default None, reset-on-toggle, edit pre-population, Custom-kept preserve.

Gap-closure (Plan 11-05, from code review)

  • CR-01 (blocker): custom/other-client VALARMs are now preserved on edit via a surfaced reminderIsCustom signal (CAL-14 / Pitfall 1 — previously silently stripped).
  • CR-02 (blocker): all-day push body no longer reads "Starts in 0 min".
  • WR-01 post-event trigger → custom; WR-02 server-side max(10080); WR-03 helper-text gating.

Verification

  • 347 API tests + 206 PWA tests green; typecheck + format:check + md:lint clean.
  • 5/5 must-haves verified; gap-closure done with RED→GREEN TDD.
  • Deferred (human): live Fastmail VALARM round-trip + on-device push fire — untestable in dev (no provider connected; tracked in backlog 999.19).

🤖 Generated with Claude Code

## Phase 11: Per-Event Reminders Implements per-event reminder lead times (CAL-13/CAL-14, NOTIF-04/05/06), promoted from backlog 999.4. ### What shipped - **VALARM layer** (`vevent.ts`): `buildTimedValarm`, `buildAllDayValarm`, `classifyValarms`, `extractValarms`, `computeAlertInstantUtc`; DURATION trigger (no VALUE=TEXT). - **Variable-lead scheduler** (`reminderScheduler.ts`): per-event lead from `reminder_lead_minutes`, `uid:dtstartMs` dedup, dropped fixed-15-min/shared-only restriction, all-day 9 AM-local fire, humanized push body. - **End-to-end plumbing**: `reminderLeadMinutes` through route/outbox schemas, preserve-on-no-change (D-08), `buildVeventString` wiring, sync-derives-from-VALARM, surfaced on `CalendarOccurrence`. - **Reminder picker** (`EventForm.tsx`): allDay-aware preset swap, default None, reset-on-toggle, edit pre-population, Custom-kept preserve. ### Gap-closure (Plan 11-05, from code review) - **CR-01 (blocker):** custom/other-client VALARMs are now preserved on edit via a surfaced `reminderIsCustom` signal (CAL-14 / Pitfall 1 — previously silently stripped). - **CR-02 (blocker):** all-day push body no longer reads "Starts in 0 min". - WR-01 post-event trigger → custom; WR-02 server-side max(10080); WR-03 helper-text gating. ### Verification - 347 API tests + 206 PWA tests green; typecheck + format:check + md:lint clean. - 5/5 must-haves verified; gap-closure done with RED→GREEN TDD. - **Deferred (human):** live Fastmail VALARM round-trip + on-device push fire — untestable in dev (no provider connected; tracked in backlog 999.19). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
luckberg added 40 commits 2026-06-14 08:32:44 -04:00
- Add failing tests for buildTimedValarm, buildAllDayValarm (no VALUE=TEXT)
- Add failing tests for buildVeventString VALARM emission (timed/all-day/null/preserve)
- Add failing tests for classifyValarms (none/preset/offlist/custom)
- Add failing tests for extractValarms (round-trip, empty, garbage)
- Add failing tests for computeAlertInstantUtc DST boundaries (spring/fall/summer/winter)
- Import ICAL from ical.js in test file for Component instanceof checks
Task 1 — buildTimedValarm, buildAllDayValarm, VALARM emission in buildVeventString:
- buildTimedValarm(leadMinutes): relative DURATION trigger via resetType('duration') +
  ICAL.Duration.fromSeconds to prevent VALUE=TEXT (Pitfall 2)
- buildAllDayValarm(alertInstantUtc): absolute DATE-TIME trigger via resetType('date-time') +
  ICAL.Time.fromJSDate(utc, true); ensures VALUE=DATE-TIME, no DURATION
- NewEventParams extended with reminderLeadMinutes, valarms, allDayAlertInstantUtc
- buildVeventString: preserve path (valarms[] wins) → all-day absolute → timed relative;
  timed 0 = None per D-06; no emission on null/undefined (CAL-13/D-08)

Task 2 — classifyValarms, extractValarms (CAL-14):
- AlarmClassification type: none | preset | offlist | custom
- PRESET_MINUTES set: 0,5,10,15,30,60,120,1440,2880,10080
- classifyValarms: ICAL.parse try/catch → none/custom/preset/offlist via instanceof ICAL.Time
- extractValarms: returns live ICAL.Component[] for re-attachment; safe on parse failure

Task 3 — computeAlertInstantUtc DST-correct 9 AM local→UTC (NOTIF-06):
- Probes UTC offset at 9 AM (not midnight) so spring-forward/fall-back DST transitions
  before 9 AM resolve with the post-transition offset
- Pure Intl.DateTimeFormat arithmetic, no timezone library; verified at 4 DST boundaries
- 37/37 vevent.test.ts pass (RED→GREEN TDD gate complete)
- 296/296 full API suite pass
- tsc --noEmit clean
- 7 new exported symbols; DST probe-at-9AM deviation documented
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- CAL-14 preserve: UPDATE with no reminderLeadMinutes preserves VALARM from rawVevent
- CAL-13 timed: CREATE with reminderLeadMinutes=15 emits TRIGGER:-PT15M
- CAL-13 clear: UPDATE with reminderLeadMinutes=null emits no VALARM (passes trivially)
- CAL-13 all-day: CREATE with allDay=true and reminderLeadMinutes=1440 emits VALUE=DATE-TIME
- Replace shared+timed filtering tests with NOTIF-04/05 variable-lead tests
- Add timed-0 guard test (D-06: 0 on timed = None — currently FAILING)
- Add personal-calendar dispatch test (isShared restriction dropped)
- Update SINGLE-FIRE test to assert uid:dtstartMs compound key
- Add RESCHEDULE test: new dtstartMs re-fires even for same uid
- Update MISSED-TICK-RECOVERY to use 60s catch-up window
- Add reminderLeadMinutes field to all makeEventRow() calls
- Add reminderLeadMinutes to eventFieldsSchema (ingress validation, T-11-06)
- Add reminderLeadMinutes to outboxPayloadSchema (drain re-validation, IN-03 defense-in-depth)
- Import extractValarms + computeAlertInstantUtc from vevent.ts
- UPDATE branch: hasExplicitReminder gate mirrors hasExplicitRecurrence (WR-01 pattern)
  - absent field → extractValarms(rawVevent) preserved verbatim (CAL-14, D-08)
  - explicit null → clear VALARM (no valarmsToPreserve, null passed to buildVeventString)
  - explicit value + allDay → computeAlertInstantUtc at 9 AM local (D-04)
  - explicit value + timed → passed through to buildTimedValarm via buildVeventString
- CREATE branch: always explicit picker value; compute allDayAlertInstantUtc when allDay
- preset TRIGGER:-PT30M → reminderLeadMinutes=30
- no VALARM → reminderLeadMinutes=null
- absolute DATE-TIME trigger → null (custom kind, D-07/NOTIF-05)
- two VALARMs → null (multiple alarms not resolvable to single lead)
- onDuplicateKeyUpdate set also carries reminderLeadMinutes (upsert keeps column current)
- Import classifyValarms from vevent.ts
- Derive reminderLeadMinutesValue: preset/offlist → leadMinutes; custom/none → null
- Add reminderLeadMinutes to .values() and .onDuplicateKeyUpdate({ set: {} })
- Scheduler now has ground truth for native-client VALARMs (T-11-07 mitigated)
- non-recurring event: occurrence carries reminderLeadMinutes=30 from master
- all-day event with 0-minute trigger: occurrence carries 0 (NULL-vs-0, D-06)
- no VALARM: occurrence carries reminderLeadMinutes=null
- D-10 series-level: all recurring occurrences inherit master's reminderLeadMinutes=60
- Replace fixed 16-min window with per-event variable-lead fire-time check
- Two separate DB queries: timed (allDay=false) + all-day (allDay=true)
- Remove eq(calendars.isShared, true) — personal events now dispatch (NOTIF-05)
- Remove eq(calendarEvents.allDay, false) — all-day handled in separate query
- Add reminder_lead_minutes IS NOT NULL WHERE predicate (NOTIF-05)
- Skip timed events with reminderLeadMinutes===0 in JS (D-06: 0 on timed = None)
- Change dedup key from bare uid to uid:dtstartMs compound key (NOTIF-06)
- Update prune loop to use compound key
- Import computeAlertInstantUtc from vevent.js (Plan 11-01, wave 2 dep)
- Add humanizeLeadMinutes export (Task 2 body formatter, used in dispatch)
- Update test helper mockTwoQueries() to handle two-query dispatch pattern
- All 14 tests GREEN; tsc --noEmit clean; setInterval retained, no node-cron
- Add reminderLeadMinutes: number | null to CalendarOccurrence interface (D-06)
- Import classifyValarms in expand.ts; derive series-level value once per VEVENT
- Add reminderLeadMinutes to both non-recurring and recurring occurrence construction
- Add reminderLeadMinutes to GET /api/events select for edit-mode pre-population
- Add 8 bucket tests: 30→'30 min', 59→'59 min', 60→'1 hr', 90→'1 hr',
  120→'2 hrs', 1440→'1 day', 2880→'2 days', 10080→'7 days'
- Add body-in-dispatch test: 1440-min lead → body='Starts in 1 day'
  (driven by configured lead, not live minutes-to-start delta)
- humanizeLeadMinutes implementation already committed in Task 1 GREEN
- All 23 tests GREEN
- Schema field + VALARM wiring in outbox worker (CAL-13/CAL-14)
- sync.ts VALARM classification → reminderLeadMinutes upsert (D-07/NOTIF-05)
- CalendarOccurrence.reminderLeadMinutes + GET select (D-10)
- 13 new TDD tests; 132/132 broker tests pass; tsc clean
- Add all-day 9 AM tests: 0-lead fires at EDT alert UTC, not midnight
- Add 1440-lead (day-before) and 10080-lead (7-day-before) tests
- Add all-day dedup test: same uid:dtstartMs fires once across ticks
- Fix all-day prune bug: store start-of-next-day as pruneMs instead of
  UTC midnight (which was always <= now by fire time, causing immediate prune)
- Separate dtstartMs (dedup key component) from pruneMs (map cleanup value)
- 28/28 tests GREEN; full API suite 314/314; tsc --noEmit clean
SUMMARY.md covers: uid:dtstartMs dedup, dropped isShared restriction,
all-day 9 AM branch, humanizeLeadMinutes buckets, pruneMs split fix.
Requirements NOTIF-04/05/06 claimed by automated tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- CalendarOccurrence: required reminderLeadMinutes: number | null (atomic mirror of expand.ts, Plan 11-03)
- CreateEventPayload: optional reminderLeadMinutes?: number | null with absent/null/0/positive contract (D-08)
- Update CalendarOccurrence fixtures in EventForm.test.tsx + EventDetailPopover.test.tsx to include the new required field (reminderLeadMinutes: null)
- pwa tsc --noEmit exits 0
- TIMED_REMINDER_PRESETS + ALLDAY_REMINDER_PRESETS sets for preset classification
- humanizeReminderLead() + deriveReminderValue() helpers for off-list synthetic options
- reminderValue state (__none__ / numeric string / __custom__ sentinels)
- allDay toggle resets reminderValue to __none__ (D-03 — no carry-over)
- Reset effect derives reminderValue from occurrence.reminderLeadMinutes on mount/cache update
- Reminder <select id=event-reminder> after Recurrence picker: allDay-conditional option swap (D-02), timed presets when !allDay, day-granularity presets when allDay; synthetic off-list option; Custom (kept) read-only disabled option
- Helper text under select in edit mode for off-list/custom states (D-07)
- executeSubmit payload: __none__ → null, numeric → integer, __custom__ → field omitted (D-08)
- 10 new Phase 11 reminder picker tests (create/allDay-swap/edit-pre-pop/payload-mapping); fix pre-existing None-ambiguity test
- pwa vitest 201/201 pass; tsc --noEmit clean
- apps/pwa/src/components/EventForm.tsx (Task 2)
- apps/api/src/broker/{expand,reminderScheduler,sync,vevent}.ts (Plans 11-01/11-03)
- apps/api/tests/broker/{reminderScheduler,sync}.test.ts (Plans 11-01/11-03)
- 11-04-SUMMARY.md: allDay-aware select, edit pre-population, Custom-kept limitation, playwright smoke results
- STATE.md: advance plan counter, add key decisions, record session
- ROADMAP.md: phase 11 now Complete (4/4 summaries)
- REQUIREMENTS.md: CAL-13 + CAL-14 marked complete
Captured during Phase 11 UAT: dev-bypass user has no calendar provider, blocking hands-on event/reminder testing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- expand.test.ts: 3 new tests asserting reminderIsCustom:true for
  absolute DATE-TIME trigger and multi-VALARM, false for relative preset
- EventForm.test.tsx: 3 new tests asserting __custom__ picker init,
  'Custom (kept)' option visibility, and payload omits reminderLeadMinutes
- Fixtures: absolute-alarm.ics (DATE-TIME VALARM), multi-alarm.ics (2 VALARMs)
- All 6 new tests FAIL (RED): reminderIsCustom field not yet on interface
- expand.ts: add reminderIsCustom:boolean to CalendarOccurrence interface;
  derived from classifyValarms kind==='custom'; propagated to both
  non-recurring and recurring occurrence branches
- client.ts: mirror reminderIsCustom on CalendarOccurrence (atomic mirror)
- EventForm.tsx: extend deriveReminderValue to accept isCustom flag;
  returns '__custom__' when true, making the existing D-08 preserve branch
  live — editing a custom-alarm event now omits reminderLeadMinutes from
  the payload so outboxWorker extractValarms keeps the original VALARM
- Fix existing test fixtures (EventForm.test.tsx, EventDetailPopover.test.tsx)
  to include reminderIsCustom:false on all CalendarOccurrence literals

Fixes CAL-14 Pitfall 1: Apple Calendar absolute DATE-TIME / multi-VALARM
alarms no longer silently stripped on any edit round-trip from the PWA.
5 new tests asserting isAllDay=true branch: lead=0→"Today", 1440→"Tomorrow",
2880→"In 2 days", 10080→"In 1 week"; timed (isAllDay=false) behavior unchanged.
All 5 FAIL (RED): humanizeLeadMinutes only accepts one argument.
- humanizeLeadMinutes: add isAllDay=false param; all-day branch returns
  "Today" (lead=0), "Tomorrow" (1440), "In 1 week" (10080), "In N days" (other)
- byKey map: store isAllDay flag (false for timed, true for all-day)
- dispatch loop: pass event.isAllDay to humanizeLeadMinutes

All-day same-day reminder push now reads "Today" instead of "Starts in 0 min".
Timed event wording unchanged (isAllDay defaults to false).
4 new tests in classifyValarms suite asserting TRIGGER:+PT15M and TRIGGER:PT30M
(positive/no-sign = fires after event) classify as {kind:'custom'}, not as
preset/offlist. Negative triggers regression guards also present.
2 tests FAIL (RED): Math.abs() discards the sign, misclassifies as preset.
classifyValarms: check sign of dur.toSeconds() before preset lookup.
Positive value = alarm fires after event (RFC 5545 TRIGGER:+PT15M or
TRIGGER;RELATED=END:PTNm) → return {kind:'custom'} for preserve path.
Compute leadMinutes as -seconds/60 (was Math.abs) for negative triggers.

Prevents alarm direction inversion: +PT15M was being stored as 15-min-before
lead and re-fired at dtstartUtc-15min — the opposite of the original intent.
- outboxPayloadSchema: 10081 must hard-fail the row (currently dispatches)
- eventFieldsSchema: POST /create with 10081 must 400 (currently 202)
- boundary 10080 and null pass (already correct, no test fails expected)
- eventFieldsSchema (events.ts): rejects reminderLeadMinutes > 10080 with 400
- outboxPayloadSchema (outboxWorker.ts): hard-fails row when value exceeds 1-week cap
- 10080 = 1 week in minutes; matches UI select maximum
- timed event with reminderLeadMinutes=10080 must show 'Custom reminder kept' helper
- currently suppressed: helper text checks !TIMED && !ALLDAY, but 10080 is in ALLDAY
- fix: gate helper text on active preset set only (allDay ? ALLDAY : TIMED)
- helper text condition now uses (allDay ? ALLDAY_REMINDER_PRESETS : TIMED_REMINDER_PRESETS)
- previously checked !TIMED && !ALLDAY: a timed event with 10080 (in ALLDAY set) was
  incorrectly treated as 'in presets' and suppressed the helper text
- synthetic option gating for each allDay/timed branch was already correct
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
One deferred human check: live Fastmail VALARM round-trip + push (untestable in dev, backlog 999.19).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
docs(phase-11): evolve PROJECT.md — per-event reminders validated
CI / changes (pull_request) Successful in 3s
CI / fast-checks (pull_request) Failing after 50s
CI / api (pull_request) Failing after 1m3s
CI / harness (pull_request) Successful in 4m12s
CI / security (pull_request) Successful in 40s
CI / gate (pull_request) Failing after 1s
156bdf057b
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
luckberg added 1 commit 2026-06-14 11:22:09 -04:00
fix(11): make CI green — pin TZ in all-day scheduler tests, drop redundant casts
CI / changes (pull_request) Successful in 2s
CI / fast-checks (pull_request) Successful in 1m25s
CI / api (pull_request) Successful in 1m3s
CI / harness (pull_request) Successful in 4m14s
CI / security (pull_request) Successful in 41s
CI / gate (pull_request) Successful in 1s
eff9b13c66
fast-checks failed on 3 no-unnecessary-type-assertion ESLint errors (reminderIsCustom is now a real CalendarOccurrence field). api failed on 4 all-day 9 AM-local tests that assumed a UTC-4 host; CI runs UTC. Pin process.env.TZ=America/New_York in the all-day describe (production code reads TZ at call time, D-04).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
luckberg merged commit e907b69145 into main 2026-06-14 14:06:45 -04:00
luckberg deleted branch gsd/phase-11-per-event-reminders 2026-06-14 14:06:46 -04:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: luckberg/familysync#19