fix: Grok-safe quick wins (tokens, a11y, perf, gates) - #1651
Conversation
- Drop unused --med-accent-soft (#157) - Document Chip compact=11px / standard=12px (#220) - Reveal phone chrome on source change at top (#176) - Passive wheel listener for modifier-gated PDF zoom (#214) - Point npm run typecheck at source-only tsconfig (#210) - Fail outstanding-issues when queue cites non-open IDs (#201) Also confirms MatchExplanationChips, Favourites empty-state, DocumentFrame role=alert, and refetching dimming claim are already resolved on main (#223/#225/#219/#246).
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:28 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 (1)
📝 WalkthroughWalkthroughChangesIssue queue lifecycle
Interaction behavior
Documentation and styling cleanup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant resolveIssue
participant pruneResolvedIdFromQueue
participant outstandingIssuesMarkdown
participant checkIssues
resolveIssue->>pruneResolvedIdFromQueue: remove resolved issue ID
pruneResolvedIdFromQueue->>outstandingIssuesMarkdown: update queue and renumber rows
resolveIssue->>outstandingIssuesMarkdown: archive resolved issue
checkIssues->>outstandingIssuesMarkdown: parse queue citations
checkIssues-->>resolveIssue: validate current open issue IDs
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
- Keep viewer wheel listener non-passive while wheelZoom is on and preventDefault on Ctrl/⌘+wheel / trackpad pinch so PDF zoom does not also zoom the browser page (Sentry/Devin). - Prune recommended-queue ID citations in resolveIssue so issues:done stays writable under the #201 gate. - Drop restated Chip pixel values from TOKENS.md; amend #214 archive note. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Devin: pruneResolvedIdFromQueue used a pre-splice openStart for renumbering, so enough solo-queue deletions could rewrite Open items headers/ids. Recompute the section limit after edits and cover it in the writer self-test. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
PR #1651 heavy review-and-fix summaryTip: Review
Sync / merge-tree
Required CI (this tip)
Local offline gates (decisive lines)
Residual risks
Merge left to you. |
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: 3
🧹 Nitpick comments (1)
tests/use-viewer-gestures.dom.test.tsx (1)
68-84: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the remaining wheel-event branches.
This test uses only
ctrlKey: trueandcancelable: true. Add ametaKey: truecase and a non-cancelable modifier-wheel case. Assertevent.defaultPreventedfor the cancelable case and verify the intended behavior when cancellation is unavailable.Suggested assertion
expect(onZoomBy).toHaveBeenCalledTimes(1); expect(preventDefault).toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(true);🤖 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/use-viewer-gestures.dom.test.tsx` around lines 68 - 84, Extend the wheel gesture tests around GestureHarness to cover modifier-wheel events with metaKey: true and with cancelable: false, in addition to the existing ctrlKey case. For the cancelable event, assert event.defaultPrevented is true; for the non-cancelable event, verify the intended zoom behavior and that cancellation cannot be applied without requiring preventDefault to succeed.
🤖 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 `@docs/branch-review-ledger.md`:
- Around line 675-685: Restore both distinct PR `#1641` ledger records removed by
merge commit b2ab472986ed5d1a1d9df6795a66ee687ce6df33, keeping the existing PR
`#1651` records intact. Apply ledger:dedupe only to exact duplicate rows and do
not remove records that differ in branch, commit, or details.
In `@docs/design-system/TOKENS.md`:
- Line 51: Update the “Chip sizes” row in TOKENS.md so the `--text-2xs` mapping
for `Chip compact` is explicitly marked as a legacy compatibility token, while
retaining the v2 `--text-xs` default mapping and existing guidance.
In `@docs/outstanding-issues.md`:
- Around line 159-160: Update queue item `#250` in the outstanding-issues ledger
to remove the resolved `#201` reference from its action text and replace it with
the remaining open work, including matching updates to the related open-item
wording. Keep `#202` actionable and ensure no stale `#201` reference remains in the
item.
---
Nitpick comments:
In `@tests/use-viewer-gestures.dom.test.tsx`:
- Around line 68-84: Extend the wheel gesture tests around GestureHarness to
cover modifier-wheel events with metaKey: true and with cancelable: false, in
addition to the existing ctrlKey case. For the cancelable event, assert
event.defaultPrevented is true; for the non-cancelable event, verify the
intended zoom behavior and that cancellation cannot be applied without requiring
preventDefault to succeed.
🪄 Autofix
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: 1f403c2d-3362-4a74-bd94-a0d74c7d1413
📒 Files selected for processing (12)
docs/branch-review-ledger.mddocs/design-system/TOKENS.mddocs/outstanding-issues.mdpackage.jsonscripts/check-outstanding-issues.mjsscripts/outstanding-issues.mjssrc/app/globals.csssrc/components/clinical-dashboard/medication-record-page.tsxsrc/components/clinical-dashboard/use-hide-on-scroll.tssrc/components/document-viewer/use-viewer-gestures.tstests/use-hide-on-scroll.test.tstests/use-viewer-gestures.dom.test.tsx
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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>
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Summary
Isolated worktree off current
origin/main(cursor/grok-quick-wins-a2c0) so this does not touch other active hot branches.--med-accent-softfrom medication accent CSS varscompact=11px (text-2xs) vsstandard=12px (text-xs) inTOKENS.md+globals.csssourceChangedso phone chrome cannot strand hidden at offset 0 after a scroll-container handoffnpm run typecheckusestsconfig.typecheck.json(source-only), matching pre-pushcheck:outstanding-issuesfails when the recommended queueID(s)column cites a non-open idVerification
npm run test -- tests/use-hide-on-scroll.test.ts tests/use-viewer-gestures.dom.test.tsx tests/document-frame.dom.test.tsx→ 38 passednpm run typecheck→ exit 0 (now source-only config)npm run check:outstanding-issues→ exit 0 (112 open, 141 archived)npm run verify:pr-local -- --dry-run --files …plan inspectednpm run verify:pr-local(includes build + full unit suite) — recommend CI / local handoffnpm run verify:phone-chromefor#176— focused unit coverage added; phone-chrome gate still recommended before mergeRisk and rollout
Notes
/home/ubuntu/wt-grok-quick-wins(does not switch/workspace)behind 0vsorigin/mainat start (c839b97d)Summary by CodeRabbit
Bug Fixes
Documentation
Chores