rewrite $ref in top-level oneOf/anyOf branches during variant generation - #35
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the schema preprocessing logic to handle top-level oneOf and anyOf branches, ensuring that external references are correctly rewritten to their corresponding request or update variants. This change is reflected in the updated FulfillmentDestination models. Feedback suggests that the current implementation is too narrow and should be expanded to cover the entire variant object, including allOf and items fields, to prevent base models from being incorrectly referenced in generated request variants.
damaz91
added
the
status:needs-triage
damaz91
added
status:under-review
and removed
status:needs-triage
|
Hi @xiaoxuanz-hub, I opened #52 to address the separate array-root request variant issue tracked in #37. The two PRs are complementary: #35 fixes request-variant I intentionally kept #52 scoped and excluded the whole-variant |
2e99557 to
34c3107
Compare
damaz91
added
status:stale-review
|
Thank you for this fix. The approach here, walking the whole variant tree for external refs rather |
…ant generation (#83) * rewrite $ref in top-level oneOf/anyOf branches during variant generation (cherry picked from commit 2a9beb9) * update (cherry picked from commit e652462) * test: add coverage for composition variant propagation and rewriting (cherry picked from commit 34c3107) * fix: tolerate line-wrapped Annotated[...] in inject_array_contains _create_single_variant now rewrites $ref inside top-level oneOf/anyOf/ allOf/items branches (see the preceding two commits, cherry-picked from #35), which lengthens some array-root item type references, e.g. total.Total becomes total_create_request.TotalCreateRequest for TotalsCreateRequest. That extra length pushes ruff's formatter to wrap Annotated[...] onto multiple lines with a trailing comma before the closing bracket. inject_array_contains spliced AfterValidator(...) in right before that closing bracket without checking for the trailing comma, landing the new element after an orphaned comma with nothing between them - a SyntaxError, not just a formatting nit. Because tests/test_codegen_pipeline.py imported the generated Totals*Request classes inside a bare "except ImportError", the SyntaxError went uncaught and took the whole test module down at collection time, failing every test in the file. Fix the splice to insert after the last real token before the closing bracket instead of blindly before it, reusing an existing trailing comma when present. Also widen the import guard to catch SyntaxError so one broken generated file degrades to HAVE_SDK = False instead of failing collection for the whole module. * chore: regenerate models against pinned UCP spec 2026-04-08 Runs generate_models.sh 2026-04-08 (README compat table: SDK 0.4.x -> UCP schema 2026-04-08) against the three preceding commits (the #35 ref-rewrite cherry-picks plus the inject_array_contains fix). Only totals_create_request.py/totals_update_request.py change beyond what the cherry-picks already carried, because those two needed both fixes together: the ref rewrite (to point at total_create_request.json instead of total.json) and the trailing-comma-safe splice (to still parse once that longer reference pushes Annotated[...] onto multiple lines). --------- Co-authored-by: xiaoxuan-hub <i.travel@live.com> Co-authored-by: damaz91 <federico.damato91@gmail.com> Co-authored-by: Vishal Katyal <vishal@katyal.ai>
|
Closed by #83 |
5 participants
Description
Fix
_create_single_variantinpreprocess_schemas.pyto rewrite external$reflinks in top-leveloneOf/anyOfbranches when generating request variant schemas.Previously,
rewrite_refs_to_variantswas only called on$refs found insideproperties. Schemas that define union types via top-leveloneOf/anyOf(with noproperties), such asfulfillment_destination.json, had their$refs left pointing to the base response schemas instead of the corresponding request variant files. This caused generated wrapper models likeFulfillmentDestinationCreateRequestto incorrectly referenceShippingDestination(response, requiresid) instead ofShippingDestinationCreateRequest(request,idoptional).The fix adds a post-processing step after property handling to also rewrite
$refs in the variant's top-leveloneOf/anyOfbranches.Affected generated models:
FulfillmentDestinationCreateRequest— now wrapsShippingDestinationCreateRequest|RetailLocationCreateRequestFulfillmentDestinationUpdateRequest— now wrapsShippingDestinationUpdateRequest|RetailLocationUpdateRequestCategory (Required)
Please select one or more categories that apply to this change.
ucp-schematool (resolver, linter, validator). (Requires Maintainer approval)Related Issues
Fixes #34
Checklist
Screenshots / Logs (if applicable)