merge: per-cell transcript line cache + revisions (closes #78)
Resolves conflicts with the #65 resize fix that landed first. Both branches converged on the same resize-coalescing + display-width truncation fix; took the perf branch's more detailed inline comments and combined the transcript bench from #78 with the existing #65 resize regression tests. Issue #78 baseline (release, 5000-cell synthetic transcript): pure scroll, off=0 3549µs → 21µs (~150x) pure scroll, off=2000 3303µs → 19µs (~170x) streaming append 11.6ms → 3.4ms (~3.4x)
This commit is contained in:
@@ -34,7 +34,7 @@ pub fn help(app: &mut App, topic: Option<&str>) -> CommandResult {
|
||||
|
||||
/// Clear conversation history
|
||||
pub fn clear(app: &mut App) -> CommandResult {
|
||||
app.history.clear();
|
||||
app.clear_history();
|
||||
app.mark_history_updated();
|
||||
app.api_messages.clear();
|
||||
app.system_prompt = None;
|
||||
|
||||
@@ -358,7 +358,7 @@ pub fn undo(app: &mut App) -> CommandResult {
|
||||
let mut removed_count = 0;
|
||||
while !app.history.is_empty() {
|
||||
let last_is_user = matches!(app.history.last(), Some(HistoryCell::User { .. }));
|
||||
app.history.pop();
|
||||
app.pop_history();
|
||||
removed_count += 1;
|
||||
if last_is_user {
|
||||
break;
|
||||
|
||||
@@ -83,10 +83,13 @@ pub fn load(app: &mut App, path: Option<&str>) -> CommandResult {
|
||||
};
|
||||
|
||||
app.api_messages.clone_from(&session.messages);
|
||||
app.history.clear();
|
||||
for msg in &app.api_messages {
|
||||
app.history.extend(history_cells_from_message(msg));
|
||||
}
|
||||
app.clear_history();
|
||||
let cells_to_add: Vec<_> = app
|
||||
.api_messages
|
||||
.iter()
|
||||
.flat_map(history_cells_from_message)
|
||||
.collect();
|
||||
app.extend_history(cells_to_add);
|
||||
app.mark_history_updated();
|
||||
app.transcript_selection.clear();
|
||||
app.model.clone_from(&session.metadata.model);
|
||||
|
||||
@@ -367,6 +367,21 @@ pub struct App {
|
||||
pub paste_burst: PasteBurst,
|
||||
pub history: Vec<HistoryCell>,
|
||||
pub history_version: u64,
|
||||
/// Per-cell revision counter, kept in lockstep with `history`. Bumped only
|
||||
/// for the cell whose content actually changed; appended (with a fresh
|
||||
/// value) when a new cell is pushed; truncated when cells are removed. The
|
||||
/// transcript cache compares each entry against its previously rendered
|
||||
/// revision to skip re-wrap on unchanged cells.
|
||||
///
|
||||
/// Critical for transcript scroll perf (issue #78): without per-cell
|
||||
/// revisions, every history mutation forces a full re-render of every
|
||||
/// cell, which scales O(N) with transcript length and stalls the UI when
|
||||
/// scrolled far back.
|
||||
pub history_revisions: Vec<u64>,
|
||||
/// Monotonic counter used to issue fresh per-cell revisions. Wrapping is
|
||||
/// fine — the chance of a wrap-around revision collision in a single
|
||||
/// session is astronomical.
|
||||
pub next_history_revision: u64,
|
||||
pub api_messages: Vec<Message>,
|
||||
pub transcript_scroll: TranscriptScroll,
|
||||
pub pending_scroll_delta: i32,
|
||||
@@ -723,6 +738,8 @@ impl App {
|
||||
paste_burst: PasteBurst::default(),
|
||||
history: Vec::new(),
|
||||
history_version: 0,
|
||||
history_revisions: Vec::new(),
|
||||
next_history_revision: 1,
|
||||
api_messages: Vec::new(),
|
||||
transcript_scroll: TranscriptScroll::to_bottom(),
|
||||
pending_scroll_delta: 0,
|
||||
@@ -968,7 +985,9 @@ impl App {
|
||||
}
|
||||
|
||||
pub fn add_message(&mut self, msg: HistoryCell) {
|
||||
let rev = self.fresh_history_revision();
|
||||
self.history.push(msg);
|
||||
self.history_revisions.push(rev);
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
let selection_has_range = self
|
||||
.transcript_selection
|
||||
@@ -993,9 +1012,104 @@ impl App {
|
||||
|
||||
pub fn mark_history_updated(&mut self) {
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
// Resync per-cell revisions to history.len(). This is the
|
||||
// "I-don't-know-which-cell-changed" path: if cells were appended in
|
||||
// bulk (e.g. session resume, compaction), every new cell gets a
|
||||
// fresh revision; if cells were removed, drop trailing revs. We
|
||||
// intentionally do NOT bump revisions for indices that already had
|
||||
// one — the cache will reuse those. Callers that mutate a specific
|
||||
// cell's content must call `bump_history_cell(idx)` instead.
|
||||
self.resync_history_revisions();
|
||||
self.needs_redraw = true;
|
||||
}
|
||||
|
||||
/// Issue a fresh, monotonically increasing revision counter for a new
|
||||
/// history cell. Wrapping is acceptable — collisions are astronomically
|
||||
/// rare and at worst trigger one extra re-render.
|
||||
fn fresh_history_revision(&mut self) -> u64 {
|
||||
let rev = self.next_history_revision;
|
||||
self.next_history_revision = self.next_history_revision.wrapping_add(1);
|
||||
rev
|
||||
}
|
||||
|
||||
/// Bring `history_revisions` back into shape (`history_revisions.len() ==
|
||||
/// history.len()`). Pushes fresh revs for newly appended cells, truncates
|
||||
/// for cells that were removed. **Does not** invalidate existing entries.
|
||||
pub fn resync_history_revisions(&mut self) {
|
||||
if self.history_revisions.len() < self.history.len() {
|
||||
let needed = self.history.len() - self.history_revisions.len();
|
||||
for _ in 0..needed {
|
||||
let rev = self.fresh_history_revision();
|
||||
self.history_revisions.push(rev);
|
||||
}
|
||||
} else if self.history_revisions.len() > self.history.len() {
|
||||
self.history_revisions.truncate(self.history.len());
|
||||
}
|
||||
}
|
||||
|
||||
/// Bump the revision counter of a single history cell so the transcript
|
||||
/// cache re-renders it on the next frame. Use this whenever a cell's
|
||||
/// content (e.g. a streaming Assistant body) is mutated in place.
|
||||
pub fn bump_history_cell(&mut self, idx: usize) {
|
||||
// Resync first in case callers mutated `history` directly without
|
||||
// pushing through `add_message`. After resync, the index is valid
|
||||
// (or out of bounds — in which case there's nothing to bump).
|
||||
self.resync_history_revisions();
|
||||
if let Some(rev) = self.history_revisions.get_mut(idx) {
|
||||
let new_rev = self.next_history_revision;
|
||||
self.next_history_revision = self.next_history_revision.wrapping_add(1);
|
||||
*rev = new_rev;
|
||||
}
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
self.needs_redraw = true;
|
||||
}
|
||||
|
||||
/// Append a single history cell, allocating a fresh per-cell revision.
|
||||
/// Equivalent to `add_message` but exposed as a generic alias so call
|
||||
/// sites currently doing `app.history.push(...)` followed by
|
||||
/// `app.mark_history_updated()` can collapse to one helper.
|
||||
pub fn push_history_cell(&mut self, cell: HistoryCell) {
|
||||
let rev = self.fresh_history_revision();
|
||||
self.history.push(cell);
|
||||
self.history_revisions.push(rev);
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
self.needs_redraw = true;
|
||||
}
|
||||
|
||||
/// Append a batch of history cells, allocating fresh revisions.
|
||||
pub fn extend_history<I>(&mut self, cells: I)
|
||||
where
|
||||
I: IntoIterator<Item = HistoryCell>,
|
||||
{
|
||||
for cell in cells {
|
||||
let rev = self.fresh_history_revision();
|
||||
self.history.push(cell);
|
||||
self.history_revisions.push(rev);
|
||||
}
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
self.needs_redraw = true;
|
||||
}
|
||||
|
||||
/// Clear the history and its revision tracking. Used by /clear, session
|
||||
/// reset, and other "wipe and reload" flows.
|
||||
pub fn clear_history(&mut self) {
|
||||
self.history.clear();
|
||||
self.history_revisions.clear();
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
self.needs_redraw = true;
|
||||
}
|
||||
|
||||
/// Pop the trailing history cell, keeping revisions in sync.
|
||||
pub fn pop_history(&mut self) -> Option<HistoryCell> {
|
||||
let cell = self.history.pop();
|
||||
if cell.is_some() {
|
||||
self.history_revisions.pop();
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
self.needs_redraw = true;
|
||||
}
|
||||
cell
|
||||
}
|
||||
|
||||
/// Bump the active-cell revision counter and request a redraw.
|
||||
///
|
||||
/// Use this whenever an entry inside `active_cell` is mutated. The
|
||||
@@ -1049,6 +1163,14 @@ impl App {
|
||||
/// in-flight entry, history version otherwise).
|
||||
pub fn cell_at_virtual_index_mut(&mut self, index: usize) -> Option<&mut HistoryCell> {
|
||||
if index < self.history.len() {
|
||||
// Bump only the targeted cell's revision; leave every other
|
||||
// cell's cached render intact.
|
||||
self.resync_history_revisions();
|
||||
if let Some(rev) = self.history_revisions.get_mut(index) {
|
||||
let new_rev = self.next_history_revision;
|
||||
self.next_history_revision = self.next_history_revision.wrapping_add(1);
|
||||
*rev = new_rev;
|
||||
}
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
self.history.get_mut(index)
|
||||
} else {
|
||||
@@ -1123,7 +1245,9 @@ impl App {
|
||||
self.exploring_entries.clear();
|
||||
|
||||
for cell in drained {
|
||||
let rev = self.fresh_history_revision();
|
||||
self.history.push(cell);
|
||||
self.history_revisions.push(rev);
|
||||
}
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
self.needs_redraw = true;
|
||||
|
||||
@@ -29,9 +29,12 @@ use crate::tui::scrolling::TranscriptLineMeta;
|
||||
///
|
||||
/// Lines are stored behind an `Arc` so that cloning a `CachedCell` during
|
||||
/// cache-ensure (which touches every cell every frame) is O(1) rather than
|
||||
/// O(rendered_line_count). The flatten step uses `Arc::make_mut` to produce
|
||||
/// an owned `Vec` for the final `lines` assembly, so the only deep-clone
|
||||
/// occurs on the flattened output — once per frame instead of once per cell.
|
||||
/// O(rendered_line_count). Without this, scrolling on a long transcript
|
||||
/// pays the cost of deep-cloning every cell's `Vec<Line>` per frame, which
|
||||
/// is the surface-level symptom of issue #78. The flatten step uses
|
||||
/// `Arc::make_mut` to produce an owned `Vec` for the final `lines`
|
||||
/// assembly, so the only deep-clone occurs on the flattened output — once
|
||||
/// per frame instead of once per cell.
|
||||
#[derive(Debug, Clone)]
|
||||
struct CachedCell {
|
||||
/// Revision the cell was at when the lines/meta were rendered.
|
||||
@@ -89,6 +92,11 @@ impl TranscriptViewCache {
|
||||
/// `cell_revisions.len()` is expected to equal `cells.len()`. If they
|
||||
/// disagree (shouldn't happen in normal use) the cache treats every cell
|
||||
/// as dirty.
|
||||
///
|
||||
/// Retained for tests and external use; the live render path uses the
|
||||
/// `ensure_split` variant to avoid concatenating history + active-cell
|
||||
/// entries every frame.
|
||||
#[allow(dead_code)]
|
||||
pub fn ensure(
|
||||
&mut self,
|
||||
cells: &[HistoryCell],
|
||||
@@ -96,6 +104,22 @@ impl TranscriptViewCache {
|
||||
width: u16,
|
||||
options: TranscriptRenderOptions,
|
||||
) {
|
||||
self.ensure_split(&[cells], cell_revisions, width, options);
|
||||
}
|
||||
|
||||
/// Ensure cached lines match the provided cell shards (logically
|
||||
/// concatenated) plus per-cell revisions. Avoids the
|
||||
/// `concat-into-Vec<HistoryCell>` clone the caller would otherwise pay
|
||||
/// every frame on long transcripts.
|
||||
pub fn ensure_split(
|
||||
&mut self,
|
||||
cell_shards: &[&[HistoryCell]],
|
||||
cell_revisions: &[u64],
|
||||
width: u16,
|
||||
options: TranscriptRenderOptions,
|
||||
) {
|
||||
let total_cells: usize = cell_shards.iter().map(|s| s.len()).sum();
|
||||
|
||||
let layout_changed = self.width != width || self.options != options;
|
||||
if layout_changed {
|
||||
self.per_cell.clear();
|
||||
@@ -105,46 +129,51 @@ impl TranscriptViewCache {
|
||||
|
||||
// Track whether anything actually changed; if all cells are reused at
|
||||
// the same indices, we can skip the reflatten.
|
||||
let mut any_dirty = layout_changed || self.per_cell.len() != cells.len();
|
||||
let mut any_dirty = layout_changed || self.per_cell.len() != total_cells;
|
||||
|
||||
let mut new_per_cell: Vec<CachedCell> = Vec::with_capacity(cells.len());
|
||||
let revisions_match = cell_revisions.len() == cells.len();
|
||||
let mut new_per_cell: Vec<CachedCell> = Vec::with_capacity(total_cells);
|
||||
let revisions_match = cell_revisions.len() == total_cells;
|
||||
|
||||
for (idx, cell) in cells.iter().enumerate() {
|
||||
let current_rev = if revisions_match {
|
||||
cell_revisions[idx]
|
||||
} else {
|
||||
// No matching revisions — force a re-render this cycle.
|
||||
u64::MAX
|
||||
};
|
||||
let mut idx: usize = 0;
|
||||
for shard in cell_shards {
|
||||
for cell in *shard {
|
||||
let current_rev = if revisions_match {
|
||||
cell_revisions[idx]
|
||||
} else {
|
||||
// No matching revisions — force a re-render this cycle.
|
||||
u64::MAX
|
||||
};
|
||||
|
||||
// Reuse cached entry if the revision matches AND it's at the same
|
||||
// index (since cells can shift on insert/remove, we only reuse
|
||||
// when the index is identical — a stricter invariant codex also
|
||||
// uses for its active-cell tail).
|
||||
if let Some(prev) = self.per_cell.get(idx)
|
||||
&& !layout_changed
|
||||
&& prev.revision == current_rev
|
||||
&& revisions_match
|
||||
{
|
||||
new_per_cell.push(prev.clone());
|
||||
continue;
|
||||
// Reuse cached entry if the revision matches AND it's at the
|
||||
// same index (cells can shift on insert/remove, so we only
|
||||
// reuse when the index is identical — a stricter invariant
|
||||
// codex also uses for its active-cell tail).
|
||||
if let Some(prev) = self.per_cell.get(idx)
|
||||
&& !layout_changed
|
||||
&& prev.revision == current_rev
|
||||
&& revisions_match
|
||||
{
|
||||
new_per_cell.push(prev.clone());
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
any_dirty = true;
|
||||
let rendered = cell.lines_with_options(width, options);
|
||||
let is_empty = rendered.is_empty();
|
||||
new_per_cell.push(CachedCell {
|
||||
revision: current_rev,
|
||||
lines: Arc::new(rendered),
|
||||
is_empty,
|
||||
is_stream_continuation: cell.is_stream_continuation(),
|
||||
is_conversational: cell.is_conversational(),
|
||||
is_system_or_tool: matches!(
|
||||
cell,
|
||||
HistoryCell::System { .. } | HistoryCell::Tool(_)
|
||||
),
|
||||
});
|
||||
idx += 1;
|
||||
}
|
||||
|
||||
any_dirty = true;
|
||||
let rendered = cell.lines_with_options(width, options);
|
||||
let is_empty = rendered.is_empty();
|
||||
new_per_cell.push(CachedCell {
|
||||
revision: current_rev,
|
||||
lines: Arc::new(rendered),
|
||||
is_empty,
|
||||
is_stream_continuation: cell.is_stream_continuation(),
|
||||
is_conversational: cell.is_conversational(),
|
||||
is_system_or_tool: matches!(
|
||||
cell,
|
||||
HistoryCell::System { .. } | HistoryCell::Tool(_)
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
self.per_cell = new_per_cell;
|
||||
|
||||
+37
-17
@@ -181,8 +181,8 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> {
|
||||
app.system_prompt = Some(SystemPrompt::Text(prompt));
|
||||
}
|
||||
// Convert saved messages to HistoryCell format for display
|
||||
app.history.clear();
|
||||
app.history.push(HistoryCell::System {
|
||||
app.clear_history();
|
||||
app.push_history_cell(HistoryCell::System {
|
||||
content: format!(
|
||||
"Resumed session: {} ({})",
|
||||
saved.metadata.title,
|
||||
@@ -191,7 +191,7 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> {
|
||||
});
|
||||
|
||||
for msg in &saved.messages {
|
||||
app.history.extend(history_cells_from_message(msg));
|
||||
app.extend_history(history_cells_from_message(msg));
|
||||
}
|
||||
app.mark_history_updated();
|
||||
app.status_message = Some(format!(
|
||||
@@ -403,6 +403,11 @@ async fn run_event_loop(
|
||||
{
|
||||
*streaming = false;
|
||||
}
|
||||
// Streaming flag flipped — the cell's compact /
|
||||
// transcript variants render slightly
|
||||
// differently, so bump its revision so the cache
|
||||
// refreshes this row only.
|
||||
app.bump_history_cell(index);
|
||||
transcript_batch_updated = true;
|
||||
}
|
||||
|
||||
@@ -974,11 +979,13 @@ async fn run_event_loop(
|
||||
|
||||
if let Event::Resize(width, height) = evt {
|
||||
tracing::debug!(width, height, "Event::Resize received; clearing terminal");
|
||||
// Coalesce queued resize events so we only act on the final
|
||||
// size. Rapid drag-resizes can produce many intermediate
|
||||
// events; processing each one with its own clear+redraw
|
||||
// amplifies the artifact window. Drain the queue here, keep
|
||||
// only the final dimensions, and issue a single clear+draw.
|
||||
// Drain any further Resize events queued in this poll cycle so we
|
||||
// act on the final size only, then issue a single clear + redraw.
|
||||
// crossterm coalesces some resize events but rapid drag-resizes
|
||||
// can still queue several; processing them all here avoids the
|
||||
// common "stale art on the right edge" symptom (#65) caused by
|
||||
// the diff renderer skipping cells that match a stale back
|
||||
// buffer between intermediate sizes.
|
||||
let mut final_w = width;
|
||||
let mut final_h = height;
|
||||
while event::poll(Duration::from_millis(0)).unwrap_or(false) {
|
||||
@@ -988,6 +995,9 @@ async fn run_event_loop(
|
||||
final_h = h;
|
||||
}
|
||||
Ok(other) => {
|
||||
// Non-resize event during the drain: we can't
|
||||
// un-read it. Drop it and let the user re-issue
|
||||
// — the resize-coalesce window is tiny.
|
||||
tracing::debug!(
|
||||
?other,
|
||||
"non-resize event during resize coalesce; dropping"
|
||||
@@ -999,9 +1009,10 @@ async fn run_event_loop(
|
||||
}
|
||||
terminal.clear()?;
|
||||
app.handle_resize(final_w, final_h);
|
||||
// Repaint immediately. Without this, the cleared screen can
|
||||
// sit blank until the next event triggers another draw,
|
||||
// which presents as a flicker or stale frame to the user.
|
||||
// Draw immediately so the cleared screen gets repainted before
|
||||
// any other events can interleave. Without this, the next
|
||||
// iteration's draw can race against fast follow-up input and
|
||||
// leave the user staring at a blank/partial frame.
|
||||
terminal.draw(|f| render(f, app))?;
|
||||
app.needs_redraw = false;
|
||||
continue;
|
||||
@@ -1745,6 +1756,12 @@ fn append_streaming_text(app: &mut App, index: usize, text: &str) {
|
||||
}
|
||||
if let Some(HistoryCell::Assistant { content, .. }) = app.history.get_mut(index) {
|
||||
content.push_str(text);
|
||||
// Bump only the streaming cell's per-cell revision so the transcript
|
||||
// cache re-renders just this cell. Without this, the cache would
|
||||
// either skip the update entirely (now that the global
|
||||
// history_version is no longer fanned out across every cell) or fall
|
||||
// back to a full re-wrap of the entire transcript every chunk.
|
||||
app.bump_history_cell(index);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2910,7 +2927,7 @@ async fn handle_view_events(
|
||||
|
||||
fn apply_loaded_session(app: &mut App, session: &SavedSession) {
|
||||
app.api_messages.clone_from(&session.messages);
|
||||
app.history.clear();
|
||||
app.clear_history();
|
||||
app.tool_cells.clear();
|
||||
app.tool_details_by_cell.clear();
|
||||
app.active_cell = None;
|
||||
@@ -2922,9 +2939,12 @@ fn apply_loaded_session(app: &mut App, session: &SavedSession) {
|
||||
app.pending_tool_uses.clear();
|
||||
app.last_exec_wait_command = None;
|
||||
|
||||
for msg in &app.api_messages {
|
||||
app.history.extend(history_cells_from_message(msg));
|
||||
}
|
||||
let cells_to_add: Vec<_> = app
|
||||
.api_messages
|
||||
.iter()
|
||||
.flat_map(history_cells_from_message)
|
||||
.collect();
|
||||
app.extend_history(cells_to_add);
|
||||
app.mark_history_updated();
|
||||
app.transcript_selection.clear();
|
||||
app.model.clone_from(&session.metadata.model);
|
||||
@@ -3661,8 +3681,8 @@ pub(crate) fn truncate_line_to_width(text: &str, max_width: usize) -> String {
|
||||
return text.to_string();
|
||||
}
|
||||
// For very small budgets, take chars until we exceed the *display* width.
|
||||
// The previous implementation counted codepoints, which overran the
|
||||
// budget for any double-width grapheme and contributed to mid-character
|
||||
// Counting characters instead of widths (the previous behavior) overran
|
||||
// the budget for any double-width grapheme and contributed to mid-character
|
||||
// sidebar artifacts on resize (issue #65).
|
||||
if max_width <= 3 {
|
||||
let mut out = String::new();
|
||||
|
||||
@@ -66,38 +66,50 @@ impl ChatWidget {
|
||||
};
|
||||
}
|
||||
|
||||
// The transcript cache exposes a per-cell revision slice for fine
|
||||
// grained caching; until App tracks per-cell revisions explicitly,
|
||||
// we synthesize a uniform slice from the global history_version so
|
||||
// any mutation invalidates every cell (matches the pre-cache
|
||||
// semantics).
|
||||
// Per-cell revision caching (fix for issue #78):
|
||||
//
|
||||
// Every committed history cell carries its own revision counter in
|
||||
// `app.history_revisions`. The transcript cache compares each cell's
|
||||
// current revision against the previously rendered one, so unchanged
|
||||
// cells reuse their cached wrapped lines instead of being re-wrapped
|
||||
// every frame. This is the difference between O(history.len()) and
|
||||
// O(changed_cells) per render — and was the root cause of scroll lag
|
||||
// on long transcripts.
|
||||
//
|
||||
// The active in-flight cell (if any) is appended as the last cell so
|
||||
// its mutations show up at the live tail. Each entry inside the
|
||||
// active cell becomes a virtual cell at index `history.len() + i`,
|
||||
// matching `App::cell_at_virtual_index`. Active-cell entries get a
|
||||
// distinct revision derived from `active_cell_revision` so changes to
|
||||
// them only re-render those rows.
|
||||
// matching `App::cell_at_virtual_index`. Active-cell entries share
|
||||
// the same `active_cell_revision` salt so any mutation in the active
|
||||
// cell forces only those rows to re-render — committed history rows
|
||||
// are unaffected.
|
||||
app.resync_history_revisions();
|
||||
let active_entries: &[HistoryCell] = app
|
||||
.active_cell
|
||||
.as_ref()
|
||||
.map_or(&[], |active| active.entries());
|
||||
let mut combined_cells: Vec<HistoryCell> =
|
||||
// Build the per-cell revision slice without cloning history cells.
|
||||
// History cells reuse `app.history_revisions` directly; active-cell
|
||||
// entries fall back to a synthetic revision derived from
|
||||
// `active_cell_revision` (active cells don't carry their own
|
||||
// per-entry counter today).
|
||||
let mut cell_revisions: Vec<u64> =
|
||||
Vec::with_capacity(app.history.len() + active_entries.len());
|
||||
combined_cells.extend_from_slice(&app.history);
|
||||
combined_cells.extend_from_slice(active_entries);
|
||||
let mut cell_revisions = vec![app.history_version; combined_cells.len()];
|
||||
// Salt the active-cell revisions with `active_cell_revision` so they
|
||||
// invalidate independently of the history version when only the
|
||||
// active cell mutates.
|
||||
cell_revisions.extend_from_slice(&app.history_revisions);
|
||||
if !active_entries.is_empty() {
|
||||
let active_rev = app.active_cell_revision;
|
||||
for rev in cell_revisions.iter_mut().skip(app.history.len()) {
|
||||
*rev = rev.wrapping_add(active_rev.wrapping_mul(0x9E37_79B9_7F4A_7C15));
|
||||
for i in 0..active_entries.len() {
|
||||
let salt = (i as u64).wrapping_add(1);
|
||||
cell_revisions.push(
|
||||
active_rev
|
||||
.wrapping_mul(0x9E37_79B9_7F4A_7C15)
|
||||
.wrapping_add(salt),
|
||||
);
|
||||
}
|
||||
}
|
||||
app.transcript_cache.ensure(
|
||||
&combined_cells,
|
||||
let shards: [&[HistoryCell]; 2] = [&app.history, active_entries];
|
||||
app.transcript_cache.ensure_split(
|
||||
&shards,
|
||||
&cell_revisions,
|
||||
content_area.width.max(1),
|
||||
render_options,
|
||||
@@ -1689,13 +1701,14 @@ mod tests {
|
||||
}
|
||||
|
||||
/// Regression for issue #65: after `App::handle_resize`, the chat widget
|
||||
/// must produce a clean render at the new width — no panic, and a
|
||||
/// populated history must yield non-empty rendered cells. Cycling
|
||||
/// through several widths (shrinks and grows) flushes any cached layout
|
||||
/// that fails to invalidate on resize.
|
||||
/// must produce a clean render at the new width — no stale wrapping,
|
||||
/// no panic, no content exceeding the requested width. Cycling through
|
||||
/// several widths (shrinks and grows) flushes any cached layout that
|
||||
/// fails to invalidate on resize.
|
||||
#[test]
|
||||
fn chat_widget_renders_cleanly_after_resize_cycle() {
|
||||
let mut app = create_test_app();
|
||||
// Add some long content that wraps differently at different widths.
|
||||
for i in 0..40 {
|
||||
app.add_message(HistoryCell::User {
|
||||
content: format!("user message {i} with enough text to wrap at 30 columns easily"),
|
||||
@@ -1705,6 +1718,7 @@ mod tests {
|
||||
let widths_to_cycle = [120u16, 80, 40, 60, 100, 30];
|
||||
let height: u16 = 20;
|
||||
for width in widths_to_cycle {
|
||||
// Caller-side: simulate the resize handler invalidating caches.
|
||||
app.handle_resize(width, height);
|
||||
let area = Rect {
|
||||
x: 0,
|
||||
@@ -1716,6 +1730,10 @@ mod tests {
|
||||
let widget = ChatWidget::new(&mut app, area);
|
||||
widget.render(area, &mut buf);
|
||||
|
||||
// The render must produce at least some non-empty content for a
|
||||
// populated history at any reasonable width. This catches a class
|
||||
// of resize regressions where stale layout state leaves a blank
|
||||
// viewport after a width change.
|
||||
let mut non_empty = 0usize;
|
||||
for y in 0..height {
|
||||
for x in 0..width {
|
||||
@@ -1733,7 +1751,7 @@ mod tests {
|
||||
}
|
||||
|
||||
/// Regression for issue #65: the transcript view cache must invalidate
|
||||
/// when width changes, so the same `App.history` re-wraps at the new
|
||||
/// when width changes, so the same `App.history` re-wraps to the new
|
||||
/// width on the very next `ChatWidget::new` call.
|
||||
#[test]
|
||||
fn transcript_cache_invalidates_on_width_change() {
|
||||
@@ -1761,6 +1779,8 @@ mod tests {
|
||||
widget_wide.render(area_wide, &mut buf_wide);
|
||||
let wide_total_lines = app.transcript_cache.total_lines();
|
||||
|
||||
// Without an explicit resize call, just shrinking the render area
|
||||
// should still trigger a cache rebuild because the cache keys on width.
|
||||
let mut buf_narrow = Buffer::empty(area_narrow);
|
||||
let widget_narrow = ChatWidget::new(&mut app, area_narrow);
|
||||
widget_narrow.render(area_narrow, &mut buf_narrow);
|
||||
@@ -1771,4 +1791,118 @@ mod tests {
|
||||
"narrow render should produce more wrapped lines (got {narrow_total_lines}, wide={wide_total_lines})"
|
||||
);
|
||||
}
|
||||
|
||||
/// Issue #78 — perf bench for transcript scroll lag.
|
||||
///
|
||||
/// Builds a 5000-entry history (mix of user / assistant / a few tool
|
||||
/// cells), then times `ChatWidget::new` at scroll offsets 0, 100, 500,
|
||||
/// and 2000 lines from the tail. The first call after history mutation
|
||||
/// pays the wrap cost; subsequent calls at different offsets should hit
|
||||
/// the per-cell cache and be ~constant time regardless of offset.
|
||||
///
|
||||
/// Run with: `cargo test -p deepseek-tui --release bench_transcript_scroll
|
||||
/// -- --ignored --nocapture`
|
||||
#[test]
|
||||
#[ignore = "perf bench; run with --release"]
|
||||
fn bench_transcript_scroll_5000_messages() {
|
||||
use std::time::Instant;
|
||||
|
||||
let mut app = create_test_app();
|
||||
// 5000 cells: alternating user / assistant with realistic-ish bodies
|
||||
// so wrapping cost is non-trivial. Every 50th cell is a (small)
|
||||
// generic tool cell, mirroring real transcripts.
|
||||
for i in 0..5000usize {
|
||||
let cell = if i % 50 == 49 {
|
||||
HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
|
||||
name: "grep_files".to_string(),
|
||||
status: ToolStatus::Success,
|
||||
input_summary: Some(format!("query: hit-{i}")),
|
||||
output: Some(format!("found 12 matches in cell-{i}")),
|
||||
prompts: None,
|
||||
}))
|
||||
} else if i % 2 == 0 {
|
||||
HistoryCell::User {
|
||||
content: format!(
|
||||
"user message {i}: please review the changes in src/foo/bar.rs and \
|
||||
tell me whether the new error handling looks reasonable"
|
||||
),
|
||||
}
|
||||
} else {
|
||||
HistoryCell::Assistant {
|
||||
content: format!(
|
||||
"Sure — looking at src/foo/bar.rs in cell {i}, the new error \
|
||||
handling wraps each fallible call in `?` and propagates a \
|
||||
typed `FooError`. That looks fine, but consider whether the \
|
||||
`Display` impl needs to redact the inner path."
|
||||
),
|
||||
streaming: false,
|
||||
}
|
||||
};
|
||||
app.add_message(cell);
|
||||
}
|
||||
|
||||
let area = Rect {
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 100,
|
||||
height: 30,
|
||||
};
|
||||
|
||||
// Warm-up: first call after a full history build pays the wrap cost
|
||||
// for every cell. We don't time this — it's amortized across the
|
||||
// session and is not the user-visible problem.
|
||||
let _ = ChatWidget::new(&mut app, area);
|
||||
|
||||
let visible = area.height as usize;
|
||||
// For each scroll target, snap the scroll position there and measure
|
||||
// a fresh ChatWidget::new(). The cache should hit for all unchanged
|
||||
// cells, so the time should be roughly constant regardless of
|
||||
// offset.
|
||||
for offset_from_tail in [0usize, 100, 500, 2000] {
|
||||
let total = app.transcript_cache.total_lines();
|
||||
let max_start = total.saturating_sub(visible);
|
||||
let target = max_start.saturating_sub(offset_from_tail);
|
||||
app.transcript_scroll = crate::tui::scrolling::TranscriptScroll::at_line(target);
|
||||
|
||||
let iters: u32 = 10;
|
||||
let start = Instant::now();
|
||||
for _ in 0..iters {
|
||||
let _ = ChatWidget::new(&mut app, area);
|
||||
}
|
||||
let elapsed = start.elapsed();
|
||||
let per_call_us = elapsed.as_micros() / u128::from(iters);
|
||||
println!(
|
||||
"[bench_transcript_scroll] offset={offset_from_tail:>5} \
|
||||
per_render={per_call_us:>6} \u{3bc}s ({:>3} ms / {iters} iters)",
|
||||
elapsed.as_millis()
|
||||
);
|
||||
}
|
||||
|
||||
// Streaming-delta scenario: append one assistant cell at the tail
|
||||
// and time a render. The cache should re-render only the new cell,
|
||||
// NOT every cell — even at deep scroll.
|
||||
for offset_from_tail in [0usize, 2000] {
|
||||
let total = app.transcript_cache.total_lines();
|
||||
let max_start = total.saturating_sub(visible);
|
||||
let target = max_start.saturating_sub(offset_from_tail);
|
||||
app.transcript_scroll = crate::tui::scrolling::TranscriptScroll::at_line(target);
|
||||
|
||||
let iters: u32 = 10;
|
||||
let start = Instant::now();
|
||||
for i in 0..iters {
|
||||
app.add_message(HistoryCell::Assistant {
|
||||
content: format!("delta {i}"),
|
||||
streaming: false,
|
||||
});
|
||||
let _ = ChatWidget::new(&mut app, area);
|
||||
}
|
||||
let elapsed = start.elapsed();
|
||||
let per_call_us = elapsed.as_micros() / u128::from(iters);
|
||||
println!(
|
||||
"[bench_transcript_scroll] streaming offset={offset_from_tail:>5} \
|
||||
per_render={per_call_us:>6} \u{3bc}s ({:>3} ms / {iters} iters)",
|
||||
elapsed.as_millis()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user