-
Notifications
You must be signed in to change notification settings - Fork 23
feat(core): Add support for map parameter type #324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This PR introduces support for the `map` parameter type in the Python SDKs, aligning it with the recent MCP Toolbox server enhancements ([#928](googleapis/genai-toolbox#928)). This allows tools to define parameters that are key-value pairs, either as a generic map with mixed-type values or as a strictly typed map. * The `ParameterSchema` in `protocol.py` has been updated to include an optional `valueType` field. * The internal `__get_type` method now correctly resolves the `map` type to the appropriate Python `dict` type hint: * `dict[str, Any]` for generic maps (when `valueType` is omitted). * `dict[str, <type>]` for typed maps (e.g., `dict[str, str]`, `dict[str, int]`). * Added extensive unit tests to `test_protocol.py` to ensure the `map` type is handled correctly. * Test coverage includes: * Generic (untyped) maps. * Typed maps for `string`, `integer`, `float`, and `boolean`. * Optional maps. * Error handling for unsupported `valueType`. > [!NOTE] > No modifications were required in `toolbox-langchain` or `toolbox-llamaindex`. They inherit this new functionality directly from `toolbox-core`.
/gcbrun |
/gcbrun |
2 similar comments
/gcbrun |
/gcbrun |
@@ -44,6 +45,12 @@ def __get_type(self) -> Type: | |||
if self.items is None: | |||
raise ValueError("Unexpected value: type is 'array' but items is None") | |||
base_type = list[self.items.__get_type()] # type: ignore | |||
elif self.type == "object": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Should this be a map type parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the manifest the type
is returned as a object
.
CC: @duwenxin99
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@duwenxin99 are there specific reasons for using different manifest and object types in the server and client?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Unresolving this comment for now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two reasons:
- To keep it consistent as the MCP manifest. MCP uses JSON schema and it doesn't have a "map" type. We are basically using "object" type to achieve the map functionality.
- If we decide to add in the "object" type parameter in the future, we can reuse the current "map" code as they are basically the same structure.
Open to discussion on this @twishabansal , @anubhav756 we can change it for the server if that's easier on the SDK side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just worried that in the config file, the user defines a map
. However, during usage with SDK or debugging, the user would see an object
. Seems like this might be a little confusing for the user. What do you folks think?
Fixes #321
This PR introduces support for the
map
parameter type in the Python SDKs, aligning it with the recent MCP Toolbox server enhancements (#928). This allows tools to define parameters that are key-value pairs, either as a generic map with mixed-type values or as a strictly typed map.Changes
ParameterSchema
inprotocol.py
has been updated to include an optionalvalueType
field.__get_type
method now correctly resolves themap
type to the appropriate Pythondict
type hint:dict[str, Any]
for generic maps (whenvalueType
is omitted).dict[str, <type>]
for typed maps (e.g.,dict[str, str]
,dict[str, int]
).Testing
test_protocol.py
to ensure themap
type is handled correctly.string
,integer
,float
, andboolean
.valueType
.Note
No modifications were required in
toolbox-langchain
ortoolbox-llamaindex
. They inherit this new functionality directly fromtoolbox-core
.