From 2708e2eb5433ffaebf29b489c1d1472acc87b5ea Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 28 Mar 2026 14:57:48 -0400 Subject: [PATCH] Enable 7 additional pedantic clippy lints Enables char_lit_as_u8, collapsible_else_if, collapsible_if, map_unwrap_or, match_same_arms, used_underscore_binding, and if_not_else. Fixes all violations: combines duplicate match arms, renames underscore-prefixed bindings that are actually used, rewrites if-not-else patterns, and applies map_or where appropriate. Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.toml | 7 ---- lib/crates/fabro-agent/src/agent_profile.rs | 3 +- lib/crates/fabro-agent/src/cli.rs | 3 +- lib/crates/fabro-agent/src/truncation.rs | 7 +--- lib/crates/fabro-agent/src/types.rs | 10 ++--- lib/crates/fabro-api/src/demo/mod.rs | 18 ++++++--- lib/crates/fabro-cli/src/commands/doctor.rs | 8 ++-- .../fabro-cli/src/commands/preflight.rs | 38 ++++++++++--------- .../fabro-cli/src/commands/run/attach.rs | 11 ++---- lib/crates/fabro-cli/src/commands/run/logs.rs | 32 +--------------- .../fabro-cli/src/commands/run/output.rs | 4 +- .../src/commands/run/run_progress.rs | 24 +++++------- .../fabro-cli/src/commands/runs/list.rs | 3 +- .../fabro-cli/src/commands/workflow/list.rs | 3 +- lib/crates/fabro-cli/src/logging.rs | 5 +-- lib/crates/fabro-core/src/context.rs | 3 +- lib/crates/fabro-devcontainer/src/lib.rs | 15 ++++---- lib/crates/fabro-hooks/src/executor.rs | 2 +- lib/crates/fabro-llm/src/cli.rs | 3 -- lib/crates/fabro-llm/src/providers/openai.rs | 7 +--- lib/crates/fabro-sandbox/src/daytona/mod.rs | 15 +++----- lib/crates/fabro-sandbox/src/ssh/mod.rs | 7 ++-- lib/crates/fabro-types/src/outcome.rs | 24 +++++------- lib/crates/fabro-types/src/settings/mod.rs | 3 +- lib/crates/fabro-util/src/text.rs | 2 +- lib/crates/fabro-workflows/src/event.rs | 3 +- .../fabro-workflows/src/handler/llm/api.rs | 6 +-- .../fabro-workflows/src/handler/llm/cli.rs | 11 +++--- lib/crates/fabro-workflows/src/lib.rs | 4 +- .../fabro-workflows/src/lifecycle/event.rs | 6 +-- .../fabro-workflows/src/lifecycle/hook.rs | 3 +- .../fabro-workflows/src/operations/create.rs | 10 ++--- .../fabro-workflows/src/operations/rewind.rs | 12 +++--- .../fabro-workflows/src/pipeline/finalize.rs | 6 +-- .../src/pipeline/initialize.rs | 5 ++- .../src/pipeline/pull_request.rs | 3 +- lib/crates/fabro-workflows/src/run_lookup.rs | 3 +- lib/crates/fabro-workflows/src/run_options.rs | 3 +- 38 files changed, 130 insertions(+), 202 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ba5cd951e..70c4566b5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,12 +72,7 @@ unreachable_pub = "warn" [workspace.lints.clippy] pedantic = { level = "warn", priority = -2 } # Allowed pedantic lints -char_lit_as_u8 = "allow" -collapsible_else_if = "allow" -collapsible_if = "allow" implicit_hasher = "allow" -map_unwrap_or = "allow" -match_same_arms = "allow" missing_errors_doc = "allow" missing_panics_doc = "allow" module_name_repetitions = "allow" @@ -86,8 +81,6 @@ similar_names = "allow" struct_excessive_bools = "allow" too_many_arguments = "allow" too_many_lines = "allow" -used_underscore_binding = "allow" -if_not_else = "allow" cast_precision_loss = "allow" doc_markdown = "allow" # Disallowed restriction lints diff --git a/lib/crates/fabro-agent/src/agent_profile.rs b/lib/crates/fabro-agent/src/agent_profile.rs index 3bf3c9f51..de869e34c 100644 --- a/lib/crates/fabro-agent/src/agent_profile.rs +++ b/lib/crates/fabro-agent/src/agent_profile.rs @@ -38,8 +38,7 @@ pub trait AgentProfile: Send + Sync { fn context_window_size(&self) -> usize { Catalog::builtin() .get(self.model()) - .map(|m| usize::try_from(m.context_window()).unwrap()) - .unwrap_or(200_000) + .map_or(200_000, |m| usize::try_from(m.context_window()).unwrap()) } fn register_subagent_tools( diff --git a/lib/crates/fabro-agent/src/cli.rs b/lib/crates/fabro-agent/src/cli.rs index f12edb36b..51308c1c5 100644 --- a/lib/crates/fabro-agent/src/cli.rs +++ b/lib/crates/fabro-agent/src/cli.rs @@ -170,14 +170,13 @@ fn summarizer_model_id(provider: Provider) -> ModelRef { ModelRef::ByName { provider, model: match provider { - Provider::OpenAi => "gpt-4o-mini", + Provider::OpenAi | Provider::OpenAiCompatible => "gpt-4o-mini", Provider::Gemini => "gemini-2.0-flash", Provider::Anthropic => "claude-haiku-4-5", Provider::Kimi => "kimi-k2.5", Provider::Zai => "glm-4.7", Provider::Minimax => "minimax-m2.5", Provider::Inception => "mercury", - Provider::OpenAiCompatible => "gpt-4o-mini", } .to_string(), } diff --git a/lib/crates/fabro-agent/src/truncation.rs b/lib/crates/fabro-agent/src/truncation.rs index f0e1ec67c..8ed70ec7c 100644 --- a/lib/crates/fabro-agent/src/truncation.rs +++ b/lib/crates/fabro-agent/src/truncation.rs @@ -24,12 +24,9 @@ fn default_char_limit(tool_name: &str) -> Option { match tool_name { "read_file" => Some(50_000), "shell" => Some(30_000), - "grep" => Some(20_000), - "glob" => Some(20_000), - "edit_file" => Some(10_000), + "grep" | "glob" | "spawn_agent" => Some(20_000), + "edit_file" | "apply_patch" => Some(10_000), "write_file" => Some(1_000), - "apply_patch" => Some(10_000), - "spawn_agent" => Some(20_000), _ => None, } } diff --git a/lib/crates/fabro-agent/src/types.rs b/lib/crates/fabro-agent/src/types.rs index cc0e00e08..9c8c6de74 100644 --- a/lib/crates/fabro-agent/src/types.rs +++ b/lib/crates/fabro-agent/src/types.rs @@ -219,7 +219,6 @@ impl AgentEvent { Self::AssistantTextStart => { debug!(session_id, "Assistant response started"); } - Self::AssistantOutputReplace { .. } => {} Self::AssistantMessage { model, usage, @@ -235,8 +234,11 @@ impl AgentEvent { "Assistant message" ); } - Self::TextDelta { .. } => {} - Self::ReasoningDelta { .. } => {} + Self::TextDelta { .. } + | Self::ReasoningDelta { .. } + | Self::AssistantOutputReplace { .. } + | Self::ToolCallOutputDelta { .. } + | Self::SubAgentEvent { .. } => {} Self::ToolCallStarted { tool_name, tool_call_id, @@ -249,7 +251,6 @@ impl AgentEvent { "Tool call started" ); } - Self::ToolCallOutputDelta { .. } => {} Self::ToolCallCompleted { tool_name, tool_call_id, @@ -362,7 +363,6 @@ impl AgentEvent { Self::SubAgentClosed { agent_id, depth } => { debug!(session_id, agent_id, depth, "Sub-agent closed"); } - Self::SubAgentEvent { .. } => {} Self::McpServerReady { server_name, tool_count, diff --git a/lib/crates/fabro-api/src/demo/mod.rs b/lib/crates/fabro-api/src/demo/mod.rs index a5175435c..164a7f7a7 100644 --- a/lib/crates/fabro-api/src/demo/mod.rs +++ b/lib/crates/fabro-api/src/demo/mod.rs @@ -198,25 +198,31 @@ pub(crate) async fn context_stub( pub(crate) async fn cancel_stub( _auth: AuthenticatedService, State(_state): State>, - Path(_id): Path, + Path(id): Path, ) -> Response { - (StatusCode::OK, Json(serde_json::json!({"id": _id, "status": "cancelled", "created_at": "2026-03-06T14:30:00Z"}))).into_response() + (StatusCode::OK, Json(serde_json::json!({"id": id, "status": "cancelled", "created_at": "2026-03-06T14:30:00Z"}))).into_response() } pub(crate) async fn pause_stub( _auth: AuthenticatedService, State(_state): State>, - Path(_id): Path, + Path(id): Path, ) -> Response { - (StatusCode::OK, Json(serde_json::json!({"id": _id, "status": "paused", "created_at": "2026-03-06T14:30:00Z"}))).into_response() + ( + StatusCode::OK, + Json( + serde_json::json!({"id": id, "status": "paused", "created_at": "2026-03-06T14:30:00Z"}), + ), + ) + .into_response() } pub(crate) async fn unpause_stub( _auth: AuthenticatedService, State(_state): State>, - Path(_id): Path, + Path(id): Path, ) -> Response { - (StatusCode::OK, Json(serde_json::json!({"id": _id, "status": "running", "created_at": "2026-03-06T14:30:00Z"}))).into_response() + (StatusCode::OK, Json(serde_json::json!({"id": id, "status": "running", "created_at": "2026-03-06T14:30:00Z"}))).into_response() } pub(crate) async fn get_run_graph( diff --git a/lib/crates/fabro-cli/src/commands/doctor.rs b/lib/crates/fabro-cli/src/commands/doctor.rs index 6a2833b80..1df1321d0 100644 --- a/lib/crates/fabro-cli/src/commands/doctor.rs +++ b/lib/crates/fabro-cli/src/commands/doctor.rs @@ -866,10 +866,10 @@ async fn probe_daytona() -> Option> { } pub(crate) fn probe_model(provider: Provider) -> String { - Catalog::builtin() - .probe_for_provider(provider) - .map(|m| m.id.clone()) - .unwrap_or_else(|| format!("unknown-{}", provider.as_str())) + Catalog::builtin().probe_for_provider(provider).map_or_else( + || format!("unknown-{}", provider.as_str()), + |m| m.id.clone(), + ) } async fn probe_llm_provider( diff --git a/lib/crates/fabro-cli/src/commands/preflight.rs b/lib/crates/fabro-cli/src/commands/preflight.rs index 9c534d788..c449f51c6 100644 --- a/lib/crates/fabro-cli/src/commands/preflight.rs +++ b/lib/crates/fabro-cli/src/commands/preflight.rs @@ -97,16 +97,18 @@ fn resolve_model_provider( let model = cli_model .or(configured_model) .or_else(|| graph.attrs.get("default_model").and_then(|v| v.as_str())) - .map(String::from) - .unwrap_or_else(|| { - let catalog = Catalog::builtin(); - let info = provider - .as_deref() - .and_then(|s| s.parse::().ok()) - .and_then(|p| catalog.default_for_provider(p)) - .unwrap_or_else(|| catalog.default_from_env()); - info.id.clone() - }); + .map_or_else( + || { + let catalog = Catalog::builtin(); + let info = provider + .as_deref() + .and_then(|s| s.parse::().ok()) + .and_then(|p| catalog.default_for_provider(p)) + .unwrap_or_else(|| catalog.default_from_env()); + info.id.clone() + }, + String::from, + ); match Catalog::builtin().get(&model) { Some(info) => ( @@ -233,14 +235,16 @@ async fn run_preflight( let mut checks: Vec = Vec::new(); let setup_command_count = settings.setup_commands().len(); - let repo_summary = origin_url - .map(|url| { + let repo_summary = origin_url.map_or_else( + || "unknown".into(), + |url| { let https = fabro_github::ssh_url_to_https(url); - fabro_github::parse_github_owner_repo(&https) - .map(|(owner, repo)| format!("{owner}/{repo}")) - .unwrap_or_else(|_| url.to_string()) - }) - .unwrap_or_else(|| "unknown".into()); + fabro_github::parse_github_owner_repo(&https).map_or_else( + |_| url.to_string(), + |(owner, repo)| format!("{owner}/{repo}"), + ) + }, + ); checks.push(CheckResult { name: "Repository".into(), diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index 4fb507dba..1b8ed5098 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -138,8 +138,7 @@ pub(crate) async fn attach_run( for _ in 0..20 { if conclusion_path.exists() || read_status_record(&status_path) - .map(|record| record.status.is_terminal()) - .unwrap_or(false) + .is_some_and(|record| record.status.is_terminal()) { break; } @@ -214,9 +213,8 @@ pub(crate) async fn attach_run( let child_alive_via_handle = engine_guard.as_mut().and_then(|guard| { guard.inner().map(|child| match child.try_wait() { - Ok(Some(_)) => false, // child exited - Ok(None) => true, // still running - Err(_) => false, // error, treat as dead + Ok(None) => true, // still running + Ok(Some(_)) | Err(_) => false, // exited or error }) }); @@ -269,14 +267,13 @@ fn drain_remaining( loop { line.clear(); match reader.read_line(line) { - Ok(0) => break, + Ok(0) | Err(_) => break, Ok(_) => { let trimmed = line.trim(); if !trimmed.is_empty() { progress_ui.handle_json_line(trimmed); } } - Err(_) => break, } } } diff --git a/lib/crates/fabro-cli/src/commands/run/logs.rs b/lib/crates/fabro-cli/src/commands/run/logs.rs index b19ae6c1b..cb0108e7e 100644 --- a/lib/crates/fabro-cli/src/commands/run/logs.rs +++ b/lib/crates/fabro-cli/src/commands/run/logs.rs @@ -80,11 +80,7 @@ fn apply_filters( let filtered: Vec = match since { Some(cutoff) => lines .iter() - .filter(|line| { - extract_timestamp(line) - .map(|ts| ts >= *cutoff) - .unwrap_or(true) - }) + .filter(|line| extract_timestamp(line).is_none_or(|ts| ts >= *cutoff)) .cloned() .collect(), None => lines.to_vec(), @@ -556,29 +552,6 @@ pub(crate) fn format_event_pretty(line: &str, styles: &Styles) -> Option styles.dim.apply_to(&ts), styles.bold_cyan.apply_to("\u{25b6}"), )), - "Agent.SessionStarted" - | "Agent.SessionEnded" - | "Agent.AssistantTextStart" - | "Agent.AssistantOutputReplace" - | "Agent.TextDelta" - | "Agent.ReasoningDelta" - | "Agent.ToolCallOutputDelta" - | "Sandbox.Initializing" - | "Sandbox.Pulling" - | "Sandbox.Creating" - | "SetupStarted" - | "SetupCommandStarted" - | "SetupCommandCompleted" - | "CheckpointCompleted" - | "CheckpointFailed" - | "GitCommit" - | "GitPush" - | "GitBranch" - | "GitWorktreeAdd" - | "GitWorktreeRemove" - | "GitFetch" - | "GitReset" - | "AssetsCaptured" => None, _ => None, } } @@ -589,8 +562,7 @@ fn str_field<'a>(value: &'a serde_json::Value, key: &str) -> Option<&'a str> { fn format_timestamp(ts: &str) -> String { ts.parse::>() - .map(|dt| dt.format("%H:%M:%S").to_string()) - .unwrap_or_else(|_| ts.to_string()) + .map_or_else(|_| ts.to_string(), |dt| dt.format("%H:%M:%S").to_string()) } fn format_duration_ms(value: Option<&serde_json::Value>) -> String { diff --git a/lib/crates/fabro-cli/src/commands/run/output.rs b/lib/crates/fabro-cli/src/commands/run/output.rs index d5a22fac2..f6ffe554e 100644 --- a/lib/crates/fabro-cli/src/commands/run/output.rs +++ b/lib/crates/fabro-cli/src/commands/run/output.rs @@ -30,9 +30,7 @@ fn print_workflow_header( graph.edges.len() )), ); - let graph_path = dot_path - .map(relative_path) - .unwrap_or_else(|| "".to_string()); + let graph_path = dot_path.map_or_else(|| "".to_string(), relative_path); eprintln!( "{} {}", styles.dim.apply_to("Graph:"), diff --git a/lib/crates/fabro-cli/src/commands/run/run_progress.rs b/lib/crates/fabro-cli/src/commands/run/run_progress.rs index c29f11b62..57cfbf066 100644 --- a/lib/crates/fabro-cli/src/commands/run/run_progress.rs +++ b/lib/crates/fabro-cli/src/commands/run/run_progress.rs @@ -244,10 +244,8 @@ impl ProgressUI { "bash" | "shell" | "execute_command" => arg("command").map(|c| truncate(c, 60)), "glob" => arg("pattern").map(String::from), "grep" | "ripgrep" => arg("pattern").map(|p| truncate(p, 40)), - "read_file" | "read" => path_arg(), - "write_file" | "write" | "create_file" => path_arg(), - "edit_file" | "edit" => path_arg(), - "list_dir" => path_arg(), + "read_file" | "read" | "write_file" | "write" | "create_file" | "edit_file" + | "edit" | "list_dir" => path_arg(), "web_search" => arg("query").map(|q| truncate(q, 60)), "web_fetch" => arg("url").map(|u| truncate(u, 60)), "spawn_agent" => arg("task").map(|t| truncate(t, 60)), @@ -371,8 +369,7 @@ impl ProgressUI { let tool_call_count = counts.map_or(0, |c| c.1); let total_tokens = usage .as_ref() - .map(|u| u.input_tokens + u.output_tokens) - .unwrap_or(0); + .map_or(0, |u| u.input_tokens + u.output_tokens); if turn_count > 0 || tool_call_count > 0 || total_tokens > 0 { let dim = Style::new().dim(); format!( @@ -754,17 +751,14 @@ impl ProgressUI { let counts = self.stage_counts.get(node_id); let turn_count = counts.map_or(0, |c| c.0); let tool_call_count = counts.map_or(0, |c| c.1); - let total_tokens = envelope - .get("usage") - .map(|u| { - u.get("input_tokens") + let total_tokens = envelope.get("usage").map_or(0, |u| { + u.get("input_tokens") + .and_then(serde_json::Value::as_i64) + .unwrap_or(0) + + u.get("output_tokens") .and_then(serde_json::Value::as_i64) .unwrap_or(0) - + u.get("output_tokens") - .and_then(serde_json::Value::as_i64) - .unwrap_or(0) - }) - .unwrap_or(0); + }); if turn_count > 0 || tool_call_count > 0 || total_tokens > 0 { let dim = Style::new().dim(); format!( diff --git a/lib/crates/fabro-cli/src/commands/runs/list.rs b/lib/crates/fabro-cli/src/commands/runs/list.rs index 7ed6368c7..81cc67490 100644 --- a/lib/crates/fabro-cli/src/commands/runs/list.rs +++ b/lib/crates/fabro-cli/src/commands/runs/list.rs @@ -88,8 +88,7 @@ pub(crate) fn list_command(args: &RunsListArgs, styles: &Styles) -> Result<()> { let dir_display = run .host_repo_path .as_deref() - .map(|p| tilde_path(Path::new(p))) - .unwrap_or_else(|| "-".to_string()); + .map_or_else(|| "-".to_string(), |p| tilde_path(Path::new(p))); vec![ short_run_id(&run.run_id) diff --git a/lib/crates/fabro-cli/src/commands/workflow/list.rs b/lib/crates/fabro-cli/src/commands/workflow/list.rs index 30986c120..9d40faaf0 100644 --- a/lib/crates/fabro-cli/src/commands/workflow/list.rs +++ b/lib/crates/fabro-cli/src/commands/workflow/list.rs @@ -46,8 +46,7 @@ pub(super) fn list_command(_args: &WorkflowListArgs) -> Result<()> { let user_path = user_wf_dir .as_deref() - .map(relative_path) - .unwrap_or_else(|| "~/.fabro/workflows".to_string()); + .map_or_else(|| "~/.fabro/workflows".to_string(), relative_path); print_section("User Workflows", &user_path, &user, name_width, &styles); eprintln!(); diff --git a/lib/crates/fabro-cli/src/logging.rs b/lib/crates/fabro-cli/src/logging.rs index c65aa6567..a5eca12d3 100644 --- a/lib/crates/fabro-cli/src/logging.rs +++ b/lib/crates/fabro-cli/src/logging.rs @@ -16,9 +16,8 @@ pub(crate) fn init_tracing( let filter = EnvFilter::try_from_env("FABRO_LOG").unwrap_or_else(|_| EnvFilter::new(default_level)); - let log_dir = dirs::home_dir() - .map(|h| h.join(".fabro").join("logs")) - .unwrap_or_else(|| ".fabro/logs".into()); + let log_dir = + dirs::home_dir().map_or_else(|| ".fabro/logs".into(), |h| h.join(".fabro").join("logs")); std::fs::create_dir_all(&log_dir) .with_context(|| format!("Failed to create log directory: {}", log_dir.display()))?; diff --git a/lib/crates/fabro-core/src/context.rs b/lib/crates/fabro-core/src/context.rs index 8e34f91b1..ce75b0a90 100644 --- a/lib/crates/fabro-core/src/context.rs +++ b/lib/crates/fabro-core/src/context.rs @@ -65,8 +65,7 @@ impl Context { pub fn node_visit_count(&self) -> usize { self.get("internal.node_visit_count") .and_then(|v| v.as_u64()) - .map(|v| usize::try_from(v).unwrap()) - .unwrap_or(0) + .map_or(0, |v| usize::try_from(v).unwrap()) } } diff --git a/lib/crates/fabro-devcontainer/src/lib.rs b/lib/crates/fabro-devcontainer/src/lib.rs index 7564cfc7c..ee4c9dd49 100644 --- a/lib/crates/fabro-devcontainer/src/lib.rs +++ b/lib/crates/fabro-devcontainer/src/lib.rs @@ -291,11 +291,10 @@ impl DevcontainerResolver { // Image or Dockerfile mode let (base_dockerfile, build_context, build_args, build_target) = if let Some(build) = &devcontainer.build { - let context_dir = build - .context - .as_ref() - .map(|c| base_dir.join(variables::substitute(c, &vars))) - .unwrap_or_else(|| base_dir.to_path_buf()); + let context_dir = build.context.as_ref().map_or_else( + || base_dir.to_path_buf(), + |c| base_dir.join(variables::substitute(c, &vars)), + ); let df_path = base_dir.join(variables::substitute( build.dockerfile.as_deref().unwrap_or("Dockerfile"), &vars, @@ -331,15 +330,15 @@ impl DevcontainerResolver { }; // Features - let resolved_features = if !devcontainer.features.is_empty() { + let resolved_features = if devcontainer.features.is_empty() { + features::ResolvedFeatures::default() + } else { features::resolve_features( &devcontainer.features, base_dir, devcontainer.remote_user.as_deref(), ) .await? - } else { - features::ResolvedFeatures::default() }; // Merge feature containerEnv with devcontainer.json containerEnv diff --git a/lib/crates/fabro-hooks/src/executor.rs b/lib/crates/fabro-hooks/src/executor.rs index 2d0b0054f..9272a5403 100644 --- a/lib/crates/fabro-hooks/src/executor.rs +++ b/lib/crates/fabro-hooks/src/executor.rs @@ -220,7 +220,7 @@ impl HookExecutorImpl { /// Resolve a model alias (e.g. "haiku") to a concrete model ID. fn resolve_model(model: Option<&String>) -> String { - let model_id = model.map(String::as_str).unwrap_or("haiku"); + let model_id = model.map_or("haiku", String::as_str); let model_info = fabro_model::Catalog::builtin().get(model_id); model_info.map_or(model_id, |m| m.id.as_str()).to_string() } diff --git a/lib/crates/fabro-llm/src/cli.rs b/lib/crates/fabro-llm/src/cli.rs index cf065119a..c8d6b76fd 100644 --- a/lib/crates/fabro-llm/src/cli.rs +++ b/lib/crates/fabro-llm/src/cli.rs @@ -626,9 +626,6 @@ async fn stream_session_text(response: reqwest::Response) -> Result<()> { } } } - "assistant_turn" => { - // Text already printed via content_delta events - } "done" => { println!(); return Ok(false); diff --git a/lib/crates/fabro-llm/src/providers/openai.rs b/lib/crates/fabro-llm/src/providers/openai.rs index 056aca70a..144acccbf 100644 --- a/lib/crates/fabro-llm/src/providers/openai.rs +++ b/lib/crates/fabro-llm/src/providers/openai.rs @@ -267,9 +267,6 @@ fn translate_input(messages: &[Message]) -> (Option, Vec { - // Skip — using preserved opaque message item instead - } ContentPart::ToolCall(tc) if !tc.name.is_empty() => { let args = tc .raw_arguments @@ -653,9 +650,7 @@ fn process_sse_event( }); } } - "response.reasoning_summary_part.added" => { - // Recognized but no-op — ReasoningStart is emitted on the first delta instead. - } + // response.reasoning_summary_part.added and other unrecognized events are no-ops _ => {} } diff --git a/lib/crates/fabro-sandbox/src/daytona/mod.rs b/lib/crates/fabro-sandbox/src/daytona/mod.rs index 8e69b985d..ad79f6657 100644 --- a/lib/crates/fabro-sandbox/src/daytona/mod.rs +++ b/lib/crates/fabro-sandbox/src/daytona/mod.rs @@ -927,9 +927,8 @@ impl Sandbox for DaytonaSandbox { let sandbox = self.sandbox()?; let start = Instant::now(); - let cwd = working_dir - .map(|d| self.resolve_path(d)) - .unwrap_or_else(|| WORKING_DIRECTORY.to_string()); + let cwd = + working_dir.map_or_else(|| WORKING_DIRECTORY.to_string(), |d| self.resolve_path(d)); let process_svc = sandbox .process() @@ -952,14 +951,14 @@ impl Sandbox for DaytonaSandbox { // prepend `export` statements as a fallback until server support // lands. The SDK sends `envs` too for forward compatibility. let command_with_env = if let Some(vars) = env_vars { - if !vars.is_empty() { + if vars.is_empty() { + command.to_string() + } else { let exports: Vec = vars .iter() .map(|(k, v)| format!("export {}={}", shell_quote(k), shell_quote(v))) .collect(); format!("{}\n{}", exports.join("\n"), command) - } else { - command.to_string() } } else { command.to_string() @@ -1098,9 +1097,7 @@ impl Sandbox for DaytonaSandbox { } async fn glob(&self, pattern: &str, path: Option<&str>) -> Result, String> { - let base = path - .map(|p| self.resolve_path(p)) - .unwrap_or_else(|| WORKING_DIRECTORY.to_string()); + let base = path.map_or_else(|| WORKING_DIRECTORY.to_string(), |p| self.resolve_path(p)); let cmd = format!( "find {} -name {} -type f | sort", diff --git a/lib/crates/fabro-sandbox/src/ssh/mod.rs b/lib/crates/fabro-sandbox/src/ssh/mod.rs index b9b62c34a..425d5fa17 100644 --- a/lib/crates/fabro-sandbox/src/ssh/mod.rs +++ b/lib/crates/fabro-sandbox/src/ssh/mod.rs @@ -448,9 +448,10 @@ impl Sandbox for SshSandbox { } async fn glob(&self, pattern: &str, path: Option<&str>) -> Result, String> { - let base = path - .map(|p| self.resolve_path(p)) - .unwrap_or_else(|| self.config.working_directory.clone()); + let base = path.map_or_else( + || self.config.working_directory.clone(), + |p| self.resolve_path(p), + ); let cmd = format!( "find {} -name {} -type f | sort", diff --git a/lib/crates/fabro-types/src/outcome.rs b/lib/crates/fabro-types/src/outcome.rs index 405fc5706..3a59e1c04 100644 --- a/lib/crates/fabro-types/src/outcome.rs +++ b/lib/crates/fabro-types/src/outcome.rs @@ -85,13 +85,8 @@ impl FromStr for FailureCategory { fn from_str(s: &str) -> std::result::Result { let normalized = s.trim().to_lowercase(); Ok(match normalized.as_str() { - "transient_infra" => Self::TransientInfra, - "deterministic" => Self::Deterministic, - "budget_exhausted" => Self::BudgetExhausted, - "compilation_loop" => Self::CompilationLoop, - "canceled" => Self::Canceled, - "structural" => Self::Structural, - "transient" + "transient_infra" + | "transient" | "transient-infra" | "infra_transient" | "transient infra" @@ -101,15 +96,16 @@ impl FromStr for FailureCategory { | "toolchain-workspace-io" | "toolchain_or_dependency_registry_unavailable" | "toolchain-dependency-registry-unavailable" => Self::TransientInfra, - "non_transient" | "non-transient" | "permanent" | "logic" | "product" => { - Self::Deterministic + "budget_exhausted" | "budget-exhausted" | "budget exhausted" | "budget" => { + Self::BudgetExhausted } - "cancelled" => Self::Canceled, - "budget-exhausted" | "budget exhausted" | "budget" => Self::BudgetExhausted, - "compilation-loop" | "compilation loop" | "compile_loop" | "compile-loop" => { - Self::CompilationLoop + "compilation_loop" | "compilation-loop" | "compilation loop" | "compile_loop" + | "compile-loop" => Self::CompilationLoop, + "canceled" | "cancelled" => Self::Canceled, + "structural" | "structure" | "scope_violation" | "write_scope_violation" => { + Self::Structural } - "structure" | "scope_violation" | "write_scope_violation" => Self::Structural, + // "deterministic" and all unrecognized values _ => Self::Deterministic, }) } diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index 287f6d921..2c1f83d8d 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -138,8 +138,7 @@ impl FabroSettings { pub fn setup_commands(&self) -> &[String] { self.setup .as_ref() - .map(|setup| setup.commands.as_slice()) - .unwrap_or(&[]) + .map_or(&[], |setup| setup.commands.as_slice()) } pub fn setup_timeout_ms(&self) -> Option { diff --git a/lib/crates/fabro-util/src/text.rs b/lib/crates/fabro-util/src/text.rs index 262d00fc8..0f817a3d4 100644 --- a/lib/crates/fabro-util/src/text.rs +++ b/lib/crates/fabro-util/src/text.rs @@ -5,7 +5,7 @@ pub fn strip_goal_decoration(goal: &str) -> &str { let line = goal.lines().next().unwrap_or(""); let line = line.trim_start_matches('#').trim(); - line.strip_prefix("Plan:").map(str::trim).unwrap_or(line) + line.strip_prefix("Plan:").map_or(line, str::trim) } #[cfg(test)] diff --git a/lib/crates/fabro-workflows/src/event.rs b/lib/crates/fabro-workflows/src/event.rs index d5fa8fac8..7970e9db1 100644 --- a/lib/crates/fabro-workflows/src/event.rs +++ b/lib/crates/fabro-workflows/src/event.rs @@ -563,8 +563,7 @@ impl WorkflowRunEvent { Self::Prompt { stage, text } => { debug!(stage, text_len = text.len(), "Prompt sent"); } - Self::Agent { .. } => {} - Self::Sandbox { .. } => {} + Self::Agent { .. } | Self::Sandbox { .. } => {} Self::SandboxInitialized { working_directory, .. } => { diff --git a/lib/crates/fabro-workflows/src/handler/llm/api.rs b/lib/crates/fabro-workflows/src/handler/llm/api.rs index cd02fb1d3..d81328bb7 100644 --- a/lib/crates/fabro-workflows/src/handler/llm/api.rs +++ b/lib/crates/fabro-workflows/src/handler/llm/api.rs @@ -68,13 +68,11 @@ fn track_file_event(event: &AgentEvent, state: &mut FileTracking) { is_error, .. } => { - if !*is_error { - if let Some(path) = state.pending.remove(tool_call_id) { + if let Some(path) = state.pending.remove(tool_call_id) { + if !*is_error { state.touched.insert(path.clone()); state.last = Some(path); } - } else { - state.pending.remove(tool_call_id); } } AgentEvent::SubAgentEvent { event: inner, .. } => { diff --git a/lib/crates/fabro-workflows/src/handler/llm/cli.rs b/lib/crates/fabro-workflows/src/handler/llm/cli.rs index 57085c378..87b90eb5b 100644 --- a/lib/crates/fabro-workflows/src/handler/llm/cli.rs +++ b/lib/crates/fabro-workflows/src/handler/llm/cli.rs @@ -333,7 +333,7 @@ fn parse_gemini_json(output: &str) -> Option { .pointer("/stats/models") .and_then(|m| m.as_object()) .and_then(|models| models.values().next()) - .map(|model_stats| { + .map_or((0, 0), |model_stats| { let input = model_stats .pointer("/tokens/input") .and_then(serde_json::Value::as_i64) @@ -343,8 +343,7 @@ fn parse_gemini_json(output: &str) -> Option { .and_then(serde_json::Value::as_i64) .unwrap_or(0); (input, output) - }) - .unwrap_or((0, 0)); + }); Some(CliResponse { text, @@ -691,7 +690,9 @@ impl CodergenBackend for AgentCliBackend { .collect(); // Find the most recently modified file by mtime - let last_file_touched = if !files_touched.is_empty() { + let last_file_touched = if files_touched.is_empty() { + None + } else { let quoted_files: Vec = files_touched .iter() .filter_map(|f| shlex::try_quote(f).ok().map(std::borrow::Cow::into_owned)) @@ -707,8 +708,6 @@ impl CodergenBackend for AgentCliBackend { } else { None } - } else { - None }; let mut stage_usage = StageUsage { diff --git a/lib/crates/fabro-workflows/src/lib.rs b/lib/crates/fabro-workflows/src/lib.rs index a44eecb5a..f134ab282 100644 --- a/lib/crates/fabro-workflows/src/lib.rs +++ b/lib/crates/fabro-workflows/src/lib.rs @@ -40,9 +40,7 @@ pub fn build_completed_stages(cp: &records::Checkpoint, run_failed: bool) -> Vec let outcome = cp.node_outcomes.get(node_id); let retries = cp.node_retries.get(node_id).copied().unwrap_or(0); - let status = outcome - .map(|o| o.status.to_string()) - .unwrap_or_else(|| "unknown".to_string()); + let status = outcome.map_or_else(|| "unknown".to_string(), |o| o.status.to_string()); let succeeded = matches!( outcome.map(|o| &o.status), diff --git a/lib/crates/fabro-workflows/src/lifecycle/event.rs b/lib/crates/fabro-workflows/src/lifecycle/event.rs index c6f0285df..363527d51 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/event.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/event.rs @@ -166,8 +166,7 @@ impl RunLifecycle for EventLifecycle { max_attempts: ctx.result.max_attempts as usize, delay_ms: ctx .backoff_delay - .map(|d| u64::try_from(d.as_millis()).unwrap()) - .unwrap_or(0), + .map_or(0, |d| u64::try_from(d.as_millis()).unwrap()), }); } Ok(()) @@ -320,8 +319,7 @@ impl RunLifecycle for EventLifecycle { let error_msg = outcome .failure .as_ref() - .map(|f| f.message.clone()) - .unwrap_or_else(|| "run failed".to_string()); + .map_or_else(|| "run failed".to_string(), |f| f.message.clone()); self.emitter.emit(&WorkflowRunEvent::WorkflowRunFailed { error: FabroError::engine(error_msg), duration_ms, diff --git a/lib/crates/fabro-workflows/src/lifecycle/hook.rs b/lib/crates/fabro-workflows/src/lifecycle/hook.rs index efde92ec2..09f809ac3 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/hook.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/hook.rs @@ -177,8 +177,7 @@ impl RunLifecycle for HookLifecycle { let error_msg = outcome .failure .as_ref() - .map(|f| f.message.clone()) - .unwrap_or_else(|| "run failed".to_string()); + .map_or_else(|| "run failed".to_string(), |f| f.message.clone()); let mut hook_ctx = HookContext::new( HookEvent::RunFailed, self.run_id.clone(), diff --git a/lib/crates/fabro-workflows/src/operations/create.rs b/lib/crates/fabro-workflows/src/operations/create.rs index a74009160..4f906a7ba 100644 --- a/lib/crates/fabro-workflows/src/operations/create.rs +++ b/lib/crates/fabro-workflows/src/operations/create.rs @@ -263,10 +263,8 @@ pub(crate) fn resolve_run_settings(mut settings: FabroSettings, graph: &Graph) - let provider = configured_provider.or(graph_provider).map(str::to_string); - let model = configured_model - .or(graph_model) - .map(str::to_string) - .unwrap_or_else(|| { + let model = configured_model.or(graph_model).map_or_else( + || { let catalog = Catalog::builtin(); provider .as_deref() @@ -275,7 +273,9 @@ pub(crate) fn resolve_run_settings(mut settings: FabroSettings, graph: &Graph) - .unwrap_or_else(|| catalog.default_from_env()) .id .clone() - }); + }, + str::to_string, + ); let (resolved_model, resolved_provider) = match Catalog::builtin().get(&model) { Some(info) => ( diff --git a/lib/crates/fabro-workflows/src/operations/rewind.rs b/lib/crates/fabro-workflows/src/operations/rewind.rs index 0089198b0..9accd7455 100644 --- a/lib/crates/fabro-workflows/src/operations/rewind.rs +++ b/lib/crates/fabro-workflows/src/operations/rewind.rs @@ -81,13 +81,13 @@ impl RunTimeline { .rev() .find(|e| e.node_name == *effective_name) .ok_or_else(|| { - if effective_name != name { + if effective_name == name { + anyhow::anyhow!("no checkpoint found for node '{name}'") + } else { anyhow::anyhow!( "node '{name}' is inside parallel '{effective_name}'; \ no checkpoint found for '{effective_name}'" ) - } else { - anyhow::anyhow!("no checkpoint found for node '{name}'") } }) } @@ -97,13 +97,13 @@ impl RunTimeline { .iter() .find(|e| e.node_name == *effective_name && e.visit == *visit) .ok_or_else(|| { - if effective_name != name { + if effective_name == name { + anyhow::anyhow!("no visit {visit} found for node '{name}'") + } else { anyhow::anyhow!( "node '{name}' is inside parallel '{effective_name}'; \ no visit {visit} found for '{effective_name}'" ) - } else { - anyhow::anyhow!("no visit {visit} found for node '{name}'") } }) } diff --git a/lib/crates/fabro-workflows/src/pipeline/finalize.rs b/lib/crates/fabro-workflows/src/pipeline/finalize.rs index c793472c7..627ecc1f7 100644 --- a/lib/crates/fabro-workflows/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflows/src/pipeline/finalize.rs @@ -252,19 +252,19 @@ pub async fn finalize( if options.preserve_sandbox { let info = sandbox.sandbox_info(); - if !info.is_empty() { + if info.is_empty() { emit_run_notice( &emitter, RunNoticeLevel::Info, "sandbox_preserved", - format!("sandbox preserved: {info}"), + "sandbox preserved", ); } else { emit_run_notice( &emitter, RunNoticeLevel::Info, "sandbox_preserved", - "sandbox preserved", + format!("sandbox preserved: {info}"), ); } } diff --git a/lib/crates/fabro-workflows/src/pipeline/initialize.rs b/lib/crates/fabro-workflows/src/pipeline/initialize.rs index 1e6e9c2a6..e02d4017b 100644 --- a/lib/crates/fabro-workflows/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflows/src/pipeline/initialize.rs @@ -142,8 +142,9 @@ async fn resolve_worktree_plan( let host_repo_path = host_repo_path_for_planning(&options.run_options, &options.sandbox); let git_status = host_repo_path .as_ref() - .map(|path| git::sync_status(path, "origin", options.run_options.base_branch.as_deref())) - .unwrap_or(GitSyncStatus::Dirty); + .map_or(GitSyncStatus::Dirty, |path| { + git::sync_status(path, "origin", options.run_options.base_branch.as_deref()) + }); let strategy = resolve_workdir_strategy( &options.sandbox, worktree_mode, diff --git a/lib/crates/fabro-workflows/src/pipeline/pull_request.rs b/lib/crates/fabro-workflows/src/pipeline/pull_request.rs index a50f06ee2..557d1f884 100644 --- a/lib/crates/fabro-workflows/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflows/src/pipeline/pull_request.rs @@ -64,8 +64,7 @@ fn truncate_pr_body(body: &str) -> String { /// Format an optional cost as `$X.XX` or an en-dash when absent. fn format_cost(cost: Option) -> String { - cost.map(outcome_format_cost) - .unwrap_or_else(|| "\u{2013}".to_string()) + cost.map_or_else(|| "\u{2013}".to_string(), outcome_format_cost) } /// Format a duration in milliseconds as a human-readable string. diff --git a/lib/crates/fabro-workflows/src/run_lookup.rs b/lib/crates/fabro-workflows/src/run_lookup.rs index aebb83d89..995ece82f 100644 --- a/lib/crates/fabro-workflows/src/run_lookup.rs +++ b/lib/crates/fabro-workflows/src/run_lookup.rs @@ -115,8 +115,7 @@ pub fn scan_runs(base: &Path) -> Result> { let mtime = mtime_dt.map(|dt| dt.to_rfc3339()).unwrap_or_default(); let run_id = std::fs::read_to_string(path.join("id.txt")) - .map(|s| s.trim().to_string()) - .unwrap_or_else(|_| dir_name.clone()); + .map_or_else(|_| dir_name.clone(), |s| s.trim().to_string()); let status_info = read_status(&path); let is_orphan = matches!(status_info.status, RunStatus::Dead); diff --git a/lib/crates/fabro-workflows/src/run_options.rs b/lib/crates/fabro-workflows/src/run_options.rs index d15e8a0d6..01ac7eab4 100644 --- a/lib/crates/fabro-workflows/src/run_options.rs +++ b/lib/crates/fabro-workflows/src/run_options.rs @@ -60,8 +60,7 @@ impl RunOptions { self.settings .assets .as_ref() - .map(|a| a.include.as_slice()) - .unwrap_or(&[]) + .map_or(&[], |a| a.include.as_slice()) } }