fix(docreader): drop data validation ranges openpyxl cannot parse instead of failing the sheet (#3599) (#3608)

* fix(docreader): drop data validation ranges openpyxl cannot parse instead of failing the sheet (#3599)

A Feishu sheet/bitable exported as .xlsx could fail to ingest with
"TypeError: expected <class 'openpyxl.worksheet.cell_range.MultiCellRange'>".
openpyxl reads a data validation's `sqref`, and a scenario list's, through a
converter that turns the real range-parse ValueError into that message and
aborts the whole workbook — in load_workbook (used by fill_merged_cells_xlsx)
and in pandas alike.

Measured on openpyxl 3.1.5, the version docreader locks: a whole column
(`C:C`), a whole row (`1:1`), a comma-separated list, `#REF!` and an
out-of-bounds range each raise it. Conditional formatting goes through the
same converter, but openpyxl already catches it there and drops the rule
with a warning, so it is left alone.

strip_unreadable_ranges_xlsx sits next to repair_xlsx_bytes and follows the
same contract: it returns rewritten bytes only when some range would fail
openpyxl's own MultiCellRange constructor — the exact step that raises — and
removes just those elements, logging how many. Every other workbook passes
through byte for byte. Neither element changes a cell's value, so dropping
one costs text extraction nothing, while keeping it cost the document.

* fix(docreader): strip unreadable ranges on the MarkItDown xlsx path too

MarkItDown reads spreadsheets through pandas/openpyxl, so a whole-column
data validation still failed that engine with FileConversionException
after the builtin engine was fixed (#3599). Run the same
strip_unreadable_ranges_xlsx pre-step before handing .xlsx to MarkItDown.

---------

Co-authored-by: wizardchen <wizardchen@tencent.com>

docreader/parser/excel_parser.py

@@ -21,7 +21,11 @@ from docreader.parser.excel_convert import (
     normalize_excel_bytes,
 )
 from docreader.parser.xlsx_merge import fill_merged_cells_xlsx
-from docreader.parser.xlsx_repair import repair_xlsx_bytes, sanitize_xlsx_styles
+from docreader.parser.xlsx_repair import (
+    repair_xlsx_bytes,
+    sanitize_xlsx_styles,
+    strip_unreadable_ranges_xlsx,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -205,6 +209,9 @@ def _prepare_xlsx_bytes(data: bytes) -> bytes:
     repaired = repair_xlsx_bytes(data)
     if repaired is not None:
         data = repaired
+    readable = strip_unreadable_ranges_xlsx(data)
+    if readable is not None:
+        data = readable
     try:
         return fill_merged_cells_xlsx(data)
     except TypeError:

docreader/parser/markitdown_parser.py

@@ -15,7 +15,10 @@ from docreader.parser.pptx_media import (
     attach_pptx_media_to_markdown,
     markdown_needs_pptx_media_attach,
 )
-from docreader.parser.xlsx_repair import sanitize_xlsx_styles
+from docreader.parser.xlsx_repair import (
+    sanitize_xlsx_styles,
+    strip_unreadable_ranges_xlsx,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -49,9 +52,14 @@ class StdMarkitdownParser(BaseParser):
             content = fill_vertical_merged_cells_docx(content)
         elif ft in ("xlsx", "xlsm"):
             # MarkItDown reads spreadsheets through pandas/openpyxl, so the
-            # same non-conforming styles.xml fills that break the builtin
-            # ExcelParser break this engine too (#3637). The sanitize pass
-            # scans only styles.xml and returns None for clean workbooks.
+            # ranges openpyxl cannot parse (#3599) and the non-conforming
+            # styles.xml fills (#3637) that break the builtin ExcelParser break
+            # this engine too. Strip the ranges first: the fill repair alone
+            # would still leave the range TypeError. Both passes return None
+            # for clean workbooks.
+            readable = strip_unreadable_ranges_xlsx(content)
+            if readable is not None:
+                content = readable
             sanitized = sanitize_xlsx_styles(content)
             if sanitized is not None:
                 content = sanitized

docreader/parser/xlsx_repair.py

@@ -48,6 +48,81 @@ def repair_xlsx_bytes(content: bytes) -> bytes | None:
         return _rewrite_zip(zin, _strip_shared_strings_manifest)
 
 
+# A data validation, or a scenario list, whose ``sqref`` openpyxl cannot parse
+# aborts the whole workbook: openpyxl reads that attribute through a converter that
+# turns the underlying range-parse ValueError into
+# "TypeError: expected <class 'MultiCellRange'>" (#3599). Measured on openpyxl
+# 3.1.5, a whole column (``C:C``), a whole row (``1:1``), a comma-separated
+# list, ``#REF!`` and an out-of-bounds range all do it, in load_workbook and in
+# pandas alike. Conditional formatting goes through the same converter, but
+# openpyxl already catches it there and drops the rule with a warning, so it
+# is not touched here. For scenarios the range sits on the ``<scenarios>``
+# container, not on each ``<scenario>``, so the container is what goes.
+_SQREF_ELEMENT_RE = re.compile(
+    r"<(?P<tag>(?:\w+:)?(?:dataValidation|scenarios))\b[^>]*?(?:/>|>.*?</(?P=tag)>)",
+    re.DOTALL,
+)
+_SQREF_ATTR_RE = re.compile(r'\bsqref="([^"]*)"')
+
+
+def strip_unreadable_ranges_xlsx(content: bytes) -> bytes | None:
+    """Drop data validations and scenario lists whose range openpyxl cannot parse.
+
+    Returns the rewritten bytes, or None when every such range is readable, in
+    which case the file is left exactly as it was. Neither element changes what
+    a cell contains — a validation constrains input, a scenario holds what-if
+    values that are not the displayed ones — so removing one costs text
+    extraction nothing, while keeping it costs the whole document.
+    """
+    if not zipfile.is_zipfile(io.BytesIO(content)):
+        return None
+
+    with zipfile.ZipFile(io.BytesIO(content), "r") as zin:
+        sheets = sorted(
+            name
+            for name in _normalized_names(zin.namelist())
+            if name.startswith("xl/worksheets/") and name.endswith(".xml")
+        )
+        dropped: Dict[str, int] = {}
+        for name in sheets:
+            sheet = zin.read(name).decode("utf-8", errors="replace")
+            count = sum(1 for m in _SQREF_ELEMENT_RE.finditer(sheet) if _unreadable(m.group(0)))
+            if count:
+                dropped[name] = count
+        if not dropped:
+            return None
+        logger.warning(
+            "Dropping %d data validation/scenario range(s) openpyxl cannot parse: %s",
+            sum(dropped.values()),
+            ", ".join(f"{name} ({n})" for name, n in dropped.items()),
+        )
+        return _rewrite_zip(zin, lambda files: _strip_unreadable_ranges(files, dropped))
+
+
+def _unreadable(element: str) -> bool:
+    """True when openpyxl would fail to build this element's range."""
+    from openpyxl.worksheet.cell_range import MultiCellRange
+
+    match = _SQREF_ATTR_RE.search(element)
+    if match is None:
+        return False
+    try:
+        MultiCellRange(match.group(1))
+    except Exception:  # noqa: BLE001 - mirrors openpyxl's own bare except
+        return True
+    return False
+
+
+def _strip_unreadable_ranges(files: Dict[str, bytes], sheets: Dict[str, int]) -> Dict[str, bytes]:
+    updated = dict(files)
+    for name in sheets:
+        sheet = updated[name].decode("utf-8")
+        sheet = _SQREF_ELEMENT_RE.sub(
+            lambda m: "" if _unreadable(m.group(0)) else m.group(0), sheet
+        )
+        updated[name] = sheet.encode("utf-8")
+    return updated
+
 def _normalized_names(namelist: Iterable[str]) -> Set[str]:
     return {name.replace("\\", "/") for name in namelist}
 

docreader/tests/test_excel_parser.py

@@ -13,8 +13,9 @@ from openpyxl.chart import BarChart, Reference
 
 from docreader.parser.excel_convert import detect_excel_format, engine_for_format
 from docreader.parser.excel_parser import ExcelParser
+from docreader.parser.markitdown_parser import StdMarkitdownParser
 from docreader.parser.xlsx_merge import fill_merged_cells_xlsx
-from docreader.parser.xlsx_repair import repair_xlsx_bytes
+from docreader.parser.xlsx_repair import repair_xlsx_bytes, strip_unreadable_ranges_xlsx
 
 
 def _xlsx_with_phantom_shared_strings() -> bytes:
@@ -51,6 +52,37 @@ def _xlsx_with_phantom_shared_strings() -> bytes:
         return out.getvalue()
 
 
+
+def _xlsx_with_sheet_fragment(fragment: str) -> bytes:
+    """A two-row workbook with ``fragment`` spliced into sheet1 before pageMargins,
+    which is where OOXML puts data validations and scenarios."""
+    wb = openpyxl.Workbook()
+    ws = wb.active
+    ws["A1"], ws["B1"] = "name", "qty"
+    ws["A2"], ws["B2"] = "apple", 3
+    bio = io.BytesIO()
+    wb.save(bio)
+
+    out = io.BytesIO()
+    with zipfile.ZipFile(io.BytesIO(bio.getvalue()), "r") as zin, zipfile.ZipFile(
+        out, "w", zipfile.ZIP_DEFLATED
+    ) as zout:
+        for info in zin.infolist():
+            data = zin.read(info.filename)
+            if info.filename == "xl/worksheets/sheet1.xml":
+                sheet = data.decode("utf-8")
+                at = sheet.index("<pageMargins")
+                data = (sheet[:at] + fragment + sheet[at:]).encode("utf-8")
+            zout.writestr(info, data)
+    return out.getvalue()
+
+
+def _validation(sqref: str, formula: str = '"a,b"') -> str:
+    return (
+        f'<dataValidation type="list" allowBlank="1" sqref="{sqref}">'
+        f"<formula1>{formula}</formula1></dataValidation>"
+    )
+
 class ExcelFormatDetectionTest(unittest.TestCase):
     def test_detect_xlsx_and_engine(self):
         wb = openpyxl.Workbook()
@@ -100,6 +132,83 @@ class ExcelFormatDetectionTest(unittest.TestCase):
         self.assertIn("legacy", document.content)
 
 
+
+class XlsxUnreadableRangesTest(unittest.TestCase):
+    """#3599: a range openpyxl cannot parse must not cost the whole document."""
+
+    def test_whole_column_validation_aborts_openpyxl_without_the_fix(self):
+        broken = _xlsx_with_sheet_fragment(
+            f'<dataValidations count="1">{_validation("C:C")}</dataValidations>'
+        )
+        with self.assertRaisesRegex(TypeError, "MultiCellRange"):
+            openpyxl.load_workbook(io.BytesIO(broken), data_only=True)
+        with self.assertRaisesRegex(TypeError, "MultiCellRange"):
+            pd.read_excel(io.BytesIO(broken), engine="openpyxl")
+
+    def test_drops_each_range_spelling_openpyxl_rejects(self):
+        # Measured on openpyxl 3.1.5; every one of these raised the reported
+        # TypeError before the fix.
+        for sqref in ("C:C", "1:1", "A1:B2,C3", "#REF!", "A1:XFD1048577"):
+            with self.subTest(sqref=sqref):
+                broken = _xlsx_with_sheet_fragment(
+                    f'<dataValidations count="1">{_validation(sqref)}</dataValidations>'
+                )
+                fixed = strip_unreadable_ranges_xlsx(broken)
+                self.assertIsNotNone(fixed)
+                df = pd.read_excel(io.BytesIO(fixed), header=None, engine="openpyxl")
+                self.assertEqual(df.values.tolist(), [["name", "qty"], ["apple", 3]])
+
+    def test_drops_a_scenario_list_with_an_unreadable_range(self):
+        broken = _xlsx_with_sheet_fragment(
+            # The range belongs to the list, not to each scenario.
+            '<scenarios sqref="A:A"><scenario name="s" count="1">'
+            '<inputCells r="A1" val="1"/></scenario></scenarios>'
+        )
+        with self.assertRaisesRegex(TypeError, "MultiCellRange"):
+            openpyxl.load_workbook(io.BytesIO(broken), data_only=True)
+        fixed = strip_unreadable_ranges_xlsx(broken)
+        self.assertIsNotNone(fixed)
+        openpyxl.load_workbook(io.BytesIO(fixed), data_only=True)
+
+    def test_keeps_a_readable_validation_next_to_an_unreadable_one(self):
+        broken = _xlsx_with_sheet_fragment(
+            '<dataValidations count="2">'
+            f'{_validation("C:C")}{_validation("D2:D10", chr(34) + "keep" + chr(34))}'
+            "</dataValidations>"
+        )
+        fixed = strip_unreadable_ranges_xlsx(broken)
+        self.assertIsNotNone(fixed)
+        ws = openpyxl.load_workbook(io.BytesIO(fixed)).active
+        self.assertEqual(
+            [str(dv.sqref) for dv in ws.data_validations.dataValidation], ["D2:D10"]
+        )
+
+    def test_leaves_a_readable_workbook_untouched(self):
+        fine = _xlsx_with_sheet_fragment(
+            f'<dataValidations count="1">{_validation("C2:C10")}</dataValidations>'
+        )
+        self.assertIsNone(strip_unreadable_ranges_xlsx(fine))
+        self.assertIsNone(strip_unreadable_ranges_xlsx(b"not a zip"))
+
+    def test_excel_parser_reads_a_workbook_with_a_whole_column_validation(self):
+        broken = _xlsx_with_sheet_fragment(
+            f'<dataValidations count="1">{_validation("C:C")}</dataValidations>'
+        )
+        document = ExcelParser(file_name="feishu.xlsx", file_type="xlsx").parse_into_text(
+            broken
+        )
+        self.assertIn("apple", document.content)
+
+    def test_markitdown_parser_reads_a_workbook_with_a_whole_column_validation(self):
+        broken = _xlsx_with_sheet_fragment(
+            f'<dataValidations count="1">{_validation("C:C")}</dataValidations>'
+        )
+        document = StdMarkitdownParser(
+            file_name="feishu.xlsx", file_type="xlsx"
+        ).parse_into_text(broken)
+        self.assertIn("apple", document.content)
+
+
 class XlsxRepairTest(unittest.TestCase):
     def test_repair_removes_phantom_shared_strings_reference(self):
         broken = _xlsx_with_phantom_shared_strings()