phase: 86_history_page_width

All criteria verified. Final report: **Phase 86 final verification pass — all green, no defects found.** - **Verified (previously implemented):** `position: relative` on shared `.table-wrap` (the one-rule A3 fix, +12 lines in `frontend/assets/styles.css`); unit pin `tests/unit/test_table_wrap_stacking.py` (3 tests); E2E story `tests/e2e/test_history_page_width.py` (5 tests: History 626→≤375, Tokens 618→≤375, SPA switch with window-sentinel, RAG regression, desktop 1280×800) - **Tests/lint (this pass):** `uv run pytest` → 1720 passed · `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99%** (>90% floor) · E2E in isolation: `test_history_page_width.py` 5 passed, `test_chat_history.py` 5 passed, `test_api_tokens.py` 9 passed, `test_smoke.py` 3 passed · `uv run ruff check . && uv run pyright` → clean, 0 errors - **Completion criteria:** ① 375px direct+SPA `scrollWidth ≤ innerWidth` on History/Tokens — PASS (E2E pins) ② in-card scroll preserved + full-width + a11y spans in DOM — PASS ③ Sources + desktop regression — PASS ④ full suite/coverage/E2E/lint gates — PASS ⑤ `git diff --stat` limited to `styles.css` (+12) + new test files + phase files, no `app/`/markup/JS — PASS ⑥ commit/move — left to harness per executor rules (working tree intact, `todo/` dir removal already reflected) - **Notable:** red→green (CSS reverted → 626px failure) and manual live check already recorded in `.agents/reports/86_history_page_width/`; pre-existing untracked `.agents/remediation_plan.md` (Sep 7 security audit) untouched - **Next pending phase:** `87_big_read_progress`
2026-09-08 02:38:45 -04:00
parent 412a560348
commit 0f6b9ff7e6
19 changed files with 951 additions and 0 deletions
@@ -0,0 +1,427 @@
+"""Phase 86 E2E (Playwright): the table pages are viewport-width — the
+owner's "the page is the width of the table" bug, pinned.
+
+Source: ``TODO.md`` L4 (owner bug report, 2026-09-07): "The history page
+appears to be the width of the table despite the table being scrollable.
+On mobile this results in half the page being blank and awkwardly
+scrollable."
+Story: n/a (owner bug report — ``TODO.md`` L4 is the story; no
+``.agents/user_stories/`` file).
+
+Run in isolation (DB must be up: ``podman compose up -d db``):
+
+    uv run pytest tests/e2e/test_history_page_width.py -v --no-cov
+
+The bug (confirmed by live reproduction, 375×812, signed-in admin):
+the History and Tokens tables carry ``position: absolute``
+``.visually-hidden`` elements (the ``<caption>`` + the Actions column
+header span). No ancestor in the chain was positioned, so their 1px
+boxes were laid out against the INITIAL containing block — the Actions
+span at the 640px table's right edge — and leaked into the DOCUMENT's
+scrollable overflow, bypassing the card's own ``overflow-x: auto``
+scrolling. Measured ``documentElement.scrollWidth`` at 375px:
+``/history.html`` **626**, ``/tokens.html`` **618** (the byte-identical
+defect, folded in per owner decision A3), ``/sources.html`` **375**
+(clean — the RAG table's headers are visible text). The fix is ONE CSS
+property (phase 86, task 01): ``position: relative`` on the shared
+``.table-wrap`` card, which becomes the containing block for those
+spans. This suite pins the FIXED contract: the document never pans
+past the viewport, the 640px tables still scroll INSIDE their cards
+(the phase-07 / AGENTS.md-rule-5 full-width contract), the a11y spans
+stay in the DOM, and the RAG view + the desktop layout are unchanged.
+
+Counterfactual (verified red→green in the phase session): with the
+task-01 rule reverted (no ``position: relative`` on ``.table-wrap``),
+``test_history_page_is_viewport_width`` fails at its
+``scrollWidth <= innerWidth`` pin (626 > 375); restored, it is green.
+
+Test → contract mapping (Playwright Mapping Rule):
+
+1. ``test_history_page_is_viewport_width`` — THE TODO.md L4 pin: admin
+   at 375×812, direct ``/history.html`` —
+   ``documentElement.scrollWidth <= innerWidth`` (626 → ≤375), the
+   table still scrolls INSIDE ``#history-table-wrap``
+   (``scrollWidth > clientWidth`` — the 640px ``min-width``), the card
+   is the container's content width (≤ the viewport), and the
+   visually-hidden Actions header + caption survived the fix (the a11y
+   names are intact).
+2. ``test_tokens_page_is_viewport_width`` — the folded-in identical
+   defect (618 pre-fix): the same two pins on direct ``/tokens.html``
+   for ``#tokens-table-wrap``.
+3. ``test_spa_switch_keeps_page_width`` — the invariant survives the
+   router, not just direct loads: at 375px on ``/history.html`` the
+   mobile menu (a REAL ``#nav-toggle`` click — the phase-85 contract)
+   opens, the Tokens link switches the view IN THE SAME DOCUMENT
+   (the phase-76 window-sentinel proof — a real navigation would wipe
+   ``window`` globals), and the Tokens page is still viewport-width
+   with its table still scrolling in-card.
+4. ``test_rag_view_regression`` — ``/sources.html`` at 375px: no
+   overflow (it was already clean at 375 pre-fix — pinned so the
+   shared-rule change cannot regress it) AND the RAG table is still
+   full-width inside its card (in-card scroll — the 640px
+   ``min-width`` still engages).
+5. ``test_desktop_unchanged`` — 1280×800 regression on ALL THREE
+   views: no document overflow at any width, and the tables render at
+   container width (the cards need no internal scroll —
+   ``scrollWidth <= clientWidth + 1``, the card is far wider than the
+   mobile 346px).
+
+House conventions: a fresh 375×812 page per mobile test via the
+session ``browser`` fixture (the conftest ``page`` is 1280×800),
+``e2e.auth_helpers.login`` for the real form login, ``_wait_settled_admin``
+copied from ``test_mobile_hamburger_nav.py`` (suites stay
+self-contained — helpers copied, only ``auth_helpers`` imported).
+Data: the History/Tokens contract holds with EMPTY tables (static
+thead + the 640px ``min-width`` — no rows needed, and the suite never
+depends on saved chats or issued tokens). The RAG view, by contrast,
+HIDES its table and shows the "Nothing indexed yet" empty state on an
+empty KB, so tests 4 and 5 seed the KB the house way (truncate +
+import the 13 fixture docs, deterministic mock embeddings) and pin
+nothing about the rows themselves — only that the table is up and
+full-width.
+"""
+from __future__ import annotations
+
+import asyncio
+from pathlib import Path
+from threading import Thread
+from typing import Any
+
+from playwright.sync_api import Browser, JSHandle, 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"
+
+MOBILE: ViewportSize = {"width": 375, "height": 812}  # the bug-report phone viewport
+DESKTOP: ViewportSize = {"width": 1280, "height": 800}  # the conftest page size
+
+
+async def _import_fixtures(mock_port: int) -> ImportSummary:
+    """The house KB seed (copied from ``test_api_tokens.py``): import
+    the fixture docs against the mock LLM's embeddings."""
+    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 = Thread(target=runner)
+    t.start()
+    t.join()
+    if "error" in box:
+        raise box["error"]
+    return box["value"]
+
+
+def _seed_kb(mock_port: int) -> ImportSummary:
+    """House reset (truncate the KB + the prompt-shaping tables so the
+    mock answers stay deterministic) + the fixture re-import — the
+    RAG view hides its table on an empty KB, so the two tests that
+    pin it need at least the fixture docs present. ``saved_chats`` and
+    ``api_tokens`` are deliberately NOT touched (the house pattern)."""
+    with SessionLocal() as db:
+        db.execute(text("TRUNCATE chunks, documents, query_log, steering_notes"))
+        db.commit()
+    return _run_in_thread(_import_fixtures(mock_port))
+
+
+def _mobile_page(browser: Browser) -> Page:
+    """A fresh 375×812 page (the conftest ``page`` is 1280×800)."""
+    return browser.new_page(viewport=MOBILE)
+
+
+def _wait_settled_admin(page: Page) -> None:
+    """Wait until whoami has resolved for the admin: the whoami reveal
+    has un-hidden the admin-only nav links — the viewport-independent
+    settled signal (copied from ``test_mobile_hamburger_nav.py``; the
+    sign-out control is viewport-dependent and not a cross-viewport
+    probe)."""
+    page.wait_for_function(
+        "() => !document.querySelector('#nav-sources').hasAttribute('hidden')",
+        timeout=10_000,
+    )
+
+
+def _open_menu(page: Page) -> None:
+    """Open the mobile menu with a REAL click on the toggle (the
+    phase-85 contract: the gate sits below the header, so a real tap
+    reaches the toggle; for a signed-in admin the gate is hidden
+    anyway)."""
+    page.click("#nav-toggle")
+    expect(page.locator("#nav-toggle")).to_have_attribute("aria-expanded", "true")
+
+
+def _wrap_handle(page: Page, view: str) -> JSHandle:
+    """The table card (``.table-wrap``) of ``view``: the id'd cards for
+    history/tokens, the RAG card found through its ``.docs-table``
+    (that card is the only one without an id)."""
+    if view == "sources":
+        return page.evaluate_handle(
+            "() => document.querySelector('.docs-table').closest('.table-wrap')"
+        )
+    return page.evaluate_handle(f"() => document.querySelector('#{view}-table-wrap')")
+
+
+def _assert_viewport_width(page: Page, label: str) -> None:
+    """THE pin (TODO.md L4): the document itself never pans past the
+    viewport — pre-fix, /history.html measured 626 and /tokens.html
+    618 at this 375px viewport."""
+    report = page.evaluate(
+        "() => ({ doc: document.documentElement.scrollWidth, inner: window.innerWidth })"
+    )
+    assert report["doc"] <= report["inner"], (
+        f"{label}: the document panned to {report['doc']}px at a "
+        f"{report['inner']}px viewport — the TODO.md L4 defect "
+        '(the page is "the width of the table"); the .table-wrap card '
+        "must be the containing block for the positioned "
+        ".visually-hidden spans (position: relative, phase 86)"
+    )
+
+
+def _assert_in_card_scroll(page: Page, wrap: JSHandle, label: str) -> None:
+    """The phase-07 / AGENTS.md-rule-5 contract the fix preserves: the
+    640px table scrolls INSIDE the card (``overflow-x: auto``), the
+    card is the container's content width (≤ the viewport), and the
+    table's ``min-width: 640px`` is what the in-card scroll is over."""
+    report = wrap.evaluate(
+        """el => {
+            const c = el.closest('.container');
+            const cs = getComputedStyle(c);
+            const ws = getComputedStyle(el);
+            return {
+                scroll: el.scrollWidth,
+                client: el.clientWidth,
+                // The container's content width minus the card's own
+                // borders (clientWidth excludes them) — the card fills
+                // the row edge to edge.
+                content: c.clientWidth - parseFloat(cs.paddingLeft) - parseFloat(cs.paddingRight)
+                          - parseFloat(ws.borderLeftWidth) - parseFloat(ws.borderRightWidth),
+                inner: window.innerWidth,
+                table: el.querySelector('table').scrollWidth,
+            };
+        }"""
+    )
+    assert report["table"] >= 640, (
+        f"{label}: the table must keep its 640px min-width, got {report['table']}px"
+    )
+    assert report["scroll"] > report["client"], (
+        f"{label}: the 640px table must still scroll INSIDE the card "
+        f"(card {report['client']}px < table {report['table']}px)"
+    )
+    assert report["client"] <= report["inner"], (
+        f"{label}: the card must not exceed the viewport "
+        f"({report['client']}px > {report['inner']}px)"
+    )
+    assert abs(report["client"] - report["content"]) <= 1, (
+        f"{label}: the card must fill the container's content width "
+        f"(card {report['client']}px vs container {report['content']}px)"
+    )
+
+
+def _assert_hidden_a11y_spans(page: Page, wrap_sel: str) -> None:
+    """The a11y names survived the fix: the visually-hidden Actions
+    column header and the table caption are still in the DOM (the fix
+    repositioned their CONTAINING BLOCK — it did not remove them)."""
+    expect(page.locator(f"{wrap_sel} th .visually-hidden")).to_have_count(1)
+    expect(page.locator(f"{wrap_sel} th .visually-hidden")).to_have_text("Actions")
+    expect(page.locator(f"{wrap_sel} caption.visually-hidden")).to_have_count(1)
+
+
+# ---------------------------------------------------------------------------
+# 1. THE TODO.md L4 pin: /history.html is viewport-width at 375px
+# ---------------------------------------------------------------------------
+
+
+def test_history_page_is_viewport_width(
+    browser: Browser, app_url: str, db_ready: None
+) -> None:
+    """THE TODO.md L4 pin: admin at 375×812, direct ``/history.html`` —
+    the document is viewport-width (pre-fix ``scrollWidth`` 626 — the
+    page panned ~250px into a blank region, "half the page being
+    blank"), the 640px table still scrolls INSIDE
+    ``#history-table-wrap`` (the phase-07 / AGENTS.md-rule-5
+    full-width contract), and the visually-hidden Actions header +
+    caption are still in the DOM (the a11y names survived the fix)."""
+    page = _mobile_page(browser)
+    try:
+        login(page, app_url, next="/history.html")
+        _wait_settled_admin(page)
+        expect(page.locator("#history-table-wrap")).to_be_visible(timeout=15_000)
+
+        _assert_viewport_width(page, "/history.html (direct)")
+        _assert_in_card_scroll(page, _wrap_handle(page, "history"), "/history.html")
+        _assert_hidden_a11y_spans(page, "#history-table-wrap")
+    finally:
+        page.close()
+
+
+# ---------------------------------------------------------------------------
+# 2. The folded-in identical defect: /tokens.html (618 pre-fix)
+# ---------------------------------------------------------------------------
+
+
+def test_tokens_page_is_viewport_width(
+    browser: Browser, app_url: str, db_ready: None
+) -> None:
+    """The owner-confirmed A3 fold-in: the Tokens view carries the
+    byte-identical defect (measured ``scrollWidth`` 618 pre-fix — the
+    same visually-hidden Actions header + caption, no positioned
+    ancestor) and is fixed by the SAME one-rule ``.table-wrap`` fix.
+    Same two pins as test 1, on direct ``/tokens.html`` for
+    ``#tokens-table-wrap``."""
+    page = _mobile_page(browser)
+    try:
+        login(page, app_url, next="/tokens.html")
+        _wait_settled_admin(page)
+        expect(page.locator("#tokens-table-wrap")).to_be_visible(timeout=15_000)
+
+        _assert_viewport_width(page, "/tokens.html (direct)")
+        _assert_in_card_scroll(page, _wrap_handle(page, "tokens"), "/tokens.html")
+        _assert_hidden_a11y_spans(page, "#tokens-table-wrap")
+    finally:
+        page.close()
+
+
+# ---------------------------------------------------------------------------
+# 3. The invariant survives the router (not just direct loads)
+# ---------------------------------------------------------------------------
+
+
+def test_spa_switch_keeps_page_width(
+    browser: Browser, app_url: str, db_ready: None
+) -> None:
+    """At 375px on ``/history.html``: open the mobile menu (a REAL
+    ``#nav-toggle`` click — the phase-85 contract; at this width the
+    nav links live in the dropdown), click the Tokens link — a
+    CLIENT-SIDE view switch in the phase-76 shell, so the same-document
+    proof is the phase-76 canonical one: the window sentinel set
+    before the click is still readable after it (a real document load
+    would wipe ``window`` globals; the navigation-timeline count is
+    deliberately NOT used — a real load resets it, so it cannot
+    distinguish pushState from a reload). On arrival the Tokens page
+    keeps BOTH pins: viewport-width AND the table still scrolling
+    inside its card."""
+    page = _mobile_page(browser)
+    try:
+        login(page, app_url, next="/history.html")
+        _wait_settled_admin(page)
+        expect(page.locator("#history-table-wrap")).to_be_visible(timeout=15_000)
+        _assert_viewport_width(page, "/history.html (pre-switch)")
+
+        # THE SWITCH (mobile menu → Tokens link), same document:
+        page.evaluate("() => { window.__phase86_width = 'phase86'; }")
+        _open_menu(page)
+        page.click("#nav-tokens")
+        expect(page).to_have_url(app_url + "/tokens.html", timeout=15_000)
+        assert page.evaluate("() => window.__phase86_width") == "phase86", (
+            "a real document load would have wiped the window sentinel — "
+            "the view switch must be same-document (the phase-76 router)"
+        )
+
+        expect(page.locator("#tokens-table-wrap")).to_be_visible(timeout=15_000)
+        _assert_viewport_width(page, "/tokens.html (SPA switch)")
+        _assert_in_card_scroll(page, _wrap_handle(page, "tokens"), "/tokens.html (SPA)")
+    finally:
+        page.close()
+
+
+# ---------------------------------------------------------------------------
+# 4. Regression: the RAG view (already clean) stays clean + full-width
+# ---------------------------------------------------------------------------
+
+
+def test_rag_view_regression(
+    browser: Browser, app_url: str, mock_llm: int, db_ready: None
+) -> None:
+    """``/sources.html`` at 375px (admin): the RAG view measured clean
+    pre-fix (``scrollWidth`` 375 — the ``.docs-table`` headers are
+    visible text, no positioned hidden spans at the table's right
+    edge). Pinned so the shared ``.table-wrap`` rule change cannot
+    regress it — AND its table is still full-width inside the card:
+    the 640px ``min-width`` still engages, so the card keeps its
+    in-card scroll (the shared rule added a containing block, not a
+    width)."""
+    summary = _seed_kb(mock_llm)
+    assert summary is not None and summary.added == 13  # A9 formats (phase 47 added quadlet+j2)
+    page = _mobile_page(browser)
+    try:
+        login(page, app_url, next="/sources.html")
+        _wait_settled_admin(page)
+        expect(page.locator(".docs-table")).to_be_visible(timeout=15_000)
+
+        _assert_viewport_width(page, "/sources.html (direct)")
+        _assert_in_card_scroll(page, _wrap_handle(page, "sources"), "/sources.html")
+    finally:
+        page.close()
+
+
+# ---------------------------------------------------------------------------
+# 5. Desktop regression: no overflow anywhere, tables at container width
+# ---------------------------------------------------------------------------
+
+
+def test_desktop_unchanged(
+    browser: Browser, app_url: str, mock_llm: int, db_ready: None
+) -> None:
+    """1280×800 regression on ALL THREE table views: no document
+    overflow at any width, and each table renders at container width —
+    the cards need no internal scroll
+    (``wrap.scrollWidth <= wrap.clientWidth + 1``) and are far wider
+    than the mobile 346px card. The zero-offset
+    ``position: relative`` changed no layout, so desktop is
+    byte-identical in behavior. The KB is seeded the house way for
+    the RAG view's table (it hides itself on an empty KB)."""
+    summary = _seed_kb(mock_llm)
+    assert summary is not None and summary.added == 13
+    page = browser.new_page(viewport=DESKTOP)
+    try:
+        login(page, app_url, next="/history.html")
+        _wait_settled_admin(page)
+
+        views = (
+            ("history", "/history.html"),
+            ("tokens", "/tokens.html"),
+            ("sources", "/sources.html"),
+        )
+        for i, (view, path) in enumerate(views):
+            if i:  # the first view is the login landing
+                page.goto(app_url + path)
+                _wait_settled_admin(page)
+            marker = ".docs-table" if view == "sources" else f"#{view}-table-wrap"
+            expect(page.locator(marker)).to_be_visible(timeout=15_000)
+
+            _assert_viewport_width(page, f"{path} (desktop)")
+            report = _wrap_handle(page, view).evaluate(
+                "el => ({ scroll: el.scrollWidth, client: el.clientWidth })"
+            )
+            assert report["client"] > 346, (
+                f"{path} (desktop): the table card must render at container "
+                f"width, got {report['client']}px (the mobile card is 346px)"
+            )
+            assert report["scroll"] <= report["client"] + 1, (
+                f"{path} (desktop): no in-card scroll is needed at this width "
+                f"(table {report['scroll']}px in a {report['client']}px card)"
+            )
+    finally:
+        page.close()