feat(creative): define delivery representations and macro resolution - #6767
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 paths require human/CODEOWNERS review, and a breaking-class wire change is present.
Why escalate
gated_paths: true(many files understatic/schemas/source/**) ANDreview_decision: REVIEW_REQUIRED(notAPPROVED). Per decision-table row 2, this is a hard approval gate that only a real human/CODEOWNERS approval can satisfy.- Independent of the path gate, the repo's constitution treats Breaking-class protocol changes as never auto-approvable. The reviewer flagged that
macro-bearing-url.jsonnarrows every existingurlasset field to absolute HTTP(S) — a tightening of a published wire shape that can break existing conformant payloads. Ratification is a human act; escalate unlessreview_decisionisAPPROVED.
Medium findings (non-blocking, for human reviewers)
- static/schemas/source/core/macro-bearing-url.json:16 — narrows all existing url asset fields to absolute HTTP(S) (potential breaking wire-shape tightening).
- docs/creative/delivery-variants.mdx:1 — feature's primary doc missing from docs.json navigation.
No critical/high findings were raised, so this is not request-changes. But the deterministic gated-paths gate is unsatisfied and a breaking-class change warrants explicit human sign-off. Once a CODEOWNER approves (review_decision: APPROVED), the gate lifts and a re-run can fall through to the normal table.
Medium findings
- static/schemas/source/core/macro-bearing-url.json:16 — macro-bearing-url.json narrows all existing url asset fields to absolute HTTP(S)
- docs/creative/delivery-variants.mdx:1 — delivery-variants.mdx missing from docs.json navigation
Why human review
- Gated paths: PR modifies files under static/schemas/source/** (e.g. macro-bearing-url.json, error.json, asset schemas) and review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required.
- Breaking-class protocol change: macro-bearing-url.json narrows all existing url asset fields to absolute HTTP(S), tightening a published wire shape; per the constitution, breaking-class changes require human ratification and are never auto-approved unless review_decision is APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/asset-union.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-selection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/error.json (modified) matchesstatic/schemas/source/**; static/schemas/source/cor
…(truncated)…
aao-secretariat
Bot
added
the
ladon/needs-human-review
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated-path approval required plus breaking-class protocol change.
This PR modifies the protocol source of truth under static/schemas/source/** (40+ files, both added and modified), which places it under a hard, deterministic approval gate. gated_paths: true and review_decision is not APPROVED (unknown/none) — per decision-table row 2 the outcome MUST be escalate until a human/CODEOWNERS approval is recorded.
Independently, the constitution's Breaking-class hard gate applies: macro-bearing-url.json narrows all existing url asset fields to absolute HTTP(S), tightening a published wire shape. Breaking-class changes are never auto-approved; ratification is a human act and requires review_decision: APPROVED.
The current reviewer pass found no new blocking or Medium findings — the delta is a single normative reframing in canonical-formats.mdx:137 (fail-closed, internally coherent). Prior medium findings (macro-bearing-url narrowing; delivery-variants.mdx nav gap) sit in files out of scope for this delta. Net: the diff is clean, but the gate is not satisfied.
Escalation reasons
- Gated paths: PR modifies
static/schemas/source/**(e.g. macro-bearing-url.json, error.json, asset schemas) andreview_decisionis not APPROVED — human/CODEOWNERS approval required. - Breaking-class protocol change: macro-bearing-url.json narrows existing url asset fields to absolute HTTP(S), tightening a published wire shape — requires human ratification per the constitution.
Why human review
- Gated paths: PR modifies files under static/schemas/source/** (macro-bearing-url.json, error.json, asset schemas, and 40+ others) and review_decision is not APPROVED — human/CODEOWNERS approval required before merge.
- Breaking-class protocol change: macro-bearing-url.json narrows all existing url asset fields to absolute HTTP(S), tightening a published wire shape; per the constitution breaking-class changes are never auto-approved and require human ratification unless review_decision is APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/asset-union.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-selection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/error.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/macro-bearing-url.json (added) matchesstatic/schemas/source/**; static/schemas/so
…(truncated)…
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated-path approval gate unsatisfied.
This PR modifies 40+ files under static/schemas/source/**, which is under a hard, deterministic approval gate (gated_paths: true), and review_decision is REVIEW_REQUIRED, not APPROVED. Per decision-table row 2, the outcome must be escalate until a human/CODEOWNERS approval is recorded.
The reviewer found no critical/high/medium code-correctness issues in the delta: error.details refactored into three named error-detail schemas selected by error code with the base kept as an additionalProperties:true extension point (wire shape unchanged), the uri-template backward-compat branch restored on macro-bearing-url.json, VAST_VERSION_MISMATCH SHOULD→MUST tightened coherently across error-code.json/compliance-error-codes.mdx/video.mdx, tests and model-context budget updated. All $refs resolve, no oneOf discriminator regression, additionalProperties defaults preserved (DR-0009). A changeset is present (.changeset/creative-delivery-contracts.md).
The prior escalation reason concerning macro-bearing-url.json narrowing url fields appears addressed (backward-compat branch restored, wire shape unchanged). However, the gated-path gate independently requires human review, and this touches protocol source of truth including a SHOULD→MUST normative tightening (breaking-class posture). Human/CODEOWNERS review required before merge.
Escalation reasons
- Gated paths: PR modifies files under
static/schemas/source/**(asset-union.json, error-code.json, macro-* schemas, and 40+ others) andreview_decisionisREVIEW_REQUIRED, notAPPROVED. Human/CODEOWNERS approval required. - Normative tightening (VAST_VERSION_MISMATCH SHOULD→MUST) on published protocol surface — breaking-class changes are never auto-approved without human ratification.
Why human review
- Gated paths: PR modifies files under static/schemas/source/** (asset-union.json, error-code.json, macro-* schemas, and 40+ others) and review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required before merge.
- Normative tightening of VAST_VERSION_MISMATCH from SHOULD to MUST on the published protocol surface is a breaking-class change; per the constitution, breaking-class changes are never auto-approved and require human ratification unless review_decision is APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/asset-union.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/
…(truncated)…
There was a problem hiding this comment.
Ladon verdict: Request changes
Request changes — 1 blocking finding.
Blocking findings
- static/schemas/source/core/vast-media-file-requirements.json:79 — New published schema ships
additionalProperties: false, contradicting DR-0009's default and every sibling requirement schema. This is a spec-drift / governance-precedent violation on the published protocol surface (static/schemas/source/**), classifiedhighby the reviewer.
Row 1 of the decision table fires: a high finding is present, so the outcome is request-changes regardless of the gated-paths/high-risk state. Fix the schema to omit or set additionalProperties: true per DR-0009 before merge.
Note: this PR also touches gated paths (static/schemas/source/**) with review_decision: REVIEW_REQUIRED, so even once the high finding is resolved, human/CODEOWNERS approval will be required before this can proceed (the prior escalate). The reviewer credits the delta since the prior run for correct minor changeset scope, schema↔docs coherence, and no dist/** or oneOf regressions.
Blocking findings
- static/schemas/source/core/vast-media-file-requirements.json:79 — New published schema closes additionalProperties, contradicting DR-0009
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated paths touched without approving review.
This delta pass has no blocking or medium findings: the prior high finding (vast-media-file-requirements.json:79 closing additionalProperties, contradicting DR-0009) is resolved. The schema now flips additionalProperties back to true, restoring the DR-0009 default of extension-carrying published schemas, and a regression test in creative-delivery-contracts.test.cjs asserts a vendor_extension key still validates. Both delta files are clean; nothing new was flagged.
However, this PR modifies numerous files under static/schemas/source/**, which are under a hard, deterministic approval gate (gated_paths: true). review_decision is CHANGES_REQUESTED, not APPROVED, so the gate is not satisfied (decision-table row 2). In addition, this is a large protocol-surface change (new schemas, enums, error codes, error-details) that classifies as Normative/Breaking-class protocol work — ratification is a human act per the constitution's hard gate. Human/CODEOWNERS review is required before this can merge.
Escalation reasons below list the gated files requiring human review.
Why human review
- Gated paths under
static/schemas/source/**are modified/added butreview_decisionis CHANGES_REQUESTED, not APPROVED — a human/CODEOWNERS approval is required to satisfy the hard schema-source approval gate. - Protocol-surface change (new schemas, enums, error codes, error-details, canonical formats) constitutes normative/breaking-class spec work whose ratification is a human act per the constitution — cannot auto-merge without an approving human review.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/asset-union.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-selection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-bearing-url.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-declaration.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-encoding.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-resolution-capability.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-resolution-result.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-translation
…(truncated)…
|
Addressed the remaining DR-0009 must-fix in 7adff6f: the nested VAST MediaFile requirements object is extension-carrying ( |
|
All review findings are addressed in 1613da7, including the VAST technical acceptance follow-up (exact byte units, MediaFile delivery/MIME/container/codec/dimensions/bitrate/maximum bytes, explicit omission semantics, and completeness guarantees). The branch now includes origin/main. Validation completed locally: all 856 schemas, 9 focused creative-delivery contracts, 6,390 server tests, typecheck, current 3.2 storyboard matrix, released 3.0 compatibility matrix, docs navigation, compliance generation, and diff checks. Three independent protocol/code reviewers report no remaining blocker/high findings, and all GitHub review threads are resolved. Requesting the required human schema-source approval while fresh CI runs. |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard schema-source approval gate unsatisfied.
This PR modifies/adds numerous files under static/schemas/source/**, which are under a hard, non-overridable approval gate (gated_paths: true). The current GitHub review decision is CHANGES_REQUESTED, not APPROVED, so decision-table row 2 fires: the gate is not satisfied and the outcome must be escalate.
Additionally, this is a protocol-surface change (new schemas, enums, error codes, error-details, canonical formats, VAST MediaFile delivery methods, new capabilities). Per the constitution's hard gate, normative/breaking-class spec work requires human ratification and cannot auto-merge without an approving human review.
Reviewer findings: No blocking or Medium findings. Schema↔docs coherence, changeset scope/type (three minor changesets covering the wire surface), oneOf disjointness, and the normative async-identity/convergence contract are all sound. The clean diff does not lift the gate — only a recorded human/CODEOWNERS APPROVED review does.
Escalation reasons:
- Gated paths under
static/schemas/source/**are modified/added whilereview_decisionisCHANGES_REQUESTED— human/CODEOWNERS approval required. - Protocol-surface/normative spec change requiring human ratification per the constitution's breaking-class hard gate.
Prior decision was also escalate for the same gate reasons; this remains sticky solely due to the unsatisfied hard gate, not new findings.
Why human review
- Gated paths under
static/schemas/source/**are modified/added butreview_decisionis CHANGES_REQUESTED, not APPROVED — a human/CODEOWNERS approval is required to satisfy the hard schema-source approval gate. - Protocol-surface change (new schemas, enums, error codes, error-details, canonical formats) is normative/breaking-class spec work whose ratification is a human act per the constitution — cannot auto-merge without an approving human review.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/asset-union.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-selection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-bearing-url.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-declaration.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/
…(truncated)…
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard schema-source approval gate not satisfied, plus breaking-class protocol surface change requiring human ratification.
This PR touches many files under static/schemas/source/**, which are under a hard, non-overridable approval gate (gated_paths: true). The current review_decision is CHANGES_REQUESTED, not APPROVED, so the gate is not satisfied (decision table row 2).
The reviewer surfaced no critical/high/medium findings this run — the delta since the prior pass (vast-version-mismatch.json oneOf→anyOf on provably-disjoint arms; a compliance fixture adding vast_version: "4.0") is clean. But a clean diff does not lift the gate: it lifts only when a human/CODEOWNERS approval is recorded. This is a normative/breaking-class protocol surface change (new core schemas, enums, error codes, error-details, canonical formats) whose ratification is a human act per the constitution's "Breaking-class changes are never auto-approved" rule.
No blocking code findings; the block is the unsatisfied approval gate. A human/CODEOWNERS approval on the PR will clear this.
Escalation reasons:
- Gated paths under
static/schemas/source/**modified/added butreview_decisionis CHANGES_REQUESTED, not APPROVED — human/CODEOWNERS approval required. - Breaking-class / normative protocol-surface change (new schemas, enums, error codes, error-details, canonical formats) — ratification is a human act; never auto-approved.
Why human review
- Gated paths under
static/schemas/source/**are modified/added but review_decision is CHANGES_REQUESTED, not APPROVED — a human/CODEOWNERS approval is required to satisfy the hard schema-source approval gate. - Breaking-class / normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — per the constitution, ratification is a human act and cannot be auto-approved without an approving human review.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/asset-union.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-selection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-bearing-url.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-declaration.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-encoding.json (added) mat
…(truncated)…
|
Ladon cannot review this PR until merge conflicts are resolved. |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard schema-source approval gate is unsatisfied and this is a Breaking-class protocol change.
This PR modifies/adds ~50 files under static/schemas/source/** (new core schemas, enums, error codes, error-details, canonical formats) plus a gated .github/workflows/** file. gated_paths: true and review_decision: CHANGES_REQUESTED (not APPROVED), so the deterministic hard approval gate (decision table row 2) is NOT satisfied — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded.
The reviewer's subsequent pass over the authored delta (storyboard CI sharding workflow + its structure test, vast_version added to 5 CTV VAST fixtures) is clean: no critical/high/medium findings. The new sharding jobs are fail-closed (aggregate gates on shard result, exactly 12 shard logs, MIN_CLEAN/MIN_PASSED floors; contiguous non-overlapping shard slices; required check name preserved). No released-artifact immutability breach (zero dist/ files touched in git diff main..head). Minor changeset present.
However, the change is Breaking-class overall, and per the constitution ratification is a human act that is never auto-approved absent an approving human review. Both the path-based gate and the prose-based Breaking-class gate require escalation while review_decision is CHANGES_REQUESTED.
Blocking findings: none.
Escalation reasons:
- Gated paths under
static/schemas/source/**(and.github/workflows/**) are modified/added but review_decision is CHANGES_REQUESTED, not APPROVED — human/CODEOWNERS approval required to satisfy the hard schema-source approval gate. - Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification is a human act and cannot be auto-approved without an approving human review.
Why human review
- Gated paths under static/schemas/source/** and .github/workflows/** are modified/added but review_decision is CHANGES_REQUESTED, not APPROVED — a human/CODEOWNERS approval is required to satisfy the hard approval gate.
- Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification is a human act and cannot be auto-approved without an approving human review.
- This PR touches a path under a hard, non-overridable approval gate (.github/workflows/training-agent-storyboards.yml (modified) matches
.github/workflows/**; static/schemas/source/core/assets/asset-union.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matches `static/schema
…(truncated)…
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard approval gate unsatisfied on a breaking-class protocol change.
This PR modifies/adds many files under static/schemas/source/** and .github/workflows/**, both of which are under a deterministic hard approval gate. gated_paths: true and review_decision: CHANGES_REQUESTED (not APPROVED), so row 2 of the decision table fires: escalate.
Independently, this is a Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats). Per the constitution's hard gate, breaking-class changes are never auto-approved — ratification is a human act — and this too requires escalation absent an approving human review.
Escalation reasons
- Gated paths under
static/schemas/source/**and.github/workflows/training-agent-storyboards.ymlare modified/added butreview_decisionisCHANGES_REQUESTED, notAPPROVED. A human/CODEOWNERS approval is required to satisfy the hard approval gate. - Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification cannot be auto-approved without an approving human review.
Delta since the prior escalate is CI-only (storyboard sharding 12→24, matrix/cap/count updates, test assertions updated) and touches no protocol surface. No new critical/high/medium findings. The escalation persists on the standing gated-paths + breaking-class gates, not on a fresh reviewer finding. The gate lifts the moment review_decision becomes APPROVED.
Why human review
- Gated paths under static/schemas/source/** and .github/workflows/training-agent-storyboards.yml are modified/added but review_decision is CHANGES_REQUESTED, not APPROVED — human/CODEOWNERS approval is required to satisfy the hard approval gate.
- Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification is a human act and cannot be auto-approved without an approving human review.
- This PR touches a path under a hard, non-overridable approval gate (.github/workflows/training-agent-storyboards.yml (modified) matches
.github/workflows/**; static/schemas/source/core/assets/asset-union.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-selection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-bearing-url.json (ad
…(truncated)…
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard approval gate unsatisfied (gated paths + breaking-class protocol change).
This PR touches gated paths (static/schemas/source/** and .github/workflows/**) which sit under a hard, non-overridable approval gate. gated_paths: true and review_decision: CHANGES_REQUESTED (not APPROVED), so decision-table row 2 fires: outcome must be escalate.
Independently, this is a Breaking-class normative protocol-surface change — new core schemas, enums, error codes, error-details, and canonical formats. Per the repo's "Hard gate: Breaking-class changes are never auto-approved," ratification is a human act and cannot be auto-approved absent an approving human review.
No critical/high/medium findings were surfaced in this run. The delta since the prior escalate is a single edit — removing fetch-depth: 0 from the checkout in the new sales_storyboard_shards job — which the reviewer confirmed is safe (no git-history operations in the job's steps). Clean diff, but the hard gates stand until a human/CODEOWNERS approval is recorded.
Escalation reasons
- Gated paths under
static/schemas/source/**and.github/workflows/training-agent-storyboards.ymlare modified/added butreview_decisionisCHANGES_REQUESTED, notAPPROVED— human/CODEOWNERS approval required. - Breaking-class normative protocol change (new core schemas, enums, error codes, error-details, canonical formats) — ratification requires an approving human review.
Why human review
- Gated paths under static/schemas/source/** and .github/workflows/training-agent-storyboards.yml are modified/added but review_decision is CHANGES_REQUESTED, not APPROVED — the hard approval gate requires a human/CODEOWNERS approval before this can merge.
- Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification is a human act and cannot be auto-approved without an approving human review.
- This PR touches a path under a hard, non-overridable approval gate (.github/workflows/training-agent-storyboards.yml (modified) matches
.github/workflows/**; static/schemas/source/core/assets/asset-union.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-selection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-bearing-url.json (added) matchesstatic/schemas/source/**; static/schemas
…(truncated)…
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard approval gate unsatisfied on a breaking-class protocol change.
This PR modifies/adds files under static/schemas/source/** and .github/workflows/training-agent-storyboards.yml, both of which are declared under a hard, non-overridable approval gate (gated_paths: true). The current review_decision is CHANGES_REQUESTED, not APPROVED, so per decision-table row 2 this cannot auto-approve and must escalate.
Independently, the diff introduces new core schemas, enums, error codes, error-details, and canonical formats — a Breaking-class normative protocol-surface change. Per the repo's "Hard gate: Breaking-class changes are never auto-approved," ratification is a human act and requires an approving human review before merge.
The reviewer surfaced no critical/high/medium findings on the delta (the storyboard shard-count 24→48 bump is self-consistent across all four load-bearing sites with the test updated to match). So there are no blocking code-level findings — but the hard gates stand regardless of how clean the diff is. This mirrors the prior escalate decision; the gate remains unsatisfied.
Escalation reasons
- Gated paths under
static/schemas/source/**and.github/workflows/training-agent-storyboards.ymlare modified/added butreview_decisionis CHANGES_REQUESTED, not APPROVED — human/CODEOWNERS approval required. - Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification is a human act and cannot be auto-approved.
Why human review
- Gated paths under static/schemas/source/** and .github/workflows/training-agent-storyboards.yml are modified/added but review_decision is CHANGES_REQUESTED, not APPROVED — the hard approval gate requires a human/CODEOWNERS approval before this can merge.
- Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification is a human act and cannot be auto-approved without an approving human review.
- This PR touches a path under a hard, non-overridable approval gate (.github/workflows/training-agent-storyboards.yml (modified) matches
.github/workflows/**; static/schemas/source/core/assets/asset-union.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-rejection.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/delivery-variant-selection.json (added) matchesstatic/schemas/source/**; static/schemas/source/c
…(truncated)…
2483928 to
4c09fd0
Compare
Resolved in subsequent commits: the schema now follows DR-0009. Later Ladon reviews report no blocker/high findings and escalate only for the required human/CODEOWNER approval.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated paths touched without an approving review.
This PR modifies/adds many files under the hard approval gate: static/schemas/source/** (core schemas, enums, error codes, error-details, canonical formats) and .github/workflows/training-agent-storyboards.yml. gated_paths is true and review_decision is REVIEW_REQUIRED (not APPROVED), so decision-table row 2 fires: outcome MUST be escalate, not approve.
Separately, the repo's Repo Context declares a prose hard gate: Breaking-class normative protocol-surface changes (new core schemas, enums, error codes, canonical formats) are never auto-approved — ratification is a human act — unless review_decision is APPROVED. That condition is also unmet here.
The reviewer's current findings are clean (the delta reviewed was CI/test infrastructure only: storyboard quarantine, deterministic socket teardown, MIN_PASSED floor drop, and new sharding assertions — no schema/docs/wire-shape changes in this delta, fail-closed preserved). No critical/high/medium findings. That is why this is not request-changes. But a clean diff cannot lift a deterministic path-based gate or the breaking-class governance gate — only a human/CODEOWNERS APPROVED review can.
Prior decision was also escalate for the same gate reasons; the gate remains unsatisfied.
Escalation reasons
- Gated paths under
static/schemas/source/**and.github/workflows/training-agent-storyboards.ymlare modified/added butreview_decisionisREVIEW_REQUIRED, notAPPROVED— human/CODEOWNERS approval is required before merge. - Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification is a human act and cannot be auto-approved without an approving human review.
Why human review
- Gated paths under static/schemas/source/** and .github/workflows/training-agent-storyboards.yml are modified/added but review_decision is REVIEW_REQUIRED, not APPROVED — the hard approval gate requires a human/CODEOWNERS approval before this can merge.
- Breaking-class normative protocol-surface change (new core schemas, enums, error codes, error-details, canonical formats) — ratification is a human act and cannot be auto-approved without an approving human review.
- This PR touches a path under a hard, non-overridable approval gate (.github/workflows/training-agent-storyboards.yml (modified) matches
.github/workflows/**; static/schemas/source/core/assets/asset-union.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-delivery-variant.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-source.json (added) matches `static
…(truncated)…
|
All CI is green (97 successful checks; the 62-job storyboard workflow and aggregate passed), all review threads are resolved, and the latest automated plus protocol/code expert reviews found no blocker/high findings. Auto-merge is enabled.\n\n@ohalushchak-exadel @bmilekic — a human/CODEOWNER approval is the only remaining merge gate when you have a moment. |
bokelley
changed the title
Schema Link Check ResultsCommit: ❌ Errors (schema not found)These schemas do not exist and the links will be broken:
|
There was a problem hiding this comment.
Ladon verdict: Request changes
Request changes — 1 blocking finding.
Blocking findings
.changeset/creative-delivery-contracts.md:2—minorchangeset ships a breaking optional→required flip:vast_versionis now required on canonicalvideo_vastmanifests. This is proven breaking by the PR having to patch previously-valid compliance vectors. Per repo policy, an optional↔required flip on a published field is a breaking wire change and must ship asmajor, notminor— this is ahighfinding. Bump the changeset tomajor.
Medium findings
- None.
Note on other gates
This PR also touches gated paths under static/schemas/source/** with review_decision unknown/not APPROVED, and is a breaking-class protocol-surface change — both of which independently require human/CODEOWNERS ratification and prevent auto-approve. Those gates remain unsatisfied. However, the decision table is top-down: the high finding fires row 1, so the outcome is request-changes. Fix the changeset type and obtain the required human approval; the low macro-declaration nit (macro-declaration.json:2964, IAB-VAST × catalog-semantic combination silently unsatisfiable) is non-blocking.
Blocking findings
.changeset/creative-delivery-contracts.md:2— minor changeset ships a breaking optional→required flip (vast_versionnow required on canonical video_vast manifests); breaking wire changes require amajorchangeset
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated paths touched and required review not yet satisfied.
The prior blocking finding (minor changeset shipping a breaking optional→required flip on vast_version) has been resolved: the requirement was removed from the canonical/legacy creative-asset.json and creative-manifest.json paths and relocated to the newly-added creative-representation.json, where requiring the field is additive. Tests were updated to match (canonical/manifest/asset paths validate with vast_version omitted; a new validateVariant test enforces the constraint on the opt-in representation path). The current run surfaces no Medium-or-worse findings.
However, this PR modifies/adds many files under static/schemas/source/**, which is a hard, deterministic approval gate (gated_paths: true). The required review status is CHANGES_REQUESTED, not APPROVED, so decision-table row 2 fires: the gate is not satisfied and the outcome cannot be approve or plain comment. Human/CODEOWNERS approval is required before this can merge.
Escalation reasons
- Protocol schema source under
static/schemas/source/**is under a hard approval gate;review_decisionisCHANGES_REQUESTED(notAPPROVED). Requires human/CODEOWNERS review. - Large protocol-surface change (new representation/macro schemas + canonical format edits) — verify schema↔docs coherence and changeset classification (potential breaking-class change) as part of human ratification.
Why human review
- Gated paths under static/schemas/source/** modified and review_decision is CHANGES_REQUESTED (not APPROVED) — human/CODEOWNERS approval required (decision-table row 2).
- Broad protocol-surface change adding new representation/macro schemas and editing canonical formats; ratification of any breaking-class protocol change is a human act.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/asset-union.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-localization.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-representation-set.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-representation.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-bearing-url.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-declaration.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-encoding.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-resolution-capability.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-resolution-result.js
…(truncated)…
Resolved in 16dc108: exact VAST version is required only for the new opt-in representation path. Ladon rerun at this head reports no Medium-or-worse findings; human/CODEOWNER approval remains required.
|
Looked at thsi on top of #6781 the two-fingerprint split (buyer source revision vs. seller-selected execution) is the right call. Adding some notes about re-review/serving, not the schema.
Minor: the “Identity model” table differs between #6781 (5 namespaces) and here (6, +locale_variant_id). No semantic conflict, but the canonical list should read identically, people copy it. What’s nice, so I’m not just piling on: the two-fingerprint split, deterministic selection (two sellers picking differently is intended and auditable), macro ownership in managed service, and the injection/encoding safety model all look right. |
|
Thanks for the careful read, @pkras. Working through each point: 1 & 2 — Seller re-selection and notification gap. The concern is real: 3 — Representation sets scoped to one format. Your reading is correct per the resolution contract: selection is "bound to an exact destination product format option," so a set groups interchangeable packagings for one format (inline vs. URL tag of the same banner; VAST 4.2 vs. 3.0 of the same video) — not cross-media-type. Worth stating outright in the spec text, as you suggest. 4 — Localization migration. The PR already calls out the exclusion constraint ("incoming and previously stored localization must be absent before selection lineage can be applied"), but the destructive sync-then-re-review path is implicit. A short migration note — "clear localization first; a re-review window exists between steps; no atomic path" — would keep the constraint visible. 5 — Identity table consistency. Minor, but a copy risk: adding Generated by Claude Code |
|
Follow-up on @pkras's review — applied two fixes and one answer: Points 3 & 4 — committed to this branch (a8995e4):
Point 5 — identity table consistency: Points 1 & 2 — seller-capability-change trigger gap: Confirmed: #6781 does not address this trigger path. It defines the re-review lifecycle for buyer-edit triggers only. The seller-capability-change scenario (@pkras's concern: seller drops a VAST version → @bokelley — the follow-up spec issue vs. adding to #6781 is your call. Generated by Claude Code |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard approval gate on gated schema paths unsatisfied.
This PR modifies and adds many files under static/schemas/source/**, which is a hard, non-overridable approval gate (gated_paths: true). The current review_decision is unknown/none — not APPROVED — so decision-table row 2 fires and the outcome must be escalate (never approve, never plain comment).
The reviewer's current pass found no critical/high/medium findings; the delta since the prior pass is docs-only (two accurate normative prose clarifications in docs/creative/representation-sets.mdx), so there are no blocking code findings. But this is a broad protocol-surface change introducing new representation/macro schemas and editing canonical formats — a governance/ratification act that is a human decision per the repo's breaking-class hard gate.
Escalation reasons:
- Files under
static/schemas/source/**(gated paths) are modified/added andreview_decisionis notAPPROVED— human/CODEOWNERS approval required (row 2). - Broad protocol-surface change adding new representation/macro schemas and editing canonical formats; ratification of any breaking-class protocol change is a human act (constitution hard gate).
No findings to fix — the gate lifts the moment a human/CODEOWNERS approval is recorded. This is consistent with the prior escalate decision; the gate condition persists.
Why human review
- Gated paths under static/schemas/source/** modified/added and review_decision is not APPROVED (unknown/none) — human/CODEOWNERS approval required (decision-table row 2).
- Broad protocol-surface change adding new representation/macro schemas and editing canonical formats; ratification of a breaking-class protocol change is a human act per the constitution's hard gate. Escalate unless review_decision is APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/assets/asset-union.json (modified) matches
static/schemas/source/**; static/schemas/source/core/assets/daast-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/daast-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/display-tag-asset.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/assets/html-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/javascript-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/pixel-tracker-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/url-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/assets/vast-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/creative-asset.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-localization.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-manifest.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/creative-representation-set.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/creative-representation.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-bearing-url.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-declaration.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-encoding.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-resolution-capability.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/macro-resolution-res
…(truncated)…
|
Addressed in
The stability/trigger rules are present in the normative representation-set and On the identity-table note: |
… fix-6767-review-comments # Conflicts: # package.json
… fix-6767-review-comments
pkras
left a comment
pkras
left a comment
There was a problem hiding this comment.
Went another layer down on this, less on the schema and more on what the feature actually does at serve time. Mechanism looks right; a couple of things bug me about the guarantees.
- “These reps are all the same ad” is pure trust - not sure if anyone checks it.
The digest proves the set didn’t get tampered with, but nothing proves rep A actually matches rep B. And the seller serves whichever one it pins without ever looking at it (“cannot claim it independently inspected hidden source representations”). So a buyer can pair a clean VAST 4.2 with a broken 2.0 and whoever gets the 2.0 eats it — bait-and-switch or just honest drift. Pre-this, the seller reviewed the exact bytes it’d run. Now it might pin bytes it never saw. Is that intentional? If so I think it needs to be said out loud, and maybe a “seller reviews every candidate it might pin” rule.
- Pinning kills fallback, and “highest compatible” is seller-compatible, not device-compatible.
Normally a player tries the new VAST and falls back to an old one if it chokes. Here the seller locks in one rep up front, no runtime fallback. And highest_compatible_vast = highest the seller accepts, not highest the viewer’s device can play. So on old CTV boxes you can pin a version that won’t play, with no backup. Feels like the multi-rep richness solves “which seller” but throws away “which device” — worth making that limitation explicit.
Backing this up: the tests kind of confirm the failure side is underbaked — every resolution vector picks a rep successfully, the “nothing fits → reject everything” path is never exercised end to end (5 of 7 rejection codes and 8 of 12 macro-failure reasons have zero coverage). The happy path is solid; the interesting paths aren’t tested.
Smaller stuff while I’m here:
representation-selection.json points its spec anchor at #deterministic-resolution, which doesn’t exist — should be #seller-bound-resolution. Nothing validates that so it’ll just rot.
New macro schemas aren’t registered in index.json even though the representation ones are — looks arbitrary.
macro-declaration.json field docs say namespace/revision are required for “IAB VAST and vendor” but the schema also requires them for DAAST — prose is missing a case.
None of this blocks the mechanism — it’s about whether the guarantees match how video actually traffics. #1 and #2 are the ones I’d want an answer on before it’s load-bearing.
|
Addressed the latest review in
Validation passed: focused creative-delivery contracts 9/9; full |
* feat(creative): add creative revision identity * test(creative): align historical locale attribution * fix(creative): gate served variant identity scope * docs(creative): clarify revision serving workflow * feat(creative): define delivery representations and macro resolution (#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> * fix(creative): gate revision guarantees on capability * fix(compliance): inspect per-creative revision errors * fix(schemas): keep macro capabilities out of canonical params --------- Co-authored-by: Claude <noreply@anthropic.com>
3 participants
Summary
Depends on #6781, which establishes the prerequisite identity hierarchy:
creative_id: durable logical creativerevision_id: immutable buyer-authored content staterepresentation_id: one equivalent source representation inside that revisionbuild_variant_id: a generated build candidatelocale_variant_id: a localized executionvariant_id: an agent-assigned served execution identityRepresentation selection does not mint a buyer revision. The complete representation set binds the source revision digest; the selected representation and derived output bind a separate execution/review fingerprint. Changing the selected execution triggers normal review without changing the buyer's source revision.
Resolution contract
build_creativerepresentation resolution is a pure, single-output, seller-authorized mode bound to an exact destination product format option.representation_selectionlineage and one shared selected-output digest projection.Relationship to #6207
This PR defines the reusable macro-processing vocabulary but does not claim that a product accepts or initiates buyer-supplied trackers.
#6207 should add a format-option-scoped tracker execution contract that composes with the exact macro capability tuple introduced here:
Preview event/macro observation is a separate runtime-evidence surface: it can prove one controlled execution, but it must not be conflated with the seller's production tracker commitment.
Verification
npm run test:schemasnpm run test:json-schemanpm run test:compliance-snippetsnpm run test:oneof-discriminatorsnpm run test:error-code-driftnode --test tests/creative-delivery-contracts.test.cjs tests/creative-revisions.test.cjs(14/14)npm run typechecknode scripts/check-changeset-protocol-scope.cjs origin/maingit diff --checkCloses #6761
Closes #6762
Closes #6763
Closes #6764