Repository navigation
Description
Measured on the PyPI wheel adcp 9.6.1 (CPython 3.12.13), and re-read against main at bef739e02 where the construction sites are unchanged.
What happens
NoExplicitNull (#1547) refuses a JSON null for an optional property whose schema does not admit one, and keeps T | None on the Python side so that Model(field=None) still reads as "not supplied" — unless ADCP_STRICT_VALIDATION is set, where _RefuseJsonNull makes the Python side strict too (src/adcp/types/base.py, python_schema=strict if _EXTRA_POLICY == "forbid" else nullable).
With that switch on, three of the SDK's own entry points refuse their own construction:
$ ADCP_STRICT_VALIDATION=1 python probe.py
_EXTRA_POLICY = forbid
create_mcp_webhook_payload(task_id=..., task_type=..., status=..., idempotency_key=..., operation_id=...)
-> ValidationError: 5 validation errors for McpWebhookPayload
notification_id Input should be a valid string [type=string_type, input_value=None]
(and message, context_id, token, protocol)
PaginationRequest(max_results=50, cursor=None)
-> ValidationError: cursor Input should be a valid string [type=string_type, input_value=None]
Error(code="INVALID_INPUT", message="m", field=None, details=None, suggestion=None)
-> ValidationError: 3 validation errors for Error (field, details, suggestion)
Every one of those is reached through SDK code with the caller supplying nothing:
site on main |
construction |
|---|---|
src/adcp/webhooks.py:272-286 |
McpWebhookPayload.model_validate({... "notification_id": notification_id, "message": message, "context_id": context_id, "token": token ...}) — every key is named whether or not the caller passed it |
src/adcp/feed_mirror.py:449 and :467 |
PaginationRequest(max_results=self._page_limit, cursor=cursor), where cursor is None on the first page |
src/adcp/canonical_formats/advisory.py:141-149 |
Error(..., field=field, details=details, suggestion=suggestion) |
src/adcp/server/principal.py:166 and :812 |
{..., "context": context} into SyncPrincipalResponse.model_validate |
With the switch unset all four are accepted, so this is invisible in a default run.
Why it looks like a defect rather than a preference
src/adcp/types/base.py recommends the switch for exactly the case where it cannot be used:
Most useful during upgrades: right after a major spec revision, run tests with
ADCP_STRICT_VALIDATION=1to surface every place the upgrade dropped a renamed field, then ship with the flag unset for the forward-compat default.
A consumer who follows that advice gets failures in SDK code rather than in their own. The class is not small: NoExplicitNull appears 7,061 times across 667 generated modules on main.
And the documents agree with the strict reading. None of these properties admits a null:
| document | property | type |
|---|---|---|
core/mcp-webhook-payload.json |
message, context_id, notification_id, token |
{"type": "string"} |
core/error.json |
field, suggestion |
{"type": "string"} |
core/error.json |
details |
{"type": "object"} |
core/pagination-request.json |
cursor |
{"type": "string"} |
core/mcp-webhook-payload.json requires [idempotency_key, operation_id, task_id, task_type, status, timestamp], so every field above is optional-and-not-nullable: the document admits absence and refuses null.
The fix costs nothing on the wire
Omitting the key instead of naming it with None produces the same bytes, because AdCPBaseModel already serialises with exclude_none. Measured, same payload both ways:
key omitted -> ['idempotency_key', 'operation_id', 'status', 'task_id', 'task_type', 'timestamp']
"message": None -> ['idempotency_key', 'operation_id', 'status', 'task_id', 'task_type', 'timestamp']
and the attribute a caller reads does not move either — the field's default is None, so payload.message is None with the key omitted.
feed_mirror.py already does this three lines below the site above, for account and if_wholesale_feed_version: the key is built into the dict only when it has a value. The same shape at the four sites would make the switch usable. For webhooks.py it is one comprehension over the optional keys.
Reproduction
# ADCP_STRICT_VALIDATION must be set before `import adcp` — the policy is read at import time
import os; os.environ["ADCP_STRICT_VALIDATION"] = "1"
from adcp.webhooks import create_mcp_webhook_payload
create_mcp_webhook_payload(
task_id="t-1", task_type="media_buy_delivery", status="completed",
idempotency_key="abcdefghijklmnop", operation_id="op-1",
)The question
Is ADCP_STRICT_VALIDATION meant to be usable by an adopter's CI on a tree that calls these helpers? Its docstring says yes, and today it is not. If the intent is instead that an explicit None from Python code is always "not supplied", then _RefuseJsonNull's coupling to _EXTRA_POLICY looks like the thing to reconsider rather than the four call sites — the switch is documented as being about unknown fields, and it also changes null handling.
Either way the omission at those four sites seems worth having on its own: it is byte-identical on the wire, it matches what feed_mirror.py already does for its sibling keys, and it is what the documents state.
Activity