Port baml wrapper to clap and unify formatting via baml_term (B-886) - #4069
Closed
codeshaunted wants to merge 4 commits into
Closed
Port baml wrapper to clap and unify formatting via baml_term (B-886)#4069codeshaunted wants to merge 4 commits into
codeshaunted wants to merge 4 commits into
Conversation
…rm (B-886) The baml wrapper hand-rolled arg parsing, help text, and error/warning output, so its rendering diverged from baml-cli in the same invocation stream. Extract the shared presentation pieces (CLAP_STYLING, ColorChoice and agent-aware init_color, the error:/warning: printers) into a new tiny baml_term crate that the wrapper, baml_exec, baml_cli, and the pack host all consume, and port the wrapper's toolchain/self-update commands to clap. The wrapper still dispatches on argv[1] so everything except its own commands passes through to baml-cli byte-for-byte; clap only parses wrapper-owned commands. Deliberate behavior changes: - wrapper usage errors follow clap conventions (styled error + usage, exit 2; bare 'baml toolchain' prints help, exit 2) - wrapper main errors render via print_anyhow_error (bold-red error: header + cause chain) instead of anyhow's one-line alternate format - the shared printers gate styling on stderr (for_stderr) instead of console's stdout default, matching where they write - baml-cli's direct-invocation warning uses the styled printer
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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 (2)
📝 WalkthroughWalkthroughThis change adds the shared ChangesTerminal output and wrapper CLI
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WrapperCli
participant baml_term
participant ToolchainCommand
User->>WrapperCli: Invoke toolchain or self-update
WrapperCli->>baml_term: Initialize color
WrapperCli->>ToolchainCommand: Parse Clap command
ToolchainCommand->>ToolchainCommand: Dispatch operation
ToolchainCommand->>baml_term: Print status or diagnostic
baml_term-->>User: Render terminal output
Possibly related PRs
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 |
⏭️ 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):
|
Binary size checks failed❌ 2 violations · ✅ 5 passed
Details & how to fixViolations:
Add/update baselines:
[artifacts.baml-cli]
file_bytes = 19528576
stripped_bytes = 19528624
gzip_bytes = 9319790
[artifacts.bridge_wasm]
file_bytes = 16160508
gzip_bytes = 4402886Generated by |
![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_term/src/lib.rs (1)
47-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd unit tests for the agent-detection helpers.
A small#[cfg(test)]module here can coverenv_truthyandrunning_in_agentwith a few env-var cases.🤖 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_term/src/lib.rs` around lines 47 - 54, Add a #[cfg(test)] module in lib.rs covering env_truthy with unset, empty, "0", and nonzero values, and covering running_in_agent when the configured AGENT_ENV_VARS variables are absent and when one is set. Ensure tests isolate and clean up environment variables to avoid affecting other 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_term/src/lib.rs`:
- Around line 47-54: Add a #[cfg(test)] module in lib.rs covering env_truthy
with unset, empty, "0", and nonzero values, and covering running_in_agent when
the configured AGENT_ENV_VARS variables are absent and when one is set. Ensure
tests isolate and clean up environment variables to avoid affecting other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 328f3fc3-2e62-4385-840f-ae5e1bdc9931
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
baml_language/Cargo.tomlbaml_language/crates/baml/Cargo.tomlbaml_language/crates/baml/src/main.rsbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/main.rsbaml_language/crates/baml_cli/src/paint.rsbaml_language/crates/baml_cli/src/reporter.rsbaml_language/crates/baml_exec/Cargo.tomlbaml_language/crates/baml_exec/src/clap_target.rsbaml_language/crates/baml_exec/src/diag_print.rsbaml_language/crates/baml_exec/src/lib.rsbaml_language/crates/baml_term/Cargo.tomlbaml_language/crates/baml_term/src/lib.rsbaml_language/stow.toml
💤 Files with no reviewable changes (1)
- baml_language/crates/baml_exec/src/diag_print.rs
…ctions after_help strings render verbatim, so the wrapper's 'Network behavior' / 'Wrapper updates' sections and baml-cli's toolchain pointer read as unformatted text next to clap's styled output. Add help_heading / help_literal to baml_term (rendered with CLAP_STYLING; clap prints help through anstream, which strips the escapes when color is off) and use them in both binaries.
…lines Move format_status into baml_term (verb style now gated on stderr, where Reporter prints, instead of console's stdout default) and use it for the wrapper's install/select/uninstall/self-update confirmations, which were plain lowercase sentences on stdout. They now render like baml-cli's Reporter lines (12-char right-aligned bold purple verb) and move to stderr, matching cargo's stream conventions. Informational listings (--version, toolchain status/list) stay plain, like cargo and rustup.
Contributor
|
Blacksmith runners detected OOM events on the following jobs:
|
This was referenced Jul 22, 2026
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.
Fixes B-886.
The
bamlwrapper hand-rolled its arg parsing, help text, and error/warning output, so its rendering diverged frombaml-cliwithin the same invocation stream: plain unstyled help vs clap's brand-purple help, unprefixed one-line anyhow errors vs the bold-rederror:+ cause chain, a copy-pasted warning prefix, and no--color/agent-aware color policy.What changed
baml_termcrate (deps:anyhow/clap/consoleonly) owning the shared presentation pieces:CLAP_STYLING,ColorChoice+ agent-awareinit_color, and theerror:/warning:printers. It exists because the wrapper can't depend onbaml_exec(pulls the engine) and the pack host can't depend onbaml_cli.baml_exec::diag_printdeleted;baml_execre-exports the printers so the pack host keeps itsbaml_exec-only footprint.baml_cli::paint/reporterre-export frombaml_termso internal call sites are unchanged.consoledropped frombaml_exec.toolchain/self-updateare now a clap derive tree styled withCLAP_STYLING, with a global--colorflag and--manifest-base-urlas a proper global arg. Top-level dispatch still peeks at argv[1], so everything except wrapper-owned commands passes through tobaml-clibyte-for-byte (a top-level clap parse would break flag forwarding).mainerrors route throughprint_anyhow_error; the freshness warning uses the sharedprint_warning.Deliberate behavior changes
baml toolchainprints help and exits 2 (was stdout, exit 0).for_stderr) instead of console's stdout default —diag_printwas gating on the wrong stream; the docs already claimed stderr behavior.baml-cli's direct-invocation warning now uses the styled printer instead of a rawwriteln!;Reporter::warningdeduped intoprint_warning.error:inside the no-toolchain message that the printer now doubles.Testing
cargo nextestonbaml(incl. freshness e2e — warning text unchanged),baml_term,baml_exec,baml_cli; clippy clean.toolchain/self-updatehelp, usage errors, typo suggestions, runtime error rendering,--version,--manifest-base-urlbefore/after subcommand, pass-through.Summary by CodeRabbit
New Features
self-update.Improvements
error:/warning:prefixes and improved chained-cause reporting.Tests