-
Notifications
You must be signed in to change notification settings - Fork 2k
fix: ensure valid parameters schema for parameter-less functions in OpenAiApi #4832
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
Conversation
…penAiApi Signed-off-by: liugddx <[email protected]>
…penAiApi Signed-off-by: liugddx <[email protected]>
…penAiApi Signed-off-by: liugddx <[email protected]>
…penAiApi Signed-off-by: liugddx <[email protected]>
|
@ilayaperumalg PTAL |
|
LGTM! |
- For both Sync/Async toolcallbacks, verify the inputSchema when setting the Spring AI ToolDefinition
- This is needed for the cases where the inputSchema doesn't include required JSON schema parameters such as "properties" when parameterless tool definition is used. This specific case is implicitly handled for both Spring AI FunctionToolCallback and McpTool annotated Tools and needs to be fixed for the case where MCP tools don't have valid inputSchema set.
- Applied the fix specifically for Sync and Async ToolCallbacks
- Added JsonSchemaUtils to ensure the schema is validated and fixed with the required parameters. Thanks to @liugddx for the fix from spring-projects#4832
- Add integration tests in OpenAiChatModelIT to verify MCP toolbacks with incorrect inputSchema is handled correctly
- Add MCP integration test to validate the same
Fixes spring-projects#4776
Co-authored-by: liugddx <[email protected]>
Signed-off-by: Ilayaperumal Gopinathan <[email protected]>
|
@liugddx Thanks for the PR! While your PR fixes the underlying issue, we don't want to fix this just for the OpenAI API. Also, Spring AI always makes sure to set the "properties" in "inputSchema" for parameter-less functions as in here. Given this issue can only happen for the MCP tools which aren't handled implicitly by the Spring AI, it makes sense to fix at both Sync/Async ToolCallbacks. Hence, I have added a PR #4855 which addresses the fix at the MCP toolcallbacks and applied your validation changes from this PR. I also made sure you as a co-author to the newly submitted PR. Could you review the PR #4855. We target to get this in for 1.1.0 GA. Thanks once again! |
- For both Sync/Async toolcallbacks, verify the inputSchema when setting the Spring AI ToolDefinition
- This is needed for the cases where the MCP servers that provide Tools with incomplete inputSchema. In particular schemas that
doesn't include JSON schema parameters such as "properties" when parameterless tool definition is used. This specific case
is implicitly handled for Spring AI's FunctionToolCallback, MethodToolCallback (@tool) and @mcptool tools when creating MCP Servers.
The fix is only needs for external (non SpringAI) MCP server that doesn't provide the required JSON fields.
- Applied the fix specifically for Sync and Async ToolCallbacks
- Added JsonSchemaUtils to ensure the schema is validated and fixed with the required parameters. Thanks to @liugddx for the fix from fix: ensure valid parameters schema for parameter-less functions in OpenAiApi #4832
- Add integration tests in OpenAiChatModelIT to verify MCP tool callbacks with incorrect inputSchema is handled correctly
- Add MCP integration test to validate the same
- Move Sync/Async ToolDefinition creation into utils
- Add assertion and updates to McpToolCallbackParameterlessToolIT
Fixes #4776
Signed-off-by: Ilayaperumal Gopinathan <[email protected]>
Co-authored-by: liugddx <[email protected]>
Close #4776
When calling the OpenAI API using Spring AI, a tool function without parameters throws the following error:
Error reason: The OpenAI API requires that the
parametersfield of all functions must include thepropertiesattribute, even if the function has no parameters.