Fix four failing CI jobs, one of which could not fail - #17
Merged
Conversation
Master has been red since 19 August. Three of the four failures were one line. render/chunked.py imported NumPy at module scope and render/__init__ imports it eagerly, so every renderer required NumPy -- including the pure-stdlib MIDI writer. `plainsong compile song.song -o out.mid`, with no audio in it anywhere, failed on a machine without NumPy. That is the project's central rule broken in exactly the direction it exists to prevent. The import now sits in the two functions that use it, with a comment saying why it may not move back. Released versions are unaffected: this landed after v1.4.0 was tagged. src/genome.py defines a classmethod named `random`, which shadows the module for the remainder of the class body -- so the next `rng: random.Random` annotation resolved to the classmethod and raised at import, taking the whole pytest run down. Deferred annotations fix it without renaming the method. tests/test_chunked.py imported NumPy and pytest at module scope. The stdlib-only job has neither and runs `unittest discover`, which imports every test module, so an absent optional dependency became a collection error instead of a skip. And the fourth: `ruff format --check` carried `continue-on-error: true` and had never once passed, reporting 60 unformatted files and exit 1 on every run while the job reported success. A check that cannot fail is decoration -- the first rule in this repository's own docs/verification.md, in this repository's own CI. The suppression is removed, the 63 files are formatted, and ruff is pinned: a formatter that gates and also upgrades itself turns the build red with nobody having touched the code. Verified: 697 tests, 7 specs with and without NumPy, ruff check and format clean, the wheel stage of verify_release 10/10, and the corpus fingerprint unmoved -- 6,321 files compile to exactly the music they did before, which is what makes a 63-file reformat safe to believe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBAjxy7cD6DzJ72NX8TJEc
The stdlib-only job installs nothing and runs `unittest discover`, which imports every test module. tests/test_genome.py and tests/test_groove_tracker.py import pytest at module scope, so all twelve platform jobs died on the import -- 699 tests ran and the run still failed on two ImportErrors. Between them they used pytest three times: `pytest.raises` twice and `pytest.approx` once. All three have exact stdlib equivalents, so the dependency buys nothing and the import is gone rather than guarded. `_assert` sits below the import block because ruff permits a sys.path hack ahead of imports and not an assignment. Found the same way as the last round, and worth saying plainly: this sandbox has pytest installed, so the local suite passed while CI could not. Both fixes in this branch were invisible to a local run for the same reason -- the environment that fails is the one nobody develops in. There is now a scratch harness that blocks numpy and pytest at the import hook and runs discovery the way that job does, which is what should have been used before the first push. Verified in each of CI's environments rather than in one: stdlib-only 696 tests 0 errors, pytest job clean, specs 7/7 with and without numpy, wheel stage 10/10, ruff check and format clean, and the corpus fingerprint unmoved at 6,321 files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PBAjxy7cD6DzJ72NX8TJEc
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.
CI on master has been red since 19 August — every run since
f5b8686. Three of the four failures trace to one line.render/chunked.pyimported NumPy at module scoperender/__init__.pyimportschunkedeagerly, so every renderer required NumPy — including the pure-stdlib MIDI writer. This is not just a CI problem:The project's most load-bearing rule, broken in exactly the direction it exists to prevent. The import now sits in the two functions that use it, with a comment explaining why it must not move back, and
TYPE_CHECKINGkeeps the annotations resolvable without importing anything at runtime.No released version is affected — this landed after
v1.4.0was tagged, verified withgit cat-file -e v1.4.0:plainsong/render/chunked.py. PyPI is clean.src/genome.pyshadowed therandommoduleDefining a classmethod named
randombinds that name in the class body, so the next annotation resolved to the classmethod:AttributeError: 'classmethod' object has no attribute 'Random', at import, taking the entire pytest run down as a collection error.from __future__ import annotationsfixes it without renaming the public method.tests/test_chunked.pyturned a missing optional dependency into a collection errorIt imported both NumPy and pytest at module scope. The stdlib-only job has neither and runs
unittest discover, which imports every test module — so an absent optional dependency killed the run instead of skipping. Nowunittest.SkipTest, understood by both runners and requiring neither package. Nothing in that file calls NumPy directly, so it asks whether it is installed rather than binding a name it never uses.The fourth is the one worth reading
It had never passed. Reading the Aug 18 log — a run the API reports as
success:Exit 1 on every run since it was written, green every time. A check that cannot fail is decoration — which is the first rule in
docs/verification.md, in this repository, in this repository's own CI. It is the same fault as the|| truethis session removed fromcuda-constraint-engine, found by chasing why a green tick disagreed with a local run.The suppression is gone, the 63 files are formatted, and ruff is pinned — a formatter that gates and upgrades itself turns the build red with nobody having touched the code. Bumping it is now a deliberate commit that carries its reformatting with it.
Verified
ruff check/format --checkverify_release.py --stage wheelThat last line is what makes a 63-file reformat safe to believe rather than merely plausible.
Note on how this was found
I recommended merging the previous PR on the strength of a green local run without checking its CI, which was red. The local suite passed because that branch predated
chunked.pyentirely, so it never met the code that was broken — a clean illustration of why "it passed here" is not evidence. Checking the logs rather than the status is what turned up thecontinue-on-error.Generated by Claude Code