Trying to use token-window memory type with easy rag results in exception #1036
Comments
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? |
|
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 |
|
I see there is an |
|
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=1000while hooked up to OpenAI, but I started seeing this: |
|
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 langchain4j/langchain4j#1668 that seems to rework some stuff about the classes involved in it, so maybe it will be fixed in the next version? |
|
I am not sure it will be fixed by langchain4j/langchain4j#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 |
|
I will investigate this a little later this morning. |
|
@langchain4j This is the contents of
I traced this down to this method in 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:
That line calls back to the |
|
The problem seems to be in |
|
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. |
|
Also, the type of @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
) |
|
I suspect that the code that generates |
|
Also it looks like |
|
@edeandrea thanks. Would you be willing to work on it? |
|
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. |
|
👍🏽 |
|
So i've been looking at this. The problem is that Jandex isn’t indexing the The call to I would think that there would be more classes than just 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? |
|
@Tarjei400 was also looking into improving tool types support |
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 ( |
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: @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? |
|
I generally want to go the opposite direction when possible... |
Is there a way to force jandex to index things? |
|
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) |
|
@edeandrea Yea I noticed that too, after upstream release I was intending to add proper handling for it in this pr #1047 1047 |
|
I've been diving into Jandex (& specifically the |
|
@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. |
|
@edeandrea Would be great, I didn't quite like my approach, please let me know if you had some success around this! |
|
@geoand / @jmartisk is there a way in Jandex to find out if a
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. |
|
You need to use |
|
|
|
Right |
|
@geoand / @Tarjei400 see #1053 |
|
@Tarjei400 / @geoand I opened #1054 to track the updates to the newer |
|
@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 |
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-windowI end up getting a big fat stack trace (see below). The app uses the easy-rag extension.quarkusbranch./mvnw clean quarkus:dev -Dquarkus.profile=ollama -Dquarkus.langchain4j.chat-memory.type=token-windowThe text was updated successfully, but these errors were encountered: