Repository navigation
Refuse a union from its shapes' own reasons, never by decoding a refused alternative - #2778
Merged
Merged
Conversation
…sed alternative UnionDecoder.decode_candidates fell back to decoding the first alternative when every shape check refused the value, so the refusal kept a cause. When the shape check and the decoder disagreed, that decode succeeded and the union accepted a value its own shape check had refused: two answers to "does this value fit", and the fallback took whichever said yes. - ValueDecoder.shape_refusal states why fits_wire refused a value. It never decides acceptance; fits_wire stays the only answer. - decode_candidates raises the union's shape_refusal when no alternative fits, naming each alternative's reason and decoding nothing; the first reason is the cause. - Records, family records, leaf documents, arrays, fixed tuples and represented values state their own reasons. RecordDecoder.checked, FamilyRecordDecoder.member, the array readers and FixedTupleDecoder.plan_items now refuse through the same method, so each refusal text is written once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
trissim
marked this pull request as ready for review
October 23, 2026 13:33
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
debt-src at 76f268f, every finding: |
shape_refusal added a fourth _member_decoder(self.target) call to _LeafDocumentDecoder, and with it a fourth mypy arg-type error (debt-src TypeErrors 114 -> 115). The leaf's member decoder is one fact; the leaf property now declares it and fits_wire, shape_refusal, decode_document and rewrite_document derive from it. Mypy on field_codec.py: 112 (base 114). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
UnionDecoder.decode_candidates(field_codec.py, from #2763 f228d09) fell back to decoding the first alternative when every shape check refused the value:tuple(filter(fits, self.alternatives)) or self.alternatives[:1]. That decode existed only to give the refusal a cause. But wheneverfits_wireand the decoder disagree (#2763 fixed one such case, the PicontextUsage.percentkey), the fallback decode succeeds and the union silently accepts a value its own shape check refused.Slop: IDEN-3. The fact is whether a wire value fits an alternative. It is answered twice, at
decode_candidatesbyfitsand bydecode, and the fallback takes whichever accepts. It should derive from the decoder'sfits_wirealone.Change
ValueDecoder.shape_refusal(data)states whyfits_wirerefused. It never decides acceptance;fits_wireremains the only answer.decode_candidatesdeletesor self.alternatives[:1]. When no alternative fits, it raises the union'sshape_refusal, which names every alternative's reason with the first as the cause, and decodes nothing. A disagreement between a shape check and its decoder is now a loud refusal, never a silent accept.RecordProblemswith missing and unknown keys;RecordDecoder.checked,FamilyRecordDecoder.member, the array readers andFixedTupleDecoder.plan_itemsnow refuse through the same method, so each refusal text is written once.fits_wireis untouched, and refusals are built only on the error path.Tests
tests/test_field_codec.py: 64 passed, includingtest_union_success_releases_rejected_payload_and_failure_keeps_first_cause. Its first-cause message now comes from the shape refusal.shape_refusal: 1 failed (that test).🤖 Generated with Claude Code