Repository navigation
Conversation
…failing) - tests/unit/test_make_quality.py checks that make quality applies ruff's fixes and then runs make quality-check, which runs uv lock --check, ruff check, ruff format --check, Import Linter, mypy and the quality guards without changing files; that pr.yml's quality-guards job runs make quality and then git diff --exit-code; and that no other pr.yml job runs one of those checks itself. Its proof reads the new fixtures under rules/fixtures/api.quality.make-quality. - scripts/rules/tests/test_makefile.py checks reading a Makefile's recipes, following a target's prerequisites and the make commands in its recipe, and that a rule's CI invocation counts when a make target the job runs has it. - test_import_linter.py checks that a make target the job runs must run every contract too, without make's `-` prefix, which ignores failures. - test_locked_installs.py checks that a uv lock --check reached through a recursive $(MAKE) call counts. All fail: scripts/rules/makefile.py doesn't exist yet (ModuleNotFoundError for test_makefile.py and test_make_quality.py), the Import Linter guard accepts `lint-imports --config pyproject.toml || true` in a make target, and the locked-installs guard reports "no pull request job runs uv lock --check for ." when the check is in a target reached through $(MAKE). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- make quality applies ruff check --fix and ruff format, then runs make
quality-check: uv lock --check, ruff check, ruff format --check, Import
Linter, mypy and the quality guards, without changing files.
- pr.yml's quality-guards job ("Quality checks") runs make quality, then
git diff --exit-code, so a pull request must include ruff's fixes. The
lint and typecheck-v2 jobs fold into it and leave CI complete's needs.
push.yml is unchanged: its release jobs wait for its lint job.
- The rules registry reads the commands of the make targets a job runs
(new scripts/rules/makefile.py), following prerequisites and $(MAKE)
calls, so the 7 rules on pr.yml's lint job and the mypy rule move to
quality-guards with the same invocations, and each still fails if its
command leaves the target. The Import Linter guard checks the exact
lint-imports command there too, and the locked-installs guard follows
$(MAKE) to find uv lock --check.
- New rule api.quality.make-quality, with its proof fixtures.
- AGENTS.md, the GitHub PRs and testing skills and README.md say to run
make quality before committing; the rules-registry skill describes
invocations in make targets.
The tests that failed now pass, and make quality passes on this branch.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4203 +/- ##
===========================================
- Coverage 92.11% 52.27% -39.85%
===========================================
Files 297 297
Lines 16673 16673
Branches 2719 2719
===========================================
- Hits 15359 8715 -6644
- Misses 798 7386 +6588
- Partials 516 572 +56
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Review round 1 of #4203 found ways a check can pass while failing or not run, and that the guards no longer run on the standard library alone in pull-request CI. New and changed tests: - test_makefile.py: a later recipe replaces an earlier one, as make runs only the last; a target-specific variable isn't a prerequisite; makefile_problems reports .IGNORE, .ONESHELL and assignments to MAKEFLAGS, SHELL and .SHELLFLAGS; a make call with -n, -q, -t or -i runs no target; and no make target counts toward a rule's invocation when the Makefile can hide a failure. - test_make_quality.py: the quality guards run with python -S, outside the project's packages as push.yml runs them, and with GITHUB_BASE_REF defaulting to master so the baseline comparison runs locally too; and continue-on-error or an if: on the make quality step, continue-on-error on the job, .IGNORE in the Makefile, a check in a make target another job runs, and `pipx run ruff` are each reported. The proof fixtures use the new guards command. The imports are sorted (ruff I001). 13 test_makefile.py tests fail to import makefile_problems; in test_make_quality.py, the 6 new cases and the check of the repository's Makefile fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- scripts/rules/makefile.py: a later recipe replaces an earlier one, as make runs only the last; a target-specific variable is no longer read as prerequisites; makefile_problems reports .IGNORE, .ONESHELL and settings of MAKEFLAGS, SHELL or .SHELLFLAGS, and a Makefile with any of them gives its targets' commands no credit; a make call with -n, -q, -t or -i (or their long forms) runs no target. The registry's "does not run" message names the Makefile setting when that is why. - The quality guards in make quality-check run with python -S, outside site-packages, so pull-request CI still shows they need only the standard library, as push.yml runs them; and with GITHUB_BASE_REF defaulting to master, so the rules guard's baseline comparison runs locally too. - The rule's check also rejects continue-on-error on the job, an if: or continue-on-error on its make quality and git diff steps, a Makefile setting that hides failures, a check another job runs through a make target, and a tool name at the end of a command. The rule statement says so. - test_declared_dependencies.py's check that every job running the guards has uv on Python 3.12 follows make targets; it failed in CI run 42065704340, where the guards run only inside make quality. - The GitHub PRs skill lists how a local run can differ from CI's, and AGENTS.md and README.md no longer say they match exactly. The tests that failed now pass, and make quality passes on this branch without changing a file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ling) Review round 2 of #4203 found that a recipe after `;`, an include, a conditional, define, $(eval) or a GNUmakefile can make make quality skip its checks while the reader and the rule's test still see them; that some make calls run nothing although the reader credits them; that a local run compared baselines with origin/master's tip, not the commit the branch shares with it; and that the job's environment and steps around git diff went unchecked. - test_makefile.py: makefile_problems reports include, -include, ifdef, ifeq, define, $(eval and a recipe after ';'; root_makefile_problems reports a GNUmakefile or makefile; make_targets_run credits nothing to -C, -f and their long forms, abbreviated long options (--dry, --just, --ign), -v, --version, -h, --help, or a MAKEFLAGS prefix. - test_baseline_entries.py: default_base_ref uses RULES_BASE_REF as given, then origin/$GITHUB_BASE_REF. - test_make_quality.py: make -n quality, run with GITHUB_ACTIONS=true, prints the fixes and every check; the guards run with RULES_BASE_REF set to the merge base with origin/$GITHUB_BASE_REF (master locally); the job runs git diff --exit-code HEAD right after make quality; MAKEFLAGS in any env block, a shell override, a step between the two, `uv lock` in quality-check and a recipe after ';' are each reported. The fixtures use the new guards command and git diff step. - test_declared_dependencies.py also reads the make commands when it looks for the locked uv run. 22 subtests in 5 unittest functions, and 10 pytest cases, fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…merge base - scripts/rules/makefile.py reports include, conditionals, define, load, $(eval ...) and a recipe after ';' as problems, and a GNUmakefile or makefile beside the Makefile, which GNU make reads first. A Makefile with any of them gives its targets' commands no credit, so a check a target no longer runs can't still count. - A make call credits no target for -C, -f and their long forms (another Makefile), abbreviated long options GNU make accepts (--dry, --ign), -v, -h, --version, --help, or MAKEFLAGS set before it. - The rules guard takes RULES_BASE_REF, a revision used as given, before origin/$GITHUB_BASE_REF. make quality-check sets it to the merge base of HEAD and origin/<base branch> (master locally), so a local run compares a branch with where it left master instead of master's newer tip, and CI compares its merge with the commit that merge is built on. - pr.yml runs git diff --exit-code HEAD, so staged changes count, and its comment lists uv lock --check. - The rule's check runs make -n quality to confirm make itself runs the fixes and every check, and rejects MAKEFLAGS in the workflow's, job's or steps' env, a shell override, a step between make quality and git diff, and a relock or format in quality-check. - The GitHub PRs skill says which Makefile syntax to avoid and how the local comparison works; the rule statement says what it now checks. The tests that failed now pass, and make quality passes without changing a file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 3 of #4203 found more GNU make spellings that hide a failing check from the Makefile reader and from make -n: .IGNORE::, GNUMAKEFLAGS, MAKEFILES, computed names such as $(M)FLAGS, and a recipe defined only outside dry runs; more workflow settings around the make quality step; make calls the reader still credits; and a `-` prefix that the registry and the locked-installs guard still credit. - test_make_quality.py runs make quality for real with a stand-in uv on PATH: with every check passing, make must run the fixes and every check in order and exit 0; with any one failing, it must fail. test_make_quality_fails_whenever_a_check_fails does this for the repository's Makefile and passes; the tricks above must be rejected through the static check or this run. - The workflow cases add a default shell for the workflow or job, an earlier step writing to GITHUB_ENV, GNUMAKEFLAGS or MAKEFILES for the job, and another working directory for the step. - test_makefile.py: makefile_problems reports .IGNORE::, GNUMAKEFLAGS, MAKEFILES and computed names; make_targets_run credits nothing to GNUMAKEFLAGS= or MAKEFILES= before make, -o, --old-file, --eval, -E, SHELL= or MAKEFLAGS= arguments, or `echo make quality`; a recipe command with the - prefix gives no credit for a rule's invocation. - test_locked_installs.py: a uv lock --check under .IGNORE:, with the - prefix, or reached through make -n, doesn't count. 18 unittest subtests and 7 pytest cases fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he gaps - The rule now also runs make quality with a stand-in uv (in test_make_quality.py): make must run the fixes and every check in order when they pass, and fail when any one fails. That catches settings and syntax the Makefile reader doesn't recognize, and a recipe defined only outside dry runs, which make -n can't show. The rule's enforcer nodes add this test and the make -n test. - scripts/rules/makefile.py reports .IGNORE:: and .ONESHELL::, settings of GNUMAKEFLAGS and MAKEFILES, and computed target or variable names. A make call credits no target for -o, --old-file, --eval or -E, for a variable that can hide failures set before make or as an argument, or when make is an argument rather than the command (echo make quality). - The rules registry gives no credit to a recipe command with the - prefix, whose failure make ignores. The locked-installs guard follows make calls through the same reader, follows none in a Makefile with a problem, and doesn't count a uv lock --check with the - prefix. - The rule's check rejects a default shell for the workflow or job, an earlier step writing to GITHUB_ENV or GITHUB_PATH, GNUMAKEFLAGS or MAKEFILES in an env block, and another working directory for the steps. - The rule statement says no other job's commands run lint-imports (the test job runs Import Linter through a pytest test); the GitHub PRs skill lists the Makefile syntax to avoid, shallow clones and leftover packages in .venv; the rules-registry skill compares with the merge base. The tests that failed now pass, and make quality passes without changing a file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in #4183's suppressions guard and the rest of master. The generated docs/generated/rules.md and the Rules block in AGENTS.md were rendered again from the merged registry, keeping this branch's make quality instruction in AGENTS.md, which master doesn't change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in #4188's asserting-tests guard and the rest of master. The generated docs/generated/rules.md and the Rules block in AGENTS.md were rendered again from the merged registry, keeping this branch's make quality instruction in AGENTS.md, which master doesn't change. make quality passes on the merge, with the asserting-tests guard reading this branch's tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 5, 2026
Brings in #4196's Codecov flags, #4200's diff-coverage job and #4205's v2 file-layer exceptions. pr.yml merged cleanly: CI complete needs every job, including diff-coverage and the coverage uploads, and the quality-guards job still runs make quality, which runs #4205's v2-layers guard. No rule refers to the removed lint or typecheck-v2 jobs. docs/generated/rules.md and the Rules block in AGENTS.md were rendered again, keeping this branch's make quality instruction in AGENTS.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Nov 5, 2026
Brings in master (#4196, #4200, #4205) through #4203's branch. The generated docs/generated/rules.md and the Rules block in AGENTS.md were rendered again, keeping this branch's make install and hook instructions in AGENTS.md, which #4203's branch doesn't change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch had an error being deployed
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.
Fixes #4202
Summary
make qualityapplies ruff's fixes (ruff check --fix ., thenruff format .), then runsmake quality-check.make quality-checkruns every check without changing files:uv lock --check,ruff check .,ruff format --check .,lint-imports --config pyproject.toml,mypy, andscripts/run_quality_guards.py. Everything except the lock check runs throughuv run --frozen --extra dev.python -S, without site-packages. Pull-request CI then still shows that they need only the standard library, which is howpush.ymlruns them.RULES_BASE_REFset togit merge-base HEAD origin/<base>, where the base is CI'sGITHUB_BASE_REF, ormasterlocally. The rules guard now takesRULES_BASE_REF, a revision used as given, beforeorigin/$GITHUB_BASE_REF.master, instead of being skipped, or wrongly compared withmaster's newer tip.GUARDS.pr.yml: thequality-guardsjob, now named "Quality checks", runsmake quality, thengit diff --exit-code HEAD(HEAD, so staged changes count too). A pull request whose code ruff would fix fails there, and the log shows the fix to commit.lintandtypecheck-v2jobs fold into it and leaveCI complete'sneeds. Its other steps are unchanged: the registry tool's unittest run, and the shellcheck install the ci-scripts guard needs.The rules registry follows
maketargets. Newscripts/rules/makefile.pyreads the Makefile's recipes and follows each target's prerequisites and themakeor$(MAKE)calls in its recipe.[[rule.ci]]invocation now counts when it is in a command of a target the job runs. The 7 rules onpr.yml'slintjob and the mypy rule move toquality-guardswith their invocations unchanged (ruff check .,ruff format --check,mypy). Each still fails the registry check if its command leaves the target.lint-importscommand in those recipes too. Make's-prefix, which ignores a command's failure, doesn't match..IGNOREor.ONESHELL(with one colon or two),MAKEFLAGS,GNUMAKEFLAGS,MAKEFILES,SHELLor.SHELLFLAGS;include, a conditional,define,load,$(eval ...), a recipe after;, or a computed target or variable name;GNUmakefileormakefilebeside it, which GNU make reads first.makecall credits no target when:-n,-q,-t,-i,-o,-v,-h, their long forms and accepted abbreviations;-C,-f,-E,--eval;makeis an argument, not the command (echo make quality).-prefix: a recipe command with it, whose failure make ignores, gives no credit to a rule's invocation or to the locked-installs guard'suv lock --check. That guard now followsmakecalls through the same reader, and follows none in a Makefile with a problem.$(MAKE)as well asmakewhen it looks foruv lock --check.New enforced rule
api.quality.make-quality. Its tests intests/unit/test_make_quality.pycheck:make quality, run for real with a stand-inuvonPATH, runs the fixes and every check in order when they pass, and fails whenever any one of them fails. This catches what the reader doesn't recognize:.IGNORE::,GNUMAKEFLAGS, computed names, or a recipe defined only outside dry runs;make -n quality, run withGITHUB_ACTIONS=true, prints the same commands;git diff --exit-code HEADcomes right aftermake quality;if:), allowed to fail (continue-on-error), or given its ownshell:orworking-directory:;env:block for them setsMAKEFLAGS,GNUMAKEFLAGSorMAKEFILES, and no earlier step writes toGITHUB_ENVorGITHUB_PATH;quality-checkneither fixes, formats nor relocks;pr.ymljob runs ruff, mypy, Import Linter or the quality guards, itself or through amaketarget.The proof uses fixtures under
rules/fixtures/api.quality.make-quality.test_declared_dependencies.pychecks that every job running the guards has uv on Python 3.12. It now followsmaketargets: it failed in CI run 42065704340, where the guards run only insidemake quality.Docs:
AGENTS.md, the GitHub PRs skill ("Commit Hygiene", which the rule cites), the testing skill andREADME.mdsay to runmake qualitybefore committing.Design choices for review
CI completeshouldn't finish later.quality-guards. Onmaster, 23 rules (24[[rule.ci]]references) and 16 rule proofs referencepr.yml:quality-guards, and Check that every noqa and type: ignore comment names a code and gives a reason #4183 and Check that every test function asserts something #4188 add more. Renaming the job would break their rules once they merge. Only the display name changes.make qualitystops at the first failing check, where the three jobs used to fail independently. The merge is blocked either way, but a pull request with several kinds of problem sees them over several runs.typecheck-v2job set noUV_PYTHON, so it ran on the Python 3.14 that.python-versionselects, with uv 0.13.7. It now runs on 3.12 with uv 0.13.4.[tool.mypy] python_version = "3.11"still decides the language version it checks against.quality-guardsjob's id stays. Check that every noqa and type: ignore comment names a code and gives a reason #4183, now merged, adds a rule onpr.yml:quality-guardstoo.make quality. They are a whole test suite, which agents in this workstream don't run locally, so only the CI job runs them.make quality. It needs every test shard's coverage report, so Require 80% coverage on the lines a pull request changes, with diff-cover #4200 runs it in apr.ymljob of its own.push.ymlis unchanged. Itsversioningandpublish-git-tagjobs wait for itslintjob, and.github/release_commit.pynames "Lint" and "Quality guards" as merge-run checks. Moving them would change what a release waits for, which is a separate decision.uv lock --checkuses the local uv. CI pins uv 0.13.4. A different local uv could disagree aboutuv.lock; uv 0.12.23 agrees on this branch.scripts/rules/, whichapi.process.rule-machinery-code-owned(doc-only) asks a core developer other than the author to approve.CI wiring and environment variables
The
quality-guardsjob:UV_PYTHON"3.12"in the job'senv:, as before. The Makefile also sets it if unset.ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSIONenv:, the literaltrue; not used.OLD_SIMULATION_GATEWAY_URLenv:, the repository Actions variable; not used.GITHUB_BASE_REFpull_requestevents to the base branch.make quality-checkusesmasterwhen it is unset or empty.RULES_BASE_REFmake quality-checkrecipe, for that one command, togit merge-base HEAD origin/<base>, and read byscripts/rules/baseline.py. If git can't find the merge base, it is empty and the guard falls back toorigin/$GITHUB_BASE_REF. The CI log doesn't show its value.GITHUB_ACTIONSactions/checkoutandastral-sh/setup-uvuse the automaticgithub.token.makeis onubuntu-latest. The job installs uv 0.13.4 and shellcheck as before.uv runsyncs.venvfromuv.lock.lintandtypecheck-v2jobs read no variable or secret either.Testing
Red, at
b10bd5120(tests and fixtures only):tests/unit/test_make_quality.pyandscripts/rules/tests/test_makefile.py:ModuleNotFoundErrorforscripts.rules.makefile.test_import_linter...test_a_make_target_the_job_runs_must_run_every_contract_too:lint-imports --config pyproject.toml || truein a make target accepted.test_locked_installs...test_a_lock_check_in_a_recursive_make_target_counts: "no pull request job runs uv lock --check for .".Green, at
4dc657fdc: the same 12 tests pass, by node ID (3 pytest, 9 unittest). This commit's CI run failed on two things that the first review round found:test_every_job_running_the_guards_has_uv_on_python_3_12didn't look inside make targets.tests/unit/test_make_quality.py's import order failed ruff's I001. A ruff cache had hidden that locally, andmake qualitywould have fixed it, so the claim that it changed no file was wrong at that commit.Review round 1, red at
b6bf00214(tests and fixtures only):test_makefile.pytests failed to importmakefile_problems.test_make_quality.py, the 6 new cases (stepcontinue-on-error, step condition, jobcontinue-on-error,.IGNORE, a check in another job's make target,pipx run ruff) failed, and so did the check of the repository's Makefile, which didn't yet run the guards withpython -Sand a base branch.Green at
916ef62c3: those tests, the changedtest_declared_dependenciestest and the rest of this PR's tests pass. That is 16 unittest IDs and 4 pytest IDs (9 cases).Review round 2, red at
eb12b1bdd(tests and fixtures only): 22 subtests in 5 unittest functions and 10 pytest cases failed. These covered:GNUmakefile;RULES_BASE_REF;make -n quality;MAKEFLAGSinenv:, ashell:override, a step betweenmake qualityandgit diff, and a relock.Green at
46fc1191b: all 21 unittest IDs and 5 pytest functions (17 cases) pass.Review round 3, red at
c84108df1(tests only): 18 unittest subtests and 7 pytest cases failed. They cover:.IGNORE::,GNUMAKEFLAGS,MAKEFILESand computed names;makecalls that should credit nothing;--prefixed commands for the registry and the locked-installs guard;GITHUB_ENV,GNUMAKEFLAGS,MAKEFILESandworking-directory;GNUMAKEFLAGSfor the rule as a whole.The run of
make qualitywith a stand-inuvalready passed for the repository's Makefile, and it reports.IGNORE::, computedMAKEFLAGSand the recipe defined only outside dry runs. Green atb76e7acdf: all 23 unittest IDs and 7 pytest functions (28 cases) pass.Then
master(with #4183) was merged in at194bd0006. The generated files were rendered again, keeping this branch'sAGENTS.mdparagraph. The same tests andmake qualitypass there.master(with #4188, then #4196, #4200 and #4205) was merged in again, at9ad5c4416and30a97a067.pr.yml,CI completeneeds every job, including Require 80% coverage on the lines a pull request changes, with diff-cover #4200'sdiff-coverageand Measure the unit tests' coverage on master and flag every Codecov upload #4196's coverage uploads.lintortypecheck-v2jobs.make qualityruns Make v2 file-layer exceptions expire and cover whole files #4205's updated v2-layers guard.diff-coverageruns onlydiff-cover.make qualitypass at30a97a067.Also run locally at
194bd0006:make quality: every step passes, and it changed no file (ruff run with--no-cachetoo);python -m scripts.rules guard --base-ref origin/masterandpython -m scripts.rules validate: pass;Per this workstream's rules, only the new tests were run locally; CI runs the rest.
🤖 Generated with Claude Code