feat(chat): thinking window scrolls again, follows the tail only while pinned
TODO.md L7: "Add scrolling back to the thinking block, but have it autoscroll while thinking content is generating." Owner direction 2026-08-27 (roadmap A2) reverses the phase-21 no-scroll choice (2026-08-24): details.thinking .thinking-text is user-scrollable again (overflow-y: auto — the 320px clip stays, owner-confirmed), and the phase-17 per-chunk bottom-pin is GATED: the window follows the live tail only while the user is pinned near its bottom (THINKING_NEAR_ BOTTOM_PX = 32); scrolling up pauses the follow, returning to the bottom re-arms it on the next chunk (the gate re-runs on every frame). - frontend/assets/styles.css: .thinking-text overflow-y: hidden -> auto; the phase-21 owner-choice comment is replaced with the 2026-08-27 direction; max-height: 320px and every other declaration in the rule byte-identical. - frontend/assets/app.js: export const THINKING_NEAR_BOTTOM_PX = 32 + isThinkingNearBottom(textEl) (scrollHeight - scrollTop - clientHeight <= band); the thinking-handler pin becomes `if (block.open && isThinkingNearBottom(textEl))` — a scrolled-up reader is never re-pinned and a closed (restored) block is never pinned; everything else in the handler (and phase 42's no page scroll) untouched. - tests/unit/test_thinking_scroll.py (new, replaces the deleted tests/unit/test_thinking_no_scroll.py): pins the CSS contract (auto + 320px + owner-direction comment, no hidden/scroll left), the exported 32px band, the gate math, the gated pin (no unconditional `if (block.open)` remains), and the surviving collapsed-restore pin. - tests/e2e/test_thinking_scroll.py (new, mock-only, replaces the deleted tests/e2e/test_thinking_no_scroll.py — its pins asserted the reversed phase-21 behavior, so both phase-21 files are deleted in this commit): user scroll restored on the frozen 4s-hesitation tail (wheel is 1:1; click+Home keyboard — the plain div is not keyboard-focusable by design, tabindex is test scaffolding; the literal drag holds the user's position — headless Chromium's overlay scrollbars are not grabbable by synthetic mouse events, documented in the suite), follow-while-pinned at the 2nd-to-last and last chunk (±1px) with the last chunk's text inside the visible rectangle, no re-pin over ≥5 mid-stream chunks after a half-window scroll-up, re-pin on the next chunk after returning to the bottom, the CSS contract, plus the phase-11 (long answer: page scrolls, bubble overflow untouched) and phase-17 (restored collapsed block with full text) regressions. - tests/unit/test_chat_persistence.py: the CSS pin flips with the contract (auto in, hidden out — owner direction 2026-08-27). - tests/unit/test_frontend_scroll.py: the "page-level band constant is gone" pin now excludes the phase-43 window-level THINKING_NEAR_BOTTOM_PX (a different band — the window's, not the page's). E2E (isolation): test_thinking_scroll 7/7 (twice); regressions test_thinking_display 5/5, test_chat_persistence 4/4, test_no_reply_autoscroll 5/5, test_smoke 3/3; unit+integration 725 passed, app/ coverage 99% (unchanged — frontend-only phase); ruff + pyright clean.
This commit is contained in:
@@ -240,9 +240,9 @@ def test_restore_renders_collapsed_thinking_block() -> None:
|
||||
|
||||
def test_thinking_block_css_uses_phase08_tokens() -> None:
|
||||
"""Phase 17 styling (Phase-08 tokens, WCAG AA): the block frame, the
|
||||
≥44px summary control (brand-ink ≈8.7:1 on surface) and the live-tail
|
||||
scratchpad (ink-soft ≈6.9:1 on surface, 320px cap; phase 21 removed the
|
||||
user scroll — owner choice 2026-08-24)."""
|
||||
≥44px summary control (brand-ink ≈8.7:1 on surface) and the scratchpad
|
||||
(ink-soft ≈6.9:1 on surface, 320px cap; user-scrollable again since
|
||||
phase 43 — owner direction 2026-08-27, TODO.md L7)."""
|
||||
css = _css()
|
||||
block = re.search(r"details\.thinking \{([\s\S]*?)\n\}", css)
|
||||
assert block, "styles.css must style details.thinking"
|
||||
@@ -262,6 +262,7 @@ def test_thinking_block_css_uses_phase08_tokens() -> None:
|
||||
tbody = text.group(1)
|
||||
assert "var(--ink-soft)" in tbody
|
||||
assert "max-height: 320px" in tbody
|
||||
# Phase 21: no user scroll back — the window is a live tail only.
|
||||
assert "overflow-y: hidden" in tbody
|
||||
assert "overflow-y: auto" not in tbody
|
||||
# Phase 43 (owner direction 2026-08-27): user-scrollable window again;
|
||||
# the phase-17 bottom-pin (gated in app.js) is the autoscroll.
|
||||
assert "overflow-y: auto" in tbody
|
||||
assert "overflow-y: hidden" not in tbody
|
||||
|
||||
@@ -40,10 +40,15 @@ def _fn_body(js: str, name: str) -> str:
|
||||
|
||||
def test_phase18_gate_is_gone() -> None:
|
||||
"""The follow-the-bottom machinery (phase 18) is removed by owner
|
||||
direction 2026-08-27: no band constant, no gate function, and no
|
||||
document-scroller measurement left anywhere in app.js."""
|
||||
direction 2026-08-27: no page-level band constant, no gate
|
||||
function, and no document-scroller measurement left anywhere in
|
||||
app.js. (The phase-43 window-level THINKING_NEAR_BOTTOM_PX /
|
||||
isThinkingNearBottom pair is a different band — the thinking
|
||||
window's, not the page's.)"""
|
||||
js = _js()
|
||||
assert "NEAR_BOTTOM_PX" not in js, "the 200px band constant must be gone"
|
||||
assert not re.search(r"(?<![A-Z_])NEAR_BOTTOM_PX\b", js), (
|
||||
"the 200px page-level band constant must be gone"
|
||||
)
|
||||
assert "isNearBottom" not in js, "the pinned-to-bottom gate must be gone"
|
||||
assert "window.scrollY" not in js, (
|
||||
"nothing in app.js measures the page scroll offset anymore"
|
||||
|
||||
@@ -1,95 +0,0 @@
|
||||
"""Unit: the "no scroll back" contract for the Thinking window (phase 21,
|
||||
owner choice 2026-08-24, roadmap A2).
|
||||
|
||||
The live Thinking block is a scratchpad, not a transcript: the 320px window
|
||||
always shows the *live tail* of the reasoning stream. The whole functional
|
||||
change is one CSS property — ``details.thinking .thinking-text`` goes from
|
||||
``overflow-y: auto`` (a user-scrollable window) to ``overflow-y: hidden``
|
||||
(a live-tail clip). ``overflow: hidden`` still permits *programmatic*
|
||||
scrolling, so the phase-17 JS bottom-pin
|
||||
(``textEl.scrollTop = textEl.scrollHeight`` on every thinking chunk) is the
|
||||
sole scroller — wheel, drag, and keyboard scrolling stop working.
|
||||
|
||||
The browser behavior itself is E2E-covered (tests/e2e/test_thinking_no_scroll.py);
|
||||
here we pin the CSS value + the owner-choice comment and the intact
|
||||
bottom-pin so a silent regression (``overflow-y`` back to ``auto``, pin
|
||||
removed) is caught without a browser.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
|
||||
APP_JS = FRONTEND / "assets" / "app.js"
|
||||
STYLES_CSS = FRONTEND / "assets" / "styles.css"
|
||||
|
||||
|
||||
def _js() -> str:
|
||||
return APP_JS.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _css() -> str:
|
||||
return STYLES_CSS.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _thinking_text_rule(css: str) -> str:
|
||||
"""Body of the `details.thinking .thinking-text { ... }` rule."""
|
||||
rule = re.search(
|
||||
r"details\.thinking \.thinking-text \{([\s\S]*?)\n\}", css
|
||||
)
|
||||
assert rule, "styles.css must style details.thinking .thinking-text"
|
||||
return rule.group(1)
|
||||
|
||||
|
||||
def test_thinking_text_is_live_tail_clip() -> None:
|
||||
"""The window stays the fixed 320px clip (owner-confirmed: no
|
||||
auto-height growth) but is NO LONGER user-scrollable."""
|
||||
body = _thinking_text_rule(_css())
|
||||
assert "max-height: 320px" in body, "the 320px clip must stay"
|
||||
assert "overflow-y: hidden" in body, "the window must not scroll"
|
||||
assert "overflow-y: auto" not in body, "no user-scrollable window remains"
|
||||
assert "overflow-y: scroll" not in body
|
||||
|
||||
|
||||
def test_thinking_text_carries_owner_choice_comment() -> None:
|
||||
"""The owner-choice comment explains WHY the window is a live tail —
|
||||
the phase-17 JS bottom-pin is the sole scroller."""
|
||||
body = _thinking_text_rule(_css())
|
||||
assert "owner choice 2026-08-24" in body
|
||||
assert "live tail" in body
|
||||
assert "sole scroller" in body
|
||||
|
||||
|
||||
def test_js_bottom_pin_intact_and_sole_scroller() -> None:
|
||||
"""The live-tail mechanism (phase 17) must survive phase 21 untouched:
|
||||
the streaming `thinking` branch pins `textEl.scrollTop =
|
||||
textEl.scrollHeight` per chunk, and it is the ONLY scrollTop
|
||||
assignment in app.js (no new user-facing scroll code was added to the
|
||||
window)."""
|
||||
js = _js()
|
||||
pin = "textEl.scrollTop = textEl.scrollHeight"
|
||||
assert js.count(pin) == 1, "the bottom-pin must exist exactly once"
|
||||
# It lives in the streaming thinking branch (before the delta branch),
|
||||
# inside the `block.open` guard so closed blocks are not scrolled.
|
||||
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]
|
||||
assert pin in thinking_branch
|
||||
assert "if (block.open)" in thinking_branch
|
||||
|
||||
|
||||
def test_no_js_change_to_thinking_scroll_behavior() -> None:
|
||||
"""Phase 21 is CSS-only: nothing else in app.js touches the
|
||||
.thinking-text scroll (no scroll-behavior, no wheel/touch handlers, no
|
||||
scrollIntoView on the block — the page-level reveal stays the
|
||||
phase-18 scrollReveal, which is not a .thinking-text scroller)."""
|
||||
js = _js()
|
||||
block_template = js.find('<div class="thinking-text">')
|
||||
assert block_template != -1, "the thinking block template must exist"
|
||||
# No inline scroll styling on the element itself.
|
||||
assert 'style="scroll' not in js
|
||||
assert "scroll-behavior" not in js
|
||||
assert "addEventListener(\"wheel\"" not in js
|
||||
assert "addEventListener('wheel'" not in js
|
||||
@@ -0,0 +1,139 @@
|
||||
"""Unit: the "thinking scroll back" contract for the Thinking window
|
||||
(phase 43, owner direction 2026-08-27, ``TODO.md`` L7, roadmap A2 —
|
||||
reversing the phase-21 owner choice 2026-08-24).
|
||||
|
||||
The Thinking window is user-scrollable again: ``details.thinking
|
||||
.thinking-text`` goes back to ``overflow-y: auto`` (the 320px clip is
|
||||
kept, owner-confirmed). Wheel, drag, and keyboard move the window; the
|
||||
autoscroll — the phase-17 bottom-pin (``textEl.scrollTop =
|
||||
textEl.scrollHeight`` per thinking chunk) — is GATED: it follows the
|
||||
live tail only while the user is pinned near the window's bottom (the
|
||||
32px band, ``THINKING_NEAR_BOTTOM_PX``). Scrolling up pauses the
|
||||
follow; returning to the bottom re-arms it (the check runs on every
|
||||
chunk, by construction).
|
||||
|
||||
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.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
|
||||
APP_JS = FRONTEND / "assets" / "app.js"
|
||||
STYLES_CSS = FRONTEND / "assets" / "styles.css"
|
||||
|
||||
|
||||
def _js() -> str:
|
||||
return APP_JS.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _css() -> str:
|
||||
return STYLES_CSS.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _thinking_text_rule(css: str) -> str:
|
||||
"""Body of the `details.thinking .thinking-text { ... }` rule."""
|
||||
rule = re.search(
|
||||
r"details\.thinking \.thinking-text \{([\s\S]*?)\n\}", css
|
||||
)
|
||||
assert rule, "styles.css must style details.thinking .thinking-text"
|
||||
return rule.group(1)
|
||||
|
||||
|
||||
def test_thinking_text_is_user_scrollable_window() -> None:
|
||||
"""The window is user-scrollable again (wheel / drag / keyboard
|
||||
move it — frozen-tail state) and keeps the fixed 320px clip."""
|
||||
body = _thinking_text_rule(_css())
|
||||
assert "overflow-y: auto" in body, "the window must be user-scrollable"
|
||||
assert "max-height: 320px" in body, "the 320px clip must stay"
|
||||
assert "overflow-y: hidden" not in body, "no live-tail-only clip remains"
|
||||
assert "overflow-y: scroll" not in body
|
||||
|
||||
|
||||
def test_thinking_text_carries_owner_direction_comment() -> None:
|
||||
"""The owner-direction comment (2026-08-27, ``TODO.md`` L7) explains
|
||||
the new contract: autoscroll follows the live tail only while the
|
||||
user is pinned near the window's bottom — scrolling up pauses the
|
||||
follow, returning to the bottom resumes it."""
|
||||
body = _thinking_text_rule(_css())
|
||||
assert "owner direction 2026-08-27" in body
|
||||
assert "TODO.md L7" in body
|
||||
|
||||
|
||||
def test_thinking_near_bottom_band_exported() -> None:
|
||||
"""The 32px follow-the-tail band is exported (same unit-pinned
|
||||
pattern as TURN_TIMEOUT_MS):
|
||||
``export const THINKING_NEAR_BOTTOM_PX = 32;``."""
|
||||
js = _js()
|
||||
assert "export const THINKING_NEAR_BOTTOM_PX = 32;" in js
|
||||
|
||||
|
||||
def test_is_thinking_near_bottom_band_math() -> None:
|
||||
"""``isThinkingNearBottom(textEl)`` is the window-level band check:
|
||||
the distance from the window's bottom
|
||||
(``scrollHeight - scrollTop - clientHeight``) must be
|
||||
``<= THINKING_NEAR_BOTTOM_PX`` — the "window bottom in view"
|
||||
threshold."""
|
||||
js = _js()
|
||||
fn = re.search(
|
||||
r"function isThinkingNearBottom\(textEl\) \{([\s\S]*?)\n\}", js
|
||||
)
|
||||
assert fn, "app.js must define isThinkingNearBottom(textEl)"
|
||||
body = fn.group(1)
|
||||
assert re.search(
|
||||
r"textEl\.scrollHeight\s*-\s*textEl\.scrollTop\s*-\s*"
|
||||
r"textEl\.clientHeight\s*<=\s*THINKING_NEAR_BOTTOM_PX",
|
||||
body,
|
||||
), "the band math must compare the bottom distance to the band"
|
||||
|
||||
|
||||
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)."""
|
||||
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).
|
||||
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)
|
||||
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
|
||||
# only gate left.
|
||||
assert "if (block.open)" not in js, (
|
||||
"no unconditional `if (block.open)` pin may remain"
|
||||
)
|
||||
|
||||
|
||||
def test_restore_path_renders_collapsed_block() -> None:
|
||||
"""Phase 17 survives: the restore path renders the thinking block
|
||||
COLLAPSED above the bubble (``block.open = false`` in
|
||||
``renderStoredMessage``) — and with the pin gated on
|
||||
``block.open``, a restored closed block is never auto-pinned."""
|
||||
js = _js()
|
||||
start = js.find("function renderStoredMessage(m) {")
|
||||
assert start != -1, "app.js must keep the phase-14 restore renderer"
|
||||
body = js[start : js.find("\n}", start)]
|
||||
assert "ensureThinkingBlock(wrap)" in body
|
||||
assert "block.open = false" in body, "restored blocks must be collapsed"
|
||||
Reference in New Issue
Block a user