Add three Clinical KB guide wireframe mockups - #1580
Closed
BigSimmo wants to merge 2 commits into
Closed
Conversation
Design-scratch phone and desktop Sheet explorations for Tip Atlas, Help Hub, and Walkthrough, grounded in live GuideDialog content and tokens, with linked entry/detail/demo/skip states. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Keep device-frame Sheet studies free of a second composer, and document the new mockup routes in the generated site map. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Contributor
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #7808 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
This was referenced Aug 5, 2026
BigSimmo
commented
Aug 5, 2026
OwnerAuthor
Closing as part of an open-PR review sweep.
Worth recording why this was surprising, because it was a real documentation gap rather than a mistake in this PR: "mockups are exempt" was being read as blanket. Mockups are exempt from the wiring and reachability gates and nothing else — they are still compiled, still typechecked, and their client chunks still count toward There is also a genuine unresolved tension underneath this, now tracked as ledger The wireframes themselves are not being judged. If you want them, open fresh against current Generated by Claude Code |
cursorBot
pushed a commit
that referenced
this pull request
Aug 5, 2026
* fix(gates): catch lint and type errors before push, not in CI Two open PRs burned full CI cycles this week on defects a single local command would have caught: #1606 on a react-hooks/set-state-in-effect lint error, #1618 on a TS2339 for `mode.devOnly` (a union member that lacks the property, where app-modes.ts already exports the correct `"devOnly" in mode` guard). Neither lint nor typecheck was in the pre-push path. Typecheck could not simply be added, because it was already unusable (outstanding-issues #210). tsconfig.json's `include` carries `.next/types/**/*.ts` and `.next/dev/types/**/*.ts` — gitignored build artifacts — so deleting a page leaves the stale generated validator importing a removed module. Reproduced rather than inferred: a planted `.next/dev/types/validator.ts` referencing a removed mockup page yields `error TS2307: Cannot find module .../mockups/deleted-mockup-route/page.js`, base config exit 2, source-only config exit 0. Full source typecheck is clean (71s cold, 8.8s warm). Red locally and green in CI is how the gate got abandoned, which is how the real type error then reached CI. - tsconfig.typecheck.json + `typecheck:source`: identical compiler options, minus the `.next` globs, with a separate tsbuildinfo so the two incremental caches cannot invalidate each other. Route-signature validation is not lost; `next build` still covers it in CI. - guard-push.mjs gains a fourth guard running eslint over the pushed files and this typecheck. Verified to reproduce both defects above with CI-identical messages. Scoped to the lint roots and to pushes that touch TS, skips loudly when node_modules is absent rather than pushing people to GUARD_PUSH_DISABLE=1, and overridable with SKIP_STATIC_GUARD=1. Also corrects a doc claim that made #1580 surprising: "mockups are exempt" was being read as blanket. Mockups are exempt from the wiring and reachability gates and nothing else — they are still typechecked, and their client chunks still count toward check:bundle-budget, which totals every built chunk rather than the initial production bundle. That the budget's scope contradicts ledger #13's "not an initial production bundle" position is a real unmade decision, now recorded as #237 rather than papered over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy * docs(issues): capture the phone Category soft-menu fix salvaged from PR #1606#1606 is closed, but it carried the one fix nothing else in the queue provides: MobileResultFilterControl's native <select> paints a harsh system-blue highlight on phones, and #1615 keeps that native select (its change is the iOS 16px anti-zoom rule). So the fix does not survive #1615 landing. Records it as #238 with the two defects the redo must not repeat: the unresolved keyboard trap on disabled options, and the set-state-in-effect lint error that PR #1620's new pre-push guard would now catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy * issues: capture #239 stale Cloud acceptance pin on PR #1617, #240 remote-container browser gate drift * Tighten guard coordinator test Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): shared source-typecheck lease and safer static pre-push Treat typecheck:source:internal as a shared read-only coordinator lease with a distinct per-worktree buildinfo file, drop the pinned in-repo cache path, and harden staticGuard: acquire a short exclusive lease (fail-open when busy), use a private eslint cache, escalate lint on eslint policy changes, fail closed when the push tip is not HEAD, cover eslint-rules, and add Vitest coverage. Align hook/docs wording with the fourth guard and point CLAUDE.md at #252. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): keep tsBuildInfoFile when run-heavy has no npm_execpath Pre-push invokes run-heavy via plain node, so the npm_execpath spawn path was skipped and the fallback dropped effectiveForwarded — undoing the per-worktree buildinfo injection. Also warn when staticGuard passes on a dirty working tree. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: refresh scripts-index for lint:changed:internal Keep docs:check-inventory green after adding the pre-push eslint wrapper script to package.json. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): address Devin findings on static pre-push guard - Treat "Database focused-test capacity is full" as coordinator busy so shared typecheck slot exhaustion fails open instead of faking a type error. - Skip source typecheck when every changed .ts path is excluded by tsconfig.typecheck.json (edge functions, archive, scratch, worktrees). - Restore check-github-shell-access.mjs (and its Role notes) in the scripts index. * chore(ledger): record PR #1620 babysit * fix(gates): emit structured heavy-run admission-busy signal Prefer exit 75 + DATABASE_HEAVY_RUN_ADMISSION_BUSY over prose matching so tsc/eslint output that quotes busy strings cannot false-pass the static guard. * fix(gates): tip-check only when static work runs; isolate typecheck cache Addresses follow-up Devin on PR #1620: - Reorder staticGuard so tip-vs-HEAD fails closed only when lint/typecheck will actually read the working tree; ignore tag refs in the tip check. - Pin a distinct tsBuildInfoFile on tsconfig.typecheck.json so direct tsc does not collide with the base config cache (run-heavy still overrides). * fix(gates): keep lint failures when typecheck admission is busy Addresses Devin on PR #1620 — a prior eslint failure must still block the push if the follow-up source typecheck cannot get a coordinator slot. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo pushed a commit
that referenced
this pull request
Aug 9, 2026
check:bundle-budget totals every built chunk, mockups included, and main sits at ~+9.4% against a 10% tolerance. The study's two scratch chunks (~9.8 KiB gzip) alone took the repo to +10.1% and failed Build — the same failure PR #1580 hit, at the same number. Measured on this branch: main alone 308 chunks / 1538.9 KiB (+9.42%, passing); with the study 310 / 1548.7 KiB (+10.1%, failing); without it 309 / 1542.4 KiB (+9.67%, passing). The implementation itself adds no new route chunk — it edits existing components. Direction 02 has shipped, so the runnable route had already served its purpose. A design-scratch route that 404s in production is the wrong thing to spend the last of that headroom on, and raising the baseline would have settled the open #13/#252 question — whether scratch should count toward this budget at all — by default, in the direction of "raise the ceiling". mockups/README.md keeps the three directions and says why the route went, so the alternatives stay recoverable from history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBAt8pJVz2TxMEUJeWEdUy
7 tasks
BigSimmo added a commit
that referenced
this pull request
Aug 9, 2026
…tes, and pin the factsheet heading census (#1779) * test(factsheets): census every h1 in the factsheet detail document The hero <h1> and the portaled print sheet's <h1> are correct and mutually exclusive by construction: on screen `.factsheet-print-sheet { display: none }` removes the print subtree, and in print `html.factsheets-printing body > *:not(.factsheet-print-portal)` removes the shell that owns the hero. Neither state exposes two headings to the accessibility tree, and the printed PDF is a separate document whose section headings are already <h2>, so demoting its title would leave it with no top-level heading. The real gap was that the existing assertion was scoped to the page testid, so the document-level invariant was asserted nowhere and a stray third <h1> would not have been caught. Pin the census instead: exactly two, one per container, both carrying the title, plus a non-empty <h2> outline in the print sheet. jsdom applies no stylesheet, so a census is the right guard rather than a visibility assertion. FactsheetPrintSheet stays in factsheet-detail-page.tsx — design-system-contract-utils.mjs scopes its raw-colour exemption to the literal factsheet-print-sheet marker and fails closed if it moves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * docs(testing): record the verified recipe for restoring local browser gates The remote/Cloud drift note said to delegate browser proof to CI and left the impression that local gates were unrecoverable. They are recoverable; the blocker was two separate image faults, and the second is why the obvious fix looks impossible. The baked node_modules is stale or incomplete — containers have shipped none at all, and earlier ones reported playwright 1.62.0 against a locked 1.62.1 with tailwind-merge absent entirely, which is an incomplete install rather than a version skew, so the lockfile pin was never wrong. And npm ci cannot repair it because jsdom@30.0.1 requires node ^22.22.2 || ^24.15.0 || >=26.0.0 while images have shipped v24.13.0, so the install dies on EBADENGINE under engine-strict. Installing Node 24.19.0 clears that, npm ci then exits 0 and parity reports all seven pinned packages, and `npx playwright install` supplies Chromium 1234 (images ship only 1194). Verified end to end this session, launch included. Keeps the existing Stop intact and makes it cheap to honour: install the matching revision rather than forcing a run against 1194. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * fix(bundle-budget): split production weight from mockup scratch One number could not honestly answer two questions. totalGzipBytes summed every built client chunk, including src/app/mockups/** design scratch that 404s in production, against a ceiling named as though it were production weight. #13 held that mockup chunks are not a production bundle; this gate charged them anyway, which is how PR #1580 blocked at +10.1% for chunks no user can load. #252 recorded the contradiction and left the metric undecided. Measured on a clean build of main at af85cbc, the blur had become the whole signal: 1546.5 KiB total was +9.96% of the 1406.4 KiB baseline — 576 bytes from failing Build — while production-only was 1279.1 KiB, 9.06% BELOW that same baseline. Every byte of the apparent regression was design scratch (267.5 KiB across 76 chunks over 66 mockup routes) and production had actually shrunk. latency-audit-2026-07-28 corroborates: 1,309,274 bytes then against 1,309,772 production-only now, flat to +0.04%, so the 2026-08-04 bump to 1,440,201 had absorbed mockup growth as production growth. Raising the ceiling again would have hidden that permanently, so this splits rather than ratchets. production (10%) covers every chunk a non-mockup route reaches plus chunks no manifest claims — framework, polyfills, runtime. mockups (25%) covers chunks reachable only from /mockups/**, as a runaway detector rather than a per-mockup gate; a ceiling tight enough to fire on the next mockup would just be --update'd reflexively. A chunk shared by both counts as production because it would be built either way. Attribution reads the per-route *_client-reference-manifest.js files under .next/server/app, since Next 16 webpack emits no app-build-manifest.json, and fails closed when that tree is missing or resolves no routes so the buckets can never silently collapse. Both fail paths proven against the real build. Also captures #296: pr-handoff-stop.test.ts fails in any root container because it chmods a fixture dir to force a write failure and root ignores permission bits — pre-existing, reproduced on clean af85cbc. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * fix(bundle-budget): fail closed on bad manifests and zero baselines Review feedback on PR #1779: treat unparseable route manifests as fatal attribution errors instead of counting them as resolved empty routes, and handle a zero mockup/production baseline without NaN percentage math. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GuideDialog/Sheetpatterns and production tokens (Clinical Sky, Geist, 48px tap targets)./mockups/guide-wireframes*so device frames are not competing with a second composer; regeneratesdocs/site-map.md.Preview routes
Local:
npm run ensure, then:/mockups/guide-wireframes/mockups/guide-wireframes/tip-atlas/mockups/guide-wireframes/help-hub/mockups/guide-wireframes/walkthroughScreenshots
Guide wireframes index
Tip Atlas tip detail
Help Hub desktop and phone
Walkthrough step 2
Verification
npm run workflow:flightplan -- --write-evidence --files …— risk classui; evidence.local/workflow-evidence/2026-08-02T11-30-47-811Z-flightplan.jsonnpx eslint+tscon new mockup paths — cleannpm run docs:update— site map includes new routesnpm run test -- tests/site-map.test.ts—Test Files 1 passed (1)/Tests 6 passed (6)ALLOW_BUILD_WITH_DEV_SERVER=1 npm run verify:pr-local -- --files <mockup paths>—Test Files 475 passed (475)/Tests 4971 passed | 4 skipped/ build +Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites).npm run ensure→http://localhost:4461; all four mockup routes HTTP 200; browser QA after chrome suppressionnpm run verify:ui— not run: design-scratch mockups only; no production chrome/owner journey change beyond suppressing shared mockup composer on these routesRisk and rollout
mockupsEnabled); no productGuideDialogbehaviour change.Notes
To show artifacts inline, enable in settings.