Skip to content

Commit 5567e2b

Browse files
bokelleyclaude
andauthored
feat(decisioning): ProposalCapabilities.auto_commit_on_put_draft (#723) (#725)
* feat(decisioning): ProposalCapabilities.auto_commit_on_put_draft (#723) Closes #723 (option B). Pre-#723 the storyboard ``media_buy_seller/proposal_finalize/create_media_buy`` was unreachable for managers that didn't declare ``finalize=True``: brief mode returned proposals as DRAFT, next call's ``try_reserve_consumption`` raised ``PROPOSAL_NOT_COMMITTED``, and the only framework path that promoted DRAFT → COMMITTED was ``finalize_proposal`` (which most v1 adopters didn't ship). Adopters worked around it by writing ``state=COMMITTED`` directly inside their ``put_draft`` implementations (salesagent PR #390). This adds the framework-side equivalent. When a manager declares ``auto_commit_on_put_draft=True``, the dispatcher calls ``ProposalStore.commit`` immediately after ``put_draft`` on every proposal returned by ``get_products`` / ``refine_products``, promoting DRAFT → COMMITTED in a single dispatch. New ProposalCapabilities fields: - ``auto_commit_on_put_draft: bool = False`` - ``auto_commit_ttl_seconds: int = 604800`` (7 days, salesagent default) Construction-time validation: - auto_commit + finalize mutually exclusive (both on would race). - ``auto_commit_ttl_seconds <= 0`` rejected (would expire on commit). Tests: 7 new in tests/test_proposal_auto_commit.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(decisioning): address review feedback on auto-commit capability (#725) - Loud-fail ``auto_commit_on_put_draft=True`` on ``sales-guaranteed`` specialism. Product-expert catch: a 7-day default TTL inventory hold issued silently from every ``get_products`` call would burn guaranteed-direct inventory across thousands of catalog probes per day. GAM / ad-server proposal lifecycles require explicit buyer-driven reservation precisely because trafficking ops won't accept silent holds. Loud-fail with a clear migration path (switch to ``sales-non-guaranteed`` or set ``finalize=True``). - Implement the >30-day TTL soft-cap warning the docstring promised. The framework permits longer TTLs (long-running RFPs legitimately need them) but warns at boot so the choice is visible at declaration time. Boundary check pins that exactly 30 days does not trigger. - Move ``from datetime import timedelta`` to the module-level import (was inline inside the per-proposal loop). - 3 new tests: ``sales-guaranteed`` rejection, >30d warning + 30d boundary, catalog-mode (store wired but manager unwired stays in DRAFT regardless). - Rename local variable ``_SOFT_CAP_SECONDS`` → ``_soft_cap_seconds`` per ruff N806 (function-local should be lowercase). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a65185e commit 5567e2b

3 files changed

Lines changed: 423 additions & 2 deletions

File tree

src/adcp/decisioning/proposal_dispatch.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
import contextvars
5252
import functools
5353
import logging
54-
from datetime import datetime, timezone
54+
from datetime import datetime, timedelta, timezone
5555
from typing import TYPE_CHECKING, Any, cast
5656

5757
from adcp.decisioning.proposal_lifecycle import (
@@ -417,8 +417,17 @@ async def maybe_persist_draft_after_get_products(
417417
* Response carries no ``proposals[]`` (catalog mode).
418418
* No recipes attached to products (v1 ``implementation_config``
419419
flow without typed recipes).
420+
421+
Per #723: when the manager declares
422+
:attr:`ProposalCapabilities.auto_commit_on_put_draft`, the
423+
framework calls :meth:`ProposalStore.commit` immediately after
424+
:meth:`put_draft` to promote ``DRAFT → COMMITTED`` so the
425+
subsequent ``create_media_buy(proposal_id=X)`` can call
426+
``try_reserve_consumption`` without a separate finalize step.
427+
Used by managers issuing directly-consumable proposals from
428+
``get_products``.
420429
"""
421-
_, store = _resolve_manager_and_store(platform, ctx)
430+
manager, store = _resolve_manager_and_store(platform, ctx)
422431
if store is None:
423432
return
424433

@@ -428,6 +437,14 @@ async def maybe_persist_draft_after_get_products(
428437

429438
products = _extract_list(response, "products") or []
430439

440+
# #723: cache the capability flag once per call — avoid attribute
441+
# lookups inside the per-proposal loop. ``manager`` may legitimately
442+
# be ``None`` (catalog-mode adopter wired a ProposalStore without
443+
# a ProposalManager); in that case auto-commit is off by default.
444+
caps = getattr(manager, "capabilities", None) if manager is not None else None
445+
auto_commit = bool(getattr(caps, "auto_commit_on_put_draft", False))
446+
auto_commit_ttl = int(getattr(caps, "auto_commit_ttl_seconds", 7 * 24 * 3600))
447+
431448
for proposal in proposals:
432449
proposal_id = _read(proposal, "proposal_id")
433450
if proposal_id is None:
@@ -453,6 +470,19 @@ async def maybe_persist_draft_after_get_products(
453470
account_id=ctx.account.id,
454471
recipes_count=len(recipes),
455472
)
473+
if auto_commit:
474+
# Promote DRAFT → COMMITTED in the same dispatch so the
475+
# next call's ``try_reserve_consumption`` finds a COMMITTED
476+
# record. The manager's ``auto_commit_ttl_seconds`` sets
477+
# the expires_at horizon.
478+
expires_at = datetime.now(timezone.utc) + timedelta(seconds=auto_commit_ttl)
479+
await _await_maybe(
480+
store.commit(
481+
str(proposal_id),
482+
expires_at=expires_at,
483+
proposal_payload=proposal_payload,
484+
)
485+
)
456486

457487

458488
def _collect_recipes_from_products(

src/adcp/decisioning/proposal_manager.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,26 @@ class ProposalCapabilities:
137137
Kevel for non-guaranteed-remnant in the same proposal).
138138
Informational in v1; the per-recipe-kind routing lands in
139139
a subsequent PR alongside the typed-recipe registry.
140+
:param auto_commit_on_put_draft: Opt-in shortcut for managers
141+
that issue directly-consumable proposals from ``get_products``
142+
without a separate ``finalize_proposal`` step. When ``True``,
143+
the framework calls :meth:`ProposalStore.commit` immediately
144+
after :meth:`ProposalStore.put_draft` on every proposal
145+
returned, promoting ``DRAFT → COMMITTED`` so that
146+
``create_media_buy(proposal_id=X)`` can call
147+
``try_reserve_consumption`` without a separate buyer round-trip.
148+
Mutually exclusive with ``finalize=True`` (finalize is the
149+
explicit lifecycle; auto-commit is the bypass). Adopters
150+
wiring their own commit lifecycle (e.g. webhook-driven
151+
approval) leave this ``False``. See #723.
152+
:param auto_commit_ttl_seconds: TTL applied to the auto-committed
153+
proposal's ``expires_at``. Used only when
154+
:attr:`auto_commit_on_put_draft` is ``True``. Defaults to
155+
``604800`` (7 days), matching the salesagent's adopter
156+
choice. Tune up for long-running RFPs; tune down for
157+
spot-market flows. Cap is enforced soft (a warning fires for
158+
values > 30 days) — buyers retrying past the TTL get
159+
``PROPOSAL_EXPIRED`` and re-request the brief.
140160
"""
141161

142162
sales_specialism: SalesSpecialism
@@ -152,6 +172,8 @@ class ProposalCapabilities:
152172
# the framework no longer reads this field. Stops appearing on new
153173
# adopter declarations; v1.6+ removes it entirely.
154174
multi_decisioning: bool = False
175+
auto_commit_on_put_draft: bool = False
176+
auto_commit_ttl_seconds: int = 7 * 24 * 3600 # 7 days, salesagent default
155177

156178
def __post_init__(self) -> None:
157179
# Spec only allows the two slugs at v1. Adopter passing a
@@ -185,6 +207,87 @@ def __post_init__(self) -> None:
185207
recovery="terminal",
186208
field="expires_at_grace_seconds",
187209
)
210+
# #723: auto-commit and finalize are mutually exclusive
211+
# lifecycles. ``finalize=True`` says "buyer drives DRAFT →
212+
# COMMITTED explicitly"; ``auto_commit_on_put_draft=True`` says
213+
# "framework promotes on put_draft so no explicit step is
214+
# needed." Both on at once produces a state-machine race
215+
# (the framework auto-commits, then the buyer's finalize call
216+
# rejects because the proposal is no longer DRAFT). Loud-fail
217+
# at construction.
218+
if self.auto_commit_on_put_draft and self.finalize:
219+
raise AdcpError(
220+
"INVALID_REQUEST",
221+
message=(
222+
"ProposalCapabilities: auto_commit_on_put_draft=True and "
223+
"finalize=True are mutually exclusive. auto-commit "
224+
"skips the explicit finalize step (proposals from "
225+
"get_products are committed-on-issuance); finalize "
226+
"requires the buyer to drive the transition. Pick one. "
227+
"See #723."
228+
),
229+
recovery="terminal",
230+
field="auto_commit_on_put_draft",
231+
)
232+
# #723 product safety: auto-commit on guaranteed-direct issues
233+
# a silent inventory hold on every ``get_products`` call. GAM /
234+
# ad-server proposal lifecycles require explicit reservation
235+
# precisely because trafficking ops won't accept silent holds
236+
# — a 7-day default TTL would burn inventory across thousands
237+
# of catalog probes per day. Loud-fail; adopters who need
238+
# auto-commit on guaranteed-direct can re-evaluate the
239+
# commercial commitment by wiring the explicit ``finalize``
240+
# path instead.
241+
if self.auto_commit_on_put_draft and self.sales_specialism == "sales-guaranteed":
242+
raise AdcpError(
243+
"INVALID_REQUEST",
244+
message=(
245+
"ProposalCapabilities: auto_commit_on_put_draft=True is "
246+
"not permitted on sales_specialism='sales-guaranteed'. "
247+
"Auto-commit issues a silent inventory hold on every "
248+
"get_products call (7-day default TTL); guaranteed-"
249+
"direct flows require explicit buyer-driven reservation "
250+
"via the finalize=True lifecycle to avoid unintended "
251+
"commitments. Either switch to "
252+
"sales_specialism='sales-non-guaranteed' (catalog / "
253+
"spot-market flows where auto-commit is appropriate) "
254+
"or set finalize=True instead."
255+
),
256+
recovery="terminal",
257+
field="auto_commit_on_put_draft",
258+
)
259+
if self.auto_commit_ttl_seconds <= 0:
260+
raise AdcpError(
261+
"INVALID_REQUEST",
262+
message=(
263+
"ProposalCapabilities.auto_commit_ttl_seconds must be "
264+
f"> 0; got {self.auto_commit_ttl_seconds!r}. Zero or "
265+
"negative TTL would mark proposals expired on commit, "
266+
"making every consumption attempt fail with "
267+
"PROPOSAL_EXPIRED."
268+
),
269+
recovery="terminal",
270+
field="auto_commit_ttl_seconds",
271+
)
272+
# Soft-cap warning: a TTL longer than 30 days holds inventory
273+
# for an entire month per catalog probe. Operators can extend
274+
# for long-running RFP flows, but the SDK surfaces a heads-up
275+
# so the default doesn't drift past what the adopter intended.
276+
_soft_cap_seconds = 30 * 24 * 3600
277+
if self.auto_commit_on_put_draft and self.auto_commit_ttl_seconds > _soft_cap_seconds:
278+
import warnings as _warnings
279+
280+
_warnings.warn(
281+
f"ProposalCapabilities.auto_commit_ttl_seconds="
282+
f"{self.auto_commit_ttl_seconds} exceeds the soft cap of "
283+
f"{_soft_cap_seconds} (30 days). Auto-committed proposals "
284+
"hold inventory for the full TTL — verify your commercial "
285+
"model supports holds this long. The framework permits "
286+
"it; this warning fires once per declaration site so the "
287+
"choice is visible at boot.",
288+
UserWarning,
289+
stacklevel=3,
290+
)
188291

189292

190293
@dataclass(frozen=True)

0 commit comments

Comments
 (0)