Add baml-cli generate and integ-tests#3315
Conversation
|
Deployment failed with the following error: View Documentation: https://vercel.com/docs/two-factor-authentication |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as baml-cli generate
participant Compiler as BAML Compiler
participant Codegen as Python Codegen
participant Codegen_Pool as ObjectPool Builder
participant Files as File System
User->>CLI: Run generate --from --output
CLI->>Compiler: Load & compile .baml files
Compiler->>Compiler: Validate, type-check
Compiler->>CLI: Return diagnostics + HIR
alt Errors present
CLI->>Files: Write diagnostics to stderr
CLI->>User: Exit with error
end
CLI->>Compiler: Discover generator blocks
Compiler->>CLI: Return generators + config
alt No generators found
CLI->>Files: Print example config
CLI->>User: Exit with error
end
CLI->>Codegen_Pool: Build ObjectPool from HIR
Codegen_Pool->>Codegen_Pool: Convert classes/enums/functions
Codegen_Pool->>CLI: Return ObjectPool
loop For each generator
CLI->>Codegen: Generate Python code (codegen_python)
Codegen->>Codegen: Render sync_client.py, async_client.py, runtime.py
Codegen->>CLI: Return file map
CLI->>Files: Create output directory
CLI->>Files: Write .py files
end
CLI->>User: Print summary, exit success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will not alter performance
|
imalsogreg
force-pushed
the
greg/baml-cli-generate
branch
from
1091e84 to
ebddba9
Compare
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ff6c72c-76e5-47d4-8d64-82a65a1d3820
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/integ_tests/python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
baml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/commands.rsbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_cli/src/lib.rsbaml_language/crates/baml_codegen_python/src/_askama/globals.pybaml_language/crates/baml_codegen_python/src/_askama/runtime.pybaml_language/crates/baml_codegen_python/src/lib.rsbaml_language/crates/baml_codegen_python/src/objects.rsbaml_language/crates/baml_codegen_python/src/objects/render/function.rsbaml_language/crates/baml_codegen_python/src/ty.rsbaml_language/crates/baml_codegen_types/Cargo.tomlbaml_language/crates/baml_codegen_types/src/from_tir.rsbaml_language/crates/baml_codegen_types/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/item_tree.rsbaml_language/crates/baml_compiler2_ppir/src/expand.rsbaml_language/crates/baml_type/src/simplify_sap.rsbaml_language/crates/bridge_python/src/runtime.rsbaml_language/integ_tests/baml_src/clients.bamlbaml_language/integ_tests/baml_src/generators.bamlbaml_language/integ_tests/baml_src/test001_basic.bamlbaml_language/integ_tests/python/.gitignorebaml_language/integ_tests/python/baml_integ_tests.egg-info/PKG-INFObaml_language/integ_tests/python/baml_integ_tests.egg-info/SOURCES.txtbaml_language/integ_tests/python/baml_integ_tests.egg-info/dependency_links.txtbaml_language/integ_tests/python/baml_integ_tests.egg-info/requires.txtbaml_language/integ_tests/python/baml_integ_tests.egg-info/top_level.txtbaml_language/integ_tests/python/pyproject.tomlbaml_language/integ_tests/python/tests/__init__.pybaml_language/integ_tests/python/tests/test_001_basic_happy_path.pybaml_language/integ_tests/run.sh
💤 Files with no reviewable changes (1)
- baml_language/crates/baml_codegen_python/src/_askama/globals.py
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
baml_language/crates/baml_cli/src/commands.rs (1)
59-60: 🛠️ Refactor suggestion | 🟠 MajorAdd unit tests for
Generateparsing and dispatch path.This wiring still lacks unit coverage in the crate for (1) Clap parsing into
Commands::Generateand (2)RuntimeCli::run()dispatchingGenerate(args)toargs.run().As per coding guidelines: "
**/*.rs: Prefer writing Rust unit tests over integration tests where possible".Also applies to: 117-117
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dea752fc-57ed-4edd-b223-27858a17e5fa
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/integ_tests/python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
baml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/commands.rsbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_cli/src/lib.rsbaml_language/crates/baml_codegen_python/src/_askama/globals.pybaml_language/crates/baml_codegen_python/src/_askama/runtime.pybaml_language/crates/baml_codegen_python/src/lib.rsbaml_language/crates/baml_codegen_python/src/objects.rsbaml_language/crates/baml_codegen_python/src/objects/render/function.rsbaml_language/crates/baml_codegen_python/src/ty.rsbaml_language/crates/baml_codegen_types/Cargo.tomlbaml_language/crates/baml_codegen_types/src/from_tir.rsbaml_language/crates/baml_codegen_types/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/item_tree.rsbaml_language/crates/baml_compiler2_ppir/src/expand.rsbaml_language/crates/baml_type/src/simplify_sap.rsbaml_language/crates/bridge_python/src/runtime.rsbaml_language/integ_tests/baml_src/clients.bamlbaml_language/integ_tests/baml_src/generators.bamlbaml_language/integ_tests/baml_src/test001_basic.bamlbaml_language/integ_tests/python/.gitignorebaml_language/integ_tests/python/baml_integ_tests.egg-info/PKG-INFObaml_language/integ_tests/python/baml_integ_tests.egg-info/SOURCES.txtbaml_language/integ_tests/python/baml_integ_tests.egg-info/dependency_links.txtbaml_language/integ_tests/python/baml_integ_tests.egg-info/requires.txtbaml_language/integ_tests/python/baml_integ_tests.egg-info/top_level.txtbaml_language/integ_tests/python/pyproject.tomlbaml_language/integ_tests/python/tests/__init__.pybaml_language/integ_tests/python/tests/test_001_basic_happy_path.pybaml_language/integ_tests/run.sh
💤 Files with no reviewable changes (1)
- baml_language/crates/baml_codegen_python/src/_askama/globals.py
baml_language/integ_tests/python/baml_integ_tests.egg-info/dependency_links.txt
Outdated
Show resolved
Hide resolved
baml_language/integ_tests/python/baml_integ_tests.egg-info/SOURCES.txt
Outdated
Show resolved
Hide resolved
baml_language/integ_tests/python/baml_integ_tests.egg-info/top_level.txt
Outdated
Show resolved
Hide resolved
imalsogreg
force-pushed
the
greg/baml-cli-generate
branch
from
ebddba9 to
02ff29b
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/baml_codegen_python/src/objects/render/function.rs (1)
9-96: 🛠️ Refactor suggestion | 🟠 MajorAdd Rust unit tests for the new renderer/coercion paths.
This file currently validates
print_signature, but notprint_sync_impl,print_async_impl,render_args_dict, or coercion behavior (especially nested class/enum returns). Please add focused unit tests here and runcargo test --libfor this Rust change.As per coding guidelines,
**/*.rs: "Prefer writing Rust unit tests over integration tests where possible" and "Always runcargo test --libif you changed any Rust code".
♻️ Duplicate comments (1)
baml_language/crates/baml_cli/src/commands.rs (1)
59-61:⚠️ Potential issue | 🟡 MinorAdd unit tests for
generatecommand parse + dispatch wiring.This introduces a new CLI path but doesn’t add unit coverage in the same area for
RuntimeCliparsing toCommands::Generateandrun()routing, which makes future regressions easier to miss.As per coding guidelines,
**/*.rs: "Prefer writing Rust unit tests over integration tests where possible".Also applies to: 117-117
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 894821d3-cf68-4421-8865-eb5e16d25096
⛔ Files ignored due to path filters (3)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_tests/snapshots/attr_disambiguation/baml_tests__attr_disambiguation__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/stream_types/baml_tests__stream_types__04_tir.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
baml_language/crates/baml_cli/src/commands.rsbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_cli/src/lib.rsbaml_language/crates/baml_codegen_python/src/objects/render/function.rsbaml_language/crates/baml_codegen_types/Cargo.tomlbaml_language/crates/baml_codegen_types/src/from_tir.rsbaml_language/crates/baml_codegen_types/src/lib.rsbaml_language/integ_tests/baml_src/ns_test003/functions.bamlbaml_language/integ_tests/baml_src/ns_test003/types.bamlbaml_language/integ_tests/baml_src/test001_basic.bamlbaml_language/integ_tests/baml_src/test002_directory/functions.bamlbaml_language/integ_tests/baml_src/test002_directory/types.bamlbaml_language/integ_tests/python/pyproject.tomlbaml_language/integ_tests/python/tests/test_001_basic_happy_path.pybaml_language/integ_tests/python/tests/test_002_directory_happy_path.pybaml_language/integ_tests/python/tests/test_003_namespace_happy_path.py
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b46ca0bb-b001-448f-ad92-754f36270cd0
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
baml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_codegen_python/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/item_tree.rsbaml_language/crates/baml_compiler2_ppir/src/expand.rsbaml_language/crates/baml_project/Cargo.tomlbaml_language/crates/baml_project/src/from_tir.rsbaml_language/crates/baml_project/src/lib.rsbaml_language/integ_tests/python/.gitignore
imalsogreg
force-pushed
the
greg/baml-cli-generate
branch
from
0f5d34c to
a313e9d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
baml_language/crates/baml_project/src/client_codegen.rs (2)
41-44:⚠️ Potential issue | 🟠 MajorPreserve the full BAML namespace in
cg::Name.These conversions keep only the leaf name plus
{Types, StreamTypes}, sofoo.Userandbar.Usercollide inObjectPooland the later insert wins. Threadpkg_info.namespace_path/ theQualifiedTypeNamesegments through codegen names instead of inferring namespace only from the$streamsuffix.Based on learnings: "bare names resolve only within the declaring file's namespace (root files in root, ns_llm files in llm, etc.)."
Also applies to: 75-78, 107-110, 158-161, 181-187, 241-257
220-370: 🧹 Nitpick | 🔵 TrivialAdd Rust unit tests for the conversion/simplification helpers.
convert_tir_to_codegen_ty,simplify_union, andnamespace_forare pure logic with edge cases that the new simple integration coverage will not pin down. A small table of unit tests here would catch regressions in nested unions, null ordering/dedup, and namespace routing.As per coding guidelines: "Prefer writing Rust unit tests over integration tests where possible".
baml_language/crates/baml_cli/src/generate.rs (2)
86-89:⚠️ Potential issue | 🟡 MinorThe sample
output_dirresolves one level too deep.Line 199 already appends
baml_client, so the printed exampleoutput_dir "../baml_client"generates into<...>/baml_client/baml_client. Either showoutput_dir ".."in the help text or stop appending the suffix indiscover_generators().Also applies to: 193-200
261-267:⚠️ Potential issue | 🟠 MajorKeep package-level
bsynchronous.
from baml_client import bis the sync import contract in the repo examples. Exportingasync_bas the default turns existing sync call sites into coroutine-returning calls.♻️ Suggested fix
from .sync_client import b as sync_b from .async_client import b as async_b -# Default client is async. -b = async_b +# Default client is sync. +b = sync_bBased on learnings: "In Python, import the autogenerated BAML client using 'from baml_client import b' for synchronous calls or 'from baml_client.async_client import b as async_b' for asynchronous calls".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fb1f5bf1-96c1-4691-8a08-ad72ef435ce9
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
baml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_codegen_python/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/item_tree.rsbaml_language/crates/baml_compiler2_ppir/src/expand.rsbaml_language/crates/baml_project/Cargo.tomlbaml_language/crates/baml_project/src/client_codegen.rsbaml_language/crates/baml_project/src/lib.rsbaml_language/integ_tests/python/.gitignore
imalsogreg
force-pushed
the
greg/baml-cli-generate
branch
4 times, most recently
from
3369936 to
83408d6
Compare
imalsogreg
force-pushed
the
greg/baml-cli-generate
branch
3 times, most recently
from
0772fc9 to
9c34cf6
Compare
imalsogreg
force-pushed
the
greg/baml-cli-generate
branch
from
9c34cf6 to
6442550
Compare
1 participant
baml-clitoPPIRandbaml_python_codegenSummary by CodeRabbit
Release Notes
New Features
baml generateCLI command to generate Python client code from BAML definitionsIntegration Tests