fix: squash blank lines in transcript render

[rollysys]

Summary

Detail pane felt "空旷" — excessive blank space between and inside events. Three sources of blanks compounded:

• every event had a trailing blank separator appended by `render_events`
• user prompts and assistant markdown often contain `\n\n\n` runs, each producing its own blank `Line`
• pulldown-cmark paragraph breaks emit blank separators too

Fix

Single-pass renderer (`render_detail`) that squashes any run of blank lines down to one. The per-event trailing blank becomes a no-op when the event already ended on a blank, so the separator between events is exactly one blank instead of two-or-more.

Same function now returns both the rendered `Line` buffer and the per-event row offsets, so `draw_detail` only renders once per frame instead of the old double-render (`render_events` + `event_row_offsets` separately). The old private `render_events` / `event_row_offsets` wrappers are dropped — all call sites go through `render_detail` now.

Stacked on

• fix: transcript scroll cap uses real render-line count #8 (`fix/transcript-scroll-cap`)
• feat: default-fold long tool_result / system / thinking in transcript #9 (`feat/transcript-fold`)

This PR auto-rebases when those land.

Validated

• `cargo build --release` clean on Mac + xserver
• `--dry-run` boots both sides
• Visual tightening verified with a session that has heavy `\n\n` usage in prompts

Test plan

• Open a session with long user prompts containing double/triple newlines → no gaping whitespace runs
• Headers still clearly separated from body / from each other (one blank between events preserved)
• Fold / unfold (`z`, `x`) still places the cursor correctly — offsets match the squashed layout
• Session grouping `[` / `]` navigation still lands on the right event (pending_jump resolves via the same offsets)

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added z keybinding to toggle between smart fold mode (collapses long tool_result, system, and thinking events) and expand all view
  • Added x keybinding to toggle fold state of the event at current scroll cursor in smart mode

• Documentation

  • Updated README files with new keybinding descriptions for the Sessions view

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces a folding feature for the Sessions view in the TUI. It adds keybindings (z to toggle fold mode, x to toggle fold state for individual events), implements smart-folding logic that collapses long ToolResult, System, and Thinking events, and refactors the transcript detail rendering pipeline to track per-event row offsets.

Changes

Cohort / File(s)	Summary
Documentation README.md, README.zh.md	Added keybinding documentation for fold mode toggle (z) and per-event fold toggle (x) in Sessions view.
Core Feature Implementation src/tui.rs	Introduced FoldMode enum (Smart, Expanded) and corresponding app state (detail_row_count, fold_mode, fold_overrides, detail_event_offsets). Added keybinding handlers for z/Z (toggle fold mode) and x/X (toggle fold state at scroll cursor). Refactored rendering pipeline: draw_detail now calls render_detail(...) which computes and returns both rendered lines and per-event row offsets. Implemented smart-fold logic that collapses ToolResult, System, and Thinking events exceeding FOLD_THRESHOLD_ROWS unless overridden. Replaced scroll clamping to use computed detail_row_count instead of transcript length proxy. Removed old render_events and event_row_offsets functions, replacing with render_detail and helper utilities.

Possibly Related PRs

• tui: render assistant/user transcript bodies as markdown #5: Directly modifies the transcript-detail rendering pipeline in src/tui.rs, overlapping with the refactored render_detail implementation.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

---

A folding feature hopped into view,
Smart collapses, offsets true,
Press z and x with glee,
Events fold how they should be! 🐰✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'fix: squash blank lines in transcript render' accurately describes the main objective of consolidating blank-line squashing, but the changeset also includes significant new features (fold mode toggling, fold overrides, per-event rendering) that represent a larger scope than just squashing blank lines.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/transcript-spacing

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/tui.rs (1)

1686-1717: ⚠️ Potential issue | 🟡 Minor

Clear detail_event_offsets when no transcript is currently rendered.

The offsets cache is only refreshed on the Some(events) branch. If the user switches to a session that's still debounced/loading, or transcript loading fails, transcript_for points at the new session while detail_event_offsets still belongs to the old one; the next x press then writes a bogus fold override for the wrong event index. Reset the offsets in the previewing… and fallback branches.

Suggested fix

         } else if self.pending_preview.is_some() {
+            self.detail_event_offsets.clear();
             vec![Line::from(Span::styled(
                 "previewing…",
                 Style::default().fg(Color::DarkGray),
             ))]
         } else {
+            self.detail_event_offsets.clear();
             vec![
                 Line::from("↑/↓ select · auto preview"),
                 Line::from("Tab to scroll · q quit · r refresh"),
             ]
         };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tui.rs` around lines 1686 - 1717, The code only updates
self.detail_event_offsets inside the Some(events) branch from render_detail,
leaving stale offsets when cur_t is None; update the two other branches (the
pending_preview branch and the final fallback branch) to explicitly clear the
cache by setting self.detail_event_offsets = Vec::new() (or Default::default())
before returning their rendered Vec<Line>, so handlers that consult
detail_event_offsets (e.g., the x handler / pending_jump logic) won't see
offsets from a different transcript.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/tui.rs`:
- Around line 2401-2411: render_one_event(ev, w) is being called before you
decide to fold, causing expensive rendering on every frame; change the flow to
first cheaply count/display-row estimate (e.g., add a helper like
count_display_rows_or_overflow(ev, w, limit)) that returns whether it would
exceed the fold threshold and how many rows up to a small cutoff, then use that
to compute foldable/should_fold (using existing is_foldable/FoldMode/overrides
logic) and only call render_one_event(ev, w) to build the full
Vec<Line<'static>> when the event will actually be expanded; keep
folded_placeholder(ev, buf.len()) for the folded case but compute buf.len() from
the cheap helper or avoid needing the full buffer when folded.
- Around line 1045-1059: toggle_fold_at_scroll can pick the previous event when
the viewport is on the blank separator row inserted by render_detail; change the
selection logic so it only toggles when scroll falls inside an event span. After
computing ev_idx from detail_event_offsets, verify that scroll is >=
detail_event_offsets[ev_idx] and < detail_event_offsets.get(ev_idx +
1).unwrap_or(INF) (or otherwise compute an end row/span per event); if the check
fails (i.e. scroll points at a separator or beyond the event end) simply return
without mutating fold_overrides. Keep references to toggle_fold_at_scroll,
detail_event_offsets, render_detail, fold_overrides and scroll when implementing
the guard.

---

Outside diff comments:
In `@src/tui.rs`:
- Around line 1686-1717: The code only updates self.detail_event_offsets inside
the Some(events) branch from render_detail, leaving stale offsets when cur_t is
None; update the two other branches (the pending_preview branch and the final
fallback branch) to explicitly clear the cache by setting
self.detail_event_offsets = Vec::new() (or Default::default()) before returning
their rendered Vec<Line>, so handlers that consult detail_event_offsets (e.g.,
the x handler / pending_jump logic) won't see offsets from a different
transcript.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 40e7b32a-e2e8-4b40-b7e3-f515ea0b24a1

📥 Commits

Reviewing files that changed from the base of the PR and between 6e0d95f and 2140d74.

📒 Files selected for processing (3)

• README.md
• README.zh.md
• src/tui.rs

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

x can toggle the wrong event when the viewport starts on a separator line.

detail_event_offsets only tracks event start rows, but render_detail always inserts a blank separator after each event on Lines 2422-2427. When scroll lands on that blank row, rposition(|&o| o <= scroll) resolves to the previous event, so x mutates the wrong override instead of behaving like the documented no-op outside an event. Cache an end row/span per event, or explicitly skip separator rows before changing fold_overrides.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tui.rs` around lines 1045 - 1059, toggle_fold_at_scroll can pick the
previous event when the viewport is on the blank separator row inserted by
render_detail; change the selection logic so it only toggles when scroll falls
inside an event span. After computing ev_idx from detail_event_offsets, verify
that scroll is >= detail_event_offsets[ev_idx] and <
detail_event_offsets.get(ev_idx + 1).unwrap_or(INF) (or otherwise compute an end
row/span per event); if the check fails (i.e. scroll points at a separator or
beyond the event end) simply return without mutating fold_overrides. Keep
references to toggle_fold_at_scroll, detail_event_offsets, render_detail,
fold_overrides and scroll when implementing the guard.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Folded events still incur the full render cost on every frame.

render_one_event(ev, w) materializes the entire event before you decide whether to replace it with a one-line placeholder. Since draw_detail calls render_detail every repaint, a huge folded tool_result / system / thinking block still pays the full wrapping/highlighting/allocation cost every frame, which will make scrolling and search preview noticeably choppy on large transcripts. Count rows with an early cutoff first, then only build Vec<Line> when the event is actually shown expanded.

Refactor sketch

-        let buf = render_one_event(ev, w);
-        let foldable = is_foldable(ev.kind, buf.len());
-        let should_fold = match fold_mode {
-            FoldMode::Expanded => false,
-            FoldMode::Smart => foldable && !overrides.contains(&i),
-        };
-        let event_lines: Vec<Line<'static>> = if should_fold {
-            vec![folded_placeholder(ev, buf.len())]
+        let can_fold = matches!(fold_mode, FoldMode::Smart)
+            && !overrides.contains(&i)
+            && matches!(
+                ev.kind,
+                TranscriptKind::ToolResult | TranscriptKind::System | TranscriptKind::Thinking
+            );
+        let row_count = if can_fold {
+            rendered_row_count_capped(ev, w, FOLD_THRESHOLD_ROWS + 1)
+        } else {
+            0
+        };
+        let should_fold = can_fold && row_count > FOLD_THRESHOLD_ROWS;
+        let event_lines: Vec<Line<'static>> = if should_fold {
+            vec![folded_placeholder(ev, row_count)]
         } else {
-            buf
+            render_one_event(ev, w)
         };

This needs a helper outside this hunk, but it avoids allocating/rendering the full body for events that stay folded.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tui.rs` around lines 2401 - 2411, render_one_event(ev, w) is being called
before you decide to fold, causing expensive rendering on every frame; change
the flow to first cheaply count/display-row estimate (e.g., add a helper like
count_display_rows_or_overflow(ev, w, limit)) that returns whether it would
exceed the fold threshold and how many rows up to a small cutoff, then use that
to compute foldable/should_fold (using existing is_foldable/FoldMode/overrides
logic) and only call render_one_event(ev, w) to build the full
Vec<Line<'static>> when the event will actually be expanded; keep
folded_placeholder(ev, buf.len()) for the folded case but compute buf.len() from
the cheap helper or avoid needing the full buffer when folded.