From 22a253dea157c2ec1e928c30376691baea38149a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Sun, 26 Jul 2026 18:30:46 +0200 Subject: [PATCH 1/2] fix(tui): spinner vanished whenever the status area was occupied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The status area (plan, multi-line info, pending user message) is its own layout region between the content area and the composer, but `paint()` *also* subtracted its height from the scratch buffer's cursor. Since the viewport is sized to exactly `desired_viewport_height()`, the content area shrank by `status_height` while the composed content simultaneously moved up by `status_height` — the bottom-aligned copy then had room for nothing but the reserved blank rows, and the spinner (plus the top of the live message) fell out of the viewport. A displayed plan made the progress spinner disappear entirely. Budget the status height against its own `status_budget` (the rows the layout can actually hand out) and leave `cursor_y` to the spinner and the live message. Cap a single entry at 20 rows like `measure_status_height` does, so paint never reserves more than the viewport height that was requested. Also retire a finished plan when the user submits the next message: it stays up for the whole turn and past the agent's closing answer, but a plan whose items are all completed belongs to the request that is over. A plan with open items survives — the new message usually continues it. --- crates/ui_terminal/src/app.rs | 41 +++++--- crates/ui_terminal/src/renderer.rs | 163 +++++++++++++++++++++++++---- crates/ui_terminal/src/state.rs | 79 +++++++++++++- 3 files changed, 245 insertions(+), 38 deletions(-) diff --git a/crates/ui_terminal/src/app.rs b/crates/ui_terminal/src/app.rs index c85b8739..e53a7d53 100644 --- a/crates/ui_terminal/src/app.rs +++ b/crates/ui_terminal/src/app.rs @@ -844,24 +844,31 @@ async fn event_loop( disabled until it finishes." .to_string(), )); - } else if activity_state - .as_ref() - .is_none_or(|s| s.is_terminal()) - { - // No agent running: start a new turn. - cancel_flag.store(false, Ordering::SeqCst); - actions.send_user_message( - session_id, - message, - attachments, - ); } else { - // Local agent running: queue the message. - actions.queue_user_message( - session_id, - message, - attachments, - ); + // The message is accepted: a plan whose + // items are all done belongs to the + // previous request and is retired here. + app_state.lock().await.clear_completed_plan(); + + if activity_state + .as_ref() + .is_none_or(|s| s.is_terminal()) + { + // No agent running: start a new turn. + cancel_flag.store(false, Ordering::SeqCst); + actions.send_user_message( + session_id, + message, + attachments, + ); + } else { + // Local agent running: queue the message. + actions.queue_user_message( + session_id, + message, + attachments, + ); + } } } } diff --git a/crates/ui_terminal/src/renderer.rs b/crates/ui_terminal/src/renderer.rs index c285cbf7..017dc33c 100644 --- a/crates/ui_terminal/src/renderer.rs +++ b/crates/ui_terminal/src/renderer.rs @@ -918,51 +918,62 @@ impl TerminalRenderer { }); } + let popup_height = self.measure_autocomplete_height(); + + // The status area is a region of its own below the content area (see the + // `Layout::vertical` further down), so its rows must NOT be reserved in + // the scratch buffer as well: doing so shifted the live content up by + // `status_height` rows while the content area only shrank by that much, + // which pushed the spinner (and the top of the live message) out of the + // viewport whenever a plan/info/pending message was displayed. + // Everything below therefore budgets against `status_budget` — the rows + // the layout can actually give the status area — and leaves `cursor_y` + // to the spinner and the live message alone. + let mut status_budget = available.saturating_sub(popup_height); let mut status_height: u16 = 0; let mut error_display: Option = None; + // Cap a single status entry the same way `measure_status_height` does, + // so the height this paint reserves matches the viewport height the Tui + // layer asked for. + const MAX_STATUS_ENTRY_HEIGHT: u16 = 20; + if let Some(ref error_msg) = self.current_error { let formatted = Self::format_error_message(error_msg); - let max_height = cursor_y.min(scratch_height).max(1); - let rendered_height = Self::measure_markdown_height(&formatted, width, max_height); - let actual_height = rendered_height.min(cursor_y); + let max_height = status_budget.min(MAX_STATUS_ENTRY_HEIGHT); + let actual_height = Self::measure_markdown_height(&formatted, width, max_height); if actual_height > 0 { - cursor_y = cursor_y.saturating_sub(actual_height); - status_height = status_height.saturating_add(actual_height); - if cursor_y > 0 { - cursor_y = cursor_y.saturating_sub(1); - status_height = status_height.saturating_add(1); + status_height = actual_height; + if status_budget > actual_height { + status_height = status_height.saturating_add(1); // gap } } error_display = Some(formatted); } else if !status_entries.is_empty() { let mut any_rendered = false; for idx in 0..status_entries.len() { - if cursor_y == 0 { + if status_budget == 0 { break; } let entry = &mut status_entries[idx]; - let max_height = cursor_y.min(scratch_height).max(1); - let rendered_height = - Self::measure_markdown_height(&entry.content, width, max_height); - let actual_height = rendered_height.min(cursor_y); + let max_height = status_budget.min(MAX_STATUS_ENTRY_HEIGHT); + let actual_height = Self::measure_markdown_height(&entry.content, width, max_height); entry.height = actual_height; if actual_height > 0 { any_rendered = true; - cursor_y = cursor_y.saturating_sub(actual_height); + status_budget = status_budget.saturating_sub(actual_height); status_height = status_height.saturating_add(actual_height); - if idx + 1 < status_entries.len() && cursor_y > 0 { - cursor_y = cursor_y.saturating_sub(1); + if idx + 1 < status_entries.len() && status_budget > 0 { + status_budget = status_budget.saturating_sub(1); status_height = status_height.saturating_add(1); } } } - if any_rendered && cursor_y > 0 { - cursor_y = cursor_y.saturating_sub(1); + if any_rendered && status_budget > 0 { status_height = status_height.saturating_add(1); } } @@ -1005,8 +1016,6 @@ impl TerminalRenderer { // Composed content occupies rows [cursor_y .. scratch_height) let total_height = scratch_height.saturating_sub(cursor_y); - let popup_height = self.measure_autocomplete_height(); - let [content_area, status_area, popup_area, input_area] = Layout::vertical([ Constraint::Min(0), Constraint::Length(status_height), @@ -1755,6 +1764,13 @@ mod tests { &self.buffer } + /// Render into a viewport sized the way the `Tui` layer sizes it in + /// production: exactly the height the renderer asks for. + fn render_at_desired_height(&mut self, textarea: &TextArea) -> &Buffer { + self.height = self.renderer.desired_viewport_height(textarea, self.width); + self.render(textarea) + } + /// Access the buffer after rendering fn buffer(&self) -> &Buffer { &self.buffer @@ -2258,6 +2274,113 @@ mod tests { ); } + /// Row index of the first line containing `needle`, or `None`. + fn find_row(buffer: &Buffer, needle: &str) -> Option { + let area = *buffer.area(); + for y in area.y..area.y + area.height { + let mut line = String::new(); + for x in area.x..area.x + area.width { + line.push_str(buffer.cell((x, y)).unwrap().symbol()); + } + if line.contains(needle) { + return Some(y); + } + } + None + } + + /// Row index of the first line carrying a spinner braille glyph. + fn find_spinner_row(buffer: &Buffer) -> Option { + const BRAILLE: [&str; 10] = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; + BRAILLE.iter().find_map(|c| find_row(buffer, c)) + } + + fn plan_with_open_item() -> PlanState { + PlanState { + entries: vec![ + PlanItem { + content: "Gather requirements".to_string(), + status: PlanItemStatus::Completed, + ..Default::default() + }, + PlanItem { + content: "Update documentation".to_string(), + ..Default::default() + }, + ], + ..Default::default() + } + } + + /// Regression: the status area (plan / info / pending) is laid out as its + /// own region *below* the live content, so its rows must not be reserved + /// in the scratch buffer as well. They used to be, which pushed the + /// spinner out of the top of the content area — the spinner simply + /// disappeared as soon as a plan was displayed. + #[test] + fn spinner_stays_visible_above_the_plan() { + let mut renderer = create_default_test_harness(); + let textarea = TextArea::new(); + + renderer.set_plan_state(Some(plan_with_open_item())); + renderer.set_plan_expanded(false); + renderer.start_new_message(1); // request sent → loading spinner + + renderer.render_at_desired_height(&textarea); + let buffer = renderer.buffer(); + + let spinner_row = find_spinner_row(buffer).expect("spinner must be rendered"); + let plan_row = find_row(buffer, "Plan: Update documentation").expect("plan must render"); + assert!( + spinner_row < plan_row, + "spinner (row {spinner_row}) must be above the plan (row {plan_row})" + ); + } + + /// Same reservation bug, reached through a pending user message instead + /// of a plan. + #[test] + fn spinner_stays_visible_above_a_pending_message() { + let mut renderer = create_default_test_harness(); + let textarea = TextArea::new(); + + // Multi-line so it lands in the status area, not the composer footer. + renderer.set_pending_user_message(Some("queued line one\nqueued line two".to_string())); + renderer.start_new_message(1); + + renderer.render_at_desired_height(&textarea); + let buffer = renderer.buffer(); + + let spinner_row = find_spinner_row(buffer).expect("spinner must be rendered"); + let pending_row = find_row(buffer, "queued line one").expect("pending must render"); + assert!( + spinner_row < pending_row, + "spinner (row {spinner_row}) must be above the pending message (row {pending_row})" + ); + } + + /// The expanded plan is taller than the collapsed summary; the spinner + /// must survive that too. + #[test] + fn spinner_stays_visible_above_an_expanded_plan() { + let mut renderer = create_default_test_harness(); + let textarea = TextArea::new(); + + renderer.set_plan_state(Some(plan_with_open_item())); + renderer.set_plan_expanded(true); + renderer.start_new_message(1); + + renderer.render_at_desired_height(&textarea); + let buffer = renderer.buffer(); + + let spinner_row = find_spinner_row(buffer).expect("spinner must be rendered"); + let plan_row = find_row(buffer, "Update documentation").expect("plan must render"); + assert!( + spinner_row < plan_row, + "spinner (row {spinner_row}) must be above the plan (row {plan_row})" + ); + } + #[test] fn test_plan_collapsed_summary_rendering() { let mut renderer = create_default_test_harness(); diff --git a/crates/ui_terminal/src/state.rs b/crates/ui_terminal/src/state.rs index 23db90e3..a437592a 100644 --- a/crates/ui_terminal/src/state.rs +++ b/crates/ui_terminal/src/state.rs @@ -3,7 +3,7 @@ use code_assistant_core::persistence::{ChatMetadata, NodeId}; use code_assistant_core::session::instance::SessionActivityState; use code_assistant_core::session::permissions::ToolPermissionRequestData; use code_assistant_core::session::service::SkillCatalogEntry; -use code_assistant_core::types::PlanState; +use code_assistant_core::types::{PlanItemStatus, PlanState}; use sandbox::SandboxPolicy; use std::collections::{HashMap, HashSet}; use tools_core::permissions::PermissionTier; @@ -227,6 +227,37 @@ impl AppState { self.plan_dirty = true; } + /// Drop the plan if every item is done. Called when the user submits a new + /// message: the plan stays on screen for the whole turn *and* after the + /// agent's closing answer (so it can be reviewed), but a finished plan + /// belongs to the request that is now over and must not linger above the + /// composer into the next one. A plan with open items survives — the new + /// message is likely a continuation of it. + /// + /// Returns whether the plan was cleared. + pub fn clear_completed_plan(&mut self) -> bool { + let all_completed = match &self.plan { + Some(plan) => { + !plan.entries.is_empty() + && plan + .entries + .iter() + .all(|entry| matches!(entry.status, PlanItemStatus::Completed)) + } + None => false, + }; + if !all_completed { + return false; + } + + self.set_plan(None); + self.plan_expanded = false; + if matches!(self.overlay_state, OverlayState::Plan) { + self.overlay_state = OverlayState::None; + } + true + } + pub fn toggle_plan_expanded(&mut self) -> bool { self.plan_expanded = !self.plan_expanded; self.overlay_state = if self.plan_expanded { @@ -261,6 +292,52 @@ mod tests { assert!(state.mark_node_seen(42)); } + fn plan_of(statuses: &[PlanItemStatus]) -> PlanState { + use code_assistant_core::types::PlanItem; + PlanState { + entries: statuses + .iter() + .enumerate() + .map(|(i, status)| PlanItem { + content: format!("item {i}"), + status: status.clone(), + ..Default::default() + }) + .collect(), + ..Default::default() + } + } + + #[test] + fn submitting_retires_a_finished_plan_but_keeps_an_open_one() { + let mut state = AppState::new(); + + // Nothing to retire without a plan. + assert!(!state.clear_completed_plan()); + + // A plan with open items survives the next user message. + state.set_plan(Some(plan_of(&[ + PlanItemStatus::Completed, + PlanItemStatus::InProgress, + ]))); + assert!(!state.clear_completed_plan()); + assert!(state.plan.is_some()); + + // All done → the plan is dropped, and the expanded overlay with it. + state.set_plan(Some(plan_of(&[ + PlanItemStatus::Completed, + PlanItemStatus::Completed, + ]))); + state.toggle_plan_expanded(); + assert!(state.is_overlay_active()); + + assert!(state.clear_completed_plan()); + assert!(state.plan.is_none()); + assert!(state.plan_dirty, "the renderer must pick the change up"); + assert!(!state.plan_expanded); + assert!(!state.is_overlay_active()); + } + #[test] fn info_message_auto_dismisses_after_deadline() { use std::time::{Duration, Instant}; From 67455230504b550f748efbbafd6cb8aac048fbaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Sun, 26 Jul 2026 20:50:05 +0200 Subject: [PATCH 2/2] cargo fmt --- crates/ui_terminal/src/renderer.rs | 6 ++++-- crates/ui_terminal/src/tool_renderers/sub_agent_renderer.rs | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/ui_terminal/src/renderer.rs b/crates/ui_terminal/src/renderer.rs index 017dc33c..c6970183 100644 --- a/crates/ui_terminal/src/renderer.rs +++ b/crates/ui_terminal/src/renderer.rs @@ -958,7 +958,8 @@ impl TerminalRenderer { let entry = &mut status_entries[idx]; let max_height = status_budget.min(MAX_STATUS_ENTRY_HEIGHT); - let actual_height = Self::measure_markdown_height(&entry.content, width, max_height); + let actual_height = + Self::measure_markdown_height(&entry.content, width, max_height); entry.height = actual_height; if actual_height > 0 { @@ -2330,7 +2331,8 @@ mod tests { let buffer = renderer.buffer(); let spinner_row = find_spinner_row(buffer).expect("spinner must be rendered"); - let plan_row = find_row(buffer, "Plan: Update documentation").expect("plan must render"); + let plan_row = + find_row(buffer, "Plan: Update documentation").expect("plan must render"); assert!( spinner_row < plan_row, "spinner (row {spinner_row}) must be above the plan (row {plan_row})" diff --git a/crates/ui_terminal/src/tool_renderers/sub_agent_renderer.rs b/crates/ui_terminal/src/tool_renderers/sub_agent_renderer.rs index 2b95eaf3..fece9302 100644 --- a/crates/ui_terminal/src/tool_renderers/sub_agent_renderer.rs +++ b/crates/ui_terminal/src/tool_renderers/sub_agent_renderer.rs @@ -65,8 +65,7 @@ fn sub_agent_lines(tool_block: &ToolUseBlock) -> Vec> { ), ]; if let Some(instructions) = tool_block.parameters.get("instructions") { - let summary = - truncate_with_ellipsis(instructions.value.trim(), INSTRUCTIONS_SUMMARY_LEN); + let summary = truncate_with_ellipsis(instructions.value.trim(), INSTRUCTIONS_SUMMARY_LEN); if !summary.is_empty() { header.push(Span::styled( format!(": {summary}"),