Repository navigation
bug(types): hand-written asset unions route 14 of 21 declared asset types; 7 silently fall through to UnknownFormatAsset #1486
Description
Summary
_forward_compat patches two hand-written open unions over the generated assets
annotations at import. Those unions are maintained by hand, and they have fallen behind the
bundled schema: of the 21 asset_type values core/format.json declares, Format.assets
names 14 and the RepeatableAssetGroup.assets group slots name 12.
The seven (and nine) remaining values do not fail loudly. They match the _unknown fallback
arm and validate as UnknownFormatAsset / UnknownGroupAsset, so a consumer silently receives
an untyped object where the schema promises a typed one.
Format.assets routes 14 asset_type values
RepeatableAssetGroup.assets routes 12 asset_type values
bundled schema declares 21
missing from Format.assets : card daast_tracker display_tag pixel_tracker
published_post vast_tracker zip
missing from group slots : brief card catalog daast_tracker display_tag
pixel_tracker published_post vast_tracker zip
Measured on a clean install of main at 21f4216ad, compared against the schema that same
install bundles (adcp/_schemas/3.2/core/format.json) — so this is an internal disagreement,
not a pin-version mismatch.
Why this is the unpaid half of #742
#742 ("Creative-format asset_type is a closed discriminated union — additive spec growth breaks
all response consumers") correctly identified that a closed generated union breaks consumers
when the spec grows additively, and #744 fixed it by replacing the generated union with a
hand-written open one carrying an UnknownFormatAsset fallback.
That bought openness and gave up completeness. Openness is now permanent and completeness is
now manual: every new asset_type in the schema requires someone to remember to add an arm, and
when they do not, the symptom is not a crash but a silent downgrade to the fallback arm. The
present 7/9 gap is that cost arriving.
The fix is not to reclose the union. It is to derive the open union from the schema, so one
derivation produces both properties — every declared type gets its typed arm, and the _unknown
arm still absorbs values the schema does not yet define.
Reproduction — no server, no database, no network
"""Routed-vs-declared asset vocabulary for the patched `assets` unions."""
import importlib, json, pathlib
from typing import Annotated, get_args, get_origin
# Arming the patch matters: importing `adcp.types` alone does NOT apply it.
importlib.import_module("adcp.types._forward_compat")
# Target exactly what _forward_compat patches. Note the public `adcp.types.Format`
# is a DIFFERENT class (canonical_creative.Format) with no `assets` field at all.
from adcp.types.domains.core.format import Format
from adcp.types.aliases import RepeatableAssetGroup
def _unwrap(ann):
"""Peel Annotated, Optional, and list[...] in any order.
The two fields differ in shape -- `list[FormatAssetUnion] | None` versus a bare
`list[GroupFormatAssetUnion]` -- so a fixed-depth unwrap reads one correctly and
the other as empty.
"""
for _ in range(8):
if get_origin(ann) is Annotated:
ann = get_args(ann)[0]; continue
args = get_args(ann)
nonnull = [a for a in args if a is not type(None)]
if args and len(nonnull) == 1 and len(args) != len(nonnull):
ann = nonnull[0]; continue
if get_origin(ann) is list:
ann = get_args(ann)[0]; continue
break
return ann
def routed(model, field):
tags = set()
inner = _unwrap(model.model_fields[field].annotation)
for arm in get_args(inner) or (inner,):
base = get_args(arm)[0] if get_origin(arm) is Annotated else arm
f = getattr(base, "model_fields", {}).get("asset_type")
if f is None:
continue
if isinstance(getattr(f, "default", None), str):
tags.add(f.default)
else:
tags.update(a for a in get_args(f.annotation) if isinstance(a, str))
return tags
def declared(path):
found = set()
def walk(n):
if isinstance(n, dict):
for k, v in n.items():
if k == "asset_type" and isinstance(v, dict):
if "const" in v:
found.add(v["const"])
found.update(v.get("enum") or [])
walk(v)
elif isinstance(n, list):
for v in n:
walk(v)
walk(json.loads(pathlib.Path(path).read_text()))
return found
if __name__ == "__main__":
import adcp, glob
root = pathlib.Path(adcp.__file__).parent
schema = glob.glob(str(root / "**" / "core" / "format.json"), recursive=True)[0]
fmt, grp, dec = routed(Format, "assets"), routed(RepeatableAssetGroup, "assets"), declared(schema)
# A zero is an instrument failure, never a result.
assert fmt and grp and dec, "empty population - the walk failed"
print("Format.assets routes %2d" % len(fmt))
print("RepeatableAssetGroup.assets routes %2d" % len(grp))
print("bundled schema declares %2d" % len(dec))
print("missing from Format.assets :", sorted(dec - fmt))
print("missing from group slots :", sorted(dec - grp))Output on main @ 21f4216ad:
Format.assets routes 14
RepeatableAssetGroup.assets routes 12
bundled schema declares 21
missing from Format.assets : ['card', 'daast_tracker', 'display_tag', 'pixel_tracker', 'published_post', 'vast_tracker', 'zip']
missing from group slots : ['brief', 'card', 'catalog', 'daast_tracker', 'display_tag', 'pixel_tracker', 'published_post', 'vast_tracker', 'zip']
Why it is easy to miss, which is part of the report
Three properties each independently hide this, and together they make a casual probe return
"no defect":
- The checked-in generated source is not the runtime shape. Reading
src/adcp/types/domains/core/format.py— by grep or bygit showat any ref — measures the
side the patch overwrites. It reads a correct 21/21 while the runtime routes 14/12. - Importing the public surface does not arm the patch.
_apply_forward_compat()runs in
the module body ofadcp.types._forward_compat; importingadcp.typesalone does not pull
it in, so a probe against the public surface reports the generated (correct) annotation. - The public
Formatis a different class.adcp.types.Formatresolves to
adcp.types.canonical_creative.Format, which has noassetsfield, so the obvious probe
raisesKeyError: 'assets'rather than reporting a number.
With three sides that can disagree pairwise — schema, generated, patched — a number is only
meaningful if you say which side it came from. This is also the family of #1141
("adcp.types.CreativeAsset resolves to a different type at runtime than under mypy").
Suggested direction
Generate the open unions during codegen and derive the routed vocabulary from the schema, so
both the typed arms and the _unknown fallback come from one derivation and the hand-maintained
list disappears. A guard that compares the routed vocabulary against the bundled schema — driven
off the schema side, with a non-empty assertion so an empty scan cannot pass green — keeps it
from drifting again.
We have this implemented and running against AdCP 3.2 in a fork, where the same reproduction
prints 21/21 with an empty missing list on both fields. Happy to open a PR if the direction is
agreeable.
Activity
KonstantinMirin commentedon Oct 13, 2026
Follow-up measurement that makes the fix much smaller than the "Suggested direction" above — codegen is already correct and complete on
main.adcp.types.domains.core.assets.asset_union(generated) already exports every class the unions omit:— the exact seven missing from
FormatAssetUnion, plusBriefAssetandCatalogAssetwhich the group union also omits. Measured bydir()on the installed module at21f4216ad.So nothing needs to be generated that is not generated already. The entire defect is that the hand-written
FormatAssetUnion/GroupFormatAssetUnioninaliases.pyenumerate a subset of the classes sitting beside them, by hand, and that enumeration has drifted.That points at a smaller and more durable fix than I suggested:
_unknownfallback arm so the openness fix(types): open Format.assets discriminated union — add UnknownFormatAsset fallback arm #744 bought is preserved.asset_typevocabulary — driven off the schema side, with a non-empty assertion so an empty scan cannot pass green.Point 2 matters more than point 1. Without it, any future mechanism that re-introduces a hand-maintained step drifts again in exactly this way, and the symptom stays invisible: the fallback arm means a missing type is a silent downgrade rather than an error.
KonstantinMirin commentedon Oct 13, 2026
Three corrections to my own comment above, two of which would mislead anyone acting on it. The defect and the numbers are unchanged; what was wrong is where I said the arms come from, and a loose line in the reproduction.
1. I named the wrong source module
I wrote that the missing classes already exist in
adcp.types.domains.core.assets.asset_union. That is wrong, and it reads as a misunderstanding of your own type split, so it is worth correcting precisely.Those classes exist, but they are not union arms and most cannot be: measured on a clean install of
21f4216ad,DaastAssethas noasset_typefield at all, so the family is not a uniformly discriminated one.The classes the unions actually route are the slot classes in
adcp/types/domains/core/format.py— theBaseIndividualAssetandBaseGroupAssetsubclasses, 21 and 21, each carrying theasset_typeLiteralthe generator pinned on it:My conclusion is unaffected and still holds: codegen is complete, nothing needs generating, and the entire defect is that the two hand-written unions enumerate a subset of generated classes. I had the right conclusion from the wrong evidence. (I see you already guard the distinction I got wrong, in
test_asset_aliases_stable.py::test_content_types_do_not_collide_with_format_assets.)2. The reproduction I posted is non-deterministic — please use this instead
My script selected the schema with
glob("**/core/format.json")[0].globis unsorted, and an install bundles four copies with different vocabularies, so the printed denominator depends on filesystem ordering:So the same script prints 21 on one machine and 15 on another, and a reviewer who happens to get 15 would reasonably conclude the report is wrong. Pin the bundle to the installed version instead — replace the schema-selection lines with:
With that, the run at
21f4216adis deterministic:routes 14 / routes 12 / declares 21, missing['card','daast_tracker','display_tag','pixel_tracker','published_post','vast_tracker','zip']fromFormat.assetsand those plus['brief','catalog']from the group slots.3. There are four hand-maintained lists, not two — and the other two decide routing first
Besides the two unions,
aliases.pycarries two frozensets (around lines 1909 and 1928) feeding the callableDiscriminator. The discriminator returns_unknownbefore the arms are consulted, so a change that replaces only the two unions and leaves those frozensets alone changes routing not at all. Anyone implementing the fix needs all four to come from one derivation, which is the substance of point 2 in my earlier comment — and is the stronger half of it.Sorry for the noise. The measurement was independently reproduced from a second clean clone at current
main(47084e83a, which the earlier comment predates) with identical results: 14, 12, 21 and the same missing lists.