From 08a9bff8418809e50568b8d73dc9625bfceee412 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Tue, 30 Jun 2026 17:17:28 +0530 Subject: [PATCH] fix(voice): in-process Whisper STT works without an external binary on macOS (#3425) (#3915) Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: M3gA-Mind --- .../components/settings/panels/VoicePanel.tsx | 17 +- .../panels/__tests__/VoicePanel.test.tsx | 71 ++++++ app/src/features/human/MicComposer.test.tsx | 31 +++ app/src/features/human/MicComposer.tsx | 28 +++ .../inference/local/service/whisper_engine.rs | 92 ++++++++ src/openhuman/inference/paths.rs | 150 ++++++++++++- src/openhuman/voice/factory/stt_providers.rs | 203 +++++++++++++++++- .../voice/schemas/handlers/provider_server.rs | 4 +- src/openhuman/voice/schemas/helpers.rs | 15 +- src/openhuman/voice/schemas_tests.rs | 10 +- 10 files changed, 600 insertions(+), 21 deletions(-) diff --git a/app/src/components/settings/panels/VoicePanel.tsx b/app/src/components/settings/panels/VoicePanel.tsx index 5c1510c80..94709ce96 100644 --- a/app/src/components/settings/panels/VoicePanel.tsx +++ b/app/src/components/settings/panels/VoicePanel.tsx @@ -553,6 +553,13 @@ const VoicePanel = ({ embedded = false }: VoicePanelProps = {}) => { const pendingLocalProviderReady = pendingKeySlug === 'whisper' ? whisperReady : pendingKeySlug === 'piper' ? piperReady : true; + // A local engine must finish downloading before its Test button does + // anything useful — exercising an un-installed Whisper/Piper just errors + // out on a missing model/binary. Cloud + external providers carry no + // local artifact, so they are never gated here. + const sttTestBlockedByInstall = sttProvider === 'whisper' && !whisperReady; + const ttsTestBlockedByInstall = ttsProvider === 'piper' && !piperReady; + return ( { variant="secondary" size="xs" data-testid="test-stt-button" - disabled={isTestingStt || !sttProvider} + disabled={isTestingStt || !sttProvider || sttTestBlockedByInstall} + title={ + sttTestBlockedByInstall ? t('voice.providers.notInstalled') : undefined + } onClick={async () => { setIsTestingStt(true); setSttTestResult(null); @@ -1115,7 +1125,10 @@ const VoicePanel = ({ embedded = false }: VoicePanelProps = {}) => { variant="secondary" size="xs" data-testid="test-tts-button" - disabled={isTestingTts || !ttsProvider} + disabled={isTestingTts || !ttsProvider || ttsTestBlockedByInstall} + title={ + ttsTestBlockedByInstall ? t('voice.providers.notInstalled') : undefined + } onClick={async () => { setIsTestingTts(true); setTtsTestResult(null); diff --git a/app/src/components/settings/panels/__tests__/VoicePanel.test.tsx b/app/src/components/settings/panels/__tests__/VoicePanel.test.tsx index b1b5f07f6..cb92706fe 100644 --- a/app/src/components/settings/panels/__tests__/VoicePanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/VoicePanel.test.tsx @@ -619,6 +619,77 @@ describe('VoicePanel', () => { ); }); + // ─── Test buttons gate on local-model install completion ──────────────────── + + it('disables Test STT while the selected Whisper model is not installed', async () => { + runtime.voiceSettings = makeVoiceSettings({ + sttProvider: { kind: 'local', engine: 'whisper', model: 'medium' }, + }); + runtime.whisperStatus = makeInstallStatus('whisper', { state: 'missing' }); + // A missing model with no runtime availability is the genuine + // "not installed" case the Test button must gate on; `whisperReady` + // also clears once `stt_available` reports a usable engine. + runtime.voiceStatus.stt_available = false; + + renderWithProviders(, { initialEntries: ['/settings/voice'] }); + + const sttSelect = (await screen.findByTestId('stt-provider-select')) as HTMLSelectElement; + await waitFor(() => expect(sttSelect.value).toBe('whisper')); + + expect(await screen.findByTestId('test-stt-button')).toBeDisabled(); + }); + + it('enables Test STT once the selected Whisper model is installed', async () => { + runtime.voiceSettings = makeVoiceSettings({ + sttProvider: { kind: 'local', engine: 'whisper', model: 'medium' }, + }); + runtime.whisperStatus = makeInstallStatus('whisper', { state: 'installed', progress: 100 }); + + renderWithProviders(, { initialEntries: ['/settings/voice'] }); + + const sttSelect = (await screen.findByTestId('stt-provider-select')) as HTMLSelectElement; + await waitFor(() => expect(sttSelect.value).toBe('whisper')); + + const testSttBtn = await screen.findByTestId('test-stt-button'); + await waitFor(() => expect(testSttBtn).toBeEnabled()); + + fireEvent.click(testSttBtn); + await waitFor(() => + expect(vi.mocked(testVoiceProvider)).toHaveBeenCalledWith('stt', 'whisper') + ); + }); + + it('disables Test TTS while the selected Piper voice is not installed', async () => { + runtime.voiceSettings = makeVoiceSettings({ + ttsProvider: { kind: 'local', engine: 'piper', model: '' }, + }); + runtime.piperStatus = makeInstallStatus('piper', { state: 'missing' }); + // Mirror the STT gate: no installed voice and no runtime availability is + // the real "not installed" case; `piperReady` also keys off `tts_available`. + runtime.voiceStatus.tts_available = false; + + renderWithProviders(, { initialEntries: ['/settings/voice'] }); + + const ttsSelect = (await screen.findByTestId('tts-provider-select')) as HTMLSelectElement; + await waitFor(() => expect(ttsSelect.value).toBe('piper')); + + expect(await screen.findByTestId('test-tts-button')).toBeDisabled(); + }); + + it('enables Test TTS once the selected Piper voice is installed', async () => { + runtime.voiceSettings = makeVoiceSettings({ + ttsProvider: { kind: 'local', engine: 'piper', model: '' }, + }); + runtime.piperStatus = makeInstallStatus('piper', { state: 'installed', progress: 100 }); + + renderWithProviders(, { initialEntries: ['/settings/voice'] }); + + const ttsSelect = (await screen.findByTestId('tts-provider-select')) as HTMLSelectElement; + await waitFor(() => expect(ttsSelect.value).toBe('piper')); + + await waitFor(() => expect(screen.getByTestId('test-tts-button')).toBeEnabled()); + }); + // ─── Whisper model picker in routing section ──────────────────────────────── it('changing the Whisper model select immediately calls persistProviders', async () => { diff --git a/app/src/features/human/MicComposer.test.tsx b/app/src/features/human/MicComposer.test.tsx index 641b31679..8d93b3ee0 100644 --- a/app/src/features/human/MicComposer.test.tsx +++ b/app/src/features/human/MicComposer.test.tsx @@ -626,6 +626,37 @@ describe('MicComposer', () => { expect(encodeBlobToWavMock).toHaveBeenCalledTimes(1); }); + it('skips native retries on a missing local binary and falls straight to WAV/in-process', async () => { + // On a no-binary macOS install (#3425) the native webm codec routes to the + // whisper-cli subprocess and errors "binary not found". That can never + // succeed on retry, and only 16kHz WAV can use the in-process engine — so + // the native codec must bail immediately (no backoff) and the WAV re-encode + // must run, where the in-process route transcribes with no external binary. + transcribeWithFactoryMock + .mockRejectedValueOnce( + new Error('[voice-stt] whisper.cpp binary not found. Set WHISPER_BIN to the absolute path…') + ) + .mockResolvedValueOnce('in-process ok'); + encodeBlobToWavMock.mockResolvedValueOnce( + new Blob([new Uint8Array([0])], { type: 'audio/wav' }) + ); + const onSubmit = vi.fn(); + render(); + + fireEvent.click(screen.getByRole('button', { name: /start recording/i })); + await waitFor(() => expect(getUserMediaMock).toHaveBeenCalled()); + await waitFor(() => + expect(screen.getByRole('button', { name: /stop recording and send/i })).toBeInTheDocument() + ); + fireEvent.click(screen.getByRole('button', { name: /stop recording and send/i })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledWith('in-process ok')); + // Exactly 1 native attempt (no retries) + 1 WAV attempt — the missing + // binary short-circuits the native backoff loop. + expect(transcribeWithFactoryMock).toHaveBeenCalledTimes(2); + expect(encodeBlobToWavMock).toHaveBeenCalledTimes(1); + }); + it('does not continue retrying after unmount during STT backoff', async () => { vi.useFakeTimers({ shouldAdvanceTime: true }); transcribeWithFactoryMock diff --git a/app/src/features/human/MicComposer.tsx b/app/src/features/human/MicComposer.tsx index eabee519c..6755bbb14 100644 --- a/app/src/features/human/MicComposer.tsx +++ b/app/src/features/human/MicComposer.tsx @@ -54,6 +54,21 @@ function isTransientError(err: unknown): boolean { return !PERMANENT_ERROR_PATTERNS.some(p => msg.includes(p)); } +/** + * A missing local STT binary (`whisper-cli`) won't reappear on retry, and the + * native MediaRecorder codec (webm/mp4) can't use the binary-free in-process + * engine — only 16kHz WAV can. This error must therefore stop the *current* + * codec's retry loop immediately (no wasted backoff), yet still let + * `transcribeWithFallback` re-encode to WAV and reach the in-process route. + * It is deliberately NOT a `PERMANENT_ERROR_PATTERN`: those bail out before the + * WAV fallback, which is exactly the path that makes local STT work without an + * external binary on macOS (issue #3425). + */ +function isMissingLocalBinaryError(err: unknown): boolean { + const msg = (err instanceof Error ? err.message : String(err)).toLowerCase(); + return msg.includes('binary not found'); +} + /** * Heuristic check for low-confidence transcripts. The backend doesn't expose * a confidence score, so we detect likely bad results from text patterns: @@ -565,6 +580,19 @@ export function MicComposer({ ); throw err; } + if (isMissingLocalBinaryError(err)) { + // The local subprocess binary is absent — retrying this codec is + // pointless. Bail out now so the caller re-encodes to 16kHz WAV and + // uses the in-process engine instead of burning the backoff budget. + composerLog( + '[session:%d] transcribe missing local binary path=%s attempt=%d — skipping retries for WAV/in-process fallback: %s', + sessionIdRef.current, + label, + attempt, + msg + ); + throw err; + } if (attempt < STT_MAX_RETRIES) { const delay = STT_RETRY_BASE_MS * Math.pow(2, attempt); composerLog( diff --git a/src/openhuman/inference/local/service/whisper_engine.rs b/src/openhuman/inference/local/service/whisper_engine.rs index d9f7d3449..02d552a5f 100644 --- a/src/openhuman/inference/local/service/whisper_engine.rs +++ b/src/openhuman/inference/local/service/whisper_engine.rs @@ -328,6 +328,33 @@ pub fn transcribe_wav_file( transcribe_pcm_f32(handle, &audio_f32, language, initial_prompt) } +/// Cheap header sniff: does this blob look like a RIFF/WAVE container? +/// +/// Used by callers (e.g. the voice STT factory) to decide whether to even +/// attempt the in-process engine — which only understands 16 kHz WAV — or +/// hand the blob straight to the container-aware `whisper-cli` subprocess. +/// This does NOT validate the sample rate; `transcribe_wav_bytes` does that +/// during decode and errors if it isn't 16 kHz, so callers fall back then. +pub(crate) fn looks_like_wav(bytes: &[u8]) -> bool { + bytes.len() >= 12 && &bytes[0..4] == b"RIFF" && &bytes[8..12] == b"WAVE" +} + +/// Decode in-memory 16 kHz mono WAV bytes and transcribe in-process. +/// +/// Returns an error if the bytes are not a 16 kHz WAV the engine can decode +/// (the caller should then fall back to the subprocess path) or if the +/// engine is not loaded. Mirrors [`transcribe_wav_file`] but takes bytes so +/// the factory doesn't have to stage a temp file. +pub(crate) fn transcribe_wav_bytes( + handle: &WhisperEngineHandle, + wav_bytes: &[u8], + language: Option<&str>, + initial_prompt: Option<&str>, +) -> Result { + let audio_f32 = decode_wav_to_f32(wav_bytes)?; + transcribe_pcm_f32(handle, &audio_f32, language, initial_prompt) +} + /// Minimal WAV decoder — extracts PCM samples as f32 from a standard /// RIFF/WAVE file. Supports 16-bit integer and 32-bit float formats. /// Resampling is NOT performed; the input should already be 16 kHz mono. @@ -477,6 +504,71 @@ mod tests { assert!(result.is_err()); } + #[test] + fn looks_like_wav_detects_riff_wave_header() { + let mut wav = Vec::new(); + wav.extend_from_slice(b"RIFF"); + wav.extend_from_slice(&[0u8; 4]); // chunk size (ignored by the sniff) + wav.extend_from_slice(b"WAVE"); + assert!(looks_like_wav(&wav)); + + // webm/opus and ogg blobs must NOT be mistaken for WAV. + assert!(!looks_like_wav(b"\x1aE\xdf\xa3 webm-ish bytes")); + assert!(!looks_like_wav(b"OggS....")); + // Too short to hold the magic. + assert!(!looks_like_wav(b"RIFF")); + assert!(!looks_like_wav(&[])); + } + + #[test] + fn transcribe_wav_bytes_rejects_non_wav() { + let handle = new_handle(); + // Not a WAV → decode fails before the engine-loaded check. + let err = transcribe_wav_bytes(&handle, b"not a wav at all", None, None) + .expect_err("non-WAV bytes must error"); + assert!(!err.is_empty()); + } + + #[test] + fn transcribe_wav_bytes_rejects_non_16khz() { + // Build a valid 8 kHz mono PCM16 WAV; the decoder must reject it + // (the in-process engine requires 16 kHz) so the caller falls back. + let wav = build_pcm16_wav(8000, 1, &[0i16; 16]); + let handle = new_handle(); + let err = transcribe_wav_bytes(&handle, &wav, None, None) + .expect_err("8 kHz WAV must be rejected"); + assert!( + err.contains("16000"), + "should cite the required rate: {err}" + ); + } + + /// Build a minimal RIFF/WAVE PCM16 file for decoder tests. + fn build_pcm16_wav(sample_rate: u32, channels: u16, samples: &[i16]) -> Vec { + let bits = 16u16; + let block_align = channels * bits / 8; + let byte_rate = sample_rate * block_align as u32; + let data_len = (samples.len() * 2) as u32; + let mut w = Vec::new(); + w.extend_from_slice(b"RIFF"); + w.extend_from_slice(&(36 + data_len).to_le_bytes()); + w.extend_from_slice(b"WAVE"); + w.extend_from_slice(b"fmt "); + w.extend_from_slice(&16u32.to_le_bytes()); + w.extend_from_slice(&1u16.to_le_bytes()); // PCM + w.extend_from_slice(&channels.to_le_bytes()); + w.extend_from_slice(&sample_rate.to_le_bytes()); + w.extend_from_slice(&byte_rate.to_le_bytes()); + w.extend_from_slice(&block_align.to_le_bytes()); + w.extend_from_slice(&bits.to_le_bytes()); + w.extend_from_slice(b"data"); + w.extend_from_slice(&data_len.to_le_bytes()); + for s in samples { + w.extend_from_slice(&s.to_le_bytes()); + } + w + } + #[test] fn convert_i16_produces_correct_length() { let handle = new_handle(); diff --git a/src/openhuman/inference/paths.rs b/src/openhuman/inference/paths.rs index 848cf9f24..ad36cdbb9 100644 --- a/src/openhuman/inference/paths.rs +++ b/src/openhuman/inference/paths.rs @@ -94,6 +94,42 @@ pub(crate) fn ollama_spawn_marker_path(config: &Config) -> PathBuf { .join("ollama.spawn") } +/// Standard Unix locations a CLI binary may live in that are **not** +/// guaranteed to be on the `PATH` a GUI app inherits. A macOS app launched +/// from Finder/Dock gets the minimal launchd `PATH` +/// (`/usr/bin:/bin:/usr/sbin:/sbin`), so Homebrew dirs (`/opt/homebrew/bin` +/// on Apple Silicon, `/usr/local/bin` on Intel) are invisible even when the +/// user installed the binary there and it runs fine from a terminal — the +/// exact symptom in issue #3425. Probe these explicitly as a last resort. +/// +/// Windows resolution relies entirely on the `PATH` scan, so this is empty +/// there (the installer drops `whisper-cli.exe` into the workspace anyway). +fn standard_unix_bin_dirs() -> Vec { + if cfg!(windows) { + return Vec::new(); + } + [ + "/opt/homebrew/bin", + "/usr/local/bin", + "/usr/bin", + "/bin", + "/usr/sbin", + "/sbin", + ] + .iter() + .map(PathBuf::from) + .collect() +} + +/// Return the first of `dirs` that holds `bin_name` as a regular file. +/// Shared by the `PATH` scan and the standard-dir fallback so both agree on +/// what "found" means. +fn resolve_binary_in_dirs(bin_name: &str, dirs: &[PathBuf]) -> Option { + dirs.iter() + .map(|dir| dir.join(bin_name)) + .find(|candidate| candidate.is_file()) +} + pub(crate) fn resolve_whisper_binary() -> Option { // Precedence: workspace install > env override > PATH lookup. The // workspace install path is the canonical drop-zone for the binary @@ -138,11 +174,24 @@ pub(crate) fn resolve_whisper_binary() -> Option { } else { "whisper-cli" }; - std::env::var_os("PATH").and_then(|path_var| { - std::env::split_paths(&path_var) - .map(|entry| entry.join(bin_name)) - .find(|candidate| candidate.is_file()) - }) + if let Some(from_path) = std::env::var_os("PATH").and_then(|path_var| { + let dirs: Vec = std::env::split_paths(&path_var).collect(); + resolve_binary_in_dirs(bin_name, &dirs) + }) { + return Some(from_path); + } + + // Last resort: a GUI app inherits a minimal PATH that omits Homebrew + // dirs, so a `brew install whisper-cpp` binary that works in a terminal + // is invisible to the scan above. Probe the standard Unix bin dirs. + if let Some(from_std) = resolve_binary_in_dirs(bin_name, &standard_unix_bin_dirs()) { + log::debug!( + "[voice-install:whisper] resolved binary from standard dir {}", + from_std.display() + ); + return Some(from_std); + } + None } /// Config-aware whisper resolution. Preference order: @@ -194,11 +243,24 @@ pub(crate) fn resolve_piper_binary() -> Option { } let bin_name = if cfg!(windows) { "piper.exe" } else { "piper" }; - std::env::var_os("PATH").and_then(|path_var| { - std::env::split_paths(&path_var) - .map(|entry| entry.join(bin_name)) - .find(|candidate| candidate.is_file()) - }) + if let Some(from_path) = std::env::var_os("PATH").and_then(|path_var| { + let dirs: Vec = std::env::split_paths(&path_var).collect(); + resolve_binary_in_dirs(bin_name, &dirs) + }) { + return Some(from_path); + } + + // Last resort: GUI-app PATH omits Homebrew dirs (see + // `standard_unix_bin_dirs`). Probe them so a `brew install piper` binary + // is found even when launched from Finder. + if let Some(from_std) = resolve_binary_in_dirs(bin_name, &standard_unix_bin_dirs()) { + log::debug!( + "[voice-install:piper] resolved binary from standard dir {}", + from_std.display() + ); + return Some(from_std); + } + None } /// Config-aware piper resolution. Same precedence shape as @@ -664,4 +726,72 @@ mod tests { assert_eq!(resolved, target); let _ = std::fs::remove_dir_all(workspace_piper_dir(&config)); } + + #[test] + fn standard_unix_bin_dirs_membership_is_platform_correct() { + let dirs = standard_unix_bin_dirs(); + if cfg!(windows) { + assert!(dirs.is_empty(), "Windows relies on PATH; no standard dirs"); + } else { + // Homebrew dirs are the whole point — a GUI app's minimal PATH + // omits them, so they MUST be probed explicitly (issue #3425). + assert!( + dirs.contains(&PathBuf::from("/opt/homebrew/bin")), + "Apple Silicon Homebrew dir must be probed" + ); + assert!( + dirs.contains(&PathBuf::from("/usr/local/bin")), + "Intel Homebrew / common /usr/local dir must be probed" + ); + } + } + + #[test] + fn resolve_binary_in_dirs_finds_first_match_in_order() { + let tmp = tempfile::tempdir().expect("tempdir"); + let first = tmp.path().join("first"); + let second = tmp.path().join("second"); + std::fs::create_dir_all(&first).expect("mkdir first"); + std::fs::create_dir_all(&second).expect("mkdir second"); + // Only the second dir holds the binary → it is returned. + let bin = second.join("whisper-cli"); + std::fs::write(&bin, b"stub").expect("write stub"); + let found = resolve_binary_in_dirs("whisper-cli", &[first.clone(), second.clone()]); + assert_eq!(found, Some(bin.clone())); + + // When both hold it, the earlier dir wins (precedence is positional). + let bin_first = first.join("whisper-cli"); + std::fs::write(&bin_first, b"stub").expect("write stub"); + let found = resolve_binary_in_dirs("whisper-cli", &[first, second]); + assert_eq!(found, Some(bin_first)); + } + + #[test] + fn resolve_binary_in_dirs_returns_none_when_absent() { + let tmp = tempfile::tempdir().expect("tempdir"); + let found = resolve_binary_in_dirs("piper", &[tmp.path().to_path_buf()]); + assert!(found.is_none(), "missing binary must resolve to None"); + } + + #[test] + fn resolve_whisper_binary_with_config_prefers_workspace_over_standard_dirs() { + // Precedence guard: even though /usr/bin etc. are now probed, an + // installed workspace binary must still win so the in-app installer + // result is never shadowed by a stray system binary. + let _g = shared_install_lock(); + let (_tmp, config) = temp_config(); + let target = workspace_whisper_binary_candidates(&config) + .into_iter() + .next() + .expect("at least one candidate"); + let _ = std::fs::remove_dir_all(workspace_whisper_dir(&config)); + std::fs::create_dir_all(target.parent().expect("parent")).expect("mkdir"); + std::fs::write(&target, b"stub").expect("write stub"); + let resolved = resolve_whisper_binary_with_config(&config).expect("workspace resolve"); + assert_eq!( + resolved, target, + "workspace install must outrank standard-dir fallback" + ); + let _ = std::fs::remove_dir_all(workspace_whisper_dir(&config)); + } } diff --git a/src/openhuman/voice/factory/stt_providers.rs b/src/openhuman/voice/factory/stt_providers.rs index 9e6d9bae5..32bb3abfc 100644 --- a/src/openhuman/voice/factory/stt_providers.rs +++ b/src/openhuman/voice/factory/stt_providers.rs @@ -1,7 +1,7 @@ //! STT provider implementations: cloud, local Whisper, and external (slug-keyed). use async_trait::async_trait; -use log::debug; +use log::{debug, warn}; use serde::Deserialize; use super::super::cloud_transcribe::{ @@ -12,10 +12,80 @@ use super::helpers::{base64_decode, extension_for_mime}; use super::traits::{SttProvider, SttResult}; use crate::openhuman::config::schema::voice_providers::SttApiStyle; use crate::openhuman::config::Config; +use crate::openhuman::inference::local as local_ai; +use crate::openhuman::inference::local::paths::resolve_stt_model_path_by_id; +use crate::openhuman::inference::local::whisper_engine; +use crate::openhuman::inference::local::LocalAiService; use crate::rpc::RpcOutcome; const LOG_PREFIX: &str = "[voice-factory]"; +/// Which local-Whisper backend a request should use. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum WhisperRoute { + /// Bundled in-process `whisper-rs` engine — no external binary, works + /// for every model size. Eligible only for 16 kHz WAV input. + InProcess, + /// `whisper-cli` subprocess — container-aware (ffmpeg) so it handles + /// webm/opus/mp4/ogg, but requires the external binary. + Subprocess, +} + +/// Decide the local-Whisper backend for a decoded audio blob. +/// +/// In-process is preferred when enabled AND the bytes are a WAV the engine +/// can decode — this is the path that makes local STT work with no external +/// binary on a fresh install (issue #3425). Everything else (non-WAV inputs, +/// or in-process disabled) routes to the container-aware subprocess. +fn choose_whisper_route(config: &Config, audio_bytes: &[u8]) -> WhisperRoute { + if config.local_ai.whisper_in_process && whisper_engine::looks_like_wav(audio_bytes) { + WhisperRoute::InProcess + } else { + WhisperRoute::Subprocess + } +} + +/// Ensure the in-process engine has the requested model loaded, loading or +/// reloading when needed. The engine holds a single model, so a per-request +/// size change (`tiny` → `large-v3-turbo`) triggers an unload + reload so the +/// right weights are used. +/// +/// **Precondition:** the caller MUST hold `service.whisper_load_lock`. That +/// same guard is then held across the subsequent transcription (see +/// [`WhisperSttProvider::try_in_process`]), so the load check, the reload, and +/// the inference form one critical section — a concurrent dispatch for a +/// different model size cannot unload/reload the engine mid-flight (which would +/// otherwise transcribe with the wrong weights or drop the request onto the +/// subprocess path). +async fn ensure_model_loaded_locked( + service: &LocalAiService, + config: &Config, + model_id: &str, +) -> Result<(), String> { + let model_path = resolve_stt_model_path_by_id(model_id, config)?; + let target = std::path::PathBuf::from(&model_path); + + // The right model is already resident — nothing to do. + if whisper_engine::loaded_model_path(&service.whisper).as_deref() == Some(target.as_path()) { + return Ok(()); + } + if whisper_engine::is_loaded(&service.whisper) { + debug!("{LOG_PREFIX} whisper model changed; reloading for {model_id}"); + whisper_engine::unload_engine(&service.whisper); + } + + let device = crate::openhuman::inference::device::detect_device_profile(); + let gpu = device.has_gpu; + let gpu_desc = device.gpu_description.clone(); + let handle = service.whisper.clone(); + let model_for_load = target.clone(); + tokio::task::spawn_blocking(move || { + whisper_engine::load_engine(&handle, &model_for_load, gpu, gpu_desc.as_deref()) + }) + .await + .map_err(|e| format!("whisper load join error: {e}"))? +} + // --------------------------------------------------------------------------- // Cloud STT // --------------------------------------------------------------------------- @@ -74,8 +144,10 @@ impl SttProvider for CloudSttProvider { // Local Whisper STT // --------------------------------------------------------------------------- -/// Local Whisper STT — wraps [`transcribe_whisper`]. Resolves `WHISPER_BIN` -/// lazily on each call. +/// Local Whisper STT. Prefers the bundled in-process `whisper-rs` engine +/// (no external binary, every model size) for 16 kHz WAV input, falling back +/// to the [`transcribe_whisper`] `whisper-cli` subprocess for container +/// formats it can't decode (webm/opus/mp4/ogg) or when in-process is off. pub struct WhisperSttProvider { model: String, } @@ -86,6 +158,69 @@ impl WhisperSttProvider { model: model.into(), } } + + /// Attempt in-process transcription. `Some(text)` on success; `None` + /// signals the caller to fall back to the subprocess path (route not + /// eligible, model not installed, load failure, or inference error) — we + /// never surface an in-process error directly so a missing local binary + /// can't turn a recoverable case into a user-facing failure. + async fn try_in_process( + &self, + config: &Config, + audio_bytes: &[u8], + language: Option<&str>, + ) -> Option { + if choose_whisper_route(config, audio_bytes) != WhisperRoute::InProcess { + debug!("{LOG_PREFIX} whisper route=subprocess (non-WAV or in-process disabled)"); + return None; + } + + let service = local_ai::global(config); + + // Hold the load lock across BOTH the load step and the transcription so + // a concurrent dispatch for a different model size can't unload/reload + // the single-model engine between load and inference. Transcriptions + // already serialize on the engine handle lock, so extending the + // critical section over the load step adds no new contention. + let load_guard = service.whisper_load_lock.lock().await; + if let Err(e) = ensure_model_loaded_locked(&service, config, &self.model).await { + warn!("{LOG_PREFIX} in-process load failed ({e}); falling back to subprocess"); + return None; + } + + let handle = service.whisper.clone(); + let bytes = audio_bytes.to_vec(); + let lang = language.map(String::from); + let result = tokio::task::spawn_blocking(move || { + whisper_engine::transcribe_wav_bytes(&handle, &bytes, lang.as_deref(), None) + }) + .await; + // Release only after inference completes — the resident model is + // guaranteed to be `self.model` for the whole transcription above. + drop(load_guard); + + match result { + Ok(Ok(r)) => { + debug!( + "{LOG_PREFIX} in-process whisper ok: {} chars, {}/{} segments", + r.text.len(), + r.segments_accepted, + r.segments_total + ); + Some(r.text) + } + Ok(Err(e)) => { + warn!( + "{LOG_PREFIX} in-process transcribe failed ({e}); falling back to subprocess" + ); + None + } + Err(e) => { + warn!("{LOG_PREFIX} in-process join error ({e}); falling back to subprocess"); + None + } + } + } } #[async_trait] @@ -106,6 +241,23 @@ impl SttProvider for WhisperSttProvider { "{LOG_PREFIX} whisper STT dispatch model={} mime={:?} lang={:?}", self.model, mime_type, language ); + + // Decode once so we can sniff the container and route. A decode + // failure here is a genuine bad-input error worth surfacing. + let audio_bytes = base64_decode(audio_base64)?; + if let Some(text) = self.try_in_process(config, &audio_bytes, language).await { + return Ok(RpcOutcome::single_log( + SttResult { + text, + provider: "whisper".to_string(), + }, + "voice-factory: in-process whisper STT completed", + )); + } + + // Fallback: container-aware whisper-cli subprocess (re-decodes the + // base64 internally). Covers webm/opus/mp4/ogg and the in-process-off + // case; binary resolution now also probes Homebrew dirs (issue #3425). let opts = WhisperTranscribeOptions { model: Some(self.model.clone()), mime_type: mime_type.map(str::to_string), @@ -318,3 +470,48 @@ impl ExternalSttProvider { Ok(text) } } + +#[cfg(test)] +mod tests { + use super::*; + + /// 12-byte RIFF/WAVE header is enough for the route sniff. + fn wav_header() -> Vec { + let mut w = b"RIFF".to_vec(); + w.extend_from_slice(&[0u8; 4]); + w.extend_from_slice(b"WAVE"); + w + } + + #[test] + fn choose_route_in_process_for_wav_when_enabled() { + let mut config = Config::default(); + config.local_ai.whisper_in_process = true; + assert_eq!( + choose_whisper_route(&config, &wav_header()), + WhisperRoute::InProcess + ); + } + + #[test] + fn choose_route_subprocess_for_non_wav() { + let mut config = Config::default(); + config.local_ai.whisper_in_process = true; + // webm/opus magic-ish bytes → not a WAV → subprocess (ffmpeg decodes). + assert_eq!( + choose_whisper_route(&config, b"\x1aE\xdf\xa3 webm"), + WhisperRoute::Subprocess + ); + } + + #[test] + fn choose_route_subprocess_when_in_process_disabled() { + let mut config = Config::default(); + config.local_ai.whisper_in_process = false; + // Even a valid WAV must go to the subprocess when in-process is off. + assert_eq!( + choose_whisper_route(&config, &wav_header()), + WhisperRoute::Subprocess + ); + } +} diff --git a/src/openhuman/voice/schemas/handlers/provider_server.rs b/src/openhuman/voice/schemas/handlers/provider_server.rs index 3d643b466..77fa87c7a 100644 --- a/src/openhuman/voice/schemas/handlers/provider_server.rs +++ b/src/openhuman/voice/schemas/handlers/provider_server.rs @@ -313,7 +313,9 @@ pub(crate) fn handle_voice_test_provider(params: Map) -> Controll crate::openhuman::voice::create_stt_provider(&p.provider, "", &config) .map_err(|e| e.to_string())?; - // 0.1s of silence as WAV (8kHz mono 16-bit PCM). + // 0.1s of silence as WAV (16kHz mono 16-bit PCM) so the local + // Whisper provider can transcribe it in-process without an + // external binary (issue #3425). let silent_wav = generate_silent_wav(); let audio_b64 = { use base64::Engine; diff --git a/src/openhuman/voice/schemas/helpers.rs b/src/openhuman/voice/schemas/helpers.rs index 1b79c4904..67e7d9a61 100644 --- a/src/openhuman/voice/schemas/helpers.rs +++ b/src/openhuman/voice/schemas/helpers.rs @@ -151,10 +151,19 @@ pub(super) async fn validate_tts_provider_key( } } -/// Generate a minimal WAV file with ~0.1s of silence (8kHz mono 16-bit PCM). +/// Generate a minimal WAV file with ~0.1s of silence (16kHz mono 16-bit PCM). +/// +/// The rate is deliberately **16kHz**, not 8kHz: the local Whisper provider +/// prefers the bundled in-process `whisper-rs` engine, whose decoder only +/// accepts 16kHz. An 8kHz fixture is rejected during decode and silently +/// falls back to the `whisper-cli` subprocess — which then errors with +/// "binary not found" on a machine that intentionally has no external binary +/// (the whole point of issue #3425). Generating the test clip at the rate the +/// in-process engine supports lets "Test STT" exercise the real, binary-free +/// path instead of failing on a missing subprocess. pub(super) fn generate_silent_wav() -> Vec { - let sample_rate: u32 = 8000; - let num_samples: u32 = 800; // 0.1s + let sample_rate: u32 = 16_000; + let num_samples: u32 = 1_600; // 0.1s let bits_per_sample: u16 = 16; let num_channels: u16 = 1; let byte_rate = sample_rate * u32::from(num_channels) * u32::from(bits_per_sample) / 8; diff --git a/src/openhuman/voice/schemas_tests.rs b/src/openhuman/voice/schemas_tests.rs index 6b43f5ffc..793c3dad3 100644 --- a/src/openhuman/voice/schemas_tests.rs +++ b/src/openhuman/voice/schemas_tests.rs @@ -483,8 +483,14 @@ fn generate_silent_wav_produces_valid_wav_header() { assert_eq!(&wav[8..12], b"WAVE"); assert_eq!(&wav[12..16], b"fmt "); assert_eq!(&wav[36..40], b"data"); - // total size = header(44) + 800 samples * 2 bytes = 1644 - assert_eq!(wav.len(), 1644); + // 16kHz so the in-process whisper engine accepts it (issue #3425). + let sample_rate = u32::from_le_bytes([wav[24], wav[25], wav[26], wav[27]]); + assert_eq!( + sample_rate, 16_000, + "fixture must be 16kHz for in-process STT" + ); + // total size = header(44) + 1600 samples * 2 bytes = 3244 + assert_eq!(wav.len(), 3244); } #[test]