Update ModelListener API to be more integration friendly

[geoand]

The idea here is to allow the onRequest method to return an object which will then be passed along to the
response and error methods.
This allows for integrations to attach anything they need when the request is made and access it when the response or error methods are called.

On the implementation side this looks a little
awkward because of the generics, but the upside
is that for users / integrators of ModelListener,
the signature of the class and methods provide very good hints on how to implement them

This follows up on the discussion I started here.

[langchain4j]

Hi @geoand I really like the idea of making it more flexible, but I am not sure about this exact implementation.
Is it a common pattern? One tiny issue is that user has to return something in the onRequest() method in order to get it in other methods.

For me it feels simpler to have a Context obejct that can be provided in every method and user is able to store some data inside (in attributes):

public class Context<Request, Response> {

    private Request request;
    private Response response;
    private final Map<Object, Object> attributes = new HashMap<>();
}

And ModelListener can be adjusted like this:

public interface ModelListener<Request, Response> {

    default void onRequest(Request request, Context<Request, Response> context) {
    }

    default void onResponse(Response response, Context<Request, Response> context) {
    }

    default void onError(Throwable error, Context<Request, Response> context) {
    }
}

Then users can store and retrieve attributes like this:

ModelListener<ChatLanguageModelRequest, ChatLanguageModelResponse> modelListener =
                new ModelListener<ChatLanguageModelRequest, ChatLanguageModelResponse>() {

                    @Override
                    public void onRequest(ChatLanguageModelRequest request,
                                          Context<ChatLanguageModelRequest, ChatLanguageModelResponse> context) {
                        context.attributes().put("id", "12345");
                    }

                    @Override
                    public void onResponse(ChatLanguageModelResponse response,
                                           Context<ChatLanguageModelRequest, ChatLanguageModelResponse> context) {
                        ChatLanguageModelRequest request = context.request();
                        Object id = context.attributes().get("id");
                    }

                    @Override
                    public void onError(Throwable error,
                                        Context<ChatLanguageModelRequest, ChatLanguageModelResponse> context) {
                        ChatLanguageModelRequest request = context.request();
                        ChatLanguageModelResponse response = context.response();
                        Object id = context.attributes().get("id");
                    }
                };

WDYT?

[geoand]

Is it a common pattern

It is used in the Vertx metrics integration fairly extensively

[geoand]

The context idea works equally well, whichever you prefer is fine with me

[ashimoon]

I like the idea of a Context object as opposed to returning a different object. That being said, if the Context object contains both the Request and Response objects, then why do we need to pass individual Request and Response parameters to the onRequest/onResponse methods?

Alternatively, perhaps we can have separate RequestContext, ResponseContext, ( and potentially ErrorContext) objects:

// all fields final for immutability
@Getter
@Builder
public class RequestContext<TRequest> {
    private final TRequest request;
    private final Map<Object, Object> attributes = new HashMap<>();
}

@Getter
@Builder
public class ResponseContext<TRequest, TResponse> {
    private final RequestContext<TRequest> requestContext;
    private final TResponse response;
}

@Getter
@Builder
public class ErrorContext<TRequest> {
    private final RequestContext<TRequest> requestContext;
    private final Exception exception;
}

public interface ModelListener<TRequest, TResponse> {

    default void onRequest(RequestContext<TRequest> context) {
    }

    default void onResponse(ResponseContext<TRequest, TResponse> context) {
    }

    default void onException(ErrorContext<TRequest> context) {
    }
}

This would also allow us to have different data associated with onRequest (only request object is available), onResponse (has both request context and response object). This could be expanded further (e.g. the request context can contain information about the chat model (e.g. model name, temperature, etc), while the response context can include completion information (token usage, etc).

[geoand]

@langchain4j any chance we could get one of these proposals in 0.31.1?

[langchain4j]

@geoand yes, I plan to check this today

[geoand]

🙏🏼

[langchain4j]

Hi @brunobat, @geoand, @ashimoon

I have doubts about generic ModelListener<Request, Response>.
This seems like a premature generalization. Some models have different needs (e.g. streaming), so it seems it is better to go with a separate listener interface for each model to not tie them together. WDYT?

[geoand]

From my POV (as an integrator, not and end user), that's fine.

[langchain4j]

@geoand thanks, do you see any major drawbacks from the user perspective?

[geoand]

Not really

[langchain4j]

Closing this as #1229 was merged