From a71096a2be4889396633bdf74dfee6ad142fdf87 Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:39:01 -0700 Subject: [PATCH] Fix: unify born-digital PDF detection between archive decision and OCR should_produce_archive() and RasterisedDocumentParser.parse() each reimplemented the "does this PDF have real text" check independently, using different normalization of pdftotext output. Raw pdftotext output can be non-empty (whitespace/form-feed layout padding) even when there is no real content, so the two checks could disagree: consumer.py treated a tagged-but-textless PDF as born-digital and skipped the archive, while the parser's own (stricter, normalized) check found no text and ran OCR anyway, leaving the document with no archive despite real OCR text (GH #13387). Both call sites now share one predicate, pdf_born_digital_text() in paperless/parsers/utils.py, so they can no longer drift apart. --- src/documents/consumer.py | 27 ++--- src/documents/tests/test_consumer_archive.py | 53 ++-------- src/paperless/parsers/tesseract.py | 27 +---- src/paperless/parsers/utils.py | 56 ++++++++++ src/paperless/tests/parsers/conftest.py | 15 +++ .../tests/parsers/test_tesseract_parser.py | 46 ++------ .../samples/tesseract/tagged-but-no-text.pdf | Bin 0 -> 5506 bytes src/paperless/tests/test_parser_utils.py | 100 ++++++++++++++++++ 8 files changed, 203 insertions(+), 121 deletions(-) create mode 100644 src/paperless/tests/samples/tesseract/tagged-but-no-text.pdf diff --git a/src/documents/consumer.py b/src/documents/consumer.py index e7f026e71..319e68ef9 100644 --- a/src/documents/consumer.py +++ b/src/documents/consumer.py @@ -57,9 +57,7 @@ from paperless.models import ArchiveFileGenerationChoices from paperless.parsers import ParserContext from paperless.parsers import ParserProtocol from paperless.parsers.registry import get_parser_registry -from paperless.parsers.utils import PDF_TEXT_MIN_LENGTH -from paperless.parsers.utils import extract_pdf_text -from paperless.parsers.utils import is_tagged_pdf +from paperless.parsers.utils import pdf_born_digital_text LOGGING_NAME: Final[str] = "paperless.consumer" @@ -160,29 +158,20 @@ def should_produce_archive( _log.debug("Archive: yes — image document, ARCHIVE_FILE_GENERATION=auto") return True if mime_type == "application/pdf": - text = extract_pdf_text(document_path) - has_text = text is not None and len(text) > 0 - if has_text and is_tagged_pdf(document_path): + text, born_digital = pdf_born_digital_text(document_path, log=_log) + if born_digital: _log.debug( - "Archive: no — born-digital PDF (structure tags detected)," - " ARCHIVE_FILE_GENERATION=auto", - ) - return False - if text is None or len(text) <= PDF_TEXT_MIN_LENGTH: - _log.debug( - "Archive: yes — scanned PDF (text_length=%d ≤ %d)," + "Archive: no - born-digital PDF (text_length=%d)," " ARCHIVE_FILE_GENERATION=auto", len(text) if text else 0, - PDF_TEXT_MIN_LENGTH, ) - return True + return False _log.debug( - "Archive: no — born-digital PDF (text_length=%d > %d)," + "Archive: yes - scanned/textless PDF (text_length=%d)," " ARCHIVE_FILE_GENERATION=auto", - len(text), - PDF_TEXT_MIN_LENGTH, + len(text) if text else 0, ) - return False + return True _log.debug( "Archive: no — MIME type %r not eligible for auto archive generation", mime_type, diff --git a/src/documents/tests/test_consumer_archive.py b/src/documents/tests/test_consumer_archive.py index c179162db..6644f2ba8 100644 --- a/src/documents/tests/test_consumer_archive.py +++ b/src/documents/tests/test_consumer_archive.py @@ -134,60 +134,29 @@ class TestShouldProduceArchive: assert should_produce_archive(parser, mime, Path("/tmp/doc")) is expected @pytest.mark.parametrize( - ("extracted_text", "expected"), + ("born_digital", "expected"), [ - pytest.param( - "This is a born-digital PDF with lots of text content. " * 10, - False, - id="born-digital-long-text-skips-archive", - ), - pytest.param(None, True, id="no-text-scanned-produces-archive"), - pytest.param("tiny", True, id="short-text-treated-as-scanned"), + pytest.param(True, False, id="born-digital-skips-archive"), + pytest.param(False, True, id="not-born-digital-produces-archive"), ], ) def test_auto_pdf_archive_decision( self, mocker: MockerFixture, settings, - extracted_text: str | None, + born_digital: bool, # noqa: FBT001 expected: bool, # noqa: FBT001 ) -> None: + """should_produce_archive() defers entirely to pdf_born_digital_text() + for the has-real-text decision, so both callers of that predicate + (this function and RasterisedDocumentParser.parse()) always agree.""" settings.ARCHIVE_FILE_GENERATION = "auto" - mocker.patch("documents.consumer.is_tagged_pdf", return_value=False) - mocker.patch("documents.consumer.extract_pdf_text", return_value=extracted_text) + mocker.patch( + "documents.consumer.pdf_born_digital_text", + return_value=("some text", born_digital), + ) parser = _parser_instance(can_produce=True, requires_rendition=False) assert ( should_produce_archive(parser, "application/pdf", Path("/tmp/doc.pdf")) is expected ) - - def test_tagged_pdf_skips_archive_in_auto_mode( - self, - mocker: MockerFixture, - settings, - ) -> None: - """Tagged PDFs (e.g. Word exports) with real text are treated as born-digital, even below PDF_TEXT_MIN_LENGTH.""" - settings.ARCHIVE_FILE_GENERATION = "auto" - mocker.patch("documents.consumer.is_tagged_pdf", return_value=True) - mocker.patch("documents.consumer.extract_pdf_text", return_value="tiny") - parser = _parser_instance(can_produce=True, requires_rendition=False) - assert ( - should_produce_archive(parser, "application/pdf", Path("/tmp/doc.pdf")) - is False - ) - - def test_tagged_pdf_without_text_produces_archive( - self, - mocker: MockerFixture, - settings, - ) -> None: - """A tagged PDF with no actual extractable text (e.g. some scanner firmware) is not - trusted as born-digital — the tag alone must not bypass OCR.""" - settings.ARCHIVE_FILE_GENERATION = "auto" - mocker.patch("documents.consumer.is_tagged_pdf", return_value=True) - mocker.patch("documents.consumer.extract_pdf_text", return_value=None) - parser = _parser_instance(can_produce=True, requires_rendition=False) - assert ( - should_produce_archive(parser, "application/pdf", Path("/tmp/doc.pdf")) - is True - ) diff --git a/src/paperless/parsers/tesseract.py b/src/paperless/parsers/tesseract.py index 78ce4c5c4..393beca39 100644 --- a/src/paperless/parsers/tesseract.py +++ b/src/paperless/parsers/tesseract.py @@ -3,7 +3,6 @@ from __future__ import annotations import importlib.resources import logging import os -import re import shutil import tempfile from pathlib import Path @@ -25,9 +24,9 @@ from paperless.config import OcrConfig from paperless.models import CleanChoices from paperless.models import ModeChoices from paperless.models import OutputTypeChoices -from paperless.parsers.utils import PDF_TEXT_MIN_LENGTH from paperless.parsers.utils import extract_pdf_text -from paperless.parsers.utils import is_tagged_pdf +from paperless.parsers.utils import pdf_born_digital_text +from paperless.parsers.utils import post_process_text from paperless.parsers.utils import read_file_handle_unicode_errors from paperless.version import __full_version_str__ @@ -509,11 +508,9 @@ class RasterisedDocumentParser: from ocrmypdf.exceptions import PriorOcrFoundError if mime_type == "application/pdf": - text_original = self.extract_text(None, document_path) - has_text = text_original is not None and len(text_original) > 0 - original_has_text = has_text and ( - is_tagged_pdf(document_path, log=self.log) - or len(text_original) > PDF_TEXT_MIN_LENGTH + text_original, original_has_text = pdf_born_digital_text( + document_path, + log=self.log, ) else: text_original = None @@ -658,17 +655,3 @@ class RasterisedDocumentParser: f"No text was found in {document_path}, the content will be empty.", ) self.text = "" - - -def post_process_text(text: str | None) -> str | None: - if not text: - return None - - collapsed_spaces = re.sub(r"([^\S\r\n]+)", " ", text) - no_leading_whitespace = re.sub(r"([\n\r]+)([^\S\n\r]+)", "\\1", collapsed_spaces) - no_trailing_whitespace = re.sub(r"([^\S\n\r]+)$", "", no_leading_whitespace) - - # TODO: this needs a rework - # replace \0 prevents issues with saving to postgres. - # text may contain \0 when this character is present in PDF files. - return no_trailing_whitespace.strip().replace("\0", " ") diff --git a/src/paperless/parsers/utils.py b/src/paperless/parsers/utils.py index 0257ab736..9b4590c21 100644 --- a/src/paperless/parsers/utils.py +++ b/src/paperless/parsers/utils.py @@ -111,6 +111,62 @@ def extract_pdf_text( return None +def post_process_text(text: str | None) -> str | None: + """Normalize extracted PDF/OCR text: collapse whitespace, strip padding. + + Returns ``None`` for ``None`` or whitespace-only input, so callers can + treat "no text" and "only layout padding" the same way. + """ + if not text: + return None + + collapsed_spaces = re.sub(r"([^\S\r\n]+)", " ", text) + no_leading_whitespace = re.sub(r"([\n\r]+)([^\S\n\r]+)", "\\1", collapsed_spaces) + no_trailing_whitespace = re.sub(r"([^\S\n\r]+)$", "", no_leading_whitespace) + + # replace \0 prevents issues with saving to postgres. + # text may contain \0 when this character is present in PDF files. + result = no_trailing_whitespace.strip().replace("\0", " ") + return result or None + + +def pdf_born_digital_text( + path: Path, + log: logging.Logger | None = None, +) -> tuple[str | None, bool]: + """Extract a PDF's text and decide whether it should be treated as born-digital. + + This is the single source of truth for "does this PDF already have real + text", used both to decide whether to produce an archive file and to + decide whether OCR can be skipped. Both decisions must agree, or a + tagged-but-textless PDF can end up with no archive AND a forced OCR pass + (see GH #13387): raw ``pdftotext -layout`` output can be non-empty + (whitespace/form-feed padding) even when there is no real content, so the + "has text" check must run against the *normalized* text, not the raw + extraction. + + Parameters + ---------- + path: + Absolute path to the PDF file. + log: + Logger for warnings. Falls back to the module-level logger when omitted. + + Returns + ------- + tuple[str | None, bool] + The normalized extracted text (or ``None``), and whether the PDF + counts as born-digital (has real text, and is either tagged or + exceeds ``PDF_TEXT_MIN_LENGTH``). + """ + text = post_process_text(extract_pdf_text(path, log=log)) + has_text = text is not None and len(text) > 0 + born_digital = has_text and ( + is_tagged_pdf(path, log=log) or len(text) > PDF_TEXT_MIN_LENGTH + ) + return text, born_digital + + def read_file_handle_unicode_errors( filepath: Path, log: logging.Logger | None = None, diff --git a/src/paperless/tests/parsers/conftest.py b/src/paperless/tests/parsers/conftest.py index 843ffdb88..61a3ac3ef 100644 --- a/src/paperless/tests/parsers/conftest.py +++ b/src/paperless/tests/parsers/conftest.py @@ -674,6 +674,21 @@ def single_page_mixed_pdf_file(tesseract_samples_dir: Path) -> Path: return tesseract_samples_dir / "single-page-mixed.pdf" +@pytest.fixture(scope="session") +def tagged_no_text_pdf_file(tesseract_samples_dir: Path) -> Path: + """Path to a tagged PDF whose only "text" is pdftotext layout padding. + + Reproduces GH #13387: ``/MarkInfo /Marked true`` is set, but the only + extractable content is a form-feed byte, not real text. + + Returns + ------- + Path + Absolute path to ``tesseract/tagged-but-no-text.pdf``. + """ + return tesseract_samples_dir / "tagged-but-no-text.pdf" + + @pytest.fixture(scope="session") def with_form_pdf_file(tesseract_samples_dir: Path) -> Path: """Path to a PDF with form sample file. diff --git a/src/paperless/tests/parsers/test_tesseract_parser.py b/src/paperless/tests/parsers/test_tesseract_parser.py index e56992d0b..f25efccc7 100644 --- a/src/paperless/tests/parsers/test_tesseract_parser.py +++ b/src/paperless/tests/parsers/test_tesseract_parser.py @@ -21,7 +21,7 @@ from documents.parsers import run_convert from paperless.models import ModeChoices from paperless.parsers import ParserProtocol from paperless.parsers.tesseract import RasterisedDocumentParser -from paperless.parsers.tesseract import post_process_text +from paperless.parsers.utils import is_tagged_pdf if TYPE_CHECKING: from pathlib import Path @@ -151,36 +151,6 @@ class TestRasterisedDocumentParserLifecycle: assert tempdir is not None and not tempdir.exists() -# --------------------------------------------------------------------------- -# post_process_text -# --------------------------------------------------------------------------- - - -class TestPostProcessText: - @pytest.mark.parametrize( - ("source", "expected"), - [ - pytest.param( - "simple string", - "simple string", - id="collapse-spaces", - ), - pytest.param( - "simple newline\n testing string", - "simple newline\ntesting string", - id="preserve-newline", - ), - pytest.param( - "utf-8 строка с пробелами в конце ", # noqa: RUF001 - "utf-8 строка с пробелами в конце", # noqa: RUF001 - id="utf8-trailing-spaces", - ), - ], - ) - def test_post_process_text(self, source: str, expected: str) -> None: - assert post_process_text(source) == expected - - # --------------------------------------------------------------------------- # Page count # --------------------------------------------------------------------------- @@ -910,25 +880,25 @@ class TestSkipArchive: self, mocker: MockerFixture, tesseract_parser: RasterisedDocumentParser, - tesseract_samples_dir: Path, + tagged_no_text_pdf_file: Path, ) -> None: """ GIVEN: - - A PDF that reports itself as tagged (/MarkInfo /Marked true) but - has no actual extractable text (some scanner firmware produces - this — see GitHub issue #13349) + - A real PDF that reports itself as tagged (/MarkInfo /Marked + true) but whose only pdftotext output is layout padding (a + lone form-feed byte), not real text (see GitHub issue #13387, + originally reported against #13349's tagged-PDF handling) - Mode: auto, produce_archive=False WHEN: - Document is parsed THEN: - The tag alone is not trusted as "has text"; OCRmyPDF still runs """ + assert is_tagged_pdf(tagged_no_text_pdf_file) is True tesseract_parser.settings.mode = ModeChoices.AUTO - mocker.patch("paperless.parsers.tesseract.is_tagged_pdf", return_value=True) - mocker.patch.object(tesseract_parser, "extract_text", return_value=None) mock_ocr = mocker.patch("ocrmypdf.ocr") tesseract_parser.parse( - tesseract_samples_dir / "multi-page-images.pdf", + tagged_no_text_pdf_file, "application/pdf", produce_archive=False, ) diff --git a/src/paperless/tests/samples/tesseract/tagged-but-no-text.pdf b/src/paperless/tests/samples/tesseract/tagged-but-no-text.pdf new file mode 100644 index 0000000000000000000000000000000000000000..5a652345a1c9e1fbe0ab1a90d183194ad344d49c GIT binary patch literal 5506 zcmc&&d0Z3M76xm@rc$ka6@^wttQ$0$NgyGKA(T}`Hd$m7d?A@Y$TD#fWD(Kd1D6(& z*9ufwDz*g_C@2`D$R<`~DIih*;C z?pft%X^kfm3~{TvZq+y9hyVm2z5zG}0|&ZVm?s6M{$q>IB7+-VP@uG22QA=i&?0YNo0VkL*L1WfdT z6#j_yUitH6SSaFTdRQpq2M2EEaU_5jKvu*N;Ee+eDOg-ilbf2yVQujiv!D~ zxA%T{+8Y1UiFUV}W`+hy!h;JMHm1k9^j(uR#ozkmtVulI;PXL7q`Rrtk(jTMF`j|7 z+U#3rRIbaM+0%ZXQQ$3Gtfm%5NV6&1xn%ATwZchQ@OI6@(3Y;@J=^CPU+J#<`qT;6 z6+tq##kzCpAJf*bnqM4?20MbkrL^9L=pDMz)-lJU?^F1$FLOHrV-F@KWxKA3d|tXB znf`)c+E-G{34eJ}R_@+5Y+6y*Z~lIHc#qqe%H4fWNc_zF#tWJw4+ri-ANB1Vw~*6w zD>M_fUPRlY8;gez>@1O=Ps-W%?DCZ&yfOJC>h1btQfioMZwVzid{K6kZT8C-4~n0R znU56|=AE6tkze2Fp0w@Gmce^X5P|UVLetNw;boq&+p%(^T+g!_B4_Q z&Po6OW1!az_Bj$&b^0L^21vFAC;OGReGMWU9rBeQd>u-)c(s;y%9H$`$X4VKZn%G z-}3v|zWPz{?uKvMH|N9;wvYIhE@NcYQNwq2Gh%z|n=(XsZa4Phi_t5hoJ-VoEhX~k zyhnD_yR`|zR>R%VwJ&;w^2c=Pc*;BO%M#Zw`eIQ_{?@bVNj+=FS2H$g4IMgB)PDde zKYy1$WL7GokeBE8oInz;mB-wucnTC9k<}H~9qB%5CT|X2AWZYl^exC39WrC?s;_;p zc1Tt@f9#3alrb)8;PiQ%3qNzj)Wu}Mos}MJ!;9}2%1IUU4{UrVnozJ@m@C6C7I=V;g(n6hjw!}8>xuI3C!^3%c1XkxbAF@Yp(LFUA6B}1Y_YeXKZ>jMi9=@wx2kjbmh1;FQY5VpTA#8TKG$gbB}cyDA&as7xMtSXguGf$ z>bgeP!Q7=yR>ypF=K6iR?K+S70kya(aqNuqfh%C4M#vnueY-M`Gc+9@i?w$Srajb; zylLj2FTZL9OM+ZaGdrs@>cSItlawx zx9F>TIT@(W2&@)19tZj>d`VA+2^^_b{Qy-1E>#K`<4#lYzIQ!^d@W!02Y-cit^xUU?=X(69 z={exg!@>t~>w4AN~jKTx}btwl_wAMagAO);z(6A6`qRGuNiZcUFnsf+MrvsIp|ondYxo zW|ev8&-Q3%v7l+t=2n;2(gAb*!}s5whM62RFE(8;D}S3L&ggmWJH>Z!#;tQ@X53!I zN7)OD>KU`%1a2=#U3_|#pG{eOSkA1S*%RBzCVEA=GyRRzq*NaY&412=IUhp>i0C$HsGL$(kHDlYVv2yV--m|~!V>P%+T}3Ylz32Dre~91*KQC>kJ|nj;!wrM}nZXe6u{F@26q zd~*(>QQyE3iKI8=ks%VcwbSJp(WuxOC+Bcdl*JRksItcC%nOCFb%4r3reY(o!ltrf zsVIbE{Y{pD7lQ&(StJgdN}>_ThQ@4&0>j2MF4Y(&8B None: + assert post_process_text(source) == expected + + +class TestPdfBornDigitalText: + """Regression coverage for GH #13387. + + should_produce_archive() and RasterisedDocumentParser.parse() must agree + on whether a PDF has real text, so both go through this one function. + """ + + def test_tagged_pdf_with_real_text_is_born_digital( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + mocker.patch( + "paperless.parsers.utils.extract_pdf_text", + return_value="tiny", + ) + mocker.patch("paperless.parsers.utils.is_tagged_pdf", return_value=True) + text, born_digital = pdf_born_digital_text(tmp_path / "doc.pdf") + assert text == "tiny" + assert born_digital is True + + def test_untagged_pdf_below_min_length_is_not_born_digital( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + mocker.patch( + "paperless.parsers.utils.extract_pdf_text", + return_value="tiny", + ) + mocker.patch("paperless.parsers.utils.is_tagged_pdf", return_value=False) + text, born_digital = pdf_born_digital_text(tmp_path / "doc.pdf") + assert text == "tiny" + assert born_digital is False + + def test_untagged_pdf_above_min_length_is_born_digital( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + mocker.patch( + "paperless.parsers.utils.extract_pdf_text", + return_value="x" * 51, + ) + mocker.patch("paperless.parsers.utils.is_tagged_pdf", return_value=False) + _text, born_digital = pdf_born_digital_text(tmp_path / "doc.pdf") + assert born_digital is True + + def test_no_text_is_not_born_digital( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + mocker.patch("paperless.parsers.utils.extract_pdf_text", return_value=None) + mocker.patch("paperless.parsers.utils.is_tagged_pdf", return_value=True) + text, born_digital = pdf_born_digital_text(tmp_path / "doc.pdf") + assert text is None + assert born_digital is False