Skip to content

fix(compilers/openapi)!: scope and build path item entries once - #1327

Merged
OmarAlJarrah merged 21 commits into
mainfrom
perf/openapi-path-item-once
Nov 2, 2026
Merged

OmarAlJarrah merged 21 commits into
mainfrom
perf/openapi-path-item-once

Conversation

@OmarAlJarrah

@OmarAlJarrah OmarAlJarrah commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Summary

A path item lowers to no node of its own, so what it declares that its operations have no home for
(its summary and description, its x- extensions and its undeclared keys) is kept under Unmodeled
on every operation it mounts. This changes how those entries are built and how the documentation
pair is keyed.

Built once per mount. mountPathItem computed the entries inside its per-operation loop, once
per operation and per layer, though none of them depends on the operation. Each layer's keys were
censused, sorted, looked up, rendered from YAML to JSON and reported again. With a counter at each
step, a path item writing two undeclared keys over eight operations ran the census 8 times and built
16 findings, of which compile.Diags kept 2. A $ref'd item with a sibling key of its own (two
layers) ran applyPathItem 16 times and built 24 findings, keeping 3. applyLayers now runs once
per mount into one map, beside itemServers and the precedence check, which were already once per
mount. keepOn then copies each entry onto each operation, cloning its value so no two operations
share bytes. The note that an operation holds its path's documentation stays per operation, since
it is sited there.
applyPathItemDocs no longer makes that note, so the carrier's provenance field had no reader left
and is removed, along with onNearestNode's context parameter.

Documentation keyed beneath the item's scope. The pair was kept under the fixed keys
openapi:pathItemSummary and openapi:pathItemDescription. An operation's census spells the same
keys for an undeclared key of that name, and the path item's text, written after the census, replaced
it. The operation's value reached the IR in no form while its warning said it was kept (#1323). The
pair is now keyed beneath the carrier's scope, as the item's extensions and undeclared keys already
are: openapi:pathItem/summary and openapi:pathItem/description. The census escapes a / in the
key it keeps, so no key of the operation's own can spell one. An item mounting no operation already
keeps its pair beneath a scope holding its own pointer (openapi:pathItem/<item pointer>/summary),
so applyPathItemDocs now keys every carrier by annotation.EntryKey(into.scope, keyword), and the
carrier's operation flag, which chose between the two spellings and the note, is removed. The
path-item-docs rule says so and now lists its tests.

No entry replaced. Building once moved the census's check for an occupied entry off the
operation's own map, onto the map the mount's entries are built in. keepOn now copies through a
new annotation.CopyEntriesInto, which never replaces an entry the carrier holds. It reports a
refused copy under openapi/unknown-key-entry-taken, in key order as the census does, but with a
message of its own: the operation keeps no copy, where the census's message says the IR holds none,
which would be false while the mount's other operations hold it. It clones each value it writes.
The scoped keys mean no input reaches that branch today. It is there so a later key that clashes
is reported rather than silently replacing the operation's entry. The note about the path's
documentation is made only when that entry is the one the operation holds.

What a consumer sees:

  • Two Unmodeled keys on operations are renamed, as above, so IRVersion moves to 0.30.0 (see
    Breaking).
  • A path item's own findings are reported before its operations' rather than after the first
    operation's. That reorders the diagnostics in two conformance goldens.
  • Nothing else moves. Besides the rename, the goldens' only change is that order.

Closes #414.
Closes #1323.

Breaking

IRVersion moves from 0.29.0 to 0.30.0. The JSON shape is unchanged, so no pin asks for the bump,
but a consumer built against 0.29.0 that reads openapi:pathItemSummary or
openapi:pathItemDescription on an operation finds the path's text gone, silently: the failure
docs/ir/ir-version.md says a version exists to report. 0.26.0 was bumped for the same reason,
when Unmodeled entries a consumer read stopped being written. Where an operation writes an
undeclared key of one of those names, the old key now holds that key's own value, which 0.29.0 let
the path's text replace. The generations row repeats 0.29.0's shape and names and states the
meaning, and docs/ir/version-history.md says what a consumer pinned to 0.29.0 sees.

Test plan

  • TestPathItem_KeepsWhatItDeclaresOncePerMount reads the walk's diagnostics before the dedupe,
    through a new serviceAsReported helper that shares walkService with serviceWithGrouping.
    Over an inline item with three operations writing a summary, a $ref'd two-layer item with two
    operations writing a summary at its use site and a description in its referent, and an item
    writing only a description, it requires one openapi/unknown-object-key per source key, one
    documentation note per operation, every entry on every operation, an operation's own entry beside
    them, and each operation owning its bytes.
    • On main (7f84912) it fails on the once-per-key assertion, on the note, which main makes
      twice on each operation of the two-layer item, and on the renamed keys.
    • Planted at 7adac1b, each of these fails it: the clone dropped, the note dropped, the note made
      for each keyword that landed, only summary checked, only description checked, entries
      written onto the first operation alone, the operation's own map replaced, the entries rebuilt
      for each operation, and the fixed keys restored.
  • TestPathItem_DocsKeepApartFromAnOperationsOwnKeys gives an operation undeclared keys spelled
    pathItemSummary and pathItem/summary under a path with a summary, and requires all three
    values under keys of their own and no openapi/unknown-key-entry-taken anywhere. On main it
    fails on the operation's pathItemSummary holding the path's text. Copying the entries onto an
    operation twice fails it, which an assertion at the operation's own key alone would let pass,
    since the refusal is sited at the path item's.
  • TestCopyEntriesInto_KeepsWhatTheCarrierHolds and TestCopyEntriesInto_ReportsInKeyOrder hold
    the copy to keeping the carrier's entry, reporting the refused one at its own pointer, naming the
    holder and claiming no loss from the IR, cloning, and sorting 64 refusals by key.
    TestKeepOn_AnnouncesOnlyDocumentationThatLanded holds the note to the entry that landed, a
    description beside a refused summary included, and to its code. Each of these mutants fails one
    of them: the held entry overwritten, the refusal left unreported, the refusal reported with the
    census's message, the sort dropped, the clone dropped, the note made whatever landed, the note
    given up at a refused summary, and the note made under another code.
    • The key-order test refuses 64 keys written in reverse. With the sort dropped it failed 400 runs
      of 400. With eight keys written in order, as it first had, it passed 46 of 400: Go iterates a map
      of eight or fewer entries as a rotation of the order they were written in.
  • TestUnknownKeys_PathItemDeclaredFieldsAreNotUndeclared refused every key beneath
    openapi:pathItem/ as a census entry for a declared field. It now admits the documentation pair
    alone, and still fails when the census is planted to treat query or servers as undeclared.
    Planting the same for summary passes it, since the census then finds the docs entry at its own
    pointer and stays silent; TestPathItemFields_MatchTheLibraryModel fails on that plant.
  • TestApplyPathItemDocs_WithoutRootNode now runs through keepOn as well, so it still holds that
    nothing is announced when nothing was kept.
  • morphic compile with a main build and a build of this branch over 2,068 specs: the 227 under
    testdata/, 1,840 real-world OpenAPI 3 specs, and the Cloudflare API schema @319dc756. It ran at
    main b206dc9, at 7f84912, which bumps the YAML library, at be889c6, which reworks extension
    promotion and adds two of the specs, at 02c972b, which retypes the local time formats, and at
    117d29e, which keeps an unmounted item's documentation, each after merging it; the last three
    runs also follow the bump. With the two keys renamed in main's
    output, and its irVersion stamp read as the branch's where the branch bumps it, exit codes, the
    IR and the multiset of diagnostics are identical on every one. Ten of the specs hold the renamed
    keys, five of them real-world.
    Diagnostic order differs on three, each a path item with undeclared keys: the two goldens above
    and testdata/openapi/unknown_keys.yaml, which has no golden.
  • The golden diffs hold nothing else. After the first commit, each changed golden equals main's
    but for the order of its diagnostics. After the second, renaming the two keys in the first
    commit's goldens leaves no difference.
  • TestIRVersion_PinsTheEncodedShape and TestIRVersion_PinsTheNames pass with the 0.30.0 row,
    whose hashes equal 0.29.0's, because it states its meaning.
  • The gate runs in CI.

A path item lowers to no node of its own, so what it declares that its
operations have no home for (its summary and description, its x-
extensions and its undeclared keys) is kept under Unmodeled on every
operation it mounts. mountPathItem computed it inside the operation
loop: every layer of the mount was censused, rendered from YAML to JSON
and reported again for each operation, though nothing in it depends on
the operation. Eight operations over two undeclared keys built sixteen
findings, of which compile.Diags kept two.

applyLayers now runs once a mount, into one map, and keepOn writes a
copy of each entry onto each operation, cloning its value so no two
operations share bytes. The note that an operation holds its path's
documentation stays per operation, since it is sited there.

The IR is unchanged. A path item's own findings are now reported before
its operations' rather than after the first one's, beside the servers
and precedence findings that were already made once a mount, which
moves them in two conformance goldens.

Closes #414.
A path item's summary and description were kept on each operation under
the fixed keys openapi:pathItemSummary and openapi:pathItemDescription.
An operation's census spells the same keys for an undeclared key of that
name, and the path item's text, written after the census, replaced it:
the operation's value reached the IR in no form while its warning said
it was kept.

The pair is now keyed beneath the carrier's scope, as the item's
extensions and undeclared keys already are: openapi:pathItem/summary and
openapi:pathItem/description on an operation. The census escapes a "/"
in the key it keeps, so no key of the operation's own can spell one.
pathItemDocFields becomes pathItemDocKeywords, with the key derived by
annotation.EntryKey, and the "pathItem" scope is named once, as
pathItemScope.

This renames two Unmodeled keys. The path-item-docs rule and three
conformance goldens follow, and nothing else in the goldens moves.

Closes #1323.
Building a path item's entries once a mount moved the census's check for
an occupied entry off the operation's own map. The check now runs on the
map the mount's entries are built in, and keepOn copied them onto each
operation without looking. No key can clash today, since every one sits
beneath the pathItem scope, but nothing would say so if one did.

annotation.CopyEntriesInto copies a set of entries onto a carrier
without replacing one it holds. It reports the refused copy under
openapi/unknown-key-entry-taken, in key order, as the census does, and
clones each value it writes. keepOn uses it, and announces the path's
documentation only when that entry is the one the operation holds.
TestUnknownKeys_PathItemDeclaredFieldsAreNotUndeclared read every key
beneath openapi:pathItem/ as an entry the census made for a key the
path item does not declare. The summary and description are now kept
there by their own reader, so the check admits exactly those two
keywords. A census entry for query or servers still fails it, and
TestPathItemFields_MatchTheLibraryModel holds the two documentation
keywords among the declared fields.
The note on which keys beneath openapi:pathItem/ the check admits moves
from the test's doc comment, which it took past the 100-word cap, to the
assertion it explains.
TestPathItem_DocsKeepApartFromAnOperationsOwnKeys says no finding reports
a key as unrepresented, but asserted that only at the operation's own key,
where no refusal is sited: annotation.CopyEntriesInto reports one at the
path item's entry. It now refuses an unknown-key-entry-taken anywhere, so
a second copy onto the operation fails it.

TestKeepOn_AnnouncesOnlyDocumentationThatLanded counted one diagnostic
where documentation lands without checking it is the note, so a note under
the wrong code passed. It now checks the code.

Two comments still said a path item's entries are built once per
operation; they are built once per mount.
read-once-per-run said #414 was still open. This branch closes it, so the
rule's Why lists it with the other repeats that were found and removed.
TestCopyEntriesInto_ReportsInKeyOrder said its eight keys left one chance
in 40,320 that an unsorted report passes. Go iterates a map of eight or
fewer entries as a rotation of the order they were written in, and the
keys were written sorted, so one rotation in eight is the sorted order:
with the sort removed, the test passed 46 of 400 runs.

It now refuses 64 keys written in reverse, which leaves the order to the
map's hash seed. With the sort removed it failed all 400 runs.
TestPathItem_KeepsWhatItDeclaresOncePerMount requires one documentation
note per operation, but each mount in its fixture wrote one of the two
keywords, so a note made for each keyword that landed passed it: an
operation under a path item writing both would draw two, which
compile.Diags then drops to one.

/a now writes both keywords, and /b writes its summary at the use site
and its description in the referent, the shape main also reported twice.
A note per keyword now fails the test.
annotation.CopyEntriesInto copies one set of entries onto several
carriers, and reported an entry a carrier already held with the census's
message, which says the key "is represented in the IR in no form at all".
Copying onto two operations where only the second held the key, the
first kept the entry while the second's finding said the IR had none.

The refusal keeps its code and gets a message of its own, saying the
node holding the other construct keeps no copy. The comments on the
function and on diag.UnknownKeyEntryTaken say the key is missing from
that one carrier. No input reaches the refusal today, since every key
copied sits beneath the path item's scope.
applyPathItem's comment read "keeps on into what a path item declares",
and carrier.keepsDocs said "Only an operation's are", though the carrier
for operations is now the map their entries are built in. Both say what
they mean now.
Writing both keywords on /a, and a summary at /b's use site, left every
mount with a summary and a description, so a note checking only one of
the two keywords passed TestPathItem_KeepsWhatItDeclaresOncePerMount.

/a now writes only a summary, a new /c only a description, and /b one
keyword at each layer. A note checking one keyword, or made for each
keyword that landed, now fails the test, as do the plants it already
caught.
TestKeepOn_AnnouncesOnlyDocumentationThatLanded refused the one
documentation entry it copied, so a keepOn that gave up at the first
keyword it was handed, landed or not, passed it: with the summary refused
and the description kept, that keepOn makes no note.

The test now copies both onto an operation already holding the summary,
and requires the refusal and then the note.
Its comment said it returns "every diagnostic after the load's", which
reads as the load's diagnostics followed by the rest, as
serviceWithGrouping returns them. It returns the later phases' alone.
Keying a path item's summary and description beneath its scope renamed
two Unmodeled keys on every operation it mounts: openapi:pathItemSummary
and openapi:pathItemDescription became openapi:pathItem/summary and
openapi:pathItem/description. The JSON shape is unchanged, so no pin
asked for a bump, but a consumer built against 0.27.0 reads the old keys
and finds the path's text gone, silently. That is the failure IRVersion
exists to report (docs/ir/ir-version.md), and 0.26.0 was bumped for the
same reason, when Unmodeled entries a consumer read stopped being written.

IRVersion moves to 0.28.0. Its generations row repeats 0.27.0's shape and
names and states the meaning that changed, and the version history says
what a consumer pinned to 0.27.0 sees: no path text under the old keys,
and, where an operation writes an undeclared key of that name, that
key's own value, which 0.27.0 let the path's text replace.
@OmarAlJarrah OmarAlJarrah changed the title fix(compilers/openapi): scope and build path item entries once fix(compilers/openapi)!: scope and build path item entries once Nov 2, 2026
main took IRVersion 0.28.0 for the local time formats (#1325), so the
renamed path item doc keys move to 0.29.0: a row after 0.28.0's with the
same shape and names, and a history entry naming what a consumer pinned
to 0.28.0 sees.
main took IRVersion 0.29.0 for Content.Docs (#1333), so the renamed path
item doc keys move to 0.30.0: a row after 0.29.0's with its new shape and
the same names, and a history entry naming what a consumer pinned to
0.29.0 sees.
main now keeps an unmounted path item's summary and description beneath
its scope (#1322), keyed by annotation.EntryKey, while an operation took
the fixed pathItemDocFields keys, told apart by a carrier.operation flag.
This branch keys an operation's pair beneath its scope as well, so every
carrier takes annotation.EntryKey(into.scope, keyword), and the note an
operation draws is keepOn's. The flag has no reader left and goes, as
the provenance field and keepsDocs did; itemConstructs reads
pathItemDocKeywords. The path-item-docs rule states both carriers' keys
and lists both branches' tests.
@OmarAlJarrah
OmarAlJarrah merged commit 6996ce4 into main Nov 2, 2026
4 checks passed
@OmarAlJarrah
OmarAlJarrah deleted the perf/openapi-path-item-once branch November 2, 2026 13:35
OmarAlJarrah added a commit that referenced this pull request Nov 2, 2026
Brings in #1327.

- #1327 took IRVersion 0.30.0, with no change of shape, so this
  branch's bump moves to 0.31.0. Its row keeps the shape both share and
  this branch's names and meaning, and its version history entry moves
  with it, pinned against 0.30.0.
- #1327 and this branch each added a test at one place in
  preserve_internal_test.go; both are kept.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant