feat(creative): declare tracker execution contracts - #6786
Conversation
There was a problem hiding this comment.
Ladon verdict: Request changes
Request changes — 1 blocking finding.
Blocking findings
- static/schemas/source/core/assets/vast-tracker-asset.json:34 — GA-published vast_tracker/daast_tracker assets are narrowed against 3.1.19 (offset ignored→forbidden, new event/target matrix) via the new constraints files. This is a breaking wire change shipped under a minor changeset, which violates the repo's changeset-scope rule (breaking wire changes require
major, notminor/patch). Either scope the changeset asmajoror avoid tightening validation on the already-published assets.
Medium findings
- static/schemas/source/core/canonical-format-option.json:45 — canonical-format-option adds a publisher_domain→format_option_id requirement (accept→reject), a validation tightening worth confirming is intended and non-breaking for existing payloads.
Row 1 of the decision table fires on the high finding, producing request-changes. Note: this PR touches gated paths under static/schemas/source/** with review_decision unknown/not APPROVED, and the reviewer flags a breaking-class change — both of which independently require human/CODEOWNERS review before merge. The schema modeling itself (discriminated tracker-execution-selector oneOf, immutable PackageFormatSnapshot with RFC 8785 digests, authority-stripping on projection surfaces) is clean and well tested; the blocker is the changeset-type/wire-compatibility mismatch on the GA-published tracker assets.
Blocking findings
- static/schemas/source/core/assets/vast-tracker-asset.json:34 — vast_tracker/daast_tracker GA assets narrowed (offset ignored→forbidden, new event/target matrix): breaking wire change vs 3.1.19 shipped under a minor changeset; requires major changeset or reverted tightening
Medium findings
- static/schemas/source/core/canonical-format-option.json:45 — canonical-format-option adds publisher_domain->format_option_id requirement (accept->reject)
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated paths not yet satisfied by human/CODEOWNERS review.
This is a subsequent pass. The prior request-changes blocking finding (vast/daast tracker assets narrowed against 3.1 GA under a minor changeset) has been resolved: the delta drops the whole-file constraints allOf and keeps only a local progress→offset requirement, restoring legacy-asset permissiveness, and new tests prove legacy assets/packages validate. The prior medium (canonical-format-option.json) file is out of the current delta and correctly skipped. No new blocking or medium findings this run.
However, this PR touches many files under static/schemas/source/**, which is a deterministic hard approval gate (gated_paths: true), and review_decision is CHANGES_REQUESTED — not APPROVED. Per decision rule row 2, the outcome must be escalate until a human/CODEOWNERS approval lifts the gate. No critical/high findings exist, so this is not request-changes; it is held for required human review only.
Escalation reasons:
- Gated paths touched (protocol schema source under
static/schemas/source/**) andreview_decisionisCHANGES_REQUESTED, notAPPROVED. Human/CODEOWNERS approval is required before merge.
Why human review
- Gated paths under static/schemas/source/** are modified/added and review_decision is CHANGES_REQUESTED (not APPROVED) — human/CODEOWNERS approval required to lift the hard approval gate (decision rule row 2).
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/daast-tracker-asset.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/canonical-format-option.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/daast-tracker-constraints.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/format.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/package-format-snapshot.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/package.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/product-format-declaration.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/representation-destination.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/representation-rejection.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/representation-selection.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/requirements/daast-asset-requirements.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/tracker-execution-contract.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/tracker-execution-selector.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/transformer.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/vast-tracker-constraints.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/x-entity-types.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/pixel-tracking-event.json (added) matchesstatic/schemas/source/**; static/schemas/source/enums/tracker-execution-actor.json (added) matchesstatic/schemas/source/**; static/schemas/source/enums/tracker-firing-path.json (added) matchesstatic/schemas/source/**; static/schemas/source/formats/canonical/audio_daast.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/get-media-buys-response.json (modified) matchesstatic/schemas/source/**; static
…(truncated)…
|
Ready for fresh CODEOWNER review on the repaired stack. The head now descends from current #6781 and preserves all parent representation/macro guarantees. Follow-up fixes require matching first-class format slots, enforce complete-parent narrowing over atomic version/actor/path tuples without treating incomplete parents as ceilings, and keep DAAST |
… tracker-execution-contract # Conflicts: # static/schemas/source/core/canonical-format-option.json # static/schemas/source/core/product-format-declaration.json
|
Pushed the latest parent integration and CI/resource fix at
Fresh human/CODEOWNERS review is requested for the exact pushed head. #6781 remains the dependency and should merge first. |
pkras
left a comment
There was a problem hiding this comment.
-
closeLinear is missing VAST 4.0 (blocker or needs a note).
In vast-tracker-constraints.json, closeLinear allows ["3.0","4.1","4.2","4.3"] but not 4.0, even though every neighboring event covers 4.0–4.3 and closeLinear exists in 4.0. As written it reads like a typo, and a seller building a 4.0 tag can’t declare it. Please either add 4.0 or add a $comment explaining why the gap is intentional. -
The “preservation” rule is described but not enforced.
The docs/spec say a child contract preserves parent commitments “unless narrower inventory genuinely excludes them,” but the actual matching only checks that the child is a subset of the parent — nothing enforces preservation, and “genuinely excludes” has no concrete definition. As-is, one seller could trim the list freely and another couldn’t, and both would pass. Can we either define this precisely or drop the sentence so implementations don’t diverge? -
Fix the IAB citations (doc-only, but worth doing here).
Two provenance claims are off: audible_video_complete is labeled “IAB type 500,” but in OpenRTB Native 1.2 the 500+ range is exchange-specific, not a defined event; and viewable_video_50 is described as requiring “audio on,” which type 4 doesn’t (audio-on is what distinguishes the audible metric). The enum values are fine — just the descriptions.
Smaller items:
The per-version VAST event rules are almost entirely untested — only loaded+4.0 is exercised. A wrong enum in any of the other branches would ship silently; a positive/negative vector per rule would close that.
Minor doc contradiction: package.json says pending items carry the digest unconditionally, while get-media-buys-response.json says “when present” — ID-less snapshots have no digest, so “when present” is the correct phrasing in both.
Heads-up that the new if publisher_domain → require format_option_id rule in canonical-format-option.json is an additional constraint beyond trackers; worth confirming no existing publisher_domain-only options would break under a minor bump.
…tract # Conflicts: # docs/contributing/x-entity-annotation.md # docs/creative/canonical-formats.mdx # docs/creative/representation-sets.mdx # docs/creative/universal-macros.mdx # package.json # static/schemas/source/core/canonical-format-option.json # static/schemas/source/core/product-format-declaration.json # static/schemas/source/core/representation-destination.json # static/schemas/source/core/representation-rejection.json # static/schemas/source/core/representation-selection.json # static/schemas/source/core/x-entity-types.json # tests/mcp-schema-projection.test.cjs # tests/tracking-event-enums.test.cjs
|
Retargeted to Verification on the exact combined tree:
All review threads remain resolved. Fresh CODEOWNERS approval is requested for this head. |
|
CI follow-up: current |
…tract # Conflicts: # docs/reference/test-vectors/index.mdx
|
Final readiness update on
Local verification: 106/106 bundles; schema suite green (879 schemas, 34 schema checks, 160 contract tests, premium/geo suites); MCP projection 27/27; capability/hoist/tracker focused suites green; typecheck green; full |
|
Exact-head hosted CI is now fully green (56 passed, 0 failed), with all review threads resolved and no new actionable comments. Auto-merge is enabled; the only remaining gate is the requested human/CODEOWNERS approval. |
|
Addressed the latest human review in
Verification on the pushed head: full schema suite green (879 schemas, 34 schema checks, 160 contract tests, premium 10/10, geo 6/6); focused tracker/package/compatibility tests green; compliance freshness and docs navigation 43/43 green. Two independent expert re-reviews found no remaining blocker/high issue. Primary references: IAB VAST 4.1 release notes, IAB OpenRTB Native 1.2. |
|
Exact-head CI is green after a targeted rerun of the one transient |
|
Question: independent seller-side impression counting — where does the spec see it living? This PR formalizes execution of buyer-supplied trackers, and deliberately excludes reporting credit ("This promises construction and handoff of the outbound action—not network delivery, HTTP success, attribution, or reporting credit"). I'd like to understand the intended path for the mirror-image case: Consider a seller agent that wants log-level impression records of its own — to attest delivery on
Under those constraints, the only mechanism that travels with the ad is a seller-injected tracker attached at trafficking/assembly time (a tracking URL set on the trafficked creative, or an
To be clear, none of this blocks this PR — the production contract for buyer trackers stands on its own. I'm trying to establish whether independent seller-side measurement is (a) intentionally protocol-invisible, (b) a 3.3 candidate, or (c) worth a fresh RFC. |
|
Good framing — each question points at a real gap. My read on all three, leaning on what the protocol actually specifies vs. what it intentionally leaves to implementation: 1. Seller-injected trackers: out-of-spec, intentionally 2. Seller-injected tracker disclosure: worth filing for 3.3 3. Impression-level join key: re-file for 3.3 Summary: (a) intentionally invisible now, (b) and (c) are both 3.3 candidates worth separate issues. Nothing changes the 3.2 scope here. Triaged by Claude Code. Session: https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID} Generated by Claude Code |
|
Thanks — that matches our read, and the follow-up is filed as #6878 (Measurement: declare seller-initiated trackers and impression-ID joinability). Mapping it to your three answers:
It also pre-empts the DR-0005 conformance-path objection that hit #6207's |
Summary
pixel_tracker,vast_tracker, anddaast_trackerassetsPreview observation sessions and runtime evidence are intentionally excluded and remain AdCP 3.3 work in #6782. URL-slot tracker selectors are the independent 3.3 extension in #6784.
Stack
This PR is stacked on #6767 because tracker compatibility participates in representation-set selection. Retarget to
mainafter #6767 merges.Validation
npm run build:schemas— 106 bundled schemas, zero failuresnpm run test:schemasnpm run test:json-schemanpm run test:compliance-snippetsnpm run test:error-code-driftnpm run test:oneof-discriminatorsnpm run test:schema-linksnode --test tests/tracker-execution-contracts.test.cjs tests/tracker-execution-package-integration.test.cjs tests/tracking-event-enums.test.cjs tests/creative-delivery-contracts.test.cjs tests/docs-nav-validation.test.cjsnode scripts/check-changeset-protocol-scope.cjs origin/vast-creative-delivery-rulesCloses #6207
Related: #6775, #6783, #6782, #6784, #6767