Skip to content

feat(types): Sequence[T] on response-only list fields for covariant adoption#635

Merged
bokelley merged 3 commits into
mainfrom
claude/issue-624-sequence-response-lists
May 12, 2026
Merged

feat(types): Sequence[T] on response-only list fields for covariant adoption#635
bokelley merged 3 commits into
mainfrom
claude/issue-624-sequence-response-lists

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Refs #624

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. This PR changes four response-only container fields to Sequence[T] — covariant in its element type — so Sequence[MyPackage] is a valid subtype of Sequence[Package] and the type: ignore is eliminated.

What changed

Four response-side fields (Category A from @bokelley's analysis in #624):

Field Model Type change
affected_packages UpdateMediaBuyResponse1 list[Package]Sequence[Package]
media_buys GetMediaBuysResponse list[MediaBuy]Sequence[MediaBuy]
packages MediaBuy (in GetMediaBuysResponse) list[Package]Sequence[Package]
media_buy_deliveries GetMediaBuyDeliveryResponse list[MediaBuyDelivery]Sequence[MediaBuyDelivery]

Implementation pathpost_generate_fixes.py (not hand-edited generated files):

  • New RESPONSE_SEQUENCE_FIELDS config list + rewrite_response_list_to_sequence() function rewrites the annotation and inserts from collections.abc import Sequence on every codegen run.
  • generate_ergonomic_coercion.py updated to detect Sequence[T] fields (via expanded is_list_of) and emit Sequence[T] (not list[T]) in the _patch_field_annotation call so _ergonomic.py's runtime coercion stays consistent with the source annotation.
  • _ergonomic.py regenerated: media_buy_deliveries coercion patch now uses Sequence[MediaBuyDelivery] and from collections.abc import Sequence is imported.

Explicitly out of scope per @bokelley's analysis:

  • Category B/C (required→optional field widening, account, idempotency_key, canceled): dissolve when Typed dispatcher rejects valid request when account is omitted on auth-resolved tools #623 lands — not an inheritance-typing problem.
  • Category D (Literal narrowing, ~3 cases): possible Pydantic mypy plugin bug, separate investigation.
  • geo_countries_exclude ×4: lives in core/targeting.py TargetingOverlay which 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.
  • Request-side list fields (packages/creatives on request types): explicitly excluded because adopters call .append() on those.

Bundled copies (src/adcp/types/generated_poc/bundled/): still use list[T] — these are not re-exported from _generated.py for these three response types and are not reachable from from 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 (excluding test_real_tls_handshake_still_validates_hostname — pre-existing network failure that also fails on main): 4159 passed
  • mypy src/adcp/: Success — no issues in 783 source files
  • ruff check src/: All checks passed

Pre-PR review

  • code-reviewer: approved — blocker (_ergonomic.py patching media_buy_deliveries back to list[T]) fixed via generate_ergonomic_coercion.py + _ergonomic.py regen; nits: bundled copies still use list[T] (explained above), \s+ vs \s* in regex (low-risk nit, field block is always multi-line in codegen output)
  • dx-expert: approved — Refs #624 (not Closes) confirmed correct; _ergonomic.py comment updated to reflect which fields are migrated vs. still pending

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_019aVaC1DXjVH6Kan4gBe1sb


Generated by Claude Code

claude added 2 commits May 10, 2026 13:27
…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
@bokelley bokelley marked this pull request as ready for review May 12, 2026 01:23
@bokelley bokelley merged commit 19be8db into main May 12, 2026
15 of 16 checks passed
@bokelley bokelley deleted the claude/issue-624-sequence-response-lists branch May 12, 2026 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants