From 03b4aa2995ebb45022cadb9b4f889a4ccce3f903 Mon Sep 17 00:00:00 2001 From: Omar Alani Date: Wed, 1 Jul 2026 20:27:22 -0500 Subject: [PATCH 1/3] fix(terminal): coalesce scroll events --- internal/terminal/app.go | 19 +++++ internal/terminal/render_internal_test.go | 70 ++++++++++++++++- internal/terminal/scroll.go | 96 +++++++++++++++++------ 3 files changed, 161 insertions(+), 24 deletions(-) diff --git a/internal/terminal/app.go b/internal/terminal/app.go index e3817aa..e3b21ad 100644 --- a/internal/terminal/app.go +++ b/internal/terminal/app.go @@ -390,6 +390,10 @@ func (app *App) handleLoopEvent(ctx context.Context, event tcell.Event) (shouldQ return app.drawLatestResize(ctx, resize) } + if delta, ok := app.scrollDeltaForEvent(event); ok { + return app.handleScrollLoopEvent(ctx, delta) + } + shouldQuit, err := app.handleEvent(ctx, event) if err != nil { app.addMessage(transcript.RoleCustom, err.Error()) @@ -408,6 +412,21 @@ func (app *App) handleLoopEvent(ctx context.Context, event tcell.Event) (shouldQ return false, true } +func (app *App) handleScrollLoopEvent(ctx context.Context, delta int) (shouldQuit, dirty bool) { + coalesced := app.coalesceScrollEvents(delta) + app.scrollTranscript(coalesced.Delta) + app.draw(ctx) + + if coalesced.Pending != nil { + shouldQuit, _ = app.handleLoopEvent(ctx, coalesced.Pending) + if shouldQuit { + return true, false + } + } + + return false, false +} + func (app *App) drawLatestResize(ctx context.Context, resize *tcell.EventResize) (shouldQuit, dirty bool) { pending := app.coalesceResizeEvents(resize) if pending.Resize != nil { diff --git a/internal/terminal/render_internal_test.go b/internal/terminal/render_internal_test.go index fc2c4e7..ad04adb 100644 --- a/internal/terminal/render_internal_test.go +++ b/internal/terminal/render_internal_test.go @@ -514,8 +514,57 @@ func TestMouseWheelScrollsTranscript(t *testing.T) { app.handleMouse(tcell.NewEventMouse(0, 0, tcell.WheelUp, tcell.ModNone)) - if app.scrollOffset != mouseScrollRows { - t.Fatalf("scroll offset = %d, want %d", app.scrollOffset, mouseScrollRows) + assert.Equal(t, mouseScrollRows, app.scrollOffset) +} + +func TestScrollLoopEventCoalescesQueuedScrolls(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + wantComposer string + queuedEvents []tcell.Event + wantOffset int + }{ + { + name: "folds queued scroll events into one draw", + wantComposer: "", + queuedEvents: []tcell.Event{ + tcell.NewEventMouse(0, 0, tcell.WheelUp, tcell.ModNone), + tcell.NewEventMouse(0, 0, tcell.WheelDown, tcell.ModNone), + }, + wantOffset: mouseScrollRows, + }, + { + name: "stops at first non-scroll event and handles it", + wantComposer: "x", + queuedEvents: []tcell.Event{ + tcell.NewEventMouse(0, 0, tcell.WheelUp, tcell.ModNone), + tcell.NewEventKey(tcell.KeyRune, "x", tcell.ModNone), + }, + wantOffset: mouseScrollRows * 2, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + must := require.New(t) + app := newScrollableRenderTestApp(t) + + events := app.screen.EventQ() + for _, event := range testCase.queuedEvents { + events <- event + } + + shouldQuit, dirty := app.handleScrollLoopEvent(context.Background(), mouseScrollRows) + + must.False(shouldQuit) + must.False(dirty) + assert.Equal(t, testCase.wantOffset, app.scrollOffset) + assert.Equal(t, testCase.wantComposer, app.composerBuffer.TextValue()) + }) } } @@ -1421,6 +1470,23 @@ func newRenderTestApp(t *testing.T) *App { return app } +func newScrollableRenderTestApp(t *testing.T) *App { + t.Helper() + + screen := newClipboardScreen() + screen.SetSize(40, 8) + + app := newRenderTestApp(t) + app.screen = screen + app.renderer = tui.NewRenderer(screen) + + for range 20 { + app.addMessage(transcript.RoleAssistant, "scrollable content") + } + + return app +} + func newTestAsyncEvent(kind asyncEventKind, text string) *asyncEvent { return &asyncEvent{ Response: nil, diff --git a/internal/terminal/scroll.go b/internal/terminal/scroll.go index d66b132..6b0e21f 100644 --- a/internal/terminal/scroll.go +++ b/internal/terminal/scroll.go @@ -7,20 +7,20 @@ const ( mouseScrollRows = 2 ) -func (app *App) handleTranscriptScroll(event *tcell.EventKey) bool { - if app.keys.matches(event, actionSelectPageUp) { - app.scrollTranscript(keyboardScrollRows) +type coalescedScrollEvent struct { + Pending tcell.Event + Delta int +} - return true +func (app *App) handleTranscriptScroll(event *tcell.EventKey) bool { + delta, ok := app.keyScrollDelta(event) + if !ok { + return false } - if app.keys.matches(event, actionSelectPageDown) { - app.scrollTranscript(-keyboardScrollRows) - - return true - } + app.scrollTranscript(delta) - return false + return true } func (app *App) handleMouse(event *tcell.EventMouse) { @@ -28,22 +28,14 @@ func (app *App) handleMouse(event *tcell.EventMouse) { return } - column, row := event.Position() - - buttons := event.Buttons() - if buttons&tcell.WheelUp != 0 { - app.scrollTranscript(mouseScrollRows) + if delta, ok := app.mouseScrollDelta(event); ok { + app.scrollTranscript(delta) return } - if buttons&tcell.WheelDown != 0 { - app.scrollTranscript(-mouseScrollRows) - - return - } - - if buttons&tcell.ButtonPrimary != 0 { + column, row := event.Position() + if event.Buttons()&tcell.ButtonPrimary != 0 { if app.selection.active { app.updateMouseSelection(column, row) @@ -60,6 +52,66 @@ func (app *App) handleMouse(event *tcell.EventMouse) { } } +func (app *App) scrollDeltaForEvent(event tcell.Event) (int, bool) { + switch typedEvent := event.(type) { + case *tcell.EventKey: + return app.keyScrollDelta(typedEvent) + case *tcell.EventMouse: + if app.mode != modeChat { + return 0, false + } + + return app.mouseScrollDelta(typedEvent) + default: + return 0, false + } +} + +func (app *App) keyScrollDelta(event *tcell.EventKey) (int, bool) { + if app.keys.matches(event, actionSelectPageUp) { + return keyboardScrollRows, true + } + + if app.keys.matches(event, actionSelectPageDown) { + return -keyboardScrollRows, true + } + + return 0, false +} + +func (app *App) mouseScrollDelta(event *tcell.EventMouse) (int, bool) { + buttons := event.Buttons() + if buttons&tcell.WheelUp != 0 { + return mouseScrollRows, true + } + + if buttons&tcell.WheelDown != 0 { + return -mouseScrollRows, true + } + + return 0, false +} + +func (app *App) coalesceScrollEvents(delta int) coalescedScrollEvent { + coalesced := coalescedScrollEvent{Pending: nil, Delta: delta} + + for { + select { + case event := <-app.screen.EventQ(): + nextDelta, ok := app.scrollDeltaForEvent(event) + if !ok { + coalesced.Pending = event + + return coalesced + } + + coalesced.Delta += nextDelta + default: + return coalesced + } + } +} + func (app *App) scrollTranscript(delta int) { app.scrollOffset = max(0, app.scrollOffset+delta) } From 8b2581da5dfb2a66cda6b6fe5627732ef89741b0 Mon Sep 17 00:00:00 2001 From: Omar Alani Date: Wed, 1 Jul 2026 22:09:44 -0500 Subject: [PATCH 2/3] fix(terminal): preserve dirty state during scroll coalescing --- internal/terminal/app.go | 5 +- internal/terminal/render_internal_test.go | 297 +++++++++++++++++++++- internal/terminal/scroll.go | 8 +- 3 files changed, 301 insertions(+), 9 deletions(-) diff --git a/internal/terminal/app.go b/internal/terminal/app.go index e3b21ad..e1a3b27 100644 --- a/internal/terminal/app.go +++ b/internal/terminal/app.go @@ -418,10 +418,7 @@ func (app *App) handleScrollLoopEvent(ctx context.Context, delta int) (shouldQui app.draw(ctx) if coalesced.Pending != nil { - shouldQuit, _ = app.handleLoopEvent(ctx, coalesced.Pending) - if shouldQuit { - return true, false - } + return app.handleLoopEvent(ctx, coalesced.Pending) } return false, false diff --git a/internal/terminal/render_internal_test.go b/internal/terminal/render_internal_test.go index ad04adb..69b3aeb 100644 --- a/internal/terminal/render_internal_test.go +++ b/internal/terminal/render_internal_test.go @@ -506,6 +506,49 @@ func TestScrollTranscriptDoesNotDrawImmediately(t *testing.T) { } } +func TestHandleTranscriptScroll(t *testing.T) { + t.Parallel() + + testCases := []struct { + event *tcell.EventKey + name string + wantOffset int + wantOK bool + }{ + { + name: "page up scrolls up", + event: tcell.NewEventKey(tcell.KeyPgUp, "", tcell.ModNone), + wantOffset: keyboardScrollRows, + wantOK: true, + }, + { + name: "page down scrolls down but clamps at top", + event: tcell.NewEventKey(tcell.KeyPgDn, "", tcell.ModNone), + wantOffset: 0, + wantOK: true, + }, + { + name: "non-scroll key is ignored", + event: tcell.NewEventKey(tcell.KeyRune, "x", tcell.ModNone), + wantOffset: 0, + wantOK: false, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + app := newRenderTestApp(t) + + ok := app.handleTranscriptScroll(testCase.event) + + assert.Equal(t, testCase.wantOK, ok) + assert.Equal(t, testCase.wantOffset, app.scrollOffset) + }) + } +} + func TestMouseWheelScrollsTranscript(t *testing.T) { t.Parallel() @@ -517,6 +560,82 @@ func TestMouseWheelScrollsTranscript(t *testing.T) { assert.Equal(t, mouseScrollRows, app.scrollOffset) } +func TestScrollDeltaForEvent(t *testing.T) { + t.Parallel() + + testCases := []struct { + event tcell.Event + name string + mode appMode + wantDelta int + wantOK bool + }{ + { + name: "mouse wheel up in chat mode", + event: tcell.NewEventMouse(0, 0, tcell.WheelUp, tcell.ModNone), + mode: modeChat, + wantDelta: mouseScrollRows, + wantOK: true, + }, + { + name: "mouse wheel down in chat mode", + event: tcell.NewEventMouse(0, 0, tcell.WheelDown, tcell.ModNone), + mode: modeChat, + wantDelta: -mouseScrollRows, + wantOK: true, + }, + { + name: "page up key in chat mode", + event: tcell.NewEventKey(tcell.KeyPgUp, "", tcell.ModNone), + mode: modeChat, + wantDelta: keyboardScrollRows, + wantOK: true, + }, + { + name: "page down key in chat mode", + event: tcell.NewEventKey(tcell.KeyPgDn, "", tcell.ModNone), + mode: modeChat, + wantDelta: -keyboardScrollRows, + wantOK: true, + }, + { + name: "non-scroll key in chat mode", + event: tcell.NewEventKey(tcell.KeyRune, "x", tcell.ModNone), + mode: modeChat, + wantDelta: 0, + wantOK: false, + }, + { + name: "mouse wheel outside chat mode", + event: tcell.NewEventMouse(0, 0, tcell.WheelUp, tcell.ModNone), + mode: modePanel, + wantDelta: 0, + wantOK: false, + }, + { + name: "page up key outside chat mode", + event: tcell.NewEventKey(tcell.KeyPgUp, "", tcell.ModNone), + mode: modePanel, + wantDelta: 0, + wantOK: false, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + app := newRenderTestApp(t) + app.mode = testCase.mode + + delta, ok := app.scrollDeltaForEvent(testCase.event) + + assert.Equal(t, testCase.wantDelta, delta) + assert.Equal(t, testCase.wantOK, ok) + }) + } +} + func TestScrollLoopEventCoalescesQueuedScrolls(t *testing.T) { t.Parallel() @@ -525,6 +644,7 @@ func TestScrollLoopEventCoalescesQueuedScrolls(t *testing.T) { wantComposer string queuedEvents []tcell.Event wantOffset int + wantDirty bool }{ { name: "folds queued scroll events into one draw", @@ -534,6 +654,7 @@ func TestScrollLoopEventCoalescesQueuedScrolls(t *testing.T) { tcell.NewEventMouse(0, 0, tcell.WheelDown, tcell.ModNone), }, wantOffset: mouseScrollRows, + wantDirty: false, }, { name: "stops at first non-scroll event and handles it", @@ -543,6 +664,16 @@ func TestScrollLoopEventCoalescesQueuedScrolls(t *testing.T) { tcell.NewEventKey(tcell.KeyRune, "x", tcell.ModNone), }, wantOffset: mouseScrollRows * 2, + wantDirty: false, + }, + { + name: "propagates pending event dirty state", + wantComposer: "", + queuedEvents: []tcell.Event{ + tcell.NewEventInterrupt(asyncTestEvent(asyncEventPromptDelta, "", "chunk", 1)), + }, + wantOffset: mouseScrollRows, + wantDirty: true, }, } @@ -552,6 +683,8 @@ func TestScrollLoopEventCoalescesQueuedScrolls(t *testing.T) { must := require.New(t) app := newScrollableRenderTestApp(t) + app.activePrompt = newTestActivePrompt(nil) + app.activePrompt.ID = 1 events := app.screen.EventQ() for _, event := range testCase.queuedEvents { @@ -561,13 +694,175 @@ func TestScrollLoopEventCoalescesQueuedScrolls(t *testing.T) { shouldQuit, dirty := app.handleScrollLoopEvent(context.Background(), mouseScrollRows) must.False(shouldQuit) - must.False(dirty) + assert.Equal(t, testCase.wantDirty, dirty) + assert.Equal(t, testCase.wantOffset, app.scrollOffset) + assert.Equal(t, testCase.wantComposer, app.composerBuffer.TextValue()) + }) + } +} + +func TestRunLoopStepHandlesQueuedEvents(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + event tcell.Event + wantComposer string + wantOffset int + wantQuit bool + wantDirty bool + }{ + { + name: "nil event quits", + event: nil, + wantComposer: "", + wantOffset: 0, + wantQuit: true, + wantDirty: false, + }, + { + name: "key event draws immediately", + event: tcell.NewEventKey(tcell.KeyRune, "x", tcell.ModNone), + wantComposer: "x", + wantOffset: 0, + wantQuit: false, + wantDirty: false, + }, + { + name: "high-volume async event returns dirty", + event: tcell.NewEventInterrupt(asyncTestEvent(asyncEventPromptDelta, "", "chunk", 1)), + wantComposer: "", + wantOffset: 0, + wantQuit: false, + wantDirty: true, + }, + { + name: "scroll event draws immediately", + event: tcell.NewEventMouse(0, 0, tcell.WheelUp, tcell.ModNone), + wantComposer: "", + wantOffset: mouseScrollRows, + wantQuit: false, + wantDirty: false, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + app := newScrollableRenderTestApp(t) + app.activePrompt = newTestActivePrompt(nil) + app.activePrompt.ID = 1 + + app.screen.EventQ() <- testCase.event + + shouldQuit, dirty := runLoopStepWithDormantTimers(t, app) + + assert.Equal(t, testCase.wantQuit, shouldQuit) + assert.Equal(t, testCase.wantDirty, dirty) assert.Equal(t, testCase.wantOffset, app.scrollOffset) assert.Equal(t, testCase.wantComposer, app.composerBuffer.TextValue()) }) } } +func runLoopStepWithDormantTimers(t *testing.T, app *App) (shouldQuit, dirty bool) { + t.Helper() + + workTicker := time.NewTicker(time.Hour) + frameTicker := time.NewTicker(time.Hour) + extensionTimer := time.NewTimer(time.Hour) + messageWarmTimer := time.NewTimer(time.Hour) + + t.Cleanup(func() { + workTicker.Stop() + frameTicker.Stop() + stopTimer(extensionTimer) + stopTimer(messageWarmTimer) + }) + + return app.runLoopStep( + context.Background(), + workTicker, + frameTicker, + extensionTimer, + messageWarmTimer, + false, + ) +} + +func TestDrawDirtyFrame(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + dirty bool + }{ + {name: "clean frame skips draw", dirty: false}, + {name: "dirty frame draws", dirty: true}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + app := newScrollableRenderTestApp(t) + + assert.False(t, app.drawDirtyFrame(context.Background(), testCase.dirty)) + }) + } +} + +func TestDrawLatestResize(t *testing.T) { + t.Parallel() + + testCases := []struct { + queuedEvent tcell.Event + name string + wantComposer string + wantLastWidth int + wantLastHeight int + }{ + { + name: "uses latest queued resize", + queuedEvent: tcell.NewEventResize(100, 40), + wantComposer: "", + wantLastWidth: 100, + wantLastHeight: 40, + }, + { + name: "stops at pending non-resize event", + queuedEvent: tcell.NewEventKey(tcell.KeyRune, "x", tcell.ModNone), + wantComposer: "x", + wantLastWidth: 80, + wantLastHeight: 24, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + app := newRenderTestApp(t) + screen := newClipboardScreen() + app.screen = screen + app.renderer = tui.NewRenderer(screen) + + app.screen.EventQ() <- testCase.queuedEvent + + shouldQuit, dirty := app.drawLatestResize(context.Background(), tcell.NewEventResize(80, 24)) + + lastWidth, lastHeight := app.lastResize.Size() + + assert.False(t, shouldQuit) + assert.False(t, dirty) + assert.Equal(t, testCase.wantComposer, app.composerBuffer.TextValue()) + assert.Equal(t, testCase.wantLastWidth, lastWidth) + assert.Equal(t, testCase.wantLastHeight, lastHeight) + }) + } +} + func TestMouseSelectionCopiesFrameText(t *testing.T) { t.Parallel() diff --git a/internal/terminal/scroll.go b/internal/terminal/scroll.go index 6b0e21f..330968e 100644 --- a/internal/terminal/scroll.go +++ b/internal/terminal/scroll.go @@ -53,14 +53,14 @@ func (app *App) handleMouse(event *tcell.EventMouse) { } func (app *App) scrollDeltaForEvent(event tcell.Event) (int, bool) { + if app.mode != modeChat { + return 0, false + } + switch typedEvent := event.(type) { case *tcell.EventKey: return app.keyScrollDelta(typedEvent) case *tcell.EventMouse: - if app.mode != modeChat { - return 0, false - } - return app.mouseScrollDelta(typedEvent) default: return 0, false From 360743970e9118fe57616c6333b89bc46bf55d16 Mon Sep 17 00:00:00 2001 From: Omar Alani Date: Wed, 1 Jul 2026 22:19:04 -0500 Subject: [PATCH 3/3] test(terminal): assert dirty frame rendering --- internal/terminal/render_internal_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/terminal/render_internal_test.go b/internal/terminal/render_internal_test.go index 69b3aeb..43fc8e7 100644 --- a/internal/terminal/render_internal_test.go +++ b/internal/terminal/render_internal_test.go @@ -795,11 +795,12 @@ func TestDrawDirtyFrame(t *testing.T) { t.Parallel() testCases := []struct { - name string - dirty bool + name string + dirty bool + wantDraw bool }{ - {name: "clean frame skips draw", dirty: false}, - {name: "dirty frame draws", dirty: true}, + {name: "clean frame skips draw", dirty: false, wantDraw: false}, + {name: "dirty frame draws", dirty: true, wantDraw: true}, } for _, testCase := range testCases { @@ -807,8 +808,11 @@ func TestDrawDirtyFrame(t *testing.T) { t.Parallel() app := newScrollableRenderTestApp(t) + screen, ok := app.screen.(*clipboardScreen) + require.True(t, ok) assert.False(t, app.drawDirtyFrame(context.Background(), testCase.dirty)) + assert.Equal(t, testCase.wantDraw, len(screen.content) > 0) }) } }