Repository navigation
refactor(bridge-cffi): add target-neutral Wasm support - #4050
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe CFFI bridge now separates native and WASM runtime implementations, supports explicit system operations during bytecode initialization, adds native and WASM function-call paths, and exposes buffer deallocation from ChangesCross-platform bridge runtime
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant BridgeCffi
participant PlatformRuntime
participant Bex
Caller->>BridgeCffi: initialize runtime and submit encoded call
BridgeCffi->>PlatformRuntime: store or retrieve platform runtime
BridgeCffi->>Bex: decode context and invoke function
Bex-->>BridgeCffi: encoded result or error
BridgeCffi-->>Caller: callback or returned payload
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 |
8173b78 to
4313734
Compare
7ae977e to
403a93a
Compare
403a93a to
361f647
Compare
![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: 361f647f21
ℹ️ 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".
| Ok(call) => call, | ||
| Err(error) => return error_to_outbound(error), | ||
| }; | ||
| call_and_encode(call.runtime, call.function_name, call.args, call.context).await |
There was a problem hiding this comment.
When this Wasm entry point is used by a Web/JS caller and a BAML function returns media or a prompt AST, reusing call_and_encode encodes with CffiHandleTableOptions::for_in_process(), which stores those values as BamlHandles instead of inline media_value/prompt_ast_value. The new Wasm platform does not export the native media handle accessors from ffi::handle, so callers receiving these bytes cannot read the returned value and the handle table entry remains opaque; the Wasm path should encode with wire options or expose equivalent handle accessors.
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: 1
🧹 Nitpick comments (1)
baml_language/crates/bridge_cffi/src/buffer.rs (1)
40-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
drop(...)for the deallocation call.
let _ = Vec::from_raw_parts(...)works, butdrop(...)makes the one-shot deallocation intent clearer.♻️ Proposed refactor
- let _ = Vec::from_raw_parts(buf.ptr as *mut u8, buf.len, buf.len); + drop(Vec::from_raw_parts(buf.ptr as *mut u8, buf.len, buf.len));🤖 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/buffer.rs` around lines 40 - 41, Replace the discard assignment around Vec::from_raw_parts with an explicit drop(...) call, preserving the existing pointer, length, and capacity arguments and one-shot deallocation behavior.Source: Path instructions
🤖 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/lib_wasm.rs`:
- Around line 61-63: Update call_function_in_wasm_sync to avoid
futures::executor::block_on for promise-backed WASM operations; either remove
this synchronous entrypoint in favor of the async call_function_in_wasm API or
make it explicitly reject work that requires awaiting JavaScript Promises.
---
Nitpick comments:
In `@baml_language/crates/bridge_cffi/src/buffer.rs`:
- Around line 40-41: Replace the discard assignment around Vec::from_raw_parts
with an explicit drop(...) call, preserving the existing pointer, length, and
capacity arguments and one-shot deallocation behavior.
🪄 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: dc11817e-43a8-4908-8361-33e7ad7e6049
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
baml_language/crates/bridge_cffi/Cargo.tomlbaml_language/crates/bridge_cffi/src/api.rsbaml_language/crates/bridge_cffi/src/buffer.rsbaml_language/crates/bridge_cffi/src/ffi/objects.rsbaml_language/crates/bridge_cffi/src/ffi/runtime.rsbaml_language/crates/bridge_cffi/src/lib.rsbaml_language/crates/bridge_cffi/src/lib_native.rsbaml_language/crates/bridge_cffi/src/lib_wasm.rs
💤 Files with no reviewable changes (1)
- baml_language/crates/bridge_cffi/src/ffi/objects.rs
Summary
bridge_cffiinto native and Wasm platform layers while keeping common runtime initialization target-neutral.initialize_runtime_from_bytecode_with_sys_opsso callers can inject platform-specific system operations without introducing asys_wasmdependency into the shared bridge.canary.Why
This creates the shared bridge layer required by TypeScript/Web without mixing browser, Workers, or
sys_wasmbehavior intobridge_cffi.Impact
This is stack 1 of 3 and is based on
canary; the TypeScript foundation it previously depended on has merged. Native consumers continue using the existing API; Wasm consumers receive the target-neutral entry point used by the next PR.Checks
mise run stowmise run fmtmise run clippycargo check -p bridge_cfficargo check -p bridge_cffi --target wasm32-unknown-unknown --no-default-featuresSummary by CodeRabbit