From dbe0eda304f4e21370d86640d86cbac782aaaef3 Mon Sep 17 00:00:00 2001 From: JessYan <15711968+JessYanCoding@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:02:07 +0800 Subject: [PATCH] Fix the remaining rag/app chunkers treating an empty upload as a missing binary (#19256) --- rag/app/book.py | 2 +- rag/app/laws.py | 25 +-- rag/app/manual.py | 4 +- rag/app/one.py | 2 +- rag/app/paper.py | 4 +- rag/app/qa.py | 8 +- rag/app/table.py | 2 +- .../unit_test/rag/app/test_book_coord_tags.py | 8 + test/unit_test/rag/app/test_manual.py | 8 + .../rag/app/test_parsers_empty_binary.py | 149 ++++++++++++++++++ 10 files changed, 178 insertions(+), 34 deletions(-) create mode 100644 test/unit_test/rag/app/test_parsers_empty_binary.py diff --git a/rag/app/book.py b/rag/app/book.py index d92a6b4d84..4f8be76c53 100644 --- a/rag/app/book.py +++ b/rag/app/book.py @@ -38,7 +38,7 @@ class Pdf(PdfParser): start = timer() callback(msg="OCR started") - self.__images__(filename if not binary else binary, zoomin, from_page, to_page, callback) + self.__images__(filename if binary is None else binary, zoomin, from_page, to_page, callback) callback(msg="OCR finished ({:.2f}s)".format(timer() - start)) start = timer() diff --git a/rag/app/laws.py b/rag/app/laws.py index 3265138c64..596dd3224c 100644 --- a/rag/app/laws.py +++ b/rag/app/laws.py @@ -36,27 +36,6 @@ class Docx(DocxParser): def __init__(self): pass - def __clean(self, line): - line = re.sub(r"\u3000", " ", line).strip() - return line - - def old_call(self, filename, binary=None, from_page=0, to_page=MAXIMUM_PAGE_NUMBER): - self.doc = Document(filename) if not binary else Document(BytesIO(binary)) - pn = 0 - lines = [] - for p in self.doc.paragraphs: - if pn > to_page: - break - if from_page <= pn < to_page and p.text.strip(): - lines.append(self.__clean(p.text)) - for run in p.runs: - if "lastRenderedPageBreak" in run._element.xml: - pn += 1 - continue - if "w:br" in run._element.xml and 'type="page"' in run._element.xml: - pn += 1 - return [line for line in lines if line] - def __table_to_html(self, tb): html = "" for r in tb.rows: @@ -82,7 +61,7 @@ class Docx(DocxParser): return html def __call__(self, filename, binary=None, from_page=0, to_page=MAXIMUM_PAGE_NUMBER): - self.doc = Document(filename) if not binary else Document(BytesIO(binary)) + self.doc = Document(filename) if binary is None else Document(BytesIO(binary)) pn = 0 lines = [] level_set = set() @@ -151,7 +130,7 @@ class Pdf(PdfParser): start = timer() callback(msg="OCR started") - self.__images__(filename if not binary else binary, zoomin, from_page, to_page, callback) + self.__images__(filename if binary is None else binary, zoomin, from_page, to_page, callback) callback(msg="OCR finished ({:.2f}s)".format(timer() - start)) start = timer() diff --git a/rag/app/manual.py b/rag/app/manual.py index 7c27f0a5f3..693df71d12 100644 --- a/rag/app/manual.py +++ b/rag/app/manual.py @@ -41,7 +41,7 @@ class Pdf(PdfParser): start = timer() callback(msg="OCR started") - self.__images__(filename if not binary else binary, zoomin, from_page, to_page, callback) + self.__images__(filename if binary is None else binary, zoomin, from_page, to_page, callback) callback(msg="OCR finished ({:.2f}s)".format(timer() - start)) logging.debug("OCR: {}".format(timer() - start)) @@ -73,7 +73,7 @@ class Docx(DocxParser): pass def __call__(self, filename, binary=None, from_page=0, to_page=MAXIMUM_PAGE_NUMBER, callback=None): - self.doc = Document(filename) if not binary else Document(BytesIO(binary)) + self.doc = Document(filename) if binary is None else Document(BytesIO(binary)) pn = 0 last_answer, last_image = "", None question_stack, level_stack = [], [] diff --git a/rag/app/one.py b/rag/app/one.py index e866a8beb1..11c4bbf66a 100644 --- a/rag/app/one.py +++ b/rag/app/one.py @@ -38,7 +38,7 @@ class Pdf(PdfParser): start = timer() callback(msg="OCR started") - self.__images__(filename if not binary else binary, zoomin, from_page, to_page, callback) + self.__images__(filename if binary is None else binary, zoomin, from_page, to_page, callback) callback(msg="OCR finished ({:.2f}s)".format(timer() - start)) start = timer() diff --git a/rag/app/paper.py b/rag/app/paper.py index 97bbc03572..83345824e3 100644 --- a/rag/app/paper.py +++ b/rag/app/paper.py @@ -38,7 +38,7 @@ class Pdf(PdfParser): start = timer() callback(msg="OCR started") - self.__images__(filename if not binary else binary, zoomin, from_page, to_page, callback) + self.__images__(filename if binary is None else binary, zoomin, from_page, to_page, callback) callback(msg="OCR finished ({:.2f}s)".format(timer() - start)) start = timer() @@ -157,7 +157,7 @@ def chunk(filename, binary=None, from_page=0, to_page=MAXIMUM_PAGE_NUMBER, lang= if name == "deepdoc": pdf_parser = Pdf() - paper = pdf_parser(filename if not binary else binary, from_page=from_page, to_page=to_page, callback=callback) + paper = pdf_parser(filename if binary is None else binary, from_page=from_page, to_page=to_page, callback=callback) sections = paper.get("sections", []) else: kwargs.pop("parse_method", None) diff --git a/rag/app/qa.py b/rag/app/qa.py index 3d3ab72d33..55ff6a759b 100644 --- a/rag/app/qa.py +++ b/rag/app/qa.py @@ -35,7 +35,7 @@ from common.float_utils import get_float class Excel(ExcelParser): def __call__(self, fnm, binary=None, callback=None): - if not binary: + if binary is None: wb = load_workbook(fnm) else: wb = load_workbook(BytesIO(binary)) @@ -75,7 +75,7 @@ class Pdf(PdfParser): def __call__(self, filename, binary=None, from_page=0, to_page=MAXIMUM_PAGE_NUMBER, zoomin=3, callback=None): start = timer() callback(msg="OCR started") - self.__images__(filename if not binary else binary, zoomin, from_page, to_page, callback) + self.__images__(filename if binary is None else binary, zoomin, from_page, to_page, callback) callback(msg="OCR finished ({:.2f}s)".format(timer() - start)) logging.debug("OCR({}~{}): {:.2f}s".format(from_page, to_page, timer() - start)) start = timer() @@ -178,7 +178,7 @@ class Docx(DocxParser): pass def __call__(self, filename, binary=None, from_page=0, to_page=MAXIMUM_PAGE_NUMBER, callback=None): - self.doc = Document(filename) if not binary else Document(BytesIO(binary)) + self.doc = Document(filename) if binary is None else Document(BytesIO(binary)) pn = 0 last_answer, last_image = "", None question_stack, level_stack = [], [] @@ -384,7 +384,7 @@ def chunk(filename, binary=None, from_page=0, to_page=MAXIMUM_PAGE_NUMBER, lang= elif re.search(r"\.pdf$", filename, re.IGNORECASE): callback(0.1, "Start to parse.") pdf_parser = Pdf() - qai_list, tbls = pdf_parser(filename if not binary else binary, from_page=from_page, to_page=to_page, callback=callback) + qai_list, tbls = pdf_parser(filename if binary is None else binary, from_page=from_page, to_page=to_page, callback=callback) for q, a, image, poss in qai_list: res.append(beAdocPdf(deepcopy(doc), q, a, eng, image, poss)) return res diff --git a/rag/app/table.py b/rag/app/table.py index c32474d384..b49f882de6 100644 --- a/rag/app/table.py +++ b/rag/app/table.py @@ -74,7 +74,7 @@ class Excel(ExcelParser): lang="English", **kwargs, ): - if not binary: + if binary is None: wb = Excel._load_excel_to_workbook(fnm) else: wb = Excel._load_excel_to_workbook(BytesIO(binary)) diff --git a/test/unit_test/rag/app/test_book_coord_tags.py b/test/unit_test/rag/app/test_book_coord_tags.py index 8c44433980..830b27aee2 100644 --- a/test/unit_test/rag/app/test_book_coord_tags.py +++ b/test/unit_test/rag/app/test_book_coord_tags.py @@ -135,6 +135,14 @@ def _env(monkeypatch): if name == "deepdoc" or name.startswith("deepdoc."): sys.modules.pop(name, None) sys.modules.update(saved) + # `rag.app.book` above was imported against the stubbed `deepdoc` tree, so + # its classes carry stub bases. Drop it as well, otherwise later tests in + # the same session reuse that copy instead of the real module. The import + # system also bound `book` as an attribute of the `rag.app` package, and + # `from rag.app import book` resolves through that attribute without + # consulting sys.modules, so both halves have to go. + sys.modules.pop("rag.app.book", None) + getattr(sys.modules.get("rag.app"), "__dict__", {}).pop("book", None) def _chunk(book, sections, fake_pdf, monkeypatch): diff --git a/test/unit_test/rag/app/test_manual.py b/test/unit_test/rag/app/test_manual.py index 08c9705f06..38ced915d5 100644 --- a/test/unit_test/rag/app/test_manual.py +++ b/test/unit_test/rag/app/test_manual.py @@ -56,6 +56,14 @@ def manual_module(): sys.modules.pop(name, None) else: sys.modules[name] = original + # `rag.app.manual` above was imported against the stubs, so it holds + # MagicMock bases. Drop it as well, otherwise later tests in the same + # session reuse that copy instead of the real module. The import system + # also bound `manual` as an attribute of the `rag.app` package, and + # `from rag.app import manual` resolves through that attribute without + # consulting sys.modules, so both halves have to go. + sys.modules.pop("rag.app.manual", None) + getattr(sys.modules.get("rag.app"), "__dict__", {}).pop("manual", None) def dummy(prog=None, msg=""): diff --git a/test/unit_test/rag/app/test_parsers_empty_binary.py b/test/unit_test/rag/app/test_parsers_empty_binary.py new file mode 100644 index 0000000000..ed92af7edd --- /dev/null +++ b/test/unit_test/rag/app/test_parsers_empty_binary.py @@ -0,0 +1,149 @@ +# +# Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +"""A 0-byte upload reaches the chunkers as b"", not None. + +`rag/svr/task_executor.py` guards only `binary is None`, then calls +`chunker.chunk(task["name"], binary=binary, ...)`. `task["name"]` is a display +name, so any branch that falls back to opening `filename` looks for a file that +does not exist. Every site below must therefore branch on `binary is None`, not +on its truthiness. + +Follows #17904 (`html_parser.py`), #18712 (`email.py`, `presentation.py`) and +#18826 (`naive.py`). +""" + +from __future__ import annotations + +import importlib +from zipfile import BadZipFile + +import pytest + +# Display names, not paths on disk. +PDF_NAME = "report.pdf" +DOCX_NAME = "report.docx" +XLSX_NAME = "report.xlsx" + +PDF_MODULES = ["book", "laws", "manual", "one", "paper", "qa"] +PDF_CHUNK_MODULES = ["paper", "qa"] +DOCX_MODULES = ["laws", "manual", "qa"] + + +class _Reached(Exception): + """Raised from the stubbed OCR entry point to stop the parser right there.""" + + +def _load(module_name: str): + """Import a `rag.app` module lazily, inside the test that needs it. + + Keeps this file independent of whether any other test module already + imported `rag.app.*` at collection time, so it behaves the same standalone + as it does in a directory run. + """ + return importlib.import_module(f"rag.app.{module_name}") + + +def _noop_callback(*args, **kwargs): + return None + + +@pytest.fixture(autouse=True) +def _stub_rag_tokenizer(monkeypatch): + def fake_tokenize(text): + return str(text) + + monkeypatch.setattr("rag.nlp.rag_tokenizer.tokenize", fake_tokenize) + monkeypatch.setattr("rag.nlp.rag_tokenizer.fine_grained_tokenize", fake_tokenize) + + +@pytest.fixture +def captured_ocr_source(monkeypatch): + """Capture what `Pdf.__call__` hands to `__images__`, then abort the parse. + + `__images__` is the first thing every `Pdf.__call__` does with the chosen + source, so it is exactly where the branch decision becomes observable. + `PdfParser.__init__` is stubbed out too: it loads the OCR and layout models + from the Hub, which this test neither needs nor should depend on. + """ + from deepdoc.parser import PdfParser + + captured: list = [] + + def capture(self, fnm, *args, **kwargs): + captured.append(fnm) + raise _Reached + + monkeypatch.setattr(PdfParser, "__init__", lambda self, *args, **kwargs: None) + monkeypatch.setattr(PdfParser, "__images__", capture) + return captured + + +@pytest.mark.p2 +@pytest.mark.parametrize("module_name", PDF_MODULES) +def test_pdf_empty_binary_reaches_ocr_with_bytes(module_name, captured_ocr_source): + """A 0-byte PDF must reach OCR as b"", not as the display name.""" + module = _load(module_name) + + with pytest.raises(_Reached): + module.Pdf()(PDF_NAME, b"", callback=_noop_callback) + + assert captured_ocr_source == [b""] + + +@pytest.mark.p2 +@pytest.mark.parametrize("module_name", PDF_CHUNK_MODULES) +def test_pdf_chunk_empty_binary_reaches_ocr_with_bytes(module_name, captured_ocr_source): + """Same guarantee through `chunk()`, which selects the source itself before + handing it to `Pdf.__call__`.""" + module = _load(module_name) + + with pytest.raises(_Reached): + module.chunk(PDF_NAME, binary=b"", callback=_noop_callback) + + assert captured_ocr_source == [b""] + + +@pytest.mark.p2 +@pytest.mark.parametrize("module_name", DOCX_MODULES) +def test_docx_empty_binary_does_not_open_display_name(module_name): + """python-docx must receive the empty bytes, so it reports a bad zip + instead of a package missing under the display name.""" + module = _load(module_name) + + with pytest.raises(BadZipFile, match="File is not a zip file"): + module.Docx()(DOCX_NAME, b"") + + +@pytest.mark.p2 +def test_qa_excel_empty_binary_loads_from_bytes(): + """openpyxl must receive the empty bytes, so the failure describes the + payload rather than a file missing under the display name.""" + qa = _load("qa") + + with pytest.raises(BadZipFile, match="File is not a zip file"): + qa.Excel()(XLSX_NAME, b"", callback=_noop_callback) + + +@pytest.mark.p2 +def test_table_excel_empty_binary_loads_from_bytes(): + """`_load_excel_to_workbook` sniffs the header and falls back to its CSV + path for anything that is not a zip/OLE container. Reaching that failure + proves it was handed the bytes and not the display name.""" + table = _load("table") + + with pytest.raises(Exception, match="Failed to parse CSV and convert to Excel Workbook"): + table.Excel()(XLSX_NAME, b"", callback=_noop_callback)