Feature/remote whisper provider - #731
Conversation
There was a problem hiding this comment.
Peer review — Remote Whisper provider
Solid work. The four commits are cleanly separated, the messages explain the why rather than the what, and the change fits the existing abstractions (TranscriptionProvider trait, reuse of the model column, serde-defaulted status fields with round-trip tests). Nothing here breaks the existing local flow.
One issue I'd fix before merge, plus a few non-blocking follow-ups. Details inline.
🔴 Health check couples to a contract the provider doesn't promise
is_model_loaded() probes GET /health, but transcription uses POST /v1/audio/transcriptions. /health is not part of the OpenAI /v1/audio/transcriptions convention — it's specific to whisper.cpp-server / faster-whisper-server. A server that correctly implements /v1/audio/transcriptions but doesn't expose /health (OpenAI's own API, a minimal proxy, a custom gateway) fails onboarding validation and the recording gate even though it transcribes fine — and since Continue is gated on the probe passing, that user is stuck in the wizard with no obvious way out. Prefer GET /v1/models (part of the OpenAI contract), or fall back to it on 404, or at minimum document that the server MUST expose /health.
🟡 Warnings
encode_wav_pcm16:data_size/riff_sizeareu32with unchecked arithmetic — silent wrap on absurdly long buffers in release. Theoretical for second-long chunks; adebug_assert!/saturating_adddocuments the invariant for free.- No retry on a transient network blip: the local path couldn't lose a chunk this way; the remote one drops the whole POST on a momentary LAN hiccup. One short-backoff retry would avoid gaps in live transcripts. Robustness regression vs local, not a blocker.
REQUEST_TIMEOUT_SECS = 60is fixed and not configurable — a large chunk on a busy server is cut and lost.
🟢 Well done
Language coercion lives in ConfigContext (not the settings panel) so provider switches without opening the modal stay consistent; the user's real preference is preserved and restored; the double-write in handleSetSelectedLanguage was removed to avoid the race; serde backward-compat is covered by two tests; hasLoadedStatusRef guards the debounced save and is released in finally; the three call sites are unified so they can't drift.
No HTTP-path test for transcribe() (would need a mock server) — acceptable, but a wiremock/httpmock fake would close the gap on the multipart + {"text": ...} parsing.
Reviewed by Hermes (agent) — model: claude-opus-4-8 (Anthropic), via the Hermes gateway — on Germani's behalf.
f66bc8a to
1246c0a
Compare
Adds a transcription provider that sends audio to a self-hosted, OpenAI-compatible Whisper server (POST /v1/audio/transcriptions) instead of loading a model in-process. This lets machines without a capable GPU offload transcription to one that has it, and keeps the "runs on your own infrastructure" property that local models provide. The server URL is stored in the existing `model` column of `transcript_settings`, so no schema change is needed. No API key is required, since the server is the user's own. Language handling needs care: Meetily's picker emits `auto` and `auto-translate` as sentinels for "let the engine decide". These are not ISO-639-1 codes, and an OpenAI-compatible server rejects them outright (faster-whisper answers 500). Omitting the field is exactly how that API is asked to auto-detect, so the sentinels are dropped rather than forwarded. `auto-translate` additionally asks for translation to English, which /v1/audio/transcriptions cannot do — translation lives behind a separate /v1/audio/translations endpoint. Rather than silently returning untranslated text as if it had been translated, the option is hidden for this provider and the stored preference is coerced on its way to the backend. The user's saved choice is left untouched, so switching back to a provider that can translate restores it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oads Onboarding assumed both engines run locally: it downloaded Parakeet (~670 MB) and a summary model unconditionally, and gated "Continue" on the transcription download finishing. A user who already runs a Whisper server, or who intends to use OpenAI/Claude/Ollama for summaries, had no way through the wizard without downloading models they would never use. Setup Overview now offers those choices behind a collapsed "Already running your own AI servers?" disclosure. Collapsed by default, so the default path is unchanged for everyone else. Choosing a remote transcription server requires testing the connection before continuing, and editing the URL invalidates a previous result — a green check must never authorise a URL the user has since changed. Downloads are effectively irreversible at this size, so it is worth proving the alternative works first. `complete_onboarding` takes `model: Option<String>`; `None` means the user brings their own summary provider, and no builtin-ai config is written. `ModelStatus` gains two optional fields recording the transcription choice; both use serde defaults, so statuses written by earlier versions still load (covered by tests). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gate that runs before every recording asked `parakeet_init` + `parakeet_has_available_models` regardless of which provider was configured. That was harmless while every working provider was local, but it makes remote transcription unusable: a user with a remote server and no local model is told "Transcription model not ready" on every attempt and is shown a model-download dialog that offers nothing for a remote server. The check now reads the configured provider. A remote server only needs to be reachable — it owns its own model lifecycle — while the local path is unchanged. If the config cannot be read, it falls back to the local check rather than granting access to a backend it could not confirm. An unreachable server no longer opens the download dialog, since downloading a local model does not fix a server that is down; the message names the URL and points at Settings instead. The three call sites (button, auto-start, sidebar) shared a copy-pasted block; they now share one readiness check and one reporting function, so they cannot drift apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both the README and the architecture overview stated that transcription always runs locally. That is no longer the whole picture now that a remote provider exists, and leaving it unqualified would be misleading. The wording keeps local as the default and frames the remote option as what it is: the work moves to another machine the user controls, not to someone else's cloud. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The readiness probe only tried `GET /health`. That is a whisper.cpp-server and faster-whisper-server extension, not part of the OpenAI-compatible contract this provider targets — OpenAI's own API answers 404 there. A server that implements `/v1/audio/transcriptions` correctly but exposes no `/health` was therefore reported dead, and since onboarding blocks "Continue" on this probe, such a user had no way through the wizard. Neither endpoint covers the field alone: a minimal Whisper server often has `/health` and no `/v1/models`, while an OpenAI-compatible gateway is usually the reverse. Both are now tried in turn. 401 and 403 count as alive. An endpoint that demands credentials has still proven it exists and answers, and treating it as dead would reintroduce the same false negative in a different shape. Also switches the WAV header arithmetic to checked conversion and saturating operations. The sizes are u32 and the current chunks are seconds long, so wrapping is unreachable today; the point is that the header can no longer silently disagree with the payload if that changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1246c0a to
2e4683a
Compare
Description
Adds a third transcription provider,
remoteWhisper, that sends audio to aself-hosted OpenAI-compatible server (
POST /v1/audio/transcriptions) instead ofloading a model in-process. Audio still never leaves infrastructure the user
controls — it just runs on a machine with a stronger GPU.
Three pieces, one per commit:
modelcolumn of
transcript_settings(no schema change), needs no API key.lets you pick the remote backend, or defer the summary model, without
downloading ~670 MB you will never use. The default path is unchanged.
useRecordingStart.tschecked for a local Parakeetmodel regardless of the configured provider, which made the feature unusable.
They ship together because 1 without 3 is a dead end: the gate blocks every
recording attempt and offers a download dialog that has no option for a remote
server.
Notes for reviewers
auto/auto-translate, which are notISO-639-1 codes; forwarding them makes faster-whisper answer
500. They aredropped, which is exactly how that API is asked to auto-detect.
auto-translateis hidden for this provider./v1/audio/transcriptionstranscribes in the source language — translation lives behind
/v1/audio/translations. Rather than return untranslated text as if it had beentranslated, the option is hidden and the stored preference is coerced on its way
to the backend, so switching back to Local Whisper restores the user's choice.
/healthproves reachability, not readiness. The green check in onboardingmeans the server answered; it cannot promise the next transcription succeeds.
Observed in practice: a server returning
200 {"status":"ok"}while inferencewas broken.
complete_onboardingsignature changed (model: String→Option<String>);Nonemeans the user brings their own summary provider.ModelStatusgains twooptional fields; both use serde defaults, so statuses written by earlier versions
still load — covered by tests.
Related Issue
Fixes #637
Implements #612, #301, #225, #527
Type of Change
Testing
7 unit tests: language sentinel handling, WAV header shape, and onboarding status
backward compatibility. Manually tested on macOS (Apple Silicon) against a
faster-whisper server running
large-v3-turboon an RTX 3060 — onboarding withthe remote backend, recording, and live transcription over several sessions.
Documentation
Checklist
Screenshots
Additional Notes
The recording-gate fix (#637) affects users who never touch remote transcription:
it also unblocks anyone using Local Whisper, which is what that issue reports.