From a915543fc0d5506bd8c3e14922db53542d4c4926 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 21 Mar 2026 10:26:20 -0400 Subject: [PATCH 1/8] Suppress stderr output in detach and create modes `prepare_workflow` unconditionally printed Workflow/Graph/Goal info to stderr, which leaked into `--detach` and `create` output that should only emit the run ID. Add a `quiet` flag to suppress this output. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-cli/src/commands/create.rs | 3 +- lib/crates/fabro-cli/src/commands/run.rs | 47 +++++++++++---------- lib/crates/fabro-cli/src/main.rs | 5 ++- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/create.rs b/lib/crates/fabro-cli/src/commands/create.rs index a226f2c0e..d9611dd91 100644 --- a/lib/crates/fabro-cli/src/commands/create.rs +++ b/lib/crates/fabro-cli/src/commands/create.rs @@ -14,13 +14,14 @@ pub async fn create_run( args: &RunArgs, run_defaults: RunDefaults, styles: &Styles, + quiet: bool, ) -> anyhow::Result<(String, PathBuf)> { let workflow_path = args .workflow .as_ref() .ok_or_else(|| anyhow::anyhow!("--workflow is required"))?; - let prep = prepare_workflow(args, run_defaults, styles)?; + let prep = prepare_workflow(args, run_defaults, styles, quiet)?; let goal = prep.graph.goal(); diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs index cd53fb4dd..7faa494bf 100644 --- a/lib/crates/fabro-cli/src/commands/run.rs +++ b/lib/crates/fabro-cli/src/commands/run.rs @@ -463,6 +463,7 @@ pub(crate) fn prepare_workflow( args: &RunArgs, mut run_defaults: RunDefaults, styles: &Styles, + quiet: bool, ) -> anyhow::Result { let workflow_path = args .workflow @@ -528,30 +529,32 @@ pub(crate) fn prepare_workflow( } } - eprintln!( - "{} {} {}", - styles.bold.apply_to("Workflow:"), - graph.name, - styles.dim.apply_to(format!( - "({} nodes, {} edges)", - graph.nodes.len(), - graph.edges.len() - )), - ); - eprintln!( - "{} {}", - styles.dim.apply_to("Graph:"), - styles.dim.apply_to(relative_path(&dot_path)), - ); + if !quiet { + eprintln!( + "{} {} {}", + styles.bold.apply_to("Workflow:"), + graph.name, + styles.dim.apply_to(format!( + "({} nodes, {} edges)", + graph.nodes.len(), + graph.edges.len() + )), + ); + eprintln!( + "{} {}", + styles.dim.apply_to("Graph:"), + styles.dim.apply_to(relative_path(&dot_path)), + ); - let goal = graph.goal(); - if !goal.is_empty() { - let stripped = fabro_util::text::strip_goal_decoration(goal); - eprintln!("{} {stripped}\n", styles.bold.apply_to("Goal:")); + let goal = graph.goal(); + if !goal.is_empty() { + let stripped = fabro_util::text::strip_goal_decoration(goal); + eprintln!("{} {stripped}\n", styles.bold.apply_to("Goal:")); + } + + print_diagnostics(&diagnostics, styles); } - print_diagnostics(&diagnostics, styles); - if diagnostics.iter().any(|d| d.severity == Severity::Error) { bail!("Validation failed"); } @@ -612,7 +615,7 @@ pub async fn run_command( model, provider, run_defaults, - } = prepare_workflow(&args, run_defaults, styles)?; + } = prepare_workflow(&args, run_defaults, styles, false)?; // Extract workflow slug from the workflow path argument. // If bare name (no extension, e.g. "smoke"), use it directly. diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 7a97ddf4f..a1f13478c 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -653,7 +653,7 @@ async fn main_inner() -> (String, Result<()>) { if args.detach { // Detach mode: create + start + print run ID let (run_id, run_dir) = - commands::create::create_run(&args, cli_config.run_defaults, styles) + commands::create::create_run(&args, cli_config.run_defaults, styles, true) .await?; commands::start::start_run(&run_dir)?; println!("{run_id}"); @@ -683,7 +683,8 @@ async fn main_inner() -> (String, Result<()>) { Box::leak(Box::new(fabro_util::terminal::Styles::detect_stderr())); let cli_config = cli_config::load_cli_config(None)?; let (run_id, _run_dir) = - commands::create::create_run(&args, cli_config.run_defaults, styles).await?; + commands::create::create_run(&args, cli_config.run_defaults, styles, true) + .await?; println!("{run_id}"); } Command::Start { run } => { From 73a7c28bd22660bb20cadfa6f4595a7fa182c67e Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 21 Mar 2026 11:27:25 -0400 Subject: [PATCH 2/8] Add --deep flag to `fabro model test` for multi-turn tool-use validation Exercises a 2+ turn tool-call round-trip with reasoning_effort("high") to catch bugs like store: false that only manifest when reasoning items from turn 1 are sent back in turn 2. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-llm/src/cli.rs | 150 ++++++++++++++++++++++++++++++-- 1 file changed, 145 insertions(+), 5 deletions(-) diff --git a/lib/crates/fabro-llm/src/cli.rs b/lib/crates/fabro-llm/src/cli.rs index 1f74e897e..8a9c76c01 100644 --- a/lib/crates/fabro-llm/src/cli.rs +++ b/lib/crates/fabro-llm/src/cli.rs @@ -16,7 +16,8 @@ use fabro_util::terminal::Styles; use fabro_model as catalog; use crate::generate::{self, GenerateParams}; -use crate::types::Message; +use crate::tools::Tool; +use crate::types::{ContentPart, Message}; use fabro_model::ModelInfo; pub struct ServerConnection { @@ -76,6 +77,10 @@ pub enum ModelsCommand { /// Test a specific model #[arg(short, long)] model: Option, + + /// Run a multi-turn tool-use test (catches reasoning round-trip bugs) + #[arg(long)] + deep: bool, }, } @@ -829,12 +834,94 @@ async fn test_model_via_server( .context("Failed to parse model test response from server") } +fn build_deep_test_params(info: &ModelInfo) -> Option { + if !info.features.tools { + return None; + } + + let add_tool = Tool::active( + "add", + "Add two integers and return the sum", + serde_json::json!({ + "type": "object", + "properties": { + "a": { "type": "integer", "description": "First number" }, + "b": { "type": "integer", "description": "Second number" } + }, + "required": ["a", "b"] + }), + |args, _ctx| async move { + let a = args.get("a").and_then(|v| v.as_i64()).unwrap_or(0); + let b = args.get("b").and_then(|v| v.as_i64()).unwrap_or(0); + Ok(serde_json::json!(a + b)) + }, + ); + + let mut params = GenerateParams::new(&info.id) + .provider(&info.provider) + .prompt( + "I have three numbers: 15, 27, and 42. \ + First use the add tool to compute 15 + 27, \ + then use the add tool to add that result to 42. \ + Finally, tell me whether the grand total is even or odd and why.", + ) + .tools(vec![add_tool]) + .max_tool_rounds(5) + .max_tokens(1024); + + if info.features.reasoning { + params = params.reasoning_effort("high"); + } + + Some(params) +} + +fn validate_deep_result( + result: &crate::types::GenerateResult, + info: &ModelInfo, +) -> (cli_table::Color, String) { + // Check tool use: need at least 2 steps (tool call + follow-up) + if result.steps.len() < 2 { + return (Color::Red, "deep: fail (model did not call tool)".to_string()); + } + + // Check that step 0 had tool results (tool was executed) + if result.steps[0].tool_results.is_empty() { + return (Color::Red, "deep: fail (tool not executed)".to_string()); + } + + // Check correctness: 15+27=42, 42+42=84 — final response should contain "84" + let final_text = result.response.text(); + if !final_text.contains("84") { + return (Color::Red, "deep: fail (wrong answer)".to_string()); + } + + // Check reasoning if the model supports it + if info.features.reasoning { + let has_reasoning = result.steps.iter().any(|step| { + step.response.message.content.iter().any(|part| { + matches!(part, ContentPart::Thinking(_)) + || matches!(part, ContentPart::Other { kind, .. } if kind == ContentPart::OPENAI_REASONING) + }) + }); + if !has_reasoning { + return (Color::Yellow, "deep: ok (no reasoning)".to_string()); + } + } + + (Color::Green, "deep: ok".to_string()) +} + async fn test_models_via_server( server: &ServerConnection, provider: Option<&str>, model: Option<&str>, + deep: bool, s: &Styles, ) -> Result<()> { + if deep { + eprintln!("Warning: --deep is not supported in server mode"); + } let models_to_test = if let Some(model_id) = model { let all = fetch_models_from_server(&server.client, &server.base_url, None).await?; let found: Vec<_> = all.into_iter().filter(|m| m.id == model_id).collect(); @@ -932,12 +1019,17 @@ pub async fn run_models( print_models_table(&models, &styles); } - ModelsCommand::Test { provider, model } => match &server { + ModelsCommand::Test { + provider, + model, + deep, + } => match &server { Some(s) => { - test_models_via_server(s, provider.as_deref(), model.as_deref(), &styles).await?; + test_models_via_server(s, provider.as_deref(), model.as_deref(), deep, &styles) + .await?; } None => { - test_models(provider.as_deref(), model.as_deref(), &styles).await?; + test_models(provider.as_deref(), model.as_deref(), deep, &styles).await?; } }, } @@ -945,7 +1037,12 @@ pub async fn run_models( Ok(()) } -async fn test_models(provider: Option<&str>, model: Option<&str>, s: &Styles) -> Result<()> { +async fn test_models( + provider: Option<&str>, + model: Option<&str>, + deep: bool, + s: &Styles, +) -> Result<()> { let models_to_test = if let Some(model_id) = model { match catalog::get_model_info(model_id) { Some(info) => vec![info], @@ -995,6 +1092,49 @@ async fn test_models(provider: Option<&str>, model: Option<&str>, s: &Styles) -> .foreground_color(color_if(use_color, result_color)), ); rows.push(row); + + if deep { + match build_deep_test_params(info) { + None => { + let mut deep_row = model_row(info, use_color); + deep_row.push( + "deep: skipped (no tool support)" + .cell() + .foreground_color(color_if(use_color, Color::Yellow)), + ); + rows.push(deep_row); + } + Some(params) => { + eprint!("Deep testing {}...", info.id); + let deep_result = tokio::time::timeout( + Duration::from_secs(90), + generate::generate(params), + ) + .await; + eprintln!(" done"); + + let (deep_color, deep_status) = match deep_result { + Ok(Ok(ref gen_result)) => validate_deep_result(gen_result, info), + Ok(Err(e)) => { + failures += 1; + (Color::Red, format!("deep: error: {e}")) + } + Err(_) => { + failures += 1; + (Color::Red, "deep: error: timeout (90s)".to_string()) + } + }; + + let mut deep_row = model_row(info, use_color); + deep_row.push( + deep_status + .cell() + .foreground_color(color_if(use_color, deep_color)), + ); + rows.push(deep_row); + } + } + } } let table = rows From 1493c848b5e31e31b744659decf721ab79794786 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 21 Mar 2026 11:28:23 -0400 Subject: [PATCH 3/8] Run only the deep test when --deep is passed, not both Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-llm/src/cli.rs | 72 ++++++++++++++++----------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/lib/crates/fabro-llm/src/cli.rs b/lib/crates/fabro-llm/src/cli.rs index 8a9c76c01..7460f9d4a 100644 --- a/lib/crates/fabro-llm/src/cli.rs +++ b/lib/crates/fabro-llm/src/cli.rs @@ -1063,46 +1063,16 @@ async fn test_models( let mut rows: Vec> = Vec::new(); let mut failures = 0u32; for info in &models_to_test { - eprint!("Testing {}...", info.id); - let params = GenerateParams::new(&info.id) - .provider(&info.provider) - .prompt("Say OK") - .max_tokens(16); - - let result = - tokio::time::timeout(Duration::from_secs(30), generate::generate(params)).await; - eprintln!(" done"); - - let (result_color, status) = match result { - Ok(Ok(_)) => (Color::Green, "ok".to_string()), - Ok(Err(e)) => { - failures += 1; - (Color::Red, format!("error: {e}")) - } - Err(_) => { - failures += 1; - (Color::Red, "error: timeout (30s)".to_string()) - } - }; - - let mut row = model_row(info, use_color); - row.push( - status - .cell() - .foreground_color(color_if(use_color, result_color)), - ); - rows.push(row); - if deep { match build_deep_test_params(info) { None => { - let mut deep_row = model_row(info, use_color); - deep_row.push( + let mut row = model_row(info, use_color); + row.push( "deep: skipped (no tool support)" .cell() .foreground_color(color_if(use_color, Color::Yellow)), ); - rows.push(deep_row); + rows.push(row); } Some(params) => { eprint!("Deep testing {}...", info.id); @@ -1125,15 +1095,45 @@ async fn test_models( } }; - let mut deep_row = model_row(info, use_color); - deep_row.push( + let mut row = model_row(info, use_color); + row.push( deep_status .cell() .foreground_color(color_if(use_color, deep_color)), ); - rows.push(deep_row); + rows.push(row); } } + } else { + eprint!("Testing {}...", info.id); + let params = GenerateParams::new(&info.id) + .provider(&info.provider) + .prompt("Say OK") + .max_tokens(16); + + let result = + tokio::time::timeout(Duration::from_secs(30), generate::generate(params)).await; + eprintln!(" done"); + + let (result_color, status) = match result { + Ok(Ok(_)) => (Color::Green, "ok".to_string()), + Ok(Err(e)) => { + failures += 1; + (Color::Red, format!("error: {e}")) + } + Err(_) => { + failures += 1; + (Color::Red, "error: timeout (30s)".to_string()) + } + }; + + let mut row = model_row(info, use_color); + row.push( + status + .cell() + .foreground_color(color_if(use_color, result_color)), + ); + rows.push(row); } } From d29fd66ac921f22a8dd5848af1f0a1bbb18cb65c Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 21 Mar 2026 11:36:26 -0400 Subject: [PATCH 4/8] Run model tests concurrently with progress bar Tests 6 models at a time in shuffled order to spread load across providers. Uses indicatif progress bar instead of per-model eprint lines. Results table is sorted back to original catalog order. Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.lock | 1 + lib/crates/fabro-llm/Cargo.toml | 1 + lib/crates/fabro-llm/src/cli.rs | 152 +++++++++++++++++--------------- 3 files changed, 82 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bf4ae7e007..9da580085 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1534,6 +1534,7 @@ dependencies = [ "futures", "http", "httpmock", + "indicatif", "insta", "rand 0.8.5", "reqwest 0.12.28", diff --git a/lib/crates/fabro-llm/Cargo.toml b/lib/crates/fabro-llm/Cargo.toml index 2d87759d2..087d52a0a 100644 --- a/lib/crates/fabro-llm/Cargo.toml +++ b/lib/crates/fabro-llm/Cargo.toml @@ -28,6 +28,7 @@ base64.workspace = true bytes.workspace = true tokio-util.workspace = true cli-table.workspace = true +indicatif.workspace = true clap.workspace = true dialoguer.workspace = true tracing.workspace = true diff --git a/lib/crates/fabro-llm/src/cli.rs b/lib/crates/fabro-llm/src/cli.rs index 7460f9d4a..ffd605bdc 100644 --- a/lib/crates/fabro-llm/src/cli.rs +++ b/lib/crates/fabro-llm/src/cli.rs @@ -1037,12 +1037,47 @@ pub async fn run_models( Ok(()) } +async fn test_one_model(info: &ModelInfo, deep: bool) -> (Color, String) { + if deep { + match build_deep_test_params(info) { + None => (Color::Yellow, "deep: skipped (no tool support)".to_string()), + Some(params) => { + let result = tokio::time::timeout( + Duration::from_secs(90), + generate::generate(params), + ) + .await; + match result { + Ok(Ok(ref gen_result)) => validate_deep_result(gen_result, info), + Ok(Err(e)) => (Color::Red, format!("deep: error: {e}")), + Err(_) => (Color::Red, "deep: error: timeout (90s)".to_string()), + } + } + } + } else { + let params = GenerateParams::new(&info.id) + .provider(&info.provider) + .prompt("Say OK") + .max_tokens(16); + + let result = + tokio::time::timeout(Duration::from_secs(30), generate::generate(params)).await; + match result { + Ok(Ok(_)) => (Color::Green, "ok".to_string()), + Ok(Err(e)) => (Color::Red, format!("error: {e}")), + Err(_) => (Color::Red, "error: timeout (30s)".to_string()), + } + } +} + async fn test_models( provider: Option<&str>, model: Option<&str>, deep: bool, s: &Styles, ) -> Result<()> { + use rand::seq::SliceRandom; + let models_to_test = if let Some(model_id) = model { match catalog::get_model_info(model_id) { Some(info) => vec![info], @@ -1056,85 +1091,58 @@ async fn test_models( bail!("No models found"); } + let test_kind = if deep { "Deep testing" } else { "Testing" }; + let pb = indicatif::ProgressBar::new(models_to_test.len() as u64); + pb.set_style( + indicatif::ProgressStyle::with_template( + &format!("{{spinner:.green}} {test_kind} {{pos}}/{{len}} models {{wide_bar}} {{eta}}"), + ) + .unwrap(), + ); + pb.enable_steady_tick(Duration::from_millis(100)); + + // Build (original_index, model_info) pairs, then shuffle for provider spread + let mut indexed: Vec<(usize, &ModelInfo)> = + models_to_test.iter().enumerate().collect(); + indexed.shuffle(&mut rand::thread_rng()); + + // Run tests concurrently, 6 at a time + let results: Vec<(usize, Color, String)> = futures::stream::iter(indexed) + .map(|(idx, info)| { + let pb = pb.clone(); + async move { + let (color, status) = test_one_model(info, deep).await; + pb.inc(1); + (idx, color, status) + } + }) + .buffer_unordered(6) + .collect() + .await; + + pb.finish_and_clear(); + + // Sort back to original catalog order + let mut sorted_results = results; + sorted_results.sort_by_key(|(idx, _, _)| *idx); + let use_color = s.use_color; let mut title = models_title(); title.push("RESULT".cell().bold(true)); let mut rows: Vec> = Vec::new(); let mut failures = 0u32; - for info in &models_to_test { - if deep { - match build_deep_test_params(info) { - None => { - let mut row = model_row(info, use_color); - row.push( - "deep: skipped (no tool support)" - .cell() - .foreground_color(color_if(use_color, Color::Yellow)), - ); - rows.push(row); - } - Some(params) => { - eprint!("Deep testing {}...", info.id); - let deep_result = tokio::time::timeout( - Duration::from_secs(90), - generate::generate(params), - ) - .await; - eprintln!(" done"); - - let (deep_color, deep_status) = match deep_result { - Ok(Ok(ref gen_result)) => validate_deep_result(gen_result, info), - Ok(Err(e)) => { - failures += 1; - (Color::Red, format!("deep: error: {e}")) - } - Err(_) => { - failures += 1; - (Color::Red, "deep: error: timeout (90s)".to_string()) - } - }; - - let mut row = model_row(info, use_color); - row.push( - deep_status - .cell() - .foreground_color(color_if(use_color, deep_color)), - ); - rows.push(row); - } - } - } else { - eprint!("Testing {}...", info.id); - let params = GenerateParams::new(&info.id) - .provider(&info.provider) - .prompt("Say OK") - .max_tokens(16); - - let result = - tokio::time::timeout(Duration::from_secs(30), generate::generate(params)).await; - eprintln!(" done"); - - let (result_color, status) = match result { - Ok(Ok(_)) => (Color::Green, "ok".to_string()), - Ok(Err(e)) => { - failures += 1; - (Color::Red, format!("error: {e}")) - } - Err(_) => { - failures += 1; - (Color::Red, "error: timeout (30s)".to_string()) - } - }; - - let mut row = model_row(info, use_color); - row.push( - status - .cell() - .foreground_color(color_if(use_color, result_color)), - ); - rows.push(row); + for (idx, result_color, status) in &sorted_results { + if *result_color == Color::Red { + failures += 1; } + let mut row = model_row(&models_to_test[*idx], use_color); + row.push( + status + .cell() + .foreground_color(color_if(use_color, *result_color)), + ); + rows.push(row); } let table = rows From 35cac9185b0b59687f93fb5ff72ac1155a158fb6 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 21 Mar 2026 11:37:30 -0400 Subject: [PATCH 5/8] Fix OpenAI provider: always set store: true store: !codex_mode was sending store: false for non-Codex models, which prevented reasoning items from being persisted. This broke multi-turn conversations where reasoning items from turn 1 need to be sent back in turn 2. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-llm/src/providers/openai.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/crates/fabro-llm/src/providers/openai.rs b/lib/crates/fabro-llm/src/providers/openai.rs index 5d4bcccdd..ff1eb60e2 100644 --- a/lib/crates/fabro-llm/src/providers/openai.rs +++ b/lib/crates/fabro-llm/src/providers/openai.rs @@ -414,7 +414,7 @@ fn build_api_request(request: &Request, stream: bool, codex_mode: bool) -> ApiRe text, stop: request.stop_sequences.clone(), metadata: request.metadata.clone(), - store: !codex_mode, + store: true, stream, } } From 025316500db482baa02adc9cf9386b30e85f6437 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 21 Mar 2026 11:44:15 -0400 Subject: [PATCH 6/8] Fix Sonnet 4.5 effort parameter error by converting to thinking API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit claude-sonnet-4-5 doesn't support output_config.effort — it needs the older thinking API with budget_tokens. Add an `effort` feature flag to ModelFeatures and have the Anthropic adapter auto-convert reasoning_effort to a thinking config for models that lack effort support. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../fabro-llm/src/providers/anthropic.rs | 60 +++++++++++++++---- lib/crates/fabro-model/src/catalog.json | 30 +++++----- lib/crates/fabro-model/src/types.rs | 6 ++ 3 files changed, 70 insertions(+), 26 deletions(-) diff --git a/lib/crates/fabro-llm/src/providers/anthropic.rs b/lib/crates/fabro-llm/src/providers/anthropic.rs index b96b8decc..0ed91b0b5 100644 --- a/lib/crates/fabro-llm/src/providers/anthropic.rs +++ b/lib/crates/fabro-llm/src/providers/anthropic.rs @@ -523,6 +523,18 @@ fn extract_thinking_config( .cloned() } +/// Map a reasoning effort level to a thinking `budget_tokens` value for models +/// that don't support the `output_config.effort` parameter (e.g. claude-sonnet-4-5). +fn effort_to_budget_tokens(effort: &str, max_tokens: i64) -> i64 { + let budget = match effort { + "low" => max_tokens / 4, + "high" => max_tokens * 3 / 4, + _ => max_tokens / 2, // "medium" or unknown + }; + // Anthropic requires budget_tokens >= 1024 + budget.max(1024) +} + fn is_auto_cache_enabled(provider_options: Option<&serde_json::Value>) -> bool { provider_options .and_then(|opts| opts.get("anthropic")) @@ -1072,22 +1084,48 @@ fn build_api_request( apply_cache_control_to_conversation_prefix(&mut api_messages); } - let thinking = extract_thinking_config(request.provider_options.as_ref()); + let explicit_thinking = extract_thinking_config(request.provider_options.as_ref()); - let output_config = request - .reasoning_effort - .as_ref() - .map(|effort| serde_json::json!({"effort": effort})); + // Check whether this model supports the `output_config.effort` parameter. + // Older reasoning models (e.g. claude-sonnet-4-5) need `thinking` with + // `budget_tokens` instead. + let model_info = fabro_model::get_model_info(&request.model); + let supports_effort = model_info.as_ref().is_none_or(|m| m.features.effort); + + let mut resolved_max_tokens = request + .max_tokens + .or_else(|| model_info.as_ref().and_then(|m| m.limits.max_output)) + .unwrap_or(65536); + + let (thinking, output_config) = if let Some(effort) = &request.reasoning_effort { + if supports_effort { + ( + explicit_thinking, + Some(serde_json::json!({"effort": effort})), + ) + } else if explicit_thinking.is_none() { + // Convert effort level to a thinking budget for models that don't + // support the effort parameter (e.g. claude-sonnet-4-5). + let budget = effort_to_budget_tokens(effort, resolved_max_tokens); + if resolved_max_tokens <= budget { + resolved_max_tokens = budget + 1024; + } + ( + Some(serde_json::json!({"type": "enabled", "budget_tokens": budget})), + None, + ) + } else { + // thinking already configured via provider_options; skip output_config + (explicit_thinking, None) + } + } else { + (explicit_thinking, None) + }; let api_request = ApiRequest { model: request.model.clone(), messages: api_messages, - max_tokens: request - .max_tokens - .or_else(|| { - fabro_model::get_model_info(&request.model).and_then(|m| m.limits.max_output) - }) - .unwrap_or(65536), + max_tokens: resolved_max_tokens, system: system_value, temperature: request.temperature, top_p: request.top_p, diff --git a/lib/crates/fabro-model/src/catalog.json b/lib/crates/fabro-model/src/catalog.json index f05f13019..64693b07b 100644 --- a/lib/crates/fabro-model/src/catalog.json +++ b/lib/crates/fabro-model/src/catalog.json @@ -6,7 +6,7 @@ "display_name": "Claude Opus 4.6", "limits": { "context_window": 1000000, "max_output": 128000 }, "training": "2025-08-01", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 15.0, "output_cost_per_mtok": 75.0, @@ -38,7 +38,7 @@ "display_name": "Claude Sonnet 4.6", "limits": { "context_window": 200000, "max_output": 64000 }, "training": "2025-08-01", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 3.0, "output_cost_per_mtok": 15.0, @@ -71,7 +71,7 @@ "display_name": "GPT-5.2", "limits": { "context_window": 1047576, "max_output": 128000 }, "training": "2025-08-31", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 1.75, "output_cost_per_mtok": 14.0, @@ -87,7 +87,7 @@ "display_name": "GPT-5 Mini", "limits": { "context_window": 1047576, "max_output": 128000 }, "training": "2025-08-31", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 0.25, "output_cost_per_mtok": 2.0, @@ -103,7 +103,7 @@ "display_name": "GPT-5.2 Codex", "limits": { "context_window": 1047576, "max_output": 128000 }, "training": "2025-08-31", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 1.75, "output_cost_per_mtok": 14.0, @@ -119,7 +119,7 @@ "display_name": "GPT-5.3 Codex", "limits": { "context_window": 1047576, "max_output": 128000 }, "training": "2025-08-31", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 1.75, "output_cost_per_mtok": 14.0, @@ -135,7 +135,7 @@ "display_name": "GPT-5.3 Codex Spark", "limits": { "context_window": 131072, "max_output": 128000 }, "training": "2025-08-31", - "features": { "tools": true, "vision": false, "reasoning": true }, + "features": { "tools": true, "vision": false, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": null, "output_cost_per_mtok": null, @@ -151,7 +151,7 @@ "display_name": "GPT-5.4", "limits": { "context_window": 1047576, "max_output": 128000 }, "training": "2025-08-31", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 2.5, "output_cost_per_mtok": 15.0, @@ -168,7 +168,7 @@ "display_name": "GPT-5.4 Pro", "limits": { "context_window": 1047576, "max_output": 128000 }, "training": "2025-08-31", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 30.0, "output_cost_per_mtok": 180.0, @@ -184,7 +184,7 @@ "display_name": "GPT-5.4 Mini", "limits": { "context_window": 400000, "max_output": 128000 }, "training": "2025-08-31", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 0.75, "output_cost_per_mtok": 4.50, @@ -200,7 +200,7 @@ "display_name": "Gemini 3.1 Pro (Preview)", "limits": { "context_window": 1048576, "max_output": 65536 }, "training": "2025-01-01", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 2.0, "output_cost_per_mtok": 12.0, @@ -217,7 +217,7 @@ "display_name": "Gemini 3.1 Pro Custom Tools (Preview)", "limits": { "context_window": 1048576, "max_output": 65536 }, "training": "2025-01-01", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 2.0, "output_cost_per_mtok": 12.0, @@ -233,7 +233,7 @@ "display_name": "Gemini 3 Flash (Preview)", "limits": { "context_window": 1048576, "max_output": 65536 }, "training": "2025-01-01", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 0.5, "output_cost_per_mtok": 3.0, @@ -249,7 +249,7 @@ "display_name": "Gemini 3.1 Flash Lite (Preview)", "limits": { "context_window": 1048576, "max_output": 65536 }, "training": "2025-01-01", - "features": { "tools": true, "vision": true, "reasoning": true }, + "features": { "tools": true, "vision": true, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 0.25, "output_cost_per_mtok": 1.5, @@ -316,7 +316,7 @@ "display_name": "Mercury 2", "limits": { "context_window": 131072, "max_output": 50000 }, "training": null, - "features": { "tools": true, "vision": false, "reasoning": true }, + "features": { "tools": true, "vision": false, "reasoning": true, "effort": true }, "costs": { "input_cost_per_mtok": 0.25, "output_cost_per_mtok": 0.75, diff --git a/lib/crates/fabro-model/src/types.rs b/lib/crates/fabro-model/src/types.rs index e5e8ef979..0e478b396 100644 --- a/lib/crates/fabro-model/src/types.rs +++ b/lib/crates/fabro-model/src/types.rs @@ -13,6 +13,12 @@ pub struct ModelFeatures { pub tools: bool, pub vision: bool, pub reasoning: bool, + /// Whether the model supports the `reasoning_effort` / `effort` parameter + /// directly (e.g. Anthropic `output_config.effort`, OpenAI `reasoning.effort`). + /// Models with `reasoning=true` but `effort=false` (e.g. claude-sonnet-4-5) + /// need the older `thinking` API with `budget_tokens` instead. + #[serde(default)] + pub effort: bool, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] From f307c1fbdfb05504ebd954baabfd157772cd1d4b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 21 Mar 2026 11:46:01 -0400 Subject: [PATCH 7/8] Fix OpenAI reasoning round-trip: use store: false with encrypted_content The OpenAI Responses API requires store: false for non-Azure endpoints. Reasoning items round-trip correctly by requesting encrypted_content via the `include` field, which embeds them in the response payload rather than relying on server-side storage. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-llm/src/cli.rs | 21 ++++++++++---------- lib/crates/fabro-llm/src/providers/openai.rs | 15 +++++++++++++- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/lib/crates/fabro-llm/src/cli.rs b/lib/crates/fabro-llm/src/cli.rs index ffd605bdc..edb435f7f 100644 --- a/lib/crates/fabro-llm/src/cli.rs +++ b/lib/crates/fabro-llm/src/cli.rs @@ -882,7 +882,10 @@ fn validate_deep_result( ) -> (cli_table::Color, String) { // Check tool use: need at least 2 steps (tool call + follow-up) if result.steps.len() < 2 { - return (Color::Red, "deep: fail (model did not call tool)".to_string()); + return ( + Color::Red, + "deep: fail (model did not call tool)".to_string(), + ); } // Check that step 0 had tool results (tool was executed) @@ -1042,11 +1045,8 @@ async fn test_one_model(info: &ModelInfo, deep: bool) -> (Color, String) { match build_deep_test_params(info) { None => (Color::Yellow, "deep: skipped (no tool support)".to_string()), Some(params) => { - let result = tokio::time::timeout( - Duration::from_secs(90), - generate::generate(params), - ) - .await; + let result = + tokio::time::timeout(Duration::from_secs(90), generate::generate(params)).await; match result { Ok(Ok(ref gen_result)) => validate_deep_result(gen_result, info), Ok(Err(e)) => (Color::Red, format!("deep: error: {e}")), @@ -1094,16 +1094,15 @@ async fn test_models( let test_kind = if deep { "Deep testing" } else { "Testing" }; let pb = indicatif::ProgressBar::new(models_to_test.len() as u64); pb.set_style( - indicatif::ProgressStyle::with_template( - &format!("{{spinner:.green}} {test_kind} {{pos}}/{{len}} models {{wide_bar}} {{eta}}"), - ) + indicatif::ProgressStyle::with_template(&format!( + "{{spinner:.green}} {test_kind} {{pos}}/{{len}} models {{wide_bar}} {{eta}}" + )) .unwrap(), ); pb.enable_steady_tick(Duration::from_millis(100)); // Build (original_index, model_info) pairs, then shuffle for provider spread - let mut indexed: Vec<(usize, &ModelInfo)> = - models_to_test.iter().enumerate().collect(); + let mut indexed: Vec<(usize, &ModelInfo)> = models_to_test.iter().enumerate().collect(); indexed.shuffle(&mut rand::thread_rng()); // Run tests concurrently, 6 at a time diff --git a/lib/crates/fabro-llm/src/providers/openai.rs b/lib/crates/fabro-llm/src/providers/openai.rs index ff1eb60e2..95c5bffad 100644 --- a/lib/crates/fabro-llm/src/providers/openai.rs +++ b/lib/crates/fabro-llm/src/providers/openai.rs @@ -138,6 +138,8 @@ struct ApiRequest { #[serde(skip_serializing_if = "Option::is_none")] metadata: Option>, store: bool, + #[serde(skip_serializing_if = "Vec::is_empty")] + include: Vec, #[serde(skip_serializing_if = "std::ops::Not::not")] stream: bool, } @@ -391,6 +393,12 @@ fn build_api_request(request: &Request, stream: bool, codex_mode: bool) -> ApiRe .as_ref() .and_then(translate_response_format); + let include = if reasoning.is_some() { + vec!["reasoning.encrypted_content".to_string()] + } else { + Vec::new() + }; + let instructions = if codex_mode { Some(instructions.unwrap_or_default()) } else { @@ -414,7 +422,12 @@ fn build_api_request(request: &Request, stream: bool, codex_mode: bool) -> ApiRe text, stop: request.stop_sequences.clone(), metadata: request.metadata.clone(), - store: true, + // store: false is required for non-Azure OpenAI endpoints. Reasoning + // items still round-trip correctly because we request encrypted_content + // via the `include` field, which embeds them in the response payload + // rather than relying on server-side storage. + store: false, + include, stream, } } From 306f5b4b6cbe2dec42cd13125cba6a1bc24dfab8 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 21 Mar 2026 12:11:09 -0400 Subject: [PATCH 8/8] Update model catalog snapshots with new effort field Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-model/src/catalog.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/crates/fabro-model/src/catalog.rs b/lib/crates/fabro-model/src/catalog.rs index ae90f4425..525a0e6ad 100644 --- a/lib/crates/fabro-model/src/catalog.rs +++ b/lib/crates/fabro-model/src/catalog.rs @@ -270,6 +270,7 @@ mod tests { tools: true, vision: true, reasoning: true, + effort: true, }, costs: ModelCosts { input_cost_per_mtok: Some( @@ -349,6 +350,7 @@ mod tests { tools: true, vision: true, reasoning: true, + effort: true, }, costs: ModelCosts { input_cost_per_mtok: Some( @@ -402,6 +404,7 @@ mod tests { tools: true, vision: true, reasoning: false, + effort: false, }, costs: ModelCosts { input_cost_per_mtok: Some( @@ -460,6 +463,7 @@ mod tests { tools: true, vision: false, reasoning: true, + effort: true, }, costs: ModelCosts { input_cost_per_mtok: Some( @@ -508,6 +512,7 @@ mod tests { tools: true, vision: true, reasoning: true, + effort: true, }, costs: ModelCosts { input_cost_per_mtok: Some( @@ -554,6 +559,7 @@ mod tests { tools: true, vision: true, reasoning: true, + effort: true, }, costs: ModelCosts { input_cost_per_mtok: Some( @@ -620,6 +626,7 @@ mod tests { tools: true, vision: false, reasoning: true, + effort: true, }, costs: ModelCosts { input_cost_per_mtok: None,