mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
lefarcenandGitHub
fd85994589
* fix(web): stop a compositor scroll clamp from silently killing auto-followThe chat log decides "did the user scroll away" from one criterion: the
position got smaller AND `scrollHeight` did not change. The file's own
comment justifies that with "content-driven displacement always comes with
a `scrollHeight` change" — true for content-driven displacement, false for
a compositor clamp.Chromium keeps its own copy of how far a scroller travels so a wheel does
not have to wait on the main thread. On the chat log that copy can go stale
while layout moves on. From a real user's diagnostic bundle (Electron 41 /
Chromium 146): scrollTop 245.5 layoutMax 718 scrollHeight 1307 unreachablePx 472.5Jump-to-latest writes `scrollTo({top: 1307})`, the position lands at 718.5
(the real layout bottom), and one wheel notch later the compositor throws it
back onto its own stale ceiling of 245.5. Content is static, so
`scrollHeight` never moves — both halves of the criterion hold, and every
clamp reads as one more user scroll-up. Auto-follow goes off without a word.The discriminator is wheel direction. A user scrolling up either carries an
upward wheel — which ChatPane's own `onWheel` already releases follow on,
ahead of the scroll event — or carries no wheel at all (scrollbar drag,
keyboard, touch). So "the only wheel in this window pointed down" is never
true of a user scroll-up; it says the user asked to move TOWARD the bottom.
A position that moves away from the bottom under that input is not their
hand. The same shape was measured independently for the freeze detector:
`scrollTop = 800`, one downward notch, thrown back to 91.The other candidates do not hold up. Landing on a known stale ceiling needs
the ceiling, which only the freeze detector's state machine can infer — and
`chat-scroll-takeover.ts` records that detector's false-positive rate as
unknown. The gap to `layoutMax` carries no information, since a user can
stop anywhere. Adjacency to a JS write is falsified by the bundle itself:
the clamp landed 3.8s after the write with zero JS write records in between,
and a window that wide swallows ordinary scrolls.This does not fix the stale ceiling — that is the takeover module's job and
it stays switched off pending a product call. It only stops the clamp from
also costing the user auto-follow.The criterion is deliberately biased to under-fire: a clamp with no wheel
witness is still treated as a user scroll-up. Welding follow on would be
worse than the bug.
* fix(web): give the wheel witness a lifetime
nettee's review on #7898 found the over-suppression this criterion was
supposed to avoid — a stale witness welding auto-follow ON, which is the
failure mode that is worse than the bug being fixed. The path is complete:
1. the user is already at the bottom and wheels down once — the position
does not move, so no `scroll` event fires and the witness is never
consumed;
2. switching conversations from the history control happens OUTSIDE the
log element, so no pointerdown/keydown reaches it, and the effect that
resets baseline and follow intent did not touch the witness;
3. a later NON-wheel move to earlier content (find-in-page, a focus-driven
scroll) is then compared against that stale downward witness, read as a
clamp, and follow never releases.
The witness now has three independent bounds, each covering a hole the other
two cannot:
· Consumed on use. Any scroll event spends it, including one that moves
nothing — one witness explains one displacement, never the next.
· Scoped to the scroll context. `WheelWitness.atScrollTop` records where
the scroller was; the displacement's origin must be exactly that, so a
baseline moved by anything else voids the note structurally rather than
on a timer. Conversation switch, log node swap and panel unmount clear it
alongside baseline and follow intent.
· Expires after a frame. The only bound that can kill a witness no scroll
event will ever consume. A frame is not a tuned constant: the clamp is
dispatched in the same rendering update as the notch that caused it, and
per the HTML update-the-rendering steps scroll events run BEFORE that
frame's animation callbacks. (Not to be confused with the 3.8s in the
diagnostic bundle — that is the gap between the JS write and the clamp,
not between the wheel and the clamp.)
Recording is skipped entirely when `requestAnimationFrame` is unavailable:
without the expiry bound a witness could outlive its displacement, and no
witness just means today's behavior.
Mutation-checked line by line. Three points are honest defence in depth and
say so in-place: the conversation-switch and unmount clears cannot be turned
red today (a conversation switch always schedules a frame, and `setTab` has
no call site), and the upward-wheel counter is pinned at the predicate level
only, because `onWheel` already releases follow on an upward notch.