fix(compiler): stop the backtick dedent from eating authored escapes - #4365
Conversation
A trailing `\n` in a backtick string literal was silently dropped, so
`` `${id.hostname}\n` `` produced a file with no trailing newline and no
error to say so.
Two causes, both in how §12 dedent handled layout vs content:
* Escapes were decoded *before* the dedent ran, so an authored `\n` had
already become a real newline by the time layout was stripped. The
dedent could not tell it apart from a line break the author typed to
lay the literal out.
* The dedent finished with a blanket `.trim()`, which removed whatever
whitespace it found at either end, including that newline, blank lines
the author left in, and trailing spaces.
Replace `preprocess_template` on the backtick path with `dedent_backtick`,
which strips layout and only layout: normalize line endings, drop the line
break after the opening delimiter and the line break plus indent before
the closing one, then strip the common leading-whitespace prefix. No trim.
Run it on the raw source text, before escape decoding, so `\n` stays two
opaque characters while layout is being removed; §13 block-tag whitespace
moves ahead of decoding for the same reason.
Normal multi-line literals dedent to exactly what they did before, so
there is no snapshot churn. What changes:
`${host}\n` -> "alpha\n" (was "alpha")
` a\n b\t ` -> " a\n b\t " (spaces no longer mangled)
`\n a\n b\n\n ` -> "a\nb\n" blank line before the closer
is now a trailing newline
`\n a \n b\n` -> "a \nb" trailing spaces are content
`preprocess_template` stays as-is for the legacy Jinja prompt path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cvpfs1fyG1RbDpTV3Ye28A
|
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 Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change replaces ChangesBacktick dedentation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BacktickStringLiteral
participant dedent_backtick
participant EscapeDecoder
participant FormatterAndRuntimeTests
BacktickStringLiteral->>dedent_backtick: raw literal with encoded escapes
dedent_backtick->>BacktickStringLiteral: normalized and dedented literal
BacktickStringLiteral->>EscapeDecoder: text after layout handling
EscapeDecoder-->>BacktickStringLiteral: decoded text segments
FormatterAndRuntimeTests->>BacktickStringLiteral: escaped-content regression cases
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
Binary size checks passed✅ 7 passed
Generated by |
Two stdlib prompt sections in `claude_code/ns_internal/cli.baml` open with
an explicit `\n\n` escape:
`\n\nThe output contract is one object with an outcome field. ...`
`\n\nConversation so far:\n${lines.join("\n")}`
They are concatenated with no separator between them —
`${instructions}${transcript_text(...)}${tool_protocol(...)}` — so that
leading `\n\n` *is* the blank line between sections. The old `.trim()`
was eating it, running the sections together in the rendered prompt.
This is the same bug as the trailing case, at the other edge.
The snapshots now show what the source asks for. No other constant moved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cvpfs1fyG1RbDpTV3Ye28A
Resolves one conflict in `baml_base/src/dedent.rs`, in the module doc. The Jinja removal (#4367) took `sys_llm::preprocess_template` with it, and this branch had already moved backtick literals onto `dedent_backtick`, so `baml_base::dedent::preprocess_template` was left with no callers. Delete it: it implements exactly the blanket-trim semantics this branch is fixing, and leaving it public next to its replacement invites someone to reach for the wrong one. Its tests move onto `dedent_backtick`. Two of them now record the new behavior instead of the old, which is the point of the change: " hello\n world" -> " hello\nworld" (was "hello\nworld"; the common prefix is the shorter indent and no trim follows it) "\t- foo\n - bar" -> unchanged (was "- foo\n - bar"; tabs and spaces share no prefix, so both lines keep their own indent) The NBSP and U+2028 char-boundary regression tests carry over unchanged — `dedent_backtick` uses the same `leading_whitespace_bytes` and `strip_leading_indent` helpers they were written to guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cvpfs1fyG1RbDpTV3Ye28A
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_base/src/dedent.rs`:
- Around line 189-203: Update the tests nbsp_indent_does_not_panic and
line_separator_indent_does_not_panic to assert that dedent_backtick returns the
exact original input, including the NBSP and U+2028 indentation characters,
rather than only checking that the calls do not panic.
🪄 Autofix
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 Plus
Run ID: 3e7a2309-5446-4985-a1d0-7e100ca4e63e
⛔ Files ignored due to path filters (3)
baml_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!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_textual.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
baml_language/crates/baml_base/src/dedent.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_tests/tests/backtick_fmt_value_preservation.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- baml_language/crates/baml_tests/tests/backtick_fmt_value_preservation.rs
- baml_language/crates/baml_compiler_syntax/src/ast.rs
The two Unicode-indent tests were written for a crash (a strip column computed in bytes landing mid-character), so they only called `dedent_backtick` and discarded the result. That checks the floor. It says nothing about whether the NBSP or the U+2028 survives, which is the property this branch is actually about. Assert the exact output. Both inputs come back byte-for-byte: NBSP and space are different characters, so by Rule 2 they share no common prefix, the strip column is zero, and each line keeps the indent its author gave it. Renamed to match what they now check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cvpfs1fyG1RbDpTV3Ye28A
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_compiler_parser/src/parser.rs`:
- Around line 11816-11817: Update the comment in the dedent assertion test to
match the asserted behavior: state that the delimiter-only line’s newline and
indentation are removed, while the resulting text ends with `!\nWelcome.`. Do
not describe that delimiter-related whitespace as belonging to the text.
🪄 Autofix
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 Plus
Run ID: 8399eb7a-5bf9-4315-b019-f86e6071b90c
📒 Files selected for processing (2)
baml_language/crates/baml_base/src/dedent.rsbaml_language/crates/baml_compiler_parser/src/parser.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/baml_base/src/dedent.rs
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: hellovai <vaibhavtheory@gmail.com>
Fixes B-1474.
The bug
A trailing
\nin a backtick string literal was silently dropped:`${id.hostname}\n` // -> "myhost", no newline, no errorThis broke generating newline-terminated files (
/etc/hostname,/etc/locale.conf). It was invisible in test logs, since a value that differs only by a trailing newline prints identically.Why
Two things in the §12 dedent conflated layout with content.
\nhad already become a real newline, indistinguishable from a line break the author typed to lay the literal out across lines. This also meant a literal written on a single source line could be sent down the multi-line path, because the "is this multi-line?" test ran on the decoded text..trim(), which removed whatever whitespace sat at either end. That newline, blank lines the author left in, and trailing spaces all went.The fix
baml_base::dedent::dedent_backtickreplacespreprocess_templateon the backtick path. It strips layout and only layout:\r\n/ lone\rto\n.No trim. It runs on the raw source text, before escape decoding, so
\nis still two opaque characters while layout is being removed. §13 block-tag whitespace moves ahead of decoding for the same reason: its line scan would otherwise treat an authored\nas a line boundary.preprocess_templateis deleted. #4367 removed the Jinja runtime and with itsys_llm::preprocess_template, its only other caller, so once backticks moved ontodedent_backtickit had none left. Leaving a public function that implements exactly the trimming semantics this PR removes, sitting next to its replacement, is a trap. Its tests move ontodedent_backtick; two of them now record the new behavior:Both differences are the absent trim. The NBSP and U+2028 char-boundary regression tests carry over unchanged —
dedent_backtickuses the same helpers they were written to guard — and now assert byte-exact preservation rather than only that nothing panics.What changes
Multi-line literals laid out the ordinary way dedent to exactly what they did before, so the formatter is unchanged and the
compiles/anddiagnostic_errors/snapshots do not move. The differences:\nstill decodes to a newline inside backticks. Making it literal would leave\`and\$decoding while\ndid not, and#"..."#is already the fully-raw form.One behavior change in the stdlib
The three
bytecode_formatsnapshots move, and it is the same bug at the other edge. Two prompt sections inclaude_code/ns_internal/cli.bamlopen with an explicit\n\n:`\n\nThe output contract is one object with an outcome field. ...` `\n\nConversation so far:\n${lines.join("\n")}`They are concatenated with nothing between them —
${instructions}${transcript_text(...)}${tool_protocol(...)}— so that leading\n\nis the blank line separating sections, and.trim()was eating it. Rendered prompts now have the separator the source asks for. No other bytecode constant moved; I diffed all three snapshots in full.The second one is worth a look:
`\n\nConversation so far:\n${...}`is a single source line and should never have been dedented at all. The old code decided that from the already-decoded text, where the escapes had become real newlines.Tests
baml_base: 17dedent_backtickcases — 9 new (trailing escape, blank-line-before-closer, trailing spaces, escapes-are-not-indentation, CRLF) plus the 8 inherited frompreprocess_template.bex_engine: 5 new end-to-end cases running the real compiled program, including the reported`${host}\n`repro on both single-line and dedented literals.baml_tests: a trailing-escape case added to the formatter value-preservation suite; threebytecode_formatsnapshots updated as above.baml_compiler_parser(154),baml_compiler_syntax(74),baml_compiler2_ast,baml_fmt(124),bex_enginebacktick (84) andllm_render(8), andbaml_tests --libat 1503.cargo fmtandcargo clippy --workspace --all-targetsclean.#4367 migrated fixtures from
#"..."#to backtick strings, straight into this code path; none of them had edge blank lines the old trim was absorbing.Local pre-commit hooks could not run (
cargo-shearrequires rustc 1.95, this toolchain is 1.93), so fmt and clippy were run directly. CI's Pre-commit Checks job passes.🤖 Generated with Claude Code
https://claude.ai/code/session_01Cvpfs1fyG1RbDpTV3Ye28A