Support for using custom Json ObjectMapper

[the-gigi]

Here is the issue. This specific instance happened during tool calls.

Oct 30 18:01:37.882
i-0c37647db12b51de5
invisible
Caused by: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Java 8 date/time type `java.time.LocalDate` not supported by default: add Module "com.fasterxml.jackson.datatype:jackson-datatype-jsr310" to enable handling

Oct 30 18:01:37.882
i-0c37647db12b51de5
invisible
... 51 more

Oct 30 18:01:37.882
i-0c37647db12b51de5
invisible
at io.github.sashirestela.openai.common.function.FunctionExecutor.execute(FunctionExecutor.java:81)

The FunctionExecutor calls Cleverclient's JsonUtil to deserialize JSON objects.

        try {
            var function = mapFunctions.get(functionName);
            var object = JsonUtil.jsonToObject(
                    functionCall.getArguments().isBlank() ? "{}" : functionCall.getArguments(),
                    function.getFunctionalClass());
            return (T) object.execute();
        } catch (RuntimeException e) {
            throw new SimpleUncheckedException("Cannot execute the function {0}.", functionName, e);
        }

If the functional class contains types that are not registered by the vanilla object mapper / object reader in Cleverclient's JsonUtil we have a problem.

public class JsonUtil {

    private static final ObjectMapper objectMapperStrict = new ObjectMapper();

    private static final ObjectReader objectReaderIgnoringUnknown = objectMapperStrict.reader()
            .without(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
    .
    .
    .
    public static <T> T jsonToObject(String json, Class<T> clazz) {
        try {
            return objectReaderIgnoringUnknown.readValue(json, clazz);
        } catch (IOException e) {
            throw new CleverClientException("Cannot convert the Json {0} to class {1}.", json, clazz.getName(), e);
        }
    }

Since the ObjectReader is generated from a vanilla ObjectMapper that doesn't register the com.fasterxml.jackson.datatype.jsr310.JavaTimeModule it can't handle dates. That's fine. Cleverclient shouldn't register any possible module. But, it should be possible to pass an external ObjectMapper/ObjectReader to all the methods in JsonUtil.

I opened an issue for Cleverclient: sashirestela/cleverclient#77

Once, it is possible to provide a custom object mapper/object reader to JsonUtil, then, the FunctionExecutor should be able to pass a custom ObjectReader to Cleverclient's JsonUtil.jsonToObject() method

Probably overload the execute() and executeAll() methods like so:

public <T> T execute(FunctionCall functionCall, ObjectReader objectReader) {
   ...
           try {
            var function = mapFunctions.get(functionName);
            var object = JsonUtil.jsonToObject(
                    functionCall.getArguments().isBlank() ? "{}" : functionCall.getArguments(),
                    function.getFunctionalClass(),
                    objectReader);
            return (T) object.execute();
        } catch (RuntimeException e) {
            throw new SimpleUncheckedException("Cannot execute the function {0}.", functionName, e);
        }
       ...
}

[kdubb]

Setting an ObjectMapper on the SimpleOpenAI.Builder would be helpful, preferred actually. When using SimpleOpenAI from Kotlin you generally want "everything" to include the Kotlin module.

[the-gigi]

Setting an ObjectMapper on the SimpleOpenAI.Builder would be helpful, preferred actually. When using SimpleOpenAI from Kotlin you generally want "everything" to include the Kotlin module.

The issue is that for different calls to jsonToObject() you may want a different ObjectReader. If it is set at the SimpleOpenAI level then whenever you need a different ObjectReader you'll have to create a new instance of SimpleOpenAI (which means passing around the API key + other initialization data).

[kdubb]

My use of preferred was in reference to our situation, needing to register a single mapper to make "anything" work. I wasn't discounting your suggestion. It seems easy enough to provide both!

[the-gigi]

Agreed. Both options can be useful for different workflows.

[sashirestela]

@the-gigi Due to my limited time availability, I prefer the option proposed by @kdubb, which involves less change. I think it could cover your requirements, but I would like to know if you agree, so I can start working on this change.

[the-gigi]

@sashirestela Yes. We can make it work for our requirements. We can aggregate the custom data types we need across all use cases into one big unified ObjectMapper that can serialize everything we need.

Thanks for adding this important capability.

[the-gigi]

@sashirestela I'm also happy to see the usage of simple-openai growing and more and more requests are coming in. Let me know if I can help with a PR or anything else.