Repository navigation
Remove Ty::Optional; lower ? to T | null - #3699
Merged
Merged
Conversation
`?` is now syntax sugar for a union with null rather than its own type variant. The `Optional` variant is deleted from all five typed-`Ty` enums (baml_type::Ty, baml_type::TyTemplate, the TIR Ty + StructuralTy, and baml_codegen_types::Ty). The `?` token still parses to TypeExpr::Optional but lowers directly to `T | null` (null last). Any union containing null now renders with the postfix `?` (`T | null` -> `T?`, `A | B | null` -> `(A | B)?`) across type display, LSP diagnostics, bytecode disassembly, codegen, and LLM prompt schemas. This unifies `T?` and `T | null` so they behave identically everywhere. Notable consequences: - An explicit `x: T | null` class field is now absent-able and decodes to bare null (previously required, and wrapped in union metadata). - The Python SDK keeps emitting `typing.Optional[T]` for single-member nullable unions; TypeScript emits `T | null`. - FFI synthesizes the `OptionalType` tag for nullable unions to preserve the wire format; SAP relies on its existing null-last union path. - Else-branch narrowing of nested nullable-union members (`int | string?`) now flattens unions to leaves so the residual narrows correctly. Helpers added on the type enums: nullable/optional, is_null, is_nullable_union, strip_null, peel_optional.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoves the Ty::Optional variant and canonicalizes optionality as Ty::Union containing a null member; adds Ty::nullable/strip_null/is_null helpers and updates lowering, traversal, runtime JSON/validation, SDK translators, generated protobuf encoders, and tests accordingly. ChangesOptional-to-Nullable-Union Type System Refactoring
🎯 4 (Complex) | ⏱️ ~60 minutes
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
…nion # Conflicts: # baml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.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):
|
![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: 3
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_lsp2_actions/src/check.rs (1)
4120-4125:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMention
unknownin this diagnostic.This branch also rejects
Ty::BuiltinUnknown, but the message only says "union, optional, or interface".implements I for unknown {}will get the wrong explanation in the editor.Proposed fix
- "cannot implement interface `{}` for `{}`: the `for` target must be a single \ - concrete type, not a union, optional, or interface", + "cannot implement interface `{}` for `{}`: the `for` target must be a single \ + concrete type, not a union, optional, interface, or `unknown`",🤖 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 4120 - 4125, The diagnostic message for DiagnosticId::ImplTargetNotConcrete currently lists "union, optional, or interface" but omits the Ty::BuiltinUnknown case; update the formatted error string in the Diagnostic::error call (the block that constructs the message using imp.interface_target.expr and imp.for_target.expr) to also mention "unknown" (e.g., "union, optional, interface, or unknown") so implementations like `implements I for unknown {}` get the correct explanation.
🧹 Nitpick comments (3)
baml_language/crates/baml_lsp2_actions/src/check.rs (1)
4115-4118: ⚡ Quick winAdd a unit test for nullable
fortargets.This branch now carries both
T?andT | nullthroughTy::Union(..), butcheck.rsdoesn't have a focused regression test for theImplTargetNotConcretepath. A smallCursorTestcovering both spellings would lock in the LSP behavior here. As per coding guidelines**/*.rs: Prefer writing Rust unit tests 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_lsp2_actions/src/check.rs` around lines 4115 - 4118, Add a focused Rust unit test in baml_lsp2_actions/src/check.rs that exercises the ImplTargetNotConcrete path for nullable `for` targets by creating a CursorTest (or similar existing test helper) which asserts the LSP/check behavior for both `T?` and `T | null` spellings; specifically, construct inputs where the resolved target becomes Ty::Union(..) (and ensure the test covers Ty::Interface(..)/Ty::Union(..)/Ty::Unknown/ Ty::BuiltinUnknown branches) and assert the same diagnostic/behavior is emitted as when a non-nullable concrete target is expected, so the regression for nullable `for` targets is locked in. Ensure the test is a unit test (#[cfg(test)] mod) near check.rs and reuses existing helpers to keep it small and focused.baml_language/crates/baml_compiler2_tir/src/normalize.rs (1)
697-708: ⚡ Quick winAdd a nullable-cycle regression and rerun the library suite.
This migration removes the dedicated optional traversal here, but the new coverage in this file only exercises subtyping. Please add a unit test for a nullable recursive alias (for example
type A = A | null) and, if this file owns the behavior, a required-field cycle case thatT | nullshould now break, then reruncargo test --lib.As per coding guidelines,
**/*.rs:Prefer writing Rust unit tests over integration tests where possibleandAlways run cargo test --lib if you changed any Rust code.Also applies to: 1979-1999, 2483-2490
🤖 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/normalize.rs` around lines 697 - 708, Add a unit test that exercises nullable recursive aliases and required-field cycles and re-run the library tests: write a Rust unit test in the same crate (prefer a unit test in baml_compiler2_tir/src/normalize.rs or its module tests) that defines a recursive alias like `type A = A | null` and asserts that ty_has_cycle/has_cycle detects the cycle (and likewise add a case where a required-field cycle involving `T | null` should be rejected or break), then run `cargo test --lib` to ensure the new tests fail/pass as appropriate; reference the cycle-detection functions ty_has_cycle and has_cycle so the test targets the code paths you modified and covers the nullable/optional traversal cases mentioned.baml_language/crates/bex_vm/src/package_baml/json.rs (1)
1431-1445: ⚡ Quick winUse
Ty::peel_optional()instead of reimplementing it locally.
baml_type::Tyalready exposes this exact nullable-union peel logic. Keeping a second copy here makes this path easy to drift the next time optional/null semantics change.Suggested simplification
fn peel_optional(ty: &Ty) -> &Ty { - if let Ty::Union(members, _) = ty { - if members.iter().any(Ty::is_null) { - if let Some(inner) = members.iter().find(|m| !m.is_null()) { - if members.iter().filter(|m| !m.is_null()).count() == 1 { - return inner; - } - } - } - } - ty + ty.peel_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/json.rs` around lines 1431 - 1445, Remove the local peel_optional function and replace its uses with the existing baml_type::Ty implementation (call Ty::peel_optional(ty) or ty.peel_optional() as provided by the baml_type crate); ensure the module imports baml_type::Ty if needed and delete the duplicate function definition in json.rs so nullable-union peeling is centralized to Ty::peel_optional.
🤖 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_mir/src/lower.rs`:
- Around line 6585-6587: Replace uses of base_ty.peel_optional() in
lower_optional_member_access (and the other listed sites) with the
null-stripping helper that removes null from any nullable union (instead of only
unwrapping single-member optionals); specifically change the binding
unwrapped_ty = base_ty.peel_optional() to call the helper that strips null from
unions so cases like (Foo | Bar | null)?.field and (int[] | string[] | null)?[0]
route to the correct field/index path. Update all occurrences called out (around
the current site and at the other ranges referenced) and add a unit test in this
crate (a Rust unit test, not integration) that exercises multi-member nullable
unions (e.g. Foo|Bar|null) with optional member access and optional indexing to
prevent regressions. Ensure the test lives under the same module/root tests as
other compiler unit tests per coding guidelines.
In `@baml_language/crates/baml_type/src/template.rs`:
- Around line 141-144: The current nullable union formatting uses write!(f,
"{}?", strs.join(" | ")) which produces "A | B?" instead of "(A | B)?"; change
the formatting in the match arm handling `non_null` so that if `non_null.len() >
1` you wrap the joined string in parentheses before appending the `?` (e.g.
write!(f, "({})?", joined) ), otherwise keep the existing write!(f, "{}?",
joined) for single-member unions; update the branch that builds `strs` and the
final write! call accordingly.
In `@baml_language/crates/bex_engine/src/conversion.rs`:
- Around line 656-669: The branch handling nullable unions currently strips null
from the type when there are multiple non-null arms by calling
maybe_wrap_union(value, &Ty::Union(non_null, attr.clone())), which loses the
optional flag in UnionMetadata; instead pass the original union (including null)
to maybe_wrap_union so metadata preserves optionality—i.e., for the "_" case
call maybe_wrap_union(value, &Ty::Union(members.clone(), attr.clone())) (keep
the existing non_null computation for the other branches but use members when
wrapping), so UnionMetadata::new will still mark the union as optional.
---
Outside diff comments:
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 4120-4125: The diagnostic message for
DiagnosticId::ImplTargetNotConcrete currently lists "union, optional, or
interface" but omits the Ty::BuiltinUnknown case; update the formatted error
string in the Diagnostic::error call (the block that constructs the message
using imp.interface_target.expr and imp.for_target.expr) to also mention
"unknown" (e.g., "union, optional, interface, or unknown") so implementations
like `implements I for unknown {}` get the correct explanation.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler2_tir/src/normalize.rs`:
- Around line 697-708: Add a unit test that exercises nullable recursive aliases
and required-field cycles and re-run the library tests: write a Rust unit test
in the same crate (prefer a unit test in baml_compiler2_tir/src/normalize.rs or
its module tests) that defines a recursive alias like `type A = A | null` and
asserts that ty_has_cycle/has_cycle detects the cycle (and likewise add a case
where a required-field cycle involving `T | null` should be rejected or break),
then run `cargo test --lib` to ensure the new tests fail/pass as appropriate;
reference the cycle-detection functions ty_has_cycle and has_cycle so the test
targets the code paths you modified and covers the nullable/optional traversal
cases mentioned.
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 4115-4118: Add a focused Rust unit test in
baml_lsp2_actions/src/check.rs that exercises the ImplTargetNotConcrete path for
nullable `for` targets by creating a CursorTest (or similar existing test
helper) which asserts the LSP/check behavior for both `T?` and `T | null`
spellings; specifically, construct inputs where the resolved target becomes
Ty::Union(..) (and ensure the test covers
Ty::Interface(..)/Ty::Union(..)/Ty::Unknown/ Ty::BuiltinUnknown branches) and
assert the same diagnostic/behavior is emitted as when a non-nullable concrete
target is expected, so the regression for nullable `for` targets is locked in.
Ensure the test is a unit test (#[cfg(test)] mod) near check.rs and reuses
existing helpers to keep it small and focused.
In `@baml_language/crates/bex_vm/src/package_baml/json.rs`:
- Around line 1431-1445: Remove the local peel_optional function and replace its
uses with the existing baml_type::Ty implementation (call Ty::peel_optional(ty)
or ty.peel_optional() as provided by the baml_type crate); ensure the module
imports baml_type::Ty if needed and delete the duplicate function definition in
json.rs so nullable-union peeling is centralized to Ty::peel_optional.
🪄 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: 48ca58d1-8401-46ed-bea4-2a16bb04647d
⛔ Files ignored due to path filters (114)
baml_language/crates/baml_tests/snapshots/baml_src/is_operator.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/match_types.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/patterns_new_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/type_reflection.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/__testing_std__/baml_tests__compiles____testing_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/closures/baml_tests__compiles__closures__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/comment_in_type/baml_tests__compiles__comment_in_type__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/comment_in_type/baml_tests__compiles__comment_in_type__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/deep_method_call/baml_tests__compiles__deep_method_call__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/event_system/baml_tests__compiles__event_system__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/function_call/baml_tests__compiles__function_call__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_field_chain/baml_tests__compiles__generic_field_chain__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_alias_basic/baml_tests__compiles__json_alias_basic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_cross_namespace_static_call/baml_tests__compiles__json_cross_namespace_static_call__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_llm_return_type/baml_tests__compiles__json_llm_return_type__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_llm_return_type/baml_tests__compiles__json_llm_return_type__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_parse_stringify_intrinsics/baml_tests__compiles__json_parse_stringify_intrinsics__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_parse_stringify_intrinsics/baml_tests__compiles__json_parse_stringify_intrinsics__04_tir.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__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_to_from_string_concrete/baml_tests__compiles__json_to_from_string_concrete__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lambda_advanced/baml_tests__compiles__lambda_advanced__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lambda_advanced/baml_tests__compiles__lambda_advanced__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lambda_field_access/baml_tests__compiles__lambda_field_access__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_basic/baml_tests__compiles__namespaces_basic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_nested/baml_tests__compiles__namespaces_nested__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_root_fallback/baml_tests__compiles__namespaces_root_fallback__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_shadow/baml_tests__compiles__namespaces_shadow__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_type_resolution/baml_tests__compiles__namespaces_type_resolution__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/numeric_invariance_ok/baml_tests__compiles__numeric_invariance_ok__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/optional_function_parameters/baml_tests__compiles__optional_function_parameters__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/optional_function_parameters/baml_tests__compiles__optional_function_parameters__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/paren_union_test/baml_tests__compiles__paren_union_test__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/parser_expressions/baml_tests__compiles__parser_expressions__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/parser_statements/baml_tests__compiles__parser_statements__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_class_destructure_namespaces/baml_tests__compiles__patterns_class_destructure_namespaces__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/reflect_type_of_user_generic/baml_tests__compiles__reflect_type_of_user_generic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_crossfile/baml_tests__compiles__stream_crossfile__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_llm_inferred_typeargs/baml_tests__compiles__stream_llm_inferred_typeargs__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_llm_inferred_typeargs/baml_tests__compiles__stream_llm_inferred_typeargs__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_llm_inferred_typeargs/baml_tests__compiles__stream_llm_inferred_typeargs__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_with_not_keyword/baml_tests__compiles__test_with_not_keyword__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_vibes_nested/baml_tests__compiles__testset_vibes_nested__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_vibes_nested/baml_tests__compiles__testset_vibes_nested__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_annotation/baml_tests__compiles__type_annotation__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/attr_disambiguation/baml_tests__diagnostic_errors__attr_disambiguation__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/attribute_validation/baml_tests__diagnostic_errors__attribute_validation__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/basic_types/baml_tests__diagnostic_errors__basic_types__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/captured_field_chain/baml_tests__diagnostic_errors__captured_field_chain__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/closure_errors/baml_tests__diagnostic_errors__closure_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/control_flow/baml_tests__diagnostic_errors__control_flow__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/duplicate_class_span/baml_tests__diagnostic_errors__duplicate_class_span__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/format_checks/baml_tests__diagnostic_errors__format_checks__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/function_type_throws/baml_tests__diagnostic_errors__function_type_throws__04_tir.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__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generics/baml_tests__diagnostic_errors__generics__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/instanceof_removed/baml_tests__diagnostic_errors__instanceof_removed__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/is_pattern_negative/baml_tests__diagnostic_errors__is_pattern_negative__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/json_static_method_arity/baml_tests__diagnostic_errors__json_static_method_arity__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/match_exhaustiveness/baml_tests__diagnostic_errors__match_exhaustiveness__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/match_exhaustiveness/baml_tests__diagnostic_errors__match_exhaustiveness__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_bare_name_rejected/baml_tests__diagnostic_errors__namespaces_bare_name_rejected__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_conflict/baml_tests__diagnostic_errors__namespaces_conflict__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_stream_direct_ref/baml_tests__diagnostic_errors__namespaces_stream_direct_ref__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_stream_direct_ref/baml_tests__diagnostic_errors__namespaces_stream_direct_ref__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/null_handling/baml_tests__diagnostic_errors__null_handling__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/numeric_invariance/baml_tests__diagnostic_errors__numeric_invariance__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/optional_parameter_defaults/baml_tests__diagnostic_errors__optional_parameter_defaults__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/parser_constructors/baml_tests__diagnostic_errors__parser_constructors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure_namespaces/baml_tests__diagnostic_errors__patterns_class_destructure_namespaces__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_new/baml_tests__diagnostic_errors__patterns_new__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/simple_function/baml_tests__diagnostic_errors__simple_function__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/stream_types/baml_tests__diagnostic_errors__stream_types__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_with_runner_ambiguity/baml_tests__diagnostic_errors__test_with_runner_ambiguity__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_aliases/baml_tests__diagnostic_errors__type_aliases__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_reflection_strict/baml_tests__diagnostic_errors__type_reflection_strict__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__chained_optional_field_access.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_call_chain_continues.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_chaining_with_null_coalesce.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_field_access.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_field_access_through_optional_alias.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_index_through_optional_alias.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_method_call_basic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__class_field_gets_stream_prefix.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__cross_file_class_reference.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__dynamic_attr_passes_through.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__enum_field_unchanged.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__field_alias_preserved_on_stream_class.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__field_description_preserved_stream_done_stripped.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__list_field_recurses.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__map_field_recurses_value.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__optional_expands_to_union_with_null.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__primitives_get_null_union.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__recursive_class.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_companion_preserves_generic_args_for_llm_return_type.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_companion_preserves_generic_args_in_class_field.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_done_and_not_null.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_done_block_attr.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_done_field_keeps_type_as_is.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_not_null_block_attr_on_referenced_class.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_not_null_field.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__type_alias_expansion.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__union_field_recurses_variants.snapis excluded by!**/*.snapbaml_language/sdks/nodejs/bridge_nodejs/dist/native.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**baml_language/sdks/python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
baml_language/crates/baml_codegen_types/src/symbols.rsbaml_language/crates/baml_codegen_types/src/ty.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/exhaustiveness.rsbaml_language/crates/baml_compiler2_tir/src/generics.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/narrowing.rsbaml_language/crates/baml_compiler2_tir/src/normalize.rsbaml_language/crates/baml_compiler2_tir/src/throw_inference.rsbaml_language/crates/baml_compiler2_tir/src/ty.rsbaml_language/crates/baml_exec/src/auto_cli.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_lsp2_actions/src/describe.rsbaml_language/crates/baml_lsp2_actions/src/type_info.rsbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/client/remap_role_invalid_type.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/client/remap_role_non_string_allowed_roles.bamlbaml_language/crates/baml_project/src/client_codegen.rsbaml_language/crates/baml_tests/baml_src/ns_is_operator/is_operator.bamlbaml_language/crates/baml_tests/src/compiler2_tir/inference.rsbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a_recursion.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase6.rsbaml_language/crates/baml_tests/tests/parse_companions.rsbaml_language/crates/baml_type/src/lib.rsbaml_language/crates/baml_type/src/simplify_sap.rsbaml_language/crates/baml_type/src/template.rsbaml_language/crates/baml_type/tests/simplify_sap_tests.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_external_types/src/bex_external_value.rsbaml_language/crates/bex_external_types/src/host_return.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/bridge_ctypes/src/value_encode.rsbaml_language/crates/sys_llm/src/lib.rsbaml_language/crates/sys_llm/src/types/output_format.rsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/nodejs/sdkgen_typescript_node/src/emit/mod.rsbaml_language/sdks/nodejs/sdkgen_typescript_node/src/translate_ty.rsbaml_language/sdks/python/rust/sdkgen_python_pydantic2/src/emit/mod.rsbaml_language/sdks/python/rust/sdkgen_python_pydantic2/src/leaf.rsbaml_language/sdks/python/rust/sdkgen_python_pydantic2/src/lib.rsbaml_language/sdks/python/rust/sdkgen_python_pydantic2/src/translate_ty.rs
💤 Files with no reviewable changes (5)
- baml_language/sdks/python/rust/sdkgen_python_pydantic2/src/emit/mod.rs
- baml_language/crates/baml_lsp2_actions/src/type_info.rs
- baml_language/crates/baml_type/src/simplify_sap.rs
- baml_language/sdks/nodejs/sdkgen_typescript_node/src/emit/mod.rs
- baml_language/crates/bex_external_types/src/host_return.rs
Binary size checks passed✅ 7 passed
Generated by |
`?` is source sugar only: it exists at parse/lowering, after which a nullable type is a plain union. Display (type errors, TIR/MIR dumps, bytecode disassembly, codegen, LSP hovers) now renders the union form `T | null` instead of re-collapsing to `T?`. This drops the collapse logic from every Display impl and reverts the stream-type snapshot churn. Also fix the PPIR stream-variant expansion to put null last (`T | null`) to match the `?` lowering (`Ty::nullable`) and SAP's `null_to_end`, so nullable types are consistently null-last everywhere.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
baml_language/crates/baml_compiler2_tir/src/ty.rs (1)
706-719:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize existing nullable unions to
null-last.
Ty::nullable(Ty::Union(...))currently returns the existing member order unchanged whennullis already present. BecauseTypreserves union order inEq/Ord/Display, that leaves two representations for the same lowered optional type (null | TandT | null). This helper is the canonical constructor for?, so it should strip any existingnullmembers and append a singlenullat the end. The PR summary calls out a null-last canonical form, and the same normalization is needed inbaml_type::Ty::optionalto keep the layers aligned.Suggested fix
pub fn nullable(inner: Ty) -> Ty { match inner { - Ty::Union(mut members, attr) => { - if !members.iter().any(Ty::is_null) { - members.push(Ty::primitive_null()); - } - Ty::Union(members, attr) + Ty::Union(members, attr) => { + let mut non_null: Vec<Ty> = members.into_iter().filter(|m| !m.is_null()).collect(); + if non_null.is_empty() { + Ty::primitive_null() + } else { + non_null.push(Ty::primitive_null()); + Ty::Union(non_null, attr) + } } n @ Ty::Primitive(PrimitiveType::Null, _) => n, other => Ty::Union(vec![other, Ty::primitive_null()], TyAttr::default()), } }🤖 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/ty.rs` around lines 706 - 719, Ty::nullable currently leaves existing Union member order untouched when null is already present, causing duplicate canonical forms like `null | T`; update Ty::nullable (the function handling `T?`) to remove any existing null members from a Ty::Union before appending a single null at the end so unions are normalized to null-last; specifically, in the Ty::Union match arm filter out members where Ty::is_null is true, then push Ty::primitive_null() once and return Ty::Union with the normalized members and the original attr; also ensure the primitive-null and primitive-null special-case (Ty::Primitive(PrimitiveType::Null, _)) remain unchanged and mirror the same canonicalization used by baml_type::Ty::optional.baml_language/crates/baml_type/src/lib.rs (1)
369-385:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Ty::optionalstill leaks non-canonical member order.If a caller passes an already-nullable union with
nullanywhere but the tail, this helper returns it unchanged. SinceTyequality, hashing, and display all preserve union order, that creates multiple representations for the same optional type (null | intvsint | null). External code already constructs nullable types through this API, soTy::optionalshould normalize by removing any existingnullmembers and appending exactly onenullat the end.Suggested fix
pub fn optional(inner: Ty) -> Self { match inner { - Ty::Union(mut members, attr) => { - if !members.iter().any(Ty::is_null) { - members.push(Ty::null()); - } - Ty::Union(members, attr) + Ty::Union(members, attr) => { + let mut non_null: Vec<Ty> = members.into_iter().filter(|m| !m.is_null()).collect(); + if non_null.is_empty() { + Ty::null() + } else { + non_null.push(Ty::null()); + Ty::Union(non_null, attr) + } } n @ Ty::Null { .. } => n, other => Ty::Union(vec![other, Ty::null()], TyAttr::default()), } }🤖 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/lib.rs` around lines 369 - 385, Ty::optional currently returns an unchanged Ty::Union when a null member exists anywhere, which preserves non-canonical union order; update the Ty::optional implementation so that when matching Ty::Union(members, attr) you remove all existing null members (e.g., filter members by !Ty::is_null), then append exactly one Ty::null() at the end and return Ty::Union(normalized_members, attr); keep the existing branches for Ty::Null and other types (other => Ty::Union(vec![other, Ty::null()], TyAttr::default())) but ensure union normalization is used for any union path so the canonical order is always "...non-null members..., null".
🤖 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/ty.rs`:
- Around line 864-871: The union-rendering in Ty::Union uses
members.iter().map(|m| m.render_with(s))... which causes function types to
appear unparenthesized (e.g. `() -> string throws never | null`) and be misread;
modify the mapping so that when a member is a function type (match Ty::Fn or the
internal representation used for functions) you wrap its rendered string in
parentheses before returning it, otherwise return the plain m.render_with(s),
then join the mapped strings with " | " as before to ensure function members are
parenthesized in unions.
In `@baml_language/crates/baml_type/src/lib.rs`:
- Around line 826-831: The Ty::Union branch in the Display impl prints union
members directly which makes function types ambiguous (e.g. "callback" becomes
"int -> string throws never | null"); fix by mapping the collected member
strings and wrapping any function-type members in parentheses before joining. In
the Ty::Union handling (inside the Display impl for Ty) change the mapping over
types.iter() so that for each member you call its ToString, detect if the member
is a function type (the Ty variant that renders as a function), and if so wrap
the string in parentheses (e.g. format!("({})", s)); then join with " | ". Apply
the same parenthesizing logic at the other Union display site noted (the similar
block around the symbols referenced for lines ~1109-1113).
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_tir/src/ty.rs`:
- Around line 706-719: Ty::nullable currently leaves existing Union member order
untouched when null is already present, causing duplicate canonical forms like
`null | T`; update Ty::nullable (the function handling `T?`) to remove any
existing null members from a Ty::Union before appending a single null at the end
so unions are normalized to null-last; specifically, in the Ty::Union match arm
filter out members where Ty::is_null is true, then push Ty::primitive_null()
once and return Ty::Union with the normalized members and the original attr;
also ensure the primitive-null and primitive-null special-case
(Ty::Primitive(PrimitiveType::Null, _)) remain unchanged and mirror the same
canonicalization used by baml_type::Ty::optional.
In `@baml_language/crates/baml_type/src/lib.rs`:
- Around line 369-385: Ty::optional currently returns an unchanged Ty::Union
when a null member exists anywhere, which preserves non-canonical union order;
update the Ty::optional implementation so that when matching Ty::Union(members,
attr) you remove all existing null members (e.g., filter members by
!Ty::is_null), then append exactly one Ty::null() at the end and return
Ty::Union(normalized_members, attr); keep the existing branches for Ty::Null and
other types (other => Ty::Union(vec![other, Ty::null()], TyAttr::default())) but
ensure union normalization is used for any union path so the canonical order is
always "...non-null members..., null".
🪄 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: 38ec6a12-49db-43e0-b76f-234debc8d0df
⛔ Files ignored due to path filters (141)
baml_language/crates/baml_tests/snapshots/baml_src/byte_strings.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/control_flow.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/maps.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/match_basics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/null_handling.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/optional_function_parameters.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/patterns_new_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/reflect_type_of.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/type_reflection.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/typed_inputs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/typed_outputs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiler2_mir/baml_tests__compiler2_mir__optional_named_gap_and_explicit_null.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__assert_std__/baml_tests__compiles____assert_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__assert_std__/baml_tests__compiles____assert_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__assert_std__/baml_tests__compiles____assert_std____06_codegen.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/__testing_std__/baml_tests__compiles____testing_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/byte_string_literals/baml_tests__compiles__byte_string_literals__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/byte_string_literals/baml_tests__compiles__byte_string_literals__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/byte_string_literals/baml_tests__compiles__byte_string_literals__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/closures/baml_tests__compiles__closures__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/comment_in_type/baml_tests__compiles__comment_in_type__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/comment_in_type/baml_tests__compiles__comment_in_type__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/deep_method_call/baml_tests__compiles__deep_method_call__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/event_system/baml_tests__compiles__event_system__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/function_call/baml_tests__compiles__function_call__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_field_chain/baml_tests__compiles__generic_field_chain__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_alias_basic/baml_tests__compiles__json_alias_basic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_cross_namespace_static_call/baml_tests__compiles__json_cross_namespace_static_call__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_llm_return_type/baml_tests__compiles__json_llm_return_type__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_llm_return_type/baml_tests__compiles__json_llm_return_type__06_codegen.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__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_to_from_string_concrete/baml_tests__compiles__json_to_from_string_concrete__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lambda_advanced/baml_tests__compiles__lambda_advanced__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lambda_advanced/baml_tests__compiles__lambda_advanced__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lambda_advanced/baml_tests__compiles__lambda_advanced__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/lambda_field_access/baml_tests__compiles__lambda_field_access__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_basic/baml_tests__compiles__namespaces_basic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_nested/baml_tests__compiles__namespaces_nested__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_root_fallback/baml_tests__compiles__namespaces_root_fallback__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_shadow/baml_tests__compiles__namespaces_shadow__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/namespaces_type_resolution/baml_tests__compiles__namespaces_type_resolution__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/numeric_invariance_ok/baml_tests__compiles__numeric_invariance_ok__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/optional_function_parameters/baml_tests__compiles__optional_function_parameters__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/optional_function_parameters/baml_tests__compiles__optional_function_parameters__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/optional_function_parameters/baml_tests__compiles__optional_function_parameters__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/paren_union_test/baml_tests__compiles__paren_union_test__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/parser_expressions/baml_tests__compiles__parser_expressions__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/parser_statements/baml_tests__compiles__parser_statements__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_class_destructure_namespaces/baml_tests__compiles__patterns_class_destructure_namespaces__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/reflect_type_of_user_generic/baml_tests__compiles__reflect_type_of_user_generic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_crossfile/baml_tests__compiles__stream_crossfile__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_llm_inferred_typeargs/baml_tests__compiles__stream_llm_inferred_typeargs__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/stream_llm_inferred_typeargs/baml_tests__compiles__stream_llm_inferred_typeargs__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_with_not_keyword/baml_tests__compiles__test_with_not_keyword__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_vibes_nested/baml_tests__compiles__testset_vibes_nested__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_vibes_nested/baml_tests__compiles__testset_vibes_nested__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_annotation/baml_tests__compiles__type_annotation__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_errors/baml_tests__compiles__type_builder_errors__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/type_builder_test/baml_tests__compiles__type_builder_test__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/attr_disambiguation/baml_tests__diagnostic_errors__attr_disambiguation__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/attribute_validation/baml_tests__diagnostic_errors__attribute_validation__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/basic_types/baml_tests__diagnostic_errors__basic_types__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/captured_field_chain/baml_tests__diagnostic_errors__captured_field_chain__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/closure_errors/baml_tests__diagnostic_errors__closure_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/control_flow/baml_tests__diagnostic_errors__control_flow__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/duplicate_class_span/baml_tests__diagnostic_errors__duplicate_class_span__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/format_checks/baml_tests__diagnostic_errors__format_checks__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/function_type_throws/baml_tests__diagnostic_errors__function_type_throws__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/function_types/baml_tests__diagnostic_errors__function_types__04_tir.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__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generics/baml_tests__diagnostic_errors__generics__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/instanceof_removed/baml_tests__diagnostic_errors__instanceof_removed__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/is_pattern_negative/baml_tests__diagnostic_errors__is_pattern_negative__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/json_static_method_arity/baml_tests__diagnostic_errors__json_static_method_arity__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/match_exhaustiveness/baml_tests__diagnostic_errors__match_exhaustiveness__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/match_exhaustiveness/baml_tests__diagnostic_errors__match_exhaustiveness__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_bare_name_rejected/baml_tests__diagnostic_errors__namespaces_bare_name_rejected__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_conflict/baml_tests__diagnostic_errors__namespaces_conflict__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_stream_direct_ref/baml_tests__diagnostic_errors__namespaces_stream_direct_ref__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/namespaces_stream_direct_ref/baml_tests__diagnostic_errors__namespaces_stream_direct_ref__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/null_handling/baml_tests__diagnostic_errors__null_handling__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/null_handling/baml_tests__diagnostic_errors__null_handling__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/numeric_invariance/baml_tests__diagnostic_errors__numeric_invariance__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/numeric_invariance/baml_tests__diagnostic_errors__numeric_invariance__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/optional_parameter_defaults/baml_tests__diagnostic_errors__optional_parameter_defaults__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/optional_parameter_defaults/baml_tests__diagnostic_errors__optional_parameter_defaults__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/parser_constructors/baml_tests__diagnostic_errors__parser_constructors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure_namespaces/baml_tests__diagnostic_errors__patterns_class_destructure_namespaces__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_new/baml_tests__diagnostic_errors__patterns_new__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_new/baml_tests__diagnostic_errors__patterns_new__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/simple_function/baml_tests__diagnostic_errors__simple_function__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/stream_types/baml_tests__diagnostic_errors__stream_types__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_wrong_runner/baml_tests__diagnostic_errors__test_expr_wrong_runner__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_wrong_runner/baml_tests__diagnostic_errors__test_expr_wrong_runner__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_with_runner_ambiguity/baml_tests__diagnostic_errors__test_with_runner_ambiguity__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_with_runner_ambiguity/baml_tests__diagnostic_errors__test_with_runner_ambiguity__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_aliases/baml_tests__diagnostic_errors__type_aliases__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/type_reflection_strict/baml_tests__diagnostic_errors__type_reflection_strict__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/unknown_type_error/baml_tests__diagnostic_errors__unknown_type_error__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/void_return_type/baml_tests__diagnostic_errors__void_return_type__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__chained_optional_field_access.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_call_chain_continues.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_chaining_with_null_coalesce.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_field_access.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_field_access_through_optional_alias.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_index_through_optional_alias.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_method_call_basic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__class_field_gets_stream_prefix.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__cross_file_class_reference.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__dynamic_attr_passes_through.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__enum_field_unchanged.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__field_alias_preserved_on_stream_class.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__field_description_preserved_stream_done_stripped.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__list_field_recurses.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__map_field_recurses_value.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__optional_expands_to_union_with_null.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__primitives_get_null_union.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__recursive_class.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_companion_preserves_generic_args_for_llm_return_type.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_companion_preserves_generic_args_in_class_field.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_done_and_not_null.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_done_block_attr.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_done_field_keeps_type_as_is.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_not_null_block_attr_on_referenced_class.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__stream_not_null_field.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__type_alias_expansion.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__stream_expansion__union_field_recurses_variants.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!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_textual.snapis excluded by!**/*.snap
📒 Files selected for processing (18)
baml_language/crates/baml_codegen_types/src/ty.rsbaml_language/crates/baml_compiler2_ppir/src/expand.rsbaml_language/crates/baml_compiler2_tir/src/ty.rsbaml_language/crates/baml_lsp2_actions/src/type_info_tests.rsbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/client/remap_role_invalid_type.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/client/remap_role_non_string_allowed_roles.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/expr/instanceof_narrowing.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/optionals.bamlbaml_language/crates/baml_tests/baml_src/ns_reflect_type_of/reflect_type_of.bamlbaml_language/crates/baml_tests/baml_src/ns_reflect_type_of_generic/reflect_type_of_generic.bamlbaml_language/crates/baml_tests/src/compiler2_tir/inference.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a_recursion.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase6.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase7.rsbaml_language/crates/baml_tests/tests/env.rsbaml_language/crates/baml_type/src/lib.rsbaml_language/crates/baml_type/src/template.rs
✅ Files skipped from review due to trivial changes (4)
- baml_language/crates/baml_tests/tests/env.rs
- baml_language/crates/baml_tests/src/compiler2_tir/inference.rs
- baml_language/crates/baml_tests/src/compiler2_tir/phase6.rs
- baml_language/crates/baml_tests/src/compiler2_tir/phase3a_recursion.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/client/remap_role_non_string_allowed_roles.baml
- MIR receiver resolution: use `strip_null` instead of `peel_optional` so multi-member nullable unions (`(A | B | null)?.field`) drop `null` and route through the union, exactly matching the old `Optional(inner)` behavior. Removes the now-unused `peel_optional` helper and adds an end-to-end regression test. - Type display: parenthesize function members when rendering a union, so a nullable callback reads `((..) -> ..) | null` rather than a function whose `throws` type is `.. | null`. Applied in baml_type::Ty, the TIR renderer, and TyTemplate. - FFI value conversion: keep `null` in the recorded union type for a multi-member nullable union so the value stays marked optional and the nullable wire shape is preserved at the bridge boundary.
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_mir/src/lower.rs (1)
9698-9703:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
Uint8Arrayarray indexing in the optional-lvalue path.Line 9699 only treats
Ty::List(..)asIndexKind::Array, but Lines 6847 and 9589 still classifyTy::Uint8Array { .. }as array-like. That meansbytes?.[i] = ...now falls onto the map-index runtime path afterstrip_null(), which is the wrong lowering for nullable byte arrays.Suggested fix
- let kind = if matches!(&unwrapped_ty, Ty::List(..)) { + let kind = if matches!(&unwrapped_ty, Ty::List(..) | Ty::Uint8Array { .. }) { IndexKind::Array } else { IndexKind::Map };Please also add a Rust unit regression for nullable
Uint8Arrayoptional indexing/assignment so this stays aligned with the other index paths. As per coding guidelines,**/*.rs: Prefer writing Rust unit tests 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_mir/src/lower.rs` around lines 9698 - 9703, The optional-lvalue path currently only treats Ty::List(..) as IndexKind::Array after strip_null(), causing nullable Uint8Array to be lowered as a map; update the match in lower.rs (the block that computes kind from unwrapped_ty after calling strip_null()) so it treats Ty::Uint8Array { .. } the same as Ty::List(..) (i.e., produce IndexKind::Array for both), and add a Rust unit test (e.g., optional_uint8array_index_assignment_regression) that exercises nullable Uint8Array optional indexing/assignment (bytes?.[i] = ...) to assert correct lowering/behavior so future changes don’t regress this path.
🤖 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_mir/src/lower.rs`:
- Around line 9698-9703: The optional-lvalue path currently only treats
Ty::List(..) as IndexKind::Array after strip_null(), causing nullable Uint8Array
to be lowered as a map; update the match in lower.rs (the block that computes
kind from unwrapped_ty after calling strip_null()) so it treats Ty::Uint8Array {
.. } the same as Ty::List(..) (i.e., produce IndexKind::Array for both), and add
a Rust unit test (e.g., optional_uint8array_index_assignment_regression) that
exercises nullable Uint8Array optional indexing/assignment (bytes?.[i] = ...) to
assert correct lowering/behavior so future changes don’t regress this path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4237ed4b-943f-40e6-8616-25e89cddd8c2
⛔ Files ignored due to path filters (16)
baml_language/crates/baml_tests/snapshots/baml_src/null_handling.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/patterns_new_runtime.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/__testing_std__/baml_tests__compiles____testing_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__testing_std__/baml_tests__compiles____testing_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_alias_basic/baml_tests__compiles__json_alias_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/paren_union_test/baml_tests__compiles__paren_union_test__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/null_handling/baml_tests__diagnostic_errors__null_handling__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/void_return_type/baml_tests__diagnostic_errors__void_return_type__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_call_chain_continues.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!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_textual.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
baml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/ty.rsbaml_language/crates/baml_tests/baml_src/ns_null_handling/null_handling.bamlbaml_language/crates/baml_tests/src/compiler2_tir/phase6.rsbaml_language/crates/baml_type/src/lib.rsbaml_language/crates/baml_type/src/template.rsbaml_language/crates/bex_engine/src/conversion.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- baml_language/crates/baml_type/src/template.rs
- baml_language/crates/baml_compiler2_tir/src/ty.rs
- baml_language/crates/bex_engine/src/conversion.rs
- baml_language/crates/baml_tests/src/compiler2_tir/phase6.rs
Makes the optional-lvalue index site consistent with the two sibling index-lowering sites, which already treat Uint8Array as array-indexed. The path is currently unreachable for uint8array (the TIR type-checker rejects optional indexing on uint8array — E0008), so this is a defensive consistency fix with no behavior change; no regression test is possible because the scenario does not type-check.
This was referenced Jun 16, 2026
Merged
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.
Summary
Removes
Ty::Optionalas a distinct type.?is now syntax sugar for a union withnull:T?lowers directly toT | null(null last), and any union containingnullrenders back with the postfix?.The
Optionalvariant is deleted from all five typed-Tyenums:baml_type::Tyandbaml_type::TyTemplateTyandStructuralTybaml_codegen_types::TyThe syntax layers (
TypeExpr::Optional,PpirTy::Optional, the formatter ASTType::Optional) are kept — the?token still parses; only the lowering into a typedTychanged.Display
Any null-containing union renders with
?, everywhere user-facing (type display, LSP diagnostics, bytecode disassembly, codegen, LLM prompt schemas):int | nullint?int | string | null(int | string)?(string | null)[](string?)[]Why this direction (
T \| null, null last)Matches the existing internal desugaring, SAP's
null_to_endcanonical form, codegen conventions (Optional[T]/T \| null), and union coercion precedence (try the real type before the null fallback).Notable consequences (
T?andT \| nullare now identical everywhere)x: T | nullclass field is now absent-able and decodes to barenull(previously it was required and wrapped in union metadata).typing.Optional[T]for single-member nullable unions; TypeScript emitsT | null.OptionalTypetag when it detects a nullable union, preserving the wire format.int | string?) now flattens unions to leaves so the residual narrows correctly (regression caught by an adversarial review pass and pinned with a new test).Helpers added on the type enums:
nullable/optional,is_null,is_nullable_union,strip_null,peel_optional.Validation
#[ignore]/toolchain-gated skips, unchanged by this PR).cargo buildandcargo clippy --all-targetsclean.| null→?display change.Summary by CodeRabbit
T | nullinstead ofT?.nullfor nullable values and update encoding/decoding behavior.