fix(chat): stop the submit up-hop and keep the thinking pin alive across paragraph breaks
Build and Push Containers / build-and-push-app (push) Successful in 3m55s
Build and Push Containers / build-and-push-db (push) Successful in 13s

- scrollReveal lands at the document bottom (window.scrollTo) instead of
  scrollIntoView({ block: 'end' }): the old alignment sat above the
  in-flow composer, so every Enter hopped the page up by the
  composer+footer height and pushed the composer below the fold.
- The thinking window's pin state is now captured BEFORE the re-render
  (const pinned = block.open && isThinkingNearBottom(textEl)): the
  post-render distance read the new chunk's rendered height, not the
  user's position, so any chunk taller than the 32px band (real-model
  deltas, '\n\n' paragraph breaks) killed the follow at the first
  2-newline gap.
- Mock LLM: new 'think in paragraphs' trigger (scratchpad with real
  blank-line breaks, 60-char frames) — the 12-char mock frames never
  rendered past the band, which is why the bug survived the E2E gates.
- E2E (both verified red against the old code):
  test_submit_does_not_hop_up, test_thinking_window_follows_across_paragraph_breaks.
- Unit source-marker tests updated to the new contracts.
This commit is contained in:
2026-08-28 17:10:02 -04:00
parent 03d26255c6
commit 3a404eb161
6 changed files with 365 additions and 55 deletions
+33 -18
View File
@@ -6,16 +6,22 @@ NEVER auto-scrolls while a turn streams (thinking / tool / delta frames
all leave the viewport alone), so a user reading earlier content is no
longer yanked down mid-answer. Scrolls happen only on explicit user
intent: the submit (the user's own message is revealed) and the phase-14
restore landing (one-shot, load-time).
restore landing (one-shot, load-time). Both user-intent scrolls land at
the DOCUMENT BOTTOM (scrollReveal's `window.scrollTo`): the old
`scrollIntoView({ block: "end" })` aligned the message's bottom to the
viewport bottom — which sits above the in-flow composer — so every
submit hopped the page UP by the composer+footer height and pushed the
composer below the fold.
The JS behavior itself is E2E-covered
(tests/e2e/test_no_reply_autoscroll.py); here we pin the source markers
of the new contract — the phase-18 gate is gone (no NEAR_BOTTOM_PX /
of the contract — the phase-18 gate is gone (no NEAR_BOTTOM_PX /
isNearBottom), scrollReveal scrolls unconditionally and is the single
scrollIntoView in app.js, addMessage takes an explicit `scroll` intent,
and the streaming handlers contain no page-scroll call at all — so a
silent regression back to per-frame autoscroll is caught without a
browser.
page scroll in app.js (a document-bottom `window.scrollTo` — no
`scrollIntoView` call remains), addMessage takes an explicit `scroll`
intent, and the streaming handlers contain no page-scroll call at all —
so a silent regression back to per-frame autoscroll (or to the
upward-hopping block:"end" reveal) is caught without a browser.
"""
from __future__ import annotations
@@ -56,25 +62,34 @@ def test_phase18_gate_is_gone() -> None:
def test_scroll_helper_is_unconditional() -> None:
"""scrollReveal is still the ONE scrollIntoView in app.js, but it now
scrolls unconditionally — no force-or-near-bottom condition in its
body, and the phase-18 `force` parameter is gone. A page scroll can
only ever happen where scrollReveal is CALLED (submit + restore)."""
"""scrollReveal is still the ONE page scroll in app.js, and it
scrolls unconditionally — no gate in its body, and the phase-18
`force` parameter is gone. A page scroll can only ever happen where
scrollReveal is CALLED (submit + restore). It lands at the document
BOTTOM via `window.scrollTo`: the old `scrollIntoView({ block:
"end" })` aligned the message's bottom to the viewport bottom, which
sits above the in-flow composer, so every submit hopped the page UP
by the composer+footer height (the "Enter scrolls the page up"
bug) — no `scrollIntoView` call may remain."""
js = _js()
body = _fn_body(js, "scrollReveal")
assert "scrollIntoView" in body
assert 'block: "end"' in body
assert "window.scrollTo(" in body
assert "document.documentElement.scrollHeight" in body
assert "if (" not in body, "the helper must have no gate — it scrolls when called"
assert "force" not in body, "the phase-18 force parameter must be gone"
assert 'block: "end"' not in body, (
"the upward-hopping block:'end' alignment must be gone"
)
# Still smooth / reduced-motion-aware through the default behavior.
assert "behavior = SCROLL" in body
# The regression pin: exactly one actual scrollIntoView CALL in the
# whole file, and it lives inside scrollReveal (the word may appear
# in comments; the call must not).
assert js.count(".scrollIntoView(") == 1, (
"app.js must call scrollIntoView exactly once (inside scrollReveal)"
# The regression pins: no scrollIntoView call anywhere in the file,
# and the one window.scrollTo call lives inside scrollReveal.
assert js.count(".scrollIntoView(") == 0, (
"app.js must not call scrollIntoView — the document-bottom "
"scrollTo replaces the block:'end' reveal"
)
assert js.find(".scrollIntoView(") > js.find("function scrollReveal")
assert js.count("window.scrollTo(") == 1
assert js.find("window.scrollTo(") > js.find("function scrollReveal")
def test_add_message_takes_explicit_scroll_intent() -> None:
+45 -20
View File
@@ -12,12 +12,22 @@ live tail only while the user is pinned near the window's bottom (the
follow; returning to the bottom re-arms it (the check runs on every
chunk, by construction).
The gate is measured against the PRE-render geometry: the chunk's
re-render grows the window's content below the old bottom, so a
post-render reading measures the new chunk's height, not the user's
position — any chunk taller than the 32px band (a real model's
sentence, or a "\n\n" paragraph break) killed the follow at the first
2-newline gap. The pin state is captured into ``pinned`` BEFORE
``textEl.innerHTML = …`` and the pin line runs inside ``if (pinned)``
after it.
The browser behavior itself is E2E-covered
(tests/e2e/test_thinking_scroll.py, task 03); here we pin the CSS
value + the owner-direction comment, the exported band, the gate
function's math, and the gated pin call — so a silent regression
(``overflow-y`` back to ``hidden``, band removed, pin ungated) is
catched without a browser.
function's math, and the pre-render capture + gated pin call — so a
silent regression (``overflow-y`` back to ``hidden``, band removed,
pin ungated, or the capture moved back after the re-render) is caught
without a browser.
"""
from __future__ import annotations
@@ -94,35 +104,50 @@ def test_is_thinking_near_bottom_band_math() -> None:
def test_thinking_pin_is_gated_on_window_bottom() -> None:
"""The phase-17 pin is now GATED: the pin line sits inside
``if (block.open && isThinkingNearBottom(textEl))`` in the
streaming thinking branch — the window follows the live tail only
while the user is pinned near its bottom, and ``block.open`` stays
in the gate so a closed block (e.g. restored collapsed, phase 17)
is never pinned. No unconditional ``if (block.open) { … pin … }``
remains anywhere in app.js (the exact old gate string is gone)."""
"""The phase-17 pin is GATED on the user's pin state captured BEFORE
the re-render: ``const pinned = block.open &&
isThinkingNearBottom(textEl)`` sits above ``textEl.innerHTML =
…`` in the streaming thinking branch, and the pin line runs inside
``if (pinned)`` below it. Measuring after the update would read the
new chunk's rendered height instead of the user's position — the
"2-newline gap" regression. ``block.open`` stays in the capture so a
closed block (e.g. restored collapsed, phase 17) is never pinned,
and no direct ``if (block.open)`` gate remains anywhere in app.js."""
js = _js()
pin = "textEl.scrollTop = textEl.scrollHeight"
assert js.count(pin) == 1, "the bottom-pin must exist exactly once"
# Surviving task-01 assertion: the pin lives in the streaming
# thinking branch (before the delta branch).
# The pin lives in the streaming thinking branch (before the delta
# branch).
thinking_idx = js.find('ev.type === "thinking"')
delta_idx = js.find('ev.type === "delta"')
assert -1 < thinking_idx < delta_idx
thinking_branch = js[thinking_idx:delta_idx]
gate = "if (block.open && isThinkingNearBottom(textEl))"
assert gate in thinking_branch, "the pin must be behind the combined gate"
# The pin line follows the gate (inside it) — the only pin in the
# branch is the gated one.
gate_idx = thinking_branch.find(gate)
capture = "block.open && isThinkingNearBottom(textEl)"
render = "textEl.innerHTML = renderMarkdown(thinkingAcc)"
gate = "if (pinned)"
assert capture in thinking_branch, (
"the combined gate must be captured (block.open + the window band)"
)
assert gate in thinking_branch, "the pin must sit inside the captured gate"
# Pre-capture → re-render → gated pin, in exactly that order: the
# pin state is the user's PRE-render position, not the chunk's
# rendered height.
cap_idx = thinking_branch.find(capture)
render_idx = thinking_branch.find(render, cap_idx)
assert render_idx != -1 and render_idx > cap_idx, (
"the pin state must be measured BEFORE the re-render"
)
gate_idx = thinking_branch.find(gate, render_idx)
assert gate_idx != -1 and gate_idx > render_idx, (
"the pin must run AFTER the re-render, inside the captured gate"
)
pin_idx = thinking_branch.find(pin, gate_idx)
assert pin_idx != -1
assert thinking_branch.count(pin) == 1
# The old unconditional gate is gone from the whole file: no
# `if (block.open)` (closed paren) — the combined condition is the
# No direct `if (block.open)` gate may remain — the capture is the
# only gate left.
assert "if (block.open)" not in js, (
"no unconditional `if (block.open)` pin may remain"
"no direct `if (block.open)` gate may remain — the capture is the gate"
)