Skip to content

Fixes and Extends BibliographyConsistencyCheck.check to Handle Custom Entry Types#14257

Merged
koppor merged 52 commits into
JabRef:mainfrom
Hrishi-Baskaran:support-customized-entry-types-consistency-check-13794
Jan 23, 2026
Merged

Fixes and Extends BibliographyConsistencyCheck.check to Handle Custom Entry Types#14257
koppor merged 52 commits into
JabRef:mainfrom
Hrishi-Baskaran:support-customized-entry-types-consistency-check-13794

Conversation

@Hrishi-Baskaran

@Hrishi-Baskaran Hrishi-Baskaran commented Nov 8, 2025

Copy link
Copy Markdown
Contributor

User description

Closes #13794

The current implementation of BibliographyConsistencyCheck.check is only aware of standard entry types of both BibTex and BibLaTex. The current implementation is not able to handle scenarios where the user defines a custom bibliography entry type. In this PR, BibliographyConsistencyCheck.check will be modified to handle consistency checks involving custom entry types in addition to new unit tests to cover the check for a variety of mocked custom entry types.

This PR also fixes a bug with the check function. The check function was also intended to check if required fields for a given type were set - returning entries that didn't have required fields set as inconsistencies. This was not mentioned in the javadoc but attempted anyway. Even then, the implementation was incorrect. In this PR, the implementation is updated to correctly determine if required fields are set. The Javadoc is also updated to explicitly mention this.

Steps to test

One can test this feature by running the tests in
BibliographyConsistencyCheckTest

In a live application, one can test this feature by going to Quality -> Check Consistency on a database. Ideally test on a database with custom entry types or with entry types that don't have required fields set to see the new update.

Mandatory checks

  • 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 described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
  • I checked the user documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

PR Type

Enhancement, Bug fix


Description

  • Consistency check now supports custom entry types and fields

  • Fixed required fields detection to properly handle OrFields constraints

  • Integrated BibEntryTypesManager throughout consistency check pipeline

  • Updated all callers to pass BibEntryTypesManager parameter

  • Enhanced test coverage with custom type scenarios


Diagram Walkthrough

flowchart LR
  A["BibliographyConsistencyCheck"] -->|uses| B["BibEntryTypesManager"]
  B -->|retrieves| C["Entry Type Definitions"]
  C -->|includes| D["Standard Types"]
  C -->|includes| E["Custom Types"]
  A -->|validates| F["Required Fields"]
  F -->|handles| G["OrFields Constraints"]
  A -->|detects| H["Field Inconsistencies"]
Loading

File Walkthrough

Relevant files
Enhancement, bug fix
1 files
BibliographyConsistencyCheck.java
Core consistency check refactored for custom types             
+21/-24 
Tests
3 files
BibliographyConsistencyCheckTest.java
Comprehensive tests for custom entry types                             
+303/-10
BibliographyConsistencyCheckResultCsvWriterTest.java
Updated CSV writer tests with required fields                       
+15/-5   
BibliographyConsistencyCheckResultTxtWriterTest.java
Updated TXT writer tests with required fields                       
+20/-8   
Enhancement
9 files
ConsistencyCheckAction.java
Pass BibEntryTypesManager to consistency check                     
+1/-1     
ConsistencyCheckDialogViewModel.java
Use display name for entry types                                                 
+1/-1     
JabRefGUI.java
Initialize LanguageServerController with entry types manager
+1/-1     
CheckConsistency.java
Pass BibEntryTypesManager to consistency check                     
+1/-1     
LspClientHandler.java
Accept BibEntryTypesManager in constructor                             
+3/-2     
LspLauncher.java
Thread LSP launcher with entry types manager                         
+6/-3     
LanguageServerController.java
Store and propagate BibEntryTypesManager                                 
+5/-2     
LspConsistencyCheck.java
Accept BibEntryTypesManager parameter                                       
+2/-2     
LspDiagnosticHandler.java
Store and use BibEntryTypesManager for diagnostics             
+5/-2     
Documentation
1 files
CHANGELOG.md
Document consistency check enhancements                                   
+1/-0     

@github-actions

github-actions Bot commented Nov 8, 2025

Copy link
Copy Markdown
Contributor

Hey @Hrishi-Baskaran!

Thank you for contributing to JabRef! Your help is truly appreciated ❤️.

We have automatic checks in place, based on which you will soon get automated feedback if any of them are failing. We also use TragBot with custom rules that scans your changes and provides some preliminary comments, before a maintainer takes a look. TragBot is still learning, and may not always be accurate. In the "Files changed" tab, you can go through its comments and just click on "Resolve conversation" if you are sure that it is incorrect, or comment on the conversation if you are doubtful.

Please re-check our contribution guide in case of any other doubts related to our contribution workflow.

@Siedlerchr

Copy link
Copy Markdown
Member

Please ensure the automatic check are green, otherwise no human will review it

@Hrishi-Baskaran

Copy link
Copy Markdown
Contributor Author

Please ensure the automatic check are green, otherwise no human will review it

Thanks for telling me about this. I will update this on Friday.

@Hrishi-Baskaran Hrishi-Baskaran force-pushed the support-customized-entry-types-consistency-check-13794 branch from e5a97e0 to e12bf76 Compare November 15, 2025 01:36
Also modifies BibliographyConsistencyCheckTest to mock BibEntryTypes
manager.
@Hrishi-Baskaran Hrishi-Baskaran force-pushed the support-customized-entry-types-consistency-check-13794 branch from e12bf76 to ba9446d Compare November 15, 2025 01:57
private final GuiPreferences preferences;
private final BibEntryTypesManager entryTypesManager;
private final UiTaskExecutor taskExecutor;
@Inject private BibEntryTypesManager bibEntryTypesManager;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't inject - we pass by constructor

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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


BibliographyConsistencyCheck consistencyCheck = new BibliographyConsistencyCheck();
BibliographyConsistencyCheck.Result result = consistencyCheck.check(databaseContext, (count, total) -> {
BibliographyConsistencyCheck.Result result = consistencyCheck.check(databaseContext, new BibEntryTypesManager(), (count, total) -> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Refactor this as variable to follow the style of Jabef

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

view update:
https://github.com/JabRef/jabref/pull/14257/files#diff-d3050a72d09ad6b3c585cb60f5d8e92bb6b9d7539ff23103b86c8c01391e247cR69-R70

Also just curious: should I be constructing a new BibEntryTypesManager here like I've done? This basically prevents the user from using custom types when using the cli consistency check. How might we let users specify a entry types manager from the cli? Or maybe I'm missing something and this isn't what we want to do.

// collects fields existing in any entry, scoped by entry type
Map<EntryType, Set<Field>> entryTypeToFieldsInAnyEntryMap = new HashMap<>();
// collects fields existing in all entries, scoped by entry type
// collects fields existing in all entries, scoped by entry typed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Typo - please undo

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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


BibDatabaseContext bibContext = new BibDatabaseContext(database);
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(bibContext, (count, total) -> {
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(bibContext, new BibEntryTypesManager(), (count, total) -> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Make this a variable in the test - and reuse it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

update:
https://github.com/JabRef/jabref/pull/14257/files#diff-641c831afa366f64368ec5c237c128d3fd3506f92ecc2c4d4a8125e23219849dR39-R62

I also made the mocked BibEntryTypesManager for this test file have some custom and unknown types (similar to what I did for BibliographyConsistencyCheckTest.java). But for this file, I don't use the custom and unknown types. Should I add some unit tests that use these, or remove these types for the mocked BibEntryTypesManager of the CSV test file?


@BeforeEach
void setUp() {
// TODO: add some custom entry types for this manager and test with it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, please :) - The issue description at #13794 should provide you with one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@koppor koppor added the status: changes-required Pull requests that are not yet complete label Nov 20, 2025
Also changes style and refactors some code to adhere to JabRef
standards.
@github-actions github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Nov 22, 2025
@Hrishi-Baskaran Hrishi-Baskaran marked this pull request as ready for review November 23, 2025 00:04
@koppor koppor added the status: changes-required Pull requests that are not yet complete label Nov 24, 2025
@github-actions

github-actions Bot commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

This PR is being closed due to continued inactivity.

@github-actions github-actions Bot closed this Dec 4, 2025
@Hrishi-Baskaran

Copy link
Copy Markdown
Contributor Author

Hello I'm very sorry about being inactive. I was going through a lot of other work I had to do and was planning on contributing again this weekend. Would it be possible to re-open this PR?

@Siedlerchr Siedlerchr reopened this Dec 4, 2025
@github-actions github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Dec 4, 2025
@koppor koppor added 📌 Pinned status: changes-required Pull requests that are not yet complete labels Dec 4, 2025
@github-actions github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Dec 7, 2025

BibliographyConsistencyCheck consistencyCheck = new BibliographyConsistencyCheck();
BibliographyConsistencyCheck.Result result = consistencyCheck.check(databaseContext, (count, total) -> {
BibEntryTypesManager bibEntryTypesManager = new BibEntryTypesManager();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No - pass this via constructor. See for instance org.jabref.gui.frame.SendAsStandardEmailAction#SendAsStandardEmailAction.


if (clientHandler.getSettings().isConsistencyCheck()) {
consistencyDiagnosticsCache.put(uri, lspConsistencyCheck.check(parserResult));
BibEntryTypesManager bibEntryTypesManager = new BibEntryTypesManager();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bibEntryTypesManager should be passed via the constructor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should be handled now

@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label Dec 7, 2025
@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label Jan 1, 2026
@github-actions github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Jan 1, 2026
@Hrishi-Baskaran Hrishi-Baskaran marked this pull request as ready for review January 1, 2026 23:11
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jan 1, 2026

Copy link
Copy Markdown
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jan 1, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Refactor to avoid passing BibEntryTypesManager

To reduce tight coupling, refactor the code to retrieve the BibEntryTypesManager
from a central service locator or dependency injection container instead of
passing it as a parameter through multiple application layers.

Examples:

jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckAction.java [39-66]
                                  BibEntryTypesManager entryTypesManager,
                                  UiTaskExecutor taskExecutor) {
        this.tabSupplier = tabSupplier;
        this.dialogService = dialogService;
        this.stateManager = stateManager;
        this.preferences = preferences;
        this.entryTypesManager = entryTypesManager;
        this.taskExecutor = taskExecutor;

        this.executable.bind(needsDatabase(stateManager));

 ... (clipped 18 lines)
jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java [113]
    public Result check(BibDatabaseContext bibContext, BibEntryTypesManager bibEntryTypesManager, BiConsumer<Integer, Integer> entriesGroupingProgress) {

Solution Walkthrough:

Before:

// In UI layer (e.g., ConsistencyCheckAction)
class ConsistencyCheckAction {
    private BibEntryTypesManager entryTypesManager;
    // ... constructor receives entryTypesManager

    public void execute() {
        BibliographyConsistencyCheck consistencyCheck = new BibliographyConsistencyCheck();
        // entryTypesManager is passed down
        return consistencyCheck.check(bibContext, entryTypesManager, ...);
    }
}

// In logic layer
class BibliographyConsistencyCheck {
    // check method signature is changed to accept the manager
    public Result check(BibDatabaseContext bibContext, BibEntryTypesManager bibEntryTypesManager, ...) {
        // ... uses bibEntryTypesManager
    }
}

After:

// In UI layer (e.g., ConsistencyCheckAction)
class ConsistencyCheckAction {
    // No longer needs to hold a reference to entryTypesManager

    public void execute() {
        BibliographyConsistencyCheck consistencyCheck = new BibliographyConsistencyCheck();
        // entryTypesManager is no longer passed
        return consistencyCheck.check(bibContext, ...);
    }
}

// In logic layer
class BibliographyConsistencyCheck {
    // check method signature is cleaner
    public Result check(BibDatabaseContext bibContext, ...) {
        // The manager is retrieved from a central service/injector
        BibEntryTypesManager bibEntryTypesManager = Injector.get(BibEntryTypesManager.class);
        // ... uses bibEntryTypesManager
    }
}
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a significant design issue of "prop drilling" the BibEntryTypesManager through multiple application layers, which increases coupling and reduces maintainability.

Medium
Learned
best practice
Fix grammatical error in changelog
Suggestion Impact:The suggestion was directly implemented. The commit changed line 5 of the changelog from "alerts on missing required fields" to "reports missing required fields", exactly as suggested.

code diff:

-- Consistency check is now aware of custom entry types, custom fields, and alerts on missing required fields. [#14257](https://github.com/JabRef/jabref/pull/14257)
+- Consistency check is now aware of custom entry types, custom fields, and reports missing required fields. [#14257](https://github.com/JabRef/jabref/pull/14257)

The word "alerts" should be corrected to "alerts users" or "reports" for
grammatical correctness and clarity in the changelog entry.

CHANGELOG.md [16]

-- Consistency check is now aware of custom entry types, custom fields, and alerts on missing required fields. [#14257](https://github.com/JabRef/jabref/pull/14257)
+- Consistency check is now aware of custom entry types, custom fields, and reports missing required fields. [#14257](https://github.com/JabRef/jabref/pull/14257)

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Correct typographical errors in documentation, code comments, configuration files, and user-facing strings. This includes fixing misspellings in variable names, method names, documentation text, changelog entries, and localization strings to maintain code quality and professionalism.

Low
Possible issue
Refactor filtering logic for correctness
Suggestion Impact:The commit directly implements the suggested refactoring. The filter lambda was changed from a complex inline expression to a block that extracts two boolean variables (hasDifferingFields and hasMissingRequiredFields) before returning their OR result. The logic remains functionally equivalent but is more readable.

code diff:

-                      .filter(entry ->
-                              // Handles violation: this entry is inconsistent because it sets a field
-                              // that not every other entry of its type sets (differing field)
-                              !Collections.disjoint(filterExcludedFields(entry.getFields()), differingFields)
-                                      // Handles violation: this entry is inconsistent because it does not set fields
-                                      // for its type that specify required fields
-                                      || (requiredFields.stream()
-                                                        .map(OrFields::getFields)
-                                                        .anyMatch(subfields ->
-                                                                Collections.disjoint(subfields, entry.getFields())
-                                                        )
-                              )
-                      )
+                      .filter(entry -> {
+                          boolean hasDifferingFields = !Collections.disjoint(filterExcludedFields(entry.getFields()), differingFields);
+                          boolean hasMissingRequiredFields = requiredFields.stream()
+                                                                           .anyMatch(orFields -> Collections.disjoint(orFields.getFields(), entry.getFields()));
+                          return hasDifferingFields || hasMissingRequiredFields;
+                      })

Refactor the filtering logic in filterAndSortEntriesWithFieldDifferences for
improved readability by extracting the conditions into separate boolean
variables.

jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java [74-94]

 List<BibEntry> filterAndSortEntriesWithFieldDifferences(Set<BibEntry> entries, Set<Field> differingFields, Set<OrFields> requiredFields) {
     return entries.stream()
-                  .filter(entry ->
-                          // Handles violation: this entry is inconsistent because it sets a field
-                          // that not every other entry of its type sets (differing field)
-                          !Collections.disjoint(filterExcludedFields(entry.getFields()), differingFields)
-                                  // Handles violation: this entry is inconsistent because it does not set fields
-                                  // for its type that specify required fields
-                                  || (requiredFields.stream()
-                                                    .map(OrFields::getFields)
-                                                    .anyMatch(subfields ->
-                                                            Collections.disjoint(subfields, entry.getFields())
-                                                    )
-                          )
-                  )
+                  .filter(entry -> {
+                      boolean hasDifferingFields = !Collections.disjoint(filterExcludedFields(entry.getFields()), differingFields);
+                      boolean hasMissingRequiredFields = requiredFields.stream()
+                                                                       .anyMatch(orFields -> Collections.disjoint(orFields.getFields(), entry.getFields()));
+                      return hasDifferingFields || hasMissingRequiredFields;
+                  })
                   .sorted(new FieldComparatorStack<>(List.of(
                           new BibEntryByCitationKeyComparator(),
                           new BibEntryByFieldsComparator()
                   )))
                   .toList();
 }

[Suggestion processed]

Suggestion importance[1-10]: 2

__

Why: The suggestion's reasoning is flawed as it misinterprets the code's intent; however, the proposed improved_code is logically equivalent to the existing_code and offers a minor improvement in readability by extracting boolean conditions into variables.

Low
  • Update

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 5, 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 labels Jan 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Your pull request conflicts with the target branch.

Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line.

@github-actions github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Jan 17, 2026

@koppor koppor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice to see the handling of the OrFields

I fixed the conflcits. Good to go.

If there are any logical flaws, user will report - hopefully ^^

@koppor koppor enabled auto-merge January 23, 2026 13:10
@koppor koppor added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Jan 23, 2026
@koppor koppor added this pull request to the merge queue Jan 23, 2026
Merged via the queue into JabRef:main with commit 5d73067 Jan 23, 2026
56 of 64 checks passed
@Hrishi-Baskaran

Copy link
Copy Markdown
Contributor Author

Thanks for fixing those remaining conflicts and merging the PR, you were such a great help during the entire process. I'll be ready to take on any user report issues about this, so please keep me in the loop.

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

Labels

component: consistency-check dev: code-quality Issues related to code or architecture decisions first contrib good first issue An issue intended for project-newcomers. Varies in difficulty. 📌 Pinned Review effort 4/5 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.

Support customized entry types in BibliographyConsistencyCheck

4 participants