Skip to content

Commit 19be8db

Browse files
bokelleyclaude
andauthored
feat(types): Sequence[T] on response-only list fields for covariant adoption (#635)
* feat(types): Sequence[T] on response-only list fields for covariant adoption Adopters who subclass SDK response models and narrow the element type (e.g. list[MyPackage] instead of list[Package]) get mypy[assignment] errors because list[T] is invariant. Changing affected_packages, media_buys, packages (on MediaBuy), and media_buy_deliveries to Sequence[T] removes those errors — Sequence is covariant so Sequence[MyPackage] is a valid subtype of Sequence[Package]. Response-only fields are safe because adopters receive these, they do not construct or mutate them. Request-side list fields (packages/ creatives on request types) remain list[T] because adopters call .append() on those. See issue #624 for the full 4-category analysis. The rewrite is implemented in post_generate_fixes.py via a new rewrite_response_list_to_sequence() function so it re-applies on every codegen run and survives schema regeneration. Closes #624 (Category A) https://claude.ai/code/session_019aVaC1DXjVH6Kan4gBe1sb * fix(types): Sequence consistency in ergonomic coercion + import ordering Fix two issues surfaced in pre-PR expert review: 1. _ergonomic.py was patching media_buy_deliveries back to list[MediaBuyDelivery] at import time, overwriting the Sequence[MediaBuyDelivery] annotation set by post_generate_fixes.py. The generate_ergonomic_coercion.py generator now detects Sequence[T] fields (via updated is_list_of) and emits Sequence[T] in the coercion patch, preserving source/runtime consistency. _ergonomic.py regenerated to reflect this. 2. Import ordering in generated files: from collections.abc import Sequence was inserted after from enum import Enum. Fixed insertion anchor in post_generate_fixes.py and corrected the two affected generated files. https://claude.ai/code/session_019aVaC1DXjVH6Kan4gBe1sb --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent edd7d0a commit 19be8db

6 files changed

Lines changed: 96 additions & 14 deletions

File tree

scripts/generate_ergonomic_coercion.py

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,26 @@ def get_base_type(annotation: Any) -> Any:
9595

9696

9797
def is_list_of(annotation: Any, item_check) -> tuple[bool, Any]:
98-
"""Check if annotation is list[X] where X passes item_check.
98+
"""Check if annotation is list[X] or Sequence[X] where X passes item_check.
9999
100-
Handles both list[X] and list[X] | None.
100+
Handles both T[X] and T[X] | None (where T is list or collections.abc.Sequence).
101101
"""
102-
# First check if the annotation itself is a list
102+
from collections.abc import Sequence as AbcSequence
103+
104+
_list_origins = (list, AbcSequence)
105+
106+
# First check if the annotation itself is a list/Sequence
103107
origin = get_origin(annotation)
104-
if origin is list:
108+
if origin in _list_origins:
105109
args = get_args(annotation)
106110
if args and item_check(args[0]):
107111
return True, args[0]
108112

109-
# Then check if it's Optional[list[X]] (i.e., list[X] | None)
113+
# Then check if it's Optional[list[X]] or Optional[Sequence[X]]
110114
base = get_base_type(annotation)
111115
if base is not None and base is not annotation:
112116
origin = get_origin(base)
113-
if origin is list:
117+
if origin in _list_origins:
114118
args = get_args(base)
115119
if args and item_check(args[0]):
116120
return True, args[0]
@@ -369,12 +373,15 @@ def _find_success_variant() -> type[_PydBaseModel]:
369373
"5. FieldModel (enum) lists accept string lists",
370374
"",
371375
"Note: List variance issues (list[Subclass] not assignable to list[BaseClass])",
372-
"are a fundamental Python typing limitation. Users extending library types",
373-
"should use Sequence[T] in their own code or cast() for type checker appeasement.",
376+
"are a fundamental Python typing limitation. Response-only container fields",
377+
"(affected_packages, media_buys, packages, media_buy_deliveries) already use",
378+
"Sequence[T] in their generated base class. For other fields not yet migrated,",
379+
"adopters should use Sequence[T] in their own code or cast() for appeasement.",
374380
'"""',
375381
"",
376382
"from __future__ import annotations",
377383
"",
384+
"from collections.abc import Sequence",
378385
"from typing import Annotated, Any",
379386
"",
380387
"from pydantic import BeforeValidator",
@@ -555,11 +562,18 @@ def _find_success_variant() -> type[_PydBaseModel]:
555562
)
556563
lines.append(" )")
557564
elif c["type"] == "subclass_list":
565+
from collections.abc import Sequence as AbcSequence
566+
558567
target = c["target_class"].__name__
559-
# Check if the field is required (no | None)
560568
field_info = cls.model_fields[field]
561569
is_optional = "None" in str(field_info.annotation)
562-
type_str = f"list[{target}] | None" if is_optional else f"list[{target}]"
570+
# Preserve Sequence[T] when the field already uses it (covariant
571+
# inheritance, set by post_generate_fixes.rewrite_response_list_to_sequence).
572+
ann = field_info.annotation
573+
base_ann = get_base_type(ann)
574+
is_seq = get_origin(base_ann if base_ann is not None else ann) is AbcSequence
575+
container = "Sequence" if is_seq else "list"
576+
type_str = f"{container}[{target}] | None" if is_optional else f"{container}[{target}]"
563577
lines.append(" _patch_field_annotation(")
564578
lines.append(f" {type_name},")
565579
lines.append(f' "{field}",')

scripts/post_generate_fixes.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,67 @@ def add_rootmodel_getattr_proxy():
547547
print(" No RootModel union types needed __getattr__ proxy")
548548

549549

550+
# Response-only list fields changed to Sequence[T] so adopters can narrow the
551+
# element type without type: ignore[assignment] under strict mypy. Only
552+
# response-side fields (received, never mutated) are safe to change; request-
553+
# side list fields (packages/creatives on request types) stay as list[T]
554+
# because adopters call .append() on them. See issue #624.
555+
RESPONSE_SEQUENCE_FIELDS: list[tuple[str, str]] = [
556+
("media_buy/update_media_buy_response.py", "affected_packages"),
557+
("media_buy/get_media_buys_response.py", "media_buys"),
558+
("media_buy/get_media_buys_response.py", "packages"),
559+
("media_buy/get_media_buy_delivery_response.py", "media_buy_deliveries"),
560+
]
561+
562+
563+
def rewrite_response_list_to_sequence() -> None:
564+
"""Change list[T] → Sequence[T] on response-only container fields.
565+
566+
list[T] is invariant so ``affected_packages: list[MyPkg]`` on a subclass
567+
triggers mypy[assignment] against the parent's ``list[Pkg]``. Sequence[T]
568+
is covariant, removing the error for adopters who extend element types.
569+
"""
570+
print("Rewriting response list fields to Sequence for covariant inheritance...")
571+
572+
for rel_path, field_name in RESPONSE_SEQUENCE_FIELDS:
573+
target = OUTPUT_DIR / rel_path
574+
if not target.exists():
575+
print(f" {rel_path}: not found (skipping)")
576+
continue
577+
578+
content = target.read_text()
579+
580+
# Idempotency: skip if field already uses Sequence
581+
if re.search(rf"{re.escape(field_name)}: Annotated\[\s+Sequence\[", content):
582+
print(f" {rel_path}: {field_name} already uses Sequence (skipping)")
583+
continue
584+
585+
new_content = re.sub(
586+
rf"({re.escape(field_name)}: Annotated\[\s+)list\[",
587+
r"\1Sequence[",
588+
content,
589+
)
590+
591+
if new_content == content:
592+
print(f" {rel_path}: {field_name} — list[ pattern not found (skipping)")
593+
continue
594+
595+
# Add Sequence import from collections.abc in stdlib block.
596+
# Anchor on the first stdlib import line (enum or typing) so Sequence
597+
# lands in correct alphabetical position (c < e < t).
598+
if "from collections.abc import Sequence" not in new_content:
599+
new_content = re.sub(
600+
r"^(from (?:enum|typing) import .+)$",
601+
r"from collections.abc import Sequence\n\1",
602+
new_content,
603+
count=1,
604+
flags=re.MULTILINE,
605+
)
606+
607+
target.write_text(new_content)
608+
print(f" {rel_path}: {field_name} → Sequence[...]")
609+
610+
550611
def fix_list_field_shadowing():
551612
"""Fix models where a field named 'list' shadows the builtin list type.
552613
@@ -1151,6 +1212,7 @@ def main():
11511212
unwrap_rootmodel_unions()
11521213
add_rootmodel_getattr_proxy()
11531214
fix_list_field_shadowing()
1215+
rewrite_response_list_to_sequence()
11541216
fix_reuse_model_discriminator_bug()
11551217
restore_format_category_deprecation_shim()
11561218
inject_literal_discriminator_defaults()

src/adcp/types/_ergonomic.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,15 @@
2525
5. FieldModel (enum) lists accept string lists
2626
2727
Note: List variance issues (list[Subclass] not assignable to list[BaseClass])
28-
are a fundamental Python typing limitation. Users extending library types
29-
should use Sequence[T] in their own code or cast() for type checker appeasement.
28+
are a fundamental Python typing limitation. Response-only container fields
29+
(affected_packages, media_buys, packages, media_buy_deliveries) already use
30+
Sequence[T] in their generated base class. For other fields not yet migrated,
31+
adopters should use Sequence[T] in their own code or cast() for appeasement.
3032
"""
3133

3234
from __future__ import annotations
3335

36+
from collections.abc import Sequence
3437
from typing import Annotated, Any
3538

3639
from pydantic import BeforeValidator
@@ -500,7 +503,7 @@ def _apply_coercion() -> None:
500503
GetMediaBuyDeliveryResponse,
501504
"media_buy_deliveries",
502505
Annotated[
503-
list[MediaBuyDelivery],
506+
Sequence[MediaBuyDelivery],
504507
BeforeValidator(coerce_subclass_list(MediaBuyDelivery)),
505508
],
506509
)

src/adcp/types/generated_poc/media_buy/get_media_buy_delivery_response.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from __future__ import annotations
66

7+
from collections.abc import Sequence
78
from enum import Enum
89
from typing import Annotated, Any
910
from collections.abc import Sequence

src/adcp/types/generated_poc/media_buy/get_media_buys_response.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from __future__ import annotations
66

7+
from collections.abc import Sequence
78
from enum import Enum
89
from typing import Annotated
910
from collections.abc import Sequence
@@ -334,7 +335,7 @@ class MediaBuy(AdCPBaseModel):
334335
),
335336
] = None
336337
packages: Annotated[
337-
list[Package],
338+
Sequence[Package],
338339
Field(
339340
description='Packages within this media buy, augmented with creative approval status and optional delivery snapshots'
340341
),

src/adcp/types/generated_poc/media_buy/update_media_buy_response.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from __future__ import annotations
66

7+
from collections.abc import Sequence
78
from typing import Annotated
89
from collections.abc import Sequence
910

0 commit comments

Comments
 (0)