diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index f7a94059..3d2d1cd5 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -11,11 +11,21 @@ use ratatui::style::Style; use ratatui::widgets::*; use serde_json::Value as JsonValue; use std::cell::Cell as StdCell; +use std::cell::Cell; use std::collections::HashMap; use std::path::PathBuf; +/// A single history entry plus its cached wrapped-line count. +struct Entry { + cell: HistoryCell, + line_count: Cell, +} + pub struct ConversationHistoryWidget { - history: Vec, + entries: Vec, + /// The width (in terminal cells/columns) that [`Entry::line_count`] was + /// computed for. When the available width changes we recompute counts. + cached_width: StdCell, scroll_position: usize, /// Number of lines the last time render_ref() was called num_rendered_lines: StdCell, @@ -27,7 +37,8 @@ pub struct ConversationHistoryWidget { impl ConversationHistoryWidget { pub fn new() -> Self { Self { - history: Vec::new(), + entries: Vec::new(), + cached_width: StdCell::new(0), scroll_position: usize::MAX, num_rendered_lines: StdCell::new(0), last_viewport_height: StdCell::new(0), @@ -73,7 +84,7 @@ impl ConversationHistoryWidget { fn scroll_up(&mut self, num_lines: u32) { // If a user is scrolling up from the "stick to bottom" mode, we need to - // map this to a specific scroll position so we can caluate the delta. + // map this to a specific scroll position so we can calculate the delta. // This requires us to care about how tall the screen is. if self.scroll_position == usize::MAX { self.scroll_position = self @@ -97,9 +108,7 @@ impl ConversationHistoryWidget { // Compute the maximum explicit scroll offset that still shows a full // viewport. This mirrors the calculation in `scroll_page_down()` and // in the render path. - let max_scroll = num_rendered_lines - .saturating_sub(viewport_height) - .saturating_add(1); + let max_scroll = num_rendered_lines.saturating_sub(viewport_height); let new_pos = self.scroll_position.saturating_add(num_lines as usize); @@ -144,7 +153,7 @@ impl ConversationHistoryWidget { // Calculate the maximum explicit scroll offset that is still within // range. This matches the logic in `scroll_down()` and the render // method. - let max_scroll = num_lines.saturating_sub(viewport_height).saturating_add(1); + let max_scroll = num_lines.saturating_sub(viewport_height); // Attempt to move down by a full page. let new_pos = self.scroll_position.saturating_add(viewport_height); @@ -166,7 +175,7 @@ impl ConversationHistoryWidget { /// Note `model` could differ from `config.model` if the agent decided to /// use a different model than the one requested by the user. pub fn add_session_info(&mut self, config: &Config, event: SessionConfiguredEvent) { - let is_first_event = self.history.is_empty(); + let is_first_event = self.entries.is_empty(); self.add_to_history(HistoryCell::new_session_info(config, event, is_first_event)); } @@ -216,12 +225,22 @@ impl ConversationHistoryWidget { } fn add_to_history(&mut self, cell: HistoryCell) { - self.history.push(cell); + let width = self.cached_width.get(); + let count = if width > 0 { + wrapped_line_count_for_cell(&cell, width) + } else { + 0 + }; + + self.entries.push(Entry { + cell, + line_count: Cell::new(count), + }); } /// Remove all history entries and reset scrolling. pub fn clear(&mut self) { - self.history.clear(); + self.entries.clear(); self.scroll_position = usize::MAX; } @@ -232,7 +251,9 @@ impl ConversationHistoryWidget { stderr: String, exit_code: i32, ) { - for cell in self.history.iter_mut() { + let width = self.cached_width.get(); + for entry in self.entries.iter_mut() { + let cell = &mut entry.cell; if let HistoryCell::ActiveExecCommand { call_id: history_id, command, @@ -250,6 +271,13 @@ impl ConversationHistoryWidget { duration: start.elapsed(), }, ); + + // Update cached line count. + if width > 0 { + entry + .line_count + .set(wrapped_line_count_for_cell(cell, width)); + } break; } } @@ -269,14 +297,15 @@ impl ConversationHistoryWidget { .unwrap_or_else(|_| serde_json::Value::String("".into())) }); - for cell in self.history.iter_mut() { + let width = self.cached_width.get(); + for entry in self.entries.iter_mut() { if let HistoryCell::ActiveMcpToolCall { call_id: history_id, fq_tool_name, invocation, start, .. - } = cell + } = &entry.cell { if &call_id == history_id { let completed = HistoryCell::new_completed_mcp_tool_call( @@ -286,7 +315,14 @@ impl ConversationHistoryWidget { success, result_val, ); - *cell = completed; + entry.cell = completed; + + if width > 0 { + entry + .line_count + .set(wrapped_line_count_for_cell(&entry.cell, width)); + } + break; } } @@ -311,105 +347,102 @@ impl WidgetRef for ConversationHistoryWidget { .border_type(BorderType::Rounded) .border_style(border_style); - // ------------------------------------------------------------------ - // Build a *window* into the history instead of cloning the entire - // history into a brand‑new Vec every time we are asked to render. - // - // There can be an unbounded number of `Line` objects in the history, - // but the terminal will only ever display `height` of them at once. - // By materialising only the `height` lines that are scrolled into - // view we avoid the potentially expensive clone of the full - // conversation every frame. - // ------------------------------------------------------------------ - // Compute the inner area that will be available for the list after // the surrounding `Block` is drawn. let inner = block.inner(area); let viewport_height = inner.height as usize; - // Collect the lines that will actually be visible in the viewport - // while keeping track of the total number of lines so the scrollbar - // stays correct. - let num_lines: usize = self.history.iter().map(|c| c.lines().len()).sum(); + // Cache (and if necessary recalculate) the wrapped line counts for + // every [`HistoryCell`] so that our scrolling math accounts for text + // wrapping. + let width = inner.width; // Width of the viewport in terminal cells. + if width == 0 { + return; // Nothing to draw – avoid division by zero. + } - let max_scroll = num_lines.saturating_sub(viewport_height) + 1; + // Recompute cache if the width changed. + let num_lines: usize = if self.cached_width.get() != width { + self.cached_width.set(width); + + let mut num_lines: usize = 0; + for entry in &self.entries { + let count = wrapped_line_count_for_cell(&entry.cell, width); + num_lines += count; + entry.line_count.set(count); + } + num_lines + } else { + self.entries.iter().map(|e| e.line_count.get()).sum() + }; + + // Determine the scroll position. Note the existing value of + // `self.scroll_position` could exceed the maximum scroll offset if the + // user made the window wider since the last render. + let max_scroll = num_lines.saturating_sub(viewport_height); let scroll_pos = if self.scroll_position == usize::MAX { max_scroll } else { self.scroll_position.min(max_scroll) }; - let mut visible_lines: Vec> = Vec::with_capacity(viewport_height); + // ------------------------------------------------------------------ + // Build a *window* into the history so we only clone the `Line`s that + // may actually be visible in this frame. We still hand the slice off + // to a `Paragraph` with an additional scroll offset to avoid slicing + // inside a wrapped line (we don’t have per-subline granularity). + // ------------------------------------------------------------------ - if self.scroll_position == usize::MAX { - // Stick‑to‑bottom mode: walk the history backwards and keep the - // most recent `height` lines. This touches at most `height` - // lines regardless of how large the conversation grows. - 'outer_rev: for cell in self.history.iter().rev() { - for line in cell.lines().iter().rev() { - visible_lines.push(line.clone()); - if visible_lines.len() == viewport_height { - break 'outer_rev; - } - } + // Find the first entry that intersects the current scroll position. + let mut cumulative = 0usize; + let mut first_idx = 0usize; + for (idx, entry) in self.entries.iter().enumerate() { + let next = cumulative + entry.line_count.get(); + if next > scroll_pos { + first_idx = idx; + break; } - visible_lines.reverse(); - } else { - // Arbitrary scroll position. Skip lines until we reach the - // desired offset, then emit the next `height` lines. - let start_line = scroll_pos; - let mut current_index = 0usize; - 'outer_fwd: for cell in &self.history { - for line in cell.lines() { - if current_index >= start_line { - visible_lines.push(line.clone()); - if visible_lines.len() == viewport_height { - break 'outer_fwd; - } - } - current_index += 1; - } + cumulative = next; + } + + let offset_into_first = scroll_pos - cumulative; + + // Collect enough raw lines from `first_idx` onward to cover the + // viewport. We may fetch *slightly* more than necessary (whole cells) + // but never the entire history. + let mut collected_wrapped = 0usize; + let mut visible_lines: Vec> = Vec::new(); + + for entry in &self.entries[first_idx..] { + visible_lines.extend(entry.cell.lines().iter().cloned()); + collected_wrapped += entry.line_count.get(); + if collected_wrapped >= offset_into_first + viewport_height { + break; } } - // We track the number of lines in the struct so can let the user take over from - // something other than usize::MAX when they start scrolling up. This could be - // removed once we have the vec in self. - self.num_rendered_lines.set(num_lines); - self.last_viewport_height.set(viewport_height); + // Build the Paragraph with wrapping enabled so long lines are not + // clipped. Apply vertical scroll so that `offset_into_first` wrapped + // lines are hidden at the top. + let paragraph = Paragraph::new(visible_lines) + .block(block) + .wrap(wrap_cfg()) + .scroll((offset_into_first as u16, 0)); - // The widget takes care of drawing the `block` and computing its own - // inner area, so we render it over the full `area`. - // We *manually* sliced the set of `visible_lines` to fit within the - // viewport above, so there is no need to ask the `Paragraph` widget - // to apply an additional scroll offset. Doing so would cause the - // content to be shifted *twice* – once by our own logic and then a - // second time by the widget – which manifested as the entire block - // drifting off‑screen when the user attempted to scroll. - - // Currently, we do not use the `wrap` method on the `Paragraph` widget - // because it messes up our scrolling math above that assumes each Line - // contributes one line of height to the widget. Admittedly, this is - // bad because users cannot see content that is clipped without - // resizing the terminal. - let paragraph = Paragraph::new(visible_lines).block(block); paragraph.render(area, buf); + // Draw scrollbar if necessary. let needs_scrollbar = num_lines > viewport_height; if needs_scrollbar { let mut scroll_state = ScrollbarState::default() - // TODO(ragona): - // I don't totally understand this, but it appears to work exactly as expected - // if we set the content length as the lines minus the height. Maybe I was supposed - // to use viewport_content_length or something, but this works and I'm backing away. + // The Scrollbar widget expects the *content* height minus the + // viewport height, mirroring the calculation used previously. .content_length(num_lines.saturating_sub(viewport_height)) .position(scroll_pos); - // Choose a thumb colour that stands out only when this pane has focus so that the + // Choose a thumb color that stands out only when this pane has focus so that the // user’s attention is naturally drawn to the active viewport. When unfocused we show // a low‑contrast thumb so the scrollbar fades into the background without becoming // invisible. - let thumb_style = if self.has_input_focus { Style::reset().fg(Color::LightYellow) } else { @@ -418,25 +451,25 @@ impl WidgetRef for ConversationHistoryWidget { StatefulWidget::render( // By default the Scrollbar widget inherits the style that was already present - // in the underlying buffer cells. That means if a coloured line (for example a + // in the underlying buffer cells. That means if a colored line (for example a // background task notification that we render in blue) happens to be underneath - // the scrollbar, the track and thumb adopt that colour and the scrollbar appears - // to “change colour”. Explicitly setting the *track* and *thumb* styles ensures + // the scrollbar, the track and thumb adopt that color and the scrollbar appears + // to "change color." Explicitly setting the *track* and *thumb* styles ensures // we always draw the scrollbar with the same palette regardless of what content // is behind it. // - // N.B. Only the *foreground* colour matters here because the scrollbar symbols + // N.B. Only the *foreground* color matters here because the scrollbar symbols // themselves are filled‐in block glyphs that completely overwrite the prior - // character cells. We therefore leave the background at its default value so it + // character cells. We therefore leave the background at its default value so it // blends nicely with the surrounding `Block`. Scrollbar::new(ScrollbarOrientation::VerticalRight) .begin_symbol(Some("↑")) .end_symbol(Some("↓")) .begin_style(Style::reset().fg(Color::DarkGray)) .end_style(Style::reset().fg(Color::DarkGray)) - // A solid thumb so that we can colour it distinctly from the track. + // A solid thumb so that we can color it distinctly from the track. .thumb_symbol("█") - // Apply the dynamic thumb colour computed above. We still start from + // Apply the dynamic thumb color computed above. We still start from // Style::reset() to clear any inherited modifiers. .thumb_style(thumb_style) // Thin vertical line for the track. @@ -447,5 +480,25 @@ impl WidgetRef for ConversationHistoryWidget { &mut scroll_state, ); } + + // Update auxiliary stats that the scroll handlers rely on. + self.num_rendered_lines.set(num_lines); + self.last_viewport_height.set(viewport_height); } } + +/// Common [`Wrap`] configuration used for both measurement and rendering so +/// they stay in sync. +#[inline] +const fn wrap_cfg() -> ratatui::widgets::Wrap { + ratatui::widgets::Wrap { trim: false } +} + +/// Returns the wrapped line count for `cell` at the given `width` using the +/// same wrapping rules that `ConversationHistoryWidget` uses during +/// rendering. +fn wrapped_line_count_for_cell(cell: &HistoryCell, width: u16) -> usize { + Paragraph::new(cell.lines().clone()) + .wrap(wrap_cfg()) + .line_count(width) +}