Repository navigation
feat(interfaces): destructure interface fields in match (BEP-044) - #3632
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements interface-head destructuring across TIR, exhaustiveness, and MIR lowering, and switches Python SDK code generation to embed borsh-serialized BAML bytecode with runtime initialization paths and corresponding test-harness and tests. ChangesInterface Destructuring and Bytecode Integration
Sequence Diagram(s)sequenceDiagram
participant CLI as cli generate
participant CodeGen as codegen_python
participant Root as __init__.py
participant Inlined as _inlinedbaml.py
CLI->>CodeGen: to_source_code_with_bytecode(pool, bytecode)
CodeGen->>Inlined: emit BYTECODE bytes literal
CodeGen->>Root: emit BamlRuntime.initialize_runtime_from_bytecode(...)
Root->>Inlined: import _inlinedbaml and pass BYTECODE to initialize_runtime_from_bytecode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
… tests
Interface-field destructuring in `match`: an interface head
(`Animal { name } => ...`) now binds the interface's declared fields across
every implementor, dispatching through each implementor's field view — so an
auto-linked field and an `as`-aliased field both resolve correctly. The
pattern matches all implementors, so it is exhaustive on its own.
- TIR (builder.rs): accept interface heads in destructure patterns; bind
fields via resolve_interface_member.
- MIR (lower.rs): project interface-pattern fields through the existing
interface field-view dispatch (try_lower_interface_field_access).
- Tests (interfaces.rs): refresh BEP-044 regression-suite comments — all
pass now; the two interface-method-as-value cases remain ignored
(unimplemented) — and add coverage for interface and concrete-implementor
destructuring.
Also bundles in-flight Python SDK / bridge / codegen fixes already present
on the working branch.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
aaronvg
force-pushed
the
aaron/interfaces2
branch
from
June 1, 2026 23:53
c6b4687 to
aac3777
Compare
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: 2
🧹 Nitpick comments (1)
baml_language/crates/baml_tests/tests/interfaces.rs (1)
2215-2223: ⚡ Quick winExercise the
Cat { name }branch in this test.
describehas two concrete destructure arms, butmain()only feeds aDog, so this runtime test never proves theCat { name }pattern executes correctly.Suggested change
function main() -> string { - let a: Animal = Dog { name: "Rex", breed: "Lab" } - return describe(a) + let dog: Animal = Dog { name: "Rex", breed: "Lab" } + let cat: Animal = Cat { name: "Luna" } + return describe(dog) + " / " + describe(cat) } @@ - BexExternalValue::String("Rex the Lab".into()) + BexExternalValue::String("Rex the Lab / Luna the cat".into())🤖 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/tests/interfaces.rs` around lines 2215 - 2223, Test only constructs a Dog currently, so the describe function's Cat { name } arm is never executed; update the test that builds main()/output.result to also exercise the Cat branch by either adding a second invocation of main (or a separate test) that constructs Animal::Cat { name: "Whiskers" } and asserts output.result.unwrap() equals BexExternalValue::String("Whiskers".into()) (or the expected string format), or modify the existing test to run both variants sequentially; target the function name main, the enum/type Animal, and the pattern arms Dog and Cat so the Cat { name } branch is actually exercised.
🤖 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_tir/src/builder.rs`:
- Around line 12748-12777: The Ty::Interface branch currently discards the
lowered per-field dpat results from analyze_and_lower and replaces them with a
generic dpat from dpat_for_type(&class_ty, scrut_ty), which loses the specific
field subpatterns and breaks exhaustiveness/refutability checks; instead,
collect each field's PatternResult.dpat (and their bindings) from the loop,
compose them into a structural/interface dpat that preserves per-field
subpatterns (rather than calling dpat_for_type unconditionally), and return that
composed dpat in the PatternResult while still setting required_ty:
Some(class_ty.clone()) and matched_ty from
intersect_pattern_flow_types(scrut_ty, &class_ty); reference analyze_and_lower,
PatternResult, dpat_for_type, Ty::Interface, bindings, and
intersect_pattern_flow_types to locate the change.
- Around line 12753-12767: The current call to resolve_interface_member permits
methods/defaults (so patterns like Animal { speak } wrongly type-check); change
this to a field-only lookup or add an explicit check after
resolve_interface_member to reject non-field members: call the field-specific
resolver (e.g., resolve_interface_field) if available, or call
resolve_interface_member and then inspect the returned member kind (compare
against MemberKind::Field or equivalent) and if it is not a field call
report_at_pat_or_expr with TirTypeError::UnresolvedMember (using
class_ty.clone(), fp.field.clone(), pat_id, at_expr) and return the Ty::Unknown
fallback as currently done. Ensure you reference iface_name, type_args,
fp.field, pat_id, and at_expr in the error path so non-field members are
rejected at this site rather than allowed.
---
Nitpick comments:
In `@baml_language/crates/baml_tests/tests/interfaces.rs`:
- Around line 2215-2223: Test only constructs a Dog currently, so the describe
function's Cat { name } arm is never executed; update the test that builds
main()/output.result to also exercise the Cat branch by either adding a second
invocation of main (or a separate test) that constructs Animal::Cat { name:
"Whiskers" } and asserts output.result.unwrap() equals
BexExternalValue::String("Whiskers".into()) (or the expected string format), or
modify the existing test to run both variants sequentially; target the function
name main, the enum/type Animal, and the pattern arms Dog and Cat so the Cat {
name } branch is actually exercised.
🪄 Autofix (Beta)
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
Run ID: c36f126a-b3bf-4044-808b-f97f0653c22c
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
baml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_tests/tests/interfaces.rsbaml_language/crates/bridge_cffi/Cargo.tomlbaml_language/crates/bridge_cffi/src/lib.rsbaml_language/sdk_tests/harness_setup/Cargo.tomlbaml_language/sdk_tests/harness_setup/src/lib.rsbaml_language/sdk_tests/harness_setup/src/python_pydantic2.rsbaml_language/sdks/python/rust/bridge_python/src/baml_core/baml_py/__init__.pyibaml_language/sdks/python/rust/bridge_python/src/runtime.rsbaml_language/sdks/python/rust/codegen_python/src/lib.rsbaml_language/sdks/python/src/baml_core/baml_py.pyi
`initialize_runtime_from_bytecode` built a `bex_engine::BexEngine` from a borsh-decoded `bex_vm_types::Program` directly, which `cargo stow` rejects: bridge crates may depend only on `bex_project` re-exports, not on bex internals (`bex_engine`, `bex_vm_types`). Add `bex_project::new_from_bytecode(&[u8], SysOps, Option<EventSink>)` that owns the borsh decode + engine construction, and re-point `bridge_cffi` at it. `bridge_cffi` drops its `bex_engine`, `bex_vm_types`, and `borsh` deps. `cargo stow` now passes. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/bex_project/src/lib.rs (1)
8-9: 💤 Low valueModule header lists only one of the two constructors.
The header says "Two public constructors:" but only documents
new. Now thatnew_from_bytecodeis a public constructor, add it here so the module doc matches the surface.📝 Proposed doc update
//! Two public constructors: //! - [`new`] — compile source files and return `Arc<dyn Bex>`. +//! - [`new_from_bytecode`] — load a pre-packed `bex_vm_types::Program` and +//! return `Arc<dyn Bex>` without compiling from source.🤖 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_project/src/lib.rs` around lines 8 - 9, Module header lists "Two public constructors" but only documents `new`; update the module-level doc comment in lib.rs to include `new_from_bytecode` as the second constructor and give a short description (e.g., "construct from bytecode and return Arc<dyn Bex>") so the doc comment matches the public API (`new` and `new_from_bytecode`).
🤖 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.
Nitpick comments:
In `@baml_language/crates/bex_project/src/lib.rs`:
- Around line 8-9: Module header lists "Two public constructors" but only
documents `new`; update the module-level doc comment in lib.rs to include
`new_from_bytecode` as the second constructor and give a short description
(e.g., "construct from bytecode and return Arc<dyn Bex>") so the doc comment
matches the public API (`new` and `new_from_bytecode`).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bed9c1c7-ad5d-42d9-a1c9-ee312f89c926
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
baml_language/crates/bex_project/Cargo.tomlbaml_language/crates/bex_project/src/lib.rsbaml_language/crates/bridge_cffi/src/lib.rs
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/bex_project/Cargo.toml
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/baml_compiler2_tir/src/builder.rs (1)
12818-12833:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRequire type args on generic interface destructures.
This branch now accepts interface heads, but it does not mirror the existing generic-class check.
class_pattern_missing_generic_args(...)only recognizesTy::Class, so a pattern likeContainer { item }againstContainer<int>falls through here withtype_args.is_empty(). The subsequentinterface_field_infos_ordered(...)call then lowersitemagainst the interface's bare type parameter (T) instead of the concrete instantiation (int), and union dispatch/exhaustiveness also lose the concrete interface member. Please reject missing type args for generic interfaces before lowering, the same way class destructures already do.🛠️ Suggested direction
let class_ty = self.resolve_class_pattern_type(class, generic_args, Some((pat_id, at_expr))); + if self.class_pattern_missing_generic_args(&class_ty, generic_args) { + let err = TirTypeError::GenericClassDestructureRequiresTypeArgs { + class_name: class.last().cloned().unwrap_or_else(|| Name::new("_")), + }; + self.context.report_simple(err, at_expr); + } + if let Ty::Interface(iface_name, type_args, _) = &class_ty {…and extend
class_pattern_missing_generic_argsto treatTy::Interfacethe same way it already treatsTy::Class.🤖 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 12818 - 12833, The interface-destructure branch accepts Ty::Interface but doesn't reject missing generic args, causing lowering (interface_field_infos_ordered) to use bare type parameters; update the pre-lowering check to reject missing type args for generic interfaces the same way as classes: extend the function/class_pattern_missing_generic_args to treat Ty::Interface like Ty::Class (or add an explicit check after resolve_class_pattern_type when class_ty is Ty::Interface) and return an error if type_args.is_empty(), ensuring patterns like Container { item } against Container<int> are rejected before calling interface_field_infos_ordered.
🧹 Nitpick comments (2)
baml_language/crates/baml_compiler2_tir/src/exhaustiveness.rs (1)
1305-1320: 💤 Low valueConsider adding unit tests for interface pattern exhaustiveness.
The exhaustiveness module has comprehensive unit tests for classes, slices, unions, and other pattern types, but no tests specifically exercise
Ctor::Interfacebehavior. While integration tests ininterfaces.rscover end-to-end behavior, unit tests here would verify:
Ctor::Interfaceequality/hash semantics- Interface arity calculation via
interface_field_types- The special-case logic in
split_ctors(lines 1126-1132) that treats interface patterns as exhaustiveA
TestingCtxextension withinterface_field_typesimplementation would enable testing interface-specific exhaustiveness scenarios. As per coding guidelines, unit tests are preferred over integration tests where possible.🤖 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/exhaustiveness.rs` around lines 1305 - 1320, Add unit tests exercising Ctor::Interface behavior in this module: extend TestingCtx with an implementation of interface_field_types so tests can construct interface arities, then add tests that assert Ctor::Interface equality/hash semantics, verify arity via interface_field_types, and cover the special-case logic in split_ctors that treats interface patterns as exhaustive (the branch around split_ctors lines handling interfaces). Use existing test helpers/constructors in the module to build interface patterns and assert expected exhaustiveness outcomes.baml_language/crates/baml_tests/tests/interfaces.rs (1)
2185-2213: ⚡ Quick winCover the
active: true/ aliased path too.Line 2204 only pins the
falsearm against an auto-linked implementor. A regression inSwitch { active: true }, or in literal subpattern projection whenactiveis backed by anasalias instead of a concrete field, would still pass this test. Please add a second runtime case for thetruebranch, ideally through an aliased implementor.🤖 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/tests/interfaces.rs` around lines 2185 - 2213, The test match_interface_destructure_respects_field_subpatterns only exercises the "off" path; add a second runtime case that constructs an implementor whose active field is true via an aliased field (use a class that implements Switch and exposes active through an `as` alias), call describe(s) with that instance and assert the result is BexExternalValue::String("on"). Update the baml_test! input to include the aliased implementor definition and a main/second invocation (or a separate test block) that returns/describes an instance with active: true so the Switch { active: true } arm is exercised at runtime.
🤖 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/baml_compiler2_tir/src/builder.rs`:
- Around line 12818-12833: The interface-destructure branch accepts
Ty::Interface but doesn't reject missing generic args, causing lowering
(interface_field_infos_ordered) to use bare type parameters; update the
pre-lowering check to reject missing type args for generic interfaces the same
way as classes: extend the function/class_pattern_missing_generic_args to treat
Ty::Interface like Ty::Class (or add an explicit check after
resolve_class_pattern_type when class_ty is Ty::Interface) and return an error
if type_args.is_empty(), ensuring patterns like Container { item } against
Container<int> are rejected before calling interface_field_infos_ordered.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler2_tir/src/exhaustiveness.rs`:
- Around line 1305-1320: Add unit tests exercising Ctor::Interface behavior in
this module: extend TestingCtx with an implementation of interface_field_types
so tests can construct interface arities, then add tests that assert
Ctor::Interface equality/hash semantics, verify arity via interface_field_types,
and cover the special-case logic in split_ctors that treats interface patterns
as exhaustive (the branch around split_ctors lines handling interfaces). Use
existing test helpers/constructors in the module to build interface patterns and
assert expected exhaustiveness outcomes.
In `@baml_language/crates/baml_tests/tests/interfaces.rs`:
- Around line 2185-2213: The test
match_interface_destructure_respects_field_subpatterns only exercises the "off"
path; add a second runtime case that constructs an implementor whose active
field is true via an aliased field (use a class that implements Switch and
exposes active through an `as` alias), call describe(s) with that instance and
assert the result is BexExternalValue::String("on"). Update the baml_test! input
to include the aliased implementor definition and a main/second invocation (or a
separate test block) that returns/describes an instance with active: true so the
Switch { active: true } arm is exercised at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e3ae5bbe-492d-4c3a-b2b8-d1e880dd5a67
📒 Files selected for processing (3)
baml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/exhaustiveness.rsbaml_language/crates/baml_tests/tests/interfaces.rs
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
aaronvg
disabled auto-merge
June 2, 2026 01:06
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 379f7e4. Configure here.
This was referenced Jun 2, 2026
This branch was previously 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.
What
Adds interface-field destructuring in
matchpatterns. An interface head binds the interface's declared fields across every implementor:nameresolves through each implementor's field view, so it works whether the field was auto-linked (Dog { name }) oras-aliased (Cat { name as nickname }). Because every implementor necessarily provides the interface's declared fields, the pattern matches them all — soAnimal { name }is exhaustive on its own (no_needed).Previously
Animal { name }was mis-lowered as a construction expression (unresolved name: Animal); only concrete-class destructure (Dog { name }) worked.How
baml_compiler2_tir/src/builder.rs):resolve_class_pattern_typeaccepts interface heads;lower_class_pathas an interface branch that binds each field's type viaresolve_interface_memberand produces a wildcard-coverDPat.baml_compiler2_mir/src/lower.rs):project_class_pattern_fieldroutes interface heads to a newproject_interface_pattern_field, reusing the existing interface field-view dispatch (try_lower_interface_field_access). The MIRTyhas no interface variant, so the route keys off the rawTir2Ty::Interface.Tests
match_destructures_interface_fields_directly(interface head, auto-linked + aliased implementors) andmatch_destructures_concrete_implementor_fields.fuzz_bug01/02) remain#[ignore]d (genuinely unimplemented).baml_tests(30 binaries) andcargo check --workspaceclean.The matching BEP-044 spec update (match syntax, this feature, and other implementation-vs-draft corrections) was pushed separately to beps.boundaryml.com.
Also included
In-flight Python SDK / bridge / codegen fixes that were already present on the working branch (
bridge_cffi,bridge_python,codegen_python,harness_setup,baml_cli/generate.rs,.pyistubs). Not authored as part of the interface work; bundled per request.🤖 Generated with Claude Code
Note
High Risk
Changes span TIR/MIR pattern lowering and exhaustiveness (easy to get match soundness wrong) plus the Python runtime initialization path that all generated SDKs use after
baml generate.Overview
Implements BEP-044 interface destructuring in
match(Animal { name } => …): TIR resolves interface pattern heads and lowersDPat::interfacewith field-view types; exhaustiveness gainsCtor::Interfaceand matrix specialization that maps interface field slots onto implementing class fields; MIR projects bound fields viaproject_interface_pattern_field/ existing interface field dispatch.Python codegen/runtime now embeds borsh-serialized bytecode instead of inlined
.bamlsource:baml generatecompiles and callsto_source_code_with_bytecode;bex_project::new_from_bytecode, CFFI/Pythoninitialize_runtime_from_bytecode, and generated_inlinedbaml.BYTECODEwiring. CI size-gate baselines forbaml-cliare bumped slightly.Large interface test suite additions (compile + VM) for destructure exhaustiveness, mixed concrete/interface arms, generics, and updated regression comments (most fuzz/wf3 cases now pass; method-as-value tests still ignored).
Reviewed by Cursor Bugbot for commit 3c25e0f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Tests
Chores