Repository navigation
feat(bridges): unify outbound BamlTy onto baml_type.proto Ty - #3830
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (38)
✅ Files skipped from review due to trivial changes (10)
🚧 Files skipped from review as they are similar to previous changes (25)
📝 WalkthroughWalkthroughUnifies the CFFI wire type schema by renaming ChangesBamlTy Schema Unification and Encoding
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. 🔧 OpenGrep (1.23.0)baml_language/crates/bridge_ctypes/src/ty_encode.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.15][ERROR]: unable to find a config; path baml_language/crates/bridge_cffi/src/ffi/host_value.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.21][ERROR]: unable to find a config; path baml_language/crates/bridge_ctypes/src/value_encode.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.20][ERROR]: unable to find a config; path
🔧 ESLint
ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ 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):
|
|
No description provided. |
![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: 75b14cb273
ℹ️ 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".
Comment on lines
+171
to
+173
| BexExternalValue::Adt(BexExternalAdt::Type(rt)) => Some(BamlValueVariant::TyValue( | ||
| crate::ty_encode::runtime_ty_to_proto_ty(rt), | ||
| )), |
There was a problem hiding this comment.
For any reflect.type_of<T>() result, this new arm now emits the ty_value oneof instead of the previous handle_value/ADT_TYPE path, but the outbound decoders still do not handle that variant: Python decode_value falls through to return None, Node decodeValueHolder falls through to return null, and typescript2/pkg-proto's deserializeValue default also returns null. In those environments, returning a reflected type silently becomes null rather than a usable type/handle, so either the host decoders need a ty_value case or this should remain on the handle path until they do.
Useful? React with 👍 / 👎.
![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: 1
🤖 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 `@typescript2/pkg-proto/src/decode.ts`:
- Line 9: The import statement in the decode.ts file references a generated
TypeScript module for BamlHandleType that does not exist. Regenerate the
TypeScript protobuf bindings from the baml_handle.proto source file to create
the missing generated module. This will generate the necessary TypeScript
definitions for the BamlHandleType enum and its constants (HANDLE_UNSPECIFIED,
UNTAGGED_RUST_DATA, UNTAGGED_BEX_HEAP) that are being used in the decode.ts
file. Ensure the protobuf code generation tool is properly configured to output
TypeScript files to the expected location matching the import path.
🪄 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: 2802928f-e32b-441e-8af8-03ae5a6100ed
⛔ Files ignored due to path filters (9)
baml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_handle.pb.gois excluded by!**/*.pb.gobaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_inbound.pb.gois excluded by!**/*.pb.gobaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_outbound.pb.gois excluded by!**/*.pb.gobaml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (22)
baml_language/crates/bridge_ctypes/src/lib.rsbaml_language/crates/bridge_ctypes/src/ty_encode.rsbaml_language/crates/bridge_ctypes/src/value_encode.rsbaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_handle.protobaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_inbound.protobaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_outbound.protobaml_language/crates/bridge_ctypes/types/buf.yamlbaml_language/sdks/go/bridge_go/build.shbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/wire_ty.tsbaml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_outbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_outbound_pb2.pyibaml_language/sdks/python/src/baml_core/proto.pybaml_language/sdks/python/tests/test_decode_handle.pybaml_language/sdks/python/tests/test_proto_generics.pytypescript2/pkg-proto/src/decode.tstypescript2/pkg-proto/src/test/encode-decode.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/bridge_ctypes/src/value_encode.rs`:
- Around line 177-180: The code in the match statement for BexExternalValue::Adt
with BexExternalAdt::TaggedHeapHandle and in the other location around lines
234-252 should not use a fallback format string like format!("{ty}") for
class_ty names when handling non-nominal types. Instead, ensure that class_ty is
only set to Some when the type is actually a valid class or interface type by
validating the result of ty_to_class_ty. For non-class/interface types, return
None or propagate an error rather than fabricating a class name, to prevent
structural type strings like unknown or map<...> from being inserted into
typemap-key fields.
In `@baml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_type.proto`:
- Around line 10-11: The header comment describing the generic function call
contract claims hosts send an ordered BamlTy list, but the actual wire format
defined in CallFunctionArgs.type_args is repeated BamlTyArg (which includes both
a TypeVar name and a type_value field). Update the header comment to accurately
describe that the host sends an ordered list of BamlTyArg entries, not bare
BamlTy entries, so SDK authors understand the correct wire shape they need to
implement.
In `@baml_language/sdks/go/bridge_go/pkg/proto.go`:
- Around line 308-313: In the switch statement handling BamlLiteralValue cases
within the literal value decoding logic, add a missing case handler for
BamlLiteralValue_BigintLiteral. After the existing cases for StringLiteral,
IntLiteral, and BoolLiteral, add a new case that matches
*pb.BamlLiteralValue_BigintLiteral and returns the result of parsing the
BigintLiteral value using a parseHexBigint function (similar to how other SDKs
handle bigint literals with hex parsing logic). This will ensure the switch
statement handles all literal value types and maintains parity with the Python
and Node SDK implementations.
🪄 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: e5d1dbde-49f8-4e4c-b5c1-f64c2759930d
⛔ Files ignored due to path filters (14)
baml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_handle.pb.gois excluded by!**/*.pb.gobaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_inbound.pb.gois excluded by!**/*.pb.gobaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_outbound.pb.gois excluded by!**/*.pb.gobaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_type.pb.gois excluded by!**/*.pb.gobaml_language/sdks/nodejs/bridge_nodejs/dist/proto.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (34)
baml_language/crates/bridge_ctypes/src/lib.rsbaml_language/crates/bridge_ctypes/src/ty_decode.rsbaml_language/crates/bridge_ctypes/src/ty_encode.rsbaml_language/crates/bridge_ctypes/src/value_decode.rsbaml_language/crates/bridge_ctypes/src/value_encode.rsbaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_handle.protobaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_inbound.protobaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_outbound.protobaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_type.protobaml_language/crates/bridge_ctypes/types/buf.yamlbaml_language/sdks/go/bridge_go/build.shbaml_language/sdks/go/bridge_go/pkg/proto.gobaml_language/sdks/nodejs/bridge_nodejs/tests/test_decode_call_result.test.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/define_function.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/wire_ty.tsbaml_language/sdks/python/src/baml_core/__init__.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_outbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_outbound_pb2.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_type_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_type_pb2.pyibaml_language/sdks/python/src/baml_core/proto.pybaml_language/sdks/python/tests/test_decode_handle.pybaml_language/sdks/python/tests/test_host_callable.pybaml_language/sdks/python/tests/test_proto_decode_typemap.pybaml_language/sdks/python/tests/test_proto_generics.pytypescript2/pkg-proto/src/decode.tstypescript2/pkg-proto/src/test/encode-decode.test.ts
✅ Files skipped from review due to trivial changes (7)
- baml_language/crates/bridge_ctypes/types/buf.yaml
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_type_pb2.py
- baml_language/sdks/python/src/baml_core/init.py
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.pyi
- baml_language/sdks/python/tests/test_decode_handle.py
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.py
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_outbound_pb2.py
🚧 Files skipped from review as they are similar to previous changes (5)
- baml_language/crates/bridge_ctypes/src/lib.rs
- baml_language/sdks/go/bridge_go/build.sh
- baml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_handle.proto
- baml_language/crates/bridge_ctypes/src/ty_encode.rs
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.py
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/sdks/python/src/baml_core/proto.py (1)
432-439: 🎯 Functional Correctness | 🟠 MajorHandle PEP 604 unions before falling back to
unknown.Line 432 only recognizes
typing.Union[...]. On Python 3.10+,typing.get_origin(int | str)returnstypes.UnionType, soBox[int | str]type args encode asunknowninstead of a structural union.Proposed fix
+import types as _types import typing+_PEP604_UNION_TYPE = getattr(_types, "UnionType", None) + def _fill_wire_ty(ty: "baml_type_pb2.BamlTy", py_type: Any) -> None:- if origin is typing.Union: + if origin is typing.Union or ( + _PEP604_UNION_TYPE is not None and origin is _PEP604_UNION_TYPE + ): non_none = [a for a in targs if a is not type(None)]🤖 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/sdks/python/src/baml_core/proto.py` around lines 432 - 439, The condition at line 432 only checks for typing.Union but doesn't handle PEP 604 union syntax (e.g., int | str) which uses types.UnionType on Python 3.10+. Update the condition that checks if origin is typing.Union to also include a check for types.UnionType (import the types module if needed) so that both union syntaxes are properly handled by the _fill_inner function and encode as structural unions instead of falling back to unknown.
🤖 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/sdks/python/src/baml_core/proto.py`:
- Around line 286-301: The Pydantic model branch (around line 286-301) has been
updated to assign the FQN to cv.class_ty.name, but the media class branch
handling BamlImage, BamlAudio, and BamlPdf (around line 253) still uses the old
cv.name assignment pattern. Update the media class encoder branch to use
cv.class_ty.name instead of cv.name to maintain consistency with the Pydantic
model approach and ensure media wrapper FQNs are properly preserved during
serialization.
---
Outside diff comments:
In `@baml_language/sdks/python/src/baml_core/proto.py`:
- Around line 432-439: The condition at line 432 only checks for typing.Union
but doesn't handle PEP 604 union syntax (e.g., int | str) which uses
types.UnionType on Python 3.10+. Update the condition that checks if origin is
typing.Union to also include a check for types.UnionType (import the types
module if needed) so that both union syntaxes are properly handled by the
_fill_inner function and encode as structural unions instead of falling back to
unknown.
🪄 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: 6e5cdcbe-d4c6-48ce-84f6-20d8409266d9
⛔ Files ignored due to path filters (14)
baml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_handle.pb.gois excluded by!**/*.pb.gobaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_inbound.pb.gois excluded by!**/*.pb.gobaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_outbound.pb.gois excluded by!**/*.pb.gobaml_language/sdks/go/bridge_go/cffi/proto/baml_core/cffi/v1/baml_type.pb.gois excluded by!**/*.pb.gobaml_language/sdks/nodejs/bridge_nodejs/dist/proto.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto.js.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/proto/baml_cffi.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/wire_ty.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (38)
baml_language/crates/bridge_cffi/src/ffi/host_value.rsbaml_language/crates/bridge_ctypes/src/lib.rsbaml_language/crates/bridge_ctypes/src/ty_decode.rsbaml_language/crates/bridge_ctypes/src/ty_encode.rsbaml_language/crates/bridge_ctypes/src/value_decode.rsbaml_language/crates/bridge_ctypes/src/value_encode.rsbaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_handle.protobaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_inbound.protobaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_outbound.protobaml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_type.protobaml_language/crates/bridge_ctypes/types/buf.yamlbaml_language/crates/bridge_wasm/tests/host_callable.rsbaml_language/sdks/go/bridge_go/build.shbaml_language/sdks/go/bridge_go/pkg/proto.gobaml_language/sdks/nodejs/bridge_nodejs/tests/test_decode_call_result.test.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/define_function.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.d.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/wire_ty.tsbaml_language/sdks/python/rust/bridge_python/src/host_value.rsbaml_language/sdks/python/src/baml_core/__init__.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_outbound_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_outbound_pb2.pyibaml_language/sdks/python/src/baml_core/cffi/v1/baml_type_pb2.pybaml_language/sdks/python/src/baml_core/cffi/v1/baml_type_pb2.pyibaml_language/sdks/python/src/baml_core/proto.pybaml_language/sdks/python/tests/test_decode_handle.pybaml_language/sdks/python/tests/test_host_callable.pybaml_language/sdks/python/tests/test_proto_decode_typemap.pybaml_language/sdks/python/tests/test_proto_generics.pytypescript2/pkg-proto/src/decode.tstypescript2/pkg-proto/src/encode.tstypescript2/pkg-proto/src/test/encode-decode.test.ts
✅ Files skipped from review due to trivial changes (10)
- baml_language/sdks/go/bridge_go/build.sh
- baml_language/crates/bridge_ctypes/types/buf.yaml
- baml_language/sdks/nodejs/bridge_nodejs/tests/test_decode_call_result.test.ts
- baml_language/sdks/python/src/baml_core/init.py
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.pyi
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.py
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_outbound_pb2.py
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_type_pb2.py
- baml_language/sdks/python/tests/test_proto_decode_typemap.py
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_handle_pb2.py
🚧 Files skipped from review as they are similar to previous changes (15)
- baml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_handle.proto
- baml_language/sdks/python/tests/test_host_callable.py
- baml_language/sdks/go/bridge_go/pkg/proto.go
- baml_language/sdks/nodejs/bridge_nodejs/typescript_src/define_function.ts
- baml_language/crates/bridge_ctypes/src/lib.rs
- baml_language/sdks/python/tests/test_decode_handle.py
- typescript2/pkg-proto/src/decode.ts
- baml_language/crates/bridge_ctypes/src/ty_decode.rs
- baml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_inbound.proto
- baml_language/crates/bridge_ctypes/types/baml_core/cffi/v1/baml_type.proto
- baml_language/crates/bridge_ctypes/src/ty_encode.rs
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_type_pb2.pyi
- typescript2/pkg-proto/src/test/encode-decode.test.ts
- baml_language/crates/bridge_ctypes/src/value_encode.rs
- baml_language/sdks/python/src/baml_core/cffi/v1/baml_inbound_pb2.pyi
Consolidate the two on-the-wire "a BAML type" encodings in bridge_ctypes onto a single representation. `BamlTy` (baml_outbound.proto) was an older, lossy type-metadata encoding riding alongside outbound container values; `Ty` (baml_type.proto) is the faithful, complete mirror of `baml_type::RuntimeTy` already used inbound. This collapses the two parallel per-SDK mappings (native->Ty writing, BamlTy->native reading) into one. Main change (02a): - New ty_encode.rs: runtime_ty_to_proto_ty(&RuntimeTy) -> Ty, the total inverse of ty_decode.rs (all 24 variants) and strictly more capable than the old ty_to_field_type, which collapsed many variants to unknown/unreachable!. Also wires the previously-dead outbound ty_value (field 21) for BexExternalAdt::Type. - Repoint the four embed sites (BamlValueList.item_type, BamlValueMap.key_type/value_type, BamlValueUnionVariant.self_type, BamlTyGenericArg.ty) from BamlTy to Ty; drop ty_to_field_type. - Unions encode structurally: nullable-single -> Optional, else Ty::Union (mirrors the inbound writer). Type-position host readers map a structural union to a wildcard. - Migrate the SDK type-position readers: Python _ty_to_python_type and Node outboundTyToBamlType now consume Ty (the literal inverse of their inbound writers); the any_type branch is gone. Go has no type-position reader (only value-level BamlTyLiteral, retained) -> regen only. - Delete the BamlTy message tree + BamlTyAny. Keep BamlTyLiteral / BamlLiteral* (they back the value-level BamlOutboundValue.literal_value) and BamlTyName / BamlTyGenericArg (now carrying Ty). Side quest (02a §7): - Extract BamlHandleType + BamlHandle into baml_handle.proto so baml_outbound.proto imports it directly, dropping the outbound->inbound proto dependency edge. Python/TS2 import repoints; Rust/Node/Go regen. Regenerated all five proto clients. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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
Consolidates the two on-the-wire "a BAML type" encodings in
bridge_ctypesonto a single representation.BamlTy(baml_outbound.proto) was an older, lossy type-metadata encoding riding alongside outbound container values;Ty(baml_type.proto) is the faithful, complete mirror ofbaml_type::RuntimeTyalready used inbound. This collapses the two parallel per-SDK mappings (native→Tywriting,BamlTy→native reading) into one per SDK.Implements
02a-unify-bamlty-and-ty.md.Main change
ty_encode.rs:runtime_ty_to_proto_ty(&RuntimeTy) -> Ty— the total inverse ofty_decode.rs(all 24 variants), strictly more capable than the oldty_to_field_type(which collapsed many variants tounknown/unreachable!). Also wires the previously-dead outboundty_value(field 21) forBexExternalAdt::Type.BamlValueList.item_type,BamlValueMap.key_type/value_type,BamlValueUnionVariant.self_type,BamlTyGenericArg.ty) fromBamlTy→Ty; dropty_to_field_type.Optional, elseTy::Union(mirrors the inbound writer). Type-position host readers map a structural union to a wildcard (§5)._ty_to_python_typeand NodeoutboundTyToBamlTypenow consumeTy— the literal inverse of their inbound writers; theany_typebranch is gone. Go has no type-position reader (only value-levelBamlTyLiteral, retained) → regen only.BamlTymessage tree +BamlTyAny. KeptBamlTyLiteral/BamlLiteral*(they back the value-levelBamlOutboundValue.literal_value) andBamlTyName/BamlTyGenericArg(now carryingTy).Side quest (§7)
Extract
BamlHandleType+BamlHandleintobaml_handle.protosobaml_outbound.protoimports it directly, dropping the outbound→inbound proto dependency edge. Python/TS2 import repoints; Rust/Node/Go regen-only.All five proto clients regenerated.
Verification
bridge_ctypesbuilds + 30 unit tests pass;bridge_python/bridge_cffi/bridge_wasm/baml_lsp_servercargo-check clean.Tyshape, all pass (4 unrelated failures confirmed pre-existing on canary).$typesround-trip); TS typechecks.Ty$caseshape).HostCallableErrorbreakage untouched).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Refactor
BamlTytype representation for primitives, containers, optionals/unions, and generic arguments.literal_valueformat.Bug Fixes
Chores