docs(design-system): perfection pass — per-component contracts, playbook, authoring DoD, usage rules - #1537
Merged
Merged
Conversation
…ook, authoring DoD, usage rules - SPEC: named layout states (§4.13), migration/adoption playbook updated for 59e4c3d (§13), authoring definition of done (§14), main-is-gated rule. - COMPONENTS §9: binding contract for every existing component (34 blocks) — purpose, modes, states, keyboard/SR, tokens, rules, open defects mapped to closing PRs. - TOKENS §7: per-group allowed/forbidden usage rules. - GATES §3a: every new rule labelled. - README: set index, reading order, source-of-truth ranking. No visual or value changes. Verified: docs:check-links green, prettier clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BigSimmo
enabled auto-merge (squash)
July 31, 2026 12:46
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
Contributor
📝 WalkthroughWalkthroughThe design-system documentation now defines component contracts, responsive layout states, governance and migration rules, authoring completion criteria, documentation authority, and token usage constraints. ChangesDesign system documentation
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
PR Summary by Qododocs(design-system): define component contracts, token usage rules, and governance gates
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
docs/design-system/COMPONENTS.md (1)
830-839: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReplace the historical commit reference and narrow the test claim. Commit
0b0f393c7b1d78b81b1ae3eae3d29f219f303169is not an ancestor ofHEAD.tests/icon-button.dom.test.tsxcovers only theXicon, not the six affected icons. Use a stable issue identifier or add coverage for all affected icons.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design-system/COMPONENTS.md` around lines 830 - 839, Update the `ui-primitives.tsx` recipes contract in `docs/design-system/COMPONENTS.md` to remove the non-ancestor commit reference and replace it with a stable issue identifier. Narrow the focused DOM-test claim to the coverage actually provided by `tests/icon-button.dom.test.tsx`, or add tests covering all six affected icons before documenting broader coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design-system/COMPONENTS.md`:
- Around line 704-711: Update the Tabs contract in the `Tabs` section to
explicitly define the all-disabled state: either require at least one enabled
tab or document an empty-state behavior, including focus handling and user
announcement. Ensure the chosen rule preserves the guarantee that the tab order
is never empty.
- Around line 719-725: Update the Pagination contract to explicitly define
pageCount = 0 behavior: either require pageCount to be at least 1 through
validation, or specify that zero-page data hides the paginator and prevents
rendering an invalid “Page X of 0” state. Ensure page validation and compact
rendering follow the chosen invariant.
- Around line 841-844: Update the OverlayProvider entry in COMPONENTS.md to
describe OverlayRoot as a planned replacement rather than an existing successor.
State that OverlayRoot is currently only documented and not implemented or
imported, while preserving the guidance not to adopt OverlayProvider.
In `@docs/design-system/GATES.md`:
- Around line 99-109: The newly added gate rows use undeclared status labels
such as “implemented” and “manual/infra.” Update the Status values in the “Rules
added by the perfection pass” table to use only the declared
vocabulary—implemented-blocking, implemented-partial, planned, or manual—and
move infrastructure or execution notes into the table’s Evidence column.
In `@docs/design-system/README.md`:
- Around line 3-6: Clarify the authority statement in the design-system README
by replacing “system of record” with “primary documentation set,” or explicitly
limiting its authority to cases where the higher-ranked sources do not conflict.
Keep the existing source-of-truth ranking intact.
- Around line 21-24: Update the canonical references in the README’s
design-system provenance section: replace the abbreviated commit with the full
immutable SHA and provide a directly verifiable link to the canonical code, then
link the design project or document a versioned export with its verification
result. Preserve the existing claims about token ownership and product-surface
adoption without guessing unverifiable details.
In `@docs/design-system/SPEC.md`:
- Around line 680-685: Update the “States complete” and “Keyboard and screen
reader declared” checklist in SPEC.md to be applicability-based: require each
component to identify which states and interaction/accessibility behaviors
apply, while recording an explicit “n/a” reason for non-applicable items such as
busy, invalid, focus behavior, activation keys, and live-region policy. Preserve
the existing requirements for applicable states and behaviors.
- Around line 658-662: Align the ownership of the Quantity/text-base-minus
migration between this PR roadmap and COMPONENTS.md §9.9. Because PR 13
explicitly makes type-scale retirement the final step, move the Quantity
migration out of PR 9 and assign it to the type-retirement tranche near PR 13,
updating both documents consistently.
- Around line 690-691: Update the “Boundaries justified” rule in SPEC.md to
permit "use client" for hooks, browser APIs, event handlers, and other
client-only behavior. Retain the needless-client-boundary defect for components
with no client-only behavior.
In `@docs/design-system/TOKENS.md`:
- Line 126: Update the Clinical state row in TOKENS.md to include --info
alongside --danger*, --warning, and --success, applying the same allowed and
forbidden usage restrictions to the complete clinical/status palette.
- Line 132: Update the Density row in TOKENS.md to distinguish ownership and
access paths: identify --spacing-tap as the `@theme` utility token, while
documenting --tap-min, rows, and cells as v2 structural tokens with their
appropriate access path. Remove the wording that groups all of these tokens
under `@theme` utilities.
- Around line 129-132: Update the Elevation and Density rows in the token
inventory to remove the literal 1px and 48px values, replacing them with the
corresponding token or semantic role references. Preserve the existing guidance
while keeping numeric definitions exclusively in their owner files.
---
Nitpick comments:
In `@docs/design-system/COMPONENTS.md`:
- Around line 830-839: Update the `ui-primitives.tsx` recipes contract in
`docs/design-system/COMPONENTS.md` to remove the non-ancestor commit reference
and replace it with a stable issue identifier. Narrow the focused DOM-test claim
to the coverage actually provided by `tests/icon-button.dom.test.tsx`, or add
tests covering all six affected icons before documenting broader coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 42c57342-1c33-4bd7-8805-9218863e6eeb
📒 Files selected for processing (5)
docs/design-system/COMPONENTS.mddocs/design-system/GATES.mddocs/design-system/README.mddocs/design-system/SPEC.mddocs/design-system/TOKENS.md
Comment on lines
+704
to
+711
| ### 9.17 `Tabs` | ||
|
|
||
| **Purpose.** View switching with real tab semantics. **Contract.** `aria-controls` only | ||
| for panels that exist in the DOM; an invalid or disabled selected value falls back to the | ||
| first enabled tab so the tab order is never empty; manual activation available for | ||
| expensive panels. **Rules.** Sort/filter choices are not tabs — they are | ||
| `SegmentedControl` (§9.18). **Open defects → PR.** phantom `aria-controls`, empty tab | ||
| order → contract-test tranche of PR 4; the split → its own tranche. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define the all-disabled Tabs state.
Because the contract does not require an enabled tab, “the first enabled tab” may not exist. Require at least one enabled tab, or define an explicit empty state with its focus and announcement behavior. This is necessary for the stated guarantee that the tab order is never empty.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/COMPONENTS.md` around lines 704 - 711, Update the Tabs
contract in the `Tabs` section to explicitly define the all-disabled state:
either require at least one enabled tab or document an empty-state behavior,
including focus handling and user announcement. Ensure the chosen rule preserves
the guarantee that the tab order is never empty.
Comment on lines
+719
to
+725
| ### 9.19 `Pagination` | ||
|
|
||
| **Contract.** `page`/`pageCount` clamped and validated; page change announces through | ||
| `LiveAnnouncer` and defines its focus policy; `compact` form is previous · "Page X of Y" · | ||
| next, used whenever the container cannot fit the numbered window (320px blocking). | ||
| **Open defects → PR.** unclamped props, 320px overflow, opacity disabled, no | ||
| focus/announce policy → PR 3, PR 8. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define pageCount = 0 behavior.
The contract validates page and pageCount but does not define whether pageCount must be at least 1. Without this rule, the component can render an invalid “Page X of 0” state. Add a minimum-page invariant or define the zero-page behavior and hide the paginator.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/COMPONENTS.md` around lines 719 - 725, Update the
Pagination contract to explicitly define pageCount = 0 behavior: either require
pageCount to be at least 1 through validation, or specify that zero-page data
hides the paginator and prevents rendering an invalid “Page X of 0” state.
Ensure page validation and compact rendering follow the chosen invariant.
Comment on lines
+841
to
+844
| ### 9.34 `OverlayProvider` — superseded | ||
|
|
||
| `main`-tier, zero imports. Deleted when `OverlayRoot` (§7) lands in PR 10. Do not adopt | ||
| it in the interim. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.{ts,tsx,js,jsx}' \
'\bOverlayProvider\b|\bOverlayRoot\b' .Repository: BigSimmo/Database
Length of output: 394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tracked references ---'
git ls-files -z | xargs -0 rg -n \
'\bOverlayProvider\b|\bOverlayRoot\b' --no-heading || true
printf '%s\n' '--- OverlayProvider source ---'
if [ -f src/components/ui/OverlayProvider.tsx ]; then
cat -n src/components/ui/OverlayProvider.tsx
fi
printf '%s\n' '--- design-system context ---'
sed -n '820,855p' docs/design-system/COMPONENTS.mdRepository: BigSimmo/Database
Length of output: 20351
Describe OverlayRoot as the planned replacement. OverlayProvider has no production or test imports, but OverlayRoot exists only in design documentation and is not implemented or imported.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/COMPONENTS.md` around lines 841 - 844, Update the
OverlayProvider entry in COMPONENTS.md to describe OverlayRoot as a planned
replacement rather than an existing successor. State that OverlayRoot is
currently only documented and not implemented or imported, while preserving the
guidance not to adopt OverlayProvider.
Comment on lines
+99
to
+109
| ## 3a · Rules added by the perfection pass (31 July, second half) | ||
|
|
||
| | Rule | Gate | Status | | ||
| | ---------------------------------------------------------------------------------- | -------------------------------------------------------- | -------------------------------------------------------------- | | ||
| | Authoring definition of done (SPEC §14) — starred proofs before registration | Gate 12 extension + review checklist | **manual → planned** (PR 12) | | ||
| | Named layout states, declared transitions, minimum widths (SPEC §4.13) | 320px/compact acceptance in the visual matrix | planned (blocking for the eight new components) | | ||
| | Main is gated; unverified direct merges are a defect (SPEC §12) | Branch protection + required checks | **manual/infra** — proven necessary by `369c01f86` | | ||
| | Per-group token usage rules (TOKENS §7) | Colour-boundary + z + motion lints | planned (PR 9) — decoration-on-text tier partially gated today | | ||
| | Per-component contracts (COMPONENTS §9) bind at review; defects map to closing PRs | Review checklist; contract tests land with each named PR | manual until each PR | | ||
| | `ui-primitives.tsx` changes require the focused DOM tests | `tests/ui-primitives.dom.test.tsx` + `icon-button` | implemented (run discipline manual) | | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the declared gate-status vocabulary.
Lines [3]-[6] define implemented-blocking, implemented-partial, planned, and manual. The new rows use implemented and manual/infra, which are not defined labels. Use a declared status value and place additional execution details in the Evidence column so reviewers and tooling can classify each rule consistently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/GATES.md` around lines 99 - 109, The newly added gate rows
use undeclared status labels such as “implemented” and “manual/infra.” Update
the Status values in the “Rules added by the perfection pass” table to use only
the declared vocabulary—implemented-blocking, implemented-partial, planned, or
manual—and move infrastructure or execution notes into the table’s Evidence
column.
Comment on lines
+3
to
+6
| The system of record for the v2 design system. **Rules and roles live here; values live | ||
| only in the token files.** Source-of-truth ranking: `AGENTS.md` → `ckb-v2-tokens.css` → | ||
| committed tests → `.design-sync/conventions.md` → this set. Where this set contradicts a | ||
| higher source, the higher source wins and the contradiction is a defect here. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Clarify the authority statement.
Line 3 calls this document set the system of record. Lines 4-6 rank it below four other sources. Use primary documentation set, or state that this set is authoritative only when higher-ranked sources do not conflict. This prevents readers from applying a lower-ranked rule as the canonical rule.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/README.md` around lines 3 - 6, Clarify the authority
statement in the design-system README by replacing “system of record” with
“primary documentation set,” or explicitly limiting its authority to cases where
the higher-ranked sources do not conflict. Keep the existing source-of-truth
ranking intact.
Comment on lines
+680
to
+685
| 3. **States complete.** default · hover · active · focus-visible · disabled (encoded via | ||
| `controlBase`, never opacity) · busy · invalid · long-content · `compact`/320px · | ||
| dark ★ · forced colours ★ · reduced motion · print (or an explicit "print n/a" with | ||
| the reason). | ||
| 4. **Keyboard and screen reader declared** — focus order, activation keys, announced name | ||
| and role, live-region policy (via `LiveAnnouncer` only). |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the state checklist applicability-based.
The checklist requires busy, invalid, focus behavior, activation keys, and live-region policy for every component. Static components such as PanelHeading and SafeBoldText cannot meaningfully implement all of these states. Require each component to declare applicable states and record an explicit n/a reason for non-applicable states.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/SPEC.md` around lines 680 - 685, Update the “States
complete” and “Keyboard and screen reader declared” checklist in SPEC.md to be
applicability-based: require each component to identify which states and
interaction/accessibility behaviors apply, while recording an explicit “n/a”
reason for non-applicable items such as busy, invalid, focus behavior,
activation keys, and live-region policy. Preserve the existing requirements for
applicable states and behaviors.
Comment on lines
+690
to
+691
| 7. **Boundaries justified.** `"use client"` only with a hook or browser API; refs | ||
| forwarded on focusable primitives; `className` accepted; `data-slot` over `testId`. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
printf '%s\n' '--- candidate file ---'
git ls-files 'docs/design-system/SPEC.md'
printf '%s\n' '--- relevant specification text ---'
sed -n '650,715p' docs/design-system/SPEC.md
printf '%s\n' '--- client-boundary and Button references ---'
rg -n -C 3 '"use client"|use client|event handler|event handlers|Button' docs src app components 2>/dev/null | head -240
printf '%s\n' '--- repository metadata relevant to local review checks ---'
git status --short
if [ -f package.json ]; then
node -e 'const p=require("./package.json"); console.log(JSON.stringify({scripts:p.scripts}, null, 2))'
fiRepository: BigSimmo/Database
Length of output: 50373
🏁 Script executed:
printf '%s\n' '--- Button files ---'
fd -i 'button' src/components docs/design-system
printf '%s\n' '--- Button declarations and directives ---'
rg -n -C 6 'use client|export .*Button|function Button|const Button|onClick|on[A-Z]' src/components/ui docs/design-system docs/frontend-architecture.md
printf '%s\n' '--- design-system component boundary guidance ---'
rg -n -C 5 'Boundar|client|event handler|event handlers|callback|Button' docs/design-system docs/frontend-architecture.md | head -260Repository: BigSimmo/Database
Length of output: 50376
🏁 Script executed:
printf '%s\n' '--- exact Button symbols and files ---'
rg -l --glob '!node_modules/**' --glob '!coverage/**' --glob '!*lock*' 'export (default )?(function|const) Button|function Button|ButtonProps' .
printf '%s\n' '--- Button contract section ---'
sed -n '535,578p' docs/design-system/COMPONENTS.md
printf '%s\n' '--- architecture boundary rule ---'
sed -n '1,32p' docs/frontend-architecture.md
printf '%s\n' '--- narrow client-boundary references ---'
rg -n -C 4 'needless client boundary|client boundary|event handlers|event handler' docs src --glob '*.md' --glob '*.tsx' --glob '*.ts' | head -180Repository: BigSimmo/Database
Length of output: 10391
🏁 Script executed:
printf '%s\n' '--- Button-related implementation lines ---'
rg -n -C 8 '(^|[^A-Za-z])Button([^A-Za-z]|$)|AsyncButton|onClick' \
src/components/ui-primitives.tsx \
src/components/navigation-back-button.tsx \
src/components/document-search-mockups.tsx \
src/app/mockups/answer-evidence-popups/page.tsx
printf '%s\n' '--- all tracked UI component files ---'
git ls-files 'src/components/ui/*' 'src/components/ui-primitives.tsx'
printf '%s\n' '--- client directives in component sources ---'
rg -l --glob '*.tsx' --glob '*.ts' '^"use client";' src/components src/app | sortRepository: BigSimmo/Database
Length of output: 23363
Broaden the client-boundary rule.
Replace "use client" only with a hook or browser API with a rule that also allows event handlers and other client-only behavior. Keep the “needless client boundary” defect for components that have no client-only behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/SPEC.md` around lines 690 - 691, Update the “Boundaries
justified” rule in SPEC.md to permit "use client" for hooks, browser APIs, event
handlers, and other client-only behavior. Retain the needless-client-boundary
defect for components with no client-only behavior.
| | Group | Allowed | Forbidden | | ||
| | ----------------------------------------------------------------------------- | --------------------------------------------- | ------------------------------------------------------------------------------------------- | | ||
| | Ink (`--text*`, `--decoration-soft`, `--disabled`) | Per the SPEC §4.3 role table | `--decoration-soft`/`--text-soft` on any text node; darkening the decoration tier to "pass" | | ||
| | Clinical state (`--danger*`, `--warning`, `--success`) | Source state and sanctioned urgency only | Decoration, numerals, charts, identity, category colour | |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add --info to the Clinical state rule.
TOKENS.md identifies --success and --info as the clinical/status palette at Line 105. This row governs --danger*, --warning, and --success only. Add --info so the reserved palette has the same restrictions. Otherwise, the contract leaves --info without an allowed or forbidden usage rule.
Proposed update
-| Clinical state (`--danger*`, `--warning`, `--success`)
+| Clinical state (`--danger*`, `--warning`, `--success`, `--info`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | Clinical state (`--danger*`, `--warning`, `--success`) | Source state and sanctioned urgency only | Decoration, numerals, charts, identity, category colour | | |
| | Clinical state (`--danger*`, `--warning`, `--success`, `--info`) | Source state and sanctioned urgency only | Decoration, numerals, charts, identity, category colour | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/TOKENS.md` at line 126, Update the Clinical state row in
TOKENS.md to include --info alongside --danger*, --warning, and --success,
applying the same allowed and forbidden usage restrictions to the complete
clinical/status palette.
Comment on lines
+129
to
+132
| | Elevation (`--e0…--e4`, `--ring-hairline`, `--shadow-inset`, `--shadow-well`) | One edge owner; ladder per SPEC §4.7 | 1px spread terms; child heavier than parent; v2 redeclaring the bevel | | ||
| | Stacking (`--z-*`) | Via `OverlayRoot`/named rungs only | Any raw `z-` value; new rungs without an `--eN` partner | | ||
| | Motion (`--duration-*`, `--ease-*`) | All transitions/animations | Hardcoded durations; animating layout properties | | ||
| | Density (`--spacing-tap`, `--tap-min`, rows, cells) | Utilities from `@theme`; `--tap-min` as alias | Setting the pair independently; reducing any 48px target | |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Remove literal values from the token inventory.
Lines 3-5 prohibit restating values in this file. The new rules repeat 1px at Line 129 and 48px at Line 132. Replace these literals with token or role references. Keep numeric values in the owner files.
Proposed update
-| ... | 1px spread terms; ...
+| ... | Spread terms; ...
-| ... | ... reducing any 48px target |
+| ... | ... reducing any `--tap-min` target |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | Elevation (`--e0…--e4`, `--ring-hairline`, `--shadow-inset`, `--shadow-well`) | One edge owner; ladder per SPEC §4.7 | 1px spread terms; child heavier than parent; v2 redeclaring the bevel | | |
| | Stacking (`--z-*`) | Via `OverlayRoot`/named rungs only | Any raw `z-` value; new rungs without an `--eN` partner | | |
| | Motion (`--duration-*`, `--ease-*`) | All transitions/animations | Hardcoded durations; animating layout properties | | |
| | Density (`--spacing-tap`, `--tap-min`, rows, cells) | Utilities from `@theme`; `--tap-min` as alias | Setting the pair independently; reducing any 48px target | | |
| | Elevation (`--e0…--e4`, `--ring-hairline`, `--shadow-inset`, `--shadow-well`) | One edge owner; ladder per SPEC §4.7 | Spread terms; child heavier than parent; v2 redeclaring the bevel | | |
| | Stacking (`--z-*`) | Via `OverlayRoot`/named rungs only | Any raw `z-` value; new rungs without an `--eN` partner | | |
| | Motion (`--duration-*`, `--ease-*`) | All transitions/animations | Hardcoded durations; animating layout properties | | |
| | Density (`--spacing-tap`, `--tap-min`, rows, cells) | Utilities from `@theme`; `--tap-min` as alias | Setting the pair independently; reducing any `--tap-min` target | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/TOKENS.md` around lines 129 - 132, Update the Elevation
and Density rows in the token inventory to remove the literal 1px and 48px
values, replacing them with the corresponding token or semantic role references.
Preserve the existing guidance while keeping numeric definitions exclusively in
their owner files.
| | Elevation (`--e0…--e4`, `--ring-hairline`, `--shadow-inset`, `--shadow-well`) | One edge owner; ladder per SPEC §4.7 | 1px spread terms; child heavier than parent; v2 redeclaring the bevel | | ||
| | Stacking (`--z-*`) | Via `OverlayRoot`/named rungs only | Any raw `z-` value; new rungs without an `--eN` partner | | ||
| | Motion (`--duration-*`, `--ease-*`) | All transitions/animations | Hardcoded durations; animating layout properties | | ||
| | Density (`--spacing-tap`, `--tap-min`, rows, cells) | Utilities from `@theme`; `--tap-min` as alias | Setting the pair independently; reducing any 48px target | |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Separate @theme utilities from v2 density tokens.
Line 101 states that --spacing-tap belongs to @theme and generates utilities. Line 54 lists --tap-min, rows, and cells in the v2 structural inventory. The text Utilities from @theme`` therefore assigns the wrong owner to part of this group. State the access path for each token subset.
Proposed update
-| Utilities from `@theme`; `--tap-min` as alias
+| `--spacing-tap` via `@theme` utilities; `--tap-min` as its v2 alias; rows/cells via v2 semantic tokens📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | Density (`--spacing-tap`, `--tap-min`, rows, cells) | Utilities from `@theme`; `--tap-min` as alias | Setting the pair independently; reducing any 48px target | | |
| | Density (`--spacing-tap`, `--tap-min`, rows, cells) | `--spacing-tap` via `@theme` utilities; `--tap-min` as its v2 alias; rows/cells via v2 semantic tokens | Setting the pair independently; reducing any 48px target | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design-system/TOKENS.md` at line 132, Update the Density row in
TOKENS.md to distinguish ownership and access paths: identify --spacing-tap as
the `@theme` utility token, while documenting --tap-min, rows, and cells as v2
structural tokens with their appropriate access path. Remove the wording that
groups all of these tokens under `@theme` utilities.
Code Review by Qodo
1. Bad token file path
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+21
to
+23
| Canonical code: branch `claude/clinical-kb-design-system-333a69` at `59e4c3dfc` | ||
| (`src/app/ckb-v2-tokens.css` is the single token file; nothing is adopted by product | ||
| surfaces yet). Design project `08d6f126-3fd0-4764-aedf-0062a467280a` conforms to the repo |
There was a problem hiding this comment.
1. Bad token file path 🐞 Bug ≡ Correctness
docs/design-system/README.md states the canonical token file is src/app/ckb-v2-tokens.css, but this PR branch’s token definitions live in src/app/globals.css, so the README’s source-of-truth pointer is misleading for anyone trying to locate tokens in the current repo state.
Agent Prompt
### Issue description
`docs/design-system/README.md` introduces a “canonical code” pointer to `src/app/ckb-v2-tokens.css`, but that file is not present in this PR branch’s worktree; the actual token definitions and `@theme` token generation live in `src/app/globals.css`. This makes the README’s source-of-truth ranking and “single token file” claim difficult to follow in the current repo state.
### Issue Context
Other documents may refer to historical branches/commits, but this README is newly added in this PR and should not direct readers to a path they cannot open in the current worktree.
### Fix Focus Areas
- docs/design-system/README.md[3-6]
- docs/design-system/README.md[21-23]
### Expected change
- Update the README to reference the correct token file path for this worktree (likely `src/app/globals.css`), or if `src/app/ckb-v2-tokens.css` is intended to exist, add it (or adjust wording to make it explicit that it exists only on the referenced branch/commit, not in the current worktree).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
7 tasks
This was referenced Jul 31, 2026
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
Docs-only; no visual, value, or behaviour changes. Builds on #1531.
Verification
🤖 Generated with Claude Code
Summary by CodeRabbit