From 7700c84e4dd684f0863f960bf8791ebb92bb2b2c Mon Sep 17 00:00:00 2001 From: null Date: Tue, 14 Jul 2026 21:46:24 -0500 Subject: [PATCH] =?UTF-8?q?test(qa):=20add=20nav-scan.sh=20=E2=80=94=20pro?= =?UTF-8?q?ve=20navigation=20dead=20ends=20statically?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QA had no net for navigation. Pass C already mandated "from every screen confirm there's always a way out", and C-NAV-004 still shipped: the pack library drew no app bar and no bottom bar, stranding the user on the system Back gesture. A human walking ~50 routes misses one. A dead end is statically provable, so it should not be hunted by eye. The shell draws chrome from two hand-kept sets — topLevelRoutes (bottom bar) and shellBackRoutes (app bar + back). A route in neither, whose screen also draws no back of its own, has no exit by construction. nav-scan.sh cross-references both sets against each route's screen composable and reports those as CRITICAL, exiting 1 so it can gate. It follows the existing scanner family (theme-scan, painter-xml-scan, wiring-scan) and is wired into Pass C as a pre-check, run before the manual sweep like the theme scan. Proven both ways rather than assumed: it flags QUESTION_PACKS when the fix is reverted, and reports 0 with the fix in place. Scanning the whole route table found no other dead ends. Excludes the entry flow (C-NAV-001: back out of Home must not resurface onboarding) and two self-contained flows whose own CTAs are the exit (PAIR_PROMPT, RECOVERY). Also records the durable substance where it belongs: - ERM landmine C-NAV-004 — the exact inverse of C-NAV-003 (wrongly *added* to shellBackRoutes = double app bar; wrongly *omitted* = no bar at all). The two failure modes point in opposite directions, so there is no safe default; the scanner is the guard. - Future.md — the Tier 3 screenshot-diff idea is no longer hypothetical. Five visual defects surfaced in one day (star glyph fallback, "Chat Bubble Outline" chip leak, mid-word truncation, double back arrow, hard-edged art) and every one was caught only by a human reading a screenshot. They share a shape: the UI stays renderable and shows the wrong thing, so compile/unit/theme-scan all stay green. That is the class the tooling is structurally blind to. Co-Authored-By: Claude Opus 4.8 --- Future.md | 10 +++ docs/Engineering_Reference_Manual.md | 7 ++ scripts/nav-scan.sh | 120 +++++++++++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100755 scripts/nav-scan.sh diff --git a/Future.md b/Future.md index d22a8143..9cded782 100644 --- a/Future.md +++ b/Future.md @@ -134,6 +134,16 @@ Improvement & feature ideas surfaced while QA-testing as a consumer (each works Implement a screenshot pipeline with Roborazzi / Shot / papAROS that renders every `AppRoute` in both light and dark, pixel-diffs them, and fails on unexpected white backgrounds or invisible text. When done, run it in CI against every UI PR. + **No longer hypothetical — 2026-07-14 evidence.** A single day's content change surfaced five visual defects, + and **every one was caught only by a human reading a screenshot**; no scanner or test flagged any of them: + 22 of 23 category glyphs silently fell back to a star (icon_name became a vocabulary `CategoryGlyph` didn't + speak), the raw `icon_name` leaked into a user-facing chip as "Chat Bubble Outline", pack names broke + mid-word ("Communicatio/n"), the pack detail page drew two back arrows, and pack art sat as a hard-edged + tile. They share a shape: **the UI stays renderable and simply shows the wrong thing**, so the compile, the + unit suite, and `theme-scan.sh` all stay green. A pixel diff over `PairedHomePreviewScreen` + the pack + library/detail routes would have caught all five. This is the highest-value test gap in the app — it guards + the class the current tooling is structurally blind to. (Nav dead-ends are now covered separately and + statically by `scripts/nav-scan.sh` — see C-NAV-004; that one didn't need pixels.) - **🟡 STARTED (R20) — Instrumented / on-device test coverage (was 0 androidTest).** First cut shipped: `app/src/androidTest/.../ui/FirstRunRenderSmokeTest.kt` — an on-device **Compose render smoke** of the first-run diff --git a/docs/Engineering_Reference_Manual.md b/docs/Engineering_Reference_Manual.md index 433250f0..9efbc2ad 100644 --- a/docs/Engineering_Reference_Manual.md +++ b/docs/Engineering_Reference_Manual.md @@ -1427,6 +1427,13 @@ OOB code = `truncate6(SHA-256(pubkey ‖ nonce))`. **Fix**: `popUpTo(WHEEL_SESSION){inclusive=true}` on session→complete navigation; removed `WHEEL_HISTORY`/`GAME_HISTORY`/`PARTNER_HOME` from `shellBackRoutes` in `AppNavigation.kt`. **Re-introduction risk**: adding new screens with their own `TopAppBar` to `shellBackRoutes` - check the route list before adding. +### C-NAV-004 - a route in neither nav set has no way out +**Symptom**: the question pack library ("Pick a doorway") rendered no app bar and no bottom bar. Reached from Home ("All packs"), the Play hub, the composer's empty state and the weekly recap, it left the **system Back gesture as the only way off the screen**. Its own detail page (`QUESTION_CATEGORY`) had a working back arrow the whole time, which made the library look intentional rather than broken. +**Root cause**: the shell draws chrome from two hand-maintained sets - `topLevelRoutes` (bottom bar) and `shellBackRoutes` (app bar + back). `QUESTION_PACKS` was in neither, and `QuestionPackLibraryScreen` draws no header of its own, so nothing rendered. Nothing fails or warns in this state; the screen just quietly has no exit. +**Fix**: added `AppRoute.QUESTION_PACKS` to `shellBackRoutes` in `AppNavigation.kt` (one line). +**Re-introduction risk**: **every new screen.** This is the exact inverse of C-NAV-003 - that one is "wrongly *added* to `shellBackRoutes` → double app bar", this one is "wrongly *omitted* → no bar at all". A new route is a dead end by default unless someone remembers one of two lists. Note the failure modes point in opposite directions, so there is no safe default to lean on: the right set depends on whether the screen draws its own header. +**The guard**: `scripts/nav-scan.sh` (Pass C pre-check, exit 1 on a finding) proves this statically - it cross-references both sets against each route's screen and flags any route with no chrome and no back of its own. Proven to flag `QUESTION_PACKS` when the fix is reverted. Run it when adding a route; do not rely on the manual sweep, which already mandated "from every screen confirm there's always a way out" and missed this for months. + ### Home duplicate pending-action card (C-HOME-001) **Symptom**: Home showed the same pending action twice - once in the `primaryAction` hero, once in the `buildPendingActions` row. **Fix**: `buildPendingActions().filterNot { it.target == primary?.target }` to dedupe (R10). diff --git a/scripts/nav-scan.sh b/scripts/nav-scan.sh new file mode 100755 index 00000000..d0f022ee --- /dev/null +++ b/scripts/nav-scan.sh @@ -0,0 +1,120 @@ +#!/bin/bash +# +# CloserApp — automated navigation dead-end scanner (Pass C pre-check) +# +# Finds screens a user can reach but cannot leave. +# +# The shell in AppNavigation.kt draws chrome from two hand-maintained sets: +# * topLevelRoutes (AppRoute.kt) -> gets the bottom nav bar +# * shellBackRoutes (AppNavigation.kt) -> gets the app bar + its back arrow +# A route in NEITHER set gets no bar at all. That is correct only if the screen +# draws its own back affordance (several deliberately do — see the exclusion +# comments in shellBackRoutes). If it does neither, the system Back gesture is +# the only way off the screen: a dead end. +# +# That is exactly how C-NAV-004 shipped — QUESTION_PACKS is drilled into from +# four places, drew no header of its own, and was missing from shellBackRoutes. +# The Pass C manual sweep already said "from every screen confirm there's always +# a way out" and still missed it, because it depends on a human walking ~50 +# routes. This check is static, so it cannot be skipped by fatigue. +# +# ⛔ CLAUDE: You may improve this script whenever you find a new way a route can +# strand the user. Keep it self-contained, runnable from the project root, and +# write findings to stdout. Update this header with any new patterns/exclusions. +# +# Exclusions: +# - onboardingAuthRoutes — the pre-app entry flow deliberately has no back +# (C-NAV-001: back out of Home must not resurface onboarding/auth). +# - SELF_EXIT_ROUTES below — self-contained flows whose own CTAs are the exit +# (PAIR_PROMPT has invite + skip buttons; RECOVERY is an unlock flow). +# +# Usage: +# ./scripts/nav-scan.sh > /tmp/claude-nav-scan-$(date +%Y%m%d).md +# +set -uo pipefail +cd "$(dirname "$0")/.." || exit 1 + +ROUTE_FILE="app/src/main/java/app/closer/core/navigation/AppRoute.kt" +NAV_FILE="app/src/main/java/app/closer/core/navigation/AppNavigation.kt" + +# Routes whose screen intentionally owns its exit rather than the shell's back arrow. +SELF_EXIT_ROUTES="PAIR_PROMPT RECOVERY" + +python3 - "$ROUTE_FILE" "$NAV_FILE" "$SELF_EXIT_ROUTES" <<'PY' +import re, subprocess, sys +from pathlib import Path + +route_file, nav_file, self_exit = sys.argv[1], sys.argv[2], set(sys.argv[3].split()) +route_src = Path(route_file).read_text() +nav_src = Path(nav_file).read_text() + +def set_of(src, name): + m = re.search(rf'{name}\s*=\s*setOf\((.*?)\n\s*\)', src, re.S) + if not m: return set() + return set(re.findall(r'(?:AppRoute\.)?([A-Z_][A-Z0-9_]*)', m.group(1))) + +defs = dict(re.findall(r'Definition\((\w+),\s*"([^"]+)"', route_src)) +top_level = set_of(route_src, "val topLevelRoutes") +entry = set_of(route_src, "val onboardingAuthRoutes") +shell_back = set_of(nav_src, "shellBackRoutes") + +# route const -> the *Screen composable the NavHost renders for it +route_screen = {} +for r, body in re.findall(r'composable\(\s*(?:route\s*=\s*)?AppRoute\.(\w+).*?\)\s*\{(.*?)\n \}', nav_src, re.S): + m = re.search(r'(\w+Screen)\s*\(', body) + if m: route_screen[r] = m.group(1) + +def draws_own_exit(screen): + """Does the screen render its own back/close affordance?""" + hits = subprocess.run(["grep", "-rl", f"fun {screen}", "app/src/main/java"], + capture_output=True, text=True).stdout.strip().splitlines() + if not hits or not hits[0]: + return None # unknown — report so a human looks + src = Path(hits[0]).read_text() + return bool(re.search(r'TopAppBar|CloserGlyphs\.Back|navigationIcon|popBackStack|onBack\b|"back"', src)) + +print("# CloserApp — navigation dead-end scan\n") +print("Routes that render no shell chrome (no bottom bar, no app bar) and no back") +print("affordance of their own. Reaching one strands the user on the system Back gesture.\n") + +crit, review = [], [] +for route, title in sorted(defs.items()): + if route in top_level or route in shell_back or route in entry or route in self_exit: + continue + screen = route_screen.get(route) + if not screen: + review.append((route, title, "(no screen resolved from NavHost)")) + continue + own = draws_own_exit(screen) + if own is None: + review.append((route, title, f"{screen} (source not found)")) + elif not own: + crit.append((route, title, screen)) + +print("## Tier 1 — Unreachable exit (CRITICAL)\n") +if crit: + for r, t, s in crit: + print(f"🔴 CRITICAL {r} \"{t}\" -> {s}") + print(f" no bottom bar, no app bar, no own back. Add AppRoute.{r} to") + print(f" shellBackRoutes in {nav_file}, or give the screen its own back.") +else: + print("none — every reachable route has a bottom bar, an app bar, or its own back.") + +print("\n## Tier 2 — Could not verify (REVIEW)\n") +if review: + for r, t, s in review: + print(f"🟡 REVIEW {r} \"{t}\" {s}") +else: + print("none") + +print("\n## Summary\n") +print(f"- routes defined: {len(defs)}") +print(f"- bottom-nav tabs: {len(top_level)}") +print(f"- shell back routes: {len(shell_back)}") +print(f"- entry flow (no back): {len(entry)}") +print(f"- self-exit exclusions: {len(self_exit)} ({', '.join(sorted(self_exit))})") +print(f"- 🔴 CRITICAL dead ends: {len(crit)}") +print(f"- 🟡 REVIEW: {len(review)}") + +sys.exit(1 if crit else 0) +PY