perf: keep document administration UI off the initial dashboard bundle - #1562
Conversation
|
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 Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe dashboard separates administration contracts and library health rendering into dedicated modules. Phone overlay reserve measurement uses fallback heights and preserves valid CSS state during transient zero-height measurements. Tests cover import boundaries, measurement behavior, and viewport settling. ChangesClinical dashboard boundary extraction
Phone overlay reserve stabilization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
🔒 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. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
🔒 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. |
🔒 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. |
🔒 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. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #7731 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
@cursoragent fix failing CI and any other issues |
|
Fixed the Production UI failure and pushed to PR #1562. Cause: Fix: In Verified: focused unit/contract tests   |
Production UI failed when Services results kept chrome hidden after a viewport shrink but the result anchor jumped +131px with scrollTop unchanged. A zero-height stack measurement used `??`, so offsetHeight 0 published 0px over the CSS seed and the next successful measure restored the full stack height. Refuse zero phone measurements and fall through with `||`; tighten the Playwright poll to require a stable positive reserve. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
🔒 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. |
1 similar comment
🔒 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/client-performance-boundaries.test.ts (1)
50-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the negative import assertion.
Line 50 matches one exact specifier string. It does not catch a relative specifier such as
./clinical-dashboard/DocumentManagerPanel, so a future eager import can reappear and this test still passes. Match the staticimport ... fromform with a regex, and keep lazyimport()allowed.♻️ Proposed tighter assertion
- expect(dashboard).not.toContain('from "`@/components/clinical-dashboard/DocumentManagerPanel`"'); + expect(dashboard).not.toMatch(/^import\s[^;]*?\sfrom\s+["'][^"']*clinical-dashboard\/DocumentManagerPanel["']/m); expect(dashboard).toContain('from "`@/components/clinical-dashboard/document-manager-contracts`"'); expect(dashboard).toContain('from "`@/components/clinical-dashboard/library-health-strip`"');🤖 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/client-performance-boundaries.test.ts` around lines 50 - 52, Update the negative assertion in the dashboard import test to use a regex matching the static `import ... from` form for the DocumentManagerPanel module, including relative and aliased specifiers, while continuing to allow dynamic import() usage. Keep the existing positive assertions unchanged.
🤖 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 `@src/components/clinical-dashboard/library-health-strip.tsx`:
- Around line 78-89: Update the button class list in the health strip button
rendering to include min-h-12, ensuring every production tap target has a 48px
minimum height while preserving the existing sizing and styling classes.
---
Nitpick comments:
In `@tests/client-performance-boundaries.test.ts`:
- Around line 50-52: Update the negative assertion in the dashboard import test
to use a regex matching the static `import ... from` form for the
DocumentManagerPanel module, including relative and aliased specifiers, while
continuing to allow dynamic import() usage. Keep the existing positive
assertions unchanged.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 900da4f8-7bbe-4183-9b3c-bf5c1b25eacd
📒 Files selected for processing (10)
docs/branch-review-ledger.mdsrc/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/DocumentManagerPanel.tsxsrc/components/clinical-dashboard/document-manager-contracts.tssrc/components/clinical-dashboard/library-health-strip.tsxsrc/components/clinical-dashboard/use-phone-overlay-chrome-reserve.tstests/client-performance-boundaries.test.tstests/header-scroll-hide-contract.test.tstests/phone-overlay-chrome-reserve.dom.test.tstests/ui-phone-scroll-page-owned.spec.ts
🔒 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. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
🔒 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. |
|
@cursoragent Unblock the current open PR. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker. If the PR is already merged or closed: confirm outcome, post one summary, and stop. Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix. Snapshot the GitHub head: tip, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot. Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop. Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push. Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip. One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me. Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress. |
|
PR #1562 unblocked at tip GitHub had reported Now: |
…zations Unblock PR #1562: GitHub reported DIRTY/CONFLICTING while merge-tree was clean (behind-by-4 staleness). Sync main so refs/pull/1562/merge can build again. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Ledger the behind-but-clean sync that cleared GitHub DIRTY mergeability on the document-admin lazy-load PR tip. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
🔒 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. |
Unblock summary — PR #1562Tip: Sync / merge-tree
What blocked / what I did
No product code changes in this unblock push beyond the main sync + ledger row. CodeRabbit’s Required CI (current tip)
Residual risks
|
2 participants
Motivation
Description
src/components/clinical-dashboard/document-manager-contracts.tsto avoid importing heavy admin code from the dashboard.src/components/clinical-dashboard/library-health-strip.tsxwhich preserves loading skeleton and content-visibility optimizations.src/components/ClinicalDashboard.tsx.DocumentManagerPanelwhile re-exporting lightweight types from the contract file so the full admin surfaces remain behind existing dynamic imports: changes insrc/components/clinical-dashboard/DocumentManagerPanel.tsx.tests/client-performance-boundaries.test.ts(asserts dashboard no longer statically imports the administration module).Testing
npm run typecheckand it passed with no errors.npm run lint -- --no-cacheand it passed (no blocking warnings).npm run testand the results were: 463 test files, 4,811 tests passed and 1 skipped.npm run buildand the Next.js production build completed successfully and the client-bundle secret scan passed.npm run check:bundle-budget -- --json: current gzip total 1,434,092 bytes across 293 chunks, verdictwithin tolerance(≈+9.53% vs baseline; enforcement enabled).npm run verify:pr-local -- --dry-run --files ...(dry-run) to produce the PR-local verification plan; plan was generated successfully.Files changed/added (key):
src/components/clinical-dashboard/document-manager-contracts.ts(types & fallback checks).src/components/clinical-dashboard/library-health-strip.tsx(small client component).src/components/ClinicalDashboard.tsx(now imports contracts + health strip instead of admin module).src/components/clinical-dashboard/DocumentManagerPanel.tsx(kept exports, removed types to contracts).tests/client-performance-boundaries.test.ts(assert import boundary).All automated checks listed above completed successfully and the bundle-budget check is within the enforced budget.
Codex Task
Summary by CodeRabbit
New Features
Bug Fixes
Tests