feat(types): adopter inheritance pattern triggers ~50 type: ignore[assignment]

[bokelley]

Problem

The recommended adopter pattern — extend a library type and override a nested field with a more-specific element type — requires # type: ignore[assignment] to satisfy mypy under strict = true. We carry roughly 50 of these in our codebase across the schema package.

The mypy Pydantic plugin treats field annotations as invariant under inheritance, even when the override is logically covariant (every AffectedPackage IS-A LibraryAffectedPackage).

Reproducer

from adcp.types import UpdateMediaBuyResponse as LibraryUpdateMediaBuyResponse
from adcp.types import AffectedPackage as LibraryAffectedPackage
from pydantic import Field

class AffectedPackage(LibraryAffectedPackage):
    # Internal-only fields excluded on the wire
    changes_applied: dict | None = Field(default=None, exclude=True)
    buyer_package_ref: str | None = Field(default=None, exclude=True)

class UpdateMediaBuyResponse(LibraryUpdateMediaBuyResponse):
    # Override to use the extended element type — same shape on the wire,
    # extra internal fields excluded via exclude=True. Pydantic accepts it
    # at runtime; mypy rejects it.
    affected_packages: list[AffectedPackage] | None = None  # type: ignore[assignment]

mypy --strict complains about affected_packages being incompatible with the parent's list[LibraryAffectedPackage]. There's nothing wrong at runtime — every AffectedPackage is a LibraryAffectedPackage. The issue is that mypy sees list[X] as invariant in X.

Where this hits us

We carry ~50 instances of # type: ignore[assignment] in src/core/schemas/ for exactly this pattern. Sample:

• _base.py:360 — affected_packages: list[AffectedPackage] | None
• _base.py:875-878 — geo exclusion lists
• delivery.py:251, 440

This is the recommended pattern (Critical Pattern #1 in our CLAUDE.md). It's not a code smell on our side — we're following the docs. The strictness ergonomics for adopters are the issue.

Proposed SDK shape

A few possible directions, in increasing order of impact:

A) Use Sequence[X] instead of list[X] in library response types where adopters are expected to override the element. Sequence is covariant in its element type, so Sequence[AffectedPackage] is a valid subtype of Sequence[LibraryAffectedPackage].

Drawback: callers who want .append() need a mutable type. May not work for in-place mutation paths.

B) Generic library types parameterised on extension points. The library generates UpdateMediaBuyResponse[TAffectedPackage = AffectedPackage], and adopters subclass with class UpdateMediaBuyResponse(LibraryUpdateMediaBuyResponse[AffectedPackage]): .... mypy is happy; no type: ignore needed.

Drawback: more codegen complexity; new public API.

C) Codegen extension hooks. datamodel-codegen emits # extension: list-element-type comments on field annotations that adopters typically override; a separate codegen step generates an Extended<Type> skeleton that adopters can subclass with the field overridden cleanly.

Drawback: most invasive.

Why this matters beyond us

Any Python adopter who extends library types under strict mypy hits this. We absorb 50 type: ignore lines because we're the most advanced adopter; subsequent adopters either (a) carry the same lines, (b) silence the errors with bare type: ignore, or (c) abandon the inheritance pattern in favour of duplication — which the docs explicitly warn against.

This is a strictness-ergonomics gap, not a runtime bug. Companion to #615 (nested model_dump() overrides), which is the same family of friction.

Files

• Our overrides: src/core/schemas/_base.py, delivery.py, creative.py
• Pattern documented: CLAUDE.md "Critical Pattern chore(main): release 0.1.0 #1: AdCP Schema: Extend Library Schemas"

[bokelley]

Triage

Classification: Feature request
Bucket(s): client (no client label exists in this repo — see run-summary gap note)
Status: ready-for-human
Milestone: (omitted — no target version specified)

What the experts said:

• dx-expert: Confirmed friction tax on strict-mypy adopters, not a blocker — bites only under --strict. Recommends executing Approach A (list[T] → Sequence[T] on overridable response fields) combined with a new pitfall example in docs showing adopters to write Sequence[SubType] rather than list[SubType] when overriding. Scores agent buildability 3/5 — coding agents will faithfully reproduce the type: ignore antipattern without a docs example showing the fix.
• code-reviewer: Approach A is breaking — changing a response field from list[X] to Sequence[X] removes .append() / .insert() and breaks callers who pass the field value to any function typed def f(x: list[Product]). Approach B (Generic library types) is not viable because UpdateMediaBuyResponse is a discriminated union alias (UpdateMediaBuyResponse1 | UpdateMediaBuyResponse2 | …), not a class — union aliases cannot be parameterised with TypeVar. Approach C just moves the type: ignore into generated code.

My take: The experts converge on the diagnosis (all three proposed approaches are either breaking or not viable) but diverge on whether to execute Approach A anyway. The core tension: list→Sequence is the only annotation change that gives mypy covariance, but it is a public-API annotation change. Whether that is acceptable as a deliberate breaking minor bump (or acceptable at all given the union-alias architecture) is a product decision. Flagging for @bokelley to weigh in: (1) accept Approach A as a semi-major annotation break with a release note, (2) ship a docs-only workaround documenting cast() or typed Sequence overrides as the canonical pattern, or (3) explore a fourth path not yet proposed.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_014aMoVeZgS8uSd4vTcZEyjo

---

Generated by Claude Code

[bokelley]

Adopter analysis — categorising our actual cases

Went back and categorised every `# type: ignore[assignment]` in our schema package. There are 4 distinct sub-patterns, and they need different fixes — picking ONE of the three approaches in the original issue won't close all of them. Here's the breakdown:

Category A — List element-type widening (~10 cases)

```python

parent: list[LibraryAffectedPackage]

affected_packages: list[AffectedPackage] | None = None # type: ignore[assignment]

also: geo_countries_exclude (×4), creatives, packages, media_buys,

media_buy_deliveries, creatives (delivery)

```

These are genuinely covariant in the element type. `Sequence[X]` fixes all of them with zero ergonomic cost — adopters who want `.append()` use the underlying `list` field directly; the type annotation is the abstract `Sequence`.

Recommendation: ship `Sequence[X]` on response/request fields where the element type is a known extension point. Lowest-risk approach, kills the largest single cluster.

Category B — Optional widening on previously-required field (~6 cases)

```python

parent: account: LibraryAccountReference (required)

account: LibraryAccountReference | None = None # type: ignore[assignment]

same pattern: idempotency_key, canceled, several copies across request types

```

These are contravariant — child making a required field optional violates LSP. mypy is correct to flag them; `Sequence[X]` does not help; `Generic[T]` does not help.

The real fix is #623 — the spec already says these fields are optional when the seller resolves identity from the auth chain (`account`) or generates dedup keys server-side (`idempotency_key`). The SDK currently emits them as required because the JSON schema is required-by-default. Once #623 makes them optional in the library types, our overrides go away entirely (the parent type matches what we want).

Recommendation: do not solve Category B in this issue. Track via #623.

Category C — Default-value override on same type (~3 cases)

```python

parent: canceled: Literal[True] = True

canceled: Literal[True] | None = Field(default=None) # type: ignore[assignment]
```

Same Literal type, different default. The annotation actually IS different (Optional widening, so this overlaps with Category B). The cases here are sub-cases of B caused by the same SDK-side default-injection issue.

Recommendation: same as B — fixed by #623 + spec-correct optional defaults.

Category D — Type narrowing on Literal (~3 cases)

```python

parent: signal_type: SomeLooserType

signal_type: Literal["marketplace", "custom", "owned"] = Field(...) # type: ignore[assignment]
```

Narrowing a parent's broader type. This IS covariant and `Sequence[X]` doesn't help (these aren't lists). May actually be a Pydantic mypy plugin bug — manually narrowing a Literal should be allowed under the plugin. Worth investigating separately.

Recommendation: investigate as a Pydantic-plugin issue rather than an SDK code change.

Final recommendation

• Approach A (`Sequence[X]`) is the right pick for THIS issue. Targets Category A specifically — ~10 of the ~25 cases I counted.
• Generic library types (Approach B) is over-engineering for the size of the problem. The benefit is theoretical; the codegen complexity is real.
• Codegen extension hooks (Approach C) is over-engineering² — same conclusion, more so.
• Categories B and C dissolve when Typed dispatcher rejects valid request when account is omitted on auth-resolved tools #623 lands. They're not really an inheritance-typing problem; they're a "library types are too strict" problem.
• Category D is a Pydantic-plugin investigation — separate work.

Net: this issue is best scoped to Approach A only, with ~10 `type: ignore` lines deleted on our side. The rest happen elsewhere.

Happy to send a PR that switches the relevant fields to `Sequence[X]` if that's the direction. Would suggest starting with the `adcp.types` response models (`UpdateMediaBuyResponse.affected_packages`, `GetMediaBuysResponse.media_buys`, etc.) where in-place mutation by adopters is rare. Request-side fields (`packages`, `creatives`) need more thought because adopters do construct them from dicts; `Sequence` works for type-narrowing but not for `.append()` patterns adopters might rely on.

If you want me to spike a PR, point me at the codegen template that emits these fields and I'll prototype.

[bokelley]

Triage (updated)

Classification: Feature request
Bucket(s): client
Status: drafting-pr
Milestone: (omitted)

What the experts said (re-triage after @bokelley's analysis):

• code-reviewer: revised verdict — list→Sequence on response-side fields is non-breaking in practice. Adopters receive these fields and don't mutate them; .append() concern only applies to request-side fields (correctly excluded). Flagged that _ergonomic.py would patch media_buy_deliveries back to list[T] — fixed by updating generate_ergonomic_coercion.py to detect Sequence[T] fields and emit matching annotations.
• dx-expert: confirmed execute for Category A response fields only. Implementation path via post_generate_fixes.py AST rewrite is correct and survives regeneration. geo_countries_exclude correctly excluded (request+response context in TargetingOverlay).

My take: @bokelley's 4-category breakdown unlocked the prior deadlock. Category A (~4 fields shipped in PR #635) eliminates the largest cluster of type: ignore. Categories B/C dissolve with #623. Category D is a separate Pydantic-plugin investigation.

Drafting PR #635 for Category A: affected_packages, media_buys, packages (MediaBuy), media_buy_deliveries. geo_countries_exclude and creatives need separate handling — this issue stays open for those.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_019aVaC1DXjVH6Kan4gBe1sb

---

Generated by Claude Code

[bokelley]

Direction: Approach A (list[X] → Sequence[X]) — scoped, not blanket

Digging into our 25 cases reveals the inheritance friction is partly a misdiagnosis. Decomposing them:

Sub-pattern	Count	Actual fix
List-element widening (extend response element type)	~10	Sequence[X] — the only real inheritance issue
Library type marks field required when spec allows omission	~6	#623 (and follow-ups) — these dissolve when the SDK aligns to spec optionality
Narrowing a str | None to str in a subclass	~3	Pydantic plugin — separate triage
Adding a spec-mandated default the library should ship	~3	Spec-side or pre_validation_hooks (#629)
Misc	~3	Case-by-case

So Approach A's actual scope is ~10 fields where adopters legitimately need to override the element type. That's the only set Sequence buys us anything for — and on those, the breaking surface is bounded:

• Sellers (the extenders) override the field at definition time and can declare list[ChildType] for .append() ergonomics; covariance flows through.
• Buyers (the consumers) read the field; they don't .append() to a response shape they didn't construct.
• Hypothetical def f(x: list[Product]) callers do exist but are rare on response payloads. Document the migration as def f(x: Sequence[Product]) in the release note.

Recommendation

1. Ship Approach A — but only on response-type fields documented as extension points. ~10 fields, not the whole API surface.
2. Don't pursue Approach B (Generic types) — confirmed not viable given discriminated-union aliases.
3. Don't pursue Approach C (codegen hooks) — moves the type: ignore rather than removing it, and invasive for a small actual problem.
4. Land Typed dispatcher rejects valid request when account is omitted on auth-resolved tools #623 in parallel — it eliminates ~6 of our ignores that look like inheritance issues but are actually spec-misalignment issues. After Typed dispatcher rejects valid request when account is omitted on auth-resolved tools #623, the remaining inheritance-cohort is closer to 10 than 25.
5. The 3 narrowing cases warrant a separate Pydantic-plugin issue — bundling them with this would muddy the design.

Offer

I can spike the Approach A patch — codegen change in datamodel-codegen post-processor to emit Sequence[X] for the documented-extension-point fields, plus the corresponding _ergonomic.py BeforeValidator updates. Should be small. Want me to open a draft?

[bokelley]

Spike result: Sequence-only approach works under mypy + Pydantic plugin

Built a 2-file mypy --strict spike against pydantic 2.13.4 + mypy 1.20.2 with the bundled Pydantic plugin enabled (matches CI). Both files are minimal Pydantic models — parent + child with field override:

repro_with_list.py — parent declares affected_packages: list[LibAP] | None. Child overrides with list[ExtAP] | None where ExtAP(LibAP).

$ mypy --strict repro_with_list.py
repro_with_list.py:41: error: Incompatible types in assignment (expression has type
"list[ExtAP] | None", base class "LibUpdateResp" defined the type as
"list[LibAP] | None")  [assignment]
Found 1 error in 1 file

This is the exact error each of salesagent's # type: ignore[assignment] lines silences.

fix_with_sequence.py — parent declares affected_packages: Sequence[LibAP] | None. Child overrides with list[ExtAP] | None. Adopter .append() ergonomics preserved (asserts at runtime; mypy types the field as list[ExtAP] at the child level so the call site sees a mutable list).

$ mypy --strict fix_with_sequence.py
Success: no issues found in 1 source file

Conclusion: the Pydantic plugin accepts Sequence[Parent] + list[Child] cleanly. Approach A is viable; this isn't just language-level covariance theory.

Next step

Implementation path of least resistance: a post-codegen processor script that rewrites list[X] → Sequence[X] for fields on an allowlist (the ~10 documented extension-point response fields), runs after sync_schemas.py, survives regen drift. Hand-editing generated_poc/ directly would be reverted on next sync.

The full patch needs:

1. The allowlist (~10 field paths — needs salesagent's actual # type: ignore[assignment] line list to map cleanly)
2. The post-processor script in scripts/
3. _ergonomic.py BeforeValidator updates if any (probably none — Sequence accepts list at construction)
4. Release note flagging the annotation change as a minor break for callers passing the field into def f(x: list[Product])

Want me to open a draft PR with this implementation now, or wait until the salesagent-side type:ignore line list is available so the allowlist is accurate?

[bokelley]

The spike result is great confirmation — the Pydantic plugin accepting Sequence[Parent] + list[Child] under --strict is exactly what the prior triage run banked on.

No need to open a new PR: draft #635 already implements this for the four response-only Category A fields (affected_packages, media_buys, packages/MediaBuy, media_buy_deliveries) via a post_generate_fixes.py post-processor (not hand-edits to generated_poc/). The spike validates the approach that's already in review there.

On the allowlist accuracy question: the four fields in #635 were selected from the prior analysis thread, not from salesagent's full # type: ignore[assignment] line list. If the spike work turned up additional Category A fields not covered by #635 (i.e., response-side list fields that adopters override with a narrower element type), drop them in a comment on #635 and the allowlist can be expanded there — no need to wait or start fresh.

Still open on this issue after #635 merges: geo_countries_exclude (request+response context ambiguity) and creatives (delivery) — both need separate handling and were explicitly left out of #635's scope.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01EfRnLvKHDNWnuzG2p2zAjJ

---

Generated by Claude Code