Skip to content

[codex] Add dimensional forecast point fields#5061

Open
bokelley wants to merge 3 commits into
mainfrom
forecast-fields-for-avails
Open

[codex] Add dimensional forecast point fields#5061
bokelley wants to merge 3 commits into
mainfrom
forecast-fields-for-avails

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 27, 2026

Summary

Adds dimensional and measurement-aware forecast point fields for #5058.

  • adds ForecastPoint.dimensions as an array of dimension constraints, so a single point can represent intersections such as placement x country, placement x device, or geo x audience
  • supports canonical kind variants for geo, placement, device type, device platform, and audience dimensions
  • constrains geo forecast dimensions to existing targeting vocabularies: country/region omit system, metro uses metro-system, and postal uses postal-system
  • makes duplicate dimension kind values a conformance error via normative schema/docs language plus x-adcp-validation.unique_item_properties: ["kind"] and test coverage
  • adds forecasted viewability ranges and forecasted vendor_metric_values ranges independent of pricing_options
  • aligns delivery viewability with forecast viewability by requiring viewability.standard whenever measured viewability values are present
  • adds optional ForecastPoint.product_id so proposal-level dimensional rows can map back to executable product allocations
  • documents row identity, additivity/rollup rules, placement routing vs package targeting, and the current delivery-reporting limitation that standard breakdowns verify one-dimensional marginals rather than exact cross-dimensional intersections
  • adds placement x-entity coverage and a minor changeset

Validation

  • npm run test:schemas
  • npm run test:examples
  • npm run test:oneof-discriminators
  • npm run test:json-schema -- --file docs/media-buy/product-discovery/media-products.mdx
  • npm run test:json-schema -- --file docs/media-buy/task-reference/get_products.mdx
  • direct AJV validation for valid/rejected forecast samples, including placement x country intersections, metro/postal system mismatches, country system rejection, and forecast/delivery viewability standard requirements
  • npm run build:schemas
  • git diff --check
  • precommit hook: image checks, npm run test:unit, npm run test:test-dynamic-imports, npm run test:callapi-state-change, npm run typecheck
  • pre-push hook: version sync, schema link convention, Mintlify broken-links check

Notes

Expert review feedback was incorporated across protocol, docs, and consumption semantics. The array shape is preserved for readable intersections, while repeated kind values are now a conformance error handled by normative docs and custom validation metadata because JSON Schema draft-07 cannot express unique-by-property for this array form.

@bokelley bokelley marked this pull request as ready for review May 27, 2026 03:20
aao-release-bot[bot]
aao-release-bot Bot previously approved these changes May 27, 2026
Copy link
Copy Markdown
Contributor

@aao-release-bot aao-release-bot Bot left a comment

Choose a reason for hiding this comment

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

LGTM. Additive optional fields on ForecastPoint, minor changeset is the right call. Follow-ups below.

Things I checked

  • forecast-vendor-metric-value.json is a clean ForecastRange-swapped twin of core/vendor-metric-value.json — same envelope, same required set [vendor, metric_id, value], same additionalProperties: false. Only value and measurable_impressions change shape.
  • forecast-point.json:48-95 viewability mirrors delivery-metrics.json:284-318 field-for-field with ForecastRange swaps. additionalProperties: true matches the delivery side.
  • oneOf in forecast-point-dimensions.json is disjoint: each branch gated by kind const + additionalProperties: false. Discriminator pattern that scripts/audit-oneof.mjs accepts — no baseline regression.
  • kind values (geo, placement, device_type, device_platform, audience) line up 1:1 with the existing reporting_dimensions keys in media-buy/get-media-buy-delivery-request.json:96,132,149,166,184. The doc's post-purchase reconciliation claim is load-bearing.
  • Buyer workflow mapping in media-products.mdx is grounded: targeting_overlay.geo_countries/geo_regions/geo_metros/geo_postal_areas, audience_include, device overlay fields, creative_assignments[].placement_refs all exist on the schemas referenced.
  • x-entity: "placement" registration is correct — composite publisher_domain + placement_id namespace, not product-scoped. Applied consistently in placement-ref.json and placement.json.
  • viewable_rate lower bound inherited from forecast-range.json minimum: 0; the allOf only needs to add maximum: 1. No drift.
  • Changeset is minor. Base schema already had additionalProperties: true; new fields are all optional. Right type.

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

  • system is free-form on forecast geo rows. forecast-point-dimensions.json:21-24 types system as bare "type": "string", but the reporting-side counterpart at media-buy/get-media-buy-delivery-request.json:105-114 uses oneOf(metro-system, postal-system), and core/targeting.json:50-52,98-101 $refs those same enums. Forecast rows can emit a system value buyers cannot reconcile against targeting/reporting. Tighten to the same oneOf for parity.
  • Forecast viewability requires standard; delivery viewability does not. forecast-point.json:87-95 enforces "any of measurable_impressions / viewable_impressions / viewable_rate / viewed_seconds present → standard required." delivery-metrics.json:284-318 carries no equivalent invariant. The stricter forecast-side rule is defensible (MRC vs GroupM rows are not interchangeable, as the doc says), but the asymmetry means a row that round-trips forecast → delivery loses the guarantee. Consider whether to backport the invariant to delivery-metrics.
  • The if/then requiring system for metro/postal_area is stricter here than at peer sites. forecast-point-dimensions.json:35-50 enforces conditional system requirement. The same constraint is prose-only at get-media-buy-delivery-response.json by_geo and core/product-filters.json required_geo_targeting. Either backfill the conditional there or note the asymmetry.

Minor nits (non-blocking)

  1. Empty viewability: {} validates. forecast-point.json:87-95 — the not(anyOf required) branch accepts an empty object. Probably intended (same effect as omitting the field), but worth a one-line schema comment so a future reader doesn't read it as a bug.
  2. viewable_rate.allOf[1] lacks "type": "object". forecast-point.json:65-74 — works in practice because the only value reaching that constraint is already an object, but delivery-metrics.json states minimum: 0 and maximum: 1 inline as one block. The split-across-allOf form is more clever than it needs to be.

Approving on the strength of the dimensional-row design (one product, many rows beats product-per-country fanout) plus the mirror discipline between forecast and delivery shapes. Safe to merge.

@bokelley
Copy link
Copy Markdown
Contributor Author

Non-blocking follow-up notes from review:

  • geo.system for metro / postal_code rows is still free-form. A follow-up should check whether we can reuse existing metro/postal enums or registry values instead of leaving this open-ended.
  • Forecast viewability.standard is required whenever forecast viewability values appear, while delivery reporting does not currently enforce the same invariant. That asymmetry is intentional enough for this PR, but worth reconciling separately.
  • The schema permits multiple dimension items with the same kind. That can be useful for hierarchical constraints, but consumers should treat ambiguous peer values, such as two countries in one point, as seller-defined or invalid unless documented.

aao-release-bot[bot]
aao-release-bot Bot previously approved these changes May 27, 2026
Copy link
Copy Markdown
Contributor

@aao-release-bot aao-release-bot Bot left a comment

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. Right shape for what this is solving — dimensions as a kind-discriminated array lets one product expose placement×country availability without product-per-slice expansion, and the canonical-id parity with reporting dimensions and targeting overlays is intact.

Things I checked

  • forecast-point-dimensions.json oneOf uses kind with const values across all five variants and required: ["kind", ...] on every branch. Classifies as discriminated per scripts/audit-oneof.mjs:188-215 — no baseline entry needed, and absent from scripts/oneof-discriminators.baseline.json.
  • Canonical identifier parity: geo_level/geo_code/system, device_type, device_platform, audience_id+audience_source all match the reporting-dimensions block in get-media-buy-delivery-response.json and the targeting overlay surfaces. placement_ref matches creative_assignments[].placement_refs.
  • viewability mirror parity vs delivery-metrics.viewability at delivery-metrics.json:284-318. New forecast version adds the ratchet at forecast-point.json:87-95 requiring standard whenever any rate field is present — branch A (no rate field) and branch B (standard present) correctly cover the cases.
  • viewable_rate allOf at forecast-point.json:64-76 correctly bounds low/mid/high to [0, 1] via override of the base forecast-range.json numeric range.
  • forecast-vendor-metric-value.json mirrors vendor-metric-value.json — same required [vendor, metric_id, value], same additionalProperties: false, breakdown passthrough preserved. Only delta is value and measurable_impressions upgraded to ForecastRange. Right call.
  • Geo if-then requiring system when geo_level is metro or postal_area works given geo_level is required at top level.
  • Docs example at media-products.mdx:1094-1166 validates against the schemas. Both example points include standard: "mrc" whenever a viewability rate field is emitted — the new ratchet is honored in the doc itself.
  • Buyer-workflow mapping at media-products.mdx:1170-1176: creative_assignments[].placement_refs as creative-routing-only is consistent with creative-assignment.json — placement narrowing happens at package targeting time, not at creative assignment.
  • Changeset minor is correct: every new field is optional and additive. No required-field flips, no enum removals, no shape changes to existing surfaces.
  • x-entity: placement annotation on both placement.json:16 and placement-ref.json:16 follows the established pattern (annotate every site carrying the identifier, dedup is the lint's job).

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

  • Placement dimension shape diverges from delivery by_placement. forecast-point-dimensions.json:65-72 uses a structured placement_ref (publisher-scoped), but get-media-buy-delivery-response.json by_placement still emits a bare placement_id string. Forecast shape is the better one — multi-publisher-safe and aligned with creative_assignments[].placement_refs — but the asymmetry means buyer agents can't cross-walk forecast→delivery rows by identical key extraction. Worth a follow-up to upgrade by_placement.placement_id to placement_ref, or an explicit note that the asymmetry is deliberate during transition.
  • standard-required ratchet asymmetric vs delivery-metrics. forecast-point.viewability requires standard when any rate field is present (forecast-point.json:87-95); delivery-metrics.viewability keeps standard optional unconditionally (delivery-metrics.json:284-318). Reasonable to enforce up-front at forecast time and rely on the seller carrying the same standard into delivery, but the inconsistency lets a buyer compare a forecasted MRC viewable_rate against an unlabeled delivered viewable_rate. Either tighten delivery-metrics in a separate change or document the divergence as intentional.
  • uniqueItems does not enforce per-kind uniqueness on dimensions. forecast-point-dimensions.json:6-8 — two items with the same kind but different codes (e.g., geo:US and geo:CA on one point) deep-compare as distinct and pass uniqueItems. Two geos in one intersection is not a meaningful row. Either add a per-kind uniqueness constraint or document in the description that intersection items must each have a distinct kind.
  • audience_id selectability constraint is only in prose. The docs at line 1173 correctly note that informational audience rows don't automatically become targeting handles, but forecast-point-dimensions.json:111-135 doesn't carry that constraint into the schema description. Mirror the prose into the schema description so buyer agents reading the schema in isolation don't try to lift kind: "audience" rows into audience_include.

Minor nits (non-blocking)

  1. Viewability additionalProperties: true plus open anyOf leaves a typo escape hatch. forecast-point.json:48-96 — a payload with a typo like viewablerate (missing underscore) would pass branch A of the anyOf because none of the spelled rate fields are present, so standard is not enforced. Either tighten to additionalProperties: false on the viewability block, or accept that the open shape is intentional for forward-compatible vendor extensions and the typo case is a buyer-side conformance bug.
  2. Empty placement_id is allowed. placement-ref.json:13-17 has no minLength on placement_id, so a placement_ref: { placement_id: \"\" } validates. Pre-existing, not introduced here, but inherited by the new placement dimension variant.
  3. Redundant type: string inside the geo if. forecast-point-dimensions.json:42-46 — the outer geo_level $ref already constrains to the enum; the nested type: string is harmless but redundant.

Approved.

@bokelley
Copy link
Copy Markdown
Contributor Author

Resolved the follow-up concerns in 42ed392693:

  • metro/postal forecast geo systems now reuse the existing metro-system / postal-system enums, with country/region rejecting system
  • delivery viewability.standard is now required whenever measured viewability values are reported, matching forecast-side semantics
  • repeated dimensions[].kind is now a normative conformance error, documented for consumers and covered by test:schemas via x-adcp-validation.unique_item_properties
  • added placement-routing clarification, row identity/rollup guidance, delivery-reporting caveat for cross-dimensional intersections, and optional ForecastPoint.product_id for proposal-level executable rows

Validation and pre-push checks passed; PR body has the full list.

Copy link
Copy Markdown
Contributor

@aao-release-bot aao-release-bot Bot left a comment

Choose a reason for hiding this comment

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

Request changes. The dimensional ForecastPoint shape is the right design — array of intersection items with kind as the set key, geo if/then constraining system to the existing metro/postal enums, ForecastRange-shaped viewability and vendor metrics, product_id for proposal-level allocation mapping. That part is clean. The block is the bundled tightening of delivery-metrics.viewability shipped under a minor changeset.

static/schemas/source/core/delivery-metrics.json:318-331 adds an anyOf that requires standard whenever measurable_impressions, viewable_impressions, viewable_rate, or viewed_seconds is present. Published dist/schemas/3.0.12/core/delivery-metrics.json had standard as a plain optional property with no conformance gate. That is an optional→required flip on an existing public field. Per MUST FIX #5 and the playbook semver rules at lines 295-298, an optional→required flip on a published field is a major change. .changeset/5058-forecast-point-dimensions.md:2 declares minor; main is in pre mode so this ships as 3.1.0-beta.N, not 4.0.0. A 3.0-conformant seller emitting viewability: { measurable_impressions: N, viewable_rate: r } without standard becomes schema-invalid against 3.1.x without a major-version migration note. The same wire-break is duplicated in static/schemas/source/compliance/comply-test-controller-request.json:696-708 for simulate_delivery.params.viewability, so any existing harness fixture that omits standard will also break.

I agree that MRC and GroupM rows are materially different and standard SHOULD be present. The data-quality argument doesn't change the semver math.

Fix options

  1. Split the delivery-metrics tightening into its own major changeset with a migration note. The forecast-point additions stay minor. This is the cleanest path.
  2. Weaken to SHOULD in prose, drop the anyOf from delivery-metrics.json and comply-test-controller-request.json. The new forecast-point viewability anyOf at forecast-point.json:91-100 is fine to keep — that's a new field, additive. Only the delivery-side tightening is the wire-break.

Either is acceptable. Pick one.

Things I checked

  • Geo if/then composes correctly under draft-07. not.required.system correctly rejects {kind:"geo", geo_level:"country", geo_code:"US", system:"us_zip"}; the metro/postal branches' nested system: $ref enums layer on top of the variant-level system: {type:string} so metro+us_zip rejects. forecast-point-dimensions.json:42-99.
  • oneOf discriminator soundness — every variant declares kind as const with required:["kind"] and distinct values. scripts/audit-oneof.mjs const-property path classifies as discriminated. No baseline ratchet needed.
  • x-adcp-validation.unique_item_properties: ["kind"] at forecast-point-dimensions.json:6-9 is docs-only — AJV won't enforce it. The buyer-side dimensions sorted by kind row-identity claim in docs/media-buy/product-discovery/media-products.mdx:984 is therefore stronger than the wire enforces.
  • Docs/schema coherence on the array shape — the JSON examples at media-products.mdx:1095-1167 use the dimensions: [...] array form consistently. The earlier object form from the first commit is gone.
  • product_id semantics are clear at forecast-point.json:19-23 and media-products.mdx:1178 — populated when a proposal-level dimensional row maps to one allocation, omitted for aggregates spanning multiple products.
  • x-entity: placement added to placement-ref.json:16, placement.json:16, x-entity-types.json:50. Composite identity documented in the type entry; matches the vendor_metric precedent.

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

  • Test 9 in tests/schema-validation.test.cjs:444-479 is thin. It validates the x-adcp-validation annotation and exercises a hand-rolled hasRepeatedKind helper. It does not compile forecast-point-dimensions.json with AJV and exercise the if/then/not.required.system country/region branches, the metro/postal system enum mismatch rejection, or the new viewability anyOf gate against valid/rejected fixtures. The PR description claims those AJV runs happened locally; the fixtures aren't in the repo. Add a fixture-based AJV pass so the conformance gates are CI-enforced.
  • x-adcp-validation.unique_item_properties is unenforced at the wire. Either ship a custom AJV keyword that conformance tooling consumes, or update the docs to say kind uniqueness is buyer-side normalization, not a schema-enforced invariant. The current state has docs claiming MUST NOT and a wire that silently passes the violation.

Reblock comes off once the delivery-metrics + comply-test-controller tightening is either pulled out into its own major or weakened to SHOULD.

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.

1 participant