fix(web): keep the navbar stuck to the top — drop the body height cap on the sticky range

2026-09-01 04:20:26 -04:00
parent 725af9fac1
commit 5fa620fde5
3 changed files with 498 additions and 4 deletions
@@ -42,7 +42,12 @@
 * { box-sizing: border-box; }
-html, body { height: 100%; }
+/* Phase 60 (owner confirmation 2026-08-31, TODO L3): body must NOT carry a
   fixed height — a sticky element's travel range is capped by its
   containing block, and `body { height: 100% }` pinned the box to one
   viewport, so the navbar un-pinned after ~1 viewport of scroll.
   `min-height: 100dvh` on body (below) is what stretches short pages. */
 html { height: 100%; }
 /* The visible page background lives on <html> (the canvas). <body> must
   stay transparent and must NOT create a stacking context, or the
@@ -202,9 +207,11 @@ html::after {
  position: sticky;
  top: 0;
  z-index: 20;
-  /* Phase 12: body is a definite-height flex column; without this the
+  /* Phase 12 (reworded phase 60): body is a flex column that stretches
-     header shrinks (flex-shrink:1) to its content minimum on any page
+     to at least one viewport (min-height: 100dvh) and grows with its
-     whose content overflows the viewport (e.g. Sources at ≤640px). */
+     content; without this the header shrinks (flex-shrink:1) to its
     content minimum on any page whose content overflows the viewport
     (e.g. Sources at ≤640px). */
  flex-shrink: 0;
 }
 /* 2px brand→cyan gradient hairline under the sticky header (phase 08;
@@ -0,0 +1,339 @@
 """Phase 60 E2E (Playwright): the navbar stays stuck to the top of the
 screen at every scroll position — and the short-page layout is intact.
 Source: ``TODO.md`` L3 — "The navbar disappears when you scroll down,
 should stay stuck to the top of the screen" (no user story file —
 TODO-derived phase).
 Run in isolation (DB must be up: ``podman compose up -d db``):
    uv run pytest tests/e2e/test_sticky_navbar.py -v --no-cov
 Mechanism under test (owner-locked A1/A2, roadmap confirmation
 2026-08-31): the sticky rule was always there — ``.app-header`` (every
 page) and the viewer's two-row ``.doc-header`` (``/document.html``)
 both carry ``position: sticky; top: 0``. What broke it: ``body {
 height: 100% }`` pinned the body box to exactly one viewport, and a
 sticky element may only travel inside its containing block — so after
 ~1 viewport of scroll the header un-pinned and scrolled away with the
 body. The CSS-only fix (task 01) dropped ``height`` from the ``body``
 rule (``html`` keeps it); the body's ``min-height: 100dvh`` still drives
 the short-page stretch. This suite proves the BROWSER behavior:
 * on a long page the header's bounding-box top is 0 at a mid-page
  scroll AND at the very bottom (the pre-fix value at the bottom was
  off-screen negative — ~−1264px in the audit's 2000px repro);
 * the document stays the scroll container (phase 52 no-inner-scroller
  contract) — the fix added no scroller, it only let the body grow;
 * the short-page layout is EXACTLY as it was: body stretched to the
  viewport, footer pinned to the viewport bottom, the phase-52
  pinned composer (``position: sticky; bottom: env(safe-area-inset-
  bottom)``) resting in its in-flow slot above the footer.
 Seeding (A3): the long-scroll surfaces are the seeded Sources table
 (``tests/fixtures/docs/`` — 13 docs, same harness as the phase-10/26
 suites) PLUS a generated scratch dir the module writes (``gen/``: 40
 short unique docs + one ``doc-long.md`` with a ~40,000-char body).
 ``import_sources`` runs in a worker thread against the deterministic
 mock embeddings server (the ``test_document_viewer.py`` pattern) — no
 real LLM is involved.
 Test → story mapping (Playwright Mapping Rule):
 1. ``test_app_header_stuck_at_top_on_sources``
 2. ``test_doc_header_stuck_at_top_on_long_document``
 3. ``test_short_page_stretch_and_resting_composer_intact``
 """
 from __future__ import annotations
 import asyncio
 from collections.abc import Iterator
 from pathlib import Path
 from threading import Thread
 from typing import Any
 import httpx
 import pytest
 from playwright.sync_api import Page, 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"
 #: The conftest ``page`` fixture's viewport (1280×800).
 VIEWPORT_W = 1280
 VIEWPORT_H = 800
 #: Tolerance for "stuck at the top" / "pinned to the viewport edge":
 #: the measured positions are sub-pixel stable (0 in the audit), so the
 #: slack is rounding headroom only.
 TOP_TOL_PX = 1
 #: Phase-52 resting-composer constants (test_pinned_composer.py): the
 #: band below a resting composer may only be chrome — the footer's own
 #: measured height plus the flow padding of the settled slot — and the
 #: box must sit in the lower part of the screen.
 BOTTOM_SLACK_PX = 56
 LOWER_PART = 0.8
 #: Seeding: 13 fixture docs (the A9 family, ``.hidden/`` skipped) + the
 #: generated scratch docs.
 SHORT_DOC_COUNT = 40
 LONG_DOC_BODY_CHARS = 40_000
 TOTAL_DOCS = 13 + SHORT_DOC_COUNT + 1  # + the one long doc
 # ---------------------------------------------------------------------------
 # KB seeding (same pattern as the phase 10/26/52 story suites)
 # ---------------------------------------------------------------------------
 def _write_generated_docs(base: Path) -> Path:
    """The scratch source dir: ``gen/doc-001.md`` … ``gen/doc-040.md``
    (one-line bodies, unique titles) plus ``gen/doc-long.md`` with a
    ~40,000-char body (deterministic numbered paragraphs — real
    scrollable content, no injected DOM)."""
    gen = base / "gen"
    gen.mkdir(parents=True, exist_ok=True)
    for i in range(1, SHORT_DOC_COUNT + 1):
        (gen / f"doc-{i:03d}.md").write_text(
            f"# Sticky probe {i:03d}\n\nShort body line for probe {i:03d}.\n",
            encoding="utf-8",
        )
    paragraphs: list[str] = []
    n = 0
    while sum(len(p) for p in paragraphs) < LONG_DOC_BODY_CHARS:
        n += 1
        paragraphs.append(
            f"Paragraph {n:04d}: the quick brown fox jumps over the "
            "lazy dog near the homelab rack."
        )
    (gen / "doc-long.md").write_text(
        "# Sticky long document\n\n" + "\n\n".join(paragraphs) + "\n",
        encoding="utf-8",
    )
    return gen
 async def _import_dirs(mock_port: int, dirs: list[Path]) -> 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(dirs, 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 _reset_db(mock_port: int, dirs: list[Path] | None) -> ImportSummary | None:
    """Truncate the KB (and query log), then optionally re-import."""
    with SessionLocal() as db:
        db.execute(text("TRUNCATE chunks, documents, query_log"))
        db.commit()
    if not dirs:
        return None
    return _run_in_thread(_import_dirs(mock_port, dirs))
@pytest.fixture(scope="module")
 def kb_db_ready(app_url: str) -> None:
    """Module-scoped twin of conftest's ``db_ready`` (conftest's one is
    function-scoped and cannot back a module-scoped fixture)."""
    body = httpx.get(f"{app_url}/api/health", timeout=5).json()
    if body["db"] != "up":
        pytest.skip("Postgres not reachable — run `podman compose up -d db` first")
@pytest.fixture(scope="module")
 def seeded_kb(
    mock_llm: int, kb_db_ready: None, tmp_path_factory: pytest.TempPathFactory
 ) -> Iterator[None]:
    """A fresh KB: the 13 fixture docs + the generated scratch dir,
    imported once for the whole module (truncated again on teardown)."""
    gen = _write_generated_docs(tmp_path_factory.mktemp("sticky_navbar"))
    summary = _reset_db(mock_llm, [FIXTURES, gen])
    assert summary is not None and summary.added == TOTAL_DOCS, (
        f"expected {TOTAL_DOCS} added docs, got {summary.added if summary else None}"
    )
    yield
    _reset_db(mock_llm, None)
 # ---------------------------------------------------------------------------
 # Measurement helpers
 # ---------------------------------------------------------------------------
 def _assert_top_is_zero(page: Page, selector: str) -> None:
    """The element's bounding-box top is the viewport top (± TOP_TOL_PX).
    ``bounding_box`` is viewport-relative and never scrolls the page
    itself, so this is exactly "where the bar sits where the user left
    it" — the TODO's question. Pre-fix, the header's top here was a
    large NEGATIVE number (scrolled away with the capped body).
    """
    box = page.locator(selector).bounding_box()
    assert box is not None, f"{selector} must be rendered (no bounding box)"
    # Playwright's bounding box is x/y/width/height — top == y.
    assert abs(box["y"]) <= TOP_TOL_PX, (
        f"{selector} is not stuck to the top: rect top={box['y']:.2f} "
        f"(want 0 ±{TOP_TOL_PX}) at scrollY={page.evaluate('() => window.scrollY'):.0f}"
    )
 # ---------------------------------------------------------------------------
 # 1. The app navbar on a long Sources page: stuck at the top at a
 #    mid-page scroll AND at the very bottom
 # ---------------------------------------------------------------------------
 def test_app_header_stuck_at_top_on_sources(
    page: Page, app_url: str, seeded_kb: None
 ) -> None:
    page.set_default_timeout(30_000)
    login(page, app_url)  # → /sources.html (the table is admin-only)
    expect(page.locator("#docs-tbody tr")).to_have_count(TOTAL_DOCS)
    # The precondition the story needs: the page must actually scroll —
    # fail loudly if the table ever stops being long enough.
    sh = page.evaluate("() => document.documentElement.scrollHeight")
    assert sh > 1.5 * VIEWPORT_H, (
        f"the Sources table must be long enough to scroll "
        f"(scrollHeight={sh}, want > {1.5 * VIEWPORT_H:.0f} at a "
        f"{VIEWPORT_W}×{VIEWPORT_H} viewport) — add more generated docs"
    )
    # Mid-page scroll (~1 viewport down — the pre-fix un-pin point): the
    # navbar is still stuck at the top.
    page.evaluate("() => window.scrollTo(0, 800)")
    assert (
        page.evaluate("() => window.scrollY") >= VIEWPORT_H - TOP_TOL_PX
    ), "the test scroll must land"
    _assert_top_is_zero(page, ".app-header")
    # And at the very bottom (the audit's −1264px repro): stuck, and
    # still visible.
    page.evaluate("() => window.scrollTo(0, 999999)")
    _assert_top_is_zero(page, ".app-header")
    expect(page.locator(".app-header")).to_be_visible()
 # ---------------------------------------------------------------------------
 # 2. The document viewer's two-row header on a long doc: stuck at the top
 #    at the bottom of the scroll
 # ---------------------------------------------------------------------------
 def test_doc_header_stuck_at_top_on_long_document(
    page: Page, app_url: str, seeded_kb: None
 ) -> None:
    page.set_default_timeout(30_000)
    # The dedicated viewer page (phase 10/26 contract) on the generated
    # long doc — source ``gen``, path ``doc-long.md``.
    page.goto(f"{app_url}/document.html?source=gen&path=doc-long.md")
    expect(page.locator("#doc-title")).to_have_text("Sticky long document")
    expect(page.locator("#doc-content .doc-md")).not_to_be_empty()
    # The doc must actually scroll (the ~40,000-char body fills many
    # viewports in the 46rem column).
    sh = page.evaluate("() => document.documentElement.scrollHeight")
    assert sh > VIEWPORT_H, (
        f"the long document must make the page scroll "
        f"(scrollHeight={sh}, want > {VIEWPORT_H})"
    )
    # To the very bottom: BOTH header rows stay pinned (the header
    # element is content-sized + sticky — phase 34).
    page.evaluate("() => window.scrollTo(0, document.documentElement.scrollHeight)")
    assert (
        page.evaluate("() => window.scrollY") >= sh - VIEWPORT_H - TOP_TOL_PX
    ), "the test scroll must land at the document bottom"
    _assert_top_is_zero(page, ".doc-header")
 # ---------------------------------------------------------------------------
 # 3. Short-page regression: the phase-52 layout is EXACTLY as it was —
 #    body stretched to the viewport, footer pinned to the viewport
 #    bottom, the pinned composer resting in its in-flow slot above the
 #    footer (the drop of the body's height cap must not have moved a
 #    single pixel of the non-scrolling layout)
 # ---------------------------------------------------------------------------
 def test_short_page_stretch_and_resting_composer_intact(
    page: Page, app_url: str
 ) -> None:
    page.set_default_timeout(30_000)
    page.goto(app_url)
    expect(page.locator("#empty-state")).to_be_visible()
    # The stretch holds: the body is exactly one viewport tall, and the
    # grow must not invent scrollable space.
    sh = page.evaluate("() => document.documentElement.scrollHeight")
    assert sh <= VIEWPORT_H + TOP_TOL_PX, (
        f"an empty chat must not be scrollable (scrollHeight={sh})"
    )
    body = page.locator("body").bounding_box()
    assert body is not None
    assert abs(body["height"] - VIEWPORT_H) <= TOP_TOL_PX, (
        f"the body must stretch to the viewport (height={body['height']:.2f}, "
        f"want {VIEWPORT_H}) — the min-height: 100dvh driver is intact"
    )
    # The footer is pinned to the viewport bottom (the audit's 800/800).
    footer = page.locator(".app-footer").bounding_box()
    assert footer is not None
    footer_bottom = footer["y"] + footer["height"]
    assert abs(footer_bottom - VIEWPORT_H) <= TOP_TOL_PX, (
        f"the footer must sit at the viewport bottom "
        f"(bottom={footer_bottom:.2f}, want {VIEWPORT_H})"
    )
    # The phase-52 pinned composer holds: the pin itself (position:
    # sticky) and its RESTING geometry — in-flow, at the bottom of the
    # screen, with only chrome (the footer + the settled slot's flow
    # padding) in the band below it, never overlapping the footer.
    assert (
        page.evaluate(
            "() => getComputedStyle(document.querySelector('#composer')).position"
        )
        == "sticky"
    ), "the composer must keep its phase-52 sticky pin"
    composer = page.locator("#composer").bounding_box()
    assert composer is not None
    composer_bottom = composer["y"] + composer["height"]
    deadband = VIEWPORT_H - composer_bottom
    assert deadband <= footer["height"] + BOTTOM_SLACK_PX, (
        f"dead band under the composer: {deadband:.0f}px below it with a "
        f"{footer['height']:.0f}px footer — the resting slot moved"
    )
    assert composer_bottom >= VIEWPORT_H * LOWER_PART, (
        f"the composer rests at {composer_bottom / VIEWPORT_H:.0%} of the "
        "viewport — it has to sit at the bottom, not under the empty state"
    )
    assert composer_bottom <= footer["y"] + TOP_TOL_PX, (
        f"the composer overlaps the footer (composer bottom="
        f"{composer_bottom:.2f}, footer top={footer['y']:.2f})"
    )
@@ -0,0 +1,148 @@
 """Unit: the sticky-header contract (phase 60, TODO L3).
 The browser behavior itself is E2E-gated by the phase-60 story suite
 (task 02); like the other frontend-adjacent unit files, this module
 pins the CSS markers the sticky contract depends on, so a silent
 regression is caught without a browser:
 * the ROOT CAUSE (owner-locked A1) is the ``body { height: 100% }``
  cap — a sticky element's travel range is constrained to its
  containing block, and the fixed body height pinned the box to one
  viewport, so ``.app-header`` / ``.doc-header`` un-pinned after
  ~1 viewport of scroll. The FIX (owner-locked A2) is CSS-only:
  ``html`` keeps ``height: 100%`` (harmless viewport baseline) and
  ``body`` carries NO ``height:`` declaration — its existing
  ``min-height: 100dvh`` keeps driving the short-page stretch (the
  phase-52 flex-stretch / pinned-composer / footer contract);
 * ``.app-header`` (every page) and ``.doc-header`` (document viewer)
  keep ``position: sticky; top: 0`` — these pins guard against a
  future "simplification" that would drop the sticky rule that was
  always the intent.
 """
 from __future__ import annotations
 import re
 from pathlib import Path
 FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
 STYLES_CSS = FRONTEND / "assets" / "styles.css"
 def _css() -> str:
    return STYLES_CSS.read_text(encoding="utf-8")
 def _block(css: str, selector: str) -> str:
    """The declaration body of the FIRST top-level ``<selector> { … }``
    rule. Line-anchored on purpose: the phase-60 provenance comment
    quotes ``body { height: 100% }`` literally, so a mid-line match
    would capture the comment instead of the rule (real rules start at
    column 0)."""
    m = re.search(r"(?m)^" + re.escape(selector) + r" \{([\s\S]*?)\n\}", css)
    assert m, f"styles.css must style {selector}"
    return m.group(1)
 # ---------- the root-cause cap is gone (A1 → A2) ----------
 def test_body_height_cap_is_gone_from_the_rule() -> None:
    """Phase 60 (A2): the exact old rule ``html, body { height: 100%;
    }`` is GONE from styles.css, and only ``html`` carries
    ``height: 100%`` now (the harmless viewport baseline stays on the
    canvas element)."""
    css = _css()
    assert "html, body { height: 100%; }" not in css, (
        "the combined html,body height rule must be gone"
    )
    assert "html { height: 100%; }" in css, ("html keeps height: 100%")
 def test_html_rule_carries_the_phase_60_provenance_comment() -> None:
    """The replacement comment cites the phase 60 provenance (owner
    confirmation 2026-08-31, TODO L3) and names the mechanism — the
    sticky travel range is capped by the containing block, and
    min-height: 100dvh is what stretches short pages."""
    css = _css()
    i = css.find("html { height: 100%; }")
    assert i != -1, "the html height rule must exist"
    comment = css[max(0, i - 900) : i]
    assert "Phase 60" in comment, "the comment cites the phase 60 provenance"
    assert "2026-08-31" in comment, "the comment cites the owner confirmation date"
    assert "TODO L3" in comment, "the comment cites the TODO line"
    assert "min-height: 100dvh" in comment, (
        "the comment names the short-page stretch driver"
    )
 def test_body_rule_has_no_height_but_keeps_min_height() -> None:
    """The ``body { … }`` rule: NO ``height:`` declaration (the A1 cap
    must never come back) and ``min-height: 100dvh`` intact — the
    flex-column stretch driver the phase-52 short-page layout (footer
    at the viewport bottom, pinned composer) depends on. The
    declaration list is otherwise UNCHANGED (the flex column
    properties stay)."""
    css = _css()
    body = _block(css, "body")
    assert not re.search(r"(?m)^\s*height\s*:", body), (
        "the body rule must carry NO height declaration"
    )
    assert "min-height: 100dvh" in body, ("the stretch driver stays on body")
    for prop in (
        "margin: 0",
        "display: flex",
        "flex-direction: column",
        "position: relative",
        "background: transparent",
    ):
        assert prop in body, f"the body rule keeps its existing {prop}"
 # ---------- the sticky rules stay (both surfaces) ----------
 def test_app_header_stays_sticky_at_the_top() -> None:
    """``.app-header`` (the navbar on every page) keeps
    ``position: sticky; top: 0`` (z-index 20, the 64px --header-h
    height) and the phase-12 ``flex-shrink: 0`` guard (reworded phase
    60: body stretches via min-height: 100dvh; the guard still covers
    content-overflow pages, e.g. Sources at ≤640px)."""
    css = _css()
    header = _block(css, ".app-header")
    assert "position: sticky" in header, ".app-header must stay sticky"
    assert "top: 0" in header, ".app-header must pin to the top"
    assert "z-index: 20" in header
    assert "height: var(--header-h)" in header
    assert "flex-shrink: 0" in header, "the shrink guard stays"
 def test_doc_header_stays_sticky_at_the_top() -> None:
    """``.doc-header`` (the document viewer's two-row header) keeps
    ``position: sticky; top: 0`` (z-index 20) and the shrink guard —
    the same contract as the app header, so BOTH rows stay pinned
    while the document scrolls."""
    css = _css()
    header = _block(css, ".doc-header")
    assert "position: sticky" in header, ".doc-header must stay sticky"
    assert "top: 0" in header, ".doc-header must pin to the top"
    assert "z-index: 20" in header
    assert "flex-shrink: 0" in header, "the shrink guard stays"
 def test_sticky_pin_comment_mentions_the_stretch_driver() -> None:
    """The reworded phase-12 comment on ``.app-header`` (inside the
    rule, above the guard) matches reality: it names
    ``min-height: 100dvh`` as the body stretch driver (the
    "definite-height" wording of the old cap era is gone) and keeps
    the content-overflow rationale for the guard."""
    css = _css()
    comment = _block(css, ".app-header")
    assert "definite-height" not in comment, (
        "the stale definite-height wording must be gone"
    )
    assert "min-height: 100dvh" in comment, (
        "the reworded comment names the actual stretch driver"
    )
    assert "flex-shrink" in comment and "Sources" in comment, (
        "the guard's rationale (content-overflow pages) stays"
    )