Skip to content

feat(decisioning): ProposalCapabilities.auto_commit_on_put_draft (#723)#725

Merged
bokelley merged 2 commits into
mainfrom
claude/issue-723-auto-commit-on-put-draft
May 13, 2026
Merged

feat(decisioning): ProposalCapabilities.auto_commit_on_put_draft (#723)#725
bokelley merged 2 commits into
mainfrom
claude/issue-723-auto-commit-on-put-draft

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #723 (option B from the issue).

Summary

Pre-#723 the storyboard step 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. Adopters wrote state=COMMITTED directly inside their put_draft implementations (salesagent PR #390); this PR ships the framework-side equivalent.

Surface

class ProposalCapabilities:
    auto_commit_on_put_draft: bool = False           # NEW
    auto_commit_ttl_seconds: int = 604800            # NEW — 7-day default

When auto_commit_on_put_draft=True, the framework calls ProposalStore.commit immediately after put_draft on every proposal returned, promoting DRAFT → COMMITTED in a single dispatch.

Validation

  • auto_commit_on_put_draft=True + finalize=TrueAdcpError. Mutually exclusive — both on would race.
  • auto_commit_ttl_seconds <= 0AdcpError. Zero would expire on commit.

Why option B (not A, C, D from the issue)

  • B is the smallest contained framework change. Adopters opt in per-manager; existing managers see no behavior change.
  • A (storyboard finalize step) is spec-honest but heavy: every adopter ships finalize_proposal.
  • C (split storyboards) is a spec-repo conversation, not a framework change.
  • D (docs only) doesn't close the gap.

Adopter migration

Salesagent drops the inline state=COMMITTED in SalesAgentProposalStore.put_draft and sets auto_commit_on_put_draft=True on its ProposalCapabilities.

Test plan

  • 7 new tests covering: happy path; back-compat default; TTL honored; multi-proposal; mutually-exclusive guard; ttl<=0 guard; default-False back-compat.
  • All 84 proposal-lifecycle tests pass locally.
  • ruff + mypy clean.

🤖 Generated with Claude Code

bokelley and others added 2 commits May 13, 2026 08:46
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>
…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>
@bokelley bokelley merged commit 5567e2b into main May 13, 2026
16 checks passed
@bokelley bokelley deleted the claude/issue-723-auto-commit-on-put-draft branch May 13, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

proposal_finalize storyboard: brief → create_media_buy has no path to COMMITTED state

1 participant