Fix chat scope to use BibDatabaseContext ID

[Ranjeet2702]

Summary

Fixes #14641

The AI Chat feature was using global group names causing
incorrect chat history when multiple libraries shared the
same group name.

Changes Made

• Removed final from bibDatabaseContext and aiChatLogic
• Added name and chatHistory as instance fields
• Added updateDatabase() method to re-instantiate
  AiChatLogic when library is switched

Note

I am a GSoC 2026 applicant. This is a draft PR for feedback.

Checklist

• I own the copyright of the code submitted and I license it under the MIT license
• I manually tested my changes in running JabRef
• I added JUnit tests for changes
• [/] I added screenshots (not applicable - internal logic change)
• I described the change in CHANGELOG.md
• [/] I checked the user documentation (not applicable)

[jabref-machine]

You have removed the section "Checklist" from your pull request description. Please adhere to our pull request template.

[koppor]

Why class variable?

[Ranjeet2702]

chatHistory needs to be a class variable because updateDatabase() method needs to update it when the database context changes. Since the method is called from outside the component, the list needs to persist at the class level.

[koppor]

Why other style?

[Ranjeet2702]

Sorry for the inconsistent formatting, I'll fix it to match the existing code style.

[Ranjeet2702]

I fix now

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 790caee
▶️ Tests: 9821 executed
⚪️ Checks: 58/58 completed

---

Learn more about TestLens at testlens.app.

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix AI chat scope to use BibDatabaseContext ID

🐞 Bug fix

Walkthroughs

Description

• Fixes chat scope to use BibDatabaseContext ID instead of global group names
• Removes final modifiers from bibDatabaseContext and aiChatLogic fields
• Adds instance fields for name and chatHistory to preserve state
• Introduces updateDatabase() method to re-instantiate AiChatLogic on library switch

Diagram

flowchart LR
  A["Chat Component"] -->|stores| B["name & chatHistory"]
  A -->|updates on switch| C["updateDatabase method"]
  C -->|creates new| D["AiChatLogic instance"]
  D -->|scoped to| E["BibDatabaseContext ID"]

Loading

File Changes

1. jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java 🐞 Bug fix +14/-4

Make chat logic scoped to BibDatabaseContext

• Changed bibDatabaseContext and aiChatLogic from final to mutable fields
• Added name and chatHistory as instance fields to preserve state across library switches
• Added updateDatabase() method to re-instantiate AiChatLogic with new context and chat history
• Updated constructor to initialize name and chatHistory fields

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. updateDatabase() lacks test coverage 📘 Rule violation ☼ Reliability

Description

A new public method changes chat scoping behavior by re-instantiating AiChatLogic and swapping
chat history, but no corresponding test was added/updated to prevent regressions. This risks
incorrect chat history when switching libraries going unnoticed.

Code

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[R129-135]

+    public void updateDatabase(BibDatabaseContext newBibDatabaseContext,
+                               ObservableList<ChatMessage> newChatHistory) {
+        this.bibDatabaseContext = newBibDatabaseContext;
+        this.chatHistory = newChatHistory;
+        this.aiChatLogic = aiService.getAiChatService().makeChat(name, newChatHistory, entries, newBibDatabaseContext);
+        uiChatHistory.setItems(aiChatLogic.getChatHistory());
+    }

Evidence

PR Compliance ID 17 requires tests to be updated for behavior changes. This PR introduces
updateDatabase(...) which changes runtime behavior, while the existing AiChatComponentTest only
asserts notice text updates and contains no assertions for updateDatabase behavior.

AGENTS.md
jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-135]
jabgui/src/test/java/org/jabref/gui/ai/components/aichat/AiChatComponentTest.java[116-152]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AiChatComponent.updateDatabase(...)` introduces new behavior (recreate chat + switch displayed chat history) but there are no tests asserting the expected behavior.
## Issue Context
There is already a `AiChatComponentTest`, but it only verifies `computeNoticeText()` behavior when provider/model changes.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-135]
- jabgui/src/test/java/org/jabref/gui/ai/components/aichat/AiChatComponentTest.java[94-152]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. ChatHistory setItems called twice ☑ 🐞 Bug ≡ Correctness

Description

AiChatComponent.updateDatabase() calls uiChatHistory.setItems(...) even though
ChatHistoryComponent#setItems is explicitly designed to be called only once; this accumulates
listeners and can cause duplicated refresh work and leaks. initialize() already calls setItems
once, so every database switch triggers the misuse.

Code

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[R129-135]

+    public void updateDatabase(BibDatabaseContext newBibDatabaseContext,
+                               ObservableList<ChatMessage> newChatHistory) {
+        this.bibDatabaseContext = newBibDatabaseContext;
+        this.chatHistory = newChatHistory;
+        this.aiChatLogic = aiService.getAiChatService().makeChat(name, newChatHistory, entries, newBibDatabaseContext);
+        uiChatHistory.setItems(aiChatLogic.getChatHistory());
+    }

Evidence

updateDatabase() calls uiChatHistory.setItems(...) after a previous initialize() call already
set it; the target component documents a one-time contract and adds a new ListChangeListener every
time setItems is invoked.

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-135]
jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[137-147]
jabgui/src/main/java/org/jabref/gui/ai/components/aichat/chathistory/ChatHistoryComponent.java[30-34]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AiChatComponent.updateDatabase()` calls `uiChatHistory.setItems(...)` again, but `ChatHistoryComponent#setItems` is documented/implemented as a one-time setup and adds a new list listener each call.
### Issue Context
- `AiChatComponent.initialize()` already calls `uiChatHistory.setItems(aiChatLogic.getChatHistory())`.
- `ChatHistoryComponent#setItems` says it must be called only once and unconditionally does `items.addListener(...)`.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-147]
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/chathistory/ChatHistoryComponent.java[30-45]
### Suggested fix
Implement a safe rebind mechanism:
1) Either **change `ChatHistoryComponent`** to support replacing the backing list (store the current list + listener, remove listener from old list before attaching to the new one; update the Javadoc accordingly).
2) Or **avoid changing the list reference**: keep a single ObservableList bound to the UI and update its contents on database switch (only valid if persistence expectations allow it).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Stale bindings after logic swap 🐞 Bug ≡ Correctness

Description

updateDatabase() replaces aiChatLogic but does not update the existing exportButton binding
and follow-up-questions listener, leaving UI state wired to the previous AiChatLogic instance.
After a database switch, export enablement and follow-up question updates can stop reflecting the
new chat session.

Code

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[R129-135]

+    public void updateDatabase(BibDatabaseContext newBibDatabaseContext,
+                               ObservableList<ChatMessage> newChatHistory) {
+        this.bibDatabaseContext = newBibDatabaseContext;
+        this.chatHistory = newChatHistory;
+        this.aiChatLogic = aiService.getAiChatService().makeChat(name, newChatHistory, entries, newBibDatabaseContext);
+        uiChatHistory.setItems(aiChatLogic.getChatHistory());
+    }

Evidence

initialize() binds exportButton.disableProperty() to the initial aiChatLogic.getChatHistory()
and registers a listener on the initial aiChatLogic.getFollowUpQuestions(). updateDatabase()
only overwrites aiChatLogic and updates the chat history view, so the binding/listener remain
attached to the old logic’s lists.

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-135]
jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[137-147]
jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[247-251]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
After `updateDatabase()` re-instantiates `aiChatLogic`, UI bindings/listeners created during `initialize()` still point to the old `AiChatLogic` instance.
### Issue Context
- `exportButton.disableProperty()` is bound once to `Bindings.isEmpty(aiChatLogic.getChatHistory())`.
- A follow-up-questions listener is added once to `aiChatLogic.getFollowUpQuestions()`.
- `updateDatabase()` swaps `aiChatLogic` but does not rewire those.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-147]
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[247-285]
### Suggested fix
In `updateDatabase()` (or a shared method used by both initialization and update):
- `exportButton.disableProperty().unbind();` then bind to the **new** `aiChatLogic.getChatHistory()`.
- Remove the old follow-up listener (store it in a field) and attach it to `aiChatLogic.getFollowUpQuestions()` of the new logic.
- Consider centralizing the (re)wiring so `initialize()` and `updateDatabase()` cannot drift.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. name field should be final ☑ 📘 Rule violation ⚙ Maintainability

Description

The new name field is assigned only in the constructor and never reassigned, but it was made
mutable, weakening immutability unnecessarily. Keeping it final better expresses intent and
reduces accidental reassignment risk.

Code

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[R79-81]

+    private StringProperty name;
+    private ObservableList<ChatMessage> chatHistory;
+    private AiChatLogic aiChatLogic;

Evidence

PR Compliance ID 6 requires preferring immutability. The field name is introduced as a non-final
instance field even though it is only set once in the constructor and used read-only afterwards
(including in updateDatabase).

AGENTS.md
jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[79-81]
jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[118-120]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `name` field is unnecessarily mutable.
## Issue Context
`name` is assigned in the constructor and is not reassigned (even when updating databases); only the `StringProperty` value may change.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[79-81]
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[99-123]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. AiChatLogic listener leak 🐞 Bug ☼ Reliability

Description

updateDatabase() creates a new AiChatLogic without disposing the old instance, but AiChatLogic
registers listeners on entries and aiPreferences. Repeated database switches will retain old
AiChatLogic instances via those listeners, causing memory leaks and duplicated work.

Code

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[R131-134]

+        this.bibDatabaseContext = newBibDatabaseContext;
+        this.chatHistory = newChatHistory;
+        this.aiChatLogic = aiService.getAiChatService().makeChat(name, newChatHistory, entries, newBibDatabaseContext);
+        uiChatHistory.setItems(aiChatLogic.getChatHistory());

Evidence

AiChatLogic attaches listeners to entries and aiPreferences during construction, and there is
no corresponding removal in AiChatComponent.updateDatabase() before replacing the aiChatLogic
reference. Those listeners can keep the old logic reachable as long as entries/aiPreferences
live.

jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-135]
jablib/src/main/java/org/jabref/logic/ai/chatting/AiChatLogic.java[84-97]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`updateDatabase()` replaces `aiChatLogic` without cleaning up the old one. `AiChatLogic` registers listeners in its constructor, so old instances can remain referenced and keep doing work.
### Issue Context
`AiChatLogic` adds:
- an `entries` list listener
- preference listeners (template/context window size)
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-135]
- jablib/src/main/java/org/jabref/logic/ai/chatting/AiChatLogic.java[84-103]
### Suggested fix
- Introduce a `dispose()`/`close()` method on `AiChatLogic` (or make it `AutoCloseable`) that removes all listeners it registered.
- Store listener instances in fields (so they can be removed later).
- In `AiChatComponent.updateDatabase()`, call `oldAiChatLogic.dispose()` before overwriting the field.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-free-for-open-source-projects]

1. updatedatabase() lacks test coverage 📘 Rule violation ☼ Reliability

A new public method changes chat scoping behavior by re-instantiating AiChatLogic and swapping
chat history, but no corresponding test was added/updated to prevent regressions. This risks
incorrect chat history when switching libraries going unnoticed.

Agent Prompt

## Issue description
`AiChatComponent.updateDatabase(...)` introduces new behavior (recreate chat + switch displayed chat history) but there are no tests asserting the expected behavior.

## Issue Context
There is already a `AiChatComponentTest`, but it only verifies `computeNoticeText()` behavior when provider/model changes.

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-135]
- jabgui/src/test/java/org/jabref/gui/ai/components/aichat/AiChatComponentTest.java[94-152]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

3. Stale bindings after logic swap 🐞 Bug ≡ Correctness

updateDatabase() replaces aiChatLogic but does not update the existing exportButton binding
and follow-up-questions listener, leaving UI state wired to the previous AiChatLogic instance.
After a database switch, export enablement and follow-up question updates can stop reflecting the
new chat session.

Agent Prompt

### Issue description
After `updateDatabase()` re-instantiates `aiChatLogic`, UI bindings/listeners created during `initialize()` still point to the old `AiChatLogic` instance.

### Issue Context
- `exportButton.disableProperty()` is bound once to `Bindings.isEmpty(aiChatLogic.getChatHistory())`.
- A follow-up-questions listener is added once to `aiChatLogic.getFollowUpQuestions()`.
- `updateDatabase()` swaps `aiChatLogic` but does not rewire those.

### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[129-147]
- jabgui/src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java[247-285]

### Suggested fix
In `updateDatabase()` (or a shared method used by both initialization and update):
- `exportButton.disableProperty().unbind();` then bind to the **new** `aiChatLogic.getChatHistory()`.
- Remove the old follow-up listener (store it in a field) and attach it to `aiChatLogic.getFollowUpQuestions()` of the new logic.
- Consider centralizing the (re)wiring so `initialize()` and `updateDatabase()` cannot drift.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[Ranjeet2702]

Hi @koppor, I have fixed the indentation in the updateDatabase() method to match the existing code style. All checks are passing now. Could you please re-review? Thank you!

[faneeshh]

I can still replicate the issue. You should look into ChatHistoryService rather than AiChatComponent. The groupsChatHistory TreeMap uses Comparator.comparing(GroupTreeNode::getName) as its key, which means groups are looked up by name alone with no awareness of which library they belong to. So the collision happens before AiChatComponent ever sees the data and updateDatabase( ) doesn't change that.

[wanling0000]

Same, can still reproduce the issue on my side.

The chat session is still shared across them when libraries have groups with the same name.

Attaching a screenshot for reference.

[Ranjeet2702]

Hi @koppor, @faneeshh pointed out that the real fix should be in ChatHistoryService.java — the groupsChatHistory TreeMap uses only group name as key. Should I modify the Comparator there to include library path/database context? Please guide me on the correct approach.

[koppor]

Hi @koppor, @faneeshh pointed out that the real fix should be in ChatHistoryService.java — the groupsChatHistory TreeMap uses only group name as key. Should I modify the Comparator there to include library path/database context? Please guide me on the correct approach.

I don't know about the architecture. Maybe @InAnYan can help.

Otherwise, the self guided approach would be to draw an UML sequence diagram covering that case.

[InAnYan]

Hi ranjeet and faneeshh! Thanks for looking into AI features and its code.

I admit, that during the development several design mistakes were made, which need a fix. But fixing this is not easy at first glance (this is why I'm preparing a big AI refactoring PR).

And yes the real culprit is in the TreeMap and how it handles chats with groups with the same name.

So, if you have ideas or fixes - you are welcome!

[Ranjeet2702]

Hi @InAnYan, thank you for the clarification! I will look into fixing the TreeMap comparator in ChatHistoryService.java to include library context. Looking forward to your refactoring PR as well!

[Ranjeet2702]

Hi @InAnYan, thank you for the clarification! Could you guide me on the correct approach to fix the TreeMap issue? I want to make sure my fix aligns with your upcoming refactoring PR.

[InAnYan]

I actually don’t know what to advise you 😅 . You can do anything which solves the problem and not too tricky.

and don’t worry about my refactoring PR, so no need to align anything

[github-actions]

Your pull request conflicts with the target branch.

Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line.

[Ranjeet2702]

Hi @koppor, my branch has too many commits due to rebase. Could you please squash the commits or should I create a new PR with only the relevant changes.

[InAnYan]

@Ranjeet2702 no need, it’s okay, your PR will be squash merged

[Ranjeet2702]

Hi @koppor, all conflicts have been resolved and all checks are passing. Could you please re-review the PR? Thank you

[koppor]

No changes not related to the PR please

[koppor]

Why method moved?

[subhramit]

Closing due to excessive AI use in code and description, and in communication.

[github-actions]

This pull requests was closed without merging. You have been unassigned from the respective issue #14641. In case you closed the PR for yourself, you can re-open it. Please also check After submission of a pull request in CONTRIBUTING.md.

[Ranjeet2702]

I apologize for using AI- generated code and comment in my PR. I am a 2nd second year student and had no idea at initial time but i wanted to contribute that's why i did . Please give me another chance this time you will not hear any complain from my side. Thanks for understanding.

[subhramit]

I apologize for using AI- generated code and comment in my PR. I am a 2nd second year student and had no idea at initial time but i wanted to contribute that's why i did . Please give me another chance this time you will not hear any complain from my side. Thanks for understanding.

Please go through our contributing guidelines. You are free to try again.