From b14c2e729b23851a0468a4320d075080f0080cb1 Mon Sep 17 00:00:00 2001 From: statxc <181730535+statxc@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:40:53 +0200 Subject: [PATCH] fix(docs): declare frac in strategy README's 'Register your own' basis() hook Fixes #274. multiply_subspace forwards the run's vram_fraction only to a basis() that declares frac (an inspect.signature gate whose else-branch is a legacy shim), so a contributor who copies a snippet without it lands in the legacy branch and silently streams at the default fraction instead of --vram-fraction -- the bug #211 fixed for rsvd. CONTRIBUTING.md, the template and run_example.py were all brought in line, but the README's 'Register your own (the updatable hook)' -- the copy a contributor meets first -- still showed the old signature. It survived because the guard test added for exactly this never covered strategy/README.md: its docstring enumerated only 'the two remaining copies'. Fix the snippet and close the hole: the markdown copies now run through one parametrized table, so a rollback or a newly added doc cannot quietly skip the check. Also assert the FULL parameter list against Transform.basis (defaults stripped), not just the presence of frac, so a snippet cannot drift on any other argument either. Credit: #275 (luciferlive112116) reported the same gap; it was closed unmerged by the 12h merge-conflict rule after #253 landed the test file underneath it. --- strategy/README.md | 7 ++- .../test_contributor_docs_basis_signature.py | 53 ++++++++++++------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/strategy/README.md b/strategy/README.md index d48a448..57d6c43 100644 --- a/strategy/README.md +++ b/strategy/README.md @@ -51,9 +51,12 @@ from strategy import Transform, register_transform, subspace_matmul, Config class MyTransform(Transform): name = "mine" - def basis(self, n, m, backend, dtype, A=None, B=None): + def basis(self, n, m, backend, dtype, A=None, B=None, frac=None): Q = ... # (n, m), ORTHONORMAL columns, on backend.xp - return Q + return Q # frac = the run's --vram-fraction; forward it + # to any stream_gemm_* helper you call. Declare + # it even if unused: multiply_subspace only + # passes the budget to a basis() that does. register_transform("mine", MyTransform) C = subspace_matmul(A, B, config=Config(transform="mine", rank_m=256)) diff --git a/tests/test_contributor_docs_basis_signature.py b/tests/test_contributor_docs_basis_signature.py index 2f62413..75ccce7 100644 --- a/tests/test_contributor_docs_basis_signature.py +++ b/tests/test_contributor_docs_basis_signature.py @@ -6,14 +6,17 @@ in the legacy branch and silently streams its basis at the default fraction instead of Config.vram_fraction (the bug #211 fixed for rsvd). -The template was brought in line already; these tests cover the two remaining -copies contributors actually read: +The template was brought in line already; these tests cover every other copy a +contributor actually reads: * CONTRIBUTING.md's "What you actually change" snippet -- the canonical "that is enough to be scored" example. + * strategy/README.md's "Register your own (the updatable hook)" -- the copy a + contributor meets first, and the one this file used to leave uncovered, which + is how it stayed on the old signature (#274). * strategy/examples/run_example.py's FirstAxes -- the worked custom transform. -Both are parsed as text/AST rather than imported: run_example.py runs a real +All are parsed as text/AST rather than imported: run_example.py runs a real subspace_matmul at module scope, which needs a GPU. Pure parsing; no GPU needed. Run: python tests/test_contributor_docs_basis_signature.py @@ -24,14 +27,21 @@ import re import sys +import pytest + sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from strategy.transforms import Transform _ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) _CONTRIBUTING = os.path.join(_ROOT, "CONTRIBUTING.md") +_README = os.path.join(_ROOT, "strategy", "README.md") _RUN_EXAMPLE = os.path.join(_ROOT, "strategy", "examples", "run_example.py") +# Every prose copy of the hook. Driven through one table so a new doc cannot be +# added -- or an existing one silently rolled back -- without being checked. +_MARKDOWN_SNIPPETS = ((_CONTRIBUTING, "CONTRIBUTING.md"), (_README, "strategy/README.md")) + def _read(path: str) -> str: with open(path, encoding="utf-8") as fh: @@ -52,12 +62,27 @@ def _basis_defs(source: str) -> list: return out -def test_contributing_snippet_declares_frac(): - """CONTRIBUTING.md's transform snippet is the contract contributors copy.""" - snippets = re.findall(r"def basis\(([^)]*)\)", _read(_CONTRIBUTING)) - assert snippets, "no `def basis(...)` snippet found in CONTRIBUTING.md" +@pytest.mark.parametrize("path,label", _MARKDOWN_SNIPPETS, ids=[s[1] for s in _MARKDOWN_SNIPPETS]) +def test_markdown_snippet_declares_frac(path, label): + """Every prose basis() a contributor copies must show the real signature.""" + snippets = re.findall(r"def basis\(([^)]*)\)", _read(path)) + assert snippets, f"no `def basis(...)` snippet found in {label}" for sig in snippets: - assert "frac" in sig, f"CONTRIBUTING.md basis() snippet omits frac: def basis({sig})" + assert "frac" in sig, f"{label} basis() snippet omits frac: def basis({sig})" + + +@pytest.mark.parametrize("path,label", _MARKDOWN_SNIPPETS, ids=[s[1] for s in _MARKDOWN_SNIPPETS]) +def test_markdown_snippet_matches_the_base_class_contract(path, label): + """Not just frac: the whole parameter list must match Transform.basis, so a + snippet cannot drift on any other argument either. Defaults are stripped -- + the snippets legitimately show `A=None`, the contract is the parameter names + and their order.""" + for sig in re.findall(r"def basis\(([^)]*)\)", _read(path)): + got = [p.strip().split("=")[0].strip() for p in sig.split(",")] + assert got == _base_params(), ( + f"{label}: def basis({', '.join(got)}) does not match " + f"Transform.basis({', '.join(_base_params())})" + ) def test_run_example_custom_transform_declares_frac(): @@ -74,14 +99,4 @@ def test_run_example_basis_matches_the_base_class_contract(): if __name__ == "__main__": - fns = [v for k, v in sorted(globals().items()) if k.startswith("test_")] - failed = 0 - for fn in fns: - try: - fn() - print(f"PASS {fn.__name__}") - except AssertionError as e: - failed += 1 - print(f"FAIL {fn.__name__}: {e}") - print(f"\n{len(fns) - failed}/{len(fns)} passed") - sys.exit(1 if failed else 0) + sys.exit(pytest.main([__file__, "-q"]))