Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ jobs:
if: matrix.python-version == '3.12'
run: mypy --strict tests/type_checks/ examples/reporting_webhook_activity.py examples/reporting_status_notifications.py examples/reporting_destination_writer.py examples/reporting_durable_materializer.py examples/reporting_receipt_ingress.py examples/reporting_production.py

- name: Run adopter type-check suite under pyright
if: matrix.python-version == '3.12'
# The fixtures that import psycopg need the pg extra, which this job's
# dev-only install does not bring; pyright has no ignore_missing_imports.
run: |
uv pip install --system -e ".[pg]"
python scripts/run_pyright_type_checks.py

- name: Check versioned typing generation
if: matrix.python-version == '3.12'
run: |
Expand Down
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ typecheck: ## Run type checker (mypy) on source code
$(MYPY) src/adcp/
@echo "✓ Type checking passed"

typecheck-all: typecheck test-type-checks check-type-ignore-contract ## Run all type-check contracts
typecheck-all: typecheck test-type-checks test-type-checks-pyright check-type-ignore-contract ## Run all type-check contracts
@echo "✓ All type-check contracts passed"

test: ## Run test suite with coverage
Expand All @@ -59,6 +59,9 @@ test-type-checks: ## Run adopter-pattern type-check suite (mypy --strict, zero t
$(MYPY) --strict tests/type_checks/
@echo "✓ Adopter type-checks passed"

test-type-checks-pyright: ## Run the adopter-pattern type-check suite under pyright (see scripts/run_pyright_type_checks.py)
$(PYTHON) scripts/run_pyright_type_checks.py

check-type-ignore-contract: ## Fail if adopter type-check fixtures use type: ignore suppressions
$(PYTHON) scripts/check_type_ignore_contract.py
@echo "✓ Adopter type-check fixtures contain no type: ignore suppressions"
Expand Down
6 changes: 6 additions & 0 deletions docs/types-9-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,12 @@ deleted and the generated models go back to agreeing with their schema.
a `TYPE_CHECKING` block with `init=False`, so a type checker refuses the
keyword the runtime refuses instead of offering a constructor argument that
raises `ValidationError`. Read them and you get `None`.
* The canonical models' overriding list fields — `GetProductsResponse.products`,
`CreateMediaBuyResponse1.packages`, `ListCreativesResponse.creatives`,
`Product.format_options` and the rest — are declared with their precise
element types (#1416). They read as `list[Product] | None` and so on under
mypy with or without `adcp.types.mypy_plugin`, and under pyright and
Pylance, where the earlier `SchemaVariant[...]` spelling was an error.
* A format reference's `agent_url` is carried as the wire string, validated
as a URL (#1384). `ref.agent_url` is a `str`, not an `AnyUrl`, so
`migrated_…` option IDs derived from a model match those derived from the
Expand Down
144 changes: 144 additions & 0 deletions scripts/run_pyright_type_checks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
#!/usr/bin/env python3
"""Grade the adopter type-check fixtures under pyright as well as mypy (#1416).

``tests/type_checks/`` is what an adopter's own strict type-check sees of this
SDK. CI graded it with mypy only, and mypy ran with ``adcp.types.mypy_plugin``,
so an annotation only that plugin understands passed CI while every pyright
and Pylance user saw errors. This runs pyright over the same fixtures.

A fixture pyright cannot grade yet is listed in ``EXCLUDED`` with the reason;
the list may only shrink. Pass ``--all`` to run the excluded fixtures too and
see what is left to fix.

Several fixtures import the ``[pg]`` extra's ``psycopg``. mypy tolerates those
modules being absent through ``ignore_missing_imports``; pyright has no
per-module equivalent, and an unresolved import is an error, not an ``Any``.
So the gate requires the extra rather than skipping the fixture: a fixture
whose top-level imports this interpreter cannot find fails the run up front,
naming the module and the extra to install, and the CI step installs it.
"""

from __future__ import annotations

import ast
import importlib.util
import subprocess
import sys
from pathlib import Path

ROOT = Path(__file__).resolve().parents[1]
TYPE_CHECK_DIR = ROOT / "tests" / "type_checks"

#: Fixtures pyright rejects today for reasons unrelated to what they grade.
#: Each entry names the pyright-only diagnostic so the fix is findable.
EXCLUDED: dict[str, str] = {
"composing_root_subclassing.py": (
"reportIncompatibleVariableOverride: pyright treats a covariant override "
"of a mutable field as incompatible; mypy admits it"
),
"cross_class_override_with_schema_variant.py": (
"SchemaVariant is a mypy-plugin marker with no pyright equivalent "
"(see adcp/types/variants.py)"
),
"extend_response_with_sequence.py": (
"reportIncompatibleVariableOverride on the covariant Sequence override"
),
"media_buy_actions.py": (
"reportAttributeAccessIssue: pyright does not resolve DurationUnit through "
"the lazy adcp.types facade"
),
"reporting_buyer_submission_intents.py": (
"reportArgumentType: ADCPClient against the ReportingReceiptSubmissionClient protocol"
),
"reporting_durable_materializer.py": (
"reportAssignmentType: store classes against the reporting store protocols "
"(Coroutine vs CoroutineType return types)"
),
"reporting_frozen_feed.py": (
"reportAssignmentType: store classes against the reporting store protocols"
),
"reporting_inline_storage.py": (
"reportAssignmentType: store classes against the reporting store protocols"
),
"reporting_receipt_ingress.py": (
"reportAssignmentType: store classes against the reporting store protocols"
),
"targeting_input_field_types.py": (
"reportAssertTypeFailure: pyright reads GeoCountry where mypy reads str"
),
"versioned_types.py": ("reportTypedDictNotRequiredAccess on NotRequired TypedDict keys"),
}


def fixtures(*, include_excluded: bool) -> list[Path]:
return sorted(
path
for path in TYPE_CHECK_DIR.glob("*.py")
if include_excluded or path.name not in EXCLUDED
)


def missing_imports(path: Path) -> list[str]:
"""Top-level packages ``path`` imports that this interpreter cannot find.

Checked before pyright runs so a missing optional dependency is reported as
what it is — an incomplete environment — instead of as type errors in the
fixture, and so a fixture is never silently dropped from the gate.
"""
roots: set[str] = set()
for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))):
if isinstance(node, ast.Import):
roots.update(alias.name.partition(".")[0] for alias in node.names)
elif isinstance(node, ast.ImportFrom) and node.module and not node.level:
roots.add(node.module.partition(".")[0])
missing: list[str] = []
for root in sorted(roots):
try:
found = importlib.util.find_spec(root) is not None
except (ImportError, ValueError):
found = False
if not found:
missing.append(root)
return missing


def main(argv: list[str] | None = None) -> int:
args = sys.argv[1:] if argv is None else argv
include_excluded = "--all" in args

stale = sorted(name for name in EXCLUDED if not (TYPE_CHECK_DIR / name).exists())
if stale:
print("run_pyright_type_checks: EXCLUDED names fixtures that no longer exist:")
for name in stale:
print(f" {name}")
return 1

selected = fixtures(include_excluded=include_excluded)
unavailable = {path.name: missing_imports(path) for path in selected}
unavailable = {name: roots for name, roots in unavailable.items() if roots}
if unavailable:
print(
"run_pyright_type_checks: these fixtures import modules this interpreter "
"cannot find; install the optional extras (e.g. `uv sync --all-extras` or "
"`pip install -e '.[dev,pg]'`) so pyright grades every fixture:"
)
for name, roots in sorted(unavailable.items()):
print(f" {name}: {', '.join(roots)}")
return 1
command = [
sys.executable,
"-m",
"pyright",
"--pythonpath",
sys.executable,
*(str(path) for path in selected),
]
result = subprocess.run(command, cwd=ROOT)
if result.returncode == 0:
skipped = "" if include_excluded else f", {len(EXCLUDED)} excluded"
print(f"✓ pyright passed on {len(selected)} adopter type-check fixtures{skipped}")
return result.returncode


if __name__ == "__main__":
raise SystemExit(main())
23 changes: 12 additions & 11 deletions src/adcp/types/canonical_creative.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@
MEDIA_BUY_LEGACY_STATUS_VALUES,
unwrap_enum_value,
)
from adcp.types.variants import SchemaVariant


def is_canonical_format_kind(
Expand Down Expand Up @@ -689,7 +688,7 @@ class Placement(_LegacyPlacement, CanonicalBoundaryModel):
if TYPE_CHECKING: # the removed field, hidden from the constructor too
format_ids: _RemovedFormatIdSequence = Field(default=None, init=False)

format_options: SchemaVariant[list[Format] | None] = Field(default=None, min_length=1)
format_options: list[Format] | None = Field(default=None, min_length=1) # type: ignore[assignment]


class Product(_LegacyProduct, CanonicalBoundaryModel):
Expand All @@ -698,10 +697,10 @@ class Product(_LegacyProduct, CanonicalBoundaryModel):
if TYPE_CHECKING: # the removed field, hidden from the constructor too
format_ids: _RemovedFormatIds = Field(default=None, init=False)

format_options: SchemaVariant[list[Format]] = Field(
format_options: list[Format] = Field( # type: ignore[assignment]
min_length=1, description="Canonical creative formats accepted by this product."
)
placements: SchemaVariant[list[Placement] | None] = Field(default=None, min_length=1)
placements: list[Placement] | None = Field(default=None, min_length=1) # type: ignore[assignment]
pricing_options: list[CanonicalPricingOption] = Field(min_length=1)


Expand Down Expand Up @@ -842,7 +841,7 @@ class DeliveryCreative(_LegacyDeliveryCreative, CanonicalBoundaryModel):
if TYPE_CHECKING: # the removed field, hidden from the constructor too
format_id: _RemovedFormatId = Field(default=None, init=False)

variants: SchemaVariant[list[_DeliveryCreativeVariant]] = _inherit(
variants: list[_DeliveryCreativeVariant] = _inherit( # type: ignore[assignment]
_LegacyDeliveryCreative, "variants"
)

Expand Down Expand Up @@ -882,7 +881,7 @@ def _validate_format_params(self) -> PackageRequest:
class PackageUpdate(_LegacyPackageUpdate, CanonicalBoundaryModel):
"""Canonical package update; creatives are canonical assets."""

creatives: SchemaVariant[list[CreativeAsset] | None] = Field(default=None, min_length=1)
creatives: list[CreativeAsset] | None = Field(default=None, min_length=1) # type: ignore[assignment]


class Package(_LegacyPackage, CanonicalBoundaryModel):
Expand Down Expand Up @@ -914,7 +913,7 @@ def _reject_legacy_fields(cls, value: Any) -> Any:
class GetProductsResponse(_LegacyGetProductsResponse, CanonicalBoundaryModel):
"""Canonical discovery response; products are canonical products."""

products: SchemaVariant[list[Product] | None] = None
products: list[Product] | None = None # type: ignore[assignment]


class CreateMediaBuyRequest(_LegacyCreateMediaBuyRequest, CanonicalBoundaryModel):
Expand All @@ -927,13 +926,15 @@ class UpdateMediaBuyRequest(_LegacyUpdateMediaBuyRequest, CanonicalBoundaryModel
"""Canonical update request; both package lists are canonical."""

packages: list[PackageUpdate] | None = None
new_packages: SchemaVariant[list[PackageRequest] | None] = None
new_packages: list[PackageRequest] | None = Field( # type: ignore[assignment]
default=None, min_length=1
)


class CreateMediaBuyResponse1(_LegacyCreateMediaBuyResponse1, CanonicalBoundaryModel):
"""Canonical create response preserving the 3.x legacy-status normalizer."""

packages: SchemaVariant[list[Package]]
packages: list[Package] # type: ignore[assignment]

@model_validator(mode="before")
@classmethod
Expand Down Expand Up @@ -997,7 +998,7 @@ class UpdateMediaBuyResponse3(_LegacyUpdateMediaBuyResponse3, CanonicalBoundaryM
class SyncCreativesRequest(_LegacySyncCreativesRequest, CanonicalBoundaryModel):
"""Canonical creative sync request; creatives are canonical assets."""

creatives: SchemaVariant[list[CreativeAsset]] = Field(min_length=1)
creatives: list[CreativeAsset] = Field(min_length=1) # type: ignore[assignment]


class ListCreativesRequest(_LegacyListCreativesRequest, CanonicalBoundaryModel):
Expand All @@ -1020,7 +1021,7 @@ def _reject_legacy_fields(cls, value: Any) -> Any:
class ListCreativesResponse(_LegacyListCreativesResponse, CanonicalBoundaryModel):
"""Canonical creative listing; rows are canonical listed creatives."""

creatives: SchemaVariant[list[Creative]]
creatives: list[Creative]


class MediaBuyPackage(_LegacyMediaBuyPackage, CanonicalBoundaryModel):
Expand Down
7 changes: 7 additions & 0 deletions src/adcp/types/variants.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ class GetMediaBuyDeliveryResponse(LibraryGetMediaBuyDeliveryResponse):
type-check via ``Sequence[T]`` covariance (PR #635) and the marker
would obscure that the override is sub-typing, not substitution.

The SDK's own models never use it either. ``adcp.types.canonical_creative``
retypes generated ``list`` fields to their canonical element classes with
the precise ``list[...]`` annotation and a per-line ``type: ignore``, which
both mypy and pyright honour, so an adopter reads ``list[Product] | None``
from ``GetProductsResponse.products`` under either checker, with or
without this plugin (#1416).

**Pyright / Pylance**: the bundled mypy plugin doesn't affect pyright.
Adopters using Pylance in VSCode will still see the LSP override
flagged on ``SchemaVariant[T]`` fields even though their mypy CI
Expand Down
Loading
Loading