fix(ui): coalesce viewport listener via microtask to avoid setState loop

[aldevv]

User-visible: the TUI crashes with React's "Maximum update depth exceeded" when scrolling fast or rapidly pressing next/prev hunk, especially with comments attached.

Steps to reproduce (on main)

1. hunk diff <range> on a multi-file diff with several hunks.
2. Optional but reliable: attach a couple of agent comments via hunk session comment apply.
3. Hold ] (next hunk) or scroll continuously for a few seconds.
4. The TUI exits with Error: Maximum update depth exceeded and a stack trace ending at updateSliderFromScrollState → onChange → handleViewportChange → updateViewport → setState.

Fix: the scrollbar change listener now coalesces events through queueMicrotask, so the resulting setScrollViewport call isn't dispatched while a layout effect is already mid-commit. Initial mount still reads the viewport synchronously.

No user-facing docs or workflows change.

Fixes #233.
Fixes #227.

[greptile-apps]

Greptile Summary

This PR fixes a "Maximum update depth exceeded" React render loop in DiffPane by coalescing OpenTUI change and viewport events into a single microtask-deferred readViewport call, preventing setScrollViewport from dispatching inside React's commit phase.

• The scheduled flag collapses multiple synchronous event firings into one microtask per paint cycle, while cancelled guards against stale reads after the effect's cleanup runs.
• The initial synchronous readViewport() call is preserved so first-paint geometry is still correct.

Confidence Score: 4/5

Safe to merge — the coalescing logic is correct and the root-cause analysis matches the described crash.

The scheduled/cancelled flags are managed correctly across effect re-runs and cleanup. The only observation is a minor flag-reset ordering inconsistency that has no practical impact since listeners are always removed before the cancelled path can be reached.

No files require special attention beyond DiffPane.tsx, which contains the entire change.

Important Files Changed

Filename	Overview
src/ui/components/panes/DiffPane.tsx	Adds microtask coalescing to the viewport-change listener to avoid setState-during-commit render loops; logic is correct with a minor ordering note on scheduled = false.

Sequence Diagram

sequenceDiagram
    participant OT as OpenTUI scrollBar
    participant HL as handleViewportChange
    participant MT as queueMicrotask
    participant RV as readViewport
    participant R as React (setScrollViewport)

    Note over OT,R: Before fix — setState during commit
    OT->>HL: emit "change" (synchronously, inside commit)
    HL->>RV: readViewport() [direct call]
    RV->>R: setScrollViewport() — setState mid-commit — render loop

    Note over OT,R: After fix — coalesced microtask
    OT->>HL: emit "change" (event 1)
    HL->>MT: "scheduled=true, queueMicrotask(fn)"
    OT->>HL: emit "change" (event 2)
    HL-->>HL: "scheduled=true, return (dropped)"
    Note over MT: microtask checkpoint
    MT->>RV: "scheduled=false, cancelled check, readViewport()"
    RV->>R: setScrollViewport() — outside commit phase

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/ui/components/panes/DiffPane.tsx:359-365
The `scheduled = false` reset happens unconditionally before the `cancelled` guard. If `readViewport()` throws (e.g., the scrollbox is torn down between the microtask being queued and it running), `scheduled` is already `false` and a new event could queue a fresh microtask before the thrown exception propagates. Resetting `scheduled` only on the non-cancelled branch makes the intent clearer and keeps the flag consistent through the full life of the microtask.

```suggestion
      queueMicrotask(() => {
        if (cancelled) {
          scheduled = false;
          return;
        }
        scheduled = false;
        readViewport();
      });
```

_{Reviews (1): Last reviewed commit: "fix(ui): coalesce viewport listener via ..." | Re-trigger Greptile}

[greptile-apps]

The scheduled = false reset happens unconditionally before the cancelled guard. If readViewport() throws (e.g., the scrollbox is torn down between the microtask being queued and it running), scheduled is already false and a new event could queue a fresh microtask before the thrown exception propagates. Resetting scheduled only on the non-cancelled branch makes the intent clearer and keeps the flag consistent through the full life of the microtask.

Suggested change

	queueMicrotask(() => {
	scheduled = false;
	if (cancelled) {
	return;
	}
	readViewport();
	});
	queueMicrotask(() => {
	if (cancelled) {
	scheduled = false;
	return;
	}
	scheduled = false;
	readViewport();
	});

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/ui/components/panes/DiffPane.tsx
Line: 359-365

Comment:
The `scheduled = false` reset happens unconditionally before the `cancelled` guard. If `readViewport()` throws (e.g., the scrollbox is torn down between the microtask being queued and it running), `scheduled` is already `false` and a new event could queue a fresh microtask before the thrown exception propagates. Resetting `scheduled` only on the non-cancelled branch makes the intent clearer and keeps the flag consistent through the full life of the microtask.

```suggestion
      queueMicrotask(() => {
        if (cancelled) {
          scheduled = false;
          return;
        }
        scheduled = false;
        readViewport();
      });
```

How can I resolve this? If you propose a fix, please make it concise.

[benvinegar]

Updated the microtask to keep scheduled true while readViewport() runs, then reset it in a finally block. Cancelled microtasks still clear the flag before returning. This preserves coalescing through the full viewport read and keeps cleanup safe.

This comment was generated by Pi using OpenAI GPT-5

[benvinegar]

@aldevv This is great but could you submit a test case?