Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,21 @@ Please remember that BAML is maintained by humans and is used in production
for a wide range of critical use cases. We expect our contributors to be
thoughtful about their work and engage with us, either in our Discord or on
the issue or pull request.

# Getting Started

See [README-DEV.md](./README-DEV.md) for environment setup (via
[mise](https://mise.jdx.dev/) and `./scripts/setup-dev.sh`), the development
workflow, and how to run tests. In short:

```bash
curl https://mise.run | sh # install mise
./scripts/setup-dev.sh # install Rust and everything else
```

Rust changes go in the `baml_language/` workspace — run `cargo test --lib`
there before submitting, and format with:

```bash
cargo fmt -- --config imports_granularity="Crate" --config group_imports="StdExternalCrate"
```
24 changes: 24 additions & 0 deletions README-DEV.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,30 @@ cd integ-tests/python && uv run pytest
cd integ-tests/ruby && rake test
```

### Rust Workspace (`baml_language/`)

The compiler, engine, and LSP server live in the `baml_language/` Cargo
workspace. Common commands (run from `baml_language/`):

```bash
# Unit tests for the whole workspace (always run these after Rust changes)
cargo test --lib

# Unit tests for one crate
cargo test --lib -p bex_project

# Format with the repo's import style (fixes most lints)
cargo fmt -- --config imports_granularity="Crate" --config group_imports="StdExternalCrate"

# The LSP/runtime crates must also compile for the browser playground
cargo check --target wasm32-unknown-unknown -p bridge_wasm
```

Start with `baml_language/ARCHITECTURE.md` for the compiler pipeline and
`baml_language/TEST_INSTRUCTIONS.md` for the snapshot-test workflow. Design
documents for larger subsystems (e.g. the LSP server's locking and rebuild
pipeline) live in `docs/design/`.

### Building

```bash
Expand Down
2 changes: 2 additions & 0 deletions baml_language/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 9 additions & 9 deletions baml_language/TEST_INSTRUCTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@
| Suite | Location | Purpose |
|-------|----------|---------|
| `baml_tests` | `crates/baml_tests/` | Snapshot tests with detailed CST/HIR/THIR output |
| `lsp_actions_tests` | `crates/lsp_actions_tests/` | LSP integration tests with inline expectations |
| `baml_lsp2_actions_tests` | `crates/baml_lsp2_actions_tests/` | LSP integration tests with inline expectations |

## Workflow: Debugging a Failing Test

### 1. Identify the issue in lsp_actions_tests
### 1. Identify the issue in baml_lsp2_actions_tests

```bash
cargo test --package lsp_actions_tests
cargo test --package baml_lsp2_actions_tests
```

Look for errors in `crates/lsp_actions_tests/test_files/syntax/`.
Look for errors in `crates/baml_lsp2_actions_tests/test_files/syntax/`.

### 2. Create a minimal repro in baml_tests

Expand Down Expand Up @@ -64,16 +64,16 @@ Edit the relevant crate (`baml_compiler_parser`, `baml_compiler_syntax`, `baml_c
cargo test --package baml_tests my_repro
cargo insta accept --all

# Update lsp_actions_tests inline expectations
UPDATE_EXPECT=1 cargo test --package lsp_actions_tests
# Update baml_lsp2_actions_tests inline expectations
UPDATE_EXPECT=1 cargo test --package baml_lsp2_actions_tests
```

### 7. Verify all tests pass

```bash
# Run all tests (can skip slow parser_stress with --skip parser_stress)
cargo test --package baml_tests -- --skip parser_stress
cargo test --package lsp_actions_tests
cargo test --package baml_lsp2_actions_tests
Comment on lines 71 to +76

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Include the required library test command.

This verification block omits cargo test --lib. Because this PR changes Rust code, add that command to the documented verification workflow.

As per coding guidelines, always run cargo test --lib when Rust code changes.

🤖 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 71 - 76, Add the required
`cargo test --lib` command to the “Verify all tests pass” section, alongside the
existing Rust test commands, so the documented workflow covers library tests for
Rust changes.

Source: Coding guidelines

```

## Quick Commands
Expand All @@ -89,7 +89,7 @@ cargo test --package baml_tests
cargo test --package baml_tests -- --skip parser_stress

# Run LSP tests and auto-update expectations
UPDATE_EXPECT=1 cargo test --package lsp_actions_tests
UPDATE_EXPECT=1 cargo test --package baml_lsp2_actions_tests

# Accept all pending snapshots
cargo insta accept --all
Expand All @@ -108,7 +108,7 @@ cargo insta review
- **Type checking**: `crates/baml_compiler2_tir/src/builder.rs`


DO NOT EDIT the diagnostics manually in lsp_actions_tests. Use update_expect=1
DO NOT EDIT the diagnostics manually in baml_lsp2_actions_tests. Use update_expect=1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use the correct environment variable name.

update_expect=1 is lowercase here, but the commands above use the case-sensitive variable UPDATE_EXPECT=1.

Proposed fix
-DO NOT EDIT the diagnostics manually in baml_lsp2_actions_tests. Use update_expect=1
+DO NOT EDIT the diagnostics manually in baml_lsp2_actions_tests. Use UPDATE_EXPECT=1
📝 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
DO NOT EDIT the diagnostics manually in baml_lsp2_actions_tests. Use update_expect=1
DO NOT EDIT the diagnostics manually in baml_lsp2_actions_tests. Use UPDATE_EXPECT=1
🤖 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` at line 111, Correct the environment
variable reference in the “DO NOT EDIT” instruction to use the case-sensitive
name UPDATE_EXPECT=1, matching the commands documented above.


Find the base-case that makes syntax fail and add that to baml_test with a good name and good folder organization.

Expand Down
2 changes: 1 addition & 1 deletion baml_language/crates/baml_cli/src/paint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ impl<'db> Highlighter<'db> {
if let Some(cached) = self.cache.borrow().get(&file) {
return Rc::clone(cached);
}
let mut toks = semantic_tokens(self.db, file);
let mut toks = semantic_tokens(self.db, file).clone();
toks.sort_by_key(|t| t.range.start());
let rc: Rc<[SemanticToken]> = toks.into();
self.cache.borrow_mut().insert(file, Rc::clone(&rc));
Expand Down
34 changes: 20 additions & 14 deletions baml_language/crates/baml_lsp2_actions/src/annotations.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Inline type / parameter-name annotations for BAML files (inlay hints).
//!
//! Provides `annotations(db, file) -> Vec<InlineAnnotation>` — a regular
//! function (not a Salsa query) that walks expression-body functions in a file
//! Provides `file_annotations(db, file) -> &Vec<InlineAnnotation>` — a Salsa
//! tracked query that walks expression-body functions in a file
//! (top-level functions, class/interface methods, and the synthesized
//! `$init_test` registration functions), recursing into lambda bodies (e.g.
//! the bodies of `test` / `testset` blocks, which lower to lambdas passed to
Expand Down Expand Up @@ -81,7 +81,7 @@ pub enum AnnotationKind {
}

/// A single inline annotation (inlay hint) to display in the editor.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Eq, salsa::Update)]
pub struct InlineAnnotation {
/// Byte offset in the file where the hint is inserted.
pub offset: TextSize,
Expand All @@ -102,10 +102,16 @@ pub struct InlineAnnotation {
/// Returns annotations sorted in document order (required by the LSP
/// `textDocument/inlayHint` contract).
///
/// Regular function (not a Salsa query). Internally calls Salsa-cached
/// queries (`function_body`, `function_body_source_map`,
/// `infer_scope_types`, `file_item_tree`, `file_semantic_index`).
pub fn annotations(db: &dyn Db, file: SourceFile) -> Vec<InlineAnnotation> {
/// Salsa tracked query (design B1): walks every function body against type
/// inference (measured 40–150ms on real projects), which is too slow to
/// recompute per request while the file is unchanged. Editors re-request
/// inlay hints on every scroll, so this is the hottest read path.
///
/// Named `file_annotations` (like `file_outline`) because the tracked-query
/// machinery claims the bare name in the type namespace, which would collide
/// with this module.
#[salsa::tracked(returns(ref))]
pub fn file_annotations(db: &dyn Db, file: SourceFile) -> Vec<InlineAnnotation> {
let item_tree = baml_compiler2_hir::file_item_tree(db, file);
let index = baml_compiler2_hir::file_semantic_index(db, file);

Expand Down Expand Up @@ -433,7 +439,7 @@ function UseEcho() -> string {
);
let project = builder.build();

let hints = annotations(&project.db, project.files[0]);
let hints = file_annotations(&project.db, project.files[0]);
let labels: Vec<_> = hints.iter().map(|hint| hint.label.as_str()).collect();

assert!(
Expand Down Expand Up @@ -473,7 +479,7 @@ function Demo(items: int[]) -> string {
"##,
);
let project = builder.build();
let hints = annotations(&project.db, project.files[0]);
let hints = file_annotations(&project.db, project.files[0]);

// The synthesized `.push(...)` calls must not surface `value:`-style hints.
assert!(
Expand Down Expand Up @@ -510,7 +516,7 @@ function Greet(name: string, items: int[]) -> string {
);
let project = builder.build();

let hints = annotations(&project.db, project.files[0]);
let hints = file_annotations(&project.db, project.files[0]);
let labels: Vec<_> = hints.iter().map(|hint| hint.label.as_str()).collect();

// `${expr}` lowers to `string.from(expr)`; that synthetic wrapper call
Expand Down Expand Up @@ -547,7 +553,7 @@ test "greets" {
);
let project = builder.build();

let hints = annotations(&project.db, project.files[0]);
let hints = file_annotations(&project.db, project.files[0]);
let labels: Vec<_> = hints.iter().map(|hint| hint.label.as_str()).collect();

assert!(
Expand Down Expand Up @@ -582,7 +588,7 @@ class Greeter {
);
let project = builder.build();

let hints = annotations(&project.db, project.files[0]);
let hints = file_annotations(&project.db, project.files[0]);
let labels: Vec<_> = hints.iter().map(|hint| hint.label.as_str()).collect();

assert!(
Expand Down Expand Up @@ -613,7 +619,7 @@ testset "math" {
);
let project = builder.build();

let hints = annotations(&project.db, project.files[0]);
let hints = file_annotations(&project.db, project.files[0]);
let labels: Vec<_> = hints.iter().map(|hint| hint.label.as_str()).collect();

assert!(
Expand Down Expand Up @@ -647,7 +653,7 @@ function Later() -> string {
builder.source("main.baml", source);
let project = builder.build();

let hints = annotations(&project.db, project.files[0]);
let hints = file_annotations(&project.db, project.files[0]);
let y_offset = TextSize::from(
u32::try_from(source.find("\"y\"").expect("test arg")).expect("offset fits"),
);
Expand Down
2 changes: 1 addition & 1 deletion baml_language/crates/baml_lsp2_actions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub trait Db: baml_compiler2_tir::Db {}
// ── Public API re-exports ─────────────────────────────────────────────────────

pub use actions::{FileAction, FileActionKind, file_actions};
pub use annotations::{AnnotationKind, InlineAnnotation, annotations};
pub use annotations::{AnnotationKind, InlineAnnotation, file_annotations};
// Re-export `DefinitionKind` so callers (e.g. bex_project) don't need to
// depend on `baml_compiler2_hir` directly just for type conversions.
pub use baml_compiler2_hir::contributions::DefinitionKind;
Expand Down
11 changes: 7 additions & 4 deletions baml_language/crates/baml_lsp2_actions/src/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl SemanticTokenType {
// ── SemanticToken ─────────────────────────────────────────────────────────────

/// A classified token ready for LSP encoding.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Eq, salsa::Update)]
pub struct SemanticToken {
pub range: TextRange,
pub token_type: SemanticTokenType,
Expand All @@ -152,9 +152,12 @@ pub struct SemanticToken {
/// Always returns tokens in document order (required by the LSP
/// `textDocument/semanticTokens/full` contract).
///
/// Regular function (not a Salsa query). Internally calls Salsa-cached queries
/// (`infer_scope_types`, `function_body`, `function_body_source_map`,
/// `file_semantic_index`, `syntax_tree`).
/// Salsa tracked query (design B1): the CST walk re-classifies every token
/// against type inference, which measured 40–150ms on real projects — too
/// slow to recompute per request while the file is unchanged. Memoization
/// keys off the file revision; edits to *other* files reuse this file's
/// result if its inference inputs are unaffected.
#[salsa::tracked(returns(ref))]
pub fn semantic_tokens(db: &dyn Db, file: SourceFile) -> Vec<SemanticToken> {
let root = baml_compiler_parser::syntax_tree(db, file);
let mut out = Vec::new();
Expand Down
8 changes: 4 additions & 4 deletions baml_language/crates/baml_lsp2_actions_tests/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{collections::HashMap, path::Path};

use baml_compiler_diagnostics::{RenderConfig, render_diagnostic};
use baml_lsp2_actions::{
annotations::{AnnotationKind, InlineAnnotation, annotations},
annotations::{AnnotationKind, InlineAnnotation, file_annotations},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Keep inlay-hint collection on the Salsa-tracked query path.

The supplied baml_lsp2_actions/src/annotations.rs defines file_annotations as a plain pub fn, so this change appears to bypass the B1 memoized query. The production LSP handlers also call this API while claiming the results are Salsa-memoized, which can re-run annotation aggregation on every request.

Make file_annotations the tracked returns(ref) query, or delegate to the tracked query and clone only at the ownership boundary.

Also applies to: 168-170

🤖 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_lsp2_actions_tests/src/runner.rs` at line 7, The
annotation collection currently bypasses Salsa memoization because
file_annotations is a plain function. Update file_annotations in annotations.rs
to be the tracked returns(ref) query, or delegate to that tracked query and
clone only at the ownership boundary; ensure the production LSP handlers and
runner use this tracked path rather than recomputing annotation aggregation.

check::check_file,
completions::completions_at,
tokens::{SemanticToken, semantic_tokens},
Expand Down Expand Up @@ -165,9 +165,9 @@ pub fn run_test(parsed: &ParsedTestFile) -> TestResult {
let actual_inlay_hints = if parsed.expected_inlay_hints.is_some() {
let mut all_hints: Vec<(String, InlineAnnotation)> = Vec::new();
for (filename, source_file) in &file_map {
let hints = annotations(&db, *source_file);
let hints = file_annotations(&db, *source_file);
for hint in hints {
all_hints.push((filename.clone(), hint));
all_hints.push((filename.clone(), hint.clone()));
}
}

Expand All @@ -184,7 +184,7 @@ pub fn run_test(parsed: &ParsedTestFile) -> TestResult {
for (filename, source_file) in &file_map {
let tokens = semantic_tokens(&db, *source_file);
for token in tokens {
all_tokens.push((filename.clone(), token));
all_tokens.push((filename.clone(), token.clone()));
}
}

Expand Down
Loading
Loading