ci: split CI into parallel fast-checks and turbo-check jobs#2672
ci: split CI into parallel fast-checks and turbo-check jobs#2672amikofalvy wants to merge 1 commit intomainfrom
Conversation
|
Error agent completed without reporting progress |
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This PR cleanly splits the monolithic CI workflow into two parallel jobs (fast-checks on ubuntu-latest and turbo-check on ubuntu-32gb), with a gate job preserving the ci status check name. The implementation follows GitHub Actions best practices with properly pinned action SHAs, appropriate permissions scoping, and sensible timeout values.
💭 Consider (2) 💭
Inline Comments:
- 💭 Consider:
.github/workflows/ci.yml:236Gate job condition could be strengthened for explicitness - 💭 Consider:
.github/workflows/ci.yml:67check:route-handler-patternsis a new CI addition (good alignment with localpnpm check)
✅ APPROVE
Summary: Well-structured CI optimization. The split correctly moves lightweight checks (format, env-descriptions, route-handler-patterns, knip) to a cheap ubuntu-latest runner while keeping build/lint/typecheck/test on the 32GB runner. The gate job pattern is correct and preserves branch protection compatibility. The two "Consider" items are minor robustness suggestions — the PR is ready to merge as-is. 🚀
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
8 | 0 | 0 | 0 | 2 | 0 | 6 |
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 8 | 0 | 0 | 0 | 2 | 0 | 6 |
Discarded (6)
| Location | Issue | Reason Discarded |
|---|---|---|
.github/workflows/ci.yml:21-71 |
fast-checks inherits workflow-level permissions | Not an issue — permissions are appropriate for read-only operations |
.github/workflows/ci.yml:77-78 |
turbo-check escalates to contents:write | Not an issue — correctly justified for OpenAPI snapshot commits |
.github/workflows/ci.yml:27 |
All actions pinned to SHA | Positive observation, not an issue |
.github/workflows/ci.yml:238-244 |
Gate job preserves 'ci' name | Positive observation, not an issue |
.github/workflows/ci.yml:23-24 |
Appropriate runner and timeout | Positive observation, not an issue |
.github/workflows/ci.yml:54 |
Inconsistent restore-keys format (cosmetic) | Too nitpicky for YAML style, MEDIUM confidence |
| @@ -192,6 +235,14 @@ jobs: | |||
| echo "Run \`pnpm check\` locally to reproduce the CI checks." >> $GITHUB_STEP_SUMMARY | |||
| fi | |||
There was a problem hiding this comment.
💭 Consider: Gate job condition could be strengthened
Issue: The current condition always() && !contains(needs.*.result, 'failure') && !contains(needs.*.result, 'cancelled') will pass if both fast-checks and turbo-check are skipped (skipped ≠ failure/cancelled).
Why: While both jobs share the same if condition so they'll skip together (maintaining prior behavior), an explicit success check makes the intent clearer and provides a safety net against future changes that might cause asymmetric skipping.
Fix: Consider adding && contains(needs.*.result, 'success') to require at least one job succeeded:
if: always() && !contains(needs.*.result, 'failure') && !contains(needs.*.result, 'cancelled') && contains(needs.*.result, 'success')Refs:
| - name: Check env.ts descriptions | ||
| run: pnpm check:env-descriptions | ||
|
|
||
| - name: Check route handler patterns |
There was a problem hiding this comment.
💭 Consider: New check added to CI
Issue: The check:route-handler-patterns step is a new addition to CI, not just a migration from the old ci job. The original workflow only ran format:check, check:env-descriptions, and knip as standalone steps.
Why: This is actually a good enhancement that aligns CI with the local pnpm check command (which includes route-handler-patterns). Just noting it since the PR description implies these checks are being "moved" rather than "added".
Fix: No code change needed — consider updating the PR description to clarify this is an intentional addition that aligns CI behavior with local pnpm check.
Refs:
- package.json check script — shows
pnpm checkincludes route-handler-patterns
Move lightweight checks (format, env-descriptions, route-handler-patterns, knip) to a fast-checks job on ubuntu-latest (~3 min). Heavy checks (turbo check, agents-docs lint/typecheck, OpenAPI snapshot, Playwright tests) stay on turbo-check with ubuntu-32gb (~8 min). A ci gate job depends on both to preserve the required status check name. Both sub-jobs run in parallel, so the total CI time is max(fast, turbo) instead of fast + turbo sequentially. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
amikofalvy
force-pushed
the
prd-6284-split-ci-parallel
branch
from
0ef0b87 to
ff4615b
Compare
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a re-review following a pull_request:synchronize event. The code is unchanged from the prior review, and both pr-review-devops and pr-review-standards returned 0 new findings.
The PR correctly splits the monolithic CI workflow into two parallel jobs:
fast-checks(ubuntu-latest, 10min timeout): Format, env-descriptions, route-handler-patterns, knipturbo-check(ubuntu-32gb, 30min timeout): Turbo build/lint/typecheck/test, Playwright, OpenAPI snapshotsci(gate job): Verifies both sub-jobs succeeded, preserves required status check name
Implementation follows GitHub Actions best practices: SHA-pinned actions, appropriate permissions scoping, proper caching, and deterministic installs.
🕐 Pending Recommendations (2)
- 💭
.github/workflows/ci.yml:286Gate job condition could be strengthened with explicit success check - 💭
.github/workflows/ci.yml:89Note:check:route-handler-patternsis a new CI addition (aligns with localpnpm check)
✅ APPROVE
Summary: No new issues found on re-review. The prior approval stands — this is a well-structured CI optimization that reduces total CI time by running lightweight checks in parallel with heavy turbo/playwright checks. The two pending Consider items from the prior review are optional enhancements; the PR is ready to merge. 🚀
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Re-review with no changes since prior APPROVE — reviewers confirmed no new issues.
1 participant
Summary
cijob into two parallel jobs:fast-checks(ubuntu-latest) andturbo-check(ubuntu-32gb)cidepends on both, preserving the required status check nameTest plan
fast-checksjob runs format:check, check:env-descriptions, check:route-handler-patterns, and knip on ubuntu-latestturbo-checkjob runs turbo check, agents-docs lint/typecheck, and OpenAPI snapshot logic on ubuntu-32gbcigate job passes when both jobs succeedcigate job fails if either job failscreate-agents-e2ejob is unchanged🤖 Generated with Claude Code