iOS: stop pausing background music on text submit - #6290
Merged
Merged
Conversation
The composer's send() (and field-blur / terminal-switch) unconditionally call dictation.cancel() -> teardown() -> stopEngineAndSession(), which accessed audioEngine.inputNode.removeTap(...) and called AVAudioSession.setActive(false, .notifyOthersOnDeactivation) every time, even when voice dictation was never started. Touching the engine input node powers up the mic input route and deactivating the shared session interrupts other apps' audio, so the user's background music paused and resumed on every text submit. Track whether beginRecognition() actually activated the session (didActivateSession) and make stopEngineAndSession() a true no-op otherwise, so a send with no dictation in flight never touches the audio system. A session we did activate is still torn down exactly as before, including on the failed-start path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesAudio Session Ownership Guard
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR fixes a background-music interruption bug on iOS where submitting text in the terminal composer momentarily paused Spotify/Apple Music. The root cause was
Confidence Score: 5/5Safe to merge — the change is tightly scoped to a single, well-understood resource-ownership invariant and does not alter any active-dictation code path. The flag accurately models 'this controller owns the AVAudioSession': it is set only after No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["send / blur / cancel"] --> B["cancel()"]
B --> C["teardown()"]
C --> D["stopEngineAndSession()"]
D --> E{"didActivateSession?"}
E -->|No| F["return early - no audio system access"]
E -->|Yes| G["didActivateSession = false"]
G --> H["audioEngine.stop()"]
H --> I["inputNode.removeTap"]
I --> J["AVAudioSession.setActive false"]
K["beginRecognition()"] --> L["setCategory + setActive true"]
L --> M["didActivateSession = true"]
M --> N["installTap + engine.start"]
N --> O["state = .listening"]
O --> P["stop() or cancel() later"]
P --> D
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["send / blur / cancel"] --> B["cancel()"]
B --> C["teardown()"]
C --> D["stopEngineAndSession()"]
D --> E{"didActivateSession?"}
E -->|No| F["return early - no audio system access"]
E -->|Yes| G["didActivateSession = false"]
G --> H["audioEngine.stop()"]
H --> I["inputNode.removeTap"]
I --> J["AVAudioSession.setActive false"]
K["beginRecognition()"] --> L["setCategory + setActive true"]
L --> M["didActivateSession = true"]
M --> N["installTap + engine.start"]
N --> O["state = .listening"]
O --> P["stop() or cancel() later"]
P --> D
Reviews (2): Last reviewed commit: "Trim comments to stay under the 500-line..." | Re-trigger Greptile |
Condense the didActivateSession doc/comments added in the prior commit so ComposerDictationController.swift stays under the workflow-guard-tests 500-line threshold (499 lines). No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
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.
Problem
On iOS, submitting text in the terminal composer momentarily paused (then resumed) the user's background music (Spotify/Apple Music). The composer's
send()was interacting with the microphone/audio session on every submit even when voice dictation was never used.Root cause
send()(and field blur / terminal switch) calldictation.cancel()unconditionally. The send-path comment even claimedcancel()is "a no-op" when idle, but it isn't:cancel()→teardown()→stopEngineAndSession(), which always:audioEngine.inputNode.removeTap(onBus: 0)— touchingAVAudioEngine.inputNodepowers up the mic input route, andAVAudioSession.setActive(false, options: .notifyOthersOnDeactivation)on the shared session.Both poke the system audio session on every submit, which interrupts other apps' playback. Hence music pausing + resuming each time you hit send.
Fix
Track whether
beginRecognition()actually activated the session (didActivateSession, set right aftersetActive(true)succeeds) and makestopEngineAndSession()a true no-op when we never activated it. A send with no dictation in flight now never touches the audio system. A session we did activate is torn down exactly as before, including the failed-start path (the flag is set before any post-setActivesetup that could throw).Principled fix: it removes the entire class of "teardown touches audio when nothing was started" rather than special-casing the send path. Only deactivate a session we own.
Test
ComposerDictationControlleris iOS-only (#if os(iOS), AVFoundation/Speech) and not host-compilable, so the existing host tests cover only the pure text-merger and state machine; there is no host seam for the AVFoundation teardown. A meaningful automated test would require a large audio-session injection seam, so this is verified by on-device dogfood: with background music playing, submit text repeatedly in the iOS composer and confirm the music keeps playing uninterrupted, and that voice dictation still works.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Small, localized change to iOS dictation teardown; behavior when dictation actually ran is unchanged, with lower risk of unintended audio side effects when idle.
Overview
Fixes background music briefly pausing on every composer send when voice dictation was never used.
send(), blur, and terminal switches calldictation.cancel(), which always reachedstopEngineAndSession()and touchedAVAudioEngine.inputNodeplusAVAudioSession.setActive(false, .notifyOthersOnDeactivation)even in idle state.ComposerDictationControllernow tracksdidActivateSession(set right after a successfulsetActive(true)inbeginRecognition()) andstopEngineAndSession()returns immediately when that flag is false, so teardown only runs for a session this controller actually activated. Active dictation and failed-start cleanup still deactivate as before.Reviewed by Cursor Bugbot for commit bd6ed10. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Stop pausing background music on iOS when sending text in the composer. We now only touch the
AVAudioSessionif voice dictation actually started.Bug Fixes
didActivateSessionaftersetActive(true)and gatestopEngineAndSession()to no-op unless we activated the session, so sends/blur/switch without dictation no longer interrupt other apps’ audio. Teardown is unchanged when dictation runs.Refactors
ComposerDictationController.swiftunder the 500-line limit. No behavior change.Written for commit bd6ed10. Summary will update on new commits.
Summary by CodeRabbit