feat(types): Sequence[T] on response-only list fields for covariant adoption#635
Merged
Merged
Conversation
…doption 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 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #624
Adopters who subclass SDK response models and narrow the element type (e.g.
list[MyPackage]instead oflist[Package]) getmypy[assignment]errors becauselist[T]is invariant. This PR changes four response-only container fields toSequence[T]— covariant in its element type — soSequence[MyPackage]is a valid subtype ofSequence[Package]and thetype: ignoreis eliminated.What changed
Four response-side fields (Category A from @bokelley's analysis in #624):
affected_packagesUpdateMediaBuyResponse1list[Package]→Sequence[Package]media_buysGetMediaBuysResponselist[MediaBuy]→Sequence[MediaBuy]packagesMediaBuy(in GetMediaBuysResponse)list[Package]→Sequence[Package]media_buy_deliveriesGetMediaBuyDeliveryResponselist[MediaBuyDelivery]→Sequence[MediaBuyDelivery]Implementation path —
post_generate_fixes.py(not hand-edited generated files):RESPONSE_SEQUENCE_FIELDSconfig list +rewrite_response_list_to_sequence()function rewrites the annotation and insertsfrom collections.abc import Sequenceon every codegen run.generate_ergonomic_coercion.pyupdated to detectSequence[T]fields (via expandedis_list_of) and emitSequence[T](notlist[T]) in the_patch_field_annotationcall so_ergonomic.py's runtime coercion stays consistent with the source annotation._ergonomic.pyregenerated:media_buy_deliveriescoercion patch now usesSequence[MediaBuyDelivery]andfrom collections.abc import Sequenceis imported.Explicitly out of scope per @bokelley's analysis:
account,idempotency_key,canceled): dissolve when Typed dispatcher rejects valid request whenaccountis omitted on auth-resolved tools #623 lands — not an inheritance-typing problem.geo_countries_exclude×4: lives incore/targeting.pyTargetingOverlaywhich is used in both request and response context — changing it could break adopters who.append()on request-side targeting lists.creatives(delivery): not yet located + same request/response ambiguity concern.packages/creativeson request types): explicitly excluded because adopters call.append()on those.Bundled copies (
src/adcp/types/generated_poc/bundled/): still uselist[T]— these are not re-exported from_generated.pyfor these three response types and are not reachable fromfrom adcp.types import .... If bundled types are ever promoted to the public API they would silently re-introduce the problem.What was tested
pytest tests/ -q(excludingtest_real_tls_handshake_still_validates_hostname— pre-existing network failure that also fails on main): 4159 passedmypy src/adcp/: Success — no issues in 783 source filesruff check src/: All checks passedPre-PR review
_ergonomic.pypatchingmedia_buy_deliveriesback tolist[T]) fixed viagenerate_ergonomic_coercion.py+_ergonomic.pyregen; nits: bundled copies still uselist[T](explained above),\s+vs\s*in regex (low-risk nit, field block is always multi-line in codegen output)Refs #624(notCloses) confirmed correct;_ergonomic.pycomment updated to reflect which fields are migrated vs. still pendingSession: https://claude.ai/code/session_019aVaC1DXjVH6Kan4gBe1sb
Generated by Claude Code