All gates green. Verification complete. **Phase 123 — final verification pass (all 4 tasks already in `complete/`)** - Verified the full implementation is in the working tree: `app/api/chat_images.py` (upload/serve pair), `ChatRequest.image`/`ChatMessage.image` (path-validated, omitted-when-None), toggle-off + stale-file hinted error frames, `build_user_content` multimodal build at both sites (chat.py deflected branch + `run_agent`), config-gated composer attach/preview/upload-then-send, restore + shared rendering, CSP `img-src 'self' data:` carve-out, mock-LLM capture buffer. - `uv run pytest` → **2796 passed**, exit 0 (unit + integration). - `uv run pytest --cov=app --cov-report=term-missing` → **TOTAL 99%** (29/4615 missed; phase-123 modules 99–100%). - `uv run pytest tests/e2e/test_chat_image_questions.py -v --no-cov` → **5 passed** in isolation. - `uv run ruff check . && uv run pyright` → clean (0 errors). **Completion criteria:** (1) attach→send→multimodal text+image to the model, bubble/reload/shared all render it, saved chat stores the PATH with `"base64" not in json.dumps(stored)` — **verified** (E2E tests 1–4 + integration round-trip); (2) `BOR_IMAGES=false` — control hidden, exact hinted error frame, zero model calls / no query_log row — **verified** (E2E test 5 + integration); (3) text-only byte-identical (`content` stays a plain `str`) — **verified** (unit + integration); (4) all gates green — **verified**; (5) commit + phase move — left to the harness per pipeline rules (no `git add`/`commit` run). No defects found; no live-infrastructure changes (repo + local dev DB only). **Next pending phase: none** — 123 is the last phase in `todo/`.
226 lines
11 KiB
Python
226 lines
11 KiB
Python
"""Unit: the no-reply-autoscroll contract in the static frontend
|
|
(phase 42, owner direction 2026-08-27, TODO.md L5).
|
|
|
|
The owner removed the phase-18 follow-the-bottom auto-follow: the page
|
|
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). 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 contract — the phase-18 gate is gone (no NEAR_BOTTOM_PX /
|
|
isNearBottom), scrollReveal scrolls unconditionally and is the single
|
|
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
|
|
|
|
import re
|
|
from pathlib import Path
|
|
|
|
FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
|
|
APP_JS = FRONTEND / "assets" / "app.js"
|
|
|
|
|
|
def _js() -> str:
|
|
return APP_JS.read_text(encoding="utf-8")
|
|
|
|
|
|
def _fn_body(js: str, name: str) -> str:
|
|
"""Source of the function starting at `function <name>` (to its closing
|
|
brace at column 0) — same slicing style as test_frontend_feedback.py."""
|
|
fn = js.find(f"function {name}")
|
|
assert fn != -1, f"{name} must exist in app.js"
|
|
return js[fn : js.find("\n}\n", fn)]
|
|
|
|
|
|
def test_phase18_gate_is_gone() -> None:
|
|
"""The follow-the-bottom machinery (phase 18) is removed by owner
|
|
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 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"
|
|
)
|
|
|
|
|
|
def test_scroll_helper_is_unconditional() -> None:
|
|
"""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 "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 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.count("window.scrollTo(") == 1
|
|
assert js.find("window.scrollTo(") > js.find("function scrollReveal")
|
|
|
|
|
|
def test_add_message_takes_explicit_scroll_intent() -> None:
|
|
"""addMessage(who, html, scroll = false, image = null): the phase-18
|
|
scrollBehavior/force parameters are gone; the bubble scrolls only
|
|
when the caller explicitly asks (submit reveal, restore landing).
|
|
Phase 123 (task 02) appended the optional `image` argument (the
|
|
question's attached image — { src, alt } on a user bubble, the ONE
|
|
renderer for live + restore + shared); the scroll contract is
|
|
untouched."""
|
|
js = _js()
|
|
body = _fn_body(js, "addMessage")
|
|
assert "function addMessage(who, html, scroll = false, image = null)" in body
|
|
assert "if (scroll) scrollReveal(wrap)" in body
|
|
assert "force" not in body
|
|
assert "scrollBehavior" not in body
|
|
|
|
|
|
def test_submit_reveals_user_message() -> None:
|
|
"""User intent kept by the owner: submitting scrolls the viewport down
|
|
so the user's own message is visible — the submit addMessage passes
|
|
the scroll intent; the streaming brain-bubble creations in the same
|
|
turn handler never do.
|
|
|
|
Phase 49: the turn handler is runTurn (extracted from handleSend) —
|
|
the user append + persistence save point 1 sit in its `!reask`
|
|
block (the redo-in-place retry path skips both: the question is
|
|
already in the DOM + conversation)."""
|
|
js = _js()
|
|
turn = js.find("async function runTurn")
|
|
assert turn != -1, "runTurn must exist (phase 49 extraction)"
|
|
body = js[turn : js.find("\n}\n", turn)]
|
|
assert "if (!reask) {" in body, "the user append is gated on !reask"
|
|
# Phase 123 (task 02): the user append gained the optional image
|
|
# argument (the attached image's { src, alt }) — the call is
|
|
# multi-line now; the contract is the same: user + the raw text +
|
|
# the explicit scroll intent true.
|
|
assert re.search(
|
|
r'addMessage\(\s*"user",\s*renderMarkdown\(text\),\s*true', body
|
|
), "the submit must reveal the user message (scroll intent true)"
|
|
for call in re.findall(r'addMessage\("brain"([^)]*)\)', body):
|
|
assert "true" not in call, (
|
|
f"streaming brain bubbles must not scroll the page: {call!r}"
|
|
)
|
|
|
|
|
|
def test_streaming_handlers_never_scroll_the_page() -> None:
|
|
"""The heart of the phase-42 contract: the thinking / tool / delta
|
|
branches contain NO page-scroll call (no scrollReveal, no raw
|
|
scrollIntoView). The thinking branch keeps the block-INTERNAL pin
|
|
(textEl.scrollTop — phase 17, reworked in phase 43): that scrolls
|
|
the block's own clip, not the page."""
|
|
js = _js()
|
|
think = js.find('ev.type === "thinking"')
|
|
tool = js.find('ev.type === "tool"')
|
|
delta = js.find('ev.type === "delta"')
|
|
done = js.find('ev.type === "done"')
|
|
assert -1 < think < tool < delta < done, "the turn handler must branch in order"
|
|
for name, branch in (
|
|
("thinking", js[think:tool]),
|
|
("tool", js[tool:delta]),
|
|
("delta", js[delta:done]),
|
|
):
|
|
assert "scrollReveal" not in branch, f"the {name} branch must not scroll the page"
|
|
assert ".scrollIntoView(" not in branch, (
|
|
f"the {name} branch must not scroll the page"
|
|
)
|
|
# The thinking window pin survives (phase 17 — untouched by this phase).
|
|
thinking_branch = js[think:delta]
|
|
assert "textEl.scrollTop = textEl.scrollHeight" in thinking_branch
|
|
assert js.count("textEl.scrollTop = textEl.scrollHeight") == 1
|
|
|
|
|
|
def test_restore_landing_is_one_shot() -> None:
|
|
"""The phase-14 restore landing keeps its one-shot scroll
|
|
(owner-kept): both restore call sites pass the explicit scroll
|
|
intent and they are the only two restore scrolls; with the submit's
|
|
single reveal, exactly three `true` intents exist in the whole file.
|
|
The old forced "auto" landing is gone, and the one-shot, load-time
|
|
contract is documented at the call site."""
|
|
js = _js()
|
|
body = _fn_body(js, "renderStoredMessage")
|
|
assert 'addMessage("user", renderMarkdown(m.text), true)' in body
|
|
assert 'addMessage("brain", renderMarkdown(m.text), true)' in body
|
|
assert js.count('"auto", true') == 0, "the old forced 'auto' landing must be gone"
|
|
# Submit reveal + the two restore landings — nothing else scrolls.
|
|
# Phase 123 (task 02): the submit call is multi-line (the optional
|
|
# image argument follows the scroll intent), so it no longer ends
|
|
# in the single-line ", true)" literal — the two restore calls do;
|
|
# the submit reveal is counted by its own (multi-line) shape.
|
|
assert js.count(", true)") == 2, (
|
|
"only the two restore calls may scroll (single-line shape)"
|
|
)
|
|
assert len(re.findall(r'addMessage\(\s*"user",\s*renderMarkdown\(text\),\s*true', js)) == 1, (
|
|
"the submit reveal may scroll (multi-line since phase 123's image argument)"
|
|
)
|
|
# The marker comment documents the one-shot, load-time contract.
|
|
assert "restore landing" in body
|
|
assert "one-shot" in body
|
|
|
|
|
|
def test_typing_bubble_does_not_scroll() -> None:
|
|
"""A typing indicator appearing must not yank the page — the phase-18
|
|
scrollReveal call in addTyping is removed with the gate."""
|
|
js = _js()
|
|
body = _fn_body(js, "addTyping")
|
|
assert "scrollReveal" not in body
|
|
assert ".scrollIntoView(" not in body
|
|
|
|
|
|
def test_scroll_constant_reduced_motion_intact() -> None:
|
|
"""The SCROLL constant is untouched (calm, don't remove): smooth by
|
|
default, "auto" under prefers-reduced-motion — the two kept scroll
|
|
call sites ride it as the default behavior."""
|
|
js = _js()
|
|
assert 'const SCROLL = reducedMotion ? "auto" : "smooth";' in js
|
|
assert 'matchMedia("(prefers-reduced-motion: reduce)")' in js
|
|
assert "Calm, don't remove" in js
|
|
|
|
|
|
def test_turn_end_focus_does_not_scroll() -> None:
|
|
"""The turn-end focus-back (phase 06's "always focus back") must not
|
|
move the viewport: focusing the composer while the user is scrolled
|
|
up would yank them to the bottom at the moment the turn ends.
|
|
preventScroll keeps the keyboard flow without the scroll."""
|
|
js = _js()
|
|
# Phase 48: the settle line gained the user-stop terminal (stop →
|
|
# idle, no banner) — the focus-back contract is unchanged.
|
|
finally_idx = js.find("// done | error | stop → idle: always settle, always focus back")
|
|
assert finally_idx != -1, "the turn's finally block must exist"
|
|
block = js[finally_idx : js.find("\n}", finally_idx)]
|
|
assert 'input.focus({ preventScroll: true })' in block
|
|
assert "input.focus()" not in block
|