Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

Why use JsonNode instead of String? #338

Open
zhuyuy opened this issue Jul 13, 2023 · 5 comments
Open

Why use JsonNode instead of String? #338

zhuyuy opened this issue Jul 13, 2023 · 5 comments

Comments

@zhuyuy
Copy link

zhuyuy commented Jul 13, 2023

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
Copy link

jrabepixelart commented Nov 9, 2023

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
Copy link

fkesheh commented Nov 23, 2023

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
Copy link

fkesheh commented Nov 23, 2023

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
Copy link

jgoeres commented Jan 24, 2024

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
Copy link

fkesheh commented Jan 24, 2024

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants