Repository navigation
Rust bridge: class methods, unbounded generics, and host callbacks - #4093
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ 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):
|
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR extends Rust SDK code generation and runtime support for generic types, generated methods, callback effect inference, host-callable dispatch, typed and opaque errors, and explicit wire type arguments. It also updates symbol traits, generated documentation, test fixtures, and Rust test gating. ChangesRust SDK code generation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GeneratedBinding
participant RustRuntime
participant BAML_Engine
participant HostCallableBridge
GeneratedBinding->>RustRuntime: invoke with kwargs and type_args
RustRuntime->>BAML_Engine: dispatch CallFunctionArgs
BAML_Engine->>HostCallableBridge: dispatch registered callable
HostCallableBridge->>HostCallableBridge: decode args and run closure
HostCallableBridge->>BAML_Engine: complete host call with result or throw
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 |
Binary size checks passed✅ 7 passed
Generated by |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
baml_language/sdks/rust/bridge_rust/src/host_value.rs (1)
117-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding
#[cfg(test)]unit tests for these pure helpers.
next_key/slot_argsare pure, easily unit-testable (positional/named slotting, missing-required-arg error, wraparound-skip-zero). Per coding guidelines, prefer unit tests over integration tests where possible; today coverage for this logic lives only in the integration tests underbridge_rust/tests/conversions.rs.As per coding guidelines: "Prefer writing Rust unit tests over integration tests where possible."
Also applies to: 556-588
🤖 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/host_value.rs` around lines 117 - 124, Add a #[cfg(test)] unit-test module in host_value.rs covering the pure helpers next_key and slot_args: verify positional and named slotting, missing-required-argument errors, and next_key’s wraparound behavior skipping zero. Keep the tests focused on these helpers and use existing test conventions and symbols.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 `@baml_language/sdks/rust/bridge_rust/src/host_value.rs`:
- Around line 247-268: Update the closure invocation in the outer DISPATCH_RT
task to run through handle.spawn_blocking, await its JoinHandle, and preserve
the existing success, argument-decode failure, and panic/error completion paths.
Ensure the sync closure body executes off the async dispatch workers and that
any spawn_blocking JoinError calls complete_bridge_failure so every in-flight
call is completed.
---
Nitpick comments:
In `@baml_language/sdks/rust/bridge_rust/src/host_value.rs`:
- Around line 117-124: Add a #[cfg(test)] unit-test module in host_value.rs
covering the pure helpers next_key and slot_args: verify positional and named
slotting, missing-required-argument errors, and next_key’s wraparound behavior
skipping zero. Keep the tests focused on these helpers and use existing test
conventions and symbols.
🪄 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: b73af529-f218-4ae1-8f14-fff813e04478
📒 Files selected for processing (31)
baml_language/crates/baml_codegen_types/src/symbols.rsbaml_language/crates/baml_project/src/client_codegen.rsbaml_language/sdk_tests/crates/rust/function_calls/customizable/test_callback_throws.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_raises.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_primitives.rsbaml_language/sdk_tests/crates/rust/type_shapes/customizable/test_generic.rsbaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_host_callable_tests/main.bamlbaml_language/sdk_tests/harness_setup/src/rust.rsbaml_language/sdks/rust/bridge_rust/src/baml_value.rsbaml_language/sdks/rust/bridge_rust/src/capi.rsbaml_language/sdks/rust/bridge_rust/src/encode.rsbaml_language/sdks/rust/bridge_rust/src/error.rsbaml_language/sdks/rust/bridge_rust/src/host_value.rsbaml_language/sdks/rust/bridge_rust/src/lib.rsbaml_language/sdks/rust/bridge_rust/src/runtime.rsbaml_language/sdks/rust/bridge_rust/tests/conversions.rsbaml_language/sdks/rust/bridge_rust/tests/live_engine.rsbaml_language/sdks/rust/sdkgen_rust/src/analyze.rsbaml_language/sdks/rust/sdkgen_rust/src/effect_rename.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/union.rsbaml_language/sdks/rust/sdkgen_rust/src/lib.rsbaml_language/sdks/rust/sdkgen_rust/src/translate_ty.rsbaml_language/sdks/rust/sdkgen_rust/src/unions.rs
03212bd to
af0db4e
Compare
Also enable/update the tests
The rust signature should reflect the implicit generic parameter for a callable-typed function parameter that omits the `throws` clause. This had been excluded due to the limitations of python and typescript, but rust requires the information be provided. Also adds some tests.
af0db4e to
36786b4
Compare
selfbut have no copy-out so mutations are not reflected in the host value.throwsclause. While type-erased in Python/TS due to their type safety limitations, Rust can generate and infer them in generic positions.Summary by CodeRabbit
# Errorssection for throwing APIs.