Ig/anyof review suggestions - #760
Merged
raginpirate merged 3 commits intoAug 21, 2026
Merged
Conversation
Both examples elided id, handler_id, and type behind a "more required fields" comment, and one also omitted credential.type. They validated only because shipped ucp-schema never applied the self-root allOf requirements in selected_payment_instrument, so payment.instruments[] was never checked against payment_instrument's required members. Showing the fields costs one line each, keeps both examples validated once that resolver bug is fixed, and removes a comment that promised required fields while showing none.
An empty Object Constraint is a valid no-op at every other position, but an empty anyOf branch is satisfied by every object, so it neutralizes the alternation rather than doing nothing. The grammar should not admit a branch that silently disables the alternation it belongs to. A minProperties sibling on the recursive $ref is legal 2020-12 composition: siblings to $ref apply alongside the referenced schema, so the branch stays an ordinary Object Constraint while gaining the non-empty requirement, at every nesting depth. Mirrors the constraint into both grammar tables, since the schema and its companion prose are one unit.
The undiscriminated form accepts nonsense: a credential declaring any card_number_type at all satisfies the cvc branch as long as it carries a cvc, so the example teaches an idiom that does not enforce what the surrounding prose claims. Pinning card_number_type with const in each branch, and naming it in that branch's required, makes a branch match only the shape it describes. Documents the consequence that pinning creates: because every branch pins the discriminator, the branch set also closes the accepted values, so a dpan credential valid under card_credential.json satisfies neither branch. That closure is the rule a per-branch conditional cannot express, and it is load-bearing enough to state. Replaces the "would restate the path" justification, which understated the case. Request Constraints conjoin, so two separately targeted constraints would require both cvc and cryptogram; the real alternative is discriminating in the path filter, which puts conditional logic in the selector. Adds the two grammar mechanics the example relies on next to the composition rules rather than in the Business guidelines: branches are alternatives rather than a partition, and properties asserts nothing about an absent member. Both are consequences of how the grammar composes, not conformance requirements, so they carry no BCP 14 keywords and belong with the grammar they describe.
raginpirate
merged commit Aug 21, 2026
6a74941
into
raginpirate/anyof-constraint
4 of 7 checks passed
raginpirate
pushed a commit
that referenced
this pull request
Aug 21, 2026
* docs: complete abridged payment instrument examples Both examples elided id, handler_id, and type behind a "more required fields" comment, and one also omitted credential.type. They validated only because shipped ucp-schema never applied the self-root allOf requirements in selected_payment_instrument, so payment.instruments[] was never checked against payment_instrument's required members. Showing the fields costs one line each, keeps both examples validated once that resolver bug is fixed, and removes a comment that promised required fields while showing none. * feat: require non-empty anyOf branches in the constraint grammar An empty Object Constraint is a valid no-op at every other position, but an empty anyOf branch is satisfied by every object, so it neutralizes the alternation rather than doing nothing. The grammar should not admit a branch that silently disables the alternation it belongs to. A minProperties sibling on the recursive $ref is legal 2020-12 composition: siblings to $ref apply alongside the referenced schema, so the branch stays an ordinary Object Constraint while gaining the non-empty requirement, at every nesting depth. Mirrors the constraint into both grammar tables, since the schema and its companion prose are one unit. * docs: discriminate the anyOf credential example The undiscriminated form accepts nonsense: a credential declaring any card_number_type at all satisfies the cvc branch as long as it carries a cvc, so the example teaches an idiom that does not enforce what the surrounding prose claims. Pinning card_number_type with const in each branch, and naming it in that branch's required, makes a branch match only the shape it describes. Documents the consequence that pinning creates: because every branch pins the discriminator, the branch set also closes the accepted values, so a dpan credential valid under card_credential.json satisfies neither branch. That closure is the rule a per-branch conditional cannot express, and it is load-bearing enough to state. Replaces the "would restate the path" justification, which understated the case. Request Constraints conjoin, so two separately targeted constraints would require both cvc and cryptogram; the real alternative is discriminating in the path filter, which puts conditional logic in the selector. Adds the two grammar mechanics the example relies on next to the composition rules rather than in the Business guidelines: branches are alternatives rather than a partition, and properties asserts nothing about an absent member. Both are consequences of how the grammar composes, not conformance requirements, so they carry no BCP 14 keywords and belong with the grammar they describe.
waynemak
added
the
area:payments
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
3 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.
Nits/suggestions for #757.
docs: complete abridged payment instrument examples— Twocompleteexamples elidedid,handler_id, andtypebehind a// ... more instrument required fieldcomment, and one also omittedcredential.type. They only validate because shippeducp-schemanever applies the self-rootallOfrequirements inselected_payment_instrument, sopayment.instruments[]is never checked againstpayment_instrument's required members. ucp-schema#44 fixes that; built against its head, this branch's base reports316 passed, 2 failedat exactly these two lines. Completing them costs one line each.feat: require non-empty anyOf branches in the constraint grammar— An empty Object Constraint is a valid no-op everywhere else, but an emptyanyOfbranch is satisfied by every object, so it neutralizes the alternation rather than doing nothing. AddsminProperties: 1as a sibling to the recursive$refin both grammar schemas (legal 2020-12 sibling composition, applies at every nesting depth) and mirrors the constraint into both grammar tables.docs: discriminate the anyOf credential example— The example's undiscriminated form does not enforce what the surrounding prose claims:{"card_number_type": "anything", "cvc": "1"}satisfies thecvcbranch. Pinscard_number_typewithconstin each branch and names it in that branch'srequired, matching the idiom #424 already uses inpayment/guide.md. Uses the existing closed enum incard_credential.json, so it introduces no dependency on #424's new credential types.Also documents the consequence that pinning creates — because every branch pins the discriminator, the branch set closes the accepted values, so a
dpancredential valid undercard_credential.jsonsatisfies neither branch — and replaces the "would restate the path" justification. Request Constraints conjoin, so two separately targeted constraints would require bothcvcandcryptogram; the real alternative is discriminating in the path filter, which puts conditional logic in the selector.