phase: 153_document_viewer_context
That last `--co` (collect-only) run was my probe mistake — it shows 34% because no tests execute under collect-only. The valid coverage run was the real gate command executed earlier: `uv run pytest --cov=app --cov-report=term-missing` → **TOTAL 99%** (exit 0, all 3544 tests executed). All gates are verified. ## Phase 153 — final verification pass report **Verified (all tasks 01–04 complete in `complete/`):** - Modal `fullPageUrl(source, path, back = null)` + `openDocumentModal(…, back = null)` with module-level `currentBack`; 5 chat call sites pass `"/"`, sources.js call site unchanged (byte-identical Sources path); `documentUrl` comment now true - Copy-path control (shared `renderDocument` core, icon+text, text = accessible name) on both surfaces; per-page clipboard helpers with the fallback-field + status-line contract; `#doc-copy-announcer` live region; styles + 44px touch floor - Unit pins: `&back=%2F` / no-`back` byte-identity, `currentBack` single-writer guard, copy strings byte-pinned, phase-13 regression pins untouched **Test / lint / coverage results:** - `uv run pytest` → **3544 passed**, 0 failed (full unit + integration) - `uv run pytest --cov=app --cov-report=term-missing` → **TOTAL 99%** (>90% gate) - `uv run pytest tests/e2e/test_document_viewer_context.py -v --no-cov` → **8 passed** in 47.33s, in isolation (DB up, mock LLM) - `uv run ruff check . && uv run pyright` → clean (0 errors/warnings) **Completion criteria:** all met — (1) chat→modal→Full page: `href="/"` + "Chat"; Sources→modal→Full page: `href="/sources.html"` + "Sources" with `back=` asserted absent; (2) no-`back` URLs byte-identical to pre-phase; (3) Copy path: "Path copied." on success (clipboard readback), prefilled select-on-focus fallback field + "Path ready — copy it from the field." on rejected clipboard, both surfaces parameterized; (4) copied string exactly `docs/homelab/kubernetes.md`; (5) full suite green, E2E isolated green, lint clean, coverage 99%; (6) commit by harness. **Documentation:** README.md reviewed in full — phase-153 feature already named in the Chat section; routes, env reference, workflows, deployment all consistent — no changes needed. AGENTS.md reviewed in full — rules, gates, and quick-reference commands verified against the repo (incl. `test_smoke.py`, `.env.example`, gitignore entries) — consistent, no changes needed. **Notable:** pytest 9.1.1 quirk — a doubled `-q` (pyproject `addopts` + explicit `-q`) swallows the final "N passed" summary line while still exiting 0; the plain `uv run pytest` gate command prints it. No defects found in prior work. **Next pending phase:** `154_sync_status_freshness`.
This commit is contained in:
@@ -32,15 +32,56 @@ source-level contract a silent regression would break:
|
||||
``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).
|
||||
|
||||
Phase 153 (task 01) — the "Full page" escape hatch carries the origin
|
||||
``back``; the module's added pins:
|
||||
|
||||
* a second ``currentBack`` declaration / a reset that does not land at
|
||||
the top of EVERY open (before the no-skeleton guard — the fallback
|
||||
path reads the stored value);
|
||||
* ``fullPageUrl`` appending ``&back=`` unconditionally (a null/absent
|
||||
``back`` must keep the URL byte-identical — the Sources path);
|
||||
* the ``openDocumentModal`` ``back`` default drifting from ``null``
|
||||
(the Sources table's 3-arg call must stay byte-identical);
|
||||
* a ``fullPageUrl`` call site dropping the stored ``back`` (the
|
||||
no-skeleton fallback and the anchor must both read ``currentBack``).
|
||||
|
||||
Phase 153 (task 03) — unit pins for the URL contract (additions on top
|
||||
of task 01's source pins; the pre-phase contract is the load-bearing
|
||||
byte-identity):
|
||||
|
||||
* ``fullPageUrl`` behavior under node — ``back = null`` and an ABSENT
|
||||
third arg are byte-identical to each other AND to the exact
|
||||
pre-phase string (no ``&back=`` anywhere — the Sources path);
|
||||
``back = "/"`` appends exactly ``&back=%2F`` to that same string;
|
||||
* ``openDocumentModal`` behavior under node — the no-skeleton
|
||||
``window.open`` fallback carries the stored ``back`` (a chat open's
|
||||
``"/"`` lands in the URL, an omitted ``back`` keeps it byte-
|
||||
identical) and the ``#doc-modal-open`` ("Full page") anchor carries
|
||||
it on a skeleton open (chat → ``&back=%2F``; a Sources-style open
|
||||
→ byte-identical, no param).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
|
||||
MODAL_JS = FRONTEND / "assets" / "document-modal.js"
|
||||
|
||||
HAVE_NODE = shutil.which("node") is not None
|
||||
|
||||
|
||||
def _run_node(script: str) -> str:
|
||||
proc = subprocess.run(["node", "-e", script], capture_output=True, text=True, timeout=60)
|
||||
assert proc.returncode == 0, f"node failed: {proc.stderr}"
|
||||
return proc.stdout
|
||||
|
||||
|
||||
def _js() -> str:
|
||||
return MODAL_JS.read_text(encoding="utf-8")
|
||||
@@ -168,7 +209,9 @@ def test_window_open_fallback_precedes_the_entry_logic() -> None:
|
||||
``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");')
|
||||
open_i = open_body.find(
|
||||
'window.open(fullPageUrl(source, path, currentBack), "_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, (
|
||||
@@ -358,3 +401,219 @@ def test_pre_phase_contract_pins_still_hold() -> None:
|
||||
)
|
||||
assert "triggerEl.focus({ preventScroll: true });" in close_body
|
||||
assert "triggerEl = null;" in close_body
|
||||
|
||||
|
||||
# ---------- phase 153 (task 01): the "Full page" hatch carries the origin ----------
|
||||
|
||||
|
||||
def test_currentback_declared_exactly_once_and_reset_at_the_top_of_every_open() -> None:
|
||||
"""REGRESSION: a second ``currentBack`` writer, or a reset that
|
||||
does not land at the top of EVERY open. ``currentBack`` is
|
||||
declared by EXACTLY ONE ``let``, and the open resets it
|
||||
(``currentBack = back;``) BEFORE the no-skeleton guard — so even
|
||||
the ``window.open`` fallback path (which early-returns there)
|
||||
reads the stored value of THIS open."""
|
||||
js = _js()
|
||||
assert js.count("let currentBack = null;") == 1
|
||||
assert len(re.findall(r"let\s+currentBack\s*=", js)) == 1
|
||||
open_body = _fn(js, "openDocumentModal")
|
||||
reset_i = open_body.find("currentBack = back;")
|
||||
guard_i = open_body.find("if (!modalEl || !titleEl || !metaEl || !contentEl) {")
|
||||
assert -1 < reset_i < guard_i, (
|
||||
"the reset must land at the TOP of every open — before the "
|
||||
"no-skeleton guard (the fallback path reads the stored value)"
|
||||
)
|
||||
# Exactly one reset site (no second writer anywhere in the module).
|
||||
assert js.count("currentBack = back;") == 1
|
||||
|
||||
|
||||
def test_fullpageurl_appends_encoded_back_only_when_truthy() -> None:
|
||||
"""REGRESSION: the ``back`` param leaking into a null/absent open
|
||||
(the Sources path must stay byte-identical — the viewer's
|
||||
/sources.html default) or the encoding drifting from the
|
||||
app.js::documentUrl contract. The signature is
|
||||
``fullPageUrl(source, path, back = null)`` and the ONLY writer
|
||||
after the base URL is the guarded ``if (back)`` append —
|
||||
percent-encoded, exactly the way documentUrl documents it."""
|
||||
js = _js()
|
||||
assert "function fullPageUrl(source, path, back = null)" in js
|
||||
fn = _fn(js, "fullPageUrl")
|
||||
assert 'if (back) url += "&back=" + encodeURIComponent(back);' in fn
|
||||
# The base URL is built first; the guarded append is the only
|
||||
# ``&back=`` writer (a null back bakes no param into the URL).
|
||||
base_i = fn.find('"/document.html?source=" + encodeURIComponent(')
|
||||
back_i = fn.find('"&back=" + encodeURIComponent(back)', base_i)
|
||||
assert -1 < base_i < back_i, "the back append must follow the base URL"
|
||||
base_part = fn.split('"&back="')[0]
|
||||
assert "back=" not in base_part, (
|
||||
"no &back= may be baked into the base URL (null back = no param)"
|
||||
)
|
||||
|
||||
|
||||
def test_open_document_modal_defaults_back_to_null() -> None:
|
||||
"""REGRESSION: a non-null default for the ``back`` argument.
|
||||
``openDocumentModal(source, path, trigger = null, back = null)`` —
|
||||
a call WITHOUT ``back`` (the Sources table's
|
||||
``openDocumentModal(d.source, d.path, link)``) must leave the
|
||||
"Full page" URL byte-identical to pre-phase (the viewer's
|
||||
/sources.html default — the byte-identity is the contract)."""
|
||||
js = _js()
|
||||
assert (
|
||||
"export function openDocumentModal(source, path, trigger = null, back = null)"
|
||||
in js
|
||||
)
|
||||
|
||||
|
||||
def test_fallback_and_anchor_both_read_the_stored_back() -> None:
|
||||
"""REGRESSION: a call site passing ``back`` positionally from a
|
||||
caller, or dropping the third argument (a dropped arg loses the
|
||||
origin — the chat path would land on "/sources.html"). BOTH the
|
||||
no-skeleton ``window.open`` fallback and the ``#doc-modal-open``
|
||||
("Full page") anchor read ``currentBack``: exactly three
|
||||
``fullPageUrl(`` sites in the module (one definition + two call
|
||||
sites), and no call site without the stored variable."""
|
||||
js = _js()
|
||||
assert 'window.open(fullPageUrl(source, path, currentBack), "_blank", "noopener");' in js
|
||||
assert "openEl.href = fullPageUrl(source, path, currentBack);" in js
|
||||
assert "fullPageUrl(source, path)" not in js, "a dropped third argument loses the back"
|
||||
assert "fullPageUrl(source, path, back)" not in js, ("call sites must read the stored variable")
|
||||
assert js.count("fullPageUrl(") == 3, "1 definition + 2 call sites"
|
||||
|
||||
|
||||
# ---------- phase 153 (task 03): the URL contract, behavior-pinned under node ----------
|
||||
|
||||
|
||||
@pytest.mark.skipif(not HAVE_NODE, reason="node not available")
|
||||
def test_fullpageurl_null_back_is_byte_identical_to_the_absent_and_pre_phase() -> None:
|
||||
"""The URL contract's load-bearing byte-identity, executed under
|
||||
node (the real function, not a re-implementation):
|
||||
|
||||
* ``back = null`` and an ABSENT third argument produce the same
|
||||
string, and that string is byte-identical to the EXACT pre-phase
|
||||
string (no ``&back=`` anywhere — the Sources path must never gain
|
||||
the param);
|
||||
* ``back = "/"`` (a chat open) appends exactly ``&back=%2F`` to
|
||||
that same pre-phase string (the encoding pinned byte-for-byte).
|
||||
|
||||
A refactor that bakes a ``back=`` into the base, reorders the
|
||||
param, or re-encodes the values breaks one of the exact strings."""
|
||||
fn = _fn(_js(), "fullPageUrl")
|
||||
out = _run_node(
|
||||
fn
|
||||
+ "\nconsole.log(fullPageUrl('H omelab', 'a/b.md'));"
|
||||
+ "\nconsole.log(fullPageUrl('H omelab', 'a/b.md', null));"
|
||||
+ "\nconsole.log(fullPageUrl('H omelab', 'a/b.md', '/'));"
|
||||
)
|
||||
absent, null_back, chat = out.splitlines()
|
||||
pre_phase = "/document.html?source=H%20omelab&path=a%2Fb.md"
|
||||
assert null_back == absent, "back = null must be byte-identical to an absent back"
|
||||
assert absent == pre_phase, (
|
||||
"a null/absent back must keep the URL byte-identical to pre-phase"
|
||||
)
|
||||
assert "back=" not in absent, "no &back= may be present for a null/absent back"
|
||||
assert chat == pre_phase + "&back=%2F", (
|
||||
"back = '/' must append exactly &back=%2F to the pre-phase string"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.skipif(not HAVE_NODE, reason="node not available")
|
||||
def test_open_document_modal_no_skeleton_fallback_carries_the_stored_back() -> None:
|
||||
"""The no-skeleton ``window.open`` fallback, executed under node
|
||||
(the real ``openDocumentModal`` with the module state stubbed):
|
||||
a chat open (``back = "/"``) lands in the URL exactly (``%2F``),
|
||||
and a Sources-style open (``back`` omitted — the 3-arg call)
|
||||
keeps the URL byte-identical (no ``&back=`` at all). The
|
||||
fallback is the ``window.open(url, "_blank", "noopener")`` shape,
|
||||
and the early ``return`` means neither open touches the
|
||||
history."""
|
||||
fn = _fn(_js(), "openDocumentModal")
|
||||
url_fn = _fn(_js(), "fullPageUrl")
|
||||
script = (
|
||||
"let currentBack = null;\n"
|
||||
"const modalEl = null, titleEl = null, metaEl = null, contentEl = null, openEl = null;\n"
|
||||
"const opened = [];\n"
|
||||
"globalThis.window = { open: (u, t, f) => opened.push([u, t, f]) };\n"
|
||||
+ url_fn
|
||||
+ "\n"
|
||||
+ fn
|
||||
+ "\nopenDocumentModal('H omelab', 'a/b c.md', null, '/');\n"
|
||||
+ "\nopenDocumentModal('S2', 'p2.md', null);\n"
|
||||
+ "\nconsole.log(JSON.stringify(opened));\n"
|
||||
)
|
||||
out = json.loads(_run_node(script))
|
||||
assert out == [
|
||||
[
|
||||
"/document.html?source=H%20omelab&path=a%2Fb%20c.md&back=%2F",
|
||||
"_blank",
|
||||
"noopener",
|
||||
],
|
||||
# the omitted back (the Sources table's 3-arg call) stays byte-identical
|
||||
["/document.html?source=S2&path=p2.md", "_blank", "noopener"],
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.skipif(not HAVE_NODE, reason="node not available")
|
||||
def test_open_document_modal_full_page_anchor_carries_the_stored_back() -> None:
|
||||
"""The ``#doc-modal-open`` ("Full page") anchor on a skeleton
|
||||
open, executed under node (the real ``openDocumentModal`` with the
|
||||
modal elements + history + fetch chain stubbed): a chat open
|
||||
(``back = "/"``) sets the anchor's ``href`` to the pre-phase string
|
||||
+ exactly ``&back=%2F``; a follow-up Sources-style open (``back``
|
||||
omitted — the defensive re-open-while-open branch, which updates
|
||||
the SAME anchor) resets it to the byte-identical no-param string.
|
||||
The open's first push is a single state-only entry (the phase-144
|
||||
contract stands); the defensive re-open replaces in place (no
|
||||
second push)."""
|
||||
fn = _fn(_js(), "openDocumentModal")
|
||||
url_fn = _fn(_js(), "fullPageUrl")
|
||||
script = (
|
||||
"let currentBack = null;\n"
|
||||
"let triggerEl = null;\n"
|
||||
"let fetchSeq = 0;\n"
|
||||
"let entry = null;\n"
|
||||
"let lastPrev = null;\n"
|
||||
"let closingViaBack = false;\n"
|
||||
"const modalEl = { hidden: true };\n"
|
||||
"const titleEl = {}, metaEl = { replaceChildren() {} };\n"
|
||||
"const contentEl = { replaceChildren() {}, focus() {} };\n"
|
||||
"const anchor = { href: '', hidden: true };\n"
|
||||
"const openEl = anchor;\n"
|
||||
"const hist = [];\n"
|
||||
"globalThis.history = { state: null,"
|
||||
" pushState(s, t) { hist.push(['push', s.modal === 'doc', s.source, s.path, t]); },"
|
||||
" replaceState(s, t) { hist.push(['replace', s.modal === 'doc', s.source, s.path]); } };\n"
|
||||
"globalThis.document = { addEventListener() {}, removeEventListener() {} };\n"
|
||||
"globalThis.fetch = () => Promise.resolve({ ok: true, json: async () => ({}) });\n"
|
||||
"function onModalKeydown() {}\n"
|
||||
"function showLoading() {}\n"
|
||||
"function setDesc() {}\n"
|
||||
"function renderNotFound() {}\n"
|
||||
"function wireCopyPath() {}\n"
|
||||
"function renderDocument() {}\n"
|
||||
'function contentUrl(s, p) { return ""; }\n'
|
||||
+ url_fn
|
||||
+ "\n"
|
||||
+ fn
|
||||
+ "\nopenDocumentModal('H omelab', 'a/b c.md', null, '/');\n"
|
||||
+ "\nconst chatHref = anchor.href;\n"
|
||||
+ "\nopenDocumentModal('S2', 'p2.md', null);\n"
|
||||
+ "\nconsole.log(JSON.stringify({ chatHref, sourcesHref: anchor.href,"
|
||||
+ " visible: !modalEl.hidden, hist }));\n"
|
||||
)
|
||||
out = json.loads(_run_node(script))
|
||||
pre_phase = "/document.html?source=H%20omelab&path=a%2Fb%20c.md"
|
||||
assert out["chatHref"] == pre_phase + "&back=%2F", (
|
||||
"a chat open must set the Full-page anchor to the pre-phase string + exactly &back=%2F"
|
||||
)
|
||||
assert out["sourcesHref"] == "/document.html?source=S2&path=p2.md", (
|
||||
"a Sources-style open (back omitted) must reset the anchor to the byte-"
|
||||
"identical no-param string"
|
||||
)
|
||||
assert out["visible"] is True
|
||||
# phase 144 stands: one state-only push, then the defensive
|
||||
# re-open replaces IN PLACE (no second push).
|
||||
assert out["hist"] == [
|
||||
["push", True, "H omelab", "a/b c.md", ""],
|
||||
["replace", True, "S2", "p2.md"],
|
||||
]
|
||||
|
||||
|
||||
Reference in New Issue
Block a user