Skip to content

Commit ba72beb

Browse files
committed
feat(handlers): expose sync_catalogs through sales-catalog-driven specialism
Issue #786: sync_catalogs was defined in ADCP_TOOL_DEFINITIONS and SyncCatalogsRequest/response types existed, but no specialism mapping surfaced it to buyers and PlatformHandler had no dispatcher shim. Changes: - Add _CATALOG_ADVERTISED_TOOLS frozenset and union it into SPECIALISM_TO_ADVERTISED_TOOLS["sales-catalog-driven"] - Add _CATALOG_ADVERTISED_TOOLS to PlatformHandler.advertised_tools ClassVar - Add _project_sync_catalogs helper (list-return ergonomic arm) - Add PlatformHandler.sync_catalogs dispatcher; passes full SyncCatalogsRequest to platform (no arg projection) so adopters retain access to delete_missing/dry_run/validation_mode - Add sync_catalogs method declaration to SalesPlatform Protocol with discovery-mode semantics documented (req.catalogs is None = read-only) - Update tests: expected set, parametrize list, three shim tests, two per-specialism filter tests, runtime_checkable update Note: adding sync_catalogs to SalesPlatform Protocol body is a minor structural consequence for @runtime_checkable isinstance checks — isinstance(obj, SalesPlatform) now requires sync_catalogs. Production code uses validate_platform (not isinstance) as its enforcement gate; no existing framework code is affected. Documented in test update. #786
1 parent 65f84d2 commit ba72beb

5 files changed

Lines changed: 309 additions & 3 deletions

File tree

src/adcp/decisioning/handler.py

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@
165165
SyncAccountsResponse,
166166
SyncAudiencesRequest,
167167
SyncAudiencesSuccessResponse,
168+
SyncCatalogsRequest,
169+
SyncCatalogsSuccessResponse,
168170
SyncCreativesRequest,
169171
SyncCreativesSuccessResponse,
170172
SyncPlansRequest,
@@ -292,6 +294,11 @@
292294
"si_terminate_session",
293295
}
294296
)
297+
_CATALOG_ADVERTISED_TOOLS: frozenset[str] = frozenset(
298+
{
299+
"sync_catalogs",
300+
}
301+
)
295302
_GOVERNANCE_ADVERTISED_TOOLS: frozenset[str] = frozenset(
296303
{
297304
"check_governance",
@@ -394,7 +401,9 @@
394401
"sales-guaranteed": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS,
395402
"sales-broadcast-tv": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS,
396403
"sales-social": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS,
397-
"sales-catalog-driven": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS,
404+
"sales-catalog-driven": (
405+
_SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS | _CATALOG_ADVERTISED_TOOLS
406+
),
398407
"sales-proposal-mode": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS,
399408
# Creative — Builder + AdServer. Builder claims expose
400409
# build_creative + optional preview_creative; AdServer adds
@@ -794,6 +803,27 @@ def _project_sync_audiences(result: Any) -> Any:
794803
return result
795804

796805

806+
def _project_sync_catalogs(result: Any) -> Any:
807+
"""Project the adopter's ``sync_catalogs`` return into the wire envelope shape.
808+
809+
Adopters may return a list of catalog-result rows (ergonomic form) or a
810+
fully-shaped :class:`SyncCatalogsSuccessResponse`. The wire envelope per
811+
``schemas/cache/media-buy/sync-catalogs-response.json`` is
812+
``{catalogs: [rows]}``. This helper wraps the list case.
813+
814+
Discovery mode (``req.catalogs is None``) returns existing synced catalogs
815+
per spec — the platform method must handle ``req.catalogs is None`` as a
816+
read-only path.
817+
"""
818+
if isinstance(result, list):
819+
return {
820+
"catalogs": [
821+
r.model_dump(mode="json") if hasattr(r, "model_dump") else r for r in result
822+
]
823+
}
824+
return result
825+
826+
797827
def _project_sync_accounts(result: Any) -> Any:
798828
"""Project the adopter's ``upsert`` return into the
799829
``sync_accounts`` wire envelope.
@@ -1014,6 +1044,7 @@ class PlatformHandler(ADCPHandler[ToolContext]):
10141044
| set(_CREATIVE_ADVERTISED_TOOLS)
10151045
| set(_SIGNALS_ADVERTISED_TOOLS)
10161046
| set(_AUDIENCE_ADVERTISED_TOOLS)
1047+
| set(_CATALOG_ADVERTISED_TOOLS)
10171048
| set(_GOVERNANCE_ADVERTISED_TOOLS)
10181049
| set(_BRAND_RIGHTS_ADVERTISED_TOOLS)
10191050
| set(_CONTENT_STANDARDS_ADVERTISED_TOOLS)
@@ -2376,6 +2407,41 @@ async def sync_audiences( # type: ignore[override]
23762407
self._maybe_auto_emit_sync_completion("sync_audiences", params, projected)
23772408
return cast("SyncAudiencesSuccessResponse", projected)
23782409

2410+
async def sync_catalogs( # type: ignore[override]
2411+
self,
2412+
params: SyncCatalogsRequest,
2413+
context: ToolContext | None = None,
2414+
) -> SyncCatalogsSuccessResponse:
2415+
"""Sync product catalogs with the platform.
2416+
2417+
The platform method receives the full :class:`SyncCatalogsRequest`
2418+
so adopters can inspect ``req.catalogs``, ``req.delete_missing``,
2419+
``req.dry_run``, and ``req.validation_mode``. Discovery mode
2420+
(``req.catalogs is None``) returns existing synced catalogs without
2421+
modification — the platform method must handle ``req.catalogs is None``
2422+
as a read-only path.
2423+
2424+
Two return arms per the per-specialism Protocol: a list of
2425+
:class:`SyncCatalogResult` rows (ergonomic form) or a fully-shaped
2426+
:class:`SyncCatalogsSuccessResponse`. The shim projects the list arm
2427+
to the wire envelope ``{catalogs: [...]}`` so adopters can return
2428+
the ergonomic form.
2429+
"""
2430+
tool_ctx = context or ToolContext()
2431+
account = await self._resolve_account(getattr(params, "account", None), tool_ctx)
2432+
ctx = self._build_ctx(tool_ctx, account)
2433+
result = await _invoke_platform_method(
2434+
self._platform,
2435+
"sync_catalogs",
2436+
params,
2437+
ctx,
2438+
executor=self._executor,
2439+
registry=self._registry,
2440+
)
2441+
projected = _project_sync_catalogs(result)
2442+
self._maybe_auto_emit_sync_completion("sync_catalogs", params, projected)
2443+
return cast("SyncCatalogsSuccessResponse", projected)
2444+
23792445
# ----- CampaignGovernancePlatform -----
23802446

23812447
async def check_governance( # type: ignore[override]

src/adcp/decisioning/specialisms/sales.py

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,12 @@
2525
* :meth:`provide_performance_feedback`
2626
* :meth:`list_creative_formats`
2727
* :meth:`list_creatives`
28-
* :meth:`sync_catalogs` — required when claiming
29-
``sales-catalog-driven``
28+
29+
Required only when claiming ``sales-catalog-driven``:
30+
31+
* :meth:`sync_catalogs` — catalog sync and discovery. ``validate_platform``
32+
hard-fails at server boot if a ``sales-catalog-driven`` platform doesn't
33+
implement this.
3034
3135
The framework's :func:`validate_platform` walks ``capabilities.specialisms``
3236
and confirms each specialism's required methods exist on the platform
@@ -64,6 +68,8 @@
6468
ListCreativesResponse,
6569
ProvidePerformanceFeedbackRequest,
6670
ProvidePerformanceFeedbackResponse,
71+
SyncCatalogsRequest,
72+
SyncCatalogsSuccessResponse,
6773
SyncCreativesRequest,
6874
SyncCreativesSuccessResponse,
6975
UpdateMediaBuyRequest,
@@ -269,3 +275,36 @@ def list_creatives(
269275
Required when claiming any ``sales-*`` specialism in v6.0 rc.1+.
270276
"""
271277
...
278+
279+
# ---- Required when claiming ``sales-catalog-driven`` ----
280+
281+
def sync_catalogs(
282+
self,
283+
req: SyncCatalogsRequest,
284+
ctx: RequestContext[TMeta],
285+
) -> MaybeAsync[SyncCatalogsSuccessResponse]:
286+
"""Sync product catalogs with the platform.
287+
288+
**Required** when claiming ``sales-catalog-driven``.
289+
``validate_platform`` hard-fails at server boot when this method is
290+
absent on a ``sales-catalog-driven`` platform.
291+
292+
Discovery mode: when ``req.catalogs is None``, return the account's
293+
existing synced catalogs without modification (read-only path per
294+
AdCP spec). Check ``req.catalogs`` before applying any mutations::
295+
296+
def sync_catalogs(self, req, ctx):
297+
if req.catalogs is None:
298+
return self._get_existing_catalogs(ctx.account_id)
299+
return self._upsert_catalogs(req.catalogs, ctx)
300+
301+
**Important:** ``req.delete_missing=True`` with ``req.catalogs=None``
302+
is spec-undefined — reject it with
303+
``AdcpError("INVALID_REQUEST", field="catalogs")`` rather than
304+
silently deleting buyer-managed catalogs.
305+
306+
Return a list of :class:`~adcp.types.SyncCatalogResult` rows
307+
(ergonomic form) or a fully-shaped
308+
:class:`~adcp.types.SyncCatalogsSuccessResponse`.
309+
"""
310+
...

tests/test_decisioning_advertised_per_specialism.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,29 @@ def build_creative(self, req, ctx):
117117
return {"creative_manifest": {"creative_id": "cr_1"}}
118118

119119

120+
class _CatalogDrivenPlatform(DecisioningPlatform):
121+
capabilities = DecisioningCapabilities(specialisms=["sales-catalog-driven"])
122+
accounts = SingletonAccounts(account_id="catalog-driven")
123+
124+
def get_products(self, req, ctx):
125+
return {"products": []}
126+
127+
def create_media_buy(self, req, ctx):
128+
return {"media_buy_id": "x", "status": "active"}
129+
130+
def update_media_buy(self, media_buy_id, patch, ctx):
131+
return {"media_buy_id": media_buy_id, "status": "active"}
132+
133+
def sync_creatives(self, req, ctx):
134+
return {"creatives": []}
135+
136+
def get_media_buy_delivery(self, req, ctx):
137+
return {"media_buy_deliveries": []}
138+
139+
def sync_catalogs(self, req, ctx):
140+
return {"catalogs": []}
141+
142+
120143
def test_sales_only_does_not_advertise_creative_or_signals_tools(executor) -> None:
121144
"""Regression: sales-only adopter saw acquire_rights, build_creative,
122145
check_governance, etc. in tools/list. After the per-specialism
@@ -333,3 +356,40 @@ def test_class_level_inspection_preserves_full_universe() -> None:
333356
assert "get_products" in tools
334357
assert "build_creative" in tools
335358
assert "acquire_rights" in tools
359+
360+
361+
def test_catalog_driven_advertises_sync_catalogs(executor) -> None:
362+
"""``sales-catalog-driven`` must expose ``sync_catalogs`` via
363+
``tools/list``. This was the missing wire-up reported in issue #786:
364+
the tool existed and types were defined, but no specialism mapping
365+
surfaced it to buyers."""
366+
handler = PlatformHandler(
367+
_CatalogDrivenPlatform(),
368+
executor=executor,
369+
registry=InMemoryTaskRegistry(),
370+
)
371+
tools = {tool["name"] for tool in get_tools_for_handler(handler)}
372+
373+
assert "sync_catalogs" in tools, (
374+
"sales-catalog-driven platform must advertise sync_catalogs; "
375+
"check SPECIALISM_TO_ADVERTISED_TOOLS['sales-catalog-driven']"
376+
)
377+
# Sales tools also present.
378+
assert "get_products" in tools
379+
assert "create_media_buy" in tools
380+
381+
382+
def test_non_catalog_sales_does_not_advertise_sync_catalogs(executor) -> None:
383+
"""``sync_catalogs`` is specific to ``sales-catalog-driven`` — no
384+
other sales-* specialism should surface it."""
385+
handler = PlatformHandler(
386+
_SalesOnlyPlatform(),
387+
executor=executor,
388+
registry=InMemoryTaskRegistry(),
389+
)
390+
tools = {tool["name"] for tool in get_tools_for_handler(handler)}
391+
392+
assert "sync_catalogs" not in tools, (
393+
"sales-non-guaranteed must not advertise sync_catalogs; "
394+
"sync_catalogs is only for sales-catalog-driven"
395+
)

tests/test_decisioning_handler_shims.py

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ def test_advertised_tools_covers_every_specialism_wire_tool() -> None:
8484
"activate_signal",
8585
# Audience
8686
"sync_audiences",
87+
# Catalog (sales-catalog-driven only)
88+
"sync_catalogs",
8789
# Governance
8890
"check_governance",
8991
"sync_plans",
@@ -131,6 +133,7 @@ def test_advertised_tools_covers_every_specialism_wire_tool() -> None:
131133
"get_signals",
132134
"activate_signal",
133135
"sync_audiences",
136+
"sync_catalogs",
134137
"check_governance",
135138
"sync_plans",
136139
"report_plan_outcome",
@@ -256,6 +259,136 @@ def sync_audiences(self, audiences, ctx):
256259
assert received_audiences == fake_audiences
257260

258261

262+
@pytest.mark.asyncio
263+
async def test_sync_catalogs_shim_passes_full_request_to_platform(executor) -> None:
264+
"""The ``sync_catalogs`` shim passes the full ``SyncCatalogsRequest``
265+
to the platform method (no arg projection). Adopters receive the full
266+
request so they can inspect ``req.catalogs``, ``req.delete_missing``,
267+
``req.dry_run``, and ``req.validation_mode``."""
268+
received_req = []
269+
270+
class _CatalogAgent(DecisioningPlatform):
271+
capabilities = DecisioningCapabilities(specialisms=["sales-catalog-driven"])
272+
accounts = SingletonAccounts(account_id="hello")
273+
274+
def get_products(self, req, ctx):
275+
return {"products": []}
276+
277+
def create_media_buy(self, req, ctx):
278+
return {"media_buy_id": "x", "status": "active"}
279+
280+
def update_media_buy(self, media_buy_id, patch, ctx):
281+
return {"media_buy_id": media_buy_id, "status": "active"}
282+
283+
def sync_creatives(self, req, ctx):
284+
return {"creatives": []}
285+
286+
def get_media_buy_delivery(self, req, ctx):
287+
return {"media_buy_deliveries": []}
288+
289+
def sync_catalogs(self, req, ctx):
290+
received_req.append(req)
291+
return {"catalogs": []}
292+
293+
handler = PlatformHandler(
294+
_CatalogAgent(),
295+
executor=executor,
296+
registry=InMemoryTaskRegistry(),
297+
)
298+
from adcp.types import SyncCatalogsRequest
299+
300+
fake_catalogs = [{"type": "product", "catalog_id": "feed-1"}]
301+
req = SyncCatalogsRequest.model_construct(catalogs=fake_catalogs)
302+
await handler.sync_catalogs(req, ToolContext())
303+
assert len(received_req) == 1
304+
assert received_req[0] is req
305+
306+
307+
@pytest.mark.asyncio
308+
async def test_sync_catalogs_discovery_mode_passes_none_catalogs(executor) -> None:
309+
"""Discovery mode (``req.catalogs is None``) passes the request
310+
through intact — the platform receives ``req`` with ``catalogs=None``
311+
and can distinguish discovery from an explicit empty push."""
312+
received_catalogs_value = []
313+
314+
class _CatalogAgent(DecisioningPlatform):
315+
capabilities = DecisioningCapabilities(specialisms=["sales-catalog-driven"])
316+
accounts = SingletonAccounts(account_id="hello")
317+
318+
def get_products(self, req, ctx):
319+
return {"products": []}
320+
321+
def create_media_buy(self, req, ctx):
322+
return {"media_buy_id": "x", "status": "active"}
323+
324+
def update_media_buy(self, media_buy_id, patch, ctx):
325+
return {"media_buy_id": media_buy_id, "status": "active"}
326+
327+
def sync_creatives(self, req, ctx):
328+
return {"creatives": []}
329+
330+
def get_media_buy_delivery(self, req, ctx):
331+
return {"media_buy_deliveries": []}
332+
333+
def sync_catalogs(self, req, ctx):
334+
received_catalogs_value.append(req.catalogs)
335+
return {"catalogs": []}
336+
337+
handler = PlatformHandler(
338+
_CatalogAgent(),
339+
executor=executor,
340+
registry=InMemoryTaskRegistry(),
341+
)
342+
from adcp.types import SyncCatalogsRequest
343+
344+
req = SyncCatalogsRequest.model_construct(catalogs=None)
345+
await handler.sync_catalogs(req, ToolContext())
346+
assert received_catalogs_value == [None]
347+
348+
349+
@pytest.mark.asyncio
350+
async def test_sync_catalogs_list_return_projected_to_envelope(executor) -> None:
351+
"""When the platform returns a list of catalog results (ergonomic arm),
352+
the shim wraps it in ``{catalogs: [...]}``."""
353+
354+
class _CatalogAgent(DecisioningPlatform):
355+
capabilities = DecisioningCapabilities(specialisms=["sales-catalog-driven"])
356+
accounts = SingletonAccounts(account_id="hello")
357+
358+
def get_products(self, req, ctx):
359+
return {"products": []}
360+
361+
def create_media_buy(self, req, ctx):
362+
return {"media_buy_id": "x", "status": "active"}
363+
364+
def update_media_buy(self, media_buy_id, patch, ctx):
365+
return {"media_buy_id": media_buy_id, "status": "active"}
366+
367+
def sync_creatives(self, req, ctx):
368+
return {"creatives": []}
369+
370+
def get_media_buy_delivery(self, req, ctx):
371+
return {"media_buy_deliveries": []}
372+
373+
def sync_catalogs(self, req, ctx):
374+
return [{"catalog_id": "feed-1", "action": "created", "item_count": 100}]
375+
376+
handler = PlatformHandler(
377+
_CatalogAgent(),
378+
executor=executor,
379+
registry=InMemoryTaskRegistry(),
380+
)
381+
from adcp.types import SyncCatalogsRequest
382+
383+
req = SyncCatalogsRequest.model_construct(
384+
catalogs=[{"type": "product", "catalog_id": "feed-1"}]
385+
)
386+
result = await handler.sync_catalogs(req, ToolContext())
387+
assert result == {
388+
"catalogs": [{"catalog_id": "feed-1", "action": "created", "item_count": 100}]
389+
}
390+
391+
259392
@pytest.mark.asyncio
260393
async def test_check_governance_shim_routes_to_platform(executor) -> None:
261394
class _GovernanceAgent(DecisioningPlatform):

0 commit comments

Comments
 (0)