Repository navigation
perf(ci): faster cargo tests via dedup, opt-level, cache sharing + migrate exec tests to BAML corpus - #4075
Merged
Conversation
…aring The two linux cargo jobs dominated CI (~11 min each, 8-vcpu saturated). Root causes and fixes: - The linux `cargo test` job re-ran baml_tests/baml_cli/baml_lsp2_actions that the snapshot job already covers on the same OS. Exclude them at the cargo level (--workspace --exclude), not just via a nextest -E filter, so their heavy test binaries aren't even built. Drop the duplicated pack_e2e step (the snapshot job runs it) and document that contract on the snapshot job. Same build-skip on macos/windows. - Tests exercise the compiler thousands of times, so test *execution*, not compilation, is the cost. Build the compiler optimized under test: CARGO_PROFILE_DEV/TEST_OPT_LEVEL=1 in CI, and salsa (the query engine that runs on every compile) at opt-level 3 in the dev profile. - The spawned-CLI e2e suites (exit_code_e2e, pack_e2e) hit a cold `<project>/.baml/cache` per test and recompiled the stdlib every time (~14s/spawn). Point them at a shared BAML_CACHE_DIR under CARGO_TARGET_TMPDIR so only the first invocation pays the stdlib compile; the cache is content-addressed and fingerprinted on the CLI binary, so sharing is safe. - exit_code_e2e used common::ensure_built() (which shells `cargo build -p baml_pack_host`, ~10s/process even when fresh) despite never running `baml pack`; switch it to baml_cli(). - Bump both linux jobs 8 -> 16 vcpu (cost-neutral: they were CPU-saturated). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01J2XzpcdbF2ddFDrCgc7fuF
…corpus
Each Rust-side `baml_test!` test recompiled a BAML project from scratch in its
own nextest process (~0.7-2.5s), while a `test { }` block in the `baml_src/`
corpus compiles once with the whole project and runs for ~40ms. Port the
compile-and-run tests out of `crates/baml_tests/tests/*.rs` and into
`baml_src/ns_*/`, one block per former Rust test (name preserved).
- 370 tests migrated across 43 files; each new block validated with
`baml-cli test` and the full corpus (2819 tests) passes.
- What stays in Rust, and why: compile-diagnostics tests (setup_test_db +
collect_diagnostics assert on error codes/messages), host-FFI tests
(BexExternalValue args, VM introspection), host infrastructure (stub
HTTP/TCP servers, symlink creation), OptLevel::Two-specific dispatch, and
#[should_panic] compile-time checks. Each is documented at its call site.
- The corpus runner snapshots each namespace's bytecode; new/extended
namespaces add the corresponding baml_src/*.snap.
Follow-up (not in this commit): feature-gate `baml_test!` behind an
off-by-default `rust-baml-exec-tests` so new BAML behavior tests are written
in the corpus, not Rust.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01J2XzpcdbF2ddFDrCgc7fuF
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
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):
|
Binary size checks failed❌ 2 violations · ✅ 5 passed
Details & how to fixViolations:
Add/update baselines:
[artifacts.baml-cli]
file_bytes = 19512048
stripped_bytes = 19512096
gzip_bytes = 9320307
[artifacts.bridge_wasm]
file_bytes = 16160508
gzip_bytes = 4402886Generated by |
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
Contributor
📝 WalkthroughWalkthroughThis PR expands BAML runtime corpus coverage across language semantics, interfaces, associated types, serialization, networking, templates, and standard-library behavior. It also refactors CLI caching and CI test distribution, adjusts optimization profiles, and moves selected runtime checks between BAML and Rust harnesses. ChangesRuntime and test coverage
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
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_tests/tests/http.rs (1)
259-260: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRetain host-backed timeout-failure coverage.
The corpus migration removes deterministic timeout-expiry tests even though they require Rust-created network peers and therefore belong in the retained host-infrastructure suite.
baml_language/crates/baml_tests/tests/http.rs#L259-L260: restore the silent-server HTTP timeout test or equivalent host-backed coverage.baml_language/crates/baml_tests/tests/net.rs#L128-L154: retain a TCP read that actually expires and verifies the timeout error.baml_language/crates/baml_tests/tests/net.rs#L219-L248: retain a UDP receive that actually expires and verifies the timeout error.🤖 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/http.rs` around lines 259 - 260, Restore host-backed timeout-failure coverage: in baml_language/crates/baml_tests/tests/http.rs:259-260, add a silent-server HTTP timeout test or equivalent; in baml_language/crates/baml_tests/tests/net.rs:128-154, retain a TCP read that genuinely expires and asserts the timeout error; and in baml_language/crates/baml_tests/tests/net.rs:219-248, retain a UDP receive that genuinely expires and asserts the timeout error.
🧹 Nitpick comments (1)
baml_language/crates/baml_tests/tests/to_json_interface.rs (1)
6-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract duplicated
compile_errorsfunction to a shared helper.Both files define an identical
compile_errorstest helper function. Consider extracting this function into a common test utility module (e.g.,tests/common/mod.rsorbaml_tests::testing) to reduce code duplication and improve maintainability.
baml_language/crates/baml_tests/tests/to_json_interface.rs#L6-L15: remove this local definition and import the shared helper.baml_language/crates/baml_tests/tests/tostring_sugar.rs#L7-L16: remove this local definition and import the shared helper.🤖 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_json_interface.rs` around lines 6 - 15, Extract the duplicated compile_errors helper into a shared test utility, preserving its current diagnostic filtering and formatting behavior. In baml_language/crates/baml_tests/tests/to_json_interface.rs lines 6-15 and baml_language/crates/baml_tests/tests/tostring_sugar.rs lines 7-16, remove the local compile_errors definitions and import the shared helper instead.
🤖 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/baml_src/ns_array_rest_binding/array_rest_binding.baml`:
- Around line 230-237: Strengthen arb_rest_bind_chain_aliases_same_slice so the
test mutates one chained rest binding before comparing through the other,
proving r and s alias the same slice rather than merely sharing its initial
length. Update the expected assertion in rest_bind_chain_aliases_same_slice to
reflect the mutation while preserving the existing binding pattern.
In
`@baml_language/crates/baml_tests/baml_src/ns_cancel_cascade/cancel_cascade.baml`:
- Around line 15-17: Update the cancellation test around the waiter spawn so it
emits a deterministic readiness signal after entering the await path, then wait
for that signal before calling waiter.cancel(). Preserve the existing slow task
and await slow behavior, ensuring cancellation exercises the await-race path
rather than pre-start cancellation.
- Around line 21-24: Remove local bindings of catch-expression results across
the affected tests: in
baml_language/crates/baml_tests/baml_src/ns_cancel_cascade/cancel_cascade.baml:21-24,
set a cancellation flag inside the catch arm and assert that flag inline; in
baml_language/crates/baml_tests/baml_src/ns_csv/csv_smoke.baml:126-137, mutate
step and done directly within the match and catch branches; in
baml_language/crates/baml_tests/baml_src/ns_errorcontext/errorcontext.baml:139-150,
have the helper return the catch expression and bind only the normal helper-call
result. Keep catch outcomes out of local bindings while preserving each test’s
existing assertions.
In
`@baml_language/crates/baml_tests/baml_src/ns_comparable_sort/comparable_sort.baml`:
- Around line 247-249: Update the explicit-binding test path by adding a helper
that assigns the result of high.compare(low) to a local variable and returns
that variable, then have phase2_user_class_compare_with_explicit_binding_in_main
assert through that helper instead of calling phase2_compare_main directly.
Preserve the expected result of 1.
- Around line 470-480: Update the float parity inputs in parity_main by
replacing or supplementing the existing 0.0 entry with a representable
negative-zero value in both f and g. Keep the arrays otherwise identical so
check validates native and comparator sorting across the ±0 boundary.
In `@baml_language/crates/baml_tests/baml_src/ns_fs/fs_from_rust.baml`:
- Around line 117-126: Avoid retaining catch-expression results in local let
bindings across all listed sites: in fs_from_rust.baml at lines 117-126, 49-56,
89-102, 158-166, 180-188, 203-210, 226-233, 248-257, 290-297, 330-337, and
369-376, perform message checks, boolean assertions, returns, and cleanup
directly within the catch/result branches; in glob.baml at lines 357-368 and
392-405, move cleanup into both result paths and return or assert the catch
expression inline. Preserve each helper’s existing validation and cleanup
behavior without assigning the caught value or branch result to threw or another
local binding.
In
`@baml_language/crates/baml_tests/baml_src/ns_interfaces_associated_types/interfaces_associated_types.baml`:
- Around line 38-48: Update atp_throws_main so both AlwaysFails
{}.value_plus_one() catch and call_value(AlwaysFails {}) catch expressions are
consumed inline in the returned expression instead of being assigned to
default_result and generic_result locals. Preserve the existing catch behavior
and explicit string conversion while avoiding boxed local results.
In `@baml_language/crates/baml_tests/baml_src/ns_interfaces/interfaces_2.baml`:
- Around line 43-60: Update F1saDescribable.describe and its F1saPair
implementation so describe accesses at least one receiver field through self,
while preserving the existing "a pair" result expected by f1sa_main and the
regression test.
In `@baml_language/crates/baml_tests/baml_src/ns_interfaces/interfaces.baml`:
- Around line 759-766: Update gbr_first_name to be generic with a type parameter
T constrained by GbrNamed, accepting T[] while returning the first item’s name.
Keep gbr_main’s concrete dogs array unchanged so the test exercises the generic
bound rather than only interface-array access.
In
`@baml_language/crates/baml_tests/baml_src/ns_streaming_parsing/streaming_parsing.baml`:
- Around line 139-146: The catch result is incorrectly stored in a local before
assertion; assert each catch expression inline, then cancel the task and return
true. Apply this in
baml_language/crates/baml_tests/baml_src/ns_streaming_parsing/streaming_parsing.baml
lines 139-146,
baml_language/crates/baml_tests/baml_src/ns_streaming_sse_primitives/streaming_sse_primitives.baml
lines 115-131, and the same file lines 324-340, preserving each test’s existing
error-status expectation.
---
Outside diff comments:
In `@baml_language/crates/baml_tests/tests/http.rs`:
- Around line 259-260: Restore host-backed timeout-failure coverage: in
baml_language/crates/baml_tests/tests/http.rs:259-260, add a silent-server HTTP
timeout test or equivalent; in
baml_language/crates/baml_tests/tests/net.rs:128-154, retain a TCP read that
genuinely expires and asserts the timeout error; and in
baml_language/crates/baml_tests/tests/net.rs:219-248, retain a UDP receive that
genuinely expires and asserts the timeout error.
---
Nitpick comments:
In `@baml_language/crates/baml_tests/tests/to_json_interface.rs`:
- Around line 6-15: Extract the duplicated compile_errors helper into a shared
test utility, preserving its current diagnostic filtering and formatting
behavior. In baml_language/crates/baml_tests/tests/to_json_interface.rs lines
6-15 and baml_language/crates/baml_tests/tests/tostring_sugar.rs lines 7-16,
remove the local compile_errors definitions and import the shared helper
instead.
🪄 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: 5a81790f-a3f4-467b-9be1-3d444bc26e54
⛔ Files ignored due to path filters (35)
baml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/array_length_snapshot.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/array_rest_binding.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/cancel_cascade.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/cleanup.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/comparable_sort.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/const_bindings.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/csv.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/defer.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/errorcontext.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/exceptions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/floats.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/from_json_interface.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/fs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/gc.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/glob.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/google_llm_env.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/http.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/interfaces.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/interfaces_associated_types.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/interfaces_class_generics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/net.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/parse_companions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/prompt_tag_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/promptast_accessors.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/shell.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/streaming_parsing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/streaming_sse_primitives.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/string_regressions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/tagged_template_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/to_json_interface.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/to_string_interface.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/tostring_sugar.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/type_error_repro.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/wildcard_type_inference.snapis excluded by!**/*.snap
📒 Files selected for processing (72)
.github/workflows/cargo-tests.reusable.yamlbaml_language/Cargo.tomlbaml_language/crates/baml_cli/tests/common/mod.rsbaml_language/crates/baml_cli/tests/exit_code_e2e.rsbaml_language/crates/baml_cli/tests/pack_e2e.rsbaml_language/crates/baml_tests/baml_src/ns_array_length_snapshot/array_length_snapshot.bamlbaml_language/crates/baml_tests/baml_src/ns_array_rest_binding/array_rest_binding.bamlbaml_language/crates/baml_tests/baml_src/ns_cancel_cascade/cancel_cascade.bamlbaml_language/crates/baml_tests/baml_src/ns_cleanup/cleanup.bamlbaml_language/crates/baml_tests/baml_src/ns_comparable_sort/comparable_sort.bamlbaml_language/crates/baml_tests/baml_src/ns_const_bindings/const_bindings.bamlbaml_language/crates/baml_tests/baml_src/ns_csv/csv_smoke.bamlbaml_language/crates/baml_tests/baml_src/ns_defer/defer.bamlbaml_language/crates/baml_tests/baml_src/ns_env/env.bamlbaml_language/crates/baml_tests/baml_src/ns_errorcontext/errorcontext.bamlbaml_language/crates/baml_tests/baml_src/ns_exceptions/exceptions.bamlbaml_language/crates/baml_tests/baml_src/ns_floats/floats.bamlbaml_language/crates/baml_tests/baml_src/ns_from_json_interface/from_json_interface.bamlbaml_language/crates/baml_tests/baml_src/ns_fs/fs_from_rust.bamlbaml_language/crates/baml_tests/baml_src/ns_gc/gc.bamlbaml_language/crates/baml_tests/baml_src/ns_glob/glob.bamlbaml_language/crates/baml_tests/baml_src/ns_google_llm_env/google_llm_env.bamlbaml_language/crates/baml_tests/baml_src/ns_http/http.bamlbaml_language/crates/baml_tests/baml_src/ns_interfaces/interfaces.bamlbaml_language/crates/baml_tests/baml_src/ns_interfaces/interfaces_2.bamlbaml_language/crates/baml_tests/baml_src/ns_interfaces/interfaces_3.bamlbaml_language/crates/baml_tests/baml_src/ns_interfaces_associated_types/interfaces_associated_types.bamlbaml_language/crates/baml_tests/baml_src/ns_interfaces_class_generics/interfaces_class_generics.bamlbaml_language/crates/baml_tests/baml_src/ns_json_auto_derive/json_auto_derive.bamlbaml_language/crates/baml_tests/baml_src/ns_net/net.bamlbaml_language/crates/baml_tests/baml_src/ns_parse_companions/parse_companions.bamlbaml_language/crates/baml_tests/baml_src/ns_prompt_tag_runtime/prompt_tag_runtime.bamlbaml_language/crates/baml_tests/baml_src/ns_promptast_accessors/promptast_accessors.bamlbaml_language/crates/baml_tests/baml_src/ns_shell/shell.bamlbaml_language/crates/baml_tests/baml_src/ns_streaming_parsing/streaming_parsing.bamlbaml_language/crates/baml_tests/baml_src/ns_streaming_sse_primitives/streaming_sse_primitives.bamlbaml_language/crates/baml_tests/baml_src/ns_string_regressions/string_regressions.bamlbaml_language/crates/baml_tests/baml_src/ns_tagged_template_runtime/tagged_template_runtime.bamlbaml_language/crates/baml_tests/baml_src/ns_to_json_interface/to_json_interface.bamlbaml_language/crates/baml_tests/baml_src/ns_to_string_interface/to_string_interface.bamlbaml_language/crates/baml_tests/baml_src/ns_tostring_sugar/tostring_sugar.bamlbaml_language/crates/baml_tests/baml_src/ns_type_error_repro/type_error_repro.bamlbaml_language/crates/baml_tests/baml_src/ns_wildcard_type_inference/wildcard_type_inference.bamlbaml_language/crates/baml_tests/tests/array_length_snapshot.rsbaml_language/crates/baml_tests/tests/array_rest_binding.rsbaml_language/crates/baml_tests/tests/byte_strings.rsbaml_language/crates/baml_tests/tests/cancel_cascade.rsbaml_language/crates/baml_tests/tests/cleanup.rsbaml_language/crates/baml_tests/tests/comparable_sort.rsbaml_language/crates/baml_tests/tests/env.rsbaml_language/crates/baml_tests/tests/errorcontext.rsbaml_language/crates/baml_tests/tests/exceptions.rsbaml_language/crates/baml_tests/tests/floats.rsbaml_language/crates/baml_tests/tests/from_json_interface.rsbaml_language/crates/baml_tests/tests/fs.rsbaml_language/crates/baml_tests/tests/glob.rsbaml_language/crates/baml_tests/tests/google_llm_env.rsbaml_language/crates/baml_tests/tests/http.rsbaml_language/crates/baml_tests/tests/interfaces_associated_types.rsbaml_language/crates/baml_tests/tests/interfaces_class_generics.rsbaml_language/crates/baml_tests/tests/json_auto_derive.rsbaml_language/crates/baml_tests/tests/net.rsbaml_language/crates/baml_tests/tests/prompt_tag_e2e.rsbaml_language/crates/baml_tests/tests/prompt_tag_runtime.rsbaml_language/crates/baml_tests/tests/shell.rsbaml_language/crates/baml_tests/tests/streaming_parsing.rsbaml_language/crates/baml_tests/tests/tagged_template_runtime.rsbaml_language/crates/baml_tests/tests/to_json_interface.rsbaml_language/crates/baml_tests/tests/to_string_interface.rsbaml_language/crates/baml_tests/tests/tostring_sugar.rsbaml_language/crates/baml_tests/tests/type_error_repro.rsbaml_language/crates/baml_tests/tests/wildcard_type_inference.rs
💤 Files with no reviewable changes (3)
- baml_language/crates/baml_tests/baml_src/ns_json_auto_derive/json_auto_derive.baml
- baml_language/crates/baml_tests/tests/interfaces_class_generics.rs
- baml_language/crates/baml_tests/tests/to_string_interface.rs
…s to Rust The `baml_src` corpus test flaked ~40% under full-suite nextest load. The failing block was `net_read_timeout_fires`: the migration replaced the original Rust controlled silent server (a tokio listener held open past the client deadline) with `baml.http.Server.bind()` relying on an unaccepted backlog connection. That BAML listener object can be GC-collected between `connect` and the throwing `read`, resetting the connection into a `baml.errors.Io` error instead of the expected `Timeout`, so the `catch Timeout` misses and the block fails — intermittently, only under GC/CPU pressure (hence it passed standalone and in isolated verification). - Move `net_read_timeout_fires` (tests/net.rs) and `http_fetch_timeout_fires` (tests/http.rs, same latent listener-GC pattern) back to Rust with their original controlled silent servers. `ns_http` had no other blocks, so it's removed. `net.snap`/`_root.snap` regenerated. - Keep `net_udp_recv_timeout_fires` in the corpus: no listener to reset, and with no sender a `recv_from` deadline can only expire into `Timeout`. - Isolate the corpus runner's BAML_CACHE_DIR/BAML_HOME per run so the CLI no longer writes `.baml/cache` into the source tree that the bytecode / emit-determinism / link-oracle tests scan concurrently; make `collect_baml_files` skip dot-dirs as defense-in-depth. - Reconcile the canary merge: its new `generate_go_writes_sdk_through_cli` used `ensure_built()` against this branch's `run_baml_cli(&Path)` signature; switch it to `baml_cli()` (it only runs `baml generate`). Verified: 3x full `cargo nextest run -p baml_tests` all green (2822 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01J2XzpcdbF2ddFDrCgc7fuF
CodeRabbit flagged five corpus tests that were faithful ports of already-weak Rust originals — each passed without exercising the behavior its name claims. Strengthen them so the assertion actually depends on that behavior. All still pass (full corpus: 2467/0), confirming the underlying BAML semantics hold. - rest_bind_chain_aliases_same_slice: push through one chained binding and observe it via the other (44), proving `r`/`s` alias one slice rather than sharing an initial length (33). - phase2_user_class_compare_with_explicit_binding_in_main: call a helper that binds `high.compare(low)` to a local first, instead of duplicating the direct-call test. - parity float inputs: add a representable -0.0 so native and comparator sort are checked across the ±0 total-order boundary the comment promised. - form1_self_accesses_receiver_fields: `describe` returns `self.label` (a real field read) instead of a constant. - generic_bound_runtime: `gbr_first_name<T extends GbrNamed>(items: T[])` over a concrete `GbrDog[]`, exercising a generic bound rather than interface-array access. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01J2XzpcdbF2ddFDrCgc7fuF
Contributor
Author
|
@coderabbitai I've addressed this review — summary by comment. (The review predates commit 936a462, which is why it's still marked changes-requested.) Strengthened (commit 936a462) — each was a faithful port of an already-weak Rust original; all now exercise the behavior their name claims, and all still pass (full corpus 2467/0):
Not changed — the "catch result in a local Not changed — @coderabbitai review |
Contributor
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Follow-up commit =='
git show --stat --oneline --decorate --no-renames 936a462b4
echo '== Locate named corpus files =='
fd -a -t f -e baml . baml_language/crates/baml_tests/baml_src | \
rg '(array_rest_binding|comparable_sort|interfaces_2|interfaces_class_generics|cancel_cascade|bigints|fs_from_rust|glob|csv_smoke|errorcontext|interfaces_associated_types|streaming_)'
echo '== Strengthened-test snippets =='
rg -n -A20 -B8 \
'rest_bind_chain_aliases_same_slice|phase2_compare_explicit_binding_main|phase2_user_class_compare_with_explicit_binding_in_main|parity_main|form1_self_accesses_receiver_fields|generic_bound_runtime|gbr_first_name' \
baml_language/crates/baml_tests/baml_src
echo '== Boxing-workaround and affected helper scopes =='
rg -n -A14 -B5 \
'boxing|boxed|catch \(e\)|let .*catch|catch.*let|function .*fn|function .*main' \
baml_language/crates/baml_tests/baml_src/ns_bigints \
baml_language/crates/baml_tests/baml_src/ns_cancel_cascade \
baml_language/crates/baml_tests/baml_src/ns_fs \
baml_language/crates/baml_tests/baml_src/ns_glob \
baml_language/crates/baml_tests/baml_src/ns_csv \
baml_language/crates/baml_tests/baml_src/ns_errorcontext \
baml_language/crates/baml_tests/baml_src/ns_interfaces_associated_types \
baml_language/crates/baml_tests/baml_src/ns_streaming_parsing \
baml_language/crates/baml_tests/baml_src/ns_streaming_sse_primitives || true
echo '== Cancellation test and original Rust test =='
rg -n -A35 -B10 \
'cancel_unblocks_await_on_non_descendant|non_descendant|waiter|slow' \
baml_language/crates/baml_tests/baml_src/ns_cancel_cascade \
baml_language/crates/baml_tests/tests/cancel_cascade.rsLength of output: 200140 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Exact follow-up diff =='
git show --format=fuller --find-renames --find-copies -- \
936a462b4 \
baml_language/crates/baml_tests/baml_src/ns_array_rest_binding/array_rest_binding.baml \
baml_language/crates/baml_tests/baml_src/ns_comparable_sort/comparable_sort.baml \
baml_language/crates/baml_tests/baml_src/ns_interfaces/interfaces_2.baml \
baml_language/crates/baml_tests/baml_src/ns_interfaces_class_generics/interfaces_class_generics.baml
echo '== Narrow current assertions =='
rg -n -A16 -B6 \
'arb_rest_bind_chain_aliases_same_slice|rest_bind_chain_aliases_same_slice|phase2_compare_explicit_binding_main|phase2_user_class_compare_with_explicit_binding_in_main|float_nan_sort_main|parity_main|form1_self_accesses_receiver_fields|gbr_first_name|generic_bound_runtime' \
baml_language/crates/baml_tests/baml_src/ns_array_rest_binding/array_rest_binding.baml \
baml_language/crates/baml_tests/baml_src/ns_comparable_sort/comparable_sort.baml \
baml_language/crates/baml_tests/baml_src/ns_interfaces/interfaces_2.baml \
baml_language/crates/baml_tests/baml_src/ns_interfaces_class_generics/interfaces_class_generics.bamlLength of output: 18331 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -A18 -B8 \
'gbr_first_name|generic_bound_runtime|GbrNamed|GbrDog' \
baml_language/crates/baml_tests/baml_srcLength of output: 4667
I also confirmed the local-boxing issue is documented as specific to Re-reviewing the updated PR now. ✏️ Learnings added
✅ Action performedReview finished.
|
Merged
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.
Goal
Get CI (`CI - BAML Language`) from ~12 min toward <5 min. The critical path was the two linux cargo jobs (~11 min each, 8-vcpu saturated). Almost all of that time is test execution, not compilation: the tests run the compiler thousands of times.
What's here (2 commits)
perf(ci)— cut cargo-test wall timecargo testjob re-ranbaml_tests/baml_cli/baml_lsp2_actionsthat the snapshot job already covers on the same OS. Now excluded at the cargo level (--workspace --exclude), not just via a nextest-Efilter, so their heavy test binaries aren't even built. The duplicatedpack_e2estep is dropped (the snapshot job runs it).CARGO_PROFILE_DEV/TEST_OPT_LEVEL=1in CI, andsalsa(the query engine that runs on every compile) at opt-level 3 in the dev profile.exit_code_e2e,pack_e2e) hit a cold<project>/.baml/cacheper test and recompiled the stdlib every time (~14s/spawn). They now share aBAML_CACHE_DIRunderCARGO_TARGET_TMPDIR, so only the first invocation pays the stdlib compile. The cache is content-addressed and fingerprinted on the CLI binary, so sharing across tests/runs is safe.exit_code_e2ecalledensure_built()(which shellscargo build -p baml_pack_host, ~10s/process even when fresh) despite never runningbaml pack; switched tobaml_cli().test(baml_tests)— move exec tests into the BAML corpusEach Rust-side
baml_test!test recompiled a BAML project from scratch in its own nextest process (~0.7–2.5s); atest { }block in thebaml_src/corpus compiles once with the whole project and runs for ~40ms.baml-cli test.setup_test_db+collect_diagnostics), host-FFI tests (BexExternalValue, VM introspection), host infrastructure (stub HTTP/TCP servers, symlink creation),OptLevel::Twodispatch, and#[should_panic]compile-time checks.baml_src/*.snap.Verification
cargo nextest run -p baml_tests: 2819/2819 pass.exit_code_e2esuite 574s CPU → 147s.Notes for review
baml_test!behind an off-by-defaultrust-baml-exec-testsso new BAML behavior tests are written in the corpus, not Rust (compile-diagnostics tests keep running under a plaincargo test).python-pydantic2 (windows)andTest Go Client Windows Supportjobs are unrelated to this change.🤖 Generated with Claude Code
Summary by CodeRabbit