refactor: decompose TranscriptionEngine god object into focused managers#6
Closed
Newarr wants to merge 10 commits intorefactor/logging-migration-stackedfrom
Closed
Conversation
added 10 commits
March 29, 2026 23:41
…iptionStreamCoordinator Extract three managers from TranscriptionEngine god object: - ModelDownloadManager: model download, loading, progress tracking - DeviceRoutingManager: CoreAudio device listeners, restart coordination - TranscriptionStreamCoordinator: audio stream capture, transcriber orchestration Key fixes from code review: - Centralized DownloadProgressDetail in ModelDownloadManager - Added storeRestartState() for proper restart state management - Added finalizeMicStream/finalizeSystemStream with draining - Fixed Sendable closure isolation with @mainactor annotations - Removed Sendable conformance from @mainactor classes - Fixed clearCache() to use .info log level - Added deinit cleanup for CoreAudio listeners
…view - Add missing import FluidAudio to DeviceRoutingManager - Add Utils/Logging.swift for centralized Log definitions - Clear restart state in stopListening() to prevent memory leaks (VadManager, backends now properly released on session end)
- Add diarizationTask property to track diarization audio feed - Cancel and await diarization task in stop/finalize paths - Check Task.isCancelled in diarization loop to exit promptly - Prevent old audio from draining into diarizer after session stop
Change onMicRestartRequested and onSystemRestartRequested from synchronous to async closures to prevent overlapping restarts when the engine responds with async work. - Change callback types to (@sendable ...) async -> Void - Await callbacks in performMicRestart() and performSystemAudioRestart() - Fix deinit to dispatch cleanup to MainActor Fixes fire-and-forget callback issue (lines 10, 117, 139, 276, 285)
Add bounded buffering to AsyncStream to prevent unbounded memory growth when transcription/diarization falls behind. Add proper error propagation with user-facing error messages. - Use .bufferingNewest(1) for all AsyncStream.makeStream() calls - Add TranscriptionStreamError enum with localized error descriptions - Change startMicStream/startSystemStream to return Result type - Provide user-friendly error messages for common failure modes Fixes unbounded buffering (lines 153, 316) and error propagation issues
Fix stale state dependency where loadModel() trusted the cached needsDownload flag instead of recomputing from the model argument. If the caller changed models between calls, the manager misreported state. - Recompute availability from model parameter at top of loadModel() - Rename local variable to avoid shadowing property Fixes stale state dependency (lines 49, 77)
Remove the duplicate DownloadProgressDetail struct from TranscriptionEngine. The type is now defined in ModelDownloadManager and shared across the module. Fixes invalid redeclaration compilation error.
- Remove nonisolated(unsafe) from diarization task (capture dm properly) - Remove @unchecked Sendable Box struct from tappedStream - Make MicCapture and SystemAudioCapture injectable via init All unsafe concurrency patterns have been removed.
- TranscriptionEngine reduced from 949 to 551 lines (42% reduction) - Delegates to ModelDownloadManager for model lifecycle - Delegates to DeviceRoutingManager for device listeners/restarts - Delegates to TranscriptionStreamCoordinator for stream orchestration - Public API preserved for SwiftUI compatibility
Critical: - Wire ModelDownloadManager progress callbacks to engine UI state - Reset downloadConfirmed/needsModelDownload on model load failure - Remove broken deinit (weak self always nil), document stopListening contract High: - Use shared static queue for CoreAudio listener add/remove calls - Replace 4 force unwraps with guard let - Remove duplicate micTask/sysTask ownership (coordinator is sole owner) - Use bufferingOldest(128) instead of bufferingNewest(1) to prevent audio drops Medium: - Nil diarizationTask synchronously in stopSystemStream to prevent race - Surface mic health check failure to user via lastError callback - Remove dead clearSystemAudioErrorIfPresent, inline at restart success Concurrency: - Extract data from buffer before yielding to diarization continuation - Use nonisolated(unsafe) for read-only AVAudioPCMBuffer across send boundaries - Wrap tappedStream iteration in Sendable TapForwarder struct
4e72249 to
d6bfcba
Compare
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
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
Decomposes the 949-line
TranscriptionEnginegod object into 3 focused, testable managers:ModelDownloadManagerDeviceRoutingManagerTranscriptionStreamCoordinatorResult
Architecture
Key Changes
New Files
ModelDownloadManager.swift- Extracted model loading/download logicDeviceRoutingManager.swift- Extracted CoreAudio device listenersTranscriptionStreamCoordinator.swift- Extracted stream orchestrationUtils/Logging.swift- Centralized Log enum (from refactor: migrate diagLog to unified os.Logger #5)TranscriptionEngine Simplification
ModelDownloadManagerDeviceRoutingManagerwith async callbacksTranscriptionStreamCoordinator@Observableproperties for SwiftUI bindingConcurrency Fixes (from code review)
.bufferingNewest(1)to AsyncStreamsnonisolated(unsafe)patterns in favor of proper SendableTesting
swift buildcompiles with Swift 6 strict concurrencyMigration Path
This is a pure refactoring with no behavioral changes. The public API remains: