fix: review feedback — literal error types, extract FatalErrorScreen, type params, fix mock signature

REPORT.md

@@ -1,66 +0,0 @@
-# Fix: Daily.co UX & ICS Meeting Dedup
-
-Branch: `fix-daily-ux` (worktree at `.worktrees/fix-daily-ux`)
-Base: `main` (`1ce1c7a9`)
-
-## Context / Incident
-
-Feb 2, ~11:30 AM Montreal — user JLee reported being "kicked out / can't rejoin" Max's room. Screenshot showed "Failed to join meeting. try again." Two active meetings were visible with ~1s creation difference, plus one recently deactivated.
-
-### Root Cause Analysis
-
-**The kick**: Exact cause unknown (server logs lost — container recreated ~Feb 3). `eject_at_room_exp` is NOT set on Daily rooms (defaults false), so room expiry did NOT eject users. Most likely a WebRTC connection drop or Daily.co infrastructure hiccup. After disconnection, rejoin failed because the meeting had passed `end_date` → join endpoint returned 400 "Meeting has ended" → frontend showed generic error.
-
-**The duplicate meetings**: Max's room uses an aggregated ICS feed (user.fm) that merges Cal.com + Google Calendar. Every Cal.com booking appears twice with different `ics_uid` values — one `@Cal.com`, one UUID from Google Calendar. Reflector deduplicates by `ics_uid`, so it creates 2 `calendar_event` rows → 2 meetings → 2 Daily rooms. This is systematic and ongoing for every Cal.com booking. Proven with production DB data across Feb 2, 3, 5.
-
-## What's Done
-
-### Commit 1: `e9e16764` — Daily.co error messages
-**File**: `www/app/[roomName]/components/DailyRoom.tsx`
-
-- Added `frame.on("error", handler)` for Daily.co `DailyEventObjectFatalError` events
-- `fatalError` state + ref to distinguish forced disconnect from voluntary leave
-- Specific error screens per `error.type`:
-  - `connection-error` → "Connection lost" + "Try Rejoining" (page reload) + "Leave"
-  - `exp-room` → "Meeting time has ended" + "Back to Room"
-  - `ejected` → "You were removed" + "Back to Room"
-  - fallback → shows `errorMsg` + "Back to Room"
-- Join failure (`joinMutation.isError`) now shows actual API error detail via `printApiError` instead of generic message
-- Added "Back to Room" button on join failure screen
-
-### Commit 2: `1da687fe` — ICS meeting dedup
-**Files**: `server/reflector/db/meetings.py`, `server/reflector/worker/ics_sync.py`, `server/tests/test_ics_dedup.py`
-
-- `MeetingController.get_by_room_and_time_window(room, start_date, end_date)` — queries for existing active meeting with exact same room + start + end times
-- In `create_upcoming_meetings_for_event`: after checking `get_by_calendar_event`, also checks `get_by_room_and_time_window`. If a meeting already exists for the same time slot, skips creation and logs it.
-- 2 tests: dedup prevents duplicate, different times still create separate meetings. Both pass.
-
-### Commit 3: `238d7684` — Review fixes
-- Import `ApiError` type instead of inline type literal in cast
-- Move `get_database` import from inline to top-of-file in test
-
-## What's Left / Known Issues
-
-### Must fix: `joinMutation.error as ApiError` cast
-In `DailyRoom.tsx:404`, the `as ApiError` cast is needed because `openapi-react-query` types the error based on the OpenAPI spec (which declares `detail: ValidationError[]` for all errors), but real 400 errors return `{ detail: "string" }`. This is a known codebase-wide issue (see `apiHooks.ts:12` XXX comment). The cast is safe at runtime (`printApiError` handles both string and array), but it's a type-level lie. Proper fix: either fix the OpenAPI error response schemas, or make `printApiError` accept `unknown` and do full runtime narrowing. Both are broader changes beyond this PR's scope.
-
-### Not tested (needs manual verification)
-- Daily.co error event rendering — requires live Daily room in browser to trigger `error` events. Cannot be tested locally without a running meeting.
-- The "Try Rejoining" button simply reloads the page. Could be improved to re-call the join endpoint directly without full reload.
-
-### Layer A (ICS feed config) not addressed
-The dedup code fix (Layer B) prevents duplicate meetings, but the root cause is Max's aggregated calendar feed including both Cal.com and Google Calendar copies. Configuring the ICS URL to point directly at Cal.com's feed (or deduplicating at the feed level) would eliminate the duplicate `calendar_event` rows too. This is a user configuration change, not a code change.
-
-### Dedup edge case: shared rooms
-The dedup check uses exact `(room_id, start_date, end_date)` match. For shared rooms where multiple people could legitimately book the same time slot, this could incorrectly skip a valid meeting. Currently not an issue since Max's room is personal, but worth noting if this logic is applied broadly. Could add a guard like `if not room.is_shared` if needed.
-
-### No DB index for dedup query
-`get_by_room_and_time_window` queries on `(room_id, start_date, end_date, is_active)`. Existing `idx_meeting_room_id` index on `room_id` is sufficient for current scale. No composite index added.
-
-## Files Changed (total: +315/-4)
-```
-www/app/[roomName]/components/DailyRoom.tsx  — +93 (error event handling, error UIs)
-server/reflector/db/meetings.py              — +21 (get_by_room_and_time_window)
-server/reflector/worker/ics_sync.py          — +17/-2 (dedup check before meeting creation)
-server/tests/test_ics_dedup.py               — +186 (new test file, 2 tests)
-```

server/reflector/worker/ics_sync.py

@@ -5,7 +5,7 @@ from celery import shared_task
 from celery.utils.log import get_task_logger
 
 from reflector.asynctask import asynctask
-from reflector.db.calendar_events import calendar_events_controller
+from reflector.db.calendar_events import CalendarEvent, calendar_events_controller
 from reflector.db.meetings import meetings_controller
 from reflector.db.rooms import Room, rooms_controller
 from reflector.redis_cache import RedisAsyncLock
@@ -86,7 +86,9 @@ def _should_sync(room) -> bool:
 MEETING_DEFAULT_DURATION = timedelta(hours=1)
 
 
-async def create_upcoming_meetings_for_event(event, create_window, room: Room):
+async def create_upcoming_meetings_for_event(
+    event: CalendarEvent, create_window: datetime, room: Room
+):
     if event.start_time <= create_window:
         return
     existing_meeting = await meetings_controller.get_by_calendar_event(event.id, room)

server/tests/test_ics_dedup.py

@@ -158,7 +158,7 @@ async def test_different_time_windows_create_separate_meetings():
 
         call_count = 0
 
-        async def mock_create_meeting(room_name_prefix, end_date, room):
+        async def mock_create_meeting(room_name_prefix, *, end_date, room):
             nonlocal call_count
             call_count += 1
             return AsyncMock(

www/app/[roomName]/components/DailyRoom.tsx

@@ -47,6 +47,70 @@ const RAW_TRACKS_NAMESPACE = "a1b2c3d4-e5f6-7890-abcd-ef1234567890";
 const RECORDING_START_DELAY_MS = 2000;
 const RECORDING_START_MAX_RETRIES = 5;
 
+function FatalErrorScreen({
+  error,
+  roomName,
+}: {
+  error: FatalError;
+  roomName: string;
+}) {
+  const router = useRouter();
+  switch (error.type) {
+    case "connection-error":
+      return (
+        <Center width="100vw" height="100vh">
+          <VStack gap={4}>
+            <Text color="red.500">
+              Connection lost. Please check your network.
+            </Text>
+            <Button onClick={() => window.location.reload()}>
+              Try Rejoining
+            </Button>
+            <Button
+              variant="outline"
+              onClick={() => router.push(`/${roomName}`)}
+            >
+              Leave
+            </Button>
+          </VStack>
+        </Center>
+      );
+    case "exp-room":
+      return (
+        <Center width="100vw" height="100vh">
+          <VStack gap={4}>
+            <Text color="red.500">The meeting time has ended.</Text>
+            <Button onClick={() => router.push(`/${roomName}`)}>
+              Back to Room
+            </Button>
+          </VStack>
+        </Center>
+      );
+    case "ejected":
+      return (
+        <Center width="100vw" height="100vh">
+          <VStack gap={4}>
+            <Text color="red.500">You were removed from this meeting.</Text>
+            <Button onClick={() => router.push(`/${roomName}`)}>
+              Back to Room
+            </Button>
+          </VStack>
+        </Center>
+      );
+    default:
+      return (
+        <Center width="100vw" height="100vh">
+          <VStack gap={4}>
+            <Text color="red.500">Something went wrong: {error.message}</Text>
+            <Button onClick={() => router.push(`/${roomName}`)}>
+              Back to Room
+            </Button>
+          </VStack>
+        </Center>
+      );
+  }
+}
+
 type Meeting = components["schemas"]["Meeting"];
 type Room = components["schemas"]["RoomDetails"];
 
@@ -84,7 +148,12 @@ const USE_FRAME_INIT_STATE = {
   joined: false as boolean,
 } as const;
 
-type FatalError = { type: string; message: string };
+type DailyFatalErrorType =
+  | "connection-error"
+  | "exp-room"
+  | "ejected"
+  | (string & {});
+type FatalError = { type: DailyFatalErrorType; message: string };
 
 // Daily js and not Daily react used right now because daily-js allows for prebuild interface vs. -react is customizable but has no nice defaults
 const useFrame = (
@@ -419,61 +488,7 @@ export default function DailyRoom({ meeting, room }: DailyRoomProps) {
   }
 
   if (fatalError) {
-    const renderFatalError = () => {
+    return <FatalErrorScreen error={fatalError} roomName={roomName} />;
-      switch (fatalError.type) {
-        case "connection-error":
-          return (
-            <VStack gap={4}>
-              <Text color="red.500">
-                Connection lost. Please check your network.
-              </Text>
-              <Button onClick={() => window.location.reload()}>
-                Try Rejoining
-              </Button>
-              <Button
-                variant="outline"
-                onClick={() => router.push(`/${roomName}`)}
-              >
-                Leave
-              </Button>
-            </VStack>
-          );
-        case "exp-room":
-          return (
-            <VStack gap={4}>
-              <Text color="red.500">The meeting time has ended.</Text>
-              <Button onClick={() => router.push(`/${roomName}`)}>
-                Back to Room
-              </Button>
-            </VStack>
-          );
-        case "ejected":
-          return (
-            <VStack gap={4}>
-              <Text color="red.500">You were removed from this meeting.</Text>
-              <Button onClick={() => router.push(`/${roomName}`)}>
-                Back to Room
-              </Button>
-            </VStack>
-          );
-        default:
-          return (
-            <VStack gap={4}>
-              <Text color="red.500">
-                Something went wrong: {fatalError.message}
-              </Text>
-              <Button onClick={() => router.push(`/${roomName}`)}>
-                Back to Room
-              </Button>
-            </VStack>
-          );
-      }
-    };
-    return (
-      <Center width="100vw" height="100vh">
-        {renderFatalError()}
-      </Center>
-    );
   }
 
   if (!roomUrl) {