fix(chat): keep a still reader at the bottom on phones without anchoring - #9391
Conversation
Entering an idle session on an iPhone opened the transcript a viewport or more above its end, and scrolling back to the bottom by hand did not re-engage follow. Both trace to decisions that were only correct because Chromium's native scroll anchoring quietly covered for them; WebKit has no scroll anchoring at all. After the entry pin the rows keep settling: a code-block stand-in swaps for the highlighted block, the top spacer reprices as rows above the window measure, the earlier-messages bar mounts. Content above the fold shrinks (the engine clamps scrollTop, the reader stays at the bottom) and then grows back with no scroll event. The automatic pin's idle rule read that gap as the reader having scrolled up and RELEASED follow, so nothing brought them back. Reproduced in Chromium with overflow-anchor disabled: 1007-1119px above the end on two of three entries; 0 with this change. The discriminator is input plus position, not distance: a reader who left the bottom has touched the scroller since we last placed them, or is no longer resting where our last write put them. When neither holds, the gap is content settling under a still reader and they are carried back, live turn or not. Either signal alone would be wrong: position alone reads our own write as consent for a reader who wheeled up and stopped on it, input alone drags back a programmatic reveal (a search hit, a pinned prompt) whose scroll event has not dispatched yet. Chrome above the rows -- the earlier-messages bar -- is observed by nothing, so its mount now re-evaluates the bottom pin through the same predicate (46px short at every drawer entry before). The viewport-coverage watchdog fired forcePin every 500ms for any reader parked at the bottom, because the tail spacer and sentinel below the last row read as uncovered list. That same-value write re-armed follow the idle rule had released and, on WebKit, cut every momentum tail short. A side the window has reached the list's end on is covered by construction. Removing the misfire exposed that releasing follow left the self-scroll reference pointing at the old bottom, so a hand scroll landing on that exact pixel read as our own write and follow never re-armed; the reference is now dropped with the release.
UX Review (Fable 5) — ✅ PASSUX-level review of This diff is purely behavioral scroll-follow logic in UX-Verdict: PASS Pure scroll-behavior fix with no visible surface: phone users now land on the latest message, and readers who scrolled away are never dragged back. [UX-REVIEWED] b481fdc |
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Sound two-signal design (input + position) that fixes the real cause; but the WebKit fix was never run on WebKit, only Chromium emulating one WebKit trait. Watch
Suggestions
[DESIGN-REVIEWED] b481fdc |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of First-Principles-Verdict: PASS Whole fix was validated with What this change shipsInventory (7 items)Intent: on a phone, an opened session should land at the latest message and stay there while content settles — a FIX (reproduction with measurements in the description; new tests encode it).
The rewritten pin's own words rested on the two cases being "indistinguishable" from distance alone; this PR introduces the input-plus-position signal that distinguishes them and keeps the release case as a sibling test with input, so the prior decision is answered, not erased. The defect's provenance is a written reproduction plus tests that fail on base; no item is undeclared, duplicated, or symptom-level (the WebKit no-anchoring root is unreachable, and the change compensates at the decision point that misread it). [FIRST-PRINCIPLES-REVIEWED] b481fdc |
GPT 5.6 Review — 🔴 changes requested (blocking)GPT 5.6 found at least one blocking issue that must be resolved before merging This comment is updated in place on each push. BLOCKING -- website/electron/ipc-registrar.js:327 -- Validated pathname can be retargeted to a sensitive file Adjudication (Opus 4.8) — is blocking on each finding proportionate?F1 is a FENCED finding — my only verdicts are UPHOLD-FENCED or FLAG, and FLAG requires the same complete evidence record (every condition at Examining Conditions for the FLAG record: (a) agent writes a symlink to a known credential path — within ordinary agent capability in a workspace, not an input the system's writer cannot produce; (b) user invokes open on the agent-authored path and confirms — plausible social-engineering flow; (c) [ADJUDICATION] b481fdc total=0 uphold=0 downgrade=0 [ADJUDICATION-FENCED] b481fdc fenced=1 flagged=0 False positive or not applicable? A repository writer can comment: |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
|
|
|
Post-merge note: the red checks on |
Problem / Motivation
On a phone, opening a session lands the transcript mid-conversation instead of at the latest message, and scrolling to the bottom by hand leaves the view unsettled. Reported from an iPhone against builds that already carry the recent scroll work (0.6.0-insider.6 includes #7916, #8574, #8001).
Reproduced in Chromium by disabling CSS scroll anchoring (
overflow-anchor: none), which is how iOS Safari behaves — it has no scroll anchoring at all. Isolated gateway + fake ACP backend, 390×844 touch viewport, a 40-turn session,/chat?sid=entry:A 60-turn session (one older page unloaded, so the earlier-messages bar mounts) landed 46 px short on every drawer entry on main; 0 with this PR. With native anchoring left on, every scenario lands at 0 on both — which is why none of this was visible on a desktop browser.
Why it matters
Every phone user opening an existing session reads the wrong part of the conversation and has to find the end by hand; a reader who then scrolls to the end does not get followed when the next turn streams. Long sessions with code blocks are the worst case, because the post-entry reprice that opens the gap scales with how much of the transcript above the viewport is still settling.
What changed (motivation → approach → change)
Symptom → cause. Per-frame tracing with
scrollTo/scrollTophooked and the scroller's children measured each frame:forcepin 0→18345);On Chromium, native scroll anchoring moves scrollTop through step 4 and the reader never leaves the bottom. On WebKit the whole reprice is paid as displacement.
Approach. The idle rule (
evaluateAutoPin,runActive) cannot tell from distance who opened the gap, and it errs toward release. The signal that can tell is input plus position: a reader who left the bottom has touched the scroller (wheel / touch / pointer / scrolling key) since we last placed them, or is no longer resting where our last write put them. When neither holds, the gap is content settling under a still reader and they are carried back, live turn or not. Both halves are load-bearing — position alone reads our own write as consent for a reader who wheeled up and stopped on it; input alone would drag back a programmatic reveal (search hit, pinned-prompt jump, find-in-page) whose scroll event has not dispatched yet, since none of those touch the scroller's input listeners.Changes (
FollowController.ts,useVirtualChat.ts):evaluateAutoPintakesreaderMovedSinceWrite(defaulttrue= legacy). With itfalseand scrollTop within epsilon oflastWriteTop, a gap is closed regardless ofrunActive; every other branch is unchanged. This deliberately reverses a pin made in perf(chat): reachable archived history and stable phone scrolling #7916 (436fe5115), whose idle branch declined an exception for "the reader is resting on our own last write" on the grounds that a content-opened gap was indistinguishable from a reader-opened one. It is distinguishable once input is on record: the absence of any hardware input since our last placement is the evidence that decision said was missing. The released-with-input case that motivated the original pin stays covered by its own tests (idlePinGuard,prePaintPinComposer,FollowController).lastPinAtat each bottom positioning (pin write, layout-clamp re-baseline, at-bottom observation) and feedshardInputAt > lastPinAtto both call sites (post-paint RO pin, pre-paint height-sync pin).forcePinevery 500 ms for any reader parked at the bottom, because the tail spacer and bottom sentinel below the last row read as uncovered list (33 px > 8 px slack). That same-value write re-armed follow the idle rule had released (measured:WRITE forcepin X->Xevery 500 ms in every inspector trace on main). On WebKit a programmaticscrollToalso ends an in-flight momentum or rubber-band animation, so this is the likely mechanism behind the bottom jitter — stated as inference: the emulation cannot reproduce WebKit scroll physics and this host cannot run Playwright's WebKit (Amazon Linux 2023, missing libgtk-4 and friends), see Manual verification. A side the window has reached the list's end on is now covered by construction.lastWriteToppointing at the old bottom, so a hand scroll landing on that exact pixel (it is still the maximum scrollTop) read as our own scroll and the re-engagement branch was skipped — follow never re-armed after a manual return. The self-scroll reference is now dropped with the release.Tests
Each mutation-verified (the mutation named is caught by the named test and by nothing weaker):
FollowController.test.ts—readerMovedSinceWrite: idle + still → carried; idle + moved → released (unchanged); no input but scrollTop left our write → not ours (reveal race); resting on a clamp-rebaselined write; never overrides a released stick or an owning restore; omitted = legacy. (drop the branch / drop either condition → red)useVirtualChat.idleCarryBack.test.tsx— clamp then regrow with no input → carried back; wheel-at-bottom then clamp → still carried (clamp re-establishes the bottom; drop the clamp stamp → red); programmatic reveal with its scroll event pending → left alone; input in the outgoing session does not disown the incoming entry pin; scrolling back onto the exact pixel of our last pin re-arms follow (dropreleaseFollowBaseline→ red).useVirtualChat.leadingChrome.test.tsx— a 46 px bar mounting above the rows carries a still idle reader to the new bottom; a reader who wheeled is left alone (disable the effect → red).useVirtualChat.coverageWatchdog.test.tsx— a reader at the bottom with page chrome below the rows gets no write per tick (drop theend < countguard → red).useVirtualChat.prePaintPinComposer.test.tsx,useVirtualChat.idlePinGuard.test.tsx.Gates:
tsc -b,eslint src, jscpd clean; all 100 virtualizer/chat test files (1082 tests) pass locally.Manual verification
Real-browser rig (Playwright, iPhone profile, real touch sequences via CDP, isolated gateway with the fake ACP backend), both with
overflow-anchor: none(iOS-like) and with native anchoring:/chat?sid=entry, drawer switch short→long, drawer re-entry after a scroll, full reload, entry mid-stream: 0 px from the end in every phase, both modes (main: 46–1119 px in the iOS-like mode).stick=true) — on main this only held because the watchdog force-pinned every 500 ms.forcepinwrites only; the per-500 msWRITE forcepin X->Xstream is gone.Not verified on a physical iPhone, and not under Playwright's WebKit engine either: it does not run on this host (Amazon Linux 2023 —
playwright install webkitreports libgtk-4, libvulkan, libicu 74, libflite… missing, and installing system libraries is out of scope). The anchoring-off emulation is the same discriminator #7916 used to find the drift class and covers the entry/landing claims exactly (they are pure scroll geometry, no physics). The momentum/rubber-band claim about the watchdog is inference from the mechanism, not a measurement — the same rig scenarios under real WebKit or a physical iPhone would be the confirming step.Related Issues
Follow-up to #7916 (which fixed the drift while reading history on a phone); this covers entering and returning to the bottom.
Pattern harvest
Rule candidate: review-prompt
Pattern: a scroll-position decision that reads "distance from bottom" as evidence of a user move is only correct where native scroll anchoring absorbs content growth; on WebKit it must be paired with an input signal.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)