Repository navigation
ci: an empty-shard assertion in the partition guard, and a bump-triggered uv pin check - #244
Merged
Merged
Conversation
The guard proved the shard selections partition the slow tier, which is not the same as proving every shard runs something. A selection that has emptied out disappears from both sides of that comparison at once: the union still equals the unsharded set, the shards are still pairwise disjoint, every ignored path is still named by somebody, and one runner installs this project for two minutes to run no tests. That case was reachable only by renaming a whole file while the catch-all picked it back up, and it was caught incidentally by pytest's exit code 5 in the shard's own test step. It stops being incidental the moment a shard names a test class rather than a file, which is the next change here: a class is one edit away from holding no slow tests -- every test in it gaining the `smoke` marker is enough -- and nothing about the partition would look wrong. So a fourth invariant, with its own error naming the shard and printing the selection that came back empty, and reported before the install and the model warm rather than behind a suite run. Two supporting changes the class case needs: Invariant 3 now compares file paths. A shard argument may be `<file>::<Class>` and the catch-all's ignore list can only name whole files, so the part before `::` is what the two lists have in common; without that reduction, naming one class of an ignored file would report the file as orphaned. Whether the file's other classes are claimed by anybody is invariant 1's question and it answers it exactly. A failed collection now names the shard. Selecting by class is what makes pytest's exit 4 reachable -- a class that has been renamed or removed is a usage error rather than an empty selection -- and the message should say which leg of the matrix to go and look at.
`UV_VERSION` was pinned because an unpinned installer in front of every gate fails on luck rather than on a diff. What the pin did not come with is anything that re-checks it. The proof that uv and pip resolve the same dependency set was produced once, by a throwaway job deleted after it reported, so all that survives of it is prose in a report and three numbers in a comment. A bump re-opens that question, and `--index-strategy` behavior is exactly the kind of thing a resolver release moves. So the retired job comes back, triggered by the bump itself. It builds each extras set twice in one runner -- pip as the independent reference, the pinned uv as the subject -- freezes both with the same pip, because `uv pip freeze` and `pip freeze` disagree about pip and setuptools themselves and that difference says nothing about the resolution, and fails with the differing lines. It also re-asserts the two properties the extra index exists for, a `+cpu` torch and no `nvidia_*` or `triton`, and on the no-aimnet leg asserts that aimnet is absent and unimportable on both sides rather than that the counts match: that leg stands for a job whose whole value is which package it ends up without, and matching counts would say nothing about which distribution left. Detection is on the pinned value read from both workflow files at the pull request's base and at its head, not on a path filter. These files are edited for every sort of reason, so a filter would fire three heavy installs on comment changes while still missing a pull request that touched only the other file. Reading both values also makes the cheap mistake visible for free: the two files are meant to be bumped together, and a pull request that moves one and not the other now fails instead of leaving a silent disagreement. Nothing runs on an ordinary pull request beyond the detection itself, and neither job is a required context. That is load-bearing for the heavy one: it is skipped almost always, and a skipped check reports no verdict, so as a required context it would leave branch protection waiting rather than merging. Both names are distinct from the five required ones, so publish.yml's reduction has nothing of theirs to collapse.
The version assertion added with the pin check could never pass: `uv --version` prints `uv 0.13.7 (x86_64-unknown-linux-gnu)`, so comparing the whole line against `uv <pin>` fails on the build target, which is not what is being pinned. Exercising the check with a real bump is what surfaced it -- the three legs failed before reaching the freeze comparison they exist for. Second field only, which is the version and nothing else.
…urations Two things, both about the matrix this lands on rather than the one it was written against. The empty-shard assertion and the file-path reduction in invariant 3 were written alongside a class-level split of the heaviest file. That split was measured and declined: it halved the binding leg's surplus over an even quarter of the tier, from about 30 s to 17 s, but seven runs put the gate at 179-198 s against a 165-181 s baseline and none beat the baseline's best, because the `Install` and model-warm steps -- identical work on every leg -- vary 10-15% between runs, which is more than the imbalance being removed. Class selections are structurally more fragile than whole-file ones, so that is not a trade worth making for a saving under the noise floor. The guard keeps both halves anyway, and the comments now say why instead of pointing at selections that are not there. Selecting a class is still the obvious next step if this tier ever needs balancing, a class is one edit away from holding no slow tests, and the invariant that catches it is cheap whether or not that day comes. The file-path reduction likewise costs nothing on whole-file selections and is what would stop a class selection from reporting its own file as orphaned. The tier also prints its slowest 25 tests per shard. It sets the run's critical path and nothing recorded where its time went, which is how a split came to be apportioned from a developer box -- 58/42 there against 46/54 on a runner. It also makes the single largest item visible without special effort: test_ani2xt_compiled_matches_eager_on_cpu is 33-43 s, about a tenth of the whole tier, which is a bigger lever than any scheduling change and is not one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The two points the CI optimization left open. The shard rebalance that was
explored alongside them was measured, found not to pay, and is not here —
see the last section.
The partition guard refuses a shard that collects nothing
assert_slow_shards_partition.pyproved the shard selections partition the slowtier, which is not the same as proving every shard runs something. A selection
that has emptied out leaves both sides of that comparison at once — the union
still equals the unsharded set, the shards are still disjoint, every ignored path
is still named — and a runner installs this project for two minutes to run
nothing.
A fourth invariant, with its own
::error::naming the shard and printing theselection that came back empty, reported in the same single pass as the others
and before the install and the model warm.
Why now, when every leg selects whole files. With whole-file selections the
case is reachable only by renaming a file the catch-all then picks back up, and
pytest's exit 5 catches it in the shard's own test step — late, and without
saying which selection emptied. The invariant is here ahead of the case that
makes it load-bearing: selecting a test class (
<file>::<Class>) is the obviousnext step if this tier ever needs balancing, and a class is one edit away from
holding no slow tests (every test in it gaining the
smokemarker is enough),with nothing about the partition looking wrong. The comments say exactly that.
Invariant 3 also now compares file paths, reducing a shard argument to the part
before
::. That costs nothing on whole-file selections and is what would stop afuture class selection from reporting its own file as orphaned. And a failed
collection now names the shard, since a selection naming something that no longer
exists is pytest's exit 4, not an empty selection.
The matrix is byte-identical to its state on the base commit — four legs,
whole-file selections, the same names — verified by parsing both and comparing,
not by reading the diff.
The slow tier reports its own test durations
It sets the whole run's critical path and nothing recorded where its time went,
which is how a balance claim came to be apportioned from a developer box (the
same file's classes divide 58/42 there against 46/54 on a runner).
--durations=25per shard also makes the single largest item visible without special effort:
test_ani2xt_compiled_matches_eager_on_cpuis 33-43 s, about a tenth of thewhole tier.
The uv pin is re-checked when it moves, and only then
UV_VERSIONwas pinned so a resolver release could not fail a build on luck, butnothing re-checked it: the proof that uv and pip resolve the same dependency set
was produced once by a throwaway job that was then deleted, leaving prose.
That job is recovered verbatim from its commit and triggered by the bump itself —
pip as the independent reference, the pinned uv as the subject, both frozen with
the same pip (because
uv pip freezeandpip freezedisagree about pip andsetuptools themselves), failing with the differing lines;
+cputorch and nonvidia_*ortritonre-asserted; and on theno-aimnetlegaimnet's absenceasserted on both sides rather than counts compared, since that job's whole value
is which package it ends up without.
Detection is on the pinned value read from both workflow files at the pull
request's base and at its head, not a path filter — these files change for every
sort of reason. It also catches, for free, a pull request that bumps one file and
not the other.
Exercised with a real bump, which caught a bug in it.
uv --versionprintsthe build target, so the exact-string version assertion could never have passed;
all three legs failed before reaching the freeze diff they exist for. Fixed to
compare the version field, re-run, and all three then proved uv 0.13.7 resolves
what pip does — 118 / 117 / 106 distributions, exactly the retired job's figures,
aimnetabsent and unimportable both ways. That bump is not carried here: thepin stays at 0.13.6.
Nothing runs on an ordinary pull request beyond the ~7 s detection. Neither job is
a required context, which is load-bearing for the heavy one: a skipped check
reports no verdict, so as a required context it would leave branch protection
waiting rather than merging. Confirmed — the skipped matrix reports
skipped.The shard rebalance was measured and dropped
Recorded because the measurement is worth more than the change would have been.
tests/test_pipeline_e2e.pywas one leg at 120 test-seconds and bound the gate,so balancing meant splitting inside it, by class. Done, it halved the binding
leg's surplus over an even quarter of the tier (29.9 s → 17.0 s, runner-speed
normalized). It did not move the gate: seven runs gave 179-198 s against a
165-181 s baseline and none beat the baseline's best 163 s, because the
Installand model-warm steps — identical work on every leg — vary 10-15% between runs,
more than the imbalance removed.
Also measured: a fifth leg would have saved a further ~20 s of leg load, and was
declined because that is inside the same variance and costs a permanent extra
runner on every pull request. And the structural cap —
tests/test_auto3D.pyhasno test classes at all, 21 module-level functions, so nothing short of node IDs
or a new marker divides its ~100 s.
Full numbers in
.superpowers/sdd/ci-rebalance-report.md.Unchanged
The slow-shards matrix, the aggregator name
slow tests (NNP integration)byte-for-byte and its treatment of
failure/cancelled/skippedas red,markers, schedules,
publish.yml,cache-keepalive.yml, the five required checknames, and
UV_VERSION. Collect-only totals still sum to 2,459; the slow union isstill 2,343 (7 + 21 + 19 + 2296).