Skip to content

fix(recording): defer media pause and volume mute until hold threshold#240

Open
mml555 wants to merge 2 commits into
kitlangton:mainfrom
mml555:fix/recording-media-volume-flicker
Open

fix(recording): defer media pause and volume mute until hold threshold#240
mml555 wants to merge 2 commits into
kitlangton:mainfrom
mml555:fix/recording-media-volume-flicker

Conversation

@mml555

@mml555 mml555 commented Jun 7, 2026

Copy link
Copy Markdown

Summary

  • Defers Pause media and Mute system volume until the hotkey hold crosses the minimum threshold, preventing pause/play and volume flicker on brief or accidental presses
  • Debounces media/volume resume on stop so rapid stop/start does not spam media keys
  • Ignores duplicate startRecording while capture is already active
  • Restores prior media/volume state immediately when starting a new recording if controls are still active

Fixes #239

Context

These fixes were developed while testing live dictation preview (#237 / #238) and are split out so they can ship independently.

Test plan

  • Enable Pause media, tap hotkey briefly (< minimum hold): music should keep playing
  • Hold hotkey past threshold: media pauses; release: media resumes once after ~200ms debounce
  • Enable Mute system volume, repeat brief tap vs sustained hold
  • Rapid double-tap hotkey: no stacked pause/play or mute/unmute flicker
  • Modifier-only hotkey (e.g. Option): defer uses modifier-only minimum (0.3s) even if user minimum is lower

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Media pause and volume mute are now deferred until the recording hold threshold is reached, preventing premature media changes.
    • Recording start is blocked when a session is already recording or transcribing, avoiding duplicate starts.
    • Media control state transitions made more reliable with debounced scheduling, reducing jitter and unexpected resume/pause behavior.

Pause/mute now waits until the hotkey hold crosses the minimum threshold,
with debounced resume and duplicate-start guards to stop flicker on brief
or accidental presses.

Fixes kitlangton#239

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e4ee9b0f-9adb-4f41-8e86-980a05db98b0

📥 Commits

Reviewing files that changed from the base of the PR and between bacefdd and 1a27b34.

📒 Files selected for processing (1)
  • Hex/Clients/RecordingClient.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Hex/Clients/RecordingClient.swift

📝 Walkthrough

Walkthrough

RecordingClient defers pause/mute until an activation delay, debounces resume actions, and centralizes media-control helpers. startRecording cancels pending resume tasks and schedules media control; stopRecording debounces resumption. TranscriptionFeature prevents starting when already recording/transcribing.

Changes

Media Control Debouncing and Recording Safety

Layer / File(s) Summary
Media control state infrastructure
Hex/Clients/RecordingClient.swift
Adds resumeMediaTask, resumeMediaDebounce, computed helpers (hasActiveMediaControlState, isCaptureActive) and mediaControlActivationDelay() to centralize media-control state and timing.
Media control and resumption helpers
Hex/Clients/RecordingClient.swift
Introduces scheduleMediaControl, applyPauseMedia, applyMute, scheduleResumeMediaIfNeeded, and resumeMediaImmediately; updates cleanup() to call immediate resume.
Refactored recording start with deferred media control
Hex/Clients/RecordingClient.swift
startRecording() cancels pending resume work, waits briefly if capture already active, resumes immediately when media-control state exists, or schedules deferred pause/mute via scheduleMediaControl.
Refactored recording stop with scheduled resumption
Hex/Clients/RecordingClient.swift
Both capture-engine and recorder-fallback stop paths call scheduleResumeMediaIfNeeded() instead of awaiting resume directly to debounce resumption.
Transcription feature recording start guard
Hex/Features/Transcription/TranscriptionFeature.swift
handleStartRecording now returns .none immediately if state.isRecording or state.isTranscribing is true.
Changeset documentation
.changeset/03f45a3f.md
Patch release note documenting that media pause and volume mute are deferred until a recording hold threshold is reached.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related issues

Possibly related PRs

  • kitlangton/Hex#100: Related changes to recording-start control flow in TranscriptionFeature.
  • kitlangton/Hex#235: Related media pause/mute and resume scheduling refactors in RecordingClient.

Poem

🐰 I nudge the pause with patient paws,
Debounce the hum of media laws,
No twin starts to spoil the scene,
Resumes that wait—soft, calm, and clean,
A hop, a whisper, capture serene.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly and accurately describes the main change: deferring media pause and volume mute until the recording hold threshold is reached, which is the core objective of the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bacefdd9fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Hex/Clients/RecordingClient.swift Outdated
Comment on lines +1055 to +1057
if isCaptureActive {
recordingLogger.notice("Ignoring duplicate startRecording while capture is active")
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve new session IDs during overlapping stop/start

When a user starts another dictation immediately after releasing the hotkey, stopRecording() can still be in its capture-engine grace sleep with captureController.isRecording == true. This guard returns before assigning a fresh recordingSessionID, so the in-flight stop does not trip shouldIgnoreStopRequest and proceeds to finish the old capture, while the UI has already entered a new recording with no underlying capture; the next release then yields an ignored/stale recording. This affects rapid successive recordings/double-tap workflows where start arrives during the 20–80 ms stop grace window.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

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

⚠️ Outside diff range comments (1)
Hex/Clients/RecordingClient.swift (1)

1287-1321: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

cleanup() still references a removed helper and will not compile.

After this refactor, resumeMediaIfNeeded() is no longer defined, but it is still called at Line 1504.

🛠️ Suggested fix
+  private func resumeMediaIfNeeded() async {
+    guard hasActiveMediaControlState else { return }
+    resumeMediaTask?.cancel()
+    resumeMediaTask = nil
+    await resumeMediaImmediately()
+  }
+
   private func resumeMediaImmediately() async {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Hex/Clients/RecordingClient.swift` around lines 1287 - 1321, The cleanup()
function still calls the removed resumeMediaIfNeeded; update cleanup() to call
the new resumeMediaImmediately() instead and ensure it's awaited from the
correct concurrency context (e.g., await resumeMediaImmediately() or Task {
await resumeMediaImmediately() } / await MainActor.run if cleanup() is not async
or runs on MainActor). Locate the call to resumeMediaIfNeeded in cleanup() and
replace it with an awaited call to resumeMediaImmediately(), making any
necessary signature/context adjustments so the code compiles.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@Hex/Clients/RecordingClient.swift`:
- Around line 1287-1321: The cleanup() function still calls the removed
resumeMediaIfNeeded; update cleanup() to call the new resumeMediaImmediately()
instead and ensure it's awaited from the correct concurrency context (e.g.,
await resumeMediaImmediately() or Task { await resumeMediaImmediately() } /
await MainActor.run if cleanup() is not async or runs on MainActor). Locate the
call to resumeMediaIfNeeded in cleanup() and replace it with an awaited call to
resumeMediaImmediately(), making any necessary signature/context adjustments so
the code compiles.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bba96e7d-6398-4bf6-a420-1a7d51248751

📥 Commits

Reviewing files that changed from the base of the PR and between 71878b7 and bacefdd.

📒 Files selected for processing (3)
  • .changeset/03f45a3f.md
  • Hex/Clients/RecordingClient.swift
  • Hex/Features/Transcription/TranscriptionFeature.swift

Assign session IDs before stop-grace overlap waits and fix cleanup to
call resumeMediaImmediately after the media defer refactor.

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

Media pause and system volume flicker on brief or accidental hotkey presses

1 participant