-
Notifications
You must be signed in to change notification settings - Fork 5
autogenerate list of blueprints #1100
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
Conversation
Greptile OverviewGreptile SummaryImplemented auto-generation of blueprint registry from codebase scanning. The PR introduces an AST-based scanner that discovers all blueprint definitions across the codebase and automatically generates the Key Changes:
The auto-generation system scans Python files, identifies top-level variables assigned to Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant BP as Blueprint Files
participant Test as test_all_blueprints_generation.py
participant Gen as AST Scanner
participant AB as all_blueprints.py
participant CLI as dimos CLI
participant Get as get_all_blueprints.py
Note over Dev,AB: Auto-generation System
Dev->>BP: Define blueprint variables<br/>(e.g., unitree_g1, demo_gps_nav)
Dev->>Test: Run pytest
Test->>Gen: Scan codebase with AST parsing
Gen->>BP: Parse Python files
Gen->>Gen: Find autoconnect() calls<br/>and blueprint methods
Gen->>Gen: Extract variable names<br/>(skip _ prefixed)
Gen->>Test: Return discovered blueprints
Test->>AB: Generate/Update all_blueprints.py
Test->>Test: Check for uncommitted changes
Note over CLI,Get: Runtime Blueprint Loading
CLI->>AB: Import all_blueprints dict
CLI->>Get: Call get_blueprint_by_name(name)
Get->>AB: Lookup blueprint path
Get->>Get: Parse "module:attr" string
Get->>BP: Dynamic import module
Get->>CLI: Return ModuleBlueprintSet instance
|
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.
No files reviewed, no comments
|
|
||
| try: | ||
| source = file_path.read_text(encoding="utf-8") | ||
| tree = ast.parse(source, filename=str(file_path)) |
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.
To me parsing and traversing the AST feels too far (e.g. over-engineered). On the receiving end "my blueprint isn't being included, but idk why?" would be some painful debugging of this black-magic. I think a KISS approach would be like pytest's naming scheme: if the file ends in "_blueprint.py" and, when imported, has a .blueprints list, use that. No AST autoconnect-detecting node-traversal.
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.
and instead of committing auto-generated code, and adding build step thats run on-commit, the simple "find _bluprints.py" could be done at runtime.
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.
I think a KISS approach would be like pytest's naming scheme: if the file ends in "_blueprint.py"
Yeah, that sounds great for locating the files. 👍
when imported, has a .blueprints list, use that. No AST autoconnect-detecting node-traversal.
The issue with doing it that way is that you have to import it, which imports all the modules. This would make test slower. But it could be acceptable because we probably have already imported everything when running the test suite. The only other issue would be that all blueprints would have to be importable in all situations, even when dependencies are missing...
and instead of committing auto-generated code, and adding build step thats run on-commit, the simple "find _bluprints.py" could be done at runtime.
Disagree with this because this then forces all modules to be imported on every run. The list of blueprints is intentionally listed as strings to be imported instead of actual imports because it's better to only import the blueprint you want to use.
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.
But it could be acceptable because we probably have already imported everything when running the test suite
We basically import everything just running dimos --help lol. Not that that is a good thing
Disagree with this because this then forces all modules to be imported on every run.
Ah that is a good point. I think I'll have a clean solution (prototype of lazy imports is tested and works) that avoid this but that probably won't be done for another week.
So in the meantime, AST it is I suppose.
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.
We basically import everything just running dimos --help lol.
We import a lot when running --help. But there's so much more! :)
when imported, has a .blueprints list, use that. No AST autoconnect-detecting node-traversal.
Tried it, but it doesn't work for the RealSenseCamera module because it imports pyrealsense2 which is a system installed package. :(
8aad8dc to
8543d7f
Compare
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.
clear improvement imo, without viable alternative proposals? can collect wishlist for this long term
dimos/robot/all_blueprints.pywhen you runpytest dimos/robot/test_all_blueprints_generation.py.git addto confirm. The test fails until you commit the changes.