fix: convert functionResponse parts to Spring AI ToolResponseMessage - #1424
fix: convert functionResponse parts to Spring AI ToolResponseMessage#1424hemasekhar-p wants to merge 1 commit into
Conversation
84aea20 to
2ced4e6
Compare
|
I tested this PR locally against contrib/spring-ai. I noticed that testToLlmPromptWithFunctionResponse creates a FunctionResponse with id("call_123"), but then uses Part.fromFunctionResponse(name, response), which drops the call ID before MessageConverter receives it. I updated the test locally to preserve the original FunctionResponse: Part.builder().functionResponse(functionResponse).build() and verified that the tool call ID is preserved: assertThat(response.id()).isEqualTo("call_123"); I also added a function-response-only test verifying that no empty UserMessage is emitted. MessageConverterTest: 32 tests, 0 failures, 0 errors. I have the additional test coverage pushed to my fork and can open a test-only PR if useful. |
2ced4e6 to
4eb6aab
Compare
| ToolResponseMessage toolResponse = (ToolResponseMessage) toolResponseMessage; | ||
| assertThat(toolResponse.getResponses()).hasSize(1); | ||
| ToolResponseMessage.ToolResponse response = toolResponse.getResponses().get(0); | ||
| assertThat(response.name()).isEqualTo("get_weather"); |
There was a problem hiding this comment.
Shouldn't we also assert the tool call id? That's the actual subject of #1423, and testToLlmPromptWithFunctionCall already does it for the other direction at line 152.
It looks like it needs a fixture change — Part.fromFunctionResponse(name, response) takes only two arguments, so the .id("call_123") set above never reaches the part and response.id() is currently "". Building it the way line 131 does, keeps the id.
| // TODO: This test is currently limited due to Spring AI 1.1.0 API constraints | ||
| // ToolResponseMessage constructors are protected, so function responses are skipped | ||
| // Once Spring AI provides public APIs, this test should be updated to verify: | ||
| // 1. ToolResponseMessage is created | ||
| // 2. Tool response data is properly converted | ||
| // 3. Tool call IDs are preserved |
There was a problem hiding this comment.
Can we finish the mentioned assertions and already resolve this TODO ?
| @@ -184,19 +185,18 @@ | |||
| Prompt prompt = messageConverter.toLlmPrompt(request); | |||
|
|
|||
| // Currently only UserMessage is created (function response is skipped) | |||
There was a problem hiding this comment.
Is this still correct? Maybe it should be removed?
|
Closing as a duplicate of #1459 |
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
When using Spring AI as the model provider in ADK and a tool call is executed, the tool execution result (FunctionResponse part inside ADK Content) was dropped during Prompt construction in MessageConverter.java. On the second request to the LLM, the prompt sent an AssistantMessage containing tool_calls followed by a user message missing the matching tool_call_id response messages. This caused provider API validation failures across LLM providers (e.g., OpenAI HTTP 400 Bad Request: "An assistant message with 'tool_calls' must be followed by tool messages responding to each 'tool_call_id'"). Furthermore, MessageConverter generated empty UserMessage("") instances on tool response turns, causing additional validation failures on strict providers
Solution:
Unit Tests:
Please include a summary of passed java test results.
Manual End-to-End (E2E) Tests:
Executed multi-turn tool execution flow with a Spring AI hello-time-agent sample using mock and live model drivers before and after fix to confirm fix works as expected or not.
Checklist