12 Commits
Author SHA1 Message Date
Kevin Meinon 906aebefa9 fix(scroll): do not unpin a pinned tail reader on an above-tail collapse clamp (#7605) 2026-09-16 07:03:36 -07:00
3a95d82531 fix(chat): prevent first response jump from snapping back (#6621)
* fix(chat): keep response jump position after session load

Cancel pending load-time bottom settling before an explicit response jump takes scroll ownership. Add a regression test covering the first-click race.

* fix: gate response jump ownership by destination

* fix: keep response jumps programmatic through smooth scroll

Native smooth-scroll frames from response jumps could reach the manual scroll listener and claim reader ownership before the final destination was known.

- own response-jump scrolling with a generation- and session-scoped lifecycle
- reconcile sticky ownership from final 80px tail geometry
- preserve current low-delta wheel takeover semantics when integrating with master
- cover visible assistant, user-row, and virtualized paths through the production scroll listener

* fix(scroll): hold reader off-bottom during jump owner + cancel jump on explicit End (#6621 gate fixes)

Codex gate found two defects in the jump-scroll ownership mechanism:
- Jump during an active stream snapped back to bottom on the next token: the
  owner suppressed the scroll listener but left the pre-jump pinned state, so
  scrollIfPinned() reclaimed the bottom. Now _beginMessageJumpScroll unpins for
  the ownership window and scrollIfPinned() no-ops while an owner exists.
- An explicit End click could be undone by the pending jump reconcile restoring
  the stale unpinned snapshot. scrollToBottom() now cancels the active jump
  owner first; _cancelMessageJumpScroll restores the preserved snapshot so a
  non-reconcile cancel doesn't leak the transient unpinned state.

* fix(scroll): drop undefined currentSid global ref in _messageJumpSessionId (#6621 brick-class scope-undef gate)

The PR's _messageJumpSessionId() referenced a bare 'currentSid' global that
does not exist in ui.js (everywhere else it's a local const from
S.session.session_id). test_static_js_scope_undef flagged it brick-class
(#3696). The S.session.session_id fallback already IS the canonical accessor;
removed the dead first line and updated the harness to drive the session-change
case via S.session.session_id.

* fix(scroll): round-2 gate fixes for jump-owner state transitions (#6621)

Re-gate found three more state-transition edge cases from the temp-unpin window:
- wheel-up interrupting an active jump owner after the programmatic latch
  expires now explicitly establishes the unpinned reader-owned state (was
  cancelling the jump but leaving pinned -> next token snapped to bottom).
- _resetStreamScrollFollow() now cancels the jump owner FIRST, before its
  pinned-state assignments, so a stream starting mid-jump can't have its pin
  undone by the snapshot restore -> auto-follow stays enabled.
- _finishMessageJumpScroll() flushes a deferred external-session refresh after
  reconciliation when the terminal state is pinned to the tail, so a refresh
  deferred during the temp-unpin window isn't stranded.

* fix(scroll): make jump-owner guards typeof-safe + widen static test window (#6621)

The two _messageJumpScrollOwner guards (scrollIfPinned, scrollToBottom) are
pulled into other scroll test harnesses (test_issue6414, test_issue4856) that
stub the scroll env without declaring the new global; bare references threw
ReferenceError. Guard with typeof (matches the PR's own jumpScrollOwned check).
Also widen test_low_delta_wheel_intent_is_tracked_separately's source-slice
window 1400->1800 to still contain the (unchanged) _lastMessageWheelIntentMs
line after the +9-line wheel-up-during-jump block was inserted.

* test(scroll): add streaming-frame-hold + wheel-during-jump regression tests (#6621 Fable finding ii)

Fable UX gate flagged that the PR's tests model only the stale load-time settle
callback, not the streaming case. Add two node-harness tests:
- a streaming render frame (scrollIfPinned) fired inside the jump-owner window
  must not snap the reader to the bottom (holds at target, 0 bottom-writes).
- a gentle wheel-up during the owner window after the programmatic latch stales
  hands ownership to the reader UNPINNED at their position, never pinned
  mid-transcript.

* fix(scroll): widen jump-owner takeover to downward + touch input (#6621 Fable S3)

Fable's re-review (on the fixed code) confirmed the streaming blocker + wheel-up
concern resolved, and probe-proved one remaining narrow regression: a gentle
wheel-DOWN or touch scroll during the owner window cancelled the owner and
restored the pinned snapshot but did NOT re-unpin (the takeover was gated on
wheelUp only), so the next streaming token yanked the reader to the bottom.
Widen the takeover to any message-pane scroll input during the owner window.
Add a wheel-down regression test; widen the #4970 static-slice window to 2000
to still contain the (unchanged) _lastMessageWheelIntentMs line.

---------

Co-authored-by: pxxD1998 <214340659+pxxD1998@users.noreply.github.com>
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
2026-08-13 21:17:42 -07:00
nesquena-hermesandallenliang2022 9fd349898f fix(chat): defer mobile overflow-anchor suppression (re-arm reachable + independent hard cap) #4856
Clean rebase of allenliang2022's #5392 (rebase-first).

Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com>
2026-07-02 04:01:38 +00:00
allenliang2022 7db6fd8650 fix(chat): keep overflow-anchor suppressed across the async post-render settle window (mobile jump-back)
The sync-frame guards (_fixMobileScrollJank / _suppressBrowserOverflowAnchor)
only cover the render frame itself. postProcessRenderedMessages() — syntax
highlight, inline diff/csv/pdf/html/excalidraw, katex/mermaid — is scheduled a
FRAME LATER via requestAnimationFrame(), after those guards have released. Each
of those can change the height of rows ABOVE the viewport; on mobile
(overflow-anchor:auto) the browser's native anchor engine then compensates
scrollTop a SECOND time in that unguarded frame, yanking an unpinned reader to
another turn (the residual mobile 往回大跳).

Wrap all three deferred post-process dispatches (fast-path cache branch, main
render tail, live-tool remount) in _postProcessWithAnchorSuppression(), which
routes through the shared _suppressBrowserOverflowAnchor() and holds suppression
one extra frame so late media/layout reflow is covered too. Desktop rests at
overflow-anchor:none so the wrapper is a verified no-op there.

Reproduced on an isolated debug instance with a cloned 1179-message session:
above-viewport +350px during the async settle window jumped scrollTop +350 on
mobile (auto) and 0 with the wrapper; desktop (none) 0 both ways. static/ui.js
only.
2026-07-01 17:46:03 +00:00
Frank SongandPaperclip 71a458784a fix(chat): guard live worklog manual scroll intent
Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-06-30 10:52:11 +08:00
allenliang2022 e2ea0789b3 fix(chat): do not treat Space on transcript controls as scroll intent (#4970 review)
Maintainer/Codex found one narrow edge in the keyboard intent stamp: because the
listener runs capture-phase and accepts any focused descendant of #messages,
Space/Spacebar on an in-transcript control (tool-card toggles, copy buttons,
role buttons, links/tabs) stamped _lastMessageKeyScrollIntentMs before the
control handler could preventDefault/stopPropagation. That made a button
activation look like a scroll intent and could mask a legitimate post-render
artifact.

Fix by excluding Space/Spacebar when the event target or active element is an
interactive transcript control: button, a[href], select, summary, role=button,
role=tab, role=menuitem, or contenteditable. Keep PageUp/PageDown/arrows/Home/End
stamping unchanged, and keep the existing INPUT/TEXTAREA/contenteditable composer
exclusion.

Add a behavioral node-harness test: Space on a focused transcript button leaves
the key-scroll stamp at -Infinity (serialized null), while PageUp on the pane
still stamps 1234. test_4856 + test_4295 green (20 passed), node --check clean.
2026-06-26 13:29:14 +00:00
allenliang2022 6825bc03b4 fix(chat): record keyboard message-pane scroll as intent (#4970 greptile P1)
Keyboard scrolling of the message pane (PageUp/PageDown, Arrow keys, Space,
Home/End) fires a native scroll event with no wheel/touch/scrollbar/non-message
intent. Inside the 1400ms post-render artifact window the suppression branch
then returned before movedUp could unpin, so a keyboard scroll-up was swallowed
and live-follow snapped the reader back to the bottom.

- Add _lastMessageKeyScrollIntentMs + _recentMessageKeyScrollIntent(), stamped
  by a capture-phase keydown listener on the scroll keys, gated to when the
  message pane is the scroll target (focused/contains focus/hovered) and not an
  editable field (composer/input/contenteditable).
- Gate the post-render suppression on !_recentMessageKeyScrollIntent() (typeof
  guard keeps the #4295 node harness inert).
- Clear the stamp in both _resetScrollDirectionTracker() and
  _resetStreamScrollFollow() (same stale-state hygiene as the wheel stamp).

Tests: keyboard-scroll-inside-window-still-unpins behavioral regression (harness
extended with injected _recentMessageKeyScrollIntent), plus a source lock for the
helper/keydown-stamp/suppression-gate/both-resets. test_4856 + test_4295 green
(19 passed), node --check clean.
2026-06-26 13:29:14 +00:00
allenliang2022 67d9580825 fix(chat): reset wheel intent on session/stream resets + gate suppression on scrollbar drag (#4970 review)
Maintainer dual-gate found two stale-state leaks in the new
_lastMessageWheelIntentMs and one adjacent pre-existing gap; all three fixed:

MUST-FIX 1 — _resetScrollDirectionTracker() (session switch) did not clear
_lastMessageWheelIntentMs, so a gentle wheel in chat A left
_recentMessageWheelIntent() true into chat B's first post-render window,
under-suppressing the artifact and falsely unpinning. Now reset to -Infinity.

MUST-FIX 2 — _resetStreamScrollFollow() (fresh stream) had the same leak: a
gentle upward wheel within 1200ms of a new stream could silently disable live
follow. Now reset to -Infinity.

SHOULD-FIX 3 — the suppression branch ignored _scrollbarDragActive, so a manual
scrollbar-drag upward scroll inside the 1400ms window was swallowed. Gate the
branch on (typeof _scrollbarDragActive==='undefined' || !_scrollbarDragActive);
typeof guard keeps the #4295 node harness inert.

Tests: scrollbar-drag-inside-window-still-unpins behavioral regression (harness
extended with injected _scrollbarDragActive), plus source locks for both resets
and the scrollbar-drag gate. test_4856 + test_4295 green (17 passed),
node --check clean.
2026-06-26 13:29:14 +00:00
allenliang2022 2ce0e731ec fix(chat): scope post-render suppression to true no-intent artifacts (#4970 review)
Maintainer dual-gate (Codex) found the suppression could swallow a genuine
low-delta trackpad wheel scroll-up for ~1.4s after a render:
_recordNonMessageScrollIntent() only recorded message-pane wheel intent at
deltaY<-30, so a gentle deltaY:-5 left both intent helpers false and the
post-render branch returned before movedUp set _messageUserUnpinned.

- Track recent low-delta upward message-pane wheel intent separately
  (_lastMessageWheelIntentMs / _recentMessageWheelIntent), recorded for any
  upward wheel (deltaY<0). The decisive deltaY<-30 sticky-unpin is unchanged.
- Require !_recentMessageWheelIntent() before suppressing the artifact, so a
  real gentle scroll-up inside the window still unpins.
- Add behavioral node-harness regressions: gentle wheel inside the window
  unpins; no-intent artifact inside the window stays suppressed; outside the
  window unpins. Plus a source lock that low-delta intent is tracked.
2026-06-26 13:29:14 +00:00
allenliang2022 f9a952f085 fix(chat): suppress post-render scroll artifacts 2026-06-26 13:29:14 +00:00
allenliang2022 ef6901c947 fix(chat): ignore DOM-wipe scroll clamp during render 2026-06-25 20:19:00 +00:00
Rod Boev 0668cfa77c fix(#4856): suppress Chromium scroll-anchor re-selection during mobile DOM rebuild 2026-06-25 01:06:09 +00:00