Files
brain-of-reese/tests/integration/test_caching_revalidation.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

222 lines
9.2 KiB
Python

"""Integration: the phase-54 revalidation contract against the REAL app.
Phase 33's two cache-busting layers (``asset_version()`` +
``CachingMiddleware``) rewrite the known HTML pages to carry
``?v=<token>`` asset references — but the ``etag`` / ``last-modified``
validators Starlette publishes for a page describe the STATIC FILE, not
the rewritten body this process built from its own token. A conditional
GET that matched those validators used to 304 out of the rewrite: the
browser kept the HTML it already had, whose ``?v=`` pinned the PREVIOUS
commit's CSS/JS — cached ``immutable`` for a year. This suite pins the
fix end-to-end (real ``app.main:app``, real ``StaticFiles`` mount on the
real ``frontend/`` tree, real ``asset_version()`` token — no mocks):
* every known page (``HTML_PAGES``) AND the dynamic ``/shared/<token>``
page 200s on a conditional GET, always with the current
``?v=<token>`` body and no validators;
* ``/assets/*`` is untouched: ``immutable`` for a year, validators
intact, a conditional GET on the versioned URL still 304s (that 304
is safe — the URL itself carries the version);
* ``/api/*`` stays byte-identical: no ``cache-control`` injected, no
validators, conditional headers pass through (the SSE chat stream's
pass-through is pinned by ``test_chat_api.py`` — the regression run
below).
Requires: podman compose up -d db
"""
from __future__ import annotations
import os
import re
from collections.abc import Iterator
from pathlib import Path
import httpx
import pytest
from fastapi.testclient import TestClient
from sqlalchemy import text
from sqlalchemy.orm import Session
from app.core.caching import (
ASSET_CACHE_CONTROL,
HTML_CACHE_CONTROL,
HTML_PAGES,
asset_version,
)
REPO = Path(__file__).resolve().parents[2]
FRONTEND = REPO / "frontend"
def _token_ref_re(token: str) -> re.Pattern[str]:
"""A local ``assets/…`` href/src reference that already carries
``?v=<token>`` (the middleware's rewrite output)."""
return re.compile(r'(?:src|href)="(?:/)?assets/[^"?#]*\?v=' + re.escape(token) + r'"')
def file_validators(page_file: Path) -> tuple[str, str]:
"""The etag / last-modified Starlette would stamp on the underlying
static file — exactly what a browser revalidates against.
``stat_result`` is passed up front: starlette 1.x's ``FileResponse``
defers the stat to ``__call__``, so without it the headers carry no
validators yet (same pattern as the unit suite's ``_file_validators``).
"""
from starlette.responses import FileResponse
headers = FileResponse(page_file, stat_result=os.stat(page_file)).headers
return headers["etag"], headers["last-modified"]
#: Phase 76 (task 01): the shell-served view paths — the URL is a
#: navbar view, the file on disk is the SHELL (the shell route in
#: app/main.py serves frontend/index.html for it). The etag
#: computation below must use the file that actually backs the
#: response, or the conditional-GET probe would carry a validator no
#: browser ever saw. Tasks 02/03 extended this as the remaining views
#: folded in (task 03 — History — completes the set: all four
#: non-chat navbar views). The page CONTRACT itself is unchanged: the
#: phase-33 middleware wraps the whole app and lists the path in
#: HTML_PAGES, so the shell-route response is normalized exactly like
#: a static page (200, no-cache, ?v=, no validators — asserted by
#: _assert_page_contract below).
SHELL_BACKED_PAGES = {
"/tuning.html": "index.html", # phase 76 task 01
"/sources.html": "index.html", # phase 76 task 02
"/git-sources.html": "index.html", # phase 76 task 02
"/history.html": "index.html", # phase 76 task 03
}
def _page_file(path: str) -> Path:
"""The static file backing a page path (``/`` → ``index.html``;
the shell-served view paths → the shell, ``SHELL_BACKED_PAGES``)."""
name = SHELL_BACKED_PAGES.get(path, path.lstrip("/") or "index.html")
file = FRONTEND / name
assert file.is_file(), f"missing page file for {path}: {file}"
return file
def _assert_page_contract(response: httpx.Response, token: str) -> None:
"""The phase-54 page contract on any known page: a full 200 with the
CURRENT process token on the asset refs, ``Cache-Control: no-cache``,
and NO validators (a page must never be revalidated against a
validator this process published)."""
assert response.status_code == 200, (
f"a page path must never 304 (got {response.status_code})"
)
assert response.headers["cache-control"] == HTML_CACHE_CONTROL
assert "etag" not in response.headers
assert "last-modified" not in response.headers
assert f"?v={token}" in response.text
assert _token_ref_re(token).search(response.text), (
"no local assets/ ref carries ?v=<current token>"
)
@pytest.fixture(autouse=True)
def clean_chats(db: Session) -> Iterator[None]:
"""``saved_chats`` is global state — the share test writes one row
(house pattern from ``test_chats_api.py``)."""
db.execute(text("TRUNCATE saved_chats"))
db.commit()
yield
db.execute(text("TRUNCATE saved_chats"))
db.commit()
def test_every_known_page_200s_on_conditional_get(client: TestClient) -> None:
"""THE phase-54 regression, real app: for EVERY known page, a plain
GET sets the contract, then a conditional GET carrying the static
FILE's etag (what a browser captured pre-fix) must still 200 with
the SAME rewritten body — pre-fix the loop failed on the very first
304, pinning the browser on the previous commit's immutable assets."""
token = asset_version()
for path in HTML_PAGES:
plain = client.get(path)
_assert_page_contract(plain, token)
etag, _ = file_validators(_page_file(path))
conditional = client.get(path, headers={"if-none-match": etag})
_assert_page_contract(conditional, token)
assert conditional.content == plain.content, (
f"{path}: the conditional 200 must serve the same rewritten body"
)
def test_dynamic_shared_page_200s_on_conditional_get(admin_client: TestClient) -> None:
"""The dynamic /shared/<token> page (phase 51) gets the same contract
via the real save+share flow: a conditional GET carrying the
``shared.html`` file's etag must still 200 with the rewritten body —
the route's ``FileResponse`` honours conditional headers, so without
the inbound strip this page 304'd too."""
token = asset_version()
created = admin_client.post(
"/api/chats",
json={
"messages": [
{"who": "user", "text": "How did I install gitlab?"},
{"who": "brain", "text": "You've got this!"},
],
"share": True,
},
)
assert created.status_code == 201
share_url = created.json()["share_url"]
plain = admin_client.get(share_url)
_assert_page_contract(plain, token)
etag, _ = file_validators(FRONTEND / "shared.html")
conditional = admin_client.get(share_url, headers={"if-none-match": etag})
_assert_page_contract(conditional, token)
assert conditional.content == plain.content
def test_assets_keep_immutable_validators_and_304(client: TestClient) -> None:
"""The inbound strip must NOT have widened to /assets/*: the versioned
asset URL keeps its validators and still 304s on a conditional GET —
that 304 is safe because the URL itself carries ?v=<token>."""
token = asset_version()
url = f"/assets/styles.css?v={token}"
plain = client.get(url)
assert plain.status_code == 200
assert plain.headers["cache-control"] == ASSET_CACHE_CONTROL
assert "etag" in plain.headers
assert "last-modified" in plain.headers
conditional = client.get(url, headers={"if-none-match": plain.headers["etag"]})
assert conditional.status_code == 304 # versioned-URL 304s stay safe
assert conditional.content == b""
assert conditional.headers["cache-control"] == ASSET_CACHE_CONTROL
def test_api_paths_get_no_cache_headers_and_untouched_stream(client: TestClient) -> None:
"""/api/* stays byte-identical: no cache-control injected, no
validators published, and conditional headers pass through to the
route untouched (the SSE chat stream's pass-through is pinned by
``tests/integration/test_chat_api.py`` — the regression run below)."""
plain = client.get("/api/health")
assert plain.status_code == 200
assert "cache-control" not in plain.headers
assert "etag" not in plain.headers
assert "last-modified" not in plain.headers
conditional = client.get("/api/health", headers={"if-none-match": "x"})
assert conditional.status_code == 200 # pass-through — the strip is page-scoped
assert conditional.json() == plain.json()
assert "cache-control" not in conditional.headers
def test_page_token_matches_process_token(client: TestClient) -> None:
"""The token embedded in the served page equals ``asset_version()`` —
the per-process ``functools.cache`` contract: one page load can never
mix two versions (phase 54, assumption 5)."""
token = asset_version()
response = client.get("/")
assert response.status_code == 200
match = re.search(r'styles\.css\?v=([^"]+)"', response.text)
assert match is not None, "styles.css ref not found in the served page"
assert match.group(1) == token