ci: gate CircleCI jobs on changed paths - #32080
Conversation
Every CircleCI job used to run on every PR. Now each job starts with a lightweight `skip_if_unrelated_changes` step that inspects the PR diff and halts the job as successful when nothing relevant changed. Docs-only PRs (*.md, *.mdx, docs/) run nothing, UI-only PRs (ui/) run just the frontend jobs, and any backend change still runs both the backend and frontend jobs. The decision logic lives in .circleci/scripts/classify_changes.sh (pure, reads the changed-file list on stdin) so it can be unit tested, while path_filter.sh handles the git plumbing and fails open (runs the job) on any uncertainty such as a missing merge base or a non-PR pipeline. Halting via `circleci-agent step halt` keeps the job green, so required status checks are never left pending. The Windows smoke job is intentionally left ungated to avoid cross-platform shell fragility
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR adds change-based job gating to the CircleCI
Confidence Score: 5/5Safe to merge. The fail-open contract is sound, both previously flagged issues are correctly fixed, and the test suite covers the critical edge cases. The two previously flagged defects — the unguarded classifier call that could silently halt jobs when the script errored, and the No files require special attention.
|
| Filename | Overview |
|---|---|
| .circleci/scripts/path_filter.sh | New wrapper script: resolves merge base, diffs against HEAD, and calls classify_changes.sh. Both previously flagged issues (non-fail-open classifier call, pipefail-unsafe logging) are correctly fixed with ` |
| .circleci/scripts/classify_changes.sh | Pure classification function mapping changed file list (stdin) + category to run/skip. Bash case pattern ordering is correct: ui/* is checked before *.md/*.mdx so markdown under ui/ counts as client, not docs. Unknown category falls through to echo run (fail-open). |
| tests/test_litellm/test_circleci_path_filter.py | 17 tests covering the classification matrix (14 parametrized + 3 edge cases) plus 4 end-to-end path_filter.sh tests including the new regression test for classifier failures. Tests use only local git repos and subprocess calls — no real network calls, complying with the repo's mock-only test rule. |
| .circleci/config.yml | Adds the skip_if_unrelated_changes reusable command and inserts it as the first post-checkout step in ~40 jobs. Backend jobs use the default category: backend; the four UI jobs (ui_build, ui_unit_tests, e2e_ui_testing, e2e_ui_testing_server_root_path) correctly use category: client. The Windows smoke job is intentionally left ungated per the PR description. |
Reviews (2): Last reviewed commit: "fix(ci): keep path filter fail-open when..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Guard the classify_changes.sh invocation with `|| run_full` so a broken or non-zero classifier runs the job instead of falling through to a silent halt, and mark the advisory logging pipe best-effort with `|| true`. Add path_filter.sh regression tests covering the docs-only halt, backend run, non-PR fail-open, and classifier-failure fail-open paths
|
@greptileai I've addressed both findings in 50945cc. The classifier invocation is now guarded with |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 50945cc. Configure here.
| if [ -z "$merge_base" ] || git merge-base --is-ancestor "$merge_base" "$candidate" 2>/dev/null; then | ||
| merge_base="$candidate" | ||
| fi | ||
| done |
There was a problem hiding this comment.
Merge base picks newest candidate
Medium Severity
When resolving the PR diff, the script keeps the newest merge base among main, litellm_internal_staging, and litellm_oss_staging instead of the base branch the PR targets. That can shrink git diff so backend edits already on the branch disappear from the changed-file list while later docs-only commits remain, and backend jobs halt as successful even though the PR still carries backend changes relative to the target branch.
Reviewed by Cursor Bugbot for commit 50945cc. Configure here.
📝 WalkthroughWalkthroughAdds CircleCI scripts ChangesCircleCI path-based job gating
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CircleCIJob
participant PathFilterSh
participant Git
participant ClassifyChangesSh
CircleCIJob->>PathFilterSh: run with category argument
PathFilterSh->>Git: fetch candidate base branches
Git-->>PathFilterSh: merge base commit
PathFilterSh->>Git: diff HEAD vs merge base
Git-->>PathFilterSh: changed file list
PathFilterSh->>ClassifyChangesSh: pipe changed files, category
ClassifyChangesSh-->>PathFilterSh: run or skip decision
PathFilterSh->>CircleCIJob: halt step or continue
Related PRs: None identified Suggested labels: ci, testing Suggested reviewers: None identified 🐰 A rabbit sniffs at paths both new and old, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_litellm/test_circleci_path_filter.py (1)
112-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the
circleci-agentstub setup into a fixture.The stub-binary creation and env-var wiring in
_run_path_filteris straightforward but could become apytest.fixtureto reduce duplication if more integration tests are added later. Not urgent given only four call sites currently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_litellm/test_circleci_path_filter.py` around lines 112 - 129, Extract the `circleci-agent` stub setup and PATH/env wiring from `_run_path_filter` into a reusable pytest fixture or helper so future tests can share it; keep `_run_path_filter` focused on invoking `subprocess.run` for `path_filter.sh`, and preserve the current `CIRCLE_PULL_REQUEST` behavior when `is_pr` is toggled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_litellm/test_circleci_path_filter.py`:
- Around line 112-129: Extract the `circleci-agent` stub setup and PATH/env
wiring from `_run_path_filter` into a reusable pytest fixture or helper so
future tests can share it; keep `_run_path_filter` focused on invoking
`subprocess.run` for `path_filter.sh`, and preserve the current
`CIRCLE_PULL_REQUEST` behavior when `is_pr` is toggled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1886f074-33c9-4d17-b912-b8187afffe78
📒 Files selected for processing (4)
.circleci/config.yml.circleci/scripts/classify_changes.sh.circleci/scripts/path_filter.shtests/test_litellm/test_circleci_path_filter.py
|
Can you confirm if skipped tests count as successful runs? |
yep. on skip the job runs circleci-agent step halt (.circleci/scripts/path_filter.sh:40). see: https://support.circleci.com/hc/en-us/articles/360015562253-Conditionally-End-a-Running-Job-Gracefully <- CircleCI documents as ending the job successfully, so GitHub checks show green |
* ci: gate CircleCI jobs on changed paths Every CircleCI job used to run on every PR. Now each job starts with a lightweight `skip_if_unrelated_changes` step that inspects the PR diff and halts the job as successful when nothing relevant changed. Docs-only PRs (*.md, *.mdx, docs/) run nothing, UI-only PRs (ui/) run just the frontend jobs, and any backend change still runs both the backend and frontend jobs. The decision logic lives in .circleci/scripts/classify_changes.sh (pure, reads the changed-file list on stdin) so it can be unit tested, while path_filter.sh handles the git plumbing and fails open (runs the job) on any uncertainty such as a missing merge base or a non-PR pipeline. Halting via `circleci-agent step halt` keeps the job green, so required status checks are never left pending. The Windows smoke job is intentionally left ungated to avoid cross-platform shell fragility * fix(ci): keep path filter fail-open when classifier errors Guard the classify_changes.sh invocation with `|| run_full` so a broken or non-zero classifier runs the job instead of falling through to a silent halt, and mark the advisory logging pipe best-effort with `|| true`. Add path_filter.sh regression tests covering the docs-only halt, backend run, non-PR fail-open, and classifier-failure fail-open paths
3 participants
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
The gating decision is a pure function that we unit test, plus a local end to end run of the real script against a throwaway git repo (a stub stands in for
circleci-agentso the halt path is observable)This PR itself touches
.circleci/andtests/, which count as backend, so every job runs here as expected; a docs-only orui/-only PR is where the skips kick inType
🚄 Infrastructure
Changes
Previously every job in the CircleCI
build_and_testworkflow ran on every PR regardless of what changed. Each job now begins, right aftercheckout, with askip_if_unrelated_changesstep that resolves the PR's merge base, diffs it againstHEAD, and halts the job as successful when the change is not relevant to that job's category. Docs-only PRs (*.md,*.mdx,docs/) run nothing,ui/-only PRs run just the four frontend jobs (ui_build,ui_unit_tests,e2e_ui_testing,e2e_ui_testing_server_root_path), and any backend change runs both the backend and frontend jobsThe logic is split so the risky part is testable.
.circleci/scripts/classify_changes.shis a pure mapping from a changed-file list on stdin plus a category torun/skip, and.circleci/scripts/path_filter.shwraps it with the git plumbing. The wrapper fails open (runs the job) on anything uncertain: a non-PR pipeline, an unresolvable merge base, or a git error, so we never skip real tests by accident. It halts withcircleci-agent step halt, which reports the job as green, so this plays nicely with branch-protection required checksThe Windows install smoke job is deliberately left ungated so we don't have to run the bash gate under PowerShell; it is a single cheap job and keeping it always-on avoids cross-platform fragility. It can be gated in a follow-up if desired
Note
Medium Risk
Incorrect path gating could skip required CI on real code changes; mitigated by fail-open behavior and dedicated regression tests, but misclassification would still be a process risk until caught.
Overview
CircleCI jobs no longer always run the full test matrix on every PR. After checkout, most jobs invoke a new
skip_if_unrelated_changesstep that diffs the PR against a resolved merge base and halts the job as successful when changes are not relevant.Docs-only PRs (
docs/,*.md,*.mdx) skip both backend and client jobs.ui/-only PRs run the four client jobs (ui_build,ui_unit_tests,e2e_ui_testing,e2e_ui_testing_server_root_pathwithcategory: client) but skip backend jobs. Any non-doc backend path runs backend jobs and also client jobs (backend changes are treated as needing UI coverage).The decision logic lives in
.circleci/scripts/classify_changes.sh(stdin file list →run/skip) and.circleci/scripts/path_filter.sh(git merge-base, PR detection,circleci-agent step halt). The wrapper fails open (runs the job) when not a PR, merge base cannot be resolved, git errors occur, or the classifier fails.using_litellm_on_windowsis intentionally left ungated.tests/test_litellm/test_circleci_path_filter.pylocks the classification contract and end-to-end halt vs run behavior.Reviewed by Cursor Bugbot for commit 50945cc. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Chores
Tests