fix: Implement audit design fixes and fallback improvements - #1351
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 6 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds route-specific not-found and error pages, clipboard diagnostics for error states, throttled image retries, query suggestions for empty search results, updated empty-state accessibility, and reduced-motion-aware UI feedback. ChangesError resilience and interface feedback
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DocumentPage
participant PublicApi
participant NextNotFound
participant DocumentNotFound
DocumentPage->>PublicApi: load document details
PublicApi-->>DocumentPage: return 404 PublicApiError
DocumentPage->>NextNotFound: call notFound()
NextNotFound->>DocumentNotFound: render missing-document state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #6430 (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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47f69ffeba
ℹ️ 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. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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". |
Fix EmptyState prop rename (action → actions) on not-found routes and re-home the DocumentViewer empty-tables EmptyState onto DocumentViewerRail after the #1311 extraction so the branch merges cleanly with main. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/components/clinical-dashboard/image-lightbox.tsx (1)
40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for the retry throttle.
Use fake timers to verify that a second click does not call
retry()and that the control becomes available after two seconds. Cover the analogousSignedImagepath as well.Based on the PR objective to add targeted tests for behavior changes.
Also applies to: 51-57
🤖 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 `@src/components/clinical-dashboard/image-lightbox.tsx` at line 40, Add regression tests around the retry throttle in the image lightbox, using fake timers to verify a second click does not invoke retry() and that the retry control becomes available after two seconds. Cover both the primary image path and the analogous SignedImage path, targeting the retryDisabled state and its associated click behavior.src/components/clinical-dashboard/search-utils.ts (1)
319-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused tests for suggestion ordering and edge cases.
Cover blank input, long queries, quoted phrases, contractions, boolean operators with extra whitespace/punctuation, and the three-item limit.
Based on the PR objective to add targeted tests for behavior changes.
🤖 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 `@src/components/clinical-dashboard/search-utils.ts` around lines 319 - 345, Add focused tests for generateQuerySuggestions covering blank and whitespace-only input, long queries, quoted phrases, contractions, boolean operators with extra whitespace or punctuation, and ensuring results never exceed three items. Assert exact suggestion ordering and expected edge-case behavior, including that contractions are handled distinctly from quoted phrases.
🤖 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/app/`(search-app)/differentials/[id]/not-found.tsx:
- Around line 15-17: Update the Link in the not-found page to derive its href
from the repository’s existing app-mode, tools-catalog, or universal-search
route source instead of the hardcoded "/differentials" path. Preserve the
current destination and Link styling while reusing the established route symbol.
In `@src/app/`(search-app)/documents/[id]/not-found.tsx:
- Around line 15-17: Update the Link destination in the not-found page to use
the repository’s existing shared route source for the document library instead
of the hardcoded "/documents" path. Preserve the current link styling and label
while referencing the established route symbol used for internal navigation.
In `@src/app/global-error.tsx`:
- Around line 22-34: Update handleCopyDiagnostics to guard against an
unavailable navigator.clipboard and catch writeText failures, setting copied
only after a successful write. Track the reset timeout, clear any existing
timeout before scheduling a new one, and preserve the two-second reset behavior
for repeated clicks.
In `@src/components/clinical-dashboard/search-utils.ts`:
- Around line 331-337: Update the quote and boolean detection in the
search-suggestion logic around trimmed and upperQuery to use token-aware
parsing: identify paired quote tokens rather than apostrophes in contractions,
recognize AND/OR/NOT at token boundaries despite repeated whitespace or
punctuation, and ignore operators inside quoted text. Preserve the existing
suggestion messages and behavior for valid detected cases.
In `@src/components/document-viewer/document-rail-panels.tsx`:
- Around line 243-247: Update the EmptyState title in the document rail panels
to retain the “clinically useful” qualifier, matching the existing body text and
accurately describing the state when auditImages still contains retained tables
or diagrams.
In `@src/components/route-error-boundary.tsx`:
- Around line 49-61: Update handleCopyDiagnostics to guard clipboard
availability and handle writeText rejections without unhandled promises,
providing the existing user-facing fallback behavior where appropriate. Store
the copied-state reset timeout identifier, clear any previously scheduled timer
before creating a new one, and retain the two-second reset after successful
copying.
---
Nitpick comments:
In `@src/components/clinical-dashboard/image-lightbox.tsx`:
- Line 40: Add regression tests around the retry throttle in the image lightbox,
using fake timers to verify a second click does not invoke retry() and that the
retry control becomes available after two seconds. Cover both the primary image
path and the analogous SignedImage path, targeting the retryDisabled state and
its associated click behavior.
In `@src/components/clinical-dashboard/search-utils.ts`:
- Around line 319-345: Add focused tests for generateQuerySuggestions covering
blank and whitespace-only input, long queries, quoted phrases, contractions,
boolean operators with extra whitespace or punctuation, and ensuring results
never exceed three items. Assert exact suggestion ordering and expected
edge-case behavior, including that contractions are handled distinctly from
quoted phrases.
🪄 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: 848da7e2-ef63-4cd7-99f5-32a2fcd2eb71
📒 Files selected for processing (15)
src/app/(search-app)/differentials/[id]/not-found.tsxsrc/app/(search-app)/documents/[id]/not-found.tsxsrc/app/(search-app)/dsm/error.tsxsrc/app/(search-app)/formulation/error.tsxsrc/app/(search-app)/medications/error.tsxsrc/app/(search-app)/therapy-compass/error.tsxsrc/app/global-error.tsxsrc/components/clinical-dashboard/answer-status.tsxsrc/components/clinical-dashboard/image-lightbox.tsxsrc/components/clinical-dashboard/search-utils.tssrc/components/clinical-dashboard/signed-image.tsxsrc/components/document-viewer/document-rail-panels.tsxsrc/components/mode-home-page-skeleton.tsxsrc/components/route-error-boundary.tsxsrc/components/ui-primitives.tsx
Safety and config checks failed on a transient npm ci network abort; Build/Unit/Static already passed on this tip. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Move differentials not-found to the real segment ancestor, call notFound() for document 404s, wire zero-result query suggestions, harden clipboard copy diagnostics, and use shared mode home hrefs for recovery links. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/ClinicalDashboard.tsx (1)
3564-3564: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a focused regression test for zero-result suggestions.
Cover both boolean queries and ordinary natural-language queries so the boolean-specific guidance remains protected.
🤖 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 `@src/components/ClinicalDashboard.tsx` at line 3564, Add a focused regression test for the zero-result response path in ClinicalDashboard, covering both boolean queries and ordinary natural-language queries. Assert that the generated response includes the appropriate query suggestions and preserves boolean-specific guidance for boolean inputs.
🤖 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 `@src/components/ClinicalDashboard.tsx`:
- Line 3564: Add a focused regression test for the zero-result response path in
ClinicalDashboard, covering both boolean queries and ordinary natural-language
queries. Assert that the generated response includes the appropriate query
suggestions and preserves boolean-specific guidance for boolean inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d70ff3b-84c1-4d4c-853e-2875d26a2aa5
📒 Files selected for processing (8)
src/app/(search-app)/differentials/not-found.tsxsrc/app/(search-app)/documents/[id]/not-found.tsxsrc/app/(search-app)/documents/[id]/page.tsxsrc/app/global-error.tsxsrc/components/ClinicalDashboard.tsxsrc/components/clinical-dashboard/search-utils.tssrc/components/document-viewer/document-rail-panels.tsxsrc/components/route-error-boundary.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/app/(search-app)/documents/[id]/not-found.tsx
- src/components/route-error-boundary.tsx
- src/app/global-error.tsx
- src/components/clinical-dashboard/search-utils.ts
- src/components/document-viewer/document-rail-panels.tsx
Missing documents now call notFound() instead of DocumentViewer’s “Source unavailable” panel; update the chromium smoke assertion. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
EmptyState titles render as paragraphs; smoke DomIntegrity requires exactly one h1, so document/differential not-found fallbacks now use an explicit heading while keeping polite status announcement. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Replace EmptyState paragraph titles so DomIntegrity’s single-h1 contract passes on document and differential missing-resource fallbacks. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Resolves missing error boundaries and accessibility issues identified in the UI audit.
Summary by CodeRabbit
New Features
Bug Fixes
Accessibility & UX