Repository navigation
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 Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughRust SDK generation, bridge runtime support, CFFI artifact builds, release-plan versioning, consumer verification, and Rust SDK test coverage were added. Release workflows now build, package, verify, publish, and expose Rust-related artifacts through toolchain manifests. ChangesRust SDK and release pipeline
Estimated code review effort: 5 (Critical) | ~120 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):
|
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: 9
🧹 Nitpick comments (11)
baml_language/sdks/rust/bridge_rust/src/error.rs (2)
65-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider delegating
Error::source()for wrapped errors.
Error::DecodeandError::SdkwrapDecodeError/SdkError, both of which implementstd::error::Error, but the blanketimpl<E: fmt::Debug> std::error::Error for Error<E> {}never overridessource(). Consumers walking the error chain (e.g. viaanyhow/eyrereporting, ortracing) will lose visibility into the underlying cause.♻️ Proposed fix
-impl<E: fmt::Debug> std::error::Error for Error<E> {} +impl<E: fmt::Debug> std::error::Error for Error<E> { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Error::Decode(e) => Some(e), + Error::Sdk(e) => Some(e), + _ => None, + } + } +}🤖 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/rust/bridge_rust/src/error.rs` around lines 65 - 98, Update the std::error::Error implementation for Error<E> to override source() for Error::Decode and Error::Sdk, returning the wrapped errors as their underlying causes; return None for all other variants and preserve the existing Display behavior.
65-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo unit tests for
Displayformatting.
Error<E>andDecodeError'sDisplayimpls have branching logic (optionalclass_name, multi-line trace rendering) but no#[cfg(test)]coverage in this file, unlikedownload.rsin the same PR which does test its formatting helpers.As per coding guidelines,
**/*.rs: "Prefer writing Rust unit tests over integration tests where possible."🤖 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/rust/bridge_rust/src/error.rs` around lines 65 - 172, Add #[cfg(test)] unit tests in this file covering the Display implementations for Error and DecodeError, including the optional class_name branches, multi-line trace output, and each DecodeError variant. Assert the exact formatted strings while keeping the production formatting logic in Error::fmt, DecodeError::fmt, and write_trace unchanged.Source: Coding guidelines
baml_language/sdk_tests/crates/rust/function_calls/customizable/test_cancellation.rs (1)
65-78: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoffTiming-based cancellation assertions could be flaky under CI load.
Several tests rely on a fixed 50ms sleep racing a 2000ms call; under heavy CI contention the timer could fire late enough to cause spurious failures/successes. This mirrors the ported Python design, so it's a pre-existing trade-off rather than a regression — flagging for awareness only.
Also applies to: 89-96, 141-152
🤖 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/sdk_tests/crates/rust/function_calls/customizable/test_cancellation.rs` around lines 65 - 78, Review the timing-based cancellation assertions in the scoped test around throws_test::SleepMs_with_ctx, along with the corresponding cases at the other referenced ranges, and replace the fixed sleep/race synchronization with a deterministic cancellation signal or readiness handshake. Preserve the existing expectation that the call is cancelled and the _assert_fast_cancellation validation remains intact.baml_language/crates/bridge_ctypes/build.rs (1)
38-48: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winClear the vendor directory before regenerating to avoid stale committed files.
If a
.protomessage/package is renamed or removed,prost_buildwon't delete the old generated file frombridge_rust/src/wire, leaving a stale file committed and shipped in the published SDK even though the CI proto-sync job may only check freshness of files that still get regenerated.♻️ Proposed fix
let rust_vendor_out = manifest_dir.join("../../sdks/rust/bridge_rust/src/wire"); + if rust_vendor_out.is_dir() { + std::fs::remove_dir_all(&rust_vendor_out)?; + } std::fs::create_dir_all(&rust_vendor_out)?;🤖 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/crates/bridge_ctypes/build.rs` around lines 38 - 48, Clear the existing rust_vendor_out directory before prost_build regenerates files, then recreate it and proceed with the current vendor_config compilation flow. Update the build logic around rust_vendor_out so removed or renamed proto outputs cannot remain committed as stale files.baml_language/crates/bex_engine/src/conversion.rs (1)
73-86: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAddress the cycle-detection bug.
The newly added comment documents a critical bug where self-referencing values cause infinite recursion.
Do you want me to generate a fix that implements the suggestedHeapPtrcycle detection to prevent the engine from hanging?🤖 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/crates/bex_engine/src/conversion.rs` around lines 73 - 86, Implement cycle detection in the recursive conversion function in conversion.rs by tracking visited HeapPtr identities during traversal. When a HeapPtr is encountered again, stop recursion and return an EngineError so callers receive the envelope’s error arm instead of hanging; preserve existing conversion behavior for acyclic values.baml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_forward_refs.rs (1)
18-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEmpty test bodies silently "pass" instead of signaling pending work.
test_round_trip_rec_listandtest_round_trip_rec_list_with_othercompile and run as no-op passing tests. Marking them#[ignore = "..."](using the existing DIVERGENCE comment as the reason) would make CI test reports accurately reflect that these are pending, not verified.♻️ Suggested fix
#[test] +#[ignore = "recursive union alias — no Rust representation yet"] fn test_round_trip_rec_list() { // DIVERGENCE(rust): ... }🤖 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/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_forward_refs.rs` around lines 18 - 49, Mark test_round_trip_rec_list and test_round_trip_rec_list_with_other with #[ignore = "..."], using their existing DIVERGENCE explanation as the ignore reason so CI reports them as pending rather than passing no-op tests.baml_language/sdks/rust/bridge_rust/src/baml_value.rs (1)
259-322: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate map encode/decode logic between
IndexMapandHashMapimpls.The
to_baml/from_bamlbodies forIndexMap<K,V>andHashMap<K,V>are identical apart from the container type. Extracting shared helpers would prevent the two paths from silently diverging on future fixes (e.g. error handling changes).♻️ Suggested refactor
+fn map_to_baml<K: BamlMapKey, V: __BamlValuePrivate>( + entries: impl Iterator<Item = (K, V)>, +) -> wire::InboundValue { + inbound(In::MapValue(wire::InboundMapValue { + entries: entries + .map(|(k, v)| wire::InboundMapEntry { + key: Some(k.to_baml_key()), + value: Some(v.to_baml()), + }) + .collect(), + })) +} + +fn map_from_baml<K: BamlMapKey, V: __BamlValuePrivate, C: FromIterator<(K, V)>>( + v: wire::BamlOutboundValue, +) -> Result<C, DecodeError> { + let v = unwrap(v); + match v.value { + Some(Out::MapValue(map)) => map + .entries + .into_iter() + .map(|entry| { + Ok(( + K::from_baml_key(&entry.key)?, + V::from_baml(entry.value.unwrap_or_default())?, + )) + }) + .collect(), + _ => Err(wrong_type("map", &v)), + } +} + impl<K: BamlMapKey, V: __BamlValuePrivate> __BamlValuePrivate for indexmap::IndexMap<K, V> { fn to_baml(&self) -> wire::InboundValue { - inbound(In::MapValue(wire::InboundMapValue { - entries: self - .iter() - .map(|(k, v)| wire::InboundMapEntry { - key: Some(k.to_baml_key()), - value: Some(v.to_baml()), - }) - .collect(), - })) + map_to_baml(self.iter().map(|(k, v)| (k.clone(), /* borrow-based variant needed */ v))) } ...Note: since
to_bamltakes&self, the shared helper would need a borrowing variant (Iterator<Item = (&K, &V)>) rather than owned tuples as sketched above; adjust signatures accordingly.🤖 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/rust/bridge_rust/src/baml_value.rs` around lines 259 - 322, Extract shared map encode/decode helpers for the identical logic used by the IndexMap and HashMap implementations of __BamlValuePrivate, with the encode helper accepting borrowed (&K, &V) iterator items as required by to_baml. Update both impls to delegate to these helpers while preserving their container-specific collection behavior and existing error handling.baml_language/sdks/rust/bridge_rust/src/decode.rs (1)
1-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the decode logic in this module.
decode_result,ClassFields,enum_variant, and the literal/error-message helpers implement non-trivial protocol-decoding rules (exit-panic short-circuit,MissingFieldvsWrongTypedistinction, float-literal parse fallback, FQN mismatch checks) but ship with no#[cfg(test)]coverage in this diff, unliketranslate_ty.rsin the same PR which tests its logic inline. As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible" — this module is a good candidate since it can be exercised directly with constructedwire::BamlOutboundValue/wire::BamlOutboundResultvalues without needing the full bridge/engine.🤖 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/rust/bridge_rust/src/decode.rs` around lines 1 - 223, Add a #[cfg(test)] module to decode.rs with focused unit tests for decode_result, ClassFields, enum_variant, unwrap/literal_to_plain, and render_message. Construct wire values directly and cover successful decoding, malformed/empty envelopes, FQN mismatches, MissingField versus WrongType, float literal parse fallback, error-message extraction, and non-exit panic handling; avoid testing the process-terminating exit path.Source: Coding guidelines
.github/workflows/build2-bridge-cffi.reusable.yaml (1)
212-224: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRoute step outputs through
env:instead of inline template expansion (zizmor: template-injection).
steps.artifact.outputs.dir/assetare interpolated directly into therun:script. Even though today's values are derived from the workflow's own hardcoded matrix, this bypasses shell quoting and is exactly the pattern zizmor flags. The "Stamp release plan" step above already uses the saferenv:pattern — mirror it here.🔒 Proposed fix
- name: Smoke test the built library if: matrix._.smoke working-directory: baml_language + env: + ARTIFACT_DIR: ${{ steps.artifact.outputs.dir }} + ARTIFACT_ASSET: ${{ steps.artifact.outputs.asset }} run: | set -euo pipefail expected="$(jq -r .canonical_version ../release-plan.json)" python3 ../scripts/smoke-bridge-cffi.py \ - "${{ steps.artifact.outputs.dir }}/${{ steps.artifact.outputs.asset }}" \ + "${ARTIFACT_DIR}/${ARTIFACT_ASSET}" \ "$expected"🤖 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 @.github/workflows/build2-bridge-cffi.reusable.yaml around lines 212 - 224, Update the “Smoke test the built library” step to pass steps.artifact.outputs.dir and steps.artifact.outputs.asset through the step’s env block, then reference those environment variables in the Python command instead of inline GitHub template expressions. Preserve the existing smoke-test arguments and behavior.Source: Linters/SAST tools
baml_language/sdks/rust/sdkgen_rust/src/analyze.rs (1)
350-438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for
compute_sccsandcompute_renames.Both are intricate, hand-rolled algorithms (iterative Tarjan SCC; cascading module-path renames) with correctness-critical outcomes (wrong output would emit stack-overflowing or non-compiling generated code), yet neither has any
#[cfg(test)]coverage in this file, unlikerouting.rs/capi.rs/manifest.rselsewhere in this PR. Worth covering at least: a multi-class cycle, a self-loop-only class, and a rename cascading through 2+ nested collision levels.As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible."
Also applies to: 443-495
🤖 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/rust/sdkgen_rust/src/analyze.rs` around lines 350 - 438, Add #[cfg(test)] unit tests in analyze.rs covering compute_sccs with a multi-node cycle, a self-loop-only node, and non-cyclic nodes, plus compute_renames with a rename cascading through at least two nested collision levels. Assert the exact SCC membership/equality behavior and final renamed paths, using the existing types and helpers exposed by these functions.Source: Coding guidelines
baml_language/sdks/rust/sdkgen_rust/src/unions.rs (1)
1-327: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the pure union-shape helpers.
shape_key,strip_null,shape_error,variant_name, andarm_is_representableare all pure and easily testable (name derivation, string-literal edge cases, discriminability rules, generic-arg rejection), but this file has no#[cfg(test)]module — coverage currently relies entirely on thesdk_testsintegration crate. As per coding guidelines,**/*.rs: "Prefer writing Rust unit tests over integration tests where possible."🤖 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/rust/sdkgen_rust/src/unions.rs` around lines 1 - 327, Add a #[cfg(test)] module in unions.rs with focused unit tests for shape_key, strip_null, shape_error, variant_name, and arm_is_representable. Cover name derivation, valid and invalid string literals, string-versus-literal discriminability, null stripping, generic-argument rejection, and representative supported/unsupported arms, using the existing Ty, Analysis, and SymbolPool test construction patterns.Source: Coding guidelines
🤖 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 @.github/workflows/build2-rust-sdk.reusable.yaml:
- Around line 65-81: Update the “Package and verify the crate” step to
consistently use the artifact directory produced by cargo package with
--target-dir target/package-build: change the stale-artifact cleanup, crate
glob, and checksum-related paths from target/package to
target/package-build/package. Preserve the existing packaging and checksum flow.
In `@baml_language/sdk_tests/crates/rust/llm_functions/customizable/test_main.rs`:
- Around line 196-251: The two tests mutate the process-wide OPENAI_API_KEY
concurrently, causing a race. Update
test_extract_resume_build_request_includes_openai_api_key and
test_streaming_extract_build_request_includes_openai_api_key to run serially or
protect their environment-variable mutations with a shared mutex, ensuring the
guard is shared by both tests.
In `@baml_language/sdk_tests/crates/rust/setup.sh`:
- Around line 35-36: Update the Cargo build command in setup.sh to target the
declared bridge_ctypes package instead of bridge_cffi, preserving the existing
workspace directory and pre-warm flow.
In `@baml_language/sdks/rust/bridge_rust/src/encode.rs`:
- Around line 26-44: Update encode::class() so parameterized class instances
preserve their concrete generic arguments in BamlTyClass.type_args instead of
always emitting an empty vector. Thread the type arguments through the helper
and its generated-class callers, or provide a generic-aware encoder, while
keeping non-parameterized classes valid.
In `@baml_language/sdks/rust/bridge_rust/src/loader/mod.rs`:
- Around line 99-112: Update set_shared_library_path to acquire the
EXPLICIT_PATH lock before checking engine_loaded() and retain that guard through
the path update, coordinating with LoaderEnv::from_process() so the first-load
snapshot cannot race this setter. Preserve the existing warning and ignored-path
behavior when the engine is already loaded.
In `@baml_language/sdks/rust/bridge_rust/src/runtime.rs`:
- Around line 69-93: Update invoke_sync so wait_blocking cannot block
indefinitely: add a bounded timeout or an explicit cancellation path for the
dispatched receiver, and return the appropriate Error when it expires. Preserve
the existing synchronous-from-async guard and decode behavior for completed
responses.
In `@baml_language/sdks/rust/sdkgen_rust/src/idents.rs`:
- Around line 16-21: Update ident to require the first character of name to be
alphabetic or an underscore, while retaining the existing allowed-character
check for the remaining characters. Ensure digit-leading names such as 3d_shape
trigger the existing generator-bug assertion before Ident conversion.
In `@baml_language/sdks/rust/sdkgen_rust/src/lib.rs`:
- Around line 351-365: Update the name sanitization in routing::route to reserve
the generated internal module names _inlinedbaml and _runtime, preventing user
namespaces from producing conflicting segments while preserving existing keyword
sanitization behavior.
In `@scripts/baml-language-version`:
- Around line 486-496: Update the “rust bridge Cargo.toml version” check in
check_release() to compare against expected.crates_io_version instead of
expected.canonical_version, matching the value written by stamp() to
RUST_BRIDGE_CARGO_TOML. Leave the canonical version check for
RUST_BRIDGE_VERSION_RS unchanged.
---
Nitpick comments:
In @.github/workflows/build2-bridge-cffi.reusable.yaml:
- Around line 212-224: Update the “Smoke test the built library” step to pass
steps.artifact.outputs.dir and steps.artifact.outputs.asset through the step’s
env block, then reference those environment variables in the Python command
instead of inline GitHub template expressions. Preserve the existing smoke-test
arguments and behavior.
In `@baml_language/crates/bex_engine/src/conversion.rs`:
- Around line 73-86: Implement cycle detection in the recursive conversion
function in conversion.rs by tracking visited HeapPtr identities during
traversal. When a HeapPtr is encountered again, stop recursion and return an
EngineError so callers receive the envelope’s error arm instead of hanging;
preserve existing conversion behavior for acyclic values.
In `@baml_language/crates/bridge_ctypes/build.rs`:
- Around line 38-48: Clear the existing rust_vendor_out directory before
prost_build regenerates files, then recreate it and proceed with the current
vendor_config compilation flow. Update the build logic around rust_vendor_out so
removed or renamed proto outputs cannot remain committed as stale files.
In
`@baml_language/sdk_tests/crates/rust/function_calls/customizable/test_cancellation.rs`:
- Around line 65-78: Review the timing-based cancellation assertions in the
scoped test around throws_test::SleepMs_with_ctx, along with the corresponding
cases at the other referenced ranges, and replace the fixed sleep/race
synchronization with a deterministic cancellation signal or readiness handshake.
Preserve the existing expectation that the call is cancelled and the
_assert_fast_cancellation validation remains intact.
In
`@baml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_forward_refs.rs`:
- Around line 18-49: Mark test_round_trip_rec_list and
test_round_trip_rec_list_with_other with #[ignore = "..."], using their existing
DIVERGENCE explanation as the ignore reason so CI reports them as pending rather
than passing no-op tests.
In `@baml_language/sdks/rust/bridge_rust/src/baml_value.rs`:
- Around line 259-322: Extract shared map encode/decode helpers for the
identical logic used by the IndexMap and HashMap implementations of
__BamlValuePrivate, with the encode helper accepting borrowed (&K, &V) iterator
items as required by to_baml. Update both impls to delegate to these helpers
while preserving their container-specific collection behavior and existing error
handling.
In `@baml_language/sdks/rust/bridge_rust/src/decode.rs`:
- Around line 1-223: Add a #[cfg(test)] module to decode.rs with focused unit
tests for decode_result, ClassFields, enum_variant, unwrap/literal_to_plain, and
render_message. Construct wire values directly and cover successful decoding,
malformed/empty envelopes, FQN mismatches, MissingField versus WrongType, float
literal parse fallback, error-message extraction, and non-exit panic handling;
avoid testing the process-terminating exit path.
In `@baml_language/sdks/rust/bridge_rust/src/error.rs`:
- Around line 65-98: Update the std::error::Error implementation for Error<E> to
override source() for Error::Decode and Error::Sdk, returning the wrapped errors
as their underlying causes; return None for all other variants and preserve the
existing Display behavior.
- Around line 65-172: Add #[cfg(test)] unit tests in this file covering the
Display implementations for Error and DecodeError, including the optional
class_name branches, multi-line trace output, and each DecodeError variant.
Assert the exact formatted strings while keeping the production formatting logic
in Error::fmt, DecodeError::fmt, and write_trace unchanged.
In `@baml_language/sdks/rust/sdkgen_rust/src/analyze.rs`:
- Around line 350-438: Add #[cfg(test)] unit tests in analyze.rs covering
compute_sccs with a multi-node cycle, a self-loop-only node, and non-cyclic
nodes, plus compute_renames with a rename cascading through at least two nested
collision levels. Assert the exact SCC membership/equality behavior and final
renamed paths, using the existing types and helpers exposed by these functions.
In `@baml_language/sdks/rust/sdkgen_rust/src/unions.rs`:
- Around line 1-327: Add a #[cfg(test)] module in unions.rs with focused unit
tests for shape_key, strip_null, shape_error, variant_name, and
arm_is_representable. Cover name derivation, valid and invalid string literals,
string-versus-literal discriminability, null stripping, generic-argument
rejection, and representative supported/unsupported arms, using the existing Ty,
Analysis, and SymbolPool test construction patterns.
🪄 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
Run ID: cc541a02-e587-43ae-8da6-ea99767610cb
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (105)
.github/workflows/build2-bridge-cffi.reusable.yaml.github/workflows/build2-rust-sdk.reusable.yaml.github/workflows/cargo-tests.reusable.yaml.github/workflows/ci.yaml.github/workflows/release-baml-language.yml.github/workflows/verify-rust-sdk.reusable.yamlbaml_language/.config/nextest.tomlbaml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_codegen_types/src/generator_fields.rsbaml_language/crates/baml_project/src/client_codegen.rsbaml_language/crates/baml_release/src/manifest.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bridge_ctypes/README.mdbaml_language/crates/bridge_ctypes/build.rsbaml_language/sdk_tests/DEVELOPMENT.mdbaml_language/sdk_tests/README.mdbaml_language/sdk_tests/crates/rust/Cargo.tomlbaml_language/sdk_tests/crates/rust/build.rsbaml_language/sdk_tests/crates/rust/docstrings_etc/customizable/test_main.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/optional_args_static.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_cancellation.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_errors.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_generic_calls.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_generic_inference.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_host_callables.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_main.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_methods_on_classes.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_optional_args.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_raises.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_stdlib_entrypoints.rsbaml_language/sdk_tests/crates/rust/llm_functions/customizable/replay_harness.rsbaml_language/sdk_tests/crates/rust/llm_functions/customizable/test_main.rsbaml_language/sdk_tests/crates/rust/llm_functions/customizable/test_streaming_e2e.rsbaml_language/sdk_tests/crates/rust/setup.ps1baml_language/sdk_tests/crates/rust/setup.shbaml_language/sdk_tests/crates/rust/src/lib.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_aliases.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_class_refs.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_enums.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_forward_refs.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_generics.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_handles.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_lists.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_literals.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_maps.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_media.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_optional.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_primitives.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_recursion.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_routing.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_streams.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_symbol_collisions.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_unions.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_void.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/test_complex_models.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/test_generic.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/test_main.rsbaml_language/sdk_tests/harness_runner/src/lib.rsbaml_language/sdk_tests/harness_setup/Cargo.tomlbaml_language/sdk_tests/harness_setup/src/lib.rsbaml_language/sdk_tests/harness_setup/src/rust.rsbaml_language/sdks/rust/bridge_rust/Cargo.tomlbaml_language/sdks/rust/bridge_rust/README.mdbaml_language/sdks/rust/bridge_rust/build-engine.ps1baml_language/sdks/rust/bridge_rust/build-engine.shbaml_language/sdks/rust/bridge_rust/src/baml_value.rsbaml_language/sdks/rust/bridge_rust/src/capi.rsbaml_language/sdks/rust/bridge_rust/src/completion.rsbaml_language/sdks/rust/bridge_rust/src/decode.rsbaml_language/sdks/rust/bridge_rust/src/encode.rsbaml_language/sdks/rust/bridge_rust/src/error.rsbaml_language/sdks/rust/bridge_rust/src/lib.rsbaml_language/sdks/rust/bridge_rust/src/loader/download.rsbaml_language/sdks/rust/bridge_rust/src/loader/log.rsbaml_language/sdks/rust/bridge_rust/src/loader/mod.rsbaml_language/sdks/rust/bridge_rust/src/runtime.rsbaml_language/sdks/rust/bridge_rust/src/test_support.rsbaml_language/sdks/rust/bridge_rust/src/version.rsbaml_language/sdks/rust/bridge_rust/src/wire/baml_bridge.cffi.v1.rsbaml_language/sdks/rust/bridge_rust/src/wire/mod.rsbaml_language/sdks/rust/bridge_rust/tests/common/mod.rsbaml_language/sdks/rust/bridge_rust/tests/conversions.rsbaml_language/sdks/rust/bridge_rust/tests/live_engine.rsbaml_language/sdks/rust/sdkgen_rust/Cargo.tomlbaml_language/sdks/rust/sdkgen_rust/src/analyze.rsbaml_language/sdks/rust/sdkgen_rust/src/emit/class.rsbaml_language/sdks/rust/sdkgen_rust/src/emit/enum_.rsbaml_language/sdks/rust/sdkgen_rust/src/emit/function.rsbaml_language/sdks/rust/sdkgen_rust/src/emit/mod.rsbaml_language/sdks/rust/sdkgen_rust/src/emit/type_alias.rsbaml_language/sdks/rust/sdkgen_rust/src/emit/union.rsbaml_language/sdks/rust/sdkgen_rust/src/idents.rsbaml_language/sdks/rust/sdkgen_rust/src/lib.rsbaml_language/sdks/rust/sdkgen_rust/src/routing.rsbaml_language/sdks/rust/sdkgen_rust/src/translate_ty.rsbaml_language/sdks/rust/sdkgen_rust/src/unions.rsbaml_language/sdks/rust/verify/baml.tomlbaml_language/sdks/rust/verify/baml_src/main.bamlbaml_language/sdks/rust/verify/consumer_main.rsbaml_language/stow.tomlscripts/baml-language-versionscripts/baml-release-manifestsscripts/smoke-bridge-cffi.py
| - name: Package and verify the crate | ||
| working-directory: baml_language | ||
| run: | | ||
| set -euo pipefail | ||
| # Guard the glob below against any stale artifact (a fresh CI | ||
| # checkout has none, but keep the step idempotent). | ||
| rm -f target/package/baml_bridge-*.crate target/package/baml_bridge-*.crate.sha256 | ||
| # --allow-dirty: the working tree carries the just-stamped version. | ||
| # `cargo package` verifies by compiling the packaged crate in | ||
| # isolation — its deps are all crates.io deps, so this proves the | ||
| # published `.crate` builds standalone. --target-dir isolates that | ||
| # verify build; the `.crate` itself still lands in target/package/. | ||
| cargo package -p baml_bridge --allow-dirty --target-dir target/package-build | ||
| crate="$(ls target/package/baml_bridge-*.crate)" | ||
| (cd "$(dirname "$crate")" && sha256sum "$(basename "$crate")" > "$(basename "$crate").sha256") | ||
| echo "Packaged ${crate}" | ||
| cat "${crate}.sha256" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: check what Cargo version the setup-rust action pins, and see if
# there is any existing project convention for cargo package + --target-dir.
cat .github/actions/setup-rust/action.yml 2>/dev/null || fd -a action.yml .github/actions/setup-rust
rg -n "target-dir" baml_language/sdks/rust/bridge_rust -C2Repository: BoundaryML/baml
Length of output: 3982
🌐 Web query:
Cargo package --target-dir .crate output location package subdirectory target-dir Cargo book
💡 Result:
When you run cargo package with the --target-dir option, it directs Cargo to use the specified directory for all generated artifacts, including the final.crate file [1][2]. By default, cargo package stores the generated.crate file in a subdirectory named target/package [3]. If you specify a custom directory using --target-dir
, Cargo will use /package as the destination for the distributable.crate file [1][3]. Key points regarding this behavior: 1. Consistency: The --target-dir flag is supported by cargo package and behaves similarly to how it works for cargo build [4][5]. It overrides the default workspace target directory (which is normally target in the root of the workspace) [1][6]. 2. Environment Variables and Config: You can achieve the same effect without using the command-line flag by setting the CARGO_TARGET_DIR environment variable or by configuring build.target-dir in your Cargo configuration [1][6][7]. 3. Workspace Behavior: In a Cargo workspace, the default behavior often places artifacts in the target directory at the root of the workspace [6][7]. However, users have historically noted that cargo package might sometimes create target directories relative to the member crate's location depending on the execution context, so explicitly setting --target-dir is a reliable way to ensure a specific, predictable output location [8]. In summary, providing --target-dir effectively tells Cargo to place the packaged output in /package [1][3].Citations:
- 1: https://doc.rust-lang.org/stable/cargo/commands/cargo-package.html
- 2: https://doc.rust-lang.org/cargo/commands/cargo-package.html
- 3: https://rust.googlesource.com/rust-lang/cargo/+show/HEAD/src/doc/man/cargo-package.md
- 4: https://doc.rust-lang.org/stable/cargo/commands/cargo-build.html
- 5: Add target directory parameter --target-dir rust-lang/cargo#5393
- 6: https://doc.rust-lang.org/cargo/reference/config.html?highlight=target
- 7: https://doc.rust-lang.org/nightly/cargo/reference/build-cache.html
- 8:
cargo packagein workspaces doesn't put artifacts in roottargetdir rust-lang/cargo#4304
--target-dir needs to be reflected in the artifact path.
cargo package --target-dir target/package-build writes baml_bridge-*.crate under target/package-build/package/, but this step still deletes and reads target/package/.... Update the glob and checksum path to match the overridden target dir, or remove --target-dir if target/package/ is intended.
🤖 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 @.github/workflows/build2-rust-sdk.reusable.yaml around lines 65 - 81, Update
the “Package and verify the crate” step to consistently use the artifact
directory produced by cargo package with --target-dir target/package-build:
change the stale-artifact cleanup, crate glob, and checksum-related paths from
target/package to target/package-build/package. Preserve the existing packaging
and checksum flow.
| pub fn set_shared_library_path(path: impl Into<PathBuf>) { | ||
| let path = path.into(); | ||
| if crate::capi::engine_loaded() { | ||
| log::warn(&format!( | ||
| "set_shared_library_path called after the BAML library was initialized; \ | ||
| path ignored: {}", | ||
| path.display() | ||
| )); | ||
| return; | ||
| } | ||
| *EXPLICIT_PATH | ||
| .lock() | ||
| .expect("explicit library path lock poisoned") = Some(path); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -a capi.rs baml_language/sdks/rust/bridge_rust
rg -n 'engine_loaded|fn api\(' baml_language/sdks/rust/bridge_rust/src/capi.rsRepository: BoundaryML/baml
Length of output: 322
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cd /home/jailuser/git
echo '--- loader/mod.rs around set_shared_library_path ---'
sed -n '1,140p' baml_language/sdks/rust/bridge_rust/src/loader/mod.rs
echo
echo '--- capi.rs around engine_loaded ---'
sed -n '1,180p' baml_language/sdks/rust/bridge_rust/src/capi.rs
echo
echo '--- search for EXPLICIT_PATH and any synchronization with engine_loaded ---'
rg -n 'EXPLICIT_PATH|engine_loaded\(|Once|Mutex|RwLock|static mut|Atomic' baml_language/sdks/rust/bridge_rust/srcRepository: BoundaryML/baml
Length of output: 15524
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cd /home/jailuser/git
echo '--- loader/mod.rs around LoaderEnv::from_process and explicit_path capture ---'
sed -n '140,220p' baml_language/sdks/rust/bridge_rust/src/loader/mod.rs
echo
echo '--- all EXPLICIT_PATH uses with context ---'
rg -n -C 3 'EXPLICIT_PATH' baml_language/sdks/rust/bridge_rust/src/loader/mod.rsRepository: BoundaryML/baml
Length of output: 3789
Serialize the explicit-path update with the first-load snapshot
set_shared_library_path can race with LoaderEnv::from_process(): a concurrent first load may clone None before this write, so the new path is silently ignored and the warning never fires. Lock EXPLICIT_PATH before the engine_loaded() check/write, or otherwise coordinate this setter with the load path.
🤖 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/rust/bridge_rust/src/loader/mod.rs` around lines 99 - 112,
Update set_shared_library_path to acquire the EXPLICIT_PATH lock before checking
engine_loaded() and retain that guard through the path update, coordinating with
LoaderEnv::from_process() so the first-load snapshot cannot race this setter.
Preserve the existing warning and ignored-path behavior when the engine is
already loaded.
| pub(crate) fn ident(name: &str) -> Ident { | ||
| assert!( | ||
| !name.is_empty() && name.chars().all(|c| c.is_alphanumeric() || c == '_'), | ||
| "generator bug: `{name}` is not representable as a Rust identifier; \ | ||
| symbols with such names must be filtered or mangled before emission" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '== idents.rs outline ==\n'
ast-grep outline baml_language/sdks/rust/sdkgen_rust/src/idents.rs --view expanded || true
printf '\n== idents.rs (lines 1-220) ==\n'
cat -n baml_language/sdks/rust/sdkgen_rust/src/idents.rs | sed -n '1,220p'Repository: BoundaryML/baml
Length of output: 4068
🏁 Script executed:
set -euo pipefail
printf '== rust files mentioning ident() ==\n'
rg -n "fn ident\\(|ident\\(" baml_language/sdks/rust/sdkgen_rust/src -g '*.rs' || true
printf '\n== tests in idents.rs around identifier cases ==\n'
cat -n baml_language/sdks/rust/sdkgen_rust/src/idents.rs | sed -n '1,260p'Repository: BoundaryML/baml
Length of output: 6213
🏁 Script executed:
set -euo pipefail
printf '== find name/identifier definitions feeding ident() ==\n'
rg -n "struct .*Name|type .*Name|enum .*Name|pub struct .*Name|identifier|identifi|lexical|alphanumeric" baml_language/sdks/rust/sdkgen_rust/src baml_language -g '*.rs' || true
printf '\n== trace rust_name fields and name types ==\n'
rg -n "rust_name|name\\.name\\.as_str\\(|name: &str" baml_language/sdks/rust/sdkgen_rust/src -g '*.rs' || trueRepository: BoundaryML/baml
Length of output: 50371
🏁 Script executed:
set -euo pipefail
printf '== file list in rust sdkgen source ==\n'
git ls-files baml_language/sdks/rust/sdkgen_rust/src | sed -n '1,200p'
printf '\n== likely parser/name files ==\n'
fd -a -t f . baml_language/sdks/rust/sdkgen_rust/src baml_language | sed -n '1,200p'Repository: BoundaryML/baml
Length of output: 15378
Identifier check should reject digit-leading names. name.chars().all(|c| c.is_alphanumeric() || c == '_') still accepts 3d_shape, which is not a valid Rust identifier. Add a first-character check (is_alphabetic or _) so this fails with the intended generator-bug message instead of falling through to proc_macro2/syn.
🤖 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/rust/sdkgen_rust/src/idents.rs` around lines 16 - 21,
Update ident to require the first character of name to be alphabetic or an
underscore, while retaining the existing allowed-character check for the
remaining characters. Ensure digit-leading names such as 3d_shape trigger the
existing generator-bug assertion before Ident conversion.
| let (file_path, tokens) = if path.is_empty() { | ||
| ( | ||
| PathBuf::from("src/lib.rs"), | ||
| quote! { | ||
| #![allow(non_camel_case_types, non_snake_case)] | ||
|
|
||
| mod _inlinedbaml; | ||
| mod _runtime; | ||
|
|
||
| pub use _runtime::init; | ||
|
|
||
| #(#mod_decls)* | ||
| #(#item_tokens)* | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '"_inlinedbaml"|"_runtime"' baml_language/sdks/rust/sdkgen_rust/src/analyze.rs baml_language/sdks/rust/sdkgen_rust/src/routing.rsRepository: BoundaryML/baml
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first.
ast-grep outline baml_language/sdks/rust/sdkgen_rust/src/analyze.rs --view expanded || true
ast-grep outline baml_language/sdks/rust/sdkgen_rust/src/routing.rs --view expanded || true
echo
echo "=== Search for module-name reservation / sanitization logic ==="
rg -n "reserve|reserved|sanitize|module|segment|namespace|path" \
baml_language/sdks/rust/sdkgen_rust/src/analyze.rs \
baml_language/sdks/rust/sdkgen_rust/src/routing.rsRepository: BoundaryML/baml
Length of output: 6538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== idents helpers ==="
ast-grep outline baml_language/sdks/rust/sdkgen_rust/src/idents.rs --view expanded || true
echo
echo "=== relevant dir_segment / sanitization code ==="
rg -n "fn dir_segment|dir_segment\\(|reserved|keyword|underscore|sanitize" \
baml_language/sdks/rust/sdkgen_rust/src/idents.rs \
baml_language/sdks/rust/sdkgen_rust/src/analyze.rs \
baml_language/sdks/rust/sdkgen_rust/src/routing.rs
echo
echo "=== inspect routing/analyze around the key logic ==="
sed -n '1,120p' baml_language/sdks/rust/sdkgen_rust/src/routing.rs
echo "---"
sed -n '430,520p' baml_language/sdks/rust/sdkgen_rust/src/analyze.rsRepository: BoundaryML/baml
Length of output: 6919
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' baml_language/sdks/rust/sdkgen_rust/src/idents.rsRepository: BoundaryML/baml
Length of output: 3082
Reserve the internal module names
src/lib.rs always emits mod _inlinedbaml; and mod _runtime;, but routing::route only sanitizes a few keywords, so a user namespace like _runtime can still produce the same segment and make the generated crate fail to compile with a duplicate module declaration.
🤖 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/rust/sdkgen_rust/src/lib.rs` around lines 351 - 365,
Update the name sanitization in routing::route to reserve the generated internal
module names _inlinedbaml and _runtime, preventing user namespaces from
producing conflicting segments while preserving existing keyword sanitization
behavior.
| # Load-bearing beyond drift hygiene: the Rust bridge's loader | ||
| # handshakes the loaded engine's version() against this constant, | ||
| # in development builds too. | ||
| "rust bridge CANONICAL_VERSION": ( | ||
| f'pub(crate) const CANONICAL_VERSION: &str = "{expected.canonical_version}";' | ||
| in RUST_BRIDGE_VERSION_RS.read_text(encoding="utf-8") | ||
| ), | ||
| "rust bridge Cargo.toml version": ( | ||
| f'version = "{expected.canonical_version}"' | ||
| in RUST_BRIDGE_CARGO_TOML.read_text(encoding="utf-8") | ||
| ), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Check the bridge Cargo.toml version against crates_io_version, not canonical_version.
stamp() writes RUST_BRIDGE_CARGO_TOML's version from plan.crates_io_version (line ~415), but check_release() verifies it against expected.canonical_version. These happen to be equal today (identity translation), but the check is validating the wrong property conceptually — if crates.io's encoding ever needs to diverge from canonical (as PyPI's already does), this check would keep comparing against the un-encoded value and could mask a real drift.
🛡️ Proposed fix
"rust bridge Cargo.toml version": (
- f'version = "{expected.canonical_version}"'
+ f'version = "{expected.crates_io_version}"'
in RUST_BRIDGE_CARGO_TOML.read_text(encoding="utf-8")
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Load-bearing beyond drift hygiene: the Rust bridge's loader | |
| # handshakes the loaded engine's version() against this constant, | |
| # in development builds too. | |
| "rust bridge CANONICAL_VERSION": ( | |
| f'pub(crate) const CANONICAL_VERSION: &str = "{expected.canonical_version}";' | |
| in RUST_BRIDGE_VERSION_RS.read_text(encoding="utf-8") | |
| ), | |
| "rust bridge Cargo.toml version": ( | |
| f'version = "{expected.canonical_version}"' | |
| in RUST_BRIDGE_CARGO_TOML.read_text(encoding="utf-8") | |
| ), | |
| # Load-bearing beyond drift hygiene: the Rust bridge's loader | |
| # handshakes the loaded engine's version() against this constant, | |
| # in development builds too. | |
| "rust bridge CANONICAL_VERSION": ( | |
| f'pub(crate) const CANONICAL_VERSION: &str = "{expected.canonical_version}";' | |
| in RUST_BRIDGE_VERSION_RS.read_text(encoding="utf-8") | |
| ), | |
| "rust bridge Cargo.toml version": ( | |
| f'version = "{expected.crates_io_version}"' | |
| in RUST_BRIDGE_CARGO_TOML.read_text(encoding="utf-8") | |
| ), |
🤖 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 `@scripts/baml-language-version` around lines 486 - 496, Update the “rust
bridge Cargo.toml version” check in check_release() to compare against
expected.crates_io_version instead of expected.canonical_version, matching the
value written by stamp() to RUST_BRIDGE_CARGO_TOML. Leave the canonical version
check for RUST_BRIDGE_VERSION_RS unchanged.
638cb73 to
988cfb0
Compare
Summary by CodeRabbit