Restructure the package, modernize the tooling, and refactor Qube - #17
Merged
Conversation
Move the package to a src/ layout and adopt the shared repo template's tooling and project scaffolding. No source or test file contents changed. Layout: - polymath/ -> src/polymath/ Packaging: - Consolidate config into pyproject.toml; delete .coveragerc and setup.cfg - Replace `packages = ["polymath"]` with packages.find where=["src"]; the old setting excluded polymath/extensions/ from built wheels - Add pytest, coverage, ruff, mypy, and pymarkdown sections, dev/docs extras, and an authors entry - requirements.txt now just installs the package in editable mode CI and scaffolding: - Adopt the template's workflows, .cursor/, .vscode/, scripts/, and GitHub issue/PR templates - Add Sphinx and PyMarkdown lint steps; keep flake8 for now, with the template's ruff/mypy commands left as a TODO - Keep the macOS and Windows test runners; narrow Python to 3.10-3.13 Docs: - Adopt the template's conf.py, index.rst, and module.rst; add contributing and code of conduct pages Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Switch CI and run-all-checks.sh from flake8 to ruff. .flake8 stays on disk for manual use but is no longer authoritative. Fix the 102 real findings and record why each rule is suppressed in pyproject.toml; RUF005 is off because Qube overloads +, so its fix rewrites vector addition into tuple unpacking. * Fix Polynomial.as_vector, which built every derivative from self instead of from the derivative being iterated. * Assert the two comparisons in test_scalar_as_index that were evaluated and then discarded, so the loop actually checks something. * Settle the line length at 90 and raise the Python floor to 3.11 across pyproject.toml, the CI matrix, the rules, and the editor settings. * Move .cursor/rules and .cursor/skills to .claude, converting the rule frontmatter to the paths model and promoting the three process rules to skills. Delete .cursor/settings.json, a copy of the VS Code one. * Add CLAUDE.md, a py.typed marker with a package namespace stub, and scripts/setup-venv.sh to create the virtualenv the checks require. * Clear the Markdown-lint violations in README.md and CONTRIBUTING.md, and add both to the CI scan. Add pyroma to CI so packaging metadata is gated there as well as locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every file held one unittest.TestCase whose single runTest method ran the whole file, so a failure named only the file. Each runTest is now split at the section comments the tests already carried, giving 781 test functions in place of 115. Splitting is applied only where a section is self-contained; a section that reads a variable an earlier one defined is merged back, so 37 files whose setup is genuinely sequential stay at one function. Coverage is unchanged at 97%, and the tests pass individually and in random order. Rewrites, all driven by the AST so multi-line calls and comments survive: assertEqual and friends become plain assert, assertRaises becomes pytest.raises, ctx.exception becomes ctx.value, self.fail becomes pytest.fail, setUp and tearDown become one autouse fixture, and helper methods move to module level. Defects this uncovered, each of which passed silently before: * assertAlmostEqual(a, b, 1.e-13) put the tolerance in unittest's `places` slot, where it was discarded. 21 sites now compare against the tolerance they were written with. * Two masked values compare equal but their difference is masked, so the translation keeps unittest's `a == b` shortcut ahead of the tolerance. * assertRaises(TypeError, lambda: -m1) needed the lambda parenthesized before the call suffix, or nothing invoked it. * Several files set Qube.prefer_builtins in setUp or mid-test, which changes return types; the fixture and the state-region grouping keep that intact. * A try/fail/except block became pytest.raises(..., match=...), and two pytest.raises blocks had their setup hoisted out. Drop the PT009, PT027, N801 and N802 suppressions, which existed only to silence 11,039 findings about the unittest suite. Add E721 and E711 for tests, where exact type checks and `== None` on overloaded operators are deliberate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each fix has a regression test that fails against the previous code. - Qube.__pow__ called as_fully_masked(), which does not exist, so any non-Scalar raised to a masked exponent raised AttributeError. - Matrix.inverse() substituted identity matrices into its own values array, corrupting the caller's object whenever a matrix was singular. - Scalar.sort() re-masked its result by assigning an unmasked value, which cleared the mask and exposed the infinity that had been used to sort masked items to the end. It now orders values and mask by one common permutation, so neither can drift from the other. - Qube.__setitem__ left arg_mask unbound when the index held non-consecutive arrays and the argument carried a scalar mask. - Qube.wod and both Polynomial converters rebound the source object's _derivs and _cache instead of copying them, leaving two objects sharing one dictionary. Polynomial.as_vector() consequently modified the Polynomial it was called on. - Qube constructors called np.ma.stack(*arg) rather than np.ma.stack(arg), so no Qube could be built from a list of MaskedArrays. - Boolean ** int called ndarray.view() on a value that is a Python int when both operands are shapeless. - Vector.int() sized its top tuple with len(top), so a single value raised TypeError. - Scalar.frac() ignored recursive=False and returned derivatives. - Scalar.exp(check=False) caught ValueError and TypeError, but NumPy reports overflow as a RuntimeWarning, so the documented ValueError was never raised. - Qube.as_size_zero() collapsed the last axis for every axis except 0. It now honors the axis given and rejects one out of range. - Three error messages were missing an f prefix, named an undefined variable, or stated the opposite of what was meant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Indexing a Scalar with an integer array cost time proportional to the length of the axis being indexed rather than the number of indices, because _prep_index() built a Python set holding one entry per element of that axis. Pulling 100 elements out of a million-element Scalar took 34 ms. The set was used only to redirect masked index values away from the elements the index really selects, so it is now built only when something is masked, and as a boolean occupancy array rather than a set of Python objects. Normalizing negative indices moved under the same guard, since NumPy already interprets them. Qube.__init__ ran np.prod() four times on small tuples of Python ints, which cost about 3 us per call and so dominated the construction of every intermediate result. math.prod() answers the same question about thirty times faster and returns an int, so the surrounding int() calls are gone as well. The same idiom is replaced in reshape_numer(), reshape_denom(), flatten(), and the pickler. Measured on a 1000-element object: Scalar + Scalar 22.6 -> 10.4 us, Vector3 + Vector3 22.2 -> 11.0 us, Vector3.cross 57.3 -> 34.1 us. An unmasked index of 100 elements is now 32 us at every array size tested, against 81 us at 1e3 and 34,418 us at 1e6 before. Indexing behavior is unchanged: 420 randomized cases spanning in-range, negative, out-of-bounds, unmasked, partly masked and fully masked indices, through both __getitem__ and __setitem__, give byte-identical values and masks, including the values that sit under a mask. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records a review of all 25 modules under src/polymath: confirmed defects with reproducers, correctness risks found by inspection, dead code, unresolved TODO and XXX markers, measured performance findings, and drift between the project rules and the current configuration. Sections 1, 5.1 and part of 5.2 are marked as fixed by the two commits that precede this one. The rest is open, most notably the derivative propagation in Polynomial.invert_line() that carries an unanswered note questioning whether the math is correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The method replaces a commented-out draft that had been carrying the note "algorithm has been validated but code has not been tested". The draft does not run: inverse() is called positionally although it is keyword-only, range(size,0) is empty so the back substitution never executes, the names `shape` and Unit._unit_div do not exist, the pivot row broadcasts against the wrong axes, and the derivative block indexes self._derivs[k] inside a loop over `key`. Rather than repair a hand written elimination, the documented contract is implemented on top of numpy.linalg.solve. Denominator axes are flattened into extra right-hand-side columns, so one code path serves both the solution and its derivatives. Singular matrices are masked the way inverse() masks them, with the identity substituted into a copy so that the caller's matrix is left alone. Derivatives follow from differentiating A X = B, which gives A dX/dt = dB/dt - (dA/dt) X, so each one is the solution of the same system with a new right-hand side. The result takes the subclass of the right-hand side where that subclass fits, so a Vector3 in gives a Vector3 out. Verified three ways: A X == B for sizes 1 through 6, agreement with numpy.linalg.solve and with inverse() * b, and derivatives against a central finite difference, including derivatives that carry a denominator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The method rejected any denominator, but its own recursive branch calls itself on each derivative, and a Jacobian-style derivative carries one. The method therefore raised a ValueError for every Vector holding a derivative of nonzero denominator rank, rather than for the unsupported case it meant to catch. The code that was supposed to handle a denominator sat behind that guard and so had never run. It referred to ndarray._shape, which does not exist, and rolled the matrix axes back with a loop that lands them in the wrong place once the denominator rank reaches two. Both passes now use moveaxis, which states the intent directly and is correct for any rank, and the guard is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
math_ops.__ipow__ was defined but never bound to Qube, so "**=" fell back to rebinding the name rather than modifying the object, unlike every other augmented operator in the module. Binding it exposed a bug in the function: it called set_unit(), which requires the new unit to be compatible with the old one, and exponentiation is precisely the operation that changes a unit's dimension. It now assigns the unit directly, as __imul__ does, refuses a non-integer result for an integer object, and captures the derivatives before mutating, because "**" with an exponent of one returns the same object. Boolean rejects "**=" as it rejects the other in-place operators, since the result is a Scalar and cannot be stored back into a Boolean. __floordiv__ lacked the fast path for a plain number that __mul__, __truediv__ and __mod__ all have, so floor division by an int converted it to a Scalar first and _floordiv_by_number was never called. The branches in Scalar.max(), min(), argmax() and argmin() that handle a scalar-shaped mask cannot be reached: they sit inside the partially masked case, where a reduction over every axis is never fully masked. Coverage confirmed the lines were never executed. The first assignment to numer and denom in Unit.__init__ is overwritten two lines later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every intermediate result went through the full public constructor, which re-derives what the caller already knows: the values are re-inspected for their dtype twice, the mask is re-validated against the shape, and the default value is recomputed from scratch. A profile of Scalar + Scalar attributed 76% of the time to __init__. Qube._new_from_parts() sets the attributes directly from a values array whose dtype and shape the caller computed itself. It still broadcasts a mask that is narrower than the values, since two operands can broadcast against each other while their masks do not, and it still reduces a NumPy scalar to a Python scalar the way the constructor does. The default is taken from the example object when the item shape and dtype both survived the operation, and recomputed otherwise; that rule is now shared with __init__ through Qube._default_for(). Ten call sites use it: __add__, __sub__, _mul_by_scalar, _div_by_scalar, dot, norm, norm_sq, cross, outer and as_diagonal. The validating constructor remains the public entry point. Scalar + Scalar goes from 10.6 to 4.3 us and Vector3 + Vector3 from 10.7 to 4.2 us. A harness comparing every shape, dtype, mask, default and unit attribute of 1069 operation results reports them byte-identical to the previous constructor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An isinstance() check against numbers.Real dispatches through __instancecheck__, which measures about four times slower than a check against a tuple of concrete classes. A profile of Scalar + Scalar showed eight such checks per operation. The hot paths now test (int, float, np.integer, np.floating) first. Where the answer has to be exact, the ABC still has the last word, so a type registered with it but not listed there, such as fractions. Fraction, is still recognized; the operator fast paths simply fall through to the general path for one. Qube._is_one_value() also rules out the types that dominate the negative answer before consulting the ABC. _dtype_and_value() now recognizes a plain array by its exact type before anything else, since that is its most frequent input by far, with the array branch factored out as _array_dtype_and_value(). Note that a MaskedArray still reaches that branch rather than the masked-object handling below it, and so is still returned with its mask intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Several operations walked their values or masks more times than needed: - _mean_or_sum() and Scalar.max(), min(), argmax() and argmin() each copied the values array and then assigned a fill value through a boolean index. np.where() does it in one pass; on a 200x200 array that is 68 us against 15. Masked max() drops from 100 us to 42 us. The unmasked count is now taken by counting the masked items and subtracting, which avoids materializing the antimask. - Scalar.maximum() and minimum() reduced their arguments through result[antimask] = scalar[antimask], a full __getitem__ and __setitem__ round trip per argument. Selecting with np.where() keeps the reduction in NumPy: three arguments of a thousand elements go from 183 us to 19 us. 8000 randomized comparisons against the previous implementation agree on values, mask, mask type, shape and dtype. - Qube.or_() and and_() recursed pairwise over three or more masks, re-slicing the argument tuple each time. They now short-circuit on the first settling value and combine the remainder in one pass. - Unit.create_name() searched combinations of standard units on every call, including from every repr. The search is memoized on the exponents and triple, which takes str() of an unnamed unit from 20 us to 3 us. - Matrix.identity() built the identity elementwise; np.eye() does it in one call. _cross_3x3() allocated its output as float64 whatever the inputs were, so integer vectors were silently promoted. Two suggestions from the review were measured and rejected. np.argwhere() for _find_corners() is 15x slower than the per-axis reduction, because it allocates a coordinate pair for every unmasked element. np.broadcast_shapes() is about twice as slow as the Python loop in broadcasted_shape(), which is always called with two or three short shapes. Both are left alone, with the reason recorded where the change would have gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CHANGES RESULTS IN THE LAST BITS. This is the only commit in the performance series that is not bit-for-bit identical to what came before, and it is kept on its own so that it can be reverted alone. Qube.dot() built the full elementwise product and then reduced it, having first copied both operands to make them contiguous. einsum contracts the last axis without materializing the product and reads strided input directly, so all three temporaries go away. It measures about three times faster on the shapes this is used with, and the saving in memory matters as much as the saving in time on the large arrays this library is aimed at. The cost is that einsum accumulates in a different order, so results move by up to 1.8e-15 in absolute terms for operands of order one. Everything that routes through dot() inherits that: matrix multiply, Vector.unit(), sep(), and the derivatives of norm() and cross(). Four tests asserted the previous order. Three compared dot() against np.sum(a * b) exactly and now compare within rounding; the fourth allowed 1e-15 absolute on a product of order ten, which was already within two units in the last place, and is now relative. The conversion of this function's np.rollaxis calls to np.moveaxis rides along, because they sit in the same contiguous block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
np.rollaxis is not deprecated, but its `start` argument means "roll the axis until it lies in front of this one", which reads as an off-by-one against np.moveaxis's plainer "move this axis to this position". The codebase already mixed the two. Twenty-two call sites convert. Where the axis moves to the end, the destination becomes -1; where it moves to the front, it is unchanged. The two calls in shaper.roll_axis() stay, because that method's own published contract is np.rollaxis's, so expressing it with np.moveaxis would mean adjusting the destination whenever start > axis, which is exactly the confusion the change is meant to remove. The conversions inside Qube.dot() and Qube.as_diagonal() went in with earlier commits, because they sit in the same contiguous blocks. All 1399 states in the comparison harness are byte-identical, which covers the affected methods across leading shapes, item ranks and denominator ranks of zero, one and two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Marks each item in section 5 as applied or rejected, replaces the benchmark table with before-and-after figures, and notes the one change that moves results in the last bits. Three of the section's own recommendations turned out to be wrong when measured, and are recorded as such rather than quietly dropped: np.broadcast_shapes for broadcasted_shape, np.argwhere for _find_corners, and einsum for as_diagonal are each slower than what they would have replaced, at the sizes this library uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Qube._pickle_debug() writes a module-level flag that makes __setstate__ attach its encoding details to the object. One test turned it on, three more read those details, and a fourth turned it off again at the end. Run with --dist loadscope, as the check script does, a module's tests share a worker and the flag reached them all. Distributed any other way, the readers landed in a process where it had never been set and failed on a missing attribute. The suite is required to be order-independent, so this was a latent failure waiting on a change in test count. A fixture now owns the flag and restores it afterwards, and each test that needs it asks for it. All four pass on their own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Vector.element_div() labelled the derivative of a quotient with the divisor's unit to the first inverse power, where the quotient rule gives the second. The values were right; only the unit was wrong. - Qube.__init__() folded an explicit nrank=0 into "unspecified", so Vector(values, nrank=0) quietly built a rank-1 object instead of refusing. An explicit rank is now honored and checked. A rank inherited from another object still defers to the subclass default, which is what lets Matrix(scalar) read the trailing axes of a rank-0 object as its items, as Matrix3.x_rotation() relies on. - Qube hashed by identity while comparing by value, because __eq__ is bound after the class is created and Python therefore never set __hash__ to None. Two equal objects hashed differently, so one used as a dictionary key could not be looked up again. Qube is also mutable. It is now explicitly unhashable. - The cached antimask was writable, and every caller receives the same array, so one caller modifying it corrupted the answer for the rest. - as_readonly() replaced entries in the cache while iterating it, which a cached object reaching back into the same dictionary could disturb. - Matrix.unitary() tested its mask against False by identity, which np.False_ fails. - Vector.clip_component() assigned the Scalar holding the upper limit into the values array rather than the value inside it, where the lower-limit branch does the latter. The eighth item, unshrink() losing the leading shape of a fully masked object, is not fixable as it was posed: such an object is shrunk to a single value, so the antimask describes only the axes it collapsed and nothing about those ahead of them. Deriving them from the antimask produces a shape that looks right and is not. The limitation is documented on the method instead, along with the `shape` argument that lets a caller supply what is missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sibling classes disagreed about which arguments were keyword-only: Scalar.as_index_and_mask() and Scalar.int() took theirs by keyword while the Vector versions took them positionally, and Qube.as_float() differed the same way from as_int() and as_bool(). They now agree, which is the Scalar convention in each case. The pickler module was bound onto Qube as an attribute, so that Sphinx would render its documentation and help() would find it. That put a module, which is neither callable nor data, into the namespace of every object. It is documented through docs/module.rst instead. __getitem__() and __setitem__() had no docstrings at all. They now describe what they accept and the two places where indexing departs from NumPy deliberately: axes selected by array indices keep their position rather than moving to the front, and a single boolean does not add a leading axis. Both are reasonable, and neither was written down. _prep_index() caught every exception and reported it as an IndexError, which turned a bug in the module into a message about the caller's index. It now converts only the errors a malformed index can raise. The invariant check in the pickler used a bare assert, which python -O removes, in the one mode where an unexpected float format would matter most. It raises now. Four docstrings described something other than the code: filled() documented a parameter it does not take and omitted the one it does, _suitable_mask() named a parameter that had been renamed, __init__() promised a ValueError where it raises a TypeError, and extract_denom() gave an example with the wrong result shape. The class docstring also now states that objects are unhashable and that nothing is synchronized. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- The runtime dependencies carried no version floor at all, so an install could resolve to a NumPy this code cannot run against. The code is written to NumPy 2 semantics; that is now declared. The floors are declared rather than tested, since CI installs the newest. - The dev extra named the package itself, which is circular. Only the reference that pulls the docs extra does any work, and it stays. - pytest measured coverage over one path while the coverage configuration named another. They agreed only by accident of the src layout, and fail_under would have started measuring something other than the run. Both now name the same path. - filterwarnings is set to "error". Much of this library's correctness rests on how NumPy reports overflow, division by zero and invalid values, and one of the defects fixed earlier was exactly a warning caught as the wrong type. The suite passes with no exemptions. - The check script ran mypy over src, which CLAUDE.md forbids and which would report nothing useful, since src is deliberately unannotated. It runs over tests, as the rule says. mypy is also declared in the dev extra, because stubtest ships inside it. - pip-audit is added on a schedule of its own. security.md asks for it in CI, but environment.md requires the merge gate to be exactly the set the check script runs, and that set is offline and finishes in ten seconds. An advisory can also appear without anything here changing, so a schedule suits it better than a diff does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The package shipped a py.typed marker and a stub covering only the package namespace, whose own docstring admitted the classes were still inferred from their unannotated implementations. That is worse than shipping no marker: it tells a type checker the package is typed and then hands it nothing, suppressing the diagnostic that would otherwise have warned the user to expect nothing. Eleven per-module stubs now describe every public class. Signature shapes are exact: every parameter, which of them are keyword-only, and which carry defaults. Types are taken from the docstrings wherever those state one unambiguously, and are left as Any where they do not, rather than guessed at, so nothing in them claims more than is known. stubtest checks the stubs against the runtime API and now runs in the check script and in CI. That matters more here than in most packages, because the API is assembled at import time: most of Qube's methods are bound onto the class from the extensions modules, so a hand-written stub would drift from the first change and nothing would notice. mypy is confined to the stubs, since the modules themselves are unannotated by design. Writing them turned up one real defect. polymath.unit.unit was a loop variable left behind in the module namespace, exported as a public name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Marks each item as closed, and notes the one place where the finding held but the remedy proposed alongside it did not: unshrink() cannot recover the leading shape of a fully masked object from the antimask alone, so that item is documented rather than fixed. Also records the two defects that writing the stubs uncovered, neither of which the review had found: a loop variable exported from the unit module, and four tests that depended on a global a fifth one set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
invert_line() inverted the derivative polynomial instead of applying the chain rule, returning (1/a', -b'/a') where the derivatives of the inverted coefficients are (-a'/a**2, -b'/a + b*a'/a**2). For a=2, a'=1, b=3, b'=4 it returned [1, -4] rather than [-0.25, -1.25], confirmed against a finite difference. The propagation loop is deleted rather than corrected: to_scalars() returns Scalars carrying their derivatives, so the arithmetic that builds the coefficients already applies the chain rule, and the loop was overwriting a correct answer with a wrong one. Building the result through Qube.from_scalars() with classes=[Polynomial] keeps the derivatives in class Polynomial, which the discarded loop had been the only thing supplying. That dependency exposed a second defect. Polynomial.__init__ guarded its derivative conversion with "type(self) is not Polynomial", so the common case, Polynomial(vector), kept Vector derivatives and only the subclass path converted. That path was itself half-done: it rebuilt _derivs but not the d_dt attribute copied from the source object, so the dictionary and the attribute disagreed about the derivative's class. The guard now tests the derivative rather than the object, and sets both. The old invert_line test asserted only that a d_dt existed and was a Polynomial, never a value, which is why this shipped. Nine tests now cover the derivative values, a finite-difference check, multiple derivatives, the non-recursive branch, the masking of a zero leading coefficient, and the class and identity of a converted derivative. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each module now states its own public surface rather than leaving it to be inferred from which names happen to lack a leading underscore. The lists hold every public top-level name the module defines: the class for each of the eleven class modules, and the bound extension functions for each module under extensions/. Two modules export nothing of their own and say so with an empty list and a comment: extensions/indexer.py, whose __getitem__ and __setitem__ are bound onto Qube as special methods, and extensions/__init__.py, which only performs those bindings. The eleven stubs move in step. stubtest reports "__all__ is not present in stub" as soon as a module declares one at runtime, so publishing __all__ without updating the stub would break the check script and CI. Verified against the runtime rather than by inspection: every listed name resolves, every public name each module defines is listed, and "from <module> import *" yields exactly __all__ for all 24 modules. _version.py is left alone; setuptools_scm generates it, and its __all__ already exists. The rendered documentation is unchanged. Only two automodule directives exist and neither takes member selection from __all__, confirmed by building module.html from a worktree at the previous commit and diffing: identical, all 596 documented entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
name_to_dict() is now a tokenizer and a recursive-descent parser in place of the string splitting it used before. It handles nesting, negative exponents and whitespace, which the old version could not, and it rejects expressions the old version accepted by accident: "(km" used to return a dictionary and now reports the missing parenthesis. The two "TODO What is the purpose of this check?" raises and their "pragma: no cover" markers went with the code that held them. Three defects came in with that rewrite and are fixed here. It dropped the isinstance(expr, dict) passthrough its own docstring still promised, which broke fourteen tests reaching it through Matrix.inverse, Scalar.sqrt and four Vector products. It made a latent KeyError in _mul_names and _div_names reachable, because create_name() yields a zero exponent for every unused dimension and those keys need not appear in the first name at all; both now pop rather than del. And its docstring described zero retention after the code had moved to dropping those keys. The name argument is gone from mul_units, div_units, sqrt_unit, unit_power and sqrt. No caller in src/ ever passed it, so the answer to the "why do we only do this for new units?" marker was that there was no reason to do it at all. Unit.KM * Unit.S and mul_units(Unit.KM, Unit.S) now agree. Removing it exposed an older defect: Unit.STER.sqrt(), and therefore Scalar(x, unit=Unit.STER).sqrt(), raised ValueError because the name "ster" has an odd exponent even though the dimensions halve cleanly, and the name argument had been the only way around it. _name_power() now returns None when a power cannot be applied to a name, leaving the unit to derive a name from its dimensions, so steradians square-root to radians. Its string-power branch is deleted; name_to_dict() can no longer return an integer, so that branch could never succeed. div_names and name_power are now private, matching _mul_names, and the conversion methods take scalars to scalars again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both unit.py markers are answered rather than merely deleted: the name argument that mul_units and div_units used to overwrite is gone, and the two "purpose of this check?" raises went with the parser rewrite. The note also records the three defects that rewrite cost on the way in, and the older Unit.STER.sqrt() failure that surfaced underneath it. The remaining markers stay by choice. The quaternion.py divide-by-zero note states a real open design question, and a marker that does that is worth more than an answer invented to clear a checklist; the two NotImplementedError markers are honest about what is not built. Section 4 is closed with them in place. Closing it left three statements in the document contradicting their own sections, all corrected here: priorities 1 and 3 were never struck through even though sections 1 and 3 have reported themselves fixed since 2026-08-09; section 1's note still claimed everything from section 2 onward was open; and the Summary read in the present tense as a live to-do list. The Summary now carries a status note marking the review closed and saying its prose is kept unedited as the record of the original reading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch was chosen from the three diagonal elements alone, so rotations near the identity selected the worst-conditioned branch: r_sq = 1 + 2*max_diag - trace approaches zero there, and the identity matrix itself produced the zero quaternion. Adding the trace as a fourth candidate restores Shepperd's method. Because the four candidates sum to twice the trace, the largest is at least half the trace, so r_sq >= 1 for any 3x3 matrix. The special case guarding against a zero divisor is therefore unreachable and has been removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The disabled code differentiated the trace-branch formulas but scaled them with
the s computed for the largest-diagonal branch, so the two never agreed. Each
branch now differentiates the expression it actually evaluates. Writing every
component as quat_over_s * s with s = 0.5/sqrt(u), the chain rule gives
dquat/dQ = s * d(quat_over_s)/dQ - 2*s**3 * quat_over_s * du/dQ
where only du/dQ and the constant pattern d(quat_over_s)/dQ vary by branch.
Verified against central finite differences in all four branches, agreeing to
1.2e-10, the noise floor of the difference. Also verified that a quaternion
derivative survives the round trip through to_matrix3() and back to within
1.6e-15, using the independently derived partials of to_matrix3().
All four branch formulas yield the same derivative whenever the matrix
derivative is tangent to the space of rotations, so the result is unambiguous
for the derivative of any proper rotation matrix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The method existed as a possible route to differentiating the matrix-to- quaternion conversion. from_matrix3() now differentiates directly, and the experimental algorithm was the less accurate of the two, so nothing depends on it any longer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A rotation matrix has nine elements but three degrees of freedom, so the equivalent unit quaternion is far more compact. The experimental methods that attempted this could not be called at all, being name-mangled by their own suffixes, and failed in six ways once enabled. The values are now carried by a Quaternion rather than by a Matrix3 clone whose _values no longer matched its _item, which was the source of the failures on masked objects and on any numeric pickle_digits setting, and of the numerator mismatch that made every derivative unrestorable. The dropped component is now the largest of the four rather than the first. The largest is at least 0.5, so making it positive is unambiguous where np.sign() returned zero for the 180-degree rotations that were silently restored as the identity, and recovering it from the unit length no longer loses precision as it approaches zero. It is swapped into index zero before being dropped, so the values still compress to a third of the default encoding rather than four ninths. Objects a quaternion cannot represent fall back on the default encoding, which stays exactly lossless: denominators, objects below the size cutoff, fully masked objects, matrices that are not proper rotations, and derivatives that are not tangent to the space of rotations. The quaternion encoding is reversible to about one part in 1.e15, not bit for bit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four calls to math.prod() ran on every fast construction, three of them over item shapes that are usually the same as the example's. The example already supplies the default value on the same reasoning, so it now supplies the products too whenever the shape, the item shape and the split between numerator and denominator carried through the operation. Where the item shape did not carry through, the numerator and denominator products multiply to give the item product, so it no longer needs a pass of its own. Measured on this machine: scalar arithmetic 7 to 11% faster, and 1 to 8% for operations that spend most of their time in NumPy. Over the test suite, 84% of constructions take the item products from the example and 98% take the shape product. The suite was also run with every _new_from_parts result checked against a fresh math.prod() of its own shapes; all 28535 constructions agreed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two adjacent sections move together into extensions/readonly_ops.py, because copying an object is what produces a writeable one from a read-only one: _array_is_readonly(), _array_to_readonly(), as_readonly(), match_readonly(), require_writeable(), require_writable(), copy() and __copy__(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "Value tests" section and the half of "Conversions" that changes an object's class move into extensions/casting.py: as_one_bool(), is_one_true(), is_one_false(), _is_one_value(), as_this_type(), _deriv_classes(), _castable_to(), cast(), as_all_constant() and as_size_zero(). The other half of "Conversions", which changes the data type rather than the class, stays behind for now. _DERIV_CLASS sat in the middle of the section but is a class attribute that subclasses override, not a method, so it joins the other default class constants at the top of the class instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Everything concerned with the mask itself gathers in one module: the coercion helpers _as_mask() and _suitable_mask(), the combiners or_() and and_(), and the whole "Object mask operations" section, from is_all_masked() and count_masked() through remask(), expand_mask(), collapse_mask() and the as_mask_where_* family. This is distinct from the existing mask_ops.py, which decides which elements to mask by comparing values; masking.py is about building a mask and about the mask an object already carries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rest of the "Support functions" section and the rest of "Conversions" both answer the same question, so they move together: _has_qube(), _has_masked_array(), _as_values_and_mask(), _dtype_and_value(), _array_dtype_and_value(), _dtype(), _casted_to_dtype(), _suitable_dtype(), _suitable_numer(), _suitable_value(), dtype(), is_numeric(), as_numeric(), and the is_float()/as_float(), is_int()/as_int() and is_bool()/as_bool() pairs. _suitable_dtype(), _suitable_numer() and _suitable_value() take the class as their first argument. A module-level @classmethod is not a function and stubtest rejects it, so each is written as a plain function and wrapped where it is bound. qube.py is left with what defines an object: the class constants, __init__(), the construction path, low-level access, the properties, the cache, __repr__()/__str__() and from_scalars(). It is 1326 lines, down from 3511. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two invariants now carry weight that nothing in the code states. The package must import polymath.extensions before any subclass module, and no module under extensions/ may import a subclass at module level, or a subclass will load before the methods its class constants need. A module-level @classmethod cannot be bound the way @staticmethod and @Property can, because stubtest rejects it. Breaking either one fails at import time or in the check script rather than quietly, but neither is guessable from reading the code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Whitespace only, inside brackets; no behavior changes. Four calls in scalar.py put "axis=axis" two columns off the paren it belongs to, and three array literals in the shrinker tests had rows two columns short of lining up as a grid. Both read as alignment and were simply wrong by two. The rest were ragged hanging indents, where a continuation was indented to an arbitrary deep column and a following line then failed to match it. Those are reflowed to an ordinary four-space continuation in errors.py, item_ops.py, mask_ops.py and math_ops.py. Left alone: the stacked 3x3 literal in Matrix3.to_ra_dec_length() and the _EASY_INT_POWERS and _EASY_FLOAT_POWERS tables in scalar.py. Both align on something pycodestyle cannot see -- the columns of a matrix, and the digit rather than the sign of a key -- so reformatting them would destroy the thing they are aligned for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ruff is the linter of record, but it implements no rule in the E121-E133 range, so continuation-line indentation is the one pycodestyle family it cannot gate. The codes are not disabled or preview-gated; they are not written, and ruff tracks them as still to do. Nothing in the check script noticed a continuation line that did not line up with the bracket it belonged to. The check script gains "flake8 --select=E12,E13 src tests", enabled by default and selectable on its own with --flake8-cont, following the existing RUN_*/ENABLE_* pattern. flake8 was already a dev dependency. CI runs the same command, since the script defines the set CI must run. This makes the per-file-ignores in .flake8 authoritative for those codes alone, which the file and CLAUDE.md now both say. Two entries are added there for alignment that pycodestyle cannot understand, each with a comment giving the reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A summary line that describes what a call evaluates to reads as a noun phrase, not an imperative: "The reciprocal of this Matrix3" rather than "Return the reciprocal of this Matrix3". Most of the library was already written that way; this brings the rest into line, across the three-valued logic operators, the Vector3 coordinate accessors, the pickler helpers and the reciprocal and derivative methods on Matrix, Matrix3 and Polynomial. A few Parameters: entries are rewrapped to fill the 90-column line they were already close to. Docstrings only; no code changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Polynomial has always been exported and carries a full entry in the generated API reference, but it was absent from the list of classes that introduces the documentation. A reader scanning that list concluded the class did not exist. It is added beside Quaternion, since both are Vector subclasses. The list is maintained twice, in the package docstring and in README.md, which docs/index.rst includes; both are updated. Nothing checks the two against __all__, so neither the warnings-as-errors nor the nitpicky Sphinx build noticed the omission. Also drops a stray character from the Scalar entry in the docstring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 221 files, which is 121 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (221)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
==========================================
+ Coverage 96.11% 96.88% +0.77%
==========================================
Files 24 32 +8
Lines 7207 7616 +409
Branches 1617 1690 +73
==========================================
+ Hits 6927 7379 +452
+ Misses 162 134 -28
+ Partials 118 103 -15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
np.random.randint() returns the platform's default integer, which is 32 bits on Windows and 64 elsewhere. norm_sq() preserves the width it is given, correctly, so the test asserted a value that only held on two of the three platforms CI runs and failed on all three Windows jobs. The docstring already stated the contract: the squared norm of an integer object is an integer. The assertion now says that. The other np.int64 assertions in the suite are unaffected, because as_int() names the width explicitly rather than taking the default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sphinx-build -n resolves every cross-reference and reports the ones that have no target. Only -W is wired into the check script and CI, and without -n these are never emitted, so a dozen of them had accumulated unseen. The genuine ones are fixed here; what remains is docstring type vocabulary such as "optional" and "array-like", which is a separate question. Two are outright defects. Qube.__init__ documented its first argument as "(Qube, array-like, float, in, or bool), :", where "in" is a typo for "int" and the comma does not belong. And Matrix3.y_rotation() lost the closing parenthesis of its angle type, which made Napoleon read the line as three parameters named "(Scalar", "array-like" and "float" and render that method's whole parameter block as nonsense. The rest name targets that do not exist. :meth:`~Matrix3.unitary` is defined on Matrix, not Matrix3. Vector.reciprocal() and Quaternion.identity() pointed at extensions.math_ops, which is not a documented path; the public names are on Qube. The pickler module docstring renders under its own automodule, where a bare "Qube" does not resolve, so both of its references now name polymath.Qube. QubeIterator and QubeNDIterator are exported but their module was not in the API reference, so docs/module.rst adds it alongside the pickler. Also gives the four Matrix3 rotation constructors the same angle type, which two of them were missing entirely, and types the ignored "out" parameter as object rather than the unresolvable "any". 581 nitpicky warnings become 573, and all 573 are type vocabulary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
doc_python asks for both a warnings-as-errors and a nitpicky Sphinx build, but only -W was ever wired up. Without -n, a cross-reference with no target is never reported, so nothing stopped one from being written. docs/conf.py now sets nitpicky = True rather than passing -n at each call site, so the check script, CI and read-docs.sh all get it and none of them can drift out of step. The build is clean, so any new dangling reference is an error from here on. Getting there meant spelling the linkable types the way Sphinx can resolve them, which is a fix rather than an exemption: np.ndarray and a bare ndarray become numpy.ndarray, "number" becomes numbers.Real, and "class" becomes type, all of which link through intersphinx or the builtins. A bare "array" becomes array-like, matching the vocabulary used everywhere else. What remains cannot be linked at all, because it names no Python object: "optional", which Napoleon splits off the end of every optional parameter, and the informal words array-like, scalar, vector-like and convertible. Those five are the only entries in nitpick_ignore_regex, each with a comment saying what it means and why it has no target. Also rewraps the docstring lines that the longer type names pushed past 90 columns, and drops a duplicated "to" found in one of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rfrenchseti
approved these changes
Aug 11, 2026
The method attaches an attribute of the caller's choosing to an object and assigns its value. It refuses a name the object already has, unless add_attr() is what added that name, and never accepts a name beginning with "d_d", which is reserved for derivatives. The names are recorded in a frozenset that _transfer_attrs() carries along, so a copy, clone, wod, deepcopy or pickle round trip preserves the attributes. An operation that computes new values does not. To draw that line, clone() is split into the public method, which carries the added attributes, and a private _clone_new_values(), which does not; the operations that build a result by copying an object and then replacing its values now call the latter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Qube.__setstate__ installs the pickled state dictionary as the instance dictionary, so an object restored from a pickle written before an attribute existed does not have that attribute, and the first operation to read it raises AttributeError. Measured against the current code, a state dictionary written before the internals were renamed lacks _is_array, _is_scalar and _ndims; the rename of _units_ to _unit is already handled. Each of the three is derived from the values or the shape, so __setstate__ recomputes any that the state does not carry, once the values have been decoded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
markshowalter
force-pushed
the
mark-reorg
branch
from
August 14, 2026 01:31
cfbab03 to
62713f0
Compare
Collaborator
Author
|
A bunch of tests seem to be "stuck", but other tests of the same name have completed. |
This was referenced Aug 15, 2026
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
2 participants
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.
Purpose
mainholds the library as it stood before a sustained modernization pass. This branch is that pass. It moves the package to asrc/layout, brings the tooling and CI in line with the standards in.claude/rules, converts the test suite to pytest, fixes the correctness defects that three written reviews turned up, banks the performance work those reviews identified, publishes PEP 561 stubs for the whole public API, and splits the 3,511-lineqube.pyinto focused modules.It is one branch rather than several because its first commit relocates every file in the repository. Any subsequent change is expressed against the new layout, so a smaller PR built on the old one would conflict immediately. The intended reading is the net diff against
main; the commit series is nonetheless ordered so that each commit passes the full check suite on its own, and it bisects cleanly.Fixes #18.
Fixes #15.
Fixes #12.
Fixes #5.
Fixes #1.
Changes/Implementation Details
57 commits. Grouped by theme rather than chronologically.
Repository layout, tooling, and CI
polymath/becomessrc/polymath/, which accounts for most of the 219-file diff. Addsscripts/,.claude/,critiques/,CLAUDE.md,.vscode/.scripts/run-all-checks.shbecomes the single source of truth for the gate, and CI runs exactly the set it enables.E121-E133range, so continuation-line indentation is gated byflake8 --select=E12,E13in both the script and CI. The 21 pre-existing violations are resolved: 16 were genuine misalignments and are fixed; 5 are deliberate column alignment (a stacked 3x3 matrix literal, and dict keys aligned on the digit rather than the sign) and are exempted per-file in.flake8with the reason recorded.filterwarnings = ["error"],--strict-markers,--strict-config.Tests
unittest.TestCaseto pytest throughout: module-leveltest_*functions, plainassert,pytest.raises(..., match=...), parametrization. 1012 tests, order-independent and safe under-n auto.Correctness fixes
Polynomialderivative propagation and derivative classes.Quaternion.from_matrix3()gains derivatives and includes the trace when selecting its branch; the experimental variant is deleted.Matrix3derivative is now typedMatrix, since the derivative of a rotation is not a rotation._DERIV_CLASSnames the substitute andQube._deriv_classes()applies it.assertin the pickler, whichpython -Oremoves, is replaced.Performance
einsumrather than broadcasting; the norm contracted rather than squaring the whole array.numbersABCs on the arithmetic hot path.clone()andwod.mainincritiques/2026-08-10-performance-critique.md: median 4.29x, geometric mean 3.71x across 41 operations, up from 2.71x and 2.91x partway through. The two operations slower thanmainare tabulated there as deliberate trades.Public API and typing
stubtestin the script and in CI.__all__declared in every module.Matrix.solve()added forA X = B;Vector.cross_product_as_matrix()supports denominators;__ipow__bound.qube.pysplitqube.pygoes from 3,511 to 1,326 lines. Seven new modules underextensions/, bound ontoQubebyextensions/__init__.pyexactly as the eleven existing extension modules already were:dtypes(626),masking(558),deriv_ops(364),casting(320),readonly_ops(207),unit_ops(182),errors(128). What remains inqube.pyis what defines an object: class constants,__init__, the construction path, low-level access, the properties, the cache,__repr__/__str__andfrom_scalars().Two invariants now carry weight and are recorded in
CLAUDE.md:polymath/__init__.pymust importpolymath.extensionsbefore any subclass module, because each subclass builds read-only constants such asScalar.ZEROwhile it loads and those calls need the bound methods. For the same reason no module underextensions/may import a subclass at module level;shrinkerwas the only one that did and now reaches forQube._SCALAR_CLASS.@staticmethodor@propertycan be bound directly, but a module-level@classmethodis not a function andstubtestrejects it, so those are written as plain functions and wrapped at the binding site.qube.pyineeded no change: every moved member still lives onQube.Documentation
__getitem__/__setitem__documented, including the two places indexing departs from NumPy deliberately.Polynomialadded to the class overview lists, which are maintained twice (package docstring andREADME.md). It was fully present in the generated API reference but missing from the introduction, so readers concluded the class did not exist.critiques/, each with its outcome recorded.Type of Change
Testing
./scripts/run-all-checks.shis green: ruff, flake8 continuation-line, pytest, pyroma, stubtest, Sphinx-W, PyMarkdown.1012 tests pass; coverage 97.22%, against a 90% floor with branch coverage on. The suite runs under
filterwarnings = ["error"]with no exemptions.Manual verification beyond the suite:
qube.pysplit was reconstructed by replaying the edits from the merge-base and verified to reproduce the finalqube.pyandextensions/__init__.pybyte-for-byte before any of it was committed.E127.mainchecked out into a temporary worktree, in the same interpreter; numbers in the critique.Potential Impacts
Public API — breaking. Three changes alter existing behavior:
Scalar.as_index_and_mask(),Scalar.int(), theirVectorcounterparts andQube.as_float()disagreed about which arguments were keyword-only; they now follow theScalarconvention. Callers passing those arguments positionally must update."(km"previously returned a dictionary and now reports the missing parenthesis.picklermodule is no longer bound ontoQubeas an attribute. It put a module, neither callable nor data, in the namespace of every object; it is documented throughdocs/module.rstinstead.Quaternion._from_matrix3_experimentalwas deleted, but it is private.Backward compatibility. Otherwise preserved.
require_writableandbroadcast_into_shapeare kept as deprecated aliases and still emit no warning.Performance. Substantially improved; see the table above and the critique. Two operations are slower than
mainby deliberate trade, both tabulated.Downstream.
rms-polymathis consumed byoopsand other RMS packages. The keyword-only changes are the ones most likely to surface there and are worth a check before release. Thesrc/layout change is internal to the repository and does not affect the installed package's import path.Checklist
ruff check,ruff format)mypypassesNotes
Two checklist items need qualifying rather than ticking silently.
ruff formatis deliberately not run. The codebase uses column-aligned assignments and imports on purpose, and the formatter collapses them.ruff checkpasses;ruff format --checkis disabled in the script with the reason recorded, andI001is ignored for the same reason.mypyis not run onsrc/.src/is deliberately unannotated — parameter and return types belong in the docstrings, and public type information is published in.pyistubs instead, whichstubtestverifies against the runtime API.[tool.mypy] strict = trueis configured fortests/only, and mypy is off by default in the check script.Known gap, not introduced here and not addressed here:
doc_pythonasks for both a warnings-as-errors and a nitpicky Sphinx build, but only-Wis wired into the script and CI.sphinx-build -nreports 581 warnings, essentially all of them docstring type words that Sphinx tries to resolve as classes (optional409,array-like55,scalar28,ndarray51). The count is identical onmainfor the shared content and unchanged by this branch's docstring work. Closing it wants anitpick_ignore_regexfor the type vocabulary plus one real fix forQubeNDIterator, which is the only unresolved target the project actually owns. Worth a follow-up PR.The size of this PR is a real cost to review, and it is worth saying plainly that it would have been better as several had the layout change not come first. The per-commit ordering is the mitigation: each commit is self-contained, passes the gate, and carries a message explaining its own reasoning.