Run six pinned-symbol jobs on one runner instead of six - #9385
Conversation
Version Compat CI spent six runner slots on 287s of combined work. On a sampled main
commit each of the six waited 23 to 31 minutes to be admitted, in order to execute for
between 15 and 128 seconds. The queue is the cost, so the slots are the target.
Why these six can share a runner
------------------------------------------------------------------------
Not a general claim about the suite, a specific one about these tests: they fetch raw
source from raw.githubusercontent.com and grep it for symbols (tests/version_compat/
_fetch.py, has_def / first_match). Nothing is pip-installed, which is the whole point of
the canary and is also what makes co-location safe -- there is no venv for "transformers
4.57.6" and "transformers main" to fight over, because neither is installed.
The repo had already proved the shape: `daily-fresh-fetch` runs all of these in one
process on cron. This makes the PR path do the same thing, in parallel.
Measured on the full set, locally:
serial 1606 passed, 182 skipped 182.8s
-n 4 1606 passed, 182 skipped 27.8s
-n 8 1606 passed, 182 skipped 75.2s
The pass counts are identical in all three, which is the number that matters: the bundle
tests exactly what the six jobs tested.
-n 4 rather than higher. -n 8 is 2.7x SLOWER, which for a fetch-bound suite is upstream
throttling and not core contention -- the job's own comment already notes the 5000/h
authenticated raw-fetch ceiling. 4 also matches the hosted runner's core count and the
-n 4 every other pytest job here uses. --dist loadfile keeps a suite on one worker.
What was deliberately NOT folded in
------------------------------------------------------------------------
zoo-imports-under-spoof and grpo-fake-run stay separate. Both install a torch stack, and
they install mutually exclusive TRL pins ('trl>=0.22,<0.26' against `pip install --upgrade
trl`), so one venv cannot hold both. A guard asserts they still exist, because folding
them into a job with no torch at all is the plausible next step and it would not fail
loudly.
Two pre-existing gaps this found
------------------------------------------------------------------------
The new guard derives coverage from the filesystem rather than from a list, and on its
first run it reported two suites that no pull_request job runs at all:
test_import_leaves_torch_globals_alone.py `import torch` inside a subprocess probe
test_trl_vllm_generation_lora_patch.py needs an installed TRL
Both pre-date this change and both are swept daily by the cron job, so they are checked
daily rather than never. Neither can join a bundle that installs only pytest. The second
belongs with grpo-fake-run, which already has that stack, but it is not in that job's list
either; it was left alone rather than wired in blind, because I could not verify it passes
in that environment first and an unverified suite would have put an unrelated red on the
change that noticed the gap. Recorded in CRON_ONLY with the reasons.
Check names
------------------------------------------------------------------------
Six checks become one. Removed:
vLLM pinned-symbol matrix (>= 0.9.0 + main)
TRL / GRPO pinned-symbol matrix
PEFT pinned-symbol matrix (pyproject window + main)
sentence-transformers pinned-symbol matrix
bitsandbytes pinned-symbol matrix
transformers pinned-symbol matrix (4.57.6 + 5.x + main)
Added:
pinned-symbol matrix (vLLM, TRL, PEFT, ST, bitsandbytes, transformers)
Branch protection naming any of the six needs updating.
Verified
------------------------------------------------------------------------
Mutation-tested, each failing exactly one guard: drop a suite from the bundle; remove
-n 4; remove --dist loadfile; sweep the directory instead of naming files.
tests/studio 4584 passed. lint_workflow_triggers OK across 41 files. No dangling
references to the six removed job ids anywhere in the tree.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd90a76a5a
ℹ️ 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".
| # rather than never. Anything added here needs the same kind of reason written down. | ||
| CRON_ONLY = { | ||
| "tests/version_compat/test_import_leaves_torch_globals_alone.py", | ||
| "tests/version_compat/test_trl_vllm_generation_lora_patch.py", |
There was a problem hiding this comment.
Run the cron-only TRL suite with its dependencies
The recorded exception does not actually receive the promised daily coverage: the checked daily-fresh-fetch job installs only pytest before sweeping tests/version_compat/, while test_trl_vllm_generation_lora_patch.py skips the entire module at collection when torch is unavailable. Repository-wide workflow search finds no other invocation of this suite, so its LoRA propagation checks never execute in CI and this exception makes the new coverage guard accept that gap indefinitely; add it to an install-bearing job or provide the required stack in the cron job.
Useful? React with 👍 / 👎.
|
Checked that the bundled job still runs everything the six jobs ran, rather than trusting the green tick. The job reports 1604 passed / 184 skipped in 93.96s on 4 workers. All eight suites are present in the log by name: Running the identical command locally gives 1606 passed / 182 skipped. Same 1788 collected either way, so nothing was dropped. Diffing the skipped test IDs rather than the counts, the two that differ are: Both skip when their package is not installed ( Removed the hardcoded 1606/182 from the guard docstring for that reason, and recorded the CI figure with the explanation, so the next reader does not chase the same discrepancy. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f230d8ed4
ℹ️ 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".
| # Both are swept by the cron-only `daily-fresh-fetch` job, so they are checked daily | ||
| # rather than never. Anything added here needs the same kind of reason written down. | ||
| CRON_ONLY = { | ||
| "tests/version_compat/test_import_leaves_torch_globals_alone.py", |
There was a problem hiding this comment.
Run the torch-global behavioral probe with torch installed
This exception does not provide the claimed daily coverage for the suite's primary behavioral check: daily-fresh-fetch installs only pytest (version-compat-ci.yml:361-369), while test_importing_the_cpu_modules_leaves_torch_globals_alone skips whenever torch is absent (test_import_leaves_torch_globals_alone.py:77-79). A repo-wide search at this commit finds no other invocation, so CI runs only the static AST check and never verifies that importing the guarded modules actually preserves torch globals; include this suite in an install-bearing job.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Honor step conditions when calculating PR coverage
Filter steps by their own if condition before counting their test paths. _named_paths currently scans every step unconditionally, and this workflow already has a scheduled/dispatch-only step at version-compat-ci.yml:317-334; if a suite is removed from the preceding PR step but remains in that scheduled step, test_every_suite_still_runs_on_a_pull_request still reports it as PR-covered, defeating the guard's stated purpose.
Useful? React with 👍 / 👎.

Version Compat CI spent six runner slots on 287s of combined work. On a sampled main commit each of the six waited 23 to 31 minutes to be admitted, in order to execute for 15 to 128 seconds. The queue is the cost, so the slots are the target.
Why these six can share a runner
Not a general claim about the suite, a specific one about these tests: they fetch raw source from
raw.githubusercontent.comand grep it for symbols (tests/version_compat/_fetch.py,has_def/first_match). Nothing is pip-installed — that is the whole point of the canary, and it is also what makes co-location safe: there is no venv for "transformers 4.57.6" and "transformers main" to fight over, because neither is installed.The repo had already proved the shape:
daily-fresh-fetchruns all of these in one process on cron. This makes the PR path do the same, in parallel.Measured
The pass counts are identical in all three, which is the number that matters: the bundle tests exactly what the six jobs tested.
-n 4, not higher.-n 8is 2.7x slower, which for a fetch-bound suite is upstream throttling rather than core contention — the job's own comment already notes the 5000/h authenticated raw-fetch ceiling. 4 also matches the hosted runner's core count and the-n 4every other pytest job here uses.--dist loadfilekeeps a suite on one worker.Deliberately NOT folded in
zoo-imports-under-spoofandgrpo-fake-runstay separate. Both install a torch stack, and they install mutually exclusive TRL pins ('trl>=0.22,<0.26'againstpip install --upgrade trl), so one venv cannot hold both. A guard asserts they still exist, because folding them into a job with no torch at all is the plausible next step and it would not fail loudly.Two pre-existing gaps this found
The new guard derives coverage from the filesystem rather than from a list, and on its first run reported two suites that no pull_request job runs at all:
test_import_leaves_torch_globals_alone.pyimport torchinside a subprocess probetest_trl_vllm_generation_lora_patch.pyBoth pre-date this change and both are swept daily by the cron job, so they are checked daily rather than never. The second belongs with
grpo-fake-run, which already has that stack, but is not in that job's list either. I left it alone rather than wiring it in blind: I could not verify it passes in that environment first, and an unverified suite would have put an unrelated red on the change that noticed the gap. Recorded inCRON_ONLYwith the reasons.Check names
Six checks become one. Branch protection naming any of the six needs updating.
Added:
pinned-symbol matrix (vLLM, TRL, PEFT, ST, bitsandbytes, transformers)Verified
-n 4; remove--dist loadfile; sweep the directory instead of naming files.tests/studio4584 passed.lint_workflow_triggers.pyOK across 41 files.