diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 6c754a684..3d927ab47 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -621,3 +621,5 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-18 | claude/clinical-kb-pwa-review-asi3wb (PR #896, plan Phase 5; content commit + this ledger follow-up) | a6c2b4e92374e9002fb00c547eb5677d01ce538c | Design-polish sweep: audit-then-fix (plan Phase 5, final phase) | Audit on post-#890 main: three strict design guards clean; full re-run of the 07-token-adoption-audit grep method shows all July 3 debt resolved (M1–M3 done, L4 reduced to the deliberate theme-aware `ring-white/N dark:ring-white/10` glass idiom, L5/L7 gone; production hex all legitimate print/brand/console/comment classes); 43-capture live sweep across 15 routes × desktop/phone + 320px spots + dark/reduced-motion/forced-colors spots found 0 overflow and 0 console errors. Three defects found and fixed: (1) forced-colors solid-button labels rendered as blank Canvas-on-Canvas backplate boxes (axe-invisible) — command controls flattened to the native HCM ButtonFace/ButtonText pairing and accent glyph tokens flipped to ButtonText inside the existing forced-colors block, regression-locked by a new ui-accessibility test; (2) tools desktop 6-up quick-action rail truncated card titles at 1440×1000 — card metrics tightened, all six titles verified unclipped; (3) privacy page rendered "systemand" from a JSX newline-adjacent-to-tag drop — explicit space, locked by a privacy-ui assertion. Dated July 18 run appended to docs/redesign/07-token-adoption-audit.md (archived design-qa.md not resurrected). | Guards + focused vitest 14/14; `verify:cheap` chain green to the known container-only pdf-extraction-budget artifact (2806/2809); `verify:ui` 220 passed/2 failed (the two long-baselined container artifacts, hosted-CI-green through #826/#835/#872/#890); `test:e2e:accessibility` 8/8 incl. the new forced-colors token test; production build + client-bundle secret scan passed; `check:bundle-budget` within tolerance vs the Phase 4 ratchet (1290.6 vs 1278.6 KiB baseline); `verify:pr-local` runtime/format/lint/typecheck/build/rag-fixtures green with the same sole unit-suite artifact. `verify:release` not run (provider-backed; awaits explicit confirmation). No provider-backed checks run. | | 2026-07-19 | all remote feature branches and registered worktrees against `origin/main` through PR #899 | 8242fa63d5f5b79fc770c9ae4f633e3a784b80e1 | branch/worktree cleanup, useful-work recovery, and protected-main merge closure | Deleted 122 stale or closed remote feature refs with exact SHA leases; four additional merged PR branches were removed by the protected-main PR workflow. Removed 32 obsolete, superseded, or merge-proven worktree registrations. Recovered useful dirty RAG work into PR #901 (deterministic and opt-in semantic reranking) and PR #902 (retrieval phase latency telemetry), preserved follow-up decisions in `docs/process-hardening.md`, and recovered four missing historical review rows. PRs #897, #899, #901, and #902 are merged with green exact-head checks and zero unresolved review threads. A detached full-repo-review worktree is deliberately retained because its ownership/activity could not be safely disproved; one unregistered `node_modules` junction residue is also retained because deletion was denied by local safety policy. | Fresh fetch/prune; full GitHub PR/check/thread inventory; `git worktree list --porcelain`; cherry-pick-aware right-only logs; exact leased remote deletes; exact-old-value local ref deletes; clean-worktree, path, and merged-PR proof before every removal. PR #899 local proof: focused Vitest 31/31, changed-file ESLint, `verify:cheap` 317 files / 2,879 tests, and `verify:ui` 239/239; exact-head hosted checks all passed. PR #901 local proof: `verify:cheap` 316 files / 2,870 tests; PR #902 focused Vitest 8/8 plus ESLint and typecheck. No OpenAI, Supabase, live clinical, deployment, or production-data workflow ran; provider-backed semantic canary evaluation remains approval-gated. | | 2026-07-19 | origin/main 24-hour merged window (d1937d78e..ef042cacd, ~30 PRs incl. #853/#859/#861/#865/#868/#871–#874/#879/#885–#888/#890–#894/#896) | ef042cacd | integrated post-merge regression audit of the full 24h window (code review + design/performance/defect angles) | No P0/P1 regression found at the merged tip. One confirmed P2: the new settings surface (`use-app-preferences.ts` + `settings-dialog.tsx`) presents jurisdiction, population, answer-style, landing, home-content, compact-citations, and all notification preferences as live controls, but no consumer reads them — only density/motion (html attributes + globals.css) and theme are functional; a clinician selecting "Conservative" answer style reasonably but wrongly believes generation changed. One plausible P2/P3: `AuthProvider.initializeSession` now requires a live `getUser()` round-trip, so a transient network failure on load resolves a valid stored session to signed_out (INITIAL_SESSION replay is also skipped); the deliberate stale/tampered-token defense does not distinguish retryable network errors. One P3: `/medications` legacy redirect drops the query string while `/applications` and `/differentials/presentations` preserve theirs. Cleared after inspection: worker image-placement dedupe (key symmetric on both sides), title-word purge/scope migration (matches its replayed review), RPC-layer-only cancellation consolidation (intentional per PR #861), services route-chunk fix intact with no other heavy client value-imports (type-only imports verified), forced-colors ButtonFace/ButtonText flip, codex-autofix/pr-policy workflow changes conform to AGENTS.md (pin retained, rename-aware routing, workflow_sha checkout), audit-metadata allowlist exhaustive over the closed AuditAction union, answer-stream merged abort signals. Environment note (not a repo defect): the session-start hook skips `npm install` when node_modules exists, so this window's dependency bumps left the container stale and `verify:cheap` failed at typecheck until `npm ci`; the hook should compare a lockfile hash. | `npm ci` then typecheck clean and full Vitest 2828 passed / 1 failed / 2 skipped — the sole failure is the long-baselined container-only `tests/pdf-extraction-budget.test.ts` artifact (hosted-CI-green through #826/#835/#872/#890). First `verify:cheap` run passed every static guard (runtime, actions-pin, ci-scope, ci-triage, pr-policy, sitemap, brand, type-scale, icon-scale, function-grants, owner-scope, lint) before the stale-deps typecheck stop. No OpenAI, Supabase, deployment, or provider-backed check ran. | +| 2026-07-19 | origin/main foreign merges post-#896 landing (541da7b #871 remediation + f4557ca #892 policy parsing; explicit user review request) | ef042ca6a34d33862936e77e4b71068978382c1e | Bug review of recent main changes (diff-review protocol) | One P2 confirmed and fixed in PR #905: #892's widened heading matcher ended a required PR-body section at ANY next heading, so `###` sub-structure inside `## Verification` truncated the section and false-rejected valid bodies (fail-closed; repo template unaffected; repro via direct evaluatePullRequestPolicy probe old-vs-new). No P0/P1. Cleared after verification: `src/lib/client-env.ts` (no env leakage; production demote-to-demo removal deliberate and fail-safe — upload gating still locked via canUsePrivateApis), applications/medications redirect routes (fixed targets, 307+HEAD alias, no open-redirect/header-injection), test infra (no weakened assertions; route-coverage spec added to all projects), and the three biggest #871 UI diffs read inline (ClinicalDashboard upload tablist roving-tabindex + hydration-safe useSyncExternalStore role switch; visual-evidence unavailable-source rows became real non-interactive elements; favourites library demo-gates prototype items with sound menu keyboard nav). Residual (report-only): pr-policy `section()` remains fence-unaware (headings inside fenced code can satisfy required-section checks — pre-existing class, author-controlled attestation surface); dev-only Turbopack persistent-cache staleness served an old globals.css compile across restarts twice this session (fixed by setting `.next` aside). | Reviewer fan-out: general lane completed with concrete probes (pr-policy self-test, node repro on old parser from f4557ca^, adversarial body probes); UI lane agent lost to session limit and re-done inline on the three biggest diffs, leaning on the merged tree's green gates (verify:ui 236-passed run in this session covers #871's own new specs). No provider-backed checks run. | +| 2026-07-19 | claude/clinical-kb-pwa-review-asi3wb (PR #905; commits b2afe66 visuals + 6531178 policy + this ledger follow-up) | 6531178c1a3427dfd58c4bfcf8e29020c5731179 | PWA install/update notice redesign (all breakpoints) + review-follow-up policy fix | Redesigned the five PWA notices (install, update, iOS hint, offline, restored) as glass lux cards: per-type semantic icon tiles, heading-ink titles, corner dismiss buttons, reduced-motion-safe 280ms entrance. Deliberate placement per screen size: phones keep the bottom card above the fixed composer (thumb zone, safe areas); ≥640px floats a 25rem card bottom-right; ≥1280px moves the stack to a top-right toast under the header (same 4.25rem+safe-area offset constant as the mode-menu popover) with the animation direction flipped. Copy, roles, and button names unchanged. Plus the outline-aware pr-policy section parser fix from the same-session review (see the review row above). | Focused vitest pwa-lifecycle.dom 9/9 + pwa-manifest 8/8; `check:pr-policy` self-test green incl. new sub-heading case; `verify:cheap` 2828/2831 (sole fail = known container-only pdf-extraction-budget artifact); `test:e2e:pwa` privacy/offline green (installability fail = known container `in-incognito` artifact); `verify:ui` 236 passed/2 failed (the two long-baselined container artifacts); production build + client-bundle secret scan + bundle budget within tolerance (1293.4 vs 1278.6 KiB baseline); visual evidence at 390/768/1440 light+dark+offline in session scratchpad. Dev caveat recorded: Turbopack persistent `.next` cache served stale globals.css across restarts twice; fixed by setting the cache aside. No provider-backed checks run. | diff --git a/scripts/pr-policy.mjs b/scripts/pr-policy.mjs index f79e92836..f7a04328a 100644 --- a/scripts/pr-policy.mjs +++ b/scripts/pr-policy.mjs @@ -52,18 +52,26 @@ function normalizeSectionHeading(value) { function section(body, heading) { const source = String(body ?? ""); - const headings = [...source.matchAll(/^#{2,6}[ \t]+(.+?)[ \t]*$/gim)]; + const headings = [...source.matchAll(/^(#{1,6})[ \t]+(.+?)[ \t]*$/gim)]; const targetHeading = normalizeSectionHeading(heading); - const matchIndex = headings.findIndex((match) => normalizeSectionHeading(match[1]) === targetHeading); + const matchIndex = headings.findIndex((match) => normalizeSectionHeading(match[2]) === targetHeading); if (matchIndex < 0) return ""; const start = (headings[matchIndex]?.index ?? 0) + (headings[matchIndex]?.[0].length ?? 0); - const end = headings[matchIndex + 1]?.index ?? source.length; + // Markdown outline semantics: the section runs until the next heading of the + // same or shallower level. Deeper headings (### under ##) are sub-structure + // and stay inside the section, so checklist evidence after them still counts. + const level = headings[matchIndex][1].length; + const next = headings.slice(matchIndex + 1).find((match) => match[1].length <= level); + const end = next?.index ?? source.length; return source.slice(start, end).trim(); } function meaningfulText(value) { const normalized = String(value ?? "") .replace(//g, "") + // Sub-headings kept inside a section by the outline-aware boundary are + // structure, not content — a section holding only headings is still empty. + .replace(/^\s*#{1,6}[ \t]+\S.*$/gm, "") .replace(/^\s*[-*]\s*/gm, "") .trim(); return Boolean(normalized && !/^(?:-|n\/?a|none|todo|tbd)$/i.test(normalized)); @@ -129,6 +137,10 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { const errors = []; const classification = classifyPullRequestFiles(files); const summary = section(body, "Summary"); + // The summary must be its own prose: content nested under a sub-heading + // (e.g. a mis-levelled `### Verification`) belongs to that sub-topic and + // cannot stand in for the required outcome summary. + const summaryDirect = summary.replace(/^[ \t]*#{1,6}[ \t]+\S[^]*$/m, ""); const verification = section(body, "Verification"); const riskAndRollout = section(body, "Risk and rollout"); const governance = section(body, "Clinical Governance Preflight"); @@ -136,7 +148,8 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { if (String(title ?? "").trim().length < 12) errors.push("Use a specific, outcome-focused PR title (at least 12 characters)."); if (branchLikeTitle(title, headRef)) errors.push("Replace the branch-style PR title with an outcome-focused title."); - if (!meaningfulText(summary)) errors.push("Complete the `## Summary` section with the outcome and affected area."); + if (!meaningfulText(summaryDirect)) + errors.push("Complete the `## Summary` section with the outcome and affected area."); if (!meaningfulText(verification)) { errors.push("Complete the `## Verification` section with exact results or a reason checks were not run."); } else if (!/-\s*\[[xX]\]/.test(verification) && !explicitNotRun(verification)) { @@ -212,6 +225,39 @@ function selfTest() { }).errors.join(" "), /verify:ui/, ); + // Outline semantics: a ### sub-heading inside a required section must not + // truncate it — checklist evidence after the sub-heading still counts. + assert.equal( + evaluatePullRequestPolicy({ + title: "ci: enforce pull request evidence", + body: completeBody.replace("## Verification\n\n", "## Verification\n\n### Unit tests\n\n"), + headRef: "codex/pr-policy", + files: [".github/workflows/pr-policy.yml"], + }).ok, + true, + ); + // A section whose only content is a nested sub-heading is still empty: the + // outline-aware boundary must not let heading lines satisfy meaningfulText. + assert.match( + evaluatePullRequestPolicy({ + title: "fix: something meaningful here", + body: "## Summary\n\n### Verification\n\n- [x] `npm run verify:pr-local`\n", + headRef: "codex/x", + files: ["docs/a.md"], + }).errors.join(" "), + /## Summary/, + ); + // ...but a level-1 heading is shallower than ## and DOES end the section, so + // evidence stranded after it must not count toward the preceding section. + assert.match( + evaluatePullRequestPolicy({ + title: "fix: update search behavior", + body: completeBody.replace("- [x] `npm run verify:ui`\n", "# Appendix\n\n- [x] `npm run verify:ui`\n"), + headRef: "codex/search-fix", + files: ["src/components/search.tsx"], + }).errors.join(" "), + /verify:ui/, + ); assert.match( evaluatePullRequestPolicy({ title: "fix: update clinical search", diff --git a/src/app/globals.css b/src/app/globals.css index fc9a554d7..567962a67 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -1950,6 +1950,10 @@ summary::-webkit-details-marker { /* Global PWA notices sit above the bottom composer and home indicator without changing page scroll ownership. The lifecycle component renders only when an offline, restored, installable, or waiting-update state needs attention. */ +/* PWA notices: bottom sheet-style on phones (thumb zone, above the fixed + composer), floating bottom-right card on medium screens, and top-right + toast under the header on desktop — beside the browser's own install + affordance and clear of the composer. */ .pwa-notice-stack { position: fixed; z-index: 95; @@ -1961,10 +1965,25 @@ summary::-webkit-details-marker { pointer-events: none; } +.pwa-notice-card { + animation: pwa-notice-in 280ms cubic-bezier(0.2, 0.9, 0.3, 1) both; +} + +@keyframes pwa-notice-in { + from { + opacity: 0; + transform: translateY(0.625rem) scale(0.98); + } + to { + opacity: 1; + transform: none; + } +} + @media (min-width: 640px) { .pwa-notice-stack { left: auto; - width: min(24rem, calc(100vw - 2rem - var(--safe-area-left) - var(--safe-area-right))); + width: min(25rem, calc(100vw - 2rem - var(--safe-area-left) - var(--safe-area-right))); } } @@ -1974,6 +1993,35 @@ summary::-webkit-details-marker { } } +/* Last so it also wins over the standalone bottom offset at desktop width. */ +@media (min-width: 1280px) { + .pwa-notice-stack { + top: calc(4.25rem + max(0.5rem, var(--safe-area-top))); + bottom: auto; + } + + @keyframes pwa-notice-in { + from { + opacity: 0; + transform: translateY(-0.625rem) scale(0.98); + } + to { + opacity: 1; + transform: none; + } + } +} + +@media (prefers-reduced-motion: reduce) { + .pwa-notice-card { + animation: none; + } +} + +html[data-motion="reduced"] .pwa-notice-card { + animation: none; +} + /* Tabular figures for data, counts, page numbers, byte sizes */ @utility nums { font-variant-numeric: tabular-nums; diff --git a/src/components/pwa-lifecycle.tsx b/src/components/pwa-lifecycle.tsx index 1e6402287..f9daa2a39 100644 --- a/src/components/pwa-lifecycle.tsx +++ b/src/components/pwa-lifecycle.tsx @@ -1,5 +1,6 @@ "use client"; +import { ArrowDownToLine, RefreshCw, Share, Wifi, WifiOff, X, type LucideIcon } from "lucide-react"; import { useEffect, useRef, useState, useSyncExternalStore } from "react"; const SERVICE_WORKER_URL = "/sw.js"; @@ -101,11 +102,34 @@ async function teardownLocalPwa() { } const cardClassName = - "pointer-events-auto rounded-xl border border-[color:var(--border-lux)] bg-[color:var(--surface-raised)] p-4 text-[color:var(--text)] shadow-[var(--shadow-elevated)]"; + "pwa-notice-card pointer-events-auto relative rounded-2xl border border-[color:var(--border-lux)] bg-[color:var(--surface-lux)] p-4 text-[color:var(--text)] shadow-[var(--shadow-lux)] ring-1 ring-white/35 backdrop-blur-md dark:ring-white/10"; const primaryButtonClassName = - "inline-flex min-h-tap items-center justify-center rounded-lg bg-[color:var(--clinical-accent)] px-3.5 py-2 text-sm font-semibold text-[color:var(--clinical-accent-contrast)] transition-colors hover:bg-[color:var(--clinical-accent-hover)] focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]"; + "inline-flex min-h-tap items-center justify-center rounded-lg bg-[color:var(--clinical-accent)] px-3.5 py-2 text-sm font-semibold text-[color:var(--clinical-accent-contrast)] shadow-[var(--shadow-tight)] transition-colors hover:bg-[color:var(--clinical-accent-hover)] focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]"; const secondaryButtonClassName = "inline-flex min-h-tap items-center justify-center rounded-lg border border-[color:var(--border-lux)] px-3.5 py-2 text-sm font-semibold text-[color:var(--text)] transition-colors hover:bg-[color:var(--surface-subtle)] focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]"; +const dismissIconButtonClassName = + "absolute right-1 top-1 inline-flex h-tap w-tap items-center justify-center rounded-full text-[color:var(--text-muted)] transition-colors hover:bg-[color:var(--surface-subtle)] hover:text-[color:var(--text)] focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]"; + +// Leading icon tile: categorical identity per notice type, on the semantic +// soft/border/ink triads so dark mode and forced-colors resolve via tokens. +function NoticeIcon({ icon: Icon, tone }: { icon: LucideIcon; tone: "accent" | "info" | "warning" | "success" }) { + const toneClassName = + tone === "accent" + ? "border-[color:var(--clinical-accent-border)] bg-[color:var(--clinical-accent-soft)] text-[color:var(--clinical-accent)]" + : tone === "info" + ? "border-[color:var(--border)] bg-[color:var(--info-soft)] text-[color:var(--info)]" + : tone === "warning" + ? "border-[color:var(--border)] bg-[color:var(--warning-soft)] text-[color:var(--warning)]" + : "border-[color:var(--border)] bg-[color:var(--success-soft)] text-[color:var(--success)]"; + return ( + + ); +} /** * Owns installability, service-worker updates, and cross-route connectivity UI. @@ -118,6 +142,7 @@ export function PwaLifecycle() { const isOnline = useSyncExternalStore(subscribeConnectivity, getConnectivitySnapshot, getServerConnectivitySnapshot); const [connectionRestored, setConnectionRestored] = useState(false); const [installPrompt, setInstallPrompt] = useState(null); + const [offlineNoticeDismissed, setOfflineNoticeDismissed] = useState(false); const [showIosHint, setShowIosHint] = useState(false); const [waitingWorker, setWaitingWorker] = useState(null); const [activatedUpdateReady, setActivatedUpdateReady] = useState(false); @@ -138,6 +163,9 @@ export function PwaLifecycle() { const handleOnline = () => { if (restoredTimer) window.clearTimeout(restoredTimer); setConnectionRestored(true); + // Dismissal is per offline episode: the next connectivity drop should + // re-surface the notice. + setOfflineNoticeDismissed(false); restoredTimer = window.setTimeout(() => setConnectionRestored(false), 4_000); }; @@ -351,83 +379,143 @@ export function PwaLifecycle() { setActivatedUpdateReady(false); }; + const showOffline = !isOnline && !offlineNoticeDismissed; const showUpdate = isOnline && (Boolean(waitingWorker) || activatedUpdateReady); const showInstall = isOnline && !showUpdate && Boolean(installPrompt); const showIosInstallHint = isOnline && !showUpdate && !showInstall && showIosHint; - if (isOnline && !connectionRestored && !showInstall && !showUpdate && !showIosInstallHint) return null; + if (!showOffline && !connectionRestored && !showInstall && !showUpdate && !showIosInstallHint) return null; return (
- {!isOnline ? ( + {showOffline ? (
-

- You appear to be offline -

-

- Clinical search, answers, private documents, uploads, and account data require a connection. -

- +
+ +
+

+ You appear to be offline +

+

+ Clinical search, answers, private documents, uploads, and account data require a connection. +

+ +
+
) : null} {connectionRestored ? (
-

Connection restored

+
+ +

Connection restored

+
) : null} {showUpdate ? (
-

- An update is ready -

-

- Refresh when convenient to use the latest Clinical KB version. -

-
- - + +
+ +
+

+ An update is ready +

+

+ Refresh when convenient to use the latest Clinical KB version. +

+
+ + +
+
) : null} {showIosInstallHint ? (
-

- Install Clinical KB -

-

- In Safari, tap Share, then Add to Home Screen. Private clinical features still require a connection. -

-
- + +
+ +
+

+ Install Clinical KB +

+

+ In Safari, tap Share, then Add to Home Screen. Private clinical features still require a connection. +

+
+ +
+
) : null} {showInstall ? (
-

- Install Clinical KB -

-

- Open it from your device like an app. Private clinical features still require a connection. -

-
- - + +
+ +
+

+ Install Clinical KB +

+

+ Open it from your device like an app. Private clinical features still require a connection. +

+
+ + +
+
) : null} diff --git a/tests/pwa-lifecycle.dom.test.tsx b/tests/pwa-lifecycle.dom.test.tsx index f6c395f52..4d10313de 100644 --- a/tests/pwa-lifecycle.dom.test.tsx +++ b/tests/pwa-lifecycle.dom.test.tsx @@ -142,6 +142,24 @@ describe("PwaLifecycle", () => { expect(await screen.findByText("Connection restored")).toBeInTheDocument(); }); + it("dismisses the offline notice per episode and re-surfaces it on the next connectivity drop", async () => { + const user = userEvent.setup(); + render(); + + Object.defineProperty(navigator, "onLine", { configurable: true, value: false }); + fireEvent.offline(window); + expect(await screen.findByRole("region", { name: "You appear to be offline" })).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Dismiss offline notice" })); + expect(screen.queryByRole("region", { name: "You appear to be offline" })).not.toBeInTheDocument(); + + Object.defineProperty(navigator, "onLine", { configurable: true, value: true }); + fireEvent.online(window); + Object.defineProperty(navigator, "onLine", { configurable: true, value: false }); + fireEvent.offline(window); + expect(await screen.findByRole("region", { name: "You appear to be offline" })).toBeInTheDocument(); + }); + it("shows install UI only after browser eligibility and invokes the deferred prompt from a user action", async () => { const user = userEvent.setup(); render();