fix(engine): materialize host-supplied json with json container typing - #4346
Merged
Conversation
Inbound containers from the language bridges (Go, Python, TypeScript)
and `--json-args` carry no element-type annotation on the wire; the
decoder defaults them to a scalar union. A `baml.json.json`-declared
slot then materialized VM maps/lists with that synthesized type, so
runtime type tests — `match (j) { let m: map<string, json> => ... }`,
and therefore `baml.json.path` / `path_or` — failed on host-supplied
objects while succeeding on BAML-born `baml.json.parse` values, whose
containers carry the `json` alias itself.
Re-annotate JSON container trees with the declared alias in the inbound
argument coercion pass, and again at VM conversion for the paths that
convert without coercion (notably host-callable return values). Trees
outside the JSON algebra are left unchanged for the existing rejection
paths.
Regression coverage: engine unit test for the re-annotation, fixture
functions exercising match narrowing and path selectors over
host-supplied json (including a host-callback return), and Go + Python
SDK tests driving them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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):
|
Binary size checks passed✅ 7 passed
Generated by |
The new canonical test IDs exist only in the Go and Python suites (the fix is engine-side and bridge-agnostic); annotate them with SDK_PARITY_LINT(skip) so absent environments are waived by the ratchet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughHost-supplied JSON values now receive canonical recursive annotations during conversion. Validation distinguishes the builtin ChangesCanonical JSON narrowing
Descriptor-aware Java host calls
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 747-755: Use the fully qualified canonical JSON alias name, not
TypeName::display_name(), in the conversion check around conversion.rs:747-755
and the corresponding check around conversion.rs:3632-3640; update the
alias-expansion exclusion around conversion.rs:3463-3465 to use the same
full-name comparison. Ensure user aliases such as user.baml.json.json retain
normal alias behavior while only the built-in baml.json.json alias receives JSON
container handling.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 69a8a288-f564-4fbf-b744-baeb98fbeaff
📒 Files selected for processing (4)
baml_language/crates/bex_engine/src/conversion.rsbaml_language/sdk_tests/crates/go/function_calls/customizable/test_json_test.gobaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_json.pybaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_go_json_tests/main.baml
…dentity Two follow-on gaps surfaced by porting the host-json narrowing tests to every SDK bridge: Swift and C++ encode json with sparse inbound `value_type` annotations (Swift on scalar leaves, C++ on a return's selected variant alternative). `value_satisfies_json` rejected any Union carrier, so annotated trees never received json container typing, and `validate_host_return`'s annotation branch rejected container-annotated json returns outright (the context-free subtype check cannot expand the alias), panicking with HostContractViolation. Peel inbound-annotation carriers whose annotation stays within the JSON algebra (new helpers runtime_ty_within_json_algebra and expected_admits_json_alias); genuine union carriers and out-of-algebra annotations (bigint, classes) stay rejected. Per review: json alias identity is now the fully qualified name — package `baml`, namespace `json`, name `json` — via the shared `is_canonical_json_alias` predicate instead of `display_name()` string comparison, which a local alias at namespace path `baml.json` could collide with. All alias checks in bex_engine/bex_external_types switched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extend the canonical IDs host_supplied_json_supports_typed_narrowing and json_returned_from_host_callback_supports_typed_narrowing from Go+Python to TypeScript (node + web), Rust, Java, Swift, and C++ — each mirroring the same semantics: json_kind narrowing on object/array/string/scalar, nested path selectors, path_or fallback, missing-field JsonPathError, and a host callback returning a json object. The Rust port is checked in but gated off (registered in TEST_MODS as Gate::Later) because sdkgen_rust has no projection for the canonical baml.json.json alias yet; it follows the suite's established pattern for ports awaiting codegen support. C# declares no function_calls suite (its native coverage is Rust-wrapped integration tests), so the two Python declarations carry the SDK_PARITY_LINT(skip) waiver naming that; parity lint: required gaps 4565 -> 4565, present declarations +48, both IDs present in all nine trackable environments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/bex_engine/src/conversion.rs (1)
748-756: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle canonical JSON in union return types.
A raw JSON container can validate against a callback return type such as
json | null. This branch only annotates whenexpected_tyis directlyRuntimeTy::TypeAlias. With a union, conversion keeps the container's inboundunknownelement or value type. Typed narrowing then fails for an accepted callback result.Run the existing union selection and coercion path before materialization, or preserve the selected canonical JSON member as the container context. Add a callback regression for
json?and a union containingjson.🤖 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/bex_engine/src/conversion.rs` around lines 748 - 756, The canonical JSON annotation in the conversion path only handles a direct RuntimeTy::TypeAlias and fails for union return types such as json? or json | null. Update the conversion flow around expected_ty and union selection so accepted raw JSON containers are coerced or annotated using the selected canonical JSON member before materialization, preserving typed container narrowing. Add callback regressions covering nullable json and a union containing json.
🤖 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.
Outside diff comments:
In `@baml_language/crates/bex_engine/src/conversion.rs`:
- Around line 748-756: The canonical JSON annotation in the conversion path only
handles a direct RuntimeTy::TypeAlias and fails for union return types such as
json? or json | null. Update the conversion flow around expected_ty and union
selection so accepted raw JSON containers are coerced or annotated using the
selected canonical JSON member before materialization, preserving typed
container narrowing. Add callback regressions covering nullable json and a union
containing json.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 83743cdb-b1ed-40e3-a8c4-08631d785360
📒 Files selected for processing (11)
baml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_external_types/src/host_return.rsbaml_language/crates/bex_external_types/src/lib.rsbaml_language/sdk_tests/crates/cpp/function_calls/customizable/tests/test_json.ccbaml_language/sdk_tests/crates/go/function_calls/customizable/test_json_test.gobaml_language/sdk_tests/crates/java/function_calls/customizable/TestJson.javabaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_json.pybaml_language/sdk_tests/crates/rust/function_calls/customizable/test_json.rsbaml_language/sdk_tests/crates/swift/function_calls/customizable/TestJson.swiftbaml_language/sdk_tests/crates/typescript/function_calls/customizable/json.test.tsbaml_language/sdk_tests/harness_setup/src/rust.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_json.py
- baml_language/sdk_tests/crates/go/function_calls/customizable/test_json_test.go
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oding The generated Java binding types a host callback parameter by its declared BAML type (e.g. `Function<baml.json.json, baml.json.json>`), but the bridge dispatched decoded RAW host values into it and encoded returns generically — a typed lambda's erased cast threw ClassCastException, forcing tests into unchecked laundering. Codegen now wraps callable arguments in `BamlTypedCallable`, a carrier pairing the callable with descriptors for its declared positional and optional parameters and return type. The dispatch path threads those descriptors through the same `decodeWithDesc` machinery the outbound result path already uses, so callback parameters materialize as their generated sealed-union/class types and returns encode against the declared type on both the sync and awaited-future paths. Slots whose types are entirely wire-driven fall back to the raw callable unchanged. TestJson's callback test now uses the properly typed lambda with no unchecked casts. sdkgen_java: 94/94; Java function_calls: green (pre-existing TestCancellation timing flake aside); full Java suite: 9/10 targets green (same flake); baml_bridge Gradle unit tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Problem
A
json-typed BAML parameter receiving an object from a host SDK materializes as a map that fails BAML's typed narrowing:match (j) { let m: map<string, json> => ... }never matches, sobaml.json.path/path_orthrow/return-default ("field access '.type' on a non-object") on host-supplied objects — while the structural$rust_functionsbaml.json.fieldandstringifywork on the same value. Pure-BAML json objects (frombaml.json.parse) match fine, so inbound FFI json diverged from BAML-born json.Reproducible without any SDK:
returned
"other"where the same value built viabaml.json.parsereturned"object".Root cause
All bridges (Go
container.go, Pythonproto.py, TypeScriptproto.ts) encode containers without avalue_typeannotation, and the wire decoder (bridge_ctypes::value_decode) defaults them to a scalar union (int|float|string|bool|uint8array|null). The engine's inbound coercion keepsbaml.json.jsonnominal and had no container arm for it, so a map bound to ajsonslot materialized on the VM heap with that scalar-union value type. BAML-bornbaml.json.parsemaps carry thejsonalias itself (serde_to_value), and thematchtype test (is_subtype, invariant in map value position) only matches the latter.Fix (engine-side, covers every bridge + CLI)
In
bex_engine::conversion:baml.json.jsonslot that inhabits the JSON algebra (value_satisfies_json) gets its container annotations rewritten recursively to the alias — lists becomejson[], mapsmap<string, json>. Non-JSON trees are left untouched for the existing rejection paths.convert_external_to_vm_value_with_ty, because host-callable return values convert without a coercion pass and had the identical bug.Also fixes json nested inside declared
map<string, json>/json[]params, class fields, andjson | imageunions.Tests
canonical_json_alias_reannotates_untyped_inbound_containers(deep re-annotation; non-JSON trees untouched).ns_go_json_tests):json_kind(match narrowing),json_path_string,json_path_string_or,json_callback_kind(host-return path)..nested.list[1].deep,path_orfallback, missing-fieldJsonPathError, callback-returned json.Verified: full
bex_enginesuite, Gofunction_callssuite, Pythonfunction_callssuite (ruff + pyright + pytest),cargo fmt/clippyclean.🤖 Generated with Claude Code
Note
Medium Risk
Touches core FFI value coercion, type matching, and host-return validation across all bridges; behavior change for json typing is intentional but could affect edge cases (user aliases shadowing display names, annotated unions).
Overview
Host-supplied
baml.json.jsonvalues from SDK bridges no longer fail BAML typed narrowing (matchonmap<string, json>,baml.json.path/path_or) because containers were materializing with wire-synthesized scalar-union element types instead of the canonicaljsonalias.The engine now re-annotates JSON-algebra containers at inbound coercion and on the host-return conversion path via
annotate_json_container_types, and tightens matching/validation:is_canonical_json_alias(package-qualified identity, notdisplay_name()),value_satisfies_jsonpeeling sparse inbound leaf annotations, and host-return acceptance for algebra-scoped inbound annotations (e.g. C++/Swift).Java bridge: codegen wraps callable arguments in
BamlTypedCallableso dispatch decodes/encodes against declared param and return descriptors (not raw wire values).Tests: engine unit tests, shared BAML fixtures (
json_kind, path helpers, callback), and cross-language SDK parity tests (Go extended; new C++/Java/Python/Swift/TS; Rust gated).Reviewed by Cursor Bugbot for commit 0cc59f8. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
New Features
Tests