Added responseFormat to Azure OpenAi Chat Model #483
Conversation
UnvirtualHH
commented
Apr 22, 2024
•
UnvirtualHH
commented
- Fixes: How to enable JSON mode for azure-openai ? #466
| @@ -116,7 +120,8 @@ private Response<AiMessage> generate(List<ChatMessage> messages, | |||
| .topP(topP) | |||
| .maxTokens(maxTokens) | |||
| .presencePenalty(presencePenalty) | |||
| .frequencyPenalty(frequencyPenalty); | |||
| .frequencyPenalty(frequencyPenalty) | |||
| .responseFormat(responseFormat); | |||
There was a problem hiding this comment.
Looking at the Lang4Chain documentation, I see
AzureOpenAiChatModel.builder()
...
.responseFormat(new ChatCompletionsJsonResponseFormat())
.build();I don't see, how the correct item is injected in the builder here, is it actually working as expected ?
There was a problem hiding this comment.
Well, yes, it works as Expected. ChatCompletionsJsonResponseFormat is also possible, but you lose the flexibility of changing it over the application properties. It is now implemented the same way as for the open ai part directly.
| @@ -205,6 +205,11 @@ public Double frequencyPenalty() { | |||
| return null; | |||
| } | |||
|
|
|||
| @Override | |||
There was a problem hiding this comment.
Shouldn't a test case for the JSON use be added as well ?
There was a problem hiding this comment.
Testing is for beginners, expert just do it ;) Well, without joking, for sure a test could be added, but I needed a quick working version and I'm pretty sure that the impact from this change is minimal to the rest of the code.
There was a problem hiding this comment.
Yes, I'm sure it won't break the rest of the code :)
Just in case, this changes, the code won't be protected :)
If I have the time, I'll try to add a test
