[DONT MERGE] LSP server #1465

imalsogreg · 2025-02-17T06:31:11Z

Draft PR for tracking work on the LSP server.

The server is meant to replace much of the existing server.ts, which implements an LSP for VSCode. The LSP in this PR is a native binary that should work with any LSP client (VSCode, emacs, vim, etc).

Features

Important

This PR introduces a Rust-based LSP server for BAML, replacing the TypeScript server, and updates the VSCode extension to integrate with the new server, supporting diagnostics, file requests, and more.

LSP Server:
- Introduces a Rust-based LSP server for BAML, replacing the existing TypeScript server.
- Supports diagnostics, file requests (did_open, did_change, did_change_watched_files), and file watching.
- Handles projects with nested directories and multiple compiler versions.
- Integrated with VSCode, emacs, and vim.
VSCode Extension:
- Updates extension.ts to activate the new LSP server using a Rust binary (baml-cli).
- Adds commands for diagnostics, test case selection, and jumping to definitions.
- Handles runtime_updated notifications and requests.
Build and Deployment:
- Updates GitHub Actions to build the Rust binary for different OS targets.
- Modifies Cargo.lock to include new dependencies for the LSP server.
- Adjusts next.config.mjs for WebAssembly loading.

^{This description was created by}^{for 4e79613. It will automatically update as commits are pushed.}

vercel · 2025-02-17T06:31:16Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
baml	❌ Failed (Inspect)			Mar 5, 2025 9:20pm

ellipsis-dev

❌ Changes requested. Reviewed everything up to 0b04ecc in 4 minutes and 46 seconds

More details

Looked at 12146 lines of code in 80 files
Skipped 2 files when reviewing.
Skipped posting 39 drafted comments based on config settings.

1. typescript/next.config.mjs:63

Draft comment:
Possible typo: the backgroundColor is set to '##9333ea'. For valid CSS colors it should likely be '#9333ea' (with one '#' instead of two).
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. flake.nix:120

Draft comment:
The updated RUSTFLAGS now include target-feature '+crt-static' and cfg 'tracing_unstable'. Ensure that this change is intentional and compatible with all target platforms.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. typescript/LICENSE:198

Draft comment:
The license file now embeds an extra appended MIT license text for an imported dependency (ruff). Verify that including third‐party license text in this file complies with your overall licensing strategy.
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
The comment is asking the PR author to verify compliance with licensing strategy, which is similar to asking for confirmation or verification. This violates the rule against asking the PR author to confirm or ensure things. The comment does not provide a specific code suggestion or point out a specific issue with the code itself.

4. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:73

Draft comment:
The code handling missing dependencies adds missingDeps array. The current implementation concatenates missing item names when dependencies are missing. It might be helpful to ensure the error message formatting is consistent and consider logging which dependency is missing more explicitly.
Reason this comment was not posted:
Confidence changes required: 40% <= threshold 50%
None

5. typescript/vscode-ext/packages/vscode/src/extension.ts:239

Draft comment:
In the code block that handles notifications and requests (lines 238–263), error handling wraps notify messages with try/catch. Ensure that logging and user notification are sufficient for diagnosing issues; consider also checking for potential race conditions with activeTextEditor access.
Reason this comment was not posted:
Confidence changes required: 30% <= threshold 50%
None

6. typescript/vscode-ext/packages/vscode/src/extension.ts:358

Draft comment:
The assignment to 'serverModule' now uses context.asAbsolutePath(path.join('language-server', 'out', 'bin')). Verify that this absolute path points to the correct executable on all supported platforms.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:59

Draft comment:
The code now uses fs.chmodSync to set executable permissions (line 355) on non‑win32 platforms. Confirm that the permission mode '755' is appropriate for your deployment environments.
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
The comment is asking the author to confirm if the permission mode '755' is appropriate for their deployment environments. This is a request for confirmation, which violates the rule against asking the PR author to confirm their intention or ensure behavior is intended.

8. typescript/vscode-ext/packages/vscode/src/extension.ts:63

Draft comment:
The backgroundColor string is set to "##9333ea" (double hash). Likely a typo – use "#9333ea" or a proper vscode.ThemeColor instead.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. typescript/vscode-ext/packages/vscode/src/extension.ts:20

Draft comment:
The global variable 'statusBarItem' is declared but its initialization appears to be commented out. Ensure this variable is instantiated before being used (e.g. in scheduleDiagnostics and runDiagnostics) to avoid runtime errors.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

10. typescript/vscode-ext/packages/vscode/src/extension.ts:354

Draft comment:
When setting file permissions using fs.chmodSync, consider using an octal literal (0o755) instead of the string '755' to avoid potential misinterpretation.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. .github/workflows/primary.yml:61

Draft comment:
There appears to be a duplicate top-level 'jobs:' key in the workflow file (one starting at line 21 and another starting at line 61). This can cause YAML parsing issues, as YAML does not allow duplicate keys. Please merge the two 'jobs:' sections into a single section to ensure proper workflow configuration.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:68

Draft comment:
Typographical Error: Consider changing "I dont think we need this line anymore..." to "I don't think we need this line anymore..." for correct punctuation.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

13. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:200

Draft comment:
Typographical Error: 'BAML_PATH_SPLTTER' may be a typo. If it's intended as a constant identifier, consider using 'BAML_PATH_SPLITTER' instead for clarity and consistency.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

14. engine/language_server/CONTRIBUTING.md:2

Draft comment:
Typographical error: 'READMe' should be capitalized as 'README' for consistency.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While this is technically correct and the file is new, this kind of minor typographical fix in documentation is very trivial. It doesn't affect functionality, and it's the kind of thing that could be fixed silently during review rather than requiring a comment. The rules specifically mention not making obvious or unimportant comments.
The inconsistent capitalization could be seen as unprofessional in documentation, and README is a well-established convention.
While correct, this is too minor of an issue to warrant a formal comment. It's the kind of thing that could be fixed directly or mentioned informally.
Delete this comment as it's too trivial and doesn't meet the bar for required code changes.

15. engine/language_server/CONTRIBUTING.md:15

Draft comment:
Typographical/grammatical error: In line 15, 'the former kind has exclusive mutable access to the state and execute immediately' should use 'executes' to agree with 'the former kind'.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While the grammar correction is technically correct, our rules state we should not make purely informative comments or obvious/unimportant changes. This is a documentation file, and the meaning is perfectly clear despite the minor grammatical error. The comment doesn't point out any significant issues that would affect code quality or functionality.
The grammar error could make the documentation look unprofessional. Clear technical writing is important for project documentation.
While clear writing is important, this is such a minor grammatical issue that it doesn't significantly impact readability or understanding. The rules specifically tell us to avoid obvious or unimportant comments.
We should delete this comment as it points out a trivial grammatical issue that doesn't meaningfully impact the documentation's effectiveness.

16. engine/language_server/CONTRIBUTING.md:28

Draft comment:
Typographical error: 'dis-allowing' should be written as 'disallowing' without the hyphen.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While technically correct, this is an extremely minor spelling issue that doesn't affect code or documentation clarity. The hyphenated version is still perfectly understandable. The rules state we should not make purely informative comments or unimportant suggestions. This feels like an overly pedantic correction.
The word is technically misspelled according to standard English, and this is documentation that will be publicly visible.
However, the hyphenated version is still completely clear in meaning and this type of minor spelling correction creates noise in PR reviews without adding meaningful value.
This comment should be removed as it's too minor and doesn't meaningfully improve the documentation.

17. engine/language_server/src/baml_db/mod.rs:67

Draft comment:
Trivial style issue: The file is missing a newline at the end. Please add a newline to adhere to common file formatting conventions.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While missing trailing newlines can cause minor issues with some tools, this is an extremely trivial issue that would likely be automatically fixed by most IDEs or formatters. The rules state not to make comments that are obvious or unimportant. This seems to fall into that category.
The missing newline could potentially cause issues with some Git tools and is considered a best practice in Unix-like systems. It's a real issue.
While technically correct, this is exactly the kind of trivial issue that should be handled by automated tooling rather than manual review comments. It adds noise to the review process.
Delete this comment as it's too trivial and would be better handled by automated formatting tools.

18. engine/language_server/src/baml_project/file_utils.rs:33

Draft comment:
Typographical Error: The comment beginning at line 33 has a typo. 'Seach for baml_src...' should be corrected to 'Search for baml_src...'.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
While this is a real typo, it's in documentation and is very minor. The rules state we should only keep comments that require code changes, and to not make purely informative comments. Documentation typos, while worth fixing, don't affect functionality and could be considered too minor to warrant a PR comment.
The typo does affect code quality in a small way since it appears in public documentation. It's also a very quick and easy fix that improves professionalism.
However, the rules explicitly state not to make purely informative comments and to only comment when there is clearly a code change required. Documentation typos fall more into the informative category.
Following the strict rules provided, we should delete this comment as it's purely informative and doesn't require actual code changes.

19. engine/language_server/src/baml_project/metadata.rs:71

Draft comment:
Typo detected in the comment: "... uses the following precedence the resolve the project's root." should likely be corrected to "... uses the following precedence to resolve the project's root."
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While this is technically a typo, it's a very minor grammatical error in a docstring that doesn't significantly impact code readability or understanding. The meaning is still clear despite the typo. Our rules say not to make purely informative comments or obvious/unimportant ones.
The typo does make the documentation slightly less professional. Documentation quality is important for maintainability.
However, this is such a minor issue that it creates more noise than value in the PR review. The meaning is still perfectly clear.
Delete this comment as it points out an extremely minor documentation issue that doesn't meaningfully impact code quality or understanding.

20. engine/language_server/src/baml_project/mod.rs:782

Draft comment:
The separator string 'BAML_PATH_SPLTTER' is used to split the file path and content; if this is a typo, consider updating it to the correct spelling (e.g., 'BAML_PATH_SPLITTER') to improve consistency.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
While the comment identifies what appears to be a typo, this is likely an intentional choice of string separator. The string is used consistently throughout the codebase (appears multiple times). Changing it would require coordinated changes across the codebase and could break compatibility. The current spelling, while unusual, works perfectly fine as a separator string - its exact spelling doesn't matter as long as it's consistent.
The comment correctly identifies an unusual spelling, and typos in string constants can cause bugs. Perhaps there's a good reason for this specific spelling that I'm missing.
The unusual spelling is used consistently throughout the code and changing it provides no functional benefit while risking breaking changes. String separators don't need to be perfectly spelled as long as they're unique and consistent.
The comment should be deleted. While it identifies an unusual spelling, changing it provides no benefit and could introduce risk. The current separator works fine and is used consistently.

21. engine/language_server/src/baml_project/watch.rs:7

Draft comment:
Typographical suggestion: In the comment at line 7, consider rephrasing "depends from where to where the path was moved:" to something like "depends on where the path was moved from and to:" for improved clarity.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While the suggested rewording is technically more grammatically correct, this is a very minor documentation change that doesn't affect code functionality or significantly impact readability. The existing wording is still understandable. The rules state not to make purely informative comments or comments that are obvious/unimportant.
The documentation is user-facing and clarity is important. Poor grammar could confuse non-native English speakers.
While documentation clarity matters, this change is too minor to warrant a PR comment. The existing wording is functional and the meaning is clear from context and examples.
Delete this comment as it suggests only a minor documentation wording change that doesn't significantly impact understanding.

22. engine/language_server/src/baml_text_size/range.rs:1

Draft comment:
It appears that the import on line 1, use cmp::Ordering;, might be missing the std:: prefix. If the intention is to import Ordering from the standard library, please update it to use std::cmp::Ordering;. This would help prevent potential confusion about where Ordering is coming from.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The comment suggests adding std:: prefix, but this isn't necessary since std::cmp is already imported on line 6. The code will work fine as is since Rust's module system will resolve cmp to std::cmp. The comment is technically incorrect since the code doesn't need fixing.
Could there be a style guide that prefers explicit std:: prefixes for clarity? Could having both imports be confusing to readers?
While explicit imports can be clearer, the current code is valid Rust and follows common patterns. Having redundant imports is more of an issue than missing the std prefix.
The comment should be deleted since it suggests an unnecessary change - the code works correctly as is due to the std::cmp import on line 6.

23. engine/language_server/src/edit.rs:26

Draft comment:
The comment on line 26 reads: "This document ID can point to either be a standalone Python file, a full notebook, or a cell within a notebook." The phrase "either be" is awkward. Consider revising it to something like "This document ID can point to a standalone Python file, a full notebook, or a cell within a notebook."
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While the comment correctly identifies a grammatical issue, it's a very minor documentation fix. The meaning is still clear despite the awkward phrasing. The rules say to not make purely informative comments or obvious/unimportant changes. This seems like a very low-priority documentation nitpick.
The documentation is technically incorrect grammatically, and clear documentation is important for maintainability. Poor grammar could reflect badly on the project.
While documentation quality matters, this is an extremely minor issue that doesn't impact understanding. The rules explicitly say not to make unimportant comments.
This comment should be deleted as it's too minor and doesn't highlight a significant issue that needs fixing.

24. engine/language_server/src/edit/range.rs:63

Draft comment:
Minor typographical suggestion: In the UTF16 branch comments (e.g., line 63), consider changing 'UTF16' to 'UTF-16' and '16 bit' to '16-bit' for improved consistency and clarity.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While technically correct (UTF-16 and 16-bit are the proper forms), this is an extremely minor documentation issue that doesn't affect code functionality. The meaning is perfectly clear either way. The rules state we should not make purely informative comments or obvious/unimportant suggestions.
The proper hyphenation of technical terms could be considered part of maintaining professional code quality. Consistent terminology might make the code more maintainable.
While proper terminology is good, this level of nitpicking about hyphens in comments is too minor to warrant a PR comment. It distracts from more important review feedback.
Delete this comment as it's too minor and purely stylistic, dealing with hyphenation in documentation rather than any meaningful code issues.

25. engine/language_server/src/server/api.rs:34

Draft comment:
Typo found: In the error message on line 34, 'TOOD: error handling' should be 'TODO: error handling'. Please correct the spelling.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
While this is a real typo, it's in an error message string that would only be seen during development. The rules say to only keep comments that require clear code changes and to avoid obvious or unimportant changes. This is a very minor cosmetic issue that doesn't affect functionality.
The typo could be confusing to other developers reading the code. Error messages are part of the developer experience.
However, this is still an extremely minor issue that doesn't impact functionality or code quality in any meaningful way. The rules emphasize focusing on substantive issues.
While technically correct, this comment about a typo in an error message is too minor to be worth keeping according to the review rules.

26. engine/language_server/src/server/api/diagnostics.rs:181

Draft comment:
Typo: In the function documentation for ensure_absolute (lines 181-186), 'taylored' should be corrected to 'tailored'. Also consider changing 'scare-quotes' to 'scare quotes' for consistency.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While this is technically about changed code since it's a new file, documentation typo fixes are generally very low value comments. The spelling error doesn't impact code functionality or readability significantly. The hyphenation suggestion is purely stylistic and even more minor.
Documentation quality does matter for maintainability. Fixing typos early prevents them from being copied elsewhere.
While documentation quality matters, this type of minor spelling/style fix creates noise in the review process and can be handled in a separate documentation pass or PR.
Delete this comment as it's too minor and doesn't meaningfully impact code quality or functionality.

27. engine/language_server/src/server/api/requests/code_lens.rs:76

Draft comment:
In the format string on line 76, 'format!("▶ Run for ${} 💥 ", testcase.parent_functions[0].name)', the '$' before {} appears to be extraneous. Consider removing it (or confirm if this was intended) so that the output is correctly formatted, e.g., "▶ Run for {} 💥".
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
This is a UI-related string that appears in the VS Code interface. The '$' could be intentional styling to make it look like a variable or parameter, which is common in UI/UX design. Since this is purely a UI/presentation concern and not a logical issue, we should follow the rule about not commenting on UI changes.
The comment might be technically correct about Rust format strings not needing '$', but am I being too quick to dismiss it as just UI styling?
Even if it's not intentional styling, this falls under the rule about not commenting on pure UI/styling changes. We should trust that the author intended this presentation.
Delete the comment as it relates to UI presentation rather than logic, and we should assume UI changes are intentional per the review rules.

28. engine/language_server/src/server/connection.rs:92

Draft comment:
Typo: In the comment before the 'handle_shutdown' function on line 92, consider adding a space between 'returns' and 'true' for improved readability. Instead of "returnstrue", it should read "returns true".
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While technically correct, this is an extremely minor formatting issue that doesn't affect code functionality or even documentation clarity significantly. The doc comment is still perfectly readable without the space. According to the rules, we should not make comments that are obvious or unimportant. This seems to fall into that category.
The consistent formatting of doc comments could be considered part of code quality. Some might argue that maintaining a consistent style throughout the codebase is important.
While consistency is good, this change is too trivial to warrant a PR comment. It would add noise to the review process for minimal benefit.
Delete this comment as it points out an extremely minor formatting issue that doesn't meaningfully impact code quality or documentation clarity.

29. engine/language_server/src/session.rs:121

Draft comment:
Typo in comment: "red knot only support a single project." should be "red knot only supports a single project."
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
While technically correct, this is an extremely minor grammatical fix in a comment. The meaning is perfectly clear either way. Comments about pure grammar/spelling in comments, unless they make the meaning unclear, are not worth the overhead of a PR review comment.
The comment is technically correct - there is a subject-verb agreement error. And clear communication is important in code comments.
While clear communication is important, this tiny grammatical error does not impact understanding at all. The overhead of a PR comment for such a minor issue is not worth it.
Delete this comment. The grammatical error is too minor to warrant a PR review comment.

30. engine/language_server/src/session.rs:161

Draft comment:
Typo in variable name: '_projet_root' should be corrected to '_project_root' for consistency.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While this is technically a typo, it's in a parameter name that is prefixed with _ indicating it's intentionally unused. The typo doesn't affect functionality since the variable is never used. The suggestion would only improve readability very slightly.
The typo could be considered a minor code quality issue that's worth fixing while we're here. It costs almost nothing to fix.
However, we should be conservative about keeping comments. This is an unused variable and the typo doesn't impact understanding or functionality. The benefit is extremely minimal.
This comment should be deleted as it points out a trivial issue with an unused variable that doesn't impact code quality in any meaningful way.

31. engine/language_server/src/session.rs:211

Draft comment:
Typo in commented-out code: "TODO: Handle reload errer" should be "TODO: Handle reload error".
Reason this comment was not posted:
Comment was on unchanged code.

32. engine/language_server/src/session/index.rs:81

Draft comment:
There's a minor inconsistency in the capitalization of error messages within the close_document method. For example, the error message in the let Some(url) branch starts with "Tried to close unavailable document..." (capitalized) while the one in the following branch starts with "tried to close document that didn't exist..." (not capitalized). Consider unifying the capitalization to maintain consistency.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While the observation is technically correct, this is an extremely minor stylistic issue that doesn't affect functionality. Error message capitalization is not typically enforced by style guides. The messages are clear and understandable regardless of capitalization. This seems like unnecessary nitpicking that adds noise to the review.
The inconsistency could theoretically indicate a lack of attention to detail that might manifest in more important ways. Some teams do care about consistent error message style.
While consistency is good, this level of nitpicking about capitalization in error messages is more likely to be distracting than helpful. The code's functionality and structure are what matter most.
This comment should be removed as it focuses on an extremely minor stylistic issue that doesn't meaningfully impact code quality or readability.

33. flake.nix:43

Draft comment:
Typographical error: In the commented-out code on line 43, "crossSystemcnofig" should be corrected to "crossSystemConfig".
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

34. typescript/fiddle-frontend/next.config.mjs:28

Draft comment:
Typo in comment: 'locak' should be 'local'.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

35. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:80

Draft comment:
On line 80, the concatenation in the console.error call does not include a space or punctuation between 'Missing required dependencies' and the list of missing items. For readability, consider changing it to something like: console.error('Missing required dependencies: ' + missingDeps.join(', ')).
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The comment is about a real issue in changed code - the console.error message is missing proper spacing. However, this is just a debug log message, not user-facing output. The actual error state that gets displayed to users (in setState) already has proper formatting. Console messages are primarily for developers debugging.
The formatting inconsistency between setState and console.error could be confusing for developers debugging issues. Having consistent error messages helps with debugging.
While consistency is good, this is an extremely minor formatting issue in a debug log that doesn't affect functionality or user experience. The important user-facing message is already properly formatted.
This comment should be deleted as it addresses a very minor formatting issue in debug logging that doesn't meaningfully impact the code quality or user experience.

36. typescript/vscode-ext/packages/language-server/src/lib/baml_project_manager.ts:146

Draft comment:
Typo: The string token 'BAML_PATH_SPLTTER' appears in the file (e.g., in update_runtime() and files() functions). It might be intended to be 'BAML_PATH_SPLITTER'. Please check and fix this typographical error.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

37. typescript/vscode-ext/packages/vscode/src/extension.ts:63

Draft comment:
Typo: The background color is set as '##9333ea'. It should be '#9333ea' (a single '#' instead of two).
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

38. typescript/vscode-ext/packages/vscode/src/extension.ts:135

Draft comment:
Typo: The log message 'Pushing test errorrrr' contains an extra 'r'. Consider correcting it to 'Pushing test error' or a more appropriate message.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

39. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:147

Draft comment:
Typographical error: The comment 'dont delete this.' should be updated to 'don't delete this.' to ensure proper punctuation.
Reason this comment was not posted:
Comment was on unchanged code.

Workflow ID: wflow_1JPiRsE8xUaP2cF8

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev · 2025-03-05T21:23:06Z

engine/language_server/src/baml_project/mod.rs

+        });
+        let formatted_files = saved_files
+            .iter()
+            .map(|(k, v)| format!("{}BAML_PATH_SPLTTER{}", k, v))


'BAML_PATH_SPLTTER' might be a typographical error. Consider renaming it to 'BAML_PATH_SPLITTER' (or the intended variant) for clarity.

ellipsis-dev · 2025-03-05T21:23:06Z

engine/language_server/src/baml_source_file/line_ranges.rs

+    /// If `offset` is passed the end of the content.
+    fn line_end(&self, offset: TextSize) -> TextSize;
+
+    /// Computes the range of this `offset`s line.


In the doc comment for full_line_range, use the proper possessive form by writing "this offset's line" instead of "this offsets line" to improve clarity.

Suggested change

/// Computes the range of this `offset`s line.

/// Computes the range of this offset's line.

ellipsis-dev · 2025-03-05T21:23:07Z

engine/language_server/src/baml_source_file/newlines.rs

+/// ## Examples
+///
+/// ```rust
+/// # use ruff_text_size::TextSize;


The documentation examples reference ruff_text_size and ruff_source_file, which are inconsistent with the current module's use of crate::baml_text_size. Please update these references to maintain consistency.

Suggested change

/// # use ruff_text_size::TextSize;

/// # use crate::baml_text_size::TextSize;

ellipsis-dev · 2025-03-05T21:23:07Z

engine/language_server/src/baml_text_size/size.rs

+    /// # Examples
+    ///
+    /// ```rust
+    /// # use ruff_text_size::*;


The documentation examples refer to ruff_text_size (e.g., # use ruff_text_size::*;), but given that the module is from baml_text_size, this appears to be a naming inconsistency. Please update the examples to use baml_text_size to match the module name.

ellipsis-dev · 2025-03-05T21:23:07Z

engine/language_server/src/server.rs

+            writeln!(stderr, "{panic_info}\n{backtrace}").ok();
+
+            try_show_message(
+                "The Ruff language server exited with a panic. See the logs for more details."


Typographical inconsistency: The panic message refers to 'The Ruff language server' while the project appears to be 'baml'. Please verify and update the message for consistency.

Suggested change

"The Ruff language server exited with a panic. See the logs for more details."

"The baml language server exited with a panic. See the logs for more details."

ellipsis-dev · 2025-03-05T21:23:07Z

engine/language_server/src/server/schedule/task.rs

+        })
+    }
+    /// Creates a local task that immediately
+    /// responds with the provided `request`.


The documentation for the immediate function (lines 78-79) refers to the provided request, but the function actually sends a response based on the given RequestId and result. Consider updating the comment to clarify the behavior, for example, by saying "Creates a local task that immediately responds with the provided result."

Suggested change

/// responds with the provided `request`.

/// Creates a local task that immediately responds with the provided result.

ellipsis-dev

👍 Looks good to me! Incremental review on 4e79613 in 2 minutes and 37 seconds

More details

Looked at 74 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 5 drafted comments based on config settings.

1. .github/workflows/primary.yml:145

Draft comment:
Duplicate job names removal/insertion: Verify that the earlier build-cli job was completely removed and that the new positioning is intentional.
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
The comment is asking the PR author to verify the removal and repositioning of a job, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code.

2. .github/workflows/primary.yml:169

Draft comment:
Artifact upload glob: Confirm that using 'engine/target/release/baml-cli*' doesn’t unintentionally capture extra files, especially on Windows (where an .exe is expected).
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
This is a "verify that..." style comment asking the author to double-check something. The glob pattern is actually standard practice for Rust projects since it needs to handle both the Unix binary name and Windows .exe. The build process is controlled and we know exactly what files will be there. This is more of a hypothetical concern than an actual issue.
The glob pattern could potentially be important for artifacts that get published. Maybe there's a real risk of picking up debug symbols or other build artifacts.
The Rust build process is well-defined and this glob pattern is commonly used. Any extra files would be caught during testing of the workflow. This is more speculative than a concrete issue.
Delete the comment. It's asking for verification without identifying a concrete problem, and uses a standard glob pattern that's appropriate for cross-platform Rust binaries.

3. .github/workflows/primary.yml:145

Draft comment:
The 'build-cli' job is reintroduced later in the file without any internal changes. If this reorder is intentional (for example to control job execution order), consider explicitly setting job dependencies using the 'needs' keyword to clarify any ordering requirements.
Reason this comment was not posted:
Comment was on unchanged code.

4. .github/workflows/primary.yml:157

Draft comment:
The new 'build-cli' job block is identical to the removed version. To preserve commit history and ease future reviews, consider reordering or moving the block rather than deleting and re-adding it.
Reason this comment was not posted:
Confidence changes required: 33% <= threshold 50%
None

5. .github/workflows/primary.yml:152

Draft comment:
Minor typo: The entry for 'macos-latest' has trailing whitespace. While this doesn't affect functionality, please remove the extra space for consistency.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While the comment is technically correct about the whitespace, this is an extremely minor formatting issue that doesn't affect functionality. GitHub Actions YAML parsing is very forgiving with whitespace. This kind of nitpick creates noise without adding value. The change was just moving the job, not creating new content.
The whitespace inconsistency could theoretically make the file slightly harder to maintain, and some teams do care about strict formatting consistency.
However, this is a CI configuration file where such minor formatting issues are not critical, and the comment violates our rule about not making purely cosmetic or unimportant suggestions.
Delete this comment as it's too minor of an issue to warrant attention and doesn't affect functionality.

Workflow ID: wflow_PSy2l3ja4ZKJfzTk

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

vercel bot deployed to Preview February 17, 2025 19:57 View deployment

vercel bot deployed to Preview February 18, 2025 23:50 View deployment

vercel bot deployed to Preview February 20, 2025 01:14 View deployment

vercel bot deployed to Preview February 20, 2025 22:06 View deployment

vercel bot deployed to Preview February 21, 2025 17:38 View deployment

vercel bot deployed to Preview February 24, 2025 23:03 View deployment

vercel bot deployed to Preview February 25, 2025 22:13 View deployment

vercel bot deployed to Preview February 25, 2025 23:49 View deployment

vercel bot deployed to Preview February 26, 2025 00:45 View deployment

vercel bot deployed to Preview February 26, 2025 19:32 View deployment

vercel bot deployed to Preview February 27, 2025 03:38 View deployment

vercel bot deployed to Preview February 28, 2025 00:03 View deployment

vercel bot deployed to Preview February 28, 2025 00:29 View deployment

imalsogreg force-pushed the greg/lsp branch from 9547e15 to 1a89c45 Compare March 4, 2025 18:48

vercel bot deployed to Preview March 4, 2025 18:54 View deployment

aaronvg and others added 7 commits March 4, 2025 11:01

Add lsp

9f55ba2

try out flake update

37659d0

revert flake update

a5ea955

debugging

5986734

logging

5a2815d

Add runtime updating to codelens requests

c1177e9

cleanup

86da074

imalsogreg force-pushed the greg/lsp branch from 1a89c45 to 86da074 Compare March 5, 2025 01:00

vercel bot had a problem deploying to Preview March 5, 2025 01:00 Failure

Build baml-cli in CI

0b04ecc

vercel bot had a problem deploying to Preview March 5, 2025 21:16 Failure

imalsogreg marked this pull request as ready for review March 5, 2025 21:18

fix github action script

4e79613

vercel bot had a problem deploying to Preview March 5, 2025 21:20 Failure

ellipsis-dev bot reviewed Mar 5, 2025

View reviewed changes

imalsogreg closed this Mar 5, 2025

imalsogreg reopened this Mar 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DONT MERGE] LSP server #1465

[DONT MERGE] LSP server #1465

imalsogreg commented Feb 17, 2025 •

edited by ellipsis-dev bot

Loading

vercel bot commented Feb 17, 2025 •

edited

Loading

ellipsis-dev bot left a comment

ellipsis-dev bot Mar 5, 2025

ellipsis-dev bot Mar 5, 2025

ellipsis-dev bot Mar 5, 2025

ellipsis-dev bot Mar 5, 2025

ellipsis-dev bot Mar 5, 2025

ellipsis-dev bot Mar 5, 2025

ellipsis-dev bot left a comment

	/// Computes the range of this `offset`s line.
	/// Computes the range of this offset's line.

	/// # use ruff_text_size::TextSize;
	/// # use crate::baml_text_size::TextSize;

	"The Ruff language server exited with a panic. See the logs for more details."
	"The baml language server exited with a panic. See the logs for more details."

	/// responds with the provided `request`.
	/// Creates a local task that immediately responds with the provided result.

[DONT MERGE] LSP server #1465

Are you sure you want to change the base?

[DONT MERGE] LSP server #1465

Conversation

imalsogreg commented Feb 17, 2025 • edited by ellipsis-dev bot Loading

vercel bot commented Feb 17, 2025 • edited Loading

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot Mar 5, 2025

Choose a reason for hiding this comment

ellipsis-dev bot Mar 5, 2025

Choose a reason for hiding this comment

ellipsis-dev bot Mar 5, 2025

Choose a reason for hiding this comment

ellipsis-dev bot Mar 5, 2025

Choose a reason for hiding this comment

ellipsis-dev bot Mar 5, 2025

Choose a reason for hiding this comment

ellipsis-dev bot Mar 5, 2025

Choose a reason for hiding this comment

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

imalsogreg commented Feb 17, 2025 •

edited by ellipsis-dev bot

Loading

vercel bot commented Feb 17, 2025 •

edited

Loading