fix(compilers/openapi): stop rounding raw-preserved numbers - #244
Merged
Conversation
RawFromNode decoded a YAML node into `any` and re-marshalled it. yaml.v3 resolves an out-of-int64 integer and every decimal into float64, so the one channel whose documented promise is verbatim preservation quietly rewrote its numbers: a 23-digit extension value came back as 1.2345678901234568e+22, and 1.000000000000000000001 came back as 1. That reached every raw-preservation site — x-* extensions, oneOf/anyOf sibling preservation, the validation-only carve-out, and tuple items-after-prefix — because all four share this one conversion. Render the node tree directly instead, taking numeric scalars from their source text through value.NumericLiteral, which already backs the Value/BigVal channel and resolves YAML's integer bases (0o17 is still 15, not 17). Walking the tree means reimplementing what the decode did structurally, so merge-key expansion, duplicate-key rejection, non-string-key refusal, alias following and sorted object keys are all kept, and pinned by tests. Two behaviour changes beyond the rounding. An explicitly tagged huge integer (`!!int 12345678901234567890123`) used to be dropped with a diagnostic and is now preserved. A literal whose rendered form is not JSON is refused rather than spliced into the document, which keeps the open NewBigVal binary-exponent gap (#45) out of the IR without settling it here. Timestamps and !!binary scalars are still rewritten rather than kept verbatim. That is a different mechanism from the rounding and is filed as #242 rather than swept in. Fixes #32
Reviewing the walk against the decode it replaced, rather than against
its own tests, turned up three inputs the decode converted and the walk
refused. Each one dropped a construct the source did write, behind a
"could not be serialized" warning.
A mapping key may be an alias. yaml.v3 resolves it before deciding
whether the mapping is string-keyed, so `&k mykey` used as `{*k: v}`
produced {"mykey":"v"}; mapKey required a scalar and refused it. Alias
resolution now goes through one bounded helper shared with the merge
reader, which also gives the merge path a chain bound it lacked.
A tag yaml.v3 can resolve no type for — `!foo`, `!!python/object`,
`!!set` — decodes to the scalar's text rather than failing, so refusing
it lost an extension value for want of a tag nothing reads anyway. The
default arm now keeps the text.
The tag is read through ShortTag so an untagged node resolves by its
value the way that decode would have.
Mutation-tested: reverting any of the three reddens a test that names it.
The equivalence corpus gains the shapes that found them, so the next
walk-versus-decode difference has somewhere to show up.
OmarAlJarrah
force-pushed
the
fix/openapi-raw-number-fidelity
branch
from
August 4, 2026 10:41
66e237f to
15f6f86
Compare
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
annotation.RawFromNodedecoded a YAML node intoanyand re-marshalled it as JSON. yaml.v3 resolves an out-of-int64 integer and every decimal intofloat64, so the one channel whose documented promise is verbatim preservation quietly rewrote its numbers:123456789012345678901231.2345678901234568e+221.0000000000000000000011-9223372036854775809-92233720368547760001.101.1One conversion backs every raw-preservation site, so all four were affected:
x-*extensions, oneOf/anyOf sibling preservation, the §4.7 validation-only carve-out, and tuple items-after-prefix. #23 fixed the same class of bug in theValue/BigValchannel; this is the raw channel's version of it.The fix renders the node tree directly, taking numeric scalars from their source text via
value.NumericLiteral— the helper that already backs theValuechannel, and that resolves YAML's integer bases, so0o17still reads 15 rather than 17.Walking the tree means the structural work the decode used to do has to be done here, so merge-key (
<<) expansion, duplicate-key rejection, non-string-key refusal, alias following — including an alias used as a key — sorted object keys, and the fallback that keeps a scalar whose tag yaml.v3 resolves no type for are all reproduced deliberately and pinned by tests.Three of those were found by reviewing the walk against the decode rather than against its own tests: an alias key, an unresolvable tag (
!foo,!!python/object), and an untagged node all converted before and were refused after. Each dropped a construct the source did write, behind a "could not be serialized" warning. They are fixed in the second commit, and the equivalence corpus gained the shapes that found them.ir.UnmodeledEntry.Value's doc comment described the rounding as intended behaviour ("normalizes … number spelling"), so it is corrected to say what now holds.Behaviour changes beyond the rounding
!!int 12345678901234567890123) used to fail to convert and be dropped with a diagnostic. It is now preserved exactly.NewBigValaccepts binary-exponent literals and stores them verbatim (ir: NewBigVal accepts binary-exponent and leading-zero literals, storing non-JSON canonical forms #45, open), so!!float 1p4would otherwise have put1p4into aRawValue. Refusing matches what the old decode did with that input and leaves ir: NewBigVal accepts binary-exponent and leading-zero literals, storing non-JSON canonical forms #45 to be settled on its own terms.Deliberately out of scope
Timestamps still normalize to RFC 3339 and
!!binarystill carries decoded bytes rather than its base64 text, which costs ill-formed UTF-8 its identity to U+FFFD. Those are a different mechanism from the float64 rounding and are filed as #242, noted in the code and in theValuedoc comment rather than swept in here.Test plan
TestRawFromNode_KeepsNumericLiteralsExact— the regression, over the spellings the old conversion rewrote, including a significant trailing zero and both integer boundaries.TestRawFromNode_ResolvesYAMLIntegerBases— the half that must not become verbatim, so the fix cannot be "read the source text" applied too far.TestRawFromNode_PreservesMergeAndOrderingSemanticsandTestRawFromNode_RefusesWhatJSONCannotName— the structural behaviour the walk had to reimplement, and the refusals compilers/openapi: a failed raw conversion preserves nothing while its diagnostic claims otherwise #144 depends on.TestRawFromNode_DiffersFromTheOldDecodeOnlyInNumbers— the equivalence oracle. The old conversion is kept in the test file, and the claim is that rounding the new output through float64 reproduces it. Reading two implementations cannot show they agree; this can.<<chain that recurses without passing back through the node walk, and an alias chain longer than any source could mean.testdata/conformance/openapi/numeric-precision.yamlgains apreservedproperty carrying high-precision numbers in an extension and undernot. The corpus had no case with a number float64 cannot hold, which is why nothing caught this.Verified by mutation rather than by inspection — each of these was planted and confirmed to redden: reverting to the float64 decode, inverting merge precedence, dropping the duplicate-key check, dropping key sorting, accepting non-string keys, removing the bounds (which turns a clean refusal into a stack overflow), un-resolving alias keys, refusing unresolvable tags again, and reading
n.Tagin place ofShortTag. The last of those initially survived, because the row meant to catch it was testing the parser rather than the dispatch; it is now covered by a case that distinguishes them. Restoring the original bug reddens the conformance case on all four preserved values, so the fixture is load-bearing rather than decorative.Full gate green:
gofmt,go vet,golangci-lint,go build, and./scripts/check-coverage.shat 100% of 4469 statements.Fixes #32