From c0aa152cf9d5724092db2871951f6d3848ac819b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sat, 23 May 2026 07:47:30 -0400 Subject: [PATCH] docs(types): refresh import-architecture references (closes #807) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLAUDE.md's "Import Architecture for Generated Types" section drifted out of sync with the actual allowlist enforced by ``tests/test_import_layering.py``. It still pointed at a ``stable.py`` that no longer exists and omitted ``capabilities.py``, ``_forward_compat.py``, and ``_generated.py`` — the three modules added after the original two-tier layering. Also drops the "Handling Missing Schema Types" example, whose only artifacts (``FormatId = str``, ``PackageRequest = dict[str, Any]``) were already replaced with real generated types in earlier PRs, and adds a "Codegen variant numbering is unstable" callout so the next person who runs ``scripts/generate_types.py`` doesn't mistake the 600-file ``ClassNameN → ClassNameM`` renumber churn for a real schema delta. Same stale ``stable.py`` reference was carried over into three other files; updated in-place: - ``src/adcp/types/capabilities.py`` docstring - ``src/adcp/decisioning/time_budget.py`` docstring - ``.github/ai-review/expert-adcp-reviewer.md`` layering-breach rule No code changes — pure documentation hygiene against #741's clean- type-baseline precondition. ``ruff check src/``, ``mypy src/adcp/``, and ``pytest tests/`` all green (5010 passed). --- .github/ai-review/expert-adcp-reviewer.md | 2 +- CLAUDE.md | 28 +++++++++++++---------- src/adcp/decisioning/time_budget.py | 8 +++---- src/adcp/types/capabilities.py | 4 ++-- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/.github/ai-review/expert-adcp-reviewer.md b/.github/ai-review/expert-adcp-reviewer.md index 959318980..37bb413cf 100644 --- a/.github/ai-review/expert-adcp-reviewer.md +++ b/.github/ai-review/expert-adcp-reviewer.md @@ -79,7 +79,7 @@ Block any PR that hits one of these: 5. **Pydantic discriminated-union regression** — removing a `UnknownFormatAsset`-style fallback arm from a discriminated union, narrowing `additionalProperties` on a published variant, removing a discriminator value without an open-union escape hatch, or changing the discriminator key on a model that's already in the wild. The forward-compat pattern in `src/adcp/types/_forward_compat.py` / `aliases.py` / `_ergonomic.py` is load-bearing — regressions there break adopter deserialization the moment upstream adds a new variant. -6. **Type-system layering breach** — non-internal modules importing directly from `src/adcp/types/generated_poc/**` or `src/adcp/types/_generated.py`. CLAUDE.md is explicit: only `stable.py`, `aliases.py`, and `_ergonomic.py` may import from those. Anything else is a brittleness leak that ships unstable generated names to adopters. +6. **Type-system layering breach** — non-internal modules importing directly from `src/adcp/types/generated_poc/**` or `src/adcp/types/_generated.py`. The allowlist is enforced by `tests/test_import_layering.py` (`aliases.py`, `capabilities.py`, `_ergonomic.py`, `_forward_compat.py`, `_generated.py`, `__init__.py`). Anything else is a brittleness leak that ships unstable generated names to adopters. 7. **CI gate regressions** — diff that disables a test instead of fixing it, removes a `ruff` rule without justification, suppresses a `mypy` error with a blanket `# type: ignore` (specific codes like `# type: ignore[no-any-return]` are fine — blanket ones are the block), or skips a CI step. The three pre-commit checks (`ruff check src/`, `mypy src/adcp/`, `pytest tests/ -v`) are the floor. diff --git a/CLAUDE.md b/CLAUDE.md index 050af6efb..b56f65d28 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,16 +6,16 @@ - Download schemas from canonical source (e.g., adcontextprotocol.org/schemas) - Generate Pydantic models automatically - keeps types in sync with spec - Validate generated code in CI (syntax check + import test) -- For missing upstream types, add type aliases with clear comments explaining why -**Handling Missing Schema Types** -When schemas reference types that don't exist upstream: -```python -# MISSING SCHEMA TYPES (referenced but not provided by upstream) -# These types are referenced in schemas but don't have schema files -FormatId = str -PackageRequest = dict[str, Any] -``` +**Codegen variant numbering is unstable** +`datamodel-code-generator` numbers anonymous variant classes (e.g. `Assets162`, +`Idempotency3`, `Type11`) by traversal order. Adding or reordering even one +sibling schema shifts the entire numbering window — a clean re-run can produce +a 600+ file diff with zero semantic change. Treat any `git diff` on +`generated_poc/` that consists of only `ClassNameN → ClassNameM` renames as +churn, not a schema delta, and discard it. `aliases.py` re-exports these +numbered classes under semantic names; accepting the renumber breaks the +alias layer for nothing. **Import Architecture for Generated Types** The type system has a strict layering to prevent brittleness: @@ -25,15 +25,19 @@ generated_poc/*.py (internal, auto-generated from schemas) ↓ _generated.py (internal consolidation) ↓ -stable.py + aliases.py + _ergonomic.py (public API / internal infrastructure) +aliases.py + capabilities.py + _ergonomic.py + _forward_compat.py ↓ __init__.py (user-facing exports) ``` -Only these modules may import from `generated_poc/` or `_generated.py`: -- `stable.py`: Re-exports base types with clean names +Only these modules may import from `generated_poc/` or `_generated.py` +(enforced by `tests/test_import_layering.py`): +- `_generated.py`: Consolidates exports from `generated_poc/` into a flat namespace - `aliases.py`: Creates semantic aliases for numbered discriminated union types +- `capabilities.py`: Re-exports `get_adcp_capabilities_response` sub-models with disambiguated names - `_ergonomic.py`: Applies BeforeValidator coercion for type ergonomics +- `_forward_compat.py`: Patches `Format.assets` / `RepeatableAssetGroup.assets` with open union types at import time +- `__init__.py`: Public API surface All other source code should import from `adcp.types` (the public API). diff --git a/src/adcp/decisioning/time_budget.py b/src/adcp/decisioning/time_budget.py index 7822e6352..65fd40d44 100644 --- a/src/adcp/decisioning/time_budget.py +++ b/src/adcp/decisioning/time_budget.py @@ -117,8 +117,7 @@ def resolve_time_budget(time_budget: Any) -> float | None: factor = _UNIT_TO_SECONDS.get(unit_str) if factor is None: logger.warning( - "[adcp.decisioning] Unrecognised time_budget unit %r — " - "treating as no deadline.", + "[adcp.decisioning] Unrecognised time_budget unit %r — " "treating as no deadline.", unit_str, ) return None @@ -128,8 +127,7 @@ def resolve_time_budget(time_budget: Any) -> float | None: interval = time_budget.get("interval") if not isinstance(interval, int) or interval < 1: logger.warning( - "[adcp.decisioning] Invalid time_budget interval %r — " - "treating as no deadline.", + "[adcp.decisioning] Invalid time_budget interval %r — " "treating as no deadline.", interval, ) return None @@ -143,7 +141,7 @@ def project_incomplete_response(*, interval: int, unit: str) -> dict[str, Any]: Returns ``{"products": [], "incomplete": [{scope, description, estimated_wait}]}`` with ``incomplete`` containing at least one entry (``min_length=1`` on the wire schema). Uses a raw dict to stay above the import-layering - boundary (only ``stable.py`` / ``aliases.py`` / ``_ergonomic.py`` may + boundary (only the whitelist in ``tests/test_import_layering.py`` may import from ``adcp.types._generated``). ``scope`` is ``"products"`` — the spec's "not all inventory sources were diff --git a/src/adcp/types/capabilities.py b/src/adcp/types/capabilities.py index 1f0c1b4b8..6dde58707 100644 --- a/src/adcp/types/capabilities.py +++ b/src/adcp/types/capabilities.py @@ -11,8 +11,8 @@ name with unrelated wire types already exported from :mod:`adcp.types`. This module sits in the import-architecture whitelist (alongside -``stable.py``, ``aliases.py``, ``_ergonomic.py``) for direct -``generated_poc`` imports. It pulls the capabilities sub-models out +``aliases.py``, ``_ergonomic.py``, ``_forward_compat.py``, +``_generated.py``) for direct ``generated_poc`` imports. It pulls the capabilities sub-models out under disambiguated names, so the colliding three don't shadow the wire types when re-exported from :mod:`adcp.types`. Adopters never import from this module directly — :mod:`adcp.decisioning.capabilities`