fix(virtual-core): sync scrollOffset in applyScrollAdjustment so end-anchored resize is not lost to browser clamp#1209
Merged
piecyk merged 1 commit intoJun 26, 2026
Conversation
…end-anchored resize is not lost to browser clamp With `anchorTo: 'end'` and a dynamically growing last item, `resizeItem` calls `applyScrollAdjustment` before `notify()`, so the `scrollTop` write lands while the sizer is still at the old `getTotalSize()`. The browser clamps the write, no scroll event fires, and `scrollOffset` stays stale. The next tick's `getVirtualDistanceFromEnd()` then exceeds `scrollEndThreshold`, `wasAtEnd` flips false, and the viewport drifts away from the end from the second resize onward. Carry the intended target in `scrollOffset` (and zero `scrollAdjustments` to keep their sum invariant) the same way the prepend path in `setOptions` does (TanStack#1176), so the next `wasAtEnd` check sees the post-adjustment position regardless of whether the DOM write was clamped.
📝 WalkthroughWalkthrough
ChangesEnd-anchored scroll adjustment sync
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
mds-ant
marked this pull request as ready for review
June 25, 2026 16:21
Contributor
Author
|
@piecyk Does this look good to you? |
piecyk
approved these changes
Jun 26, 2026
piecyk
left a comment
piecyk
left a comment
Collaborator
There was a problem hiding this comment.
Nice fix 👏 Let's 🚢 🇮🇹
Just a side note, and non-blocking: the iOS path isn’t covered here. The isIOSWebKit() && scrolling branch defers and never reaches the new code, so the same drift could still happen there.
There are a few broader issues around isIOSWebKit, so I’ll add this there.
|
View your CI Pipeline Execution ↗ for commit 75a5a2b
☁️ Nx Cloud last updated this comment at |
Collaborator
|
Thanks @mds-ant |
Merged
Merged
4 tasks
5 tasks
jonastemplestein
added a commit
to iterate/iterate
that referenced
this pull request
Jul 10, 2026
…dinate alignment; meter under composer (#1848) ## Summary / motivation Rebased on #1847 (which fixed the core agent-feed virtualization — verified live and it holds). This PR covers what's still broken or missing on main, with the headline change being **how the feed follows its tail**. ## Why hand-roll tail-following when TanStack Virtual ships chat support? Fair question — we audited this hard, including an adversarial subagent review whose explicit mandate was to prove the custom hook unnecessary by reading the virtual-core 3.17.3 source. The library's chat machinery (`anchorTo: 'end'`, `followOnAppend`, the `wasAtEnd` resize compensation) genuinely solves stick-to-bottom **for the world it assumes: a fixed-height scroll viewport**. Its official chat example scrolls inside a fixed 600px box. Our feeds violate that assumption by product requirement: **The composer below the feed has variable height** — the input grows with multi-line drafts, attachment chips stack, the token strip renders. When it grows, the feed viewport shrinks with **no scroll event fired**, and the library has no code path that reacts: - Verified in source (two independent reads): a scroll-element rect change only updates `scrollRect` and recalculates the visible range. `anchorTo` is consulted in exactly two places — the count-change block in `setOptions` and the item-resize compensation in `resizeItem` — neither reachable from a viewport resize. No re-pin exists. - Worse, viewport shrinkage **poisons the library's own follow gate**: `followOnAppend` only fires when `isAtEnd(scrollEndThreshold)` was true before an append. Each composer growth silently increases the reader's distance-from-end (scrollTop stays put while maxScrollOffset grows); once cumulative growth passes the 80px threshold, the library concludes the reader scrolled away and **stops following appends entirely** — stranded ~80px up, permanently, with no user action to explain it. There is no option, callback, or layout trick that closes this. (We checked the obvious restructure — absolutely overlaying the composer so the scroller never resizes — but then the feed needs bottom clearance equal to the composer's variable height, and `paddingEnd` changes trigger no re-pin path either. Same problem, moved.) ## The fix: `useStickToBottom` — tail-following in DOM truth A ~95-line hook (`apps/os/src/lib/use-stick-to-bottom.ts`): while the reader is "stuck", keep `scrollTop = scrollHeight`, measured on the real elements. - **One `ResizeObserver`** on the scroll element + the content sizer. Content growth (appends, streaming, skeleton→content swaps, late measurements) and viewport changes (composer growing/shrinking) both fire it. No timers, no polling, no rAF loops. - **Release** on real user intent: upward wheel / touchstart / keydown / pointerdown. A reader in history is never yanked. (Downward wheel at the bottom deliberately does not release — that's not leaving-the-tail intent.) - **Re-stick** when a user scroll lands back within 2px of the bottom — Slack semantics. - Every stick write fires a normal scroll event, which is what keeps the virtualizer's internal offset synced to DOM truth. This is not an exotic pattern: it's the same architecture as StackBlitz's `use-stick-to-bottom` (the de-facto standard under AI chat UIs — content ResizeObserver + direct scrollTop writes + escape-on-user-intent), except ours also observes the scroll element itself, which is precisely the piece the variable composer requires. Ours is ~95 lines to their ~700 because we skip spring animations. Once the hook exists (it must, for the resize case), `followOnAppend` goes **off** rather than running two concurrent scroll-writers: the library's follow drives an *uncancellable* multi-frame reconcile loop (TanStack/virtual#1221) toward its internal notion of the end, and single-ownership is strictly easier to reason about. The virtualizer keeps everything it's genuinely good at: - `anchorTo: 'end'` — measurement compensation so readers **mid-history** aren't shifted by late row measurements below them. - `paddingStart`/`paddingEnd` — the feeds' vertical breathing room moved from wrapper CSS padding (chrome the virtualizer can't see, which skewed every DOM-vs-virtual end computation by 44px and caused the original "newest message hidden below the fold" bug) into the virtualizer's own coordinate space, per the API's intent. - Windowing, measurement, and #1847's tail-anchored opening row window (unchanged). The adversarial review's verdict, having read the library source against every alternative configuration: **right-sized** — every simpler library-native configuration fails the variable-composer requirement, and the layout escape hatch relocates the problem instead of removing it. Its three concrete demands (directional wheel release, two stale comments, documenting the 2px/80px interaction band) are all applied. ## Also in this PR - **Upgrade `@tanstack/react-virtual` 3.14.2 → 3.14.5** (core 3.17.0 → 3.17.3): upstream fixes for exactly the failure modes we first hit — streaming-pin death against a stale clamped scrollHeight (TanStack/virtual#1209) and measurement-wave compensation while scrolled up (#1199, #1212). - **Router scroll restoration opt-out for stream pages** (`scrollRestoration`'s function form): restoration records *any* scrolled element by CSS path and re-applies stale positions on render, racing the open-at-latest behavior. Feeds always open at the newest message, so the location opts out. - **Context meter under the composer**, right-aligned with `px-4` so it sits inside the pill's `rounded-3xl` radius (was above it, misaligned). - **Paperclip / + alignment**: icon centers now on the same axis (`mb-1`, measured `deltaY = 0`). - **streams-example-app**: completes the `directDomUpdates` contract it was violating (JSX wrote `minHeight`/`translateY` which that mode owns; `measureElement` was conditional so pending rows were never positioned). Now verbatim the first-party chat example's shape. ## Proof (live browser against local dev, CDP-measured, ~180-message thread) | Scenario | Result | | --- | --- | | Open long thread | `distanceFromBottom: 0`, 29 of ~180 rows mounted | | **Composer grows 4 lines while pinned** (viewport −90px, no scroll event) | stays at exact bottom | | Burst: 12 rapid appends w/ varied heights | pinned throughout; final message fully visible | | Append while reading history (wheel-up + scroll) | viewport does not move | | Scroll back to bottom, then append | re-stuck: new message chased and fully visible | | Downward wheel at bottom, then append | still stuck (directional release) | | Reload (stale saved scroll position present) | opens at exact bottom | | Clear client DB → full mirror replay | ends at exact bottom, composer visible | - [x] `pnpm typecheck`, `pnpm lint`, `pnpm format` - [x] apps/os tests: 967 passed - [x] react-doctor + rules-of-hooks/exhaustive-deps: zero findings in PR-touched files (linter validated against a seeded violation) - [x] streams-example-app playwright: pass/fail set identical to an origin/main baseline (10 pre-existing local-env failures; the suite must run **without** the OS doppler env or its vite server boots auth-enabled and dies on the sign-in page) - [x] Full CI green including preview deploy + e2e 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CLOUDFLARE_PREVIEW --> ## Environment Config Lease <!-- CLOUDFLARE_PREVIEW_STATE --> <!-- { "apps": { "os": { "appDisplayName": "OS", "appSlug": "os", "status": "deployed", "updatedAt": "2026-07-10T13:05:30.809Z", "headSha": "88f7cde26f6aee15df3be7f4bfd72954684c1d36", "message": null, "publicUrl": "https://os.iterate-preview-5.com", "runUrl": "https://github.com/iterate/iterate/actions/runs/411401509079840", "shortSha": "88f7cde", "deployDurationMs": 81171, "testDurationMs": 131926, "testRetries": "3 retried: provided integrations > a project mounts ocado into the collection; connections + secret confinement hold (vitest x1) · DESIRED: a live-capability fetch handler serves WebSockets at an app host (vitest x1) · reactivity page appends a batch and renders every delivered marker (specs x1)", "workerSizeKib": 15743.44, "workerGzipKib": 4257.29, "mainWorkerGzipKib": 4257.29 }, "semaphore": { "appDisplayName": "Semaphore", "appSlug": "semaphore", "status": "deployed", "updatedAt": "2026-07-10T12:52:45.292Z", "headSha": "3661d6268a0041348e03cc5363d41af3832b307f", "message": null, "publicUrl": "https://semaphore.iterate-preview-5.com", "runUrl": "https://github.com/iterate/iterate/actions/runs/570786174103773", "shortSha": "3661d62", "deployDurationMs": 14406, "testDurationMs": 4838, "testRetries": null, "workerSizeKib": 1914.76, "workerGzipKib": 440.78, "mainWorkerGzipKib": null }, "auth": { "appDisplayName": "Auth", "appSlug": "auth", "status": "deployed", "updatedAt": "2026-07-10T13:03:19.405Z", "headSha": "88f7cde26f6aee15df3be7f4bfd72954684c1d36", "message": null, "publicUrl": "https://auth.iterate-preview-5.com", "runUrl": "https://github.com/iterate/iterate/actions/runs/411401509079840", "shortSha": "88f7cde", "deployDurationMs": 19830, "testDurationMs": 521, "testRetries": null, "workerSizeKib": 4031.55, "workerGzipKib": 819.37, "mainWorkerGzipKib": null }, "streams-example-app": { "appDisplayName": "Streams Example App", "appSlug": "streams-example-app", "status": "deployed", "updatedAt": "2026-07-10T12:52:57.517Z", "headSha": "3661d6268a0041348e03cc5363d41af3832b307f", "message": null, "publicUrl": "https://streams.iterate-preview-5.com", "runUrl": "https://github.com/iterate/iterate/actions/runs/570786174103773", "shortSha": "3661d62", "deployDurationMs": 15612, "testDurationMs": 17060, "testRetries": null, "workerSizeKib": 4708.81, "workerGzipKib": 1354.9, "mainWorkerGzipKib": null } }, "environmentConfigLease": { "dopplerConfig": "preview_5", "leasedUntil": 1783774996957, "leaseId": "15aec379-e907-4cdf-b132-1e38d3d4953d", "slug": "preview-5", "type": "environment-config-lease" }, "notice": null } --> <!-- /CLOUDFLARE_PREVIEW_STATE --> <details> <summary>Lease: preview-5 | Doppler config: preview_5 | Type: environment-config-lease | Leased until: 2026-07-11T13:03:16.957Z</summary> | app | status | commit | preview | size (gzip) | deploy duration | test duration | retries | cleanup duration | workflow run | updated | summary | | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | | Auth | deployed | `88f7cde` | [https://auth.iterate-preview-5.com](https://auth.iterate-preview-5.com) | 819.4 KiB | 19.8s | 521ms | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/411401509079840) | 2026-07-10T13:03:19.405Z | | | OS | deployed | `88f7cde` | [https://os.iterate-preview-5.com](https://os.iterate-preview-5.com) | 4.16 MiB (±0 vs main) | 81.2s | 131.9s | 3 retried: provided integrations > a project mounts ocado into the collection; connections + secret confinement hold (vitest x1) · DESIRED: a live-capability fetch handler serves WebSockets at an app host (vitest x1) · reactivity page appends a batch and renders every delivered marker (specs x1) | | [Workflow run](https://github.com/iterate/iterate/actions/runs/411401509079840) | 2026-07-10T13:05:30.809Z | | | Semaphore | deployed | `3661d62` | [https://semaphore.iterate-preview-5.com](https://semaphore.iterate-preview-5.com) | 440.8 KiB | 14.4s | 4.8s | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/570786174103773) | 2026-07-10T12:52:45.292Z | | | Streams Example App | deployed | `3661d62` | [https://streams.iterate-preview-5.com](https://streams.iterate-preview-5.com) | 1.32 MiB | 15.6s | 17.1s | | | [Workflow run](https://github.com/iterate/iterate/actions/runs/570786174103773) | 2026-07-10T12:52:57.517Z | | </details> <!-- /CLOUDFLARE_PREVIEW --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches core chat scroll behavior and router scroll restoration on stream routes; regressions would show as wrong open position or tail not following composer growth, but changes are localized to feed UI and a small hook. > > **Overview** > Adds **`useStickToBottom`** so agent and raw feed scrollers pin to the real DOM bottom via `ResizeObserver` and direct `scrollTop` writes, with release on user intent and re-stick near the bottom. **`followOnAppend` is turned off** on those virtualizers so tail scrolling has a single owner (avoids fighting TanStack’s reconcile loop when the composer resizes the viewport without a scroll event). > > **Agent feed** and **feed items view** wire the hook, move vertical spacing into virtualizer **`paddingStart`/`paddingEnd`** (instead of wrapper padding that skewed end detection), and drop hand-rolled `scrollToEnd` / input listeners—opening still only toggles when the tail row is loaded. > > Also: **`@tanstack/react-virtual` 3.14.2 → 3.14.5**; router **scroll restoration disabled** on `/streams` paths; **token usage strip** moved under the composer with horizontal padding; paperclip **mb-1** alignment; **streams-example-app** fixes **`directDomUpdates`** (container ref, unconditional `measureElement`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 88f7cde. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- loc-report --> | Group | Lines | Significant | | --- | ---: | ---: | | Product | +127 -10 | +58 -10 🟩🟩⬜⬜⬜ | | UI components | +83 -62 | +42 -44 🟩🟥⬜⬜⬜ | | Config | +2 -2 | +2 -2 🟩⬜⬜⬜⬜ | | Docs | +0 -0 | +0 -0 ⬜⬜⬜⬜⬜ | | Generated | +141 -75 | +133 -75 🟩🟩🟩🟥🟥 | | **Total** | **+353 -149** | **+235 -131** 🟩🟩🟩🟥🟥 | <sub>Lines counts every changed line; Significant ignores blank lines and JS comments. Between 537d6bc and 88f7cde, bucketed first-match-wins into the groups defined in `scripts/ci/loc-report.ts`.</sub> <!-- /loc-report --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
With
anchorTo: 'end'and a dynamically growing last item (e.g. a streaming chat reply measured viameasureElement), the viewport drifts away from the end starting on the second resize tick.resizeItem'swasAtEndbranch runs in this order:itemSizeCache→getTotalSize()is now largerapplyScrollAdjustment(newTotal − prevTotal)→_scrollToOffset(getScrollOffset(), {adjustments: delta})→el.scrollTo({top: scrollOffset + delta})notify(false)→ consumer re-renders → sets the sizerstyle.heightto the newgetTotalSize()Step 2 writes
scrollTopwhile the container is still at the old total height (the consumer hasn't re-rendered yet), so the browser clamps the write back to the old max. BecausescrollTopdidn't move, no scroll event fires,this.scrollOffsetstays stale, and the next tick'sgetVirtualDistanceFromEnd()readsnewTotal − viewport − staleOffset > scrollEndThreshold→wasAtEnd = false→ no further adjustments.Fix: in
applyScrollAdjustment, after_scrollToOffsethas recorded_intendedScrollOffset, also bake the adjustment intothis.scrollOffsetand zeroscrollAdjustments(so their sum stays invariant for callers like theitemStart < getScrollOffset() + scrollAdjustmentspredicate). The DOM write may still be clamped, butscrollOffsetnow carries the intended position so the nextwasAtEndcheck stays true and the adjustment lands once the consumer has grown the sizer. This is the same idea as #1176 (eagerscrollOffsetadjustment for prepend insetOptions), applied to the resize-adjust path.✅ Checklist
pnpm run test:pr.🚀 Release Impact