feat(reporting): sort direction and applied-sort echo for breakdowns - #6622
feat(reporting): sort direction and applied-sort echo for breakdowns#6622bokelley wants to merge 3 commits into
Conversation
Breakdown sorting was descending-only with a silent fallback to spend when the seller does not report the requested metric — an agentic buyer asking for top geos by completion_rate gets spend-ordered rows and reasons over them as completion-ordered. Add sort_direction (asc/desc, default desc) to the six sortable dimensions, enabling bottom-N queries that cannot be recovered from a truncated descending pull, and a per-breakdown applied-sort echo (by_X_sorted_by / by_X_sort_direction, MUST whenever the breakdown is present) so the fallback is visible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema paths require human/CODEOWNERS approval.
This is a clean, additive delivery-reporting change: it adds an optional sort_direction (asc/desc, default desc) to six sortable breakdown dimensions plus a per-breakdown applied-sort echo, with docs updated in lockstep (no schema↔docs drift), a correctly-scoped minor changeset for a non-breaking additive change on main's beta line, and a new sort-contract test wired into test:schemas covering all six dimensions. The reviewer surfaced no critical/high/medium findings — only one Low nit (redundant "(descending)" wording outside the diff).
However, this PR modifies files under static/schemas/source/**, which the repo declares as a hard, deterministic approval gate (gated_paths: true). Because review_decision is not APPROVED (unknown/none), decision-table row 2 fires: the outcome MUST be escalate, not approve, regardless of the clean diff. Human/CODEOWNERS review of the schema change is required to satisfy the gate.
No blocking findings exist; the escalation is purely the unsatisfied gated-paths gate. Once a human/CODEOWNERS approval lands, this PR can fall through to a normal approve.
Escalation reasons
- Modifies gated schema files
static/schemas/source/media-buy/get-media-buy-delivery-request.jsonandget-media-buy-delivery-response.jsonunderstatic/schemas/source/**;review_decisionis notAPPROVED. Human/CODEOWNERS review required.
Why human review
- Modifies gated schema file static/schemas/source/media-buy/get-media-buy-delivery-request.json under static/schemas/source/** while review_decision is not APPROVED — human/CODEOWNERS approval required
- Modifies gated schema file static/schemas/source/media-buy/get-media-buy-delivery-response.json under static/schemas/source/** while review_decision is not APPROVED — human/CODEOWNERS approval 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
Review findings: the six sort_by descriptions still claimed descending- only next to the new sort_direction field; the spend fallback had no grain (package vs row) so two conformant sellers could return disjoint top-N sets; rows lacking the sort metric had undefined order (NULLS FIRST vs LAST) exactly where bottom-N queries live; and direction had no honor rule, letting a fallback gut the asc use case. Canonical rule now at every site: fallback triggers only when the metric is not reported at the breakdown's row grain, direction resets to desc on metric fallback and is otherwise a MUST, rows lacking the applied metric order last. Sortable-dimension invariants are now derived from the schema instead of a hardcoded list, so a future sortable dimension cannot ship without the echo contract. Also fixes the stale descending claim in managing-response-size.mdx. 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 paths modified without recorded approval.
This PR modifies two files under the hard, non-overridable approval gate static/schemas/source/**:
static/schemas/source/media-buy/get-media-buy-delivery-request.jsonstatic/schemas/source/media-buy/get-media-buy-delivery-response.json
gated_paths: true and review_decision is unknown/none (not APPROVED), so decision rule row 2 forces escalation. Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
The reviewer reports a clean, clarification-only delta: container-subsumption prose propagated consistently across schemas and docs, the by_X_sort_direction echoes tightened to be consistent with the by_X_sorted_by siblings, a scoped MUST on viewability.standard, and two hardened tests. No critical/high/medium findings survive, and schema↔docs coherence holds. A changeset is present.
Escalation reasons
- Modifies gated schema file
static/schemas/source/media-buy/get-media-buy-delivery-request.jsonunderstatic/schemas/source/**whilereview_decisionis not APPROVED — human/CODEOWNERS approval required. - Modifies gated schema file
static/schemas/source/media-buy/get-media-buy-delivery-response.jsonunderstatic/schemas/source/**whilereview_decisionis not APPROVED — human/CODEOWNERS approval required.
Why human review
- Modifies gated schema file static/schemas/source/media-buy/get-media-buy-delivery-request.json under static/schemas/source/** while review_decision is not APPROVED — human/CODEOWNERS approval required
- Modifies gated schema file static/schemas/source/media-buy/get-media-buy-delivery-response.json under static/schemas/source/** while review_decision is not APPROVED — human/CODEOWNERS approval 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.
1 participant
Summary
Stacked on #6618 (base retargets to
mainautomatically when it merges). Companion to the leaf-identity PR: it makes the sort contract honest.Two gaps in breakdown sorting today:
limitit cannot be recovered client-side (top-25 descending says nothing about the tail; the geo default limit is 25 against potentially thousands of postal rows).sort_direction(asc/desc, defaultdesc) is added to the six sortable dimensions, reusing the existingsort-directionenum already used bylist_creativesandlist_tasks.sort_byfalls back tospendwhen the seller doesn't report the requested metric, with no indication in the response — the worst failure class for agentic buyers: rows that look ordered by the requested metric but aren't. Each sortable breakdown now has an applied-sort echo (by_X_sorted_by/by_X_sort_direction), MUST whenever the breakdown array is present, following theby_X_truncatedsibling-flag pattern and theattribution_windowecho precedent.spotis untouched (fixedaired_atascending, no sort fields — asserted in the contract test).Backward compatible: both request fields optional with defaults preserving current behavior; echo fields are new response fields with MUST force only on 3.2 sellers.
Not in this PR
creative/keyword/catalog_itembreakdowns have nosort_by/limitnegotiation at all today (they're returned automatically) — giving them sort knobs changes their contract shape and deserves its own RFC rather than a rider here.🤖 Generated with Claude Code