phase: 143_mobile_keyboard_after_answer
**Phase 143 final verification pass — 100% green, no code changes needed (all work verified in-tree)**
- **Verified implementation:** `COARSE_POINTER` constant (L395, L391-twin `typeof`-guarded shape) + gated finally-block focus `if (!COARSE_POINTER) input.focus({ preventScroll: true })` in `frontend/assets/app.js`; SETTLE lines/anchor untouched; extended unit pin in `test_frontend_scroll.py`; dedicated two-context E2E `tests/e2e/test_mobile_keyboard_focus.py`
- **C1 desktop byte-identical refocus:** ✅ unit pin passes (gated shape keeps `preventScroll`); desktop E2E asserts composer active; `test_loading_feedback.py` 5 passed (phase-06 regression)
- **C2 mobile keyboard stays down:** ✅ `test_mobile_turn_end_keeps_keyboard_down` passes — `pointer: coarse` sanity assert first, composer NOT active after done
- **C3 gates:** ✅ `uv run pytest --cov=app --cov-report=term-missing` → 3359 passed, `app/` TOTAL 99% (>90%); `uv run pytest tests/e2e/test_mobile_keyboard_focus.py -v --no-cov` → 2 passed in isolation; `uv run ruff check .` clean; `uv run pyright` 0 errors
- **C4 no regression:** ✅ full suite green (all frontend source pins: char counter, send flow, never-scroll); `test_stop_generation.py` 3 passed (single settle)
- **Docs:** README.md fully reviewed — all env vars cross-checked against `app/config.py` defaults, routes/endpoints/tools verified in code; AGENTS.md rules + quick reference verified against repo. Consistent — no changes needed
- **No live-infrastructure changes** (dev Postgres container already running)
- **Next pending phase:** `144_modal_back_button`
This commit is contained in:
@@ -0,0 +1,248 @@
|
||||
"""Phase 143 E2E (Playwright): the mobile keyboard stays DOWN after an
|
||||
answer completes — one file, two device contexts.
|
||||
|
||||
Owner complaint (TODO.md L5): "mobile keyboard pops back up after answer
|
||||
is complete and that's annoying." The fix (task 01) gates the turn's
|
||||
single settle focus-back in ``frontend/assets/app.js`` on
|
||||
``matchMedia("(pointer: coarse)")``: fine pointers (desktops) keep the
|
||||
phase-06 "always focus back" courtesy — the follow-up question is typed
|
||||
without moving a hand — while coarse pointers (phones/tablets) stay
|
||||
unfocused so the virtual keyboard does not pop back over the completed
|
||||
answer; the composer is a ≥44px tap target (PLAN §7.1), so the next
|
||||
question starts with a tap.
|
||||
|
||||
This file is the browser-visible proof on BOTH devices, in ONE variable:
|
||||
|
||||
* ``test_desktop_turn_end_refocuses_composer`` — the DEFAULT Chromium
|
||||
context (1280×800, no touch — ``pointer: fine``): a completed turn
|
||||
leaves ``document.activeElement`` on the composer's textarea
|
||||
(``#message-input``) — the phase-06/42 contract (refocus,
|
||||
``preventScroll``) preserved on desktop.
|
||||
* ``test_mobile_turn_end_keeps_keyboard_down`` — a mobile-emulated
|
||||
context (360×800, ``isMobile`` + ``hasTouch`` — the ≤640px CSS
|
||||
breakpoint applies AND Chromium's touch emulation flips the media
|
||||
feature to ``pointer: coarse``): FIRST the sanity assert that
|
||||
``matchMedia("(pointer: coarse)").matches`` IS true (if a future
|
||||
Playwright/Chromium change breaks the emulation, this line fails
|
||||
loudly and names why, instead of the focus assertion failing
|
||||
mysteriously), THEN a completed turn leaves the composer UNFOCUSED —
|
||||
the keyboard stays down.
|
||||
|
||||
Both contexts run against the SAME fixture KB (the per-module
|
||||
truncate/re-import, the house E2E convention — suites run in isolation)
|
||||
and the SAME on-topic question the chat story suites use; the mock LLM
|
||||
answers immediately, so the done state settles fast.
|
||||
|
||||
Run in isolation (mock LLM; DB up: ``podman compose up -d db``):
|
||||
|
||||
uv run pytest tests/e2e/test_mobile_keyboard_focus.py -v --no-cov
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import os
|
||||
import threading
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from playwright.sync_api import Browser, BrowserContext, Page, ViewportSize, expect
|
||||
from sqlalchemy import text
|
||||
|
||||
from app.config import Settings
|
||||
from app.db import SessionLocal
|
||||
from app.rag.importer import ImportSummary, import_sources
|
||||
from app.rag.llm import LLMClient
|
||||
from e2e.auth_helpers import login
|
||||
|
||||
REPO = Path(__file__).resolve().parents[2]
|
||||
FIXTURES = REPO / "tests" / "fixtures" / "docs"
|
||||
MOCK_PORT = int(os.environ.get("E2E_MOCK_PORT", "8901"))
|
||||
|
||||
# The same on-topic question the chat story suites (phase 06 loading
|
||||
# feedback) use — the fixture KB grounds it and the mock answers
|
||||
# immediately (no warm-up marker), so both contexts settle fast.
|
||||
ON_TOPIC = "How is my Kubernetes cluster set up?"
|
||||
|
||||
# The composer's textarea — the element app.js's turn `finally` block
|
||||
# focuses back on fine pointers (task 01's `#message-input` pin), and
|
||||
# the `login` helper already types questions into.
|
||||
COMPOSER = "#message-input"
|
||||
# The typing indicator is itself a .msg.brain — exclude its bubble.
|
||||
ANSWER = ".msg.brain .bubble:not(.typing)"
|
||||
|
||||
# The mobile context: a 360×800 phone (the ≤640px CSS breakpoint
|
||||
# applies — 360 is the phase-88 suite's narrow owner-device edge) with
|
||||
# isMobile + hasTouch — Chromium's CDP touch emulation is what makes
|
||||
# ``matchMedia("(pointer: coarse)")`` match (the test's sanity assert
|
||||
# proves it is doing its job).
|
||||
MOBILE_VIEWPORT: ViewportSize = {"width": 360, "height": 800}
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# KB seeding (same pattern as the phase 06 story suite — truncate +
|
||||
# re-import the fixture docs per test, so the file runs in isolation)
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
async def _import_fixtures(mock_port: int) -> ImportSummary:
|
||||
kwargs: dict[str, Any] = {"_env_file": None, "llm_base_url": f"http://127.0.0.1:{mock_port}/v1"}
|
||||
settings = Settings(**kwargs) # pyright: ignore[reportCallIssue]
|
||||
return await import_sources([FIXTURES], LLMClient(settings))
|
||||
|
||||
|
||||
def _run_in_thread(coro: Any) -> Any:
|
||||
"""Run a coroutine on a worker thread.
|
||||
|
||||
Playwright's sync API keeps an asyncio loop running on the test thread,
|
||||
so ``asyncio.run`` cannot be called directly from a test body.
|
||||
"""
|
||||
box: dict[str, Any] = {}
|
||||
|
||||
def runner() -> None:
|
||||
try:
|
||||
box["value"] = asyncio.run(coro)
|
||||
except BaseException as e: # noqa: BLE001 — re-raised on the test thread
|
||||
box["error"] = e
|
||||
|
||||
t = threading.Thread(target=runner)
|
||||
t.start()
|
||||
t.join()
|
||||
if "error" in box:
|
||||
raise box["error"]
|
||||
return box["value"]
|
||||
|
||||
|
||||
def _reset_db(mock_port: int, seed: bool) -> ImportSummary | None:
|
||||
"""Truncate the KB (and query log), then optionally re-import fixtures."""
|
||||
with SessionLocal() as db:
|
||||
db.execute(text("TRUNCATE chunks, documents, query_log"))
|
||||
db.commit()
|
||||
if not seed:
|
||||
return None
|
||||
return _run_in_thread(_import_fixtures(mock_port))
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# The shared turn driver + device contexts
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def turn_to_completion(page: Page) -> None:
|
||||
"""Ask the on-topic question and wait for the DONE state.
|
||||
|
||||
The same settle wait the chat story suites (phase 06) use: the
|
||||
answer bubble has streamed in AND the send button is back to its
|
||||
idle contract (enabled, "Send" label — phase 48). By the time the
|
||||
label reads "Send", the turn's ``finally`` block — the single
|
||||
settle, including the phase-143 gated focus-back — has already run
|
||||
(the ``setUiState(idle)`` that flips the label is the line just
|
||||
above the focus call).
|
||||
"""
|
||||
page.fill(COMPOSER, ON_TOPIC)
|
||||
page.click("#send-btn")
|
||||
|
||||
page.locator(ANSWER).wait_for(state="visible", timeout=30_000)
|
||||
expect(page.locator("#send-label")).to_have_text("Send", timeout=30_000)
|
||||
expect(page.locator("#send-btn")).to_be_enabled()
|
||||
|
||||
|
||||
def composer_is_active(page: Page) -> bool:
|
||||
"""``document.activeElement === #message-input`` (compared IN the
|
||||
page — Element handles are not identity-comparable in Python)."""
|
||||
return page.evaluate(
|
||||
"() => document.activeElement === document.querySelector('#message-input')"
|
||||
)
|
||||
|
||||
|
||||
def _mobile_page(browser: Browser) -> tuple[BrowserContext, Page]:
|
||||
"""A fresh 360×800 MOBILE page (``isMobile`` + ``hasTouch``): the
|
||||
context whose touch emulation makes Chromium report
|
||||
``pointer: coarse`` (the conftest ``page`` fixture is a 1280×800
|
||||
fine-pointer page — the desktop control). The caller closes page +
|
||||
context in ``finally``."""
|
||||
context = browser.new_context(
|
||||
is_mobile=True, has_touch=True, viewport=MOBILE_VIEWPORT
|
||||
)
|
||||
return context, context.new_page()
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# 1. Desktop (fine pointer): the preserved "always focus back" contract
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_desktop_turn_end_refocuses_composer(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
"""The regression guard (phase 06): in the DEFAULT Chromium context
|
||||
— no touch, ``pointer: fine`` — a completed turn (the done state:
|
||||
answer streamed, send button back to the idle "Send" contract)
|
||||
leaves the composer FOCUSED — ``document.activeElement`` IS
|
||||
``#message-input`` — the follow-up question is typed immediately.
|
||||
The never-scroll ``preventScroll`` argument itself is source-pinned
|
||||
by the unit pin (tests/unit/test_frontend_scroll.py) and is
|
||||
unobservable from the focus assert (it suppresses viewport motion,
|
||||
not focus)."""
|
||||
summary = _reset_db(mock_llm, seed=True)
|
||||
assert summary is not None and summary.added == 13 # A9 formats (phase 47 added quadlet+j2)
|
||||
page.set_default_timeout(30_000)
|
||||
login(page, app_url, next="/")
|
||||
|
||||
# Control sanity: this context really is a FINE pointer (the pair
|
||||
# of contexts differs in exactly one variable — the device).
|
||||
assert not page.evaluate("matchMedia('(pointer: coarse)').matches"), (
|
||||
"the desktop control context must report pointer: fine — the "
|
||||
"conftest page is a no-touch 1280x800 Chromium context"
|
||||
)
|
||||
|
||||
turn_to_completion(page)
|
||||
assert composer_is_active(page), (
|
||||
"desktop (fine pointer): after a turn ends, the composer must be "
|
||||
"the active element again — the phase-06 'always focus back' "
|
||||
"contract is desktop-only, never removed"
|
||||
)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# 2. Mobile (coarse pointer): the keyboard stays down
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_mobile_turn_end_keeps_keyboard_down(
|
||||
browser: Browser, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
"""The owner's fix, browser-visible: in a 360×800 mobile context
|
||||
(``isMobile`` + ``hasTouch`` → ``pointer: coarse``), a completed
|
||||
turn leaves the composer UNFOCUSED — ``document.activeElement`` is
|
||||
NOT ``#message-input`` — so the virtual keyboard stays DOWN over
|
||||
the completed answer (the composer's ≥44px tap target, PLAN §7.1,
|
||||
starts the next question). The first assert proves the emulation
|
||||
is doing its job: if a future Playwright/Chromium change stops
|
||||
flipping the media feature, THIS line fails loudly and names why,
|
||||
instead of the focus assert failing mysteriously."""
|
||||
summary = _reset_db(mock_llm, seed=True)
|
||||
assert summary is not None and summary.added == 13
|
||||
context, page = _mobile_page(browser)
|
||||
try:
|
||||
page.set_default_timeout(30_000)
|
||||
|
||||
# Emulation sanity FIRST (see the docstring): the touch
|
||||
# emulation must have flipped the media feature.
|
||||
assert page.evaluate("matchMedia('(pointer: coarse)').matches"), (
|
||||
"the mobile context (isMobile + hasTouch, 360x800) must "
|
||||
"report pointer: coarse — Chromium's touch emulation is not "
|
||||
"flipping the media feature; the focus assert below would "
|
||||
"be meaningless (fail this line, not the focus one)"
|
||||
)
|
||||
|
||||
login(page, app_url, next="/")
|
||||
turn_to_completion(page)
|
||||
assert not composer_is_active(page), (
|
||||
"mobile (coarse pointer): after a turn ends, the composer "
|
||||
"must NOT be re-focused — the focus-back is a desktop "
|
||||
"courtesy, and re-focusing pops the virtual keyboard over "
|
||||
"the completed answer (the owner's complaint, TODO L5)"
|
||||
)
|
||||
finally:
|
||||
page.close()
|
||||
context.close()
|
||||
@@ -214,12 +214,39 @@ 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."""
|
||||
preventScroll keeps the keyboard flow without the scroll.
|
||||
|
||||
Phase 143 (TODO L5): the focus-back is now GATED on pointer type —
|
||||
`if (!COARSE_POINTER) input.focus({ preventScroll: true })`. On a
|
||||
coarse pointer (phone/tablet) focusing the composer pops the
|
||||
virtual keyboard over the completed answer, so the composer stays
|
||||
unfocused there (the ≥44px tap target starts the next question).
|
||||
This pin's job: the focus-back exists, is preventScroll (the
|
||||
phase-06/42 contract, byte-preserved on desktop), and is
|
||||
coarse-gated — a bare ungated `input.focus(...)` line in the
|
||||
finally block is a failure, as a missing or mistyped
|
||||
`COARSE_POINTER` constant is."""
|
||||
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
|
||||
# Phase 143: the gated shape — the focus call exists, keeps
|
||||
# preventScroll, and is wrapped in the coarse-pointer gate.
|
||||
assert "if (!COARSE_POINTER) input.focus({ preventScroll: true })" in block
|
||||
# A bare, ungated focus line (the pre-phase-143 shape) is gone:
|
||||
# every focus call in the block rides the gate.
|
||||
focus_lines = [ln for ln in block.splitlines() if "input.focus(" in ln]
|
||||
assert focus_lines, "the turn-end focus-back must still exist"
|
||||
assert all("if (!COARSE_POINTER)" in ln for ln in focus_lines), (
|
||||
"the turn-end focus-back must be coarse-pointer gated (phase 143)"
|
||||
)
|
||||
assert "input.focus()" not in block
|
||||
# The L391-twin constant: the same typeof-guarded matchMedia shape
|
||||
# as reducedMotion, bound to COARSE_POINTER.
|
||||
assert re.search(
|
||||
r'const COARSE_POINTER\s*=\s*typeof matchMedia === "function"'
|
||||
r' && matchMedia\("\(pointer: coarse\)"\)\.matches;',
|
||||
js,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user