Native json type: alias, parse/stringify, auto-derived to_json/from_json#3484
Merged
Conversation
Adds the recursive `json` type alias and the `baml.json` namespace foundation for BEP-038. By the end of this phase a class field of type `json` compiles, top-level `let j: json = ...` type-checks for each union arm, and `match (j: json)` is exhaustive. - New stdlib file `ns_json/json.baml` defining `type json = null | bool | int | float | string | json[] | map<string, json>` plus `JsonParseError`, `JsonDecodeError`, `JsonSerializationError`. - Register `ns_json/json.baml` in the builtin manifest. - Lower the `json` keyword to a path to the FQN `baml.json.json`, which the existing TIR resolution path turns into `Ty::TypeAlias`. - Centralize the alias FQN as `BAML_JSON_JSON` in baml_base.
Wires `baml.json.parse`, `baml.json.stringify`, and `baml.json.stringify_pretty` as `$rust_function` natives through the existing namespace-function pathway (no MIR intrinsic dispatch — these are plain unary calls with no compile-time orchestration, same shape as `baml.array.push` / `baml.string.length`). - New `bex_vm/src/package_baml/json.rs` implementing the three handlers on the auto-generated `BamlNamespaceJson` trait. `parse` invokes `bex_sap::parse` against the `baml.json.json` alias and wraps SAP errors into a thrown `JsonParseError`. `stringify` / `stringify_pretty` round-trip the runtime `Value` through `serde_json`. - Add `VmRustFnError::Thrown(Value)` so native handlers can raise BAML-level errors without going through `Result<_, RustError>`. - Mark `baml.json.parse` as fallible in the codegen so the `Result` return shape is preserved at the trait boundary. - HIR builder: accept single-segment uppercase class names in builtin `throws` clauses so `throws JsonParseError` resolves against the same-file class declaration. - Add 8 integration tests in `tests/json_parse_stringify.rs` covering parse/stringify roundtrip, int/float disambiguation, garbage-input throw, and pretty-print output.
Two surgical alias-FQN sentinels make the recursive `baml.json.json` alias a
usable LLM-function return type without provider strict-mode breakage and
without divergent traversal:
- `sys_llm/types/output_format.rs`: render the literal "Respond with valid
JSON." for `Ty::TypeAlias("baml.json.json")` and emit no auto-prefix.
- `sys_llm/lib.rs`: `walk_ty` skips the alias for the recursive-alias
schema-collection pass so it doesn't diverge on the `json[]` / `map<string,
json>` arms.
Also includes a Phase 2 follow-up: HIR builder accepts single-segment
`baml.json.*` error class names in stdlib `throws` clauses, mirroring the
existing `baml.errors.*` exemption.
Test coverage:
- `json_llm_return_type` compile fixture (`function ExtractAny() -> json`).
- Inline tests in `output_format.rs` covering the sentinel + non-json alias
fallback + explicit-prefix override.
- `bex_engine/tests/llm_render.rs`: end-to-end render of the alias prompt body.
- `bex_sap/tests/test_streaming.rs`: streaming behaviour for json-typed return.
- Refresh `bytecode_format` snapshots that lagged behind Phase 2's globals.
…reflection
Two new native fns in the `baml.json` namespace serialize and deserialize
arbitrary BAML values driven by their runtime `Ty`. The type-arg `T`
travels through the BEP-039 channel:
- `Instruction::Call` saves/restores `pending_call_type_args` around the
callee invocation so native handlers see the explicit type-args even
when no bytecode frame is pushed.
- `BexVm::current_call_type_args()` exposes the slot to native code.
`bex_vm_types::ClassField` gains `field_template: TyTemplate` next to
the existing erased `field_type: Ty`; emit populates it from the TIR
field type using the class's `generic_params`, so a `Container<T>::item`
field recovers its concrete binding via
`field_template.substitute(instance.class_type_args)`. The runtime
walkers (to_string / from_string) use the substituted template; per
BEP-038 they ignore `@alias` and `@skip` (those decorators stay
LLM-path-only).
Bug fixed along the way: `lower_multi_segment_path_as_field_chain` now
substitutes the receiver's class type-args when chaining through fields
so `b.value.name` (where `b: Box<User>`) emits `load_field .0` instead
of falling back to the dynamic map-key form. Visible in the
`generic_field_chain` snapshot diff.
`BexVm::resolved_class_names` also folds in enum HeapPtrs so
`from_string<Color>("\"Blue\"")` can find the enum at runtime.
Tests: 27 runtime tests in `baml_tests/tests/json_to_from_string.rs`
(round-trip, generic forwarding, three-level, closure capture, enum,
optional, recursive, media tagged-form, alias/skip raw-name semantics,
json passthrough, and error paths) plus four compile-time fixture
projects exercising LoadType + ntypeargs in the MIR.
Foundation for BEP-038's `to_json` / `from_json` protocol on user classes. This commit lands the synthesizer machinery; per-field method dispatch (needed for nested-override honoring) follows in a subsequent commit once primitive companion classes (`Int`, `Float`, `Bool`, `Null`) gain `to_json` so `self.f.to_json()` can bottom out. Changes ------- - New `FunctionOrigin::AutoDerive` variant in both `baml_compiler2_ast::ast::FunctionOrigin` and `bex_vm_types::types::FunctionOrigin`, plus the AST→runtime mapping in `baml_compiler2_emit`. - New `baml_compiler2_ast::auto_derive_json` module that appends synthesized `to_json` / `from_json` methods to every user class. Skips synthesis if the user already declared either method (BEP "user override wins" rule). Current bodies are temporary wrappers around the existing `baml.json.to_string<Self>` / `from_string<Self>` intrinsics — pure-AST per-field synthesis is the next iteration. - Hook in `lower_cst::lower_class` after companion expansion. - `baml_tests::engine::display_user_functions_with_options` filters auto-derived methods from the default bytecode display so unrelated class tests don't see snapshot bloat. New `show_auto_derive: <bool>` named field on the `baml_test!` macro opts in. Tests ----- - `simple_class_to_json_roundtrip` — `User.from_json(u.to_json())` round-trips. - `auto_derive_filtered_from_bytecode_by_default` — default snapshots stay clean. - `auto_derive_visible_with_show_auto_derive_flag` — opt-in flag exposes them. - `user_to_json_override_suppresses_auto_derive` — user override wins, both methods skipped. All prior JSON suites (`json_to_from_string`, `json_alias`, `json_parse_stringify`) and `classes` tests pass with no regressions.
Add `Int`, `Float`, `Bool`, `Null` companion classes (mirroring `String`),
each with a `to_json(self) -> baml.json.json` method. Add `to_json` to
existing `String`, `Uint8Array`, and the four media classes.
This unblocks per-field `self.<f>.to_json()` dispatch in the upcoming
auto-derive synthesizer (Phase 5b.5). `Uint8Array.to_json` unconditionally
throws per BEP-038 §"non-representable"; media `to_json` returns the
tagged-object form `{ kind, source, value, mime }`.
Codegen update: `receiver_input_type_with_vm_usage` switches media-class
receivers from `&view::media::Cls<'_>` to `&Value` for `//baml:mut_vm`
methods, avoiding the split-borrow between `vm.as_instance` and `&mut BexVm`.
… to_json Extend `PrimitiveType::builtin_class_path` for `Int`/`Float`/`Bool`/`Null`/ `String`, and add bridging arms in `resolve_member` / `try_resolve_member_on_ty` so `(42).to_json()`, `3.14.to_json()`, etc. resolve to the companion classes added in 5b.1. Also covers `Ty::Literal(...)` since `let n: int = 42` stores a literal type. Add universal `to_json` / `from_json` arms on `Ty::TypeVar` (typecheck-only): `x.to_json()` where `x: T` no longer fails with `[E0007] type 'T' has no member 'to_json'`. Throws clause is conservatively widened to `JsonSerializationError | JsonParseError`. MIR-side dispatch wiring remains for Phase 5b.4. Helpers `json_alias_ty()`, `json_serialization_error_ty()`, `json_parse_error_ty()`, etc. inlined in `builder.rs`.
…verride-honoring Both containers gain `to_json(self) -> baml.json.json` declared as `$rust_function` with `//baml:mut_vm //baml:may_yield` in `containers.baml`. `Array<T>.to_json` (native): `ToJsonContinuation` state machine in `bex_vm/src/package_baml/array.rs` dispatches `v.to_json()` per element via `YieldToCall`, accumulating into a json array. (Note: the plan suggested a BAML-level `self.map(...)` body; native impl is behaviorally equivalent and avoids relying on the typecheck-only `Ty::TypeVar` arm at runtime.) `Map<K,V>.to_json` (native): `MapToJsonContinuation` in `bex_vm/src/package_baml/map.rs`. Map keys are guaranteed `Value::String` at runtime (regardless of declared K — see plan Pre-Implementation Findings #2), so each entry pairs the key directly with the dispatched value's `to_json()` result. Shared helper `make_to_json_callee` in `package_baml/mod.rs` performs runtime-value-based dispatch — including class-instance FQN lookup so user `to_json` overrides on element types are honored. New tests: `array_of_int_to_json_returns_json_array`, `array_of_class_to_json_honors_override` (B's override → [99, 99]), `map_of_int_to_json_returns_json_map`.
Replace the Phase 5a wrapper body in `auto_derive_json::build_to_json_body`
with the BEP-038-compliant per-field map literal:
{ "f1": self.f1.to_json(), "f2": self.f2?.to_json(), ... }
Nullable fields (`T?` / union-with-null) use `OptionalMemberAccess` →
`Call` → `OptionalChain`. Fields with unsafe types (`$rust_type`,
function types, `unknown`, `error`) trigger a per-class fallback to the
old wrapper body via `build_to_json_wrapper_body`.
This unlocks user `to_json` override-honoring on nested class fields,
arrays of class, and maps of class — every `self.<f>.to_json()`
dispatches through the runtime method table, and combined with Phase
5b.4's container handlers, overrides bubble up correctly.
Supporting fixes:
- `throws_analysis.rs`: pass `unwrap_optional = true` for
`OptionalMemberAccess` callees so `self.f?.to_json()` doesn't trip the
"throws contract violation: missing unknown" check.
- `builder.rs` (TIR): enum `to_json` resolves to a `Ty::Function` with
`ret = json`, `throws = never`. Added a `Ty::Union` arm in
`instantiated_callee_throws` that folds throws across union-of-functions
callees (avoids spurious `Ty::Unknown` injection when calling a method
that resolves to a union-typed function — e.g. union-typed field with
`to_json` on every variant). Also added a `Ty::Union` fold in
`check_call_inner` that treats a union-of-functions as callable.
- `check_throws_contract` gains `warn_extraneous` bool; auto-derived
methods pass `false` to silence the extraneous-throws warning, since
their throws clause is conservatively declared.
- `testing/registry.baml`: explicit `to_json` declarations on
`TestRegistration` and `TestSetRegistration` (their function-typed
fields aren't AST-detectable as unsafe).
`from_json` body keeps the wrapper (deferred per plan §"What We're NOT
Doing").
…honoring + tests
Add `baml.json.to_json<T>(v: T) -> json` free function (native impl using
`make_to_json_callee` + `YieldToCall` pass-through continuation). It does
runtime dispatch on the value's actual type, so overrides defined on the
inner class are honored regardless of the static `T`.
Auto-derive synthesizer (`auto_derive_json.rs::build_to_json_body`): for a
field whose declared type is a bare class generic parameter (e.g. `value: T`
in `class Box<T>`), emit `baml.json.to_json(self.<f>)` instead of the method
form `self.<f>.to_json()`. The method form fails at MIR/runtime — the
receiver type erases to `Ty::Void` before MIR, `lower_member_access` finds
no class match, and the lowering falls through to a generic map-element load
that panics on primitive/instance values. Concrete-typed fields keep the
direct method-dispatch form (cheaper).
This drops the Phase 5b.5 workaround that fell back the entire class to the
wrapper body for TypeVar fields — that workaround broke BEP-038's
override-honoring contract across generic boundaries.
Phase 5b.6 tests added in `json_auto_derive.rs`:
- `primitive_field_to_json` — `class C { x int }` → `{"x":42}`
- `array_of_primitive_to_json` — `class C { xs int[] }` → `{"xs":[1,2,3]}`
- `nested_class_override_honored_direct` — A.to_json honors B's user override
- `nested_class_override_honored_in_array` — override applied per element
- `nested_class_override_honored_in_map` — override applied per map value
- `recursive_class_to_json` — Tree with children Tree[]
- `generic_class_to_json_concrete` — `Box<int>{value:42}.to_json()`
- `generic_class_honors_override_through_typevar` — `Box<Secret>` where
Secret has a user `to_json`; the override wins through the TypeVar
boundary (the BEP-038 contract).
Snapshots regenerated for fixtures whose synthesized bodies now mention
`baml.json.to_json` (generic_field_chain, json_to_from_string_composite_generic,
diagnostic_errors/generics, baml_std auto-derive on generic builtins).
…honoring
Replace the Phase 5a wrapper-style `from_json` with a BEP-038 per-field
class-instance constructor mirroring Phase 5b's `to_json` design. Every
field routes through `baml.json.from_json<F>(baml.json.field(j, "f"))`,
centralizing override dispatch in the native handler so user `from_json`
overrides on nested classes (including through `T[]`, `map<string, T>`,
`T?`, and TypeVar boundaries) are honored.
Stdlib (`baml/ns_json/json.baml`):
- `field(j, key) -> json` extracts a map-arm field by name (returns null
if `j` isn't a map or the key is absent).
- `from_json<T>(j) -> T` is the central type-driven dispatcher.
Native (`bex_vm/src/package_baml/json.rs`):
- `field` reads the underlying `Object::Map`.
- `json_from_json_dispatch`:
- Class T → look up `<fqn>.from_json`, YieldToCall with the class's
instance type-args (so `Box<Secret>.from_json` sees `T = Secret`).
- List/Map → continuation trampoline that walks elements/values, yields
to user `<fqn>.from_json` per item to honor override semantics
(mirrors 5b.4.2's `Map.to_json` trampoline).
- Optional → null pass-through then recurse on inner.
- Primitives/enum/alias/media → structural decode via Phase 5a's
`ty_serde_to_value`.
`NativeCallResult::YieldToCall` extension: added `type_args` field so
native helpers can dispatch a generic class' static method with the
right type-arg substitution. Mirrors the `Call` instruction's BEP-039
type-arg channel; both yield paths in `vm.rs` set `pending_call_type_args`
and seed `frame.type_args` consistently.
Synthesizer (`auto_derive_json.rs`):
- `build_from_json_body` now emits `Self<T1,...> { f: from_json<F>(...) }`
per field (with the wrapper-body fallback retained for unsafe types).
- `maybe_synthesize_json_methods` synthesizes `to_json` and `from_json`
independently — defining only one no longer suppresses the other.
Parser/lower (`lower_expr_body.rs`):
- Added `find_callee_generic_args` so `Box<Secret>.from_json(j)` threads
the receiver type's `<Secret>` as the call's type-args. The parser
previously parsed it but `lower_call_expr` only looked at direct
GENERIC_ARGS children of the callee.
TIR (`builder.rs`, `infer_context.rs`, `inference.rs`):
- `resolve_explicit_type_args` includes the enclosing class's generic
params for `UnboundMethod`/`Free` static-method calls, so
`Box<Secret>.from_json(j)` arity-checks against `[T]` (class params)
rather than the method's empty params.
- `is_auto_derived_body` flag on `TypeInferenceBuilder` toggles a
suppress flag on `InferContext` that gates `UnresolvedMember` /
`UnresolvedType` / `UnresolvedName` / `NotCallable` diagnostics
emitted from synthesized references to user types/fields. Auto-derive
also skips `check_throws_contract` entirely (the synthesized clause is
best-effort wide and the user can't fix it).
- `resolve_member`'s `Ty::TypeAlias` arm now calls `expand_alias_chains`
(which has a 64-iteration cap) instead of recursing, so cyclic aliases
like `type One = Two; type Two = One;` no longer stack-overflow when
reached via field method dispatch.
Client codegen (`baml_project/src/client_codegen.rs`):
- Skip `FunctionOrigin::AutoDerive` methods when collecting class
static/instance methods so SDK bindings don't surface them.
Tests (`baml_tests/tests/json_auto_derive.rs`): 8 new Phase 5c tests
mirroring the to_json side — `primitive_field_from_json`,
`array_of_primitive_from_json`, `nested_class_override_honored_direct_from_json`,
`nested_class_override_honored_in_array_from_json`,
`nested_class_override_honored_in_map_from_json`,
`recursive_class_from_json`, `generic_class_from_json_concrete`,
`generic_class_honors_override_through_typevar_from_json`.
Snapshot churn across HIR/MIR/TIR/codegen/diagnostic fixtures from the
new per-field body shape and the loosened diagnostic gating.
Closes the gaps identified in the test-audit pass. 10 new tests covering
shapes the original Phase 5c suite missed:
- `optional_class_override_honored_from_json_non_null` /
`optional_class_override_honored_from_json_null` — `class A { b: B? }`
with B's user `from_json`. Exercises the native `Optional` arm's
null short-circuit and the non-null recurse-on-inner path.
- `optional_primitive_field_from_json` — `class C { x: int? }` with both
`null` and concrete payloads, structural-decode through Optional.
- `map_of_primitive_from_json` — `class C { lookup: map<string, int> }`
exercises the no-yield branch of the Map walker (primitive values
decode synchronously without trampoline).
- `top_level_from_json_call_concrete_class` /
`top_level_from_json_call_primitive` — direct `baml.json.from_json<T>`
call from main, not via auto-derive.
- `independent_synthesis_only_from_json_defined` /
`independent_synthesis_only_to_json_defined` — assert the loosened
Phase 5c rule that synthesizes `to_json` and `from_json` independently:
defining one doesn't suppress the other.
- `round_trip_with_overrides_both_directions` — end-to-end
`to_json → stringify → parse → from_json` with user overrides on both
sides; verifies the two contracts compose.
- `from_json_wrapper_body_fallback` — class with `unknown`-typed field
trips `class_is_safe_for_per_field_synthesis = false`, so the
synthesizer emits the wrapper body
(`baml.json.from_string<Self>(stringify(j))`).
…static calls Multi-segment static-method calls (`root.pkg.inner.Box<int>.from_json(j)`) were misclassified as bound instance-method calls. The `base_is_value` heuristic in `Expr::MemberAccess` (`builder.rs:1671`) only treated single-segment `Path` bases as potential type names; multi-segment paths fell into the catch-all `_ => true` branch and resolved as `BoundMethod`. `resolve_explicit_type_args` then skipped the class-params lookup, so the receiver's `<int>` was validated against `from_json`'s 0 method-level type-params, producing E0005 "function `from_json` expects 0 type argument(s), got 1". Fix: extend the heuristic to all `Path` bases (`!segments.is_empty()`). For `root.pkg.inner.Box`, `root` is not a local, so the access becomes `UnboundMethod` and the receiver's `<int>` correctly fills the class's generic-param slot. Also adds the previously-uncovered cases flagged in the Phase 5c audit: - `tests/json_auto_derive.rs`: positive runtime tests for alias-chain dispatch through `to_json` / `from_json` (`Outer = Inner = B` chain), exercising the `Ty::TypeAlias` arm of `resolve_member`. - `projects/diagnostic_errors/json_static_method_arity/`: snapshot pinning the existing `Box<int, string>.from_json(j)` arity error and documenting that `Box.from_json(j)` (missing args) is currently NOT an error — the type-checker infers `T` from the surrounding return type. - `projects/compiles/json_cross_namespace_static_call/`: cross-namespace fixture with both user-defined `Box<T>.from_json` and auto-derived `AutoBox<T>.from_json`, called via `root.pkg.inner.<Class><int>.from_json(j)`. Regression test for the bug above.
…attern-syntax changes Rebase fixups required after replaying json's 13 commits onto canary (post reflect.type_of merge + Sam's pattern-system overhaul). Two real runtime bugs surfaced once json's auto-derive code ran against canary's compact-bytecode dispatch: 1. VM PC overrun in callers of native functions whose `YieldToCall` callback was a synchronously-Done native (no further yield, no bytecode frame pushed). The outer Native continuation frame was left on the stack but `*frame_idx` wasn't advanced past the caller, so the inner dispatch loop's `frame_idx == orig_frame_idx` "no change" branch let it keep stepping the caller's bytecode past `Return`. Triggered by `[1,2,3].to_json()` because Array's native YieldToCalls per element to `json.to_json<int>` (Native → Done). Fix: advance `*frame_idx = self.frames.len() - 1` after the recursive `execute_call_from_locals_offset` returns from the Native YieldToCall arm, so the inner loop breaks out and exec_compact's continuation handler runs. 2. `baml.json.from_json: missing type argument` from native handlers that read `pending_call_type_args`. The compact `Call` opcode dispatch popped explicit type args off the stack and seeded the *new bytecode frame's* `type_args`, but never stashed them in `pending_call_type_args` for native callees to read via `current_call_type_args()`. The Native YieldToCall path already does this dance — Call had to mirror it. Other carry-over fixes: - `auto_derive_json.rs`: add `docstring: None` to synthesized `to_json`/`from_json` `FunctionDef`s (canary's #3472 added the field). - `outline.rs`: skip `FunctionOrigin::AutoDerive` methods from the LSP file outline; they have no real source span, so `describe_item_member` would fail when slicing source text. - `compiler2_mir/mod.rs::generic_class_destructure_field_*`: scope the "no projected field through void local" assertion to `user.f`'s body — auto-derived `to_json` / `from_json` on `Box<T>` legitimately have void locals (T isn't instantiated in the auto-derive body). - `json_alias_basic.baml`, `json_parse_stringify_intrinsics.baml`, `json_alias.rs`, `json_to_from_string.rs`, `json_auto_derive.rs`, `json_parse_stringify.rs`: rewrite old typed-binding match patterns (`x: int =>`) to canary's required `let x: int =>` form per Sam's pattern-system overhaul. Snapshots regenerated via `cargo insta accept`. Remaining failure: rig integration tests' `pyright` check on generated Python — the recursive `baml.json.json` `TypeAliasType` alias trips `reportInvalidTypeForm`. Known codegen issue, will be addressed by special-casing `json` to emit `Any` in a follow-up.
Pyright reports `reportInvalidTypeForm` on the generated recursive
`json` alias because the inner forward-ref strings inside
`TypeAliasType("json", typing.Union[..., typing.List["json"], ...])`
aren't fully resolved at parse-checking time, even though pydantic
walks them later.
Special-case the stdlib `baml.json.json` alias in `render_type_alias`
to emit `json: typing.TypeAlias = typing.Any` instead. Precise enough
for the surfaces user code touches (parse / stringify / decoded
JSON shapes), and unblocks the rig pyright integration tests.
TODO marker left on the special-case for the eventual recursive
representation once pyright handles `TypeAliasType` forward-refs (or
once we tighten the codegen surface).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds a recursive ChangesJSON Type Support and Auto-Derivation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (12)
baml_language/crates/baml_compiler2_ast/src/lower_type_expr.rs (1)
395-399: ⚡ Quick winUse the shared JSON sentinel constant instead of hardcoding segments.
The new mapping is correct, but reusing the canonical
baml.json.jsonconstant here would avoid cross-phase drift if the sentinel ever changes.🤖 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_compiler2_ast/src/lower_type_expr.rs` around lines 395 - 399, Replace the hardcoded segments vec![Name::new("baml"), Name::new("json"), Name::new("json")] used to build the TypeExpr::Path with the shared canonical sentinel for "baml.json.json" (the module-level constant that represents the JSON sentinel) to avoid duplication; import or reference that sentinel constant where the mapping into TypeExpr::Path occurs (the block constructing TypeExpr::Path for the "json" case) and use it to populate the segments field instead of calling Name::new(...) three times.baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs (1)
33-61: ⚡ Quick winAdd focused unit tests for call-site generic-arg extraction.
This helper is parser-shape-sensitive; a few unit tests for direct
<T>, receiver-carried<T>, and method-level-over-receiver precedence would lock behavior and prevent subtle regressions.As per coding guidelines
**/*.rs: Prefer writing Rust unit tests over integration tests where possible.Also applies to: 1561-1564
🤖 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_compiler2_ast/src/lower_expr_body.rs` around lines 33 - 61, Add focused Rust unit tests in the same module as find_callee_generic_args to exercise parser shapes: (1) a direct call-site case where GENERIC_ARGS is a child of the callee PATH_EXPR (e.g. foo<T>(...)) and assert find_callee_generic_args returns that GENERIC_ARGS node; (2) a receiver-carried case where the call is a FIELD_ACCESS_EXPR or OPTIONAL_FIELD_ACCESS_EXPR whose base PATH_EXPR has GENERIC_ARGS (e.g. Box<T>.method(...)) and assert it returns the base's GENERIC_ARGS; and (3) a precedence case where both receiver and method-level GENERIC_ARGS exist (e.g. Container<int>.method<U>(...)) and assert the direct child GENERIC_ARGS (method-level) is returned. Use the same parsing helpers used elsewhere in this file to build a SyntaxNode from snippets and compare node.kind() == SyntaxKind::GENERIC_ARGS or Option::is_some/is_none as appropriate so the tests lock the intended behavior for find_callee_generic_args.baml_language/crates/baml_builtins2/baml_std/baml/null.baml (1)
1-5: ⚡ Quick winPattern is intentional — Null.to_json correctly omits
//baml:mut_vm.Verification confirms the absence is by design: all scalar primitives (
Null,Float,Int,Bool) consistently omit the annotation, while heap-allocated types (String,Uint8Array,Array<T>,Map<K,V>) include it. This reflects the distinction between immediate-valued JSON variants and heap-allocated JSON objects. No VM correctness issue exists.Consider adding a brief comment at the class level explaining this pattern for future maintainers, e.g.:
// Null produces an immediate JSON value; no //baml:mut_vm needed class Null { ... }🤖 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_builtins2/baml_std/baml/null.baml` around lines 1 - 5, Add a brief explanatory comment above the class Null to document why Null.to_json omits the //baml:mut_vm annotation — note that Null (and other scalar primitives like Float, Int, Bool) produce immediate JSON values and thus do not require mutating the VM, whereas heap-allocated types (String, Uint8Array, Array<T>, Map<K,V>) include //baml:mut_vm; place the comment adjacent to the class Null declaration so future maintainers see the intent when inspecting Null.to_json.baml_language/crates/baml_compiler2_tir/src/inference.rs (1)
451-454: 💤 Low value
is_auto_deriveis computed twice — capture it once.
builder.set_auto_derived(...)at Line 452 andlet is_auto_derive = matches!(...)at Line 567 evaluate the exact samematches!predicate. If the skip condition is ever extended (e.g., additional origins that should also skip the contract check), the two sites can silently diverge.♻️ Proposed refactor
- builder.set_auto_derived(matches!( - func_data.origin, - ast::FunctionOrigin::AutoDerive - )); + let is_auto_derive = + matches!(func_data.origin, ast::FunctionOrigin::AutoDerive); + builder.set_auto_derived(is_auto_derive); // ... (existing body inference) ... - let is_auto_derive = - matches!(func_data.origin, ast::FunctionOrigin::AutoDerive); if !is_auto_derive { builder.check_throws_contract(Also applies to: 561-577
🤖 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_compiler2_tir/src/inference.rs` around lines 451 - 454, The same predicate matches!(func_data.origin, ast::FunctionOrigin::AutoDerive) is evaluated twice; compute it once into a local variable (e.g., let is_auto_derive = ...) and reuse that variable both when calling builder.set_auto_derived(...) and in the subsequent skip/contract-check logic (the block that currently recomputes matches! around func_data.origin). Update all places that currently recompute that predicate (including the skip condition in the later contract-check code) to reference is_auto_derive so future changes to the skip criteria remain consistent.baml_language/crates/baml_compiler2_hir/src/builder.rs (1)
1508-1529: 💤 Low value
is_builtin_class_refsubsumes uppercase generic params, bypassing the declared-params check.For a single-segment uppercase name like
T,E, orResult:
is_builtin_class_refistrue(single segment, no generic args, uppercase first char).is_allowed_genericisfalse(gated by!is_builtin_class_ref).- The final
if !is_builtin_error && !is_builtin_class_ref && !is_allowed_genericpasses becauseis_builtin_class_refistrue.This means an undeclared type variable (e.g.,
throws UndeclaredError) in a builtin function passes phase-1 validation via the class-ref path, bypassing theallowed_generic_paramsguard. Conversely, the intent ofis_allowed_genericwas to restrict generic params to those actually declared on the function. TIR will catch a missing class at name-resolution time, so this isn't a correctness hole — but it does weaken the phase-1 contract for builtin authors.If the intent is that
is_builtin_class_refis the deliberate safety valve (relying on TIR for full validation), the logic is fine; just worth a comment documenting this trade-off.🤖 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_compiler2_hir/src/builder.rs` around lines 1508 - 1529, The bug is that is_builtin_class_ref currently subsumes single-segment uppercase generic names and prevents the declared-params check from ever succeeding; to fix, compute is_allowed_generic independently (remove the && !is_builtin_class_ref guard so allowed_generic_params.iter().any(...) can be true even if is_builtin_class_ref is true) and then change the final guard to explicitly allow either a builtin class or a declared generic: replace the existing if !is_builtin_error && !is_builtin_class_ref && !is_allowed_generic check with if !is_builtin_error && !(is_builtin_class_ref || is_allowed_generic), keeping the existing definitions of segments, generic_args, is_builtin_error, is_builtin_class_ref, and allowed_generic_params so declared generic params take precedence for validation.baml_language/crates/baml_tests/projects/compiles/json_cross_namespace_static_call/main.baml (1)
18-26: ⚡ Quick winNarrow the throws contract on these
from_jsonfixtures.
from_jsontakes an already-parsedjson, soJsonParseErrorshould not be part of the expected surface here. Keeping onlyJsonDecodeErrormakes this regression tighter and avoids masking an accidental parse step in either the user-defined or auto-derived path.Suggested diff
-function decode_root(j: baml.json.json) -> root.pkg.inner.Box<int> throws baml.json.JsonParseError | baml.json.JsonDecodeError { +function decode_root(j: baml.json.json) -> root.pkg.inner.Box<int> throws baml.json.JsonDecodeError { root.pkg.inner.Box<int>.from_json(j) } -function decode_auto(j: baml.json.json) -> root.pkg.inner.AutoBox<int> throws baml.json.JsonParseError | baml.json.JsonDecodeError { +function decode_auto(j: baml.json.json) -> root.pkg.inner.AutoBox<int> throws baml.json.JsonDecodeError { root.pkg.inner.AutoBox<int>.from_json(j) }🤖 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_tests/projects/compiles/json_cross_namespace_static_call/main.baml` around lines 18 - 26, The functions decode_root and decode_auto currently declare they throw both baml.json.JsonParseError and baml.json.JsonDecodeError but since from_json consumes an already-parsed baml.json.json the parse error cannot originate here; update the throws clause on both decode_root(...) -> root.pkg.inner.Box<int> and decode_auto(...) -> root.pkg.inner.AutoBox<int> to only include baml.json.JsonDecodeError so the signatures reflect the narrower contract and prevent accidentally hiding a parse step.baml_language/crates/baml_compiler2_emit/src/lib.rs (1)
297-341: ⚡ Quick winAdd a focused Rust unit test for generic
field_templatelowering.This path now carries class type params through both
field_typeandfield_template, and that regression will be much easier to localize with a small unit test in this crate than with compile fixtures alone. A targeted case likeclass Box<T> { value T }assertingvaluelowers toTypeArgRef(0)would lock this down well. Please also runcargo test --libafter wiring it in. As per coding guidelines "Prefer writing Rust unit tests over integration tests where possible" and "Always runcargo test --libif you changed any Rust code".🤖 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_compiler2_emit/src/lib.rs` around lines 297 - 341, Add a focused Rust unit test in this crate that verifies class generic params are threaded into field_template lowering: create a small test that builds a type/AST for something like class Box<T> { value: T } (or use lower_type_expr_in_ns to parse a TypeExpr for T with class_generic_params set to ["T"]), call baml_compiler2_tir::lower_type_expr_in_ns to get tir_ty, then call baml_compiler2_mir::tir2_to_template with the same class_generic_params and assert the resulting TyTemplate for the field is TyTemplate::TypeArgRef(0); place the test near other unit tests for tir2_to_template or class lowering, run cargo test --lib to ensure it passes, and reference symbols class_generic_params, lower_type_expr_in_ns, tir2_to_template, and TyTemplate::TypeArgRef in the test to lock the behavior.baml_language/crates/baml_compiler2_tir/src/infer_context.rs (1)
737-741: 💤 Low valueDoc says only
UnresolvedMember, but suppression actually covers 4 kinds.Both this setter's doc comment ("Toggle suppression of
UnresolvedMemberdiagnostics") and the field namesuppress_member_lookup_errorsunderstate the scope.is_synthesized_code_diag(Lines 717‑725) suppressesUnresolvedMember,UnresolvedType,UnresolvedName, andNotCallable. A future maintainer reading just the setter signature could easily miss thatUnresolvedName/NotCallableget silenced too. Renaming the field is more invasive, but fixing the doc is one line:📝 Doc-only tweak
- /// Toggle suppression of `UnresolvedMember` diagnostics for the - /// current inference run. See `suppress_member_lookup_errors`. + /// Toggle suppression of synthesized-code diagnostics + /// (`UnresolvedMember`, `UnresolvedType`, `UnresolvedName`, `NotCallable`) + /// for the current inference run. See `suppress_member_lookup_errors` + /// and `is_synthesized_code_diag`. pub fn set_suppress_member_lookup_errors(&self, value: bool) {If you'd rather rename the field/setter to
suppress_synthesized_code_diagsfor accuracy, I can prepare that diff — it's a few call sites inbuilder.rs/inference.rs.🤖 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_compiler2_tir/src/infer_context.rs` around lines 737 - 741, The setter set_suppress_member_lookup_errors and its doc comment understate what they suppress; update the doc on set_suppress_member_lookup_errors (and optionally the field name suppress_member_lookup_errors) to state that it toggles suppression of UnresolvedMember, UnresolvedType, UnresolvedName, and NotCallable diagnostics (these are controlled by is_synthesized_code_diag). Keep the setter behavior unchanged but make the comment explicit about all four diagnostic kinds; if you prefer, rename to suppress_synthesized_code_diags and update callers in builder.rs and inference.rs to match.baml_language/crates/baml_compiler2_ast/src/auto_derive_json.rs (1)
132-202: 💤 Low valueStale docstrings on
synthesize_to_json/synthesize_from_json.Both function-level doc comments describe only the wrapper‑body fallback (
baml.json.parse(baml.json.to_string<Self>(self))andbaml.json.from_string<Self>(baml.json.stringify(j))), but the primary path is the per‑field map literal / per‑field constructor produced bybuild_to_json_body/build_from_json_body. The wrapper is taken only whenclass_is_safe_for_per_field_synthesisreturns false. The file header (Lines 8‑54) describes the real behavior accurately, so this is just a header‑comment skew on these two helpers.📝 Suggested doc updates
-/// Synthesize `function to_json(self) -> json throws JsonSerializationError | JsonParseError { -/// baml.json.parse(baml.json.to_string<Self>(self)) -/// }` +/// Synthesize a `to_json(self) -> json` method. +/// +/// The body is built by `build_to_json_body`, which emits a per-field map +/// literal (`{ "f": self.f.to_json(), ... }`) when every field type is safe +/// for per-field dispatch, or otherwise falls back to a wrapper: +/// `baml.json.parse(baml.json.to_string<Self>(self))`. fn synthesize_to_json(class: &ClassDef, span: TextRange) -> FunctionDef {-/// Synthesize `function from_json(j: json) -> Self throws JsonParseError | JsonDecodeError { -/// baml.json.from_string<Self>(baml.json.stringify(j)) -/// }` +/// Synthesize a `from_json(j: json) -> Self` method. +/// +/// The body is built by `build_from_json_body`, which emits a per-field +/// class-instance constructor (`Self { f: baml.json.from_json<F>(baml.json +/// .field(j, "f")), ... }`) when every field type is safe for per-field +/// dispatch, or otherwise falls back to a wrapper: +/// `baml.json.from_string<Self>(baml.json.stringify(j))`. fn synthesize_from_json(class: &ClassDef, span: TextRange) -> FunctionDef {🤖 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_compiler2_ast/src/auto_derive_json.rs` around lines 132 - 202, The doc comments on synthesize_to_json and synthesize_from_json are stale: update each function-level docstring to state that the primary behavior is per-field map literal / per-field constructor synthesis produced by build_to_json_body and build_from_json_body, and that the wrapper calls shown in the old comments (baml.json.parse(...)/baml.json.from_string(...)) are only used as a fallback when class_is_safe_for_per_field_synthesis returns false; align wording with the file header (lines 8–54) and mention the fallback condition so the comments accurately reflect both paths.baml_language/crates/baml_builtins2/baml_std/baml/ns_json/json.baml (1)
17-29: 💤 Low valueParam syntax is inconsistent within this new file.
parse,stringify, andstringify_prettydeclare params asname type, while every other function in the same file (to_string,to_json,from_string,from_json,field) usesname: type. Both are valid BAML, but for a freshly added file it's worth picking one. The colon form dominates the rest of this file and matches thefield(j: json, key: string)helper called from synthesizedfrom_jsonbodies.✏️ Suggested normalization
//baml:mut_vm -function parse(s string) -> json throws JsonParseError { +function parse(s: string) -> json throws JsonParseError { $rust_function } //baml:mut_vm -function stringify(j json) -> string { +function stringify(j: json) -> string { $rust_function } //baml:mut_vm -function stringify_pretty(j json) -> string { +function stringify_pretty(j: json) -> string { $rust_function }🤖 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_builtins2/baml_std/baml/ns_json/json.baml` around lines 17 - 29, Normalize parameter syntax in this file by changing the parameter declarations for parse, stringify, and stringify_pretty to use the colon style (e.g., "s: string", "j: json") to match the rest of the file and the synthesized from_json bodies that call field(j: json, key: string); update the signatures of parse, stringify, and stringify_pretty accordingly so they are consistent with to_string, to_json, from_string, from_json, and field.baml_language/crates/baml_builtins2_codegen/src/codegen.rs (1)
1465-1468: ⚡ Quick winRemove the dead
receiver_input_typewrapper.Line 965 switched to
receiver_input_type_with_vm_usage(recv, b.vm_usage), and this forwarding function now has no callers. The#[allow(dead_code)]annotation confirms it's unused. Removing it eliminates a stale symbol that could mislead future readers into using the non-VM-aware variant.Also update the doc comment for
receiver_input_type_with_vm_usage(line 1470) from "Likereceiver_input_typebut …" to remove the reference to the deleted function.♻️ Proposed removal
-#[allow(dead_code)] -fn receiver_input_type(recv: &Receiver) -> String { - receiver_input_type_with_vm_usage(recv, VmUsage::None) -} - /// Like `receiver_input_type` but switches media class receivers to `&Value`🤖 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_builtins2_codegen/src/codegen.rs` around lines 1465 - 1468, Remove the now-dead forwarding function receiver_input_type (and its #[allow(dead_code)] attribute) since callers were switched to receiver_input_type_with_vm_usage, and update the doc comment on receiver_input_type_with_vm_usage to drop the phrase "Like `receiver_input_type` but …" so it no longer references the deleted symbol; ensure references to VmUsage and the function name receiver_input_type_with_vm_usage remain correct and self-contained in the comment.baml_language/crates/baml_compiler2_mir/src/lower.rs (1)
260-322: ⚡ Quick winUnify the two
Tir2Ty -> TyTemplateimplementations to avoid drift.
tir2_to_templateandLoweringContext::ty_to_templatenow encode almost the same match tree. Keeping a single implementation will reduce future divergence in generic/type-template behavior.♻️ Suggested consolidation
fn ty_to_template(&self, ty: &Tir2Ty, generic_params: &[baml_base::Name]) -> TyTemplate { - match ty { - // ... duplicated conversion logic ... - } + tir2_to_template(ty, &self.resolved_aliases, generic_params) }🤖 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_compiler2_mir/src/lower.rs` around lines 260 - 322, The two functions tir2_to_template and LoweringContext::ty_to_template duplicate the same match logic; extract the shared logic into one helper (e.g., a private function like tir2_ty_to_template_inner) that both tir2_to_template and LoweringContext::ty_to_template call, or have tir2_to_template delegate to LoweringContext::ty_to_template (or vice versa), preserving existing behavior for TypeVar handling, Class/type-arg branching (using baml_compiler2_tir::generics::contains_typevar), the use of convert_tir2_ty and qtn_to_type_name, and passing through resolved and generic_params; update call sites to use the single implementation so the match tree is not duplicated.
🤖 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/baml_compiler2_ast/src/auto_derive_json.rs`:
- Around line 405-487: The is_typevar check only matches a bare TypeVar and
misses nested optional/union wrappers (e.g., T? or T | null), so nullable
generic fields still generate optional-chain calls and re-introduce the
receiver-erasure bug; update the detection to recursively inspect the type
expression (or add a helper like type_expr_contains_typevar) and use that in
place of type_expr_is_typevar_reference when computing is_typevar so any
Optional or Union whose inner type is a TypeVar will be treated as a TypeVar
field and thus emitted as baml.json.to_json(self.<field>) (adjust the logic
around the existing is_typevar / is_nullable checks so is_typevar wins for
nested TypeVar cases).
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 1464-1545: The current fold for Ty::Union uses the first arm's
params blindly (fn_components -> first_params) which is incorrect when function
arms have differing arity/parameter types; before constructing folded_fn and
calling check_call_inner, add a pairwise compatibility check over fn_components'
params (e.g., same arity and compatible param types/effective shapes) — if all
arms' params are compatible, proceed to build folded_fn as before, otherwise
abort the fold and treat the union as ambiguous/non-callable (i.e., do not call
check_call_inner with the folded_fn and let the existing non-fold path report
the callee as ambiguous). Ensure the compatibility check uses the same expansion
logic (expanded via expand_alias_chains) and references fn_components,
first_params, folded_fn, and the call into check_call_inner.
- Around line 5286-5299: The enum `to_json` branch currently always returns an
unbound function type (Ty::Function with a self parameter) which breaks
bound-method semantics and the side-effect-free resolver; change the
implementation for the Ty::Enum / member.as_str() == "to_json" case to
synthesize both the unbound signature (function taking self: base_ty and
returning json_alias_ty()) and the bound signature (callable with no explicit
self) the same way class/builtin paths do, and ensure the same dual-signature
logic is mirrored in the side-effect-free resolver; update the analogous enum
handling at the other location (the code region around the second enum branch
you noted) so both places produce consistent bound vs. unbound signatures and
use Ty::Never for throws as before.
- Around line 4310-4332: The current code converts the collected throw facts
(all_throws) via Self::ty_from_concrete_facts(...).unwrap_or(Ty::Never { .. })
which under-approximates when ty_from_concrete_facts returns None; instead, keep
the analysis conservative by returning a union of the collected facts when
concrete conversion fails. Replace the unwrap_or(Ty::Never) usage in the
Ty::Union handling (around typed_callee / all_throws / ty_from_concrete_facts)
so that if ty_from_concrete_facts(&all_throws) returns Some(t) you return that,
otherwise construct and return a Ty::Union built from the members in all_throws
(with an appropriate TyAttr::default()), rather than falling back to Ty::Never.
---
Nitpick comments:
In `@baml_language/crates/baml_builtins2_codegen/src/codegen.rs`:
- Around line 1465-1468: Remove the now-dead forwarding function
receiver_input_type (and its #[allow(dead_code)] attribute) since callers were
switched to receiver_input_type_with_vm_usage, and update the doc comment on
receiver_input_type_with_vm_usage to drop the phrase "Like `receiver_input_type`
but …" so it no longer references the deleted symbol; ensure references to
VmUsage and the function name receiver_input_type_with_vm_usage remain correct
and self-contained in the comment.
In `@baml_language/crates/baml_builtins2/baml_std/baml/ns_json/json.baml`:
- Around line 17-29: Normalize parameter syntax in this file by changing the
parameter declarations for parse, stringify, and stringify_pretty to use the
colon style (e.g., "s: string", "j: json") to match the rest of the file and the
synthesized from_json bodies that call field(j: json, key: string); update the
signatures of parse, stringify, and stringify_pretty accordingly so they are
consistent with to_string, to_json, from_string, from_json, and field.
In `@baml_language/crates/baml_builtins2/baml_std/baml/null.baml`:
- Around line 1-5: Add a brief explanatory comment above the class Null to
document why Null.to_json omits the //baml:mut_vm annotation — note that Null
(and other scalar primitives like Float, Int, Bool) produce immediate JSON
values and thus do not require mutating the VM, whereas heap-allocated types
(String, Uint8Array, Array<T>, Map<K,V>) include //baml:mut_vm; place the
comment adjacent to the class Null declaration so future maintainers see the
intent when inspecting Null.to_json.
In `@baml_language/crates/baml_compiler2_ast/src/auto_derive_json.rs`:
- Around line 132-202: The doc comments on synthesize_to_json and
synthesize_from_json are stale: update each function-level docstring to state
that the primary behavior is per-field map literal / per-field constructor
synthesis produced by build_to_json_body and build_from_json_body, and that the
wrapper calls shown in the old comments
(baml.json.parse(...)/baml.json.from_string(...)) are only used as a fallback
when class_is_safe_for_per_field_synthesis returns false; align wording with the
file header (lines 8–54) and mention the fallback condition so the comments
accurately reflect both paths.
In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs`:
- Around line 33-61: Add focused Rust unit tests in the same module as
find_callee_generic_args to exercise parser shapes: (1) a direct call-site case
where GENERIC_ARGS is a child of the callee PATH_EXPR (e.g. foo<T>(...)) and
assert find_callee_generic_args returns that GENERIC_ARGS node; (2) a
receiver-carried case where the call is a FIELD_ACCESS_EXPR or
OPTIONAL_FIELD_ACCESS_EXPR whose base PATH_EXPR has GENERIC_ARGS (e.g.
Box<T>.method(...)) and assert it returns the base's GENERIC_ARGS; and (3) a
precedence case where both receiver and method-level GENERIC_ARGS exist (e.g.
Container<int>.method<U>(...)) and assert the direct child GENERIC_ARGS
(method-level) is returned. Use the same parsing helpers used elsewhere in this
file to build a SyntaxNode from snippets and compare node.kind() ==
SyntaxKind::GENERIC_ARGS or Option::is_some/is_none as appropriate so the tests
lock the intended behavior for find_callee_generic_args.
In `@baml_language/crates/baml_compiler2_ast/src/lower_type_expr.rs`:
- Around line 395-399: Replace the hardcoded segments vec![Name::new("baml"),
Name::new("json"), Name::new("json")] used to build the TypeExpr::Path with the
shared canonical sentinel for "baml.json.json" (the module-level constant that
represents the JSON sentinel) to avoid duplication; import or reference that
sentinel constant where the mapping into TypeExpr::Path occurs (the block
constructing TypeExpr::Path for the "json" case) and use it to populate the
segments field instead of calling Name::new(...) three times.
In `@baml_language/crates/baml_compiler2_emit/src/lib.rs`:
- Around line 297-341: Add a focused Rust unit test in this crate that verifies
class generic params are threaded into field_template lowering: create a small
test that builds a type/AST for something like class Box<T> { value: T } (or use
lower_type_expr_in_ns to parse a TypeExpr for T with class_generic_params set to
["T"]), call baml_compiler2_tir::lower_type_expr_in_ns to get tir_ty, then call
baml_compiler2_mir::tir2_to_template with the same class_generic_params and
assert the resulting TyTemplate for the field is TyTemplate::TypeArgRef(0);
place the test near other unit tests for tir2_to_template or class lowering, run
cargo test --lib to ensure it passes, and reference symbols
class_generic_params, lower_type_expr_in_ns, tir2_to_template, and
TyTemplate::TypeArgRef in the test to lock the behavior.
In `@baml_language/crates/baml_compiler2_hir/src/builder.rs`:
- Around line 1508-1529: The bug is that is_builtin_class_ref currently subsumes
single-segment uppercase generic names and prevents the declared-params check
from ever succeeding; to fix, compute is_allowed_generic independently (remove
the && !is_builtin_class_ref guard so allowed_generic_params.iter().any(...) can
be true even if is_builtin_class_ref is true) and then change the final guard to
explicitly allow either a builtin class or a declared generic: replace the
existing if !is_builtin_error && !is_builtin_class_ref && !is_allowed_generic
check with if !is_builtin_error && !(is_builtin_class_ref ||
is_allowed_generic), keeping the existing definitions of segments, generic_args,
is_builtin_error, is_builtin_class_ref, and allowed_generic_params so declared
generic params take precedence for validation.
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 260-322: The two functions tir2_to_template and
LoweringContext::ty_to_template duplicate the same match logic; extract the
shared logic into one helper (e.g., a private function like
tir2_ty_to_template_inner) that both tir2_to_template and
LoweringContext::ty_to_template call, or have tir2_to_template delegate to
LoweringContext::ty_to_template (or vice versa), preserving existing behavior
for TypeVar handling, Class/type-arg branching (using
baml_compiler2_tir::generics::contains_typevar), the use of convert_tir2_ty and
qtn_to_type_name, and passing through resolved and generic_params; update call
sites to use the single implementation so the match tree is not duplicated.
In `@baml_language/crates/baml_compiler2_tir/src/infer_context.rs`:
- Around line 737-741: The setter set_suppress_member_lookup_errors and its doc
comment understate what they suppress; update the doc on
set_suppress_member_lookup_errors (and optionally the field name
suppress_member_lookup_errors) to state that it toggles suppression of
UnresolvedMember, UnresolvedType, UnresolvedName, and NotCallable diagnostics
(these are controlled by is_synthesized_code_diag). Keep the setter behavior
unchanged but make the comment explicit about all four diagnostic kinds; if you
prefer, rename to suppress_synthesized_code_diags and update callers in
builder.rs and inference.rs to match.
In `@baml_language/crates/baml_compiler2_tir/src/inference.rs`:
- Around line 451-454: The same predicate matches!(func_data.origin,
ast::FunctionOrigin::AutoDerive) is evaluated twice; compute it once into a
local variable (e.g., let is_auto_derive = ...) and reuse that variable both
when calling builder.set_auto_derived(...) and in the subsequent
skip/contract-check logic (the block that currently recomputes matches! around
func_data.origin). Update all places that currently recompute that predicate
(including the skip condition in the later contract-check code) to reference
is_auto_derive so future changes to the skip criteria remain consistent.
In
`@baml_language/crates/baml_tests/projects/compiles/json_cross_namespace_static_call/main.baml`:
- Around line 18-26: The functions decode_root and decode_auto currently declare
they throw both baml.json.JsonParseError and baml.json.JsonDecodeError but since
from_json consumes an already-parsed baml.json.json the parse error cannot
originate here; update the throws clause on both decode_root(...) ->
root.pkg.inner.Box<int> and decode_auto(...) -> root.pkg.inner.AutoBox<int> to
only include baml.json.JsonDecodeError so the signatures reflect the narrower
contract and prevent accidentally hiding a parse step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+4310
to
+4332
| // When the callee has a union type (e.g., a method called on a union-typed | ||
| // field like `(string | int | MyClass).to_json()`), every member of the union | ||
| // is a separate function that might execute. Conservatively union all of | ||
| // their throws — if every member is a function we can compute a precise | ||
| // answer; otherwise fall through to return `None` (unknown). | ||
| if let Ty::Union(ref members, _) = typed_callee { | ||
| let mut all_throws: BTreeSet<Ty> = BTreeSet::new(); | ||
| for member in members { | ||
| if let Ty::Function { throws, .. } = member { | ||
| all_throws.extend(crate::throw_inference::flatten_ty_to_facts(throws)); | ||
| } else { | ||
| // At least one member is not a function — can't compute throws, | ||
| // return None to let the fallback handle it. | ||
| return None; | ||
| } | ||
| } | ||
| // All members were functions; return the union of their throws. | ||
| return Some( | ||
| Self::ty_from_concrete_facts(&all_throws).unwrap_or(Ty::Never { | ||
| attr: TyAttr::default(), | ||
| }), | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't turn unresolved union throws into never.
ty_from_concrete_facts returns None when any arm contributes a type var / unknown / error throw fact. The new unwrap_or(Ty::Never) then under-approximates escaping throws for generic or partially-known callees and can suppress throws / catch diagnostics. Joining the collected facts directly keeps the analysis conservative.
Suggested fix
- return Some(
- Self::ty_from_concrete_facts(&all_throws).unwrap_or(Ty::Never {
- attr: TyAttr::default(),
- }),
- );
+ return Some(Self::facts_to_ty(&all_throws));🤖 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_compiler2_tir/src/builder.rs` around lines 4310 -
4332, The current code converts the collected throw facts (all_throws) via
Self::ty_from_concrete_facts(...).unwrap_or(Ty::Never { .. }) which
under-approximates when ty_from_concrete_facts returns None; instead, keep the
analysis conservative by returning a union of the collected facts when concrete
conversion fails. Replace the unwrap_or(Ty::Never) usage in the Ty::Union
handling (around typed_callee / all_throws / ty_from_concrete_facts) so that if
ty_from_concrete_facts(&all_throws) returns Some(t) you return that, otherwise
construct and return a Ty::Union built from the members in all_throws (with an
appropriate TyAttr::default()), rather than falling back to Ty::Never.
Comment on lines
5286
to
+5299
| Ty::Enum(enum_name, _) => { | ||
| // `to_json` on an enum: returns the variant name as a JSON string. | ||
| // BEP-038 specifies the enum JSON representation as its variant name string. | ||
| // Throws `never` — enum serialization always succeeds. | ||
| if member.as_str() == "to_json" { | ||
| return Ty::Function { | ||
| params: vec![(Some(Name::new("self")), base_ty.clone())], | ||
| ret: Box::new(json_alias_ty()), | ||
| throws: Box::new(Ty::Never { | ||
| attr: TyAttr::default(), | ||
| }), | ||
| attr: TyAttr::default(), | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Make synthetic enum to_json follow bound-method semantics consistently.
These branches always return a function type with an explicit self parameter. That works for an immediate call only because call checking special-cases method syntax, but a bound reference like let f = color.to_json; f() is still typed as needing one argument, and the helper path used for unions still has no enum mirror. Please synthesize bound vs. unbound signatures the same way the class/builtin paths do, and mirror the enum case in the side-effect-free resolver.
Also applies to: 5652-5664
🤖 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_compiler2_tir/src/builder.rs` around lines 5286 -
5299, The enum `to_json` branch currently always returns an unbound function
type (Ty::Function with a self parameter) which breaks bound-method semantics
and the side-effect-free resolver; change the implementation for the Ty::Enum /
member.as_str() == "to_json" case to synthesize both the unbound signature
(function taking self: base_ty and returning json_alias_ty()) and the bound
signature (callable with no explicit self) the same way class/builtin paths do,
and ensure the same dual-signature logic is mirrored in the side-effect-free
resolver; update the analogous enum handling at the other location (the code
region around the second enum branch you noted) so both places produce
consistent bound vs. unbound signatures and use Ty::Never for throws as before.
Merging this PR will degrade performance by 63.05%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | engine_init_cost |
2.3 ms | 3.8 ms | -40.37% |
| ❌ | WallTime | e2e_hello_world |
52.7 ms | 142.6 ms | -63.05% |
| ❌ | WallTime | e2e_arithmetic |
53 ms | 139.3 ms | -61.96% |
| ❌ | WallTime | vm_closure_call_50k |
10.9 ms | 13.3 ms | -17.9% |
| ❌ | WallTime | e2e_class_and_loop |
54.3 ms | 145.1 ms | -62.6% |
| ❌ | WallTime | vm_fib_20 |
5.5 ms | 8.2 ms | -32.53% |
| ❌ | WallTime | vm_array_push_50k |
9.8 ms | 11.6 ms | -15.46% |
| ❌ | WallTime | vm_nested_loop |
5.9 ms | 7.5 ms | -21.61% |
| ❌ | WallTime | vm_class_create_50k |
16.1 ms | 18.2 ms | -11.22% |
| ❌ | WallTime | e2e_100_functions |
136.5 ms | 233.8 ms | -41.62% |
| ❌ | WallTime | vm_field_access_50k |
8.4 ms | 10.2 ms | -17.31% |
| ❌ | WallTime | startup_empty_expression |
53.1 ms | 142 ms | -62.6% |
| ❌ | WallTime | compile_to_engine |
53.3 ms | 142.9 ms | -62.73% |
| ❌ | WallTime | vm_array_iter_10k |
5.6 ms | 7.1 ms | -20.6% |
| ❌ | WallTime | vm_call_chain_100_x_5k |
44.9 ms | 51.4 ms | -12.72% |
| ❌ | WallTime | e2e_fib_20 |
57 ms | 146.8 ms | -61.15% |
Comparing antonio/native-json-type (9ca04c5) with canary (cb7008f)
Auto-derived `to_json` for `class Box<T> { value: T? }` previously emitted
`self.value?.to_json()`, but after `?.` short-circuits null the receiver
still has TypeVar type — MIR erases it to `Ty::Void`, method dispatch
fails (`field_idx = None`), and the lowering falls through to a dynamic
map-load that panics on primitives/instances. Same hazard for `T | null`.
Extend the existing TypeVar detection to also match `Optional<TypeVar>`
and unions whose non-null variants reduce to TypeVar, routing those
fields through `baml.json.to_json(self.<f>)` so dispatch happens on the
runtime value's actual type. User overrides remain honored
(`make_to_json_callee` looks up `<runtime-fqn>.to_json` per call).
Other cleanups bundled:
- Guard the union-callable fold in `check_call_inner` behind pairwise
arity / non-self param-type compatibility; otherwise report not
callable instead of silently typechecking against the first arm.
- Drop commit-narrating Phase-X / 5b.4 comments from json.rs,
auto_derive_json.rs, and mir/lower.rs.
- Lift the duplicated MediaKind→tag-string match into
`MediaKind::tag_str()` on `baml_base`; Display routes through it.
- Replace the 6× `saved_len/write!/truncate` path-tracking dance in
ty_value_to_serde / ty_serde_to_value with a `with_path_segment`
helper.
Tests:
- 3 new regressions in json_auto_derive: nullable-TypeVar field with
non-null value, with null value, and override-honoring through the
nullable-TypeVar path.
- 2 existing diagnostic_errors/generics snapshots updated — they were
capturing the buggy `self.value?.to_json()` output for a fixture
with `class MaybeBox<T> { value: T? }`.
antoniosarosi
enabled auto-merge
May 7, 2026 19:47
This was referenced May 29, 2026
Merged
This was referenced Jun 9, 2026
This was referenced Jun 25, 2026
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.
Summary
Native
baml.jsonnamespace built on top of BEP-039 reflect.type_of<T>() — exposes ajsontype alias, parse/stringify intrinsics, type-reflection-driven serialisation (to_string<T>/from_string<T>), and per-class auto-derivedto_json/from_jsonmethods that honour user overrides.13 phased feature commits plus 2 rebase-integration fixups:
User-facing surface
baml.json.jsontype alias — JSON-shaped union (bool | int | float | str | json[] | map<string, json> | null).baml.json.parse(s)/baml.json.stringify(j)/baml.json.stringify_pretty(j)— native parse and pretty-print.baml.json.to_string<T>(v)/baml.json.from_string<T>(s)— type-reflection-driven encode/decode.class.to_json()/Class.from_json(j)on every user class (suppressed when the user defines either method explicitly). Honors per-field overrides via runtime dispatch —self.f.to_json()calls whateverto_jsonf's type defines, including auto-derived methods on nested classes,Array<T>.to_json,Map<K,V>.to_json, and primitive bridges.-> jsonreturn types end-to-end.Phases (commit-by-commit)
jsontype alias, error classes,ns_jsonnamespace, keyword sugar.parse/stringify/stringify_prettynatives.function F() -> json.to_string<T>/from_string<T>via runtime type reflection (BEP-039).to_json/from_jsonscaffold (FunctionOrigin::AutoDerive).Array<T>/Map<K,V>to_json, per-field auto-deriveto_jsonbody, generic-classto_jsonwith TypeVar override-honoring.from_jsonper-field with override-honoring + edge-case test coverage.Rebase integration (last 2 commits)
After replaying the 13 commits onto canary (post reflect.type_of merge + Sam's pattern-system overhaul), two real runtime bugs surfaced and a few mechanical updates were needed:
YieldToCallcallback was a synchronously-Done native —*frame_idxwasn't advanced past the caller's bytecode frame, so the inner dispatch loop kept stepping pastReturn. Fix: advance*frame_idxafter the recursiveexecute_call_from_locals_offsetreturns.baml.json.from_json: missing type argumentfrom native handlers readingpending_call_type_args. The compactCallopcode dispatch popped explicit type args off the stack and seeded the new bytecode frame'stype_args, but never stashed them inpending_call_type_argsfor native callees. Fix: mirror the YieldToCall plumbing.x: int =>→let x: int =>per canary's pattern-system overhaul (4 .rs test files, 2 .baml fixtures).docstring: None(canary's docstring-preservation merge added the field).FunctionOrigin::AutoDerivemethods sodescribe_item_memberdoesn't try to slice source for synthesised methods.compiler2_mir::generic_class_destructure_field_*MIR test scopes its void-local check touser.f's body — auto-derived methods onBox<T>legitimately have void locals.baml.json.jsonto emittyping.Any(TODO marker left for the recursive representation once pyright handlesTypeAliasTypeforward-refs).Test plan
cargo test --workspace --exclude bridge_python— all 159 test results pass.cargo insta test --accept— snapshots regenerated for the merged surface.pyrighton generated Python) pass with the json→Anycodegen fix.Summary by CodeRabbit
New Features
Tests