Follow-up to #646 / PR #648.
The tests in tests/test_spec_compat_hooks.py (added in PR #648) reach into private helpers (_hook_get_products, _infer_asset_type, _coerce_asset, _make_sync_creatives_hook) and assert on internal shapes. Per CLAUDE.md the testing contract is:
- Test what you import — call the public API, not internals.
- Validate with
.model_validate() — prove hook output is actually 4.4-schema-valid by feeding it through the generated SyncCreativesRequest / GetProductsRequest models from adcp.types.
Scope
- Rewrite assertions to go through
spec_compat_hooks() + the published hook dict surface (e.g. via create_tool_caller with the hooks installed), not the private helpers.
- Add at least one test that runs the coerced
sync_creatives payload through SyncCreativesRequest.model_validate() and asserts no ValidationError.
- Expand the
create_tool_caller integration test (currently only checks the buying_mode default on get_products) to exercise the harder paths:
format_id wrap (string → object)
asset_type inference for unknown assets
image → url demotion through the actual sync_creatives tool path
- Optional: emit a
warnings.warn when exclude= contains a name not in the canonical set, so typos like exclude={"sync_creative"} don't silently leave the hook active.
Why not block PR #648 on this
The implementation in src/adcp/server/spec_compat.py is correct and matches the spec table from #646. The current tests do exercise every code path (just at the wrong level); regression coverage is real. Splitting the test refactor out keeps the feature shippable.
Follow-up to #646 / PR #648.
The tests in
tests/test_spec_compat_hooks.py(added in PR #648) reach into private helpers (_hook_get_products,_infer_asset_type,_coerce_asset,_make_sync_creatives_hook) and assert on internal shapes. Per CLAUDE.md the testing contract is:.model_validate()— prove hook output is actually 4.4-schema-valid by feeding it through the generatedSyncCreativesRequest/GetProductsRequestmodels fromadcp.types.Scope
spec_compat_hooks()+ the published hook dict surface (e.g. viacreate_tool_callerwith the hooks installed), not the private helpers.sync_creativespayload throughSyncCreativesRequest.model_validate()and asserts noValidationError.create_tool_callerintegration test (currently only checks thebuying_modedefault on get_products) to exercise the harder paths:format_idwrap (string → object)asset_typeinference for unknown assetsimage→urldemotion through the actualsync_creativestool pathwarnings.warnwhenexclude=contains a name not in the canonical set, so typos likeexclude={"sync_creative"}don't silently leave the hook active.Why not block PR #648 on this
The implementation in
src/adcp/server/spec_compat.pyis correct and matches the spec table from #646. The current tests do exercise every code path (just at the wrong level); regression coverage is real. Splitting the test refactor out keeps the feature shippable.