feat(desktop): improve audio import experience and error handling#3899
feat(desktop): improve audio import experience and error handling#3899ComputelessComputer merged 5 commits intomainfrom
Conversation
✅ Deploy Preview for hyprnote-storybook canceled.
|
✅ Deploy Preview for hyprnote canceled.
|
ComputelessComputer
force-pushed
the
feat/improve-audio-import
branch
from
11b0801 to
e2e3d8f
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| Effect.tap(() => Effect.sync(() => clearBatchSession(sessionId))), | ||
| Effect.flatMap(() => Effect.promise(() => runBatch(path))), |
There was a problem hiding this comment.
🔴 clearBatchSession before runBatch causes silent failure when STT connection is unavailable
When an audio file is imported, clearBatchSession(sessionId) is called at line 172 before runBatch(path) at line 173. Inside useRunBatch (hooks/useRunBatch.ts:69-71), if !store || !conn || !runBatch, the callback returns undefined without throwing. Since Effect.promise(() => runBatch(path)) resolves successfully with undefined, the catchAll error handler never fires.
Root Cause and Impact
The sequence of operations is:
handleBatchStarted(sessionId)— creates batch entry with{ percentage: 0 }audioImportsucceeds — audio file is importedclearBatchSession(sessionId)— removes the batch entry entirelyrunBatch(path)— if STT connection (conn) is null (e.g., no provider configured, local model not downloaded, cloud model without auth),useRunBatch's callback silently returnsundefined
At this point:
- The audio was imported successfully
- The batch entry was cleared (step 3)
runBatchdidn't create a new batch entry or throw an error- The
catchAlldoesn't fire because the promise resolved - The
TranscriptionProgresscomponent shows nothing — no progress indicator, no error message
The user sees the "Importing audio..." indicator appear, then disappear, with no indication that transcription failed to start. The conn being null is a common scenario — it happens whenever the user hasn't fully configured their STT provider, the local model isn't downloaded yet, or the cloud auth session expired.
Impact: Users who import audio without a working STT connection get no error feedback. The audio is imported but transcription silently doesn't start.
Prompt for agents
The fix should ensure that if runBatch resolves without starting transcription (silent early return), the user gets error feedback. There are two approaches:
1. (Preferred) In hooks/useRunBatch.ts lines 69-71, change the early return to throw an Error instead of silently returning:
if (!store || !conn || !runBatch) {
throw new Error("No STT connection available. Please configure a speech-to-text provider.");
}
This way the Effect.promise wrapper will reject, and the catchAll in options-menu.tsx will call handleBatchFailed with a meaningful message.
2. (Alternative) In options-menu.tsx, after the Effect.promise(() => runBatch(path)) step, add a check that verifies the batch actually started, and if not, call handleBatchFailed with an appropriate message. For example, wrap runBatch in a helper that detects the silent return and throws.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const statusLabel = useMemo(() => { | ||
| if (!progressRaw || progressRaw.percentage === 0) { | ||
| return "..."; | ||
| return "Importing audio..."; |
There was a problem hiding this comment.
🟡 "Importing audio..." label shown during batch transcription phase (not just audio import)
The statusLabel in TranscriptionProgress returns "Importing audio..." whenever progressRaw is null or progressRaw.percentage === 0. After the audio import completes, clearBatchSession is called (options-menu.tsx:172), which removes the batch entry. Then runBatch is called (options-menu.tsx:173), which internally calls handleBatchStarted (general.ts:482), resetting the batch state to { percentage: 0, isComplete: false }. At this point, the audio import is already done and transcription is starting, but the progress indicator still shows "Importing audio..." until the first batchProgress event arrives with a non-zero percentage.
Root Cause and Impact
The TranscriptionProgress component has no way to distinguish between the "importing audio" phase and the "starting transcription" phase — both have percentage: 0. The label "Importing audio..." is hardcoded for this state at progress.tsx:19.
This also affects the "redo transcript" flow (header.tsx:154: await runBatch(audioPath)), where no audio import happens at all. In that case, the user sees "Importing audio..." when they're just re-running transcription on existing audio.
Actual: User sees "Importing audio..." during the transcription phase and during redo operations.
Expected: The label should reflect the actual phase — e.g., "Processing..." or "Starting transcription..." when transcription is beginning, and "Importing audio..." only during actual audio import.
| return "Importing audio..."; | |
| return "Processing..."; | |
Was this helpful? React with 👍 or 👎 to provide feedback.
| Effect.tap(() => Effect.sync(() => clearBatchSession(sessionId))), | ||
| Effect.flatMap(() => Effect.promise(() => runBatch(path))), |
There was a problem hiding this comment.
🟡 clearBatchSession before runBatch causes transient 'inactive' session mode, potentially dropping transcript tab
clearBatchSession(sessionId) at options-menu.tsx:172 removes the batch entry from state, causing getSessionMode to return "inactive". The subsequent runBatch(path) at options-menu.tsx:173 re-creates the batch entry via handleBatchStarted inside general.ts:482. Between these two calls, any React re-render would see sessionMode === "inactive".
Detailed Explanation
The useEditorTabs hook at header.tsx:633 uses sessionMode to determine which tabs to show:
if (sessionMode === "active" || sessionMode === "running_batch") {
const tabs: EditorView[] = [{ type: "raw" }, { type: "transcript" }];
return tabs;
}When sessionMode is "inactive" (after clearBatchSession but before runBatch re-establishes the batch state), and there's no transcript yet (which is the case during first audio import), useEditorTabs returns [{ type: "raw" }] — without the transcript tab. This could cause the view to briefly switch away from the transcript tab.
Similarly, TranscriptEmptyState at shared/index.tsx:130 would show "No transcript available" instead of "Generating transcript..." during this window.
While React 18 batching may mitigate this in most cases (since clearBatchSession and handleBatchStarted inside runBatch may run in the same synchronous block), the Effect.promise wrapper and async function boundaries make this timing-dependent and fragile.
Impact: Potential UI flicker where the transcript tab disappears and reappears, or the empty state briefly shows the wrong message.
Prompt for agents
Instead of calling clearBatchSession before runBatch (which creates a window where the session mode is 'inactive'), remove the clearBatchSession call at options-menu.tsx:172 entirely. The runBatch function in general.ts already handles the batch state lifecycle internally (it calls handleBatchStarted at line 482 and clearBatchSession on completion). The initial handleBatchStarted at options-menu.tsx:153 should be kept to show the importing state, but the clearBatchSession should not be called before runBatch. Instead, either: (1) add a phase/stage field to the batch state to distinguish 'importing' from 'transcribing', or (2) let runBatch's internal handleBatchStarted reset the percentage naturally without clearing first. The guard at general.ts:469-474 that prevents running batch when already in running_batch mode would need to be adjusted to allow re-starting from the same flow (e.g., by passing a flag or checking if the caller is the same audio import flow).
Was this helpful? React with 👍 or 👎 to provide feedback.
ComputelessComputer
force-pushed
the
feat/improve-audio-import
branch
from
45a460b to
ba8f3c7
Compare
| Effect.flatMap(() => Effect.promise(() => runBatch(path))), | ||
| Effect.catchAll((error: unknown) => | ||
| Effect.sync(() => { | ||
| const msg = error instanceof Error ? error.message : String(error); | ||
| handleBatchFailed(sessionId, msg); | ||
| }), |
There was a problem hiding this comment.
🔴 Effect.catchAll cannot catch rejections from Effect.promise, so runBatch errors bypass the error handler
The catchAll at line 174 is intended to catch errors from both audioImport and runBatch, but Effect.promise (line 173) treats promise rejections as defects (unexpected errors), not recoverable errors. Effect.catchAll only catches recoverable errors (from Effect.fail/Effect.tryPromise), not defects.
Root Cause and Impact
The fromResult helper at apps/desktop/src/effect.ts:5-17 uses Effect.tryPromise, which maps rejections into the error channel (E) — catchable by Effect.catchAll. However, Effect.promise at line 173 maps rejections into defects (Cause.Die) — NOT catchable by Effect.catchAll.
When runBatch(path) rejects (e.g., the provider check at apps/desktop/src/hooks/useRunBatch.ts:77 throws "Batch transcription is not supported for provider: ..." before state.runBatch is even called), the error becomes a defect. Effect.catchAll doesn't intercept it, so handleBatchFailed is never invoked. The defect propagates to Effect.runPromise, which rejects, and the outer .catch at line 226 only logs the error to the console.
At that point, clearBatchSession (line 172) has already removed the batch entry, so the user sees "No transcript available" with no error feedback — even though the audio was imported but transcription silently failed.
For errors originating inside state.runBatch (in general.ts), handleBatchFailed IS called internally before rejecting, partially mitigating the issue. But for early failures in useRunBatch (like an unsupported provider), the error is completely swallowed from the user's perspective.
Fix: Replace Effect.promise with Effect.tryPromise so rejections enter the error channel:
Effect.flatMap(() => Effect.tryPromise({
try: () => runBatch(path),
catch: (error) => error,
})),| Effect.flatMap(() => Effect.promise(() => runBatch(path))), | |
| Effect.catchAll((error: unknown) => | |
| Effect.sync(() => { | |
| const msg = error instanceof Error ? error.message : String(error); | |
| handleBatchFailed(sessionId, msg); | |
| }), | |
| Effect.flatMap(() => | |
| Effect.tryPromise({ | |
| try: () => runBatch(path), | |
| catch: (error) => error, | |
| }), | |
| ), | |
| Effect.catchAll((error: unknown) => | |
| Effect.sync(() => { | |
| const msg = error instanceof Error ? error.message : String(error); | |
| handleBatchFailed(sessionId, msg); | |
| }), | |
| ), |
Was this helpful? React with 👍 or 👎 to provide feedback.
ComputelessComputer
force-pushed
the
feat/improve-audio-import
branch
from
ba8f3c7 to
558c510
Compare
devin-ai-integration
bot
changed the title
1 participant
Summary
Review & Testing Checklist for Human
Notes
Part 1 of 3 in a stack (#3899 → #3903 → #3906). Review this PR first.
Link to Devin run: https://app.devin.ai/sessions/1b9ae9acc87349e393883ca54ed961c6
Requested by: @ComputelessComputer
This is part 1 of 3 in a stack made with GitButler: