From daf88e0b42c57a204589f7bc0497839009c51b68 Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:03:45 +0800 Subject: [PATCH 01/13] feat(config)!: drop legacy YAML/JSON config migration A config.yaml, config.yml or config.json without a config.toml now fails with an error pointing at config.toml and `sumvox init`, instead of being migrated or silently replaced by defaults. `init` no longer treats legacy files as an existing config. serde_yaml is removed. BREAKING CHANGE: legacy YAML/JSON configs are no longer read or migrated. Co-Authored-By: Claude Opus 5.5 (1M context) --- Cargo.lock | 20 ----- Cargo.toml | 1 - src/config.rs | 219 +++----------------------------------------------- src/main.rs | 25 ++---- tests/cli.rs | 33 +++++--- 5 files changed, 40 insertions(+), 258 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c1d7fa..cf6a344 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1371,19 +1371,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_yaml" -version = "0.9.34+deprecated" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" -dependencies = [ - "indexmap", - "itoa", - "ryu", - "serde", - "unsafe-libyaml", -] - [[package]] name = "sharded-slab" version = "0.1.7" @@ -1487,7 +1474,6 @@ dependencies = [ "reqwest", "serde", "serde_json", - "serde_yaml", "shellexpand", "tempfile", "thiserror 1.0.69", @@ -1893,12 +1879,6 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" -[[package]] -name = "unsafe-libyaml" -version = "0.2.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" - [[package]] name = "untrusted" version = "0.9.0" diff --git a/Cargo.toml b/Cargo.toml index 2d5d440..25d41f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,6 @@ tokio = { version = "1", features = ["rt-multi-thread", "macros", "fs", "io-util reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls", "charset", "http2", "macos-system-configuration"] } serde = { version = "1", features = ["derive"] } serde_json = "1" -serde_yaml = "0.9" toml = "0.8" chrono = { version = "0.4", features = ["serde"] } thiserror = "1" diff --git a/src/config.rs b/src/config.rs index 42cfb1c..e967a4c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,6 +1,5 @@ // Configuration loading and validation -// Unified config at ~/.config/sumvox/config.toml with array-based provider fallback; -// a legacy config.yaml or config.json is migrated to TOML on load +// Unified config at ~/.config/sumvox/config.toml with array-based provider fallback use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -562,64 +561,35 @@ impl SumvoxConfig { Ok(home.join(".config").join("sumvox")) } - /// Get the standard config path: ~/.config/sumvox/config.json (deprecated) - pub fn config_path() -> Result { - Ok(Self::config_dir()?.join("config.json")) - } - - /// Get the YAML config path: ~/.config/sumvox/config.yaml - pub fn yaml_config_path() -> Result { - Ok(Self::config_dir()?.join("config.yaml")) - } - /// Get the TOML config path: ~/.config/sumvox/config.toml pub fn toml_config_path() -> Result { Ok(Self::config_dir()?.join("config.toml")) } - /// Load configuration from ~/.config/sumvox/config.toml (preferred) with auto-migration + /// Load configuration from ~/.config/sumvox/config.toml, or defaults when none exists pub fn load_from_home() -> Result { - // Priority 1: Try TOML (new format) let toml_path = Self::toml_config_path()?; if toml_path.exists() { tracing::info!("Loading config from {:?}", toml_path); return Self::load_toml(toml_path); } - // Priority 2: Try migrating from YAML/JSON - if let Some(migrated_path) = Self::migrate_legacy_config()? { - tracing::info!("Auto-migrated legacy config: {:?}", migrated_path); - return Self::load_toml(Self::toml_config_path()?); + // A YAML/JSON config is no longer read; silently using defaults would hide it + for legacy in ["config.yaml", "config.yml", "config.json"] { + let legacy_path = Self::config_dir()?.join(legacy); + if legacy_path.exists() { + return Err(VoiceError::Config(format!( + "Found legacy config {:?} but no config.toml. YAML/JSON configs are no longer supported: convert it to {:?} or run `sumvox init`.", + legacy_path, toml_path + ))); + } } - // Priority 3: No config file found, use defaults + // No config file found, use defaults tracing::info!("No config file found, using defaults"); Ok(Self::default()) } - /// Load configuration from a JSON file - pub fn load_json(path: PathBuf) -> Result { - let content = std::fs::read_to_string(&path).map_err(|e| { - VoiceError::Config(format!("Failed to read config file {:?}: {}", path, e)) - })?; - - let config: SumvoxConfig = serde_json::from_str(&content)?; - config.validate()?; - Ok(config) - } - - /// Load configuration from a YAML file - pub fn load_yaml(path: PathBuf) -> Result { - let content = std::fs::read_to_string(&path).map_err(|e| { - VoiceError::Config(format!("Failed to read config file {:?}: {}", path, e)) - })?; - - let config: SumvoxConfig = serde_yaml::from_str(&content) - .map_err(|e| VoiceError::Config(format!("Failed to parse YAML config: {}", e)))?; - config.validate()?; - Ok(config) - } - /// Load configuration from a TOML file pub fn load_toml(path: PathBuf) -> Result { let content = std::fs::read_to_string(&path).map_err(|e| { @@ -649,64 +619,6 @@ impl SumvoxConfig { self.save_toml(config_path) } - /// Backup a config file with timestamp - fn backup_config(path: &std::path::Path) -> Result { - if !path.exists() { - return Err(VoiceError::Config(format!( - "Config file {:?} does not exist", - path - ))); - } - let timestamp = chrono::Utc::now().format("%Y%m%d-%H%M%S"); - let backup_name = format!( - "{}.backup-{}", - path.file_name().unwrap().to_string_lossy(), - timestamp - ); - let backup_path = path.parent().unwrap().join(backup_name); - std::fs::copy(path, &backup_path) - .map_err(|e| VoiceError::Config(format!("Failed to backup config: {}", e)))?; - tracing::info!("Created backup: {:?}", backup_path); - Ok(backup_path) - } - - /// Migrate legacy config (YAML/JSON) to TOML - fn migrate_legacy_config() -> Result> { - let yaml_path = Self::yaml_config_path()?; - let json_path = Self::config_path()?; - - let (source_path, format_name) = if yaml_path.exists() { - (yaml_path, "YAML") - } else if json_path.exists() { - (json_path, "JSON") - } else { - return Ok(None); // No legacy config - }; - - tracing::info!( - "Migrating {} config to TOML: {:?}", - format_name, - source_path - ); - - // Load legacy config - let config = if format_name == "YAML" { - Self::load_yaml(source_path.clone())? - } else { - Self::load_json(source_path.clone())? - }; - - // Backup original file - Self::backup_config(&source_path)?; - - // Save as TOML - let toml_path = Self::toml_config_path()?; - config.save_toml(toml_path.clone())?; - - tracing::info!("Migration completed: {} -> TOML", format_name); - Ok(Some(source_path)) - } - /// Validate configuration fn validate(&self) -> Result<()> { // Validate LLM parameters @@ -773,56 +685,8 @@ impl SumvoxConfig { #[cfg(test)] mod tests { use super::*; - use std::io::Write; use tempfile::NamedTempFile; - #[test] - fn test_load_new_format() { - let config_json = r#"{ - "enabled": true, - "llm": { - "providers": [ - { - "name": "google", - "model": "gemini-2.5-flash", - "api_key": "test-key", - "timeout": 10 - }, - { - "name": "ollama", - "model": "llama3.2" - } - ], - "parameters": { - "max_tokens": 100, - "temperature": 0.3 - } - }, - "tts": { - "providers": [ - { - "name": "macos", - "voice": "Tingting", - "rate": 200 - } - ] - } - }"#; - - let mut temp_file = NamedTempFile::new().unwrap(); - temp_file.write_all(config_json.as_bytes()).unwrap(); - let path = temp_file.path().to_path_buf(); - - let config = SumvoxConfig::load_json(path).unwrap(); - assert_eq!(config.llm.providers.len(), 2); - assert_eq!(config.llm.providers[0].name, "google"); - assert_eq!( - config.llm.providers[0].api_key, - Some("test-key".to_string()) - ); - assert_eq!(config.tts.providers[0].name, "macos"); - } - fn openai_tts_provider(api_key: Option) -> TtsProviderConfig { TtsProviderConfig { name: "openai".to_string(), @@ -866,8 +730,7 @@ mod tests { toml_str ); - // The same cast applies to the optional per-provider knobs, which a - // YAML -> TOML migration rewrites. + // The same cast applies to the optional per-provider knobs. let provider = TtsProviderConfig { name: "elevenlabs".to_string(), model: None, @@ -954,46 +817,6 @@ mod tests { assert!(json.contains("${PROVIDER_API_KEY}")); } - #[test] - fn test_load_yaml_format() { - let config_yaml = r#" -llm: - providers: - - name: google - model: gemini-2.5-flash - api_key: test-key - timeout: 10 - - name: ollama - model: llama3.2 - parameters: - max_tokens: 100 - temperature: 0.3 -tts: - providers: - - name: macos - voice: Tingting - rate: 200 -"#; - - let mut temp_file = NamedTempFile::new().unwrap(); - temp_file.write_all(config_yaml.as_bytes()).unwrap(); - let mut path = temp_file.path().to_path_buf(); - - // Rename to .yaml extension - let yaml_path = path.with_extension("yaml"); - std::fs::rename(&path, &yaml_path).unwrap(); - path = yaml_path; - - let config = SumvoxConfig::load_yaml(path).unwrap(); - assert_eq!(config.llm.providers.len(), 2); - assert_eq!(config.llm.providers[0].name, "google"); - assert_eq!( - config.llm.providers[0].api_key, - Some("test-key".to_string()) - ); - assert_eq!(config.tts.providers[0].name, "macos"); - } - #[test] fn test_load_save_toml() { let temp_dir = tempfile::tempdir().unwrap(); @@ -1011,22 +834,6 @@ tts: ); } - #[test] - fn test_backup_creation() { - let temp_dir = tempfile::tempdir().unwrap(); - let path = temp_dir.path().join("config.yaml"); - std::fs::write(&path, "test content").unwrap(); - - let backup = SumvoxConfig::backup_config(&path).unwrap(); - assert!(backup.exists()); - assert!(backup - .file_name() - .unwrap() - .to_str() - .unwrap() - .starts_with("config.yaml.backup-")); - } - #[test] fn test_service_account_key_reads_file() { use std::io::Write; diff --git a/src/main.rs b/src/main.rs index 97a62fe..57f8f99 100644 --- a/src/main.rs +++ b/src/main.rs @@ -224,27 +224,14 @@ async fn handle_json(args: JsonArgs) -> Result<()> { // ============================================================================ async fn handle_init(args: InitArgs) -> Result<()> { - // Check for an existing config: the TOML we write, or a legacy YAML/JSON one let toml_path = SumvoxConfig::toml_config_path()?; - let yaml_path = SumvoxConfig::yaml_config_path()?; - let json_path = SumvoxConfig::config_path()?; - - if !args.force { - if let Some(existing_path) = [&toml_path, &yaml_path, &json_path] - .into_iter() - .find(|path| path.exists()) - { - eprintln!("Config file already exists at: {:?}", existing_path); - eprintln!(); - eprintln!("To reset to defaults, use --force:"); - eprintln!(" sumvox init --force"); - return Ok(()); - } - } - // Remove the old JSON config when --force rewrites it as TOML - if args.force && json_path.exists() { - std::fs::remove_file(&json_path).ok(); + if toml_path.exists() && !args.force { + eprintln!("Config file already exists at: {:?}", toml_path); + eprintln!(); + eprintln!("To reset to defaults, use --force:"); + eprintln!(" sumvox init --force"); + return Ok(()); } // Create default config with recommended settings diff --git a/tests/cli.rs b/tests/cli.rs index 7a688e5..48f7cd2 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -244,28 +244,20 @@ fn test_init_creates_config() { } #[test] -fn test_init_force() { +fn test_init_replaces_legacy_config() { let env = TestEnv::new(); - // Create a legacy config.yaml to trigger "already exists" check + // A legacy config.yaml no longer counts as an existing config: init writes the TOML let config_dir = env.home_path().join(".config/sumvox"); fs::create_dir_all(&config_dir).unwrap(); fs::write(config_dir.join("config.yaml"), "version: '1.0.0'").unwrap(); - // Without --force: should report existing config - env.cmd() - .arg("init") - .assert() - .success() - .stderr(predicate::str::contains("already exists")); - - // With --force: should overwrite and create config.toml - env.cmd().args(["init", "--force"]).assert().success(); + env.cmd().arg("init").assert().success(); let config_path = config_dir.join("config.toml"); assert!( config_path.exists(), - "config.toml should be created after init --force" + "config.toml should be created by init next to a legacy config.yaml" ); } @@ -298,6 +290,23 @@ fn test_init_does_not_overwrite_existing_toml() { ); } +#[test] +fn test_legacy_config_without_toml_is_rejected() { + for legacy in ["config.yaml", "config.yml", "config.json"] { + let env = TestEnv::new(); + let config_dir = env.home_path().join(".config/sumvox"); + fs::create_dir_all(&config_dir).unwrap(); + fs::write(config_dir.join(legacy), "{}").unwrap(); + + env.cmd() + .args(["say", "hi"]) + .assert() + .failure() + .stderr(predicate::str::contains(legacy)) + .stderr(predicate::str::contains("sumvox init")); + } +} + #[test] fn test_sum_no_llm_config() { let env = TestEnv::new(); From 4285398d10a43da6c91a094a89cc9203fa9abcb9 Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:04:22 +0800 Subject: [PATCH 02/13] refactor(tts): drop create_tts_from_config Auto mode built a provider only to discard it and then rebuilt the whole chain in speak_with_provider_fallback. Auto now goes straight to the fallback chain. The chain keeps the old contract: if no provider could even be tried it returns "No TTS provider available", a usable audio_file entry still counts as available (silent, not an error), and providers that were tried and failed degrade to silence. The "Tried:" list in that error now names providers as they report themselves (e.g. "espeak", not the configured "espeak_ng") and leaves out audio_file entries. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/pipeline.rs | 96 +++++++++++++++++++++-------------------- src/pipeline/tests.rs | 21 +++++++++ src/tts/mod.rs | 99 ------------------------------------------- tests/cli.rs | 51 ++++++++++++++++++++++ 4 files changed, 121 insertions(+), 146 deletions(-) diff --git a/src/pipeline.rs b/src/pipeline.rs index a0c3325..e8e48f8 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -9,8 +9,7 @@ use crate::llm::{GenerationRequest, LlmProvider}; use crate::notify_log; use crate::provider_factory::ProviderFactory; use crate::tts::{ - create_single_tts, create_tts_from_config, resolve_tts_provider, strip_leading_audio_tag, - TtsEngine, TtsProvider, + create_single_tts, resolve_tts_provider, strip_leading_audio_tag, TtsEngine, TtsProvider, }; /// TTS options @@ -266,8 +265,9 @@ pub async fn speak_text(config: &SumvoxConfig, tts_opts: &TtsOptions, text: &str tts_opts.volume, )?, EngineChoice::Engine(TtsEngine::Auto) => { - // Use config fallback chain - create_tts_from_config(&config.tts.providers)? + // Config fallback chain; the hook-level volume overrides each provider's own + return speak_with_provider_fallback(&config.tts.providers, text, tts_opts.volume) + .await; } // An explicitly selected engine overrides which configured provider to use; // all attributes come from that config entry, with only explicit CLI/hook @@ -348,36 +348,19 @@ pub async fn speak_text(config: &SumvoxConfig, tts_opts: &TtsOptions, text: &str return Ok(()); } - // Estimate and log cost for cloud providers - let cost = provider.estimate_cost(text.len()); - if cost > 0.0 { - tracing::info!("TTS cost estimate: ${:.6} for {} chars", cost, text.len()); - } - - // Speak with error handling and fallback for Auto mode - match choice { - EngineChoice::Engine(TtsEngine::Auto) => { - // For Auto mode, try all providers in config order - // Pass volume override so hook-level volume (stop_volume/notification_volume) is applied - speak_with_provider_fallback(&config.tts.providers, text, tts_opts.volume).await + let text = if provider.supports_audio_tags() { + text + } else { + strip_leading_audio_tag(text) + }; + match provider.speak(text).await { + Ok(_) => { + tracing::debug!("TTS playback completed"); + Ok(()) } - _ => { - // Single provider mode - just try once - let text = if provider.supports_audio_tags() { - text - } else { - strip_leading_audio_tag(text) - }; - match provider.speak(text).await { - Ok(_) => { - tracing::debug!("TTS playback completed"); - Ok(()) - } - Err(e) => { - tracing::warn!("TTS playback failed: {}. Notification will be silent.", e); - Ok(()) - } - } + Err(e) => { + tracing::warn!("TTS playback failed: {}. Notification will be silent.", e); + Ok(()) } } } @@ -416,23 +399,38 @@ async fn speak_with_provider_fallback( ) }); - speak_with_fallback(&mut candidates, text).await + let result = speak_with_fallback(&mut candidates, text).await; + + // A usable audio_file entry is never spoken through, but it still counts as an + // available TTS: such a chain stays silent instead of erroring. + if result.is_err() + && providers.iter().any(|c| { + c.name.parse::().ok() == Some(TtsEngine::AudioFile) + && create_single_tts(c).is_ok_and(|p| p.is_available()) + }) + { + tracing::warn!("No speech TTS providers available. Notification will be silent."); + return Ok(()); + } + result } /// Speak with the first provider that is available and succeeds. An `Err` entry is a -/// provider that could not be created. Always `Ok`: total failure is only logged. +/// provider that could not be created. Total failure of tried providers is only +/// logged; `Err` means no provider could be tried at all. async fn speak_with_fallback( providers: &mut dyn Iterator, text: &str, ) -> Result<()> { let mut last_error = None; + let mut skipped = Vec::new(); for provider in providers { let provider = match provider { Ok(p) => p, Err(e) => { tracing::debug!("Failed to create TTS provider {}", e); - last_error = Some(e); + skipped.push(e); continue; } }; @@ -443,7 +441,7 @@ async fn speak_with_fallback( "TTS provider {} not available, trying next", provider.name() ); - last_error = Some(format!("{}: not available", provider.name())); + skipped.push(format!("{}: not available", provider.name())); continue; } @@ -478,17 +476,21 @@ async fn speak_with_fallback( } } - // All providers failed - if let Some(err) = last_error { - tracing::warn!( - "All TTS providers failed. Last error: {}. Notification will be silent.", - err - ); - } else { - tracing::warn!("No TTS providers available. Notification will be silent."); + // A provider that was tried and failed degrades to silence; a chain where + // nothing could even be tried is a configuration error worth surfacing. + match last_error { + Some(err) => { + tracing::warn!( + "All TTS providers failed. Last error: {}. Notification will be silent.", + err + ); + Ok(()) + } + None => Err(VoiceError::Config(format!( + "No TTS provider available. Tried: {}", + skipped.join("; ") + ))), } - - Ok(()) } #[cfg(test)] diff --git a/src/pipeline/tests.rs b/src/pipeline/tests.rs index 0668b13..2e19685 100644 --- a/src/pipeline/tests.rs +++ b/src/pipeline/tests.rs @@ -201,6 +201,27 @@ async fn tts_unavailable_and_uncreatable_providers_are_skipped() { assert_eq!(*spoken.lock().unwrap(), ["c:hi"]); } +#[tokio::test] +async fn tts_nothing_available_is_an_error() { + let spoken = Calls::default(); + let err = run_tts( + vec![ + Err("a: bad config".to_string()), + tts("b", false, true, false, &spoken), + ], + "hi", + ) + .await + .expect_err("a chain with nothing to try must not succeed silently") + .to_string(); + assert!( + err.contains("No TTS provider available"), + "unexpected: {err}" + ); + assert!(err.contains("a: bad config") && err.contains("b: not available")); + assert!(spoken.lock().unwrap().is_empty()); +} + #[tokio::test] async fn tts_total_failure_is_silent_ok() { let spoken = Calls::default(); diff --git a/src/tts/mod.rs b/src/tts/mod.rs index 24af160..4fd70df 100644 --- a/src/tts/mod.rs +++ b/src/tts/mod.rs @@ -119,41 +119,6 @@ pub use openai::OpenAiTtsProvider; pub use piper::PiperProvider; pub use xai::XaiTtsProvider; -/// Create TTS provider from config array with automatic fallback -/// -/// Tries each provider in order until one is available. -/// Returns an error if no provider can be created. -pub fn create_tts_from_config(providers: &[TtsProviderConfig]) -> Result> { - let mut errors = Vec::new(); - - for config in providers { - match create_single_tts(config) { - Ok(provider) => { - if provider.is_available() { - tracing::info!( - "Using TTS provider: {} (voice: {})", - config.name, - config.voice.as_deref().unwrap_or("default") - ); - return Ok(provider); - } else { - tracing::debug!("TTS {} created but not available, trying next", config.name); - errors.push(format!("{}: not available", config.name)); - } - } - Err(e) => { - tracing::debug!("Failed to create TTS {}: {}", config.name, e); - errors.push(format!("{}: {}", config.name, e)); - } - } - } - - Err(VoiceError::Config(format!( - "No TTS provider available. Tried: {}", - errors.join("; ") - ))) -} - fn build_piper(config: &TtsProviderConfig, volume: u32) -> Result { // For piper, the voice IS a downloaded .onnx model file. `voice` // takes precedence because it is the field the CLI overlays, so @@ -745,60 +710,6 @@ mod tests { assert!(provider.is_available()); } - #[test] - #[cfg(target_os = "macos")] - fn test_create_tts_fallback() { - let _env = crate::test_support::env_guard(); - // Google TTS without API key should fallback to macOS - let providers = vec![ - TtsProviderConfig { - name: "google".to_string(), - model: Some("gemini-2.5-flash-preview-tts".to_string()), - voice: Some("Zephyr".to_string()), - api_key: None, // No API key - rate: None, - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, - }, - TtsProviderConfig { - name: "macos".to_string(), - model: None, - voice: Some("Tingting".to_string()), - api_key: None, - rate: Some(200), - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, - }, - ]; - - // Every env var the google provider's key lookup reads. Only removed, never - // set, anywhere in the suite, so concurrent tests cannot observe a flip. - for var in [ - "GOOGLE_CLOUD_PROJECT", - "GCP_PROJECT", - "GEMINI_API_KEY", - "GOOGLE_API_KEY", - ] { - std::env::remove_var(var); - } - - let result = create_tts_from_config(&providers); - assert!(result.is_ok()); - assert_eq!(result.unwrap().name(), "macos"); - } - #[test] fn test_resolve_tts_provider_uses_config_and_cli_override() { let providers = vec![TtsProviderConfig { @@ -840,14 +751,4 @@ mod tests { .expect("configured piper entry should resolve"); assert_eq!(provider.name(), "piper"); } - - #[test] - fn test_create_tts_empty_providers() { - let providers: Vec = vec![]; - - let result = create_tts_from_config(&providers); - assert!(result.is_err()); - let err = result.err().unwrap(); - assert!(err.to_string().contains("No TTS provider")); - } } diff --git a/tests/cli.rs b/tests/cli.rs index 48f7cd2..2e57046 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -652,6 +652,57 @@ fn test_sumvox_disable_short_circuits() { ); } +fn config_tts_only(tts: &str) -> String { + format!( + r#"[llm] +providers = [] +[llm.parameters] +max_tokens = 100 +temperature = 0.3 + +[tts] +{tts} + +[summarization] +turns = 1 +system_message = "Test" +prompt_template = "Summarize: {{context}}" +"# + ) +} + +#[test] +fn test_auto_chain_with_only_audio_file_usable_stays_silent() { + let env = TestEnv::new(); + let sound = env.home_path().join("sound.wav"); + fs::write(&sound, b"not real audio").unwrap(); + env.setup_with_config(&config_tts_only(&format!( + "[[tts.providers]]\nname = \"audio_file\"\npath = \"{}\"\n\n[[tts.providers]]\nname = \"elevenlabs\"\nvoice = \"v\"\nmodel = \"m\"", + sound.display() + ))); + + env.cmd() + .env_remove("ELEVENLABS_API_KEY") + .args(["say", "hi"]) + .assert() + .success(); +} + +#[test] +fn test_auto_chain_with_nothing_usable_fails() { + let env = TestEnv::new(); + env.setup_with_config(&config_tts_only( + "[[tts.providers]]\nname = \"elevenlabs\"\nvoice = \"v\"\nmodel = \"m\"", + )); + + env.cmd() + .env_remove("ELEVENLABS_API_KEY") + .args(["say", "hi"]) + .assert() + .failure() + .stderr(predicate::str::contains("No TTS provider available")); +} + #[test] fn test_muted_say_records_history_without_playing() { let env = TestEnv::new(); From 76b8b7fa744bdbc864973007feccdf1fc2f3b660 Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:05:05 +0800 Subject: [PATCH 03/13] refactor(tts): drop estimate_cost and the TTS cost log lines estimate_cost fed only an info log. Removes it from the TtsProvider trait and every impl, with the per-char price constants, the cost tests and the now-unused CloudTtsProvider::is_gemini. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/audio/file.rs | 6 ------ src/pipeline.rs | 6 ------ src/pipeline/tests.rs | 3 --- src/tts/cloud_tts.rs | 35 ----------------------------------- src/tts/elevenlabs.rs | 35 ----------------------------------- src/tts/espeak.rs | 6 ------ src/tts/google.rs | 8 -------- src/tts/macos.rs | 6 ------ src/tts/mod.rs | 4 ---- src/tts/openai.rs | 10 ---------- src/tts/piper.rs | 6 ------ src/tts/xai.rs | 8 -------- 12 files changed, 133 deletions(-) diff --git a/src/audio/file.rs b/src/audio/file.rs index ee035db..e7f7271 100644 --- a/src/audio/file.rs +++ b/src/audio/file.rs @@ -133,11 +133,6 @@ impl TtsProvider for AudioFileProvider { Ok(true) } - - fn estimate_cost(&self, _char_count: usize) -> f64 { - // Audio file playback is free - 0.0 - } } #[cfg(test)] @@ -156,7 +151,6 @@ mod tests { assert_eq!(provider.name(), "audio_file"); assert_eq!(provider.volume, 80); assert!(matches!(provider.mode, AudioFileMode::SingleFile(_))); - assert_eq!(provider.estimate_cost(100), 0.0); assert!(provider.is_available()); assert_eq!(provider.get_playback_file().unwrap(), temp_file.path()); } diff --git a/src/pipeline.rs b/src/pipeline.rs index e8e48f8..3545efb 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -447,12 +447,6 @@ async fn speak_with_fallback( tracing::info!("Using TTS provider: {}", provider.name()); - // Estimate and log cost for cloud providers - let cost = provider.estimate_cost(text.len()); - if cost > 0.0 { - tracing::info!("TTS cost estimate: ${:.6} for {} chars", cost, text.len()); - } - // Try to speak (strip audio tags for providers that would read them aloud) let provider_text = if provider.supports_audio_tags() { text diff --git a/src/pipeline/tests.rs b/src/pipeline/tests.rs index 2e19685..e034267 100644 --- a/src/pipeline/tests.rs +++ b/src/pipeline/tests.rs @@ -145,9 +145,6 @@ impl TtsProvider for FakeTts { Err(VoiceError::Config("tts boom".into())) } } - fn estimate_cost(&self, _char_count: usize) -> f64 { - 0.0 - } fn supports_audio_tags(&self) -> bool { self.tags } diff --git a/src/tts/cloud_tts.rs b/src/tts/cloud_tts.rs index 9bd423e..3028a94 100644 --- a/src/tts/cloud_tts.rs +++ b/src/tts/cloud_tts.rs @@ -12,14 +12,9 @@ use crate::error::{Result, VoiceError}; use crate::tts::cloud_tts_auth::CloudTtsAuth; const API_ENDPOINT: &str = "https://texttospeech.googleapis.com/v1/text:synthesize"; -const COST_PER_CHAR: f64 = 0.000004; // $4 per 1M chars (Standard voices) const MAX_TEXT_BYTES: usize = 5000; // Gemini-TTS caps input `text` at 4000 bytes (prompt is billed separately). const MAX_TEXT_BYTES_GEMINI: usize = 4000; -// Gemini-TTS is billed per audio token, not per character. Rough estimate: -// $10 / 1M audio tokens × 25 tokens/sec ÷ ~15 chars/sec ≈ 0.0000167 / char. -// Coarse approximation — real cost depends on synthesized audio duration. -const COST_PER_CHAR_GEMINI: f64 = 0.0000167; /// Google Cloud TTS provider pub struct CloudTtsProvider { @@ -96,13 +91,6 @@ impl CloudTtsProvider { } } - /// Whether this provider is configured for a Gemini-TTS model. - fn is_gemini(&self) -> bool { - self.model - .as_deref() - .is_some_and(|m| m.to_lowercase().contains("gemini")) - } - /// Byte cap for a single synthesis chunk (Gemini-TTS is stricter). fn max_chunk_bytes(&self) -> usize { if self.model.is_some() { @@ -254,17 +242,6 @@ impl TtsProvider for CloudTtsProvider { tracing::debug!("Voice playback completed"); Ok(true) } - - fn estimate_cost(&self, char_count: usize) -> f64 { - // Gemini-TTS bills per audio token; use a coarse per-char proxy. - // Traditional voices keep the exact $4/1M-char rate. - let rate = if self.is_gemini() { - COST_PER_CHAR_GEMINI - } else { - COST_PER_CHAR - }; - char_count as f64 * rate - } } #[cfg(test)] @@ -401,16 +378,4 @@ mod tests { assert_eq!(create_gemini_provider().max_chunk_bytes(), 4000); assert_eq!(create_test_provider().max_chunk_bytes(), 5000); } - - #[test] - fn test_gemini_cost_uses_token_estimate() { - let p = create_gemini_provider(); - let cost = p.estimate_cost(1_000_000); - // Coarse per-char proxy for token billing (~$16.7 / 1M chars). - assert!((cost - 16.7).abs() < 0.1, "unexpected gemini cost: {cost}"); - - // Non-Gemini voices keep the flat per-char price - let traditional = create_test_provider().estimate_cost(1_000_000); - assert!((traditional - 4.0).abs() < 0.01); - } } diff --git a/src/tts/elevenlabs.rs b/src/tts/elevenlabs.rs index 1149760..da95525 100644 --- a/src/tts/elevenlabs.rs +++ b/src/tts/elevenlabs.rs @@ -18,11 +18,6 @@ const DEFAULT_OUTPUT_FORMAT: &str = "mp3_44100_128"; /// Maximum characters per request (ElevenLabs limit) const MAX_TEXT_LENGTH: usize = 5_000; -/// Per-character cost for Flash v2.5 ($0.05 per 1K chars). -/// Multilingual models cost ~2x; this is a coarse estimate. -const COST_PER_CHAR_FLASH: f64 = 0.00005; -const COST_PER_CHAR_MULTILINGUAL: f64 = 0.0001; - pub struct ElevenLabsProvider { api_key: String, voice_id: String, @@ -209,15 +204,6 @@ impl TtsProvider for ElevenLabsProvider { tracing::debug!("Voice playback completed"); Ok(true) } - - fn estimate_cost(&self, char_count: usize) -> f64 { - let per_char = if self.model_id.contains("multilingual") { - COST_PER_CHAR_MULTILINGUAL - } else { - COST_PER_CHAR_FLASH - }; - char_count as f64 * per_char - } } fn truncate_to_limit(text: &str, limit: usize) -> &str { @@ -285,27 +271,6 @@ mod tests { assert!(!placeholder.is_available()); } - #[test] - fn test_cost_estimation_flash() { - // (model, expected cost for 1M chars): flash is half the multilingual price - for (model, expected) in [ - ("eleven_flash_v2_5", 50.0), - ("eleven_multilingual_v2", 100.0), - ] { - let provider = ElevenLabsProvider::new( - "test-key".to_string(), - "21m00Tcm4TlvDq8ikWAM".to_string(), - model.to_string(), - None, - None, - None, - 100, - ); - assert_eq!(provider.name(), "elevenlabs"); - assert!((provider.estimate_cost(1_000_000) - expected).abs() < 0.001); - } - } - #[tokio::test] async fn test_speak_empty_message() { let provider = ElevenLabsProvider::new( diff --git a/src/tts/espeak.rs b/src/tts/espeak.rs index fd9b407..0883957 100644 --- a/src/tts/espeak.rs +++ b/src/tts/espeak.rs @@ -209,11 +209,6 @@ impl TtsProvider for EspeakProvider { tracing::debug!("espeak-ng playback completed"); Ok(true) } - - fn estimate_cost(&self, _char_count: usize) -> f64 { - // Local engine: free. - 0.0 - } } #[cfg(test)] @@ -311,6 +306,5 @@ mod tests { !provider.is_available(), "a missing engine must be skipped by the chain, not error" ); - assert_eq!(provider.estimate_cost(1000), 0.0); } } diff --git a/src/tts/google.rs b/src/tts/google.rs index 8b82b66..120060f 100644 --- a/src/tts/google.rs +++ b/src/tts/google.rs @@ -12,9 +12,6 @@ use crate::error::{Result, VoiceError}; /// Gemini TTS API base URL const GEMINI_TTS_API_BASE: &str = "https://generativelanguage.googleapis.com/v1beta"; -/// Cost per character for Gemini TTS (estimated) -const COST_PER_CHAR: f64 = 0.000016; - /// Gemini TTS provider using Google AI Studio API pub struct GoogleTtsProvider { api_key: String, @@ -257,10 +254,6 @@ impl TtsProvider for GoogleTtsProvider { tracing::debug!("Voice playback completed"); Ok(true) } - - fn estimate_cost(&self, char_count: usize) -> f64 { - char_count as f64 * COST_PER_CHAR - } } #[cfg(test)] @@ -277,7 +270,6 @@ mod tests { ); assert!(!provider.is_available()); assert_eq!(provider.name(), "google"); - assert!(provider.estimate_cost(1000) > 0.0); } #[tokio::test] diff --git a/src/tts/macos.rs b/src/tts/macos.rs index 453cdec..e246b83 100644 --- a/src/tts/macos.rs +++ b/src/tts/macos.rs @@ -103,11 +103,6 @@ impl TtsProvider for MacOsTtsProvider { Ok(true) } - - fn estimate_cost(&self, _char_count: usize) -> f64 { - // macOS say is free - 0.0 - } } #[cfg(test)] @@ -120,7 +115,6 @@ mod tests { let result = provider.speak("").await.unwrap(); assert!(!result); assert!(!provider.speak(" ").await.unwrap()); - assert_eq!(provider.estimate_cost(100), 0.0); } // Exercises the full render-to-file + playback path at a low volume; fails diff --git a/src/tts/mod.rs b/src/tts/mod.rs index 4fd70df..361a1d9 100644 --- a/src/tts/mod.rs +++ b/src/tts/mod.rs @@ -30,10 +30,6 @@ pub trait TtsProvider: Send + Sync { /// Returns true if speech was initiated successfully async fn speak(&self, text: &str) -> Result; - /// Estimate cost per character (for cloud providers) - /// Returns 0.0 for local engines - fn estimate_cost(&self, char_count: usize) -> f64; - /// Whether this provider interprets `[tag]`-style audio/emotion tags /// (e.g. ElevenLabs eleven_v3). Providers that don't must have such /// tags stripped before speaking, or they get read aloud literally. diff --git a/src/tts/openai.rs b/src/tts/openai.rs index ee46a59..7f2cc3f 100644 --- a/src/tts/openai.rs +++ b/src/tts/openai.rs @@ -13,11 +13,6 @@ use crate::error::{Result, VoiceError}; /// OpenAI speech synthesis endpoint const OPENAI_TTS_API_URL: &str = "https://api.openai.com/v1/audio/speech"; -/// Cost per character for OpenAI TTS. -/// Derived from user-measured billing (2026-07): $0.009 for 4 notifications -/// (~120 chars total) => $0.000075/char. -const COST_PER_CHAR: f64 = 0.000075; - /// Maximum input length per request (OpenAI limit: 4096 chars) const MAX_TEXT_LENGTH: usize = 4_096; @@ -162,10 +157,6 @@ impl TtsProvider for OpenAiTtsProvider { tracing::debug!("Voice playback completed"); Ok(true) } - - fn estimate_cost(&self, char_count: usize) -> f64 { - char_count as f64 * COST_PER_CHAR - } } #[cfg(test)] @@ -211,7 +202,6 @@ mod tests { assert_eq!(provider(Some(5.0)).speed, Some(4.0)); assert_eq!(provider(Some(0.1)).speed, Some(0.25)); assert_eq!(provider(Some(1.5)).speed, Some(1.5)); - assert!(provider(None).estimate_cost(1000) > 0.0); assert!(!provider(None).supports_audio_tags()); } diff --git a/src/tts/piper.rs b/src/tts/piper.rs index 4c4272e..75453d4 100644 --- a/src/tts/piper.rs +++ b/src/tts/piper.rs @@ -237,11 +237,6 @@ impl TtsProvider for PiperProvider { tracing::debug!("piper playback completed"); Ok(true) } - - fn estimate_cost(&self, _char_count: usize) -> f64 { - // Local engine: free. - 0.0 - } } #[cfg(test)] @@ -343,6 +338,5 @@ mod tests { !provider.is_available(), "a missing engine must be skipped by the chain, not error" ); - assert_eq!(provider.estimate_cost(1000), 0.0); } } diff --git a/src/tts/xai.rs b/src/tts/xai.rs index 782dc2a..a044e3f 100644 --- a/src/tts/xai.rs +++ b/src/tts/xai.rs @@ -12,9 +12,6 @@ use crate::error::{Result, VoiceError}; /// xAI TTS API endpoint const XAI_TTS_API_URL: &str = "https://api.x.ai/v1/tts"; -/// Cost per character for xAI TTS ($15.00 / 1M characters) -const COST_PER_CHAR: f64 = 0.000015; - /// Maximum text length per request const MAX_TEXT_LENGTH: usize = 15_000; @@ -148,10 +145,6 @@ impl TtsProvider for XaiTtsProvider { tracing::debug!("Voice playback completed"); Ok(true) } - - fn estimate_cost(&self, char_count: usize) -> f64 { - char_count as f64 * COST_PER_CHAR - } } fn truncate_to_limit(text: &str, limit: usize) -> &str { @@ -179,7 +172,6 @@ mod tests { assert_eq!(provider.language, "auto"); assert_eq!(provider.volume, 100); assert!(provider.is_available()); - assert!(provider.estimate_cost(1000) > 0.0); let custom = XaiTtsProvider::new( "test-api-key".to_string(), From c4a7b8feb4aaa46d4c82054d022bd27f7410f25f Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:05:30 +0800 Subject: [PATCH 04/13] chore: remove unused VoiceError::Llm and the chrono serde and nix process features Co-Authored-By: Claude Opus 5.5 (1M context) --- Cargo.lock | 1 - Cargo.toml | 4 ++-- src/error.rs | 3 --- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cf6a344..9301462 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -185,7 +185,6 @@ dependencies = [ "iana-time-zone", "js-sys", "num-traits", - "serde", "wasm-bindgen", "windows-link", ] diff --git a/Cargo.toml b/Cargo.toml index 25d41f5..b077d58 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ reqwest = { version = "0.12", default-features = false, features = ["json", "rus serde = { version = "1", features = ["derive"] } serde_json = "1" toml = "0.8" -chrono = { version = "0.4", features = ["serde"] } +chrono = "0.4" thiserror = "1" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } @@ -26,7 +26,7 @@ async-trait = "0.1" shellexpand = "3" clap = { version = "4", features = ["derive"] } base64 = "0.22" -nix = { version = "0.29", features = ["fs", "process", "user"] } +nix = { version = "0.29", features = ["fs", "user"] } rand = "0.8" jsonwebtoken = "9" diff --git a/src/error.rs b/src/error.rs index 2e7ff95..1665f07 100644 --- a/src/error.rs +++ b/src/error.rs @@ -19,9 +19,6 @@ pub enum VoiceError { #[error("Voice engine error: {0}")] Voice(String), - #[error("LLM error: {0}")] - Llm(#[from] LlmError), - #[error("Queue error: {0}")] Queue(String), } From c486c7fc3b4dcafac23a75fbe249896dd768e253 Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:05:35 +0800 Subject: [PATCH 05/13] chore: remove unused Cross.toml, .cargo/config.toml and scripts/test_hooks.sh Cross.toml installed ALSA headers for rodio, which is gone; the other two were a comments-only file and an unreferenced script superseded by tests/e2e.rs. Co-Authored-By: Claude Opus 5.5 (1M context) --- .cargo/config.toml | 4 ---- Cross.toml | 16 -------------- scripts/test_hooks.sh | 50 ------------------------------------------- 3 files changed, 70 deletions(-) delete mode 100644 .cargo/config.toml delete mode 100644 Cross.toml delete mode 100755 scripts/test_hooks.sh diff --git a/.cargo/config.toml b/.cargo/config.toml deleted file mode 100644 index 1b3b52b..0000000 --- a/.cargo/config.toml +++ /dev/null @@ -1,4 +0,0 @@ -# Cross-compilation configuration -# Note: This file is intentionally minimal. The cross tool handles -# most configuration via Docker containers. macOS targets use native -# compilation without custom linkers. diff --git a/Cross.toml b/Cross.toml deleted file mode 100644 index 4fcafa7..0000000 --- a/Cross.toml +++ /dev/null @@ -1,16 +0,0 @@ -# Cross-compilation configuration for Linux targets -# Install ALSA development libraries needed by rodio - -[target.x86_64-unknown-linux-gnu] -pre-build = [ - "dpkg --add-architecture amd64", - "apt-get update", - "apt-get install -y libasound2-dev:amd64" -] - -[target.aarch64-unknown-linux-gnu] -pre-build = [ - "dpkg --add-architecture arm64", - "apt-get update", - "apt-get install -y libasound2-dev:arm64" -] diff --git a/scripts/test_hooks.sh b/scripts/test_hooks.sh deleted file mode 100755 index c8b74e5..0000000 --- a/scripts/test_hooks.sh +++ /dev/null @@ -1,50 +0,0 @@ -#!/bin/bash -# Test script for SumVox hooks - -BINARY="./target/release/sumvox" -TEST_DIR="/tmp/claude" - -# Colors for output -GREEN='\033[0;32m' -YELLOW='\033[1;33m' -NC='\033[0m' # No Color - -echo "=== Testing SumVox hooks ===" -echo - -# Setup -mkdir -p "$TEST_DIR" -echo '{"type":"assistant","message":{"role":"assistant","content":[{"type":"text","text":"Test completed successfully."}]}}' > "$TEST_DIR/test_transcript.jsonl" -touch "$TEST_DIR/empty.jsonl" - -# Test 1: permission_prompt (should speak) -echo -e "${GREEN}Test 1: permission_prompt${NC}" -echo '{"session_id":"test","transcript_path":"'$TEST_DIR'/empty.jsonl","permission_mode":"default","hook_event_name":"Notification","message":"Claude needs permission","notification_type":"permission_prompt"}' \ - | RUST_LOG=info "$BINARY" 2>&1 | grep -E "Notification type|Skipping|Processing" -echo - -# Test 2: idle_prompt (should speak) -echo -e "${GREEN}Test 2: idle_prompt${NC}" -echo '{"session_id":"test","transcript_path":"'$TEST_DIR'/empty.jsonl","permission_mode":"default","hook_event_name":"Notification","message":"Claude is waiting","notification_type":"idle_prompt"}' \ - | RUST_LOG=info "$BINARY" 2>&1 | grep -E "Notification type|Skipping|Processing" -echo - -# Test 3: elicitation_dialog (should speak) -echo -e "${GREEN}Test 3: elicitation_dialog${NC}" -echo '{"session_id":"test","transcript_path":"'$TEST_DIR'/empty.jsonl","permission_mode":"default","hook_event_name":"Notification","message":"Need more input","notification_type":"elicitation_dialog"}' \ - | RUST_LOG=info "$BINARY" 2>&1 | grep -E "Notification type|Skipping|Processing" -echo - -# Test 4: auth_success (should skip) -echo -e "${YELLOW}Test 4: auth_success (should skip)${NC}" -echo '{"session_id":"test","transcript_path":"'$TEST_DIR'/empty.jsonl","permission_mode":"default","hook_event_name":"Notification","message":"Auth OK","notification_type":"auth_success"}' \ - | RUST_LOG=debug "$BINARY" 2>&1 | grep -E "Notification type|Skipping|Processing" -echo - -# Test 5: Stop hook (should read transcript) -echo -e "${GREEN}Test 5: Stop hook${NC}" -echo '{"session_id":"test","transcript_path":"'$TEST_DIR'/test_transcript.jsonl","permission_mode":"default","hook_event_name":"Stop"}' \ - | RUST_LOG=info "$BINARY" 2>&1 | grep -E "Processing|Extracted|Generated summary" -echo - -echo "=== All tests completed ===" From b9050d179ed9f6adc7415727387e6d8640ba6dce Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:06:40 +0800 Subject: [PATCH 06/13] refactor(config): derive Default for TtsProviderConfig Replaces the thirteen-field struct literals with ..Default::default(). Co-Authored-By: Claude Opus 5.5 (1M context) --- src/config.rs | 59 +++++----------------------------------------- src/main.rs | 23 ++---------------- src/tts/mod.rs | 64 +++++--------------------------------------------- 3 files changed, 14 insertions(+), 132 deletions(-) diff --git a/src/config.rs b/src/config.rs index e967a4c..d07e42c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -232,7 +232,7 @@ impl Default for LlmConfig { // ============================================================================ /// Individual TTS provider configuration -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, Default, Deserialize, Serialize)] pub struct TtsProviderConfig { /// Provider name: google, macos pub name: String, @@ -389,31 +389,12 @@ impl Default for TtsConfig { name: "google".to_string(), model: Some("gemini-2.5-flash-preview-tts".to_string()), voice: Some("Zephyr".to_string()), - api_key: None, - rate: None, - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() }, TtsProviderConfig { name: "macos".to_string(), - model: None, - voice: None, - api_key: None, rate: Some(200), - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() }, ], } @@ -690,18 +671,8 @@ mod tests { fn openai_tts_provider(api_key: Option) -> TtsProviderConfig { TtsProviderConfig { name: "openai".to_string(), - model: None, - voice: None, api_key, - rate: None, - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() } } @@ -733,18 +704,10 @@ mod tests { // The same cast applies to the optional per-provider knobs. let provider = TtsProviderConfig { name: "elevenlabs".to_string(), - model: None, - voice: None, - api_key: None, - rate: None, - volume: None, - path: None, - service_account_key: None, - language_code: None, speed: Some(0.9), stability: Some(0.4), style: Some(0.2), - style_prompt: None, + ..Default::default() }; let toml_str = toml::to_string_pretty(&provider).unwrap(); for expected in ["speed = 0.9\n", "stability = 0.4\n", "style = 0.2\n"] { @@ -846,18 +809,8 @@ mod tests { let config = TtsProviderConfig { name: "cloud_tts".to_string(), - model: None, - voice: None, - api_key: None, - rate: None, - volume: None, - path: None, service_account_key: Some(temp_file.path().to_string_lossy().to_string()), - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() }; let content = config.get_service_account_key(); diff --git a/src/main.rs b/src/main.rs index 57f8f99..5e8f914 100644 --- a/src/main.rs +++ b/src/main.rs @@ -249,33 +249,14 @@ async fn handle_init(args: InitArgs) -> Result<()> { config.tts.providers = vec![ TtsProviderConfig { name: "macos".to_string(), - model: None, - voice: None, // Use system default voice - api_key: None, rate: Some(200), - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() }, TtsProviderConfig { name: "google".to_string(), model: Some("gemini-2.5-flash-preview-tts".to_string()), voice: Some("Aoede".to_string()), - api_key: None, - rate: None, - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() }, ]; diff --git a/src/tts/mod.rs b/src/tts/mod.rs index 361a1d9..b3a27cd 100644 --- a/src/tts/mod.rs +++ b/src/tts/mod.rs @@ -437,16 +437,8 @@ mod tests { name: "gemini_tts".to_string(), model: Some("gemini-2.5-flash-tts".to_string()), voice: Some("Kore".to_string()), - api_key: None, - rate: None, - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, style_prompt: Some("Say it warmly.".to_string()), + ..Default::default() }; let err = match create_single_tts(&config) { Ok(_) => panic!("expected error without service account key"), @@ -466,21 +458,11 @@ mod tests { // must still pick the gemini_tts entry, not the first alias match. let base = TtsProviderConfig { name: "cloud_tts".to_string(), - model: None, // No voice: selecting this entry fails with "voice is required", // which discriminates it from the gemini_tts entry below. - voice: None, - api_key: None, - rate: None, - volume: None, - path: None, // /dev/null reads as empty content, passing the sa-key lookup. service_account_key: Some("/dev/null".to_string()), - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() }; let gemini = TtsProviderConfig { name: "gemini_tts".to_string(), @@ -520,18 +502,9 @@ mod tests { fn espeak_config(name: &str, voice: Option<&str>, rate: Option) -> TtsProviderConfig { TtsProviderConfig { name: name.to_string(), - model: None, voice: voice.map(str::to_string), - path: None, - api_key: None, rate, - volume: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() } } @@ -556,15 +529,7 @@ mod tests { model: model.map(str::to_string), voice: voice.map(str::to_string), path: path.map(str::to_string), - api_key: None, - rate: None, - volume: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() } } @@ -630,15 +595,7 @@ mod tests { model: model.map(str::to_string), voice: voice.map(str::to_string), api_key: Some("test-api-key".to_string()), - rate: None, - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() } } @@ -710,18 +667,9 @@ mod tests { fn test_resolve_tts_provider_uses_config_and_cli_override() { let providers = vec![TtsProviderConfig { name: "macos".to_string(), - model: None, voice: Some("Meijia".to_string()), - api_key: None, rate: Some(200), - volume: None, - path: None, - service_account_key: None, - language_code: None, - speed: None, - stability: None, - style: None, - style_prompt: None, + ..Default::default() }]; // CLI voice override wins over config voice; engine sourced from config. From 8cd0f17cb3327084253aa92a4190eb4ca6e38343 Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:07:00 +0800 Subject: [PATCH 07/13] refactor(config): share one API-key resolver and placeholder check The six get_*_api_key bodies collapse into resolve_key, and the repeated `${` placeholder test in config and five providers becomes is_usable_key. A set-but-empty GEMINI_API_KEY now falls through to GOOGLE_API_KEY. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/config.rs | 74 +++++++++++++++---------------------------- src/llm/anthropic.rs | 2 +- src/llm/gemini.rs | 2 +- src/llm/openai.rs | 2 +- src/tts/elevenlabs.rs | 2 +- src/tts/openai.rs | 2 +- 6 files changed, 31 insertions(+), 53 deletions(-) diff --git a/src/config.rs b/src/config.rs index d07e42c..47b4e99 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,6 +6,23 @@ use std::path::PathBuf; use crate::error::{Result, VoiceError}; +/// A key is usable when it is non-empty and not an unexpanded `${VAR}` placeholder. +pub fn is_usable_key(key: &str) -> bool { + !key.is_empty() && !key.starts_with("${") +} + +/// The configured key if usable, else the first non-empty value among `env_vars`. +fn resolve_key(configured: Option<&str>, env_vars: &[&str]) -> Option { + configured + .filter(|key| is_usable_key(key)) + .map(str::to_string) + .or_else(|| { + env_vars + .iter() + .find_map(|var| std::env::var(var).ok().filter(|k| !k.is_empty())) + }) +} + /// Default timeout in seconds for LLM requests fn default_timeout() -> u64 { 10 @@ -23,7 +40,7 @@ where { use serde::Serialize; match key { - Some(k) if !k.is_empty() && !k.starts_with("${") => k.serialize(serializer), + Some(k) if is_usable_key(k) => k.serialize(serializer), _ => "${PROVIDER_API_KEY}".serialize(serializer), } } @@ -126,16 +143,7 @@ pub fn effective_disable_thinking(provider: &LlmProviderConfig, params: &LlmPara impl LlmProviderConfig { /// Get API key from config or environment variable pub fn get_api_key(&self) -> Option { - // Config value takes priority - if let Some(ref key) = self.api_key { - if !key.is_empty() && !key.starts_with("${") { - return Some(key.clone()); - } - } - - // Try environment variable - let env_var = Self::env_var_name(&self.name); - std::env::var(env_var).ok().filter(|k| !k.is_empty()) + resolve_key(self.api_key.as_deref(), &[Self::env_var_name(&self.name)]) } /// Get environment variable name for provider @@ -310,55 +318,25 @@ pub struct TtsProviderConfig { impl TtsProviderConfig { /// Get ElevenLabs API key from config or environment pub fn get_elevenlabs_api_key(&self) -> Option { - if let Some(ref key) = self.api_key { - if !key.is_empty() && !key.starts_with("${") { - return Some(key.clone()); - } - } - - std::env::var("ELEVENLABS_API_KEY") - .ok() - .filter(|k| !k.is_empty()) + resolve_key(self.api_key.as_deref(), &["ELEVENLABS_API_KEY"]) } /// Get Gemini API key from config or environment pub fn get_api_key(&self) -> Option { - // Config value takes priority - if let Some(ref key) = self.api_key { - if !key.is_empty() && !key.starts_with("${") { - return Some(key.clone()); - } - } - - // Try environment variables - std::env::var("GEMINI_API_KEY") - .ok() - .or_else(|| std::env::var("GOOGLE_API_KEY").ok()) - .filter(|k| !k.is_empty()) + resolve_key( + self.api_key.as_deref(), + &["GEMINI_API_KEY", "GOOGLE_API_KEY"], + ) } /// Get xAI API key from config or environment pub fn get_xai_api_key(&self) -> Option { - if let Some(ref key) = self.api_key { - if !key.is_empty() && !key.starts_with("${") { - return Some(key.clone()); - } - } - - std::env::var("XAI_API_KEY").ok().filter(|k| !k.is_empty()) + resolve_key(self.api_key.as_deref(), &["XAI_API_KEY"]) } /// Get OpenAI API key from config or environment pub fn get_openai_api_key(&self) -> Option { - if let Some(ref key) = self.api_key { - if !key.is_empty() && !key.starts_with("${") { - return Some(key.clone()); - } - } - - std::env::var("OPENAI_API_KEY") - .ok() - .filter(|k| !k.is_empty()) + resolve_key(self.api_key.as_deref(), &["OPENAI_API_KEY"]) } /// Get service account key file content diff --git a/src/llm/anthropic.rs b/src/llm/anthropic.rs index 888c27d..b2022de 100644 --- a/src/llm/anthropic.rs +++ b/src/llm/anthropic.rs @@ -90,7 +90,7 @@ impl LlmProvider for AnthropicProvider { } fn is_available(&self) -> bool { - !self.api_key.is_empty() && !self.api_key.starts_with("${") + crate::config::is_usable_key(&self.api_key) } async fn generate(&self, request: &GenerationRequest) -> LlmResult { diff --git a/src/llm/gemini.rs b/src/llm/gemini.rs index 16a936b..8f17807 100644 --- a/src/llm/gemini.rs +++ b/src/llm/gemini.rs @@ -131,7 +131,7 @@ impl LlmProvider for GeminiProvider { } fn is_available(&self) -> bool { - !self.api_key.is_empty() && !self.api_key.starts_with("${") + crate::config::is_usable_key(&self.api_key) } async fn generate(&self, request: &GenerationRequest) -> LlmResult { diff --git a/src/llm/openai.rs b/src/llm/openai.rs index e412752..48150a9 100644 --- a/src/llm/openai.rs +++ b/src/llm/openai.rs @@ -164,7 +164,7 @@ impl LlmProvider for OpenAIProvider { } fn is_available(&self) -> bool { - !self.api_key.is_empty() && !self.api_key.starts_with("${") + crate::config::is_usable_key(&self.api_key) } async fn generate(&self, request: &GenerationRequest) -> LlmResult { diff --git a/src/tts/elevenlabs.rs b/src/tts/elevenlabs.rs index da95525..30b2913 100644 --- a/src/tts/elevenlabs.rs +++ b/src/tts/elevenlabs.rs @@ -119,7 +119,7 @@ impl TtsProvider for ElevenLabsProvider { } fn is_available(&self) -> bool { - !self.api_key.is_empty() && !self.api_key.starts_with("${") + crate::config::is_usable_key(&self.api_key) } fn supports_audio_tags(&self) -> bool { diff --git a/src/tts/openai.rs b/src/tts/openai.rs index 7f2cc3f..739cdcd 100644 --- a/src/tts/openai.rs +++ b/src/tts/openai.rs @@ -86,7 +86,7 @@ impl TtsProvider for OpenAiTtsProvider { } fn is_available(&self) -> bool { - !self.api_key.is_empty() && !self.api_key.starts_with("${") + crate::config::is_usable_key(&self.api_key) } async fn speak(&self, text: &str) -> Result { From e01fe03aad6417a846d898fb726e0755f9e45d2c Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:08:08 +0800 Subject: [PATCH 08/13] refactor(tts): keep the engine alias table in one place TtsEngine::NAMES now drives from_str, the create_single_tts factory, the pipeline's engine lookup and the audio_file check, replacing four copies and nine near-identical dispatch arms. A test pins the full set of accepted names. `--tts gemini` now parses as the google engine, as the factory already accepted it. The spelling the user typed is tried first, so "provider not found" errors name it (e.g. "say", not "macos"). Co-Authored-By: Claude Opus 5.5 (1M context) --- src/pipeline.rs | 94 +++++++----------------------- src/tts/mod.rs | 148 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 134 insertions(+), 108 deletions(-) diff --git a/src/pipeline.rs b/src/pipeline.rs index 3545efb..0f04eaf 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -272,75 +272,26 @@ pub async fn speak_text(config: &SumvoxConfig, tts_opts: &TtsOptions, text: &str // An explicitly selected engine overrides which configured provider to use; // all attributes come from that config entry, with only explicit CLI/hook // voice/volume layered on top. Nothing is hardcoded. - EngineChoice::Engine(TtsEngine::MacOS) => resolve_tts_provider( - &config.tts.providers, - &["macos", "say"], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, - EngineChoice::Engine(TtsEngine::Google) => resolve_tts_provider( - &config.tts.providers, - &["google", "google_tts", "gcloud", "gemini"], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, - EngineChoice::Engine(TtsEngine::CloudTts) => resolve_tts_provider( - &config.tts.providers, - &[ - engine_name.as_str(), - "cloud_tts", - "gcp_tts", - "google_cloud", - "gemini_tts", - ], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, - EngineChoice::Engine(TtsEngine::AudioFile) => resolve_tts_provider( - &config.tts.providers, - &["audio_file", "audio", "file"], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, - EngineChoice::Engine(TtsEngine::Xai) => resolve_tts_provider( - &config.tts.providers, - &["xai", "xai_tts", "grok"], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, - EngineChoice::Engine(TtsEngine::ElevenLabs) => resolve_tts_provider( - &config.tts.providers, - &["elevenlabs", "eleven_labs", "11labs"], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, - EngineChoice::Engine(TtsEngine::OpenAi) => resolve_tts_provider( - &config.tts.providers, - &["openai", "openai_tts"], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, - EngineChoice::Engine(TtsEngine::Espeak) => resolve_tts_provider( - &config.tts.providers, - &["espeak", "espeak_ng", "espeak-ng"], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, - EngineChoice::Engine(TtsEngine::Piper) => resolve_tts_provider( - &config.tts.providers, - &["piper", "piper_tts"], - tts_opts.voice.as_deref(), - tts_opts.rate, - tts_opts.volume, - )?, + EngineChoice::Engine(engine) => { + // The spelling the user typed goes first so it wins over other aliases + // (cloud_tts vs gemini_tts) when the config holds both. + let aliases: Vec<&str> = std::iter::once(engine_name.as_str()) + .chain( + engine + .aliases() + .iter() + .copied() + .filter(|alias| *alias != engine_name), + ) + .collect(); + resolve_tts_provider( + &config.tts.providers, + &aliases, + tts_opts.voice.as_deref(), + tts_opts.rate, + tts_opts.volume, + )? + } }; if !provider.is_available() { @@ -377,10 +328,7 @@ async fn speak_with_provider_fallback( let mut candidates = providers.iter().filter_map(|provider_config| { // Skip audio_file providers - they play sound effects, // not speech synthesis, and cannot render arbitrary text. - if matches!( - provider_config.name.to_lowercase().as_str(), - "audio_file" | "audio" | "file" - ) { + if provider_config.name.parse::().ok() == Some(TtsEngine::AudioFile) { tracing::debug!( "Skipping audio_file provider in fallback chain (not a speech synthesizer)" ); diff --git a/src/tts/mod.rs b/src/tts/mod.rs index b3a27cd..7f89ab9 100644 --- a/src/tts/mod.rs +++ b/src/tts/mod.rs @@ -68,40 +68,58 @@ pub enum TtsEngine { Auto, } +impl TtsEngine { + /// Every engine except Auto, with all spellings it is selected by. The one + /// place that knows which names mean which engine; the first is canonical. + const NAMES: &'static [(TtsEngine, &'static [&'static str])] = &[ + (TtsEngine::MacOS, &["macos", "say"]), + ( + TtsEngine::Google, + &["google", "google_tts", "gcloud", "gemini"], + ), + ( + TtsEngine::CloudTts, + &["cloud_tts", "gcp_tts", "google_cloud", "gemini_tts"], + ), + (TtsEngine::Xai, &["xai", "xai_tts", "grok"]), + ( + TtsEngine::ElevenLabs, + &["elevenlabs", "eleven_labs", "11labs"], + ), + (TtsEngine::OpenAi, &["openai", "openai_tts"]), + (TtsEngine::AudioFile, &["audio_file", "audio", "file"]), + (TtsEngine::Espeak, &["espeak", "espeak_ng", "espeak-ng"]), + (TtsEngine::Piper, &["piper", "piper_tts"]), + ]; + + /// All spellings of this engine, canonical first; empty for Auto. + pub fn aliases(self) -> &'static [&'static str] { + Self::NAMES + .iter() + .find(|(engine, _)| *engine == self) + .map_or(&[], |(_, names)| names) + } +} + impl FromStr for TtsEngine { type Err = VoiceError; fn from_str(s: &str) -> std::result::Result { - match s.to_lowercase().as_str() { - "macos" | "say" => Ok(TtsEngine::MacOS), - "google" | "google_tts" | "gcloud" => Ok(TtsEngine::Google), - "cloud_tts" | "gcp_tts" | "google_cloud" | "gemini_tts" => Ok(TtsEngine::CloudTts), - "xai" | "xai_tts" | "grok" => Ok(TtsEngine::Xai), - "elevenlabs" | "eleven_labs" | "11labs" => Ok(TtsEngine::ElevenLabs), - "openai" | "openai_tts" => Ok(TtsEngine::OpenAi), - "audio_file" | "audio" | "file" => Ok(TtsEngine::AudioFile), - "espeak" | "espeak_ng" | "espeak-ng" => Ok(TtsEngine::Espeak), - "piper" | "piper_tts" => Ok(TtsEngine::Piper), - "auto" => Ok(TtsEngine::Auto), - _ => Err(VoiceError::Config(format!("Unknown TTS engine: {}", s))), + let name = s.to_lowercase(); + if name == "auto" { + return Ok(TtsEngine::Auto); } + Self::NAMES + .iter() + .find(|(_, names)| names.contains(&name.as_str())) + .map(|(engine, _)| *engine) + .ok_or_else(|| VoiceError::Config(format!("Unknown TTS engine: {}", s))) } } impl std::fmt::Display for TtsEngine { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - TtsEngine::MacOS => write!(f, "macos"), - TtsEngine::Google => write!(f, "google"), - TtsEngine::CloudTts => write!(f, "cloud_tts"), - TtsEngine::Xai => write!(f, "xai"), - TtsEngine::ElevenLabs => write!(f, "elevenlabs"), - TtsEngine::OpenAi => write!(f, "openai"), - TtsEngine::AudioFile => write!(f, "audio_file"), - TtsEngine::Espeak => write!(f, "espeak"), - TtsEngine::Piper => write!(f, "piper"), - TtsEngine::Auto => write!(f, "auto"), - } + write!(f, "{}", self.aliases().first().copied().unwrap_or("auto")) } } @@ -138,13 +156,14 @@ fn build_piper(config: &TtsProviderConfig, volume: u32) -> Result pub fn create_single_tts(config: &TtsProviderConfig) -> Result> { let volume = config.volume.unwrap_or(100); - match config.name.to_lowercase().as_str() { - "macos" | "say" => { + let engine = config.name.parse::().ok(); + match engine { + Some(TtsEngine::MacOS) => { let voice = config.voice.clone(); let rate = config.rate.unwrap_or(200); Ok(Box::new(MacOsTtsProvider::new(voice, rate, volume))) } - "google" | "google_tts" | "gcloud" | "gemini" => { + Some(TtsEngine::Google) => { let api_key = config.get_api_key().ok_or_else(|| { VoiceError::Config( "Gemini API key not found. Set in config or env var GEMINI_API_KEY".into(), @@ -168,7 +187,7 @@ pub fn create_single_tts(config: &TtsProviderConfig) -> Result { + Some(TtsEngine::CloudTts) => { let sa_json = config.get_service_account_key().ok_or_else(|| { VoiceError::Config("Cloud TTS requires service_account_key".into()) })?; @@ -193,7 +212,7 @@ pub fn create_single_tts(config: &TtsProviderConfig) -> Result { + Some(TtsEngine::Xai) => { let api_key = config.get_xai_api_key().ok_or_else(|| { VoiceError::Config( "xAI API key not found. Set in config or env var XAI_API_KEY".into(), @@ -211,7 +230,7 @@ pub fn create_single_tts(config: &TtsProviderConfig) -> Result { + Some(TtsEngine::ElevenLabs) => { let api_key = config.get_elevenlabs_api_key().ok_or_else(|| { VoiceError::Config( "ElevenLabs API key not found. Set in config or env var ELEVENLABS_API_KEY" @@ -238,7 +257,7 @@ pub fn create_single_tts(config: &TtsProviderConfig) -> Result { + Some(TtsEngine::OpenAi) => { let api_key = config.get_openai_api_key().ok_or_else(|| { VoiceError::Config( "OpenAI API key not found. Set in config or env var OPENAI_API_KEY".into(), @@ -268,14 +287,14 @@ pub fn create_single_tts(config: &TtsProviderConfig) -> Result { + Some(TtsEngine::Espeak) => { // No required field: espeak-ng ships its own default voice. let voice = config.voice.clone(); let rate = config.rate.unwrap_or(espeak::DEFAULT_RATE); Ok(Box::new(EspeakProvider::new(voice, rate, volume))) } - "piper" | "piper_tts" => Ok(Box::new(build_piper(config, volume)?)), - "audio_file" | "audio" | "file" => { + Some(TtsEngine::Piper) => Ok(Box::new(build_piper(config, volume)?)), + Some(TtsEngine::AudioFile) => { let path_str = config.path.as_ref().ok_or_else(|| { VoiceError::Config( "Audio file provider requires 'path' field. Set to a file or directory path." @@ -288,7 +307,7 @@ pub fn create_single_tts(config: &TtsProviderConfig) -> Result Err(VoiceError::Config(format!( + Some(TtsEngine::Auto) | None => Err(VoiceError::Config(format!( "Unknown TTS provider: {}", config.name ))), @@ -427,6 +446,65 @@ mod tests { } } + /// Every spelling any engine-name path accepted before the alias table was unified + const ACCEPTED_NAMES: &[&str] = &[ + "macos", + "say", + "google", + "google_tts", + "gcloud", + "gemini", + "cloud_tts", + "gcp_tts", + "google_cloud", + "gemini_tts", + "xai", + "xai_tts", + "grok", + "elevenlabs", + "eleven_labs", + "11labs", + "openai", + "openai_tts", + "audio_file", + "audio", + "file", + "espeak", + "espeak_ng", + "espeak-ng", + "piper", + "piper_tts", + "auto", + ]; + + fn is_unknown_provider_error(name: &str) -> bool { + let config = TtsProviderConfig { + name: name.to_string(), + ..Default::default() + }; + create_single_tts(&config) + .err() + .is_some_and(|e| e.to_string().contains("Unknown TTS provider")) + } + + #[test] + fn test_accepted_engine_names_are_pinned() { + let _env = crate::test_support::env_guard(); + for name in ACCEPTED_NAMES { + let parsed = name.parse::().is_ok(); + let built = !is_unknown_provider_error(name); + assert!(parsed, "{name} no longer parses as an engine"); + assert!( + *name == "auto" || built, + "{name} is no longer built by the factory" + ); + } + for name in ["unknown", "espeakng", "", "macos2"] { + assert!(name.parse::().is_err(), "{name}"); + assert!(is_unknown_provider_error(name), "{name}"); + } + } + #[test] fn test_factory_recognizes_gemini_tts_alias() { let _env = crate::test_support::env_guard(); From 2809b98f95beb7570a37e1042d93480599420abe Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:08:52 +0800 Subject: [PATCH 09/13] refactor: build every provider's reqwest client through one helper http::client applies no_proxy and the timeout at all nine sites that each hand-built a client. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/http.rs | 21 +++++++++++++++++++++ src/llm/anthropic.rs | 6 +----- src/llm/gemini.rs | 6 +----- src/llm/ollama.rs | 6 +----- src/llm/openai.rs | 6 +----- src/main.rs | 1 + src/tts/cloud_tts.rs | 7 +------ src/tts/cloud_tts_auth.rs | 7 +------ src/tts/elevenlabs.rs | 7 +------ src/tts/google.rs | 9 +-------- src/tts/openai.rs | 7 +------ src/tts/xai.rs | 7 +------ 12 files changed, 32 insertions(+), 58 deletions(-) create mode 100644 src/http.rs diff --git a/src/http.rs b/src/http.rs new file mode 100644 index 0000000..8132841 --- /dev/null +++ b/src/http.rs @@ -0,0 +1,21 @@ +// Shared HTTP client construction for every provider + +use std::time::Duration; + +use reqwest::Client; + +use crate::error::{Result, VoiceError}; + +const TTS_TIMEOUT: Duration = Duration::from_secs(30); + +/// A client with system proxy detection off: on macOS it can crash inside +/// CoreFoundation, so every provider must build its client through here. +pub fn client(timeout: Duration) -> reqwest::Result { + Client::builder().no_proxy().timeout(timeout).build() +} + +/// Client for TTS providers, whose synthesis calls are slower than LLM requests. +pub fn tts_client() -> Result { + client(TTS_TIMEOUT) + .map_err(|e| VoiceError::Voice(format!("Failed to create HTTP client: {}", e))) +} diff --git a/src/llm/anthropic.rs b/src/llm/anthropic.rs index b2022de..e194989 100644 --- a/src/llm/anthropic.rs +++ b/src/llm/anthropic.rs @@ -75,11 +75,7 @@ impl AnthropicProvider { } fn client(&self) -> Client { - Client::builder() - .no_proxy() // Disable system proxy detection to avoid CoreFoundation crash - .timeout(self.timeout) - .build() - .unwrap_or_else(|_| Client::new()) + crate::http::client(self.timeout).unwrap_or_else(|_| Client::new()) } } diff --git a/src/llm/gemini.rs b/src/llm/gemini.rs index 8f17807..71a11f1 100644 --- a/src/llm/gemini.rs +++ b/src/llm/gemini.rs @@ -107,11 +107,7 @@ impl GeminiProvider { } fn client(&self) -> Client { - Client::builder() - .no_proxy() // Disable system proxy detection to avoid CoreFoundation crash - .timeout(self.timeout) - .build() - .unwrap_or_else(|_| Client::new()) + crate::http::client(self.timeout).unwrap_or_else(|_| Client::new()) } fn extract_model_name(&self) -> &str { diff --git a/src/llm/ollama.rs b/src/llm/ollama.rs index 6e9b4b6..1569c6e 100644 --- a/src/llm/ollama.rs +++ b/src/llm/ollama.rs @@ -54,11 +54,7 @@ impl OllamaProvider { } fn client(&self) -> Client { - Client::builder() - .no_proxy() // Disable system proxy detection to avoid CoreFoundation crash - .timeout(self.timeout) - .build() - .unwrap_or_else(|_| Client::new()) + crate::http::client(self.timeout).unwrap_or_else(|_| Client::new()) } fn extract_model_name(&self) -> &str { diff --git a/src/llm/openai.rs b/src/llm/openai.rs index 48150a9..273c7d6 100644 --- a/src/llm/openai.rs +++ b/src/llm/openai.rs @@ -87,11 +87,7 @@ impl OpenAIProvider { } fn client(&self) -> Client { - Client::builder() - .no_proxy() // Disable system proxy detection to avoid CoreFoundation crash - .timeout(self.timeout) - .build() - .unwrap_or_else(|_| Client::new()) + crate::http::client(self.timeout).unwrap_or_else(|_| Client::new()) } fn extract_model_name(&self) -> &str { diff --git a/src/main.rs b/src/main.rs index 5e8f914..41ef9a4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,6 +6,7 @@ mod cli; mod config; mod error; mod hooks; +mod http; mod llm; mod notify_log; mod pipeline; diff --git a/src/tts/cloud_tts.rs b/src/tts/cloud_tts.rs index 3028a94..4e75724 100644 --- a/src/tts/cloud_tts.rs +++ b/src/tts/cloud_tts.rs @@ -5,7 +5,6 @@ use async_trait::async_trait; use base64::Engine; use reqwest::Client; use serde::{Deserialize, Serialize}; -use std::time::Duration; use super::TtsProvider; use crate::error::{Result, VoiceError}; @@ -102,11 +101,7 @@ impl CloudTtsProvider { /// Create HTTP client fn create_client() -> Result { - Client::builder() - .no_proxy() - .timeout(Duration::from_secs(30)) - .build() - .map_err(|e| VoiceError::Voice(format!("Failed to create HTTP client: {}", e))) + crate::http::tts_client() } /// Split text into chunks at sentence boundaries, capped at `max_bytes`. diff --git a/src/tts/cloud_tts_auth.rs b/src/tts/cloud_tts_auth.rs index 0dd03f0..e0a4a14 100644 --- a/src/tts/cloud_tts_auth.rs +++ b/src/tts/cloud_tts_auth.rs @@ -2,7 +2,6 @@ // Generates JWT and exchanges for access token use jsonwebtoken::{encode, Algorithm, EncodingKey, Header}; -use reqwest::Client; use serde::{Deserialize, Serialize}; use std::sync::RwLock; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -100,11 +99,7 @@ impl CloudTtsAuth { // Exchange JWT for access token let token_uri = sa.token_uri.as_deref().unwrap_or(TOKEN_URI); - let client = Client::builder() - .no_proxy() // Disable system proxy detection to avoid CoreFoundation crash - .timeout(Duration::from_secs(30)) - .build() - .map_err(|e| VoiceError::Voice(format!("Failed to create HTTP client: {}", e)))?; + let client = crate::http::tts_client()?; let params = [ ("grant_type", "urn:ietf:params:oauth:grant-type:jwt-bearer"), diff --git a/src/tts/elevenlabs.rs b/src/tts/elevenlabs.rs index 30b2913..a6e9480 100644 --- a/src/tts/elevenlabs.rs +++ b/src/tts/elevenlabs.rs @@ -5,7 +5,6 @@ use async_trait::async_trait; use reqwest::Client; use serde::Serialize; -use std::time::Duration; use super::TtsProvider; use crate::error::{Result, VoiceError}; @@ -73,11 +72,7 @@ impl ElevenLabsProvider { } fn create_client() -> Result { - Client::builder() - .no_proxy() - .timeout(Duration::from_secs(30)) - .build() - .map_err(|e| VoiceError::Voice(format!("Failed to create HTTP client: {}", e))) + crate::http::tts_client() } fn play_audio(&self, audio_data: &[u8]) -> Result<()> { diff --git a/src/tts/google.rs b/src/tts/google.rs index 120060f..864b0ef 100644 --- a/src/tts/google.rs +++ b/src/tts/google.rs @@ -4,7 +4,6 @@ use async_trait::async_trait; use base64::Engine; use reqwest::Client; use serde::{Deserialize, Serialize}; -use std::time::Duration; use super::TtsProvider; use crate::error::{Result, VoiceError}; @@ -113,13 +112,7 @@ impl GoogleTtsProvider { /// Create HTTP client lazily (avoids issues in parallel tests) fn create_client() -> Result { - Client::builder() - .no_proxy() // Disable system proxy detection to avoid CoreFoundation crash - .timeout(Duration::from_secs(30)) - .build() - .map_err(|e| { - crate::error::VoiceError::Voice(format!("Failed to create HTTP client: {}", e)) - }) + crate::http::tts_client() } /// Play audio data through the platform player diff --git a/src/tts/openai.rs b/src/tts/openai.rs index 739cdcd..86b377a 100644 --- a/src/tts/openai.rs +++ b/src/tts/openai.rs @@ -5,7 +5,6 @@ use async_trait::async_trait; use reqwest::Client; use serde::Serialize; -use std::time::Duration; use super::TtsProvider; use crate::error::{Result, VoiceError}; @@ -59,11 +58,7 @@ impl OpenAiTtsProvider { } fn create_client() -> Result { - Client::builder() - .no_proxy() - .timeout(Duration::from_secs(30)) - .build() - .map_err(|e| VoiceError::Voice(format!("Failed to create HTTP client: {}", e))) + crate::http::tts_client() } fn play_audio(&self, audio_data: &[u8]) -> Result<()> { diff --git a/src/tts/xai.rs b/src/tts/xai.rs index a044e3f..69273d1 100644 --- a/src/tts/xai.rs +++ b/src/tts/xai.rs @@ -4,7 +4,6 @@ use async_trait::async_trait; use reqwest::Client; use serde::Serialize; -use std::time::Duration; use super::TtsProvider; use crate::error::{Result, VoiceError}; @@ -49,11 +48,7 @@ impl XaiTtsProvider { } fn create_client() -> Result { - Client::builder() - .no_proxy() - .timeout(Duration::from_secs(30)) - .build() - .map_err(|e| VoiceError::Voice(format!("Failed to create HTTP client: {}", e))) + crate::http::tts_client() } fn play_audio(&self, audio_data: &[u8]) -> Result<()> { From bb76962abfdfe01c9dd36192435a610bf99bdfcc Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:09:27 +0800 Subject: [PATCH 10/13] refactor(tts): share the espeak and piper synth-and-play routine Both engines spawn a binary, feed it text on stdin, enforce a timeout and play the rendered WAV. That body now lives in tts::render. Temp files for espeak are named sumvox_espeak-ng_* instead of sumvox_espeak_*. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/tts/espeak.rs | 110 +++++----------------------------------------- src/tts/mod.rs | 1 + src/tts/piper.rs | 98 +++++------------------------------------ src/tts/render.rs | 102 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 188 deletions(-) create mode 100644 src/tts/render.rs diff --git a/src/tts/espeak.rs b/src/tts/espeak.rs index 0883957..edd1a7e 100644 --- a/src/tts/espeak.rs +++ b/src/tts/espeak.rs @@ -7,16 +7,13 @@ // amplitude flag is deliberately unused — volume is a playback-time knob. use std::path::Path; -use std::sync::atomic::{AtomicU64, Ordering}; -use std::time::Duration; use async_trait::async_trait; -use tokio::io::AsyncWriteExt; -use tokio::process::Command; +use super::render::render_then_play; use super::TtsProvider; use crate::audio::player; -use crate::error::{Result, VoiceError}; +use crate::error::Result; /// espeak-ng's own default speech rate, in words per minute. pub const DEFAULT_RATE: u32 = 175; @@ -25,14 +22,6 @@ pub const DEFAULT_RATE: u32 = 175; /// paths can be exercised with `true`/`false` stand-ins. const BINARY: &str = "espeak-ng"; -/// Wall-clock cap on synthesis. espeak is ~9 ms for a short sentence, so this -/// is ~3000x headroom: the cap exists to break a wedge, not to police latency. -const SYNTH_TIMEOUT: Duration = Duration::from_secs(30); - -// Per-call counter so the temp path is unique even for concurrent calls that -// share a PID — same collision-safety scheme as audio/normalize.rs. -static CALL_SEQ: AtomicU64 = AtomicU64::new(0); - /// Bounds espeak-ng accepts for `-s`; outside them it rejects the argument. const MIN_RATE: u32 = 80; const MAX_RATE: u32 = 450; @@ -114,13 +103,6 @@ impl TtsProvider for EspeakProvider { } async fn speak(&self, text: &str) -> Result { - // Guard before any temp-path or spawn work: nothing to say is a - // deliberate no-op, not a failure. Matches every other provider. - if text.trim().is_empty() { - tracing::warn!("Empty message, skipping voice notification"); - return Ok(false); - } - tracing::info!( "Speaking with espeak-ng: voice={:?}, rate={}, volume={}", self.voice, @@ -128,86 +110,14 @@ impl TtsProvider for EspeakProvider { self.volume ); - // Qualify by PID + per-call counter so concurrent invocations never - // clobber each other's file. - let wav_path = std::env::temp_dir().join(format!( - "sumvox_espeak_{}_{}.wav", - std::process::id(), - CALL_SEQ.fetch_add(1, Ordering::Relaxed) - )); - - let args = espeak_args(self.voice.as_deref(), self.rate, &wav_path); - tracing::debug!("espeak-ng argv: {:?}", args); - - let spawned = Command::new(&self.binary) - .args(&args) - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::piped()) - // If we bail out early (timeout), don't leave the child running. - .kill_on_drop(true) - .spawn(); - - let mut child = spawned.map_err(|e| { - VoiceError::Voice(format!("espeak-ng synthesis failed: could not run: {}", e)) - })?; - - // The text goes to stdin, never onto the command line. A write failure - // is not fatal on its own — the exit status decides. - if let Some(mut stdin) = child.stdin.take() { - if let Err(e) = stdin.write_all(text.as_bytes()).await { - tracing::debug!("espeak-ng: failed writing text to stdin: {}", e); - } - // Dropping/closing stdin signals EOF so espeak stops reading. - let _ = stdin.shutdown().await; - } - - let output = match tokio::time::timeout(SYNTH_TIMEOUT, child.wait_with_output()).await { - Ok(Ok(output)) => output, - Ok(Err(e)) => { - let _ = std::fs::remove_file(&wav_path); - return Err(VoiceError::Voice(format!( - "espeak-ng synthesis failed: {}", - e - ))); - } - Err(_) => { - // kill_on_drop reaps the child as `child` is dropped here. - let _ = std::fs::remove_file(&wav_path); - return Err(VoiceError::Voice(format!( - "espeak-ng synthesis failed: timed out after {}s", - SYNTH_TIMEOUT.as_secs() - ))); - } - }; - - if !output.status.success() { - // espeak may have left a partial file behind before failing. - let _ = std::fs::remove_file(&wav_path); - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(VoiceError::Voice(format!( - "espeak-ng synthesis failed: {}", - stderr.trim() - ))); - } - - // Exit zero with no audio must not reach the player: that would look - // like success while producing silence. - let rendered = std::fs::metadata(&wav_path).map(|m| m.len()).unwrap_or(0); - if rendered == 0 { - let _ = std::fs::remove_file(&wav_path); - return Err(VoiceError::Voice( - "espeak-ng synthesis failed: produced no audio".to_string(), - )); - } - - // Clean up on every path, including the playback-error path. - let result = player::play_file(&wav_path, self.volume); - let _ = std::fs::remove_file(&wav_path); - result?; - - tracing::debug!("espeak-ng playback completed"); - Ok(true) + render_then_play( + "espeak-ng", + &self.binary, + |wav| espeak_args(self.voice.as_deref(), self.rate, wav), + text, + self.volume, + ) + .await } } diff --git a/src/tts/mod.rs b/src/tts/mod.rs index 7f89ab9..123948b 100644 --- a/src/tts/mod.rs +++ b/src/tts/mod.rs @@ -9,6 +9,7 @@ pub mod google; pub mod macos; pub mod openai; pub mod piper; +mod render; pub mod xai; use async_trait::async_trait; diff --git a/src/tts/piper.rs b/src/tts/piper.rs index 75453d4..63d11b8 100644 --- a/src/tts/piper.rs +++ b/src/tts/piper.rs @@ -13,16 +13,13 @@ // player, exactly as for espeak and macOS `say`. use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicU64, Ordering}; -use std::time::Duration; use async_trait::async_trait; -use tokio::io::AsyncWriteExt; -use tokio::process::Command; +use super::render::render_then_play; use super::TtsProvider; use crate::audio::player; -use crate::error::{Result, VoiceError}; +use crate::error::Result; /// SumVox's own default rate, chosen so it lands exactly on piper's default /// length-scale of 1.0. @@ -31,12 +28,6 @@ pub const DEFAULT_RATE: u32 = 200; /// The program we invoke. Overridable in tests only. const BINARY: &str = "piper"; -/// Wall-clock cap on synthesis. piper needs ~690 ms, so this is ~40x headroom. -const SYNTH_TIMEOUT: Duration = Duration::from_secs(30); - -// Per-call counter so concurrent calls sharing a PID get distinct temp paths. -static CALL_SEQ: AtomicU64 = AtomicU64::new(0); - /// Resolve which `.onnx` model to speak with, from a config entry's three /// candidate fields. /// @@ -152,13 +143,6 @@ impl TtsProvider for PiperProvider { } async fn speak(&self, text: &str) -> Result { - // Guard before any temp-path or spawn work: nothing to say is a - // deliberate no-op, not a failure. Matches every other provider. - if text.trim().is_empty() { - tracing::warn!("Empty message, skipping voice notification"); - return Ok(false); - } - tracing::info!( "Speaking with piper: model={:?}, rate={}, volume={}", self.model_path, @@ -166,76 +150,14 @@ impl TtsProvider for PiperProvider { self.volume ); - let wav_path = std::env::temp_dir().join(format!( - "sumvox_piper_{}_{}.wav", - std::process::id(), - CALL_SEQ.fetch_add(1, Ordering::Relaxed) - )); - - let args = piper_args(&self.model_path, self.rate, &wav_path); - tracing::debug!("piper argv: {:?}", args); - - let spawned = Command::new(&self.binary) - .args(&args) - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::piped()) - .kill_on_drop(true) - .spawn(); - - let mut child = spawned.map_err(|e| { - VoiceError::Voice(format!("piper synthesis failed: could not run: {}", e)) - })?; - - // Text goes to stdin, never onto the command line. A write failure is - // not fatal on its own — the exit status decides. - if let Some(mut stdin) = child.stdin.take() { - if let Err(e) = stdin.write_all(text.as_bytes()).await { - tracing::debug!("piper: failed writing text to stdin: {}", e); - } - let _ = stdin.shutdown().await; - } - - let output = match tokio::time::timeout(SYNTH_TIMEOUT, child.wait_with_output()).await { - Ok(Ok(output)) => output, - Ok(Err(e)) => { - let _ = std::fs::remove_file(&wav_path); - return Err(VoiceError::Voice(format!("piper synthesis failed: {}", e))); - } - Err(_) => { - let _ = std::fs::remove_file(&wav_path); - return Err(VoiceError::Voice(format!( - "piper synthesis failed: timed out after {}s", - SYNTH_TIMEOUT.as_secs() - ))); - } - }; - - if !output.status.success() { - let _ = std::fs::remove_file(&wav_path); - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(VoiceError::Voice(format!( - "piper synthesis failed: {}", - stderr.trim() - ))); - } - - // Exit zero with no audio must not reach the player. - let rendered = std::fs::metadata(&wav_path).map(|m| m.len()).unwrap_or(0); - if rendered == 0 { - let _ = std::fs::remove_file(&wav_path); - return Err(VoiceError::Voice( - "piper synthesis failed: produced no audio".to_string(), - )); - } - - // Clean up on every path, including the playback-error path. - let result = player::play_file(&wav_path, self.volume); - let _ = std::fs::remove_file(&wav_path); - result?; - - tracing::debug!("piper playback completed"); - Ok(true) + render_then_play( + "piper", + &self.binary, + |wav| piper_args(&self.model_path, self.rate, wav), + text, + self.volume, + ) + .await } } diff --git a/src/tts/render.rs b/src/tts/render.rs new file mode 100644 index 0000000..df1791c --- /dev/null +++ b/src/tts/render.rs @@ -0,0 +1,102 @@ +// Shared synth-then-play routine for engines that render text to a WAV file by +// running a local binary (espeak-ng, piper) + +use std::path::Path; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Duration; + +use tokio::io::AsyncWriteExt; +use tokio::process::Command; + +use crate::audio::player; +use crate::error::{Result, VoiceError}; + +const SYNTH_TIMEOUT: Duration = Duration::from_secs(30); + +/// Per-call counter, so concurrent invocations never share a temp file. +static CALL_SEQ: AtomicU64 = AtomicU64::new(0); + +/// Run `binary` with the argv `args` builds for the output path, feed it `text` on +/// stdin, then play the rendered WAV and remove it. Empty text is a no-op (`false`). +pub async fn render_then_play( + label: &str, + binary: &str, + args: impl FnOnce(&Path) -> Vec, + text: &str, + volume: u32, +) -> Result { + if text.trim().is_empty() { + tracing::warn!("Empty message, skipping voice notification"); + return Ok(false); + } + + let wav_path = std::env::temp_dir().join(format!( + "sumvox_{}_{}_{}.wav", + label, + std::process::id(), + CALL_SEQ.fetch_add(1, Ordering::Relaxed) + )); + let fail = |detail: String| VoiceError::Voice(format!("{label} synthesis failed: {detail}")); + + let args = args(&wav_path); + tracing::debug!("{} argv: {:?}", label, args); + + let mut child = Command::new(binary) + .args(&args) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::piped()) + // If we bail out early (timeout), don't leave the child running. + .kill_on_drop(true) + .spawn() + .map_err(|e| fail(format!("could not run: {e}")))?; + + // The text goes to stdin, never onto the command line. A write failure is + // not fatal on its own: the exit status decides. + if let Some(mut stdin) = child.stdin.take() { + if let Err(e) = stdin.write_all(text.as_bytes()).await { + tracing::debug!("{}: failed writing text to stdin: {}", label, e); + } + // Closing stdin signals EOF so the engine stops reading. + let _ = stdin.shutdown().await; + } + + let output = match tokio::time::timeout(SYNTH_TIMEOUT, child.wait_with_output()).await { + Ok(Ok(output)) => output, + Ok(Err(e)) => { + let _ = std::fs::remove_file(&wav_path); + return Err(fail(e.to_string())); + } + Err(_) => { + // kill_on_drop reaps the child as `child` is dropped here. + let _ = std::fs::remove_file(&wav_path); + return Err(fail(format!( + "timed out after {}s", + SYNTH_TIMEOUT.as_secs() + ))); + } + }; + + if !output.status.success() { + // The engine may have left a partial file behind before failing. + let _ = std::fs::remove_file(&wav_path); + return Err(fail( + String::from_utf8_lossy(&output.stderr).trim().to_string(), + )); + } + + // Exit zero with no audio must not reach the player: that would look like + // success while producing silence. + if std::fs::metadata(&wav_path).map_or(0, |m| m.len()) == 0 { + let _ = std::fs::remove_file(&wav_path); + return Err(fail("produced no audio".to_string())); + } + + // Clean up on every path, including the playback-error path. + let result = player::play_file(&wav_path, volume); + let _ = std::fs::remove_file(&wav_path); + result?; + + tracing::debug!("{} playback completed", label); + Ok(true) +} From 898f14e9bda20720882fe9acc4d8cb7a84a9bb85 Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:11:35 +0800 Subject: [PATCH 11/13] feat(config)!: make `sumvox init` write SumvoxConfig::default() Config::default() was Google-first with voice Zephyr while init wrote macOS-first with voice Aoede and a macos notification provider. The init settings are now the single definition: init serializes the default unchanged, and a test checks the written file round-trips to it. A config with no [tts] table, or no [hooks.claude_code] table, now gets these defaults, and the Notification hook defaults to macos in that case. BREAKING CHANGE: the built-in default TTS chain is macos then google (voice Aoede), and the default Notification TTS provider is macos. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/config.rs | 40 ++++++++++++++++++++++++++++++++++------ src/main.rs | 32 ++++---------------------------- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/config.rs b/src/config.rs index 47b4e99..40e886a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -364,14 +364,14 @@ impl Default for TtsConfig { Self { providers: vec![ TtsProviderConfig { - name: "google".to_string(), - model: Some("gemini-2.5-flash-preview-tts".to_string()), - voice: Some("Zephyr".to_string()), + name: "macos".to_string(), + rate: Some(200), ..Default::default() }, TtsProviderConfig { - name: "macos".to_string(), - rate: Some(200), + name: "google".to_string(), + model: Some("gemini-2.5-flash-preview-tts".to_string()), + voice: Some("Aoede".to_string()), ..Default::default() }, ], @@ -474,7 +474,7 @@ impl Default for ClaudeCodeHookConfig { fn default() -> Self { Self { notification_filter: default_notification_filter(), - notification_tts_provider: default_auto_tts(), + notification_tts_provider: Some("macos".to_string()), stop_tts_provider: default_auto_tts(), notification_volume: None, // Will use 80 in runtime if None stop_volume: None, // Will use 100 in runtime if None @@ -758,6 +758,34 @@ mod tests { assert!(json.contains("${PROVIDER_API_KEY}")); } + #[test] + fn test_default_config_round_trips_through_toml() { + // `sumvox init` writes SumvoxConfig::default() and nothing else. Unset API keys + // serialize as a placeholder, so compare the canonical TOML, not the structs. + let temp_dir = tempfile::tempdir().unwrap(); + let path = temp_dir.path().join("config.toml"); + let default = SumvoxConfig::default(); + + default.save_toml(path.clone()).unwrap(); + + let written = std::fs::read_to_string(&path).unwrap(); + assert_eq!(written, toml::to_string_pretty(&default).unwrap()); + let loaded = SumvoxConfig::load_toml(path).unwrap(); + assert_eq!( + toml::to_string_pretty(&loaded).unwrap(), + toml::to_string_pretty(&default).unwrap() + ); + assert_eq!(loaded.tts.providers[0].name, "macos"); + assert_eq!( + loaded + .hooks + .claude_code + .notification_tts_provider + .as_deref(), + Some("macos") + ); + } + #[test] fn test_load_save_toml() { let temp_dir = tempfile::tempdir().unwrap(); diff --git a/src/main.rs b/src/main.rs index 41ef9a4..9158e8b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,7 +21,7 @@ use std::io::{IsTerminal, Read}; use clap::Parser; use cli::{Cli, Commands, InitArgs, JsonArgs, SayArgs, SumArgs}; -use config::{SumvoxConfig, TtsProviderConfig}; +use config::SumvoxConfig; use error::{Result, VoiceError}; use hooks::claude_code::ClaudeCodeInput; use hooks::HookFormat; @@ -235,33 +235,9 @@ async fn handle_init(args: InitArgs) -> Result<()> { return Ok(()); } - // Create default config with recommended settings - let mut config = SumvoxConfig::default(); - - // Apply recommended settings - config.summarization.system_message = - "You are a voice notification assistant. Generate concise summaries suitable for voice playback.".to_string(); - config.summarization.fallback_message = "Task completed".to_string(); - - // Set notification TTS to macos by default (fast and free) - config.hooks.claude_code.notification_tts_provider = Some("macos".to_string()); - - // Update default TTS to prefer macOS - config.tts.providers = vec![ - TtsProviderConfig { - name: "macos".to_string(), - rate: Some(200), - ..Default::default() - }, - TtsProviderConfig { - name: "google".to_string(), - model: Some("gemini-2.5-flash-preview-tts".to_string()), - voice: Some("Aoede".to_string()), - ..Default::default() - }, - ]; - - // Save as TOML (preferred format) + // The defaults are the recommended settings + let config = SumvoxConfig::default(); + config.save_to_home()?; eprintln!("✓ Created config at: {:?}", toml_path); From 2d647e2d4b8637f9a748c1155db16d17ed9460b3 Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 16:11:44 +0800 Subject: [PATCH 12/13] docs: changelog for config migration removal and unified defaults Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8be0a1..a73c66b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **BREAKING: the built-in defaults now match `sumvox init`.** The default TTS chain is `macos` then `google` (voice `Aoede`, was `Zephyr`), and the Notification hook speaks through `macos` by default. `sumvox init` writes exactly these defaults. A config with no `[tts]` table, or no `[hooks.claude_code]` table, picks them up. +- `--tts gemini` is now accepted as an alias for the `google` engine, matching the provider-name alias. +- A set-but-empty `GEMINI_API_KEY` no longer hides `GOOGLE_API_KEY`. + +### Removed +- **BREAKING: legacy `config.yaml` / `config.yml` / `config.json` are no longer read or migrated.** If one of them exists without a `config.toml`, sumvox now fails with an error telling you to convert it to `config.toml` or run `sumvox init`. `sumvox init` no longer treats them as an existing config. +- The `TTS cost estimate` info log line is gone. + ### Fixed - Over-long text for ElevenLabs and xAI TTS no longer crashes sumvox when it contains Chinese or other multi-byte characters; it is cut at a character boundary. - `--rate` is now optional: when omitted, the `rate` from your config (or the engine default) is used instead of being overwritten with 200. From 50b202c7e5a74f1a3d36d1715773c92b0186938f Mon Sep 17 00:00:00 2001 From: musingfox Date: Thu, 1 Oct 2026 17:36:49 +0800 Subject: [PATCH 13/13] feat(config): default to the platform's local TTS engine The defaults that `sumvox init` writes named `macos` on every platform, so on Linux the Notification hook failed unless the user edited it. The local engine is now picked at compile time: macOS `say` on macOS, espeak-ng elsewhere, matching what the Linux e2e job already uses. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 +- src/config.rs | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a73c66b..49c419d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed -- **BREAKING: the built-in defaults now match `sumvox init`.** The default TTS chain is `macos` then `google` (voice `Aoede`, was `Zephyr`), and the Notification hook speaks through `macos` by default. `sumvox init` writes exactly these defaults. A config with no `[tts]` table, or no `[hooks.claude_code]` table, picks them up. +- **BREAKING: the built-in defaults now match `sumvox init`.** The default TTS chain is the platform's local engine (`macos` on macOS, `espeak` elsewhere) then `google` (voice `Aoede`, was `Zephyr`), and the Notification hook speaks through that local engine by default. `sumvox init` writes exactly these defaults. A config with no `[tts]` table, or no `[hooks.claude_code]` table, picks them up. - `--tts gemini` is now accepted as an alias for the `google` engine, matching the provider-name alias. - A set-but-empty `GEMINI_API_KEY` no longer hides `GOOGLE_API_KEY`. diff --git a/src/config.rs b/src/config.rs index 40e886a..0812107 100644 --- a/src/config.rs +++ b/src/config.rs @@ -359,12 +359,20 @@ pub struct TtsConfig { pub providers: Vec, } +/// The local TTS engine `init` and the defaults use on this platform: macOS +/// `say` where it exists, espeak-ng elsewhere. +pub(crate) const LOCAL_TTS: &str = if cfg!(target_os = "macos") { + "macos" +} else { + "espeak" +}; + impl Default for TtsConfig { fn default() -> Self { Self { providers: vec![ TtsProviderConfig { - name: "macos".to_string(), + name: LOCAL_TTS.to_string(), rate: Some(200), ..Default::default() }, @@ -474,7 +482,7 @@ impl Default for ClaudeCodeHookConfig { fn default() -> Self { Self { notification_filter: default_notification_filter(), - notification_tts_provider: Some("macos".to_string()), + notification_tts_provider: Some(LOCAL_TTS.to_string()), stop_tts_provider: default_auto_tts(), notification_volume: None, // Will use 80 in runtime if None stop_volume: None, // Will use 100 in runtime if None @@ -775,14 +783,14 @@ mod tests { toml::to_string_pretty(&loaded).unwrap(), toml::to_string_pretty(&default).unwrap() ); - assert_eq!(loaded.tts.providers[0].name, "macos"); + assert_eq!(loaded.tts.providers[0].name, LOCAL_TTS); assert_eq!( loaded .hooks .claude_code .notification_tts_provider .as_deref(), - Some("macos") + Some(LOCAL_TTS) ); }