diff --git a/src/adcp/decisioning/dispatch.py b/src/adcp/decisioning/dispatch.py index 9751deec..1be72651 100644 --- a/src/adcp/decisioning/dispatch.py +++ b/src/adcp/decisioning/dispatch.py @@ -39,6 +39,7 @@ import difflib import functools import logging +import os import warnings from concurrent.futures import ThreadPoolExecutor from typing import TYPE_CHECKING, Any @@ -333,6 +334,61 @@ } +# --------------------------------------------------------------------------- +# RECOMMENDED_METHODS_PER_SPECIALISM — v6.0 rc.1 promotion staging +# --------------------------------------------------------------------------- + +#: Methods the SalesPlatform Protocol docstring marks "Required when +#: claiming any sales-* specialism in v6.0 rc.1+" but which the v6.0 alpha +#: enforced subset (REQUIRED_METHODS_PER_SPECIALISM) tolerates as absent. +#: ``validate_platform`` emits one ``UserWarning`` per missing method +#: pointing the adopter at the Protocol docstring; in strict mode +#: (``ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM=1``) the same misses +#: project to ``AdcpError("INVALID_REQUEST")`` instead. +#: +#: Promotion path (DX-423): when v6.0 rc.1 ships, fold these entries +#: into ``REQUIRED_METHODS_PER_SPECIALISM`` and delete this map. Until +#: then the soft-warn lets adopters mid-upgrade ship without a hard +#: break, while still flagging the gap loudly enough that nobody ships +#: a sales-* platform missing four spec-required methods by accident +#: (the v3 ref seller did exactly that until the deep review caught it). +#: +#: All five "narrow" sales-* slugs share the same recommended set; the +#: catalog-driven slug inherits the same four (its REQUIRED set already +#: adds ``sync_catalogs``). +_SALES_RECOMMENDED: frozenset[str] = frozenset( + { + "get_media_buys", + "provide_performance_feedback", + "list_creative_formats", + "list_creatives", + } +) +RECOMMENDED_METHODS_PER_SPECIALISM: dict[str, frozenset[str]] = { + "sales-non-guaranteed": _SALES_RECOMMENDED, + "sales-guaranteed": _SALES_RECOMMENDED, + "sales-broadcast-tv": _SALES_RECOMMENDED, + "sales-social": _SALES_RECOMMENDED, + "sales-proposal-mode": _SALES_RECOMMENDED, + "sales-catalog-driven": _SALES_RECOMMENDED, +} + +#: Env var that flips recommended-method misses from ``UserWarning`` to +#: ``AdcpError("INVALID_REQUEST")`` at server boot. Set to ``"1"`` to +#: opt in; any other value (including ``"true"``, unset, empty) leaves +#: the soft-warn behavior. Adopters who've completed the v6.0 rc.1 +#: surface migration should set this in CI to lock the gain in. +_STRICT_VALIDATE_ENV = "ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM" + + +def _strict_validate_platform() -> bool: + """True when the strict-validate env var is set to ``"1"``.""" + # Inline the literal name so docstring-vs-code consistency tests can + # match it via plain regex (the test scans for ``os.environ.get("FOO")`` + # patterns and doesn't follow the indirection through ``_STRICT_VALIDATE_ENV``). + return os.environ.get("ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM", "") == "1" + + # --------------------------------------------------------------------------- # INTERNAL_ERROR breadcrumbs (Emma AudioStack P2) # --------------------------------------------------------------------------- @@ -449,8 +505,15 @@ def validate_platform(platform: DecisioningPlatform) -> None: 3. Each claimed specialism's required methods are implemented on the platform subclass. Unknown specialisms emit ``UserWarning`` (forward-compat with v6.x+ specs); known - specialisms missing methods raise ``AdcpError("INVALID_REQUEST")``. - 4. **Governance opt-in fail-fast (D15 round-4):** if any claimed + specialisms missing methods raise an INVALID_REQUEST error. + 4. Each claimed specialism's *recommended* methods (the v6.0 rc.1 + staging set in :data:`RECOMMENDED_METHODS_PER_SPECIALISM` — + sales-* surface broadening per DX-423) are implemented on the + platform subclass. Misses emit one ``UserWarning`` per + method (deduped across overlapping specialisms). Setting + ``ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM=1`` flips the soft + warning into a hard INVALID_REQUEST error. + 5. **Governance opt-in fail-fast (D15 round-4):** if any claimed specialism is in :data:`GOVERNANCE_SPECIALISMS` AND ``capabilities.governance_aware`` is False AND the platform hasn't wired a custom :class:`StateReader` (i.e., the dispatch @@ -608,6 +671,67 @@ def validate_platform(platform: DecisioningPlatform) -> None: details={"missing": [{"specialism": s, "method": m} for s, m in missing]}, ) + # Recommended (v6.0 rc.1 staging) coverage — soft-warn by default, + # hard-fail under ``ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM=1``. + # Dedup by method name: a platform claiming both ``sales-guaranteed`` + # and ``sales-non-guaranteed`` shares the same recommended set, so + # ``get_media_buys`` should warn once, not twice. We walk specialisms + # in declared order and remember the first specialism that surfaced + # each missing method — that becomes the "blame" specialism in the + # diagnostic. + recommended_missing: list[tuple[str, str]] = [] + seen_methods: set[str] = set() + for specialism in platform.capabilities.specialisms: + recommended = RECOMMENDED_METHODS_PER_SPECIALISM.get(specialism) + if recommended is None: + continue + for method_name in sorted(recommended): + if method_name in seen_methods: + continue + if not _has_overridden_method(platform, method_name): + recommended_missing.append((specialism, method_name)) + seen_methods.add(method_name) + + if recommended_missing: + if _strict_validate_platform(): + raise AdcpError( + "INVALID_REQUEST", + message=( + "DecisioningPlatform claims sales-* specialism(s) but is " + f"missing v6.0 rc.1 required methods: {recommended_missing}. " + "Strict mode is enabled " + f"({_STRICT_VALIDATE_ENV}=1); implement each on your " + "subclass. See the SalesPlatform Protocol docstring at " + "src/adcp/decisioning/specialisms/sales.py:184-227 for the " + "canonical method list." + ), + recovery="terminal", + details={ + "missing_recommended": [ + {"specialism": s, "method": m} for s, m in recommended_missing + ], + "strict_env_var": _STRICT_VALIDATE_ENV, + }, + ) + # ``stacklevel=3`` so the warning points at the adopter's + # ``serve(platform)`` call site, not the SDK internals + # (validate_platform is invoked from serve, which is invoked by + # the adopter — three frames up lands on adopter code). + for specialism, method_name in recommended_missing: + warnings.warn( + ( + f"DecisioningPlatform claims {specialism!r} but is missing " + f"{method_name!r} — required by the SalesPlatform Protocol " + "for any sales-* specialism in v6.0 rc.1+. See the Protocol " + "docstring at src/adcp/decisioning/specialisms/sales.py:" + "184-227 for the full required method list. The framework " + "currently soft-warns to ease v6.0 rc.1 migration; set " + f"{_STRICT_VALIDATE_ENV}=1 to fail-fast at boot instead." + ), + UserWarning, + stacklevel=3, + ) + # Governance opt-in fail-fast (D15 round-4). if governance_specialisms_claimed and not platform.capabilities.governance_aware: raise AdcpError( @@ -1144,6 +1268,7 @@ async def _project_workflow_handoff( __all__ = [ + "RECOMMENDED_METHODS_PER_SPECIALISM", "REQUIRED_METHODS_PER_SPECIALISM", "SPEC_SPECIALISM_ENUM", "compose_caller_identity", diff --git a/tests/test_validate_platform_warnings.py b/tests/test_validate_platform_warnings.py new file mode 100644 index 00000000..a28473a1 --- /dev/null +++ b/tests/test_validate_platform_warnings.py @@ -0,0 +1,286 @@ +"""Tests for the v6.0 rc.1 sales-* surface-broadening soft-warn path +(DX-423). + +Covers ``validate_platform``'s ``RECOMMENDED_METHODS_PER_SPECIALISM`` +walk: a sales-* platform that implements the five strict-required +methods but is missing one of the four v6.0 rc.1 recommended methods +(``get_media_buys``, ``provide_performance_feedback``, +``list_creative_formats``, ``list_creatives``) emits a +``UserWarning`` per missing method, deduped across overlapping +specialisms. Setting ``ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM=1`` +flips the warning into an ``AdcpError("INVALID_REQUEST")``. + +Regression guard: the v3 reference seller, post-broadening (PR #408), +implements all nine methods, so importing its platform and running +``validate_platform`` MUST emit zero warnings. +""" + +from __future__ import annotations + +import warnings + +import pytest + +from adcp.decisioning import ( + AdcpError, + DecisioningCapabilities, + DecisioningPlatform, + SingletonAccounts, +) +from adcp.decisioning.dispatch import ( + RECOMMENDED_METHODS_PER_SPECIALISM, + validate_platform, +) + +# --------------------------------------------------------------------------- +# Helpers — platforms that satisfy the strict required surface but vary +# in their recommended-method coverage. +# --------------------------------------------------------------------------- + + +def _add_strict_required(cls: type[DecisioningPlatform]) -> None: + """Stamp the five strict-required sales-* methods onto a class so + each test only has to declare its recommended-method coverage.""" + + def get_products(self, req, ctx): + return {"products": []} + + def create_media_buy(self, req, ctx): + return {"media_buy_id": "mb_1"} + + def update_media_buy(self, media_buy_id, patch, ctx): + return {"media_buy_id": media_buy_id, "status": "active"} + + def sync_creatives(self, req, ctx): + return {"creatives": []} + + def get_media_buy_delivery(self, req, ctx): + return {"deliveries": []} + + cls.get_products = get_products # type: ignore[attr-defined] + cls.create_media_buy = create_media_buy # type: ignore[attr-defined] + cls.update_media_buy = update_media_buy # type: ignore[attr-defined] + cls.sync_creatives = sync_creatives # type: ignore[attr-defined] + cls.get_media_buy_delivery = get_media_buy_delivery # type: ignore[attr-defined] + + +class _PartialSalesPlatform(DecisioningPlatform): + """sales-non-guaranteed seller missing all four recommended methods.""" + + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + accounts = SingletonAccounts(account_id="acct-1") + + +_add_strict_required(_PartialSalesPlatform) + + +class _FullSalesPlatform(DecisioningPlatform): + """sales-non-guaranteed seller implementing all 9 methods.""" + + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + accounts = SingletonAccounts(account_id="acct-1") + + def get_media_buys(self, req, ctx): + return {"media_buys": []} + + def provide_performance_feedback(self, req, ctx): + return {"acknowledged": True} + + def list_creative_formats(self, req, ctx): + return {"formats": []} + + def list_creatives(self, req, ctx): + return {"creatives": []} + + +_add_strict_required(_FullSalesPlatform) + + +class _DualSalesPlatform(DecisioningPlatform): + """Platform claiming TWO sales-* specialisms with overlapping + recommended sets — used to exercise the dedup path.""" + + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed", "sales-guaranteed"]) + accounts = SingletonAccounts(account_id="acct-1") + + +_add_strict_required(_DualSalesPlatform) + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +def test_warns_when_sales_recommended_method_missing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """sales-non-guaranteed platform missing get_media_buys (and the + other three recommended methods) emits one UserWarning per missing + method. Each warning identifies the specialism + missing method and + points at the SalesPlatform Protocol docstring.""" + + monkeypatch.delenv("ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM", raising=False) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + validate_platform(_PartialSalesPlatform()) + + user_warnings = [w for w in caught if issubclass(w.category, UserWarning)] + # All four recommended methods missing. + assert len(user_warnings) == 4 + methods_warned = set() + for w in user_warnings: + msg = str(w.message) + assert "sales-non-guaranteed" in msg + assert "src/adcp/decisioning/specialisms/sales.py:184-227" in msg + assert "ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM" in msg + # Pull the warned method name out of the message. + for method in ( + "get_media_buys", + "provide_performance_feedback", + "list_creative_formats", + "list_creatives", + ): + if f"'{method}'" in msg: + methods_warned.add(method) + assert methods_warned == { + "get_media_buys", + "provide_performance_feedback", + "list_creative_formats", + "list_creatives", + } + + +def test_no_warning_when_all_recommended_methods_present( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A sales-* platform implementing all 9 methods (five required + + four recommended) emits zero warnings from validate_platform.""" + + monkeypatch.delenv("ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM", raising=False) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + validate_platform(_FullSalesPlatform()) + + assert [w for w in caught if issubclass(w.category, UserWarning)] == [] + + +def test_strict_mode_raises_instead_of_warning( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM=1 flips recommended + misses from UserWarning to AdcpError("INVALID_REQUEST").""" + + monkeypatch.setenv("ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM", "1") + with pytest.raises(AdcpError) as exc_info: + validate_platform(_PartialSalesPlatform()) + err = exc_info.value + assert err.code == "INVALID_REQUEST" + assert "Strict mode is enabled" in str(err) + missing = err.details["missing_recommended"] + methods = {entry["method"] for entry in missing} + assert methods == { + "get_media_buys", + "provide_performance_feedback", + "list_creative_formats", + "list_creatives", + } + assert all(entry["specialism"] == "sales-non-guaranteed" for entry in missing) + + +def test_strict_mode_only_triggers_on_value_one( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The env-var contract is exactly ``"1"``. Other truthy strings + (``"true"``, ``"yes"``) keep soft-warn behavior. Documenting this + pin via test so a future ``"true"``-tolerant rewrite has to update + both sides intentionally.""" + + monkeypatch.setenv("ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM", "true") + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + validate_platform(_PartialSalesPlatform()) + assert any(issubclass(w.category, UserWarning) for w in caught) + + +def test_warning_dedup_across_overlapping_specialisms( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Platform claiming two sales-* specialisms with the same + recommended set warns once per missing method, not once per + (specialism, method) pair.""" + + monkeypatch.delenv("ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM", raising=False) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + validate_platform(_DualSalesPlatform()) + + user_warnings = [w for w in caught if issubclass(w.category, UserWarning)] + # Four recommended methods, two specialisms with overlapping sets: + # eight warnings WITHOUT dedup, four WITH dedup. + assert len(user_warnings) == 4 + + +def test_recommended_map_covers_all_sales_specialisms() -> None: + """Sanity guard: every sales-* slug in the spec enum that has a + Protocol surface MUST appear in RECOMMENDED_METHODS_PER_SPECIALISM, + so a future spec slug addition that forgets the wiring fails this + test rather than shipping silently-unenforced.""" + + expected_sales_slugs = { + "sales-non-guaranteed", + "sales-guaranteed", + "sales-broadcast-tv", + "sales-social", + "sales-proposal-mode", + "sales-catalog-driven", + } + assert expected_sales_slugs <= set(RECOMMENDED_METHODS_PER_SPECIALISM.keys()) + expected_recommended = frozenset( + { + "get_media_buys", + "provide_performance_feedback", + "list_creative_formats", + "list_creatives", + } + ) + for slug in expected_sales_slugs: + assert ( + RECOMMENDED_METHODS_PER_SPECIALISM[slug] == expected_recommended + ), f"recommended-method drift on {slug}" + + +def test_v3_reference_seller_passes_with_no_warnings( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Regression guard for PR #408: the v3 reference seller broadened + its surface to include all four v6.0 rc.1 recommended methods, so + validate_platform on a v3 ref seller instance MUST emit zero + UserWarnings. If this test fails, PR #408 has regressed. + + The seller's ``__init__`` requires a SQLAlchemy ``async_sessionmaker`` + and an optional ``MockAdServer``; we build the instance with a sentinel + sessionmaker because ``validate_platform`` only walks attributes — + it never executes the methods, so the sessionmaker is never touched. + """ + + v3_platform = pytest.importorskip("examples.v3_reference_seller.src.platform") + seller_cls = v3_platform.V3ReferenceSeller + + # Sentinel sessionmaker — never invoked by validate_platform. + class _StubSessionmaker: + def __call__(self, *args, **kwargs): + raise RuntimeError("validate_platform must not open a session") + + seller = seller_cls(sessionmaker=_StubSessionmaker()) + + monkeypatch.delenv("ADCP_DECISIONING_STRICT_VALIDATE_PLATFORM", raising=False) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + validate_platform(seller) + + user_warnings = [w for w in caught if issubclass(w.category, UserWarning)] + assert user_warnings == [], ( + "v3 ref seller emitted unexpected UserWarning(s) — DX-423 regression: " + f"{[str(w.message) for w in user_warnings]}" + )