Skip to content

Conversation

antoniosarosi
Copy link
Contributor

@antoniosarosi antoniosarosi commented Sep 2, 2025

Important

Adds support for enums in BAML compiler and VM, including bytecode generation, type checking, and runtime handling, with corresponding tests and client updates.

  • Behavior:
    • Adds support for enums in BAML compiler and VM, including bytecode generation and runtime handling.
    • Updates compile_thir_to_bytecode() in codegen.rs to handle enums and their variants.
    • Adds Enum and Variant handling in vm.rs and bytecode.rs.
    • Updates typecheck_expression() in typecheck.rs to handle enum types.
  • Tests:
    • Adds tests for enum functionality in tests/vm.rs and test_vm_async_runtime.py.
    • Updates existing tests to reflect changes in global index handling.
  • Misc:
    • Updates Python and TypeScript clients to handle new enum-related functions.
    • Fixes and refactors various parts of the codebase to support enums.

This description was created by Ellipsis for 30c77e0. You can customize this summary. It will automatically update as commits are pushed.

Copy link

vercel bot commented Sep 2, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
promptfiddle Skipped Skipped Sep 7, 2025 2:12pm

Copy link

🔒 Entelligence AI Vulnerability Scanner

No security vulnerabilities found!

Your code passed our comprehensive security analysis.


Copy link

LGTM 👍

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.

Important

Looks good to me! 👍

Reviewed everything up to 9f44e52 in 1 minute and 6 seconds. Click for details.
  • Reviewed 239 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/baml-vm/src/vm.rs:407
  • Draft comment:
    Added a DivisionByZero error variant improves clarity. Consider documenting its usage consistently.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. engine/baml-vm/src/vm.rs:948
  • Draft comment:
    Good check for zero divisor in integer division. This avoids potential runtime panics.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. engine/baml-vm/src/vm.rs:962
  • Draft comment:
    Modulo operation (BinOp::Mod) lacks a zero check. Consider handling modulo-by-zero similarly to division.
  • Reason 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-vm/src/vm.rs:818
  • Draft comment:
    Additional blank lines throughout the instruction match arms improve readability.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_6iLctOC2VCJWh7eF

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

github-actions bot commented Sep 2, 2025

Copy link

github-actions bot commented Sep 2, 2025

Copy link

codecov bot commented Sep 2, 2025

Copy link

github-actions bot commented Sep 2, 2025

Copy link

github-actions bot commented Sep 2, 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.

Important

Looks good to me! 👍

Reviewed bca1c37 in 2 minutes and 27 seconds. Click for details.
  • Reviewed 323 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 9 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/baml-vm/tests/vm.rs:278
  • Draft comment:
    Tests like array_constructor and function_returning_string use hard-coded object indices (e.g. ObjectIndex::from_raw(5) or 0) to compare expected results. This can be brittle if the object pool ordering changes. Consider using a lookup utility or asserting on the underlying values instead of fixed indices.
  • Reason 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-vm/tests/vm.rs:120
  • Draft comment:
    The helper function setup_and_exec_program reconstructs the VM from the compiled program. Ensure that any changes in the compiler (especially the ordering of globals/objects) are reflected in these tests, as they assume a deterministic ordering of object indices.
  • Reason 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-vm/tests/vm.rs:125
  • Draft comment:
    In the bytecode-focused tests, the VM is constructed manually with explicit object indices. This pattern may be fragile if the object pool implementation changes. Consider abstracting VM creation to avoid hard-coded ObjectIndex values.
  • Reason 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-vm/tests/vm.rs:1210
  • Draft comment:
    The commented out test 'basic_method_decl' indicates incomplete or unsupported functionality. If not planned for this release, consider adding a TODO note or referencing an issue for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
5. engine/baml-vm/tests/vm.rs:1910
  • Draft comment:
    Tests in the 'maps' module properly verify error cases such as NoSuchKeyInMap. Ensure similar edge cases (e.g., division by zero, invalid type for binary operators) are also covered in additional tests.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. engine/baml-vm/tests/vm.rs:1748
  • Draft comment:
    The for-loop tests (including those in the c_for_loops module) are well-structured. Consider adding comments on the expected iteration counts and side effects to help future maintainers adjust tests if loop semantics 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.
7. engine/baml-vm/tests/vm.rs:1867
  • Draft comment:
    Tests using assert_vm_executes_with_inspection include additional debugging output (such as dbg! calls). While useful for development, consider removing or gating these behind a debug flag to avoid cluttering test output.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
8. engine/baml-runtime/src/async_vm_runtime.rs:327
  • Draft comment:
    Typo in comment: 'Spanwed' should be 'Spawned'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
9. engine/baml-vm/src/vm.rs:758
  • Draft comment:
    Spotted a typographical issue in the comment on line 758. Consider changing "Seems to low level for an embedder" to "Seems too low-level for an embedder", or similar phrasing, 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 TODO comment. TODO comments are temporary by nature and the meaning is still clear despite the typo. The comment doesn't point out any actual code issues or suggest meaningful improvements. The typo does make the code slightly less professional looking. And since it's a new addition in this PR, now would be the time to fix it. However, we should focus review comments on substantive issues. Nitpicking typos in TODO comments, which are temporary by nature, adds noise to the review process. While technically correct, this comment about a typo in a TODO comment is too minor to be worth keeping and would just add noise to the review.

Workflow ID: wflow_0a7rOJ5H94wcJfAo

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

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.

Important

Looks good to me! 👍

Reviewed d808fe3 in 1 minute and 19 seconds. Click for details.
  • Reviewed 24 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/baml-compiler/src/codegen.rs:1015
  • Draft comment:
    In the Var branch of compile_expression, an undefined variable triggers a panic with a raw message. Consider converting this into a controlled compilation error (including span/context) for more user‐friendly 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.
2. engine/baml-compiler/src/codegen.rs:844
  • Draft comment:
    The assert_loop method panics if break/continue is used outside a loop. Consider emitting a user-friendly error (with context such as source span) instead of panicking.
  • Reason 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-compiler/src/codegen.rs:1390
  • Draft comment:
    In patch_jump_to, a non-jump instruction triggers a panic with an internal error message. Although this is an invariant check, enhancing it with additional context could improve debugging and error reporting.
  • Reason 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-compiler/src/codegen.rs:1342
  • Draft comment:
    A 'todo!' is used for unsupported expressions (e.g. Function or Builtin). Replacing this with a proper error reporting mechanism would help provide user-friendly VM 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.

Workflow ID: wflow_RGWq8fxcxRHnn8UW

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

github-actions bot commented Sep 2, 2025

Copy link

github-actions bot commented Sep 2, 2025

@antoniosarosi antoniosarosi changed the title User friendly VM errors User friendly VM errors & Value convertions (Baml RT -> Baml VM) Sep 2, 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.

Important

Looks good to me! 👍

Reviewed f0a81c1 in 1 minute and 2 seconds. Click for details.
  • Reviewed 217 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/baml-vm/tests/vm.rs:1209
  • Draft comment:
    The block of code for 'basic_method_decl' is commented out (lines 1209–1229). Remove or update it if no longer needed to keep the test file clean.
  • Reason 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-vm/tests/vm.rs:411
  • Draft comment:
    Several tests use raw ObjectIndex::from_raw(...) constants (e.g. in function_returning_string). Consider introducing named constants or helper functions to improve readability and reduce brittleness if object ordering changes.
  • Reason 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-vm/tests/vm.rs:1588
  • Draft comment:
    In 'test_array_element_method_field_assignment', the test modifies a field on a method call (get_data()) and expects the original object's field to change, despite a comment stating methods return copies. Please clarify or document the intended value‐semantics behavior 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.

Workflow ID: wflow_hn8ygKssneUIlx0p

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

github-actions bot commented Sep 7, 2025

Copy link

github-actions bot commented Sep 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.

Important

Looks good to me! 👍

Reviewed 960ea6b in 56 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/baml-compiler/src/codegen.rs:1710
  • Draft comment:
    The expected instruction vectors include many hard-coded numeric jump offsets (e.g. JumpIfFalse(4), Jump(3), etc.) which are brittle to internal changes. Consider using helper functions or symbolic constants in tests to ease future maintenance.
  • Reason 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-compiler/src/codegen.rs:3200
  • Draft comment:
    This test module is very large and covers many scenarios. Splitting the tests into submodules by functionality (e.g. loops, conditionals, object construction, enums, maps) would improve readability and maintainability.
  • Reason 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-compiler/src/codegen.rs:3510
  • Draft comment:
    In the 'UseMapNoKey' test the source uses a key ('world') that does not match the key defined in CreateMap ('hello'). Ensure that the intended behavior is documented and that the expected instructions correctly reflect the lookup behavior.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. engine/baml-compiler/src/codegen.rs:3105
  • Draft comment:
    The tests for functions with early returns and complex jump flows (e.g. in 'early_return') mix jump instructions with various pop operations. Inline comments within the expected instruction arrays would help clarify the control flow for future maintainers.
  • Reason 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_xRBTHlkOPFlv6mpD

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

github-actions bot commented Sep 7, 2025

Copy link

github-actions bot commented Sep 7, 2025

Copy link

github-actions bot commented Sep 7, 2025

Copy link

github-actions bot commented Sep 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.

Important

Looks good to me! 👍

Reviewed 30c77e0 in 2 minutes and 37 seconds. Click for details.
  • Reviewed 272 lines of code in 15 files
  • Skipped 0 files when reviewing.
  • Skipped posting 18 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. engine/baml-lib/baml/tests/bytecode_files/array_access.baml:48
  • Draft comment:
    Added missing enum comment for std::HttpMethod. This clarifies supported types in the test output.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. engine/baml-lib/baml/tests/bytecode_files/assert.baml:35
  • Draft comment:
    Added enum std::HttpMethod comment. Improves type documentation in this test file.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. engine/baml-lib/baml/tests/bytecode_files/function_calls.baml:57
  • Draft comment:
    Inserted enum std::HttpMethod comment for consistency. Ensure the enum is supported in VM output.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
4. engine/baml-lib/baml/tests/bytecode_files/literal_values.baml:49
  • Draft comment:
    Added enum std::HttpMethod comment to literal_values.baml. This addition maintains consistency across test files.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. engine/baml-lib/baml/tests/bytecode_files/llm_functions.baml:38
  • Draft comment:
    Added missing enum comments for both std::HttpMethod and Sentiment. This clarifies type usage in LLM-related functions.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
6. engine/baml-lib/baml/tests/bytecode_files/loops/break.baml:93
  • Draft comment:
    Inserted enum std::HttpMethod comment in loops/break.baml for consistency with other test files.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
7. engine/baml-lib/baml/tests/bytecode_files/loops/c_for.baml:55
  • Draft comment:
    Updated global index for std.Array.len from 5 to 6. Ensure that the VM's global function table reflects this change.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
8. engine/baml-lib/baml/tests/bytecode_files/loops/for.baml:62
  • Draft comment:
    Adjusted call index for std.Array.len (from 5 to 6) in NestedFor. Verify that global indices remain consistent with VM updates.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
9. engine/baml-lib/baml/tests/bytecode_files/loops/while_loops.baml:127
  • Draft comment:
    Added enum std::HttpMethod comment to while_loops.baml; maintains uniform documentation across loop tests.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
10. engine/baml-lib/baml/tests/bytecode_files/maps.baml:117
  • Draft comment:
    Updated global function indices for std.Map.contains and std.Map.len. Double-check that these indices match the current VM function ordering.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
11. engine/baml-lib/baml/tests/bytecode_files/mut_variables.baml:31
  • Draft comment:
    Added enum std::HttpMethod comment for consistency in mutability tests.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
12. engine/baml-lib/baml/tests/bytecode_files/return_statement.baml:100
  • Draft comment:
    Inserted enum std::HttpMethod comment in return_statement.baml to align with other test files.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
13. engine/baml-lib/baml/tests/bytecode_files/simple_function.baml:8
  • Draft comment:
    Added enum std::HttpMethod comment to simple_function.baml. This ensures consistency in documenting available types.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
14. engine/baml-lib/baml/tests/bytecode_tests.rs:106
  • Draft comment:
    Added new match arms for Object::Enum, Object::Variant, and Object::Media in the bytecode output. This enhances VM output coverage.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
15. PR_TITLE:1
  • Draft comment:
    Consider correcting the typo in the PR title: 'Type Convertions' should be 'Type Conversions'.
  • Reason 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/bytecode_files/function_calls.baml:56
  • Draft comment:
    Typo/Consistency: The comment on line 56 should follow the same format as other similar annotations. Consider adding a colon after "Enum" (i.e. change to "Enum: std::HttpMethod") 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 with bytecode annotations. The comment points out an inconsistency in formatting where "Enum" lacks a colon while other similar annotations ("Class:", "Function:") have colons. While this is technically correct, it's an extremely minor formatting issue in a test file that doesn't affect functionality. The inconsistency might be intentional, or there might be a specific reason for the different format. Also, this is just a test file where formatting isn't critical. While the formatting might be intentional, maintaining consistency in code comments helps with readability and demonstrates attention to detail. The comment should be deleted. While technically correct, it's an extremely minor formatting issue in a test file, and doesn't meet the bar of being an important enough issue to raise.
17. engine/baml-lib/baml/tests/bytecode_files/loops/c_for.baml:133
  • Draft comment:
    Typo: The new comment on line 133 is written as "// Enum std::HttpMethod" whereas similar annotations use a colon after the type (e.g., "Class: std::Request with 3 fields"). Consider changing it to "// Enum: std::HttpMethod" 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 containing bytecode and type information. The comment is technically correct about the inconsistency, but it's a very minor style issue in what appears to be test/documentation content. The comment doesn't point out any functional issues or code quality problems that need fixing. Style consistency in test file comments is not a high priority issue. The inconsistency could make the file harder to parse if it's being used for automated testing or documentation generation. Maybe the colon is functionally important? Given this is a test file and there's no evidence the colon is functionally important (the file appears to be human-readable documentation of bytecode), this is likely just a cosmetic issue. This comment should be deleted as it points out a very minor style inconsistency in test file documentation that doesn't impact functionality or code quality.
18. engine/baml-lib/baml/tests/bytecode_files/return_statement.baml:103
  • Draft comment:
    Typographical inconsistency: Consider adding a colon after 'Enum' to match the format of other comments (e.g., 'Class: ...', 'Function: ...').
  • 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, it's an extremely minor formatting issue. The meaning is perfectly clear without the colon. This kind of nitpicky formatting comment doesn't meet the bar of "clearly a code change required" and could be considered obvious/unimportant. The inconsistency could make the code slightly harder to parse automatically if there's tooling that relies on this format. Also, maintaining consistent style can be important for readability. Even if there is tooling, it should be robust enough to handle both formats. The readability impact is negligible, and this kind of minor style issue should be handled by automated formatters, not PR comments. Delete this comment as it's too minor and doesn't meet the bar of "clearly a code change required." Style consistency at this level should be handled by tooling, not manual reviews.

Workflow ID: wflow_mOkW2IxC8pR0zKHh

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@antoniosarosi antoniosarosi added this pull request to the merge queue Sep 7, 2025
Merged via the queue into canary with commit 51fc365 Sep 7, 2025
24 of 25 checks passed
@antoniosarosi antoniosarosi deleted the antonio/vm-errors-and-panics branch September 7, 2025 14:39
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.

1 participant