feat(reflect): sealed type-kind read-back (BEP-066 s1, PR 5 — capstone) - #4334
Conversation
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds BEP-066 reflection type-kind views for nine runtime categories. It updates compiler parsing, type checking, metadata emission, code generation, VM accessors, runtime type classification, and test coverage. ChangesReflection contracts and metadata
Compiler and runtime behavior
Validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Action performedReview finished.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
baml_language/crates/bex_vm/src/package_baml/type_kinds.rs (2)
94-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
$trait_namemacro parameter.
impl_as_type!accepts$trait_name:identbut never expands it. The generated body is identical for all nine traits. Rust does not warn on an unused macro metavariable, so the argument at every call site suggests a per-trait specialization that does not exist.Drop the parameter and update the nine call sites to
impl_as_type!();.♻️ Proposed change to the macro definition
macro_rules! impl_as_type { - ($trait_name:ident) => { + () => { fn as_type(_vm: &BexVm, r#type: &Value) -> Value { *r#type } }; }Then update each call site, for example:
impl BamlClassReflectClassType for PackageBamlImpl { impl_as_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/bex_vm/src/package_baml/type_kinds.rs` around lines 94 - 100, Remove the unused $trait_name parameter from the impl_as_type! macro definition, then update all nine invocations to use impl_as_type!() without an argument. Keep the generated as_type implementation unchanged.
33-57: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider narrowing the heap clone to the data each caller needs.
reflected_classandreflected_enumdeep-copy the whole definition object, includingVec<ClassField>and every per-fieldIndexMap. Themetacallers at Line 131 and Line 164 read only fourOption<String>values plusother, so they pay for the full field-vector copy on every call.The clone resolves a real borrow conflict: the
vm.get_objectborrow must end before the TLAB allocations take&mut BexVm. You can keep that property and still avoid the field copy by extracting only the owned values inside the borrow scope, for example ameta-specific helper that returns(Option<String>, Option<String>, Option<String>, IndexMap<String, String>).This is a reflection path, not the interpreter hot loop, so it is optional.
🤖 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_vm/src/package_baml/type_kinds.rs` around lines 33 - 57, Reduce cloning in the reflection metadata path by adding a helper that extracts only the required owned class or enum metadata—three Option<String> values and the other IndexMap—while the vm.get_object borrow is active. Update the callers around reflected_class and reflected_enum metadata handling to use this helper, ensuring the borrow ends before any mutable BexVm/TLAB allocations and avoiding cloning ClassField vectors and per-field IndexMaps.baml_language/crates/baml_type/src/normalize.rs (1)
772-774: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd runtime
typesubtype tests for reflection-kind classes.
type_kind.rsonly coversis_type_kind_classrecognizer rules.normalize.rsstill lacks a unit test that exercisesNormalTy::Class(baml.reflect.<kind>.Type) <: NormalTy::Typeand keeps subtype checks on other reflection kinds/NormalTy::Typenon-convertible.🤖 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/normalize.rs` around lines 772 - 774, Add unit tests in normalize.rs covering runtime subtype behavior for NormalTy::Class(baml.reflect.<kind>.Type) against NormalTy::Type: reflection-kind classes must convert to Type, while other reflection kinds and NormalTy::Type itself remain non-convertible. Reuse the existing subtype-testing helpers and representative reflection-kind symbols, and keep the production match arm unchanged.Source: Path instructions
baml_language/crates/baml_compiler2_tir/src/builder.rs (1)
6116-6125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the reflection-kind construction guard into one helper. Both sites run the identical check-and-report sequence against a different
Tyvalue. Duplicating this logic risks the two call sites drifting apart if the diagnostic condition changes later.
baml_language/crates/baml_compiler2_tir/src/builder.rs#L6116-L6125: replace this block with a call to a new helper, e.g.self.report_if_reflection_kind_class(&ty, expr_id);.baml_language/crates/baml_compiler2_tir/src/builder.rs#L6274-L6283: replace this block withself.report_if_reflection_kind_class(expected, expr_id);, using the same helper.♻️ Proposed helper
+ fn report_if_reflection_kind_class(&mut self, ty: &Ty, expr_id: ExprId) { + if let Ty::Class(class_name, _, _) = ty + && is_type_kind_class(class_name) + { + self.context.report_simple( + TirTypeError::CannotConstructReflectionKind { + class_name: class_name.clone(), + }, + expr_id, + ); + } + }🤖 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 6116 - 6125, Extract the duplicated reflection-kind check and diagnostic reporting into a shared helper such as report_if_reflection_kind_class in builder.rs. Update the anchor site at baml_language/crates/baml_compiler2_tir/src/builder.rs lines 6116-6125 to call it with &ty, and update the sibling site at lines 6274-6283 to call it with expected, preserving the existing expr_id and diagnostic behavior at both sites.
🤖 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_tir/src/builder.rs`:
- Around line 6116-6125: Extract the duplicated reflection-kind check and
diagnostic reporting into a shared helper such as
report_if_reflection_kind_class in builder.rs. Update the anchor site at
baml_language/crates/baml_compiler2_tir/src/builder.rs lines 6116-6125 to call
it with &ty, and update the sibling site at lines 6274-6283 to call it with
expected, preserving the existing expr_id and diagnostic behavior at both sites.
In `@baml_language/crates/baml_type/src/normalize.rs`:
- Around line 772-774: Add unit tests in normalize.rs covering runtime subtype
behavior for NormalTy::Class(baml.reflect.<kind>.Type) against NormalTy::Type:
reflection-kind classes must convert to Type, while other reflection kinds and
NormalTy::Type itself remain non-convertible. Reuse the existing subtype-testing
helpers and representative reflection-kind symbols, and keep the production
match arm unchanged.
In `@baml_language/crates/bex_vm/src/package_baml/type_kinds.rs`:
- Around line 94-100: Remove the unused $trait_name parameter from the
impl_as_type! macro definition, then update all nine invocations to use
impl_as_type!() without an argument. Keep the generated as_type implementation
unchanged.
- Around line 33-57: Reduce cloning in the reflection metadata path by adding a
helper that extracts only the required owned class or enum metadata—three
Option<String> values and the other IndexMap—while the vm.get_object borrow is
active. Update the callers around reflected_class and reflected_enum metadata
handling to use this helper, ensuring the borrow ends before any mutable
BexVm/TLAB allocations and avoiding cloning ClassField vectors and per-field
IndexMaps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 75ea7948-483c-41d6-a668-58f948d02c6b
⛔ Files ignored due to path filters (19)
baml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/reflect_type_of.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_kinds/baml_tests__compiles__type_kinds__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_kinds/baml_tests__compiles__type_kinds__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_kinds/baml_tests__compiles__type_kinds__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_kinds/baml_tests__compiles__type_kinds__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_kinds/baml_tests__compiles__type_kinds__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_kinds/baml_tests__compiles__type_kinds__10_formatter__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_kinds/baml_tests__diagnostic_errors__type_kinds__03_ppir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_kinds/baml_tests__diagnostic_errors__type_kinds__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_kinds/baml_tests__diagnostic_errors__type_kinds__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_kinds/baml_tests__diagnostic_errors__type_kinds__10_formatter__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snap
📒 Files selected for processing (46)
baml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_array/array.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_class/class.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_enum/enum.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_function/function.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_interface/interface.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_literal/literal.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_map/map.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_primitive/primitive.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/ns_union/union.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_reflect/reflect.bamlbaml_language/crates/baml_builtins2/baml_std/baml/type_class.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_builtins2_codegen/src/codegen.rsbaml_language/crates/baml_compiler2_ast/src/disambiguate.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/emit.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/infer_context.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_fmt/src/ast/expressions.rsbaml_language/crates/baml_fmt/src/ast/types.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_tests/baml_src/ns_reflect_type_of/reflect_type_of.bamlbaml_language/crates/baml_tests/projects/compiles/type_kinds/main.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/type_kinds/main.bamlbaml_language/crates/baml_tests/tests/type_kinds.rsbaml_language/crates/baml_type/src/lib.rsbaml_language/crates/baml_type/src/normalize.rsbaml_language/crates/baml_type/src/type_kind.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_heap/src/gc.rsbaml_language/crates/bex_heap/src/tlab.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/resolve.rsbaml_language/crates/bex_vm/src/package_baml/type_class.rsbaml_language/crates/bex_vm/src/package_baml/type_kinds.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm/tests/method_class_type_args.rsbaml_language/crates/bex_vm_types/src/link.rsbaml_language/crates/bex_vm_types/src/types/class.rsbaml_language/crates/bex_vm_types/src/types/enums.rs
1 participant
BEP-066 slice-1 stack, PR 5 of 5 — the capstone. Chained on #4331. With this green, the slice-1 stack is complete: the reflection read API (K/V/N rule families) is fully live.
What
class/enum/interface/functionlegal as path segments after.across type parsing, expression paths, patterns, map entries, generic lookahead, AST lowering, and formatting — bare keywords still rejected.reflect.class.Type…reflect.function.Type) + the closedbaml.reflect.TypeKindunion alias; identity-preservingkind()(K-5), all nine nullableas_*()(K-6 — never throw),as_type().<: typesealed edge in shared normalization;Object::Typereports its precise kind class while keeping the physical TYPE tag;implement … for typepreserved by teaching impl resolution to follow the sealed edge (to_string()verified on all nine kinds).docstring+ string-valuedothercolumns on Class/Field/Enum/Variant; emit preserves aliases, descriptions, docstrings, and custom annotations (found + fixed + pinned a pre-existing hoist bug where the custom-attribute path consumedstream.*).Oracle coverage
Exhaustive nine-arm
TypeKindmatch + missing-arm diagnostics · everyas_*positive/null path · mint identity throughkind().as_type()(I-2 × K-5) · non-throwing accessor contracts · full metadata read-back · conformance queries · recursive type walking ·of_valueprecision.Gates
Full corpus + parser + LSP (reviewed) + all-features baml_cli + tir + project green; fmt/clippy/rustdoc clean; snapshot accepts grouped in the commit (new type_kinds corpus + describe-listing growth).
Implemented by a Codex (gpt-5.6-sol) worker under stack-manager review. Both BEP-066 foundation stacks are now complete at 5/5.
Summary by CodeRabbit
New Features
Bug Fixes