Initial rust bridge#4045
Conversation
Currently disabled but will be flipped once we are ready
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Rust SDK generation, a dynamic runtime bridge, extensive Rust SDK tests, platform-contract-driven release matrices, crates.io packaging, release-plan schema v2, and consumer-style artifact verification. ChangesRust SDK generation and runtime
Release and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/baml-language-version (1)
502-503: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCheck the Node package against
npm_version.Stamping uses the registry-specific value, but drift validation uses
canonical_version. This will reject correctly stamped plans if npm encoding ever diverges.- checks["node package version"] = f'"version": "{expected.canonical_version}"' in NODE_PACKAGE.read_text(encoding="utf-8") + checks["node package version"] = f'"version": "{expected.npm_version}"' in NODE_PACKAGE.read_text(encoding="utf-8")🤖 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 `@scripts/baml-language-version` around lines 502 - 503, Update the Node package version check in the checks-building logic to compare against the registry-specific npm_version used during stamping, rather than expected.canonical_version. Preserve the existing package file reading and validation behavior.
🟠 Major comments (24)
baml_language/sdks/rust/bridge_rust/src/completion.rs-89-134 (1)
89-134: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftPropagate callback failure instead of stranding the receiver.
If
trampolinepanics after removing the registry entry, Lines 182-190 only log the failure. Neitherwait_blockingnorwaithas a failure state, so the caller can remain blocked or pending forever.Add a terminal error state, wake/notify the receiver on callback failure, and return
Result<Vec<u8>, CompletionError>from both waiting paths.Also applies to: 156-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/bridge_rust/src/completion.rs` around lines 89 - 134, Update the completion state machine used by `Receiver::wait_blocking`, `Receiver::wait`, and the `trampoline` failure path to add a terminal callback-failure state carrying `CompletionError`. Set this state when the callback panics, notify the blocking condition variable and wake any stored waker, and change both waiting methods to return `Result<Vec<u8>, CompletionError>` while propagating either ready bytes or the failure.baml_language/sdks/rust/bridge_rust/src/loader/mod.rs-192-236 (1)
192-236: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRequire discovered library paths to be regular files.
metadata()succeeds for directories, so a directory at an explicit, environment, cache, or system path is returned as a library. For cache hits this also suppresses download-based recovery and guarantees a later load failure. Checkmetadata.is_file()before accepting each candidate.Proposed approach
- Ok(_) => { + Ok(metadata) if metadata.is_file() => { log::debug(&format!( "Using BAML library path set via set_shared_library_path(): {}", path.display() )); Ok(path.clone()) } + Ok(_) => Err(LoaderError::LoadLibrary(format!( + "{} is not a regular file", + path.display() + ))),Apply the equivalent file check to environment, cache, and system-path candidates.
Also applies to: 259-268
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/bridge_rust/src/loader/mod.rs` around lines 192 - 236, Update the library-path discovery logic to accept candidates only when metadata().is_file() is true, not merely when metadata succeeds. Apply this to the explicit path and environment path branches shown, plus the cached candidate and system-path candidate handling near the related loader logic, preserving download recovery when a cache path is not a regular file.baml_language/sdk_tests/crates/rust/function_calls/customizable/test_errors.rs-280-287 (1)
280-287: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not let the clean-exit contract pass without being tested.
This empty test reports success while checking neither
exit(0)norexit(7). Add the documented subprocess harness, or mark the case ignored with an explicit tracking reason until that harness exists.🤖 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/rust/function_calls/customizable/test_errors.rs` around lines 280 - 287, Update test_clean_exit_terminates_process_with_code so it no longer passes without assertions: implement a subprocess harness that invokes throws_test::DoExit with codes 0 and 7 and verifies each child ExitStatus::code(), or explicitly mark the test ignored with a tracking reason until the harness is available.baml_language/sdks/rust/sdkgen_rust/src/unions.rs-173-214 (1)
173-214: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReject container unions that cannot decode an empty value unambiguously.
shape_errorpermits shapes such asint[] | string[], while both arms encode an empty list identically. Declared-order trial decoding therefore silently returns the first variant regardless of the original typed arm. The same problem applies to multiple map arms.Either add an explicit wire discriminator or conservatively reject overlapping container arms.
Also applies to: 252-269
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/sdkgen_rust/src/unions.rs` around lines 173 - 214, Update shape_error to reject unions containing multiple list/container arms or multiple map arms when their empty encodings overlap, preventing declared-order trial decoding from selecting an arbitrary variant. Reuse the existing type/variant inspection helpers in shape_error and return a descriptive shape error while preserving current validation for literals, strings, and duplicate variant names.baml_language/sdks/rust/sdkgen_rust/src/unions.rs-148-170 (1)
148-170: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTraverse generic class arguments when collecting nested unions.
register_unions_inskipsTy::Class(_, args), so a type such asWrapper<int | string>never registers its required synthesized enum and can fail during translation.Proposed fix
Ty::List(inner) => register_unions_in(inner, leaf, analysis, registry), Ty::Map { key: _, value } => register_unions_in(value, leaf, analysis, registry), + Ty::Class(_, args) => { + for arg in args { + register_unions_in(arg, leaf, analysis, registry); + } + } _ => {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/sdkgen_rust/src/unions.rs` around lines 148 - 170, Update register_unions_in to match Ty::Class(_, args) and recursively visit every generic argument, preserving the existing traversal for unions, lists, and map values so nested unions such as Wrapper<int | string> are registered.baml_language/crates/baml_release/src/manifest.rs-93-97 (1)
93-97: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate SDK coordinates against the manifest release.
This currently accepts values such as a
crates_ioSDK version unrelated toself.version, so a validated release manifest can direct consumers to the wrong crate. Pass the manifest version intovalidate_sdkand enforce the registry-specific canonical-version relationship.Also applies to: 154-157
🤖 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/baml_release/src/manifest.rs` around lines 93 - 97, Update the SDK validation flow in the manifest validation method to pass self.version into validate_sdk for every SDK entry. Extend validate_sdk to enforce the registry-specific canonical version relationship, including requiring crates_io SDK versions to match the manifest release version, while preserving existing validation for other registries.scripts/baml-language-version-345-370 (1)
345-370: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject registry versions that disagree with the canonical version.
The loader currently accepts arbitrary/coerced values such as
null, unknown registry keys, or a crates.io version unrelated tocanonical_version. Sincestamp()trusts this map, a malformed plan can publish mutually inconsistent versions.Proposed validation
if not isinstance(raw_registry_versions, dict): fail(f"{path} registry_versions must be a JSON object") - registry_versions = {str(key): str(value) for key, value in raw_registry_versions.items()} - missing = [name for name in PLAN_REGISTRIES if name not in registry_versions] - if missing: - fail(f"{path} registry_versions is missing {', '.join(missing)}") + if not all( + isinstance(key, str) and isinstance(value, str) + for key, value in raw_registry_versions.items() + ): + fail(f"{path} registry_versions keys and values must be strings") + + registry_versions = dict(raw_registry_versions) + if set(registry_versions) != set(PLAN_REGISTRIES): + fail(f"{path} registry_versions must contain exactly {', '.join(PLAN_REGISTRIES)}") + + canonical_version = str(data["canonical_version"]) + expected_registry_versions = registry_versions_for(canonical_version) + if registry_versions != expected_registry_versions: + fail(f"{path} registry_versions does not match canonical_version") try: return ReleasePlan( schema=PLAN_SCHEMA, channel=str(data["channel"]), canary_version=str(data["canary_version"]), - canonical_version=str(data["canonical_version"]), + canonical_version=canonical_version,🤖 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 `@scripts/baml-language-version` around lines 345 - 370, Tighten validation in the ReleasePlan loader before constructing registry_versions: require registry_versions to contain exactly the keys in PLAN_REGISTRIES, reject non-string keys or values instead of coercing them, and require every registry version to equal the plan’s canonical_version. Preserve the existing missing-field and schema validation errors while rejecting unknown, null, or mismatched entries before ReleasePlan is returned.baml_language/sdk_tests/crates/rust/llm_functions/customizable/test_main.rs-196-223 (1)
196-223: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSerialize and restore the shared
OPENAI_API_KEYmutation.These tests run in parallel and can overwrite the variable before the sibling builds its request, producing flaky assertions. Serialize the environment-dependent tests and restore the previous value afterward.
Also applies to: 225-251
🤖 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/rust/llm_functions/customizable/test_main.rs` around lines 196 - 223, The environment-dependent tests around test_extract_resume_build_request_includes_openai_api_key and the sibling test at the additional range must not mutate OPENAI_API_KEY concurrently. Serialize both tests with a shared synchronization guard, capture each test’s prior environment value, and restore it on completion, including when the variable was previously unset.baml_language/sdks/rust/sdkgen_rust/src/analyze.rs-480-491 (1)
480-491: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftEnsure collision renames are themselves unique.
Appending one underscore can collide with an existing sibling module or type named
foo_, merging distinct routed paths or producing invalid Rust. Select a suffix that is unused by both sibling paths and emitted names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/sdkgen_rust/src/analyze.rs` around lines 480 - 491, Update the collision-handling logic around renamed_child and renames.insert so it searches for an available suffix rather than always appending one underscore. Ensure each generated child name is unused by both sibling paths in types_in and existing emitted names in renames, while preserving the current name when no collision exists..github/workflows/build2-bridge-cffi.reusable.yaml-80-89 (1)
80-89: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict the matrix job’s token permissions.
This job inherits repository-default
GITHUB_TOKENpermissions despite only requiring checkout access. Add an explicit read-only permission block.Proposed fix
matrix: name: Compute target matrix runs-on: ubuntu-latest + permissions: + contents: read outputs:🤖 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 @.github/workflows/build2-bridge-cffi.reusable.yaml around lines 80 - 89, Update the “Compute target matrix” job to declare an explicit read-only permissions block, granting only the repository contents permission required by actions/checkout and leaving all other GITHUB_TOKEN permissions unavailable.Source: Linters/SAST tools
baml_language/sdks/rust/bridge_rust/src/decode.rs-40-44 (1)
40-44: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReject malformed float literals instead of coercing them to strings
Literal::FloatValue(s)should fail onparse::<f64>()errors. Falling back toStringValuecan make a bad float match a string union arm and hide wire-contract drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/bridge_rust/src/decode.rs` around lines 40 - 44, Update the Literal::FloatValue branch in the decode logic to propagate or return the parse::<f64>() error instead of converting malformed input into Out::StringValue(s). Preserve Out::FloatValue for successfully parsed values and ensure invalid float literals fail decoding.baml_language/sdks/rust/bridge_rust/src/loader/download.rs-134-147 (1)
134-147: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not replace the atomic install with a direct copy.
std::fs::copytruncatesdest_pathand writes it incrementally. Concurrent loaders can observe a partial library, and an interrupted fallback can destroy a valid cached copy. Preserve atomicity: treat an existing destination as a concurrent winner and otherwise return the rename failure instead of copying into the final path.Proposed fix
if let Err(rename_err) = std::fs::rename(&tmp_path, &dest_path) { - log::warn(&format!( - "Atomic rename failed ({rename_err}), attempting copy fallback" - )); - copy_file(&tmp_path, &dest_path).map_err(|copy_err| { - LoaderError::DownloadFailed(format!( - "failed moving temp file {} to {}: rename failed ({rename_err}) \ - and copy failed ({copy_err})", - tmp_path.display(), - dest_path.display() - )) - })?; - log::info("Copy fallback succeeded"); + if dest_path.is_file() { + log::info("Another process installed the downloaded library"); + } else { + return Err(LoaderError::DownloadFailed(format!( + "failed moving temp file {} to {}: {rename_err}", + tmp_path.display(), + dest_path.display() + ))); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/bridge_rust/src/loader/download.rs` around lines 134 - 147, Update the fallback in the download/install flow around the atomic rename: remove the direct copy_file operation into dest_path. If the destination already exists, treat it as a concurrent winner and continue successfully; otherwise propagate the original rename failure as LoaderError::DownloadFailed, preserving atomic installation and any valid cached file.baml_language/sdks/rust/bridge_rust/src/loader/download.rs-83-119 (1)
83-119: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject empty artifact responses before installation.
When the checksum sidecar is unavailable, an HTTP 200 with an empty body passes validation and is cached as the engine library. Fail when no bytes were downloaded.
Proposed fix
progress.finish(); + if progress.current == 0 { + return Err(LoaderError::DownloadFailed(format!( + "downloaded library {filename} is empty" + ))); + } + let actual_checksum = hex::encode(hasher.finalize());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/bridge_rust/src/loader/download.rs` around lines 83 - 119, Update the download validation flow around the read loop and checksum handling to reject an empty response before caching or installation. Track whether any bytes were written, and return the existing download failure error when the response contains zero bytes, including the target filename or path in the message.baml_language/sdks/rust/bridge_rust/src/baml_value.rs-275-282 (1)
275-282: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject map entries with missing values.
unwrap_or_default()turns malformed entries into wire nulls, soMap<String, Option<T>>can silently accept missing data asNone. Return a decode error whenentry.valueis absent in both implementations.Also applies to: 309-316
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/bridge_rust/src/baml_value.rs` around lines 275 - 282, Update both map-decoding implementations around the visible map entry conversion closures to reject entries whose entry.value is absent instead of calling unwrap_or_default(). Return the existing decode error type for missing values, while preserving K::from_baml_key and V::from_baml handling for present values.baml_language/sdks/rust/sdkgen_rust/src/idents.rs-52-58 (1)
52-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake reserved-name mangling collision-free.
These mappings collide with valid BAML names such as
self_,crate_, and_1, producing duplicate Rust identifiers or module paths. Use an injective escape scheme or resolve collisions against the emitted symbol table.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/sdkgen_rust/src/idents.rs` around lines 52 - 58, Update non_raw_able and the related Rust identifier-generation flow to use collision-free, injective escaping for reserved names, including crate, self, Self, super, and _. Ensure escaped outputs cannot collide with valid BAML names such as crate_, self_, Self_, or _1, including when used as module paths..github/workflows/verify-rust-sdk.reusable.yaml-254-257 (1)
254-257: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winInspect packaged contents, not only tar entry names.
This only rejects archive members beginning with
/; it does not detect absolute build paths embedded in source or metadata. Extract the crate and scan its contents for$GITHUB_WORKSPACE,$RUNNER_TEMP, and other build roots.🤖 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 @.github/workflows/verify-rust-sdk.reusable.yaml around lines 254 - 257, Update the crate validation step after the archive is produced to extract the package and scan file contents, rather than checking only tar entry names. Use the existing artifact path from steps.art.outputs.crate, search extracted contents for $GITHUB_WORKSPACE, $RUNNER_TEMP, and other relevant absolute build roots, and fail with an error when any are found. Retain the existing absolute-member-path check if appropriate.baml_language/crates/baml_project/src/client_codegen.rs-667-679 (1)
667-679: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not ship
neverasUnit.A user-authored
-> nevercurrently generates a normally returning API (()/None), breaking the declared contract. Addcg::Ty::Neverand map it per backend before relying on this bridge behavior.🤖 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/baml_project/src/client_codegen.rs` around lines 667 - 679, Update the codegen type model to add cg::Ty::Never, then map it to each backend’s non-returning type (Python typing.Never, TypeScript never, and Rust ::core::convert::Infallible). Remove Never from the defensive Unit conversion in the relevant client codegen conversion, preserving Unit only for genuine recovery sentinels and ensuring direct never return slots retain their declared contract.baml_language/sdks/rust/sdkgen_rust/src/lib.rs-357-365 (1)
357-365: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReserve
_inlinedbamland_runtimebefore emitting root symbols.A root class, enum, or namespace with either name collides with these internal modules and produces an uncompilable generated SDK. Add both names to the root collision-analysis reservation set and cover them with generator unit tests.
Would you like me to help implement the reservation and tests?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/sdkgen_rust/src/lib.rs` around lines 357 - 365, Update the root collision-analysis reservation set used by analyze.rs to reserve both _inlinedbaml and _runtime before root classes, enums, or namespaces are emitted. Ensure collision renaming preserves the internal module names and add generator unit tests covering root symbols with each reserved name..github/workflows/build2-python-sdk.reusable.yaml-33-57 (1)
33-57: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict the matrix job’s token to read-only access.
The job inherits the workflow’s default permissions despite only requiring checkout access.
Proposed fix
matrix: name: Compute target matrix runs-on: ubuntu-latest + permissions: + contents: read outputs:🤖 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 @.github/workflows/build2-python-sdk.reusable.yaml around lines 33 - 57, Add a job-level permissions declaration to the matrix job (the job containing the `matrix` identifier), granting only `contents: read` for checkout and leaving all other token permissions disabled.Source: Linters/SAST tools
baml_language/sdks/rust/bridge_rust/src/capi.rs-95-104 (1)
95-104: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAvoid blocking first-load on current-thread Tokio
load()still falls through to a synchronousload_inner(&loader::LoaderEnv::from_process())on current-thread runtimes, so the first library download/load blocks the executor thread. Usespawn_blocking/a dedicated blocking path here or expose an async initialization route for first use.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/bridge_rust/src/capi.rs` around lines 95 - 104, Update load() so first-use loading does not synchronously execute load_inner on a current-thread Tokio runtime. Add a spawn_blocking or equivalent dedicated blocking path for that runtime case, while preserving the existing block_in_place behavior for multi-thread runtimes and direct loading when no Tokio runtime is active.baml_language/crates/baml_release/src/platforms.rs-20-24 (1)
20-24: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject unknown platform-contract fields.
A typo such as
experimantalsilently becomesfalse, while a misspelled artifact key silently becomes unsupported. Add#[serde(deny_unknown_fields)]to every contract struct and test rejection of unknown keys.Proposed hardening
#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] pub struct Platforms {Apply the same attribute to
Platform,Artifacts,ToolchainArtifact,PythonArtifact,SdkArtifact, andCffiArtifact.Also applies to: 26-40, 42-52, 55-62, 65-78, 83-88, 91-106
🤖 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/baml_release/src/platforms.rs` around lines 20 - 24, Add #[serde(deny_unknown_fields)] to the contract structs Platforms, Platform, Artifacts, ToolchainArtifact, PythonArtifact, SdkArtifact, and CffiArtifact. Add deserialization tests confirming unknown platform and artifact keys are rejected, while preserving acceptance of all declared fields.baml_language/sdk_tests/crates/rust/llm_functions/customizable/replay_harness.rs-119-125 (1)
119-125: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not perform an unbounded join after best-effort shutdown.
If
post_shutdownfails or the server ignores it,Drophangs the entire test process forever. PollJoinHandle::is_finished()with the intended timeout and only join once finished; otherwise report and detach.🤖 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/rust/llm_functions/customizable/replay_harness.rs` around lines 119 - 125, Update the shutdown logic in the relevant Drop implementation to avoid unbounded thread.join after post_shutdown. Poll JoinHandle::is_finished() for the intended 10-second timeout, joining only when the thread finishes; if it remains running, report the timeout and detach the handle instead of blocking the test process.baml_language/sdk_tests/crates/rust/llm_functions/customizable/replay_harness.rs-103-132 (1)
103-132: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSerialize access to the process-global replay environment.
Parallel tests can overwrite these variables, then one guard can remove them while another server is still active. Hold a global lock for the guard’s lifetime and restore the previous values on drop.
🤖 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/rust/llm_functions/customizable/replay_harness.rs` around lines 103 - 132, Update the RunningServer lifecycle to acquire and retain a global lock covering the replay environment variables for the guard’s entire lifetime, preventing parallel instances from overlapping. Capture each variable’s prior value before set_var, then restore those values on Drop instead of unconditionally removing them; release the lock only after shutdown, cleanup, and environment restoration are complete.baml_language/sdks/rust/sdkgen_rust/src/emit/function.rs-68-72 (1)
68-72: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftHandle nullable multi-arm unions explicitly.
impl Into<Option<GeneratedUnion>>does not accept a bare arm throughFrom<Arm> for GeneratedUnion, soT | ... | nullparameters still reject the bare-arm call forms promised here. Add an explicit adapter for nullable unions and a compile-pass test for a bare-arm call site.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/rust/sdkgen_rust/src/emit/function.rs` around lines 68 - 72, Update the multi-arm union handling in the function parameter emission logic to detect nullable unions and adapt bare arm arguments into the generated optional union type, rather than requiring Into<Option<GeneratedUnion>> directly. Preserve the existing conversion path for non-nullable unions, and add a compile-pass test covering a bare-arm call to a T | ... | null parameter.
🟡 Minor comments (6)
baml_language/sdk_tests/crates/rust/function_calls/customizable/test_stdlib_entrypoints.rs-4-15 (1)
4-15: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReplace the always-passing intrinsic test with an enforceable assertion.
The empty test succeeds even if intrinsic entry-point bindings are accidentally generated, while
_generated_sdk_fileis never used. Add a generated-source or compile-fail assertion, or remove the test until it can enforce the contract.Also applies to: 34-43
🤖 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/rust/function_calls/customizable/test_stdlib_entrypoints.rs` around lines 4 - 15, The intrinsic entry-point test currently performs no assertion and leaves _generated_sdk_file unused. Replace the empty test around test_compiler_intrinsics_are_not_emitted_as_entry_points with an assertion that generated sources do not contain intrinsic entry-point bindings, handling absent files as expected; alternatively remove the test until an enforceable generated-source or compile-fail check can be added.baml_language/crates/bridge_ctypes/build.rs-38-54 (1)
38-54: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSort the proto inputs before generating the vendored Rust output. The four
baml_bridge.cffi.v1protos land in the same generated module, so an unstable traversal order can reshuffle the emitted file across filesystems.🤖 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_ctypes/build.rs` around lines 38 - 54, Sort the proto input paths before passing them to vendor_config.compile_protos in the vendored Rust generation flow. Ensure all baml_bridge.cffi.v1 inputs use a deterministic order so the generated module remains stable across filesystems, while preserving the existing output directory and formatting configuration.baml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_aliases.rs-13-29 (1)
13-29: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDo not report unsupported roundtrips as passing tests.
These empty
#[test]bodies pass and overstate recursive-alias coverage. Mark them#[ignore = "..."]or remove the test attributes until executable assertions exist.Proposed fix
#[test] +#[ignore = "recursive union aliases are not generated yet"] fn test_round_trip_rec_list() { @@ #[test] +#[ignore = "recursive union aliases are not generated yet"] fn test_round_trip_alias_container() {Also applies to: 31-45
🤖 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/rust/type_shapes/customizable/roundtrip_tests/test_aliases.rs` around lines 13 - 29, The empty test functions test_round_trip_rec_list and the similarly structured recursive-alias tests must not pass as successful coverage. Mark each unsupported round-trip test with an explanatory #[ignore = "..."] attribute, or remove its #[test] attribute until executable assertions and generated representations exist.baml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_forward_refs.rs-17-30 (1)
17-30: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMark unsupported roundtrips as ignored instead of passing empty tests.
These tests currently report success without exercising anything, making recursive-union support appear covered.
Proposed fix
#[test] +#[ignore = "recursive union aliases are not representable in Rust yet"] fn test_round_trip_rec_list() {#[test] +#[ignore = "recursive union aliases are not representable in Rust yet"] fn test_round_trip_rec_list_with_other() {Also applies to: 32-49
🤖 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/rust/type_shapes/customizable/roundtrip_tests/test_forward_refs.rs` around lines 17 - 30, Mark the unsupported recursive-union roundtrip tests, including test_round_trip_rec_list and the additional tests in the indicated range, with the Rust ignored-test attribute instead of commenting out their bodies. Preserve the existing explanatory divergence comments and intended implementations, while ensuring the tests are reported as ignored rather than passing without assertions.baml_language/sdk_tests/DEVELOPMENT.md-112-125 (1)
112-125: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the surrounding target-count and setup-breadcrumb documentation.
Adding Rust makes “For BOTH targets” and “Both targets’
build.rs” stale. The breadcrumb section should also documentSDK_TEST_RUST_SETUP=1.Also applies to: 145-150
🤖 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/DEVELOPMENT.md` around lines 112 - 125, Update the surrounding target-count documentation to account for Rust instead of referring to “For BOTH targets.” Revise the “Both targets’ build.rs” wording to accurately describe the applicable targets and setup behavior. In the breadcrumb/setup section, document the SDK_TEST_RUST_SETUP=1 environment variable alongside the existing setup controls.baml_language/sdk_tests/README.md-43-46 (1)
43-46: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAvoid using a gated test in the filtering example.
Both optional-argument modules are currently
Gate::Later, socargo test optional_argsruns no matching tests. Use an enabled test name, or showcargo test -- --listbefore applying the filter.🤖 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/README.md` around lines 43 - 46, Update the Rust filtering example in the SDK tests README to use an enabled test name rather than the gated optional_args test, or precede the filter with a cargo test -- --list example. Keep the existing nextest command unchanged and ensure the documented cargo test command actually matches runnable tests.
🧹 Nitpick comments (4)
baml_language/sdk_tests/crates/rust/function_calls/customizable/test_generic_inference.rs (1)
111-121: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDo not register documentation-only placeholders as passing tests.
These
#[test]functions execute no code, so regressions cannot fail them while test reports count them as coverage. Convert reachable compile-time cases to compile-fail/UI tests; otherwise retain the divergence notes without#[test].Also applies to: 230-238, 334-377, 385-395, 404-438, 447-491, 553-564, 698-711, 719-730, 778-811, 818-826, 834-839, 916-934
🤖 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/rust/function_calls/customizable/test_generic_inference.rs` around lines 111 - 121, Remove #[test] from documentation-only placeholder functions, including test_identity_unbound_generic_instance_round_trips and the additionally identified ranges, when their bodies execute no code. For reachable compile-time behavior, convert the cases into appropriate Rust compile-fail/UI tests; otherwise preserve the divergence comments as non-test documentation without registering them as passing tests.baml_language/sdk_tests/harness_setup/src/rust.rs (1)
73-75: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEnable Tokio’s
timefeature for the cancellation port.
test_cancellation.rsusestokio::time::{sleep, timeout}. Flipping its gate toNowwill fail compilation with the current generated manifest.Proposed fix
[dev-dependencies] -tokio = { version = "1", features = ["rt", "macros"] } +tokio = { version = "1", features = ["rt", "macros", "time"] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdk_tests/harness_setup/src/rust.rs` around lines 73 - 75, Update the MANIFEST_EXTRA dev-dependencies declaration to include Tokio’s time feature alongside the existing rt and macros features, so test_cancellation.rs can compile its tokio::time usage when the cancellation gate is set to Now.baml_language/sdks/rust/bridge_rust/src/encode.rs (1)
14-59: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd unit tests for these wire constructors.
Cover omitted versus explicit-null kwargs, class FQNs/fields, and enum wire values. These helpers can be tested directly without the integration harness.
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/sdks/rust/bridge_rust/src/encode.rs` around lines 14 - 59, Add focused Rust unit tests for the wire constructors kwargs, class, and enum_value. Verify kwargs omits None entries while preserving explicit-null values, class encodes the supplied FQN and fields, and enum_value encodes both the FQN and variant wire value; keep the tests direct and local without using the integration harness.Source: Coding guidelines
baml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_literals.rs (1)
18-68: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover invalid inputs for primitive-backed literal parameters.
Because these signatures expose
i64,String, andbool, callers can supply values outside the declared literal. Add assertions that values such asround_trip_literal42(41)and a non-"draft"string are rejected, preserving literal semantics at the runtime boundary.🤖 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/rust/type_shapes/customizable/roundtrip_tests/test_literals.rs` around lines 18 - 68, Extend the round-trip literal tests around round_trip_literal42 and round_trip_literal_draft to assert that invalid primitive values, such as 41 and a string other than "draft", return an error. Preserve the existing successful-input assertions and literal semantics for valid values.
🤖 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_engine/src/conversion.rs`:
- Around line 73-86: Update the conversion walk in conversion.rs to thread an
active identity-based set of HeapPtrs through recursive calls, detecting
re-entry before descending and returning an EngineError for cyclic values.
Remove each pointer from the active set after its branch completes so acyclic
shared references convert normally. Add unit coverage for both a
self-referential value producing an error and a repeated shared reference
converting successfully.
---
Outside diff comments:
In `@scripts/baml-language-version`:
- Around line 502-503: Update the Node package version check in the
checks-building logic to compare against the registry-specific npm_version used
during stamping, rather than expected.canonical_version. Preserve the existing
package file reading and validation behavior.
---
Major comments:
In @.github/workflows/build2-bridge-cffi.reusable.yaml:
- Around line 80-89: Update the “Compute target matrix” job to declare an
explicit read-only permissions block, granting only the repository contents
permission required by actions/checkout and leaving all other GITHUB_TOKEN
permissions unavailable.
In @.github/workflows/build2-python-sdk.reusable.yaml:
- Around line 33-57: Add a job-level permissions declaration to the matrix job
(the job containing the `matrix` identifier), granting only `contents: read` for
checkout and leaving all other token permissions disabled.
In @.github/workflows/verify-rust-sdk.reusable.yaml:
- Around line 254-257: Update the crate validation step after the archive is
produced to extract the package and scan file contents, rather than checking
only tar entry names. Use the existing artifact path from
steps.art.outputs.crate, search extracted contents for $GITHUB_WORKSPACE,
$RUNNER_TEMP, and other relevant absolute build roots, and fail with an error
when any are found. Retain the existing absolute-member-path check if
appropriate.
In `@baml_language/crates/baml_project/src/client_codegen.rs`:
- Around line 667-679: Update the codegen type model to add cg::Ty::Never, then
map it to each backend’s non-returning type (Python typing.Never, TypeScript
never, and Rust ::core::convert::Infallible). Remove Never from the defensive
Unit conversion in the relevant client codegen conversion, preserving Unit only
for genuine recovery sentinels and ensuring direct never return slots retain
their declared contract.
In `@baml_language/crates/baml_release/src/manifest.rs`:
- Around line 93-97: Update the SDK validation flow in the manifest validation
method to pass self.version into validate_sdk for every SDK entry. Extend
validate_sdk to enforce the registry-specific canonical version relationship,
including requiring crates_io SDK versions to match the manifest release
version, while preserving existing validation for other registries.
In `@baml_language/crates/baml_release/src/platforms.rs`:
- Around line 20-24: Add #[serde(deny_unknown_fields)] to the contract structs
Platforms, Platform, Artifacts, ToolchainArtifact, PythonArtifact, SdkArtifact,
and CffiArtifact. Add deserialization tests confirming unknown platform and
artifact keys are rejected, while preserving acceptance of all declared fields.
In
`@baml_language/sdk_tests/crates/rust/function_calls/customizable/test_errors.rs`:
- Around line 280-287: Update test_clean_exit_terminates_process_with_code so it
no longer passes without assertions: implement a subprocess harness that invokes
throws_test::DoExit with codes 0 and 7 and verifies each child
ExitStatus::code(), or explicitly mark the test ignored with a tracking reason
until the harness is available.
In
`@baml_language/sdk_tests/crates/rust/llm_functions/customizable/replay_harness.rs`:
- Around line 119-125: Update the shutdown logic in the relevant Drop
implementation to avoid unbounded thread.join after post_shutdown. Poll
JoinHandle::is_finished() for the intended 10-second timeout, joining only when
the thread finishes; if it remains running, report the timeout and detach the
handle instead of blocking the test process.
- Around line 103-132: Update the RunningServer lifecycle to acquire and retain
a global lock covering the replay environment variables for the guard’s entire
lifetime, preventing parallel instances from overlapping. Capture each
variable’s prior value before set_var, then restore those values on Drop instead
of unconditionally removing them; release the lock only after shutdown, cleanup,
and environment restoration are complete.
In `@baml_language/sdk_tests/crates/rust/llm_functions/customizable/test_main.rs`:
- Around line 196-223: The environment-dependent tests around
test_extract_resume_build_request_includes_openai_api_key and the sibling test
at the additional range must not mutate OPENAI_API_KEY concurrently. Serialize
both tests with a shared synchronization guard, capture each test’s prior
environment value, and restore it on completion, including when the variable was
previously unset.
In `@baml_language/sdks/rust/bridge_rust/src/baml_value.rs`:
- Around line 275-282: Update both map-decoding implementations around the
visible map entry conversion closures to reject entries whose entry.value is
absent instead of calling unwrap_or_default(). Return the existing decode error
type for missing values, while preserving K::from_baml_key and V::from_baml
handling for present values.
In `@baml_language/sdks/rust/bridge_rust/src/capi.rs`:
- Around line 95-104: Update load() so first-use loading does not synchronously
execute load_inner on a current-thread Tokio runtime. Add a spawn_blocking or
equivalent dedicated blocking path for that runtime case, while preserving the
existing block_in_place behavior for multi-thread runtimes and direct loading
when no Tokio runtime is active.
In `@baml_language/sdks/rust/bridge_rust/src/completion.rs`:
- Around line 89-134: Update the completion state machine used by
`Receiver::wait_blocking`, `Receiver::wait`, and the `trampoline` failure path
to add a terminal callback-failure state carrying `CompletionError`. Set this
state when the callback panics, notify the blocking condition variable and wake
any stored waker, and change both waiting methods to return `Result<Vec<u8>,
CompletionError>` while propagating either ready bytes or the failure.
In `@baml_language/sdks/rust/bridge_rust/src/decode.rs`:
- Around line 40-44: Update the Literal::FloatValue branch in the decode logic
to propagate or return the parse::<f64>() error instead of converting malformed
input into Out::StringValue(s). Preserve Out::FloatValue for successfully parsed
values and ensure invalid float literals fail decoding.
In `@baml_language/sdks/rust/bridge_rust/src/loader/download.rs`:
- Around line 134-147: Update the fallback in the download/install flow around
the atomic rename: remove the direct copy_file operation into dest_path. If the
destination already exists, treat it as a concurrent winner and continue
successfully; otherwise propagate the original rename failure as
LoaderError::DownloadFailed, preserving atomic installation and any valid cached
file.
- Around line 83-119: Update the download validation flow around the read loop
and checksum handling to reject an empty response before caching or
installation. Track whether any bytes were written, and return the existing
download failure error when the response contains zero bytes, including the
target filename or path in the message.
In `@baml_language/sdks/rust/bridge_rust/src/loader/mod.rs`:
- Around line 192-236: Update the library-path discovery logic to accept
candidates only when metadata().is_file() is true, not merely when metadata
succeeds. Apply this to the explicit path and environment path branches shown,
plus the cached candidate and system-path candidate handling near the related
loader logic, preserving download recovery when a cache path is not a regular
file.
In `@baml_language/sdks/rust/sdkgen_rust/src/analyze.rs`:
- Around line 480-491: Update the collision-handling logic around renamed_child
and renames.insert so it searches for an available suffix rather than always
appending one underscore. Ensure each generated child name is unused by both
sibling paths in types_in and existing emitted names in renames, while
preserving the current name when no collision exists.
In `@baml_language/sdks/rust/sdkgen_rust/src/emit/function.rs`:
- Around line 68-72: Update the multi-arm union handling in the function
parameter emission logic to detect nullable unions and adapt bare arm arguments
into the generated optional union type, rather than requiring
Into<Option<GeneratedUnion>> directly. Preserve the existing conversion path for
non-nullable unions, and add a compile-pass test covering a bare-arm call to a T
| ... | null parameter.
In `@baml_language/sdks/rust/sdkgen_rust/src/idents.rs`:
- Around line 52-58: Update non_raw_able and the related Rust
identifier-generation flow to use collision-free, injective escaping for
reserved names, including crate, self, Self, super, and _. Ensure escaped
outputs cannot collide with valid BAML names such as crate_, self_, Self_, or
_1, including when used as module paths.
In `@baml_language/sdks/rust/sdkgen_rust/src/lib.rs`:
- Around line 357-365: Update the root collision-analysis reservation set used
by analyze.rs to reserve both _inlinedbaml and _runtime before root classes,
enums, or namespaces are emitted. Ensure collision renaming preserves the
internal module names and add generator unit tests covering root symbols with
each reserved name.
In `@baml_language/sdks/rust/sdkgen_rust/src/unions.rs`:
- Around line 173-214: Update shape_error to reject unions containing multiple
list/container arms or multiple map arms when their empty encodings overlap,
preventing declared-order trial decoding from selecting an arbitrary variant.
Reuse the existing type/variant inspection helpers in shape_error and return a
descriptive shape error while preserving current validation for literals,
strings, and duplicate variant names.
- Around line 148-170: Update register_unions_in to match Ty::Class(_, args) and
recursively visit every generic argument, preserving the existing traversal for
unions, lists, and map values so nested unions such as Wrapper<int | string> are
registered.
In `@scripts/baml-language-version`:
- Around line 345-370: Tighten validation in the ReleasePlan loader before
constructing registry_versions: require registry_versions to contain exactly the
keys in PLAN_REGISTRIES, reject non-string keys or values instead of coercing
them, and require every registry version to equal the plan’s canonical_version.
Preserve the existing missing-field and schema validation errors while rejecting
unknown, null, or mismatched entries before ReleasePlan is returned.
---
Minor comments:
In `@baml_language/crates/bridge_ctypes/build.rs`:
- Around line 38-54: Sort the proto input paths before passing them to
vendor_config.compile_protos in the vendored Rust generation flow. Ensure all
baml_bridge.cffi.v1 inputs use a deterministic order so the generated module
remains stable across filesystems, while preserving the existing output
directory and formatting configuration.
In
`@baml_language/sdk_tests/crates/rust/function_calls/customizable/test_stdlib_entrypoints.rs`:
- Around line 4-15: The intrinsic entry-point test currently performs no
assertion and leaves _generated_sdk_file unused. Replace the empty test around
test_compiler_intrinsics_are_not_emitted_as_entry_points with an assertion that
generated sources do not contain intrinsic entry-point bindings, handling absent
files as expected; alternatively remove the test until an enforceable
generated-source or compile-fail check can be added.
In
`@baml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_aliases.rs`:
- Around line 13-29: The empty test functions test_round_trip_rec_list and the
similarly structured recursive-alias tests must not pass as successful coverage.
Mark each unsupported round-trip test with an explanatory #[ignore = "..."]
attribute, or remove its #[test] attribute until executable assertions and
generated representations exist.
In
`@baml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_forward_refs.rs`:
- Around line 17-30: Mark the unsupported recursive-union roundtrip tests,
including test_round_trip_rec_list and the additional tests in the indicated
range, with the Rust ignored-test attribute instead of commenting out their
bodies. Preserve the existing explanatory divergence comments and intended
implementations, while ensuring the tests are reported as ignored rather than
passing without assertions.
In `@baml_language/sdk_tests/DEVELOPMENT.md`:
- Around line 112-125: Update the surrounding target-count documentation to
account for Rust instead of referring to “For BOTH targets.” Revise the “Both
targets’ build.rs” wording to accurately describe the applicable targets and
setup behavior. In the breadcrumb/setup section, document the
SDK_TEST_RUST_SETUP=1 environment variable alongside the existing setup
controls.
In `@baml_language/sdk_tests/README.md`:
- Around line 43-46: Update the Rust filtering example in the SDK tests README
to use an enabled test name rather than the gated optional_args test, or precede
the filter with a cargo test -- --list example. Keep the existing nextest
command unchanged and ensure the documented cargo test command actually matches
runnable tests.
---
Nitpick comments:
In
`@baml_language/sdk_tests/crates/rust/function_calls/customizable/test_generic_inference.rs`:
- Around line 111-121: Remove #[test] from documentation-only placeholder
functions, including test_identity_unbound_generic_instance_round_trips and the
additionally identified ranges, when their bodies execute no code. For reachable
compile-time behavior, convert the cases into appropriate Rust compile-fail/UI
tests; otherwise preserve the divergence comments as non-test documentation
without registering them as passing tests.
In
`@baml_language/sdk_tests/crates/rust/type_shapes/customizable/roundtrip_tests/test_literals.rs`:
- Around line 18-68: Extend the round-trip literal tests around
round_trip_literal42 and round_trip_literal_draft to assert that invalid
primitive values, such as 41 and a string other than "draft", return an error.
Preserve the existing successful-input assertions and literal semantics for
valid values.
In `@baml_language/sdk_tests/harness_setup/src/rust.rs`:
- Around line 73-75: Update the MANIFEST_EXTRA dev-dependencies declaration to
include Tokio’s time feature alongside the existing rt and macros features, so
test_cancellation.rs can compile its tokio::time usage when the cancellation
gate is set to Now.
In `@baml_language/sdks/rust/bridge_rust/src/encode.rs`:
- Around line 14-59: Add focused Rust unit tests for the wire constructors
kwargs, class, and enum_value. Verify kwargs omits None entries while preserving
explicit-null values, class encodes the supplied FQN and fields, and enum_value
encodes both the FQN and variant wire value; keep the tests direct and local
without using the integration harness.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Also bumps size gate
|
pls let me merge first 🙏 |
…-8af9bbcb133a4d0a63bd6e5ffcc2f480db3ab0d9' into kai/rust-bridge
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Deployment failed with the following error: |
|
Deployment failed with the following error: |
- regenerate the Go and TypeScript proto bindings (the LITE_RUNTIME option changed every descriptor; Rust/Python were already refreshed) - drop the stale platform-contract drift test in baml_release::manifest: it read the pre-#4045 platforms.json schema, and canary's baml_release::platforms module now owns the same check against the new schema - cap cmake build parallelism in the harness driver and bridge smoke (bare -j is unbounded with Makefiles; eight concurrent fixture builds starved the CI runner to death) and embed the vendored pb sources gzipped (build.rs): baml-cli carries every generator, and 1.9 MB of plain protobuf text blew the 3% size gate; compressed it is +150 KB (+2.0% measured, back under the gate)
2 participants
Has support for basic BAML features up to type aliases an unions. Dynamically loads the cffi dylib and publishes to crates.io as
baml_bridgeSummary by CodeRabbit
New Features
rustoutput support and expanded SDK runtime/test coverage across many data shapes and end-to-end behaviors.baml_bridgeruntime for Rust SDKs, including engine loading/caching, download with checksum verification, and structured error handling.Release & CI