Don't allow parameters with checks/asserts

[antoniosarosi]

Fix #1578

---

Important

Disallow types with checks/asserts as function parameters and update client files to reflect this change.

• Behavior:
  • Disallow types with checks/asserts as function parameters in functions.rs by updating has_checks_nested() to handle TypeAliasId and TypeExpId.
  • Add test check_in_parameter_type_alias.baml to verify disallowed types with checks as parameters.
• Client Updates:
  • Update function signatures in async_client.py, sync_client.py, and async_client.ts to remove checks from parameters.
  • Modify openapi.yaml to reflect changes in parameter types.
• Misc:
  • Rename visited to visited_classes and add visited_type_aliases in NestedChecks struct in functions.rs.

^{This description was created by for 3902b35. 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	Apr 7, 2025 9:27am

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 9b23bd8 in 2 minutes and 13 seconds

More details

• Looked at 85 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 6 drafted comments based on config settings.

1. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/functions.rs:245

• Draft comment:
  Typo in doc comment: "Recusively" should be "Recursively".
• Reason 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/baml-core/src/validate/validation_pipeline/validations/functions.rs:269

• Draft comment:
  Ensure cycle detection for type alias resolution uses expected ID type; current implementation mirrors class cycle detection.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

3. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/functions.rs:232

• Draft comment:
  Consider adding a brief comment on the new 'visited_type_aliases' field to clarify its purpose in cycle detection for type aliases.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

4. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/functions.rs:269

• Draft comment:
  Ensure that the recursive handling for type aliases (using type_alias_walker.resolved()) safely handles cyclic aliases, similar to the class case.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

5. engine/baml-lib/baml-core/src/validate/validation_pipeline/validations/functions.rs:245

• Draft comment:
  Typo in comment: 'Recusively' should be corrected to 'Recursively'.
• Reason 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-lib/baml/tests/validation_files/functions_v2/check_in_parameter_type_alias.baml:6

• Draft comment:
  Typographical error detected: 'openai/gpt-4o' on line 6. If this is a typo and it should be 'openai/gpt-4', please update accordingly.
• 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 that's deliberately testing validation errors. The 'gpt-4o' might actually be intentionally incorrect to test error handling or validation. Since this is in a test directory and specifically testing validation errors, we shouldn't assume that apparent typos are actually mistakes - they could be intentional test cases.
  The typo could actually be a real mistake that slipped into the test file. Test files should still maintain correct syntax where not specifically testing errors.
  While true, the primary purpose of this file is testing validation errors, and we don't have strong evidence that this is an actual mistake rather than an intentional test case. We should err on the side of not interfering with test files.
  Delete the comment as it's pointing out a potential issue in a test file where the "mistake" could be intentional, and we don't have strong evidence that it's actually a problem.

Workflow ID: wflow_3Gw0qrQxhemEqArX

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 3902b35 in 1 minute and 48 seconds

More details

• Looked at 740 lines of code in 23 files
• Skipped 0 files when reviewing.
• Skipped posting 14 drafted comments based on config settings.

1. integ-tests/typescript/baml_client/sync_request.ts:74

• Draft comment:
  Changed parameter type from Checked<number, "gt_ten"> to number. This aligns with the PR goal to eliminate check/assert parameters.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  This comment is purely informative, explaining a change that aligns with the PR goal. It does not provide a suggestion, ask for confirmation, or highlight a potential issue.

2. integ-tests/typescript/baml_client/sync_request.ts:2503

• Draft comment:
  Updated signature: Parameter type changed from Checked<number, "gt_ten"> to number in ReturnAliasWithMergedAttributes.
• Reason 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. integ-tests/typescript/baml_client/sync_request.ts:1814

• Draft comment:
  Parameter 'money' now uses plain number (removed Checked/assert wrappers). Confirm that any runtime validation formerly provided by Checked is now handled appropriately.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 30% <= threshold 50%
  The comment is asking the author to confirm that runtime validation is handled appropriately after removing Checked/assert wrappers. This is a request for confirmation, which is against the rules. However, it does point out a potential issue with validation that might need to be addressed.

4. integ-tests/typescript/baml_client/sync_request.ts:1615

• Draft comment:
  The union type for parameter 'p' in 'PrimitiveAlias' redundantly includes 'number' twice; consider simplifying it 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.

5. integ-tests/openapi/baml_client/openapi.yaml:5226

• Draft comment:
  There are several instances where the $ref reference to the 'Check' schema is missing a slash after the ‘#’. For example, it currently appears as '#components/schemas/Check' but should be '#/components/schemas/Check' in order to correctly resolve the reference. Please update all such occurrences.
• Reason 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. integ-tests/python/baml_client/inlinedbaml.py:26

• Draft comment:
  Trivial: Consider replacing the informal contraction 'cause' with 'because' in the comment (e.g., '// This is tricky because this class should be hoisted...') to enhance 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/react/baml_client/async_request.ts:1495

• Draft comment:
  Typo: In the union type for the parameter 'c' in function 'NestedAlias', 'number' appears twice. Please remove the duplicate 'number' to clean up the type definition.
• Reason 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/react/baml_client/inlinedbaml.ts:23

• Draft comment:
  Consider replacing 'cause' with 'because' in the comment ('// This is tricky cause this class should be hoisted...') for improved 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.

9. integ-tests/react/baml_client/react/server_streaming.ts:1444

• Draft comment:
  Minor typographical issue: In the NestedAlias function, the parameter 'c' has a union type that repeats 'number' (i.e., number | string | boolean | number | string[] | Record<string, string[]>). Please remove the duplicate 'number' from the union 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.

10. integ-tests/react/baml_client/sync_client.ts:68

• Draft comment:
  Typographical error: The error message for the 'stream' getter is missing a closing backtick and should say "must be async" instead of "must by async". Please correct it to something like: "stream is not available in BamlSyncClient. Use `import { b } from 'baml_client/async_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.

11. integ-tests/react/baml_client/sync_client.ts:1906

• Draft comment:
  Typographical issue: In the union type for the parameter 'p' of 'PrimitiveAlias', the type 'number' is listed twice (i.e. 'number | string | boolean | number'). It would be clearer to remove the duplicate and use '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.

12. integ-tests/typescript/baml_client/inlinedbaml.ts:14

• Draft comment:
  In the comment "// This is tricky cause this class should be hoisted...", consider replacing 'cause' with 'because' 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.

13. integ-tests/typescript/baml_client/sync_client.ts:68

• Draft comment:
  The error message in the 'stream' getter is missing a closing backtick. Consider changing:

  throw new Error("stream is not available in BamlSyncClient. Use `import { b } from 'baml_client/async_client")

to something like:

throw new Error("stream is not available in BamlSyncClient. Use `import { b } from 'baml_client/async_client`")

for clarity and correctness.

• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

14. integ-tests/typescript/baml_client/sync_client.ts:1906

• Draft comment:
  The type union in the 'PrimitiveAlias' method is defined as number | string | boolean | number, which includes 'number' twice. This is redundant and could be 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.

Workflow ID: wflow_KSAq8pvspIGKYISs

---

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