* fix(chat): keep transcript pinned across composer auto-resize (#5514) Reported by @JeffArcLogic. autoResize() sets the textarea to height:'auto' before measuring scrollHeight and restoring the height. The transient collapse grows the flex:1 #messages viewport and the browser clamps a bottom-anchored scrollTop down by the collapse delta, stranding the reader a few px above the bottom on every keystroke while the composer is multi-row (delta proportional to composer height) — and the clamp's async scroll event sticky-unpins the reader, dead-ending the #5516 grow-path re-pin and stream auto-follow. The prior #5516 fix only re-pinned on NET growth, so a steady-state keystroke was missed. Fix: snapshot #messages.scrollTop before the height round-trip and restore it after, undoing the transient clamp within the same synchronous task. Protects pinned AND near-bottom scrolled-up readers, takes no _programmaticScroll latch, inert to iOS toolbar reflows. The grow-gated re-pin + ResizeObserver remain for genuine net growth. * test(#5514): make near-bottom-unpinned case non-vacuous (Codex gate nit) The companion unpinned case used scrollTop=7900, below the transient clamp max (7903), so it never actually clamped and passed on master too. Move it to 7990 (inside the clamp zone: 7903 < 7990 < settled-bottom 8023) so it genuinely exercises the transient-clamp yank the fix prevents. * Release exp-v0.52.52: keep transcript pinned across composer auto-resize (#5962, #5514) --------- Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
This commit is contained in:
1 parent
3722f2a743
commit
7d94b1c890
3 files changed
+181
-20
No files matched your search
@@ -13,6 +13,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **The transcript no longer jumps up while you type in a multi-row composer.** When the composer had grown to multiple rows and you were scrolled to the bottom, every keystroke nudged the transcript upward (by roughly the composer's height) and quietly broke stream auto-follow — you'd have to scroll back down. The auto-resize now preserves the scroll position of a bottom-pinned reader, so typing keeps you pinned and streaming keeps following. Readers who deliberately scrolled up are unaffected. Thanks @JeffArcLogic for the report and repro. (#5962, #5514)
|
||||
|
||||
- **The reasoning-effort chip is correct right after an empty startup.** On a normal boot with no active session restored, the top-bar reasoning chip wasn't refreshed for the hydrated default model, so it could show a stale value until you changed the model. It now refreshes on empty boot too. Thanks @rodboev. (#5967, #5954)
|
||||
|
||||
- **Transparent Stream no longer bloats memory (or freezes the tab) on long tool-heavy chats.** With Transparent Stream enabled, opening a conversation with many prompts built a DOM node for every activity event of every past turn at load and eagerly rendered each tool row's full detail body — a long reasoning-heavy history could balloon to multiple GB and lock up the tab. Settled tool rows now render header-only and build their detail on first expand (identical to the old view once opened), and a turn with a very large number of steps shows its most recent 30 behind a "Show N earlier steps" control. Live turns are unchanged. Thanks @curtisszmania-cytocync for the report. (#5974, #5966)
|
||||
|
||||
+27
-7
@@ -6376,16 +6376,36 @@ function autoResize(){
|
||||
}
|
||||
const el=$('msg');
|
||||
const _prevComposerH=el.offsetHeight;
|
||||
// #5514: autoResize() momentarily sets the textarea to height:'auto' (collapses
|
||||
// a multi-row composer toward its 1-row min) before reading scrollHeight and
|
||||
// restoring the measured height. That transient collapse GROWS the flex:1
|
||||
// #messages viewport, and reading scrollHeight forces a synchronous reflow — so
|
||||
// the browser CLAMPS a bottom-anchored scrollTop DOWN by the collapse delta and
|
||||
// does NOT restore it when the height snaps back. The reader is left stranded
|
||||
// Δpx above the bottom on EVERY keystroke while the composer is multi-row (Δ ∝
|
||||
// composer height), and the clamp's async scroll event also sticky-unpins the
|
||||
// reader (_messageUserUnpinned=true), dead-ending the grow-path re-pin below and
|
||||
// stream auto-follow until they manually scroll back. #5516's net-growth gate
|
||||
// never caught this because a steady-state keystroke has no NET height change.
|
||||
// Root-cause fix: snapshot the transcript's scrollTop BEFORE the height
|
||||
// round-trip and restore it AFTER, undoing the transient clamp within the same
|
||||
// synchronous task so the poisoning scroll event never fires. This protects
|
||||
// pinned readers AND near-bottom readers who scrolled up to re-read (their exact
|
||||
// position is preserved), takes no _programmaticScroll latch, and is inert to
|
||||
// iOS dynamic-toolbar reflows. A genuine NET grow/shrink still lands the reader
|
||||
// Δnet off-bottom; the grow-gated re-pin below (and the #composerWrap
|
||||
// ResizeObserver) then snap a still-pinned reader to the true bottom.
|
||||
const _msgs=$('messages');
|
||||
const _prevScrollTop=_msgs?_msgs.scrollTop:0;
|
||||
el.style.height='auto';
|
||||
el.style.height=Math.min(el.scrollHeight,200)+'px';
|
||||
if(_msgs&&_msgs.scrollTop!==_prevScrollTop) _msgs.scrollTop=_prevScrollTop;
|
||||
updateSendBtn();
|
||||
// #5514/#5515: growing the composer shrinks the flex:1 transcript viewport, so
|
||||
// a reader pinned to the bottom would be stranded above it ("scrolls up 1 row
|
||||
// per composer row"). Re-pin the transcript when it's genuinely still pinned —
|
||||
// but only when the composer actually GREW (a shrink enlarges the viewport and
|
||||
// can't strand a reader), so the common no-height-change keystroke skips the
|
||||
// re-pin's DOM read entirely. The #composerWrap ResizeObserver is the safety
|
||||
// net for any growth path that doesn't route through here.
|
||||
// Genuine NET growth (a new row that keeps the composer taller than before)
|
||||
// still shrinks the settled viewport, so a pinned reader must be re-pinned to
|
||||
// the true bottom. Guarded to fire only on real growth and only when genuinely
|
||||
// pinned (the helper no-ops for a scrolled-away reader). The #composerWrap
|
||||
// ResizeObserver is the safety net for growth paths that don't route here.
|
||||
if(el.offsetHeight>_prevComposerH && typeof _repinMessagesAfterComposerResize==='function') _repinMessagesAfterComposerResize();
|
||||
}
|
||||
function scheduleComposerAutoResize(){
|
||||
|
||||
@@ -12,15 +12,20 @@ composer growth via paths that don't route through the input->autoResize seam
|
||||
(paste, draft restore, programmatic value set, reflow) reads to the user as a
|
||||
random upward jump.
|
||||
|
||||
Fix: `_repinMessagesAfterComposerResize()` (static/ui.js) re-pins the transcript
|
||||
to the bottom ONLY when the reader is genuinely still pinned (honors
|
||||
`_messageUserUnpinned` / `_scrollPinned` so a reader who scrolled away is never
|
||||
yanked back). It is called (a) from autoResize() and (b) from a ResizeObserver on
|
||||
the #composerWrap wrapper that catches every height-change path (typed input,
|
||||
paste, draft restore, attachment tray / selection chip).
|
||||
Fix (v2, #5514 root cause): `autoResize()` now snapshots `#messages.scrollTop`
|
||||
BEFORE its `height:'auto'` → measure → restore round-trip and restores it AFTER,
|
||||
undoing the transient viewport-grow clamp within the same synchronous task. This
|
||||
protects BOTH a bottom-pinned reader and a near-bottom reader who scrolled up to
|
||||
re-read (their exact position is preserved), and — because the poisoning async
|
||||
scroll event never fires — it stops the clamp from sticky-unpinning the reader
|
||||
(which had dead-ended the #5516 grow-path re-pin and stream auto-follow). The
|
||||
grow-gated `_repinMessagesAfterComposerResize()` (static/ui.js) and the
|
||||
`#composerWrap` ResizeObserver REMAIN for genuine NET growth: they re-pin the
|
||||
transcript to the bottom ONLY when the reader is still pinned (honoring
|
||||
`_messageUserUnpinned` / `_scrollPinned`).
|
||||
|
||||
This module verifies BOTH the static wiring and the helper's guard logic via a
|
||||
node `vm` sandbox that reproduces the pinned-vs-unpinned viewport-shrink scenario.
|
||||
This module verifies the static wiring, the helper's guard logic, AND the actual
|
||||
autoResize() round-trip clamp-and-restore via a node `vm` sandbox.
|
||||
"""
|
||||
import json
|
||||
import shutil
|
||||
@@ -58,16 +63,46 @@ def test_helper_exists_and_is_pin_guarded():
|
||||
assert "<=1) return;" in body
|
||||
|
||||
|
||||
def test_autoresize_calls_the_repin():
|
||||
def _autoresize_body() -> str:
|
||||
start = MESSAGES_JS.find("function autoResize(")
|
||||
assert start != -1
|
||||
end = MESSAGES_JS.find("function scheduleComposerAutoResize(", start)
|
||||
assert end > start
|
||||
body = MESSAGES_JS[start:end]
|
||||
# The height write must still be there...
|
||||
return MESSAGES_JS[start:end]
|
||||
|
||||
|
||||
def test_autoresize_preserves_scrolltop_across_height_roundtrip():
|
||||
# #5514 root-cause fix: autoResize() momentarily collapses the textarea to
|
||||
# height:'auto' before restoring the measured height. That transient collapse
|
||||
# grows the flex:1 transcript viewport and the browser clamps a bottom-anchored
|
||||
# scrollTop DOWN. The fix snapshots #messages.scrollTop BEFORE the round-trip
|
||||
# and restores it AFTER, undoing the clamp in the same synchronous task (so the
|
||||
# poisoning async scroll event never fires and near-bottom UNPINNED readers are
|
||||
# protected too, not just pinned ones).
|
||||
body = _autoresize_body()
|
||||
# The height round-trip must still be there.
|
||||
assert "el.style.height='auto'" in body
|
||||
assert "el.style.height=Math.min(el.scrollHeight,200)+'px'" in body
|
||||
# ...and the re-pin must be called after it, guarded so it only fires when the
|
||||
# composer actually grew (skips the DOM read on a no-height-change keystroke).
|
||||
# scrollTop is snapshotted BEFORE the round-trip...
|
||||
assert "const _prevScrollTop=" in body
|
||||
prev_at = body.find("const _prevScrollTop=")
|
||||
auto_at = body.find("el.style.height='auto'")
|
||||
assert prev_at != -1 and auto_at != -1 and prev_at < auto_at, (
|
||||
"scrollTop must be captured before the height:'auto' collapse"
|
||||
)
|
||||
# ...and restored AFTER the final height write (undo the transient clamp).
|
||||
restore = "if(_msgs&&_msgs.scrollTop!==_prevScrollTop) _msgs.scrollTop=_prevScrollTop;"
|
||||
assert restore in body
|
||||
write_at = body.find("el.style.height=Math.min(el.scrollHeight,200)+'px'")
|
||||
restore_at = body.find(restore)
|
||||
assert write_at < restore_at, "scrollTop must be restored AFTER the settled height write"
|
||||
|
||||
|
||||
def test_autoresize_keeps_grow_gated_repin_for_net_growth():
|
||||
# The preserve-scrollTop fix handles the transient clamp; genuine NET growth
|
||||
# still shrinks the settled viewport, so the grow-gated re-pin must REMAIN to
|
||||
# snap a still-pinned reader to the true bottom on a real new row.
|
||||
body = _autoresize_body()
|
||||
assert "_repinMessagesAfterComposerResize()" in body
|
||||
assert "el.offsetHeight>_prevComposerH" in body
|
||||
|
||||
@@ -139,3 +174,107 @@ def test_pinned_but_already_at_bottom_is_a_noop():
|
||||
# bottomDist is already <=1 at call time (8768-8155-613 = 0), so no re-pin call.
|
||||
assert out["pinnedCalled"] is False
|
||||
assert out["bottomDist"] == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Behavioral (node vm) — the autoResize() round-trip clamp-and-restore (#5514)
|
||||
# ---------------------------------------------------------------------------
|
||||
# This exercises the ACTUAL autoResize() body extracted from messages.js (not
|
||||
# just the helper), reproducing the transient height:'auto' collapse that grows
|
||||
# the transcript viewport and clamps a bottom-anchored scrollTop. It is the
|
||||
# non-vacuous mechanism proof: on the pre-fix (net-growth-gated) autoResize this
|
||||
# harness leaves the reader stranded; with the preserve-scrollTop fix it does not.
|
||||
|
||||
def _run_autoresize(scenario):
|
||||
node = shutil.which("node")
|
||||
if not node: # pragma: no cover
|
||||
pytest.skip("node not available")
|
||||
body = _autoresize_body()
|
||||
harness = textwrap.dedent(
|
||||
"""
|
||||
// Model the browser's scroll clamp faithfully: scrollTop is a STORED
|
||||
// value. When clientHeight grows (viewport enlarges) the browser lowers
|
||||
// the stored scrollTop to the new max (scrollHeight-clientHeight) — and
|
||||
// does NOT restore it when clientHeight shrinks back. That persisted
|
||||
// downward clamp is exactly the #5514 strand, so clientHeight must be a
|
||||
// setter that re-clamps the stored top (a naive read-time getter would
|
||||
// silently "heal" the clamp and hide the bug).
|
||||
function makeEl(scrollHeight, clientHeight, scrollTop){
|
||||
let _top = scrollTop, _ch = clientHeight;
|
||||
return {
|
||||
scrollHeight,
|
||||
get clientHeight(){ return _ch; },
|
||||
set clientHeight(v){ _ch = v; if(_top > this.scrollHeight - _ch) _top = this.scrollHeight - _ch; },
|
||||
get scrollTop(){ return _top; },
|
||||
set scrollTop(v){ _top = Math.max(0, Math.min(v, this.scrollHeight - _ch)); },
|
||||
};
|
||||
}
|
||||
// The composer textarea: height:'auto' collapses it to its 1-row min,
|
||||
// which enlarges the messages viewport by (currentHeight - minHeight);
|
||||
// restoring the measured height shrinks the viewport back.
|
||||
const MIN_H = 44, FULL_H = %(composerH)s, SCROLLH = 8768, BASE_CLIENT = 745;
|
||||
// messages viewport at the SETTLED composer height:
|
||||
const msgsEl = makeEl(SCROLLH, BASE_CLIENT, %(scrolltop)s);
|
||||
let _composerH = FULL_H;
|
||||
const msgEl = {
|
||||
// offsetHeight tracks the styled height; scrollHeight (content) is FULL_H.
|
||||
get offsetHeight(){ return _composerH; },
|
||||
scrollHeight: FULL_H,
|
||||
style: {
|
||||
set height(v){
|
||||
if(v === 'auto'){ _composerH = MIN_H; msgsEl.clientHeight = BASE_CLIENT + (FULL_H - MIN_H); }
|
||||
else { _composerH = Math.min(parseInt(v,10) || FULL_H, 200);
|
||||
msgsEl.clientHeight = BASE_CLIENT + (FULL_H - _composerH); }
|
||||
},
|
||||
get height(){ return _composerH + 'px'; },
|
||||
},
|
||||
};
|
||||
const $ = (id) => (id === 'msg' ? msgEl : id === 'messages' ? msgsEl : null);
|
||||
let _messageUserUnpinned = %(unpinned)s, _scrollPinned = %(pinned)s;
|
||||
let _composerAutoResizeRaf = 0;
|
||||
let _repinCalls = 0;
|
||||
function updateSendBtn(){}
|
||||
function _messageBottomDistance(){ return msgsEl.scrollHeight - msgsEl.scrollTop - msgsEl.clientHeight; }
|
||||
function _setMessageScrollToBottom(){ msgsEl.scrollTop = msgsEl.scrollHeight; }
|
||||
function _repinMessagesAfterComposerResize(){
|
||||
_repinCalls++;
|
||||
if(_messageUserUnpinned || !_scrollPinned) return;
|
||||
if(_messageBottomDistance() <= 1) return;
|
||||
_setMessageScrollToBottom();
|
||||
}
|
||||
|
||||
%(autoresize)s
|
||||
|
||||
// Steady-state keystroke: composer already FULL_H, no NET height change.
|
||||
autoResize();
|
||||
const bottomDist = msgsEl.scrollHeight - msgsEl.scrollTop - msgsEl.clientHeight;
|
||||
console.log(JSON.stringify({ scrollTop: msgsEl.scrollTop, bottomDist, repinCalls: _repinCalls }));
|
||||
"""
|
||||
) % {**scenario, "autoresize": body}
|
||||
proc = subprocess.run([node, "-e", harness], capture_output=True, text=True, timeout=30)
|
||||
assert proc.returncode == 0, f"node harness failed: {proc.stderr}"
|
||||
return json.loads(proc.stdout.strip())
|
||||
|
||||
|
||||
def test_steady_state_keystroke_does_not_strand_pinned_reader():
|
||||
# THE #5514 REGRESSION. Pinned at the true bottom, multi-row (164px) composer,
|
||||
# a keystroke with NO net height change. Pre-fix (net-growth gate) this left
|
||||
# bottomDist == (FULL_H - MIN_H) = 120 (stranded); the preserve-scrollTop fix
|
||||
# restores the reader to the bottom within the same task.
|
||||
out = _run_autoresize({"unpinned": "false", "pinned": "true", "scrolltop": 8023, "composerH": 164})
|
||||
assert out["bottomDist"] == 0, f"pinned reader must stay glued to the bottom; got {out}"
|
||||
|
||||
|
||||
def test_steady_state_keystroke_preserves_near_bottom_unpinned_reader():
|
||||
# Fable's residual: a reader who scrolled up a little (sticky-unpinned) to
|
||||
# re-read the tail while composing must keep their EXACT position across the
|
||||
# round-trip — the re-pin approach would skip them and the transient clamp
|
||||
# would still move them. Preserve-scrollTop keeps them put.
|
||||
# scrolltop 7990 sits INSIDE the clamp zone: settled bottom = 8768-745 = 8023,
|
||||
# transient max (composer collapsed) = 8768-865 = 7903. So on the pre-fix path
|
||||
# the transient clamp pulls 7990 -> 7903 (an 87px yank); the fix restores 7990.
|
||||
# (Chosen > 7903 so the case genuinely exercises the clamp — a value below the
|
||||
# transient max would never clamp and the assertion would be vacuous.)
|
||||
out = _run_autoresize({"unpinned": "true", "pinned": "false", "scrolltop": 7990, "composerH": 164})
|
||||
assert out["scrollTop"] == 7990, f"near-bottom unpinned reader must not move; got {out}"
|
||||
assert out["repinCalls"] == 0, "must not re-pin an unpinned reader"
|
||||
Reference in new issue
Block a user