Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
🔒 Entelligence AI Vulnerability Scanner ✅ No security vulnerabilities found! Your code passed our comprehensive security analysis. |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 83b4992 in 1 minute and 39 seconds. Click for details.
- Reviewed
211lines of code in7files - Skipped
1files when reviewing. - Skipped posting
7draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/language_server/src/server/api/requests/execute_command.rs:78
- Draft comment:
Good improvement: logging command execution and replacing unwrap() with error checks for forwarding messages avoids panics. Consider clarifying or addressing the TODO comment about 'select_function' (which appears on line 68) to provide clearer intent. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. engine/tools/Cargo.toml:13
- Draft comment:
Using an explicit 'tokio' dependency with features set to ["full"] improves clarity and ensures the full runtime is available. This change looks good. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. engine/tools/src/bin/language-server-hot-reload.rs:43
- Draft comment:
Replacing the hardcoded binary path with a dynamic relative path using env!(CARGO_MANIFEST_DIR) enhances portability and avoids environment‐specific paths. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. jetbrains/build.gradle.kts:148
- Draft comment:
The runIde task is now configured to auto-open the integ-tests directory and the prepareSandbox task has been updated to trust necessary paths. These changes improve the developer experience; just verify that the XML formatting for trusted paths meets security expectations. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
5. jetbrains/src/main/kotlin/com/boundaryml/jetbrains_ext/BamlLanguageServer.kt:11
- Draft comment:
The addition of the findBamlWorkspaceRoot function dynamically locates the workspace root by searching for 'engine/Cargo.toml'. This avoids hardcoded paths and improves portability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. jetbrains/src/main/kotlin/com/boundaryml/jetbrains_ext/BamlPlaygroundAction.kt:14
- Draft comment:
Updating the tool window ID to 'BAML Playground (beta)' ensures consistency with the plugin configuration. Moving the command execution outside the show callback may also help UI responsiveness. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. jetbrains/src/main/resources/META-INF/plugin.xml:32
- Draft comment:
The plugin.xml update changes the toolWindow ID to 'BAML Playground (beta)' and includes a comment to synchronize changes with BamlPlaygroundAction. This is helpful for maintenance and consistency. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_Pc6HfKJtlB5vKvM4
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
🌿 Preview your docs: https://boundary-preview-dcab53be-f002-40e6-b823-a0bb9cb0aa47.docs.buildwithfern.com |
Review Summary🏷️ Draft Comments (1)
🔍 Comments beyond diff scope (1)
|
| private fun findBamlWorkspaceRoot(startPath: Path): Path? { | ||
| var current = startPath | ||
| while (current.parent != null) { | ||
| if (Files.exists(current.resolve("engine/Cargo.toml"))) { | ||
| return current | ||
| } | ||
| current = current.parent | ||
| } | ||
| return null | ||
| } |
There was a problem hiding this comment.
correctness: findBamlWorkspaceRoot will not check the root directory itself for engine/Cargo.toml, potentially missing valid workspaces at the filesystem root.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In jetbrains/src/main/kotlin/com/boundaryml/jetbrains_ext/BamlLanguageServer.kt, lines 11-20, the function `findBamlWorkspaceRoot` does not check the root directory itself for `engine/Cargo.toml`, so it may miss valid workspaces at the filesystem root. Update the loop to check the current directory even when `current.parent == null` by using a `while (true)` loop and breaking when the parent is null after the check.
📝 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.
| private fun findBamlWorkspaceRoot(startPath: Path): Path? { | |
| var current = startPath | |
| while (current.parent != null) { | |
| if (Files.exists(current.resolve("engine/Cargo.toml"))) { | |
| return current | |
| } | |
| current = current.parent | |
| } | |
| return null | |
| } | |
| private fun findBamlWorkspaceRoot(startPath: Path): Path? { | |
| var current = startPath | |
| while (true) { | |
| if (Files.exists(current.resolve("engine/Cargo.toml"))) { | |
| return current | |
| } | |
| if (current.parent == null) { | |
| break | |
| } | |
| current = current.parent | |
| } | |
| return null | |
| } |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed b67454f in 1 minute and 49 seconds. Click for details.
- Reviewed
4235lines of code in44files - Skipped
0files when reviewing. - Skipped posting
7draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/playground-server/src/bin/playground_barebones.rs:13
- Draft comment:
The project directory is hard-coded. Ensure that this value is appropriate for integration tests and not left for local development. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. jetbrains/src/main/kotlin/com/boundaryml/jetbrains_ext/BamlToolWindowFactory.kt:56
- Draft comment:
The Vite hot reload HTML contains hard-coded URLs (http://localhost:3030). Verify that this is intended only for development and consider guarding it appropriately for production usage. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. typescript/packages/playground-common/src/baml_wasm_web/EventListener.tsx:347
- Draft comment:
Good update: the dependency array for the effect now includes setSelectedFunction along with setSelectedTestcase. This reduces the risk of using stale callbacks. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it simply acknowledges a change made in the code without providing any actionable feedback or suggestions. It does not align with the rules for useful comments in a pull request review.
4. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-render-wrapper.tsx:98
- Draft comment:
Consider adding error handling for the clipboard write in the handleCopy function to catch potential failures from navigator.clipboard.writeText. - 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.
5. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-render-wrapper.tsx:117
- Draft comment:
For improved accessibility, consider adding appropriate aria-labels to the Tabs triggers. - 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.
6. typescript/packages/playground-common/src/baml_wasm_web/EventListener.tsx:346
- Draft comment:
Typographical error: In the comment, "If we dont add the jotai atom callbacks..." should use the contraction "don't" instead of "dont". - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and focuses on a typographical error in a comment, which does not affect the functionality of the code. It does not provide a code suggestion or address a potential issue in the code itself.
7. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/index.tsx:82
- Draft comment:
There's a potential typo in the text 'ipsum2 placeholder content'. Did you mean 'ipsum placeholder content' or 'lorem ipsum placeholder content'? Please verify. - 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% This is clearly temporary placeholder text. Whether it says "ipsum2" or "lorem ipsum" makes no functional difference. This kind of comment about UI text is exactly the kind we should avoid per the rules - it's not about logic and assumes the author didn't intend the exact text they wrote. Maybe this placeholder text is actually part of some important test case that relies on this exact wording? Even if it was part of a test, that would be caught by the test suite itself. We should trust the author to handle placeholder content appropriately. This comment should be deleted as it's about UI text content and doesn't impact functionality.
Workflow ID: wflow_7QmUon5jm5Lnj6ee
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| "build": "pnpm run release", | ||
| "release": "pnpm run pack --release", | ||
| "pack": "wasm-pack build ../ --target bundler --out-dir ./web/dist" | ||
| "release": "pnpm run pack --dev", |
There was a problem hiding this comment.
The release script is still using '--dev', but the PR description says 'revert wasm-pack to --release'. Please update the release script accordingly.
| "release": "pnpm run pack --dev", | |
| "release": "pnpm run pack --release", |
|
🌿 Preview your docs: https://boundary-preview-527ce0f8-1339-4d5b-a554-34f7cd945ef4.docs.buildwithfern.com |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed dd2dd92 in 1 minute and 8 seconds. Click for details.
- Reviewed
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/baml-schema-wasm/web/package.json:10
- Draft comment:
Switching the flag from '--dev' to '--release' ensures an optimized build. Confirm this is the intended behavior for your releases. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. engine/baml-schema-wasm/web/package.json:11
- Draft comment:
Removed extra spacing in the 'pack' command for cleaner syntax. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. engine/baml-schema-wasm/web/package.json:8
- Draft comment:
Typo: It appears that the inline comment refers to using 'wasm pack --dev', but the command used elsewhere is 'wasm-pack'. Consider updating the comment for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_ohN4ZLoebY31B3en
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
🌿 Preview your docs: https://boundary-preview-0f7e7e05-c811-4192-ad2b-4b8ce0a6cbb8.docs.buildwithfern.com |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 2359ed7 in 36 seconds. Click for details.
- Reviewed
14lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. jetbrains/src/main/kotlin/com/boundaryml/jetbrains_ext/BamlLanguageServer.kt:31
- Draft comment:
Good improvement—explicitly checking for the JETBRAINS_PROJECT_DIR env var before using it. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_mFPBYYgdDfwetDD0
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
🌿 Preview your docs: https://boundary-preview-54a8d450-5c9e-4caf-8cc5-e8c86da406b5.docs.buildwithfern.com |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6e87647 in 54 seconds. Click for details.
- Reviewed
20lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/packages/playground-common/src/baml_wasm_web/EventListener.tsx:331
- Draft comment:
Consider extracting the hardcoded 1000ms delay into a named constant and handling timeout cancellation on unmount to avoid potential leaks. This workaround for the JetBrains recursive object issue should be clearer and more resilient. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_mTjWWXEpjMixvxTG
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
🌿 Preview your docs: https://boundary-preview-5134095d-903c-4e09-ae15-316257de6fe2.docs.buildwithfern.com |
|
🌿 Preview your docs: https://boundary-preview-42c732dc-10a0-47ab-abc7-4b4cf2108d73.docs.buildwithfern.com |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 739139e in 3 minutes and 48 seconds. Click for details.
- Reviewed
132lines of code in6files - Skipped
0files when reviewing. - Skipped posting
20draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. integ-tests/go/baml_client/baml_source_map.go:105
- Draft comment:
Replaced placeholder with explicit test argument in TestStructureDocument1559. Verify that using 'document_txt "hello"' is appropriate for the JSON schema. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to verify if using a specific test argument is appropriate for the JSON schema. This falls under asking the author to confirm their intention, which is not allowed. The comment does not provide a specific suggestion or point out a clear issue with the code.
2. integ-tests/python-v1/baml_client/inlinedbaml.py:100
- Draft comment:
TestStructureDocument1559 now provides an explicit argument ('document_txt "hello"') instead of a placeholder comment. Confirm that 'hello' is a valid test input. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm if 'hello' is a valid test input. This falls under the category of asking the author to confirm their intention, which is against the rules. The comment does not provide a specific code suggestion or ask for a specific test to be written, so it should be removed.
3. integ-tests/python/baml_client/inlinedbaml.py:127
- Draft comment:
Updated TestStructureDocument1559 by replacing the placeholder comment with a concrete argument ('document_txt "hello"'). Ensure that this hard-coded value meets the test’s requirements. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that the hard-coded value meets the test's requirements. This falls under the rule of not asking the author to ensure the behavior is intended or to double-check things. Therefore, this comment should be removed.
4. integ-tests/python/baml_client/inlinedbaml.py:195
- Draft comment:
A new function 'ReturnCategory' was added. Verify that this simple identity function is intentional and consider adding tests for it. - 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.
5. integ-tests/react/baml_client/inlinedbaml.ts:110
- Draft comment:
TestStructureDocument1559 now uses 'document_txt "hello"' instead of a placeholder. Confirm that this value satisfies the expected schema. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that a specific value satisfies the expected schema. This falls under the rule of not asking the author to confirm their intention or to ensure that the behavior is intended. Therefore, this comment should be removed.
6. integ-tests/react/baml_client/inlinedbaml.ts:155
- Draft comment:
New function 'ReturnCategory' was inserted in the expression functions. Ensure its intended identity behavior and add tests as needed. - 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. integ-tests/typescript-esm/baml_client/inlinedbaml.ts:110
- Draft comment:
The TestStructureDocument1559 test now explicitly passes 'document_txt "hello"'. Please confirm this meets expected input requirements. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment asks the PR author to confirm if the test meets expected input requirements, which violates the rule against asking for confirmation of intention or correctness. It doesn't provide a specific suggestion or point out a clear issue.
8. integ-tests/typescript-esm/baml_client/inlinedbaml.ts:155
- Draft comment:
Added the 'ReturnCategory' function. Verify its purpose as an identity function and consider adding tests to validate its behavior. - 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. integ-tests/typescript/baml_client/inlinedbaml.ts:110
- Draft comment:
TestStructureDocument1559 now includes a 'document_txt "hello"' argument instead of a placeholder. Confirm that this value is valid. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm the validity of a specific change, which is not allowed according to the rules. It does not provide a specific suggestion or point out a clear issue with the code.
10. integ-tests/typescript/baml_client/inlinedbaml.ts:155
- Draft comment:
New function 'ReturnCategory' is added. Ensure this change is intentional and that adequate tests cover its behavior. - 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. integ-tests/go/baml_client/baml_source_map.go:128
- Draft comment:
There appear to be typographical issues in the test assertion strings. In the first @@Assert call, the text "hi, thereerwc[]{}[]" seems to contain a typo ("thereerwc"). Additionally, in the second @@Assert call, the string "let me know if ,er" includes an unexpected comma. Please check if these are intentional or need correction. - Reason this comment was not posted:
Comment was on unchanged code.
12. integ-tests/python-v1/baml_client/inlinedbaml.py:125
- Draft comment:
Typo alert: In the assert string, "hi, thereerwc[]{}[]" seems to have an unexpected sequence "thereerwc". Please verify if this is intentional or a typo. - Reason this comment was not posted:
Comment was on unchanged code.
13. integ-tests/python-v1/baml_client/inlinedbaml.py:125
- Draft comment:
Typo alert: The assertion @@Assert({{"let me know if ,er"}}) contains ",er" which appears to be a typographical error. Please check if this should be corrected. - 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. integ-tests/python/baml_client/inlinedbaml.py:125
- Draft comment:
In the assert string for test "asserts.baml", the text "thereerwc[]{}[]" looks like it might be a typo. Please verify if it should be "there" or something similar. - Reason this comment was not posted:
Comment was on unchanged code.
15. integ-tests/python/baml_client/inlinedbaml.py:125
- Draft comment:
Also in the same block, the string "let me know if ,er" appears to have a misplaced comma. It might be a typo for "let me know if error" or similar. Please confirm the intended text. - Reason this comment was not posted:
Comment was on unchanged code.
16. integ-tests/react/baml_client/inlinedbaml.ts:133
- Draft comment:
Typographical issues: - In the assert string, "hi, thereerwc[]{}[]" appears to contain a typo. Perhaps 'thereerwc' should be something else? - In the subsequent assertion, the string "let me know if ,er" seems to have a typo (',er' likely should be 'error' or similar). - Also, the datetime string "Wed, 06 nov (PST)" uses a lowercase 'nov'; consider using 'Nov' for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
17. integ-tests/react/baml_client/inlinedbaml.ts:135
- Draft comment:
Minor stylistic/typographical note: In the AddRole template, the text "You are a {{ foo }}. be nice" might be intended to read "You are a {{ foo }}. Be nice" with a capitalized 'B' for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
18. integ-tests/typescript-esm/baml_client/inlinedbaml.ts:133
- Draft comment:
In the string literal for @@Assert, the substring "thereerwc" appears to be a typographical error. Please verify if this should be corrected (e.g. to 'there' or a proper term). - Reason this comment was not posted:
Comment was on unchanged code.
19. integ-tests/typescript-esm/baml_client/inlinedbaml.ts:133
- Draft comment:
The assertion @@Assert({{"let me know if ,er"}}) seems to contain a typo ('if ,er'). Consider revising it to the intended wording. - Reason this comment was not posted:
Comment was on unchanged code.
20. integ-tests/typescript/baml_client/inlinedbaml.ts:133
- Draft comment:
There's a potential typographical error in the assertion messages within the string literal for 'test-files/test-asserts-checks/asserts.baml'. Specifically, the substring "thereerwc[]{}[]" seems off, and the assertion message "let me know if ,er" contains an unexpected comma. Please confirm these are intended or correct them if they are typos. - 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.
Workflow ID: wflow_JMPU3NWUGtNnxPKC
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Important
Enhance BAML playground functionality with improved function/test selection, JetBrains integration, and hot-reload support, while removing unused components.
EventListener.tsx.function-test-name.tsx.BamlLanguageServer.ktandcors_bypass_proxy.rs.test-panel.1directory and its components.VITE_HOT_RELOAD_HTMLtoBamlToolWindowFactory.ktfor JetBrains.build.gradle.ktsto auto-openinteg-testsdirectory and configure trusted paths.execute_command.rsandplayground_barebones.rs.Cargo.tomlandlanguage-server-hot-reload.rsfor hot-reload functionality.plugin.xmlandRun Plugin in IDE.run.xml.This description was created by
for 739139e. You can customize this summary. It will automatically update as commits are pushed.