Repository navigation
feat(stdlib): baml.ToString interface + string.from<T> driver - #3792
Merged
Merged
Conversation
Replace the implicit to_string path with an opt-in `baml.ToString`
interface and a `string.from<T>(value)` driver (strings only; json
deferred).
- `interface ToString` (new conversions.baml) with a default `to_string`
body, so `implements baml.ToString {}` with no override renders the
same structural default as a non-implementor.
- `string.from(x)` renders any value: a user `implements baml.ToString`
override is resolved on the value's runtime class (`_to_string_shim` /
`make_to_string_callee`); everything else gets a structural,
human-readable default (`_to_string_default`).
A static `match (value) { baml.ToString => value.to_string() }` cannot be
used in the driver: an interface match in the stdlib `baml` package can't
see downstream user implementors, so dispatch resolves on the runtime
class instead — mirroring `baml.json.to_json` and `Sortable.sort`'s
`_compare_shim`.
Tests: crates/baml_tests/tests/to_string_interface.rs. Stdlib snapshots
updated for the added functions.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (9)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Changesstring.from and baml.ToString
Python SDK keyword escaping
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_tests/tests/to_string_interface.rs (1)
24-153: ⚡ Quick winAdd focused VM unit tests alongside these integration tests.
Great e2e coverage; please also add crate-level unit tests for
make_to_string_calleeandto_string_default_recursiveso failures localize faster and align with the repo’s testing preference.
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_tests/tests/to_string_interface.rs` around lines 24 - 153, Add unit tests at the crate level (in the module where the functions are defined) for the two core functions make_to_string_callee and to_string_default_recursive. These unit tests should test the functions in isolation rather than through the integration test approach shown in this file, allowing failures to localize faster and aligning with the repository's testing preference for unit tests over integration tests.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_tests/tests/to_string_interface.rs`:
- Around line 24-153: Add unit tests at the crate level (in the module where the
functions are defined) for the two core functions make_to_string_callee and
to_string_default_recursive. These unit tests should test the functions in
isolation rather than through the integration test approach shown in this file,
allowing failures to localize faster and aligning with the repository's testing
preference for unit tests over integration tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 345eb03d-adb7-44e2-b433-621f2b69ff83
⛔ Files ignored due to path filters (7)
baml_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_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/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
baml_language/crates/baml_builtins2/baml_std/baml/conversions.bamlbaml_language/crates/baml_builtins2/baml_std/baml/string.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_tests/tests/to_string_interface.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/root.rs
…rface checks
Explain why `string.from` resolves to_string on the runtime class via
`_to_string_shim` instead of the natural `if value is baml.ToString {
value.to_string() } else { ... }`: `is <interface>` and interface
dispatch are resolved at compile time from a per-package implementor set
(dependencies + self, never dependents), so the `baml` stdlib package
can't see a downstream user `implements baml.ToString` and the test folds
to constant false.
TODO marks both sites to drop the shim and use the literal `is` form once
the TIR rework moves interface conformance checks to runtime.
… to_string
Move every builtin `to_string` onto the `baml.ToString` interface and
forbid declaring `to_string` directly on a class — it is provided by the
interface, not as a magic method.
Stdlib migration (TypeValue, Uint8Array, StackTrace, PlainTime, Instant,
PlainDate, PlainDateTime, ZonedDateTime):
- The native/formatter moves to a private `_to_string_impl`; the public
`to_string` is an `implements baml.ToString { ... }` block delegating to
it.
- Infallible formatters delegate directly. The fallible time formatters
(`Instant`/`PlainDate`/`PlainDateTime`/`ZonedDateTime`, which throw
`InvalidArgument` for out-of-RFC-3339-range years etc.) keep throwing
from `_to_string_impl`; the `throws never` interface method catches and
`panic`s instead. `to_json` is left untouched — it still calls the
throwing `_to_string_impl` and surfaces a `JsonSerializationError`.
Ban: a `to_string` method declared directly in a class body is now an
error (E0139) pointing the user at `implements baml.ToString { ... }`.
Methods inside `implements` blocks (in `c.implements`, not `c.methods`)
are unaffected.
Tests in to_string_interface.rs cover builtin dispatch via the interface
and both sides of the ban. Stdlib/bytecode snapshots updated.
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 `@baml_language/crates/baml_tests/tests/to_string_interface.rs`:
- Around line 49-67: The assertion in the to_string_via_interface_is_allowed
test function only verifies that no errors contain "to_string", which allows
unrelated compiler errors to slip through. Replace the current assertion that
checks !errors.iter().any(|e| e.contains("to_string")) with a simpler and
stricter check using errors.is_empty() to ensure that the compilation produces
no errors at all, not just no to_string-related errors.
🪄 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: c9eebd8b-fe66-404f-910e-c08bafefa7a0
⛔ Files ignored due to path filters (7)
baml_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_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/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snap
📒 Files selected for processing (17)
baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/stack_trace.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/instant.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/plaindate.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/plaindatetime.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/plaintime.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/zoneddatetime.bamlbaml_language/crates/baml_builtins2/baml_std/baml/type_class.bamlbaml_language/crates/baml_builtins2/baml_std/baml/uint8array.bamlbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/diagnostic.rsbaml_language/crates/baml_compiler_diagnostics/src/diagnostic.rsbaml_language/crates/baml_tests/tests/shell.rsbaml_language/crates/baml_tests/tests/to_string_interface.rsbaml_language/crates/bex_vm/src/package_baml/stack_trace.rsbaml_language/crates/bex_vm/src/package_baml/time.rsbaml_language/crates/bex_vm/src/package_baml/type_class.rsbaml_language/crates/bex_vm/src/package_baml/uint8array.rs
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/baml_tests/tests/shell.rs
# Conflicts: # baml_language/crates/baml_compiler_diagnostics/src/diagnostic.rs # baml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snap # baml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snap
⏭️ 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):
|
This comment has been minimized.
This comment has been minimized.
|
No description provided. |
…cribe snapshots - sdkgen_python_pydantic2: `from` is a Python hard keyword, so `string.from` generated `def from(...)` — a syntax error that broke ruff/pyright/pytest. Escape Python keywords on the rendered identifier (`from` -> `from_`), generalizing the existing `assert` -> `assert_` package rule. The BAML dispatch FQN is built from the raw name, so calls still target `from`. TypeScript is unaffected (`from` is legal there). - root.rs: the `_to_string_shim` doc linked the `pub(super)` `make_to_string_callee` via an intra-doc link, which `-D rustdoc::private-intra-doc-links` rejects; demote to a plain code span. - Refresh the `describe` snapshots (baml_cli + baml_lsp2_actions): the builtin `string` listing shifted with the added `from` method / `ToString` impl and the merged `Equals`/`Compare` impls.
…spatch The per-namespace bytecode snapshots embed builtin call FQNs, which shifted from `baml.X.to_string` to `baml.X.baml.ToString.to_string` when to_string moved onto the interface (plus the merged ops impls). Snapshot-only.
codeshaunted
added a commit
that referenced
this pull request
Jun 18, 2026
…olation Canary #3792 replaced magic/implicit `to_string` with the opt-in `baml.ToString` interface + a `string.from<T>(value)` universal renderer, and added an HIR rule forbidding a direct `to_string` method on a class. This branch's string interpolation was built on the now-removed magic to_string, so the merge adopts canary's design: - Untagged `${expr}` now elaborates to `string.from(expr)` (the universal renderer) instead of `expr.to_string()`. Removed the magic to_string auto-derive and the obsolete `to_string`-stringability TIR check (the null guard stays). `string.from` is total, so every value interpolates without the type needing a `to_string` method. - Took canary's primitive companions (Int/Float/Bool/String no longer declare a direct `to_string`) and removed the now-orphaned Rust `fn to_string` impls. - Removed the dead MIR primitive-union `to_string` dispatch (interpolation no longer emits `union.to_string()`); kept the union member-resolution arm, which is load-bearing for BEP-044 union field access. - Updated tests for the new model: a user class customises rendering via `implements baml.ToString`; a class without it renders structurally (`Point { x: 1, y: 2 }`); the tagged-template counter uses `string.from(i)`. - Regenerated snapshots (the per-class auto-derived `to_string` is gone from the rendered IR). Full suite green: 6275 passed, 0 snapshot drift.
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.
What
Replaces the implicit
to_stringpath with an opt-inbaml.ToStringinterface and astring.from<T>(value)driver. Strings only — the JSON side (baml.ToJson/json.from) is deferred.implements baml.ToString { function to_string ... }to control its rendering.implements baml.ToString {}(no override) renders the same structural default as a non-implementor.string.from(x)renders any value: primitives naturally (42,true), containers/instances structurally ([1, 2],Point { x: 1, y: 2 }), nested strings quoted.Why dispatch is a runtime shim, not a static
matchThe natural form
match (value) { baml.ToString => value.to_string(), _ => default }can't work inside a stdlib driver: an interface match in thebamlpackage lowers against the per-package implementor table, andbamlis a dependency of user code — so it can never see a downstreamimplements baml.ToString. The arm would compile to an unconditionalgoto failureand never fire for user types (the VMIsTypeopcode doesn't consult the global implementor table).So
string.fromresolvesto_stringon the value's runtime class via a native shim (_to_string_shim→make_to_string_callee, looking up{class_fqn}.baml.ToString.to_stringin VM globals), falling back to the structural default. This mirrors the existingbaml.json.to_jsonnative andSortable.sort's_compare_shim.Changes
baml_builtins2/baml_std/baml/conversions.baml(new):ToStringinterface +_to_string_default/_to_string_shimnatives; registered inlib.rs.string.baml:string.from<T>static method onString.bex_vm/.../root.rs: structural Display walker + the two natives.bex_vm/.../mod.rs:make_to_string_callee(runtime-class dispatch).Tests
crates/baml_tests/tests/to_string_interface.rs(7 tests): primitives, containers, instances, user override dispatch, default-body == non-implementor, direct method calls. Fullbaml_testssuite green; stdlib + bytecode snapshots updated for the added functions.Follow-ups
baml.ToJson+json.from(replacingto_json<T>).match (value) { baml.ToString => ... }driver form would need a global runtime interface is-test (a new VM capability overinterface_implementors).Summary by CodeRabbit
baml.ToStringinterface with runtime dispatch for human-readable rendering, plusString.from<T>(value)with structural fallback.Uint8Array,TypeValue) to implementbaml.ToString.to_string/to_jsonerror handling for time types to prevent unexpected panics.to_stringis defined outside animplements baml.ToStringblock.Instant).