fix(e2e+ui): clear the run-4012 matrix remainder — SW navigation hijack, WebKit quirks, Firefox tab order, CIWA alias#996
Conversation
…ck, WebKit focus/forced-colors/stale-style, Firefox tab order, CIWA term alias Seven fixes from the run-4012 triage, each root-caused: - playwright.config.ts: serviceWorkers 'block' suite-wide (+ ui-pwa.spec.ts 'allow' opt-in). In production builds public/sw.js registers in every test, claims the page (clients.claim) and serves every navigation — probe-proven to bypass route interception for navigations on Chromium and to wedge Playwright-Firefox's reload path under an active route (both ui-smoke reload hangs). Only ui-pwa exercises the worker itself; its offline/CacheStorage journey passes under the opt-in. - master-search-header.tsx: close the desktop mode menu on Tab from the trigger (APG menu-button pattern). WebKit's sequential focus navigation can fail to deliver a wrapper-escaping focusout (links excluded from its Tab order; backward wrap into the menu), leaving the menu open. - ui-accessibility.spec.ts: webkit skip for the forced-colors token test — WebKit has no forced-colors implementation, so the media remap under test cannot engage there. - ui-formulation.spec.ts: pin the step-nav Previous button enabled with opacity 1 before the axe scan — WebKit can serve a stale :disabled computed style whose 0.4 opacity axe folds into a phantom contrast violation for a WCAG-1.4.3-exempt state. - ui-smoke.spec.ts: step over Firefox's scrollable-container tab stop (the sheet body sits between Close and 'New chat' in DOM order). - scripts/eval-retrieval.ts: ciwa -> ciwa-ar content alias. The matcher is whitespace-delimited, so the bare fixture term can never match the scale's hyphenated written name; canary runs #50/#51 ranked the CIWA-Ar dosing-table region top-5 yet the gate reported a miss. - tests/helpers/zero-touch.ts: shared stubZeroTouchPoints helper replacing the six inline copies from #995 (review follow-up). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe PR updates Playwright service-worker and touch-capability setup, centralizes WebKit test initialization, strengthens UI focus and state assertions, adds CIWA retrieval aliases, changes desktop mode-menu Tab handling, and records the run-4012 review remainder. ChangesRun-4012 remainder
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Seven root-caused fixes for the browser-matrix remainder (run 4012 baseline: 28 failures) plus the canary CIWA gap — every finding agent-diagnosed with probe or arithmetic evidence before any code was written:
playwright.config.ts+tests/ui-pwa.spec.ts): in production buildspublic/sw.jsregisters in every matrix test (pwa-lifecycle.tsxregisters wheneverNODE_ENV === "production"),clients.claim()s the page seconds in, and serves every subsequent navigation. A Chromium probe proved SW-served reloads bypasspage.routeinterception entirely, and Playwright-Firefox wedges on exactly this path — its only twopage.reload()calls in the whole suite are the two 60-second ui-smoke hangs (2206 and the @critical 2932). Fix:serviceWorkers: "block"in the shared config;ui-pwa.spec.ts— the one spec whose subject is the worker — opts back in withtest.use({ serviceWorkers: "allow" }), and its offline/CacheStorage privacy journey passes locally under the opt-in.src/components/clinical-dashboard/master-search-header.tsx): dismissal relied solely on a wrapperfocusout; WebKit's sequential focus navigation can move focus nowhere (links are excluded from its Tab order — the previous tabbable is the skip link) or wrap backward into the open menu, so no wrapper-escaping focusout ever fires. Fix: close onTabin the trigger's keydown handler (APG menu-button pattern; arrow keys remain the entry). Agent-verified no existing test depends on the old forward-Tab behavior; pointer/Escape/blur paths unchanged.tests/ui-accessibility.spec.ts): WebKit has never implementedforced-colors; Playwright plumbs the emulation flag but the@media (forced-colors: active)remap under test cannot engage. Fix: webkittest.skipin the repo's established capability-guard style. Chromium/Firefox keep full coverage; the sibling generic forced-colors usability test stays unguarded (it passed on WebKit).tests/ui-formulation.spec.ts): the reported#b5bbc2at 1.93:1 is byte-exact the 0.4-opacity disabled composite of--text-mutedon white — but the Previous button is provably enabled at scan time and axe skips truly disabled controls. Only consistent mechanism: WebKit serving a stale:disabledcomputed style after React re-enables the button. Fix: pintoBeEnabled()+toHaveCSS("opacity", "1")before the scan — heals the stale style or converts the failure into direct proof (fallback documented in-test). WCAG 1.4.3 exempts disabled controls, so no design change is warranted.tests/ui-smoke.spec.ts): Firefox includes scrollable containers in the tab order; the sheet body (overflow-y-auto, measured genuinely overflowing at the test viewport) sits between Close and "New chat" in DOM order. Fix: conditional step-over in the test (an app-sidetabIndex={-1}on the shared Sheet primitive would remove Firefox's keyboard-scroll affordance — a product decision deliberately not taken here).scripts/eval-retrieval.ts): canary runs Save Codex local changes #50 and Refactor monolithic front‑end and decouple domain logic #51 rank the CIWA-Ar dosing-table region in the top 5, yet the content gate reports "ciwa OR score OR threshold" missing — becausetextContainsClinicalTermis whitespace-delimited and the scale's written name is the hyphenated token "CIWA-Ar", which the bare fixture term can never match. Fix:ciwa: ["ciwa", "ciwa-ar"]inclinicalContentAliases— the eval's own documented drift-absorption mechanism (plan-authorized fixture-alias route). The clinical substance of the expectation is unchanged.tests/helpers/zero-touch.ts+ six specs): extracts test(e2e): report real zero-touch capability to WebKit for command-surface specs #995's six inlinemaxTouchPointsstubs into one helper — the CodeRabbit review follow-up.Verification
eval-retrieval,search-command-surface) — 25/25npm run test— 3012 passed; sole failure is the long-baselined container-onlypdf-extraction-budgetartifactnpm run verify:cheap— all static checks, lint, typecheck green; same single artifact in the test stepnpm run build+ client-bundle secret scan — passui-pwaunder the new SW config — the cold-offline/CacheStorage privacy journey passes with theallowopt-in; the installability test shows only the known container-onlyin-incognitoartifact (hosted-CI-green since feat(pwa): retirement kill-switch, offline-version binding guard, and dev teardown flag #826)UI verification not run: the local Chromium UI suites carry documented container artifacts (16/20
ui-universal-searchfailures identical on unmodified main; Playwright browser-build mismatch after the dependabot bump) — hostedui-critical/ui-advisoryon this PR plus the post-merge matrix dispatch are the authoritative UI verdicts.npm run eval:retrieval:qualitycannot run locally (live keys exist only in Actions); the alias change is validated by the next canary run — the failing case's evidence chain (#50/#51 logs) is recorded in the ledger row.Risk and rollout
serviceWorkers: "block"returns every non-PWA test to the pre-July-17 (pre-sw.js) behavior they were written under; the worker's own coverage is preserved via the ui-pwa opt-in and the untouched Vitest SW policy suites. The mode-menu Tab-close is a one-branch keyboard refinement matching the APG pattern; remaining changes are test-side or the eval fixture alias.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
ui-stress:409,ui-tools:2087, and the WebKit answer-flow subset — the post-merge dispatch measures those; anything surviving gets its own diagnosis round.🤖 Generated with Claude Code
https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Tests