Conversation
…3 runner Three defects, all the same shape: an instrument reporting success while measuring something else. 1. Duplicate YAML keys are invisible to every tool that does not raise on them. PyYAML resolves them last-wins, silently. A workflow that wires a parity gate under a key it already used parses cleanly, a grep for the intended module still succeeds, and the run executes the *other* copy. tests/validation/test_parity_workflow_wiring.py covers parity.yml; this makes the check whole-repo, since a duplicate key is a file-level class. 2. `pytest -k "..."` that matches nothing exits 0, so a step selecting a test that was renamed or deleted goes green having run nothing. Every -k/-m any workflow passes is now collected and must select at least one test. Vacuous (exit 5) and unverifiable (collection error) are reported differently, so a broken collection is never mistaken for vacuity. 3. `macos-13` is not in GitHub's public-runner label set, so `Wheels (macos-13)` can never be scheduled. Run 36526980634 has sat at 35/36 jobs complete with that one job queued since 2026-09-30. Replaced with `macos-15-intel`, which preserves x86_64 coverage, plus a guard test so a retired label cannot return unnoticed. tests/test_ci_contract.py converted from text search to parsing: the concurrency contracts and the job-body extractor now read the parsed tree, so a duplicate key can no longer make an assertion pass against a copy that never runs. Its deployment-target assertions moved to parsed values -- they were checking the author's quotes, not the target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
What this is
Three workflow defects, all the same shape: an instrument reporting success while measuring something else. Plus the conversion the first finding makes necessary.
cancel-in-progress: false-kmatching zero tests passes vacuously1. Duplicate YAML keys — the whole-repo class
PyYAML resolves duplicate mapping keys last-wins and says nothing. So a workflow that wires a parity gate under a key it already used parses cleanly,
grepfor the intended module still succeeds, and the run executes the other copy.tests/validation/test_parity_workflow_wiring.py(from #74) already coversparity.yml. This makes the class whole-repo, because it is a file-level property, not aparity.ymlproperty.Failing-first
A duplicate
run:key injected intoformal.yml, which silently replaces the Lean smoke command:Smoke.leannever runs.safe_loaddoes not complain.Files touched:
.github/workflows/ci.yml,.github/workflows/release.yml,tests/test_workflow_contract.py(new),tests/workflow_yaml.py(new),tests/test_ci_contract.py.parity.ymlis deliberately untouched.The existing suite was completely blind to it:
38 green tests, one silently disabled Lean semantic check.
The new check, same tree, same instant:
Tree restored and hash-verified afterwards (
2b0ca529…,git statusclean).2.
pytestselectors — the premise of this finding is wrongI was asked to check whether a
-k/-mmatching zero tests can passvacuously. It cannot. Measured, in the exact step form
ci.ymluses:pytest exits 5 (
no tests collected) on an empty selection, and GitHubfails any step with a non-zero exit. A renamed or deleted selector turns the
step red, not green. A missing target file likewise exits 4. pytest
already fails closed here, and I am not claiming a bug I did not find.
The risk that is real: silent narrowing
Exit codes only catch zero. They are silent when a selector still matches
something but far less than intended — a rename leaving 1 test where there
were 12, or a
-kclause that stops matching after a refactor. Nothing turnsred and the job's coverage quietly shrank. Same shape, one level down.
The check measures the count, so shrinking is visible at review time:
| Workflow | Selector | Collects |
| |---|---|
|
ci.ymlpython-integration|-k "atomizer"| 479 ||
ci.ymlpython-integration|-k "model"| 70 ||
parity.ymlnfsim-parity|-m "nf and not slow"| 6 ||
parity.ymlnfsim-parity|-k "test_nf_vs_native and simple_system"| 1 ||
parity.ymlnfsim-parity|-k "test_nf_fixed_seed_direct_matches_native_at_final_endpoint"| 1 |The last two legitimately select exactly one test, so a bare "≥1" floor adds
nothing for them; the value is that the number is now recorded rather than
inferred, and a rename dropping it to 0 is caught here with a named message
rather than by a step that fails much later, after building two oracles.
The check, and its own honest limit
It parses every workflow, extracts every
-k/-m, collects each, fails onzero. Injected bogus selector into
weekly.yml:My first version called anything with zero collected tests "VACUOUS". That
lied: a missing dependency produces a collection error with zero node ids,
and the check reported a defect it had not found. Fixed by keying on exit
code — exit 5 →
VACUOUS SELECTOR; exit 2/3/4 → `COULD NOT VERIFY …not evidence that the selector is vacuous**, which fails loudly without
inventing a finding.
test_the_selector_check_itself_is_not_vacuousasserts the extractor stillfinds ≥5 pytest invocations and ≥1 selector, so deleting every
-k/-mfromevery workflow turns this module loud instead of green.
3. The 2026-09-29 stall:
cancel-in-progress: falsedoes not explain itThe brief asked whether the
ci.yml:10-14/parity.yml:13-16change fullyexplains the queueing. It does not. Two independent causes, measured.
Cause A — a retired runner image (this is the 46-hour run)
macos-13is absent from GitHub's public-runner label set. The"Standard GitHub-hosted runners for public repositories" table lists
macos-15-intel/macos-26-intel(Intel) andmacos-14/macos-15/macos-26(arm64). There is no macOS 13.So
Wheels (macos-13)can never be scheduled, and it fails open — the runis created, every other job passes, and one job sits
queuedforever, so therun never reaches a conclusion.
The oldest queued run,
36526980634(CI,main, push,17e0eed):Its 35 other jobs all report
success, including everymacos-14job.Nothing else about that run is wrong.
Fixed by moving to
macos-15-intel, which keeps x86_64 macOS wheel coveragethat
macos-13was providing in intent only — it has not built a wheel inthe entire history of this workflow. A guard test now rejects any label
outside the documented set, so this cannot return unnoticed.
Cause B — throughput, which the concurrency change aggravates
cancel-in-progress: falseis not the cause. It is an aggravator: itstops stale pending runs being pruned, so the queue only grows.
The repo is public (
gh api repos/RuleWorld/BNG3→"private": false,"visibility": "public", noplanon the owner). The documented Free-planlimits:
Against that, measured:
total_count)needs two macOS waves against a cap of 5
~495 runs into the queue in ~80 minutes
On the specific question — does the job count exceed the limit? Not by
itself. 36 jobs per run is under 20 only in the sense that jobs run
concurrently; the limit binds on simultaneous jobs across all queued runs,
and with 501 runs queued and 2.4 h median duration, the queue is bounded by
throughput, not by any single run's size.
One correction to an earlier reading of mine, since it changes the conclusion:
"run status = queued" does not mean "no job started." A run stays
queueduntil every job finishes. All 40 queued runs I sampled had alreadystarted at least one job. The correct instrument is the job status, and at
job level the only genuinely stuck job in the entire backlog is the
macos-13one.4. Grep-class checks in
tests/converted to parsingtests/test_ci_contract.pyhad 7read_text()+inworkflow checks andimported no YAML at all. Converted:
test_pull_request_runs_keep_exact_head_evidence_availabletest_external_parity_workflow_is_present_and_keeps_exact_head_evidencetest_formal_workflow_runs_pinned_kernel_and_nfnext_contracts_workflow_job/_workflow_job_from— the regex that sliced a job body outof the file text, which every wheel/parity/release contract then asserted against
These were the worst instances:
test_pull_request_runs_keep_exact_head_evidence_availableasserts on
cancel-in-progress: false, and a duplicateconcurrency:key isprecisely the defect that would survive it.
Shared parsing lives in
tests/workflow_yaml.py(one strict loader, not asecond one). Substring assertions are kept — the haystack is now the parsed
job, so
assert "x" in jobreads executed configuration. Rendering is viarender_scalars, notyaml.safe_dump, because a dump re-folds and re-quoteslong shell commands and would force assertions onto formatting.
tests/test_ci_contract.py: 38 passed after conversion.One assertion was not testing behaviour at all
That asserts the author's quotes.
macos_deployment_target: 10.13is thesame deployment target. Now asserted by parsed value via
job_matrix_values(...). Same for theworkflow_dispatchtrigger and theneeds:list.5. Wiring
New step in
ci.yml::python-integration(notlint):lintinstalls onlyblack ruff pytest numpy, so it cannot collecttests/python/at all — running the selector check there would reportCOULD NOT VERIFYon every push, which is how a guard becomes noise and isthen ignored.
parity.ymlis not touched.TierParityExpandowns thetierp-direct-path-sweepjob there and this PR does not collide with it. Iverified their selector independently rather than assuming:
6. Finding 2 — the divergence marker is checked; the brief was wrong
I was told
_KNOWN_SEED_STATE_ORDER_DIVERGENCESis "checked by no CI job",because
parity.yml:151selects-m 'nf and not slow'and neither-kselector names it. That is not what the tree does.
parity.yml:154-162runs
tests/validation/test_parity_nfsim_seed.pywith no-kand no-mselector at all, and
test_seed_state_order_marker_list_is_empty_and_the_families_still_run(that file, line 499) carries no
@pytest.mark.slow, so it is not something-m "nf and not slow"would have excluded anyway. Collected, not read:What is genuinely unchecked in CI is the other direction: the
slowtier-P sweep that cross-validates the marker against measured divergences
(both unexpected and stale). The docstring at
test_parity_nfsim_seed.py:457-460overstates this as "nothing in CI currently checks that the marker is honest"
— line 499 does check emptiness. Correcting that sentence is
TierParityExpand's file, not mine.TierParityExpandowns thetierp-direct-path-sweepjob inparity.ymlandis closing the sweep gap there. This PR adds no job for the marker and does
not touch
parity.yml.not_claimed
tests/python/needs a builtbionetgen._bionetgen_cpp; on this machinetests/python/conftest.py:81raisesNameError(the fix(python): conftest's missing-build guard raised NameError instead of explaining #74 diagnostic bug) andcollection exits 4 →
COULD NOT VERIFY. The 479/70 counts above weremeasured with a throwaway
sys.modulesstub at/tmp/wf_stub.py, outsidethe repo, which supplies only the node IDs that
-kmatches on. Under thestub,
tests/python/test_thermodynamic_parse_path.pyerrors onModuleNotFoundError: _bionetgen_cpp— a stub artifact, not a repo defect;in CI the extension is built. The check is therefore verified against the
three
parity.ymlselectors, which collect with no build at all; the twoci.ymlselectors have a measured count (479, 70) but their check path isunexercised locally.
pytest already exits 5 there. The check's value is the count and the early
named failure, not a pass/fail repair.
macos-15-intelactually builds these wheels. Thelabel is documented as available to public repositories and the substitution
preserves the intended architecture, but no run has exercised it. First CI
run on this branch is the real test.
jobs, 5 macOS.
cancel-in-progress: trueon the SHA-keyed groups wouldprune stale runs, but it would also cancel the in-flight evidence these
concurrency groups exist to preserve, so I did not change it unilaterally.
This needs a decision, not a patch.
Fixing it means restructuring the matrix, which changes what is built.
Main's to run once after all lanes land.