Skip to content

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

Open
bokelley wants to merge 8 commits into
verify-get-products-filtersfrom
review-issue-5588
Open

feat(media-buy): add identifier-based place targeting#6093
bokelley wants to merge 8 commits into
verify-get-products-filtersfrom
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

@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

@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.

This PR touches multiple files under static/schemas/source/**, which is a hard, deterministic approval gate (gated_paths: true). The current review_decision is not APPROVED (unknown/none), so the gate is NOT satisfied and per decision-table row 2 the outcome must be escalate. This is also a protocol wire-shape change (new geo-place schemas, new enum, targeting/product-filters/error-code modifications) — a Breaking/Normative-class change whose ratification is a human act.

The prior Medium finding (compliance overlap probe never exercising the cross-version include/exclude case) has been resolved: the delta adds the reject_cross_version_place_overlap probe exercising the same-place include-under-v0 / exclude-under-v1 → INVALID_REQUEST case, with matching context_outputs and requires_capability gating. No new findings in the current run — this is a clean net improvement.

However, the diff continues to touch gated schema paths and no human/CODEOWNERS approval is recorded, so the escalation persists on the deterministic gate alone (not as a "still has issues" judgment). A human/CODEOWNERS review is required before merge.

Escalation reasons

  • Gated paths 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 not APPROVED — human/CODEOWNERS approval required.
  • Breaking/Normative-class protocol wire-shape change — ratification is a human act.

Why human review

  • Gated paths: PR modifies protocol schema files under static/schemas/source/** and review_decision is not APPROVED — human/CODEOWNERS approval required before merge.
  • Breaking/Normative-class protocol wire-shape change (new geo-place schemas, new geo-targeting-level enum, targeting/product-filters/error-code changes) — ratification is a human act per the constitution hard gate.
  • 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 'unknown', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@lukasz-pubx

Copy link
Copy Markdown

Strong support for the motivation here — structured {country, system, place_type, values} is unambiguously better than "Los Angeles" in brief prose, and the country-keyed countries → [place_type] map is the right call for avoiding Cartesian over-declaration. Three things I'd like to raise before this lands, one of which I think is worth blocking on.

1. place is the first geo dimension where target granularity exceeds forecast granularity

AdCP has one geo vocabulary for forecasting and reporting — geo-level.json — and it drives all three reporting surfaces:

  • forecast-dimension-geo.json (forecast breakdown rows on get_products)
  • geo-delivery-metrics.json (delivery reporting)
  • get-media-buy-delivery-request.json (delivery queries)

This PR doesn't extend that enum. It adds a second, wider one, and says so explicitly in geo-targeting-level.json:

Geographic levels available for product discovery and execution targeting. Reporting uses the narrower geo-level vocabulary until place reporting is standardized.

and in targeting.mdx:

Place forecast and delivery breakdown rows are intentionally not part of this release: geo_level: "place" remains invalid on reporting surfaces.

For metro and postal_area, a buyer can request a forecast at exactly the granularity they will later enforce. For place they cannot. The concrete failure:

  1. get_products with a national brief (or filters.countries: ["US"]) returns firm pricing_options and a DeliveryForecast.
  2. create_media_buy applies targeting_overlay.geo_places for Los Angeles.
  3. Realistic delivery and effective CPM are materially different from what was quoted — and the buyer had no protocol-level way to estimate the delta beforehand, because no place-level forecast row can be requested.

The new filters.places doesn't close this. It's ANY/intersection coverage matching, and the schema states "The response does not identify which value matched" — it tells the buyer a product touches Los Angeles, not what volume or price it yields there.

Nor does anything guard the create step. pricing-option.json carries no targeting dependency, create_media_buy.mdx imposes no re-forecast or re-quote obligation on targeting_overlay, and the discovery→buy guidance in targeting.mdx actively permits the narrowing: "then apply the same values (or refined versions) as overlays at buy time." The only backstop is TARGETING_TOO_NARROW, which is scoped to zero inventory — not to materially reduced inventory at an unchanged committed price.

To be clear, this hole partly pre-exists — the same is true of geo_metros today. But place is the first dimension where the buyer can't even measure the exposure, which is what changes the risk profile.

Ask: either gate geo_places behind the follow-up reporting RFC so target and forecast granularity stay aligned, or add a normative rule on the discovery→create granularity delta — the seller MUST reject or re-quote when an overlay narrows geography materially below what the returned forecast and pricing assumed. If the intent is instead that firm pricing is unconditional on overlay granularity and the risk sits with the buyer, that's a defensible position, but it should be stated rather than left undefined. As written, two conformant sellers can behave very differently here.

2. Undefined failure disposition for places / required_geo_targeting

The capability schema this PR adds says:

Sellers MUST reject unsupported systems, country/type pairs, versions, and identifiers rather than silently dropping them.

That sentence sits in get-adcp-capabilities-response.json describing the geo_places capability generally, not scoped to the overlay. But get_products runs on the opposite convention — required_metrics: "Sellers MUST silently exclude products that cannot meet the list — filter-not-fail; do not return an error."

Nothing in the diff says which applies when a buyer sends filters.places with an unsupported system, or a system_version outside supported_versions. One seller returns INVALID_REQUEST; another returns zero products. Related: places accepts system_version (it shares the geo-place-area.json $ref with the overlay) but its meaning at discovery time is never defined — is coverage evaluated as of that catalog version, or is it ignored? In the overlay system_version is normatively load-bearing; in the filter it's undocumented.

Ask: one sentence in the places description resolving the disposition, and one on what system_version means at discovery.

3. No brief-mode or coverage-filter compliance coverage

The storyboard's only get_products step, get_place_targetable_product, runs buying_mode: "wholesale" and exercises only required_geo_targeting. So filters.places — a normative surface with specified ANY/intersection semantics — has no agent-observable probe in any mode, and place discovery has none in brief mode at all. Schema-level coverage exists in schema-validation.test.cjs, but nothing verifies seller behaviour.

Ask: a brief-mode step exercising filters.places, asserting intersection semantics.


Points 2 and 3 are small. Point 1 is the one I'd want resolved before merge — happy to be told the pricing risk is intentionally the buyer's, but I don't think it should be silent.

@bokelley

bokelley commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

All three concerns are well-founded. Here's the synthesis.

1. Forecast granularity gap — clarify intent, don't gate

The asymmetry is real, but it follows the existing overlay model. geo_metros and geo_postal_areas overlays already let buyers narrow national-forecast products without a revised quote, and AdCP doesn't require sellers to provide sub-national breakdown rows in DeliveryForecast. Place is more granular than metro and has no hierarchical geo-code proxy, so the exposure is less visible — but structurally it's the same design. geo_level: "place" is intentionally absent from geo-level.json (there's even a schema test asserting it's INVALID on reporting surfaces).

Option (c) stated explicitly is the right call — not gating, which would couple independent problems and delay adoption with no benefit. One sentence in targeting.mdx:

"Product-level forecast and pricing_options returned from get_products are not revised to reflect targeting_overlay.geo_places. Firm pricing is unconditional on overlay granularity, following the same precedent as geo_metros and geo_postal_areas. Delivery risk for overlay-narrowed execution sits with the buyer."

Side issue surfaced: TARGETING_TOO_NARROW appears in create_media_buy.mdx but is absent from error-code.json. Pre-existing drift, but this PR makes it visible — worth reconciling before merge (add to enum with recovery class, or remove the doc table row).

2. Filter-not-fail vs. MUST-reject — fix before merge

This is a real interop gap. The MUST-reject sentence in get-adcp-capabilities-response.json was written for execution time (targeting_overlay.geo_places on create_media_buy) but carries no scope annotation. A seller reading it literally would return INVALID_REQUEST on a get_products filters.places call with an unsupported system; another seller would correctly apply filter-not-fail and return zero products. Both are conformant under the current text, and buyer agents can't distinguish a bug from a design choice.

Two sentences needed in the filters.places field description:

"An unrecognized or unsupported system in filters.places MUST be treated as no-match (filter-not-fail, consistent with required_metrics); sellers MUST NOT return an error. The MUST-reject rule in get_adcp_capabilities applies to targeting_overlay.geo_places on spend-committing operations only."

Plus one sentence on system_version at discovery time: "Sellers evaluate coverage against their current catalog; system_version in filters.places is informational and MUST NOT be required or cause an error when omitted."

3. Storyboard brief-mode gap — fix before merge

Confirmed: filters.places has no storyboard coverage in any mode. The compliance harness cannot observe whether a seller honors ANY/intersection semantics — a seller returning zero products for every filters.places request would pass. One brief-mode get_products step is sufficient: assert a product covering at least one requested place is returned, and a product covering none of the requested places is absent.


@bokelley — actionable pre-merge items are (2) and (3). The normative statement in (1) is a clarity add on a deferral targeting.mdx already partially documents; none of these require schema changes. The TARGETING_TOO_NARROW reconciliation is separate but worth bundling.


Generated by Claude Code

bokelley pushed a commit that referenced this pull request Aug 4, 2026
- Clarify pricing/forecast granularity in targeting.mdx: firm pricing is
  unconditional on overlay granularity for geo_places, consistent with
  geo_metros and geo_postal_areas precedent
- Add TARGETING_TOO_NARROW to error-code.json enum with description and
  enumMetadata (correctable recovery) — reconciles drift with
  create_media_buy.mdx
- Add filter-not-fail rules to filters.places description in
  product-filters.json: unsupported system MUST yield no-match (not error);
  MUST-reject applies only to targeting_overlay.geo_places on spend-committing
  operations; system_version is informational at discovery time
- Add brief-mode filters.places storyboard phase in geo_place_targeting.yaml:
  positive probe (covered place returns product) and negative probe
  (uncovered place returns empty — not error) to verify ANY/intersection
  semantics are observable by the compliance harness
@bokelley

bokelley commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Applied all four items from the pre-merge review in commit 3c78787:

1. targeting.mdx — pricing/forecast clarity
Added one sentence after the place-forecast-deferral paragraph: firm pricing is unconditional on overlay granularity for geo_places, consistent with geo_metros/geo_postal_areas precedent. Delivery risk sits with the buyer.

2. product-filters.json — filter-not-fail rules (MUST fix)
Extended the filters.places description with three rules:

  • Unrecognised or unsupported system MUST be treated as no-match (filter-not-fail, consistent with required_metrics); sellers MUST NOT return an error.
  • The MUST-reject rule applies to targeting_overlay.geo_places on spend-committing operations only — not to discovery-time filtering.
  • system_version is informational; MUST NOT be required and MUST NOT cause an error when omitted.

3. geo_place_targeting.yaml — brief-mode storyboard (MUST fix)
Added brief_mode_place_filter_coverage phase with two steps:

  • get_products_brief_covered_place: Amsterdam (GeoNames 2759794, NL) — expects ≥1 product (positive/ANY-intersection probe).
  • get_products_brief_uncovered_place: Berlin (GeoNames 2950158, DE, outside seller coverage) — expects empty products[], no error (negative/filter-not-fail probe).

4. error-code.jsonTARGETING_TOO_NARROW (side fix)
Added at index 41 (immediately after PLACE_TARGET_UNAVAILABLE), with enumDescriptions entry and enumMetadata recovery: correctable. Reconciles the drift between the docs reference in create_media_buy.mdx and the enum registry.


Generated by Claude Code

@aao-secretariat

Copy link
Copy Markdown
Contributor

Ladon cannot review this PR until merge conflicts are resolved.

bokelley and others added 6 commits August 5, 2026 08:04
- Clarify pricing/forecast granularity in targeting.mdx: firm pricing is
  unconditional on overlay granularity for geo_places, consistent with
  geo_metros and geo_postal_areas precedent
- Add TARGETING_TOO_NARROW to error-code.json enum with description and
  enumMetadata (correctable recovery) — reconciles drift with
  create_media_buy.mdx
- Add filter-not-fail rules to filters.places description in
  product-filters.json: unsupported system MUST yield no-match (not error);
  MUST-reject applies only to targeting_overlay.geo_places on spend-committing
  operations; system_version is informational at discovery time
- Add brief-mode filters.places storyboard phase in geo_place_targeting.yaml:
  positive probe (covered place returns product) and negative probe
  (uncovered place returns empty — not error) to verify ANY/intersection
  semantics are observable by the compliance harness
@bokelley
bokelley force-pushed the review-issue-5588 branch from 0d0bc72 to 295f578 Compare August 5, 2026 06:52
@bokelley
bokelley changed the base branch from main to verify-get-products-filters August 5, 2026 06:52
# Conflicts:
#	docs/media-buy/task-reference/create_media_buy.mdx
#	docs/media-buy/task-reference/get_products.mdx
#	docs/protocol/get_adcp_capabilities.mdx
#	package.json
#	skills/adcp-media-buy/SKILL.md
#	static/schemas/source/core/creative-assignment.json
#	static/schemas/source/core/product.json
#	static/schemas/source/media-buy/get-products-request.json
…o exec/pr-6093

# Conflicts:
#	docs/media-buy/task-reference/create_media_buy.mdx
#	docs/media-buy/task-reference/get_products.mdx
#	skills/adcp-media-buy/SKILL.md
#	static/schemas/source/core/creative-assignment.json
#	static/schemas/source/media-buy/get-products-request.json
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

3 participants