Fix all component-library audit findings#897
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe PR adds design-system contract checks, migrates Therapy Compass to a dedicated stylesheet, standardizes interactive sizing with tap tokens, introduces shared async and surface primitives, centralizes theme bootstrapping, and expands UI, responsive, accessibility, and contract tests. ChangesDesign-system and shared UI migration
Therapy Compass migration
Validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9b16cac8b
ℹ️ 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".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #3477 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@scripts/check-design-system-contract.mjs`:
- Around line 104-105: Update the className validation around jsxClassText and
LEGACY_TAP_CLASS_TEST to inspect string and template literals within composed
JSX expressions, rather than testing only the full expression source. Ensure
literals such as "h-11" and "w-11" are individually checked for legacy tap
classes while preserving validation of ordinary className text and preventing
baseline totals from masking replacements.
- Around line 20-37: Update RAW_COLOR_EXEMPTIONS and the checker logic so
Therapy Compass and factsheet exemptions apply only to their documented fixed
paper-color rules, not entire files; preserve dark-mode enforcement for
application chrome. Add the missing factsheet category to the documented
category enumeration, and ensure all exemption categories are explicitly
represented.
- Around line 229-236: Update the paperRules extraction near the paper-rule
token assertion to validate that both selector sentinels are present and the
start sentinel precedes the end sentinel before calling slice. Make the contract
check fail when either index is -1 or the boundaries are misordered, while
preserving the existing token-leak assertion for a valid range.
In `@src/components/therapy-compass/screens/brief-screen.tsx`:
- Around line 77-104: Replace the incomplete tab semantics with toggle-button
semantics in the duration selector around the brief screen controls: remove
role="tab", role="tablist", and aria-selected, and use aria-pressed on each
button based on its active state. Apply the same correction to the
comparison-field selector in
src/components/therapy-compass/screens/compare-screen.tsx lines 189-217,
preserving the existing handlers and selection logic.
In `@src/components/therapy-compass/screens/sheets-screen.tsx`:
- Around line 271-278: Add aria-expanded={open} to the therapy picker trigger
button in the sheets screen, preserving its existing toggle behavior and
styling.
In `@src/components/therapy-compass/therapy-compass.css`:
- Around line 851-861: The .tc-clinician-track button currently limits the
interactive area to 42×24px. Update the button to be at least var(--spacing-tap)
wide and tall, while preserving the existing 42×24px visual pill through an
inner element or pseudo-element and retaining the current styling behavior.
- Around line 899-900: Update the therapy compass stylesheet by inserting an
empty line between the --tc-paper-shadow custom property and the background
declaration, and change the page-size value A4 to lowercase a4 in the affected
rule.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a47042ff-958e-4d37-a1ea-52ff25ed755c
📒 Files selected for processing (71)
docs/design-system.mdpackage.jsonscripts/check-design-system-contract.mjsscripts/design-system-contract-baseline.jsonsrc/app/globals.csssrc/app/layout.tsxsrc/app/therapy-compass/layout.tsxsrc/components/AccessibleTable.tsxsrc/components/ClinicalDashboard.tsxsrc/components/DocumentManagementActions.tsxsrc/components/DocumentViewer.tsxsrc/components/applications-launcher-page.tsxsrc/components/clinical-dashboard/ClinicalSidebar.tsxsrc/components/clinical-dashboard/account-setup-dialog.tsxsrc/components/clinical-dashboard/auth-panel.tsxsrc/components/clinical-dashboard/cross-mode-links.tsxsrc/components/clinical-dashboard/differentials-home.tsxsrc/components/clinical-dashboard/document-admin.tsxsrc/components/clinical-dashboard/document-results.tsxsrc/components/clinical-dashboard/document-search-results.tsxsrc/components/clinical-dashboard/evidence-panels.tsxsrc/components/clinical-dashboard/favourites-command-library-page.tsxsrc/components/clinical-dashboard/favourites-hub.tsxsrc/components/clinical-dashboard/favourites-library-nav.tsxsrc/components/clinical-dashboard/image-lightbox.tsxsrc/components/clinical-dashboard/master-search-header.tsxsrc/components/clinical-dashboard/mode-action-popup.tsxsrc/components/clinical-dashboard/settings-dialog.tsxsrc/components/clinical-dashboard/signed-image.tsxsrc/components/clinical-dashboard/universal-search-command-surface.tsxsrc/components/clinical-dashboard/visual-evidence.tsxsrc/components/differentials/diagnosis-map-panel.tsxsrc/components/differentials/differential-detail-page.tsxsrc/components/differentials/differential-presentation-actions.tsxsrc/components/differentials/differential-presentation-workflow-page.tsxsrc/components/differentials/differential-stream-page.tsxsrc/components/document-viewer/non-pdf-source-preview.tsxsrc/components/document-viewer/pdf-canvas-viewer.tsxsrc/components/dsm/dsm-comparison-page.tsxsrc/components/dsm/dsm-diagnosis-page.tsxsrc/components/dsm/dsm-differential-considerations-page.tsxsrc/components/dsm/dsm-search-page.tsxsrc/components/factsheets/factsheet-detail-page.tsxsrc/components/forms/form-detail-page.tsxsrc/components/pwa-lifecycle.tsxsrc/components/services/service-detail-page.tsxsrc/components/services/services-navigator-page.tsxsrc/components/therapy-compass/bindings.tsxsrc/components/therapy-compass/controls.tssrc/components/therapy-compass/nav.tsxsrc/components/therapy-compass/screens/brief-screen.tsxsrc/components/therapy-compass/screens/compare-screen.tsxsrc/components/therapy-compass/screens/detail-screen.tsxsrc/components/therapy-compass/screens/other-screen.tsxsrc/components/therapy-compass/screens/pathways-screen.tsxsrc/components/therapy-compass/screens/recommend-screen.tsxsrc/components/therapy-compass/screens/search-screen.tsxsrc/components/therapy-compass/screens/sheets-screen.tsxsrc/components/therapy-compass/style-utils.tssrc/components/therapy-compass/styles.tsxsrc/components/therapy-compass/therapy-card.tsxsrc/components/therapy-compass/therapy-compass.csssrc/components/therapy-compass/ui.tsxsrc/components/therapy-compass/workspace.tsxsrc/components/ui-primitives.tsxsrc/lib/theme.tstests/theme.test.tstests/therapy-compass-responsive-contract.test.tstests/ui-accessibility.spec.tstests/ui-overlay-css-contract.test.tstests/ui-primitives.dom.test.tsx
💤 Files with no reviewable changes (2)
- src/components/therapy-compass/styles.tsx
- src/components/therapy-compass/style-utils.ts
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 7 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 file(s) based on 7 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@scripts/design-system-contract-utils.mjs`:
- Around line 116-134: Update the brace-balancing loop in the exemption-block
parser to recognize and skip CSS block comments, tracking comment state until
the closing */ while ignoring braces inside comments. Preserve the existing
quote and escape handling in the surrounding parser so only braces outside
strings and comments affect depth and determine the returned range.
In `@tests/therapy-compass-responsive-contract.test.ts`:
- Around line 104-107: Update the CSS checks in the responsive contract test to
first extract the complete .tc-clinician-track and .tc-clinician-track::before
rule blocks, stopping at each block’s closing brace. Assert the width and height
declarations against their respective extracted blocks so declarations from
later selectors cannot satisfy the expectations.
- Around line 97-103: Strengthen the accessibility assertions in the responsive
contract test by targeting the toggle element itself rather than checking
file-wide strings. Update the relevant test around the source assertions to use
a focused regex or rendered DOM query that verifies the same element has the
toggle trigger semantics, role="group", aria-pressed, and aria-expanded={open}
boolean state, while preserving the existing tab-role exclusions.
🪄 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 Plus
Run ID: 61bf2c01-b429-4e45-be5e-e9d88eba3f24
📒 Files selected for processing (13)
docs/design-system.mdscripts/check-design-system-contract.mjsscripts/design-system-contract-baseline.jsonscripts/design-system-contract-utils.mjssrc/app/globals.csssrc/components/therapy-compass/screens/brief-screen.tsxsrc/components/therapy-compass/screens/compare-screen.tsxsrc/components/therapy-compass/screens/sheets-screen.tsxsrc/components/therapy-compass/therapy-compass.csstests/design-system-contract-utils.test.tstests/therapy-compass-responsive-contract.test.tstests/ui-accessibility.spec.tstests/ui-overlay-css-contract.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- scripts/design-system-contract-baseline.json
- tests/ui-overlay-css-contract.test.ts
- tests/ui-accessibility.spec.ts
- src/components/therapy-compass/screens/brief-screen.tsx
- docs/design-system.md
- src/components/therapy-compass/therapy-compass.css
- scripts/check-design-system-contract.mjs
- src/components/therapy-compass/screens/compare-screen.tsx
- src/components/therapy-compass/screens/sheets-screen.tsx
- src/app/globals.css
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5973c5d72
ℹ️ 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".
Summary
check:design-system-contractbaseline so parser, focus, contrast, paper, composer, literal tap-size, glass-recipe, and semantic-control regressions fail locally and inverify:cheap.Verification
npm run verify:pr-local— passed: runtime, changed-file formatting, lint, typecheck, 312 Vitest files / 2,834 tests, production build, client-bundle secret scan, and 36 golden offline RAG fixtures across 21 suites.npm run verify:cheapafter review fixes — passed: runtime/policy/design checks, lint, typecheck, 313 Vitest files, and 2,840 tests.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run test:e2e:accessibility -- --grep "Therapy Compass preserves"(including toggle state, picker expansion, 44px switch target, focus, overflow, paper tokens, and Axe) andnpm run test:e2e -- --project=chromium --grep "mobile search focus is singular, visible, and contained at clipped edges".UI verification not run: The full multi-route
npm run verify:uisuite was not repeated; the changed Therapy surface and the CI-regression composer focus path both passed focused Chromium scenarios on isolated production builds,verify:cheappassed after all review fixes, andverify:pr-localpassed the full production build for the original remediation.npm run verify:releasebefore release or handoff confidence claimsnpm run eval:retrieval:quality(must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changednpm run eval:rag -- --limit 15+npm run eval:quality -- --rag-onlywhen answer generation, the synthesis prompt, or answer post-processing changednpm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changedNEXT_PUBLIC_SUPABASE_URL,SUPABASE_SERVICE_ROLE_KEY, andOPENAI_API_KEYin this clean worktree. No production/provider state was touched.npm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedAdditional contract proof:
npm run check:design-system-contractpassed over 500 production files (raw colors 9, literal shadows 5, legacy tap classes 21).Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
origin/main; no force push was used.Summary by CodeRabbit