perf: optimize CI/CD pipeline with caching and parallel builds#42
perf: optimize CI/CD pipeline with caching and parallel builds#42anandgupta42 merged 2 commits intomainfrom
Conversation
- Add Bun dependency caching to CI and Release workflows - Add pip caching to Python test, docs, and engine publish workflows - Separate lint into its own fast job (dev-only deps, no warehouses) - Split release binary builds into 3 parallel matrix jobs (linux/darwin/win32) - Add --targets flag to build.ts for OS-based target filtering - Remove unnecessary publish-npm dependency from GitHub Release job - Add concurrency control to release workflow Expected improvements: - Release: ~18min → ~8-9min (parallel builds + parallel publishing) - CI: faster repeat runs via dependency caching Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| @@ -177,11 +202,12 @@ | |||
There was a problem hiding this comment.
thought: github-release now depends only on [build] instead of [build, publish-npm]. This means the GitHub Release is created before npm publish completes. If npm publish fails, you'll have a release with binaries but no npm package. Intentional tradeoff for speed?
|
|
||
| const targets = singleFlag | ||
| ? allTargets.filter((item) => { | ||
| if (item.os !== process.platform || item.arch !== process.arch) { |
There was a problem hiding this comment.
praise: Clean approach — the --targets flag keeps the build script backwards-compatible while enabling the matrix split. No changes needed to the build logic itself.
| working-directory: packages/altimate-engine | ||
|
|
||
| - name: Lint | ||
| run: ruff check src |
There was a problem hiding this comment.
suggestion: Since this lint job only needs ruff, you could install just ruff directly instead of .[dev] (which may pull in pytest and other dev deps). Would make this job even faster:
- name: Install linter
run: pip install ruffThough .[dev] ensures version consistency with what devs use locally, so this is a minor tradeoff.
jontsai
left a comment
There was a problem hiding this comment.
PR #42 Review Tour Guide
File Order
| # | File | +/- | Purpose |
|---|---|---|---|
| 1 | .github/workflows/release.yml |
+36/-10 | Matrix build strategy, parallel jobs, caching |
| 2 | .github/workflows/ci.yml |
+30/-4 | Separate lint job, Bun + pip caching |
| 3 | packages/altimate-code/script/build.ts |
+6/-1 | --targets flag for OS-based filtering |
| 4 | .github/workflows/docs.yml |
+2/-0 | pip caching |
| 5 | .github/workflows/publish-engine.yml |
+1/-0 | pip caching |
Stop 1: Release Workflow — Matrix Build (release.yml)
Context: The 11 binary targets are split into 3 parallel matrix jobs by OS (linux/darwin/win32). Each uploads dist-{os} artifacts, and downstream jobs use merge-multiple: true to combine them.
Review:
- ✅
fail-fast: false— correct, one OS failure shouldnt block others - ✅
concurrencywithcancel-in-progress: false— prevents overlapping releases - ✅
merge-multiple: trueon download — clean artifact merging ⚠️ github-releaseno longer depends onpublish-npm(posted inline comment)
Stop 2: CI Workflow — Lint Separation (ci.yml)
Context: Python linting moved from the test matrix into its own job, avoiding heavy warehouse deps (snowflake/bigquery SDKs).
Review:
- ✅ Good separation — lint doesnt need warehouse deps
- 💡 Could use just
pip install ruffinstead of.[dev](posted inline suggestion) - ✅ Bun caching with
bun.lockhash key
Stop 3: Build Script — --targets Flag (build.ts)
Context: Simple flag parsing to filter allTargets by OS name.
Review:
- ✅ Backward compatible — no flag builds all 11 targets
- ✅ Clean integration with existing
singleFlaglogic
Summary Checklist
| Area | Status | Notes |
|---|---|---|
| Matrix build | ✅ Good | 3-way parallel, proper artifact naming |
| Caching | ✅ Good | Bun + pip across all workflows |
| Dependency chain | github-release no longer waits for npm | |
| Build script | ✅ Good | Backward compatible --targets flag |
| publish-engine | ✅ Good | Correctly decoupled from build artifacts |
LGTM — ship it! 🚢
suryaiyer95
left a comment
There was a problem hiding this comment.
Multi-Model Code Review — 6 AI Models
Verdict: APPROVE (with suggestions)
Critical: 0 | Major: 1 | Minor: 4 | Nits: 2
Reviewed by Claude Opus 4.6, Kimi K2.5, Grok 4, MiniMax M2.5, GLM-5, Qwen 3.5 Plus — converged in 1 round.
Major Issues
1. GitHub Release created before npm publish completes — partial release risk
Design — .github/workflows/release.yml:144 (needs: [build], changed from needs: [build, publish-npm])
Previously the GitHub Release was only created after npm publish succeeded. Now it runs in parallel. If npm publish fails:
- Users see a GitHub Release with
npm installinstructions that don't work - No automatic rollback — the release stays up
- Downstream consumers (Homebrew tap) reference the release but need npm packages
This saves ~7min but the failure mode creates user confusion.
Fix: Either restore needs: [build, publish-npm] for atomicity, add a post-publish verification step, or add a YAML comment documenting the tradeoff.
Flagged by: Claude, Kimi, GLM-5, Grok (Consensus)
Minor Issues
2. docs.yml pip cache uses mkdocs.yml as dependency path
Bug / Performance — .github/workflows/docs.yml:29-30
cache: "pip"
cache-dependency-path: docs/mkdocs.ymlmkdocs.yml is a config file, not a pip requirements file. The cache key won't change when mkdocs-material releases a new version, serving stale packages.
Fix: Create docs/requirements.txt with mkdocs-material and reference it. (Claude, GLM-5, MiniMax, Qwen, Grok, Kimi — Consensus)
3. publish-engine.yml missing cache-dependency-path
Performance — .github/workflows/publish-engine.yml:21
Pip cache enabled but without cache-dependency-path, so invalidation relies on auto-detection which may miss pyproject.toml.
Fix: Add cache-dependency-path: packages/altimate-engine/pyproject.toml. (GLM-5, MiniMax, Grok, Kimi — Consensus)
4. --targets flag has no validation for invalid OS values
Code Quality — packages/altimate-code/script/build.ts:123
--targets=windows (instead of win32) silently produces zero targets. The rm -rf dist runs first, then nothing gets built — no error, no warning.
Fix: Validate against ['linux', 'darwin', 'win32'] and exit with an error for invalid values. (Claude, GLM-5, Grok, Qwen, Kimi — Consensus)
5. publish-engine has no needs: — runs immediately on tag push
Design — .github/workflows/release.yml:116-117
Engine publishes to PyPI independently, without waiting for build. While the engine doesn't need build artifacts, a PyPI release could exist for a broken CLI version.
Fix: Add a YAML comment documenting this is intentional, or restore needs: build. (Claude, GLM-5, Qwen, Kimi — Consensus)
Nits
- Redundant concurrency group name at
release.yml:9—release-${{ github.workflow }}evaluates torelease-Release. Use justreleaseor${{ github.workflow }}. - Repeated Bun cache block (3 identical 6-line blocks) — could be extracted to a composite action.
Positive Observations
- Matrix strategy is correct:
fail-fast: falseensures all platforms build.needs: [build]correctly waits for ALL matrix jobs. - Artifact merge pattern:
dist-${{ matrix.os }}upload +pattern: dist-*withmerge-multiple: trueis the proper GitHub Actions pattern. - Lint job separation: Moving lint to its own job with only
[dev]deps (no heavy warehouse packages) is a smart optimization. - Bun caching: Well-configured with
hashFiles('bun.lock')and fallback restore-keys. --targetsflag: Clean implementation that integrates naturally with existing flags.- Concurrency control:
cancel-in-progress: falseis correct — never cancel a running release. - Cross-compilation: Building darwin/win32 on ubuntu-latest using Bun's native cross-compile is efficient and correct.
Missing Tests
--targetswith invalid OS name (e.g.,--targets=windows)--targetswith empty string--targetscombined with--single(precedence behavior)
Finding Attribution
| Issue | Origin | Type |
|---|---|---|
| 1. GitHub Release before npm publish | Claude, Kimi, GLM-5, Grok | Consensus |
| 2. docs.yml pip cache misconfigured | Claude, GLM-5, MiniMax, Qwen, Grok, Kimi | Consensus |
| 3. publish-engine.yml missing cache path | GLM-5, MiniMax, Grok, Kimi | Consensus |
| 4. --targets flag no validation | Claude, GLM-5, Grok, Qwen, Kimi | Consensus |
| 5. publish-engine no needs dependency | Claude, GLM-5, Qwen, Kimi | Consensus |
| 6. Redundant concurrency group name | Claude, Kimi | Consensus |
| 7. Repeated Bun cache block | Claude | Unique |
Reviewed by 6 models: Claude Opus 4.6, Kimi K2.5, Grok 4, MiniMax M2.5, GLM-5, Qwen 3.5 Plus. Convergence: 1 round.
Notable rejected findings:
- Cross-compilation on ubuntu-latest (Bun supports this natively)
cancel-in-progress: falserace condition (concurrency group queues runs correctly)- Matrix
needsdoesn't check success (needs: [build]waits for ALL matrix jobs by default) - Cache key should use
matrix.os(shared cache is optimal since all jobs run on same runner)
- Restore github-release dependency on publish-npm to prevent partial releases where npm install instructions point to unpublished packages - Use `pip install ruff` in lint job instead of `.[dev]` for faster runs - Create docs/requirements.txt for proper pip cache invalidation - Add cache-dependency-path to publish-engine and release publish-engine - Validate --targets flag against known OS values (linux/darwin/win32) - Add comment documenting intentional publish-engine parallelism - Simplify concurrency group name Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* perf: optimize CI/CD pipeline with caching and parallel builds - Add Bun dependency caching to CI and Release workflows - Add pip caching to Python test, docs, and engine publish workflows - Separate lint into its own fast job (dev-only deps, no warehouses) - Split release binary builds into 3 parallel matrix jobs (linux/darwin/win32) - Add --targets flag to build.ts for OS-based target filtering - Remove unnecessary publish-npm dependency from GitHub Release job - Add concurrency control to release workflow Expected improvements: - Release: ~18min → ~8-9min (parallel builds + parallel publishing) - CI: faster repeat runs via dependency caching Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback - Restore github-release dependency on publish-npm to prevent partial releases where npm install instructions point to unpublished packages - Use `pip install ruff` in lint job instead of `.[dev]` for faster runs - Create docs/requirements.txt for proper pip cache invalidation - Add cache-dependency-path to publish-engine and release publish-engine - Validate --targets flag against known OS values (linux/darwin/win32) - Add comment documenting intentional publish-engine parallelism - Simplify concurrency group name Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
linux/darwin/win32), reducing build time from ~11min to ~4min~/.bun/install/cache) and pip cache across all workflowspublish-engineruns concurrently with build (no artifact dependency needed)Files changed
.github/workflows/release.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/publish-engine.ymlpackages/altimate-code/script/build.ts--targetsflag for OS-based filteringExpected impact
Release workflow dependency graph
Test plan
bun test)--targetsflag unit tested: linux=6, darwin=3, win32=2, total=11--targetsflag builds all 11 targets🤖 Generated with Claude Code