Skip to content

Accept integer input for union types that include number#2523

Open
sohumt123 wants to merge 1 commit into
huggingface:mainfrom
sohumt123:fix/union-number-int-coercion
Open

Accept integer input for union types that include number#2523
sohumt123 wants to merge 1 commit into
huggingface:mainfrom
sohumt123:fix/union-number-int-coercion

Conversation

@sohumt123

Copy link
Copy Markdown

Problem

validate_tool_arguments supports coercing an integer to a number, but only when the expected type is the scalar string "number". When "number" appears inside a union type list (e.g. a parameter annotated float | str, which _parse_union_type normalizes to {"type": ["number", "string"]}), the coercion never fires and a valid whole-number JSON value (a Python int) is rejected:

TypeError: Argument argument_a has type 'integer' but should be '['number', 'string']'

Root cause is the coercion escape hatch in src/smolagents/tools.py:

if actual_type == "integer" and expected_type == "number":
    continue

The surrounding mismatch guard already handles union lists correctly (actual_type not in expected_type), but the coercion branch is scalar-only, so an int fed to a union that contains number skips the escape hatch and raises.

This is not just a validation-helper edge case: it reaches production through execute_tool_call, where the TypeError is wrapped into an AgentToolCallError (src/smolagents/agents.py:1476-1478). A model that emits 1 instead of 1.0 for a tool whose parameter is typed float | str (or any union including a number) fails unnecessarily, even though the scalar-number version of the same tool accepts the exact same value.

Minimal repro:

from smolagents.tools import tool, validate_tool_arguments

@tool
def my_tool(x: float | str) -> str:
    """Demo tool.

    Args:
        x: a number or a string
    """
    return str(x)

validate_tool_arguments(my_tool, {"x": 1})  # raises TypeError before this change

Fix

Broaden the coercion so it fires whenever number is an acceptable type — the scalar case, or a union list that contains "number":

if actual_type == "integer" and (
    expected_type == "number" or (isinstance(expected_type, list) and "number" in expected_type)
):
    continue

Notes:

  • The isinstance(expected_type, list) guard keeps the in membership check off plain strings, so there is no accidental substring matching.
  • Booleans are unaffected: _get_json_schema_type maps bool to "boolean", never "integer", so True/False still cannot slip into a number union (the existing (str | int, True, True) case continues to reject).
  • The check is order-independent, which matches reality: _parse_union_type sorts union members, so both float | str and str | float normalize to ["number", "string"].

The docstring Note was updated to reflect that coercion also applies when "number" is one member of a union type list.

I intentionally kept this scoped to the dict-argument coercion branch. The non-dict single-argument branch (tools.py, the else at the bottom of the function) has separate pre-existing gaps (no int→number coercion at all, and a != comparison that does not account for list expected types), and a union containing Any is also rejected because that wildcard check is scalar-only. Those are worth a follow-up but are out of scope here.

Testing

Extended the existing parametrized test_validate_tool_arguments with union-with-number cases (no new fixtures; the @tool-decorated closure builds the schema exactly as production does):

(float | str, 1, False),     # int into a union including number — failed before, passes after
(str | float, 1, False),     # reversed hint order — normalizes to the same sorted schema
(float | str, 1.5, False),   # float into the union — regression guard on the happy path
(float | str, "a", False),   # string into the union — symmetry guard

Before the fix (new cases added, fix not yet applied):

FAILED tests/test_tools.py::test_validate_tool_arguments[tool_input_type13-1-False]
FAILED tests/test_tools.py::test_validate_tool_arguments[tool_input_type14-1-False]
E   TypeError: Argument argument_a has type 'integer' but should be '['number', 'string']'
2 failed, 28 passed, 2 skipped

After the fix:

python -m pytest tests/test_tools.py -k validate_tool_arguments -q
30 passed, 2 skipped

Wider regression run and the quality gate both pass:

python -m pytest tests/test_tools.py -q -k 'not gradio and not mcp'   ->  72 passed, 2 skipped
ruff check examples src tests                                          ->  All checks passed!
ruff format --check examples src tests                                 ->  73 files already formatted

Tested locally on Python 3.10.

validate_tool_arguments coerces an integer to a number when the
expected type is the scalar string "number", but not when "number"
appears inside a union type list such as ["number", "string"]
(e.g. a parameter annotated `float | str`). In that case a valid
whole-number JSON value (a Python int) is rejected with
"Argument ... has type 'integer' but should be '['number', 'string']'".

This surfaces in production through execute_tool_call, where the
TypeError is wrapped into an AgentToolCallError, so a model that
passes 1 instead of 1.0 to such a tool fails unnecessarily.

Broaden the coercion so it fires whenever "number" is acceptable:
the scalar case, or a union list that contains "number". The
isinstance(expected_type, list) guard keeps the membership check
off plain strings. Booleans are unaffected since bool maps to
"boolean", never "integer".

Extend test_validate_tool_arguments with union-with-number cases
(int and float into `float | str` / `str | float`, plus a string
case) covering both hint orderings, which normalize to the same
sorted schema.
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