Skip to content

Invert condition for citation key uniqueness check#15533

Merged
calixtus merged 1 commit intomainfrom
InAnYan-patch-2
Apr 12, 2026
Merged

Invert condition for citation key uniqueness check#15533
calixtus merged 1 commit intomainfrom
InAnYan-patch-2

Conversation

@InAnYan
Copy link
Copy Markdown
Member

@InAnYan InAnYan commented Apr 12, 2026

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

@InAnYan InAnYan added the dev: no-bot-comments If set, there should be no comments from our bots label Apr 12, 2026
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix citation key uniqueness check condition inversion

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Inverts condition for citation key uniqueness check
• Fixes logic error preventing summary storage with valid keys
Diagram
flowchart LR
  A["Check citation key validity"] -->|Before: incorrect logic| B["Skip storage if key IS valid"]
  A -->|After: corrected logic| C["Skip storage if key NOT valid"]
  C --> D["Store summary correctly"]
Loading

Grey Divider

File Changes

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

Invert citation key uniqueness validation condition

• Inverted boolean condition in citation key validation check
• Changed from CitationKeyCheck.citationKeyIsPresentAndUnique() to
 !CitationKeyCheck.citationKeyIsPresentAndUnique()
• Fixes logic error where summaries were skipped when keys were valid instead of when invalid

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


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 12, 2026

Code Review by Qodo

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

Grey Divider


Action required

1. Regenerate returns cached summary 🐞
Description
With this change, GenerateSummaryTask will persist summaries when the citation key is present and
unique, but SummariesService.regenerateSummary uses an inverted uniqueness check and therefore skips
clearing stored summaries for valid citation keys. As a result, invoking “Regenerate” can
immediately return the previously stored summary instead of recomputing it.
Code

jablib/src/main/java/org/jabref/logic/ai/summarization/GenerateSummaryTask.java[R123-128]

        if (bibDatabaseContext.getDatabasePath().isEmpty()) {
            LOGGER.info("No database path is present. Summary will not be stored in the next sessions");
-        } else if (CitationKeyCheck.citationKeyIsPresentAndUnique(bibDatabaseContext, entry)) {
+        } else if (!CitationKeyCheck.citationKeyIsPresentAndUnique(bibDatabaseContext, entry)) {
            LOGGER.info("No valid citation key is present. Summary will not be stored in the next sessions");
        } else {
            summariesStorage.set(bibDatabaseContext.getDatabasePath().get(), entry.getCitationKey().get(), summary);
Evidence
GenerateSummaryTask first tries to load an existing stored summary by (databasePath, citationKey)
and returns it when present, and after this PR it will also store summaries when CitationKeyCheck
reports the key is present+unique. However, SummariesService.regenerateSummary currently logs “No
valid citation key” and does NOT clear the stored summary when the key is present+unique (it checks
... || citationKeyIsPresentAndUnique(...)), so the next GenerateSummaryTask run will read the old
stored summary and skip regeneration. The GUI’s “Regenerate” action calls
SummariesService.regenerateSummary, making this user-visible.

jablib/src/main/java/org/jabref/logic/ai/summarization/GenerateSummaryTask.java[93-107]
jablib/src/main/java/org/jabref/logic/ai/summarization/GenerateSummaryTask.java[123-129]
jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java[148-162]
jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryComponent.java[142-153]

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(...)` currently refuses to clear stored summaries when the citation key is valid (present and unique) due to an inverted condition. After this PR enables storing summaries for valid citation keys, clicking “Regenerate” can become a no-op because `GenerateSummaryTask` will load and return the previously stored summary.

### Issue Context
- `GenerateSummaryTask` reads from storage first and uses the stored summary when present.
- The Summary UI triggers regeneration via `SummariesService.regenerateSummary(...)`.

### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/ai/summarization/SummariesService.java[149-159]
- (optional) add a regression test covering regenerate-clears-then-recomputes behavior (new test file near existing summarization tests)

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


Grey Divider

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

Grey Divider

Qodo Logo

@calixtus calixtus added this pull request to the merge queue Apr 12, 2026
@github-actions github-actions bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Apr 12, 2026
Merged via the queue into main with commit 7ec5e96 Apr 12, 2026
70 of 78 checks passed
@calixtus calixtus deleted the InAnYan-patch-2 branch April 12, 2026 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev: no-bot-comments If set, there should be no comments from our bots status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants