-
Notifications
You must be signed in to change notification settings - Fork 259
feat(python): Consolidate mutually recursive types to fix circular imports #9888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(python): Consolidate mutually recursive types to fix circular imports #9888
Conversation
- Implemented CircularDependencyDetector using Tarjan's algorithm to find strongly connected components in the type dependency graph - Modified PydanticModelGenerator to consolidate circular clusters into single files (e.g., and_operator_all.py) - Added is_in_circular_cluster() to context to skip update_forward_refs() for types in circular clusters (prevents Pydantic recursion errors) - Added deep-circular-references test fixture mimicking elevenlabs AST operator structure This matches the working solution demonstrated in elevenlabs-python: consolidate mutually recursive types + skip update_forward_refs = no circular imports AND no Pydantic recursion errors. Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should just be empty {}, see the other generators.yml in test-definitions
seed/python-sdk/seed.yml
Outdated
deep-circular-references: | ||
- customConfig: | ||
pydantic_config: | ||
version: v2 | ||
outputFolder: no-custom-config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add fixtures for v1 and v1_on_v2 (or whatever that's called)
…est fixtures - Fixed import sorting (ruff I001 errors) - Removed unused AST import - Simplified generators.yml to empty groups (per test convention) - Added pydantic-v1 and pydantic-v1-on-v2 test fixtures per review feedback Co-Authored-By: [email protected] <[email protected]>
…vanced Per feedback, consolidated the deep operator types (AndOperator, OrOperator, etc.) into the existing circular-references-advanced test fixture instead of creating a new test definition. Changes: - Added operator types to circular-references-advanced/definition/ast.yml - Updated seed.yml to test circular-references-advanced with all pydantic versions (v1, v2, v1_on_v2) - Deleted deep-circular-references test definition This provides better test coverage for the circular import consolidation fix. Co-Authored-By: [email protected] <[email protected]>
When types are consolidated into a single file (e.g., node_all.py), other parts of the codebase still need to import from the individual files (node.py, tree.py). This fix creates stub files that re-export from the consolidated file, ensuring backward compatibility and correct imports. Changes: - Added stub file generation in _generate_circular_cluster() - Each type in a circular cluster gets its own stub file that imports from the consolidated file - Imported AST module needed for CodeWriter Test results: - examples:no-custom-config now passes (was failing with import errors) Co-Authored-By: [email protected] <[email protected]>
The previous implementation used source_file_factory.create() for stub files, which automatically registered exports. This caused duplicate imports when both the stub file and consolidated file tried to export the same type. Solution: Use project.add_file() for stub files, which writes files directly without registering exports. This ensures __init__.py only imports from the consolidated file. Changes: - Replaced source_file_factory.create() with direct file writing for stubs - Stub files no longer register their exports in __init__.py - Fixed the elevenlabs SDK generation issue where SystemToolConfigInputParams was imported twice Generated new circular-references-advanced:pydantic-v2 fixture showing the consolidation pattern. Co-Authored-By: [email protected] <[email protected]>
When types have TypedDict request variants, they get exported from both: - types/type_name.py (normal Pydantic model) - types/requests/type_name.py (TypedDict params variant) This was causing duplicate exports in SDK __init__.py files. The fix now creates stub files in both locations, importing from the consolidated *_all.py file in the parent types/ directory. Co-Authored-By: [email protected] <[email protected]>
For types in circular clusters, we need to generate both: 1. TypedDict request variants (in requests/ directory) - generated normally 2. Consolidated Pydantic models (*_all.py) - all models in one file 3. Stub files for Pydantic models - re-export from consolidated file Previously we were only creating stub files in the requests/ directory, which caused import errors. Now we generate the TypedDict variants normally and only create stubs for the Pydantic models in types/. Co-Authored-By: [email protected] <[email protected]>
Devin is currently unreachable - the session may have died. |
Description
Linear ticket: N/A (reported via GitHub issue)
Session: https://app.devin.ai/sessions/c031897cbe8e43b1965c05d812d77295
Requested by: [email protected] (@tjb9dc)
Fixes circular import issues in the Fern Python SDK generator, specifically addressing deeply recursive type definitions that cause Pydantic recursion errors and duplicate exports. This was reported in elevenlabs/elevenlabs-python#637.
The core issue: When types are mutually recursive (like AST operators where
AndOperator
referencesOrOperator
which references back toAndOperator
), generating them in separate files causes:update_forward_refs()
types/
andtypes/requests/
)Changes Made
CircularDependencyDetector
class that uses Tarjan's algorithm to find strongly connected components (SCCs) in the type dependency graphPydanticModelGenerator
to:*_all.py
files containing all types in a clusterand_operator.py
) that re-export from the consolidated filetypes/requests/
subdirectory for TypedDict request variants to prevent duplicate exports in SDK__init__.py
update_forward_refs()
for types in circular clusters to prevent Pydantic recursioncircular-references-advanced
API definitionTesting
circular-references-advanced:pydantic-v2
fixture locallyKey Areas for Review
pydantic_model_generator.py
):The import path for request variant stubs is
from ..{consolidated_filename}
- this assumesrequests/
is a subdirectory of the type's location. Please verify this is correct for all SDK structures.fern_aware_pydantic_model.py
):We now skip
update_forward_refs()
for types in circular clusters. This prevents Pydantic recursion but could affect type resolution. Please verify this doesn't break any existing use cases.The circular dependency detection uses Tarjan's algorithm. Please review the algorithm implementation in
circular_dependency_detector.py
for correctness.Stub files use
project.add_file()
instead ofwrite_source_file()
to avoid double-registration in module exports. This is intentional but worth understanding.Notes
types/
andtypes/requests/