fix(api-catalog): distinguish $defs keys for same-file property refs [DEVEX-965] - #168
Merged
Conversation
…[DEVEX-965] derive_defs_key_for_path keyed external $ref targets by bare file stem only, ignoring any #/properties/<name> fragment. Refs into different properties of the same model file (e.g. FulfillmentPlan.yaml's storeId vs orderId) collapsed onto one $defs key, so listFulfillments' storeId path parameter resolved to the order-reference schema instead of its own. Fold the property path into the derived key when present so distinct properties from the same file get distinct keys, then regenerate the catalog to pick up the fix.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes API-catalog $defs key derivation so $refs that target different #/properties/<name> fragments within the same model file no longer collapse onto a single key, preventing incorrect schema resolution in generated domain catalogs (DEVEX-965).
Changes:
- Updated
derive_defs_key_for_pathto fold#/properties/...fragments into the derived$defskey (in addition to existing component-schema fragment handling). - Added unit tests covering simple and nested
#/properties/fragment key derivation. - Regenerated the API catalog outputs, updating
orders.json,fulfillments.json, andmanifest.json.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust/tools/generate-api-catalog/src/main.rs | Adjusts $defs key derivation to incorporate #/properties/... fragments and adds tests for the new behavior. |
| rust/schemas/api/orders.json | Regenerated catalog output reflecting distinct per-property $defs keys for property-fragment refs. |
| rust/schemas/api/fulfillments.json | Regenerated catalog output splitting previously-colliding property-fragment refs into distinct $defs keys. |
| rust/schemas/api/manifest.json | Regenerated manifest timestamp and domain listing output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…th key segments Joining #/properties/ path segments with a bare "_" let a literally underscored property name collide with a differently-nested property path resolving to the same suffix. Double literal underscores in each segment before joining so the encoding is unambiguous.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
rust/schemas/api/manifest.json:8
- The regeneration changed the ordering of entries under
domains, which makesmanifest.jsondiffs noisy. This is likely becauseCatalogManifest.domainsis aHashMap(rust/tools/generate-api-catalog/src/main.rs:134-137), so serialization order isn’t stable across runs. Consider switching to an ordered map (e.g.,BTreeMap/IndexMap) or sorting keys before writingmanifest.json, then regenerate so future runs are deterministic.
"domains": {
"catalog-products": {
"file": "catalog-products.json",
"title": "Catalog GraphQL API",
"endpointCount": 1
},
qcai-godaddy
approved these changes
Jul 30, 2026
This was referenced Jul 30, 2026
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.
Summary
derive_defs_key_for_pathderived a$defskey from a$reftarget's bare file stem, ignoring any#/properties/<name>fragment. Refs into different properties of the same model file (e.g.FulfillmentPlan.yaml#/properties/storeIdvs#/properties/orderId) collapsed onto one$defskey, solistFulfillments'storeIdpath parameter resolved to the wrong (order-reference) schema.#/properties/<name>path into the derived key when present, so distinct properties from the same file get distinct keys (e.g.FulfillmentPlan_storeId,FulfillmentPlan_orderId). Each path segment has its literal_doubled before joining, so a property literally named e.g.a_bcan't collide with nesteda/bsegments (refined during review).cargo run -p generate-api-catalog); onlyfulfillments.json,orders.json, andmanifest.jsonwere affected — the only domains with property-fragment refs into a shared file.Fixes DEVEX-965.
Test plan
#/properties/fragments)cargo check/cargo clippy -- -D warnings/cargo fmt --checkclean across the workspacecargo test --workspacepasses (452 tests)fulfillments.json:listFulfillments'sstoreIdparam now resolves toFulfillmentPlan_storeId("ID of the store...") instead of the order-reference schema;orderIdandfulfillmentIdsimilarly split into correct, distinct keys