Tool parameters - use indexes #1053
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for looking into this, but I am pretty sure this is not the proper solution and likely works by accident (and likely won't work for the next type we attempt to tackle). Can you paste the request / response to / from OpenAI? |
|
I cut out the calls to the embedding model - that part is irrelevant. I think I see what you're saying. The marshaller or whatever doesn't seem to understand how to construct a But it did get further than it did before! Have any thoughts on what the next step might be? |
is exactly what I wanted, thanks. That's the thing we don't want as |
|
FWIW - it works fine using upstream LangChain4j...
|
|
Yeah and that is wrong as well :) Like I said, it works for this case so we can go ahead and include, but I guarantee you it will fail soon for some other JDK type one of us is going to use as a Tool parameter. |
Well right now it works in upstream LangChain4j but fails in Quarkus LangChain4j. |
|
We are talking past each other, so let me try and put it another way. I'll include this now, but your on the hook for providing a proper solution when the next JDK type fails (because of this erroneous way of representing types) 😉 |
Maybe we are, but I'm still confused :)
As it stands, this PR doesn't solve the problem. It successfully describes the parameters, but something is still missing on the receiving side when the tool wants to be invoked. Quarkus LangChain4j doesn't know how to create the object to pass to the tool. |
Ah okay, I didn't get that. Wht doesn't it solve it? What issue are you facing? |
|
This PR as it stands gets Quarkus LangChain4j to describe the parameter, but Quarkus LangChain4j doesn't know how to create the object to pass to the tool, whereas upstream LangChain4j does. Quarkus barfs on the tool invocation: whereas when I run it with pure LangChain4j (using Spring Boot), the tool invocation works fine. LangChain4j knows how to take |
|
I don't know why it would work in upstream LangChain4j as it uses the Jackson to deserialize as well. You'll need to debug it |
Well, it does work. I'll see if I can debug it a bit and see what LangChain4j is doing. |
|
Yeah, I am not denying it works, just saying that the only way to see why is to debug the working and non working versions |
|
The other interesting thing is that with upstream I can use tools with streaming responses - something I can't do in Quarkus :/ |
|
I remember trying tools with streaming response recently and seeing that work (whereas I didn't expect it to) |
|
I've debugged upstream a bit. All the magic happens in
Seems for object type arguments LangChain4j delegates to Gson. It converts the argument to Json
and then un-marshals from Json into the object. Gson must know how to do it. |
|
Interesting. That means that upstream will soon break as well as it is going to move to Jackson. One more reason to not use this solution :) |
Or we can see it as an opportunity to improve what we do :) Let me debug what we're doing a bit. |
|
|
|
Here's the magic that makes it work in upstream.... private static final Gson GSON = new GsonBuilder()
.setPrettyPrinting()
.registerTypeAdapter(
LocalDate.class,
(JsonSerializer<LocalDate>) (localDate, type, context) ->
new JsonPrimitive(localDate.format(ISO_LOCAL_DATE))
)
.registerTypeAdapter(
LocalDate.class,
(JsonDeserializer<LocalDate>) (json, type, context) -> {
if (json.isJsonObject()) {
JsonObject jsonObject = (JsonObject) json;
int year = jsonObject.get("year").getAsInt();
int month = jsonObject.get("month").getAsInt();
int day = jsonObject.get("day").getAsInt();
return LocalDate.of(year, month, day);
} else {
return LocalDate.parse(json.getAsString(), ISO_LOCAL_DATE);
}
}
) |
|
I can just change my tool to be @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,
@P("month of the new flight date") int newFlightDateMonth,
@P("day of the month of the new flight date") int newFlightDateDayOfMonth,
@P("year of the new flight date") int newFlightDateYear,
@P("3-letter code for departure airport") String newDepartureAirport,
@P("3-letter code for arrival airport") String newArrivalAirport
) {
service.changeBooking(bookingNumber, firstName, lastName, LocalDate.of(newFlightDateYear, newFlightDateMonth, newFlightDateDayOfMonth), newDepartureAirport, newArrivalAirport);
}and now everything works :) This is probably a better solution anyways? |
|
It's a solution, I wouldn't say it's better :) |
|
There is no way around having special handling for some JDK types, because these types actually represent things |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tool parameters - Allow using Jandex for classes that aren't in the initial index
Fixes #1036