Host callable errors#3680
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors error handling across engine, VM, sys-op codegen, bridges, SDKs, and tests to use VmBamlError/VmRustFnError; adds host-value error handles, HostContractViolation panic, host-call throws typing, and preserves host-thrown Instance delivery across bridges. ChangesUnified VM/sys-op errors and host-throw plumbing
Sequence Diagram(s)Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
⏭️ 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):
|
2kai2kai2
force-pushed
the
kai/host-callable-errors
branch
from
c81b30f to
dd84a44
Compare
Binary size checks passed✅ 7 passed
Generated by |
This comment has been minimized.
This comment has been minimized.
![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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
baml_language/crates/bridge_wasm/src/wasm_http.rs (1)
69-91:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep regular HTTP transport failures on the synthetic
status_code = 0path.Lines 69-91 now turn fetch callback failures/rejected promises into thrown
VmBamlError::Io.baml_language/crates/sys_native/src/ops/http.rsstill returns a syntheticHttpResponsewithstatus_code: 0for the same class of send failures, so BAML code can branch onok()/fallback instead of catching. This makesbaml.http.fetch/sendbehave differently by runtime and will break browser-only fallback flows.Suggested direction
- let result = JsFuture::from(promise).await.map_err(|e| { - let msg = e - .as_string() - .or_else(|| { - e.dyn_ref::<js_sys::Error>() - .map(|err| String::from(err.message())) - }) - .unwrap_or_else(|| format!("{e:?}")); - VmBamlError::Io { - message: format!("HTTP request failed: {msg}"), - } - })?; + let result = match JsFuture::from(promise).await { + Ok(result) => result, + Err(e) => { + let msg = e + .as_string() + .or_else(|| { + e.dyn_ref::<js_sys::Error>() + .map(|err| String::from(err.message())) + }) + .unwrap_or_else(|| format!("{e:?}")); + let mut headers = indexmap::IndexMap::new(); + headers.insert("x-baml-error".to_string(), msg.clone()); + let key = registry.store_body_promise(js_sys::Promise::resolve( + &wasm_bindgen::JsValue::from_str(&msg), + )); + let body: Arc<dyn std::any::Any + Send + Sync> = + Arc::new(WasmResponseBody { registry, key }); + return Ok(io::owned::http::Response { + status_code: 0, + headers, + url: request.url.clone(), + _body: body, + }); + } + };🤖 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_wasm/src/wasm_http.rs` around lines 69 - 91, The current code converts fetch invocation failures and rejected Promise results into VmBamlError::Io; instead, preserve the existing runtime semantics by returning the synthetic HttpResponse with status_code = 0 on those failure paths (so browser transport failures mirror sys_native behavior). Concretely, change the error branches around the fetch invocation (the closure that maps errors when building `promise: Promise`) and the await of `JsFuture::from(promise).await` (the closure that maps the rejected promise into `VmBamlError::Io`) to produce and return an HttpResponse-like value with status_code: 0 (and appropriate empty body/headers) instead of Err(VmBamlError::Io); keep references to VmBamlError::Io only for real I/O panics, and ensure the caller of the function receives the synthetic response so BAML code can branch on ok()/fallback as before.baml_language/crates/bridge_cffi/src/ffi/host_value.rs (2)
93-104:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
BridgeFailurefor the null-pointer-with-length ABI violation.This branch is the same class of bridge wire-protocol bug as the later
length > isize::MAXand invalidis_errorchecks, but it currently emits a catchableVmBamlError::Io. Classifying it asVmInternalError::BridgeFailurekeeps bridge bugs consistently non-user-contract errors.Suggested fix
- VmBamlError::Io { + VmInternalError::BridgeFailure { message: format!( "complete_host_call: null content pointer with length {length}" ), },🤖 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/host_value.rs` around lines 93 - 104, Replace the current use of VmBamlError::Io for the null-pointer-with-length ABI violation with the bridge-failure internal error variant: construct a VmBamlError::VmInternalError containing VmInternalError::BridgeFailure with the same descriptive message (instead of OpError::new(..., VmBamlError::Io{...}) use OpError::new(..., VmBamlError::VmInternalError(VmInternalError::BridgeFailure { message: format!(...) }))). Update the branch around host_dispatch::complete_with_error and SysOp::BamlHostCallHostValue to emit that BridgeFailure variant so this ABI violation is classified as an internal bridge failure.
64-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate
complete_host_calldocs to match the strict 0/1is_errorcontract.The doc still says “non-zero for error,” but runtime now rejects values other than
0or1. This can mislead bridge implementers and trigger avoidableBridgeFailures.Suggested doc fix
-/// - `is_error` — 0 for success, non-zero for error. +/// - `is_error` — 0 for success, 1 for error. ... -/// **Error** (`is_error != 0`): `content` is a protobuf-encoded +/// **Error** (`is_error == 1`): `content` is a protobuf-encoded🤖 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/host_value.rs` around lines 64 - 75, The doc comment for complete_host_call in host_value.rs incorrectly states "non-zero for error" while the runtime requires a strict 0/1 value; update the comment for the parameters (especially `is_error`) and the Success/Error sections to state that `is_error` must be 0 for success and 1 for error, and clarify that other values are rejected, referencing the function name complete_host_call and the host_value.rs doc block so bridge implementers know to send exactly 0 or 1.baml_language/crates/sys_ops/src/lib.rs (1)
643-649:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate unexpected
env.getfailures here.Line 644 currently drops every
Err(_)fromio.env_get(...). That suppresses host-thrown / contract-violation errors from customenvimplementations and turns them into a later missing-API-key failure instead of surfacing the original structured sys-op error. If shorthand should stay best-effort on platforms without env support, only swallowVmBamlError::Unsupported(andOk(None)); return everything else.Suggested fix
let io = ctx.runtime_io.clone(); SysOpOutput::async_op(async move { - if let Ok(Some(val)) = io.env_get(env_var.to_string()).await { - client.options.api_key = Some(val); - } - // Body never errors; annotate the error type so the - // `Result<_, VmRustFnError>` return contract is locked in. - Ok::<_, bex_vm_types::errors::VmRustFnError>(client) + match io.env_get(env_var.to_string()).await { + Ok(Some(val)) => client.options.api_key = Some(val), + Ok(None) => {} + Err(VmRustFnError::BamlError(VmBamlError::Unsupported { .. })) => {} + Err(err) => return Err(err), + } + Ok::<_, VmRustFnError>(client) })🤖 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/sys_ops/src/lib.rs` around lines 643 - 649, The current async sys-op silently ignores all Err from io.env_get(env_var) which hides host/contract errors; change the closure in SysOpOutput::async_op to match on io.env_get(...).await and only treat Ok(None) and Err(VmBamlError::Unsupported) as no-ops, set client.options.api_key on Ok(Some(val)), and propagate any other Err by returning an Err converted into the expected bex_vm_types::errors::VmRustFnError (e.g. via .into() or explicit mapping) instead of swallowing it.baml_language/crates/bex_vm_types/src/types.rs (1)
184-204:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve Borsh enum discriminants when adding
ParseError
SysOpErrorCategoryderivesBorshSerialize/BorshDeserialize, andParseErroris currently declared betweenInvalidArgumentandUnsupported; with Borsh’s enum encoding-by-variant-order, this shifts the tag values for all subsequent variants and can break decoding of previously serialized/persisted data. Consider appendingParseErrorto the end (or use explicit discriminants via#[borsh(use_discriminant=true)]) to keep discriminants stable.Suggested fix
pub enum SysOpErrorCategory { Io, Timeout, InvalidArgument, - /// A parser surfaced a structurally-bad input (e.g. UTF-8 decode, JSON - /// parse, base64 decode). Distinct from `InvalidArgument` so callers - /// can distinguish "your argument shape was wrong" from "the bytes/ - /// stream we tried to parse are malformed". - ParseError, Unsupported, NotImplemented, AccessError, RenderPrompt, LlmClient, DevOther, HostCallable, + /// A parser surfaced a structurally-bad input (e.g. UTF-8 decode, JSON + /// parse, base64 decode). Distinct from `InvalidArgument` so callers + /// can distinguish "your argument shape was wrong" from "the bytes/ + /// stream we tried to parse are malformed". + ParseError, }🤖 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_vm_types/src/types.rs` around lines 184 - 204, The new ParseError variant was inserted into the SysOpErrorCategory enum (which derives BorshSerialize/BorshDeserialize), shifting Borsh ordinal discriminants and breaking decoding; fix by preserving discriminants: either move ParseError to the end of the SysOpErrorCategory variants list or explicitly assign stable discriminants (e.g., via #[borsh(use_discriminant = "true")] or explicit numeric values) so that existing serialized data continues to decode correctly; update the enum declaration around SysOpErrorCategory to apply one of these approaches.baml_language/sdks/python/rust/bridge_python/src/py_handle.rs (1)
81-100:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
BamlPyHandlenow wraps keys it cannot safely own.
take_pyhandle_from_table()now returns aBamlPyHandleforHOST_VALUE_*keys even though those keys are not inHANDLE_TABLE. The rest of the type still assumes every key is a table row (Drop,__copy__, andput_pyhandle_into_tableall hitHANDLE_TABLE), so a decoded host-value handle can release or clone the wrong row when numeric keys collide, or fail if it is ever re-encoded. This needs a host-value-specific ownership path or a separate wrapper type.🤖 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/rust/bridge_python/src/py_handle.rs` around lines 81 - 100, take_pyhandle_from_table() currently constructs a BamlPyHandle for HOST_VALUE_* keys even though those keys are not rows in HANDLE_TABLE, causing Drop/__copy__/put_pyhandle_into_table to operate on HANDLE_TABLE incorrectly; fix by giving host-value handles a distinct ownership path: either add a flag/enum variant to BamlPyHandle (e.g., HostValue variant) or create a separate HostBamlPyHandle creator (e.g., BamlPyHandle::from_host_value) in take_pyhandle_from_table and set it for BamlHandleType::HostValueCallable/Error, then update BamlPyHandle::drop, __copy__, and put_pyhandle_into_table to branch on that variant and call the host-value registry APIs (see crate::host_value::lookup_host_value and any host-value insert/release helpers) instead of touching HANDLE_TABLE so host-value keys are never inserted, cloned, or removed from HANDLE_TABLE.baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_host_callables.py (1)
319-335:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis still codifies the old uncaught host-call behavior.
The PR objective says these sysop/host-call throws should now unwind as catchable BAML errors, but this test still expects
call_with_throwing()to bubblebaml.errors.HostCallableback to Python. If this is the catch fixture described in the docstring, the assertion needs to flip to the catch-path result instead of preserving the old routing bug.🤖 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/python_pydantic2/function_calls/customizable/test_host_callables.py` around lines 319 - 335, The test currently asserts the old behavior of bubbling a baml.errors.HostCallable; instead update it to verify the new catch-path: call call_with_throwing(callback=cb, x=1) without using pytest.raises, capture its return value, and assert that it matches the BAML catch-expression's expected result (replace the two assertions that look for "baml.errors.HostCallable" / "RuntimeError" with a single assertion that the returned value equals the catch-path result defined by the fixture). Ensure you update references inside test_call_with_throwing_surfaces_declared_host_callable_error and keep the callback cb unchanged.baml_language/crates/bex_engine/tests/host_value_callable.rs (1)
1271-1290:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stale failure message text in the fallback branch.
Line 1289 still says
UnhandledThrow(HostCallable), but this test now expectsHostContractViolation; the mismatch makes failures harder to diagnose.Suggested patch
- other => panic!("expected UnhandledThrow(HostCallable), got {other:?}"), + other => panic!("expected UnhandledThrow(HostContractViolation), got {other:?}"),🤖 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/tests/host_value_callable.rs` around lines 1271 - 1290, Update the test's final panic message to reflect the expected error type change: in the match on result where other => panic!("expected UnhandledThrow(HostCallable), got {other:?}"), replace that message to mention HostContractViolation (e.g., "expected UnhandledThrow(HostContractViolation), got {other:?}") so the failure text matches the asserted class_name; this change should be made in the match handling of EngineError::UnhandledThrow in the test in host_value_callable.rs.
🧹 Nitpick comments (3)
baml_language/crates/sys_native/src/host_dispatch.rs (1)
253-272: ⚡ Quick winAdd a unit test for
complete_with_throw.This is the only path that produces
OpErrorPayload::HostThrown, but the current tests still only exercise value/error completion. A small unit test here would pin that the thrown payload survives the call-table round trip.As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible".
Example test shape
+ #[tokio::test] + async fn complete_with_throw_preserves_host_thrown_payload() { + let (result, completion) = SysOpResult::pending(SysOp::BamlHostCallHostValue); + let call_id = next_call_id(); + assert!(insert(call_id, completion)); + + complete_with_throw(call_id, BexExternalValue::Int(7)); + + let err = match result { + SysOpResult::Async(fut) => fut.await.expect_err("expected thrown payload"), + SysOpResult::Ready(Ok(_)) => panic!("expected error"), + SysOpResult::Ready(Err(e)) => e, + }; + + assert!(matches!( + err.payload, + sys_types::OpErrorPayload::HostThrown(v) + if matches!(v.as_ref(), BexExternalValue::Int(7)) + )); + }🤖 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/sys_native/src/host_dispatch.rs` around lines 253 - 272, Add a unit test that exercises complete_with_throw: create a call entry (use the same call-table helper used by other tests or the take/complete path), invoke complete_with_throw(call_id, value) with a BexExternalValue, then retrieve the completed result from the call record and assert the OpError payload is HostThrown (constructed via OpError::host_thrown_value with SysOp::BamlHostCallHostValue) and that the carried BexExternalValue matches; locate and reuse the test helpers around the call-table/take/complete logic so the test mirrors existing value/error completion tests and verifies the thrown-path round-trip.baml_language/crates/sys_native/src/host_impls.rs (1)
11-18: ⚡ Quick winUpdate the host-throw docs to match the new three-way behavior.
The comment still says every on-contract host throw becomes
baml.errors.HostCallable, but this PR also allows throws that are directly convertible into the declared BAML type/primitive. The docs should reflect that split so future changes do not reintroduce the old behavior by accident.🤖 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/sys_native/src/host_impls.rs` around lines 11 - 18, Update the comment for complete_host_call to describe the new three-way behavior: a host completion now either (1) returns a successful value validated against the declared return type `T` (`type_arg_0`), (2) if the host threw a value that is directly convertible into the declared BAML return type/primitive treat it as a successful return (conversion path), or (3) if the host threw a value matching the declared throws contract `E` (`type_arg_1`) propagate it as a catchable baml.errors.HostCallable; any other host throw should surface as baml.panics.HostContractViolation; mention these symbols (complete_host_call, type_arg_0, type_arg_1, baml.errors.HostCallable, baml.panics.HostContractViolation) in the updated docs.baml_language/crates/bex_vm/src/vm.rs (1)
3565-3613: ⚡ Quick winAdd unit coverage for the external-unwind entry point.
try_handle_external_exceptionnow controls handler matching outside the normal dispatch loop. Please add local VM unit tests for at least: skipping top native frames, and surfacing unhandled throws on native-only/empty stacks. Also runcargo test --liblocally before merge.As per coding guidelines,
**/*.rs: Prefer writing Rust unit tests over integration tests where possibleandAlways run cargo test --lib if you changed any Rust code`.🤖 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_vm/src/vm.rs` around lines 3565 - 3613, Add unit tests for try_handle_external_exception in bex_vm: write at least three Rust #[test]s (e.g. test_try_handle_external_exception_skips_top_native_frames, test_try_handle_external_exception_native_only_unhandled, test_try_handle_external_exception_empty_stack_unhandled) that construct a local VM, push frames to simulate a mix of Native and Bytecode frames and verify that try_handle_external_exception skips top Native frames and lands in a bytecode handler (assert Ok(())), and that native-only or empty frame stacks return Err(VmError::ThrownUnhandled) with a captured trace (use capture_stack_trace()/load_function()/try_unwind_exception helpers as needed to set up frames); run cargo test --lib locally before merging.
🤖 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/baml_builtins2/baml_std/baml/ns_errors/errors.baml`:
- Around line 58-64: The schema declares the opaque slot `_handle` as required
but the docs state foreign runtimes may see `null`; change the `_handle` field
to be nullable/optional in the schema (make `_handle` accept null / be optional)
so the fallback path is representable, and update the related decoder/generator
paths that handle HostValueKind::Error to treat a null `_handle` as the
foreign-runtime/released-key case; ensure generated bindings and any validation
now allow the absent/null `_handle` and preserve the existing metadata fallback
behavior.
In `@baml_language/crates/bex_engine/src/conversion.rs`:
- Around line 728-753: Add unit tests for the new helper peel_to_rust_type:
create a #[cfg(test)] mod that imports Ty, TyAttr, TypeName and add tests
covering direct Opaque("baml.rust.RustType"), Optional<RustType>, singleton
Union (RustType | Null), non-singleton Union (RustType | String), duplicate
RustType members (RustType | RustType) and a non-RustType (String) case; each
test should call peel_to_rust_type(&ty) and assert Some(()) or None as
appropriate, using helper fn rust_type() -> Ty to build the Opaque type and
constructing Ty::Optional and Ty::Union with TyAttr::default() for attributes;
run cargo test --lib to verify.
In `@baml_language/crates/bridge_wasm/src/handle.rs`:
- Around line 25-27: The Drop impl currently always calls
HANDLE_TABLE.release(self.key) which will evict unrelated entries for host-owned
handle types like BamlHandleType::HostValueError; update the Drop logic for the
handle struct (the Drop for BamlHandle / impl Drop) to skip calling
HANDLE_TABLE.release when self.ty is a host-owned variant (e.g., match on
self.ty and treat BamlHandleType::HostValueError and any other host-owned
variants as no-op), or add an is_host_owned() helper used by Drop to only
release keys for non-host-owned handle types so host-owned keys are never
removed from HANDLE_TABLE.
In `@baml_language/crates/bridge_wasm/src/wasm_http.rs`:
- Around line 97-105: The code currently reads the JS `status` via
Reflect::get(...).as_f64() and casts to i64, which lets NaN/Infinity and
non-integer values slip through; update the extraction in wasm_http.rs so after
calling as_f64() you validate that the f64 is finite (value.is_finite()) and an
integer (value.fract() == 0.0) before casting, and if not return a
VmBamlError::Io with a clear message like "Response 'status' must be a finite
integer"; keep the existing Reflect::get error branch and only cast to i64 after
these checks so downstream code that expects integer HTTP status codes cannot be
misled by truncated or infinite values.
In `@baml_language/crates/sys_native/src/host_impls.rs`:
- Around line 67-75: The current match accepts any BexExternalValue::HostValue
and proceeds to dispatch, which lets propagated HostCallable error handles be
invoked; update the early extraction to reject non-callable host handles by
inspecting the underlying HostValueArc (the HostValue enum inside HostValueArc,
e.g., variants like Callable vs Error) after extracting from
BexExternalValue::HostValue and, if it represents an error/non-callable, return
SysOpOutput::err with an appropriate VmBamlError (similar shape to the existing
InvalidArgument) instead of continuing to dispatch in the host call path (the
code around HostValueArc / BexExternalValue::HostValue in host_impls.rs).
In `@baml_language/sdks/nodejs/bridge_nodejs/src/host_value.rs`:
- Around line 277-287: The register_error_release_callback currently builds an
ErrorReleaseTsfn with .weak::<false>() and stores it in the process-wide static
ERROR_RELEASE_CALLBACK (OnceLock<Arc<ErrorReleaseTsfn>>), which pins the Node
event loop; change the builder to use .weak::<true>() so the TSFN is unrefed and
won't keep the event loop alive, and/or add a teardown API (e.g.
unregister_error_release_callback) that clears/drops ERROR_RELEASE_CALLBACK (and
any Arc clones) to explicitly destroy the ThreadsafeFunction; update
register_error_release_callback and the OnceLock usage (ERROR_RELEASE_CALLBACK)
accordingly to ensure the TSFN can be dropped on shutdown.
In `@baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.ts`:
- Around line 747-755: The catch block that rolls back registrations iterates
ctx.registered and calls releaseHostCallable(key) but a failure in any single
releaseHostCallable will abort the loop and leak other registered callables;
update the rollback to protect each release call (e.g., wrap each
releaseHostCallable(key) invocation in its own try/catch and continue on error,
optionally logging the error) so all entries in ctx.registered are attempted
even if some releases throw — locate the catch around ctx.registered in proto.ts
and modify the loop that calls releaseHostCallable to handle individual release
failures.
In `@baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.js`:
- Around line 162-163: The encodeDelimited helpers (e.g.,
BamlHandle.encodeDelimited) must avoid calling .ldelim() directly on a Writer
that already has buffered bytes; update each encodeDelimited to check if a
writer was provided and has existing content (writer.len or equivalent), and if
so create a fork (var fork = writer.fork()), call this.encode(message,
fork).ldelim() and return the original writer; otherwise call and return
this.encode(message, writer).ldelim() as before—apply this change to all
occurrences of the pattern in the file.
---
Outside diff comments:
In `@baml_language/crates/bex_engine/tests/host_value_callable.rs`:
- Around line 1271-1290: Update the test's final panic message to reflect the
expected error type change: in the match on result where other =>
panic!("expected UnhandledThrow(HostCallable), got {other:?}"), replace that
message to mention HostContractViolation (e.g., "expected
UnhandledThrow(HostContractViolation), got {other:?}") so the failure text
matches the asserted class_name; this change should be made in the match
handling of EngineError::UnhandledThrow in the test in host_value_callable.rs.
In `@baml_language/crates/bex_vm_types/src/types.rs`:
- Around line 184-204: The new ParseError variant was inserted into the
SysOpErrorCategory enum (which derives BorshSerialize/BorshDeserialize),
shifting Borsh ordinal discriminants and breaking decoding; fix by preserving
discriminants: either move ParseError to the end of the SysOpErrorCategory
variants list or explicitly assign stable discriminants (e.g., via
#[borsh(use_discriminant = "true")] or explicit numeric values) so that existing
serialized data continues to decode correctly; update the enum declaration
around SysOpErrorCategory to apply one of these approaches.
In `@baml_language/crates/bridge_cffi/src/ffi/host_value.rs`:
- Around line 93-104: Replace the current use of VmBamlError::Io for the
null-pointer-with-length ABI violation with the bridge-failure internal error
variant: construct a VmBamlError::VmInternalError containing
VmInternalError::BridgeFailure with the same descriptive message (instead of
OpError::new(..., VmBamlError::Io{...}) use OpError::new(...,
VmBamlError::VmInternalError(VmInternalError::BridgeFailure { message:
format!(...) }))). Update the branch around host_dispatch::complete_with_error
and SysOp::BamlHostCallHostValue to emit that BridgeFailure variant so this ABI
violation is classified as an internal bridge failure.
- Around line 64-75: The doc comment for complete_host_call in host_value.rs
incorrectly states "non-zero for error" while the runtime requires a strict 0/1
value; update the comment for the parameters (especially `is_error`) and the
Success/Error sections to state that `is_error` must be 0 for success and 1 for
error, and clarify that other values are rejected, referencing the function name
complete_host_call and the host_value.rs doc block so bridge implementers know
to send exactly 0 or 1.
In `@baml_language/crates/bridge_wasm/src/wasm_http.rs`:
- Around line 69-91: The current code converts fetch invocation failures and
rejected Promise results into VmBamlError::Io; instead, preserve the existing
runtime semantics by returning the synthetic HttpResponse with status_code = 0
on those failure paths (so browser transport failures mirror sys_native
behavior). Concretely, change the error branches around the fetch invocation
(the closure that maps errors when building `promise: Promise`) and the await of
`JsFuture::from(promise).await` (the closure that maps the rejected promise into
`VmBamlError::Io`) to produce and return an HttpResponse-like value with
status_code: 0 (and appropriate empty body/headers) instead of
Err(VmBamlError::Io); keep references to VmBamlError::Io only for real I/O
panics, and ensure the caller of the function receives the synthetic response so
BAML code can branch on ok()/fallback as before.
In `@baml_language/crates/sys_ops/src/lib.rs`:
- Around line 643-649: The current async sys-op silently ignores all Err from
io.env_get(env_var) which hides host/contract errors; change the closure in
SysOpOutput::async_op to match on io.env_get(...).await and only treat Ok(None)
and Err(VmBamlError::Unsupported) as no-ops, set client.options.api_key on
Ok(Some(val)), and propagate any other Err by returning an Err converted into
the expected bex_vm_types::errors::VmRustFnError (e.g. via .into() or explicit
mapping) instead of swallowing it.
In
`@baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_host_callables.py`:
- Around line 319-335: The test currently asserts the old behavior of bubbling a
baml.errors.HostCallable; instead update it to verify the new catch-path: call
call_with_throwing(callback=cb, x=1) without using pytest.raises, capture its
return value, and assert that it matches the BAML catch-expression's expected
result (replace the two assertions that look for "baml.errors.HostCallable" /
"RuntimeError" with a single assertion that the returned value equals the
catch-path result defined by the fixture). Ensure you update references inside
test_call_with_throwing_surfaces_declared_host_callable_error and keep the
callback cb unchanged.
In `@baml_language/sdks/python/rust/bridge_python/src/py_handle.rs`:
- Around line 81-100: take_pyhandle_from_table() currently constructs a
BamlPyHandle for HOST_VALUE_* keys even though those keys are not rows in
HANDLE_TABLE, causing Drop/__copy__/put_pyhandle_into_table to operate on
HANDLE_TABLE incorrectly; fix by giving host-value handles a distinct ownership
path: either add a flag/enum variant to BamlPyHandle (e.g., HostValue variant)
or create a separate HostBamlPyHandle creator (e.g.,
BamlPyHandle::from_host_value) in take_pyhandle_from_table and set it for
BamlHandleType::HostValueCallable/Error, then update BamlPyHandle::drop,
__copy__, and put_pyhandle_into_table to branch on that variant and call the
host-value registry APIs (see crate::host_value::lookup_host_value and any
host-value insert/release helpers) instead of touching HANDLE_TABLE so
host-value keys are never inserted, cloned, or removed from HANDLE_TABLE.
---
Nitpick comments:
In `@baml_language/crates/bex_vm/src/vm.rs`:
- Around line 3565-3613: Add unit tests for try_handle_external_exception in
bex_vm: write at least three Rust #[test]s (e.g.
test_try_handle_external_exception_skips_top_native_frames,
test_try_handle_external_exception_native_only_unhandled,
test_try_handle_external_exception_empty_stack_unhandled) that construct a local
VM, push frames to simulate a mix of Native and Bytecode frames and verify that
try_handle_external_exception skips top Native frames and lands in a bytecode
handler (assert Ok(())), and that native-only or empty frame stacks return
Err(VmError::ThrownUnhandled) with a captured trace (use
capture_stack_trace()/load_function()/try_unwind_exception helpers as needed to
set up frames); run cargo test --lib locally before merging.
In `@baml_language/crates/sys_native/src/host_dispatch.rs`:
- Around line 253-272: Add a unit test that exercises complete_with_throw:
create a call entry (use the same call-table helper used by other tests or the
take/complete path), invoke complete_with_throw(call_id, value) with a
BexExternalValue, then retrieve the completed result from the call record and
assert the OpError payload is HostThrown (constructed via
OpError::host_thrown_value with SysOp::BamlHostCallHostValue) and that the
carried BexExternalValue matches; locate and reuse the test helpers around the
call-table/take/complete logic so the test mirrors existing value/error
completion tests and verifies the thrown-path round-trip.
In `@baml_language/crates/sys_native/src/host_impls.rs`:
- Around line 11-18: Update the comment for complete_host_call to describe the
new three-way behavior: a host completion now either (1) returns a successful
value validated against the declared return type `T` (`type_arg_0`), (2) if the
host threw a value that is directly convertible into the declared BAML return
type/primitive treat it as a successful return (conversion path), or (3) if the
host threw a value matching the declared throws contract `E` (`type_arg_1`)
propagate it as a catchable baml.errors.HostCallable; any other host throw
should surface as baml.panics.HostContractViolation; mention these symbols
(complete_host_call, type_arg_0, type_arg_1, baml.errors.HostCallable,
baml.panics.HostContractViolation) in the updated docs.
🪄 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: 461d7889-5382-4bfd-84ba-6e7ff37d4da5
⛔ Files ignored due to path filters (21)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_namespace_env.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/exceptions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snapbaml_language/sdks/nodejs/bridge_nodejs/dist/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/index.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/index.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/native.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/native.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**
📒 Files selected for processing (63)
baml_language/crates/baml_builtins2/baml_std/baml/ns_env/env.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_fs/fs.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_host/host.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_panics/panics.bamlbaml_language/crates/baml_builtins2_codegen/src/codegen_io.rsbaml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_lsp_server/src/playground_http.rsbaml_language/crates/baml_lsp_server/src/playground_io.rsbaml_language/crates/baml_tests/tests/fs.rsbaml_language/crates/baml_tests/tests/http.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/host_value_callable.rsbaml_language/crates/bex_external_types/src/bex_external_value.rsbaml_language/crates/bex_resource_types/src/host_value.rsbaml_language/crates/bex_vm/src/errors.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm_types/Cargo.tomlbaml_language/crates/bex_vm_types/src/errors.rsbaml_language/crates/bex_vm_types/src/lib.rsbaml_language/crates/bex_vm_types/src/types.rsbaml_language/crates/bridge_cffi/src/ffi/host_value.rsbaml_language/crates/bridge_ctypes/src/value_decode.rsbaml_language/crates/bridge_ctypes/src/value_encode.rsbaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_inbound.protobaml_language/crates/bridge_wasm/src/handle.rsbaml_language/crates/bridge_wasm/src/host_value.rsbaml_language/crates/bridge_wasm/src/lib.rsbaml_language/crates/bridge_wasm/src/wasm_env.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/bridge_wasm/src/wasm_io.rsbaml_language/crates/bridge_wasm/src/wasm_io_fs.rsbaml_language/crates/bridge_wasm/src/wasm_io_glob.rsbaml_language/crates/bridge_wasm/src/wasm_sys.rsbaml_language/crates/bridge_wasm/tests/host_callable.rsbaml_language/crates/sys_llm/src/lib.rsbaml_language/crates/sys_llm/src/types/mod.rsbaml_language/crates/sys_native/src/host_dispatch.rsbaml_language/crates/sys_native/src/host_impls.rsbaml_language/crates/sys_native/src/io_impls.rsbaml_language/crates/sys_native/src/lib.rsbaml_language/crates/sys_native/src/ops/http.rsbaml_language/crates/sys_ops/src/lib.rsbaml_language/crates/sys_types/src/lib.rsbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_host_callables.pybaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_host_callable_tests/main.bamlbaml_language/sdks/nodejs/bridge_nodejs/src/host_value.rsbaml_language/sdks/nodejs/bridge_nodejs/tests/host_callable.test.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/host_error_registry.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/index.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/native.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/python/rust/bridge_python/src/host_value.rsbaml_language/sdks/python/rust/bridge_python/src/lib.rsbaml_language/sdks/python/rust/bridge_python/src/py_handle.rsbaml_language/sdks/python/src/baml_core/baml_py.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyibaml_language/sdks/python/src/baml_core/proto.pybaml_language/sdks/python/tests/test_host_callable.py
2kai2kai2
force-pushed
the
kai/host-callable-errors
branch
from
dd84a44 to
c69303b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.js (1)
169-170:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore writer forking in
encodeDelimited.This still regresses
protobufjsframing:this.encode(message, writer).ldelim()prefixes the current writer state, so appending a delimited message to a non-empty writer can corrupt the stream. Please restore the previous fork-aware pattern everywhere this was regenerated.Suggested fix
- return this.encode(message, writer).ldelim(); + return this.encode(message, writer && writer.len ? writer.fork() : writer).ldelim();Also applies to: 633-634, 1091-1092, 1339-1340, 1643-1644, 1977-1978, 2247-2248, 2489-2490, 2749-2750, 3016-3017, 3314-3315, 3608-3609, 4087-4088, 4685-4686, 5055-5056, 5325-5326, 5559-5560, 5778-5779, 6053-6054, 6322-6323, 6617-6618, 6903-6904, 7215-7216, 7604-7605, 7975-7976, 8282-8283, 8541-8542, 8823-8824, 9104-9105, 9551-9552, 10185-10186, 10380-10381, 10575-10576, 10770-10771, 10965-10966, 11160-11161, 11355-11356, 11550-11551, 11745-11746, 11951-11952, 12178-12179, 12421-12422, 12648-12649, 12922-12923, 13231-13232, 13497-13498, 13724-13725, 13956-13957, 14188-14189, 14431-14432, 14681-14682, 14913-14914
🤖 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/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.js` around lines 169 - 170, The generated encodeDelimited implementations (e.g., BamlHandle.encodeDelimited) were changed to call this.encode(message, writer).ldelim(), which breaks protobufjs framing when appending to a non-empty writer; revert them to the fork-aware pattern: create a forked writer before encoding (call writer = writer ? writer.fork() : $Writer.create()), call this.encode(message, writer).ldelim(), then if an original writer was provided call writer.ldelim() or use the standard pattern of encoding into forked writer and returning writer unless a parent writer exists; update every encodeDelimited function (search for encodeDelimited definitions such as BamlHandle.encodeDelimited and the other listed occurrences) to restore the original fork-and-ldelim pattern used by protobufjs so delimited messages append safely to non-empty writers.
🧹 Nitpick comments (3)
baml_language/crates/sys_ops/src/lib.rs (1)
1682-1755: ⚡ Quick winAdd a unit test for the new net glue wiring.
with_net_instance()now manually wires ninebaml_net_*slots. A copy/paste mix-up here can still compile if the signatures line up, so a small in-module test that overrides one TCP op and one UDP op would give this path much better regression coverage.As per 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/sys_ops/src/lib.rs` around lines 1682 - 1755, Add a small in-module unit test for with_net_instance that verifies the wiring by providing a test implementation of io::IoNamespaceNet which records calls (e.g., sets a flag or increments a counter) for one TCP op and one UDP op, then construct the builder/runtime using with_net_instance(instance) and invoke the corresponding wired closures (or public API paths that trigger them) — for example call the functions behind baml_net_tcpstream_write and baml_net_udpsocket_send_to — and assert the test impl's flags/counters were hit; place this under #[cfg(test)] mod tests in the same module so the test fails if a copy/paste wiring mistake leaves any of the two slots unhooked.baml_language/crates/bex_engine/src/lib.rs (1)
3156-3168: ⚡ Quick winKeep the new throw-routing helpers unit-covered locally.
This comment block explicitly moves coverage to
host_value_callable, but helpers likehost_call_type_arg,value_runtime_baml_ty, andextract_host_diagnostics_from_valueare small enough to pin with crate-local unit tests. When this path regresses, a focused unit failure will be much easier to diagnose than an end-to-end host-call integration failure. As per coding guidelines,**/*.rs: Prefer writing Rust unit tests over integration tests where possible, and always runcargo test --libif you changed any Rust code.🤖 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/lib.rs` around lines 3156 - 3168, Add focused crate-local unit tests in bex_engine::lib (run via cargo test --lib) to cover the small throw-routing helpers rather than relying only on the host_value_callable integration test: write tests for host_call_type_arg to assert correct type-arg extraction and failure paths, for value_runtime_baml_ty to assert mapping/None cases for various Value inputs, and for extract_host_diagnostics_from_value to assert diagnostic extraction and error cases; place tests adjacent to the corresponding functions (in the same file/module) and include both normal and edge-case inputs so regressions surface locally.baml_language/crates/bridge_wasm/tests/host_callable.rs (1)
356-389: ⚡ Quick winThis helper change isn't actually asserted by the test.
host_callable_error_surfaces_as_baml_errorstill passes on anyErr, so a malformedHostCallablepayload, missing_handle, or wrong class name would all go green as long as the call fails. Please assert at least the surfaced message/class so this fixture verifies the new error-shape contract instead of just the failure 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/crates/bridge_wasm/tests/host_callable.rs` around lines 356 - 389, The test helper make_dispatch_error currently builds a HostCallable payload but the test host_callable_error_surfaces_as_baml_error only asserts that the call returns Err, so malformed payloads would still pass; update the test host_callable_error_surfaces_as_baml_error to inspect the returned Err and assert the surfaced error's message and class_name match the fixture (e.g., "test boom" and "RuntimeError") — locate the call site that invokes make_dispatch_error and change its assertion to pattern-match the Err into the Baml error variant (or decode the returned Inbound/ClassValue) and assert the inbound class name and the "message" field explicitly to ensure the new error-shape contract is verified.
🤖 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/baml_builtins2/baml_std/baml/ns_host/host.baml`:
- Around line 4-10: Update the doc block to reflect that the declared error
contract E is now enforced: state that `throws_ty` is threaded into the engine
and that host throws not assignable to `E` are converted to
`baml.panics.HostContractViolation` at runtime (instead of "planned for a future
phase"); keep the explanation of type-arg channels (`type_arg_0` / `type_arg_1`)
and that an untyped host value erases `E` to `unknown`, and mention the specific
symbol `throws_ty` and `baml.panics.HostContractViolation` so readers of the
`host.baml` docs know the current behavior.
In `@baml_language/crates/bex_engine/src/lib.rs`:
- Around line 2552-2563: When operation == SysOp::BamlHostCallHostValue you must
fail-closed if the ABI omits or supplies a non-Type for ret_ty or throws_ty
instead of treating them as None; update the host_ret_ty and host_throws_ty
handling (the host_call_type_arg call and the variables
host_ret_ty/host_throws_ty) to return/propagate an internal/ABI error when
host_call_type_arg(...) yields None or an unexpected variant, so downstream
validation and enforcement always run; locate the surrounding function in lib.rs
where these variables are set and replace the permissive None branch with an
explicit error return (or Err) that describes the malformed host-call ABI.
In `@baml_language/crates/bridge_wasm/src/lib.rs`:
- Around line 147-149: The documentation incorrectly states that thrown payloads
are always `baml.errors.HostCallable`; update the comment around
`completeHostCall(..., isError != 0, content)` to say it accepts the thrown
`InboundValue` itself (the actual payload), and clarify that `HostCallable` is
only a fallback wrapper used for opaque native exceptions; reference the
typed-throw path in `bridge_wasm/src/host_value.rs` so implementers know to
prefer passing the typed `InboundValue` rather than wrapping everything as
`HostCallable`.
In `@baml_language/crates/sys_ops/src/lib.rs`:
- Around line 569-576: lookup_llm_function currently collapses NotFound and
Ambiguous to None, so change the call sites (the three places using let
Some(info) = lookup_llm_function(&function_name, &ctx.llm_functions) else { ...
} at the shown block and the ones at the other noted offsets) to return and
inspect the full ResolveOutcome instead of using pattern match to None; when
ResolveOutcome::Ambiguous is returned, produce a distinct VmBamlError::DevOther
message (via SysOpOutput::err) that includes the list of ambiguous
candidates/namespace info, and when ResolveOutcome::NotFound produce the
existing "LLM function not found" message—use ResolveOutcome,
lookup_llm_function, SysOpOutput::err, and VmBamlError::DevOther as the
referenced symbols when making the change.
In `@baml_language/sdks/nodejs/bridge_nodejs/src/host_value.rs`:
- Around line 169-218: The release path currently always enqueues
ERROR_RELEASE_CALLBACK even for callable keys, which can overflow the TS queue;
change drop_registry_entry to return whether the registry removal found a
callable (e.g., return true if the removed Option<Arc<DispatchTsfn>> is Some,
false otherwise), then update host_release_callback to call
drop_registry_entry(host_value_key) and early-return when it indicates a
callable so only non-callable (error) keys enqueue ERROR_RELEASE_CALLBACK;
adjust signatures/usages for drop_registry_entry and host_release_callback
accordingly and keep the existing poison-handling/logging behavior.
---
Duplicate comments:
In `@baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.js`:
- Around line 169-170: The generated encodeDelimited implementations (e.g.,
BamlHandle.encodeDelimited) were changed to call this.encode(message,
writer).ldelim(), which breaks protobufjs framing when appending to a non-empty
writer; revert them to the fork-aware pattern: create a forked writer before
encoding (call writer = writer ? writer.fork() : $Writer.create()), call
this.encode(message, writer).ldelim(), then if an original writer was provided
call writer.ldelim() or use the standard pattern of encoding into forked writer
and returning writer unless a parent writer exists; update every encodeDelimited
function (search for encodeDelimited definitions such as
BamlHandle.encodeDelimited and the other listed occurrences) to restore the
original fork-and-ldelim pattern used by protobufjs so delimited messages append
safely to non-empty writers.
---
Nitpick comments:
In `@baml_language/crates/bex_engine/src/lib.rs`:
- Around line 3156-3168: Add focused crate-local unit tests in bex_engine::lib
(run via cargo test --lib) to cover the small throw-routing helpers rather than
relying only on the host_value_callable integration test: write tests for
host_call_type_arg to assert correct type-arg extraction and failure paths, for
value_runtime_baml_ty to assert mapping/None cases for various Value inputs, and
for extract_host_diagnostics_from_value to assert diagnostic extraction and
error cases; place tests adjacent to the corresponding functions (in the same
file/module) and include both normal and edge-case inputs so regressions surface
locally.
In `@baml_language/crates/bridge_wasm/tests/host_callable.rs`:
- Around line 356-389: The test helper make_dispatch_error currently builds a
HostCallable payload but the test host_callable_error_surfaces_as_baml_error
only asserts that the call returns Err, so malformed payloads would still pass;
update the test host_callable_error_surfaces_as_baml_error to inspect the
returned Err and assert the surfaced error's message and class_name match the
fixture (e.g., "test boom" and "RuntimeError") — locate the call site that
invokes make_dispatch_error and change its assertion to pattern-match the Err
into the Baml error variant (or decode the returned Inbound/ClassValue) and
assert the inbound class name and the "message" field explicitly to ensure the
new error-shape contract is verified.
In `@baml_language/crates/sys_ops/src/lib.rs`:
- Around line 1682-1755: Add a small in-module unit test for with_net_instance
that verifies the wiring by providing a test implementation of
io::IoNamespaceNet which records calls (e.g., sets a flag or increments a
counter) for one TCP op and one UDP op, then construct the builder/runtime using
with_net_instance(instance) and invoke the corresponding wired closures (or
public API paths that trigger them) — for example call the functions behind
baml_net_tcpstream_write and baml_net_udpsocket_send_to — and assert the test
impl's flags/counters were hit; place this under #[cfg(test)] mod tests in the
same module so the test fails if a copy/paste wiring mistake leaves any of the
two slots unhooked.
🪄 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: 06844d01-f32d-477f-8770-bbd40169bca6
⛔ Files ignored due to path filters (23)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_namespace_env.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/exceptions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snapbaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_inbound.pb.gois excluded by!**/*.pb.gobaml_language/sdks/nodejs/bridge_nodejs/dist/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/index.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/index.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/native.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/native.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**baml_language/sdks/python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
baml_language/Cargo.tomlbaml_language/crates/baml_builtins2/baml_std/baml/ns_env/env.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_fs/fs.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_host/host.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_panics/panics.bamlbaml_language/crates/baml_builtins2_codegen/src/codegen_io.rsbaml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_lsp_server/src/playground_http.rsbaml_language/crates/baml_lsp_server/src/playground_io.rsbaml_language/crates/baml_tests/tests/fs.rsbaml_language/crates/baml_tests/tests/http.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/host_value_callable.rsbaml_language/crates/bex_external_types/src/bex_external_value.rsbaml_language/crates/bex_resource_types/src/host_value.rsbaml_language/crates/bex_vm/src/errors.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm_types/Cargo.tomlbaml_language/crates/bex_vm_types/src/errors.rsbaml_language/crates/bex_vm_types/src/lib.rsbaml_language/crates/bex_vm_types/src/types.rsbaml_language/crates/bridge_cffi/src/ffi/host_value.rsbaml_language/crates/bridge_ctypes/src/value_decode.rsbaml_language/crates/bridge_ctypes/src/value_encode.rsbaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_inbound.protobaml_language/crates/bridge_wasm/Cargo.tomlbaml_language/crates/bridge_wasm/src/handle.rsbaml_language/crates/bridge_wasm/src/host_value.rsbaml_language/crates/bridge_wasm/src/lib.rsbaml_language/crates/bridge_wasm/src/wasm_env.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/bridge_wasm/src/wasm_io.rsbaml_language/crates/bridge_wasm/src/wasm_io_fs.rsbaml_language/crates/bridge_wasm/src/wasm_io_glob.rsbaml_language/crates/bridge_wasm/src/wasm_sys.rsbaml_language/crates/bridge_wasm/tests/host_callable.rsbaml_language/crates/sys_llm/src/lib.rsbaml_language/crates/sys_llm/src/types/mod.rsbaml_language/crates/sys_native/src/host_dispatch.rsbaml_language/crates/sys_native/src/host_impls.rsbaml_language/crates/sys_native/src/io_impls.rsbaml_language/crates/sys_native/src/lib.rsbaml_language/crates/sys_native/src/ops/http.rsbaml_language/crates/sys_ops/src/lib.rsbaml_language/crates/sys_types/src/lib.rsbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_host_callables.pybaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_host_callable_tests/main.bamlbaml_language/sdks/nodejs/bridge_nodejs/src/host_value.rsbaml_language/sdks/nodejs/bridge_nodejs/tests/host_callable.test.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/host_error_registry.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/index.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/native.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/python/rust/bridge_python/src/host_value.rsbaml_language/sdks/python/rust/bridge_python/src/lib.rsbaml_language/sdks/python/rust/bridge_python/src/py_handle.rsbaml_language/sdks/python/src/baml_core/baml_py.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyibaml_language/sdks/python/src/baml_core/proto.pybaml_language/sdks/python/tests/test_host_callable.py
💤 Files with no reviewable changes (8)
- baml_language/sdks/python/rust/bridge_python/src/lib.rs
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyi
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.py
- baml_language/sdks/python/tests/test_host_callable.py
- baml_language/sdks/python/src/baml_core/baml_py.pyi
- baml_language/sdks/python/src/baml_core/proto.py
- baml_language/sdks/python/rust/bridge_python/src/py_handle.rs
- baml_language/sdks/python/rust/bridge_python/src/host_value.rs
✅ Files skipped from review due to trivial changes (2)
- baml_language/crates/baml_builtins2_codegen/src/extract.rs
- baml_language/crates/sys_llm/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (41)
- baml_language/crates/sys_native/src/lib.rs
- baml_language/crates/baml_tests/tests/http.rs
- baml_language/crates/bex_vm_types/Cargo.toml
- baml_language/crates/bex_vm_types/src/lib.rs
- baml_language/sdk_tests/fixtures/function_calls/baml_src/ns_host_callable_tests/main.baml
- baml_language/crates/bridge_ctypes/src/value_encode.rs
- baml_language/crates/bex_vm_types/src/errors.rs
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/native.d.ts
- baml_language/crates/bex_vm/src/errors.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_env/env.baml
- baml_language/crates/baml_tests/tests/fs.rs
- baml_language/crates/bridge_wasm/src/wasm_sys.rs
- baml_language/crates/sys_native/src/ops/http.rs
- baml_language/crates/bex_external_types/src/bex_external_value.rs
- baml_language/crates/sys_llm/src/types/mod.rs
- baml_language/crates/bridge_wasm/src/wasm_io_glob.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.baml
- baml_language/crates/baml_lsp_server/src/playground_http.rs
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/host_error_registry.ts
- baml_language/sdks/nodejs/bridge_nodejs/tests/host_callable.test.ts
- baml_language/crates/sys_native/src/host_dispatch.rs
- baml_language/crates/baml_builtins2_codegen/src/codegen_io.rs
- baml_language/crates/bex_vm_types/src/types.rs
- baml_language/crates/baml_lsp_server/src/playground_io.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_fs/fs.baml
- baml_language/crates/bridge_wasm/src/wasm_io.rs
- baml_language/crates/bridge_cffi/src/ffi/host_value.rs
- baml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_inbound.proto
- baml_language/crates/bridge_ctypes/src/value_decode.rs
- baml_language/crates/bridge_wasm/src/host_value.rs
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/index.ts
- baml_language/crates/bridge_wasm/src/wasm_env.rs
- baml_language/crates/bex_resource_types/src/host_value.rs
- baml_language/crates/bex_vm/src/vm.rs
- baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_host_callables.py
- baml_language/crates/sys_native/src/host_impls.rs
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.ts
- baml_language/crates/bridge_wasm/src/wasm_io_fs.rs
- baml_language/crates/sys_types/src/lib.rs
- baml_language/crates/bex_engine/tests/host_value_callable.rs
- baml_language/crates/sys_native/src/io_impls.rs
2kai2kai2
force-pushed
the
kai/host-callable-errors
branch
from
c69303b to
0c88708
Compare
2kai2kai2
force-pushed
the
kai/host-callable-errors
branch
from
0c88708 to
259aae3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/baml_builtins2/baml_std/baml/ns_host/host.baml`:
- Around line 6-10: The comment incorrectly states HostContractViolation is
"uncatchable"; update the wording in the Host completion-site doc (the block
referencing `type_arg_0` / `type_arg_1`) to say that return/throw mismatches are
routed through the VM unwinder and produce `baml.panics.HostContractViolation`,
which is catchable via `catch` like other panics (see
`baml_language/crates/baml_builtins2/baml_std/baml/ns_panics/panics.baml` and
references to `HostContractViolation`); replace "uncatchable" with language
clarifying it is delivered via the VM unwinder and can be intercepted by
`catch`.
🪄 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: e20f7cc8-779c-4def-ac71-5a67f747519b
⛔ Files ignored due to path filters (8)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/sdks/nodejs/bridge_nodejs/dist/native.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/native.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**baml_language/sdks/python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
baml_language/Cargo.tomlbaml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_host/host.bamlbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/host_value_callable.rsbaml_language/crates/bex_vm_types/src/errors.rsbaml_language/crates/bridge_cffi/src/ffi/host_value.rsbaml_language/crates/bridge_wasm/Cargo.tomlbaml_language/crates/bridge_wasm/src/handle.rsbaml_language/crates/bridge_wasm/src/lib.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/bridge_wasm/src/wasm_sys.rsbaml_language/crates/sys_native/src/host_impls.rsbaml_language/crates/sys_ops/src/lib.rsbaml_language/sdk_tests/crates/typescript_node/function_calls/customizable/host_callables.test.tsbaml_language/sdks/nodejs/bridge_nodejs/src/host_value.rsbaml_language/sdks/nodejs/bridge_nodejs/tests/host_callable.test.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/native.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/python/tests/test_host_callable.py
✅ Files skipped from review due to trivial changes (2)
- baml_language/Cargo.toml
- baml_language/crates/bridge_wasm/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (16)
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/native.d.ts
- baml_language/crates/bridge_wasm/src/handle.rs
- baml_language/sdk_tests/crates/typescript_node/function_calls/customizable/host_callables.test.ts
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.js
- baml_language/sdks/nodejs/bridge_nodejs/tests/host_callable.test.ts
- baml_language/crates/bridge_wasm/src/wasm_sys.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.baml
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.ts
- baml_language/crates/bridge_cffi/src/ffi/host_value.rs
- baml_language/sdks/python/tests/test_host_callable.py
- baml_language/crates/bex_engine/src/conversion.rs
- baml_language/crates/sys_native/src/host_impls.rs
- baml_language/sdks/nodejs/bridge_nodejs/src/host_value.rs
- baml_language/crates/bex_vm_types/src/errors.rs
- baml_language/crates/bex_engine/tests/host_value_callable.rs
- baml_language/crates/sys_ops/src/lib.rs
2kai2kai2
force-pushed
the
kai/host-callable-errors
branch
from
259aae3 to
5c42129
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/bex_resource_types/src/host_value.rs`:
- Around line 580-607: The test
intern_collision_callable_vs_error_release_build_keeps_existing currently
freezes release behavior that coerces mismatched HostValueKind values; instead,
make HostValueArc::intern perform the kind check unconditionally (remove
cfg-only debug_assert) so a key->kind mismatch is treated as a contract
violation at runtime, and update the test to expect a failure (panic or Err)
when calling HostValueArc::intern with the same numeric key but different
HostValueKind (replace assertions that Arc::ptr_eq and coerced.kind ==
HostValueKind::Callable with an assertion that the intern call fails/panics),
leaving FIRED assertions adjusted or removed to reflect that no alias/coercion
occurs.
🪄 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: 8f12e127-6525-4acb-96f8-869b944133e3
⛔ Files ignored due to path filters (27)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_namespace_env.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/exceptions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snapbaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_inbound.pb.gois excluded by!**/*.pb.gobaml_language/sdks/nodejs/bridge_nodejs/dist/baml_node.darwin-arm64.nodeis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/host_error_registry.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/host_error_registry.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/host_error_registry.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/host_error_registry.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/index.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/index.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/native.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/native.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**
📒 Files selected for processing (66)
baml_language/Cargo.tomlbaml_language/crates/baml_builtins2/baml_std/baml/ns_env/env.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_fs/fs.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_host/host.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_panics/panics.bamlbaml_language/crates/baml_builtins2_codegen/src/codegen_io.rsbaml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_lsp_server/src/playground_http.rsbaml_language/crates/baml_lsp_server/src/playground_io.rsbaml_language/crates/baml_tests/tests/fs.rsbaml_language/crates/baml_tests/tests/http.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/host_value_callable.rsbaml_language/crates/bex_external_types/src/bex_external_value.rsbaml_language/crates/bex_resource_types/src/host_value.rsbaml_language/crates/bex_vm/src/errors.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm_types/Cargo.tomlbaml_language/crates/bex_vm_types/src/errors.rsbaml_language/crates/bex_vm_types/src/lib.rsbaml_language/crates/bex_vm_types/src/types.rsbaml_language/crates/bridge_cffi/src/ffi/host_value.rsbaml_language/crates/bridge_ctypes/src/value_decode.rsbaml_language/crates/bridge_ctypes/src/value_encode.rsbaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_inbound.protobaml_language/crates/bridge_wasm/Cargo.tomlbaml_language/crates/bridge_wasm/src/handle.rsbaml_language/crates/bridge_wasm/src/host_value.rsbaml_language/crates/bridge_wasm/src/lib.rsbaml_language/crates/bridge_wasm/src/wasm_env.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/bridge_wasm/src/wasm_io.rsbaml_language/crates/bridge_wasm/src/wasm_io_fs.rsbaml_language/crates/bridge_wasm/src/wasm_io_glob.rsbaml_language/crates/bridge_wasm/src/wasm_sys.rsbaml_language/crates/bridge_wasm/tests/host_callable.rsbaml_language/crates/sys_llm/src/lib.rsbaml_language/crates/sys_llm/src/types/mod.rsbaml_language/crates/sys_native/src/host_dispatch.rsbaml_language/crates/sys_native/src/host_impls.rsbaml_language/crates/sys_native/src/io_impls.rsbaml_language/crates/sys_native/src/lib.rsbaml_language/crates/sys_native/src/ops/http.rsbaml_language/crates/sys_ops/src/lib.rsbaml_language/crates/sys_types/src/lib.rsbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_host_callables.pybaml_language/sdk_tests/crates/typescript_node/function_calls/customizable/host_callables.test.tsbaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_host_callable_tests/main.bamlbaml_language/sdks/nodejs/bridge_nodejs/src/host_value.rsbaml_language/sdks/nodejs/bridge_nodejs/tests/host_callable.test.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/host_error_registry.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/index.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/native.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/python/rust/bridge_python/src/host_value.rsbaml_language/sdks/python/rust/bridge_python/src/lib.rsbaml_language/sdks/python/rust/bridge_python/src/py_handle.rsbaml_language/sdks/python/src/baml_core/baml_py.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyibaml_language/sdks/python/src/baml_core/proto.pybaml_language/sdks/python/tests/test_host_callable.py
💤 Files with no reviewable changes (24)
- baml_language/crates/sys_native/src/lib.rs
- baml_language/crates/sys_native/src/ops/http.rs
- baml_language/sdks/python/src/baml_core/proto.py
- baml_language/sdk_tests/fixtures/function_calls/baml_src/ns_host_callable_tests/main.baml
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/index.ts
- baml_language/sdks/python/rust/bridge_python/src/py_handle.rs
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/host_error_registry.ts
- baml_language/sdks/python/rust/bridge_python/src/lib.rs
- baml_language/sdks/nodejs/bridge_nodejs/tests/host_callable.test.ts
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyi
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.js
- baml_language/sdks/nodejs/bridge_nodejs/src/host_value.rs
- baml_language/sdk_tests/crates/typescript_node/function_calls/customizable/host_callables.test.ts
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/native.d.ts
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.d.ts
- baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_host_callables.py
- baml_language/sdks/python/tests/test_host_callable.py
- baml_language/sdks/python/rust/bridge_python/src/host_value.rs
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.py
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.ts
- baml_language/sdks/python/src/baml_core/baml_py.pyi
- baml_language/crates/sys_ops/src/lib.rs
- baml_language/crates/sys_types/src/lib.rs
- baml_language/crates/sys_native/src/io_impls.rs
✅ Files skipped from review due to trivial changes (5)
- baml_language/crates/bridge_wasm/Cargo.toml
- baml_language/crates/baml_builtins2_codegen/src/extract.rs
- baml_language/crates/bex_vm_types/Cargo.toml
- baml_language/crates/bridge_wasm/src/lib.rs
- baml_language/crates/sys_llm/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (34)
- baml_language/crates/bex_vm_types/src/lib.rs
- baml_language/crates/bex_external_types/src/bex_external_value.rs
- baml_language/crates/bridge_wasm/src/handle.rs
- baml_language/crates/baml_tests/tests/fs.rs
- baml_language/crates/sys_llm/src/types/mod.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_env/env.baml
- baml_language/crates/baml_lsp_server/src/playground_http.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.baml
- baml_language/crates/baml_builtins2/baml_std/baml/ns_panics/panics.baml
- baml_language/crates/bridge_wasm/tests/host_callable.rs
- baml_language/crates/bridge_wasm/src/wasm_env.rs
- baml_language/crates/bridge_ctypes/src/value_decode.rs
- baml_language/crates/bridge_ctypes/src/value_encode.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_host/host.baml
- baml_language/crates/bridge_wasm/src/wasm_io_glob.rs
- baml_language/crates/bex_vm/src/errors.rs
- baml_language/crates/bridge_wasm/src/wasm_io.rs
- baml_language/crates/sys_native/src/host_dispatch.rs
- baml_language/crates/baml_builtins2_codegen/src/codegen_io.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_fs/fs.baml
- baml_language/crates/baml_lsp_server/src/playground_io.rs
- baml_language/crates/bex_vm_types/src/types.rs
- baml_language/crates/bridge_wasm/src/wasm_sys.rs
- baml_language/crates/bridge_cffi/src/ffi/host_value.rs
- baml_language/crates/bridge_wasm/src/wasm_http.rs
- baml_language/crates/bex_vm_types/src/errors.rs
- baml_language/crates/sys_native/src/host_impls.rs
- baml_language/crates/bex_engine/src/conversion.rs
- baml_language/crates/bex_engine/src/lib.rs
- baml_language/crates/bridge_wasm/src/wasm_io_fs.rs
- baml_language/crates/bridge_wasm/src/host_value.rs
- baml_language/crates/bex_vm/src/vm.rs
- baml_language/crates/bex_engine/tests/host_value_callable.rs
- baml_language/crates/baml_tests/tests/http.rs
2kai2kai2
force-pushed
the
kai/host-callable-errors
branch
from
5c42129 to
577a3d4
Compare
- windows sdk tests build faster now (less redundant uv sync / pnpm install) - use 8vcpu windows runners (they schedule faster) - on 8vcpu windows runners, use -j16 to better saturate cpu (sccache usage means build is i/o bound) - use mise for tool installation always - make snapshot tests use an 8vcpu machine <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Consolidated tool installation into a shared setup, added nextest/insta tooling, and updated cache versioning. * Adjusted self-hosted runner targets and job-level build parallelism for faster, more consistent CI runs. * **Tests** * Streamlined SDK and snapshot test workflows and improved Windows Python test setup by prebuilding and installing shared wheels. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This will allow BAML to hold opaque handles to non-BAML error types of the host language and (potentially) pass them through with a nice call stack without lossy transformation of host-language error values.
- Makes sysop errors unwind like other BAML errors, making them catchable. Previously they were always treated as fatal internal errors. - Unifies the error type to be the same as normal `$rust_function` errors.
If the host callable returned/threw a value that did not match the function type, we produce a `HostContractViolation` panic.
When a Python function calls a BAML function with a lambda, the lambda throws a Python error, then the BAML function re-throws the opaque HostCallable error it got, then the Python function will recieve the *same* object that the lambda threw. Nice.
2kai2kai2
force-pushed
the
kai/host-callable-errors
branch
from
577a3d4 to
1900bbb
Compare
This was referenced Jun 22, 2026
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
2 participants
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.
When a host function called from BAML throws an error, we can take one of three paths:
HostContractViolationpanic is raised in BAMLHostCallableBAML error which is thrown in BAML. If theHostCallableis returned back out to the host then it will get the value from the table.This also includes making sysop errors properly catchable: previously all sysop errors were just converted into fatal internal engine errors.
Summary by CodeRabbit
New Features
Bug Fixes