Skip to content

Conversation

@its-thepoe
Copy link
Contributor

@its-thepoe its-thepoe commented Dec 17, 2025

Previously, the crop modal only showed the first frame of the video (a static
screenshot). I couldn't crop based on content that appeared later in the
recording, and I had to guess the crop and go back to the main timeline to check.

This change adds a timeline slider to the crop modal, allowing users to scrub
through the entire video and select any frame as their cropping reference.

What's New

  • Timeline slider below the crop preview with current time and total duration
  • Instant video scrubbing using native
  • Preload strategy where video metadata preloads on editor open, and full video preloads on Crop button hover/focus for instant display
  • Loading indicator that shows a "Loading" pill while video buffers
  • Smooth crossfade with a 150ms transition from screenshot to video
  • Screenshot fallback that gracefully degrades if video fails to load

Technical Notes

  • Uses raw source video (display.mp4) for scrubbing, not the rendered output with effects. This is intentional as crop applies to the background layer, and effects render on top.
  • Video is positioned absolute with object-contain to prevent layout shift while maintaining aspect ratio.

How to Test

  1. Open a recording in the editor
  2. Hover over the "Crop" button (triggers video preload)
  3. Click "Crop" to open the modal
  4. Verify:
    • Loading indicator appears briefly (if video not yet cached)
    • Smooth crossfade from screenshot to video
    • Slider shows current time (left) and total duration (right)
    • Dragging slider updates video frame instantly
    • Crop bounds can be adjusted at any frame
    • Clicking "Save" persists the crop correctly

Additional improvements since:

  • remove unconditional reset of enable_new_recording_flow
    The flag was being reset to false on every app start, making the setting
    unusable. Users can now toggle between old and new recording flow in settings.

  • remove debug telemetry fetch calls from App.tsx, (window-chrome).tsx,
    and setup.tsx. Deleted all #region agent log blocks and hardcoded local
    ingest endpoint calls.

  • remove debug console.log statements from context.ts and Suspense
    fallbacks

  • correct IconLucideLoader2 import path to use kebab-case
    (loader-2.jsx instead of loader2.jsx)

  • correct localTime calculation in currentSegment memo
    Changed from seg.start + (time - elapsed) * seg.timescale to
    seg.start / seg.timescale + (time - elapsed) for proper time conversion

Summary by CodeRabbit

  • New Features

    • Live video preview with playback/timeline controls in the editor crop dialog.
    • Background video preloading with hover/focus-triggered preloads.
    • Expanded routing with many lazily-loaded pages for improved code-splitting.
    • New global loader icon available for UI use.
  • Bug Fixes

    • New recording flow default set to disabled.
    • Startup now checks microphone and camera permissions.
  • Refactor

    • Simplified suspense/fallback and route mounting structure.
  • Chores

    • Added docs directory to ignore rules.
    • Build config updated to pre-bundle additional dependencies; client init defers OS detection.

✏️ Tip: You can customize this high-level summary in your review settings.


Note

Adds a video-backed crop modal with a timeline slider and preloading for instant scrubbing, plus permission checks for mic/camera, lazy-loaded routing, and updated defaults.

  • Editor:
    • Crop modal: Replaces static screenshot with video preview and a timeline Slider for frame-accurate cropping; adds loading indicator and smooth crossfade.
    • Preloading: Introduces routes/editor/cropVideoPreloader.ts and triggers full preload on Crop hover/focus.
  • Permissions/Setup:
    • Setup now checks microphone and camera permissions and surfaces them in the UI.
    • Startup gating updated to include mic/camera permission checks.
  • Settings Defaults:
    • enable_new_recording_flow default set to false and initialized when preference not set.
  • Routing/Client:
    • Replaces FileRoutes with explicit, lazily-loaded Routes for main/settings pages and overlays.
    • entry-client.tsx: dynamically imports @tauri-apps/plugin-os before mount; Vite optimizeDeps.include updated.
  • Misc:
    • Add .docs/ to .gitignore.
    • Fix icon auto-import path (IconLucideLoader2).
    • Minor benchmark.rs receive loop cleanup.

Written by Cursor Bugbot for commit 62110cc. This will update automatically on new commits. Configure here.

…ropping

Previously, the crop modal only showed the first frame of the video (a static
screenshot). I couldn't crop based on content that appeared later in the
recording, and I had to guess the crop and go back to the main timeline to check.

This change adds a timeline slider to the crop modal, allowing users to scrub
through the entire video and select any frame as their cropping reference.

## What's New

- **Timeline slider** below the crop preview with current time and total duration
- **Instant video scrubbing** using native <video> element with hardware-accelerated seeking
- **Preload strategy** where video metadata preloads on editor open, and full video preloads on Crop button hover/focus for instant display
- **Loading indicator** that shows a "Loading" pill while video buffers
- **Smooth crossfade** with a 150ms transition from screenshot to video
- **Screenshot fallback** that gracefully degrades if video fails to load

## Technical Notes

- Uses raw source video (display.mp4) for scrubbing, not the rendered output with effects. This is intentional as crop applies to the background layer, and effects render on top.
- Video is positioned absolute with object-contain to prevent layout shift while maintaining aspect ratio.

## Files Changed

- [apps/desktop/src/routes/editor/Editor.tsx](cci:7://file:///Users/MAC/Desktop/Desktop%20-%20Poe%27s%20MacBook%20Pro/Engineering/Open%20Source/Cap/apps/desktop/src/routes/editor/Editor.tsx:0:0-0:0) - crop dialogue with video, slider, loading indicator
- [apps/desktop/src/routes/editor/Player.tsx](cci:7://file:///Users/MAC/Desktop/Desktop%20-%20Poe%27s%20MacBook%20Pro/Engineering/Open%20Source/Cap/apps/desktop/src/routes/editor/Player.tsx:0:0-0:0) - preload on Crop button hover/focus
- [apps/desktop/src/routes/editor/context.ts](cci:7://file:///Users/MAC/Desktop/Desktop%20-%20Poe%27s%20MacBook%20Pro/Engineering/Open%20Source/Cap/apps/desktop/src/routes/editor/context.ts:0:0-0:0) - preload metadata on editor mount
- [apps/desktop/src/routes/editor/cropVideoPreloader.ts](cci:7://file:///Users/MAC/Desktop/Desktop%20-%20Poe%27s%20MacBook%20Pro/Engineering/Open%20Source/Cap/apps/desktop/src/routes/editor/cropVideoPreloader.ts:0:0-0:0) - new preload manager module

## How to Test

1. Open a recording in the editor
2. Hover over the "Crop" button (triggers video preload)
3. Click "Crop" to open the modal
4. Verify:
   - Loading indicator appears briefly (if video not yet cached)
   - Smooth crossfade from screenshot to video
   - Slider shows current time (left) and total duration (right)
   - Dragging slider updates video frame instantly
   - Crop bounds can be adjusted at any frame
   - Clicking "Save" persists the crop correctly
@its-thepoe its-thepoe marked this pull request as draft December 17, 2025 00:49
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 17, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Refactors desktop routing to a nested, lazy Route tree; adds editor video preloading and an in-crop live preview with slider; expands startup/setup permission checks to include microphone and camera and flips a recording-flow default; updates Vite optimizeDeps and typings; and simplifies a benchmark loop.

Changes

Cohort / File(s) Change Summary
Ignore & Build Config
/.gitignore, apps/desktop/app.config.ts
Added .docs/ to .gitignore; added entries to optimizeDeps.include in the desktop Vite config.
Startup & Settings
apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/general_settings.rs
Startup permission checks now include microphone and camera; default enable_new_recording_flow changed from true to false and init preserves false when picker preference unset.
Client Init & Routing
apps/desktop/src/entry-client.tsx, apps/desktop/src/App.tsx, apps/desktop/src/routes/(window-chrome).tsx, apps/desktop/src/routes/(window-chrome)/setup.tsx
Deferred dynamic OS plugin import in client init; replaced FileRoutes with a nested Route tree and many lazy-loaded pages; removed exported route info const; added mic/camera entries to setup UI.
Editor — Preview & Preload
apps/desktop/src/routes/editor/Editor.tsx, apps/desktop/src/routes/editor/Player.tsx, apps/desktop/src/routes/editor/context.ts, apps/desktop/src/routes/editor/cropVideoPreloader.ts
Added in-crop live video preview with playback sync and a previewTime slider; wired hover/focus preloading for Crop button; invoked metadata preload at editor init; added new cropVideoPreloader.ts exporting metadata/full preload, state getters, and cleanup.
Types & Examples
packages/ui-solid/src/auto-imports.d.ts, crates/frame-converter/examples/benchmark.rs
Added global IconLucideLoader2 declaration; simplified benchmark loop to rely solely on a deadline (removed per-iteration receive/counting).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Entry as entry-client.tsx
    participant App as App.tsx
    participant Startup as src-tauri/lib.rs
    participant Router
    participant Setup as setup.tsx
    participant Editor as editor/context.ts
    participant Preloader as cropVideoPreloader.ts

    User->>Entry: launch app
    Entry->>Entry: initApp() (dynamic OS import)
    Entry->>App: mount app
    App->>Startup: check permissions (screen, accessibility, microphone, camera)
    alt all permissions granted
        Startup->>Router: open Main window
    else missing perms
        Startup->>Router: open Setup
        Router->>Setup: show mic & camera prompts
        User->>Setup: grant perms
        Setup->>Router: open Main window
    end

    User->>Router: navigate to /editor
    Router->>Editor: initialize editor/context
    Editor->>Preloader: preloadCropVideoMetadata(segment-0)
    User->>Editor: hover/focus Crop button
    Editor->>Preloader: preloadCropVideoFull()
    User->>Editor: click Crop -> show cropper with video + slider
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Files/areas needing extra attention:

  • apps/desktop/src/App.tsx — routing refactor, many lazy routes and Suspense behavior.
  • apps/desktop/src/routes/editor/Editor.tsx — playback sync, previewTime slider, video DOM timing and seek behavior.
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts — DOM-manipulating preload lifecycle and cleanup.
  • apps/desktop/src-tauri/src/lib.rs & apps/desktop/src/routes/(window-chrome)/setup.tsx — permission-check changes affecting startup routing and UI.
  • apps/desktop/src-tauri/src/general_settings.rs — default change impacting recording-flow behavior.

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • Brendonovich

Poem

🐰 I hopped through routes and lazy loads bright,
Preloaded segments for crop-time delight,
Mic and camera checks kept startup polite,
Sliders and playbacks now synced just right,
A tiny rabbit cheers—code glowing at night! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.88% 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 describes the main feature addition: a timeline slider for frame-accurate cropping in the crop modal, which is the primary focus of this PR.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3127103 and 62110cc.

📒 Files selected for processing (2)
  • apps/desktop/src/routes/editor/Editor.tsx (3 hunks)
  • apps/desktop/src/routes/editor/context.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use TanStack Query v5 for all client-side server state and data fetching in TypeScript files

Files:

  • apps/desktop/src/routes/editor/context.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use strict TypeScript; avoid any type; leverage shared types from @cap/* packages
Follow camelCase naming for variables and functions; PascalCase for components; hooks must start with 'use' prefix
Use Biome for linting and formatting; match existing formatting conventions in the codebase
Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Use static skeletons for loading states that mirror content; avoid bouncing animations
Memoize expensive work, code-split naturally, and use Next/Image for remote assets

**/*.{ts,tsx,js,jsx}: Use 2-space indent for TypeScript files; format with Biome using pnpm format
Use Biome for code formatting and linting; run pnpm format regularly
Use kebab-case for file names (e.g., user-menu.tsx); use PascalCase for components

Files:

  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Never add any form of comments to code (single-line //, multi-line /* /, JSDoc /* */, or any other comment syntax); code must be self-explanatory through naming, types, and structure
Directory naming must use lowercase-dashed convention

Files:

  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/Editor.tsx
apps/desktop/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

apps/desktop/**/*.ts: Use @tanstack/solid-query for server state management in SolidJS components
Use generated commands and events from tauri_specta for IPC; never manually construct IPC calls
Listen directly to generated events from tauri_specta and use typed event interfaces

Files:

  • apps/desktop/src/routes/editor/context.ts
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (AGENTS.md)

Never add comments to code (//, /* */, ///, //!, #, etc.); code must be self-explanatory through naming, types, and structure

Files:

  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/Editor.tsx
**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Use React Query hooks with Server Actions for mutations and perform precise cache updates using setQueryData/setQueriesData instead of broad invalidations

Files:

  • apps/desktop/src/routes/editor/Editor.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Memoize expensive work, code-split naturally, and use Next/Image for remote assets
🧬 Code graph analysis (1)
apps/desktop/src/routes/editor/Editor.tsx (3)
apps/desktop/src/routes/editor/utils.ts (1)
  • formatTime (1-13)
apps/desktop/src/routes/editor/ui.tsx (1)
  • Slider (65-149)
apps/desktop/src/routes/editor/context.ts (1)
  • FPS (60-60)
⏰ 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). (1)
  • GitHub Check: Cursor Bugbot
🔇 Additional comments (9)
apps/desktop/src/routes/editor/context.ts (3)

15-15: LGTM!

The new imports support the crop video preloader feature and the memory leak fix for the worker readiness effect.

Also applies to: 43-46


755-760: LGTM!

The cleanup logic properly handles both the worker readiness effect disposal and the crop video preloader cleanup. The optional chaining safely handles the case where cleanup runs before the editor instance is fully created.


767-769: Memory leak resolved; early metadata preload looks good.

The createRoot now properly captures and stores its dispose function, which is invoked during cleanup. This addresses the memory leak flagged in previous reviews. The early metadata preload for segment-0 optimizes the crop modal's initial load experience.

Also applies to: 787-792

apps/desktop/src/routes/editor/Editor.tsx (6)

47-55: LGTM!

The new imports properly support the timeline slider controls and time formatting in the crop modal.


425-441: LGTM!

The expanded context and new video state signals are properly initialized. previewTime correctly inherits the current playback position, and videoLoaded appropriately defaults to false.


443-457: Critical issue resolved: localTime calculation now correct.

The localTime calculation at line 451 now correctly produces seconds by dividing seg.start by seg.timescale before adding the offset. This resolves the critical issue flagged in previous reviews where the calculation was producing timescale units instead of seconds.


459-479: Previous issue resolved: videoLoaded now resets on segment change.

The effect at lines 465-473 properly resets videoLoaded when the segment index changes, addressing the medium-severity issue from previous reviews. The video sync effect at lines 475-479 correctly updates currentTime to enable instant frame-accurate scrubbing as described in the PR objectives.


641-674: LGTM!

The video overlay implementation correctly provides a smooth 150ms crossfade from screenshot to video with a loading indicator. The absolute positioning with object-contain prevents layout shift while preserving aspect ratio, and the fallback behavior gracefully handles video load failures.


678-694: LGTM!

The timeline slider controls provide frame-accurate scrubbing with proper time displays. The step size of 1/FPS ensures users can select any frame, and the reactive binding to previewTime enables instant video updates while dragging, as specified in the PR objectives.


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.

@its-thepoe its-thepoe marked this pull request as ready for review December 17, 2025 00:50
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (4)
crates/frame-converter/examples/benchmark.rs (1)

68-75: Handle the Option return value from recv_timeout.

Line 69 uses let _ = to discard the Option returned by recv_timeout, which violates the coding guideline requiring all Option/Result types to be handled. While the intent—draining the output channel to prevent backpressure—is clear, the return value should be handled explicitly.

Consider one of these approaches:

     while Instant::now() < deadline {
-        let _ = pool.recv_timeout(Duration::from_millis(100));
+        let _ = pool.recv_timeout(Duration::from_millis(100)).is_some();
         let stats = pool.stats();

Or make the intent explicit by consuming the value:

     while Instant::now() < deadline {
-        let _ = pool.recv_timeout(Duration::from_millis(100));
+        while pool.try_recv().is_some() {}
+        std::thread::sleep(Duration::from_millis(100));
         let stats = pool.stats();

Based on coding guidelines: "Always handle Result/Option or types marked #[must_use]; never ignore them"

apps/desktop/src/routes/editor/cropVideoPreloader.ts (2)

7-17: Missing error handler for video load failure.

If the video fails to load during metadata preload, preloadState will remain stuck at "metadata", preventing any retry or fallback behavior. Consider adding an onerror handler to reset state on failure.

 export function preloadCropVideoMetadata(videoPath: string) {
 	if (preloadState !== "idle") return;
 
 	currentVideoPath = videoPath;
 	preloadedVideo = document.createElement("video");
 	preloadedVideo.preload = "metadata";
 	preloadedVideo.src = convertFileSrc(videoPath);
 	preloadedVideo.muted = true;
+	preloadedVideo.onerror = () => {
+		preloadState = "idle";
+		currentVideoPath = null;
+		preloadedVideo = null;
+	};
 	preloadedVideo.load();
 	preloadState = "metadata";
 }

19-30: Add error handler for full preload as well.

Similarly, if the full preload fails, the state will be stuck at "full". Reset state on error for consistent behavior.

 export function preloadCropVideoFull() {
 	if (!preloadedVideo || preloadState === "full" || preloadState === "ready")
 		return;
 
 	preloadedVideo.preload = "auto";
 	preloadedVideo.load();
 	preloadState = "full";
 
 	preloadedVideo.oncanplaythrough = () => {
 		preloadState = "ready";
 	};
+	preloadedVideo.onerror = () => {
+		preloadState = "idle";
+		currentVideoPath = null;
+		preloadedVideo = null;
+	};
 }
apps/desktop/src/routes/editor/Editor.tsx (1)

459-469: Consider resetting videoLoaded when segment changes.

When videoSrc changes (user scrubs to a different segment), the video reloads but videoLoaded remains true, so the loading indicator won't appear during segment transitions. If this causes noticeable stalls without feedback, consider resetting the state.

+createEffect(
+	on(videoSrc, () => {
+		setVideoLoaded(false);
+	})
+);

 createEffect(() => {
 	if (videoRef && videoLoaded()) {
 		videoRef.currentTime = currentSegment().localTime;
 	}
 });
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db01c9d and a1d2ac0.

📒 Files selected for processing (14)
  • .gitignore (1 hunks)
  • apps/desktop/app.config.ts (1 hunks)
  • apps/desktop/src-tauri/src/general_settings.rs (2 hunks)
  • apps/desktop/src-tauri/src/lib.rs (1 hunks)
  • apps/desktop/src/App.tsx (6 hunks)
  • apps/desktop/src/entry-client.tsx (1 hunks)
  • apps/desktop/src/routes/(window-chrome).tsx (2 hunks)
  • apps/desktop/src/routes/(window-chrome)/setup.tsx (2 hunks)
  • apps/desktop/src/routes/editor/Editor.tsx (3 hunks)
  • apps/desktop/src/routes/editor/Player.tsx (2 hunks)
  • apps/desktop/src/routes/editor/context.ts (2 hunks)
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts (1 hunks)
  • crates/frame-converter/examples/benchmark.rs (1 hunks)
  • packages/ui-solid/src/auto-imports.d.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Use React Query hooks with Server Actions for mutations and perform precise cache updates using setQueryData/setQueriesData instead of broad invalidations

Files:

  • apps/desktop/src/entry-client.tsx
  • apps/desktop/src/routes/(window-chrome).tsx
  • apps/desktop/src/routes/(window-chrome)/setup.tsx
  • apps/desktop/src/routes/editor/Player.tsx
  • apps/desktop/src/App.tsx
  • apps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use strict TypeScript; avoid any type; leverage shared types from @cap/* packages
Follow camelCase naming for variables and functions; PascalCase for components; hooks must start with 'use' prefix
Use Biome for linting and formatting; match existing formatting conventions in the codebase
Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Use static skeletons for loading states that mirror content; avoid bouncing animations
Memoize expensive work, code-split naturally, and use Next/Image for remote assets

**/*.{ts,tsx,js,jsx}: Use 2-space indent for TypeScript files; format with Biome using pnpm format
Use Biome for code formatting and linting; run pnpm format regularly
Use kebab-case for file names (e.g., user-menu.tsx); use PascalCase for components

Files:

  • apps/desktop/src/entry-client.tsx
  • apps/desktop/src/routes/(window-chrome).tsx
  • apps/desktop/app.config.ts
  • apps/desktop/src/routes/(window-chrome)/setup.tsx
  • packages/ui-solid/src/auto-imports.d.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/Player.tsx
  • apps/desktop/src/App.tsx
  • apps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Never add any form of comments to code (single-line //, multi-line /* /, JSDoc /* */, or any other comment syntax); code must be self-explanatory through naming, types, and structure
Directory naming must use lowercase-dashed convention

Files:

  • apps/desktop/src/entry-client.tsx
  • apps/desktop/src/routes/(window-chrome).tsx
  • apps/desktop/app.config.ts
  • apps/desktop/src/routes/(window-chrome)/setup.tsx
  • packages/ui-solid/src/auto-imports.d.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/Player.tsx
  • apps/desktop/src/App.tsx
  • apps/desktop/src/routes/editor/Editor.tsx
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (AGENTS.md)

Never add comments to code (//, /* */, ///, //!, #, etc.); code must be self-explanatory through naming, types, and structure

Files:

  • apps/desktop/src/entry-client.tsx
  • apps/desktop/src/routes/(window-chrome).tsx
  • apps/desktop/app.config.ts
  • crates/frame-converter/examples/benchmark.rs
  • apps/desktop/src/routes/(window-chrome)/setup.tsx
  • apps/desktop/src-tauri/src/lib.rs
  • packages/ui-solid/src/auto-imports.d.ts
  • apps/desktop/src-tauri/src/general_settings.rs
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/Player.tsx
  • apps/desktop/src/App.tsx
  • apps/desktop/src/routes/editor/Editor.tsx
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use TanStack Query v5 for all client-side server state and data fetching in TypeScript files

Files:

  • apps/desktop/app.config.ts
  • packages/ui-solid/src/auto-imports.d.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
  • apps/desktop/src/routes/editor/context.ts
**/*.config.{js,ts,mjs,cjs}

📄 CodeRabbit inference engine (CLAUDE.md)

Configuration files must follow their respective tool conventions (Next.js, Tailwind, etc.)

Files:

  • apps/desktop/app.config.ts
apps/desktop/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

apps/desktop/**/*.ts: Use @tanstack/solid-query for server state management in SolidJS components
Use generated commands and events from tauri_specta for IPC; never manually construct IPC calls
Listen directly to generated events from tauri_specta and use typed event interfaces

Files:

  • apps/desktop/app.config.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
  • apps/desktop/src/routes/editor/context.ts
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Never use dbg!() macro; use proper logging (tracing::debug!, etc.) instead
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them
Use duration.saturating_sub(other) instead of duration - other to avoid panics on underflow
Merge nested if statements: write 'if a && b { }' instead of 'if a { if b { } }'
Don't call .clone() on Copy types (integers, bools, etc.); copy them directly
Use function references directly: iter.map(foo) instead of iter.map(|x| foo(x))
Accept &[T] or &str instead of &Vec or &String in function parameters for flexibility
Use .is_empty() instead of .len() == 0 or .len() > 0 / .len() != 0
Don't assign () to a variable: write foo(); instead of let _ = foo(); or let x = foo(); when return is unit
Use .unwrap_or(val) instead of .unwrap_or_else(|| val) when the default is a simple/cheap value
Use 'for item in &collection' or 'for (i, item) in collection.iter().enumerate()' instead of 'for i in 0..collection.len()'
Use value.clamp(min, max) instead of manual if chains or .min(max).max(min) patterns
Always handle Result/Option or types marked #[must_use]; never ignore them

**/*.rs: Use rustfmt and workspace clippy lints for Rust code formatting and linting
Use snake_case for Rust module names and kebab-case for crate names
Never use dbg!() macro in Rust code; use proper logging instead (Clippy: dbg_macro = deny)
Always handle Result/Option or types marked #[must_use]; never ignore them (Rust compiler lint: unused_must_use = deny)
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them (Clippy: let_underscore_future = deny)
Use saturating_sub instead of - for Duration to avoid panics (Clippy: unchecked_duration_subtraction = deny)
Merge nested if statements: use if a && b { } instead of if a { if b { } } (Clippy: collapsible_if = deny)
Don't call .clone() on Copy types; just copy them directly (Clippy: clone_on_copy = deny)
U...

Files:

  • crates/frame-converter/examples/benchmark.rs
  • apps/desktop/src-tauri/src/lib.rs
  • apps/desktop/src-tauri/src/general_settings.rs
🧠 Learnings (4)
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/web/**/*.tsx : Prefer Server Components for initial data fetching and SEO; pass initialData to client components and let React Query take over

Applied to files:

  • apps/desktop/src/entry-client.tsx
  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/desktop/**/*.ts : Use tanstack/solid-query for server state management in SolidJS components

Applied to files:

  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/web/**/*.tsx : Minimize useEffect usage in React components; compute during render, handle logic in event handlers, and ensure cleanups for subscriptions/timers

Applied to files:

  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:40.743Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T14:29:40.743Z
Learning: Applies to apps/web/**/*.{ts,tsx} : Use `useEffectQuery`/`useEffectMutation` from `@/lib/EffectRuntime` in client components; do not call `EffectRuntime.run*` directly

Applied to files:

  • apps/desktop/src/App.tsx
🧬 Code graph analysis (4)
crates/frame-converter/examples/benchmark.rs (1)
crates/frame-converter/src/pool.rs (1)
  • stats (218-225)
apps/desktop/src/routes/editor/context.ts (1)
apps/desktop/src/routes/editor/cropVideoPreloader.ts (1)
  • preloadCropVideoMetadata (7-17)
apps/desktop/src/routes/editor/Player.tsx (1)
apps/desktop/src/routes/editor/cropVideoPreloader.ts (1)
  • preloadCropVideoFull (19-30)
apps/desktop/src/routes/editor/Editor.tsx (3)
apps/desktop/src/routes/editor/utils.ts (1)
  • formatTime (1-13)
apps/desktop/src/routes/editor/ui.tsx (1)
  • Slider (65-149)
apps/desktop/src/routes/editor/context.ts (1)
  • FPS (56-56)
🔇 Additional comments (12)
.gitignore (1)

53-53: Verify the documentation directory ignore is intentional.

The .docs/ pattern addition appears unrelated to the PR's primary objectives (crop modal timeline slider). Per the AI summary, there are no related documentation file changes indicated in this PR. Please confirm whether this is intentional cleanup or should be part of a separate PR.

apps/desktop/app.config.ts (1)

40-55: LGTM!

The optimizeDeps.include configuration appropriately pre-bundles the dependencies used across the lazy-loaded routes, improving development server cold start performance and aligning with the dynamic import pattern introduced in App.tsx.

apps/desktop/src-tauri/src/lib.rs (1)

2678-2698: LGTM!

The startup flow now correctly gates on all four required permissions (screen recording, accessibility, microphone, and camera) before showing the main window, aligning with the expanded permission prompts in the setup UI.

apps/desktop/src/routes/(window-chrome)/setup.tsx (1)

42-52: LGTM!

The Microphone and Camera permissions are correctly added with appropriate descriptions, aligning with the updated startup permission requirements in the Rust backend.

apps/desktop/src/routes/editor/Player.tsx (1)

261-262: LGTM!

The preload triggers on onMouseEnter and onFocus implement the second stage of the two-stage preloading strategy effectively. The preloader module handles idempotency, so multiple trigger events won't cause redundant loads.

apps/desktop/src/routes/editor/context.ts (1)

756-758: Verify path assumption for multi-segment recordings.

The preload path hardcodes segment-0, which should work for single-segment recordings. For multi-segment recordings, confirm that segment-0/display.mp4 is always the appropriate video for the crop preview, or consider using the first segment from instance.recordings.segments[0] if available.

apps/desktop/src/entry-client.tsx (1)

4-16: LGTM!

The refactor to async initialization with dynamic import is well-structured. Error handling gracefully falls back to mounting without the platform class, and the dynamic import pattern aligns with the broader code-splitting strategy in this PR.

apps/desktop/src/App.tsx (2)

21-74: LGTM!

The lazy-loaded route components are well-organized with clear naming conventions. This code-splitting approach will improve initial load time by deferring the loading of route components until they're needed.


220-268: LGTM on the routing structure.

The explicit Route tree with nested layouts provides clear route organization:

  • Settings routes correctly nested under SettingsLayout
  • Window chrome routes grouped under WindowChromeLayout
  • Standalone routes (camera, capture-area, etc.) appropriately placed outside the chrome layout

This gives better control over route configuration than the previous FileRoutes approach.

apps/desktop/src/routes/editor/cropVideoPreloader.ts (1)

32-47: LGTM!

The getter functions and cleanup are straightforward and properly reset all module state.

apps/desktop/src/routes/editor/Editor.tsx (2)

631-684: LGTM!

The video overlay with crossfade transitions, loading indicator, and timeline slider are well-implemented. The aria-label on the slider and frame-accurate step (1 / FPS) are good details.


425-431: LGTM!

Appropriate expansion of context destructuring to support the new video playback and timeline functionality.

@its-thepoe its-thepoe marked this pull request as draft December 17, 2025 01:13
@its-thepoe its-thepoe marked this pull request as ready for review December 17, 2025 01:14
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/desktop/src/routes/editor/Editor.tsx (1)

459-469: Consider resetting videoLoaded when video source changes.

When scrubbing across segment boundaries, videoSrc() changes but videoLoaded remains true until the new video loads. This could briefly show a stale frame. Consider resetting the state when the segment changes:

+								createEffect(
+									on(
+										() => currentSegment().index,
+										() => setVideoLoaded(false),
+										{ defer: true },
+									),
+								);
+
 								createEffect(() => {
 									if (videoRef && videoLoaded()) {
 										videoRef.currentTime = currentSegment().localTime;
 									}
 								});
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1d2ac0 and bb67f03.

📒 Files selected for processing (7)
  • apps/desktop/src-tauri/src/general_settings.rs (2 hunks)
  • apps/desktop/src/App.tsx (3 hunks)
  • apps/desktop/src/routes/(window-chrome).tsx (1 hunks)
  • apps/desktop/src/routes/(window-chrome)/setup.tsx (2 hunks)
  • apps/desktop/src/routes/editor/Editor.tsx (3 hunks)
  • apps/desktop/src/routes/editor/context.ts (2 hunks)
  • packages/ui-solid/src/auto-imports.d.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/(window-chrome)/setup.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Never use dbg!() macro; use proper logging (tracing::debug!, etc.) instead
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them
Use duration.saturating_sub(other) instead of duration - other to avoid panics on underflow
Merge nested if statements: write 'if a && b { }' instead of 'if a { if b { } }'
Don't call .clone() on Copy types (integers, bools, etc.); copy them directly
Use function references directly: iter.map(foo) instead of iter.map(|x| foo(x))
Accept &[T] or &str instead of &Vec or &String in function parameters for flexibility
Use .is_empty() instead of .len() == 0 or .len() > 0 / .len() != 0
Don't assign () to a variable: write foo(); instead of let _ = foo(); or let x = foo(); when return is unit
Use .unwrap_or(val) instead of .unwrap_or_else(|| val) when the default is a simple/cheap value
Use 'for item in &collection' or 'for (i, item) in collection.iter().enumerate()' instead of 'for i in 0..collection.len()'
Use value.clamp(min, max) instead of manual if chains or .min(max).max(min) patterns
Always handle Result/Option or types marked #[must_use]; never ignore them

**/*.rs: Use rustfmt and workspace clippy lints for Rust code formatting and linting
Use snake_case for Rust module names and kebab-case for crate names
Never use dbg!() macro in Rust code; use proper logging instead (Clippy: dbg_macro = deny)
Always handle Result/Option or types marked #[must_use]; never ignore them (Rust compiler lint: unused_must_use = deny)
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them (Clippy: let_underscore_future = deny)
Use saturating_sub instead of - for Duration to avoid panics (Clippy: unchecked_duration_subtraction = deny)
Merge nested if statements: use if a && b { } instead of if a { if b { } } (Clippy: collapsible_if = deny)
Don't call .clone() on Copy types; just copy them directly (Clippy: clone_on_copy = deny)
U...

Files:

  • apps/desktop/src-tauri/src/general_settings.rs
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (AGENTS.md)

Never add comments to code (//, /* */, ///, //!, #, etc.); code must be self-explanatory through naming, types, and structure

Files:

  • apps/desktop/src-tauri/src/general_settings.rs
  • apps/desktop/src/routes/editor/Editor.tsx
  • apps/desktop/src/routes/(window-chrome).tsx
  • apps/desktop/src/App.tsx
  • packages/ui-solid/src/auto-imports.d.ts
**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Use React Query hooks with Server Actions for mutations and perform precise cache updates using setQueryData/setQueriesData instead of broad invalidations

Files:

  • apps/desktop/src/routes/editor/Editor.tsx
  • apps/desktop/src/routes/(window-chrome).tsx
  • apps/desktop/src/App.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use strict TypeScript; avoid any type; leverage shared types from @cap/* packages
Follow camelCase naming for variables and functions; PascalCase for components; hooks must start with 'use' prefix
Use Biome for linting and formatting; match existing formatting conventions in the codebase
Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Use static skeletons for loading states that mirror content; avoid bouncing animations
Memoize expensive work, code-split naturally, and use Next/Image for remote assets

**/*.{ts,tsx,js,jsx}: Use 2-space indent for TypeScript files; format with Biome using pnpm format
Use Biome for code formatting and linting; run pnpm format regularly
Use kebab-case for file names (e.g., user-menu.tsx); use PascalCase for components

Files:

  • apps/desktop/src/routes/editor/Editor.tsx
  • apps/desktop/src/routes/(window-chrome).tsx
  • apps/desktop/src/App.tsx
  • packages/ui-solid/src/auto-imports.d.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Never add any form of comments to code (single-line //, multi-line /* /, JSDoc /* */, or any other comment syntax); code must be self-explanatory through naming, types, and structure
Directory naming must use lowercase-dashed convention

Files:

  • apps/desktop/src/routes/editor/Editor.tsx
  • apps/desktop/src/routes/(window-chrome).tsx
  • apps/desktop/src/App.tsx
  • packages/ui-solid/src/auto-imports.d.ts
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use TanStack Query v5 for all client-side server state and data fetching in TypeScript files

Files:

  • packages/ui-solid/src/auto-imports.d.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Memoize expensive work, code-split naturally, and use Next/Image for remote assets
📚 Learning: 2025-12-07T14:29:40.743Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T14:29:40.743Z
Learning: Applies to **/tauri.ts|**/queries.ts|apps/desktop/src-tauri/gen/** : Never edit auto-generated files: `**/tauri.ts`, `**/queries.ts`, `apps/desktop/src-tauri/gen/**`

Applied to files:

  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/desktop/**/*.ts : Use tanstack/solid-query for server state management in SolidJS components

Applied to files:

  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/web/**/*.tsx : Prefer Server Components for initial data fetching and SEO; pass initialData to client components and let React Query take over

Applied to files:

  • apps/desktop/src/App.tsx
🧬 Code graph analysis (2)
apps/desktop/src/routes/editor/Editor.tsx (3)
apps/desktop/src/routes/editor/utils.ts (1)
  • formatTime (1-13)
apps/desktop/src/routes/editor/ui.tsx (1)
  • Slider (65-149)
apps/desktop/src/routes/editor/context.ts (1)
  • FPS (56-56)
apps/desktop/src/routes/(window-chrome).tsx (1)
apps/desktop/src/components/Loader.tsx (1)
  • AbsoluteInsetLoader (1-9)
🔇 Additional comments (8)
packages/ui-solid/src/auto-imports.d.ts (1)

85-85: No action needed—the import path is correct.

The Lucide icon library includes a loader-2 icon, and the current code properly uses the kebab-case import path loader-2.jsx, which is consistent with Lucide's naming convention and all other icons in this file.

apps/desktop/src-tauri/src/general_settings.rs (1)

136-138: Logic is now consistent and clearer.

The default for enable_new_recording_flow is now false, and the initialization logic only sets it to false for users who haven't set their picker preference yet. This is a cleaner approach than the previous implementation that had an unconditional reset.

Also applies to: 261-264

apps/desktop/src/routes/(window-chrome).tsx (1)

41-48: Simplified Suspense fallbacks look good.

The fallback simplification removes unnecessary custom functions and logging. The inner Suspense without a fallback correctly prevents content flicker during route transitions.

apps/desktop/src/routes/editor/Editor.tsx (3)

443-457: Segment time calculation is now correct.

The localTime calculation has been fixed to return seconds (seg.start / seg.timescale + (time - elapsed)), which is the correct unit for video.currentTime. This addresses the issue from the previous review.


631-664: Video/image composite with loading state is well implemented.

The layered approach with crossfade transitions, loading indicator, and error fallback to the screenshot provides a good user experience. The object-contain class correctly preserves aspect ratio.


668-684: Timeline slider implementation is solid.

Frame-accurate seeking with step={1 / FPS}, proper time formatting on both ends, and accessibility via aria-label all align with the PR objectives.

apps/desktop/src/App.tsx (2)

21-74: Lazy loading setup looks good.

All page components are properly lazy-loaded, enabling code splitting. This aligns with the learning about memoizing expensive work and code-splitting naturally. Based on learnings.


145-173: Nested route structure with WindowChromeLayout is well-organized.

The separation between chrome-wrapped routes (main, settings, etc.) and standalone routes (camera, editor, capture-area) is logical. Settings sub-routes are properly nested.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/desktop/src/routes/editor/cropVideoPreloader.ts (1)

7-22: Consider handling path changes when already preloading.

If the editor navigates to a different recording while metadata is still loading, subsequent calls with a different videoPath are silently ignored due to the preloadState !== "idle" guard.

Consider resetting state when a different path is requested:

 export function preloadCropVideoMetadata(videoPath: string) {
-	if (preloadState !== "idle") return;
+	if (preloadState !== "idle") {
+		if (currentVideoPath === videoPath) return;
+		cleanup();
+	}

 	currentVideoPath = videoPath;
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13b2c30 and ef3e306.

📒 Files selected for processing (2)
  • apps/desktop/src/routes/editor/context.ts (4 hunks)
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use TanStack Query v5 for all client-side server state and data fetching in TypeScript files

Files:

  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use strict TypeScript; avoid any type; leverage shared types from @cap/* packages
Follow camelCase naming for variables and functions; PascalCase for components; hooks must start with 'use' prefix
Use Biome for linting and formatting; match existing formatting conventions in the codebase
Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Use static skeletons for loading states that mirror content; avoid bouncing animations
Memoize expensive work, code-split naturally, and use Next/Image for remote assets

**/*.{ts,tsx,js,jsx}: Use 2-space indent for TypeScript files; format with Biome using pnpm format
Use Biome for code formatting and linting; run pnpm format regularly
Use kebab-case for file names (e.g., user-menu.tsx); use PascalCase for components

Files:

  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Never add any form of comments to code (single-line //, multi-line /* /, JSDoc /* */, or any other comment syntax); code must be self-explanatory through naming, types, and structure
Directory naming must use lowercase-dashed convention

Files:

  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
apps/desktop/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

apps/desktop/**/*.ts: Use @tanstack/solid-query for server state management in SolidJS components
Use generated commands and events from tauri_specta for IPC; never manually construct IPC calls
Listen directly to generated events from tauri_specta and use typed event interfaces

Files:

  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (AGENTS.md)

Never add comments to code (//, /* */, ///, //!, #, etc.); code must be self-explanatory through naming, types, and structure

Files:

  • apps/desktop/src/routes/editor/context.ts
  • apps/desktop/src/routes/editor/cropVideoPreloader.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Memoize expensive work, code-split naturally, and use Next/Image for remote assets
🧬 Code graph analysis (1)
apps/desktop/src/routes/editor/context.ts (1)
apps/desktop/src/routes/editor/cropVideoPreloader.ts (1)
  • preloadCropVideoMetadata (7-22)
🔇 Additional comments (4)
apps/desktop/src/routes/editor/context.ts (1)

754-757: LGTM - Preload initialization looks correct.

The preload call is appropriately placed after instance creation and before frame rendering setup. The path construction aligns with the segment directory structure.

apps/desktop/src/routes/editor/cropVideoPreloader.ts (3)

3-5: Module-level singleton state is appropriate for this use case.

The singleton pattern works well here since only one crop modal can be open at a time. Consumers will need to poll getPreloadState() since the state isn't reactive.


24-40: LGTM - Full preload upgrade logic is correct.

The guard conditions properly prevent redundant loads, and the state transitions from "metadata"/"full" to "ready" on canplaythrough are appropriate for the preload strategy.


50-57: LGTM - Cleanup properly releases resources.

Setting src = "" releases network resources. Since the video is always muted, explicitly calling pause() isn't necessary here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/App.tsx (1)

209-212: Remove debug console.log.

The console logging statement in the suspense fallback is debug code and should be removed before merging.

Apply this diff:

-					fallback={
-						(() => {
-							console.log("Root suspense fallback showing");
-						}) as any
-					}
+					fallback={<></>}
♻️ Duplicate comments (1)
apps/desktop/src/App.tsx (1)

91-104: Remove debug telemetry code before merging.

These debug fetch calls to localhost and the surrounding // #region agent log comments were already flagged in a previous review. The comments violate the coding guideline prohibiting any form of comments, and this debug instrumentation should not be merged to production.

Also applies to: 115-128, 133-146, 176-198

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef3e306 and f1a6d66.

📒 Files selected for processing (3)
  • apps/desktop/src-tauri/src/lib.rs (1 hunks)
  • apps/desktop/src/App.tsx (6 hunks)
  • apps/desktop/src/routes/editor/Player.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/routes/editor/Player.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Use React Query hooks with Server Actions for mutations and perform precise cache updates using setQueryData/setQueriesData instead of broad invalidations

Files:

  • apps/desktop/src/App.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use strict TypeScript; avoid any type; leverage shared types from @cap/* packages
Follow camelCase naming for variables and functions; PascalCase for components; hooks must start with 'use' prefix
Use Biome for linting and formatting; match existing formatting conventions in the codebase
Use Tailwind CSS for styling in web components; stay consistent with spacing and tokens
Use static skeletons for loading states that mirror content; avoid bouncing animations
Memoize expensive work, code-split naturally, and use Next/Image for remote assets

**/*.{ts,tsx,js,jsx}: Use 2-space indent for TypeScript files; format with Biome using pnpm format
Use Biome for code formatting and linting; run pnpm format regularly
Use kebab-case for file names (e.g., user-menu.tsx); use PascalCase for components

Files:

  • apps/desktop/src/App.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Never add any form of comments to code (single-line //, multi-line /* /, JSDoc /* */, or any other comment syntax); code must be self-explanatory through naming, types, and structure
Directory naming must use lowercase-dashed convention

Files:

  • apps/desktop/src/App.tsx
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (AGENTS.md)

Never add comments to code (//, /* */, ///, //!, #, etc.); code must be self-explanatory through naming, types, and structure

Files:

  • apps/desktop/src/App.tsx
  • apps/desktop/src-tauri/src/lib.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Never use dbg!() macro; use proper logging (tracing::debug!, etc.) instead
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them
Use duration.saturating_sub(other) instead of duration - other to avoid panics on underflow
Merge nested if statements: write 'if a && b { }' instead of 'if a { if b { } }'
Don't call .clone() on Copy types (integers, bools, etc.); copy them directly
Use function references directly: iter.map(foo) instead of iter.map(|x| foo(x))
Accept &[T] or &str instead of &Vec or &String in function parameters for flexibility
Use .is_empty() instead of .len() == 0 or .len() > 0 / .len() != 0
Don't assign () to a variable: write foo(); instead of let _ = foo(); or let x = foo(); when return is unit
Use .unwrap_or(val) instead of .unwrap_or_else(|| val) when the default is a simple/cheap value
Use 'for item in &collection' or 'for (i, item) in collection.iter().enumerate()' instead of 'for i in 0..collection.len()'
Use value.clamp(min, max) instead of manual if chains or .min(max).max(min) patterns
Always handle Result/Option or types marked #[must_use]; never ignore them

**/*.rs: Use rustfmt and workspace clippy lints for Rust code formatting and linting
Use snake_case for Rust module names and kebab-case for crate names
Never use dbg!() macro in Rust code; use proper logging instead (Clippy: dbg_macro = deny)
Always handle Result/Option or types marked #[must_use]; never ignore them (Rust compiler lint: unused_must_use = deny)
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them (Clippy: let_underscore_future = deny)
Use saturating_sub instead of - for Duration to avoid panics (Clippy: unchecked_duration_subtraction = deny)
Merge nested if statements: use if a && b { } instead of if a { if b { } } (Clippy: collapsible_if = deny)
Don't call .clone() on Copy types; just copy them directly (Clippy: clone_on_copy = deny)
U...

Files:

  • apps/desktop/src-tauri/src/lib.rs
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Memoize expensive work, code-split naturally, and use Next/Image for remote assets
📚 Learning: 2025-12-07T14:29:40.743Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T14:29:40.743Z
Learning: Applies to **/tauri.ts|**/queries.ts|apps/desktop/src-tauri/gen/** : Never edit auto-generated files: `**/tauri.ts`, `**/queries.ts`, `apps/desktop/src-tauri/gen/**`

Applied to files:

  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/desktop/**/*.ts : Use tanstack/solid-query for server state management in SolidJS components

Applied to files:

  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/web/**/*.tsx : Prefer Server Components for initial data fetching and SEO; pass initialData to client components and let React Query take over

Applied to files:

  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:19.180Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-07T14:29:19.180Z
Learning: Applies to apps/web/**/*.tsx : Minimize useEffect usage in React components; compute during render, handle logic in event handlers, and ensure cleanups for subscriptions/timers

Applied to files:

  • apps/desktop/src/App.tsx
📚 Learning: 2025-12-07T14:29:40.743Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T14:29:40.743Z
Learning: Applies to apps/web/**/*.{ts,tsx} : Use `useEffectQuery`/`useEffectMutation` from `@/lib/EffectRuntime` in client components; do not call `EffectRuntime.run*` directly

Applied to files:

  • apps/desktop/src/App.tsx
⏰ 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). (1)
  • GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
apps/desktop/src/App.tsx (2)

1-1: LGTM! Well-structured lazy loading.

The addition of explicit Route imports and extensive lazy-loaded page components aligns with best practices for code-splitting and improves initial load performance. The lazy loading pattern is correctly applied across all route components.

Also applies to: 8-8, 21-74


219-267: LGTM! Clean route structure.

The explicit Route tree is well-organized with appropriate nesting. The WindowChromeLayout parent route provides consistent layout for main app routes, while utility routes (camera, editor, debug) remain standalone. The settings subroutes are properly nested, and all components correctly reference the lazy-loaded definitions.

Comment on lines 2700 to +2703
if !permissions.screen_recording.permitted()
|| !permissions.accessibility.permitted()
|| !permissions.microphone.permitted()
|| !permissions.camera.permitted()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Read context around lines 2700-2703 to understand the permission flow
sed -n '2685,2730p' apps/desktop/src-tauri/src/lib.rs

Repository: CapSoftware/Cap

Length of output: 1862


🏁 Script executed:

# Find where the permissions check is implemented
rg "fn do_permissions_check" --max-count 5 -B 2 -A 20

Repository: CapSoftware/Cap

Length of output: 2006


🏁 Script executed:

# Find the OSPermissionsCheck struct and its permitted() method
rg "struct OSPermissionsCheck|fn permitted" --max-count 10 -B 2 -A 5

Repository: CapSoftware/Cap

Length of output: 1107


🏁 Script executed:

# Look for the Setup window implementation to see if it's skippable
rg "ShowCapWindow::Setup" --max-count 10 -B 5 -A 10

Repository: CapSoftware/Cap

Length of output: 2610


🏁 Script executed:

# Look for the Setup window file or implementation
fd "setup" --type f --extension tsx --extension ts --extension rs | head -20

Repository: CapSoftware/Cap

Length of output: 265


🏁 Script executed:

# Read the setup window implementation
cat -n apps/desktop/src/routes/\(window-chrome\)/setup.tsx | head -100

Repository: CapSoftware/Cap

Length of output: 3350


🏁 Script executed:

# Continue reading the setup window implementation
sed -n '100,250p' apps/desktop/src/routes/\(window-chrome\)/setup.tsx

Repository: CapSoftware/Cap

Length of output: 4244


Confirm that mandatory microphone and camera permissions at startup align with product goals.

The Setup window requires users to grant all four permissions (screen recording, accessibility, microphone, and camera) before accessing the main application. The "Continue" button is disabled until every permission shows as granted or not needed. Users cannot proceed without granting all permissions, even if they only intend to use screen recording features.

While the PR indicates this expansion is intentional, verify with product/design that forcing microphone and camera grants at initial onboarding is the desired UX pattern. This creates a hard gate on features that users may not need, potentially causing drop-off during onboarding. If these permissions are truly optional features, consider deferring their requests until users attempt to use those specific features.

🤖 Prompt for AI Agents
In apps/desktop/src-tauri/src/lib.rs around lines 2700-2703, the setup gating
logic requires microphone and camera permissions alongside screen recording and
accessibility before enabling Continue; confirm with product/design whether mic
and camera should be mandatory at onboarding, and if they are optional change
the gating logic to only require truly mandatory permissions (e.g., screen
recording and accessibility) and defer requesting mic/camera until the user
invokes features that need them; alternatively, if they must remain optional but
still shown, update the UI to indicate they are optional and allow Continue when
required permissions are granted, and implement lazy permission prompts where
mic/camera requests are triggered at feature use.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

elapsed += segDuration;
}
return { index: 0, localTime: 0 };
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Slider at end position shows wrong frame

The currentSegment memo uses strict less-than comparison (time < elapsed + segDuration) which fails when the slider is at the maximum position. When previewTime() equals totalDuration(), the condition time < elapsed + segDuration evaluates to false for all segments, causing the fallback { index: 0, localTime: 0 } to be returned. This displays the first frame of the first segment instead of the last frame of the last segment when the user drags the timeline slider to the end.

Fix in Cursor Fix in Web

@richiemcilroy richiemcilroy merged commit 2528bde into CapSoftware:main Dec 17, 2025
14 of 16 checks passed
@richiemcilroy
Copy link
Member

Awesome work!

@its-thepoe
Copy link
Contributor Author

Thanks!

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.

2 participants