ci: optimize pr-check-secondary-examples workflow (#1949)#2292
ci: optimize pr-check-secondary-examples workflow (#1949)#2292Mounil2005 wants to merge 8 commits into
Conversation
github-actions
Bot
added
scope: CI/CD
There was a problem hiding this comment.
Pull request overview
Optimizes the pr-check-secondary-examples workflow to reduce CI minutes by adding path/branch trigger filters, concurrency cancellation, a two-phase Node.js runner (changed examples first, full set second), a timeout, and a Jest-based test suite for the runner script. Also drops the pinned soloVersion, mirrorNodeVersion, and hieroVersion parameters from the hiero-solo-action step (unrelated to the optimization).
Changes:
- Restrict
push/pull_requesttriggers via branch+paths filters; addconcurrencywithcancel-in-progressand a 30-minute job timeout. - Replace the inline bash example loop with a Node.js script using
spawnSyncand a two-phase changed/remaining execution plan. - Add
package.json,package-lock.json,jest.config.js, and a Jest unit test suite for the runner; ignorenode_modulesand.claude/.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/pr-check-secondary-examples.yml | Adds triggers/paths filter, concurrency, timeout, Node setup, npm test step, invokes new JS runner; also unpins solo/mirror/hiero versions. |
| .github/scripts/pr-check-secondary-examples.js | New two-phase runner using git ls-files, git diff, and spawnSync("uv", ...). |
| .github/scripts/tests/pr-check-secondary-examples.test.js | Jest tests covering toModule, getAllExamples, getChangedExamples, computeExecutionPlan, runExample, runAll. |
| .github/scripts/jest.config.js | Scopes Jest to __tests__/ to avoid picking up existing node:test scripts. |
| .github/scripts/package.json / package-lock.json | Adds Jest as a dev dependency for the CI script tests. |
| .gitignore | Ignores .github/scripts/node_modules/ and .claude/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Mounil2005
force-pushed
the
optimize-examples-workflow
branch
from
d1a2836 to
f8040cd
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughNode.js script discovers tracked Python example files, computes changed vs remaining modules against the PR base ref, runs changed examples first then remaining examples using ChangesExample Checking Workflow Optimization
Sequence Diagram(s)sequenceDiagram
participant Script as pr-check-secondary-examples.js
participant Git as git
participant UV as uv run -m
Script->>Git: git ls-files examples
Git-->>Script: tracked example files
Script->>Script: filter out __init__.py
Script->>Git: git fetch origin (GITHUB_BASE_REF)
Script->>Git: git rev-parse --verify remoteRef
Script->>Git: git diff --name-only remoteRef...HEAD
Git-->>Script: changed example files
Script->>Script: computeExecutionPlan(all, changed) -> {changed, remaining}
alt changed non-empty
Script->>UV: Phase 1: runAll(changed)
UV-->>Script: exit 0/1
end
alt remaining non-empty and Phase 1 succeeded
Script->>UV: Phase 2: runAll(remaining)
UV-->>Script: exit 0/1
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 04deb248-27d4-4536-809f-b1687e071612
⛔ Files ignored due to path filters (1)
.github/scripts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.github/scripts/__tests__/pr-check-secondary-examples.test.js.github/scripts/jest.config.js.github/scripts/package.json.github/scripts/pr-check-secondary-examples.js.github/workflows/pr-check-secondary-examples.yml.gitignore
Mounil2005
force-pushed
the
optimize-examples-workflow
branch
from
86e5ea9 to
8d37e7c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 77680d90-8268-4db4-b853-167786f966e6
⛔ Files ignored due to path filters (1)
.github/scripts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/scripts/__tests__/pr-check-secondary-examples.test.js.github/scripts/jest.config.js.github/scripts/package.json.github/scripts/pr-check-secondary-examples.js.github/workflows/pr-check-secondary-examples.yml
github-actions
Bot
added
open to community review
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/scripts/pr-check-secondary-examples.js (1)
89-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the filtered phase-1 list from
computeExecutionPlanin main orchestration.Line 90 computes filtered
changedbut discards it, and Line 97/Line 99 still execute the rawchangedlist. This can reintroduce failures on deleted/renamed examples that are not runnable.Proposed fix
if (require.main === module) { const all = getAllExamples(); const changed = getChangedExamples(); - const { remaining } = computeExecutionPlan(all, changed); + const { changed: runnableChanged, remaining } = computeExecutionPlan(all, changed); console.log("\n=== Example Execution Plan ==="); - console.log("Changed:", changed.length ? changed : "(none)"); + console.log("Changed:", runnableChanged.length ? runnableChanged : "(none)"); console.log("Remaining:", remaining.length); console.log(""); - if (changed.length > 0) { + if (runnableChanged.length > 0) { console.log("🚀 Phase 1: Running CHANGED examples..."); - runAll(changed); + runAll(runnableChanged); } else { console.log("ℹ️ No changed examples detected"); }Also applies to: 93-100
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 636ee56a-61ac-41d0-b5d5-fd27408778c2
📒 Files selected for processing (3)
.github/scripts/__tests__/pr-check-secondary-examples.test.js.github/scripts/pr-check-secondary-examples.js.github/workflows/pr-check-secondary-examples.yml
Mounil2005
force-pushed
the
optimize-examples-workflow
branch
from
ed61aa8 to
7672ed4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2bbc188a-e011-44ad-bb03-1c206bd426a7
⛔ Files ignored due to path filters (1)
.github/scripts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/scripts/__tests__/pr-check-secondary-examples.test.js.github/scripts/jest.config.js.github/scripts/package.json.github/scripts/pr-check-secondary-examples.js.github/workflows/pr-check-secondary-examples.yml
aceppaluni
left a comment
aceppaluni
left a comment
There was a problem hiding this comment.
@Mounil2005 Did you test this?
|
Hello, this is the OfficeHourBot. This is a reminder that the Hiero Python SDK Office Hours will begin in approximately 3 hours (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here for any changes. From, |
aceppaluni
added
the
status: Needs Developer Revision
Mounil2005
force-pushed
the
optimize-examples-workflow
branch
from
6b87f1f to
350cd15
Compare
|
The changes are covered by a Jest unit test suite (21 scenarios) that runs as part of the workflow itself before the Solo network spins up. Tests cover:
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 29e4dd88-016f-4890-808d-ec373c02a73d
⛔ Files ignored due to path filters (1)
.github/scripts/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/scripts/__tests__/pr-check-secondary-examples.test.js.github/scripts/jest.config.js.github/scripts/package.json.github/scripts/pr-check-secondary-examples.js.github/workflows/pr-check-secondary-examples.yml
- Restrict push trigger to main; add paths filter for examples/**, src/**, and the workflow file itself - Add concurrency group to cancel stale runs - Replace inline bash with two-phase Node.js script (changed examples first, then rest) - Use spawnSync with arg array for uv run -m (fixes Codacy command injection warning) - Reuse git ls-files pattern from pr-check-test-files.js - Add Jest with 18 test scenarios; runs before Solo network spins up - Add timeout-minutes: 30 Signed-off-by: IGwork <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Mounil2005
force-pushed
the
optimize-examples-workflow
branch
from
ef82f6e to
2e1d55d
Compare
Description:
Optimize the
pr-check-secondary-examplesworkflow to eliminate unnecessary CI runsand reduce resource consumption, addressing the 16,000+ minutes/month reported in #1949.
Builds on the Node.js approach from exploreriii's prior work in #2073.
mainand addpathsfilter (examples/**,src/**,workflow file) so the job skips documentation-only and unrelated changes
cancel-in-progress: trueto cancel stale runson force-push or rapid commits
examples changed in the PR (fast feedback); Phase 2 runs the rest as a full
regression check, only if Phase 1 passes
spawnSyncwith an argument array instead of shell string interpolationwhen invoking
uv run -m, eliminating the Codacy command-injection warninggit ls-files examplespattern frompr-check-test-files.jsforconsistent file discovery across CI scripts
toModule,getAllExamples,getChangedExamples,computeExecutionPlan,runExample, andrunAll;runs in workflow before Solo network spins up so failures surface early
jest.config.jsscoping Jest to__tests__/— pre-existing scriptsuse Node's built-in
node:testrunner and are incompatible with Jesttimeout-minutes: 30to prevent hung runs from blocking CI indefinitelyRelated issue(s):
Fixes #1949
Notes for reviewer:
This builds directly on exploreriii's draft PR #2073 (
ci: guard examples and optimise on diff), which was closed due to inactivity. The two-phase executionstrategy and Node.js script structure are preserved; the following gaps from
that PR are addressed here:
spawnSyncwith arg arrayjest.config.jsadded so pre-existingnode:testfiles arenot accidentally picked up by Jest
exported functions
The workflow change is backwards-compatible —
workflow_dispatchis retainedso the job can still be triggered manually for any branch.
Checklist