test(clinical-dashboard): assert upload-layout hook wires useSyncExternalStore - #1047
Conversation
…rnalStore 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
This pull request has been ignored for the connected project Preview Branches by Supabase. |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe regression test now verifies that the upload desktop layout hook passes the subscription function, snapshot getter, and ChangesUpload layout regression coverage
Estimated code review effort: 2 (Simple) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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:ddbb7192de
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/audit-navigation-auth-regressions.test.ts`:
- Around line 185-186: Update the useSyncExternalStore assertion for
uploadDesktopHookSource to match the complete returned call: require the return
keyword, all expected arguments, and the closing parenthesis so comments,
strings, prefixes, or unused helpers cannot satisfy the regression check.
🪄 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: af4b8efe-bcf9-41f3-b99c-04cbca778ddb
📒 Files selected for processing (1)
tests/audit-navigation-auth-regressions.test.ts
Uh oh!
There was an error while loading. Please reload this page.
…rted 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
Summary
Test-only follow-up to #1042 (merged). The
audit-navigation-auth-regressionsguard that pins theupload view's viewport-driven
region/tabpanelrole asserted only that the extracted hook filecontains the string
subscribeToUploadDesktopLayout— which passes even if the helper isdeclared-but-unused or the subscription is disconnected. CodeRabbit flagged exactly this on #1042,
but that PR merged with the weaker assertion, so the hollow guard is now on
main.This strengthens the assertion to match the actual wiring —
useSyncExternalStore(subscribeToUploadDesktopLayout, getUploadDesktopLayoutSnapshot, () => false)—so the guard now fails on a disconnected subscription, a dropped SSR server-snapshot, or a
present-but-unused helper. No production code changes:
use-upload-desktop-layout.tsis untouched;only the test is hardened.
Verification
npx vitest run tests/audit-navigation-auth-regressions.test.ts— 8/8 passingnpx eslint tests/audit-navigation-auth-regressions.test.ts— cleanregression variants (disconnected subscription, dropped
() => falsesnapshot, present-but-unusedhelper), proving the guard actually guards rather than just matching a symbol name.
Risk and rollout
Clinical Governance Preflight
Not applicable — this is a test-only change to a navigation/auth regression guard. It touches no
ingestion, answer generation, retrieval/ranking, source governance, document access, privacy,
schema, or production behaviour (
scripts/pr-policy.mjsclassifies the path as non-clinical).Notes
🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
useSyncExternalStorewith the expected subscribe and snapshot providers.