Skip to content

Cassandra Chat Memory: Fixed message order #2839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

linarkou
Copy link
Contributor

@linarkou linarkou commented Apr 22, 2025

Fixes issue #2815
This PR solves problem with message ordering:
CassandraChatMemory fetches rows in DESC order (by message_timestamp), and MessageChatMemoryAdvisor get and add all messages in that order - from last to first. So messages list needs to be reversed.

Same problem of JDBC Chat Memory solved here #2781

@linarkou
Copy link
Contributor Author

@michaelsembwever @tzolov As main contributor/reviewer of CassandraChatMemory feature, could you please take a look at this PR?
Thank you!


import java.time.Duration;
import java.util.List;
import java.util.UUID;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it correct to move import java… down to here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's how Intellij IDEA orders imports by default.
Didn't know about code style guide of Spring projects.


Reordered as it described in https://github.com/spring-projects/spring-framework/wiki/Code-Style

@linarkou linarkou mentioned this pull request Apr 23, 2025
@ThomasVitale
Copy link
Contributor

@linarkou thanks so much for your contribution! I'd like to reference here the comment I left on the similar PR for JDBC about the new APIs and changes we have just delivered for chat memory: #2781 (comment)

@linarkou
Copy link
Contributor Author

I would implement CassandraChatMemoryRepository in different PR and leave this one as bug fix for existing implementation.

@sobychacko
Copy link
Contributor

@linarkou Merged the PR upstream. I needed to update the autoconfig tests in the PR. See the commit here for details: 1bf5787. Thanks!

@sobychacko sobychacko closed this Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants