Skip to content

Fix group filter to use OR semantics for space separated terms#15350

Merged
calixtus merged 35 commits intoJabRef:mainfrom
NishantDG-SST:fix-group-filter-or-semantics-12721
Apr 5, 2026
Merged

Fix group filter to use OR semantics for space separated terms#15350
calixtus merged 35 commits intoJabRef:mainfrom
NishantDG-SST:fix-group-filter-or-semantics-12721

Conversation

@NishantDG-SST
Copy link
Copy Markdown
Contributor

@NishantDG-SST NishantDG-SST commented Mar 16, 2026

Related issues and pull requests

Closes #12721

PR Description

The group filter in the side pane previously required ALL typed words to appear in a group name, so typing machine learning would hide "Deep Learning" even though it contains "learning". This fix changes the behavior to OR-any matching word is enough. It reuses the existing Search.g4 ANTLR grammar for parsing So explicit AND, OR, NOT, and parentheses also work in the filter box.

Steps to test

  1. Open any library in JabRef
  2. Open the Groups pane (View → Groups)
  3. Create these subgroups under "All Entries": "Machine Learning", "Deep Learning", "Neural Networks", "Computer Vision", "Reinforcement Learning"
Screenshot 2026-03-17 at 12 25 41 AM
  1. Type "deep learning" in the Filter groups box → all three groups containing "Learning" should appear

Before
Screenshot 2026-03-17 at 12 24 26 AM

After
Screenshot 2026-03-17 at 12 47 08 AM

  1. Type "machine vision" → "Machine Learning" and "Computer Vision" should appear

Before

Screenshot 2026-03-17 at 12 25 35 AM

After

Screenshot 2026-03-17 at 12 47 34 AM
  1. Type "NOT learning" → subgroups with "Learning" should not appear
Screenshot 2026-03-17 at 2 52 24 AM
  1. Type "machine AND learning" → only "Machine Learning" should appear
Screenshot 2026-03-17 at 12 52 25 AM

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
Screenshot 2026-03-17 at 2 47 51 AM

AI Usage

I used copilot suggesstion code (#15350 (comment)) in GroupNameFilterVisitor.java, I reviewed the suggested fix, tested it locally, and verified it solves the issue without side effects.
I used ai to write the steps to reciprocate the steps to test more clearly in pr description
I used Copilot for suggestions related to spaceImpliesOr,parenthesesWithNotWork,complexExpressionWithNotLearning test cases. I reviewed the suggested cases, and no code was generated by AI.

When filtering groups in the side pane, typing multiple space-separated
words (e.g. 'machine learning') now shows groups containing ANY of the
words instead of requiring ALL words to be present.

Implemented by reusing the existing Search.g4 ANTLR grammar. A new
GroupNameFilterVisitor evaluates the parse tree against the group name
string directly, without Lucene. The key change is using anyMatch (OR)
instead of allMatch (AND) for implicit space-separated bare terms.
Complex expressions with parentheses and NOT continue to use AND
between top-level parts.

Explicit AND, OR, NOT and parentheses continue to work as expected.

Closes JabRef#12721
Copilot AI review requested due to automatic review settings March 16, 2026 21:34
@github-actions github-actions bot added the status: changes-required Pull requests that are not yet complete label Mar 16, 2026
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Implement OR semantics for group filter with advanced query support

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Implement OR semantics for space-separated group filter terms
• Reuse existing Search.g4 ANTLR grammar for advanced query support
• Support explicit AND, OR, NOT operators and parentheses
• Add comprehensive test coverage for filter behavior
Diagram
flowchart LR
  A["Group Filter Input"] --> B["SearchQuery Parser"]
  B --> C["GroupNameFilterVisitor"]
  C --> D["OR for space-separated terms"]
  C --> E["AND/OR/NOT operators"]
  C --> F["Parentheses support"]
  D --> G["Match Result"]
  E --> G
  F --> G
Loading

Grey Divider

File Changes

1. jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java ✨ Enhancement +14/-2

Integrate ANTLR-based group filter with OR semantics

• Import new GroupNameFilterVisitor and SearchQuery classes
• Replace simple containsIgnoreCase logic with ANTLR-based query parsing
• Implement fallback to plain contains matching for malformed queries
• Remove trailing comment from acceptableDrop method documentation

jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java


2. jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java 🧪 Tests +90/-0

Add comprehensive group filter matching tests

• Add 11 test cases covering space-separated OR semantics
• Test explicit AND, OR, NOT operators and parentheses
• Verify case-insensitive matching and blank query handling
• Test malformed query fallback and complex expressions

jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java


3. jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java ✨ Enhancement +73/-0

Create visitor for group name filter query evaluation

• Create new visitor class extending SearchBaseVisitor for parse tree evaluation
• Implement OR semantics in visitImplicitAndExpression using anyMatch for simple terms
• Support explicit AND/OR/NOT operators and parentheses expressions
• Use case-insensitive substring matching against group display name

jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java


View more (1)
4. CHANGELOG.md 📝 Documentation +1/-0

Document group filter OR semantics fix

• Document fix for group filter OR semantics behavior
• Reference issue #12721 in changelog entry

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 16, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Action required

1. Javadoc block in GroupNameFilterVisitor📘 Rule violation ≡ Correctness
Description
The new GroupNameFilterVisitor introduces a multi-line Javadoc-style block comment (/** ... */)
instead of the preferred Markdown Javadoc triple-slash (///) format. This creates inconsistent
documentation style compared to the project’s stated convention.
Code

jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java[R6-12]

+/**
+ * Evaluates a Search.g4 parse tree against a group display name string.
+ * Key behavioral difference from {@link SearchQueryVisitor}:
+ * {@code visitImplicitAndExpression} uses OR semantics (anyMatch) instead of AND,
+ * so space-separated bare terms like "machine learning" match any group containing
+ * either word. Explicit AND/OR/NOT and parentheses work as expected.
+ */
Evidence
PR Compliance ID 16 requires multi-line Javadoc-style documentation to use ///. The new file adds
a /** ... */ multi-line documentation comment at the referenced lines.

AGENTS.md
jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java[6-12]

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

## Issue description
A new multi-line documentation comment uses `/** ... */` instead of the preferred Markdown Javadoc triple-slash (`///`) style.
## Issue Context
JabRef prefers `///` for multi-line Javadoc-style documentation to keep commenting consistent across the codebase.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java[6-12]

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



Remediation recommended

2. Repeated query parsing🐞 Bug ➹ Performance
Description
GroupNodeViewModel.isMatchedBy parses the ANTLR query for every predicate evaluation, and
RecursiveTreeItem’s recursive filtering calls that predicate many times per node, which can cause UI
lag on large group trees while typing in the filter box.
Code

jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java[R341-344]

+        try {
+            SearchParser.StartContext ctx = SearchQuery.getStartContext(searchString);
+            GroupNameFilterVisitor visitor = new GroupNameFilterVisitor(getDisplayName());
+            return visitor.visit(ctx);
Evidence
The PR moved filtering from a single case-insensitive substring check to building a full ANTLR
lexer/parser per call to isMatchedBy. The filter predicate is bound as `group ->
group.isMatchedBy(text)` and RecursiveTreeItem evaluates it for every node and recursively for
children, amplifying the cost (repeated parsing per node).

jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java[337-348]
jabgui/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java[107-113]
jabgui/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java[74-86]

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

## Issue description
`GroupNodeViewModel.isMatchedBy` builds a new ANTLR lexer/parser for every evaluation. The group tree filtering logic evaluates the predicate for every node and recursively walks children, multiplying the parsing cost and potentially causing UI lag.
## Issue Context
The filter predicate is created in `GroupTreeViewModel` and applied by `RecursiveTreeItem.showNode`, which calls the predicate repeatedly per node.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java[337-348]
- jabgui/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java[107-113]
- jabgui/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java[74-86]
## Suggested approach
- Move `SearchQuery.getStartContext(text)` into the lambda created in `GroupTreeViewModel` (runs once per text change), not into `GroupNodeViewModel.isMatchedBy`.
- Have the bound predicate look like:
- if blank -> `group -> true`
- else try parse once -> capture `StartContext ctx`
- `group -> new GroupNameFilterVisitor(group.getDisplayName()).visit(ctx)`
- catch ParseCancellationException once -> capture fallback matcher (e.g., `group -> StringUtil.containsIgnoreCase(group.getDisplayName(), text)`)
- Optionally change `GroupNodeViewModel.isMatchedBy` to accept a pre-parsed context/predicate to avoid rebuilding objects.

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


3. Locale-dependent lowercasing🐞 Bug ≡ Correctness
Description
GroupNameFilterVisitor uses String.toLowerCase() without Locale.ROOT for both the group name and
search term, which can produce incorrect matching under certain user locales (e.g., Turkish casing)
and is inconsistent with other search code paths.
Code

jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java[R18-20]

+    public GroupNameFilterVisitor(String groupName) {
+        this.groupName = groupName.toLowerCase();
+    }
Evidence
The new visitor lowercases using the JVM default locale. Elsewhere in the codebase, deterministic
case-folding uses Locale.ROOT (e.g., SearchQueryVisitor lowercases field names with Locale.ROOT),
indicating the expected pattern for stable matching regardless of OS/user locale.

jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java[18-21]
jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java[68-72]
jablib/src/main/java/org/jabref/logic/search/query/SearchQueryVisitor.java[85-87]

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

## Issue description
`GroupNameFilterVisitor` lowercases strings using the default locale, which can break case-insensitive matching in locales with special casing rules.
## Issue Context
Other search-related code (e.g., `SearchQueryVisitor`) uses `Locale.ROOT` for deterministic casing.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java[18-21]
- jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java[68-72]
- jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java[13-23]
## Suggested approach
- Import `java.util.Locale` and change:
- `groupName.toLowerCase()` -> `groupName.toLowerCase(Locale.ROOT)`
- `...toLowerCase()` for `term` -> `...toLowerCase(Locale.ROOT)`
- In the test fallback branch, also use `toLowerCase(Locale.ROOT)` to mirror production behavior.

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



Advisory comments

4. Misnamed fallback test🐞 Bug ≡ Correctness
Description
The test method malformedQueryFallsBackToContains uses a valid query ("test"), so its name implies
it covers the parse-failure fallback when it does not, which can mislead future maintenance of the
fallback behavior.
Code

jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java[R71-74]

+    @Test
+    void malformedQueryFallsBackToContains() {
+        assertTrue(matches("test group", "test"));
+    }
Evidence
Search.g4 allows a bare searchValue as a complete expression, so "test" parses successfully and
will not trigger the ParseCancellationException catch branch in matches(). Thus the test name is
misleading relative to what it exercises.

jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java[13-23]
jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java[71-74]
jablib/src/main/antlr/org/jabref/search/Search.g4[63-66]

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

## Issue description
A test name claims to validate malformed-query fallback, but the provided query is well-formed.
## Issue Context
The fallback branch in `matches()` only runs when `SearchQuery.getStartContext(query)` throws `ParseCancellationException`.
## Fix Focus Areas
- jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java[71-74]
## Suggested approach
- Option A (simplest): rename the test to `simpleQueryMatches()`.
- Option B: keep the test and add another test that triggers parse failure, e.g. query `"test AND"` (incomplete), and assert the fallback result against a group name that contains the literal substring (e.g., `"my test AND group"`) so the fallback path is demonstrably exercised.

ⓘ 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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates JabRef’s Groups side-pane filter to treat space-separated terms as an OR search (matching any typed word), while leveraging the existing Search.g4 grammar so that explicit boolean operators (AND, OR, NOT) and parentheses can be used in the filter input.

Changes:

  • Added GroupNameFilterVisitor to evaluate Search.g4 parse trees against group display names with OR semantics for implicit whitespace-separated terms.
  • Updated GroupNodeViewModel#isMatchedBy to parse and evaluate filter input via SearchQuery.getStartContext(...), with a fallback to plain containsIgnoreCase on parse errors.
  • Added GUI-level JUnit tests covering OR/AND/OR/NOT/parentheses behavior and updated CHANGELOG.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java New visitor implementing group-name matching based on Search.g4, with modified implicit-whitespace behavior.
jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java Routes group filter matching through the new ANTLR-based parser/visitor with a malformed-query fallback.
jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java Adds tests for the new filter semantics and operator handling.
CHANGELOG.md Documents the user-visible change in group filtering behavior.

You can also share your feedback on Copilot code review. Take the survey.


@Test
void malformedQueryFallsBackToContains() {
assertTrue(matches("test group", "test"));
@testlens-app

This comment has been minimized.

@testlens-app

This comment has been minimized.

@github-actions github-actions bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Mar 16, 2026
koppor
koppor previously requested changes Mar 16, 2026
@github-actions github-actions bot added status: changes-required Pull requests that are not yet complete and removed status: no-bot-comments labels Mar 16, 2026
…expanded CHANGELOG to address all functionalities
@testlens-app

This comment has been minimized.

@github-actions github-actions bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Mar 16, 2026
@testlens-app

This comment has been minimized.

@testlens-app

This comment has been minimized.

@subhramit
Copy link
Copy Markdown
Member

Please add disclosures on wherever AI has been used in this PR.

@testlens-app

This comment has been minimized.

@NishantDG-SST NishantDG-SST requested a review from subhramit March 25, 2026 17:12
@testlens-app

This comment has been minimized.

@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 Apr 1, 2026

✅ All tests passed ✅

🏷️ Commit: 1ad23ed
▶️ Tests: 10234 executed
⚪️ Checks: 50/50 completed


Learn more about TestLens at testlens.app.

@calixtus calixtus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 4, 2026
@github-actions github-actions bot added status: changes-required Pull requests that are not yet complete and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers status: no-bot-comments labels Apr 4, 2026
@calixtus calixtus enabled auto-merge April 4, 2026 15:50
@github-actions github-actions bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Apr 4, 2026
@calixtus calixtus dismissed koppor’s stale review April 5, 2026 21:37

Comments addressed

@calixtus calixtus added this pull request to the merge queue Apr 5, 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 5, 2026
Merged via the queue into JabRef:main with commit fbd2d47 Apr 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: groups status: no-bot-comments 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.

Group search should be OR - not AND

5 participants