Conversation
Contributor
There was a problem hiding this comment.
LGTM. Additive wire change with the right shape — new signal-coverage-forecast.json composes on top of existing ForecastPoint/ForecastRange primitives with a proper kind: "signal" discriminator, and coverage_rate joins the closed enum the same way measured_impressions and plays did. Follow-ups noted below.
Things I checked
static/schemas/source/core/forecast-point.json:48-58—coverage_rateoverlay usesallOf: [forecast-range.json, {properties: {low/mid/high: maximum:1}}]. allOf is conjunctive, soforecast-range.json'sanyOf(mid OR low+high) is preserved. Cap is per-point, not per-sum — correctly orthogonal tobucket_semantics: "overlapping".static/schemas/source/core/forecast-point-dimensions.json:181-263— new signal arm carrieskind: const "signal". Audit walker atscripts/audit-oneof.mjsstays satisfied; no baseline ratchet needed.static/schemas/source/core/forecast-point-dimensions.json:221-262—if/thenconditionals onpresenceare wired both directions:absentforcessignal_value: nullANDrequired,presentconstrains type to[string,number,boolean]. Asymmetry onsignal_valuerequiredness in thepresentbranch is intentional per the description at L204.static/schemas/source/enums/forecastable-metric.json:24,43—coverage_rateappended to enum + enumDescription. Closed-enum additive precedent holds.static/schemas/source/core/wholesale-feed-event.json:583-593— wholesale parity uses$refto the samesignal-coverage-forecast.json. Identity stays structurally aligned withget_signals.signals[]..changeset/contextual-signal-coverage-forecasts.md—minorcorrectly matches an additive wire change. Description names thecoverage_percentagecompatibility story.- Docs vs schema:
docs/signals/tasks/get_signals.mdx:93-142,684-686anddocs/media-buy/product-discovery/media-products.mdx:984,1180— field names, optionality, and the bucket_semantics/bucket_completeness orthogonality match the schema. No drift.
Follow-ups (non-blocking — file as issues)
coverage_percentageisrequiredanddeprecated: trueat the same time.static/schemas/source/signals/get-signals-response.json:114-121keeps it inrequired; L87-93 of the same file marks it deprecated. Same pattern atstatic/schemas/source/core/wholesale-feed-event.json:583-589. Generated client types will surface a required-and-deprecated field, which is a confusing surface. Either drop fromrequirednow (the planned exit ramp) or downgrade the field to a description-only "legacy" note until a future major. Both subagents flagged this independently.enclosing_signal_identity_matchis prose-only.static/schemas/source/core/signal-coverage-forecast.json:8-10defines the resolution rule but JSON Schema cannot enforce it. A buyer materializing a wholesalesignal.*event out of context — webhook fan-out, event-stream replay — has no schema guarantee that a baresignal_idshorthand undercoverage_forecast.points[].dimensionsbelongs to the enclosing signal. Either requiresignal_ref(drop shorthand) when the enclosing scope is wholesale-feed-event, or pin the gap with a conformance test.- Test helpers in
tests/schema-validation.test.cjs:622-682prove themselves, not the wire.signalDimensionMatchesEnclosingSignalandcompleteExclusiveCoverageRatesPartitionrun against hand-crafted JS objects, never against a payload that has just passedvalidateGetSignalsResponse. Feeding a schema-valid mismatched-identity payload through and asserting the helper fails would document the gap so the next person who tries to enforce it on the wire sees a red test. coverage_ratecap tested only onmid.tests/schema-validation.test.cjs:509-513exercises{ mid: 1.2 }. The schema capslow/mid/high; add twoassertInvalidcases for{ low: 1.5, high: 2.0 }and{ low: 0.2, high: 1.5 }so per-bound mechanics are pinned.
Minor nits (non-blocking)
additionalProperties: trueat the top ofsignal-coverage-forecast.json:152. A typo likebucket_completnesswould silently ride the wire. Open is intentional insidescope(L100) for seller qualifiers; the top of the object doesn't need it.presence: "present"with omittedsignal_valuehas noassertValidtest.static/schemas/source/core/forecast-point-dimensions.json:244-260deliberately allows this per the description at L204 — worth one positive test to pin the intent.
Approving on the strength of clean ad-tech-protocol-expert and code-reviewer verdicts plus the schema/docs coherence audit.
This was referenced May 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds contextual signal coverage forecasts for
get_signalsand product-relative availability planning.SignalCoverageForecastwith explicit denominator scope, bucket overlap/completeness semantics, and availability-only forecast pointskind: "signal"forecast dimensions with canonicalsignal_ref, presence semantics, and signal-value bucketscoverage_rateas a forecastable availability metriccoverage_forecastthrough bothget_signals.signals[]and wholesale signal feed event payloadsCloses #5086.
Validation
npm run test:schemasnpm run test:composednpm run test:json-schemanpm run test:examplesnpm run test:build-schemas-hoist-enumsnpm run test:oneof-discriminatorsnpm run test:schema-utf8npm run build:schemasgit diff --checknpm run test:unit,npm run test:test-dynamic-imports,npm run test:callapi-state-change,npm run typecheckExpert Review
Ran protocol, product/workflow, and schema/versioning expert reviews. Addressed blockers around wholesale feed parity, canonical signal identity, bucket semantics, stricter schema invariants, changeset coverage, and conformance hints.