Repository navigation
native types codegen design and generated schemas - #857
Conversation
5d75777 to
cf12e65
Compare
jonathan343
left a comment
There was a problem hiding this comment.
Thanks @stobrien89!
The design itself looks great. Most of my feedback is about keeping the doc focused on the goal state. Anything that describes current state, specific PRs, or the Java migration will go stale as we land the implementation, so I'd move that to the PR description or #797.
69872c9 to
bbf1e2a
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Output-path safety, stale-file cleanup, selector evaluation, and generated-name collisions remain unresolved.
5 open issues
Reject drive-qualified path components before native path construction · New Prevent stale cleanup through symlinked ancestor paths · New Detect public name collisions across declarations and synthesized classes · New Include resource lifecycle and collection operations in topdown filter · New Reject non-finite decimal trait values before ordered comparisons · New
What changed in this PR
This PR lays the foundation for native Python standalone-types generation under #797. It adds compact schema emission and shared generation infrastructure while leaving the Java generator authoritative.
Changes:
- Add selector parsing, standalone-type selection, generation plans, and compact schema emission.
- Introduce dependency tracking and staged file publication with ownership manifests.
- Expand unit coverage and document the planned standalone-types pipeline.
| File | Description |
|---|---|
packages/smithy-python/tests/unit/test_writer.py |
Test dependency collection from type references. |
packages/smithy-python/tests/unit/test_symbols.py |
Cover dependency metadata and streaming schema names. |
packages/smithy-python/tests/unit/test_selectors.py |
Test selector parsing and evaluation. |
packages/smithy-python/tests/unit/test_schemas.py |
Test compact encoding, hydration, and deterministic output. |
packages/smithy-python/tests/unit/test_generation.py |
Test generation settings, context, and plans. |
packages/smithy-python/tests/unit/test_files.py |
Test staging, ownership, cleanup, and path checks. |
packages/smithy-python/tests/unit/test_dependencies.py |
Test dependency validation and merging. |
packages/smithy-python/tests/unit/test_data_selection.py |
Cover standalone-type selection rules. |
packages/smithy-python/src/smithy_python/writer.py |
Record dependencies from nested references. |
packages/smithy-python/src/smithy_python/symbols.py |
Plan schema and union names; attach dependencies. |
packages/smithy-python/src/smithy_python/selection.py |
Add standalone-type selection and dependency closure. |
packages/smithy-python/src/smithy_python/schemas.py |
Plan and emit compact, lazy schemas. |
packages/smithy-python/src/smithy_python/model/_shapes.py |
Define shared declaration shape types. |
packages/smithy-python/src/smithy_python/model/__init__.py |
Export declaration shape types. |
packages/smithy-python/src/smithy_python/generation.py |
Introduce shared settings, context, and plans. |
packages/smithy-python/src/smithy_python/files.py |
Implement staged publication and ownership tracking. |
packages/smithy-python/src/smithy_python/exceptions.py |
Add selector syntax errors. |
packages/smithy-python/src/smithy_python/dependencies.py |
Define dependency records and runtime requirements. |
packages/smithy-python/src/smithy_python/_selector.py |
Implement Smithy selector parsing and evaluation. |
packages/smithy-python/.changes/next-release/smithy-python-feature-schema-generation.json |
Announce native compact schema generation. |
packages/smithy-core/src/smithy_core/_schema_compact.py |
Expose the compact encoding version. |
designs/codegen/writer.md |
Document writer dependency tracking. |
designs/codegen/types.md |
Describe the standalone-types architecture and compatibility goals. |
designs/codegen/symbols.md |
Document dependency-bearing type references. |
designs/codegen/selection.md |
Describe standalone selection and selectors. |
designs/codegen/index.md |
Link the standalone-types design. |
designs/codegen/cli.md |
Update documented artifact-specific selection rules. |
Review effort: Balanced
743a2bb to
eee35b8
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved correctness issues affect schema semantics, shape selection, and safe output publication.
6 open issues
Track qualification in traversal visited state · New Handle symlink aliases during output collision and cleanup checks · New Preserve primitive target identity and inherited defaults · New Include virtual trait shapes in selector starting set · New Allow empty paths in scoped context references · New Exclude streaming unions from declaration name checks · New
5 resolved since last review
Detect public name collisions across declarations and synthesized classes Prevent stale cleanup through symlinked ancestor paths Reject drive-qualified path components before native path construction Reject non-finite decimal trait values before ordered comparisons Include resource lifecycle and collection operations in topdown filter
Review effort: Balanced
|
I'm curious 🐵 about a few parameters. let's take EC2's model as the input.
To what extent is multithreading possible when we generate hundreds of clients? |
| return result | ||
|
|
||
|
|
||
| def _plan_schema_names(selection: Selection) -> dict[ShapeId, str]: |
There was a problem hiding this comment.
organize public APIs toward the top of the file and internal APIs towards the bottom
class A:
def m():
def _m():
def a():
class _B:
def _b():There was a problem hiding this comment.
updated all changes to order by visibility
eee35b8 to
9dfce0e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved runtime compatibility, schema emission, selector correctness, and alias-allocation issues affect generated artifacts.
4 open issues
6 resolved since last review
Preserve primitive target identity and inherited defaults Handle symlink aliases during output collision and cleanup checks Track qualification in traversal visited state Exclude streaming unions from declaration name checks Allow empty paths in scoped context references Include virtual trait shapes in selector starting set
Review effort: Balanced
9dfce0e to
ba2bae3
Compare
ba2bae3 to
a509722
Compare
65953e4 to
e19987a
Compare
6bf3583 to
aa092a5
Compare
5bbd9cc to
0ffed12
Compare
| source = staging.joinpath(*PurePosixPath(path).parts) | ||
| target = output.joinpath(*PurePosixPath(path).parts) | ||
| target.parent.mkdir(parents=True, exist_ok=True) | ||
| os.replace(source, target) |
There was a problem hiding this comment.
Can you walk me through the intended output ownership and overwrite behavior here? I noticed we preserve modified stale files, but replace current files even if they've been edited or weren't generated by us. Curious about the reasoning behind that distinction and how you expect callers to use the output directory.
There was a problem hiding this comment.
The intended distinction is between current and stale generated files. Current files are replaced because the new generation is authoritative, while modified stale files are preserved to avoid losing changes.
That said, I don't think we should replace a current file the manifest doesn’t own. I think we should treat that as a collision and require a clean directory for initial generation.
jonathan343
left a comment
There was a problem hiding this comment.
One thing I'd like us to keep in mind as this develops is customers eventually calling codegen from Python to customize symbols or contribute files and dependencies. We don't need to define that API yet, but let's keep the generation pipeline usable independently of the CLI so we have a straightforward path to supporting it.
| if self.kind == "empty": | ||
| return False | ||
| if self.kind == "node" and self.value is None: | ||
| return False |
There was a problem hiding this comment.
Can we distinguish a missing trait from one whose value is null? A member with @default(null) currently fails [trait|default], even though the Smithy CLI selects it. This can silently exclude structures from generation.
There was a problem hiding this comment.
yes, a null trait value is still present. I’ll distinguish from a missing value and add coverage for @default(null).
| self._union_unknown_names[shape.id] = result | ||
| return result | ||
|
|
||
| def type_reference(self, shape_id: ShapeId | str) -> TypeReference: |
There was a problem hiding this comment.
How should callers customize type resolution? Overriding type_reference() for timestamps changes direct references, but lists still resolve to list[datetime] because nested resolution bypasses the override. If we intend to support this, the same rule should apply in both cases.
There was a problem hiding this comment.
Nested references should use the same resolution path as direct references. I’ll route collection arguments through type_reference() and add coverage for customized list and map targets.
Agreed. I’m keeping the stack layered that way. This PR establishes reusable selection, symbols, planning, dependencies, and output management. The model PR builds on those, and CLI integration stays in the final PR. The CLI only assembles the context, runs the generator, and publishes files, so the pipeline remains usable directly from Python. |


Issue #, if available:
#797
Description
Adds native Python schema generation and the supporting infrastructure for standalone types packages.
This PR includes the approved standalone types design and implements the schema-generation portion. Model generation and CLI integration will follow in separate PRs.
Changes
smithy-core.Follow-up work
smithy-python generate typesartifact pipeline with the CLI.Compatibility
The Java generator remains authoritative until the native implementation passes complete parity validation. Existing generation and generated packages are unaffected.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.