String interpolation: syntax highlighting + inlay-hint suppression - #3797
Merged
Conversation
…odes
`${expr}` lowers to `string.from(expr)` and `${ let … }` builds a concat-scope
accumulator (`let " __m3_concat" = ""`); both carry spans inside the backtick
template, so the inlay-hint provider emitted a `value:` parameter hint on every
interpolation plus a `: string` hint on the accumulator — noise the user never
wrote.
Skip them in `annotations`:
- a call whose span equals its callee's span is a synthesized wrapper (a real
call's span covers `callee(args…)`), so emit no parameter hints for it.
- a `let` with `LetOrigin::Compiler` is compiler-generated, so emit no type hint.
Real bindings and calls — including a user's explicit `${ let n = … }` — keep
their hints. Covered by a unit test and an `inlay_hints/backtick_interpolation`
golden file.
…mmar
Add TextMate highlighting for BEP-049 backtick strings to the BAML grammar:
- backtick-string: `...` / ``...`` / ```...``` (N-tick opener closed by N ticks
via a \1 backreference, so inner single backticks stay literal)
- backtick-escape: \` \$ \\ \n \r \t \0 \uXXXX (\${ stays literal text)
- interpolation: ${...} rendered as embedded code, with ${for}/${if}/${else}/
${endfor}/${endif} as control keywords; nested braces balance so object
literals and if/else blocks don't close the interp early
- tagged-template-tag: the tag in prompt`...`
Consolidate the grammar into a single source of truth, @b/pkg-grammar:
- canonical baml.tmLanguage.json + jinja.tmLanguage.json live there
- app-promptfiddle imports them directly (local copies removed)
- app-vscode-ext keeps a committed mirror under syntaxes/ (VS Code loads the
grammar from the physical path in contributes.grammars; the .vsix ships
bundled with no node_modules), regenerated by sync.mjs via predev/prebuild
- a grammar-single-source pre-commit hook fails if the mirror drifts
Verified by tokenizing all BEP-049 cases with vscode-textmate; declarative
LLM-function highlighting (client/prompt #"..."# + jinja) is unaffected.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces ChangesGrammar Single-Source Package
Compiler Synthetic Node Tracking
LSP Inlay Hint Suppression
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
⏭️ 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Replace the span-equality heuristic the inlay-hint provider used to detect
synthesized `string.from(${…})` interpolation wrappers ("a real call's span is
strictly larger than its callee's") with an explicit, universal marker.
`AstSourceMap` now carries `synthetic_exprs` / `synthetic_stmts` /
`synthetic_patterns` sets. Lowering populates them via a scoped `synthesizing`
flag on the `alloc_expr`/`alloc_stmt`/`alloc_pattern` chokepoints: backtick
elaboration flips it on, so every generated node (the `string.from` wrapper, the
concat accumulator, the `+` folds, …) is recorded, while the user-written
segments lowered beforehand keep their non-synthetic ids. Nested backtick
strings save/restore the flag.
The annotations walker now asks `source_map.is_synthetic_expr/stmt(...)` instead
of comparing spans (call hint) and instead of matching `LetOrigin::Compiler`
(let type hint) — one uniform concept for "the compiler generated this," no
reliance on an unenforced span invariant.
Pure side metadata: IR output (HIR/TIR/MIR/codegen) is unchanged. The 10
`AstSourceMap { … }` literals in lower_cst/auto_derive_json are converted to
`..Default::default()` so future fields don't have to touch every site.
Tests: 500 lsp2_actions (incl. the interpolation golden + unit test), 70 ast,
19 backtick fixtures, 98 json/derive — all green; clippy --all-targets clean.
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_compiler2_ast/src/ast.rs`:
- Around line 594-600: Update the doc comments for the methods
`is_synthetic_stmt` and `is_synthetic_pattern` to correctly reference the fields
they actually check. In the `is_synthetic_stmt` method, change the doc comment
reference from `synthetic_exprs` to `synthetic_stmts` to match the field being
checked. In the `is_synthetic_pattern` method, change the doc comment reference
from `synthetic_exprs` to `synthetic_patterns` to match the field being checked.
🪄 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: bef9cc15-3ea5-4600-802c-810e86bed8a8
📒 Files selected for processing (5)
baml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/auto_derive_json.rsbaml_language/crates/baml_compiler2_ast/src/lower_cst.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_lsp2_actions/src/annotations.rs
With the inlay-hint provider switched to the synthetic-node marker, nothing
distinguished `LetOrigin::Compiler` from `Source` anymore — every remaining
reader (type_info / completions / outline) lumped it into the `_` arm. Remove
the variant; the compiler-generated concat/loop accumulators now lower with
`LetOrigin::Source`, and "this binding is synthesized" is carried solely by
`AstSourceMap::synthetic_stmts`.
`LetOrigin` is now `{ Source, Client, RetryPolicy }`. The three readers match
all three explicitly (no wildcard), so adding a future variant is a compile
error rather than a silent fall-through.
No IR change (no snapshot prints the origin); workspace build clean, clippy
--all-targets clean, 570 lsp2_actions + ast tests and the backtick/json/derive
fixtures all green.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs (1)
3348-3351:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMark tagged-template body allocations synthetic too.
elaborate_tagged_bodycreates compiler-only__tt_parts/__tt_values/__tt_curlets and helper calls, but it is called withself.synthesizing == false, so the allocation helpers at Lines 490-509 never record these nodes inAstSourceMap.synthetic_*. After the newLetOrigin::Sourceassignments, LSP consumers that skip synthetic nodes can still see these tagged-template internals as source-authored.Suggested fix
fn elaborate_tagged_body(&mut self, segments: &[TemplateSegment], span: TextRange) -> ExprId { + let prev_synth = std::mem::replace(&mut self.synthesizing, true); let parts = Name::new(" __tt_parts"); let values = Name::new(" __tt_values"); let cur = Name::new(" __tt_cur"); @@ - self.alloc_expr( + let body = self.alloc_expr( Expr::Block { stmts, tail_expr: Some(tail), }, span, - ) + ); + self.synthesizing = prev_synth; + body }Also applies to: 3381-3382, 3406-3443, 3621-3653
🤖 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_compiler2_ast/src/lower_expr_body.rs` around lines 3348 - 3351, The calls to elaborate_tagged_body in the lower_expr_body.rs file (at the locations specified in the comment and additional lines mentioned) are creating compiler-internal variables like __tt_parts, __tt_values, and __tt_cur, but since self.synthesizing is false during these calls, the allocation helpers at lines 490-509 do not record these nodes as synthetic in AstSourceMap. To fix this, before each call to elaborate_tagged_body, save the current value of self.synthesizing, set self.synthesizing to true, call elaborate_tagged_body, and then restore the original self.synthesizing value. This ensures that all compiler-generated nodes created within elaborate_tagged_body are properly marked as synthetic so LSP consumers can correctly skip them.
🤖 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.
Outside diff comments:
In `@baml_language/crates/baml_compiler2_ast/src/lower_expr_body.rs`:
- Around line 3348-3351: The calls to elaborate_tagged_body in the
lower_expr_body.rs file (at the locations specified in the comment and
additional lines mentioned) are creating compiler-internal variables like
__tt_parts, __tt_values, and __tt_cur, but since self.synthesizing is false
during these calls, the allocation helpers at lines 490-509 do not record these
nodes as synthetic in AstSourceMap. To fix this, before each call to
elaborate_tagged_body, save the current value of self.synthesizing, set
self.synthesizing to true, call elaborate_tagged_body, and then restore the
original self.synthesizing value. This ensures that all compiler-generated nodes
created within elaborate_tagged_body are properly marked as synthetic so LSP
consumers can correctly skip them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f198b221-78e8-4a84-bc38-3b8daff977af
📒 Files selected for processing (6)
baml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_lsp2_actions/src/annotations.rsbaml_language/crates/baml_lsp2_actions/src/completions.rsbaml_language/crates/baml_lsp2_actions/src/outline.rsbaml_language/crates/baml_lsp2_actions/src/type_info.rs
💤 Files with no reviewable changes (1)
- baml_language/crates/baml_compiler2_ast/src/ast.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/baml_lsp2_actions/src/annotations.rs
is_synthetic_stmt/is_synthetic_pattern referenced synthetic_exprs; point them at the fields they actually check (per PR review).
The synthesizing flag was only wrapped around the untagged backtick path, so
`tag`...`` desugaring (both `lower_tagged_template_expr` and
`build_prompt_tag_closure`) allocated its `__tt_parts`/`__tt_values`/`__tt_cur`
accumulators and `.push(...)` calls with `synthesizing = false` — leaving them
unmarked in the source map. Wrap both `elaborate_tagged_body` call sites,
mirroring the untagged path; the user-written segments and `${…}` interps are
lowered beforehand and keep their non-synthetic ids.
No observable change today (those nodes happen to be skipped by the inlay
walker's empty-span / type-annotation guards), but it makes "synthetic =
compiler-generated" uniform across both template paths and robust to future
span changes — per PR review.
Tests:
- ast: `ast_tagged_template_body_marked_synthetic` asserts the elaborated body
block + its stmts are synthetic while the tag expr and user `${…}` interp are
not (verified it fails when the wrap is removed).
- lsp2: `annotations_skip_tagged_template_synthetic_hints` checks a custom tag in
an expression function yields only the user's `let q: string` hint.
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
Fixes two editor problems with BEP-049 backtick strings /
${...}interpolation.1. Syntax highlighting (the big one)
The BAML TextMate grammar had zero support for backtick strings, so everything inside
`...`was tokenized as arbitrary BAML code — hence the "very bad" highlighting. Added:backtick-string—`...`,```...```(an N-tick opener is closed by N consecutive ticks via a\1backreference, so inner single backticks likecodebacktick-escape—\`,\$,\\,\n,\r,\t,\0,\uXXXX(\${stays literal text, not an interpolation)interpolation—${...}rendered as embedded code;${for}/${if}/${else}/${endfor}/${endif}as control keywords; nested braces balance so object literals (${ {a: 1} }) and${if (p) {..} else {..}}don't close the interp earlytagged-template-tag— the tag inprompt`...`Verified by tokenizing all BEP-049 cases (interpolation, escapes, multi-tick ladders, multi-line
${for}/${endfor},${if}/${else}/${endif}, tagged templates, nested backtick-in-interpolation) withvscode-textmate. Regression-checked that normal"...",#"..."#,`a` == `b`, and the declarative LLM-function sample (client<llm>,prompt #"..."#+ jinja) are unaffected.2. Inlay hints inside
${...}${expr}lowers to a syntheticstring.from(expr)call and${ let … }builds a compiler-generated concat accumulator — both carried spans inside the template, so the inlay-hint provider emitted spuriousvalue:/: stringhints the user never wrote. Suppressed (commit5661b7c0): a call whose span equals its callee's span is a synthesized wrapper, and aletwithLetOrigin::Compileris compiler-generated. User-written${ let n = … }keeps its hint. Covered by a unit test + golden file; full LSP suite (500 tests) green.Single source of truth for the grammar
The grammar previously existed as duplicated copies across apps. Consolidated into a new workspace package
@b/pkg-grammar(typescript2/pkg-grammar/):baml.tmLanguage.json+jinja.tmLanguage.jsonlive theresyntaxes/because VS Code loads the grammar from the physical path incontributes.grammars[]and the.vsixships bundled (nonode_modules). The mirror is regenerated byscripts/sync.mjsviapredev/prebuild.grammar-single-sourcepre-commit hook fails if the mirror drifts from the packageTo edit the grammar going forward: change the files in
typescript2/pkg-grammar/, thennode typescript2/pkg-grammar/scripts/sync.mjs(or build vscode-ext) and commit the regenerated mirror.Verification
pkg-protoprotobufs + un-built@b/bridge_wasm, unrelated)grammar-single-sourcehook: passes; drift test confirms it catches a tampered mirrorSummary by CodeRabbit
${ ... }interpolation.