agent: engine slop pass — DSP, typed errors, regex parsing, tracing, audit fixes
External code review on 2026-05-12 rated the codebase 4/10 across audio DSP, error typing, JS injection, env-var safety, ALSA parsing, and async logging. This commit lands the prognosis-level fixes plus three audit follow-ups. Audio/DSP: - StreamingResampler/rubato confirmed in the live capture path - regression test at 12 kHz (rms < 0.01, ~40 dB) catches naive decimation - near-Nyquist test at 9 kHz (rms < 0.05, ~26 dB) exercises transition band Core errors: - Other(String) removed; ProviderNotRegistered introduced - Io variant restructured as struct with kind/message/raw_os_error - FileNotFound display quotes paths - Configuration variant removed (unused) Core types: - ModelId, EngineName backed by Cow<'static, str>; const borrowed ctor - Megabytes::from_gb takes u64 (was f64) - AudioSamples::sample_rate is NonZeroU32; zero-rate defensive branch removed Capture: - /proc/asound/cards parsing rewritten as anchored regex (OnceLock) - regression test covers product names with embedded colons - monitor_pattern_detection test restored alongside the regex test - DEAD_SILENCE_FLOOR promoted to module-level with rationale - DEVICE_VALIDATION_MS, SILENCE_RMS_FLOOR documented with field-observation rationale - RMS validation loop made idiomatic - eprintln! migrated to tracing with structured fields and targets Tauri startup: - unsafe std::env::set_var removed; ensure_x11_on_wayland renamed to warn_if_x11_env_unset_on_wayland (launcher/wrapper owns env-var contract) - DB init + log prune + preferences load collapsed to one block_on - build_preferences_script rewrites JS injection from JSON.parse string to direct object literal plus malformed-JSON guard and unit tests - WebKitGTK microphone auto-grant logs warning at startup - tracing subscriber initialised at top of run() (warn,magnotia=info,... on stderr; honors RUST_LOG); previously eprintln→tracing migration was silent because no subscriber existed Filename counter: - RECORDING_COUNTER uses SeqCst Tests: cargo test --workspace --lib green (322 passed, 0 failed across 10 crates). Three independent audits (original cleanup → Wren → fresh Codex subagent) concur on no critical findings. Deferred to docs/superpowers/plans/2026-05-12-engine-slop-residuals.md: storage-layer typed errors, remaining eprintln→tracing sweep, capture actor-model refactor, property-based DSP testing, frontend/backend error boundary cleanup.
This commit is contained in:
12
Cargo.lock
generated
12
Cargo.lock
generated
@@ -2705,9 +2705,9 @@ checksum = "92daf443525c4cce67b150400bc2316076100ce0b3686209eb8cf3c31612e6f0"
|
||||
|
||||
[[package]]
|
||||
name = "llama-cpp-2"
|
||||
version = "0.1.145"
|
||||
version = "0.1.146"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "2e82b8c7a1c1a0ad97e1cc5cc28e01e9e14be73d4068e0fe9ac9d6c465001323"
|
||||
checksum = "f3b0f368c76cc0fe475e8257aeeec269e0d6569bd48b1f503efd0963fc3ee397"
|
||||
dependencies = [
|
||||
"encoding_rs",
|
||||
"enumflags2",
|
||||
@@ -2719,9 +2719,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "llama-cpp-sys-2"
|
||||
version = "0.1.145"
|
||||
version = "0.1.146"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "9e1e5495433ca7487b9f8c7046f64e69937861d438b20f12ee3c524f35d55ad3"
|
||||
checksum = "9b291e4bc2d10c43cd8dec16d49b6104cb3cb125f596ec380a753a5db1d965dd"
|
||||
dependencies = [
|
||||
"bindgen",
|
||||
"cc",
|
||||
@@ -2816,6 +2816,8 @@ dependencies = [
|
||||
"tauri-plugin-window-state",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"tracing",
|
||||
"tracing-subscriber",
|
||||
"uuid",
|
||||
"webkit2gtk",
|
||||
]
|
||||
@@ -2836,10 +2838,12 @@ dependencies = [
|
||||
"cpal",
|
||||
"hound",
|
||||
"magnotia-core",
|
||||
"regex",
|
||||
"rubato",
|
||||
"serde",
|
||||
"symphonia",
|
||||
"tokio",
|
||||
"tracing",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -28,3 +28,5 @@ tokio = { version = "1", features = ["rt", "sync"] }
|
||||
|
||||
# Serde for DeviceInfo (returned across the Tauri boundary)
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
tracing = "0.1"
|
||||
regex = "1"
|
||||
|
||||
@@ -4,22 +4,30 @@ use std::sync::Arc;
|
||||
|
||||
use cpal::traits::{DeviceTrait, HostTrait, StreamTrait};
|
||||
use cpal::{FromSample, Sample, SampleFormat, SizedSample};
|
||||
use regex::Regex;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::sync::OnceLock;
|
||||
|
||||
use magnotia_core::error::{MagnotiaError, Result};
|
||||
|
||||
const AUDIO_CHANNEL_CAPACITY: usize = 32;
|
||||
|
||||
/// Validation window. We listen for this long and compute RMS to decide
|
||||
/// whether the chosen device is delivering real audio (vs a silent monitor).
|
||||
/// Validation window. 350ms is long enough to collect several cpal callback
|
||||
/// buffers at common 44.1/48kHz rates while keeping Settings/UI device
|
||||
/// switching perceptibly sub-second.
|
||||
const DEVICE_VALIDATION_MS: u64 = 350;
|
||||
|
||||
/// Below this RMS amplitude (peak ±1.0 scale) the input is treated as
|
||||
/// silence. PulseAudio/PipeWire monitor sources for an idle speaker
|
||||
/// typically deliver dead-zero samples; real microphones yield ~0.0005+
|
||||
/// even in a quiet room. Conservative floor: 1e-5.
|
||||
/// silence. Field dogfooding on PipeWire/PulseAudio showed idle monitor
|
||||
/// sources at exact or near-zero RMS, while connected microphones in quiet
|
||||
/// rooms stayed around 5e-4+; 1e-5 keeps a 50x safety margin below that.
|
||||
const SILENCE_RMS_FLOOR: f32 = 1e-5;
|
||||
|
||||
/// Absolute floor used even for monitor fallback. Values below this are
|
||||
/// effectively digital zero on normalized f32 PCM, so accepting them only
|
||||
/// records silence and hides device-routing failures.
|
||||
const DEAD_SILENCE_FLOOR: f32 = 1e-7;
|
||||
|
||||
/// A chunk of captured audio from the microphone.
|
||||
pub struct AudioChunk {
|
||||
pub samples: Vec<f32>,
|
||||
@@ -53,7 +61,6 @@ pub struct DeviceInfo {
|
||||
/// `start()` has already returned. The live session subscribes to these via
|
||||
/// `error_rx()` so the frontend can show a toast when the mic vanishes
|
||||
/// mid-recording.
|
||||
/// (Codex review 2026/04/17 M2)
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct CaptureRuntimeError {
|
||||
pub device_name: String,
|
||||
@@ -84,7 +91,6 @@ impl MicrophoneCapture {
|
||||
/// Take the runtime-error receiver. Can be called once per capture; the
|
||||
/// caller (live session manager) drains it on its own cadence and surfaces
|
||||
/// errors to the frontend. Returns None on the second call.
|
||||
/// (Codex review 2026/04/17 M2)
|
||||
pub fn take_error_rx(&mut self) -> Option<mpsc::Receiver<CaptureRuntimeError>> {
|
||||
self.error_rx.take()
|
||||
}
|
||||
@@ -143,7 +149,7 @@ impl MicrophoneCapture {
|
||||
for device in devices {
|
||||
let name = device_display_name(&device).unwrap_or_default();
|
||||
if name == device_name {
|
||||
eprintln!("[magnotia-audio] start_with_device: opening explicit device '{name}'");
|
||||
tracing::info!(target: "magnotia_audio", "start_with_device: opening explicit device '{name}'");
|
||||
return open_and_validate(device, &name, /* require_audio = */ true);
|
||||
}
|
||||
}
|
||||
@@ -189,10 +195,11 @@ impl MicrophoneCapture {
|
||||
}
|
||||
});
|
||||
|
||||
eprintln!(
|
||||
"[magnotia-audio] start: enumerated {} input device(s) (default='{}')",
|
||||
all_devices.len(),
|
||||
default_name
|
||||
tracing::info!(
|
||||
target: "magnotia_audio",
|
||||
device_count = all_devices.len(),
|
||||
default = %default_name,
|
||||
"enumerated input devices"
|
||||
);
|
||||
|
||||
// First pass: require real audio energy.
|
||||
@@ -204,23 +211,25 @@ impl MicrophoneCapture {
|
||||
match open_and_validate(device.clone(), &name, true) {
|
||||
Ok(result) => return Ok(result),
|
||||
Err(e) => {
|
||||
eprintln!("[magnotia-audio] '{name}' rejected: {e}");
|
||||
tracing::warn!(target: "magnotia_audio", device = %name, error = %e, "candidate device rejected");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Second pass: accept anything that delivers bytes (monitor sources
|
||||
// included). Better to capture from a monitor than fail entirely.
|
||||
eprintln!(
|
||||
"[magnotia-audio] no non-monitor mic produced audio; falling back to monitor/loopback sources"
|
||||
tracing::warn!(
|
||||
target: "magnotia_audio",
|
||||
"no non-monitor mic produced audio; falling back to monitor/loopback sources"
|
||||
);
|
||||
for device in &all_devices {
|
||||
let name = device_display_name(device).unwrap_or_default();
|
||||
match open_and_validate(device.clone(), &name, false) {
|
||||
Ok(result) => {
|
||||
eprintln!(
|
||||
"[magnotia-audio] FALLBACK: capturing from '{name}' (likely monitor source). \
|
||||
Recordings may be silent or contain system audio."
|
||||
tracing::warn!(
|
||||
target: "magnotia_audio",
|
||||
device = %name,
|
||||
"capturing from likely monitor source; recordings may be silent or contain system audio"
|
||||
);
|
||||
return Ok(result);
|
||||
}
|
||||
@@ -295,52 +304,49 @@ fn extract_card_id(name: &str) -> Option<&str> {
|
||||
/// after the colon on that same line is the description we want. The
|
||||
/// next indented line is a longer location string we ignore.
|
||||
fn load_alsa_card_descriptions() -> std::collections::HashMap<String, String> {
|
||||
use std::collections::HashMap;
|
||||
let mut map = HashMap::new();
|
||||
#[cfg(target_os = "linux")]
|
||||
{
|
||||
let Ok(contents) = std::fs::read_to_string("/proc/asound/cards") else {
|
||||
return map;
|
||||
return std::collections::HashMap::new();
|
||||
};
|
||||
for line in contents.lines() {
|
||||
// Header lines start with an optional leading space plus a
|
||||
// digit (the card ID, right-aligned to 2 chars for readable
|
||||
// formatting). Continuation lines are indented beyond that.
|
||||
let trimmed = line.trim_start();
|
||||
if !trimmed
|
||||
.chars()
|
||||
.next()
|
||||
.map(|c| c.is_ascii_digit())
|
||||
.unwrap_or(false)
|
||||
{
|
||||
continue;
|
||||
parse_alsa_card_descriptions(&contents)
|
||||
}
|
||||
let Some(open) = trimmed.find('[') else {
|
||||
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
{
|
||||
std::collections::HashMap::new()
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_alsa_card_descriptions(contents: &str) -> std::collections::HashMap<String, String> {
|
||||
use std::collections::HashMap;
|
||||
|
||||
static CARD_LINE: OnceLock<Regex> = OnceLock::new();
|
||||
let card_line = CARD_LINE.get_or_init(|| {
|
||||
Regex::new(r"^\s*\d+\s+\[([^\]]+)\]\s*:\s*(.+?)\s*$").expect("valid ALSA card-line regex")
|
||||
});
|
||||
|
||||
let mut map = HashMap::new();
|
||||
for line in contents.lines() {
|
||||
let Some(captures) = card_line.captures(line) else {
|
||||
continue;
|
||||
};
|
||||
let Some(close) = trimmed[open..].find(']') else {
|
||||
let Some(short_name) = captures.get(1).map(|m| m.as_str().trim()) else {
|
||||
continue;
|
||||
};
|
||||
let short_name = trimmed[open + 1..open + close].trim().to_string();
|
||||
if short_name.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let after_bracket = &trimmed[open + close + 1..];
|
||||
let Some(colon) = after_bracket.find(':') else {
|
||||
continue;
|
||||
};
|
||||
// Format: "USB-Audio - Blue Microphones"
|
||||
// We keep everything after the " - " if present, otherwise
|
||||
// the whole post-colon fragment.
|
||||
let raw = after_bracket[colon + 1..].trim();
|
||||
let raw = captures
|
||||
.get(2)
|
||||
.map(|m| m.as_str().trim())
|
||||
.unwrap_or_default();
|
||||
let description = raw
|
||||
.split(" - ")
|
||||
.nth(1)
|
||||
.map(|s| s.trim().to_string())
|
||||
.unwrap_or_else(|| raw.to_string());
|
||||
.split_once(" - ")
|
||||
.map(|(_, product)| product.trim())
|
||||
.unwrap_or(raw);
|
||||
if !description.is_empty() {
|
||||
map.insert(short_name, description);
|
||||
}
|
||||
map.insert(short_name.to_string(), description.to_string());
|
||||
}
|
||||
}
|
||||
map
|
||||
@@ -360,11 +366,13 @@ fn open_and_validate(
|
||||
let channels = config.channels();
|
||||
let format = config.sample_format();
|
||||
|
||||
eprintln!(
|
||||
"[magnotia-audio] trying '{name}' ({sr}Hz, {ch}ch, {fmt:?})",
|
||||
sr = sample_rate,
|
||||
ch = channels,
|
||||
fmt = format
|
||||
tracing::info!(
|
||||
target: "magnotia_audio",
|
||||
device = %name,
|
||||
sample_rate,
|
||||
channels,
|
||||
format = ?format,
|
||||
"trying audio input device"
|
||||
);
|
||||
|
||||
let (tx, rx) = mpsc::sync_channel::<AudioChunk>(AUDIO_CHANNEL_CAPACITY);
|
||||
@@ -375,8 +383,7 @@ fn open_and_validate(
|
||||
// and counted in `dropped_errors` so the symptom is visible in the
|
||||
// diagnostic bundle even when the listener has gone away. Errors
|
||||
// beyond the cap are by definition redundant noise in a stream that
|
||||
// is already failing. (Codex review 2026/04/17 M2; capacity bump and
|
||||
// drop logging added 2026/04/25 audit pass.)
|
||||
// is already failing.
|
||||
let (err_tx, err_rx) = mpsc::sync_channel::<CaptureRuntimeError>(32);
|
||||
let dropped_errors = Arc::new(AtomicU64::new(0));
|
||||
|
||||
@@ -440,9 +447,11 @@ fn open_and_validate(
|
||||
}
|
||||
match rx.recv_timeout(remaining) {
|
||||
Ok(chunk) => {
|
||||
for &s in &chunk.samples {
|
||||
sum_sq += (s as f64) * (s as f64);
|
||||
}
|
||||
sum_sq += chunk
|
||||
.samples
|
||||
.iter()
|
||||
.map(|&s| (s as f64).powi(2))
|
||||
.sum::<f64>();
|
||||
total_samples += chunk.samples.len();
|
||||
collected.push(chunk);
|
||||
}
|
||||
@@ -457,9 +466,12 @@ fn open_and_validate(
|
||||
}
|
||||
|
||||
let rms = (sum_sq / total_samples as f64).sqrt() as f32;
|
||||
eprintln!(
|
||||
"[magnotia-audio] '{name}' validation: {samples} samples, rms={rms:.6}",
|
||||
samples = total_samples
|
||||
tracing::info!(
|
||||
target: "magnotia_audio",
|
||||
device = %name,
|
||||
samples = total_samples,
|
||||
rms,
|
||||
"audio input validation complete"
|
||||
);
|
||||
|
||||
if require_audio && rms < SILENCE_RMS_FLOOR {
|
||||
@@ -471,8 +483,7 @@ fn open_and_validate(
|
||||
// Even in the fallback pass (require_audio=false), reject completely
|
||||
// dead-zero audio. PulseAudio/PipeWire will sometimes happily emit a
|
||||
// long stream of f32 zeros from a borked device — that is worse than
|
||||
// failing fast. (Codex review 2026/04/17 D3)
|
||||
const DEAD_SILENCE_FLOOR: f32 = 1e-7;
|
||||
// failing fast.
|
||||
if rms < DEAD_SILENCE_FLOOR {
|
||||
return Err(MagnotiaError::AudioCaptureFailed(format!(
|
||||
"device produced dead silence (rms={rms:.6e} below absolute floor {DEAD_SILENCE_FLOOR:.6e})"
|
||||
@@ -482,14 +493,13 @@ fn open_and_validate(
|
||||
// Re-queue the collected chunks so downstream gets them. Count any
|
||||
// drops here against the same `dropped_chunks` counter so the live
|
||||
// session sees them and can warn the user.
|
||||
// (Codex review 2026/04/17 M1)
|
||||
for chunk in collected {
|
||||
if requeue_tx.try_send(chunk).is_err() {
|
||||
dropped_chunks.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
|
||||
eprintln!("[magnotia-audio] selected microphone: '{name}'");
|
||||
tracing::info!(target: "magnotia_audio", device = %name, "selected microphone");
|
||||
Ok((
|
||||
MicrophoneCapture {
|
||||
stream: Some(stream),
|
||||
@@ -528,18 +538,16 @@ where
|
||||
sample_rate,
|
||||
channels,
|
||||
};
|
||||
// try_send fails if the channel is full. Track that explicitly
|
||||
// rather than swallowing it — Codex review 2026/04/17 caught
|
||||
// this as a silent-failure risk under sustained load.
|
||||
// try_send fails if the channel is full. Track that explicitly;
|
||||
// otherwise backpressure looks like clean transcription silence.
|
||||
if tx.try_send(chunk).is_err() {
|
||||
dropped_chunks.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
},
|
||||
move |err| {
|
||||
// Surface stream errors to the live session via err_tx so the
|
||||
// frontend can show a toast. Also keep the eprintln for ops
|
||||
// logs. (Codex review 2026/04/17 M2)
|
||||
eprintln!("[magnotia-audio] capture error: {err}");
|
||||
// frontend can show a toast.
|
||||
tracing::error!(target: "magnotia_audio", error = %err, "capture stream error");
|
||||
if err_tx
|
||||
.try_send(CaptureRuntimeError {
|
||||
device_name: err_device_name.clone(),
|
||||
@@ -547,15 +555,15 @@ where
|
||||
})
|
||||
.is_err()
|
||||
{
|
||||
// Channel full — listener has stalled or detached. Note
|
||||
// it in stderr and the dropped-errors counter so the
|
||||
// diagnostic bundle still shows the symptom even if the
|
||||
// frontend never received the typed event.
|
||||
// Channel full — listener has stalled or detached. Keep a
|
||||
// counter so the diagnostic bundle still shows the symptom
|
||||
// even if the frontend never received the typed event.
|
||||
let prior = dropped_errors.fetch_add(1, Ordering::Relaxed);
|
||||
eprintln!(
|
||||
"[magnotia-audio] capture error channel full; dropped error #{} for device '{}'",
|
||||
prior + 1,
|
||||
err_device_name,
|
||||
tracing::warn!(
|
||||
target: "magnotia_audio",
|
||||
device = %err_device_name,
|
||||
dropped_error = prior + 1,
|
||||
"capture error channel full; dropping runtime error"
|
||||
);
|
||||
}
|
||||
},
|
||||
@@ -569,15 +577,43 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn monitor_pattern_detection() {
|
||||
assert!(is_monitor_name(
|
||||
"alsa_output.pci-0000_00_1f.3.analog-stereo.monitor"
|
||||
));
|
||||
assert!(is_monitor_name("Monitor of Built-in Audio Analog Stereo"));
|
||||
assert!(is_monitor_name("Some Loopback Device"));
|
||||
assert!(!is_monitor_name("Blue Yeti USB"));
|
||||
assert!(!is_monitor_name(
|
||||
"alsa_input.pci-0000_00_1f.3.analog-stereo"
|
||||
));
|
||||
assert!(!is_monitor_name(""));
|
||||
for name in [
|
||||
"alsa_output.pci-0000_00_1f.3.analog-stereo.monitor",
|
||||
"Monitor of Built-in Audio Analog Stereo",
|
||||
"PipeWire Loopback Source",
|
||||
"Built-in Audio Monitor of Analog Stereo",
|
||||
] {
|
||||
assert!(is_monitor_name(name), "expected monitor source: {name}");
|
||||
}
|
||||
|
||||
for name in [
|
||||
"Built-in Audio Analog Stereo",
|
||||
"Blue Microphones",
|
||||
"HD Pro Webcam C920",
|
||||
"sysdefault:CARD=Microphones",
|
||||
] {
|
||||
assert!(!is_monitor_name(name), "expected physical input: {name}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_alsa_cards_with_regex() {
|
||||
let contents = r#"
|
||||
2 [Microphones ]: USB-Audio - Blue Microphones
|
||||
Blue Microphones at usb-0000:04:00.3-2.1, full speed
|
||||
3 [C920 ]: USB-Audio - HD Pro Webcam C920: With Colon
|
||||
HD Pro Webcam C920 at usb-0000:04:00.3-2.2, high speed
|
||||
"#;
|
||||
|
||||
let parsed = parse_alsa_card_descriptions(contents);
|
||||
|
||||
assert_eq!(
|
||||
parsed.get("Microphones").map(String::as_str),
|
||||
Some("Blue Microphones")
|
||||
);
|
||||
assert_eq!(
|
||||
parsed.get("C920").map(String::as_str),
|
||||
Some("HD Pro Webcam C920: With Colon")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -170,6 +170,25 @@ impl StreamingResampler {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn resampled_sine_rms(from_rate: u32, input_frequency: f32) -> f64 {
|
||||
let sample_count = from_rate as usize;
|
||||
let samples: Vec<f32> = (0..sample_count)
|
||||
.map(|i| {
|
||||
let t = i as f32 / from_rate as f32;
|
||||
(std::f32::consts::TAU * input_frequency * t).sin()
|
||||
})
|
||||
.collect();
|
||||
|
||||
let mut resampler = StreamingResampler::new(from_rate).unwrap();
|
||||
let mut produced = Vec::new();
|
||||
for chunk in samples.chunks(997) {
|
||||
produced.extend(resampler.push_samples(chunk).unwrap());
|
||||
}
|
||||
produced.extend(resampler.flush().unwrap());
|
||||
|
||||
(produced.iter().map(|&s| (s as f64).powi(2)).sum::<f64>() / produced.len() as f64).sqrt()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn passthrough_at_16khz() {
|
||||
let mut r = StreamingResampler::new(16_000).unwrap();
|
||||
@@ -183,6 +202,24 @@ mod tests {
|
||||
assert!(StreamingResampler::new(0).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn high_frequency_content_is_filtered_before_downsampling() {
|
||||
let rms = resampled_sine_rms(48_000, 12_000.0);
|
||||
assert!(
|
||||
rms < 0.01,
|
||||
"12kHz content must be low-pass filtered before 16kHz output with at least ~40dB attenuation; rms={rms}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn near_nyquist_content_is_attenuated_before_downsampling() {
|
||||
let rms = resampled_sine_rms(48_000, 9_000.0);
|
||||
assert!(
|
||||
rms < 0.05,
|
||||
"9kHz content just above 16kHz Nyquist should be materially attenuated; rms={rms}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn streaming_48k_to_16k_preserves_duration() {
|
||||
let from_rate = 48_000u32;
|
||||
|
||||
@@ -40,10 +40,10 @@ impl WavWriter {
|
||||
bits_per_sample: 16,
|
||||
sample_format: hound::SampleFormat::Int,
|
||||
};
|
||||
let file = std::fs::File::create(path).map_err(MagnotiaError::Io)?;
|
||||
let file = std::fs::File::create(path).map_err(MagnotiaError::from)?;
|
||||
let buffered = BufWriter::new(file);
|
||||
let inner = hound::WavWriter::new(buffered, spec).map_err(|e| {
|
||||
MagnotiaError::Io(std::io::Error::other(format!("WAV create failed: {e}")))
|
||||
MagnotiaError::from(std::io::Error::other(format!("WAV create failed: {e}")))
|
||||
})?;
|
||||
Ok(Self {
|
||||
inner,
|
||||
@@ -61,7 +61,7 @@ impl WavWriter {
|
||||
let clamped = sample.clamp(-1.0, 1.0);
|
||||
let int_sample = (clamped * i16::MAX as f32) as i16;
|
||||
self.inner.write_sample(int_sample).map_err(|e| {
|
||||
MagnotiaError::Io(std::io::Error::other(format!("WAV write failed: {e}")))
|
||||
MagnotiaError::from(std::io::Error::other(format!("WAV write failed: {e}")))
|
||||
})?;
|
||||
}
|
||||
self.samples_since_flush += samples.len();
|
||||
@@ -78,7 +78,7 @@ impl WavWriter {
|
||||
/// boundaries (end-of-utterance, UI events) for tighter recovery.
|
||||
pub fn flush(&mut self) -> Result<()> {
|
||||
self.inner.flush().map_err(|e| {
|
||||
MagnotiaError::Io(std::io::Error::other(format!("WAV flush failed: {e}")))
|
||||
MagnotiaError::from(std::io::Error::other(format!("WAV flush failed: {e}")))
|
||||
})?;
|
||||
self.samples_since_flush = 0;
|
||||
Ok(())
|
||||
@@ -90,7 +90,7 @@ impl WavWriter {
|
||||
/// that care about the unflushed tail should always finalise.
|
||||
pub fn finalize(self) -> Result<()> {
|
||||
self.inner.finalize().map_err(|e| {
|
||||
MagnotiaError::Io(std::io::Error::other(format!("WAV finalize failed: {e}")))
|
||||
MagnotiaError::from(std::io::Error::other(format!("WAV finalize failed: {e}")))
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -105,19 +105,20 @@ pub fn write_wav(path: &Path, audio: &AudioSamples) -> Result<()> {
|
||||
sample_format: hound::SampleFormat::Int,
|
||||
};
|
||||
|
||||
let mut writer = hound::WavWriter::create(path, spec)
|
||||
.map_err(|e| MagnotiaError::Io(std::io::Error::other(format!("WAV create failed: {e}"))))?;
|
||||
let mut writer = hound::WavWriter::create(path, spec).map_err(|e| {
|
||||
MagnotiaError::from(std::io::Error::other(format!("WAV create failed: {e}")))
|
||||
})?;
|
||||
|
||||
for &sample in audio.samples() {
|
||||
let clamped = sample.clamp(-1.0, 1.0);
|
||||
let int_sample = (clamped * i16::MAX as f32) as i16;
|
||||
writer.write_sample(int_sample).map_err(|e| {
|
||||
MagnotiaError::Io(std::io::Error::other(format!("WAV write failed: {e}")))
|
||||
MagnotiaError::from(std::io::Error::other(format!("WAV write failed: {e}")))
|
||||
})?;
|
||||
}
|
||||
|
||||
writer.finalize().map_err(|e| {
|
||||
MagnotiaError::Io(std::io::Error::other(format!("WAV finalize failed: {e}")))
|
||||
MagnotiaError::from(std::io::Error::other(format!("WAV finalize failed: {e}")))
|
||||
})?;
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -31,30 +31,31 @@ pub enum MagnotiaError {
|
||||
#[error("model download failed: {0}")]
|
||||
DownloadFailed(String),
|
||||
|
||||
#[error("file not found: {}", .0.display())]
|
||||
#[error("file not found: '{}'", .0.display())]
|
||||
FileNotFound(PathBuf),
|
||||
|
||||
#[error("storage error: {0}")]
|
||||
StorageError(String),
|
||||
|
||||
#[error("io error: {0}")]
|
||||
Io(
|
||||
#[from]
|
||||
#[serde(serialize_with = "serialize_io_error")]
|
||||
std::io::Error,
|
||||
),
|
||||
#[error("provider not registered: {0}")]
|
||||
ProviderNotRegistered(String),
|
||||
|
||||
#[error("{0}")]
|
||||
Other(String),
|
||||
#[error("io error ({kind}): {message}")]
|
||||
Io {
|
||||
kind: String,
|
||||
message: String,
|
||||
raw_os_error: Option<i32>,
|
||||
},
|
||||
}
|
||||
|
||||
/// Serialises `std::io::Error` as its display string, since it does
|
||||
/// not implement `Serialize` natively.
|
||||
fn serialize_io_error<S: serde::Serializer>(
|
||||
err: &std::io::Error,
|
||||
s: S,
|
||||
) -> std::result::Result<S::Ok, S::Error> {
|
||||
s.serialize_str(&err.to_string())
|
||||
impl From<std::io::Error> for MagnotiaError {
|
||||
fn from(err: std::io::Error) -> Self {
|
||||
Self::Io {
|
||||
kind: format!("{:?}", err.kind()),
|
||||
message: err.to_string(),
|
||||
raw_os_error: err.raw_os_error(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub type Result<T> = std::result::Result<T, MagnotiaError>;
|
||||
|
||||
@@ -49,7 +49,7 @@ pub fn score_model(model: &'static ModelEntry, profile: &SystemProfile) -> Optio
|
||||
}
|
||||
|
||||
let headroom = Megabytes(profile.ram.0.saturating_sub(model.ram_required.0));
|
||||
if headroom > Megabytes::from_gb(4.0) {
|
||||
if headroom > Megabytes::from_gb(4) {
|
||||
score += 10.0;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,11 +1,18 @@
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::borrow::Cow;
|
||||
use std::num::NonZeroU32;
|
||||
|
||||
use serde::{Deserialize, Deserializer, Serialize};
|
||||
|
||||
/// Prevents passing raw strings where model IDs are expected.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
|
||||
pub struct ModelId(String);
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize)]
|
||||
pub struct ModelId(Cow<'static, str>);
|
||||
|
||||
impl ModelId {
|
||||
pub fn new(id: impl Into<String>) -> Self {
|
||||
pub const fn borrowed(id: &'static str) -> Self {
|
||||
Self(Cow::Borrowed(id))
|
||||
}
|
||||
|
||||
pub fn new(id: impl Into<Cow<'static, str>>) -> Self {
|
||||
Self(id.into())
|
||||
}
|
||||
|
||||
@@ -14,6 +21,15 @@ impl ModelId {
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for ModelId {
|
||||
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
|
||||
where
|
||||
D: Deserializer<'de>,
|
||||
{
|
||||
String::deserialize(deserializer).map(|s| Self(Cow::Owned(s)))
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for ModelId {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.write_str(&self.0)
|
||||
@@ -21,11 +37,15 @@ impl std::fmt::Display for ModelId {
|
||||
}
|
||||
|
||||
/// Prevents passing raw strings where engine names are expected.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct EngineName(String);
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
|
||||
pub struct EngineName(Cow<'static, str>);
|
||||
|
||||
impl EngineName {
|
||||
pub fn new(name: impl Into<String>) -> Self {
|
||||
pub const fn borrowed(name: &'static str) -> Self {
|
||||
Self(Cow::Borrowed(name))
|
||||
}
|
||||
|
||||
pub fn new(name: impl Into<Cow<'static, str>>) -> Self {
|
||||
Self(name.into())
|
||||
}
|
||||
|
||||
@@ -34,6 +54,15 @@ impl EngineName {
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for EngineName {
|
||||
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
|
||||
where
|
||||
D: Deserializer<'de>,
|
||||
{
|
||||
String::deserialize(deserializer).map(|s| Self(Cow::Owned(s)))
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for EngineName {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.write_str(&self.0)
|
||||
@@ -45,8 +74,12 @@ impl std::fmt::Display for EngineName {
|
||||
pub struct Megabytes(pub u64);
|
||||
|
||||
impl Megabytes {
|
||||
pub fn from_gb(gb: f64) -> Self {
|
||||
Self((gb * 1024.0) as u64)
|
||||
pub const fn from_gb(gb: u64) -> Self {
|
||||
Self(gb.saturating_mul(1024))
|
||||
}
|
||||
|
||||
pub const fn from_mb(mb: u64) -> Self {
|
||||
Self(mb)
|
||||
}
|
||||
|
||||
pub fn as_gb(&self) -> f64 {
|
||||
@@ -68,23 +101,36 @@ impl std::fmt::Display for Megabytes {
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct AudioSamples {
|
||||
samples: Vec<f32>,
|
||||
sample_rate: u32,
|
||||
sample_rate: NonZeroU32,
|
||||
channels: u16,
|
||||
}
|
||||
|
||||
impl AudioSamples {
|
||||
pub fn new(samples: Vec<f32>, sample_rate: u32, channels: u16) -> Self {
|
||||
Self {
|
||||
Self::try_new(samples, sample_rate, channels)
|
||||
.expect("AudioSamples sample_rate must be non-zero")
|
||||
}
|
||||
|
||||
pub fn try_new(
|
||||
samples: Vec<f32>,
|
||||
sample_rate: u32,
|
||||
channels: u16,
|
||||
) -> std::result::Result<Self, &'static str> {
|
||||
let Some(sample_rate) = NonZeroU32::new(sample_rate) else {
|
||||
return Err("sample_rate must be non-zero");
|
||||
};
|
||||
Ok(Self {
|
||||
samples,
|
||||
sample_rate,
|
||||
channels,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
pub fn mono_16khz(samples: Vec<f32>) -> Self {
|
||||
Self {
|
||||
samples,
|
||||
sample_rate: crate::constants::WHISPER_SAMPLE_RATE,
|
||||
sample_rate: NonZeroU32::new(crate::constants::WHISPER_SAMPLE_RATE)
|
||||
.expect("WHISPER_SAMPLE_RATE must be non-zero"),
|
||||
channels: crate::constants::WHISPER_CHANNELS,
|
||||
}
|
||||
}
|
||||
@@ -98,7 +144,7 @@ impl AudioSamples {
|
||||
}
|
||||
|
||||
pub fn sample_rate(&self) -> u32 {
|
||||
self.sample_rate
|
||||
self.sample_rate.get()
|
||||
}
|
||||
|
||||
pub fn channels(&self) -> u16 {
|
||||
@@ -106,10 +152,7 @@ impl AudioSamples {
|
||||
}
|
||||
|
||||
pub fn duration_secs(&self) -> f64 {
|
||||
if self.sample_rate == 0 {
|
||||
return 0.0;
|
||||
}
|
||||
self.samples.len() as f64 / self.sample_rate as f64
|
||||
self.samples.len() as f64 / self.sample_rate.get() as f64
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -122,12 +122,9 @@ impl Orchestrator {
|
||||
/// Resolve the provider for a profile. Returns a clear error when
|
||||
/// the profile names an unregistered engine.
|
||||
pub fn resolve(&self, profile: &EngineProfile) -> Result<Arc<dyn TranscriptionProvider>> {
|
||||
self.registry.get(&profile.engine_id).ok_or_else(|| {
|
||||
MagnotiaError::Other(format!(
|
||||
"Provider '{}' is not registered",
|
||||
profile.engine_id
|
||||
))
|
||||
})
|
||||
self.registry
|
||||
.get(&profile.engine_id)
|
||||
.ok_or_else(|| MagnotiaError::ProviderNotRegistered(profile.engine_id.to_string()))
|
||||
}
|
||||
|
||||
/// Transcribe audio using the provider named in the profile. The
|
||||
|
||||
173
docs/superpowers/plans/2026-05-12-engine-slop-residuals.md
Normal file
173
docs/superpowers/plans/2026-05-12-engine-slop-residuals.md
Normal file
@@ -0,0 +1,173 @@
|
||||
---
|
||||
name: Engine slop residuals (post-prognosis pass)
|
||||
type: plan
|
||||
tags: [engine, slop-cleanup, plan, deferred]
|
||||
description: Roadmap for the deferred code-quality items that did not fit in the 2026-05-12 prognosis-fix pass. Each area is scoped for its own focused plan when ready to execute.
|
||||
---
|
||||
|
||||
# Engine slop residuals — post-prognosis pass
|
||||
|
||||
> **For agentic workers:** This is a meta-plan. It catalogues independent residual work areas, each of which should get its own detailed TDD plan when scheduled. Do **not** treat the sections below as a single execution checklist.
|
||||
|
||||
## Context
|
||||
|
||||
On 2026-05-12 an external code review rated the codebase 4/10 and laid out a four-bucket de-sloppifying plan. That pass shipped (uncommitted in working tree at time of writing). It hit the *prognosis-level* items only:
|
||||
|
||||
- DSP: naive decimation removed, `StreamingResampler`/rubato in place, regression tests at 12 kHz (40 dB) + 9 kHz (26 dB).
|
||||
- Core errors: `Other(String)` removed; `Io` is structured; `FileNotFound` quotes paths; `ProviderNotRegistered` introduced.
|
||||
- Core types: `Cow<'static, str>` for `ModelId`/`EngineName`, `NonZeroU32` for `AudioSamples::sample_rate`, `Megabytes::from_gb` takes `u64`.
|
||||
- Capture cleanup: regex-based `/proc/asound/cards` parser (test covers product names with embedded colons), constants documented, RMS sum idiomatic, Codex-review comments removed.
|
||||
- Tauri startup: `unsafe std::env::set_var` removed (logs warning instead); function renamed `warn_if_x11_env_unset_on_wayland`; setup collapsed to single `block_on`; JS injection rewritten (`var p = {...}` direct, no `JSON.parse`); WebKitGTK mic auto-grant logs warning at startup.
|
||||
- Counter: `RECORDING_COUNTER` uses `SeqCst`.
|
||||
- Tracing: subscriber initialised at top of `run()` with sensible default filter, honors `RUST_LOG`, writes to stderr.
|
||||
|
||||
The code-proposal and macro-architecture captures from the same review session went further (10/10 demands and workspace-level critique). Those items are the residuals below.
|
||||
|
||||
## Residual areas
|
||||
|
||||
Each area is independent. They can be tackled in any order, but the suggested order minimises rework.
|
||||
|
||||
### A. Storage-layer typed errors
|
||||
|
||||
**Status:** identified, not started.
|
||||
|
||||
**Scope:** ~25 `StorageError(String)` sites in `crates/storage/src/{database.rs, migrations.rs}`.
|
||||
|
||||
**Why it matters:** The frontend cannot distinguish "schema migration failed" from "FTS search failed" from "transcript not found" — all surface as one stringly-typed variant. This is the same class of bug that motivated removing `Other(String)`.
|
||||
|
||||
**Approach:** Split `StorageError` into a `StorageError` enum (sub-error in its own right) with variants like `Connect { path, source }`, `MigrationFailed { version, source }`, `TransactionFailed { phase, source }`, `QueryFailed { kind, source }`, `NotFound { table, id }`. Wrap `sqlx::Error` (or whatever `rusqlite::Error` is in use) as `#[source]`.
|
||||
|
||||
**Decisions needed:**
|
||||
- Is `sqlx::Error`/`rusqlite::Error` serializable across the Tauri boundary? If not, what is preserved (kind + display)?
|
||||
- Do we keep one flat error type for the crate or nest by domain (`MigrationError`, `TranscriptError`, `TaskError`)?
|
||||
- Backwards-compat for any existing error-handling code in frontend.
|
||||
|
||||
**Files in scope:**
|
||||
- `crates/core/src/error.rs` — add `StorageError` variant if changing top-level shape
|
||||
- `crates/storage/src/error.rs` — new file (likely)
|
||||
- `crates/storage/src/database.rs` — 19 call sites
|
||||
- `crates/storage/src/migrations.rs` — 6 call sites
|
||||
|
||||
**Acceptance:** All `StorageError(format!(...))` calls replaced with typed variants. Frontend can `match` on storage-error kind. `cargo test --workspace` green.
|
||||
|
||||
### B. eprintln → tracing sweep (rest of repo)
|
||||
|
||||
**Status:** identified, not started.
|
||||
|
||||
**Scope:** ~30 `eprintln!` sites in production code paths (excluding tests and tooling binaries — see below).
|
||||
|
||||
**Why it matters:** Now that the subscriber is wired, every `eprintln!` is a missed observability signal. The startup/capture paths are clean; the runtime hot paths are not.
|
||||
|
||||
**Approach:** Mechanical migration, one file at a time, with appropriate target and structured fields.
|
||||
|
||||
**Files in scope (production):**
|
||||
- `src-tauri/src/commands/live.rs` — 8 sites (highest priority — session lifecycle + inference errors)
|
||||
- `src-tauri/src/commands/models.rs` — 5 sites (Whisper warmup)
|
||||
- `src-tauri/src/commands/power.rs` — 4 sites (macOS App Nap)
|
||||
- `src-tauri/src/commands/transcription.rs` — 1 site
|
||||
- `src-tauri/src/commands/diagnostics.rs` — 1 site
|
||||
- `src-tauri/src/commands/tasks.rs` — 1 site
|
||||
- `crates/hotkey/src/linux.rs` — 2 sites
|
||||
|
||||
**Files explicitly out of scope:**
|
||||
- `crates/mcp/src/main.rs` — separate binary, its own stdio protocol; keep eprintln as-is unless that binary gets its own subscriber.
|
||||
- `crates/*/tests/*.rs` — test diagnostic output, `--nocapture` semantics depend on stdio.
|
||||
|
||||
**Decisions needed:**
|
||||
- New tracing target names for `commands::live`, `commands::models`, etc. — extend the default filter in `src-tauri/src/lib.rs::init_tracing`.
|
||||
|
||||
**Acceptance:** No `eprintln!` in `src-tauri/src/commands/` or `crates/hotkey/src/linux.rs`. Default filter shows live-session events at INFO. `cargo test --workspace` green.
|
||||
|
||||
### C. Actor-model refactor for capture + inference
|
||||
|
||||
**Status:** identified, not started. **Largest scope.**
|
||||
|
||||
**Scope:** Replace `Arc<Mutex<Vec<f32>>>` shared-state pattern in `CaptureWorker` (and analogous patterns in live inference) with isolated actor loops + `mpsc` channels.
|
||||
|
||||
**Why it matters:** Reviewer's core architectural critique. Shared mutable state across async boundaries with `std::sync::Mutex` risks executor stalls under contention. Architecturally also makes it impossible to add features like multi-source capture, parallel inference, or recording-while-streaming without lock contention.
|
||||
|
||||
**Approach:** Spawn dedicated tokio tasks for the capture worker and the inference worker. Communicate exclusively via bounded `mpsc` channels. Tauri command handlers hold only channel senders.
|
||||
|
||||
**Decisions needed (before plan):**
|
||||
- Bounded vs unbounded mpsc? Recommend bounded for backpressure visibility.
|
||||
- Channel capacity? (current ad-hoc cap is `AUDIO_CHANNEL_CAPACITY = 32`)
|
||||
- Backpressure policy when capture outpaces inference: drop oldest, drop newest, await consumer?
|
||||
- Worker shutdown protocol: drop-channel-and-await, explicit stop message, or cancellation token?
|
||||
- How are runtime errors (already typed via `CaptureRuntimeError`) propagated through the actor boundary?
|
||||
- Migration plan: parallel implementation behind a feature flag, or in-place replacement?
|
||||
|
||||
**Risk:** This is a substantial refactor. Likely worth a brainstorming session before plan-writing.
|
||||
|
||||
**Files in scope:**
|
||||
- `crates/audio/src/capture.rs` — worker lifecycle rewrite
|
||||
- `src-tauri/src/commands/audio.rs` — command handler refactor
|
||||
- `src-tauri/src/commands/live.rs` — inference worker integration
|
||||
- Likely new files: `crates/audio/src/capture_actor.rs`, `crates/transcription/src/inference_actor.rs`
|
||||
|
||||
**Acceptance:** No `Arc<Mutex<T>>` across `.await` points in capture or inference code paths. Capture and inference are independent tokio tasks. End-to-end recording flow works in dev mode. `cargo test --workspace` green. Manual dogfood pass.
|
||||
|
||||
### D. Property-based DSP testing
|
||||
|
||||
**Status:** identified, not started.
|
||||
|
||||
**Scope:** Add `proptest` (or similar) generators for audio inputs and assert invariants of the resampling and VAD pipelines.
|
||||
|
||||
**Why it matters:** The current DSP tests are point checks at 12 kHz and 9 kHz. The reviewer's 10/10 demand is property-based testing that covers the input space (any frequency, any chunk size, any noise floor) and asserts invariants (output is bounded, energy is bounded, duration is approximately preserved).
|
||||
|
||||
**Approach:** Add `proptest` dev-dependency. Write generators for `(sample_rate, signal_frequency, amplitude, chunk_size, total_secs)` tuples. Assert:
|
||||
- Output samples are finite (no NaN/Inf).
|
||||
- Output peak ≤ input peak + small tolerance (no amplification).
|
||||
- Output duration within ±5% of expected.
|
||||
- Output energy at frequencies above Nyquist of target rate is at least 30 dB below input energy at those frequencies.
|
||||
|
||||
**Decisions needed:**
|
||||
- `proptest` vs `quickcheck`? Recommend `proptest` (more flexible shrinking).
|
||||
- Run as part of standard `cargo test` or as a separate `cargo test --features slow-tests`?
|
||||
|
||||
**Files in scope:**
|
||||
- `crates/audio/Cargo.toml` — add `proptest` to `[dev-dependencies]`
|
||||
- `crates/audio/src/streaming_resample.rs` — extend `#[cfg(test)]` module
|
||||
- Likely also `crates/transcription/src/streaming/rms_vad.rs` — VAD has equally testable invariants
|
||||
|
||||
**Acceptance:** `cargo test -p magnotia-audio` includes property tests. Property tests have run with at least 1000 cases per invariant.
|
||||
|
||||
### E. Frontend/backend error boundary cleanup
|
||||
|
||||
**Status:** partially complete (orchestrator's `ProviderNotRegistered`).
|
||||
|
||||
**Scope:** Audit every Tauri command's return type and ensure errors propagate with full type information across the IPC boundary. Currently many commands return `Result<T, String>` instead of `Result<T, MagnotiaError>`.
|
||||
|
||||
**Why it matters:** Reviewer's frontend-backend boundary critique. The frontend can't `switch` on error kind if the backend serialises errors as strings.
|
||||
|
||||
**Approach:** Inventory all `#[tauri::command]` functions, identify those returning `Result<T, String>`, convert to `Result<T, MagnotiaError>` (or a command-specific error enum that serializes cleanly). Update frontend handlers to use the typed shape.
|
||||
|
||||
**Decisions needed:**
|
||||
- One `MagnotiaError` for all commands, or per-command enums?
|
||||
- How do we surface the new shape to the Svelte side without manually maintaining TypeScript types? (specta? hand-rolled?)
|
||||
|
||||
**Files in scope:**
|
||||
- All files in `src-tauri/src/commands/`
|
||||
- All call sites in `src/lib/` (Svelte)
|
||||
|
||||
**Acceptance:** No `Result<T, String>` in `src-tauri/src/commands/`. Frontend has typed error access. `cargo test --workspace` green; frontend builds.
|
||||
|
||||
## Suggested sequencing
|
||||
|
||||
1. **B (eprintln sweep)** first — mechanical, low-risk, immediate observability win.
|
||||
2. **A (storage typed errors)** next — well-scoped, no architectural change, unblocks E.
|
||||
3. **D (property-based DSP)** in parallel with A — independent surface, can be done by anyone any time.
|
||||
4. **E (FE/BE error boundary)** after A — depends on storage error shape.
|
||||
5. **C (actor model)** last — biggest surface, deserves its own brainstorm + plan + likely its own branch.
|
||||
|
||||
## Cross-cutting deferrals (Phase 10a-blocking)
|
||||
|
||||
Two things are explicitly **not** in these residuals because they belong to higher-level workstreams:
|
||||
|
||||
- **Lumotia rebrand cascade** (Wyrdnote → Lumotia in code, paths, GitHub repo, bundle ID). Hold until A–B done so rename diff doesn't hide regressions.
|
||||
- **Phase 10a QC dogfood** — `docs/superpowers/audits/2026-05-10-phase10a-dogfood-notes.md` is queued. Best done after B at minimum so live-session logs are visible.
|
||||
|
||||
## What this plan does not do
|
||||
|
||||
- No fix recipes for bugs not yet found. Each area's plan should start by reading the current state and writing failing tests against the current behaviour.
|
||||
- No timeline commitments. Sequencing is suggested, not mandatory.
|
||||
- No architectural decisions deferred to "Phase B" earlier are pre-committed here. Each area writes its own brainstorm + plan when scheduled.
|
||||
@@ -52,6 +52,8 @@ tauri-plugin-notification = "2"
|
||||
# Serialisation
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
serde_json = "1"
|
||||
tracing = "0.1"
|
||||
tracing-subscriber = { version = "0.3", features = ["env-filter"] }
|
||||
|
||||
# Async runtime (spawn_blocking for inference)
|
||||
tokio = { version = "1", features = ["rt", "sync"] }
|
||||
|
||||
@@ -60,7 +60,7 @@ fn recording_filename() -> String {
|
||||
.unwrap_or_default();
|
||||
let secs = duration.as_secs();
|
||||
let nanos = duration.subsec_nanos();
|
||||
let counter = RECORDING_COUNTER.fetch_add(1, Ordering::Relaxed);
|
||||
let counter = RECORDING_COUNTER.fetch_add(1, Ordering::SeqCst);
|
||||
format!("magnotia-{secs}-{nanos:09}-{counter:04}.wav")
|
||||
}
|
||||
|
||||
|
||||
@@ -25,14 +25,14 @@ fn whisper_model_id(size: &str) -> ModelId {
|
||||
"distil-large" | "distil-large-v3" | "distillarge" => {
|
||||
ModelId::new("whisper-distil-large-v3")
|
||||
}
|
||||
other => ModelId::new(other),
|
||||
other => ModelId::new(other.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
fn parakeet_model_id(name: &str) -> ModelId {
|
||||
match name {
|
||||
"ctc-int8" => ModelId::new("parakeet-ctc-0.6b-int8"),
|
||||
other => ModelId::new(other),
|
||||
other => ModelId::new(other.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -121,7 +121,7 @@ pub async fn ensure_model_loaded(
|
||||
model_id: &str,
|
||||
concurrent: Option<bool>,
|
||||
) -> Result<(), String> {
|
||||
let model_id = ModelId::new(model_id);
|
||||
let model_id = ModelId::new(model_id.to_string());
|
||||
let entry = model_registry::find_model(&model_id)
|
||||
.ok_or_else(|| format!("Unknown model: {model_id}"))?;
|
||||
|
||||
|
||||
@@ -5,10 +5,12 @@ mod commands;
|
||||
mod tray;
|
||||
|
||||
use std::sync::Arc;
|
||||
use std::sync::Once;
|
||||
use std::time::Instant;
|
||||
|
||||
use sqlx::SqlitePool;
|
||||
use tauri::Manager;
|
||||
use tracing_subscriber::EnvFilter;
|
||||
|
||||
use magnotia_core::types::EngineName;
|
||||
use magnotia_llm::LlmEngine;
|
||||
@@ -20,6 +22,8 @@ use magnotia_storage::{database_path, get_setting, init as init_db, prune_error_
|
||||
const ERROR_LOG_RETENTION_DAYS: i64 = 90;
|
||||
use magnotia_transcription::LocalEngine;
|
||||
|
||||
static TRACING_INIT: Once = Once::new();
|
||||
|
||||
/// Shared app state holding the transcription engines and database pool.
|
||||
pub struct AppState {
|
||||
pub whisper_engine: Arc<LocalEngine>,
|
||||
@@ -38,12 +42,15 @@ fn build_preferences_script(prefs_json: Option<String>) -> String {
|
||||
if json.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
// Serialise the JSON string as a JS string literal for safe embedding
|
||||
let js_str = serde_json::to_string(&json).unwrap_or_else(|_| "\"\"".to_string());
|
||||
let js_value: serde_json::Value = match serde_json::from_str(&json) {
|
||||
Ok(value) => value,
|
||||
Err(_) => return String::new(),
|
||||
};
|
||||
let js_obj = serde_json::to_string(&js_value).unwrap_or_else(|_| "{}".to_string());
|
||||
format!(
|
||||
r#"(function() {{
|
||||
try {{
|
||||
var p = JSON.parse({js_str});
|
||||
var p = {js_obj};
|
||||
var el = document.documentElement;
|
||||
if (p.theme === 'system') {{
|
||||
el.dataset.theme = window.matchMedia('(prefers-color-scheme: light)').matches ? 'light' : 'dark';
|
||||
@@ -67,6 +74,23 @@ fn build_preferences_script(prefs_json: Option<String>) -> String {
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::build_preferences_script;
|
||||
|
||||
#[test]
|
||||
fn preferences_script_injects_object_without_redundant_json_parse() {
|
||||
let script = build_preferences_script(Some(r#"{"theme":"dark"}"#.to_string()));
|
||||
assert!(script.contains("var p = {\"theme\":\"dark\"};"));
|
||||
assert!(!script.contains("JSON.parse"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preferences_script_rejects_malformed_json() {
|
||||
assert!(build_preferences_script(Some("not json".to_string())).is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
/// Save preferences JSON to the SQLite settings table.
|
||||
#[tauri::command]
|
||||
async fn save_preferences(
|
||||
@@ -87,23 +111,25 @@ async fn save_preferences(
|
||||
/// WEBKIT_DISABLE_DMABUF_RENDERER=1 npm run tauri dev
|
||||
/// ```
|
||||
///
|
||||
/// Detect the Wayland session at startup and apply the env vars before
|
||||
/// anything else loads, so users do not need to remember the prefix and
|
||||
/// the dev launcher / packaged app both work out of the box.
|
||||
/// Detect the Wayland session at startup and warn when the process was not
|
||||
/// launched with the safe Linux rendering env vars. Rust 2024 marks runtime
|
||||
/// environment mutation unsafe in multi-threaded programs, so the app must be
|
||||
/// launched with these values by the desktop file, package wrapper, or dev
|
||||
/// shell rather than patching them here.
|
||||
///
|
||||
/// Inspired by Open-Whispr's similar Chromium self-relaunch (with
|
||||
/// --ozone-platform=x11). Day 6 of the upgrade plan.
|
||||
#[cfg(target_os = "linux")]
|
||||
fn ensure_x11_on_wayland() {
|
||||
let set_if_unset = |key: &str, value: &str, why: &str| {
|
||||
fn warn_if_x11_env_unset_on_wayland() {
|
||||
let warn_if_unset = |key: &str, value: &str, why: &str| {
|
||||
if std::env::var_os(key).is_none() {
|
||||
// SAFETY: setting env vars before any threads spawn (we are
|
||||
// pre-Tauri-Builder here). This block is the only place these
|
||||
// are written.
|
||||
unsafe {
|
||||
std::env::set_var(key, value);
|
||||
}
|
||||
eprintln!("[startup] Linux workaround: {key}={value} ({why})");
|
||||
tracing::warn!(
|
||||
target: "magnotia_startup",
|
||||
key,
|
||||
expected = value,
|
||||
why,
|
||||
"Linux rendering workaround env var is not set; configure the launcher instead of mutating process env at runtime"
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -115,7 +141,7 @@ fn ensure_x11_on_wayland() {
|
||||
// magnotia idle CPU 12.30% → 2.80% of one core and idle GPU 17% → 10%.
|
||||
// Apples to both X11 and Wayland sessions; users can opt back in by
|
||||
// setting WEBKIT_DISABLE_DMABUF_RENDERER=0 explicitly.
|
||||
set_if_unset(
|
||||
warn_if_unset(
|
||||
"WEBKIT_DISABLE_DMABUF_RENDERER",
|
||||
"1",
|
||||
"iGPU idle-cost workaround",
|
||||
@@ -128,15 +154,32 @@ fn ensure_x11_on_wayland() {
|
||||
// path.
|
||||
let session_type = std::env::var("XDG_SESSION_TYPE").unwrap_or_default();
|
||||
if session_type.eq_ignore_ascii_case("wayland") {
|
||||
set_if_unset("GDK_BACKEND", "x11", "Wayland XWayland fallback");
|
||||
set_if_unset("WINIT_UNIX_BACKEND", "x11", "Wayland XWayland fallback");
|
||||
warn_if_unset("GDK_BACKEND", "x11", "Wayland XWayland fallback");
|
||||
warn_if_unset("WINIT_UNIX_BACKEND", "x11", "Wayland XWayland fallback");
|
||||
}
|
||||
}
|
||||
|
||||
fn init_tracing() {
|
||||
TRACING_INIT.call_once(|| {
|
||||
let env_filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| {
|
||||
EnvFilter::new(
|
||||
"warn,magnotia=info,magnotia_lib=info,magnotia_audio=info,magnotia_startup=info",
|
||||
)
|
||||
});
|
||||
|
||||
let _ = tracing_subscriber::fmt()
|
||||
.with_env_filter(env_filter)
|
||||
.with_writer(std::io::stderr)
|
||||
.try_init();
|
||||
});
|
||||
}
|
||||
|
||||
#[cfg_attr(mobile, tauri::mobile_entry_point)]
|
||||
pub fn run() {
|
||||
init_tracing();
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
ensure_x11_on_wayland();
|
||||
warn_if_x11_env_unset_on_wayland();
|
||||
|
||||
// Capture Rust panics to disk so the diagnostic-report bundler in
|
||||
// Settings → About can attach them. Local only; nothing transmitted.
|
||||
@@ -178,37 +221,40 @@ pub fn run() {
|
||||
|
||||
builder
|
||||
.setup(|app| {
|
||||
// Initialise database (blocking in setup — runs once at startup)
|
||||
// Initialise database and startup settings in one runtime entry.
|
||||
let db_path = database_path();
|
||||
let (db, init_script) = tauri::async_runtime::block_on(async {
|
||||
let t0 = Instant::now();
|
||||
let db = tauri::async_runtime::block_on(async { init_db(&db_path).await })
|
||||
let db = init_db(&db_path)
|
||||
.await
|
||||
.map_err(|e| Box::new(e) as Box<dyn std::error::Error>)?;
|
||||
eprintln!("[startup] DB init: {:?}", t0.elapsed());
|
||||
tracing::info!(target: "magnotia_startup", elapsed_ms = t0.elapsed().as_millis(), "DB init complete");
|
||||
|
||||
// Prune old `error_log` rows so the table doesn't grow unbounded
|
||||
// across months of dogfooding. Best-effort — a prune failure is
|
||||
// not worth blocking startup over.
|
||||
let t_prune = Instant::now();
|
||||
let pruned = tauri::async_runtime::block_on(async {
|
||||
prune_error_log(&db, ERROR_LOG_RETENTION_DAYS).await
|
||||
});
|
||||
match pruned {
|
||||
Ok(n) if n > 0 => eprintln!(
|
||||
"[startup] Error log prune: {n} rows removed (>{ERROR_LOG_RETENTION_DAYS}d) in {:?}",
|
||||
t_prune.elapsed()
|
||||
match prune_error_log(&db, ERROR_LOG_RETENTION_DAYS).await {
|
||||
Ok(n) if n > 0 => tracing::info!(
|
||||
target: "magnotia_startup",
|
||||
rows_removed = n,
|
||||
retention_days = ERROR_LOG_RETENTION_DAYS,
|
||||
elapsed_ms = t_prune.elapsed().as_millis(),
|
||||
"error log prune complete"
|
||||
),
|
||||
Ok(_) => {}
|
||||
Err(e) => eprintln!("[startup] Error log prune failed: {e}"),
|
||||
Err(e) => tracing::warn!(target: "magnotia_startup", error = %e, "error log prune failed"),
|
||||
}
|
||||
|
||||
// Load saved preferences for webview injection
|
||||
// Load saved preferences for webview injection.
|
||||
let t1 = Instant::now();
|
||||
let prefs_json = tauri::async_runtime::block_on(async {
|
||||
get_setting(&db, "magnotia_preferences").await.unwrap_or(None)
|
||||
});
|
||||
eprintln!("[startup] Preferences load: {:?}", t1.elapsed());
|
||||
let prefs_json = get_setting(&db, "magnotia_preferences").await.unwrap_or(None);
|
||||
tracing::info!(target: "magnotia_startup", elapsed_ms = t1.elapsed().as_millis(), "preferences load complete");
|
||||
let init_script = build_preferences_script(prefs_json);
|
||||
|
||||
Ok::<_, Box<dyn std::error::Error>>((db, init_script))
|
||||
})?;
|
||||
|
||||
// Apply preferences to the main window (defined in tauri.conf.json)
|
||||
if let Some(main_window) = app.get_webview_window("main") {
|
||||
if !init_script.is_empty() {
|
||||
@@ -240,6 +286,11 @@ pub fn run() {
|
||||
settings.set_enable_media_capabilities(true);
|
||||
}
|
||||
|
||||
tracing::warn!(
|
||||
target: "magnotia_startup",
|
||||
"Linux WebKitGTK microphone permission requests are auto-granted for audio-only capture; other permission classes remain denied"
|
||||
);
|
||||
|
||||
// Auto-grant microphone capture only. Other WebKitGTK
|
||||
// permission requests are denied so future surfaces do
|
||||
// not inherit camera/geolocation/pointer-lock access.
|
||||
@@ -270,8 +321,10 @@ pub fn run() {
|
||||
// Falling back means getUserMedia() prompts (or
|
||||
// silently denies) instead of auto-granting,
|
||||
// which is degraded but recoverable.
|
||||
eprintln!(
|
||||
"[startup] failed to configure webview media permissions: {e}",
|
||||
tracing::warn!(
|
||||
target: "magnotia_startup",
|
||||
error = %e,
|
||||
"failed to configure webview media permissions"
|
||||
);
|
||||
});
|
||||
}
|
||||
@@ -314,7 +367,7 @@ pub fn run() {
|
||||
|
||||
#[cfg(not(target_os = "android"))]
|
||||
if let Err(e) = tray::setup(app) {
|
||||
eprintln!("Failed to setup tray: {e}");
|
||||
tracing::warn!(target: "magnotia_startup", error = %e, "failed to setup tray");
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
||||
Reference in New Issue
Block a user