Skip to content

Create groups where paper from - SLR#15429

Open
LoayTarek5 wants to merge 15 commits intoJabRef:mainfrom
LoayTarek5:create-groups-where-paper-from-SLR-12542
Open

Create groups where paper from - SLR#15429
LoayTarek5 wants to merge 15 commits intoJabRef:mainfrom
LoayTarek5:create-groups-where-paper-from-SLR-12542

Conversation

@LoayTarek5
Copy link
Copy Markdown
Contributor

Related issues and pull requests

Closes #12542

PR Description

During SLR crawls, fetched entries are now automatically grouped by their source.
Group merging is also fixed to union-merge instead of replacing or deleting existing groups.

Steps to test

  1. Tools -> Start new systematic literature review
  2. Create a new SLR study with 2+ fetchers (arxiv + springer)
  3. Choose an empty folder, then run
  4. Groups panel should show one group per fetcher
  5. Select two entries from different fetchers -> right-click -> Merge entries
  6. The Groups field in the merged entry should contain both source groups
image image image image image

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
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add automatic source groups and union-merge for SLR results

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Automatic source groups created for SLR fetched entries by fetcher name
• Group merging now union-merges instead of replacing existing groups
• Keyword separator support added for group field merging
• Three-way merge auto-merges groups when both entries have values
Diagram
flowchart LR
  A["SLR Crawl Results"] --> B["Fetcher 1<br/>ArXiv"]
  A --> C["Fetcher 2<br/>Springer"]
  B --> D["Create Group<br/>by Fetcher Name"]
  C --> D
  D --> E["Tag Entries<br/>with Group"]
  E --> F["Persist to<br/>Result Files"]
  G["Merge Entries"] --> H["Union-Merge<br/>Groups Field"]
  H --> I["Preserve All<br/>Source Groups"]
Loading

Grey Divider

File Changes

1. jabgui/src/main/java/org/jabref/gui/mergeentries/BatchEntryMergeTask.java ✨ Enhancement +5/-2

Add keyword separator support to batch merge task

• Added keywordSeparator parameter to constructor and field
• Passed keywordSeparator to MergeEntriesHelper.mergeEntries() call

jabgui/src/main/java/org/jabref/gui/mergeentries/BatchEntryMergeTask.java


2. jabgui/src/main/java/org/jabref/gui/mergeentries/BatchEntryMergeWithFetchedDataAction.java ✨ Enhancement +2/-1

Pass keyword separator from preferences to merge task

• Extracted keyword separator from preferences
• Passed separator to BatchEntryMergeTask constructor

jabgui/src/main/java/org/jabref/gui/mergeentries/BatchEntryMergeWithFetchedDataAction.java


3. jabgui/src/main/java/org/jabref/gui/mergeentries/MergeEntriesHelper.java ✨ Enhancement +21/-4

Implement union-merge for groups field during entry merge

• Added keywordSeparator parameter to mergeEntries() method
• Implemented union-merge logic for GROUPS field using KeywordList.merge()
• Prevented removal of GROUPS field when not present in fetcher entry
• Added imports for KeywordList and StandardField

jabgui/src/main/java/org/jabref/gui/mergeentries/MergeEntriesHelper.java


View more (5)
4. jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/FieldRowViewModel.java ✨ Enhancement +8/-0

Auto-merge groups in three-way merge when both entries have values

• Added auto-merge logic for groups field when both entries have non-empty values
• Automatically calls mergeFields() if field can be merged and both sides differ

jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/FieldRowViewModel.java


5. jabgui/src/test/java/org/jabref/gui/mergeentries/MergeEntriesHelperTest.java 🧪 Tests +94/-0

Add unit tests for entry merge group handling

• Added 5 unit tests for group merging behavior
• Tests cover: groups preservation, union-merge, deduplication, addition, and field removal

jabgui/src/test/java/org/jabref/gui/mergeentries/MergeEntriesHelperTest.java


6. jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java ✨ Enhancement +54/-0

Implement automatic source group creation for SLR results

• Added addFetcherGroup() method to create and manage fetcher-named groups
• Groups are created for each fetcher and assigned to fetched entries
• Groups are added to fetcher result files, query result files, and final study result file
• Existing groups are reused instead of creating duplicates
• Added imports for group-related classes and collections

jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java


7. jablib/src/test/java/org/jabref/logic/crawler/StudyRepositoryTest.java 🧪 Tests +61/-13

Add tests for automatic group creation in SLR results

• Added 3 new test methods for group creation verification
• Tests verify groups created in fetcher result files and study result file
• Updated existing tests to expect entries with group assignments
• Added helper methods to create mock results with group fields

jablib/src/test/java/org/jabref/logic/crawler/StudyRepositoryTest.java


8. CHANGELOG.md 📝 Documentation +1/-0

Document SLR automatic source groups feature

• Added entry documenting automatic source groups for SLR results
• References issue #12542

CHANGELOG.md


Grey Divider

Qodo Logo

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

qodo-free-for-open-source-projects bot commented Mar 27, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Weak contains group assertions📘 Rule violation ⛯ Reliability
Description
MergeEntriesHelperTest asserts group membership using String.contains, which can pass with
incorrect formatting/order or partial matches. This weakens the test and may miss regressions in
group union-merging behavior.
Code

jabgui/src/test/java/org/jabref/gui/mergeentries/MergeEntriesHelperTest.java[R50-53]

+        String groups = entryFromLibrary.getField(StandardField.GROUPS).orElse("");
+        assertTrue(groups.contains("IEEE"), "IEEE group should be preserved");
+        assertTrue(groups.contains("Springer"), "Springer group should be added");
+    }
Evidence
Compliance requires tests to assert exact expected values/structures rather than weak predicate
checks. The added test uses substring checks (contains) instead of asserting the exact merged
group value or an exact parsed set of groups.

jabgui/src/test/java/org/jabref/gui/mergeentries/MergeEntriesHelperTest.java[50-53]
Best Practice: Learned patterns

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 test `groupsAreUnionMergedWhenBothEntriesHaveGroups()` uses `String.contains(...)` assertions, which are weak and may pass even if group serialization/merging is wrong.
## Issue Context
Compliance requires unit tests to assert exact expected values/structures rather than predicate/substring checks.
## Fix Focus Areas
- jabgui/src/test/java/org/jabref/gui/mergeentries/MergeEntriesHelperTest.java[50-53]

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


2. setField used in tests📘 Rule violation ⚙ Maintainability
Description
StudyRepositoryTest mutates BibEntry test data via setField(...) instead of using immutable
withField(...) withers. This violates the test construction convention and can make test data
setup harder to reason about.
Code

jablib/src/test/java/org/jabref/logic/crawler/StudyRepositoryTest.java[R327-342]

+    private List<BibEntry> getArXivQuantumMockResultsWithGroups() {
+        List<BibEntry> entries = getArXivQuantumMockResults();
+        entries.forEach(e -> e.setField(StandardField.GROUPS, "ArXiv"));
+        return entries;
+    }
+
+    private List<BibEntry> getSpringerQuantumMockResultsWithGroups() {
+        List<BibEntry> entries = getSpringerQuantumMockResults();
+        entries.forEach(e -> e.setField(StandardField.GROUPS, "Springer"));
+        return entries;
+    }
+
+    private List<BibEntry> getSpringerCloudComputingMockResultsWithGroups() {
+        List<BibEntry> entries = getSpringerCloudComputingMockResults();
+        entries.forEach(e -> e.setField(StandardField.GROUPS, "Springer"));
+        return entries;
Evidence
The checklist requires using BibEntry.withField(...) instead of setField(...) in tests. The
newly added helper methods apply setField across entries to add StandardField.GROUPS.

AGENTS.md
jablib/src/test/java/org/jabref/logic/crawler/StudyRepositoryTest.java[327-342]

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

## Issue description
New test helpers in `StudyRepositoryTest` use `BibEntry#setField(...)` to add groups, contrary to the convention of using immutable `withField(...)`.
## Issue Context
JabRef test conventions prefer the functional `withField(...)` style for constructing/modifying `BibEntry` instances.
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/crawler/StudyRepositoryTest.java[327-342]

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


3. Unexpected auto-merge fields🐞 Bug ✓ Correctness
Description
FieldRowViewModel auto-invokes mergeFields() for any field where FieldMergerFactory.canMerge(field)
is true, which includes KEYWORDS/COMMENT/FILE, not just GROUPS. This changes the default merge
outcome by union-merging these fields whenever both sides are non-empty and different, even if the
user did not request that merge.
Code

jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/FieldRowViewModel.java[R92-98]

+        // Auto union merge groups so no source group is lost when both entries have groups
+        if (FieldMergerFactory.canMerge(field)
+                && !hasEqualLeftAndRightValues()
+                && !StringUtil.isNullOrEmpty(getLeftFieldValue())
+                && !StringUtil.isNullOrEmpty(getRightFieldValue())) {
+            mergeFields();
+        }
Evidence
The new constructor block calls mergeFields() when FieldMergerFactory.canMerge(field) is true and
both values are present; canMerge() explicitly returns true for GROUPS, KEYWORDS, COMMENT, and FILE.
mergeFields() then overwrites both left and right values with the merged value, thereby changing the
default merge content for those fields.

jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/FieldRowViewModel.java[90-99]
jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FieldMergerFactory.java[15-34]
jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/FieldRowViewModel.java[193-207]

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

## Issue description
`FieldRowViewModel` currently auto-merges any field supported by `FieldMergerFactory.canMerge` (GROUPS, KEYWORDS, COMMENT, FILE). This makes merges of keywords/comments/files happen automatically whenever both sides are non-empty and different, which changes the default merge result beyond the PR’s stated goal (preserving source groups).
### Issue Context
The comment says this is for groups, but the condition uses `FieldMergerFactory.canMerge(field)`.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/FieldRowViewModel.java[92-98]
- jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FieldMergerFactory.java[32-34]
### Suggested fix
Change the condition to only auto-merge when `field == StandardField.GROUPS` (or an equivalent explicit check), leaving other mergeable fields user-controlled.

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



Remediation recommended

4. Multi-source tags lost on dedup🐞 Bug ✓ Correctness
Description
During SLR persistence, DatabaseMerger drops duplicates without merging fields, and
addFetcherGroup() only tags the specific BibEntry instances passed in. If a fetcher returns an entry
considered duplicate of an entry already inserted from another fetcher, the surviving entry in the
merged database will not receive the second fetcher’s GROUPS tag, so it can appear in only one
source group.
Code

jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java[R378-385]

+                // tag entries + add group to per-fetcher .bib
+                addFetcherGroup(existingFetcherResult, fetcherResult.getFetcherName(), fetcherEntries.getEntries());
+                fetcherEntryMap.computeIfAbsent(fetcherResult.getFetcherName(), k -> new ArrayList<>())
+                               .addAll(fetcherEntries.getEntries());
+
             // Aggregate each fetcher result into the query result
             merger.merge(queryResultEntries, fetcherEntries);
Evidence
StudyRepository records per-fetcher entries (including potential duplicates) and later calls
addFetcherGroup(context, fetcherName, entries). addFetcherGroup ultimately uses WordKeywordGroup.add
(via ExplicitGroup) which mutates only the entries passed in. But DatabaseMerger.mergeEntries
filters out duplicates and does not merge any fields into the already-present entry, so the retained
entry never gets the additional GROUPS value from the discarded duplicate instance.

jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java[361-405]
jablib/src/main/java/org/jabref/logic/database/DatabaseMerger.java[52-59]
jablib/src/main/java/org/jabref/model/groups/WordKeywordGroup.java[69-80]
jablib/src/test/java/org/jabref/logic/crawler/StudyRepositoryTest.java[290-310]

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

## Issue description
When a fetcher entry is identified as a duplicate during merging, it is dropped, and its GROUPS/source tag is never applied to the already-present (retained) entry. This can cause entries found by multiple fetchers to only appear in the first fetcher’s group.
### Issue Context
`DatabaseMerger.mergeEntries` filters duplicates and inserts only non-duplicates. Group membership for explicit groups is stored in the entry field (`StandardField.GROUPS`), and `ExplicitGroup.add(...)` mutates only the `BibEntry` instances it is given.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java[361-405]
- jablib/src/main/java/org/jabref/logic/database/DatabaseMerger.java[52-59]
- jablib/src/main/java/org/jabref/model/groups/WordKeywordGroup.java[69-80]
### Suggested fix
When a duplicate is detected, locate the existing entry in the target database (e.g., via the same duplicate check used for filtering) and union-merge/append the fetcher group into that existing entry’s `GROUPS` field (or add that existing entry to the fetcher’s ExplicitGroup). Ensure this happens for per-query and/or final study result aggregation paths.

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


5. Hard-coded root group name 🐞 Bug ⚙ Maintainability
Description
StudyRepository creates a new AllEntriesGroup root with the hard-coded name "All Entries" instead of
using the standard localized factory, causing inconsistent UI text and missing default icon
settings. This affects any new SLR result file where no group tree existed previously.
Code

jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java[R451-455]

+            // Get existing root node or create a new AllEntriesGroup root if none exists yet
+            GroupTreeNode root = context.getMetaData().getGroups().orElseGet(() -> {
+                GroupTreeNode newRoot = GroupTreeNode.fromGroup(new AllEntriesGroup("All Entries"));
+                context.getMetaData().setGroups(newRoot);
+                return newRoot;
Evidence
StudyRepository instantiates new AllEntriesGroup("All Entries") directly. Elsewhere,
GroupsFactory.createAllEntriesGroup() constructs the All Entries group using localized text and
applies the default icon, so bypassing it creates inconsistency.

jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java[451-456]
jablib/src/main/java/org/jabref/logic/groups/GroupsFactory.java[18-22]

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 group root is created with a hard-coded English label and without the standard defaults (e.g., icon).
### Issue Context
There is already a canonical factory for the root group (`GroupsFactory.createAllEntriesGroup()`), which handles localization and default icon.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java[451-456]
- jablib/src/main/java/org/jabref/logic/groups/GroupsFactory.java[18-22]
### Suggested fix
Replace `new AllEntriesGroup("All Entries")` with `GroupsFactory.createAllEntriesGroup()` (and wrap it with `GroupTreeNode.fromGroup(...)`).

ⓘ 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

@testlens-app

This comment has been minimized.

@testlens-app

This comment has been minimized.

@testlens-app

This comment has been minimized.

@testlens-app
Copy link
Copy Markdown

testlens-app bot commented Mar 28, 2026

✅ All tests passed ✅

🏷️ Commit: b2ad00a
▶️ Tests: 10215 executed
⚪️ Checks: 50/50 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SLR should create groups where paper is from

2 participants