diff --git a/ollama/_utils.py b/ollama/_utils.py index 15f1cc0c..925cb1c5 100644 --- a/ollama/_utils.py +++ b/ollama/_utils.py @@ -56,21 +56,28 @@ def _parse_docstring(doc_string: Union[str, None]) -> dict[str, str]: def convert_function_to_tool(func: Callable) -> Tool: doc_string_hash = str(hash(inspect.getdoc(func))) parsed_docstring = _parse_docstring(inspect.getdoc(func)) + signature = inspect.signature(func) + # Parameters with a default value are optional and should not appear in `required`. + params_with_defaults = {k for k, v in signature.parameters.items() if v.default is not inspect.Parameter.empty} schema = type( func.__name__, (pydantic.BaseModel,), { - '__annotations__': {k: v.annotation if v.annotation != inspect._empty else str for k, v in inspect.signature(func).parameters.items()}, - '__signature__': inspect.signature(func), + '__annotations__': {k: v.annotation if v.annotation != inspect._empty else str for k, v in signature.parameters.items()}, + '__signature__': signature, '__doc__': parsed_docstring[doc_string_hash], }, ).model_json_schema() + if params_with_defaults and schema.get('required'): + schema['required'] = [k for k in schema['required'] if k not in params_with_defaults] + for k, v in schema.get('properties', {}).items(): # If type is missing, the default is string types = {t.get('type', 'string') for t in v.get('anyOf')} if 'anyOf' in v else {v.get('type', 'string')} if 'null' in types: - schema['required'].remove(k) + if k in schema.get('required', []): + schema['required'].remove(k) types.discard('null') schema['properties'][k] = { diff --git a/tests/test_utils.py b/tests/test_utils.py index cb9e0d4f..83ece440 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -256,3 +256,48 @@ def func_with_parentheses_and_args(a: int, b: int): tool = convert_function_to_tool(func_with_parentheses_and_args).model_dump() assert tool['function']['parameters']['properties']['a']['description'] == 'First (:thing) number to add' assert tool['function']['parameters']['properties']['b']['description'] == 'Second number to add' + + +def test_function_with_default_values_not_required(): + """Parameters with default values should be optional, not required. + + Regression test: previously a parameter like `units: str = "celsius"` + was incorrectly included in the `required` list even though Python + treats it as optional. See OpenAI tool schema spec. + """ + + def get_weather(city: str, units: str = 'celsius', detailed: bool = False) -> str: + """Get the current weather for a city. + Args: + city: The city to look up. + units: Temperature units (celsius or fahrenheit). + detailed: Whether to return a detailed report. + """ + return f'{city}: 20 {units}' + + tool = convert_function_to_tool(get_weather).model_dump() + required = tool['function']['parameters']['required'] + + # Only `city` (no default) should be required. + assert required == ['city'], f'expected [city], got {required}' + + # All three params should still appear in properties with descriptions. + properties = tool['function']['parameters']['properties'] + assert set(properties.keys()) == {'city', 'units', 'detailed'} + assert properties['units']['description'] == 'Temperature units (celsius or fahrenheit).' + assert properties['detailed']['type'] == 'boolean' + + +def test_function_with_all_defaults_has_empty_required(): + def all_optional(a: int = 1, b: str = 'x') -> str: + """All optional. + Args: + a: first + b: second + """ + return f'{a}{b}' + + tool = convert_function_to_tool(all_optional).model_dump() + required = tool['function']['parameters']['required'] + # An empty required list is acceptable; some pydantic versions omit the key. + assert not required, f'expected no required params, got {required}'