feat(design-system): build ErrorState and ratchet raw gap literals - #1786
Merged
Conversation
GATES.md §3 lists the prohibition "Render '0 matches' after a failed request" with the gate "ErrorState adoption + check", status planned. Measured at origin/main 199b303, ErrorState existed nowhere in src or tests — only in COMPONENTS.md, GATES.md and SPEC.md. This builds it. The invariant is clinical, not cosmetic. A search that failed has no count to report, so reporting zero is a false statement about the corpus: on the services page "0 matches" asserts there are no crisis services when the search never ran, and on favourites it reads as "you have saved nothing" rather than "we could not load them". COMPONENTS.md:322 draws the same line from the other side — "no result count is available" is not a MissingValue. The component therefore takes no count and no children. There is no prop through which a number can arrive, and the generated dtsPropsFor entry now records that as the published API. The one remaining route, a caller writing a count into title or body, is covered by a development-time tripwire that matches a figure against a counted noun ("0 matches", "no results") so an error code or a duration does not trip it. It warns and never throws: on the one screen already reporting a failure, a thrown error is a blank page. Requirements came from the surfaces that hand-roll this guard today. Three do, and their comments state the rule outright: search-results-header-band.tsx:210 ("no number may reach the DOM"), services-navigator-page.tsx:634 ("a blocked registry must not reach the band as '0 matches'") and favourites-command-library-page.tsx:1182. They are correct, just not shared; converting them is a live-look change and deliberately not here. Three further sites carried into this task as hand-rolled guards are not that, measured at this HEAD, and are recorded so the next reader does not convert them: differentials-home.tsx:716,729 renders "0 matches"/"No matches" when sourcesChecked is true, i.e. a legitimate zero after a search that SUCCEEDED; specifiers-home-page.tsx:211 is a comment about not showing a stale zero above real catalogue results, and lives in src/components/specifiers/, not clinical-dashboard/; document-search-results gates on recordStatus for loading, not for a failed count. Registered per gates 11 and 12: source, design-sync export, preview, prop contract, publication test entry, behavioural DOM tests, adoption-contract family, and both generators regenerated (54 components, 59 roots). Uses the shared floatingControl recipe rather than a hand-rolled control, so the tap floor, focus ring and forced-colors border come from one owner. Adds zero arbitrary padding/gap/radius/line-height, so the ratchet landed in the previous commit is unaffected and the two stay separately revertible. Scope: ErrorState only. OfflineState, PermissionDeniedState, NotFoundState and UnavailableState share the pattern but have no gate pointing at them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…uncovered #1780 landed rawPaddingLiterals, rawRadiusLiterals and rawLineHeightLiterals for #262 part 3. Gap was the one remaining spacing surface a hand-picked value could hide in: gap-[9px] and `gap: 18px` were counted by no ratchet at all. This adds rawGapLiterals on that commit's own predicate and wiring. Measured against origin/main ef9bb51: 34 sites across 9 files — 21 Tailwind utilities, every one under src/components/therapy-compass/, plus 13 CSS declarations in globals.css that a utility-only scan misses. Covering both spellings is the same reason #1780 counts both: otherwise a literal escapes by moving from a class into globals.css. Kept as its own metric rather than folded into rawPaddingLiterals so the therapy-compass cleanup can be paid down and re-pinned independently of the padding debt, which is spread across fifteen unrelated files. Reuses RAW_LITERAL_VALUE unchanged, so a value containing a CSS function (env(, clamp(, max(, calc() stays a sanctioned computed form and is exempt. Also corrects the §3 prohibition row, which #1780 left reading "implemented-partial (colour/shadow/tap literals only)" and which named none of the metrics it had just shipped. A row that understates shipped work is what sends the next session to rebuild it — this change was itself started as a duplicate of #262 part 3 for exactly that reason. Mutation-verified in both halves, each naming the metric and the path: a gap-[3px] utility gives "rawGapLiterals increased from 34 to 35" plus "at src/components/ui/missing-value.tsx increased from 0 to 1"; a `gap: 19px` declaration gives the same total plus "at src/app/globals.css increased from 13 to 14". Baseline diff is additive only. legacyShadowAliases measures 218 against its pinned 220 on main; that slack is left exactly as found. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four rows (#298-#301), added with npm run issues:add. Ids 295-297 were claimed by main while this branch was open, which is ledger #156's read-modify-write race behaving exactly as recorded. - #298 (P2 task) the ErrorState enforcement check. GATES.md still reads "planned" for the 0-matches prohibition and nothing in scripts/ or eslint-rules/ references ErrorState, so the component exists but is not required. Deliberately not flipped to implemented. - #299 (P3 task) adopting ErrorState at the three surfaces that genuinely hand-roll the guard. Live-look change, downstream of the redesign. - #300 (P2 issue) three sites miscarried into M4 as guards that are not, so the next reader does not convert them. differentials-home renders its zero after a search that SUCCEEDED. - #301 (P3 issue) two sessions built #262 part 3 in parallel because the §3 row understated what had shipped. Proposes asserting that every baseline metric key appears in GATES.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6ba5eb2d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
14 tasks
BigSimmo
enabled auto-merge (squash)
August 9, 2026 11:18
BigSimmo
disabled auto-merge
August 9, 2026 11:21
Codex review on #1786 (P2). The doc comment described the tripwire as development-only, but the emitter only silenced NODE_ENV === "test", so a production caller supplying count-bearing title/body copy had the full caller-provided string written to console.warn. On a clinical surface that copy can quote the query — "0 results for <query>" — which turns a copy defect into a disclosure risk. Nobody reads a production browser console for design-system warnings, so the emit is now development-only and an unset NODE_ENV is treated as production: fail quiet. The gate is an exported predicate rather than an inline comparison because an inline comparison is untestable here. Vite statically replaces process.env.NODE_ENV inside src/ modules, so under Vitest the check compiles to `"test" === "development"` and no stubEnv can move it. The review asked for a production console-spy check; written that way it would have passed while proving nothing, staying silent for the wrong reason and continuing to pass even if the guard were deleted. shouldEmitErrorStateDiagnostic is asserted directly instead — development true; production, test and unset false — with the console spy kept alongside as the weaker check that catches an emitter which warns unconditionally. Verified: typecheck 0 errors, lint 0, format:check 0, check:design-system-contract 0, Tests 70 passed (70) across error-state.dom and design-sync-visual-exports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Three independently revertible commits, each listed separately because they are unrelated changes that happen to share a session.
ErrorState, the gate with nothing behind it.GATES.md§3 lists the prohibition "Render '0 matches' after a failed request" with the gate "ErrorStateadoption + check", statusplanned. Measured atorigin/main199b303b7,ErrorStateexisted nowhere insrcortests— only inCOMPONENTS.md,GATES.mdandSPEC.md. This builds and registers it. The invariant is clinical, not cosmetic: a search that failed has no count to report, so reporting zero is a false statement about the corpus. On the services page "0 matches" asserts there are no crisis services when the search never ran. The component therefore takes no count and no children — there is no prop through which a number can arrive, and the generateddtsPropsForentry now records that as the published API. The one remaining route, a caller writing a count intotitle/body, is covered by a development-time tripwire that matches a figure against a counted noun, so "Error 503" and "retry in 30 seconds" do not trip it. It warns and never throws: on the one screen already reporting a failure, a thrown error is a blank page. Registered per gates 11 and 12 (source, export, preview, prop contract, publication test, behavioural DOM tests, adoption family, both generators regenerated).rawGapLiterals, the family#1780left uncovered.#1780landedrawPaddingLiterals,rawRadiusLiteralsandrawLineHeightLiteralsfor#262part 3. Gap was the one remaining spacing surface a hand-picked value could hide in:gap-[9px]andgap: 18pxwere counted by no ratchet at all. 34 sites across 9 files — 21 Tailwind utilities, all undertherapy-compass/, plus 13 CSS declarations inglobals.cssthat a utility-only scan misses. Built on#1780's ownRAW_LITERAL_VALUEpredicate and wiring, so values containing a CSS function (env(,clamp(,max(,calc() stay sanctioned computed forms and remain exempt. Kept as its own metric rather than folded intorawPaddingLiteralsso the therapy-compass cleanup can be paid down and re-pinned independently. This commit also corrects the §3 row and the §1 summary, which#1780left readingimplemented-partial (colour/shadow/tap literals only)while naming none of the metrics it had just shipped.Four outstanding-issues rows (
#298–#301), written withnpm run issues:add. The ErrorState enforcement check that is still missing; adoptingErrorStateat the three real surfaces; three sites miscarried as guards that must not be converted; and the duplicate-work incident below.Why the third commit matters more than it looks. This branch originally contained a full parallel implementation of
#262part 3 —arbitraryPadding/arbitraryGap/arbitraryRadius/arbitraryLeading— built against the same four files as#1780, on the same day, by a session that had readGATES.mdfirst. The duplicate was discovered only when syncing before opening this PR, and was dropped. Root cause is recorded as#301: the §3 row advertised the work as unstarted, so it read as available. This is the same failure as the 2026-08-09 finding that four of#264's six prohibitions were already gated while their rows readplanned. Both rows are corrected here;#301proposes asserting that every metric key indesign-system-contract-baseline.jsonappears somewhere inGATES.md, so a shipped gate cannot keep advertising itself as unbuilt.Three claims inherited from the session brief failed measurement and are recorded rather than repeated: padding measured 77 raw, not 65;
differentials-home.tsx:716,729renders its "0 matches" whensourcesCheckedis true, i.e. a legitimate zero after a search that succeeded, not a failed-request guard; andspecifiers-home-page.tsxis not underclinical-dashboard/. Only three of the six cited surfaces genuinely hand-roll the guard.Verification
npm run typecheck— exit 0, zeroerror TSnpm run lint— exit 0npm run format:check— exit 0 (whole tree)npm run check:design-system-contract— exit 0.Scale ratchets: raw padding literals 67; raw radius literals 24; raw gap literals 34; raw line-height literals 3.·design-system adoption checked: 54 components, 59 roots·design-sync contract checked: 54 components and 7 guidelinesnpm run check:icon-scale—✓ icon-scale: no retired 4.5 (18px) half-step icon sizes in src.(not inside the contract check, and strict)npm run check:type-scale—✓ type-scale: no arbitrary text-[<n>px|rem|em] font sizes in src.npm run check:outstanding-issues—299 rows (147 open, 152 archived), unique ids, next-id=302 above the highest, no merge driver, no ids deleted from base ef9bb51662dbTest Files 3 passed (3),Tests 100 passed (100)(error-state.dom,design-sync-visual-exports,design-system-contract-utils)The gap ratchet is mutation-verified in both halves, each naming the metric and the path. A
gap-[3px]utility givesrawGapLiterals increased from 34 to 35plusrawGapLiterals at src/components/ui/missing-value.tsx increased from 0 to 1; agap: 19pxdeclaration gives the same total plusrawGapLiterals at src/app/globals.css increased from 13 to 14. All 34 recorded sites were read in source before the baseline was pinned, per the lesson inGATES.md§5. The baseline diff is additive only — 12 insertions, 0 deletions.npm run verify:cheapwas run against the pre-reshape version of this work and exited 1 withTest Files 7 failed | 541 passed (548). Every failure was attributed and none belonged to the diff: 5 files passed on isolated re-run (failures wereTest timed out in 30000msandEPERM: operation not permitted, rename ...clinical-kb-heavy-locks...owner.json, i.e. cross-worktree contention),codex-cloud-setupwas a cold-cache 30s timeout that passes alone (Tests 23 passed (23)), andpr-handoff-stop's 5 failures reproduce identically on a pristine199b303b7worktree carrying none of this work (Tests 5 failed | 29 passed (34)). It has not been re-run since the reshape ontoef9bb5166; the gates listed above have.npm run verify:ui— UI verification not run:ErrorStateis built and registered but adopted by no route, so no rendered surface changes in this PR. Adoption is#299and carries the Chromium pass.Risk and rollout
ErrorStatehas no consumers, so it cannot change a rendered page; the gap ratchet is a per-path ratchet pinned at the measured 34, so it fails only on new debt and cannot fail on existing code.git revertany single commit. They are deliberately separate — a component and a gate must be independently revertible — and each was verified to leave the other's gate green. Note that this repo squash-merges, so a post-merge revert of one item means reverting its hunks of the squash commit by hand.Notes
Auto-merge is deliberately not armed: this PR changes a gate.
legacyShadowAliasesmeasures 218 against its pinned 220 onmain. That slack is left exactly as found — tightening an unrelated ratchet here would have fused two independently revertible commits, and retiring those aliases is#262part 1.