Skip to content

[codex] Add product-scoped signal targeting groups#5009

Merged
bokelley merged 5 commits into
mainfrom
bokelley/signal-targeting-overlays
May 25, 2026
Merged

[codex] Add product-scoped signal targeting groups#5009
bokelley merged 5 commits into
mainfrom
bokelley/signal-targeting-overlays

Conversation

@bokelley

@bokelley bokelley commented May 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds product-scoped signal metadata and signal targeting for media buys. Products now have two distinct surfaces:

  • included_signals: non-selectable signals already included in, bundled with, or planned into the product.
  • signal_targeting_allowed, optional inline signal_targeting_options, and signal_targeting_rules: package-level signal selection controls.

Packages select or echo eligible targetable signals through targeting_overlay.signal_targeting_groups with top-level operator: "all" and child any / none groups.

Wholesale signal discovery stays on get_signals instead of adding another product field:

  • get_products(wholesale) can return signal_targeting_allowed: true without inline signal_targeting_options; buyers then call get_signals for the selectable signal feed.
  • get_products(brief) and get_products(refine) can inline relevant or product-specific signal_targeting_options when the seller wants to provide pricing, activation handles, defaults, grouping hints, or a curated subset.
  • signal_targeting_rules remains product-scoped because one seller can route products through different ad servers or planning layers.

SignalRef is the canonical named-signal identity surface across discovery, targeting, audience selectors, and wholesale events:

  • scope: "data_provider" resolves through a catalog-published adagents.json signal using data_provider_domain + signal_id.
  • scope: "signal_source" resolves a source-native signal that is not catalog-published, using signal_source_url + signal_id.
  • scope: "product" resolves only in the selected product/package context.

Product signal listings share one signal-ref-plus-definition shape. Product-local refs require inline name and value_type; data-provider and signal-source refs can be reference-only because the authoritative definition lives at the referenced catalog or source.

Legacy SignalId / signal_id.source and get_signals.signal_ids remain as deprecated compatibility fields. adagents.json signals[].id and authorization signal_ids are still local catalog ids, not the deprecated SignalId object.

Product / Protocol Feedback Addressed

  • Adds included_signals as the non-selectable analogue to covered properties/placements: descriptive product metadata, not buyer-selected targeting.
  • Keeps the wholesale assembly model simple: cache products and get_signals, then compose package signal groups without duplicating the full signal menu inline.
  • Keeps targeting capability and rules on each product rather than get_adcp_capabilities, since one seller can route products through different platforms.
  • Enforces groups-only buy-time targeting by rejecting legacy flat targeting_overlay.signal_targeting.
  • Adds schema invariants: inline options or rules require signal_targeting_allowed: true; wholesale products may declare rules without inline options and rely on get_signals for the selectable feed.
  • Relaxes product signal listings so data-provider refs do not have to redefine name or value_type; product-local refs still must define them inline.
  • Tightens SignalRef branch hygiene so refs reject incompatible namespace carry-over like data_provider_domain, signal_source_url, agent_url, source, and legacy id where they do not apply.
  • Updates get_signals response to require signal_ref; request lookup now uses signal_refs, with legacy signal_ids deprecated.
  • Updates audience selectors, signal targeting constraints, and wholesale feed events to use signal_ref instead of the deprecated signal_id object.
  • Documents selection_group_rules: each child package group must contain signals from exactly one selection_group and one targeting mode; sellers reject mixed/collapsed groups.
  • Keeps non-guaranteed wholesale products in the normal direct-targeting path: selected signals are targeting predicates discovered through get_signals, not seller-planned audience products.
  • Cleans stale docs language around data-provider signals, first-party audiences, signal_agent_segment_id, and agent/source naming.

Validation

  • npm run test:schemas
  • npm run test:composed
  • npm run test:json-schema
  • npm run test:docs-nav
  • npm run test:examples
  • npm run test:targeting-overlay-vectors
  • git diff --check
  • precommit hook: test:unit, test:test-dynamic-imports, test:callapi-state-change, typecheck
  • push hook: version sync, docs schema-link convention, Mintlify broken-links check

@bokelley bokelley force-pushed the bokelley/signal-targeting-overlays branch 8 times, most recently from 204ec7b to 54c07c4 Compare May 25, 2026 02:39
@bokelley bokelley marked this pull request as ready for review May 25, 2026 02:48
aao-release-bot[bot]
aao-release-bot Bot previously approved these changes May 25, 2026

@aao-release-bot aao-release-bot 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.

Approving. SignalRef as the canonical named-signal identity is the right shape, the product-scoped Boolean composition baseline is portable, and the new schema invariants (options/rules → signal_targeting_allowed: true; rules require options) cover the fixed/default-only echo case.

Things I checked

  • SignalRef scope hygiene. All three branches in static/schemas/source/core/signal-ref.json enforce cross-scope rejection via not.anyOf against the legacy source/id keys plus the other scopes' anchors. The 12 carry-over rejection cases in tests/composed-schema-validation.test.cjs:631-685 exercise every branch.
  • Product invariants. The two allOf clauses in static/schemas/source/core/product.json:535-561 are coherent. Fixed/default-only products still set signal_targeting_allowed: true per the field description (product.json:168-172) and the tests at composed-schema-validation.test.cjs:715-750 lock the gate both directions.
  • Legacy flat signal_targeting rejection in static/schemas/source/core/targeting.json:444-447 is a defensive guard, not a regression — git show fe6d7e658:static/schemas/source/core/targeting.json has zero signal_targeting references pre-PR.
  • Required-list swap on get_signals.signals[]. signal_idsignal_ref, with data_provider and pricing_options relaxed out. Consistent with the beta-cycle rename pattern from the brand-protocol status field swaps in 3.1.0-beta.3 (CHANGELOG.md:85), but see follow-up below.
  • Test vector. New signal-targeting-groups-echo case in static/test-vectors/media-buy/package-status-targeting-overlay-echo.json:127-225 exercises both any and none child groups with pricing_option_id and signal_agent_segment_id echo.
  • Docs↔schema coherence. docs/media-buy/advanced-topics/targeting.mdx:23-535, docs/media-buy/task-reference/create_media_buy.mdx:535-617, and docs/signals/tasks/get_signals.mdx use the new signal_ref shape consistently. No drift.
  • Two expert verdicts: code-reviewer Approve-with-caveats; ad-tech-protocol-expert sound-with-caveats. Neither flagged a blocker.

Follow-ups (non-blocking — file as issues)

  1. Numeric expression min_value <= max_value is prose-only. static/schemas/source/core/signal-targeting-expression.json:59,79 claims the constraint as MUST but no structural if/then. Same gap pre-existed in signal-targeting.json:58-79. Either tighten or downgrade to SHOULD.
  2. scope: "product" is reachable from get_signals responses with no product context. signals/get-signals-response.json:27-30 permits the full signal-ref.json oneOf including scope: "product", but get-signals-request.json has no product_id scoping field — a seller can emit product-local refs the buyer cannot resolve. Either narrow the response's signal_ref oneOf to non-product scopes, or add request-side product scoping with an if/then.
  3. signal_ref declared but not required on signal.* events in static/schemas/source/core/wholesale-feed-event.json:250-251,289-291,334-335,391-393. The PR description says events "now carry signal_ref" but a producer can omit it and stay valid. Tighten the required list on signal.created/signal.updated, or document that signal_agent_segment_id remains the canonical key and signal_ref is best-effort.
  4. Test gaps for the new structural constraints: product-signal-targeting-option.json's requires_activation → signal_agent_segment_id if/then; targeting.json's not.required: [signal_targeting] legacy-shape rejection; signal-targeting-expression.json's numeric anyOf [min_value | max_value]. The carry-over hygiene coverage is strong; these three new invariants deserve symmetric assertions.
  5. signal_source vs data_provider tie-break is correctly stated in signal-ref.json:71 ("use data_provider even if the catalog publisher is also the seller or signal source") but is unenforceable on the wire. Worth a buyer-side reconciliation note — try both before failing.
  6. Negative test vector for selection_group_rules (signal-targeting-rules.json:43-85) — the "one selection_group + one targeting_mode per child group" rule is runtime-only. The current vector covers the happy path; a mixed-group rejection vector would lock implementers in.
  7. Changeset transparency. The required[] swap on get_signals.signals[] is a more aggressive move than the additive-deprecate Option E used for media_buy_status in beta.3 (CHANGELOG.md:85). Consistent with beta-cycle precedent, but worth one sentence in the changeset body acknowledging the divergence so beta-line integrators aren't surprised.

Minor nits (non-blocking)

  1. not.anyOf carry-over lists in signal-ref.json:27-35, 59-66, 90-97 are maintenance-coupled. When a fourth scope arrives, every existing branch needs its not.anyOf extended. Worth a one-line comment on the file or a lint check.

Safe to merge.

@geneticallymodifiedfoodforthought

Copy link
Copy Markdown

Strong direction. The product-scoped signal targeting model maps directly onto a real case we're shipping at iHeartMedia.For linear broadcast radio, the seller agent is structurally better positioned to bundle inventory + audience than the buyer is to compose them separately — not because portable third-party segments can't apply (they can, and AudioGraph supports this), but because the planning complexity lives on the sell side. Broadcast inventory is scheduled across time: dayparts, weekly rotations, avail windows. The buyer isn't equipped to reason about which audience will be available when, what reach looks like across a weekly schedule, or how current avails map to targeting goals. That planning expertise belongs to the seller. The audience selection is product-scoped not because the signal itself is non-portable, but because the intersection of that signal with available inventory across time is something only the seller can resolve.AudioGraph (launching at Cannes Lions 2026) is exactly this shape: matched listening signals across broadcast, streaming, and podcast, resolved into audience profiles and planned against iHeartMedia's broadcast schedule. A buyer agent could bring a portable third-party segment — but without the seller's planning layer, it can't know what that segment looks like against this week's avails, across this daypart mix, at this reach target. scope: "product" is the right primitive. The question is whether it carries enough semantics to express this.Three questions on the current design:

  1. Temporal planning as a product-scope dimension. The current spec describes scope: "product" as "seller-local exact matching, not portable semantic discovery." That framing covers namespace isolation, but doesn't capture the case where the signal is portable but the planning resolution isn't. Is there a mechanism — or should there be — for a seller to signal that product-scoped targeting involves temporal planning that the buyer shouldn't attempt to decompose? This feels like a meaningful semantic distinction worth surfacing explicitly.

  2. Combo packaging. When inventory + audience planning are inseparable (as in broadcast), is there a pattern for expressing "this product requires an audience selection" — i.e., audience selection is mandatory, not optional overlay? Or does that live in signal_targeting_rules?

  3. Cross-product audience consistency within a seller. AudioGraph audiences will appear across multiple iHeartMedia products (broadcast, streaming, podcast). With scope: "product" being seller-local exact matching, what's the recommended pattern when the same logical audience needs to be offered consistently across a seller's product portfolio? Is that a scope: "data_provider" case where iHeartMedia publishes its own catalog, or does product-scope handle this via shared signal_targeting_options definitions?

@bokelley

Copy link
Copy Markdown
Contributor Author

Thanks for the AudioGraph context — the iHeartMedia broadcast case is exactly the shape this design was built around, and the three questions surface real edges worth addressing directly.

Q1 — Temporal planning as a product-scope dimension

You're right that the current framing of scope: "product" as "seller-local exact matching, not portable semantic discovery" covers namespace isolation but not the planning-resolution distinction you're describing. There's currently no structured field to say "this signal involves seller-managed temporal resolution the buyer shouldn't attempt to decompose."

The practical guidance for AudioGraph is: if the audience profiles are portable across products (matched listening signals that mean the same thing across broadcast, streaming, podcast), they belong in scope: "data_provider" (see Q3). The buyer selects what audience; the seller resolves which avails at booking time — the schema separates those roles by design. But if you need a structured field for "buyer-composable, seller-resolved-to-scheduled-inventory" (distinct from pure opaque product-local signals), that's a genuine gap worth a follow-up issue. description can carry the narrative today; a structured resolution_model field is the right additive fix if it matters for agent decision-making.

Q2 — Mandatory audience selection

signal_targeting_rules.selection_mode: "required" covers this directly. The seller sets minimum required selections; buyers that omit signal_targeting_groups get rejected:

// Product requires at least one audience signal
{
  "signal_targeting_rules": {
    "selection_mode": "required"
  }
}

For mixed cases (mandatory audience tier, optional exclusions), use selection_group_rules:

{
  "signal_targeting_rules": {
    "selection_mode": "optional",
    "selection_group_rules": [
      {
        "selection_group": "audience_tier",
        "selection_mode": "required",
        "min_selected_signals": 1
      }
    ]
  }
}

Child groups with operator: "none" remain optional; the audience_tier include group is enforced.

Q3 — Cross-product audience consistency

scope: "data_provider" is the right pattern. Publish the AudioGraph catalog in iHeartMedia's adagents.json; each product (broadcast, streaming, podcast) exposes those signals in signal_targeting_options using data-provider refs. The logical audience is defined once; product-scoped pricing and rules stay product-local:

// signal_targeting_options entry on each iHeartMedia product
{
  "signal_ref": {
    "scope": "data_provider",
    "data_provider_domain": "iheartmedia.com",
    "signal_id": "audiograph_commuters"
  },
  "name": "AudioGraph Commuters",
  "value_type": "binary",
  "pricing_options": [...]
}

Buyers reference the same signal_ref across products; sellers resolve it against their product-specific inventory at booking time. scope: "product" is only appropriate for signals that are meaningfully product-specific (e.g., a daypart signal with no equivalent on streaming) — using it for shared audience definitions would fragment the namespace and break cross-product consistency.

The broadcast-specific daypart-availability signal is the one case that genuinely warrants scope: "product" — it has no stable meaning outside the broadcast product's scheduling context.


Generated by Claude Code

aao-release-bot[bot]
aao-release-bot Bot previously approved these changes May 25, 2026

@aao-release-bot aao-release-bot 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.

LGTM. Follow-ups noted below. signal_ref as the canonical named-signal identity across discovery, targeting, audience selectors, and wholesale events is the right shape — one discriminator (scope) does the work that SignalId.source + four overlapping fields were doing badly.

Things I checked

  • SignalRef branch hygiene. static/schemas/source/core/signal-ref.json L27-34 / L59-65 / L90-96 — each scope branch has not.anyOf against the carry-over fields from the other branches plus the legacy SignalId fields (source, id). tests/composed-schema-validation.test.cjs L631-685 exercises six rejection paths. Clean.
  • Product allOf invariants. static/schemas/source/core/product.json L535-562 — if signal_targeting_options OR signal_targeting_rules → signal_targeting_allowed: true, and signal_targeting_rules → signal_targeting_options. Tests at L743-760 exercise both rejections. additionalProperties: true doesn't leak the bag.
  • Targeting overlay closes the legacy door. core/targeting.json L444-447 rejects the flat signal_targeting field; groups-only on the wire. The hard cutover is what makes the buy-time shape coherent.
  • value=true constant for binary in groups. signal-targeting-expression.json L24-27 hardcodes value: { const: true } while the legacy signal-targeting.json L23-26 still allows boolean. Intentional — include/exclude lives on the parent group operator, so value: false would be a second way to express exclusion. Documented at expression L5.
  • Semver. minor on 3.1-beta absorbs the breaking surfaces — flat signal_targeting rejection, required signal_ref on get-signals responses, audience-selector swap. Pre-release is the right window for this.
  • oneOf walker. All new discriminated unions (signal-ref, signal-targeting-expression, package-signal-targeting allOf-wrap) carry explicit discriminator blocks. No new undiscriminated oneOf introduced.

Follow-ups (non-blocking — file as issues)

  1. wholesale-feed-event.json $defs/signal contradicts the get_signals relaxation. L593-603 still requires data_provider and pricing_options. The changeset advertises the relaxation specifically for source-native, free, and bundled signals — get-signals-response.json L104-112 dropped those fields from required, but the same signal shape on signal.created / signal.updated webhook payloads still requires them. A source-native signal emitted via the webhook now fails validation. Drop both from the required list, or document why the wholesale-feed shape is stricter.

  2. index.json registry incomplete. L361 registers signal-ref and signal-targeting-expression but not the five other new schemas: product-signal-targeting-option, signal-targeting-rules, package-signal-targeting, package-signal-targeting-group, package-signal-targeting-groups. The analogous legacy entries (signal-id, signal-targeting, data-provider-signal-selector) are all registered. Downstream codegen and the docs portal discover schemas via this index.

  3. whats-new-in-3-1.mdx doesn't headline the new packaging. Nine new schema files and a wire-affecting reshape of the targeting overlay, but the doc only mentions SignalRef sideways in the Wholesale-signals row. Add a signal_targeting_groups section.

  4. Glossary gaps. docs/reference/glossary.mdx has no entries for Signal Targeting Options, Signal Targeting Rules, Signal Targeting Group, Selection Group, or Resolution Model (direct_targeting / seller_planned). These are new wire-level concepts buyers need to look up.

  5. Test coverage gaps in composed-schema-validation.test.cjs. Five SignalRef carry-over rejections missing (product scope vs agent_url/id; signal_source scope vs agent_url/id). No test exercises the requires_activationsignal_agent_segment_id required arm at product-signal-targeting-option.json L111-125. No rejection tests against package-signal-targeting-groups composition (missing top-level all, missing child operator, binary value: false, empty signals[]).

Minor nits (non-blocking)

  1. get_signals.signal_refs + signal_ids co-presence is undefined. signals/get-signals-request.json L151-168 accepts any combination. Add either a not clause forbidding both or a "if both present, signal_refs wins" sentence in the description.

  2. docs/signals/tasks/get_signals.mdx L908+ example omits signal_ref on the signal entry. Response schema now requires it; the example shows the deprecated shape. Either add signal_ref or label the block as "legacy response shape."

  3. docs/signals/key-concepts.mdx L67-79. "Signal references" table shows product scope as just signal_id, omitting the required scope discriminator. Per signal-ref.json L26, both scope and signal_id are required.

  4. scope: \"data_provider\" vs scope: \"signal_source\" disambiguation. signal-ref.json L71 says use data_provider when catalog-published "even if the catalog publisher is also the seller or signal source." The rule is right but a worked example would prevent two implementers picking different scopes for the same signal.

  5. signal-targeting.json numeric branch lacks the anyOf: [{required: [min_value]}, {required: [max_value]}] guard that the new signal-targeting-expression.json L80-83 has. Pre-existing, but worth aligning while you're in the neighborhood.

@aao-release-bot aao-release-bot 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.

Hold. The shape of the change is right, but the changeset is minor and three wire schemas plus the overlay reject-clause break existing valid 3.x traffic without the deprecated-alias soft launch that get-signals-response.json does correctly. Either bump the changeset to major or mirror that soft-launch pattern on the other rename sites.

Things I checked

  • signal_ref discriminated union — clean. discriminator: scope + oneOf + per-branch not.anyOf reject incompatible namespace carry-over (data_provider_domain, signal_source_url, agent_url, legacy source/id). tests/composed-schema-validation.test.cjs:630-690 exercises the branch hygiene end-to-end.
  • New oneOf shapes (signal-ref.json, signal-targeting-expression.json, package-signal-targeting-groups.json) all carry discriminator.propertyName, so the audit walker should classify them as discriminated. No baseline ratchet needed for the new files.
  • targeting_overlay.signal_targeting_groups composition — operator: "all" const at the top, child any for include groups, child none for exclusion. (any of include) AND NOT (any of exclude) is the right portable baseline.
  • Product.signal_targeting_* schema invariants at static/schemas/source/core/product.json:545-600 correctly gate options/source/rules behind signal_targeting_allowed: true, require either inline options or an explicit get_signals-backed source for rules, and require inline options for a product-backed source.
  • data_provider_signals deprecation — additive, properly marked deprecated: true. No required-field flips on that field.

Must fix before merge

  1. minor changeset on schemas that break previously-valid wire payloads. .changeset/signal-targeting-overlay.md:2 declares minor, but the diff renames signal_idsignal_ref as a required field on static/schemas/source/core/audience-selector.json:30,60,90, static/schemas/source/core/signal-targeting.json:28,53,78, and static/schemas/source/core/wholesale-feed-event.json:594 — with no aliased signal_id retained. Any 3.0/3.1 client that serializes { "signal_id": {...}, "value_type": "binary" } against these schemas will now fail validation. The get-signals-response.json:22-30 site does this correctly — signal_id is retained as optional with deprecated: true and signal_ref joins it on the response. Either upgrade the changeset to major, or extend that soft-launch pattern to the three rename sites. The get_signals pattern is the model; copy it.

  2. targeting.json:444-447 not.required: ["signal_targeting"] actively rejects production traffic. Same root cause. Buyers emitting the legacy flat targeting_overlay.signal_targeting array — which was valid before this PR — will now hit a schema rejection on create_media_buy and update_media_buy. This is intentional per the PR description, but intentional ≠ non-breaking. Pair this with a major bump, or downgrade to a deprecation note + grader rule until 4.0.

  3. wholesale-feed-event.json inline signal $def at lines 593-603 still requires data_provider and pricing_options. The new get-signals-response.json:104-112 drops both from required[] (so source-native, free, bundled, or caller-hidden cases work). The wholesale-feed event description at line 519 explicitly says "same wire shape as an item in get_signals.signals[]" — the two have drifted. Either drop data_provider and pricing_options from the inline required[] here, or amend the comment.

Follow-ups (non-blocking — file as issues)

  • product.json:584-600 validation invariant misaligned with signal_targeting_options_source default. Line 172 declares "default": "product", and the "product-backed source requires inline options" clause guards on required: ["signal_targeting_options_source"]. JSON Schema does not materialize defaults during validation, so a product with signal_targeting_allowed: true and no explicit signal_targeting_options_source and no inline options will validate cleanly even though runtime resolution would treat it as "product" (and therefore require inline options). Drop the default, or add an extra allOf clause that fires when signal_targeting_allowed: true and the source field is absent.
  • scripts/oneof-discriminators.baseline.json:159 lists core/audience-selector.json as dangerous. The in-place rename probably keeps the file disjoint in the same way, so the audit should still pass, but if npm run test:oneof-discriminators flips classification post-edit, run scripts/audit-oneof.mjs --update and ratchet the baseline in this PR.
  • Test coverage gap on the targeting overlay reject-clause. tests/composed-schema-validation.test.cjs adds positive and negative coverage on signal-ref.json and product.json but no test asserts that core/targeting.json rejects a payload containing the legacy flat signal_targeting array. Given (2) is the single most behavior-changing line in the PR, lock it with a rejection test.
  • Test coverage gap on signal-ref.json required-field branches. No negative test for scope: "data_provider" missing data_provider_domain or scope: "signal_source" missing signal_source_url. Three more lines.
  • signal_targeting_rules.selection_group_rules against real GAM-style backends. The "exactly one selection_group and one targeting_mode per child group" rule is clean for sellers who can publish their plane partitioning, but it asks the buyer to know it. Worth sanity-checking with the GAM-backed adopters before locking it.
  • resolution_model: "seller_planned" naming. Programmatic shops call this "guaranteed" or "PG". seller_planned is fine inside AdCP but worth a one-liner in the glossary mapping to the prevailing industry terms.

Minor nits

  1. glossary.mdx:26 Audience Selector entry is stale. Reads "either a signal reference (pointing to a specific signal_id) or a natural language description." Schema is signal_ref now; the value_type variants (binary/categorical/numeric) and the description branch should both be mentioned.
  2. signal-ref.json discriminator keyword inside draft-07. OpenAPI keyword, ignored by strict draft-07 validators. AJV with discriminator: true honors it; the oneOf + const: scope does the real work. A $comment clarifying it is advisory would help future maintainers.

Once the rename surfaces match the get_signals soft-launch pattern (or the changeset bumps to major), and the wholesale-feed-event.json required[] is reconciled with get-signals-response.json, this is mergeable.

@bokelley bokelley force-pushed the bokelley/signal-targeting-overlays branch from 1c9aa17 to d0afd3f Compare May 25, 2026 10:21
aao-release-bot[bot]
aao-release-bot Bot previously approved these changes May 25, 2026

@aao-release-bot aao-release-bot 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.

LGTM. Follow-ups noted below. Clean discriminator design and the not.anyOf carry-over guards on signal-ref.json are the right shape — the three scope branches are correctly disjoint and reject the legacy namespace fields per branch.

Things I checked

  • signal-ref.json oneOf is discriminated on scope; each branch blocks the other branches' namespace keys (product blocks data_provider_domain, signal_source_url, agent_url, source, id; data_provider blocks agent_url, signal_source_url, source, id; signal_source blocks data_provider_domain, agent_url, source, id). Right shape.
  • product.json allOf invariant at L535-553: signal_targeting_options ∨ signal_targeting_rules ⇒ signal_targeting_allowed: const true. Forces the wholesale "rules only, no inline options" path to still declare the surface.
  • targeting.json root-level not.required: signal_targeting correctly rejects the legacy flat field. Belt-and-braces — the prior schema had no signal_targeting property to begin with, so this is purely defensive against accidental client emission.
  • package-status-targeting-overlay-echo.json test vector matches the new shape: top-level operator: "all", child any/none, signal_ref.scope: "product", value_type: "binary", value: true.
  • composed-schema-validation.test.cjs exercises the not.anyOf carry-over rejection and the product allOf invariant including the wholesale-rules-without-options path.
  • index.json registers the seven new schemas.
  • get_signals response now requires signal_ref and relaxes data_provider + pricing_options from required to optional — consistent with the PR description's source-native / free / bundled cases.

Follow-ups (non-blocking — file as issues)

  1. Required-set swap is technically a breaking wire change on a minor changeset. get-signals-response.signals[].required previously included signal_id + data_provider; now requires signal_ref and drops both. Same swap on audience-selector.json, signal-targeting.json, and wholesale-feed-event.json $defs/signal.required. A pre-PR conforming server emitting only signal_id fails the new schema. Defensible as minor because signal_ref is the canonical replacement and the legacy field is retained as deprecated, but the changeset should explicitly call this out as a required-field swap with a migration note — or, more conservatively, accept either via anyOf: [{required: [signal_ref]}, {required: [signal_id]}] for one minor cycle so 3.0 emitters don't fail validation against 3.1 schemas. ad-tech-protocol-expert: sound-with-caveats.

  2. wholesale-feed-event.json $defs/signal vs get-signals-response.signals[] parity. L519 description says "the same wire shape as an item in get_signals.signals[]", but L593-603 still requires data_provider and pricing_options while the get-signals shape now relaxes both. Either drop them from required on the $defs/signal to keep the stated parity, or update the description to admit the divergence.

  3. signal_ref is optional on signal. webhook payloads.* wholesale-feed-event.json L250-251, L289-291, L334-336, L391-393 add a sibling signal_ref next to signal_agent_segment_id but don't require it on the four signal.* event branches. The PR positions signal_ref as the canonical identity surface — leaving it optional on these payloads erodes that claim. Consider requiring it on signal.created / signal.updated at minimum.

  4. Audit baseline note drift. scripts/oneof-discriminators.baseline.json:159-163 for audience-selector.json##/oneOf still says req=[type,signal_id,value_type,...] even though the variants now use signal_ref. Rank stays dangerous/variants=4 so --check won't fail, but the note is now misleading. Run node scripts/audit-oneof.mjs --update to refresh.

  5. signal-targeting-rules.json selection_group_rules.selection_mode description omits optional. L70-78 enumerates [optional, required, fixed] but the prose only narrates required and fixed. Add a one-line explanation of optional so consumers don't infer the enum was meant to be 2-value.

Minor nits (non-blocking)

  1. signal-ref.json product scope description. L13 says the signal_id MUST match a Product.signal_targeting_options[].signal_ref.signal_id for that product. Worth one example illustrating that scope product ids are not portable across products even when the string value coincides — easy reader trap.

Safe to merge.

@aao-release-bot aao-release-bot 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.

Block. The changeset says minor, but three wire surfaces flip a required field from signal_idsignal_ref with no oneOf compat branch — any pre-cutover client emitting the old shape fails validation. AdCP semver says a breaking wire change ships major (or carries a transition path); this does neither.

The new SignalRef shape is the right one. scope as resolution-path discriminator is correct, the not.anyOf namespace-rejection on each branch is clean, and product-scoped composition is the right place for it (sellers route products through different ad servers). Land it — but land it honestly.

Must fix

  1. Changeset is wrong, or schemas are. Pick one. static/schemas/source/core/audience-selector.json:30,60,90 flips required: [..., "signal_id", ...][..., "signal_ref", ...] on every signal branch. static/schemas/source/signals/get-signals-response.json:109-117 adds signal_ref to required and drops signal_id from required — signal_id is kept as deprecated optional, but signal_ref is now mandatory, so a seller emitting only the legacy shape fails validation. static/schemas/source/core/wholesale-feed-event.json:598-608 requires signal_ref on every $defs/signal instance (catalog enroll/update/price/remove at lines 250, 289, 334, 391). The PR description claims a deprecation window for SignalId; the schemas don't actually implement that. Fix one of: (a) bump to major with a migration note in the changeset, or (b) widen the three surfaces to oneOf signal_id | signal_ref for one minor while old clients catch up. ad-tech-protocol-expert graded this sound-with-caveats conditional on those surfaces being pre-GA — confirm that and update the changeset prose, or fix the schemas.

  2. wholesale-feed-event.jsonget-signals-response.json drift. static/schemas/source/core/wholesale-feed-event.json:519 describes $defs/signal as "same wire shape as an item in get_signals.signals[]", but the required list at 598-608 still mandates data_provider and pricing_options, both of which get-signals-response.json:109-117 just relaxed to optional. Either propagate the relaxation here or amend the description; one validator written against both surfaces will mis-reject the other.

Follow-ups (non-blocking — file as issues)

  • static/schemas/source/index.json registers signal-ref, signal-listing, and signal-targeting-expression but not signal-targeting-rules, product-signal-targeting-option, package-signal-targeting, package-signal-targeting-group, or package-signal-targeting-groups. SDK generators driven off the registry won't surface the new composition types.
  • Wholesale menu source ambiguity. A product carrying signal_targeting_allowed: true + signal_targeting_rules + no inline signal_targeting_options puts the burden on the buyer to know "go call get_signals." Worth an explicit signal_menu_source: inline | wholesale_feed enum on signal_targeting_rules — modeled after format_ids / format_options dual-emission contract on product.json:42-50, which carries an SDK-enforced same-canonical invariant.
  • resolution_model: "seller_planned" auditability. Sellers MUST echo applied signal_targeting_groups on package state, which lets auditors detect "ignored entirely," but not "covertly direct-targeted under a seller_planned label." Optional resolution_evidence field on package state would close that hole.
  • tests/composed-schema-validation.test.cjs:636-685 asserts the not.anyOf carry-over rejection per scope (good — line 647 quotes 'SignalRef product scope rejects SignalId source carry-over'). No positive vector covers targeting.json:444-447's flat-signal_targeting reject; the 187 new lines don't actually exercise that invariant. Worth adding.

Things I checked

  • New schemas use proper discriminator propertyName (scope on signal-ref, value_type on signal-targeting-expression). Each oneOf branch in signal-ref carries not.anyOf rejecting incompatible namespace fields — data_provider_domain, signal_source_url, agent_url, source, legacy id.
  • product.json:543-560 allOf invariant enforces signal_targeting_allowed: true when signal_targeting_options or signal_targeting_rules is present. Right shape.
  • targeting.json:444-447 rejects the legacy flat signal_targeting overlay field. Right call — forces groups-only buy-time composition.
  • scripts/oneof-discriminators.baseline.json unchanged — the new oneOfs are properly discriminated, so the audit walker is happy.
  • signal-targeting.json (orphaned — no other schema $refs it) had signal_idsignal_ref renamed cleanly; doc/index alignment is fine for the orphan.
  • signal-listing.json:60-80 allOf if/then correctly enforces inline name + value_type for scope: "product" but lets data-provider and signal-source refs ride on the catalog's authoritative definition.

Minor nits (non-blocking)

  1. Normative prose in description fields. signal-targeting-rules.json:54 carries "Sellers MUST reject duplicate, mixed, or collapsed groups…" as schema description text rather than as a JSON Schema constraint. Move to an x-adcp-validation block or accept that it's a server-side rule. Either is fine; pick one.

Bump the changeset to major, widen the three surfaces with a oneOf compat branch, or document a transition path that matches the PR description's claim. Pick one and ship.

@bokelley bokelley force-pushed the bokelley/signal-targeting-overlays branch from bd541f8 to 9dbc80e Compare May 25, 2026 12:16
aao-release-bot[bot]
aao-release-bot Bot previously approved these changes May 25, 2026

@aao-release-bot aao-release-bot 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.

Approving. Changeset is minor, schemas and docs are coherent, the SignalRef discriminator is properly fenced, and the product-invariant allOf matches the test asserts.

Things I checked

  • static/schemas/source/core/signal-ref.json:7-99scope discriminator with const + required on every branch; per-branch not.anyOf rejects namespace carry-over (data_provider_domain, signal_source_url, agent_url, source, id). Walker will classify as discriminated. All three scopes covered positive + negative at tests/composed-schema-validation.test.cjs:632-685.
  • static/schemas/source/core/product.json:543-560allOf if/then couples signal_targeting_options/signal_targeting_rules to signal_targeting_allowed: true. Both directions (missing + explicit false) rejected at tests/composed-schema-validation.test.cjs:805-838.
  • static/schemas/source/core/product-signal-targeting-option.json:26required: ["signal_ref"] correctly overrides the looser anyOf [signal_ref, signal_id] inherited from signal-listing; legacy signal_id on product options is rejected at tests/composed-schema-validation.test.cjs:777-784.
  • static/schemas/source/signals/get-signals-response.json:109-120signal_id, data_provider, and pricing_options dropped from required; anyOf [signal_ref|signal_id] keeps identity mandatory. Matches changeset (free/bundled signals can legitimately omit pricing).
  • static/schemas/source/core/targeting.json:443-451 — legacy flat signal_targeting retained as deprecated: true, schema-valid. Migration-window assertion at tests/composed-schema-validation.test.cjs:866-877.
  • .changeset/signal-targeting-overlay.mdminor is the right bracket: additive surface plus anyOf-gated legacy fields. No required field tightened on a published variant.

Follow-ups (non-blocking — file as issues)

  1. PR body says "rejecting"; schemas, changeset, and docs say "deprecating". "Enforces groups-only buy-time targeting by rejecting legacy flat `targeting_overlay.signal_targeting`" in the description doesn't match targeting.json:443-451, the test at tests/composed-schema-validation.test.cjs:866, or docs/media-buy/advanced-topics/targeting.mdx ("remains schema-valid during the SignalRef migration window"). Reword the PR body so the merge commit and the 3.1 changelog don't diverge from the schema.
  2. docs/reference/whats-new-in-3-1.mdx is silent on the migration. Largest single docs gap. Adopters on that page won't learn about SignalRef, signal_targeting_groups, signal_targeting_allowed/options/rules/included_signals, or the deprecation window. Add a short "SignalRef migration and product-scoped signal targeting" block.
  3. docs/reference/glossary.mdx:26 — Audience Selector backslides to signal_id. Same PR updates docs/governance/campaign/specification.mdx:390 to "a signal_ref or a natural language description"; the glossary entry should match its sibling.
  4. No removal version named for signal_id / signal_ids / flat signal_targeting. Changeset and signal-id.json:6 use "deprecated" + open-ended "migration window" with no x-removed-in anchor. Pick a target (4.0?) so the carry cost doesn't drift indefinitely.
  5. No direct positive/rejection tests for package-signal-targeting-groups.json and package-signal-targeting-group.json. The echo vector at static/test-vectors/media-buy/package-status-targeting-overlay-echo.json exercises the wire shape, but composed-schema-validation has no cases for: reject top-level operator: \"any\", reject empty groups/signals, reject child operator other than any/none. Add when convenient.
  6. docs/reference/glossary.mdx:312-313 — "Signal Source" overloads two concepts. signal_type: signal_source (the value-type enum) and signal_ref.scope: \"signal_source\" (the discriminator) want either split entries or an explicit cross-reference.

Minor nits (non-blocking)

  1. additionalProperties: true on the package signal targeting group schemas (package-signal-targeting-groups.json:26, package-signal-targeting-group.json:29, all three signal-ref.json branches). Consistent with the codebase. The not.anyOf carry-over rejections are doing the load-bearing branch-hygiene work — worth knowing the gate is weaker than false if a future field appears that needs strict rejection.

Safe to merge. Worth fixing the PR-body wording before the changelog gets cut so 3.1 ships with one story about flat signal_targeting, not two.

@aao-release-bot aao-release-bot 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.

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

This is an automated message from the Argus AI review workflow.

aao-release-bot[bot]
aao-release-bot Bot previously approved these changes May 25, 2026

@aao-release-bot aao-release-bot 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.

Approving. Coherent minor-version protocol move with clean schema architecture and an honest legacy/canonical migration story.

Things I checked

  • SignalRef scope hygiene is load-bearing. static/schemas/source/core/signal-ref.json:27-35,59-66,90-97 — the not.anyOf clause on every branch rejects foreign-namespace carry-over (data_provider_domain, signal_source_url, agent_url, source, id). Each clause is exercised by a negative test in tests/composed-schema-validation.test.cjs:636-685. The product-scope value-collision case is the right invariant to lock in.
  • Product invariants are tested both directions. static/schemas/source/core/product.json:543-561 (signal_targeting_options or signal_targeting_rulessignal_targeting_allowed: true) is matched by positive + two negative tests at tests/composed-schema-validation.test.cjs:786-838. Resolution-model enum rejection covers buyer_planned correctly.
  • Binary value: false cannot leak through package-level entries. package-signal-targeting.json:7-11 allOfs signal-targeting-expression.json:24-27 (value: const true); the wrapper does not redeclare value. Wire shape matches the docs' "use parent group none for exclusion" claim at docs/media-buy/advanced-topics/targeting.mdx:536.
  • Audit-oneof gate is satisfied. New discriminated oneOf schemas (signal-ref.json on scope, signal-targeting-expression.json on value_type) classify as discriminated and need no baseline entry. audience-selector.json stays dangerous in baseline — same kind, no regression.
  • adagents.json claim verified. static/schemas/source/adagents.json signals[].id and authorized_agents[].signal_ids[] remain local catalog strings, not the deprecated SignalId object — the PR description is accurate.
  • minor designation is grounded. docs/reference/versioning.mdx:23,128 explicitly sanctions required → optional flips within a minor, and CHANGELOG.md:1291 is the canonical precedent (protocol-envelope required:[status,payload] relaxation). The get_signals.signals[].data_provider / pricing_options relaxation lives in the same risk class.
  • Index registrations are complete. static/schemas/source/index.json:361-406 exposes all seven new core schemas.
  • Compliance scenario error.field path is correct. static/compliance/source/protocols/media-buy/scenarios/product_signal_targeting.yaml:371-372 cites packages[0].targeting_overlay.signal_targeting_groups.groups[0].signals[0].signal_ref — matches the schema nesting exactly.

Follow-ups (non-blocking — file as issues)

  1. audience-selector.json numeric branch is missing the min/max anyOf. Lines 99-119 require value_type but not min_value or max_value. signal-targeting-expression.json:80-83 carries the anyOf:[{required:min_value},{required:max_value}] that the selector should mirror. Today, {type:"signal", value_type:"numeric", signal_ref:{...}} with neither bound schema-validates — a nonsensical "any value of this numeric signal" predicate. Tightening this in a follow-up patch is allowed under versioning.mdx:130 and aligns with the inline comment at audience-selector.json:106.
  2. No AJV coverage on the headline package-level payload. tests/composed-schema-validation.test.cjs adds 270 lines and still has zero references to signal_targeting_groups, package-signal-targeting, or selection_group_rules. The compliance YAML exercises the shape end-to-end at runtime, but the AJV unit suite should also lock in: the operator: const "all" top, the binary value: const true rejection (proving the "use parent none" claim), and at least one positive + negative for selection_group_rules (missing required selection_group, invalid targeting_mode, minItems:1).
  3. Same signal_ref in both any and none groups is currently unguarded. docs/media-buy/advanced-topics/targeting.mdx:435 has a SHOULD-reject rule for geo_countries / geo_countries_exclude same-value overlap; nothing analogous appears near the signal_targeting_groups section at :524-540. Decide whether to add a SHOULD-reject or leave implementation-defined, but pick one.
  4. min_selected_signals: 0 + selection_mode: "required" is schema-valid but semantically contradictory. signal-targeting-rules.json:79-83 (top-level and inside selection_group_rules[]). Prose says sellers MUST treat the minimum as 1; consider an if/then raising minimum to 1 when selection_mode === "required", or surface as a documented runtime-enforced rule. Otherwise adopters will encode the contradiction.
  5. Legacy value: false → new none group migration row is missing from the docs migration table. The shape divergence is described in the deprecation note at targeting.mdx:529,536 but not represented as a row in any migration table. Buyers reading only the legacy signal-targeting.json:28-31 schema will not find the mapping.
  6. scope: "product" portability caveat lives only in the schema description. product-filters.json:205 calls out "seller-local exact option matching only, not a portable semantic identifier." The docs analog at media-products.mdx:505 says the weaker "meaningful only within the selected product/package context." Surface the portability framing in targeting.mdx near the signal_targeting_groups section so a buyer building multi-product discovery filters lands on it before the bug.

Minor nits (non-blocking)

  1. Notable that 2051 added lines and a new compliance scenario ship without an AJV positive case for signal_targeting_groups itself. The compliance YAML covers the wire path; the unit suite should still pin the shape so a future refactor of the oneOf branches doesn't silently widen it.

LGTM. Follow-ups noted below.

@geneticallymodifiedfoodforthought

Copy link
Copy Markdown

Q1. Agreed: "the buyer selects what audience; the seller resolves which avails at booking time" is the right framing, and it captures the AudioGraph case cleanly. The schema separation between buyer-selected signals and seller-resolved booking is the correct primitive. A structured resolution_model field would be useful as an additive future improvement — it would let agents reason about when not to attempt buyer-side optimization — but description carries the narrative for now. Happy to follow up with a separate issue if the working group thinks it's worth scoping.

Q2. Got it — selection_mode: "required" plus selection_group_rules covers what we need. The mixed mandatory/optional pattern (required audience tier + optional exclusions) is exactly the shape AudioGraph products will use.

Q3. Makes sense — scope: "data_provider" for the portable AudioGraph audiences, scope: "product" reserved for genuinely product-specific signals like broadcast daypart-availability. That's the right partition and it keeps the namespace clean across iHeartMedia's product portfolio.One extension worth flagging: the temporal-planning rationale for seller-side resolution isn't unique to broadcast radio. Live events show the same pattern — sports, political programming, tentpole live broadcasts. Inventory arrives in bursts, requires advance planning, and isn't typically traded in real-time. The buyer can't reason about which audiences will be available against which live windows; the seller has to resolve audience-to-avail at planning time, not impression time.This suggests the "buyer-composable, seller-resolved-to-scheduled-inventory" pattern is a broader category than broadcast radio alone — it's any inventory where supply is scheduled in advance and constrained by time-bound availability rather than continuously auctioned. Worth naming explicitly in the spec so live-event sellers know which primitives apply to them. Could be a docs addition rather than a schema change.

@bokelley

Copy link
Copy Markdown
Contributor Author

Q1 and Q2 confirmations close those threads — the schema separation and selection_mode: "required" + selection_group_rules shape land exactly as intended. The resolution_model follow-up is a clean additive future issue whenever the working group wants to scope it.

The Q3 live-events observation is worth capturing explicitly. The "buyer-composable, seller-resolved-to-scheduled-inventory" rationale does generalize cleanly to any time-bound supply that arrives in bursts and requires advance planning rather than real-time auction: live sports, political programming, tentpole broadcasts. The pattern is the same primitive; broadcast radio is just the motivating example in scope for this PR.

@bokelley — this feels like a one-to-two sentence addition to whatever narrative section explains when sellers resolve audiences rather than buyers. Worth folding into this PR while the context is warm, or would you prefer a follow-up issue so this PR can close without scope creep?


Generated by Claude Code

Adds one sentence clarifying that the seller_planned model applies to
any time-bound supply scheduled in advance, not only linear broadcast
audio — live sports, political programming, and tentpole broadcasts
follow the same primitive.

Prompted by iHeartMedia AudioGraph comment on PR thread.

https://claude.ai/code/session_019VvFeX77WCZen9dZCmkgsq
@bokelley

Copy link
Copy Markdown
Contributor Author

Folded the live-events generalization in — ec29498.

Added one sentence at the end of the seller_planned guidance paragraph in docs/media-buy/product-discovery/media-products.mdx:

The seller_planned model applies to any supply type where inventory is scheduled in advance and constrained by time-bound availability rather than continuously auctioned: live sports, political programming, and tentpole broadcasts follow the same pattern as linear broadcast audio.

@geneticallymodifiedfoodforthought — the AudioGraph context surfaced a real gap in the docs. The pattern was already there in the schema; it just needed naming for live-event sellers.


Generated by Claude Code

@bokelley

Copy link
Copy Markdown
Contributor Author

Good generalization — seller_planned has always been supply-type-agnostic at the schema level, so naming it explicitly for live-event inventory closes a real doc gap for non-audio sellers. No follow-up needed from my side.


Generated by Claude Code

aao-release-bot[bot]
aao-release-bot Bot previously approved these changes May 25, 2026

@aao-release-bot aao-release-bot 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.

LGTM. Follow-ups noted below. SignalRef migration is internally consistent — branch hygiene via not.anyOf blocks, allOf if/then gating signal_targeting_options on signal_targeting_allowed: true, and explicit discriminator on scope / value_type all land cleanly.

Things I checked

  • SignalRef branch hygiene. signal-ref.json:27-35,59-66,90-97 — each scope branch rejects cross-namespace fields (data_provider_domain, signal_source_url, agent_url, source, id). Verified the composed-schema test asserts this at tests/composed-schema-validation.test.cjs:631-685.
  • Product gating invariant. product.json:543-560 allOf if/then requires signal_targeting_allowed: true whenever signal_targeting_options or signal_targeting_rules is present. Composed-schema test confirms at 805-821.
  • Binary expression shape. signal-targeting-expression.json:23-27 pins value: true const for binary, so exclusion comes only from a parent none group. Docs at targeting.mdx and create_media_buy.mdx agree.
  • Discriminator audit. Both new oneOf schemas (signal-ref.json, signal-targeting-expression.json) declare discriminator.propertyName with const+required values. scripts/audit-oneof.mjs --check should classify them as discriminated; no baseline ratchet needed.
  • Legacy fields all deprecated: true. signal-id.json:5, get-signals-response.json:31, audience-selector.json:23,57,97, product.json:148. Migration window contract is honest.
  • Legacy signal_id fallback on wholesale events. wholesale-feed-event.json:611-614 $defs/signal carries anyOf: [signal_ref, signal_id]. The changeset's "signal_id remains accepted on wholesale signal events" claim is wired through the shared listing definition, not duplicated per-event.
  • Index registration. static/schemas/source/index.json:361-407 registers all 8 new core schemas; no duplicate $ids.
  • Changeset present and named. .changeset/signal-targeting-overlay.md exists with migration prose.

Follow-ups (non-blocking — file as issues)

  • get-signals-response.json:109-120 and the mirrored $defs/signal at wholesale-feed-event.json:603-609 drop data_provider and pricing_options from required. Consumer-side contract relaxation: a 3.0 buyer that destructures signals[].data_provider or signals[].pricing_options[0] will read undefined from a 3.1 server that omits them. Changeset is minor and frames this intentionally ("buyers should not assume…"), and this fits AdCP 3.1's documented migration posture. Worth a separate sanity pass against external buyer-agent pins — if any are hard-coded to required data_provider / pricing_options, consider an if/then keeping them required for signal_type: marketplace and relaxing only the source-native cases.
  • docs/reference/glossary.mdx:26 (Audience Selector entry). Reads "either a signal reference (pointing to a specific signal_id)" — re-introduces the signal_id framing in the same file that already has a "Signal Ref" entry at line 310 and a "Signal ID — Deprecated" entry at line 304. Should read signal_ref for internal consistency.
  • tests/example-validation.test.cjs only updates the existing get_signals fixture; no new example covers signal_targeting_groups, signal_targeting_options, or included_signals. Composed-schema is doing the load-bearing coverage, but example coverage catches MDX drift later.
  • Compliance scenario fixtures product_signal_targeting.yaml:69-87 omit several product.json-required fields (description, delivery_measurement, pricing rate/currency, reporting_capabilities). Confirm the compliance controller hydrates these before the validations.response_schema check at line 126 fires — otherwise schema validation fails before field-value asserts run.
  • Glossary entries for signal_targeting_groups, signal_targeting_options, signal_targeting_rules, included_signals, selection_group, resolution_model — five new public schema concepts repeating across media-products.mdx, targeting.mdx, create_media_buy.mdx, and whats-new. Glossary anchors per concept close the loop.

Minor nits (non-blocking)

  1. Loose additionalProperties: true on SignalRef branches. signal-ref.json:36,67,98. The not.anyOf block correctly rejects the named cross-namespace fields, but any typo or unlisted deprecated alias still passes the type check. Acceptable migration-window tradeoff; tighten in a future major bump.
  2. Co-located signal_ids in adagents.json authorization selector. docs/signals/data-providers.mdx:194,391 and whats-new-in-3-1.mdx:139 reference signal_ids arrays unrelated to the deprecated SignalId object — they're local catalog IDs in the authorization grammar. A casual reader skimming for signal_id will trip. One-line clarifying parenthetical helps.
  3. docs/media-buy/task-reference/create_media_buy.mdx:537-616 duplicates the (A OR B) AND NOT (C) example already present at targeting.mdx:556-590. Keep one minimal example here, link out for the longer walk-through.

Safe to merge.

@aao-release-bot aao-release-bot 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.

Request changes. The product-scoped signal targeting design is the right shape — included_signals for descriptive metadata, signal_targeting_options for selectable controls, signal_targeting_groups as the portable Boolean baseline, signal_ref with scope discriminator, legacy signal_id kept as deprecated. One asymmetric break needs to be reconciled with the rest of the PR before merge.

MUST FIX

product-filters.json:203-225 silently rejects legacy signal_id on get_products.filters.signal_targeting[]. Before this PR, the filter item used $ref: /schemas/core/signal-targeting.json, which accepts signal_ref OR deprecated signal_id via anyOf (signal-targeting.json:34-37). The new shape allOfs /schemas/core/signal-targeting-expression.json, which requires signal_ref and has no signal_id branch (signal-targeting-expression.json:29,54,79). A buyer agent calling get_products with filters.signal_targeting[].signal_id — the only shape that existed before signal_ref — now fails validation.

This is the only site in the PR where signal_id is dropped. targeting.json:443-451, signal-targeting.json:34-37, audience-selector.json:36-39, signal-listing.json:64-67, wholesale-feed-event.json:610-613, and get-signals-request.json:65-72 all keep the anyOf [signal_ref, signal_id] deprecation bridge. The changeset (signal-targeting-overlay.md:13) explicitly promises "During this minor-version migration window, legacy signal_id remains accepted on signal listings, audience selectors, legacy flat signal targeting, and wholesale signal events" — the filter slipped the list.

Two acceptable fixes:

  1. Add the anyOf [signal_ref, signal_id] bridge to the filter item to match every other surface in the PR.
  2. Bump the changeset to major and add a migration note calling out this filter explicitly.

Per the AdCP semver rule (CHANGELOG), shipping a tightening that rejects previously-valid input under a minor changeset is the block. The asymmetry was flagged independently by code-reviewer and ad-tech-protocol-expert.

Things I checked

  • signal-ref.json is properly discriminated on scope, with not.anyOf fencing carry-over of data_provider_domain, signal_source_url, agent_url, source, id across branches. The audit walker's discriminated classification holds.
  • product.json adds an allOf if/then invariant: presence of signal_targeting_options or signal_targeting_rules forces signal_targeting_allowed: true (product.json:543-560). Right shape.
  • signal-listing.json requires inline name + value_type only when signal_ref.scope === "product" — data-provider and signal-source refs defer to the authoritative catalog. Right shape.
  • targeting.json:443-451 correctly preserves flat signal_targeting as deprecated; new selection uses signal_targeting_groups.
  • get-signals-response.json:117-120 keeps anyOf [signal_ref, signal_id] for the wholesale-feed response — backwards-compatible.
  • Wholesale-feed event payloads (wholesale-feed-event.json:610-613) carry the same anyOf bridge.
  • The compliance scenario at static/compliance/source/protocols/media-buy/scenarios/product_signal_targeting.yaml:362-373 correctly exercises seller rejection of an unadvertised signal_ref with error.field pointing at the offending path.
  • Docs are consistent with the schema vocabulary (product / data_provider / signal_source scopes; all / any / none operators) across media-products.mdx, targeting.mdx, create_media_buy.mdx, get_products.mdx, get_signals.mdx, whats-new-in-3-1.mdx, glossary.mdx.
  • Changeset is minor with deprecated paths kept; the wire-impact justifies minor if the filter is reconciled. Otherwise major.

Follow-ups (non-blocking — file as issues)

  • get_signals.mdx:481,914 — two example response payloads still emit signal_agent_segment_id without signal_ref. The field-descriptions block at line 121 says new responses SHOULD include it; the canonical example at line 82 does. Backfill the two legacy examples.
  • media-products.mdx:409 and create_media_buy.mdx:537 — scope list mentions only product and data_provider; add signal_source for symmetry with targeting.mdx:23 and key-concepts.mdx.
  • signal-listing.json:64-67 + if/then at 68-89 — a legacy signal_id-only listing escapes the name + value_type requirement that applies to signal_ref.scope === "product". Acceptable as a migration-window concession but worth a comment.
  • signal-ref.json:36,67,98 — each branch's not.anyOf list is hand-maintained per scope. Adding a future identity field requires updating two other branches' exclusion lists. Consider additionalProperties: false plus an explicit allowlist when the migration window closes.

Minor nits (non-blocking)

  1. package-signal-targeting-groups.json:9-12. operator: { const: "all" } boxes v1 in; future expansion to any / none at the top level would be a non-breaking enum widening for read-validators but a codegen break for clients that ship the const as a literal type. Consider enum: ["all"] now.
  2. product_signal_targeting.yaml:99-104. stateful: false on get_products_wholesale while it feeds product_id into downstream stateful steps is inconsistent with adjacent discover→create scenarios. Probably harmless given storyboard semantics; worth aligning.
  3. signal-catalog-type.json:14-15. Em-dashes replaced with ASCII hyphens in two enumDescriptions. Stylistic only.

Drop the filter break or bump to major and this ships clean.

@aao-release-bot aao-release-bot 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.

LGTM. Follow-ups noted below. The SignalRef discriminator with explicit not.anyOf branch hygiene is the right shape — legacy SignalId stays opt-in via anyOf on every consumer surface while new buy-time entrypoints (signal-targeting-expression.json, product-signal-targeting-option.json) require canonical signal_ref. Four-surface product model (included_signals / signal_targeting_allowed / signal_targeting_options / signal_targeting_rules) is non-overlapping, and the allOf invariant at static/schemas/source/core/product.json:543-560 (options-or-rules → signal_targeting_allowed: true) is enforced where it matters.

Things I checked

  • Changeset .changeset/signal-targeting-overlay.md exists, classified minor, prose matches wire impact (legacy SignalId/signal_ids/flat signal_targeting retained as deprecated, no required-flips on request fields).
  • No new undiscriminated oneOf. SignalRef is discriminated on scope; signal-targeting-expression.json is discriminated on value_type. No regression against scripts/oneof-discriminators.baseline.json.
  • Cross-surface alignment: signal_ref threads through audience-selector.json, signal-targeting.json, signal-listing.json, wholesale-feed-event.json, get-signals-request.json / get-signals-response.json. Every consumer that previously accepted signal_id keeps it as anyOf with the deprecation noted.
  • Buy-time fence holds: package-signal-targeting.json extends signal-targeting-expression.json, which requires signal_ref and does not accept legacy signal_id. product-signal-targeting-option.json:26 requires signal_ref.
  • Docs↔schema coherence on the headline surfaces — media-products.mdx, get_products.mdx, create_media_buy.mdx, advanced-topics/targeting.mdx, whats-new-in-3-1.mdx, glossary.mdx. The five user-visible terms (included_signals, signal_targeting_allowed, signal_targeting_options, signal_targeting_rules, signal_targeting_groups) thread cleanly through all four pages.
  • Test coverage: tests/composed-schema-validation.test.cjs (+287) exercises the inline-options-requires-allowed invariant, branch hygiene on each SignalRef scope, and the product-local name+value_type requirement. tests/example-validation-simple.test.cjs (+114) covers the package-group composition path.
  • Compliance scenario static/compliance/source/protocols/media-buy/scenarios/product_signal_targeting.yaml (+373) exercises the wholesale get_signals → compose → create_media_buy flow end-to-end.
  • get_signals.signals[] required-list relaxation (data_provider, pricing_options → optional). Conservative direction for a response field; called out in the changeset.

Follow-ups (non-blocking — file as issues)

  • PR description overstates two invariants vs what shipped. Body says "Enforces groups-only buy-time targeting by rejecting legacy flat `targeting_overlay.signal_targeting`" — schema at `static/schemas/source/core/targeting.json:443` and the composed test at `tests/composed-schema-validation.test.cjs:866-878` confirm the flat form is still schema-valid (deprecated, not rejected — which matches the changeset prose). Body also says "Updates `get_signals` response to require `signal_ref`"; `static/schemas/source/signals/get-signals-response.json:117-119` requires `anyOf: [signal_ref, signal_id]` during the migration window. Either tighten the schemas to match the description, or reword the description. The migration-window posture is the right call; the body is the thing that's wrong.
  • `error-code.json` parity miss. `static/schemas/source/enums/error-code.json:323` `SIGNAL_NOT_FOUND.suggestion` still says "verify signal_id via get_signals" while the description at L127 was updated to `signal_ref`. One-line fix.
  • oneOf baseline note is stale (not regressed). `scripts/oneof-discriminators.baseline.json:159-163` notes `core/audience-selector.json` as `dangerous` with the pre-PR required list. Walker keys diff by file/pointer so CI won't fail, but the next baseline regen should refresh the note to reflect the four `signal_ref`-or-`signal_id` `anyOf` variants.
  • Compliance storyboard only exercises the happy wholesale path. `product_signal_targeting.yaml` doesn't cover (a) seller-side rejection of flat `signal_targeting` at buy time, (b) the `signal_targeting_options` ⇒ `signal_targeting_allowed: true` invariant, (c) `seller_planned` resolution, or (d) the `selection_group_rules` "exactly one selection_group per child group" rule from `signal-targeting-rules.json:54`. Schema tests cover the structural invariants; add storyboard phases or trim the body claim.
  • `signal-listing.json` and `get-signals-response.json` allow `signal_ref` and `signal_id` to coexist with no consistency constraint. A listing carrying both pointed at different signals is schema-valid. Add a "MUST prefer `signal_ref` when both are present" callout in the docs, or an `oneOf`-style guard. Low risk during migration but worth fencing before the deprecation window closes.
  • Field duplication between `wholesale-feed-event.json` and `signal-listing.json`. `wholesale-feed-event.json:516-616` re-declares most listing fields via `$defs` rather than `$ref`. First field added to `signal-listing.json` will drift. Either inline the `$ref` or add a drift guard.

Minor nits (non-blocking)

  1. Real brand domain reintroduced in get_signals examples. The follow-up commit `f293938a3` swapped most provider names, but `docs/signals/tasks/get_signals.mdx` L228, L373, L483, L895, L980 still carry `experian.com`. `tests/example-validation.test.cjs:296` already does the `pinnacle-data.example` substitution on the fixture side — mirror it in the .mdx.
  2. Long single bullets in docs. `docs/media-buy/product-discovery/media-products.mdx:415` collapses six independent `signal_targeting_rules` fields into one ~600-word bullet; `docs/media-buy/task-reference/get_products.mdx:180` is a ~200-word table cell. Both are accurate, but agent-readers parse better when each rule is its own sub-bullet.
  3. Pre-existing drift, surface while you're in the file. `docs/signals/tasks/get_signals.mdx:1025` references `"field": "filters.platforms"` in an error example, but `signal-filters.json` has no `platforms` property — `destinations` is the correct filter. Not yours, but worth a one-line fix.
  4. `signal-listing.json:71-87` conditional only triggers on `signal_ref.scope === 'product'`. A listing carrying only legacy `signal_id` slips past the name/value_type requirement. Safe in practice because legacy `signal_id` is data_provider/agent-scoped, but a comment noting why it's intentionally unconstrained would help the next reader.

Safe to merge once the description is reconciled with the deprecated-not-rejected reality.

@bokelley bokelley merged commit 1b6831e into main May 25, 2026
30 checks passed
@bokelley bokelley deleted the bokelley/signal-targeting-overlays branch May 25, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants