docs: update baml_language documentation - #4130
Closed
sxlijin wants to merge 1 commit into
Closed
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis documentation-focused change updates compiler test workflows, VM and heap architecture descriptions, SDK testing guidance, Java bridge references, runtime distribution instructions, benchmark documentation, and cloud test teardown notes. ChangesCompiler testing
VM and heap architecture
SDK testing and Java bridge
Runtime distribution and tooling
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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):
|
![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: 8
🧹 Nitpick comments (1)
baml_language/crates/bex_engine/README.md (1)
7-8: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClarify that shared globals are frozen read-only views.
SharedGlobalsis the post-$initglobal pool shared by clonedcall_functionVMs;StoreGlobalis rejected against this shared view. Document that ownership rule explicitly so readers do not assume per-call snapshots or writable global state.🤖 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/bex_engine/README.md` around lines 7 - 8, Update the concurrency description around BexEngine::call_function to state that SharedGlobals is the post-$init global pool shared by cloned VMs as a frozen read-only view, not a per-call snapshot. Explicitly note that StoreGlobal is rejected for this shared view and clarify that global state is not writable per call.
🤖 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/BENCHMARKS.md`:
- Around line 3-9: Update the introductory description in BENCHMARKS.md to call
the entries “benchmark and profiling binaries” instead of “Divan benchmark
binaries,” while preserving the existing table and its individual benchmark
descriptions.
- Line 38: Update the Linux profiling guidance in BENCHMARKS.md to include the
concrete perf stat command for ./target/profiling/cache_profile_workload, using
the event list instructions, cycles, L1-icache-load-misses,
L1-dcache-load-misses, and branch-misses instead of the vague
equivalent-workload wording.
In `@baml_language/crates/bex_heap/README.md`:
- Around line 7-17: Update the heap storage description to clarify that
fixed-address ChunkedVec chunks prevent relocation when the backing storage
grows, not during garbage collection. Explicitly state that copying/compacting
GC may relocate objects and that HeapPtr values can be forwarded or otherwise
updated accordingly.
In `@baml_language/crates/tools_compile_profile/README.md`:
- Line 5: Update the README description of the compile profiling flow to state
that get_bytecode() runs conditionally, not on every invocation. Reflect the
--check-only mode and the path where user-file errors prevent emission, while
preserving the existing pipeline description for runs that perform bytecode
generation.
In `@baml_language/forks/aws-config-systest/README.md`:
- Line 73: Synchronize teardown guidance in
baml_language/forks/aws-config-systest/README.md at lines 73-73 by updating
resource filters and new-auth cleanup instructions to use the
e2e-<timestamp>-<pid> names and centralized finally cleanup. In
baml_language/forks/google-cloud-auth-systest/README.md at lines 67-67, replace
the stale trap-based cleanup instructions with the centralized finally path and
update dependent verification guidance accordingly.
In `@baml_language/sdk_tests/README.md`:
- Line 52: Update the sdk_test_go target description in the README to use the
exact package_edges fixture identifier instead of “package-edge fixture,”
preserving the surrounding wording and matrix structure.
In `@baml_language/sdks/agent-docs/bridge-ref/ref-java-state-of-completeness.md`:
- Line 11: Align the nightly-version contract across both references: in
baml_language/sdks/agent-docs/bridge-ref/ref-java-state-of-completeness.md lines
11-11, explicitly define the “.c” suffix as part of the canonical nightly format
or replace the concrete version with the documented format; in
baml_language/sdks/agent-docs/bridge-ref/ref-java-packaging.md lines 45-48,
update the nightly versioning rule and example to use the same canonical format.
In `@baml_language/TEST_INSTRUCTIONS.md`:
- Around line 105-106: Update the documentation sentence describing where
projects belong to call them “project fixtures” rather than “generated project
tests.” Keep the listed tier directories and guidance about choosing the tier
based on expected compiler outcome, while clarifying that build.rs generates the
Rust tests separately.
---
Nitpick comments:
In `@baml_language/crates/bex_engine/README.md`:
- Around line 7-8: Update the concurrency description around
BexEngine::call_function to state that SharedGlobals is the post-$init global
pool shared by cloned VMs as a frozen read-only view, not a per-call snapshot.
Explicitly note that StoreGlobal is rejected for this shared view and clarify
that global state is not writable per call.
🪄 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: a62c87e3-3ba8-4d22-adb2-d94bfa519b80
📒 Files selected for processing (31)
baml_language/ARCHITECTURE.mdbaml_language/TEST_INSTRUCTIONS.mdbaml_language/crates/baml_tests/BENCHMARKS.mdbaml_language/crates/baml_tests/README.mdbaml_language/crates/bex_engine/README.mdbaml_language/crates/bex_heap/README.mdbaml_language/crates/bex_vm/README.mdbaml_language/crates/bridge_ctypes/README.mdbaml_language/crates/tools_compile_profile/README.mdbaml_language/crates/tools_semantic_tokens/README.mdbaml_language/crates/tools_stow/README.mdbaml_language/forks/aws-config-systest/README.mdbaml_language/forks/google-cloud-auth-systest/README.mdbaml_language/sdk_tests/DEVELOPMENT.mdbaml_language/sdk_tests/README.mdbaml_language/sdk_tests/fixtures/llm_functions/recordings/README.mdbaml_language/sdks/agent-docs/bridge-ref/ref-java-codegen-conventions.mdbaml_language/sdks/agent-docs/bridge-ref/ref-java-examples.mdbaml_language/sdks/agent-docs/bridge-ref/ref-java-inbound-encoding.mdbaml_language/sdks/agent-docs/bridge-ref/ref-java-outbound-decoding.mdbaml_language/sdks/agent-docs/bridge-ref/ref-java-packaging.mdbaml_language/sdks/agent-docs/bridge-ref/ref-java-state-of-completeness.mdbaml_language/sdks/agent-docs/bridge-ref/ref-java-type-mappings.mdbaml_language/sdks/cpp/README.mdbaml_language/sdks/java/baml-bridge-kotlin/README.mdbaml_language/sdks/java/baml_bridge/PUBLISHING.mdbaml_language/sdks/java/examples/quickstart/README.mdbaml_language/sdks/java/gradle-plugin/README.mdbaml_language/sdks/python/README.mdbaml_language/sdks/python/rust/bridge_python/README.mdbaml_language/sdks/rust/bridge_rust/README.md
Comment on lines
+3
to
+9
| This crate currently defines three Divan benchmark binaries: | ||
|
|
||
| ## Running Benchmarks | ||
| | Bench | Source | Measures | | ||
| |---|---|---| | ||
| | `compiler_benchmark` | `benches/compiler_benchmark.rs` | Cold compilation of an empty project and the full `baml_src/` test project | | ||
| | `runtime_benchmark` | `benches/runtime_benchmark.rs` | Pure VM execution workloads generated from `tools/speedtest/workloads/**/*.md` | | ||
| | `cache_profile` | `benches/cache_profile.rs` | Hardware instruction/cache/branch counters for fixed VM workloads on Apple Silicon | |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe cache_profile as a profiling binary, not a Divan benchmark.
cache_profile.rs is a standalone profiling executable rather than a Divan suite, so calling all three entries “Divan benchmark binaries” contradicts the later description and may mislead readers. Use “benchmark and profiling binaries” here.
🤖 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/BENCHMARKS.md` around lines 3 - 9, Update the
introductory description in BENCHMARKS.md to call the entries “benchmark and
profiling binaries” instead of “Divan benchmark binaries,” while preserving the
existing table and its individual benchmark descriptions.
| - Check that `after/` only contains changed files | ||
| - Verify `.delete` markers are empty files | ||
| - Look for parse errors in benchmark files No newline at end of file | ||
| On Linux, use `perf stat` around an equivalent workload; the executable prints the suggested event list on unsupported hosts. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the concrete Linux profiling command.
On Linux, cache_profile prints a specific perf stat command targeting ./target/profiling/cache_profile_workload; “around an equivalent workload” is too vague for users following this guide.
Suggested wording
-On Linux, use `perf stat` around an equivalent workload; the executable prints the suggested event list on unsupported hosts.
+On Linux, run:
+
+```bash
+perf stat -e instructions,cycles,L1-icache-load-misses,L1-dcache-load-misses,branch-misses ./target/profiling/cache_profile_workload
+```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| On Linux, use `perf stat` around an equivalent workload; the executable prints the suggested event list on unsupported hosts. | |
| On Linux, run: | |
🤖 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/BENCHMARKS.md` at line 38, Update the Linux
profiling guidance in BENCHMARKS.md to include the concrete perf stat command
for ./target/profiling/cache_profile_workload, using the event list
instructions, cycles, L1-icache-load-misses, L1-dcache-load-misses, and
branch-misses instead of the vague equivalent-workload wording.
Comment on lines
+7
to
+17
| The heap is shared as `Arc<BexHeap>`. Objects live in fixed-address `ChunkedVec<Object>` storage divided into these generations: | ||
|
|
||
| ## Object Indexing | ||
| | Generation | Purpose | | ||
| |---|---| | ||
| | `CompileTime` | Permanent functions, classes, enums, types, and other program objects | | ||
| | `Gen0` | Nursery for every new TLAB allocation | | ||
| | `Gen1` | Intermediate generation for Gen0 survivors | | ||
| | `Gen2` | Old generation for long-lived objects | | ||
| | inactive | Copy destination used during collection | | ||
|
|
||
| Objects are referenced via `ObjectIndex` (a `usize` with optional epoch for debug): | ||
| Each VM owns a `Tlab`. The fast allocation path bumps its private cursor and writes directly into its reserved Gen0 region. Refilling a TLAB atomically reserves another chunk; the default reservation is 1,024 object slots. The backing `ChunkedVec` uses 4,096-element chunks so growth does not move object storage held by other VMs. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify that GC may relocate objects.
“Fixed-address ChunkedVec<Object> storage” can imply that HeapPtr addresses remain stable across collections, but this README later defines copying/compacting GC and pointer forwarding. Limit the guarantee to ChunkedVec growth and explicitly state that GC may relocate objects.
Proposed wording
-The heap is shared as `Arc<BexHeap>`. Objects live in fixed-address `ChunkedVec<Object>` storage divided into these generations:
+The heap is shared as `Arc<BexHeap>`. Objects live in chunked `ChunkedVec<Object>` storage divided into these generations. Chunk growth does not move existing slots, but copying GC may relocate objects and requires pointer forwarding.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| The heap is shared as `Arc<BexHeap>`. Objects live in fixed-address `ChunkedVec<Object>` storage divided into these generations: | |
| ## Object Indexing | |
| | Generation | Purpose | | |
| |---|---| | |
| | `CompileTime` | Permanent functions, classes, enums, types, and other program objects | | |
| | `Gen0` | Nursery for every new TLAB allocation | | |
| | `Gen1` | Intermediate generation for Gen0 survivors | | |
| | `Gen2` | Old generation for long-lived objects | | |
| | inactive | Copy destination used during collection | | |
| Objects are referenced via `ObjectIndex` (a `usize` with optional epoch for debug): | |
| Each VM owns a `Tlab`. The fast allocation path bumps its private cursor and writes directly into its reserved Gen0 region. Refilling a TLAB atomically reserves another chunk; the default reservation is 1,024 object slots. The backing `ChunkedVec` uses 4,096-element chunks so growth does not move object storage held by other VMs. | |
| The heap is shared as `Arc<BexHeap>`. Objects live in chunked `ChunkedVec<Object>` storage divided into these generations. Chunk growth does not move existing slots, but copying GC may relocate objects and requires pointer forwarding. | |
| | Generation | Purpose | | |
| |---|---| | |
| | `CompileTime` | Permanent functions, classes, enums, types, and other program objects | | |
| | `Gen0` | Nursery for every new TLAB allocation | | |
| | `Gen1` | Intermediate generation for Gen0 survivors | | |
| | `Gen2` | Old generation for long-lived objects | | |
| | inactive | Copy destination used during collection | | |
| Each VM owns a `Tlab`. The fast allocation path bumps its private cursor and writes directly into its reserved Gen0 region. Refilling a TLAB atomically reserves another chunk; the default reservation is 1,024 object slots. The backing `ChunkedVec` uses 4,096-element chunks so growth does not move object storage held by other VMs. |
🤖 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/bex_heap/README.md` around lines 7 - 17, Update the heap
storage description to clarify that fixed-address ChunkedVec chunks prevent
relocation when the backing storage grows, not during garbage collection.
Explicitly state that copying/compacting GC may relocate objects and that
HeapPtr values can be forwarded or otherwise updated accordingly.
| This is the tool to reach for when someone says **"`baml check` is slow on | ||
| my project"**. It runs the same pipeline `baml check` runs | ||
| (parse → HIR → PPIR → TIR → MIR → emit), and produces: | ||
| This is the tool to reach for when someone says **"compiling my project is slow"**. By default it runs `ProjectDatabase::check()` followed by `get_bytecode()` (parse → HIR → PPIR → TIR → MIR → emit), and produces: |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify get_bytecode() as conditional.
tools_compile_profile/src/main.rs skips db.get_bytecode() for --check-only and when user-file errors are reported. These descriptions currently imply that every run performs emission, which can make missing emit timings look like a documentation or runtime failure.
Suggested wording
-By default it runs `ProjectDatabase::check()` followed by `get_bytecode()` (parse → HIR → PPIR → TIR → MIR → emit), and produces:
+By default it runs `ProjectDatabase::check()` and, unless `--check-only` or user-file errors prevent emission, calls `get_bytecode()` (parse → HIR → PPIR → TIR → MIR → emit), and produces:
-It runs `check + get_bytecode` from a fresh database, not the incremental editor path.
+It runs `check` and conditionally `get_bytecode` from a fresh database, not the incremental editor path.Also applies to: 41-41
🤖 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/tools_compile_profile/README.md` at line 5, Update the
README description of the compile profiling flow to state that get_bytecode()
runs conditionally, not on every invocation. Reflect the --check-only mode and
the path where user-file errors prevent emission, while preserving the existing
pipeline description for runs that perform bytecode generation.
| `trap … EXIT INT TERM` master cleanup, so the EC2 instance, S3 bucket, IAM roles, instance | ||
| profile, and Lambda function are **always** removed — even on Ctrl-C or mid-run failure. After a | ||
| cloud run the script also leaves nothing behind; verify independently with: | ||
| The cloud tier gives every resource a `baml-systest` name containing the `e2e-<timestamp>-<pid>` run suffix and registers a single `finally` master cleanup (with SIGTERM converted to an exit), so the EC2 instance, S3 bucket, IAM roles, instance profile, and Lambda function are **always** removed — even on Ctrl-C or mid-run failure. After a cloud run the script also leaves nothing behind; verify independently with: |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Synchronize all cloud-system-test teardown guidance.
Both READMEs introduce centralized finally cleanup but retain instructions written for the previous trap-based contract. Update every dependent instruction and verification query:
baml_language/forks/aws-config-systest/README.md#L73-L73: align the resource filter and the “new auth form” cleanup guidance with the new names andfinallypath.baml_language/forks/google-cloud-auth-systest/README.md#L67-L67: replace the staletrapcleanup instruction with the centralizedfinallypath.
📍 Affects 2 files
baml_language/forks/aws-config-systest/README.md#L73-L73(this comment)baml_language/forks/google-cloud-auth-systest/README.md#L67-L67
🤖 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/forks/aws-config-systest/README.md` at line 73, Synchronize
teardown guidance in baml_language/forks/aws-config-systest/README.md at lines
73-73 by updating resource filters and new-auth cleanup instructions to use the
e2e-<timestamp>-<pid> names and centralized finally cleanup. In
baml_language/forks/google-cloud-auth-systest/README.md at lines 67-67, replace
the stale trap-based cleanup instructions with the centralized finally path and
update dependent verification guidance accordingly.
| | `sdk_test_typescript_web` | TypeScript generator and Web/Wasm bridge | generated ESM, `tsc`, Chromium Vitest, Workers Vitest | | ||
| | `sdk_test_rust` | Rust generator and `baml_bridge` | rustfmt, Clippy, Cargo tests with the engine library | | ||
| | `sdk_test_java` | Java generator, Java runtime jar, and JNI bridge | Gradle `compileTestJava`, JUnit for currently enabled fixtures | | ||
| | `sdk_test_go` | Go generator and Go bridge | `go test` for the supported source fixtures and synthetic package-edge fixture | |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the canonical package_edges fixture name.
The target matrix says “package-edge fixture”, but the actual fixture identifier is package_edges in the surrounding documentation. Keep the exact name so readers can locate and filter it reliably.
🤖 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/sdk_tests/README.md` at line 52, Update the sdk_test_go target
description in the README to use the exact package_edges fixture identifier
instead of “package-edge fixture,” preserving the surrounding wording and matrix
structure.
| Architecture: minimum supported Java is **17** (records + sealed interfaces; the de-facto library baseline — Spring Boot 3+ requires it). That rules out Panama/FFM (final only in JDK 22+), so the native binding is JNI. Proposed shape: a `bridge_java` Rust cdylib using the `jni` crate that links `bridge_cffi` in-process — the JVM analog of `bridge_python` (pyo3) / `bridge_nodejs` (napi) — speaking the same `baml_bridge.cffi.v1` protobuf envelopes. In-process linking also gives a true sync call path (like pyo3/napi) instead of synthesizing sync by blocking on the async-callback-only C ABI; async siblings return `CompletableFuture<T>` completed from the result callback. (Alternative if we ever need pure-dlopen: the `bridge_go` model over `bridge.h`, at the cost of hand-rolled JNI glue or a JNA/JNR dependency.) | ||
|
|
||
| **Status (2026-07-17):** the full value-model slice is green — `type_shapes` runs **110/110** against the live engine (primitives incl. bigint round-trip beyond i64 — `test_round_trip_bigint` + `test_return_bigint`, added 2026-07-20, closing the last 🚧 value-table row; unions 6/6, optional 5/5, lists 5/5, maps 4/4, enums 5/5, media 9/9, handles 4/4 (`baml.fs.File`/`baml.http.Response` shells incl. cursor-state encode-back), stream partial-values 5/5 (host-constructed `Resume$stream`/`Foo$stream` round-trips — engine accepts them; `$`-preserved in-package companions, TS-aligned, decided 2026-07-17), classes/refs/recursion/routing all green, generics 6/6 + 2/2, aliases 3/3, forward-refs 4/4); All 21 test files compile and pass — the compile-exclude list is empty. **Streaming *calls* — `BamlStream` — have now landed (the last unimplemented capability): `llm_functions` TestStreamingE2e runs 6/6 against a keyless in-process replay harness (native `BridgeEnv` setenv shim + BAML replay server), and `llm_functions` is now in the enforced gate (21/21, javac+junit, fully deterministic offline).** `function_calls` is now in the enforced CI gate too, all slices landed: **157 run / 153 pass / 0 fail / 4 skips** (155 + 2 new value-level async host-callable tests) (the skips are `@Disabled`/xfail cases: host-callable release-on-GC, a partial-subscript arity xfail, and two negative-runtime-case placeholders) — smoke 2/2, static+instance methods with `_async` siblings 7/7, optional args 4/4, stdlib entrypoints 3/3, `@throws` Javadoc 8/8, cancellation 7/7, errors/panics/os-exit green, generics (inferred + explicit) green, host-callables green (incl. a concurrent registry-isolation test); the compile-exclude list is empty. The bridge registers with the versioned C ABI at init (`BridgeLanguage::Java = 7`, canonical-version validated). Unions follow the cross-language team decision: the runtime's generic arity family `baml_bridge.Union2<A,B>`…`Union10<...>` (sealed, nested records `Arm0..Arm{n-1}` in declaration order → exhaustive `switch` on Java 21+) with **type-directed decode** — generated bindings pass a type-descriptor string; the wire/protobuf layer is unchanged. Recursive type aliases keep a nominal sealed type named after the alias. `baml generate` accepts `output_type = "java"`. 🚧 = implemented/emitted but not yet parity-verified. **Packaging (2026-07-16): `com.boundaryml:baml-bridge:0.15.0-nightly.1` is live on Maven Central** — GPG-signed main + linux-x86_64 natives + sources/javadoc jars; verified by a clean consumer resolving from Central only (no local copies) and executing BAML functions through the embedded native library. Remaining packaging work: full platform matrix via CI, GMM variant auto-selection, Gradle plugin (in progress). | ||
| **Status (2026-07-22):** the full value-model, function-call, streaming, generics, host-callable, error, cancellation, and stdlib slices are implemented and enforced by the Java SDK fixtures; their compile-exclude lists are empty. The bridge registers with the versioned C ABI at init (`BridgeLanguage::Java = 7`, canonical-version validated). Anonymous unions use `baml_bridge.Union2<A,B>` through `Union10<...>` with type-directed decode from generated `BamlType` descriptors; recursive aliases keep nominal sealed types. `baml generate` accepts `output_type = "java"`. Packaging is live on Maven Central at `0.15.1-nightly.20260722.c`: `baml-bridge` has the full eight-classifier platform set plus sources/javadoc, and the same release publishes `baml-gradle-plugin`, its marker POM, and `baml-bridge-kotlin`. The Gradle plugin is the supported automatic platform-selection layer; raw `baml-bridge` consumers select a `natives-<platform>` classifier explicitly because Gradle Module Metadata variants are not implemented. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the Maven nightly-version contract across both references.
The completeness snapshot uses 0.15.1-nightly.20260722.c, while the packaging guide documents 0.15.0-nightly.YYYYMMDD. This can produce invalid dependency coordinates unless the .c suffix is explicitly defined as part of the canonical format.
baml_language/sdks/agent-docs/bridge-ref/ref-java-state-of-completeness.md#L11-L11: document the canonical nightly suffix or correct the concrete version.baml_language/sdks/agent-docs/bridge-ref/ref-java-packaging.md#L45-L48: update the nightly versioning rule and example to match.
📍 Affects 2 files
baml_language/sdks/agent-docs/bridge-ref/ref-java-state-of-completeness.md#L11-L11(this comment)baml_language/sdks/agent-docs/bridge-ref/ref-java-packaging.md#L45-L48
🤖 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/sdks/agent-docs/bridge-ref/ref-java-state-of-completeness.md`
at line 11, Align the nightly-version contract across both references: in
baml_language/sdks/agent-docs/bridge-ref/ref-java-state-of-completeness.md lines
11-11, explicitly define the “.c” suffix as part of the canonical nightly format
or replace the concrete version with the documented format; in
baml_language/sdks/agent-docs/bridge-ref/ref-java-packaging.md lines 45-48,
update the nightly versioning rule and example to use the same canonical format.
Comment on lines
+105
to
+106
| Generated project tests must live under one of `projects/broken_syntax/`, `projects/diagnostic_errors/`, `projects/compiles/`, `projects/passing/`, or `projects/passing_llm/`. Choose the tier that matches the expected compiler outcome; the build script generates different phase tests and invariant checks for each tier. | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Call these project fixtures, not generated tests.
build.rs discovers fixture projects under projects/<tier>/<project>/ and emits the generated Rust tests into src/generated_tests.rs. Rename this sentence to avoid directing contributors toward the wrong artifact.
Proposed wording
-Generated project tests must live under one of `projects/broken_syntax/`, `projects/diagnostic_errors/`, `projects/compiles/`, `projects/passing/`, or `projects/passing_llm/`.
+Project fixtures must live under one of `projects/broken_syntax/`, `projects/diagnostic_errors/`, `projects/compiles/`, `projects/passing/`, or `projects/passing_llm/`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Generated project tests must live under one of `projects/broken_syntax/`, `projects/diagnostic_errors/`, `projects/compiles/`, `projects/passing/`, or `projects/passing_llm/`. Choose the tier that matches the expected compiler outcome; the build script generates different phase tests and invariant checks for each tier. | |
| Project fixtures must live under one of `projects/broken_syntax/`, `projects/diagnostic_errors/`, `projects/compiles/`, `projects/passing/`, or `projects/passing_llm/`. Choose the tier that matches the expected compiler outcome; the build script generates different phase tests and invariant checks for each tier. |
🤖 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/TEST_INSTRUCTIONS.md` around lines 105 - 106, Update the
documentation sentence describing where projects belong to call them “project
fixtures” rather than “generated project tests.” Keep the listed tier
directories and guidance about choosing the tier based on expected compiler
outcome, while clarifying that build.rs generates the Rust tests separately.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41385f771d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+34
to
+35
| cargo build -p baml_tests --bench cache_profile --profile profiling | ||
| sudo ./target/profiling/cache_profile --output cache-profile.json |
There was a problem hiding this comment.
On macOS/Apple Silicon, following this new cache_profile recipe builds a benchmark target, but cargo build --bench places harness=false bench executables under target/profiling/deps/cache_profile-<hash>, not at target/profiling/cache_profile (confirmed with Cargo's --bench target selection and a local smoke build). As written, the sudo command points at a nonexistent file, so users cannot run the profiling workflow; either run it via cargo bench ... -- --output ... or document how to locate/copy the hashed bench executable.
Useful? React with 👍 / 👎.
Binary size checks passed✅ 7 passed
Generated by |
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.
Summary by CodeRabbit