Repository navigation
fix: correct generated runtime install guidance - #4299
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughGenerated Python banners now recommend ChangesSDK banner updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9664033258
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rs (1)
761-771: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the leaf banner in the regression test.
PYTHON_BANNERandHEADERare separate constants, but this test only inspects the root__init__.py. Add assertions for a generated leaf such asbaml/__init__.py; otherwise, a regression inHEADERcould reintroduce stale installation guidance without failing the test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rs` around lines 761 - 771, Extend generated_sdk_lists_python_runtime_install_commands_in_preferred_order to inspect the generated leaf baml/__init__.py in addition to the root __init__.py, asserting it contains the preferred baml-bridge commands and excludes the stale installation guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@baml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rs`:
- Around line 761-771: Extend
generated_sdk_lists_python_runtime_install_commands_in_preferred_order to
inspect the generated leaf baml/__init__.py in addition to the root __init__.py,
asserting it contains the preferred baml-bridge commands and excludes the stale
installation guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1736a25c-7089-4b6e-9dbb-bcc9999a3ba7
📒 Files selected for processing (4)
baml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/lib.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/sdkgen_typescript.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/sdkgen_typescript_web.rs
Binary size checks passed✅ 7 passed
Generated by |
9664033 to
1303a8f
Compare
|
Addressed the CodeRabbit nitpick in |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rs`:
- Around line 53-55: Update the Python Conda installation flow to emit “conda
install baml-bridge=={runtime_version}” instead of invoking pip under conda.
Apply this in the generator output and HEADER in
baml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rs (lines 53-55
and 86-88), then update the regression expectation there (lines 805-815) and the
E2E expectation in baml_language/crates/baml_cli/tests/exit_code_e2e.rs (lines
323-327) to match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b9781622-d4f4-4d12-a5ff-61ef105908dd
📒 Files selected for processing (9)
baml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_cli/tests/exit_code_e2e.rsbaml_language/sdk_tests/harness_setup/src/python_pydantic2.rsbaml_language/sdk_tests/harness_setup/src/typescript.rsbaml_language/sdk_tests/harness_setup/src/typescript_web.rsbaml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/lib.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/sdkgen_typescript.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/sdkgen_typescript_web.rs
1303a8f to
e390d71
Compare
e390d71 to
2b5a4d1
Compare
Summary
uv add baml-bridge,pip install baml-bridge, and a Conda-environment install of the published PyPI package@boundaryml/baml-bridgeand@boundaryml/baml-bridge-webbaml-cliis available from the runtime packageRoot cause
The v1 Python SDK emitter hardcoded
pip install bamleven though generated code imports the separately publishedbaml-bridgeruntime. The shared TypeScript emitter selected the correct runtime package but only showed one package-manager command.baml-bridgeis published to PyPI rather than Conda channels, so the Conda-oriented option installs the PyPI wheel inside the active Conda environment.The commands are intentionally unpinned: the project does not have a reliable mapping between the
baml.tomltoolchain version and published Python/runtime package versions.Validation
cargo fmt --manifest-path baml_language/Cargo.toml --package sdkgen_python_pydantic2 --package sdkgen_typescript_shared -- --checkcargo test --manifest-path baml_language/Cargo.toml -p sdkgen_python_pydantic2 -p sdkgen_typescript_shared(118 Python tests and 45 TypeScript tests)cargo clippy --manifest-path baml_language/Cargo.toml -p sdkgen_python_pydantic2 -p sdkgen_typescript_shared --all-targets -- -D warningsLinear: https://linear.app/boundaryml2/issue/B-1121/baml-generate-points-to-the-wrong-package
Summary by CodeRabbit
baml-bridgeinstallation commands for uv, pip, and conda.