From 1107b723b12845fa980b4c4ebb92218176462751 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Sun, 26 Apr 2026 13:54:54 -0500 Subject: [PATCH] chore: simplify pass + clippy clean for v0.6.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cleanup pass after the issue fixes (#64, #71, #80, #63): Simplifications: - sidebar.rs: extract `push_agent_row` closure to remove the duplicated two-line agent rendering (cached + progress-only paths used the same shape with different summary text). - engine.rs: replace `error_categories.iter().any(|c| c == X)` with `.contains(&X)` (clippy::manual_contains). - widgets/mod.rs: replace `for idx in menu_top..menu_bottom` index loop with `.iter().enumerate().take(menu_bottom).skip(menu_top)` (clippy::needless_range_loop). Build hygiene (CI runs `cargo clippy ... -- -D warnings`): - error_taxonomy.rs: per-item `#[allow(dead_code)]` on `ErrorSeverity`, `ErrorEnvelope`, and `ErrorEnvelope::new` with TODO notes referencing #66. Keeps deepseek's removal of the file-wide allow but stops the scaffold from breaking the build until #66 follows up. - app.rs: per-field `#[allow(dead_code)]` on `fancy_animations` (pending #61 footer animation consumer). - config/lib.rs: complete the OpenRouter/Novita variant scaffolding so `match ProviderKind { ... }` is exhaustive — add api_key/base_url env loading (`OPENROUTER_API_KEY`, `NOVITA_API_KEY`, optional `*_BASE_URL` overrides), wire `api_key_for` / `base_url_for` arms with the documented defaults, and extend `normalize_model_for_provider` so generic V4 model names map to each provider's catalog ID. Full /provider picker UI still pending #52. Verified: cargo fmt clean, cargo clippy --workspace --all-targets --all-features --locked -- -D warnings clean, full test suite passes (979 + adjacent crate tests). --- crates/config/src/lib.rs | 42 +++++++++++++++++++++++++ crates/tui/src/client/chat.rs | 2 +- crates/tui/src/core/engine.rs | 13 +++----- crates/tui/src/error_taxonomy.rs | 15 ++++++++- crates/tui/src/mcp.rs | 21 ++++++------- crates/tui/src/settings.rs | 5 ++- crates/tui/src/tui/app.rs | 3 ++ crates/tui/src/tui/sidebar.rs | 52 ++++++++++++++----------------- crates/tui/src/tui/ui.rs | 8 ++--- crates/tui/src/tui/widgets/mod.rs | 8 +++-- 10 files changed, 111 insertions(+), 58 deletions(-) diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 88ca6a97..f047f467 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -483,6 +483,22 @@ fn normalize_model_for_provider(provider: ProviderKind, model: &str) -> String { "deepseek-v4-flash" | "deepseek-v4flash" | "deepseek-chat" | "deepseek-reasoner" | "deepseek-r1" | "deepseek-v3" | "deepseek-v3.2", ) => DEFAULT_NVIDIA_NIM_FLASH_MODEL.to_string(), + (ProviderKind::Openrouter, "deepseek-v4-pro" | "deepseek-v4pro") => { + DEFAULT_OPENROUTER_MODEL.to_string() + } + ( + ProviderKind::Openrouter, + "deepseek-v4-flash" | "deepseek-v4flash" | "deepseek-chat" | "deepseek-reasoner" + | "deepseek-r1" | "deepseek-v3" | "deepseek-v3.2", + ) => DEFAULT_OPENROUTER_FLASH_MODEL.to_string(), + (ProviderKind::Novita, "deepseek-v4-pro" | "deepseek-v4pro") => { + DEFAULT_NOVITA_MODEL.to_string() + } + ( + ProviderKind::Novita, + "deepseek-v4-flash" | "deepseek-v4flash" | "deepseek-chat" | "deepseek-reasoner" + | "deepseek-r1" | "deepseek-v3" | "deepseek-v3.2", + ) => DEFAULT_NOVITA_FLASH_MODEL.to_string(), _ => model.to_string(), } } @@ -606,9 +622,13 @@ struct EnvRuntimeOverrides { deepseek_api_key: Option, openai_api_key: Option, nvidia_api_key: Option, + openrouter_api_key: Option, + novita_api_key: Option, deepseek_base_url: Option, nvidia_base_url: Option, openai_base_url: Option, + openrouter_base_url: Option, + novita_base_url: Option, } impl EnvRuntimeOverrides { @@ -647,6 +667,18 @@ impl EnvRuntimeOverrides { openai_base_url: std::env::var("OPENAI_BASE_URL") .ok() .filter(|v| !v.trim().is_empty()), + openrouter_api_key: std::env::var("OPENROUTER_API_KEY") + .ok() + .filter(|v| !v.trim().is_empty()), + novita_api_key: std::env::var("NOVITA_API_KEY") + .ok() + .filter(|v| !v.trim().is_empty()), + openrouter_base_url: std::env::var("OPENROUTER_BASE_URL") + .ok() + .filter(|v| !v.trim().is_empty()), + novita_base_url: std::env::var("NOVITA_BASE_URL") + .ok() + .filter(|v| !v.trim().is_empty()), } } @@ -658,6 +690,8 @@ impl EnvRuntimeOverrides { .clone() .or_else(|| self.deepseek_api_key.clone()), ProviderKind::Openai => self.openai_api_key.clone(), + ProviderKind::Openrouter => self.openrouter_api_key.clone(), + ProviderKind::Novita => self.novita_api_key.clone(), } } @@ -666,6 +700,14 @@ impl EnvRuntimeOverrides { ProviderKind::Deepseek => self.deepseek_base_url.clone(), ProviderKind::NvidiaNim => self.nvidia_base_url.clone(), ProviderKind::Openai => self.openai_base_url.clone(), + ProviderKind::Openrouter => self + .openrouter_base_url + .clone() + .or_else(|| Some("https://openrouter.ai/api/v1".to_string())), + ProviderKind::Novita => self + .novita_base_url + .clone() + .or_else(|| Some("https://api.novita.ai/v1".to_string())), } } } diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index e8637999..a2b5cb18 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -9,8 +9,8 @@ use std::pin::Pin; use std::time::Duration; use anyhow::{Context, Result}; -use tokio::time::timeout as tokio_timeout; use serde_json::{Value, json}; +use tokio::time::timeout as tokio_timeout; /// Default idle timeout for SSE stream reads (300 seconds = 5 minutes). /// After this period with no data, the stream is considered stalled and diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index c325479f..7558856b 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -3335,16 +3335,16 @@ impl Engine { mode: AppMode, step_error_count: usize, consecutive_tool_error_steps: u32, - #[allow(clippy::needless_pass_by_ref_mut)] // error_categories will be used in future escalation logic + #[allow(clippy::needless_pass_by_ref_mut)] + // error_categories will be used in future escalation logic error_categories: &[crate::error_taxonomy::ErrorCategory], ) -> bool { if step_error_count == 0 && consecutive_tool_error_steps < 2 { return false; } - let has_context_overflow = error_categories - .iter() - .any(|&cat| cat == crate::error_taxonomy::ErrorCategory::InvalidInput); + let has_context_overflow = + error_categories.contains(&crate::error_taxonomy::ErrorCategory::InvalidInput); if !has_context_overflow && consecutive_tool_error_steps < 2 { // Only escalate on non-context errors when we have consecutive failures @@ -3380,10 +3380,7 @@ impl Engine { return false; } - let category_labels: Vec = error_categories - .iter() - .map(|c| c.to_string()) - .collect(); + let category_labels: Vec = error_categories.iter().map(|c| c.to_string()).collect(); self.apply_verify_and_replan( turn, mode, diff --git a/crates/tui/src/error_taxonomy.rs b/crates/tui/src/error_taxonomy.rs index 7da83227..75b4d9c8 100644 --- a/crates/tui/src/error_taxonomy.rs +++ b/crates/tui/src/error_taxonomy.rs @@ -21,6 +21,11 @@ pub enum ErrorCategory { } /// Severity hint for UI and logs. +/// +/// Adopted in `From` / `From` and exercised by tests, but +/// not yet read by an audit-log writer or TUI severity colorer. Pending #66 +/// follow-up; the type is stable. +#[allow(dead_code)] #[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "snake_case")] pub enum ErrorSeverity { @@ -31,6 +36,11 @@ pub enum ErrorSeverity { } /// Unified envelope used when crossing subsystem boundaries. +/// +/// Constructed by the `From` / `From` impls below and +/// validated by tests, but the engine still emits errors as `(String, bool)` +/// pairs on the event channel. Pending #66 follow-up. +#[allow(dead_code)] #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct ErrorEnvelope { pub category: ErrorCategory, @@ -79,6 +89,7 @@ impl fmt::Display for ErrorEnvelope { impl std::error::Error for ErrorEnvelope {} impl ErrorEnvelope { + #[allow(dead_code)] #[must_use] pub fn new( category: ErrorCategory, @@ -220,7 +231,9 @@ pub fn classify_error_message(message: &str) -> ErrorCategory { if lower.contains("parse") || lower.contains("syntax") || lower.contains("malformed") { return ErrorCategory::Parse; } - if lower.contains("not found") || lower.contains("unavailable") || lower.contains("not available") + if lower.contains("not found") + || lower.contains("unavailable") + || lower.contains("not available") { return ErrorCategory::State; } diff --git a/crates/tui/src/mcp.rs b/crates/tui/src/mcp.rs index ec721fdd..98aab9f3 100644 --- a/crates/tui/src/mcp.rs +++ b/crates/tui/src/mcp.rs @@ -361,16 +361,12 @@ impl SseTransport { tx: tokio::sync::mpsc::UnboundedSender, cancel_token: tokio_util::sync::CancellationToken, ) -> Result<()> { - let response = client - .get(&url) - .send() - .await - .with_context(|| { - format!( - "MCP SSE connect failed (transport=http url={})", - mask_url_secrets(&url), - ) - })?; + let response = client.get(&url).send().await.with_context(|| { + format!( + "MCP SSE connect failed (transport=http url={})", + mask_url_secrets(&url), + ) + })?; let status = response.status(); if !status.is_success() { let body_excerpt = bounded_body_excerpt(response, ERROR_BODY_PREVIEW_BYTES).await; @@ -1916,6 +1912,9 @@ mod tests { let redacted = redact_body_preview("error message api_key=sk-12345&other=val"); assert!(redacted.contains("api_key=***"), "redacted: {redacted}"); assert!(!redacted.contains("sk-12345"), "leaked: {redacted}"); - assert!(redacted.contains("other=val"), "non-secret preserved: {redacted}"); + assert!( + redacted.contains("other=val"), + "non-secret preserved: {redacted}" + ); } } diff --git a/crates/tui/src/settings.rs b/crates/tui/src/settings.rs index 0c261353..ddc9f726 100644 --- a/crates/tui/src/settings.rs +++ b/crates/tui/src/settings.rs @@ -282,7 +282,10 @@ impl Settings { ("auto_compact", "Auto-compact conversations: on/off"), ("calm_mode", "Calmer UI defaults: on/off"), ("low_motion", "Reduce animation and redraw churn: on/off"), - ("fancy_animations", "Fancy footer animations (water-spout strip): on/off"), + ( + "fancy_animations", + "Fancy footer animations (water-spout strip): on/off", + ), ("show_thinking", "Show model thinking: on/off"), ("show_tool_details", "Show detailed tool output: on/off"), ( diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index 6d6cca11..5261f4b3 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -404,6 +404,9 @@ pub struct App { pub auto_compact: bool, pub calm_mode: bool, pub low_motion: bool, + /// Pending #61 (animated working strip). Set from config but not read + /// until the footer widget consumes it. + #[allow(dead_code)] pub fancy_animations: bool, pub show_thinking: bool, pub show_tool_details: bool, diff --git a/crates/tui/src/tui/sidebar.rs b/crates/tui/src/tui/sidebar.rs index 7cecff4a..8cd65320 100644 --- a/crates/tui/src/tui/sidebar.rs +++ b/crates/tui/src/tui/sidebar.rs @@ -352,24 +352,26 @@ fn render_sidebar_subagents(f: &mut Frame, area: Rect, app: &App) { let usable_rows = area.height.saturating_sub(3) as usize; let max_agents = usable_rows.saturating_sub(lines.len()); + let push_agent_row = + |lines: &mut Vec>, summary: &str, detail: &str, color| { + lines.push(Line::from(Span::styled( + truncate_line_to_width(summary, content_width.max(1)), + Style::default().fg(color), + ))); + lines.push(Line::from(Span::styled( + format!( + " {}", + truncate_line_to_width(detail, content_width.saturating_sub(2).max(1)) + ), + Style::default().fg(palette::TEXT_DIM), + ))); + }; + // Live (progress-only) agents first — they're the freshest signal. let mut rendered = 0usize; for (id, msg) in progress_only.iter().take(max_agents) { - let summary = format!( - "{} starting", - truncate_line_to_width(id, 10), - ); - lines.push(Line::from(Span::styled( - truncate_line_to_width(&summary, content_width.max(1)), - Style::default().fg(palette::STATUS_WARNING), - ))); - lines.push(Line::from(Span::styled( - format!( - " {}", - truncate_line_to_width(msg, content_width.saturating_sub(2).max(1)) - ), - Style::default().fg(palette::TEXT_DIM), - ))); + let summary = format!("{} starting", truncate_line_to_width(id, 10)); + push_agent_row(&mut lines, &summary, msg, palette::STATUS_WARNING); rendered += 1; } @@ -391,20 +393,12 @@ fn render_sidebar_subagents(f: &mut Frame, area: Rect, app: &App) { truncate_line_to_width(&agent.agent_id, 10), agent.steps_taken ); - lines.push(Line::from(Span::styled( - truncate_line_to_width(&summary, content_width.max(1)), - Style::default().fg(status_color), - ))); - lines.push(Line::from(Span::styled( - format!( - " {}", - truncate_line_to_width( - &agent.assignment.objective, - content_width.saturating_sub(2).max(1) - ) - ), - Style::default().fg(palette::TEXT_DIM), - ))); + push_agent_row( + &mut lines, + &summary, + &agent.assignment.objective, + status_color, + ); rendered += 1; } diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 52b9a5ea..a99bb445 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -23,8 +23,8 @@ use ratatui::{ text::Span, widgets::Block, }; -use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; use tracing; +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; use crate::audit::log_sensitive_event; use crate::client::DeepSeekClient; @@ -519,10 +519,8 @@ async fn run_event_loop( "agent_spawn" | "agent_swarm" | "agent_cancel" | "todo_write" ) { let tasks = task_manager.list_tasks(Some(10)).await; - app.task_panel = tasks - .into_iter() - .map(task_summary_to_panel_entry) - .collect(); + app.task_panel = + tasks.into_iter().map(task_summary_to_panel_entry).collect(); last_task_refresh = Instant::now(); } } diff --git a/crates/tui/src/tui/widgets/mod.rs b/crates/tui/src/tui/widgets/mod.rs index 0332a783..0afb9eb8 100644 --- a/crates/tui/src/tui/widgets/mod.rs +++ b/crates/tui/src/tui/widgets/mod.rs @@ -383,8 +383,12 @@ impl Renderable for ComposerWidget<'_> { }; let menu_bottom = (menu_top + menu_visible_rows).min(menu_total); - for idx in menu_top..menu_bottom { - let entry = &menu_entries[idx]; + for (idx, entry) in menu_entries + .iter() + .enumerate() + .take(menu_bottom) + .skip(menu_top) + { let is_selected = idx == selected; let style = if is_selected { Style::default()