Fix manual transcript reading position

[felipeggv]

Summary

• treat manual scroll-away intent as authoritative before the throttled React state update
• prevent settled final-response DOM mutations from pulling the transcript back to the bottom
• resume follow mode when the user returns to the bottom or uses the existing jump-to-bottom action
• add regression coverage for the race between manual scrolling and an internal DOM mutation

Validation

• 34,702 tests passed; 108 skipped
• renderer production build passed
• TypeScript, ESLint, and Prettier checks passed

Summary by CodeRabbit

• Bug Fixes
  • Preserved a manually selected reading position in terminal output when new content or DOM updates appear.
  • Prevented automatic scrolling from unexpectedly returning the view to the bottom after users scroll upward.
  • Improved scroll-position handling when switching sessions, restoring views, or resuming output.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1404cc81-b8c6-4805-836a-f0f6376aaede

📥 Commits

Reviewing files that changed from the base of the PR and between be414fe and b80d3ea.

📒 Files selected for processing (2)

• src/__tests__/renderer/components/TerminalOutput/useTerminalOutputScroll.test.ts
• src/renderer/components/TerminalOutput/hooks/useTerminalOutputScroll.ts

---

📝 Walkthrough

Walkthrough

useTerminalOutputScroll now tracks user scroll-away intent independently of throttled state updates and prevents automatic scrolling from overriding manually selected positions during DOM mutations, restoration, tab changes, and content updates. A regression test covers the delayed-update scenario.

Changes

Terminal scroll preservation

Layer / File(s)	Summary
Track scroll-away intent and lifecycle state src/renderer/components/TerminalOutput/hooks/useTerminalOutputScroll.ts	Adds synchronous scroll-away tracking and updates it across scroll handling, restoration, tab changes, read-state synchronization, resets, and resume operations.
Gate auto-scroll and verify mutation behavior src/renderer/components/TerminalOutput/hooks/useTerminalOutputScroll.ts, src/__tests__/renderer/components/TerminalOutput/useTerminalOutputScroll.test.ts	Requires the user to remain marked as following before automatic bottom scrolling or mutation pinning, with regression coverage for preserved manual positions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• RunMaestro/Maestro#1263: Changes terminal auto-scroll gating and adds coverage for preserving scroll position.
• RunMaestro/Maestro#1295: Updates scroll restoration and auto-follow gating around users reading away from the bottom.
• RunMaestro/Maestro#1181: Modifies terminal output scroll tracking and mutation-driven auto-scroll behavior.

Suggested labels: approved

Suggested reviewers: chr1syy, jsydorowicz21

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main user-facing fix: preserving manual transcript reading position.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR preserves a user's manual transcript position across throttled scroll handling and later DOM mutations.

• Tracks manual scroll-away intent synchronously before the throttled state update.
• Prevents mutation-driven bottom jumps while the user is reading earlier output.
• Clears manual intent when the user returns to the bottom or invokes jump-to-bottom.
• Adds regression coverage for the scroll-event and DOM-mutation race.

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

The immediate scroll-intent ref closes the throttling race while preserving existing restore, programmatic-scroll, and jump-to-bottom behavior, and the regression test covers the targeted failure.

Important Files Changed

Filename	Overview
src/renderer/components/TerminalOutput/hooks/useTerminalOutputScroll.ts	Adds synchronous user-intent tracking and consistently applies it to transcript follow, restore, mutation-observer, and resume paths.
src/tests/renderer/components/TerminalOutput/useTerminalOutputScroll.test.ts	Adds focused regression coverage proving that a settled DOM mutation does not override a recent manual scroll-away.

_{Reviews (1): Last reviewed commit: "fix: preserve manual transcript reading ..." | Re-trigger Greptile}

[pedramamini]

@felipeggv Thanks for the contribution, and for the thorough writeup. This is a good catch and a clean fix.

Verification

I pulled the branch and confirmed the regression coverage is real: reverting only useTerminalOutputScroll.ts back to base makes Case 6 fail with expected 9000 to be 2000 (the transcript gets yanked back to the bottom), and it passes with the fix in place. That's exactly the bug described. All 8 tests in the file pass on the PR head.

CI is green on all four legs (ubuntu-latest and windows-latest, both shards), plus lint-and-format. No merge conflicts, base is rc, and both Greptile and CodeRabbit came back with no actionable findings.

On the approach

The mechanism holds up. The root problem is that isAtBottomRef.current = isAtBottom runs on every render, so any synchronous write to that ref gets clobbered by the throttled React state on the next commit. A dedicated ref that nothing reassigns from state is the right way out, and splitting the intent capture into an unthrottled wrapper in front of the throttled handler keeps the leading/trailing-edge behavior intact. The atBottom / parkedAtProgrammaticTarget branching in the new handleScroll matches handleScrollInner exactly, so the two cannot disagree today, and every new gate is a strict && tightening, which keeps the failure mode on the safe side (show the jump-to-bottom pill rather than steal the scroll position).

Non-blocking follow-ups

Nothing here needs to change before merge, just flagging for whenever this file is next touched:

1. The at-bottom measurement is now duplicated verbatim between handleScroll and handleScrollInner, including the bare 50 slack. Since correctness depends on those two staying identical, a small shared measureScrollState(container) helper plus an AT_BOTTOM_SLACK_PX constant next to PROGRAMMATIC_TARGET_EPSILON_PX would make future divergence impossible rather than merely unlikely.
2. This hook now carries four overlapping notions of follow state: isAtBottom, isAtBottomRef, userScrolledAwayRef, and autoScrollPaused. Each was added for a real bug (Bug: Auto-scroll jumps output to bottom on re-render of long content #1140, J1, and now this one), but they are getting hard to reason about together. Worth a consolidation pass at some point.

Approving and tagging approved. Nice work.