Files
brain-of-reese/tests/e2e/test_nav_rename_sources.py
T
ducoterra ffa919b8bf fix(chat): keep in-flight answers alive across in-app view switches
Root cause (owner repro, verified in a real browser 2026-09-06): the
five navbar views (Chat, RAG, Sources, Tuning, History) were separate
HTML documents, so a navbar click was a REAL cross-document navigation
— the chat page unloaded, the in-flight SSE fetch was aborted, and the
phase-48 teardown (app/api/chat.py `finally`, "chat: turn cancelled")
stopped the model. Observed: send question -> click RAG mid-stream ->
click Chat -> the answer never finished: no `query_log` row, and on
return a dangling question with no brain record (the pre-token pagehide
partial persist skips because `acc` is empty).

Phase-48 LOCKED-DECISION REFINEMENT (owner-confirmed 2026-09-06,
flagged per AGENTS.md rule 3, not silently deviated): "real navigation
cancels the fetch" now means LEAVING THE APP — tab close,
external/other-document navigation, the Stop button. In-app navbar
switches are client-side view switches and no longer cancel.

Fix — Option A (SPA shell), chosen over B (Service Worker owns the
stream) and C (server-side turn registry + resume):
- frontend/index.html is the shell: ONE `<main id="main">` holds the
  five `<section class="view">` blocks; hidden views carry BOTH
  `hidden` and `inert` (WCAG — no focus/keyboard traversal). The
  shared header, the single `doc-modal-*` skeleton, and the
  `#app-version` footer each exist exactly once; the per-view copies
  from the four folded pages are dropped.
- New frontend/assets/router.js (vanilla module — no framework, no
  bundler, No-CDN rule intact): lazy-imports a view module on FIRST
  show only (mount-once, hide-forever — the chat view's in-flight SSE
  reader persists across switches; that persistence IS the fix);
  intercepts same-shell navbar links with preventDefault +
  history.pushState (never a document load); handles popstate; single
  writer of `.nav-link` active state (is-active + aria-current),
  document.title, and the per-view meta description (values carried
  over from the old pages' heads, brand-resolved at write time).
- Each folded page's JS becomes `export async function mount(root)` —
  root-scoped queries; `initSharedHeader()` dropped (the header boots
  once in the shell via the chat module; the admin flag comes from the
  same cached `fetchIsAdmin()` promise — zero extra requests).
- app/main.py: a small list-driven route factory serves the shell for
  /tuning.html, /sources.html, /git-sources.html, /history.html —
  registered AFTER the API routers and BEFORE the static catch-all
  (routes-first). The phase-33 caching middleware applies no-cache +
  `?v=` rewriting unchanged; app/core/caching.py needed NO change
  (the view paths did not change — pinned by the integration tests).
- The four old view .html files are DELETED (one source of truth);
  deep links to the old URLs keep working (the router picks the view
  from the pathname); `/?chat=<id>` is unaffected; the Containerfile
  bundles router.js (inlining the lazy view modules) and drops the
  folded page files.
- app/schemas.py: HistoryTurn.text cap 4000 -> 32000 — the shell
  keeps long saved answers in the chat, and the old cap (stricter than
  the 24_000-char total history budget) 422-rejected any second turn
  in such a chat (found by the phase-42 E2E suite on the shell).

Boundaries: login.html, shared.html, doc-edit.html, document.html
REMAIN separate documents (flow pages, not navbar tabs); a mid-stream
navigation to doc-edit/document.html still cancels per phase 48
(follow-up candidate, out of scope). The SSE API is unchanged. Real
departures still cancel the turn — phase 48 intact (pinned by
tests/e2e/test_stop_generation.py, unchanged, and by the new suite's
real-departure control).

Tests:
- Phase-20 suite REWRITTEN to the new semantics
  (tests/e2e/test_sources_midstream_bug.py): a navbar switch no longer
  cancels — the stream survives the switch and the FULL answer
  settles; the pagehide partial persist REMAINS for real departures
  (the partial's exact shape — first streamed chunk prefix, no done
  metadata — is still pinned there).
- NEW story suite tests/e2e/test_nav_switch_keeps_stream.py (mock
  LLM): the owner repro (send -> RAG mid-stream -> Chat: window
  sentinel survives = same document, FULL answer, exactly one brain
  turn in bor.chat.v1, exactly one settled query_log row, auto-saved
  row matches) + the same mid-stream switch against the other three
  views + the real-departure-still-cancels control + the no-switch
  baseline.
- tests/unit/test_frontend_router.py: source-level pins of the router
  invariants (click interceptor targets ONLY same-shell view paths,
  pushState-only switches, mount-once guard, hidden+inert pair,
  single-writer active state/title); shell-route integration tests
  (each folded path serves the shell with no-cache + `?v=` body; a
  non-view path still 404s); the file-reading unit pins re-pointed at
  the shell (the four view files are gone — the shell is the source
  of truth).

Verification (this commit): full suite green — 1565 unit+integration
tests, app/ coverage 99% (>90% floor); ruff + pyright clean; the
phase's E2E suites green in isolation (house protocol, AGENTS.md rule
9). Owner repro verified in a real browser against the real LLM
(dev server :8010, headful Chromium): "tell me about everquest" ->
RAG mid-stream -> Chat — the answer completed with one brain bubble
and no error banner, `query_log` gained exactly one settled row
(deflected=True: the dev KB holds no EverQuest docs — the settle, not
the topic, is the proof), zero "chat: turn cancelled" lines for that
turn; the control (real navigation to /shared.html mid-stream) still
cancelled (no settled row, the cancel line logged, the partial
persisted on return). Screenshots: .agents/screenshots/76_manual_*.

Phase 76 (76_spa_nav_shell) complete — moved to
.agents/phases/complete/.
2026-09-06 06:31:31 -04:00

408 lines
16 KiB
Python
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
"""Phase 48 story E2E (Playwright): the nav rename — "Sources" becomes
"RAG", "Git sources" becomes "Sources".
Story: ``.agents/user_stories/nav-sources-rag-rename.md``
Run in isolation (DB must be up: ``podman compose up -d db``):
uv run pytest tests/e2e/test_nav_rename_sources.py -v --no-cov
Owner request (2026-08-28): the two admin-only nav items read like the
same thing, so they are relabeled — the document-catalog link
(``#nav-sources`` → /sources.html) becomes **"RAG"** and the
source-manager link (``#nav-git-sources`` → /git-sources.html) becomes
**"Sources"**. Everything else is UNCHANGED (the phase's locked
decision, label-only): element ids, hrefs, the physical nav order, the
phase-16/19/35 ship-hidden/reveal contract, ``header.js`` behavior, and
every other label on the pages — the document viewer's "Sources" back
button (a different control, phase 13) and the "Sync sources" button
(phase 32) in particular.
The six pages under test (all carry the one shared header, phase
19/34): chat (/), the RAG catalog (/sources.html), the Sources manager
(/git-sources.html), Tuning (/tuning.html), the login page
(/login.html), and the document viewer (/document.html — seeded with
one fixture document row first, the test_nav_consistency.py viewer
pattern; #doc-title must settle before any bar assertion).
Contract under test (desktop viewport 1280×800, settled whoami state —
every assertion waits for the initSharedHeader pass to land first):
* admin: on EACH of the six pages ``#nav-sources`` is visible with the
exact text "RAG" and href /sources.html, ``#nav-git-sources`` is
visible with the exact text "Sources" and href /git-sources.html,
and the nav DOM order reads Chat, RAG, Sources, Tuning; the
current page's link is the ONLY one carrying is-active +
aria-current="page" (the login and viewer pages mark none —
neither is a nav page).
* from the chat page: clicking "RAG" (``#nav-sources``) lands on
/sources.html with that link active; clicking "Sources"
(``#nav-git-sources``) lands on /git-sources.html with that link
active.
* anonymous: on / and /login.html both links are PRESENT in the DOM
(the ship-hidden contract — header.js toggles the hidden attribute,
the markup is never removed) but hidden, and #sign-in-link is
visible.
* the rename did not leak: on /sources.html the Sync button still
reads "Sync sources" (``#sync-label``), and on the settled viewer
page the back button's span still reads "Sources" (href
/sources.html).
Determinism note: the seed truncates documents/chunks/query_log/
steering_notes and re-imports the fixture docs (mock embeddings) so the
viewer URL resolves to "Kubernetes Homelab Cluster" on every run. No
chat turn is submitted and #sync-btn is never clicked.
Test → story mapping (Playwright Mapping Rule):
1. ``test_admin_labels_on_all_six_pages``
2. ``test_click_navigates_with_marker``
3. ``test_anonymous_sees_neither``
4. ``test_untouched_controls_stay``
"""
from __future__ import annotations
import asyncio
import re
from pathlib import Path
from threading import Thread
from typing import Any
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"
CHAT_URL = "/"
SOURCES_URL = "/sources.html"
GIT_SOURCES_URL = "/git-sources.html"
TUNING_URL = "/tuning.html"
LOGIN_URL = "/login.html"
#: A seeded fixture doc (source=docs), URL-encoded — the same document
#: every viewer suite uses (title "Kubernetes Homelab Cluster").
VIEWER_URL = "/document.html?source=docs&path=homelab%2Fkubernetes.md"
DOC_TITLE = "Kubernetes Homelab Cluster"
#: The six pages of the app, in the story's order.
SIX_PAGES = (
("chat", CHAT_URL),
("sources", SOURCES_URL),
("git-sources", GIT_SOURCES_URL),
("tuning", TUNING_URL),
("login", LOGIN_URL),
("viewer", VIEWER_URL),
)
#: The locator of the link that carries the current-page marker on each
#: page (None — login and viewer are not nav pages — marks none).
CURRENT_LINK: dict[str, str | None] = {
"chat": ".app-nav a[href='/']",
"sources": "#nav-sources",
"git-sources": "#nav-git-sources",
"tuning": "#nav-tuning",
"login": None,
"viewer": None,
}
#: The four primary nav links, in their physical DOM order — the labels
#: after the phase-48 swap (ids/hrefs unchanged).
NAV_LABELS = ("Chat", "RAG", "Sources", "Tuning")
#: The FIFTH a.nav-link in every page header (phase 53 saved-chat
#: history — ship-hidden, revealed by header.js for admins). The DOM
#: enumeration below therefore always sees it (pre-existing since phase
#: 53; the list below now matches the real nav).
NAV_TAIL = ("History",)
#: The login.js script — route pattern for the redirect suppression.
LOGIN_JS_ROUTE = re.compile(r"/assets/login\.js(\?.*)?$")
#: is-active as a word-boundary regex (to_have_class matches against the
#: whole class string — the test_git_sources_admin.py convention).
IS_ACTIVE = re.compile(r"\bis-active\b")
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 owns the test loop)."""
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_db(mock_port: int) -> None:
"""Fresh KB + the fixture docs so the viewer URL resolves (the
test_nav_consistency.py seeding pattern)."""
with SessionLocal() as db:
db.execute(text("TRUNCATE chunks, documents, query_log, steering_notes"))
db.commit()
_run_in_thread(_import_fixtures(mock_port))
def _wait_settled_admin(page: Page) -> None:
"""Wait for initSharedHeader's whoami toggle to land for a signed-in
admin: the whoami reveal has un-hidden the admin-only nav links. The
nav link is the viewport-independent settled signal (the auth pair is
the bar copy on desktop but the dropdown copy at ≤640px, phase 46) —
and the sign-in/out state settles in the SAME initSharedHeader pass.
"""
page.wait_for_function(
"() => !document.querySelector('#nav-sources').hasAttribute('hidden')",
timeout=15_000,
)
def _wait_settled_anonymous(page: Page) -> None:
"""Wait for the whoami toggle to land for an anonymous visitor: the
bar Sign in copy (``#sign-in-link``) loses its ship-hidden attribute
(probed by attribute — at ≤640px the bar copy is CSS-hidden behind
the dropdown copy, phase 46)."""
page.wait_for_function(
"() => !document.querySelector('#sign-in-link').hasAttribute('hidden')",
timeout=15_000,
)
def _assert_renamed_labels(page: Page, name: str) -> None:
"""AC1 on one page: the swapped labels, the unchanged hrefs, and the
nav DOM order (Chat, RAG, Sources, Tuning) on a settled admin bar."""
rag = page.locator("#nav-sources")
expect(rag).to_be_visible(timeout=15_000)
expect(rag).to_have_text("RAG")
expect(rag).to_have_attribute("href", "/sources.html")
git = page.locator("#nav-git-sources")
expect(git).to_be_visible(timeout=15_000)
expect(git).to_have_text("Sources")
expect(git).to_have_attribute("href", "/git-sources.html")
# The nav links (class nav-link) in physical DOM order — the four
# primaries plus the phase-53 admin-only History link.
nav_texts = page.eval_on_selector_all(
".app-nav a.nav-link", "els => els.map(e => e.textContent.trim())"
)
assert nav_texts == [*NAV_LABELS, *NAV_TAIL], (
f"{name}: nav link order/labels are {nav_texts}, expected "
f"{[*NAV_LABELS, *NAV_TAIL]}"
)
# …and the full anchor sequence of the nav (it also carries the
# phase-46 mobile sign-in copy) opens with the same four, in order.
all_texts = page.eval_on_selector_all(
".app-nav a", "els => els.map(e => e.textContent.trim())"
)
assert all_texts[:4] == list(NAV_LABELS), (
f"{name}: .app-nav anchor sequence {all_texts} does not open with "
f"Chat, RAG, Sources, Tuning"
)
def _assert_current_marker(page: Page, name: str) -> None:
"""AC1 on one page: the current page's link carries is-active +
aria-current="page" — and ONLY it does (login/viewer mark none)."""
current = CURRENT_LINK[name]
if current is None:
assert page.locator(".app-nav a.is-active").count() == 0, (
f"{name}: no nav page is current — no link may carry is-active"
)
assert page.locator('.app-nav a[aria-current="page"]').count() == 0, (
f"{name}: no nav page is current — no link may carry aria-current"
)
return
expect(page.locator(current)).to_have_class(IS_ACTIVE)
expect(page.locator(current)).to_have_attribute("aria-current", "page")
assert page.locator(".app-nav a.is-active").count() == 1, (
f"{name}: exactly one nav link may carry is-active"
)
assert page.locator('.app-nav a[aria-current="page"]').count() == 1, (
f"{name}: exactly one nav link may carry aria-current"
)
def _visit_admin_page(page: Page, app_url: str, name: str, url: str) -> None:
"""Goto a page as the signed-in admin, wait for the settled header
(and the document title on the viewer), assert the rename contract."""
page.goto(app_url + url)
_wait_settled_admin(page)
if name == "viewer":
# The document itself has settled (rendered, not Loading…/
# not-found) before any bar assertion — the test_nav_consistency
# viewer-pass pattern.
expect(page.locator("#doc-title")).to_have_text(DOC_TITLE, timeout=15_000)
_assert_renamed_labels(page, name)
_assert_current_marker(page, name)
def _visit_login_as_admin(page: Page, app_url: str) -> None:
"""The login page redirects a signed-in admin away (login.js —
phase 16), so this ONE visit serves login.js with the redirect lines
suppressed (a test-local route, the test_nav_consistency.py pattern;
the page's header — settled by the same initSharedHeader pass — is
what gets measured, and the page stays put).
The browser cache is cleared first: phase 33 caches ``/assets/*``
``immutable`` for a year, and the earlier form login already fetched
the (unmodified) login.js — a cache hit would bypass the route.
"""
login_js = (REPO / "frontend" / "assets" / "login.js").read_text(encoding="utf-8")
assert "window.location.replace(safeNext())" in login_js
suppressed = login_js.replace(
"window.location.replace(safeNext())",
"window.__e2e_redirectSuppressed = true; // test: observe the header",
)
page.route(
LOGIN_JS_ROUTE,
lambda route: route.fulfill(
status=200, content_type="text/javascript", body=suppressed
),
)
try:
cdp = page.context.new_cdp_session(page)
try:
cdp.send("Network.clearBrowserCache")
finally:
cdp.detach()
page.goto(app_url + LOGIN_URL)
expect(page).to_have_url(app_url + LOGIN_URL, timeout=15_000)
_visit_admin_page(page, app_url, "login", LOGIN_URL)
finally:
page.unroute(LOGIN_JS_ROUTE)
# ---------------------------------------------------------------------------
# 1. Admin: the swapped labels, unchanged hrefs/order/markers, on all
# six pages
# ---------------------------------------------------------------------------
def test_admin_labels_on_all_six_pages(
page: Page, app_url: str, mock_llm: int, db_ready: None
) -> None:
page.set_viewport_size({"width": 1280, "height": 800})
_seed_db(mock_llm)
login(page, app_url, next=CHAT_URL)
expect(page).to_have_url(app_url + CHAT_URL, timeout=30_000)
for name, url in SIX_PAGES:
if name == "login":
_visit_login_as_admin(page, app_url)
else:
_visit_admin_page(page, app_url, name, url)
# ---------------------------------------------------------------------------
# 2. The renamed links navigate: "RAG" → /sources.html (active),
# "Sources" → /git-sources.html (active)
# ---------------------------------------------------------------------------
def test_click_navigates_with_marker(
page: Page, app_url: str, mock_llm: int, db_ready: None
) -> None:
page.set_viewport_size({"width": 1280, "height": 800})
_seed_db(mock_llm)
login(page, app_url, next=CHAT_URL)
expect(page).to_have_url(app_url + CHAT_URL, timeout=30_000)
_wait_settled_admin(page)
# "RAG" (the renamed catalog label) → the RAG catalog page, where
# #nav-sources is the active link.
expect(page.locator("#nav-sources")).to_have_text("RAG")
page.click("#nav-sources")
expect(page).to_have_url(app_url + SOURCES_URL, timeout=30_000)
_wait_settled_admin(page)
_assert_current_marker(page, "sources")
# "Sources" (the renamed manager label) → the Sources manager page,
# where #nav-git-sources is the active link.
page.goto(app_url + CHAT_URL)
_wait_settled_admin(page)
expect(page.locator("#nav-git-sources")).to_have_text("Sources")
page.click("#nav-git-sources")
expect(page).to_have_url(app_url + GIT_SOURCES_URL, timeout=30_000)
_wait_settled_admin(page)
_assert_current_marker(page, "git-sources")
# ---------------------------------------------------------------------------
# 3. Anonymous: both links present in the DOM (ship-hidden contract) but
# hidden — #sign-in-link visible
# ---------------------------------------------------------------------------
def test_anonymous_sees_neither(
page: Page, app_url: str, mock_llm: int, db_ready: None
) -> None:
page.set_viewport_size({"width": 1280, "height": 800})
_seed_db(mock_llm)
# No login: a fresh context is anonymous by construction.
for name, url in (("chat", CHAT_URL), ("login", LOGIN_URL)):
page.goto(app_url + url)
_wait_settled_anonymous(page)
rag = page.locator("#nav-sources")
git = page.locator("#nav-git-sources")
# Present in the DOM (the markup ships, header.js toggles the
# hidden attribute)…
assert rag.count() == 1, f"{name}: #nav-sources must be in the DOM"
assert git.count() == 1, f"{name}: #nav-git-sources must be in the DOM"
# …and hidden for anonymous (the ship-hidden contract, unchanged
# by the rename).
expect(rag).to_be_hidden()
expect(git).to_be_hidden()
# The reduced bar's settled sign-in control is visible.
expect(page.locator("#sign-in-link")).to_be_visible()
# ---------------------------------------------------------------------------
# 4. The rename did not leak: the Sync button label and the viewer back
# button label (different controls) are untouched
# ---------------------------------------------------------------------------
def test_untouched_controls_stay(
page: Page, app_url: str, mock_llm: int, db_ready: None
) -> None:
page.set_viewport_size({"width": 1280, "height": 800})
_seed_db(mock_llm)
# On /sources.html (admin — the button ships hidden and the page
# boot reveals it on the same cached whoami) the Sync button still
# reads "Sync sources".
login(page, app_url, next=SOURCES_URL)
expect(page).to_have_url(app_url + SOURCES_URL, timeout=30_000)
_wait_settled_admin(page)
expect(page.locator("#sync-btn")).to_be_visible(timeout=15_000)
expect(page.locator("#sync-label")).to_have_text("Sync sources")
# Never clicked — a real sync is test_sync_button.py's job.
# On the settled viewer page the back button's span still reads
# "Sources" (the viewer back link is a different control — phase 13;
# the rename only touched the nav items).
page.goto(app_url + VIEWER_URL)
expect(page.locator("#doc-title")).to_have_text(DOC_TITLE, timeout=15_000)
back = page.locator("#doc-back")
expect(back).to_have_attribute("href", "/sources.html")
expect(back.locator("span")).to_have_text("Sources")