Skip to content

feat(reporting): add by_format delivery breakdown - #6616

Open
bokelley wants to merge 6 commits into
mainfrom
adcp-reporting-gaps-3-2
Open

feat(reporting): add by_format delivery breakdown#6616
bokelley wants to merge 6 commits into
mainfrom
adcp-reporting-gaps-3-2

Conversation

@bokelley

@bokelley bokelley commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

What changed

  • adds reporting_dimensions.format to get_media_buy_delivery
  • adds capability-gated by_format rows keyed by canonical format_kind
  • adds by_format_truncated and supports_format_breakdown
  • documents custom aggregation, GET-path negotiation, webhook/window parity, and safe reconciliation semantics
  • adds focused schema contract tests and a minor protocol changeset

Why

Buyers cannot currently ask how package delivery splits across canonical creative formats without joining creative IDs to another API response and aggregating client-side. Some sellers can report format-grain delivery without exposing individual creative performance, so by_creative is not a complete substitute.

This implements the additive direction in #6442 while incorporating its review findings. The breakdown deliberately identifies canonical creative shape, not duration, dimensions, or other format-option parameters.

WG decision

This PR proposes format_kind as the 3.2 grain. That answers comparisons across canonical shapes (video_hosted vs. image vs. native_in_feed) and keeps the row key bounded and interoperable. It intentionally does not answer 15s-vs.-30s or size/option comparisons within one kind; those require a future option-level identity such as format_option_ref. Collapsing adopter-defined shapes into custom has the same tradeoff. Reviewers should ratify that coarse grain rather than infer duration support from the name by_format.

Compatibility

All request, response, and capability fields are optional. Existing sellers and buyers require no changes unless they advertise or request the new dimension.

Validation

  • node --test tests/format-delivery-reporting-contract.test.cjs
  • npm run test:schemas
  • Changesets protocol-scope and status checks passed
  • repository unit suite passed 1,043/1,043 tests on the original implementation head
  • independent code and protocol expert reviews have no remaining blockers

@bokelley bokelley changed the title [codex] feat(reporting): add by_format delivery breakdown feat(reporting): add by_format delivery breakdown Aug 16, 2026
@bokelley
bokelley marked this pull request as ready for review August 16, 2026 20:15
@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Aug 16, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ladon verdict: Escalate to human review

Escalate — gated path change requires human/CODEOWNERS approval.

This PR modifies four files under static/schemas/source/**, which are under a hard, deterministic approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so decision-table row 2 fires: the gate is not satisfied and the PR cannot be approved without a human/CODEOWNERS review.

There are no blocking (critical/high) or medium findings. The reviewer confirmed the change is an additive Normative (non-breaking) capability-gated by_format delivery breakdown keyed by canonical format_kind on get_media_buy_delivery. Schema↔docs coherence checks out across both docs pages, it mirrors the established by_device_type allOf + _truncated pattern, carries a correct minor changeset, introduces no new oneOf, edits no dist/ artifacts, and wires a contract test into test:schemas. Absent the approval gate this would approve — the escalation is purely to obtain the required human review on the gated schema-source files.

Gated files requiring approval:

  • static/schemas/source/core/canonical-reporting-capabilities.json
  • static/schemas/source/core/reporting-capabilities.json
  • static/schemas/source/media-buy/get-media-buy-delivery-request.json
  • static/schemas/source/media-buy/get-media-buy-delivery-response.json

Why human review

  • Touches gated schema-source files under static/schemas/source/** with review_decision=REVIEW_REQUIRED (not APPROVED) — human/CODEOWNERS approval required before merge (decision-table row 2).
  • Gated files: static/schemas/source/core/canonical-reporting-capabilities.json, static/schemas/source/core/reporting-capabilities.json, static/schemas/source/media-buy/get-media-buy-delivery-request.json, static/schemas/source/media-buy/get-media-buy-delivery-response.json.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/canonical-reporting-capabilities.json (modified) matches static/schemas/source/**; static/schemas/source/core/reporting-capabilities.json (modified) matches static/schemas/source/**; 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) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

Panel review of the reporting stack found by_format would be the one
negotiated breakdown without the ratified sort contract once the
sort-direction PR merges: sort_by descending-only and no applied-sort
echo, making a spend fallback invisible. Add sort_direction and the
by_format_sorted_by / by_format_sort_direction echo with the same
row-grain fallback, direction-MUST, and nulls-last semantics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
"description": "Metric to sort breakdown rows by, in `sort_direction` order (descending by default). Falls back to 'spend' when the seller does not report the requested metric at this breakdown's row grain; on fallback the sort direction resets to 'desc'. Rows lacking a value for the applied sort metric order last regardless of direction. The applied sort is echoed in the response.",
"default": "spend"
},
"sort_direction": {

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.

Medium: sort_direction and the two MUST-return response echoes (by_format_sorted_by, by_format_sort_direction) aren't reflected in the task reference. docs/media-buy/task-reference/get_media_buy_delivery.mdx still enumerates the accepted sort params as "optional limit and sort_by" only, and its response section describes by_format without the sort echo. The changeset advertises "the full sort contract," but an implementer reading the normative doc learns neither that sort is directional nor that two echo fields MUST be emitted whenever by_format is present. Schema and docs drift on a normative wire contract; document sort_direction and the echo fields (including the fallback/nulls-last semantics) on the reference page.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0e8b2b1: the task reference now documents sort_direction (params table + Dimension Breakdowns paragraph with the row-grain fallback, direction MUST, and nulls-last rules) and the by_format_sorted_by/by_format_sort_direction echo in the by_format response section. Same commit merges main and refreshes the mcp-schema-analysis pins on the merged tree (the new sort fields moved the definition counts, which was the Canonical tests (1/4) failure).

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ladon verdict: Escalate to human review

Escalate — gated schema-source files touched without an APPROVED review.

This PR modifies files under static/schemas/source/**, which sit behind the repo's hard, deterministic approval gate (gated_paths: true). The current GitHub review decision is REVIEW_REQUIRED, not APPROVED, so decision-table row 2 fires: Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

Gated files requiring human/CODEOWNERS approval:

  • static/schemas/source/core/canonical-reporting-capabilities.json
  • static/schemas/source/core/reporting-capabilities.json
  • static/schemas/source/media-buy/get-media-buy-delivery-request.json
  • static/schemas/source/media-buy/get-media-buy-delivery-response.json

Medium findings

  • static/schemas/source/media-buy/get-media-buy-delivery-request.json:253 — sort_direction and by_format sort echo fields (by_format_sorted_by/by_format_sort_direction) not documented in the task reference (schema↔docs coherence gap).

Note on the change itself: the reviewer found the enum $refs resolve, validation tests cover accept/reject paths, and the minor changeset is correctly scoped for an additive extension — no critical/high spec-drift, changeset, oneOf, or immutability issues. This is a net-neutral update from the prior escalate (which had no findings); the sole open item is the doc-coherence medium finding. The escalation persists purely because the gated-path approval gate is still unsatisfied, not because of the medium finding. A human/CODEOWNERS approval lifts the gate and the doc gap should be closed before merge.

Medium findings

  • static/schemas/source/media-buy/get-media-buy-delivery-request.json:253 — sort_direction and by_format sort echoes undocumented in task reference

Why human review

  • Touches gated schema-source files under static/schemas/source/** with review_decision=REVIEW_REQUIRED (not APPROVED) — human/CODEOWNERS approval required before merge (decision-table row 2).
  • Gated files: static/schemas/source/core/canonical-reporting-capabilities.json, static/schemas/source/core/reporting-capabilities.json, static/schemas/source/media-buy/get-media-buy-delivery-request.json, static/schemas/source/media-buy/get-media-buy-delivery-response.json.
  • Open medium: sort_direction and by_format sort echo fields undocumented in the task reference (schema↔docs coherence gap).
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/canonical-reporting-capabilities.json (modified) matches static/schemas/source/**; static/schemas/source/core/reporting-capabilities.json (modified) matches static/schemas/source/**; 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) matches static/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

bokelley and others added 2 commits August 17, 2026 11:09
…pins

Ladon review flagged that the format sort_direction and applied-sort
echo landed in the schema without task-reference documentation. Also
merges main and re-measures the mcp schema-analysis pins on the merged
tree (the three new sort fields and main's own additions moved the
definition counts).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verification review: the format dimension's sort_direction description
omitted the canonical bottom-N sentence used at every other site,
leaving it the one divergent copy after the sort-contract PR merges.

Also raises the precommit server-unit timeout budget from 240s to 420s:
the full suite (438 files after the @adcp/sdk 13.0.0 merge) passes in
~310s serially, so the stale budget was killing green runs — the guard
remains, sized to the suite it wraps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant