chore: [OpenAI] Improve e2e test stability#936
Conversation
Jonas-Isr
changed the title
CharlesDuboisSAP
left a comment
CharlesDuboisSAP
left a comment
There was a problem hiding this comment.
Should we also change the unit tests and documentation as well?
| record WeatherRequest(String location, WeatherMethod.Unit unit) {} | ||
|
|
||
| // 1. Define the function | ||
| final List<OpenAiTool> tools = | ||
| List.of( | ||
| OpenAiTool.forFunction(WeatherMethod::getCurrentWeather) | ||
| .withArgument(WeatherMethod.Request.class) |
There was a problem hiding this comment.
Why did you move the Request from the WeatherMethod.Request to the service class, it's duplication.
There was a problem hiding this comment.
You need this workaround since forFunction() in OpenAiTool needs a function with only a single argument as input. To then still use the WeatherMethod.getCurrentWeather() method you need the local WeatherRequest method. But this ensures that the WeatherMethod.getCurrentWeather() is only called with primitive arguments.
There was a problem hiding this comment.
That doesn't answer the question, I didn't talk about getCurrentWeather ?
| assertThat(toolCall2.name()).isEqualTo("getCurrentWeather"); | ||
| assertThat(toolCall1.arguments()) | ||
| .matches("\\{\"arg0\": \\{\"location\": \"[^\"]*Potsdam[^\"]*\", \"unit\": \"C\"}}"); | ||
| .matches("\\{\"arg0\": \"[^\"]*Potsdam[^\"]*\", \"arg1\": \"C\"}"); |
There was a problem hiding this comment.
Can't arguments be properly parsed by spring to as location and unit? arg0 might be worse for reliability
There was a problem hiding this comment.
As far as I know this is the (only) way Spring does it.
There was a problem hiding this comment.
I had managed to do it with Copilot a couple weeks ago but reverted it, I think it's possible
Yes, changed the unit tests as well. For the documentation see the PR linked in the description above. |
2 participants
Context
Changed the
WeatherMethodexample used for tool calls:Requestinner class,Also changed all uses of the test class accordingly
Please also consider the docs PR: