docs(types): refresh import-architecture references (closes #807)#828
Merged
Conversation
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).
There was a problem hiding this comment.
LGTM. Pure docs hygiene catching CLAUDE.md up with reality — stable.py was already gone, the FormatId = str / PackageRequest = dict[str, Any] stubs were already replaced, and the actual allowlist is enforced by tests/test_import_layering.py:33-48. The doc was lying; this fixes it.
Things I checked
- Allowlist in
tests/test_import_layering.py:33-48matches the new CLAUDE.md list exactly:aliases.py,_ergonomic.py,_generated.py,__init__.py,capabilities.py,_forward_compat.py. Nostable.py. Claim is sound. src/adcp/types/stable.pydoes not exist on disk — the old doc was pointing at a phantom.grepforFormatId = strandPackageRequest = dict[str, Any]insrc/returns nothing — confirmed the stubs the old section described are gone.aliases.pystill re-exportsFormatId = FormatReferenceStructuredObject(a real generated type), which is the load-bearing form the PR body calls out. Not regressed.src/adcp/types/capabilities.py:13-15docstring whitelist now agrees with CLAUDE.md and the test.src/adcp/decisioning/time_budget.py:144docstring now points at the test as the source of truth instead of naming an outdated subset. Right move — the doc no longer has to be updated when the allowlist grows..github/ai-review/expert-adcp-reviewer.md:82MUST FIX rule 6 now cites the test-enforced allowlist. The reviewer prompt was the third site carrying thestable.pyghost; good catch.- Codegen-variant-numbering callout in CLAUDE.md is the right warning to bake in.
datamodel-code-generator's traversal-order numbering producing 600+ files ofClassNameN → ClassNameMchurn on a clean re-run is exactly the kind of trap that costs a day if you don't know about it. - Public API surface: untouched.
__init__.pynot in the diff. - Generated layer: untouched.
generated_poc/**and_generated.pynot in the diff. - Test plan: all four checkboxes checked.
pytest tests/reports 5010 passed, including the 50 tests intest_import_layering.py.
Minor nits (non-blocking)
- Commit message says "No code changes" but
time_budget.pyreformats twologger.warningstrings from explicit multi-line continuation to single-line implicit-concatenation (time_budget.py:120, 130). The runtime string is identical — Python concatenates adjacent literals at compile time — so no behavior change, but the new form reads worse on a 120-col terminal and isn't part of the stated docs-hygiene scope. Probably an autoformat side effect that snuck in. Notable, not blocking.
LGTM.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tests/test_import_layering.pyallowlist (drops nonexistentstable.py; addscapabilities.py,_forward_compat.py,_generated.py).FormatId = str/PackageRequest = dict[str, Any]were both replaced with real generated types in earlier PRs; the section was misleading.scripts/generate_types.pydoesn't mistake the ~600-fileClassNameN → ClassNameMrenumber churn for a real schema delta.stable.pydoc fix through three other files that referenced it:src/adcp/types/capabilities.pydocstring,src/adcp/decisioning/time_budget.pydocstring,.github/ai-review/expert-adcp-reviewer.mdlayering-breach rule.Why these are the only changes
#807's stated scope was (a) re-sync schemas, (b) regenerate types and inspect diff, (c) drop obsolete handrolled aliases, (d) update CLAUDE.md.
3.1.0-beta.3via chore(schemas): resync 3.1.0-beta.3 cache + fix broken manifest patch (refs #807) #821.Idempotency3 → Idempotency1,Assets162 → Assets161, etc.) —datamodel-code-generatornumbers anonymous variant classes by traversal order, and even alatest-keyed re-run on the same pinned schema produces shifts. Committing the regen would breakaliases.py's semantic re-exports of the numbered classes for zero spec value, so it's discarded. The new CLAUDE.md callout documents the trap.FormatId/PackageRequeststubs called out in the original CLAUDE.md no longer exist in code — earlier PRs replaced them.aliases.pydoes still re-exportFormatId = FormatReferenceStructuredObject(a real generated type), which is the load-bearing form and stays.Net: pure documentation hygiene. No public API or generated-type changes.
Test plan
ruff check src/— all checks passedmypy src/adcp/— no issues in 892 source filespytest tests/— 5010 passed, 40 skipped, 1 xfailedtests/test_import_layering.pyspecifically green (50 tests)Closes #807.