Repository navigation
fix(sdk-rust): box class cycles that pass through type aliases - #5374
Merged
Merged
Conversation
Alias references stay nominal in the symbol pool, but the containment
analysis assumed they were inlined and gave them no edges. A cycle
through an alias, such as
type Part = Text | Extra
class Extra { part Part }
emitted `pub part: crate::Part`, and the generated crate failed to
compile with E0072 (recursive type has infinite size).
Make emitted aliases nodes of the containment graph, with edges to what
their target holds inline. An alias reference on a cycle (directly, as
an optional, or as a union arm) is then boxed like a class reference.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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 identifies the main change: boxing class cycles that pass through type aliases in the Rust SDK generator. |
| Docstring Coverage | |
Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ 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 the alias trail,
Through SCC loops and paths that wind.
The boxes guard recursive links,
While lists keep their elements light.
No warning rustles in the output.
The rabbit hops back, pleased to find.
Comment @coderabbitai help to get the list of available commands.
This was referenced Oct 22, 2026
⏭️ 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 = 26478742
gzip_bytes = 7756788
[artifacts.packed-program]
file_bytes = 40444040
gzip_bytes = 16459965Generated by |
rossirpaulo
marked this pull request as ready for review
October 22, 2026 19:45
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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]>
meefs
pushed a commit
to meefs/baml
that referenced
this pull request
Oct 23, 2026
) **Stack:** top of three. Stacked on BoundaryML#5287 (`json` → `serde_json::Value`), which is stacked on BoundaryML#5374. Merge those first, then retarget this PR to `canary`. Closes BoundaryML#4969. The old engine's Rust generator gave generated types serde impls (BoundaryML#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 `serde` feature on the generated crate: ```toml baml_sdk = { path = "baml_sdk", features = ["serde"] } ``` The feature adds only a `serde` dependency (with `derive`); union impls use the `serde_json` that `baml_bridge` already re-exports. Without the feature, the serde attributes and impls compile away. ## Shape The JSON matches BAML's own `baml.json` (`to_string` and `from_string`): - **Classes** derive `Serialize`/`Deserialize`. Keys are the BAML field names, so a field whose Rust name differs (`r#type`, `crate_`) gets `#[serde(rename = "...")]`. `None` serializes as `null`. A missing optional field reads as `None`, and unknown keys are ignored. - **Enums** serialize as the variant name. - **Unions** are untagged. Deserializing tries the arms in declaration order and takes the first that fits, as BAML does. A string-literal arm is its string. - **`json` fields** are `baml_bridge::Json` (`serde_json::Value`, from BoundaryML#5287), so they serialize as the JSON itself, including full-width `i64`/`u64` integers. ```rust #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] pub struct Label { pub text: ::std::string::String, pub note: ::std::option::Option<::std::string::String>, } ``` Union impls are written out instead of using `#[serde(untagged)]`. `baml_bridge` turns on `serde_json/arbitrary_precision` (until BoundaryML#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 through `serde_json::Value` instead, 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: - classes holding a `bigint` (num-bigint's serde form is `[sign, digits]`, not BAML's JSON number); - classes holding a `uint8array` (BAML has no JSON form for it); - classes holding functions or handles; - BAML standard-library classes, because several of them serialize through a `baml.ToJson` override (`baml.time.PlainDate` is a date string). The impls are structural, so a `baml.ToJson` override on a user class is not applied. The symbol pool doesn't record `implements` yet. This is noted in the generated `Cargo.toml`. ## Tests - `sdkgen_rust` unit tests (74 passed) cover the manifest feature, the class, enum and union shapes, classes and unions holding `json`, and the types that get no impls. - A new `type_shapes` fixture namespace, `ns_json_interchange`, adds generic `to_json<T>` / `from_json<T>` wrappers around `baml.json`. Seven Rust end-to-end tests compare serde_json and BAML on the same values and the same text: - classes, optional fields (absent and `null`), enums, unions, maps, lists, nested classes and keyword-named fields; - a class with `json`, `json?` and `json[]` fields, including `i64::MIN` and `u64::MAX`; - BAML reads serde's output back, and serde reads BAML's; - inputs that BAML rejects are rejected by both. - The Rust sdk-test harness now runs `clippy` and `cargo test` on 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. - The new fixture namespace is shared: the Go, Python and TypeScript `type_shapes` suites pass with it (10/10), and the Java, C++, Swift and TypeScript-web generators emit it without errors. 🤖 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** * Generated Rust SDK types now support optional Serde serialization and deserialization for compatible classes, enums, and unions, using their BAML field and variant names. * Compatible types can be used with BAML JSON interchange, including nested values, optional fields, and JSON fields. * **Bug Fixes** * Union deserialization now checks matching variants in declaration order. * **Tests** * Rust SDK checks now include all features and cover JSON interchange, serialization, and invalid input handling. <!-- 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: bottom of three. #5287 (
json→serde_json::Value) is stacked on this PR, and #5375 (optional serde impls) on #5287. This one targetscanarydirectly.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 generateemitspub part: crate::Part, and the generated crate does not compile: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_boxthen treat an alias reference like a class reference. That covers a direct field, an optional field, a union arm and alias chains:Production diff: +50/−32 in
sdkgen_rust(analyze.rs,translate_ty.rs, one doc line inemit/type_alias.rs). Most of the removed lines are comments that described the old "aliases are inlined" assumption.Tests
recursion_through_a_type_alias_boxes_the_alias_referenceinsdkgen_rust/src/lib.rs. Withcanary's generator it fails (theExtrafields 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.baml-cliand checked withcargo checkandcargo clippy -D warnings. It fails with E0072 oncanaryand is clean with the fix.🤖 Generated with Claude Code
Summary by CodeRabbit