[codex] Fix cursor sync after recording pause#399
Merged
webadderall merged 2 commits intomainfrom May 1, 2026
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds renderer-exposed pause/resume cursor-capture APIs and implements pause-aware cursor telemetry: new IPC handlers, state fields, elapsed-time calculation that excludes paused intervals, sampling short-circuits when paused, and integration into recording pause/resume flows with tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer (Hook/UI)
participant Preload as Preload (contextBridge)
participant Main as Main Process (ipcMain)
participant Telemetry as Telemetry Module
participant State as IPC State
Renderer->>Preload: pauseCursorCapture(boundaryMs?)
Preload->>Main: invoke('pause-cursor-capture', boundaryMs?)
Main->>Telemetry: sampleCursorPoint(boundaryMs?) %% sample at boundary
Main->>Telemetry: pauseCursorCapture(pausedAtMs)
Telemetry->>State: setCursorCapturePauseStartedAtMs(pausedAtMs)
Telemetry->>State: setCursorCaptureAccumulatedPausedMs(updated)
Telemetry-->>Main: { success: true }
Main-->>Preload: result
Preload-->>Renderer: Promise resolves
Renderer->>Preload: resumeCursorCapture(boundaryMs?)
Preload->>Main: invoke('resume-cursor-capture', boundaryMs?)
Main->>Telemetry: resumeCursorCapture(resumedAtMs)
Telemetry->>State: apply accumulated paused duration
Main->>Telemetry: sampleCursorPoint(afterResumeMs)
Telemetry-->>Main: { success: true }
Main-->>Preload: result
Preload-->>Renderer: Promise resolves
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useScreenRecorder.ts (1)
1430-1466:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftKeep the recorder clock and cursor clock on the same pause boundary.
These branches now record
markRecordingPaused/Resumed(Date.now())only after awaiting the cursor IPC, sogetRecordingDurationMs()and the WindowspauseSegmentspick up IPC latency instead of the actual recorder pause/resume edge. Thecatchblocks also still flip local pause state after a cursor IPC failure, which reintroduces cursor/video desync on the error path. Capture a singleboundaryMsat the moment the recorder pauses/resumes, pass that through to cursor capture, and only commit local pause state if the cursor transition succeeds (or roll the recorder transition back).Also applies to: 1473-1509
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` around lines 1430 - 1466, When pausing/resuming, capture a single boundaryMs (const boundaryMs = Date.now()) immediately at the moment you perform the recorder transition (e.g. right after calling mediaRecorder.current.pause()/resume() and webcamRecorder.current.pause()/resume(), or after successful window.electronAPI.pauseNativeScreenRecording()/resumeNativeScreenRecording()), then pass that boundaryMs into the cursor IPC (window.electronAPI.pauseCursorCapture(boundaryMs) / resumeCursorCapture(boundaryMs)). Only call markRecordingPaused(boundaryMs)/markRecordingResumed(boundaryMs) and setPaused(...) after the cursor IPC resolves successfully; if the cursor IPC fails, roll the recorder/native transition back (call mediaRecorder.current.resume()/pause() and webcamRecorder.current.resume()/pause() as needed and, if you called pauseNativeScreenRecording()/resumeNativeScreenRecording(), call the opposite native API to revert) and log the cursor error. Ensure this change is applied in the blocks that call pauseNativeScreenRecording, pauseCursorCapture, mediaRecorder.current, webcamRecorder.current, markRecordingPaused, markRecordingResumed, and setPaused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 1430-1466: When pausing/resuming, capture a single boundaryMs
(const boundaryMs = Date.now()) immediately at the moment you perform the
recorder transition (e.g. right after calling
mediaRecorder.current.pause()/resume() and
webcamRecorder.current.pause()/resume(), or after successful
window.electronAPI.pauseNativeScreenRecording()/resumeNativeScreenRecording()),
then pass that boundaryMs into the cursor IPC
(window.electronAPI.pauseCursorCapture(boundaryMs) /
resumeCursorCapture(boundaryMs)). Only call
markRecordingPaused(boundaryMs)/markRecordingResumed(boundaryMs) and
setPaused(...) after the cursor IPC resolves successfully; if the cursor IPC
fails, roll the recorder/native transition back (call
mediaRecorder.current.resume()/pause() and
webcamRecorder.current.resume()/pause() as needed and, if you called
pauseNativeScreenRecording()/resumeNativeScreenRecording(), call the opposite
native API to revert) and log the cursor error. Ensure this change is applied in
the blocks that call pauseNativeScreenRecording, pauseCursorCapture,
mediaRecorder.current, webcamRecorder.current, markRecordingPaused,
markRecordingResumed, and setPaused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 87986be3-cc3b-4f80-a401-f5491ebe9633
📒 Files selected for processing (8)
electron/electron-env.d.tselectron/ipc/cursor/interaction.tselectron/ipc/cursor/telemetry.test.tselectron/ipc/cursor/telemetry.tselectron/ipc/register/recording.tselectron/ipc/state.tselectron/preload.tssrc/hooks/useScreenRecorder.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Fix cursor telemetry timing so pausing and resuming a recording no longer desynchronizes the cursor in the editor.
Why
The video timeline already removes paused time, but cursor telemetry was still being timestamped against wall-clock elapsed time. After a pause/resume boundary, cursor events drifted later than the recorded video.
Root cause
Cursor sampling and cursor interaction events continued to measure elapsed time from the original recording start without subtracting paused duration.
Implementation
useScreenRecorderso the branch typechecks cleanlyImpact
Recordings that include pauses should keep cursor motion and click effects aligned with the video timeline in the editor after resume.
Validation
npx tsc -p tsconfig.json --noEmitnpx vitest --run electron/ipc/cursor/telemetry.test.ts src/hooks/useScreenRecorder.test.tsSummary by CodeRabbit