Skip to content

feat(schemas): x-adcp-hoist opt-in marker for canonically shared object schemas#4630

Merged
bokelley merged 2 commits into
mainfrom
bokelley/issue-4557
May 17, 2026
Merged

feat(schemas): x-adcp-hoist opt-in marker for canonically shared object schemas#4630
bokelley merged 2 commits into
mainfrom
bokelley/issue-4557

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 17, 2026

Closes #4557.

Summary

Opt-in companion to the pure-enum auto-hoist (hoistDuplicateInlineEnums, #3170). Spec authors set x-adcp-hoist: true on a source schema's root; the bundler moves it to root \$defs, replaces every inline occurrence with a \$ref, and strips the directive from bundled output.

Why opt-in for complex objects: structural identity ≠ semantic identity (BriefAsset and VASTAsset share fields today but represent different lifecycle concepts). Auto-hoisting would lock in coupling that's hard to walk back. x-adcp-hoist is the deliberate exception, not the default — and per-type decisions stay separate from landing the mechanism.

Naming follows the x-adcp-* precedent set by x-adcp-validation; the schema-extensions reference documents the directive's contract for source-tree consumers.

Behavior

  • Hoists at any occurrence count (≥1). The marker declares intent; honoring it for single uses means adding a second reference later doesn't change the codegen surface.
  • title is required — missing/empty → build error (the directive is meant to be deliberate).
  • Same title + different shape is a build error — silently suffixing one to Foo2 would defeat the "canonical name" guarantee.
  • Collision suffixing (PriceBlock2) only for a pre-existing \$defs key (non-marker name collision), matching the enum-hoist convention.
  • Different titles never collapse, even with structurally identical bodies — preserves the BriefAsset/VASTAsset distinction by default.
  • Recurses into hoisted defs so nested markers also collapse to refs.
  • Final sweep strips stray markers that survived a pre-existing \$defs block — the directive never appears in bundled output.

Dry-run validation

Marked core/duration.json with x-adcp-hoist: true and rebuilt locally:

Check Result
Bundles changed semantically 12 — exactly the bundles that referenced Duration
Bundles with timestamp-only diffs 69 — all other bundles unchanged
\$defs.Duration present in changed bundles ✅ canonical entry with title, no x-adcp-hoist
x-adcp-hoist anywhere in bundled output ✅ zero (directive stripped)
Inline \"title\": \"Duration\" outside \$defs ✅ zero (all 6 inline copies in create-media-buy-request collapsed)
Ajv compiles the hoisted bundle ✅ clean

Source change reverted before commit. No source schemas opt in here — per-type decisions ship in follow-ups.

Confirmed against this branch's full build: zero bundles change semantically vs. main (timestamp-only diffs only).

Review-driven changes

Two expert reviews flagged:

  • BUG: tests not wired into npm test → fixed (test:build-schemas-hoist-marked added to the chain).
  • BUG: missing changeset → added (.changeset/4557-x-adcp-hoist-opt-in-marker.md).
  • RISK: stray markers inside pre-existing \$defs weren't stripped → fixed (final sweep + dedicated tests).
  • RISK: same-title-different-shape silently produced Foo/Foo2 → fixed (build-time error + test).
  • PROTOCOL: bare x-hoist namespace → renamed to x-adcp-hoist (matches x-adcp-validation precedent).
  • PROTOCOL: source-tree publication surface → docs/reference/schema-extensions.mdx entry documents the directive contract, conformance, and SDK codegen impact.

Out of scope

  • Per-type decisions for BriefAsset / VASTAsset / DAASTAsset / CatalogAsset (RFC default lean: keep-split).

Test plan

  • 14 unit tests in tests/build-schemas-hoist-marked.test.cjs (basic hoist, single-occurrence, missing/empty title errors, same-title-different-shape error, PascalCase sanitization, pre-existing $defs collision, distinct-titles preservation, no-op, array items, nested markers, pre-existing $defs same-shape, stray-marker stripping in two configurations)
  • Existing hoist-enums and preserve-subschema-ids tests still pass
  • npm test chain now includes test:build-schemas-hoist-marked
  • Full node scripts/build-schemas.cjs runs clean (81 bundled schemas, zero semantic diffs vs main)
  • Ajv-compile spot-check on hoisted bundle (validated via dry-run)

🤖 Generated with Claude Code

bokelley and others added 2 commits May 17, 2026 05:57
…hemas (#4557)

Opt-in companion to the pure-enum auto-hoist (`hoistDuplicateInlineEnums`,
#3170). Spec authors set `x-hoist: true` on a source schema's root; the
bundler moves it to root `$defs`, replaces every inline occurrence with a
`$ref`, and strips the directive from bundled output.

Why opt-in for complex objects: structural identity ≠ semantic identity
(BriefAsset and VASTAsset share fields today but represent different
lifecycle concepts). Auto-hoisting would lock in coupling that's hard to
walk back. `x-hoist` is the deliberate exception, not the default — and
per-type decisions stay separate from landing the mechanism.

Behavior:
- Hoists at any occurrence count (>=1). The marker declares intent; honoring
  it for single uses means adding a second reference later doesn't change
  the codegen surface.
- `title` is required — missing/empty -> build error (the directive is
  meant to be deliberate).
- Collision suffixing (`PriceBlock2`) when a `$defs` name already exists,
  matching the enum-hoist convention.
- Different titles never collapse, even with structurally identical
  bodies — preserves the BriefAsset/VASTAsset distinction by default.
- Recurses into hoisted defs so nested markers also collapse to refs.

Dry-run validation: marking `core/duration.json` and rebuilding hoisted 6
inline copies in `create-media-buy-request` (and 5 other media-buy bundles)
into a single `$defs.Duration` entry, with Ajv compiling cleanly and
`x-hoist` absent from output. Reverted before commit — no source schemas
opt in here. Per-type decisions ship in follow-ups.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Rename `x-hoist` → `x-adcp-hoist` to match the `x-adcp-*` namespacing
  convention documented in schema-extensions.mdx. Prevents collision with
  any future cross-vendor `x-hoist` keyword and matches the precedent set
  by `x-adcp-validation`.
- Strip stray `x-adcp-hoist` markers from bundled output even when they
  live inside a pre-existing $defs block (which Pass 1 skips). The
  directive must never leak into bundled artifacts regardless of where
  it was authored.
- Reject two distinct schemas marked with the same `title` — the
  collision-suffix path would silently rename one of them, defeating the
  directive's "canonical name" guarantee. Pre-existing $defs collisions
  still suffix (that case is a non-marker name collision, not a spec
  author bug).
- Wire `test:build-schemas-hoist-marked` into the npm `test` chain so the
  unit tests actually run in CI.
- Add changeset for the bundler mechanism.
- Add `docs/reference/schema-extensions.mdx` entry covering directive
  contract, bundler behavior, SDK/codegen impact, and conformance — the
  canonical reference for source-tree consumers who dereference source
  schemas directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley changed the title feat(schemas): x-hoist opt-in marker for canonically shared object schemas feat(schemas): x-adcp-hoist opt-in marker for canonically shared object schemas May 17, 2026
@bokelley bokelley merged commit eb14837 into main May 17, 2026
25 checks passed
@bokelley bokelley deleted the bokelley/issue-4557 branch May 17, 2026 10:18
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.

RFC: x-hoist opt-in marker for canonically shared object schemas in bundler

1 participant