various fixes
This commit is contained in:
@@ -6,27 +6,25 @@ toggle) to ANONYMOUS visitors — it shipped VISIBLE in all six pages'
|
||||
markup and ``assets/header.js`` removed it only after ``/api/whoami``
|
||||
resolved, so the button flashed for the whole whoami round-trip.
|
||||
|
||||
The fix mirrors the admin-only nav links (phase 19/29/35 contract):
|
||||
the toggle now SHIPS ``hidden`` in every page and ``initSharedHeader``
|
||||
unhides it only when whoami says admin; the anonymous end-state is
|
||||
unchanged (toggle + panel REMOVED from the DOM — phase 16 "absent,
|
||||
not hidden"). This suite proves the browser-level contract:
|
||||
Phase 40 fixed it with the ship-hidden / reveal-for-admin contract
|
||||
(the admin-only nav links). The owner then asked for the button to go
|
||||
away entirely (2026-08-28): the navbar ``#steering-toggle`` is now
|
||||
ABSENT from every page — for the admin AND anonymous — so the
|
||||
never-visible contract holds by construction, and note management
|
||||
lives on ``/tuning.html``. The anonymous end-state is unchanged: the
|
||||
``#steering-panel`` is REMOVED from the DOM (phase 16 "absent, not
|
||||
hidden") and ``/api/steering`` is never fetched. This suite proves the
|
||||
browser-level contract:
|
||||
|
||||
* a MutationObserver (installed via ``add_init_script`` before any
|
||||
page code runs) records every frame in which ``#steering-toggle``
|
||||
is both in the DOM and visible (``offsetParent !== null`` or
|
||||
``!hidden``) — an anonymous load records ZERO such frames, on
|
||||
every page, from first paint to the settled state;
|
||||
* after the whoami round-trip the toggle is ABSENT from the DOM for
|
||||
anonymous visitors (removed, not hidden);
|
||||
* the admin UX is untouched (phase 15/34 behavior): the toggle is
|
||||
revealed, clicking opens ``#steering-panel``
|
||||
(``aria-expanded="true"``) and the count badge matches the note
|
||||
list — including a self-check that the observer records the admin
|
||||
reveal, so the zero-frame anonymous assertions are not vacuous;
|
||||
* the ship-hidden nav-link contract this phase relies on
|
||||
(``#nav-sources`` / ``#nav-git-sources`` / ``#nav-tuning``) is
|
||||
intact: hidden for anonymous, revealed for admin.
|
||||
* ``#steering-toggle`` is absent from the DOM on every page, for
|
||||
anonymous AND admin — nothing can flash, because nothing ships;
|
||||
* the anonymous end-state survives the removal: ``#steering-panel``
|
||||
is removed from the DOM, not just hidden;
|
||||
* the admin UX around the removal is intact: the header panel section
|
||||
still ships hidden (never opened from the header anymore), and the
|
||||
ship-hidden nav-link contract this phase relies on
|
||||
(``#nav-sources`` / ``#nav-git-sources`` / ``#nav-tuning``) holds:
|
||||
hidden for anonymous, revealed for admin.
|
||||
|
||||
Story: ``.agent/user_stories/tuning-toggle-flash.md``
|
||||
Run in isolation (DB must be up: ``podman compose up -d db``):
|
||||
@@ -42,82 +40,27 @@ Test → story mapping (Playwright Mapping Rule):
|
||||
from __future__ import annotations
|
||||
|
||||
from playwright.sync_api import Page, expect
|
||||
from sqlalchemy import text
|
||||
|
||||
from app.db import SessionLocal
|
||||
from e2e.auth_helpers import login
|
||||
|
||||
#: The pages the contract must hold on besides the chat page (mapping
|
||||
#: rule 2). document.html / git-sources.html are covered by the
|
||||
#: source-level unit pins (tests/unit/test_steering_toggle_visibility.py)
|
||||
#: — the four pages here are the ones an anonymous visitor actually
|
||||
#: lands on.
|
||||
#: source-level unit pins (tests/unit/test_steering_toggle_removal.py)
|
||||
#: — the three pages here are the other pages an anonymous visitor
|
||||
#: actually lands on.
|
||||
OTHER_PAGES = ("/sources.html", "/tuning.html", "/login.html")
|
||||
|
||||
ADMIN_NOTE = "PHASE40-E2E note — badge check"
|
||||
|
||||
#: Admin-only nav links — the ship-hidden / reveal-for-admin family the
|
||||
#: steering toggle now belongs to (phase 19/29/35 contract).
|
||||
#: steering toggle belonged to (phase 19/29/35 contract).
|
||||
NAV_IDS = ("#nav-sources", "#nav-git-sources", "#nav-tuning")
|
||||
|
||||
#: Runs in every new document BEFORE any page script (addInitScript):
|
||||
#: arms a MutationObserver over the whole DOM and records every frame
|
||||
#: in which #steering-toggle is attached AND visible — visible meaning
|
||||
#: rendered (offsetParent !== null) OR carrying no [hidden] attribute
|
||||
#: (!el.hidden). A shipped-VISIBLE toggle (the old bug) is recorded the
|
||||
#: moment the parser inserts it; a shipped-hidden toggle that is later
|
||||
#: revealed is recorded at the reveal mutation. The array is fresh per
|
||||
#: document, so each navigation asserts its own frames.
|
||||
VISIBILITY_OBSERVER_JS = """
|
||||
window.__tuningVisibleFrames = [];
|
||||
(() => {
|
||||
const visible = (el) =>
|
||||
!!el && el.isConnected && (el.offsetParent !== null || !el.hidden);
|
||||
const check = () => {
|
||||
if (visible(document.getElementById("steering-toggle"))) {
|
||||
window.__tuningVisibleFrames.push({
|
||||
at: Math.round(performance.now()),
|
||||
href: location.pathname,
|
||||
});
|
||||
}
|
||||
};
|
||||
const start = () => {
|
||||
check();
|
||||
new MutationObserver(check).observe(document.documentElement, {
|
||||
childList: true,
|
||||
subtree: true,
|
||||
attributes: true,
|
||||
attributeFilter: ["hidden"],
|
||||
});
|
||||
};
|
||||
if (document.documentElement) start();
|
||||
else document.addEventListener("DOMContentLoaded", start);
|
||||
})();
|
||||
"""
|
||||
|
||||
|
||||
def install_visibility_observer(page: Page) -> None:
|
||||
"""Arm the never-visible frame counter on every document this page
|
||||
creates — the initial load, the post-login redirect, re-gotos."""
|
||||
page.add_init_script(VISIBILITY_OBSERVER_JS)
|
||||
|
||||
|
||||
def visible_frames(page: Page) -> list[dict[str, object]]:
|
||||
"""The frames the observer recorded in the CURRENT document."""
|
||||
return page.evaluate("() => window.__tuningVisibleFrames || []")
|
||||
|
||||
|
||||
def _assert_no_flash(page: Page, path: str) -> None:
|
||||
"""Zero visible frames + the phase-16 absent end-state, for one
|
||||
anonymously loaded page."""
|
||||
frames = visible_frames(page)
|
||||
assert frames == [], f"{path}: the toggle was visible {len(frames)}x: {frames!r}"
|
||||
def _assert_no_toggle(page: Page, path: str) -> None:
|
||||
"""The navbar toggle is absent from the DOM for one page — nothing
|
||||
to flash, for either role."""
|
||||
assert page.locator("#steering-toggle").count() == 0, (
|
||||
f"{path}: #steering-toggle must be REMOVED from the DOM for "
|
||||
"anonymous (phase 16 'absent, not hidden')"
|
||||
)
|
||||
assert page.locator("#steering-panel").count() == 0, (
|
||||
f"{path}: #steering-panel must be removed together with the toggle"
|
||||
f"{path}: #steering-toggle must be ABSENT from the DOM "
|
||||
"(removed from the navbar at owner request, 2026-08-28)"
|
||||
)
|
||||
|
||||
|
||||
@@ -125,14 +68,21 @@ def _wait_header_settled_anonymous(page: Page) -> None:
|
||||
"""Whoami resolved on the current document: the anonymous state
|
||||
reveals the Sign in link and keeps Sign out hidden (the pair is
|
||||
decided by the SAME initSharedHeader pass that removes the
|
||||
toggle)."""
|
||||
steering panel)."""
|
||||
page.wait_for_load_state("networkidle")
|
||||
expect(page.locator("#sign-in-link")).to_be_visible(timeout=15_000)
|
||||
expect(page.locator("#sign-out-btn")).to_be_hidden()
|
||||
|
||||
|
||||
def _wait_header_settled_admin(page: Page) -> None:
|
||||
"""Whoami resolved for a signed-in admin on the current document."""
|
||||
page.wait_for_load_state("networkidle")
|
||||
expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
|
||||
expect(page.locator("#sign-in-link")).to_be_hidden()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 1. Anonymous chat load: zero visible frames, toggle absent afterwards
|
||||
# 1. Anonymous chat load: zero toggle, panel removed (never visible)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -140,14 +90,19 @@ def test_anonymous_never_sees_toggle(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
page.set_default_timeout(30_000)
|
||||
install_visibility_observer(page)
|
||||
page.goto(app_url + "/")
|
||||
_wait_header_settled_anonymous(page)
|
||||
_assert_no_flash(page, "/")
|
||||
_assert_no_toggle(page, "/")
|
||||
# The phase-16 end-state survives the removal: the panel is REMOVED
|
||||
# from the DOM, not just hidden.
|
||||
assert page.locator("#steering-panel").count() == 0, (
|
||||
"/: #steering-panel must be removed from the DOM for anonymous "
|
||||
"(phase 16 'absent, not hidden')"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 2. Anonymous loads of the other pages: same zero-flash contract
|
||||
# 2. Anonymous loads of the other pages: same no-toggle contract
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -155,15 +110,19 @@ def test_anonymous_other_pages_never_flash(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
page.set_default_timeout(30_000)
|
||||
install_visibility_observer(page)
|
||||
for path in OTHER_PAGES:
|
||||
page.goto(app_url + path)
|
||||
_wait_header_settled_anonymous(page)
|
||||
_assert_no_flash(page, path)
|
||||
_assert_no_toggle(page, path)
|
||||
assert page.locator("#steering-panel").count() == 0, (
|
||||
f"{path}: #steering-panel must be removed from the DOM for "
|
||||
"anonymous (phase 16 'absent, not hidden')"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 3. Admin: revealed, clickable, count badge matches the list
|
||||
# 3. Admin: the toggle is gone too — the panel still ships hidden and
|
||||
# the nav links are revealed
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -171,60 +130,26 @@ def test_admin_toggle_revealed_and_working(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
page.set_default_timeout(30_000)
|
||||
with SessionLocal() as db:
|
||||
db.execute(text("TRUNCATE steering_notes"))
|
||||
db.commit()
|
||||
|
||||
install_visibility_observer(page)
|
||||
login(page, app_url, next="/")
|
||||
page.wait_for_load_state("networkidle")
|
||||
expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
|
||||
_wait_header_settled_admin(page)
|
||||
|
||||
toggle = page.locator("#steering-toggle")
|
||||
panel = page.locator("#steering-panel")
|
||||
|
||||
# Revealed — the [hidden] attribute is gone and the button renders.
|
||||
expect(toggle).to_be_visible()
|
||||
assert toggle.get_attribute("hidden") is None
|
||||
|
||||
# Observer self-check: the reveal IS a recorded visible frame, so
|
||||
# the zero-frame anonymous assertions above cannot be vacuous.
|
||||
assert visible_frames(page) != [], "the admin reveal was not observed"
|
||||
|
||||
# Click: the panel opens, aria-expanded tracks it (phase 15/34).
|
||||
toggle.click()
|
||||
expect(panel).to_be_visible()
|
||||
expect(toggle).to_have_attribute("aria-expanded", "true")
|
||||
|
||||
# Count badge matches the list — first the empty state…
|
||||
expect(page.locator("#steering-count")).to_have_text("0")
|
||||
expect(page.locator("#steering-list .steering-note")).to_have_count(0)
|
||||
expect(page.locator("#steering-empty")).to_be_visible()
|
||||
|
||||
# …then with one note created through the real admin API.
|
||||
page.evaluate(
|
||||
"""async (note) => {
|
||||
const r = await fetch("/api/steering", {
|
||||
method: "POST",
|
||||
headers: {"Content-Type": "application/json"},
|
||||
body: JSON.stringify({note}),
|
||||
});
|
||||
if (!r.ok) throw new Error("steering POST failed: " + r.status);
|
||||
}""",
|
||||
ADMIN_NOTE,
|
||||
)
|
||||
toggle.click() # close
|
||||
toggle.click() # re-open (refreshes the list)
|
||||
expect(panel).to_be_visible()
|
||||
expect(toggle).to_have_attribute("aria-expanded", "true")
|
||||
expect(page.locator("#steering-count")).to_have_text("1")
|
||||
expect(page.locator("#steering-list .steering-note")).to_have_count(1)
|
||||
expect(page.locator("#steering-list .steering-note-text")).to_have_text(ADMIN_NOTE)
|
||||
# The navbar toggle was removed at owner request (2026-08-28) —
|
||||
# absent for the admin as well; nothing in the header can flash
|
||||
# it back, because nothing ships it anymore.
|
||||
_assert_no_toggle(page, "/")
|
||||
# The header panel section survives (kept fresh by the chat page's
|
||||
# per-bubble Tune form) but ships hidden — no header control opens
|
||||
# it anymore; note management lives on /tuning.html.
|
||||
assert page.locator("#steering-panel").count() == 1
|
||||
expect(page.locator("#steering-panel")).to_be_hidden()
|
||||
# The surviving path to the notes: the admin-only Tuning nav link
|
||||
# is revealed.
|
||||
expect(page.locator("#nav-tuning")).to_be_visible()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 4. Nav-contract regression (phase 19/34): the ship-hidden family the
|
||||
# toggle now belongs to is intact
|
||||
# toggle used to belong to is intact
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -245,9 +170,8 @@ def test_nav_contract_regression(
|
||||
)
|
||||
|
||||
# Admin: the same links are revealed — the exact contract the
|
||||
# steering toggle now mirrors.
|
||||
# steering toggle mirrored before it was removed.
|
||||
login(page, app_url, next="/")
|
||||
page.wait_for_load_state("networkidle")
|
||||
expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
|
||||
_wait_header_settled_admin(page)
|
||||
for nav in NAV_IDS:
|
||||
expect(page.locator(nav)).to_be_visible()
|
||||
|
||||
Reference in New Issue
Block a user