Repository navigation
refactor(typescript): prepare the SDK for Node, Web, and Workers - #4051
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (47)
📒 Files selected for processing (135)
💤 Files with no reviewable changes (3)
📝 WalkthroughWalkthroughThe PR migrates the Node.js SDK bridge to a shared TypeScript bridge workspace, adds runtime-configurable Node/Web SDK generation, introduces native runtime, handle, media, cancellation, tracing, and host-callable APIs, and expands the TypeScript SDK test harness across Node, Web, and Workers layouts. ChangesTypeScript SDK bridge migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SDKTestHarness
participant TypeScriptGenerator
participant TypeScriptBridge
participant BamlRuntime
SDKTestHarness->>TypeScriptGenerator: generate Node/Web/Workers SDK sources
TypeScriptGenerator->>SDKTestHarness: write runtime-specific fixture outputs
SDKTestHarness->>TypeScriptBridge: initialize runtime and encode calls
TypeScriptBridge->>BamlRuntime: callFunction or callFunctionSync
BamlRuntime-->>TypeScriptBridge: encoded result or error envelope
TypeScriptBridge-->>SDKTestHarness: decoded SDK result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages. 🔧 ast-grep (0.44.1)baml_language/sdks/typescript/bridge_typescript/typescript_src/proto/baml_cffi.jsast-grep timed out on this file 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 |
sxlijin
force-pushed
the
sam/typescript-runtime-matrix
branch
from
July 16, 2026 04:43
8173b78 to
4313734
Compare
sxlijin
marked this pull request as ready for review
July 16, 2026 05:00
![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: 43137347dc
ℹ️ 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".
| use baml_codegen_types::SymbolPool; | ||
| pub use baml_codegen_types::{NamingConvention, OutputType}; | ||
|
|
||
| const RUNTIME_PACKAGE: &str = "@boundaryml/baml-bridge-web"; |
There was a problem hiding this comment.
When a user selects the newly accepted output_type = "typescript/web", the CLI now routes into this generator and every emitted SDK imports @boundaryml/baml-bridge-web. I checked the repo/package manifests with rg "baml-bridge-web" and there is no package providing that name (the only runtime package is @boundaryml/baml-bridge, and the web tests are explicitly ignored until the runtime lands), so generated web SDKs cannot install or typecheck. Either keep this output type unavailable until the web bridge package is added, or point it at an existing package.
Useful? React with 👍 / 👎.
![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: 5
🧹 Nitpick comments (7)
baml_language/sdks/typescript/bridge_typescript/src/baml_call_context.rs (1)
29-33: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrain the active call IDs to avoid redundant cancellations and clones.
Cloning the vector leaves the IDs inside
active_call_ids. Ifabort()is called multiple times, it will unnecessarily re-cancel the same IDs. Usingstd::mem::takeclears the vector while returning its contents, preventing redundant cancellations and avoiding the allocation cost of cloning.⚡ Proposed refactor
let call_ids = self .active_call_ids .lock() - .map(|ids| ids.clone()) + .map(|mut ids| std::mem::take(&mut *ids)) .unwrap_or_default();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/src/baml_call_context.rs` around lines 29 - 33, Update the active call ID extraction in the relevant abort flow to use std::mem::take on the locked active_call_ids vector instead of cloning it. Ensure the vector is cleared while its contents are returned, preserving the existing default behavior if the mutex is poisoned and preventing repeated abort calls from re-canceling IDs.baml_language/sdks/typescript/bridge_typescript/src/runtime.rs (1)
152-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd Rust unit tests for
decode_argsvalidation paths.
decode_argshas real branching logic (zerocall_idrejection, type_args translation, kwargs conversion) with no visible Rust unit tests covering it directly; coverage currently relies on the TS integration suite. As per path instructions,**/*.rs: "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/sdks/typescript/bridge_typescript/src/runtime.rs` around lines 152 - 173, Add Rust unit tests targeting the private decode_args function, covering rejection of a zero call_id, successful type_args translation, kwargs conversion, and malformed protobuf input. Construct representative CallFunctionArgs protobuf payloads, invoke decode_args directly, and assert both returned DecodedCallArgs fields and the expected errors while keeping existing behavior unchanged.Source: Path instructions
baml_language/sdks/typescript/bridge_typescript/src/handle.rs (1)
47-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a Rust unit test for the
HandleKeybit round-trip.
to_u64/from_u64is pure, napi-independent logic (no JS runtime needed) that underpins every handle key in the bridge — a regression here would silently corrupt keys. It's currently only exercised indirectly through TS integration tests (e.g.test_decode_handle.test.ts). As per coding guidelines,**/*.rsshould prefer unit tests over integration tests where possible, and this is a case where a direct unit test is trivial to add.✅ Suggested unit test
#[cfg(test)] mod tests { use super::HandleKey; #[test] fn handle_key_round_trips() { for v in [0u64, 1, u32::MAX as u64, u64::MAX, 1u64 << 32, 0x1234_5678_9abc_def0] { assert_eq!(HandleKey::from_u64(v).to_u64(), v); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/src/handle.rs` around lines 47 - 58, Add a Rust unit-test module near HandleKey that directly exercises HandleKey::from_u64 and HandleKey::to_u64. Define a handle_key_round_trips test covering zero, boundary values, u64::MAX, a 32-bit shift, and a representative mixed-bit value, asserting each value round-trips unchanged without requiring a napi or JavaScript runtime.Source: Coding guidelines
baml_language/sdks/typescript/bridge_typescript/src/media.rs (1)
49-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConfusing error message field name in
create_media.
c_string(value, context)(Line 56) passes the operation name ("fromUrl","fromFile","fromBase64") as thefieldargument, so a NUL byte in the value produces"fromUrl cannot contain NUL"instead of naming the actual offending value (url/file/base64 string). Consider passing a value-specific field label distinct from thecontextused forstatus_to_napi.💡 Proposed fix
fn create_media( constructor: MediaConstructor, media_kind: MediaTypeEnum, value: String, mime_type: Option<String>, context: &str, ) -> napi::Result<(u64, i32)> { - let value = c_string(value, context)?; + let value = c_string(value, "value")?; let mime_type = mime_type.map(|s| c_string(s, "mimeType")).transpose()?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/src/media.rs` around lines 49 - 76, Update create_media so the value passed to c_string uses a value-specific field label (such as url, file, or base64) rather than context, while preserving context for status_to_napi error reporting. Ensure NUL-byte errors identify the offending media value field instead of the operation name.baml_language/sdks/typescript/bridge_typescript/tests/host_callable.test.ts (1)
161-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest doesn't exercise real concurrency for the collision check.
caughtFirstfully resolves beforecaughtSecondstarts, so the two registry entries are never live simultaneously — this doesn't actually stress the collision scenario the test name implies. Consider awaiting both concurrently.♻️ Proposed fix for genuine concurrency
- const caughtFirst = await callFunction(rt, 'CallCb', { - callback: (_x: number): string => { throw raisedFirst; }, - x: 1, - }).catch(e => e); - const caughtSecond = await callFunction(rt, 'CallCb', { - callback: (_x: number): string => { throw raisedSecond; }, - x: 2, - }).catch(e => e); + const [caughtFirst, caughtSecond] = await Promise.all([ + callFunction(rt, 'CallCb', { + callback: (_x: number): string => { throw raisedFirst; }, + x: 1, + }).catch(e => e), + callFunction(rt, 'CallCb', { + callback: (_x: number): string => { throw raisedSecond; }, + x: 2, + }).catch(e => e), + ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/tests/host_callable.test.ts` around lines 161 - 176, Update the test around callFunction so both CallCb invocations are started before awaiting either result, using concurrent promise handling while preserving the separate raisedFirst and raisedSecond callbacks and identity assertions. Ensure both registry entries remain active simultaneously to validate that rapid concurrent errors do not collide.baml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.ts (2)
53-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider preserving the original function's parameter types.
The current generic signature
(...args: unknown[]) => Tdiscards the parameter types of the original functionfn, which might result in a loss of type safety or IDE autocompletion for the wrapped function. You can preserve the exact signature by making the entire function type generic.💡 Proposed refactor
- traceFn<T>(name: string, fn: (...args: unknown[]) => T): (...args: unknown[]) => T { + traceFn<F extends (...args: any[]) => any>(name: string, fn: F): (...args: Parameters<F>) => ReturnType<F> { const self = this; - return function (this: unknown, ...args: unknown[]): T { + return function (this: unknown, ...args: Parameters<F>): ReturnType<F> { const mgr = self.cloneContext();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.ts` around lines 53 - 69, Update the traceFn method to be generic over the entire function type, preserving fn’s parameter tuple and return type in the wrapped function signature. Keep the existing context tracing, invocation, and error-handling behavior unchanged while replacing the unknown[] parameter typing with inferred function parameters.
71-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider preserving the original async function's parameter types.
Similar to
traceFn, the generic signature here drops the original parameter types of the function being wrapped. UsingParameters<F>will retain type safety.💡 Proposed refactor
- traceFnAsync<T>(name: string, fn: (...args: unknown[]) => Promise<T>): (...args: unknown[]) => Promise<T> { + traceFnAsync<F extends (...args: any[]) => Promise<any>>(name: string, fn: F): (...args: Parameters<F>) => ReturnType<F> { const self = this; - return async function (this: unknown, ...args: unknown[]): Promise<T> { + return async function (this: unknown, ...args: Parameters<F>): ReturnType<F> { const mgr = self.cloneContext();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.ts` around lines 71 - 87, Update traceFnAsync to be generic over the wrapped function type and derive its arguments from Parameters<F>, while preserving the Promise return type and this binding behavior. Ensure the returned wrapper accepts the original function’s parameter types instead of (...args: unknown[]), matching the type-safety approach used by traceFn.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@baml_language/sdk_tests/crates/typescript/function_calls/customizable/cancellation.node.test.ts`:
- Around line 137-158: Update the reused-call-context test “surfaces a reused
call context as AbortError with BAML reason” to capture a start timestamp before
invoking callFunction and call expectFastCancellation(start) after verifying the
cancellation reason. Preserve the existing abort and rejection assertions.
In `@baml_language/sdks/typescript/bridge_typescript/src/host_value.rs`:
- Around line 129-140: Update register_host_callable to handle
REGISTRY.table.lock() poisoning explicitly instead of calling unwrap. Match the
lock result consistently with drop_registry_entry and host_dispatch_callback,
log the warning through the file’s existing mechanism, and return an appropriate
napi::Result error without panicking across the FFI boundary.
In
`@baml_language/sdks/typescript/bridge_typescript/typescript_src/define_function.ts`:
- Around line 234-239: Update typeArgsFor and the corresponding non-generic
handling near the later type-argument path to call resolveTypesOption even when
isGeneric is false. Reject any supplied $types value with the same strict
validation/error behavior as the generic path, while continuing to return no
type arguments when $types is absent.
In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/errors.ts`:
- Around line 81-84: Update wrapNativeError to inspect the native error’s
message and code, then return the corresponding typed subclass such as
BamlInvalidArgumentError or BamlClientError instead of always constructing
BamlError. Preserve the existing String(err) fallback for non-Error values and
ensure unmapped errors continue returning the generic BamlError.
In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/wire_ty.ts`:
- Around line 141-167: Update outboundTyToBamlType to preserve wire-encoded
never and union types emitted by lowerTypeToWireTy instead of falling through to
undefined. Decode each representation into the corresponding BamlType token or
binding, while retaining the existing handling for primitive, collection,
optional, and class types.
---
Nitpick comments:
In `@baml_language/sdks/typescript/bridge_typescript/src/baml_call_context.rs`:
- Around line 29-33: Update the active call ID extraction in the relevant abort
flow to use std::mem::take on the locked active_call_ids vector instead of
cloning it. Ensure the vector is cleared while its contents are returned,
preserving the existing default behavior if the mutex is poisoned and preventing
repeated abort calls from re-canceling IDs.
In `@baml_language/sdks/typescript/bridge_typescript/src/handle.rs`:
- Around line 47-58: Add a Rust unit-test module near HandleKey that directly
exercises HandleKey::from_u64 and HandleKey::to_u64. Define a
handle_key_round_trips test covering zero, boundary values, u64::MAX, a 32-bit
shift, and a representative mixed-bit value, asserting each value round-trips
unchanged without requiring a napi or JavaScript runtime.
In `@baml_language/sdks/typescript/bridge_typescript/src/media.rs`:
- Around line 49-76: Update create_media so the value passed to c_string uses a
value-specific field label (such as url, file, or base64) rather than context,
while preserving context for status_to_napi error reporting. Ensure NUL-byte
errors identify the offending media value field instead of the operation name.
In `@baml_language/sdks/typescript/bridge_typescript/src/runtime.rs`:
- Around line 152-173: Add Rust unit tests targeting the private decode_args
function, covering rejection of a zero call_id, successful type_args
translation, kwargs conversion, and malformed protobuf input. Construct
representative CallFunctionArgs protobuf payloads, invoke decode_args directly,
and assert both returned DecodedCallArgs fields and the expected errors while
keeping existing behavior unchanged.
In `@baml_language/sdks/typescript/bridge_typescript/tests/host_callable.test.ts`:
- Around line 161-176: Update the test around callFunction so both CallCb
invocations are started before awaiting either result, using concurrent promise
handling while preserving the separate raisedFirst and raisedSecond callbacks
and identity assertions. Ensure both registry entries remain active
simultaneously to validate that rapid concurrent errors do not collide.
In
`@baml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.ts`:
- Around line 53-69: Update the traceFn method to be generic over the entire
function type, preserving fn’s parameter tuple and return type in the wrapped
function signature. Keep the existing context tracing, invocation, and
error-handling behavior unchanged while replacing the unknown[] parameter typing
with inferred function parameters.
- Around line 71-87: Update traceFnAsync to be generic over the wrapped function
type and derive its arguments from Parameters<F>, while preserving the Promise
return type and this binding behavior. Ensure the returned wrapper accepts the
original function’s parameter types instead of (...args: unknown[]), matching
the type-safety approach used by traceFn.
🪄 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: 8ca825c2-1b2c-4c54-b61f-8a91878f0720
⛔ Files ignored due to path filters (47)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/sdks/typescript/bridge_typescript/dist/ctx_manager.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/ctx_manager.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/ctx_manager.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/ctx_manager.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/define_function.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/define_function.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/define_function.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/define_function.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/errors.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/errors.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/errors.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/errors.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/exit_hook.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/exit_hook.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/exit_hook.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/exit_hook.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/host_value_registry.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/host_value_registry.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/host_value_registry.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/host_value_registry.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/index.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/index.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/index.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/native.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/native.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/proto.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/proto.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/proto.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/proto.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/proto/baml_cffi.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/proto/baml_cffi.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/stream.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/stream.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/stream.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/stream.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/typemap.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/typemap.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/typemap.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/typemap.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/wire_ty.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/wire_ty.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/wire_ty.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/wire_ty.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (135)
.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/cargo-tests.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish2-nodejs-sdk.yaml.gitignorebaml_language/.config/nextest.tomlbaml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_codegen_types/src/generator_fields.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/host_value_callable.rsbaml_language/crates/bridge_ctypes/README.mdbaml_language/sdk_tests/DEVELOPMENT.mdbaml_language/sdk_tests/README.mdbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_methods_on_classes.pybaml_language/sdk_tests/crates/typescript/Cargo.tomlbaml_language/sdk_tests/crates/typescript/build.rsbaml_language/sdk_tests/crates/typescript/docstrings_etc/customizable/main.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/cancellation.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/host_callables.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/main.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/methods_on_classes.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/optional_args.static.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/optional_args.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/runtime.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/stdlib_entrypoints.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/union_throws.test.tsbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/main.test.tsbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/replay_harness.node.tsbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/streaming_e2e.node.test.tsbaml_language/sdk_tests/crates/typescript/setup.ps1baml_language/sdk_tests/crates/typescript/setup.shbaml_language/sdk_tests/crates/typescript/src/lib.rsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/main.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_aliases.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_class_refs.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_complex_models.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_enums.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_forward_refs.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_generics.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_handles.node.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_lists.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_literals.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_maps.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_media.node.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_optional.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_primitives.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_recursion.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_routing.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_streams.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_symbol_collisions.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_unions.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_void.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/test_generic.test.tsbaml_language/sdk_tests/crates/typescript_node/build.rsbaml_language/sdk_tests/fixtures/function_calls/baml_src/main.bamlbaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_generic_tests/types.bamlbaml_language/sdk_tests/harness_runner/Cargo.tomlbaml_language/sdk_tests/harness_runner/src/lib.rsbaml_language/sdk_tests/harness_setup/Cargo.tomlbaml_language/sdk_tests/harness_setup/src/lib.rsbaml_language/sdk_tests/harness_setup/src/templates/package.jsonbaml_language/sdk_tests/harness_setup/src/templates/tsconfig.jsonbaml_language/sdk_tests/harness_setup/src/templates/tsconfig_web.jsonbaml_language/sdk_tests/harness_setup/src/templates/tsconfig_workers.jsonbaml_language/sdk_tests/harness_setup/src/templates/vitest_node.config.tsbaml_language/sdk_tests/harness_setup/src/templates/vitest_web.config.tsbaml_language/sdk_tests/harness_setup/src/templates/vitest_workers.config.tsbaml_language/sdk_tests/harness_setup/src/typescript.rsbaml_language/sdk_tests/harness_setup/src/typescript_node.rsbaml_language/sdks/go/bridge_go/pkg/tests/host_callable_test.gobaml_language/sdks/nodejs/bridge_nodejs/typescript_src/generated-header.txtbaml_language/sdks/python/rust/bridge_python/src/host_value.rsbaml_language/sdks/typescript/bridge_typescript/.gitignorebaml_language/sdks/typescript/bridge_typescript/Cargo.tomlbaml_language/sdks/typescript/bridge_typescript/build.rsbaml_language/sdks/typescript/bridge_typescript/package.jsonbaml_language/sdks/typescript/bridge_typescript/src/baml_call_context.rsbaml_language/sdks/typescript/bridge_typescript/src/errors.rsbaml_language/sdks/typescript/bridge_typescript/src/handle.rsbaml_language/sdks/typescript/bridge_typescript/src/host_value.rsbaml_language/sdks/typescript/bridge_typescript/src/lib.rsbaml_language/sdks/typescript/bridge_typescript/src/media.rsbaml_language/sdks/typescript/bridge_typescript/src/runtime.rsbaml_language/sdks/typescript/bridge_typescript/src/types/collector.rsbaml_language/sdks/typescript/bridge_typescript/src/types/host_span_manager.rsbaml_language/sdks/typescript/bridge_typescript/src/types/mod.rsbaml_language/sdks/typescript/bridge_typescript/tests/call_function.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/host_callable.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/run_vitest.rsbaml_language/sdks/typescript/bridge_typescript/tests/test_decode_call_result.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_decode_handle.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_engine.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_media.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_stream.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_typemap.test.tsbaml_language/sdks/typescript/bridge_typescript/tsconfig.jsonbaml_language/sdks/typescript/bridge_typescript/typescript_src/clean-dist.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/copy-native-dts.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/copy-proto.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/define_function.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/errors.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/exit_hook.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/generated-header.txtbaml_language/sdks/typescript/bridge_typescript/typescript_src/host_value_registry.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/index.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/native.d.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/proto.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/proto/baml_cffi.d.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/proto/baml_cffi.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/stream.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/tag-generated-files.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/typemap.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/wire_ty.tsbaml_language/sdks/typescript/bridge_typescript/vitest.config.tsbaml_language/sdks/typescript/sdkgen_typescript_shared/Cargo.tomlbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/class.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/enum_.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/function.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/method.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/mod.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/type_alias.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/typemap_file.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/lib.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/routing.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/sdkgen_typescript.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/sdkgen_typescript_web.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rsbaml_language/stow.tomlpnpm-workspace.yamlscripts/baml-language-versiontools/build
💤 Files with no reviewable changes (3)
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/generated-header.txt
- baml_language/sdk_tests/crates/typescript_node/build.rs
- baml_language/sdk_tests/harness_setup/src/typescript_node.rs
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🧹 Nitpick comments (7)
baml_language/sdks/typescript/bridge_typescript/src/baml_call_context.rs (1)
29-33: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrain the active call IDs to avoid redundant cancellations and clones.
Cloning the vector leaves the IDs inside
active_call_ids. Ifabort()is called multiple times, it will unnecessarily re-cancel the same IDs. Usingstd::mem::takeclears the vector while returning its contents, preventing redundant cancellations and avoiding the allocation cost of cloning.⚡ Proposed refactor
let call_ids = self .active_call_ids .lock() - .map(|ids| ids.clone()) + .map(|mut ids| std::mem::take(&mut *ids)) .unwrap_or_default();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/src/baml_call_context.rs` around lines 29 - 33, Update the active call ID extraction in the relevant abort flow to use std::mem::take on the locked active_call_ids vector instead of cloning it. Ensure the vector is cleared while its contents are returned, preserving the existing default behavior if the mutex is poisoned and preventing repeated abort calls from re-canceling IDs.baml_language/sdks/typescript/bridge_typescript/src/runtime.rs (1)
152-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd Rust unit tests for
decode_argsvalidation paths.
decode_argshas real branching logic (zerocall_idrejection, type_args translation, kwargs conversion) with no visible Rust unit tests covering it directly; coverage currently relies on the TS integration suite. As per path instructions,**/*.rs: "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/sdks/typescript/bridge_typescript/src/runtime.rs` around lines 152 - 173, Add Rust unit tests targeting the private decode_args function, covering rejection of a zero call_id, successful type_args translation, kwargs conversion, and malformed protobuf input. Construct representative CallFunctionArgs protobuf payloads, invoke decode_args directly, and assert both returned DecodedCallArgs fields and the expected errors while keeping existing behavior unchanged.Source: Path instructions
baml_language/sdks/typescript/bridge_typescript/src/handle.rs (1)
47-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a Rust unit test for the
HandleKeybit round-trip.
to_u64/from_u64is pure, napi-independent logic (no JS runtime needed) that underpins every handle key in the bridge — a regression here would silently corrupt keys. It's currently only exercised indirectly through TS integration tests (e.g.test_decode_handle.test.ts). As per coding guidelines,**/*.rsshould prefer unit tests over integration tests where possible, and this is a case where a direct unit test is trivial to add.✅ Suggested unit test
#[cfg(test)] mod tests { use super::HandleKey; #[test] fn handle_key_round_trips() { for v in [0u64, 1, u32::MAX as u64, u64::MAX, 1u64 << 32, 0x1234_5678_9abc_def0] { assert_eq!(HandleKey::from_u64(v).to_u64(), v); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/src/handle.rs` around lines 47 - 58, Add a Rust unit-test module near HandleKey that directly exercises HandleKey::from_u64 and HandleKey::to_u64. Define a handle_key_round_trips test covering zero, boundary values, u64::MAX, a 32-bit shift, and a representative mixed-bit value, asserting each value round-trips unchanged without requiring a napi or JavaScript runtime.Source: Coding guidelines
baml_language/sdks/typescript/bridge_typescript/src/media.rs (1)
49-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConfusing error message field name in
create_media.
c_string(value, context)(Line 56) passes the operation name ("fromUrl","fromFile","fromBase64") as thefieldargument, so a NUL byte in the value produces"fromUrl cannot contain NUL"instead of naming the actual offending value (url/file/base64 string). Consider passing a value-specific field label distinct from thecontextused forstatus_to_napi.💡 Proposed fix
fn create_media( constructor: MediaConstructor, media_kind: MediaTypeEnum, value: String, mime_type: Option<String>, context: &str, ) -> napi::Result<(u64, i32)> { - let value = c_string(value, context)?; + let value = c_string(value, "value")?; let mime_type = mime_type.map(|s| c_string(s, "mimeType")).transpose()?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/src/media.rs` around lines 49 - 76, Update create_media so the value passed to c_string uses a value-specific field label (such as url, file, or base64) rather than context, while preserving context for status_to_napi error reporting. Ensure NUL-byte errors identify the offending media value field instead of the operation name.baml_language/sdks/typescript/bridge_typescript/tests/host_callable.test.ts (1)
161-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest doesn't exercise real concurrency for the collision check.
caughtFirstfully resolves beforecaughtSecondstarts, so the two registry entries are never live simultaneously — this doesn't actually stress the collision scenario the test name implies. Consider awaiting both concurrently.♻️ Proposed fix for genuine concurrency
- const caughtFirst = await callFunction(rt, 'CallCb', { - callback: (_x: number): string => { throw raisedFirst; }, - x: 1, - }).catch(e => e); - const caughtSecond = await callFunction(rt, 'CallCb', { - callback: (_x: number): string => { throw raisedSecond; }, - x: 2, - }).catch(e => e); + const [caughtFirst, caughtSecond] = await Promise.all([ + callFunction(rt, 'CallCb', { + callback: (_x: number): string => { throw raisedFirst; }, + x: 1, + }).catch(e => e), + callFunction(rt, 'CallCb', { + callback: (_x: number): string => { throw raisedSecond; }, + x: 2, + }).catch(e => e), + ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/tests/host_callable.test.ts` around lines 161 - 176, Update the test around callFunction so both CallCb invocations are started before awaiting either result, using concurrent promise handling while preserving the separate raisedFirst and raisedSecond callbacks and identity assertions. Ensure both registry entries remain active simultaneously to validate that rapid concurrent errors do not collide.baml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.ts (2)
53-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider preserving the original function's parameter types.
The current generic signature
(...args: unknown[]) => Tdiscards the parameter types of the original functionfn, which might result in a loss of type safety or IDE autocompletion for the wrapped function. You can preserve the exact signature by making the entire function type generic.💡 Proposed refactor
- traceFn<T>(name: string, fn: (...args: unknown[]) => T): (...args: unknown[]) => T { + traceFn<F extends (...args: any[]) => any>(name: string, fn: F): (...args: Parameters<F>) => ReturnType<F> { const self = this; - return function (this: unknown, ...args: unknown[]): T { + return function (this: unknown, ...args: Parameters<F>): ReturnType<F> { const mgr = self.cloneContext();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.ts` around lines 53 - 69, Update the traceFn method to be generic over the entire function type, preserving fn’s parameter tuple and return type in the wrapped function signature. Keep the existing context tracing, invocation, and error-handling behavior unchanged while replacing the unknown[] parameter typing with inferred function parameters.
71-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider preserving the original async function's parameter types.
Similar to
traceFn, the generic signature here drops the original parameter types of the function being wrapped. UsingParameters<F>will retain type safety.💡 Proposed refactor
- traceFnAsync<T>(name: string, fn: (...args: unknown[]) => Promise<T>): (...args: unknown[]) => Promise<T> { + traceFnAsync<F extends (...args: any[]) => Promise<any>>(name: string, fn: F): (...args: Parameters<F>) => ReturnType<F> { const self = this; - return async function (this: unknown, ...args: unknown[]): Promise<T> { + return async function (this: unknown, ...args: Parameters<F>): ReturnType<F> { const mgr = self.cloneContext();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.ts` around lines 71 - 87, Update traceFnAsync to be generic over the wrapped function type and derive its arguments from Parameters<F>, while preserving the Promise return type and this binding behavior. Ensure the returned wrapper accepts the original function’s parameter types instead of (...args: unknown[]), matching the type-safety approach used by traceFn.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@baml_language/sdk_tests/crates/typescript/function_calls/customizable/cancellation.node.test.ts`:
- Around line 137-158: Update the reused-call-context test “surfaces a reused
call context as AbortError with BAML reason” to capture a start timestamp before
invoking callFunction and call expectFastCancellation(start) after verifying the
cancellation reason. Preserve the existing abort and rejection assertions.
In `@baml_language/sdks/typescript/bridge_typescript/src/host_value.rs`:
- Around line 129-140: Update register_host_callable to handle
REGISTRY.table.lock() poisoning explicitly instead of calling unwrap. Match the
lock result consistently with drop_registry_entry and host_dispatch_callback,
log the warning through the file’s existing mechanism, and return an appropriate
napi::Result error without panicking across the FFI boundary.
In
`@baml_language/sdks/typescript/bridge_typescript/typescript_src/define_function.ts`:
- Around line 234-239: Update typeArgsFor and the corresponding non-generic
handling near the later type-argument path to call resolveTypesOption even when
isGeneric is false. Reject any supplied $types value with the same strict
validation/error behavior as the generic path, while continuing to return no
type arguments when $types is absent.
In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/errors.ts`:
- Around line 81-84: Update wrapNativeError to inspect the native error’s
message and code, then return the corresponding typed subclass such as
BamlInvalidArgumentError or BamlClientError instead of always constructing
BamlError. Preserve the existing String(err) fallback for non-Error values and
ensure unmapped errors continue returning the generic BamlError.
In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/wire_ty.ts`:
- Around line 141-167: Update outboundTyToBamlType to preserve wire-encoded
never and union types emitted by lowerTypeToWireTy instead of falling through to
undefined. Decode each representation into the corresponding BamlType token or
binding, while retaining the existing handling for primitive, collection,
optional, and class types.
---
Nitpick comments:
In `@baml_language/sdks/typescript/bridge_typescript/src/baml_call_context.rs`:
- Around line 29-33: Update the active call ID extraction in the relevant abort
flow to use std::mem::take on the locked active_call_ids vector instead of
cloning it. Ensure the vector is cleared while its contents are returned,
preserving the existing default behavior if the mutex is poisoned and preventing
repeated abort calls from re-canceling IDs.
In `@baml_language/sdks/typescript/bridge_typescript/src/handle.rs`:
- Around line 47-58: Add a Rust unit-test module near HandleKey that directly
exercises HandleKey::from_u64 and HandleKey::to_u64. Define a
handle_key_round_trips test covering zero, boundary values, u64::MAX, a 32-bit
shift, and a representative mixed-bit value, asserting each value round-trips
unchanged without requiring a napi or JavaScript runtime.
In `@baml_language/sdks/typescript/bridge_typescript/src/media.rs`:
- Around line 49-76: Update create_media so the value passed to c_string uses a
value-specific field label (such as url, file, or base64) rather than context,
while preserving context for status_to_napi error reporting. Ensure NUL-byte
errors identify the offending media value field instead of the operation name.
In `@baml_language/sdks/typescript/bridge_typescript/src/runtime.rs`:
- Around line 152-173: Add Rust unit tests targeting the private decode_args
function, covering rejection of a zero call_id, successful type_args
translation, kwargs conversion, and malformed protobuf input. Construct
representative CallFunctionArgs protobuf payloads, invoke decode_args directly,
and assert both returned DecodedCallArgs fields and the expected errors while
keeping existing behavior unchanged.
In `@baml_language/sdks/typescript/bridge_typescript/tests/host_callable.test.ts`:
- Around line 161-176: Update the test around callFunction so both CallCb
invocations are started before awaiting either result, using concurrent promise
handling while preserving the separate raisedFirst and raisedSecond callbacks
and identity assertions. Ensure both registry entries remain active
simultaneously to validate that rapid concurrent errors do not collide.
In
`@baml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.ts`:
- Around line 53-69: Update the traceFn method to be generic over the entire
function type, preserving fn’s parameter tuple and return type in the wrapped
function signature. Keep the existing context tracing, invocation, and
error-handling behavior unchanged while replacing the unknown[] parameter typing
with inferred function parameters.
- Around line 71-87: Update traceFnAsync to be generic over the wrapped function
type and derive its arguments from Parameters<F>, while preserving the Promise
return type and this binding behavior. Ensure the returned wrapper accepts the
original function’s parameter types instead of (...args: unknown[]), matching
the type-safety approach used by traceFn.
🪄 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: 8ca825c2-1b2c-4c54-b61f-8a91878f0720
⛔ Files ignored due to path filters (47)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/sdks/typescript/bridge_typescript/dist/ctx_manager.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/ctx_manager.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/ctx_manager.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/ctx_manager.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/define_function.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/define_function.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/define_function.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/define_function.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/errors.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/errors.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/errors.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/errors.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/exit_hook.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/exit_hook.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/exit_hook.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/exit_hook.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/host_value_registry.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/host_value_registry.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/host_value_registry.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/host_value_registry.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/index.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/index.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/index.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/native.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/native.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/proto.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/proto.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/proto.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/proto.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/proto/baml_cffi.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/proto/baml_cffi.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/stream.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/stream.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/stream.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/stream.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/typemap.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/typemap.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/typemap.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/typemap.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/wire_ty.d.tsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/wire_ty.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/dist/wire_ty.jsis excluded by!**/dist/**baml_language/sdks/typescript/bridge_typescript/dist/wire_ty.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/typescript/bridge_typescript/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (135)
.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/cargo-tests.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish2-nodejs-sdk.yaml.gitignorebaml_language/.config/nextest.tomlbaml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_codegen_types/src/generator_fields.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/host_value_callable.rsbaml_language/crates/bridge_ctypes/README.mdbaml_language/sdk_tests/DEVELOPMENT.mdbaml_language/sdk_tests/README.mdbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_methods_on_classes.pybaml_language/sdk_tests/crates/typescript/Cargo.tomlbaml_language/sdk_tests/crates/typescript/build.rsbaml_language/sdk_tests/crates/typescript/docstrings_etc/customizable/main.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/cancellation.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/host_callables.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/main.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/methods_on_classes.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/optional_args.static.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/optional_args.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/runtime.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/stdlib_entrypoints.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/union_throws.test.tsbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/main.test.tsbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/replay_harness.node.tsbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/streaming_e2e.node.test.tsbaml_language/sdk_tests/crates/typescript/setup.ps1baml_language/sdk_tests/crates/typescript/setup.shbaml_language/sdk_tests/crates/typescript/src/lib.rsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/main.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_aliases.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_class_refs.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_complex_models.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_enums.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_forward_refs.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_generics.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_handles.node.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_lists.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_literals.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_maps.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_media.node.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_optional.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_primitives.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_recursion.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_routing.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_streams.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_symbol_collisions.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_unions.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_void.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/test_generic.test.tsbaml_language/sdk_tests/crates/typescript_node/build.rsbaml_language/sdk_tests/fixtures/function_calls/baml_src/main.bamlbaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_generic_tests/types.bamlbaml_language/sdk_tests/harness_runner/Cargo.tomlbaml_language/sdk_tests/harness_runner/src/lib.rsbaml_language/sdk_tests/harness_setup/Cargo.tomlbaml_language/sdk_tests/harness_setup/src/lib.rsbaml_language/sdk_tests/harness_setup/src/templates/package.jsonbaml_language/sdk_tests/harness_setup/src/templates/tsconfig.jsonbaml_language/sdk_tests/harness_setup/src/templates/tsconfig_web.jsonbaml_language/sdk_tests/harness_setup/src/templates/tsconfig_workers.jsonbaml_language/sdk_tests/harness_setup/src/templates/vitest_node.config.tsbaml_language/sdk_tests/harness_setup/src/templates/vitest_web.config.tsbaml_language/sdk_tests/harness_setup/src/templates/vitest_workers.config.tsbaml_language/sdk_tests/harness_setup/src/typescript.rsbaml_language/sdk_tests/harness_setup/src/typescript_node.rsbaml_language/sdks/go/bridge_go/pkg/tests/host_callable_test.gobaml_language/sdks/nodejs/bridge_nodejs/typescript_src/generated-header.txtbaml_language/sdks/python/rust/bridge_python/src/host_value.rsbaml_language/sdks/typescript/bridge_typescript/.gitignorebaml_language/sdks/typescript/bridge_typescript/Cargo.tomlbaml_language/sdks/typescript/bridge_typescript/build.rsbaml_language/sdks/typescript/bridge_typescript/package.jsonbaml_language/sdks/typescript/bridge_typescript/src/baml_call_context.rsbaml_language/sdks/typescript/bridge_typescript/src/errors.rsbaml_language/sdks/typescript/bridge_typescript/src/handle.rsbaml_language/sdks/typescript/bridge_typescript/src/host_value.rsbaml_language/sdks/typescript/bridge_typescript/src/lib.rsbaml_language/sdks/typescript/bridge_typescript/src/media.rsbaml_language/sdks/typescript/bridge_typescript/src/runtime.rsbaml_language/sdks/typescript/bridge_typescript/src/types/collector.rsbaml_language/sdks/typescript/bridge_typescript/src/types/host_span_manager.rsbaml_language/sdks/typescript/bridge_typescript/src/types/mod.rsbaml_language/sdks/typescript/bridge_typescript/tests/call_function.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/host_callable.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/run_vitest.rsbaml_language/sdks/typescript/bridge_typescript/tests/test_decode_call_result.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_decode_handle.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_engine.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_media.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_stream.test.tsbaml_language/sdks/typescript/bridge_typescript/tests/test_typemap.test.tsbaml_language/sdks/typescript/bridge_typescript/tsconfig.jsonbaml_language/sdks/typescript/bridge_typescript/typescript_src/clean-dist.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/copy-native-dts.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/copy-proto.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/ctx_manager.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/define_function.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/errors.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/exit_hook.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/generated-header.txtbaml_language/sdks/typescript/bridge_typescript/typescript_src/host_value_registry.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/index.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/native.d.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/proto.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/proto/baml_cffi.d.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/proto/baml_cffi.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/stream.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/tag-generated-files.jsbaml_language/sdks/typescript/bridge_typescript/typescript_src/typemap.tsbaml_language/sdks/typescript/bridge_typescript/typescript_src/wire_ty.tsbaml_language/sdks/typescript/bridge_typescript/vitest.config.tsbaml_language/sdks/typescript/sdkgen_typescript_shared/Cargo.tomlbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/class.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/enum_.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/function.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/method.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/mod.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/type_alias.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/emit/typemap_file.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/leaf.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/lib.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/routing.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/sdkgen_typescript.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/sdkgen_typescript_web.rsbaml_language/sdks/typescript/sdkgen_typescript_shared/src/translate_ty.rsbaml_language/stow.tomlpnpm-workspace.yamlscripts/baml-language-versiontools/build
💤 Files with no reviewable changes (3)
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/generated-header.txt
- baml_language/sdk_tests/crates/typescript_node/build.rs
- baml_language/sdk_tests/harness_setup/src/typescript_node.rs
🛑 Comments failed to post (5)
baml_language/sdk_tests/crates/typescript/function_calls/customizable/cancellation.node.test.ts (1)
137-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing fast-cancellation timing assertion in the last test.
Every other test in this suite captures
startand ends withexpectFastCancellation(start); this test (reused call context) omits both, so it won't catch a regression where cancellation on a reused context becomes slow.🕐 Proposed fix to add the timing check
it("surfaces a reused call context as AbortError with BAML reason", async () => { + const start = performance.now(); const ctx = new BamlCallContext(); const pending = callFunction( getRuntime(), SLEEP_FQN, { ms: 2000 }, undefined, undefined, ctx, ); await new Promise((resolve) => setTimeout(resolve, 50)); ctx.abort(); try { await pending; throw new Error("expected callFunction to reject"); } catch (error) { expectBamlCancelledReason(error); } + + expectFastCancellation(start); });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.it("surfaces a reused call context as AbortError with BAML reason", async () => { const start = performance.now(); const ctx = new BamlCallContext(); const pending = callFunction( getRuntime(), SLEEP_FQN, { ms: 2000 }, undefined, undefined, ctx, ); await new Promise((resolve) => setTimeout(resolve, 50)); ctx.abort(); try { await pending; throw new Error("expected callFunction to reject"); } catch (error) { expectBamlCancelledReason(error); } expectFastCancellation(start); }); });🤖 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/typescript/function_calls/customizable/cancellation.node.test.ts` around lines 137 - 158, Update the reused-call-context test “surfaces a reused call context as AbortError with BAML reason” to capture a start timestamp before invoking callFunction and call expectFastCancellation(start) after verifying the cancellation reason. Preserve the existing abort and rejection assertions.baml_language/sdks/typescript/bridge_typescript/src/host_value.rs (1)
129-140: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Inconsistent mutex-poisoning handling vs. rest of the file.
Every other lock site in this file (
drop_registry_entry,host_dispatch_callback) explicitly matchesErron.lock()and degrades gracefully with a logged warning. Here,.unwrap()(Line 138) would panic on a poisoned mutex instead of returning a clearnapi::Resulterror, which is inconsistent with the file's stated design intent of never panicking across this FFI boundary.🛡️ Suggested fix
- let key = next_key(); - REGISTRY.table.lock().unwrap().insert(key, Arc::new(tsfn)); - Ok(HandleKey::from_u64(key)) + let key = next_key(); + match REGISTRY.table.lock() { + Ok(mut t) => { + t.insert(key, Arc::new(tsfn)); + Ok(HandleKey::from_u64(key)) + } + Err(e) => Err(napi::Error::new( + napi::Status::GenericFailure, + format!("host-callable registry mutex poisoned during registration: {e}"), + )), + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pub fn register_host_callable(callable: Function<'_, DispatchArgs, ()>) -> napi::Result<HandleKey> { let tsfn: DispatchTsfn = callable .build_threadsafe_function() .callee_handled::<false>() .weak::<false>() // Bound the queue (napi's default is unbounded); see DISPATCH_QUEUE_SIZE. .max_queue_size::<DISPATCH_QUEUE_SIZE>() .build()?; let key = next_key(); match REGISTRY.table.lock() { Ok(mut t) => { t.insert(key, Arc::new(tsfn)); Ok(HandleKey::from_u64(key)) } Err(e) => Err(napi::Error::new( napi::Status::GenericFailure, format!("host-callable registry mutex poisoned during registration: {e}"), )), } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/src/host_value.rs` around lines 129 - 140, Update register_host_callable to handle REGISTRY.table.lock() poisoning explicitly instead of calling unwrap. Match the lock result consistently with drop_registry_entry and host_dispatch_callback, log the warning through the file’s existing mechanism, and return an appropriate napi::Result error without panicking across the FFI boundary.baml_language/sdks/typescript/bridge_typescript/typescript_src/define_function.ts (1)
234-239: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject
$typesinstead of silently ignoring it on non-generic calls.Both
isGenericbranches return[]without invokingresolveTypesOption, so$typesis accepted and discarded despite the strict contract documented at Line 236.Proposed fix
- const isGeneric = typeParams.length > 0; const typeArgsFor = (built: BuiltArgs): WireTypeArg[] => - isGeneric ? buildTypeArgs(undefined, built.types, typeParams, []) : []; + buildTypeArgs(undefined, built.types, typeParams, []); - const isGeneric = typeParams.length > 0 || classTypeParams.length > 0; - const makeArgs = (self: unknown, args: unknown[]): BuiltArgs => { ... const typeArgsFor = (built: BuiltArgs): WireTypeArg[] => - isGeneric ? buildTypeArgs(self, built.types, typeParams, classTypeParams) : []; + buildTypeArgs(self, built.types, typeParams, classTypeParams);Also applies to: 298-311
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/define_function.ts` around lines 234 - 239, Update typeArgsFor and the corresponding non-generic handling near the later type-argument path to call resolveTypesOption even when isGeneric is false. Reject any supplied $types value with the same strict validation/error behavior as the generic path, while continuing to return no type arguments when $types is absent.baml_language/sdks/typescript/bridge_typescript/typescript_src/errors.ts (1)
81-84: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== errors.ts ==\n' ast-grep outline baml_language/sdks/typescript/bridge_typescript/typescript_src/errors.ts --view expanded || true printf '\n== index.ts ==\n' ast-grep outline baml_language/sdks/typescript/bridge_typescript/typescript_src/index.ts --view expanded || true printf '\n== search typed errors / wrapNativeError ==\n' rg -n "wrapNativeError|BamlInvalidArgumentError|BamlClientError|BamlCancelledError|BamlPanic|BamlAbortError|napi::Error|typed Baml" baml_language/sdks/typescript/bridge_typescript/typescript_src -S printf '\n== relevant slices ==\n' sed -n '1,220p' baml_language/sdks/typescript/bridge_typescript/typescript_src/errors.ts printf '\n--- index.ts slice ---\n' sed -n '1,260p' baml_language/sdks/typescript/bridge_typescript/typescript_src/index.ts printf '\n--- handle.rs search ---\n' rg -n "status_to_napi|GenericFailure|InvalidArgument|Client|Cancelled|Abort" baml_language -SRepository: BoundaryML/baml
Length of output: 50372
🏁 Script executed:
#!/bin/bash set -euo pipefail # If the previous outline was too broad, grab narrower line-numbered slices. sed -n '1,220p' baml_language/sdks/typescript/bridge_typescript/typescript_src/errors.ts | cat -n printf '\n---\n' sed -n '1,320p' baml_language/sdks/typescript/bridge_typescript/typescript_src/index.ts | cat -nRepository: BoundaryML/baml
Length of output: 12928
🏁 Script executed:
#!/bin/bash set -euo pipefail # Minimal evidence gathering: show exact lines around the wrapper and its call sites. for f in \ baml_language/sdks/typescript/bridge_typescript/typescript_src/errors.ts \ baml_language/sdks/typescript/bridge_typescript/typescript_src/index.ts do echo "===== $f =====" nl -ba "$f" | sed -n '1,260p' done echo "===== usages =====" rg -n "wrapNativeError|BamlInvalidArgumentError|BamlClientError|BamlCancelledError|BamlPanic|BamlAbortError" baml_language/sdks/typescript/bridge_typescript/typescript_src -SRepository: BoundaryML/baml
Length of output: 279
Parse native errors into typed subclasses
wrapNativeErroralways returnsBamlError, so nativenapi::Errorfailures can never surface asBamlInvalidArgumentErrororBamlClientError. Map the native message/code to the right subclass here before rethrowing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/errors.ts` around lines 81 - 84, Update wrapNativeError to inspect the native error’s message and code, then return the corresponding typed subclass such as BamlInvalidArgumentError or BamlClientError instead of always constructing BamlError. Preserve the existing String(err) fallback for non-Error values and ensure unmapped errors continue returning the generic BamlError.baml_language/sdks/typescript/bridge_typescript/typescript_src/wire_ty.ts (1)
141-167: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve
neverand union bindings during wire decoding.These types are valid
BamlTypetokens and are emitted bylowerTypeToWireTy, but the inverse path currently converts them toundefined. Returned generic instances therefore lose their concrete type bindings.Proposed fix
if (!ty) return undefined; + if (ty.never) return Never; if (ty.primitive) { return TY_PRIMITIVE_TOKEN[ty.primitive.kind ?? -1] ?? undefined; } ... if (ty.optional) return { optional: outboundTyToBamlType(ty.optional.inner) }; + if (ty.union) { + return { + union: (ty.union.options ?? []).map((option) => + outboundTyToBamlType(option) + ), + }; + } if (ty.classTy) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export function outboundTyToBamlType( ty: baml_bridge.cffi.v1.IBamlTy | null | undefined, ): BamlType { if (!ty) return undefined; if (ty.never) return Never; if (ty.primitive) { return TY_PRIMITIVE_TOKEN[ty.primitive.kind ?? -1] ?? undefined; } if (ty.list) return { list: outboundTyToBamlType(ty.list.item) }; if (ty.map) { return { map: [outboundTyToBamlType(ty.map.key), outboundTyToBamlType(ty.map.value)] }; } if (ty.optional) return { optional: outboundTyToBamlType(ty.optional.inner) }; if (ty.union) { return { union: (ty.union.options ?? []).map((option) => outboundTyToBamlType(option) ), }; } if (ty.classTy) { const fqn = ty.classTy.name ?? ''; const args = (ty.classTy.typeArgs ?? []).map((a) => outboundTyToBamlType(a)); let ctor: BamlClassCtor | undefined; try { ctor = getTypeMap().getClass(fqn) as BamlClassCtor; } catch { ctor = undefined; // unmapped FQN — leave unbound } if (!ctor) return undefined; return args.length ? { class: ctor, args } : ctor; } // union / enum / literal / media / type_var / unknown / any other → // unbound wildcard (a structural union is unbound for `class<args>`). return undefined; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript/typescript_src/wire_ty.ts` around lines 141 - 167, Update outboundTyToBamlType to preserve wire-encoded never and union types emitted by lowerTypeToWireTy instead of falling through to undefined. Decode each representation into the corresponding BamlType token or binding, while retaining the existing handling for primitive, collection, optional, and class types.
This was referenced Jul 16, 2026
Closed
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sdks/typescriptso Node and Web can share the same SDK foundation.typescript/webas a generator output and make the SDK test harness runtime-aware withvitest_node,vitest_web, andvitest_workersconfigurations.Why
This isolates the TypeScript/Node refactor needed for multi-runtime testing from the shared Wasm bridge and browser/Workers implementations, so reviewers can verify that the existing Node SDK remains intact before the new runtime code lands.
Impact
This is stack 1 of 3 and is based directly on
canary. The Web and Workers configurations are structural in this PR; their runtime implementations arrive in the later PRs.Checks
mise run stowmise run fmtmise run clippySummary by CodeRabbit
New Features
Bug Fixes
Chores