Skip to content

Conversation

richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Oct 3, 2025

  • prefetch listWindowsWithThumbnails and listDisplaysWithThumbnails when the main window opens so the new recording picker list view feels quicker to open
  • filter cached thumbnails against the cheaper listWindows/listScreens data to drop closed or detached targets without refetching
  • preserve existing refetch logic and menu interactions while relying on Solid Query’s request deduping

Summary by CodeRabbit

  • Bug Fixes

    • Display and window target lists now show only currently available screens/windows, eliminating stale or missing entries.
    • Targets refresh automatically without needing to open menus first, improving accuracy when selecting a screen or window.
  • Refactor

    • Streamlined data fetching and filtering to reduce duplication and improve reliability, resulting in a more consistent selection experience.

Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Walkthrough

The route component now always fetches display/window targets plus current screens/windows. It derives existing IDs from screens/windows and filters the target lists to only those present. Query declarations for screens/windows were consolidated earlier in the component, and duplicated declarations later were removed.

Changes

Cohort / File(s) Summary
Target filtering and query restructuring
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
Removed conditional query enabling; added screens and windows queries; derived existing display/window ID sets; filtered displayTargetsData and windowTargetsData to existing IDs; moved and deduplicated screens/windows query declarations.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant UI as NewMain Route Component
  participant Q1 as Query: screens
  participant Q2 as Query: windows
  participant Q3 as Query: displayTargets
  participant Q4 as Query: windowTargets
  participant F as Filter Logic
  participant V as View

  UI->>Q1: fetch screens
  UI->>Q2: fetch windows
  UI->>Q3: fetch displayTargets
  UI->>Q4: fetch windowTargets

  Q1-->>UI: screens[]
  Q2-->>UI: windows[]
  Q3-->>UI: displayTargets[]
  Q4-->>UI: windowTargets[]

  UI->>F: derive existingDisplayIds/existingWindowIds
  F-->>UI: filtered displayTargets/windowTargets

  UI->>V: render filtered targets
  note over V: Only targets present in current screens/windows are shown
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I twitch my whiskers, sift the streams,
Screens and windows trim their seams.
Targets hop in tidy rows,
Only those the meadow knows.
Queries bloom, duplicates fade—
A cleaner warren, neatly laid. 🌿🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately captures the primary change of prewarming the new recording picker’s thumbnails on window initialization to speed up its first render, which aligns directly with the described prefetching of listWindowsWithThumbnails and listDisplaysWithThumbnails and the removal of conditional fetch logic. It is concise, specific, and free of extraneous detail, making it clear to reviewers what the main improvement is.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch prewarm-list-options

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57e7b7c and a2384c2.

📒 Files selected for processing (1)
  • apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management

Files:

  • apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add inline, block, or docstring comments in any language; code must be self-explanatory

Files:

  • apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; leverage shared types from packages

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Files:

  • apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
apps/desktop/src/utils/queries.ts (2)
  • listScreens (45-50)
  • listWindows (28-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (3)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (3)

319-330: LGTM! Prewarming achieves the stated goal.

The queries now fetch immediately on mount rather than waiting for user interaction, which will make the dropdown menus feel more responsive. The screens query polls every 1000ms (per its definition in utils/queries.ts), while windows does not auto-poll, relying instead on the refetch logic later in the component.


335-359: LGTM! Filtering logic is well-structured.

The implementation correctly:

  • Uses Set for O(1) membership checks
  • Falls back to unfiltered data when the cheaper queries haven't completed
  • Preserves undefined when the base query hasn't succeeded, maintaining proper loading states
  • Filters out stale thumbnails for closed/detached targets without triggering refetches

478-495: Refetch logic safe; no action required
The signature === undefined guard delays thumbnail refetch until both screen/window lists have loaded, preventing empty-list flashes.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@richiemcilroy richiemcilroy merged commit 5c83606 into main Oct 3, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant