Skip to content

fix(compilers/openapi): stop rounding raw-preserved numbers - #244

Merged
OmarAlJarrah merged 2 commits into
mainfrom
fix/openapi-raw-number-fidelity
Aug 4, 2026
Merged

fix(compilers/openapi): stop rounding raw-preserved numbers#244
OmarAlJarrah merged 2 commits into
mainfrom
fix/openapi-raw-number-fidelity

Conversation

@OmarAlJarrah

@OmarAlJarrah OmarAlJarrah commented Aug 3, 2026

Copy link
Copy Markdown
Member

Summary

annotation.RawFromNode decoded a YAML node into any and re-marshalled it as JSON. 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:

source preserved as (before)
12345678901234567890123 1.2345678901234568e+22
1.000000000000000000001 1
-9223372036854775809 -9223372036854776000
1.10 1.1

One 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 the Value/BigVal channel; 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 the Value channel, and that resolves YAML's integer bases, so 0o17 still 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

Deliberately out of scope

Timestamps still normalize to RFC 3339 and !!binary still 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 the Value doc 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_PreservesMergeAndOrderingSemantics and TestRawFromNode_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.
  • Bounds: an alias cycle, a multiplicative alias chain, a << 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.yaml gains a preserved property carrying high-precision numbers in an extension and under not. 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.Tag in place of ShortTag. 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.sh at 100% of 4469 statements.

Fixes #32

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
OmarAlJarrah force-pushed the fix/openapi-raw-number-fidelity branch from 66e237f to 15f6f86 Compare August 4, 2026 10:41
@OmarAlJarrah
OmarAlJarrah merged commit 7d860e2 into main Aug 4, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the fix/openapi-raw-number-fidelity branch August 4, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openapi: raw preservation routes numbers through float64, corrupting extensions and preserved keywords

1 participant