fix(codegen)!: refuse a JSON null on optional properties whose schema forbids one

docs/design/type-generation.md

@@ -139,6 +139,22 @@ Each of these operates on the staging copy only. The cached bundle, the schemas
 ships and any direct-from-schema validation are untouched, which is what keeps the
 transforms from changing what the SDK validates against.
 
+One more transform runs once over the whole staged tree after every file is written,
+because it has to resolve `$ref`s into the documents the generator will read:
+`mark_no_explicit_null_properties`. JSON Schema separates two things the generator
+conflates — a property outside `required` may be *absent*, and `null` is a valid *value*
+only where the declared type admits it — and the generator renders both as
+`T | None = None`, so the model accepted `{"creatives": null}` where the bundled validator
+reports a `type` error (#1547). The pass grades every optional property with the same
+`jsonschema` validator the bundled arm uses (`is_valid(None)` on the property node, refs
+resolved against the staged tree), and marks the ones that refuse `null` with
+`x-adcp-no-explicit-null: true`. The generator carries the mark into
+`Field(json_schema_extra=...)` via `--field-extra-keys`, and the first post-generation fix
+turns it into the `NoExplicitNull` annotation (below). The key exists only in the staged
+input; the shipped schemas never carry it. A property whose schema admits `null` — a type
+list, an `anyOf` with a null arm, a `$ref` to a nullable schema — is not marked and keeps
+accepting it.
+
 Two of them carry explicit ceilings rather than trusting a bump not to move the problem.
 `normalize_version_envelope_composition` raises if it would rewrite more than 12 schemas:
 *"A bump changed the shape of the problem: re-measure which schemas declare the version
@@ -159,8 +175,18 @@ on the file being present and absent.
 `scripts/post_generate_fixes.py` is 7367 lines holding 130 module-level functions: 72 fix
 entrypoints, built as one ordered list in `main()` and run in list order, plus 58 helpers.
 
-Three ordering constraints are commented in the list itself and must be preserved:
-
+Four ordering constraints are commented in the list itself and must be preserved:
+
+- `point_omittable_fields_at_no_explicit_null` runs first. It removes the
+  `json_schema_extra={'x-adcp-no-explicit-null': True}` keyword the generator emitted for
+  every marked property and appends `adcp.types.base.NoExplicitNull` to the field's
+  `Annotated` metadata, re-rendering the field the way black lays it out, so every later
+  fix reads exactly the text the generator produced on the pinned flags plus that one
+  element. The annotation stays `T | None = None`: omitted still reads as `None`, a JSON
+  `null` is refused (with the type's own error; a Python `None` stays the spelling of absence
+  unless `ADCP_STRICT_VALIDATION` is set), and the `null` arm leaves the published JSON
+  Schema. A marked field whose annotation already refuses `None` (a
+  `Literal` with a default, a field an `allOf` arm made required) only loses the keyword.
 - `point_integer_fields_at_the_schema_integer_type` retypes every `type: integer`,
   including an integer root's own `root` annotation, so it must precede
   `rewrite_scalar_rootmodels` — run after, and an integer root is still spelled

docs/validation-contract.md

@@ -13,6 +13,23 @@ another composition keyword. `x-adcp-validation` carries behavioral rules that
 neither Pydantic nor a standard JSON Schema validator interprets generically;
 the SDK implements security- and commitment-critical instances explicitly.
 
+Absence and `null` are two different things, and the models keep them apart
+the way the schemas do. A property outside `required` may be omitted, and
+reads back as `None`; an explicit JSON `null` is a value, valid only where the
+schema's type admits it. The generator marks every optional property whose
+schema refuses `null` with `adcp.types.base.NoExplicitNull`: a JSON document
+carrying `null` there is refused (`model_validate_json`, with the type's own
+error, e.g. `list_type`) and `model_json_schema()` publishes no null arm, while
+the annotation stays `T | None = None`. A property whose schema lists `null`
+(`"type": ["string", "null"]`, an `anyOf` with a null arm) keeps accepting it.
+
+On the Python side `None` remains the spelling of "not supplied":
+`Model(field=None)` and `model_validate` on a dict accept it, it reads back as
+`None`, and `model_dump()` (which defaults to `exclude_none=True`) leaves it
+off the wire — the same document an omitted property produces. Set
+`ADCP_STRICT_VALIDATION=1` (the switch that also forbids unknown fields) to
+refuse a Python `None` as well, for suites that want to catch it.
+
 ## Wire-boundary behavior
 
 - SDK clients validate requests before sending and responses after receiving,

scripts/generate_types.py

@@ -19,6 +19,7 @@
 import subprocess
 import sys
 import tempfile
+import warnings
 from pathlib import Path
 from typing import Any
 
@@ -953,6 +954,153 @@ def flatten_root_object_validation_union(schema: dict, schema_path: Path) -> dic
     return schema
 
 
+#: Pre-generation mark on an optional property whose schema does not admit
+#: ``null``. The generator carries it into ``Field(json_schema_extra=...)``
+#: (``--field-extra-keys``), and ``post_generate_fixes.point_omittable_fields_at_no_explicit_null``
+#: turns that into the ``adcp.types.base.NoExplicitNull`` annotation. The key
+#: exists only in the temporary generator input; the shipped schemas never
+#: carry it.
+NO_EXPLICIT_NULL_KEY = "x-adcp-no-explicit-null"
+
+
+def _file_uri_resource(path: Path):
+    """Load *path* as a ``referencing`` resource addressed by its file URI.
+
+    The flattened tree spells every cross-file ``$ref`` as a relative path
+    (``../core/account_ref.json``). Addressing each document by its own file
+    URI, rather than the canonical ``$id`` the bundle declares, makes those
+    paths resolve against the tree the generator actually reads.
+    """
+    from referencing import Resource
+    from referencing.jsonschema import DRAFT7
+
+    with open(path) as f:
+        contents = json.load(f)
+    if isinstance(contents, dict):
+        contents["$id"] = path.resolve().as_uri()
+    return Resource.from_contents(contents, default_specification=DRAFT7)
+
+
+def _property_admits_null(validator, node: Any) -> bool:
+    """Whether the schema *node* accepts JSON ``null``, as the bundled validator decides.
+
+    Delegating to the validator rather than reading ``type`` by hand means every
+    nullable spelling the arm honours — a type list, ``anyOf`` with a null arm,
+    a ``$ref`` to a nullable schema, ``enum`` containing ``null`` — is honoured
+    here too, so the model and the JSON-Schema arm reach one verdict.
+    """
+    return bool(validator.evolve(schema=node).is_valid(None))
+
+
+def _required_names(node: Any, resolver, seen: frozenset[str] = frozenset()) -> set[str]:
+    """Every property *node* requires, including through ``allOf`` and ``$ref``.
+
+    A branch that composes a base schema (``allOf: [{"$ref": ...}]``) and
+    restates one of its properties leaves ``required`` to the base. The
+    generator merges the two and renders the field required; grading the
+    restated property as optional would mark it, and the mark alone is what
+    makes the generator stop merging it (#1547). So a property any composed
+    schema requires is not optional here either.
+    """
+    if not isinstance(node, dict):
+        return set()
+    required = set(node.get("required", []) if isinstance(node.get("required"), list) else [])
+    ref = node.get("$ref")
+    if isinstance(ref, str) and ref not in seen:
+        resolved = resolver.lookup(ref)
+        required |= _required_names(resolved.contents, resolved.resolver, seen | {ref})
+    for arm in node.get("allOf", []) if isinstance(node.get("allOf"), list) else []:
+        required |= _required_names(arm, resolver, seen)
+    return required
+
+
+def _mark_no_explicit_null(node: Any, validator, resolver, counter: list[int]) -> None:
+    if isinstance(node, list):
+        for item in node:
+            _mark_no_explicit_null(item, validator, resolver, counter)
+        return
+    if not isinstance(node, dict):
+        return
+    properties = node.get("properties")
+    if isinstance(properties, dict):
+        required_names = _required_names(node, resolver)
+        for name, property_schema in properties.items():
+            if name in required_names or not isinstance(property_schema, dict):
+                continue
+            if not _property_admits_null(validator, property_schema):
+                property_schema[NO_EXPLICIT_NULL_KEY] = True
+                counter[0] += 1
+    for key, value in node.items():
+        if key == NO_EXPLICIT_NULL_KEY:
+            continue
+        _mark_no_explicit_null(value, validator, resolver, counter)
+
+
+def mark_no_explicit_null_properties(temp_dir: Path) -> int:
+    """Mark every optional property whose schema refuses an explicit ``null``.
+
+    JSON Schema separates two things the generator conflates: a property
+    outside ``required`` may be *absent*, and ``null`` is a valid *value* only
+    where the declared type admits it. The generator renders both as
+    ``T | None = None``, so without this pass the model accepts
+    ``{"creatives": null}`` where the bundled schema — and so the SDK's own
+    ``validate_request`` — reports a ``type`` error (#1547).
+
+    This runs over the whole flattened tree once every file is written, so a
+    ``$ref`` resolves to the document the generator will read. Each property
+    is graded by the same ``jsonschema`` validator the bundled arm uses, and
+    a property it says cannot be ``null`` is marked with
+    :data:`NO_EXPLICIT_NULL_KEY`. A property whose schema admits ``null`` is
+    left alone and keeps accepting it. Returns the number of marked properties.
+    """
+    from jsonschema.validators import validator_for
+    from referencing import Registry
+
+    def retrieve(uri: str):
+        from urllib.parse import urlparse
+        from urllib.request import url2pathname
+
+        parsed = urlparse(uri)
+        if parsed.scheme == "file":
+            return _file_uri_resource(Path(url2pathname(parsed.path)))
+        # The references in PRESERVE_CANONICAL_URL_REFS keep their canonical
+        # URL; the generator reads those from a mirror of the same cache, and
+        # this pass reads them from the flattened tree the mirror was built from.
+        canonical = re.fullmatch(r"https://adcontextprotocol\.org/schemas/[^/]+/(.+)", uri)
+        if canonical is None:
+            raise LookupError(uri)
+        relative = Path(*(part.replace("-", "_") for part in Path(canonical.group(1)).parts))
+        return _file_uri_resource(temp_dir / relative)
+
+    registry = Registry(retrieve=retrieve)
+    marked = 0
+    for schema_file in sorted(temp_dir.rglob("*.json")):
+        with open(schema_file) as f:
+            schema = json.load(f)
+        if not isinstance(schema, dict):
+            continue
+        # Validate with a copy addressed by file URI; write the original back,
+        # so the mark is the only thing this pass adds to the generator input.
+        document = copy.deepcopy(schema)
+        document["$id"] = schema_file.resolve().as_uri()
+        with warnings.catch_warnings():
+            # A few root documents name themselves rather than a metaschema in
+            # ``$schema``; jsonschema grades those with its latest draft and
+            # says so. The verdict on ``null`` is the same under every draft.
+            warnings.simplefilter("ignore", DeprecationWarning)
+            validator_class = validator_for(document)
+        validator = validator_class(document, registry=registry)
+        resolver = registry.resolver(base_uri=document["$id"])
+        counter = [0]
+        _mark_no_explicit_null(schema, validator, resolver, counter)
+        if counter[0]:
+            with open(schema_file, "w") as f:
+                json.dump(schema, f, indent=2)
+            marked += counter[0]
+    print(f"  Marked {marked} optional properties that refuse an explicit null")
+    return marked
+
+
 def flatten_schemas(temp_dir: Path):
     """
     Copy schemas to temp directory, preserving directory structure.
@@ -1047,6 +1195,7 @@ def flatten_schemas(temp_dir: Path):
         print(f"  {rel_path}")
 
     count = len(schema_files)
+    mark_no_explicit_null_properties(temp_dir)
     if _VERSION_ENVELOPE_NORMALIZED > _VERSION_ENVELOPE_NORMALIZED_CEILING:
         raise ValueError(
             f"version-envelope normalization rewrote {_VERSION_ENVELOPE_NORMALIZED} schemas, "
@@ -1161,6 +1310,11 @@ def _run_datamodel_codegen(input_path: Path, output_path: Path) -> subprocess.Co
         "--set-default-enum-member",
         "--enum-field-as-literal",
         "one",
+        # Carry the optional-but-not-nullable mark set by
+        # ``mark_no_explicit_null_properties`` into ``Field(json_schema_extra=...)``,
+        # where the post-generation pass turns it into ``NoExplicitNull``.
+        "--field-extra-keys",
+        NO_EXPLICIT_NULL_KEY,
         "--allow-remote-refs",
     ]