Support customized entry types in BibliographyConsistencyCheck #13799

dcarpentiero · 2025-09-01T09:57:29Z

Closes #13794

Steps to test

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.

…blatexEntryTypeDefinitions#ALL`

…s-13794

jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java

…to customized-entry-types-13794

jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java

koppor · 2025-09-03T21:33:58Z

jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java

+        List<BibEntryType> entryTypeDefinitions = bibEntryTypesManager
+                .getAllTypes(bibContext.getMode())
+                .stream()
+                .toList();


No need to convert to a list - Collection should directly work?

Collection is a interface though–i need some class to implement it right? For example HashSet class implement the Set interface.

Also (correct me if I'm wrong, I'm here to learn) the expected type is a list, so I can't provide something else.

…iographyConsistencyCheck.java Co-authored-by: Oliver Kopp <[email protected]>

…cyCheck constructor

…where needed

…own as parameter

dcarpentiero · 2025-09-04T13:56:01Z

I've modified the BibliographyConsistencyCheck constructor to accept a CliPreferences parameter. Problem is, to make everything work I should change existing tests as the BibliographyConsistencyCheck constructor needs a param to work.
The assignment said I should't be changing existing tests, how should I proceed?

koppor · 2025-09-05T09:07:20Z

I've modified the BibliographyConsistencyCheck constructor to accept a CliPreferences parameter.

Nice

. Problem is, to make everything work I should change existing tests as the BibliographyConsistencyCheck constructor needs a param to work.
The assignment said I should't be changing existing tests, how should I proceed?

Oh, let's think if the intention of that... The intention was that test inputs and outputs should not change. If the test setup changes (e.g., new parameter required), it is perfectly fine!

…ckResultTxtWriterTest` and `BibliographyConsistencyCsvWriterTest` testing with `CliPreference` parameter

trag-bot · 2025-09-08T11:44:36Z

jabgui/src/main/java/org/jabref/gui/preferences/PreferencesDialogViewModel.java

        }

-        Injector.setModelOrService(BibEntryTypesManager.class, preferences.getCustomEntryTypesRepository());
+        Injector.setModelOrService(BibEntryTypesManager.class, preferences.getCustomEntryTypesRepository(bibEntryTypesManager));


Direct dependency injection manipulation in GUI code violates layered architecture principle. This logic should be moved to a service layer in org.jabref.logic.

…stency` and `PdfUpdate`

dcarpentiero · 2025-09-09T13:12:03Z

@subhramit hello I might need some help to resolve the conflicts. I tried to solve them but they still appear. What's still happening? Thank you

subhramit · 2025-09-09T13:26:14Z

@subhramit hello I might need some help to resolve the conflicts. I tried to solve them but they still appear. What's still happening? Thank you

Go to Resolve conflicts in the GitHub editor.

Here, there will be two pieces of code separated by =======
The upper section will contain your changes (in this branch)
The lower changes will contain what main has

These sections of code are basically conflicting, that is, trying to take the same lines. In most cases, only one of them should be there after your changes, but sometimes a combination can also be there, depending on your branch's logic.

After resolution, pull your changes to your local.

Please ask again if something is unclear.

Alternative:
If you pull upstream/main from IntelliJ, it will open a merge conflict resolution window where you can pick changes from your branch and main (wherever conflicting) and create the net file without conflicts.
After resolution, push your changes to remote.

dcarpentiero · 2025-09-09T13:47:51Z

@subhramit hello I might need some help to resolve the conflicts. I tried to solve them but they still appear. What's still happening? Thank you

Go to Resolve conflicts in the GitHub editor.

Here, there will be two pieces of code separated by ======= The upper section will contain your changes (in this branch) The lower changes will contain what main has

These sections of code are basically conflicting, that is, trying to take the same lines. In most cases, only one of them should be there after your changes, but sometimes a combination can also be there, depending on your branch's logic.

After resolution, pull your changes to your local.

Please ask again if something is unclear.

Alternative: If you pull upstream/main from IntelliJ, it will open a merge conflict resolution window where you can pick changes from your branch and main (wherever conflicting) and create the net file without conflicts. After resolution, push your changes to remote.

Thank you I've resolved by following this, will surely ask again if necessity arises.

koppor

Too many changes regarding BibEntryTypesManager. Most of them can be undone.

Start with reverting all changes in CliPreferences

koppor · 2025-09-09T19:54:46Z

jabgui/src/main/java/org/jabref/Launcher.java

        Injector.setModelOrService(BuildInfo.class, new BuildInfo());

        final JabRefGuiPreferences preferences = JabRefGuiPreferences.getInstance();
+        final BibEntryTypesManager bibEntryTypesManager = new BibEntryTypesManager();


Please do not store it as variable - you only need it once below.

koppor · 2025-09-09T19:55:20Z

jabgui/src/main/java/org/jabref/Launcher.java

        }

-        PreferencesMigrations.runMigrations(preferences);
+        PreferencesMigrations.runMigrations(preferences, bibEntryTypesManager);


Remove the parameter.

Later, use preferences.getCustomEntryTypesRepository() to get the bibEntryTypesManager

What is the advantage of declaring BibEntryTypesManager bibEntryTypesManager = preferences.getCustomEntryTypesRepository(); and then upgradeStoredBibEntryTypes(preferences, mainPrefsNode, bibEntryTypesManager); instead of just upgradeStoredBibEntryTypes(preferences, mainPrefsNode, preferences.getCustomEntryTypesRepository());? Am i missing something here?

koppor · 2025-09-09T19:56:49Z

jabgui/src/main/java/org/jabref/gui/JabRefGUI.java

                dialogService,
                fileUpdateMonitor,
                preferences,
+                cliPreferences,


Please remove this.

See that GuiPreferences extends CliPreferences.

Thus, the GuiPreferences have all the CliPreferences

koppor · 2025-09-09T19:58:32Z

jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java

    @Override
-    public BibEntryTypesManager getCustomEntryTypesRepository() {
-        BibEntryTypesManager bibEntryTypesManager = new BibEntryTypesManager();
+    public BibEntryTypesManager getCustomEntryTypesRepository(BibEntryTypesManager bibEntryTypesManager) {


Please undo this - the point here is that a new repository is generated.

Ohhh I should implement both, the one without the parameter and then override with the parameter did I get it right?

…extends `CliPreferences`

…one parameter, the `BibEntryTypesManager` is retrieved later

… instance

jabgui/src/main/java/org/jabref/gui/JabRefGUI.java

…ferences` interface

jabref-machine · 2025-09-10T08:02:12Z

Your code currently does not meet JabRef's code guidelines. We use Checkstyle to identify issues. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Source Code Tests / Checkstyle (pull_request)" and click on it.

In case of issues with the import order, double check that you activated Auto Import. You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.

Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues, commit, and push.

…ing) fields (example "- - -");

jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java

…fields.

…eld set or a missing field

dcarpentiero · 2025-09-12T07:29:07Z

@subhramit Hi, I'm running into a design issue with the filtering logic in filterAndSortEntriesWithFieldDifferences.

I have two types of inconsistencies to handle:

Missing required fields (e.g., BIBLATEX with Online type): all entries in the group should be included
Only inconsistencies between optional fields (e.g., BIBTEX with unknown type): only entries that have those fields should be included

The problem is that the method only receives differingFields and requiredFields, but cannot distinguish whether there are actually entries with missing required fields in the specific group.

The failing tests show that:

checkComplexLibrary expects the sixth entry (which has none of the inconsistent fields)
unsetFieldsReportedInBibtexMode does NOT want the withoutDate entry (which lacks the date field)

The current logic cannot satisfy both cases. I need a refactoring to pass more context or change the filtering approach.

I'd like some help on how to proceed?

jabref-machine · 2025-09-16T10:05:45Z

Your code currently does not meet JabRef's code guidelines. IntelliJ auto format covers some cases. There seem to be issues with your code style and autoformat configuration. Please reformat your code (Ctrl+Alt+L) and commit, then push.

In special cases, consider using // formatter:off and // formatter:on annotations to allow deviation from the code style.

trag-bot · 2025-09-16T10:07:16Z

jabgui/src/main/java/org/jabref/gui/consistency/ConsistencyCheckAction.java

                BibDatabaseContext bibContext = databaseContext.get();

-                BibliographyConsistencyCheck consistencyCheck = new BibliographyConsistencyCheck();
+                BibliographyConsistencyCheck consistencyCheck = new BibliographyConsistencyCheck(preferences, entryTypesManager);


The patch introduces a constructor call with parameters, but it does not ensure that null is never passed to the method, which violates the guideline that null should never be passed to a method.

subhramit · 2025-09-16T18:17:11Z

@subhramit Hi, I'm running into a design issue with the filtering logic in filterAndSortEntriesWithFieldDifferences.

I have two types of inconsistencies to handle:

Missing required fields (e.g., BIBLATEX with Online type): all entries in the group should be included

Only inconsistencies between optional fields (e.g., BIBTEX with unknown type): only entries that have those fields should be included

The problem is that the method only receives differingFields and requiredFields, but cannot distinguish whether there are actually entries with missing required fields in the specific group.

The failing tests show that:

checkComplexLibrary expects the sixth entry (which has none of the inconsistent fields)

unsetFieldsReportedInBibtexMode does NOT want the withoutDate entry (which lacks the date field)

The current logic cannot satisfy both cases. I need a refactoring to pass more context or change the filtering approach.

I'd like some help on how to proceed?

Passing more context is fine, also, when you say change the filtering approach, what do you propose?
I also think I am not the best person to answer this, tagging @koppor

dcarpentiero added 2 commits September 1, 2025 11:42

feat: draft PR for substituting BibEntryTypesManager instead of `Bi…

94e1770

…blatexEntryTypeDefinitions#ALL`

Merge remote-tracking branch 'origin/main' into customized-entry-type…

b42b7c8

…s-13794

dcarpentiero changed the title ~~Customized entry types 13794~~ Customized entry types Sep 1, 2025

calixtus changed the title ~~Customized entry types~~ Suppoer customized entry types in BibliographyConsistencyCheck Sep 1, 2025

calixtus changed the title ~~Suppoer customized entry types in BibliographyConsistencyCheck~~ Support customized entry types in BibliographyConsistencyCheck Sep 1, 2025

calixtus reviewed Sep 1, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java Outdated Show resolved Hide resolved

dcarpentiero and others added 5 commits September 2, 2025 10:03

fix: removing comments

274d859

fix: resolving conflicts

b9d69c2

Merge branch 'main' into customized-entry-types-13794

b2bc3a4

Merge branch 'main' into customized-entry-types-13794

8128685

Merge branch 'main' into customized-entry-types-13794

831ed3b

dcarpentiero requested a review from calixtus September 3, 2025 06:57

dcarpentiero added 3 commits September 3, 2025 09:04

fix: modern best practices: Set.copyOf() instead of new HashSet<>()

9942817

Merge remote-tracking branch 'origin/customized-entry-types-13794' in…

ff8006e

…to customized-entry-types-13794

feat: double constructor: a default and accepting manager constructor

d453516

koppor requested changes Sep 3, 2025

View reviewed changes

dcarpentiero and others added 5 commits September 4, 2025 09:12

Update jablib/src/main/java/org/jabref/logic/quality/consistency/Bibl…

53b83d9

…iographyConsistencyCheck.java Co-authored-by: Oliver Kopp <[email protected]>

feat: passing down the BibEntryTypes manager to BibliographyConsisten…

b7145af

…cyCheck constructor

feat: starting substituting CliPreferences with JabRefCliPreferences …

baeff03

…where needed

Merge branch 'main' into customized-entry-types-13794

3a79c98

fix: BibliographyConsistencyCheck now accept a CliPreference passed d…

d302f7c

…own as parameter

Merge branch 'main' into customized-entry-types-13794

76394d7

feat: BibliographyConsistencyCheckTest, `BibliographyConsistencyChe…

3f97a75

…ckResultTxtWriterTest` and `BibliographyConsistencyCsvWriterTest` testing with `CliPreference` parameter

trag-bot bot reviewed Sep 8, 2025

View reviewed changes

dcarpentiero added 4 commits September 8, 2025 14:07

fix: passing getCustomEntryTypes parameter to JabKit, `CheckConsi…

37416ff

…stency` and `PdfUpdate`

fix: typo

66b2bbf

fix: typo

2031f0e

fix: typo and parameter order

ad131c6

dcarpentiero added 3 commits September 9, 2025 14:47

fix: resolving conflicts

1fc3810

fix: resolving conflicts

d30b731

fix: resolving conflicts

462657a

Merge branch 'main' into customized-entry-types-13794

704885f

dcarpentiero added 2 commits September 9, 2025 16:05

fix: removing double imports

c321b78

fix: removing double imports

79af35f

koppor requested changes Sep 9, 2025

View reviewed changes

dcarpentiero added 3 commits September 10, 2025 09:22

fix: removing CliPreferences redundancy–– GuiPreferences already …

3befca7

…extends `CliPreferences`

fix: PreferencesMigrations.runMigrations(preferences); now accepts …

7125b76

…one parameter, the `BibEntryTypesManager` is retrieved later

fix: passing null to getCustomEntryTypesRepository to get the first…

e580f9a

… instance

trag-bot bot reviewed Sep 10, 2025

View reviewed changes

jabgui/src/main/java/org/jabref/gui/JabRefGUI.java Outdated Show resolved Hide resolved

fix: adding no-parameter getCustomEntryRepository method to `CliPre…

d3bfbd8

…ferences` interface

dcarpentiero added 3 commits September 10, 2025 10:02

fix: typo

6ed613a

fix: adding missing method to BibliographyConsistencyCheckTest

6cc90e2

fix: including previously excluded entries that have consistent (miss…

f1adef0

…ing) fields (example "- - -");

trag-bot bot reviewed Sep 10, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java Outdated Show resolved Hide resolved

dcarpentiero added 4 commits September 10, 2025 15:57

fix: typo

6cea857

feat: now accepting entries that present consistent (eg. all absent) …

c899efa

…fields.

fix: when filtering entries, checking if the entry has a differing fi…

3e6c4b1

…eld set or a missing field

fix: when filtering entries, checking if the entry has a differing fi…

d681cd8

…eld set or a missing field

Merge branch 'main' into customized-entry-types-13794

5040b0c

trag-bot bot reviewed Sep 16, 2025

View reviewed changes

Siedlerchr added the status: changes-required Pull requests that are not yet complete label Sep 23, 2025

Uh oh!

Support customized entry types in BibliographyConsistencyCheck #13799

Are you sure you want to change the base?

Support customized entry types in BibliographyConsistencyCheck #13799

Uh oh!

Conversation

dcarpentiero commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps to test

Mandatory checks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcarpentiero commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

koppor commented Sep 5, 2025

Uh oh!

trag-bot bot Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

dcarpentiero commented Sep 9, 2025

Uh oh!

subhramit commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcarpentiero commented Sep 9, 2025

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jabref-machine commented Sep 10, 2025

Uh oh!

Uh oh!

dcarpentiero commented Sep 12, 2025

Uh oh!

jabref-machine commented Sep 16, 2025

Uh oh!

trag-bot bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

subhramit commented Sep 16, 2025

Uh oh!

Uh oh!

dcarpentiero commented Sep 1, 2025 •

edited

Loading

dcarpentiero commented Sep 4, 2025 •

edited

Loading

subhramit commented Sep 9, 2025 •

edited

Loading