Fix citation key check logic in summary regeneration

[InAnYan]

Related issues and pull requests

Closes _____

PR Description

Steps to test

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 (always required)
• [.] I added JUnit tests for changes (if applicable)
• [.] I added screenshots in the PR description (if change is visible to the user)
• [.] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
• [.] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
• [.] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

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

Review Summary by Qodo

Fix citation key validation logic in summary regeneration

🐞 Bug fix

Walkthroughs

Description

• Fix inverted logic in citation key validation check
• Add missing negation operator to condition evaluation
• Ensure summary regeneration only proceeds with valid citation keys

Diagram

flowchart LR
  A["Citation Key Check"] --> B{"isEmpty OR<br/>NOT isUnique?"}
  B -->|True| C["Skip Summary Clear<br/>Invalid Key"]
  B -->|False| D["Clear Stored Summary<br/>Valid Key"]

Loading

File Changes

1. jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java 🐞 Bug fix +1/-1

Add negation operator to citation key uniqueness check

• Fixed inverted boolean logic in citation key validation condition
• Added missing negation operator (!) before CitationKeyCheck.citationKeyIsPresentAndUnique()
 call
• Corrects condition to properly skip summary clearing when citation key is not unique
• Ensures summary regeneration only proceeds with valid and unique citation keys

jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java

---

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

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (1)   📎 Requirement gaps (0)   🖥 UI issues (0)   🎨 UX Issues (0)

🐞\ ≡ Correctness (1)

📘\ ☼ Reliability (1)

1. regenerateSummary missing unit tests 📘 ☼

Description

The PR changes the citation-key validity logic inside SummariesService.regenerateSummary, but
there are no corresponding unit tests covering this behavior to prevent regressions. Changes in
org.jabref.logic are required to be accompanied by updated/adapted tests for the changed behavior.

Code

jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java[R155-156]

+        } else if (bibEntry.getCitationKey().isEmpty() || !CitationKeyCheck.citationKeyIsPresentAndUnique(bibDatabaseContext, bibEntry)) {
            LOGGER.info("No valid citation key is present. Could not clear stored summary for regeneration");

Evidence

The compliance checklist requires tests to be added/updated when behavior in org.jabref.logic
changes. The diff shows a behavior change in SummariesService.regenerateSummary (citation key
validation condition), while the existing tests in the same package cover only storage behavior (no
tests for regenerateSummary).

AGENTS.md
AGENTS.md
jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java[149-162]
jablib/src/test/java/org/jabref/logic/ai/summarization/SummariesStorageTest.java[42-56]

Agent prompt

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

## Issue description
`SummariesService.regenerateSummary(...)` behavior was changed (citation-key validity condition), but there are no unit tests asserting the expected behavior for empty/invalid/non-unique citation keys.

## Issue Context
This is a logic-layer behavior change in `org.jabref.logic`, so tests should be added/updated to prevent regressions.

## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java[149-162]
- jablib/src/test/java/org/jabref/logic/ai/summarization/SummariesServiceTest.java[1-200]
- jablib/src/test/java/org/jabref/logic/ai/summarization/SummariesStorageTest.java[16-56]

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

---

2. Storage validity check inverted 🐞 ≡

Description

GenerateSummaryTask currently skips persisting summaries when citationKeyIsPresentAndUnique(...) is
true and persists them otherwise, which can overwrite summaries across entries sharing a key and
prevents persistence for valid keys. With this PR fixing regeneration’s validity predicate,
regeneration/clearing and persistence now disagree and summary storage remains incorrect.

Code

jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java[R155-158]

+        } else if (bibEntry.getCitationKey().isEmpty() || !CitationKeyCheck.citationKeyIsPresentAndUnique(bibDatabaseContext, bibEntry)) {
            LOGGER.info("No valid citation key is present. Could not clear stored summary for regeneration");
        } else {
            summariesStorage.clear(bibDatabaseContext.getDatabasePath().get(), bibEntry.getCitationKey().get());

Evidence

SummariesService documents that storage requires a set and unique citation key, and CitationKeyCheck
returns true exactly for that case; yet GenerateSummaryTask logs “No valid citation key” when the
predicate is true and only stores in the else-branch. Since MVStoreSummariesStorage uses citationKey
as the map key, storing for non-unique keys can overwrite other entries’ summaries.

jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java[32-36]
jablib/src/main/java/org/jabref/logic/ai/util/CitationKeyCheck.java[12-22]
jablib/src/main/java/org/jabref/logic/ai/summarization/GenerateSummaryTask.java[123-129]
jablib/src/main/java/org/jabref/logic/ai/summarization/storages/MVStoreSummariesStorage.java[20-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
`GenerateSummaryTask` uses `CitationKeyCheck.citationKeyIsPresentAndUnique(...)` with inverted meaning when deciding whether to persist a summary, resulting in persistence for invalid/duplicate citation keys and skipping persistence for valid keys.

### Issue Context
This PR corrects the validity predicate in `SummariesService.regenerateSummary()` (clearing only when the citation key is valid). Persistence should follow the same definition of “valid citation key” to avoid overwrites and inconsistent behavior.

### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/ai/summarization/GenerateSummaryTask.java[123-129]
- jablib/src/main/java/org/jabref/logic/ai/util/CitationKeyCheck.java[12-22]
- jablib/src/main/java/org/jabref/logic/ai/summarization/storages/MVStoreSummariesStorage.java[20-34]

### What to change
- Flip the conditional in `GenerateSummaryTask` to log/skip storing when `!CitationKeyCheck.citationKeyIsPresentAndUnique(...)`, and store only when it returns `true`.
- Ensure the log message matches the branch condition.
- (Optional but recommended) consider also applying the same validity check when loading a saved summary to avoid returning a summary for a duplicate key.

ⓘ 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