Bugs in conversations

[javipacheco]

This PR tries to resolve 2 bugs:

Bug 1

When we are working in conversations in Xef (remember that we are creating by default the conversionId in CoreAIScope)

The problem is the order of the messages when we are sending the messages to OpenAI

Currently, we are storing the messages in the conversation in the wrong order. When we send the request to OpenAI the messages have the following order:

ASSISTANT: <response>
USER: <question>
ASSISTANT: <response>
USER: <question>

And when we add a new question from the user, we add this message to the end like this:

ASSISTANT: <response>
USER: <question>
ASSISTANT: <response>
USER: <question>
USER: <new-question>

In my tests, this is a problem that sometimes OpenAI isn't responding to the last question, it is responding to the second to last

The right order should be:

USER: <question>
ASSISTANT: <response>
USER: <question>
ASSISTANT: <response>
USER: <new-question>

This PR changes the way that we are storing the conversation in the memory in order to fix that

Note: You can revert the changes in Chat.kt and you will see that the Conversation.kt example gives the same result both times

Bug 2

Resolving this issue

[victorcrrd]

I think the problem comes from here, where memories is being reversed. Those memories include, in some way, the messages that form the chat, and they are included in a way that older (question, answer) pairs go first, and questions go before answers. That is, Q1, A1, ... Qn, An. When reversing memories, we have An, Qn, ..., A1, Q1, so answers go before questions.

I don't know if it's better to reverse the order in which question, answer pairs are added to the memory (what you are doing) or if we should take into account that Chat memories come in pairs and that we should reverse them keeping the relative order of pairs the same (and thus changing that messages function. I guess that for this case, your solution is better.

[raulraja]

my bad 🙋 , thank you!

[javipacheco]

I hadn't noticed the reversed(), it's better to do it like this, thank you @victorcrrd

I have found other 2 bugs in the conversations when we are storing the messages in the LocalVectorStore. I'm fixing them now

[javipacheco]

I have updated the PR with the new fixes and added some tests

@victorcrrd @raulraja could you please review it?

[javipacheco]

@victorcrrd @raulraja I have fixed some problems in the order messages on CombinedVectorStore and I have created some tests for this

We are using CombinedVectorStore in contextScope inside Xef. They are important changes in the base code. Could you please review the changes?

[javipacheco]

@raulraja I have been talking with @victorcrrd because we are going to change the memories() implementation in order to sort the messages by timestamp

On the other hand, should we review the Lucene implementation? This class is using VectorStore too

[javipacheco]

@raulraja @Montagon This PR is ready for review. Here are the changes:

• When we are getting messages from the VectorStore implementations, the messages are sorted by timestamp
• Messages are always sent in ascending order
• We should address the Lucene implementation in a separate PR. We have to include integrations tests for that and this is not the proposal for this PR

[raulraja]

thank you!