feat: per-cell thinking fold/unfold via Space key#2385
Conversation
Previously, Space on a thinking cell hid it entirely from the transcript. Now Space toggles between folded (summary preview) and unfolded (full content) for thinking cells, while other cells retain the existing hide/show behavior. Changes: - Add folded_thinking HashSet to App for per-cell fold tracking - Add lines_with_options_folded / lines_with_copy_metadata_folded that accept an explicit folded flag, overriding the global verbose setting - Update transcript cache to pass fold state during rendering - Update Space key handler to toggle fold for thinking cells - Update affordance text to mention Space for expanding folded thinking Closes Hmbown#2348
There was a problem hiding this comment.
Code Review
This pull request introduces a per-cell folding feature for thinking cells in the TUI, allowing users to toggle individual thinking blocks between a collapsed summary and full content using the Space key. The feedback focuses on improving the toggle logic and user experience: first, by using an XOR operation (folded ^ !options.verbose) to allow unfolding individual blocks even when global verbose mode is off; second, by updating the Space key handler to support this relative toggle behavior; and third, by dynamically updating the footer label to show 'Space to fold' or 'Space to expand' depending on the block's current state.
| *streaming, | ||
| *duration_secs, | ||
| !options.verbose, | ||
| folded || !options.verbose, |
There was a problem hiding this comment.
Using folded || !options.verbose means that when global verbose mode is off (!options.verbose is true), the expression is always true. As a result, individual thinking blocks can never be unfolded when verbose mode is off, even though pressing the Space key will still toggle the state in folded_thinking and show misleading status messages like 'Thinking block expanded'.
By changing this to folded ^ !options.verbose (XOR), we can treat folded_thinking as a toggle relative to the default state. This allows users to fold individual blocks when verbose mode is on, and unfold individual blocks when verbose mode is off, making the feature truly independent of the global setting.
| folded || !options.verbose, | |
| folded ^ !options.verbose, |
| if is_thinking { | ||
| if app.folded_thinking.contains(&idx) { | ||
| app.folded_thinking.remove(&idx); | ||
| app.status_message = | ||
| Some("Thinking block expanded".to_string()); | ||
| } else { | ||
| app.folded_thinking.insert(idx); | ||
| app.status_message = | ||
| Some("Thinking block folded".to_string()); | ||
| } |
There was a problem hiding this comment.
To support the XOR-based toggle logic that allows folding/unfolding independent of the global verbose setting, we should update the Space key handler to toggle the presence of the cell index in folded_thinking relative to the current visual state.
if is_thinking {
let is_currently_folded = app.folded_thinking.contains(&idx) ^ !app.transcript_render_options().verbose;
if is_currently_folded {
if app.transcript_render_options().verbose {
app.folded_thinking.remove(&idx);
} else {
app.folded_thinking.insert(idx);
}
app.status_message = Some("Thinking block expanded".to_string());
} else {
if app.transcript_render_options().verbose {
app.folded_thinking.insert(idx);
} else {
app.folded_thinking.remove(&idx);
}
app.status_message = Some("Thinking block folded".to_string());
}
}| let label = if streaming { | ||
| "More reasoning in Ctrl+O" | ||
| } else { | ||
| "Full reasoning in Ctrl+O" | ||
| "Space to expand · Full reasoning in Ctrl+O" | ||
| }; |
There was a problem hiding this comment.
Since the bottom rail/footer is shown in both folded and unfolded states, hardcoding 'Space to expand' when the block is already unfolded is misleading. It should dynamically show 'Space to fold' (or 'Space to collapse') when unfolded, and 'Space to expand' when folded.
Note: If the fourth parameter of render_thinking is named differently (e.g., folded or summary), please adjust the parameter name in the suggestion accordingly.
let label = if streaming {
"More reasoning in Ctrl+O"
} else if collapsed {
"Space to expand · Full reasoning in Ctrl+O"
} else {
"Space to fold · Full reasoning in Ctrl+O"
};|
Thanks for pushing on this. The foldable thinking UI is exactly the kind of readability polish CodeWhale needs, but I am leaving this open for one more pass before merge. Two things look important to tighten: the toggle updates Next best step: add a focused regression for folded-thinking cache invalidation and filtered/original index handling, then run |
Change from to so Space toggles the collapsed state relative to the global verbose flag: - verbose off (default): thinking collapsed; Space unfolds it - verbose on: thinking expanded; Space folds it This lets users expand individual thinking blocks even when the global verbose mode is off, without needing to toggle verbose for all blocks.
|
Updated to use XOR for the fold toggle, so Space works relative to the global verbose setting:
|
Two bugs in the folded-thinking rendering path: 1. Cache invalidation: changing folded_thinking did not invalidate cached cell renders because the cell revision stayed the same. Now track folded_cells in the cache and force re-render when it changes. 2. Index mapping: in the slow path (with collapsed cells), the cache idx is a filtered position, but folded_cells uses original virtual indices. Add original_index_map parameter to ensure_split so fold lookups resolve correctly. Added two regression tests: - folded_thinking_cache_invalidation: verifies fold/unfold triggers re-render - folded_thinking_with_collapsed_cells_uses_original_indices: verifies correct fold behavior when collapsed_cells filtering is active
|
Fixed both issues:
Added two regression tests as requested:
cargo test -p codewhale-tui folded_thinking -- --nocapture passes cleanly. |
|
Thanks @HUQIANTAO — the follow-up fixed the cache/index pieces I was worried about, and I pushed a tiny maintainer polish on top so the regression stays warning-free after the main-branch update.\n\nWhat I verified on the updated head:\n- merged cleanly with current |
Problem
Thinking output is long and takes up a lot of space in the conversation view, making it hard to read the actual assistant responses. Users want to fold/collapse individual thinking blocks to see summaries instead of full content.
Solution
Add per-cell thinking fold/unfold via the Space key. When the composer is empty and the cursor is on a thinking cell, Space toggles between folded (summary preview) and unfolded (full content). Other cells retain the existing hide/show behavior.
This is independent of the global
/verbosetoggle — even when verbose mode is on, individual thinking blocks can be folded.Changes
folded_thinking: HashSet<usize>to App for per-cell fold trackinglines_with_options_folded/lines_with_copy_metadata_foldedmethods that accept an explicitfoldedflag, overriding the global verbose settingCloses #2348
Greptile Summary
This PR adds per-cell fold/unfold of thinking blocks via the Space key. It introduces a
folded_thinking: HashSet<usize>onApp, newlines_with_options_folded/lines_with_copy_metadata_foldedrendering methods, and plumbs fold state through the transcript cache'sensure_splitcall so that toggling fold state properly invalidates cached renders.verboseflag so Space always toggles the current visual state regardless of whether verbose mode is on or off.filtered_to_originalmap asoriginal_index_mapso fold-set lookups use original virtual indices.Confidence Score: 3/5
Two correctness bugs in the Space key handler need fixing before this ships: one causes inverted status messages under the default verbose=false setting, and the other looks up the wrong history cell when any cell is collapsed.
The Space handler has two defects on the changed path. First, the XOR-based collapse logic means folded_thinking.insert() expands the cell when verbose is off (the default), so every user who presses Space sees a status message that says the opposite of what happened visually. Second, app.history.get(idx) receives the filtered position index from line_meta rather than the original virtual index; in the slow path this reads the wrong slot in history, making is_thinking unreliable and storing an incorrect index in folded_thinking. The cache invalidation logic and index mapping in transcript.rs and widgets/mod.rs are correct on their own.
crates/tui/src/tui/ui.rs — both bugs are in the Space key handler block.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant SpaceHandler as Space Key Handler (ui.rs) participant App participant Cache as TranscriptViewCache (transcript.rs) participant Renderer as HistoryCell::lines_with_options_folded (history.rs) User->>SpaceHandler: Press Space on thinking cell SpaceHandler->>App: detail_target_cell_index(app) to idx Note over SpaceHandler: Fast path: idx == original. Slow path: idx == filtered (bug) SpaceHandler->>App: history.get(idx) to is_thinking SpaceHandler->>App: folded_thinking.insert/remove(idx) SpaceHandler->>App: "mark_history_updated + needs_redraw=true" User->>Cache: Next frame render Cache->>Cache: folded_changed check alt folded state changed Cache->>Cache: per_cell.clear full re-render end loop each cell Cache->>Cache: original_idx via original_index_map Cache->>Cache: "folded = folded_cells.contains(original_idx)" Cache->>Renderer: lines_with_copy_metadata_folded(width, options, folded) Renderer->>Renderer: "collapsed = folded XOR not verbose" Renderer-->>Cache: rendered lines end Cache-->>User: Updated transcript viewComments Outside Diff (1)
crates/tui/src/tui/widgets/mod.rs, line 201-208 (link)In the slow path,
filtered_cellsis built by excluding collapsed cells — so its sequential indices (0, 1, 2…) no longer correspond to original virtual indices. Butfolded_thinkingstores original virtual indices, and insideensure_splitthe lookup isfolded_cells.contains(&idx)whereidxis the position in the filtered set.Consider: cells at original indices [0=User, 1=Tool(collapsed), 2=Thinking(folded)]. After filtering, the thinking cell is at filtered index 1, but
folded_thinkingcontains{2}.folded_cells.contains(&1)returnsfalse, so the cell renders unfolded.The fast path is unaffected (no filtering). The slow path should translate each filtered index back to its original index before the lookup, using the
filtered_to_originalmap that is already being built inChatWidget::new.Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile