phase: 111_chat_banner_retry

## Phase 111 Completion Report **Implemented/Verified:** - `#kb-banner` contains a `<button type="button" class="banner-retry" id="banner-retry">` (hidden by default, Retry label + SVG) - `showErrorBanner(detail, opts)` reveals the button only when `opts.retryable` is true AND `lastBrainWrap` exists - Turn-error path passes `{ retryable: true }`; all non-turn callers (share, save-doc, stale) remain text-only - `clearErrorBanner()` re-hides the button - `ERROR_HINT` changed from "Try again — …" to "If this persists, check the LLM is reachable." - `.banner-retry` CSS styled as a pill (matching `.stale-regenerate` family) - 12 source-assertion unit tests in `tests/unit/test_frontend_banner_retry.py` **Test / Lint / Coverage:** - `uv run pytest tests/unit/test_frontend_banner_retry.py -v --no-cov` → 12 passed - `uv run pytest --cov=app --cov-report=term-missing` → 2362 passed, 99% coverage - `uv run ruff check .` → All checks passed - `uv run pyright` → 0 errors - `tests/e2e/test_llm_retry.py` → 4 passed (in isolation) - `tests/e2e/test_smoke.py` → 3 passed (in isolation) **Completion Criteria:** - ✅ Retry button visible after failed chat turn, re-runs last question - ✅ Non-turn callers show text-only banner (no button) - ✅ pytest green, coverage >90%, ruff + pyright clean - ✅ Phase dir to be moved by pipeline gate **Next pending phase:** `112_honesty_gate_weak_hits`
2026-09-14 23:07:56 -04:00
parent f37c517590
commit 2683128876
17 changed files with 697 additions and 4 deletions
@@ -0,0 +1,270 @@
+"""Unit: the banner Retry button contract (phase 111).
+
+The banner Retry button was added in task 01 of phase 111.  This module
+pins the markup, the handler wiring, the reveal condition, and the new
+hint copy so that a silent regression is caught without a browser.
+
+House convention: source-assertion style (``tests/unit/test_frontend_*.py``).
+"""
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
+APP_JS = FRONTEND / "assets" / "app.js"
+STYLES_CSS = FRONTEND / "assets" / "styles.css"
+
+
+def _js() -> str:
+    return APP_JS.read_text(encoding="utf-8")
+
+
+def _css() -> str:
+    return STYLES_CSS.read_text(encoding="utf-8")
+
+
+def _html() -> str:
+    return (FRONTEND / "index.html").read_text(encoding="utf-8")
+
+
+# ---------- markup (index.html) ----------
+
+
+def test_banner_retry_button_exists_hidden_and_typed() -> None:
+    """The banner contains a real <button> with id=banner-retry,
+    type=button, and the hidden attribute (hidden by default)."""
+    html = _html()
+    # Must be inside the #kb-banner block.
+    banner_start = html.find('id="kb-banner"')
+    assert banner_start != -1, "#kb-banner must exist"
+    banner_block = html[banner_start : html.find("</div>", banner_start) + 6]
+    assert 'id="banner-retry"' in banner_block, (
+        "#banner-retry must be inside #kb-banner"
+    )
+    assert 'type="button"' in banner_block, (
+        "the button must be type=button"
+    )
+    assert re.search(r'id="banner-retry"\s+hidden', banner_block) or re.search(
+        r'hidden\s+id="banner-retry"', banner_block
+    ), "#banner-retry must carry the hidden attribute"
+
+
+def test_banner_retry_button_has_visible_retry_label() -> None:
+    """The button contains a <span>Retry</span> so screen readers
+    and visual users see the label."""
+    html = _html()
+    banner_start = html.find('id="kb-banner"')
+    banner_block = html[banner_start : html.find("</div>", banner_start) + 6]
+    # The button has an SVG (decoration) and a <span>Retry</span> (label).
+    btn_start = banner_block.find('id="banner-retry"')
+    btn_end = banner_block.find("</button>", btn_start)
+    btn = banner_block[btn_start : btn_end + 9]
+    assert '<span>Retry</span>' in btn, "the button must have a visible 'Retry' label"
+    assert 'aria-hidden="true"' in btn, "the SVG is decoration (aria-hidden)"
+
+
+def test_banner_retry_css_exists() -> None:
+    """.banner-retry must be styled (same pill family as .stale-regenerate)."""
+    css = _css()
+    assert ".banner-retry {" in css, "styles.css must define .banner-retry"
+    block = re.search(r"\.banner-retry \{([\s\S]*?)\n\}", css)
+    assert block, ".banner-retry block must be parseable"
+    body = block.group(1)
+    assert "background:" in body, "must have a background color"
+    assert "color:" in body, "must have text color"
+    assert "border-radius:" in body or "border-radius" in body.replace(" ", ""), (
+        "pill shape — border-radius"
+    )
+
+
+# ---------- ERROR_HINT copy (app.js) ----------
+
+
+def test_error_hint_no_longer_mimics_button() -> None:
+    """ERROR_HINT must not start with 'Try again' — that action
+    moves to the button.  The new hint is informational only."""
+    js = _js()
+    hint_match = re.search(
+        r'const ERROR_HINT\s*=\s*"([^"]*)"', js
+    )
+    assert hint_match, "ERROR_HINT constant must exist"
+    hint = hint_match.group(1)
+    assert not hint.startswith("Try again"), (
+        f"ERROR_HINT must not start with 'Try again' — got: {hint!r}"
+    )
+    assert "check the LLM is reachable" in hint, (
+        "the hint must guide the user to check LLM reachability"
+    )
+
+
+# ---------- showErrorBanner wiring (app.js) ----------
+
+
+def test_show_error_banner_accepts_retryable_opt() -> None:
+    """showErrorBanner(detail, opts={}) must accept an optional
+    retryable flag that reveals the button."""
+    js = _js()
+    fn = js.find("function showErrorBanner")
+    assert fn != -1, "showErrorBanner must exist"
+    body = _extract_fn_body(js, fn)
+    assert "opts.retryable" in body, (
+        "showErrorBanner must check opts.retryable"
+    )
+    assert "banner-retry" in body, (
+        "showErrorBanner must reference #banner-retry"
+    )
+    assert 'btn.hidden = false' in body or "btn.hidden=false" in body, (
+        "the button must be revealed (hidden=false) when retryable"
+    )
+
+
+def test_show_error_banner_wires_retryLastTurn() -> None:
+    """When retryable is true AND lastBrainWrap exists, the button's
+    click handler calls retryLastTurn(lastBrainWrap)."""
+    js = _js()
+    fn = js.find("function showErrorBanner")
+    body = _extract_fn_body(js, fn)
+    assert "retryLastTurn(lastBrainWrap)" in body, (
+        "the click handler must call retryLastTurn(lastBrainWrap)"
+    )
+
+
+def test_show_error_banner_checks_last_brain_wrap() -> None:
+    """The button is revealed ONLY when both opts.retryable AND
+    lastBrainWrap are truthy — no button for callers without a
+    retryable bubble."""
+    js = _js()
+    fn = js.find("function showErrorBanner")
+    body = _extract_fn_body(js, fn)
+    # The reveal must be guarded by the lastBrainWrap check.
+    assert "lastBrainWrap" in body, (
+        "showErrorBanner must check lastBrainWrap before revealing"
+    )
+
+
+def test_turn_error_path_passes_retryable() -> None:
+    """The UI state-machine's turn-error path (setUiState →
+    UI_STATE.error) must pass { retryable: true } to showErrorBanner."""
+    js = _js()
+    # The setUiState function handles the error transition.
+    set_fn = js.find("export function setUiState")
+    assert set_fn != -1, "setUiState must exist"
+    body = _extract_fn_body(js, set_fn)
+    assert "showErrorBanner(errorDetail" in body, (
+        "setUiState must call showErrorBanner with errorDetail"
+    )
+    assert "retryable: true" in body, (
+        "the turn-error path must pass retryable: true"
+    )
+
+
+def test_non_turn_callers_do_not_pass_retryable() -> None:
+    """Non-turn callers (save-doc, share, stale-chat) must NOT
+    pass retryable — they get text-only banners."""
+    js = _js()
+    # Find the setUiState function body.
+    set_fn = js.find("export function setUiState")
+    assert set_fn != -1, "setUiState must exist"
+    set_body = _extract_fn_body(js, set_fn)
+    # The turn-error path inside setUiState passes retryable.
+    assert "retryable: true" in set_body, (
+        "the turn-error path in setUiState must pass retryable: true"
+    )
+    # Now check that NO other showErrorBanner call (outside setUiState)
+    # passes retryable.
+    # The function's opening brace is the first `{` after set_fn.
+    set_brace = js.index("{", set_fn)
+    set_end = set_brace + len(set_body) + 2  # +2 for opening + closing braces
+    # Search for retryable outside setUiState.
+    after_set = js[set_end:]
+    # Also check before setUiState.
+    before_set = js[: set_fn]
+    for section in (before_set, after_set):
+        for m in re.finditer(r'showErrorBanner\(([^)]*)\)', section):
+            assert "retryable" not in m.group(1), (
+                f"Non-turn callers must not pass retryable: {m.group(0)}"
+            )
+
+
+def test_clear_error_banner_re_hides_button() -> None:
+    """clearErrorBanner must re-hide the banner Retry button."""
+    js = _js()
+    fn = js.find("function clearErrorBanner")
+    assert fn != -1, "clearErrorBanner must exist"
+    body = _extract_fn_body(js, fn)
+    assert "banner-retry" in body, (
+        "clearErrorBanner must reference #banner-retry"
+    )
+    assert 'btn.hidden = true' in body or "btn.hidden=true" in body, (
+        "clearErrorBanner must re-hide the button"
+    )
+
+
+# ---------- banner role attribute ----------
+
+
+def test_banner_role_alert_on_error() -> None:
+    """The banner must get role=alert when shown as an error."""
+    js = _js()
+    fn = js.find("function showErrorBanner")
+    body = _extract_fn_body(js, fn)
+    assert '"role", "alert"' in body, (
+        "showErrorBanner must set role=alert on the banner"
+    )
+
+
+# ---------- CSS polish ----------
+
+
+def test_banner_retry_hover_and_focus_visible() -> None:
+    """.banner-retry must have hover styles; :focus-visible rides the
+    global ring (WCAG AA)."""
+    css = _css()
+    assert re.search(
+        r"\.banner-retry:hover\s*\{", css
+    ), ".banner-retry must have a hover state"
+    # :focus-visible is handled by the global rule — the .banner-retry
+    # block itself does not need a dedicated :focus-visible (consistent
+    # with .stale-regenerate).
+    assert ":focus-visible" in css, (
+        "a global :focus-visible rule must exist for WCAG AA"
+    )
+
+
+# ---------- helpers ----------
+
+
+def _extract_fn_body(js: str, fn_start: int) -> str:
+    """Extract the body of a function starting at *fn_start* in *js*.
+
+    Returns the text between the opening ``{`` and the matching closing ``}``.
+    Skips empty object literals like ``{}`` in parameter defaults.
+    """
+    # Find the real opening brace: skip empty object literals ``{}``.
+    pos = fn_start
+    brace = -1
+    while pos < len(js):
+        idx = js.find("{", pos)
+        if idx == -1:
+            break
+        # Check if this is an empty literal ``{}``
+        next_non_ws = idx + 1
+        while next_non_ws < len(js) and js[next_non_ws] in (" ", "\t", "\n", "\r"):
+            next_non_ws += 1
+        if next_non_ws < len(js) and js[next_non_ws] == "}":
+            # Empty literal — skip past it
+            pos = next_non_ws + 1
+            continue
+        brace = idx
+        break
+    assert brace != -1, f"no opening brace found after position {fn_start}"
+    depth = 1
+    i = brace + 1
+    while i < len(js) and depth > 0:
+        if js[i] == "{":
+            depth += 1
+        elif js[i] == "}":
+            depth -= 1
+        i += 1
+    return js[brace + 1 : i - 1]