fix(lang-server): handle non-baml-src baml files #2486
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
sxlijin
temporarily deployed
to
boundary-tools-dev
GitHub Actions
Inactive
sxlijin
temporarily deployed
to
boundary-tools-dev
GitHub Actions
Inactive
sxlijin
temporarily deployed
to
boundary-tools-dev
GitHub Actions
Inactive
|
🌿 Preview your docs: https://boundary-preview-2ae9c344-a18e-476b-8285-4030779cb6d2.docs.buildwithfern.com |
|
🔒 Entelligence AI Vulnerability Scanner ✅ No security vulnerabilities found! Your code passed our comprehensive security analysis. |
Review Summary🏷️ Draft Comments (6)
🔍 Comments beyond diff scope (3)
|
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| tracing::info!( | ||
| "BAML file not in baml_src directory, not publishing diagnostics: {}", | ||
| file_url | ||
| ); | ||
| return Ok(()); |
There was a problem hiding this comment.
correctness: publish_session_lsp_diagnostics silently returns without notifying the client with a diagnostic when a .baml file is outside baml_src, so editors may show stale errors or no feedback.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In engine/language_server/src/server/api/diagnostics.rs, lines 100-104, when a `.baml` file is not in a `baml_src` directory, the function returns early without notifying the client, which can leave editors with stale or missing diagnostics. Update this block to call `notifier.notify::<lsp_types::notification::PublishDiagnostics>(not_in_baml_src_diagnostic(file_url)).ok();` before returning, so the client always receives a diagnostic explaining the issue.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| tracing::info!( | |
| "BAML file not in baml_src directory, not publishing diagnostics: {}", | |
| file_url | |
| ); | |
| return Ok(()); | |
| tracing::info!( | |
| "BAML file not in baml_src directory, not publishing diagnostics: {}", | |
| file_url | |
| ); | |
| notifier.notify::<lsp_types::notification::PublishDiagnostics>(not_in_baml_src_diagnostic(file_url)).ok(); | |
| return Ok(()); |
| if params.changes.iter().any(|change| { | ||
| let Ok(path) = change.uri.to_file_path() else { | ||
| return true; | ||
| }; | ||
| session.get_or_create_project(&path).is_err() | ||
| }) { |
There was a problem hiding this comment.
correctness: params.changes.iter().any(...) returns true if ANY change cannot be resolved to a project, causing the function to return early and skip processing ALL changes, even if some are valid; this can suppress updates for valid files.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In engine/language_server/src/server/api/notifications/did_change_watched_files.rs, lines 27-32, the code returns early if ANY file change cannot be resolved to a project, which causes all changes to be ignored even if some are valid. Change the logic so that it only returns early if ALL changes are invalid (i.e., cannot be resolved to a project). Replace the use of `.any(...)` with `.all(...)` and update the condition accordingly.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if params.changes.iter().any(|change| { | |
| let Ok(path) = change.uri.to_file_path() else { | |
| return true; | |
| }; | |
| session.get_or_create_project(&path).is_err() | |
| }) { | |
| let all_invalid = params.changes.iter().all(|change| { | |
| let Ok(path) = change.uri.to_file_path() else { | |
| return true; | |
| }; | |
| session.get_or_create_project(&path).is_err() | |
| }); | |
| if all_invalid { | |
| return Ok(()); | |
| } |
| let locked = project.lock(); | ||
| let default_flags = vec!["beta".to_string()]; | ||
| let effective_flags = session | ||
| .baml_settings | ||
| .feature_flags | ||
| .as_ref() | ||
| .unwrap_or(&default_flags); | ||
| let client_version = session.baml_settings.get_client_version(); | ||
|
|
||
| let generator_version = locked.get_common_generator_version(); | ||
| send_generator_version(¬ifier, &locked, generator_version.as_ref().ok()); | ||
| } else { | ||
| tracing::error!("Failed to get or create project for path: {:?}", file_path); | ||
| show_err_msg!("Failed to get or create project for path: {:?}", file_path); | ||
| } | ||
| tracing::info!("after get_or_create_project"); | ||
| let generator_version = locked.get_common_generator_version(); | ||
| send_generator_version(¬ifier, &locked, generator_version.as_ref().ok()); |
There was a problem hiding this comment.
correctness: project is locked and used after the early return, but if session.get_or_create_project(&path) fails, the function returns early and project is never defined, which is correct; however, if get_or_create_project returns Ok(project) but the lock fails (e.g., poisoned mutex), this will panic at runtime.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In engine/language_server/src/server/api/notifications/did_open.rs, lines 79-89, the code calls `project.lock()` directly, which can panic if the mutex is poisoned. Change this to handle the poisoned lock gracefully: use `match project.lock()` and return early with an error log if locking fails, instead of panicking. Apply this fix to ensure runtime stability.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| let locked = project.lock(); | |
| let default_flags = vec!["beta".to_string()]; | |
| let effective_flags = session | |
| .baml_settings | |
| .feature_flags | |
| .as_ref() | |
| .unwrap_or(&default_flags); | |
| let client_version = session.baml_settings.get_client_version(); | |
| let generator_version = locked.get_common_generator_version(); | |
| send_generator_version(¬ifier, &locked, generator_version.as_ref().ok()); | |
| } else { | |
| tracing::error!("Failed to get or create project for path: {:?}", file_path); | |
| show_err_msg!("Failed to get or create project for path: {:?}", file_path); | |
| } | |
| tracing::info!("after get_or_create_project"); | |
| let generator_version = locked.get_common_generator_version(); | |
| send_generator_version(¬ifier, &locked, generator_version.as_ref().ok()); | |
| let locked = match project.lock() { | |
| Ok(locked) => locked, | |
| Err(poisoned) => { | |
| tracing::error!("Project lock poisoned"); | |
| return Ok(()); | |
| } | |
| }; | |
| let default_flags = vec!["beta".to_string()]; | |
| let effective_flags = session | |
| .baml_settings | |
| .feature_flags | |
| .as_ref() | |
| .unwrap_or(&default_flags); | |
| let client_version = session.baml_settings.get_client_version(); | |
| let generator_version = locked.get_common_generator_version(); | |
| send_generator_version(¬ifier, &locked, generator_version.as_ref().ok()); |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| if params.changes.iter().any(|change| { | ||
| let Ok(path) = change.uri.to_file_path() else { | ||
| return true; | ||
| }; | ||
| session.get_or_create_project(&path).is_err() | ||
| }) { | ||
| return Ok(()); |
There was a problem hiding this comment.
[P1] Process batches even when some watched files are outside baml_src
The new early-return in DidChangeWatchedFiles now triggers whenever any change in the batch cannot be resolved to a baml project. File watching APIs often deliver mixed batches (e.g. a baml_src file together with a non-BAML file). In such cases this condition skips the entire handler, so the valid BAML file update will never reload the session or republish diagnostics. The previous logic only returned when no change was under baml_src. Consider checking that at least one change resolves to a project (all/any inversion) so unrelated files do not block handling of legitimate ones.
Useful? React with 👍 / 👎.
sxlijin
temporarily deployed
to
boundary-tools-dev
GitHub Actions
Inactive
sxlijin
temporarily deployed
to
boundary-tools-dev
GitHub Actions
Inactive
sxlijin
temporarily deployed
to
boundary-tools-dev
GitHub Actions
Inactive
|
🌿 Preview your docs: https://boundary-preview-b28c190c-b22d-49f7-a8a1-5de30700107f.docs.buildwithfern.com |
This also removes a bunch of panics, because if we can't resolve a
.bamlfile to abaml_srcproject we will no longer crash.