Skip to content

Commit c41d4a8

Browse files
bokelleyclaude
andauthored
feat(decisioning): PgProposalStore + framework package derivation (#727) (#732)
Two patterns moved upstream from the salesagent fork (bokelley/salesagent#390): **`PgProposalStore`** — Postgres-backed `ProposalStore` Protocol implementation. Mirrors `PgTaskRegistry` / `PgBackend` patterns: pool injection, `is_durable=True` class var, `create_schema()` idempotent bootstrap, `migration_sql(table_name=...)` classmethod for Alembic adopters, ASCII-safe identifier guards. The proposal lifecycle CAS (`COMMITTED → CONSUMING`) uses `SELECT ... FOR UPDATE` inside a transaction; `commit()` does the same to close the previous SELECT-then-UPDATE race. **Framework package derivation** — opt-in via `ProposalCapabilities.derive_packages_from_allocations=True` OR by implementing `ProposalManager.derive_packages(...)`. When enabled and the buyer omits `packages[]` on `create_media_buy(proposal_id=...)`, the framework mutates `req.packages` from the proposal's `allocations[]` via percentage math. `start_time` / `end_time` propagate when present. Standalone `derive_packages_from_proposal()` helper for off-path workflows. Security: closes the cross-tenant write surface on `commit()`, `mark_consumed()`, `discard()` by adding required `expected_account_id` to the Protocol — without it the PG store's `(account_id, proposal_id)` PK could be hit on the wrong tenant's row via colliding `proposal_id`s. Surfaces `pricing_option_id`-missing as a seller-side `INTERNAL_ERROR` (was incorrectly `INVALID_REQUEST` to the buyer); framework auto-picks the product's single pricing_option when unambiguous. Reviewed by code-reviewer, security-reviewer, ad-tech-protocol-expert, adtech-product-expert, python-expert, dx-expert; all Must-Fix items addressed. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 884873a commit c41d4a8

15 files changed

Lines changed: 2840 additions & 72 deletions

examples/sales_proposal_mode_seller/src/proposal_manager.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,19 @@ def _draft_proposal_payload(allocations: dict[str, float] | None = None) -> dict
121121
Required fields: ``proposal_id``, ``name``, ``allocations``. Each
122122
allocation entry carries ``product_id`` + ``allocation_percentage``
123123
(0-100). Percentages must sum to 100.
124+
125+
The ``pricing_option_id`` on each allocation feeds the framework's
126+
package derivation (per #727) — when the buyer calls
127+
``create_media_buy(proposal_id=..., total_budget=...)`` with no
128+
explicit ``packages[]``, the framework derives one package per
129+
allocation using ``pricing_option_id``.
124130
"""
125131
alloc = allocations or {"ctv-premium-q2": 60.0, "display-run-q2": 40.0}
132+
# Map each product to the canonical pricing_option_id from _PRODUCTS.
133+
pricing_option_ids = {
134+
"ctv-premium-q2": "po-ctv-cpm",
135+
"display-run-q2": "po-display-cpm",
136+
}
126137
return {
127138
"proposal_id": PROPOSAL_ID,
128139
"name": "Balanced Reach Q2 — CTV-led",
@@ -132,6 +143,7 @@ def _draft_proposal_payload(allocations: dict[str, float] | None = None) -> dict
132143
{
133144
"product_id": pid,
134145
"allocation_percentage": pct,
146+
"pricing_option_id": pricing_option_ids.get(pid, "po-default"),
135147
"rationale": f"Indicative {pct:.0f}% allocation to {pid}.",
136148
}
137149
for pid, pct in alloc.items()
@@ -154,6 +166,9 @@ class ProposalModeProposalManager:
154166
# 60s grace absorbs clock skew between the seller's
155167
# expires_at and the buyer's create_media_buy call.
156168
expires_at_grace_seconds=60,
169+
# #727: let the framework derive packages from the proposal's
170+
# allocations when the buyer omits packages[] on create_media_buy.
171+
derive_packages_from_allocations=True,
157172
)
158173

159174
async def get_products(

src/adcp/decisioning/__init__.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ def create_media_buy(
8484
AuthInfo,
8585
RequestContext,
8686
)
87+
from adcp.decisioning.derive_packages import derive_packages_from_proposal
8788
from adcp.decisioning.dispatch import validate_platform
8889
from adcp.decisioning.errors import (
8990
AccountNotFoundError,
@@ -260,7 +261,11 @@ def create_media_buy(
260261
# ``Pg*`` convention shared with PgReplayStore / PgBuyerAgentRegistry /
261262
# PgWebhookDeliverySupervisor.
262263
try:
263-
from adcp.decisioning.pg import PgTaskRegistry, PostgresTaskRegistry # noqa: F401
264+
from adcp.decisioning.pg import ( # noqa: F401
265+
PgProposalStore,
266+
PgTaskRegistry,
267+
PostgresTaskRegistry,
268+
)
264269
except ImportError: # pragma: no cover — exercised by the [pg] extra tests
265270
from typing import ClassVar as _ClassVar
266271

@@ -283,6 +288,22 @@ def __init__(self, *args: object, **kwargs: object) -> None:
283288
# Deprecated alias preserved through 4.4.x.
284289
PostgresTaskRegistry: type[PgTaskRegistry] = PgTaskRegistry # type: ignore[no-redef]
285290

291+
class PgProposalStore: # type: ignore[no-redef]
292+
"""Stub raised when ``adcp[pg]`` isn't installed.
293+
294+
Attempting to instantiate raises :class:`ImportError` with the
295+
install-hint text from :mod:`adcp.decisioning.pg.proposal_store`.
296+
"""
297+
298+
is_durable: _ClassVar[bool] = True
299+
300+
def __init__(self, *args: object, **kwargs: object) -> None:
301+
raise ImportError(
302+
"PgProposalStore requires psycopg3 and psycopg-pool. "
303+
"Install the 'pg' extra: `pip install 'adcp[pg]'` "
304+
"(Poetry: `poetry add 'adcp[pg]'`)."
305+
)
306+
286307

287308
__all__ = [
288309
"Account",
@@ -338,6 +359,7 @@ def __init__(self, *args: object, **kwargs: object) -> None:
338359
"NoAuth",
339360
"OAuthCredential",
340361
"PermissionDeniedError",
362+
"PgProposalStore",
341363
"PgTaskRegistry",
342364
"LazyPlatformRouter",
343365
"PlatformRouter",
@@ -415,6 +437,7 @@ def __init__(self, *args: object, **kwargs: object) -> None:
415437
"create_tenant_store",
416438
"create_translation_map",
417439
"create_upstream_http_client",
440+
"derive_packages_from_proposal",
418441
"require_account_match",
419442
"require_advertiser_match",
420443
"require_org_scope",
Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
"""Framework-level package derivation from proposal allocations.
2+
3+
When a buyer calls ``create_media_buy(proposal_id=..., total_budget=...)``
4+
without supplying ``packages[]``, the spec lets the seller derive
5+
packages from the committed proposal's ``allocations[]`` array via
6+
percentage math. Every production adopter writes essentially the same
7+
loop:
8+
9+
.. code-block:: python
10+
11+
for allocation in proposal_payload["allocations"]:
12+
pkg_budget = total_budget.amount * (allocation["allocation_percentage"] / 100.0)
13+
packages.append(PackageRequest(
14+
product_id=allocation["product_id"],
15+
budget=pkg_budget,
16+
pricing_option_id=allocation["pricing_option_id"],
17+
))
18+
19+
The framework auto-invokes :func:`derive_packages_from_proposal` from
20+
:func:`adcp.decisioning.proposal_dispatch.maybe_hydrate_recipes_for_create_media_buy`
21+
when ``req.packages`` is empty AND the proposal has ``allocations[]``,
22+
so seller adapters see a normal ``create_media_buy`` with populated
23+
packages — they never need to write the loop.
24+
25+
Adopters with custom derivation logic (auction pricing options needing
26+
``bid_price``, multi-currency proposals, capability-overlap filtering)
27+
override the behavior by implementing
28+
:meth:`ProposalManager.derive_packages` on their manager subclass; the
29+
framework dispatches there in preference to the built-in even-split
30+
derivation.
31+
32+
Adopters with workflows outside the auto-injection path (admin-side
33+
inspection tools, draft-preview UIs, off-path replay) can call this
34+
helper directly.
35+
"""
36+
37+
from __future__ import annotations
38+
39+
import logging
40+
from collections.abc import Mapping
41+
from typing import TYPE_CHECKING, Any
42+
43+
from adcp.decisioning.types import AdcpError
44+
45+
logger = logging.getLogger("adcp.decisioning.derive_packages")
46+
47+
if TYPE_CHECKING:
48+
from adcp.decisioning.recipe import Recipe
49+
from adcp.types import PackageRequest
50+
51+
52+
def derive_packages_from_proposal(
53+
proposal_payload: Mapping[str, Any],
54+
total_budget: Any,
55+
*,
56+
recipes: Mapping[str, Recipe] | None = None,
57+
) -> list[PackageRequest]:
58+
"""Derive a ``list[PackageRequest]`` from a committed proposal's allocations.
59+
60+
Default even-percentage distribution per ``ProductAllocation``:
61+
62+
* ``budget = total_budget.amount * allocation.allocation_percentage / 100``
63+
* ``product_id``, ``pricing_option_id`` copied from the allocation
64+
* ``start_time`` / ``end_time`` copied when present (per-flight
65+
scheduling per spec)
66+
* ``pacing``, ``targeting_overlay``, ``bid_price`` etc. are NOT
67+
derived — adopters whose proposals carry these per allocation
68+
should override :meth:`ProposalManager.derive_packages` and emit
69+
them explicitly.
70+
71+
**Currency.** Per spec, ``PackageRequest.budget`` is in
72+
``total_budget.currency`` (the media-buy-level unit). This helper
73+
treats ``allocation_percentage`` as a unit-less multiplier; it
74+
does not inspect or compare any currency carried by the proposal's
75+
products' pricing_options. Multi-currency adopters should override
76+
:meth:`ProposalManager.derive_packages` and apply their own FX
77+
conversion before emitting packages.
78+
79+
:param proposal_payload: The committed proposal's wire payload
80+
(typically ``ProposalRecord.proposal_payload``). Must contain
81+
an ``allocations[]`` array; absent / empty allocations are
82+
treated as a buyer error and surfaced as ``INVALID_REQUEST``.
83+
:param total_budget: The ``TotalBudget`` (or dict-shaped equivalent
84+
with ``amount`` / ``currency`` keys) from the buyer's
85+
``CreateMediaBuyRequest``. Must be non-None — the buyer must
86+
supply ``total_budget`` whenever they omit ``packages``.
87+
:param recipes: Unused by the built-in derivation. Threaded through
88+
so the signature matches :meth:`ProposalManager.derive_packages`
89+
for adopters who delegate to this helper from inside their
90+
override.
91+
92+
:raises AdcpError: ``INVALID_REQUEST`` when the proposal payload
93+
lacks ``allocations[]``, when an allocation is missing required
94+
fields (``product_id``, ``pricing_option_id``,
95+
``allocation_percentage``), or when ``total_budget`` is missing.
96+
"""
97+
# Local imports — :class:`PackageRequest` lives in adcp.types and we
98+
# avoid a top-level circular when adcp.types reimports adcp helpers.
99+
from adcp.types import PackageRequest
100+
101+
del recipes # built-in derivation doesn't consult recipes
102+
103+
if total_budget is None:
104+
raise AdcpError(
105+
"INVALID_REQUEST",
106+
message=(
107+
"create_media_buy(proposal_id=...) requires total_budget "
108+
"when packages are omitted; the publisher derives package "
109+
"budgets by applying the proposal's allocation_percentage "
110+
"values to total_budget.amount."
111+
),
112+
recovery="correctable",
113+
field="total_budget",
114+
)
115+
116+
budget_amount = _read_attr(total_budget, "amount")
117+
if budget_amount is None:
118+
raise AdcpError(
119+
"INVALID_REQUEST",
120+
message="total_budget.amount is required for package derivation.",
121+
recovery="correctable",
122+
field="total_budget.amount",
123+
)
124+
125+
allocations = (
126+
proposal_payload.get("allocations") if isinstance(proposal_payload, Mapping) else None
127+
)
128+
if not allocations or not isinstance(allocations, list):
129+
raise AdcpError(
130+
"INVALID_REQUEST",
131+
message=(
132+
"Cannot derive packages: the committed proposal carries no "
133+
"allocations[]. The buyer must supply packages[] explicitly "
134+
"or the seller must regenerate the proposal with allocations."
135+
),
136+
recovery="terminal",
137+
field="proposal_id",
138+
)
139+
140+
packages: list[PackageRequest] = []
141+
for idx, allocation in enumerate(allocations):
142+
product_id = _read_attr(allocation, "product_id")
143+
if not product_id:
144+
raise AdcpError(
145+
"INVALID_REQUEST",
146+
message=(f"Cannot derive packages: allocations[{idx}] is missing " "product_id."),
147+
recovery="terminal",
148+
field=f"proposal.allocations[{idx}].product_id",
149+
)
150+
pct = _read_attr(allocation, "allocation_percentage")
151+
if pct is None:
152+
raise AdcpError(
153+
"INVALID_REQUEST",
154+
message=(
155+
f"Cannot derive packages: allocations[{idx}] for product "
156+
f"{product_id!r} is missing allocation_percentage."
157+
),
158+
recovery="terminal",
159+
field=f"proposal.allocations[{idx}].allocation_percentage",
160+
)
161+
pricing_option_id = _read_attr(allocation, "pricing_option_id")
162+
if not pricing_option_id:
163+
# Seller-side gap, NOT buyer-correctable. ProductAllocation
164+
# `pricing_option_id` is optional on the wire (it's only a
165+
# "recommended" pricing option) but PackageRequest requires
166+
# one. The built-in even-percentage derivation has no way to
167+
# pick — only the seller knows whether the product is auction-
168+
# priced, has multiple options, etc. INTERNAL_ERROR signals
169+
# this to the buyer as a seller bug; the seller's options
170+
# are (a) populate allocation.pricing_option_id at proposal-
171+
# assembly time, (b) ensure products under the proposal have
172+
# exactly one pricing_options[] entry (framework auto-picks),
173+
# or (c) implement ProposalManager.derive_packages for
174+
# auction / multi-option semantics.
175+
logger.error(
176+
"Cannot derive packages from proposal allocation %d: "
177+
"product %r is missing pricing_option_id. Adopter must "
178+
"set allocation.pricing_option_id at proposal-assembly "
179+
"time, expose a single product.pricing_options entry, "
180+
"or implement ProposalManager.derive_packages. The "
181+
"buyer's create_media_buy will fail until this is fixed.",
182+
idx,
183+
product_id,
184+
)
185+
raise AdcpError(
186+
"INTERNAL_ERROR",
187+
message=(
188+
f"Seller configuration error: proposal allocation for "
189+
f"product {product_id!r} is missing pricing_option_id. "
190+
"Contact the seller — this is not a buyer-correctable "
191+
"input."
192+
),
193+
recovery="terminal",
194+
)
195+
196+
pkg_budget = float(budget_amount) * (float(pct) / 100.0)
197+
# Spec-defined per-allocation flight scheduling — when the
198+
# seller's proposal carries allocation.start_time/end_time,
199+
# propagate them to the derived package. ``ProductAllocation``
200+
# docs them as "allows publishers to propose per-flight
201+
# scheduling within a proposal." Dropping them silently would
202+
# erase seller intent.
203+
kwargs: dict[str, Any] = {
204+
"product_id": str(product_id),
205+
"budget": pkg_budget,
206+
"pricing_option_id": str(pricing_option_id),
207+
}
208+
start_time = _read_attr(allocation, "start_time")
209+
if start_time is not None:
210+
kwargs["start_time"] = start_time
211+
end_time = _read_attr(allocation, "end_time")
212+
if end_time is not None:
213+
kwargs["end_time"] = end_time
214+
packages.append(PackageRequest(**kwargs))
215+
return packages
216+
217+
218+
def _read_attr(obj: Any, name: str) -> Any:
219+
"""Read an attribute or mapping value, normalizing the two shapes.
220+
221+
Mirrors the helper inside :mod:`adcp.decisioning.proposal_dispatch`;
222+
duplicated here to keep this module dependency-free.
223+
"""
224+
if obj is None:
225+
return None
226+
if isinstance(obj, Mapping):
227+
return obj.get(name)
228+
return getattr(obj, name, None)
229+
230+
231+
__all__ = ["derive_packages_from_proposal"]

src/adcp/decisioning/pg/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,14 @@
3232
PG_AVAILABLE,
3333
PgBuyerAgentRegistry,
3434
)
35+
from adcp.decisioning.pg.proposal_store import PgProposalStore
3536
from adcp.decisioning.pg.task_registry import PgTaskRegistry, PostgresTaskRegistry
3637

3738
__all__ = [
3839
"DEFAULT_TABLE_NAME",
3940
"PG_AVAILABLE",
4041
"PgBuyerAgentRegistry",
42+
"PgProposalStore",
4143
"PgTaskRegistry",
4244
"PostgresTaskRegistry",
4345
]

0 commit comments

Comments
 (0)