Skip to content

Commit 772ac76

Browse files
l0lawrenceCopilotmsyyc
authored andcommitted
[python] fix TypedDict rendering for template-instantiated models (#11161)
fix #11149 ## Why PR #10439 enabled TypedDict body-overload generation in the default `dpg` mode. In real Azure mgmt libraries (e.g. `azure-mgmt-netapp`), this produced dangling `_types.CacheUpdate` references that were absent from `types.py`, causing an `AttributeError` at import time. ## Root cause Template-instantiated models all share the *template's* `crossLanguageDefinitionId`. For example, every `Azure.ResourceManager.Foundations.ResourceUpdateModel<T, P>` instantiation (`CacheUpdate`, `VolumeUpdate`, `ActiveDirectoryConfigUpdate`, ...) carries `clid = Azure.ResourceManager.Foundations.ResourceUpdateModel`. The `typeddict_models` dedup keyed the dpg-vs-copy pairing on that `clid`, so it treated genuinely distinct models as duplicates and skipped all but the first one. In netapp, `ActiveDirectoryConfigUpdate` was seen first, so `CacheUpdate` was dropped from `types.py` while still being referenced via `_types.CacheUpdate`. ## Fix - Key the dpg-vs-copy pairing on the model `name` instead of `crossLanguageDefinitionId`. The typeddict copy is a shallow copy of the source, so it shares the source's name; distinct template instantiations have distinct names, and model names are unique within a `types.py` module. - Harden `_find_existing_typeddict` in preprocess to also match on `name` (same root cause, tightly coupled). This keeps TypedDict generation on in `dpg` mode (intentional per #10439) and just corrects the rendering so referenced TypedDicts actually land in `types.py`. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Yuchao Yan <yuchaoyan@microsoft.com>
1 parent 89c1316 commit 772ac76

6 files changed

Lines changed: 584 additions & 29 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
changeKind: fix
3+
packages:
4+
- "@typespec/http-client-python"
5+
---
6+
7+
Fix dangling `_types.X` references when template-instantiated models (e.g. `ResourceUpdateModel<Foo, FooProperties>`) share a `crossLanguageDefinitionId`. The TypedDict deduplication now pairs each model with its own copy by name, so distinct models such as `CacheUpdate` and `VolumeUpdate` are all rendered in `types.py`.
8+
9+
Also stop emitting unused TypedDicts for response-only models in `types.py`. Output-only models already render as classes in `models/` and are referenced via `_models.X`, so their TypedDict copies (e.g. `GetResponse`) were dead code. The set of TypedDicts (and discriminated-base union aliases) rendered in `types.py` is now the transitive closure of the request-body input models over their base classes, discriminated subtypes and property types. Input body overloads (including spread bodies whose usage lacks the `Input` flag) are still emitted, and any output-only model reachable from an input model — such as a discriminated subtype or an ARM `SystemData` property — is kept so no forward reference is left undefined. This fixes a `NameError` at import time when an output-only union alias (e.g. `Dinosaur = Union[TRex]`) referenced an excluded subtype, and a pyright `reportUndefinedVariable` error when an input model referenced an excluded property type (e.g. `SystemData`).

packages/http-client-python/generator/pygen/codegen/models/model_type.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ def __init__(
8585
def is_usage_output(self) -> bool:
8686
return bool(self.usage & UsageFlags.Output.value)
8787

88+
@property
89+
def is_usage_input(self) -> bool:
90+
return bool(self.usage & UsageFlags.Input.value)
91+
8892
@property
8993
def is_used_in_operations_via_types(self) -> bool:
9094
"""Whether this model would be imported from types.py (not models) in operations."""

packages/http-client-python/generator/pygen/codegen/serializers/types_serializer.py

Lines changed: 105 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
# --------------------------------------------------------------------------
66
import keyword
77
import re
8-
from typing import Optional
8+
from functools import cached_property
9+
from typing import Any, Optional
910
from ..models import ModelType, CodeModel
1011
from ..models.enum_type import EnumType
1112
from ..models.imports import FileImport, ImportType
@@ -71,39 +72,131 @@ def declare_literal_enum(self, enum: EnumType) -> str:
7172
values = [enum.get_declaration(v.value) for v in enum.values]
7273
return f"{enum.name} = Literal[{', '.join(values)}]"
7374

75+
@staticmethod
76+
def _renders_as_input_typeddict(m: "ModelType") -> bool:
77+
"""Whether a non-json model is a *seed* for the types.py input surface.
78+
79+
TypedDicts (and discriminated-base union aliases) in ``types.py`` describe request-body
80+
(*input*) shapes. Output-only models already render as classes in ``models/`` and are
81+
referenced via ``_models.*``, so a response-only model that nothing input references would be
82+
dead code. A model seeds the input surface when it is:
83+
84+
* a typeddict copy (``base == "typeddict"``) — these only exist as input body overloads
85+
(their ``usage`` may carry ``Spread``/``Json`` rather than ``Input``), or
86+
* ``is_typed_dict_only`` — includes every model in full ``typeddict`` mode (responses too),
87+
and input-only anonymous bodies, or
88+
* used as input (``is_usage_input``) — e.g. a model shared between request and response.
89+
90+
The full set rendered in types.py is the transitive closure of these seeds over base
91+
classes, discriminated subtypes and property types (see :attr:`_types_file_model_names`), so
92+
an output-only model that *is* referenced by an input model (e.g. ARM ``SystemData`` on
93+
``Resource``) is still rendered.
94+
"""
95+
return m.base == "typeddict" or m.is_typed_dict_only or m.is_usage_input
96+
97+
@staticmethod
98+
def _iter_referenced_models(base_type: Any):
99+
"""Yield ModelType instances directly referenced by a type, recursing through containers.
100+
101+
Handles list/dict ``element_type``, constant/enum ``value_type`` and combined ``types``.
102+
A referenced ModelType is yielded but not descended into — the closure walk descends into a
103+
model's own properties/parents/subtypes when that model is itself visited.
104+
"""
105+
stack = [base_type]
106+
seen: set[int] = set()
107+
while stack:
108+
t = stack.pop()
109+
if t is None or id(t) in seen:
110+
continue
111+
seen.add(id(t))
112+
if isinstance(t, ModelType):
113+
yield t
114+
continue
115+
for attr in ("element_type", "value_type"):
116+
child = getattr(t, attr, None)
117+
if child is not None:
118+
stack.append(child)
119+
for child in getattr(t, "types", []) or []:
120+
stack.append(child)
121+
122+
@cached_property
123+
def _types_file_model_names(self) -> set[str]:
124+
"""Names of every model that must be rendered in types.py.
125+
126+
Starts from the input seeds (:meth:`_renders_as_input_typeddict`) and takes the transitive
127+
closure over base classes, discriminated subtypes and property types. Keyed on model
128+
``name`` (dpg models and their typeddict copies share a name and render as one TypedDict), so
129+
the result is stable regardless of which copy a reference points at.
130+
131+
Cached: the closure is a full walk over the model graph and is read several times per file
132+
(via :attr:`typeddict_models` and :attr:`discriminated_base_models`). ``self._models`` is set
133+
once at construction and never mutated, so memoizing on the instance is safe.
134+
"""
135+
needed: set[str] = set()
136+
stack = [m for m in self._models if m.base != "json" and self._renders_as_input_typeddict(m)]
137+
while stack:
138+
m = stack.pop()
139+
if m.base == "json" or m.name in needed:
140+
continue
141+
needed.add(m.name)
142+
stack.extend(m.parents)
143+
stack.extend(m.discriminated_subtypes.values())
144+
for prop in m.properties:
145+
stack.extend(self._iter_referenced_models(prop.type))
146+
return needed
147+
74148
@property
75149
def typeddict_models(self) -> list[ModelType]:
76150
"""Models that should be rendered as TypedDicts (excluding discriminated bases which become unions).
77151
78-
When both a dpg model and its typeddict copy exist (same crossLanguageDefinitionId),
152+
When both a dpg model and its typeddict copy exist for the same model,
79153
prefer the dpg model (it already renders as a TypedDict in types.py) and skip the copy.
154+
155+
The pairing is keyed on the model ``name`` (the copy is a shallow copy of the source, so it
156+
shares the source's name). ``crossLanguageDefinitionId`` cannot be used here: template
157+
instantiated models such as ``ResourceUpdateModel<Foo, FooProperties>`` all share the
158+
template's cross-language id, so keying on it would wrongly collapse distinct models
159+
(e.g. ``CacheUpdate`` and ``VolumeUpdate``) into one and drop the rest from types.py.
160+
161+
Only models in the input-surface closure (:attr:`_types_file_model_names`) are rendered, so
162+
response-only models (e.g. ``GetResponse``) are dropped while models reachable from an input
163+
model — including output-only ones such as a discriminated subtype or an ARM ``SystemData``
164+
property — are kept, ensuring no forward reference is left undefined.
80165
"""
81-
candidates = [m for m in self._models if m.base != "json" and not m.discriminated_subtypes]
82-
seen_ids: dict[str, "ModelType"] = {}
166+
needed = self._types_file_model_names
167+
candidates = [
168+
m for m in self._models if m.base != "json" and not m.discriminated_subtypes and m.name in needed
169+
]
170+
seen_names: dict[str, "ModelType"] = {}
83171
result: list["ModelType"] = []
84172
for m in candidates:
85-
clid = m.yaml_data.get("crossLanguageDefinitionId")
86-
if clid and clid in seen_ids:
173+
name = m.name
174+
if name in seen_names:
87175
# Prefer the dpg model over the typeddict copy
88-
if m.base == "dpg" and seen_ids[clid].base == "typeddict":
176+
if m.base == "dpg" and seen_names[name].base == "typeddict":
89177
# Replace the typeddict copy with the dpg model
90-
result = [r if r is not seen_ids[clid] else m for r in result]
91-
seen_ids[clid] = m
178+
result = [r if r is not seen_names[name] else m for r in result]
179+
seen_names[name] = m
92180
# Otherwise skip this duplicate
93181
continue
94-
if clid:
95-
seen_ids[clid] = m
182+
seen_names[name] = m
96183
result.append(m)
97184
return result
98185

99186
@property
100187
def discriminated_base_models(self) -> list[ModelType]:
101188
"""Discriminated base models that become Union type aliases in types.py.
102189
190+
Only bases in the input-surface closure (:attr:`_types_file_model_names`) are emitted: an
191+
output-only ``Dinosaur = Union[TRex]`` alias is dead code and would reference subtype
192+
TypedDicts that are themselves (correctly) omitted from types.py, causing a ``NameError`` at
193+
import time.
194+
103195
Topologically sorted so that nested discriminated bases (e.g. Shark)
104196
are defined before their parents (e.g. Fish = Union[Salmon, Shark]).
105197
"""
106-
bases = [m for m in self._models if m.base != "json" and m.discriminated_subtypes]
198+
needed = self._types_file_model_names
199+
bases = [m for m in self._models if m.base != "json" and m.discriminated_subtypes and m.name in needed]
107200
base_names = {m.name for m in bases}
108201
sorted_bases: list[ModelType] = []
109202
visited: set[str] = set()

packages/http-client-python/generator/pygen/preprocess/__init__.py

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,18 @@ def is_tsp(self) -> bool:
307307
return self.options.get("tsp_file", False)
308308

309309
@staticmethod
310-
def _find_existing_typeddict(code_model: dict[str, Any], cross_lang_id: Optional[str]) -> Optional[dict[str, Any]]:
311-
"""Find an existing typeddict copy with the given crossLanguageDefinitionId."""
310+
def _find_existing_typeddict(
311+
code_model: dict[str, Any],
312+
cross_lang_id: Optional[str],
313+
name: Optional[str] = None,
314+
) -> Optional[dict[str, Any]]:
315+
"""Find an existing typeddict copy for the given model.
316+
317+
Matches on both ``crossLanguageDefinitionId`` and ``name``. The name is required because
318+
template-instantiated models (e.g. ``ResourceUpdateModel<Foo, FooProperties>``) all share
319+
the template's cross-language id, so matching on the id alone would reuse one model's copy
320+
(e.g. ``CacheUpdate``) for a different model (e.g. ``VolumeUpdate``).
321+
"""
312322
if not cross_lang_id:
313323
return None
314324
return next(
@@ -318,10 +328,50 @@ def _find_existing_typeddict(code_model: dict[str, Any], cross_lang_id: Optional
318328
if t.get("type") == "model"
319329
and t.get("base") == "typeddict"
320330
and t.get("crossLanguageDefinitionId") == cross_lang_id
331+
and (name is None or t.get("name") == name)
321332
),
322333
None,
323334
)
324335

336+
@staticmethod
337+
def _find_spread_original(code_model: dict[str, Any], json_model: dict[str, Any]) -> Optional[dict[str, Any]]:
338+
"""Recover the dpg model that a spread (json) body was cloned from.
339+
340+
When a spread body type is also used elsewhere, the emitter clones it, renames the clone to
341+
``<Method>Request`` and sets ``base = "json"`` while keeping the original
342+
``crossLanguageDefinitionId``. To reference the real model's TypedDict we look the clone up
343+
by that id.
344+
345+
Distinct template-instantiated models share a single ``crossLanguageDefinitionId``, so an id
346+
match alone is ambiguous. We only reuse an original when the choice is unambiguous:
347+
348+
* a dpg candidate whose ``name`` equals the json model's name (the body was not renamed), or
349+
* exactly one dpg candidate carries the id.
350+
351+
Otherwise we return ``None`` so the caller falls back to the json model itself, avoiding a
352+
reference to the wrong model's TypedDict.
353+
"""
354+
cross_lang_id = json_model.get("crossLanguageDefinitionId")
355+
if not cross_lang_id:
356+
return None
357+
candidates = [
358+
t
359+
for t in code_model["types"]
360+
if t.get("type") == "model"
361+
and t.get("base") == "dpg"
362+
and t.get("crossLanguageDefinitionId") == cross_lang_id
363+
and t is not json_model
364+
]
365+
if not candidates:
366+
return None
367+
name = json_model.get("name")
368+
for candidate in candidates:
369+
if candidate.get("name") == name:
370+
return candidate
371+
if len(candidates) == 1:
372+
return candidates[0]
373+
return None
374+
325375
@staticmethod
326376
def _insert_typeddict_overload(
327377
code_model: dict[str, Any],
@@ -376,25 +426,14 @@ def add_body_param_type(
376426
# Add typeddict overload for non-spread dpg models
377427
if self.options["models-mode"] == "dpg" and is_dpg_model:
378428
cross_lang_id = model_type.get("crossLanguageDefinitionId")
379-
existing_td = self._find_existing_typeddict(code_model, cross_lang_id)
429+
existing_td = self._find_existing_typeddict(code_model, cross_lang_id, model_type.get("name"))
380430
self._insert_typeddict_overload(code_model, body_parameter, model_type, origin_type, existing_td)
381431

382432
# For spread bodies (json base), add a typeddict overload that references
383433
# the original model. This replaces the JSON single-body overload.
384434
if is_json_model:
385435
cross_lang_id = model_type.get("crossLanguageDefinitionId")
386-
original = None
387-
if cross_lang_id:
388-
original = next(
389-
(
390-
t
391-
for t in code_model["types"]
392-
if t.get("type") == "model"
393-
and t.get("crossLanguageDefinitionId") == cross_lang_id
394-
and t is not model_type
395-
),
396-
None,
397-
)
436+
original = self._find_spread_original(code_model, model_type)
398437

399438
if is_typeddict_only and original:
400439
# In typeddict-only mode, the original dpg model already renders
@@ -407,7 +446,7 @@ def add_body_param_type(
407446
body_parameter["type"]["types"].insert(1, td_list_or_dict)
408447
else:
409448
source = original or model_type
410-
existing_td = self._find_existing_typeddict(code_model, cross_lang_id)
449+
existing_td = self._find_existing_typeddict(code_model, cross_lang_id, source.get("name"))
411450
self._insert_typeddict_overload(code_model, body_parameter, source, origin_type, existing_td)
412451

413452
if len(body_parameter["type"]["types"]) == 1:
@@ -420,7 +459,6 @@ def add_body_param_type(
420459

421460
code_model["types"].append(body_parameter["type"])
422461

423-
424462
def pad_reserved_words(self, name: str, pad_type: PadType, yaml_type: dict[str, Any]) -> str:
425463
# we want to pad hidden variables as well
426464
if not name:

0 commit comments

Comments
 (0)