diff --git a/internal/ui/model.go b/internal/ui/model.go index a0568dde..a994c070 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -1266,7 +1266,11 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.scrollList.autoScroll = false case tea.MouseWheelDown: m.scrollList.ScrollBy(scrollLines) - if m.scrollList.AtBottom() { + // Only re-enable auto-scroll when the user is not actively + // selecting text. Otherwise a wheel-down during a drag-select + // would re-arm GotoBottom on the next stream chunk, shifting + // the highlighted row out from under the cursor. + if m.scrollList.AtBottom() && !m.scrollList.IsMouseDown() { m.scrollList.autoScroll = true } } @@ -1274,9 +1278,14 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // ── Mouse click selection (crush-style character-level) ────────────────── case tea.MouseClickMsg: if msg.Button == tea.MouseLeft { - // Calculate viewport-relative coordinates. - viewY := msg.Y - m.scrollbackYOffset - if viewY >= 0 && viewY < m.scrollList.height { + // Compute the scrollback origin from the current frame's layout + // rather than the stale cached value from the previous View(). + // scrollbackYOffset/scrollList.height are only refreshed inside + // View() and lag behind any state change that resized the header + // (extension widgets, warning rows, etc.) since the last render. + yOff, vpHeight := m.currentScrollbackBounds() + viewY := msg.Y - yOff + if viewY >= 0 && viewY < vpHeight { // Clear any previous selection on a new click. // HandleMouseDown will set up new selection state. if m.scrollList.HandleMouseDown(msg.X, viewY) { @@ -1287,8 +1296,9 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // ── Mouse motion/drag for character-level selection ────────────────────── case tea.MouseMotionMsg: - viewY := msg.Y - m.scrollbackYOffset - if viewY >= 0 && viewY < m.scrollList.height { + yOff, vpHeight := m.currentScrollbackBounds() + viewY := msg.Y - yOff + if viewY >= 0 && viewY < vpHeight { m.scrollList.HandleMouseDrag(msg.X, viewY) } @@ -1618,10 +1628,16 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // ── Cancel timer expired ───────────────────────────────────────────────── case uicore.CancelTimerExpiredMsg: + if m.canceling { + m.layoutDirty = true + } m.canceling = false // ── Ctrl+C reset timer expired ──────────────────────────────────────────── case uicore.CtrlCResetMsg: + if m.ctrlCPressedOnce { + m.layoutDirty = true + } m.ctrlCPressedOnce = false // ── Input submitted ────────────────────────────────────────────────────── @@ -3763,7 +3779,12 @@ func (m *AppModel) appendStreamingChunk(role, content string) { } // Auto-scroll to bottom if enabled (iteratr pattern) // Don't call SetItems() - the slice reference hasn't changed - if m.scrollList != nil { + // + // CRITICAL: never scroll the viewport while the user is actively + // selecting text (mouse button held). Doing so shifts the + // highlighted content out from under the cursor and produces the + // off-by-N-row drift users see when copy-selecting during streaming. + if m.scrollList != nil && !m.scrollList.IsMouseDown() { if m.scrollList.autoScroll { m.scrollList.GotoBottom() } else if m.scrollList.AtBottom() { @@ -3791,6 +3812,36 @@ func (m *AppModel) appendStreamingChunk(role, content string) { m.refreshContent() } +// currentScrollbackBounds returns the live (yOffset, viewportHeight) for the +// scrollback region, computed from the current state — not from the cached +// values populated inside View(). +// +// scrollbackYOffset and scrollList.height are refreshed once per render, so +// any state change that resizes the header (extension widget toggles, +// warning rows, queued messages, etc.) leaves the cached values one frame +// stale. Mouse click handlers in Update() can then place the cursor on the +// wrong line, producing the off-by-N-row drift seen during copy-selection. +// +// This recomputes the header height by rendering it (cheap — the renderer +// returns "" when no extension header is set) and recomputes the viewport +// height the same way distributeHeight() does, so both inputs to the +// y → (item, line) mapping are always current. +func (m *AppModel) currentScrollbackBounds() (yOffset, viewportHeight int) { + // Force a fresh layout if anything in Update() marked the state dirty; + // otherwise scrollList.height still reflects the previous frame. + if m.layoutDirty { + m.distributeHeight() + m.layoutDirty = false + } + if headerView := m.renderHeaderFooter(m.getHeader); headerView != "" { + yOffset = lipgloss.Height(headerView) + } + if m.scrollList != nil { + viewportHeight = m.scrollList.height + } + return yOffset, viewportHeight +} + // distributeHeight recalculates child component heights after a window resize, // queue change, widget update, or state transition, and propagates the computed // stream height to the StreamComponent. @@ -3863,7 +3914,20 @@ func (m *AppModel) distributeHeight() { headerFooterLines += lipgloss.Height(footerView) } - streamHeight := max(m.height-separatorLines-widgetLines-headerFooterLines-queuedLines-inputLines-statusBarLines, 0) + // Account for transient warning rows that View() injects between the + // scrollback and the separator. These flags are toggled by ESC/Ctrl+C + // handlers; without subtracting them here the joined view exceeds + // m.height by one line per active warning and the bottom of the screen + // gets silently clipped — which in turn invalidates scrollbackYOffset. + var warningLines int + if m.canceling { + warningLines++ + } + if m.ctrlCPressedOnce { + warningLines++ + } + + streamHeight := max(m.height-separatorLines-widgetLines-headerFooterLines-queuedLines-inputLines-statusBarLines-warningLines, 0) // In alt screen mode, give the calculated height to ScrollList instead of stream. // The stream component still exists but is embedded as the last item in scrollList. diff --git a/internal/ui/scrolllist.go b/internal/ui/scrolllist.go index 6f1d06ef..b28ba1d7 100644 --- a/internal/ui/scrolllist.go +++ b/internal/ui/scrolllist.go @@ -60,10 +60,13 @@ func NewScrollList(width, height int) *ScrollList { } // SetItems replaces the items in the scroll list. If auto-scroll is enabled, -// the viewport will scroll to the bottom to show the latest content. +// the viewport will scroll to the bottom to show the latest content — EXCEPT +// when the user is actively selecting text (mouse button held), in which case +// the scroll position is locked so the highlighted content stays under the +// cursor. The pending bottom-scroll is deferred to MouseUp. func (s *ScrollList) SetItems(items []MessageItem) { s.items = items - if s.autoScroll { + if s.autoScroll && !s.sel.MouseDown { s.GotoBottom() } } @@ -157,6 +160,10 @@ func (s *ScrollList) HandleMouseDown(x, y int) bool { // HandleMouseDrag handles mouse motion while button is held. // Updates the selection endpoint for character-level precision. // Returns true if selection was updated. +// +// Defensively disables auto-scroll on every drag update — even if the +// MouseDown handler missed (e.g. click landed in viewport padding), any +// active drag means the user is selecting and the viewport must not jump. func (s *ScrollList) HandleMouseDrag(x, y int) bool { if !s.sel.MouseDown { return false @@ -171,6 +178,9 @@ func (s *ScrollList) HandleMouseDrag(x, y int) bool { return false } + // Hard-lock the viewport while dragging. + s.autoScroll = false + s.sel.DragItemIdx = itemIdx s.sel.DragLineIdx = lineIdx s.sel.DragCol = x @@ -178,6 +188,13 @@ func (s *ScrollList) HandleMouseDrag(x, y int) bool { return true } +// IsMouseDown reports whether the user currently has the mouse button held +// (i.e. a selection drag is in progress). Used by the parent model to avoid +// re-enabling auto-scroll during streaming while the user is selecting. +func (s *ScrollList) IsMouseDown() bool { + return s.sel.MouseDown +} + // HandleMouseUp handles mouse button release. // Returns true if there was an active selection. func (s *ScrollList) HandleMouseUp() bool { diff --git a/internal/ui/scrolllist_test.go b/internal/ui/scrolllist_test.go new file mode 100644 index 00000000..a88f9d23 --- /dev/null +++ b/internal/ui/scrolllist_test.go @@ -0,0 +1,132 @@ +package ui + +import ( + "fmt" + "strings" + "testing" +) + +// fakeItem is a deterministic MessageItem for ScrollList tests. +type fakeItem struct { + id string + lines int +} + +func (f *fakeItem) ID() string { return f.id } +func (f *fakeItem) Render(_ int) string { + if f.lines <= 0 { + return "" + } + parts := make([]string, f.lines) + for i := range parts { + parts[i] = fmt.Sprintf("%s-line-%d", f.id, i) + } + return strings.Join(parts, "\n") +} +func (f *fakeItem) Height() int { return f.lines } + +// makeItems builds n fake items of `lines` height each. +func makeItems(n, lines int) []MessageItem { + out := make([]MessageItem, n) + for i := range out { + out[i] = &fakeItem{id: fmt.Sprintf("item-%d", i), lines: lines} + } + return out +} + +// TestScrollList_MouseDownPreventsAutoScroll verifies the core fix for the +// copy-selection drift bug: while the user has the mouse button held +// (drag-selecting), incoming content updates must NOT shift the viewport, +// because doing so moves the highlighted content out from under the cursor. +func TestScrollList_MouseDownPreventsAutoScroll(t *testing.T) { + sl := NewScrollList(80, 10) + sl.SetItems(makeItems(20, 2)) // 40 lines of content into a 10-line viewport + // Capture the auto-scrolled-to-bottom position. + startOffsetIdx := sl.offsetIdx + startOffsetLine := sl.offsetLine + + // User clicks somewhere in the visible area, starting a drag-select. + if !sl.HandleMouseDown(5, 3) { + t.Fatalf("HandleMouseDown should accept a click inside the viewport") + } + if !sl.IsMouseDown() { + t.Fatalf("IsMouseDown should be true after HandleMouseDown") + } + + // New content arrives. With autoScroll still true, SetItems would + // normally call GotoBottom() and shift the viewport. The fix should + // suppress that while MouseDown is held. + sl.SetItems(makeItems(30, 2)) // 60 lines now + if sl.offsetIdx != startOffsetIdx || sl.offsetLine != startOffsetLine { + t.Errorf("viewport scrolled during active drag: was (%d,%d), now (%d,%d)", + startOffsetIdx, startOffsetLine, sl.offsetIdx, sl.offsetLine) + } + + // User releases the mouse — drag is over. + sl.HandleMouseUp() + if sl.IsMouseDown() { + t.Fatalf("IsMouseDown should be false after HandleMouseUp") + } + + // After release, a fresh content update should resume auto-scrolling + // (move the offset to track the new bottom). + afterReleaseIdx := sl.offsetIdx + afterReleaseLine := sl.offsetLine + sl.SetItems(makeItems(50, 2)) + if sl.offsetIdx == afterReleaseIdx && sl.offsetLine == afterReleaseLine { + t.Errorf("autoscroll did not resume after MouseUp: offset stuck at (%d,%d)", + afterReleaseIdx, afterReleaseLine) + } +} + +// TestScrollList_DragDisablesAutoScroll verifies that any successful +// HandleMouseDrag call clears autoScroll, even when HandleMouseDown didn't +// observe it (e.g. a stale wheel-down event set it back to true mid-stream). +func TestScrollList_DragDisablesAutoScroll(t *testing.T) { + sl := NewScrollList(80, 10) + sl.SetItems(makeItems(20, 2)) + + // Begin a selection. + if !sl.HandleMouseDown(5, 3) { + t.Fatalf("HandleMouseDown failed") + } + // Simulate an external code path that re-enabled autoScroll while + // MouseDown is still held (the precise condition that caused drift). + sl.autoScroll = true + + // Drag motion should hard-lock the viewport again. + if !sl.HandleMouseDrag(10, 4) { + t.Fatalf("HandleMouseDrag failed") + } + if sl.autoScroll { + t.Errorf("HandleMouseDrag must clear autoScroll to prevent mid-drag jumps") + } +} + +// TestScrollList_SetItemsRespectsMouseDown is the most direct regression +// test: even with autoScroll enabled and new content appended at the +// bottom, SetItems must not move the viewport while a mouse drag is in +// progress. This is what caused the "highlighting shifts by 1+ rows +// during streaming" symptom reported by the user. +func TestScrollList_SetItemsRespectsMouseDown(t *testing.T) { + sl := NewScrollList(80, 5) + sl.SetItems(makeItems(10, 2)) // 20 lines into a 5-line viewport + // At bottom. + preIdx, preLine := sl.offsetIdx, sl.offsetLine + + // Hold mouse down (no actual drag needed). + if !sl.HandleMouseDown(0, 0) { + t.Fatalf("HandleMouseDown failed") + } + + // Append several more items as if streaming. With the bug, each + // SetItems would call GotoBottom and shift the offset. + for n := 11; n <= 15; n++ { + sl.SetItems(makeItems(n, 2)) + if sl.offsetIdx != preIdx || sl.offsetLine != preLine { + t.Fatalf("viewport drifted during streaming with mouse held: "+ + "start=(%d,%d) now=(%d,%d) after adding item %d", + preIdx, preLine, sl.offsetIdx, sl.offsetLine, n) + } + } +}