feat(types)!: let the schema decide which unknown keys a model keeps

MIGRATION_v9_to_v10.md

@@ -28,3 +28,4 @@ the SDK-relevant inventory is [docs/adcp-3.3-sdk-inventory.md](docs/adcp-3.3-sdk
 | `model_dump()` writes wire names | A field the generator aliased because its wire key is not a Python identifier is emitted under the wire key without `by_alias=True`: `$schema` instead of `field_schema`, `pass` instead of `pass_`, `from` instead of `from_`, `media-buy` instead of `media_buy`, `_truncation` instead of `field_truncation`, `2xl` instead of `field_2xl`. Code that reads a dump by the Python name must read the wire name; pass `by_alias=False` to keep the SDK 9 output. Construction accepts both spellings (`validate_by_name` is on). |
 | A received `null` is re-emitted; a Python `None` keyword argument is "not supplied" | A property whose schema admits `null` and that arrived as `null` (`model_validate`, `model_validate_json`, nested validation) stays in `model_fields_set` and is written back as `null` by `model_dump()`. Passing `None` as a keyword argument to a constructor, assigning `None` to a field, or passing `None` in `model_copy(update=...)` now leaves the field *unset*: it is omitted from the dump, and `model_dump(exclude_unset=True)` no longer lists it. A `None` inside a dict you validate is a wire `null`. Properties whose schema does not admit `null` are unchanged (`NoExplicitNull` refuses a JSON `null` and never emits one). |
 | `model_dump()` leaves an unset schema default off | A property the schema lists outside `required` with a `default` (or a `const`) is emitted only when it was set: by the document that was parsed, by a keyword argument, or by assignment. `sandbox: false`, `timezone: "inventory_local"`, `assignment_limit: 50` and the other schema defaults no longer appear on a dump unless you set them; the attribute still reads the default. Required discriminators (`asset_type`, `pricing_model`) are emitted either way. Pass `exclude_unset=False` for the SDK 9 output. Golden-file tests that compare whole dumps need regenerating. If you relied on a default reaching the peer, set it explicitly. `model_dump_as_received()` is deprecated: it returns the same document as `model_dump(mode="json")` and is removed in SDK 11. Generated root models (a union root such as `LocalizedCreativeAsset`) now subclass `AdCPRootModel` and dump with `exclude_none=True` like every other AdCP model. |
+| Unknown keys on open objects are kept and written back | `AdCPBaseModel` defaults to `extra='allow'`: a property the pinned model does not declare (a field a newer spec added, a sibling arm's property the schema admits) survives `model_validate` → `model_dump` in `model_extra`, including a `null` value. Generated classes whose schema object is closed (`additionalProperties: false`) keep `extra='forbid'`. SDK 9 dropped unknown keys (`extra='ignore'`). If your code passed stray keyword arguments and relied on them being dropped, remove them; `ADCP_STRICT_VALIDATION=1` turns the default into `forbid` so tests find them. |

docs/validation-contract.md

@@ -129,7 +129,6 @@ with the issue that tracks it, and the list only shrinks:
 
 | class | what happens | where it stands |
 | --- | --- | --- |
-| `field_dropped` | `extra='ignore'` drops an unknown or sibling-arm property | #1614; not recoverable by any dump. No digest-covered schema is affected. |
 | `discriminator_emitted` | a document valid under one arm is routed to a sibling arm, whose required single-value `Literal` it does not carry, and the arm's injected default appears | #1414; closed by #1522's construction dispatch. |
 
 ## Wire-boundary behavior

src/adcp/types/base.py

@@ -97,30 +97,32 @@
 MessageFormatter = Callable[[Any], str]
 
 
-def _resolve_extra_policy() -> Literal["ignore", "forbid"]:
+def _resolve_extra_policy() -> Literal["allow", "forbid"]:
     """Choose the ``extra`` policy for :class:`AdCPBaseModel`.
 
-    ``ignore`` (default) silently drops unknown fields, preserving
-    forward compatibility when newer spec versions add fields. This is
-    the production-safe default — a client on spec N sending to a
-    server on spec N+1 keeps working.
+    ``allow`` (default) keeps an unknown field in ``model_extra`` and writes
+    it back on dump. The generator sets ``extra='forbid'`` on every class
+    whose schema object is closed (``additionalProperties: false``) and
+    ``extra='allow'`` where the schema says ``true``; a class whose schema
+    says nothing inherits this default, which is JSON Schema's own reading of
+    an absent ``additionalProperties``. A document that carried a property the
+    pinned model does not declare -- a field a newer spec added, a sibling
+    arm's property the schema admits -- therefore survives parse → dump
+    (#1614). SDK 9 dropped it (``ignore``).
 
-    ``forbid`` raises on unknown fields so CI catches the silent-drop
-    case that production-safe defaults obscure. Most useful during
-    upgrades: right after a major spec revision, run tests with
-    ``ADCP_STRICT_VALIDATION=1`` to surface every place the upgrade
-    dropped a renamed field, then ship with the flag unset for the
-    forward-compat default.
+    ``forbid`` raises on unknown fields so a test run catches a stray keyword
+    argument or a renamed field. Set ``ADCP_STRICT_VALIDATION=1`` in CI during
+    upgrades, then ship with the flag unset.
 
     Values accepted: ``"1"``, ``"true"``, ``"yes"``, ``"on"`` (any
     case). Anything else — including empty string and ``"0"`` — keeps
-    the ``ignore`` default.
+    the ``allow`` default.
     """
     raw = os.environ.get("ADCP_STRICT_VALIDATION", "").strip().lower()
-    return "forbid" if raw in {"1", "true", "yes", "on"} else "ignore"
+    return "forbid" if raw in {"1", "true", "yes", "on"} else "allow"
 
 
-_EXTRA_POLICY: Literal["ignore", "forbid"] = _resolve_extra_policy()
+_EXTRA_POLICY: Literal["allow", "forbid"] = _resolve_extra_policy()
 
 
 def _build_deferred_serializers(value: Any, seen: set[int]) -> None:
@@ -687,19 +689,41 @@ def _keep_opaque_nulls(
     return kept
 
 
+def _put_back_null_extras(
+    value: dict[str, Any], extra: dict[str, Any], info: SerializationInfo
+) -> None:
+    """Re-emit an unknown key whose value is ``null`` on an open model.
+
+    ``exclude_none`` drops it with the ``None`` fields; an unknown key is a
+    value the document carried, and the schema that admits it admits its
+    ``null`` (#1614).
+    """
+    include, exclude = info.include, info.exclude
+    for key, item in extra.items():
+        if (
+            item is None
+            and key not in value
+            and (include is None or key in include)
+            and (exclude is None or key not in exclude)
+        ):
+            value[key] = None
+
+
 class AdCPBaseModel(BaseModel):
     """Base model for AdCP types with spec-compliant serialization.
 
-    Defaults to ``extra='ignore'`` so unknown fields from newer spec
-    versions are silently dropped rather than causing validation
-    errors. Generated types whose schemas set
-    ``additionalProperties: true`` override this with ``extra='allow'``
-    in their own ``model_config``.
+    Defaults to ``extra='allow'``: an unknown field from a newer spec
+    version, or a sibling arm's property the schema admits, is kept in
+    ``model_extra`` and written back on dump instead of being dropped
+    (#1614). Generated types whose schemas close the object
+    (``additionalProperties: false``) set ``extra='forbid'`` in their own
+    ``model_config``; the rest follow the schema's reading of an absent
+    ``additionalProperties``, which is open.
 
     Set ``ADCP_STRICT_VALIDATION=1`` in the environment (``"1"``,
     ``"true"``, ``"yes"``, ``"on"`` are accepted) to flip the default
-    to ``extra='forbid'``. Use this during spec upgrades to catch
-    silently-dropped renamed fields in tests. See :func:`_resolve_extra_policy`.
+    to ``extra='forbid'``. Use this in tests to catch a stray keyword
+    argument or a renamed field. See :func:`_resolve_extra_policy`.
 
     .. important::
        The env var is resolved **once at module import time**. Set it
@@ -764,8 +788,10 @@ def _finish_wire_dump(self, value: Any, info: SerializationInfo) -> Any:
             return value
         cls = type(self)
         extra = self.__pydantic_extra__
-        if extra and info.exclude_none and not cls.model_fields:
-            return _keep_opaque_nulls(value, extra, info)
+        if extra and info.exclude_none:
+            if not cls.model_fields:
+                return _keep_opaque_nulls(value, extra, info)
+            _put_back_null_extras(value, extra, info)
         if not info.exclude_unset and not _EMIT_UNSET_DEFAULTS.get():
             self._drop_unset_defaults(value, info)
         if info.exclude_none:

tests/conformance/test_wire_round_trip.py

@@ -52,11 +52,6 @@
     "discriminator_emitted": (
         "a document routed to an arm it does not satisfy gains the arm's discriminator (#1414)"
     ),
-    # ``extra='ignore'`` drops a property the pinned model does not declare,
-    # and a union arm declares only its own properties while the schema's arm
-    # admits its siblings'. No digest-covered schema is affected at this pin;
-    # the hazard is a newer-spec field inside one. #1614.
-    "field_dropped": "unknown and sibling-arm properties are dropped (#1614)",
 }
 
 #: Schemas whose bytes the protocol binds: no difference of any class, under
@@ -183,13 +178,11 @@ def test_every_accepted_class_still_occurs() -> None:
     assert not unknown, f"ACCEPTED names classes the sweep does not produce: {unknown}"
 
 
-def test_dump_as_received_leaves_only_unknown_fields() -> None:
-    """The explicit "as received" dump recovers everything but what the model never held.
+def test_the_explicit_as_received_settings_leave_no_residue() -> None:
+    """``exclude_unset=True, exclude_none=False, by_alias=True`` is the document too.
 
-    ``AdCPBaseModel.model_dump_as_received`` leaves off unset defaults, keeps
-    an admitted explicit ``null`` and writes wire names, so the only residue is
-    a property the model dropped on input (#1614). This is the path a verifier
-    uses to recompute a digest over a parsed object today.
+    The explicit settings SDK 9's ``model_dump_as_received`` used recover the
+    same bytes as the default dump; nothing the model held is left behind.
     """
     residue: dict[str, set[str]] = defaultdict(set)
     for schema in SCHEMAS:
@@ -199,9 +192,9 @@ def test_dump_as_received_leaves_only_unknown_fields() -> None:
             for d in rt.round_trip(document, as_received=True)[0]:
                 residue[d.kind].add(schema)
     unexpected = {
-        kind: sorted(schemas) for kind, schemas in residue.items() if kind != "field_dropped"
+        kind: sorted(schemas) for kind, schemas in residue.items() if kind not in ACCEPTED
     }
-    assert not unexpected, f"dump-as-received residue beyond dropped fields: {unexpected}"
+    assert not unexpected, f"as-received residue: {unexpected}"
 
 
 def test_refusing_schemas_are_declared() -> None:

tests/test_extra_policy.py

@@ -36,26 +36,27 @@
 GENERATED_DIR = Path(__file__).parent.parent / "src" / "adcp" / "types" / "domains"
 
 
-def test_base_model_config_is_ignore() -> None:
-    """Sanity check: AdCPBaseModel must have extra='ignore'."""
-    assert AdCPBaseModel.model_config.get("extra") == "ignore", (
+def test_base_model_config_is_allow() -> None:
+    """Sanity check: AdCPBaseModel must have extra='allow' (SDK 10, #1614)."""
+    assert AdCPBaseModel.model_config.get("extra") == "allow", (
         f"AdCPBaseModel.model_config is {AdCPBaseModel.model_config!r} — "
         "is the package installed from the correct branch?"
     )
 
 
 class TestBaseModelDefault:
-    """AdCPBaseModel defaults to extra='ignore'."""
+    """AdCPBaseModel defaults to extra='allow'."""
 
-    def test_base_model_drops_extra_fields(self) -> None:
-        """Unknown fields are silently dropped, not stored or rejected."""
+    def test_base_model_keeps_extra_fields(self) -> None:
+        """Unknown fields are kept in model_extra and written back, not dropped (#1614)."""
 
         class DefaultType(AdCPBaseModel):
             name: str
 
-        obj = DefaultType(name="test", unknown_field="dropped")
+        obj = DefaultType(name="test", unknown_field="kept")
         assert obj.name == "test"
-        assert not hasattr(obj, "unknown_field")
+        assert obj.model_extra == {"unknown_field": "kept"}
+        assert obj.model_dump() == {"name": "test", "unknown_field": "kept"}
 
     def test_base_model_accepts_known_fields(self) -> None:
         class DefaultType(AdCPBaseModel):
@@ -77,16 +78,16 @@ class ExtensibleType(AdCPBaseModel):
         assert obj.name == "test"
         assert obj.extra_field == "stored"  # type: ignore[attr-defined]
 
-    def test_ignore_inherited_without_explicit_config(self) -> None:
-        """A subclass without its own model_config inherits ignore from base."""
+    def test_allow_inherited_without_explicit_config(self) -> None:
+        """A subclass without its own model_config inherits allow from base."""
 
         class InheritedType(AdCPBaseModel):
             name: str
             value: int
 
-        obj = InheritedType(name="test", value=1, surprise="dropped")
+        obj = InheritedType(name="test", value=1, surprise="kept")
         assert obj.name == "test"
-        assert not hasattr(obj, "surprise")
+        assert obj.model_extra == {"surprise": "kept"}
 
     def test_open_payload_field_preserves_extension_data(self) -> None:
         """x-adcp-open-payload fields keep arbitrary structured payload data."""

tests/test_open_objects.py

@@ -0,0 +1,110 @@
+"""An open object keeps the keys it does not declare (#1614).
+
+JSON Schema reads an absent ``additionalProperties`` as open, and the
+generator sets ``extra='forbid'`` only where a schema object is closed. In
+SDK 9 ``AdCPBaseModel`` defaulted to ``extra='ignore'``, so a property the
+pinned model did not declare -- a field a newer spec added, a sibling arm's
+property the schema admits -- was dropped on parse and absent after dump. SDK
+10 defaults to ``extra='allow'``: the key survives in ``model_extra`` and is
+written back, ``null`` included. ``tests/conformance/test_wire_round_trip.py``
+holds every generated model to it with no ``field_dropped`` allowance.
+"""
+
+from __future__ import annotations
+
+import json
+
+import pytest
+from pydantic import ConfigDict, ValidationError
+
+from adcp.types.base import AdCPBaseModel
+
+
+class Open(AdCPBaseModel):
+    x: int | None = None
+
+
+class Closed(AdCPBaseModel):
+    model_config = ConfigDict(extra="forbid")
+    x: int | None = None
+
+
+def test_the_default_policy_is_allow() -> None:
+    assert AdCPBaseModel.model_config["extra"] == "allow"
+    assert Open.model_config["extra"] == "allow"
+
+
+def test_an_unknown_key_survives_parse_and_dump() -> None:
+    document = {"x": 1, "added_in_3_4": {"nested": [1, 2]}, "nothing": None}
+    parsed = Open.model_validate_json(json.dumps(document))
+    assert parsed.model_extra == {"added_in_3_4": {"nested": [1, 2]}, "nothing": None}
+    assert parsed.model_dump(mode="json") == document
+    assert json.loads(parsed.model_dump_json()) == document
+
+
+def test_include_and_exclude_apply_to_a_null_unknown_key() -> None:
+    parsed = Open.model_validate({"x": 1, "nothing": None})
+    assert parsed.model_dump(mode="json", exclude={"nothing"}) == {"x": 1}
+    assert parsed.model_dump(mode="json", include={"x"}) == {"x": 1}
+
+
+def test_a_stray_keyword_argument_goes_to_the_wire() -> None:
+    """Documented: a closed schema object refuses it; an open one carries it."""
+    assert Open(x=1, stray="s").model_dump(mode="json") == {"x": 1, "stray": "s"}
+    with pytest.raises(ValidationError):
+        Closed(x=1, stray="s")
+
+
+def test_a_generated_closed_object_refuses_an_unknown_key() -> None:
+    from adcp.types.domains.media_buy.request_proposals_response import PurchaseContinuation1
+
+    assert PurchaseContinuation1.model_config["extra"] == "forbid"
+    with pytest.raises(ValidationError):
+        PurchaseContinuation1.model_validate(
+            {
+                "kind": "legacy_create",
+                "continuation_token": "0123456789abcdef",
+                "continuation_expires_at": "2026-10-01T00:00:00Z",
+                "source_adcp_version": "3.1",
+                "product_ids": ["prod_1"],
+                "losses": ["feed_version_not_atomic", "pricing_version_not_atomic"],
+                "requires_explicit_acceptance": True,
+                "unknown": 1,
+            }
+        )
+
+
+def test_a_generated_open_object_keeps_an_unknown_key() -> None:
+    from adcp.types.domains.core.targeting_input import TargetingOverlayInput
+
+    assert TargetingOverlayInput.model_config["extra"] == "allow"
+    document = {"geo_regions": ["US-NY"], "added_in_3_4": {"nested": None}, "nothing": None}
+    parsed = TargetingOverlayInput.model_validate_json(json.dumps(document))
+    assert parsed.model_extra == {"added_in_3_4": {"nested": None}, "nothing": None}
+    assert parsed.model_dump(mode="json") == document
+
+
+def test_the_generated_tree_follows_its_schemas() -> None:
+    """Closed schema objects forbid; everything else allows. Nothing ignores."""
+    import importlib
+    import pkgutil
+
+    import adcp.types.domains as domains
+
+    policies: dict[str, int] = {}
+    for info in pkgutil.walk_packages(domains.__path__, prefix=domains.__name__ + "."):
+        try:
+            module = importlib.import_module(info.name)
+        except ImportError:
+            continue
+        for obj in list(vars(module).values()):
+            if (
+                isinstance(obj, type)
+                and issubclass(obj, AdCPBaseModel)
+                and obj.__module__ == module.__name__
+            ):
+                policy = str(obj.model_config.get("extra"))
+                policies[policy] = policies.get(policy, 0) + 1
+    assert set(policies) <= {"allow", "forbid"}, policies
+    assert policies.get("ignore", 0) == 0
+    assert policies["allow"] > 1000 and policies["forbid"] > 500, policies

tests/test_strict_validation_env.py

@@ -30,15 +30,15 @@ def test_falsy_or_unknown_values_keep_default(value: str) -> None:
     forward-compat ``ignore`` default. Whitespace-around-1 is caught by
     strip(), but ``"tRUE "`` specifically tests that trailing space is
     tolerated."""
-    expected = "forbid" if value.strip().lower() in {"1", "true", "yes", "on"} else "ignore"
+    expected = "forbid" if value.strip().lower() in {"1", "true", "yes", "on"} else "allow"
     with patch.dict("os.environ", {"ADCP_STRICT_VALIDATION": value}):
         assert _resolve_extra_policy() == expected
 
 
 def test_unset_var_keeps_default() -> None:
     """No env var at all → default (forward-compat safe)."""
     with patch.dict("os.environ", {}, clear=True):
-        assert _resolve_extra_policy() == "ignore"
+        assert _resolve_extra_policy() == "allow"
 
 
 def test_default_is_ignore_for_production_forward_compat() -> None:
@@ -51,7 +51,7 @@ def test_default_is_ignore_for_production_forward_compat() -> None:
     # In the test environment with no ADCP_STRICT_VALIDATION set, it
     # must be ``ignore`` — a regression here indicates the default
     # flipped.
-    assert _EXTRA_POLICY in {"ignore", "forbid"}
+    assert _EXTRA_POLICY in {"allow", "forbid"}
     # If this assertion fails, the test environment has the env var
     # set — check your shell, not the SDK.