Skip to content

fix(utils): exclude parameters with default values from required#666

Open
Ghraven wants to merge 1 commit into
ollama:mainfrom
Ghraven:fix/tool-required-with-defaults
Open

fix(utils): exclude parameters with default values from required#666
Ghraven wants to merge 1 commit into
ollama:mainfrom
Ghraven:fix/tool-required-with-defaults

Conversation

@Ghraven
Copy link
Copy Markdown

@Ghraven Ghraven commented May 17, 2026

Summary

convert_function_to_tool currently lists every signature parameter in required, even ones that have a default value. Under both the JSON Schema and OpenAI tool schema spec, a parameter with a default is optional and should not be in required.

Repro

from ollama._utils import convert_function_to_tool

def get_weather(city: str, units: str = 'celsius') -> str:
    """Get the weather.
    Args:
        city: The city.
        units: Temperature units.
    """
    return ''

tool = convert_function_to_tool(get_weather).model_dump()
print(tool['function']['parameters']['required'])

Before: ['city', 'units']
After: ['city']

In practice this confuses some tool-calling models into always passing every argument (often with the literal default repeated back), and it doesn't match what the upstream OpenAI / Anthropic-compat endpoints expect.

Fix

  1. Collect parameter names whose inspect.Parameter.default is not inspect.Parameter.empty.
  2. After pydantic generates the schema, remove those names from schema['required'].
  3. Hardened the existing Union[X, None] block to check membership before calling .remove() so the two paths don't double-remove.

The pydantic model is still built from __annotations__ + __signature__, so introspection metadata is unchanged.

Tests

  • Added test_function_with_default_values_not_required — covers a mix of typed defaults including bool and str.
  • Added test_function_with_all_defaults_has_empty_required — covers the all-optional case.
  • All 12 tests in tests/test_utils.py pass locally.
$ pytest tests/test_utils.py -q
............                                                             [100%]
12 passed in 0.24s

Happy to adjust naming, split commits, or fold the regression test into the existing parametrized case if that would fit the codebase better.

`convert_function_to_tool` previously listed every signature parameter
in `required`, even ones with default values. This is incorrect under
both the JSON Schema and OpenAI tool schema spec: a parameter with a
default is optional.

Example:

    def get_weather(city: str, units: str = 'celsius'): ...

    # Before:  required = ['city', 'units']
    # After:   required = ['city']

The fix collects parameters that have a non-empty `default` from
`inspect.signature(func)` and removes them from the `required` list
after pydantic generates the schema. The existing `Union[X, None]`
handling is preserved and now guards against double-removal.
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