Cancel in duplicate resolver incorrectly keeps the left entry

[geritwagner]

PR Description

Fixes the duplicate resolver so that clicking Cancel returns BREAK instead of KEEP_LEFT. This stops duplicate resolution without removing either entry from the current pair.

Steps to test

AI usage

---

Checklist

• I own the copyright of the code submitted and I license it under the MIT license
• [/] If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
• 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

[github-actions]

Hey @geritwagner! 👋

Thank you for contributing to JabRef!

We have automated checks in place, based on which you will soon get feedback if any of them are failing. We also use Qodo for review assistance. It will update your pull request description with a review help and offer suggestions to improve the pull request.

After all automated checks pass, a maintainer will also review your contribution. Once that happens, you can go through their comments in the "Files changed" tab and act on them, or reply to the conversation if you have further inputs. You can read about the whole pull request process in our contribution guide.

Please ensure that your pull request is in line with our AI Usage Policy and make necessary disclosures.

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

PR Summary by Qodo

Fix Cancel action in duplicate resolver to stop resolution (BREAK)

🐞 Bug fix 🕐 Less than 10 minutes

AI Description

• Make Duplicate Resolver **Cancel** return BREAK instead of KEEP_LEFT.
• Prevent accidental retention/removal behavior when users abort duplicate resolution.
• Ensure current duplicate pair remains unchanged when cancelling.

Diagram

graph TD
  U(("User")) --> D["DuplicateResolverDialog"] --> R["Result: BREAK"] --> F["Duplicate resolution flow"]

  subgraph Legend
    direction LR
    _actor(("Actor")) ~~~ _ui["UI component"] ~~~ _res["Result enum"] ~~~ _flow["Workflow"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Bind results via button userData/action handlers

• ➕ Removes brittle button.equals(...) chain
• ➕ Makes each button’s outcome explicit and localized
• ➕ Reduces risk of future mis-mapping when UI changes
• ➖ Slightly more wiring in dialog initialization
• ➖ May require minor refactor beyond the minimal fix

2. Add a focused UI/controller regression test for Cancel behavior

• ➕ Prevents recurrence of Cancel mapping regressions
• ➕ Documents expected semantics: cancel stops without selecting either entry
• ➖ May require test harness/mocking for JavaFX dialogs
• ➖ More effort than the one-line fix

Recommendation: Current one-line fix is appropriate for restoring correct behavior quickly. Consider a follow-up to bind results directly to button metadata/handlers and/or add a small regression test to ensure Cancel always maps to BREAK.

Files changed (1) +1 / -1

Bug fix (1) +1 / -1

DuplicateResolverDialog.javaReturn BREAK when Cancel is pressed +1/-1

Return BREAK when Cancel is pressed

• Changes the Cancel button handling to return 'DuplicateResolverResult.BREAK' instead of 'KEEP_LEFT'. This ensures cancelling stops duplicate resolution without implicitly choosing the left entry.

jabgui/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java

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

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Cancel doesn't cancel scan ✓ Resolved 🐞 Bug ➹ Performance

Description

Cancel now returns DuplicateResolverResult.BREAK, but in DuplicateSearch the BREAK handling only
clears the queue and flips libraryAnalyzed without stopping the background
searchPossibleDuplicates task. This can keep the O(n²) duplicate scan running after the user
cancels, wasting CPU and continuing to enqueue duplicates until the scan completes.

Code

jabgui/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java[R150-154]

       } else if (button.equals(removeExact)) {
           return DuplicateResolverResult.AUTOREMOVE_EXACT;
       } else if (button.equals(cancel)) {
-                return DuplicateResolverResult.KEEP_LEFT;
+                return DuplicateResolverResult.BREAK;
       }

Evidence

The dialog now maps Cancel to BREAK, and BREAK is handled in DuplicateSearch by clearing the
consumer queue only. The producer (searchPossibleDuplicates) never checks libraryAnalyzed/a
cancel flag and will keep adding duplicates; it only stops if interrupted, but the scan is launched
via executeInterruptableTask which does not return a Future to cancel.

jabgui/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java[137-156]
jabgui/src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java[95-120]
jabgui/src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java[183-197]
jablib/src/main/java/org/jabref/logic/util/HeadlessExecutorService.java[99-101]

Agent prompt

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

## Issue description
Clicking **Cancel** now returns `DuplicateResolverResult.BREAK`, which stops UI-driven resolution, but the background producer (`searchPossibleDuplicates`) is not cancelled and can continue scanning and enqueueing duplicates.
## Issue Context
- `DuplicateSearch` starts the scan via `HeadlessExecutorService.INSTANCE.executeInterruptableTask(...)`, which currently provides no handle to cancel.
- BREAK handling clears the queue, but the producer loop does not observe any cancel flag (it only checks `Thread.interrupted()`).
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java[150-154]
- jabgui/src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java[95-120]
- jabgui/src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java[183-197]
- jablib/src/main/java/org/jabref/logic/util/HeadlessExecutorService.java[99-101]
## Suggested approach
- Change duplicate scan execution to return a `Future<?>` (e.g., use `submit(...)`) and store it in `DuplicateSearch`.
- When resolver result is `BREAK`, call `future.cancel(true)` (and/or set an explicit `AtomicBoolean cancelled` checked inside `searchPossibleDuplicates`).
- Update `searchPossibleDuplicates` to exit early when cancellation is requested (in addition to `Thread.interrupted()`).

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

---

[koppor]

Think, this looks good

[Siedlerchr]

@geritwagner Looks good to me overall, can you comment on the qodo comment? Is that a valid concern?

[geritwagner]

@geritwagner Looks good to me overall, can you comment on the qodo comment? Is that a valid concern?

I believe runtime is a valid concern in the deduplication procedure. In my view, fully separating duplicate search from duplicate resolution should be preferred (see #15190). Once this is implemented, the cancel option in duplicate resolution should no longer affect the runtime of duplicate search. More broadly, a practical solution to the O(n²) runtime issue would be to introduce a blocking stage (see 1).

[Siedlerchr]

@geritwagner That is indeed a point I have thought about as well in trying to optimize the runtime of the duplicate search. I now implemented at least the task cancelling here a well.

I created an internal issue for tackling the runtime problem

[geritwagner]

The blocking procedures of ASySD and BibDedupe may be a suitable starting point for that.