Skip to content

LLM function composition #1722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 10, 2025
Merged

LLM function composition #1722

merged 1 commit into from
Apr 10, 2025

Conversation

imalsogreg
Copy link
Contributor

@imalsogreg imalsogreg commented Apr 3, 2025

Add several new syntactic forms to BAML, allowing the definition of functions whose bodies are expressions that may compose calls to other expression-bodied functions as well as the existing LLM functions.

The changes to the language in this PR are not meant to be used in production BAML apps. This is foundation work for a future, user-facing release. All syntax introduced here is very likely to change.

Closes #1707

Needs cleanup and more tests.


Important

Introduces function composition in BAML, adding expression-bodied functions and updating the type system, code generation, and testing framework to support this foundational change.

  • Behavior:
    • Introduces function composition in BAML, allowing expression-bodied functions to compose calls to other functions.
    • Not intended for production; syntax likely to change.
  • IR and Helpers:
    • Adds ExprFunctionNode and ExprFunctionWalker in ir_helpers/mod.rs.
    • Updates IRHelper trait with find_expr_fn and find_expr_fn_test methods.
  • Type System:
    • Adds Arrow type to FieldType in field_type/mod.rs.
    • Updates type coercion and checking to handle Arrow types.
  • Code Generation:
    • Updates Python, Ruby, and TypeScript code generation to handle new function types.
    • Adds handling for Arrow types in generated type definitions.
  • Testing and Validation:
    • Adds tests for expression functions in validation_files/expr.
    • Updates validation pipeline to include expression function validation and type checking.
  • Miscellaneous:
    • Updates flake.nix and flake.lock for dependency management.
    • Enhances VSCode extension with new commands and event listeners for function testing.

This description was created by Ellipsis for 0b1fa01. It will automatically update as commits are pushed.

Copy link

vercel bot commented Apr 3, 2025

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

Name Status Preview Comments Updated (UTC)
baml ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 10, 2025 3:57am

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on b6f17df in 7 minutes and 59 seconds

More details
  • Looked at 7425 lines of code in 85 files
  • Skipped 1 files when reviewing.
  • Skipped posting 51 drafted comments based on config settings.
1. next.config.mjs:17
  • Draft comment:
    Duplicate WASM asset rule pushed at L17-20 and again at L26-30. Consider deduplicating.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. ParsedResponseRender.tsx:49
  • Draft comment:
    Parsing response JSON on each render may impact performance. Consider memoizing or caching the parsed value if the response doesn't change frequently.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
3. vscode-ext/packages/vscode/src/extension.ts:192
  • Draft comment:
    Multiple registrations of 'executeCommand' request handler (L192 and L252) may be redundant; consider consolidating to a single registration.
  • 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. vscode-ext/packages/vscode/src/extension.ts:273
  • Draft comment:
    Consider removing debug console logs, e.g., those logging port info or original URL, before shipping production.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
5. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:195
  • Draft comment:
    The 'set_flashing_regions' event handler only logs debugging info without updating application state. Verify if further implementation is needed.
  • 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 that the handler isn't doing anything besides logging, we don't have enough context to know if this is actually a problem. The feature might be in development, or the logging might be intentional for debugging. Without knowing the intended behavior, we can't be certain a change is required.
    The comment violates the rule about asking for verification or confirmation. It uses "Verify if..." which is explicitly called out as likely not useful. Additionally, we don't have strong evidence that more implementation is definitely needed.
    The comment's core observation about missing implementation might be valid, but its speculative nature and request for verification make it unsuitable.
    The comment should be deleted because it asks for verification and makes speculative suggestions without strong evidence that changes are definitely required.
6. typescript/vscode-ext/packages/vscode/src/extension.ts:550
  • Draft comment:
    The animation interval of 500 ms is hard‐coded. Consider making this configurable or documenting the rationale behind this value.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
7. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:364
  • Draft comment:
    Typo: In the unreachable! message at line 364, "FieldType::Constrainted" appears to have a typographical error. Consider changing it to "FieldType::Constrained" for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
8. engine/baml-lib/baml-core/src/ir/repr.rs:503
  • Draft comment:
    Typo in commented-out code: the function name 'predend_to_be_llm_function' appears to be misspelled. It should read 'pretend_to_be_llm_function'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
9. engine/baml-lib/baml-core/src/ir/walker.rs:226
  • Draft comment:
    Minor typographical issue: In the implementation impl<'a> Walker<'a, (&'a ExprFunctionNode, &'a TestCase )>, there is an extra space before the closing parenthesis in the type parameter. Consider changing it to (&'a ExprFunctionNode, &'a TestCase) for consistency.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While the comment is technically correct about the inconsistent spacing, this is an extremely minor style issue that doesn't affect functionality. The codebase appears to be Rust, which typically has automated formatting tools like rustfmt that would handle such spacing issues automatically. Making developers manually fix tiny spacing issues isn't a good use of their time.
    The comment is factually correct and does point out an actual inconsistency. Consistent formatting can make code more readable.
    While consistent formatting is good, this is too minor to warrant a PR comment. This kind of issue should be handled by automated formatting tools, not manual code review.
    Delete the comment. The spacing issue is too minor to warrant a manual fix and should be handled by automated formatting tools.
10. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/expr_fns.rs:73
  • Draft comment:
    Typographical issue: The comment on line 73 reads 'for augmenting an passing down.' Consider correcting it to 'for augmenting and passing down.' 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 = 10% vs. threshold = 50%
    While the comment is technically correct and points out a real typo, it's an extremely minor issue that doesn't affect code functionality or readability significantly. The rules state we should not make purely informative comments or comments that are unimportant. A typo in a comment, while worth fixing, is not important enough to warrant a PR comment.
    The typo could make the code comment slightly less clear to future readers. Documentation quality is important.
    While documentation quality matters, this particular typo is so minor that readers would easily understand the intended meaning. The cost of the PR comment outweighs the benefit.
    Delete this comment as it points out an unimportant issue that doesn't meaningfully impact code quality or understanding.
11. engine/baml-lib/baml-types/src/baml_value.rs:585
  • Draft comment:
    Typo: The comment says "both classs' fields". Please correct "classs" to "classes".
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. engine/baml-lib/baml-types/src/baml_value.rs:639
  • Draft comment:
    Typo: The comment currently reads "These items have to children.". It likely should be "These items have no children."
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. engine/baml-lib/baml-types/src/expr.rs:257
  • Draft comment:
    Typo: 'encounteres' should be corrected to 'encounters' in the comment on line 257.
  • Reason this comment was not posted:
    Marked as duplicate.
14. engine/baml-lib/baml-types/src/expr.rs:253
  • Draft comment:
    Typo: 'Spetial' should be corrected to 'Special' in the comment on line 253.
  • Reason this comment was not posted:
    Marked as duplicate.
15. engine/baml-lib/baml-types/src/value_expr.rs:15
  • Draft comment:
    The comment on line 15 contains 'Repred as a string, but guaranteed to be a number.' which appears to be a typo. Consider changing 'Repred' to 'Represented', or another more appropriate term.
  • 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.
16. engine/baml-lib/baml/tests/validation_files/expr/mixed_pipeline.baml:16
  • Draft comment:
    It appears that the file is missing a newline at the end. Adding a newline at the end of the file can improve consistency and avoid potential issues with some tools.
  • 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%
    Missing newlines at end of file are typically handled by code formatters and linters. This is a test file, and the issue is very minor. The rules specifically say not to comment on obvious or unimportant issues. This seems like the kind of thing that would be caught by automated tools.
    Maybe some build systems or tools could have issues with files lacking final newlines, making this a legitimate concern?
    While true, this is exactly the kind of issue that should be caught by automated checks rather than manual review comments. If it's important enough, it should be enforced by tooling.
    Delete the comment as it's too minor and should be handled by automated tooling rather than PR comments.
17. engine/baml-lib/baml/tests/validation_files/expr/unknown_name.baml:12
  • Draft comment:
    Typographical inconsistency: In function Go3, consider revising 'fn Go3(x:int) -> int {' to 'fn Go3(x: int) -> int {' for consistency in parameter declaration spacing.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a test file specifically created to validate error handling of unknown variables and functions. The spacing inconsistency is real, but this file's purpose is to test error cases, not to demonstrate proper style. The file contains intentional errors and is part of the test suite. Style consistency is less important in test files, especially ones demonstrating error cases.
    The inconsistency is real and could be fixed easily. Consistent style even in test files might make them more readable and maintainable.
    While consistency is good, this is an error test case file where the focus should be on the error conditions being tested, not style. The comment distracts from the file's primary purpose.
    Delete the comment. Style consistency in error test files is not important enough to warrant a comment, as it distracts from the file's purpose of testing error handling.
18. engine/baml-lib/diagnostics/src/span.rs:3
  • Draft comment:
    Consider changing 'datamodel' to 'data model' in the comment on line 3 for 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.
19. engine/baml-lib/jinja-runtime/src/output_format/types.rs:420
  • Draft comment:
    Typo found in the doc comment: 'you own't get any rendering!' should be corrected to 'you won't get any rendering!'.
  • 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.
20. engine/baml-lib/jsonish/src/deserializer/semantic_streaming.rs:57
  • Draft comment:
    Typo: In the comment above process_node, 'vial @stream.with_state' should be 'via @stream.with_state'. Please correct the typo.
  • 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.
21. engine/baml-lib/parser-database/src/walkers/mod.rs:34
  • Draft comment:
    Typo: In the comment "A generic walker. Only walkers intantiated with a concrete ID type (I) are useful.", change 'intantiated' to 'instantiated'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
22. engine/baml-lib/schema-ast/src/ast/argument.rs:30
  • Draft comment:
    Typo in the example doc comment: It currently prints 'myidix', which looks like it should be 'myindex'. Please correct this to maintain consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
23. engine/baml-lib/schema-ast/src/ast/expr.rs:2
  • Draft comment:
    Consider capitalizing 'LLM' in the comment on line 2 (e.g., 'non-LLM functions') for consistency and clarity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While "LLM" is conventionally written in caps, this is a relatively minor documentation style issue. The meaning is clear either way. The rules state not to make purely informative comments and to only comment if there's clearly a code change required. Documentation style suggestions like this don't qualify as necessary code changes.
    The capitalization could affect code searchability and consistency with other documentation. Maybe this is part of a larger style guide requirement?
    Even if there is a style guide, this is too minor of an issue to warrant a PR comment. The meaning is clear, and this is purely cosmetic.
    Delete the comment as it's purely about documentation style and doesn't indicate a necessary code change.
24. engine/baml-lib/schema-ast/src/ast/expression.rs:251
  • Draft comment:
    Typo: The function name as_identifer is misspelled. It should be as_identifier to correctly reflect that it returns an identifier.
  • 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.
25. engine/baml-lib/schema-ast/src/ast/top.rs:44
  • Draft comment:
    Typo detected: in the get_type method the variant Top::Client returns "client". Consider correcting it to "client" or the intended string.
  • 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.
26. engine/baml-lib/schema-ast/src/parser/datamodel.pest:246
  • Draft comment:
    Typo detected: 'Anaonymous function.' should be corrected to 'Anonymous function.'
  • Reason this comment was not posted:
    Marked as duplicate.
27. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:234
  • Draft comment:
    There's a typo in the comment on line 234: 'panicks' should be 'panics'.
  • 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/baml-lib/schema-ast/src/parser/parse_expression.rs:269
  • Draft comment:
    In the doc comment starting at line 269, 'therefor' should be corrected to 'therefore', and the phrase 'should be contain "\n"' should be revised to 'should contain "\n"' for correct grammar.
  • 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.
29. engine/baml-lib/schema-ast/src/parser/parse_named_args_list.rs:71
  • Draft comment:
    On line 71, the unreachable!() error message refers to "parse_named_args_list:" whereas the actual function is named "parse_named_argument_list". Please update the error string for consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
30. engine/baml-lib/schema-ast/src/parser/parse_named_args_list.rs:31
  • Draft comment:
    The rule names 'openParan' (and similarly 'closeParan' on line 38) appear to be misspelled. If these rules represent 'open parenthesis' and 'close parenthesis', consider renaming them to 'openParen' and 'closeParen' for clarity, or add a comment if the naming is 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.
31. engine/baml-lib/schema-ast/src/parser/parse_schema.rs:178
  • Draft comment:
    Typo: The panic message currently says "This should never happend.". Please change it to "This should never happened." or better yet "This should never happen."
  • 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.
32. engine/baml-runtime/src/internal/prompt_renderer/render_output_format.rs:211
  • Draft comment:
    Typographical error: In the comment on line 211, the function is referred to as relevant_dat_models but the actual function name is relevant_data_models. Please update it for 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.
33. engine/baml-runtime/src/lib.rs:669
  • Draft comment:
    Typo: In the comment on line 669, 'requeset' should be corrected to 'request'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
34. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:74
  • Draft comment:
    Typographical error: In the comment, please change "I dont think we need this line anymore..." to "I don't think we need this line anymore...".
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
35. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:205
  • Draft comment:
    Typographical error: The string 'BAML_PATH_SPLTTER' likely has a typo. It should probably be 'BAML_PATH_SPLITTER'.
  • 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.
36. engine/language_client_codegen/src/python/generate_types.rs:239
  • Draft comment:
    Typo: Consider correcting "Short-circuite" to "Short-circuit" in the comment on line 239. This improves 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.
37. engine/language_client_codegen/src/ruby/generate_types.rs:43
  • Draft comment:
    Typographical error: The comment on line 43 contains 'Partial'. It looks like an extra 'j' got inserted by mistake. Consider changing it to 'Partial'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
38. engine/language_client_codegen/src/typescript/mod.rs:202
  • Draft comment:
    Typographical error: The identifier 'TypscriptLlmParser' appears to be misspelled. Consider renaming it to 'TypescriptLlmParser' for consistency with other naming conventions in the code.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
39. typescript/fiddle-frontend/next.config.mjs:33
  • Draft comment:
    Typo detected: the comment on line 33 says "watch my locak pnpm package @gloo-ai/playground-common for changes". Please correct "locak" to the intended word (e.g., "lock" or "local") for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
40. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:80
  • Draft comment:
    The comment on line 80 seems to be missing a word. Currently it reads "We don't use ASTContext.provider because we should the default value of the context". It might be clearer as "We don't use ASTContext.provider because we want to use the default value of the context" or similar. Consider rephrasing for 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.
41. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:86
  • Draft comment:
    Typo: The commented out line reads "// console.log("2Try to set flashing regions")". Consider removing the '2' to read "// console.log("Try to set flashing regions")" if that was not intended.
  • 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 code in question is explicitly marked as "TEMPORARY" and is commented out. It's debug/test code that isn't even active. Making style suggestions about commented-out temporary code that will likely be removed anyway isn't valuable. The "2" could even be intentional for testing purposes.
    Maybe the "2" indicates something important about the testing sequence or versioning that other developers need to know about?
    Given it's explicitly marked as temporary debug code, any formatting or naming conventions are irrelevant since this code isn't meant to be permanent.
    This comment should be deleted as it's making a style suggestion about temporary, commented-out debug code that isn't even active.
42. typescript/vscode-ext/packages/language-server/src/server.ts:55
  • Draft comment:
    Typo: The string 'cant load webcrypto' should be corrected to "can't load webcrypto" for proper punctuation.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
43. typescript/vscode-ext/packages/language-server/src/server.ts:305
  • Draft comment:
    Typo: In the comment, "mutliple" should be spelled as "multiple".
  • 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.
44. typescript/vscode-ext/packages/vscode/server/darwin/baml-cli:1
  • Draft comment:
    The file is missing a trailing newline at the end. While this is a very minor, cosmetic issue, adding a newline at the end of the file will improve consistency with typical file formatting standards.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a symlink file, not a regular source code file. Symlink files don't typically require trailing newlines as they are interpreted by the OS, not by code editors or compilers. The comment is making a purely cosmetic suggestion that doesn't impact functionality.
    Maybe there are some systems or tools that expect newlines even in symlink files? Maybe this could cause issues in certain environments?
    Symlinks are handled at the OS level and are widely used without trailing newlines. There's no evidence this would cause any real issues.
    The comment should be deleted as it's making a purely cosmetic suggestion about a symlink file format that doesn't impact functionality.
45. typescript/vscode-ext/packages/vscode/server/linux/baml-cli:1
  • Draft comment:
    The file is missing a newline at the end. Adding a newline would improve the file's consistency with common style guidelines.
  • 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%
    Symlink files are special files that just contain a path. Whether they have a trailing newline or not is generally irrelevant to their function. This seems like an overly pedantic comment that doesn't impact functionality. The comment is about a changed file, but the change itself (adding a symlink) works fine either way.
    Perhaps there are system tools or scripts that expect all text files to end with newlines, and this could cause issues in some edge cases?
    Even if some tools prefer newlines, for a symlink file this is extremely unlikely to cause any real issues since it's not processed as a regular text file.
    This comment is too pedantic and doesn't highlight a real issue that needs fixing. The symlink will work perfectly fine without a trailing newline.
46. typescript/vscode-ext/packages/vscode/src/extension.ts:74
  • Draft comment:
    Typo: The backgroundColor value is set to '##9333ea'. It should be '#9333ea' (only one '#' character) to correctly represent the intended color.
  • 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.
47. typescript/vscode-ext/packages/vscode/src/extension.ts:146
  • Draft comment:
    Typographical error: The debug log message 'Pushing test errorrrr' seems to contain an extra 'r' in 'errorrrr'. Consider correcting it to 'Pushing test error' or revising the message if this was unintended.
  • 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.
48. typescript/vscode-ext/packages/vscode/src/panels/WebPanelView.ts:181
  • Draft comment:
    Typo found: In the documentation comment for the _setWebviewMessageListener method, 'recieved' should be spelled as 'received'.
  • 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.
49. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:138
  • Draft comment:
    Typo: Consider changing "dont delete this." to "don't delete this." for correct punctuation.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
50. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:230
  • Draft comment:
    Typographical error: Change "doesnt" to "doesn't" in the log message for better readability.
  • 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 an extremely minor stylistic issue in a debug log message. Debug logs are for developers only and don't impact functionality. The missing apostrophe doesn't affect readability or understanding. This kind of nitpicky comment about debug logs creates unnecessary noise in the PR review.
    The code style could be seen as slightly unprofessional with the typo. Some might argue that maintaining consistent style even in debug messages is important for code quality.
    While consistent style is good, debug log messages are not user-facing and this level of nitpicking creates more friction than value in the review process. The benefit of fixing this is extremely minimal.
    Delete this comment as it's an extremely minor stylistic issue in a debug log that doesn't impact functionality or readability.
51. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:281
  • Draft comment:
    Typo: Please update "doesnt" to "doesn't" in this log message for proper punctuation.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_OYRR5lNcLSxdLFzt


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on aefca4f in 2 minutes and 54 seconds

More details
  • Looked at 2721 lines of code in 29 files
  • Skipped 0 files when reviewing.
  • Skipped posting 42 drafted comments based on config settings.
1. engine/baml-runtime/src/eval_expr.rs:156
  • Draft comment:
    Avoid unwrapping in critical substitution logic. Using unwrap in beta_reduce (line 156) may mask underlying substitution errors. Consider propagating errors instead.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/components/ParsedResponseRender.tsx:32
  • Draft comment:
    The component name 'ParsedResponseRender' is used recursively inside 'ParsedResponseRenderer'; consider renaming the inner component to avoid confusion and possible recursion issues.
  • 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. engine/baml-runtime/src/cli/serve/mod.rs:25
  • Draft comment:
    Consider adding more robust error handling for server startup and network binding. Context messages when binding to port (lines 203-208) could include suggestions for alternative ports.
  • 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. flake.nix:21
  • Draft comment:
    Ensure consistent LLVM version usage. The code uses llvmPackages_17. Verify that this is intentional and update comments if 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.
5. typescript/vscode-ext/packages/vscode/src/extension.ts:423
  • Draft comment:
    dispose timers and subscriptions properly. Check that interval timers and decorations are disposed in deactivate to avoid resource leaks.
  • 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/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:86
  • Draft comment:
    Consider error handling improvements in tests. The function runTest uses try-catch and calls clearHighlights in catch; ensure that error messaging is user-friendly.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:62
  • Draft comment:
    Review the usage of node-fetch: ensure compatibility with latest Node.js and consider using native fetch in Node 18+.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
8. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/components/ParsedResponseRender.tsx:49
  • Draft comment:
    When parsing the response JSON, handle non-object responses gracefully. The try-catch block should log parse errors and avoid silently returning raw text.
  • 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. flake.nix:135
  • Draft comment:
    Double-check configuration for test phases in the Nix build. The checkPhase disables most tests—verify that only intended library tests run.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
10. typescript/vscode-ext/packages/vscode/src/extension.ts:289
  • Draft comment:
    Proxy middleware: ensure thorough header stripping and error handling for 'baml-original-url'.
  • 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. flake.nix:22
  • Draft comment:
    Using llvmPackages_17 and related paths; ensure that these versions remain consistent across your toolchain and any dependencies that expect a specific LLVM version.
  • 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. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/components/ParsedResponseRender.tsx:14
  • Draft comment:
    The naming of ParsedResponseRenderer vs. ParsedResponseRender is confusing. Consider renaming one to clarify their roles.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. engine/baml-runtime/src/eval_expr.rs:560
  • Draft comment:
    Swallowing errors when collecting param expressions with unwrap_or(vec![]); consider handling errors explicitly for better feedback.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
14. engine/baml-runtime/src/internal/prompt_renderer/mod.rs:56
  • Draft comment:
    Using a temporary workaround in mk_fake() is acceptable, but add a TODO with a reference to planned improvements.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
15. typescript/vscode-ext/packages/vscode/src/extension.ts:252
  • Draft comment:
    Duplicate registration of 'executeCommand' request in the client; consolidate these to avoid potential conflicts.
  • 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.
16. engine/baml-runtime/src/runtime/runtime_interface.rs:560
  • Draft comment:
    When constructing parameter expressions in call_function_with_expr_events, errors are silently swallowed with unwrap_or(vec![]). Consider propagating these errors or logging them for easier debugging.
  • 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.
17. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:170
  • Draft comment:
    Tests are currently executed sequentially per batch; consider parallelizing across batches when WASM runtime issues are resolved.
  • 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.
18. typescript/vscode-ext/packages/vscode/src/extension.ts:405
  • Draft comment:
    Ensure that proper cleanup is done when disposing status bar items and the server; double-check that all subscriptions are disposed on deactivation.
  • 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.
19. engine/baml-schema-wasm/Cargo.toml:19
  • Draft comment:
    Dependencies are pinned using workspace attributes; ensure that the removal of modules (e.g., moniker) has been deliberate and tests cover any affected functionality.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
20. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:112
  • Draft comment:
    Strongly validate and sanitize any URLs in the proxy middleware to ensure that no sensitive headers or unintended origins are forwarded.
  • 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.
21. engine/baml-lib/baml-core/src/ir/ir_helpers/mod.rs:527
  • Draft comment:
    Typo: In the doc comment, 'stors its constraints' should be 'stores its constraints'.
  • 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.
22. engine/baml-lib/baml-core/src/ir/repr.rs:1531
  • Draft comment:
    Typo: In the comment within the assign_param_types_to_body_variables function, please change 'admissable' to 'admissible'.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
23. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/expr_typecheck.rs:92
  • Draft comment:
    Typographical issue: The error message string 'A Type mismatch' includes an unexpected 'A' prefix. Consider removing it to maintain consistency with other error messages.
  • 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.
24. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/expr_typecheck.rs:114
  • Draft comment:
    Typographical issue: The error message string 'B Type mismatch in lambda: ...' includes an unexpected 'B' prefix. It would be clearer to standardize this message (e.g., 'Type mismatch in lambda: ...').
  • 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.
25. engine/baml-lib/baml-types/src/baml_value.rs:610
  • Draft comment:
    Typo: In the comment, "both classs' fields in parallel" should be corrected to "both classes' fields in parallel".
  • Reason this comment was not posted:
    Comment was on unchanged code.
26. engine/baml-lib/baml-types/src/baml_value.rs:672
  • Draft comment:
    Typo: The comment "These items have to children." should be corrected to "These items have no children.".
  • 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.
27. engine/baml-lib/baml-types/src/expr.rs:237
  • Draft comment:
    Typo: Change 'Attemp' to 'Attempt' in the documentation comment describing the as_atom function.
  • Reason this comment was not posted:
    Comment was on unchanged code.
28. engine/baml-lib/baml-types/src/expr.rs:238
  • Draft comment:
    Typo: Fix the spelling 'encounteres' to 'encounters' in the as_atom function's documentation comment.
  • Reason this comment was not posted:
    Comment was on unchanged code.
29. engine/baml-lib/jinja-runtime/src/output_format/types.rs:424
  • Draft comment:
    Typo: In the doc comment above the function render_possibly_recursive_type, 'you own't get any rendering!' should be corrected to 'you won't get any rendering!'
  • 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.
30. engine/baml-lib/schema-ast/src/ast/expression.rs:252
  • Draft comment:
    Typographical error: the method name as_identifer is misspelled. It should be as_identifier.
  • 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.
31. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:234
  • Draft comment:
    In the comment on line 234, the word 'panicks' looks like a typo. It should be 'panics'.
  • 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.
32. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:274
  • Draft comment:
    On line 274, there are a couple of minor issues: 'therefor' should be 'therefore' and 'should be contain' should be corrected to 'should contain'.
  • 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.
33. engine/baml-runtime/src/lib.rs:678
  • Draft comment:
    Typo: The word 'requeset' in the comment should be corrected to 'request'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
34. engine/baml-schema-wasm/Cargo.toml:3
  • Draft comment:
    The package name on line 3 is set to "baml-schema-build", which may be a typographical error given that the file is in the baml-schema-wasm directory and the rest of the configuration suggests this package is intended for WASM targets. Please verify if this name should be "baml-schema-wasm".
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
35. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:205
  • Draft comment:
    Typo: The string 'BAML_PATH_SPLTTER' appears to be misspelled. It likely should be 'BAML_PATH_SPLITTER'.
  • 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.
36. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:74
  • Draft comment:
    Typo: In the comment starting with 'I dont think we need this line anymore', consider changing 'dont' to "don't" for correct spelling.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
37. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/components/ParsedResponseRender.tsx:64
  • Draft comment:
    Minor cosmetic issue: The className on the
    in line 64 contains an extra space ('max-h-[500px] text-xs'). While this doesn't affect functionality, cleaning up the spacing would improve consistency.
  • Reason this comment was not posted:
    Comment was on unchanged code.
38. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:87
  • Draft comment:
    In the commented-out code on line 87, there's a typographical error: "2Try to set flashing regions". Consider correcting it to "Try to set flashing regions" (or removing the extraneous '2') for 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.
39. typescript/vscode-ext/packages/vscode/src/extension.ts:70
  • Draft comment:
    Typographical error: The hex color code for 'statusBarItem.backgroundColor' is incorrectly written as '##9333ea'. It should be '#9333ea'.
  • 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.
40. typescript/vscode-ext/packages/vscode/src/extension.ts:142
  • Draft comment:
    Typographical note: The debug log message 'Pushing test errorrrr' appears to contain an extra 'r'. Consider revising to 'Pushing test error' if this is not 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.
41. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:138
  • Draft comment:
    Typographical Error: In the comment "// dont delete this.", please change "dont" to "don't" for correct grammar.
  • 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.
42. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:230
  • Draft comment:
    Typographical Error: In the log message "root path doesnt match current file", please change "doesnt" to "doesn't" to fix the grammatical error.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_Ov0LxtPUqy7SyxiK


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on e5bfc34 in 2 minutes and 54 seconds

More details
  • Looked at 232 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 12 drafted comments based on config settings.
1. engine/baml-lib/schema-ast/src/parser/datamodel.pest:115
  • Draft comment:
    Reordering of the jinja_expression alternative avoids duplicates. Verify that parser ordering still meets intended disambiguation.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to verify that the parser ordering still meets intended disambiguation, which is against the rules. It does not provide a specific suggestion or point out a specific issue with the code.
2. engine/baml-runtime/src/cli/serve/mod.rs:286
  • Draft comment:
    Changing the route handler to use an async closure with explicit extraction improves clarity. Ensure that error handling in baml_call_axum remains consistent.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to ensure consistency in error handling, which is a form of asking them to double-check their work. This violates the rule against asking the author to ensure behavior is intended or tested.
3. engine/baml-runtime/src/cli/serve/mod.rs:416
  • Draft comment:
    Simplifying the signature of baml_call_axum by handling extraction in the route closure is clearer. Confirm that no extra validation is lost.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
4. engine/baml-runtime/src/cli/serve/mod.rs:548
  • Draft comment:
    Similarly, the adjusted signature in baml_stream_axum2 avoids repeated extract annotation. Looks good.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment seems to be purely informative, as it is just stating what the code change does without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements.
5. engine/baml-runtime/src/eval_expr.rs:307
  • Draft comment:
    Similar use of 'expect("TODO")' here in map evaluation. Replace with proper error propagation when possible.
  • Reason this comment was not posted:
    Marked as duplicate.
6. engine/baml-runtime/src/test_executor/mod.rs:225
  • Draft comment:
    Changed call to create_ctx_manager now returns by value instead of Arc. Validate that lifetime and ownership semantics remain correct.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to validate lifetime and ownership semantics, which is similar to asking them to double-check or ensure behavior. This violates the rule against asking the author to confirm or ensure things. The comment does not provide a specific suggestion or point out a specific issue, making it unapproved.
7. engine/baml-runtime/src/cli/serve/mod.rs:286
  • Draft comment:
    Good update: using explicit async closures with extractors improves clarity. Ensure error paths are well‐tested.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
8. engine/baml-runtime/src/eval_expr.rs:258
  • Draft comment:
    Avoid using 'expect("TODO")' when evaluating list items. Propagate errors instead of panicking.
  • Reason this comment was not posted:
    Marked as duplicate.
9. engine/baml-runtime/src/eval_expr.rs:155
  • Draft comment:
    Avoid using unwrap inside the fold for substitution. Consider propagating errors instead of unwrapping in beta reduction.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. engine/baml-runtime/src/eval_expr.rs:174
  • Draft comment:
    Double unwrap on evaluation of LLMFunction arguments is risky. Replace 'val.unwrap().unwrap()' with proper error handling.
  • 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/baml-lib/schema-ast/src/parser/datamodel.pest:243
  • Draft comment:
    Typo: In the comment for function application, 'applicatino' should be 'application'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. engine/baml-lib/schema-ast/src/parser/datamodel.pest:246
  • Draft comment:
    Typo: In the comment for lambda (anonymous function), 'Anaonymous' should be 'Anonymous'.
  • 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_bE4Z21E6nh5QeA7q


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 2052991 in 2 minutes and 3 seconds

More details
  • Looked at 25 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. engine/baml-runtime/src/eval_expr.rs:14
  • Draft comment:
    Using a global MAX_STEPS constant improves clarity, but consider documenting or parameterizing it for flexibility. Some expressions may require more than 1000 steps.
  • 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%
  1. The constant is a reasonable default that likely works well for most cases. 2. Making it configurable adds complexity without clear benefit. 3. The comment is speculative - "may require more than 1000 steps" without evidence. 4. Documentation could be nice but isn't critical since the purpose is clear from usage.
    The comment raises a valid point about documentation and flexibility. Perhaps there are real use cases that need more steps?
    Without evidence of actual cases needing more steps, this is speculative. The code shows 1000 steps is likely sufficient, and making it configurable adds unnecessary complexity.
    Delete the comment. It makes speculative suggestions without evidence of real problems, and the constant's current implementation is reasonable.
2. engine/baml-runtime/src/eval_expr.rs:14
  • Draft comment:
    Consider adding a brief comment describing MAX_STEPS (e.g., maximum beta reduction iterations) to improve clarity.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
3. engine/baml-runtime/src/eval_expr.rs:358
  • Draft comment:
    Consider including the current iteration count in the error message (e.g., 'Max steps reached after X iterations') to aid in debugging.
  • 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_ROJoSFT8qLrzhyyg


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on c932321 in 2 minutes and 25 seconds

More details
  • Looked at 71 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 8 drafted comments based on config settings.
1. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:17
  • Draft comment:
    Removed the WebPanelView import in favor of using WebviewPanelHost. Confirm that there are no remaining references to WebPanelView.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to confirm that there are no remaining references to WebPanelView. This falls under the rule of not asking the author to confirm or double-check things. Therefore, this comment should be removed.
2. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:193
  • Draft comment:
    Removed several onRequest handlers (e.g. executeCommand, baml_settings_updated, and runtime_updated) that used WebPanelView. Ensure their removal is intentional and covered by tests, as these may relate to future function composition features.
  • 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 is factually wrong - it claims code was removed when it was actually just moved. The only real change was switching from WebPanelView to WebviewPanelHost. This kind of incorrect comment could mislead developers into thinking functionality was removed when it wasn't.
    Could the comment still be valuable in highlighting the change from WebPanelView to WebviewPanelHost? That seems to be a real change.
    While the WebPanelView to WebviewPanelHost change is real, the comment focuses on the wrong thing (removal of handlers) and would need to be completely rewritten to be useful. Better to delete it.
    Delete the comment since it is factually incorrect about handlers being removed. The handlers still exist, just in a different location in the file.
3. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:298
  • Draft comment:
    Updated serverModule assignment now uses 'language-server/out/bin'. Verify that this new path correctly reflects the build output and is consistent across environments.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to verify the correctness and consistency of a path change across environments. This falls under the category of asking the author to double-check or ensure something, which is against the rules. The comment does not provide a specific suggestion or point out a clear issue with the code.
4. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:17
  • Draft comment:
    Removed unused WebPanelView import. Ensure that any functionality relying on panel messaging now uses WebviewPanelHost.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:193
  • Draft comment:
    Removed LSP request handlers ('executeCommand', 'baml_settings_updated', 'runtime_updated'). Confirm that these removals are intentional and that no client depends on these channels.
  • 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 is factually incorrect since the handlers still exist in the file. The code was just reorganized, not removed. Making incorrect claims about code removal could mislead other reviewers or the PR author. Since the comment is based on a false premise, it should be deleted.
    Could the comment still be valuable as a prompt to verify these handlers are working correctly after being moved? Should we keep it as a reminder to test this functionality?
    No - per the rules, we should not make comments asking authors to verify or test things. The comment is factually wrong about removal, and we shouldn't convert it into a testing reminder.
    Delete the comment since it incorrectly claims code was removed when it was actually just moved within the file. The comment provides no value and could be misleading.
6. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:335
  • Draft comment:
    Updated serverModule path to use 'language-server/out/bin'. Verify that this new path works correctly in all environments and is properly included in packaging.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
7. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:138
  • Draft comment:
    Typographical error: In the comment 'dont delete this.', an apostrophe is missing. Please change it to "don't delete this.".
  • 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. typescript/vscode-ext/packages/vscode/src/plugins/language-server/index.ts:241
  • Draft comment:
    Typographical error: In the log message 'root path doesnt match current file', the word 'doesnt' is missing an apostrophe. Please change it to "doesn't match current file".
  • 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_xn9YIwHuh99fLwkC


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on 3b23034 in 2 minutes and 7 seconds

More details
  • Looked at 144 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 16 drafted comments based on config settings.
1. engine/baml-lib/schema-ast/src/parser/parse_expr.rs:66
  • Draft comment:
    Good removal of debug print statements. Ensure similar cleanup in future refactorings.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. engine/baml-runtime/src/eval_expr.rs:260
  • Draft comment:
    Avoid using unwrap/expect in evaluation loops. Replace with proper error handling if used in production.
  • 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. engine/baml-runtime/src/eval_expr.rs:285
  • Draft comment:
    Similar use of unwrap in map evaluation; consider more robust error handling for production code.
  • 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. engine/baml-runtime/src/lib.rs:316
  • Draft comment:
    Switching from manual iteration to using find_expr_fn is clear; note the commented out code for type builders—cleanup or further tests may be required.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
5. engine/baml-runtime/src/lib.rs:540
  • Draft comment:
    The branch handling expression-based functions via eval_expr::eval_to_value uses unwrap in parts. Mark as work-in-progress for cleanup.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
6. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:86
  • Draft comment:
    Rename or remove outdated debug comments. The temporary debugging helper message now reads clearly.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
7. typescript/vscode-ext/packages/vscode/src/extension.ts:342
  • Draft comment:
    Good addition: focusing the active text editor to ensure rapid styling updates; verify that this behaves as expected in all scenarios.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
8. engine/baml-lib/schema-ast/src/parser/parse_expr.rs:80
  • Draft comment:
    Remove or replace development debug prints (e.g. eprintln! calls) before production use.
  • 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. engine/baml-runtime/src/eval_expr.rs:157
  • Draft comment:
    Avoid using unwrap() during beta-reduction substitutions; propagate errors instead.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. engine/baml-runtime/src/eval_expr.rs:260
  • Draft comment:
    Replace expect('TODO') calls with proper error handling to avoid panics during evaluation.
  • 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/baml-runtime/src/eval_expr.rs:284
  • Draft comment:
    Avoid using expect('TODO') for map evaluation; use proper error propagation instead.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. engine/baml-runtime/src/lib.rs:576
  • Draft comment:
    Avoid swallowing errors with unwrap_or(vec![]); consider propagating or logging conversion errors when mapping params to Expr::Atom.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. typescript/vscode-ext/packages/vscode/src/extension.ts:70
  • Draft comment:
    Fix the background color value—remove the extra '#' (should be '#9333ea' not '##9333ea').
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
14. engine/baml-runtime/src/lib.rs:692
  • Draft comment:
    Typo: 'requeset' should be corrected to 'request' in the comment about building the request body.
  • 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.
15. typescript/vscode-ext/packages/vscode/src/extension.ts:70
  • Draft comment:
    Typographical error: The backgroundColor property is set to '##9333ea' (double hash). It should be '#9333ea'.
  • 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.
16. typescript/vscode-ext/packages/vscode/src/extension.ts:142
  • Draft comment:
    Typographical error: The log message says 'Pushing test errorrrr'. Consider correcting it to 'Pushing test error' or another appropriate message.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_wzneJwOMkfqP53b4


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

@imalsogreg imalsogreg changed the title Core support for function composition LLM function composition Apr 7, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on c5fc8db in 3 minutes and 11 seconds

More details
  • Looked at 212 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 19 drafted comments based on config settings.
1. engine/baml-runtime/src/eval_expr.rs:317
  • Draft comment:
    Consider comparing the new expression with the current state (e.g. current_expr) rather than the original expr to better detect progress in beta reduction.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. engine/baml-runtime/src/eval_expr.rs:178
  • Draft comment:
    Avoid chaining unwraps (e.g. val.unwrap().unwrap()) -- consider using proper error handling to avoid panics.
  • 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. engine/baml-runtime/src/internal/prompt_renderer/mod.rs:41
  • Draft comment:
    The pattern using 'let Some(config) = ... else { ... }' is concise; ensure that this style is documented for team consistency.
  • Reason this comment was not posted:
    Confidence changes required: 30% <= threshold 50%
    None
4. typescript/fiddle-frontend/next.config.mjs:32
  • Draft comment:
    The watchOptions regex pattern for '@baml/playground-common' is specific; ensure that it matches all intended paths and test with multiple scenarios.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
5. typescript/vscode-ext/packages/vscode/src/extension.ts:448
  • Draft comment:
    Ensure that statusBarItem is not null before calling dispose(), especially if it might not have been initialized.
  • 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/vscode-ext/packages/vscode/src/extension.ts:250
  • Draft comment:
    When setting up proxy middleware, consider uncommenting or cleaning up old commented code to avoid confusion.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
7. typescript/vscode-ext/packages/vscode/src/extension.ts:466
  • Draft comment:
    In CodeActionProvider, ensure that diagnostic.code strings follow a consistent format, as the fix text is derived by stripping '[linter]'.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
8. engine/baml-runtime/src/eval_expr.rs:158
  • Draft comment:
    Avoid using unwrap() in substitution (e.g. in beta_reduce fold) as it may panic; consider propagating errors instead.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. engine/baml-runtime/src/eval_expr.rs:169
  • Draft comment:
    Avoid chaining unwrap() after substitution; propagate errors for better context in beta reduction.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. typescript/fiddle-frontend/next.config.mjs:32
  • Draft comment:
    Verify that the watchOptions glob pattern ('/node_modules/!(@baml/playground-common)') behaves as expected across environments.
  • 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. typescript/vscode-ext/packages/vscode/src/extension.ts:387
  • Draft comment:
    The onDidChangeTextEditorSelection handler posts frequent 'update_cursor' messages; consider debouncing to avoid performance issues.
  • 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. typescript/vscode-ext/packages/vscode/src/extension.ts:454
  • Draft comment:
    Ensure the server is properly disposed (server?.close()) to avoid lingering open sockets; validate cleanup in deactivate.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
13. engine/baml-lib/baml-types/src/expr.rs:88
  • Draft comment:
    Consider enhancing the pretty-print of lambda expressions (dump_str) to include clearer argument representation.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
14. engine/baml-lib/baml-core/src/ir/ir_helpers/mod.rs:200
  • Draft comment:
    Typo: The comment currently reads "This is why a and b are swapped in the parameters check, and no in the return type check." It is likely meant to say "not in the return type check" instead of "no in the return type check." Please update 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 = 10% vs. threshold = 50%
    While the comment correctly identifies a typo, it's a very minor issue in a code comment that doesn't affect functionality. The meaning is still clear even with the typo. According to the rules, we should not make purely informative comments or comments that are obvious/unimportant. Fixing a minor typo in a comment falls into this category.
    The typo could theoretically make the comment slightly harder to understand for non-native English speakers. The comment is also technically correct in identifying the issue.
    While technically correct, this kind of minor comment cleanup doesn't meet the bar for requiring a code change. The meaning is still clear enough.
    Delete this comment as it points out a minor typo in a comment that doesn't significantly impact code understanding or functionality.
15. engine/baml-lib/baml-types/src/expr.rs:238
  • Draft comment:
    Typographical error: In the doc comment for as_atom, 'encounteres' should be corrected to 'encounters'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
16. engine/baml-lib/schema-ast/src/parser/datamodel.pest:243
  • Draft comment:
    Typographical error corrected: Changed '// Function applicatino.' to '// Function application.'.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative as it only points out a typographical correction. It doesn't provide any actionable feedback or suggestions for improvement.
17. typescript/fiddle-frontend/next.config.mjs:28
  • Draft comment:
    Typo found: In the comment on line 28, 'locak' appears, which should likely be corrected to 'local'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
18. typescript/vscode-ext/packages/vscode/src/extension.ts:70
  • Draft comment:
    Typographical error: The color code has an extra '#' in '##9333ea'. It should be '#9333ea' to ensure the correct color is applied.
  • 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.
19. typescript/vscode-ext/packages/vscode/src/extension.ts:142
  • Draft comment:
    Typographical error: The log message 'Pushing test errorrrr' appears to have a typo. Consider changing it to 'Pushing test error' or a more appropriate message.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_i31mE9EuC5ykkOXk


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 4bf910f in 2 minutes and 51 seconds

More details
  • Looked at 331 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 12 drafted comments based on config settings.
1. typescript/fiddle-frontend/next.config.mjs:24
  • Draft comment:
    Good: Trailing comma added for consistency in object literals.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:119
  • Draft comment:
    Improved readability: Arrow function now explicitly wraps parameters with parentheses and uses trailing commas consistently.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. typescript/vscode-ext/packages/vscode/src/extension.ts:21
  • Draft comment:
    Code style update: Removed semicolons and extra blank lines in variable declarations to improve consistency.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. typescript/vscode-ext/packages/vscode/src/extension.ts:337
  • Draft comment:
    Consistent formatting in command registration: Object properties now have spaces after colons and trailing commas for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:270
  • Draft comment:
    Formatting and consistency: The WebviewPanelHost message listener now formats its object literals consistently; no functional issues detected.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
6. typescript/fiddle-frontend/next.config.mjs:24
  • Draft comment:
    Good use of trailing commas for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
7. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:119
  • Draft comment:
    Explicitly wrapping the arrow function parameter in parentheses improves clarity.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
8. typescript/vscode-ext/packages/vscode/src/extension.ts:353
  • Draft comment:
    Consider the disposable registration inside the 'baml.setFlashingRegions' command handler. Repeated invocations will push new disposables into context.subscriptions, which might accumulate over time. If this command is meant to be registered only once, consider moving this disposable registration outside the handler.
  • Reason this comment was not posted:
    Comment was on unchanged code.
9. typescript/fiddle-frontend/next.config.mjs:28
  • Draft comment:
    Typographical error: In the comment on line 28, 'locak pnpm package' should be corrected to 'local pnpm package'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
10. typescript/vscode-ext/packages/vscode/src/extension.ts:69
  • Draft comment:
    The background color is set to '##9333ea' (with two '#' characters). It should likely be '#9333ea'. This is a minor typographical error.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. typescript/vscode-ext/packages/vscode/src/extension.ts:141
  • Draft comment:
    The log message 'Pushing test errorrrr' contains an extra 'r'. Consider correcting it to 'Pushing test error' or a more appropriate message.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:194
  • Draft comment:
    Typo found: Please correct 'recieved' to 'received' in the comment describing the _setWebviewMessageListener method.
  • 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_JUtogxwn0YVXolA4


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on f5fb760 in 2 minutes and 46 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. engine/baml-runtime/src/eval_expr.rs:350
  • Draft comment:
    Test annotation commented out intentionally. Consider clarifying intent: either enable #[tokio::test] for running tests or remove if not needed.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. engine/baml-runtime/src/eval_expr.rs:347
  • Draft comment:
    The #[tokio::test] attribute was commented out, disabling this test. If this test is intended to be run (even in development), consider either enabling it or marking it with #[ignore] and adding a TODO note.
  • 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 test appears to be intentionally disabled with a clear comment "Uncomment to run". This suggests it's meant to be a development/debug test that the author deliberately wants disabled by default. The test contains an assertion assert!(false) which confirms it's likely a development test. The comment doesn't add value since the code already clearly indicates its purpose.
    Maybe this test being disabled could cause issues for other developers who need to run it. Maybe there should be clearer documentation about when to enable it.
    The existing comment "Uncomment to run" is already clear and sufficient. The test's assert(false) shows it's clearly a development test that shouldn't be enabled by default.
    Delete the comment. The code's intention is already clear from the existing comment, and the test is intentionally disabled for development purposes.

Workflow ID: wflow_suFRqo8xhGR1KjXt


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on d64fe33 in 6 minutes and 31 seconds

More details
  • Looked at 7062 lines of code in 81 files
  • Skipped 1 files when reviewing.
  • Skipped posting 54 drafted comments based on config settings.
1. engine/language_client_codegen/src/openapi.rs:21
  • Draft comment:
    The OpenAPI generator code looks solid. The openapi spec schema is constructed with clear comments. No actionable issues here.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. engine/language_client_codegen/src/python/generate_types.rs:239
  • Draft comment:
    The tests for optional types and unions return the expected strings. Ensure that docstring formatting remains consistent across modules.
  • 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. engine/language_client_codegen/src/typescript/mod.rs:327
  • Draft comment:
    Overall, the TypeScript templates and conversion methods are well‐structured. Note that FieldType::Arrow remains unimplemented (todo), so ensure this path never occurs in production.
  • 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. typescript/fiddle-frontend/next.config.mjs:21
  • Draft comment:
    The webpack configuration now correctly adds a rule for .wasm files. This is good – verify that your asset/resource rule does not interfere with other built-in loaders.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to verify something, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code. Therefore, it should be removed.
5. typescript/nextjs-plugin/src/index.ts:43
  • Draft comment:
    The Next.js plugin properly detects the Next.js version and configures webpack accordingly. No issues noted.
  • 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/playground-common/src/baml_wasm_web/EventListener.tsx:81
  • Draft comment:
    The React EventListener attaches and cleans up its 'message' event listener correctly. The component style and usage of jotai atoms look appropriate.
  • 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/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:192
  • Draft comment:
    Test runner sequentially awaits test execution. Consider if parallelization would improve performance, but note the comment regarding wasm issues.
  • 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. typescript/vscode-ext/packages/vscode/src/extension.ts:286
  • Draft comment:
    The extension activation logic appears robust. Ensure proper disposal of statusBarItem and other disposables in the deactivate method.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:200
  • Draft comment:
    The webview panel host correctly sets up HTML content and message listeners. Good use of getNonce and localResourceRoots for security.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. typescript/vscode-ext/packages/vscode/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:398
  • Draft comment:
    The test-runner’s use of the VS Code API to update cursor positions and highlight spans is well integrated; the clearHighlights helper is applied on both success and error paths.
  • 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. typescript/vscode-ext/packages/vscode/src/extension.ts:427
  • Draft comment:
    The CodeLens provider and commands are registered correctly. Ensure that any asynchronous commands do proper error handling.
  • 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. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:427
  • Draft comment:
    The webview panel message listener correctly retrieves resources and validates input. Keep an eye on potential URI parsing issues.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. typescript/vscode-ext/packages/vscode/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:398
  • Draft comment:
    The test-runner’s integration with VS Code for cursor updates and highlight clearance is well handled; the clearHighlights helper is applied consistently.
  • 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. typescript/vscode-ext/packages/vscode/src/extension.ts:427
  • Draft comment:
    The CodeLens provider and commands registration appears well-implemented. Double-check asynchronous error handling for robustness.
  • 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.
15. .github/workflows/primary.yml:152
  • Draft comment:
    Remove the trailing space after 'macos-latest' to maintain consistency and avoid potential formatting warnings.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
16. .github/workflows/primary.yml:174
  • Draft comment:
    It appears that the file is missing a newline at the end. Please add a newline at the end of the file to adhere to best practices.
  • 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.
17. engine/baml-lib/baml-core/src/ir/ir_helpers/mod.rs:207
  • Draft comment:
    In the comment explaining that functions are covariant in their return type and contravariant in their arguments, the phrase 'and no in the return type check' appears to be a typographical error. Consider changing it to 'and not in the return type check' 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 comment is technically correct about the typo, it's a very minor issue in a code comment that doesn't affect functionality. The meaning is still clear even with the typo. The rules say not to make purely informative comments or comments that are obvious/unimportant. This seems to fall into that category.
    The typo could potentially confuse readers trying to understand the complex concept of function covariance/contravariance. Clear documentation is important for maintainability.
    While clear documentation is valuable, this typo is minor enough that most readers would understand the intended meaning. The comment doesn't add enough value to justify the overhead of a PR comment.
    Delete the comment as it points out a very minor typo that doesn't significantly impact code understanding and falls under the rule against purely informative comments.
18. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:359
  • Draft comment:
    Typographical suggestion: Consider replacing 'A json value may not be coerced into a function type' with 'A JSON value may not be coerced into a function type' for consistent terminology.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While technically correct that JSON should be capitalized, this is a very minor style issue in an error message. The meaning is completely clear either way. The rules say not to make comments that are obvious or unimportant. Style nitpicks about capitalization in error messages seem to fall into this category.
    The capitalization of JSON is a widely accepted standard, and consistency in error messages can be important for professionalism and documentation.
    While technically correct, this level of nitpicking about capitalization in error messages doesn't meet the bar of being an important enough issue to block or modify the PR. The current message is perfectly understandable.
    Delete this comment as it's too minor of a style issue to be worth addressing in a PR review.
19. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:363
  • Draft comment:
    Typographical error: The message 'The return value of distribute_constraints can never be FieldType::Constrainted' contains a typo. It should likely read 'Constrained'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
20. engine/baml-lib/baml-core/src/ir/walker.rs:226
  • Draft comment:
    Typographical/cosmetic issue: In the type parameter for Walker<'a, (&'a ExprFunctionNode, &'a TestCase )>, there's an extraneous space before the closing parenthesis. This is inconsistent with other similar type signatures. Please remove the extra space to maintain consistent formatting.
  • 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 inconsistency, this is a very minor formatting issue. The code will work exactly the same either way. Most teams would handle this kind of minor formatting with automated tools like rustfmt rather than code review comments. The comment doesn't point out any functional issues or suggest meaningful improvements.
    The formatting inconsistency could make the code slightly harder to read and maintain if different styles are mixed throughout the codebase.
    While consistency is good, this level of nitpicking about spaces creates noise in code reviews and distracts from more important issues. This should be handled by automated formatting tools.
    Delete this comment. Minor formatting issues like extra spaces should be handled by automated tools rather than manual code review comments.
21. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/expr_fns.rs:73
  • Draft comment:
    Typo in comment: consider changing "an passing down." to "and passing down."
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While the comment is technically correct about the typo, our rules state we should not make purely informative comments. A typo in a comment is a very minor issue that doesn't affect code functionality. The comment doesn't suggest any required code changes - it's just a style/grammar fix.
    The typo could make the comment slightly less clear to future readers. Documentation quality is important.
    While documentation is important, this is an extremely minor typo that doesn't significantly impact readability. The meaning is still clear from context.
    Delete this comment as it's too minor and doesn't suggest any required code changes.
22. engine/baml-lib/baml-types/src/baml_value.rs:612
  • Draft comment:
    Typo: In the comment within the zip_meta function, 'classs' should be corrected to 'classes'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
23. engine/baml-lib/baml-types/src/value_expr.rs:15
  • Draft comment:
    Typo in the comment: "Repred as a string, but guaranteed to be a number." should likely be "Represented as a string, but guaranteed to be a number."
  • 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.
24. engine/baml-lib/baml/tests/validation_files/expr/constructors.baml:22
  • Draft comment:
    There's a missing newline at the end of the file. Consider adding a newline after line 22 to adhere to common file formatting conventions.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While having a newline at the end of files is a common convention, this is a very minor formatting issue that would typically be handled by an editor or linter automatically. The rules state not to make comments that are obvious or unimportant. This seems like a trivial formatting issue that doesn't impact code functionality.
    Missing newlines at end of file can cause issues with some tools and git diffs. It's a widely accepted best practice.
    While true, this is exactly the kind of minor formatting issue that should be handled by automated tools rather than manual review comments. It doesn't warrant human attention.
    Delete this comment as it's too minor of a formatting issue to warrant a review comment, and such issues should be handled by automated tooling.
25. engine/baml-lib/baml/tests/validation_files/expr/expr_fn.baml:11
  • Draft comment:
    The file is missing a trailing newline at the end. Please add a newline after the last line to ensure consistent file formatting.
  • 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 having a trailing newline is a common convention, this kind of formatting issue is typically handled automatically by IDE settings, git configurations, or linters. It's a very minor issue that doesn't affect functionality. The rules specifically state not to make comments about obvious or unimportant things.
    A missing trailing newline can cause issues with some tools and make diffs less clean. It's a real formatting standard.
    While true, this is exactly the kind of minor formatting issue that should be handled by automated tools rather than manual review comments. It's too trivial for a PR comment.
    The comment should be deleted as it's too minor and would typically be handled by automated tooling rather than manual review.
26. engine/baml-lib/baml/tests/validation_files/expr/expr_full.baml:71
  • Draft comment:
    There is no newline at the end of the file. Adding a newline at the end is a minor formatting improvement needed to adhere to standard POSIX file conventions.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is purely informative, suggesting a minor formatting improvement. It doesn't ask for a specific action or provide a code suggestion. It doesn't align with the rules for good comments.
27. engine/baml-lib/baml/tests/validation_files/expr/expr_list.baml:7
  • Draft comment:
    Please add a newline at the end of the file to adhere to the common convention. This is a minor typographical issue.
  • 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 adding a trailing newline is a common convention, this seems like a very minor formatting issue. Most modern IDEs and git configurations handle this automatically. This is more of a style preference than a functional issue. The comment doesn't point out any logical or functional problems with the code.
    The trailing newline convention does help with git diffs and some Unix tools. Some teams consider this an important standard.
    While technically correct, this is too minor of an issue to warrant a PR comment. It's the kind of thing that should be handled by automated tooling or pre-commit hooks rather than manual review comments.
    Delete this comment as it's too minor and obvious of an issue to warrant a review comment. This should be handled by automated tooling.
28. engine/baml-lib/baml/tests/validation_files/expr/expr_list.baml:6
  • Draft comment:
    Line 6 has an indentation of only one space which is inconsistent with the 4-space indentation used in the previous function. Please consider aligning the indentation 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 = 20% vs. threshold = 50%
    This is a test file being added, and the comment points out a real indentation inconsistency. However, it's a minor style issue in a test file. Test files often have more relaxed style requirements. The inconsistency doesn't affect functionality or readability significantly.
    The comment is technically correct, but am I being too pedantic about style in a test file? Would fixing this really improve code quality meaningfully?
    While it's a test file, consistent style helps maintainability and readability. However, this is such a minor issue that it may not be worth the noise of a PR comment.
    While technically correct, this style comment on a test file is too minor to warrant a PR comment. It would create noise without adding significant value.
29. engine/baml-lib/baml/tests/validation_files/expr/top_level_binding.baml:6
  • Draft comment:
    The file does not end with a newline. Please add a newline at the end of the file for consistency and to avoid potential issues with some tools.
  • 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%
    Missing newlines at end of files can cause issues with some tools and is generally considered good practice. This is a new file, so the comment is about a change. The suggestion is clear and actionable. However, this is a very minor issue that would likely be caught by linters or formatters, and doesn't affect functionality.
    This could be considered too minor of an issue to comment on. It might be automatically fixed by tooling.
    While minor, missing newlines can cause real issues and the fix is trivial. However, this is exactly the kind of thing that should be handled by automated tooling rather than manual review comments.
    The comment should be removed as it's too minor and is better handled by automated tooling like linters.
30. engine/baml-lib/baml/tests/validation_files/expr/unknown_name.baml:12
  • Draft comment:
    In function 'Go3', the parameter 'x:int' is written without a space after the colon, unlike the other functions (e.g., 'x: int' in 'Go'). Consider adding a space after the colon 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%
    This is a test file specifically created to test validation of unknown names/variables. The spacing inconsistency is minor and doesn't affect functionality. The file's purpose is to test error cases, not to demonstrate proper style. Making style suggestions on test files, especially those focused on error cases, adds noise without value.
    The inconsistency could theoretically make the test file harder to read. Also, maintaining consistent style even in test files could be important for setting good examples.
    The file's primary purpose is to test error handling, and the style inconsistency doesn't impact its ability to serve that purpose. The comments in the file are focused on the actual error cases being tested.
    Delete this comment as it's a minor style issue on a test file specifically created to test error handling scenarios.
31. engine/baml-lib/jinja-runtime/src/output_format/types.rs:424
  • Draft comment:
    Typographical error: The comment says "you own't get any rendering!". Please change "own't" to "won't" for 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.
32. engine/baml-lib/jsonish/src/deserializer/semantic_streaming.rs:57
  • Draft comment:
    Typo: In the comment describing '@stream.with_state', 'vial' should be corrected to 'via'.
  • 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.
33. engine/baml-lib/jsonish/src/deserializer/semantic_streaming.rs:262
  • Draft comment:
    Typo: In the comment for the needed_fields function, 'implemenation' should be corrected to 'implementation'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
34. engine/baml-lib/schema-ast/src/ast/expression.rs:251
  • Draft comment:
    Typo: The method name 'as_identifer' is misspelled. It should be 'as_identifier' for consistency and 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.
35. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:234
  • Draft comment:
    Typo: The comment on the hex logic mentions "panicks" which should be corrected to "panics".
  • 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.
36. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:271
  • Draft comment:
    Typo in the comment for parsing JinjaExpression: it currently reads "therefor the backing string should be contain "\n""; this should be corrected to "therefore the backing string should contain "\n"".
  • 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/baml-lib/schema-ast/src/parser/parse_schema.rs:180
  • Draft comment:
    Typographical error: In the panic message at line 180, "happend" should be "happened".
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
38. engine/baml-runtime/src/lib.rs:692
  • Draft comment:
    Typo: 'requeset' should be changed to 'request' in the TODO comment about the request body.
  • 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/baml-schema-wasm/Cargo.toml:3
  • Draft comment:
    Typographical error: The package name is set to 'baml-schema-build' even though the file is located under 'baml-schema-wasm'. Please confirm if this is intentional or correct it to 'baml-schema-wasm' if 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.
40. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:68
  • Draft comment:
    Typo: Consider changing "I dont think" to "I don't think" for proper punctuation.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
41. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:198
  • Draft comment:
    Typo: It appears 'BAML_PATH_SPLTTER' should likely be 'BAML_PATH_SPLITTER' (with an extra 'I' after SPL). Please confirm the intended naming.
  • 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.
42. engine/language_client_codegen/src/python/generate_types.rs:238
  • Draft comment:
    Typo: The comment says "Short-circuite". Consider changing it to "Short-circuit" for 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.
43. engine/language_client_codegen/src/python/generate_types.rs:286
  • Draft comment:
    It looks like the format macro is invoked with square brackets (format![...]) instead of parentheses (format!(...)). This might be a typographical error. Please verify if this is 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.
44. engine/language_client_codegen/src/ruby/generate_types.rs:43
  • Draft comment:
    Typo in the comment: 'Partial' should likely be 'Partial' to maintain consistency and 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.
45. engine/language_client_codegen/src/typescript/mod.rs:202
  • Draft comment:
    Typographical error: 'TypscriptLlmParser' should be renamed to 'TypescriptLlmParser' for consistency with the rest of the codebase.
  • 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.
46. flake.nix:31
  • Draft comment:
    Unresolved merge conflict markers detected in the toolchain list (lines 31-35). Please resolve the conflict by choosing the appropriate version of the code and removing the conflict markers before merging.
  • Reason this comment was not posted:
    Marked as duplicate.
47. typescript/fiddle-frontend/next.config.mjs:28
  • Draft comment:
    Typo: In the comment 'watch my locak pnpm package @gloo-ai/playground-common for changes', 'locak' appears to be a typo. Consider correcting it to 'local' (or appropriate intended word).
  • Reason this comment was not posted:
    Comment was on unchanged code.
48. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:80
  • Draft comment:
    Typo in the comment on line 80: "We don't use ASTContext.provider because we should the default value of the context" seems to be missing a word (perhaps 'use'). Consider updating it to something like "We don't use ASTContext.provider because we should use the default value of the context".
  • 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.
49. typescript/vscode-ext/packages/language-server/src/server.ts:55
  • Draft comment:
    Typographical error: Consider changing "cant load webcrypto" to "can't load webcrypto" for proper spelling.
  • 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.
50. typescript/vscode-ext/packages/language-server/src/server.ts:305
  • Draft comment:
    Typographical error: In the comment, change "mutliple baml_src dirs" to "multiple baml_src dirs".
  • 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.
51. typescript/vscode-ext/packages/vscode/server/linux/baml-cli:1
  • Draft comment:
    Consider adding a newline at the end of the file to adhere to common file formatting conventions.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
    Symlink files are special files that just contain a path. Adding a newline to a symlink file could actually break the symlink functionality. This is not a regular text file where the newline convention applies. The comment shows a lack of understanding of how symlinks work.
    Maybe there are some systems or tools that expect newlines even in symlink files? Maybe this is a platform-specific concern?
    No, symlinks are handled at the filesystem level and adding a newline would actually modify the target path. The comment is technically incorrect.
    Delete this comment as it suggests a change that would break the symlink functionality.
52. typescript/vscode-ext/packages/vscode/src/extension.ts:69
  • Draft comment:
    Typographical error: In the assignment to statusBarItem.backgroundColor, the color value is set to '##9333ea' with an extra '#' at the beginning. This should probably be corrected to '#9333ea'.
  • 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.
53. typescript/vscode-ext/packages/vscode/src/extension.ts:141
  • Draft comment:
    Typographical error: The debug log message 'Pushing test errorrrr' appears to have an extra 'r' in 'errorrrr'. Consider revising it to a standard message (e.g. 'Pushing test error').
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
54. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:194
  • Draft comment:
    Typo found: 'recieved' should be corrected to 'received'.
  • 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_fqggWWGptVG8DC36


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

Ok(())
} else {
diagnostics.push_error(DatamodelError::new_validation_error(
"A Type mismatch",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message string "A Type mismatch" (line 92-96) includes an unexpected letter prefix ('A'). This seems inconsistent compared to other messages and might confuse users. Consider removing or standardizing these prefixes (e.g. remove 'A') for clarity.

Suggested change
"A Type mismatch",
"Type mismatch",

Some(expr) => format!("..{}", expr.dump_str()),
None => String::new(),
};
format!("Class({} {{ {}{} }}", name, fields, spread)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typographical suggestion: The format string for the ClassConstructor pretty-print seems to be missing a closing parenthesis. Consider changing it to something like format!("Class({} {{ {}{} }})", name, fields, spread) for balanced output.

Suggested change
format!("Class({} {{ {}{} }}", name, fields, spread)
format!("Class({} {{ {}{} }})", name, fields, spread)

@@ -223,6 +261,20 @@ impl TopId {
}
}

pub fn as_toplevel_assignment_id(self) -> Option<TopLevelAssignmentId> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method as_toplevel_assignment_id (lines 264-268) is virtually identical to as_top_level_assignment_id (lines 292-296). For consistency with the rest of the code (using 'TopLevel' with an underscore) and to avoid duplicate functionality, consider removing or renaming one of these methods.

@imalsogreg imalsogreg force-pushed the greg/expr-v1 branch 2 times, most recently from b11fc31 to a2b097d Compare April 7, 2025 20:50
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on a2b097d in 4 minutes and 34 seconds

More details
  • Looked at 7053 lines of code in 81 files
  • Skipped 1 files when reviewing.
  • Skipped posting 65 drafted comments based on config settings.
1. engine/language_client_codegen/src/ruby/field_type.rs:60
  • Draft comment:
    Arrow types are currently marked with a TODO. In generated Ruby type definitions, consider explicitly rejecting them to aid debugging.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. typescript/vscode-ext/packages/vscode/src/extension.ts:69
  • Draft comment:
    Hex color value for backgroundColor includes an extra '#' ("##9333ea"). It should be corrected to '#9333ea'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:112
  • Draft comment:
    Remove or reduce debug logging of 'selectedFunc' to avoid unnecessary console noise.
  • 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. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:193
  • Draft comment:
    Consider parallelizing test execution within each batch to improve performance if supported by the runtime.
  • 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/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:241
  • Draft comment:
    In the proxy router, validate the 'baml-original-url' header thoroughly. Ensure that malformed header values are handled appropriately.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to ensure that malformed header values are handled appropriately, which is a form of asking them to double-check their work. This violates the rule against asking the author to ensure behavior is intended or tested.
6. engine/language_client_codegen/src/python/generate_types.rs:365
  • Draft comment:
    Arrow types are not supported here (todo!). Please ensure that users cannot accidentally use arrow function types in BAML code generation. This is a known limitation but should be clearly documented and/or validated upstream.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 30% <= threshold 50%
    The comment is informative and suggests ensuring that users cannot accidentally use arrow function types in BAML code generation. It also mentions documenting or validating this limitation upstream. However, it doesn't provide a specific code suggestion or ask for a specific test to be written. It seems to be more of a general advice rather than a specific actionable comment.
7. engine/language_client_codegen/src/ruby/field_type.rs:60
  • Draft comment:
    Consistently, arrow types are marked as unsupported. Confirm that this limitation is acceptable for now and that future work will either implement composition or properly error–if arrow types appear in user code.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to confirm a limitation and future work plans, which violates the rule against asking for confirmation of intentions or future plans. It does not provide a specific code suggestion or request for a test.
8. flake.nix:1
  • Draft comment:
    The flake file appears to correctly pin nixpkgs to nixos-24.05 and defines inputs and outputs appropriately. No action needed.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
9. typescript/fiddle-frontend/next.config.mjs:21
  • Draft comment:
    The webpack configuration includes proper experiment flags and adds a WASM rule. If possible, consider verifying that the ignore pattern in watchOptions works in all environments.
  • Reason this comment was not posted:
    Confidence changes required: 30% <= threshold 50%
    None
10. typescript/nextjs-plugin/src/index.ts:3
  • Draft comment:
    The getNextJsVersion method handles fallback gracefully. Consider logging errors for debugging (if not too noisy) but this approach is acceptable.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50%
    None
11. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:80
  • Draft comment:
    Event listener setup and useEffect hooks appear standard. The use of debounced callbacks for file updates is appropriate.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
12. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:80
  • Draft comment:
    Test runner uses clearHighlights after errors and at end-of-run. This ensures UI cleaning. Ensure that test status updates (queued, running, done) are consistent with backend results.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50%
    None
13. typescript/vscode-ext/packages/vscode/src/extension.ts:240
  • Draft comment:
    The extension code properly registers commands, sets up a proxy, and disposes resources. Check that all disposables are correctly released on deactivate.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50%
    None
14. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:86
  • Draft comment:
    WebviewPanelHost sets up the panel and message listeners correctly. Ensure that _disposables are cleared and that telemetry events are throttled if necessary.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50%
    None
15. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:116
  • Draft comment:
    The use of vscode.postMessage for flashing regions is correct. Double-check that conversion of spans (start_line, start, etc.) correctly maps to editor ranges.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50%
    None
16. typescript/vscode-ext/packages/vscode/src/extension.ts:362
  • Draft comment:
    When establishing the proxy middleware in the extension, ensure that header manipulations (removing 'baml-original-url', 'origin') do not impact other requests. Validate your regex for file extensions in GET requests is robust.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
17. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:168
  • Draft comment:
    Webview HTML content is constructed with proper nonce and local resource URIs. Verify that the getUri() helper produces correct URIs when packaged.
  • Reason this comment was not posted:
    Confidence changes required: 10% <= threshold 50%
    None
18. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:200
  • Draft comment:
    The _setWebviewMessageListener handler is comprehensive, handling many commands. Ensure all RPC messages are correctly processed, particularly custom commands like 'LOAD_ENV' and 'LOAD_AWS_CREDS', with appropriate error handling.
  • Reason this comment was not posted:
    Confidence changes required: 30% <= threshold 50%
    None
19. engine/baml-lib/baml-core/src/ir/ir_helpers/mod.rs:198
  • Draft comment:
    Typographical error: In the comment explaining the covariance/contravariance of arrow functions, it says "and no in the return type check." Please change "no" to "not" 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 this is about a changed line in the diff, comments about typos in code comments are generally not important enough to keep. The typo doesn't affect functionality and is a very minor readability issue. The comment is technically correct but not impactful enough to warrant a PR comment.
    The typo could make the explanation about covariance/contravariance slightly harder to understand for someone learning about these type system concepts.
    While true, this is still an extremely minor issue. Code comments can be fixed in follow-up cleanup PRs rather than cluttering PR reviews with typo fixes.
    Delete the comment as it's too minor of an issue to warrant a PR comment, even though it's technically correct.
20. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:359
  • Draft comment:
    Minor: In the error message for FieldType::Arrow, consider capitalizing 'json' to 'JSON' for consistency with standard naming. It currently reads "A json value may not be coerced into a function type".
  • 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 technically correct that JSON is typically capitalized, this is a very minor style issue that doesn't affect functionality. The comment is about a newly added error message, so it is about changed code. However, error messages are meant for developers and the meaning is perfectly clear either way.
    The comment is technically correct and follows standard naming conventions. Error messages are part of the API and consistency in terminology could be valuable.
    While technically correct, this is an extremely minor style issue that doesn't meaningfully impact code quality or user experience. The current message is perfectly clear and understandable.
    Delete the comment. While technically correct, it's too minor of a style issue to warrant a PR comment.
21. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:364
  • Draft comment:
    Trivial: There appears to be a typographical error in the unreachable message: "The return value of distribute_constraints can never be FieldType::Constrainted". It seems like it should be "FieldType::Constrained".
  • Reason this comment was not posted:
    Comment was on unchanged code.
22. engine/baml-lib/baml-core/src/ir/repr.rs:1973
  • Draft comment:
    Typographical Error: In the comment for the Prompt enum, 'stirng' should be corrected to 'string'.
  • 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.
23. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/expr_fns.rs:73
  • Draft comment:
    Typo in comment: "for augmenting an passing down." should probably be "for augmenting and passing down" to improve clarity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While this is technically a valid correction, it's an extremely minor issue in a comment. The meaning is still clear despite the typo. According to the rules, we should not make purely informative comments or comments that are obvious/unimportant. A typo in a comment that doesn't significantly impact understanding falls into this category.
    The typo could be seen as affecting code quality and documentation standards. Clear documentation is important for maintainability.
    While documentation quality matters, this particular typo is so minor that fixing it provides negligible value and the comment creates unnecessary noise in the PR review.
    Delete the comment as it points out an unimportant issue that doesn't meaningfully impact code quality or understanding.
24. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/expr_typecheck.rs:115
  • Draft comment:
    The error message "B Type mismatch in lambda:" on line 115 includes a prefixed letter 'B' that seems out of place. It is recommended to remove or revise this prefix to maintain clarity and consistency in error messages.
  • 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 is technically about a code change (new file), the suggestion is very minor and doesn't affect functionality. The letter prefixes could be intentional to help with error tracking/debugging. Without understanding the broader error handling strategy, removing them could be counterproductive. The prefixes appear to be used systematically (A, B) rather than randomly.
    The letter prefixes could be part of an intentional error tracking system. Without understanding the full codebase's error handling strategy, removing them might break error debugging workflows.
    However, if the prefixes were truly important, they should be documented or used more consistently throughout the codebase. The current usage seems arbitrary.
    The comment should be deleted because it suggests a change that could interfere with error tracking without clear benefit, and we don't have enough context to be sure the change would be an improvement.
25. engine/baml-lib/baml-types/src/baml_value.rs:610
  • Draft comment:
    Typo: In the comment inside the zip_meta function, "both classs' fields" should be corrected to "both classes' fields" for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
26. engine/baml-lib/baml-types/src/baml_value.rs:671
  • Draft comment:
    Typo: In the comment under the match for BamlValueWithMeta variants, "These items have to children." should be corrected to "These items have no children."
  • 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.
27. engine/baml-lib/baml-types/src/value_expr.rs:15
  • Draft comment:
    Typo found: Consider replacing 'Repred as a string, but guaranteed to be a number.' with 'Represented as a string, but guaranteed to be a number.' for 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.
28. engine/baml-lib/baml/tests/validation_files/expr/constructors.baml:22
  • Draft comment:
    Trivial: Please add a newline at the end of the file to follow common file formatting conventions.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The rules state not to make comments that are obvious or unimportant. Missing newlines at end of file are typically handled automatically by IDE settings or git configurations. This is an extremely minor issue that doesn't affect functionality. The rules emphasize focusing on logic issues rather than pure formatting.
    The missing newline could cause minor git diff issues or affect some tools' behavior. It is a real, objective issue that was detected.
    While technically correct, this is exactly the kind of trivial formatting issue that the rules tell us to ignore. It's too minor to warrant a PR comment.
    This comment should be deleted as it violates the rule about not making obvious or unimportant comments. The issue is too trivial.
29. engine/baml-lib/baml/tests/validation_files/expr/constructors.baml:18
  • Draft comment:
    Trivial: The indentation for the member variables in class 'Person' (lines 19-21) is inconsistent with that in class 'MyClass' (lines 2-3). Consider using consistent indentation (e.g., 4 spaces) across all class definitions.
  • 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 is technically correct about the inconsistency, this appears to be a test file (it's in a tests/ directory). Test files often have intentionally varied syntax to test different parsing scenarios. Additionally, indentation in most languages is a style issue that would be better handled by an auto-formatter rather than manual review comments.
    The inconsistent indentation could make the code harder to read and maintain, and might indicate deeper structural issues in how the code is organized.
    Since this is a test file, varied formatting might be intentional for testing purposes. Even if not, this is a minor style issue that should be handled by automated tools rather than PR comments.
    Delete this comment as it's a minor style issue in a test file that would be better handled by automated formatting tools.
30. engine/baml-lib/baml/tests/validation_files/expr/expr_full.baml:3
  • Draft comment:
    There is a potential typographical error with the client name 'GPT4o' on lines 3 and 60. Please confirm if 'GPT4o' is the intended identifier or if it should be 'GPT4' (or similar).
  • 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 suggests there might be a typo, but the consistent usage and explicit client definition shows this is intentional. The comment is asking for confirmation of intention, which violates our rules. We should not ask authors to confirm their intentions when the code is consistent and complete.
    Maybe there's a company-wide naming convention for GPT models that this violates? Maybe 'o' suffix has special meaning?
    Even if there were naming conventions, this is a test file and the client is properly defined. The code is internally consistent and functional.
    Delete the comment. It violates the rule against asking authors to confirm their intentions, and the code shows clear evidence that GPT4o is the intended name.
31. engine/baml-lib/baml/tests/validation_files/expr/mixed_pipeline.baml:16
  • Draft comment:
    The file is missing a newline at the end. Adding a newline at the end of the file would improve consistency with typical file style guidelines.
  • 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 about a minor formatting issue. While having a newline at the end of files is a common convention, this kind of issue should typically be handled by automated formatters or linters rather than manual code review comments. The rules specifically state to not make comments that are obvious or unimportant, and to focus on logic rather than style issues.
    This could be considered a valid style issue that might cause problems with some tools or version control systems. Some would argue it's worth fixing.
    While technically correct, this is exactly the kind of minor formatting issue that should be handled by automated tools rather than taking up reviewer and developer time in code review.
    This comment should be removed as it's a minor formatting issue that doesn't warrant manual intervention in code review.
32. engine/baml-lib/baml/tests/validation_files/expr/mixed_pipeline.baml:2
  • Draft comment:
    The identifier 'GPT4o' used on line 2 (and consistently on line 10 and in the model name on line 13) might be a typographical error. If the intention was to refer to a GPT-4 model, consider renaming it to 'GPT4' or 'GPT-4' 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 = 10% vs. threshold = 50%
    The 'o' suffix appears consistently in both the client name and model name. This is a test file, and the naming could be intentionally different from standard GPT-4 to test something specific. We shouldn't assume it's a typo when it's used consistently. The comment is speculative and asks for confirmation of intention, which violates our rules.
    Maybe the 'o' suffix really is a typo that was consistently copied throughout the file. Maybe there's a convention I'm not aware of.
    Even if it is a typo, asking for confirmation of intention violates our rules. The consistent usage suggests deliberate naming, and this is a test file where such variations might be intentional.
    Delete the comment as it's speculative, asks for confirmation of intention, and the naming appears to be consistent and intentional in this test file.
33. engine/baml-lib/baml/tests/validation_files/expr/top_level_binding.baml:6
  • Draft comment:
    Minor style: Consider adding a newline at the end of the file to conform with common conventions and ensure compatibility with tools that require it.
  • 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 trailing newlines are a common convention, this is a test file in a tests directory. The comment is about a minor style issue that doesn't affect functionality. The rules state not to make purely informative comments or comments about obvious/unimportant things. Many IDEs and tools automatically handle this anyway.
    The trailing newline convention is widely accepted and could cause issues with some tools. Maybe this is important enough to mention?
    While true, this is a test file and the rules explicitly state not to make comments about obvious or unimportant things. If it's truly important, it should be handled by linting rules or IDE settings.
    The comment should be deleted as it's too minor and obvious of an issue, especially for a test file.
34. engine/baml-lib/baml/tests/validation_files/expr/unknown_name.baml:12
  • Draft comment:
    Typographical Inconsistency: In the function signature on line 12 (fn Go3(x:int) -> int {), there's a missing space between the parameter name and the colon. For consistency with the other functions, please change 'x:int' to 'x: int'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a test file specifically designed to test error handling. The inconsistent spacing appears intentional as part of the test cases. The file's purpose is to validate error handling for unknown names, not to test code style. Making style suggestions on test files that are intentionally written to test error cases could be counterproductive.
    Maybe consistent styling is important even in test files? The inconsistency could confuse readers.
    While consistency is generally good, this is clearly a test file focused on error handling. The spacing inconsistency doesn't affect the test's functionality and could even be intentionally varied to test the parser's robustness.
    Delete the comment. Style suggestions on test files specifically designed to test error handling are not valuable and could interfere with the test's purpose.
35. engine/baml-lib/jinja-runtime/src/output_format/types.rs:424
  • Draft comment:
    Typographical error: Please change 'own't' to 'won't' in the doc comment describing render_possibly_recursive_type.
  • 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.
36. engine/baml-lib/jsonish/src/deserializer/semantic_streaming.rs:57
  • Draft comment:
    Typo: In the comment at line 57, 'vial @stream.with_state' should be corrected to 'via @stream.with_state'.
  • 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/baml-lib/schema-ast/src/ast/expression.rs:252
  • Draft comment:
    Typo: The function name "as_identifer" is misspelled. Please rename it to "as_identifier" for 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.
38. engine/baml-lib/schema-ast/src/ast/top.rs:44
  • Draft comment:
    Typographical issue: The string literal for the Client variant in the get_type() method is "client". Consider revising it if this is unintended (e.g., to "client").
  • 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/baml-lib/schema-ast/src/ast/top.rs:63
  • Draft comment:
    Typographical/naming consistency concern: The method name as_value_exp might be a typo or inconsistency. Given that the value type is named ValueExprBlock, consider renaming this method to as_value_expr for 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.
40. engine/baml-lib/schema-ast/src/parser/datamodel.pest:246
  • Draft comment:
    Typo: The comment for the lambda function uses 'Anaonymous' instead of 'Anonymous'. Please correct the spelling.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
    While this is a real spelling error in the new code, it's just a typo in a comment. Comments are documentation and should be correct, but this is a very minor issue. The misspelling doesn't affect code functionality or clarity - the meaning is still clear.
    The rules don't explicitly address spelling errors in comments. Maybe incorrect documentation could lead to confusion for future developers?
    While documentation should be correct, this particular spelling error is extremely minor and the meaning is still completely clear. The rules emphasize focusing on important, actionable issues.
    Delete this comment. While it points out a real spelling error, it's too minor and doesn't affect code functionality or clarity.
41. engine/baml-lib/schema-ast/src/parser/datamodel.pest:197
  • Draft comment:
    Typo in comment: In the test name parse_comments_without_crasing_or_loosing_info, 'crasing' should be 'crashing' and 'loosing' should be 'losing'. Please update the test name 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.
42. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:274
  • Draft comment:
    Typo: 'therefor' should be corrected to 'therefore' in the comment explaining the backing string for the JinjaExpression.
  • 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.
43. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:234
  • Draft comment:
    Typo: 'panicks' should be corrected to 'panics' in the comment regarding the handling of escape sequences in the unescape_string function.
  • 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.
44. engine/baml-lib/schema-ast/src/parser/parse_schema.rs:29
  • Draft comment:
    Typo in the documentation comment: it says 'PSL' but since this file deals with BAML schemas, consider changing 'PSL' to 'BAML' for consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
45. engine/baml-lib/schema-ast/src/parser/parse_schema.rs:180
  • Draft comment:
    Typo in the panic message: 'This should never happend.' should be corrected to 'This should never happen.'
  • 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.
46. engine/baml-lib/schema-ast/src/parser/parse_type_expression_block.rs:11
  • Draft comment:
    The inline comment on the import at Line 11 refers to 'DatamodelParser', but the function imported is 'parse_type_expr'. Consider updating or removing this comment to avoid confusion.
  • 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.
47. engine/baml-runtime/src/internal/prompt_renderer/render_output_format.rs:211
  • Draft comment:
    There's a typo in the comment above the function relevant_data_models: it refers to the function as 'relevant_dat_models' instead of 'relevant_data_models'. Please correct 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.
48. engine/baml-runtime/src/lib.rs:692
  • Draft comment:
    Typographical error: please change 'requeset' to 'request' in the comment above the HTTPRequest construction.
  • 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.
49. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:68
  • Draft comment:
    Minor typo in the comment: "I dont think" should be "I don't think" for proper punctuation.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
50. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:198
  • Draft comment:
    Typographical error in the string literal: "BAML_PATH_SPLTTER" appears to be a typo. Consider changing it to "BAML_PATH_SPLITTER" (or the intended string).
  • 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.
51. engine/language_client_codegen/src/openapi.rs:660
  • Draft comment:
    There's a minor typographical issue in the comment above FieldType::Optional: 'something i saw suggested doing this' should have 'I' capitalized (i.e., 'something I saw suggested doing this'). This is just a small, cosmetic improvement.
  • 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.
52. engine/language_client_codegen/src/python/generate_types.rs:239
  • Draft comment:
    Typo: The comment says "Short-circuite". It should be corrected to "short-circuit".
  • 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.
53. engine/language_client_codegen/src/ruby/generate_types.rs:43
  • Draft comment:
    Typo in the comment: 'Partial' should likely be 'Partial'. Please fix the typo to make the comment clearer.
  • 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.
54. engine/language_client_codegen/src/typescript/mod.rs:307
  • Draft comment:
    Typographical error: The variable name 'typscript_client' appears multiple times (e.g., line 307). Consider renaming it to 'typescript_client' for consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
55. engine/language_client_codegen/src/typescript/mod.rs:202
  • Draft comment:
    Typographical error: The struct 'TypscriptLlmParser' seems to be misspelled. It likely should be 'TypescriptLlmParser' to match the rest of the file's naming conventions.
  • 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.
56. typescript/fiddle-frontend/next.config.mjs:28
  • Draft comment:
    Typo in comment: 'locak' should be 'local'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
57. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:80
  • Draft comment:
    The comment on line 80, "We don't use ASTContext.provider because we should the default value of the context", is a bit unclear. It seems to be missing a word (perhaps 'use' or 'want') to clarify why the default context value is preferred. Consider rewording it to something like "We don't use ASTContext.provider because we want to use the default value of the context."
  • 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.
58. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:107
  • Draft comment:
    On line 107, the comment reads "// todo: maybe we use a derived atom to reset it. But for now this useeffect works." It's recommended to capitalize 'useEffect' correctly to maintain consistency with React hook naming conventions.
  • 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.
59. typescript/vscode-ext/packages/language-server/src/server.ts:55
  • Draft comment:
    Typo: In the log message on line 55, change "cant load webcrypto" to "can't load webcrypto".
  • 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.
60. typescript/vscode-ext/packages/language-server/src/server.ts:305
  • Draft comment:
    Typo: On line 305 in the comment, change "mutliple" to "multiple".
  • 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.
61. typescript/vscode-ext/packages/language-server/src/server.ts:119
  • Draft comment:
    Typo/Clarity: In the comment on line 119, consider changing "why remove this prevents" to "why removing this prevents" to improve readability.
  • 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.
62. typescript/vscode-ext/packages/vscode/server/linux/baml-cli:1
  • Draft comment:
    The file is missing a newline at the end. Please add a newline for consistent formatting.
  • 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 symlink file, not a regular text file. Symlinks don't need newlines at the end - they are just pointers to other files/directories. The comment is applying a text file convention to a non-text file. This shows a lack of understanding of what the file actually is.
    Could there be some platform-specific or tool-specific reason why a newline would be needed in a symlink file?
    No, symlinks are handled at the filesystem level and their format is standardized. The content of a symlink is just the target path, and newlines are not relevant.
    Delete this comment as it incorrectly applies text file formatting rules to a symlink file.
63. typescript/vscode-ext/packages/vscode/src/extension.ts:69
  • Draft comment:
    Typographical error: The backgroundColor for statusBarItem is set as '##9333ea'. Please change it to '#9333ea'.
  • 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.
64. typescript/vscode-ext/packages/vscode/src/extension.ts:141
  • Draft comment:
    Typographical error: The debug log message reads "Pushing test errorrrr". Consider correcting the extra 'r' if this is unintentional.
  • 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.
65. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:193
  • Draft comment:
    Typo: In the JSDoc comment for _setWebviewMessageListener, 'recieved' should be corrected to 'received'.
  • 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_vpI2OkNXhcjEUydP


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 319d50c in 1 minute and 40 seconds

More details
  • Looked at 18 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. engine/baml-lib/baml/tests/validation_files/strings/unquoted_strings.baml:47
  • Draft comment:
    Removed redundant error message comments for invalid field definitions. Ensure test expectations are updated to reflect the new syntax changes.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to ensure that test expectations are updated, which violates the rule against asking the author to ensure things are tested. It doesn't provide a specific suggestion or ask for a specific test to be written, which would be acceptable.
2. engine/baml-lib/baml/tests/validation_files/strings/unquoted_strings.baml:47
  • Draft comment:
    Removed a block of commented-out expected errors; ensure the remaining test expectations fully align with the unified BAML value/type behavior and future LLM function composition semantics.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None

Workflow ID: wflow_b8nntggOSj1bl12j


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on 1a4735b in 2 minutes and 38 seconds

More details
  • Looked at 193 lines of code in 8 files
  • Skipped 0 files when reviewing.
  • Skipped posting 19 drafted comments based on config settings.
1. engine/baml-lib/validate/validation_pipeline/validations/expr_fns.rs:72
  • Draft comment:
    Consider removing the unused shadowing of scope (copying the HashSet) if it’s no longer needed in validate_stmt. Ensure tests cover this change.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. engine/baml-lib/parser-database/src/walkers/expr_fn.rs:23
  • Draft comment:
    Updating the accessor to return 'stmt.body' instead of 'stmt.body.expr' seems consistent with the AST changes. Confirm that all clients of TopLevelAssignmentWalker are updated accordingly.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to confirm that all clients of a specific function are updated to use a new accessor. This falls under the category of asking the author to double-check or ensure something, which is against the rules. Therefore, this comment should be removed.
3. engine/baml-lib/schema-ast/src/parser/parse_expr.rs:82
  • Draft comment:
    The added debug eprintln! calls before and after parse_expr_block aid in tracing parsing issues. Remember to remove or gate these under a debug flag if production performance is a concern.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, reminding the author to remove or gate debug statements. It doesn't suggest a specific code change or ask for confirmation of intent. It violates the rule against making purely informative comments.
4. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:43
  • Draft comment:
    Mapping the result of parse_expr_block to Expression::ExprBlock with the overall span is a good design; ensure that the span handling matches the intended semantics (inner vs outer).
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that the span handling matches the intended semantics, which is a form of asking for confirmation or verification. This violates the rule against asking the author to confirm their intention or ensure behavior is intended.
5. engine/baml-lib/schema-ast/src/parser/parse_schema.rs:444
  • Draft comment:
    The updated test now inspects the Expression::ExprBlock structure inside a top-level assignment, which is good. Ensure that any previous tests expecting a different shape (with a separate 'expr' field) are updated.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the author to ensure that previous tests are updated to reflect changes in the code structure. This falls under the category of asking the author to ensure something is tested or verified, which is against the rules.
6. engine/baml-runtime/src/eval_expr.rs:148
  • Draft comment:
    In beta_reduce, the lambda application is handled using a fold with subst2. Ensure that substitution avoids variable capture in more complex cases. Also, the use of Box::pin for async calls is acceptable but consider verifying if the recursion limit (MAX_STEPS) needs adjustment.
  • 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. engine/baml-runtime/src/eval_expr.rs:175
  • Draft comment:
    The branch handling LLMFunction applications invokes the runtime call_function. Confirm that unwraps on tx.unbounded_send and subsequent unwrapping of function results are safe in production; consider adding error handling or logging for failed sends.
  • 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/baml-runtime/src/eval_expr.rs:254
  • Draft comment:
    The loop in eval_to_value iterates up to MAX_STEPS. Consider logging the number of steps if max steps is reached to aid debugging infinite loops.
  • 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. engine/baml-runtime/src/eval_expr.rs:56
  • Draft comment:
    In the Lambda branch of subst2, there's no check for bound variable shadowing which may cause capture issues.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. engine/baml-runtime/src/eval_expr.rs:158
  • Draft comment:
    Unwrap() in the fold during substitution may panic if substitution fails; consider propagating the error.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. engine/baml-runtime/src/eval_expr.rs:177
  • Draft comment:
    Double unwrap on eval_to_value results is unsafe; errors should be propagated instead.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. engine/baml-runtime/src/eval_expr.rs:193
  • Draft comment:
    tx.unbounded_send is unwrapped without error handling; consider handling the potential send errors.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. engine/baml-runtime/src/eval_expr.rs:317
  • Draft comment:
    The temporary_same_state check may be too coarse, risking infinite loops if no progress is detected.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
14. engine/baml-runtime/src/eval_expr.rs:345
  • Draft comment:
    Excessive use of Box::pin in beta_reduce calls could affect performance; consider refactoring to reduce boxing.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
15. engine/baml-runtime/src/eval_expr.rs:469
  • Draft comment:
    Test code contains an 'assert!(false)' placeholder; replace with proper test assertions before merging.
  • 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.
16. engine/baml-lib/schema-ast/src/ast/expression.rs:252
  • Draft comment:
    Typographical error: The function name 'as_identifer' is misspelled. Consider renaming it to 'as_identifier' to maintain consistency and 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.
17. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:239
  • Draft comment:
    Typo: The comment uses 'panicks' which should be corrected to 'panics'.
  • 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.
18. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:279
  • Draft comment:
    Typo: The comment contains 'therefor' which should be 'therefore'.
  • 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.
19. engine/baml-lib/schema-ast/src/parser/parse_schema.rs:180
  • Draft comment:
    Typographical error: The word 'happend' in the panic message should be corrected to 'happened'.
  • 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_L6GsTl0dfDU7ba3S


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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on 3e1dbfa in 5 minutes and 55 seconds

More details
  • Looked at 6610 lines of code in 85 files
  • Skipped 1 files when reviewing.
  • Skipped posting 62 drafted comments based on config settings.
1. engine/language_client_codegen/src/go/generate_types.rs:23
  • Draft comment:
    Consider using consistent formatting when converting type names (e.g. use filters::type_name_without_pointer) to avoid duplication in patterns like handling class and union 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.
2. engine/language_client_codegen/src/openapi.rs:90
  • Draft comment:
    The schema definitions in the OpenAPI file use fixed hard-coded string literals. Verify these reflect current API requirements.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
3. engine/language_client_codegen/src/python/generate_types.rs:60
  • Draft comment:
    Consider checking or documenting the behavior for FieldType::Arrow; usage of 'todo!' ensures these code paths are not currently supported. Ensure this is acceptable in a foundational phase.
  • 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. engine/language_client_codegen/src/python/mod.rs:21
  • Draft comment:
    The conversion implementations for Python clients (async, sync, parser, etc.) seem repetitive; consider reducing duplication by factoring common conversion logic.
  • 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. engine/language_client_codegen/src/ruby/generate_types.rs:168
  • Draft comment:
    Use clear naming for functions generating default values for fields. The functions add_default_value and has_none_default are clear, but comments would help clarify edge cases—especially for Optional vs. non-Optional.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
6. engine/typescript/fiddle-frontend/next.config.mjs:21
  • Draft comment:
    Ensure that adding WebAssembly resource rule (lines 22-25) is tested with different asset types. Also check whether legacy caching issues have been resolved.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
7. engine/nextjs-plugin/src/index.ts:43
  • Draft comment:
    The function getNextJsVersion falls back gracefully; consider logging a warning if both attempts fail.
  • Reason this comment was not posted:
    Confidence changes required: 30% <= threshold 50%
    None
8. engine/playground-common/src/baml_wasm_web/EventListener.tsx:115
  • Draft comment:
    Consider debouncing the scroll event listener to avoid excessive calls; current implementation appears fine but check performance on large documents.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
9. engine/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:86
  • Draft comment:
    The test runner uses detailed logging and proper error handling. Ensure that clear warnings and clearing highlights are applied in both success and error paths.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
10. engine/vscode-ext/packages/vscode/src/extension.ts:23
  • Draft comment:
    The proxy middleware uses a custom pathRewrite function; please verify the regex used for file extensions and the handling of trailing slashes works as expected in all edge cases.
  • 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/vscode-ext/packages/vscode/src/extension.ts:445
  • Draft comment:
    Ensure that cleanup and disposal in the deactivate() function properly release resources (eg, StatusBarPanel, server closure) to avoid memory leaks.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
12. engine/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:240
  • Draft comment:
    Consider handling possible rejected promises or errors in the message listener callback (_setWebviewMessageListener). The branching logic is complex; verify all rails are covered.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
13. engine/typescript/nextjs-plugin/src/index.ts:40
  • Draft comment:
    Overall, the configuration merging appears careful; consider adding inline comments to clarify the condition when adding serverExternalPackages in non-Turbo mode.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
14. engine/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:370
  • Draft comment:
    Using a loop with yield_next() to process test results is fine but verify that no dangling async tasks or open channels remain after tests are completed.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
15. typescript/fiddle-frontend/next.config.mjs:69
  • Draft comment:
    The background color hex string contains a double ‘#’ ("##9333ea"). This is likely a typo and may cause unexpected styling. Consider changing it to "#9333ea".
  • 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.
16. typescript/vscode-ext/packages/vscode/src/extension.ts:69
  • Draft comment:
    The status bar background color is set to '##9333ea' (double hash). This may be a typo; using a single '#' ("#9333ea") is standard.
  • 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.
17. engine/language_client_codegen/src/ruby/field_type.rs:60
  • Draft comment:
    The match arm for FieldType::Arrow uses 'todo!()'. If Arrow types are not intended to be used in generated code, please document this clearly or provide a graceful error message to handle unexpected cases.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
18. engine/language_client_codegen/src/python/generate_types.rs:255
  • Draft comment:
    Arrow types are currently marked with a 'todo!' in the partial type reference function. Ensure that Arrow types never occur in production or handle them gracefully with an error message.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
19. engine/language_client_codegen/src/typescript/mod.rs:678
  • Draft comment:
    The match arm for FieldType::Arrow uses a 'todo!' macro. Make sure these cases are either proven unreachable or implement a safe fallback to avoid runtime panics if an Arrow type is encountered.
  • Reason this comment was not posted:
    Marked as duplicate.
20. engine/language_client_codegen/src/ruby/generate_types.rs:465
  • Draft comment:
    There is a TODO comment for handling fields with both @stream.done and @stream.not_null. Please address this TODO and ensure the proper default value is generated.
  • 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.
21. typescript/vscode-ext/packages/vscode/src/extension.ts:227
  • Draft comment:
    When rewriting the proxy request, consider more strictly validating or sanitizing the 'baml-original-url' header to prevent any potential misuse or security issues.
  • 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.
22. engine/Cargo.lock:1
  • Draft comment:
    The Cargo.lock file is automatically generated by Cargo and contains a complete list of all the packages, versions, sources, and checksums required by your project. The complete new contents (after the diff is applied) are the full listing from the top of the file (e.g., starting at line 1) all the way through to the end (line 6551 in your case). In other words, no manual modifications are needed or expected here—this is managed by Cargo itself. If you are updating dependencies, please let Cargo regenerate this file, and commit the resulting lock file. Otherwise, there's nothing to manually review or change in this file.
  • 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.
23. engine/baml-lib/baml-core/src/ir/ir_helpers/mod.rs:199
  • Draft comment:
    Consider revising the comment to improve clarity. For example, instead of saying 'and no in the return type check', change it to 'and not in the return type check' to clarify the intended meaning.
  • 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 identifies a real typo, it's a very minor issue in a code comment that doesn't affect functionality. The rules state not to make purely informative comments or comments that are obvious/unimportant. Fixing a small typo in a comment falls into this category - it's not a code change that meaningfully improves the codebase.
    The typo does make the comment slightly harder to understand at first glance, so there is some small value in fixing it. The comment is also very specific and actionable.
    While specific and actionable, the benefit is extremely minor. Code comments are meant to be helpful guides, not perfect prose. This kind of nitpick creates noise in the PR review.
    The comment should be deleted as it points out a trivial typo in a comment that doesn't meaningfully impact code understanding.
24. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:359
  • Draft comment:
    Minor typographical issue: consider changing "A json value may not be coerced into a function type" to "A JSON value may not be coerced into a function type" for consistency with the common acronym styling.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While technically correct that JSON is typically capitalized, this is a very minor style issue in an error message. The meaning is completely clear either way. The comment doesn't point out any actual bugs or important code quality issues. It's the kind of nitpicky style comment that creates noise without adding much value.
    The capitalization of JSON is a widely accepted standard, and consistency in error messages can be important for professionalism. Maybe this is worth fixing for polish.
    While consistent capitalization is good, this is an extremely minor issue in an error message that will rarely be seen. The comment creates more overhead than the value of the fix.
    Delete this comment. It's too minor of a style issue to be worth the overhead of a code change and review.
25. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:364
  • Draft comment:
    Typographical error: "FieldType::Constrainted" should likely be corrected to "FieldType::Constrained".
  • Reason this comment was not posted:
    Comment was on unchanged code.
26. engine/baml-lib/baml-core/src/ir/repr.rs:1957
  • Draft comment:
    Typo detected in the comment for the Prompt enum: 'stirng' should be corrected to 'string'.
  • 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.
27. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/expr_typecheck.rs:93
  • Draft comment:
    The error message reads "A Type mismatch". The leading 'A' seems like an accidental prefix or debug leftover. Consider removing it 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 = 0% vs. threshold = 50%
    The letter prefixes appear to be intentionally used throughout the code for debugging/tracing purposes. The "A", "B", etc. prefixes help identify which code path generated the error. Removing them could make debugging harder. The comment assumes these are accidental when they appear to be deliberate.
    I could be wrong about the prefixes being intentional - they could truly be accidental. The inconsistent use (some messages have prefixes, others don't) could suggest they are leftover debug code.
    Even if the prefixes are accidental, removing them would be a purely cosmetic change that doesn't affect functionality. The error messages are still clear and understandable with the prefixes.
    The comment should be deleted. The letter prefixes appear to be intentional debug aids, and even if they aren't, removing them would be a trivial cosmetic change that doesn't improve code quality.
28. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/expr_typecheck.rs:115
  • Draft comment:
    The error message in the lambda branch is prefixed with "B Type mismatch in lambda:". If this prefix is not intentional (for example, if it was used for debugging), consider removing the 'B' to improve clarity and consistency with other messages.
  • 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 lettered prefixes appear to be intentional for debugging purposes. There's "A Type mismatch" earlier in the code, and in the commented-out section there are "C", "D", "E", and "F" prefixes too. This suggests it's a deliberate system for tracking where type mismatches occur. Removing just the "B" would make the system inconsistent.
    I could be wrong about the debugging system - the letters could be leftover artifacts that should be cleaned up. The commented-out code with other lettered prefixes might be obsolete.
    Even if the letters are artifacts, removing just one letter would make the error messages inconsistent. A broader cleanup of all lettered prefixes should be done as a separate change.
    The comment should be rejected since the "B" prefix appears to be part of a wider system of lettered error messages. Removing just one letter would reduce consistency.
29. engine/baml-lib/baml-types/src/baml_value.rs:720
  • Draft comment:
    Typo: The word 'classs' should be corrected to 'classes' in the comment (line ~720).
  • 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.
30. engine/baml-lib/baml-types/src/field_type/mod.rs:246
  • Draft comment:
    There's a minor typographical error in the TODO comment: 'its pretty hard to get type aliases here'. It would be more grammatically correct to say "it's pretty hard to get type aliases here".
  • 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.
31. engine/baml-lib/baml-types/src/value_expr.rs:15
  • Draft comment:
    Typo: In the comment "Repred as a string, but guaranteed to be a number.", 'Repred' should be corrected to 'Represented' for 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.
32. engine/baml-lib/baml/tests/validation_files/expr/mixed_pipeline.baml:16
  • Draft comment:
    The file does not end with a newline. It's generally a good practice to include a trailing newline in files to avoid issues with file concatenation and tooling. Please add a newline at the end of this file.
  • 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 having a trailing newline is indeed a common convention, this seems like a very minor issue that would typically be handled automatically by IDE settings or enforced by linters. It's not a logic issue and doesn't affect functionality. The file is also a test file, which makes style issues even less critical.
    The missing newline could potentially cause issues with some Unix tools and version control systems. It's a real issue that some teams care about.
    While true, this is exactly the kind of minor issue that should be handled by automated tooling rather than manual code review comments. It's too trivial for a human review comment.
    Delete this comment as it's too minor of an issue for a code review comment and would be better handled by automated tooling.
33. engine/baml-lib/baml/tests/validation_files/expr/unknown_name.baml:12
  • Draft comment:
    Typographical Issue: In the function definition for Go3, the parameter is defined as 'x:int' (line 12) instead of using a space as in other functions ('x: int'). Consider adding a space for consistency.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a test file specifically created to validate error handling, as evidenced by the error messages at the bottom. The spacing inconsistency is real but trivial, especially in test code. The file's purpose is to test the handling of unknown variables and functions, not to demonstrate proper formatting.
    The inconsistency does exist, so technically the comment is correct. And consistency in code style can be important for maintainability.
    While code consistency matters in production code, this is clearly a test file specifically created to test error conditions. The spacing issue is too minor to matter in test code and distracts from the file's actual purpose.
    Delete the comment. Style nitpicks in test files, especially ones designed to test error conditions, add noise without value.
34. engine/baml-lib/jinja-runtime/src/output_format/types.rs:422
  • Draft comment:
    Typo detected in the documentation: "you own't get any rendering!" should be "you won't get any rendering!".
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
35. engine/baml-lib/jsonish/src/deserializer/semantic_streaming.rs:56
  • Draft comment:
    Typographical error: The documentation comment incorrectly uses 'vial' instead of 'via'. Please update it to 'via' for 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.
36. engine/baml-lib/parser-database/src/walkers/mod.rs:34
  • Draft comment:
    Typo: In the comment describing the generic walker, 'intantiated' should be corrected to 'instantiated'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
37. engine/baml-lib/schema-ast/src/ast.rs:121
  • Draft comment:
    Typographical error: The error message in the Index implementation for TypeAliasId is 'expected type expression', but it should probably be 'expected type alias assignment' to accurately reflect the context. Please update the message.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
38. engine/baml-lib/schema-ast/src/ast/expr.rs:2
  • Draft comment:
    Typographical consistency: The comment on line 2 says 'non-llm functions'. Consider using 'non-LLM functions' instead for consistency with common uppercase usage for the LLM acronym.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While technically correct that LLM is commonly written in uppercase, this is a minor stylistic issue in a doc comment. The meaning is perfectly clear either way. The rules state not to make purely informative comments or comments about unimportant issues.
    The inconsistent casing could potentially cause confusion for new developers, and maintaining consistent terminology across the codebase is valuable.
    While consistency is good, this is too minor of an issue to warrant a PR comment. The lowercase version is still completely understandable and doesn't impact code functionality.
    Delete the comment as it violates the rule about not making comments that are obvious or unimportant. This is a purely stylistic suggestion that doesn't meaningfully impact code quality.
39. engine/baml-lib/schema-ast/src/ast/top.rs:44
  • Draft comment:
    The string literal for Top::Client in the get_type() function reads as "client". This seems unusual and might be a typographical error. Please double-check if the additional '' is 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.
40. engine/baml-lib/schema-ast/src/parser/datamodel.pest:246
  • Draft comment:
    Typo: The comment above the lambda rule says 'Anaonymous function.' Please correct it to 'Anonymous 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 this is technically correct and about a changed part of the code, comments about typos in comments are generally very low value. They don't affect functionality and can be fixed during normal code maintenance. However, since it's a clear error and very easy to fix, it might be worth keeping.
    Am I being too lenient? The rules say to only keep comments that require code changes, and this is just a documentation fix.
    The rules specifically say not to make purely informative comments, and this is just pointing out a spelling error in a comment. While it's correct, it's not the type of feedback we want to keep.
    Delete this comment as it only points out a spelling error in documentation and doesn't require any functional code changes.
41. engine/baml-lib/schema-ast/src/parser/datamodel.pest:198
  • Draft comment:
    Typo: In the comment referencing the test parse_comments_without_crasing_or_loosing_info, please correct 'crasing' to 'crashing' and 'loosing' to 'losing'.
  • 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.
42. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:239
  • Draft comment:
    Typographical error: 'panicks' should be corrected to 'panics'.
  • 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.
43. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:279
  • Draft comment:
    Typographical error in doc comment: 'therefor' should be 'therefore' and 'should be contain' should be 'should contain'.
  • 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.
44. engine/baml-lib/schema-ast/src/parser/parse_named_args_list.rs:70
  • Draft comment:
    Typo: In the unreachable! error message (line 70), the text 'parse_named_args_list:' does not match the function name 'parse_named_argument_list'. Consider correcting it for consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
45. engine/baml-lib/schema-ast/src/parser/parse_schema.rs:180
  • Draft comment:
    Typo: In the panic message on line 180, "This should never happend." should be corrected to "This should never happen."
  • 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.
46. engine/baml-runtime/src/lib.rs:697
  • Draft comment:
    Typo: In the comment above the build_request function, 'requeset body' should be corrected to 'request body'.
  • 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.
47. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:67
  • Draft comment:
    Minor typographical issue: The comment on line 67 reads "I dont think we need this line anymore...". Consider changing it to "I don't think we need this line anymore...".
  • 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.
48. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:198
  • Draft comment:
    Typographical error: In the files() method, the string literal "BAML_PATH_SPLTTER" appears to have a typo. Consider changing it to "BAML_PATH_SPLITTER" (or the intended correct term).
  • 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.
49. engine/language_client_codegen/src/go/mod.rs:169
  • Draft comment:
    Consider using the full word 'functions' instead of the abbreviation 'fns' in the panic message for clarity (e.g., 'Generation is not supported with expression functions').
  • 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 would make the error message slightly clearer, this is a panic message that would only be seen by developers in an error case. The current message is not ambiguous - "fns" is a very common abbreviation for "functions" in programming contexts. The change would be purely cosmetic and doesn't affect functionality.
    The comment does point out a way to make the code more readable. Error messages should generally be clear and descriptive.
    However, this is an internal panic message in a code generator, not a user-facing error. The current message is clear enough for its intended audience of developers.
    The comment should be removed as it suggests a purely cosmetic change to an internal error message that is already sufficiently clear for its purpose.
50. engine/language_client_codegen/src/go/mod.rs:224
  • Draft comment:
    Consider using the full word 'functions' instead of the abbreviation 'fns' in the panic message for clarity (e.g., 'Generation is not supported with expression functions').
  • 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 suggestion would make the message slightly more clear, this is an internal error message that will only be seen by developers if they try to use an unsupported feature. The abbreviation "fns" is a common and well-understood shorthand in programming contexts. The meaning is clear enough, and this is a very minor stylistic suggestion that doesn't impact functionality.
    The message could be slightly clearer with the full word. Some developers who are not native English speakers might find the abbreviation confusing.
    The abbreviation "fns" is extremely common in programming contexts, especially in Rust code. The message is clear enough for its purpose as an internal error.
    This comment suggests a very minor stylistic change that doesn't meaningfully improve code quality or clarity. The existing message is clear enough.
51. engine/language_client_codegen/src/python/generate_types.rs:239
  • Draft comment:
    Typo: 'Short-circuite' should be 'Short-circuit' in the comment above the add_default_value function.
  • 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.
52. engine/language_client_codegen/src/ruby/generate_types.rs:43
  • Draft comment:
    Typo: The comment on line 43 reads "Partial". It looks like there's an extra 'j' and should probably be "Partial".
  • 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.
53. engine/language_client_codegen/src/typescript/mod.rs:203
  • Draft comment:
    Typo: The struct name 'TypscriptLlmParser' appears to be a misspelling. It should likely be 'TypescriptLlmParser' to maintain consistency with other identifiers in this file.
  • 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.
54. typescript/fiddle-frontend/next.config.mjs:28
  • Draft comment:
    Typographical error: In the comment on line 28, 'locak' should be corrected to 'local'.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is pointing out a typographical error in a comment within the code. While it is not asking the author to confirm their intention or to test something, it is also not providing a code suggestion or asking for a test to be written. It is purely informative, as it is simply pointing out a typo in a comment, which does not affect the functionality of the code.
55. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:80
  • Draft comment:
    The comment on line 80 reads: "We don't use ASTContext.provider because we should the default value of the context". It seems a word is missing after 'should' (perhaps "use" or "have"). Please revise the comment for 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.
56. typescript/vscode-ext/packages/language-server/src/server.ts:55
  • Draft comment:
    Typo: Consider changing "cant load webcrypto" to "can't load webcrypto" for proper grammar.
  • 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.
57. typescript/vscode-ext/packages/language-server/src/server.ts:305
  • Draft comment:
    Typo: In the comment, 'mutliple' should be corrected to 'multiple'.
  • 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.
58. typescript/vscode-ext/packages/vscode/server/darwin/baml-cli:1
  • Draft comment:
    The file does not end with a newline character. It is generally a good practice to have a newline at the end of files to adhere to POSIX standards and avoid potential issues with certain tools.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a symlink file, not a regular text file. The format of symlink files is specific and doesn't necessarily follow the same conventions as text files. The newline character suggestion, while generally good practice for text files, is not relevant or important for symlink files. The symlink will work perfectly fine without a newline.
    Maybe some tools or systems could have issues with symlinks without newlines? Maybe there's a platform-specific consideration I'm not aware of?
    Symlinks are handled at the filesystem level and their functionality is not affected by newline characters. This is a well-established behavior across platforms.
    The comment should be deleted as it's not relevant for symlink files and doesn't impact functionality.
59. typescript/vscode-ext/packages/vscode/src/extension.ts:69
  • Draft comment:
    Typo: The background color for the status bar is set as '##9333ea'. It looks like there is an extra '#' character. It should probably be '#9333ea'.
  • 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.
60. typescript/vscode-ext/packages/vscode/src/extension.ts:141
  • Draft comment:
    Typo: The log message 'Pushing test errorrrr' contains an extra 'r'. Consider changing it to 'Pushing test error' or updating it appropriately.
  • 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.
61. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:195
  • Draft comment:
    Typographical error: The word "recieved" should be corrected to "received" in the comment describing the method.
  • 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.
62. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:270
  • Draft comment:
    The log message "WEBPANELVIEW set_flashing_regions" appears inconsistent with the rest of the file (which uses "webview"). Consider changing it to "WEBVIEW set_flashing_regions" or otherwise ensuring consistency in naming.
  • 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 inconsistency, this is a very minor issue about console.log formatting. Console.log messages are typically for debugging and don't affect functionality. The comment doesn't point out a significant code quality issue that needs to be fixed.
    The inconsistency could make logs harder to grep/search if there's a mix of different prefixes. Also, consistency in code style can be important for maintainability.
    While consistency is good, this is an extremely minor issue about debug log formatting that doesn't impact functionality or maintainability in any meaningful way. The benefit of fixing this doesn't justify the overhead of a code change.
    Delete this comment as it points out an extremely minor inconsistency in debug logging that doesn't warrant a code change.

Workflow ID: wflow_Ulo1VOTxTGElQGDk


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

@imalsogreg imalsogreg added this pull request to the merge queue Apr 10, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on 0b1fa01 in 4 minutes and 11 seconds

More details
  • Looked at 6660 lines of code in 87 files
  • Skipped 1 files when reviewing.
  • Skipped posting 78 drafted comments based on config settings.
1. engine/language_client_codegen/src/go/generate_types.rs:23
  • Draft comment:
    Good handling of different FieldType variants. The use of todo! for Arrow types properly signals that they are not supported. Consider adding a comment reminding future developers why Arrow types are unsupported.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. engine/language_client_codegen/src/openapi.rs:12
  • Draft comment:
    The OpenAPI schema generation code is clear and uses serde_json macros effectively. No obvious issues found.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. engine/language_client_codegen/src/python/generate_types.rs:16
  • Draft comment:
    Askama templates are correctly used for generating Python types. Arrow types use todo! which is acceptable for staging; consider documenting the planned support for Arrow types.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. engine/language_client_codegen/src/ruby/field_type.rs:60
  • Draft comment:
    The Ruby conversion for FieldType is handled as expected. Arrow types invoke a todo! which is a clear signal that no generation is supported. This is consistent with our strategy.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. engine/language_client_codegen/src/ruby/generate_types.rs:19
  • Draft comment:
    The code for generating Ruby types looks clean and consistent. The conversion of enums and classes is straightforward.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
6. engine/language_client_codegen/src/typescript/mod.rs:21
  • Draft comment:
    The TypeScript generation code is well‐structured with separate templates for React server actions, hooks, and clients. The use of a framework switch is a good practice.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
7. flake.nix:3
  • Draft comment:
    The flake.nix file specifies the correct NixOS branch and includes appropriate build time configuration. Everything appears in order.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
8. integ-tests/typescript/package.json:7
  • Draft comment:
    The package.json file for integration tests increases the testTimeout to 60000 ms, which should help avoid flaky tests under load.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
9. typescript/fiddle-frontend/next.config.mjs:22
  • Draft comment:
    The Next.js configuration properly includes rules for loading WebAssembly files; consider ensuring that the rule does not conflict with other asset handling rules.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
10. typescript/nextjs-plugin/src/index.ts:50
  • Draft comment:
    The Next.js plugin correctly checks the Next.js version and supports conditional logic based on the version. Ensure that the fallback major version (14) remains in sync with documented behavior.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
11. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:25
  • Draft comment:
    The EventListener component sets up debounced atom callbacks and listens to messages via window.addEventListener. Consider verifying that all dependencies are correctly passed to useEffect to avoid stale closures.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
12. typescript/nextjs-plugin/src/index.ts:136
  • Draft comment:
    The plugin’s webpack extension logic adds the required loaders for .node and .wasm files. Looks correct overall; consider reviewing the watchOptions ignoring rule in line 33 if any edge cases arise.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
13. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:30
  • Draft comment:
    The test-runner implements batching and state updates with debouncing, which is good for performance. Ensure that the error handling within runTest handles missing test cases robustly.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
14. typescript/vscode-ext/packages/vscode/src/extension.ts:20
  • Draft comment:
    The extension registers watchers for diagnostics, commands and proxies. The use of CORS and http-proxy-middleware appears sound. Consider removing debug logging once stable.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
15. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:55
  • Draft comment:
    The webview panel host code sets up HTML content and message listeners correctly. The function _getWebviewContent cleanly injects resource URIs. Consider abstracting frequent message commands into constants.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
16. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:370
  • Draft comment:
    In the parallel test runner, error handling and state updates are implemented in a similar manner. Consider refactoring common state update code to a shared helper.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
17. typescript/vscode-ext/packages/vscode/src/extension.ts:435
  • Draft comment:
    The proxy middleware setup for GET requests handles image files by rewriting paths. Ensure that the regex and header handling cover edge cases on Windows.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
18. typescript/vscode-ext/packages/vscode/src/extension.ts:430
  • Draft comment:
    The command 'SET_PROXY_SETTINGS' updates workspace config; ensure the updated configuration correctly propagates to dependent modules.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
19. typescript/nextjs-plugin/src/index.ts:121
  • Draft comment:
    The function wrapping webpack config to support both production and development appears robust. Please ensure that in production mode, source maps and production browser source maps are validated.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
20. engine/language_client_codegen/src/go/generate_types.rs:29
  • Draft comment:
    Using unwrap() on underlying_type (and similarly in render_value_coercion) assumes the field type is guaranteed to have a value. Consider replacing unwrap() with expect() to include context in case of an invariant violation.
  • 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.
21. engine/language_client_codegen/src/go/generate_types.rs:56
  • Draft comment:
    Ensure that any TODO remaining (e.g. Arrow types not supported) is tracked in issue trackers to avoid accidentally shipping incomplete functionality.
  • 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.
22. engine/language_client_codegen/src/ruby/generate_types.rs:145
  • Draft comment:
    The default value logic in add_default_value relies on substring matching for 'Optional['. This approach might be brittle; consider refactoring to use a type‐based check.
  • 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.
23. engine/language_client_codegen/src/python/generate_types.rs:180
  • Draft comment:
    The TODO for FieldType::Arrow remains; ensure that this unimplemented case is tracked appropriately.
  • 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.
24. engine/language_client_codegen/src/typescript/mod.rs:595
  • Draft comment:
    The handling of union types and the optional wrapping logic in to_partial_type_ref is complex. Consider adding unit tests to cover various union edge cases.
  • Reason this comment was not posted:
    Comment was on unchanged code.
25. flake.nix:1
  • Draft comment:
    The flake file now targets nixos-24.05. Verify that all platform-specific dependencies (e.g. LLVM version and lib paths) work as expected.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to verify platform-specific dependencies, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue.
26. integ-tests/typescript/package.json:11
  • Draft comment:
    TestTimeout is increased to 60000ms. Confirm that this longer timeout is appropriate for integration tests and will not mask performance issues.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
27. typescript/fiddle-frontend/next.config.mjs:21
  • Draft comment:
    The webpack configuration adds WebAssembly and source map settings. Consider removing commented out code if it is no longer needed to keep the file clean.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
28. typescript/nextjs-plugin/src/index.ts:6
  • Draft comment:
    The getNextJsVersion function falls back to the plugin's dependencies if resolution from the project fails. This is acceptable, but consider caching the version to avoid redundant file system accesses on repeated calls.
  • 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.
29. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:90
  • Draft comment:
    The useEffect adding the window 'message' event listener correctly returns a cleanup function. The overall code looks clean – ensure that dependency arrays are up‑to‑date to prevent stale closures.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
30. typescript/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/test-panel/test-runner.ts:80
  • Draft comment:
    The test runner hook distinguishes between sequential and parallel test execution. Consider reviewing the error handling and state updates for potential race conditions with asynchronous test execution.
  • Reason this comment was not posted:
    Comment was on unchanged code.
31. typescript/vscode-ext/packages/vscode/src/extension.ts:21
  • Draft comment:
    The extension.ts file sets up status bar items and the server correctly. Ensure that disposing of resources (e.g. statusBarItem, diagnosticsCollection) is robust in case of errors on deactivation.
  • 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.
32. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:166
  • Draft comment:
    The _getWebviewContent function builds HTML for the panel. The nonce is generated and used correctly. Consider extracting the HTML into a separate template if it becomes more complex.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
33. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:210
  • Draft comment:
    The _setWebviewMessageListener function branches on the message.command property. Ensure that each case properly validates incoming data to mitigate potential security issues.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
34. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:390
  • Draft comment:
    In the GET_WEBVIEW_URI and related message handling, ensure proper error handling when reading files (e.g., using encodeBuffer and catching errors) to prevent the extension from failing unexpectedly.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
35. integ-tests/typescript/package.json:11
  • Draft comment:
    The test script timeout has been increased to 60000ms. Confirm that this is acceptable for integration tests and that tests do not time out unnecessarily.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
36. engine/baml-lib/baml-core/src/ir/ir_helpers/mod.rs:200
  • Draft comment:
    In the comment explaining function arrow types, the phrase 'and no in the return type check' seems like a typo. It would be clearer as 'but not in the return type check.'
  • 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 is about changed code and identifies a real typo, comments that only fix typos in code comments (rather than actual code) are generally not important enough to keep. The typo doesn't affect code functionality and the meaning is still reasonably clear from context. The comment is more noise than signal.
    The typo could make the code comment slightly harder to understand for future readers, and good documentation is important for maintainability.
    While documentation is important, this particular typo is minor enough that most readers would understand the intent. The benefit of fixing it doesn't outweigh the noise added to the PR review.
    Delete the comment as it only addresses a minor documentation typo that doesn't significantly impact code understanding.
37. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:359
  • Draft comment:
    Consider changing 'A json value may not be coerced into a function type' to 'A JSON value may not be coerced into a function type' 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 technically correct that JSON is conventionally capitalized, this is a very minor stylistic change that doesn't affect functionality. The message is still clear and understandable either way. This seems like the kind of nitpicky comment that adds noise without much value.
    The capitalization of JSON is a widely accepted convention, and consistency in error messages can be important for professionalism.
    While technically correct, this level of nitpicking about capitalization in error messages doesn't meet the bar of being an important enough issue to flag in a PR review.
    Delete this comment as it's too minor and doesn't indicate a real problem that needs fixing.
38. engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs:364
  • Draft comment:
    There's a typographical error in the unreachable message: 'The return value of distribute_constraints can never be FieldType::Constrainted'. Consider correcting 'Constrainted' to 'Constrained'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
39. engine/baml-lib/baml-core/src/ir/repr.rs:1957
  • Draft comment:
    Typo: In the comment for the Prompt enum, 'stirng' should be corrected to 'string'.
  • 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.
40. engine/baml-lib/baml-types/src/baml_value.rs:720
  • Draft comment:
    There's a typographical error in the comment at line 721 where 'classs' should probably be 'classes' or 'class's'. Please correct 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.
41. engine/baml-lib/baml-types/src/field_type/mod.rs:246
  • Draft comment:
    Typographical error: In the comment "// TODO: its pretty hard to get type aliases here", 'its' should be "it's" for proper contraction of "it is".
  • 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.
42. engine/baml-lib/baml-types/src/value_expr.rs:15
  • Draft comment:
    Typo: The comment at line 15 uses "Repred"; please change it to "Represented".
  • 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.
43. engine/baml-lib/baml/tests/validation_files/expr/expr_full.baml:71
  • Draft comment:
    There's a minor typographical issue: the file does not end with a newline. Please add a newline at the end to conform to common style conventions.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While having a trailing newline is a common convention, this kind of formatting issue would typically be handled by automated tooling like linters or formatters. It's a very minor issue that doesn't affect functionality. The rules state not to make comments about obvious or unimportant things.
    Missing trailing newlines can cause issues with some tools and git diffs. It's a real issue that should be fixed.
    While true, this is exactly the kind of minor formatting issue that should be handled by automated tooling rather than manual code review comments. If it's important enough to enforce, it should be part of the CI/build process.
    Delete the comment. This kind of minor formatting issue should be handled by automated tooling rather than manual review comments.
44. engine/baml-lib/baml/tests/validation_files/expr/mixed_pipeline.baml:10
  • Draft comment:
    Minor typographical note: The generic type parameter in the client declaration is written in lowercase as 'llm' on line 10, whereas elsewhere (e.g., the function name LLM) it uses uppercase. Consider using a consistent casing (e.g., 'LLM') 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 consistency in naming is generally good, this seems like a very minor stylistic issue. The lowercase llm appears to be a type parameter which often follows different casing conventions than function names. Without seeing the rest of the codebase's conventions, we can't be certain this is even inconsistent with the project's style guide. The different cases don't impact functionality.
    I might be underestimating the importance of consistent naming conventions. Inconsistent casing could confuse new developers.
    However, type parameters and function names often intentionally use different casing conventions in many languages (e.g., Java generics use lowercase, while class names are uppercase).
    This comment should be deleted as it's too minor and speculative without knowledge of the project's style guidelines.
45. engine/baml-lib/baml/tests/validation_files/expr/mixed_pipeline.baml:13
  • Draft comment:
    Typographical note: The model name 'gpt-4o' on line 13 might be a typo. Please verify if the intended model name is 'gpt-4' or if 'gpt-4o' is correct, and update 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 = 10% vs. threshold = 50%
    This is a test file in a validation directory, which suggests it's meant to test specific scenarios. The 'o' suffix is used consistently in both the client name (GPT4o) and model name (gpt-4o), indicating this is likely intentional. Being a test file, unusual names might be used deliberately to test validation logic.
    I might be wrong if there's a strict requirement that only real OpenAI model names can be used, even in test files.
    Since this is clearly a test file and the 'o' suffix is used consistently, it's more likely that this unusual name is intentional for testing purposes.
    Delete the comment because it's questioning something that appears to be intentionally named for testing purposes.
46. engine/baml-lib/baml/tests/validation_files/expr/unknown_name.baml:12
  • Draft comment:
    Typographical style issue: In the declaration of Go3, the parameter is written as x:int without a space. Consider changing it to x: int to maintain consistency with the rest of the file.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a test file specifically created to demonstrate validation errors (unknown variables and functions). The inconsistent spacing appears intentional as part of the test cases. The file includes expected error messages at the bottom. Style consistency is not the focus of this test file.
    The inconsistent style could make the test file harder to read. Maybe maintaining consistent style is important even in test files?
    This is clearly a validation test file where the focus is on demonstrating error cases. Style consistency is not relevant to its purpose, and the inconsistency might even be intentional for testing.
    Delete the comment. Style fixes are not relevant for a test file specifically created to demonstrate validation errors.
47. engine/baml-lib/baml/tests/validation_files/strings/unquoted_strings.baml:4
  • Draft comment:
    There appears to be a potential typographical error at line 4 with the field value "hello'world". The use of an apostrophe between "hello" and "world" seems inconsistent with typical styling. Please verify if this is intentional or if it should be corrected to avoid confusion.
  • 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.
48. engine/baml-lib/jinja-runtime/src/output_format/types.rs:424
  • Draft comment:
    Typo in comment: change 'own't get any rendering' to 'won't get any rendering'.
  • 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.
49. engine/baml-lib/jsonish/src/deserializer/semantic_streaming.rs:55
  • Draft comment:
    Typo in comment: Change 'vial @stream.with_state' to 'via @stream.with_state'.
  • 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.
50. engine/baml-lib/jsonish/src/deserializer/semantic_streaming.rs:261
  • Draft comment:
    Typo in comment: Correct 'implemenation' to 'implementation'.
  • 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.
51. engine/baml-lib/schema-ast/src/ast.rs:121
  • Draft comment:
    Typographical note: In the impl of Index for TypeAliasId (line 121), the expect message reads "expected type expression", but since this is used for type alias assignments (as evident from the method call as_type_alias_assignment()), it would be clearer if it said "expected type alias assignment". Consider updating this message for consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
52. engine/baml-lib/schema-ast/src/ast/expr.rs:2
  • Draft comment:
    Consider changing 'non-llm functions' to 'non-LLM functions' for consistency with the pull request title and naming conventions.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a very minor stylistic change in a documentation comment. While consistency in documentation is good, this kind of minor capitalization issue doesn't affect code functionality or clarity. The rules state not to make purely informative comments and to only comment if there's clearly a code change required.
    The capitalization inconsistency could make the documentation look unprofessional, and consistent terminology is important in technical documentation.
    While consistency is good, this is too minor of an issue to warrant a PR comment. It doesn't affect code functionality or significantly impact readability.
    Delete this comment as it's too minor and doesn't require a clear code change. It falls under the category of "obvious or unimportant" comments that should be avoided.
53. engine/baml-lib/schema-ast/src/ast/expression.rs:252
  • Draft comment:
    Typographical Error: The method name 'as_identifer' is misspelled. Please rename it to 'as_identifier' for clarity and consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
54. engine/baml-lib/schema-ast/src/ast/top.rs:44
  • Draft comment:
    The string literal 'client' in the match arm for Top::Client looks unusual and appears to be a typographical error. Consider replacing it with the intended string (possibly 'client').
  • 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.
55. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:239
  • Draft comment:
    Typo in comment: "panicks" should be corrected to "panics".
  • 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.
56. engine/baml-lib/schema-ast/src/parser/parse_expression.rs:277
  • Draft comment:
    Typo in comment: "therefor" should be "therefore", and consider rephrasing "should be contain \n" to "should contain \n" for 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.
57. engine/baml-lib/schema-ast/src/parser/parse_schema.rs:180
  • Draft comment:
    Typographical error: "happend" should be "happen" in the panic message.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
58. engine/baml-lib/schema-ast/src/parser/parse_schema.rs:29
  • Draft comment:
    Potential typographical or documentation error: The comment states 'Parse a PSL string and return its AST.', but the rest of the file and file extension checks indicate it is parsing a BAML file. Consider updating 'PSL' to 'BAML' if appropriate.
  • 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.
59. engine/baml-lib/schema-ast/src/parser/parse_template_string.rs:63
  • Draft comment:
    On line 63, inside the templated error message, there's an unexpected double quote after the '#' in the code snippet ("template_string {}(param1: String, param2: String) #"). This extra quote might confuse readers and should probably be removed to ensure the message is clear.
  • 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.
60. engine/baml-runtime/src/lib.rs:697
  • Draft comment:
    Typo: In the comment on line 697, "requeset" should be corrected to "request".
  • 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.
61. engine/baml-schema-wasm/src/runtime_wasm/mod.rs:198
  • Draft comment:
    It appears that 'BAML_PATH_SPLTTER' may be a typo. Possibly it was intended to be 'BAML_PATH_SPLITTER'? Please check and correct the spelling.
  • 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.
62. engine/language_client_cffi/src/ctypes.rs:152
  • Draft comment:
    Typographical error: The file string "./cffi_place_holder.baml" contains an underscore which is unconventional; consider changing it to "./cffi_placeholder.baml" for consistency.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
63. engine/language_client_codegen/src/openapi.rs:661
  • Draft comment:
    Typo: In the comment just above the Optional branch handling, 'something i saw suggested doing this' should use a capital 'I' ('something I saw suggested doing this') to follow proper grammar conventions.
  • 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.
64. engine/language_client_codegen/src/python/generate_types.rs:239
  • Draft comment:
    Typo: The comment says "Short-circuite". It should be "Short-circuit".
  • 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.
65. engine/language_client_codegen/src/ruby/generate_types.rs:43
  • Draft comment:
    There's a typographical error in the doc comment on line 43: "Partial". It seems like a stray 'j' has been inserted. Consider correcting it to something like "Partial" (if that reflects the intended meaning).
  • 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.
66. engine/language_client_codegen/src/typescript/mod.rs:203
  • Draft comment:
    Typographical error: The struct name 'TypscriptLlmParser' appears to be misspelled; it should likely be 'TypescriptLlmParser' to maintain consistency with other identifiers.
  • 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.
67. engine/language_client_codegen/src/typescript/mod.rs:308
  • Draft comment:
    Typographical consistency issue: Several variables (e.g., 'typscript_client') in TryFrom implementations appear to be misspelled. For clarity and consistency, consider renaming these to 'typescript_client'.
  • 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.
68. flake.nix:31
  • Draft comment:
    Minor formatting issue: Line 31 appears to be indented using a tab instead of spaces. Please align it with the rest of the file for consistency. This is a trivial stylistic issue.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While the comment is technically correct about the inconsistency, style-only comments that don't affect functionality should be handled by automated formatters rather than manual review comments. The file appears to be a Nix configuration file, and the inconsistency doesn't impact its functionality. Additionally, this kind of nitpick can distract from more important review feedback.
    The comment identifies a real inconsistency in the code formatting. Having consistent formatting can make code more maintainable.
    While consistent formatting is good, this kind of feedback is better handled through automated tooling like formatters and linters rather than manual code review comments.
    This comment should be removed as it's a minor style issue that doesn't affect functionality and would be better handled by automated tooling.
69. typescript/fiddle-frontend/next.config.mjs:28
  • Draft comment:
    Typo detected: In the comment on line 28, 'locak' should be corrected to 'local'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
70. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:80
  • Draft comment:
    The comment reads: "We don't use ASTContext.provider because we should the default value of the context". It looks like a word is missing (perhaps 'use' or 'set') to complete the sentence. Consider clarifying 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.
71. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:107
  • Draft comment:
    In the comment on line 107, "todo: maybe we use a derived atom to reset it. But for now this useeffect works.", the term 'useeffect' should be corrected to 'useEffect' for consistency and readability.
  • 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.
72. typescript/playground-common/src/baml_wasm_web/EventListener.tsx:239
  • Draft comment:
    In the comment on line 239, "If we dont add the jotai atom callbacks here...", the word 'dont' is missing an apostrophe. It should be spelled as "don't" for proper grammar.
  • 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.
73. typescript/vscode-ext/packages/language-server/src/server.ts:55
  • Draft comment:
    Typo: 'cant' should be "can't" for proper contraction in the message printed in the catch block.
  • 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.
74. typescript/vscode-ext/packages/language-server/src/server.ts:305
  • Draft comment:
    Typo: 'mutliple' should be 'multiple' in the comment regarding deleting multiple baml_src dirs.
  • 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.
75. typescript/vscode-ext/packages/vscode/server/darwin/baml-cli:1
  • Draft comment:
    The new file does not end with a newline. While this is mainly a stylistic issue, it's generally good practice to end text files with a newline. Consider updating the file to add a newline at the end.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a symlink file, not a regular code file. Symlinks in Unix systems are special files that just contain a path reference. The presence or absence of a newline in a symlink file is irrelevant to its functionality. This is an overly pedantic comment that doesn't affect the code's behavior or maintainability.
    Maybe there's some edge case where newlines in symlink files matter that I'm not aware of? Maybe it's a standard practice in this codebase?
    Even if there were such cases, this would be a trivial issue that doesn't affect functionality. The comment violates our rule about not making obvious or unimportant comments.
    Delete this comment as it's suggesting a trivial change to a symlink file that has no practical impact.
76. typescript/vscode-ext/packages/vscode/src/extension.ts:69
  • Draft comment:
    Typo detected: The hex color code '##9333ea' appears to have an extra '#' at the beginning. It should be '#9333ea'. Please fix this to use a valid hex color code.
  • 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.
77. typescript/vscode-ext/packages/vscode/src/extension.ts:141
  • Draft comment:
    Typographical error detected in a debug log: 'console.log('Pushing test errorrrr')' appears to contain an extra 'r' in 'errorrrr'. Consider correcting it to 'error' or an appropriate message.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
78. typescript/vscode-ext/packages/vscode/src/panels/WebviewPanelHost.ts:195
  • Draft comment:
    Typographical error: 'recieved' should be corrected to 'received'.
  • 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_UQ4Yo5XVbDBQNjEY


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

Some(expr) => format!("..{}", expr.dump_str()),
None => String::new(),
};
format!("Class({} {{ {}{} }}", name, fields, spread)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that the format string in the ClassConstructor branch of dump_str() is missing a closing parenthesis. Consider changing

format!("Class({} {{ {}{} }}", name, fields, spread)

to something like

format!("Class({} {{ {}{} }})", name, fields, spread)

to ensure proper output formatting.

Suggested change
format!("Class({} {{ {}{} }}", name, fields, spread)
format!("Class({} {{ {}{} }})", name, fields, spread)

Merged via the queue into canary with commit 63b4e44 Apr 10, 2025
11 of 12 checks passed
@imalsogreg imalsogreg deleted the greg/expr-v1 branch April 10, 2025 03:52
imalsogreg added a commit that referenced this pull request Apr 10, 2025
github-merge-queue bot pushed a commit that referenced this pull request Apr 10, 2025
This reverts commit 63b4e44.
<!-- ELLIPSIS_HIDDEN -->

----

> [!IMPORTANT]
> Revert LLM function composition, removing expression function logic
and updating code generation for Python, Ruby, and TypeScript.
> 
>   - **Revert LLM Function Composition**:
> - Remove `ExprFunction` and related logic from `ir_helpers/mod.rs`,
`repr.rs`, and `walker.rs`.
> - Delete validation logic for expression functions in
`validation_pipeline/validations/expr_fns.rs` and `expr_typecheck.rs`.
> - Remove expression function handling in
`parser-database/src/walkers/expr_fn.rs`.
>   - **Code Generation**:
> - Update Python, Ruby, and TypeScript code generation to exclude
expression functions.
> - Modify `generate_types.rs` in language-specific directories to
remove handling of `ExprFunction`.
>   - **Miscellaneous**:
> - Update `flake.nix` and `flake.lock` to reflect dependency changes.
> - Adjust `next.config.mjs` and `index.ts` in Next.js plugin to remove
WebAssembly configuration.
> 
> <sup>This description was created by </sup>[<img alt="Ellipsis"
src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup>
for 67d99a4. It will automatically
update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
imalsogreg added a commit that referenced this pull request Apr 10, 2025
github-merge-queue bot pushed a commit that referenced this pull request Apr 10, 2025
This reverts commit e25d147, and
reintroduces Exprs.

Add several new syntactic forms to BAML, allowing the definition of
functions whose bodies are expressions that may compose calls to other
expression-bodied functions as well as the existing LLM functions.

The changes to the language in this PR are not meant to be used in
production BAML apps. This is foundation work for a future, user-facing
release. All syntax introduced here is very likely to change.

Closes #1707

Needs cleanup and more tests.
<!-- ELLIPSIS_HIDDEN -->


----

> [!IMPORTANT]
> Reintroduces expression functions in BAML, adding foundational support
for complex function compositions and updates across the codebase to
handle new syntax and types.
> 
>   - **Behavior**:
> - Reintroduces expression functions in BAML, allowing function bodies
to compose calls to other expression-bodied functions.
> - Not intended for production use; foundational for future releases.
>   - **IR Changes**:
> - Adds `ExprFunctionNode` and `ExprFunctionWalker` in
`ir_helpers/mod.rs`.
> - Updates `IntermediateRepr` to include `expr_fns` and
`toplevel_assignments`.
>   - **Type System**:
> - Introduces `Arrow` type in `field_type/mod.rs` for function types.
>     - Updates type coercion and checking to handle new `Arrow` type.
>   - **Validation**:
> - Adds validation for expression functions in `expr_fns.rs` and
`expr_typecheck.rs`.
>   - **Code Generation**:
> - Updates Python, Ruby, and TypeScript code generation to handle new
function types, though `Arrow` types are not yet supported in generated
definitions.
>   - **Testing**:
>     - Adds tests for expression functions in `validation_files/expr/`.
>   - **Misc**:
>     - Updates `flake.nix` and `flake.lock` for dependency management.
> - Adjusts build scripts and configurations in `flake.nix` and
`package.json`.
> 
> <sup>This description was created by </sup>[<img alt="Ellipsis"
src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup>
for 33c071e. It will automatically
update as commits are pushed.</sup>


<!-- ELLIPSIS_HIDDEN -->
imalsogreg added a commit that referenced this pull request Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify expression values and types with BAML values and types
2 participants