Reset the scroll to the top at first paint, not when the task starts

When a main view re-renders content different from what it last showed, the
scroll resets to the top. That reset fired synchronously when the task started —
but with the off-screen render the previous content stays displayed until the
swap, so resetting the origin up front scrolled that still-visible content to the
top before the new content replaced it: a distracting jump when switching commits
(or any item) while scrolled down.

Defer the reset to the task's first paint (the swap), alongside the restore that
already runs there: the previous content stays at its scroll until the new content
takes its place, then the new content appears at the top. A same-content re-render
keeps its scroll (no reset); a restore places the scroll itself. The "loading..."
indicator path also resets the origin now, since it clears the previous content to
show the message and must put it at the top.

The reset moves out of NewTask (it no longer needs the task key or the pending
restore for this) into the read loop, driven by LinesToRead.ResetOrigin, which the
cmd/pty wrappers set from the key comparison the reset used to do. The manager's
onNewKey callback is renamed resetOrigin to match its now-decoupled timing.

pkg/gui/pty.go

@@ -158,10 +158,14 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 		// focused main view on escape), let the task re-establish the scroll
 		// position and selection as it first paints, reading to end of input so a
 		// deep target line is found and the scrollbar ends up accurate.
-		if restore := manager.GetRestoreForNextTask(); restore != nil {
+		restore := manager.GetRestoreForNextTask()
+		if restore != nil {
 			linesToRead.Restore = restore
 			linesToRead.Total = -1
 		}
+		// New content scrolls back to the top at the first paint; same content keeps
+		// its scroll, and a restore places the scroll itself. See LinesToRead.ResetOrigin.
+		linesToRead.ResetOrigin = restore == nil && cmdStr != manager.GetTaskKey()
 		return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr)
 	})
 

pkg/gui/tasks_adapter.go

@@ -56,10 +56,15 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
 	// on escape), let the task re-establish the scroll position and selection as
 	// it first paints. It also reads to end of input so a deep target line is
 	// found and the scrollbar ends up accurate. See RenderRestore.
-	if restore := manager.GetRestoreForNextTask(); restore != nil {
+	restore := manager.GetRestoreForNextTask()
+	if restore != nil {
 		linesToRead.Restore = restore
 		linesToRead.Total = -1
 	}
+	// New content (a different command than the view last showed) scrolls back to
+	// the top, at the first paint; same content keeps its scroll, and a restore
+	// places the scroll itself. See LinesToRead.ResetOrigin.
+	linesToRead.ResetOrigin = restore == nil && cmdStr != manager.GetTaskKey()
 	if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr); err != nil {
 		gui.c.Log.Error(err)
 	}

pkg/tasks/tasks.go

@@ -72,7 +72,14 @@ type ViewBufferManager struct {
 	// thread; nil when no task is running.
 	readLines atomic.Pointer[chan LinesToRead]
 	taskKey   string
-	onNewKey  func()
+
+	// Resets the view's scroll position to the top. A render whose content is
+	// different from what the view last showed (a different command key) calls
+	// this — but at its *first paint*, not when the task starts: the off-screen
+	// render leaves the previous content displayed until the swap, so resetting
+	// the origin up front would scroll that still-displayed content to the top
+	// before the new content replaces it. See LinesToRead.ResetOrigin.
+	resetOrigin func()
 
 	// When non-nil, the next cmd/pty task re-establishes the view's scroll
 	// position and selection once it has re-rendered the content (returning to a
@@ -147,6 +154,15 @@ type LinesToRead struct {
 	// RenderRestore.
 	Restore *RenderRestore
 
+	// When true, the view's scroll position is reset to the top at the first paint
+	// (the swap), because this render's content differs from what the view last
+	// showed. Set for a new command key with no Restore; left false when
+	// re-rendering the same content (keep the scroll) or when a Restore will place
+	// the scroll itself. Doing it at the first paint rather than at task start
+	// keeps the previous content, still displayed off-screen until the swap, from
+	// visibly jumping to the top first.
+	ResetOrigin bool
+
 	// Function to call after reading the lines is done
 	Then func()
 }
@@ -181,7 +197,7 @@ func NewViewBufferManager(
 	beforeStart func(),
 	refreshView func(),
 	onEndOfInput func(),
-	onNewKey func(),
+	resetOrigin func(),
 	beginRender func(),
 	swapInRender func(),
 	newGocuiTask func() gocui.Task,
@@ -193,7 +209,7 @@ func NewViewBufferManager(
 		beforeStart:  beforeStart,
 		refreshView:  refreshView,
 		onEndOfInput: onEndOfInput,
-		onNewKey:     onNewKey,
+		resetOrigin:  resetOrigin,
 		beginRender:  beginRender,
 		swapInRender: swapInRender,
 		newGocuiTask: newGocuiTask,
@@ -385,6 +401,13 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
 				loadingMutex.Lock()
 				if !loaded {
 					self.beforeStart()
+					// beforeStart cleared the previous content to show "loading...". If
+					// this is new content, reset the scroll to the top now so the message
+					// is visible (beforeStart doesn't touch the origin); for a same-content
+					// re-render we keep the scroll, as the first paint will too.
+					if linesToRead.ResetOrigin && self.resetOrigin != nil {
+						self.resetOrigin()
+					}
 					_, _ = self.writer.Write([]byte("loading..."))
 					self.refreshView()
 				}
@@ -426,12 +449,14 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
 			linesRead := 0
 
 			// The first paint swaps the off-screen render in to reveal the new
-			// content. When a restore is pending (returning to a focused main view on
-			// escape, see RenderRestore), it scrolls to the saved position and
-			// restores the selection in the same step, so the real content first
-			// appears already at the right place rather than at the top. firstPaint
-			// happens once, either when we've read far enough (below) or at end of
-			// input for content shorter than that.
+			// content, and settles the scroll position in the same step — so the new
+			// content first appears already where it belongs. A restore (returning to
+			// a focused main view on escape, see RenderRestore) scrolls to its saved
+			// position and restores the selection; otherwise, if this is new content
+			// (ResetOrigin), the scroll resets to the top. Either way it happens at
+			// the swap, not before, so the previous content — displayed until the swap
+			// — doesn't visibly jump first. firstPaint happens once, either when we've
+			// read far enough (below) or at end of input for content shorter than that.
 			restore := linesToRead.Restore
 			painted := false
 			firstPaint := func() {
@@ -442,6 +467,8 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
 				self.swapInRender()
 				if restore != nil {
 					restore.Apply()
+				} else if linesToRead.ResetOrigin && self.resetOrigin != nil {
+					self.resetOrigin()
 				}
 			}
 
@@ -694,31 +721,18 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
 			return
 		}
 
-		// Reset the origin to the top when the command changed, unless a restore is
-		// pending: a restore re-establishes the scroll position itself (and we keep
-		// the placeholder showing at its current scroll until it does), so resetting
-		// to the top would just flicker. The restore isn't cleared here: it must
-		// outlive a task being stopped and replaced before it could paint, and it
-		// validates itself against the content it lands in (see restoreForNextTask),
-		// so a stale one can't apply to the wrong place — it's cleared once a task
-		// has applied it.
-		resetOrigin := self.GetTaskKey() != key && self.onNewKey != nil && self.restoreForNextTask == nil
+		// Note we don't reset the origin here even when the command key changed:
+		// that's deferred to the task's first paint (see resetOrigin / the task's
+		// ResetOrigin), so the previous content — left displayed off-screen until
+		// the swap — doesn't visibly jump to the top before the new content appears.
+		// The restore isn't cleared here either: it must outlive a task being stopped
+		// and replaced before it could paint, and it validates itself against the
+		// content it lands in (see restoreForNextTask), so a stale one can't apply to
+		// the wrong place — it's cleared once a task has applied it.
 		self.taskKey = key
 
 		self.taskIDMutex.Unlock()
 
-		if resetOrigin {
-			// onNewKey resets the view's scroll origin, which is view state the
-			// UI thread reads while laying out and drawing, so do it there. This
-			// must happen after releasing taskIDMutex: it blocks until the UI
-			// thread runs it, and a NewTask call on the UI thread takes
-			// taskIDMutex, so holding it here would deadlock.
-			_ = self.onUIThread(func() error {
-				self.onNewKey()
-				return nil
-			})
-		}
-
 		self.waitingMutex.Lock()
 
 		// Re-check staleness after acquiring waitingMutex: a newer task