mirror of
https://github.com/mark3labs/kit.git
synced 2026-06-14 03:30:26 +00:00
Fix autoscroll for streaming messages (iteratr pattern)
Root cause: GotoBottom() was calculating heights using Height() which returns 0 for non-cached items. Reasoning blocks never cache renders due to live duration updates, causing incorrect scroll calculations during reasoning → assistant transitions. Fix: Calculate heights directly from rendered strings instead of relying on cached Height() values. This ensures accurate scroll positioning for all message types. Changes: - ScrollList.GotoBottom(): Render items and calculate height from string - ScrollList.AtBottom(): Same pattern for bottom detection - appendStreamingChunk(): Call GotoBottom() directly for existing messages - refreshContent(): Remove redundant GotoBottom() (handled by SetItems) Tested with 'explore this repo' prompt - autoscroll now works correctly throughout reasoning and assistant streaming phases.
This commit is contained in:
@@ -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
|
||||
+4
-13
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user