Repository navigation
iOS: run voice-dictation audio activation off the main thread (fix mic-button animation lag) - #6868
Merged
Merged
Conversation
The composer mic button hitched on every press because the whole dictation start/stop path ran synchronously on the @mainactor ComposerDictationController: AVAudioSession.setActive(true) and AVAudioEngine.start() (and their stop counterparts) are blocking audio-hardware calls (~100-300ms each) that froze the button/field animation (issue #6284). Extract a thread-safe ComposerDictationAudioEngine (@unchecked Sendable) that owns the AVAudioEngine + shared AVAudioSession lifecycle on its own serial queue, exposing start(tapBlock:onReady:)/stop() with @sendable callbacks, so the main actor only ever enqueues the work and never blocks on the hardware. The tap block captures the non-Sendable SFSpeechAudioBufferRecognitionRequest via nonisolated(unsafe) (append is thread-safe); the non-Sendable request/recognizer never leave the main actor — the recognition task is created in the main-actor engine-ready callback. Because activation is now asynchronous, a second mic tap / send / navigation during the ~100-300ms spin-up can abandon the start. A monotonic startToken (bumped on every start and teardown) plus the pure, host-testable composerDictationStartDisposition(...) helper let a late engine-ready callback detect it was superseded and discard its result, preventing a double-started engine or leaked tap. teardown()'s off-main stop is serialized after the in-flight start and before any later start on the owner's queue. No user-facing strings changed. The threading fix is iOS-only and not host-testable; the new supersession logic is covered by host unit tests, and the module type-checks against the iOS 26.2 SDK in Swift 6 mode. Fixes #6284 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughDictation now starts audio capture through a dedicated iOS audio engine helper, routes engine-ready callbacks through a token/state disposition helper, and updates controller start, stop, and teardown paths to use the new off-main lifecycle. ChangesDictation audio lifecycle
Sequence Diagram(s)sequenceDiagram
participant ComposerDictationController
participant ComposerDictationAudioEngine
participant AVAudioSession
participant AVAudioEngine
participant composerDictationStartDisposition
ComposerDictationController->>ComposerDictationAudioEngine: start(tapBlock:onReady:)
ComposerDictationAudioEngine->>AVAudioSession: setCategory(.record, .measurement)
ComposerDictationAudioEngine->>AVAudioSession: setActive(true)
ComposerDictationAudioEngine->>AVAudioEngine: installTap() / prepare() / start()
ComposerDictationAudioEngine-->>ComposerDictationController: onReady(true)
ComposerDictationController->>composerDictationStartDisposition: evaluate callbackToken/currentToken/state
composerDictationStartDisposition-->>ComposerDictationController: .apply or .discardStale
ComposerDictationController->>ComposerDictationAudioEngine: stop()
ComposerDictationAudioEngine->>AVAudioEngine: stop() / removeTap()
ComposerDictationAudioEngine->>AVAudioSession: setActive(false, .notifyOthersOnDeactivation)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
✨ 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 |
…dictation-mic-button-has
Contributor
Greptile SummaryThis PR extracts
Confidence Score: 5/5Safe to merge. The threading invariants are sound, the supersession logic is exhaustively unit-tested, and the change is strictly additive to the existing public API surface. The audio engine carve-out correctly confines all blocking hardware calls to a private serial queue; dispatchPrecondition enforces the isolation contract at runtime. The monotonic startToken plus state-gated startDisposition form a correct supersession check: both the token comparison and the requestingPermission state guard must hold before a callback can transition to .listening. FIFO ordering of stop() after start() on the same serial queue eliminates double-start and tap-leak scenarios. The nonisolated(unsafe) weak capture of the non-Sendable request is safe because SFSpeechAudioBufferRecognitionRequest.append(_:) is documented thread-safe. No new user-facing strings, no test seams in production source, no ambient global state. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant U as User (mic tap)
participant C as ComposerDictationController (@MainActor)
participant AE as ComposerDictationAudioEngine (serial queue)
participant SR as SFSpeechRecognizer (arbitrary queue)
U->>C: toggle()
C->>C: "state = .requestingPermission, startToken += 1"
C->>AE: start(tapBlock:onReady:) [async enqueue]
Note over C: returns immediately, no block
AE->>AE: setActive(true), installTap(), engine.start()
AE-->>C: onReady(true) [on audio queue]
C->>C: "Task @MainActor enqueued"
alt "token matches and state == .requestingPermission"
C->>SR: recognitionTask(with: request)
C->>C: "state = .listening"
SR-->>C: "partials -> onText(merged text)"
else superseded by 2nd tap, send, or nav
C->>C: "teardown(): startToken += 1, audioEngine.stop() enqueued"
C->>C: handleEngineReady: discardStale, return
Note over AE: stop() serialized after start() on FIFO queue
end
U->>C: toggle() stop
C->>C: "state = .stopping"
C->>AE: stop() [async enqueue]
AE->>AE: engine.stop(), removeTap(), setActive(false)
SR-->>C: "final result -> finishGraceful(), state = .idle"
%%{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"}}}%%
sequenceDiagram
participant U as User (mic tap)
participant C as ComposerDictationController (@MainActor)
participant AE as ComposerDictationAudioEngine (serial queue)
participant SR as SFSpeechRecognizer (arbitrary queue)
U->>C: toggle()
C->>C: "state = .requestingPermission, startToken += 1"
C->>AE: start(tapBlock:onReady:) [async enqueue]
Note over C: returns immediately, no block
AE->>AE: setActive(true), installTap(), engine.start()
AE-->>C: onReady(true) [on audio queue]
C->>C: "Task @MainActor enqueued"
alt "token matches and state == .requestingPermission"
C->>SR: recognitionTask(with: request)
C->>C: "state = .listening"
SR-->>C: "partials -> onText(merged text)"
else superseded by 2nd tap, send, or nav
C->>C: "teardown(): startToken += 1, audioEngine.stop() enqueued"
C->>C: handleEngineReady: discardStale, return
Note over AE: stop() serialized after start() on FIFO queue
end
U->>C: toggle() stop
C->>C: "state = .stopping"
C->>AE: stop() [async enqueue]
AE->>AE: engine.stop(), removeTap(), setActive(false)
SR-->>C: "final result -> finishGraceful(), state = .idle"
Reviews (5): Last reviewed commit: "iOS: satisfy Aziz policy checks for dict..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In
`@Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationController.swift`:
- Around line 398-400: The closure in ComposerDictationController.swift uses an
invalid weak reference declaration (`weak let weakRequest`), which prevents
compilation. Update the `weakRequest` binding to use a weak variable form
compatible with ARC, and keep the `nonisolated(unsafe)` capture around the
`SFSpeechAudioBufferRecognitionRequest` reference. Verify the `return { buffer,
_ in ... }` closure still appends only when the request is alive and that the
symbol `weakRequest` remains the single capture point.
- Around line 235-236: The request is being ended before the audio input tap is
fully removed, so the tap can still append audio after shutdown starts. Update
the stop/teardown flow in ComposerDictationController (the code around
request?.endAudio() and audioEngine.stop()) so endAudio() happens only after
removeTap has completed, or guard the tap callback before ending the request.
Use the existing request, audioEngine, and tap-removal sequence in
ComposerDictationController to keep the shutdown ordering correct.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e22a469-6e88-4e73-8118-8ab6e4d10c54
📒 Files selected for processing (4)
Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationAudioEngine.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationController.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationTextMerger.swiftPackages/iOS/CmuxMobileSupport/Tests/CmuxMobileSupportTests/ComposerDictationTests.swift
|
Found 1 test failure on Blacksmith runners: Failure
|
Making engine activation asynchronous (issue #6284) opened a 100-300ms editable window: in the already-authorized path the controller now stays in `.requestingPermission` while the engine spins up off-main, and that state did not set `locksComposerField`. Text typed in that window is not in the captured `baseText`, so the first speech partial (base + transcript) overwrote it. The previous synchronous start reached `.listening` before returning, so the field locked immediately. Lock the field from `.requestingPermission` through `.listening` and `.stopping`, restoring the original instant lock. During the genuine first-ever auth-pending flavor the system permission alert is modal, so the field is not interactable anyway and the lock is harmless. Found by structured review on PR #6868. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The iOS package-conventions lint (`free-function` rule) requires functionality to be scoped to a type, not a top-level free function. Move `composerDictationStartDisposition(callbackToken:currentToken:state:)` to an instance method `ComposerDictationState.startDisposition(callbackToken:currentToken:)`, alongside the enum's existing `isListening`/`locksComposerField` accessors. Behavior is unchanged; the result enum keeps its cases (not a namespace type), and tests/call site use the method form. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Structured review flagged the new owner's serial DispatchQueue + @unchecked Sendable as a manual-synchronization island. Keep the queue (it is the right tool, not an actor) but make the rationale and the invariant explicit: - Document why an actor is wrong here: setActive/engine.start/engine.stop are synchronous ~100-300ms blocking hardware calls that would block a cooperative-pool thread on an actor; and the supersession invariant needs stop() enqueued synchronously, in deterministic FIFO order, from the @mainactor controller's sync path — which a serial DispatchQueue gives and a cross-actor `await` (Task { await … }) does not. This mirrors the established AVFoundation-session-on-a-serial-queue pattern already used for capture in QRCodeCaptureController. - Self-enforce the isolation contract with dispatchPrecondition(.onQueue(queue)) in teardownLocked(), so a future off-queue caller traps loudly instead of silently racing — directly addressing the "safety depends on remembering to hop through the queue" concern. Carve-out marked lint:allow serial-audio-queue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three cmux-policy-check findings on the dictation diff: - Add a nearby safety-argument comment at the `nonisolated(unsafe)` tap capture in makeTapBlock (the doc comment was >3 lines away). - Move the added `ComposerDictationStartDisposition` enum + its `ComposerDictationState.startDisposition` extension into their own file (one major type per file), restoring ComposerDictationTextMerger.swift to its two pre-existing types. - Make `makeTapBlock` a `nonisolated` instance method instead of a `static` one (it uses no `self`), mirroring `makeRecognitionResultHandler` and clearing the static-as-namespace heuristic — the enclosing controller is heavily stateful, so the static form was a false positive. Behavior unchanged; host tests, iOS type-check, lint, and budget all pass. 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.
Fixes #6284
Problem
On iOS, tapping the composer mic button to start/stop voice dictation produced a visible animation hitch. The whole dictation start/stop path ran synchronously on the
@MainActorComposerDictationController:AVAudioSession.setActive(true)andAVAudioEngine.start()(and the symmetricengine.stop()+setActive(false)on stop) are synchronous audio-hardware calls that block the caller ~100-300ms each, freezing the mic button / field animation on every press.The crash and text-loss bugs were already fixed separately; this PR tackles the remaining animation lag, via the architectural change the issue proposed.
Fix
Extract a thread-safe
ComposerDictationAudioEngine(@unchecked Sendable) that owns theAVAudioEngine+ sharedAVAudioSessionlifecycle on its own serial queue, exposingstart(tapBlock:onReady:)/stop()with@Sendablecallbacks. The main actor now only enqueues the activation/teardown — it never blocks on the audio hardware.SFSpeechAudioBufferRecognitionRequestvianonisolated(unsafe)(itsappendis thread-safe), so it can cross into the owner's queue.request/recognizernever leave the main actor — the recognition task is created in the main-actor engine-ready callback (handleEngineReady), sidestepping the Swift 6 region-isolation errors the issue notes a naiveTask.detachedhits.Supersession (new async window)
Because activation is now asynchronous, a second mic tap / send / navigation during the ~100-300ms spin-up can abandon the start. A monotonic
startToken(bumped on every start and every teardown) plus a pure, host-testable helpercomposerDictationStartDisposition(callbackToken:currentToken:state:)let a late engine-ready callback detect it was superseded and discard its result — preventing a double-started engine or a leaked input tap.teardown()'s off-mainstop()is serialized on the owner's queue after the in-flight start's activation and before any later start, so the engine is reliably torn down and never double-started.Files
ComposerDictationAudioEngine.swift(new) — off-main@unchecked Sendableaudio engine owner.ComposerDictationController.swift— delegate activation/teardown to the owner; addstartToken+handleEngineReady; dropdidActivateSession/stopEngineAndSession(now internal to the owner).ComposerDictationTextMerger.swift— add the purecomposerDictationStartDispositionsupersession helper (host-compilable, no Speech/AVFoundation).ComposerDictationTests.swift— unit tests for the supersession rule.Testing
startDisposition*tests covering the supersession partition (token match × state).CmuxMobileSupportmodule type-checks against the iOS 26.2 SDK in Swift 6 language mode with no new errors/warnings (the@unchecked Sendableowner, the@Sendableclosures crossing the queue, and thenonisolated(unsafe)capture all compile).AVAudioEngine, requires a device + mic permission, timing-flaky), so it's verified by behavior on-device rather than an automated regression test; the new supersession logic that the async change introduces is unit-tested.toggle/stop/cancel/state/isAvailable/locksComposerFieldare unchanged, so the other consumer (ChatComposerView) is unaffected.🤖 Generated with Claude Code
Summary by cubic
Runs voice dictation audio activation off the main thread to eliminate mic-button animation hitches, and locks the composer field during engine spin-up to prevent edit loss. Fixes #6284.
Bug Fixes
AVAudioSession.setActiveandAVAudioEngine.start/stopto a background serial queue to remove 100–300ms UI freezes when tapping the mic..requestingPermissionthrough.listening/.stoppingto avoid overwriting text typed during async engine spin-up.Refactors
ComposerDictationAudioEnginewithstart(tapBlock:onReady:)andstop(), and updated the controller to create the recognition task on engine-ready (main actor).ComposerDictationState.startDisposition(callbackToken:currentToken:); added unit tests for this rule and the field-lock behavior.dispatchPrecondition(.onQueue(...))in teardown to guard isolation.ComposerDictationStartDisposition.swift, mademakeTapBlockanonisolatedinstance method, and added a nearby safety note for thenonisolated(unsafe)capture (no behavior change).Written for commit c3e95f5. Summary will update on new commits.
Summary by CodeRabbit