Skip to content

fix: room query batching#848

Open
deardarlingoose wants to merge 7 commits intomainfrom
fix-room-query-batching
Open

fix: room query batching#848
deardarlingoose wants to merge 7 commits intomainfrom
fix-room-query-batching

Conversation

@deardarlingoose
Copy link
Contributor

@deardarlingoose deardarlingoose commented Feb 6, 2026

User description

EITHER that PR OR #850

problem: we have performance issues with 100% CPU load

the pr doesn't resolve it. instead, it opens path for resolution in another way: declutter logs, make LLMs stop from hallucinating biasing into "the problem is frontend spamming with those requests"

the long-term effect: mild performance leak mitigation

LLM DESCRIPTION PROOFRED:

Summary

  • Batch N+1 room status queries into a single bulk POST via @yornaath/batshit DataLoader pattern (2N+2 requests → 3 requests for N rooms)
  • Add frontend test infrastructure: Jest + ts-jest + jsdom + testing-library, with first integration test for the batcher
  • Fix CI workflow: pnpm version mismatch (8 → 10 auto-detect), add concurrency group, remove redundant setup-node step

Backend

  • New POST /v1/rooms/meetings/bulk-status endpoint
  • Bulk DB methods: RoomController.get_by_names(), MeetingController.get_all_active_for_rooms()

Frontend

  • meetingStatusBatcher.ts: DataLoader-style batcher using @yornaath/batshit with 10ms window
  • apiHooks.ts: useRoomActiveMeetings / useRoomUpcomingMeetings now go through batcher
  • meetingStatusBatcher.test.tsx: integration tests verifying batch collapse

CI

  • .github/workflows/test_next_server.yml: fix pnpm 8→10, add concurrency group, dedupe setup-node

Test plan

  • CI workflow runs on this PR (triggers on www/** changes)
  • Rooms page loads with batched requests (verify single bulk POST in network tab)
  • Individual room pages still work (batcher handles single-room case)

LLM, PROFRED:

Batcher vs rooms: Room[] prop-drilling — trade-offs

Current approach (batcher + per-room hooks):

  • Each independently calls useRoomActiveMeetings(name) / useRoomUpcomingMeetings(name)
  • Batcher collapses N calls into 1 bulk POST within a 10ms window
  • Pros: each component is self-contained, no prop threading, react-query handles caching/refetch per room
  • Cons: implicit batching is magic — harder to debug, new dependency (@yornaath/batshit)

Alternative (prop-drilling Room[] with meeting status):

  • Parent fetches all rooms + their meeting statuses in one query, passes Room & { active_meetings, upcoming_events } down
  • Pros: explicit data flow, single queryKey, no batcher dependency, standard $api.useQuery pattern, easier to type (Room[] with extended fields)
  • Cons: parent owns all refetch logic, adding a room card to a different page means re-implementing the parent fetch, cache invalidation is coarser (invalidate entire list vs single room)

Key difference: batcher preserves per-component data ownership (each card manages its own loading/error state). Prop-drilling centralizes it (parent is the single source of truth). For a list page where all cards load together, prop-drilling is simpler. For cases
where room cards appear in varied contexts, the batcher is more flexible


PR Type

Enhancement, Tests


Description

  • Batch room status queries into single bulk POST

  • Add frontend test infrastructure with Jest

  • Fix CI workflow for pnpm version

  • Implement DataLoader pattern for query batching


Changes walkthrough 📝

Relevant files
Enhancement
6 files
rooms.py
Add bulk meeting status endpoint                                                 
+67/-1   
calendar_events.py
Add method to get upcoming events for multiple rooms         
+20/-0   
meetings.py
Add method to get active meetings for multiple rooms         
+17/-0   
rooms.py
Add method to get rooms by names                                                 
+5/-0     
apiHooks.ts
Refactor room hooks to use batcher                                             
+30/-40 
meetingStatusBatcher.ts
Implement DataLoader-style batcher for meeting status       
+37/-0   
Tests
1 files
meetingStatusBatcher.test.tsx
Add tests for meeting status batcher                                         
+217/-0 
Documentation
1 files
reflector-api.d.ts
Update API types for bulk status endpoint                               
+64/-0   
Configuration changes
2 files
jest.config.js
Configure Jest for React testing                                                 
+19/-5   
test_next_server.yml
Fix CI workflow configuration                                                       
+5/-7     
Dependencies
2 files
pnpm-lock.yaml
Add testing dependencies                                                                 
+794/-14
package.json
Add batshit and testing libraries                                               
+5/-0     

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Reduces rooms list page from 2N+2 HTTP requests to 1 POST request.
    Backend: POST /v1/rooms/meetings/bulk-status with 3 DB queries total.
    Frontend: @yornaath/batshit DataLoader-style batcher with 10ms window.
    BATSHIT_REPORT.md: full context on bulk query batching — business goal,
    approach, all changes, verification status, and how to run.
    FRONTEND_TEST_RESEARCH.md: research on unit testing react-query hooks
    with jest.mock, renderHook, and batcher testing patterns.
    @deardarlingoose deardarlingoose changed the title Fix room query batching fix: room query batching Feb 6, 2026
    - Fix pnpm version mismatch in test_next_server.yml (8 → auto-detect 10)
    - Add concurrency group to cancel stale CI runs
    - Remove redundant setup-node step
    - Update jest.config.js for jsdom + tsx support
    - Add meetingStatusBatcher integration test (3 tests)
    - Extract createMeetingStatusBatcher factory for testability
    @deardarlingoose deardarlingoose force-pushed the fix-room-query-batching branch from c9bd667 to 931c344 Compare February 6, 2026 00:06
    dearlordylord and others added 3 commits February 5, 2026 19:36
    - Add max_length=100 on BulkStatusRequest.room_names to prevent abuse
    - Filter bulk endpoint results to rooms user can see (owned or shared)
    - Throw on bulk-status fetch error instead of silently returning empty data
    - Fix room_by_id type annotation: dict[str, DbRoom] instead of Any
    - Remove stale "200ms" comment in test
    - Enable strict: true in jest tsconfig
    - Remove working docs from tracked files
    - Simplify redundant ternary in test helper
    Replace custom meetingStatusKeys with $api.queryOptions()-derived keys
    so cache identity matches the original per-room GET endpoints.
    @deardarlingoose
    Copy link
    Contributor Author

    self-reviewed

    deardarlingoose pushed a commit that referenced this pull request Feb 6, 2026
    Alternative to the batcher approach (#848): parent fetches all room
    meeting statuses in a single bulk POST and passes data down as props.
    No extra dependency (@yornaath/batshit), no implicit batching magic.
    
    Backend: POST /v1/rooms/meetings/bulk-status + bulk DB methods.
    Frontend: useRoomsBulkMeetingStatus hook in RoomList, MeetingStatus
    receives data as props instead of calling per-room hooks.
    CI: fix pnpm 8→10 auto-detect, add concurrency group.
    Tests: Jest+jsdom+testing-library for bulk hook.
    @deardarlingoose deardarlingoose marked this pull request as ready for review February 6, 2026 01:06
    @pr-agent-monadical
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The new bulk endpoint doesn't have proper error handling for cases where room_names might be empty or if the database queries fail.

    @router.post("/rooms/meetings/bulk-status", response_model=dict[str, RoomMeetingStatus])
    async def rooms_bulk_meeting_status(
        request: BulkStatusRequest,
        user: Annotated[Optional[auth.UserInfo], Depends(auth.current_user_optional)],
    ):
        user_id = user["sub"] if user else None
    
        all_rooms = await rooms_controller.get_by_names(request.room_names)
        # Filter to rooms the user can see (owned or shared), matching rooms_list behavior
        rooms = [
            r
            for r in all_rooms
            if r.is_shared or (user_id is not None and r.user_id == user_id)
        ]
        room_by_id: dict[str, DbRoom] = {r.id: r for r in rooms}
        room_ids = list(room_by_id.keys())
    
        current_time = datetime.now(timezone.utc)
        active_meetings, upcoming_events = await asyncio.gather(
            meetings_controller.get_all_active_for_rooms(room_ids, current_time),
            calendar_events_controller.get_upcoming_for_rooms(room_ids),
        )
    
        # Group by room name
        active_by_room: dict[str, list[Meeting]] = defaultdict(list)
        for m in active_meetings:
            room = room_by_id.get(m.room_id)
            if not room:
                continue
            m.platform = room.platform
            if user_id != room.user_id and m.platform == "whereby":
                m.host_room_url = ""
            active_by_room[room.name].append(m)
    
        upcoming_by_room: dict[str, list[CalendarEventResponse]] = defaultdict(list)
        for e in upcoming_events:
            room = room_by_id.get(e.room_id)
            if not room:
                continue
            if user_id != room.user_id:
                e.description = None
                e.attendees = None
            upcoming_by_room[room.name].append(e)
    
        result: dict[str, RoomMeetingStatus] = {}
        for name in request.room_names:
            result[name] = RoomMeetingStatus(
                active_meetings=active_by_room.get(name, []),
                upcoming_events=upcoming_by_room.get(name, []),
            )
    
        return result
    Error Propagation

    The batcher error handling is minimal and might not properly propagate errors to the UI. Consider adding more specific error types and handling.

    fetcher: async (roomNames: string[]): Promise<MeetingStatusResult[]> => {
      const unique = [...new Set(roomNames)];
      const { data, error } = await client.POST(
        "/v1/rooms/meetings/bulk-status",
        { body: { room_names: unique } },
      );
      if (error || !data) {
        throw new Error(
          `bulk-status fetch failed: ${JSON.stringify(error ?? "no data")}`,
        );
      }

    model_validate(from_attributes=True) needed to convert DB Meeting and
    CalendarEvent to their view-layer Pydantic counterparts.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants