Repository navigation
feat(sdk-rust): optional serde impls on generated types - #5375
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Oct 22, 2026
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: adding optional Serde implementations to generated Rust types. |
| Linked Issues check | |
The changes satisfy the coding requirements in [#4969]. The generated Rust manifest adds an optional serde dependency with derive support. Generated eligible classes and enums receive feature-gate… |
| Out of Scope Changes check | |
The changes remain within [#4969]. Generator analysis and emission implement optional serialization for generated Rust types. The bridge feature update, shared JSON fixture, Rust harness updates, and … |
| Docstring Coverage | |
Docstring coverage is 82.76% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 9 files. (1 skipped: 1 … |
Full details: Linked Issues check
-
Explanation
The changes satisfy the coding requirements in [
#4969]. The generated Rust manifest adds an optionalserdedependency withderivesupport. Generated eligible classes and enums receive feature-gatedSerializeandDeserializederives. Field and variant renames preserve BAML JSON names. Eligible unions receive feature-gated implementations with declaration-order matching. Analysis excludes unsupported types and documents the blocking field. The added unit, interoperability, and fixture tests cover the feature, JSON round trips, names, optional fields, unions, and unsupported types.
Full details: Out of Scope Changes check
-
Explanation
The changes remain within [
#4969]. Generator analysis and emission implement optional serialization for generated Rust types. The bridge feature update, shared JSON fixture, Rust harness updates, and setup updates support or validate that implementation. The reported changes do not establish unrelated product behavior.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 82.76% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 9 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.
A rabbit checks each field with care
Then tests the names the types declare
JSON hops out and makes a round
Two readers match the shapes they found
All features join the test-day play
Comment @coderabbitai help to get the list of available commands.
rossirpaulo
force-pushed
the
paulo/rust-serde-derives
branch
from
October 22, 2026 20:08
4ffa5bb to
199382d
Compare
⏭️ 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 failed❌ 2 violations · ✅ 1 passed
Details & how to fixViolations:
Add/update baselines:
[artifacts.bridge_wasm]
file_bytes = 26678038
gzip_bytes = 7816354
[artifacts.packed-program]
file_bytes = 40720888
gzip_bytes = 16571892Generated by |
rossirpaulo
marked this pull request as ready for review
October 22, 2026 20:53
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-merge-queue
Bot
removed this pull request from the merge queue because a pull request earlier in the stack was removed
Oct 23, 2026
Generated Rust types get `serde::Serialize` and `serde::Deserialize` behind an opt-in `serde` feature on the generated crate, restoring what the old engine's Rust generator offered (#3118). Fixes #4969. The JSON matches BAML's own `baml.json`: class keys are the BAML field names (renamed where the Rust name differs, as for `r#type` and `crate_`), an enum is its variant name, and a union is untagged, deserializing to the first arm that fits in declaration order. Union impls are written out and buffer through `serde_json::Value`, because serde's untagged buffer cannot read floats under serde_json's `arbitrary_precision`, which `baml_bridge` enables. Classes holding a `bigint`, `uint8array`, function or handle, and BAML standard-library classes (several serialize through `baml.ToJson` overrides), get no impls; their docs name the field. The Rust sdk-test harness now lints and tests fixture crates with `--all-features`, and a new `type_shapes` namespace checks serde_json against `baml.json.to_string` and `baml.json.from_string` end to end. Co-Authored-By: Claude Opus 5.5 <[email protected]>
`baml_bridge::Json` is `serde_json::Value`, whose serde form is the JSON value itself. The serde fixpoint instead judged the stdlib `json` alias by its expansion, which has a `bigint` arm, so under the `serde` feature a class with a `json` field compiled without serde impls. Treat the canonical `baml.json.json` alias as having serde impls. An end-to-end case checks that a class with `json`, `json?` and `json[]` fields serializes to the same text as `baml.json.to_string`, including `i64::MIN` and `u64::MAX`, and that BAML and serde read each other's output back. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The generated union impls buffer a value as `serde_json::Value`, which needed an optional `serde_json` dependency on the generated crate. `baml_bridge` re-exports the `serde_json` its `Json` type comes from, so use that and drop the extra dependency: the `serde` feature now enables only `serde`. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
rossirpaulo
force-pushed
the
paulo/rust-serde-derives
branch
from
October 23, 2026 01:17
d81fc96 to
a45b316
Compare
meefs
pushed a commit
to meefs/baml
that referenced
this pull request
Oct 23, 2026
…aryML#5374) **Stack:** bottom of three. BoundaryML#5287 (`json` → `serde_json::Value`) is stacked on this PR, and BoundaryML#5375 (optional serde impls) on BoundaryML#5287. This one targets `canary` directly. ## Bug The Rust generator boxes a class field when the field sits on a containment cycle, but its cycle analysis ignored type aliases. It assumed alias references were inlined before codegen. They are not: they stay nominal in the symbol pool. So a cycle through an alias was never boxed: ```baml class Text { text string } class Extra { part Part } type Part = Text | Extra ``` `baml generate` emits `pub part: crate::Part`, and the generated crate does not compile: ``` error[E0072]: recursive types `Extra` and `TextOrExtra` have infinite size ``` Nothing in the current fixtures hits it. ## Fix Emitted type aliases become nodes of the existing containment graph. Each one gets edges to whatever its target holds inline. The SCC pass and `needs_box` then treat an alias reference like a class reference. That covers a direct field, an optional field, a union arm and alias chains: ```rust pub struct Extra { pub part: ::std::boxed::Box<crate::Part>, pub maybe: ::std::option::Option<::std::boxed::Box<crate::Part>>, pub alt: ::std::boxed::Box<crate::PartOrString>, // `Part | string` pub outer: ::std::boxed::Box<crate::Outer>, // `type Outer = Part` pub parts: ::std::vec::Vec<crate::Part>, // a Vec already breaks the cycle } pub struct Holder { pub part: crate::Part } // not on the cycle: no box ``` Production diff: +50/−32 in `sdkgen_rust` (`analyze.rs`, `translate_ty.rs`, one doc line in `emit/type_alias.rs`). Most of the removed lines are comments that described the old "aliases are inlined" assumption. ## Tests - New `recursion_through_a_type_alias_boxes_the_alias_reference` in `sdkgen_rust/src/lib.rs`. With `canary`'s generator it fails (the `Extra` fields come out unboxed); with the fix it passes. - `cargo test -p sdkgen_rust`: 63 passed. - `cargo nextest run -p sdk_test_rust`: 17/17 passed. That run generates, rustfmts, clippies and tests every Rust fixture crate. - End to end: the project above, generated with `baml-cli` and checked with `cargo check` and `cargo clippy -D warnings`. It fails with E0072 on `canary` and is clean with the fix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Rust SDK generation for recursive type aliases, including aliases referenced through class fields, nullable fields, and unions. Generated references are now boxed where needed to support recursive structures while leaving references outside the cycle unboxed. Generation no longer reports warnings for the covered recursive alias patterns. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5.5 <[email protected]>
meefs
pushed a commit
to meefs/baml
that referenced
this pull request
Oct 23, 2026
…oundaryML#5287) **Stack:** middle of three. Stacked on BoundaryML#5374 (the alias-cycle fix, number assigned when pushed); merge that first, then retarget this PR to `canary`. BoundaryML#5375 (optional serde impls) is stacked on this one. ## Bug The Rust SDK generator silently drops everything that touches BAML's `json` type. With ```baml class Event { id: string, payload: json, } function echo(event: Event) -> Event { event } function count(items: json[]) -> int { items.length() } ``` all three symbols are skipped for Rust. TypeScript, Python and Go emit them. ``` warning: skipped `user.Event`: references skipped or unknown type `baml.json.json` warning: skipped `user.count`: argument `items`: references skipped or unknown type `baml.json.json` warning: skipped `user.echo`: return: references skipped or unknown type `user.Event` ``` The warnings only show with `baml generate -F display_all_warnings`. Projects whose types contain free-form JSON (OpenAPI `{}` schemas, `additionalProperties`, opaque payloads) lose most of their Rust surface. **Cause.** `json` is the recursive stdlib alias `null | bool | int | bigint | float | string | json[] | map<string, json>`. `sdkgen_rust` emits aliases as Rust `type` items, which cannot be recursive, so `analyze.rs` skips every recursive alias, and the fixpoint then skips every class, alias and function that references one. ## Fix Special-case the canonical alias by its exact FQN (`BAML_JSON_JSON`, as the Go generator does) and map it onto `serde_json::Value`. - **Bridge (`baml_bridge`):** - `baml_bridge::Json` (= `serde_json::Value`), with conversions in both directions and `baml_ty()` naming the `baml.json.json` alias. - `serde_json` is re-exported, so generated crates and their users need no new dependency (`baml_bridge::serde_json::json!`). - New `DecodeError::NotJson` (`DecodeError` is `#[non_exhaustive]`). - **Generator (`sdkgen_rust`):** - `baml.json.json` translates to `::baml_bridge::Json` everywhere: fields, arguments, returns, containers and generic arguments. - The alias item is emitted as `pub type json = ::baml_bridge::Json;`, and user aliases of it resolve to the same type. - Look-alike user recursive aliases keep the existing skip. - **`json?` is `Option<Json>`**, following the generator's uniform `T?` → `Option<T>` rule. BAML null decodes as `None`. - **Unions.** A bare `json` arm next to other arms (`json | Foo`) overlaps them on the wire, so such unions still skip (fail closed). Container arms (`json[] | string`) can be told apart and are emitted. - **Clippy allow.** Emitting `json` makes the OpenAI vendor client representable. Its verbatim docstring trips `clippy::doc_lazy_continuation`, which failed the SDK suite's `clippy -D warnings` gate, so the lint is allowed at the generated crate root next to the existing naming allowances. ### Numbers BAML `int` is 63-bit (the VM keeps a tag bit), so its range is `[-2^62, 2^62 - 1]`, and the engine rejects a wider host `int`. A json integer outside that range is therefore sent as a BAML `bigint`, which is the arm `baml.json.parse` gives the same token. A `bigint` coming back decodes when it fits `i64` or `u64`. Every `i64` and `u64` round-trips, encoding cannot fail, and no bigint type appears in the Rust API. | Rust value sent | BAML sees | Comes back as | | --- | --- | --- | | `json!(42)` | `int` | `42` | | `json!(4611686018427387904)` (2^62) | `bigint` | same (`i64`) | | `json!(i64::MIN)` | `bigint` | same | | `json!(u64::MAX)` | `bigint` | same (`u64`) | | (BAML returns `(2n).pow(64n)` in a `json`) | `bigint` | `Error::Decode(NotJson)`: "baml.json.json value has no serde_json form: an integer outside the i64/u64 range" | | (BAML returns `float.nan()` in a `json`) | `float` | `Error::Decode(NotJson)`: "... a non-finite float" | Floats are finite `f64`, and an integral float stays a float. Objects follow `serde_json::Map` order. Decoding also fails with `NotJson` on a value that is not JSON at all (class, enum, media, bytes, handle). Only when some crate in the build enables `serde_json`'s `arbitrary_precision` can a `Number` hold more than `i64`/`u64`/`f64`. Then a wider integer is sent exactly as a `bigint`, and a float beyond `f64`'s range (`1e400`) is sent as ±infinity, which is how `str::parse::<f64>` reads it. Both are documented on `Json`. ### `serde_json` features (BoundaryML#4527) Today `baml_bridge` turns on `serde_json`'s `arbitrary_precision` and `preserve_order` in every downstream crate; BoundaryML#4527 stops that. This PR neither needs nor adds either feature, and its tests pass with them on (current `canary`) and off (`canary` + BoundaryML#4527). The bridge names its `serde_json` in one place (`pub use serde_json;` in `lib.rs`; `json.rs` imports through `crate::serde_json`), so whichever of the two PRs lands second changes that one line. ## Tests - `cargo test -p sdkgen_rust`: 69 passed. Five of the six new tests fail on the base branch's generator: canonical json in fields and signatures with no skips, no alias item needed in the pool, a user alias of `json`, a bare-json union failing closed, and `json[] | string` being emitted. The sixth pins that a look-alike recursive alias still skips. - `cargo nextest run -p baml_bridge --all-features`: 100 passed. - Unit tests: nested round trips; BAML int bounds; `2^62`, `i64::MIN`, `i64::MAX` and `u64::MAX` sent as `bigint` and decoded back exactly; rejection of out-of-range bigint, invalid hex, NaN/±inf, class, media and bytes; literal envelopes; `Option<Json>`; the `arbitrary_precision`-only numbers; and `CallOptions::argument(.., json!({"id": u64::MAX}))`, which panicked on the previous revision of this PR. - Live engine tests: round trips at the top level, in lists, maps and classes, as `json?`, as generic `T = json` and as `json[] | string`, all with full-width integers; BAML-side narrowing (a host `2^62` matches `let b: bigint`); `baml.json.parse` of `i64::MAX`/`i64::MIN`; a BAML bigint `2^64 - 1` decoding to `u64::MAX`; `2^70` and NaN failing to decode. - `cargo nextest run -p sdk_test_rust --all-features`: 17/17 passed. `function_calls/test_json.rs`, previously gated on this projection, is now enabled. It covers the Go suite's canonical cases plus full-width integers as arguments and as host-callback return values. - `cargo fmt --check` with the repo's import options, and `cargo clippy --all-targets --all-features -- -D warnings` on `baml_bridge`, `sdkgen_rust`, `sdk_test_codegen` and `sdk_test_rust`: clean. ## Open questions for reviewers 1. **`1e400` under `arbitrary_precision`** becomes ±infinity rather than an error. Encoding is infallible (`to_baml` has no error channel), so the alternative is an error side channel just for this case. 2. **`json?` as `Option<Json>`**, versus collapsing it to `Json` the way Go does (`any`, not `*any`). 3. **Unions with a bare `json` arm** stay skipped. The engine keeps `baml.json.json` intact in a union's `self_type` and sends the selected index, so `json | SomeClass` looks supportable later. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **New Features** * Added JSON value support to the Rust SDK, including nested values, optional nulls, and JSON fields in generated types. * Made `serde_json` available through the SDK for constructing JSON values without a separate dependency. * **Bug Fixes** * Improved error messages when BAML values cannot be represented as JSON. * **Tests** * Expanded coverage for JSON round trips, numeric boundaries, unions, callbacks, and decoding errors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5.5 <[email protected]>
This branch was successfully 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.
Stack: top of three. Stacked on #5287 (
json→serde_json::Value), which is stacked on #5374. Merge those first, then retarget this PR tocanary.Closes #4969.
The old engine's Rust generator gave generated types serde impls (#3118). The new generator's types don't have them, so a Rust host can't persist or replay a typed result without writing its own mapping code. This PR restores that, behind an opt-in
serdefeature on the generated crate:The feature adds only a
serdedependency (withderive); union impls use theserde_jsonthatbaml_bridgealready re-exports. Without the feature, the serde attributes and impls compile away.Shape
The JSON matches BAML's own
baml.json(to_stringandfrom_string):Serialize/Deserialize. Keys are the BAML field names, so a field whose Rust name differs (r#type,crate_) gets#[serde(rename = "...")].Noneserializes asnull. A missing optional field reads asNone, and unknown keys are ignored.jsonfields arebaml_bridge::Json(serde_json::Value, from fix(sdk-rust): map json to serde_json::Value instead of skipping it #5287), so they serialize as the JSON itself, including full-widthi64/u64integers.Union impls are written out instead of using
#[serde(untagged)].baml_bridgeturns onserde_json/arbitrary_precision(until #4527 lands), and with that feature serde's untagged buffer cannot read floats (1.5, or a class arm with a float field). The impls buffer throughserde_json::Valueinstead, which works with the feature on or off.Not covered
These types get no impls, and the generated docs name the field that blocks it:
bigint(num-bigint's serde form is[sign, digits], not BAML's JSON number);uint8array(BAML has no JSON form for it);baml.ToJsonoverride (baml.time.PlainDateis a date string).The impls are structural, so a
baml.ToJsonoverride on a user class is not applied. The symbol pool doesn't recordimplementsyet. This is noted in the generatedCargo.toml.Tests
sdkgen_rustunit tests (74 passed) cover the manifest feature, the class, enum and union shapes, classes and unions holdingjson, and the types that get no impls.type_shapesfixture namespace,ns_json_interchange, adds genericto_json<T>/from_json<T>wrappers aroundbaml.json. Seven Rust end-to-end tests compare serde_json and BAML on the same values and the same text:null), enums, unions, maps, lists, nested classes and keyword-named fields;json,json?andjson[]fields, includingi64::MINandu64::MAX;clippyandcargo teston each fixture crate with--all-features, so every generated fixture type compiles with the feature on and is linted with-D warnings.cargo nextest run -p sdk_test_rust: 17/17 passed.type_shapessuites pass with it (10/10), and the Java, C++, Swift and TypeScript-web generators emit it without errors.🤖 Generated with Claude Code
Summary by CodeRabbit