ci: gate e2e jobs on pre-commit success by aoshen02 · Pull Request #26 · vllm-project/vime

aoshen02 · 2026-05-23T13:25:01Z

Summary

Add a pre-commit gate job to the PR Test workflow. Every existing e2e job in pr-test.yml (e2e-test-short, e2e-test-megatron, e2e-test-ckpt, e2e-test-image, e2e-test-changed*, e2e-test-plugin-contracts, ...) now has needs: pre-commit. If the gate fails, every downstream job — including self-hosted GPU jobs — is skipped.

.github/workflows/pre-commit.yml is untouched (byte-identical to main), so any branch-protection rule that references its pre-commit / Run pre-commit check still works. The new gate inside PR Test is an additional, internal gate.

Why this is not duplicating an existing PR

gh pr list --repo vllm-project/vime --state open searches for "pre-commit ci" and "needs pre-commit" returned no matches. Closest unrelated PR: #18 ([Clean] Remove SGlang runtime code).

Code changes

File	Change
`.github/workflows/pr-test.yml.j2`	Add a `pre-commit` job at the top using `pre-commit/action@<sha>` (v3.0.1, SHA-pinned). Add `needs: pre-commit` to every job in the `jobs.items()` loop and to `e2e-test-changed-detect`. Change `e2e-test-changed` to `needs: [pre-commit, e2e-test-changed-detect]`.
`.github/workflows/pr-test.yml`	Regenerated from the `.j2` template via `python .github/workflows/generate_github_workflows.py`.

pre-commit/action@<sha> is the same action vLLM uses (reference); SHA-pinned because it is a newly-added dependency.

Behavior

Push to main: unchanged — pre-commit.yml runs as before. PR Test does not run on push (its push trigger is commented out).
PR opened / synchronized: pre-commit.yml runs as a separate status check (as today). Inside PR Test, the new pre-commit gate also runs; if it fails, all downstream e2e jobs (labeled and always-on) are skipped.
Label added (e.g. run-ci-megatron): the PR Test workflow is triggered; the gate runs first; only if it passes does the labeled GPU job start. This is the primary cost saving — self-hosted GPU runners no longer fire on lint-broken code.
workflow_dispatch: gate runs first, then every job runs (matches existing behavior).

There will be one extra pre-commit run per PR (one in pre-commit.yml, one as the gate in pr-test.yml) — both on the free ubuntu-latest runner. This is the cost of keeping pre-commit.yml byte-identical so existing branch protection isn't disturbed.

Test commands run

# Regen round-trip
uv run --with jinja2 python .github/workflows/generate_github_workflows.py

# YAML parses
uv run --with pyyaml python -c "
import yaml; yaml.safe_load(open('.github/workflows/pr-test.yml')); print('YAML OK')
"
# -> YAML OK

# pre-commit hooks pass on the touched files
uv run --with pre-commit pre-commit run --files \
    .github/workflows/pr-test.yml .github/workflows/pr-test.yml.j2
# -> check yaml / case conflicts / private key / large files: Passed
#    (other hooks: no files to check / Skipped)

# Sanity: every e2e-* job in the generated YAML has `needs: pre-commit`
grep -nE '^  [a-z][a-z0-9-]+:|^    needs:' .github/workflows/pr-test.yml

Functional verification ("does GitHub actually skip the e2e jobs when the gate fails") will happen on this PR itself once CI runs — please review the GitHub Actions run on this PR before merging.

What I deliberately did NOT do

vLLM's pre-run-check (author-reputation gate: requires ready/verified label OR author ≥ 4 merged PRs to even run pre-commit). vime is a smaller project with a controlled contributor set; this is overkill today.
Auto-comment on pre-commit failure (mergify-style help comment with uv pip install pre-commit instructions). Was prototyped on this branch (commits 8d785e6 history) but reverted in favor of simplicity. Easy to add later as a focused PR if maintainers find it useful.
Migrating to SHA-pin all third-party actions. Existing vime workflows tag-pin (actions/checkout@v6); this PR only SHA-pins the newly-added pre-commit/action. Consistent SHA migration is a separate decision.

AI assistance disclosure

This PR was prepared with assistance from Claude Code (Opus 4.7). The submitting human (aoshen02) reviewed every changed line and ran the verification commands above. Per AGENTS.md §1.

Add a `pre-commit` job at the top of the PR Test workflow and make every e2e-test-* job depend on it via `needs: pre-commit`. Self-hosted GPU runners no longer start label-triggered runs until lint passes. Regenerated pr-test.yml from pr-test.yml.j2 via `python .github/workflows/generate_github_workflows.py`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

gemini-code-assist · 2026-05-23T13:25:06Z

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

Two small follow-ups inspired by vllm-project/vllm: 1. Replace the manual `pip install pre-commit` + cache pair with the official `pre-commit/action@<sha>` (SHA-pinned to v3.0.1). The action handles hook caching internally and is the same one vLLM uses. Touches both: - The new `pre-commit` gate job in pr-test.yml.j2 (regenerated). - The standalone pre-commit.yml workflow. 2. In pre-commit.yml, when the hook fails on a PR, post a one-time comment with the exact local-fix instructions (uv pip install pre-commit / pre-commit run --all-files). Idempotent via an HTML-comment marker so re-runs don't spam the PR. This replaces what vLLM does with mergify; vime doesn't have mergify so we do it inline with actions/github-script (same pattern as bot-slash-lint.yaml). Requires `pull-requests: write` on this job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Revert the second-commit changes to .github/workflows/pre-commit.yml (auto-comment + pre-commit/action swap + pull-requests:write). The auto-comment was the heaviest piece of this PR (~35 lines of JS plus dedup logic) and isn't load-bearing for the stated goal — "GPU CI should not start until pre-commit passes" is fully achieved by the gate job in pr-test.yml alone. Net effect of this PR is now just two files: pr-test.yml.j2 (gate + needs:) and pr-test.yml (regenerated). pre-commit.yml is byte-identical to origin/main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

CalvinXKY · 2026-05-24T01:22:28Z

@@ -109,6 +130,7 @@ jobs:


  e2e-test-sglang-config:


There are quite a few SGLang configurations in these two files. Can they be removed?

Will merge pr 18 first and then it will be removed.

Resolve conflict in .github/workflows/pr-test.yml by regenerating from pr-test.yml.j2 so the renamed e2e-test-vllm-config job keeps the 'needs: pre-commit' gate added on main (#26). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: gate e2e jobs on pre-commit success Add a `pre-commit` job at the top of the PR Test workflow and make every e2e-test-* job depend on it via `needs: pre-commit`. Self-hosted GPU runners no longer start label-triggered runs until lint passes. Regenerated pr-test.yml from pr-test.yml.j2 via `python .github/workflows/generate_github_workflows.py`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: switch to pre-commit/action and auto-comment fix steps on failure Two small follow-ups inspired by vllm-project/vllm: 1. Replace the manual `pip install pre-commit` + cache pair with the official `pre-commit/action@<sha>` (SHA-pinned to v3.0.1). The action handles hook caching internally and is the same one vLLM uses. Touches both: - The new `pre-commit` gate job in pr-test.yml.j2 (regenerated). - The standalone pre-commit.yml workflow. 2. In pre-commit.yml, when the hook fails on a PR, post a one-time comment with the exact local-fix instructions (uv pip install pre-commit / pre-commit run --all-files). Idempotent via an HTML-comment marker so re-runs don't spam the PR. This replaces what vLLM does with mergify; vime doesn't have mergify so we do it inline with actions/github-script (same pattern as bot-slash-lint.yaml). Requires `pull-requests: write` on this job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: drop pre-commit.yml changes; keep only the gate Revert the second-commit changes to .github/workflows/pre-commit.yml (auto-comment + pre-commit/action swap + pull-requests:write). The auto-comment was the heaviest piece of this PR (~35 lines of JS plus dedup logic) and isn't load-bearing for the stated goal — "GPU CI should not start until pre-commit passes" is fully achieved by the gate job in pr-test.yml alone. Net effect of this PR is now just two files: pr-test.yml.j2 (gate + needs:) and pr-test.yml (regenerated). pre-commit.yml is byte-identical to origin/main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Reverts PR #26 (pre-commit gate) + PR #110 (e2e-test-unit). Syncs from slime: opened/reopened trigger types, --pull=always, pip deps (requests ray safetensors), cpu-unittest rename. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Reverts PR #26 (pre-commit gate) + PR #110 (e2e-test-unit). Syncs from slime: opened/reopened trigger types, --pull=always, pip deps (requests ray safetensors), cpu-unittest rename. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai>

* sync: complete slime #1920, #1967, #1985 — lint, fully_async, PYTHONUNBUFFERED Traced 3 individual diffs back to their source slime PRs (以点带面) and synced all remaining changes from each: slime #1985 (make tests shorter): - Wrap NamedTemporaryFile across 3 lines in test_vllm_config_mixed_offload_ft.py - Remove extra blank line in test_vllm_config_mixed_offload.py (parameter shortening already synced in vime PR #218) slime #1920 (move fully_async example to main codebase): - Rewrite README.md to match upstream (qwen2.5-0.5B, not qwen3-4b) - Add run-qwen2.5-0.5B-fully_async.sh with proper vLLM translations - Delete run-qwen3-4b-fully_async.sh (upstream removed it) slime #1967 (fix PYTHONBUFFERED typo): - Fix PYTHONBUFFERED=16 → PYTHONUNBUFFERED=1 in 3 scripts: run-glm4.7-30B-A3B.sh, run-glm4.7-355B-A32B.sh, run-minimax-m2.sh (command_utils.py already fixed in prior sync) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix: correct §2.4 flag translations in analyze_profile.py --enforce-eager → --vllm-enforce-eager --gpu-memory-utilization → --vllm-gpu-memory-utilization These are diagnostic hint strings, not CLI invocations, but should still use the canonical vime flag names (§2.4 translation rules). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix: mirror slime underline + revert group_id comment to rollout_id - analyze_profile.py: match slime's 30-char underline (was 28) - run_qwen36_35b_a3b_swe_8nodes.sh: revert group_id→rollout_id in comment (partial sync of slime #2013, target commit 44d29ee5) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * refactor(arguments): use FlexibleArgumentParser; revert dist.gather_object patch arguments.py (361→346 lines): - Import FlexibleArgumentParser from vllm.utils.argparse_utils; use it in vllm_parse_args() and get_vllm_cli_action_table() so vLLM's deprecated kwarg is handled natively on Python 3.12 without a shim - Remove _ARGPARSE_UNSUPPORTED_KWARGS + _strip_unsupported_argparse_kwargs - Remove import logging / logger (unused) reloadable_process_group.py: - Drop dist.gather_object monkey-patch (added by PR #22, never in slime; callers explicitly use all_gather_object to stay on the patched path) Architecture note: subprocess vllm serve kept intentionally — run_server() is not in vllm.__all__ and changes across minor versions; AReaL uses the same Popen pattern for the same reason. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * ci: revert aoshen02 CI additions; align J2 template with slime Reverts PR #26 (pre-commit gate) + PR #110 (e2e-test-unit). Syncs from slime: opened/reopened trigger types, --pull=always, pip deps (requests ray safetensors), cpu-unittest rename. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * docs(docker): translate Chinese to English in README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix(ci): stub vllm_router in plugin_contracts for cpu-unittest Mirrors slime's with_sglang_router stub pattern: add with_vllm_router kwarg to install_stubs() and pass it from test_plugin_generate_contracts. vllm_rollout.py imports vllm_router at module level; without the stub the cpu-unittest job fails with ModuleNotFoundError. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix(ci): add missing with_vllm_router=True stub to remaining plugin contracts test_plugin_rollout_contracts and test_plugin_path_loading_contracts both import vllm_rollout (which has bare `import vllm_router` at module level) but were not passing with_vllm_router=True to install_stubs — mirroring the same gap fixed in test_plugin_generate_contracts. Mirrors slime: all three tests pass with_sglang_router=True. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * docs(coding_agent_rl): align generate.py and README.md wording with slime - generate.py: drop backtick-wrapping around /inference/v1/generate in module docstring; collapse to one line matching slime style - README.md: condense rollout-max-*-len paragraph (remove verbose "sampling-params" / "generation length" verbiage, restore `max_tokens` inline like slime's `max_new_tokens` form) - README.md: trim vLLM response-structure detail (choices[0].token_ids / choices[0].logprobs.content[i].logprob) from token-out bullet — matches slime's abstraction level - README.md: add missing 3-line unit-test sentence after provenance paragraph (slime has it, vime was missing it) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * refactor(arguments): remove dist_ckpt_optim_fully_reshardable warning block Not present in slime; drop to maintain mirror parity. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * refactor(rollout): remove redundant assertions and finalization re-assignments on_policy_distillation.py: drop two assertions not present in slime (len(plp)==len(sample.tokens) and len(t_log_prob)>=response_length). vllm_streaming_rollout.py: drop 9-line finalization block that re-set sample.tokens/response/response_length/rollout_log_probs/loss_mask after the streaming loop had already written the same values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * style: apply black formatting to fix pre-commit CI Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> --------- Signed-off-by: aoshen02 <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

aoshen02 and others added 2 commits May 23, 2026 13:54

CalvinXKY reviewed May 24, 2026

View reviewed changes

CalvinXKY merged commit f9043c9 into main May 27, 2026
10 of 13 checks passed

aoshen02 deleted the ci/precommit-gate branch June 8, 2026 14:17

Navigation Menu

Uh oh!

ci: gate e2e jobs on pre-commit success - #26

ci: gate e2e jobs on pre-commit success#26
CalvinXKY merged 3 commits into
mainfrom
ci/precommit-gate

aoshen02 commented May 23, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot commented May 23, 2026

Uh oh!

CalvinXKY May 24, 2026

Uh oh!

aoshen02 May 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

aoshen02 commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this is not duplicating an existing PR

Code changes

Behavior

Test commands run

What I deliberately did NOT do

AI assistance disclosure

Uh oh!

gemini-code-assist Bot commented May 23, 2026

Uh oh!

CalvinXKY May 24, 2026

Choose a reason for hiding this comment

Uh oh!

aoshen02 May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aoshen02 commented May 23, 2026 •

edited

Loading

aoshen02 May 26, 2026 •

edited

Loading