feat(creative): add creative revision identity - #6781
Conversation
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema paths require human/CODEOWNERS review.
This PR touches the protocol source of truth (static/schemas/source/**), which is under a hard, deterministic approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so decision-table row 2 fires: the gate is not satisfied and the PR cannot be auto-approved.
No critical/high findings were reported. The reviewer surfaced two medium findings and two low findings — none blocking on their own — but the deterministic gate takes precedence.
Medium findings (non-blocking, for human reviewer awareness):
- static/schemas/source/core/creative-variant.json:26 — variant_id uniqueness scope tightened (per-creative → per-agent) on a minor changeset
- static/schemas/source/creative/sync-creatives-request.json:150 — New creatives[] per-batch uniqueness constraint not reflected in docs
Escalation reasons:
- Gated paths touched (14 files under
static/schemas/source/**, including additions creative-revision-id.json and creative-revision-content-mismatch.json plus modifications to creative-variant.json, error-code.json, index.json, and several creative task schemas) andreview_decisionisREVIEW_REQUIRED. Human/CODEOWNERS approval is required before merge.
Human reviewers should confirm the two coherence points above (variant_id scope semantics and the undocumented per-batch creative_id uniqueness constraint) as part of that review.
Medium findings
- static/schemas/source/core/creative-variant.json:26 — variant_id uniqueness scope tightened (per-creative -> per-agent) on a minor changeset
- static/schemas/source/creative/sync-creatives-request.json:150 — New creatives[] uniqueness constraint not reflected in docs
Why human review
- Gated paths: 14 files under static/schemas/source/** are modified/added while review_decision is REVIEW_REQUIRED (not APPROVED) — human/CODEOWNERS approval required before merge.
- Medium coherence gaps for reviewer attention: variant_id uniqueness scope tightened per-creative→per-agent, and a new per-batch creative_id uniqueness constraint is undocumented.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/creative-revision-id.json (added) matches
static/schemas/source/**; static/schemas/source/core/creative-variant.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/x-entity-types.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/creative-status-changed-webhook.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/get-creative-delivery-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/list-creatives-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/preview-creative-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/preview-creative-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/sync-creatives-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/sync-creatives-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/error-details/creative-revision-content-mismatch.json (added) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
aao-secretariat
Bot
added
the
ladon/needs-human-review
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated-paths hard approval gate not yet satisfied.
This PR modifies/adds 14 files under static/schemas/source/**, which are under a hard, deterministic approval gate (gated_paths: true). The current review_decision is unknown/not APPROVED, so per decision-table row 2 the outcome must be escalate — Ladon cannot auto-approve gated protocol-schema changes until a human/CODEOWNERS approval is recorded.
The delta itself is clean: the developer walked back both prior medium findings. variant_id uniqueness is no longer tightened (default stays unique-within-agent-and-creative; agent-unique/immutable only applies when an agent advertises creative.supports_revisions), applied coherently across creative-variant.json, get-creative-delivery-response.json, preview-creative-request/response.json and the corresponding docs, with tests updated. sync_creatives.mdx now documents the creatives[] unique creative_id constraint. Schema↔docs coherence holds. No new findings in this run.
Because both prior mediums are resolved and no fresh actionable findings exist, sticky escalation (row 6) does not apply on its own — but the gated-paths gate (row 2) still stands and requires human review.
Escalation reasons
- 14 files under
static/schemas/source/**are modified/added whilereview_decisionis notAPPROVED— human/CODEOWNERS approval required before merge.
Why human review
- Gated paths: 14 files under static/schemas/source/** are modified/added while review_decision is not APPROVED — human/CODEOWNERS approval required before merge (decision-table row 2). Affected files include static/schemas/source/core/creative-revision-id.json (added), static/schemas/source/core/creative-variant.json (modified), static/schemas/source/enums/error-code.json (modified), static/schemas/source/creative/sync-creatives-request.json (modified) and others.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/creative-revision-id.json (added) matches
static/schemas/source/**; static/schemas/source/core/creative-variant.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/x-entity-types.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/creative-status-changed-webhook.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/get-creative-delivery-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/list-creatives-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/preview-creative-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/preview-creative-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/sync-creatives-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/creative/sync-creatives-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/error-details/creative-revision-content-mismatch.json (added) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'unknown', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
|
Read through #6781 — solid foundation, and it does land the two core pieces of #6347 (buyer 1. Serving during re-review — needs to be spelled out.
Visibility itself looks fine (sync response acks the transition, 2. Managed service / seller-authored creatives. Happy to dig into either. |
|
@pkras — read both against the current spec + adjacent tasks. Think both are answerable by composing what's already there, with two small non-normative doc clarifications on this PR to close the gap you flagged. No new normative rules needed. 1. Serving during re-review Per-creative
Suggested one-sentence addition to the revisions section of
2. Managed service / seller-authored Path already exists as a composition. Suggested one-line pointer in the same revisions section:
The audit trail of seller-side drafts before the buyer syncs is genuinely outside AdCP — that's a managed-service SLA artifact between the parties, not a wire concern. A "seller revision" type would muddy input integrity as you noted and doesn't solve a protocol-layer problem; agree it should stay off this PR. Both clarifications are docs-only and additive — no schema or semantic change. @bokelley to decide whether they fold into this PR or ship as a follow-up. Generated by Claude Code |
…ntity # Conflicts: # package.json
|
@pkras Folded both clarifications into ff5ec0a. The revision section now states explicitly that a revised |
pkras
left a comment
pkras
left a comment
There was a problem hiding this comment.
Two smaller content-definition notes, both docs/schema-only:
-
ext shouldn’t count as revision content. The canonical-content rule excludes 8 named top-level fields and treats everything else — including ext — as revision-bearing (creative-asset.json is additionalProperties: true). That means a drifting vendor/telemetry value in ext trips CREATIVE_REVISION_CONTENT_MISMATCH on an otherwise-identical re-sync. Suggest adding ext (or a designated non-content envelope) to the excluded set so buyers aren’t penalized for data that doesn’t change what renders.
-
The “digest-bound asset” escape hatch doesn’t exist yet. The revision section points buyers needing byte-level identity to “immutable URLs or digest-bound asset forms where available” — but there’s no content-digest field on creative-asset.json, so immutable URLs are the only real tool today. Either add the digest field or drop the reference so it’s not sending buyers after something unimplemented.
…6767) * feat(creative): define delivery and macro resolution contracts * docs(creative): gate tracker firing on product capabilities * fix(creative): preserve URL compatibility and type error details * feat(creative): define complete VAST technical requirements * fix(creative): keep VAST requirements extensible * fix(creative): close delivery contract review gaps * fix(creative): satisfy canonical VAST gates * fix(compliance): version CTV VAST fixtures * feat(creative): model equivalent representation sets * fix(creative): satisfy representation CI gates * fix(creative): keep VAST version requirement additive * docs(creative): clarify representation set format scoping and add localization migration warning Per @pkras review on #6767: - state explicitly that a representation set is scoped to one format_kind - add a Warning callout documenting the destructive, non-atomic path for migrating an existing localized creative to representation selection * docs(creative): pin accepted representation selections * fix(creative): clarify representation review guarantees --------- Co-authored-by: Claude <noreply@anthropic.com>
…ntity # Conflicts: # docs.json # tests/docs-nav-validation.test.cjs
|
Ladon cannot review this PR until merge conflicts are resolved. |
|
Ready for fresh CODEOWNER review. Since the previous approval, this branch was updated onto current main, preserved the new versioned-doc navigation, capability-gated revision echo/readback on |
|
CI follow-up: the repeated 4 GiB sales storyboard failure was a deterministic schema-projection expansion, not runner flakiness. |
|
Follow-up on the two review notes:
The latest head also contains the schema-graph/CI memory fix described above. Fresh approval is requested for |
|
CI is now fully green at the exact head (58 passing, 0 failing; 2 expected skips), all review threads are resolved, and independent protocol/code reviews found no blocker/high issues. @pkras @ohalushchak-exadel @bmilekic — a fresh CODEOWNERS approval is the sole remaining merge gate; auto-merge is enabled. |
2 participants
Summary
revision_idtosync_creativeswith immutable effective-input semantics and a typed mismatch errorvariant_idan agent-unique immutable execution identity so historical delivery and variant preview remain unambiguousIdentity model
This PR does not add revision history, rollback, or staged serving. Experimental staged activation is tracked separately in #6780.
Verification
npm run test:schemasnpm run test:json-schemanpm run test:docs-navnpm run test:compliance-snippetsnpm run test:error-code-driftnpm run test:oneof-discriminatorsnpm run test:storyboard-context-entitynode --test tests/creative-revisions.test.cjsnode scripts/check-changeset-protocol-scope.cjs origin/mainnpx --yes @changesets/cli@^3.0.0 status --since=origin/mainCloses #6347