[gsmith85]

Summary

Implements Task 6 (PR 3a) of the two-layer ucp-schema code-generation pipeline: pre-slicing inline conditional variant hoisting (src/codegen/reify.rs) and post-slicing AST shape normalizers (src/codegen/normalizer.rs), stacked on #85.

Changes

• Pre-Slicing Inline Conditional Variant Hoisting (src/codegen/reify.rs):
  • Adds hoist_inline_conditional_variants and derive_variant_name before Stage 4 (slice_all_schemas).
  • Inspects allOf branches containing if: { properties: { <disc>: { const | enum([single]) } } } with an inline then object (!then.contains_key("$ref")) that introduces new properties or overrides a property's $ref or items.$ref (skipping scalar value-range constraints such as exclusiveMaximum: 0 in Total).
  • Synthesizes standalone top-level definitions (ShippingMethod, PickupMethod, Oauth2Provider), deep-merging inline then.properties over inherited parent properties (items, type, ucp_request, ucp_response, uniqueItems), propagating parent dependentRequired constraints, setting the discriminator const, and rewriting then to {"$ref": "#/$defs/<VariantName>"} so Stage 4 directional slicing slices hoisted root variants alongside their parent union.
• Post-Slicing Codegen AST Shape Normalizers (src/codegen/normalizer.rs):
  • strip_contains_allof: Removes allOf entries that only assert array contains constraints (such as Totals.allOf) so downstream code generators emit clean list types (list[Total]) instead of wrapper classes.
  • distribute_bare_anyof_properties: Distributes parent properties and required into bare anyOf: [{required: [...]}] constraint branches (ValueConstraint, StayCreateRequest), isolating $ref properties (allOf: [{"$ref": ...}]) when extra keys are merged.
  • normalize_scalar_or_array_unions: Rewrites oneOf unions of a scalar/ref branch S and array<S> (such as CapabilityBase.properties.extends) to anyOf: [S, array<S>] so both single values and arrays remain valid without exclusive oneOf evaluation overhead.
  • normalize_presence_if_then_to_dependent_required: Lowers property-presence if (required: [...] or anyOf: [{required: [...]}]) + then: {required: [...]} rules inside allOf into standard JSON Schema 2020-12 dependentRequired maps (Location.timezone when hours or exception_hours is present).
  • flatten_single_object_allof: Fixed-point pass that flattens object schemas with allOf references to non-union base definitions (ShippingDestination, LocationDestination, CardPaymentInstrument, LookupLocation, UcpPlatformSchema, Totals.items), merging properties (additionalProperties, items, const, default, uniqueItems) and object-level constraints (dependentRequired, minProperties, maxProperties, propertyNames) while preserving top-level non-$ref if/then/not constraints.

Stacked PR Series Roadmap

1. PR 1a (feat(codegen): add identifier naming, UCP keyword stripping, and $defs ref rewriting #81): Identifier qualification, UCP keyword stripping, $ref rewriting (src/codegen/normalizer.rs)
2. PR 1b (feat(codegen): add directional schema slicing and $ref alignment #82): Directional schema slicing & $ref alignment (src/codegen/normalizer.rs)
3. PR 1c (feat(codegen): add capability reachability, composition, and generate_types() API #83): Capability/extension reachability, composition & generate_types() library API (src/codegen/{mod,reachability,hoist,compose}.rs)
4. PR 1d (feat(cli): add generate-types subcommand #84): CLI generate-types subcommand (src/bin/ucp-schema.rs, tests/generate_types_test.rs)
5. PR 2 (feat(codegen): add self-contained .well-known/ucp profile scoping for generate-types #85): Self-contained .well-known/ucp --profile scoping (src/codegen/profile.rs)
6. PR 3a (this PR): Codegen AST shape normalizers & inline conditional variant hoisting (src/codegen/normalizer.rs, src/codegen/reify.rs)
7. PR 3b (feat(codegen): lower open string discriminator unions into ordered anyOf with <Union>Base #87): Open string discriminator union lowering (<Union>Base) (src/codegen/reify.rs)
8. PR 3c (feat(codegen): discover and register decentralized extended subtypes into open unions #88): Cross-file extended subtype discovery & registration (src/codegen/reify.rs)
9. PR 4a (python-sdk#101): Python SDK direct JSON Schema codegen (generate_models.sh, postprocess_models.py, src/ucp_sdk/models/)
10. PR 4b (js-sdk#87): JS/TS SDK direct JSON Schema codegen (generate_models.sh, scripts/generate-zod-from-types.mjs, src/spec_generated.ts)
11. PR 5 (feat(openapi): add profile-driven OpenAPI 3.1 service and type binding (generate-openapi) #89): OpenAPI 3.1 REST service & type binding (ucp-schema generate-openapi, src/openapi/mod.rs)

Test Plan

• cargo fmt --check
• cargo test --all-targets
• cargo clippy --all-targets -- -D warnings

[damaz91]

Great work on Stage 4 inline conditional variant hoisting and Stage 6 AST shape normalizers! Running hoist_inline_conditional_variants before Stage 5 directional slicing and distinguishing structural shape variants from scalar if/then constraints is a very clean design.

Left a few inline comments on edge cases around $ref property merging, discriminator UCP_ANNOTATIONS handling, parent allOf preservation during variant hoisting, and compound if conditions.

[ptiper]

Approving. Nice work, this stage is well put together.

I used an AI assistant (Antigravity) to help with this review. It compared this PR against its own base branch (feat/codegen-profile-mode), not main, and I ran the full test suite and clippy on the branch: all green. The comments below are about schema shapes that do not exist today but could show up as new verticals are added.

Two inline comments that I think are worth a small fix plus a test. Three smaller notes here so they do not get lost:

• Inline branch titles can collide: used_titles is written to but never checked before a title is stamped (around L1340). Three anyOf branches requiring ["id","a"], ["id","b"], ["code"] give the first two the same title <Parent>Id. Falling back to the next field, then Variant{n}, when a title is taken would fix it.
• singularize_pascal: the "ses" rule removes two letters, which turns Purchases, Releases, Leases, Licenses, Cases into Purchas, Releas, Leas, Licens, Cas. A small exception for ases / eases / enses would cover it.
• attach_default_if_const gives integer constants "type": "number". Using "integer" when the value has no fraction would let typed generators emit int.

All fine to defer if you want to land the stack first; say so and I will open issues to track them.

[ptiper]

This returns false as soon as the object has a top-level if, before looking at properties, allOf, or $ref. So whether a type gets a request version depends on whether the author put an if/then at the top level or inside allOf.

Example in the corpus: common/types/location.json has a top-level if and inherits id with "ucp_request": "required" from location_summary.json. Its request version is not empty, but LocationCreateRequest / LocationUpdateRequest are dropped here (and the test at generate_types_test.rs:396 expects that). Harmless today because Location only appears in responses. A future capability that accepts a Location in a request would silently get no request type. Stage 8 already removes request types nobody references, so this early return is not needed for that.

Suggestion: remove the early return and let if / contains simply not count as content:

let has_props = ...;
let has_composition = ["allOf", "oneOf", "anyOf"].iter().any(|k| {
    obj.get(*k).and_then(Value::as_array)
        .is_some_and(|a| a.iter().any(is_non_empty_request_slice))
});
let has_ref = ...;
has_props || has_composition || has_ref

Total and Totals still come out empty with this. Happy to be corrected if the early return is there for another reason.

[gsmith85]

Fixed in 11d30b1 — removed the early return on if/contains in is_non_empty_request_slice so root-level if schemas with properties/refs keep their request slices while pure if/then and contains branches continue to return false.

[ptiper]

When a branch is flattened, only properties, required, dependentRequired, minProperties, maxProperties and propertyNames are carried over. Anything else on that branch is silently lost: not, dependentSchemas, patternProperties, and an explicit additionalProperties: false (the result then gets additionalProperties: true).

No schema today has these inside a flattenable branch, but allOf: [{ "not": { "required": ["a", "b"] } }] ("not both at once") is a very common pattern and would be the first thing a new vertical reaches for.

Suggestion: either add not, dependentSchemas, patternProperties (and additionalProperties == false) to the list of keywords that make a branch non-flattenable, here and in is_flattenable_base_object, so the branch stays in kept_all_of; or carry them over in merge_props_and_reqs_from_object. A unit test with a not branch that must survive would lock it in.

[gsmith85]

Fixed in 11d30b1 — is_inline_flattenable and is_flattenable_base_object now exclude not, dependentSchemas, patternProperties, unevaluatedProperties, and non-true additionalProperties so those constraint branches stay in kept_all_of (covered by unit test).

[gsmith85]

Thanks @ptiper! Also addressed the 3 notes from your review body in 11d30b1:

1. derive_branch_title now checks used_titles across candidate suffixes so multi-branch anyOf/oneOf unions with common_req = ∅ never collide on the same branch title.
2. singularize_pascal now only strips 2 chars for -sses, -uses, -iases, and -yses so silent-e plurals (Purchases, Releases, Licenses, Cases) singularize cleanly.
3. attach_default_if_const now infers "type": "integer" for integral const numbers.