Skip to content

fix(tools): convert Enum tool arguments to the declared member - #7636

Open
Somay-kousis wants to merge 1 commit into
google:mainfrom
Somay-kousis:fix/r5-b
Open

Somay-kousis wants to merge 1 commit into
google:mainfrom
Somay-kousis:fix/r5-b

Conversation

@Somay-kousis

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

2. Or, if no issue exists, describe the change:

Problem:

A function tool with an Enum parameter got the raw value ("celsius") instead of the member on the default path (FUNCTION_TOOL_ARG_VALIDATION off). The declaration lists the enum's values, so the model sends one back, and tool code that uses the member (unit.value, is Unit.X) raised and ended the run.

Solution:

_schema_utils.preprocess_args now converts the argument to its Enum member when the parameter is annotated with an Enum subclass, Optional[...] of one, or ... | None. It sits next to the existing pydantic model and int handling on the default path.

  • A valid value ("celsius") becomes the member (Unit.CELSIUS).
  • An IntEnum value that came back as a float (2.0, from a proto Struct round trip) is matched too.
  • None and values that already are the member are left alone.
  • An unknown value passes through unchanged with a warning, the same as a failed pydantic conversion. Rejecting it stays the job of FUNCTION_TOOL_ARG_VALIDATION.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

New tests in tests/unittests/tools/test_function_tool.py:

  • test_run_async_converts_enum_value_to_member
  • test_run_async_converts_optional_enum_value_to_member (also checks that None logs no warning)
  • test_run_async_converts_int_enum_value_from_float
  • test_run_async_passes_through_unknown_enum_value
  • test_run_async_keeps_enum_member_arg
main: 3 failed, 105 passed
fix:  108 passed

The two that pass on main are guards (unknown value passes through, an existing member is kept). Breaking each part of the change (the lookup, the try/except, the None guard) makes at least one new test fail. tests/unittests/tools and tests/unittests/utils pass (3883 tests). isort and pyink are clean.

Manual End-to-End (E2E) Tests:

The repro in the issue (Agent + InMemoryRunner, scripted model):

before: AttributeError: 'str' object has no attribute 'value'
after:  {'city': 'Paris', 'unit': 'celsius'}

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

Containers of enums (list[Unit]) are not converted here.

The function declaration lists an Enum parameter's values, so the model
sends one of them back. With FUNCTION_TOOL_ARG_VALIDATION off (the
default), preprocess_args passed that raw value through, so a tool that
used the member (unit.value, unit is Unit.X) raised and ended the run.

preprocess_args now looks up the member for Enum, Optional[Enum] and
Enum | None parameters. None and existing members are left alone, and an
unknown value is passed through with a warning, as a failed pydantic
conversion is.
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.

FunctionTool passes a plain str to an Enum-typed parameter, so the tool crashes

2 participants