Skip to content
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

ChatMemory is persisted over request boundaries #95

Closed
andreas-eberle opened this issue Dec 5, 2023 · 4 comments · Fixed by #96
Closed

ChatMemory is persisted over request boundaries #95

andreas-eberle opened this issue Dec 5, 2023 · 4 comments · Fixed by #96
Assignees

Comments

@andreas-eberle
Copy link
Contributor

I was not looking a bit more into the examples from the guides page and especially the chat memory management (https://docs.quarkiverse.io/quarkus-langchain4j/dev/ai-services.html#memory). Unfortunately, I got a bit confused when I tested it as it seems to behave quite different from what I initially expected. Hopefully you can help to clarify things.

In the example code, the ChatMemoryBean is created as a RequestScoped bean. However, when you just use it, its get method is only called once with default as memoryId. At the end of the first request, it's close method is also called (as expected) and the memories map is freed. However, when you do the second request, the bean is not used any more and no new memory is created. Instead, all other requests just use the same memory.

Is this expected? If yes, why is the bean even request scoped?

One thing I noticed is that the get method is called by dev.langchain4j.service.AiServiceContext. And AiServiceContext itself has a map doing basically the same as ChatMemoryBean in the sense that it has a map from memory id to ChatMemory. I think this is where the memory lives longer than the request scope.

see also https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/Quarkus.20Langchain.3A.20Chat.20Memory.20Management.20Questions

@geoand
Copy link
Collaborator

geoand commented Dec 5, 2023

This is definitely a bug. I think I have a fix but it will have to wait for #83 to go in first

@andreas-eberle
Copy link
Contributor Author

I think this might also play into #47.

@geoand
Copy link
Collaborator

geoand commented Dec 5, 2023

Yeah, that's a more general thing we need to address.

geoand added a commit that referenced this issue Dec 5, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Fixes: #95
@geoand
Copy link
Collaborator

geoand commented Dec 5, 2023

#96 should take care of it

geoand added a commit that referenced this issue Dec 5, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Fixes: #95
@geoand geoand self-assigned this Dec 5, 2023
geoand added a commit that referenced this issue Dec 6, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Furthermore, add a way to remove memory entries when the service goes out of scope

Fixes: #95
geoand added a commit that referenced this issue Dec 6, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Furthermore, add a way to remove memory entries when the service goes out of scope

Fixes: #95
geoand added a commit that referenced this issue Dec 6, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Furthermore, add a way to remove memory entries when the service goes out of scope

Fixes: #95
geoand added a commit that referenced this issue Dec 7, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Furthermore, add a way to remove memory entries when the service goes out of scope

Fixes: #95
geoand added a commit that referenced this issue Dec 7, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Furthermore, add a way to remove memory entries when the service goes out of scope

Fixes: #95
geoand added a commit that referenced this issue Dec 7, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Furthermore, add a way to remove memory entries when the service goes out of scope

Fixes: #95
geoand added a commit that referenced this issue Dec 7, 2023
This is done because otherwise the chat memory
does not get cleared properly.

Furthermore, add a way to remove memory entries when the service goes out of scope

Fixes: #95
@geoand geoand closed this as completed in #96 Dec 7, 2023
geoand added a commit that referenced this issue Dec 7, 2023
Make @RegisterAiService beans request scoped by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants