Skip to content

fix: calibrate virtual row heights from measurements, not flat constants - #8

Draft
alanjds wants to merge 12 commits into
masterfrom
claude/webui-virtual-height-calibration
Draft

alanjds wants to merge 12 commits into
masterfrom
claude/webui-virtual-height-calibration

Conversation

@alanjds

@alanjds alanjds commented Aug 24, 2026 •

Copy link
Copy Markdown
Owner

While chasing a scroll-drift bug I found the virtual scroll geometry runs ~34% short of the real transcript height (326k px vs 493k px on a 2000-message session): only ~45 rows ever get measured, and every other row falls back to a flat per-role constant. The measured average the code already computes was passed in as defaultHeight but never actually reached, because rowHeightFor() always took the constant instead.

Now it keeps a measured mean per role and uses that for unmeasured rows, so the geometry is ~6.6% off instead of 34%. Roles fall back to the old constant until they have samples, so first paint is unchanged.

Honest caveat: this did not fix the drift I was chasing — that turned out to be elsewhere, and the drift measured identically before and after. So this is a correctness fix with no user-visible symptom attached. Nothing else changed; targeted suite green, and the new test fails if the calibration is reverted. virtualize_transcript only. Feel free to merge or drop.

claude and others added 12 commits August 24, 2026 20:23
… constants

Groundwork for nesquena#4343, and a dead-code fix. NOT a fix for the scroll
oscillation itself — see the measurement below before assuming otherwise.

_messageVirtualWindow's rowHeightFor() gives every UNMEASURED row a height so
the virtual scroll geometry can be computed. It sourced that from the flat
per-role constants in MESSAGE_VIRTUAL_DEFAULT_ROW_HEIGHTS:

    return roleForIdx ? _messageVirtualDefaultHeightForRole(roleForIdx(idx))
                      : defaultHeight;

_updateMessageVirtualMeasurements already maintained a measurement-derived mean
in _messageVirtualEstimatedRowHeight, and _currentMessageVirtualWindow already
passed it down as `defaultHeight` — but roleForIdx is ALWAYS supplied, so that
branch never ran and the calibrated number was dead code. Every row that never
entered the render window kept the guess forever.

On a 2000-message session only ~45 rows are ever measured, so 98% of the
geometry came from the guess, and the guesses are wrong in both directions
(measured median 214 against user:120 / assistant:160 / tool_call:400). The
errors do not cancel: total scroll height came out 326,267px against a real
492,942px, 34% short.

Track a running mean PER ROLE instead and use it for unmeasured rows (a single
global mean would be worse than the constants, since a tool_call row and a user
row differ enormously). A role falls back to its static seed until it has
MESSAGE_VIRTUAL_ROLE_SAMPLE_MIN samples, so first paint is unchanged, and a
session switch drops the samples with the rest of the height cache. Applied at
all three unmeasured-row sites (rowHeightFor, _messageVirtualPrependedHeightDelta,
_messageVirtualScrollTopForVisibleIdx) rather than just the one, so they cannot
disagree about the same row's height.

Effect, measured with real wheel events on that session:

  - scroll geometry: 326,267px -> 460,631px against a real 492,942px
    (34% short -> 6.6% short)
  - drift of the row the reader is looking at, after the measurement pass
    settles: 800px median / 848px max BEFORE, and 800px median / 848px max
    AFTER. Unchanged. Zero.

So the geometry error is real and now largely fixed, but it is NOT what causes
the nesquena#4343 oscillation. Trapping the scrollTop setter names the actual writer:
_scrollAfterMessageRender -> _followMessagesAfterDomReplace -> scrollToBottom
-> _setMessageScrollToBottom, firing on the measurement-driven re-render even
though the caller passes preserveScroll:true and _messageUserUnpinned is
already true; the delayed _settleMessageScrollToBottom writes look like the
live culprit, re-scheduled by each re-render after _cancelBottomSettle clears
them. That is a separate change and virtualize_transcript stays default-off
until it lands.

The existing virtualization tests all still pass with the static fallback (no
samples recorded), so they would pass against a reverted fix and prove nothing
on their own; test_unmeasured_rows_use_measured_role_mean_once_samples_exist
pins the behaviour that actually changed and fails (120 != 300) when the
calibration branch is removed. Five tests that eval these helpers in an
isolated node scope needed the new state/companion declared.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018y9pr7ppZgDuCZCT6x96kY
…x is provably identical; compression-anchor coverage (nesquena#6826 r3)
Removes the local re-import of api.streaming._session_payload_with_full_messages
inside test_in_tail_duplicate_guard_refuses_bounded_and_full_revision_accepted;
it is already imported at module level (line 17) and unused in the function body,
tripping F401 on CI's hosted lint. Mechanical maintainer fix per auto-fix policy.

Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
Bounded sidecar-anchored tail read for regenerate (closes nesquena#6826). Thanks @webtecnica — six rounds of adversarial re-gate to convergence.
…ail read (nesquena#7204, @webtecnica)

Adds the CHANGELOG entry for nesquena#7204 (closes nesquena#6826). Release-metadata only;
the code change ships via the merge of nesquena#7204 (contributor webtecnica).
Release exp-v0.52.264: fast regenerate via bounded sidecar-anchored tail read (nesquena#7204, @webtecnica)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants