fix(ui): Ctrl+O expands thinking blocks still in active_cell
After `ThinkingComplete` the finalized thinking entry sits in `app.active_cell` with `streaming = false` until the active cell flushes to history at end-of-turn. During that window the transcript rendered the "thinking collapsed; press Ctrl+O for full text" affordance from `render_thinking`, but `open_thinking_pager` only searched `app.history` — so the handler surfaced "No thinking blocks to expand" while pointing at the affordance. The affordance was truthful; the handler was lying. Routed the lookup through `cell_at_virtual_index` / `virtual_cell_count`, the existing virtual-index API that `open_tool_details_pager` already uses for the same active-cell window. The selection-based path resolves through the virtual index too, so dragging into an in-flight thinking block and pressing Ctrl+O now works as well. Regression guard: `open_thinking_pager_finds_thinking_in_active_cell` drives the entry into active_cell, finalizes it so the "collapsed" affordance is what render_thinking emits, then asserts Ctrl+O pushes the Pager view instead of surfacing the "No thinking blocks" status.
This commit is contained in:
@@ -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 <name>` while the root `/` menu stays
|
||||
|
||||
+24
-16
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user