fix(webui): reserve real user-row height in non-virtualized transcripts to stop scroll jump-back

[allenliang2022]

Summary

Fixes a chat scroll jump-back (viewport snaps hundreds/thousands of px toward the top) that fires on touch / coarse-pointer devices while a response is streaming and the reader is scrolled up in history. It is a follow-up to #5637 / #5638: those fixed the collapse for the virtualized wipe-and-rebuild path, but the non-virtualized transcript path (the #4325 opt-out, _virtualizeTranscript === false) was left uncovered.

Root cause

When transcript virtualization is disabled, renderMessages() renders every row with no windowing and never runs the virtualized measure pass — _updateMessageVirtualMeasurements early-returns when !virtualWindow.virtualized, and that pass is what remembers a user row's real height.

Under @media (pointer: coarse), .msg-row[data-role="user"] carries content-visibility: auto; contain-intrinsic-size: auto 96px. Every renderMessages() rebuild does inner.innerHTML='' then recreates all rows as fresh elements. A fresh, off-screen tall user row (e.g. a long paste measuring thousands of px) has never painted at full size, so it reserves only the flat contain-intrinsic-size estimate instead of its real height. scrollHeight shrinks by (realHeight − estimate), the browser force-clamps scrollTop to keep it within range, and the viewport jumps backward.

This is a browser clamp, not a JS scrollTop write, so scroll-anchor / stale-snapshot compensation paths cannot catch it — the row genuinely has no height that frame.

Desktop rests at content-visibility: visible (intrinsic-size is inert), which is why the mouse/desktop path never reproduces it.

Fix (three coordinated parts, all in static/ui.js)

1. CJK-aware estimate — _estimateUserRowIntrinsicHeight now weights full-width / CJK characters as ~2 columns (they wrap at ~24 chars/line, not 48). A long Chinese/Japanese/Korean paste previously under-estimated by ~2x; now a fresh never-measured row reserves close to its real height. This is the only backstop for a row the reader has never scrolled into view (a never-painted content-visibility:auto row reports only its reserve, so there is nothing to measure — the estimate must carry it).
2. max(remembered, estimate) in _applyUserRowIntrinsicHeight — a remembered height can be a partial paint: a row taller than the viewport only ever paints its intersecting slice under content-visibility:auto, so its measured height is a fraction of the real row. Reserving the larger of the remembered value and the content estimate prevents a partial measurement from under-reserving. A full measurement (short row, fully painted) still wins when it exceeds the estimate, preserving the fix(webui): stop mobile scroll jump-back — scope content-visibility + hold unpinned position (#5637) #5638 behavior.
3. Pre-wipe capture — _rememberRenderedUserRowIntrinsicHeights() runs just before inner.innerHTML='' inside renderMessages (the non-virtualized analog of fix(webui): stop mobile scroll jump-back — scope content-visibility + hold unpinned position (#5637) #5638's virtualized measure pass). It reads the still-laid-out old rows' real heights — reliable because those elements have painted — and persists them keyed by session-relative index. It only trusts rows currently within the viewport band (a fully off-screen never-painted row reports its collapsed reserve and must not poison the remembered map) and floors every persisted value at the content estimate.

Verification

Reproduced in a mobile-emulated (coarse-pointer) browser against a long real-world transcript with a tall user row and virtualization disabled. Measured the same rebuild-while-scrolled operation before and after:

Scenario	scrollHeight change	scrollTop clamp (jump-back)
Before fix	−1640 px	−1612 px
After fix — reader scrolled up to top-of-history (the reported locus)	no shrink	0
After fix — tall row never scrolled into view (estimate backstop)	~0	0
After fix — desktop pointer: fine (content-visibility inert)	0	0 (no change)

Tests

Adds tests/test_issue5744_nonvirtual_userrow_collapse_jumpback.py — 7 node-harness tests, each mutation-checked (reverting the CJK weighting, the in-viewport guard, or the pre-wipe ordering each makes the corresponding test fail). The existing #5637 / #5638 suites and the render / virtualization suites all pass.

Notes for the maintainer

I could not attach a screen recording — the only long transcripts that reproduce this are private conversations, so the evidence here is the mutation-checked harness plus the before/after measurement table above. Happy to add any additional automated coverage you'd like.

This targets the same jump-back class as #5637 / #5638 but a distinct, uncovered code path (non-virtualized full rebuild vs. virtualized windowed rebuild), so it is a separate one-logical-change PR.

[greptile-apps]

Greptile Summary

This PR fixes mobile transcript scroll jump-back when virtualization is disabled. The main changes are:

• CJK-aware user-row height estimates in static/ui.js.
• Larger-of remembered and estimated intrinsic-size reservation.
• Pre-wipe user-row height capture for non-virtualized transcript rebuilds.
• Node regression tests for the non-virtualized collapse path.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
static/ui.js	Updates transcript rendering helpers to preserve user-row intrinsic heights across non-virtualized rebuilds.
tests/test_issue5744_nonvirtual_userrow_collapse_jumpback.py	Adds regression coverage for CJK estimates, max-based reservation, pre-wipe ordering, viewport filtering, and estimate flooring.

_{Reviews (2): Last reviewed commit: "fix(webui): reserve real user-row height..." | Re-trigger Greptile}

[nesquena-hermes]

Thanks for this — the non-virtualized row-height reserve is the right analog to #5638's virtualized measure pass, and reading the pre-wipe rects (before innerHTML='' destroys them) is exactly the reliable way to get real off-screen heights. The behavioral logic is sound.

One blocker before this can go green, and it's a trivial one:

CI red — tests/test_issue4856_android_scroll_regression.py::test_rebuild_path_marks_dom_wipe_scroll_as_programmatic fails on all three Python shards.

The failure is not a real behavioral regression — your _programmaticScroll=true marker is correctly placed at static/ui.js:14078, immediately before the real inner.innerHTML='' at 14080, so the #4856 Android jump-back guard still holds at runtime. The problem is that the shipped guard test locates its search window with a naive string scan:

fix_idx  = UI_JS.find("window._fixMobileScrollJank()")
wipe_idx = UI_JS.find("innerHTML=''", fix_idx)          # first literal occurrence
window   = UI_JS[fix_idx:wipe_idx]
assert "_programmaticScroll=true" in window

Your new explanatory comment at ui.js:14064 contains the literal string innerHTML='':

// Pre-wipe capture: read the still-laid-out user rows' REAL heights before innerHTML=''

So UI_JS.find("innerHTML=''", fix_idx) now matches the comment at 14064 instead of the code at 14080, truncating the search window before the _programmaticScroll=true marker → false failure.

Fix (pick either):

1. Simplest — reword the comment so it doesn't contain the literal innerHTML='' token (e.g. "…read the still-laid-out user rows' REAL heights before the wipe destroys them"). One-word change, keeps the guard test as-is.
2. Or, if you'd rather harden the brittle test: make it locate the code wipe rather than the first textual occurrence (e.g. search for inner.innerHTML='' with the inner. receiver, which the comment doesn't contain).

I'd lean option 1 — the test is a pre-existing shipped guard and the comment is the only thing tripping it.

Also heads-up: this touches the crown-jewel chat scroll surface, so once CI is green it'll go through the visible-UI review gate (screen-recording proof of the jump-back fix across desktop + mobile) before it ships — I'll handle that side. Just get the comment fixed so CI clears.

[allenliang2022]

Thanks for the precise diagnosis — you're exactly right, my explanatory comment carried the literal innerHTML='' token and the #4856 guard test's UI_JS.find("innerHTML=''", fix_idx) matched the comment instead of the real code wipe, truncating its search window before the _programmaticScroll=true marker.

Took option 1: reworded the comment to "…read the still-laid-out user rows' REAL heights before the wipe below destroys them" so it no longer contains the token. No behavior change — the _programmaticScroll=true marker and the pre-wipe capture call are untouched and still sit before the real inner.innerHTML=''.

Pushed as an amend (b6319c7d, force-with-lease). Verified locally:

• test_issue4856_android_scroll_regression.py — passes (was the red one)
• my test_issue5744_* suite + the test_issue5637_* / test_issue5638_* sibling suites — all green

Understood on the visible-UI review gate — I'll leave the screen-recording proof to you as you offered. Let me know if anything else needs adjusting.

[nesquena-hermes]

Confirmed — the reword fixes the guard-test collision cleanly, and CI is now green across all 15 shards + lint + browser-smoke.

What was actually broken

The #4856 guard test (tests/test_issue4856_android_scroll_regression.py:96-110) scans the region between the scroll-jank guard call and the DOM wipe, and asserts the programmatic-scroll marker sits inside it:

fix_idx = UI_JS.find("window._fixMobileScrollJank()")
wipe_idx = UI_JS.find("innerHTML=''", fix_idx)
window = UI_JS[fix_idx:wipe_idx]
assert "_programmaticScroll=true" in window

Your original pre-wipe capture comment carried the literal inner.innerHTML='' token. Because find("innerHTML=''", fix_idx) returns the first match after fix_idx, wipe_idx landed on the comment (~ui.js:14072) instead of the real wipe, truncating the search window before _programmaticScroll=true — so the assertion failed even though the runtime ordering was always correct.

Why the reworded head is correct

At the PR head the ordering in renderMessages() is:

• ui.js:14049 — if(window._fixMobileScrollJank) window._fixMobileScrollJank();
• ui.js:14072 — _rememberRenderedUserRowIntrinsicHeights() (reworded comment: "…before the wipe below destroys them" — no innerHTML='' token)
• ui.js:14078 — _programmaticScroll=true;
• ui.js:14080 — inner.innerHTML='';

Now the first innerHTML='' after fix_idx is the real wipe at 14080, so the search window includes _programmaticScroll=true at 14078. The guard passes, and the runtime behavior (marker set before wipe) is unchanged. Exactly option 1 as discussed, no behavioral delta.

On the behavioral core

Re-reading the diff, the two-part reserve logic is sound. _applyUserRowIntrinsicHeight (ui.js:1319-1330) now takes Math.max(remembered, estimate) so a partial-paint measurement can't under-reserve below the content estimate, and _rememberRenderedUserRowIntrinsicHeights (ui.js:1406-1450) only trusts in-viewport (painted) rows and floors every persisted value at the estimate. The CJK column-weighting in _estimateUserRowIntrinsicHeight (ui.js:1292-1315) is a nice touch — a full-width paste wraps at ~24 chars/line, so counting wide codepoints as 2 columns keeps the fresh-off-screen reserve close to reality, which matters because content-visibility:auto reports only the reserve for a never-painted row.

This is the correct non-virtualized analog of #5638's virtualized measure pass. LGTM from my read; deferring the final merge call to a maintainer.

[nesquena-hermes]

🔬 Gate certification — GREEN ✅ (rebased onto current master v0.51.922)

Full authoritative gate (Codex + Fable + full suite + my own checks), on a worktree with current master merged in — because this PR was 8 commits behind and #5742 (desktop scroll compensation) shipped in v0.51.922 since your last push, the key question was whether the two scroll fixes interact. They don't. Clean merge, disjoint ui.js regions.

Codex — SAFE TO SHIP. Verified: _fixMobileScrollJank() still runs before the DOM wipe (ui.js:14124); the pre-wipe capture (ui.js:14147) only reads geometry + writes containIntrinsicSize, never scrollTop; _programmaticScroll=true (ui.js:14153) is still set before inner.innerHTML='' (ui.js:14155) so the #4856 clamp-marker ordering holds; rebuilt rows reserve max(remembered, estimate) (ui.js:1317) so partial paints can't under-reserve; only in-viewport rows are trusted (ui.js:1435) and the tallest value is kept (ui.js:1445), so a shrunken transient can't poison the map; #5742's desktop realign (ui.js:13617) is independent.

Fable UX — SHIP-UX, with a full platform-matrix proof of the #5742 interaction:

• Desktop (pointer: fine): fix(webui): reserve real user-row height in non-virtualized transcripts to stop scroll jump-back #5751 is geometrically inert — its contain-intrinsic-size writes only take effect under content-visibility: auto, which style.css:5823 scopes to @media (pointer: coarse). Desktop scrollHeight is bit-identical, so fix(webui): compensate stale snapshot.top on desktop live-refresh restore (#5637 follow-up) #5742's inputs are unchanged. No interaction.
• Android / coarse touch: fix(webui): compensate stale snapshot.top on desktop live-refresh restore (#5637 follow-up) #5742's desktop realign branch is unreachable (_isTouchLikeMessageViewport=true → the touch-refusal guard runs, native overflow-anchor holds). The two never write scrollTop in the same regime.
• iOS WebKit (the only dual-live surface): they compose sequentially — fix(webui): reserve real user-row height in non-virtualized transcripts to stop scroll jump-back #5751 sets geometry during the rebuild, fix(webui): compensate stale snapshot.top on desktop live-refresh restore (#5637 follow-up) #5742 measures the residual anchor movement after from live rects (currentOffset - capturedOffset applied to live el.scrollTop), so double-compensation is structurally impossible. Defense-in-depth, not a race. (The snapshot.top + delta design that would have fought is the one fix(webui): compensate stale snapshot.top on desktop live-refresh restore (#5637 follow-up) #5742 explicitly rejected.)

Full suite: 12236 passed / 0 failed. Scroll cluster: #4856 + #5637 + your new #5744 tests = 51 passed on the merged base. node --check + scope-undef gate clean.

This is the correct non-virtualized analog of #5638's virtualized measure pass, on a distinct uncovered code path. Nicely scoped, mutation-checked tests, real coverage. Queuing for the maintainer's ship nod (visible scroll surface — the only reason it's not auto-merged is the standing visible-UI sign-off rule; the motion repro needs a long coarse-pointer transcript so it's a trust-the-gate call, same as #5742).

[nesquena-hermes]

🔬 Gate certification — GREEN ✅ (full gate: Codex + Fable-UX + suite) · ⏸️ visible scroll → Nathan sign-off · #5637/#5638 family

Certified head: sha:aee593d2 (clean rebase, branch gate-rebase/5751-nonvirt-userrow-height) · PR: #5751 · allenliang2022 (T1), fix(webui): reserve real user-row height in non-virtualized transcripts to stop scroll jump-back
Verdict: GREEN. A #5638 follow-up closing the non-virtualized-transcript gap in the user-row-height reservation — stops the scrollHeight-collapse jump-back on that path. Codex SAFE, Fable-UX SHIP-UX, full suite green.

What I ran (rebased worktree /tmp/wt-rebase-5751) — full gate: Codex + Fable + suite

Gate	Result
Rebase onto current master	✅ git apply clean
Codex (max-reserve / over-reserve / CJK / non-virt-only / composes)	SAFE TO SHIP — 0 findings
Fable-UX	SHIP-UX — no jump-back, no whitespace gap
Full pytest suite	✅ 12323 passed, 0 failed (+ 572 scroll/anchor/pin tests)

Findings

✅ Clean fix: the non-virtualized transcript path (_virtualizeTranscript===false, the #4325 opt-out) never ran the virtualized measure pass, so a user row's real height was never remembered → scrollHeight collapsed on rebuild → jump-back. The fix reserves max(remembered-measurement, content-estimate): _estimateUserRowIntrinsicHeight is CJK-aware (wide chars weighted as 2 columns), and the max protects against partial-paint under-reads (a row taller than the viewport that only partially painted via content-visibility:auto reports a short height — the estimate floors it). Codex confirmed: no under-reserve on partial paint, no over-reserve whitespace/scroll-past-end, CJK weighting sound, non-virtualized-only (no double-apply with the virtualized #5638 pass), content-visibility interaction correct, composes with the whole #5637 family (#5635/#5638/#5666/#5672/#5681/#5685/#5742). Fable confirmed no jump-back on rebuild, no whitespace gap, pinned reader still follows. Full suite green (572 scroll tests).

Recommendation to Nathan

GREEN — merge from branch gate-rebase/5751-nonvirt-userrow-height (sha:aee593d2) — after a screen-recording glance (non-virtualized transcript: scroll up into history during a live rebuild → reader stays put, no jump-back, no whitespace gap under a tall user row). Completes the #5637/#5638 scroll-stability family by covering the non-virtualized (#4325 opt-out) path. Codex SAFE + Fable SHIP-UX + full suite green. concept 4/5. Author @allenliang2022 (T1). crit=3, scroll.

---

_Gate-certifier layer (warm-up → gate → release). Non-virtualized transcript (virtualizeTranscript===false, #4325 opt-out) never remembered user-row height → scrollHeight collapse → jump-back. Fix reserves max(remembered, CJK-aware estimate) — max floors partial-paint under-reads (content-visibility:auto short-reports), estimate doesn't over-reserve. Codex SAFE (no under/over-reserve, CJK sound, non-virt-only, composes #5637 family) + Fable-UX SHIP-UX (no jump-back, no whitespace gap) + full suite green (0 failed, 12323) + 572 scroll tests. Visible scroll → Nathan. Cert valid for sha:aee593d2.

[nesquena-hermes]

Shipped in v0.51.923 — thanks @allenliang2022. 🎉

This completes the scroll jump-back cluster: #5637/#5638 (virtualized) + #5742 (desktop) + now #5751 (non-virtualized / coarse-pointer). With virtualization off, a fresh off-screen tall user row under content-visibility: auto reserved only its flat placeholder → scrollHeight shrank on re-render → the browser clamped scrollTop and the viewport jumped back. The rebuild now reserves each row's real pre-wipe height (CJK-aware estimate backstop + max(remembered, estimate) floor + pre-wipe capture of in-viewport painted rows).

Gate (rebased onto current master v0.51.923, since #5742 shipped since your last push):

• Codex: SAFE TO SHIP — bug: WebUI is unusable on Android - transcript jumps to top on every interaction (regression in v0.51.576) #4856 marker ordering intact, pre-wipe capture writes no scrollTop, floor + in-viewport-trust + tallest-wins all verified.
• Fable UX: SHIP-UX — with a full platform-matrix proof that this and fix(webui): compensate stale snapshot.top on desktop live-refresh restore (#5637 follow-up) #5742 don't fight over scrollTop (desktop: this is inert; Android: fix(webui): compensate stale snapshot.top on desktop live-refresh restore (#5637 follow-up) #5742's branch unreachable; iOS: they compose sequentially, double-compensation structurally impossible).
• Full suite: 12236/0; scroll cluster (bug: WebUI is unusable on Android - transcript jumps to top on every interaction (regression in v0.51.576) #4856+Mobile scroll jump-back: off-screen .msg-row content-visibility (#5541) collapses scrollHeight → browser clamps scrollTop #5637+fix(#5723): clear dead steer busy state #5744) 51/51.

The re-worded comment fixed the #4856 guard-test collision exactly as discussed. Nicely scoped, mutation-checked tests, correct non-virtualized analog of #5638's measure pass. Deployed and verified live. (CI needed a couple of re-runs for an unrelated Playwright-browser-install runner flake — not your code.)