fix: resolve entity local refs before inlining (python-sdk d650f0b) - #11
Merged
Conversation
Ports python-sdk PR #79, which fixes python-sdk#72 — the dangling-$ref defect this SDK reported. flatten_entity_reference deep-copied ucp.json#/$defs/entity into capability.json, payment_handler.json and service.json without resolving the entity's own document-relative refs, leaving 24 refs to "#/$defs/version" pointing at nothing. Upstream resolves the entity's local refs once, at extraction, while the body still sits in ucp.json; every copy made afterwards is self-contained and flatten_entity_reference is untouched. preprocess.ResolveLocalRefs is that port, called from Preprocess at the same point. Three behaviours are faithful to python rather than to Go taste, because goldens are byte-compared against its output: sibling keys override the resolved target, an unresolvable ref is left alone rather than erroring, and the walk continues with the caller's seen set. All three are documented at the code. ResolveRef's fallback, which resolved these refs against ucp.json and is why our emitted models were never affected, is deleted: the corpus now contains no unresolvable local ref. TestResolveRefDoesNotRescueDangling- LocalRefs pins the stricter rule in its place. Generated models change for the three affected packages: the inlined schema no longer routes through ucp.json's $defs, so Version UCPVersion becomes Version string with the pattern checked in place. Same validation strength, but a breaking change for callers naming the type. The oracle can now compile the 71 targets it previously could not, which roughly tripled differential coverage — 693 payloads across 157 types to 1,024 across 228 — and the first wider run found a real gap. Fields the emitter carries as json.RawMessage are invisible to Validate, so a payload invalid only inside one is accepted. Reported and counted every run rather than absorbed, with attribution proved from the oracle's own error locations: every leaf must fall inside a raw field or the payload stays a mismatch. Two properties are affected — error_response's ucp, to break an import cycle, and capability's extends, which has no single Go shape. Fixing that gap is a separate design decision.
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
1 participant
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.
Ports python-sdk PR #79, which fixes python-sdk#72 — the dangling-
$refdefect this SDK reported. Both issues we filed upstream are now closed and tracked here.Spec is unchanged at
a2d8bf0; only the preprocessor moved,35af25c→d650f0b. 3 of 145 preprocessed files differ, and unresolvable local refs go 24 → 0.The port
flatten_entity_referencecopieducp.json#/$defs/entityinto three documents without resolving the entity's own document-relative refs, so#/$defs/versiondangled in each new host. Upstream resolves those refs once, at extraction, while the body still sits inucp.json; every later copy is self-contained andflatten_entity_referenceitself never changed.preprocess.ResolveLocalRefsis that port, wired intoPreprocessat the same point.Three behaviours follow python rather than Go taste, because goldens are byte-compared against its output — sibling keys override the resolved target, an unresolvable ref is left alone rather than erroring, and the post-substitution walk continues with the caller's
seenset. Documented at the code, including that the last one would recurse without bound on a genuinely cyclic local ref, in python too.ResolveRef's fallback — the reason our models were never affected by the upstream bug — is deleted, since no unresolvable local ref remains.TestResolveRefDoesNotRescueDanglingLocalRefspins the stricter rule in its place.Breaking change
The inlined schema no longer routes through
ucp.json's$defs:Same validation strength — the pattern is checked in place, and
nullstill fails — but a breaking change for callers naming the type. Not tagged.What the wider coverage found
The oracle can now compile the 71 targets it previously could not, which roughly tripled what the harness compares: 693 payloads across 157 types → 1,024 across 228, skips 23 → 3.
The first run of the wider corpus found a real gap. Fields the emitter carries as
json.RawMessageare invisible toValidate, so a payload invalid only inside one is accepted —shopping/types/error_response.json'sucp(carried raw to break an import cycle) and capability'sextends(no single Go shape).It is reported and counted every run, not absorbed. Attribution is proved rather than assumed: every leaf of the oracle's rejection must fall inside a raw field, or the payload stays a mismatch. Deleting the field and revalidating would not work — these properties are required, so removal trades the real complaint for a missing-property one and every disagreement would look explained.
TestRawFieldExplainsRejectionpins both directions, including that a root-level complaint is never attributable.Closing that gap is a design decision, not part of this port.
Verification
gofmtclean,go build,go vet, root tests and the conformance module all pass.grep -c '^require' go.mod= 0.TestPreprocessMatchesGoldens= 145 files, 0 mismatches. README example re-extracted and run verbatim; output matched.