test(qa): add nav-scan.sh — prove navigation dead ends statically
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 <noreply@anthropic.com>
This commit is contained in:
parent
089437e415
commit
7700c84e4d
10
Future.md
10
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
|
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,
|
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.
|
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:
|
- **🟡 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
|
`app/src/androidTest/.../ui/FirstRunRenderSmokeTest.kt` — an on-device **Compose render smoke** of the first-run
|
||||||
|
|
|
||||||
|
|
@ -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`.
|
**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.
|
**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)
|
### 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.
|
**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).
|
**Fix**: `buildPendingActions().filterNot { it.target == primary?.target }` to dedupe (R10).
|
||||||
|
|
|
||||||
|
|
@ -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
|
||||||
Loading…
Reference in New Issue