Add Parallel Mode transcription with Settings UI toggle and ~/Documents model discovery - #273
gauravsaini wants to merge 1 commit into
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughChangesParallel transcription is now persisted as a setting, configurable from the General settings UI, and dispatched through WhisperKit using 30-second audio chunks. Model discovery checks additional local directories and tolerates remote failures. Settings serialization and model matching receive coverage. Transcription settings and execution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant GeneralSectionView
participant SettingsFeature
participant TranscriptionFeature
participant TranscriptionClientLive
participant WhisperKit
User->>GeneralSectionView: Enable Parallel Mode
GeneralSectionView->>SettingsFeature: toggleParallelMode(true)
SettingsFeature-->>GeneralSectionView: Store .parallel mode
TranscriptionFeature->>TranscriptionClientLive: transcribe with concurrentWorkerCount=8
TranscriptionClientLive->>WhisperKit: Transcribe 30-second audio chunks
WhisperKit-->>TranscriptionClientLive: Chunk text results
TranscriptionClientLive-->>TranscriptionFeature: Concatenated transcription
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
…ts model discovery - Add Parallel Mode chunked decoding in TranscriptionClient using WhisperKit batch processing across Apple Silicon cores. - Add Parallel Mode setting toggle to GeneralSectionView and wire toggleParallelMode action in SettingsFeature. - Add multi-candidate path resolution in TranscriptionClient to scan ~/Documents and subfolders for custom/offline models. - Fix concurrentWorkerCount default to 0 when Parallel Mode is disabled to execute sequential transcription. - Add public log privacy annotations for performance duration and realtime speed metrics. - Add ParallelModeAndSettingsTests unit test suite for HexSettings serialization and model pattern matching. - Include patch changeset fragment.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Hex/Clients/TranscriptionClient.swift (1)
461-498: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftSplit model discovery from managed model paths.
modelPath(for:)resolves user-supplied Documents paths and is used bydeleteModel,downloadModelIfNeeded,loadWhisperKitModel, andtokenizerPath. Withcom.apple.security.files.user-selected.read-write, deleting an existing user folder or creating/moving a new model download into that folder can mutate user-supplied model locations. Keep discovery read-only and only create/delete inmodelsBaseFolder.🤖 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/TranscriptionClient.swift` around lines 461 - 498, Separate discovery from managed model storage: keep modelCandidatePaths(for:) for read-only lookup, but change modelPath(for:) to resolve only under modelsBaseFolder using the sanitized variant and never return Documents candidates. Update deleteModel, downloadModelIfNeeded, loadWhisperKitModel, and tokenizerPath to use the appropriate discovery or managed-path behavior so user-supplied Documents folders are only read, while creation, download moves, and deletion occur exclusively in modelsBaseFolder.
🤖 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 `@Hex/Clients/TranscriptionClient.swift`:
- Around line 418-430: Replace the custom linear-interpolation implementation in
resample(_:from:to:) with AVAudioConverter-based resampling configured for the
target rate and existing channel/format requirements, so downsampling applies
proper anti-alias filtering. Preserve the method’s [Float] input/output contract
and target-length behavior while removing the per-sample interpolation loop.
- Around line 302-313: Change the file-name interpolations in the two
transcription notices around transcribeParallel and the standard WhisperKit
transcription path from privacy: .public to privacy: .private, while keeping
model and timing values public. Also update capturedURL.lastPathComponent in
TranscriptionFeature.swift at line 417 to .private, leaving result.count public.
- Around line 336-346: Update the chunking logic around chunkSec and the offset
loop to avoid hard, non-overlapping 30-second boundaries: add a small overlap
between adjacent chunks and deduplicate overlapping transcription results at
each seam, or split chunks using VAD-detected silence. Before exposing this as a
user-facing toggle, measure and validate the resulting transcription accuracy
and preserve the advertised performance behavior.
- Around line 204-244: The getAvailableModels discovery is too broad, hides
remote failures, and performs expensive unscoped Documents scanning. In
getAvailableModels, preserve the throwing contract by handling
WhisperKit.fetchAvailableModels errors with at least an error log, remove the
docsURL root from scanDirectories, and restrict scanning to customModelDirectory
plus the known Hugging Face/model layouts. Tighten the artifact predicate so
generic names containing “model” do not qualify as transcription models, while
preserving valid model and tokenizer/config detection.
- Around line 181-194: tighten the model completeness checks in
isModelDownloaded: replace broad name matching with actual expected artifacts,
requiring an .mlmodelc directory and a tokenizer directory or config.json, while
avoiding stray files and partial-download artifacts. In the existing directory
enumeration catch block, log the caught error at debug level before continuing
so permission failures remain diagnosable.
- Around line 400-414: In the audio sample conversion logic, explicitly validate
that floatChannelData or int16ChannelData is non-nil before constructing the
corresponding UnsafeBufferPointer; throw the existing unsupported-format error
when channel data is unavailable instead of passing a nil start pointer with a
nonzero count. Apply this to both branches while preserving the existing
resampling behavior.
- Around line 297-299: Update the transcription flow around audioDuration(of:)
so the logging-only duration lookup is non-throwing and cannot prevent
WhisperKit from processing supported formats; use an optional or fallback result
while preserving transcription behavior when duration lookup fails. Adjust the
realtime-factor logging guard to handle audioLen == 0 safely and avoid invalid
calculations.
- Around line 366-378: Update the result aggregation loop in TranscriptionClient
so chunk failures are not silently represented as empty transcript text. After
tracking successCount and failures, surface the failure to the caller—at minimum
throw when successCount is zero, or when the failure ratio exceeds the
established threshold—so the caller can fall back to the sequential
transcription path.
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Around line 411-413: Update the transcription options construction in the
sequential/parallel mode flow so disabling parallel mode explicitly configures
WhisperKit for sequential processing rather than passing concurrentWorkerCount
as 0. For parallel mode, derive the worker cap from available system CPU and
memory resources instead of hardcoding 8, while preserving the existing .vad
chunking strategy and TranscriptionClient’s parallel-mode detection.
In `@HexCore/Sources/HexCore/Settings/HexSettings.swift`:
- Around line 312-319: Update TranscriptionClientLive to use
HexSettings.customModelDirectory as the base for model lookup, download, and
deletion paths instead of always relying on Application Support/Documents
fallbacks. Ensure the configured directory is honored when present, while
preserving the existing fallback behavior when it is unset; otherwise remove the
customModelDirectory SettingsField and its persistence wiring.
---
Outside diff comments:
In `@Hex/Clients/TranscriptionClient.swift`:
- Around line 461-498: Separate discovery from managed model storage: keep
modelCandidatePaths(for:) for read-only lookup, but change modelPath(for:) to
resolve only under modelsBaseFolder using the sanitized variant and never return
Documents candidates. Update deleteModel, downloadModelIfNeeded,
loadWhisperKitModel, and tokenizerPath to use the appropriate discovery or
managed-path behavior so user-supplied Documents folders are only read, while
creation, download moves, and deletion occur exclusively in modelsBaseFolder.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89afc566-87cd-459f-afec-a36d1b0e07b0
📒 Files selected for processing (7)
Hex/Clients/TranscriptionClient.swiftHex/Features/Settings/GeneralSectionView.swiftHex/Features/Settings/ModelSectionView.swiftHex/Features/Settings/SettingsFeature.swiftHex/Features/Transcription/TranscriptionFeature.swiftHex/Models/AppHexSettings.swiftHexCore/Sources/HexCore/Settings/HexSettings.swift
| do { | ||
| let contents = try fileManager.contentsOfDirectory(atPath: modelFolderPath) | ||
| guard !contents.isEmpty else { continue } | ||
|
|
||
| // Check for specific model structure - need both tokenizer and model files | ||
| let hasModelFiles = contents.contains { $0.hasSuffix(".mlmodelc") || $0.contains("model") } | ||
| let tokenizerFolderPath = tokenizerPath(for: modelName).path | ||
| let hasTokenizer = fileManager.fileExists(atPath: tokenizerFolderPath) | ||
| let hasModelFiles = contents.contains { $0.hasSuffix(".mlmodelc") || $0.contains("model") || $0 == "config.json" } | ||
| let tokenizerFolderPath = candidate.appendingPathComponent("tokenizer", isDirectory: true).path | ||
| let hasTokenizer = fileManager.fileExists(atPath: tokenizerFolderPath) || contents.contains(where: { $0.contains("tokenizer") }) | ||
|
|
||
| // Both conditions must be true for a model to be considered downloaded | ||
| return hasModelFiles && hasTokenizer | ||
| } catch { | ||
| return false | ||
| if hasModelFiles && hasTokenizer { | ||
| return true | ||
| } | ||
| } catch { | ||
| continue | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Completeness heuristics are too loose and can mask a broken model dir.
$0.contains("model") matches almost anything (including partial download artifacts), and $0.contains("tokenizer") matches a stray file. Since downloadModelIfNeeded skips the download when isModelDownloaded returns true, a false positive turns into a hard load failure later. Prefer checking for the actual expected artifacts (*.mlmodelc directories plus a tokenizer folder / config.json).
Also, the catch { continue } discards the error entirely — log it at debug level so directory permission problems are diagnosable.
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 187-187: Trailing closure syntax should be used whenever possible
(trailing_closure)
🤖 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/TranscriptionClient.swift` around lines 181 - 194, tighten the
model completeness checks in isModelDownloaded: replace broad name matching with
actual expected artifacts, requiring an .mlmodelc directory and a tokenizer
directory or config.json, while avoiding stray files and partial-download
artifacts. In the existing directory enumeration catch block, log the caught
error at debug level before continuing so permission failures remain
diagnosable.
| /// Lists all model variants available in remote repository plus local models in App Support and Documents. | ||
| func getAvailableModels() async throws -> [String] { | ||
| var names = try await WhisperKit.fetchAvailableModels() | ||
| var names = (try? await WhisperKit.fetchAvailableModels()) ?? [] | ||
| #if canImport(FluidAudio) | ||
| for model in ParakeetModel.allCases.reversed() { | ||
| if !names.contains(model.identifier) { names.insert(model.identifier, at: 0) } | ||
| } | ||
| #endif | ||
|
|
||
| let fm = FileManager.default | ||
| var scanDirectories: [URL] = [] | ||
|
|
||
| if let baseDir = try? URL.hexModelsDirectory.appendingPathComponent("argmaxinc/whisperkit-coreml", isDirectory: true) { | ||
| scanDirectories.append(baseDir) | ||
| } | ||
|
|
||
| let docsURL = fm.urls(for: .documentDirectory, in: .userDomainMask).first | ||
| if let docsURL { | ||
| scanDirectories.append(docsURL.appendingPathComponent("huggingface/models/argmaxinc/whisperkit-coreml", isDirectory: true)) | ||
| scanDirectories.append(docsURL.appendingPathComponent("models/argmaxinc/whisperkit-coreml", isDirectory: true)) | ||
| scanDirectories.append(docsURL.appendingPathComponent("models", isDirectory: true)) | ||
| scanDirectories.append(docsURL) | ||
| } | ||
|
|
||
| for dir in scanDirectories { | ||
| guard let subdirs = try? fm.contentsOfDirectory(at: dir, includingPropertiesForKeys: [.isDirectoryKey], options: [.skipsHiddenFiles]) else { continue } | ||
| for subdir in subdirs { | ||
| var isDir: ObjCBool = false | ||
| if fm.fileExists(atPath: subdir.path, isDirectory: &isDir), isDir.boolValue { | ||
| let folderName = subdir.lastPathComponent | ||
| if folderName != "argmaxinc" && folderName != "whisperkit-coreml" && folderName != "huggingface" && folderName != "models" { | ||
| if let contents = try? fm.contentsOfDirectory(atPath: subdir.path), | ||
| contents.contains(where: { $0.hasSuffix(".mlmodelc") || $0.contains("model") || $0 == "tokenizer" || $0 == "config.json" }) { | ||
| if !names.contains(folderName) { | ||
| names.append(folderName) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Scanning all of ~/Documents is broad, slow, and swallows remote failures.
Three concerns:
- Line 206:
try?converts a network/API failure into an empty list while the function still advertisesthrows, so callers can't distinguish "offline" from "no models". Log the error at minimum. - Line 225: adding
docsURLitself means every top-level folder in the user's Documents whose contents happen to include a name containing"model"is surfaced as a transcription model. That's a very high false-positive rate. - The scan runs on every call and stats each subdirectory; on a large Documents folder that's noticeable blocking I/O inside the actor.
Consider restricting discovery to the explicit customModelDirectory setting plus the known huggingface/models/... layouts, and tightening the artifact predicate.
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 209-209: where clauses are preferred over a single if inside a for
(for_where)
🤖 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/TranscriptionClient.swift` around lines 204 - 244, The
getAvailableModels discovery is too broad, hides remote failures, and performs
expensive unscoped Documents scanning. In getAvailableModels, preserve the
throwing contract by handling WhisperKit.fetchAvailableModels errors with at
least an error log, remove the docsURL root from scanDirectories, and restrict
scanning to customModelDirectory plus the known Hugging Face/model layouts.
Tighten the artifact predicate so generic names containing “model” do not
qualify as transcription models, while preserving valid model and
tokenizer/config detection.
| // Check if parallel mode is requested via concurrentWorkerCount > 0 | ||
| let useParallel = options.concurrentWorkerCount > 0 | ||
| let audioLen = try await audioDuration(of: url) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
audioDuration(of:) is a throwing pre-flight opened only for logging.
This opens the file on every transcription and propagates any AVAudioFile error, failing the request before WhisperKit ever gets a chance — a regression for formats WhisperKit can handle but AVAudioFile can't. Make it non-throwing for logging purposes.
🛡️ Proposed fix
- let audioLen = try await audioDuration(of: url)
+ let audioLen = (try? await audioDuration(of: url)) ?? 0Guard the realtime-factor log against audioLen == 0 accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if parallel mode is requested via concurrentWorkerCount > 0 | |
| let useParallel = options.concurrentWorkerCount > 0 | |
| let audioLen = try await audioDuration(of: url) | |
| // Check if parallel mode is requested via concurrentWorkerCount > 0 | |
| let useParallel = options.concurrentWorkerCount > 0 | |
| let audioLen = (try? await audioDuration(of: url)) ?? 0 |
🤖 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/TranscriptionClient.swift` around lines 297 - 299, Update the
transcription flow around audioDuration(of:) so the logging-only duration lookup
is non-throwing and cannot prevent WhisperKit from processing supported formats;
use an optional or fallback result while preserving transcription behavior when
duration lookup fails. Adjust the realtime-factor logging guard to handle
audioLen == 0 safely and avoid invalid calculations.
| transcriptionLogger.notice("Using parallel chunked transcription for model=\(model, privacy: .public) file=\(url.lastPathComponent, privacy: .public) duration=\(String(format: "%.1f", audioLen), privacy: .public)s") | ||
| let text = try await transcribeParallel(whisperKit: whisperKit, url: url, model: model, options: options) | ||
| transcriptionLogger.info("Parallel transcription total elapsed \(String(format: "%.2f", Date().timeIntervalSince(startAll)), privacy: .public)s") | ||
| return text | ||
| } | ||
|
|
||
| // Standard transcription path. | ||
| transcriptionLogger.notice("Transcribing with WhisperKit model=\(model, privacy: .public) file=\(url.lastPathComponent, privacy: .public)") | ||
| let startTx = Date() | ||
| let results = try await whisperKit.transcribe(audioPath: url.path, decodeOptions: options) | ||
| transcriptionLogger.info("WhisperKit transcription took \(String(format: "%.2f", Date().timeIntervalSince(startTx)))s") | ||
| transcriptionLogger.info("WhisperKit request total elapsed \(String(format: "%.2f", Date().timeIntervalSince(startAll)))s") | ||
| let txDuration = Date().timeIntervalSince(startTx) | ||
| transcriptionLogger.info("WhisperKit transcription took \(String(format: "%.2f", txDuration), privacy: .public)s for \(String(format: "%.1f", audioLen), privacy: .public)s audio (realtime=\(String(format: "%.1f", audioLen/txDuration), privacy: .public)x)") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Recording file names are marked privacy: .public in the new log lines. The public-annotation pass applied to timing values was also applied to url.lastPathComponent, which is path data and should stay redacted in the unified log.
Hex/Clients/TranscriptionClient.swift#L302-L313: changefile=\(url.lastPathComponent, privacy: .public)to.privateon both the parallel-mode notice (Line 302) and the standard-path notice (Line 309); keep the timing/model values public.Hex/Features/Transcription/TranscriptionFeature.swift#L417-L417: change\(capturedURL.lastPathComponent, privacy: .public)to.private, leavingresult.countpublic.
As per coding guidelines: "use privacy annotations (, privacy: .private) for sensitive data like transcript text or file paths."
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 313-313: Operators should be surrounded by a single whitespace when they are being used
(operator_usage_whitespace)
📍 Affects 2 files
Hex/Clients/TranscriptionClient.swift#L302-L313(this comment)Hex/Features/Transcription/TranscriptionFeature.swift#L417-L417
🤖 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/TranscriptionClient.swift` around lines 302 - 313, Change the
file-name interpolations in the two transcription notices around
transcribeParallel and the standard WhisperKit transcription path from privacy:
.public to privacy: .private, while keeping model and timing values public. Also
update capturedURL.lastPathComponent in TranscriptionFeature.swift at line 417
to .private, leaving result.count public.
Source: Coding guidelines
| // Split into fixed 30s chunks (P60 default chunk_sec) | ||
| let chunkSec = 30 | ||
| let chunkSamples = chunkSec * sampleRate | ||
| var chunks: [[Float]] = [] | ||
| var offset = 0 | ||
| while offset < totalSamples { | ||
| let end = min(offset + chunkSamples, totalSamples) | ||
| let chunk = Array(audioArray[offset..<end]) | ||
| chunks.append(chunk) | ||
| offset = end | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Hard 30 s splits with no overlap will clip words at every chunk boundary.
Cutting at exact sample offsets almost always lands mid-word, and Whisper has no cross-chunk context, so each boundary risks a dropped or hallucinated word — one per 30 s of audio. The usual mitigations are a small overlap (0.2–1 s) with de-duplication at the seam, or splitting on VAD-detected silence rather than a fixed count.
Given the toggle is user-facing and advertised as "up to 10x faster", the accuracy tradeoff should at least be measured before shipping it as a plain on/off switch.
🤖 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/TranscriptionClient.swift` around lines 336 - 346, Update the
chunking logic around chunkSec and the offset loop to avoid hard,
non-overlapping 30-second boundaries: add a small overlap between adjacent
chunks and deduplicate overlapping transcription results at each seam, or split
chunks using VAD-detected silence. Before exposing this as a user-facing toggle,
measure and validate the resulting transcription accuracy and preserve the
advertised performance behavior.
| var texts: [String] = [] | ||
| var successCount = 0 | ||
| for result in results { | ||
| switch result { | ||
| case .success(let transcriptionResults): | ||
| let text = transcriptionResults.map(\.text).joined(separator: " ") | ||
| texts.append(text) | ||
| successCount += 1 | ||
| case .failure(let error): | ||
| transcriptionLogger.error("P60: chunk failed: \(error.localizedDescription)") | ||
| texts.append("") | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A failed chunk silently becomes an empty string.
The user gets a transcript that is missing 30 s of speech with no indication anything went wrong. At minimum, surface the failure — e.g. throw when successCount == 0, or when the failure ratio exceeds a threshold — so the caller can fall back to the sequential path instead of pasting a silently truncated result.
🛡️ Suggested fallback
let realtimeFactor = totalDuration / totalTxTime
+ if successCount == 0, numChunks > 0 {
+ throw NSError(
+ domain: "TranscriptionClient",
+ code: -7,
+ userInfo: [NSLocalizedDescriptionKey: "All \(numChunks) parallel chunks failed to transcribe"]
+ )
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var texts: [String] = [] | |
| var successCount = 0 | |
| for result in results { | |
| switch result { | |
| case .success(let transcriptionResults): | |
| let text = transcriptionResults.map(\.text).joined(separator: " ") | |
| texts.append(text) | |
| successCount += 1 | |
| case .failure(let error): | |
| transcriptionLogger.error("P60: chunk failed: \(error.localizedDescription)") | |
| texts.append("") | |
| } | |
| } | |
| var texts: [String] = [] | |
| var successCount = 0 | |
| for result in results { | |
| switch result { | |
| case .success(let transcriptionResults): | |
| let text = transcriptionResults.map(\.text).joined(separator: " ") | |
| texts.append(text) | |
| successCount += 1 | |
| case .failure(let error): | |
| transcriptionLogger.error("P60: chunk failed: \(error.localizedDescription)") | |
| texts.append("") | |
| } | |
| } | |
| if successCount == 0, numChunks > 0 { | |
| throw NSError( | |
| domain: "TranscriptionClient", | |
| code: -7, | |
| userInfo: [NSLocalizedDescriptionKey: "All \(numChunks) parallel chunks failed to transcribe"] | |
| ) | |
| } |
🤖 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/TranscriptionClient.swift` around lines 366 - 378, Update the
result aggregation loop in TranscriptionClient so chunk failures are not
silently represented as empty transcript text. After tracking successCount and
failures, surface the failure to the caller—at minimum throw when successCount
is zero, or when the failure ratio exceeds the established threshold—so the
caller can fall back to the sequential transcription path.
| if format.commonFormat == .pcmFormatFloat32 { | ||
| let samples = Array(UnsafeBufferPointer(start: buffer.floatChannelData?[0], count: Int(buffer.frameLength))) | ||
| if srcRate != targetRate { | ||
| return resample(samples, from: srcRate, to: targetRate) | ||
| } | ||
| return samples | ||
| } else if format.commonFormat == .pcmFormatInt16 { | ||
| let intSamples = Array(UnsafeBufferPointer(start: buffer.int16ChannelData?[0], count: Int(buffer.frameLength))) | ||
| let samples = intSamples.map { Float($0) / Float(Int16.max) } | ||
| if srcRate != targetRate { | ||
| return resample(samples, from: srcRate, to: targetRate) | ||
| } | ||
| return samples | ||
| } | ||
| throw NSError(domain: "TranscriptionClient", code: -6, userInfo: [NSLocalizedDescriptionKey: "Unsupported audio format"]) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
UnsafeBufferPointer(start: nil, count: n) crashes when channel data is nil.
buffer.floatChannelData?[0] (and the int16 equivalent) is optional, but the optional chain only nils out the start pointer — count is still frameLength, which trips a runtime precondition. Guard explicitly instead.
Note also that AVAudioFile.processingFormat is float32 deinterleaved by default, so the .pcmFormatInt16 branch is likely unreachable; if the intent is broad input-format support, AVAudioConverter would be more robust than hand-rolled branches.
🛡️ Proposed fix
if format.commonFormat == .pcmFormatFloat32 {
- let samples = Array(UnsafeBufferPointer(start: buffer.floatChannelData?[0], count: Int(buffer.frameLength)))
+ guard let channelData = buffer.floatChannelData?[0] else {
+ throw NSError(domain: "TranscriptionClient", code: -6, userInfo: [NSLocalizedDescriptionKey: "Missing float channel data"])
+ }
+ let samples = Array(UnsafeBufferPointer(start: channelData, count: Int(buffer.frameLength)))
if srcRate != targetRate {
return resample(samples, from: srcRate, to: targetRate)
}
return samples
} else if format.commonFormat == .pcmFormatInt16 {
- let intSamples = Array(UnsafeBufferPointer(start: buffer.int16ChannelData?[0], count: Int(buffer.frameLength)))
+ guard let channelData = buffer.int16ChannelData?[0] else {
+ throw NSError(domain: "TranscriptionClient", code: -6, userInfo: [NSLocalizedDescriptionKey: "Missing int16 channel data"])
+ }
+ let intSamples = Array(UnsafeBufferPointer(start: channelData, count: Int(buffer.frameLength)))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if format.commonFormat == .pcmFormatFloat32 { | |
| let samples = Array(UnsafeBufferPointer(start: buffer.floatChannelData?[0], count: Int(buffer.frameLength))) | |
| if srcRate != targetRate { | |
| return resample(samples, from: srcRate, to: targetRate) | |
| } | |
| return samples | |
| } else if format.commonFormat == .pcmFormatInt16 { | |
| let intSamples = Array(UnsafeBufferPointer(start: buffer.int16ChannelData?[0], count: Int(buffer.frameLength))) | |
| let samples = intSamples.map { Float($0) / Float(Int16.max) } | |
| if srcRate != targetRate { | |
| return resample(samples, from: srcRate, to: targetRate) | |
| } | |
| return samples | |
| } | |
| throw NSError(domain: "TranscriptionClient", code: -6, userInfo: [NSLocalizedDescriptionKey: "Unsupported audio format"]) | |
| if format.commonFormat == .pcmFormatFloat32 { | |
| guard let channelData = buffer.floatChannelData?[0] else { | |
| throw NSError(domain: "TranscriptionClient", code: -6, userInfo: [NSLocalizedDescriptionKey: "Missing float channel data"]) | |
| } | |
| let samples = Array(UnsafeBufferPointer(start: channelData, count: Int(buffer.frameLength))) | |
| if srcRate != targetRate { | |
| return resample(samples, from: srcRate, to: targetRate) | |
| } | |
| return samples | |
| } else if format.commonFormat == .pcmFormatInt16 { | |
| guard let channelData = buffer.int16ChannelData?[0] else { | |
| throw NSError(domain: "TranscriptionClient", code: -6, userInfo: [NSLocalizedDescriptionKey: "Missing int16 channel data"]) | |
| } | |
| let intSamples = Array(UnsafeBufferPointer(start: channelData, count: Int(buffer.frameLength))) | |
| let samples = intSamples.map { Float($0) / Float(Int16.max) } | |
| if srcRate != targetRate { | |
| return resample(samples, from: srcRate, to: targetRate) | |
| } | |
| return samples | |
| } | |
| throw NSError(domain: "TranscriptionClient", code: -6, userInfo: [NSLocalizedDescriptionKey: "Unsupported audio format"]) |
🤖 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/TranscriptionClient.swift` around lines 400 - 414, In the audio
sample conversion logic, explicitly validate that floatChannelData or
int16ChannelData is non-nil before constructing the corresponding
UnsafeBufferPointer; throw the existing unsupported-format error when channel
data is unavailable instead of passing a nil start pointer with a nonzero count.
Apply this to both branches while preserving the existing resampling behavior.
| private func resample(_ samples: [Float], from srcRate: Float, to dstRate: Float) -> [Float] { | ||
| let ratio = srcRate / dstRate | ||
| let outLen = Int(Float(samples.count) / ratio) | ||
| var out = [Float](repeating: 0, count: outLen) | ||
| for i in 0..<outLen { | ||
| let srcIdx = Float(i) * ratio | ||
| let lo = Int(srcIdx) | ||
| let hi = min(lo + 1, samples.count - 1) | ||
| let frac = srcIdx - Float(lo) | ||
| out[i] = samples[lo] * (1 - frac) + samples[hi] * frac | ||
| } | ||
| return out | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Linear-interpolation downsampling has no anti-alias filter.
Going from 44.1/48 kHz to 16 kHz without a low-pass first folds everything above 8 kHz back into the band, which measurably degrades Whisper accuracy — exactly the thing this feature is trading against speed. AVAudioConverter (or AVAudioFile read into a 16 kHz mono processingFormat) does this correctly and is faster than a per-sample Swift loop.
🤖 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/TranscriptionClient.swift` around lines 418 - 430, Replace the
custom linear-interpolation implementation in resample(_:from:to:) with
AVAudioConverter-based resampling configured for the target rate and existing
channel/format requirements, so downsampling applies proper anti-alias
filtering. Preserve the method’s [Float] input/output contract and target-length
behavior while removing the per-sample interpolation loop.
| concurrentWorkerCount: isParallelMode ? 8 : 0, | ||
| chunkingStrategy: .vad | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
WhisperKit DecodingOptions concurrentWorkerCount 0 meaning default value
💡 Result:
In WhisperKit, the concurrentWorkerCount property within DecodingOptions determines the number of audio chunks processed in parallel [1][2]. A value of 0 (or in some older versions, the absence of an explicit setting) has historically been interpreted as a signal to process all available audio chunks in a single batch, or effectively without a specific cap on concurrency [3]. However, recent updates have moved toward setting a specific default value to manage memory usage [4]. As of the current implementation, the default value for concurrentWorkerCount is 16 [4]. This change was introduced to cap peak memory consumption, particularly for long audio files when using Voice Activity Detection (VAD) chunking, preventing devices from running out of memory by limiting the number of concurrent asynchronous CoreML predictions [5][4]. Users are encouraged to adjust this value based on their specific hardware: - 16 for macOS devices with >16GB of RAM [6]. - 8 for macOS devices with 8-16GB of RAM [6]. - 4 for iOS devices [6]. - 1 for sequential, memory-minimal processing [6].
Citations:
- 1: Async batch predictions argmaxinc/argmax-oss-swift#107
- 2: https://github.com/argmaxinc/whisperkit/blob/main/Sources/WhisperKit/Core/Configurations.swift
- 3: https://github.com/argmaxinc/WhisperKit/blob/2870b46b/Sources/WhisperKit/Core/WhisperKit.swift
- 4: argmaxinc/argmax-oss-swift@3cd3ef1
- 5: VAD detection uses unbounded amount of memory argmaxinc/argmax-oss-swift#204
- 6: https://argmaxinc-whisperkit.mintlify.app/advanced/memory-management
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files around TranscriptionFeature.swift =="
git ls-files | rg 'TranscriptionFeature\.swift|TranscriptionClient\.swift|Package\.swift|Package\.lock' || true
echo
echo "== TranscriptionFeature relevant lines =="
sed -n '370,440p' Hex/Features/Transcription/TranscriptionFeature.swift 2>/dev/null || true
echo
echo "== TranscriptionClient occurrences =="
rg -n "concurrentWorkerCount|parallel|ParallelMode|parallelMode|Sequential|sequential" Hex -S || true
echo
echo "== WhisperKit dependency pins =="
if [ -f Package.swift ]; then
sed -n '1,160p' Package.swift
fi
if [ -f Package.resolved ]; then
cat Package.resolved
fi
if [ -d .build ]; then
find .build -name Package.resolved -o -name Package.swift 2>/dev/null | head -20
fi
echo
echo "== Check if WhisperKit local source exists in sandbox =="
fd -a 'Configurations.swift|TranscriptionFeature.swift' . | head -50Repository: kitlangton/Hex
Length of output: 4984
🌐 Web query:
WhisperKit DecodingOptions concurrentWorkerCount source 0 unbounded default 16
💡 Result:
In WhisperKit, the concurrentWorkerCount parameter within DecodingOptions controls the number of concurrent tasks (workers) used for audio processing and batch inference [1][2]. Historically, this value was set to 0, which effectively allowed for an unbounded number of concurrent tasks when using features like VAD (Voice Activity Detection) chunking [3][4]. This behavior could lead to excessive memory consumption and crashes on resource-constrained devices, particularly when processing long audio files [3]. To mitigate these memory issues, the default value was updated to 16 in September 2024 (via PR #205) to provide a balance between throughput and peak memory usage [3][4]. Current implementations typically utilize this default of 16, though it can be adjusted based on the target platform's available memory [5][2]. For instance, on iOS or memory-limited environments, it is often recommended to use a lower value (such as 4), while high-memory macOS environments may support higher values [5][2][6].
Citations:
- 1: Async batch predictions argmaxinc/argmax-oss-swift#107
- 2: https://deepwiki.com/argmaxinc/WhisperKit/2.2-basic-usage
- 3: VAD detection uses unbounded amount of memory argmaxinc/argmax-oss-swift#204
- 4: argmaxinc/argmax-oss-swift@3cd3ef1
- 5: https://argmaxinc-whisperkit.mintlify.app/advanced/memory-management
- 6: https://deepwiki.com/argmaxinc/WhisperKit/4.4-custom-configurations
🌐 Web query:
site:github.com/argmaxinc/WhisperKit DecodingOptions concurrentWorkerCount default 0
💡 Result:
In the WhisperKit framework, the concurrentWorkerCount parameter within DecodingOptions controls the number of concurrent tasks (specifically audio chunks) processed during transcription [1][2]. While earlier versions of the framework utilized a default value of 0—which signified that all audio arrays were processed in a single batch (effectively unlimited concurrency relative to the number of input items) [2]—this was later changed to mitigate memory issues during Voice Activity Detection (VAD) processing [3][4]. As of the current implementation, the default value for concurrentWorkerCount is 16 [3][5]. This value was selected to provide a balance between maintaining high throughput and limiting peak memory consumption, particularly on devices like those running iOS [4]. Users may still manually configure this value to optimize performance based on their specific hardware and input files [4].
Citations:
- 1: Async batch predictions argmaxinc/argmax-oss-swift#107
- 2: argmaxinc/argmax-oss-swift@65cb888
- 3: argmaxinc/argmax-oss-swift@3cd3ef1
- 4: VAD detection uses unbounded amount of memory argmaxinc/argmax-oss-swift#204
- 5: argmaxinc/argmax-oss-swift@c2f1b57
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TranscriptionClient parallel code section =="
sed -n '270,330p' Hex/Clients/TranscriptionClient.swift 2>/dev/null || true
echo
echo "== Package dependency references =="
rg -n "WhisperKit|github.com/argmaxinc/WhisperKit|\.package(url:.*WhisperKit|name: \"WhisperKit\"" . -S || true
echo
echo "== Swift config package files =="
if [ -f HexCore/Package.swift ]; then
sed -n '1,220p' HexCore/Package.swift
fiRepository: kitlangton/Hex
Length of output: 4753
Make sequential transcription explicitly sequential
Current WhisperKit docs indicate concurrentWorkerCount of 0 does not force sequential work and newer versions default this to a capped concurrent value. As written, disabling Parallel Mode uses the standard WhisperKit transcribe(...) path with options that may still run concurrently, while TranscriptionClient treats options.concurrentWorkerCount > 0 as parallel. Use an explicit sequential value/config if the setting is meant to disable parallel chunking, and avoid hardcoding 8 by deriving the cap from available cores/memory.
🤖 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/Features/Transcription/TranscriptionFeature.swift` around lines 411 -
413, Update the transcription options construction in the sequential/parallel
mode flow so disabling parallel mode explicitly configures WhisperKit for
sequential processing rather than passing concurrentWorkerCount as 0. For
parallel mode, derive the worker cap from available system CPU and memory
resources instead of hardcoding 8, while preserving the existing .vad chunking
strategy and TranscriptionClient’s parallel-mode detection.
1 participant
Summary
This PR adds Parallel Mode chunked decoding for Whisper models in Hex, alongside a user-facing toggle setting and local
~/Documentsmodel discovery.Key Changes
Parallel Mode Core Engine (
TranscriptionClient.swift):concurrentWorkerCountlogic so passing0when Parallel Mode is OFF ensures standard single-pass sequential transcription is executed without WhisperKit overridingnilto16.Settings UI Integration (
GeneralSectionView.swift&SettingsFeature.swift):toggleParallelModeaction inSettingsFeaturereducer to persist user preference inhex_settings.json.~/DocumentsLocal Model Discovery (TranscriptionClient.swift):~/Documents,~/Documents/models, or~/Documents/huggingface/models.Public Log Privacy Annotations (
HexLog.transcription):privacy: .publicannotations to duration, realtime speed, and chunk count log interpolations for clean diagnostic streams in Console.Release Changeset:
.changeset/9b51fe61.mdpatch fragment following repository guidelines.Benchmarks (Whisper Large v3 Turbo, 10-core Apple Silicon)
Summary by CodeRabbit