Skip to content

feat(media-buy): add identifier-based place targeting - #6093

Open
bokelley wants to merge 4 commits into
mainfrom
review-issue-5588
Open

feat(media-buy): add identifier-based place targeting#6093
bokelley wants to merge 4 commits into
mainfrom
review-issue-5588

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

What changed

  • Add geo_places and geo_places_exclude targeting using stable IDs, explicit countries, identifier systems, catalog versions, and place types.
  • Add exact country/type capability discovery, collision-safe registry extensions, version lifecycle metadata, and a standard name/ID resolver contract.
  • Add get_products place coverage and capability filters so buyers can discover support before creating a buy.
  • Require persisted package-state echo with the applied catalog version and define PLACE_TARGET_UNAVAILABLE for correctable catalog-rollover failures.
  • Add compliance coverage for discovery, create/update/readback, exclusion-only targeting, unknown IDs, mismatched labels, and include/exclude conflicts.
  • Document execution semantics and explicitly defer place-level forecasting and delivery breakdowns to a follow-up.

Why

Raw city names are ambiguous, localized, and unstable across platforms. Buyers need a portable way to express named-place intent without silently targeting the wrong Bergen, Springfield, municipality, borough, or neighborhood.

The capability shape uses a country-keyed type map to avoid Cartesian false positives. Registered numeric systems have exact semantics, while owner-controlled HTTPS URIs provide collision-safe extensions for private catalogs and types.

Review

Independent product, protocol-interoperability, and JSON Schema experts reviewed the implementation. Their findings on resolver disambiguation, cross-version overlap, exclusion-only behavior, catalog rollover, deterministic compliance, and error recovery were addressed; all three report no remaining blockers.

Validation

  • Schema validation: 21/21
  • Composed schema validation: 118/118
  • Targeting overlay vectors: 30/30
  • Example validation: 55/55
  • Documentation JSON validation: 285/285
  • General unit suite: 1,025/1,025
  • C2PA targeted server tests: 14/14
  • Schema/compliance builds and storyboard lints
  • Current storyboard matrix: all six tenants above floors
  • AdCP 3.0 compatibility matrix: all six tenants above floors
  • TypeScript typecheck, error-code drift, oneOf audit, schema links, UTF-8, and diff hygiene

Closes #5588.

@bokelley bokelley changed the title [codex] Add identifier-based place targeting feat(media-buy): add identifier-based place targeting Jul 30, 2026
@bokelley
bokelley marked this pull request as ready for review July 30, 2026 10:23
values: ["2759794"]
idempotency_key: "$generate:uuid_v4#geo_place_targeting_overlap"
context:
correlation_id: "geo_place_targeting--create_with_include_exclude_overlap"

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.

Medium: The include/exclude overlap probe uses the same system_version ($context.place_system_version) on both sides, so it only exercises same-version overlap. The spec makes the cross-version case the load-bearing rule — targeting.mdx: "Sellers MUST reject the same (country, system, place_type, value) in both include and exclude lists, even when the include and exclude entries specify different catalog versions: version is not part of stable place identity." A seller that (wrongly) keys place identity on version passes this scenario yet ships the exact ambiguity the rule forbids. The unit suite covers cross-version disjointness (schema-validation.test.cjs ~L1507), but that is not the seller-conformance surface. Add a variant that differs only in system_version and still requires INVALID_REQUEST.

@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 paths require human/CODEOWNERS approval and review is not yet APPROVED.

Verdict: This PR touches the published protocol surface 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, this must escalate.

The change itself looks sound: an additive minor introducing identifier-based place targeting (new geo_places/geo_places_exclude overlays, capability declaration, get_products filters, a resolver contract, PLACE_TARGET_UNAVAILABLE, and compliance coverage). Schema↔docs coherence holds across targeting/get_products/capabilities/error-code surfaces, no undiscriminated oneOf, dist/** untouched, changeset is a correct minor for a non-breaking addition, and draft-07-inexpressible constraints are documented via x-adcp-validation and enforced in tests. No critical/high findings.

Medium findings (1, non-blocking on their own):

  • static/compliance/source/protocols/media-buy/scenarios/geo_place_targeting.yaml:567 — Compliance overlap probe only exercises same-version include/exclude, never the cross-version case the spec singles out.

Escalation reason: Human/CODEOWNERS approval is required before merge because the PR modifies gated protocol-schema files under static/schemas/source/** and review_decision is not APPROVED.

Medium findings

  • static/compliance/source/protocols/media-buy/scenarios/geo_place_targeting.yaml:567 — Compliance overlap probe never exercises the cross-version include/exclude case the spec singles out

Why human review

  • Gated paths: PR modifies protocol schema files under static/schemas/source/** (added: geo-place-*.json, geo-targeting-level.json; modified: product-filters.json, targeting.json, error-code.json, index.json, get-media-buys-response.json, get-adcp-capabilities-response.json) and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required before merge.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/geo-place-area.json (added) matches static/schemas/source/**; static/schemas/source/core/geo-place-catalog-capability.json (added) matches static/schemas/source/**; static/schemas/source/core/geo-place-catalog-entry.json (added) matches static/schemas/source/**; static/schemas/source/core/geo-place-resolver.json (added) matches static/schemas/source/**; static/schemas/source/core/geo-place-support.json (added) matches static/schemas/source/**; static/schemas/source/core/geo-place-system.json (added) matches static/schemas/source/**; static/schemas/source/core/geo-place-type.json (added) matches static/schemas/source/**; static/schemas/source/core/get-geo-place-resolution-request.json (added) matches static/schemas/source/**; static/schemas/source/core/get-geo-place-resolution-response.json (added) matches static/schemas/source/**; static/schemas/source/core/product-filters.json (modified) matches static/schemas/source/**; static/schemas/source/core/targeting.json (modified) matches static/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matches static/schemas/source/**; static/schemas/source/enums/geo-targeting-level.json (added) 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/protocol/get-adcp-capabilities-response.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Jul 30, 2026
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.

RFC: add identifier-based city/place geo targeting

1 participant