Fix typography audit issues - #1294
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 19 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 (2)
📝 WalkthroughWalkthroughThe PR scopes diagnosis-detail UI assertions, adds heading-hierarchy and low-RAM guard contract tests, changes low-RAM CI behavior to warn and continue while remaining fail-closed locally, and records review and CI status updates. ChangesUI validation and heading contracts
Low-RAM build guard
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #6041 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
@cursoragent please resolve all CI failing tests after understanding the issue. Please also fix any comments and resolve them and also ensure that you fix with smallest required fix with best coding practice and u derstabding the fix rather than simple patch. Please fix any merge conflicts and ensure any issues are found. Please resolve any issues you find. Please also run bugbot. Please mark comments resolved also once you resolve them. |
|
CI is fixed on PR #1294. Root cause: Production UI failed on Fix: Scoped the locator to Comments / Bugbot: 0 unresolved review threads (nothing to resolve). Bugbot found no Verification:
  |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Production UI failed when getByTestId("differential-detail-page")
matched both the live shell page and a hidden streaming clone.
Scope to mobile-composer-reserve-pad, matching presentation and
service detail tests.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Static PR format:check failed on the ui-tools locator change. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
CI babysit update ( Root causeProduction UI failed on Fix
Comments / Bugbot
Verification
|
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>
|
Closeout: Hosted Production UI + PR required passed on product tip |
|
@cursoragent please resolve all CI failing tests after understanding the issue. Please also fix any comments and resolve them and also ensure that you fix with smallest required fix with best coding practice and u derstabding the fix rather than simple patch. Please fix any merge conflicts and ensure any issues are found. Please resolve any issues you find. Please also run bugbot. Please mark comments resolved also once you resolve them. |
|
CI and review comments on PR #1294 are resolved. What was wrong
What we did
Status: Product tip |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Babysit re-request (
|
|
@codex review PR code review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b941bb23e
ℹ️ 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. Hooray! 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". |
Private GitHub-hosted ubuntu runners report ~7–8 GiB, so the local 10 GiB Docker Desktop rail was fail-closing Build. Keep the hard fail for local builds; warn and continue under CI/GITHUB_ACTIONS. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Build failure fixed ( Root causeAfter syncing Fix
Other status
|
Codex P2: promoting sheet/modal titles to h2 under Section h2 flattened the mockup outline (same hierarchy regression recorded on PR #1200). Restore h3 child titles and pin the contract in Vitest. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
![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
🤖 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/guard-next-build-contract.test.ts`:
- Around line 8-19: Replace the source-text assertions in the guard contract
tests with execution-based coverage of the low-memory guard. Exercise both local
and CI/GITHUB_ACTIONS conditions through a pure decision helper or controlled
child process, asserting the local path exits with status 1 and emits the
warning/error while the hosted CI path continues without failing and emits its
continuation message.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: b64dc21b-ec6f-4dab-abc6-1c01d349db36
📒 Files selected for processing (3)
docs/branch-review-ledger.mdscripts/guard-next-build.mjstests/guard-next-build-contract.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/branch-review-ledger.md
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/answer-evidence-popups-heading-contract.test.ts`:
- Around line 12-25: Update the heading assertions in the answer-evidence popup
contract test to scope each match to its own component body, preventing headings
from later components from satisfying the regex. Ensure each assertion verifies
the exact expected heading level and rejects an additional incorrect heading,
preferably through rendered DOM or AST assertions; cover Section,
MobileSheetFrame, DesktopEvidenceModal, TableDialog, and WeakEvidencePopup.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: ac99b947-102d-40f9-a557-9a7447b340a8
📒 Files selected for processing (2)
docs/branch-review-ledger.mdtests/answer-evidence-popups-heading-contract.test.ts
Container app-image failed for the same <10 GiB host floor: buildx does not set CI/GITHUB_ACTIONS. Detect DOCKER_BUILD=1 and /.dockerenv, set DOCKER_BUILD in the app Dockerfile build stage, and keep local fail-closed behavior. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Follow-up: Container |
Resolve Dockerfile and guard-next-build conflicts by taking main's #1307 ALLOW_LOW_RAM_BUILD design (exported evaluator + CI build-arg). Drop the superseded source-only guard contract test. Tighten the answer-evidence heading contract to component-scoped bodies. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Status update (
|
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
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>
|
Done ( CI
Fixes this pass
Bugbot / threads
Unique delta vs main: |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
Production UI failed twice on this PR with the same signature but a different test each time: a strict-mode violation where a document-wide getByTestId resolved to two identical elements, one of them hidden. First `differentials-search-results`, then `global-search-input`. That is the Next streaming `S:` clone of the page root, which CI load makes observable and which #1294 already documented and fixed the same way on the differentials detail page. It is not caused by this branch: the test that failed first passed untouched on the next run. Scope the affected assertions to the visible element, which is the idiom already used for this exact testid in ui-overlap.spec.ts. Applied to the class rather than the single instance, so the flake does not simply move to whichever spec loses the race next. Deliberately not changed: the `toHaveCount` assertions on this testid. Those count elements on purpose as single-owner guards, and visible-scoping them would change what they assert. Verified locally: ui-chrome-scroll + ui-phone-scroll 65 passed; ui-smoke 91 passed with only the pre-existing PDF-canvas failure, which is a local browser-build artifact and passes in CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
2 participants
Fixes for issues identified in the typography layout audit report. Most fixes are already in main; this just fixes the h3->h2 hierarchy in answer-evidence-popups.
Summary by CodeRabbit
Bug Fixes
Tests