feat(media-buy): structure proposal refinement negotiation - #6547
Conversation
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema paths require human/CODEOWNERS approval, plus a rename on the protocol surface.
No blocking (critical/high) findings. Schema↔docs coherence, oneOf discriminators, and changeset scope all check out per the reviewer.
Escalation reasons
gated_paths: truewithreview_decision: REVIEW_REQUIRED: this PR modifies/adds 10 files understatic/schemas/source/**(includingindex.json,product-filters.json,product-offer-filters.json,proposal-refinement.json,refine-proposals-response.json,get-adcp-capabilities-response.json). Hard approval gate — human/CODEOWNERS approval required before merge.- High-risk modified schema files combined with medium findings (row 5) independently trigger escalation.
- The diff renames
instructions→askon the refine_proposals surface; reviewer notes it is unreleased, but a human should confirm the breaking-class classification and changeset bump before this ships.
Medium findings
- static/schemas/source/media-buy/proposal-refinement.json:29 —
constraints.total_budgethas no defined outcome whencommercial_terms.total_budgetis absent - static/schemas/source/media-buy/proposal-refinement.json:45 —
alternatives.countis an unbounded response multiplier - server/tests/unit/training-agent.test.ts:13663 — batch-wide
UNSUPPORTED_FEATUREscope is untested and undocumented
Nits (non-blocking)
- docs/media-buy/task-reference/refine_proposals.mdx:15 — points at mutable
/schemas/latest/behind a hand-maintained TODO - static/schemas/source/media-buy/proposal-budget-constraint.json:1 — duplicates
budget-range.jsonfield-for-field; consider$ref
Medium findings
- static/schemas/source/media-buy/proposal-refinement.json:29 — constraints.total_budget has no defined outcome when commercial_terms.total_budget is absent
- static/schemas/source/media-buy/proposal-refinement.json:45 — alternatives.count is an unbounded response multiplier
- server/tests/unit/training-agent.test.ts:13663 — Batch-wide UNSUPPORTED_FEATURE scope is untested and undocumented
Why human review
- Gated paths: 10 files under
static/schemas/source/**changed whilereview_decisionis REVIEW_REQUIRED — human/CODEOWNERS approval required. - High-risk modified schema files (
proposal-refinement.json,refine-proposals-response.json,get-adcp-capabilities-response.json,index.json,product-filters.json,product-offer-filters.json) carry medium findings. - Field rename
instructions→askon the refine_proposals protocol surface needs human confirmation of breaking-class classification and changeset bump. - This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/budget-range.json (added) matches
static/schemas/source/**; static/schemas/source/core/product-change-map.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/product-filters.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/product-offer-filters.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/proposal-refinement-reason.json (added) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-budget-constraint.json (added) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-refinement.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/refine-proposals-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matchesstatic/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.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema paths require human/CODEOWNERS approval, plus a breaking-class protocol change.
Why escalate (decision table):
- Row 2 fires:
gated_paths: true(10 files understatic/schemas/source/**) ANDreview_decisionis notAPPROVED(unknown/none). The hard approval gate is not satisfied — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded. - Additionally, the repo's "Breaking-class changes are never auto-approved" hard gate applies: this PR renames/removes fields on the unreleased 3.2 proposal-refinement negotiation surface (e.g.
instructions→ask). Ratification is a human act and requires escalation unlessreview_decisionisAPPROVED.
No critical/high findings surfaced this pass (row 1 does not fire). Schema↔docs coherence holds (reason_code enum, capability supported_dimensions enum, lifecycle_tools gate all match docs), the changeset is present and correctly scoped as minor for the unreleased compact surface, and new response-validator branches are well tested.
Medium findings
static/schemas/source/media-buy/proposal-refinement.json:30— constraints.total_budget asserts a comparison against optional commercial_terms.total_budget with no defined outcome when that field is absent (persists from prior run).
Iteration credit: Two prior mediums were addressed — alternatives.count unbounded multiplier and the untested batch-wide UNSUPPORTED_FEATURE scope. The remaining single medium is a net improvement over the prior pass. The escalation here is driven entirely by the unsatisfied path/breaking-class gates, not by unresolved blocking findings.
Human/CODEOWNERS approval on the gated schema files (and confirmation of the field-rename breaking-class classification) will lift the gate.
Medium findings
- static/schemas/source/media-buy/proposal-refinement.json:30 — constraints.total_budget has no defined outcome when commercial_terms.total_budget is absent
Why human review
- Gated paths: 10 files under
static/schemas/source/**changed whilereview_decisionis not APPROVED — human/CODEOWNERS approval required. - Breaking-class protocol change: field rename
instructions→askand removals on the proposal-refinement / refine_proposals surface — ratification is a human act; escalate unless review_decision is APPROVED. - Remaining medium: static/schemas/source/media-buy/proposal-refinement.json:30 — constraints.total_budget has no defined outcome when commercial_terms.total_budget is absent.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/budget-range.json (added) matches
static/schemas/source/**; static/schemas/source/core/product-change-map.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/product-offer-filters.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/proposal-refinement-reason.json (added) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-budget-constraint.json (added) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-refinement.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/refine-proposals-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard approval gate on static/schemas/source/** is unsatisfied, plus a Breaking-class protocol change requires human ratification.
This PR restructures the (unreleased) refine_proposals surface: instructions/criteria → typed constraints/product_changes/alternatives/ask, adds a proposal_refinement_reason enum and a proposal_refinement capability block. Schema, docs, and server validation move in lockstep; the changeset is a correct minor and no released dist/** artifact is touched. Prior findings #2 (unbounded alternatives) and #3 (untested batch-wide UNSUPPORTED_FEATURE) have been addressed via max_alternatives and new pre-mutation rejection tests — good iteration.
Why this cannot auto-approve:
- Gated paths (decision row 2): 10 files under
static/schemas/source/**are changed andreview_decisionis notAPPROVED(unknown/REVIEW_REQUIRED). This is a deterministic, non-overridable gate — human/CODEOWNERS approval must be recorded first. - Breaking-class hard gate: the
instructions→askfield rename on the refine_proposals protocol surface is a breaking-class wire change; ratification is a human act and is never auto-approved unlessreview_decisionisAPPROVED.
Remaining medium finding (does not itself block, surfaced for the human reviewer):
- static/schemas/source/media-buy/proposal-refinement.json:30 —
constraints.total_budgethas no defined outcome whencommercial_terms.total_budgetis absent.
Prior escalation stands: fresh medium finding still present and both hard gates remain unsatisfied.
Medium findings
- static/schemas/source/media-buy/proposal-refinement.json:30 — constraints.total_budget has no defined outcome when commercial_terms.total_budget is absent
Why human review
- Gated paths: 10 files under
static/schemas/source/**changed whilereview_decisionis not APPROVED (unknown/REVIEW_REQUIRED) — human/CODEOWNERS approval required before merge. - Breaking-class protocol change: field rename
instructions→askon the refine_proposals surface requires human ratification per the constitution's hard gate; never auto-approved unless review_decision is APPROVED. - Open medium finding: proposal-refinement.json:30 — constraints.total_budget has undefined outcome when commercial_terms.total_budget is absent; warrants a human eye alongside the gated schema review.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/budget-range.json (added) matches
static/schemas/source/**; static/schemas/source/core/product-change-map.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/product-offer-filters.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/proposal-refinement-reason.json (added) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-budget-constraint.json (added) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-refinement.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/refine-proposals-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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.
|
Reviewed at First — thank you for this. It's a faithful implementation of every position from the RFC discussion, and several parts go beyond what was asked: the reference validator enforces what JSON Schema can't ( Two asks before merge, two smaller notes: 1. 2. No disposition for 3. (note) The 4. (note) The dimension-rejection detail payload the training agent emits ( |
|
Two interop gaps and two clean-up notes — all legitimate. Synthesis and recommended path for each: Ask 1 — Recommending option (a): add Option (a) — one schema edit: // static/schemas/source/protocol/get-adcp-capabilities-response.json
"enum": ["total_budget", "product_selection", "alternatives"]
+ "enum": ["total_budget", "product_selection", "alternatives", "criteria"]Sellers that support criteria refinement advertise it; those that don't omit it and the pre-flight signal is deterministic. Option (b) — normative prose, no enum change: // refine_proposals.mdx — proposal_refinement capability section
+ Criteria support in refinements is not declared in supported_dimensions;
+ a seller's acceptance of criteria fields follows from its get_products
+ targeting capability surface.Valid, but buyers need two lookups to pre-flight a criteria refinement. @bokelley — confirm (a) or (b) and I'll push the edit. Ask 2 — No disposition for Recommending task-level Option A — task-level {
"code": "VALIDATION_ERROR",
"message": "alternatives.count (6) exceeds seller's max_alternatives (4)",
"field": "refinements.0.alternatives.count"
}Option B — bless {
"result": "partial",
"proposals": ["/* up to max_alternatives drafts */"],
"reason": "alternatives_unavailable"
}Also acceptable but buyers must reconstruct from context whether the shortfall was a ceiling or a supply issue; SDK retry strategies would need to handle both readings of the same reason code. @bokelley — confirm A or B and I'll add the normative sentence to the Note 3 — schema cross-reference.
Can push this as a follow-up commit — mechanical, no normative impact. Note 4 — detail payload. The If you'd prefer to mark it explicitly implementation-specific instead, a one-sentence prose note in Either way, can push this commit alongside note 3. Holding on asks 1 and 2 pending your call — those involve normative schema/doc edits that should be your decision. Notes 3 and 4 are ready to push on your go-ahead. Generated by Claude Code |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema paths require human/CODEOWNERS approval and a breaking-class protocol change is present.
The developer addressed the prior blocking medium finding: proposal-refinement.json now defines the outcome when commercial_terms.total_budget is absent (partial/unable + constraint_unsatisfiable + unsatisfied_constraints:['total_budget'], never revised), enforced in source-schema.ts via proposalSatisfiesBudgetConstraint with full test coverage. The reviewer surfaced no new Critical/High/Medium findings in the delta, and schema↔docs coherence, the max_alternatives VALIDATION_ERROR contract, and the additive wire change (minor changeset) all check out. This is a genuine net improvement over the prior run.
However, two hard gates still stand and neither is satisfied:
-
Gated paths (row 2): 10 files under
static/schemas/source/**are changed andreview_decisionis not APPROVED (unknown/none). This is a deterministic, non-overridable approval gate — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded. -
Breaking-class protocol change (prose hard gate): the
instructions→askfield rename on the refine_proposals surface is a Breaking-class change per the constitution. Ratification is a human act and is never auto-approved unlessreview_decisionis APPROVED.
Both gates lift the moment a human/CODEOWNERS approval is recorded on the PR. No blocking code-level findings remain; the sole obstacle to merge is the required human ratification.
Why human review
- Gated paths: 10 files under static/schemas/source/** changed while review_decision is not APPROVED (unknown/none) — human/CODEOWNERS approval required before merge.
- Breaking-class protocol change: field rename
instructions→askon the refine_proposals surface requires human ratification per the constitution's hard gate; never auto-approved unless review_decision is APPROVED. - This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/budget-range.json (added) matches
static/schemas/source/**; static/schemas/source/core/product-change-map.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/product-offer-filters.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/proposal-refinement-reason.json (added) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-budget-constraint.json (added) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-refinement.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/refine-proposals-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated-path approval gate unsatisfied and breaking-class protocol change pending human ratification.
This PR adds hard upper bounds (maximum: 10 on alternatives.count / max_alternatives, maxItems: 25 on refinements) to the refine_proposals surface, with schema↔docs coherence maintained across refine_proposals.mdx and get_adcp_capabilities.mdx and new tests covering schema-level rejection plus server-side atomic batch rejection. The reviewer surfaced no critical/high/medium findings — the diff is mechanically clean.
However, the decision cannot auto-approve:
Escalation reasons
- Gated paths: 10 files under
static/schemas/source/**changed whilereview_decisionis not APPROVED (unknown/none). Human/CODEOWNERS approval is required before merge (decision rule row 2 — hard, non-overridable gate). - Breaking-class protocol change carried forward from the prior review: the
instructions→askfield rename on the refine_proposals surface is a breaking-class change per the constitution's hard gate, never auto-approved unless review_decision is APPROVED.
The prior decision was escalate for the same gated-path/breaking-class reasons; those conditions persist in this run (files still touch the gated surface, review_decision still not APPROVED). Once a human/CODEOWNERS approval is recorded, this clean diff can fall through to approval.
Why human review
- Gated paths: 10 files under static/schemas/source/** changed while review_decision is not APPROVED (unknown/none) — human/CODEOWNERS approval required before merge.
- Breaking-class protocol change: field rename instructions → ask on the refine_proposals surface requires human ratification per the constitution's hard gate; never auto-approved unless review_decision is APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/budget-range.json (added) matches
static/schemas/source/**; static/schemas/source/core/product-change-map.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/product-offer-filters.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/proposal-refinement-reason.json (added) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-budget-constraint.json (added) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-refinement.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/refine-proposals-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/refine-proposals-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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.
|
All review feedback is addressed in |
Red-team and market-scan follow-ups on refine_proposals: - Typed cpm, impressions, and flight hard constraints, capability-gated like total_budget; unsatisfied_constraints opens to free string keys - Rename capability dimension product_selection to product_changes so dimensions match their request fields - constraint_unsatisfiable precedence over other reason codes; typed failures never masquerade as commercially_declined - hold_unavailable and batch_aborted reason codes plus double-finalize INVALID_STATE rule and seller hold-policy guidance - Undeclared-dimension rejection upgraded to MUST with an error-details/unsupported-refinement-dimension.json details shape - Every refinement successor carries parent_proposal_id equal to its source; response schema and validator enforce lineage - Validator recomputes terms_digest from JCS-canonicalized commercial_terms and checks alternative distinctness on terms, not digest strings - Normative text: partial drafts satisfy every unlisted constraint, only commercial_terms is contractual, ask is untrusted input, price-probe friction guidance, legacy budget_range soft-vs-hard facade warning, response example with lineage Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Ran a four-way red team (protocol, market fit, adversarial security, implementor DX) plus a market scan against IAB Tech Lab's AAMP before finalizing. Contract-breaking-if-released-later (schema):
Semantics that two conformant sellers would have implemented differently:
Trust and abuse (doc-normative):
AAMP scan result: AAMP 2.3's published negotiation surface types exactly one dimension ( 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated paths require human/CODEOWNERS approval, and a breaking-class protocol change is in scope.
This PR extends the refine_proposals surface with new hard constraints (cpm/impressions/flight), negotiation lineage via a required parent_proposal_id, a buyer-verifiable terms_digest, and new reason codes. Schema↔docs coherence, changeset scope/type, oneOf discriminators, and reference validators all check out — the reviewer found no blocking or Medium findings. That's a clean diff, but it does not clear the hard gates.
Escalation reasons
- Gated paths: 13 files under
static/schemas/source/**changed whilereview_decisionisREVIEW_REQUIRED(notAPPROVED). This is a deterministic hard approval gate — human/CODEOWNERS approval is required before merge regardless of how clean the diff is. Affected files includestatic/schemas/source/media-buy/refine-proposals-request.json,refine-proposals-response.json,proposal-refinement.json,core/canonical-proposal.json,enums/error-code.json, and the new added schemas. - Breaking-class protocol change: introducing a required
parent_proposal_id(negotiation lineage) and new required commercial-terms structure on the refine_proposals surface is a breaking-class change under the constitution's decision classes. Per the hard gate, breaking-class changes are never auto-approved — ratification is a human act — unlessreview_decisionisAPPROVED.
Prior decision was also escalate for the same gates; those conditions still hold (row 2). No blocking findings otherwise. Once a human/CODEOWNERS APPROVED review is recorded, this can fall through to the normal table.
Why human review
- Gated paths: 13 files under static/schemas/source/** changed while review_decision is REVIEW_REQUIRED (not APPROVED) — human/CODEOWNERS approval required before merge.
- Breaking-class protocol change on the refine_proposals surface (required parent_proposal_id negotiation lineage + new required commercial-terms structure) — never auto-approved unless review_decision is APPROVED per the constitution's hard gate.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/core/budget-range.json (added) matches
static/schemas/source/**; static/schemas/source/core/canonical-proposal.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/product-change-map.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/product-offer-filters.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/proposal-refinement-reason.json (added) matchesstatic/schemas/source/**; static/schemas/source/error-details/unsupported-refinement-dimension.json (added) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-budget-constraint.json (added) matchesstatic/schemas/source/**; static/schemas/source/media-buy/proposal-refinement.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/refine-proposals-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/refine-proposals-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matchesstatic/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.
|
Re-reviewed at
The hardening delta holds up on inspection: |
Summary
instructionsfield with the shared negotiation nounasktotal_budget,cpm,impressions, andflight— plus product include/omit changes and requested alternative countshold_unavailableandbatch_abortedfor atomic finalize failurestotal_budget,cpm,impressions,flight,product_changes,alternatives,criteria) separately from free-text negotiation competence; undeclared dimensions MUST fail task-level pre-mutation with a registerederror-detailsshapeparent_proposal_idlineage on every refinement successor and buyer-recomputableterms_digestsemantics (RFC 8785 JCS + sha256), with alternative distinctness defined oncommercial_termsaskis untrusted input, deterministic constraint responses form a price oracle worth rate-limiting, holds are seller-gated with seller-chosen expiry, and onlycommercial_termsis contractualrefine_proposalsand legacyget_productsrefinement, including the soft-filter vs hard-constraintbudget_rangeinversion adapters must convertWhy
The compact proposal lifecycle introduced by #6115 had only free-text revision semantics. Deterministic buyers could not express hard boundaries or distinguish commercial refusal from unsupported or uninterpreted requests. This implements the structured negotiation contract discussed in #6450 while preserving immutable proposal snapshots and keeping preferences and diversity strategy in free text.
A four-way pre-merge red team (protocol, market fit, adversarial security, implementor DX) then hardened the contract while it is still unreleased: rate and volume are the top negotiated dimensions on real desks and needed typed paths; reason-code selection was ambiguous enough that two conformant sellers would diverge; and the
product_selectioncapability name would have shipped a permanent mismatch with its request field. A parallel scan of IAB Tech Lab's AAMP confirmed no shape changes are needed for future interop — its published negotiation surface types a singlebuyer_pricedimension that maps into this contract as a degenerate case.Closes #6450.
Validation