-
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
Open
devin-ai-integration
wants to merge
11
commits into
main
Choose a base branch
from
devin/1760652383-fix-circular-import-consolidation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
fde7adf
feat: Consolidate mutually recursive types to fix circular imports
devin-ai-integration[bot] 230342c
fix: Address PR feedback - fix linting and add pydantic v1/v1_on_v2 t…
devin-ai-integration[bot] 24c3c2b
precommit
tjb9dc 407fc07
refactor: Move deep circular reference test to circular-references-ad…
devin-ai-integration[bot] e52a9cc
fix: Generate stub files for consolidated circular type clusters
devin-ai-integration[bot] 0b00501
fix snapshots, rerun python-sdk seed
tjb9dc a342113
fastapi and pydantic seed
tjb9dc dfa2c8e
remove broken v1_on_v2
tjb9dc dcfff19
fix: Prevent duplicate __init__.py exports for stub files
devin-ai-integration[bot] 0c8322b
fix: Create stub files for request variants in requests/ subdirectory
devin-ai-integration[bot] da458b5
fix: Generate TypedDict request variants for circular types
devin-ai-integration[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
121 changes: 121 additions & 0 deletions
121
generators/python/src/fern_python/generators/pydantic_model/circular_dependency_detector.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
from __future__ import annotations | ||
|
||
from typing import Dict, List, Set, TYPE_CHECKING | ||
|
||
if TYPE_CHECKING: | ||
import fern.ir.resources as ir_types | ||
|
||
|
||
class CircularDependencyDetector: | ||
""" | ||
Detects mutually recursive type clusters that should be consolidated into a single file. | ||
|
||
This addresses the issue where deeply mutually recursive types (like AST operators) | ||
cause circular import errors and Pydantic recursion issues when split across files. | ||
""" | ||
|
||
def __init__(self, ir: ir_types.IntermediateRepresentation): | ||
self._ir = ir | ||
self._type_clusters: Dict[ir_types.TypeId, Set[ir_types.TypeId]] = {} | ||
self._computed = False | ||
|
||
def get_type_cluster(self, type_id: ir_types.TypeId) -> Set[ir_types.TypeId]: | ||
""" | ||
Returns the cluster of mutually recursive types that includes the given type_id. | ||
If the type is not part of a circular dependency cluster, returns a set with just the type_id. | ||
""" | ||
if not self._computed: | ||
self._compute_clusters() | ||
return self._type_clusters.get(type_id, {type_id}) | ||
|
||
def is_in_circular_cluster(self, type_id: ir_types.TypeId) -> bool: | ||
"""Returns True if the type is part of a mutually recursive cluster with 2+ types.""" | ||
cluster = self.get_type_cluster(type_id) | ||
return len(cluster) > 1 | ||
|
||
def _compute_clusters(self) -> None: | ||
""" | ||
Computes clusters of mutually recursive types using graph analysis. | ||
|
||
Algorithm: | ||
1. Build a directed graph of type references | ||
2. Find strongly connected components (SCCs) | ||
3. SCCs with 2+ nodes are circular dependency clusters | ||
""" | ||
graph: Dict[ir_types.TypeId, Set[ir_types.TypeId]] = {} | ||
for type_id, type_decl in self._ir.types.items(): | ||
graph[type_id] = set(type_decl.referenced_types) | ||
|
||
sccs = self._find_strongly_connected_components(graph) | ||
|
||
for scc in sccs: | ||
if len(scc) > 1: | ||
scc_set = set(scc) | ||
for type_id in scc: | ||
self._type_clusters[type_id] = scc_set | ||
else: | ||
self._type_clusters[scc[0]] = {scc[0]} | ||
|
||
self._computed = True | ||
|
||
def _find_strongly_connected_components( | ||
self, graph: Dict[ir_types.TypeId, Set[ir_types.TypeId]] | ||
) -> List[List[ir_types.TypeId]]: | ||
""" | ||
Tarjan's algorithm for finding strongly connected components. | ||
Returns a list of SCCs, where each SCC is a list of type_ids. | ||
""" | ||
index_counter = [0] | ||
stack: List[ir_types.TypeId] = [] | ||
lowlinks: Dict[ir_types.TypeId, int] = {} | ||
index: Dict[ir_types.TypeId, int] = {} | ||
on_stack: Dict[ir_types.TypeId, bool] = {} | ||
sccs: List[List[ir_types.TypeId]] = [] | ||
|
||
def strongconnect(node: ir_types.TypeId) -> None: | ||
index[node] = index_counter[0] | ||
lowlinks[node] = index_counter[0] | ||
index_counter[0] += 1 | ||
stack.append(node) | ||
on_stack[node] = True | ||
|
||
successors = graph.get(node, set()) | ||
for successor in successors: | ||
if successor not in index: | ||
strongconnect(successor) | ||
lowlinks[node] = min(lowlinks[node], lowlinks[successor]) | ||
elif on_stack.get(successor, False): | ||
lowlinks[node] = min(lowlinks[node], index[successor]) | ||
|
||
if lowlinks[node] == index[node]: | ||
scc: List[ir_types.TypeId] = [] | ||
while True: | ||
successor = stack.pop() | ||
on_stack[successor] = False | ||
scc.append(successor) | ||
if successor == node: | ||
break | ||
sccs.append(scc) | ||
|
||
for node in graph.keys(): | ||
if node not in index: | ||
strongconnect(node) | ||
|
||
return sccs | ||
|
||
def get_all_circular_clusters(self) -> List[Set[ir_types.TypeId]]: | ||
"""Returns all circular dependency clusters (with 2+ types).""" | ||
if not self._computed: | ||
self._compute_clusters() | ||
|
||
seen_clusters: Set[frozenset[ir_types.TypeId]] = set() | ||
circular_clusters: List[Set[ir_types.TypeId]] = [] | ||
|
||
for type_id, cluster in self._type_clusters.items(): | ||
if len(cluster) > 1: | ||
cluster_key = frozenset(cluster) | ||
if cluster_key not in seen_clusters: | ||
seen_clusters.add(cluster_key) | ||
circular_clusters.append(cluster) | ||
|
||
return circular_clusters |
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
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
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
6 changes: 6 additions & 0 deletions
6
test-definitions/fern/apis/deep-circular-references/definition/__package__.yml
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
exports: | ||
types: | ||
- AndOperator | ||
- OrOperator | ||
- EqualsOperator | ||
- GreaterThanOperator |
1 change: 1 addition & 0 deletions
1
test-definitions/fern/apis/deep-circular-references/definition/api.yml
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
name: api |
70 changes: 70 additions & 0 deletions
70
test-definitions/fern/apis/deep-circular-references/definition/operators.yml
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
types: | ||
# Boolean operators (AND/OR) that can contain comparison operators | ||
AndOperator: | ||
properties: | ||
children: list<AndOperatorChild> | ||
|
||
AndOperatorChild: | ||
union: | ||
and_operator: AndOperator | ||
or_operator: OrOperator | ||
eq_operator: EqualsOperator | ||
gt_operator: GreaterThanOperator | ||
boolean_literal: boolean | ||
|
||
OrOperator: | ||
properties: | ||
children: list<OrOperatorChild> | ||
|
||
OrOperatorChild: | ||
union: | ||
and_operator: AndOperator | ||
or_operator: OrOperator | ||
eq_operator: EqualsOperator | ||
gt_operator: GreaterThanOperator | ||
boolean_literal: boolean | ||
|
||
# Comparison operators that can have boolean operators on left/right | ||
EqualsOperator: | ||
properties: | ||
left: EqualsOperatorLeft | ||
right: EqualsOperatorRight | ||
|
||
EqualsOperatorLeft: | ||
union: | ||
and_operator: AndOperator | ||
or_operator: OrOperator | ||
eq_operator: EqualsOperator | ||
gt_operator: GreaterThanOperator | ||
number_literal: double | ||
string_literal: string | ||
|
||
EqualsOperatorRight: | ||
union: | ||
and_operator: AndOperator | ||
or_operator: OrOperator | ||
eq_operator: EqualsOperator | ||
gt_operator: GreaterThanOperator | ||
number_literal: double | ||
string_literal: string | ||
|
||
GreaterThanOperator: | ||
properties: | ||
left: GreaterThanOperatorLeft | ||
right: GreaterThanOperatorRight | ||
|
||
GreaterThanOperatorLeft: | ||
union: | ||
and_operator: AndOperator | ||
or_operator: OrOperator | ||
eq_operator: EqualsOperator | ||
gt_operator: GreaterThanOperator | ||
number_literal: double | ||
|
||
GreaterThanOperatorRight: | ||
union: | ||
and_operator: AndOperator | ||
or_operator: OrOperator | ||
eq_operator: EqualsOperator | ||
gt_operator: GreaterThanOperator | ||
number_literal: double |
Oops, something went wrong.
Oops, something went wrong.
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.
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)