refactor(components): extract upload-desktop-layout store into a hook (maturity X3)#1042
Conversation
… (maturity X3) Continue the ClinicalDashboard decomposition with a small, self-contained pure move: the upload view's desktop-layout media-query store (uploadDesktopMediaQuery + subscribeToUploadDesktopLayout + getUploadDesktopLayoutSnapshot, driven by useSyncExternalStore) becomes a named hook, src/components/clinical-dashboard/use-upload-desktop-layout.ts. The container now calls `useUploadDesktopLayout()` instead of inlining the store plumbing; behaviour is identical, including the `() => false` getServerSnapshot that avoids an SSR hydration mismatch. ClinicalDashboard.tsx 4,160 -> 4,140 lines; the maintainability budget is ratcheted to 4,140. The audit-navigation-auth-regressions guard that pinned the media-query subscription now follows it: it asserts the container wires `useUploadDesktopLayout()` and the hook module carries `subscribeToUploadDesktopLayout` (so the viewport-driven region/tabpanel a11y role stays guarded). Off the RAG-ranking lane; no behaviour change. Verified: typecheck, lint, knip, maintainability budgets, and the full unit suite (3,102 passing; only the pre-existing container-only pdf-extraction-budget flake fails). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jc1ZYHFjXjn6mE6U6riVU
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthrough
ChangesUpload layout extraction
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/audit-navigation-auth-regressions.test.ts (1)
177-182: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the hook wiring, not just the helper name.
Lines 180-182 pass even if
subscribeToUploadDesktopLayoutremains unused. Match the actualuseSyncExternalStore(...)call, including the server snapshot, so this guard detects a disconnected subscription or SSR regression.Suggested assertion
- expect(source("src/components/clinical-dashboard/use-upload-desktop-layout.ts")).toContain( - "subscribeToUploadDesktopLayout", - ); + const uploadDesktopHookSource = source("src/components/clinical-dashboard/use-upload-desktop-layout.ts"); + expect(uploadDesktopHookSource).toMatch( + /useSyncExternalStore\(\s*subscribeToUploadDesktopLayout,\s*getUploadDesktopLayoutSnapshot,\s*\(\)\s*=>\s*false/, + );🤖 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 `@tests/audit-navigation-auth-regressions.test.ts` around lines 177 - 182, Strengthen the assertions around useUploadDesktopLayout() and subscribeToUploadDesktopLayout so they verify the hook actually wires the subscription through useSyncExternalStore(...), including its server snapshot argument. Ensure the test fails when the helper is merely declared or unused and preserves coverage for disconnected subscriptions and SSR behavior.
🤖 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.
Nitpick comments:
In `@tests/audit-navigation-auth-regressions.test.ts`:
- Around line 177-182: Strengthen the assertions around useUploadDesktopLayout()
and subscribeToUploadDesktopLayout so they verify the hook actually wires the
subscription through useSyncExternalStore(...), including its server snapshot
argument. Ensure the test fails when the helper is merely declared or unused and
preserves coverage for disconnected subscriptions and SSR behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b738c832-a9bd-4922-b45f-3c25bd3ab308
📒 Files selected for processing (4)
scripts/check-maintainability-budgets.mjssrc/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/use-upload-desktop-layout.tstests/audit-navigation-auth-regressions.test.ts
…rnalStore (#1047) * test(clinical-dashboard): assert upload-layout hook wires useSyncExternalStore The audit-navigation guard asserted the hook file merely *contains* `subscribeToUploadDesktopLayout`, which passes even if the helper is declared-but-unused or the subscription is disconnected. Match the actual useSyncExternalStore(subscribe, snapshot, () => false) wiring instead, so the guard fails on a disconnected subscription, a dropped SSR server-snapshot, or a present-but-unused helper. Follow-up to CodeRabbit's nitpick on #1042, which merged with the weaker assertion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jc1ZYHFjXjn6mE6U6riVU * test(clinical-dashboard): anchor upload-layout hook guard to the exported return Scope the useSyncExternalStore wiring assertion to the exported useUploadDesktopLayout body and require the returned call to close right after the () => false server snapshot. This closes the gaps Codex and CodeRabbit both flagged: a stale/dead call elsewhere in the file, a present-but-unused helper, a missing return, or a mutated snapshot like `() => false || getSnapshot()` can no longer satisfy the guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jc1ZYHFjXjn6mE6U6riVU --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
Maturity X3, continued — another small, self-contained
ClinicalDashboard.tsxextraction.You asked me to do the safe next step.
rag.ts(the largest remaining monolith) is not saferight now — the RAG/eval lane is actively merging (a
feat(rag)PR landed minutes before this),and touching the repo's most-protected file mid-series would conflict and re-enter the merge race.
So this is a clean, low-coupling extraction on a non-RAG UI surface instead.
The upload view's desktop-layout media-query store —
uploadDesktopMediaQuery+subscribeToUploadDesktopLayout+getUploadDesktopLayoutSnapshot, consumed viauseSyncExternalStore— becomes a named hook:src/components/clinical-dashboard/use-upload-desktop-layout.ts→useUploadDesktopLayout()The container now calls
useUploadDesktopLayout()instead of inlining the store plumbing.Behaviour is identical, including the
() => falsegetServerSnapshotthat prevents an SSRhydration mismatch (preserved exactly).
ClinicalDashboard.tsx4,160 → 4,140 lines; themaintainability budget is ratcheted to 4,140. Off the RAG-ranking lane — no
src/lib/rag/**,retrieval, ranking, or eval files touched.
Verification
npm run typecheck— 0 errorsnpm run lint— 0 warnings (the now-unuseduseSyncExternalStoreimport is removed)npm run test— 3,102 passing. The only failure is the pre-existing container-onlypdf-extraction-budgetflake (Python OCR stack absent in this VM).npm run check:maintainability-budgets— ratcheted to 4,140, passesnpm run check:knip— no dead code / orphaned exports (the hook export is consumed)audit-navigation-auth-regressionstest.The
audit-navigation-auth-regressionsguard that pinned the media-query subscription nowfollows the code: it asserts the container wires
useUploadDesktopLayout()and the hookmodule carries
subscribeToUploadDesktopLayout, so the viewport-drivenregion/tabpanela11yrole stays guarded.
Risk and rollout
(the
getServerSnapshotis preserved, so SSR/hydration is unchanged).Clinical Governance Preflight
This is a pure presentational refactor (a media-query store extracted verbatim into a hook). It
changes no clinical, privacy, source-governance, or document-access behaviour. The checklist is
completed because the new file path (
.../use-upload-desktop-layout.ts) matches the policyclassifier's
uploadpath pattern, which requires the full preflight even though nodocument-upload or clinical surface is touched.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
rag.tsremains the largest open decompositiontarget, deferred until the RAG lane is confirmed quiet.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes