#20347: fix(webchat): resolve streaming scroll race condition

## Summary Replaces the JS-based scroll tracking in WebChat (`app-scroll.ts`) with an **IntersectionObserver + CSS `overflow-anchor`** approach. This eliminates the race condition where programmatic `scrollTo` during streaming fires browser `scroll` events that fight against user scrolling. **Fixes:** #18800, #14959, #12638 ## Problem The current scroll code grew organically through 6 contributors over 6 weeks without intentional architecture: | Phase | Date | Author | Change | |-------|------|--------|--------| | 1 | Jan 1-6 | Peter Steinberger | MVP: `scrollTop = scrollHeight` | | 2 | Jan 12 | Shadow (external) | Introduced `chatUserNearBottom`, `handleChatScroll`, retry `setTimeout` — **origin of race condition** | | 3 | Jan 14 | Peter Steinberger | Refactored into `app-scroll.ts` (no logic change) | | 4 | Feb 2 | Marco Marandiz (external, PR #7226) | Threshold 200→450px, `effectiveForce`, `chatNewMessagesBelow` | | 4.5 | Feb 2 | Shakker (maintainer) | Added `scrollToBottom()` + "New messages ↓" button UI | | 5 | Feb 7 | Tyler Yust | Added `smooth` param, `chatManualRefreshInFlight`, `prefers-reduced-motion` | The race condition works as follows: 1. During streaming, every token triggers `scheduleChatScroll` → `rAF` → `scrollTo(bottom)` 2. The programmatic `scrollTo` fires the browser's `scroll` event → `handleChatScroll` recomputes `chatUserNearBottom = true` (because the *programmatic* scroll just put the user near bottom) 3. User's trackpad scroll can't escape the 450px threshold before the next token's `scrollTo` pulls them back 4. The retry `setTimeout` (120ms) adds a **second** forced scroll per token 5. When user fights and streaming ends, pending timeouts fire with stale `scrollHeight` → scroll jumps to wrong position ## Approach: What ChatGPT and Claude.ai Actually Do I reverse-engineered both ChatGPT and Claude.ai's scroll implementations: - **ChatGPT**: Searched all 267 JS bundles for scroll-management keywords (`scrollTo`, `scrollTop`, `scrollHeight`, `scrollIntoView`, `nearBottom`, `autoScroll`). **Zero matches** for any ongoing scroll logic. Uses CSS `overflow-anchor: auto` with a single initial `scrollIntoView` call. - **Claude.ai**: Same pattern — CSS `overflow-anchor` handles content growth automatically, with zero JS scroll management during streaming. Both services let the browser's native scroll anchoring handle everything. No thresholds, no retry timers, no race conditions. ## What This PR Does 1. **CSS `overflow-anchor`** on a sentinel element at the bottom of `.chat-thread` — browser natively keeps the viewport pinned as content grows (works in Chrome, Firefox, Edge) 2. **IntersectionObserver** on the sentinel — sets `chatUserNearBottom` based on whether the sentinel is visible. Crucially, **IO callbacks are NOT triggered by programmatic scrollTo**, which eliminates the root cause of the race condition 3. **Removes** the 450px `NEAR_BOTTOM_THRESHOLD` constant — no more threshold tuning 4. **Removes** the retry `setTimeout` — no more stale-scrollHeight jumps after streaming ends 5. **Simplifies** `handleChatScroll` — only clears the "New messages ↓" indicator (IO handles `chatUserNearBottom`) 6. Safari fallback: Safari doesn't support `overflow-anchor`, but the IO approach still works correctly — `scheduleChatScroll` performs programmatic `scrollTo` when the sentinel is visible, providing the same behavior ### Files Changed - `ui/src/styles/chat/layout.css` — `overflow-anchor` rules + sentinel styling - `ui/src/ui/views/chat.ts` — Added sentinel `<div class="chat-scroll-anchor">` after message list - `ui/src/ui/app.ts` — `chatScrollTimeout` → `chatScrollObserver` property - `ui/src/ui/app-scroll.ts` — Core rewrite: IO-based `chatUserNearBottom`, removed threshold + retry - `ui/src/ui/app-lifecycle.ts` — IO init/teardown in lifecycle hooks - `ui/src/ui/app-scroll.test.ts` — Tests rewritten for IO-based API ## Why This Over #19783 PR #19783 takes the existing JS scroll architecture and tunes it: reduces threshold to 50px and removes the retry timer. This helps, but: | | This PR | #19783 | |---|---|---| | Race condition root cause | **Eliminated** — IO ignores programmatic scrollTo | Still present — programmatic `scrollTo` still fires `scroll` event → `handleChatScroll` still recalculates | | Threshold tuning | **None needed** — binary sentinel visibility | 50px threshold (still arbitrary, can still fight trackpad) | | Lines of scroll logic | ~60 lines | ~80 lines (same structure, adjusted numbers) | | Approach validation | Matches ChatGPT + Claude.ai production patterns | Tuning of organic 6-phase architecture | | Safari support | IO fallback works identically | Same as before | The fundamental issue is architectural: as long as `scrollTo` triggers `scroll` events that recalculate `chatUserNearBottom`, any threshold-based approach will have edge cases. IntersectionObserver sidesteps this entirely. ## Verification - `pnpm build` ✅ - `pnpm check` (format + tsgo + lint) ✅ - `pnpm test` (860 files, 7457 tests) ✅ <!-- greptile_comment --> <h3>Greptile Summary</h3> Replaces the JS-based scroll tracking in WebChat with an IntersectionObserver + CSS `overflow-anchor` approach, eliminating the race condition where programmatic `scrollTo` during streaming fires browser `scroll` events that fight against user scrolling. The approach is well-motivated and matches production patterns used by ChatGPT and Claude.ai. - Adds a sentinel `<div class="chat-scroll-anchor">` at the bottom of `.chat-thread` and uses CSS `overflow-anchor` to pin the viewport during streaming - Uses `IntersectionObserver` on the sentinel to track `chatUserNearBottom`, replacing the 450px threshold and retry timer - Simplifies `handleChatScroll` to only clear the "New messages" indicator (IO handles the scroll-position tracking) - **Bug: observer not re-initialized after tab switch** — `initChatScrollObserver` runs only in `handleFirstUpdated` (once). Switching from chat to another tab destroys the `.chat-thread` DOM; switching back re-creates it, but the observer still watches the old detached sentinel. This breaks scroll-up detection after a tab round-trip. See inline comment for a fix. <h3>Confidence Score: 3/5</h3> - This PR has a solid architectural approach but contains a bug where the IntersectionObserver is not re-initialized after tab switches, which would break scroll-up detection for users who navigate away from and back to the chat tab. - The core IO + overflow-anchor approach is well-designed and the code is clean. However, the observer lifecycle has a gap: it's only initialized once in firstUpdated, but the sentinel DOM is destroyed/recreated on tab switches. This is a functional regression from the old code where the scroll event handler was re-bound automatically via Lit's template re-rendering. The fix is straightforward (re-init in handleUpdated on tab change), but it needs to be addressed before merge. - `ui/src/ui/app-lifecycle.ts` needs re-initialization of the IntersectionObserver when switching back to the chat tab _{Last reviewed commit: 4edbaa1} <!-- greptile_other_comments_section --> _{(2/5) Greptile learns from your feedback when you react with thumbs up/down!} <!-- /greptile_comment -->