-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(function_schema): add enforce_type_annotations flag for stricter schema validation with type enforcement, clearer errors, and updated docstrings #1092
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
base: main
Are you sure you want to change the base?
Conversation
… schema validation with type enforcement, clearer errors, and updated docstrings Problem: Previously, the `function_schema` function silently defaulted unannotated parameters to `Any`, leading to: - Ambiguous JSON schemas (`"type": "any"`) - Potential runtime errors due to invalid inputs - Reduced reliability of LLM tool calls This made it harder to ensure type safety without manual validation. Solution: This PR introduces the `enforce_type_annotations` flag to `function_schema`, allowing developers to: 1. Enforce type annotations by setting `enforce_type_annotations=True`, which raises a `ValueError` for unannotated parameters. 2. Maintain backward compatibility by defaulting to `Any` when `enforce_type_annotations=False` (default). Example error message: ```python raise ValueError( f"Parameter '{name}' must be type-annotated. Example: def func({name}: str)" ) ``` Changes Made 1. New Parameter: ```python def function_schema( func: Callable[..., Any], enforce_type_annotations: bool = False, # New parameter ... ): ``` 2. Validation Logic: - If `enforce_type_annotations=True`, unannotated parameters now raise a `ValueError` with a clear example. - Falls back to `Any` if disabled (default behavior preserved). 3. Docstring Updates: - Added detailed documentation for `enforce_type_annotations`: ```python Args: enforce_type_annotations: If True, raises a ValueError for any unannotated parameters. If False (default), unannotated parameters are assumed to be of type `Any`. ``` 4. Error Message Improvements: - Clear guidance for developers: `Example: def func(x: str)` Backward Compatibility: - Preserved: Existing code continues to work as-is (default `enforce_type_annotations=False`). - Opt-in: Type annotation enforcement is optional, allowing gradual adoption. Testing Guidance: To verify the change: ```python def test_enforce_type_annotations(): def func(x): ... with pytest.raises(ValueError): function_schema(func, enforce_type_annotations=True) # Should not raise function_schema(func, enforce_type_annotations=False) ``` Impact: - Type Safety: Prevents ambiguous schemas and improves LLM tool reliability. - Developer Experience: Clear error messages guide users to fix missing annotations. - Flexibility: Maintains backward compatibility while enabling stricter validation. Example Usage: ```python # Strict mode: raises error for unannotated params schema = function_schema(my_func, enforce_type_annotations=True) # Default mode: works as before schema = function_schema(my_func) # silently defaults to Any ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
having unit tests would be appreciated too
src/agents/function_schema.py
Outdated
@@ -186,6 +186,7 @@ def generate_func_documentation( | |||
|
|||
def function_schema( | |||
func: Callable[..., Any], | |||
enforce_type_annotations: bool = False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding this option may be a good one, but adding here (=2nd arg) is a breaking change. new arguments must be added as the last one when a method accepts both args and keyword args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! You're absolutely right, the enforce_type_annotations parameter has been moved to the end of the argument list in the latest update to preserve backward compatibility.
Adds a new enforce_type_annotations flag to function_schema to improve type safety: - When enabled (True): Raises an error for unannotated parameters (e.g., def func(x): ...). - When disabled (False): Falls back to Any (default behavior remains unchanged). - Backward compatibility: Positional arguments still work (e.g., function_schema(my_func, "sphinx")). Example Error Message: Parameter 'x' must be type-annotated. Example: def func(x: str) Test Coverage: https://github.com/openai/openai-agents-python/blob/main/tests/test_function_schema.py
@seratch
Tests are here:
|
Adds a new enforce_type_annotations flag to function_schema to improve type safety: - When enabled (True): Raises an error for unannotated parameters (e.g., def func(x): ...). - When disabled (False): Falls back to Any (default behavior remains unchanged). - Backward compatibility: Positional arguments still work (e.g., function_schema(my_func, "sphinx")). Example Error Message: Parameter 'x' must be type-annotated. Example: def func(x: str) Test Coverage: https://github.com/openai/openai-agents-python/blob/main/tests/test_function_schema.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks good to me, but @rm-openai if you have a different opinion, please share it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My general stance is to be careful about adding new options/settings bc each one makes it that much harder to understand the code. In this case, my thought is -- not using types is your choice, why would someone not use type annotations but also use enforce_type_annotations?
Thank you for the thoughtful feedback! The enforce_type_annotations flag is designed as an opt-in tool for teams gradually adopting type safety, not a default requirement. This allows:
|
Thanks for the thorough feedback @seratch and @rm-openai! Key Updates:
This balances flexibility and safety while respecting existing workflows. Let me know your thoughts! |
Problem:
Previously, the
function_schema
function silently defaulted unannotated parameters toAny
, leading to:"type": "any"
)Solution:
This PR introduces the
enforce_type_annotations
flag tofunction_schema
, allowing developers to:enforce_type_annotations=True
, which raises aValueError
for unannotated parameters.Any
whenenforce_type_annotations=False
(default).Example error message:
Changes Made
enforce_type_annotations=True
, unannotated parameters now raise aValueError
with a clear example.Any
if disabled (default behavior preserved).enforce_type_annotations
:Example: def func(x: str)
Backward Compatibility:
enforce_type_annotations=False
).Testing Guidance:
To verify the change:
Impact:
Example Usage: