ci: optimize smart test routing and browser gates - #1789
Conversation
…llout-20260809 # Conflicts: # docs/outstanding-issues.md # docs/testing.md
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughCI routing now distinguishes baseline refreshes, dependency changes, UI changes, and release-browser coverage. Playwright shards use duration profiles and critical-test exclusions. Chromium preflight validates container revisions. Local verification adds dependency dry runs and avoids duplicate checks. ChangesCI hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ChangedFiles
participant ScopeClassifier
participant LocalVerifier
participant CIWorkflow
participant PlaywrightShards
ChangedFiles->>ScopeClassifier: classify workflow, UI, and dependency changes
ChangedFiles->>LocalVerifier: detect changed manifests and scripts
ScopeClassifier->>CIWorkflow: select CI coverage
LocalVerifier->>CIWorkflow: select local verification scripts
CIWorkflow->>PlaywrightShards: run shards with critical exclusions
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refines the repository’s CI/verification routing to reduce wasted work (duplicate UI coverage, net-negative cache transfers, redundant workflow-contract runs), while adding stricter fail-closed behavior around Playwright Chromium revision drift and earlier detection of dependency/lockfile install issues.
Changes:
- Rebalances Production UI PR sharding using a per-file timing profile, and avoids rerunning
@criticaltests in the later shards once the fail-fast job has already proved them. - Removes cross-job Next.js cache transport (artifact-based) after evidence showed it increased wall time, and adjusts release-browser gating to avoid repeating production Chromium when already proven in-run.
- Adds
npm ci --dry-run --ignore-scriptstoverify:pr-localselection when dependency manifests/lockfiles change, and avoids duplicating workflow-contract suites when the full unit/coverage suite already includes them.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/verify-pr-local.test.ts | Adds contract tests for new PR-local selection behavior (npm-ci dry-run first; no duplicate workflow suite). |
| tests/test-runner-safety.test.ts | Updates CI workflow invariants around removed Next cache transport and new shard args. |
| tests/playwright-pr-shards.test.ts | Strengthens shard/parity tests (matcher alignment, balance check, exclude-critical behavior). |
| tests/playwright-browser-preflight.test.ts | Adds drift fail-closed tests for designated container overrides. |
| tests/ci-cache-safety.test.ts | Adds assertions for critical/shard disjointness, removed cache transport, and refined workflow/release routing. |
| scripts/verify-pr-local.mjs | Adds dependency-manifest detection and routing to check:npm-ci-dry-run; avoids repeating workflow suite under heavy scope. |
| scripts/playwright-pr-shards.mjs | Converts shard membership into a per-file timing profile; adds --exclude-critical wiring and estimated-duration helper. |
| scripts/playwright-browser-preflight.mjs | Adds revision-drift enforcement in preflight and routes drift failures with a specific message before build/locks. |
| scripts/ci-change-scope.mjs | Marks .github/actions/setup-ui-e2e/** as UI-scoped to ensure action changes exercise UI lanes. |
| package.json | Adds check:npm-ci-dry-run script for fast install/lockfile integrity checking. |
| docs/testing.md | Updates testing-speed guidance (exclude-critical rationale, cache transport evidence, revision-preflight behavior). |
| docs/scripts-index.md | Updates script-surface count to reflect the new npm script entry. |
| docs/process-hardening.md | Records measured shard imbalance and cache-transfer cost; documents the corrected routing approach. |
| docs/outstanding-issues.md | Moves #167/#204/#255 into resolved with outcomes matching these changes. |
| docs/branch-review-ledger.md | Appends a review/rollout reconciliation record referencing PR #1686 evidence. |
| AGENTS.md | Adds an explicit rule about fast-fail subsets preceding broader gates only when the broader gate excludes them. |
| .github/workflows/ci.yml | Implements refined CI routing: exclude-critical shards on PRs, removes cache transport, scopes release matrix and baseline refresh dispatch behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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: c81fe9c65b
ℹ️ 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. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch codex/ci-perfected-rollout-20260809 at starting commit c81fe9c; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:codex/ci-perfected-rollout-20260809, then verify that the pull request head contains the published commit before reporting success. 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. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. 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". |
Unblock PR #1789 false-DIRTY (behind-but-clean merge-tree). Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Extend ci-change-scope so preflight, revision-pin, and PR-shard helpers trip the browser gate and the post-merge release matrix.
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #9571 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
![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/verify-pr-local.test.ts (1)
65-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the dry-run in the mixed workflow/package case.
The test checks
testand the absence oftest:ci-workflows. It does not verifycheck:npm-ci-dry-run. Add that assertion and its ordering check so a regression in mixed routing cannot pass.Proposed test assertion
expect(output).toContain("- npm run test"); + expect(output).toContain("- npm run check:npm-ci-dry-run"); + expect(output.indexOf("- npm run check:npm-ci-dry-run")).toBeLessThan( + output.indexOf("- npm run lint"), + ); expect(output).not.toContain("- npm run test:ci-workflows");🤖 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/verify-pr-local.test.ts` around lines 65 - 69, Update the mixed workflow/package dry-run test in the focused test case to assert that check:npm-ci-dry-run is included and appears in the required order relative to the existing test command. Preserve the current assertion that test:ci-workflows is absent.
🤖 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 `@scripts/playwright-browser-preflight.mjs`:
- Around line 179-190: Update the revision validation in the downloadsDisabled
flow of scripts/playwright-browser-preflight.mjs to validate the headless-shell
revision actually selected by the resolver, rather than only the general
Chromium revision; preserve the existing drift result when that selected
revision is invalid. Add a regression test in
tests/playwright-browser-preflight.test.ts covering coexistence of chromium-1234
and chromium_headless_shell-1194 and asserting that revision 1194 is rejected.
---
Nitpick comments:
In `@tests/verify-pr-local.test.ts`:
- Around line 65-69: Update the mixed workflow/package dry-run test in the
focused test case to assert that check:npm-ci-dry-run is included and appears in
the required order relative to the existing test command. Preserve the current
assertion that test:ci-workflows is absent.
🪄 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: 244f7bbe-2ce8-4666-b2d2-b6b471af2991
📒 Files selected for processing (17)
.github/workflows/ci.ymlAGENTS.mddocs/branch-review-ledger.mddocs/outstanding-issues.mddocs/process-hardening.mddocs/scripts-index.mddocs/testing.mdpackage.jsonscripts/ci-change-scope.mjsscripts/playwright-browser-preflight.mjsscripts/playwright-pr-shards.mjsscripts/verify-pr-local.mjstests/ci-cache-safety.test.tstests/playwright-browser-preflight.test.tstests/playwright-pr-shards.test.tstests/test-runner-safety.test.tstests/verify-pr-local.test.ts
Babysit summary (merging)
|
Summary
@criticaltests after the fail-fast job has proved themverify:pr-localiteration-focused by selectingnpm ci --dry-run --ignore-scriptsfor dependency manifests and avoiding a focused workflow-suite rerun when the full unit suite already owns those testsRAG impact: no retrieval behaviour change — CI selection, test infrastructure, and process documentation only.
Verification
npm run verify:pr-localPartial local result on Windows: runtime, installed-lock parity, formatting, npm-ci dry-run, sitemap/docs/ledger checks, GitHub Actions/policy/scope/gate checks, Codex workflow guard, lint, and typecheck passed. The aggregate first stopped on shared-test contention, then reached the full unit suite. A standalone full unit run completed with 5,872 passed and 14 skipped, but 22 failures across 11 files: resource timeouts under heavy machine load plus the pre-existing Windows
pr-handoff-stopPOSIX-command failures. The production build produced no application error but exceeded the 15-minute outer command timeout. Hosted Linux CI remains the required arbiter for those two boundaries.Focused CI and runner contracts: 5 files passed, 87 tests passed, 11 intentionally skipped
npm run check:rag:fixtures— 36 golden cases across 23 suitesnpm run check:outstanding-issues,npm run check:branch-review-ledger, documentation checks, verification-plan self-test, action pins, CI scope, and gate manifestnpm run verify:ui— not run locally because no product UI behavior changed; this workflow diff deliberately triggers hosted Production UInpm run verify:release— not run; this is a draft PR, not release evidenceRisk and rollout
@criticalcoverage, full fallback paths, exact production-spec parity, duration balance, and fail-closed unknown scope.Clinical Governance Preflight
Not applicable: no ingestion, answer generation, retrieval/ranking, source rendering, document-access, privacy, production-environment, or clinical-output behavior changes.
Notes
Summary by CodeRabbit
Bug Fixes
Testing
Documentation