Skip to content

Add drift guards for the generated expression surface - #601

Open
eriknw wants to merge 4 commits into
20-networkx-ingestionfrom
21-autogenerate-drift-guard
Open

Add drift guards for the generated expression surface#601
eriknw wants to merge 4 commits into
20-networkx-ingestionfrom
21-autogenerate-drift-guard

Conversation

@eriknw

@eriknw eriknw commented Aug 4, 2026

Copy link
Copy Markdown
Member

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.

  • test_automethods.py (0cf95e5): every public attribute of
    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.
  • Infix coverage (43d78e6): coverage is the intersection across plain,
    index, and infix expression classes rather than the union, so a name
    missing only from the infix classes is caught too.
  • autogenerate.py --check and 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; --check guards generated file content against the
generator.


Stack created with GitHub Stacks CLIGive Feedback 💬

@eriknw eriknw changed the title 21 autogenerate drift guard Add drift guards for the generated expression surface Aug 4, 2026
@eriknw
eriknw marked this pull request as ready for review August 4, 2026 16:07
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch from 6ead79d to 2968622 Compare August 4, 2026 16:12
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch from 2968622 to 4405133 Compare August 5, 2026 00:06
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch 2 times, most recently from 3a77564 to d202125 Compare August 5, 2026 17:44
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch from d202125 to 7edb8b1 Compare August 5, 2026 18:03
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch 2 times, most recently from 4183028 to c11cd07 Compare August 6, 2026 07:59
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch from c11cd07 to c3933ff Compare August 6, 2026 15:39
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch from c3933ff to 76c9d16 Compare August 6, 2026 15:41
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch 2 times, most recently from 2450829 to 8e098e0 Compare August 6, 2026 20:41
@eriknw
eriknw force-pushed the 21-autogenerate-drift-guard branch from 8e098e0 to 8788937 Compare August 7, 2026 02:48
eriknw added 4 commits August 7, 2026 00:09
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
eriknw force-pushed the 21-autogenerate-drift-guard branch from 8788937 to 32893f7 Compare August 7, 2026 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant