Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughGeneration now emits a versioned bytecode envelope with embedded, validated ChangesBytecode compatibility
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BamlGenerate
participant EmbeddedBamlToml
participant SerializeBytecode
participant CSharpGenerator
BamlGenerate->>EmbeddedBamlToml: Read and validate baml.toml
EmbeddedBamlToml-->>BamlGenerate: Return manifest with compatibility metadata
BamlGenerate->>SerializeBytecode: Serialize compiled Program and manifest
SerializeBytecode-->>BamlGenerate: Return versioned bytecode envelope
BamlGenerate->>CSharpGenerator: Pass Program and serialized envelope
CSharpGenerator->>CSharpGenerator: Derive callable identities from Program
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):
|
sxlijin
force-pushed
the
agent/bytecode-version-skew
branch
from
July 31, 2026 19:51
cee7fac to
1d5336b
Compare
sxlijin
force-pushed
the
agent/bytecode-version-skew
branch
from
July 31, 2026 20:02
1d5336b to
ab33c73
Compare
This comment has been minimized.
This comment has been minimized.
Binary size checks passed✅ 7 passed
Generated by |
sxlijin
marked this pull request as ready for review
July 31, 2026 20:23
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
baml_language/crates/bex_project/src/lib.rs (2)
117-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCouple the serde field name to
BYTECODE_METADATA_TABLE.Line 119 repeats the literal
"__baml_codegen"that line 99 already defines asBYTECODE_METADATA_TABLE.serde(rename = ...)requires a literal, so the compiler cannot detect divergence. If the const changes, the producer writes the new table name and the runtime rejects every payload with a "missing table" error.Add a unit test that pins the two together.
♻️ Suggested guard test
#[test] fn metadata_table_name_matches_serde_rename() { let toml = format!("[{BYTECODE_METADATA_TABLE}]\nbaml_cli_version = \"x\"\nbytecode_format_version = 1\n"); let parsed: EmbeddedBamlToml = toml::from_str(&toml).expect("parse"); assert!(parsed.codegen.is_some()); }🤖 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_project/src/lib.rs` around lines 117 - 121, Keep EmbeddedBamlToml’s serde rename synchronized with BYTECODE_METADATA_TABLE by adding the suggested unit test in the existing test area. Construct TOML using BYTECODE_METADATA_TABLE, deserialize it into EmbeddedBamlToml, and assert parsed.codegen is present so future constant changes expose a mismatch.
206-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the envelope prefix layout with a test.
Lines 206-210 assume the Borsh encoding places
envelope_versionat bytes 8..12. That holds only whilemagicis declared beforeenvelope_versioninBytecodeEnvelope. If a later change reorders the fields, this pre-check reads thebaml_tomllength prefix instead and rejects valid bytecode with a wrong envelope-version message. No test currently covers the byte offsets or an envelope-version mismatch.Note also that lines 222-226 cannot be reached. Line 200 already proves the first 8 bytes equal
BYTECODE_ENVELOPE_MAGIC, and Borsh decodes those same bytes intomagic.♻️ Suggested tests
#[test] fn envelope_prefix_layout_is_stable() { let bytes = borsh::to_vec(&BytecodeEnvelope { magic: BYTECODE_ENVELOPE_MAGIC, envelope_version: BYTECODE_ENVELOPE_VERSION, baml_toml: String::new(), program: Vec::new(), }) .expect("envelope"); assert_eq!(&bytes[..8], &BYTECODE_ENVELOPE_MAGIC); assert_eq!( u32::from_le_bytes(bytes[8..12].try_into().expect("prefix")), BYTECODE_ENVELOPE_VERSION ); } #[test] fn rejects_unsupported_envelope_version() { let mut bytes = borsh::to_vec(&BytecodeEnvelope { magic: BYTECODE_ENVELOPE_MAGIC, envelope_version: BYTECODE_ENVELOPE_VERSION + 1, baml_toml: manifest(baml_version::CANONICAL_VERSION, BYTECODE_FORMAT_VERSION), program: Vec::new(), }) .expect("envelope"); let message = deserialize_bytecode(&bytes) .expect_err("envelope version mismatch") .to_string(); assert!(message.contains("envelope version"), "{message}"); bytes.truncate(10); let truncated = deserialize_bytecode(&bytes).expect_err("truncated"); assert!(truncated.to_string().contains("truncated"), "{truncated}"); }🤖 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_project/src/lib.rs` around lines 206 - 226, Add tests for deserialize_bytecode that serialize BytecodeEnvelope and assert the magic occupies bytes 0..8 and envelope_version is encoded at bytes 8..12, pinning the prefix layout. Add coverage for an unsupported envelope version and a truncated prefix, asserting both produce the expected compatibility errors; also remove or avoid relying on the unreachable envelope.magic validation after the existing prefix check.
🤖 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/sdk_tests/harness_setup/src/csharp.rs`:
- Around line 119-126: The fixture bytecode paths currently serialize synthetic
manifests instead of preserving each fixture’s project manifest. In
baml_language/sdk_tests/harness_setup/src/csharp.rs lines 119-126, read the
fixture baml.toml, inject the reserved compatibility metadata into an in-memory
copy, and serialize it; apply the same manifest-copy change in
baml_language/sdk_tests/harness_setup/src/lib.rs lines 229-236, preserving all
existing project settings in both paths.
---
Nitpick comments:
In `@baml_language/crates/bex_project/src/lib.rs`:
- Around line 117-121: Keep EmbeddedBamlToml’s serde rename synchronized with
BYTECODE_METADATA_TABLE by adding the suggested unit test in the existing test
area. Construct TOML using BYTECODE_METADATA_TABLE, deserialize it into
EmbeddedBamlToml, and assert parsed.codegen is present so future constant
changes expose a mismatch.
- Around line 206-226: Add tests for deserialize_bytecode that serialize
BytecodeEnvelope and assert the magic occupies bytes 0..8 and envelope_version
is encoded at bytes 8..12, pinning the prefix layout. Add coverage for an
unsupported envelope version and a truncated prefix, asserting both produce the
expected compatibility errors; also remove or avoid relying on the unreachable
envelope.magic validation after the existing prefix check.
🪄 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: 10425171-d315-438a-828b-764bd712025a
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
baml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_lsp_server/src/playground_server.rsbaml_language/crates/bex_project/Cargo.tomlbaml_language/crates/bex_project/src/lib.rsbaml_language/crates/bridge_cffi/src/baml_to_host.rsbaml_language/crates/bridge_cffi/src/ffi/runtime.rsbaml_language/crates/bridge_cffi/src/lib.rsbaml_language/sdk_tests/harness_setup/Cargo.tomlbaml_language/sdk_tests/harness_setup/src/csharp.rsbaml_language/sdk_tests/harness_setup/src/lib.rsbaml_language/sdks/csharp/sdkgen_csharp/Cargo.tomlbaml_language/sdks/csharp/sdkgen_csharp/src/lib.rsbaml_language/sdks/csharp/sdkgen_csharp/src/model.rsbaml_language/sdks/typescript/bridge_typescript_web/src/errors.rs
💤 Files with no reviewable changes (1)
- baml_language/sdks/csharp/sdkgen_csharp/Cargo.toml
sxlijin
force-pushed
the
agent/bytecode-version-skew
branch
from
July 31, 2026 20:40
ab33c73 to
a3cf7ea
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/sdk_tests/harness_setup/src/lib.rs (1)
150-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused unit coverage for manifest augmentation.
If coverage does not already exist, test missing
baml.toml, malformed TOML, the reserved metadata table, and the exact injected version values. Keep these tests in the Rust module where possible.As per coding guidelines, prefer 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/sdk_tests/harness_setup/src/lib.rs` around lines 150 - 180, Add focused Rust unit tests for embedded_fixture_baml_toml covering a missing baml.toml, malformed TOML, an existing reserved BYTECODE_METADATA_TABLE, and successful augmentation with exact CANONICAL_VERSION and BYTECODE_FORMAT_VERSION values. Keep the tests in the same Rust module and assert the relevant fallback output or panic behavior without changing the production function.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.
Nitpick comments:
In `@baml_language/sdk_tests/harness_setup/src/lib.rs`:
- Around line 150-180: Add focused Rust unit tests for
embedded_fixture_baml_toml covering a missing baml.toml, malformed TOML, an
existing reserved BYTECODE_METADATA_TABLE, and successful augmentation with
exact CANONICAL_VERSION and BYTECODE_FORMAT_VERSION values. Keep the tests in
the same Rust module and assert the relevant fallback output or panic behavior
without changing the production function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 82512876-249b-45f6-845c-2e550ee43450
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
baml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_lsp_server/src/playground_server.rsbaml_language/crates/bex_project/Cargo.tomlbaml_language/crates/bex_project/src/lib.rsbaml_language/crates/bridge_cffi/src/baml_to_host.rsbaml_language/crates/bridge_cffi/src/ffi/runtime.rsbaml_language/crates/bridge_cffi/src/lib.rsbaml_language/sdk_tests/harness_setup/Cargo.tomlbaml_language/sdk_tests/harness_setup/src/csharp.rsbaml_language/sdk_tests/harness_setup/src/lib.rsbaml_language/sdks/csharp/sdkgen_csharp/Cargo.tomlbaml_language/sdks/csharp/sdkgen_csharp/src/lib.rsbaml_language/sdks/csharp/sdkgen_csharp/src/model.rsbaml_language/sdks/typescript/bridge_typescript_web/src/errors.rs
💤 Files with no reviewable changes (1)
- baml_language/sdks/csharp/sdkgen_csharp/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (12)
- baml_language/crates/baml_cli/Cargo.toml
- baml_language/crates/bridge_cffi/src/baml_to_host.rs
- baml_language/sdk_tests/harness_setup/src/csharp.rs
- baml_language/sdks/csharp/sdkgen_csharp/src/model.rs
- baml_language/crates/bridge_cffi/src/lib.rs
- baml_language/sdk_tests/harness_setup/Cargo.toml
- baml_language/crates/bex_project/Cargo.toml
- baml_language/sdks/csharp/sdkgen_csharp/src/lib.rs
- baml_language/crates/bridge_cffi/src/ffi/runtime.rs
- baml_language/crates/baml_cli/src/generate.rs
- baml_language/sdks/typescript/bridge_typescript_web/src/errors.rs
- baml_language/crates/bex_project/src/lib.rs
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
baml.tomlwith the actual producer release and bytecode-format versionProgramRoot cause
baml generatepreviously embedded a raw BorshProgram, while independently installed bridge runtimes deserialized it using their own schema without knowing which CLI release or format produced it. Version skew therefore surfaced as a low-level decode error or could be accepted without semantic compatibility evidence.Compatibility policy
Generated bytecode is accepted only when the producer BAML release and bytecode-format version exactly match the consuming bridge. Moving channels are recorded as the concrete CLI release that performed generation. Legacy or unversioned payloads are rejected with regeneration guidance.
Validation
cargo test -p bex_project bytecode_compatibility_tests --no-default-featurescargo test -p baml_cli generate::tests --no-default-featurescargo test -p baml_cli generate_valid_project_returns_zero_exit_code --no-default-featurescargo test -p bridge_cffi bytecode_initializer --no-default-featurescargo check -p bridge_cffi -p sdk_test_harness_setup --no-default-featurescargo check -p bridge_python -p bridge_typescript --no-default-featurescargo check -p bridge_typescript_web --target wasm32-unknown-unknown --no-default-featurescargo clippy -p bex_project --no-default-features -- -D warningscargo clippy -p baml_cli -- -D warningscargo fmt --all --checkgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation