chore: improve CI performance by splitting build and test phases#2092
chore: improve CI performance by splitting build and test phases#2092nick-inkeep merged 1 commit intomainfrom
Conversation
Split the single `pnpm check` step into two phases: 1. Build all packages first (heavy compilation) 2. Run lint, typecheck, and test (builds already cached) This prevents Playwright browser tests (Monaco editor) from competing with heavy Next.js/agents-api builds for CPU on cache-miss runs, which caused the Monaco editor to get stuck in skeleton loading state and the screenshot test to time out on ubuntu-16gb runners. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You could take this in a different direction and setup the correct dependencies within turborepo + constrain parallel task count in turbo repo. This will do a better job of responding to cache hits/misses. |
![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
💭 Consider (1) 💭
Inline Comments:
- 💭 Consider:
ci.yml:112-114AddENVIRONMENT: testto build step for cache key consistency with the check step
✅ APPROVE
Summary: This is a well-reasoned CI optimization that separates heavy builds from browser tests to prevent CPU contention on cache-miss runs. The approach is sound — Turbo's task graph ensures builds complete before lint/typecheck/test, and cached outputs will be reused in the check step. The one minor consideration is adding ENVIRONMENT: test to the build step to ensure cache key consistency (since it's in globalEnv), but this is non-blocking. The PR description clearly explains the problem, solution, and test plan. Ship it! 🚀
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
ci.yml |
Approach validation (positive) | Not an issue — reviewer confirmed the two-phase build-then-check pattern is correctly implemented |
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
2 | 0 | 0 | 0 | 1 | 0 | 1 |
| Total | 2 | 0 | 0 | 0 | 1 | 0 | 1 |
| env: | ||
| NODE_OPTIONS: --max-old-space-size=4096 | ||
| CI: true |
There was a problem hiding this comment.
💭 Consider: Add ENVIRONMENT: test for cache key consistency
Issue: The "Build packages" step doesn't set ENVIRONMENT, while the "Run CI checks" step sets ENVIRONMENT: test. Since ENVIRONMENT is in Turbo's globalEnv (turbo.json:11), it affects cache key computation for all tasks.
Why: When the build step runs with ENVIRONMENT=undefined and the check step runs with ENVIRONMENT=test, Turbo computes different cache keys. This could cause the check step to miss the build cache and re-run builds, partially defeating the optimization goal.
Fix:
| env: | |
| NODE_OPTIONS: --max-old-space-size=4096 | |
| CI: true | |
| env: | |
| ENVIRONMENT: test | |
| NODE_OPTIONS: --max-old-space-size=4096 | |
| CI: true |
Refs:
2 participants
Summary
pnpm checkCI step into two phases: build first, then run quality checksProblem
On PR branches with Turbo cache misses (11-13 out of 44 tasks), the
pnpm checkstep runs all builds, lint, typecheck, and tests in parallel with concurrency 15. Heavy builds (Next.js, agents-api, agents-docs) starve Playwright/Monaco of CPU, causing the Monaco editor to stay in skeleton loading state and the screenshot test (should properly highlight nested error state) to time out.This only affects PR branches — main has warm Turbo cache (0 misses) so builds are instant.
Solution
pnpm turbo run buildfirst, completing ALL compilation before tests startpnpm check(lint + typecheck + test) with builds already cachedWhen
pnpm checkruns in phase 2, Turbo sees all build outputs are cached → skips builds → only runs lint, typecheck, and test. The browser test gets full CPU instead of competing with Next.js compilation.Test plan
ubuntu-16gb)🤖 Generated with Claude Code