Claude/errorstate ratchets m4 - #1784
Conversation
Both rows were left open after their implementing PRs merged. Verified in source against origin/main 199b303, not inferred: - #218 (cn() has no tailwind-merge) — src/components/ui-primitives.tsx:36 is twMergeClinical(...), package.json:267 carries tailwind-merge ^3.6.0, and src/lib/tailwind-merge.ts declares this repo's @theme scales to extendTailwindMerge. Landed in aeba5a2 (PR #1678). - #270 (declaring tap in tailwind-merge) — "tap" is listed in CLINICAL_TWMERGE_THEME.spacing at src/lib/tailwind-merge.ts:104, the stale 22-site/18-drop comment is corrected in place, and the pinning test in tests/tailwind-merge-config.test.ts is replaced by a positive same-variant/cross-variant suite. Landed in 80cf781 (PR #1738). Both commits confirmed ancestors of origin/main via git merge-base --is-ancestor. Rows written with npm run issues:done, never by hand; the writer also dropped #218 from the recommended queue and renumbered it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…#262) GATES.md §3's "Raw pixel size, padding, radius or line-height in markup" row has read implemented-partial (colour/shadow/tap literals only) since the table was written. These four metrics are the missing half, added inside check:design-system-contract rather than as a new npm script: a new script breaks docs:check-inventory until docs:update runs, and check:gate-manifest binds every verify:cheap:internal gate to the browser-less static-pr job. Measured against origin/main 199b303: arbitraryPadding 58 across 18 files (ratchet, per-path pins) arbitraryGap 21 across 8 files (ratchet, all in therapy-compass) arbitraryRadius 2 across 2 files (ratchet) arbitraryLeading 0 (hard zero + text backstop) Kept as four metrics rather than one pooled count so the 59th padding site cannot be traded against the first leading-[1.15]. Two exemptions, both measured. The first draft used the same (?!var\() lookahead the shadow and tracking ratchets carry and found 100 sites; 20 were not debt. Safe-area values (19) reference env(safe-area-inset-*) or this repo's runtime wrappers over it, which no token can restate and which docs/search-chrome-behaviour.md mandates — the strict reading fails a correct new phone composer and passes an incorrect one that drops the inset. Token-only arithmetic (1) is pl-[calc(var(--pad-card)_+_var(--rule-w))], the form the ratchet exists to encourage. Replacing the lookahead with isSanctionedRawValue also caught one site it had been hiding: px-[var(--pad-panel,1.5rem)], a literal fallback restating a TOKENS.md value. All 100 initial findings were read in source before anything was pinned, per the §5 lesson. Mutation-verified: each family fails naming the metric and the path, a leading-[…] outside any class root fails the text backstop, and both exemptions leave the gate green. GATES.md §6 records the evidence. legacyShadowAliases measures 218 against its pinned 220; that slack is left exactly as found, because tightening an unrelated ratchet here would make the two non-revertible as one commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
Four rows, all measured against origin/main 199b303 in this session: - #295 (P2 task) the ErrorState enforcement check. GATES.md:106 still reads "planned" and grep over scripts/ and eslint-rules/ returns zero references to ErrorState, so the component exists but nothing requires it. Deliberately not flipped to implemented. - #296 (P3 task) adopting ErrorState at the three surfaces that genuinely hand-roll the guard. Live-look change, downstream of the redesign. - #297 (P3 issue) the safe-area exemption in the new raw-value ratchet, which is by form rather than by absence of a literal. - #298 (P2 issue) the three sites carried into M4 as guards that are not guards, so the next reader does not convert them. Written with npm run issues:add, never by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 59 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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![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: f5173da96d
ℹ️ 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".
| * design defect. `leading-[…]` is at zero and stays there, so any text match at | ||
| * all is a real regression, whether or not the AST pass can see it. | ||
| */ | ||
| const ARBITRARY_LEADING_TEXT = /(?:^|[\s"'`])leading-\[([^\]]+)\]/g; |
There was a problem hiding this comment.
When an arbitrary line-height is returned through an expression the AST analyzer cannot resolve, a variant such as sm:leading-[1.15] also bypasses this whole-file backstop because the regex requires whitespace or a quote immediately before leading, while the actual preceding character is :. A focused probe using className={getTypography()} with that value produces zero AST findings and zero text matches, so the newly documented hard-zero line-height gate passes the regression; allow Tailwind variant prefixes in the backstop and add this unresolved-helper case to the contract tests.
Useful? React with 👍 / 👎.
| * character class so no non-padding utility can be swept in: `place-items-*` | ||
| * and `pointer-events-*` both start with `p` and neither can match. | ||
| */ | ||
| const ARBITRARY_PADDING_UTILITY = /^p(?:[xytrbl]|[se])?-\[([^\]]+)\]$/; |
There was a problem hiding this comment.
When logical arbitrary padding is published as a single-token recipe, for example export const logicalPadding = "ps-[13px]", this matcher never runs because looksLikeClassExpression still recognizes only p[trblxy] prefixes and therefore does not create a class root for ps- or pe-. A direct analyzer probe returns no arbitraryPadding finding while the equivalent pl-[13px] is detected, allowing new logical-padding debt to bypass the per-path ratchet; add s and e to the discovery prefix and cover both forms with a focused helper test.
Useful? React with 👍 / 👎.
|
Closing as superseded. Not abandoned work — two of its three commits turned out to be duplicates of work that landed on
The only part not covered by #1780 was the Superseded by #1786, which carries: Branch |
1 participant
Summary
Verification
npm run verify:pr-localDuring development, use
npm run verify:cheapas the faster iteration gate before the final PR-local preflight.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsFor retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract changes,
verify:pr-localrunseval:rag:offlineautomatically. Run the offline command directly during iteration before spending a live eval.npm run eval:retrieval:quality(must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed — CI cannot run it (needs live keys), so run it locally and paste the summary. A metadata/governance-weighting change once buried correct docs (recall 1.0→0.76) and only this eval caught it.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-onlywhen answer generation, the synthesis prompt, or answer post-processing changed (grounded-supported must not drop; citation-failure 0)npm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changednpm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedRisk and rollout
Complete this section for clinical, data, API, auth/privacy, workflow, dependency, build, or deployment changes.
Clinical Governance Preflight
Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes