Skip to content

fix(compilers/openapi): keep raw timestamp and binary spelling - #246

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

fix(compilers/openapi): keep raw timestamp and binary spelling#246
OmarAlJarrah merged 2 commits into
mainfrom
fix/openapi-raw-scalar-verbatim

Conversation

@OmarAlJarrah

@OmarAlJarrah OmarAlJarrah commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

ir.UnmodeledEntry.Value is the channel whose documented promise is verbatim
preservation, and it rewrote the two YAML scalars JSON has no type for. Neither
source could be recovered from what reached the IR.

A timestamp was rendered as the RFC 3339 instant it resolves to, so 2021-1-1
arrived as "2021-01-01T00:00:00Z" — a padding, a time and a zone the source
never wrote. A !!binary was rendered as its decoded bytes in a JSON string,
which lost the base64 spelling on every value and lost the data outright on any
byte that is not valid UTF-8: /w== is 0xFF, encoding/json substitutes
U+FFFD for it, and nothing downstream can tell that from a source that really
wrote U+FFFD.

Every raw-preservation site reaches this through the one function, RawFromNode,
so all of them carried it. To read the list off the tree rather than trust a
recital of it:

grep -rn 'RawFromNode\|Preserve(c\|PreserveNode(c' --include='*.go' compilers/ | grep -v _test

which is x-* extensions, the §4.7 validation-only keywords, openapi:allOf/<i>
branch preservation, openapi:items-after-prefix, and contentSchema.

Both scalars now keep the text the source wrote. The resolved form is derivable
from the spelling and the spelling is not derivable from the resolved form,
which is the reasoning the sibling ir.Value channel already applies to a
timestamp — the two channels disagreed for the same source scalar until now.
ir.UnmodeledEntry.Value's doc comment states the rule, since that comment is
where a consumer learns what the field guarantees.

Deliberately out of scope

The decode stays, as the tag check it has always been: a scalar that does not
satisfy the type it declares (!!timestamp notadate, !!binary "###") is still
refused and still costs the caller the whole construct. Only the kept spelling
moved, so nothing about what the compiler accepts changed here.

That refusal is inherited rather than chosen — it exists because the conversion
used to decode these scalars to produce its output — and it is arguably wrong
now that nothing consumes the decoded value. That is a question about what the
walk accepts rather than what it preserves, so it is filed as #245 and the
code comment on the arm points there rather than settling it as a side effect.

Test plan

  • New unit rows pin the spelling for both tags: single-digit and padded dates, a
    space-separated datetime, a written zone, an explicit !!timestamp, flow and
    block !!binary, and the 0xFF case.
  • The differential oracle against the pre-openapi: raw preservation routes numbers through float64, corrupting extensions and preserved keywords #32 decode kept its force rather than
    losing the rows that now diverge. rawDivergences records each one with both
    spellings — the new one because it is the preservation this change exists for,
    the old one so a row that quietly stopped diverging cannot leave the oracle
    asserting nothing. A companion test fails if a recorded row is not in the
    corpus, so a divergence cannot be parked in the map and never evaluated.
    Two timestamp spellings the old decode already reproduced exactly are kept as
    equivalence rows, which is what keeps the divergence set tight.
  • The conformance corpus was blind to this. No golden held a normalized
    timestamp and no !!binary reached the raw channel anywhere, so the corpus
    could not have caught the rewrite — the fixture named for dates covered only
    the ir.Value channel and stayed green throughout. Both fixtures grew the
    missing sites: raw-preserved dates in a vendor extension and under the §4.7
    carve-out, and !!binary extensions in flow and block form.
  • A nested row pins that one divergent member diverges the whole construct,
    which is how every raw site holding a structure rather than a bare scalar
    reaches the walk.
  • Restoring the old rendering reddens both new conformance fixtures and every
    new unit row, so the regenerated goldens are not vacuous.
  • assertYAMLTimestampScalars previously required zero diagnostics; it now
    requires that the only notice is the §4.7 carve-out's info, so a warning
    from a dropped or degraded date still reddens it.
  • Gate green locally: gofmt clean, go vet clean, golangci-lint 0 issues,
    go build ./..., coverage 100% (4509/4509 statements).

Closes #242

The channel whose documented promise is verbatim preservation rewrote the
two YAML scalars JSON has no type for, so a source could not be recovered
from what reached the IR.

A timestamp was rendered as the RFC 3339 instant it resolves to, giving
`2021-1-1` a padding, a time and a zone the source never wrote. A !!binary
was rendered as its decoded bytes in a JSON string, which lost the base64
spelling on every value and lost the data itself on any byte that is not
valid UTF-8: `/w==` is 0xFF, and encoding/json substitutes U+FFFD for it,
leaving nothing to tell it from a source that wrote U+FFFD.

Both now keep the text the source wrote. The resolved form is derivable
from the spelling and the spelling is not derivable from the resolved
form, which is the same reasoning the ir.Value channel already applies to
a timestamp; the two channels agreed on nothing here before.

The decode stays as the tag check it has always been, so a scalar that
does not satisfy the type it declares is still refused and what a source
gets back for one is unchanged. Whether such a scalar should instead
survive as text is a question about what the walk accepts rather than
what it preserves, and is left open as #245.

Both conformance fixtures that reach these sites were blind to this: no
golden held a normalized timestamp and no !!binary reached the raw
channel at all, so the corpus could not have caught the rewrite and
cannot show the fix without new cases. Reverting the change reddens both.

Closes #242
Three corrections to the raw-scalar work, found reviewing the branch as it
will be merged.

The corpus grouped 2021-01-01 10:20:30 under a comment calling it a
spelling the old decode reproduced exactly, while rawDivergences two
declarations above recorded it as a divergence — the old decode rewrote
it to a T and a Z. The block-form !!binary sat under a header reserved
for rows the old conversion refused, which it is not. Both rows now sit
with the scalars they belong to, under a comment that says which kind is
which and why the equivalence ones earn their place.

Nesting had no unit row. Every raw site that holds a structure rather
than a bare scalar reaches the walk that way, so a mapping whose members
diverge is now recorded with both spellings; reverting the fix reddens
it alongside the scalar rows.

Also restores the fractional-seconds row dropped when that table was
rewritten, and drops a stale count from a test doc that said "the two
preconditions" after a third was added.
@OmarAlJarrah
OmarAlJarrah merged commit 3d5e4fc into main Aug 4, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the fix/openapi-raw-scalar-verbatim branch August 4, 2026 11:50
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 rewrites timestamp and binary scalars, losing source spelling and bytes

1 participant