Skip to content

Commit ee3ca6c

Browse files
Yogthosyogthos
authored andcommitted
Fix UI lock when scrolling up during streaming output
Reported symptom: scrolling up while the agent streams responses makes the UI 'stuck at wherever you scrolled up', especially when mouse- selecting text — the selection rectangle drifts away from the cursor as new content arrives. Root cause: scroll_offset is 'lines from the bottom edge', a fixed quantity. As streaming tokens push new lines into the buffer, total grows but scroll_offset stays the same, so render_viewport's computed start = total - scroll_offset - visible drifts forward. The user's view silently follows the bottom at the fixed offset instead of staying on the absolute content they scrolled to. Selection indices are absolute (correct), but the highlighted rows drift along with the viewport so mouse-drags select the wrong lines. Fix: when content is appended to the buffer while scroll_offset > 0, bump scroll_offset by the number of lines added (clamped to max_offset). This keeps start anchored to the same absolute index. When scroll_offset == 0 the view continues to follow the bottom — that path is unchanged. Routes all buffer.push calls in commit_partial / write_line / write through a new private push_buffer_line helper. replace_from (used by the streaming-token markdown re-render) shifts scroll_offset by the size delta to preserve the anchor across rewrites. 6 new tests covering: - view stays anchored through 8 token appends while scrolled up - replace_from preserves anchor on both grow and shrink rewrites - bottom-anchored view (scroll_offset == 0) still follows new content - selection indices remain absolute under streaming appends - push_buffer_line clamps scroll_offset to max for tiny buffers - commit_partial routes through the anchor-aware push path
1 parent a5af4e6 commit ee3ca6c

1 file changed

Lines changed: 219 additions & 6 deletions

File tree

src/ui/renderer.rs

Lines changed: 219 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,22 @@ impl Renderer {
9797

9898
pub fn replace_from(&mut self, start: usize, lines: Vec<LineEntry>) {
9999
self.commit_partial();
100+
let old_len = self.buffer.len();
100101
self.buffer.truncate(start);
101102
self.buffer.extend(lines);
102-
self.lines = self.buffer.len() as u16;
103+
let new_len = self.buffer.len();
104+
self.lines = new_len as u16;
103105
self.col = 0;
104106
self.partial.clear();
105107
let visible = self.visible_lines();
106-
let max_offset = self.buffer.len().saturating_sub(visible);
107-
if self.scroll_offset > max_offset {
108+
let max_offset = new_len.saturating_sub(visible);
109+
// When the user is scrolled up, keep the view anchored to the same
110+
// absolute content by shifting scroll_offset to match the size delta.
111+
if self.scroll_offset > 0 {
112+
let delta = new_len as isize - old_len as isize;
113+
let new_offset = (self.scroll_offset as isize + delta).max(0) as usize;
114+
self.scroll_offset = new_offset.min(max_offset);
115+
} else if self.scroll_offset > max_offset {
108116
self.scroll_offset = max_offset;
109117
}
110118
}
@@ -190,7 +198,7 @@ impl Renderer {
190198
let max_width = self.max_line_width();
191199
let c = self.partial_color;
192200
for chunk in self.wrap_line(&self.partial, max_width) {
193-
self.buffer.push(LineEntry {
201+
self.push_buffer_line(LineEntry {
194202
text: chunk,
195203
color: c,
196204
});
@@ -199,6 +207,20 @@ impl Renderer {
199207
}
200208
}
201209

210+
/// Append a line to the scrollback buffer. If the user is currently
211+
/// scrolled up (scroll_offset > 0), bumps the offset by one so the
212+
/// view stays anchored to the same absolute content rather than drifting
213+
/// forward as new lines arrive. The selection (which uses absolute
214+
/// indices) is unaffected.
215+
fn push_buffer_line(&mut self, entry: LineEntry) {
216+
self.buffer.push(entry);
217+
if self.scroll_offset > 0 {
218+
let visible = self.visible_lines();
219+
let max_offset = self.buffer.len().saturating_sub(visible);
220+
self.scroll_offset = (self.scroll_offset + 1).min(max_offset);
221+
}
222+
}
223+
202224
pub fn is_scrolling(&self) -> bool {
203225
self.scroll_offset > 0
204226
}
@@ -354,7 +376,7 @@ impl Renderer {
354376
for segment in text.split('\n') {
355377
let wrapped = self.wrap_line(segment, max_width);
356378
for chunk in &wrapped {
357-
self.buffer.push(LineEntry {
379+
self.push_buffer_line(LineEntry {
358380
text: chunk.clone(),
359381
color,
360382
});
@@ -398,7 +420,7 @@ impl Renderer {
398420
self.partial.push_str(segment);
399421
self.commit_partial();
400422
} else if !had_content {
401-
self.buffer.push(LineEntry {
423+
self.push_buffer_line(LineEntry {
402424
text: CompactString::new(""),
403425
color,
404426
});
@@ -662,3 +684,194 @@ pub fn copy_to_clipboard(text: &str) {
662684
}
663685
}
664686
}
687+
688+
#[cfg(test)]
689+
mod tests {
690+
use super::*;
691+
692+
/// Create a renderer with a synthetic buffer of `n` short lines so we
693+
/// can drive scroll/append behavior without touching a real terminal.
694+
fn fresh_with_lines(n: usize) -> Renderer {
695+
let mut r = Renderer::new().expect("renderer");
696+
for i in 0..n {
697+
r.buffer.push(LineEntry {
698+
text: CompactString::new(&format!("line {i}")),
699+
color: Color::White,
700+
});
701+
}
702+
r.lines = r.buffer.len() as u16;
703+
r
704+
}
705+
706+
/// Absolute index of the first visible line in the current viewport,
707+
/// matching the formula used by `render_viewport`.
708+
fn view_start(r: &Renderer) -> usize {
709+
let visible = r.visible_lines();
710+
let total = r.buffer.len();
711+
let start = if r.scroll_offset == 0 {
712+
total.saturating_sub(visible)
713+
} else {
714+
total.saturating_sub(r.scroll_offset + visible)
715+
};
716+
start.min(total.saturating_sub(visible))
717+
}
718+
719+
// Regression: previously, when the user scrolled up while output was
720+
// streaming, scroll_offset stayed fixed but the buffer grew — so the
721+
// viewport drifted forward into newer content. The fix bumps
722+
// scroll_offset by one per appended line so the view stays anchored to
723+
// the same absolute lines.
724+
#[test]
725+
fn regression_scrolled_up_view_stays_anchored_through_appends() {
726+
let mut r = fresh_with_lines(50);
727+
// Scroll up 10 lines. View start changes; record it.
728+
for _ in 0..10 {
729+
r.scroll_line_up();
730+
}
731+
let pinned_start = view_start(&r);
732+
733+
// Stream in 8 new lines while the user is scrolled up.
734+
for i in 0..8 {
735+
r.push_buffer_line(LineEntry {
736+
text: CompactString::new(&format!("new {i}")),
737+
color: Color::White,
738+
});
739+
}
740+
741+
// The first visible line index hasn't moved.
742+
assert_eq!(view_start(&r), pinned_start);
743+
}
744+
745+
// Regression: replace_from (used by the streaming-token markdown path)
746+
// also has to honor the scroll anchor. If the agent's current response
747+
// grows (or shrinks) while the user is scrolled up viewing earlier
748+
// content, the earlier content must stay in view.
749+
#[test]
750+
fn regression_replace_from_keeps_view_anchored_when_scrolled_up() {
751+
let mut r = fresh_with_lines(50);
752+
for _ in 0..10 {
753+
r.scroll_line_up();
754+
}
755+
let pinned_start = view_start(&r);
756+
757+
// Replace from line 40 with twice as many lines.
758+
let new_lines: Vec<LineEntry> = (0..20)
759+
.map(|i| LineEntry {
760+
text: CompactString::new(&format!("repl {i}")),
761+
color: Color::White,
762+
})
763+
.collect();
764+
r.replace_from(40, new_lines);
765+
766+
assert_eq!(view_start(&r), pinned_start);
767+
768+
// Now replace with FEWER lines (response got shorter via re-render).
769+
let shorter: Vec<LineEntry> = (0..5)
770+
.map(|i| LineEntry {
771+
text: CompactString::new(&format!("sh {i}")),
772+
color: Color::White,
773+
})
774+
.collect();
775+
// After the first replace, len = 40 + 20 = 60. Now truncate at 40,
776+
// extend by 5 → len = 45. delta = -15. The view should attempt to
777+
// stay anchored at pinned_start, clamped.
778+
r.replace_from(40, shorter);
779+
let after = view_start(&r);
780+
// It must NOT have drifted upward (smaller absolute index) past where
781+
// the user originally was; staying ≥ pinned_start - shrink-room is ok.
782+
assert!(
783+
after <= pinned_start,
784+
"view must not skip past anchor; was {pinned_start}, now {after}"
785+
);
786+
}
787+
788+
// When the user is AT the bottom (scroll_offset == 0), new content must
789+
// be visible — the view follows the bottom. The anchor behavior must not
790+
// accidentally pin the bottom-anchored view.
791+
#[test]
792+
fn at_bottom_view_follows_new_content() {
793+
let mut r = fresh_with_lines(50);
794+
assert_eq!(r.scroll_offset, 0);
795+
796+
for i in 0..5 {
797+
r.push_buffer_line(LineEntry {
798+
text: CompactString::new(&format!("new {i}")),
799+
color: Color::White,
800+
});
801+
}
802+
assert_eq!(r.scroll_offset, 0, "bottom-anchored view must stay at 0");
803+
804+
let visible = r.visible_lines();
805+
let total = r.buffer.len();
806+
assert_eq!(view_start(&r), total.saturating_sub(visible));
807+
}
808+
809+
// Selection indices are absolute and must NOT shift when content
810+
// streams in. Prior to the anchor fix the selection rectangle visually
811+
// drifted because scroll_offset stayed put while the viewport advanced;
812+
// now the indices are still preserved and the viewport stays anchored,
813+
// so the selection rectangle stays where the user dragged it.
814+
#[test]
815+
fn selection_indices_stay_absolute_under_streaming_appends() {
816+
let mut r = fresh_with_lines(50);
817+
for _ in 0..10 {
818+
r.scroll_line_up();
819+
}
820+
r.selection_active = true;
821+
r.selection_start = Some(15);
822+
r.selection_end = Some(20);
823+
824+
for i in 0..7 {
825+
r.push_buffer_line(LineEntry {
826+
text: CompactString::new(&format!("new {i}")),
827+
color: Color::White,
828+
});
829+
}
830+
831+
// Selection indices are absolute and remain untouched.
832+
assert_eq!(r.selection_start, Some(15));
833+
assert_eq!(r.selection_end, Some(20));
834+
}
835+
836+
// Boundary: a tiny buffer where appending pushes scroll_offset past
837+
// max_offset. The clamp inside push_buffer_line keeps it in range.
838+
#[test]
839+
fn push_clamps_scroll_offset_to_max_when_buffer_grows() {
840+
let mut r = fresh_with_lines(2);
841+
let visible = r.visible_lines();
842+
// Force a non-zero offset (clamp may already prevent it on tiny
843+
// buffers; assert behavior either way).
844+
r.scroll_offset = 100;
845+
for _ in 0..3 {
846+
r.push_buffer_line(LineEntry {
847+
text: CompactString::new("more"),
848+
color: Color::White,
849+
});
850+
}
851+
let max_offset = r.buffer.len().saturating_sub(visible);
852+
assert!(
853+
r.scroll_offset <= max_offset,
854+
"scroll_offset {} must be ≤ max {}",
855+
r.scroll_offset,
856+
max_offset
857+
);
858+
}
859+
860+
// Streaming via commit_partial (the path used by `write` for streamed
861+
// tokens) also goes through push_buffer_line. Verify the partial commit
862+
// bumps the offset when scrolled up.
863+
#[test]
864+
fn commit_partial_routes_through_anchor_aware_push() {
865+
let mut r = fresh_with_lines(50);
866+
for _ in 0..10 {
867+
r.scroll_line_up();
868+
}
869+
let pinned_start = view_start(&r);
870+
871+
r.partial = CompactString::new("a streamed token chunk");
872+
r.partial_color = Color::White;
873+
r.commit_partial();
874+
875+
assert_eq!(view_start(&r), pinned_start);
876+
}
877+
}

0 commit comments

Comments
 (0)