search

quarkiverse / quarkus-langchain4j

Quarkus Langchain4j extension
https://docs.quarkiverse.io/quarkus-langchain4j/dev/index.html
Apache License 2.0
148 stars 89 forks source link

Trying to use token-window memory type with easy rag results in exception #1036

Open edeandrea opened 3 weeks ago

edeandrea commented 3 weeks ago

I have an app (https://github.com/edeandrea/java-ai-playground/tree/quarkus) that if I try to set quarkus.langchain4j.chat-memory.type=token-window I end up getting a big fat stack trace (see below). The app uses the easy-rag extension.

  1. Clone https://github.com/edeandrea/java-ai-playground
  2. Check out the quarkus branch
  3. Make sure Ollama is running
  4. Run ./mvnw clean quarkus:dev -Dquarkus.profile=ollama -Dquarkus.langchain4j.chat-memory.type=token-window
  5. See the stack trace:
2024-11-01 08:07:22,362 ERROR [io.qua.dep.dev.IsolatedDevModeMain] (main) Failed to start quarkus: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
        [error]: Build step io.quarkus.arc.deployment.ArcProcessor#validate threw an exception: jakarta.enterprise.inject.spi.DeploymentException: jakarta.enterprise.inject.UnsatisfiedResolutionException: Unsatisfied dependency for type dev.langchain4j.model.Tokenizer and qualifiers [@jakarta.enterprise.inject.Default]
        - synthetic injection point
        - declared on SYNTHETIC bean [types=[dev.langchain4j.memory.chat.ChatMemoryProvider, java.lang.Object], qualifiers=[@Default, @Any], target=n/a]
        at io.quarkus.arc.processor.BeanDeployment.processErrors(BeanDeployment.java:1576)
        at io.quarkus.arc.processor.BeanDeployment.init(BeanDeployment.java:338)
        at io.quarkus.arc.processor.BeanProcessor.initialize(BeanProcessor.java:178)
        at io.quarkus.arc.deployment.ArcProcessor.validate(ArcProcessor.java:492)
        at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:733)
        at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
        at io.quarkus.builder.BuildContext.run(BuildContext.java:256)
        at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
        at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
        at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
        at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
        at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
        at java.base/java.lang.Thread.run(Thread.java:1583)
        at org.jboss.threads.JBossThread.run(JBossThread.java:499)
Caused by: jakarta.enterprise.inject.UnsatisfiedResolutionException: Unsatisfied dependency for type dev.langchain4j.model.Tokenizer and qualifiers [@jakarta.enterprise.inject.Default]
        - synthetic injection point
        - declared on SYNTHETIC bean [types=[dev.langchain4j.memory.chat.ChatMemoryProvider, java.lang.Object], qualifiers=[@Default, @Any], target=n/a]
        at io.quarkus.arc.processor.Beans.resolveInjectionPoint(Beans.java:546)
        at io.quarkus.arc.processor.BeanInfo.init(BeanInfo.java:698)
        at io.quarkus.arc.processor.BeanDeployment.init(BeanDeployment.java:323)
        ... 12 more

        at io.quarkus.runner.bootstrap.AugmentActionImpl.runAugment(AugmentActionImpl.java:354)
        at io.quarkus.runner.bootstrap.AugmentActionImpl.createInitialRuntimeApplication(AugmentActionImpl.java:272)
        at io.quarkus.runner.bootstrap.AugmentActionImpl.createInitialRuntimeApplication(AugmentActionImpl.java:62)
        at io.quarkus.deployment.dev.IsolatedDevModeMain.firstStart(IsolatedDevModeMain.java:89)
        at io.quarkus.deployment.dev.IsolatedDevModeMain.accept(IsolatedDevModeMain.java:428)
        at io.quarkus.deployment.dev.IsolatedDevModeMain.accept(IsolatedDevModeMain.java:55)
        at io.quarkus.bootstrap.app.CuratedApplication.runInCl(CuratedApplication.java:138)
        at io.quarkus.bootstrap.app.CuratedApplication.runInAugmentClassLoader(CuratedApplication.java:93)
        at io.quarkus.deployment.dev.DevModeMain.start(DevModeMain.java:131)
        at io.quarkus.deployment.dev.DevModeMain.main(DevModeMain.java:62)
Caused by: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
        [error]: Build step io.quarkus.arc.deployment.ArcProcessor#validate threw an exception: jakarta.enterprise.inject.spi.DeploymentException: jakarta.enterprise.inject.UnsatisfiedResolutionException: Unsatisfied dependency for type dev.langchain4j.model.Tokenizer and qualifiers [@jakarta.enterprise.inject.Default]
        - synthetic injection point
        - declared on SYNTHETIC bean [types=[dev.langchain4j.memory.chat.ChatMemoryProvider, java.lang.Object], qualifiers=[@Default, @Any], target=n/a]
        at io.quarkus.arc.processor.BeanDeployment.processErrors(BeanDeployment.java:1576)
        at io.quarkus.arc.processor.BeanDeployment.init(BeanDeployment.java:338)
        at io.quarkus.arc.processor.BeanProcessor.initialize(BeanProcessor.java:178)
        at io.quarkus.arc.deployment.ArcProcessor.validate(ArcProcessor.java:492)
        at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:733)
        at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
        at io.quarkus.builder.BuildContext.run(BuildContext.java:256)
        at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
        at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
        at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
        at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
        at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
        at java.base/java.lang.Thread.run(Thread.java:1583)
        at org.jboss.threads.JBossThread.run(JBossThread.java:499)
Caused by: jakarta.enterprise.inject.UnsatisfiedResolutionException: Unsatisfied dependency for type dev.langchain4j.model.Tokenizer and qualifiers [@jakarta.enterprise.inject.Default]
        - synthetic injection point
        - declared on SYNTHETIC bean [types=[dev.langchain4j.memory.chat.ChatMemoryProvider, java.lang.Object], qualifiers=[@Default, @Any], target=n/a]
        at io.quarkus.arc.processor.Beans.resolveInjectionPoint(Beans.java:546)
        at io.quarkus.arc.processor.BeanInfo.init(BeanInfo.java:698)
        at io.quarkus.arc.processor.BeanDeployment.init(BeanDeployment.java:323)
        ... 12 more

        at io.quarkus.builder.Execution.run(Execution.java:124)
        at io.quarkus.builder.BuildExecutionBuilder.execute(BuildExecutionBuilder.java:79)
        at io.quarkus.deployment.QuarkusAugmentor.run(QuarkusAugmentor.java:161)
        at io.quarkus.runner.bootstrap.AugmentActionImpl.runAugment(AugmentActionImpl.java:350)
        ... 9 more
Caused by: jakarta.enterprise.inject.spi.DeploymentException: jakarta.enterprise.inject.UnsatisfiedResolutionException: Unsatisfied dependency for type dev.langchain4j.model.Tokenizer and qualifiers [@jakarta.enterprise.inject.Default]
        - synthetic injection point
        - declared on SYNTHETIC bean [types=[dev.langchain4j.memory.chat.ChatMemoryProvider, java.lang.Object], qualifiers=[@Default, @Any], target=n/a]
        at io.quarkus.arc.processor.BeanDeployment.processErrors(BeanDeployment.java:1576)
        at io.quarkus.arc.processor.BeanDeployment.init(BeanDeployment.java:338)
        at io.quarkus.arc.processor.BeanProcessor.initialize(BeanProcessor.java:178)
        at io.quarkus.arc.deployment.ArcProcessor.validate(ArcProcessor.java:492)
        at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:733)
        at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
        at io.quarkus.builder.BuildContext.run(BuildContext.java:256)
        at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
        at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
        at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
        at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
        at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
        at java.base/java.lang.Thread.run(Thread.java:1583)
        at org.jboss.threads.JBossThread.run(JBossThread.java:499)
Caused by: jakarta.enterprise.inject.UnsatisfiedResolutionException: Unsatisfied dependency for type dev.langchain4j.model.Tokenizer and qualifiers [@jakarta.enterprise.inject.Default]
        - synthetic injection point
        - declared on SYNTHETIC bean [types=[dev.langchain4j.memory.chat.ChatMemoryProvider, java.lang.Object], qualifiers=[@Default, @Any], target=n/a]
        at io.quarkus.arc.processor.Beans.resolveInjectionPoint(Beans.java:546)
        at io.quarkus.arc.processor.BeanInfo.init(BeanInfo.java:698)
        at io.quarkus.arc.processor.BeanDeployment.init(BeanDeployment.java:323)
        ... 12 more
jmartisk commented 3 weeks ago

The javadoc for TOKEN_WINDOW says

If {@code token-window} is used, then the application must also provide a bean of type {@link Tokenizer}.

I assume you should create a CDI producer of Tokenizer that returns the same tokenizer that the chat model is using.. But we don't seem to have any examples :/

edeandrea commented 3 weeks ago

I assume you should create a CDI producer of Tokenizer that returns the same tokenizer that the chat model is using.. But we don't seem to have any examples :/

I scoured and couldn't find anything. I don't even really know which tokenizer to use, since I'm using easy-rag and everything is being done for me :) Shouldn't the tokenizer setup be done too?

jmartisk commented 3 weeks ago

Quarkus doesn't know what tokenizer is used under the hood when you're calling a remote model over HTTP (so you don't actually tokenize messages in the app's JVM), so that is tricky

edeandrea commented 3 weeks ago

I see there is an OpenAiTokenizer and a HuggingFaceTokenizer. Which one would I want if I was using Ollama with Llama 3.2?

edeandrea commented 3 weeks ago

I tried to add this:

@Dependent
public class LangChain4jTokenizerConfig {
    @Produces
    @ApplicationScoped
    @UnlessBuildProfile("ollama")
    public Tokenizer openAITokenizer(@ConfigProperty(name = "quarkus.langchain4j.openai.chat-model.model-name") String modelName) {
        return new OpenAiTokenizer(modelName);
    }

    @Produces
    @ApplicationScoped
    @IfBuildProfile("ollama")
    public Tokenizer ollamaTokenizer() {
        return new HuggingFaceTokenizer();
    }
}

and then re-run with

quarkus.langchain4j.chat-memory.type=token-window
quarkus.langchain4j.chat-memory.token-window.max-tokens=1000

while hooked up to OpenAI, but I started seeing this:

Exception in AssistantService.java:19
          17  
          18      public String chat(String chatId, String userMessage) {
        → 19          return langChain4JAssistant.chat(chatId, userMessage);
          20      }
          21  }

: java.lang.reflect.InvocationTargetException
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:115)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at com.vaadin.hilla.EndpointInvoker.invokeVaadinEndpointMethod(EndpointInvoker.java:454)
        at com.vaadin.hilla.EndpointInvoker.invoke(EndpointInvoker.java:203)
        at com.vaadin.hilla.EndpointController.doServeEndpoint(EndpointController.java:251)
        at com.vaadin.hilla.EndpointController.serveEndpoint(EndpointController.java:199)
        at com.github.mcollovati.quarkus.hilla.QuarkusEndpointController.serveEndpoint(QuarkusEndpointController.java:79)
        at com.github.mcollovati.quarkus.hilla.QuarkusEndpointController$quarkusrestinvoker$serveEndpoint_19ea4aa8e36421f8414cd9ce9157408f2c9d0890.invoke(Unknown Source)
        at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)
        at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:7)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.invokeHandler(AbstractResteasyReactiveContext.java:231)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
        at org.jboss.resteasy.reactive.server.handlers.RestInitialHandler.beginProcessing(RestInitialHandler.java:48)
        at io.quarkus.resteasy.reactive.server.servlet.runtime.ResteasyReactiveServlet.service(ResteasyReactiveServlet.java:31)
        at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:614)
        at io.undertow.servlet.handlers.ServletHandler.handleRequest(ServletHandler.java:74)
        at io.undertow.servlet.handlers.security.ServletSecurityRoleHandler.handleRequest(ServletSecurityRoleHandler.java:63)
        at io.undertow.servlet.handlers.ServletChain$1.handleRequest(ServletChain.java:68)
        at io.undertow.servlet.handlers.ServletDispatchingHandler.handleRequest(ServletDispatchingHandler.java:36)
        at io.undertow.servlet.handlers.RedirectDirHandler.handleRequest(RedirectDirHandler.java:67)
        at io.undertow.servlet.handlers.security.SSLInformationAssociationHandler.handleRequest(SSLInformationAssociationHandler.java:133)
        at io.undertow.servlet.handlers.security.ServletAuthenticationCallHandler.handleRequest(ServletAuthenticationCallHandler.java:57)
        at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
        at io.undertow.security.handlers.AbstractConfidentialityHandler.handleRequest(AbstractConfidentialityHandler.java:46)
        at io.undertow.servlet.handlers.security.ServletConfidentialityConstraintHandler.handleRequest(ServletConfidentialityConstraintHandler.java:65)
        at io.undertow.security.handlers.AuthenticationMechanismsHandler.handleRequest(AuthenticationMechanismsHandler.java:60)
        at io.undertow.servlet.handlers.security.CachedAuthenticatedSessionHandler.handleRequest(CachedAuthenticatedSessionHandler.java:77)
        at io.undertow.security.handlers.NotificationReceiverHandler.handleRequest(NotificationReceiverHandler.java:50)
        at io.undertow.security.handlers.AbstractSecurityContextAssociationHandler.handleRequest(AbstractSecurityContextAssociationHandler.java:43)
        at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
        at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43)
        at io.undertow.servlet.handlers.ServletInitialHandler.handleFirstRequest(ServletInitialHandler.java:247)
        at io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:111)
        at io.undertow.servlet.handlers.ServletInitialHandler$2.call(ServletInitialHandler.java:108)
        at io.undertow.servlet.core.ServletRequestContextThreadSetupAction$1.call(ServletRequestContextThreadSetupAction.java:48)
        at io.undertow.servlet.core.ContextClassLoaderSetupAction$1.call(ContextClassLoaderSetupAction.java:43)
        at io.quarkus.undertow.runtime.UndertowDeploymentRecorder$8$1.call(UndertowDeploymentRecorder.java:643)
        at io.undertow.servlet.handlers.ServletInitialHandler.dispatchRequest(ServletInitialHandler.java:227)
        at io.undertow.servlet.handlers.ServletInitialHandler.handleRequest(ServletInitialHandler.java:152)
        at io.undertow.server.handlers.CanonicalPathHandler.handleRequest(CanonicalPathHandler.java:49)
        at io.quarkus.undertow.runtime.UndertowDeploymentRecorder$1.handleRequest(UndertowDeploymentRecorder.java:126)
        at io.undertow.server.Connectors.executeRootHandler(Connectors.java:284)
        at io.undertow.server.DefaultExchangeHandler.handle(DefaultExchangeHandler.java:18)
        at io.quarkus.undertow.runtime.UndertowDeploymentRecorder$4$2.run(UndertowDeploymentRecorder.java:443)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
        at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:627)
        at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
        at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
        at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
        at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
        at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:11)
        at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:11)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.NullPointerException: Cannot invoke "java.util.Map.forEach(java.util.function.BiConsumer)" because "properties" is null
        at dev.langchain4j.model.openai.InternalOpenAiHelper.toOpenAiProperties(InternalOpenAiHelper.java:262)
        at dev.langchain4j.model.openai.InternalOpenAiHelper.toOpenAiJsonSchemaElement(InternalOpenAiHelper.java:275)
        at dev.langchain4j.model.openai.InternalOpenAiHelper.lambda$toOpenAiProperties$2(InternalOpenAiHelper.java:263)
        at java.base/java.util.HashMap.forEach(HashMap.java:1429)
        at dev.langchain4j.model.openai.InternalOpenAiHelper.toOpenAiProperties(InternalOpenAiHelper.java:262)
        at dev.langchain4j.model.openai.InternalOpenAiHelper.toOpenAiParameters(InternalOpenAiHelper.java:246)
        at dev.langchain4j.model.openai.InternalOpenAiHelper.toTool(InternalOpenAiHelper.java:204)
        at dev.langchain4j.model.openai.InternalOpenAiHelper.lambda$toTools$1(InternalOpenAiHelper.java:196)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
        at dev.langchain4j.model.openai.InternalOpenAiHelper.toTools(InternalOpenAiHelper.java:197)
        at dev.langchain4j.model.openai.OpenAiChatModel.generate(OpenAiChatModel.java:230)
        at dev.langchain4j.model.openai.OpenAiChatModel.generate(OpenAiChatModel.java:179)
        at dev.langchain4j.model.chat.ChatLanguageModel_XNMsOaekknG7BdNZ5YSUkjh1SqE_Synthetic_ClientProxy.generate(Unknown Source)
        at io.quarkiverse.langchain4j.runtime.aiservice.AiServiceMethodImplementationSupport.doImplement(AiServiceMethodImplementationSupport.java:254)
        at io.quarkiverse.langcruntime.aiservice.AiServiceMethodImplementationSupport.implement(AiServiceMethodImplementationSupport.java:122)
        at io.quarkiverse.langchain4j.runtime.aiservice.MethodImplementationSupportProducer$1$1.apply(MethodImplementationSupportProducer.java:31)
        at io.quarkiverse.langchain4j.runtime.aiservice.MethodImplementationSupportProducer$1$1.apply(MethodImplementationSupportProducer.java:28)
        at io.quarkiverse.langchain4j.runtime.aiservice.SpanWrapper.wrap(SpanWrapper.java:32)
        at io.quarkiverse.langchain4j.runtime.aiservice.MethodImplementationSupportProducer$1$2.apply(MethodImplementationSupportProducer.java:40)
        at io.quarkiverse.langchain4j.runtime.aiservice.MethodImplementationSupportProducer$1$2.apply(MethodImplementationSupportProducer.java:37)
        at io.quarkiverse.langchain4j.runtime.aiservice.MethodImplementationSupportProducer$1.implement(MethodImplementationSupportProducer.java:46)
        at org.vaadin.marcus.langchain4j.LangChain4jAssistant$$QuarkusImpl.chat(Unknown Source)
        at org.vaadin.marcus.langchain4j.LangChain4jAssistant$$QuarkusImpl_ClientProxy.chat(Unknown Source)
        at org.vaadin.marcus.client.AssistantService.chat(AssistantService.java:19)
        at org.vaadin.marcus.client.AssistantService_ClientProxy.chat(Unknown Source)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        ... 54 more
jmartisk commented 3 weeks ago

Hmm when looking at this, I see a NPE somewhere in the insides of upstream langchain4j, so it may be a bug there. I also found this PR https://github.com/langchain4j/langchain4j/pull/1668 that seems to rework some stuff about the classes involved in it, so maybe it will be fixed in the next version? Maybe @langchain4j knows more details...

langchain4j commented 3 weeks ago

I am not sure it will be fixed by https://github.com/langchain4j/langchain4j/pull/1668 as I am not sure what exactly is happening there... It seems that there is a tool defined somewhere with some unsupported parameter types? @edeandrea could you please check what's inside the ToolParameters in InternalOpenAiHelper.toOpenAiParameters when this happens? It seems there is a parameter of type Object but without any internal properties

edeandrea commented 3 weeks ago

I will investigate this a little later this morning.

edeandrea commented 3 weeks ago

@langchain4j This is the contents of ToolParameters when the above NullPointerException happens:

ToolParameters { type = "object", properties = {newFlightDate={type=object}, firstName={type=string}, lastName={type=string}, newDepartureAirport={description=3-letter code for departure airport, type=string}, newArrivalAirport={description=3-letter code for arrival airport, type=string}, bookingNumber={type=string}}, required = [bookingNumber, firstName, lastName, newFlightDate, newDepartureAirport, newArrivalAirport] }

I traced this down to this method in InternalOpenAiHelper:

  private static dev.ai4j.openai4j.chat.JsonSchemaElement toOpenAiJsonSchemaElement(Map<String, ?> properties, boolean strict) {
        // TODO rewrite when JsonSchemaElement will be used for ToolSpecification.properties
        Object type = properties.get("type");
        String description = (String) properties.get("description");
        if ("object".equals(type)) {
            List<String> required = (List<String>) properties.get("required");
            dev.ai4j.openai4j.chat.JsonObjectSchema.Builder builder = dev.ai4j.openai4j.chat.JsonObjectSchema.builder()
                    .description(description)
this is where it blows up --->     .properties(toOpenAiProperties((Map<String, ?>) properties.get("properties"), strict)); 
            if (required != null) {
                builder.required(required);
            }
            if (strict) {
                builder
                        // when strict, all fields must be required:
                        // https://platform.openai.com/docs/guides/structured-outputs/all-fields-must-be-required
                        .required(new ArrayList<>(((Map<String, ?>) properties.get("properties")).keySet()))
                        // when strict, additionalProperties must be false:
                        // https://platform.openai.com/docs/guides/structured-outputs/additionalproperties-false-must-always-be-set-in-objects
                        .additionalProperties(false);
            }
            return builder.build();
        } else if ("array".equals(type)) {
            return dev.ai4j.openai4j.chat.JsonArraySchema.builder()
                    .description(description)
                    .items(toOpenAiJsonSchemaElement((Map<String, ?>) properties.get("items"), strict))
                    .build();
        } else if (properties.get("enum") != null) {
            return dev.ai4j.openai4j.chat.JsonEnumSchema.builder()
                    .description(description)
                    .enumValues((List<String>) properties.get("enum"))
                    .build();
        } else if ("string".equals(type)) {
            return dev.ai4j.openai4j.chat.JsonStringSchema.builder()
                    .description(description)
                    .build();
        } else if ("integer".equals(type)) {
            return dev.ai4j.openai4j.chat.JsonIntegerSchema.builder()
                    .description(description)
                    .build();
        } else if ("number".equals(type)) {
            return dev.ai4j.openai4j.chat.JsonNumberSchema.builder()
                    .description(description)
                    .build();
        } else if ("boolean".equals(type)) {
            return dev.ai4j.openai4j.chat.JsonBooleanSchema.builder()
                    .description(description)
                    .build();
        } else {
            throw new IllegalArgumentException("Unknown type " + type);
        }
    }

When it blows up, here arethe value of all the objects:

image

That line calls back to the toOpenAiProperties method, where properties == null.

langchain4j commented 3 weeks ago

The problem seems to be in newFlightDate={type=object}, there are no properties inside. What type do you use for it?

edeandrea commented 3 weeks ago

In this particular case I only asked the AI "I'd like to change my booking", so I haven't yet provided the assistant with the details of what i'd like to change to.

edeandrea commented 3 weeks ago

Also, the type ofnewFlightDate is java.time.LocalDate. This is the method signature of the Tool:

@Tool("""
            Modifies an existing booking.
            This includes making changes to the flight date, and the departure and arrival airports.
            """)
    public void changeBooking(
        String bookingNumber,
        String firstName,
        String lastName,
        LocalDate newFlightDate,
        @P("3-letter code for departure airport") String newDepartureAirport,
        @P("3-letter code for arrival airport") String newArrivalAirport
    )
langchain4j commented 3 weeks ago

I suspect that the code that generates ToolSpecification does not handle LocalDate properly. AFAIK, this code is in Quarkus-LC4j project, seems like ToolProcessor?

edeandrea commented 3 weeks ago

@geoand / @jmartisk it seems that io.quarkiverse.langchain4j.deployment.ToolProcessor doesn't handle the LocalDate parameter properly?

When using straight LangChain4j upstream everything seems to work fine.

https://github.com/quarkiverse/quarkus-langchain4j/blob/4c7252264534c457c1b4f7a7481ac5886d92cfa3/core/deployment/src/main/java/io/quarkiverse/langchain4j/deployment/ToolProcessor.java#L418-L510

edeandrea commented 3 weeks ago

Also it looks like ToolProcessor uses a bunch of stuff thats deprecated in LangChain4j? JsonSchemaProperty has been deprecated in favor of the JsonSchemaElement API.

geoand commented 3 weeks ago

@edeandrea thanks.

Would you be willing to work on it?

edeandrea commented 2 weeks ago

I can try, but honestly I'm pretty unfamiliar with all the jandex stuff. I can give it a go and reach out for help should I need it.

geoand commented 2 weeks ago

👍🏽

edeandrea commented 2 weeks ago

So i've been looking at this. The problem is that Jandex isn’t indexing the java.time.LocalDate class, so when it hits this code:

https://github.com/quarkiverse/quarkus-langchain4j/blob/974e59ec00b133c62cb1e13672e55fed5ed7a95a/core/deployment/src/main/java/io/quarkiverse/langchain4j/deployment/ToolProcessor.java#L485-L507

The call to index.getClassByName(type.name()); returns null. My guess is that we need some special handling here? The problem is that I'm unsure what that special handling is.

I would think that there would be more classes than just LocalDate that would need handling. Essentially anything that Jandex isn't indexing.

Doing something like

if (class == LocalDate.class) then ....
else if (class == Duration.class) then ...
else if (class == LocalDateTime.class) then....

Doesn't seem like a good (or scalable) solution to me, so at this point I'm not sure what the right solution is?

@jmartisk / @langchain4j any thoughts?

geoand commented 2 weeks ago

@Tarjei400 was also looking into improving tool types support

edeandrea commented 2 weeks ago

Also it looks like ToolProcessor uses a bunch of stuff thats deprecated in LangChain4j? JsonSchemaProperty has been deprecated in favor of the JsonSchemaElement API.

And for this, I can't do anything about that until upstream LangChain4j has a new release, as there are things in there that aren't in the latest release (ToolSpecification.Builder.parameters(JsonObjectSchema) specifically).

edeandrea commented 2 weeks ago

@Tarjei400 was also looking into improving tool types support

Looks like in upstream LangChain4j it uses reflection to figure things out: https://github.com/langchain4j/langchain4j/blob/main/langchain4j-core/src/main/java/dev/langchain4j/model/chat/request/json/JsonSchemaElementHelper.java

We could do that if its not in the jandex index. There is already a public static method: ToolSpecifications.toolSpecificationFrom(Method), although that uses reflection on the entire method. There are several other variants in the ToolSpecifications class that do it for a particular Parameter (or collection of Parameters). We'd need to change scope from private to public so as to expose those upstream, but again we'd have to wait for a LangChain4j release, since the ToolsSpecifications class in upstream has been completely re-written from the current released version (0.35.0).

@geoand / @jmartisk / @langchain4j What do you think about this approach? Could we fall back to using reflection on method parameters in the case it isn't indexed by jandex?

geoand commented 2 weeks ago

I generally want to go the opposite direction when possible...

edeandrea commented 2 weeks ago

I generally want to go the opposite direction when possible...

Is there a way to force jandex to index things?

geoand commented 2 weeks ago

JDK types need to be handled regardless of indexing as they are not POJOs where you derive meaning from the fields - they have meaning on their own (regardless of the type system used to represent them)

Tarjei400 commented 2 weeks ago

@edeandrea Yea I noticed that too, after upstream release I was intending to add proper handling for it in this pr #1047 1047

edeandrea commented 2 weeks ago

I've been diving into Jandex (& specifically the CombinedIndexBuildItem. I think I can use the getComputingIndex to get the jandex index for the jdk class....let me try it out.

edeandrea commented 2 weeks ago

@Tarjei400 I may have a solution here where we don't need to specify additional types directly. I'm working on a small POC.

I also started the conversion to the new JsonSchemaElement API but got blocked. I have a bunch of stuff done already too. Happy to share that once I figure this other thing out.

Tarjei400 commented 2 weeks ago

@edeandrea Would be great, I didn't quite like my approach, please let me know if you had some success around this!

edeandrea commented 2 weeks ago

@geoand / @jmartisk is there a way in Jandex to find out if a FieldInfo is static? I got Jandex to index the LocalDate class, but its pulling in static final fields as fields() in the ClassInfo.

image

We really only care about the non-static fields in this case, but I can't figure out how to filter out the static things.

geoand commented 2 weeks ago

You need to use Modifier.isStatic(f.flags()) - or something along those lines (I'm on a phone now)

edeandrea commented 2 weeks ago

Modifier is from the java.lang.reflect package. Is that what you mean? There is a Modifiers class in Jandex, but it doesn't have an isStatic method.

geoand commented 2 weeks ago

Right

edeandrea commented 2 weeks ago

@geoand / @Tarjei400 see #1053

edeandrea commented 2 weeks ago

@Tarjei400 / @geoand I opened #1054 to track the updates to the newer JsonSchemaElement API.

Tarjei400 commented 2 weeks ago

@edeandrea That's better than what I initially tried! I will rebase my changes once this is merged

edeandrea commented 2 weeks ago

@edeandrea That's better than what I initially tried! I will rebase my changes once this is merged

Thanks! I had to learn a bit about Jandex, but if something isn't indexed you can use the computingIndex, which it will compute the index on the fly and then cache it.