test(sdk_tests): cover interface types (BEP-044) at the SDK boundary#3939
test(sdk_tests): cover interface types (BEP-044) at the SDK boundary#3939sxlijin wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ 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 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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds a BAML interface fixture plus Python and TypeScript SDK tests covering interface-typed returns, parameter dispatch, and roundtrip behavior, including pinned known-gap failures. ChangesInterface Fixture and Roundtrip Tests
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test Suite
participant BAML Runtime
participant Encode/FFI Boundary
participant Engine
Test Suite->>BAML Runtime: call Shape-typed function
BAML Runtime->>Encode/FFI Boundary: encode value across boundary
Encode/FFI Boundary->>Engine: dispatch area() on Shape
alt concrete implementor
Engine-->>Encode/FFI Boundary: resolved result
Encode/FFI Boundary-->>BAML Runtime: decoded value
BAML Runtime-->>Test Suite: concrete class instance or result
else non-implementor or lost class identity
Engine-->>Encode/FFI Boundary: dispatch failure
Encode/FFI Boundary-->>BAML Runtime: error propagated
BAML Runtime-->>Test Suite: throw or reject
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e94c57d438
ℹ️ 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".
| import pytest | ||
|
|
||
| import baml_sdk # noqa: F401 — initializes the BAML runtime | ||
| from baml_core.errors import BamlPanic |
There was a problem hiding this comment.
Import BamlPanic from the generated SDK
In the Python SDK test fixture, this module is collected inside the generated per-fixture environment, whose pyproject installs baml_bridge and exposes the public error wrappers through baml_sdk.baml (as the existing error/cancellation tests do). There is no packaged baml_core.errors module in that environment, so pytest fails during collection with ModuleNotFoundError before any of the new interface tests can run.
Useful? React with 👍 / 👎.
Adds a `ns_interfaces` fixture: an interface with in-body and out-of-body `implements` forms, functions that return and receive interface-typed values, and round trips in bare, list, optional, and class-field positions. Host tests pin the current per-SDK behavior, which diverges: - python: fully working — interface returns and params carry the concrete class across the boundary and the engine dispatches interface methods on host-constructed instances. - nodejs: returns work, but inbound encode is driven by the declared codegen type (`unknown` for interface positions), so class identity is lost — interface-method dispatch panics and round-tripped values come back as plain objects. Both SDKs emit host bindings for interface-impl methods that the engine registers under a synthetic `Iface$for$Class` name, so calling them panics with "Function not found" (also pinned). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Binary size checks passed✅ 7 passed
Generated by |
e94c57d to
725e5b7
Compare
Adds SDK e2e coverage for what happens when BAML functions receive and return interface-typed values.
Changes
sdk_tests/fixtures/type_shapes/baml_src/ns_interfaces/types.baml: aShapeinterface with three implementors (Square/Rectvia in-bodyimplements,Circlevia out-of-bodyimplements Shape for), functions returningShape, functions receivingShape(with engine-sidearea()dispatch), and round trips in bare, list, optional, and class-field (ShapeBox) positions.roundtrip_tests/test_interfaces.py, 17 tests) and TypeScript tests (roundtrip_interfaces.test.ts, 16 tests), each pinning that SDK's current behavior.Behavior findings (pinned by these tests)
Interface positions codegen as
typing.Any/unknown(client_codegen.rs:TirTy::Interface(..) => cg::Ty::BuiltinUnknown); the values crossing the boundary are concrete class instances.implementsforms.unknown), so class identity is lost: interface-method dispatch panics withVM internal error: virtual call could not resolve interface method, and round-tripped interface positions come back as plain objects. Receiving interfaces is effectively unusable from the node SDK today.BamlPanic, not as a host-side TypeError.area/area_asyncbindings on implementing classes underuser.interfaces.Square.area, but the engine registers interface-impl methods under a syntheticShape$for$Squarename, so calling the binding panics withFunction not found.The KNOWN-GAP tests pin current behavior deliberately, so fixing any of these flips a test and forces an intentional pin update.
Test plan
cargo nextest run -p sdk_test_python_pydantic2 -p sdk_test_typescript_node— 29/29 pass locally (pytest/ruff/pyright, vitest/tsc/esm_output/attw across all fixtures).🤖 Generated with Claude Code
Summary by CodeRabbit