Bring back extension support in the baml-cli #2154
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to d9337e6 in 2 minutes and 15 seconds. Click for details.
- Reviewed
2196lines of code in18files - Skipped
2files when reviewing. - Skipped posting
15draft 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/playground/playground_server.rs:29
- Draft comment:
The code now always uses a local dist directory and spawns a proxy server on port+1. Ensure that using a fixed offset for the proxy port won’t cause conflicts in production. - 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 using a fixed offset for the proxy port won't cause conflicts in production. This is a request for confirmation of intention, which violates the rules. The comment does not provide a specific suggestion or ask for a specific test to be written.
2. engine/language_server/src/playground/proxy.rs:62
- Draft comment:
Fallback API keys for development are embedded. Remember to remove or secure these before production deployment. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. typescript/packages/playground-common/package.json:67
- Draft comment:
The dependency 'react-pdf' was added. Verify that the bundle size and compatibility with other PDF modules is acceptable. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about a dependency change, specifically about verifying bundle size and compatibility. According to the rules, comments on dependency changes or asking for verification are not allowed.
4. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/pdf-viewer.tsx:18
- Draft comment:
The custom PDF viewer supports zoom, pan, and virtual scrolling. Consider performance testing with large PDFs and possibly lazy-loading pages to mitigate excessive rendering. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/webview-media.tsx:18
- Draft comment:
Blob URL creation and cleanup is well implemented and the copy-to-clipboard UX is a nice addition. Consider debouncing or throttling the copy action on rapid clicks. - Reason this comment was not posted:
Comment looked like it was already resolved.
6. typescript/packages/playground-common/src/shared/baml-project-panel/vscode.ts:55
- Draft comment:
The VSCodeAPIWrapper now supports WebSocket RPC for non-VSCode environments. Review the error and timeout handling in ensureWebSocketRpcConnection for robustness in various network conditions. - 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/packages/ui/src/styles/globals.css:12
- Draft comment:
Global CSS variables and dark mode overrides have been updated. Double-check that the new '--border' and '--input' values in dark mode meet the design guidelines. - 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.
8. engine/language_server/src/playground/definitions.rs:39
- Draft comment:
The addition of active_connections to track client connections is a clear improvement. Ensure that this state is synchronized properly with your event buffering logic. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is informative and suggests ensuring synchronization, which violates the rule against asking the author to ensure behavior. It doesn't provide a specific code suggestion or ask for a specific test.
9. engine/zed/Cargo.toml:12
- Draft comment:
Enabling the 'debug' feature by default for the zed project is useful for development, but ensure that release builds disable this feature for optimized performance. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is suggesting to ensure that release builds have the 'debug' feature disabled for optimized performance. This is a general suggestion to double-check the build configuration, which violates the rule against asking the PR author to ensure behavior is intended or tested.
10. engine/language_server/src/playground/mod.rs:9
- Draft comment:
The line starting withpub use playground_server_helpers::{seems incomplete—there's no closing brace or exported items. Please verify if this statement is intended to export members fromplayground_server_helpersand update accordingly. - 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. engine/language_server/src/playground/playground_server.rs:15
- Draft comment:
There's a minor wording issue in the comment on line 15: "// Defines where the playground server will look for to fetch the frontend". Consider rephrasing it to something like "// Defines where the playground server will fetch the frontend from" for better clarity. - 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/language_server/src/playground/playground_server.rs:30
- Draft comment:
Typographical error: In the comment on line 30, "enviroment" should be "environment". - Reason this comment was not posted:
Comment was on unchanged code.
13. engine/language_server/src/playground/playground_server_helpers.rs:147
- Draft comment:
Consider using consistent casing for "WebSocket" in the log message. Currently, it uses "RPC websocket..." which might be better as "RPC WebSocket..." for consistency with other parts of the code. - 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 inconsistent capitalization, this is an extremely minor issue that only affects a log message. It doesn't impact functionality, readability, or maintainability in any meaningful way. The rules state not to make comments that are obvious or unimportant. Log message capitalization falls into the "unimportant" category. The comment does point out a real inconsistency in the codebase. Consistent naming could be considered part of code quality. While consistency is good, this is far too minor of an issue to warrant a PR comment. Log message capitalization doesn't affect code quality in any meaningful way. Delete this comment as it focuses on an extremely minor stylistic issue in a log message that doesn't meaningfully impact code quality.
14. typescript/packages/ui/src/styles/globals.css:184
- Draft comment:
Typo: The variable '--vscode-descriptionForeground' uses camelCase, which is inconsistent with the similar '--vscode-description-foreground' pattern. Please verify if this is intentional or correct it 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 = 0% vs. threshold = 50% While the comment correctly identifies an inconsistency in naming convention, both variables are intentionally defined and used. This appears to be deliberate to support both naming conventions, possibly for compatibility reasons. The code is working as intended - both variables are defined and mapped to the same value. This isn't a bug or issue that needs fixing. The comment assumes this is a typo when it could be intentional compatibility support. We should be careful about assuming inconsistencies are mistakes. The presence of both variables being explicitly defined and used in the codebase suggests this is intentional rather than a mistake that needs fixing. This comment should be deleted as it flags an intentional pattern as a potential mistake. The code is working as designed by supporting both naming conventions.
15. typescript/packages/ui/src/styles/globals.css:203
- Draft comment:
Typo: The variable '--vscode-button-hoverBackground' uses camelCase while another similar variable '--vscode-button-hover-background' exists. This inconsistency might cause confusion. Please review the naming convention. - 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 appears to be intentional - both variables are defined to support different VSCode naming conventions. This is a common pattern in VSCode theming where some variables use kebab-case and others use camelCase. Having both ensures compatibility with different parts of VSCode that might expect one format or the other. The fact that they have the same value suggests this is deliberate aliasing. Could this duplication cause maintenance issues if one variable is updated but not the other? Could there be a better way to handle this aliasing? The duplication is necessary for VSCode compatibility and both variables are defined right next to each other, making it unlikely that one would be updated without the other. This is a standard pattern in VSCode theming. This is not a typo but rather intentional aliasing to support different VSCode naming conventions. The comment should be deleted.
Workflow ID: wflow_zKaMhxqSDdGIsAc7
You can customize 
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
typescript/packages/playground-common/src/shared/baml-project-panel/vscode.ts
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed cbb063d in 1 minute and 51 seconds. Click for details.
- Reviewed
95lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft 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/playground/playground_server.rs:67
- Draft comment:
Undefined variable 'local_dist'. The if/else already assigns to 'dist_dir' but here 'local_dist' is used incorrectly. Likely you meant to use the computed 'dist_dir'. - Reason this comment was not posted:
Comment was on unchanged code.
2. engine/zed/Cargo.toml:14
- Draft comment:
Changing the default feature to empty (default = []) is intentional? Ensure this does not break dependencies expecting the debug mode by default. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =30%<= threshold50%The comment is asking the author to ensure that the change does not break dependencies, which violates the rule against asking the author to ensure behavior is intended or tested. However, it does ask if the change is intentional, which is allowed.
Workflow ID: wflow_6CwzCTY0Q3Ch93Z8
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 15b5383 in 1 minute and 46 seconds. Click for details.
- Reviewed
387lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5draft 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. .github/workflows/release.yml:6
- Draft comment:
Verify that the branch trigger 'egor/extension-fixes' is intentional for release pushes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify their intention regarding a branch trigger. This falls under the rule of not asking the author to confirm their intention or to double-check things. Therefore, this comment should be removed.
2. .github/workflows/release.yml:20
- Draft comment:
Default version is set to '0.1.0'. Confirm if this is intended (previously 'canary' was used). - 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. .github/workflows/release.yml:29
- Draft comment:
Ensure that sanitizing tag names (replacing '/' with '-') meets the versioning 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 a specific change meets versioning requirements. This falls under the rule of not asking the author to ensure behavior is intended or tested. It doesn't provide a specific suggestion or point out a clear issue.
4. .github/workflows/release.yml:69
- Draft comment:
Confirm that the artifact pattern 'playground-dist-*' aligns with the outputs from the build process. - 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. .github/workflows/release.yml:83
- Draft comment:
The GitHub release body is hardcoded ('Playground dist test'); verify if this placeholder is intended for production. - 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 appears to be an intentionally simplified version of the workflow for testing purposes. The hardcoded message is likely intentional for this test version. The branch name and commented sections make it clear this is not meant for production. The comment is asking the author to verify something that is clearly intentional for testing. I could be wrong about this being a test version - maybe this is actually meant to be the new production version of the workflow. The evidence strongly suggests this is for testing: the branch name contains "fixes", most jobs are commented out with an explicit "playground dist test" comment, and the simplified workflow structure all point to this being temporary. The comment should be deleted because it's asking to verify something that is intentionally temporary/for testing purposes.
Workflow ID: wflow_UiTTHqClJhWgaBzZ
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 36bde7b in 2 minutes and 1 seconds. Click for details.
- Reviewed
297lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft 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. .github/workflows/build-vscode-release.reusable.yaml:68
- Draft comment:
Consider using a conditional flag instead of commenting out the CLI artifact steps. Commented code may lead to confusion long-term. - 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 makes a valid point about code quality - commented code can lead to maintenance issues. However, the comment "for playground dist testing" suggests this is an intentional temporary state during testing. The change appears to be a quick way to disable functionality during testing rather than a permanent solution. Making a more complex conditional system for a temporary testing state might be overengineering. The comment raises a valid long-term code quality concern, but may be missing the temporary nature of this change. Not every temporary testing change needs to be engineered with full conditional logic. While using conditionals is generally better practice, the temporary nature and testing context makes the current approach reasonable for short-term use. The comment should be deleted as it suggests a more complex solution for what appears to be an intentionally temporary testing state.
2. .github/workflows/build-vscode-release.reusable.yaml:84
- Draft comment:
Similarly, the download and listing artifact steps are commented out. Consider using an input flag or condition rather than commenting out for clarity. - 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 suggestion to use flags/conditions is generally good practice, this appears to be a temporary change for testing purposes as indicated by the comment "for playground dist testing". The commented sections are likely intentionally disabled during development/testing and will be re-enabled later. Making a structural change to use flags would be overkill for what appears to be temporary testing. The comment does raise a valid point about code maintainability - commented out code can be confusing and harder to maintain than feature flags. However, for temporary testing scenarios, commenting out code is a common and acceptable practice. The intent is clearly documented in the comment. The comment should be deleted as it suggests a structural change that's unnecessary for what appears to be temporary testing code.
3. .github/workflows/build-vscode-release.reusable.yaml:97
- Draft comment:
The extraction and binary copy step is commented out. Using a conditional toggle might be preferable for maintainability. - 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 code is commented out with a clear purpose ("for playground dist testing") and appears to be a temporary change. The suggestion to use a conditional toggle would add complexity for what seems to be a temporary testing situation. The comment doesn't point out a clear problem that needs fixing. The suggestion could improve maintainability if this toggling between modes happens frequently. However, if this is just for temporary testing, adding a conditional system would be overengineering. Given the context that this is for testing and the clear comment explaining why it's disabled, the current approach is reasonable and doesn't warrant additional complexity. The comment should be deleted as it suggests adding unnecessary complexity for what appears to be a temporary testing situation.
4. .github/workflows/build-vscode-release.reusable.yaml:204
- Draft comment:
The VSCode packaging steps are also commented out. If these are needed later for extension support, consider enabling them via conditionals instead of manual commenting. - 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% The comment is suggesting an alternative implementation, but the current commenting appears to be an intentional temporary change for testing. The comment header explicitly states this is "for playground dist testing". Making this suggestion now could be premature since this may be reverted after testing is complete. The comment might have a point about using conditionals being a better long-term solution. What if the testing phase extends longer than expected? While conditionals could be cleaner, the explicit commenting makes the temporary nature clear and serves as documentation. This appears to be a deliberate testing state that will likely be reverted. The comment should be deleted as it's suggesting changes to an intentionally temporary testing state, and the current implementation adequately serves its temporary purpose.
Workflow ID: wflow_LYia8kFAXZYJceAE
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 9ace5bd in 3 minutes and 36 seconds. Click for details.
- Reviewed
146lines of code in5files - Skipped
0files when reviewing. - Skipped posting
8draft 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. .github/workflows/build-vscode-release.reusable.yaml:253
- Draft comment:
Artifact name changed to 'playground-dist' (was previously playground-dist-${{ inputs.version }}). Confirm that removing the version from the artifact name is intentional. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. .github/workflows/release.yml:69
- Draft comment:
Updated artifact download config: replaced pattern 'playground-dist-*' with explicit name 'playground-dist'. Ensure this matches the updated artifact name in the build workflow. - 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 updated artifact name matches the build workflow. This is similar to asking the author to double-check or verify something, which is against the rules. Therefore, this comment should not be approved.
3. engine/baml-runtime/src/internal/llm_client/primitive/anthropic/anthropic_client.rs:150
- Draft comment:
Changed 'resolve_pdf_urls' from Always to Never. Verify this change aligns with Anthropic’s expected PDF handling 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.
4. fern/03-reference/baml_client/pdf.mdx:151
- Draft comment:
Updated PDF model compatibility documentation to a table format. Verify the table renders correctly and the provided info (e.g. model support details) is up-to-date. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify the table rendering and the accuracy of the information, which falls under asking the author to double-check things. This violates the rules.
5. fern/03-reference/baml_client/video.mdx:40
- Draft comment:
Video docs updated: API clarifications (e.g. using fromUrlToBase64 in place of fromUrlAsync), added a Warning block, and a model compatibility table. Also note the commented-out ParamField sections. Ensure these reflect the current API and are intentional. - 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. fern/03-reference/baml_client/pdf.mdx:161
- Draft comment:
There's an inconsistency in how 'base‑64' is rendered. In the table rows, it’s written as 'base‑64' (with a hyphen that’s a special character), but here it appears as 'base64' without a hyphen. Consider normalizing the formatting 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 inconsistency is real, this is an extremely minor formatting issue in documentation. The meaning is perfectly clear either way. The rules state not to make comments that are obvious or unimportant. This feels like an unimportant nitpick that doesn't affect code quality or functionality. The inconsistency could potentially confuse readers or make the documentation look less professional. Consistency in technical documentation is valuable. While consistency is good, this is too minor of an issue to warrant a PR comment. The meaning is clear either way, and this kind of nitpicking creates noise in the review process. Delete this comment as it focuses on an extremely minor formatting inconsistency that doesn't meaningfully impact the documentation's clarity or usefulness.
7. fern/03-reference/baml_client/video.mdx:13
- Draft comment:
There appears to be a possible typo in the model name 'GPT-4o'. Please verify if this should be 'GPT-4' instead. - 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 points out what appears to be a genuine typo ("GPT-4o" vs "GPT-4"). However, without access to OpenAI's exact product naming, I can't be 100% certain this isn't some new variant or internal name. The compatibility table later in the file uses just "OpenAI" without specifying the model name, which suggests the detailed model name might not be critical here. I could be wrong about this being a typo - it could be an intentional designation for a specific OpenAI model variant. Also, even if it is a typo, it doesn't affect the technical accuracy of the documentation. While the exact model name might be intentional, the inconsistency between the warning section and the compatibility table (which just uses "OpenAI") suggests this is likely a genuine typo that should be fixed for clarity and consistency. The comment should be deleted. While it might be pointing out a real typo, it's a minor documentation issue that doesn't affect functionality or technical accuracy. The meaning is clear either way.
8. fern/03-reference/baml_client/video.mdx:120
- Draft comment:
It looks like there is an extraneous '*/}' appended to the '' tag on this line. If this is not intentional, please remove the stray characters. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_g67PMSZEhRQDJYah
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 0a72970 in 1 minute and 45 seconds. Click for details.
- Reviewed
41lines of code in3files - 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. .github/workflows/build-vscode-release.reusable.yaml:250
- Draft comment:
Artifact name now includes the version, which helps prevent conflicts in artifact naming. This is a good practice. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. .github/workflows/release.yml:66
- Draft comment:
Using a pattern ('playground-dist-*') with 'merge-multiple: true' ensures all relevant artifacts are downloaded. This update improves robustness. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. fern/03-reference/baml_client/video.mdx:157
- Draft comment:
Removed the note about video demos extracting frames for Anthropic, aligning the documentation with the current API support. Ensure this reflects the intended provider capabilities. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_D1bMBUrXBofvVIXk
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed fab7145 in 2 minutes and 27 seconds. Click for details.
- Reviewed
32lines 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. .github/workflows/release.yml:69
- Draft comment:
Using 'path: .' may clutter the workspace and risk unintended file conflicts. Also, removal of merge-multiple means multiple artifacts won’t be merged. Confirm this behavior is intended. - 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% The change appears intentional and well-considered. They switched from a pattern-based download to a specific named artifact, so merge-multiple is no longer needed. The path change to '.' is fine since they're only downloading specific files (playground-dist-*.tar.gz) as shown in the ls command and files list. The workspace won't be cluttered since they're handling specific files. I might be missing potential edge cases where multiple matching artifacts could exist. The workspace could get messy if the artifact contains unexpected files. The specific artifact name and file patterns in both the ls command and release files section show clear intent and control over what files are being handled. The removal of merge-multiple is appropriate given the switch to named artifact. The comment should be deleted as it raises speculative concerns about an intentional change that appears well-controlled through specific artifact naming and file patterns.
2. .github/workflows/release.yml:75
- Draft comment:
Verify that the ls glob 'playground-dist-.tar.gz' does not match unintended files. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. .github/workflows/release.yml:87
- Draft comment:
Ensure the new file globs for the GitHub Release step precisely match the intended artifacts to avoid release failures. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_ki5XEteLwf70t40n
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 7de79bf in 2 minutes and 12 seconds. Click for details.
- Reviewed
34lines 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. .github/workflows/release.yml:57
- Draft comment:
Removal of 'environment: release' and explicit 'permissions' may remove intended release protections. Confirm this change is deliberate. - 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% This is a security-related concern about GitHub Actions permissions. Removing environment protections and explicit permissions could potentially make the release process less secure. However, the changes also make the release draft and prerelease, suggesting this is an intentional change for testing. The branch name suggests this is for testing extension fixes. I might be overvaluing the security implications - GitHub Actions has default permissions that might be sufficient. Also, this appears to be a testing branch, not production. While security is important, this appears to be an intentional change on a testing branch, with releases marked as draft/prerelease. The author likely knows what they're doing. The comment should be deleted as it's asking for confirmation of an intentional change, which violates our rules about not asking authors to confirm their intentions.
2. .github/workflows/release.yml:72
- Draft comment:
The ls command now falls back to echo on failure. Ensure that suppressing errors for missing tarballs is intended. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. .github/workflows/release.yml:80
- Draft comment:
Draft and prerelease flags are now set to true. Verify this aligns with your release intentions (it will create a draft/pre-release release). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify their intentions regarding the draft and prerelease flags. This violates the rule against asking the author to confirm their intention or ensure behavior is intended.
Workflow ID: wflow_hbBmMNOsoeehZgYB
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 5e247c3 in 3 minutes and 15 seconds. Click for details.
- Reviewed
845lines of code in7files - Skipped
1files when reviewing. - Skipped posting
8draft 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. .github/workflows/build-vscode-release.reusable.yaml:68
- Draft comment:
Remove obsolete commented-out artifact steps if they are no longer needed to clean up the workflow. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. .github/workflows/primary.yml:268
- Draft comment:
Consider adding error handling or logging in the sync-zed-repository job steps to robustly handle failures during rsync and file copy operations. - 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 job is using standard shell commands that will fail the workflow if they error. GitHub Actions provides comprehensive logging and error reporting by default. The operations are fairly simple file copies. Additional error handling would add complexity without much benefit. The current implementation follows standard GitHub Actions patterns. The comment raises a valid point about robustness. File operations can fail in unexpected ways and more detailed error messages could help debugging. GitHub Actions' built-in error handling is sufficient for this use case. The operations are simple and additional error handling would be overengineering. The comment should be deleted as it suggests adding unnecessary complexity when the default error handling is adequate.
3. .github/workflows/release.yml:47
- Draft comment:
Default version is set to '0.1.0' for branches; confirm if this is intentional or if a more descriptive version (e.g. 'canary') is desired. - 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.
4. .github/workflows/release.yml:277
- Draft comment:
The awk-based changelog extraction is brittle and may break if the CHANGELOG.md format changes; consider using a more robust method. - 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 raises a valid concern about code maintainability, it doesn't provide any specific alternative solution. The current awk implementation, while not perfect, is a common pattern for changelog parsing. The format it expects (## [version]) is a standard markdown changelog format. Without a concrete better alternative, this comment may not be actionable enough. The comment might be identifying a real maintainability issue - if the changelog format changes, this could break silently. Having a more robust parser could prevent future issues. However, the current implementation follows standard changelog conventions, and any alternative would likely be more complex. The simplicity of the current approach might actually be a feature, not a bug. The comment should be deleted because it raises a speculative concern without providing a concrete, clearly better alternative. The current implementation is simple and follows standard conventions.
5. engine/language_server/src/playground/playground_server.rs:32
- Draft comment:
Consider renaming the environment variable 'VSCODE_DEBUG_MODE_DONT_USE_THIS' to a clearer and more production-friendly name. - 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. engine/language_server/src/playground/playground_server_helpers.rs:447
- Draft comment:
For large asset downloads, consider streaming the extraction process to reduce memory usage instead of loading the entire archive into memory. - 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/apps/playground/vite.config.ts:29
- Draft comment:
Ensure that the static copy paths (e.g., for pdf.worker files) resolve correctly across different deployment environments. Consider using environment-specific configuration if needed. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
8. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/pdf-viewer.tsx:367
- Draft comment:
The hardcoded 'if (true)' condition always renders the custom PDF viewer, bypassing the iframe fallback. Consider making the viewer selection configurable (e.g., based on the PDF URL or a prop). - 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_PbU64T81y8wuZSir
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed f6a100e in 1 minute and 34 seconds. Click for details.
- Reviewed
15lines of code in1files - Skipped
1files when reviewing. - Skipped posting
0draft 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.
Workflow ID: wflow_XXWr2fIBADl9GM5i
You can customize
.github/workflows/primary.yml
Outdated
| sync-zed-repository: | ||
| runs-on: ubuntu-latest | ||
| # Only run on pushes to canary branch or manual dispatch | ||
| # if: github.ref == 'refs/heads/canary' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') |
There was a problem hiding this comment.
Debug conditions left commented out; this results in the job running unconditionally. Reinstate proper branch/event filters or use a toggle (e.g., via env var) to prevent unintended runs. Also, the use of format() in a GitHub expression may not work as expected.
Suggested change
| # if: github.ref == 'refs/heads/canary' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') | |
| if: github.ref == 'refs/heads/canary' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 1eed581 in 10 minutes and 54 seconds. Click for details.
- Reviewed
19384lines of code in299files - Skipped
1files when reviewing. - Skipped posting
39draft 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. CHANGELOG.md:12
- Draft comment:
Typo: 'primitiive' should be 'primitive'. - 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 fix, I need to consider whether fixing typos in changelogs is valuable. The changelog is a historical record and the typo doesn't affect functionality. The rules say to only keep comments that require clear code changes. Fixing typos in documentation/changelogs is not a critical code change. The typo is real and fixing it would improve the quality of the documentation. Documentation quality can be important. However, the rules clearly state to only comment if there is "clearly a code change required". A changelog typo fix is not a required code change - it's a minor documentation improvement. The comment should be removed since it points out a non-critical documentation typo rather than a required code change.
2. CHANGELOG.md:14
- Draft comment:
Typo: 'Propertly' should be 'Properly'. - 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 the comment correctly identifies a typo, the rules state that we should only keep comments that require clear code changes. This is a changelog file, not code, and fixing typos in documentation/changelogs is not a critical issue that needs to be flagged in PR comments. The typo does exist and fixing it would improve the quality of the documentation. Documentation quality is important for project professionalism. While documentation quality matters, the rules explicitly state to focus on code changes and not make purely informative comments. This is a minor documentation issue that doesn't affect functionality. Delete this comment as it only points out a minor documentation typo and doesn't address code changes or logic issues.
3. CHANGELOG.md:29
- Draft comment:
Typo: 'experiemental' should be 'experimental'. - 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 valid typo correction, the rules state that we should only keep comments that require clear code changes. Typos in changelog entries are not critical code issues. The changelog is documentation rather than functional code. The typo does affect readability and professionalism of the documentation. Documentation quality could be considered important. However, the rules explicitly state to only comment on code changes requiring fixes, not documentation or non-functional changes. The changelog is clearly documentation. This comment should be removed as it only points out a documentation typo rather than a required code change.
4. engine/baml-runtime/src/internal/llm_client/orchestrator/stream.rs:44
- Draft comment:
Typographical error: The content on line 44 appears to be a stray closing parenthesis ()) that seems out of place. Please review if this should be replaced or removed. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. engine/baml-runtime/src/tracingv2/storage/storage.rs:372
- Draft comment:
Typo: The comment on this line has a double occurrence of 'we'. Consider revising it from "only cache if we we've finished the function" to "only cache if we've finished the function". - 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 the comment is technically correct about the typo, fixing typos in code comments is extremely low value and not worth a PR comment. Comments are for documentation purposes and a minor grammatical error doesn't impact code functionality or readability significantly. This kind of nitpick creates noise in the PR review process. The typo does make the comment slightly less professional looking. Some teams may care about maintaining high quality even in comments. The benefit of fixing this minor typo is far outweighed by the cost of the PR comment and review cycle. We should focus PR comments on more substantive issues. Delete this comment as it focuses on an extremely minor typo in a code comment that doesn't meaningfully impact code quality or readability.
6. engine/generators/languages/go/generated_tests/array_types/baml_client/functions_stream.go:303
- Draft comment:
Typographical suggestion: In the comment, consider capitalizing "if" to "If" for consistency, i.e., "This should never happen. If it does, please file an issue..." - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is purely about a typographical suggestion, which is not a code suggestion or a request for a test. It doesn't address any potential issues or improvements in the code itself. Therefore, it doesn't align with the rules for useful comments.
7. engine/generators/languages/go/generated_tests/array_types/baml_client/functions_stream.go:381
- Draft comment:
Typo: In the comment, consider capitalizing "if" to "If" to maintain consistent sentence casing (e.g., "This should never happen. If it does, please file an issue ..."). - Reason this comment was not posted:
Comment was on unchanged code.
8. engine/generators/languages/go/generated_tests/array_types/baml_client/functions_stream.go:615
- Draft comment:
Typo suggestion: In the comment, "if it does" should begin with a capital letter, i.e. "If it does," to properly start the sentence. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =30%<= threshold50%This comment is suggesting a minor grammatical change in a comment within the code. While it is a valid suggestion, it doesn't contribute significantly to the functionality or logic of the code. The focus should be on code functionality rather than minor grammatical issues in comments.
9. engine/generators/languages/go/generated_tests/edge_cases/baml_client/functions_stream.go:69
- Draft comment:
Typo: The comment starts with 'if' in lowercase after the period. It would be clearer if this started with an uppercase letter (e.g. 'If it does...'). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =30%<= threshold50%This comment is pointing out a minor typographical issue in a comment within the code. While it is a suggestion for improvement, it is not critical to the functionality or understanding of the code. It does not violate the rules, but it is not particularly useful either.
10. engine/generators/languages/go/generated_tests/edge_cases/baml_client/functions_stream.go:147
- Draft comment:
Typographical note: In the comment starting on this line, the sentence "This should never happen. if it does, please file an issue..." should begin with a capitalized "If" instead of lowercase. - 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 issue in a comment, which does not affect the functionality or logic of the code. It does not provide a suggestion for code improvement or highlight a potential issue with the code itself.
11. engine/generators/languages/go/generated_tests/edge_cases/baml_client/functions_stream.go:225
- Draft comment:
Typo: In the comment, the sentence starting with "if it does..." should start with a capital 'If' for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
12. engine/generators/languages/go/generated_tests/edge_cases/baml_client/functions_stream.go:303
- Draft comment:
Typographical suggestion: In the comment starting on this line, consider capitalizing the 'if' after the full stop (i.e., change "if it does, please file an issue..." to "If it does, please file an issue...") for proper sentence case. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely about a typographical suggestion in a comment, which is not related to the functionality or correctness of the code. It doesn't provide any meaningful insight or suggestion for code improvement.
13. engine/generators/languages/go/generated_tests/edge_cases/baml_client/functions_stream.go:381
- Draft comment:
Typo: In the comment text, "if it does" should be capitalized as "If it does". - 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 minor typographical issue in a comment, which does not affect the functionality or logic of the code. It does not provide a meaningful suggestion or highlight a potential issue in the code itself.
14. engine/generators/languages/go/generated_tests/edge_cases/baml_client/functions_stream.go:459
- Draft comment:
Typo: In the comment at line 459, consider capitalizing the first letter in the sentence ('if it does' -> 'If it does') for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
15. engine/generators/languages/go/generated_tests/enums/baml_client/runtime.go:31
- Draft comment:
Typographical: The comment refers to the Go language as "go". Consider capitalizing it to "Go" for consistency and clarity. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide a specific code suggestion or highlight a potential issue. It focuses on a typographical preference rather than a functional or structural aspect of the code.
16. engine/generators/languages/go/generated_tests/literal_types/baml_client/functions_stream.go:69
- Draft comment:
Typographical issue: The comment starts with a sentence "This should never happen. if it does, please file an issue...". The 'if' should be capitalized ('If') to maintain proper sentence case. - Reason this comment was not posted:
Comment was on unchanged code.
17. engine/generators/languages/go/generated_tests/literal_types/baml_client/functions_stream.go:225
- Draft comment:
Typographical note: In the comment starting on this line, the word 'if' should be capitalized ('If') after the period for proper sentence capitalization. - 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 note, which doesn't provide a meaningful code suggestion or improvement. It doesn't align with the rules for useful comments.
18. engine/generators/languages/go/generated_tests/map_types/baml_client/functions_stream.go:147
- Draft comment:
Typo alert: In the comment on this line, consider capitalizing "if" at the beginning of the sentence for consistency. Suggestion: Change "if it does..." to "If it does...". - Reason this comment was not posted:
Comment was on unchanged code.
19. engine/generators/languages/go/generated_tests/map_types/baml_client/functions_stream.go:1083
- Draft comment:
Typo: In the comment, "if it does" should begin with a capital letter: "If it does" to maintain proper sentence casing. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is pointing out a minor typographical issue in a comment within the code. While this is a valid observation, it doesn't contribute significantly to the functionality or logic of the code. The comment is more about style and doesn't align with the rules for useful comments, which should focus on functionality, logic, or potential issues in the code.
20. engine/generators/languages/go/generated_tests/mixed_complex_types/baml_client/functions_stream.go:69
- Draft comment:
Typo: The comment on this line starts a new sentence with a lowercase letter. Consider changing "if it does" to "If it does". - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is pointing out a minor typographical issue in a comment within the code. While this is a valid observation, it doesn't impact the functionality or logic of the code. The comment is purely informative and doesn't suggest a change that affects the code's behavior or structure.
21. engine/generators/languages/go/generated_tests/mixed_complex_types/baml_client/functions_stream.go:147
- Draft comment:
Typographical suggestion: In the comment, "this should never happen. if it does, please file an issue..." consider capitalizing 'if' to 'If' for proper grammar. - 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 minor typographical suggestion. It doesn't provide any substantial feedback or code improvement suggestion. It doesn't align with the rules for good comments, which should focus on code logic, potential issues, or improvements.
22. engine/generators/languages/go/generated_tests/nested_structures/baml_client/runtime.go:31
- Draft comment:
Minor style note: the comment says "Since go uses empty strings...". Consider capitalizing "go" to "Go" to match common conventions for referring to the Go language. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and suggests a minor stylistic change that doesn't impact the functionality or correctness of the code. It doesn't align with the rules for useful comments, which should focus on code functionality, potential issues, or improvements.
23. engine/generators/languages/go/generated_tests/optional_nullable/baml_client/functions_stream.go:147
- Draft comment:
Typo: The comment on this line starts with a lowercase 'if'. Consider capitalizing it to "If it does, please file an issue..." for improved readability. - Reason this comment was not posted:
Comment was on unchanged code.
24. engine/generators/languages/go/generated_tests/optional_nullable/baml_client/functions_stream.go:381
- Draft comment:
Typo in comment: The sentence starting with "if it does..." should use a capital 'I' in "If" for proper grammatical structure. - 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 minor grammatical issue in a comment, which does not affect the functionality or logic of the code. It does not provide a code suggestion or address a potential issue in the code itself.
25. engine/generators/languages/go/generated_tests/primitive_types/baml_client/functions_stream.go:147
- Draft comment:
Typo in comment: Consider capitalizing "if" to "If" at the beginning of the sentence for consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is suggesting a minor stylistic change in a comment, which is not critical to the functionality or understanding of the code. It doesn't provide a significant improvement to the code quality or logic.
26. engine/generators/languages/go/generated_tests/primitive_types/baml_client/functions_stream.go:381
- Draft comment:
Typo: In the comment on this line, "if" should be capitalized to "If". - 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 minor typographical issue in a comment, which does not affect the functionality or logic of the code. It does not provide a meaningful suggestion or highlight a potential issue in the code itself.
27. engine/generators/languages/go/generated_tests/primitive_types/baml_client/functions_stream.go:539
- Draft comment:
Typo/naming suggestion: The variable name 'wrapped_err' uses snake_case. In Go, the conventional style is camelCase, so consider renaming it to 'wrappedErr'. - 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.
28. engine/generators/languages/go/generated_tests/primitive_types/baml_client/functions_stream.go:693
- Draft comment:
Typographical suggestion: Consider capitalizing 'if' to 'If' in the comment sentence "This should never happen. if it does, please file an issue...". - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is suggesting a typographical change in a comment within the code. This is a minor suggestion and does not impact the functionality or logic of the code. It is not a critical issue that needs to be addressed in a pull request review.
29. engine/generators/languages/go/generated_tests/primitive_types/baml_client/functions_stream.go:771
- Draft comment:
Typographical note: In the comment, consider capitalizing 'if' to 'If' for consistency after a full stop (i.e., "This should never happen. If it does, please file an issue..."). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and suggests a typographical change that doesn't impact the functionality or logic of the code. It doesn't align with the rules for useful comments, which should focus on code logic, potential issues, or improvements.
30. engine/generators/languages/go/generated_tests/recursive_types/baml_client/runtime.go:31
- Draft comment:
Typographical note: In the comment, consider capitalizing 'go' to 'Go' for proper reference to the Go programming language. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide a specific code suggestion or address a potential issue in the code. It focuses on a typographical preference rather than a functional or structural aspect of the code.
31. engine/generators/languages/go/generated_tests/semantic_streaming/baml_client/functions_stream.go:69
- Draft comment:
Typographical suggestion: Consider capitalizing the first word in the sentence. For instance, change "if it does" to "If it does" in the comment. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%This comment is suggesting a typographical change, which is not related to the functionality or logic of the code. It doesn't provide any substantial value to the code review process, as it focuses on a minor stylistic issue that doesn't affect the code's operation.
32. engine/generators/languages/go/generated_tests/semantic_streaming/baml_client/functions_stream.go:147
- Draft comment:
Typographical suggestion: Consider capitalizing the 'if' in the comment ('This should never happen. If it does, please file an issue ...') for improved readability. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =10%<= threshold50%This comment is purely about a typographical suggestion in a comment, which is not critical to the functionality or logic of the code. It doesn't provide a meaningful improvement to the code's operation or structure.
33. engine/generators/languages/go/generated_tests/union_types_extended/baml_client/functions_stream.go:225
- Draft comment:
Typographical suggestion: Consider capitalizing 'if' to 'If' in the comment (line 225) for consistency with sentence case. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =30%<= threshold50%The comment is suggesting a typographical change for consistency in comment style. This is a minor suggestion and does not impact the functionality or logic of the code. It is not asking for confirmation or testing, so it doesn't violate the rules. However, it is purely informative and doesn't provide a substantial improvement to the code's functionality or readability.
34. engine/generators/languages/go/generated_tests/unions/baml_client/functions_stream.go:68
- Draft comment:
Typo: The comment starts with 'if it does' (lowercase 'i'). Consider changing it to 'If it does' to maintain proper sentence casing. - Reason this comment was not posted:
Comment was on unchanged code.
35. engine/generators/languages/go/src/_templates/runtime.go.j2:18
- Draft comment:
Typographical suggestion: In the comment on this line, consider capitalizing "go" to "Go" to conform with standard naming conventions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is suggesting a typographical change in a comment within the code. This is a minor suggestion and does not impact the functionality or logic of the code. It seems to be purely informative and does not align with the rules for useful comments.
36. engine/generators/languages/typescript/generated_tests/asserts/baml_client/async_request.ts:25
- Draft comment:
Typographical suggestion: The import statement on line 25 does not include spacing within the curly braces. For consistency with the other import statements, consider changingimport type {Person} from "./types"toimport type { Person } from "./types". - 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. engine/generators/languages/typescript/generated_tests/classes/baml_client/partial_types.ts:23
- Draft comment:
There's an extra space in the import on this line: import type { SimpleClass } from "./types" Consider removing the extra space so it reads: import type { SimpleClass } from "./types" - Reason this comment was not posted:
Comment was on unchanged code.
38. engine/generators/languages/typescript/generated_tests/optional_nullable/baml_client/sync_request.ts:25
- Draft comment:
Typographical error: There is a missing space between the closing brace andfromin the import statement on line 25. It should beUser} from "./types". - 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. engine/generators/languages/typescript/generated_tests/sample/baml_client/partial_types.ts:23
- Draft comment:
There appears to be extra spacing in the import statement on this line (i.e.,import type { Example, Example2 }contains double spaces before the identifiers). Please remove the extra spaces. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_l0Ir8IkYQDVVK0ZT
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 8536593 in 2 minutes and 38 seconds. Click for details.
- Reviewed
133lines of code in7files - Skipped
0files when reviewing. - Skipped posting
12draft 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-lib/baml/tests/validation_files/prompt_fiddle_example.baml:427
- Draft comment:
Changed from using 'file' to 'url' for PDF input. Ensure the external URL is reliably accessible in all test environments. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. engine/baml-lib/baml/tests/validation_files/prompt_fiddle_example.baml:453
- Draft comment:
Switched the video test input from a local file to a YouTube URL. Verify that the video input handling (e.g. embed detection) works as expected. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. turbo.json:26
- Draft comment:
Increased concurrency from '15' to '20'. Confirm that your system/environment can handle the increased concurrency level without causing resource contention. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. typescript/apps/fiddle-web-app/public/_examples/all-projects/baml_src/multi-modal/pdf-input.baml:20
- Draft comment:
Updated the PDF input test to use a remote URL instead of a local file. Make sure network access is available during tests, especially in CI. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. typescript/apps/fiddle-web-app/public/_examples/all-projects/baml_src/multi-modal/video-input.baml:21
- Draft comment:
Changed the video input test to use a YouTube URL. Confirm that the embed logic properly recognizes and displays YouTube videos. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. typescript/apps/playground/vite.config.ts:24
- Draft comment:
Removed the PDF.js worker file copy configuration. Ensure the PdfViewer component still locates and loads its worker scripts correctly. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
7. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/webview-media.tsx:267
- Draft comment:
Adjusted image container styling with min-height, max-height, and overflow-hidden. Verify that these layout changes render well across different viewport sizes. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
8. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/webview-media.tsx:465
- Draft comment:
Increased icon sizes and updated color classes for media type headers. Ensure that these visual changes are consistent with the overall design system. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
9. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/webview-media.tsx:477
- Draft comment:
Updated file detail labels for PDF and video outputs to include 'url' (e.g., 'PDF url', 'Video url'). Confirm that these label changes accurately reflect the type of content being displayed. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
10. typescript/packages/playground-common/src/shared/baml-project-panel/vscode.ts:112
- Draft comment:
Removed the verbose console log for RPC WebSocket messages. Ensure adequate logging is maintained for troubleshooting in production if needed. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
11. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/webview-media.tsx:480
- Draft comment:
Typographical note: Consider capitalizing URL for consistency. For instance, 'PDF url' might be clearer as 'PDF URL' if that’s the convention used in the codebase. - 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 a very minor UI text change. While technically correct that URL is conventionally capitalized, this is not a functional issue. The comment is about pure UI text styling. According to the rules, we should not comment on pure UI changes - we should assume the author chose the styling they wanted. The capitalization of URL is a widely accepted convention in technical writing. Not following it could be seen as unprofessional. While URL capitalization is conventional, this is still just a UI text styling choice. The rules explicitly state not to comment on UI/styling changes. The current lowercase version is perfectly readable and functional. Delete this comment as it violates the rule about not commenting on UI/styling changes.
12. typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/webview-media.tsx:482
- Draft comment:
Typographical note: Consider capitalizing URL for consistency. 'Video url' could be written as 'Video URL' to align with the standard acronym presentation in the code. - 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 a UI text change suggestion. According to the rules, we should NOT comment on pure frontend UI or styling changes - we should assume the author made UI choices correctly. While URL is commonly capitalized, this is ultimately a UI styling choice that doesn't affect functionality. The comment is technically correct - URL is commonly capitalized. And it would make the code more consistent with standard conventions. However, the rules explicitly state not to comment on UI/styling changes. The casing of UI text is a UI decision that should be left to the author. Delete this comment as it relates to UI text styling which is outside the scope of what we should review.
Workflow ID: wflow_oZMOBEXkyYmhkuB5
You can customize
There was a problem hiding this comment.
Skipped PR review on 7262df6 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b724c6d in 1 minute and 15 seconds. Click for details.
- Reviewed
12lines 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. engine/language_client_python/uv.lock:3
- Draft comment:
Bumping the minimum Python version to 3.13 breaks compatibility. Ensure this is intentional given that 3.13 isn’t widely available. - 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 a dependency/version requirement change. According to our rules, we should NOT comment on dependency changes or library versions. Even though the comment raises a valid concern about Python 3.13 being too new, our rules explicitly state to ignore dependency-related issues. The comment does raise a legitimate technical concern about compatibility that could affect users. Maybe dependency issues should be flagged when they're this significant? While the concern is valid, our rules are clear about not commenting on dependency changes. The team likely has their reasons for this version bump and dependency management should be handled through other channels. Delete this comment as it violates our rule about not commenting on dependency changes or library versions.
Workflow ID: wflow_NZBxRr3RYN6UlfgN
You can customize
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
Reintroduces extension support in
baml-cli, enhances media handling in the playground, and fixes a freeze issue with base64 images.baml-cli.playground.ProxyServerinproxy.rsto handle CORS and API key injection.playground_server.rsto start a proxy server on a separate port.playground_server_helpers.rsto include RPC WebSocket and static file serving.BamlPdfandBamlVideoclasses inmedia.ts.j2for handling PDF and video media types.PdfViewerinpdf-viewer.tsxfor improved PDF rendering.webview-media.tsxto optimize media URL handling and display.Cargo.tomlto includebase64andbytesdependencies.README.mdandextension.tomlfor Zed extension support.turbo.jsonto increase concurrency to 20.This description was created by
for b724c6d. You can customize this summary. It will automatically update as commits are pushed.