fix(tui2): avoid scroll stickiness at cell boundaries (#8695)

Mouse/trackpad scrolling in tui2 applies deltas in visual lines, but the
transcript scroll state was anchored only to CellLine entries.

When a 1-line scroll landed on the synthetic inter-cell Spacer row
(inserted between non-continuation cells),
`TranscriptScroll::anchor_for` would skip that row and snap back to the
adjacent cell line. That makes the resolved top offset unchanged for
small/coalesced scroll deltas, so scrolling appears to get stuck right
before certain cells (commonly user prompts and command output cells).

Fix this by making spacer rows a first-class scroll anchor:
- Add `TranscriptScroll::ScrolledSpacerBeforeCell` and resolve it back
to the spacer row index when present.
- Update `anchor_for`/`scrolled_by` to preserve spacers instead of
skipping them.
- Treat the new variant as "already anchored" in
`lock_transcript_scroll_to_current_view`.

Tests:
- cargo test -p codex-tui2
This commit is contained in:
Josh McKinney 2026-01-03 12:26:40 -08:00 committed by GitHub
parent 0c1658d0ec
commit 279283fe02
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 99 additions and 18 deletions

View file

@ -1135,7 +1135,8 @@ impl App {
let max_start = total_lines.saturating_sub(max_visible);
let top_offset = match self.transcript_scroll {
TranscriptScroll::ToBottom => max_start,
TranscriptScroll::Scrolled { .. } => {
TranscriptScroll::Scrolled { .. }
| TranscriptScroll::ScrolledSpacerBeforeCell { .. } => {
// Already anchored; nothing to lock.
return;
}

View file

@ -17,7 +17,7 @@
//! the newly flattened line list on the next frame.
//!
//! Spacer rows between non-continuation cells are represented as `TranscriptLineMeta::Spacer`.
//! They are not valid anchors; `anchor_for` will pick the nearest non-spacer line when needed.
//! They are valid scroll anchors so 1-line scrolling does not "stick" at cell boundaries.
pub(crate) mod mouse;
pub(crate) use mouse::MouseScrollState;
@ -78,6 +78,13 @@ pub(crate) enum TranscriptScroll {
cell_index: usize,
line_in_cell: usize,
},
/// Anchor the viewport to the spacer row immediately before a cell.
///
/// This exists because spacer rows are real, visible transcript rows, and users may scroll
/// through them one line at a time (especially with trackpads). Without a dedicated spacer
/// anchor, a 1-line scroll that lands on a spacer would snap back to the adjacent cell line
/// and appear to "stick" at boundaries.
ScrolledSpacerBeforeCell { cell_index: usize },
}
impl TranscriptScroll {
@ -108,6 +115,13 @@ impl TranscriptScroll {
None => (Self::ToBottom, max_start),
}
}
Self::ScrolledSpacerBeforeCell { cell_index } => {
let anchor = spacer_before_cell_index(line_meta, cell_index);
match anchor {
Some(idx) => (self, idx.min(max_start)),
None => (Self::ToBottom, max_start),
}
}
}
}
@ -142,6 +156,11 @@ impl TranscriptScroll {
} => anchor_index(line_meta, cell_index, line_in_cell)
.unwrap_or(max_start)
.min(max_start),
Self::ScrolledSpacerBeforeCell { cell_index } => {
spacer_before_cell_index(line_meta, cell_index)
.unwrap_or(max_start)
.min(max_start)
}
};
let new_top = if delta_lines < 0 {
@ -164,15 +183,35 @@ impl TranscriptScroll {
/// This is the inverse of "resolving a scroll state to a top-row offset":
/// given a concrete flattened line index, pick a stable `(cell_index, line_in_cell)` anchor.
///
/// See `resolve_top` for `line_meta` semantics. This prefers the nearest line at or after `start`
/// (skipping spacer rows), falling back to the nearest line before it when needed.
/// See `resolve_top` for `line_meta` semantics. This prefers the line at `start` (including
/// spacer rows), falling back to the nearest non-spacer line after or before it when needed.
pub(crate) fn anchor_for(line_meta: &[TranscriptLineMeta], start: usize) -> Option<Self> {
let anchor =
anchor_at_or_after(line_meta, start).or_else(|| anchor_at_or_before(line_meta, start));
anchor.map(|(cell_index, line_in_cell)| Self::Scrolled {
cell_index,
line_in_cell,
})
if line_meta.is_empty() {
return None;
}
let start = start.min(line_meta.len().saturating_sub(1));
match line_meta[start] {
TranscriptLineMeta::CellLine {
cell_index,
line_in_cell,
} => Some(Self::Scrolled {
cell_index,
line_in_cell,
}),
TranscriptLineMeta::Spacer => {
if let Some((cell_index, _)) = anchor_at_or_after(line_meta, start) {
Some(Self::ScrolledSpacerBeforeCell { cell_index })
} else {
anchor_at_or_before(line_meta, start).map(|(cell_index, line_in_cell)| {
Self::Scrolled {
cell_index,
line_in_cell,
}
})
}
}
}
}
}
@ -198,6 +237,26 @@ fn anchor_index(
})
}
/// Locate the flattened line index for the spacer row immediately before `cell_index`.
///
/// The spacer itself is not uniquely tagged in `TranscriptLineMeta`, so we locate the first
/// visual line of the cell (`line_in_cell == 0`) and, if it is preceded by a spacer row, return
/// that spacer's index. If the spacer is missing (for example when the cell is a stream
/// continuation), we fall back to the cell's first line index so scrolling remains usable.
fn spacer_before_cell_index(line_meta: &[TranscriptLineMeta], cell_index: usize) -> Option<usize> {
let cell_first = anchor_index(line_meta, cell_index, 0)?;
if cell_first > 0
&& matches!(
line_meta.get(cell_first.saturating_sub(1)),
Some(TranscriptLineMeta::Spacer)
)
{
Some(cell_first.saturating_sub(1))
} else {
Some(cell_first)
}
}
/// Find the first transcript line at or after the given flattened index.
fn anchor_at_or_after(line_meta: &[TranscriptLineMeta], start: usize) -> Option<(usize, usize)> {
if line_meta.is_empty() {
@ -272,6 +331,33 @@ mod tests {
assert_eq!(top, 2);
}
#[test]
fn scrolled_by_can_land_on_spacer_rows() {
let meta = meta(&[
cell_line(0, 0),
TranscriptLineMeta::Spacer,
cell_line(1, 0),
cell_line(1, 1),
]);
let scroll = TranscriptScroll::Scrolled {
cell_index: 1,
line_in_cell: 0,
};
assert_eq!(
scroll.scrolled_by(-1, &meta, 2),
TranscriptScroll::ScrolledSpacerBeforeCell { cell_index: 1 }
);
assert_eq!(
TranscriptScroll::ScrolledSpacerBeforeCell { cell_index: 1 }.scrolled_by(-1, &meta, 2),
TranscriptScroll::Scrolled {
cell_index: 0,
line_in_cell: 0
}
);
}
#[test]
fn resolve_top_scrolled_falls_back_when_anchor_missing() {
let meta = meta(&[cell_line(0, 0), TranscriptLineMeta::Spacer, cell_line(1, 0)]);
@ -350,17 +436,11 @@ mod tests {
assert_eq!(
TranscriptScroll::anchor_for(&meta, 0),
Some(TranscriptScroll::Scrolled {
cell_index: 0,
line_in_cell: 0
})
Some(TranscriptScroll::ScrolledSpacerBeforeCell { cell_index: 0 })
);
assert_eq!(
TranscriptScroll::anchor_for(&meta, 2),
Some(TranscriptScroll::Scrolled {
cell_index: 1,
line_in_cell: 0
})
Some(TranscriptScroll::ScrolledSpacerBeforeCell { cell_index: 1 })
);
assert_eq!(
TranscriptScroll::anchor_for(&meta, 3),