fix(autodiff): close four gaps between the two expansion passes - #139
Open
AlekseiChirkovVention wants to merge 3 commits into
Open
fix(autodiff): close four gaps between the two expansion passes#139AlekseiChirkovVention wants to merge 3 commits into
AlekseiChirkovVention wants to merge 3 commits into
Conversation
Each case was reproduced against the current code before being written, so none is speculative. The constant reader returns a descriptor that can disagree with the node's own declared type, though the declaration is documented as the single authority a consumer materializes from. The agreement test could not catch it: it built both sides from one dict. It now spells them independently and asserts a disagreeing declaration is rejected. A mean whose operand declares dimensions too large to convert passes every clause of the supported-reduction predicate and then raises a bare builtin from the reciprocal. A large-but-convertible count is pinned as still expanding, so the boundary is explicit rather than implied. The two passes disagree about which identifiers a minted one must avoid. The gradient pass indexes values a program merely reads; the forward pass indexes only produced values, so a minted identifier can alias a value a node reads and silently rewire that consumer instead of failing closed. Four cases pin both passes against both positions. Two further cases pin the handler requirement the documentation calls the most likely omission: a rank-reducing derivative program lowered without a reshape handler must fail closed on the seed reshape the rewrite leaves in place. That behaviour is already correct and was covered by nothing.
The gradient read-position case fed the reserved identifier into the chain it was expanding, which stopped the region matching, so nothing was minted and the collision it asserts could never fire. Read the identifier from a node outside the region instead. Assertions unchanged.
The constant reader now enforces the agreement its node's declaration is documented to be the authority for. A descriptor disagreeing with the declared dtype or shape is rejected naming the node, so a consumer cannot materialize one tensor while the graph declares another. The check runs after parameter validation, so every existing rejection keeps its category, and a node with no declaration still reads. An operand declaring dimensions too large to convert now fails validation naming the node and the count, rather than raising a bare builtin from the reciprocal. Such a shape violates no clause -- it is a positive integer -- so this is a limit on what the expansion can express, which is what the residual category covers. The threshold is the conversion limit, not the exactness limit: a count above the latter still expands to well within the documented tolerance, and refusing it would narrow the supported domain rather than guard it. Both passes now derive collision candidates from one helper covering every identifier an artifact mentions -- produced, read, declared as an input, and declared as an output. Previously each half indexed a different set, so the forward pass could mint an identifier a node already read and silently rewire that consumer to a generated constant instead of failing closed. Sharing one helper removes the contradiction rather than correcting it in two places.
AlekseiChirkovVention
force-pushed
the
fix/expansion-artifact-guards
branch
from
August 28, 2026 08:02
48775b4 to
18292ee
Compare
code-tc
approved these changes
Aug 28, 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
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.
Close four defects that only became visible once the whole change was read as one artifact.
Why these were not caught earlier
Each branch in this series was reviewed against its own diff. That bounded review is what kept
each one tractable, but it means no reviewer was positioned to see an inconsistency between
two branches. All four fixes here are of that kind, and each was reproduced against the code
before being approved — none is speculative.
The four
The two passes held contradictory contracts for the same invariant. The gradient path
indexes value ids a program merely reads and documents why; the forward path indexed only
values it produces. So a generated identifier could collide with a value a node already reads,
and the result was not an error — the consumer was silently rewired to read a generated
constant instead. Both passes now derive collision candidates from one shared helper covering
every identifier an artifact mentions: produced, read, declared as an input, declared as an
output. The gradient side gained a position it was also missing.
Sharing one helper is the point. Correcting the forward side by copying the gradient side's
logic into a second place would have left the same drift to recur.
The constant reader was not authoritative about its own node's declared type. A node
declaring one dtype and shape while its parameters said another read back as the parameters,
with no complaint — though the declaration is documented as the single authority a consumer
materializes from. A handler would have allocated one tensor while the graph declared another.
The cross-check runs after parameter validation, so every existing rejection keeps its category
and a node with no declaration still reads.
An oversized declared shape raised a bare builtin. An operand declaring very large
dimensions passes every clause of the supported-reduction predicate and then overflowed inside
the reciprocal. It now fails validation naming the node and the count.
The threshold is deliberate: counts are rejected only when they cannot be converted, not when
they exceed the exactness limit. A count above the latter still expands to well within the
documented tolerance, so rejecting there would have narrowed the supported domain rather than
guarded it — a behaviour change wearing a guard's clothing.
A documented requirement had no test. The documentation calls out that the rank-reducing
tier needs a reshape handler in both artifacts, because the derivative transform's own seed
reshape survives expansion. That behaviour was correct and pinned by nothing; the missing
fail-closed case now covers it.
Why this is safe on top of eight reviewed branches
These are guards. For any well-formed artifact both passes emit exactly what they emitted
before, and every pre-existing test passes unmodified — that is the regression signal, and
review confirmed it by expanding sixteen well-formed artifacts through all four entry points
against the previous implementation and finding zero differences.
Focused suite 895 → 910, exactly the new cases. Broad non-integration suite: 1161 passed,
9 failed, 1 skipped — the same 9 environment-dependent failures, identical in composition, that
this series has carried from its first branch.
Two test amendments, both disclosed
One pre-existing test was strengthened, as this fix-task's own plan required: the agreement test
built both sides from a single dict and so could not fail. It now spells them independently and
asserts a disagreeing declaration is rejected. The three original assertions survive verbatim.
One test authored earlier on this branch had its fixture corrected: it fed the reserved
identifier into the very region being expanded, which stopped the region matching, so nothing
was minted and the collision it asserts could never fire. It was red, but for the wrong reason,
and would have stayed red permanently. Assertions unchanged. Worth stating plainly, because a
self-authored test that quietly stops testing what it claims is the same class of defect this
branch exists to close.
Stack position
Stack position 9 of 9 — based on
chore/mean-expansion-regression-coverage.The tip of the series. Cannot merge into
mainbefore the eight branches below it.