Fable senior review (COMMENT, no blockers) + DRY nit:
- Widen the ResizeObserver from #msg to the whole #composerWrap so growth from the
attachment tray / selection chips (not just textarea rows) also re-pins a bottom-
pinned reader. Fable flagged tray/chip growth as a likely REMAINING #5515 trigger
— same viewport-shrink physics, now covered by the same guarded re-pin at one seam.
#composerWrap contains #msg, so the textarea path is still covered.
- DRY: _repinMessagesAfterComposerResize() now reuses _messageBottomDistance()
instead of re-implementing the bottom-distance expression inline.
Re-verified live (composer grow -> bottomDist stays 0) + 524 scroll/composer
regression tests green + 6 issue tests green.
#5514 (deterministic): with the chat pinned to the bottom, growing the composer
past one row (multi-line typing, Shift+Enter, a multi-line / WisprFlow paste)
shrinks the flex:1 .messages viewport by the same delta. scrollTop is unchanged
but max-scrollTop rose by delta, so the reader is stranded delta-px above the
bottom — the transcript 'scrolls up one row per composer row.' autoResize()
resized the textarea and called updateSendBtn() but never re-pinned the transcript.
Reproduced live: composer 176px -> viewport shrank 745->613 -> transcript stranded
132px above bottom while _scrollPinned stayed true.
#5515 (intermittent 'random scroll up'): the same viewport-shrink from composer
growth via paths that don't route through input->autoResize (paste, draft restore,
programmatic value set, reflow) reads to the user as a random upward jump. Covered
by a ResizeObserver seam (below).
Fix: new _repinMessagesAfterComposerResize() (static/ui.js) re-pins to the bottom
ONLY when the reader is genuinely still pinned (honors _messageUserUnpinned /
_scrollPinned — never yanks a reader who scrolled away; verified live). Called from:
(a) autoResize() after the height write, and
(b) a ResizeObserver on the #msg textarea that re-pins on GROW only, catching
every composer height-change path (not just the typed-input seam).
Verified live on a 40-message seeded session: pinned reader stays at bottomDist=0
after a 4-line composer grow; an unpinned (scrolled-up) reader is NOT moved.
Tests: tests/test_issue5514_composer_grow_scroll_pin.py (3 static wiring + 3 node-vm
behavioral). 524 scroll/composer regression tests across 39 files still green.
NOTE on #5515: this resolves the composer-growth cause (which the reporter's
WisprFlow-paste symptom points at). If a purely non-composer intermittent jump
persists, the remaining suspect is touch-device overflow-anchor:auto on content-
above height change — that path has no repro yet and is tracked as needinfo on
#5515.
Codex gate findings (2 SILENT, both fixed):
- ui.js: incremental prose node now flows through the shared row-decoration
block (data-anchor-scene-row/-row-id/-row-role/-source-event-type) instead of
returning early, so live incremental rows keep the identity attrs the scene
reconciler matches on.
- messages.js: _cancelThrottledSnapshotTimer() + _clearAnchorProseIncrementalNode()
now run on apperror, cancel, _handleStreamError, and _restoreSettledSession
terminal paths (previously only fallback/done/stream_end) — closes the
snapshot-timer/anchor-cache/window-global teardown gap.
- test: extend test_messages_js_stream_perf_cleanup_lifecycle to pin teardown on
all four additional terminal paths.
CORE equivalence (Codex flagged smd != renderMd): reconciled — Opus's 1498-frame
harness against real smd.min.js proves incremental-smd == whole-smd byte-identical;
live-smd-vs-renderMd difference is the already-shipped fidelity model (main live
body streams smd, settles renderMd), and the incremental path is gated on !settled
so settled DOM still comes from renderMd. Not a regression.
In compact-worklog mode the anchor activity scene re-renders every frame, and
its live prose row was rebuilt with renderMd(fullText) each time — re-parsing
the entire growing answer per frame, which is O(n^2) over a long streamed
response (the incremental smd parser that the main body already uses was
bypassed and its segment hidden).
Keep a persistent per-segment smd parser + DOM node (keyed by the prose row's
local_id, using the same safe renderer as the live body) and feed it only the
delta each frame; hand that persistent node back to the scene builder instead
of re-parsing. Turns the per-frame prose cost from O(n) into O(delta), i.e. the
whole answer from O(n^2) to O(n).
Safe by construction: settled rows and any failure (no smd, no stable key,
parser error, text divergence) fall through to the existing renderMd path, so
worst case is identical to today. Cache is bounded (<=32 segments) and self-heals
on non-append text changes, mirroring _smdWrite.
Verified: node --check + ESLint runtime guard clean. Streaming render profiled
in-browser via a per-frame timing probe.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex re-gate found 2 SILENT regressions the certifier missed:
- inline lost the 'Fit to screen' toolbar button (PR gated Fit to lightbox-only)
- the new readable-scale/envelope path leaked into lightbox mode (changed zoom-out floor + fit init for wide diagrams)
Fix: toolbar restored to master shape (Fit for all modes, Fullscreen inline-only);
_minScale() returns the flat _MERMAID_VIEWER_MIN_SCALE for lightbox; lightbox init
restored to master's _fitScale()/box*initialFit; the new readable-height sizing is
now scoped strictly to inline mode. Tests updated to the master-consistent contract.
Rebase PR #5162 (rename busy_input_mode -> default_message_mode; flip the
default from 'queue' to 'steer') onto current origin/master WITHOUT dropping
the shipped #5170 localStorage persistence mirror.
Rename the mirror machinery to the new setting name for consistency:
_BUSY_INPUT_MODES -> _DEFAULT_MESSAGE_MODES (values unchanged)
_normalizeBusyInputMode -> _normalizeDefaultMessageMode (fallback now 'steer')
_persistBusyInputMode -> _persistDefaultMessageMode
_readPersistedBusyInputMode -> _readPersistedDefaultMessageMode
window._busyInputMode -> window._defaultMessageMode (+ renamed exports)
localStorage: write the new 'hermes-default-message-mode' key; read it with a
fallback to the legacy 'hermes-busy-input-mode' key so an existing user's
persisted preference survives the rename.
Preserve #5170 behavior at every mirror site under the new names:
- boot success -> window._defaultMessageMode=_persistDefaultMessageMode(...)
- boot FAILURE -> window._defaultMessageMode=_readPersistedDefaultMessageMode()
(NOT a hardcoded 'steer' — a saved 'interrupt'/'queue' must still apply when
the server is unreachable; do not regress #5167/#5132)
- preferences autosave, settings-panel load, and _applySavedSettingsUi all
persist through _persistDefaultMessageMode(...)
Tests updated for the rename while keeping the persistence-behavior assertions
(test_1062, test_5145, test_5167); test_5167 gains explicit guards that the
load-failure path reads the persisted pref and never hardcodes a literal mode,
plus autosave/panel-load mirror-write coverage.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
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.
Root cause (mobile-only, never reproduces on desktop): .messages CSS resting
overflow-anchor is 'auto' on touch devices but 'none' on hover+fine-pointer
desktops (style.css media query). When _restoreMessageViewportAnchor writes
scrollTop to realign the reader's anchor row AND content height above the
viewport changed in the same frame, a mobile browser's native scroll-anchoring
ALSO shifts scrollTop -- the two compensations stack and yank the reader to an
unrelated earlier turn. Desktop never has the browser layer, which is why this
reproduced only on phones.
Fix: _suppressBrowserOverflowAnchor() sets overflow-anchor:none for the JS
scrollTop write, releases (restores prior value) next frame. Engages ONLY when
computed value is 'auto' (mobile) -- pure no-op on desktop (already none).
Verified on isolated debug instance (mobile-viewport Playwright):
- mobile auto: 800px above-viewport growth compensation 800px -> 0 (browser layer suppressed)
- desktop none: helper returns null, inline value untouched (byte-identical behavior)
- streaming: real turn, mid-read follow, 0 jumps, content held
- scroll-regression suite green
/api/clarify/pending always returns HTTP 200 when present (returns
{"pending": null} for an unknown session — it never 404s). The front-end
clarify poller warned "Clarify endpoint unavailable. Please restart
server." on ANY caught error whose message merely contained "404" or
"not found", so an unrelated stale-session 404 ("Session not found", e.g.
an old-profile session polling briefly after a profile switch) or a
transient error produced a misleading missing-endpoint toast that pointed
operators at the wrong layer.
Clarify polling now branches on the structured HTTP status that api()
attaches to the thrown Error (err.status):
- 404 "Session not found" -> handled as a stale-session poll (stop + hide
card silently), no toast;
- restart-server warning fires only on a genuine route-not-found 404 whose
body is NOT session-scoped;
- poll failures are logged with path, status, polling session id, and
current session id for diagnosis.
Interrupt provenance: cancelStream()/cancelSessionStream() now log a
'[stream] cancel requested' line with the trigger reason
(composer-stop / slash-stop / slash-interrupt / busy-interrupt /
sidebar-stop). Passive UI lifecycle events (session switch, tab hide, page
unload) already tear down only the local SSE transport via
closeLiveStream() and never call /api/chat/cancel — only explicit
Stop/interrupt paths interrupt the backend agent/tool run. This is
confirmed by test_clarify_pending_never_404s locking the handler shape.
Supersedes #5343 (which handled only the profile-switch sub-case and kept
the broad message-scrape). Adds tests/test_issue5345_*.py (9 tests).
Co-authored-by: claw-io <claw-io@users.noreply.github.com>
Co-authored-by: ruizanthony <ruizanthony@users.noreply.github.com>
The RED gate-cert (Codex-verified) found the keep-open was NOT the one-frame
transient the PR claimed: _disarmKeepSettledWorklogOpen() only cleared the token
with no follow-up render, so the just-settled worklog stayed force-open and could
be cached into _sessionHtmlCache and persist across session restores.
Two coordinated fixes:
1. Don't cache the forced-open DOM. Add _isKeepSettledWorklogOpenArmed() and gate
the _sessionHtmlCache.set() population on a typeof-safe !_keepOpenArmed local, so
the armed settle render is never cached. The typeof guard also keeps standalone
renderMessages() node harnesses (which don't define the helper) working.
2. Collapse pass after disarm for BOTH pin states. The STREAM_DONE handler disarms,
then runs _renderMessagesWithScrollSnapshot() UNCONDITIONALLY (a scroll-preserving
re-render that collapses the worklog back to its copied live/user disclosure
state), and only THEN scrollToBottom() for followers to re-settle at the tail.
The second RED gate-cert caught that gating the collapse pass to the non-following
path left a pinned follower forced-open, because scrollToBottom() only settles
scroll position and does NOT re-render. Running the collapse render before the
follower re-settle fixes both pin states. The same-frame JS restore absorbs the
collapse height change instead of leaving it to native scroll-anchoring (which
mobile suppresses mid-render), so there is no jump, and this unarmed render is
the one that populates the cache.
Net: keep-open is genuinely one settle render for pinned AND unpinned readers —
neither persists nor caches — and a user-collapsed worklog is preserved. Verified
e2e on an isolated debug instance for both pin states (worklog open-while-armed ->
collapsed-after-pass; pinned follower stays pinned at tail). Regression tests lock
the cache-skip and the collapse-before-scrollToBottom ordering; the scrollToBottom
adjacency keeps the #1360 done-follow test green.
Maintainer review on #5260 flagged that dropping the pin gate means an unpinned
reader who manually collapsed the live worklog now gets a one-frame flash-open at
the settle render before it re-collapses. Document the mechanism in-tree at the
call site (disclosure state preserved + keep-open disarms after the render, so
the next re-render honors the collapsed state) so a future reader doesn't mistake
the single-frame override for a lost preference. Doc-only; no behavior change.
The STREAM_DONE keep-open exception (#4970 round 6/7) was scoped to PINNED
followers only, on the assumption that unpinned readers 'preserve their viewport
normally'. That holds on desktop (overflow-anchor:none + JS snapshot restore) but
NOT on mobile.
Repro (isolated debug instance + CDP touch emulation, faithful to a real phone):
a reader who scrolls UP to read inside the just-settled turn is UNPINNED. At
STREAM_DONE the live worklog/thinking cards collapse into a compact summary,
shrinking the transcript by hundreds of px ABOVE their viewport. On mobile the
CSS resting value is overflow-anchor:auto, but _fixMobileScrollJank() flips an
inline overflow-anchor:none over the settle render — so native scroll-anchoring
is suppressed during exactly the frame the unpinned reader needs it to absorb the
above-viewport shrink. The content then leaps to the top of the latest turn (the
'往回大跳' report). Single-variable measurement: collapse-with-auto shifts the
reader 0px; collapse-with-none shifts -248px.
Fix: keep the just-settled worklog open for BOTH pin states (still one-shot
scoped to the turn that just settled, so historical worklogs still collapse
compact). This removes the shrink entirely, fixing the jump for every
device/anchor-mode instead of fighting the anchor engine. The #4856 guard and the
mobile overflow-anchor:auto CSS are untouched, so Boev's Android DOM-wipe fix and
akrhin's #MOBILESCROLL fix both stay intact.
- Rename _shouldKeepSettledWorklogOpenForPinnedFollow ->
_shouldKeepSettledWorklogOpenForStreamSettle (drops the _scrollPinned /
_messageUserUnpinned gate; keeps the one-shot stream-id token).
- Update the #4970 behavioral regression test to assert keep-open is TRUE for the
armed turn under BOTH pin states, FALSE for historical turns and after disarm.
Two related 'jump back' classes where a reader who is following the live stream
gets stranded mid-transcript, both verified live (OLD stranded the viewport
470-580px from bottom; FIX lands at 1px).
1. Content-grew-beneath-a-pinned-viewport (static/ui.js scroll listener).
While streaming on a tall transcript (especially on mobile, where chunks land
fast), new content increases scrollHeight under a stationary viewport. The
reader never scrolled (top did not move up, _messageUserUnpinned is false), but
bottomDistance crosses the nearBottom threshold, so the code fell through to
_scrollPinned=false, killing auto-follow mid-stream. The follow writer and the
scroll listener then fought frame-by-frame; the viewport stalled while content
kept growing and was progressively stranded. Fix: in the !_messageUserUnpinned
branch, when the viewport did NOT move up and auto-follow is on, keep the pin
and re-snap to the true bottom via _setMessageScrollToBottom() instead of
unpinning. The genuine scroll-away fallthrough is unchanged.
2. SSE-recovery follow-restore (static/messages.js). _handleStreamError (SSE
drop), the Task-cancelled apply + fallback paths, and the reconnect-stream-dead
cleanup all push/replace S.messages then renderMessages({preserveScroll:true}).
preserveScroll's restore path keys on the pre-render snapshot's bottom-distance,
which during a live stream can read large (content grew under a followed
viewport), so it yanked a following reader up to a stale historical position on
a process restart / SSE drop / cancel. Fix: capture follow-intent
(_isMessagePaneNearBottom) BEFORE mutating S.messages, and after the recovery
render, scrollToBottom() if the reader was following — so they see the
interruption/cancellation notice in place. Readers who scrolled up to read
history are left where they were.
Structural source-lock tests in tests/test_sse_recovery_scroll_stranding.py;
behavioral A/B verified via Playwright on a local source build.
A turn that streamed only prose (a long plain-text answer, or a degeneration
burst that floods the body with repeated tokens) still projected an anchor
activity scene whose activity_rows were all prose/terminal — zero tool/thinking
rows. The settle path promoted it to a collapsed worklog anyway (the gate only
checked activity_rows.length), hiding the whole answer and shrinking the
transcript by the full streamed height at STREAM_DONE, so the browser clamped a
bottom-pinned viewport back to the top (the 'jump back' report).
Add a worklog-worthiness predicate at both gates: the generation gate
(_anchorSceneHasWorklogWorthyRows in messages.js, decides whether to attach a
scene at all) and the render gate (_anchorSceneSceneHasWorklogWorthyRows in
ui.js, defense-in-depth for already-persisted all-prose scenes). A scene is
worklog-worthy only if it has >=1 tool/thinking row or a compression lifecycle
card; pure prose is not, so the turn renders as normal visible prose and the
viewport stays pinned at the bottom.
Behavioral + structural tests in tests/test_pure_prose_turn_not_worklog.py.
Address maintainer must-fix: _shouldKeepSettledWorklogOpenForPinnedFollow()
keyed only on global pin state, so every historical settled worklog re-expanded
on any pinned re-render — defeating the compact default. Gate the keep-open
exception on a one-shot token = the just-settled stream id (armed at STREAM_DONE
renderMessages, disarmed right after), threaded as _shouldKeep...(streamId).
Unpinned path unchanged. Replace source-string test with a behavioral Node test
that drives a second pinned turn and asserts it collapses.
Follow-up to maintainer review on #5058: make the pinned-follower helper comment
explicit that avoiding the visible STREAM_DONE shrink jump intentionally takes
precedence over a transient user-collapsed live worklog while the reader remains
pinned. Add a source lock so that intent stays documented.
Focused scroll locks remain green (22 passed), node --check clean.
A new post-#4970 scroll-jump class surfaced after Release YH: when a streamed
assistant turn with a large live worklog/tool trace settles, the settled compact
worklog collapses hundreds of pixels at STREAM_DONE. The reader is still pinned
and scroll state is correct, but the net scrollHeight shrink makes the browser
clamp scrollTop down by the same amount, which is visible as a large backward
jump.
Fix by keeping the just-settled activity worklog open for pinned followers
(_scrollPinned && !_messageUserUnpinned), so the live->settled DOM swap is
height-stable. Unpinned readers still get compact settled worklogs and keep their
viewport normally. Also make _anchorSceneWorklogGroup respect opts.collapsed;
previously it hard-coded collapsed: !live so the caller could not request an open
settled group.
Regression locks assert the pinned-follow helper, that settled rendering passes
collapsed:!keepSettledWorklogOpen, and that _anchorSceneWorklogGroup honors an
explicit opts.collapsed. Verified locally with a Playwright repro: before the
fix a large tool-worklog answer produced scrollHeight -367/-422px and scrollTop
-367/-422px at STREAM_DONE; after the fix nShrinks=0 and nBack=0. Local focused
scroll locks pass: 22 passed.
