[codex] Implement BEP-57 associated types#3652
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughAdds BEP-057: associated type declarations, bindings, and projections for interfaces across parser, AST, CST→AST lowering, HIR, TIR, inference, validation, MIR/PPIR/emit/codegen, formatter, LSP, and tests. ChangesAssociated Type Support Across the Compiler Stack
🎯 5 (Critical) | ⏱️ ~120 minutes
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
aaronvg
marked this pull request as ready for review
June 3, 2026 01:35
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
baml_language/crates/baml_compiler2_ppir/src/ty.rs (2)
321-329:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPPIR still strips associated-type bindings on round-trip.
PpirTy::Namedhas nowhere to storeassociated_type_bindings, and this re-emits every named type with an empty binding list. That means any stream-expanded alias/class/function built from a type likeIterator<Item = int>is silently weakened to bareIteratorbefore TIR sees the synthesized item.🤖 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_ppir/src/ty.rs` around lines 321 - 329, PpirTy::Named currently drops associated type bindings when converting back to a TypeExpr; update the PpirTy definition to include a field for associated_type_bindings (the same shape used by TypeExpr::Path), propagate that field wherever PpirTy::Named is constructed/parsed, and modify PpirTy::to_type_expr to pass that stored associated_type_bindings into the produced TypeExpr::Path (rather than always emitting an empty vec). Touch the constructors/parsers that create PpirTy::Named and any clone/convert helpers so the new field is preserved.
410-414:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUpdate this
TypeExpr::Pathtest fixture for the new AST field.This constructor is now missing
associated_type_bindings, so the test module will not compile after theTypeExpr::Pathshape change.🔧 Minimal fix
let type_expr = TypeExpr::Path { segments: vec![Name::new("Fizz")], generic_args: vec![], + associated_type_bindings: vec![], attrs: vec![make_attr("stream.done")], };🤖 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_ppir/src/ty.rs` around lines 410 - 414, The TypeExpr::Path construction in the test fixture is missing the new associated_type_bindings field introduced to the AST shape; update the TypeExpr::Path initializer (the instance created with TypeExpr::Path { segments: vec![Name::new("Fizz")], generic_args: vec![], attrs: vec![make_attr("stream.done")] }) to include associated_type_bindings (e.g., an empty Vec or appropriate default) so the test compiles against the new TypeExpr::Path definition.baml_language/crates/baml_project/src/client_codegen.rs (1)
560-565:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCodegen conversion still drops interface associated bindings.
The new
TirTy::Interfacepayload is destructured here, but onlytype_argssurvive intocg::Ty::Class. Generated client signatures will therefore collapse incompatible types likeIterator<Item = int>andIterator<Item = string>into the same codegen type.🤖 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_project/src/client_codegen.rs` around lines 560 - 565, The pattern matching on TirTy::Interface currently ignores its associated bindings, causing different interface-instantiations (e.g., Iterator<Item=int> vs Item=string) to collapse; update the TirTy::Interface arm in the conversion where TirTy::Interface(qtn, type_args, _, _) is matched so you capture the associated-binding slot (e.g., change to TirTy::Interface(qtn, type_args, assoc_bindings, _)), convert each binding using the same convert_tir_to_codegen_ty (with alias_map and recursive_aliases), and include the converted bindings in the resulting cg::Ty::Class (or the appropriate cg::Ty variant) alongside name_from_qtn(qtn) and the converted type_args so codegen preserves per-interface associated types.baml_language/crates/baml_compiler2_emit/src/lib.rs (1)
988-1036:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPreserve associated bindings in the runtime implementor registry.
This path now sees the expanded
TirTy::Interfaceshape, but it still records onlyiface_type_args. Distinct interface witnesses likeIterator<Item = int>andIterator<Item = string>will collapse to the same reflection entry, soimplements/implementorscan report false positives once BEP-57 types reach runtime reflection.🤖 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 988 - 1036, The code extracts iface_type_args from rule.interface_ty but ignores the Interface variant's associated bindings (the fourth tuple element in baml_compiler2_tir::ty::Ty::Interface(iface_qtn, iface_type_args, _, _)), causing distinct witnesses like Iterator<Item=int> vs Iterator<Item=string> to collapse; fix by capturing that bindings field (e.g. let associated_bindings = /* the unused field */), convert those bindings alongside converted_iface_args (via the same convert_tir2_* helper used for type args), and include the converted associated bindings in the iface_args passed to add_impl (instead of clearing to Vec::new when contains_tir_type_var). Ensure the logic around contains_tir_type_var uses the full combined iface_args (converted_iface_args + converted_associated_bindings) so reflection records preserve associated bindings.baml_language/crates/baml_tests/src/compiler2_tir/mod.rs (1)
72-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRender associated-type bindings in pattern heads.
Both printers now accept extra
Pattern::Classfields via.., but they still only emitgeneric_argsandfields. That means patterns likeSource<Item = int> { value }stringify the same asSource { value }, so the new destructure/narrowing cases inbaml_language/crates/baml_tests/tests/interfaces_associated_types.rslose the binding information that distinguishes them.Also applies to: 1391-1424
🤖 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/src/compiler2_tir/mod.rs` around lines 72 - 100, The Pattern::Class branch currently only prints class, generic_args and fields, so associated-type bindings on the Pattern::Class (the extra field added via .. that contains bindings like Item = int) are dropped; update the printing in this branch (and the similar printer at the 1391-1424 range) to extract the associated-type bindings field from Pattern::Class (commonly named something like associated_types / bindings / assoc), render each binding as "Name = <type>" (use pat_desc or the same type-to-string helper for the RHS), join with ", " and include them when formatting the class head (e.g. alongside generic_args inside the angle brackets or immediately after them) so patterns render as "Source<Item = int> { value }" rather than losing the binding information.baml_language/crates/baml_compiler2_mir/src/lower.rs (2)
238-240:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't drop associated-type constraints when generic args are omitted.
This returns
InterfaceClassGuard::Anybeforerequested_iface_associs checked. A request likeFoo<Item = int>will accept everyFoo<...>candidate here whenever the interface's generic args were omitted, so runtime dispatch andistests can select the wrong 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_compiler2_mir/src/lower.rs` around lines 238 - 240, The early return that yields InterfaceClassGuard::Any when requested_iface_args.is_empty() and !impl_iface_args.is_empty() is dropping associated-type constraints; update the condition so associated constraints are considered — e.g., only return InterfaceClassGuard::Any if requested_iface_args.is_empty() AND requested_iface_assoc.is_empty(), or alternatively move the requested_iface_assoc check before returning Any so that a request like Foo<Item=int> still enforces the assoc constraints; adjust the logic around requested_iface_args, impl_iface_args and requested_iface_assoc in the matching code to ensure associated-type constraints are always validated before returning Any.
7973-8029:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftTyped interface pattern tests still ignore non-class implementors.
interface_implementor_class_guardsonly walksself.interface_implementors, but this file separately tracks primitive / out-of-body implementors inself.interface_type_implementors. The newemit_is_tir_type_branch_innerpath forInterface<...>with args/assoc relies on this helper, sovalue is I<Item = int>can never succeed for rules likeimplements I<Item = int> for int, even though method dispatch resolves those throughinterface_type_implementors.🤖 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 7973 - 8029, interface_implementor_class_guards currently only iterates self.interface_implementors (class implementors) and thus ignores entries in self.interface_type_implementors; update interface_implementor_class_guards to also iterate self.interface_type_implementors.get(iface_tn) and apply the same matching logic used for class impls: compute target_views via interface_closure_type_name_views for each non-class implementor target, compare with requested_views, build the corresponding guard with the same guard-creation path (interface_class_guard_for_args or the equivalent guard generator used for non-class impls) and push the (implementor, guard) into out so emit_is_tir_type_branch_inner can succeed for rules like implements I<Item=int> for int. Ensure you reuse resolve_implements_target_view/ interface_closure_type_name_views/ interface_class_guard_for_args symbols and handle cloning/typing of the implementor identifier consistently with the existing out Vec.baml_language/crates/baml_lsp2_actions/src/check.rs (1)
2967-2987:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDistinct parent instantiations collapse when only associated bindings differ.
collect_interface_members_with_subststill dedups the extends traversal bysegments.clone(). With BEP-57,requires Parent<Item = int>andrequires Parent<Item = string>now produce different interface views but share the same path segments, so the second branch is skipped entirely. That drops member/conflict information for one instantiation and can makeextends/implementsvalidation accept invalid hierarchies.The visited key needs to include resolved interface identity plus the instantiated generic/associated bindings, not just the path text.
Also applies to: 3081-3136
🤖 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_lsp2_actions/src/check.rs` around lines 2967 - 2987, The dedup key for extends traversal currently only uses the interface path/name (visited: HashSet<Vec<Name>>) so different instantiations (e.g., Parent<Item=int vs Parent<Item=string>) are collapsed; update the visited/check logic inside collect_interface_members_with_subst to include the instantiated identity as well — incorporate the resolved substitution/generic & associated-type bindings (the subst/root_type_args or a canonical representation of the instantiated TypeExprs) into the visited key alongside iface.name or segments, update the InterfaceMemberStackEntry handling and where visited.insert and visited.contains are called to use this composite key, and apply the same fix to the analogous code around 3081-3136 so each distinct instantiation is traversed separately.baml_language/crates/baml_compiler2_hir/src/builder.rs (1)
1714-1748:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRecurse into
TypeExpr::Pathwhen checking for$rust_type.
validate_type_expr_phase1()depends ontype_expr_contains_rust(), but this matcher still returnsfalseforTypeExpr::Path. With the new BEP-57 surface, that lets non-builtin code hide$rust_typeinside path generic args or associated-type bindings likeIterator<Item = $rust_type>without tripping the builtin-only diagnostic.Suggested fix
fn type_expr_contains_rust(type_expr: &ast::TypeExpr) -> bool { match type_expr { ast::TypeExpr::Rust { .. } => true, + ast::TypeExpr::Path { + generic_args, + associated_type_bindings, + .. + } => { + generic_args.iter().any(Self::type_expr_contains_rust) + || associated_type_bindings + .iter() + .any(|binding| Self::type_expr_contains_rust(binding.ty.as_ref())) + } ast::TypeExpr::Optional { inner, .. } | ast::TypeExpr::List { inner, .. } => { Self::type_expr_contains_rust(inner) } ast::TypeExpr::Map { key, value, .. } => { Self::type_expr_contains_rust(key) || Self::type_expr_contains_rust(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_compiler2_hir/src/builder.rs` around lines 1714 - 1748, type_expr_contains_rust currently misses Rust types nested inside TypeExpr::Path; update type_expr_contains_rust to handle ast::TypeExpr::Path by recursing into the path's segments' generic arguments and associated-type bindings (inspect each segment's generic arg list, type arguments and any AssociatedTypeBinding types) and call Self::type_expr_contains_rust on those inner TypeExprs so constructs like Iterator<Item = $rust_type> or path generic args are detected; reference the function type_expr_contains_rust and the TypeExpr::Path variant and ensure you also traverse segment bindings and generic arg lists similarly to how Function/Map/Union are handled.baml_language/crates/baml_compiler2_visualization/src/control_flow/from_ast.rs (1)
1163-1167:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd
associated_type_bindingsto theTypeExpr::Pathfixture infrom_ast.rs
ast::TypeExpr::Pathincludes anassociated_type_bindingsfield, but theint_tyunit test fixture inbaml_language/crates/baml_compiler2_visualization/src/control_flow/from_ast.rsconstructsTypeExpr::Pathwithout it, which will failcargo test --lib.Suggested fix
let int_ty = ast::TypeExpr::Path { segments: vec!["int".into()], generic_args: vec![], + associated_type_bindings: vec![], attrs: vec![], };🤖 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_visualization/src/control_flow/from_ast.rs` around lines 1163 - 1167, The test fixture constructing ast::TypeExpr::Path (named int_ty) omits the associated_type_bindings field; update the int_ty construction in from_ast.rs to include associated_type_bindings: vec![] alongside segments, generic_args, and attrs so the struct literal matches ast::TypeExpr::Path's definition and tests compile.baml_language/crates/baml_compiler2_tir/src/generics.rs (1)
533-539:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInfer through interface associated bindings as well as generic args.
This interface arm only zips the positional type arguments. A formal like
Iterator<Item = T>against an actualIterator<Item = int>never bindsT, so associated-type-constrained inference falls through to the cannot-infer path.Proposed fix
- (Ty::Class(fn_name, f_args, _), Ty::Class(an_name, a_args, _)) - | (Ty::Interface(fn_name, f_args, _, _), Ty::Interface(an_name, a_args, _, _)) + (Ty::Class(fn_name, f_args, _), Ty::Class(an_name, a_args, _)) if fn_name == an_name => { for (ft, at) in f_args.iter().zip(a_args.iter()) { infer_bindings_inner(ft, at, bindings, allow_typevar_actuals); } } + ( + Ty::Interface(fn_name, f_args, f_assoc, _), + Ty::Interface(an_name, a_args, a_assoc, _), + ) if fn_name == an_name => { + for (ft, at) in f_args.iter().zip(a_args.iter()) { + infer_bindings_inner(ft, at, bindings, allow_typevar_actuals); + } + + let actual_assoc: FxHashMap<_, _> = a_assoc.iter().cloned().collect(); + for (name, formal_ty) in f_assoc { + if let Some(actual_ty) = actual_assoc.get(name) { + infer_bindings_inner(formal_ty, actual_ty, bindings, allow_typevar_actuals); + } + } + }baml_language/crates/baml_compiler2_tir/src/interfaces.rs (1)
996-1004:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude interface associated bindings in the fast-path guards.
contains_bound_typevarandcontains_generic_function_bindersonly recurse through interface generic args here, notassociated_bindings. That letsmatch_ty_pattern_intotake the normalized-equality shortcut for patterns whose only binders live inIterator<Item = T>, which can skip the actual binding/canonicalization work.Suggested fix
- Ty::Class(_, args, _) | Ty::Interface(_, args, _, _) | Ty::Union(args, _) => args - .iter() - .any(|arg| contains_bound_typevar(arg, generic_params)), + Ty::Class(_, args, _) | Ty::Union(args, _) => args + .iter() + .any(|arg| contains_bound_typevar(arg, generic_params)), + Ty::Interface(_, args, associated_bindings, _) => { + args.iter() + .any(|arg| contains_bound_typevar(arg, generic_params)) + || associated_bindings + .iter() + .any(|(_, ty)| contains_bound_typevar(ty, generic_params)) + } @@ - Ty::Class(_, args, _) | Ty::Interface(_, args, _, _) | Ty::Union(args, _) => { + Ty::Class(_, args, _) | Ty::Union(args, _) => { args.iter().any(contains_generic_function_binders) } + Ty::Interface(_, args, associated_bindings, _) => { + args.iter().any(contains_generic_function_binders) + || associated_bindings + .iter() + .any(|(_, ty)| contains_generic_function_binders(ty)) + }Also applies to: 1028-1036
🤖 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/interfaces.rs` around lines 996 - 1004, The fast-path guards in contains_bound_typevar and contains_generic_function_binders miss interface associated_bindings, so update the Ty::Interface arm (and the equivalent arm used at the other location noted) to also iterate over interface's associated_bindings and recurse into each binding when checking for bound type variables or generic function binders; specifically, for Ty::Interface(_, args, associated_bindings, _) call contains_bound_typevar/contains_generic_function_binders on each associated binding in addition to the existing generic args checks so that patterns with binders only in associated types are detected.
🧹 Nitpick comments (1)
baml_language/crates/baml_compiler_parser/src/parser.rs (1)
476-542: ⚡ Quick winAdd parser-unit coverage for the new projection/binding split.
These helpers now own the grammar disambiguation for
(T as I).Itemand<Item = Type>, but this file still has no direct unit tests for them. Please add parser-level cases for projection parsing, named associated bindings inTYPE_ARGS, and one rejected impl-bodytype Item extends ...form so parser regressions fail here before they surface in broader compiler suites.As per coding guidelines,
**/*.rs: Prefer writing Rust unit tests over integration tests where possible.Also applies to: 2271-2318, 5551-5592
🤖 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_compiler_parser/src/parser.rs` around lines 476 - 542, Add Rust unit tests in parser.rs that exercise the new projection/binding disambiguation: create cases that assert looks_like_associated_type_projection correctly recognizes `(T as I).Item` and rejects non-projections; add tests for parsing TYPE_ARGS with named associated bindings like `<Item = Type>` (use the parser entry that handles type argument lists) and a negative test that ensures an impl-body form like `type Item extends ...` is rejected by the parser; ensure tests call the specific parser entry points used for type argument parsing and the helper looks_like_associated_type_projection so regressions fail locally.
🤖 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_compiler_parser/src/parser.rs`:
- Around line 3058-3075: The parser currently allows an `extends` bound on
impl-side associated types because `require_binding` only enforces the `=`
branch; update the `if p.at(TokenKind::Extends)` branch to reject `extends` when
parsing impl-associated-type witnesses (i.e., when `require_binding` is true).
Concretely, inside the `if p.at(TokenKind::Extends)` block check `if
require_binding` and call `p.error_unexpected_token` with a message like "impl
associated types must be `= Type`; `extends` not allowed", then still `p.bump()`
and attempt to recover by parsing the type (use the existing
`p.is_at_type_start()` / `p.parse_type()` or `p.error_unexpected_token` logic)
so the parser can continue. This uses the existing symbols
`p.at(TokenKind::Extends)`, `require_binding`, `p.error_unexpected_token`,
`p.bump()`, `p.is_at_type_start()`, and `p.parse_type()`.
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 290-297: The loop in lower.rs re-checks associated types with
strict equality causing requested TypeVars to fail matches; modify the loop over
requested_iface_assoc so that when requested_ty is an unbound/non-class TypeVar
(the wildcard case handled earlier in infer_interface_class_bindings) you skip
the equality check and accept any impl_ty, otherwise continue to call
baml_compiler2_tir::normalize::is_same_normalized_type(impl_ty, requested_ty,
aliases) against the found impl_ty from substituted_assoc; reference the
variables requested_iface_assoc, substituted_assoc and the
normalize::is_same_normalized_type call to locate and change that conditional
logic.
In `@baml_language/crates/baml_compiler2_tir/src/generics.rs`:
- Around line 168-182: substitute_type_expr currently matches TypeExpr::Path
with a single-segment and returns bindings.get(&segments[0]).cloned(), which
strips away any generic_args or associated_type_bindings (e.g., T<U> or
T<Item=...>) and bypasses subsequent lowering/validation; update the match arms
in substitute_type_expr so that the single-segment arm only returns the binding
when both generic_args and associated_type_bindings are empty, and otherwise
preserve the original TypeExpr (or proceed into the new lowering/validation
path) for cases where generic_args or associated_type_bindings are present;
specifically adjust the TypeExpr::Path patterns that reference segments,
generic_args, associated_type_bindings and the code that uses bindings.get(...)
so single-segment paths with args/bindings are not collapsed to the bare
binding.
In `@baml_language/crates/baml_compiler2_tir/src/inference.rs`:
- Around line 805-851: The current code computes defaults for
iface_data.associated_types before inserting explicit bindings from
explicit_bindings, which allows defaults to see stale bindings; instead, first
process explicit_bindings (for each binding with a type_expr: compute resolved =
substitute_self_in if self_replacement is Some, lower it via
crate::generics::lower_type_expr_with_generics collecting binding_diags and
calling builder.report_at_span(diag, te.span), then insert into type_bindings
and iface_type_bindings), and only after all explicit bindings are inserted
iterate iface_data.associated_types and lower defaults for those names not
already present (using the same lowering path and reporting diagnostics at
default.span) so explicit witnesses take precedence over derived defaults
(affecting symbols: explicit_bindings, associated_types, type_bindings,
iface_type_bindings, substitute_self_in, lower_type_expr_with_generics,
builder.report_at_span).
In `@baml_language/crates/baml_compiler2_tir/src/interfaces.rs`:
- Around line 1159-1169: The associated-binding/default lowering is using the
caller's pkg_items instead of the interface's defining package items; compute
the interface's package items with baml_compiler2_ppir::package_items(...) for
the interface's package id (derived from iface_data / iface_loc.file(db) or the
qtn.package() used by resolve_path_to_interface_identity) and pass that
package_items value in place of the existing pkg_items when calling
lower_interface_associated_bindings (and the other lowering helpers at the other
call sites mentioned) so the defaults/bindings resolve against the interface's
own symbol table.
- Around line 541-583: The associated-type lowering loop does not update the
lowering environment, so later associated types can't see earlier resolved
types; when you compute the resolved Ty for an associated type (the result of
generics::lower_type_expr_with_generics for either a binding from
block_associated_bindings or assoc.default), insert or update that entry in the
bindings map (key = assoc.name.clone()) before returning it from the iterator so
subsequent iterations see the resolved type; modify the closure in
iface.associated_types.iter().filter_map(...) to store the resolved Ty into
bindings (the same Ty::TypeVar/returned Ty) immediately after lowering and then
return the (assoc.name.clone(), resolved_ty).
In `@baml_language/crates/baml_compiler2_tir/src/normalize.rs`:
- Around line 598-612: The associated_bindings for Ty::Interface are collected
in an order-sensitive Vec which can produce non-deterministic Normalized types;
modify the Ty::Interface arm in normalize_impl so that before mapping you sort
the associated_bindings by their name (e.g., sort_by(|(a, _), (b, _)| a.cmp(b)))
and then map/normalize them into the Vec; ensure the same stable ordering is
used for the args if applicable and return StructuralTy::Interface(qn.clone(),
args, associated_bindings) with the sorted, normalized bindings.
In `@baml_language/crates/baml_fmt/src/ast/declarations.rs`:
- Around line 1032-1034: The formatter currently maps
SyntaxKind::IMPLEMENTS_BLOCK to ClassItem::Unknown (in declarations.rs), causing
implements bodies to be re-emitted raw; update the parser branch that handles
SyntaxKind::IMPLEMENTS_BLOCK to construct a proper ClassItem variant (or extend
ClassItem with an ImplementsBody/ImplementsItem) that contains the parsed inner
nodes (associated-type witnesses and other members) instead of text_range(), and
update the corresponding printer/pretty-printer logic that handles ClassItem
(and the codepaths mentioned around the other region ~1298-1352) to recursively
format and normalize those inner associated-type witness nodes; ensure the new
variant uses the same node types as other class member handling so existing
formatting helpers apply.
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 1246-1268: The subtype check is lowering the bound using only
generic_param substitution (via substitute_type_vars/generic_subst) which leaves
associated-type names unresolved; fix by constructing the full associated-type
substitution (merge associated defaults and already-supplied bindings—reuse
associated_type_subst_from_bindings or associated_type_subst_from_type_args
logic) and apply that substitution to the bound expression before calling
lower_type_expr_in_ns; specifically, when computing bound_expr and before
calling baml_compiler2_tir::lower_type_expr::lower_type_expr_in_ns for bound_ty,
extend generic_subst with the associated-type substitution (the same change
should be mirrored in the other affected sites mentioned: the default-bound
validation and interface-type binding validation ranges).
- Around line 1220-1235: The diagnostic currently anchors the missing associated
type error to assoc.name_span with the implementor file_id, which can point to
the wrong file; change the primary span to the implementor’s implements/impl
block span (e.g., the variable holding the implementor target span in the
surrounding code, such as impl_span or implements_span) when building the
Diagnostic::error in the loop over iface.associated_types, and instead add the
interface declaration span (assoc.name_span with the interface’s file_id) as
related info on the diagnostic so LSP shows the marker in the implementor file
and points back to the interface declaration.
In `@baml_language/crates/baml_tests/src/compiler2_tir/mod.rs`:
- Around line 1346-1359: The AssociatedTypeProjection branch loses
generic/associated-type instantiations because it calls type_expr_to_string_hir
for the interface while type_expr_to_string_hir's TypeExpr::Path arm drops
generic_args and associated_type_bindings; update the rendering so qualifier
instantiations are preserved: either change type_expr_to_string_hir to include
generic_args and associated_type_bindings in its TypeExpr::Path arm, or in the
TypeExpr::AssociatedTypeProjection arm render the interface with a helper that
includes those fields, so `(Document as Codec<TextFormat>).Output` and
`(Document as Codec<CodeFormat>).Output` remain distinct (refer to
TypeExpr::AssociatedTypeProjection, type_expr_to_string_hir, TypeExpr::Path,
generic_args, associated_type_bindings).
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_emit/src/lib.rs`:
- Around line 988-1036: The code extracts iface_type_args from rule.interface_ty
but ignores the Interface variant's associated bindings (the fourth tuple
element in baml_compiler2_tir::ty::Ty::Interface(iface_qtn, iface_type_args, _,
_)), causing distinct witnesses like Iterator<Item=int> vs Iterator<Item=string>
to collapse; fix by capturing that bindings field (e.g. let associated_bindings
= /* the unused field */), convert those bindings alongside converted_iface_args
(via the same convert_tir2_* helper used for type args), and include the
converted associated bindings in the iface_args passed to add_impl (instead of
clearing to Vec::new when contains_tir_type_var). Ensure the logic around
contains_tir_type_var uses the full combined iface_args (converted_iface_args +
converted_associated_bindings) so reflection records preserve associated
bindings.
In `@baml_language/crates/baml_compiler2_hir/src/builder.rs`:
- Around line 1714-1748: type_expr_contains_rust currently misses Rust types
nested inside TypeExpr::Path; update type_expr_contains_rust to handle
ast::TypeExpr::Path by recursing into the path's segments' generic arguments and
associated-type bindings (inspect each segment's generic arg list, type
arguments and any AssociatedTypeBinding types) and call
Self::type_expr_contains_rust on those inner TypeExprs so constructs like
Iterator<Item = $rust_type> or path generic args are detected; reference the
function type_expr_contains_rust and the TypeExpr::Path variant and ensure you
also traverse segment bindings and generic arg lists similarly to how
Function/Map/Union are handled.
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 238-240: The early return that yields InterfaceClassGuard::Any
when requested_iface_args.is_empty() and !impl_iface_args.is_empty() is dropping
associated-type constraints; update the condition so associated constraints are
considered — e.g., only return InterfaceClassGuard::Any if
requested_iface_args.is_empty() AND requested_iface_assoc.is_empty(), or
alternatively move the requested_iface_assoc check before returning Any so that
a request like Foo<Item=int> still enforces the assoc constraints; adjust the
logic around requested_iface_args, impl_iface_args and requested_iface_assoc in
the matching code to ensure associated-type constraints are always validated
before returning Any.
- Around line 7973-8029: interface_implementor_class_guards currently only
iterates self.interface_implementors (class implementors) and thus ignores
entries in self.interface_type_implementors; update
interface_implementor_class_guards to also iterate
self.interface_type_implementors.get(iface_tn) and apply the same matching logic
used for class impls: compute target_views via interface_closure_type_name_views
for each non-class implementor target, compare with requested_views, build the
corresponding guard with the same guard-creation path
(interface_class_guard_for_args or the equivalent guard generator used for
non-class impls) and push the (implementor, guard) into out so
emit_is_tir_type_branch_inner can succeed for rules like implements I<Item=int>
for int. Ensure you reuse resolve_implements_target_view/
interface_closure_type_name_views/ interface_class_guard_for_args symbols and
handle cloning/typing of the implementor identifier consistently with the
existing out Vec.
In `@baml_language/crates/baml_compiler2_ppir/src/ty.rs`:
- Around line 321-329: PpirTy::Named currently drops associated type bindings
when converting back to a TypeExpr; update the PpirTy definition to include a
field for associated_type_bindings (the same shape used by TypeExpr::Path),
propagate that field wherever PpirTy::Named is constructed/parsed, and modify
PpirTy::to_type_expr to pass that stored associated_type_bindings into the
produced TypeExpr::Path (rather than always emitting an empty vec). Touch the
constructors/parsers that create PpirTy::Named and any clone/convert helpers so
the new field is preserved.
- Around line 410-414: The TypeExpr::Path construction in the test fixture is
missing the new associated_type_bindings field introduced to the AST shape;
update the TypeExpr::Path initializer (the instance created with TypeExpr::Path
{ segments: vec![Name::new("Fizz")], generic_args: vec![], attrs:
vec![make_attr("stream.done")] }) to include associated_type_bindings (e.g., an
empty Vec or appropriate default) so the test compiles against the new
TypeExpr::Path definition.
In `@baml_language/crates/baml_compiler2_tir/src/interfaces.rs`:
- Around line 996-1004: The fast-path guards in contains_bound_typevar and
contains_generic_function_binders miss interface associated_bindings, so update
the Ty::Interface arm (and the equivalent arm used at the other location noted)
to also iterate over interface's associated_bindings and recurse into each
binding when checking for bound type variables or generic function binders;
specifically, for Ty::Interface(_, args, associated_bindings, _) call
contains_bound_typevar/contains_generic_function_binders on each associated
binding in addition to the existing generic args checks so that patterns with
binders only in associated types are detected.
In
`@baml_language/crates/baml_compiler2_visualization/src/control_flow/from_ast.rs`:
- Around line 1163-1167: The test fixture constructing ast::TypeExpr::Path
(named int_ty) omits the associated_type_bindings field; update the int_ty
construction in from_ast.rs to include associated_type_bindings: vec![]
alongside segments, generic_args, and attrs so the struct literal matches
ast::TypeExpr::Path's definition and tests compile.
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 2967-2987: The dedup key for extends traversal currently only uses
the interface path/name (visited: HashSet<Vec<Name>>) so different
instantiations (e.g., Parent<Item=int vs Parent<Item=string>) are collapsed;
update the visited/check logic inside collect_interface_members_with_subst to
include the instantiated identity as well — incorporate the resolved
substitution/generic & associated-type bindings (the subst/root_type_args or a
canonical representation of the instantiated TypeExprs) into the visited key
alongside iface.name or segments, update the InterfaceMemberStackEntry handling
and where visited.insert and visited.contains are called to use this composite
key, and apply the same fix to the analogous code around 3081-3136 so each
distinct instantiation is traversed separately.
In `@baml_language/crates/baml_project/src/client_codegen.rs`:
- Around line 560-565: The pattern matching on TirTy::Interface currently
ignores its associated bindings, causing different interface-instantiations
(e.g., Iterator<Item=int> vs Item=string) to collapse; update the
TirTy::Interface arm in the conversion where TirTy::Interface(qtn, type_args, _,
_) is matched so you capture the associated-binding slot (e.g., change to
TirTy::Interface(qtn, type_args, assoc_bindings, _)), convert each binding using
the same convert_tir_to_codegen_ty (with alias_map and recursive_aliases), and
include the converted bindings in the resulting cg::Ty::Class (or the
appropriate cg::Ty variant) alongside name_from_qtn(qtn) and the converted
type_args so codegen preserves per-interface associated types.
In `@baml_language/crates/baml_tests/src/compiler2_tir/mod.rs`:
- Around line 72-100: The Pattern::Class branch currently only prints class,
generic_args and fields, so associated-type bindings on the Pattern::Class (the
extra field added via .. that contains bindings like Item = int) are dropped;
update the printing in this branch (and the similar printer at the 1391-1424
range) to extract the associated-type bindings field from Pattern::Class
(commonly named something like associated_types / bindings / assoc), render each
binding as "Name = <type>" (use pat_desc or the same type-to-string helper for
the RHS), join with ", " and include them when formatting the class head (e.g.
alongside generic_args inside the angle brackets or immediately after them) so
patterns render as "Source<Item = int> { value }" rather than losing the binding
information.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler_parser/src/parser.rs`:
- Around line 476-542: Add Rust unit tests in parser.rs that exercise the new
projection/binding disambiguation: create cases that assert
looks_like_associated_type_projection correctly recognizes `(T as I).Item` and
rejects non-projections; add tests for parsing TYPE_ARGS with named associated
bindings like `<Item = Type>` (use the parser entry that handles type argument
lists) and a negative test that ensures an impl-body form like `type Item
extends ...` is rejected by the parser; ensure tests call the specific parser
entry points used for type argument parsing and the helper
looks_like_associated_type_projection so regressions fail locally.
🪄 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: 23999614-9ffe-49af-a247-6aff372e5213
⛔ Files ignored due to path filters (2)
baml_language/crates/baml_tests/snapshots/compiles/patterns_class_destructure_namespaces/baml_tests__compiles__patterns_class_destructure_namespaces__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__02_parser__patterns_class_destructure.snapis excluded by!**/*.snap
📒 Files selected for processing (40)
baml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/auto_derive_json.rsbaml_language/crates/baml_compiler2_ast/src/companions.rsbaml_language/crates/baml_compiler2_ast/src/lower_cst.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_compiler2_ast/src/lower_type_expr.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/contributions.rsbaml_language/crates/baml_compiler2_hir/src/item_tree.rsbaml_language/crates/baml_compiler2_hir/src/signature.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_ppir/src/lib.rsbaml_language/crates/baml_compiler2_ppir/src/ty.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/exhaustiveness.rsbaml_language/crates/baml_compiler2_tir/src/generics.rsbaml_language/crates/baml_compiler2_tir/src/inference.rsbaml_language/crates/baml_compiler2_tir/src/interfaces.rsbaml_language/crates/baml_compiler2_tir/src/lower_type_expr.rsbaml_language/crates/baml_compiler2_tir/src/normalize.rsbaml_language/crates/baml_compiler2_tir/src/package_interface.rsbaml_language/crates/baml_compiler2_tir/src/ty.rsbaml_language/crates/baml_compiler2_visualization/src/control_flow/from_ast.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_compiler_syntax/src/syntax_kind.rsbaml_language/crates/baml_fmt/src/ast/declarations.rsbaml_language/crates/baml_fmt/src/ast/expressions.rsbaml_language/crates/baml_fmt/src/ast/pattern.rsbaml_language/crates/baml_fmt/src/ast/tokens.rsbaml_language/crates/baml_fmt/src/ast/types.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_lsp2_actions/src/utils.rsbaml_language/crates/baml_project/src/client_codegen.rsbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase5.rsbaml_language/crates/baml_tests/tests/interfaces_associated_types.rsbaml_language/crates/bex_project/src/bex_lsp/multi_project/request.rs
👮 Files not reviewed due to content moderation or server errors (1)
- baml_language/crates/baml_compiler2_tir/src/builder.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/generics.rs (1)
690-699:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
erase_unresolved_typevarsdoesn't recurse intoTy::Interfacetype args or associated bindings.
Ty::Interfacefalls through toother => other.clone(), so nested types in itstype_argsandassociated_bindingsare not walked. This is inconsistent witherase_typevars_matching(lines 721-731) which properly handlesTy::Interface. If an interface type with associated bindings reaches this function, nested type vars won't be processed.Proposed fix
Ty::Class(name, type_args, attr) => Ty::Class( name.clone(), type_args .iter() .map(|t| erase_unresolved_typevars(t, diagnostics)) .collect(), attr.clone(), ), + Ty::Interface(name, type_args, associated_bindings, attr) => Ty::Interface( + name.clone(), + type_args + .iter() + .map(|t| erase_unresolved_typevars(t, diagnostics)) + .collect(), + associated_bindings + .iter() + .map(|(n, t)| (n.clone(), erase_unresolved_typevars(t, diagnostics))) + .collect(), + attr.clone(), + ), other => other.clone(),🤖 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/generics.rs` around lines 690 - 699, The match in erase_unresolved_typevars fails to recurse into Ty::Interface, so update erase_unresolved_typevars to handle the Ty::Interface variant (similar to how Ty::Class and erase_typevars_matching do): add a Ty::Interface arm that clones the interface name and maps erase_unresolved_typevars(diagnostics) over its type_args and also over each type in associated_bindings (rebuilding the bindings with the transformed types), ensuring diagnostics is passed through; leave other arms unchanged.
🧹 Nitpick comments (1)
baml_language/crates/baml_fmt/src/ast/declarations.rs (1)
1491-1506: ⚡ Quick winInconsistent delimiter handling compared to
ClassItem::Field.
ClassItem::Field(lines 1728-1743) always normalizes to a trailing comma, butImplementsItemvariants handle delimiters inconsistently:
AssociatedTypeandFieldLinkignore their delimiter entirelyFieldonly prints if the original was a comma, skipping semicolons and noneThis could cause formatted output to lose delimiters or produce non-idempotent results.
♻️ Suggested fix for consistent delimiter handling
impl Printable for ImplementsItem { fn print(&self, shape: Shape, printer: &mut Printer) -> PrintInfo { match self { - ImplementsItem::AssociatedType(decl, _) => decl.print(shape, printer), - ImplementsItem::FieldLink(link, _) => link.print(shape, printer), + ImplementsItem::AssociatedType(decl, delimiter) => { + let info = decl.print(shape, printer); + match delimiter { + Some(ClassFieldDelimiter::Comma(comma)) => printer.print_raw_token(comma), + Some(ClassFieldDelimiter::Semicolon(_)) | None => printer.print_str(","), + } + info + } + ImplementsItem::FieldLink(link, delimiter) => { + let info = link.print(shape, printer); + match delimiter { + Some(ClassFieldDelimiter::Comma(comma)) => printer.print_raw_token(comma), + Some(ClassFieldDelimiter::Semicolon(_)) | None => printer.print_str(","), + } + info + } ImplementsItem::Field(field, delimiter) => { let info = field.print(shape, printer); match delimiter { Some(ClassFieldDelimiter::Comma(comma)) => printer.print_raw_token(comma), - Some(ClassFieldDelimiter::Semicolon(_)) | None => {} + Some(ClassFieldDelimiter::Semicolon(_)) | None => printer.print_str(","), } info } ImplementsItem::Function(function) => function.print(shape, printer), } }🤖 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_fmt/src/ast/declarations.rs` around lines 1491 - 1506, The ImplementsItem::print implementation is inconsistent with ClassItem::Field delimiter normalization: update impl Printable for ImplementsItem (the print method handling ImplementsItem::AssociatedType, ::FieldLink and ::Field) so all variants normalize to a trailing comma like ClassItem::Field does—i.e., consult the variant's delimiter (ClassFieldDelimiter) and always emit a trailing comma token (or the canonical comma form) after printing the item regardless of whether the original delimiter was Comma, Semicolon, or None; ensure AssociatedType and FieldLink also respect and print that normalized delimiter and that Field prints the normalized trailing comma even when delimiter was Semicolon or None.
🤖 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_tests/src/compiler2_tir/mod.rs`:
- Around line 1266-1269: The code is incorrectly qualifying every one-segment
Path by checking `segments.len() == 1 || local_type_names.contains(first)`,
causing non-local single-segment names to be package-qualified; change the
condition to only qualify when the single segment is a known local type (e.g.,
use `segments.len() == 1 && local_type_names.contains(first)`), so update the
logic that builds `rendered` (the branch that uses `pkg_prefix` and `path`) to
mirror the behavior of `qualify_type_name` and only apply `pkg_prefix` when
`first` is in `local_type_names`.
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_tir/src/generics.rs`:
- Around line 690-699: The match in erase_unresolved_typevars fails to recurse
into Ty::Interface, so update erase_unresolved_typevars to handle the
Ty::Interface variant (similar to how Ty::Class and erase_typevars_matching do):
add a Ty::Interface arm that clones the interface name and maps
erase_unresolved_typevars(diagnostics) over its type_args and also over each
type in associated_bindings (rebuilding the bindings with the transformed
types), ensuring diagnostics is passed through; leave other arms unchanged.
---
Nitpick comments:
In `@baml_language/crates/baml_fmt/src/ast/declarations.rs`:
- Around line 1491-1506: The ImplementsItem::print implementation is
inconsistent with ClassItem::Field delimiter normalization: update impl
Printable for ImplementsItem (the print method handling
ImplementsItem::AssociatedType, ::FieldLink and ::Field) so all variants
normalize to a trailing comma like ClassItem::Field does—i.e., consult the
variant's delimiter (ClassFieldDelimiter) and always emit a trailing comma token
(or the canonical comma form) after printing the item regardless of whether the
original delimiter was Comma, Semicolon, or None; ensure AssociatedType and
FieldLink also respect and print that normalized delimiter and that Field prints
the normalized trailing comma even when delimiter was Semicolon or None.
🪄 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: 0bb767de-8492-4e72-85ee-185bec3642d2
⛔ Files ignored due to path filters (13)
baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/closures/baml_tests__compiles__closures__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_field_chain/baml_tests__compiles__generic_field_chain__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_cross_namespace_static_call/baml_tests__compiles__json_cross_namespace_static_call__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_to_from_string_composite_generic/baml_tests__compiles__json_to_from_string_composite_generic__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_class_destructure_namespaces/baml_tests__compiles__patterns_class_destructure_namespaces__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_llm_inferred_typeargs/baml_tests__compiles__stream_llm_inferred_typeargs__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_class_destructure_no_type_args/baml_tests__diagnostic_errors__generic_class_destructure_no_type_args__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generics/baml_tests__diagnostic_errors__generics__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/is_pattern_negative/baml_tests__diagnostic_errors__is_pattern_negative__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/json_static_method_arity/baml_tests__diagnostic_errors__json_static_method_arity__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure_namespaces/baml_tests__diagnostic_errors__patterns_class_destructure_namespaces__03_hir.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
baml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/generics.rsbaml_language/crates/baml_compiler2_tir/src/inference.rsbaml_language/crates/baml_compiler2_tir/src/interfaces.rsbaml_language/crates/baml_compiler2_tir/src/normalize.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_fmt/src/ast/declarations.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_tests/tests/interfaces_associated_types.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- baml_language/crates/baml_compiler2_tir/src/normalize.rs
- baml_language/crates/baml_compiler2_tir/src/inference.rs
- baml_language/crates/baml_tests/tests/interfaces_associated_types.rs
- baml_language/crates/baml_compiler_parser/src/parser.rs
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_compiler2_ast/src/lib.rs (1)
391-436: 💤 Low valueTest appears unrelated to associated types.
This test validates reserved LLM client parameter handling, which doesn't seem related to the BEP-57 associated types feature described in the PR objectives. The test itself is technically correct and comprehensive, but it may represent scope creep or an unrelated bugfix bundled into this PR.
If this test is addressing a separate issue, consider moving it to a focused PR or adding a comment explaining its connection to associated types.
🤖 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/lib.rs` around lines 391 - 436, The test llm_function_user_client_param_is_reserved in baml_compiler2_ast::lib.rs appears unrelated to the BEP-57 associated types work; either move this test to a separate focused PR (or test module) that targets reserved LLM client param behavior, or keep it here but add a brief comment above the test explaining its relevance to the current PR and referencing the associated issue/commit; ensure the test function name llm_function_user_client_param_is_reserved and the checked function Extract are preserved so reviewers can locate and understand the intent.
🤖 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/baml_compiler2_ast/src/lib.rs`:
- Around line 391-436: The test llm_function_user_client_param_is_reserved in
baml_compiler2_ast::lib.rs appears unrelated to the BEP-57 associated types
work; either move this test to a separate focused PR (or test module) that
targets reserved LLM client param behavior, or keep it here but add a brief
comment above the test explaining its relevance to the current PR and
referencing the associated issue/commit; ensure the test function name
llm_function_user_client_param_is_reserved and the checked function Extract are
preserved so reviewers can locate and understand the intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6190da48-3354-42c8-88a5-fa13602c08f0
⛔ Files ignored due to path filters (23)
baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/closures/baml_tests__compiles__closures__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_field_chain/baml_tests__compiles__generic_field_chain__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_cross_namespace_static_call/baml_tests__compiles__json_cross_namespace_static_call__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_to_from_string_composite_generic/baml_tests__compiles__json_to_from_string_composite_generic__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_to_from_string_generic_forwarding/baml_tests__compiles__json_to_from_string_generic_forwarding__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_to_from_string_three_level/baml_tests__compiles__json_to_from_string_three_level__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_class_destructure_namespaces/baml_tests__compiles__patterns_class_destructure_namespaces__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_crossfile/baml_tests__compiles__stream_crossfile__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_llm_inferred_typeargs/baml_tests__compiles__stream_llm_inferred_typeargs__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/basic_types/baml_tests__diagnostic_errors__basic_types__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_call_ambiguity/baml_tests__diagnostic_errors__generic_call_ambiguity__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_class_destructure_no_type_args/baml_tests__diagnostic_errors__generic_class_destructure_no_type_args__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generics/baml_tests__diagnostic_errors__generics__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/is_pattern_negative/baml_tests__diagnostic_errors__is_pattern_negative__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/json_static_method_arity/baml_tests__diagnostic_errors__json_static_method_arity__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_bare_name_rejected/baml_tests__diagnostic_errors__namespaces_bare_name_rejected__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure_namespaces/baml_tests__diagnostic_errors__patterns_class_destructure_namespaces__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_aliases/baml_tests__diagnostic_errors__type_aliases__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/unknown_type_error/baml_tests__diagnostic_errors__unknown_type_error__03_hir.snapis excluded by!**/*.snap
📒 Files selected for processing (22)
baml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/companions.rsbaml_language/crates/baml_compiler2_ast/src/lib.rsbaml_language/crates/baml_compiler2_ast/src/lower_cst.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_ppir/src/lib.rsbaml_language/crates/baml_compiler2_ppir/src/ty.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_visualization/src/control_flow/from_ast.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_compiler_syntax/src/syntax_kind.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_generic_requires_cycle_changes_args.bamlbaml_language/crates/baml_project/src/client_codegen.rsbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/test_complex_models.pybaml_language/sdk_tests/crates/typescript_node/type_shapes/customizable/roundtrip_complex_models.test.tsbaml_language/sdk_tests/fixtures/type_shapes/baml_src/ns_complex_models/types.baml
💤 Files with no reviewable changes (3)
- baml_language/sdk_tests/crates/typescript_node/type_shapes/customizable/roundtrip_complex_models.test.ts
- baml_language/sdk_tests/fixtures/type_shapes/baml_src/ns_complex_models/types.baml
- baml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/test_complex_models.py
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/interface/error_generic_requires_cycle_changes_args.baml
🚧 Files skipped from review as they are similar to previous changes (12)
- baml_language/crates/baml_compiler2_ast/src/companions.rs
- baml_language/crates/baml_project/src/client_codegen.rs
- baml_language/crates/baml_compiler2_emit/src/lib.rs
- baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs
- baml_language/crates/baml_compiler2_ppir/src/lib.rs
- baml_language/crates/baml_compiler2_hir/src/builder.rs
- baml_language/crates/baml_compiler_syntax/src/syntax_kind.rs
- baml_language/crates/baml_compiler2_ast/src/lower_cst.rs
- baml_language/crates/baml_compiler2_ast/src/ast.rs
- baml_language/crates/baml_tests/src/compiler2_tir/mod.rs
- baml_language/crates/baml_compiler_syntax/src/ast.rs
- baml_language/crates/baml_compiler_parser/src/parser.rs
aaronvg
enabled auto-merge
June 3, 2026 07:11
aaronvg
disabled auto-merge
June 3, 2026 07:14
aaronvg
enabled auto-merge
June 3, 2026 07:14
aaronvg
enabled auto-merge
June 3, 2026 15:17
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/interfaces.rs (1)
769-778:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMatch interface associated bindings from the pattern side, not the concrete side.
This loop makes
Ty::Interfacematching asymmetric:Readablefails to matchReadable<Item = int>, whileReadable<Item = int>can match a concreteReadablewith no binding at all. That will produce false positives/negatives when rule matching depends on associated-type constraints.Suggested fix
- for (name, concrete_ty) in c_assoc { - let (_, pattern_ty) = p_assoc.iter().find(|(p_name, _)| p_name == name)?; - match_ty_pattern_into(pattern_ty, concrete_ty, generic_params, aliases, bindings)?; + for (name, pattern_ty) in p_assoc { + let (_, concrete_ty) = c_assoc.iter().find(|(c_name, _)| c_name == name)?; + match_ty_pattern_into(pattern_ty, concrete_ty, generic_params, aliases, bindings)?; } Some(())🤖 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/interfaces.rs` around lines 769 - 778, The current Ty::Interface branch iterates over c_assoc causing asymmetric matching; change it to iterate over p_assoc (the pattern's associated bindings) and for each (name, pattern_ty) look up the corresponding concrete_ty in c_assoc (e.g., via find or get), then call match_ty_pattern_into(pattern_ty, concrete_ty, generic_params, aliases, bindings) for each; update the block handling (Ty::Interface(p_qtn, p_args, p_assoc, _), Ty::Interface(...)) accordingly so matching uses p_assoc as the source of expected associated bindings and c_assoc only as the lookup target.
🧹 Nitpick comments (1)
baml_language/crates/baml_type/src/template.rs (1)
85-97: ⚡ Quick winAdd unit coverage for the new
TyTemplate::Interfacepaths.
substitute,is_fully_concrete, andDisplaynow each have interface-specific logic, but this module still only exercises the older variants. A couple of unit tests for something likeInterface<#0, Item = string>would lock in both associated-binding substitution and the rendered<...>format.Proposed unit tests
#[cfg(test)] mod tests { use super::*; @@ fn class_no_args_is_fully_concrete() { let tmpl = TyTemplate::Class(TypeName::local(crate::Name::new("User")), vec![]); assert!(tmpl.is_fully_concrete()); let result = tmpl.substitute(&[]); assert_eq!( result, Ty::Class( TypeName::local(crate::Name::new("User")), vec![], crate::TyAttr::default() ) ); } + + #[test] + fn interface_template_substitutes_associated_bindings() { + let tmpl = TyTemplate::Interface( + TypeName::local(crate::Name::new("Iterable")), + vec![TyTemplate::TypeArgRef(0)], + vec![( + crate::Name::new("Item"), + TyTemplate::Concrete(Ty::string()), + )], + ); + let result = tmpl.substitute(&[Ty::int()]); + assert_eq!( + result, + Ty::Interface( + TypeName::local(crate::Name::new("Iterable")), + vec![Ty::int()], + vec![(crate::Name::new("Item"), Ty::string())], + crate::TyAttr::default(), + ) + ); + assert!(!tmpl.is_fully_concrete()); + } + + #[test] + fn interface_template_display_includes_args_and_assoc_bindings() { + let tmpl = TyTemplate::Interface( + TypeName::local(crate::Name::new("Iterable")), + vec![TyTemplate::Concrete(Ty::int())], + vec![( + crate::Name::new("Item"), + TyTemplate::Concrete(Ty::string()), + )], + ); + assert_eq!(tmpl.to_string(), "Iterable<int, Item = string>"); + } }As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible."
Also applies to: 113-173
🤖 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_type/src/template.rs` around lines 85 - 97, Add unit tests that exercise the TyTemplate::Interface variant to cover its substitute, is_fully_concrete, and Display logic: create a template like Interface(name, vec![Ty::Param(0) or similar placeholder], vec![(“Item”, Ty::Primitive(String))]) and assert that substitute with a concrete type replaces the parameter in args and associated_bindings, is_fully_concrete returns true/false appropriately before and after substitution, and Display renders the interface using the expected "<...>" generic syntax; place tests alongside existing unit tests in this module and target the functions/methods substitute, is_fully_concrete, and the Display impl for TyTemplate/Ty so the new Interface paths are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baml_language/crates/bex_engine/src/conversion.rs`:
- Around line 177-185: The validation currently uses the erased
class_field.field_type instead of the substituted per-instance type derived from
class_field.field_template and instance.class_type_args, so host-returned
generics can bypass proper checking; update validate_host_return_schema (and any
validation path that checks class fields) to substitute the field_template with
instance.class_type_args and validate against that substituted field_type (the
same value passed into convert_vm_value_to_external_with_type) rather than the
erased class_field.field_type to ensure host values are validated against the
real instantiated type.
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_tir/src/interfaces.rs`:
- Around line 769-778: The current Ty::Interface branch iterates over c_assoc
causing asymmetric matching; change it to iterate over p_assoc (the pattern's
associated bindings) and for each (name, pattern_ty) look up the corresponding
concrete_ty in c_assoc (e.g., via find or get), then call
match_ty_pattern_into(pattern_ty, concrete_ty, generic_params, aliases,
bindings) for each; update the block handling (Ty::Interface(p_qtn, p_args,
p_assoc, _), Ty::Interface(...)) accordingly so matching uses p_assoc as the
source of expected associated bindings and c_assoc only as the lookup target.
---
Nitpick comments:
In `@baml_language/crates/baml_type/src/template.rs`:
- Around line 85-97: Add unit tests that exercise the TyTemplate::Interface
variant to cover its substitute, is_fully_concrete, and Display logic: create a
template like Interface(name, vec![Ty::Param(0) or similar placeholder],
vec![(“Item”, Ty::Primitive(String))]) and assert that substitute with a
concrete type replaces the parameter in args and associated_bindings,
is_fully_concrete returns true/false appropriately before and after
substitution, and Display renders the interface using the expected "<...>"
generic syntax; place tests alongside existing unit tests in this module and
target the functions/methods substitute, is_fully_concrete, and the Display impl
for TyTemplate/Ty so the new Interface paths are covered.
🪄 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: bc475f91-bacd-4c1d-848a-d8d0e27382a4
📒 Files selected for processing (15)
baml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/interfaces.rsbaml_language/crates/baml_tests/tests/interfaces_associated_types.rsbaml_language/crates/baml_type/src/lib.rsbaml_language/crates/baml_type/src/template.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_sap/src/sap_model/convert.rsbaml_language/crates/bex_vm/src/package_baml/json.rsbaml_language/crates/bex_vm/src/package_baml/type_class.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm_types/src/types.rsbaml_language/crates/bridge_ctypes/src/value_encode.rsbaml_language/crates/sys_llm/src/types/output_format.rs
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/bex_engine/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- baml_language/crates/bex_vm/src/package_baml/json.rs
- baml_language/crates/sys_llm/src/types/output_format.rs
- baml_language/crates/baml_type/src/lib.rs
- baml_language/crates/bridge_ctypes/src/value_encode.rs
- baml_language/crates/bex_vm/src/package_baml/type_class.rs
- baml_language/crates/bex_sap/src/sap_model/convert.rs
- baml_language/crates/bex_vm/src/vm.rs
- baml_language/crates/baml_compiler2_emit/src/lib.rs
- baml_language/crates/baml_compiler2_mir/src/lower.rs
This was referenced Jun 4, 2026
This was referenced Jun 22, 2026
This was referenced Jul 7, 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
Implements BEP-57 associated types for interfaces across parsing, AST/HIR/TIR/MIR, formatter, LSP diagnostics, and codegen-facing surfaces.
Key pieces:
implementsblocks.(Type as Interface<...>).Memberdisambiguation..as<T>as upcast-equivalent while enforcing associated binding compatibility.implements Iterator<Item = int>; witnesses must be declared inside the block (type Item = int) Rust/Swift-style.Root Cause / Design Notes
The parser can recognize
Iterator<Item = int>as a type expression, but BEP-57 semantics reserve associated type witnesses for the body of an implementation. The implementation therefore preserves interface type bindings for values/bounds/projections while rejecting bindings onimplementstargets.Validation
cargo fmtcargo check -p baml_compiler2_tir -p baml_compiler2_mir -p baml_lsp2_actions -p baml_compiler2_ast -p baml_compiler_parser -p baml_fmtcargo build -p baml_clicargo test -p baml_tests --test interfaces_associated_types -- --nocapture(93 passed)cargo test -p baml_tests --test interfaces -- --nocapture(379 passed, 2 ignored)cargo test -p baml_tests(full package passed)Note
High Risk
Large, cross-cutting compiler and runtime changes to interface typing, dispatch, and reflection; incorrect binding matching could cause subtle dispatch or pattern-match bugs.
Overview
Adds BEP-057 associated types end-to-end: interfaces can declare associated types (bounds/defaults), and
implementsblocks can witness them withtype Name = …instead of putting bindings on theimplementstarget.The AST/HIR layer gains
AssociatedTypeBinding,associated_type_bindingsonTypeExpr::Path,AssociatedTypeProjection(Base.Item/(Base as Interface).Item), and matching fields on interfaces,implementsblocks, class destructuring, and method-to-interface metadata. Lowering wires CSTTYPE_ARGS/ associated decls throughlower_type_exprandlower_cst.MIR and emit stop erasing interfaces to classes:
Ty::Interfacekeeps type args plus associated bindings; interface dispatch, field access, pattern matching,is-type tests, and the runtime implementor registry all compare generic args and associated bindings (e.g.Source<Item = int>vsItem = string). Builtin codegen treats projections as unknown types for native extraction.Reviewed by Cursor Bugbot for commit cecf4f4. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Tests
Tooling
Runtime & Serialization