Skip to content

feat(media-buy): add targeting-aware product discovery - #6203

Open
bokelley wants to merge 18 commits into
security-wave-12-get-products-idempotencyfrom
verify-get-products-filters
Open

feat(media-buy): add targeting-aware product discovery#6203
bokelley wants to merge 18 commits into
security-wave-12-get-products-idempotencyfrom
verify-get-products-filters

Conversation

@bokelley

@bokelley bokelley commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Stack dependency

Depends on #6115. This PR is intentionally based on security-wave-12-get-products-idempotency; review the targeting-aware discovery changes relative to that branch.

Summary

This proposes the AdCP 3.2 targeting-aware product-discovery contract:

  • filters select eligible offers and apply consistently in brief, wholesale, and refine modes.
  • targeting_overlay carries concrete targeting the buyer intends to apply, so availability, pricing, and forecasting can reflect it.
  • required_overlay_support declares dimensions that must remain independently selectable when the buyer later creates packages.
  • sellers return sparse targeting_resolution.modifications only when applied structured targeting differs from the request.
  • hard targeting inferred from brief prose remains binding; sellers should confirm the structured interpretation, and must do so when it materially changes eligibility, price, or forecast.
  • request-specific configurations use opaque, expiring custom product IDs rather than assuming catalog IDs are stable.
  • placement, property, and collection selection follow the same targeting lifecycle.
  • typed device_platform_exclude uses exclusion-wins semantics and independently declared product support.

The implementation includes schemas, normative design and migration guidance, task/reference docs, buyer education, the media-buy skill, curriculum guidance, compliance storyboards, test vectors, and regression coverage.

Contract decisions

  • Prefer structured targeting over equivalent brief prose: it is deterministic, token-efficient, and lossless. Explicit hard prose is still binding.
  • Do not echo an unchanged structured overlay. If the seller modifies it—for example by mapping ages to supported buckets—the response discloses the modification, and selecting that configured product accepts it.
  • Keep offer filtering, concrete targeting, and future package-time support distinct. required_overlay_support promises independent selectability, not one product per requested geography or audience value.
  • Fixed placement sets can inherently satisfy an exact named request without becoming independently selectable. Placement updates and creative-routing replacements are atomic.
  • External buyers do not receive a generic ad-server control plane. Arbitrary custom, key_value_pairs, and equivalent raw targeting maps remain explicitly out of scope.
  • Every committed allocation identifies its exact pricing_option_id; binding commercial terms cannot be inferred from an ambiguous product-level price list.
  • Recognizable, caller-authorized configured products return PRODUCT_EXPIRED after expiry. Unknown, evicted, or cross-account/lineage IDs return PRODUCT_NOT_FOUND without leaking cross-tenant existence.

Compatibility and rollout

  • The new structured discovery fields are negotiated through AdCP 3.2 version selection (adcp.supported_versions and request adcp_version); there is no second feature flag that could disagree with the negotiated schema.
  • Existing AdCP 3.1 and 3.0 request/response behavior remains version-gated and compatibility-tested.
  • filters are not wholesale-only. Their predicates apply in brief, wholesale, and refine discovery.
  • The training runtime fails closed with UNSUPPORTED_FEATURE for 3.2 split-flow structured targeting until generated 3.2 SDK support lands in Implement targeting-aware discovery in the training agent after 3.2 beta #6199. It does not silently ignore preview fields or pretend to reprice/reforecast them.
  • Targeting extensions round-trip, while auth_token values on property and collection lists are removed from create/get/update wire responses.

Issue coverage

Expert review

Three independent review passes—ad-tech protocol, protocol/schema workflow, and code/security behavior—approved the integrated tree as merge-ready. Review feedback drove the lifecycle-specific resolution schemas, fixed-versus-selectable placement model, exact product-ID pagination behavior, credential stripping, fail-closed training rollout, and separate request/support shapes.

Validation

  • Required current-version storyboard matrix: all seven tenants clean (/sales: 95 storyboards, 479 steps).
  • Released AdCP 3.0 compatibility matrix: all seven tenants clean (/sales: 65 storyboards, 310 steps).
  • Focused canonical_formats: 15/15 steps clean after applying the repository's version-pinned SDK rc.13 postinstall shim.
  • SDK shim tests: 8/8 passed.
  • Root unit suite: 67 files, 1,040 tests passed.
  • Targeting-aware discovery, schema parity, proposal lifecycle, MCP projection, composed-schema, examples, and schema-link checks passed.
  • Compliance build and storyboard lint suite passed.
  • Docs navigation: 21/21 checks passed.
  • TypeScript typecheck passed.

Community feedback requested

  1. Is response-level brief_targeting confirmation the right scope for a curated result set?
  2. Is selecting a configured product sufficient acceptance of sparse targeting modifications, or should a future version carry an explicit digest acknowledgment?
  3. Does required_overlay_support communicate independent package-time selection clearly enough?
  4. Is the fixed-versus-selectable placement model operationally sufficient?
  5. Is is_custom the right marker for request-bound configured offers?

@bokelley bokelley changed the title Add targeting-aware product discovery feat(media-buy): add targeting-aware product discovery Aug 5, 2026
@bokelley
bokelley marked this pull request as ready for review August 5, 2026 05:49
@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Aug 5, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated-path change requires human/CODEOWNERS review.

This PR modifies/adds ~15 files under static/schemas/source/**, which are under a hard, deterministic approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so decision-table row 2 fires and the outcome cannot be approve or plain comment.

Independent of the gate, this is a Breaking-class-eligible protocol change (new AdCP 3.2 targeting-aware product-discovery contract) touching published wire surface. Under the repo's "Breaking-class changes are never auto-approved" hard gate, ratification is a human act.

The reviewer found no Critical/High/Medium findings: schema↔docs coherence verified (product.json overlay_support / targeting_resolution / is_custom / expires_at match get_products.mdx and targeting.mdx), the demographic_targeting_resolution→targeting_resolution.demographics rename is on an unreleased 3.2-beta field (absent from dist/**) so the minor changeset is correct, both new oneOf schemas carry discriminators (mode/operation), and released dist artifacts are untouched. Clean diff, but the deterministic gate stands until a human/CODEOWNERS approval lands.

Escalation reasons

  • Modifies/adds ~22 files under static/schemas/source/** (gated path) with review_decision: REVIEW_REQUIRED — human/CODEOWNERS approval required before merge.
  • New published protocol contract (AdCP 3.2 targeting-aware discovery) requires human ratification per the Breaking-class hard gate.

No blocking code findings; escalation is procedural (approval gate), not a defect.

Why human review

  • Modifies/adds ~22 files under static/schemas/source/** (gated path) while review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required (row 2).
  • New AdCP 3.2 published protocol contract requires human ratification per the repo's Breaking-class hard gate.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/creative-assignment.json (modified) matches static/schemas/source/**; static/schemas/source/core/package-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/package.json (modified) matches static/schemas/source/**; static/schemas/source/core/placement-selection.json (added) matches static/schemas/source/**; static/schemas/source/core/placement.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-filters.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/product.json (modified) matches static/schemas/source/**; static/schemas/source/core/targeting-modification.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-requirements.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-support.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting.json (modified) matches static/schemas/source/**; static/schemas/source/core/x-entity-types.json (modified) matches static/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matches static/schemas/source/**; static/schemas/source/index.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buys-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-request.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/media-buy/package-request.json (modified) matches `static

…(truncated)…

@aao-secretariat

Copy link
Copy Markdown
Contributor

Ladon cannot review this PR until merge conflicts are resolved.

…ilters

# Conflicts:
#	docs/media-buy/task-reference/create_media_buy.mdx
#	skills/adcp-media-buy/SKILL.md
#	static/schemas/source/media-buy/get-products-request.json
…ilters

# Conflicts:
#	docs/media-buy/task-reference/create_media_buy.mdx
#	docs/media-buy/task-reference/get_products.mdx
#	package.json
#	static/schemas/source/core/product.json
#	static/schemas/source/media-buy/get-products-request.json

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — hard approval gate not satisfied.

This PR modifies/adds ~22 files under static/schemas/source/**, a gated path under a hard, non-overridable approval gate. The current GitHub review_decision is REVIEW_REQUIRED, not APPROVED, so decision-table row 2 fires and the outcome cannot be approve or plain comment.

Additionally, this is a new AdCP protocol contract change (targeting-aware product discovery — new fields on the request/Product/targeting overlay, and a renamed Package.demographic_targeting_resolutiontargeting_resolution.demographics). A field rename is a Breaking-class wire change, and the repo's Breaking-class hard gate states such changes are never auto-approved — ratification is a human act — unless review_decision is APPROVED.

The reviewer found the diff clean on schema↔docs coherence, changeset type, oneOf discrimination, and released-artifact immutability — no blocking or Medium code-correctness findings. That is good, but it does not lift the gate; only a recorded human/CODEOWNERS approval does.

Prior decision was also escalate for the same gated-path reason. The gate persists because review_decision is still REVIEW_REQUIRED.

Escalation reasons

  • Modifies/adds ~22 files under static/schemas/source/** (gated path) while review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required (row 2).
  • Breaking-class change (field rename Package.demographic_targeting_resolutiontargeting_resolution.demographics) — ratification must be a human act per the repo's Breaking-class hard gate.

Why human review

  • Modifies/adds ~22 files under static/schemas/source/** (gated path) while review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required (decision-table row 2).
  • Breaking-class protocol change: Package.demographic_targeting_resolution renamed to targeting_resolution.demographics. Per the repo's Breaking-class hard gate, ratification is a human act and is never auto-approved unless review_decision is APPROVED.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/creative-assignment.json (modified) matches static/schemas/source/**; static/schemas/source/core/package-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/package.json (modified) matches static/schemas/source/**; static/schemas/source/core/placement-selection.json (added) matches static/schemas/source/**; static/schemas/source/core/placement.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-filters.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/product.json (modified) matches static/schemas/source/**; static/schemas/source/core/targeting-modification.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-requirements.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-support.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting.json (modified) matches static/schemas/source/**; static/schemas/source/core/x-entity-types.json (modified) matches static/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matches static/schemas/source/**; static/schemas/source/index.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buys-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-request.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-targeting-resolution.json (added) matches `stati

…(truncated)…

@bokelley

bokelley commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Final merge-readiness pass complete on 02ed818:

  • merged current main and resolved overlapping targeting-aware discovery, audience-evidence, attestation, canonical-format, docs, and aggregate-test changes
  • preserved targeting support/resolution and audience-evidence decision readback together in Product and get_products field projection
  • aligned wholesale cache identity across request schema, response schema, task docs, and regression coverage so targeting_overlay and required_overlay_support cannot share an incorrect feed version
  • clarified that filters apply identically in brief, wholesale, and refine, while targeting_overlay is delivery intent and required_overlay_support is later package-time capability
  • revalidated docs/examples, all 692 schemas, targeting vectors, audience evidence/attestations, compliance build, current training storyboards, 3.0-compatible storyboards, typecheck, 363 server-unit files (5,281 passing tests), and server integration
  • all GitHub checks are green; Ladon reports no blocking or Medium correctness findings
  • no unresolved review threads; PR is mergeable

The only remaining gate is the intentional human/CODEOWNERS approval for the published protocol/schema change.

…ilters

# Conflicts:
#	static/schemas/source/core/creative-assignment.json

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated-path hard gate not satisfied + Breaking-class protocol change.

This PR modifies/adds ~22 files under static/schemas/source/**, a hard, non-overridable approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so decision-table row 2 fires: Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

Separately, the repo's Breaking-class hard gate applies: the change renames a published wire field (Package.demographic_targeting_resolutiontargeting_resolution.demographics). Breaking-class ratification is a human act and is never auto-approved unless review_decision is APPROVED.

Escalation reasons

  • Touches gated paths under static/schemas/source/** while review_decision is REVIEW_REQUIRED (not APPROVED) — human/CODEOWNERS review required (row 2).
  • Breaking-class protocol change (published field rename) — human ratification required per repo Breaking-class hard gate.

Note on iteration: the reviewer reports a clean delta with no Critical/High/Medium findings (enum extension is additive with a minor changeset; deprecation sweep is metadata-only with new test coverage; certification server paths verified race-safe and timeout-bounded). No blocking code findings exist, but the two hard gates above stand until a human approval is recorded.

Blocking findings: none.

Why human review

  • Touches gated paths under static/schemas/source/** (22 files modified/added) while review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision-table row 2).
  • Breaking-class protocol change: published field Package.demographic_targeting_resolution renamed to targeting_resolution.demographics. Per the repo's Breaking-class hard gate, ratification is a human act and is never auto-approved unless review_decision is APPROVED.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/creative-assignment.json (modified) matches static/schemas/source/**; static/schemas/source/core/package-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/package.json (modified) matches static/schemas/source/**; static/schemas/source/core/placement-selection.json (added) matches static/schemas/source/**; static/schemas/source/core/placement.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-filters.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/product.json (modified) matches static/schemas/source/**; static/schemas/source/core/targeting-modification.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-requirements.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-support.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting.json (modified) matches static/schemas/source/**; static/schemas/source/core/x-entity-types.json (modified) matches static/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matches static/schemas/source/**; static/schemas/source/index.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buys-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-request.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/media-buy/package-request.json (modified) matches `static/schemas/source/*

…(truncated)…

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated-paths hard approval gate not yet satisfied.

This PR modifies/adds 22 files under static/schemas/source/**, which is a hard, deterministic approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so decision-table row 2 fires: Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

The reviewer found no Critical/High/Medium findings — the delta is a docs/compliance/test refinement pass on the targeting-aware-discovery feature. Description-only schema edits land on net-new 3.2 rollup fields (absent from released 3.1.10), stay coherent with the updated docs, and carry the correct minor changeset. That clean-diff assessment is credited, but it does not lift the gate.

The prior decision was also escalate for the same gated-paths reason. The gate persists purely because review_decision is still not APPROVED.

Escalation reasons

  • Touches gated paths under static/schemas/source/** (22 files modified/added) while review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (row 2).
  • Schema-source changes fall under spec-ownership governance; ratification is a human act.

No blocking findings exist; once a human/CODEOWNERS approval is recorded, this PR falls through to a normal approve on the clean diff.

Why human review

  • Touches gated paths under static/schemas/source/** (22 files modified/added) while review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision-table row 2).
  • Protocol schema-source edits fall under the repo's spec-ownership governance; ratification is a human act and cannot be auto-approved until review_decision is APPROVED.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/creative-assignment.json (modified) matches static/schemas/source/**; static/schemas/source/core/package-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/package.json (modified) matches static/schemas/source/**; static/schemas/source/core/placement-selection.json (added) matches static/schemas/source/**; static/schemas/source/core/placement.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-filters.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/product.json (modified) matches static/schemas/source/**; static/schemas/source/core/targeting-modification.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-requirements.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-support.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting.json (modified) matches static/schemas/source/**; static/schemas/source/core/x-entity-types.json (modified) matches static/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matches static/schemas/source/**; static/schemas/source/index.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buys-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-request.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/media-buy/package-request.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/package-update.json (modified) matches static/schemas/source/**; static/schemas/source/protocol/get-ad

…(truncated)…

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated schema-source paths require human/CODEOWNERS approval.

No blocking findings. Reviewer reports a clean pass: the get_products GetProductsRejected arm, rights-grant attestation binding, 3.2 advisory brand-authorization cross-check, and account-webhook proof-of-control are all coherent across schema↔docs↔server↔tests. Changesets are present and correctly minor/patch; wire changes are additive or ride experimental (x-status) surfaces. No new undiscriminated oneOf. Op-readiness paths (webhook-challenge/account-handlers) are fail-closed and test-covered.

However, this PR modifies/adds 22 files under static/schemas/source/**, which the repo declares under a hard, non-overridable approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED. Per decision-table row 2, this must escalate until a human/CODEOWNERS approval is recorded. The repo's spec-ownership governance reinforces this: protocol schema-source edits are a human act of ratification and cannot be auto-approved.

This sticky-escalates the prior escalate decision — but note it holds on row 2 (the deterministic gated-paths gate), independent of row 6. No fresh actionable findings surfaced this run; the escalation is driven purely by the unsatisfied approval gate.

Escalation reasons

  • Touches gated paths under static/schemas/source/** (22 files added/modified) while review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (row 2).
  • Protocol schema-source edits fall under spec-ownership governance; ratification is a human act.

Once a human/CODEOWNERS approval is recorded, this clean diff falls through to approve.

Why human review

  • Touches gated paths under static/schemas/source/** (22 files added/modified) while review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision-table row 2).
  • Protocol schema-source edits fall under the repo's spec-ownership governance; ratification is a human act and cannot be auto-approved until review_decision is APPROVED.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/creative-assignment.json (modified) matches static/schemas/source/**; static/schemas/source/core/package-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/package.json (modified) matches static/schemas/source/**; static/schemas/source/core/placement-selection.json (added) matches static/schemas/source/**; static/schemas/source/core/placement.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-filters.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/product.json (modified) matches static/schemas/source/**; static/schemas/source/core/targeting-modification.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-requirements.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-support.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting.json (modified) matches static/schemas/source/**; static/schemas/source/core/x-entity-types.json (modified) matches static/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matches static/schemas/source/**; static/schemas/source/index.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buys-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-request.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-products-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-pr

…(truncated)…

…ilters

# Conflicts:
#	static/compliance/source/universal/version-negotiation.yaml
@lukasz-pubx

Copy link
Copy Markdown

Proposal review — reviewed at head c911d0e514b277be7049d3f65b12804b4923d3cd (full tree read: all 7 new schemas, request/response surfaces, design spec, migration doc, task docs, both new compliance scenarios; baseline: live docs get_products @ 3.1.2)

The tree delivers what the description promises — schemas, docs, scenarios, tests all present and consistently wired — and several parts deserve explicit support: the lifecycle-specific resolution schemas that make invalid combinations unrepresentable, the machine-checkable product.json conditionals, deterministic INVALID_REQUEST on conflicting legacy+overlay dual input, the wholesale cache-tuple update for the new request fields, and the seeded negative-control filter storyboard that turns brief-mode filter no-ops into certification failures. Findings below, most important first.


1. warning — create-time pricing/forecast disposition for overlay_support-reserved dimensions is undefined; REQUOTE_REQUIRED only exists on update

error-code.json scopes the guard to updates: "An update_media_buy request changes the parameter envelope (budget, flight dates, volume, targeting) the original quote was priced against." But the analogous create-time path has no disposition at all. A buyer who used required_overlay_support (values unknown at discovery) gets discovery pricing/forecasts that are explicitly not broken out by the future values (get_products.mdx: required_overlay_support "does not request products or packages broken out by DMA or placement"), then applies those values as a package targeting_overlay at create_media_buy — legal, because they're declared in overlay_support. The only stated rule is package-request.json's "The seller applies the intersection exactly or rejects the package."

Failure narrative: a fixed-price product quoted against US-wide discovery assumptions; the buyer books it with an LA-DMA-only package overlay. Seller A honors the discovery CPM against a forecast that no longer holds; seller B rejects the package with no defined code for "this shape needs a new quote." Two conformant sellers, observably divergent, on the booking path — and the buyer's impression estimate is stale either way.

Ask — pick one and state it normatively:

  • (a) extend REQUOTE_REQUIRED semantics to create_media_buy for package overlays that leave the priced envelope (the error-code description currently forbids this reading);
  • (b) state that discovery pricing_options remain binding for any package targeting within declared overlay_support — the seller carries the risk, which also prices the incentive to declare support honestly; or
  • (c) state that forecast staleness under required_overlay_support is carried by the buyer, and buyers needing bound quotes MUST re-discover with concrete targeting_overlay values before booking.

Any of the three closes it; today the tree implies none of them.

2. warning — the required_overlay_support coverage rule is undefined for boolean-requirement × object-support, and the spec's own example falls into the hole

All three copies state the same two clauses — targeting-overlay-requirements.json: "covers a required field when: (1) the product field is true… or (2) both values are objects…" — yet the same description says "Numeric seller limits… do not participate in requirement matching." These conflict when the requirement is true and the support is an object. placement_selection is boolean-only on the request side while its support-side object form contains only limit fields, so for the design spec's own example ("placement_selection": true), a product disclosing placement_selection: {max_values_per_package: 3} matches neither clause — but "limits do not participate" says it should match. Same for geo_metros: true vs {systems: [...]}. Two conformant sellers return different product sets for the identical requirement, and sellers learn to hide limits behind bare true to stay matchable. Ask: add a third clause (e.g., "a support object covers a true requirement when its non-limit constraints are unrestricted" or "limit-only objects count as true for matching"), mirror it in schema + get_products.mdx + spec, and add one scenario assertion.

3. warning — the new product_id opacity rule collides with the ratified wholesale mirror model

product.json now says buyers "MUST NOT assume cross-context stability or use it as permanent catalog identity" — for all products. The untouched specs/wholesale-feed-webhooks.md is built on the opposite assumption: webhook payloads carry product_id for consumers to "update local wholesale product-feed… mirrors without immediately re-reading the full feed" — a webhook is not a discovery context, so mirror correlation requires cross-context identity. This PR even strengthens the storefront pattern (if_pricing_version: "storefronts that re-price compositions far more often than they re-render product mirrors"). A conformant seller may re-mint wholesale IDs per response and break every mirror; a conformant buyer obeying the MUST NOT cannot implement the mirror spec. Ask: scope the opacity rule to is_custom: true products, or define that non-custom products keep stable IDs within a cache_scope while wholesale_feed_version tracks them.

4. warning — exact-equality placement sets at create_media_buy without prior discovery acceptance: valid or not?

placement-selection.json / placement.json: a selected set "exactly equal to the product's complete included placement set is satisfied inherently and may be echoed on the package." But fixed products MUST NOT declare overlay_support.placement_selection, and package-request.json requires overlay fields to be "declared in the product's overlay_support unless they were already accepted during discovery." Seller A accepts the exact-equality set at create (inherent match); seller B rejects with UNSUPPORTED_FEATURE — both citing this PR. Ask: one sentence settling whether inherent matching applies only during get_products or also at create/update, plus the matching scenario probe.

5. warning — PRODUCT_EXPIRED is a normative rejection path with zero compliance coverage

product.json expires_at: "After this time, create_media_buy rejects the product with PRODUCT_EXPIRED." No storyboard probes it (grep -rn PRODUCT_EXPIRED static/compliance/source/ is empty), and the boundary with PRODUCT_NOT_FOUND implicitly requires expiry tombstones — a garbage-collecting seller conformantly returns PRODUCT_NOT_FOUND for the same expired ID. Ask: add a probe (seed short-expires_at, attempt create after expiry, assert code), and state the tombstone expectation explicitly (SHOULD return PRODUCT_EXPIRED while the ID is still recognizable; PRODUCT_NOT_FOUND acceptable once unresolvable).

6. warning — remove_values supports /device_platform but not /device_platform_exclude

targeting-modification.json enumerates /device_platform, /device_type, /device_type_exclude — but not /device_platform_exclude, the string-set field this same PR adds. Sellers must fall back to whole-array replace for exclusion relief, and extending the enum post-ratification is a version event. Ask: add the path now, or state in the enum description why it's excluded.

7. warning — stale demographic_targeting_resolution reference in package.json

package.json (audience_evidence_selections description): "applied targeting remains exclusively in targeting_overlay and demographic_targeting_resolution" — a normative pointer to the field this PR renames to targeting_resolution.demographics. Only occurrence in the tree; cheap fix, and it's the copy implementations will cite back.

8. note — declare the certification-gate change in the PR body

version-negotiation.yaml / error-compliance.yaml promote release-precision advertisement/echo from advisory to required for 3.2 certification. This executes main's pre-recorded plan and is load-bearing for the compatibility window — but it's a certification-gate change adopters should see stated in the description.


On the community-feedback questions, briefly: (1) one response-level brief_targeting is the right scope; consider MUST (not SHOULD) when inferred hard targeting excluded products or changed pricing — that's when money moves on an interpretation the buyer never saw. (2) Selection-as-acceptance is adequate given distinguishable IDs, mandatory targeting_resolution projection, and expires_at; a digest ack would be theater while the digest is non-canonical — instead state explicitly that selection is binding acceptance and the review burden is the buyer's. (3) required_overlay_support correctly promises independent selection, but finding 2 is where that promise actually lives — fix the algebra first. (4) Fixed-vs-selectable is operationally sufficient once finding 4 is settled; the atomic placement/creative-assignment validation is the strongest part of the design. (5) is_custom is the right marker; finding 3 is the boundary that needs drawing around it. (6) Independent include/exclude is the right model and worth stating as the general rule for future typed dimensions — but carry it into the modification vocabulary too (finding 6).

@bokelley

Copy link
Copy Markdown
Contributor Author

The eight findings are substantive; addressing them in order.

Decisions needed (1–4):

1 — Create-time pricing disposition under required_overlay_support. The gap is real. Options (a)–(c) each close it. The most consistent with the documented tradeoff — required_overlay_support explicitly exchanges pricing granularity for discovery flexibility — is (c): forecast staleness at book-time is carried by the buyer, and buyers needing bound quotes must re-discover with concrete targeting_overlay values. This aligns with how targeting_overlay already works at discovery: using it is described as the way to get pricing that actually reflects delivery intent. One sentence in the error-code description and/or get_products.mdx settles it. @bokelley — confirm (c) or choose differently.

2 — boolean-requirement × object-support matching gap. The two existing clauses leave the case unhandled: requirement: true, support: {max_values_per_package: 3}. A third clause is needed. The reviewer proposes "limit-only objects count as true for matching" or "object covers true when non-limit constraints are unrestricted." Both phrasings give the same answer for placement_selection, but they may diverge for objects that mix limits and capability constraints (e.g., geo_metros: {systems: ["dma"]}). @bokelley — which phrasing is correct, and does it apply to {systems: [...]} support too? Once decided: schema annotation update + matching sentence in get_products.mdx + one scenario assertion. Finding 3 in the community-feedback question (#3 in the PR body) depends on this algebra being correct first.

3 — product_id opacity vs. wholesale mirror model. The MUST NOT assume cross-context stability is logically scoped to is_custom: true products — configured offers priced at a specific discovery context — but the current language applies to all products. The wholesale feed model requires cross-context stable IDs for mirror consumers maintaining incremental state, and wholesale_feed_version / if_pricing_version are specifically built on that assumption. Scoping the MUST NOT to is_custom: true (or defining that non-custom products keep stable IDs within cache_scope while wholesale_feed_version tracks them) resolves the conflict. @bokelley — confirm scope and preferred formulation.

4 — Inherent placement matching at create/update. Whether inherent satisfaction applies throughout the lifecycle or only during get_products needs one sentence. If the intent is that a buyer accepting a configured product during discovery has already accepted its fixed placement set (the seller indicated availability by returning the product), then say so — inherent satisfaction is a property of the product, not the discovery step. If create-time inherent matching is not intended, buyers receive no valid path to book a fixed-placement product with a placement field, since those products MUST NOT declare overlay_support.placement_selection. @bokelley — one sentence either way settles the interop question.

Addressable fixups (5–7):

5 — PRODUCT_EXPIRED compliance coverage. The normative rejection path has no storyboard probe. Add: seed a short expires_at, attempt create after expiry, assert PRODUCT_EXPIRED. Also state the tombstone expectation explicitly — SHOULD return PRODUCT_EXPIRED while the ID is still resolvable; PRODUCT_NOT_FOUND acceptable once garbage-collected. Without this, the boundary between the two error codes is implementation-defined.

6 — /device_platform_exclude missing from targeting-modification.json enum. remove_values lists /device_platform but not /device_platform_exclude, which this PR introduces as a first-class field. Adding the path now avoids making it a post-ratification version event; the symmetry is clearly the design intent.

7 — Stale demographic_targeting_resolution reference. package.json's audience_evidence_selections description still names the old field. One-line fix: demographic_targeting_resolutiontargeting_resolution.demographics.

Note 8 — Certification-gate disclosure. The version-negotiation.yaml / error-compliance.yaml promotion from advisory to required for 3.2 certification is load-bearing for adopters' compatibility windows. Worth a sentence in the PR body — reviewers scanning the description to understand their compliance surface need to see it stated explicitly.


@bokelley — findings 1–4 are the decision-shaped ones; 5–7 are clean fixups for the next commit once the merge conflict is resolved; note 8 is a PR body addition.


Generated by Claude Code

…ilters

# Conflicts:
#	docs/building/by-layer/L3/error-handling.mdx
#	docs/media-buy/task-reference/create_media_buy.mdx
#	docs/media-buy/task-reference/get_products.mdx
#	docs/media-buy/task-reference/update_media_buy.mdx
#	package.json
@bokelley
bokelley changed the base branch from main to security-wave-12-get-products-idempotency August 12, 2026 04:10
…empotency' into verify-get-products-filters

# Conflicts:
#	static/schemas/source/media-buy/request-proposals-response.json

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated-path hard approval gate not satisfied.

This PR touches files under static/schemas/source/** and .github/workflows/**, both declared under the repo's hard, non-overridable approval gate (gated_paths: true). The current review_decision is unknown/none — NOT APPROVED. Decision-table row 2 fires: gated paths + no human/CODEOWNERS approval = escalate.

This is a subsequent pass. The reviewer confirmed the 77 substance files are disciplined and internally consistent, with no Critical/High/Medium findings: the demographic_targeting_resolution → targeting_resolution.demographics rename is coherent across schemas/docs/tests, both new oneOf schemas carry explicit discriminators (no undiscriminated regression), the minor changeset is correct for pre-GA 3.2 dev-cycle surface, and product.json / positive-postal-area-support gates are correct and test-covered. The prior escalate persists solely because the gated-path condition remains unmet — not due to any code defect.

Additionally, protocol schema-source edits fall under the repo's spec-ownership / breaking-class governance: ratification is a human act and cannot be auto-approved until review_decision is APPROVED. Once a human/CODEOWNERS approval is recorded, this clean diff can fall through to approval.

No blocking findings; escalation is purely gate-driven.

Escalation reasons

  • Touches gated paths under static/schemas/source/** (30+ files) and .github/workflows/check-schema-links.yml while review_decision is not APPROVED — human/CODEOWNERS approval required (decision-table row 2).
  • Protocol schema-source edits fall under spec-ownership/breaking-class governance; ratification is a human act and cannot be auto-approved until review_decision is APPROVED.

Why human review

  • Touches gated paths under static/schemas/source/** (30+ files added/modified) and .github/workflows/check-schema-links.yml while review_decision is not APPROVED — human/CODEOWNERS approval required (decision-table row 2).
  • Protocol schema-source edits fall under the repo's spec-ownership / breaking-class governance; ratification is a human act and cannot be auto-approved until review_decision is APPROVED.
  • This PR touches a path under a hard, non-overridable approval gate (.github/workflows/check-schema-links.yml (modified) matches .github/workflows/**; static/schemas/source/core/creative-assignment.json (modified) matches static/schemas/source/**; static/schemas/source/core/package-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/package.json (modified) matches static/schemas/source/**; static/schemas/source/core/placement-selection.json (added) matches static/schemas/source/**; static/schemas/source/core/placement.json (modified) matches static/schemas/source/**; static/schemas/source/core/positive-postal-area-support.json (added) matches static/schemas/source/**; static/schemas/source/core/pricing-option.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-allocation.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-filters.json (modified) matches static/schemas/source/**; static/schemas/source/core/product-targeting-resolution.json (added) matches static/schemas/source/**; static/schemas/source/core/product.json (modified) matches static/schemas/source/**; static/schemas/source/core/proposal.json (modified) matches static/schemas/source/**; static/schemas/source/core/targeting-modification.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-requirements.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting-overlay-support.json (added) matches static/schemas/source/**; static/schemas/source/core/targeting.json (modified) matches static/schemas/source/**; stat

…(truncated)…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update_media_buy: add first-class purchased-placement selection to PackageUpdate

2 participants