Maintain field order when rendering class in Python

[antoniosarosi]

Fix #1477

Converting Python objects to Rust objects in the FFI code used a HashMap which messes up ordering when rendering the Rust objects in the prompt.

---

Important

Replaces HashMap with IndexMap to maintain field order in FFI conversions and adds a test to verify this behavior.

• Behavior:
  • Replaces HashMap with IndexMap in parse_py_type.rs to maintain field order when converting Python objects to Rust objects.
  • Adds a new test test_maintain_field_order in prompt_renderer.test.ts to verify field order is preserved.
• Code Removal:
  • Removes IntoIterator implementation for MinijinjaBamlClass in baml_value_to_jinja_value.rs.
• Misc:
  • Adds MaintainFieldOrder class to type_builder.ts to support new functionality.

^{This description was created by for 33923ee. You can customize this summary. It will automatically update as commits are pushed.}

[vercel]

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	May 9, 2025 10:40pm

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 4dd7fe0 in 2 minutes and 58 seconds. Click for details.

• Reviewed 4044 lines of code in 59 files
• Skipped 0 files when reviewing.
• Skipped posting 17 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. integ-tests/typescript/tests/prompt_renderer.test.ts:6

• Draft comment:
  The test correctly verifies that the rendered JSON maintains the order of fields (a, b, c). The multi‑line string formatting is clear.
• 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 praises the test and formatting without providing any actionable feedback or suggestions for improvement. It doesn't align with the rules for useful comments.

2. integ-tests/typescript/baml_client/type_builder.ts:475

• Draft comment:
  The TypeBuilder initialization for MaintainFieldOrder is appropriately defined to preserve order using 'a', 'b', 'c'.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that something is good without offering any further insight or recommendations.

3. integ-tests/typescript/tests/prompt_renderer.test.ts:5

• Draft comment:
  The test ensuring that the prompt retains the field order for MaintainFieldOrder is clear. It expects the JSON keys to appear in order (‘a’, ‘b’, ‘c’) when rendered. Make sure that any whitespace or formatting differences (e.g. extra newlines) are consistently handled so the test does not break due to minor changes.
• 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 whitespace or formatting differences are consistently handled. It is not making a specific code suggestion or pointing out a specific issue, but rather asking for a general check. This violates the rule against asking the author to ensure or verify things.

4. engine/baml-lib/jinja-runtime/src/baml_value_to_jinja_value.rs:47

• Draft comment:
  Typo in comment: "For now we wont suppport enum alias rendering." should be "For now we won't support enum alias 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.

5. engine/baml-lib/jinja-runtime/src/baml_value_to_jinja_value.rs:137

• Draft comment:
  Typo in error message: "BamlImage has no callable attribute" might be a copy-paste error and should likely read "BamlMedia has no callable attribute" to correctly reflect the type in 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.

6. engine/language_client_python/src/parse_py_type.rs:102

• Draft comment:
  Typo: The variable name 'infered' at line 102 is misspelled. Consider renaming it to 'inferred' 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.

7. integ-tests/openapi/baml_client/openapi.yaml:6300

• Draft comment:
  There are several instances where $ref values for the Check schema are written as '#components/schemas/Check' instead of the correct '#/components/schemas/Check'. Please add the missing slash to ensure the references are valid.
• Reason 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. integ-tests/python/baml_client/async_client.py:5698

• Draft comment:
  There are several function signatures where parameters are separated by commas without an ensuing space. For example, in methods like DescribeImage2 (e.g. 'classWithImage: types.ClassWithImage,img2: baml_py.Image') and Completion (e.g. 'prefix: str,suffix: str,language: str'), the comma is not followed by a space, which affects readability. It would be better to insert a space after each comma in the parameter lists.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. integ-tests/python/baml_client/async_request.py:456

• Draft comment:
  There are several instances where parameters in function definitions are written without a space after the comma (e.g. 'prefix: str,suffix: str,language: str' in the Completion method, 'classWithImage: types.ClassWithImage,img2: baml_py.Image' in DescribeImage2/3/4, 'img: baml_py.Image,client_sector: str,client_name: str' in DescribeMedia1599, 'i1: Dict[types.MapKey, str],i2: Dict[types.MapKey, str]' in InOutEnumMapKey, 'input: str,not_cached: str' in TestCaching, and 'myArg: types.NamedArgsSingleClass,myArg2: types.NamedArgsSingleClass' in TestMulticlassNamedArgs). This is a minor typographical issue that affects readability. Please add a space after each comma in parameter lists to improve 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.

10. integ-tests/python/tests/test_prompt_renderer.py:30

• Draft comment:
  It looks like the file doesn't end with a newline. Although this is a minor stylistic issue, it's best practice to have a newline at the end of the file.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 20% <= threshold 50% The comment is pointing out a minor stylistic issue regarding the absence of a newline at the end of the file. While it is a best practice to have a newline at the end of a file, this comment is more informative and doesn't suggest a specific code change or improvement. It doesn't align with the rules for useful comments, which should focus on specific code suggestions or potential issues.

11. test-files/not-valid-json-1559/not-valid-json.baml:4

• Draft comment:
  There's an inconsistency in the numbering between class names: 'Document1559' and 'ClientDetails1559' use '1559', but 'Note1599' uses '1599'. Please verify if 'Note1599' should be renamed to 'Note1559' to maintain consistent 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.

12. integ-tests/react/baml_client/react/server_streaming_types.ts:24

• Draft comment:
  Typographical issue: In the import statement 'import type { partial_types }from "../partial_types";', consider adding a space between the closing brace and 'from' to improve readability. It should be 'import type { partial_types } from "../partial_types";'.
• Reason this comment was not posted:
  Comment was on unchanged code.

13. integ-tests/ruby/baml_client/types.rb:199

• Draft comment:
  The class 'MaintainFieldOrder' is defined two times in the file – first as an empty stub and later with a full implementation. This duplicate definition could lead to confusion. Please remove the redundant one or merge them.
• 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% Looking through the file, I notice this is a consistent pattern - every class is first declared as an empty stub with T::Struct and then fully implemented later. This appears to be intentional code organization, not a mistake. The comment suggests removing one definition but that would break the established pattern and likely the code's functionality. Could this pattern actually be required for the code to work properly? Maybe the stubs are needed for forward declarations or circular dependencies? Yes, looking at the file header comments and overall structure, this is clearly a generated file with an intentional pattern of stub declarations followed by implementations. The comment is suggesting to break a pattern that is likely required for the code to work. The comment should be deleted because it's suggesting to "fix" an intentional and consistent code pattern that appears throughout the generated file.

14. integ-tests/typescript-esm/baml_client/parser.ts:1488

• Draft comment:
  In the PrimitiveAlias return type, there is a duplicate 'number' (i.e., defined as 'number | string | boolean | number'). Please remove the extra 'number' to improve 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.

15. integ-tests/typescript-esm/baml_client/sync_client.ts:1813

• Draft comment:
  Typographical error: The union type for the parameter 'c' in 'NestedAlias' includes redundant 'number' entries. Please remove the duplicate so that the union type reads 'number | string | boolean | string[] | Record<string, 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.

16. integ-tests/typescript-esm/baml_client/sync_client.ts:1952

• Draft comment:
  Typographical error: The union type for the parameter 'p' in 'PrimitiveAlias' includes a duplicate 'number' type. Please remove the redundant 'number' so that the union type is simplified to 'number | string | boolean'.
• Reason 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. integ-tests/typescript/tests/prompt_renderer.test.ts:33

• Draft comment:
  There is no newline at the end of the file. Please add an extra newline at the end to comply with standard formatting practices.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While missing trailing newlines are a common linting issue, this is something that should be caught by standard linting tools and CI checks. It's a very minor formatting issue that doesn't affect functionality. According to the rules, we shouldn't comment on things that would be obviously caught by the build. Missing newlines can cause issues with some tools and are considered a best practice. The comment is technically correct. While correct, this is exactly the kind of minor formatting issue that should be handled by automated tools rather than manual review comments. Delete the comment as it violates the rule about not commenting on things that would be caught by the build/linting process.

Workflow ID: wflow_CbqFzHuixc1noTrS

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

[ellipsis-dev]

Skipped PR review on 59cb9a4 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed f9c932c in 2 minutes and 0 seconds. Click for details.

• Reviewed 11 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 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. integ-tests/python/uv.lock:1049

• Draft comment:
  This lock file appears to be auto‑generated and now includes reordered fields. Verify that changes to field ordering are intentional and do not affect dependency resolution.
• 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 intention behind the changes in the lock file, which is against the rules. It doesn't provide a specific suggestion or point out a clear issue that needs addressing.

2. uv.lock:1

• Draft comment:
  The lock file now shows a consistent, maintained order of fields for each package (e.g. name, version, source, etc.). This aligns with the PR’s intent to preserve field order when rendering Python classes. Please confirm that these reorderings are intentional and that test expectations are updated accordingly, as lock files are often auto‐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.

Workflow ID: wflow_67s6hE6NqDeNOKSP

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

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 33923ee in 6 minutes and 20 seconds. Click for details.

• Reviewed 18 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. integ-tests/react/baml_client/react/hooks.tsx:8937

• Draft comment:
  Explicitly typing 'action' as ServerAction improves clarity and type safety. Ensure that 'ServerAction' fully represents possible values.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. integ-tests/react/baml_client/react/hooks.tsx:8940

• Draft comment:
  Casting 'props' to HookInput ensures type compatibility, but verify that all expected fields are present in props. Consider using a more specific type if available.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. integ-tests/react/baml_client/react/hooks.tsx:8937

• Draft comment:
  Explicitly annotating 'action' as ServerAction clarifies its intended type. Ensure that ServerAction is correctly imported and that the annotation remains consistent as other actions may change.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. integ-tests/react/baml_client/react/hooks.tsx:8943

• Draft comment:
  Casting 'props' as HookInput may hide type mismatches. Consider refining the type definitions in useBamlAction instead of using an explicit cast.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment identifies a potential type safety issue with casting. However, looking at the broader context: 1. This is auto-generated code 2. The pattern is used consistently throughout the file 3. The type cast seems intentional as part of the hook's design 4. Changing this would require modifying the code generator or base hook implementation The comment raises a valid point about type safety, but may not be considering that this is generated code where type casts could be an intentional design choice. Since this is generated code, suggesting changes to the type system would need to be addressed at the generator level, not in the generated output. The current implementation likely has reasons for its type structure. The comment should be deleted because it suggests modifying generated code, which should be changed at the generator level instead.

Workflow ID: wflow_ymNXAmhocP9HLpaP

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