Skip to content

fix: enforce conditional numeric bounds in generated models - #71

Merged
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
yingzhehu-TT:fix/enforce-conditional-numeric-bounds
Aug 13, 2026
Merged

fix: enforce conditional numeric bounds in generated models#71
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
yingzhehu-TT:fix/enforce-conditional-numeric-bounds

Conversation

@yingzhehu-TT

Copy link
Copy Markdown
Contributor

Summary

Restore JSON Schema if/then numeric bounds, which
datamodel-code-generator drops, by injecting a model_validator alongside the
existing conditional-required family.

Motivation

total.json constrains amount by type: discount / items_discount must be
exclusiveMaximum: 0, and subtotal / fulfillment / tax / fee must be
minimum: 0. The generated Total carries no validator at all, so:

Total(type="discount", amount=500)   # accepted — a positive discount
Total(type="tax", amount=-999)       # accepted — a negative tax

find_conditional_required already handles a then that adds required fields,
but returns early on a then that narrows a range, so this family was never
covered.

Scope

Mirrors the conditional-required trio (find_ / inject_ / _patch_) so both
read the same way. Two differences worth calling out:

  • allOf scoping. These rules are carried as allOf branches, which have no
    sibling properties of their own — the object they constrain is the enclosing
    schema. The scan therefore threads the enclosing property set into each branch.
    A rule is still rejected unless it has a single const/enum discriminator and
    a then containing only numeric-bound keywords; anything else is skipped with a
    warning rather than approximated.
  • Request variants are not a warning. total_create_request.json strips every
    property, so the rule names fields the class does not have. That is inapplicable
    rather than malformed, and is skipped silently — otherwise every generation run
    would print four spurious warnings.

type remains an open vocabulary: a value carrying no rule (total, or a vendor
value) is unconstrained, and a None amount is not checked.

Validation

python -m unittest discover -s tests — 77 passed (72 before; 5 new).
ruff check adds no diagnostic outside the D,E501 set this repo ignores in
pre-commit; ruff format clean.

Regeneration against the pinned 2026-04-08 spec touches only total.py and
totals.py. Running postprocess_models.py a second time leaves the output
byte-identical.

Behaviour of the generated model, all verified:

input before after
type="discount", amount=500 accepted rejected
type="items_discount", amount=1 accepted rejected
type="tax", amount=-999 accepted rejected
type="fee", amount=-1 accepted rejected
type="discount", amount=-500 accepted accepted
type="tax", amount=100 accepted accepted
type="subtotal", amount=0 accepted accepted
type="total", amount=-5 (no rule) accepted accepted
type="custom_vendor_type", amount=-7 accepted accepted

js-sdk already enforces the same two rules on TotalResponseSchema, so this
closes a cross-SDK divergence.

@damaz91 damaz91 added status:needs-triage Signal that the PR is ready for human triage status:under-review and removed status:needs-triage Signal that the PR is ready for human triage labels Aug 13, 2026
@damaz91
damaz91 merged commit 9a4ff69 into Universal-Commerce-Protocol:main Aug 13, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants