Skip to content

fix(compilers/openapi): keep keys the source model does not name - #356

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-unknown-key-census
Open

fix(compilers/openapi): keep keys the source model does not name#356
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-unknown-key-census

Conversation

@OmarAlJarrah

@OmarAlJarrah OmarAlJarrah commented Aug 9, 2026

Copy link
Copy Markdown
Member

Summary

A key the parsed OpenAPI model has no field for reached neither an IR field, an
Unmodeled entry, nor a diagnostic. Twelve object types were probed and all
twelve dropped it in silence, so two documents differing only in such a key
compiled to the same IR — a losslessness failure (invariant 2), and the reason a
misspelt operationId survives review.

The parser already takes the census: every core model records the keys it did not
name. Nothing read it. This adds one shared reader over that census and calls it
at each object's lowering, keying entries the way the rest of the compiler does —
unscoped on an object that lowers to a node with an Unmodeled map of its own,
and scoped by the source path from the carrier down for the objects that ride on
one (openapi:info/contact/slack, openapi:tags/0/color; tags by index, not
name, since two tags spelled alike would otherwise leave one entry).

The two vocabularies are graded apart, because the specifications differ:

  • Inside a JSON Schema, an unrecognized keyword is legal — the specification
    requires implementations to ignore what they do not recognize, and allows such a
    keyword to carry meaning for other tooling. Kept, and announced at info
    under openapi/unknown-schema-keyword: a recorded decision, not a fault.
  • Inside an OpenAPI object, extensions must be prefixed x-, so a key that is
    neither defined nor prefixed is invalid input. Kept all the same, and announced
    at warning under openapi/unknown-object-key.

Warning rather than error for the reason ReservedHeaderName is one: the document
still lowers, and harness.Check stops at the first error diagnostic, which would
hide every later finding in the file and leave any fixture carrying a stray key
unable to reach the invariant checks.

Both carry ReasonOutOfScope. That is a property of the construct — no IR node is
coming for either — while which of the two a key is says something about the
document, and the diagnostic channel is where that belongs.

Three further points:

  • The census is the complement of what the compiler read, not of what the model
    names.
    It runs last at each position and leaves alone any key already
    recorded, so $vocabulary and dependentRequired keep the reasons their own
    readers give them. Keywords no reader leaves a trace of are named in
    annotation.DecidedKeywords: $comment and $dynamicAnchor are deliberate
    drops recorded in the 2020-12 vocabulary table, and an expanded $dynamicRef
    must not also be preserved.
  • Bounded, at MaxUnknownKeys per object, with the remainder reported under
    openapi/unknown-key-budget rather than dropped in silence. The census is also
    sorted on a copy before use: the library builds it from a parallel walk, so its
    order is neither source order nor stable.
  • The corpus turned out to contain one instance already. A flow mapping in
    allof-oneof-cooccurrence.yaml split description: named by position, not by target into a description and a stray not by target key; the description is
    now quoted so the fixture says what it meant.

Boundary with the neighbouring work

This is keys the source model does not name at all. Keys the model does name
which the elected lowering never reads are a different gap with a different fix
(#268, #283), and the census cannot see them by construction. Vendor x-*
extensions are out of scope here and covered by #345.

Not swept, and why

Path items are excluded: the library folds an unrecognized key into the path
item's embedded operations map and reports a type mismatch instead, so there is no
census to read. paths, responses, components and callbacks are maps where
every key is a valid entry, so "unknown" is undefined for them. The remaining
objects — example, link, encoding, the OAuth flows, discriminator, xml, and
externalDocs below the document root — lower to no node that holds an
Unmodeled map today; the carriers for them are what #345 is adding, so their
census belongs on top of that rather than duplicating its scaffolding. Filed as
#357.

Test plan

  • testdata/openapi/unknown_keys.yaml writes one undeclared key at each of the
    eighteen positions, valued with the object it was written on. Living under
    testdata/ is what runs it through all six oracles — including the two-order
    diff and the round-trip — rather than through assertions alone.
  • TestUnknownKeys_KeptAtEveryObject finds each entry by walking the document's
    value graph and matching on key and value, so an entry written to the wrong
    carrier fails rather than passing because the assertion looked only where it
    expected. Every row was red before the change; reverting the lowering with the
    test kept turns all eighteen red again.
  • TestUnknownKeys_SchemaAndObjectAreGradedApart pins the info/warning split and
    the provenance of both.
  • TestUnknownKeys_WellFormedDocumentRecordsNothing is the control: a document
    writing only what the model names keeps nothing and reports nothing.
  • Unit tests in annotation cover the sort, the bound, the already-recorded skip,
    the decided-keyword skip, an unconvertible value, and the shapes the reader must
    survive rather than panic on.
  • Deleting a key from the fixture reddens the suite, and the full gate passes.

Closes #297

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: a key the source model does not name is dropped with no trace

1 participant