TheoKanning / openai-java

OpenAI Api Client in Java
MIT License
4.76k stars 1.2k forks source link

Why use JsonNode instead of String? #338

Open zhuyuy opened 1 year ago

zhuyuy commented 1 year ago

I noticed that the arguments property in com.theokanning.openai.completion.chat.ChatFunctionCall class, which is supposed to be a serialized JSON string, is being represented as a JsonNode in the code. I would like to understand why JsonNode is chosen over directly using the String type. Since TextNode also stores a string, I'm not quite sure why String type wasn't used directly. What are the advantages of using JsonNode? I apologize for not considering its benefits earlier.

jrabepixelart commented 11 months ago

Hi, the useage of JsonNode is also causing some Errors in receiving Json Arguments via streams.

If a message Junk is "\":\"" this is serialized to to : which leads to false concatination of the accumulated message.

Possible faulty Json Arguments like this are a result "arguments": "{"grapeKey:merlot"}"

Speaking of this part in the OpenAiService Class in Line 460:

String argumentsPart = messageChunk.getFunctionCall().getArguments() == null ? "" : messageChunk.getFunctionCall().getArguments().asText();

Behaviour is easily reproducable with this simple test: objectMapper.readTree("\":\"")

fkesheh commented 11 months ago

The issue is probably because it tries to leverage the same structure of full messages to chunks. This example didn't work for me https://github.com/TheoKanning/openai-java/blob/main/example/src/main/java/example/OpenAiApiFunctionsWithStreamExample.java

fkesheh commented 11 months ago

First I thought it was a Jackson version but it's gpt-4-1106-preview that works different than GPT-4 and GPT-3.5-turbo. Even the example doesn't work

jgoeres commented 9 months ago

Is there any news on this issue? I also ran into it when trying to stream function calls and accumulating the JsonNodes of the individual chunks that hold the args, by getting their textValue() (or asText() etc., I tried all) and concatenating them. For example where I would expect the concatenated result to be

{"id":1,"name": "foo", "desc": "something"}

I would get

{"id":1,"name:foo,desc:something"}

Note that the quoting of the first parameter seems to be OK. This however at first glance makes the quoting or lack thereof totally unpredicatable and thus hard to work around.

fkesheh commented 9 months ago

See my solution on #422 . The JsonNode removes some of the " causing that issue.