From 2737e6164eae5e1a179d8eb28f60be192d270712 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 3 Mar 2026 14:49:02 -0500 Subject: [PATCH] Add colored output to models list and test, fix clippy warnings Add ANSI color to `arc models` and `arc models test` output when stdout is a TTY: bold model IDs, dim provider/aliases, cyan speed, green/red test results. Add `Styles::detect_stdout()` to arc-util. Also fix pre-existing clippy warnings: derive Default instead of manual impls for enums in server_config, remove unused FailureDetail imports in arc-workflows error tests, inline print literal in test_models header. Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.lock | 1 + crates/arc-api/src/server_config.rs | 27 +++++-------------- crates/arc-llm/Cargo.toml | 1 + crates/arc-llm/src/cli.rs | 40 +++++++++++++++++------------ crates/arc-util/src/terminal.rs | 8 ++++++ crates/arc-workflows/src/error.rs | 3 +-- 6 files changed, 41 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31d5b2a45..78560a0af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -227,6 +227,7 @@ name = "arc-llm" version = "0.1.0" dependencies = [ "anyhow", + "arc-util", "async-trait", "base64", "bytes", diff --git a/crates/arc-api/src/server_config.rs b/crates/arc-api/src/server_config.rs index 52f0709f2..65374ae62 100644 --- a/crates/arc-api/src/server_config.rs +++ b/crates/arc-api/src/server_config.rs @@ -3,19 +3,14 @@ use std::path::PathBuf; use arc_workflows::cli::run_config::RunDefaults; use serde::Deserialize; -#[derive(Debug, Clone, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Deserialize, PartialEq)] #[serde(rename_all = "snake_case")] pub enum AuthProvider { + #[default] Github, InsecureDisabled, } -impl Default for AuthProvider { - fn default() -> Self { - Self::Github - } -} - #[derive(Debug, Clone, Default, Deserialize, PartialEq)] pub struct AuthConfig { #[serde(default)] @@ -24,19 +19,14 @@ pub struct AuthConfig { pub allowed_usernames: Vec, } -#[derive(Debug, Clone, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Deserialize, PartialEq)] #[serde(rename_all = "snake_case")] pub enum ApiAuthenticationStrategy { + #[default] Jwt, InsecureDisabled, } -impl Default for ApiAuthenticationStrategy { - fn default() -> Self { - Self::Jwt - } -} - #[derive(Debug, Clone, Deserialize, PartialEq)] pub struct ApiConfig { #[serde(default = "default_base_url")] @@ -58,18 +48,13 @@ impl Default for ApiConfig { } } -#[derive(Debug, Clone, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Deserialize, PartialEq)] #[serde(rename_all = "snake_case")] pub enum GitProvider { + #[default] Github, } -impl Default for GitProvider { - fn default() -> Self { - Self::Github - } -} - #[derive(Debug, Clone, Default, Deserialize, PartialEq)] pub struct GitConfig { #[serde(default)] diff --git a/crates/arc-llm/Cargo.toml b/crates/arc-llm/Cargo.toml index 965b18a30..9ac5dbb76 100644 --- a/crates/arc-llm/Cargo.toml +++ b/crates/arc-llm/Cargo.toml @@ -29,6 +29,7 @@ bytes.workspace = true tokio-util.workspace = true clap.workspace = true tracing.workspace = true +arc-util = { path = "../arc-util" } [dev-dependencies] http = "1" diff --git a/crates/arc-llm/src/cli.rs b/crates/arc-llm/src/cli.rs index 100371627..98c8efa91 100644 --- a/crates/arc-llm/src/cli.rs +++ b/crates/arc-llm/src/cli.rs @@ -5,6 +5,8 @@ use anyhow::{bail, Context, Result}; use clap::{Args, Subcommand}; use futures::StreamExt; +use arc_util::terminal::Styles; + use crate::catalog; use crate::generate::{self, GenerateParams}; use crate::types::Message; @@ -96,22 +98,24 @@ fn format_speed(tps: Option) -> String { } } -fn print_models_table(models: &[crate::types::ModelInfo]) { +fn print_models_table(models: &[crate::types::ModelInfo], s: &Styles) { println!( - "{:<24} {:<12} {:<24} {:>10} {:>7} {:>7} {:>10}", - "MODEL", "PROVIDER", "ALIASES", "CONTEXT", "COST", "", "SPEED" + "{b}{d}{:<24} {:<12} {:<24} {:>10} {:>7} {:>7} {:>10}{r}", + "MODEL", "PROVIDER", "ALIASES", "CONTEXT", "COST", "", "SPEED", + b = s.bold, d = s.dim, r = s.reset, ); for model in models { let aliases = model.aliases.join(", "); println!( - "{:<24} {:<12} {:<24} {:>10} {:>7} / {:<7} {:>10}", + "{b}{:<24}{r} {d}{:<12}{r} {d}{:<24}{r} {:>10} {:>7} / {:<7} {c}{:>10}{r}", model.id, model.provider, aliases, format_context_window(model.context_window), format_cost(model.input_cost_per_million), format_cost(model.output_cost_per_million), - format_speed(model.estimated_output_tps) + format_speed(model.estimated_output_tps), + b = s.bold, d = s.dim, c = s.cyan, r = s.reset, ); } } @@ -334,6 +338,8 @@ pub async fn run_models(command: Option) -> Result<()> { query: None, }); + let styles = Styles::detect_stdout(); + match command { ModelsCommand::List { provider, query } => { let mut models = catalog::list_models(provider.as_deref()); @@ -349,17 +355,17 @@ pub async fn run_models(command: Option) -> Result<()> { }); } - print_models_table(&models); + print_models_table(&models, &styles); } ModelsCommand::Test { provider, model } => { - test_models(provider.as_deref(), model.as_deref()).await?; + test_models(provider.as_deref(), model.as_deref(), &styles).await?; } } Ok(()) } -async fn test_models(provider: Option<&str>, model: Option<&str>) -> Result<()> { +async fn test_models(provider: Option<&str>, model: Option<&str>, s: &Styles) -> Result<()> { let models_to_test = if let Some(model_id) = model { match catalog::get_model_info(model_id) { Some(info) => vec![info], @@ -374,8 +380,9 @@ async fn test_models(provider: Option<&str>, model: Option<&str>) -> Result<()> } println!( - "{:<24} {:<12} {:>10} {:>7} {:>7} {:>10} {}", - "MODEL", "PROVIDER", "CONTEXT", "COST", "", "SPEED", "RESULT" + "{b}{d}{:<24} {:<12} {:>10} {:>7} {:>7} {:>10} RESULT{r}", + "MODEL", "PROVIDER", "CONTEXT", "COST", "", "SPEED", + b = s.bold, d = s.dim, r = s.reset, ); let mut failures = 0u32; @@ -388,26 +395,27 @@ async fn test_models(provider: Option<&str>, model: Option<&str>) -> Result<()> let result = tokio::time::timeout(Duration::from_secs(30), generate::generate(params)).await; - let status = match result { - Ok(Ok(_)) => "ok".to_string(), + let (status_color, status) = match result { + Ok(Ok(_)) => (s.green, "ok".to_string()), Ok(Err(e)) => { failures += 1; - format!("error: {e}") + (s.red, format!("error: {e}")) } Err(_) => { failures += 1; - "error: timeout (30s)".to_string() + (s.red, "error: timeout (30s)".to_string()) } }; println!( - "{:<24} {:<12} {:>10} {:>7} / {:<7} {:>10} {status}", + "{b}{:<24}{r} {d}{:<12}{r} {:>10} {:>7} / {:<7} {c}{:>10}{r} {sc}{status}{r}", info.id, info.provider, format_context_window(info.context_window), format_cost(info.input_cost_per_million), format_cost(info.output_cost_per_million), - format_speed(info.estimated_output_tps) + format_speed(info.estimated_output_tps), + b = s.bold, d = s.dim, c = s.cyan, r = s.reset, sc = status_color, ); } diff --git a/crates/arc-util/src/terminal.rs b/crates/arc-util/src/terminal.rs index 2c442026c..f0224167f 100644 --- a/crates/arc-util/src/terminal.rs +++ b/crates/arc-util/src/terminal.rs @@ -49,6 +49,14 @@ impl Styles { let use_color = std::io::stderr().is_terminal() && std::env::var_os("NO_COLOR").is_none(); Self::new(use_color) } + + /// Create styles based on whether stdout is a TTY. + /// Respects `NO_COLOR` environment variable. + #[must_use] + pub fn detect_stdout() -> Self { + let use_color = std::io::stdout().is_terminal() && std::env::var_os("NO_COLOR").is_none(); + Self::new(use_color) + } } #[cfg(test)] diff --git a/crates/arc-workflows/src/error.rs b/crates/arc-workflows/src/error.rs index f4164f129..de4c6d995 100644 --- a/crates/arc-workflows/src/error.rs +++ b/crates/arc-workflows/src/error.rs @@ -1648,7 +1648,6 @@ mod tests { #[test] fn e2e_llm_error_to_outcome_to_event_preserves_classification() { use crate::event::WorkflowRunEvent; - use crate::outcome::FailureDetail; // 1. Create SdkError → ArcError let sdk_err = SdkError::Provider { @@ -1736,7 +1735,7 @@ mod tests { #[test] fn e2e_failure_detail_in_outcome_serde_roundtrip() { - use crate::outcome::{FailureDetail, Outcome}; + use crate::outcome::Outcome; let outcome = Outcome::fail_classify("rate limit exceeded") .with_signature(Some("api_transient|openai|rate_limited"));