fix(canonical-formats): compare every parameter a format reference declares

src/adcp/canonical_formats/compat_helpers.py

@@ -28,6 +28,27 @@
     r"^display_(?P<width>[1-9][0-9]*)x(?P<height>[1-9][0-9]*)(?:_image)?$"
 )
 
+#: The parameters a format reference carries beyond its ``(agent_url, id)``
+#: identity, derived from the model instead of listed.
+#:
+#: A hand-written list here is the defect that dropped ``pixel_ratio``: three
+#: of the four parameters were copied by name, so a creative that claimed no
+#: pixel density compared equal to a product format that requires 2x, two
+#: references declaring *conflicting* densities compared equal, and the
+#: legacy-size upgrade discarded the density it was handed. Deriving the set
+#: means a parameter added to ``core/format-id.json`` — and so to the
+#: generated model — is compared without being named here.
+_PARAMETER_FIELDS: tuple[str, ...] = tuple(sorted(set(FormatId.model_fields) - {"agent_url", "id"}))
+
+#: Parameters the schema gives a value to when they are absent.
+#:
+#: ``core/format-id.json`` describes omitting ``pixel_ratio`` as selecting
+#: "the backward-compatible 1x variant", which is a value, not silence. An
+#: omitted width or duration means "template, unparameterized"; an omitted
+#: density means 1x. So a product fixed at 2x refuses a request that says
+#: nothing, while one fixed at 1x still serves it.
+_IMPLIED_WHEN_ABSENT: dict[str, float] = {"pixel_ratio": 1.0}
+
 
 def _coerce_format_id(
     value: FormatIdInput,
@@ -74,11 +95,14 @@ def _upgrade_legacy_format_id(
 
     return FormatId.model_validate(
         {
+            # Every parameter the reference carried, then the two the legacy
+            # size ID itself states. Reparameterizing the ID must not drop a
+            # parameter the caller supplied alongside it.
+            **{field: getattr(fid, field) for field in _PARAMETER_FIELDS},
             "agent_url": str(fid.agent_url),
             "id": "display_image",
             "width": int(match.group("width")),
             "height": int(match.group("height")),
-            "duration_ms": fid.duration_ms,
         }
     )
 
@@ -110,8 +134,9 @@ def formats_are_equivalent(
     Both inputs are first passed through :func:`upgrade_legacy_format_id`.
     Declared parameters must not conflict, but an omitted parameter on either
     side is treated as unspecified rather than a mismatch. Use
-    :func:`format_is_supported` for product/capability gating where a
-    supported fixed size or duration requires the request to state that value.
+    :func:`format_is_supported` for product/capability gating, where a
+    supported fixed size, duration or pixel density requires the request to
+    state that value.
     """
     left = _upgrade_legacy_format_id(a, default_agent_url=default_agent_url, param="a")
     right = _upgrade_legacy_format_id(b, default_agent_url=default_agent_url, param="b")
@@ -120,7 +145,7 @@ def formats_are_equivalent(
     if left.id != right.id:
         return False
 
-    for field in ("width", "height", "duration_ms"):
+    for field in _PARAMETER_FIELDS:
         left_value = getattr(left, field)
         right_value = getattr(right, field)
         if left_value is not None and right_value is not None and left_value != right_value:
@@ -139,8 +164,14 @@ def format_is_supported(
     This is intentionally stricter than :func:`formats_are_equivalent`.
     A broad supported format such as ``display_image`` accepts a specific
     request such as ``display_image`` 300x250, but a fixed supported product
-    format requires the request to provide and match every fixed parameter
-    (``width``, ``height``, and ``duration_ms``).
+    format requires the request to provide and match every parameter the
+    supported side fixes — every parameter ``core/format-id.json`` declares,
+    not a chosen few.
+
+    ``pixel_ratio`` is the one parameter whose absence carries a value rather
+    than meaning "unspecified": the schema describes omitting it as selecting
+    the backward-compatible 1x variant. A product fixed at 2x therefore
+    refuses a request that omits the field, and one fixed at 1x accepts it.
     """
     req = _upgrade_legacy_format_id(
         requested, default_agent_url=default_agent_url, param="requested"
@@ -151,11 +182,14 @@ def format_is_supported(
     if not formats_are_equivalent(req, sup, default_agent_url=default_agent_url):
         return False
 
-    for field in ("width", "height", "duration_ms"):
+    for field in _PARAMETER_FIELDS:
         supported_value = getattr(sup, field)
         if supported_value is None:
             continue
-        if getattr(req, field) != supported_value:
+        requested_value = getattr(req, field)
+        if requested_value is None:
+            requested_value = _IMPLIED_WHEN_ABSENT.get(field)
+        if requested_value != supported_value:
             return False
     return True
 

tests/test_canonical_formats_compatibility.py

@@ -16,6 +16,7 @@
 )
 from adcp.types import FormatCard
 from adcp.types.legacy import LegacyFormatId as FormatId
+from adcp.validation.schema_loader import get_named_schema_document
 
 
 def test_upgrade_legacy_display_size_to_parameterized_canonical_format_id() -> None:
@@ -186,3 +187,98 @@ def test_projection_accepts_a_reference_taken_off_a_model_field() -> None:
 
     assert projected.diagnostic is None
     assert projected.declaration is not None
+
+
+def _schema_parameters() -> frozenset[str]:
+    """The parameters ``core/format-id.json`` declares beyond the identity pair.
+
+    Read from the bundled schema rather than from the generated model, so the
+    guard below fails when a helper stops comparing a parameter the *spec*
+    declares — not merely when it disagrees with the model it was written
+    against.
+    """
+    document = get_named_schema_document("core/format-id.json")
+    assert document is not None, "core/format-id.json is absent from the bundled schema tree"
+    return frozenset(document["properties"]) - frozenset(document["required"])
+
+
+#: One pair of conflicting, dependency-legal values per declared parameter.
+#: ``pixel_ratio`` needs ``width``/``height`` present, which ``_BASE`` supplies.
+_CONFLICTING_VALUES: dict[str, tuple[object, object]] = {
+    "width": (300, 728),
+    "height": (250, 90),
+    "duration_ms": (15000, 30000),
+    "pixel_ratio": (2, 3),
+}
+
+_BASE = {
+    "agent_url": CANONICAL_CREATIVE_AGENT_URL,
+    "id": "display_image",
+    "width": 300,
+    "height": 250,
+}
+
+
+def test_the_conflict_table_covers_every_parameter_the_schema_declares() -> None:
+    """Keeps the parametrized guard below honest as the schema grows.
+
+    A new parameter in ``core/format-id.json`` fails here until it is given a
+    conflicting pair, which then puts it under the comparison guard.
+    """
+    assert set(_CONFLICTING_VALUES) == set(_schema_parameters())
+
+
+@pytest.mark.parametrize("parameter", sorted(_CONFLICTING_VALUES))
+def test_the_helpers_refuse_a_conflict_in_every_declared_parameter(parameter: str) -> None:
+    """Two references that disagree about any declared parameter differ.
+
+    The comparison walks the parameters the model declares, so a parameter
+    added to the schema is compared without being named here.
+    """
+    supported_value, requested_value = _CONFLICTING_VALUES[parameter]
+    supported = {**_BASE, parameter: supported_value}
+    requested = {**_BASE, parameter: requested_value}
+
+    assert not formats_are_equivalent(requested, supported)
+    assert not format_is_supported(requested, supported)
+
+
+def test_format_is_supported_refuses_a_request_that_omits_a_required_pixel_ratio() -> None:
+    """A 2x product is not served by a creative that never claims a density.
+
+    ``core/format-id.json`` makes the omitted form the 1x variant ("Omit for
+    the backward-compatible 1x variant"), so silence here is a 1x asset being
+    delivered into a 2x slot, not an unspecified parameter.
+    """
+    supported = {**_BASE, "pixel_ratio": 2}
+
+    assert formats_are_equivalent(_BASE, supported)
+    assert not format_is_supported(_BASE, supported)
+
+
+def test_format_is_supported_accepts_the_omitted_form_of_a_1x_product() -> None:
+    """The mirror of the case above, and the reason it is not a plain equality.
+
+    A seller that spells ``pixel_ratio: 1`` publishes the same render as one
+    that omits it, so a request omitting the field must still be served.
+    """
+    assert format_is_supported(_BASE, {**_BASE, "pixel_ratio": 1})
+
+
+def test_upgrade_legacy_format_id_keeps_every_parameter_it_was_given() -> None:
+    """The legacy-size upgrade reparameterizes the ID; it drops no parameter."""
+    legacy = FormatId(
+        agent_url=CANONICAL_CREATIVE_AGENT_URL,
+        id="display_300x250",
+        width=300,
+        height=250,
+        duration_ms=15000,
+        pixel_ratio=2,
+    )
+
+    upgraded = upgrade_legacy_format_id(legacy)
+
+    assert upgraded.id == "display_image"
+    assert (upgraded.width, upgraded.height) == (300, 250)
+    assert upgraded.duration_ms == 15000
+    assert upgraded.pixel_ratio == 2