Add drift guards for the generated expression surface - #601
Open
eriknw wants to merge 4 commits into
Open
Conversation
eriknw
marked this pull request as ready for review
August 4, 2026 16:07
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
from
August 4, 2026 16:12
6ead79d to
2968622
Compare
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
from
August 5, 2026 00:06
2968622 to
4405133
Compare
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
2 times, most recently
from
August 5, 2026 17:44
3a77564 to
d202125
Compare
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
from
August 5, 2026 18:03
d202125 to
7edb8b1
Compare
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
2 times, most recently
from
August 6, 2026 07:59
4183028 to
c11cd07
Compare
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
from
August 6, 2026 15:39
c11cd07 to
c3933ff
Compare
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
from
August 6, 2026 15:41
c3933ff to
76c9d16
Compare
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
2 times, most recently
from
August 6, 2026 20:41
2450829 to
8e098e0
Compare
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
from
August 7, 2026 02:48
8e098e0 to
8788937
Compare
Adding a method to Matrix/Vector/Scalar without updating the automethods name sets left the expression classes silently missing it, and no test failed. The new introspection test closes that: - Forward: every public attribute (and value-forwarding dunder) of Scalar/Vector/Matrix/TransposedMatrix is either auto-computed on the expression classes or listed in an explicit OPT_OUT table with a reason (52 entries: mutators, constructors, native metadata, storage flags). - Reverse: every generated name still exists on the concrete type. - Hygiene: OPT_OUT entries must be live and not redundantly covered, so the table cannot rot. Coverage is derived at runtime from what the generator emitted (getter __module__ is graphblas.core.automethods), not from a second copy of the name sets, so reorganizing the sets does not break the test. Failure messages name the attribute and point to the sets and scripts/autogenerate.py. Teeth: adding a fake public method to Vector makes the test fail with that message.
core/infixmethods.py copies the same generated automethods surface onto the infix expression classes (VectorInfixExpr and friends, minus the private _get_value), so a name missing only from the infix classes was a gap test_automethods.py could not see: coverage was the UNION across expression classes, so a name present on the plain expression class masked its absence elsewhere. Coverage is now the INTERSECTION across plain, index, and infix classes (identical sets today: Scalar 42/42/41, Vector 49/49/48, Matrix 58/58/57, differing only by _get_value). Teeth: hiding to_coo on MatrixInfixExpr alone makes the Matrix forward test fail, naming the attribute, all three expression classes, and scripts/autogenerate.py.
test_automethods.py guards the NAMES of the generated expression surface but not the generated file content: hand-edit a block between the auto-generated code markers, or edit the name sets in automethods._main without rerunning the generator, and the on-disk blocks go stale while CI stays green. scripts/autogenerate.py --check now regenerates every generated file into a scratch tree and compares, naming the files that drifted. The generator mains gained internal base-dir and callblack parameters to support this. The two guards are easy to confuse, so the scope is worth stating. The generator emits from the literal name sets in automethods._main rather than by introspecting the classes, so adding a method to Matrix does not change its output and cannot surface here; test_automethods.py covers that side. This covers the other one, generated files that no longer match the generator claiming to produce them. The check compares parsed syntax, not bytes. The generators shell out to black when it is on PATH and skip it when it is not, so a byte comparison reports drift on automethods.py and infixmethods.py in every environment lacking black, and black is in no test extra. Guarding that with skipif(black is None) meant the test never ran in the pytest_normal CI jobs, which is exactly where it needs teeth. Comparing ASTs makes the result independent of whether black is installed, so the check runs everywhere, and layout is already enforced repo-wide by black in pre-commit and the lint job. The residual gap is a layout-only edit inside a generated block, which black --check catches and this does not. The script also puts its own repo root on sys.path before importing graphblas. For a script sys.path[0] is the script's own directory, so a bare import resolves to whatever is installed; that coincides with the checkout in an ordinary dev setup and diverges in a git worktree, where --check would validate a tree nobody asked about while reporting green. The check now prints the package it validated and the test asserts on that line rather than inferring correctness from an exit code. Scratch files go to a TemporaryDirectory outside the repo. Comparing parsed syntax removed the reason to keep them inside it (letting black discover the project pyproject.toml), and an interrupted run no longer leaves .autogen_check_* directories in the working tree.
eriknw
force-pushed
the
21-autogenerate-drift-guard
branch
from
August 7, 2026 05:09
8788937 to
32893f7
Compare
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.
Nothing failed when the generated expression surface drifted: a method added
to Matrix/Vector/Scalar without updating the automethods name sets, or a
hand-edit inside a generated block, sailed through CI. Two complementary
guards close this.
Scalar/Vector/Matrix/TransposedMatrix must be auto-computed on the
expression classes or listed in an explicit OPT_OUT table with a reason;
every generated name must still exist on the concrete type; OPT_OUT
entries must stay live so the table cannot rot.
index, and infix expression classes rather than the union, so a name
missing only from the infix classes is caught too.
autogenerate.py --checkand a drift-guard test (6ead79d):regenerates every generated file into a scratch tree and compares parsed
ASTs (so the result does not depend on whether black is installed),
naming the files that drifted. The script pins sys.path to its own repo
root and prints which package it validated, so a worktree cannot
green-light the wrong checkout.
The two guards cover different failure modes: the test guards the name sets
against the classes;
--checkguards generated file content against thegenerator.
Stack created with GitHub Stacks CLI • Give Feedback 💬