Repository navigation
fix(vm): serialize enum variants as their name in union/optional json fields (B-728) - #3954
Conversation
…rter (B-728) A class field whose declared type is a union containing an enum — including the optional enum `E?` (== `E | null`) — serialized to `null` through the typed `baml.json.to_string<T>` walker. `ty_value_to_serde`'s `RuntimeTy::Union` arm delegates to the untyped `value_to_serde`, which mapped every `Object::Variant` to `null` instead of its variant name. Render an enum variant as its variant-name string in `value_to_serde`, matching the typed (`ty_value_to_serde`) and structural (`render_to_serde`) walkers, so union/optional enum fields serialize as the variant name. Adds regression tests covering enum and string-literal-union class fields through both the structural `.to_json()` path and the typed `to_string<T>` path.
|
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):
|
|
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 (2)
📝 WalkthroughWalkthroughThis change fixes JSON serialization of ChangesEnum Variant Serialization Fix
Estimated code review effort: 2 (Simple) | ~10 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 |
B-728 — enum & string-literal-union fields fail
to_json()/--output-format jsonWhat was wrong
A class field whose declared type is a union containing an enum — most commonly the optional enum
E?(which desugars toE | null), but alsoE | F— serialized tonullwhen rendered through the typed serializerbaml.json.to_string<T>:Root cause
ty_value_to_serde'sRuntimeTy::Unionarm dispatches structurally on the runtime value by delegating to the untyped convertervalue_to_serde. That converter lumpedObject::Variantinto the catch-all arm that returnsserde_json::Value::Null, so any enum variant reaching it (via a union/optional field) was dropped tonull.The structural walker
render_to_serde(which backs.to_json()/baml.json.fromand--output-format jsonviabaml.json.serialize) already renders variants correctly — this is why bareenumfields and"X" | "Y"string-literal-union fields serialize fine. The gap was only in the untyped converter used by the typedto_string<T>path and theRuntimeTy::Union/Void/TypeAliasdelegations.Fix
Render an enum variant as its variant-name string in
value_to_serde, matching the typed (ty_value_to_serde) and structural (render_to_serde) walkers. This is the minimal change to the value→JSON conversion; it only affects the previously-nullvariant case, so no existing behavior changes.Tests
Adds four regression tests in
to_json_interface.rs:enum_field_serializes_as_variant_name—class C { e E }via.to_json()→{"e":"A"}string_literal_union_field_serializes_as_string—class D { f "X" | "Y" }→{"f":"X"}optional_enum_field_serializes_via_typed_to_string—E?field viato_string<T>→{"e":"A"}(fails onmain, produced{"e":null})enum_union_field_serializes_via_typed_to_string—E | Ffield viato_string<T>→{"v":"Y"}(fails onmain)Local CI
cargo nextest run --all-features -E 'not package(baml_tests) and not package(/^sdk_test_/) and not binary(=pack_e2e)'→ 4245 passedcargo test -p baml_tests→ 0 failedcargo fmt --checkclean ·cargo clippy --all-targets --all-features -- -D warningscleanSummary by CodeRabbit