Repository navigation
Skip the generic to_string path for string and primitive interpolation holes - #5432
Merged
antoniosarosi merged 2 commits intoOct 27, 2026
Conversation
…n holes A template hole lowers to baml.String.from(value): a generic call, a virtual call that never caches (the answer is structural), an override probe and a copy, even when the value is already a string. No package can implement baml.ToString for a primitive (E0139), so for a primitive the answer is fixed. MIR lowering now rewrites string.from(x) by x's checked type: a string (or string literal, or a union of them) is used as is, and an int, float, bool or null calls the new non-generic native baml._to_string_primitive. At runtime, _to_string_default returns a top-level string itself and formats int/float/bool/null without the walker, which covers generic T, unknown and x.to_string(); the walker no longer probes nested strings and floats for overrides. The text is unchanged: the walker and the fast paths share one leaf formatter.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Primitive formatting contract and VM implementation baml_language/crates/baml_builtins2/baml_std/baml/conversions.baml, baml_language/crates/bex_vm/src/package_baml/root.rs |
The built-in declaration adds _to_string_primitive for int, float, bool, and null. The VM adds shared primitive formatting, returns top-level strings unchanged in the override-aware renderer, and excludes strings and floats from override collection. |
Primitive conversion lowering baml_language/crates/baml_compiler2_mir/src/lower.rs |
The compiler classifies primitive renderings. For a single argument with a consistent primitive rendering, it assigns strings directly or calls _to_string_primitive; other cases use the existing lowering path. |
Rendering correctness coverage baml_language/crates/baml_tests/baml_src/ns_primitive_rendering/primitive_rendering.baml, baml_language/crates/baml_tests/src/compiler2_mir/mod.rs, baml_language/crates/baml_tests/tests/optimizer_binary_prefix.rs, baml_language/crates/baml_tests/tests/rust_backend.rs |
Tests cover primitive rendering across conversion paths, template holes, structural values, and mixed unions. Optimizer instruction-count expectations and backend fixture output also change. |
Template speed-test workloads baml_language/tools/speedtest/workloads/string/template-request-line-10k/*, baml_language/tools/speedtest/workloads/string/template-two-strings-10k/* |
Two deterministic workloads generate 10,000 template strings in BAML, JavaScript, and Python. Benchmark metadata defines expected totals, job counts, and warmup values. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Refactor
Sequence Diagram(s)
sequenceDiagram
participant Lowerer
participant MIR
participant PackageBamlImpl
Lowerer->>MIR: Emit _to_string_primitive for eligible values
MIR->>PackageBamlImpl: Call _to_string_primitive
PackageBamlImpl-->>MIR: Return formatted string
Possibly related PRs
- BoundaryML/baml#5433: Both PRs change primitive rendering behavior in
PackageBamland share leaf formatting used by generic string conversion.
Merge Risk: 
0a910
-
The primitive-rendering changes appear mergeable after normal checks. No concrete compatibility or runtime issue remains identified.
Architecture risk: 
0a910
-
The change affects 1 system.
Changed systems:
baml_languageArchitecture concerns
No architecture-level concerns identified.Review details
-
Systems and components
- observed — baml_language (service) was modified; 15 changed files map to changed impact.
Before / after behavior
- observed — Modified behavior in baml_language/crates/baml_builtins2/baml_std/baml/conversions.baml: Added documentation and the VM-backed
_to_string_primitivefunction acceptingint | float | bool | nulland returning a non-throwing string. - observed — Modified behavior in baml_language/crates/baml_compiler2_mir/src/lower.rs: Adds
PrimitiveRenderingandprimitive_rendering, which classify strings asItself, numeric/boolean/null primitives asFormatted, and returnNonefor non-primitives or unions whose members do not all share the same rendering. - observed — Modified behavior in baml_language/crates/baml_compiler2_mir/src/lower.rs: Adds a
string.fromlowering fast path when there is exactly one argument, one lowered operand, and its checked type has a consistent primitive rendering. String values are assigned directly; formatted primitives callbaml._to_string_primitive, using a temporary for non-local destinations. Other calls continue through the existing lowering path. - observed — Modified behavior in baml_language/crates/baml_tests/baml_src/ns_primitive_rendering/primitive_rendering.baml: Added generic and unknown conversion helpers that return
string.from(value).
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: bypassing the generic to_stringpath for string and primitive interpolation holes.Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 12 functions across 8 files. (7 skipped: 6 unsupported, 1 too large.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.
A rabbit tests each number’s glow,
Then checks how strings and nulls will show.
Through template holes the values run,
With floats and booleans joining in the fun.
Two workloads hop through lines of text,
And leave their measured totals next.
Comment @coderabbitai help to get the list of available commands.
⏭️ 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):
|
Contributor
Author
|
@coderabbitai review |
String holes are now used as is, so the backtick cases in optimizer_binary_prefix fold to constants; a class hole keeps a call result in a + prefix there. The rust_backend fixture's main gets a class hole so it still reaches a stdlib BAML function.
aaronvg
approved these changes
Oct 27, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
|
Binary size checks failed❌ 2 violations · ✅ 1 passed
Details & how to fixViolations:
Add/update baselines:
[artifacts.bridge_wasm]
file_bytes = 27534291
gzip_bytes = 8086975
[artifacts.packed-program]
file_bytes = 41348440
gzip_bytes = 16855562Generated by |
meefs
pushed a commit
to meefs/baml
that referenced
this pull request
Oct 27, 2026
`join` was written in BAML. A rest pattern copied the array, then every element went through a generic `string.from` call and a `+` that built a rope node. It is now a native that appends into one buffer. A string element is appended as is. Any other element is rendered exactly as `string.from` renders it, through the override-honoring walker, one element at a time. So `ToString` overrides still apply at every depth and run in element order, as in the BAML loop. The separator semantics are unchanged: an empty array joins to `""`, one element to its own rendering. New tests pin those cases plus override classes, structural classes, a structural class holding an override, enums, maps, nested lists, and a garbage collection inside an override. The 12-hole `join` below joins the speedtest suite as `string::join-request-line-10k`. This is the top of the stack (BoundaryML#5431 → BoundaryML#5432 → this PR), so the table also compares with canary before any of the three. | workload | canary | PR 2 | this PR | vs PR 2 | vs canary | |---|---:|---:|---:|---:|---:| | 12-hole request line, `join` | 513.14 | 432.20 | 22.87 | 18.9× faster | 22.4× faster | | `[a, "/", b].join("")` | 80.55 | 70.48 | 7.70 | 9.2× faster | 10.5× faster | | 12-hole request line, template | 117.77 | 30.27 | 31.02 | +2% | 3.8× faster | | `` `${a}/${b}` `` | 20.06 | 4.17 | 4.33 | +4% | 4.6× faster | | `interfaces::to-string-interpolation-10k` | 51.39 | 32.89 | 33.34 | +1% | −35% | | repro `` `${a}${b}` `` (ns/op) | 1,563 | 100 | 98 | −1% | 15.9× faster | | repro `` `/v1/items/${a}/tags/${b}` `` (ns/op) | 1,703 | 104 | 104 | −1% | 16.5× faster | | repro `` `n=${k}` `` (ns/op) | 764 | 244 | 230 | −6% | 3.3× faster | | repro `a.to_string() + b.to_string()` (ns/op) | 826 | 454 | 432 | −5% | −48% | | repro loop `` out = `${out}${c}` `` (ns/op) | 2,508 | 540 | 556 | +3% | 4.5× faster | | repro `a + b` (ns/op, control) | 114 | 114 | 113 | −1% | −1% | | loop, no string built (control) | 2.43 | 2.46 | 2.64 | +7% | +9% | | `a + "/" + b` (control) | 4.07 | 4.09 | 4.37 | +7% | +7% | | `programs::richards-50` (control) | 510.13 | 496.96 | 515.00 | +4% | +1% | | `compute::generic-apply-inferred-1m` (control) | 527.90 | 492.42 | 530.13 | +8% | 0% | | `compute::fib32-recursive` (control) | 744.02 | 739.12 | 740.95 | 0% | 0% | | `compute::pure-call-1m` (control) | 115.37 | 115.58 | 116.12 | 0% | +1% | | `string::trim-padded-literal-100k` (control) | 33.89 | 33.93 | 35.77 | +5% | +6% | The controls that move run no `join` and no template. Against PR 2 they execute the same number of instructions (`perf stat`: `richards-50` 18.3266 vs 18.3265 billion, `generic-apply` 18.43 vs 18.43 billion). Only front-end stall cycles grow (`richards-50` 415M → 685M). That is code layout in the fat-LTO binary, not extra work. The same controls also swing ±3% between canary, PR 1 and PR 2, which don't touch them either. ms per job (median of 5) unless marked ns/op (median of 10). AMD Ryzen 9 5950X, Linux 7.0, release builds (fat LTO, rustc 1.98.0), `speedtest` VM row, telemetry off, pinned to CPU 7, canary and the three stacked PRs interleaved in 5 rounds. Rows without a `::` name are temporary workloads that build one string per iteration, 10,000 iterations per job. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **New Features** * Array joining now honors custom string representations for values, including nested values, while preserving element order and separators. * Joining supports empty and single-element arrays, primitive values, enums, classes, unions, and generic calls. * **Bug Fixes** * Improved consistency when custom string representations run during garbage collection. * **Tests** * Expanded coverage for joining behavior and added a performance workload for large batches of request-line strings. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This branch was successfully 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.
Every
${x}in a template lowers tobaml.String.from(x). That is a generic call. Its body makes a virtualto_stringcall that resolves to the structural default, which the call-site cache never stores. The default then probes for overrides, walks the value and copies it, even whenxis already a string.No package can implement
baml.ToStringfor a primitive (E0139), so for a primitive the answer never changes. This PR uses that twice:string.from(x)by the checked type ofx. A string, a string literal or a union of them is used as is. Anint,float,boolornullcalls one non-generic native,baml._to_string_primitive. Any other type keepsstring.from, so generic, class, enum, container and mixed-union holes behave exactly as before._to_string_defaultreturns a top-level string as itself and formatsint,float,boolandnullwithout the walker. This covers what the compiler cannot see: a genericT,unknown, andx.to_string(). The walker also no longer probes nested strings and floats for overrides.The text stays byte-identical: the walker and both fast paths share one leaf formatter. New tests pin the output for every route (template hole,
string.from,.to_string(), generic,unknown, nested), covering int edge cases, floats (integral,-0.0,NaN, infinities, large and small magnitudes), bools, null and unicode strings. Two of the workloads below join the speedtest suite asstring::template-two-strings-10kandstring::template-request-line-10k.This is Notion TASK-18; the repro is https://github.com/BoundaryML/baml-interpolation-perf-repro. Its
`${a}${b}`now costs the same asa + b.`${a}${b}``/v1/items/${a}/tags/${b}``n=${k}`"n=" + string.from(k)a.to_string() + b.to_string()out = `${out}${c}`a + b(control)`${a}/${b}`, strings`id=${k}`, int`v=${f}`, float`ok=${b}`, bools = `${s}x`loop"id=" + k.to_string()interfaces::to-string-interpolation-10k`m=${m}`,ToStringoverride`p=${p}`, structural class`m=${mp}`, map[a, "/", b].join("")a + "/" + b(control)+(control)programs::richards-50(control)compute::fib32-recursive(control)compute::pure-call-1m(control)string::concat-loop-10k(control)Repro rows are ns per op (median of 10 runs); the other rows are ms per job (median of 5). AMD Ryzen 9 5950X, Linux 7.0, release builds (fat LTO, rustc 1.98.0),
speedtestVM row, telemetry off, pinned to CPU 7, canary and the three stacked PRs interleaved in 5 rounds. Rows without a::name are temporary workloads that build one string per iteration, 10,000 iterations per job.Summary by CodeRabbit
string.fromnow render integer, float, boolean, and null values directly as text. Strings remain unchanged, while complex values continue to use their existing rendering behavior.