-
Notifications
You must be signed in to change notification settings - Fork 537
feat(desktop): improve audio import experience and error handling #3899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4601ab2
78d369f
4674a92
f33edd6
558c510
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,7 @@ import { | |||||||||||||||||||||||||||||||||||||
| TooltipTrigger, | ||||||||||||||||||||||||||||||||||||||
| } from "@hypr/ui/components/ui/tooltip"; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import { useListener } from "../../../../../contexts/listener"; | ||||||||||||||||||||||||||||||||||||||
| import { fromResult } from "../../../../../effect"; | ||||||||||||||||||||||||||||||||||||||
| import { useRunBatch } from "../../../../../hooks/useRunBatch"; | ||||||||||||||||||||||||||||||||||||||
| import * as main from "../../../../../store/tinybase/store/main"; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -45,6 +46,9 @@ export function OptionsMenu({ | |||||||||||||||||||||||||||||||||||||
| const [open, setOpen] = useState(false); | ||||||||||||||||||||||||||||||||||||||
| const runBatch = useRunBatch(sessionId); | ||||||||||||||||||||||||||||||||||||||
| const queryClient = useQueryClient(); | ||||||||||||||||||||||||||||||||||||||
| const handleBatchStarted = useListener((state) => state.handleBatchStarted); | ||||||||||||||||||||||||||||||||||||||
| const handleBatchFailed = useListener((state) => state.handleBatchFailed); | ||||||||||||||||||||||||||||||||||||||
| const clearBatchSession = useListener((state) => state.clearBatchSession); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const store = main.UI.useStore(main.STORE_ID); | ||||||||||||||||||||||||||||||||||||||
| const { user_id } = main.UI.useValues(main.STORE_ID); | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -139,7 +143,18 @@ export function OptionsMenu({ | |||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return pipe( | ||||||||||||||||||||||||||||||||||||||
| fromResult(fsSyncCommands.audioImport(sessionId, path)), | ||||||||||||||||||||||||||||||||||||||
| Effect.sync(() => { | ||||||||||||||||||||||||||||||||||||||
| if (sessionTab) { | ||||||||||||||||||||||||||||||||||||||
| updateSessionTabState(sessionTab, { | ||||||||||||||||||||||||||||||||||||||
| ...sessionTab.state, | ||||||||||||||||||||||||||||||||||||||
| view: { type: "transcript" }, | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| handleBatchStarted(sessionId); | ||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||
| Effect.flatMap(() => | ||||||||||||||||||||||||||||||||||||||
| fromResult(fsSyncCommands.audioImport(sessionId, path)), | ||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||
| Effect.tap(() => | ||||||||||||||||||||||||||||||||||||||
| Effect.sync(() => { | ||||||||||||||||||||||||||||||||||||||
| void analyticsCommands.event({ | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -154,10 +169,20 @@ export function OptionsMenu({ | |||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||
| Effect.tap(() => Effect.sync(() => clearBatchSession(sessionId))), | ||||||||||||||||||||||||||||||||||||||
| Effect.flatMap(() => Effect.promise(() => runBatch(path))), | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+172
to
173
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 clearBatchSession before runBatch causes transient 'inactive' session mode, potentially dropping transcript tab
Detailed ExplanationThe if (sessionMode === "active" || sessionMode === "running_batch") {
const tabs: EditorView[] = [{ type: "raw" }, { type: "transcript" }];
return tabs;
}When Similarly, While React 18 batching may mitigate this in most cases (since Impact: Potential UI flicker where the transcript tab disappears and reappears, or the empty state briefly shows the wrong message. Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||||||||||
| Effect.catchAll((error: unknown) => | ||||||||||||||||||||||||||||||||||||||
| Effect.sync(() => { | ||||||||||||||||||||||||||||||||||||||
| const msg = error instanceof Error ? error.message : String(error); | ||||||||||||||||||||||||||||||||||||||
| handleBatchFailed(sessionId, msg); | ||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
173
to
+178
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 The Root Cause and ImpactThe When At that point, For errors originating inside Fix: Replace Effect.flatMap(() => Effect.tryPromise({
try: () => runBatch(path),
catch: (error) => error,
})),
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| [ | ||||||||||||||||||||||||||||||||||||||
| clearBatchSession, | ||||||||||||||||||||||||||||||||||||||
| handleBatchFailed, | ||||||||||||||||||||||||||||||||||||||
| handleBatchStarted, | ||||||||||||||||||||||||||||||||||||||
| queryClient, | ||||||||||||||||||||||||||||||||||||||
| runBatch, | ||||||||||||||||||||||||||||||||||||||
| sessionId, | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,18 @@ | ||
| import { AudioLinesIcon } from "lucide-react"; | ||
|
|
||
| export function TranscriptEmptyState() { | ||
| import { Spinner } from "@hypr/ui/components/ui/spinner"; | ||
|
|
||
| export function TranscriptEmptyState({ isBatching }: { isBatching?: boolean }) { | ||
| return ( | ||
| <div className="h-full flex flex-col items-center justify-center gap-3 text-neutral-400"> | ||
| <AudioLinesIcon className="w-8 h-8" /> | ||
| <p className="text-sm">No transcript available</p> | ||
| {isBatching ? ( | ||
| <Spinner size={28} /> | ||
| ) : ( | ||
| <AudioLinesIcon className="w-8 h-8" /> | ||
| )} | ||
| <p className="text-sm"> | ||
| {isBatching ? "Generating transcript..." : "No transcript available"} | ||
| </p> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,7 +16,7 @@ export function TranscriptionProgress({ sessionId }: { sessionId: string }) { | |||||||
|
|
||||||||
| const statusLabel = useMemo(() => { | ||||||||
| if (!progressRaw || progressRaw.percentage === 0) { | ||||||||
| return "..."; | ||||||||
| return "Importing audio..."; | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 "Importing audio..." label shown during batch transcription phase (not just audio import) The Root Cause and ImpactThe This also affects the "redo transcript" flow ( Actual: User sees "Importing audio..." during the transcription phase and during redo operations.
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||
| } | ||||||||
|
|
||||||||
| const percent = Math.round(progressRaw.percentage * 100); | ||||||||
|
|
@@ -45,7 +45,11 @@ export function TranscriptionProgress({ sessionId }: { sessionId: string }) { | |||||||
| <div className="mb-3"> | ||||||||
| <div className="flex w-fit items-center gap-2 rounded-full bg-neutral-900 px-3 py-1 text-xs text-white shadow-xs"> | ||||||||
| <Spinner size={12} className="text-white/80" /> | ||||||||
| <span>Processing · {statusLabel}</span> | ||||||||
| <span> | ||||||||
| {!progressRaw || progressRaw.percentage === 0 | ||||||||
| ? statusLabel | ||||||||
| : `Processing · ${statusLabel}`} | ||||||||
| </span> | ||||||||
| </div> | ||||||||
| </div> | ||||||||
| ); | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 clearBatchSession before runBatch causes silent failure when STT connection is unavailable
When an audio file is imported,
clearBatchSession(sessionId)is called at line 172 beforerunBatch(path)at line 173. InsideuseRunBatch(hooks/useRunBatch.ts:69-71), if!store || !conn || !runBatch, the callback returnsundefinedwithout throwing. SinceEffect.promise(() => runBatch(path))resolves successfully withundefined, thecatchAllerror 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 returnsundefinedAt this point:
runBatchdidn't create a new batch entry or throw an errorcatchAlldoesn't fire because the promise resolvedTranscriptionProgresscomponent shows nothing — no progress indicator, no error messageThe user sees the "Importing audio..." indicator appear, then disappear, with no indication that transcription failed to start. The
connbeing 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
Was this helpful? React with 👍 or 👎 to provide feedback.