Repository navigation
fix(tools): declare non-string Enum values as strings in the legacy fallback - #8071
Open
hyeonsang010716 wants to merge 1 commit into
Open
hyeonsang010716 wants to merge 1 commit into
hyeonsang010716 wants to merge 1 commit into
Conversation
…allback
When a parameter cannot be parsed by _parse_schema_from_parameter, the
legacy declaration path generates the schema of every parameter with
pydantic. Pydantic declares an Enum with non-string values as, for an
IntEnum, {'enum': [1, 2], 'type': 'integer'}, and types.Schema rejects
it because its enum only takes strings, so building the declaration
raised a ValueError.
Generate these schemas with a GenerateJsonSchema that declares every
Enum as a STRING schema with string values, as
_parse_schema_from_parameter does, and render an Enum default the same
way, so it is one of the enum values.
Fixes google#8070
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to Issue or Description of Change
Problem:
With
JSON_SCHEMA_FOR_FUNC_DECLdisabled, a function with a parameter that_parse_schema_from_parametercannot parse (such astuple[str, int]) has theschema of every parameter generated by pydantic. Pydantic declares an Enum with
non-string values as, for an
IntEnum,{'enum': [1, 2], 'type': 'integer'},and
types.Schema.model_validaterejects it becauseSchema.enumonly takesstrings. Building the declaration raised a
ValueError, so the agent failed onits first model request. #6981 fixed this only in
_parse_schema_from_parameter.Solution:
Generate the fallback schemas with
_StringEnumGenerateJsonSchema, aGenerateJsonSchemawhoseenum_schemadeclares every Enum as a STRING schemawith string values, as
_parse_schema_from_parameterdoes. This also covers anEnum nested in another type, such as
list[Level]or a pydantic model field.The values are rendered from their JSON form, so an Enum of dates still gets the
same values as before.
The default of an Enum parameter is rendered the same way, so it is one of the
enum values (
'2'rather thanLevel.HIGH). For the existing date Enum test,the default is now
'1970-01-01'rather than the member, the value it wasalready serialized to.
Literalschemas are not changed:Literal[1, 2]is still rejected, since thelegacy path only supports string literals.
Testing Plan
Unit Tests:
I have added or updated unit tests for my change.
All unit tests pass locally.
test_int_enum_falling_back_to_pydantic(both variants): anIntEnumparameter next to a
tupleparameter is a STRING schema withenum == ['1', '2']anddefault == '2'.test_nested_int_enum_falling_back_to_pydantic: the same forlist[Level].test_enum_fallback_with_member_defaultnow checks the enum and the rendereddefault.
The new tests and the updated one fail without the fix.
The full
tests/unittestsrun has one failure,test_release_dependencies.py::test_inclusive_upper_bounds_name_an_installed_release,which fails the same way on
main.pre-commit runpasses on the changed files, and mypy reports no issues in thetwo changed source files.
Manual End-to-End (E2E) Tests:
Ran the reproduction from the issue: both functions now declare
levelasSTRINGwith['1', '2'].Also ran an
LlmAgentwith a mocked model through aRunnerwith the flagdisabled, with a tool taking
level: Leveland atupleparameter, and themodel calling it with
{"level": "2"}. Before the change, building the firstrequest raised the
ValueError. Now the declared schema is{'type': 'STRING', 'enum': ['1', '2'], 'default': '1'}, and the tool receivesLevel.HIGH.Checklist