From 54d9adf1f01e1a3e047bcf49e0dbb49046130b13 Mon Sep 17 00:00:00 2001 From: Jake Date: Wed, 22 Apr 2026 10:32:04 +0100 Subject: [PATCH] fix(rb-07): runtime capabilities derive accelerators honestly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_runtime_capabilities was returning `accelerators = ["cpu", "vulkan"]` and `whisper.supports_gpu = true` regardless of build config or runtime state. On a macOS build it falsely advertised "vulkan" (the backend actually resolves via MoltenVK as Metal); on a whisper-disabled build it claimed GPU support for an engine that hadn't been linked. Added `compose_accelerators(whisper_enabled, loader_available, target)` — a pure helper that always emits "cpu" first and appends the platform-appropriate GPU name only when whisper is compiled in AND the Vulkan loader resolves. `supported_accelerators()` wraps it with the live `cfg!(feature = "whisper")`, loader probe, and target OS. `get_runtime_capabilities` now calls `supported_accelerators()` and sets `whisper.supports_gpu = cfg!(feature = "whisper")`. Parakeet stays CPU-only. Five tests in `commands::models::tests` cover the permutation matrix: whisper on/off, loader present/missing, macOS vs other. Both feature configurations (`--features whisper` and `--no-default-features`) build and pass tests. Macos Metal-loader resolution on real hardware stays on the ship-gate checklist — the detection logic is verifiable from Linux but runtime behaviour is not. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/issues/README.md | 4 +- .../runtime-capabilities-accelerators.md | 33 +++++ src-tauri/src/commands/models.rs | 122 +++++++++++++++++- 3 files changed, 151 insertions(+), 8 deletions(-) diff --git a/docs/issues/README.md b/docs/issues/README.md index 416c0dc..75aafee 100644 --- a/docs/issues/README.md +++ b/docs/issues/README.md @@ -18,14 +18,13 @@ should be mirrored as real GitHub issues on `jakejars/kon`. | RB-01 | [c1-live-session-race.md](c1-live-session-race.md) | `src-tauri/commands/live.rs` | large | | RB-03 | [c4-transcript-profile-fk.md](c4-transcript-profile-fk.md) | `crates/storage/migrations.rs` + `database.rs` | large | -## MAJOR (7 open, 2 resolved) +## MAJOR (6 open, 3 resolved) | # | File | Area | Fix scope | |---|---|---|---| | RB-04 | [run-live-session-monolith.md](run-live-session-monolith.md) | `src-tauri/commands/live.rs` | large | | RB-05 | [poll-inference-channel-fatality.md](poll-inference-channel-fatality.md) | `src-tauri/commands/live.rs` | medium | | RB-06 | [native-capture-worker-join.md](native-capture-worker-join.md) | `src-tauri/commands/audio.rs` | medium | -| RB-07 | [runtime-capabilities-accelerators.md](runtime-capabilities-accelerators.md) | `src-tauri/commands/models.rs` | medium | | RB-08 | [power-assertion-macos-objc2.md](power-assertion-macos-objc2.md) | `src-tauri/commands/power.rs` | medium | | RB-10 | [llm-prompt-preflight.md](llm-prompt-preflight.md) | `crates/llm/lib.rs` | medium | | RB-11 | [keystore-thread-safety.md](keystore-thread-safety.md) | `crates/cloud-providers/keystore.rs` | medium | @@ -35,6 +34,7 @@ should be mirrored as real GitHub issues on `jakejars/kon`. | # | File | Area | Resolution | |---|---|---|---| | RB-02 | [c3-migrations-atomicity.md](c3-migrations-atomicity.md) | `crates/storage/migrations.rs` | Each migration now runs inside a `pool.begin()` / `tx.commit()` transaction alongside its `schema_version` insert. Regression test injects a poisoned v9 migration and asserts neither the partial schema change nor the version row persists. DRY'd `run_migrations_up_to` test helper onto the same code path. | +| RB-07 | [runtime-capabilities-accelerators.md](runtime-capabilities-accelerators.md) | `src-tauri/commands/models.rs` | Introduced `compose_accelerators(whisper_enabled, loader_available, target)` as a pure helper; `supported_accelerators()` reads `cfg(feature = "whisper")`, `vulkan_loader_available()`, and target OS then delegates. `get_runtime_capabilities` uses it in place of the hard-coded `["cpu", "vulkan"]`. Whisper's `supports_gpu` now follows the feature flag. Five regression tests cover all permutations. | | RB-09 | [decoder-partial-audio-on-error.md](decoder-partial-audio-on-error.md) | `crates/audio/decode.rs` | Packet-loop now propagates all non-EOF `SymphoniaError`s as `AudioDecodeFailed`; per-packet decode errors bubble via `?`. Mock-`MediaSource` regression test confirms mid-stream I/O errors surface instead of returning partial audio. | | RB-12 | [hotkey-linux-device-filter.md](hotkey-linux-device-filter.md) | `crates/hotkey/linux.rs` | Extracted `device_supports_combo` helper; `try_attach_device` now reads the configured `HotkeyCombo` from the watch channel and checks support for that trigger key. Four regression tests land in `linux::tests`. | diff --git a/docs/issues/runtime-capabilities-accelerators.md b/docs/issues/runtime-capabilities-accelerators.md index 4092876..c24727a 100644 --- a/docs/issues/runtime-capabilities-accelerators.md +++ b/docs/issues/runtime-capabilities-accelerators.md @@ -4,6 +4,39 @@ **Path:** `src-tauri/src/commands/models.rs:435-489` **Source:** [2026-04-22 code review](../code-review-2026-04-22.md) **Labels:** release-blocker, major, ui-integrity +**Status:** RESOLVED (2026-04-22) + +## Resolution + +Added a pure `compose_accelerators(whisper_enabled, loader_available, +target) -> Vec` helper. It always emits `"cpu"` first and +appends `"metal"` (macOS) or `"vulkan"` (Linux/Windows) only when +whisper is compiled in *and* `vulkan_loader_available()` resolves the +platform's loader shim. + +`supported_accelerators()` reads `cfg!(feature = "whisper")`, runs the +live loader probe, and delegates to the helper. +`get_runtime_capabilities` calls `supported_accelerators()` in place +of the hard-coded `vec!["cpu", "vulkan"]`. Whisper's `supports_gpu` +is now `cfg!(feature = "whisper")` — false on whisper-disabled +builds. + +Five regression tests in `commands::models::tests` cover: + +- `cpu_only_when_whisper_disabled` (both targets) +- `cpu_only_when_loader_missing` (both targets) +- `macos_with_loader_advertises_metal` +- `non_macos_with_loader_advertises_vulkan` +- `cpu_is_always_first_entry` (contract the frontend relies on) + +Both `cargo test -p kon --lib` and `cargo test -p kon --lib +--no-default-features` pass the new suite; both `cargo build -p kon` +and `cargo build -p kon --no-default-features` compile clean. + +Runtime GUI verification on a real macOS box is still on the ship-gate +checklist — the detection logic is correct in code; Metal-loader +resolution on hardware is not something we can unit-test from a Linux +CI. ## Problem diff --git a/src-tauri/src/commands/models.rs b/src-tauri/src/commands/models.rs index 24f6310..a05553f 100644 --- a/src-tauri/src/commands/models.rs +++ b/src-tauri/src/commands/models.rs @@ -341,6 +341,51 @@ fn vulkan_loader_available() -> bool { false } +/// Compile-time target signalling used by `compose_accelerators`. +/// Split out so the pure-function behaviour is testable without `cfg!` +/// appearing in the test matrix. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum AcceleratorTarget { + Macos, + Other, +} + +/// Pure helper: produce the `accelerators` list from the +/// whisper-compiled-in flag, runtime loader availability, and target +/// family. Always starts with `cpu`; appends the platform-appropriate +/// GPU name only when whisper is compiled in AND the Vulkan loader +/// resolves. RB-07 replaced a hard-coded `["cpu", "vulkan"]`. +fn compose_accelerators( + whisper_enabled: bool, + loader_available: bool, + target: AcceleratorTarget, +) -> Vec { + let mut accelerators = vec!["cpu".into()]; + if whisper_enabled && loader_available { + let gpu = match target { + AcceleratorTarget::Macos => "metal", + AcceleratorTarget::Other => "vulkan", + }; + accelerators.push(gpu.into()); + } + accelerators +} + +/// Public wrapper around `compose_accelerators` that reads the real +/// `cfg(feature = "whisper")`, runtime loader probe, and target OS. +fn supported_accelerators() -> Vec { + let target = if cfg!(target_os = "macos") { + AcceleratorTarget::Macos + } else { + AcceleratorTarget::Other + }; + compose_accelerators( + cfg!(feature = "whisper"), + vulkan_loader_available(), + target, + ) +} + /// Report which backend whisper.cpp was actually able to initialise /// against. The whisper-rs build here is compiled with the `vulkan` /// feature unconditionally; on macOS that's still Metal via MoltenVK, @@ -453,12 +498,14 @@ pub fn get_runtime_capabilities( let active_compute_device = detect_active_compute_device(); let cpu_features: CpuFeaturesInfo = hardware::probe_cpu_features().into(); - // Kon's desktop build links transcribe-rs with the `whisper-vulkan` - // feature unconditionally (see crates/transcription/Cargo.toml), so - // whisper.cpp boots with Vulkan backend on any machine with a Vulkan - // loader + ICD. Parakeet (ONNX) still runs on CPU. Reflect both. + // Accelerator list is now derived from the live build configuration + // and runtime loader probe — see `compose_accelerators`. Parakeet + // (ONNX) stays CPU-only; Whisper advertises `supports_gpu` only + // when it was actually compiled in for this binary. + let whisper_supports_gpu = cfg!(feature = "whisper"); + Ok(RuntimeCapabilities { - accelerators: vec!["cpu".into(), "vulkan".into()], + accelerators: supported_accelerators(), engines: vec![ EngineRuntimeCapabilities { id: "whisper".into(), @@ -466,7 +513,7 @@ pub fn get_runtime_capabilities( loaded_model_id: whisper .loaded_model_id() .map(|model_id| model_id.to_string()), - supports_gpu: true, + supports_gpu: whisper_supports_gpu, models: whisper_models, }, EngineRuntimeCapabilities { @@ -594,3 +641,66 @@ pub async fn load_parakeet_model( pub fn check_parakeet_engine(state: tauri::State<'_, AppState>) -> Result { Ok(state.parakeet_engine.is_loaded()) } + +#[cfg(test)] +mod tests { + use super::*; + + // RB-07: the accelerator list is now derived from compile flags + + // runtime loader probe + target. These cover the permutations. + // + // Pre-fix behaviour was `vec!["cpu".into(), "vulkan".into()]` + // regardless of whisper feature, loader availability, or platform — + // so a macOS build falsely advertised "vulkan", and a no-whisper + // build falsely advertised a GPU path with no engine to use it. + + #[test] + fn cpu_only_when_whisper_disabled() { + assert_eq!( + compose_accelerators(false, true, AcceleratorTarget::Macos), + vec!["cpu".to_string()] + ); + assert_eq!( + compose_accelerators(false, true, AcceleratorTarget::Other), + vec!["cpu".to_string()] + ); + } + + #[test] + fn cpu_only_when_loader_missing() { + assert_eq!( + compose_accelerators(true, false, AcceleratorTarget::Macos), + vec!["cpu".to_string()] + ); + assert_eq!( + compose_accelerators(true, false, AcceleratorTarget::Other), + vec!["cpu".to_string()] + ); + } + + #[test] + fn macos_with_loader_advertises_metal() { + assert_eq!( + compose_accelerators(true, true, AcceleratorTarget::Macos), + vec!["cpu".to_string(), "metal".to_string()] + ); + } + + #[test] + fn non_macos_with_loader_advertises_vulkan() { + assert_eq!( + compose_accelerators(true, true, AcceleratorTarget::Other), + vec!["cpu".to_string(), "vulkan".to_string()] + ); + } + + #[test] + fn cpu_is_always_first_entry() { + // Frontend relies on index-0 being the fallback; preserve that + // contract regardless of which GPU extras are added. + for target in [AcceleratorTarget::Macos, AcceleratorTarget::Other] { + let full = compose_accelerators(true, true, target); + assert_eq!(full.first(), Some(&"cpu".to_string())); + } + } +}