diff --git a/AUTOSCROLL_FIX_SUMMARY.md b/AUTOSCROLL_FIX_SUMMARY.md new file mode 100644 index 00000000..14806be0 --- /dev/null +++ b/AUTOSCROLL_FIX_SUMMARY.md @@ -0,0 +1,80 @@ +# Autoscroll Fix - Final Summary + +## Root Cause + +The autoscroll was failing for streaming assistant messages due to a bug in how `GotoBottom()` calculated item heights. + +### The Problem + +1. **Reasoning blocks** (`StreamingMessageItem` with `role="reasoning"`) are **never cached** because they have live duration counters that update every render +2. The `Height()` method returns `0` when `cachedRender == ""` +3. `GotoBottom()` was calling: + ```go + itemHeight := item.Height() // Returns 0 for reasoning + if itemHeight == 0 { + item.Render(s.width) // Renders but doesn't cache (reasoning) + itemHeight = item.Height() // Still returns 0! + } + ``` +4. This caused incorrect scroll position calculations, especially during reasoning → assistant transitions + +## The Solution + +Changed `GotoBottom()` and `AtBottom()` to calculate height **directly from the rendered string** instead of relying on the cached height: + +```go +// OLD: item.Height() which checks cached render +itemHeight := item.Height() +if itemHeight == 0 { + item.Render(s.width) + itemHeight = item.Height() // Still might be 0! +} + +// NEW: Calculate from rendered string directly +rendered := item.Render(s.width) +itemHeight := strings.Count(rendered, "\n") + 1 +``` + +This works for **all** items regardless of whether they cache their render or not. + +## Files Changed + +### `internal/ui/scrolllist.go` +- **`GotoBottom()`**: Calculate height from rendered string (2 loops) +- **`AtBottom()`**: Calculate height from rendered string (1 loop) + +### `internal/ui/model.go` +- **`appendStreamingChunk()`**: For existing messages, call `GotoBottom()` directly (iteratr pattern) +- **`refreshContent()`**: Simplified to only call `SetItems()` (removed redundant `GotoBottom()`) +- **Bash streaming handler**: Removed redundant `GotoBottom()` after `refreshContent()` + +## Testing Results + +✅ **Test prompt**: "explore this repo" + +**Before fix**: +- Autoscroll stopped after reasoning block completed +- Viewport stuck showing end of reasoning ("Thought for 203ms") +- Assistant response streamed off-screen below + +**After fix**: +- Autoscroll works throughout reasoning block +- Autoscroll continues during reasoning → assistant transition +- Viewport stays at bottom showing latest assistant content +- Final position shows end of response (build commands section) + +## Behavior Verified + +1. ✅ Streaming text auto-scrolls to bottom +2. ✅ Works across reasoning → assistant transition +3. ✅ Manual scroll up (PgUp) disables autoscroll +4. ✅ Scroll to bottom (Alt+End) re-enables autoscroll +5. ✅ Accurate positioning with no offset errors + +## Performance Note + +The fix calls `Render()` on all items during `GotoBottom()` calculations. This is acceptable because: +- `Render()` is already optimized with caching for non-reasoning items +- `GotoBottom()` is only called during content updates (not every frame) +- Reasoning blocks need to render anyway for live duration updates +- This matches iteratr's approach of ensuring items are rendered before height calculations diff --git a/internal/ui/model.go b/internal/ui/model.go index b0da6c40..4b626aff 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -1548,14 +1548,9 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } - // Refresh ScrollList + // Refresh ScrollList (handles autoscroll internally) m.refreshContent() - // Auto-scroll to bottom - if m.scrollList != nil && m.scrollList.autoScroll { - m.scrollList.GotoBottom() - } - case app.ToolCallContentEvent: // In streaming mode this text was already delivered via StreamChunkEvents // and will be flushed before the next tool call. Ignore to avoid @@ -2086,13 +2081,8 @@ func (m *AppModel) refreshContent() { return } - // MessageItem implements ScrollItem interface, so we can use copy + // SetItems handles autoscroll internally if enabled m.scrollList.SetItems(m.messages) - - // Only adjust scroll position if auto-scroll is enabled - if m.scrollList.autoScroll { - m.scrollList.GotoBottom() - } } // renderScrollback returns the scrollback content from ScrollList. @@ -2886,7 +2876,8 @@ func (m *AppModel) appendStreamingChunk(role, content string) { // If last message is a StreamingMessageItem with matching role, append to it if streamMsg, ok := lastMsg.(*StreamingMessageItem); ok && streamMsg.role == role { streamMsg.AppendChunk(content) - // Auto-scroll to bottom if enabled + // Auto-scroll to bottom if enabled (iteratr pattern) + // Don't call SetItems() - the slice reference hasn't changed if m.scrollList != nil && m.scrollList.autoScroll { m.scrollList.GotoBottom() } diff --git a/internal/ui/scrolllist.go b/internal/ui/scrolllist.go index 7db81394..30d944bd 100644 --- a/internal/ui/scrolllist.go +++ b/internal/ui/scrolllist.go @@ -361,9 +361,13 @@ func (s *ScrollList) GotoBottom() { } // Calculate total height including gaps + // Ensure items are rendered before checking height (iteratr pattern) totalHeight := 0 for i, item := range s.items { - totalHeight += item.Height() + // Render to get actual content (handles non-cached items like reasoning blocks) + rendered := item.Render(s.width) + itemHeight := strings.Count(rendered, "\n") + 1 + totalHeight += itemHeight // Add gap after each item except the last if s.itemGap > 0 && i < len(s.items)-1 { totalHeight += s.itemGap @@ -380,7 +384,9 @@ func (s *ScrollList) GotoBottom() { // Otherwise, position viewport at bottom remaining := totalHeight - s.height for idx := 0; idx < len(s.items); idx++ { - itemHeight := s.items[idx].Height() + // Render to get actual content + rendered := s.items[idx].Render(s.width) + itemHeight := strings.Count(rendered, "\n") + 1 if remaining < itemHeight { s.offsetIdx = idx s.offsetLine = remaining @@ -411,10 +417,13 @@ func (s *ScrollList) AtBottom() bool { } // Calculate visible height from current position including gaps + // Calculate height directly from rendered content (handles non-cached items) visibleHeight := 0 for idx := s.offsetIdx; idx < len(s.items); idx++ { item := s.items[idx] - itemHeight := item.Height() + // Render to get actual content + rendered := item.Render(s.width) + itemHeight := strings.Count(rendered, "\n") + 1 if idx == s.offsetIdx { visibleHeight += itemHeight - s.offsetLine