diff --git a/CHANGELOG.md b/CHANGELOG.md index f97c0a26..9482793e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,19 @@ coverage additions. `reasoning_content` replay, preventing second-turn HTTP 400s after tool calls when users keep the onboarding default model alias. +- **`Ctrl+O` expands thinking blocks still in flight.** + `open_thinking_pager` only searched `app.history`, but after + `ThinkingComplete` the finalized thinking entry sits in + `app.active_cell` with `streaming = false` until the active cell + flushes at end-of-turn. During that window the transcript still + rendered the "thinking collapsed; press Ctrl+O for full text" + affordance from `render_thinking`, so the handler surfaced + "No thinking blocks to expand" while the affordance pointed at + it. Routed through the existing `cell_at_virtual_index` / + `virtual_cell_count` resolver that `open_tool_details_pager` + already uses; selection-based and most-recent lookups both reach + in-flight entries now. Regression-guarded by + `open_thinking_pager_finds_thinking_in_active_cell`. - **Skill completions no longer flood the top-level slash menu** (#1437, PR #1442 from **@reidliu41**). Installed skills now complete under `/skill ` while the root `/` menu stays diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 0bd1ea5d..34fa5779 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -8493,8 +8493,16 @@ fn open_pager_for_last_message(app: &mut App) -> bool { /// Open a pager showing the full thinking block. Targets the cell at the /// current selection if it's a Thinking cell; otherwise falls back to the -/// most recent Thinking cell in history. Bound to Ctrl+O so users can read -/// reasoning content that's been collapsed in calm-mode rendering. +/// most recent Thinking cell across the virtual transcript (history + +/// in-flight `active_cell`). Bound to Ctrl+O so users can read reasoning +/// content that's been collapsed in calm-mode rendering. +/// +/// The virtual-index lookup matters: after `ThinkingComplete` fires the +/// finalized thinking entry sits in `active_cell` with `streaming = false` +/// until the active cell flushes to history. During that window the +/// transcript already renders the "thinking collapsed; press Ctrl+O for +/// full text" affordance, so the handler must address active-cell entries +/// or the affordance becomes a lie. fn open_thinking_pager(app: &mut App) -> bool { let selected_cell = app .viewport @@ -8510,23 +8518,18 @@ fn open_thinking_pager(app: &mut App) -> bool { }) .filter(|&idx| { matches!( - app.history.get(idx), + app.cell_at_virtual_index(idx), Some(crate::tui::history::HistoryCell::Thinking { .. }) ) }); let target_idx = selected_cell.or_else(|| { - app.history - .iter() - .enumerate() - .rev() - .find_map(|(idx, cell)| { - if matches!(cell, crate::tui::history::HistoryCell::Thinking { .. }) { - Some(idx) - } else { - None - } - }) + (0..app.virtual_cell_count()).rev().find(|&idx| { + matches!( + app.cell_at_virtual_index(idx), + Some(crate::tui::history::HistoryCell::Thinking { .. }) + ) + }) }); let Some(idx) = target_idx else { @@ -8534,13 +8537,18 @@ fn open_thinking_pager(app: &mut App) -> bool { return true; }; - let cell = &app.history[idx]; let width = app .viewport .last_transcript_area .map(|area| area.width) .unwrap_or(80); - let text = history_cell_to_text(cell, width); + let text = { + let Some(cell) = app.cell_at_virtual_index(idx) else { + app.status_message = Some("No thinking blocks to expand".to_string()); + return true; + }; + history_cell_to_text(cell, width) + }; app.view_stack.push(PagerView::from_text( "Thinking", &text, diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index 21ec45f5..e7084279 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -3948,6 +3948,42 @@ fn flush_active_cell_finalizes_unclosed_thinking_block() { ); } +#[test] +fn open_thinking_pager_finds_thinking_in_active_cell() { + // After ThinkingComplete fires, the finalized thinking entry stays in + // `app.active_cell` with `streaming = false` until the active cell is + // flushed to history (end-of-turn, or when an assistant text arrives). + // During that window the transcript still renders the + // "thinking collapsed; press Ctrl+O for full text" affordance from + // `render_thinking`, so the handler must reach across the virtual + // transcript — not just `app.history` — or the promise is a lie. + // Regression guard for the v0.8.29 affordance/handler mismatch. + let mut app = create_test_app(); + let _ = ensure_streaming_thinking_active_entry(&mut app); + append_streaming_thinking(&mut app, 0, "deliberating"); + let finalized = finalize_streaming_thinking_active_entry(&mut app, Some(1.2), ""); + assert!(finalized); + assert!( + app.history.is_empty(), + "thinking entry stays in active_cell until flush" + ); + let active = app.active_cell.as_ref().expect("active cell present"); + assert!(matches!( + active.entries().first(), + Some(HistoryCell::Thinking { + streaming: false, + .. + }) + )); + + assert!(open_thinking_pager(&mut app)); + assert_eq!( + app.view_stack.top_kind(), + Some(ModalKind::Pager), + "pager must open for thinking entries still in active_cell" + ); +} + #[test] fn engine_error_finalizes_active_thinking_block() { use crate::error_taxonomy::StreamError;