Skip to content

Commit fcfee67

Browse files
bokelleyclaude
andauthored
test(server): rewrite spec_compat tests against public API (#657)
Closes #653. The previous tests in tests/test_spec_compat_hooks.py reached into private helpers (_hook_get_products, _make_sync_creatives_hook, _coerce_asset, _infer_asset_type) and asserted on internal shapes, which hid a class of bugs the public surface would expose. Changes: - All tests now exercise spec_compat_hooks() and the returned dict only. No private imports. - Add four model_validate() proofs that hook output is actually 4.4-valid: one for get_products (buying_mode default), three for sync_creatives (format_id wrap, asset_type inference, image→url demotion). Failure here means the hook is producing dicts that pass our internal asserts but fail at the dispatcher boundary. - Expand the create_tool_caller integration suite from one buying_mode smoke test to four end-to-end paths: get_products default, sync_creatives format_id wrap, asset_type inference, image→url demotion. - spec_compat_hooks(exclude={...}) now emits a UserWarning for unknown hook names (typos like "sync_creative" used to silently leave the hook active). Document the behaviour in the factory's exclude= section. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1b4f3e0 commit fcfee67

2 files changed

Lines changed: 509 additions & 137 deletions

File tree

‎src/adcp/server/spec_compat.py‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
from __future__ import annotations
3232

33+
import warnings
3334
from collections.abc import Callable, Collection
3435
from typing import Any, TypeAlias
3536

@@ -44,6 +45,13 @@
4445
creative-format registries.
4546
"""
4647

48+
_VALID_HOOK_NAMES: frozenset[str] = frozenset({"get_products", "sync_creatives"})
49+
"""Tool names ``spec_compat_hooks()`` returns hooks for.
50+
51+
Exposed for ``exclude=`` validation — a typo like ``exclude={"sync_creative"}``
52+
emits a :class:`UserWarning` instead of silently leaving the hook active.
53+
"""
54+
4755
# Exact set of asset_type discriminator values the spec defines.
4856
# Used by the asset_type inference heuristic — only exact key matches count.
4957
_KNOWN_ASSET_TYPES: frozenset[str] = frozenset(
@@ -119,9 +127,7 @@ def _coerce_asset(asset_key: str, asset: dict[str, Any]) -> dict[str, Any]:
119127
# Only fires when url is present — if url is also absent the asset is
120128
# structurally unusable either way; leave it for schema validation to
121129
# report rather than silently changing the type to something equally invalid.
122-
if asset.get("asset_type") == "image" and not (
123-
"width" in asset and "height" in asset
124-
):
130+
if asset.get("asset_type") == "image" and not ("width" in asset and "height" in asset):
125131
if "url" in asset:
126132
asset = {k: v for k, v in asset.items() if k not in ("width", "height")}
127133
asset = {**asset, "asset_type": "url"}
@@ -217,10 +223,11 @@ def spec_compat_hooks(
217223
only one callable per tool name.
218224
219225
Args:
220-
exclude: Tool names to exclude from the returned dict. Members that
221-
are not valid hook names are silently ignored. Example:
222-
``exclude={"sync_creatives"}`` returns only the ``get_products``
223-
hook.
226+
exclude: Tool names to exclude from the returned dict. Names not in
227+
:data:`_VALID_HOOK_NAMES` raise a :class:`UserWarning` (a typo
228+
like ``exclude={"sync_creative"}`` would otherwise silently leave
229+
the hook active). Example: ``exclude={"sync_creatives"}`` returns
230+
only the ``get_products`` hook.
224231
creative_agent_url: Registry URL injected when wrapping a bare
225232
``format_id`` string. Defaults to
226233
``CANONICAL_CREATIVE_AGENT_URL``. Override for private
@@ -231,6 +238,16 @@ def spec_compat_hooks(
231238
``serve(pre_validation_hooks=...)``.
232239
"""
233240
excluded: frozenset[str] = frozenset(exclude) if exclude else frozenset()
241+
unknown = excluded - _VALID_HOOK_NAMES
242+
if unknown:
243+
warnings.warn(
244+
"spec_compat_hooks(exclude=...) received unknown hook name(s): "
245+
f"{sorted(unknown)}. Valid names: {sorted(_VALID_HOOK_NAMES)}. "
246+
"Typos here are silent — verify the name matches the tool you "
247+
"intended to opt out.",
248+
UserWarning,
249+
stacklevel=2,
250+
)
234251
hooks: PreValidationHooks = {}
235252

236253
if "get_products" not in excluded:

0 commit comments

Comments
 (0)