deepset-ai
Add top_k parameter to ChatMessageRetriever
Is your feature request related to a problem? Please describe. The ChatMessageRetriever for RAG + Chat always provides the whole conversation history. That way, at some point the context window is exceeded.
Describe the solution you'd like Something similar to a top_k parameter in the init of the component. That way, only a specified number of the latest chat messages is retrieved. This could the also potentially be provided to a summary prompt node, that summarizes multiple ChatMessages.
Describe alternatives you've considered Custom logic, where the last chat messages are stored separately and are concatenated to the query.
Thanks for the initiative @MetroCat69 Sure, please feel free to create a draft pull request early on so that we can give feedback.
What's the proper name here top_k, last_k or something else cc @TuanaCelik @Emil-io @julian-risch ?
What's the proper name here top_k, last_k or something else cc @TuanaCelik @Emil-io @julian-risch ?
Maybe last_k, so that people do not think it also filters down to the best matching text messages? Also it better describes what this parameter is doing, but this is just my opinion.
Yeah, good points @Emil-io thank you - I'll update the PR to use last_k as it better clarifies what's going on.
Hmm, but we also then deviate from retriever + top_k usage. Hmm, really not sure how to name this one. @TuanaCelik @julian-risch ?
I think we are making a mistake by calling this a retriever and this conversation made me come to this conclusion.
A retriever has too much connotation for something that will be retrieving based on similarity, embedding, etc. In which case top_k makes sense.
However, my understanding from the ChatMessageRetriever is that it simply fetches all of the messages in the store right? Would we have any use case where this retriever would be used to retrieve 'relevant messages' based on a similarity metirc? In which case, I think having both top_k and last_k may make sense but I'm a bit worried this mixes up concepts too much.
TLDR: I don't think we should use top_k to do something that is not actual 'top k'...
@anakin87 - What's your take on this?
I'm just commenting on the best name to give to this parameter (not the component name).
I think something like last_k, last_k_messages, or last_n_messages might work...
last_k it is, we can add top_k later when these become searching retrievers.