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
2 changes: 1 addition & 1 deletion .github/ai-review/expert-adcp-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
28 changes: 16 additions & 12 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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).

Expand Down
8 changes: 3 additions & 5 deletions src/adcp/decisioning/time_budget.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/adcp/types/capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
Loading