From 840383fcf7a35ea2d7da7fd2df88e32699489760 Mon Sep 17 00:00:00 2001 From: vikingowl Date: Wed, 27 Aug 2025 23:58:57 +0200 Subject: [PATCH] [feat] add JSON and quiet output modes for `models` subcommands, update UI suppression logic, and enhance CLI test coverage --- crates/polyscribe-cli/src/cli.rs | 27 ++-- crates/polyscribe-cli/src/main.rs | 163 +++++++++++++++----- crates/polyscribe-cli/src/output.rs | 36 +++++ crates/polyscribe-cli/tests/models_smoke.rs | 42 +++++ 4 files changed, 220 insertions(+), 48 deletions(-) create mode 100644 crates/polyscribe-cli/src/output.rs create mode 100644 crates/polyscribe-cli/tests/models_smoke.rs diff --git a/crates/polyscribe-cli/src/cli.rs b/crates/polyscribe-cli/src/cli.rs index 201e0ef..32d7975 100644 --- a/crates/polyscribe-cli/src/cli.rs +++ b/crates/polyscribe-cli/src/cli.rs @@ -1,4 +1,4 @@ -use clap::{Parser, Subcommand, ValueEnum}; +use clap::{Args, Parser, Subcommand, ValueEnum}; use std::path::PathBuf; #[derive(Debug, Clone, ValueEnum)] @@ -10,21 +10,33 @@ pub enum GpuBackend { Vulkan, } +#[derive(Debug, Clone, Args)] +pub struct OutputOpts { + /// Emit machine-readable JSON to stdout; suppress decorative logs + #[arg(long, global = true, action = clap::ArgAction::SetTrue)] + pub json: bool, + /// Reduce log chatter (errors only unless --json) + #[arg(long, global = true, action = clap::ArgAction::SetTrue)] + pub quiet: bool, +} + #[derive(Debug, Parser)] #[command( name = "polyscribe", version, - about = "PolyScribe – local-first transcription and plugins" + about = "PolyScribe – local-first transcription and plugins", + propagate_version = true, + arg_required_else_help = true, )] pub struct Cli { + /// Global output options + #[command(flatten)] + pub output: OutputOpts, + /// Increase verbosity (-v, -vv) #[arg(short, long, action = clap::ArgAction::Count)] pub verbose: u8, - /// Quiet mode (suppresses non-error logs) - #[arg(short, long, default_value_t = false)] - pub quiet: bool, - /// Never prompt for user input (non-interactive mode) #[arg(long, default_value_t = false)] pub no_interaction: bool, @@ -105,9 +117,6 @@ pub struct ModelCommon { /// Limit download rate in bytes/sec (approximate) #[arg(long)] pub limit_rate: Option, - /// Emit machine JSON output - #[arg(long, default_value_t = false)] - pub json: bool, } #[derive(Debug, Subcommand)] diff --git a/crates/polyscribe-cli/src/main.rs b/crates/polyscribe-cli/src/main.rs index 70d46ad..6fe3ae6 100644 --- a/crates/polyscribe-cli/src/main.rs +++ b/crates/polyscribe-cli/src/main.rs @@ -1,8 +1,10 @@ mod cli; +mod output; use anyhow::{Context, Result, anyhow}; use clap::{CommandFactory, Parser}; use cli::{Cli, Commands, GpuBackend, ModelsCmd, ModelCommon, PluginsCmd}; +use output::OutputMode; use polyscribe_core::model_manager::{ModelManager, Settings, ReqwestClient}; use polyscribe_core::ui; fn normalized_similarity(a: &str, b: &str) -> f64 { @@ -50,22 +52,15 @@ use polyscribe_host::PluginManager; use tokio::io::AsyncWriteExt; use tracing_subscriber::EnvFilter; -fn init_tracing(quiet: bool, verbose: u8) { - let log_level = if quiet { - "error" - } else { - match verbose { - 0 => "info", - 1 => "debug", - _ => "trace", - } - }; - - let filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(log_level)); +fn init_tracing(json_mode: bool, quiet: bool, verbose: u8) { + // In JSON mode, suppress human logs; route errors to stderr only. + let level = if json_mode || quiet { "error" } else { match verbose { 0 => "info", 1 => "debug", _ => "trace" } }; + let filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(level)); tracing_subscriber::fmt() .with_env_filter(filter) .with_target(false) .with_level(true) + .with_writer(std::io::stderr) .compact() .init(); } @@ -73,9 +68,17 @@ fn init_tracing(quiet: bool, verbose: u8) { fn main() -> Result<()> { let args = Cli::parse(); - init_tracing(args.quiet, args.verbose); + // Determine output mode early for logging and UI configuration + let output_mode = if args.output.json { + OutputMode::Json + } else { + OutputMode::Human { quiet: args.output.quiet } + }; - polyscribe_core::set_quiet(args.quiet); + init_tracing(matches!(output_mode, OutputMode::Json), args.output.quiet, args.verbose); + + // Suppress decorative UI output in JSON mode as well + polyscribe_core::set_quiet(args.output.quiet || matches!(output_mode, OutputMode::Json)); polyscribe_core::set_no_interaction(args.no_interaction); polyscribe_core::set_verbose(args.verbose); polyscribe_core::set_no_progress(args.no_progress); @@ -128,15 +131,19 @@ fn main() -> Result<()> { ModelsCmd::Ls { common } => { let mm: ModelManager = ModelManager::new(handle_common(&common))?; let list = mm.ls()?; - if list.is_empty() { - println!("No models installed."); - } else { - if common.json { - println!("{}", serde_json::to_string_pretty(&list)?); - } else { - println!("Model (Repo)"); - for r in list { - println!("{} ({})", r.file, r.repo); + match output_mode { + OutputMode::Json => { + // Always emit JSON array (possibly empty) + output_mode.print_json(&list); + } + OutputMode::Human { quiet } => { + if list.is_empty() { + if !quiet { println!("No models installed."); } + } else { + if !quiet { println!("Model (Repo)"); } + for r in list { + if !quiet { println!("{} ({})", r.file, r.repo); } + } } } } @@ -158,15 +165,27 @@ fn main() -> Result<()> { let alias = derive_alias(&repo, &file); match mm.add_or_update(&alias, &repo, &file) { Ok(rec) => { - if common.json { println!("{}", serde_json::to_string_pretty(&rec)?); } - else { println!("installed: {} -> {}/{}", alias, repo, rec.file); } + match output_mode { + OutputMode::Json => output_mode.print_json(&rec), + OutputMode::Human { quiet } => { + if !quiet { println!("installed: {} -> {}/{}", alias, repo, rec.file); } + } + } EXIT_OK } Err(e) => { // On not found or similar errors, try suggesting close matches interactively - if common.json || polyscribe_core::is_no_interaction() { - if common.json { println!("{{\"error\":{}}}", serde_json::to_string(&e.to_string())?); } - else { eprintln!("error: {e}"); } + if matches!(output_mode, OutputMode::Json) || polyscribe_core::is_no_interaction() { + match output_mode { + OutputMode::Json => { + // Emit error JSON object + #[derive(serde::Serialize)] + struct ErrObj<'a> { error: &'a str } + let eo = ErrObj { error: &e.to_string() }; + output_mode.print_json(&eo); + } + _ => { eprintln!("error: {e}"); } + } EXIT_NOT_FOUND } else { ui::warn(format!("{}", e)); @@ -204,7 +223,13 @@ fn main() -> Result<()> { let mm2: ModelManager = ModelManager::new(settings)?; let alias2 = derive_alias(&repo, cand); match mm2.add_or_update(&alias2, &repo, cand) { - Ok(rec) => { println!("installed: {} -> {}/{}", alias2, repo, rec.file); EXIT_OK } + Ok(rec) => { + match output_mode { + OutputMode::Json => output_mode.print_json(&rec), + OutputMode::Human { quiet } => { if !quiet { println!("installed: {} -> {}/{}", alias2, repo, rec.file); } } + } + EXIT_OK + } Err(e2) => { eprintln!("error: {e2}"); EXIT_NETWORK } } } @@ -233,7 +258,13 @@ fn main() -> Result<()> { let mm2: ModelManager = ModelManager::new(settings)?; let alias2 = derive_alias(&repo, chosen); match mm2.add_or_update(&alias2, &repo, chosen) { - Ok(rec) => { println!("installed: {} -> {}/{}", alias2, repo, rec.file); EXIT_OK } + Ok(rec) => { + match output_mode { + OutputMode::Json => output_mode.print_json(&rec), + OutputMode::Human { quiet } => { if !quiet { println!("installed: {} -> {}/{}", alias2, repo, rec.file); } } + } + EXIT_OK + } Err(e2) => { eprintln!("error: {e2}"); EXIT_NETWORK } } } @@ -254,19 +285,41 @@ fn main() -> Result<()> { ModelsCmd::Rm { alias, common } => { let mm: ModelManager = ModelManager::new(handle_common(&common))?; let ok = mm.rm(&alias)?; - if common.json { println!("{{\"removed\":{}}}", ok); } - else { println!("{}", if ok { "removed" } else { "not found" }); } + match output_mode { + OutputMode::Json => { + #[derive(serde::Serialize)] + struct R { removed: bool } + output_mode.print_json(&R { removed: ok }); + } + OutputMode::Human { quiet } => { + if !quiet { println!("{}", if ok { "removed" } else { "not found" }); } + } + } if ok { EXIT_OK } else { EXIT_NOT_FOUND } } ModelsCmd::Verify { alias, common } => { let mm: ModelManager = ModelManager::new(handle_common(&common))?; let found = mm.ls()?.into_iter().any(|r| r.alias == alias); if !found { - if common.json { println!("{{\"ok\":false,\"error\":\"not found\"}}"); } else { println!("not found"); } + match output_mode { + OutputMode::Json => { + #[derive(serde::Serialize)] + struct R<'a> { ok: bool, error: &'a str } + output_mode.print_json(&R { ok: false, error: "not found" }); + } + OutputMode::Human { quiet } => { if !quiet { println!("not found"); } } + } EXIT_NOT_FOUND } else { let ok = mm.verify(&alias)?; - if common.json { println!("{{\"ok\":{}}}", ok); } else { println!("{}", if ok { "ok" } else { "corrupt" }); } + match output_mode { + OutputMode::Json => { + #[derive(serde::Serialize)] + struct R { ok: bool } + output_mode.print_json(&R { ok }); + } + OutputMode::Human { quiet } => { if !quiet { println!("{}", if ok { "ok" } else { "corrupt" }); } } + } if ok { EXIT_OK } else { EXIT_VERIFY_FAILED } } } @@ -276,7 +329,17 @@ fn main() -> Result<()> { for rec in mm.ls()? { match mm.add_or_update(&rec.alias, &rec.repo, &rec.file) { Ok(_) => {} - Err(e) => { rc = EXIT_NETWORK; if common.json { println!("{{\"alias\":\"{}\",\"error\":{}}}", rec.alias, serde_json::to_string(&e.to_string())?); } else { eprintln!("update {}: {e}", rec.alias); } } + Err(e) => { + rc = EXIT_NETWORK; + match output_mode { + OutputMode::Json => { + #[derive(serde::Serialize)] + struct R<'a> { alias: &'a str, error: String } + output_mode.print_json(&R { alias: &rec.alias, error: e.to_string() }); + } + _ => { eprintln!("update {}: {e}", rec.alias); } + } + } } } rc @@ -284,15 +347,37 @@ fn main() -> Result<()> { ModelsCmd::Gc { common } => { let mm: ModelManager = ModelManager::new(handle_common(&common))?; let (files_removed, entries_removed) = mm.gc()?; - if common.json { println!("{{\"files_removed\":{},\"entries_removed\":{}}}", files_removed, entries_removed); } - else { println!("files_removed={} entries_removed={}", files_removed, entries_removed); } + match output_mode { + OutputMode::Json => { + #[derive(serde::Serialize)] + struct R { files_removed: usize, entries_removed: usize } + output_mode.print_json(&R { files_removed, entries_removed }); + } + OutputMode::Human { quiet } => { if !quiet { println!("files_removed={} entries_removed={}", files_removed, entries_removed); } } + } EXIT_OK } ModelsCmd::Search { repo, query, common } => { let res = polyscribe_core::model_manager::search_repo(&repo, query.as_deref()); match res { - Ok(files) => { if common.json { println!("{}", serde_json::to_string_pretty(&files)?); } else { for f in files { println!("{}", f); } } EXIT_OK } - Err(e) => { if common.json { println!("{{\"error\":{}}}", serde_json::to_string(&e.to_string())?); } else { eprintln!("error: {e}"); } EXIT_NETWORK } + Ok(files) => { + match output_mode { + OutputMode::Json => output_mode.print_json(&files), + OutputMode::Human { quiet } => { for f in files { if !quiet { println!("{}", f); } } } + } + EXIT_OK + } + Err(e) => { + match output_mode { + OutputMode::Json => { + #[derive(serde::Serialize)] + struct R { error: String } + output_mode.print_json(&R { error: e.to_string() }); + } + _ => { eprintln!("error: {e}"); } + } + EXIT_NETWORK + } } } }; diff --git a/crates/polyscribe-cli/src/output.rs b/crates/polyscribe-cli/src/output.rs new file mode 100644 index 0000000..7e9ee33 --- /dev/null +++ b/crates/polyscribe-cli/src/output.rs @@ -0,0 +1,36 @@ +use std::io::{self, Write}; + +#[derive(Clone, Debug)] +pub enum OutputMode { + Json, + Human { quiet: bool }, +} + +impl OutputMode { + pub fn is_quiet(&self) -> bool { + matches!(self, OutputMode::Json) || matches!(self, OutputMode::Human { quiet: true }) + } + + pub fn print_json(&self, v: &T) { + if let OutputMode::Json = self { + // Write compact JSON to stdout without prefixes + // and ensure a trailing newline for CLI ergonomics + let s = serde_json::to_string(v).unwrap_or_else(|e| format!("\"JSON_ERROR:{}\"", e)); + println!("{}", s); + } + } + + pub fn print_line(&self, s: impl AsRef) { + match self { + OutputMode::Json => { + // Suppress human lines in JSON mode + } + OutputMode::Human { quiet } => { + if !quiet { + let _ = writeln!(io::stdout(), "{}", s.as_ref()); + } + } + } + } +} + diff --git a/crates/polyscribe-cli/tests/models_smoke.rs b/crates/polyscribe-cli/tests/models_smoke.rs new file mode 100644 index 0000000..bbd0748 --- /dev/null +++ b/crates/polyscribe-cli/tests/models_smoke.rs @@ -0,0 +1,42 @@ +use assert_cmd::cargo::cargo_bin; +use std::process::Command; + +fn bin() -> std::path::PathBuf { cargo_bin("polyscribe") } + +#[test] +fn models_help_shows_global_output_flags() { + let out = Command::new(bin()) + .args(["models", "--help"]) // subcommand help + .output() + .expect("failed to run polyscribe models --help"); + assert!(out.status.success(), "help exited non-zero: {:?}", out.status); + let stdout = String::from_utf8(out.stdout).expect("stdout not utf-8"); + assert!(stdout.contains("--json"), "--json not shown in help: {stdout}"); + assert!(stdout.contains("--quiet"), "--quiet not shown in help: {stdout}"); +} + +#[test] +fn models_version_contains_pkg_version() { + let out = Command::new(bin()) + .args(["models", "--version"]) // propagate_version + .output() + .expect("failed to run polyscribe models --version"); + assert!(out.status.success(), "version exited non-zero: {:?}", out.status); + let stdout = String::from_utf8(out.stdout).expect("stdout not utf-8"); + let want = env!("CARGO_PKG_VERSION"); + assert!(stdout.contains(want), "version output missing {want}: {stdout}"); +} + +#[test] +fn models_ls_json_quiet_emits_pure_json() { + let out = Command::new(bin()) + .args(["models", "ls", "--json", "--quiet"]) // global flags + .output() + .expect("failed to run polyscribe models ls --json --quiet"); + assert!(out.status.success(), "ls exited non-zero: {:?}", out.status); + let stdout = String::from_utf8(out.stdout).expect("stdout not utf-8"); + serde_json::from_str::(stdout.trim()).expect("stdout is not valid JSON"); + // Expect no extra logs on stdout; stderr should be empty in success path + assert!(out.stderr.is_empty(), "expected no stderr noise"); +} +