Repository navigation
fix(chat): release held disclosure anchor when reader scrolls up - #1855
jiangchuanso wants to merge 1 commit into
Conversation
A reader scroll-up (releasedFollow) must also drop any disclosure anchor the transcript is holding, matching what markScrollGesture already does for real input. Otherwise a held anchor can be restored by the ResizeObserver during a later stream and yank the viewport back, stealing the reader's scroll position while they read history.
9fa2f7c to
c031985
Compare
vastsa
left a comment
There was a problem hiding this comment.
Thanks for the fix and the long-history coverage. I could not verify the reported gap on the current event path: the fixture dispatches a bubbling WheelEvent on .thread-scroll, and the existing listener on .thread-wrap classifies it as an accepted wheel (readScrollInputContext sees the transcript as the same owner), so markScrollGesture already calls releaseDisclosureAnchor() before handleScroll runs.
The added call also runs for layout-driven upward movement. releasedFollow is computed as movedUp && distanceFromBottom > 0 without checking gesturing, while the following branch explicitly treats non-gesture events as programmatic/layout noise. That can drop a held disclosure title during a layout clamp and stop the observer from preserving it through the transition. Please either demonstrate a real scroll path that bypasses the existing listener, or limit this release to an accepted gesture and cover the layout-clamp case as well. Thanks!
| // anchor, exactly as a real gesture does in `markScrollGesture`. Without | ||
| // this, a held anchor can still be restored by the ResizeObserver during a | ||
| // later stream and steal the reader's position. | ||
| if (transition.releasedFollow) releaseDisclosureAnchor(); |
There was a problem hiding this comment.
This condition is broader than the stated user-scroll case: releasedFollow can be true for a non-gesture layout clamp, which this handler immediately classifies as layout noise below. Releasing the disclosure anchor here can make the next observer frame lose the held title. The PR's E2E wheel event already bubbles to markScrollGesture, which releases the anchor earlier.
Summary
Fixes a transcript follow-scroll regression surfaced by the upstream
trusted-extensions-e2eworkflow: thetranscriptLongHistoryProbelongHistoryphase asserts"stream stole the reader's scroll position"(scripts/e2e/transcript-long-history.tsx:280). When the reader has scrolled up away from the bottom, an incoming stream delta must not move the viewport.Root cause
Clicking a disclosure header (e.g.
.tool-activity-header) holds a disclosure anchor so the toggled title stays in view during its height transition (disclosure.tsx→notifyAnchor→onHold→releaseFollow). That anchor is released only bymarkScrollGesture(real input: wheel / touch / key / pointer). When the reader scrolls up without that path releasing it — which is exactly what the test simulates with a synthetic gesture + manualscrollTop— the anchor stays held. TheResizeObserverthen runsfollowScrollNow→restoreDisclosureAnchor(), which re-asserts the held position during a later stream and yanks the viewport back, stealing the reader's position.Fix
In
handleScroll, whentransition.releasedFollowis true (the reader moved up and took the viewport back), also callreleaseDisclosureAnchor(), mirroring whatmarkScrollGesturealready does for real gestures. This guarantees a held anchor cannot outlive a reader scroll-up.releasedFollowbranches are unchanged).releasedFollowis only true once the reader actually scrolls up.Validation
The
longHistorytranscript e2e covers this exact scenario (disclosure click → while-reading stream → reader scrolls up → reader-stays stream → position must not move). Upstream CI will run it.