Repository navigation
feat(bridges): good error messages when baml CLI and installed runtime version don't match - #4315
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe change embeds ChangesEmbedded metadata generation
Bridge ABI and runtime validation
SDK bridge integration
Release version flow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant BamlCLI
participant SDKGenerator
participant GeneratedSDK
participant BridgeCFFI
participant Runtime
BamlCLI->>SDKGenerator: pass embedded baml.toml and bytecode
SDKGenerator->>GeneratedSDK: emit metadata-aware initialization
GeneratedSDK->>BridgeCFFI: initialize bytecode with metadata
BridgeCFFI->>BridgeCFFI: validate bridge identity and toolchain version
BridgeCFFI->>Runtime: load validated bytecode
Possibly related PRs
Suggested reviewers: 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 |
c86a924 to
182d562
Compare
182d562 to
358686c
Compare
sxlijin
changed the title
![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: 9cb51a1340
ℹ️ 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".
| return RegisterProgram( | ||
| contractVersion, | ||
| generatedVersion, | ||
| requiredBridgeVersion); | ||
| bytecode, | ||
| fingerprint, | ||
| embeddedBamlToml: null, | ||
| registry); |
There was a problem hiding this comment.
When an SDK generated before metadata support is used with a different Baml.Bridge package, it calls this overload with generatedVersion and requiredBridgeVersion; forwarding directly to the null-metadata path now ignores both values. The previous implementation rejected either mismatch before loading bytecode, whereas the replacement only verifies the installed bridge against its native runtime, so incompatible legacy-generated bytecode can now reach deserialization or run without an exact generator/bridge check. Preserve the compatibility checks for this overload before forwarding.
Useful? React with 👍 / 👎.
| _ = sdkVersion | ||
| let versionBytes = Array(BamlBridgeIdentity.toolchainVersion.utf8) |
There was a problem hiding this comment.
For Swift SDKs generated before embedded metadata was added, the generated root still calls initialize(bytecode:sdkVersion:) with its generating toolchain version. Discarding that value and registering the installed bridge's own toolchain identity means an older generated SDK paired with a newer bridge always passes registration, losing the exact version-skew rejection this compatibility argument previously provided; raw bytecode then reaches deserialization without any generating-version validation. Compare a non-null legacy sdkVersion with the bridge toolchain version before initializing.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
baml_language/sdks/cpp/sdkgen_cpp/src/lib.rs (1)
2252-2293: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBoth generators pick the metadata-aware runtime call via a fragile string-replace instead of branching directly.
sdkgen_cppandsdkgen_javaboth write the legacy (non-metadata) runtime-init call into a template string first, then callString::replaceto substitute the metadata-aware call whenembedded_baml_tomlisSome.String::replacesilently returns its input unchanged when the search string does not match exactly, so any future formatting change to either template (whitespace, wording) would silently leave the legacy, unvalidated initializer in generated SDKs with no compiler or obvious test failure elsewhere in the codebase. Since the entire point of this PR is to make toolchain-compatibility validation reliable, this pattern should branch onembedded_baml_toml.is_some()up front and construct the correct call text directly, rather than generate-then-patch.
baml_language/sdks/cpp/sdkgen_cpp/src/lib.rs#L2252-L2293: inrender_inlinedbaml, compute theinit_callstring based onembedded_baml_toml.is_some()before the singlewriteln!that assembles theEnsureRuntimefunction body, and remove the trailingbuf.replace(...)step.baml_language/sdks/java/sdkgen_java/src/lib.rs#L393-L424: build thestatic { ... }block's initializer call directly fromembedded_baml_toml.is_some()when constructinganchor_body, rather than formatting the legacy call first and replacing it afterward.🤖 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/cpp/sdkgen_cpp/src/lib.rs` around lines 2252 - 2293, Replace generate-then-patch initialization with direct branching on embedded metadata. In baml_language/sdks/cpp/sdkgen_cpp/src/lib.rs lines 2252-2293, update render_inlinedbaml to compute init_call from embedded_baml_toml.is_some() before the EnsureRuntime writeln and remove buf.replace; in baml_language/sdks/java/sdkgen_java/src/lib.rs lines 393-424, construct anchor_body with the metadata-aware or legacy initializer selected directly by embedded_baml_toml.is_some(), without replacement.baml_language/sdks/go/sdkgen_go/src/lib.rs (1)
4569-4593: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse Go-compatible string escaping for
embeddedBamlToml.
{embedded_baml_toml:?}can emit Rust Debug escapes like\u{XXXX}for non-printable characters, but Go string literals only accept\uXXXXor\UXXXXXXXX. Add a Go-specific string escaper for manifest text or a safe fallback sequence before embedding it inbootstrap.go.Ensure
baml_go.InitializeWithMetadata(bytecode []byte, embeddedBamlToml string) errorexists with this exact signature.🤖 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/go/sdkgen_go/src/lib.rs` around lines 4569 - 4593, Update render_bootstrap to escape embedded_baml_toml using Go-compatible string-literal escaping instead of Rust Debug formatting, including valid \uXXXX or \UXXXXXXXX sequences for non-printable characters before emitting embeddedBamlToml. Also verify the generated SDK exposes baml_go.InitializeWithMetadata with the exact signature (bytecode []byte, embeddedBamlToml string) error, adding or correcting it if needed.
🧹 Nitpick comments (9)
baml_language/crates/bridge_cffi/src/api.rs (1)
99-100: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDocument that
baml_tomlmay be null.
initialize_runtime_from_bytecode_with_metadatatreatsbaml_toml == nullas no manifest metadata; document that behavior on the function pointer type or the correspondingBamlApiV1field so C/CFFI consumers do not assume a non-null pointer is required.🤖 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_cffi/src/api.rs` around lines 99 - 100, Document the nullable-pointer contract for baml_toml on BamlInitializeRuntimeFromBytecodeWithMetadataFn or its corresponding BamlApiV1 field: a null value must be treated as absent manifest metadata, while non-null values continue to provide the manifest. Ensure the documentation is visible to C/CFFI consumers.baml_language/sdks/rust/bridge_rust/src/runtime.rs (1)
21-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a unit test for the new metadata dispatch path.
initialize_from_bytecode_with_metadataadds branching logic (metadata vs. legacy FFI call) and an interior-NUL rejection path. No accompanying unit test is visible for this new logic in the provided context.Add a unit test asserting that a manifest string containing
\0returnsSdkErrorbefore any FFI call executes.
Based on coding guidelines, "Prefer writing Rust unit tests over integration tests where possible" for**/*.rsfiles.🤖 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/runtime.rs` around lines 21 - 49, Add a Rust unit test for initialize_from_bytecode_with_metadata that passes embedded metadata containing an interior NUL, asserts it returns SdkError, and verifies the FFI initialization function is not called.Source: Coding guidelines
baml_language/crates/bridge_cffi/src/ffi/runtime.rs (1)
347-403: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the two new size branches.
The tests cover the full struct and the below-legacy struct. They do not cover:
struct_size == legacy_size, which must derivelegacy_runtime_name()and reusesdk_versionas the bridge runtime version.legacy_size < struct_size < size_of::<BamlBridgeInfoV1>(), which must return"truncated appended BAML bridge registration:".These branches carry the legacy-compatibility contract of this PR. Add them as
#[cfg(test)]unit tests in this module.Based on coding guidelines: "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/crates/bridge_cffi/src/ffi/runtime.rs` around lines 347 - 403, Add #[cfg(test)] unit tests alongside the existing registration tests for the two missing struct-size branches in register_bridge_ffi: verify struct_size == legacy_size derives legacy_runtime_name() and reuses sdk_version for the bridge runtime version, and verify legacy_size < struct_size < size_of::<BamlBridgeInfoV1>() returns a message starting with "truncated appended BAML bridge registration:".Source: Coding guidelines
baml_language/crates/bridge_cffi/src/identity.rs (1)
169-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the new validation branches with unit tests.
register_bridgeadds three failure paths: emptybridge_runtime_name, emptybridge_runtime_version, and a toolchain mismatch. The module tests only cover registry idempotence and conflict. Add unit tests for the three new rejections, including the mismatch message that names both the required toolchain andbaml_version::CANONICAL_VERSION.Based on coding guidelines: "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/crates/bridge_cffi/src/identity.rs` around lines 169 - 190, Add Rust unit tests for the three rejection branches in register_bridge: empty bridge_runtime_name, empty bridge_runtime_version, and an incompatible toolchain_version. Assert each returns an error, and verify the mismatch error includes both the required toolchain version and baml_version::CANONICAL_VERSION while leaving the existing registry tests unchanged.Source: Coding guidelines
baml_language/sdks/swift/Sources/CBamlBridge/include/baml_cffi.h (1)
299-301: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a doc comment for
BamlInitializeRuntimeFromBytecodeWithMetadataFn.Every other function-pointer typedef in this header documents ownership, nullability, and lifetime rules for its parameters (for example,
BamlRegisterBridgeFnat lines 460-467). Document whetherbaml_tomlmay be null for the legacy path, and the ownership/lifetime of the returnedBamlBuffer, to match the surrounding style.🤖 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/swift/Sources/CBamlBridge/include/baml_cffi.h` around lines 299 - 301, Add a documentation comment immediately above BamlInitializeRuntimeFromBytecodeWithMetadataFn describing parameter ownership, nullability—including whether baml_toml may be null for the legacy path—and the ownership and lifetime rules for the returned BamlBuffer, matching the surrounding typedef documentation style.baml_language/sdks/typescript/bridge_typescript_web/tests/runtime_errors.test.ts (1)
66-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the
BamlClientErrorassertion.This path must preserve both the
BamlClientErrortype and the deserialization diagnostic. The message-only assertion does not detect a regression to an unstructured native error.Proposed fix
+ expect(() => BamlRuntime.initializeRuntimeFromBytecode(new Uint8Array([1, 2, 3]))).toThrow(BamlClientError); expect(() => BamlRuntime.initializeRuntimeFromBytecode(new Uint8Array([1, 2, 3]))).toThrow(/Failed to deserialize BAML bytecode/);🤖 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/typescript/bridge_typescript_web/tests/runtime_errors.test.ts` at line 66, Update the test around BamlRuntime.initializeRuntimeFromBytecode to assert that invalid bytecode throws a BamlClientError while still matching the “Failed to deserialize BAML bytecode” diagnostic; retain both type and message validation rather than using only a message-based assertion.baml_language/sdks/cpp/bridge_cpp/include/baml/runtime.h (2)
94-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument why
sdk_versionis now ignored.
initialize_runtime_from_bytecodediscards the caller-suppliedsdk_versionand always registers with the bridge's own canonicaltoolchain_version(). This is a behavior change from using the caller-supplied identity. Add a brief comment explaining thatsdk_versionis retained only for the legacy call signature and no longer affects registration, so a future reader debugging a version mismatch does not assume this parameter still has an effect.🤖 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/cpp/bridge_cpp/include/baml/runtime.h` around lines 94 - 96, Add a brief explanatory comment next to static_cast<void>(sdk_version) in initialize_runtime_from_bytecode, stating that sdk_version remains only for the legacy call signature and does not affect registration, which uses the bridge’s canonical toolchain_version().
94-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared setup between the two initializers.
initialize_runtime_from_bytecode(Lines 94-105) andinitialize_runtime_from_bytecode_with_metadata(Lines 107-118) repeat the same three-step setup:detail::ensure_registered(...),register_unhandled_spawn_error_callback(...), andinstall_shutdown_hook(). Only the final native initialization call differs.Extract a shared private helper (for example
detail::prepare_runtime()) that both functions call before dispatching to their respective native initializer. This removes the duplication and keeps the two entry points from drifting apart if the setup sequence changes later.♻️ Proposed refactor
+inline void prepare_runtime() { + detail::ensure_registered(toolchain_version(), kBridgeRuntimeName, + bridge_runtime_version()); + detail::api().register_unhandled_spawn_error_callback( + baml_cpp_unhandled_spawn_error_trampoline); + install_shutdown_hook(); +} + inline void initialize_runtime_from_bytecode(const uint8_t* bytecode, size_t length, const char* sdk_version) { static_cast<void>(sdk_version); - detail::ensure_registered(toolchain_version(), kBridgeRuntimeName, - bridge_runtime_version()); - detail::api().register_unhandled_spawn_error_callback( - baml_cpp_unhandled_spawn_error_trampoline); - install_shutdown_hook(); + prepare_runtime(); detail::owned_buffer failure{ detail::api().initialize_runtime_from_bytecode(bytecode, length)}; if (!failure.empty()) { throw error(failure.to_string()); } } inline void initialize_runtime_from_bytecode_with_metadata( const uint8_t* bytecode, size_t length, const char* embedded_baml_toml) { - detail::ensure_registered(toolchain_version(), kBridgeRuntimeName, - bridge_runtime_version()); - detail::api().register_unhandled_spawn_error_callback( - baml_cpp_unhandled_spawn_error_trampoline); - install_shutdown_hook(); + prepare_runtime(); detail::owned_buffer failure{ detail::api().initialize_runtime_from_bytecode_with_metadata( bytecode, length, embedded_baml_toml)}; if (!failure.empty()) { throw error(failure.to_string()); } }🤖 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/cpp/bridge_cpp/include/baml/runtime.h` around lines 94 - 118, Extract the repeated setup from initialize_runtime_from_bytecode and initialize_runtime_from_bytecode_with_metadata into a shared private helper, such as detail::prepare_runtime(). Have both initializers call this helper before invoking their respective native initialization functions, preserving the existing setup order and behavior.baml_language/sdk_tests/harness_setup/src/csharp.rs (1)
122-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider centralizing the embedded-manifest TOML template.
This function builds the
[__baml_codegen]TOML manifest as an inline literal. If other per-language harness_setup files duplicate this exact template, a future change tometadata_versionor the table structure requires updating every copy in lockstep.Extract a shared helper (for example in a common harness_setup module) that builds this manifest string, and call it from each language's
generate_fixture.🤖 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/harness_setup/src/csharp.rs` around lines 122 - 130, Centralize construction of the embedded BAML manifest currently defined by the local embedded_baml_toml format string in generate_fixture. Add a shared harness_setup helper that produces the metadata_version and toolchain version tables, then update each language-specific generate_fixture, including the C# flow, to call it instead of duplicating the TOML template.
🤖 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/crates/bridge_cffi/src/ffi/runtime.rs`:
- Around line 189-234: Move the calls to bytecode_preflight_failure and
CStr::from_ptr(...).to_str() from initialize_runtime_from_bytecode and
initialize_runtime_from_bytecode_with_metadata into the panic-catching scope of
initialize_runtime_from_bytecode_inner, or wrap both operations in catch_unwind.
Ensure any panic is converted into the existing UTF-8 diagnostic Buffer instead
of crossing the extern "C" boundary, while preserving null-pointer and
invalid-manifest handling.
In `@baml_language/sdks/csharp/bridge_csharp/src/RuntimeIdentity.cs`:
- Around line 13-17: Rename the public BamlBridge class containing
GetToolchainVersion and GetBridgeRuntimeVersion to avoid shadowing the
BamlBridge namespace, preferably using BamlBridgeVersion or another existing
public type. Update all references to the renamed type while preserving both
RuntimeIdentity-backed version accessors; properties may be used instead if
adopting .NET naming conventions.
In `@baml_language/sdks/python/src/baml_bridge/__init__.py`:
- Around line 32-33: Update the __all__ declaration in baml_bridge to include
the imported get_bridge_runtime_version and get_toolchain_version accessors
alongside get_version, so wildcard imports expose all three version functions.
In `@baml_language/sdks/typescript/bridge_typescript_web/src/lib.rs`:
- Around line 24-33: Update the error mapping in init so
bridge_cffi::register_bridge failures use setup_error with the CLIENT error
category and the original error, rather than JsValue::from_str. Preserve the
successful registration behavior and ensure the returned JsValue retains the
structured bridge error name and code contract.
---
Outside diff comments:
In `@baml_language/sdks/cpp/sdkgen_cpp/src/lib.rs`:
- Around line 2252-2293: Replace generate-then-patch initialization with direct
branching on embedded metadata. In baml_language/sdks/cpp/sdkgen_cpp/src/lib.rs
lines 2252-2293, update render_inlinedbaml to compute init_call from
embedded_baml_toml.is_some() before the EnsureRuntime writeln and remove
buf.replace; in baml_language/sdks/java/sdkgen_java/src/lib.rs lines 393-424,
construct anchor_body with the metadata-aware or legacy initializer selected
directly by embedded_baml_toml.is_some(), without replacement.
In `@baml_language/sdks/go/sdkgen_go/src/lib.rs`:
- Around line 4569-4593: Update render_bootstrap to escape embedded_baml_toml
using Go-compatible string-literal escaping instead of Rust Debug formatting,
including valid \uXXXX or \UXXXXXXXX sequences for non-printable characters
before emitting embeddedBamlToml. Also verify the generated SDK exposes
baml_go.InitializeWithMetadata with the exact signature (bytecode []byte,
embeddedBamlToml string) error, adding or correcting it if needed.
---
Nitpick comments:
In `@baml_language/crates/bridge_cffi/src/api.rs`:
- Around line 99-100: Document the nullable-pointer contract for baml_toml on
BamlInitializeRuntimeFromBytecodeWithMetadataFn or its corresponding BamlApiV1
field: a null value must be treated as absent manifest metadata, while non-null
values continue to provide the manifest. Ensure the documentation is visible to
C/CFFI consumers.
In `@baml_language/crates/bridge_cffi/src/ffi/runtime.rs`:
- Around line 347-403: Add #[cfg(test)] unit tests alongside the existing
registration tests for the two missing struct-size branches in
register_bridge_ffi: verify struct_size == legacy_size derives
legacy_runtime_name() and reuses sdk_version for the bridge runtime version, and
verify legacy_size < struct_size < size_of::<BamlBridgeInfoV1>() returns a
message starting with "truncated appended BAML bridge registration:".
In `@baml_language/crates/bridge_cffi/src/identity.rs`:
- Around line 169-190: Add Rust unit tests for the three rejection branches in
register_bridge: empty bridge_runtime_name, empty bridge_runtime_version, and an
incompatible toolchain_version. Assert each returns an error, and verify the
mismatch error includes both the required toolchain version and
baml_version::CANONICAL_VERSION while leaving the existing registry tests
unchanged.
In `@baml_language/sdk_tests/harness_setup/src/csharp.rs`:
- Around line 122-130: Centralize construction of the embedded BAML manifest
currently defined by the local embedded_baml_toml format string in
generate_fixture. Add a shared harness_setup helper that produces the
metadata_version and toolchain version tables, then update each
language-specific generate_fixture, including the C# flow, to call it instead of
duplicating the TOML template.
In `@baml_language/sdks/cpp/bridge_cpp/include/baml/runtime.h`:
- Around line 94-96: Add a brief explanatory comment next to
static_cast<void>(sdk_version) in initialize_runtime_from_bytecode, stating that
sdk_version remains only for the legacy call signature and does not affect
registration, which uses the bridge’s canonical toolchain_version().
- Around line 94-118: Extract the repeated setup from
initialize_runtime_from_bytecode and
initialize_runtime_from_bytecode_with_metadata into a shared private helper,
such as detail::prepare_runtime(). Have both initializers call this helper
before invoking their respective native initialization functions, preserving the
existing setup order and behavior.
In `@baml_language/sdks/rust/bridge_rust/src/runtime.rs`:
- Around line 21-49: Add a Rust unit test for
initialize_from_bytecode_with_metadata that passes embedded metadata containing
an interior NUL, asserts it returns SdkError, and verifies the FFI
initialization function is not called.
In `@baml_language/sdks/swift/Sources/CBamlBridge/include/baml_cffi.h`:
- Around line 299-301: Add a documentation comment immediately above
BamlInitializeRuntimeFromBytecodeWithMetadataFn describing parameter ownership,
nullability—including whether baml_toml may be null for the legacy path—and the
ownership and lifetime rules for the returned BamlBuffer, matching the
surrounding typedef documentation style.
In
`@baml_language/sdks/typescript/bridge_typescript_web/tests/runtime_errors.test.ts`:
- Line 66: Update the test around BamlRuntime.initializeRuntimeFromBytecode to
assert that invalid bytecode throws a BamlClientError while still matching the
“Failed to deserialize BAML bytecode” diagnostic; retain both type and message
validation rather than using only a message-based assertion.
🪄 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: 68280d71-63a9-48f7-a3b4-d9f1e615efe9
⛔ Files ignored due to path filters (8)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/sdks/csharp/bridge_csharp/src/Generated/V1/BamlGeneratedContract.csis excluded by!**/generated/**baml_language/sdks/typescript/bridge_typescript/dist/index.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/index.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/index.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/native.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/native.jsis excluded by!**/dist/**
📒 Files selected for processing (87)
.github/workflows/ci.yaml.github/workflows/release-baml-language.ymlbaml_language/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_version/src/lib.rsbaml_language/crates/bridge_cffi/Cargo.tomlbaml_language/crates/bridge_cffi/include/baml_cffi.hbaml_language/crates/bridge_cffi/src/api.rsbaml_language/crates/bridge_cffi/src/error.rsbaml_language/crates/bridge_cffi/src/ffi/runtime.rsbaml_language/crates/bridge_cffi/src/identity.rsbaml_language/crates/bridge_cffi/src/lib.rsbaml_language/crates/bridge_cffi/src/lib_native.rsbaml_language/crates/bridge_cffi/tests/abi_assertions.hbaml_language/crates/bridge_cffi/tests/abi_layout.cbaml_language/crates/bridge_cffi/tests/abi_layout.rsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/bridge_surface.test.tsbaml_language/sdk_tests/harness_runner/src/lib.rsbaml_language/sdk_tests/harness_setup/src/csharp.rsbaml_language/sdks/cpp/bridge_cpp/include/baml/detail/loader.hbaml_language/sdks/cpp/bridge_cpp/include/baml/runtime.hbaml_language/sdks/cpp/bridge_cpp/include/baml/version.hbaml_language/sdks/cpp/sdkgen_cpp/src/lib.rsbaml_language/sdks/csharp/bridge_csharp/src/Cffi/NativeApi.csbaml_language/sdks/csharp/bridge_csharp/src/Cffi/NativeTypes.csbaml_language/sdks/csharp/bridge_csharp/src/Proto/HostCallableProtocol.csbaml_language/sdks/csharp/bridge_csharp/src/Proto/MediaProtocol.csbaml_language/sdks/csharp/bridge_csharp/src/Proto/PrimitiveProtocol.csbaml_language/sdks/csharp/bridge_csharp/src/Runtime/ProgramRegistrar.csbaml_language/sdks/csharp/bridge_csharp/src/RuntimeIdentity.csbaml_language/sdks/csharp/bridge_csharp/tests/Baml.Bridge.Tests/Program.csbaml_language/sdks/csharp/sdkgen_csharp/src/lib.rsbaml_language/sdks/csharp/sdkgen_csharp/src/semantic.rsbaml_language/sdks/go/baml_go/internal/cffi/include/baml_cffi.hbaml_language/sdks/go/baml_go/native_unix.gobaml_language/sdks/go/baml_go/native_unsupported.gobaml_language/sdks/go/baml_go/native_windows.gobaml_language/sdks/go/baml_go/runtime.gobaml_language/sdks/go/baml_go/version.gobaml_language/sdks/go/baml_go/version_test.gobaml_language/sdks/go/sdkgen_go/src/lib.rsbaml_language/sdks/java/baml_bridge/src/main/java/baml_bridge/BamlFfi.javabaml_language/sdks/java/baml_bridge/src/main/java/baml_bridge/BamlVersion.javabaml_language/sdks/java/bridge_java/Cargo.tomlbaml_language/sdks/java/bridge_java/src/lib.rsbaml_language/sdks/java/sdkgen_java/src/lib.rsbaml_language/sdks/python/rust/bridge_python/src/baml_core/baml_py/__init__.pyibaml_language/sdks/python/rust/bridge_python/src/errors.rsbaml_language/sdks/python/rust/bridge_python/src/lib.rsbaml_language/sdks/python/rust/bridge_python/src/runtime.rsbaml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rsbaml_language/sdks/python/src/baml_bridge/__init__.pybaml_language/sdks/python/src/baml_bridge/baml_py.pyibaml_language/sdks/rust/bridge_rust/src/capi.rsbaml_language/sdks/rust/bridge_rust/src/lib.rsbaml_language/sdks/rust/bridge_rust/src/runtime.rsbaml_language/sdks/rust/bridge_rust/src/version.rsbaml_language/sdks/rust/sdkgen_rust/src/lib.rsbaml_language/sdks/swift/Sources/BamlBridge/Api.swiftbaml_language/sdks/swift/Sources/BamlBridge/Runtime.swiftbaml_language/sdks/swift/Sources/BamlBridge/RuntimeIdentity.swiftbaml_language/sdks/swift/Sources/CBamlBridge/include/baml_cffi.hbaml_language/sdks/swift/rust/sdkgen_swift/src/lib.rsbaml_language/sdks/typescript/bridge_typescript/src/errors.rsbaml_language/sdks/typescript/bridge_typescript/src/lib.rsbaml_language/sdks/typescript/bridge_typescript/src/runtime.rsbaml_language/sdks/typescript/bridge_typescript/src/version.rsbaml_language/sdks/typescript/bridge_typescript/typescript_src/index.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/native.d.tsbaml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjsbaml_language/sdks/typescript/bridge_typescript_web/src/errors.rsbaml_language/sdks/typescript/bridge_typescript_web/src/lib.rsbaml_language/sdks/typescript/bridge_typescript_web/src/runtime.rsbaml_language/sdks/typescript/bridge_typescript_web/src/version.rsbaml_language/sdks/typescript/bridge_typescript_web/tests/runtime_errors.test.tsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/index.tsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/native.tsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/wasm/bridge_web_core.d.tsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.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.rsrelease/bridge-cffi-public-exports.txtscripts/assemble-go-sdk-mirrorscripts/baml-language-versionscripts/tests/test_baml_language_version.pyscripts/tests/test_release_pipeline_contract.py
💤 Files with no reviewable changes (1)
- baml_language/sdks/java/bridge_java/Cargo.toml
| if bytecode.is_null() && length != 0 { | ||
| return Buffer::from(b"bytecode pointer is null but length is nonzero".to_vec()); | ||
| } | ||
| if let Some(error) = bytecode_preflight_failure(length) { | ||
| return error; | ||
| } | ||
| initialize_runtime_from_bytecode_inner(bytecode, length, None) | ||
| } | ||
|
|
||
| /// Initialize generated bytecode after validating its embedded `baml.toml`. | ||
| #[allow(clippy::not_unsafe_ptr_arg_deref)] | ||
| #[unsafe(no_mangle)] | ||
| pub extern "C" fn initialize_runtime_from_bytecode_with_metadata( | ||
| bytecode: *const u8, | ||
| length: usize, | ||
| baml_toml: *const libc::c_char, | ||
| ) -> Buffer { | ||
| if bytecode.is_null() && length != 0 { | ||
| return Buffer::from(b"bytecode pointer is null but length is nonzero".to_vec()); | ||
| } | ||
| if let Some(error) = bytecode_preflight_failure(length) { | ||
| return error; | ||
| } | ||
| let manifest = if baml_toml.is_null() { | ||
| None | ||
| } else { | ||
| match unsafe { CStr::from_ptr(baml_toml) }.to_str() { | ||
| Ok(manifest) => Some(manifest), | ||
| Err(error) => { | ||
| return Buffer::from( | ||
| format!( | ||
| "BAML startup failed: generation metadata is invalid.\n\nThe embedded `baml.toml` is not valid UTF-8: {error}" | ||
| ) | ||
| .into_bytes(), | ||
| ); | ||
| } | ||
| } | ||
| }; | ||
| initialize_runtime_from_bytecode_inner(bytecode, length, manifest) | ||
| } | ||
|
|
||
| fn bytecode_preflight_failure(length: usize) -> Option<Buffer> { | ||
| crate::validate_bytecode_startup_preconditions(length == 0) | ||
| .err() | ||
| .map(|error| Buffer::from(error.to_string().into_bytes())) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Run the preflight checks inside the panic-catching scope.
bytecode_preflight_failure calls crate::validate_bytecode_startup_preconditions before any catch_unwind. The same applies to CStr::from_ptr(...).to_str(). If any of this code panics, the unwind crosses the extern "C" boundary and the process aborts instead of returning a UTF-8 diagnostic buffer. The previous initializer performed all work inside catch_unwind.
Move the preflight and manifest decoding into initialize_runtime_from_bytecode_inner, or wrap them in their own catch_unwind.
🤖 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_cffi/src/ffi/runtime.rs` around lines 189 - 234,
Move the calls to bytecode_preflight_failure and CStr::from_ptr(...).to_str()
from initialize_runtime_from_bytecode and
initialize_runtime_from_bytecode_with_metadata into the panic-catching scope of
initialize_runtime_from_bytecode_inner, or wrap both operations in catch_unwind.
Ensure any panic is converted into the existing UTF-8 diagnostic Buffer instead
of crossing the extern "C" boundary, while preserving null-pointer and
invalid-manifest handling.
| public static class BamlBridge | ||
| { | ||
| public static string GetToolchainVersion() => RuntimeIdentity.ToolchainVersion; | ||
| public static string GetBridgeRuntimeVersion() => RuntimeIdentity.BridgeRuntimeVersion; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename the public BamlBridge class to avoid shadowing the BamlBridge namespace.
The generated protobuf types live in the BamlBridge.Cffi.V1 namespace. This new public class is named BamlBridge. Inside the bridge assembly, the class name now shadows the namespace root, which is why baml_language/sdks/csharp/bridge_csharp/src/Proto/MediaProtocol.cs (Line 138) and baml_language/sdks/csharp/bridge_csharp/src/Proto/PrimitiveProtocol.cs (Lines 179 and 800) required global::BamlBridge.Cffi.V1.BamlHandle. Every future reference to that namespace needs the same workaround.
Rename the class, for example to BamlBridgeVersion or add the accessors to an existing public type. Alternatively, expose them as properties instead of Get* methods to match .NET conventions.
♻️ Proposed rename
-public static class BamlBridge
+public static class BamlBridgeVersion
{
- public static string GetToolchainVersion() => RuntimeIdentity.ToolchainVersion;
- public static string GetBridgeRuntimeVersion() => RuntimeIdentity.BridgeRuntimeVersion;
+ public static string ToolchainVersion => RuntimeIdentity.ToolchainVersion;
+ public static string BridgeRuntimeVersion => RuntimeIdentity.BridgeRuntimeVersion;
}📝 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.
| public static class BamlBridge | |
| { | |
| public static string GetToolchainVersion() => RuntimeIdentity.ToolchainVersion; | |
| public static string GetBridgeRuntimeVersion() => RuntimeIdentity.BridgeRuntimeVersion; | |
| } | |
| public static class BamlBridgeVersion | |
| { | |
| public static string ToolchainVersion => RuntimeIdentity.ToolchainVersion; | |
| public static string BridgeRuntimeVersion => RuntimeIdentity.BridgeRuntimeVersion; | |
| } |
🤖 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/csharp/bridge_csharp/src/RuntimeIdentity.cs` around lines
13 - 17, Rename the public BamlBridge class containing GetToolchainVersion and
GetBridgeRuntimeVersion to avoid shadowing the BamlBridge namespace, preferably
using BamlBridgeVersion or another existing public type. Update all references
to the renamed type while preserving both RuntimeIdentity-backed version
accessors; properties may be used instead if adopting .NET naming conventions.
| get_bridge_runtime_version, | ||
| get_toolchain_version, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the new version accessors to __all__.
get_bridge_runtime_version and get_toolchain_version are imported at Line 32-33, but __all__ does not list them. from baml_bridge import * will not expose these functions, unlike get_version.
🐛 Proposed fix
"get_runtime",
+ "get_bridge_runtime_version",
+ "get_toolchain_version",
"get_version",Also applies to: 535-567
🤖 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/src/baml_bridge/__init__.py` around lines 32 - 33,
Update the __all__ declaration in baml_bridge to include the imported
get_bridge_runtime_version and get_toolchain_version accessors alongside
get_version, so wildcard imports expose all three version functions.
| pub fn init() -> Result<(), JsValue> { | ||
| bridge_cffi::register_bridge(bridge_cffi::BridgeInfo { | ||
| language: bridge_cffi::BridgeLanguage::Web, | ||
| bridge_runtime_name: version::BRIDGE_RUNTIME_NAME.to_string(), | ||
| bridge_runtime_version: version::BRIDGE_RUNTIME_VERSION.to_string(), | ||
| toolchain_version: version::TOOLCHAIN_VERSION.to_string(), | ||
| }) | ||
| .map(|_| ()) | ||
| .map_err(|error| JsValue::from_str(&error)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return a structured setup error from init.
JsValue::from_str throws a string when bridge registration fails. Consumers cannot read the expected error name or code.
Use setup_error(CLIENT, error) so startup registration failures preserve the bridge error contract.
Proposed fix
- .map_err(|error| JsValue::from_str(&error))
+ .map_err(|error| crate::errors::setup_error(crate::errors::CLIENT, error))🤖 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/typescript/bridge_typescript_web/src/lib.rs` around lines
24 - 33, Update the error mapping in init so bridge_cffi::register_bridge
failures use setup_error with the CLIENT error category and the original error,
rather than JsValue::from_str. Preserve the successful registration behavior and
ensure the returned JsValue retains the structured bridge error name and code
contract.
## Summary - add Python integration coverage for generated-bytecode toolchain mismatches - verify the complete bridge identity diagnostic survives the PyO3 boundary - verify compatibility validation runs before bytecode deserialization ## Root cause and relation to BoundaryML#4315 The B-1473 report used generator `0.15.1-nightly.20260731.a` with `baml_bridge==0.15.0`. That generator predates BoundaryML#4315 and emitted only raw bytecode, so the bridge had no independently readable generator identity and surfaced `Unexpected variant tag: 7`. BoundaryML#4315 already fixed the supported regenerated-SDK path by embedding generation metadata and validating it before deserialization. Current coverage tests metadata emission and core `bridge_cffi` diagnostics separately; this PR closes the remaining Python host-boundary gap. Legacy SDKs generated before BoundaryML#4315 must be regenerated to gain the compatibility preflight because their raw-bytecode-only payload has no toolchain identity to inspect. ## Controlled reproduction | State | Generator | Installed Python bridge | Result | | --- | --- | --- | --- | | Before BoundaryML#4315 | `0.15.1-nightly.20260731.a` | `0.15.0` | Reproduces `BamlPanic: Failed to deserialize BAML bytecode: Unexpected variant tag: 7` | | With BoundaryML#4315 | `0.16.0` | `0.15.1.dev2026080700` | Raises an actionable version-skew `RuntimeError` before deserialization with generated, installed, and required versions plus repair guidance | ## Validation - `uv run pytest -n 0 tests/test_engine.py::TestBasics -q` — 4 passed, 1 expected xfail - `cargo test -p bridge_cffi generated_metadata_tests` — 5 passed - `uv run ruff check tests/test_engine.py` - `git diff --check` Linear: B-1473 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved runtime handling of bytecode generated with an incompatible toolchain. * Displays a clear version-mismatch error with installed and expected versions instead of an initialization failure. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - add `baml toolchain pin <canary|nightly|version|path>` to select a project-local toolchain in the nearest `baml.toml` - share installation, channel activation, and local-path validation behavior with `baml toolchain use` - preserve manifest formatting and comments while replacing conflicting `toolchain.version`, `toolchain.channel`, or `toolchain.path` selectors - update generated-bytecode version-skew diagnostics to recommend the executable pin command - identify each bridge's package ecosystem in upgrade guidance, including “the Python package,” npm, Go, Rust, NuGet, Maven, Swift, and C++ ## Why The existing version-skew error told users to pin `toolchain.version` manually or change their machine-wide default with `baml toolchain use`. BAML did not provide a command that performed the project-local edit. This makes the primary recovery path directly executable, supports the same selector forms as `baml toolchain use`, and keeps the alternative bridge-upgrade guidance specific to the active SDK ecosystem. ## Behavior ```text baml toolchain pin canary baml toolchain pin nightly baml toolchain pin 0.15.1-nightly.20260807.a baml toolchain pin ./target/debug/baml-cli ``` The command: - finds the nearest `baml.toml` from the current directory - validates the manifest before downloading or writing - installs exact versions when missing - installs and activates channel selections - validates local binaries and stores a normalized path - atomically writes the matching `[toolchain]` key: `version`, `channel`, or `path` - removes conflicting selectors while preserving TOML comments and formatting ## Validation - `cargo test -p baml -p bridge_cffi` — 43 wrapper unit tests, 5 wrapper integration tests, 37 bridge unit tests, and bridge ABI/header/unhandled-spawn tests passed - `cargo clippy -p baml -p bridge_cffi --all-targets -- -D warnings` - `cargo fmt --all -- --check` - rebuilt the Python bridge with `uv run maturin develop --uv` - `uv run pytest -n 0 tests/test_engine.py::TestBasics::test_generated_bytecode_version_skew_fails_before_deserialization -q` — 1 passed - `uv run ruff check tests/test_engine.py` - `git diff --check` Linear: [B-1123](https://linear.app/boundaryml2/issue/B-1123/bytecode-version-skew-errors) Follow-up to [BoundaryML#4315](BoundaryML#4315) and [BoundaryML#4380](BoundaryML#4380). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added `baml toolchain pin` to select a project-specific toolchain by version, release channel, or local executable path. - Toolchain settings are saved in the nearest project manifest, with paths normalized automatically. - Added offline status support for verifying pinned toolchains. - **Bug Fixes** - Improved version-mismatch guidance with clear steps to pin or upgrade the toolchain and regenerate code. - Preserved existing manifest formatting and comments when updating toolchain settings. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This branch was previously deployed
1 participant
Summary
baml.tomland owned codegen metadata alongside bytecode in every generated SDKbridge_cffiwhile preserving the legacy null-metadata pathWhy
Generated bytecode previously lacked an independently readable compatibility identity, so version skew surfaced as low-level decode failures and package versions could not reliably identify the required toolchain.
Impact
Newly generated SDKs fail before deserialization with actionable upgrade or downgrade guidance when the bridge and generator toolchains differ. Existing generated SDKs continue using the legacy raw-bytecode initializer.
Validation
Proposal: generated bytecode compatibility and bridge version diagnostics
Summary
baml generatewill embed an independently parseable copy of the project'sbaml.tomlalongside the raw bytecode in every generated SDK. The embedded copy will preserve the project manifest and append a reserved__baml_codegentable containing the metadata schema version and the concrete resolved BAML toolchain version that performed generation.New generated SDKs will pass both the raw bytecode and the embedded manifest to the bridge initializer. When the manifest is present, the resolved toolchain version is the sole compatibility identity for the serialized BAML program. The bridge accepts the bytecode only when
__baml_codegen.metadata_versionis1and__baml_codegen.toolchain.versionexactly equals the toolchain version required by that bridge.Legacy generated SDKs will continue calling the initializer without a manifest. A null manifest deliberately preserves the existing raw-bytecode behavior and skips metadata and toolchain compatibility checks. Legacy SDKs therefore do not receive the improved version-skew diagnostics.
Every bridge will expose its required toolchain version and its own published package version. The required toolchain version uses canonical BAML SemVer. The bridge runtime version preserves the exact spelling used by that ecosystem's dependency manager, including the PEP 440 PyPI version and the Go module
vprefix.Every bridge host will also hardcode its package name for diagnostics. Before bytecode initialization, the host will register its package name, bridge runtime version, and required toolchain version with
bridge_cffi.bridge_cffiwill use that registered identity to validate bytecode and return the complete error and repair guidance as one UTF-8 exception string.Goals
baml.tomlalongside the bytecode in every newly generated SDK for diagnostics and future metadata without modifying the on-disk manifest.scripts/baml-language-versionthe single release-time authority that stamps and verifies every bridge version constant.Non-goals
baml.toml.__baml_codegentable.Compatibility identities
There are three distinct identities:
Source-based runtime initialization does not use this path and does not run generated-bytecode compatibility checks.
Testing
Generator tests
baml.toml.baml.tomlis byte-for-byte unchanged.metadata_versionis always integer1.toolchain.versionis the executing canonical toolchain version even when[toolchain].versionis absent.__baml_codegentable fails generation.Loader tests
Bridge tests
pyproject.toml.package.json.v.Release tooling tests
planproduces canonical and registry-specific identities from one release.stampupdates every bridge constant and package manifest.checkdetects drift between getter constants and dependency manifests.Rollout
This change must ship as one coordinated BAML language release because the generator, metadata-aware initializer, native runtime, host registration data, public getters, and package versions form one contract for newly generated SDKs.
The implementation sequence is:
scripts/baml-language-versionstamping and checks.baml generateemit the augmented manifest as a separate string literal in every generated SDK.bridge_cffibytecode initializer.Legacy generated SDKs continue to load through the existing raw-bytecode path. They do not receive metadata validation, toolchain compatibility checks, or improved version-skew diagnostics until regenerated.
Acceptance criteria
baml.tomlalongside its raw bytecode, with__baml_codegen.metadata_version = 1and a populated__baml_codegen.toolchain.version.scripts/baml-language-version stampwrites every version constant from the frozen release plan, andcheckdetects drift.bridge_cffireturns the complete toolchain-mismatch error and repair guidance as one string.Summary by CodeRabbit