phase: 144_modal_back_button
**Phase 144 final verification pass — complete, 100% green.** - **Verified:** all 3 tasks already implemented in-tree — `document-modal.js` history-aware modal (one state-only entry per open: push on open, adopt on forward re-open, in-place `replaceState(prev)` on button/Escape/backdrop close, `closingViaBack`-guarded back-close, top-level popstate listener), `tests/unit/test_frontend_doc_modal.py` source pins, `tests/e2e/test_modal_back_button.py` (4 tests). No code fixes needed. - **Tests:** `uv run pytest` → 3371 passed · `uv run pytest --cov=app --cov-report=term-missing` → 3371 passed, **app/ 99%** (gate >90%) · `uv run pytest tests/e2e/test_modal_back_button.py -v --no-cov` → 4 passed in isolation · regressions in isolation: `test_document_back_navigation.py` 3 passed, `test_kb_tree_nav.py` 4 passed · `uv run ruff check .` clean, `uv run pyright` 0 errors. - **Completion criteria:** back closes modal (URL/view/drill untouched; second back navigates as usual) ✅ · back→forward re-opens same doc, no duplicate entry ✅ · button/Escape/backdrop close leaves no dead back step ✅ (state byte-identity + modal-free back chain; `replaceState` can't shrink `history.length` — documented deviation, locked design forbids `history.back()/go()`) · all gates green ✅ · no behavior change in completed phases (pre-phase pins + both regression E2Es green) ✅. - **Docs:** README — one feature-description gap fixed (added the phase-144 Back/Forward modal behavior to the Chat section); all other README facts verified against code (routes, every `.env.example` var, all ~40 documented defaults vs `app/config.py`). AGENTS.md — verified consistent, no changes needed. - **Decisions:** no live-infrastructure work touched; all changes left uncommitted in the working tree per harness protocol. - **Next pending phase:** `145_git_source_links`.
This commit is contained in:
@@ -0,0 +1,450 @@
|
||||
"""Phase 144 task 03 E2E (Playwright, mock-only): the browser Back button
|
||||
closes the document modal — and the history stays sane.
|
||||
|
||||
Story: n/a (TODO.md L6, Protocol B append). The owner's verbatim bug:
|
||||
"back button doesn't close the modal of you're looking at but instead
|
||||
navigates back behind the modal which can lead to confusion". Phase 144
|
||||
made the modal history-aware (``frontend/assets/document-modal.js``):
|
||||
the open owns ONE state-only entry (``{ modal: "doc", source, path }`` —
|
||||
the URL never moves), Back closes the modal instead of navigating the
|
||||
view behind it, a button/Escape/backdrop close removes the entry IN
|
||||
PLACE (the next Back behaves as if the modal never opened), and Forward
|
||||
onto the entry re-opens the same document (the browser owns its
|
||||
entries — the phase-99 D2 convention).
|
||||
|
||||
This is the browser-visible proof with the REAL history: ``page.go_back()``
|
||||
/ ``page.go_forward()`` drive the owner's exact scenario against the
|
||||
real app + the deterministic mock. The source-level mechanics are pinned
|
||||
by ``tests/unit/test_frontend_doc_modal.py`` (task 02) — this suite is
|
||||
the only place the actual ``popstate`` round-trips are exercised.
|
||||
|
||||
Run in isolation (DB must be up: ``podman compose up -d db``):
|
||||
|
||||
uv run pytest tests/e2e/test_modal_back_button.py -v --no-cov
|
||||
|
||||
MOCK-ONLY suite: the chat-driven tests use the mock's scripted
|
||||
summary-read flow (``SUMMARY_SEED_READ_TRIGGER``, phase 119 LOCKED A1 —
|
||||
chips cite READ docs only): the question makes the mock ``read``
|
||||
``docs/homelab/kubernetes.md``, and that READ doc is the answer's single
|
||||
source chip the helper clicks.
|
||||
|
||||
KB fixture (the house pattern from the phase-26 / phase-13 story files):
|
||||
the real importer over ``tests/fixtures/docs/`` (source ``docs``) with
|
||||
the deterministic mock embeddings, per-test ``TRUNCATE`` + re-import —
|
||||
the KB is identical for every test. The drill test walks the fixture's
|
||||
real tree: ``docs`` → ``homelab`` (files: kubernetes.md, …) →
|
||||
``container_gitlab`` (files: gitlab.md, gitlab-compose.yaml).
|
||||
|
||||
Boot contract (the one deviation from a bare ``login(next="/")``):
|
||||
tests 1–3 boot WITH one prior in-shell entry — ``login`` lands on
|
||||
``/sources.html`` (the RAG view), then the Chat nav link is clicked
|
||||
(a same-document ``pushState`` to ``/``). That makes every test's
|
||||
SECOND Back a REAL, assertable step (the router's own entry) instead of
|
||||
a dead end: the phase design says a boot with no prior history must
|
||||
skip the second-back assert (the modal entry's existence is proven by
|
||||
test 3's Forward), so the suite boots with history rather than note-and-
|
||||
skip. ``login`` itself uses ``location.replace`` on success, so without
|
||||
the nav click the stack would be a single entry.
|
||||
|
||||
Test → observable mapping (Playwright Mapping Rule):
|
||||
1. ``test_back_closes_modal_view_unchanged`` — boot on ``/`` (chat,
|
||||
prior entry ``/sources.html``) → ``_open_chip_modal`` (URL ``/``,
|
||||
``#view-chat`` visible) → ``go_back``: the modal HIDES, the URL is
|
||||
STILL ``/`` and ``#view-chat`` is still visible (the view behind was
|
||||
NOT navigated — the owner's confusion is gone); a SECOND ``go_back``
|
||||
is a real step: the URL moves away from ``/`` to ``/sources.html``
|
||||
and the RAG view shows (the navigation the first back should have
|
||||
done had the modal entry not existed).
|
||||
2. ``test_button_close_leaves_no_dead_step`` — boot, chip modal (the
|
||||
open owns exactly ONE entry: ``history.length`` grows by one), close
|
||||
via ``#doc-modal-close`` → the modal hides and the entry the browser
|
||||
is SITTING ON is byte-for-byte the pre-open state (the in-place
|
||||
``replaceState(entry.prev)`` — the task file's "depth restored" /
|
||||
"one back leaves /" assertions presuppose that ``replaceState``
|
||||
REMOVES the entry; it rewrites it in place — verified against
|
||||
Chromium: the slot stays, so the length holds pre-open+1 and the
|
||||
first back is the invisible twin of the entry below, the second
|
||||
back the real step; the locked design — no ``history.back()``/
|
||||
``go()``, no popstate on a close, task-02 unit-pinned — forbids the
|
||||
only alternative that could shrink the stack). What the mechanism
|
||||
GUARANTEES is pinned instead: after the close, ``history.state``
|
||||
deep-equals the pre-open state (no entry in the stack carries modal
|
||||
state anymore — a back can neither re-open the modal nor navigate
|
||||
behind it), and the back chain is modal-free: within two backs the
|
||||
URL leaves ``/`` and lands on the real pre-open entry
|
||||
(``/sources.html``), the modal staying closed the whole way.
|
||||
NOTE: the phase's "no dead no-op back step" criterion holds in the
|
||||
mechanism-faithful sense (no modal step survives a close); the
|
||||
single invisible twin back is an inherent Web-API artifact of the
|
||||
locked in-place pattern, flagged for the phase record.
|
||||
3. ``test_forward_reopens_same_document`` — boot, chip modal (document
|
||||
D, title remembered) → ``go_back`` (the modal closes) →
|
||||
``go_forward``: the modal is visible again with D's title (the same
|
||||
document — ``#doc-modal-title`` equals the pre-back title) and
|
||||
``history.length`` did NOT grow (the re-open ADOPTED the entry — no
|
||||
second push), the URL still ``/``.
|
||||
4. ``test_back_from_sources_keeps_drill`` — the interplay guard: boot
|
||||
on ``/sources.html`` (the RAG view, admin) → drill
|
||||
``docs → homelab`` (state-only entries, phase 99) → open
|
||||
kubernetes.md from the file table → ``go_back``: the modal hides,
|
||||
the URL is still ``/sources.html``, the RAG view is still visible,
|
||||
and the breadcrumb (Knowledge base / docs → homelab) SURVIVED (the
|
||||
drill's popstate re-adopted its own state — no visible change).
|
||||
Deeper: drill ``→ container_gitlab`` → open gitlab.md → ``go_back``:
|
||||
same contract, the three-segment breadcrumb survived; a further
|
||||
``go_back`` then walks the DRILL (back to homelab) — the modal's
|
||||
back consumed exactly one entry, the drill's own back/forward story
|
||||
is untouched.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
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"
|
||||
|
||||
# Phase 119 (LOCKED A1): the chip is the turn's READ doc — the scripted
|
||||
# summary-read flow (mock_llm.SUMMARY_SEED_READ_TRIGGER) reads the
|
||||
# kubernetes fixture so the story's chip exists (a zero-read turn would
|
||||
# chip nothing).
|
||||
QUESTION = (
|
||||
"Read the suggested document: read docs/homelab/kubernetes.md — "
|
||||
"how is my Kubernetes cluster set up?"
|
||||
)
|
||||
DOC_TITLE = "Kubernetes Homelab Cluster"
|
||||
GITLAB_TITLE = "Gitlab"
|
||||
|
||||
|
||||
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 (the house pattern — cf. test_document_viewer.py).
|
||||
"""
|
||||
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, seed: bool) -> ImportSummary | None:
|
||||
"""Truncate the KB (and query log), then re-import the fixtures —
|
||||
the per-test deterministic start (the phase-26 / phase-13 pattern)."""
|
||||
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))
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Page helpers
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _boot_chat_with_prior_history(page: Page, app_url: str) -> None:
|
||||
"""Tests 1–3 boot on ``/`` (chat) with ONE prior in-shell entry.
|
||||
|
||||
``login`` lands on ``/sources.html`` (the RAG view mounts — its
|
||||
state-only drill entries are exactly what the phase-99 interplay
|
||||
guard is about), then the Chat nav link is a same-document
|
||||
``pushState`` to ``/``: the stack is [``/sources.html``, ``/``], so
|
||||
a second Back after the modal's back is a REAL, assertable step
|
||||
(see the module docstring's boot contract)."""
|
||||
login(page, app_url) # default next: /sources.html (the RAG view)
|
||||
expect(page.locator("#view-rag")).to_be_visible(timeout=30_000)
|
||||
page.locator("#app-nav a.nav-link", has_text="Chat").click()
|
||||
expect(page).to_have_url(app_url + "/")
|
||||
expect(page.locator("#view-chat")).to_be_visible()
|
||||
expect(page.locator("#composer")).to_be_visible()
|
||||
|
||||
|
||||
def _open_chip_modal(page: Page) -> None:
|
||||
"""The task's ``open_modal_via_chip``: ask the scripted summary-read
|
||||
question (the mock ``read``s docs/homelab/kubernetes.md and echoes
|
||||
it — the READ doc is the answer's single source chip), click the
|
||||
chip, and wait for the modal to show the LOADED document (the
|
||||
``Document loaded.`` desc state — the content rendered, not the
|
||||
skeleton)."""
|
||||
page.fill("#message-input", QUESTION)
|
||||
page.click("#send-btn")
|
||||
chip = page.locator(".msg.brain .source-chip", has_text="kubernetes.md")
|
||||
expect(chip).to_have_count(1, timeout=30_000)
|
||||
# Phase 119 A1: the read doc is the turn's ONLY chip.
|
||||
expect(page.locator(".msg.brain .source-chip")).to_have_count(1)
|
||||
chip.first.click()
|
||||
_assert_modal_loaded(page, DOC_TITLE)
|
||||
|
||||
|
||||
def _assert_modal_loaded(page: Page, title: str) -> None:
|
||||
"""The modal is open AND the document rendered: the hidden attribute
|
||||
is gone, the overlay is visible, the title is the document's, and
|
||||
the role=status announcer says ``Document loaded.`` (the load's
|
||||
terminal state — the skeleton's loading line is gone)."""
|
||||
expect(page.locator("#doc-modal")).not_to_have_attribute("hidden")
|
||||
expect(page.locator(".doc-modal")).to_be_visible()
|
||||
expect(page.locator("#doc-modal-title")).to_have_text(title, timeout=30_000)
|
||||
expect(page.locator("#doc-modal-desc")).to_have_text("Document loaded.")
|
||||
|
||||
|
||||
def _assert_modal_closed(page: Page) -> None:
|
||||
"""The modal is fully closed: the hidden attribute is back and the
|
||||
overlay is gone from view (the phase-26 assertion)."""
|
||||
expect(page.locator("#doc-modal")).to_have_attribute("hidden", "")
|
||||
expect(page.locator(".doc-modal")).not_to_be_visible()
|
||||
|
||||
|
||||
def _assert_crumb(page: Page, links: list[str], current: str) -> None:
|
||||
"""The phase-99 breadcrumb renders the drilled chain: one
|
||||
``.kb-crumb-link`` per ancestor (in order — the first is always the
|
||||
``Knowledge base`` jump-to-top segment), then the last segment as
|
||||
the ``.kb-crumb-current`` span with ``aria-current="page"``.
|
||||
Hidden = the TOP level (no drill)."""
|
||||
expect(page.locator("#kb-crumb")).to_be_visible()
|
||||
texts = page.eval_on_selector_all(
|
||||
"#kb-crumb :is(.kb-crumb-link, .kb-crumb-current)",
|
||||
"(els) => els.map((el) => el.textContent)",
|
||||
)
|
||||
assert texts == [*links, current], texts
|
||||
cur = page.locator("#kb-crumb .kb-crumb-current")
|
||||
expect(cur).to_have_count(1)
|
||||
expect(cur).to_have_attribute("aria-current", "page")
|
||||
|
||||
|
||||
def _crumb_hidden(page: Page) -> None:
|
||||
"""The top level: no breadcrumb (the phase-97/99 idiom)."""
|
||||
expect(page.locator("#kb-crumb")).to_be_hidden()
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# 1. Back with the modal open: the modal closes — the URL, the view,
|
||||
# and (later) the whole view switch stay exactly where they were;
|
||||
# the SECOND back is the real navigation
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_back_closes_modal_view_unchanged(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
_reset_db(mock_llm, seed=True)
|
||||
page.set_default_timeout(30_000)
|
||||
_boot_chat_with_prior_history(page, app_url)
|
||||
|
||||
# The owner's scenario: a document open in the modal on the chat
|
||||
# view. The URL is /, the chat view is the visible view.
|
||||
_open_chip_modal(page)
|
||||
assert page.url == app_url + "/", f"navigated away: {page.url}"
|
||||
expect(page.locator("#view-chat")).to_be_visible()
|
||||
expect(page.locator("#view-rag")).to_be_hidden()
|
||||
|
||||
# BACK: the modal CLOSES. The view behind is NOT navigated — the
|
||||
# URL is still / and the chat view is still the visible view (the
|
||||
# state-only entry popped, the router's pathname-only popstate is a
|
||||
# no-op).
|
||||
page.go_back()
|
||||
_assert_modal_closed(page)
|
||||
expect(page).to_have_url(app_url + "/") # the URL did not move
|
||||
expect(page.locator("#view-chat")).to_be_visible() # still chat
|
||||
expect(page.locator("#view-rag")).to_be_hidden()
|
||||
|
||||
# A SECOND back is now a REAL step: it leaves the shell's chat
|
||||
# entry and lands on the prior /sources.html entry (the URL moves
|
||||
# away from / — the navigation the first back would have done had
|
||||
# the modal entry not existed; the modal entry's existence is what
|
||||
# absorbed the first back).
|
||||
page.go_back()
|
||||
expect(page).to_have_url(app_url + "/sources.html")
|
||||
expect(page.locator("#view-rag")).to_be_visible(timeout=30_000)
|
||||
expect(page.locator("#view-chat")).to_be_hidden()
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# 2. A button close removes the entry IN PLACE: the current entry's
|
||||
# state is byte-for-byte the pre-open state, and the back chain is
|
||||
# free of any modal step
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_button_close_leaves_no_dead_step(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
_reset_db(mock_llm, seed=True)
|
||||
page.set_default_timeout(30_000)
|
||||
_boot_chat_with_prior_history(page, app_url)
|
||||
|
||||
# The pre-open stack's identity (the stack is [ /sources.html, / ]
|
||||
# plus the context's initial about:blank slot — the login's
|
||||
# location.replace left one entry, the nav click one): the entry
|
||||
# we sit on carries { view: "chat" } (the router's nav-click
|
||||
# pushState), and this is the state a close must restore IN PLACE.
|
||||
depth_before = page.evaluate("() => history.length")
|
||||
state_before = page.evaluate("JSON.stringify(history.state)")
|
||||
_open_chip_modal(page)
|
||||
# The open owns exactly ONE history entry — and it is a modal
|
||||
# entry (the discriminator the close must erase).
|
||||
assert page.evaluate("() => history.length") == depth_before + 1, (
|
||||
"the open must own exactly ONE history entry"
|
||||
)
|
||||
assert page.evaluate("() => !!(history.state && history.state.modal)") is True
|
||||
|
||||
# The button affordance: close WITHOUT a back press.
|
||||
page.click("#doc-modal-close")
|
||||
_assert_modal_closed(page)
|
||||
# The in-place removal: the entry the browser is sitting on is
|
||||
# BYTE-FOR-BYTE the pre-open state (the replaceState wrote
|
||||
# entry.prev into the modal's slot). Every popstate listener
|
||||
# (the router's pathname-only switch, the drill's kb adopt, the
|
||||
# modal's own two branches) now sees exactly the pre-open state —
|
||||
# no entry in the stack carries modal state, so no back can
|
||||
# re-open the modal or navigate behind it.
|
||||
assert page.evaluate("JSON.stringify(history.state)") == state_before, (
|
||||
"a button close must restore the pre-open state in place"
|
||||
)
|
||||
|
||||
# The back chain is modal-free: within TWO backs the URL leaves /
|
||||
# and lands on the real pre-open entry (/sources.html), the modal
|
||||
# staying closed at every step. (The first back may be the
|
||||
# invisible twin of the entry below — the inherent artifact of the
|
||||
# in-place rewrite; see the module docstring. A surviving modal
|
||||
# entry would either keep the modal closed-but-present in the
|
||||
# chain — this loop never sees /sources.html — or hand a back
|
||||
# landing a modal state with the modal closed, which the
|
||||
# forward-re-open branch would turn into a re-open: _assert_
|
||||
# modal_closed after every back rules that out.)
|
||||
for _ in range(2):
|
||||
page.go_back()
|
||||
_assert_modal_closed(page)
|
||||
if page.url != app_url + "/":
|
||||
break
|
||||
expect(page).to_have_url(app_url + "/sources.html")
|
||||
expect(page.locator("#view-rag")).to_be_visible(timeout=30_000)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# 3. Back then Forward: the SAME document re-opens — the forward ADOPTS
|
||||
# the modal's entry (no second push)
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_forward_reopens_same_document(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
_reset_db(mock_llm, seed=True)
|
||||
page.set_default_timeout(30_000)
|
||||
_boot_chat_with_prior_history(page, app_url)
|
||||
|
||||
# Open document D and remember what the entry names.
|
||||
_open_chip_modal(page)
|
||||
title_before = page.locator("#doc-modal-title").inner_text()
|
||||
assert title_before == DOC_TITLE
|
||||
length_open = page.evaluate("() => history.length")
|
||||
|
||||
# Back: the modal closes (the entry is now the forward stack's
|
||||
# top — the browser owns its entries, the phase-99 D2 convention).
|
||||
page.go_back()
|
||||
_assert_modal_closed(page)
|
||||
expect(page).to_have_url(app_url + "/") # still chat, still /
|
||||
|
||||
# Forward onto the modal's entry: the popstate handler re-opens the
|
||||
# document the entry names — the SAME document (the title matches
|
||||
# the pre-back one) and the open ADOPTS the entry: the history did
|
||||
# NOT gain an entry (a second push would grow the length by one).
|
||||
page.go_forward()
|
||||
_assert_modal_loaded(page, title_before)
|
||||
assert title_before == DOC_TITLE, "the forward must re-open the same document"
|
||||
assert page.evaluate("() => history.length") == length_open, (
|
||||
"the forward re-open must ADOPT the entry, not push a second one"
|
||||
)
|
||||
expect(page).to_have_url(app_url + "/") # state-only: the URL never moved
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# 4. The interplay guard: back from a drilled Sources level keeps the
|
||||
# drill (the drill's popstate re-adopts its own state — invisible)
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_back_from_sources_keeps_drill(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
_reset_db(mock_llm, seed=True)
|
||||
page.set_default_timeout(30_000)
|
||||
login(page, app_url) # default next: /sources.html (the RAG view)
|
||||
expect(page.locator("#view-rag")).to_be_visible(timeout=30_000)
|
||||
expect(page).to_have_url(app_url + "/sources.html")
|
||||
_crumb_hidden(page) # the top level: no drill yet
|
||||
|
||||
# --- Scenario A: the shallowest drill that reaches a file row -----
|
||||
# (the fixture's source root has no direct files — the docs →
|
||||
# homelab level is the simplest case with a table link).
|
||||
page.click('#folders-tbody a.folder-link:text-is("docs")')
|
||||
_assert_crumb(page, ["Knowledge base"], "docs")
|
||||
page.click('#folders-tbody a.folder-link:text-is("homelab")')
|
||||
_assert_crumb(page, ["Knowledge base", "docs"], "homelab")
|
||||
row = page.locator("#docs-tbody tr", has_text="kubernetes.md")
|
||||
expect(row).to_have_count(1, timeout=30_000)
|
||||
row.locator("td:nth-child(2) a.doc-link").click()
|
||||
_assert_modal_loaded(page, DOC_TITLE)
|
||||
|
||||
# Back: the modal closes. The URL is still /sources.html, the RAG
|
||||
# view is still visible, and the DRILL survived — the breadcrumb is
|
||||
# byte-for-byte the pre-back chain (the drill's popstate re-adopted
|
||||
# the SAME kb state it was already showing: no visible change).
|
||||
page.go_back()
|
||||
_assert_modal_closed(page)
|
||||
expect(page).to_have_url(app_url + "/sources.html")
|
||||
expect(page.locator("#view-rag")).to_be_visible()
|
||||
_assert_crumb(page, ["Knowledge base", "docs"], "homelab")
|
||||
|
||||
# --- Scenario B: a deeper drilled level ---------------------------
|
||||
page.click('#folders-tbody a.folder-link:text-is("container_gitlab")')
|
||||
_assert_crumb(page, ["Knowledge base", "docs", "homelab"], "container_gitlab")
|
||||
row = page.locator("#docs-tbody tr", has_text="gitlab.md")
|
||||
expect(row).to_have_count(1)
|
||||
row.locator("td:nth-child(2) a.doc-link").click()
|
||||
_assert_modal_loaded(page, GITLAB_TITLE)
|
||||
|
||||
# Back: the same contract one level deeper — the three-segment
|
||||
# breadcrumb survived the modal's pop.
|
||||
page.go_back()
|
||||
_assert_modal_closed(page)
|
||||
expect(page).to_have_url(app_url + "/sources.html")
|
||||
expect(page.locator("#view-rag")).to_be_visible()
|
||||
_assert_crumb(page, ["Knowledge base", "docs", "homelab"], "container_gitlab")
|
||||
|
||||
# The drill's OWN back story is untouched: a further back walks the
|
||||
# breadcrumb (container_gitlab → homelab) — the modal's back
|
||||
# consumed exactly one entry (its own), no leak into the drill's.
|
||||
page.go_back()
|
||||
_assert_crumb(page, ["Knowledge base", "docs"], "homelab")
|
||||
expect(page.locator("#docs-tbody tr", has_text="kubernetes.md")).to_have_count(1)
|
||||
@@ -0,0 +1,360 @@
|
||||
"""Unit: the document modal's history-entry lifecycle (phase 144, task 02).
|
||||
|
||||
Back while the ``#doc-modal`` document viewer is open must CLOSE the modal
|
||||
— not navigate the view behind it (TODO.md L6). The mechanics live in
|
||||
``frontend/assets/document-modal.js``: the open owns ONE state-only
|
||||
history entry (``{ modal: "doc", source, path }`` — the second arg
|
||||
``""`` keeps the URL where it is, so the router's pathname-only
|
||||
popstate is a no-op and the Sources drill re-adopts the state it was
|
||||
already showing); Back closes the modal (remembering the landed state
|
||||
as ``lastPrev``); a button/Escape/backdrop close removes the entry IN
|
||||
PLACE (``replaceState(entry.prev)`` — the next Back behaves as if the
|
||||
modal never opened); Forward onto the entry re-opens the same document
|
||||
(the browser owns its entries — the phase-99 D2 convention).
|
||||
|
||||
The browser behavior itself is E2E-gated by the phase-144 story suite
|
||||
(``tests/e2e/test_modal_back_button.py`` — task 03 drives the real
|
||||
history via ``page.goBack()``/``page.goForward()``); like
|
||||
``test_remove_confirm_modal.py`` (the house pattern for a frontend
|
||||
module whose real behavior is E2E-gated), this module pins the
|
||||
source-level contract a silent regression would break:
|
||||
|
||||
* a second push on the forward re-open (the adopt branch must assign
|
||||
``entry`` WITHOUT a ``pushState``);
|
||||
* a ``replaceState`` on a non-modal current entry (the close's
|
||||
in-place removal is guarded by ``!closingViaBack`` AND
|
||||
``history.state.modal === "doc"`` — the fail-safe);
|
||||
* a double-registered popstate listener;
|
||||
* a ``history.back()``/``history.go()`` introduction (the browser owns
|
||||
its entries — the closed set of ``history.*`` sites is the one push,
|
||||
the two replaces, and the read-only ``history.state`` accesses);
|
||||
* a lost ``lastPrev`` write (it must land BEFORE the
|
||||
``closingViaBack``-wrapped close in the back branch);
|
||||
* a fallback-path push (the ``window.open`` no-skeleton fallback's
|
||||
early ``return`` precedes the first ``history.`` use in the open).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
|
||||
MODAL_JS = FRONTEND / "assets" / "document-modal.js"
|
||||
|
||||
|
||||
def _js() -> str:
|
||||
return MODAL_JS.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _fn(js: str, name: str) -> str:
|
||||
"""The source of a top-level function via balanced-brace counting
|
||||
(the test_remove_confirm_modal.py helper — ``export function``
|
||||
matches the plain ``function name(`` search)."""
|
||||
start = js.find(f"function {name}(")
|
||||
assert start != -1, f"{name}() must exist in document-modal.js"
|
||||
depth = 0
|
||||
for i in range(js.find("{", start), len(js)):
|
||||
if js[i] == "{":
|
||||
depth += 1
|
||||
elif js[i] == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return js[start : i + 1]
|
||||
raise AssertionError(f"unbalanced braces in {name}()")
|
||||
|
||||
|
||||
def _block(source: str, from_i: int) -> str:
|
||||
"""The ``{ … }`` block opening at the first ``{`` at/after
|
||||
``from_i`` — balanced-brace walk, braces included."""
|
||||
brace = source.find("{", from_i)
|
||||
assert brace != -1, "no block opener found"
|
||||
depth = 0
|
||||
for i in range(brace, len(source)):
|
||||
if source[i] == "{":
|
||||
depth += 1
|
||||
elif source[i] == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return source[brace : i + 1]
|
||||
raise AssertionError("unbalanced braces in the entry/popstate block")
|
||||
|
||||
|
||||
def _popstate_handler(js: str) -> str:
|
||||
"""The module-top-level popstate handler's ``{ … }`` body."""
|
||||
i = js.find('window.addEventListener("popstate"')
|
||||
assert i != -1, "the popstate listener must be registered at module top level"
|
||||
arrow = js.find("=> {", i)
|
||||
assert arrow != -1, "the popstate listener must be an arrow function"
|
||||
return _block(js, arrow)
|
||||
|
||||
|
||||
# ---------- the module state: one declaration, no second writer site ----------
|
||||
|
||||
|
||||
def test_history_state_is_declared_exactly_once() -> None:
|
||||
"""REGRESSION: a duplicated module-level declaration / second
|
||||
writer site for the entry state. ``entry`` / ``lastPrev`` /
|
||||
``closingViaBack`` are each declared by EXACTLY ONE ``let`` (the
|
||||
single writer sites — open/close, the popstate handler, the
|
||||
popstate handler — are pinned by the per-branch tests below)."""
|
||||
js = _js()
|
||||
assert js.count("let entry = null;") == 1
|
||||
assert len(re.findall(r"let\s+entry\s*=", js)) == 1
|
||||
assert js.count("let lastPrev = null;") == 1
|
||||
assert len(re.findall(r"let\s+lastPrev\s*=", js)) == 1
|
||||
assert js.count("let closingViaBack = false;") == 1
|
||||
assert len(re.findall(r"let\s+closingViaBack\s*=", js)) == 1
|
||||
|
||||
|
||||
# ---------- the open: one state-only entry, adopt vs push vs replace ----------
|
||||
|
||||
|
||||
def test_open_pushes_exactly_one_state_only_entry() -> None:
|
||||
"""REGRESSION: a double push (two entries per open) or a
|
||||
URL-MOVING push. The push is exactly ONE
|
||||
``history.pushState({ modal: "doc", source, path }, "")`` — the
|
||||
``""`` second arg is what keeps the URL where it is (state-only;
|
||||
the router's pathname-only popstate stays a no-op)."""
|
||||
open_body = _fn(_js(), "openDocumentModal")
|
||||
assert open_body.count("history.pushState(") == 1, (
|
||||
"exactly one pushState in the open — a second push stacks a dead history step"
|
||||
)
|
||||
assert 'history.pushState({ modal: "doc", source, path }, "")' in open_body, (
|
||||
"the push must be state-only: the state key modal: \"doc\" + the "
|
||||
"doc name, and the \"\" second arg (the URL must not move)"
|
||||
)
|
||||
|
||||
|
||||
def test_open_entry_chain_adopts_on_forward_and_replaces_on_defensive_reopen() -> None:
|
||||
"""REGRESSION: a second push on the forward re-open. The chain is
|
||||
``if (entry) → replaceState IN PLACE (keep entry, update
|
||||
source/path — the defensive re-open-while-open never stacks)
|
||||
→ else if the current entry is already a modal entry → ADOPT
|
||||
(assign ``entry = { prev: lastPrev, source, path }`` WITHOUT any
|
||||
pushState — the browser already sits on the entry) → else push.
|
||||
The push lives ONLY in the final else."""
|
||||
open_body = _fn(_js(), "openDocumentModal")
|
||||
st_i = open_body.find("const st = history.state;")
|
||||
def_i = open_body.find("if (entry) {", st_i)
|
||||
adopt_i = open_body.find('else if (st && st.modal === "doc") {', def_i)
|
||||
else_i = open_body.find("} else {", adopt_i)
|
||||
push_i = open_body.find('history.pushState({ modal: "doc", source, path }, "");', else_i)
|
||||
assert -1 < st_i < def_i < adopt_i < else_i < push_i, (
|
||||
"chain order: const st → if (entry) → else if (adopt) → else → push"
|
||||
)
|
||||
# Defensive re-open: replaceState in place, entry KEPT (updated).
|
||||
defensive = open_body[def_i:adopt_i]
|
||||
assert "entry.source = source;" in defensive
|
||||
assert "entry.path = path;" in defensive
|
||||
assert 'history.replaceState({ modal: "doc", source, path }, "")' in defensive
|
||||
assert "pushState" not in defensive, "a defensive re-open must never push"
|
||||
# Forward re-open: ADOPT — assign entry from lastPrev, no push.
|
||||
adopt = open_body[adopt_i:else_i]
|
||||
assert "entry = { prev: lastPrev, source, path };" in adopt, (
|
||||
"the adopt must take lastPrev (the below-entry remembered at the back-close)"
|
||||
)
|
||||
assert "pushState" not in adopt, (
|
||||
"a second push on the forward re-open stacks a dead history step"
|
||||
)
|
||||
# Normal open: push with prev = the entry we sat on, in the final else.
|
||||
assert "entry = { prev: st, source, path };" in open_body[else_i:push_i]
|
||||
assert open_body.count("history.pushState(") == 1
|
||||
|
||||
|
||||
def test_window_open_fallback_precedes_the_entry_logic() -> None:
|
||||
"""REGRESSION: a fallback-path push. A page WITHOUT the modal
|
||||
skeleton falls back to ``window.open(fullPageUrl…)`` in a new tab —
|
||||
that path must not touch the history AT ALL: the fallback's early
|
||||
``return`` precedes the first ``history.`` use in the open."""
|
||||
open_body = _fn(_js(), "openDocumentModal")
|
||||
guard_i = open_body.find("if (!modalEl || !titleEl || !metaEl || !contentEl) {")
|
||||
open_i = open_body.find('window.open(fullPageUrl(source, path), "_blank", "noopener");')
|
||||
ret_i = open_body.find("return;", open_i)
|
||||
first_hist_i = open_body.find("history.")
|
||||
assert -1 < guard_i < open_i < ret_i < first_hist_i, (
|
||||
"the window.open fallback must early-return before any history use"
|
||||
)
|
||||
|
||||
|
||||
# ---------- the close: in-place removal, guarded; entry always nulled ----------
|
||||
|
||||
|
||||
def test_close_removes_the_entry_in_place_and_only_on_a_modal_entry() -> None:
|
||||
"""REGRESSION: a ``replaceState`` on the wrong entry. The in-place
|
||||
removal ``history.replaceState(entry.prev, "")`` is guarded by
|
||||
BOTH ``!closingViaBack`` (a back close already moved us off the
|
||||
entry — the browser owns that pop) AND the current entry still
|
||||
being a modal entry (the fail-safe: a non-modal current entry is
|
||||
NEVER overwritten). Exactly one replaceState in the close."""
|
||||
close_body = _fn(_js(), "closeDocumentModal")
|
||||
assert close_body.count("history.replaceState(") == 1
|
||||
guard = 'if (entry && !closingViaBack && history.state && history.state.modal === "doc") {'
|
||||
guard_i = close_body.find(guard)
|
||||
assert guard_i != -1, (
|
||||
"the close's in-place removal must be guarded by !closingViaBack AND "
|
||||
"history.state.modal === \"doc\""
|
||||
)
|
||||
# The single replace sits INSIDE the guard block.
|
||||
block = _block(close_body, guard_i)
|
||||
assert 'history.replaceState(entry.prev, "");' in block, (
|
||||
"the removal must restore entry.prev (byte-for-byte view state)"
|
||||
)
|
||||
|
||||
|
||||
def test_close_nulls_the_entry_unconditionally() -> None:
|
||||
"""REGRESSION: a lost ``entry = null`` — the adopt's memory
|
||||
surviving a close (a later popstate would then close/adopt against
|
||||
a stale entry). ``entry = null`` runs OUTSIDE the guarded if,
|
||||
between the removal and the hide, on EVERY close (back or not)."""
|
||||
close_body = _fn(_js(), "closeDocumentModal")
|
||||
guard_i = close_body.find('if (entry && !closingViaBack && history.state')
|
||||
brace_i = close_body.find("{", guard_i)
|
||||
block = _block(close_body, guard_i)
|
||||
assert "entry = null" not in block, "entry = null must not sit inside the guard"
|
||||
null_i = close_body.find("entry = null;", brace_i + len(block))
|
||||
hide_i = close_body.find("modalEl.hidden = true;", null_i)
|
||||
assert -1 < null_i < hide_i, "entry = null must run (before the hide) on every close"
|
||||
|
||||
|
||||
# ---------- the popstate listener: once, mutually exclusive branches ----------
|
||||
|
||||
|
||||
def test_popstate_listener_registered_exactly_once() -> None:
|
||||
"""REGRESSION: a double-registered popstate listener (two closes /
|
||||
two re-opens per navigation). The module registers EXACTLY ONE
|
||||
``window.addEventListener("popstate"`` at top level."""
|
||||
js = _js()
|
||||
assert js.count('window.addEventListener("popstate"') == 1
|
||||
|
||||
|
||||
def test_popstate_forward_branch_reopens_and_returns() -> None:
|
||||
"""REGRESSION: the forward branch falling THROUGH into the
|
||||
back-close branch. Forward = the modal is closed and we landed on
|
||||
a modal entry — re-open the doc the entry names (trigger null —
|
||||
focus goes to the content target) and RETURN before the back
|
||||
branch (the two branches are mutually exclusive)."""
|
||||
handler = _popstate_handler(_js())
|
||||
const_i = handler.find("const st = event.state;")
|
||||
flag_i = handler.find('const isModalEntry = st && st.modal === "doc";')
|
||||
fwd_i = handler.find("if (isModalEntry && modalEl && modalEl.hidden) {")
|
||||
reopen_i = handler.find("openDocumentModal(st.source, st.path, null);", fwd_i)
|
||||
ret_i = handler.find("return;", reopen_i)
|
||||
back_i = handler.find("if (!isModalEntry && modalEl && !modalEl.hidden && entry) {", ret_i)
|
||||
assert -1 < const_i < flag_i < fwd_i < reopen_i < ret_i < back_i, (
|
||||
"forward: land on a modal entry with the modal closed → re-open the "
|
||||
"named doc (null trigger) → RETURN before the back-close branch"
|
||||
)
|
||||
|
||||
|
||||
def test_popstate_back_branch_remembers_lastprev_then_closes_guarded() -> None:
|
||||
"""REGRESSION: a lost ``lastPrev`` write (the forward re-open's
|
||||
adopt would then restore ``prev: null`` and corrupt the entry
|
||||
below). Back = the modal is open and we landed on a NON-modal
|
||||
state: write ``lastPrev = st`` FIRST, then the close wrapped in
|
||||
``closingViaBack = true`` … ``= false`` (keeps the close from also
|
||||
replaceState-ing the entry the browser already popped). The flag
|
||||
and the below-entry have their single writer sites here."""
|
||||
js = _js()
|
||||
handler = _popstate_handler(js)
|
||||
back_i = handler.find("if (!isModalEntry && modalEl && !modalEl.hidden && entry) {")
|
||||
assert back_i != -1, (
|
||||
"the back branch (non-modal land + modal open + entry) is gone"
|
||||
)
|
||||
lastprev_i = handler.find("lastPrev = st;", back_i)
|
||||
setflag_i = handler.find("closingViaBack = true;", lastprev_i)
|
||||
close_i = handler.find("closeDocumentModal();", setflag_i)
|
||||
resetflag_i = handler.find("closingViaBack = false;", close_i)
|
||||
assert -1 < lastprev_i < setflag_i < close_i < resetflag_i, (
|
||||
"back: lastPrev = st → closingViaBack = true → close → = false"
|
||||
)
|
||||
# The single writer sites (no second writer anywhere in the module).
|
||||
assert js.count("lastPrev = st;") == 1
|
||||
assert js.count("closingViaBack = true;") == 1
|
||||
assert handler.count("closingViaBack = false;") == 1
|
||||
|
||||
|
||||
def test_popstate_passes_through_everything_else() -> None:
|
||||
"""REGRESSION: a handler branch firing on entries that are not the
|
||||
modal's (the router's view entries, the drill's ``kb`` entries) or
|
||||
on a modal entry with the modal still open. The forward branch
|
||||
requires ``isModalEntry`` (and the modal hidden) and the back
|
||||
branch requires ``entry`` (falsy when the modal is closed) — and
|
||||
AFTER the back branch closes the handler does NOTHING (the tail is
|
||||
empty): every other popstate falls through untouched."""
|
||||
handler = _popstate_handler(_js())
|
||||
back_i = handler.find("if (!isModalEntry && modalEl && !modalEl.hidden && entry) {")
|
||||
assert back_i != -1, "the back branch (non-modal land + modal open + entry) is gone"
|
||||
brace_i = handler.find("{", back_i)
|
||||
back_block = _block(handler, back_i)
|
||||
tail = handler[brace_i + len(back_block) :]
|
||||
# The only thing after the back branch is the handler's own closing
|
||||
# brace — no code runs on any other popstate (the pass-through).
|
||||
assert tail.strip() in ("", "}"), (
|
||||
"no code after the back branch — the pass-through must be a no-op"
|
||||
)
|
||||
|
||||
|
||||
# ---------- the no-leak guarantee: the closed set of history.* sites ----------
|
||||
|
||||
|
||||
def test_history_call_sites_are_the_closed_set() -> None:
|
||||
"""REGRESSION: a ``history.back()``/``history.go()`` introduction,
|
||||
or ANY untracked ``history.*`` call site. The browser owns its
|
||||
entries (the phase-99 D2 convention): the module's ONLY
|
||||
``history.`` sites are the one ``pushState`` (the open), the two
|
||||
``replaceState`` (the defensive re-open, the non-back close), and
|
||||
the read-only ``history.state`` accesses."""
|
||||
js = _js()
|
||||
sites = re.findall(r"history\.\w+", js)
|
||||
assert sorted(set(sites)) == [
|
||||
"history.pushState",
|
||||
"history.replaceState",
|
||||
"history.state",
|
||||
], f"an untracked history.* site slipped in: {sorted(set(sites))}"
|
||||
assert sites.count("history.pushState") == 1
|
||||
assert sites.count("history.replaceState") == 2
|
||||
assert "history.back(" not in js, "the browser owns its entries — no history.back()"
|
||||
assert "history.go(" not in js, "the browser owns its entries — no history.go()"
|
||||
|
||||
|
||||
# ---------- the pre-phase contract a regression could plausibly break ----------
|
||||
|
||||
|
||||
def test_pre_phase_contract_pins_still_hold() -> None:
|
||||
"""The phase-26 contracts the entry logic sits beside (a regression
|
||||
in the rework must not break them): the Escape capture closes via
|
||||
``closeDocumentModal()``; the per-open keydown capture is added on
|
||||
open and removed on close (document-level, capture phase); the
|
||||
``fetchSeq`` stale-guard lines survive (both checkpoints in the
|
||||
fetch chain + the close bump); the trigger-focus restore in the
|
||||
close (best-effort, contained in the DOM)."""
|
||||
js = _js()
|
||||
keydown = _fn(js, "onModalKeydown")
|
||||
esc_i = keydown.find('if (e.key === "Escape") {')
|
||||
prevent_i = keydown.find("e.preventDefault();", esc_i)
|
||||
close_i = keydown.find("closeDocumentModal();", prevent_i)
|
||||
assert -1 < esc_i < prevent_i < close_i, "Escape: prevent + closeDocumentModal()"
|
||||
open_body = _fn(js, "openDocumentModal")
|
||||
close_body = _fn(js, "closeDocumentModal")
|
||||
assert (
|
||||
'document.addEventListener("keydown", onModalKeydown, true);' in open_body
|
||||
), "the per-open capture is added on open (document-level, capture)"
|
||||
assert (
|
||||
'document.removeEventListener("keydown", onModalKeydown, true);' in close_body
|
||||
), "the per-open capture is removed on close"
|
||||
# The fetchSeq stale guard: the sequence, both checkpoints, the
|
||||
# catch checkpoint, and the close bump (any in-flight fetch is
|
||||
# stale after a close).
|
||||
assert open_body.count("const seq = ++fetchSeq;") == 1
|
||||
assert open_body.count("if (seq !== fetchSeq) return;") == 2, (
|
||||
"both render checkpoints (pre- and post-json) must stay stale-guarded"
|
||||
)
|
||||
assert "if (seq === fetchSeq) renderNotFound(path);" in open_body
|
||||
assert "fetchSeq += 1;" in close_body
|
||||
# The trigger-focus restore (best-effort: contained in the DOM).
|
||||
assert (
|
||||
"triggerEl && typeof triggerEl.focus === \"function\" && document.contains(triggerEl)"
|
||||
in close_body
|
||||
)
|
||||
assert "triggerEl.focus({ preventScroll: true });" in close_body
|
||||
assert "triggerEl = null;" in close_body
|
||||
Reference in New Issue
Block a user