diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 41793062166..777a78893c2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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" +``` diff --git a/README-DEV.md b/README-DEV.md index 3f0575bfefb..80694a2def3 100644 --- a/README-DEV.md +++ b/README-DEV.md @@ -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 diff --git a/baml_language/Cargo.lock b/baml_language/Cargo.lock index f6336e8e2d8..d51461f7beb 100644 --- a/baml_language/Cargo.lock +++ b/baml_language/Cargo.lock @@ -1332,6 +1332,7 @@ dependencies = [ "regex", "serde", "serde_json", + "sys_native", "sys_ops", "sys_types", "text-size", @@ -1339,6 +1340,7 @@ dependencies = [ "tokio", "tracing", "vfs", + "walkdir", "wasm-bindgen-futures", ] diff --git a/baml_language/TEST_INSTRUCTIONS.md b/baml_language/TEST_INSTRUCTIONS.md index 49a6eb2422d..2a5df9ad576 100644 --- a/baml_language/TEST_INSTRUCTIONS.md +++ b/baml_language/TEST_INSTRUCTIONS.md @@ -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 @@ -64,8 +64,8 @@ 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 @@ -73,7 +73,7 @@ UPDATE_EXPECT=1 cargo test --package lsp_actions_tests ```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 ``` ## Quick Commands @@ -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 @@ -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 Find the base-case that makes syntax fail and add that to baml_test with a good name and good folder organization. diff --git a/baml_language/crates/baml_cli/src/paint.rs b/baml_language/crates/baml_cli/src/paint.rs index c9181a77d6e..c65c04a8661 100644 --- a/baml_language/crates/baml_cli/src/paint.rs +++ b/baml_language/crates/baml_cli/src/paint.rs @@ -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)); diff --git a/baml_language/crates/baml_lsp2_actions/src/annotations.rs b/baml_language/crates/baml_lsp2_actions/src/annotations.rs index ea5da87127f..81a5312e2c3 100644 --- a/baml_language/crates/baml_lsp2_actions/src/annotations.rs +++ b/baml_language/crates/baml_lsp2_actions/src/annotations.rs @@ -1,7 +1,7 @@ //! Inline type / parameter-name annotations for BAML files (inlay hints). //! -//! Provides `annotations(db, file) -> Vec` — a regular -//! function (not a Salsa query) that walks expression-body functions in a file +//! Provides `file_annotations(db, file) -> &Vec` — 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 @@ -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, @@ -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 { +/// 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 { let item_tree = baml_compiler2_hir::file_item_tree(db, file); let index = baml_compiler2_hir::file_semantic_index(db, file); @@ -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!( @@ -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!( @@ -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 @@ -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!( @@ -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!( @@ -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!( @@ -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"), ); diff --git a/baml_language/crates/baml_lsp2_actions/src/lib.rs b/baml_language/crates/baml_lsp2_actions/src/lib.rs index b02a07109d7..f59a1bcc9e7 100644 --- a/baml_language/crates/baml_lsp2_actions/src/lib.rs +++ b/baml_language/crates/baml_lsp2_actions/src/lib.rs @@ -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; diff --git a/baml_language/crates/baml_lsp2_actions/src/tokens.rs b/baml_language/crates/baml_lsp2_actions/src/tokens.rs index b9d5727fcb0..3577439b8b4 100644 --- a/baml_language/crates/baml_lsp2_actions/src/tokens.rs +++ b/baml_language/crates/baml_lsp2_actions/src/tokens.rs @@ -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, @@ -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 { let root = baml_compiler_parser::syntax_tree(db, file); let mut out = Vec::new(); diff --git a/baml_language/crates/baml_lsp2_actions_tests/src/runner.rs b/baml_language/crates/baml_lsp2_actions_tests/src/runner.rs index 7bde5fa90af..bf68c87d5a9 100644 --- a/baml_language/crates/baml_lsp2_actions_tests/src/runner.rs +++ b/baml_language/crates/baml_lsp2_actions_tests/src/runner.rs @@ -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}, check::check_file, completions::completions_at, tokens::{SemanticToken, semantic_tokens}, @@ -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())); } } @@ -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())); } } diff --git a/baml_language/crates/baml_lsp_server/src/playground_server.rs b/baml_language/crates/baml_lsp_server/src/playground_server.rs index 71ff0562eda..3b9bc4dce9f 100644 --- a/baml_language/crates/baml_lsp_server/src/playground_server.rs +++ b/baml_language/crates/baml_lsp_server/src/playground_server.rs @@ -1661,16 +1661,27 @@ async fn handle_function_run( }; let broadcast_tx = state.broadcast_tx.clone(); - let project_generation = state.bex.project_generation(&project).unwrap_or(0); - // Pin the run's control-flow graph while its generation is still - // current, so overlay spans stay resolvable after later recompiles. - if let Some(function_name) = overlay_function_name_for_target(&target.run_target) { - let _ = state.bex.control_flow_graph_for_generation( - &project, - project_generation, - function_name, - ); - } + // One coherent launch snapshot (design I5): engine + generation captured + // in a single transaction, with the overlay control-flow graph pinned + // for that generation so overlay spans stay resolvable after later + // recompiles. Replaces the racy generation → graph → engine triple-read. + let prepared = match state.bex.prepare_function_run( + &project, + overlay_function_name_for_target(&target.run_target), + ) { + Ok(prepared) => prepared, + Err(e) => { + send_ws( + sink, + &client.error("projectNotReady", format!("Cannot start run: {e}")), + ) + .await; + return; + } + }; + let project_generation = prepared.generation; + let bex = prepared.engine; + let fs_path = bex_project::FsPath::from_str(project); let boundary_id = BoundaryId::new_random(); let value_capture = @@ -1683,21 +1694,6 @@ async fn handle_function_run( }) .with_value_capture(value_capture.clone()); - let bex = match state.bex.get_bex_for_project(&fs_path) { - Ok(bex) => bex, - Err(e) => { - send_ws( - sink, - &client.error( - "projectMissing", - format!("Failed to get Bex for project: {e}"), - ), - ) - .await; - return; - } - }; - let run_store = state.run_store.clone(); let history_store = state.history_store.clone(); let value_store = state.value_store.clone(); @@ -1949,10 +1945,10 @@ async fn handle_ws_in_message( return; } }; - let project_id = state + let run_identity = state .run_store .snapshot(boundary_id) - .map(|run| run.request.project_id.0); + .map(|run| (run.request.project_id.0, run.request.project_generation.0)); let response = match state.run_store.cancel_run(boundary_id, epoch_ms(), None) { bex_events::run::CancelRunEffect::CancelHostCall { @@ -1962,10 +1958,25 @@ async fn handle_ws_in_message( broadcast_run_patch(&state.broadcast_tx, &patch); state.io_state.cancel_for_host_call(&host_call_id); state.env_state.cancel_for_host_call(&host_call_id); - match (host_call_id, project_id) { - (HostCallId::Native(call_id), Some(project_id)) => { - let fs_path = bex_project::FsPath::from_str(project_id); - match state.bex.get_bex_for_project(&fs_path) { + match (host_call_id, run_identity) { + (HostCallId::Native(call_id), Some((project_id, generation))) => { + // Best-effort cancel: `engine_for_generation` + // resolves only while the run's generation is + // still the installed one (only one engine is + // retained), so a run pinned to a superseded + // engine is not reachable from here — the + // current-engine fallback cannot find its call + // and the run keeps executing. Full D8 run + // registration is deferred. + let engine = state + .bex + .engine_for_generation(&project_id, generation) + .map(Ok) + .unwrap_or_else(|| { + let fs_path = bex_project::FsPath::from_str(project_id.clone()); + state.bex.get_bex_for_project(&fs_path) + }); + match engine { Ok(bex) => match bex.cancel_function_call(call_id) { Ok(()) => WsOutMessage::CommandAck { request_id, diff --git a/baml_language/crates/bex_engine/src/lib.rs b/baml_language/crates/bex_engine/src/lib.rs index c693dfa9629..cafcfde9910 100644 --- a/baml_language/crates/bex_engine/src/lib.rs +++ b/baml_language/crates/bex_engine/src/lib.rs @@ -698,6 +698,15 @@ pub struct BexEngine { /// construction (the config is read-once per process). Gates every /// profiling emission and the per-resume ring refresh. prof_enabled: bool, + + /// Whether this engine's profiling lifecycle has been activated + /// (metadata registered with the profiling consumer). Engines built via + /// [`BexEngine::new`] activate at construction. Candidate engines built + /// via [`BexEngine::new_with_deferred_profiling`] stay inactive until a + /// winning conditional commit calls [`BexEngine::activate_profiling`]; + /// a superseded candidate therefore drops without registering metadata + /// or emitting an `engine_closed` tombstone. + prof_activated: AtomicBool, } impl Drop for BexEngine { @@ -707,8 +716,12 @@ impl Drop for BexEngine { /// closes its `.bamlprof`, and frees its metadata. Without this, /// long-lived engine-churning hosts (LSP recompiles) accumulate open /// files and heartbeat work for dead engines. + /// + /// An engine whose profiling lifecycle was never activated (a discarded + /// candidate) drops quietly: it registered no metadata, so it must not + /// emit a close notification or leave a closed-engine tombstone. fn drop(&mut self) { - if self.prof_enabled { + if self.prof_enabled && self.prof_activated.load(Ordering::Acquire) { bex_events::prof::engine_closed(self.engine_id.0); } } @@ -1274,6 +1287,25 @@ impl BexEngine { bytecode_program: bex_vm_types::Program, sys_ops: std::sync::Arc, argv: Vec, + ) -> Result { + let engine = Self::new_with_deferred_profiling(bytecode_program, sys_ops, argv)?; + engine.activate_profiling(); + Ok(engine) + } + + /// Like [`BexEngine::new`], but keeps the profiling lifecycle inactive. + /// + /// Used for *candidate* engines that may be discarded before ever + /// becoming the installed engine (LSP conditional commit): construction — + /// including `$init` — runs normally, but no profiling metadata is + /// registered and dropping the candidate emits no `engine_closed` + /// notification. The winning commit calls + /// [`BexEngine::activate_profiling`] immediately before making the + /// engine reachable. + pub fn new_with_deferred_profiling( + bytecode_program: bex_vm_types::Program, + sys_ops: std::sync::Arc, + argv: Vec, ) -> Result { let argv: Arc<[String]> = Arc::from(argv); let process_euid = ProcessEuid::current(); @@ -1331,15 +1363,9 @@ impl BexEngine { obj }) .collect(); - if prof_enabled { - // The .bamlprof header consumes the M0 metadata table (its ids - // match the ids stamped on each Function above — same walk - // order, same 1-based sequence). - bex_events::prof::register_engine_metadata( - engine_id.0, - prof_engine_metadata(&program_metadata), - ); - } + // Profiling metadata registration is deferred to + // `activate_profiling()` so discarded candidate engines never leave + // consumer-side state (`BexEngine::new` activates immediately). // Pre-compute class and enum indices before moving objects to heap. // This is used for allocating instances/variants from sys-op results. @@ -1580,9 +1606,33 @@ impl BexEngine { error_class_ptrs, panic_class_ptrs, prof_enabled, + prof_activated: AtomicBool::new(false), }) } + /// Activate this engine's profiling lifecycle: register the `.bamlprof` + /// header metadata with the profiling consumer. Idempotent; a no-op when + /// the `BAML_PROFILE` master switch is off. + /// + /// The .bamlprof header consumes the M0 metadata table (its ids match + /// the ids stamped on each Function during construction — same walk + /// order, same 1-based sequence). + pub fn activate_profiling(&self) { + if !self.prof_enabled { + return; + } + if self + .prof_activated + .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) + .is_ok() + { + bex_events::prof::register_engine_metadata( + self.engine_id.0, + prof_engine_metadata(&self.program_metadata), + ); + } + } + #[must_use] pub fn process_euid(&self) -> ProcessEuid { self.process_euid diff --git a/baml_language/crates/bex_project/Cargo.toml b/baml_language/crates/bex_project/Cargo.toml index 3de69e3b288..6ab7400e424 100644 --- a/baml_language/crates/bex_project/Cargo.toml +++ b/baml_language/crates/bex_project/Cargo.toml @@ -53,6 +53,10 @@ vfs = { workspace = true } ignore = { workspace = true } tokio = { workspace = true, features = [ "sync", "rt", "time" ] } +[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] +sys_native = { workspace = true } +walkdir = { workspace = true } + [target.'cfg(target_arch = "wasm32")'.dependencies] wasm-bindgen-futures = { workspace = true } diff --git a/baml_language/crates/bex_project/src/bex_lsp/mod.rs b/baml_language/crates/bex_project/src/bex_lsp/mod.rs index c56c6015799..cf5c2d217fb 100644 --- a/baml_language/crates/bex_project/src/bex_lsp/mod.rs +++ b/baml_language/crates/bex_project/src/bex_lsp/mod.rs @@ -5,6 +5,7 @@ pub(crate) mod notification; mod request; mod multi_project; +mod position_codec; mod protocol; use std::{path::PathBuf, sync::Arc}; @@ -40,7 +41,9 @@ pub enum LspError { #[error("Project not found: {}", .0.as_str())] ProjectNotFound(vfs::VfsPath), - #[error("Unknown error code: {0}")] + /// Legacy constructor kept for the WASM bridge; serialized as + /// `InternalError`, never as `-32001`. + #[error("{0}")] UnknownErrorCode(String), #[error("Invalid command arguments for command: {command}: {message}")] @@ -60,6 +63,81 @@ pub enum LspError { #[error("No projects found")] NoProjectsFound, + + /// The request's view became stale: sources were mutated while it waited + /// (LSP `ContentModified`, `-32801`). + #[error("Content modified: {0}")] + ContentModified(String), + + /// Syntactically valid request that cannot be served right now: + /// same-revision busy timeout, overload, or unavailable target + /// (LSP `RequestFailed`, `-32803`). + #[error("{0}")] + RequestFailed(String), + + /// Violated invariant, poisoned state, or other internal failure + /// (LSP `InternalError`, `-32603`). + #[error("Internal error: {0}")] + Internal(String), + + /// Malformed params, position, or range (LSP `InvalidParams`, `-32602`). + #[error("Invalid params: {0}")] + InvalidParams(String), + + /// Request arrived before `initialize` completed + /// (LSP `ServerNotInitialized`, `-32002`). + #[error("Server not initialized: {0}")] + ServerNotInitialized(String), +} + +impl LspError { + /// The one LSP error-code mapping (I7). Every request error is serialized + /// through this table; `-32001 UnknownErrorCode` is never emitted. + #[must_use] + pub fn to_response_error(&self) -> lsp_server::ResponseError { + use lsp_server::ErrorCode; + let code = match self { + // Malformed params / positions / ranges. + LspError::NotificationExtractError(_) + | LspError::RequestExtractError(_) + | LspError::InvalidCommandArguments { .. } + | LspError::InvalidParams(_) => ErrorCode::InvalidParams, + + // Unsupported method. + LspError::NotificationNotSupported(_) | LspError::RequestNotSupported(_) => { + ErrorCode::MethodNotFound + } + + // Request before initialize completed. + LspError::ServerNotInitialized(_) => ErrorCode::ServerNotInitialized, + + // The request's view became stale under an applied source change. + LspError::ContentModified(_) => ErrorCode::ContentModified, + + // Valid request that cannot be served: unknown project/file, + // same-revision busy timeout, overload. + LspError::ProjectRootNotFound(..) + | LspError::ProjectNotFound(_) + | LspError::FileNotFound(_) + | LspError::InvalidPath { .. } + | LspError::InvalidVFSPath { .. } + | LspError::NoProjectsFound + | LspError::RequestFailed(_) => ErrorCode::RequestFailed, + + // Internal failures: poison, violated invariants, serialization, + // runtime errors, and the legacy catch-all constructor. + LspError::RequestSerializeError(_) + | LspError::Runtime(_) + | LspError::ClientClosed + | LspError::UnknownErrorCode(_) + | LspError::Internal(_) => ErrorCode::InternalError, + }; + lsp_server::ResponseError { + code: code as i32, + message: self.to_string(), + data: None, + } + } } // --------------------------------------------------------------------------- @@ -209,6 +287,15 @@ pub trait PlaygroundSender: Send + Sync { fn send_playground_notification(&self, notification: PlaygroundNotification); } +/// A coherent run-launch snapshot (I5): engine and generation captured in one +/// source→runtime transaction, with the overlay control-flow graph already +/// pinned for that generation. Holding `engine` keeps the launched-on engine +/// alive for the run's duration, so cancel can always target it (D8). +pub struct PreparedRun { + pub generation: u64, + pub engine: Arc, +} + // --------------------------------------------------------------------------- // BexLsp trait // --------------------------------------------------------------------------- @@ -222,6 +309,27 @@ pub trait BexLsp: Send + Sync + notification::BexLspNotification + request::BexL project_root: &crate::fs::FsPath, ) -> Result, crate::RuntimeError>; + /// Capture a coherent run-launch snapshot (I5): validates that the + /// installed engine matches current sources, and pins the overlay + /// control-flow graph for `overlay_function` under the same transaction. + /// + /// Errors are typed for the playground boundary: `ContentModified` when a + /// rebuild is needed first, `RequestFailed` when the project is busy. + fn prepare_function_run( + &self, + project_root: &str, + overlay_function: Option<&str>, + ) -> Result; + + /// The engine a run launched on, looked up by its pinned generation + /// (D8 cancel targeting). `None` once that generation has been replaced + /// and released. + fn engine_for_generation( + &self, + project_root: &str, + generation: u64, + ) -> Option>; + fn request_playground_state(&self); /// Seed workspace roots when the LSP is launched without an editor client @@ -319,3 +427,102 @@ pub trait BexLsp: Send + Sync + notification::BexLspNotification + request::BexL } pub use multi_project::{BackgroundSpawner, LspClientSenderTrait, new_lsp}; + +#[cfg(test)] +mod tests { + use super::*; + + /// The retired catch-all code (pre-0.14.2 `UnknownErrorCode`). No error + /// may serialize to it (design I7). + const LEGACY_UNKNOWN: i32 = -32001; + + fn code(e: &LspError) -> i32 { + e.to_response_error().code + } + + #[test] + fn typed_errors_map_to_spec_codes() { + use lsp_server::ErrorCode; + + assert_eq!( + code(&LspError::ContentModified("edit raced".into())), + ErrorCode::ContentModified as i32, + ); + assert_eq!( + code(&LspError::RequestFailed("busy".into())), + ErrorCode::RequestFailed as i32, + ); + assert_eq!( + code(&LspError::Internal("poisoned".into())), + ErrorCode::InternalError as i32, + ); + assert_eq!( + code(&LspError::InvalidParams("bad position".into())), + ErrorCode::InvalidParams as i32, + ); + assert_eq!( + code(&LspError::ServerNotInitialized("early".into())), + ErrorCode::ServerNotInitialized as i32, + ); + assert_eq!( + code(&LspError::RequestNotSupported("x/y".into())), + ErrorCode::MethodNotFound as i32, + ); + assert_eq!( + code(&LspError::NotificationNotSupported("x/y".into())), + ErrorCode::MethodNotFound as i32, + ); + } + + #[test] + fn resource_lookup_failures_are_request_failed() { + use lsp_server::ErrorCode; + let root = vfs::VfsPath::new(vfs::MemoryFS::new()); + for e in [ + LspError::ProjectRootNotFound(root.clone(), "not a project".into()), + LspError::ProjectNotFound(root.clone()), + LspError::FileNotFound(root), + LspError::NoProjectsFound, + ] { + assert_eq!(code(&e), ErrorCode::RequestFailed as i32, "{e}"); + } + } + + #[test] + fn legacy_unknown_code_is_never_emitted() { + let root = vfs::VfsPath::new(vfs::MemoryFS::new()); + let all = [ + LspError::NotificationNotSupported("m".into()), + LspError::RequestNotSupported("m".into()), + LspError::RequestSerializeError(serde_json::from_str::<()>("x").unwrap_err()), + LspError::Runtime(crate::RuntimeError::Other("x".into())), + LspError::ClientClosed, + LspError::ProjectRootNotFound(root.clone(), "x".into()), + LspError::ProjectNotFound(root.clone()), + // The legacy constructor itself now serializes as InternalError. + LspError::UnknownErrorCode("legacy".into()), + LspError::InvalidCommandArguments { + command: "c".into(), + message: "m".into(), + }, + LspError::FileNotFound(root), + LspError::InvalidPath { + path: std::path::PathBuf::from("/x"), + message: "m".into(), + }, + LspError::NoProjectsFound, + LspError::ContentModified("m".into()), + LspError::RequestFailed("m".into()), + LspError::Internal("m".into()), + LspError::InvalidParams("m".into()), + LspError::ServerNotInitialized("m".into()), + ]; + for e in &all { + assert_ne!(code(e), LEGACY_UNKNOWN, "{e} must not emit -32001"); + } + assert_eq!( + code(&LspError::UnknownErrorCode("legacy".into())), + lsp_server::ErrorCode::InternalError as i32, + ); + } +} diff --git a/baml_language/crates/bex_project/src/bex_lsp/multi_project/diagnostics.rs b/baml_language/crates/bex_project/src/bex_lsp/multi_project/diagnostics.rs index fd3040f9728..68d30918b6e 100644 --- a/baml_language/crates/bex_project/src/bex_lsp/multi_project/diagnostics.rs +++ b/baml_language/crates/bex_project/src/bex_lsp/multi_project/diagnostics.rs @@ -1,238 +1,104 @@ +//! Conversion of owned [`DiagnosticCandidate`]s into publishable LSP +//! diagnostics (design I2/I6/0B). +//! +//! Diagnostics are computed into fully owned, revision-tagged candidates +//! under the source gate ([`crate::project::collect_diagnostic_candidate`]); +//! this module converts them with the negotiated position codec at publish +//! time, with no database access. The candidate carries every file's text so +//! related-information spans in other files convert without a lock. + use std::collections::HashMap; use baml_base::FileId; -use crate::bex_lsp::multi_project::wasm_helpers; - -/// Configuration for LSP diagnostic conversion. -struct LspConversionConfig<'a> { - /// Maps `FileId` to file path for URL generation. - pub file_paths: &'a HashMap, - /// Maps `FileId` to (`source_text`, `line_starts`) for range conversion. - pub file_sources: &'a HashMap)>, +use crate::{ + bex_lsp::{ + multi_project::wasm_helpers, + position_codec::{PositionCodec, PositionEncoding}, + }, + project::DiagnosticCandidate, +}; + +/// One publish-ready document: the exact open-document version the text was +/// checked at (`None` for closed/disk-only files) plus converted diagnostics. +/// Every source file is represented, so stale editor markers clear. +pub(super) struct PublishableDocument { + pub path: std::path::PathBuf, + pub version: Option, + pub diagnostics: Vec, } -/// A convenient enumeration for supported text encodings. Can be converted to [`lsp_types::PositionEncodingKind`]. -// Please maintain the order from least to greatest priority for the derived `Ord` impl. -#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] -pub(super) enum PositionEncoding { - /// UTF 16 is the encoding supported by all LSP clients. - #[default] - UTF16, - - /// Second choice because UTF32 uses a fixed 4 byte encoding for each character (makes conversion relatively easy) - #[allow(dead_code)] - UTF32, - - /// BAML's preferred encoding - UTF8, +/// Convert an owned candidate into publish-ready documents using the +/// negotiated encoding. Pure conversion: no locks, no database. +pub(super) fn candidate_to_publishable( + candidate: &DiagnosticCandidate, + encoding: PositionEncoding, +) -> Vec { + let codecs: HashMap> = candidate + .file_texts + .iter() + .map(|(file_id, (_, text))| (*file_id, PositionCodec::new(text, encoding))) + .collect(); + let paths: HashMap = candidate + .file_texts + .iter() + .map(|(file_id, (path, _))| (*file_id, path)) + .collect(); + + candidate + .documents + .iter() + .map(|doc| PublishableDocument { + path: doc.path.clone(), + version: doc.version, + diagnostics: doc + .diagnostics + .iter() + .filter_map(|d| to_lsp_diagnostic(d, &codecs, &paths)) + .collect(), + }) + .collect() } fn to_lsp_diagnostic( - diagnostic: baml_compiler_diagnostics::Diagnostic, - config: &LspConversionConfig, - encoding: PositionEncoding, + diagnostic: &baml_compiler_diagnostics::Diagnostic, + codecs: &HashMap>, + paths: &HashMap, ) -> Option { let primary_span = diagnostic.primary_span()?; - let (source_text, line_starts) = config.file_sources.get(&primary_span.file_id)?; + let codec = codecs.get(&primary_span.file_id)?; - let diagnostic = lsp_types::Diagnostic { + Some(lsp_types::Diagnostic { severity: Some(match diagnostic.severity { baml_compiler_diagnostics::Severity::Error => lsp_types::DiagnosticSeverity::ERROR, baml_compiler_diagnostics::Severity::Warning => lsp_types::DiagnosticSeverity::WARNING, baml_compiler_diagnostics::Severity::Info => lsp_types::DiagnosticSeverity::INFORMATION, }), - range: span_to_lsp_range(primary_span.range, source_text, line_starts, encoding), + range: codec.byte_range_to_lsp(primary_span.range), code: Some(lsp_types::NumberOrString::String( diagnostic.code().to_string(), )), - message: diagnostic.message, + message: diagnostic.message.clone(), code_description: None, source: Some("baml".to_string()), related_information: Some( diagnostic .related_info - .into_iter() + .iter() .filter_map(|r| { - let path = config.file_paths.get(&r.span.file_id)?; - let (source_text, line_starts) = config.file_sources.get(&r.span.file_id)?; - let range = span_to_lsp_range(r.span.range, source_text, line_starts, encoding); + let path = paths.get(&r.span.file_id)?; + let codec = codecs.get(&r.span.file_id)?; Some(lsp_types::DiagnosticRelatedInformation { location: lsp_types::Location { uri: wasm_helpers::from_file_path(path).ok()?, - range, + range: codec.byte_range_to_lsp(r.span.range), }, - message: r.message, + message: r.message.clone(), }) }) .collect(), ), tags: None, data: None, - }; - - Some(diagnostic) -} - -/// Convert a `TextRange` to an LSP Range. -pub(super) fn span_to_lsp_range( - range: text_size::TextRange, - source_text: &str, - line_starts: &[u32], - encoding: PositionEncoding, -) -> lsp_types::Range { - let start_offset: u32 = range.start().into(); - let end_offset: u32 = range.end().into(); - - let start = offset_to_position(start_offset, source_text, line_starts, encoding); - let end = offset_to_position(end_offset, source_text, line_starts, encoding); - - lsp_types::Range { start, end } -} - -/// Convert a byte offset to an LSP Position. -/// -/// The character position is computed based on the position encoding negotiated -/// with the LSP client: -/// - UTF-8: character offset is byte offset from line start -/// - UTF-16: character offset is UTF-16 code unit count from line start -/// - UTF-32: character offset is character (codepoint) count from line start -fn offset_to_position( - offset: u32, - source_text: &str, - line_starts: &[u32], - encoding: PositionEncoding, -) -> lsp_types::Position { - // Binary search for the line containing this offset - let line = match line_starts.binary_search(&offset) { - Ok(line) => line, - Err(line) => line.saturating_sub(1), - }; - - let line_start = line_starts.get(line).copied().unwrap_or(0); - - // Calculate character position based on encoding - let character = match encoding { - PositionEncoding::UTF8 => { - // UTF-8 encoding: character offset equals byte offset - offset.saturating_sub(line_start) - } - PositionEncoding::UTF16 => { - // UTF-16 encoding: count UTF-16 code units - let line_start_usize = line_start as usize; - let offset_usize = offset as usize; - if offset_usize <= line_start_usize || offset_usize > source_text.len() { - 0 - } else { - // Get the slice from line start to offset - let slice = &source_text[line_start_usize..offset_usize.min(source_text.len())]; - // Count UTF-16 code units - u32::try_from(slice.encode_utf16().count()).unwrap_or(0) - } - } - PositionEncoding::UTF32 => { - // UTF-32 encoding: count Unicode codepoints (characters) - let line_start_usize = line_start as usize; - let offset_usize = offset as usize; - if offset_usize <= line_start_usize || offset_usize > source_text.len() { - 0 - } else { - // Get the slice from line start to offset - let slice = &source_text[line_start_usize..offset_usize.min(source_text.len())]; - // Count characters - u32::try_from(slice.chars().count()).unwrap_or(0) - } - } - }; - - lsp_types::Position { - line: u32::try_from(line).unwrap_or(0), - character, - } -} - -pub(super) trait WithDiagnostics { - /// The position encoding negotiated with the LSP client. - /// This is essential for correct character position calculation in files - /// containing multi-byte UTF-8 characters (like 'é' or emoji). - fn diagnostics_by_file( - &self, - position_encoding: PositionEncoding, - ) -> std::collections::HashMap>; -} - -impl WithDiagnostics for crate::project::BexProject { - /// Collect diagnostics for all files in the project (compiler2 only). - fn diagnostics_by_file( - &self, - position_encoding: PositionEncoding, - ) -> std::collections::HashMap> { - let Ok(db) = self.db.try_lock() else { - log::warn!("diagnostics_by_file: db mutex already locked, skipping"); - return HashMap::new(); - }; - - let source_files = db.get_source_files(); - - let mut file_sources: HashMap)> = HashMap::new(); - let mut file_paths: HashMap = HashMap::new(); - let mut diags_by_file: Vec<( - std::path::PathBuf, - Vec, - )> = Vec::new(); - - for file in &source_files { - let file_id = file.file_id(&*db); - let Some(path) = db.file_id_to_path(file_id).cloned() else { - continue; - }; - - let text = file.text(&*db).clone(); - let line_starts = compute_line_starts(&text); - file_sources.insert(file_id, (text, line_starts)); - file_paths.insert(file_id, path.clone()); - - let diags = baml_lsp2_actions::check_file(&*db, *file); - diags_by_file.push((path, diags)); - } - - let config = LspConversionConfig { - file_paths: &file_paths, - file_sources: &file_sources, - }; - - // Seed every known file with an empty vec so cleared diagnostics - // get an empty publish (removing stale markers). - let mut grouped: HashMap> = file_paths - .values() - .map(|p| (p.clone(), Vec::new())) - .collect(); - - for (path, diags) in diags_by_file { - for diag in diags { - if let Some(lsp_diag) = to_lsp_diagnostic(diag, &config, position_encoding) { - grouped.entry(path.clone()).or_default().push(lsp_diag); - } - } - } - - grouped - } -} - -/// Build line starts for a source file. -/// -/// Returns byte offsets of each line start. Uses `char_indices()` to get -/// byte positions rather than character indices, which is essential for -/// files containing multi-byte UTF-8 characters. -pub(super) fn compute_line_starts(source: &str) -> Vec { - let mut line_starts = vec![0]; - for (byte_offset, c) in source.char_indices() { - if c == '\n' { - // The next line starts at byte_offset + 1 - // (newline '\n' is always 1 byte in UTF-8) - line_starts.push(u32::try_from(byte_offset + 1).unwrap_or(0)); - } - } - line_starts + }) } diff --git a/baml_language/crates/bex_project/src/bex_lsp/multi_project/mod.rs b/baml_language/crates/bex_project/src/bex_lsp/multi_project/mod.rs index 23a61a1a1d7..11d1df3535a 100644 --- a/baml_language/crates/bex_project/src/bex_lsp/multi_project/mod.rs +++ b/baml_language/crates/bex_project/src/bex_lsp/multi_project/mod.rs @@ -1,12 +1,39 @@ +//! Multi-project LSP server core. +//! +//! Implements the refresh → diagnostics → rebuild pipeline from +//! `docs/design/lsp-latency-and-mutex-fix.md`: +//! +//! - **Refresh (I1):** every editor/watcher/playground event becomes one +//! [`crate::project::SourceBatch`] applied atomically with document +//! versions; the source revision advances with every applied batch. +//! - **Diagnostics (0B/I6):** each project owns a latest-revision dirty +//! fence. Publication converts an owned revision-tagged candidate; `Busy` +//! retains the last publication and schedules a trailing retry; stale +//! candidates are discarded; poison surfaces as an internal failure and +//! stops retrying. +//! - **Engine rebuild (0A/B3):** the debounce epoch is a pre-work ticket +//! only. The rebuild itself is single-flight per project and installs +//! through [`crate::project::BexProject::commit_engine_if_current`] — a +//! superseded candidate changes nothing. +//! - **Test collection/expansion (0A.7/0A.8):** collection runs under an +//! atomically captured ticket and installs through an ABA fence keyed by +//! engine generation + collection epoch; expansions serialize on the +//! installed registry's mutation gate. Stale results emit nothing. +//! - **Typed errors (I7):** `send_response` is the one place request errors +//! become wire codes; `-32001` is never emitted. + mod commands; mod diagnostics; mod notification; mod request; mod wasm_helpers; -use std::{collections::HashMap, io::Read}; +use std::{ + collections::{HashMap, HashSet}, + io::Read, + sync::{Arc, Mutex, OnceLock}, +}; -use ::std::sync::Arc; use baml_workspace::{BAML_SRC_DIR, BAML_TOML, find_baml_project_root_from_ancestors}; pub use wasm_helpers::BackgroundSpawner; @@ -18,24 +45,133 @@ use crate::{ RuntimeError, bex_lsp::{ LspError, - multi_project::diagnostics::{PositionEncoding, WithDiagnostics}, + multi_project::diagnostics::{PublishableDocument, candidate_to_publishable}, + position_codec::PositionEncoding, + }, + project::{ + BexProject, DbReadError, DiagnosticCandidate, EngineBuildOutcome, PrepareRunError, + RegistryLeaseError, SourceBatch, SourceGuard, SourceRevision, }, }; +/// Debounce window for the diagnostics/project-update tail (B3). +#[cfg(not(target_arch = "wasm32"))] +const DIAGNOSTICS_DEBOUNCE: std::time::Duration = std::time::Duration::from_millis(150); + +/// Debounce window for the heavy engine tail (bytecode + `$init`). +#[cfg(not(target_arch = "wasm32"))] +const ENGINE_REBUILD_DEBOUNCE: std::time::Duration = std::time::Duration::from_millis(300); + +/// Backoff before retrying a `Busy` diagnostics read (0B). +#[cfg(not(target_arch = "wasm32"))] +const DIAGNOSTICS_BUSY_RETRY: std::time::Duration = std::time::Duration::from_millis(40); + +/// Text + version of one open editor document. Version and text travel +/// together so publications carry the exact checked document version (I1). +struct OverlayDocument { + text: String, + /// `Some` for editor-owned documents; `None` for playground edits. + version: Option, +} + +/// Latest-revision diagnostics publication fence (0B). +/// +/// ```text +/// source mutation → mark latest revision dirty; schedule attempt +/// Ready(current revision) → conditionally publish; compare-and-clear +/// Ready(stale revision) → discard; retain latest dirty +/// Busy → publish nothing; trailing retry with backoff +/// Poisoned → publish nothing; surface internal failure +/// ``` +/// +/// Staleness is decided against the authoritative source revision (I1), not +/// only the dirty mark: an unsolicited candidate (e.g. a superseded +/// rebuild's) and a candidate racing `mark_dirty` are both discarded once a +/// newer revision exists, so they can never regress newer markers. +#[derive(Default)] +struct DiagnosticsFence { + /// Newest revision requiring publication. Compare-and-clear only. + dirty: Option, + /// Files covered by the last successful publication, so deleted files + /// get one empty publish. Not updated by busy/stale attempts. + last_published: HashSet, + /// A trailing busy-retry is already scheduled. Only the native scheduler + /// can observe `Busy`; the WASM tail is synchronous and single-threaded. + #[cfg(not(target_arch = "wasm32"))] + retry_scheduled: bool, +} + +impl DiagnosticsFence { + /// A mutation happened: the newest revision needs publication. + fn mark_dirty(&mut self, revision: SourceRevision) { + self.dirty = Some(self.dirty.map_or(revision, |d| d.max(revision))); + } + + /// Decide a computed candidate's fate. `true` compare-and-clears the + /// dirty revision and admits publication; `false` discards a stale + /// candidate — a newer mutation owns the next publication, whether or + /// not its dirty mark has landed yet (`refresh_project` always + /// schedules a tail for it). + fn admit( + &mut self, + candidate_revision: SourceRevision, + current_revision: SourceRevision, + ) -> bool { + if candidate_revision < current_revision { + return false; + } + if let Some(dirty) = self.dirty { + if candidate_revision < dirty { + return false; + } + self.dirty = None; + } + true + } + + /// Record a successful publication's file coverage. Returns the files + /// covered by the previous publication but absent now — each needs one + /// empty publish so stale editor markers clear (0B). + fn record_publication( + &mut self, + current: HashSet, + ) -> Vec { + let deleted = self.last_published.difference(¤t).cloned().collect(); + self.last_published = current; + deleted + } +} + struct LiveProject { - project: crate::project::BexProject, - in_memory_changes: - std::sync::Arc>>, - /// Tracks file paths for which we last published diagnostics, so we can - /// send an empty publish for files that disappear (deleted) on the next - /// full refresh. - last_published_files: - std::sync::Arc>>, - /// Debounce epoch for scheduled engine rebuilds: every refresh bumps it, - /// and a scheduled rebuild only runs if its captured epoch is still - /// current after the debounce delay. + project: BexProject, + in_memory_changes: Mutex>, + diagnostics_fence: Mutex, + /// Debounce tickets (pre-work suppression only — never authorization; + /// installation is guarded by the revision-conditional commit). + #[cfg(not(target_arch = "wasm32"))] + diagnostics_epoch: std::sync::atomic::AtomicU64, #[cfg(not(target_arch = "wasm32"))] rebuild_epoch: std::sync::atomic::AtomicU64, + /// Single-flight gate for engine rebuilds (0A.5): concurrent debounced + /// tails queue here instead of racing two `spawn_blocking` builds. + #[cfg(not(target_arch = "wasm32"))] + rebuild_gate: tokio::sync::Mutex<()>, +} + +impl LiveProject { + fn new(project: BexProject) -> Self { + Self { + project, + in_memory_changes: Mutex::new(HashMap::new()), + diagnostics_fence: Mutex::new(DiagnosticsFence::default()), + #[cfg(not(target_arch = "wasm32"))] + diagnostics_epoch: std::sync::atomic::AtomicU64::new(0), + #[cfg(not(target_arch = "wasm32"))] + rebuild_epoch: std::sync::atomic::AtomicU64::new(0), + #[cfg(not(target_arch = "wasm32"))] + rebuild_gate: tokio::sync::Mutex::new(()), + } + } } #[derive(Clone)] @@ -46,10 +182,10 @@ struct BexMulitProject { sender: std::sync::Arc, playground_sender: std::sync::Arc, - /// The position encoding negotiated with the LSP client. - /// This is essential for correct character position calculation in files - /// containing multi-byte UTF-8 characters (like 'é' or emoji). - position_encoding: PositionEncoding, + /// The position encoding negotiated during `initialize` (C1): UTF-8 when + /// the client offers it, otherwise UTF-16. Set exactly once; reading + /// before negotiation never freezes a default. + negotiated_encoding: std::sync::Arc>, /// Workspace root directories provided by the LSP client during /// `initialize`. Used by `on_notification_initialized` to scope @@ -65,29 +201,51 @@ struct BexMulitProject { pub trait LspClientSenderTrait { fn send_notification(&self, msg: lsp_server::Notification) -> Result<(), LspError>; fn send_response_impl(&self, msg: lsp_server::Response) -> Result<(), LspError>; + /// The one request error-code mapping boundary (I7). Every error is + /// serialized through [`LspError::to_response_error`]; `-32001` is dead. fn send_response( &self, id: lsp_server::RequestId, msg: Result, ) -> Result<(), LspError> { let (result, error) = match msg { - Err(error) => (None, Some(error)), + Err(error) => (None, Some(error.to_response_error())), Ok(result) => (Some(result), None), }; - let response = lsp_server::Response { - id, - result, - error: error.map(|e| lsp_server::ResponseError { - code: lsp_server::ErrorCode::UnknownErrorCode as i32, - message: e.to_string(), - data: None, - }), - }; + let response = lsp_server::Response { id, result, error }; self.send_response_impl(response) } fn make_request(&self, msg: lsp_server::Request) -> Result<(), LspError>; } +/// Map a bounded database read failure to its typed LSP error (Phase 0C): +/// timeout with a revision change is `ContentModified`, a same-revision +/// timeout is `RequestFailed`, poison is `InternalError`. +pub(super) fn db_read_error_to_lsp(e: DbReadError) -> LspError { + match e { + DbReadError::Busy { + revision_changed: true, + } => LspError::ContentModified( + "sources changed while the request waited for the project database".to_string(), + ), + DbReadError::Busy { + revision_changed: false, + } => LspError::RequestFailed( + "project database is busy; the request timed out waiting".to_string(), + ), + DbReadError::Broken => { + LspError::Internal("project is in a broken state (poisoned lock)".to_string()) + } + } +} + +/// Bounded request-lane read of a project's source gate. +pub(super) fn read_for_request(project: &BexProject) -> Result, LspError> { + project + .read_source_for_request() + .map_err(db_read_error_to_lsp) +} + enum ProjectRefreshMode { Full, /// Re-apply open-editor buffers. When `changed` is set, only that file is @@ -96,7 +254,9 @@ enum ProjectRefreshMode { InMemoryChangesOnly { changed: Option, }, - Only(Vec), + /// Documents were closed: re-apply their on-disk content and drop their + /// open-document versions in the same batch. + ClosedDocuments(Vec), } impl BexMulitProject { @@ -112,17 +272,56 @@ impl BexMulitProject { sys_op_factory, sender, playground_sender, - position_encoding: PositionEncoding::UTF8, + negotiated_encoding: std::sync::Arc::new(OnceLock::new()), workspace_roots: std::sync::Arc::new(std::sync::Mutex::new(Vec::new())), fs, spawner, } } + /// Encoding for request handlers: requests before `initialize` completes + /// are a protocol error and must not freeze a default (C1). + fn encoding_for_request(&self) -> Result { + self.negotiated_encoding.get().copied().ok_or_else(|| { + LspError::ServerNotInitialized( + "position encoding has not been negotiated yet".to_string(), + ) + }) + } + + /// Encoding for server-initiated publications. Pre-negotiation + /// publications (CLI-seeded workspaces) fall back to UTF-16 *without* + /// initializing the cell, so a later `initialize` still selects freely. + fn encoding_for_publication(&self) -> PositionEncoding { + match self.negotiated_encoding.get() { + Some(e) => *e, + None => { + log::debug!("publishing before encoding negotiation; using UTF-16 for this batch"); + PositionEncoding::UTF16 + } + } + } + + fn negotiate_encoding( + &self, + client_capabilities: &lsp_types::ClientCapabilities, + ) -> PositionEncoding { + let offered = client_capabilities + .general + .as_ref() + .and_then(|g| g.position_encodings.as_deref()); + let selected = PositionEncoding::negotiate(offered); + // First negotiation wins; a duplicate initialize cannot flip it. + let _ = self.negotiated_encoding.set(selected); + *self + .negotiated_encoding + .get() + .expect("negotiated encoding was just set") + } + fn get_path_from_uri(&self, uri: &lsp_types::Url) -> Result { - let path = wasm_helpers::to_file_path(uri).map_err(|()| { - LspError::UnknownErrorCode("Failed to convert URI to path".to_string()) - })?; + let path = wasm_helpers::to_file_path(uri) + .map_err(|()| LspError::InvalidParams(format!("URI is not a file path: {uri}")))?; self.fs.get_path_from_path(&path, "get_path_from_uri") } @@ -142,19 +341,19 @@ impl BexMulitProject { let sys_ops = (self.sys_op_factory)(&root_path); let project = crate::project::BexProject::new(&root_path, sys_ops); - let project = std::sync::Arc::new(LiveProject { - project, - in_memory_changes: std::sync::Arc::new(std::sync::Mutex::new(HashMap::new())), - last_published_files: std::sync::Arc::new(std::sync::Mutex::new( - std::collections::HashSet::new(), - )), - #[cfg(not(target_arch = "wasm32"))] - rebuild_epoch: std::sync::atomic::AtomicU64::new(0), - }); + let project = std::sync::Arc::new(LiveProject::new(project)); projects.insert(crate::fs::FsPath::from_vfs(&root_path), project.clone()); Ok(project) } + fn find_project(&self, project_root_str: &str) -> Option> { + let projects = self.projects.lock().unwrap(); + projects + .iter() + .find(|(k, _)| k.as_path().to_string_lossy() == project_root_str) + .map(|(_, v)| v.clone()) + } + fn get_bex_for_project( &self, project_root: &crate::fs::FsPath, @@ -270,10 +469,6 @@ impl BexMulitProject { Ok(files) } - fn refresh_project(&self, project_root: &vfs::VfsPath, refresh_mode: ProjectRefreshMode) { - self.refresh_project_async(project_root, refresh_mode); - } - fn has_baml_toml(path: &vfs::VfsPath) -> bool { path.join(BAML_TOML) .ok() @@ -471,12 +666,17 @@ impl BexMulitProject { } } - fn refresh_project_async(&self, project_root: &vfs::VfsPath, refresh_mode: ProjectRefreshMode) { + // ── Refresh pipeline ───────────────────────────────────────────────── + + /// Apply one editor/watcher/playground event as a source batch, then + /// schedule the diagnostics and engine tails. `didChange` cost is the + /// batch apply plus timer resets (B3); all computation is debounced. + fn refresh_project(&self, project_root: &vfs::VfsPath, refresh_mode: ProjectRefreshMode) { use crate::bex_lsp::notification::BexLspNotification; let mode_label = match &refresh_mode { ProjectRefreshMode::Full => "Full", ProjectRefreshMode::InMemoryChangesOnly { .. } => "InMemoryChangesOnly", - ProjectRefreshMode::Only(_) => "Only", + ProjectRefreshMode::ClosedDocuments(_) => "ClosedDocuments", }; tracing::debug!( "refresh_project({}, mode={})", @@ -488,171 +688,286 @@ impl BexMulitProject { return; }; - let is_full_refresh = matches!(refresh_mode, ProjectRefreshMode::Full); + let batch = match self.build_source_batch(project_root, &project, refresh_mode) { + Ok(batch) => batch, + Err(e) => { + let _ = self.send_notification_show_message(lsp_types::ShowMessageParams { + typ: lsp_types::MessageType::ERROR, + message: format!("Failed to read project files for {project_root:?}: {e}"), + }); + return; + } + }; + + let Ok(revision) = project.project.mutate_sources(batch) else { + log::error!("refresh_project: project is broken; dropping refresh"); + return; + }; + + // Catalog is never gated (B4): announce on every refresh. + self.send_list_projects(); + + project + .diagnostics_fence + .lock() + .unwrap() + .mark_dirty(revision); + + #[cfg(not(target_arch = "wasm32"))] + { + self.schedule_diagnostics_tail(project_root, &project); + self.schedule_engine_rebuild(project_root, &project); + } + + // WASM has no timers or blocking facilities wired here: run the + // whole tail synchronously (single-threaded, so never busy). + #[cfg(target_arch = "wasm32")] + self.run_project_tail_blocking(project_root, &project); + + tracing::debug!("refresh_project done"); + } + + /// Build the atomic source batch for a refresh mode: texts plus the + /// open-document version ops that must commit with them (I1). + fn build_source_batch( + &self, + project_root: &vfs::VfsPath, + project: &LiveProject, + refresh_mode: ProjectRefreshMode, + ) -> Result { match refresh_mode { ProjectRefreshMode::Full => { - tracing::debug!(" loading sources from disk..."); - let mut sources = match self.load_project_sources(project_root) { - Ok(sources) => sources, - Err(e) => { - let _ = self.send_notification_show_message(lsp_types::ShowMessageParams { - typ: lsp_types::MessageType::ERROR, - message: format!( - "Failed to read project files for {project_root:?}: {e}" - ), - }); - return; - } - }; - tracing::debug!(" loaded {} source file(s)", sources.len()); - + let mut sources = self.load_project_sources(project_root)?; + let mut versions = Vec::new(); { - let in_memory_changes = project.in_memory_changes.lock().unwrap(); - for (path, source) in in_memory_changes.iter() { - sources.insert(path.clone(), source.clone()); + let overlay = project.in_memory_changes.lock().unwrap(); + for (path, doc) in overlay.iter() { + sources.insert(path.clone(), doc.text.clone()); + versions.push((path.clone(), doc.version)); } } - - let project = &project.project; - - tracing::debug!(" apply_all_sources..."); - project.apply_all_sources(&sources); - tracing::debug!(" apply_all_sources done"); + Ok(SourceBatch { + replace_all: true, + sources, + versions, + }) } ProjectRefreshMode::InMemoryChangesOnly { changed } => { - let in_memory_changes = project.in_memory_changes.lock().unwrap(); + let overlay = project.in_memory_changes.lock().unwrap(); // A didChange names the one document that changed; re-applying // every open buffer would dirty their whole query chains. - let sources: HashMap<_, _> = match &changed { + let selected: Vec<(crate::fs::FsPath, &OverlayDocument)> = match &changed { Some(path) => { let key = crate::fs::FsPath::from_vfs(path); - in_memory_changes + overlay .get(&key) - .map(|source| (key, source.clone())) + .map(|doc| (key, doc)) .into_iter() .collect() } - None => in_memory_changes + None => overlay .iter() - .map(|(path, source)| (path.clone(), source.clone())) + .map(|(path, doc)| (path.clone(), doc)) .collect(), }; - drop(in_memory_changes); - - let project = &project.project; - project.apply_some_sources(&sources); + let mut sources = HashMap::new(); + let mut versions = Vec::new(); + for (path, doc) in selected { + sources.insert(path.clone(), doc.text.clone()); + versions.push((path, doc.version)); + } + Ok(SourceBatch { + replace_all: false, + sources, + versions, + }) } - ProjectRefreshMode::Only(paths) => { - // TODO: make this smarter and only read that the required files, instead of reading all files - let mut sources = match self.load_project_sources(project_root) { - Ok(sources) => sources, - Err(e) => { - let _ = self.send_notification_show_message(lsp_types::ShowMessageParams { - typ: lsp_types::MessageType::ERROR, - message: format!( - "Failed to read project files for {project_root:?}: {e}" - ), - }); - return; - } - }; - - { - let in_memory_changes = project.in_memory_changes.lock().unwrap(); - for (path, source) in in_memory_changes.iter() { - sources.insert(path.clone(), source.clone()); + ProjectRefreshMode::ClosedDocuments(paths) => { + let disk_sources = self.load_project_sources(project_root)?; + let mut sources = HashMap::new(); + let mut versions = Vec::new(); + for path in paths { + let key = crate::fs::FsPath::from_vfs(&path); + if let Some(text) = disk_sources.get(&key) { + sources.insert(key.clone(), text.clone()); } + versions.push((key, None)); } - - let sources = paths - .into_iter() - .filter_map(|path| { - let key = crate::fs::FsPath::from_vfs(&path); - sources.remove(&key).map(|source| (key, source)) - }) - .collect(); - - let project = &project.project; - project.apply_some_sources(&sources); + Ok(SourceBatch { + replace_all: false, + sources, + versions, + }) } } + } - // Diagnostics are computed straight from the database below; the - // heavy tail (bytecode + engine + test collection) is debounced onto - // a background task on native so fast typing costs one rebuild, not - // one per keystroke. WASM has no timer/blocking facilities wired - // here, so it keeps the synchronous rebuild. - #[cfg(target_arch = "wasm32")] - let _ = project.project.update_bex(); - - tracing::debug!(" computing diagnostics..."); - let diagnostics = project.project.diagnostics_by_file(self.position_encoding); - tracing::debug!(" diagnostics computed for {} file(s)", diagnostics.len()); + // ── Diagnostics tail (0B) ──────────────────────────────────────────── - // Always publish diagnostics for every file the compiler knows about - // (including files with 0 diagnostics, to clear stale markers). - let current_paths: std::collections::HashSet = diagnostics - .keys() - .map(|p| crate::fs::FsPath::from_str(p.to_string_lossy().into_owned())) - .collect(); + /// Debounced diagnostics/project-update tail. The epoch only suppresses + /// superseded timers; staleness of results is decided by the fence. + #[cfg(not(target_arch = "wasm32"))] + fn schedule_diagnostics_tail( + &self, + project_root: &vfs::VfsPath, + project: &std::sync::Arc, + ) { + use std::sync::atomic::Ordering; + let epoch = project.diagnostics_epoch.fetch_add(1, Ordering::SeqCst) + 1; + let this = self.clone(); + let project = project.clone(); + let project_root = project_root.clone(); + self.spawner.spawn(async move { + tokio::time::sleep(DIAGNOSTICS_DEBOUNCE).await; + if project.diagnostics_epoch.load(Ordering::SeqCst) != epoch { + return; + } + let compute_project = project.clone(); + let outcome = tokio::task::spawn_blocking(move || { + compute_project.project.diagnostic_candidate_nowait() + }) + .await + .unwrap_or(Ok(None)); + this.handle_diagnostics_outcome(&project_root, &project, outcome); - for path in ¤t_paths { - let file_diagnostics = diagnostics.get(path.as_path()).cloned().unwrap_or_default(); - let Ok(uri) = wasm_helpers::from_file_path(path.as_path()) else { - continue; - }; - let _ = self.send_notification_publish_diagnostics( - lsp_types::PublishDiagnosticsParams::new(uri, file_diagnostics, None), - ); - } + // The playground project snapshot rides the same debounced tail + // (B3 step 6): one owned payload per quiet period, not one per + // keystroke. + this.send_update_project(&project_root, &project); + }); + } - // On a full refresh, also clear diagnostics for files that no longer - // exist (deleted since the last refresh). - if is_full_refresh { - let mut prev = project.last_published_files.lock().unwrap(); - for deleted in prev.difference(¤t_paths) { - let Ok(uri) = wasm_helpers::from_file_path(deleted.as_path()) else { - continue; - }; - let _ = self.send_notification_publish_diagnostics( - lsp_types::PublishDiagnosticsParams::new(uri, vec![], None), + /// Apply one diagnostics computation outcome to the fence. + #[cfg(not(target_arch = "wasm32"))] + fn handle_diagnostics_outcome( + &self, + project_root: &vfs::VfsPath, + project: &std::sync::Arc, + outcome: Result, crate::project::ProjectBroken>, + ) { + match outcome { + Err(_) => { + // Poison is an internal failure, never an empty publication + // (I6). The project is terminally broken; do not retry. + log::error!( + "diagnostics: project {} is broken; keeping last publication", + project_root.as_str() ); } - *prev = current_paths; + Ok(None) => { + self.schedule_diagnostics_busy_retry(project_root, project); + } + Ok(Some(candidate)) => { + self.publish_candidate(&candidate, project); + } } + } - let flat_diags = Self::flatten_diagnostics(&diagnostics); + /// Busy: keep the last publication and re-arm one trailing retry (0B). + /// The retry is fence-tagged, so it publishes only if still relevant. + #[cfg(not(target_arch = "wasm32"))] + fn schedule_diagnostics_busy_retry( + &self, + project_root: &vfs::VfsPath, + project: &std::sync::Arc, + ) { + { + let mut fence = project.diagnostics_fence.lock().unwrap(); + if fence.dirty.is_none() || fence.retry_scheduled { + return; + } + fence.retry_scheduled = true; + } + let this = self.clone(); + let project = project.clone(); + let project_root = project_root.clone(); + self.spawner.spawn(async move { + tokio::time::sleep(DIAGNOSTICS_BUSY_RETRY).await; + { + let mut fence = project.diagnostics_fence.lock().unwrap(); + fence.retry_scheduled = false; + if fence.dirty.is_none() { + return; + } + } + let compute_project = project.clone(); + let outcome = tokio::task::spawn_blocking(move || { + compute_project.project.diagnostic_candidate_nowait() + }) + .await + .unwrap_or(Ok(None)); + this.handle_diagnostics_outcome(&project_root, &project, outcome); + }); + } - self.send_list_projects(); - self.send_update_project(project_root, &project, flat_diags); + /// Conditionally publish an owned candidate through the fence (0B): + /// stale candidates are discarded; publication compare-and-clears the + /// dirty revision, so a newer mutation that raced in stays scheduled. + fn publish_candidate( + &self, + candidate: &DiagnosticCandidate, + project: &std::sync::Arc, + ) { + use crate::bex_lsp::notification::BexLspNotification; - // Auto-trigger runtime test collection when BexEngine is ready - if project.project.is_bex_current() { - self.request_collect_tests_impl(project_root.as_str()); + let encoding = self.encoding_for_publication(); + let documents = candidate_to_publishable(candidate, encoding); + + // The current revision is read under the fence lock: publications + // serialize on it, so a candidate admitted here can never regress + // one already admitted for a newer revision. + let mut fence = project.diagnostics_fence.lock().unwrap(); + if !fence.admit( + candidate.source_revision, + project.project.current_revision(), + ) { + // A newer mutation owns the next publication; its tail is + // already scheduled. Publishing this would regress markers. + return; } - #[cfg(not(target_arch = "wasm32"))] - self.schedule_engine_rebuild(project_root, project); + let current_paths: HashSet = + documents.iter().map(|d| d.path.clone()).collect(); - tracing::debug!("refresh_project done"); + for doc in &documents { + let Ok(uri) = wasm_helpers::from_file_path(&doc.path) else { + continue; + }; + let _ = self.send_notification_publish_diagnostics( + lsp_types::PublishDiagnosticsParams::new(uri, doc.diagnostics.clone(), doc.version), + ); + } + + // Clear markers for files that disappeared since the last successful + // publication (busy/stale attempts never update this set). + for deleted in fence.record_publication(current_paths) { + let Ok(uri) = wasm_helpers::from_file_path(&deleted) else { + continue; + }; + let _ = self.send_notification_publish_diagnostics( + lsp_types::PublishDiagnosticsParams::new(uri, vec![], None), + ); + } } + // ── Engine tail (0A) ───────────────────────────────────────────────── + /// Debounced engine rebuild: bytecode generation, `BexEngine::new` (which /// executes `$init`), and test collection are the heavy tail of a refresh. - /// Running them per keystroke burned CPU and heap on engines that were - /// discarded milliseconds later, so they run on a background task after - /// the project has been quiet for the debounce window. + /// The debounce epoch is a pre-work ticket; the rebuild gate makes builds + /// single-flight; installation is authorized only by the + /// revision-conditional commit inside `rebuild_once` (I3). #[cfg(not(target_arch = "wasm32"))] fn schedule_engine_rebuild( &self, project_root: &vfs::VfsPath, - project: std::sync::Arc, + project: &std::sync::Arc, ) { use std::sync::atomic::Ordering; - const ENGINE_REBUILD_DEBOUNCE: std::time::Duration = std::time::Duration::from_millis(300); let epoch = project.rebuild_epoch.fetch_add(1, Ordering::SeqCst) + 1; let this = self.clone(); + let project = project.clone(); let project_root = project_root.clone(); self.spawner.spawn(async move { tokio::time::sleep(ENGINE_REBUILD_DEBOUNCE).await; @@ -660,36 +975,91 @@ impl BexMulitProject { // A newer refresh superseded this one; its own rebuild is scheduled. return; } + let _flight = project.rebuild_gate.lock().await; + if project.rebuild_epoch.load(Ordering::SeqCst) != epoch { + return; + } let rebuild_project = project.clone(); - let rebuilt = - tokio::task::spawn_blocking(move || rebuild_project.project.update_bex().is_ok()) - .await - .unwrap_or(false); - if !rebuilt || project.rebuild_epoch.load(Ordering::SeqCst) != epoch { + let Ok(report) = + tokio::task::spawn_blocking(move || rebuild_project.project.rebuild_once()).await + else { + log::error!("engine rebuild task panicked"); return; + }; + this.apply_rebuild_report(&project_root, &project, report); + }); + } + + /// Publish a rebuild's diagnostics through the fence and, on a winning + /// commit, announce the ready runtime state and collect tests. Superseded + /// candidates change nothing (I3). + fn apply_rebuild_report( + &self, + project_root: &vfs::VfsPath, + project: &std::sync::Arc, + report: crate::project::RebuildReport, + ) { + if let Some(candidate) = &report.diagnostics { + self.publish_candidate(candidate, project); + } + + match report.engine { + EngineBuildOutcome::Committed(receipt) => { + log::info!( + "engine rebuild: generation {} committed at {}", + receipt.generation, + receipt.source_revision + ); + self.send_update_project(project_root, project); + self.collect_tests_for_project(project_root.as_str(), project); } - // Re-announce the project now that the engine is current, and - // collect tests against it (mirrors the synchronous tail that ran - // per keystroke before debouncing). - let diagnostics = project.project.diagnostics_by_file(this.position_encoding); - let flat_diags = Self::flatten_diagnostics(&diagnostics); - this.send_update_project(&project_root, &project, flat_diags); - if project.project.is_bex_current() { - this.request_collect_tests_impl(project_root.as_str()); + EngineBuildOutcome::BlockedByDiagnostics { source_revision } => { + log::info!("engine rebuild: blocked by diagnostics at {source_revision}"); + self.send_update_project(project_root, project); } - }); + EngineBuildOutcome::Failed { + source_revision, + message, + } => { + log::warn!("engine rebuild failed at {source_revision}: {message}"); + self.send_update_project(project_root, project); + } + EngineBuildOutcome::Superseded { current_revision } => { + log::info!( + "engine rebuild superseded by {current_revision}; candidate dropped quietly" + ); + } + EngineBuildOutcome::Broken => { + log::error!("engine rebuild: project is broken"); + } + } } + /// Synchronous project tail for WASM (no timers/blocking facilities): + /// rebuild, publish diagnostics, announce, and collect. + #[cfg(target_arch = "wasm32")] + fn run_project_tail_blocking( + &self, + project_root: &vfs::VfsPath, + project: &std::sync::Arc, + ) { + let report = project.project.rebuild_once(); + self.apply_rebuild_report(project_root, project, report); + } + + // ── Playground state ───────────────────────────────────────────────── + fn flatten_diagnostics( - diagnostics: &std::collections::HashMap>, + documents: &[PublishableDocument], ) -> Vec { let mut out = Vec::new(); - for (path, diags) in diagnostics { - let filename = path + for doc in documents { + let filename = doc + .path .file_name() .map(|f| f.to_string_lossy().into_owned()) .unwrap_or_default(); - for d in diags { + for d in &doc.diagnostics { let severity = match d.severity { Some(lsp_types::DiagnosticSeverity::ERROR) => "error", Some(lsp_types::DiagnosticSeverity::WARNING) => "warning", @@ -706,15 +1076,21 @@ impl BexMulitProject { out } - fn build_project_update( - project: &LiveProject, - diagnostics: Vec, - ) -> crate::bex_lsp::ProjectUpdate { + /// Build one owned, complete `ProjectUpdate` payload with a bounded + /// source read. `None` means the project was busy or broken — the caller + /// keeps the last announced state rather than sending a wiped payload. + fn build_project_update(&self, project: &LiveProject) -> Option { let is_bex_current = project.project.is_bex_current(); - let db_guard = project.project.db.lock().unwrap(); - let db = db_guard.db(); - let listing = baml_project::list_functions_with_metadata(db); + let guard = project.project.read_source_for_request().ok()?; + let candidate = crate::project::collect_diagnostic_candidate(&guard); + let listing = baml_project::list_functions_with_metadata(guard.db()); + let tests_listing = baml_project::list_tests_with_metadata(guard.db()); + drop(guard); + + let documents = candidate_to_publishable(&candidate, self.encoding_for_publication()); + let diagnostics = Self::flatten_diagnostics(&documents); + let functions = listing .functions .into_iter() @@ -738,7 +1114,7 @@ impl BexMulitProject { params: f.params, }) .collect(); - let tests = baml_project::list_tests_with_metadata(db) + let tests = tests_listing .into_iter() .map(|test| crate::bex_lsp::TestInfo { name: test.name, @@ -747,13 +1123,13 @@ impl BexMulitProject { }) .collect(); - crate::bex_lsp::ProjectUpdate { + Some(crate::bex_lsp::ProjectUpdate { is_bex_current, functions, tests, types: Some(listing.types), diagnostics, - } + }) } fn send_list_projects(&self) { @@ -762,18 +1138,20 @@ impl BexMulitProject { .keys() .map(|p| p.as_path().to_string_lossy().into_owned()) .collect(); + drop(projects); self.playground_sender.send_playground_notification( crate::bex_lsp::PlaygroundNotification::ListProjects { projects: roots }, ); } - fn send_update_project( - &self, - project_root: &vfs::VfsPath, - project: &LiveProject, - diagnostics: Vec, - ) { - let update = Self::build_project_update(project, diagnostics); + fn send_update_project(&self, project_root: &vfs::VfsPath, project: &LiveProject) { + let Some(update) = self.build_project_update(project) else { + log::debug!( + "skipping UpdateProject for busy project {}", + project_root.as_str() + ); + return; + }; self.playground_sender.send_playground_notification( crate::bex_lsp::PlaygroundNotification::UpdateProject { project: project_root.as_str().to_string(), @@ -782,74 +1160,71 @@ impl BexMulitProject { ); } + // ── Test collection / runs (0A.7, 0A.8, I5) ────────────────────────── + fn request_collect_tests_impl(&self, project_root_str: &str) { - log::info!("[request_collect_tests_impl] project={project_root_str}"); - // Resolve project and get the concrete BexEngine (before trait erasure) - let (engine, test_state) = { - let projects = self.projects.lock().unwrap(); - let project = projects - .iter() - .find(|(k, _)| k.as_path().to_string_lossy() == project_root_str) - .map(|(_, v)| v.clone()); - let Some(project) = project else { - return; - }; - let Ok(engine) = project.project.get_bex() else { - return; - }; - (engine, project.project.test_state()) + let Some(project) = self.find_project(project_root_str) else { + return; }; + self.collect_tests_for_project(project_root_str, &project); + } - // Cancel in-flight collection tasks and clear stale registry. The - // generation tracks compiled project snapshots and is bumped only when a - // new BexEngine/CFG snapshot is installed. - let (generation, cancel) = { - let mut state = test_state.lock().unwrap(); - state.cancel.cancel(); - state.cancel = sys_types::CancellationToken::new(); - state.registry = None; - (state.generation, state.cancel.clone()) + /// Start one test-collection attempt against the installed engine. The + /// ticket captures engine, generation, cancel token, and collection + /// epoch atomically; installation and every emission are fenced by that + /// identity, so stale collections emit nothing (0A.7). + fn collect_tests_for_project( + &self, + project_root_str: &str, + project: &std::sync::Arc, + ) { + log::info!("[collect_tests] project={project_root_str}"); + let ticket = match project.project.begin_test_collection() { + Ok(Some(ticket)) => ticket, + Ok(None) => { + log::info!("[collect_tests] no current engine; skipping"); + return; + } + Err(_) => return, }; let sender = self.playground_sender.clone(); + let live = project.clone(); let project = project_root_str.to_string(); let package = "user".to_string(); let call_id = sys_types::CallId::next(); + let generation = ticket.generation; + let cancel = ticket.cancel.clone(); + let engine = ticket.engine.clone(); - // Spawn async collection task self.spawner.spawn(async move { match engine .collect_tests(&package, call_id, cancel.clone()) .await { Ok(registry) => { - // Discard stale results if the engine was swapped during collection. - // The guard is scoped to this block so it is dropped before the await below. - let should_continue = { - let mut state = test_state.lock().unwrap(); - if state.generation != generation { + // Extract Handle from BexExternalValue::Handle. + // Null means the project has no tests ($init_test absent). + let handle = match ®istry { + bex_engine::BexExternalValue::Handle(h) => Some(h.clone()), + bex_engine::BexExternalValue::Null => None, + _ => { + log::error!("[collect_tests] unexpected result type"); + return; + } + }; + + // ABA fence: install only if the engine generation and + // collection epoch still match; stale results emit nothing. + match live.project.install_collected_registry(&ticket, handle) { + Ok(true) => {} + Ok(false) => { log::info!( - "[collect_tests] discarding stale result (gen {generation} vs current {})", - state.generation + "[collect_tests] discarding stale result (gen {generation})" ); - false - } else { - // Extract Handle from BexExternalValue::Handle. - // Null means the project has no tests ($init_test absent). - let handle = match ®istry { - bex_engine::BexExternalValue::Handle(h) => Some(h.clone()), - bex_engine::BexExternalValue::Null => None, - _ => { - log::error!("[collect_tests] unexpected result type"); - return; - } - }; - state.registry = handle; - true + return; } - }; - if !should_continue { - return; + Err(_) => return, } // If the project has no tests, send an empty test tree. @@ -882,6 +1257,12 @@ impl BexMulitProject { .await { Ok(serialized) => { + // Emission is fenced too: if a newer engine or + // collection superseded us during serialize, stay + // silent (the newer attempt owns the tree). + if !live.project.collection_ticket_is_current(&ticket) { + return; + } let data = serde_json::to_vec(&bex_value_to_json(&serialized)) .unwrap_or_default(); sender.send_playground_notification( @@ -896,6 +1277,9 @@ impl BexMulitProject { } Err(e) => { log::error!("[collect_tests] serialize failed: {e}"); + if !live.project.collection_ticket_is_current(&ticket) { + return; + } sender.send_playground_notification( crate::bex_lsp::PlaygroundNotification::TestCollectionResult { project: project.clone(), @@ -910,8 +1294,13 @@ impl BexMulitProject { } } Err(e) => { - // Collection failed — notify the frontend with an empty result so it unblocks + // Stale/canceled collection emits nothing; a failure for + // the still-current build unblocks the frontend with an + // empty result. log::error!("[collect_tests] collect_tests failed: {e}"); + if !live.project.collection_ticket_is_current(&ticket) { + return; + } sender.send_playground_notification( crate::bex_lsp::PlaygroundNotification::TestCollectionResult { project, @@ -933,45 +1322,29 @@ impl BexMulitProject { test_name: &str, ctx: bex_engine::FunctionCallContext, ) -> Result { - let (engine, registry_value) = { - let projects = self.projects.lock().unwrap(); - let project = projects - .iter() - .find(|(k, _)| k.as_path().to_string_lossy() == project_root_str) - .map(|(_, v)| v.clone()) - .ok_or_else(|| bex_engine::EngineError::FunctionNotFound { - name: format!("project not found: {project_root_str}"), - })?; - let engine = project.project.get_bex().map_err(|e| { - bex_engine::EngineError::FunctionNotFound { - name: format!("engine not ready: {e}"), - } - })?; - let test_state = project.project.test_state(); - let state = test_state.lock().unwrap(); - if state.generation != generation { - return Err(bex_engine::EngineError::FunctionNotFound { - name: "stale generation".to_string(), - }); + let project = self.find_project(project_root_str).ok_or_else(|| { + bex_engine::EngineError::FunctionNotFound { + name: format!("project not found: {project_root_str}"), } - let registry_value = match &state.registry { - Some(handle) => bex_engine::BexExternalValue::Handle(handle.clone()), - None => { - return Err(bex_engine::EngineError::FunctionNotFound { - name: "no test registry".to_string(), - }); - } - }; - (engine, registry_value) - }; + })?; + + // One coherent lease (I5): validates the generation against the + // installed engine, requires current source (D7), and captures the + // engine + registry handle atomically. + let lease = project.project.lease_registry(generation).map_err(|e| { + bex_engine::EngineError::FunctionNotFound { + name: registry_lease_error_message(e).to_string(), + } + })?; log::info!("[call_test_function] test_name={test_name} generation={generation}"); - let result = engine + let result = lease + .engine .call_function_with_trace( "testing.TestRegistry.run_test", vec![ - registry_value, + bex_engine::BexExternalValue::Handle(lease.handle.clone()), bex_engine::BexExternalValue::String(test_name.into()), ], ctx, @@ -988,37 +1361,38 @@ impl BexMulitProject { } fn expand_test_set_impl(&self, project_root_str: &str, generation: u64, testset_name: &str) { - let (engine, registry_value, cancel) = { - let projects = self.projects.lock().unwrap(); - let Some(project) = projects - .iter() - .find(|(k, _)| k.as_path().to_string_lossy() == project_root_str) - .map(|(_, v)| v.clone()) - else { - return; - }; - let Ok(engine) = project.project.get_bex() else { - return; - }; - let test_state = project.project.test_state(); - let state = test_state.lock().unwrap(); - if state.generation != generation { + let Some(project) = self.find_project(project_root_str) else { + return; + }; + // Stale expansion requests emit nothing: the newer collection owns + // the tree (D8 cancels stale tree maintenance). + let lease = match project.project.lease_registry(generation) { + Ok(lease) => lease, + Err(e) => { + log::info!( + "[expand_test_set] not expanding '{testset_name}': {}", + registry_lease_error_message(e) + ); return; } - let registry_value = match &state.registry { - Some(handle) => bex_engine::BexExternalValue::Handle(handle.clone()), - None => return, - }; - let cancel = state.cancel.clone(); - (engine, registry_value, cancel) }; let call_id = sys_types::CallId::next(); let sender = self.playground_sender.clone(); + let live = project; let project = project_root_str.to_string(); let name = testset_name.to_string(); self.spawner.spawn(async move { + // One mutation owner per installed registry (0A.8): expansions + // mutate the registry heap object in place, so they serialize. + #[cfg(not(target_arch = "wasm32"))] + let _mutation_owner = lease.expansion_gate.lock().await; + + let engine = lease.engine.clone(); + let registry_value = bex_engine::BexExternalValue::Handle(lease.handle.clone()); + let cancel = lease.cancel.clone(); + let ctx = bex_engine::FunctionCallContextBuilder::new(call_id) .with_cancel_token(cancel.clone()) .with_profile_enabled(false) @@ -1039,6 +1413,10 @@ impl BexMulitProject { .await { log::error!("[expand_test_set] expand failed for testset '{name}': {e}"); + if cancel.is_cancelled() { + // Superseded mid-expansion: emit nothing. + return; + } // Re-serialize and send the current (pre-expansion) state so the // UI unblocks from the loading spinner instead of spinning forever. let ctx_resend = @@ -1065,6 +1443,9 @@ impl BexMulitProject { serde_json::to_vec(&serde_json::json!([])).unwrap_or_default() } }; + if !live.project.registry_lease_is_current(&lease) { + return; + } sender.send_playground_notification( crate::bex_lsp::PlaygroundNotification::TestCollectionResult { project, @@ -1096,6 +1477,10 @@ impl BexMulitProject { .await { Ok(serialized) => { + // Stale expansion success emits nothing (0A.7 fence). + if !live.project.registry_lease_is_current(&lease) { + return; + } let data = serde_json::to_vec(&bex_value_to_json(&serialized)).unwrap_or_default(); sender.send_playground_notification( @@ -1110,6 +1495,9 @@ impl BexMulitProject { } Err(e) => { log::error!("[expand_test_set] serialize after expanding '{name}' failed: {e}"); + if !live.project.registry_lease_is_current(&lease) { + return; + } // Send empty result so the UI unblocks sender.send_playground_notification( crate::bex_lsp::PlaygroundNotification::TestCollectionResult { @@ -1126,6 +1514,17 @@ impl BexMulitProject { } } +fn registry_lease_error_message(e: RegistryLeaseError) -> &'static str { + match e { + RegistryLeaseError::NeedsCurrentBuild => { + "engine is not current with the latest sources; wait for the rebuild" + } + RegistryLeaseError::NoRegistry => "no test registry collected yet", + RegistryLeaseError::NoTests => "project has no tests", + RegistryLeaseError::Broken => "project is in a broken state", + } +} + /// Convert a `BexExternalValue` to a `serde_json::Value` for serialization. /// /// Only handles the primitive/structural variants that appear in test reports. @@ -1275,13 +1674,59 @@ impl super::BexLsp for BexMulitProject { self.get_bex_for_project(project_root) } + fn prepare_function_run( + &self, + project_root: &str, + overlay_function: Option<&str>, + ) -> Result { + let project = self + .find_project(project_root) + .ok_or_else(|| LspError::RequestFailed(format!("project not found: {project_root}")))?; + let snapshot = project + .project + .prepare_function_run(overlay_function) + .map_err(|e| match e { + PrepareRunError::NeedsCurrentBuild => LspError::ContentModified( + "engine is not current with the latest sources; wait for the rebuild" + .to_string(), + ), + PrepareRunError::Busy => { + LspError::RequestFailed("project is busy; retry shortly".to_string()) + } + PrepareRunError::Broken => { + LspError::Internal("project is in a broken state".to_string()) + } + })?; + Ok(super::PreparedRun { + generation: snapshot.generation, + engine: snapshot.engine, + }) + } + + fn engine_for_generation( + &self, + project_root: &str, + generation: u64, + ) -> Option> { + let project = self.find_project(project_root)?; + project + .project + .engine_for_generation(generation) + .map(|engine| engine as Arc) + } + fn all_env_var_names(&self) -> Vec { - let projects = self.projects.lock().unwrap(); + let projects: Vec<_> = { + let projects = self.projects.lock().unwrap(); + projects.values().cloned().collect() + }; let mut names = std::collections::BTreeSet::new(); - for project in projects.values() { - let db_guard = project.project.db.lock().unwrap(); - let db = db_guard.db(); - for name in baml_lsp2_actions::all_env_var_names(db) { + for project in projects { + // Loop lane: skip busy projects instead of blocking dispatch. + let Ok(Some(guard)) = project.project.read_source_nowait() else { + continue; + }; + for name in baml_lsp2_actions::all_env_var_names(guard.db()) { names.insert(name); } } @@ -1299,8 +1744,8 @@ impl super::BexLsp for BexMulitProject { let mut sources = self.load_project_sources(&project_root)?; { let in_memory_changes = project_handle.in_memory_changes.lock().unwrap(); - for (path, source) in in_memory_changes.iter() { - sources.insert(path.clone(), source.clone()); + for (path, doc) in in_memory_changes.iter() { + sources.insert(path.clone(), doc.text.clone()); } } @@ -1346,7 +1791,15 @@ impl super::BexLsp for BexMulitProject { let project_handle = self.get_or_create_project(project_root.clone())?; let mut in_memory_changes = project_handle.in_memory_changes.lock().unwrap(); - in_memory_changes.insert(crate::fs::FsPath::from_vfs(&source_path), content); + // Playground edits are unversioned; a previously known editor version + // no longer describes this text. + in_memory_changes.insert( + crate::fs::FsPath::from_vfs(&source_path), + OverlayDocument { + text: content, + version: None, + }, + ); drop(in_memory_changes); self.refresh_project( @@ -1375,12 +1828,18 @@ impl super::BexLsp for BexMulitProject { fn request_playground_state(&self) { self.send_list_projects(); - let projects = self.projects.lock().unwrap(); - for (fs_path, project) in projects.iter() { + let projects: Vec<(crate::fs::FsPath, std::sync::Arc)> = { + let projects = self.projects.lock().unwrap(); + projects + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect() + }; + for (fs_path, project) in projects { let root_str = fs_path.as_path().to_string_lossy().into_owned(); - let diags_by_file = project.project.diagnostics_by_file(self.position_encoding); - let flat_diags = Self::flatten_diagnostics(&diags_by_file); - let update = Self::build_project_update(project, flat_diags); + let Some(update) = self.build_project_update(&project) else { + continue; + }; self.playground_sender.send_playground_notification( crate::bex_lsp::PlaygroundNotification::UpdateProject { project: root_str, @@ -1394,10 +1853,15 @@ impl super::BexLsp for BexMulitProject { &self, function_name: &str, ) -> Option { - let projects = self.projects.lock().ok()?; - for project in projects.values() { - let db = project.project.db.lock().ok()?; - if let Some(graph) = db.ast_control_flow_graph(function_name) { + let projects: Vec<_> = { + let projects = self.projects.lock().ok()?; + projects.values().cloned().collect() + }; + for project in projects { + let Ok(guard) = read_for_request(&project.project) else { + continue; + }; + if let Some(graph) = guard.db().ast_control_flow_graph(function_name) { return Some(graph); } } @@ -1405,11 +1869,8 @@ impl super::BexLsp for BexMulitProject { } fn project_generation(&self, project_root: &str) -> Option { - let projects = self.projects.lock().ok()?; - projects - .iter() - .find(|(path, _)| path.as_path().to_string_lossy() == project_root) - .map(|(_, project)| project.project.current_generation()) + let project = self.find_project(project_root)?; + Some(project.project.current_generation()) } fn control_flow_graph_for_generation( @@ -1419,15 +1880,9 @@ impl super::BexLsp for BexMulitProject { function_name: &str, ) -> Option> { // Clone the project handle out of the registry lock: building a - // missing graph takes the project's database lock, which must not be + // missing graph takes the project's source gate, which must not be // held while the registry lock is. - let project = { - let projects = self.projects.lock().ok()?; - projects - .iter() - .find(|(path, _)| path.as_path().to_string_lossy() == project_root) - .map(|(_, project)| project.clone())? - }; + let project = self.find_project(project_root)?; project .project .control_flow_graph_for_generation(generation, function_name) @@ -1464,30 +1919,36 @@ impl super::BexLsp for BexMulitProject { cursor_offset: None, }; - let Ok(projects) = self.projects.lock() else { - return empty; + let projects: Vec<_> = { + let Ok(projects) = self.projects.lock() else { + return empty; + }; + projects.values().cloned().collect() }; - for project in projects.values() { - let Ok(db) = project.project.db.lock() else { + for project in projects { + let Ok(guard) = read_for_request(&project.project) else { continue; }; + let db = guard.db(); - // Convert line/column to byte offset using the source file text. // The file_path from Monaco may be relative — find matching file. let Some(source_file) = db.find_source_file(file_path) else { continue; }; - let text: &str = source_file.text(&*db); - let position = lsp_types::Position { + // Playground wire coordinates are fixed zero-based UTF-16 (C2), + // independent of the negotiated LSP encoding. + let text: &str = source_file.text(db); + let codec = + crate::bex_lsp::position_codec::PositionCodec::new(text, PositionEncoding::UTF16); + let byte_offset = match codec.position_to_offset(lsp_types::Position { line, character: column, + }) { + Ok(offset) => u32::from(offset), + Err(_) => 0, }; - let byte_offset = u32::try_from(baml_project::position::lsp_position_to_offset( - text, &position, - )) - .unwrap_or(0); return db.playground_cursor_context(file_path, byte_offset); } @@ -1535,10 +1996,15 @@ impl super::BexLsp for BexMulitProject { } fn resolve_file_id(&self, file_id: u32) -> Option { - let projects = self.projects.lock().unwrap(); - for project in projects.values() { - let db = project.project.db.lock().unwrap(); - if let Some(path) = db.file_id_to_path(baml_base::FileId::new(file_id)) { + let projects: Vec<_> = { + let projects = self.projects.lock().unwrap(); + projects.values().cloned().collect() + }; + for project in projects { + let Ok(Some(guard)) = project.project.read_source_nowait() else { + continue; + }; + if let Some(path) = guard.db().file_id_to_path(baml_base::FileId::new(file_id)) { return Some(path.to_string_lossy().to_string()); } } @@ -1662,6 +2128,90 @@ mod tests { ); } + #[test] + fn diagnostics_fence_publishes_current_and_discards_stale() { + let mut fence = DiagnosticsFence::default(); + + // r1 dirty, candidate at r1: publish and clear. + fence.mark_dirty(SourceRevision(1)); + assert!(fence.admit(SourceRevision(1), SourceRevision(1))); + assert_eq!(fence.dirty, None); + + // r2 and r3 dirty (newest wins), candidate from r2 arrives after r3 + // was marked: discard, r3 stays dirty for its own attempt. + fence.mark_dirty(SourceRevision(2)); + fence.mark_dirty(SourceRevision(3)); + assert!(!fence.admit(SourceRevision(2), SourceRevision(3))); + assert_eq!(fence.dirty, Some(SourceRevision(3))); + + // The r3 candidate publishes and clears. + assert!(fence.admit(SourceRevision(3), SourceRevision(3))); + assert_eq!(fence.dirty, None); + + // A candidate *newer* than the dirty mark also publishes (the fence + // only rejects candidates older than the newest known mutation). + fence.mark_dirty(SourceRevision(4)); + assert!(fence.admit(SourceRevision(5), SourceRevision(5))); + assert_eq!(fence.dirty, None); + + // With nothing dirty, an unsolicited current candidate (e.g. a + // winning rebuild's) still publishes. + assert!(fence.admit(SourceRevision(5), SourceRevision(5))); + } + + /// Candidates older than the authoritative revision are discarded even + /// when nothing is dirty: a superseded rebuild's diagnostics (computed + /// before a newer edit) and a candidate racing `mark_dirty` must never + /// regress markers a newer revision already owns (design 0B: "complete + /// v7 diagnostics after v8 arrives and observe no v7 publication"). + #[test] + fn diagnostics_fence_discards_stale_unsolicited_candidates() { + let mut fence = DiagnosticsFence::default(); + + // r5 published normally; dirty is clear. + fence.mark_dirty(SourceRevision(5)); + assert!(fence.admit(SourceRevision(5), SourceRevision(5))); + + // A superseded rebuild finishes late with r4 diagnostics: discarded. + assert!(!fence.admit(SourceRevision(4), SourceRevision(5))); + + // An edit advanced the revision to r6 but its dirty mark has not + // landed yet (`mark_dirty` runs after `mutate_sources` returns): an + // in-flight r5 candidate is already stale. + assert!(!fence.admit(SourceRevision(5), SourceRevision(6))); + + // The r6 tail publishes normally. + fence.mark_dirty(SourceRevision(6)); + assert!(fence.admit(SourceRevision(6), SourceRevision(6))); + assert_eq!(fence.dirty, None); + } + + #[test] + fn diagnostics_fence_out_of_order_marks_keep_newest() { + let mut fence = DiagnosticsFence::default(); + fence.mark_dirty(SourceRevision(7)); + fence.mark_dirty(SourceRevision(5)); + assert_eq!(fence.dirty, Some(SourceRevision(7))); + } + + #[test] + fn diagnostics_fence_publication_coverage_clears_deleted_files() { + let mut fence = DiagnosticsFence::default(); + let a = std::path::PathBuf::from("/p/a.baml"); + let b = std::path::PathBuf::from("/p/b.baml"); + + let deleted = fence.record_publication([a.clone(), b.clone()].into_iter().collect()); + assert!(deleted.is_empty()); + + // b disappeared: exactly one empty publish for it. + let deleted = fence.record_publication([a.clone()].into_iter().collect()); + assert_eq!(deleted, vec![b]); + + // Steady state: no repeat clears. + let deleted = fence.record_publication([a].into_iter().collect()); + assert!(deleted.is_empty()); + } + #[test] fn vfs_walk_skips_generated_dirs_and_finds_markers() { let root = vfs::VfsPath::new(vfs::MemoryFS::new()); diff --git a/baml_language/crates/bex_project/src/bex_lsp/multi_project/notification.rs b/baml_language/crates/bex_project/src/bex_lsp/multi_project/notification.rs index a20a3b99f7a..034bf0d813b 100644 --- a/baml_language/crates/bex_project/src/bex_lsp/multi_project/notification.rs +++ b/baml_language/crates/bex_project/src/bex_lsp/multi_project/notification.rs @@ -1,4 +1,12 @@ -use super::{BexMulitProject, LspError, ProjectRefreshMode}; +//! LSP notification handlers. +//! +//! Document lifecycle discipline (I1): the overlay map stores text *and* +//! version together, and every refresh applies them as one atomic +//! [`crate::project::SourceBatch`]. While a document is open the overlay is +//! authoritative over disk; `didClose` re-applies disk content and drops the +//! version in the same batch. + +use super::{BexMulitProject, LspError, OverlayDocument, ProjectRefreshMode}; use crate::bex_lsp::notification::BexLspNotification; impl BexLspNotification for BexMulitProject { @@ -9,6 +17,34 @@ impl BexLspNotification for BexMulitProject { Box::new(move |notif| sender.send_notification(notif)) } + fn on_notification_cancel_request( + &self, + _params: lsp_notification_params!("$/cancelRequest"), + ) -> Result<(), LspError> { + // Accepted as a no-op (design D6): the dispatch loop is synchronous, + // so by the time a cancel arrives its request is finished or next in + // line. Accepting it (instead of erroring "not supported") keeps + // clients from logging noise; bounded reads keep worst-case latency + // finite without cooperative cancellation. + Ok(()) + } + + fn on_notification_set_trace( + &self, + _params: lsp_notification_params!("$/setTrace"), + ) -> Result<(), LspError> { + // Trace verbosity is not implemented; accept quietly per spec. + Ok(()) + } + + fn on_notification_will_save( + &self, + _params: lsp_notification_params!("textDocument/willSave"), + ) -> Result<(), LspError> { + // Advertised in capabilities; nothing to prepare before a save. + Ok(()) + } + fn on_notification_exit( &self, _params: lsp_notification_params!("exit"), @@ -40,10 +76,15 @@ impl BexLspNotification for BexMulitProject { let mut in_memory_changes = project_handle.in_memory_changes.lock().unwrap(); in_memory_changes.insert( crate::fs::FsPath::from_vfs(&path), - params.text_document.text, + OverlayDocument { + text: params.text_document.text, + version: Some(params.text_document.version), + }, ); drop(in_memory_changes); + // Full refresh: the first open in a lazily-created project must load + // the rest of the project from disk too. Overlays win over disk. self.refresh_project(&project_root, ProjectRefreshMode::Full); Ok(()) } @@ -85,7 +126,7 @@ impl BexLspNotification for BexMulitProject { let new_text = match params.content_changes.as_slice() { [event] if event.range.is_none() => event.text.clone(), _ => { - return Err(LspError::RequestNotSupported( + return Err(LspError::InvalidParams( "Expected a single full-document change event (TextDocumentSyncKind::FULL)" .to_string(), )); @@ -97,7 +138,13 @@ impl BexLspNotification for BexMulitProject { let project = self.get_or_create_project(project_root.clone())?; let mut in_memory_changes = project.in_memory_changes.lock().unwrap(); - in_memory_changes.insert(crate::fs::FsPath::from_vfs(&path), new_text); + in_memory_changes.insert( + crate::fs::FsPath::from_vfs(&path), + OverlayDocument { + text: new_text, + version: Some(params.text_document.version), + }, + ); drop(in_memory_changes); self.refresh_project( @@ -121,7 +168,12 @@ impl BexLspNotification for BexMulitProject { in_memory_changes.remove(&crate::fs::FsPath::from_vfs(&path)); drop(in_memory_changes); - self.refresh_project(&project_root, ProjectRefreshMode::Only(vec![path])); + // Re-apply disk content and drop the open-document version in the + // same batch: future publications for this file become unversioned. + self.refresh_project( + &project_root, + ProjectRefreshMode::ClosedDocuments(vec![path]), + ); Ok(()) } @@ -133,12 +185,15 @@ impl BexLspNotification for BexMulitProject { let project_root = Self::get_baml_project_root(&path)?; let project = self.get_or_create_project(project_root)?; + // Buffer == disk at save time, so the overlay entry is redundant + // until the next didChange re-adds it. The document stays open and + // its version stays valid: the open-document version map is only + // cleared by didClose. let mut in_memory_changes = project.in_memory_changes.lock().unwrap(); in_memory_changes.remove(&crate::fs::FsPath::from_vfs(&path)); drop(in_memory_changes); - // We don't need to refresh the project here, because the in-memory - // and disk versions of the file are already in sync + // No refresh needed: the database already holds this exact text. Ok(()) } } diff --git a/baml_language/crates/bex_project/src/bex_lsp/multi_project/request.rs b/baml_language/crates/bex_project/src/bex_lsp/multi_project/request.rs index f283b75c138..88e10aa0e50 100644 --- a/baml_language/crates/bex_project/src/bex_lsp/multi_project/request.rs +++ b/baml_language/crates/bex_project/src/bex_lsp/multi_project/request.rs @@ -1,3 +1,14 @@ +//! LSP request handlers. +//! +//! Every handler follows the same discipline (design C1 + Phase 0C): +//! +//! - Database access is a *bounded* read ([`super::read_for_request`]): +//! busy projects produce typed `ContentModified`/`RequestFailed` errors +//! instead of stalling the loop or bursting `-32001`. +//! - Positions and ranges cross the LSP boundary through one +//! [`PositionCodec`] built with the encoding negotiated during +//! `initialize`. Compiler APIs stay byte-based. + use lsp_types::{ CodeLens, CodeLensOptions, CompletionOptions, HoverProviderCapability, InlayHintOptions, InlayHintServerCapabilities, SaveOptions, SemanticTokensFullOptions, SemanticTokensLegend, @@ -7,15 +18,23 @@ use lsp_types::{ WorkspaceServerCapabilities, }; -use super::{BexMulitProject, LspError, WithDiagnostics, commands, wasm_helpers}; -use crate::bex_lsp::{multi_project::commands::BexLspCommand, protocol, request::BexLspRequest}; +use super::{BexMulitProject, LspError, commands, read_for_request, wasm_helpers}; +use crate::bex_lsp::{ + multi_project::commands::BexLspCommand, + position_codec::{PositionCodec, PositionEncoding}, + protocol, + request::BexLspRequest, +}; /// Server capabilities advertised during the LSP `initialize` handshake. /// /// Defined here so that both the native stdio server and the WASM bridge /// share a single source of truth for what the LSP implementation supports. -pub(super) fn server_capabilities() -> ServerCapabilities { +/// `encoding` is the negotiated position encoding (C1) — advertising it is +/// mandatory whenever the client offered `positionEncodings`. +pub(super) fn server_capabilities(encoding: PositionEncoding) -> ServerCapabilities { ServerCapabilities { + position_encoding: Some(encoding.to_lsp_kind()), // Diagnostics are delivered via push (`publishDiagnostics`) only. // Pull diagnostics (`textDocument/diagnostic`) is disabled to avoid // the editor showing each diagnostic twice. @@ -91,9 +110,9 @@ pub(super) fn server_capabilities() -> ServerCapabilities { } } -fn initialize_result() -> lsp_types::InitializeResult { +fn initialize_result(encoding: PositionEncoding) -> lsp_types::InitializeResult { lsp_types::InitializeResult { - capabilities: server_capabilities(), + capabilities: server_capabilities(encoding), server_info: Some(lsp_types::ServerInfo { name: "baml-lsp".to_string(), version: Some(baml_version::CANONICAL_VERSION.to_string()), @@ -129,6 +148,10 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("initialize"), ) -> Result { + // Negotiate the position encoding first (C1): UTF-8 when offered, + // UTF-16 baseline otherwise. Everything after this reads the cell. + let encoding = self.negotiate_encoding(¶ms.capabilities); + let mut roots = Vec::new(); if let Some(folders) = ¶ms.workspace_folders { @@ -149,32 +172,33 @@ impl BexLspRequest for BexMulitProject { } tracing::info!( - "Workspace roots: {:?}", - roots.iter().map(vfs::VfsPath::as_str).collect::>() + "Workspace roots: {:?}; position encoding: {:?}", + roots.iter().map(vfs::VfsPath::as_str).collect::>(), + encoding, ); *self.workspace_roots.lock().unwrap() = roots; - Ok(initialize_result()) + Ok(initialize_result(encoding)) } fn on_request_text_document_code_lens( &self, params: lsp_request_params!("textDocument/codeLens"), ) -> Result { + let encoding = self.encoding_for_request()?; let path = self.get_path_from_uri(¶ms.text_document.uri)?; let root_path = Self::get_baml_project_root(&path)?; let project_handle = self.get_or_create_project(root_path.clone())?; let lenses = { - let project = project_handle.project.try_lock_db()?; - let lsp_db = project.db(); + let guard = read_for_request(&project_handle.project)?; + let lsp_db = guard.db(); let Some(source_file) = lsp_db.get_file(std::path::Path::new(path.as_str())) else { return Ok(None); }; let text = source_file.text(lsp_db); - let line_starts = compute_line_starts(text); - let encoding = self.position_encoding; + let codec = PositionCodec::new(text, encoding); // Use compiler2 file_actions — finds functions + tests via // file_symbol_contributions (Salsa-cached, no type inference needed). @@ -183,12 +207,7 @@ impl BexLspRequest for BexMulitProject { file_actions .into_iter() .map(|action| { - let range = super::diagnostics::span_to_lsp_range( - action.name_span, - text, - &line_starts, - encoding, - ); + let range = codec.byte_range_to_lsp(action.name_span); let command = match action.kind { baml_lsp2_actions::FileActionKind::RunInPlayground => { super::commands::OpenBamlPanel { @@ -237,46 +256,31 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("textDocument/inlayHint"), ) -> Result { + let encoding = self.encoding_for_request()?; let path = self.get_path_from_uri(¶ms.text_document.uri)?; let root_path = Self::get_baml_project_root(&path)?; let project_handle = self.get_or_create_project(root_path)?; - let project = project_handle.project.try_lock_db()?; - let lsp_db = project.db(); + let guard = read_for_request(&project_handle.project)?; + let lsp_db = guard.db(); let Some(source_file) = lsp_db.get_file(std::path::Path::new(path.as_str())) else { return Ok(None); }; let text = source_file.text(lsp_db); + let codec = PositionCodec::new(text, encoding); + let range = codec.range_to_byte_range(params.range)?; - // Compute the byte-offset bounds of the requested range. - let range_start = text_size::TextSize::from( - u32::try_from(baml_project::position::lsp_position_to_offset( - text, - ¶ms.range.start, - )) - .unwrap_or(0), - ); - let range_end = text_size::TextSize::from( - u32::try_from(baml_project::position::lsp_position_to_offset( - text, - ¶ms.range.end, - )) - .unwrap_or(u32::MAX), - ); - - // Compute inline annotations using compiler2 (type hints + param hints). - let hints = baml_lsp2_actions::annotations(lsp_db, source_file); + // Inline annotations are Salsa-memoized per file revision (B1); the + // handler only converts the cached hints into the requested range. + let hints = baml_lsp2_actions::file_annotations(lsp_db, source_file); let lsp_hints: Vec = hints - .into_iter() - .filter(|h| h.offset >= range_start && h.offset < range_end) + .iter() + .filter(|h| h.offset >= range.start() && h.offset < range.end()) .map(|h| lsp_types::InlayHint { - position: baml_project::position::offset_to_lsp_position( - text, - usize::from(h.offset), - ), - label: lsp_types::InlayHintLabel::String(h.label), + position: codec.offset_to_position(h.offset.into()), + label: lsp_types::InlayHintLabel::String(h.label.clone()), kind: Some(match h.kind { baml_lsp2_actions::AnnotationKind::Type => lsp_types::InlayHintKind::TYPE, baml_lsp2_actions::AnnotationKind::Parameter => { @@ -302,53 +306,48 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("textDocument/semanticTokens/full"), ) -> Result { + let encoding = self.encoding_for_request()?; let path = self.get_path_from_uri(¶ms.text_document.uri)?; let root_path = Self::get_baml_project_root(&path)?; let project_handle = self.get_or_create_project(root_path)?; - let project = project_handle.project.try_lock_db()?; - let lsp_db = project.db(); + let guard = read_for_request(&project_handle.project)?; + let lsp_db = guard.db(); let Some(source_file) = lsp_db.get_file(std::path::Path::new(path.as_str())) else { return Ok(None); }; let text = source_file.text(lsp_db); + let codec = PositionCodec::new(text, encoding); - // Get the semantic tokens using compiler2 (hybrid CST + type-aware). - // Always returns tokens in document order. + // Semantic tokens are Salsa-memoized per file revision (B1); the + // handler only delta-encodes the cached classification. let tokens = baml_lsp2_actions::semantic_tokens(lsp_db, source_file); - // Convert to LSP delta-encoded format - let line_index = baml_project::position::LineIndex::new(text); + // Delta-encode in the negotiated encoding. Multiline tokens are split + // into same-line segments — VS Code does not advertise the multiline + // token capability, and splitting is valid for every client (C1). let mut lsp_tokens = Vec::with_capacity(tokens.len()); let mut prev_line = 0u32; let mut prev_start = 0u32; - for token in &tokens { - let start_offset: u32 = token.range.start().into(); - let end_offset: u32 = token.range.end().into(); - let length = end_offset - start_offset; - - let Some(pos) = line_index.offset_to_position(start_offset) else { - continue; - }; - - let delta_line = pos.line - prev_line; - let delta_start = if delta_line == 0 { - pos.character - prev_start - } else { - pos.character - }; - - lsp_tokens.push(lsp_types::SemanticToken { - delta_line, - delta_start, - length, - token_type: token.token_type.legend_index(), - token_modifiers_bitset: 0, - }); - - prev_line = pos.line; - prev_start = pos.character; + for token in tokens { + for segment in codec.token_segments(token.range) { + let delta_line = segment.line - prev_line; + let delta_start = if delta_line == 0 { + segment.start_character - prev_start + } else { + segment.start_character + }; + lsp_tokens.push(lsp_types::SemanticToken { + delta_line, + delta_start, + length: segment.length, + token_type: token.token_type.legend_index(), + token_modifiers_bitset: 0, + }); + prev_line = segment.line; + prev_start = segment.start_character; + } } Ok(Some(lsp_types::SemanticTokensResult::Tokens( @@ -363,34 +362,20 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("textDocument/codeAction"), ) -> Result { + let encoding = self.encoding_for_request()?; let path = self.get_path_from_uri(¶ms.text_document.uri)?; let root_path = Self::get_baml_project_root(&path)?; let project_handle = self.get_or_create_project(root_path.clone())?; let actions: Vec = { - let project = project_handle.project.try_lock_db()?; - let lsp_db = project.db(); + let guard = read_for_request(&project_handle.project)?; + let lsp_db = guard.db(); let Some(source_file) = lsp_db.get_file(std::path::Path::new(path.as_str())) else { return Ok(None); }; let text = source_file.text(lsp_db); - - // Convert the LSP range to a byte range for fixes_at. - let start_offset = text_size::TextSize::from( - u32::try_from(baml_project::position::lsp_position_to_offset( - text, - ¶ms.range.start, - )) - .unwrap_or(0), - ); - let end_offset = text_size::TextSize::from( - u32::try_from(baml_project::position::lsp_position_to_offset( - text, - ¶ms.range.end, - )) - .unwrap_or(u32::MAX), - ); - let range = text_size::TextRange::new(start_offset, end_offset); + let codec = PositionCodec::new(text, encoding); + let range = codec.range_to_byte_range(params.range)?; // Use compiler2 fixes_at — currently returns "Open in Playground". let fixes = baml_lsp2_actions::fixes_at(lsp_db, source_file, range); @@ -493,9 +478,7 @@ impl BexLspRequest for BexMulitProject { // Use compiler2 completions_at — context-aware completions from CST + HIR/TIR. let completions = self.compute_on_position( ¶ms.text_document_position, - |db, source_file, _project, offset| { - baml_lsp2_actions::completions_at(db, source_file, offset) - }, + |db, source_file, offset, _| baml_lsp2_actions::completions_at(db, source_file, offset), )?; // Convert domain Completion → LSP CompletionItem. @@ -555,7 +538,7 @@ impl BexLspRequest for BexMulitProject { ) -> Result { let type_info = self.compute_on_position( ¶ms.text_document_position_params, - |db, source_file, _project, offset| baml_lsp2_actions::type_at(db, source_file, offset), + |db, source_file, offset, _| baml_lsp2_actions::type_at(db, source_file, offset), )?; match type_info { @@ -577,22 +560,17 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("textDocument/definition"), ) -> Result { - let position_encoding = self.position_encoding; self.compute_on_position( ¶ms.text_document_position_params, - |db, source_file, _, offset| { + |db, source_file, offset, encoding| { let loc = baml_lsp2_actions::definition_at(db, source_file, offset)?; let file_id = loc.file.file_id(db); let path = db.file_id_to_path(file_id)?; let target_uri = wasm_helpers::from_file_path(path).ok()?; - let target_text = loc.file.text(db); - let line_starts = compute_line_starts(target_text); - let range = super::diagnostics::span_to_lsp_range( - loc.range, - target_text, - &line_starts, - position_encoding, - ); + // The definition may live in another file: convert with the + // target file's codec, not the request file's. + let target_codec = PositionCodec::new(loc.file.text(db), encoding); + let range = target_codec.byte_range_to_lsp(loc.range); Some(Ok(lsp_types::GotoDefinitionResponse::Scalar( lsp_types::Location { uri: target_uri, @@ -608,10 +586,9 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("textDocument/references"), ) -> Result { - let position_encoding = self.position_encoding; let references: Vec = self.compute_on_position( ¶ms.text_document_position, - |db, source_file, _, offset| { + |db, source_file, offset, encoding| { // Use compiler2 usages_at — returns Vec (file + TextRange). let usages = baml_lsp2_actions::usages_at(db, source_file, offset); @@ -621,14 +598,8 @@ impl BexLspRequest for BexMulitProject { let file_id = loc.file.file_id(db); let path = db.file_id_to_path(file_id)?; let target_uri = wasm_helpers::from_file_path(path).ok()?; - let target_text = loc.file.text(db); - let line_starts = compute_line_starts(target_text); - let range = super::diagnostics::span_to_lsp_range( - loc.range, - target_text, - &line_starts, - position_encoding, - ); + let target_codec = PositionCodec::new(loc.file.text(db), encoding); + let range = target_codec.byte_range_to_lsp(loc.range); Some(lsp_types::Location { uri: target_uri, range, @@ -649,23 +620,32 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("textDocument/diagnostic"), ) -> Result { + // Pull diagnostics is not advertised, but answer correctly for + // clients that ask anyway: same candidate + codec conversion as the + // push path. + let encoding = self.encoding_for_request()?; let path = self.get_path_from_uri(¶ms.text_document.uri)?; let root_path = Self::get_baml_project_root(&path)?; let project_handle = self.get_or_create_project(root_path)?; - let mut diagnostics = project_handle - .project - .diagnostics_by_file(self.position_encoding); - let diagnostics = diagnostics - .remove(std::path::Path::new(path.as_str())) + let candidate = { + let guard = read_for_request(&project_handle.project)?; + crate::project::collect_diagnostic_candidate(&guard) + }; + let documents = super::diagnostics::candidate_to_publishable(&candidate, encoding); + let items = documents + .into_iter() + .find(|doc| doc.path.as_path() == std::path::Path::new(path.as_str())) + .map(|doc| doc.diagnostics) .unwrap_or_default(); + Ok(lsp_types::DocumentDiagnosticReportResult::Report( lsp_types::DocumentDiagnosticReport::Full( lsp_types::RelatedFullDocumentDiagnosticReport { related_documents: None, full_document_diagnostic_report: lsp_types::FullDocumentDiagnosticReport { result_id: None, - items: diagnostics, + items, }, }, ), @@ -676,15 +656,22 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("workspace/symbol"), ) -> Result { + let encoding = self.encoding_for_request()?; let query = ¶ms.query; let mut symbols = Vec::new(); - let projects = self.projects.lock().unwrap(); - for project_handle in projects.values() { - let Ok(db_guard) = project_handle.project.try_lock_db() else { + // Fan-out across every project: skip busy projects instead of + // serializing bounded waits (a symbol search should not stack + // N seconds of deadlines). + let projects: Vec<_> = { + let projects = self.projects.lock().unwrap(); + projects.values().cloned().collect() + }; + for project_handle in projects { + let Ok(Some(guard)) = project_handle.project.read_source_nowait() else { continue; }; - let lsp_db = db_guard.db(); + let lsp_db = guard.db(); // Use compiler2 search_symbols — iterates all user source files and // filters by the query string. file_outline is Salsa-cached per file, @@ -700,13 +687,8 @@ impl BexLspRequest for BexMulitProject { let Ok(uri) = wasm_helpers::from_file_path(path) else { continue; }; - let text = sym.file.text(lsp_db); - let range = super::diagnostics::span_to_lsp_range( - sym.name_span, - text, - &compute_line_starts(text), - super::diagnostics::PositionEncoding::UTF16, - ); + let codec = PositionCodec::new(sym.file.text(lsp_db), encoding); + let range = codec.byte_range_to_lsp(sym.name_span); symbols.push(lsp_types::WorkspaceSymbol { name: sym.name, @@ -732,12 +714,9 @@ impl BexLspRequest for BexMulitProject { ) -> Result { fn convert_outline_item( item: &baml_lsp2_actions::OutlineItem, - text: &str, - line_starts: &[u32], - encoding: super::diagnostics::PositionEncoding, + codec: &PositionCodec<'_>, ) -> lsp_types::DocumentSymbol { - let range = - super::diagnostics::span_to_lsp_range(item.name_span, text, line_starts, encoding); + let range = codec.byte_range_to_lsp(item.name_span); let children = if item.children.is_empty() { None @@ -745,7 +724,7 @@ impl BexLspRequest for BexMulitProject { Some( item.children .iter() - .map(|child| convert_outline_item(child, text, line_starts, encoding)) + .map(|child| convert_outline_item(child, codec)) .collect(), ) }; @@ -763,24 +742,23 @@ impl BexLspRequest for BexMulitProject { } } + let encoding = self.encoding_for_request()?; let path = self.get_path_from_uri(¶ms.text_document.uri)?; let root_path = Self::get_baml_project_root(&path)?; let project_handle = self.get_or_create_project(root_path)?; - let project = project_handle.project.try_lock_db()?; - let lsp_db = project.db(); + let guard = read_for_request(&project_handle.project)?; + let lsp_db = guard.db(); let Some(source_file) = lsp_db.get_file(std::path::Path::new(path.as_str())) else { return Ok(None); }; - let text = source_file.text(lsp_db); - let line_starts = compute_line_starts(text); - let encoding = self.position_encoding; + let codec = PositionCodec::new(source_file.text(lsp_db), encoding); let outline = baml_lsp2_actions::file_outline(lsp_db, source_file); let symbols: Vec<_> = outline .iter() - .map(|item| convert_outline_item(item, text, &line_starts, encoding)) + .map(|item| convert_outline_item(item, &codec)) .collect(); if symbols.is_empty() { @@ -794,16 +772,18 @@ impl BexLspRequest for BexMulitProject { &self, params: lsp_request_params!("textDocument/formatting"), ) -> Result { + let encoding = self.encoding_for_request()?; let path = self.get_path_from_uri(¶ms.text_document.uri)?; let root_path = Self::get_baml_project_root(&path)?; let project_handle = self.get_or_create_project(root_path)?; // Get current file text from the project database. let text = { - let db = project_handle.project.try_lock_db()?; - let Some(source_file) = db.get_file(std::path::Path::new(path.as_str())) else { + let guard = read_for_request(&project_handle.project)?; + let lsp_db = guard.db(); + let Some(source_file) = lsp_db.get_file(std::path::Path::new(path.as_str())) else { return Err(LspError::FileNotFound(path)); }; - source_file.text(&*db).clone() + source_file.text(lsp_db).clone() }; // Map LSP FormattingOptions → baml_fmt FormatOptions. @@ -827,26 +807,17 @@ impl BexLspRequest for BexMulitProject { return Ok(None); } - // Compute end position of the original text for the replacement range. - let line_count = u32::try_from(text.lines().count()).unwrap_or(u32::MAX); - let last_line_len = - u32::try_from(text.lines().last().map_or(0, str::len)).unwrap_or(u32::MAX); - let (end_line, end_char) = if text.ends_with('\n') { - (line_count, 0) - } else { - (line_count.saturating_sub(1), last_line_len) - }; - + // Replace the whole document: [0:0, document end) in the negotiated + // encoding (the old line-count arithmetic miscounted `\r\n` and + // non-ASCII last lines). + let codec = PositionCodec::new(&text, encoding); Ok(Some(vec![lsp_types::TextEdit { range: lsp_types::Range { start: lsp_types::Position { line: 0, character: 0, }, - end: lsp_types::Position { - line: end_line, - character: end_char, - }, + end: codec.document_end(), }, new_text: formatted, }])) @@ -881,45 +852,39 @@ fn definition_kind_to_lsp_symbol_kind( } } -/// Alias for the `compute_line_starts` helper from the diagnostics module. -#[inline] -fn compute_line_starts(source: &str) -> Vec { - super::diagnostics::compute_line_starts(source) -} - impl BexMulitProject { + /// Shared scaffolding for position-based requests: bounded read, file + /// lookup, and incoming position conversion through the negotiated codec. + /// + /// The closure receives the database, the file, the byte offset of the + /// request position, and the negotiated encoding (for converting result + /// spans that may live in *other* files). fn compute_on_position( &self, params: &lsp_types::TextDocumentPositionParams, op: impl FnOnce( &baml_project::ProjectDatabase, baml_db::SourceFile, - baml_workspace::Project, text_size::TextSize, + PositionEncoding, ) -> T, ) -> Result { + let encoding = self.encoding_for_request()?; let path = self.get_path_from_uri(¶ms.text_document.uri)?; let root_path = Self::get_baml_project_root(&path)?; let project_handle = self.get_or_create_project(root_path)?; let position = params.position; - let project = project_handle.project.try_lock_db()?; - let lsp_db = project.db(); - let Some(project) = project.project() else { - return Err(LspError::ProjectNotFound(path)); - }; + let guard = read_for_request(&project_handle.project)?; + let lsp_db = guard.db(); let Some(source_file) = lsp_db.get_file(std::path::Path::new(path.as_str())) else { return Err(LspError::FileNotFound(path)); }; let text = source_file.text(lsp_db); - let offset = text_size::TextSize::from( - u32::try_from(baml_project::position::lsp_position_to_offset( - text, &position, - )) - .unwrap_or(0), - ); + let codec = PositionCodec::new(text, encoding); + let offset = codec.position_to_offset(position)?; - Ok(op(lsp_db, source_file, project, offset)) + Ok(op(lsp_db, source_file, offset, encoding)) } } @@ -929,7 +894,7 @@ mod tests { #[test] fn initialize_metadata_matches_protocol_contract() { - let result = initialize_result(); + let result = initialize_result(PositionEncoding::UTF16); let experimental = result .capabilities .experimental @@ -957,4 +922,16 @@ mod tests { Some(baml_version::CANONICAL_VERSION.to_string()) ); } + + #[test] + fn capabilities_advertise_negotiated_encoding() { + assert_eq!( + server_capabilities(PositionEncoding::UTF8).position_encoding, + Some(lsp_types::PositionEncodingKind::UTF8) + ); + assert_eq!( + server_capabilities(PositionEncoding::UTF16).position_encoding, + Some(lsp_types::PositionEncodingKind::UTF16) + ); + } } diff --git a/baml_language/crates/bex_project/src/bex_lsp/position_codec.rs b/baml_language/crates/bex_project/src/bex_lsp/position_codec.rs new file mode 100644 index 00000000000..c7e2d0c92c2 --- /dev/null +++ b/baml_language/crates/bex_project/src/bex_lsp/position_codec.rs @@ -0,0 +1,471 @@ +//! Negotiated LSP position boundary (design C1). +//! +//! One codec owns all conversions between compiler byte offsets and LSP +//! positions. Compiler APIs stay byte-based; every LSP handler converts at +//! this boundary with the encoding negotiated during `initialize` (UTF-8 +//! when the client offers it, otherwise UTF-16 — Decision D4). +//! +//! The codec recognizes LF, CRLF, and bare CR line terminators. Incoming +//! overlong character positions clamp to their line's content end; +//! nonexistent lines, malformed ranges, and positions inside an encoding +//! unit are `InvalidParams`. Outgoing conversions are lenient (compiler +//! spans are trusted): out-of-bounds offsets clamp. + +use text_size::{TextRange, TextSize}; + +/// Position encodings supported for negotiation. +/// +/// Ordered least- to greatest-priority for the derived `Ord`: UTF-16 is the +/// mandatory baseline, UTF-8 is BAML's preferred fast path. UTF-32 is not +/// negotiated (Decision D4) and exists only to convert if a future decision +/// adds it. +#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum PositionEncoding { + /// The encoding every LSP client must support. + #[default] + UTF16, + /// Fixed 4-byte units; conversion is codepoint counting. + #[allow(dead_code)] + UTF32, + /// BAML's preferred encoding: characters are byte offsets. + UTF8, +} + +impl PositionEncoding { + /// Select the encoding for this session: UTF-8 when the client offers + /// it, otherwise UTF-16 (Decision D4; UTF-32 is never selected). + pub(crate) fn negotiate( + client_offered: Option<&[lsp_types::PositionEncodingKind]>, + ) -> PositionEncoding { + let offered_utf8 = client_offered + .unwrap_or_default() + .contains(&lsp_types::PositionEncodingKind::UTF8); + if offered_utf8 { + PositionEncoding::UTF8 + } else { + PositionEncoding::UTF16 + } + } + + pub(crate) fn to_lsp_kind(self) -> lsp_types::PositionEncodingKind { + match self { + PositionEncoding::UTF8 => lsp_types::PositionEncodingKind::UTF8, + PositionEncoding::UTF16 => lsp_types::PositionEncodingKind::UTF16, + PositionEncoding::UTF32 => lsp_types::PositionEncodingKind::UTF32, + } + } +} + +/// Why an incoming LSP position or range could not be converted. Serialized +/// as `InvalidParams` (`-32602`) at the error boundary. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum PositionCodecError { + LineOutOfRange { + line: u32, + line_count: u32, + }, + /// Range end precedes range start. + MalformedRange, + /// The character index falls strictly inside one encoded character + /// (e.g. between UTF-16 surrogate halves or inside a UTF-8 sequence). + InsideEncodingUnit { + line: u32, + character: u32, + }, +} + +impl std::fmt::Display for PositionCodecError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + PositionCodecError::LineOutOfRange { line, line_count } => { + write!( + f, + "line {line} out of range (document has {line_count} lines)" + ) + } + PositionCodecError::MalformedRange => write!(f, "range end precedes range start"), + PositionCodecError::InsideEncodingUnit { line, character } => write!( + f, + "position {line}:{character} falls inside an encoded character" + ), + } + } +} + +impl From for crate::bex_lsp::LspError { + fn from(e: PositionCodecError) -> Self { + crate::bex_lsp::LspError::InvalidParams(e.to_string()) + } +} + +/// One same-line semantic-token segment in negotiated encoding units. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct TokenSegment { + pub line: u32, + pub start_character: u32, + pub length: u32, +} + +/// Line-aware position codec for one document snapshot. +pub(crate) struct PositionCodec<'a> { + text: &'a str, + encoding: PositionEncoding, + /// Byte offset where line `i` starts. `line_starts[0] == 0`. + line_starts: Vec, + /// Byte offset where line `i`'s content ends (excludes the LF / CRLF / + /// CR terminator). Parallel to `line_starts`. + line_content_ends: Vec, +} + +impl<'a> PositionCodec<'a> { + pub(crate) fn new(text: &'a str, encoding: PositionEncoding) -> Self { + let bytes = text.as_bytes(); + let mut line_starts = vec![0u32]; + let mut line_content_ends = Vec::new(); + let mut i = 0usize; + while i < bytes.len() { + match bytes[i] { + b'\n' => { + line_content_ends.push(to_u32(i)); + i += 1; + line_starts.push(to_u32(i)); + } + b'\r' => { + line_content_ends.push(to_u32(i)); + i += if bytes.get(i + 1) == Some(&b'\n') { + 2 + } else { + 1 + }; + line_starts.push(to_u32(i)); + } + _ => i += 1, + } + } + line_content_ends.push(to_u32(bytes.len())); + debug_assert_eq!(line_starts.len(), line_content_ends.len()); + Self { + text, + encoding, + line_starts, + line_content_ends, + } + } + + fn line_count(&self) -> u32 { + to_u32(self.line_starts.len()) + } + + /// Line index containing `offset` (clamped into the document). + fn line_of_offset(&self, offset: u32) -> usize { + self.line_starts.partition_point(|&s| s <= offset) - 1 + } + + /// Encoded length of a byte slice known to lie within one line. + fn encoded_len(&self, slice: &str) -> u32 { + match self.encoding { + PositionEncoding::UTF8 => to_u32(slice.len()), + PositionEncoding::UTF16 => to_u32(slice.chars().map(char::len_utf16).sum::()), + PositionEncoding::UTF32 => to_u32(slice.chars().count()), + } + } + + // ── Incoming: LSP position/range → byte offset/range ──────────────── + + /// Convert an incoming LSP position to a byte offset. + /// + /// Overlong `character` clamps to the line's content end. A nonexistent + /// line or a position inside one encoded character is an error. + pub(crate) fn position_to_offset( + &self, + position: lsp_types::Position, + ) -> Result { + let line = position.line as usize; + if line >= self.line_starts.len() { + return Err(PositionCodecError::LineOutOfRange { + line: position.line, + line_count: self.line_count(), + }); + } + let line_start = self.line_starts[line] as usize; + let content_end = self.line_content_ends[line] as usize; + let slice = &self.text[line_start..content_end]; + + if position.character == 0 { + return Ok(TextSize::from(to_u32(line_start))); + } + + let mut units = 0u32; + for (byte_in_line, ch) in slice.char_indices() { + if units == position.character { + return Ok(TextSize::from(to_u32(line_start + byte_in_line))); + } + let width = match self.encoding { + PositionEncoding::UTF8 => to_u32(ch.len_utf8()), + PositionEncoding::UTF16 => to_u32(ch.len_utf16()), + PositionEncoding::UTF32 => 1, + }; + if units + width > position.character { + return Err(PositionCodecError::InsideEncodingUnit { + line: position.line, + character: position.character, + }); + } + units += width; + } + // `character` is at or beyond the line's end: clamp to content end. + Ok(TextSize::from(to_u32(content_end))) + } + + /// Convert an incoming LSP range to a byte range. Both positions must be + /// valid and start must not exceed end. + pub(crate) fn range_to_byte_range( + &self, + range: lsp_types::Range, + ) -> Result { + let start = self.position_to_offset(range.start)?; + let end = self.position_to_offset(range.end)?; + if start > end { + return Err(PositionCodecError::MalformedRange); + } + Ok(TextRange::new(start, end)) + } + + // ── Outgoing: byte offset/span → LSP position/range ───────────────── + + /// Convert a byte offset to an LSP position. Lenient: out-of-bounds + /// offsets clamp to the document end, offsets inside a UTF-8 sequence + /// snap back to the character start, and offsets inside a line + /// terminator report the line's content end. + pub(crate) fn offset_to_position(&self, offset: u32) -> lsp_types::Position { + let mut offset = (offset as usize).min(self.text.len()); + while offset > 0 && !self.text.is_char_boundary(offset) { + offset -= 1; + } + let line = self.line_of_offset(to_u32(offset)); + let line_start = self.line_starts[line] as usize; + let content_end = self.line_content_ends[line] as usize; + let clamped = offset.min(content_end).max(line_start); + lsp_types::Position { + line: to_u32(line), + character: self.encoded_len(&self.text[line_start..clamped]), + } + } + + /// Convert a byte span to an LSP range. + pub(crate) fn byte_range_to_lsp(&self, range: TextRange) -> lsp_types::Range { + lsp_types::Range { + start: self.offset_to_position(range.start().into()), + end: self.offset_to_position(range.end().into()), + } + } + + /// Position of the document end (for formatting's full-document range). + pub(crate) fn document_end(&self) -> lsp_types::Position { + self.offset_to_position(to_u32(self.text.len())) + } + + // ── Semantic tokens ────────────────────────────────────────────────── + + /// Split a byte span into same-line segments with negotiated-encoding + /// starts and lengths. Newline units are excluded and zero-length + /// segments discarded — valid for every client and required for + /// VS Code's missing multiline-token capability. + pub(crate) fn token_segments(&self, range: TextRange) -> Vec { + let start = (u32::from(range.start()) as usize).min(self.text.len()); + let end = (u32::from(range.end()) as usize).min(self.text.len()); + if start >= end { + return Vec::new(); + } + + let first_line = self.line_of_offset(to_u32(start)); + let mut segments = Vec::new(); + for line in first_line..self.line_starts.len() { + let line_start = self.line_starts[line] as usize; + if line_start >= end { + break; + } + let content_end = self.line_content_ends[line] as usize; + let seg_start = start.max(line_start).min(content_end); + let seg_end = end.min(content_end); + if seg_start >= seg_end { + continue; + } + segments.push(TokenSegment { + line: to_u32(line), + start_character: self.encoded_len(&self.text[line_start..seg_start]), + length: self.encoded_len(&self.text[seg_start..seg_end]), + }); + } + segments + } +} + +fn to_u32(v: usize) -> u32 { + u32::try_from(v).unwrap_or(u32::MAX) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn pos(line: u32, character: u32) -> lsp_types::Position { + lsp_types::Position { line, character } + } + + #[test] + fn line_terminators_lf_crlf_and_bare_cr() { + let codec = PositionCodec::new("a\nb\r\nc\rd", PositionEncoding::UTF8); + // Lines: "a" @0, "b" @2, "c" @5, "d" @7. + assert_eq!(codec.position_to_offset(pos(0, 0)).unwrap(), 0.into()); + assert_eq!(codec.position_to_offset(pos(1, 0)).unwrap(), 2.into()); + assert_eq!(codec.position_to_offset(pos(2, 0)).unwrap(), 5.into()); + assert_eq!(codec.position_to_offset(pos(3, 1)).unwrap(), 8.into()); + assert_eq!(codec.offset_to_position(7), pos(3, 0)); + // Offset inside the CRLF terminator reports the content end of line 1. + assert_eq!(codec.offset_to_position(4), pos(1, 1)); + } + + #[test] + fn utf16_and_utf8_round_trip_multibyte() { + // "é" is 2 bytes / 1 UTF-16 unit; "😀" is 4 bytes / 2 UTF-16 units. + let text = "é😀x"; + let utf8 = PositionCodec::new(text, PositionEncoding::UTF8); + let utf16 = PositionCodec::new(text, PositionEncoding::UTF16); + + // Offset of 'x' is 6 bytes. + assert_eq!(utf8.position_to_offset(pos(0, 6)).unwrap(), 6.into()); + assert_eq!(utf16.position_to_offset(pos(0, 3)).unwrap(), 6.into()); + assert_eq!(utf8.offset_to_position(6), pos(0, 6)); + assert_eq!(utf16.offset_to_position(6), pos(0, 3)); + } + + #[test] + fn position_inside_encoding_unit_is_error() { + let text = "😀"; // 4 bytes, 2 UTF-16 units. + let utf16 = PositionCodec::new(text, PositionEncoding::UTF16); + assert!(matches!( + utf16.position_to_offset(pos(0, 1)), + Err(PositionCodecError::InsideEncodingUnit { .. }) + )); + let utf8 = PositionCodec::new(text, PositionEncoding::UTF8); + assert!(matches!( + utf8.position_to_offset(pos(0, 2)), + Err(PositionCodecError::InsideEncodingUnit { .. }) + )); + } + + #[test] + fn overlong_character_clamps_to_line_end_and_missing_line_errors() { + let codec = PositionCodec::new("ab\ncd", PositionEncoding::UTF8); + assert_eq!(codec.position_to_offset(pos(0, 99)).unwrap(), 2.into()); + assert!(matches!( + codec.position_to_offset(pos(9, 0)), + Err(PositionCodecError::LineOutOfRange { .. }) + )); + } + + #[test] + fn malformed_range_is_rejected() { + let codec = PositionCodec::new("abc", PositionEncoding::UTF8); + let range = lsp_types::Range { + start: pos(0, 2), + end: pos(0, 1), + }; + assert_eq!( + codec.range_to_byte_range(range), + Err(PositionCodecError::MalformedRange) + ); + } + + #[test] + fn outgoing_offset_snaps_inside_utf8_sequence_and_clamps_out_of_bounds() { + let codec = PositionCodec::new("é", PositionEncoding::UTF16); + assert_eq!(codec.offset_to_position(1), pos(0, 0)); // inside 'é' + assert_eq!(codec.offset_to_position(99), pos(0, 1)); // clamp to end + } + + #[test] + fn document_end_for_trailing_newline_and_without() { + let with_nl = PositionCodec::new("a\n", PositionEncoding::UTF8); + assert_eq!(with_nl.document_end(), pos(1, 0)); + let without = PositionCodec::new("a\nbc", PositionEncoding::UTF8); + assert_eq!(without.document_end(), pos(1, 2)); + } + + #[test] + fn token_segments_split_multiline_and_skip_newline_units() { + let text = "ab\ncd\r\nef"; + let codec = PositionCodec::new(text, PositionEncoding::UTF8); + // Span covering "b\ncd\r\ne" = bytes 1..8. + let segments = codec.token_segments(TextRange::new(1.into(), 8.into())); + assert_eq!( + segments, + vec![ + TokenSegment { + line: 0, + start_character: 1, + length: 1 + }, + TokenSegment { + line: 1, + start_character: 0, + length: 2 + }, + TokenSegment { + line: 2, + start_character: 0, + length: 1 + }, + ] + ); + } + + #[test] + fn token_segments_lengths_are_encoded_units() { + let text = "😀x"; + let utf16 = PositionCodec::new(text, PositionEncoding::UTF16); + let segments = utf16.token_segments(TextRange::new(0.into(), 5.into())); + assert_eq!( + segments, + vec![TokenSegment { + line: 0, + start_character: 0, + length: 3 + }] + ); + } + + #[test] + fn token_segments_discard_zero_length() { + let text = "a\n\nb"; + let codec = PositionCodec::new(text, PositionEncoding::UTF8); + // Span covering "\n\n" only — the empty middle line yields nothing. + let segments = codec.token_segments(TextRange::new(1.into(), 3.into())); + assert!(segments.is_empty()); + } + + #[test] + fn negotiation_prefers_utf8_and_falls_back_to_utf16() { + let offered = vec![ + lsp_types::PositionEncodingKind::UTF16, + lsp_types::PositionEncodingKind::UTF8, + ]; + assert_eq!( + PositionEncoding::negotiate(Some(&offered)), + PositionEncoding::UTF8 + ); + let utf16_only = vec![lsp_types::PositionEncodingKind::UTF16]; + assert_eq!( + PositionEncoding::negotiate(Some(&utf16_only)), + PositionEncoding::UTF16 + ); + assert_eq!(PositionEncoding::negotiate(None), PositionEncoding::UTF16); + // UTF-32 alone is never selected (D4). + let utf32_only = vec![lsp_types::PositionEncodingKind::UTF32]; + assert_eq!( + PositionEncoding::negotiate(Some(&utf32_only)), + PositionEncoding::UTF16 + ); + } +} diff --git a/baml_language/crates/bex_project/src/bex_lsp/request.rs b/baml_language/crates/bex_project/src/bex_lsp/request.rs index 5fbc5e9a5e0..5f939bd255b 100644 --- a/baml_language/crates/bex_project/src/bex_lsp/request.rs +++ b/baml_language/crates/bex_project/src/bex_lsp/request.rs @@ -52,7 +52,7 @@ macro_rules! define_lsp_request_trait { } ),*, other => { - let _ = sender(notif.id, Err(LspError::UnknownErrorCode(format!("request not supported: {}", other)))); + let _ = sender(notif.id, Err(LspError::RequestNotSupported(other.to_string()))); return; } } diff --git a/baml_language/crates/bex_project/src/lib.rs b/baml_language/crates/bex_project/src/lib.rs index eded70c86bc..8be8b3167a5 100644 --- a/baml_language/crates/bex_project/src/lib.rs +++ b/baml_language/crates/bex_project/src/lib.rs @@ -111,6 +111,6 @@ pub use baml_project::{FieldSchema, FieldSchemaField, ParamSchema, TypeSchema}; pub use bex_lsp::{ BackgroundSpawner, BexLsp, FunctionInfo, FunctionKind, FunctionOrigin, LlmCapabilities, LspClientSenderTrait, LspError, PlaygroundNotification, PlaygroundSender, PlaygroundSourceFile, - ProjectDiagnostic, ProjectUpdate, TestExpandError, new_lsp, + PreparedRun, ProjectDiagnostic, ProjectUpdate, TestExpandError, new_lsp, }; pub use fs::{BamlVFS, BulkReadFileSystem, DefaultBulkReadFileSystem, FsPath}; diff --git a/baml_language/crates/bex_project/src/project.rs b/baml_language/crates/bex_project/src/project.rs index 1fd588efe41..d7492b95bb1 100644 --- a/baml_language/crates/bex_project/src/project.rs +++ b/baml_language/crates/bex_project/src/project.rs @@ -1,4 +1,50 @@ -use std::collections::{HashMap, VecDeque}; +//! Project core: one source gate, owned build candidates, and a coherent +//! runtime state with revision-conditional engine commits. +//! +//! # Architecture (docs/design/lsp-latency-and-mutex-fix.md) +//! +//! - **I1 — `SourceRevision` is authoritative.** [`SourceState`] holds the +//! Salsa database, a monotonic revision, and the open-document version map +//! behind one mutex (the *source gate*). Every accepted source-mutation +//! batch advances the revision in the same critical section that mutates +//! the database and version map. A lock-free [`BexProject::observed_revision`] +//! mirror exists for observation (error mapping) only — it never +//! authorizes a commit. +//! - **I2 — background work produces owned, revision-tagged candidates.** +//! [`CompilationOutcome`] / [`DiagnosticCandidate`] / [`EngineCandidate`] +//! are fully owned; no database guard survives candidate creation. +//! - **I3 — engine installation is one atomic conditional commit.** +//! [`BexProject::commit_engine_if_current`] takes the *same* source gate +//! used by mutations, compares revisions, and only then installs. A +//! superseded candidate changes no runtime state and drops quietly +//! (profiling was never activated). +//! - **I4 — runtime identity is coherent.** [`RuntimeState`] replaces the +//! old `(bool, Option>)` currentness flag and separate +//! `TestState`. Currentness is *derived* from +//! `installed.source_revision == source.revision`, so a source mutation +//! makes the engine non-current by definition, not by flag ordering. +//! - **I5 — run and test entry use coherent snapshots.** +//! [`BexProject::prepare_function_run`] and [`BexProject::lease_registry`] +//! validate and capture engine/generation/graph/registry in one +//! source→runtime transaction. +//! - **Phase 0C — bounded request waits.** +//! [`BexProject::read_source_for_request`] retries `try_lock` for up to +//! [`REQUEST_DB_DEADLINE`] instead of failing instantly (the pre-0.14.2 +//! `-32001` burst) or blocking forever. Poison is terminal +//! ([`BexProject::mark_broken`]), never treated as recoverable contention. +//! +//! Lock order (I8): source gate → runtime state → caches. The `run_cfgs` +//! cache is a leaf mutex: it may be taken while the source gate is held +//! ([`BexProject::prepare_function_run`]'s cached-graph fast path), and no +//! source or runtime lock is ever acquired while holding it. + +use std::{ + collections::{HashMap, VecDeque}, + sync::{ + Arc, Mutex, MutexGuard, OnceLock, + atomic::{AtomicU64, Ordering}, + }, +}; use bex_engine::BexEngine; use bex_external_types::Handle; @@ -6,15 +52,414 @@ use sys_ops::SysOps; use crate::RuntimeError; +// --------------------------------------------------------------------------- +// Source revision +// --------------------------------------------------------------------------- + +/// Monotonic revision of the project's sources (I1). +/// +/// Advances exactly once per accepted source-mutation batch: open/change/ +/// close, watched-file refresh, full replacement, file add/remove, and +/// playground edit. Captured into every candidate and compared by the +/// conditional engine commit. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(crate) struct SourceRevision(pub(crate) u64); + +impl std::fmt::Display for SourceRevision { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "r{}", self.0) + } +} + +// --------------------------------------------------------------------------- +// Bounded request wait configuration (Phase 0C) +// --------------------------------------------------------------------------- + +/// Deadline for a request handler waiting on the source gate. Ordinary +/// rebuild holds (measured 156–302ms) fit comfortably; pathological +/// multi-second holds still get a finite escape. +#[cfg(not(target_arch = "wasm32"))] +pub(crate) const REQUEST_DB_DEADLINE: std::time::Duration = std::time::Duration::from_secs(1); + +/// Poll interval while waiting on the source gate. +#[cfg(not(target_arch = "wasm32"))] +pub(crate) const REQUEST_DB_RETRY_INTERVAL: std::time::Duration = + std::time::Duration::from_millis(2); + +/// Waits longer than this are logged (observability for the containment +/// scaffolding; the whole wait mechanism is disposable once shared ingress +/// lands). +#[cfg(not(target_arch = "wasm32"))] +pub(crate) const REQUEST_DB_LOG_THRESHOLD: std::time::Duration = + std::time::Duration::from_millis(50); + +// --------------------------------------------------------------------------- +// Errors +// --------------------------------------------------------------------------- + +/// The project observed a poisoned lock: a writer panicked while holding +/// shared mutable state. The state cannot be trusted, so the project enters +/// a terminal broken state (I7): requests are rejected with an internal +/// error, and no path "clears" the poison and continues. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct ProjectBroken; + +/// Typed outcome of a bounded database read on the request lane (I6/I7). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum DbReadError { + /// The gate stayed contended past the deadline. `revision_changed` + /// reports whether the source revision advanced while waiting: `true` + /// maps to `ContentModified` (the request's view genuinely became + /// stale), `false` to `RequestFailed` (same-revision busy timeout). + Busy { revision_changed: bool }, + /// Terminal poisoned/broken project. + Broken, +} + +// --------------------------------------------------------------------------- +// Owned candidates (I2) +// --------------------------------------------------------------------------- + +/// Diagnostics for one file, captured with the exact open-document version +/// the text was checked at (`None` for closed/disk-only files). +#[derive(Debug, Clone)] +pub(crate) struct DocumentDiagnostics { + pub path: std::path::PathBuf, + pub version: Option, + pub diagnostics: Vec, +} + +/// Owned, revision-tagged diagnostics for a whole project (I2). +/// +/// Spans stay byte-based (`baml_compiler_diagnostics::Diagnostic`); the LSP +/// boundary converts them with the negotiated position codec at publish +/// time. `file_texts` carries every source file's text so related-info +/// spans in other files can be converted without touching the database. +#[derive(Debug, Clone)] +pub(crate) struct DiagnosticCandidate { + pub source_revision: SourceRevision, + pub file_texts: HashMap, + pub documents: Vec, + pub has_errors: bool, +} + +/// Owned compiled program tagged with the revision it was compiled from. +pub(crate) struct CompiledCandidate { + pub source_revision: SourceRevision, + pub program: bex_vm_types::Program, +} + +/// Outcome of compiling the current sources into an owned candidate. +/// Invalid source is a first-class outcome — it carries the diagnostics +/// that must still publish, and requires no engine commit. +pub(crate) enum CompilationOutcome { + /// Boxed: the compiled program dwarfs the other variants. + Ready(Box, DiagnosticCandidate), + BlockedByDiagnostics(DiagnosticCandidate), + /// Bytecode emission failed without a user-visible diagnostic error + /// (an internal compiler failure). Diagnostics still publish. + EmitFailed { + diagnostics: DiagnosticCandidate, + message: String, + }, +} + +/// An unpublished engine built from a [`CompiledCandidate`]. Its profiling +/// lifecycle is inactive; only a winning conditional commit activates it and +/// wraps it in the installed `Arc` (I3). +pub(crate) struct EngineCandidate { + pub source_revision: SourceRevision, + engine: BexEngine, +} + +/// Proof of a winning conditional commit (I3). The installed engine itself +/// lives in [`RuntimeState`]; consumers re-capture it transactionally. +#[derive(Clone)] +pub(crate) struct CommitReceipt { + pub source_revision: SourceRevision, + pub generation: u64, +} + +/// Result of [`BexProject::commit_engine_if_current`]. +pub(crate) enum CommitOutcome { + Committed(CommitReceipt), + /// The source revision advanced past the candidate; nothing changed. + Superseded { + current_revision: SourceRevision, + }, +} + +/// End-to-end outcome of one rebuild attempt ([`BexProject::rebuild_once`]). +pub(crate) enum EngineBuildOutcome { + Committed(CommitReceipt), + BlockedByDiagnostics { + source_revision: SourceRevision, + }, + Failed { + source_revision: SourceRevision, + message: String, + }, + Superseded { + current_revision: SourceRevision, + }, + Broken, +} + +/// A rebuild attempt always yields diagnostics to publish (even for invalid +/// source or engine failure) plus the engine outcome. +pub(crate) struct RebuildReport { + pub diagnostics: Option, + pub engine: EngineBuildOutcome, +} + +/// Collect an owned, revision-tagged diagnostics candidate from a held +/// source guard (I2). Every source file is represented — files with zero +/// diagnostics get an empty entry so stale editor markers clear — and each +/// document records the exact open-document version its text was checked at. +pub(crate) fn collect_diagnostic_candidate(guard: &SourceGuard<'_>) -> DiagnosticCandidate { + let db = guard.db(); + let source_files = db.get_source_files(); + + let mut file_texts: HashMap = HashMap::new(); + let mut documents = Vec::new(); + let mut has_errors = false; + + for file in &source_files { + let file_id = file.file_id(db); + let Some(path) = db.file_id_to_path(file_id).cloned() else { + continue; + }; + let text = file.text(db).clone(); + file_texts.insert(file_id, (path.clone(), text)); + + let diagnostics = baml_lsp2_actions::check_file(db, *file); + has_errors |= diagnostics + .iter() + .any(|d| d.severity == baml_compiler_diagnostics::Severity::Error); + documents.push(DocumentDiagnostics { + version: guard.document_version(&path), + path, + diagnostics, + }); + } + + DiagnosticCandidate { + source_revision: guard.revision(), + file_texts, + documents, + has_errors, + } +} + +// --------------------------------------------------------------------------- +// Source state (the one mutation gate) +// --------------------------------------------------------------------------- + +/// Database + revision + open-document versions behind one mutex (I1). +struct SourceState { + db: baml_project::ProjectDatabase, + revision: SourceRevision, + /// LSP versions for currently open editor documents, keyed by the same + /// path identity the database uses. Updated in the same transaction as + /// the text they describe. + open_documents: HashMap, +} + +/// Read guard over the source gate. Exposes the database plus the revision +/// and document versions captured under the same lock, so callers can build +/// revision-tagged owned results without a second racy lookup. +pub(crate) struct SourceGuard<'a> { + inner: MutexGuard<'a, SourceState>, + #[cfg(not(target_arch = "wasm32"))] + acquired_at: std::time::Instant, + #[cfg(not(target_arch = "wasm32"))] + label: &'static str, +} + +impl SourceGuard<'_> { + #[cfg(not(target_arch = "wasm32"))] + fn new<'a>(inner: MutexGuard<'a, SourceState>, label: &'static str) -> SourceGuard<'a> { + SourceGuard { + inner, + acquired_at: std::time::Instant::now(), + label, + } + } + + #[cfg(target_arch = "wasm32")] + fn new<'a>(inner: MutexGuard<'a, SourceState>, _label: &'static str) -> SourceGuard<'a> { + SourceGuard { inner } + } + + pub(crate) fn db(&self) -> &baml_project::ProjectDatabase { + &self.inner.db + } + + pub(crate) fn revision(&self) -> SourceRevision { + self.inner.revision + } + + pub(crate) fn document_version(&self, path: &std::path::Path) -> Option { + self.inner.open_documents.get(path).copied() + } +} + +#[cfg(not(target_arch = "wasm32"))] +impl Drop for SourceGuard<'_> { + fn drop(&mut self) { + let held = self.acquired_at.elapsed(); + if held >= REQUEST_DB_LOG_THRESHOLD { + tracing::info!("source gate held {}ms [{}]", held.as_millis(), self.label); + } + } +} + +/// One accepted source-mutation batch (I1): database mutation, revision +/// advance, and open-document version updates commit atomically. +#[derive(Default)] +pub(crate) struct SourceBatch { + /// `true` removes database files absent from `sources` (full refresh). + pub replace_all: bool, + pub sources: HashMap, + /// Open-document version updates: `Some(v)` records the version of an + /// open document, `None` marks the document closed (published + /// diagnostics fall back to unversioned). + pub versions: Vec<(crate::fs::FsPath, Option)>, +} + +// --------------------------------------------------------------------------- +// Runtime state (I4) +// --------------------------------------------------------------------------- + +/// The installed engine plus the identity it was built from. +#[derive(Clone)] +pub(crate) struct InstalledEngine { + pub source_revision: SourceRevision, + pub generation: u64, + pub engine: Arc, +} + +/// The installed test registry plus the identity it was collected under. +struct InstalledRegistry { + generation: u64, + collection_epoch: u64, + /// `Some` when the project has tests; `None` when collection completed + /// and found none (`$init_test` absent). Distinguished from "not yet + /// collected", which is `RuntimeState::registry == None`. + handle: Option, + /// Serializes expansion mutations: the registry heap object has exactly + /// one mutation owner (Phase 0A.8). + #[cfg(not(target_arch = "wasm32"))] + expansion_gate: Arc>, +} + +/// Coherent runtime identity (I4). All fields swap together under one lock. +struct RuntimeState { + installed: Option, + /// Allocator for engine generations; only a winning commit consumes one. + next_generation: u64, + /// Epoch for project-derived work (collection/expansion). Bumped by + /// every source mutation and every commit. + derived_epoch: u64, + /// Cancels project-derived work when source moves or an engine commit + /// supersedes it. Never cancels run-owned function/test tokens (D8). + derived_cancel: sys_types::CancellationToken, + /// Fences test-collection installs so two collections on one engine + /// generation cannot complete out of order (Phase 0A.7). + collection_epoch: u64, + registry: Option, +} + +impl RuntimeState { + fn new() -> Self { + Self { + installed: None, + next_generation: 1, + derived_epoch: 0, + derived_cancel: sys_types::CancellationToken::new(), + collection_epoch: 0, + registry: None, + } + } + + /// Cancel derived work and hand out a fresh token. + fn supersede_derived(&mut self) { + self.derived_cancel.cancel(); + self.derived_cancel = sys_types::CancellationToken::new(); + self.derived_epoch += 1; + } +} + +/// Ticket for one test-collection attempt, captured atomically (I5). +pub(crate) struct CollectionTicket { + pub generation: u64, + pub collection_epoch: u64, + pub engine: Arc, + pub cancel: sys_types::CancellationToken, +} + +/// Coherent lease for test-registry work (test run / expansion), captured in +/// one source→runtime transaction. +pub(crate) struct RegistryLease { + pub generation: u64, + /// Epoch the leased registry was collected under. Emission fencing + /// compares this too: a same-generation re-collection replaces the + /// registry object, and results computed against the old one are stale. + pub collection_epoch: u64, + pub engine: Arc, + pub handle: Handle, + pub cancel: sys_types::CancellationToken, + /// One mutation owner per installed registry (native only; WASM is + /// single-threaded and keeps its historical unserialized behavior). + #[cfg(not(target_arch = "wasm32"))] + pub expansion_gate: Arc>, +} + +/// Why a registry lease could not be produced. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RegistryLeaseError { + /// Requested generation is not the installed one, or the installed + /// engine no longer matches current source (D7: never silently run + /// last-known-good). + NeedsCurrentBuild, + /// Collection has not produced a registry for this generation yet. + NoRegistry, + /// Collection completed and the project has no tests. + NoTests, + Broken, +} + +/// Coherent snapshot for launching a function run (I5). The overlay +/// control-flow graph is pinned into the generation-keyed cache as part of +/// preparation; runs resolve it later via +/// [`BexProject::control_flow_graph_for_generation`]. +pub(crate) struct RunSnapshot { + pub generation: u64, + pub engine: Arc, +} + +/// Why a run snapshot could not be produced. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum PrepareRunError { + /// No engine, or the installed engine predates current source (D7). + NeedsCurrentBuild, + Busy, + Broken, +} + +// --------------------------------------------------------------------------- +// Run CFG cache +// --------------------------------------------------------------------------- + /// Prepared control-flow graphs pinned for playground runs, keyed by -/// `(project generation, function name)`. +/// `(engine generation, function name)`. /// -/// A run captures the project generation at launch and resolves its graph +/// A run captures the engine generation at launch and resolves its graph /// overlay against that generation later, possibly after several recompiles. -/// Graphs are built lazily for the function a run actually executes (see -/// [`BexProject::control_flow_graph_for_generation`]) instead of eagerly for -/// every function on every compile — fully-inlined graphs grow with -/// call-site fan-out, so eager per-compile snapshots dominated LSP memory. +/// Graphs are built lazily for the function a run actually executes instead +/// of eagerly for every function on every compile — fully-inlined graphs +/// grow with call-site fan-out, so eager per-compile snapshots dominated LSP +/// memory. const RETAINED_RUN_CFGS: usize = 64; #[derive(Default)] @@ -55,251 +500,773 @@ impl RunCfgCache { } } -pub(crate) struct TestState { - pub generation: u64, - pub cancel: sys_types::CancellationToken, - pub registry: Option, -} - -impl TestState { - fn new() -> Self { - Self { - generation: 0, - cancel: sys_types::CancellationToken::new(), - registry: None, - } - } -} +// --------------------------------------------------------------------------- +// BexProject +// --------------------------------------------------------------------------- pub(crate) struct BexProject { - pub(crate) db: std::sync::Arc>, - sys_ops: std::sync::Arc, - current_bex: std::sync::RwLock<(bool, Option>)>, - /// `(generation, cancel_token, registry)` — generation is bumped on every engine swap; - /// stale async tasks compare their captured generation before storing results. - /// The cancel token is cancelled and replaced on engine swap so in-flight tasks abort. - /// The registry (a `Handle` to a live `testing.TestRegistry` on the heap) is cleared on engine swap. - test_state: std::sync::Arc>, - run_cfgs: std::sync::Mutex, + /// The source gate (I1): database + revision + open-document versions. + source: Mutex, + /// Lock-free mirror of `source.revision`, updated inside the mutation + /// critical section. Observation only (timeout error mapping); never + /// authorizes commits or publication. + observed_revision: AtomicU64, + /// Terminal broken marker (poisoned lock observed). Set once; never + /// cleared (I8: clearing mutex poison is not recovery). + broken: OnceLock, + sys_ops: Arc, + runtime: Mutex, + run_cfgs: Mutex, } impl BexProject { - pub(crate) fn try_lock_db( - &self, - ) -> Result, crate::LspError> { - self.db.try_lock().map_err(|_| { - crate::LspError::UnknownErrorCode( - "Database mutex is locked (possibly from a prior panic)".to_string(), - ) - }) - } - - pub(crate) fn new(root_path: &vfs::VfsPath, sys_ops: std::sync::Arc) -> Self { + pub(crate) fn new(root_path: &vfs::VfsPath, sys_ops: Arc) -> Self { let mut db = baml_project::ProjectDatabase::new(); db.set_project_root(crate::fs::FsPath::from_vfs(root_path).as_path()); Self { - db: std::sync::Arc::new(std::sync::Mutex::new(db)), + source: Mutex::new(SourceState { + db, + revision: SourceRevision(0), + open_documents: HashMap::new(), + }), + observed_revision: AtomicU64::new(0), + broken: OnceLock::new(), sys_ops, - current_bex: std::sync::RwLock::new((false, None)), - test_state: std::sync::Arc::new(std::sync::Mutex::new(TestState::new())), - run_cfgs: std::sync::Mutex::new(RunCfgCache::default()), + runtime: Mutex::new(RuntimeState::new()), + run_cfgs: Mutex::new(RunCfgCache::default()), } } - /// Update all sources in the project (removes any sources that are not in the new sources) - pub(crate) fn update_all_sources( - &self, - sources: &std::collections::HashMap, - ) { - self.apply_all_sources(sources); + // ── Broken-state handling ──────────────────────────────────────────── - // We don't care about the result here. - // If someone cares, they should get the diagnostics from the diagnostics_by_file method. - let _ = self.update_bex(); + /// Record the terminal broken state (first reason wins) and log it. + fn mark_broken(&self, reason: &str) -> ProjectBroken { + if self.broken.set(reason.to_string()).is_ok() { + log::error!("project entered terminal broken state: {reason}"); + } + ProjectBroken } - /// Apply a full source set to the database (removing sources not present) - /// WITHOUT rebuilding the engine. The engine is marked outdated; callers - /// schedule [`Self::update_bex`] separately (the LSP debounces it off the - /// keystroke path). - pub(crate) fn apply_all_sources( - &self, - sources: &std::collections::HashMap, - ) { - let mut db = self.db.lock().unwrap(); - let mut existing_paths: std::collections::HashSet<_> = - db.non_builtin_file_paths().collect(); - for (path, source) in sources { - db.add_or_update_file(path.as_path(), source); - existing_paths.remove(path.as_path()); + // ── Lock lanes ─────────────────────────────────────────────────────── + + /// Writer lane: blocking acquisition of the source gate. Used by source + /// mutations and background candidate builds (never on a path that must + /// stay responsive under contention). + fn lock_source_blocking(&self) -> Result, ProjectBroken> { + if self.broken.get().is_some() { + return Err(ProjectBroken); } - for path in existing_paths { - db.remove_file(&path); + match self.source.lock() { + Ok(guard) => Ok(guard), + Err(_) => Err(self.mark_broken("source gate poisoned by a panicked writer")), } - drop(db); - self.set_bex_outdated(); } - /// Like [`Self::apply_all_sources`], but never removes sources. - pub(crate) fn apply_some_sources( - &self, - sources: &std::collections::HashMap, - ) { - let mut db = self.db.lock().unwrap(); - for (path, source) in sources { - db.add_or_update_file(path.as_path(), source); + fn lock_runtime(&self) -> Result, ProjectBroken> { + if self.broken.get().is_some() { + return Err(ProjectBroken); + } + match self.runtime.lock() { + Ok(guard) => Ok(guard), + Err(_) => Err(self.mark_broken("runtime state poisoned by a panicked writer")), } - drop(db); - self.set_bex_outdated(); } - pub(crate) fn take(self) -> Result, RuntimeError> { - let current_bex = self.current_bex.into_inner().unwrap(); - if current_bex.0 { - #[allow(clippy::redundant_clone)] - current_bex.1.clone().ok_or(RuntimeError::Compilation { - message: "No bex".to_string(), - }) + /// Request lane (Phase 0C): bounded wait on the source gate — 1s + /// deadline, 2ms retry interval, 50ms wait-log threshold. On WASM the + /// server is single-threaded, so this is a single non-waiting attempt. + pub(crate) fn read_source_for_request(&self) -> Result, DbReadError> { + if self.broken.get().is_some() { + return Err(DbReadError::Broken); + } + + #[cfg(target_arch = "wasm32")] + { + match self.source.try_lock() { + Ok(inner) => Ok(SourceGuard::new(inner, "request")), + Err(std::sync::TryLockError::WouldBlock) => Err(DbReadError::Busy { + revision_changed: false, + }), + Err(std::sync::TryLockError::Poisoned(_)) => { + self.mark_broken("source gate poisoned by a panicked writer"); + Err(DbReadError::Broken) + } + } + } + + #[cfg(not(target_arch = "wasm32"))] + { + let revision_at_start = self.current_revision(); + let started = std::time::Instant::now(); + loop { + match self.source.try_lock() { + Ok(inner) => { + let waited = started.elapsed(); + if waited >= REQUEST_DB_LOG_THRESHOLD { + tracing::info!( + "request waited {}ms for the source gate", + waited.as_millis() + ); + } + return Ok(SourceGuard::new(inner, "request")); + } + Err(std::sync::TryLockError::WouldBlock) => { + if started.elapsed() >= REQUEST_DB_DEADLINE { + let busy = self.classify_request_timeout(revision_at_start); + log::warn!( + "request timed out after {}ms waiting for the source gate \ + ({busy:?})", + started.elapsed().as_millis() + ); + return Err(busy); + } + std::thread::sleep(REQUEST_DB_RETRY_INTERVAL); + } + Err(std::sync::TryLockError::Poisoned(_)) => { + self.mark_broken("source gate poisoned by a panicked writer"); + return Err(DbReadError::Broken); + } + } + } + } + } + + /// Classify a request-lane timeout (Phase 0C): a revision that moved + /// while the request waited means the result would have been stale + /// anyway (`ContentModified` at the LSP boundary); an unchanged revision + /// is plain congestion (`RequestFailed`). + #[cfg(not(target_arch = "wasm32"))] + fn classify_request_timeout(&self, revision_at_start: SourceRevision) -> DbReadError { + DbReadError::Busy { + revision_changed: self.current_revision() != revision_at_start, + } + } + + /// Loop lane: one non-waiting attempt. `Ok(None)` means "busy, skip me" + /// (only `WouldBlock`); poison is terminal, never conflated with busy + /// (I6). + pub(crate) fn read_source_nowait(&self) -> Result>, ProjectBroken> { + if self.broken.get().is_some() { + return Err(ProjectBroken); + } + match self.source.try_lock() { + Ok(inner) => Ok(Some(SourceGuard::new(inner, "nowait"))), + Err(std::sync::TryLockError::WouldBlock) => Ok(None), + Err(std::sync::TryLockError::Poisoned(_)) => { + Err(self.mark_broken("source gate poisoned by a panicked writer")) + } + } + } + + // ── Source mutation (I1) ───────────────────────────────────────────── + + /// Apply one source-mutation batch. Database mutation, revision advance, + /// and document-version updates happen under the source gate; runtime + /// invalidation (derived-work cancellation, epoch bumps) happens in the + /// same transaction before the gate is released. + pub(crate) fn mutate_sources( + &self, + batch: SourceBatch, + ) -> Result { + #[cfg(not(target_arch = "wasm32"))] + let mutate_started = std::time::Instant::now(); + let mut source = self.lock_source_blocking()?; + + if batch.replace_all { + let mut existing_paths: std::collections::HashSet<_> = + source.db.non_builtin_file_paths().collect(); + for (path, text) in &batch.sources { + source.db.add_or_update_file(path.as_path(), text); + existing_paths.remove(path.as_path()); + } + for path in existing_paths { + source.db.remove_file(&path); + } } else { - Err(RuntimeError::Compilation { - message: "Bex is outdated".to_string(), - }) + for (path, text) in &batch.sources { + source.db.add_or_update_file(path.as_path(), text); + } + } + + for (path, version) in batch.versions { + let key = path.as_path().to_path_buf(); + match version { + Some(v) => { + source.open_documents.insert(key, v); + } + None => { + source.open_documents.remove(&key); + } + } } + + source.revision = SourceRevision(source.revision.0 + 1); + let revision = source.revision; + self.observed_revision.store(revision.0, Ordering::Release); + + // Same-transaction runtime invalidation (I4): derived work for the + // previous engine state is superseded immediately; the installed + // engine becomes non-current by revision comparison. Run-owned + // function/test tokens are untouched (D8). + { + let mut runtime = self.lock_runtime()?; + runtime.supersede_derived(); + runtime.collection_epoch += 1; + } + + #[cfg(not(target_arch = "wasm32"))] + { + let held = mutate_started.elapsed(); + if held >= REQUEST_DB_LOG_THRESHOLD { + tracing::info!("source gate held {}ms [mutate_sources]", held.as_millis()); + } + } + + Ok(revision) } - /// Returns the shared test state (generation, cancel token, registry). - /// Callers capture `generation` and clone `cancel` before starting async work. - pub(crate) fn test_state(&self) -> std::sync::Arc> { - self.test_state.clone() + /// Latest committed source revision, readable without any lock (I1). + pub(crate) fn current_revision(&self) -> SourceRevision { + SourceRevision(self.observed_revision.load(Ordering::Acquire)) } - pub(crate) fn is_bex_current(&self) -> bool { - let current_bex = self.current_bex.read().unwrap(); - current_bex.0 + // ── Candidate construction (I2) ────────────────────────────────────── + + /// Capture the current revision and compile an owned + /// [`CompilationOutcome`]. The source gate is held only while reading + /// the database; the returned outcome is fully detached. + pub(crate) fn compile_outcome(&self) -> Result { + let guard = SourceGuard::new(self.lock_source_blocking()?, "compile_outcome"); + Ok(Self::compile_outcome_locked(&guard)) } - pub(crate) fn current_generation(&self) -> u64 { - self.test_state.lock().unwrap().generation + fn compile_outcome_locked(guard: &SourceGuard<'_>) -> CompilationOutcome { + let diagnostics = collect_diagnostic_candidate(guard); + if diagnostics.has_errors { + return CompilationOutcome::BlockedByDiagnostics(diagnostics); + } + match guard.db().get_bytecode() { + Ok(program) => CompilationOutcome::Ready( + Box::new(CompiledCandidate { + source_revision: guard.revision(), + program, + }), + diagnostics, + ), + Err(e) => CompilationOutcome::EmitFailed { + diagnostics, + message: e.to_string(), + }, + } } - /// Return the prepared control-flow graph for `function_name` as of the - /// given project generation. + /// Compute an owned diagnostics candidate without waiting (fence lane). + /// `Ok(None)` is `Busy` — the caller retains its last publication and + /// schedules a trailing retry. Only the native debounced tail computes + /// diagnostics off-thread; the WASM tail rides `rebuild_once`. + #[cfg(not(target_arch = "wasm32"))] + pub(crate) fn diagnostic_candidate_nowait( + &self, + ) -> Result, ProjectBroken> { + let Some(guard) = self.read_source_nowait()? else { + return Ok(None); + }; + Ok(Some(collect_diagnostic_candidate(&guard))) + } + + /// Construct an engine candidate from a compiled program. Runs `$init` + /// synchronously and candidate-locally, outside the source gate; the + /// profiling lifecycle stays inactive until a winning commit + /// (Phase 0A.9/0A.10). + pub(crate) fn construct_engine_candidate( + &self, + compiled: CompiledCandidate, + ) -> Result { + let engine = BexEngine::new_with_deferred_profiling( + compiled.program, + self.sys_ops.clone(), + Vec::new(), + ) + .map_err(RuntimeError::Engine)?; + Ok(EngineCandidate { + source_revision: compiled.source_revision, + engine, + }) + } + + // ── Conditional commit (I3) ────────────────────────────────────────── + + /// Atomically install `candidate` iff its revision still matches the + /// authoritative source revision. Uses the same gate as source + /// mutations, so commit and mutation are linearized. /// - /// Graphs are built on demand and cached: a hit serves any generation - /// still cached; a miss can only be built while `generation` is still - /// current (the database has moved on otherwise). Playground run launches - /// call this immediately after capturing their generation, which pins the - /// graph for later overlay resolutions of that run. - pub(crate) fn control_flow_graph_for_generation( + /// A superseded candidate changes nothing: no generation, no derived + /// cancellation, no registry/CFG/publication state, no profiling + /// metadata (the candidate drops quietly). + pub(crate) fn commit_engine_if_current( &self, - generation: u64, - function_name: &str, - ) -> Option> { - if let Some(graph) = self - .run_cfgs - .lock() - .unwrap() - .graph(generation, function_name) - { - return Some(graph); + candidate: EngineCandidate, + ) -> Result { + self.commit_engine_if_current_with_hook(candidate, || {}) + } + + /// Internal seam for proving the commit/source-mutation serialization + /// boundary. The hook runs after the authoritative source gate is held + /// but before the revision comparison; production callers use the no-op + /// wrapper above, tests use the hook to place an edit exactly against + /// the conditional commit without sleeps or scheduler timing + /// assumptions. + fn commit_engine_if_current_with_hook( + &self, + candidate: EngineCandidate, + after_source_lock: impl FnOnce(), + ) -> Result { + let source = self.lock_source_blocking()?; + after_source_lock(); + if source.revision != candidate.source_revision { + return Ok(CommitOutcome::Superseded { + current_revision: source.revision, + }); } - if generation != self.current_generation() { - return None; + + // The revision comparison won: activate the candidate's profiling + // lifecycle now — non-awaiting, before the engine becomes reachable. + candidate.engine.activate_profiling(); + let engine = Arc::new(candidate.engine); + + let receipt = { + let mut runtime = self.lock_runtime()?; + let generation = runtime.next_generation; + runtime.next_generation += 1; + runtime.installed = Some(InstalledEngine { + source_revision: candidate.source_revision, + generation, + engine, + }); + // Derived work bound to the previous engine is superseded; the + // registry is cleared atomically with the engine swap. + runtime.supersede_derived(); + runtime.collection_epoch += 1; + runtime.registry = None; + CommitReceipt { + source_revision: candidate.source_revision, + generation, + } + }; + drop(source); + Ok(CommitOutcome::Committed(receipt)) + } + + /// One full rebuild attempt: compile owned outcome → drop the guard → + /// construct the engine candidate (`$init`) → conditional commit. + /// Always returns the diagnostics candidate for fence publication. + pub(crate) fn rebuild_once(&self) -> RebuildReport { + let outcome = match self.compile_outcome() { + Ok(outcome) => outcome, + Err(ProjectBroken) => { + return RebuildReport { + diagnostics: None, + engine: EngineBuildOutcome::Broken, + }; + } + }; + + match outcome { + CompilationOutcome::BlockedByDiagnostics(diagnostics) => { + let source_revision = diagnostics.source_revision; + RebuildReport { + diagnostics: Some(diagnostics), + engine: EngineBuildOutcome::BlockedByDiagnostics { source_revision }, + } + } + CompilationOutcome::EmitFailed { + diagnostics, + message, + } => { + let source_revision = diagnostics.source_revision; + log::warn!("rebuild: bytecode emission failed at {source_revision}: {message}"); + RebuildReport { + diagnostics: Some(diagnostics), + engine: EngineBuildOutcome::Failed { + source_revision, + message, + }, + } + } + CompilationOutcome::Ready(compiled, diagnostics) => { + let source_revision = compiled.source_revision; + let engine = match self.construct_engine_candidate(*compiled) { + Ok(candidate) => match self.commit_engine_if_current(candidate) { + Ok(CommitOutcome::Committed(receipt)) => { + log::info!( + "rebuild: committed engine generation {} at {source_revision}", + receipt.generation + ); + EngineBuildOutcome::Committed(receipt) + } + Ok(CommitOutcome::Superseded { current_revision }) => { + log::info!( + "rebuild: candidate at {source_revision} superseded by \ + {current_revision}; dropping quietly" + ); + EngineBuildOutcome::Superseded { current_revision } + } + Err(ProjectBroken) => EngineBuildOutcome::Broken, + }, + Err(e) => { + log::warn!("rebuild: engine construction failed: {e}"); + EngineBuildOutcome::Failed { + source_revision, + message: e.to_string(), + } + } + }; + RebuildReport { + diagnostics: Some(diagnostics), + engine, + } + } } - let graph = { - let db = self.db.lock().unwrap(); - let graph = db.ast_control_flow_graph(function_name)?; - std::sync::Arc::new( - baml_compiler2_visualization::control_flow::prepare_control_flow_graph_for_visualization( - &graph, - ), - ) + } + + // ── Status / compat accessors ──────────────────────────────────────── + + /// Derived currentness (I4): `installed.source_revision == current + /// revision`, read under both gates. For reporting; work capture uses + /// the transactional APIs below. + pub(crate) fn is_bex_current(&self) -> bool { + let Ok(source) = self.lock_source_blocking() else { + return false; }; - let mut run_cfgs = self.run_cfgs.lock().unwrap(); - // The generation may have moved while building; only cache a graph - // that really reflects the requested generation. - if generation != self.current_generation() { - return None; + let source_revision = source.revision; + let Ok(runtime) = self.lock_runtime() else { + return false; + }; + runtime + .installed + .as_ref() + .is_some_and(|i| i.source_revision == source_revision) + } + + /// Generation of the installed engine (0 when none) — playground compat. + pub(crate) fn current_generation(&self) -> u64 { + self.lock_runtime() + .ok() + .and_then(|rt| rt.installed.as_ref().map(|i| i.generation)) + .unwrap_or(0) + } + + /// Latest installed engine regardless of currentness (playground render + /// paths that tolerate a stale engine; run launch uses + /// [`Self::prepare_function_run`] instead). + pub(crate) fn get_bex(&self) -> Result, RuntimeError> { + let runtime = self + .lock_runtime() + .map_err(|ProjectBroken| RuntimeError::Compilation { + message: "project is in a broken state".to_string(), + })?; + runtime + .installed + .as_ref() + .map(|i| i.engine.clone()) + .ok_or(RuntimeError::Compilation { + message: "No bex has been created yet".to_string(), + }) + } + + /// Consume the project, returning the engine iff it matches current + /// source (embedding API used by [`crate::new`]). + pub(crate) fn take(self) -> Result, RuntimeError> { + if !self.is_bex_current() { + return Err(RuntimeError::Compilation { + message: "Bex is outdated".to_string(), + }); } - run_cfgs.insert(generation, function_name, graph.clone()); - Some(graph) + self.get_bex() } - pub(crate) fn get_bex(&self) -> Result, RuntimeError> { - let current_bex = self.current_bex.read().unwrap(); - current_bex.1.clone().ok_or(RuntimeError::Compilation { - message: "No bex has been created yet".to_string(), - }) + /// Full replacement + synchronous rebuild (embedding API and the WASM + /// LSP path, which has no background scheduler). + pub(crate) fn update_all_sources(&self, sources: &HashMap) { + let batch = SourceBatch { + replace_all: true, + sources: sources.clone(), + versions: Vec::new(), + }; + if self.mutate_sources(batch).is_err() { + return; + } + let _ = self.rebuild_once(); } - fn get_bytecode(&self) -> Result { - let db = self.db.try_lock().map_err(|_| RuntimeError::Compilation { - message: "Database mutex is locked (possibly from a prior panic)".to_string(), - })?; - db.get_bytecode().map_err(|e| RuntimeError::Compilation { - message: e.to_string(), - }) + // ── Test collection (Phase 0A.7) ───────────────────────────────────── + + /// Begin a test-collection attempt against the installed engine. + /// Returns `None` when there is no current engine to collect against. + /// Captures engine, generation, fresh derived token, and a new + /// collection epoch atomically, superseding any in-flight collection. + pub(crate) fn begin_test_collection(&self) -> Result, ProjectBroken> { + let source = self.lock_source_blocking()?; + let source_revision = source.revision; + let mut runtime = self.lock_runtime()?; + let Some(installed) = runtime.installed.clone() else { + return Ok(None); + }; + if installed.source_revision != source_revision { + // Stale engine: collection would be derived work for a dead + // revision (D8 cancels stale tree maintenance). + return Ok(None); + } + runtime.supersede_derived(); + runtime.collection_epoch += 1; + let ticket = CollectionTicket { + generation: installed.generation, + collection_epoch: runtime.collection_epoch, + engine: installed.engine, + cancel: runtime.derived_cancel.clone(), + }; + drop(runtime); + drop(source); + Ok(Some(ticket)) } - fn set_bex_outdated(&self) { - let mut current_bex = self.current_bex.write().unwrap(); - current_bex.0 = false; + /// Install a collected registry iff the ticket still matches the + /// installed generation and collection epoch. Returns `false` (emit + /// nothing) for stale results — the ABA fence. + pub(crate) fn install_collected_registry( + &self, + ticket: &CollectionTicket, + handle: Option, + ) -> Result { + let mut runtime = self.lock_runtime()?; + let matches = runtime + .installed + .as_ref() + .is_some_and(|i| i.generation == ticket.generation) + && runtime.collection_epoch == ticket.collection_epoch; + if !matches { + return Ok(false); + } + runtime.registry = Some(InstalledRegistry { + generation: ticket.generation, + collection_epoch: ticket.collection_epoch, + handle, + #[cfg(not(target_arch = "wasm32"))] + expansion_gate: Arc::new(tokio::sync::Mutex::new(())), + }); + Ok(true) } - fn set_current_bex(&self, bex: BexEngine) { - let mut current_bex = self.current_bex.write().unwrap(); - current_bex.1 = Some(std::sync::Arc::new(bex)); - current_bex.0 = true; - // Cancel in-flight tasks, bump generation, and clear stale test registry - let mut state = self.test_state.lock().unwrap(); - state.cancel.cancel(); - state.generation += 1; - state.cancel = sys_types::CancellationToken::new(); - state.registry = None; + /// Emission fence for collection results (0A.7): `true` while the + /// ticket's engine generation and collection epoch are still installed. + pub(crate) fn collection_ticket_is_current(&self, ticket: &CollectionTicket) -> bool { + let Ok(runtime) = self.lock_runtime() else { + return false; + }; + runtime + .installed + .as_ref() + .is_some_and(|i| i.generation == ticket.generation) + && runtime.collection_epoch == ticket.collection_epoch } - pub(crate) fn update_bex(&self) -> Result<(), RuntimeError> { - self.set_bex_outdated(); + /// Emission fence for expansion results: `true` while the lease's + /// engine generation is still installed *and* the leased registry object + /// is still the installed one (a same-generation re-collection replaces + /// the registry, making results against the old object stale). + pub(crate) fn registry_lease_is_current(&self, lease: &RegistryLease) -> bool { + let Ok(runtime) = self.lock_runtime() else { + return false; + }; + runtime + .installed + .as_ref() + .is_some_and(|i| i.generation == lease.generation) + && runtime.registry.as_ref().is_some_and(|r| { + r.generation == lease.generation && r.collection_epoch == lease.collection_epoch + }) + } - // Skip bytecode generation if there are any diagnostic errors. - { - let db = self.db.lock().unwrap(); - let diagnostics = baml_project::collect_compiler2_diagnostics(&db); - let has_errors = diagnostics - .iter() - .any(|d| d.severity == baml_compiler_diagnostics::Severity::Error); - if has_errors { - log::info!("update_bex: skipping — diagnostic errors present"); - return Err(RuntimeError::Compilation { - message: "Cannot generate bytecode: diagnostic errors present".to_string(), + /// The installed engine iff its generation matches (no currency + /// requirement): the cancel-targeting lookup for already-launched runs + /// (D8). Runs pin their own engine handle at launch; this covers + /// adapters that only kept the generation. + pub(crate) fn engine_for_generation(&self, generation: u64) -> Option> { + let runtime = self.lock_runtime().ok()?; + runtime + .installed + .as_ref() + .filter(|i| i.generation == generation) + .map(|i| i.engine.clone()) + } + + /// Coherent lease for registry work (test run / expansion): validates + /// `generation` against the installed engine, requires the engine to + /// match current source (D7), and captures the registry handle plus its + /// expansion gate in one transaction. + pub(crate) fn lease_registry( + &self, + generation: u64, + ) -> Result { + let source = match self.lock_source_blocking() { + Ok(g) => g, + Err(ProjectBroken) => return Err(RegistryLeaseError::Broken), + }; + let source_revision = source.revision; + let runtime = match self.lock_runtime() { + Ok(g) => g, + Err(ProjectBroken) => return Err(RegistryLeaseError::Broken), + }; + let Some(installed) = runtime.installed.as_ref() else { + return Err(RegistryLeaseError::NeedsCurrentBuild); + }; + if installed.generation != generation || installed.source_revision != source_revision { + return Err(RegistryLeaseError::NeedsCurrentBuild); + } + let Some(registry) = runtime.registry.as_ref() else { + return Err(RegistryLeaseError::NoRegistry); + }; + debug_assert_eq!(registry.generation, generation); + let Some(handle) = registry.handle.clone() else { + return Err(RegistryLeaseError::NoTests); + }; + let lease = RegistryLease { + generation, + collection_epoch: registry.collection_epoch, + engine: installed.engine.clone(), + handle, + cancel: runtime.derived_cancel.clone(), + #[cfg(not(target_arch = "wasm32"))] + expansion_gate: registry.expansion_gate.clone(), + }; + drop(runtime); + drop(source); + Ok(lease) + } + + // ── Run preparation (I5) ───────────────────────────────────────────── + + /// Prepare a function run: one transaction that validates the installed + /// engine against current source (D7), captures the engine + generation, + /// and — when `overlay_function` is set — obtains the pinned + /// control-flow graph for that generation. + /// + /// Graph building holds only the serialized source gate (never runtime + /// or active-run locks); the cached-graph fast path briefly takes the + /// leaf `run_cfgs` mutex while the source gate is held, and the + /// generation-keyed CFG cache is populated only after all guards are + /// released (I5 ordering). + pub(crate) fn prepare_function_run( + &self, + overlay_function: Option<&str>, + ) -> Result { + // Fast path: cached graph + coherent validation without building. + let cached_graph = |generation: u64| -> Option< + std::sync::Arc, + > { + let name = overlay_function?; + self.run_cfgs.lock().ok()?.graph(generation, name) + }; + + let source = match self.read_source_for_request() { + Ok(g) => g, + Err(DbReadError::Busy { .. }) => return Err(PrepareRunError::Busy), + Err(DbReadError::Broken) => return Err(PrepareRunError::Broken), + }; + let source_revision = source.revision(); + let installed = { + let runtime = match self.lock_runtime() { + Ok(g) => g, + Err(ProjectBroken) => return Err(PrepareRunError::Broken), + }; + let Some(installed) = runtime.installed.clone() else { + return Err(PrepareRunError::NeedsCurrentBuild); + }; + installed + }; + if installed.source_revision != source_revision { + return Err(PrepareRunError::NeedsCurrentBuild); + } + + // While the source gate is held, commits cannot interleave, so a + // graph built here reflects exactly `installed`'s revision. + let mut built_graph = None; + if let Some(name) = overlay_function { + if cached_graph(installed.generation).is_none() { + built_graph = source.db().ast_control_flow_graph(name).map(|g| { + ( + installed.generation, + name.to_string(), + std::sync::Arc::new( + baml_compiler2_visualization::control_flow::prepare_control_flow_graph_for_visualization(&g), + ), + ) }); } } + drop(source); - let bytecode = match self.get_bytecode() { - Ok(bc) => bc, - Err(e) => { - log::warn!("update_bex: get_bytecode failed: {e}"); - return Err(RuntimeError::Compilation { - message: format!("get_bytecode failed: {e}"), - }); + // Populate the generation-keyed cache only after the source/runtime + // guards are released (I5 step 3). + if let Some((generation, name, graph)) = built_graph { + if let Ok(mut cache) = self.run_cfgs.lock() { + cache.insert(generation, &name, graph); } - }; - let runtime = match BexEngine::new(bytecode, self.sys_ops.clone(), Vec::new()) { - Ok(r) => r, - Err(e) => { - log::warn!("update_bex: BexEngine::new failed: {e}"); - return Err(RuntimeError::Engine(e)); + } + + Ok(RunSnapshot { + generation: installed.generation, + engine: installed.engine, + }) + } + + /// Return the prepared control-flow graph for `function_name` as of the + /// given engine generation. + /// + /// A cache hit serves any generation still retained; a miss can only be + /// built while `generation` is still the installed, current engine (the + /// database has moved on otherwise). Active-run overlays resolve from + /// graphs pinned at launch by [`Self::prepare_function_run`]; this is + /// the fallback for overlays whose run outlived the cache. + pub(crate) fn control_flow_graph_for_generation( + &self, + generation: u64, + function_name: &str, + ) -> Option> { + if let Ok(cache) = self.run_cfgs.lock() { + if let Some(graph) = cache.graph(generation, function_name) { + return Some(graph); } - }; - self.set_current_bex(runtime); - log::info!("update_bex: success"); - Ok(()) + } + + let source = self.read_source_for_request().ok()?; + { + let runtime = self.lock_runtime().ok()?; + let installed = runtime.installed.as_ref()?; + if installed.generation != generation || installed.source_revision != source.revision() + { + return None; + } + } + let graph = source.db().ast_control_flow_graph(function_name)?; + let graph = std::sync::Arc::new( + baml_compiler2_visualization::control_flow::prepare_control_flow_graph_for_visualization( + &graph, + ), + ); + drop(source); + + if let Ok(mut cache) = self.run_cfgs.lock() { + cache.insert(generation, function_name, graph.clone()); + } + Some(graph) } } #[cfg(test)] mod tests { + use sys_native::SysOpsExt as _; + use super::*; #[test] @@ -339,4 +1306,517 @@ mod tests { assert!(cache.graph(8, "Workflow").is_some()); assert_eq!(cache.order.len(), 2); } + + // ── Revision / commit-race tests (I1/I3/I4) ────────────────────────── + + fn test_project() -> BexProject { + let root = vfs::VfsPath::new(vfs::MemoryFS::new()); + BexProject::new(&root, Arc::new(sys_ops::SysOps::native())) + } + + fn batch(files: &[(&str, &str)], versions: &[(&str, Option)]) -> SourceBatch { + SourceBatch { + replace_all: false, + sources: files + .iter() + .map(|(p, t)| (crate::fs::FsPath::from_str(p.to_string()), (*t).to_string())) + .collect(), + versions: versions + .iter() + .map(|(p, v)| (crate::fs::FsPath::from_str(p.to_string()), *v)) + .collect(), + } + } + + const VALID_SOURCE: &str = "function main() -> int {\n 1\n}\n"; + + #[test] + fn mutation_advances_revision_and_tracks_versions_atomically() { + let project = test_project(); + + let r1 = project + .mutate_sources(batch( + &[("/p/a.baml", VALID_SOURCE)], + &[("/p/a.baml", Some(3))], + )) + .unwrap(); + assert_eq!(r1, SourceRevision(1)); + + { + let guard = project.read_source_nowait().unwrap().unwrap(); + assert_eq!(guard.revision(), r1); + assert_eq!( + guard.document_version(std::path::Path::new("/p/a.baml")), + Some(3) + ); + } + + // Closing drops the version in the same batch that advances the + // revision (didClose semantics). + let r2 = project + .mutate_sources(batch(&[], &[("/p/a.baml", None)])) + .unwrap(); + assert_eq!(r2, SourceRevision(2)); + let guard = project.read_source_nowait().unwrap().unwrap(); + assert_eq!( + guard.document_version(std::path::Path::new("/p/a.baml")), + None + ); + } + + #[test] + fn superseded_candidate_commits_nothing_and_current_candidate_wins() { + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + + // Compile an owned candidate at r1… + let CompilationOutcome::Ready(compiled, _) = project.compile_outcome().unwrap() else { + panic!("trivial source should compile"); + }; + assert_eq!(compiled.source_revision, SourceRevision(1)); + + // …then a mutation advances to r2 before the commit. + project + .mutate_sources(batch( + &[("/p/a.baml", "function main() -> int {\n 2\n}\n")], + &[], + )) + .unwrap(); + + let candidate = project.construct_engine_candidate(*compiled).unwrap(); + match project.commit_engine_if_current(candidate).unwrap() { + CommitOutcome::Superseded { current_revision } => { + assert_eq!(current_revision, SourceRevision(2)); + } + CommitOutcome::Committed(_) => panic!("stale candidate must not install"), + } + // Nothing changed: no engine, no generation, not current. + assert_eq!(project.current_generation(), 0); + assert!(!project.is_bex_current()); + assert!(project.get_bex().is_err()); + + // A candidate built from the current revision commits. + let CompilationOutcome::Ready(compiled, _) = project.compile_outcome().unwrap() else { + panic!("trivial source should compile"); + }; + let candidate = project.construct_engine_candidate(*compiled).unwrap(); + match project.commit_engine_if_current(candidate).unwrap() { + CommitOutcome::Committed(receipt) => { + assert_eq!(receipt.source_revision, SourceRevision(2)); + assert_eq!(receipt.generation, 1); + } + CommitOutcome::Superseded { .. } => panic!("current candidate must install"), + } + assert!(project.is_bex_current()); + assert_eq!(project.current_generation(), 1); + + // The next mutation makes the engine non-current by definition (I4) + // and the next winning commit consumes the next generation. + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + assert!(!project.is_bex_current()); + let report = project.rebuild_once(); + match report.engine { + EngineBuildOutcome::Committed(receipt) => assert_eq!(receipt.generation, 2), + _ => panic!("rebuild of valid source should commit"), + } + assert!(project.is_bex_current()); + } + + /// An edit that lands while the commit holds the source gate is + /// serialized after it: the commit installs its (immediately + /// last-known-good) engine, and the queued edit makes it non-current. + /// The rendezvous hook places the edit exactly at the commit boundary — + /// no sleeps or scheduler timing assumptions. + #[test] + fn edit_at_commit_boundary_never_leaves_a_false_current_engine() { + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + let CompilationOutcome::Ready(compiled, _) = project.compile_outcome().unwrap() else { + panic!("trivial source should compile"); + }; + let candidate = project.construct_engine_candidate(*compiled).unwrap(); + + let rendezvous = std::sync::Barrier::new(2); + let (gate_observation_tx, gate_observation_rx) = std::sync::mpsc::sync_channel(1); + + std::thread::scope(|scope| { + let edit = scope.spawn(|| { + rendezvous.wait(); + let commit_holds_source_gate = matches!( + project.source.try_lock(), + Err(std::sync::TryLockError::WouldBlock) + ); + gate_observation_tx + .send(commit_holds_source_gate) + .expect("commit test receiver should stay alive"); + project + .mutate_sources(batch( + &[("/p/a.baml", "function main() -> int {\n 2\n}\n")], + &[], + )) + .unwrap() + }); + + let outcome = project + .commit_engine_if_current_with_hook(candidate, || { + rendezvous.wait(); + assert!( + gate_observation_rx + .recv() + .expect("edit thread should report the gate state"), + "commit must hold the same source gate required by edits" + ); + }) + .unwrap(); + + let CommitOutcome::Committed(receipt) = outcome else { + panic!("commit holding the gate must finish before the queued edit"); + }; + let r2 = edit.join().expect("edit thread should finish"); + assert_eq!(receipt.source_revision, SourceRevision(1)); + assert_eq!(r2, SourceRevision(2)); + }); + + // The installed engine remains last-known-good but is not current, + // so run admission refuses it (D7). + assert_eq!(project.current_generation(), 1); + assert!(!project.is_bex_current()); + assert!(matches!( + project.prepare_function_run(None), + Err(PrepareRunError::NeedsCurrentBuild) + )); + } + + /// The request lane's bounded wait times out with a typed busy error + /// whose classification depends on whether the source revision moved + /// while waiting. The writer releases the gate only after the timeout + /// result is asserted, so the test never races the deadline. + #[test] + fn bounded_request_wait_classifies_timeout_by_revision_change() { + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + + // Gate held with no mutation: the timeout is plain congestion + // (RequestFailed at the LSP boundary, never -32001). + std::thread::scope(|scope| { + let (held_tx, held_rx) = std::sync::mpsc::sync_channel::<()>(0); + let (release_tx, release_rx) = std::sync::mpsc::sync_channel::<()>(0); + let project = &project; + scope.spawn(move || { + let _guard = project.read_source_nowait().unwrap().unwrap(); + held_tx.send(()).expect("test thread should be waiting"); + release_rx.recv().expect("main thread should release us"); + }); + held_rx.recv().expect("holder should signal"); + assert!(matches!( + project.read_source_for_request(), + Err(DbReadError::Busy { + revision_changed: false + }) + )); + release_tx.send(()).expect("holder should be waiting"); + }); + + // A revision that moved during the wait marks the timeout stale + // (ContentModified) rather than congested. + let revision_at_start = project.current_revision(); + project + .mutate_sources(batch( + &[("/p/a.baml", "function main() -> int {\n 2\n}\n")], + &[], + )) + .unwrap(); + assert!(matches!( + project.classify_request_timeout(revision_at_start), + DbReadError::Busy { + revision_changed: true + } + )); + assert!(matches!( + project.classify_request_timeout(project.current_revision()), + DbReadError::Busy { + revision_changed: false + } + )); + } + + #[test] + fn rebuild_with_errors_publishes_diagnostics_without_engine() { + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", "function broken( {")], &[])) + .unwrap(); + + let report = project.rebuild_once(); + let diagnostics = report.diagnostics.expect("diagnostics always publish"); + assert!(diagnostics.has_errors); + assert!(matches!( + report.engine, + EngineBuildOutcome::BlockedByDiagnostics { .. } + )); + assert_eq!(project.current_generation(), 0); + } + + #[test] + fn collection_ticket_is_fenced_by_epoch_and_generation() { + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + let _ = project.rebuild_once(); + + let ticket = project + .begin_test_collection() + .unwrap() + .expect("current engine yields a ticket"); + assert!(project.collection_ticket_is_current(&ticket)); + + // A source mutation supersedes the collection: the ticket is stale, + // its cancel token fires, and installation is refused. + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + assert!(!project.collection_ticket_is_current(&ticket)); + assert!(ticket.cancel.is_cancelled()); + assert!(!project.install_collected_registry(&ticket, None).unwrap()); + } + + #[test] + fn stale_engine_yields_no_collection_ticket() { + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + let _ = project.rebuild_once(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + + assert!(project.begin_test_collection().unwrap().is_none()); + } + + #[test] + fn registry_lease_requires_current_engine_and_installed_registry() { + fn lease_error(project: &BexProject, generation: u64) -> RegistryLeaseError { + match project.lease_registry(generation) { + Ok(_) => panic!("lease unexpectedly granted"), + Err(e) => e, + } + } + + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + + // No engine at all. + assert_eq!( + lease_error(&project, 1), + RegistryLeaseError::NeedsCurrentBuild + ); + + let _ = project.rebuild_once(); + let generation = project.current_generation(); + + // Engine current, but no collection installed yet. + assert_eq!( + lease_error(&project, generation), + RegistryLeaseError::NoRegistry + ); + + // Install a "no tests" registry through the ticket path. + let ticket = project.begin_test_collection().unwrap().unwrap(); + assert!(project.install_collected_registry(&ticket, None).unwrap()); + assert_eq!( + lease_error(&project, generation), + RegistryLeaseError::NoTests + ); + + // A wrong generation is rejected regardless. + assert_eq!( + lease_error(&project, generation + 1), + RegistryLeaseError::NeedsCurrentBuild + ); + + // A mutation invalidates the lease path (D7: no last-known-good). + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + assert_eq!( + lease_error(&project, generation), + RegistryLeaseError::NeedsCurrentBuild + ); + } + + #[test] + fn prepare_function_run_validates_and_pins_generation() { + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + + assert!(matches!( + project.prepare_function_run(None), + Err(PrepareRunError::NeedsCurrentBuild) + )); + + let _ = project.rebuild_once(); + let snapshot = project.prepare_function_run(None).unwrap(); + assert_eq!(snapshot.generation, 1); + + // The engine of a retained generation stays addressable for cancel + // targeting (D8); a superseded generation stops matching once a new + // engine installs. + assert!(project.engine_for_generation(1).is_some()); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + assert!(matches!( + project.prepare_function_run(None), + Err(PrepareRunError::NeedsCurrentBuild) + )); + let _ = project.rebuild_once(); + assert!(project.engine_for_generation(1).is_none()); + assert!(project.engine_for_generation(2).is_some()); + } + + #[test] + fn nowait_read_reports_busy_while_gate_is_held() { + let project = test_project(); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + + let held = project.read_source_nowait().unwrap().unwrap(); + assert!(matches!(project.read_source_nowait(), Ok(None))); + assert!(matches!(project.diagnostic_candidate_nowait(), Ok(None))); + drop(held); + assert!(project.read_source_nowait().unwrap().is_some()); + } + + #[test] + fn poisoned_source_gate_is_terminal_broken_state() { + let project = std::sync::Arc::new(test_project()); + project + .mutate_sources(batch(&[("/p/a.baml", VALID_SOURCE)], &[])) + .unwrap(); + + // Panic while holding the source gate on another thread. + let poisoner = project.clone(); + let _ = std::thread::spawn(move || { + let _guard = poisoner.source.lock().unwrap(); + panic!("poison the source gate"); + }) + .join(); + + // Every lane observes terminal Broken — never Busy, never a clear. + assert!(matches!( + project.read_source_for_request(), + Err(DbReadError::Broken) + )); + assert!(project.read_source_nowait().is_err()); + assert!(project.mutate_sources(batch(&[], &[])).is_err()); + assert!(matches!( + project.prepare_function_run(None), + Err(PrepareRunError::Broken) + )); + assert!(matches!( + project.rebuild_once().engine, + EngineBuildOutcome::Broken + )); + assert!(!project.is_bex_current()); + } + + // ── Scale / phase timings (local dev; prints to stderr) ─────────────── + + fn load_disk_project(root: &std::path::Path) -> Option<(BexProject, usize, usize)> { + if !root.join("baml.toml").exists() { + return None; + } + let vfs_root = vfs::VfsPath::new(vfs::MemoryFS::new()); + let project = BexProject::new(&vfs_root, Arc::new(sys_ops::SysOps::native())); + + let mut sources = HashMap::new(); + let baml_src = root.join("baml_src"); + let scan_root = if baml_src.is_dir() { + baml_src + } else { + root.to_path_buf() + }; + let mut file_count = 0usize; + let mut line_count = 0usize; + for entry in walkdir::WalkDir::new(&scan_root) + .into_iter() + .filter_map(Result::ok) + .filter(|e| e.path().extension().is_some_and(|x| x == "baml")) + { + let text = std::fs::read_to_string(entry.path()).ok()?; + line_count += text.lines().count(); + file_count += 1; + let rel = entry.path().strip_prefix(root).ok()?; + let key = format!("/{}", rel.to_string_lossy().replace('\\', "/")); + sources.insert(crate::fs::FsPath::from_str(key), text); + } + + project + .mutate_sources(SourceBatch { + replace_all: true, + sources, + versions: vec![], + }) + .ok()?; + Some((project, file_count, line_count)) + } + + #[test] + #[ignore = "local scale benchmark; run: cargo test -p bex_project bench_sandbox_scale_timings -- --ignored --nocapture"] + #[allow(clippy::print_stdout)] + fn bench_sandbox_scale_timings() { + let root = std::path::Path::new("/Users/rossir/dev/sandbox"); + let Some((project, files, lines)) = load_disk_project(root) else { + println!("bench_sandbox_scale_timings: skipped (no project at {root:?})"); + return; + }; + println!("bench_sandbox_scale_timings: files={files} lines={lines}"); + + let t0 = std::time::Instant::now(); + let guard = project.read_source_nowait().unwrap().unwrap(); + collect_diagnostic_candidate(&guard); + let diag_cold_ms = t0.elapsed().as_millis(); + drop(guard); + + let t1 = std::time::Instant::now(); + let guard = project.read_source_nowait().unwrap().unwrap(); + collect_diagnostic_candidate(&guard); + let diag_warm_ms = t1.elapsed().as_millis(); + drop(guard); + + let t2 = std::time::Instant::now(); + let _ = project.compile_outcome().unwrap(); + let compile_ms = t2.elapsed().as_millis(); + + let main_rel = "baml_src/main.baml"; + let main_text = std::fs::read_to_string(root.join(main_rel)).unwrap(); + let edited = format!("{main_text}\n// bench-touch\n"); + project + .mutate_sources(batch(&[(&format!("/{main_rel}"), &edited)], &[])) + .unwrap(); + let t3 = std::time::Instant::now(); + let guard = project.read_source_nowait().unwrap().unwrap(); + collect_diagnostic_candidate(&guard); + let diag_incremental_ms = t3.elapsed().as_millis(); + + println!( + " diagnostics_cold={diag_cold_ms}ms diagnostics_warm={diag_warm_ms}ms \ + compile_outcome={compile_ms}ms diagnostics_after_one_file_edit={diag_incremental_ms}ms" + ); + } } diff --git a/docs/design/lsp-latency-and-mutex-fix.md b/docs/design/lsp-latency-and-mutex-fix.md new file mode 100644 index 00000000000..cefa672671e --- /dev/null +++ b/docs/design/lsp-latency-and-mutex-fix.md @@ -0,0 +1,1139 @@ +# BAML LSP latency and mutex regression — implementation-qualified design + +> **Status:** v4 is the implementation contract for Phase 0 and Phase 1. The root-cause +> diagnosis and B1 memoization are ready to execute; the remaining active work has explicit merge +> gates below. All product/behavior decisions are resolved, so the active design is ready for +> phased implementation. Shared Salsa snapshots, cancel-on-write, and the request worker pool are +> deferred. +> +> **Provenance:** v1 was produced 2026-07-08 from a multi-agent investigation against +> `canary` and installed 0.14.0/0.14.1 binaries. v2 incorporated a first independent +> review on 2026-07-09. v3 incorporates a second review by independent engine/runtime, +> protocol/encoding, and client/ingress reviewers against local `canary` at +> `d7bb6f9555405f33200cff4822e79962f7812477`. The local `origin/canary` +> reference was three commits ahead, but those commits did not touch the reviewed paths. +> v4 records the selected D1-D8 behavior, treats `$init` locality as an +> engineering fact rather than a product decision, and defers the unwind-dependent Phase 2. +> The external latency harness was not rerun for v3 or v4. +> +> **Measurement harness:** `/tmp/baml-inlay-repro` (`drive_lsp.py`, +> `measure.py`). Measurements below are from 0.14.1 on a shared machine and carry +> the original ±15% qualification. + +## 0. Implementation status and phase gates + +| Area | Status | Gate | +|---|---|---| +| Root-cause diagnosis | Qualified | Preserve the measured mechanism and rerun the harness after each latency phase. | +| B1 Salsa memoization | Ready | Warm-hit and RSS tests must pass. | +| Phase 0 containment | Engineering gates open | Source revision, conditional commit, coherent runtime state, durable diagnostics, and profiling cleanup must land before the bounded-wait hotfix is complete. | +| Client ownership (A1/A2) | D1 resolved | One outer owner multiplexes nested semantic projects; initialization, selector, watcher, command routing, and server discovery share that boundary. | +| Minimal ingress (B2) | D2-D3 resolved | One active browser session, shared stdio/browser policy, fail-fast excess reads, exactly-once cancellation, and lifecycle/mutation barriers. | +| Debounced tail (B3/B4) | Depends on B2 | Internal timer work must have reserved admission and source-revision fencing. | +| Position correctness (C1/C2) | D4 resolved | Negotiate UTF-8 when offered, otherwise UTF-16; keep a separate fixed-UTF-16 playground contract. | +| Demand-gated runtime (B5) | D5-D8 resolved | Warm the selected project, render preparation immediately, require current source for new Run/Test, and let already-started runs finish pinned. | +| Phase 2 snapshots/workers | Deferred | The abort-profile LSP may not race shared Salsa clones with writes or use Salsa cancellation. Reconsider only through a separate value-qualified proposal. | + +If a compatibility release cannot fit Phase 0's revision and commit invariants, restore the +serialized pre-0.14 rebuild behavior. An unfenced background installer is not an acceptable +hotfix. + +## 1. Root cause and evidence + +### 1.1 Regression mechanism + +PR #3938 (`f2bdbdfc2`, shipped in 0.14.0) moved engine rebuilds off the single +LSP dispatch thread into a 300ms-debounced `spawn_blocking` task. That introduced a +second thread holding `BexProject`'s `std::sync::Mutex` +during the diagnostics gate and full non-incremental bytecode generation. + +Request handlers acquire that mutex through `try_lock_db` +(`bex_project/src/project.rs:87-95`). Any failure—including ordinary contention and +poisoning—is currently serialized as JSON-RPC `-32001` +(`multi_project/mod.rs:80-84`). Before 0.14, rebuilds ran on the one dispatch thread, +so requests queued slowly but never raced the database mutex. That is why the regression +bisects to #3938. + +Two correctness bugs share the mechanism: + +- `get_bytecode` uses `try_lock` and can silently abandon a rebuild + (`project.rs:235-242`, `multi_project/mod.rs:664-669`); +- `diagnostics_by_file` maps contention or poison to an empty map + (`diagnostics.rs:166-173`), which can clear every published diagnostic. + +The v3 review found a more serious engine race: `rebuild_epoch` is only a debounce +ticket. A stale task can install its engine before the scheduler's post-build epoch check. +Two tasks can finish out of order, allowing an older engine to permanently overwrite a newer +one. Source-revision-conditional commit is therefore a Phase 0 blocker, not Phase 2 polish. + +### 1.2 Measured symptoms + +| Metric | @400 functions | @1500 functions | +|---|---:|---:| +| inlayHint p50 while typing | 234-256ms | 1145ms | +| didChange → publishDiagnostics p50 | 45ms | 459ms | +| Post-pause rebuild DB hold | 156-166ms | ~302ms observed; ≲1s bound | +| `-32001` failures per pause | 42-48 | 115 | +| Five idle hints | 68/113/159/209/251ms | — | +| Hint p50 under 10ms spam | 2.1-2.4s; max 4.8s | — | +| Unanswered requests at shutdown | 114-146 | 198 | +| didChanges delivered in 15s | 100/100 | 16/~100 | + +Other verified behavior: + +- `$/cancelRequest` is rejected and the target request still completes; +- per-keystroke diagnostics and playground payload construction run synchronously; +- `build_project_update` takes a blocking DB lock; +- stdio dispatch is serial; +- the browser LSP bridge has a separate unbounded serial dispatcher; +- several playground paths hold the projects-registry lock across database work. + +## 2. Cross-phase correctness invariants + +These are engineering invariants, not product choices. Every phase must preserve them. + +### I1. `SourceRevision` is authoritative + +`BexProject` owns a monotonic source revision. It advances once for every accepted +source-mutation batch: open/change/close, watched-file refresh, full replacement, file +add/remove, and playground edit. Database mutation, revision advance, client URI, open-document +version, and text identity are one transaction. A captured `SourceSnapshot` contains those +values together; diagnostics may not combine database contents from one revision with a version +map captured under another lock. + +The revision and database contents are read or changed under one mutation gate. Lock-free +revision observation may be added for diagnostics, but it cannot authorize commit. +`rebuild_epoch` is renamed or documented as a debounce ticket only: it may suppress +work before it begins, but it may never authorize installation, publication, test collection, +or run launch. + +### I2. Background work produces owned, revision-tagged candidates + +Database reads produce an owned, revision-tagged outcome. Invalid source is a first-class +outcome and does not require a `Program` or engine commit: + +```rust +struct DiagnosticCandidate { + source_revision: SourceRevision, + documents: OwnedVersionedDiagnostics, +} + +struct CompiledCandidate { + source_revision: SourceRevision, + program: bex_vm_types::Program, + diagnostics: DiagnosticCandidate, +} + +struct EngineCandidate { + source_revision: SourceRevision, + engine: BexEngine, // unpublished; profiling lifecycle is inactive + diagnostics: DiagnosticCandidate, +} + +enum CompilationOutcome { + Ready(CompiledCandidate), + BlockedByDiagnostics(DiagnosticCandidate), +} +``` + +`DiagnosticCandidate` publishes through its own source-revision/document-version fence. +Only engine currentness, ready `UpdateProject` state, CFG/test work, and other +engine-derived output require an engine `CommitReceipt`. Only a winning commit activates +profiling and wraps the candidate engine in the installed `Arc`. + +No DB guard, Salsa snapshot, borrowed query result, projects-registry guard, runtime-state +guard, or dedupe-cache guard may survive candidate creation or cross an `await`. + +### I3. Engine installation is one atomic conditional commit + +Source mutation and `commit_engine(candidate)` use the same serialization gate and a +documented lock order. Commit compares the candidate revision with the authoritative current +revision and swaps all identity-bearing runtime fields in one critical section. + +After the revision comparison wins—but before the engine becomes reachable—commit activates the +candidate's profiling lifecycle, registers metadata, wraps it in `Arc`, and installs it. This +activation is non-awaiting and part of the same commit outcome. + +The only successful result is a `CommitReceipt` containing the source revision, +engine generation, and engine handle. A mismatched candidate returns `Superseded`. +A superseded candidate must not: + +- install or mark an engine current; +- increment generation; +- cancel current runtime work; +- clear or install a test registry; +- populate a CFG cache; +- publish diagnostics through the engine path or publish `UpdateProject` (the separately fenced + I2 `DiagnosticCandidate` remains authoritative); +- register profiling metadata, emit `engine_closed`, or create a closed-engine tombstone; +- start test collection. + +A separate atomic revision load followed by a later engine swap is still racy and does not +satisfy this invariant. + +### I4. Runtime identity is coherent + +Replace the split `current_bex`, `TestState`, and mutable currentness flag +with one runtime state: + +```rust +struct RuntimeState { + installed: Option, // source revision + generation + Arc + derived_epoch: u64, + derived_cancel: CancellationToken, + collection_epoch: u64, + registry: Option, // revision + generation + collection epoch + handle +} +``` + +Currentness is derived from +`installed.source_revision == current_source_revision`. A source mutation immediately +makes the installed engine non-current. In the same mutation transaction it increments +`derived_epoch` and `collection_epoch`, cancels the derived token, and marks the installed +registry stale and non-launchable. This invalidates collection/expansion immediately; it does +not cancel the separately owned function/test-run tokens retained by Decision D8. + +A winning engine commit allocates one generation, replaces the engine, installs a fresh derived +token, and clears or replaces the registry atomically. + +### I5. Run and test entry use coherent snapshots + +Replace `project_generation → CFG pin → get_bex_for_project` with one +`prepare_and_register_run` transaction: + +```rust +struct RunLease { + run_id: BoundaryId, + source_revision: SourceRevision, + generation: EngineGeneration, + engine: Arc, + graph: Option>, + test_registry: Option, + cancellation: RunCancellationHandle, +} +``` + +Run preparation is ordered as follows: + +1. If the target needs a graph, obtain or build an owned + `CfgCandidate { source_revision, generation, graph: Arc }` while holding + at most the serialized live-database lane—never runtime, active-run, or cache locks—and then + drop the database guard. +2. Enter the source/runtime registration transaction. Revalidate the CFG candidate, confirm that + the installed engine still matches current source, capture the engine and—for a test run—the + registry handle, then insert the `RunLease` before releasing the gates. +3. Populate any generation-keyed CFG cache after those guards are released. Active-run overlays + resolve directly from the graph retained by the lease; they never rebuild from the live + database. + +This transaction linearizes edit versus run start: + +- if the edit commits first, preparation returns `NeedsCurrentBuild(new_revision)` and D7 + rebuilds/retries against current source; +- if run registration commits first, the immutable lease owns the old engine state and D8 lets it + continue after the edit. + +Explicit Cancel resolves the lease by `run_id` and targets its retained engine/cancellation +handle, never the project's current engine. One terminal outcome removes the lease; natural +completion, explicit cancellation, and transport/session changes race through the run's own +exactly-once terminal state. + +Test-tree collection and expansion are project-derived work: they atomically capture matching +engine, generation, derived token, registry, and operation epoch, then publish through I9. Their +stale success, empty, cancellation, and error paths emit nothing. + +Function and test execution are run-owned work. Their events and terminal result publish by +`{ run_id, monotonically increasing run_sequence }`, not by project currentness, and remain valid +after the source revision advances. + +### I6. Diagnostics never confuse empty, busy, and broken + +Diagnostics use: + +```rust +enum DiagnosticRead { + Ready(T), + Busy, // only TryLockError::WouldBlock + Poisoned, +} +``` + +An owned result carries its source revision and the exact URI/document version for each open +file. `Busy` preserves the last publication and schedules a trailing retry. +`Poisoned` is an internal failure and never becomes an empty diagnostic set. + +### I7. LSP errors are typed at one serialization boundary + +Each connection owns an `LspSession`: a fresh session ID/epoch, lifecycle state, immutable +negotiated capabilities after initialize, outstanding-request registry, open-document ownership, +and outbound sink. Project/workspace state is shared separately. Dispatch carries an opaque +`ResponseToken { session_id, request_id }`; a bare request ID is never sufficient. + +Expand `LspClientSenderTrait` (or replace it with `LspSessionSender`) so +`send_response(ResponseToken, TypedResult)` is the only request error-code mapping and +response-routing boundary for both transports. It also owns transport-level protocol errors with +an optional/null ID. + +| Condition | Code | +|---|---:| +| Invalid JSON/framing | `ParseError` (`-32700`) | +| Invalid JSON-RPC request/envelope | `InvalidRequest` (`-32600`) | +| Request before initialize | `ServerNotInitialized` (`-32002`) | +| Unsupported method | `MethodNotFound` (`-32601`) | +| Malformed params/position/range | `InvalidParams` (`-32602`) | +| Unwind-enabled panic, poison, violated invariant | `InternalError` (`-32603`) | +| Explicit client cancellation wins response ownership | `RequestCanceled` (`-32800`) | +| External request invalidated by an applied source revision | `ContentModified` (`-32801`) | +| Same-revision busy timeout, overload, or unavailable valid request | `RequestFailed` (`-32803`) | + +`UnknownErrorCode` (`-32001`) is not a fallback. +`ServerCancelled` (`-32802`) remains unused unless a particular method and +client capability explicitly support it. + +This table does not imply that an abort-profile panic can be serialized. If a poisoned project is +observable in an unwind-enabled development/test build, it enters terminal `ProjectBroken`: +the current request receives +`InternalError`, later project requests are rejected, and the LSP schedules controlled +exit/restart. It never clears poison and continues. A true release panic aborts by current product +policy; v4 does not add in-process panic recovery. + +A later `didChange` waiting in ingress is not proof that an earlier request is +invalid. Queue lookahead may not synthesize `ContentModified`. + +### I8. Lock and publication discipline + +The canonical order is: + +1. clone project handles out of the projects registry; +2. acquire the source/database gate; +3. acquire runtime state only for brief capture or commit; +4. acquire the active-run registry only after runtime state when atomically registering a run; +5. touch the reserved publication mailbox only through a nonblocking enqueue while the source + gate is held; it may never wait for or call the transport; +6. acquire caches only after source/runtime guards are released; +7. serialize and send with no shared lock held. + +No code catches a panic while retaining a shared mutable guard and then continues as though +the state were repaired. Clearing mutex poison is not recovery from a partially applied write. + +### I9. Project-derived publication is a conditional transaction + +Every asynchronous project-derived publication—diagnostics, catalog/runtime state, CFG, test-tree +collection, or expansion—carries `session_id`, `project_id` plus project incarnation, +`source_revision`, and, when applicable, engine generation plus derived/collection epoch. Its +final currentness comparison and insertion into a reserved, nonblocking per-project outbound +mailbox happen while holding the same mutation gate used by source changes. Sending/serialization +happens later with no shared lock held. + +The source-mutation transaction advances that mailbox's invalidation watermark. The sequencer +drops an envelope below the watermark even if it was queued before the mutation, and the +playground frontend independently rejects payloads older than its latest project revision/epoch. +LSP diagnostics additionally carry their exact document version. This closes the check-then-send +race without holding a project lock across transport I/O. + +Run-owned function/test events are explicitly outside this watermark. They publish through the +run lease's immutable `run_id`, ordered sequence, and exactly-once terminal state, so a pinned run +can finish after its project source changes without allowing stale project state to reappear. + +## 3. Phase 0 — correctness and compatibility containment + +### Phase 0A. Source revision, candidates, and coherent runtime state + +This lands before bounded request waiting. + +1. Add authoritative `SourceRevision` to the database mutation gate. +2. Introduce the coherent `RuntimeState` and remove independently mutable + currentness. +3. Split rebuild into: + + ```text + capture revision R + compile owned CompilationOutcome + → drop database guard + → publish DiagnosticCandidate(R) through the diagnostics fence + → if Ready, construct EngineCandidate(R) + → commit_engine_if_current(R) + → only CommitReceipt may publish ready runtime state or collect tests + ``` + +4. Treat the debounce ticket only as a pre-work optimization. +5. Make `ensure_engine_current` single-flight by project and revision and return a + coherent receipt/snapshot, never `()` followed by unrelated engine reads. +6. Replace run launch with I5's atomic `prepare_and_register_run`. +7. Capture engine/registry/test identity atomically and add a collection epoch so two + collections on one engine generation cannot complete out of order. +8. Serialize expansion mutations per installed registry; the registry handle has one mutation + owner. +9. Keep candidate profiling inactive throughout construction. Only the winning conditional commit + registers metadata and activates the engine's profiling lifecycle immediately before install. + Failed or superseded candidates drop quietly: no `engine_closed` observer notification and no + permanent closed-engine tombstone. +10. Run synchronous `$init` as ordinary candidate construction outside the source mutation gate. + It evaluates top-level bindings into that candidate's owned globals; events are dropped and + async/sys-op yields fail initialization. It is deterministic and candidate-local, so a stale + candidate may complete `$init` and then be rejected by conditional commit. + +**Phase 0A merge gate** + +- forced A/R1 and B/R2 builds finish in both orders; only R2 may remain installed; +- final invalid R2 while A/R1 is constructing may not install R1; +- invalid R2 publishes its versioned diagnostics without requiring an engine commit; +- a rejected candidate changes no generation/cancel/registry/CFG/publication state; +- run launch returns one engine/generation/CFG revision; +- collection/expansion stale success and error paths emit nothing; +- failed and discarded candidates leave no profiling metadata, file, ring, or observer state. + +### Phase 0B. Durable, versioned diagnostics + +Store open editor documents as `{ client_uri, version, text }`; do not discard LSP +versions. + +Each project owns a latest-revision diagnostics state machine: + +```text +source mutation → mark latest revision dirty; schedule trailing attempt +Ready(current revision) → conditionally publish; clear dirty +Ready(stale revision) → discard; retain latest dirty +Busy → publish nothing; retry with bounded backoff +Poisoned → publish nothing; surface internal failure +``` + +“Clear dirty” is compare-and-clear: it succeeds only when +`dirty_revision == result.source_revision`. If a newer mutation wins between computation and +publication, retain/re-arm the newest dirty revision. The final publication uses I9's conditional +outbound transaction rather than a separate check followed by send. + +The trailing attempt is independent of engine success, `$init`, test collection, +playground demand, and further typing. A final invalid edit must eventually publish. + +For open files, `publishDiagnostics.version` is the exact checked document version. +Closed/disk-only files use `None`. A busy full refresh does not update +`last_published_files` and does not clear deleted-file state. + +Phase 0 may reuse the existing background scheduler for the rare busy retry, but the retry is +tagged with the source revision, retries until success or supersession, and always runs even +when engine construction returns invalid source or another terminal error. B3 later moves this +work onto the shared ingress tail. + +**Phase 0B merge gate** + +- hold the DB through the final syntax-error edit, release it, send no further edit, and observe + the latest versioned diagnostics; +- complete v7 diagnostics after v8 arrives and observe no v7 publication; +- mutate to v8 between v7's ready check and dirty clear; v8 remains scheduled and publishes; +- deterministically distinguish ready, busy, and poison; +- engine or `$init` failure cannot suppress diagnostics. + +### Phase 0C. Bounded request wait + +Keep the native 1s deadline, 2ms retry interval, and 50ms wait-log threshold as disposable +compatibility scaffolding: + +- request handlers use `try_lock_db_wait_for_request`; +- workspace-wide loops use `try_lock_db_nowait` and skip only `WouldBlock`; +- poison maps once to `InternalError`, transitions the project to `ProjectBroken`, and schedules + controlled restart; it is never treated as recoverable contention; +- timeout maps to `ContentModified` only when the captured source revision actually + changed; otherwise it maps to `RequestFailed`; +- WASM retains the non-waiting single-thread path; +- formatting and other requests receive typed failures, never silent success/no-op; +- `$/cancelRequest` is accepted without the current error log, though real cancellation + arrives with B2. + +Change background bytecode acquisition from silent `try_lock` failure to the owned +candidate flow in Phase 0A. Blocking acquisition alone is not a publication fence. + +Expected containment effect: ordinary 156-302ms holds fall below the deadline, eliminating the +observed `-32001` burst while preserving a finite escape for pathological multi-second +holds. It does not fix per-keystroke diagnostics cost or request spam backlog. + +### Phase 0D. Compatibility verification + +- rerun harness exp3/exp5: zero `-32001`, hints succeed after pauses; +- rerun exp6: no cancel error log; +- assert same-revision timeout is `-32803`, actual revision invalidation is + `-32801`, explicit cancellation is `-32800`, and poison is `-32603` + followed by terminal restart in unwind-enabled test builds; +- record longest wait and rebuild phase timings in the reporting workspace; +- run the out-of-order engine, final invalid edit, coherent run, collection ABA, and document + version tests above, including invalidation at the final outbound-enqueue barrier without an + engine-generation change. + +## 4. Phase 1 — bounded latency and ownership + +Tracks A and C can begin in parallel after their decision gates. Track B remains ordered. + +**Abort-profile invariant.** The shipped CLI/LSP remains `panic = "abort"`. Therefore no +`ProjectDatabase::clone()` may outlive the source/database gate, no shipped request path may +invoke Salsa local cancellation or `Cancelled::catch`, and no shared Salsa storage clone may +race a write. Live-database work stays on one mutex-serialized lane. Background work may run +concurrently only after it has produced fully owned, Salsa-detached inputs. Explicit request +cancellation may claim/suppress the response but may not unwind an in-flight Salsa query. + +### Track A. One client ownership domain per document + +Distinguish: + +- **ownership root:** a non-overlapping LanguageClient process/event boundary; +- **semantic project root:** the closest ancestor marked by `baml.toml` or + `baml_src/`, used to select a `LiveProject`. + +Every eligible URI matches exactly one ownership root. Ownership is independent of document +open order. Nested semantic projects may share one non-overlapping owner if Decision D1 selects +the recommended topology. + +The extension must expose two distinct resolvers: + +```text +resolveSemanticProjectRoot(uri) = nearest ancestor containing baml.toml or baml_src/ +resolveOwnershipRoot(uri) = outermost marked ancestor in that same ancestor chain +``` + +The second rule is the recommended D1 topology: sibling top-level projects retain separate +clients while nested projects share their outer owner. Both resolvers use one canonical path +identity helper: URI decode, absolute/component normalization, resolution of symlinks through the +nearest existing ancestor, and filesystem-aware case normalization. Preserve the client URI +separately for publication. The walk ends at the filesystem/volume root; if it finds no marker, +the file has no owner under the current standalone-file contract. + +Overlapping VS Code workspace folders do not create competing owners; the canonical marker chain +does. Adding or removing a marker invalidates both resolutions. One extension-level ownership +coordinator serializes migration: detach routing from the old owner, stop it if unused, then attach +or start the new owner. No document or command may be routed to two clients during migration. + +#### A1. Complete VS Code ownership scoping + +For each ownership root: + +- construct a synthetic `vscode.WorkspaceFolder`; +- set `LanguageClientOptions.workspaceFolder`; +- use one `vscode.RelativePattern(ownerFolder, '**/*.baml')` for both + `documentSelector` and `createFileSystemWatcher`; +- never use an interpolated absolute glob. + +Marker topology is not owned by individual language clients. The extension-level ownership +coordinator installs ref-counted exact ancestor-directory watchers for `baml.toml` and +`baml_src` along each active document's canonical chain (plus one scoped topology watcher per +VS Code workspace). Marker create/delete/rename events recompute and serialize the migration +described above; they do not broadcast the same file event through every client. + +Setting only selector and watcher is insufficient: omitted `workspaceFolder` makes +vscode-languageclient populate initialize with every VS Code workspace folder and register +global workspace-folder handling. + +Key the client map by `resolveOwnershipRoot`, not by the nearest semantic root. Update +`startForUri`, `activeClient`, `projectKeyForPath`, restart/log commands, and playground +command routing to use that owner key. Semantic project lookup remains server-side and nearest +marker wins. + +Opening a file outside VS Code workspaces does not attach it to an arbitrary existing client. +If it belongs to a marked BAML project, activation creates one scoped owner for that project. +Unmarked arbitrary standalone files retain today's unsupported server behavior; adding general +standalone support is a separate feature. + +#### A2. Enforce ownership on the server + +Initialization roots are authoritative boundaries: + +- discover semantic projects only inside the declared ownership roots; +- route nested files to the nearest semantic project; +- validate every URI-bearing request, notification, workspace event, and playground command + before project lookup or filesystem access, using component-aware canonical containment; +- ignore and log out-of-bound notifications; return typed `RequestFailed` for out-of-bound + requests or commands; +- preserve multi-root discovery only for browser/CLI callers that explicitly own multiple + disjoint roots; +- do not broaden ownership when a project is added or removed. + +A1 already removes the global startup matrix by narrowing initialize roots. A2 is defense in +depth and covers non-VS-Code entry points; it is not a separate claim that only server work +fixes startup. + +**Ownership gate** + +- nested inner-first and outer-first opens produce the same topology; +- each change, hover, hint, and watcher event reaches one dispatcher; +- N semantic projects refresh N times total, not N²; +- Windows separators and paths with spaces work; +- symlink and case aliases resolve to one owner while publications retain the client URI; +- overlapping workspace folders and marker add/remove migrations never create two owners; +- restart, log, and playground commands route to the same owner as document requests; +- an external marked project gets one dedicated owner; +- an external unmarked file neither broadens a workspace owner nor attaches to all clients. + +### Track B. Server latency work + +#### B1. Memoize annotations and semantic tokens + +Make `annotations()` and `semantic_tokens()` +`#[salsa::tracked(returns(ref))]`, with required `Eq`/`salsa::Update` +derives and handler changes. The existing `file_outline` query is the precedent. + +Gate: + +- first call remains roughly the current cold cost; +- repeated unchanged calls are <2ms; +- the request-spam harness leaves approximately no unanswered requests; +- measure RSS for small, 400-function, 1500-function, and many-file projects. + +#### B2. Minimal shared bounded ingress + +Stdio and `/api/lsp` use the same process-scoped ingress/scheduler implementation. +Transport adapters only parse/frame and route output. + +Each connection constructs the I7 `LspSession` and follows: + +```text +PreInitialize → InitializeResponding → InitializeResponded → Initialized + → ShutdownResponding → Shutdown → Exited +``` + +Only one `initialize` request is accepted. The state advances to +`InitializeResponded` only after that response is ordered onto the owning session's +outbound stream; only then is `initialized` accepted. Duplicate `initialize` receives +`InvalidRequest`; `initialized` in any other state is ignored and logged. Before initialize, +other requests receive `ServerNotInitialized` and notifications are dropped except +`exit`. Between the initialize response and valid `initialized`, other requests likewise +receive `ServerNotInitialized` and notifications are dropped. + +Only one `shutdown` request is accepted in `Initialized`; the state becomes +`Shutdown` only after its response is ordered. Afterward, requests receive +`InvalidRequest`, notifications are ignored except `exit`, and `exit` transitions to +`Exited`. `exit` before a completed shutdown terminates that session abnormally (nonzero for +stdio); after shutdown it is normal. Remove transport-local shutdown/exit shortcuts so both +transports use this state machine. Explicit CLI roots may be recorded before the handshake but +may not trigger position-bearing LSP publication. + +Every admitted non-cancel message receives a sequence number. Phase 1 starts them in FIFO order: +neither a later request nor a later mutation crosses any earlier admitted message. Lifecycle, +source mutation, workspace mutation, and side-effecting requests remain barriers for any future +concurrency proposal. + +The first B2 delivery contains only: + +1. one stdio/browser ingress policy; +2. lifecycle and mutation barriers; +3. exactly-once explicit cancellation; +4. adjacent same-URI FULL-sync `didChange` coalescing; +5. bounded admission with the Decision D3 overload policy. + +`$/cancelRequest` and transport-close control use a separate bounded control path and do +not wait behind normal work. Lifecycle/mutation admission has reserved item and byte capacity. +Read/formatting requests have a smaller explicit budget; overload rejects the newly arriving +request and never evicts an already admitted one. If only non-droppable mutation/command work +remains and its reserve is full, the transport stops reading to apply backpressure. Adjacent FULL +coalescing replaces the old body and releases its byte charge. + +Adjacent coalescing retains the newest full text and document version and never crosses another +URI, request, open, close, save, watched-file event, workspace event, or source mutation. + +Outstanding requests are keyed by `(session_id, request_id)` and have one response +owner: + +```text +Queued → Running → Responded + └──────────────→ Responded(RequestCanceled) +``` + +Cancellable work receives an operation-owned token. Cancellation and the normal result race to +claim the one response slot: queued cancellation removes the work and responds; running +cancellation claims the response and signals the token; a later handler result is discarded. +Repeated/unknown cancellation is idempotent. Under the abort-profile invariant, the token is +checked only at safe handler boundaries; it never triggers Salsa's unwind-based local +cancellation inside a live query. +For a running side-effecting request, cancellation owns only the response; it does not imply that +already-applied effects were rolled back. Each side-effecting method must define whether it is +cancelable before its commit point or non-cancelable once running. After a non-cancelable commit +point, a cancel notification is a no-op and the normal result retains response ownership. + +Do not include queued-change `ContentModified`, interactive priority, arbitrary +latest-wins request cancellation, or request dedupe in B2. Add them only after measurement and +only if they preserve barriers and protocol semantics. + +The browser deletes its per-WebSocket unbounded dispatcher. Responses route only to their owning +session; a lossy global broadcast is not a response transport. Inbound and outbound queues both +have item and serialized-byte budgets. Each admitted request reserves one outbound response slot; +responses are never dropped. Revisioned notifications use reserved/coalescing project mailboxes. +A browser session that cannot drain by a configured write deadline is deterministically closed; +stdio request admission is coupled to writer capacity. No transport may hide an unbounded writer +queue. + +**B2 gate** + +- identical recorded traces through stdio/browser produce identical ordering/outcomes; +- queued/running/repeated cancel races produce exactly one response; +- only adjacent same-URI FULL changes coalesce; +- final source equals the newest admitted change; +- inbound and outbound memory remain bounded under request, mutation, and slow-reader spam; +- lifecycle/control remains admissible under saturation; +- malformed browser/stdio JSON receives null-ID `ParseError` or `InvalidRequest` as + applicable; +- duplicate initialize, wrong-state initialized, shutdown, post-shutdown request, and early/late + exit traces are identical through both transports; +- browser takeover during a request or edit closes old overlays, rejects late old-session output, + tolerates reused request IDs, and negotiates fresh capabilities; +- an unprocessed queued later edit alone never turns an earlier request into + `ContentModified`. + +#### B3. Debounced diagnostics plus the minimum B4 tail move + +After source apply/version bookkeeping, `didChange` only: + +- advances source/diagnostics revision; +- marks runtime non-current; +- resets the engine timer; +- resets a 150ms diagnostics/project-update timer. + +The timer never computes work itself. It admits at most one reserved internal +`RunProjectTail { project, epoch, source_revision }` item. A newer epoch replaces the +older item; normal ingress saturation cannot drop it. + +At dequeue: + +1. discard stale epoch/revision; +2. compute diagnostics; +3. on `Busy`, retain and re-arm with bounded backoff; +4. on poison/panic, surface an internal failure and stop retrying broken state; +5. conditionally enqueue versioned diagnostics through I9's atomic publication transaction; +6. if runtime subscribers exist and this project is demanded, build one owned complete + `ProjectUpdate` candidate; +7. conditionally enqueue that candidate through I9; stale or removed project incarnations emit + nothing. + +This minimum payload move is part of B3. Without it, `didChange` is not apply-only. + +#### B4. Separate editor catalog from runtime snapshots + +**Catalog — `ListProjects`.** It feeds the VS Code extension and runtime clients. +Never receiver-gate it. Sort and change-dedupe routine sends; force a catalog after +initialization and every explicit `requestState`. `requestState` does not by itself +create runtime demand for every catalog entry. + +**Runtime detail — `UpdateProject`.** Rename the trait signal to +`has_runtime_subscribers`. Check it before flattening playground diagnostics, listing +functions/types, serializing, or locking the project database. With zero subscribers, skip all +runtime candidate work while editor diagnostics and catalog continue. + +That global bit answers only whether delivery is possible. B5's per-project demand lease is a +second required predicate for project-specific runtime construction. + +When subscribers exist, compare the complete owned payload: runtime state/currentness, functions, +origins, capabilities, client names, params, types, and diagnostics. A hash may accelerate but +cannot replace equality. New subscribers and explicit state requests force full snapshots and +bypass global dedupe for currently demanded projects. + +`request_playground_state` clones `(root, Arc)` under the registry +lock, drops it, builds owned candidates with short project reads, drops all guards, then sends. + +Install a WebSocket subscription before requesting the forced catalog/snapshots so the response +cannot race the listener. Catalog removal evicts the server's cached payload and increments a +project incarnation; the frontend purges all payload, test, runtime, and selection state for that +incarnation. Remove→re-add may never be suppressed by a warm dedupe cache. + +#### B5. Demand-gated runtime with asynchronous first demand + +`has_runtime_subscribers` is only a transport/payload-delivery gate. Runtime work is demanded per +project. Each browser session owns at most one selected-project lease, changed through an explicit +`EnsureProjectRuntime { project_id, incarnation }` message (and release on selection change or +disconnect). Run and test requests independently acquire demand for their target project. + +With zero demand leases for a project, its edits still update source, editor diagnostics, and +catalog, but do not run bytecode generation, `$init`, engine installation, test collection, +or `UpdateProject` construction. A global `requestState` alone never warms every project. + +Under recommended Decision D5, first playground open chooses the active-editor/previously selected +project after catalog delivery and demands only that project; selection changes move the lease. +The warm-all alternative remains an explicit product choice. + +`ensure_engine_current(project, requested_revision)` is asynchronous, single-flight, +and revision-conditional: + +- current revision returns immediately; +- concurrent callers join one build; +- newer source supersedes the old candidate and waiters follow the new revision; +- invalid source returns `BlockedByDiagnostics`; +- engine/`$init` failure returns a typed terminal state; +- no projects-registry lock is held during compile or await. + +Under the recommended Decision D6 behavior, first subscriber state streams immediately, exposes +`preparing current build`, and warms in the background. Run/test await that same +latest-revision flight before capturing runtime identity. + +Replace the ambiguous boolean with a backward-compatible runtime state carrying at least: + +- `idleStale`; +- `building`; +- `ready`; +- `blockedByDiagnostics`; +- `failed`; +- requested and installed revisions; +- whether a last-known-good engine exists. + +Retain `isBexCurrent` only as a derived compatibility field. + +Once a demanded build begins, a transient subscriber disconnect does not cancel it. Future +zero-demand edits remain gated. Source supersession—not receiver churn—owns cancellation. + +Terminal build failure is keyed by source revision plus a revision of enumerated runtime inputs +(configuration/environment/toolchain inputs used by `$init`). A relevant input change permits +one new single-flight attempt. Otherwise the error remains visible and an explicit +`Retry build` action advances an attempt nonce; repeated state requests never form a retry +loop. Invalid source remains `BlockedByDiagnostics` and retries only after source change. + +## 5. Track C — complete position correctness + +### C1. Negotiated LSP position boundary + +Initialize selects UTF-8 when offered; otherwise UTF-16, and explicitly advertises the selected +`ServerCapabilities.position_encoding`. UTF-32 is not negotiated unless Decision D4 +requires it. + +Negotiated state belongs to the I7 `LspSession`, not the shared project/workspace. +Use a fresh `Arc>` or equivalent across only the dispatcher clones +bound to that connection. Reading before initialize is an error; never +`get_or_init(UTF16)`. A replacement browser connection gets a new session/config even +though it reuses the shared project registry. Project seeding may happen early, but outbound LSP +payloads wait until initialize/initialized. + +Create one `LspPositionCodec` at the LSP boundary. Compiler APIs remain byte-based. +The codec owns text plus line starts/ends and converts: + +- LSP position/range ↔ byte offset/range; +- byte span → LSP range; +- same-line byte span → encoded semantic-token length; +- document end → formatting range. + +It recognizes LF, CRLF, and bare CR. Overlong characters clamp to that line's end; nonexistent +lines, malformed ranges, and positions inside an encoding unit return `InvalidParams`. + +Audit all current paths: + +- incoming completion, hover, definition, references, inlay ranges, and code-action ranges; +- outgoing primary/related diagnostics, CodeLens, inlay hints, definition/reference locations, + workspace/document symbols, formatting edits, and semantic tokens. + +No LSP handler may directly construct a nontrivial position/range or call the current raw-byte +`lsp_position_to_offset`/`offset_to_lsp_position` helpers. + +Semantic tokens encode both `deltaStart` and `length` in the negotiated +encoding. Always split multiline spans into same-line segments; this is valid for all clients +and required for VS Code's no-multiline capability. Discard zero-length segments, exclude newline +units, keep sorted order, and assert unsupported overlaps are absent. + +### C2. Fixed playground editor coordinates + +The playground protocol does not inherit negotiated LSP encoding: + +- wire `line/column/endLine/endColumn` are zero-based UTF-16 code units, matching + VS Code positions; the Monaco adapter converts to/from Monaco's one-based `IPosition`; +- `startOffset/endOffset/cursorOffset` are UTF-8 byte offsets; +- incoming cursor coordinates convert fixed UTF-16 → byte offset; +- outgoing graph/source spans convert byte range → fixed UTF-16; +- LSP UTF-8 negotiation cannot change playground coordinates. + +Advertise a playground protocol capability such as +`sourcePositionEncoding: "utf16-zero-based-v1"`. The frontend uses the new adapter only when +advertised; otherwise it preserves legacy conversion. This capability may be waived only if the +release process proves all server/frontend consumers update atomically. Remove the current graph +navigation subtraction only behind that coordinated gate. + +**C1/C2 gate** + +- ASCII, accent, CJK, and emoji fixtures cover every handler under UTF-8 and UTF-16; +- LF, CRLF, and bare CR are covered; +- semantic-token starts/lengths around emoji decode correctly and multiline spans split; +- formatting uses encoded document end; +- pre-initialize access errors without initializing or freezing negotiation; +- a clone created before initialize observes the later shared config; +- browser session replacement starts with fresh, uninitialized capabilities; +- playground cursor and graph navigation remain UTF-16 under either LSP encoding; +- old/new playground protocol pairings preserve navigation or negotiate the new wire contract. + +## 6. Deferred work — shared Salsa snapshots and request workers + +This is not part of the v4 implementation plan. + +`ProjectDatabase::clone()` shares Salsa storage while cloning the database's metadata maps. +Salsa cancels a cloned reader for a pending write with `panic::resume_unwind`. The shipped +CLI/LSP `release` and `dist` profiles use `panic = "abort"`, so one edit racing a cloned +reader can abort the entire server; `Cancelled::catch`, Tokio task isolation, or worker +replacement cannot contain it. + +Consequently, v4 does not ship: + +- a shared-storage database snapshot that outlives the source gate; +- same-project parallel Salsa queries; +- cancel-on-write or Salsa local request cancellation; +- a database request worker pool. + +A pool operating only on fully owned, Salsa-detached inputs remains allowed, but it does not +provide the deferred same-project query parallelism. Blocking writes until all snapshots finish +is also rejected: it restores edit stalls and one missed gate becomes a release abort. + +Reconsider this only in a separate proposal after Phase 1 measurements show a material remaining +latency problem. That proposal must first quantify the user-visible value and target SLO, then +choose an isolation model—an unwind-enabled artifact, a subprocess, or a different database +architecture—before specifying workers. Snapshot clone-cost experiments may continue offline +under an unwind build, but they are not evidence that the release artifact is shippable. + +## 7. Product and behavior decision record + +D1-D8 adopt the recommended behavior. There are no remaining product choices in the active plan. +Speculative `$init` is resolved as an engineering fact, and unwind-dependent snapshot +parallelism is deferred rather than selected. + +### D1. Nested project process/toolchain isolation + +One parent `RelativePattern('**/*.baml')` also matches nested projects. Static parent +and child clients therefore cannot both be exact owners. + +- **Recommended:** one non-overlapping outer owner process multiplexes nested semantic projects; + the server routes each file to its nearest project marker. +- **Alternative:** each nested project gets a separate process/toolchain. This requires a central + exact-owner router or dynamic exclusion/restart design in the extension. + +**Decision D1:** use one non-overlapping outer owner process and route nested files to their +nearest semantic project. + +### D2. Browser LSP sessions + +Today multiple `/api/lsp` sockets share one Bex state and globally broadcast output. +Request IDs, document versions, and negotiated capabilities can collide. + +- **Recommended:** one active browser LSP session per process; the newest connection atomically + supersedes and closes the prior one, which handles reload cleanly. +- **Alternative:** true concurrent editable tabs, requiring per-session response routing, + capabilities, and document-writer ownership/conflict behavior. + +Under the recommended policy, takeover is one ingress barrier. It tombstones the old session and +its queued/running response tokens, revokes its sink, invalidates its diagnostic-tail items, and +closes its owned document overlays as one source mutation before the replacement initializes. +Late old-session work is rejected by session epoch and source revision even when request IDs are +reused. The replacement negotiates fresh capabilities and then reopens current buffers through +normal `didOpen` messages; no old overlay is silently transferred. + +**Decision D2:** support one active browser LSP session; the newest connection supersedes the +previous session. + +### D3. Ingress overload behavior + +Mutations cannot be dropped. A bounded request queue must choose between visible feature-request +failure and a stale multi-second backlog. + +- **Recommended:** preserve lifecycle, mutations, and side-effecting commands; reject excess + read/formatting requests promptly with `RequestFailed`; backpressure only when the + queue contains nothing safely rejectable. +- **Alternative:** protect selected explicit formatting/command requests with reserved admission, + accepting more backpressure. + +**Decision D3:** under extreme overload, fail newly arriving hover/hint/formatting requests; +never evict admitted work, and preserve mutation/lifecycle/command capacity. + +### D4. UTF-32 negotiation + +UTF-16 is mandatory in LSP and UTF-8 is BAML's internal fast path. UTF-32 adds another complete +handler and token test matrix without a known client requirement. + +- **Recommended:** negotiate UTF-8 when offered, otherwise UTF-16; do not select UTF-32. + +**Decision D4:** support UTF-8 when offered and UTF-16 otherwise; do not add UTF-32. + +### D5. First runtime-demand breadth + +The current global `requestState` walks every project. Treating one WebSocket subscriber as +runtime demand would compile and run `$init` for all projects on first playground open—44 +engines in the reporting workspace. + +- **Recommended:** catalog all projects, but warm only the active-editor or explicitly selected + project. Moving selection moves a per-session lease; Run/Test demand their own target. +- **Alternative:** eagerly warm every project so switching is instant, accepting the startup CPU, + memory, and `$init` cost. + +**Decision D5:** first playground open warms only the active or selected project. + +### D6. First playground presentation + +After editor-only demand gating, first open may not have a current engine. + +- **Recommended:** stream catalog/source/diagnostics immediately, show “Preparing current + build…”, warm asynchronously, and disable runtime-dependent controls until ready. +- **Alternative:** block the playground presentation until compile/`$init` completes. + +**Decision D6:** render immediately with a preparation state and disable runtime-dependent +controls until the selected project is ready. + +### D7. Run/test against stale source + +When source is newer than the last successful engine: + +- **Recommended:** wait for the latest single-flight build; if current source is invalid, fail + explicitly. Never silently run last-known-good. +- **Alternative:** expose an explicit, prominently labeled `Run last successful build` + action. It is never selected implicitly. + +For invalid source, the recommended test-tree UX retains the prior tree, marks it stale, and +disables launches. Collection failure retains the tree and shows an error; an empty tree means +zero tests, not failure. + +**Decision D7:** normal Run/Test always uses current source; do not add a stale-run action in this +plan. + +### D8. Runs already active when source changes + +Here “collection” means rebuilding and serializing the test-discovery tree for an engine; +“expansion” means enumerating child cases for a test set in that tree. Neither operation executes +a user test. They are stale tree-maintenance work once source changes, so their old results must +be canceled or discarded. + +- **Recommended:** allow an already-started run to continue on its pinned engine/CFG revision for + reproducibility. Cancel only stale test-tree discovery/serialization/expansion. Keep a + run-scoped `{ engine, graph, cancellation }` handle until terminal completion so explicit + cancel still targets the old function or test run after a new engine commits. Old engines remain + in memory while pinned runs finish. +- **Alternative:** cancel user runs on every edit. Source mutation claims one terminal + `canceledBySourceChange` outcome; a concurrent explicit cancel or natural completion loses the + exactly-once terminal race. + +**Decision D8:** already-started function and test runs continue on their pinned engine. New runs +follow D7 and require current source. Each active run retains its own engine/cancellation handle so +explicit Cancel continues to work; the old engine remains in memory only until its pinned runs +reach terminal completion. Run events/results are ordered by run identity and remain publishable +after a project-revision change; the stale-project watermark applies only to project-derived +state. + +### Resolved engineering fact — discarded-candidate `$init` + +`$init` is synchronous, deterministic candidate initialization. It populates the candidate's +owned globals; notifications are ignored, events are dropped, and an async/sys-op yield fails +initialization. A stale candidate may therefore finish `$init` and be discarded normally. + +The separate issue is constructor profiling bookkeeping: `BexEngine::new` currently registers +metadata before `$init`, while cleanup normally occurs from `BexEngine::drop`. If +`$init` returns an error, no engine reaches `Drop`; if a successfully constructed candidate +is superseded, normal `Drop` emits observer-close bookkeeping and leaves a closed-engine +tombstone. Phase 0 therefore keeps profiling inactive until conditional commit succeeds. Failed +or superseded candidates never register and drop quietly. These bookkeeping issues do not make +`$init` externally observable. + +### Deferred rather than decided — unwind and snapshots + +No unwind artifact or packaging decision is part of v4. Section 6 defers the Salsa snapshot and +worker design that required it. Revisit only if post-Phase-1 measurements establish enough value +for a separate proposal. + +## 8. Acceptance matrix + +### Engine/runtime + +- old/new candidates finish in both orders; only the newest revision installs; +- edit wins exactly around commit without a false-current state; +- final invalid edit publishes diagnostics without an engine commit and never permits an older + candidate to install; +- run registration returns same-revision engine/generation/CFG and, for tests, registry; +- an edit winning before registration forces the new run through D7; registration winning first + creates a D8 pinned lease; +- collection ABA and stale expansion results emit nothing; +- source mutation immediately invalidates derived work while preserving the D8 user-run policy; +- outbound/backend and frontend reject deliberately injected older-revision project payloads even + when engine generation is unchanged, while pinned run streams continue by run identity; +- after edit and new-engine commit, explicit Cancel and terminal completion still target old pinned + function and test runs exactly once; +- `$init` failure and discarded candidates leave no profiling metadata or resources. + +### Diagnostics + +- final invalid edit eventually publishes without another keystroke; +- publications carry document versions captured atomically with source revision/text; +- busy preserves prior diagnostics and retries; poison is explicit; +- mutation between ready-check and compare-and-clear cannot lose the newer dirty revision; +- saturated ingress cannot lose the one pending project-tail item; +- projects debounce/retry independently. + +### Ownership/catalog/runtime delivery + +- every URI/event has exactly one owner independent of open order; +- ownership and semantic resolvers produce stable outer/nearest roots, including commands, + symlink/case aliases, overlapping folders, and marker migration; +- initialize contains only owner roots; +- startup work is linear; +- `ListProjects` reaches the extension with zero runtime subscribers; +- zero-demand project edits perform no `UpdateProject` construction or engine work; +- a global state request never warms undemanded projects; selection/run/test demand only their + specified project; +- a new subscriber receives a forced full snapshot for demanded projects even with warm dedupe + caches; +- remove→re-add purges server/frontend caches and sends the new project incarnation; +- failed same-revision builds do not retry-loop; runtime-input change or explicit Retry starts one + joined attempt; +- changing any complete payload field emits an update; +- no registry/DB/cache lock is held while sending. + +### Ingress/protocol + +- no path emits `-32001`; +- every typed condition maps to the table in I7; +- response tokens route reused request IDs to the owning session and cancellation races emit one + response; +- stdio/browser traces are equivalent; +- inbound/outbound queue items and bytes are bounded; mutations retain FIFO order and slow readers + cannot create an unbounded writer queue; +- an unprocessed queued edit alone never causes server-originated `ContentModified`; +- initialize/shutdown responses precede their state transitions and all lifecycle error traces are + transport-identical; +- browser takeover revokes old overlays, sinks, requests, tails, and capabilities. + +### Encoding + +- ASCII, accent, CJK, and emoji round-trip under UTF-8/UTF-16; +- LF/CRLF/bare-CR line ends are correct; +- every current handler has integration coverage; +- semantic-token starts and lengths decode correctly and multiline spans split; +- pre-initialize access errors and cannot freeze negotiation; +- playground coordinates remain fixed UTF-16 under either LSP encoding; +- Monaco's one-based adapter and old/new playground protocol pairings pass the capability gate. + +### Abort-profile boundary + +- release-path tests assert no shared `ProjectDatabase` clone outlives the source gate; +- shipped request handlers do not invoke Salsa local cancellation or + `Cancelled::catch`; +- canceling a running request claims its response and discards its later result without unwinding + the in-flight Salsa query; +- background tasks accept only fully owned, Salsa-detached inputs. + +## 9. Rejected approaches + +- **Generic `ContentModified` for busy or queued edits:** wrong semantics and often + silently drops editor features. +- **Plain blocking DB lock everywhere:** an open playground/CFG build can freeze all dispatch. +- **Diagnostics `Option` plus “heal next keystroke”:** loses the final invalid edit and + conflates poison. +- **Debounce before a durable internal queue:** risks dropped tail work or background DB holds + racing source mutation. +- **Static parent and child client selectors:** parent globs include nested roots. +- **Receiver-gating `ListProjects`:** breaks the VS Code catalog. +- **Partial `ProjectUpdate` hashes:** suppress valid function/schema/type changes. +- **A global negotiated encoding read with `get_or_init` fallback:** early refresh can + permanently choose the wrong encoding; clones can diverge. +- **Incremental sync before C1:** raw-byte incoming positions would corrupt edits. +- **Snapshot retained through `$init` or commit:** blocks writers and violates owned + candidate boundaries. +- **Per-request `spawn_blocking` snapshots:** unbounded clones make writer cancellation + scale with request spam. +- **Shared Salsa snapshots under `panic=abort`:** a pending write unwinds and therefore + aborts the shipped LSP process. +- **Clearing a poisoned mutation lock and continuing:** partially updated state is not repaired. +- **Priority/dedup before barriers and measurements:** risks reading pre-mutation state and + inventing cancellation semantics. + +## 10. Remaining measurement and engineering questions + +These do not require a UX decision before their spike or instrumentation: + +1. Actual DB-hold distribution in the reporting multi-project workspace. +2. Exact 1500-function split between diagnostics, emit, and `$init`. +3. Runtime-subscriber duty cycle while editing. +4. Non-VS-Code client behavior for typed errors. +5. Symlink URI identity (`/tmp` vs `/private/tmp`). +6. Queue budgets derived from maximum mutation admission-to-apply latency. + +Every measurement gate records the command, fixture, raw output, build profile, and commit SHA.