feat(generics): inbound value-inference + baml_type_runtime + union-TypeVar fix#3833
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (30)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (23)
📝 WalkthroughWalkthroughAdds shared runtime generic inference, class arity metadata, host-only value synthesis, generic-call validation, structured type-mismatch transport, Python ChangesShared Generic Inference and Host Transport
Sequence Diagram(s)sequenceDiagram
participant SDK as baml_core.__init__
participant Engine as BexEngine::call_function_bound_args_with_trace
participant Conversion as bex_engine::conversion::check_generic_arg
participant Bridge as bridge_cffi::result_to_outbound
participant Proto as baml_core.proto::decode_call_result
SDK->>Engine: send arguments and `_types`
Engine->>Conversion: infer and validate generic bindings
Conversion-->>Engine: bindings or TypeMismatch
Engine->>Bridge: return runtime result
Bridge->>Proto: emit `baml.errors.TypeMismatch`
Proto->>SDK: raise Python TypeError
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.0)baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsThanks 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 |
⏭️ 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):
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b67b347b45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
No description provided. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
b67b347 to
35852c6
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_type/src/generics.rs (1)
288-330: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider co-locating unit tests for these pure primitives.
This new module hosts the shared inference/union algorithm but has no in-crate
#[cfg(test)]coverage. The unifier unit tests live inbex_engine(conversion.rs) and exercise the runtime port, not thesebaml_typeentrypoints directly. A small#[cfg(test)] mod testshere (e.g.union_tydedup/flatten,normalize_union_memberscollapse-to-Never/bare,nullable_non_null_part) would lock in behavior at the source.As per coding guidelines: "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_type/src/generics.rs` around lines 288 - 330, Add a #[cfg(test)] module at the end of the generics.rs file to provide unit test coverage for the pure primitive functions. Create tests for union_ty to verify deduplication and flattening of nested unions, tests for normalize_union_members to verify it correctly collapses to Never for empty input, returns a bare type for single member input, and properly constructs unions for multiple members, and tests for nullable_non_null_part if it exists in this module. These tests should directly exercise the baml_type entrypoints rather than relying on tests in bex_engine conversion.rs which test the runtime port.Source: Coding guidelines
🤖 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_type/src/generics.rs`:
- Around line 288-330: Add a #[cfg(test)] module at the end of the generics.rs
file to provide unit test coverage for the pure primitive functions. Create
tests for union_ty to verify deduplication and flattening of nested unions,
tests for normalize_union_members to verify it correctly collapses to Never for
empty input, returns a bare type for single member input, and properly
constructs unions for multiple members, and tests for nullable_non_null_part if
it exists in this module. These tests should directly exercise the baml_type
entrypoints rather than relying on tests in bex_engine conversion.rs which test
the runtime port.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 850bbecb-9f9b-43ae-8e61-72396a2d2199
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
baml_language/crates/baml_compiler2_tir/src/generics.rsbaml_language/crates/baml_type/Cargo.tomlbaml_language/crates/baml_type/src/generics.rsbaml_language/crates/baml_type/src/lib.rsbaml_language/crates/bex_engine/Cargo.tomlbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/generics_inference.rsbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_calls.pybaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.pybaml_language/sdks/python/src/baml_core/__init__.py
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/bex_engine/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- baml_language/crates/bex_engine/tests/generics_inference.rs
- baml_language/crates/bex_engine/src/conversion.rs
- baml_language/crates/bex_engine/src/lib.rs
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
35852c6 to
d19a0b4
Compare
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
d19a0b4 to
08a8b10
Compare
sxlijin
changed the title
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
08a8b10 to
5fff2bc
Compare
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
5fff2bc to
8edd37d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
baml_language/crates/baml_tests/tests/match_union_typevar.rs (1)
61-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReject unexpected diagnostic IDs in these regression tests.
containslets the test pass even if the compiler also emits unrelated errors. Assert the expected diagnostic set, or at least verify every emitted ID is expected.Example tightening
assert!( errors.contains(&DiagnosticId::NonExhaustiveMatch), "expected NonExhaustiveMatch when the `let v: T` arm is dropped, got: {errors:?}" ); + assert!( + errors.iter().all(|id| *id == DiagnosticId::NonExhaustiveMatch), + "unexpected diagnostics alongside NonExhaustiveMatch: {errors:?}" + );assert!( errors.contains(&DiagnosticId::UnreachableArm), "expected concrete arms after a bare `let v: T` to be unreachable, got: {errors:?}" ); + assert!( + errors.iter().all(|id| *id == DiagnosticId::UnreachableArm), + "unexpected diagnostics alongside UnreachableArm: {errors:?}" + );Also applies to: 84-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_tests/tests/match_union_typevar.rs` around lines 61 - 65, The regression test currently only checks that DiagnosticId::NonExhaustiveMatch is present, so unrelated compiler diagnostics can still slip through. Tighten the assertion in match_union_typevar.rs by validating the full diagnostic set returned from error_ids(src), or by explicitly asserting that every emitted ID is one of the expected IDs in this test case. Update the affected test block(s) using error_ids and assert to reject any unexpected diagnostic IDs.
🤖 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_type_runtime/src/lib.rs`:
- Around line 127-132: The same-length Ty::Union matching path in
infer_bindings_inner is taking precedence over the residual TypeVar logic, so
route union inference through the residual solver before doing any positional
zip. Update the union handling block to first subtract concrete siblings from
the residual set and only then bind the remaining TypeVar portion, instead of
inferring member-to-member order directly. Make the same change anywhere the
same union arm logic is duplicated in the referenced range so cases like T |
string against string | int resolve to T = int rather than order-based binding.
- Around line 367-397: normalize_union_members only flattens one Union level, so
nested Ty::Union values can survive and keep duplicates. Update the logic in
normalize_union_members to recursively unwrap union members (including unions
nested inside inner unions) before deduplicating and pushing into normalized,
while still skipping Ty::Never and preserving the existing TyAttr behavior. Use
the normalize_union_members function and Ty::Union/Ty::Never matching as the
main entry points for the fix.
In
`@baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py`:
- Around line 3-7: The module docstring in test_generic_inference.py contradicts
the actual test coverage because it says there is no _types= usage, but the
suite includes a partial _types binding case in the relevant generic-call test.
Update the top-level docstring to accurately describe that the file mainly
covers bare calls while also including a partial _types= scenario, and verify
the wording matches the behavior exercised by the generic inference tests.
---
Nitpick comments:
In `@baml_language/crates/baml_tests/tests/match_union_typevar.rs`:
- Around line 61-65: The regression test currently only checks that
DiagnosticId::NonExhaustiveMatch is present, so unrelated compiler diagnostics
can still slip through. Tighten the assertion in match_union_typevar.rs by
validating the full diagnostic set returned from error_ids(src), or by
explicitly asserting that every emitted ID is one of the expected IDs in this
test case. Update the affected test block(s) using error_ids and assert to
reject any unexpected diagnostic IDs.
🪄 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: 01dc3445-8e91-43f9-9a45-0a6a2e6dfbff
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
baml_language/Cargo.tomlbaml_language/crates/baml_compiler2_tir/Cargo.tomlbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/generics.rsbaml_language/crates/baml_tests/tests/match_union_typevar.rsbaml_language/crates/baml_type/Cargo.tomlbaml_language/crates/baml_type_runtime/Cargo.tomlbaml_language/crates/baml_type_runtime/src/lib.rsbaml_language/crates/bex_engine/Cargo.tomlbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/generics_inference.rsbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_calls.pybaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.pybaml_language/sdks/python/src/baml_core/__init__.py
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/baml_type_runtime/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (5)
- baml_language/crates/baml_type/Cargo.toml
- baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_calls.py
- baml_language/crates/bex_engine/src/lib.rs
- baml_language/sdks/python/src/baml_core/init.py
- baml_language/crates/bex_engine/src/conversion.rs
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
8edd37d to
d1c5493
Compare
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
41b2537 to
9bc50ce
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py (1)
54-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the transported
baml_tracein the shared helper.
decode_call_result()attachesbaml_tracetoTypeMismatch-derivedTypeErrors, but this helper only checks the exception type/message. These tests would still pass if the structured trace transport regressed. Please pin that contract here too.Suggested tweak
def _assert_type_error(call, *needles): """A failed generic call must surface as a Python ``TypeError`` (the engine's `EngineError::TypeMismatch` ⇒ `baml.errors.TypeMismatch` ⇒ native `TypeError`, not an opaque `BamlPanic`), and its message must mention each `needle`. Pins both the *type* and the *message* of every inference rejection.""" with pytest.raises(TypeError) as excinfo: call() + assert isinstance(getattr(excinfo.value, "baml_trace", None), list) message = str(excinfo.value) for needle in needles: assert needle in message, f"missing {needle!r} in TypeError message: {message!r}" return excinfo🤖 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/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py` around lines 54 - 64, The shared helper in _assert_type_error currently verifies only the TypeError type and message, but it should also pin the structured baml_trace attached by decode_call_result() for TypeMismatch-derived errors. Update this helper to assert the raised exception exposes the expected baml_trace payload, so regressions in trace transport are caught alongside message checks. Keep the check centralized here so all customizable generic inference tests exercise the same contract.
🤖 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/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py`:
- Around line 54-64: The shared helper in _assert_type_error currently verifies
only the TypeError type and message, but it should also pin the structured
baml_trace attached by decode_call_result() for TypeMismatch-derived errors.
Update this helper to assert the raised exception exposes the expected
baml_trace payload, so regressions in trace transport are caught alongside
message checks. Keep the check centralized here so all customizable generic
inference tests exercise the same contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: aaa4a5f4-2bc6-4b1a-8bb1-f5183fab40a6
⛔ Files ignored due to path filters (8)
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/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.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/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!**/*.snapbaml_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 (17)
baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.bamlbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/generics_inference.rsbaml_language/crates/bex_heap/src/gc.rsbaml_language/crates/bex_heap/src/tlab.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm/tests/method_class_type_args.rsbaml_language/crates/bex_vm_types/src/types/class.rsbaml_language/crates/bridge_cffi/src/baml_to_host.rsbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_calls.pybaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.pybaml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/roundtrip_tests/test_maps.pybaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_generic_tests/types.bamlbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/python/src/baml_core/proto.py
✅ Files skipped from review due to trivial changes (3)
- baml_language/crates/bex_vm/src/vm.rs
- baml_language/crates/bex_heap/src/tlab.rs
- baml_language/crates/bex_heap/src/gc.rs
🚧 Files skipped from review as they are similar to previous changes (11)
- baml_language/crates/baml_compiler2_emit/src/lib.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.baml
- baml_language/crates/bex_vm_types/src/types/class.rs
- baml_language/crates/bridge_cffi/src/baml_to_host.rs
- baml_language/crates/bex_vm/tests/method_class_type_args.rs
- baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_calls.py
- baml_language/sdks/python/src/baml_core/proto.py
- baml_language/crates/bex_engine/src/lib.rs
- baml_language/sdk_tests/fixtures/function_calls/baml_src/ns_generic_tests/types.baml
- baml_language/crates/bex_engine/src/conversion.rs
- baml_language/crates/bex_engine/tests/generics_inference.rs
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
9bc50ce to
2ca4684
Compare
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
2ca4684 to
23a2236
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py (1)
54-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the attached BAML trace in the shared
TypeErrorhelper.
decode_call_result()now re-raises these failures as nativeTypeErrorwithbaml_trace, but this helper only checks the exception type and message. A regression in trace attachment would leave the whole negative suite green.Suggested tweak
def _assert_type_error(call, *needles): """A failed generic call must surface as a Python ``TypeError`` (the engine's `EngineError::TypeMismatch` ⇒ `baml.errors.TypeMismatch` ⇒ native `TypeError`, not an opaque `BamlPanic`), and its message must mention each `needle`. Pins both the *type* and the *message* of every inference rejection.""" with pytest.raises(TypeError) as excinfo: call() + assert hasattr(excinfo.value, "baml_trace") message = str(excinfo.value) for needle in needles: assert needle in message, f"missing {needle!r} in TypeError message: {message!r}" return excinfo🤖 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/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py` around lines 54 - 64, The shared negative-test helper `_assert_type_error` only verifies the native `TypeError` message, so it can miss regressions where `decode_call_result()` stops attaching `baml_trace`. Update `_assert_type_error` in `test_generic_inference.py` to also assert the caught exception has the expected BAML trace attached (for example via the exception’s trace attribute/metadata used by `decode_call_result`). Keep the existing checks on type and message needles, and make the trace assertion part of the same helper so all generic-inference failure cases validate it consistently.baml_language/crates/baml_type/src/template.rs (1)
1-1: 📐 Maintainability & Code Quality | 🔵 TrivialScope the deprecation expectation to the specific
TypeArgRefOrWildcardmatches.baml_language/crates/baml_type/src/template.rs:1uses a crate-level#![expect(deprecated)], which suppresses any future deprecated use insidebaml_type; move this to the concrete match sites that still need it so unrelated deprecations keep surfacing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_type/src/template.rs` at line 1, The crate-level deprecated expectation in template.rs is too broad and hides unrelated future deprecations. Move the `#[expect(deprecated)]` handling from the top of the file to the specific `TypeArgRefOrWildcard` match sites inside the relevant template logic, so only those uses are suppressed and other deprecated APIs still surface.
🤖 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/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py`:
- Around line 197-206: Update the comment near the partial `_types=` binding in
`test_generic_inference` so it matches the Python SDK contract: `_types=` should
be described as a supported user-facing way to bind type parameters, especially
for uninferable own type params, not as mostly internal wiring. Keep the
reference to `make_triple` and the partial-bind case, but remove guidance that
prefers subscripts over `_types=` and align the wording with the later `_types=`
usage in `one_type_arg()` and `parse_as()`.
---
Nitpick comments:
In `@baml_language/crates/baml_type/src/template.rs`:
- Line 1: The crate-level deprecated expectation in template.rs is too broad and
hides unrelated future deprecations. Move the `#[expect(deprecated)]` handling
from the top of the file to the specific `TypeArgRefOrWildcard` match sites
inside the relevant template logic, so only those uses are suppressed and other
deprecated APIs still surface.
In
`@baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py`:
- Around line 54-64: The shared negative-test helper `_assert_type_error` only
verifies the native `TypeError` message, so it can miss regressions where
`decode_call_result()` stops attaching `baml_trace`. Update `_assert_type_error`
in `test_generic_inference.py` to also assert the caught exception has the
expected BAML trace attached (for example via the exception’s trace
attribute/metadata used by `decode_call_result`). Keep the existing checks on
type and message needles, and make the trace assertion part of the same helper
so all generic-inference failure cases validate it consistently.
🪄 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: f30592af-a9a1-4105-85e7-d5094a34f0fb
⛔ Files ignored due to path filters (8)
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/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.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/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!**/*.snapbaml_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 (18)
baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.bamlbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_type/src/template.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_engine/tests/generics_inference.rsbaml_language/crates/bex_heap/src/gc.rsbaml_language/crates/bex_heap/src/tlab.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm/tests/method_class_type_args.rsbaml_language/crates/bex_vm_types/src/types/class.rsbaml_language/crates/bridge_cffi/src/baml_to_host.rsbaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_calls.pybaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.pybaml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/roundtrip_tests/test_maps.pybaml_language/sdk_tests/fixtures/function_calls/baml_src/ns_generic_tests/types.bamlbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/python/src/baml_core/proto.py
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/bex_heap/src/tlab.rs
🚧 Files skipped from review as they are similar to previous changes (15)
- baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/errors.baml
- baml_language/crates/baml_compiler2_emit/src/lib.rs
- baml_language/crates/bex_vm/src/vm.rs
- baml_language/crates/bex_vm/tests/method_class_type_args.rs
- baml_language/crates/bridge_cffi/src/baml_to_host.rs
- baml_language/crates/bex_heap/src/gc.rs
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.js
- baml_language/sdk_tests/fixtures/function_calls/baml_src/ns_generic_tests/types.baml
- baml_language/crates/bex_vm_types/src/types/class.rs
- baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_calls.py
- baml_language/crates/bex_engine/src/lib.rs
- baml_language/sdks/python/src/baml_core/proto.py
- baml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/roundtrip_tests/test_maps.py
- baml_language/crates/bex_engine/tests/generics_inference.rs
- baml_language/crates/bex_engine/src/conversion.rs
Comment on lines
+197
to
+206
| # C2/T17: bind A explicitly via a partial `_types=` dict; B and C are inferred. | ||
| # | ||
| # NOTE: this is an *unusual* situation — only SOME type vars are explicitly | ||
| # bound while the rest are inferred. Users should generally NOT reach for | ||
| # `_types=` at all: inbound inference binds every value-carried TypeVar from | ||
| # the arguments (see the rest of this file), and the explicit *subscript* | ||
| # form (`make_triple[int, str, bool](...)`, test_make_triple_subscript_*) is | ||
| # the supported surface for the rare case where a binding must be forced. | ||
| # `_types=` is an internal wiring detail kept mainly for this partial-bind | ||
| # escape hatch; prefer plain inference. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The _types= guidance here no longer matches the SDK contract.
These comments describe _types= as mostly internal and steer readers to subscripts, but the Python binding treats _types= as a supported user-facing path and the only way to bind uninferable own type params. That mismatch is especially confusing since this same file uses _types= later for one_type_arg() and parse_as().
Suggested wording
- # NOTE: this is an *unusual* situation — only SOME type vars are explicitly
- # bound while the rest are inferred. Users should generally NOT reach for
- # `_types=` at all: inbound inference binds every value-carried TypeVar from
- # the arguments (see the rest of this file), and the explicit *subscript*
- # form (`make_triple[int, str, bool](...)`, test_make_triple_subscript_*) is
- # the supported surface for the rare case where a binding must be forced.
- # `_types=` is an internal wiring detail kept mainly for this partial-bind
- # escape hatch; prefer plain inference.
+ # NOTE: this is an unusual case — only SOME type vars are explicitly bound
+ # while the rest are inferred. Plain inference is preferred when the values
+ # carry enough evidence, but `_types=` remains the supported escape hatch for
+ # partial binding and for type vars that no argument can carry.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # C2/T17: bind A explicitly via a partial `_types=` dict; B and C are inferred. | |
| # | |
| # NOTE: this is an *unusual* situation — only SOME type vars are explicitly | |
| # bound while the rest are inferred. Users should generally NOT reach for | |
| # `_types=` at all: inbound inference binds every value-carried TypeVar from | |
| # the arguments (see the rest of this file), and the explicit *subscript* | |
| # form (`make_triple[int, str, bool](...)`, test_make_triple_subscript_*) is | |
| # the supported surface for the rare case where a binding must be forced. | |
| # `_types=` is an internal wiring detail kept mainly for this partial-bind | |
| # escape hatch; prefer plain inference. | |
| # C2/T17: bind A explicitly via a partial `_types=` dict; B and C are inferred. | |
| # | |
| # NOTE: this is an unusual case — only SOME type vars are explicitly bound | |
| # while the rest are inferred. Plain inference is preferred when the values | |
| # carry enough evidence, but `_types=` remains the supported escape hatch for | |
| # partial binding and for type vars that no argument can carry. |
🤖 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/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_generic_inference.py`
around lines 197 - 206, Update the comment near the partial `_types=` binding in
`test_generic_inference` so it matches the Python SDK contract: `_types=` should
be described as a supported user-facing way to bind type parameters, especially
for uninferable own type params, not as mostly internal wiring. Keep the
reference to `make_triple` and the partial-bind case, but remove guidance that
prefers subscripts over `_types=` and align the wording with the later `_types=`
usage in `one_type_arg()` and `parse_as()`.
…ypeVar fix Let Python call generic BAML functions without explicit type args: the engine solves each TypeVar from argument values, round-tripping through the Python<->BAML FFI. Extract the shared Ty-walking generic inference into a new baml_type_runtime crate that depends only on baml_type (not the compiler frontend, not BEX), so the TIR (compile-time) and the Python bridge (runtime) share ONE inference implementation. Verified: no bex_engine -> baml_compiler2_tir dependency edge. Fix the union-with-concrete-sibling TypeVar bug (reverses 00b3 G5), e.g. tag_or_value<T>(x: T | string | null). Inference: a new arm subtracts the concrete siblings an actual already satisfies and routes the residual to the lone TypeVar member (coercion-free; >1 TypeVar => ambiguous => unbound). Introspection: union_targets_for_pattern is made atom-aware so a concrete pattern no longer over-claims an open TypeVar union member (fixing a spurious E0063 unreachable arm) without regressing the standard library's nullable-TypeVar matches. Tested at the baml_type_runtime, bex_engine, baml_tests, and python_pydantic2 layers; full Python->BAML round-trip verified (tag_or_value(5) == "int"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… + opaque round-trip carrier
…t-specify, deep unbound recursion Tests (bex + python): - heterogeneous array element unification (elem_type) - unbound-instance forcing-formal recovery (ContainerShapes, GenericRecursive, nested GenericPair) — now incl. FULLY-unbound deep nesting - concrete baml_sdk types in covariant joins (enum + class) - partial _types= noted as unusual; prefer inference / subscript Inference errors -> native TypeError: - route EngineError::TypeMismatch to a structured baml.errors.TypeMismatch (bridge_cffi) instead of an opaque SdkPanic; map to Python TypeError in proto.py; assert type + message on every must-specify / conflict case - add baml.errors.TypeMismatch stdlib class; refresh builtin/HIR/TIR/bytecode snapshots Class-method must-specify (rule 3): - carry generic_param_count on the runtime Class; Gate A demands a method's OWN generic params (suffix after the class prefix), so a body-only own var (reflect_t<T>()) is rejected, matching the free-function analogue Deep unbound recursion: - reconstruct_unbound_instance_args is formal-aware, recursing through nested unbound instances so all vars are recovered from leaf field values Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sxlijin
force-pushed
the
sam/inbound-inferred
branch
from
23a2236 to
53f3bec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Lets a Python caller invoke a generic BAML function without specifying type args — the engine solves each
TypeVarfrom argument values, round-tripping through the Python↔BAML FFI (e.g.identity(k) == k).Changes
New
baml_type_runtimecrate. Extracts the sharedTy-walking generic inference out ofbaml_type::genericsinto a dedicated crate that depends only onbaml_type— not the compiler frontend, not BEX. The TIR (compile-time) and the Python bridge (runtime) now share one inference implementation. Verified:cargo tree -p bex_enginehas nobaml_compiler2_tiredge.Union-
TypeVarbug fixed (reverses the priorG5carve-out).tag_or_value<T>(x: T | string | null)now works, both halves:infer_bindings_innersubtracts the concrete union siblings an actual already satisfies (coercion-freeis_subtype_of, sointis not absorbed by afloatsibling) and routes the residual to the loneTypeVar.>1TypeVar member ⇒ ambiguous ⇒ left unbound (Gate A governs). The equal-length union↔union positional-zip arm is now guarded to only fire when the formal has no directTypeVarmember (otherwise a same-length actual union would bind by accidental ordering instead of routing the residual — a soundness fix).union_targets_for_patternis made atom-aware so a concrete pattern (let s: string,null) no longer over-claims an openTypeVarunion member (which spuriously reported thelet v: Tarm as[E0063] unreachable), while preserving the standard library's nullable-TypeVarmatches (filter_map/sort_by_keyoverU | null) and the streamingTStream | StreamNoYieldbinding.Value-synthesis fixes at the
call_functioninterface.synth_ty_from_valuenow binds concrete BAML types forMedia(image/audio/…),Collector(resource), andPromptAstargs instead of erasing them to host-onlyrust_type; and a non-empty map whose values carry no evidence still binds itsstringkey type (K) instead of collapsing to no-evidence.Tests (each fix lands with a named test)
baml_type_runtime: 10 unit tests (route-to-typevar, concrete-sibling-absorbs, null routing, multi-typevar ambiguity, float-doesn't-absorb-int, union-precedence/positional-zip guard).bex_engine/tests/generics_inference.rs: 23 passing (incl. end-to-endtag_or_value(5) → 5round-trip, media binding, non-empty-map key) + 3#[ignore]d tests documenting deferred bugs (class-method body-only Gate A hole; empty-args generic Instance; theM27null-sibling spec discrepancy).baml_tests/tests/match_union_typevar.rs: compiles; missinglet v: T⇒ non-exhaustive; usefulness preserved;TypeVar | Classunion binding compiles.python_pydantic2:tag_or_value(5) == "int"andtag_or_value("hi")rejects — verified via the full Python→BAML FFI round-trip.Verification
cargo build --workspace;cargo test -p baml_type_runtime -p baml_compiler2_tir -p bex_engine -p baml_tests;cargo nextest run -p sdk_test_python_pydantic2 function_calls::pytest;cargo clippy -D warningsandcargo fmt --checkon touched crates — all green.Design/research/alternatives notes (incl. real A2/A3 prototypes, both confirm-discard) live under
thoughts/sam-projects/bridge-generics/inbound-inference/(02a–02c,02-spike-A2/A3-result.md,02-bug-hunt.md).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
_types=.Bug Fixes