feat(reporting): add requested_metrics to get_media_buy_delivery - #6626
feat(reporting): add requested_metrics to get_media_buy_delivery#6626bokelley wants to merge 4 commits into
Conversation
The reporting webhook can narrow which metrics are delivered (reporting_webhook.requested_metrics); the GET path could not, so the interactive surface where agent context budget matters most always paid full payload cost. Add the same narrowing to the request: omitted means unchanged full payloads; impressions and spend are always included; requesting a leaf metric identity returns its canonical nested carrier; and missing_metrics MUST NOT flag absences caused solely by request narrowing. Implements RFC #6624. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # docs/media-buy/media-buys/optimization-reporting.mdx
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
| }, | ||
| "requested_metrics": { | ||
| "type": "array", | ||
| "description": "Optional list of metrics to include in the response. When omitted, all available metrics are included (unchanged behavior). Applies to every metrics-bearing object in the response: totals, by_package, daily and window slices, and breakdown rows. impressions and spend are always included regardless of this list. Requesting a leaf metric identity returns its canonical nested carrier — e.g. requesting viewable_rate returns the viewability object, requesting quartile_75 returns quartile_data — never a flat duplicate. Metrics requested but not available for this buy are omitted from the response without error; contract accountability is unchanged — missing_metrics still reconciles against committed_metrics, but sellers MUST NOT list a metric in missing_metrics when its absence is solely due to this narrowing. Must be a subset of the product's reporting_capabilities.available_metrics; values outside the declared set are ignored. Subset evaluation follows the container-subsumption rule in enums/available-metric.json. Sort is evaluated before narrowing: excluding a metric from this list never triggers the sort_by fallback, and breakdown rows may be ordered by a metric absent from the narrowed payload — the applied-sort echo still names it. Same semantics as reporting_webhook.requested_metrics.", |
There was a problem hiding this comment.
Medium: "Same semantics as reporting_webhook.requested_metrics" isn't literally true, in two ways. (1) This field adds minItems: 1; the webhook field (core/reporting-webhook.json:53-59) has no minItems, so [] is valid on the webhook and rejected here — a client sharing metric-narrowing validation across both paths hits the divergence. (2) The "impressions and spend are always included" rule is asserted here but absent from the webhook description; if the webhook shares that behavior, it belongs on the webhook side too (single source of truth), otherwise the two contracts genuinely differ. The parity guard in tests/requested-metrics-contract.test.cjs only checks type and items.$ref, so it won't catch either drift. Either align the constraints or soften the "identical semantics" wording in both schemas and the two docs pages.
There was a problem hiding this comment.
Fixed in the latest commit: the always-included impressions/spend rule now lives on the webhook field too, the webhook's empty-array case is defined as equivalent to omission, and both descriptions state the one intentional shape difference (GET requires ≥1 entry when present; omit for full payloads) instead of claiming identical semantics.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated paths touched without required approval.
This PR adds an optional requested_metrics field to the get_media_buy_delivery request schema, mirroring reporting_webhook. The change is coherent: schema, both docs pages, changeset, and a contract test are all consistent; the new field $refs the closed available-metric enum and the response metric fields are all optional, so narrowing produces valid payloads.
However, the PR modifies files under static/schemas/source/**, which are under a hard, deterministic approval gate (gated_paths: true). review_decision is not APPROVED (unknown/none), so per decision rule row 2 the gate is not satisfied and the outcome must be escalate, not approve.
Medium findings
- static/schemas/source/media-buy/get-media-buy-delivery-request.json:59 — The claim of "same/identical semantics as reporting_webhook.requested_metrics" is not literally true (minItems:1 and the always-include-impressions/spend rule diverge), and the parity test won't catch it. Non-blocking coherence gap.
Escalation reasons
- Modifies gated schema files (get-media-buy-delivery-request.json, get-media-buy-delivery-response.json) under
static/schemas/source/**; human/CODEOWNERS review required sincereview_decisionis not APPROVED.
Medium findings
- static/schemas/source/media-buy/get-media-buy-delivery-request.json:59 — Claimed parity with reporting_webhook.requested_metrics is not literal
Why human review
- Modifies gated schema files under static/schemas/source/** (get-media-buy-delivery-request.json, get-media-buy-delivery-response.json) — hard approval gate, review_decision is not APPROVED, human/CODEOWNERS review required
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/media-buy/get-media-buy-delivery-request.json (modified) matches
static/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'unknown', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
aao-secretariat
Bot
added
the
ladon/needs-human-review
Ladon review: "same semantics as reporting_webhook.requested_metrics" was not literally true — the GET field adds minItems: 1 and the always-included impressions/spend rule was stated only on the GET side. State the impressions/spend rule on the webhook field too, define the webhook's empty-array case as equivalent to omission, and describe the one intentional shape difference explicitly on both sides. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema files require human/CODEOWNERS approval.
This PR modifies published protocol source under static/schemas/source/** (reporting-webhook.json, get-media-buy-delivery-request.json, get-media-buy-delivery-response.json), which is a hard, path-based approval gate (gated_paths: true). The current review_decision is unknown/not APPROVED, so per decision-table row 2 the outcome must be escalate — a clean diff cannot lift this gate; only a recorded human/CODEOWNERS approval can.
Net improvement acknowledged: the prior run's single medium finding (get-media-buy-delivery-request.json:59 — overclaimed "Same semantics"/parity with reporting_webhook.requested_metrics) has been resolved. The two requested_metrics descriptions now cross-reference each other precisely: the webhook field (no minItems, empty array = omission) and the delivery-request field (minItems: 1) each spell out the shape divergence, and the "impressions and spend are always included" line matches existing docs. Schema↔docs coherence checks out and there are no new Critical/High/Medium findings.
Escalation reasons:
- Modifies gated schema files under
static/schemas/source/**(reporting-webhook.json, get-media-buy-delivery-request.json, get-media-buy-delivery-response.json) — hard approval gate,review_decisionis not APPROVED. Human/CODEOWNERS review required.
Once a human/CODEOWNERS approval is recorded, this PR is otherwise clean and should fall through to approve.
Why human review
- Modifies gated schema files under static/schemas/source/** (reporting-webhook.json, get-media-buy-delivery-request.json, get-media-buy-delivery-response.json) — hard approval gate (gated_paths: true), review_decision is not APPROVED. Human/CODEOWNERS review required before merge.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/reporting-webhook.json (modified) matches
static/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'unknown', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
…face The empty-array-equals-omission rule was stated only on the GET request's description; a reader of reporting-webhook.json alone could derive empty = impressions+spend only. State it where it applies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema files touched and no human/CODEOWNERS approval on record.
This PR modifies three files under static/schemas/source/** (reporting-webhook.json, get-media-buy-delivery-request.json, get-media-buy-delivery-response.json), which sit under a hard, non-overridable approval gate (gated_paths: true). The current review_decision is unknown/none — not APPROVED — so decision-table row 2 fires and Ladon cannot auto-approve regardless of how clean the diff is.
The reviewer surfaced no critical/high/medium findings. The delta since the prior pass is a single description clarification on reporting-webhook.json's requested_metrics: an empty array now explicitly means the same as omission (all metrics). It matches the schema (no minItems on the webhook field, so an empty array validates) and agrees with the sibling get-media-buy-delivery-request field (which carries minItems:1 and cross-references the intentional shape difference). Schema↔docs coherence checks out and no spec drift, changeset scope, oneOf-discriminator, or artifact-immutability issues were found.
Escalation reasons
- Modifies gated schema files under
static/schemas/source/**— hard approval gate (gated_paths: true),review_decisionis notAPPROVED. Human/CODEOWNERS approval required before merge.
The diff is mechanically clean; this escalation is purely the gated-path/review-status gate. Once a human/CODEOWNERS approval is recorded, this PR would fall through to a normal approve.
Why human review
- Modifies gated schema files under static/schemas/source/** (reporting-webhook.json, get-media-buy-delivery-request.json, get-media-buy-delivery-response.json) — hard approval gate (gated_paths: true), review_decision is not APPROVED. Human/CODEOWNERS review required before merge.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/reporting-webhook.json (modified) matches
static/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'unknown', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
1 participant
Summary
Stacked on #6622 (base retargets as the stack merges). Implements RFC #6624.
The reporting webhook can narrow which metrics are delivered (
reporting_webhook.requested_metrics); the GET path could not — so the interactive surface where agent context budget matters most always paid full payload cost. This adds the same optional narrowing toget_media_buy_delivery, applying to totals, by_package, daily/window slices, and breakdown rows.Resolutions of the RFC's open questions (all normative in the schema text):
impressionsandspendare always included.viewable_ratereturns theviewabilityobject, never a flat duplicate (consistent with feat(reporting): add leaf metric identities for nested delivery values #6618's no-duplicates rule and the container-subsumption rule now stated inavailable-metric.json).missing_metricsstill reconciles againstcommitted_metrics, but sellers MUST NOT flag a metric whose absence is solely due to request narrowing.sort_byfallback; rows may be ordered by a metric absent from the narrowed payload and the applied-sort echo still names it. Conformance note: graders must not order-verify from response bodies in that case.Webhook parity is guarded by a contract test asserting
reporting_webhook.requested_metricskeeps the same item vocabulary.🤖 Generated with Claude Code