OCR integration #13313

Kaan0029 · 2025-06-12T19:01:52Z

Closes #13267.

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
[.] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
[.] Tests created for changes (if applicable)
[.] Manually tested changed features in running JabRef (always required)
[.] Screenshots added in PR description (if change is visible to the user)
[.] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
[.] Checked 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 to the documentation repository.

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

jablib/src/main/java/org/jabref/logic/ocr/OcrException.java

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

jabgui/src/main/java/org/jabref/gui/linkedfile/OcrAction.java

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

subhramit · 2025-07-03T13:49:23Z

Your pull request conflicts with the target branch.

Please merge upstream/main 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.

Tip for future - always take a fresh pull from upstream/main before beginning to work on a branch (if there has been a decent time gap).

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

jabref-machine · 2025-07-07T21:57:42Z

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 "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.

jablib/src/main/java/org/jabref/logic/ocr/TesseractOcrProvider.java

trag-bot · 2025-07-21T21:08:27Z

jablib/src/main/java/org/jabref/logic/ai/AiService.java

+
+        this.ocrService = new OcrService(filePreferences);
+
+        // Log if OCR is not available but don't fail


The comment simply restates what the code does in the next lines. It doesn't provide additional information about why this approach was chosen or any other valuable context.

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

trag-bot · 2025-07-21T21:40:35Z

jablib/src/main/java/org/jabref/logic/ocr/TesseractOcrProvider.java

+        }
+        return false;   // nothing usable found
+    }
+


Catching generic Exception is too broad and masks specific issues. Should catch specific exceptions like IOException or SecurityException for better error handling.

Yes, trag bot probably selected wrong lines, but overall he's right, the Exception is too generic

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

jablib/src/main/java/org/jabref/logic/FilePreferences.java

InAnYan

Some refactoring comments

InAnYan · 2025-07-21T19:31:24Z

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

+                    Localization.lang("Failed to initialize AI service. OCR functionality will be unavailable.") + "\n\n" +
+                            Localization.lang("Error details: ") + e.getMessage());
+            // Set aiService to null - the application can continue without AI features
+            JabRefGUI.aiService = null;


No, it can't.

Trust me. It must be present and not null.

I think this is outdated, right?

InAnYan · 2025-07-21T19:31:38Z

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

+                    dialogService,
+                    taskExecutor);
+            Injector.setModelOrService(AiService.class, aiService);
+        } catch (OcrException e) {


Can you remove the throw of exception and not use them?

I think this is outdated, right?

InAnYan · 2025-07-21T19:32:45Z

jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextMenuFactory.java

@@ -86,9 +97,46 @@ private ContextMenu createContextMenuForFile(LinkedFileViewModel linkedFile) {
                factory.createMenuItem(StandardActions.DELETE_FILE, singleCommandFactory.build(StandardActions.DELETE_FILE, linkedFile))
        );

+        // Add OCR menu item for PDF files
+        if (linkedFile.getFile().getFileType().equalsIgnoreCase("pdf")) {


Try to search a method in class FilesUtil (or FileUtil) like isPDF

InAnYan · 2025-07-21T19:38:21Z

jabgui/src/main/java/org/jabref/gui/theme/ThemeManager.java

@@ -121,7 +121,7 @@ private void applyDarkModeToWindow(Stage stage, boolean darkMode) {
            return;
        }

-        themeWindowManager.setDarkModeForWindowFrame(stage, darkMode);
+        // themeWindowManager.setDarkModeForWindowFrame(stage, darkMode);


But this doesn't solve the original problem? Shouldn't you made a try/catch, so that other people who use JabRef could still use dark theme, if for some other machines it doesn't work

InAnYan · 2025-07-21T19:39:10Z

jablib/src/main/java/org/jabref/logic/ai/AiService.java


    public AiService(AiPreferences aiPreferences,
                     FilePreferences filePreferences,
                     CitationKeyPatternPreferences citationKeyPatternPreferences,
                     NotificationService notificationService,
                     TaskExecutor taskExecutor
-    ) {
+    ) throws OcrException {


Not the best idea

I think this is outdated, right?

InAnYan · 2025-07-22T08:26:54Z

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

+    /**
+     * Factory method for creating a success result.
+     */
+    static OcrResult success(@NonNull String text) {


Remove @NonNull and always assume that the input is not null

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

InAnYan · 2025-07-22T08:29:41Z

jablib/src/main/java/org/jabref/logic/ocr/TesseractOcrProvider.java

+        }
+        return false;   // nothing usable found
+    }
+


Yes, trag bot probably selected wrong lines, but overall he's right, the Exception is too generic

InAnYan · 2025-07-22T08:30:10Z

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

-                getBoolean(OPEN_FILE_EXPLORER_IN_LAST_USED_DIRECTORY));
+                getBoolean(OPEN_FILE_EXPLORER_IN_LAST_USED_DIRECTORY),
+                // TODO: replace placeholder once real source tag / persistence is implemented
+                "");


I think you are able to handle this. This is not hard

trag-bot · 2025-07-24T17:26:51Z

jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextMenuFactory.java

+        // Create the OCR action
+        OcrAction ocrAction = new OcrAction(


Comment is trivial and doesn't add any new information beyond what is clearly visible in the code. It simply restates what the code is doing.

trag-bot · 2025-07-24T17:26:55Z

jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextMenuFactory.java

+        // Set the action to execute when clicked
+        ocrItem.setOnAction(event -> ocrAction.execute());
+
+        // Disable if the action is not executable (file doesn't exist)


Both comments are redundant and simply describe what the code does without providing additional context or reasoning for the implementation choices.

trag-bot · 2025-07-24T17:29:24Z

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

+    /**
+     * Factory method for creating a success result with text and output file.
+     */
+    static OcrResult success(@NonNull String text, Path outputFile) {


The outputFile parameter lacks @nonnull annotation, and the method doesn't validate if the path is null before wrapping it in Optional.of(), which could cause NullPointerException.

trag-bot · 2025-07-24T17:29:25Z

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

+     */
+    record Failure(String errorMessage) implements OcrResult {
+        public Failure {
+            // Provide default message instead of throwing exception


The comment merely restates what the code does without providing additional insight or reasoning. Comments should add new information not plainly derived from the code.

calixtus · 2025-07-24T18:24:52Z

Commit pushed about one hour ago. We sadly had no time to review the changes in time for the meeting.

trag-bot · 2025-07-24T19:31:03Z

jablib/src/main/java/org/jabref/logic/ocr/OcrMyPdfWrapper.java

+        public boolean skipTextPages = true;
+        public boolean clean = false;
+        public boolean forceOcr = false;
+        public String language = "eng";
+        public int optimizeLevel = 1;


Fields in OcrMyPdfOptions are declared public, violating encapsulation principles. They should be private with getter/setter methods to maintain proper encapsulation.

trag-bot · 2025-07-24T19:31:45Z

jablib/src/main/java/org/jabref/logic/ocr/OcrProvider.java

+     *
+     * @return Configuration error message, or empty string if properly configured
+     */
+    String getConfigurationError();


Method returns String which could be null. According to modern Java practices and clean architecture, Optional should be used to handle potential empty states.

trag-bot · 2025-07-24T19:33:17Z

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

+     * @param method The method to use for creating the searchable PDF
+     * @return The result of the operation
+     */
+    public OcrResult createSearchablePdf(Path inputPdfPath, Path outputPdfPath, OcrMethod method) {


Method parameters lack null validation. All parameters should be validated against null at the start of the method to prevent NullPointerException.

trag-bot · 2025-07-24T19:34:06Z

jablib/src/main/java/org/jabref/logic/ocr/TesseractOcrProvider.java

+     * Gets the list of default tessdata paths based on the operating system.
+     */
+    private List<String> getDefaultTessdataPaths() {
+        List<String> paths = new ArrayList<>();


Usage of ArrayList constructor instead of modern Java collections factory methods. Should use List.of() and new methods for building collections.

Kaan0029 and others added 2 commits June 12, 2025 20:57

Initial implementation using tess4j

a51e3b0

merge

aca504a

calixtus changed the title ~~Initial implementation using tess4j~~ OCR integration Jun 12, 2025

calixtus added component: external-files project: gsoc component: pdf-viewer labels Jun 12, 2025

calixtus reviewed Jun 12, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Outdated Show resolved Hide resolved

add brew to jna path

48ffb06

Siedlerchr reviewed Jun 12, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Outdated Show resolved Hide resolved

subhramit reviewed Jun 12, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Outdated Show resolved Hide resolved

subhramit reviewed Jun 12, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrException.java Outdated Show resolved Hide resolved

subhramit added the component: ocr label Jun 12, 2025

Adapted Exception handling and configured tessdata variable

5a256ae

trag-bot bot reviewed Jun 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java Outdated Show resolved Hide resolved

Siedlerchr reviewed Jun 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Outdated Show resolved Hide resolved

Siedlerchr reviewed Jun 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java Outdated Show resolved Hide resolved

Siedlerchr reviewed Jun 19, 2025

View reviewed changes

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

subhramit reviewed Jun 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java Outdated Show resolved Hide resolved

subhramit reviewed Jun 19, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Outdated Show resolved Hide resolved

addressed mentor code feedback after 19.06.25

f80cec8

calixtus reviewed Jul 3, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Outdated Show resolved Hide resolved

koppor added this to the 6.0 milestone Jul 4, 2025

Merge branch 'upstream-main' into gsoc-ocr-tess4j-initial-implementation

db1f577

Kaan0029 force-pushed the gsoc-ocr-tess4j-initial-implementation branch from 9f91505 to db1f577 Compare July 10, 2025 10:49

Siedlerchr added 4 commits July 10, 2025 17:10

fix java modules

4020d3a

fix tessdata path

4987978

fix tessdata path amd module infor for lept4j

9842dd4

fix tessdata path amd module infor for lept4j

8b133e6

Kaan0029 added 2 commits July 21, 2025 18:02

fix tessdata path issue by removing getParent call

da6b8c8

delete OcrException and return optional or result (OcrResult) instead

a4e5d20

koppor reviewed Jul 21, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/TesseractOcrProvider.java Outdated Show resolved Hide resolved

koppor reviewed Jul 21, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/TesseractOcrProvider.java Show resolved Hide resolved

Kaan0029 added 4 commits July 21, 2025 22:23

adjust commenting style to be in line with JabRef conventions

af085b9

implemented string constant for 'tessdata' as it it used more than once

dd8d211

separate final and non-final fields

a3a90a8

store whole exception object instead of only exception message

e7dba5e

trag-bot bot reviewed Jul 21, 2025

View reviewed changes

Kaan0029 added 2 commits July 21, 2025 23:14

implement OcrBackgroundTask class

d13ea24

add method for getting property in filepreferences

14cf72d

trag-bot bot reviewed Jul 21, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Show resolved Hide resolved

trag-bot bot reviewed Jul 21, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Show resolved Hide resolved

trag-bot bot reviewed Jul 21, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/FilePreferences.java Show resolved Hide resolved

InAnYan requested changes Jul 22, 2025

View reviewed changes

This was referenced Jul 22, 2025

Added markdown document for ocr engine comparison #13572

Closed

added markdown document for ocr engine comparison #13573

Draft

Add initial implementation of embedded text layer using pdfbox

c5b0860

trag-bot bot reviewed Jul 24, 2025

View reviewed changes

subhramit removed the project: gsoc label Jul 24, 2025

Initial implementation of ocrmypdf as alternative to pdfbox

66bfdd6

trag-bot bot reviewed Jul 24, 2025

View reviewed changes


		this.ocrService = new OcrService(filePreferences);

		// Log if OCR is not available but don't fail

		// Create the OCR action
		OcrAction ocrAction = new OcrAction(

Uh oh!

OCR integration #13313

Are you sure you want to change the base?

OCR integration #13313

Uh oh!

Conversation

Kaan0029 commented Jun 12, 2025 • edited by InAnYan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mandatory checks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

subhramit commented Jul 3, 2025

Uh oh!

Uh oh!

jabref-machine commented Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

trag-bot bot Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

trag-bot bot Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

InAnYan 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

calixtus commented Jul 24, 2025

Uh oh!

trag-bot bot Jul 24, 2025

Kaan0029 commented Jun 12, 2025 •

edited by InAnYan

Loading