Add NVIDIA Parakeet TDT 0.6b v3#123
Open
ollieatkinson wants to merge 10 commits intoamicalhq:mainfrom
Open
Conversation
📝 WalkthroughWalkthroughAdds Parakeet ONNX local transcription and artifact-aware model handling: extends model schema with per-model artifacts and runtime, updates download/sync to per-model artifact validation and directories, implements Parakeet provider and feature extractor, and adapts service/recording flows to handle runtime-aware routing and transcription-service availability. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as RecordingManager
participant TS as TranscriptionService
participant PP as ParakeetProvider
participant FE as ParakeetFeatureExtractor
participant ONNX as ONNXRuntime
participant FS as FileSystem
Client->>TS: processStreamingChunk(audio, context.modelId)
TS->>PP: transcribe({audio, modelId})
PP->>PP: buffer frames & VAD heuristics
alt need preload / model not loaded
PP->>FS: read artifacts (vocab, encoder.onnx, decoder.onnx, ...)
FS-->>PP: artifact files
PP->>ONNX: create sessions (preproc?, encoder, decoder)
ONNX-->>PP: sessions ready
end
PP->>FE: extract(features)
FE-->>PP: inputFeatures, shape
PP->>ONNX: encoder inference
ONNX-->>PP: encoded representations
PP->>ONNX: stepped decoder-joint inference
ONNX-->>PP: token ids
PP->>PP: decodeParakeetTokens()
PP-->>TS: transcript text
TS->>Client: emit transcription
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/services/model-service.ts (1)
120-134:⚠️ Potential issue | 🔴 CriticalAdd
runtimefield to database schema and populate it for Parakeet models.The database schema is missing the
runtimefield entirely, buttranscription-service.tsline 96 depends onmodel?.runtime === "parakeet-onnx"to route Parakeet models to the Parakeet provider. Without this field in the schema, local Parakeet bundles cannot be persisted with their runtime discriminator and will fail provider selection.Required changes:
- Add
runtime?: stringfield to themodelstable schema (apps/desktop/src/db/schema.ts)- Populate
runtime: "parakeet-onnx"in thewhisperModelsDatapayload at lines 120-134 for Parakeet model IDs- Include
runtimein theupsertModelcall at lines 513-537Also applies to: 513-537
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/services/model-service.ts` around lines 120 - 134, Add a nullable runtime?: string field to the models table schema (in apps/desktop/src/db/schema.ts) so model records can carry a runtime discriminator; then when building whisperModelsData from AVAILABLE_MODELS (the mapping that creates id/name/description/... and artifacts) include runtime: "parakeet-onnx" for the Parakeet model IDs (only set for those models) so Parakeet bundles persist their runtime, and finally ensure the upsertModel call (the payload assembled in the upsertModel invocation) includes the runtime field so the database upsert writes it.
🧹 Nitpick comments (1)
apps/desktop/src/db/models.ts (1)
236-260: Consider extracting localFiles parsing into a helper.The nested type-narrowing for
existingLocalFilesis verbose but necessary for type safety with JSON columns. Consider extracting this into a reusable helper function to reduce complexity and improve readability.♻️ Optional: Extract helper function
function extractLocalFiles(originalModel: unknown): string[] { if ( originalModel && typeof originalModel === "object" && !Array.isArray(originalModel) && "localFiles" in originalModel && Array.isArray((originalModel as { localFiles?: unknown }).localFiles) ) { return (originalModel as { localFiles: unknown[] }).localFiles.filter( (value): value is string => typeof value === "string" ); } return []; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/db/models.ts` around lines 236 - 260, Extract the verbose type-narrowing that computes existingLocalFiles into a reusable helper (e.g., extractLocalFiles(originalModel: unknown): string[]) and replace the inline block that defines existingLocalFiles with a call to that helper; the helper should perform the same checks (non-null object, not an array, has "localFiles" as an array) and filter entries to strings using the same type guard, so localFilesChanged logic (which references existingLocalFiles and resolvedFiles) remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/pipeline/providers/transcription/parakeet-provider.ts`:
- Around line 127-159: The code calls this.reset() prematurely (after
snapshotting frameBufferSpeechProbabilities and before VAD, feature extraction,
and transcribeTdt), which can permanently drop audio on any downstream error;
change the flow so buffers are not cleared until transcription succeeds: either
(a) defer calling this.reset() until after transcribeTdt(...) returns
successfully, or (b) keep a copy of the buffer/frames (e.g. const savedFrames =
[...this.frameBufferSpeechProbabilities] and const savedRaw = rawAudio) and in
the catch block restore them by reassigning the originals (so
extractSpeechFromVad, extractFeatures, and transcribeTdt can be retried). Update
references around aggregateFrames(), extractSpeechFromVad(...),
extractFeatures(...), transcribeTdt(...), and reset() to implement the chosen
approach and ensure the catch path restores or avoids clearing buffers before
successful completion.
- Around line 471-513: The code currently calls this.releaseSessions() before
the new ONNX/ORT sessions are fully created, which can leave the provider in a
broken state if later session creation (createSessionWithFallback for
resolved.nemoPreprocessorPath, resolved.encoderModelPath,
resolved.decoderJointModelPath) fails; change the flow to build all new
resources into local variables first (call resolveModelPaths, loadModelConfig,
instantiate ParakeetFeatureExtractor, loadParakeetVocabulary, then call
createSessionWithFallback for preprocessor/encoder/decoder into local consts
like newPreprocessorResult, newEncoderResult, newDecoderResult and keep local
preprocessorProviders), and only after all succeeded call this.releaseSessions()
and atomically assign this.tdtPreprocessorSession, this.tdtEncoderSession,
this.tdtDecoderJointSession, this.featureSize, this.featureExtractor,
this.vocabulary, and this.maxTokensPerStep; on any failure during creation, make
sure to dispose/close any partially created sessions from the locals before
rethrowing so there are no leaked sessions.
In `@apps/desktop/src/pipeline/utils/parakeet-feature-extractor.ts`:
- Around line 279-310: In loadParakeetVocabulary, guard against an entirely
skipped/empty vocabulary by validating tokensById and blankTokenId: after
populating tokensById check tokensById.size and throw a clear Error if zero;
compute maxId only when tokensById has entries; after constructing tokens,
verify that tokens.length > 0 and that blankTokenId (computed from
tokens.findIndex) is >= 0 and throw an Error if not. Reference
loadParakeetVocabulary, tokensById, maxId, tokens, and blankTokenId when making
these checks so the decoder never receives -1 or an empty tokens array.
In `@apps/desktop/src/services/model-service.ts`:
- Around line 446-486: Attach the fileStream "error" listener immediately after
creating fileStream (right after the fs.createWriteStream call) to avoid
unhandled errors during open/write, and move the existing fileStream.on("error",
...) registration into that spot; additionally, handle backpressure from
fileStream.write(value) inside the read loop by checking the boolean return
value and, when write returns false, pause reading by awaiting a Promise that
resolves on fileStream.once("drain"), then resume reading; keep existing
abortController checks and the final fileStream.end logic unchanged but ensure
error and drain handlers are registered before any writes occur.
---
Outside diff comments:
In `@apps/desktop/src/services/model-service.ts`:
- Around line 120-134: Add a nullable runtime?: string field to the models table
schema (in apps/desktop/src/db/schema.ts) so model records can carry a runtime
discriminator; then when building whisperModelsData from AVAILABLE_MODELS (the
mapping that creates id/name/description/... and artifacts) include runtime:
"parakeet-onnx" for the Parakeet model IDs (only set for those models) so
Parakeet bundles persist their runtime, and finally ensure the upsertModel call
(the payload assembled in the upsertModel invocation) includes the runtime field
so the database upsert writes it.
---
Nitpick comments:
In `@apps/desktop/src/db/models.ts`:
- Around line 236-260: Extract the verbose type-narrowing that computes
existingLocalFiles into a reusable helper (e.g.,
extractLocalFiles(originalModel: unknown): string[]) and replace the inline
block that defines existingLocalFiles with a call to that helper; the helper
should perform the same checks (non-null object, not an array, has "localFiles"
as an array) and filter entries to strings using the same type guard, so
localFilesChanged logic (which references existingLocalFiles and resolvedFiles)
remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b9b935d-8684-4f6f-832e-c3c37b2c42e6
⛔ Files ignored due to path filters (1)
apps/desktop/public/icons/models/nvidia.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
apps/desktop/src/constants/models.tsapps/desktop/src/db/models.tsapps/desktop/src/main/managers/recording-manager.tsapps/desktop/src/main/managers/service-manager.tsapps/desktop/src/pipeline/core/pipeline-types.tsapps/desktop/src/pipeline/providers/transcription/parakeet-provider.tsapps/desktop/src/pipeline/utils/parakeet-feature-extractor.tsapps/desktop/src/services/model-service.tsapps/desktop/src/services/transcription-service.tsapps/desktop/src/trpc/routers/models.tsapps/desktop/tests/services/model-validity.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/services/model-service.ts (1)
936-953:⚠️ Potential issue | 🟠 Major
getBestAvailableModelPath()still drops Parakeet when no model is selected.Lines 941-942 will return a Parakeet
localPathif it is explicitly selected, but the fallback on Lines 946-953 only searches Whisper IDs. After this PR, a Parakeet-only install can therefore make the service report local speech models as available while this helper still returnsnullwhenever the selection is unset or cleared. Either reuselocalSpeechPreferencehere or make this helper explicitly Whisper-only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/services/model-service.ts` around lines 936 - 953, getBestAvailableModelPath currently only falls back to Whisper models even though getSelectedModel/selectedModelId can return a Parakeet model; update getBestAvailableModelPath to also consider the local speech preference before trying the Whisper preferredOrder: after retrieving downloadedModels and selectedModelId, if no selectedModelId check localSpeechPreference (or the same source that stores Parakeet selection) and if that ID exists in downloadedModels return its localPath; otherwise continue the existing preferredOrder search for Whisper IDs. Ensure you reference getBestAvailableModelPath, downloadedModels, selectedModelId, and localSpeechPreference (or the method that provides it) when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/services/model-service.ts`:
- Around line 468-473: The cleanup is happening before the file descriptor is
released; replace any listeners or synchronous cleanup that wait for "finish" or
call fileStream.close() then immediately unlink with gating on the "close" event
instead: for the abort path where abortController.signal.aborted currently calls
fileStream.close() and unlinks artifactPath, add a fileStream.once("close")
handler and perform fs.unlinkSync(artifactPath) inside that handler (and only
call fileStream.close() to trigger it); similarly change the
fileStream.once("finish") handlers at the download-completion block to
fileStream.once("close"), and apply the same pattern for the checksum cleanup
(use checksumPath) and the general error cleanup paths so unlinking happens only
after fileStream emits "close". Ensure you reference the same fileStream,
abortController, artifactPath and checksumPath variables already used in the
function.
---
Outside diff comments:
In `@apps/desktop/src/services/model-service.ts`:
- Around line 936-953: getBestAvailableModelPath currently only falls back to
Whisper models even though getSelectedModel/selectedModelId can return a
Parakeet model; update getBestAvailableModelPath to also consider the local
speech preference before trying the Whisper preferredOrder: after retrieving
downloadedModels and selectedModelId, if no selectedModelId check
localSpeechPreference (or the same source that stores Parakeet selection) and if
that ID exists in downloadedModels return its localPath; otherwise continue the
existing preferredOrder search for Whisper IDs. Ensure you reference
getBestAvailableModelPath, downloadedModels, selectedModelId, and
localSpeechPreference (or the method that provides it) when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3cdcb56c-6cb9-4b39-bf89-89cc9110b708
📒 Files selected for processing (3)
apps/desktop/src/pipeline/providers/transcription/parakeet-provider.tsapps/desktop/src/pipeline/utils/parakeet-feature-extractor.tsapps/desktop/src/services/model-service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/pipeline/providers/transcription/parakeet-provider.ts
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/desktop/src/services/model-service.ts (3)
298-305: Consider adding a defensive guard for empty arrays.The function's return type is
string, but returningdownloadedModelIds[0]on an empty array yieldsundefined. While callers currently guard against this, adding a defensive check would improve type safety.🛡️ Optional defensive check
private pickPreferredLocalModelId(downloadedModelIds: string[]): string { + if (downloadedModelIds.length === 0) { + throw new Error("Cannot pick from empty model list"); + } for (const candidateId of this.localSpeechPreference) { if (downloadedModelIds.includes(candidateId)) { return candidateId; } } return downloadedModelIds[0]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/services/model-service.ts` around lines 298 - 305, The method pickPreferredLocalModelId currently returns downloadedModelIds[0] which can be undefined for an empty array; add a defensive guard at the top of pickPreferredLocalModelId to handle downloadedModelIds.length === 0 (e.g., return an empty string '' or throw a clear error) so the function always respects its string return type; update any call sites if you choose to throw to handle the error accordingly.
774-789: Checksum algorithm detection heuristic is reasonable but implicit.The 64-character length check for SHA-256 vs SHA-1 fallback works for the expected use cases. Consider adding a brief comment documenting the supported formats, as unexpected hash lengths (e.g., MD5's 32 chars) would silently use SHA-1 and fail validation.
📝 Optional: Add documentation comment
+ // Calculate file checksum (auto-detect algorithm from expected hash length) + // Supported: SHA-256 (64 hex chars), SHA-1 (40 hex chars, default) private async calculateFileChecksum( filePath: string, expectedChecksum?: string, ): Promise<string> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/services/model-service.ts` around lines 774 - 789, Add a short documentation comment above the calculateFileChecksum method explaining the checksum detection heuristic: it treats a 64-character expectedChecksum as SHA-256 and otherwise uses SHA-1, and that other lengths (e.g., 32‑char MD5) will fall back to SHA-1 and may therefore fail validation; reference the function name calculateFileChecksum and the expectedChecksum parameter so future readers understand the supported formats and behavior on unexpected hash lengths.
957-985: Consider renaminggetBestAvailableModelPathtogetBestWhisperModelPathfor clarity.The function is correctly WhisperProvider-specific (called only from whisper-provider.ts:312). The exclusion of Parakeet is intentional—ParakeetProvider has its own separate model initialization logic via
initializeModel(). The generic name "BestAvailableModelPath" may suggest it handles all model types, so renaming would make the Whisper-specific purpose explicit. Also,whisper-large-v1is absent from the codebase intentionally (not in any preference lists).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/services/model-service.ts` around lines 957 - 985, Rename the Whisper-specific helper getBestAvailableModelPath to getBestWhisperModelPath to make its purpose explicit: change the function name in model-service.ts (the async method currently named getBestAvailableModelPath) and update every usage/import to the new name (notably the call in whisper-provider.ts around the initialization logic). Ensure exported symbols/signatures and any unit tests or type references are updated accordingly; keep the function body and its preferredOrder list unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/services/model-service.ts`:
- Around line 298-305: The method pickPreferredLocalModelId currently returns
downloadedModelIds[0] which can be undefined for an empty array; add a defensive
guard at the top of pickPreferredLocalModelId to handle
downloadedModelIds.length === 0 (e.g., return an empty string '' or throw a
clear error) so the function always respects its string return type; update any
call sites if you choose to throw to handle the error accordingly.
- Around line 774-789: Add a short documentation comment above the
calculateFileChecksum method explaining the checksum detection heuristic: it
treats a 64-character expectedChecksum as SHA-256 and otherwise uses SHA-1, and
that other lengths (e.g., 32‑char MD5) will fall back to SHA-1 and may therefore
fail validation; reference the function name calculateFileChecksum and the
expectedChecksum parameter so future readers understand the supported formats
and behavior on unexpected hash lengths.
- Around line 957-985: Rename the Whisper-specific helper
getBestAvailableModelPath to getBestWhisperModelPath to make its purpose
explicit: change the function name in model-service.ts (the async method
currently named getBestAvailableModelPath) and update every usage/import to the
new name (notably the call in whisper-provider.ts around the initialization
logic). Ensure exported symbols/signatures and any unit tests or type references
are updated accordingly; keep the function body and its preferredOrder list
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d552e57-dc35-4e5b-8cd6-3b1d37812dc9
📒 Files selected for processing (1)
apps/desktop/src/services/model-service.ts
# Conflicts: # apps/desktop/src/services/model-service.ts # apps/desktop/src/trpc/routers/models.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/services/model-service.ts (1)
538-544: Minor: Progress bar may jump when Content-Length updates totalBytes.When an artifact's
sizeis undefined but the server returnsContent-Length,totalBytesincreases mid-download (line 543). This can cause the progress percentage to momentarily decrease. Consider pre-fetching Content-Length for all artifacts with HEAD requests, or accepting this as acceptable behavior for edge cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/services/model-service.ts` around lines 538 - 544, The progress bar can jump because artifactBytes (computed from response.headers.get("content-length") or artifact.size) is applied to progress.totalBytes mid-download when artifact.size was undefined; update the logic in the download flow (the function handling the response parsing where artifactBytes and progress.totalBytes are updated) to avoid changing totalBytes after bytesReceived for that artifact > 0 — either prefetch Content-Length for artifacts lacking size using HEAD requests before starting downloads, or only add artifactBytes to progress.totalBytes if progress has not yet started receiving bytes for that artifact (i.e., bytesReceived === 0); ensure you reference and update the artifactBytes/artifact.size check and progress.totalBytes modification accordingly.apps/desktop/src/db/models.ts (1)
288-294: Minor:filePathfallback may not match actual primary file location.When
resolvedFilesisnull(model not fully downloaded),filePathfalls back topath.join(modelsDirectory, model.id, model.filename). This path is only used in thefileExistsbranch (line 297), but the assignment happens unconditionally. The logic is correct sincefileExistsguards usage, but the fallback path construction could be misleading during debugging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/db/models.ts` around lines 288 - 294, The code unconditionally constructs a fallback path into filePath even when resolvedFiles is null, which is misleading; update the logic in the area using resolveRequiredLocalFiles, filePath and fileExists so the fallback path (path.join(modelsDirectory, model.id, model.filename)) is only computed when resolvedFiles is truthy or when you actually need it (i.e., when fileExists is true). Concretely, make filePath derived from resolvedFiles (using resolvedFiles.find(...)) and avoid building the join(...) fallback unless you confirm resolvedFiles is null and you intend to check the fallback, or set filePath to undefined/null when resolvedFiles is null and compute the fallback later right before the branch that uses fileExists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/managers/recording-manager.ts`:
- Around line 582-588: getTranscriptionService() already emits the
"transcription_failed" widget-notification and returns null, so throwing an
AppError here causes the catch block to emit a second notification; instead of
throwing from this location, handle the null inline: check the result of
getTranscriptionService(), emit (or rely on the existing) notification once, and
perform an early return/cleanup from the current method (the surrounding
function in recording-manager.ts that calls getTranscriptionService()) so the
catch path is not triggered. Ensure you remove the throw of AppError here and
keep any necessary cleanup logic locally rather than relying on the outer catch
to notify.
---
Nitpick comments:
In `@apps/desktop/src/db/models.ts`:
- Around line 288-294: The code unconditionally constructs a fallback path into
filePath even when resolvedFiles is null, which is misleading; update the logic
in the area using resolveRequiredLocalFiles, filePath and fileExists so the
fallback path (path.join(modelsDirectory, model.id, model.filename)) is only
computed when resolvedFiles is truthy or when you actually need it (i.e., when
fileExists is true). Concretely, make filePath derived from resolvedFiles (using
resolvedFiles.find(...)) and avoid building the join(...) fallback unless you
confirm resolvedFiles is null and you intend to check the fallback, or set
filePath to undefined/null when resolvedFiles is null and compute the fallback
later right before the branch that uses fileExists.
In `@apps/desktop/src/services/model-service.ts`:
- Around line 538-544: The progress bar can jump because artifactBytes (computed
from response.headers.get("content-length") or artifact.size) is applied to
progress.totalBytes mid-download when artifact.size was undefined; update the
logic in the download flow (the function handling the response parsing where
artifactBytes and progress.totalBytes are updated) to avoid changing totalBytes
after bytesReceived for that artifact > 0 — either prefetch Content-Length for
artifacts lacking size using HEAD requests before starting downloads, or only
add artifactBytes to progress.totalBytes if progress has not yet started
receiving bytes for that artifact (i.e., bytesReceived === 0); ensure you
reference and update the artifactBytes/artifact.size check and
progress.totalBytes modification accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da20a18c-bc19-4029-8f4c-8249633977ed
📒 Files selected for processing (5)
apps/desktop/src/db/models.tsapps/desktop/src/main/managers/recording-manager.tsapps/desktop/src/services/model-service.tsapps/desktop/src/services/transcription-service.tsapps/desktop/src/trpc/routers/models.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/trpc/routers/models.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/services/transcription-service.ts
Comment on lines
+582
to
+588
| const transcriptionService = this.getTranscriptionService(); | ||
| if (!transcriptionService) { | ||
| throw new AppError( | ||
| "Transcription service unavailable", | ||
| ErrorCodes.WORKER_INITIALIZATION_FAILED, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Duplicate notification when transcription service is unavailable.
When the service is unavailable, getTranscriptionService() emits a widget-notification (lines 887-890) before returning null. The subsequent AppError thrown here is caught by the catch block, which emits another widget-notification at line 613. This results in the user seeing duplicate "transcription_failed" notifications.
Consider handling the unavailability inline to avoid the throw/catch path that emits a second notification:
🐛 Proposed fix
// NORMAL - get transcription and paste
let result = "";
try {
const transcriptionService = this.getTranscriptionService();
if (!transcriptionService) {
- throw new AppError(
- "Transcription service unavailable",
- ErrorCodes.WORKER_INITIALIZATION_FAILED,
- );
+ // Notification already emitted by getTranscriptionService()
+ this.resetSessionState();
+ this.setState("idle");
+ return;
}
result = await transcriptionService.finalizeSession({📝 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.
Suggested change
| const transcriptionService = this.getTranscriptionService(); | |
| if (!transcriptionService) { | |
| throw new AppError( | |
| "Transcription service unavailable", | |
| ErrorCodes.WORKER_INITIALIZATION_FAILED, | |
| ); | |
| } | |
| const transcriptionService = this.getTranscriptionService(); | |
| if (!transcriptionService) { | |
| // Notification already emitted by getTranscriptionService() | |
| this.resetSessionState(); | |
| this.setState("idle"); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/managers/recording-manager.ts` around lines 582 - 588,
getTranscriptionService() already emits the "transcription_failed"
widget-notification and returns null, so throwing an AppError here causes the
catch block to emit a second notification; instead of throwing from this
location, handle the null inline: check the result of getTranscriptionService(),
emit (or rely on the existing) notification once, and perform an early
return/cleanup from the current method (the surrounding function in
recording-manager.ts that calls getTranscriptionService()) so the catch path is
not triggered. Ensure you remove the throw of AppError here and keep any
necessary cleanup logic locally rather than relying on the outer catch to
notify.
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 #45
Summary by CodeRabbit
New Features
Improvements
Tests