diff --git a/CHANGELOG.md b/CHANGELOG.md index f11d2a11..615254d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `[subagents] heartbeat_timeout_secs` window (default 300s), releasing their concurrency slot and unblocking parent turns that would otherwise wait forever (#2603, #2614, #2620). +- **Work panel state survives transient lock misses.** The sidebar caches the + last successful Work summary so checklist and strategy progress no longer + disappear into "Work state updating..." while the engine briefly owns the + shared todo/plan locks (#2606, #2616). - **SiliconFlow-CN no longer breaks main.** Filled the missing CLI provider exhaustiveness arms and removed the duplicate/unreachable TUI config arms left by the #2615 landing; direct auth now stores the China-region variant in diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index f11d2a11..615254d5 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -31,6 +31,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `[subagents] heartbeat_timeout_secs` window (default 300s), releasing their concurrency slot and unblocking parent turns that would otherwise wait forever (#2603, #2614, #2620). +- **Work panel state survives transient lock misses.** The sidebar caches the + last successful Work summary so checklist and strategy progress no longer + disappear into "Work state updating..." while the engine briefly owns the + shared todo/plan locks (#2606, #2616). - **SiliconFlow-CN no longer breaks main.** Filled the missing CLI provider exhaustiveness arms and removed the duplicate/unreachable TUI config arms left by the #2615 landing; direct auth now stores the China-region variant in diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index 8307b5b8..3eb494f1 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -40,6 +40,7 @@ use crate::tui::history::{HistoryCell, TranscriptRenderOptions}; use crate::tui::paste_burst::{FlushResult, PasteBurst}; use crate::tui::scrolling::{MouseScrollState, TranscriptLineMeta, TranscriptScroll}; use crate::tui::selection::{SelectionAutoscroll, TranscriptSelection}; +use crate::tui::sidebar::SidebarWorkSummary; use crate::tui::streaming::StreamingState; use crate::tui::transcript::TranscriptViewCache; use crate::tui::views::ViewStack; @@ -1297,6 +1298,9 @@ pub struct App { pub sidebar_hover: SidebarHoverState, /// Current hover tooltip text, if any. pub sidebar_hover_tooltip: Option, + /// Last successfully rendered Work panel summary. Transient mutex misses + /// should not wipe completed checklist/strategy state from the sidebar. + pub(crate) cached_work_summary: Option, /// Last known mouse position for tooltip placement. pub last_mouse_pos: Option<(u16, u16)>, /// Whether the user is currently dragging the sidebar resize handle. @@ -2034,6 +2038,7 @@ impl App { sidebar_focus, sidebar_hover: SidebarHoverState::default(), sidebar_hover_tooltip: None, + cached_work_summary: None, last_mouse_pos: None, sidebar_resizing: false, sidebar_resize_anchor_x: 0, diff --git a/crates/tui/src/tui/sidebar.rs b/crates/tui/src/tui/sidebar.rs index 9645bd8f..fa1b26ce 100644 --- a/crates/tui/src/tui/sidebar.rs +++ b/crates/tui/src/tui/sidebar.rs @@ -168,7 +168,7 @@ struct SidebarWorkStrategyStep { } #[derive(Debug, Clone, Default)] -struct SidebarWorkSummary { +pub(crate) struct SidebarWorkSummary { goal_objective: Option, goal_token_budget: Option, goal_completed: bool, @@ -226,56 +226,77 @@ impl SidebarWorkSummary { } } -fn sidebar_work_summary(app: &App) -> SidebarWorkSummary { - let mut summary = SidebarWorkSummary { - goal_objective: app.hunt.quarry.clone(), - goal_token_budget: app.hunt.token_budget, - goal_completed: app.hunt.verdict == HuntVerdict::Hunted, - goal_started_at: app.hunt.started_at, - tokens_used: app.session.total_conversation_tokens, - ..SidebarWorkSummary::default() - }; +fn sidebar_work_summary(app: &mut App) -> SidebarWorkSummary { + let fresh = (|| { + let todos = app.todos.try_lock().ok()?; + let plan = app.plan_state.try_lock().ok()?; - match app.todos.try_lock() { - Ok(todos) => { - let snapshot = todos.snapshot(); - summary.checklist_completion_pct = snapshot.completion_pct; - summary.checklist_items = snapshot - .items - .into_iter() - .map(|item| SidebarWorkChecklistItem { - id: item.id, - content: item.content, - status: item.status, - }) - .collect(); - } - Err(_) => { - summary.state_updating = true; - } - } + let snapshot = todos.snapshot(); + let checklist_completion_pct = snapshot.completion_pct; + let checklist_items = snapshot + .items + .into_iter() + .map(|item| SidebarWorkChecklistItem { + id: item.id, + content: item.content, + status: item.status, + }) + .collect(); - match app.plan_state.try_lock() { - Ok(plan) => { - if !plan.is_empty() { - summary.strategy_explanation = plan.explanation().map(str::to_string); - summary.strategy_steps = plan - .steps() + let (strategy_explanation, strategy_steps) = if plan.is_empty() { + (None, Vec::new()) + } else { + ( + plan.explanation().map(str::to_string), + plan.steps() .iter() .map(|step| SidebarWorkStrategyStep { text: step.text.clone(), status: step.status.clone(), elapsed: step.elapsed_str(), }) - .collect(); - } - } - Err(_) => { - summary.state_updating = true; - } + .collect(), + ) + }; + + Some(SidebarWorkSummary { + goal_objective: app.hunt.quarry.clone(), + goal_token_budget: app.hunt.token_budget, + goal_completed: app.hunt.verdict == HuntVerdict::Hunted, + goal_started_at: app.hunt.started_at, + tokens_used: app.session.total_conversation_tokens, + checklist_completion_pct, + checklist_items, + strategy_explanation, + strategy_steps, + state_updating: false, + }) + })(); + + if let Some(summary) = fresh { + app.cached_work_summary = Some(summary.clone()); + return summary; } - summary + if let Some(cached) = app.cached_work_summary.as_ref() { + let mut summary = cached.clone(); + summary.goal_objective = app.hunt.quarry.clone(); + summary.goal_token_budget = app.hunt.token_budget; + summary.goal_completed = app.hunt.verdict == HuntVerdict::Hunted; + summary.goal_started_at = app.hunt.started_at; + summary.tokens_used = app.session.total_conversation_tokens; + return summary; + } + + SidebarWorkSummary { + goal_objective: app.hunt.quarry.clone(), + goal_token_budget: app.hunt.token_budget, + goal_completed: app.hunt.verdict == HuntVerdict::Hunted, + goal_started_at: app.hunt.started_at, + tokens_used: app.session.total_conversation_tokens, + state_updating: true, + ..SidebarWorkSummary::default() + } } fn work_panel_lines( @@ -1946,8 +1967,8 @@ mod tests { AutoSidebarState, SidebarAgentRow, SidebarHoverSection, SidebarHoverState, SidebarSubagentSummary, SidebarToolRow, SidebarWorkChecklistItem, SidebarWorkStrategyStep, SidebarWorkSummary, ToolRowOrder, auto_sidebar_panels, editorial_tool_rows, - normalize_activity_text, subagent_panel_lines, task_panel_lines, work_panel_empty_hint, - work_panel_lines, + normalize_activity_text, sidebar_work_summary, subagent_panel_lines, task_panel_lines, + work_panel_empty_hint, work_panel_lines, }; use crate::config::Config; use crate::palette; @@ -1955,7 +1976,7 @@ mod tests { use crate::tools::plan::StepStatus; use crate::tools::todo::TodoStatus; use crate::tui::active_cell::ActiveCell; - use crate::tui::app::{App, TaskPanelEntry, TuiOptions}; + use crate::tui::app::{App, HuntVerdict, TaskPanelEntry, TuiOptions}; use crate::tui::history::{ ExecCell, ExecSource, GenericToolCell, HistoryCell, ToolCell, ToolStatus, }; @@ -2245,6 +2266,82 @@ mod tests { ); } + #[test] + fn sidebar_work_summary_caches_on_success() { + let mut app = create_test_app(); + { + let mut todos = app.todos.try_lock().expect("todos lock"); + todos.add("cache test".to_string(), TodoStatus::InProgress); + } + + let summary = sidebar_work_summary(&mut app); + + assert!(!summary.state_updating, "should not be updating"); + assert_eq!(summary.checklist_items.len(), 1); + assert!( + app.cached_work_summary.is_some(), + "cache should be populated" + ); + } + + #[test] + fn sidebar_work_summary_falls_back_to_cache_when_todos_lock_busy() { + let mut app = create_test_app(); + { + let mut todos = app.todos.try_lock().expect("todos lock"); + todos.add("will be cached".to_string(), TodoStatus::Completed); + } + let _first = sidebar_work_summary(&mut app); + assert!(app.cached_work_summary.is_some()); + + let held_arc = app.todos.clone(); + let _held = held_arc.try_lock().expect("hold todos lock"); + + let summary = sidebar_work_summary(&mut app); + + assert!(!summary.state_updating, "should fall back to cache"); + assert!( + summary + .checklist_items + .iter() + .any(|item| item.content == "will be cached"), + "cached item should be present" + ); + } + + #[test] + fn sidebar_work_summary_returns_updating_when_no_cache_and_locks_busy() { + let mut app = create_test_app(); + let held_arc = app.todos.clone(); + let _held = held_arc.try_lock().expect("hold todos lock"); + + let summary = sidebar_work_summary(&mut app); + + assert!(summary.state_updating, "should be updating without cache"); + } + + #[test] + fn sidebar_work_summary_keeps_live_fields_on_cache_fallback() { + let mut app = create_test_app(); + app.hunt.quarry = Some("test quarry".to_string()); + app.hunt.verdict = HuntVerdict::Hunted; + { + let mut todos = app.todos.try_lock().expect("todos lock"); + todos.add("item".to_string(), TodoStatus::Pending); + } + let _first = sidebar_work_summary(&mut app); + + app.hunt.quarry = Some("updated quarry".to_string()); + app.hunt.verdict = HuntVerdict::Hunting; + let held_arc = app.todos.clone(); + let _held = held_arc.try_lock().expect("hold todos lock"); + + let summary = sidebar_work_summary(&mut app); + + assert_eq!(summary.goal_objective.as_deref(), Some("updated quarry")); + assert!(!summary.goal_completed, "verdict should be live"); + } + #[test] fn tasks_panel_renders_active_tool_rows_before_background_empty_state() { let mut app = create_test_app();