From bee3404ecdcf81c074b65ab552964d71ea246b5a Mon Sep 17 00:00:00 2001 From: Adam Fourney Date: Tue, 15 Sep 2026 16:37:29 -0700 Subject: [PATCH 1/6] Add _image_to_html helper for docx ocr conversion. --- .../_docx_converter_with_ocr.py | 257 ++------ .../tests/test_docx_converter.py | 80 +-- .../tests/test_docx_inheritance.py | 285 ++++++++ .../converter_utils/docx/_images.py | 116 ++++ .../markitdown/converters/_docx_converter.py | 32 + packages/markitdown/tests/test_docx_images.py | 606 ++++++++++++++++++ 6 files changed, 1106 insertions(+), 270 deletions(-) create mode 100644 packages/markitdown-ocr/tests/test_docx_inheritance.py create mode 100644 packages/markitdown/src/markitdown/converter_utils/docx/_images.py create mode 100644 packages/markitdown/tests/test_docx_images.py diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py b/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py index f0201689dc..28e6bbd912 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py @@ -1,228 +1,71 @@ -""" -Enhanced DOCX Converter with OCR support for embedded images. -Extracts images from Word documents and performs OCR while maintaining context. -""" +"""DOCX image OCR using the core document conversion pipeline.""" -import io -import re -import sys +import hashlib +import html from typing import Any, BinaryIO, Optional +from warnings import warn -from markitdown.converters import HtmlConverter -from markitdown.converter_utils.docx.pre_process import pre_process_docx from markitdown import DocumentConverterResult, StreamInfo -from markitdown._exceptions import ( - MissingDependencyException, - MISSING_DEPENDENCY_MESSAGE, -) -from ._ocr_service import LLMVisionOCRService - -# Try loading dependencies -_dependency_exc_info = None -try: - import mammoth - from docx import Document -except ImportError: - _dependency_exc_info = sys.exc_info() - -# Placeholder injected into HTML so that mammoth never sees the OCR markers. -# Must be a single token with no special markdown characters. -_PLACEHOLDER = "MARKITDOWNOCRBLOCK{}" +from markitdown.converters import DocxConverter -_UNDERLINE_STYLE_MAP = "u => u" - - -def _read_embedded_style_map(file_stream: BinaryIO) -> Optional[str]: - """Read the style map embedded in a .docx, if it has one.""" - position = file_stream.tell() - file_stream.seek(0) - try: - return mammoth.read_embedded_style_map(file_stream) - finally: - file_stream.seek(position) +from ._ocr_service import LLMVisionOCRService -class DocxConverterWithOCR(HtmlConverter): - """ - Enhanced DOCX Converter with OCR support for embedded images. - Maintains document flow while extracting text from images inline. - """ +class DocxConverterWithOCR(DocxConverter): + """Recognize embedded images while inheriting native DOCX conversion.""" def __init__(self, ocr_service: Optional[LLMVisionOCRService] = None): super().__init__() - self._html_converter = HtmlConverter() + if not hasattr(DocxConverter, "_image_to_html"): + raise RuntimeError( + "DOCX OCR requires the core DocxConverter._image_to_html hook. " + "Install markitdown and markitdown-ocr from the same source checkout." + ) self.ocr_service = ocr_service - def accepts( + def convert( self, file_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any, - ) -> bool: - mimetype = (stream_info.mimetype or "").lower() - extension = (stream_info.extension or "").lower() - - if extension == ".docx": - return True - - if mimetype.startswith( - "application/vnd.openxmlformats-officedocument.wordprocessingml" - ): - return True - - return False + ) -> DocumentConverterResult: + # Keep repeated-image recognition local to this document, not the instance. + kwargs["_docx_ocr_cache"] = {} + return super().convert(file_stream, stream_info, **kwargs) - def convert( + def _image_to_html( self, - file_stream: BinaryIO, + image_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any, - ) -> DocumentConverterResult: - if _dependency_exc_info is not None: - raise MissingDependencyException( - MISSING_DEPENDENCY_MESSAGE.format( - converter=type(self).__name__, - extension=".docx", - feature="docx", - ) - ) from _dependency_exc_info[1].with_traceback( - _dependency_exc_info[2] - ) # type: ignore[union-attr] - - # Get OCR service if available (from kwargs or instance) - ocr_service: Optional[LLMVisionOCRService] = ( - kwargs.get("ocr_service") or self.ocr_service - ) - - # Pre-process once, up front, so that every subsequent read sees the - # repaired archive. pre_process_docx() also fixes .docx files whose ZIP - # local file headers disagree with the central directory; reading the - # original stream first would make those raise, or silently yield no - # images and drop the OCR output. - pre_process_stream = pre_process_docx(file_stream) - - # Read the embedded style map and combine with any provided style map - caller_style_map = kwargs.get("style_map") - embedded_style_map = _read_embedded_style_map(pre_process_stream) - - style_map = "\n".join( - part - for part in ( - caller_style_map, - embedded_style_map, - _UNDERLINE_STYLE_MAP, - ) - if part - ) - - if ocr_service: - # 1. Extract and OCR images — returns raw text per image - pre_process_stream.seek(0) - image_ocr_map = self._extract_and_ocr_images( - pre_process_stream, ocr_service - ) - - # 2. Convert DOCX → HTML via mammoth - pre_process_stream.seek(0) - html_result = mammoth.convert_to_html( - pre_process_stream, - style_map=style_map, - include_embedded_style_map=False, - ).value - - # 3. Replace tags with plain placeholder tokens so that - # mammoth's HTML→markdown step never escapes our OCR markers. - html_with_placeholders, ocr_texts = self._inject_placeholders( - html_result, image_ocr_map - ) - - # 4. Convert HTML → markdown - md_result = self._html_converter.convert_string( - html_with_placeholders, **kwargs + ) -> Optional[str]: + ocr_service = kwargs.get("ocr_service") or self.ocr_service + if ocr_service is None: + return None + + cache: dict[bytes, Optional[str]] = kwargs.get("_docx_ocr_cache", {}) + key = hashlib.sha256(image_stream.read()).digest() + image_stream.seek(0) + if key in cache: + return cache[key] + + # Preserve compatibility with services accepting only an image stream. + result = ocr_service.extract_text(image_stream) + if result.error: + warn( + f"DOCX image OCR failed: {result.error}. Keeping the native image.", + RuntimeWarning, + stacklevel=2, ) - md = md_result.markdown - - # 5. Swap placeholders for the actual OCR blocks (post-conversion - # so * and _ are never escaped by the markdown converter). - for i, raw_text in enumerate(ocr_texts): - placeholder = _PLACEHOLDER.format(i) - ocr_block = f"*[Image OCR]\n{raw_text}\n[End OCR]*" - md = md.replace(placeholder, ocr_block) - - return DocumentConverterResult(markdown=md) - else: - # Standard conversion without OCR - pre_process_stream.seek(0) - return self._html_converter.convert_string( - mammoth.convert_to_html( - pre_process_stream, - style_map=style_map, - include_embedded_style_map=False, - ).value, - **kwargs, - ) - - def _extract_and_ocr_images( - self, file_stream: BinaryIO, ocr_service: LLMVisionOCRService - ) -> dict[str, str]: - """ - Extract images from DOCX and OCR them. - - Returns: - Dict mapping image relationship IDs to raw OCR text (no markers). - """ - ocr_map = {} - - try: - file_stream.seek(0) - doc = Document(file_stream) - - for rel in doc.part.rels.values(): - if "image" in rel.target_ref.lower(): - try: - image_bytes = rel.target_part.blob - image_stream = io.BytesIO(image_bytes) - ocr_result = ocr_service.extract_text(image_stream) - - if ocr_result.text.strip(): - # Store raw text only — markers added later - ocr_map[rel.rId] = ocr_result.text.strip() - - except Exception: - continue - - except Exception: - pass - - return ocr_map - - def _inject_placeholders( - self, html: str, ocr_map: dict[str, str] - ) -> tuple[str, list[str]]: - """ - Replace tags with numbered placeholder tokens. - - Returns: - (html_with_placeholders, ordered list of raw OCR texts) - """ - if not ocr_map: - return html, [] - - ocr_texts = list(ocr_map.values()) - used: list[int] = [] - - def replace_img(match: re.Match) -> str: # type: ignore[type-arg] - for i in range(len(ocr_texts)): - if i not in used: - used.append(i) - return f"

{_PLACEHOLDER.format(i)}

" - return "" # remove image if all OCR texts already used - - result = re.sub(r"]*>", replace_img, html) - - # Any OCR texts that had no matching tag go at the end - for i in range(len(ocr_texts)): - if i not in used: - result += f"

{_PLACEHOLDER.format(i)}

" - - return result, ocr_texts + cache[key] = None + return None + text = result.text.strip() + if not text: + cache[key] = None + return None + + text = text.replace("\r\n", "\n").replace("\r", "\n") + content = html.escape(text).replace("\n", "
") + fragment = f"

[Image OCR]
{content}
[End OCR]

" + cache[key] = fragment + return fragment diff --git a/packages/markitdown-ocr/tests/test_docx_converter.py b/packages/markitdown-ocr/tests/test_docx_converter.py index 8988c0334a..fc359d6c52 100644 --- a/packages/markitdown-ocr/tests/test_docx_converter.py +++ b/packages/markitdown-ocr/tests/test_docx_converter.py @@ -4,10 +4,8 @@ For each DOCX test file: convert with a mock OCR service then compare the full output string against the expected snapshot. -OCR block format used by the converter: - *[Image OCR] - MOCK_OCR_TEXT_12345 - [End OCR]* +OCR blocks pass through the shared HTML converter, including literal-text +escaping and two-space Markdown hard breaks in direct conversion. """ import io @@ -30,6 +28,7 @@ TEST_DATA_DIR = Path(__file__).parent / "ocr_test_data" _MOCK_TEXT = "MOCK_OCR_TEXT_12345" +_MOCK_BLOCK = "*[Image OCR] \nMOCK\\_OCR\\_TEXT\\_12345 \n[End OCR]*" class MockOCRService: @@ -52,7 +51,7 @@ def _convert(filename: str, ocr_service: MockOCRService) -> str: with open(path, "rb") as f: return converter.convert( f, StreamInfo(extension=".docx"), ocr_service=ocr_service - ).text_content + ).markdown # --------------------------------------------------------------------------- @@ -63,7 +62,7 @@ def _convert(filename: str, ocr_service: MockOCRService) -> str: def test_docx_image_start(svc: MockOCRService) -> None: expected = ( "Document with Image at Start\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" + f"{_MOCK_BLOCK}\n\n" "This is the main content after the header image.\n\n" "More text content here." ) @@ -80,7 +79,7 @@ def test_docx_image_middle(svc: MockOCRService) -> None: "# Introduction\n\n" "This is the introduction section.\n\n" "We will see an image below.\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" + f"{_MOCK_BLOCK}\n\n" "# Analysis\n\n" "This section comes after the image." ) @@ -98,7 +97,7 @@ def test_docx_image_end(svc: MockOCRService) -> None: "Main findings of the report.\n\n" "Details and analysis.\n\n" "Recommendations.\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + f"{_MOCK_BLOCK}" ) assert _convert("docx_image_end.docx", svc) == expected @@ -112,9 +111,9 @@ def test_docx_multiple_images(svc: MockOCRService) -> None: expected = ( "Multi-Image Document\n\n" "First section\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" + f"{_MOCK_BLOCK}\n\n" "Second section with another image\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" + f"{_MOCK_BLOCK}\n\n" "Conclusion" ) assert _convert("docx_multiple_images.docx", svc) == expected @@ -130,7 +129,7 @@ def test_docx_multipage(svc: MockOCRService) -> None: "# Page 1 - Mixed Content\n\n" "This is the first paragraph on page 1.\n\n" "BEFORE IMAGE: Important content appears here.\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" + f"{_MOCK_BLOCK}\n\n" "AFTER IMAGE: This content follows the image.\n\n" "More text on page 1.\n\n" "# Page 2 - Image at End\n\n" @@ -138,9 +137,9 @@ def test_docx_multipage(svc: MockOCRService) -> None: "Multiple paragraphs of text.\n\n" "Building up to the image...\n\n" "Final paragraph before image.\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" + f"{_MOCK_BLOCK}\n\n" "# Page 3 - Image at Start\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" + f"{_MOCK_BLOCK}\n\n" "Content that follows the header image.\n\n" "AFTER IMAGE: This text is after the image." ) @@ -161,55 +160,11 @@ def test_docx_complex_layout(svc: MockOCRService) -> None: "| Authentication | Active |\n" "| Encryption | Enabled |\n\n" "Security notice:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + f"{_MOCK_BLOCK}" ) assert _convert("docx_complex_layout.docx", svc) == expected -# --------------------------------------------------------------------------- -# _inject_placeholders — internal unit tests (no file I/O) -# --------------------------------------------------------------------------- - - -def test_inject_placeholders_single_image() -> None: - converter = DocxConverterWithOCR() - html = "

Before

After

" - result_html, texts = converter._inject_placeholders(html, {"rId1": "TEXT"}) - assert " None: - converter = DocxConverterWithOCR() - html = "

Mid

" - result_html, texts = converter._inject_placeholders( - html, {"rId1": "FIRST", "rId2": "SECOND"} - ) - assert "MARKITDOWNOCRBLOCK0" in result_html - assert "MARKITDOWNOCRBLOCK1" in result_html - assert result_html.index("MARKITDOWNOCRBLOCK0") < result_html.index( - "MARKITDOWNOCRBLOCK1" - ) - assert len(texts) == 2 - - -def test_inject_placeholders_no_img_tag_appends_at_end() -> None: - converter = DocxConverterWithOCR() - html = "

No images

" - result_html, texts = converter._inject_placeholders(html, {"rId1": "ORPHAN"}) - assert "MARKITDOWNOCRBLOCK0" in result_html - assert texts == ["ORPHAN"] - - -def test_inject_placeholders_empty_map_leaves_html_unchanged() -> None: - converter = DocxConverterWithOCR() - html = "

Content

" - result_html, texts = converter._inject_placeholders(html, {}) - assert result_html == html - assert texts == [] - - # --------------------------------------------------------------------------- # No OCR service — no OCR tags emitted # --------------------------------------------------------------------------- @@ -221,7 +176,7 @@ def test_docx_no_ocr_service_no_tags() -> None: pytest.skip(f"Test file not found: {path}") converter = DocxConverterWithOCR() with open(path, "rb") as f: - md = converter.convert(f, StreamInfo(extension=".docx")).text_content + md = converter.convert(f, StreamInfo(extension=".docx")).markdown assert "*[Image OCR]" not in md assert "[End OCR]*" not in md @@ -273,7 +228,7 @@ def test_docx_styles_with_redundant_default_namespace( assert "# Introduction" in actual assert actual == expected if use_ocr: - assert _MOCK_TEXT in actual + assert _MOCK_BLOCK in actual # --------------------------------------------------------------------------- @@ -399,8 +354,7 @@ def test_docx_zip_filename_casing_mismatch_preserves_ocr(svc: MockOCRService) -> """OCR output survives a .docx whose local file headers disagree with the central directory on casing. - The image extraction swallows every exception, so an unrepaired stream used - to yield an empty OCR map and silently drop the OCR blocks rather than fail. + The inherited core converter repairs the archive before Mammoth opens images. """ path = TEST_DATA_DIR / "docx_image_middle.docx" if not path.exists(): @@ -424,5 +378,5 @@ def test_docx_zip_filename_casing_mismatch_preserves_ocr(svc: MockOCRService) -> io.BytesIO(mismatched), StreamInfo(extension=".docx"), ocr_service=svc ).markdown - assert _MOCK_TEXT in actual + assert _MOCK_BLOCK in actual assert actual == expected diff --git a/packages/markitdown-ocr/tests/test_docx_inheritance.py b/packages/markitdown-ocr/tests/test_docx_inheritance.py new file mode 100644 index 0000000000..84f1a2b36a --- /dev/null +++ b/packages/markitdown-ocr/tests/test_docx_inheritance.py @@ -0,0 +1,285 @@ +"""Full DOCX OCR trips through the inherited core image/HTML pipeline.""" + +import base64 +import inspect +import io +from typing import Any +from unittest.mock import Mock + +from bs4 import BeautifulSoup +from docx import Document +from PIL import Image +import pytest + +from markitdown import FileConversionException, MarkItDown, StreamInfo +from markitdown.converters import DocxConverter +from markitdown.converters import _docx_converter +import markitdown._markitdown as markitdown_module +from markitdown_ocr import _plugin +from markitdown_ocr._docx_converter_with_ocr import DocxConverterWithOCR +from markitdown_ocr._ocr_service import OCRResult + + +_INFO = StreamInfo(extension=".docx") + + +def _png(color: str) -> bytes: + output = io.BytesIO() + Image.new("RGB", (2, 2), color).save(output, format="PNG") + return output.getvalue() + + +_RED = _png("red") +_BLUE = _png("blue") + + +def _document( + image_data: tuple[bytes, ...] = (_RED,), + *, + in_table: bool = False, + inline: bool = False, +) -> bytes: + document = Document() + document.add_heading("Heading", level=1) + for data in image_data: + if in_table: + table = document.add_table(rows=2, cols=2) + table.cell(0, 0).text = "Item" + table.cell(0, 1).text = "Details" + table.cell(1, 0).text = "A" + paragraph = table.cell(1, 1).paragraphs[0] + else: + paragraph = document.add_paragraph() + if inline: + paragraph.add_run("Before ") + paragraph.add_run().add_picture(io.BytesIO(data)) + if inline: + paragraph.add_run(" After") + document.add_paragraph("Native content").runs[0].bold = True + output = io.BytesIO() + document.save(output) + return output.getvalue() + + +def _service(text: str = "recognized") -> Mock: + # A one-argument service must remain supported, without injected keywords. + return Mock(extract_text=Mock(side_effect=lambda stream: OCRResult(text=text))) + + +def _convert(converter: DocxConverter, data: bytes, **kwargs: Any) -> str: + return converter.convert(io.BytesIO(data), _INFO, **kwargs).markdown + + +def test_docx_ocr_is_a_thin_subclass() -> None: + assert issubclass(DocxConverterWithOCR, DocxConverter) + assert DocxConverterWithOCR.accepts is DocxConverter.accepts + assert inspect.signature(DocxConverterWithOCR.convert) == inspect.signature( + DocxConverter.convert + ) + assert not hasattr(DocxConverterWithOCR, "_inject_placeholders") + assert not hasattr(DocxConverterWithOCR, "_extract_and_ocr_images") + + +def test_plugin_registration_full_trip(monkeypatch: pytest.MonkeyPatch) -> None: + entry_point = Mock() + entry_point.load.return_value = _plugin + entry_points = Mock(return_value=[entry_point]) + monkeypatch.setattr(markitdown_module, "entry_points", entry_points) + monkeypatch.setattr(markitdown_module, "_plugins", None) + client = Mock() + client.chat.completions.create.return_value.choices = [ + Mock(message=Mock(content="Recognized_text")) + ] + md = MarkItDown( + enable_plugins=True, + llm_client=client, + llm_model="vision-model", + llm_prompt="Read the text", + ) + registered = [ + registration + for registration in md._converters + if isinstance(registration.converter, DocxConverterWithOCR) + ] + assert len(registered) == 1 and registered[0].priority == -1 + + result = md.convert_stream(io.BytesIO(_document(inline=True)), stream_info=_INFO) + + assert result.markdown == ( + "# Heading\n\nBefore\n\n" + "*[Image OCR]\nRecognized\\_text\n[End OCR]*\n\n" + "After\n\n**Native content**" + ) + entry_points.assert_called_once_with(group="markitdown.plugin") + client.chat.completions.create.assert_called_once() + request = client.chat.completions.create.call_args.kwargs + assert request["model"] == "vision-model" + content = request["messages"][0]["content"] + assert content[0]["text"] == "Read the text" + assert content[1]["image_url"]["url"] == ( + "data:image/png;base64," + base64.b64encode(_RED).decode("ascii") + ) + + +@pytest.mark.parametrize("keep_data_uris", [False, True]) +@pytest.mark.parametrize("fallback", [None, "", " \n\t"]) +def test_no_service_or_empty_text_matches_core( + keep_data_uris: bool, fallback: str | None +) -> None: + service = None if fallback is None else _service(fallback) + data = _document(inline=True) + expected = _convert(DocxConverter(), data, keep_data_uris=keep_data_uris) + + actual = _convert( + DocxConverterWithOCR(ocr_service=service), + data, + keep_data_uris=keep_data_uris, + ) + + assert actual == expected + + +def test_recognition_cache_is_local_and_service_override_is_preserved() -> None: + first = _service("first") + second = _service("second") + converter = DocxConverterWithOCR(ocr_service=first) + data = _document((_RED,) * 12) + + default = _convert(converter, data) + overridden = _convert(converter, data, ocr_service=second) + again = _convert(converter, data) + + assert default.count("*[Image OCR] \nfirst \n[End OCR]*") == 12 + assert overridden.count("*[Image OCR] \nsecond \n[End OCR]*") == 12 + assert again == default + assert first.extract_text.call_count == 2 + second.extract_text.assert_called_once() + + +def test_image_identity_not_relationship_order_and_failed_images_stay_native() -> None: + document = Document(io.BytesIO(_document((_RED, _BLUE, _RED)))) + red, blue = document.paragraphs[1:3] + red._p.addprevious(blue._p) + stream = io.BytesIO() + document.save(stream) + calls = [] + + def recognize(image_stream): + image = image_stream.read() + calls.append(image) + return OCRResult(text="blue" if image == _BLUE else "") + + service = Mock(extract_text=Mock(side_effect=recognize)) + result = _convert(DocxConverterWithOCR(service), stream.getvalue()) + + assert calls == [_BLUE, _RED] + assert result.count("*[Image OCR]") == 1 + assert result.count("data:image/png;base64...") == 2 + assert result.index("blue") < result.index("data:image/png;base64...") + + +def test_ocr_text_is_escaped_and_line_endings_are_preserved_as_html_breaks( + monkeypatch: pytest.MonkeyPatch, +) -> None: + service = _service("A_B *literal*\r\n & value\rfinal") + converter = DocxConverterWithOCR(service) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + result = _convert(converter, _document()) + + assert ( + "

[Image OCR]
A_B *literal*
<tag> & value" + "
final
[End OCR]

" + ) in convert_html.call_args.args[0] + assert ( + "*[Image OCR] \nA\\_B \\*literal\\* \n" " & value \nfinal \n[End OCR]*" + ) in result + assert "data-markitdown-image-" not in convert_html.call_args.args[0] + + +def test_ocr_inside_a_table_reaches_html_conversion_inside_the_cell( + monkeypatch: pytest.MonkeyPatch, +) -> None: + converter = DocxConverterWithOCR(_service("Serial: 12345\nStatus: active")) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + result = _convert(converter, _document(in_table=True)) + + soup = BeautifulSoup(convert_html.call_args.args[0], "html.parser") + assert len(soup.find_all("td")) == 4 + assert str(soup.find_all("td")[-1]) == ( + "

[Image OCR]
Serial: 12345
" + "Status: active
[End OCR]

" + ) + assert not soup.select("p p") + assert "| A | *[Image OCR] Serial: 12345 Status: active [End OCR]* |" in result + assert "**Native content**" in result + + +def test_style_maps_and_html_options_are_inherited() -> None: + document = Document(io.BytesIO(_document())) + document.add_paragraph().add_run("underlined").underline = True + stream = io.BytesIO() + document.save(stream) + + result = _convert( + DocxConverterWithOCR(_service("OCR_text")), + stream.getvalue(), + style_map="u => strong", + escape_underscores=False, + heading_style="underlined", + ) + + assert result.startswith("Heading\n=======") + assert "OCR_text" in result + assert "**underlined**" in result + + +def test_reported_ocr_error_warns_and_keeps_native_images() -> None: + service = Mock( + extract_text=Mock(return_value=OCRResult(text="", error="quota exceeded")) + ) + data = _document((_RED, _RED)) + + with pytest.warns(RuntimeWarning, match="quota exceeded") as warnings: + result = _convert(DocxConverterWithOCR(service), data) + + assert len(warnings) == 1 + assert result == _convert(DocxConverter(), data) + service.extract_text.assert_called_once() + + +def test_raised_service_error_uses_normal_dispatcher_fallback() -> None: + error = RuntimeError("custom service failed") + service = Mock(extract_text=Mock(side_effect=error)) + converter = DocxConverterWithOCR(service) + data = _document() + with pytest.raises(RuntimeError) as caught: + _convert(converter, data) + assert caught.value is error + + md = MarkItDown() + md.register_converter(converter, priority=-1) + assert md.convert_stream(io.BytesIO(data), stream_info=_INFO).markdown == _convert( + DocxConverter(), data + ) + without_fallback = MarkItDown(enable_builtins=False) + without_fallback.register_converter(converter, priority=-1) + with pytest.raises(FileConversionException) as aggregate: + without_fallback.convert_stream(io.BytesIO(data), stream_info=_INFO) + assert aggregate.value.attempts is not None + assert any( + attempt.exc_info and attempt.exc_info[1] is error + for attempt in aggregate.value.attempts + ) + + +def test_older_core_is_rejected_instead_of_silently_skipping_ocr( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delattr(_docx_converter.DocxConverter, "_image_to_html") + + with pytest.raises(RuntimeError, match="same source checkout"): + DocxConverterWithOCR() diff --git a/packages/markitdown/src/markitdown/converter_utils/docx/_images.py b/packages/markitdown/src/markitdown/converter_utils/docx/_images.py new file mode 100644 index 0000000000..a873046709 --- /dev/null +++ b/packages/markitdown/src/markitdown/converter_utils/docx/_images.py @@ -0,0 +1,116 @@ +"""Internal bridge from the DOCX image hook to document HTML.""" + +import mimetypes +from typing import Any, Callable, Optional +from uuid import uuid4 + +from bs4 import BeautifulSoup, Doctype, Tag +from bs4.builder import HTMLTreeBuilder +from mammoth import html, images +from mammoth.docx.files import InvalidFileReferenceError + +from ..._stream_info import StreamInfo + + +_BLOCK_ELEMENTS = HTMLTreeBuilder.DEFAULT_BLOCK_ELEMENTS | { + "details", + "dialog", + "hgroup", + "menu", + "search", + "summary", +} +_PHRASING_CONTAINERS = set( + "a abbr b bdi bdo cite code data del dfn em i ins kbd label mark q s samp " + "small span strong sub sup time u var p h1 h2 h3 h4 h5 h6 pre".split() +) + + +def _lift_out_of_parent(node: Tag, soup: BeautifulSoup) -> None: + """Split a paragraph/inline wrapper around generated block content.""" + parent = node.parent + assert isinstance(parent, Tag) + before = soup.new_tag(parent.name) + before.attrs.update(parent.attrs) + for sibling in list(parent.contents): + if sibling is node: + break + before.append(sibling.extract()) + if before.contents: + parent.insert_before(before) + parent.attrs.pop("id", None) + parent.insert_before(node.extract()) + if not parent.contents: + if parent.has_attr("id") and not node.has_attr("id"): + node["id"] = parent["id"] + parent.decompose() + + +class _DocxImages: + def __init__( + self, + render: Callable[..., Optional[str]], + options: dict[str, Any], + ): + self._render = render + self._options = options + self._attribute = "data-markitdown-image-" + uuid4().hex + self._fragments: dict[str, BeautifulSoup] = {} + + def convert_image(self, image: Any) -> list[Any]: + stream_info = StreamInfo( + mimetype=image.content_type, + extension=( + mimetypes.guess_extension(image.content_type) + if image.content_type + else None + ), + ) + with image.open() as image_stream: + try: + fragment = self._render(image_stream, stream_info, **self._options) + except InvalidFileReferenceError as exc: + # Mammoth swallows this exception for missing document images, + # but an error raised by the override must reach the dispatcher. + raise RuntimeError("_image_to_html failed") from exc + if fragment is not None and not isinstance(fragment, str): + raise TypeError("_image_to_html must return an HTML string or None") + if fragment is None or not fragment.strip(): + return images.data_uri(image) + + soup = BeautifulSoup(fragment, "html.parser") + if soup.find(["html", "head", "body"]) or any( + isinstance(node, Doctype) for node in soup.descendants + ): + raise ValueError("_image_to_html must return a fragment, not a document") + key = str(len(self._fragments)) + self._fragments[key] = soup + return [html.element("img", {self._attribute: key})] + + def replace_images(self, html_content: str) -> str: + if not self._fragments: + return html_content + + soup = BeautifulSoup(html_content, "html.parser") + for image in soup.find_all("img", attrs={self._attribute: True}): + key = image[self._attribute] + assert isinstance(key, str) + fragment = self._fragments[key] + blocks = fragment.find_all(_BLOCK_ELEMENTS) + links = fragment.find_all("a") + image.replace_with(*list(fragment.contents)) + + # Only generated content is lifted, never its table cell or list item. + for block in blocks: + while ( + isinstance(block.parent, Tag) + and block.parent.name in _PHRASING_CONTAINERS + ): + _lift_out_of_parent(block, soup) + for link in links: + outer_link = link.find_parent("a") + if outer_link is not None: + while link.parent is not outer_link: + _lift_out_of_parent(link, soup) + _lift_out_of_parent(link, soup) + return str(soup) diff --git a/packages/markitdown/src/markitdown/converters/_docx_converter.py b/packages/markitdown/src/markitdown/converters/_docx_converter.py index 8248417a65..8e5a736b8c 100644 --- a/packages/markitdown/src/markitdown/converters/_docx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_docx_converter.py @@ -102,10 +102,42 @@ def convert( if part ) + image_adapter = None + mammoth_kwargs: dict[str, Any] = {} + if type(self)._image_to_html is not DocxConverter._image_to_html: + from ..converter_utils.docx._images import _DocxImages + + image_adapter = _DocxImages(self._image_to_html, kwargs) + mammoth_kwargs["convert_image"] = image_adapter.convert_image + html_result = mammoth.convert_to_html( pre_process_stream, style_map=style_map, include_embedded_style_map=False, + **mammoth_kwargs, ).value + if image_adapter is not None: + html_result = image_adapter.replace_images(html_result) + return self._html_converter.convert_string(html_result, **kwargs) + + def _image_to_html( + self, + image_stream: BinaryIO, + stream_info: StreamInfo, + **kwargs: Any, + ) -> Optional[str]: + """Override to render an embedded image as an HTML fragment. + + The stream is borrowed, seekable, and positioned at zero; do not close + or retain it. StreamInfo describes the image, not the document. Existing + conversion options are forwarded through kwargs. + + Return None or blank text to retain the native image representation. + Otherwise return HTML, escaping any literal text. Inline HTML stays + inline; block HTML splits enclosing paragraph/inline wrappers but stays + inside its table cell or list item. Hook failures propagate through the + normal conversion failure path. + """ + return None diff --git a/packages/markitdown/tests/test_docx_images.py b/packages/markitdown/tests/test_docx_images.py new file mode 100644 index 0000000000..9d19161130 --- /dev/null +++ b/packages/markitdown/tests/test_docx_images.py @@ -0,0 +1,606 @@ +"""DOCX subclasses customize image HTML without replacing document conversion.""" + +import base64 +import inspect +import io +from pathlib import Path +from typing import Any, BinaryIO, Callable, Optional +from unittest.mock import Mock +import zipfile + +from bs4 import BeautifulSoup +import mammoth +from mammoth.docx.files import InvalidFileReferenceError +import pytest + +from markitdown import ( + FileConversionException, + MarkItDown, + MissingDependencyException, + StreamInfo, +) +from markitdown.converters import DocxConverter, HtmlConverter +from markitdown.converters import _docx_converter + + +_PNG = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAM" + "BAQDJ/pLvAAAAAElFTkSuQmCC" +) +_GIF = base64.b64decode("R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBTAA7") +_INFO = StreamInfo(extension=".docx") +_FILES = Path(__file__).parent / "test_files" + + +def _image(rid: str = "rIdPng", number: int = 1) -> str: + return f""" + + + + + + + + + + + +""" + + +def _text(text: str) -> str: + return f'{text}' + + +def _paragraph(content: str) -> str: + return f"{content}" + + +_BODY = _paragraph(_text("Before") + _image() + _text("After")) + + +def _docx(body: str = _BODY, *, embedded_style_map: Optional[str] = None) -> io.BytesIO: + stream = io.BytesIO() + with zipfile.ZipFile(stream, "w") as archive: + archive.writestr( + "[Content_Types].xml", + """ + + + + + +""", + ) + archive.writestr( + "_rels/.rels", + """ + +""", + ) + archive.writestr( + "word/_rels/document.xml.rels", + """ + + +""", + ) + archive.writestr( + "word/document.xml", + f""" + {body} +""", + ) + archive.writestr("word/media/image.png", _PNG) + archive.writestr("word/media/image.gif", _GIF) + if embedded_style_map is not None: + stream.seek(0) + mammoth.embed_style_map(stream, embedded_style_map) + stream.seek(0) + return stream + + +class _ImageConverter(DocxConverter): + def __init__(self, render: Callable[..., Optional[str]]): + super().__init__() + self.render = render + + def _image_to_html( + self, image_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any + ) -> Optional[str]: + return self.render(image_stream, stream_info, **kwargs) + + +def test_image_hook_does_not_change_public_conversion_signature() -> None: + assert list(inspect.signature(DocxConverter.convert).parameters) == [ + "self", + "file_stream", + "stream_info", + "kwargs", + ] + assert _ImageConverter.convert is DocxConverter.convert + assert list(inspect.signature(DocxConverter.__init__).parameters) == ["self"] + + +@pytest.mark.parametrize("via_dispatcher", [False, True]) +def test_inherited_hook_receives_images_and_options_in_order( + via_dispatcher: bool, +) -> None: + seen = [] + streams = [] + service = object() + options: dict[str, Any] = { + "ocr_service": service, + "heading_style": "underlined", + } + + def render(stream: BinaryIO, info: StreamInfo, **kwargs: Any) -> str: + assert stream.tell() == 0 + data = stream.read() + stream.seek(0) + assert stream.read() == data + seen.append((data, info, kwargs)) + streams.append(stream) + return f"image{len(seen)}" + + class InheritedImages(_ImageConverter): + pass + + converter = InheritedImages(render) + source = _docx( + _paragraph( + _image("rIdGif", 1) + + _text(" ") + + _image("rIdPng", 2) + + _text(" ") + + _image("rIdGif", 3) + ) + ) + original = source.getvalue() + info = StreamInfo( + extension=".docx", + filename="document.docx", + url="https://example.test/document.docx", + local_path="/document.docx", + ) + if via_dispatcher: + markitdown = MarkItDown() + markitdown.register_converter(converter, priority=-1) + result = markitdown.convert_stream(source, stream_info=info, **options) + else: + source.seek(7) + result = converter.convert(source, info, **options) + + assert [entry[:2] for entry in seen] == [ + (_GIF, StreamInfo(mimetype="image/gif", extension=".gif")), + (_PNG, StreamInfo(mimetype="image/png", extension=".png")), + (_GIF, StreamInfo(mimetype="image/gif", extension=".gif")), + ] + assert all(entry[2]["ocr_service"] is service for entry in seen) + assert all(entry[2]["heading_style"] == "underlined" for entry in seen) + assert result.markdown == "**image1** **image2** **image3**" + assert all(stream.closed for stream in streams) + assert not source.closed and source.getvalue() == original + + +@pytest.mark.parametrize("keep_data_uris", [False, True]) +@pytest.mark.parametrize("fallback", [None, "", " \n\t"]) +def test_declining_hook_preserves_native_output( + monkeypatch: pytest.MonkeyPatch, fallback: Optional[str], keep_data_uris: bool +) -> None: + expected_converter = DocxConverter() + expected_html = Mock(wraps=expected_converter._html_converter.convert_string) + monkeypatch.setattr( + expected_converter._html_converter, "convert_string", expected_html + ) + expected = expected_converter.convert(_docx(), _INFO, keep_data_uris=keep_data_uris) + converter = _ImageConverter(Mock(return_value=fallback)) + actual_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", actual_html) + + actual = converter.convert(_docx(), _INFO, keep_data_uris=keep_data_uris) + + assert actual_html.call_args == expected_html.call_args + assert actual.markdown == expected.markdown + assert actual.title == expected.title + + +def test_native_converter_does_not_add_image_processing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + convert_html = Mock(wraps=mammoth.convert_to_html) + monkeypatch.setattr(mammoth, "convert_to_html", convert_html) + + class UnmodifiedSubclass(DocxConverter): + pass + + for converter in (DocxConverter(), UnmodifiedSubclass()): + result = converter.convert(_docx(), _INFO) + assert "![Image 1](data:image/png;base64...)" in result.markdown + assert "convert_image" not in convert_html.call_args.kwargs + + +def test_override_can_delegate_to_super() -> None: + class NativeImages(DocxConverter): + def _image_to_html(self, image_stream, stream_info, **kwargs): + return super()._image_to_html(image_stream, stream_info, **kwargs) + + actual = NativeImages().convert(_docx(), _INFO) + assert actual.markdown == DocxConverter().convert(_docx(), _INFO).markdown + + +@pytest.mark.parametrize( + ("fragment", "expected"), + [ + ("text", "

BeforetextAfter

"), + ("one
two", "

Beforeone
twoAfter

"), + ("A & B < C", "

BeforeA & B < CAfter

"), + ( + "

one

two

", + "

Before

one

two

After

", + ), + ( + "

one

two

", + "

Before

one

two

After

", + ), + ( + "
  • one
  • two
", + "

Before

  • one
  • two

After

", + ), + ( + "
Key
Value
", + "

Before

" + "
Key
Value

After

", + ), + ( + "
Title

Body

", + "

Before

Title

Body

" + "

After

", + ), + ( + "lead

block

tail", + "

Beforelead

block

tailAfter

", + ), + ( + "
a < b\n  *literal*
", + "

Before

a < b\n  *literal*

After

", + ), + ], +) +def test_fragment_placement_precedes_shared_html_conversion( + monkeypatch: pytest.MonkeyPatch, fragment: str, expected: str +) -> None: + converter = _ImageConverter(Mock(return_value=fragment)) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + options = {"escape_asterisks": False, "custom_option": "forwarded"} + + result = converter.convert(_docx(), _INFO, **options) + + assert convert_html.call_args.args == (expected,) + assert convert_html.call_args.kwargs == options + assert ( + result.markdown + == HtmlConverter().convert_string(expected, escape_asterisks=False).markdown + ) + assert "data-markitdown-image" not in expected + + +@pytest.mark.parametrize( + ("style_map", "expected"), + [ + (None, "

one

two

"), + ("p => ul > li:fresh", "
  • one

    two

"), + ("p => blockquote > p:fresh", "

one

two

"), + ("p => h2:fresh > strong", "

one

two

"), + ], +) +def test_standalone_block_image_does_not_leave_empty_wrappers( + monkeypatch: pytest.MonkeyPatch, style_map: Optional[str], expected: str +) -> None: + converter = _ImageConverter(Mock(return_value="

one

two

")) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + converter.convert(_docx(_paragraph(_image())), _INFO, style_map=style_map) + + assert convert_html.call_args.args == (expected,) + + +def test_block_image_keeps_its_table_cell( + monkeypatch: pytest.MonkeyPatch, +) -> None: + body = ( + "" + "" + + _paragraph(_text("Item")) + + "" + + _paragraph(_text("Details")) + + "" + + _paragraph(_text("A")) + + "" + + _paragraph(_image()) + + "" + ) + converter = _ImageConverter( + Mock(return_value="

Serial: 12345

Status: active

") + ) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + result = converter.convert(_docx(body), _INFO) + + soup = BeautifulSoup(convert_html.call_args.args[0], "html.parser") + assert len(soup.find_all("tr")) == 2 + assert len(soup.find_all("td")) == 4 + assert ( + str(soup.find_all("td")[-1]) + == "

Serial: 12345

Status: active

" + ) + assert not soup.select("p p") + assert "| A | Serial: 12345 Status: active |" in result.markdown + + +def test_nested_run_formatting_is_preserved_around_blocks( + monkeypatch: pytest.MonkeyPatch, +) -> None: + converter = _ImageConverter(Mock(return_value="

block

")) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + converter.convert(_docx(), _INFO, style_map="p => p:fresh > em > strong") + + assert convert_html.call_args.args == ( + "

Before

block

" + "

After

", + ) + + +def test_hook_inherits_preprocessing_and_style_precedence() -> None: + body = ( + _BODY + + _paragraph( + 'underlined' + ) + + _paragraph("deleted") + + _paragraph("x") + ) + + result = _ImageConverter(Mock(return_value="image")).convert( + _docx(body, embedded_style_map="u => em"), + _INFO, + style_map="u => strong", + ) + + assert result.markdown == "BeforeimageAfter\n\n**underlined**\n\n~~deleted~~\n\n$x$" + + +def test_multiple_images_and_native_fallback_keep_their_own_positions() -> None: + converter = _ImageConverter(Mock(side_effect=[None, "recognized", ""])) + body = _paragraph( + _image(number=1) + _text(" ") + _image(number=2) + _text(" ") + _image(number=3) + ) + + result = converter.convert(_docx(body), _INFO) + + assert result.markdown == ( + "![Image 1](data:image/png;base64...) *recognized* " + "![Image 3](data:image/png;base64...)" + ) + + +def test_many_images_have_independent_replacements() -> None: + converter = _ImageConverter( + Mock(side_effect=[f"

Image {number}

" for number in range(12)]) + ) + body = _paragraph("".join(_image(number=number) for number in range(12))) + + result = converter.convert(_docx(body), _INFO) + + assert result.markdown == "\n\n".join(f"Image {number}" for number in range(12)) + + +def test_returned_image_and_literal_text_use_normal_html_rendering() -> None: + render = Mock( + return_value=( + "

<literal> *not emphasis* & text

" + 'generated' + ) + ) + result = _ImageConverter(render).convert(_docx(_paragraph(_image())), _INFO) + + assert ( + result.markdown + == r" \*not emphasis\* & text" + "\n\n![generated](image.png)" + ) + render.assert_called_once() + + +@pytest.mark.parametrize("result", [False, 123, b"

not a string

"]) +def test_invalid_return_is_not_silent_native_fallback(result: Any) -> None: + with pytest.raises(TypeError, match="HTML string or None"): + _ImageConverter(Mock(return_value=result)).convert(_docx(), _INFO) + + +@pytest.mark.parametrize( + "fragment", + [ + "document", + "title", + "content", + "

content

", + ], +) +def test_full_document_is_not_a_valid_fragment(fragment: str) -> None: + with pytest.raises(ValueError, match="fragment, not a document"): + _ImageConverter(Mock(return_value=fragment)).convert(_docx(), _INFO) + + +@pytest.mark.parametrize("via_dispatcher", [False, True]) +def test_hook_errors_propagate_and_close_the_image_stream(via_dispatcher: bool) -> None: + streams = [] + error = RuntimeError("Image service failed") + + def render(stream, info, **kwargs): + streams.append(stream) + raise error + + converter = _ImageConverter(render) + if via_dispatcher: + markitdown = MarkItDown(enable_builtins=False) + markitdown.register_converter(converter, priority=-1) + with pytest.raises(FileConversionException) as caught: + markitdown.convert_stream(_docx(), stream_info=_INFO) + assert caught.value.attempts is not None + assert any( + attempt.exc_info and attempt.exc_info[1] is error + for attempt in caught.value.attempts + ) + else: + with pytest.raises(RuntimeError) as caught_direct: + converter.convert(_docx(), _INFO) + assert caught_direct.value is error + assert streams and all(stream.closed for stream in streams) + + +def test_dispatcher_can_fall_back_to_native_docx_after_hook_error() -> None: + render = Mock(side_effect=RuntimeError("Image service failed")) + markitdown = MarkItDown() + markitdown.register_converter(_ImageConverter(render), priority=-1) + + result = markitdown.convert_stream(_docx(), stream_info=_INFO) + + assert result.markdown == DocxConverter().convert(_docx(), _INFO).markdown + render.assert_called() + + +def test_mammoth_does_not_swallow_hook_file_reference_errors() -> None: + error = InvalidFileReferenceError("hook failed") + render = Mock(side_effect=error) + + with pytest.raises(RuntimeError, match="_image_to_html failed") as caught: + _ImageConverter(render).convert(_docx(), _INFO) + + assert caught.value.__cause__ is error + render.assert_called_once() + + +def test_per_call_options_and_fragments_do_not_leak_between_conversions() -> None: + converter = _ImageConverter(lambda stream, info, **kwargs: kwargs.get("image_text")) + first = converter.convert(_docx(), _INFO, image_text="

first

") + second = converter.convert(_docx(), _INFO, image_text="

second

") + third = converter.convert(_docx(), _INFO) + + assert first.markdown == "Before\n\nfirst\n\nAfter" + assert second.markdown == "Before\n\nsecond\n\nAfter" + assert third.markdown == DocxConverter().convert(_docx(), _INFO).markdown + + +def test_unreferenced_media_does_not_invoke_the_hook() -> None: + render = Mock(side_effect=AssertionError("unexpected image")) + + result = _ImageConverter(render).convert( + _docx(_paragraph(_text("Only text"))), _INFO + ) + + assert result.markdown == "Only text" + render.assert_not_called() + + +@pytest.mark.parametrize( + "fragment", + [ + 'new', + '

new

', + ], +) +def test_image_html_does_not_create_nested_links( + monkeypatch: pytest.MonkeyPatch, fragment: str +) -> None: + body = _paragraph( + '' + + _text("Before") + + _image() + + _text("After") + + "" + ) + converter = _ImageConverter(Mock(return_value=fragment)) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + result = converter.convert(_docx(body), _INFO) + + soup = BeautifulSoup(convert_html.call_args.args[0], "html.parser") + assert not soup.select("a a") + assert [link.get("href") for link in soup.find_all("a")] == [ + "#target", + "https://example.test/new", + "#target", + ] + assert "[Before](#target)" in result.markdown + assert "[new](https://example.test/new)" in result.markdown + assert "[After](#target)" in result.markdown + + +@pytest.mark.parametrize("keep_data_uris", [False, True]) +def test_declining_hook_preserves_a_real_document(keep_data_uris: bool) -> None: + content = (_FILES / "test.docx").read_bytes() + expected = DocxConverter().convert( + io.BytesIO(content), _INFO, keep_data_uris=keep_data_uris + ) + render = Mock(return_value=None) + + actual = _ImageConverter(render).convert( + io.BytesIO(content), _INFO, keep_data_uris=keep_data_uris + ) + + assert actual.markdown == expected.markdown + assert actual.title == expected.title + render.assert_called() + + +def test_hook_reads_images_from_the_preprocessed_archive() -> None: + import struct + + stream = _docx() + data = bytearray(stream.getvalue()) + with zipfile.ZipFile(stream) as archive: + offset = archive.getinfo("word/media/image.png").header_offset + length = struct.unpack_from("image" + + result = _ImageConverter(render).convert(io.BytesIO(data), _INFO) + + assert observed == [_PNG] + assert result.markdown == "BeforeimageAfter" + + +def test_missing_dependencies_fail_before_invoking_hook( + monkeypatch: pytest.MonkeyPatch, +) -> None: + error = ModuleNotFoundError("No module named 'mammoth'") + monkeypatch.setattr( + _docx_converter, "_dependency_exc_info", (ModuleNotFoundError, error, None) + ) + render = Mock() + + with pytest.raises(MissingDependencyException) as caught: + _ImageConverter(render).convert(_docx(), _INFO) + + assert caught.value.__cause__ is error + render.assert_not_called() From 58e7dd135bb942e0ccddd3bdb4e9d3151145966e Mon Sep 17 00:00:00 2001 From: Adam Fourney Date: Tue, 15 Sep 2026 17:16:30 -0700 Subject: [PATCH 2/6] Ensure compatibility with core markitdown version --- packages/markitdown-ocr/README.md | 2 ++ packages/markitdown-ocr/pyproject.toml | 2 +- .../src/markitdown_ocr/_docx_converter_with_ocr.py | 5 +++-- packages/markitdown-ocr/tests/test_docx_inheritance.py | 3 ++- packages/markitdown/src/markitdown/__about__.py | 2 +- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/markitdown-ocr/README.md b/packages/markitdown-ocr/README.md index d0883db4ac..ac592be083 100644 --- a/packages/markitdown-ocr/README.md +++ b/packages/markitdown-ocr/README.md @@ -14,6 +14,8 @@ Uses the same `llm_client` / `llm_model` pattern that MarkItDown already support ## Installation +Requires `markitdown>=0.1.8b3`, which introduces the DOCX image-rendering hook used by this plugin. Installing the plugin automatically resolves a compatible core version. + ```bash pip install markitdown-ocr ``` diff --git a/packages/markitdown-ocr/pyproject.toml b/packages/markitdown-ocr/pyproject.toml index eda3cdda58..96d97d8c16 100644 --- a/packages/markitdown-ocr/pyproject.toml +++ b/packages/markitdown-ocr/pyproject.toml @@ -25,7 +25,7 @@ classifiers = [ # Core dependencies — matches the file-format libraries markitdown already uses dependencies = [ - "markitdown>=0.1.0", + "markitdown>=0.1.8b3", "pdfminer.six>=20251230", "pdfplumber>=0.11.9", "PyMuPDF>=1.24.0", diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py b/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py index 28e6bbd912..e3090808b7 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py @@ -18,8 +18,9 @@ def __init__(self, ocr_service: Optional[LLMVisionOCRService] = None): super().__init__() if not hasattr(DocxConverter, "_image_to_html"): raise RuntimeError( - "DOCX OCR requires the core DocxConverter._image_to_html hook. " - "Install markitdown and markitdown-ocr from the same source checkout." + "DOCX OCR requires markitdown>=0.1.8b3 for the " + "DocxConverter._image_to_html hook. " + "Upgrade with: pip install --upgrade 'markitdown>=0.1.8b3'." ) self.ocr_service = ocr_service diff --git a/packages/markitdown-ocr/tests/test_docx_inheritance.py b/packages/markitdown-ocr/tests/test_docx_inheritance.py index 84f1a2b36a..3b485b5d24 100644 --- a/packages/markitdown-ocr/tests/test_docx_inheritance.py +++ b/packages/markitdown-ocr/tests/test_docx_inheritance.py @@ -281,5 +281,6 @@ def test_older_core_is_rejected_instead_of_silently_skipping_ocr( ) -> None: monkeypatch.delattr(_docx_converter.DocxConverter, "_image_to_html") - with pytest.raises(RuntimeError, match="same source checkout"): + with pytest.raises(RuntimeError, match=r"markitdown>=0\.1\.8b3") as caught: DocxConverterWithOCR() + assert "pip install --upgrade 'markitdown>=0.1.8b3'" in str(caught.value) diff --git a/packages/markitdown/src/markitdown/__about__.py b/packages/markitdown/src/markitdown/__about__.py index 123c671b03..721f543f00 100644 --- a/packages/markitdown/src/markitdown/__about__.py +++ b/packages/markitdown/src/markitdown/__about__.py @@ -1,4 +1,4 @@ # SPDX-FileCopyrightText: 2024-present Adam Fourney # # SPDX-License-Identifier: MIT -__version__ = "0.1.8b2" +__version__ = "0.1.8b3" From bb474fec48acc8c64fd177e57e5b3213e6812624 Mon Sep 17 00:00:00 2001 From: Adam Fourney Date: Wed, 16 Sep 2026 07:43:36 -0700 Subject: [PATCH 3/6] Have pptx, xlsx and docx expose an _image_to_html semi-private method, overridable by plugins. --- packages/markitdown-ocr/README.md | 30 +- .../_pptx_converter_with_ocr.py | 282 ++-------- .../_xlsx_converter_with_ocr.py | 255 ++------- .../tests/test_pptx_converter.py | 38 +- .../tests/test_pptx_inheritance.py | 291 ++++++++++ .../tests/test_xlsx_converter.py | 59 +- .../tests/test_xlsx_inheritance.py | 333 ++++++++++++ .../src/markitdown/converter_utils/_image.py | 19 + .../converter_utils/_xlsx_images.py | 115 ++++ .../converter_utils/docx/_images.py | 13 +- .../markitdown/converters/_pptx_converter.py | 163 +++--- .../markitdown/converters/_xlsx_converter.py | 82 ++- packages/markitdown/tests/test_pptx_images.py | 502 ++++++++++++++++++ packages/markitdown/tests/test_xlsx_images.py | 466 ++++++++++++++++ 14 files changed, 2044 insertions(+), 604 deletions(-) create mode 100644 packages/markitdown-ocr/tests/test_pptx_inheritance.py create mode 100644 packages/markitdown-ocr/tests/test_xlsx_inheritance.py create mode 100644 packages/markitdown/src/markitdown/converter_utils/_image.py create mode 100644 packages/markitdown/src/markitdown/converter_utils/_xlsx_images.py create mode 100644 packages/markitdown/tests/test_pptx_images.py create mode 100644 packages/markitdown/tests/test_xlsx_images.py diff --git a/packages/markitdown-ocr/README.md b/packages/markitdown-ocr/README.md index ac592be083..8a87969a37 100644 --- a/packages/markitdown-ocr/README.md +++ b/packages/markitdown-ocr/README.md @@ -14,7 +14,7 @@ Uses the same `llm_client` / `llm_model` pattern that MarkItDown already support ## Installation -Requires `markitdown>=0.1.8b3`, which introduces the DOCX image-rendering hook used by this plugin. Installing the plugin automatically resolves a compatible core version. +Requires `markitdown>=0.1.8b3`, which introduces the Office image-rendering hooks used by this plugin. Installing the plugin automatically resolves a compatible core version. ```bash pip install markitdown-ocr @@ -99,9 +99,11 @@ When a file is converted: 1. The OCR converter accepts the file 2. It extracts embedded images from the document 3. Each image is sent to the LLM with an extraction prompt -4. The returned text is inserted inline, preserving document structure +4. The returned text is placed alongside document content (XLSX images follow their sheet's table) 5. If the LLM call fails, conversion continues without that image's text +The DOCX, PPTX, and XLSX converters subclass their core counterparts and override the same semi-private `_image_to_html` method. Core handles native content, preprocessing, and placement; the plugin supplies escaped OCR HTML, which passes through the shared HTML-to-Markdown renderer. PDF uses its separate existing pipeline. + ## Supported File Formats ### PDF @@ -112,21 +114,23 @@ When a file is converted: ### DOCX -- Images are extracted via document part relationships (`doc.part.rels`). -- OCR is run before the DOCX→HTML→Markdown pipeline executes: placeholder tokens are injected into the HTML so that the markdown converter does not escape the OCR markers, and the final placeholders are replaced with the formatted `*[Image OCR]...[End OCR]*` blocks after conversion. -- Document flow (headings, paragraphs, tables) is fully preserved around the OCR blocks. +- Inherits core DOCX preprocessing, styles, math, and Mammoth conversion. +- Mammoth provides each embedded image to `_image_to_html`. OCR fragments are inserted into the document's HTML before Markdown rendering, not substituted into finished Markdown. +- Block fragments split enclosing paragraphs where necessary and remain inside their table cell or list item. Table-cell line breaks follow the shared HTML converter's existing limitations. ### PPTX - Picture shapes, placeholder shapes with images, and images inside groups are all supported. -- Shapes are processed in top-to-left reading order per slide. +- Inherits core shape ordering, native text, tables, charts, and speaker notes. +- Slide content now uses the core converter's real line breaks rather than the old plugin's literal `\n` text, and inherits its empty-title and empty-notes handling. - If an `llm_client` is configured, the LLM is asked for a description first; OCR is used as the fallback when no description is returned. ### XLSX -- Images embedded in worksheets (`sheet._images`) are extracted per sheet. -- Cell position is calculated from the image anchor coordinates (column/row → Excel letter notation). +- Inherits core workbook repair and table rendering; images are read from the same repaired workbook. - Images are listed under a `### Images in this sheet:` section after the sheet's data table — they are not interleaved into the table rows. +- Sheet heading spacing follows the core converter; no new cell-position labels are added. +- Legacy `.xls` files remain handled by the existing core converter, without image OCR. ### Output format @@ -138,6 +142,10 @@ Every extracted OCR block is wrapped as: [End OCR]* ``` +For Office formats, recognized text is escaped as literal HTML text before Markdown rendering. Markdown escaping and line breaks therefore follow the shared HTML converter: for example, underscores may be backslash-escaped, and direct converter results use Markdown hard breaks. `MarkItDown` subsequently strips trailing whitespace from each output line. Empty recognition retains the native image representation (XLSX normally omits images). + +Repeated image bytes are recognized once per conversion, while the result is placed at every occurrence. The cache is not shared across documents or service overrides. + ## Troubleshooting ### OCR text missing from output @@ -167,6 +175,8 @@ markitdown --list-plugins # should show: ocr The plugin propagates LLM API errors as warnings and continues conversion. Check your API key, quota, and that the chosen model supports vision inputs. +For Office OCR, a service-reported error emits a warning and retains native image rendering. Exceptions raised by custom OCR services propagate from direct converter calls; `MarkItDown` can retry another applicable converter through its normal fallback behavior. + ## Development ### Running Tests @@ -180,8 +190,8 @@ pytest tests/ -v ```bash git clone https://github.com/microsoft/markitdown.git -cd markitdown/packages/markitdown-ocr -pip install -e . +cd markitdown +pip install -e 'packages/markitdown[docx,pptx,xlsx]' -e packages/markitdown-ocr ``` ## Contributing diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py b/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py index 5988c1659d..b856d79981 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py @@ -1,253 +1,71 @@ -""" -Enhanced PPTX Converter with improved OCR support. -Already has LLM-based image description, this enhances it with traditional OCR fallback. -""" +"""PPTX image OCR using the core presentation conversion pipeline.""" -import io -import sys +import hashlib +import html from typing import Any, BinaryIO, Optional +from warnings import warn -from typing import BinaryIO, Any, Optional +from markitdown import DocumentConverterResult, StreamInfo +from markitdown.converters import PptxConverter -from markitdown.converters import HtmlConverter -from markitdown import DocumentConverter, DocumentConverterResult, StreamInfo -from markitdown._exceptions import ( - MissingDependencyException, - MISSING_DEPENDENCY_MESSAGE, -) from ._ocr_service import LLMVisionOCRService -_dependency_exc_info = None -try: - import pptx -except ImportError: - _dependency_exc_info = sys.exc_info() - -class PptxConverterWithOCR(DocumentConverter): - """Enhanced PPTX Converter with OCR fallback.""" +class PptxConverterWithOCR(PptxConverter): + """Recognize embedded images while inheriting native PPTX conversion.""" def __init__(self, ocr_service: Optional[LLMVisionOCRService] = None): super().__init__() - self._html_converter = HtmlConverter() + if not hasattr(PptxConverter, "_image_to_html"): + raise RuntimeError( + "PPTX OCR requires markitdown>=0.1.8b3 for the " + "PptxConverter._image_to_html hook. " + "Upgrade with: pip install --upgrade 'markitdown>=0.1.8b3'." + ) self.ocr_service = ocr_service - def accepts( + def convert( self, file_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any, - ) -> bool: - mimetype = (stream_info.mimetype or "").lower() - extension = (stream_info.extension or "").lower() - - if extension == ".pptx": - return True - - if mimetype.startswith( - "application/vnd.openxmlformats-officedocument.presentationml" - ): - return True - - return False + ) -> DocumentConverterResult: + kwargs["_pptx_ocr_cache"] = {} + return super().convert(file_stream, stream_info, **kwargs) - def convert( + def _image_to_html( self, - file_stream: BinaryIO, + image_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any, - ) -> DocumentConverterResult: - if _dependency_exc_info is not None: - raise MissingDependencyException( - MISSING_DEPENDENCY_MESSAGE.format( - converter=type(self).__name__, - extension=".pptx", - feature="pptx", - ) - ) from _dependency_exc_info[1].with_traceback( - _dependency_exc_info[2] - ) # type: ignore[union-attr] - - # Get OCR service (from kwargs or instance) - ocr_service: Optional[LLMVisionOCRService] = ( - kwargs.get("ocr_service") or self.ocr_service - ) - llm_client = kwargs.get("llm_client") - - presentation = pptx.Presentation(file_stream) - md_content = "" - slide_num = 0 - - for slide in presentation.slides: - slide_num += 1 - md_content += f"\\n\\n\\n" - - title = slide.shapes.title - - def get_shape_content(shape, **kwargs): - nonlocal md_content - - # Pictures - if self._is_picture(shape): - # Get image data - image_stream = io.BytesIO(shape.image.blob) - - # Try LLM description first if available - llm_description = "" - if llm_client and kwargs.get("llm_model"): - try: - from ._llm_caption import llm_caption - - image_filename = shape.image.filename - image_extension = None - if image_filename: - import os - - image_extension = os.path.splitext(image_filename)[1] - - image_stream_info = StreamInfo( - mimetype=shape.image.content_type, - extension=image_extension, - filename=image_filename, - ) - - llm_description = llm_caption( - image_stream, - image_stream_info, - client=llm_client, - model=kwargs.get("llm_model"), - prompt=kwargs.get("llm_prompt"), - ) - except Exception: - pass - - # Try OCR if LLM failed or not available - ocr_text = "" - if not llm_description and ocr_service: - try: - image_stream.seek(0) - ocr_result = ocr_service.extract_text(image_stream) - if ocr_result.text.strip(): - ocr_text = ocr_result.text.strip() - except Exception: - pass - - # Format extracted content using unified OCR block format - content = (llm_description or ocr_text or "").strip() - if content: - md_content += f"\n*[Image OCR]\n{content}\n[End OCR]*\n" - - # Tables - if self._is_table(shape): - md_content += self._convert_table_to_markdown(shape.table, **kwargs) - - # Charts - if shape.has_chart: - md_content += self._convert_chart_to_markdown(shape.chart) - - # Text areas - elif shape.has_text_frame: - if shape == title: - md_content += "# " + shape.text.lstrip() + "\\n" - else: - md_content += shape.text + "\\n" - - # Group Shapes - if shape.shape_type == pptx.enum.shapes.MSO_SHAPE_TYPE.GROUP: - sorted_shapes = sorted( - shape.shapes, - key=lambda x: ( - float("-inf") if not x.top else x.top, - float("-inf") if not x.left else x.left, - ), - ) - for subshape in sorted_shapes: - get_shape_content(subshape, **kwargs) - - sorted_shapes = sorted( - slide.shapes, - key=lambda x: ( - float("-inf") if not x.top else x.top, - float("-inf") if not x.left else x.left, - ), + ) -> Optional[str]: + ocr_service = kwargs.get("ocr_service") or self.ocr_service + if ocr_service is None: + return None + + cache: dict[bytes, Optional[str]] = kwargs.get("_pptx_ocr_cache", {}) + key = hashlib.sha256(image_stream.read()).digest() + image_stream.seek(0) + if key in cache: + return cache[key] + + # Preserve compatibility with services accepting only an image stream. + result = ocr_service.extract_text(image_stream) + if result.error: + warn( + f"PPTX image OCR failed: {result.error}. Keeping the native image.", + RuntimeWarning, + stacklevel=2, ) - for shape in sorted_shapes: - get_shape_content(shape, **kwargs) - - md_content = md_content.strip() - - if slide.has_notes_slide: - md_content += "\\n\\n### Notes:\\n" - notes_frame = slide.notes_slide.notes_text_frame - if notes_frame is not None: - md_content += notes_frame.text - md_content = md_content.strip() - - return DocumentConverterResult(markdown=md_content.strip()) - - def _is_picture(self, shape): - if shape.shape_type == pptx.enum.shapes.MSO_SHAPE_TYPE.PICTURE: - return True - if shape.shape_type == pptx.enum.shapes.MSO_SHAPE_TYPE.PLACEHOLDER: - if hasattr(shape, "image"): - return True - return False - - def _is_table(self, shape): - if shape.shape_type == pptx.enum.shapes.MSO_SHAPE_TYPE.TABLE: - return True - return False - - def _convert_table_to_markdown(self, table, **kwargs): - import html - - html_table = "" - first_row = True - for row in table.rows: - html_table += "" - for cell in row.cells: - if first_row: - html_table += "" - else: - html_table += "" - html_table += "" - first_row = False - html_table += "
" + html.escape(cell.text) + "" + html.escape(cell.text) + "
" - - return ( - self._html_converter.convert_string(html_table, **kwargs).markdown.strip() - + "\\n" - ) - - def _convert_chart_to_markdown(self, chart): - try: - md = "\\n\\n### Chart" - # ChartTitle.text_frame is documented as destructive -- it creates - # a text frame if one isn't already present, so it never returns - # None. has_text_frame is the property that actually reflects - # whether a text frame exists. - if chart.has_title and chart.chart_title.has_text_frame: - md += f": {chart.chart_title.text_frame.text}" - md += "\\n\\n" - data = [] - category_names = [c.label for c in chart.plots[0].categories] - series_names = [s.name for s in chart.series] - data.append(["Category"] + series_names) - - for idx, category in enumerate(category_names): - row = [category] - for series in chart.series: - row.append(series.values[idx]) - data.append(row) - - markdown_table = [] - for row in data: - markdown_table.append("| " + " | ".join(map(str, row)) + " |") - header = markdown_table[0] - separator = "|" + "|".join(["---"] * len(data[0])) + "|" - return md + "\\n".join([header, separator] + markdown_table[1:]) - except ValueError as e: - if "unsupported plot type" in str(e): - return "\\n\\n[unsupported chart]\\n\\n" - except Exception: - return "\\n\\n[unsupported chart]\\n\\n" + cache[key] = None + return None + text = result.text.strip() + if not text: + cache[key] = None + return None + + text = text.replace("\r\n", "\n").replace("\r", "\n") + content = html.escape(text).replace("\n", "
") + fragment = f"

[Image OCR]
{content}
[End OCR]

" + cache[key] = fragment + return fragment diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_xlsx_converter_with_ocr.py b/packages/markitdown-ocr/src/markitdown_ocr/_xlsx_converter_with_ocr.py index 481e071953..8ae36ad1e6 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_xlsx_converter_with_ocr.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_xlsx_converter_with_ocr.py @@ -1,225 +1,70 @@ -""" -Enhanced XLSX Converter with OCR support for embedded images. -Extracts images from Excel spreadsheets and performs OCR while maintaining cell context. -""" +"""XLSX image OCR using the core spreadsheet conversion pipeline.""" -import io -import sys +import hashlib +import html from typing import Any, BinaryIO, Optional +from warnings import warn -from markitdown.converters import HtmlConverter -from markitdown import DocumentConverter, DocumentConverterResult, StreamInfo -from markitdown._exceptions import ( - MissingDependencyException, - MISSING_DEPENDENCY_MESSAGE, -) -from ._ocr_service import LLMVisionOCRService +from markitdown import DocumentConverterResult, StreamInfo +from markitdown.converters import XlsxConverter -# Try loading dependencies -_xlsx_dependency_exc_info = None -try: - import pandas as pd - from openpyxl import load_workbook -except ImportError: - _xlsx_dependency_exc_info = sys.exc_info() +from ._ocr_service import LLMVisionOCRService -class XlsxConverterWithOCR(DocumentConverter): - """ - Enhanced XLSX Converter with OCR support for embedded images. - Extracts images with their cell positions and performs OCR. - """ +class XlsxConverterWithOCR(XlsxConverter): + """Recognize embedded images while inheriting native XLSX conversion.""" def __init__(self, ocr_service: Optional[LLMVisionOCRService] = None): super().__init__() - self._html_converter = HtmlConverter() + if not hasattr(XlsxConverter, "_image_to_html"): + raise RuntimeError( + "XLSX OCR requires markitdown>=0.1.8b3 for the " + "XlsxConverter._image_to_html hook. " + "Upgrade with: pip install --upgrade 'markitdown>=0.1.8b3'." + ) self.ocr_service = ocr_service - def accepts( + def convert( self, file_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any, - ) -> bool: - mimetype = (stream_info.mimetype or "").lower() - extension = (stream_info.extension or "").lower() - - if extension == ".xlsx": - return True - - if mimetype.startswith( - "application/vnd.openxmlformats-officedocument.spreadsheetml" - ): - return True - - return False + ) -> DocumentConverterResult: + kwargs["_xlsx_ocr_cache"] = {} + return super().convert(file_stream, stream_info, **kwargs) - def convert( + def _image_to_html( self, - file_stream: BinaryIO, + image_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any, - ) -> DocumentConverterResult: - if _xlsx_dependency_exc_info is not None: - raise MissingDependencyException( - MISSING_DEPENDENCY_MESSAGE.format( - converter=type(self).__name__, - extension=".xlsx", - feature="xlsx", - ) - ) from _xlsx_dependency_exc_info[1].with_traceback( - _xlsx_dependency_exc_info[2] - ) # type: ignore[union-attr] - - # Get OCR service if available (from kwargs or instance) - ocr_service: Optional[LLMVisionOCRService] = ( - kwargs.get("ocr_service") or self.ocr_service - ) - - if ocr_service: - # Remove ocr_service from kwargs to avoid duplicate argument error - kwargs_without_ocr = {k: v for k, v in kwargs.items() if k != "ocr_service"} - return self._convert_with_ocr( - file_stream, ocr_service, **kwargs_without_ocr + ) -> Optional[str]: + ocr_service = kwargs.get("ocr_service") or self.ocr_service + if ocr_service is None: + return None + + cache: dict[bytes, Optional[str]] = kwargs.get("_xlsx_ocr_cache", {}) + key = hashlib.sha256(image_stream.read()).digest() + image_stream.seek(0) + if key in cache: + return cache[key] + + result = ocr_service.extract_text(image_stream) + if result.error: + warn( + f"XLSX image OCR failed: {result.error}. Keeping the native image.", + RuntimeWarning, + stacklevel=2, ) - else: - return self._convert_standard(file_stream, **kwargs) - - def _convert_standard( - self, file_stream: BinaryIO, **kwargs: Any - ) -> DocumentConverterResult: - """Standard conversion without OCR.""" - file_stream.seek(0) - sheets = pd.read_excel(file_stream, sheet_name=None, engine="openpyxl") - md_content = "" - - for sheet_name in sheets: - md_content += f"## {sheet_name}\n" - html_content = sheets[sheet_name].to_html(index=False) - md_content += ( - self._html_converter.convert_string( - html_content, **kwargs - ).markdown.strip() - + "\n\n" - ) - - return DocumentConverterResult(markdown=md_content.strip()) - - def _convert_with_ocr( - self, file_stream: BinaryIO, ocr_service: LLMVisionOCRService, **kwargs: Any - ) -> DocumentConverterResult: - """Convert XLSX with image OCR.""" - file_stream.seek(0) - wb = load_workbook(file_stream) - - md_content = "" - - for sheet_name in wb.sheetnames: - sheet = wb[sheet_name] - md_content += f"## {sheet_name}\n\n" - - # Convert sheet data to markdown table - file_stream.seek(0) - try: - df = pd.read_excel( - file_stream, sheet_name=sheet_name, engine="openpyxl" - ) - html_content = df.to_html(index=False) - md_content += ( - self._html_converter.convert_string( - html_content, **kwargs - ).markdown.strip() - + "\n\n" - ) - except Exception: - # If pandas fails, just skip the table - pass - - # Extract and OCR images in this sheet - images_with_ocr = self._extract_and_ocr_sheet_images(sheet, ocr_service) - - if images_with_ocr: - md_content += "### Images in this sheet:\n\n" - for img_info in images_with_ocr: - ocr_text = img_info["ocr_text"] - md_content += f"*[Image OCR]\n{ocr_text}\n[End OCR]*\n\n" - - return DocumentConverterResult(markdown=md_content.strip()) - - def _extract_and_ocr_sheet_images( - self, sheet: Any, ocr_service: LLMVisionOCRService - ) -> list[dict]: - """ - Extract and OCR images from an Excel sheet. - - Args: - sheet: openpyxl worksheet - ocr_service: OCR service - - Returns: - List of dicts with 'cell_ref' and 'ocr_text' - """ - results = [] - - try: - # Check if sheet has images - if hasattr(sheet, "_images"): - for img in sheet._images: - try: - # Get image data - if hasattr(img, "_data"): - image_data = img._data() - elif hasattr(img, "image"): - # Some versions store it differently - image_data = img.image - else: - continue - - # Create image stream - image_stream = io.BytesIO(image_data) - - # Get cell reference - cell_ref = "unknown" - if hasattr(img, "anchor"): - anchor = img.anchor - if hasattr(anchor, "_from"): - from_cell = anchor._from - if hasattr(from_cell, "col") and hasattr( - from_cell, "row" - ): - # Convert column number to letter - col_letter = self._column_number_to_letter( - from_cell.col - ) - cell_ref = f"{col_letter}{from_cell.row + 1}" - - # Perform OCR - ocr_result = ocr_service.extract_text(image_stream) - - if ocr_result.text.strip(): - results.append( - { - "cell_ref": cell_ref, - "ocr_text": ocr_result.text.strip(), - "backend": ocr_result.backend_used, - } - ) - - except Exception: - continue - - except Exception: - pass - - return results - - @staticmethod - def _column_number_to_letter(n: int) -> str: - """Convert column number to Excel column letter (0-indexed).""" - result = "" - n = n + 1 # Make 1-indexed - while n > 0: - n -= 1 - result = chr(65 + (n % 26)) + result - n //= 26 - return result + cache[key] = None + return None + text = result.text.strip() + if not text: + cache[key] = None + return None + + text = text.replace("\r\n", "\n").replace("\r", "\n") + content = html.escape(text).replace("\n", "
") + fragment = f"

[Image OCR]
{content}
[End OCR]

" + cache[key] = fragment + return fragment diff --git a/packages/markitdown-ocr/tests/test_pptx_converter.py b/packages/markitdown-ocr/tests/test_pptx_converter.py index 989724f3b7..a0fc098f71 100644 --- a/packages/markitdown-ocr/tests/test_pptx_converter.py +++ b/packages/markitdown-ocr/tests/test_pptx_converter.py @@ -4,13 +4,12 @@ For each PPTX test file: convert with a mock OCR service then compare the full output string against the expected snapshot. -OCR block format used by the converter: +OCR blocks use shared HTML escaping and Markdown hard breaks: *[Image OCR] - MOCK_OCR_TEXT_12345 + MOCK\\_OCR\\_TEXT\\_12345 [End OCR]* -Note: PPTX slide text uses literal backslash-n (\\n) sequences from the -underlying PPTX converter template; OCR blocks use real newlines. +Slide text, notes, and layout come from the core PPTX converter. """ import sys @@ -30,7 +29,7 @@ TEST_DATA_DIR = Path(__file__).parent / "ocr_test_data" _MOCK_TEXT = "MOCK_OCR_TEXT_12345" -_OCR_BLOCK = f"*[Image OCR]\n{_MOCK_TEXT}\n[End OCR]*" +_OCR_BLOCK = "*[Image OCR] \nMOCK\\_OCR\\_TEXT\\_12345 \n[End OCR]*" class MockOCRService: @@ -65,10 +64,7 @@ def _convert(filename: str, ocr_service: MockOCRService) -> str: def test_pptx_image_start(svc: MockOCRService) -> None: # Slide 1: title "Welcome" followed by an image - expected = ( - "\\n\\n\\n# Welcome\\n\\n" - "\n*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" - ) + expected = "\n# Welcome\n\n\n" + _OCR_BLOCK assert _convert("pptx_image_start.pptx", svc) == expected @@ -80,10 +76,10 @@ def test_pptx_image_start(svc: MockOCRService) -> None: def test_pptx_image_middle(svc: MockOCRService) -> None: # Slide 1: Introduction | Slide 2: Architecture + image | Slide 3: Conclusion # noqa: E501 expected = ( - "\\n\\n\\n# Introduction" - "\\n\\n\\n\\n\\n# Architecture\\n\\n" - "\n*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" - "\\n\\n\\n# Conclusion\\n\\n" + "\n# Introduction" + "\n\n\n# Architecture\n\n\n" + + _OCR_BLOCK + + "\n\n\n# Conclusion" ) assert _convert("pptx_image_middle.pptx", svc) == expected @@ -96,9 +92,8 @@ def test_pptx_image_middle(svc: MockOCRService) -> None: def test_pptx_image_end(svc: MockOCRService) -> None: # Slide 1: Presentation | Slide 2: Thank You + image expected = ( - "\\n\\n\\n# Presentation" - "\\n\\n\\n\\n\\n# Thank You\\n\\n" - "\n*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + "\n# Presentation" + "\n\n\n# Thank You\n\n\n" + _OCR_BLOCK ) assert _convert("pptx_image_end.pptx", svc) == expected @@ -110,11 +105,7 @@ def test_pptx_image_end(svc: MockOCRService) -> None: def test_pptx_multiple_images(svc: MockOCRService) -> None: # Slide 1: two images, no title text - expected = ( - "\\n\\n\\n# \\n" - "\n*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" - "\n\n*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" - ) + expected = "\n\n" + _OCR_BLOCK + "\n\n" + _OCR_BLOCK assert _convert("pptx_multiple_images.pptx", svc) == expected @@ -125,9 +116,8 @@ def test_pptx_multiple_images(svc: MockOCRService) -> None: def test_pptx_complex_layout(svc: MockOCRService) -> None: expected = ( - "\\n\\n\\n# Product Comparison" - "\\n\\nOur products lead the market\\n" - "\n*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + "\n# Product Comparison" + "\n\nOur products lead the market\n\n" + _OCR_BLOCK ) assert _convert("pptx_complex_layout.pptx", svc) == expected diff --git a/packages/markitdown-ocr/tests/test_pptx_inheritance.py b/packages/markitdown-ocr/tests/test_pptx_inheritance.py new file mode 100644 index 0000000000..8a9923f13a --- /dev/null +++ b/packages/markitdown-ocr/tests/test_pptx_inheritance.py @@ -0,0 +1,291 @@ +"""Full PPTX OCR conversions inherit core slides, image hooks, and HTML.""" + +import base64 +import inspect +import io +from pathlib import Path +from typing import Any +from unittest.mock import Mock + +from PIL import Image +from pptx import Presentation +from pptx.util import Inches +import pytest + +from markitdown import FileConversionException, MarkItDown, StreamInfo +from markitdown.converters import PptxConverter +from markitdown.converters import _pptx_converter +import markitdown._markitdown as markitdown_module +from markitdown_ocr import _plugin +from markitdown_ocr._ocr_service import OCRResult +from markitdown_ocr._pptx_converter_with_ocr import PptxConverterWithOCR + + +_INFO = StreamInfo(extension=".pptx") +_CORE_FILES = Path(__file__).parents[2] / "markitdown" / "tests" / "test_files" + + +def _png(color: str) -> bytes: + stream = io.BytesIO() + Image.new("RGB", (2, 2), color).save(stream, format="PNG") + return stream.getvalue() + + +_RED = _png("red") +_BLUE = _png("blue") + + +def _presentation(images: tuple[bytes, ...] = (_RED,)) -> bytes: + presentation = Presentation() + slide = presentation.slides.add_slide(presentation.slide_layouts[5]) + slide.shapes.title.text = "Heading" + slide.shapes.title.top = 0 + for index in reversed(range(len(images))): + picture = slide.shapes.add_picture( + io.BytesIO(images[index]), 0, Inches(index + 1), width=Inches(0.5) + ) + picture.name = f"Picture {index + 1}" + picture._element._nvXxPr.cNvPr.set("descr", "") + slide.notes_slide.notes_text_frame.text = "Speaker notes" + stream = io.BytesIO() + presentation.save(stream) + return stream.getvalue() + + +def _service(text: str = "recognized") -> Mock: + return Mock(extract_text=Mock(side_effect=lambda stream: OCRResult(text=text))) + + +def _convert(converter: PptxConverter, data: bytes, **kwargs: Any) -> str: + return converter.convert(io.BytesIO(data), _INFO, **kwargs).markdown + + +def test_pptx_ocr_is_a_thin_subclass() -> None: + assert issubclass(PptxConverterWithOCR, PptxConverter) + assert inspect.signature(PptxConverterWithOCR.convert) == inspect.signature( + PptxConverter.convert + ) + for method in ( + "accepts", + "_is_picture", + "_get_image_info", + "_find_svg_blip_part", + "_convert_picture_to_markdown", + "_is_table", + "_convert_table_to_markdown", + "_convert_chart_to_markdown", + ): + assert getattr(PptxConverterWithOCR, method) is getattr(PptxConverter, method) + + +@pytest.mark.parametrize( + ("fallback", "keep_data_uris"), [(None, False), ("", True), (" \r\n\t", False)] +) +def test_no_service_or_empty_recognition_matches_native_core( + fallback: str | None, keep_data_uris: bool +) -> None: + service = None if fallback is None else _service(fallback) + data = _presentation() + + assert _convert( + PptxConverterWithOCR(service), data, keep_data_uris=keep_data_uris + ) == _convert(PptxConverter(), data, keep_data_uris=keep_data_uris) + + +def test_ocr_cache_is_document_local_and_honors_service_override() -> None: + first = _service("first") + second = _service("second") + converter = PptxConverterWithOCR(first) + data = _presentation((_RED,) * 12) + + original = _convert(converter, data) + overridden = _convert(converter, data, ocr_service=second) + again = _convert(converter, data) + + assert original.count("*[Image OCR] \nfirst \n[End OCR]*") == 12 + assert overridden.count("*[Image OCR] \nsecond \n[End OCR]*") == 12 + assert again == original + assert first.extract_text.call_count == 2 + second.extract_text.assert_called_once() + + +def test_recognition_uses_image_identity_in_reading_order_with_native_fallback() -> ( + None +): + calls = [] + + def recognize(stream): + assert stream.tell() == 0 + image = stream.read() + calls.append(image) + return OCRResult(text="blue" if image == _BLUE else "") + + service = Mock(extract_text=Mock(side_effect=recognize)) + result = _convert( + PptxConverterWithOCR(service), _presentation((_BLUE, _RED, _BLUE)) + ) + + assert calls == [_BLUE, _RED] + assert result.count("*[Image OCR]") == 2 + assert result.count("![Picture 2](Picture2.jpg)") == 1 + assert result.index("blue") < result.index("![Picture 2]") < result.rindex("blue") + assert result.endswith("### Notes:\nSpeaker notes") + + +def test_ocr_text_is_escaped_with_normalized_html_breaks_and_html_options( + monkeypatch: pytest.MonkeyPatch, +) -> None: + converter = PptxConverterWithOCR(_service("A_B *literal*\r\n & value\rfinal")) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + data = _presentation() + + result = _convert(converter, data) + + assert convert_html.call_args.args == ( + "

[Image OCR]
A_B *literal*
<tag> & value" + "
final
[End OCR]

", + ) + assert ( + "*[Image OCR] \nA\\_B \\*literal\\* \n" " & value \nfinal \n[End OCR]*" + ) in result + assert "*literal*" in _convert( + converter, data, escape_asterisks=False, escape_underscores=False + ) + assert "A_B" in _convert(converter, data, escape_underscores=False) + + +def test_reported_errors_warn_once_per_image_and_keep_native_output() -> None: + service = Mock( + extract_text=Mock( + return_value=OCRResult(text="discard", error="quota exceeded") + ) + ) + data = _presentation((_RED, _RED)) + + with pytest.warns( + RuntimeWarning, match="PPTX image OCR failed: quota exceeded" + ) as seen: + result = _convert(PptxConverterWithOCR(service), data, keep_data_uris=True) + + assert len(seen) == 1 + assert result == _convert(PptxConverter(), data, keep_data_uris=True) + service.extract_text.assert_called_once() + + +def test_thrown_errors_propagate_and_use_normal_dispatcher_fallback() -> None: + error = RuntimeError("custom OCR failed") + converter = PptxConverterWithOCR(Mock(extract_text=Mock(side_effect=error))) + data = _presentation() + with pytest.raises(RuntimeError) as caught: + _convert(converter, data) + assert caught.value is error + md = MarkItDown() + md.register_converter(converter, priority=-1) + assert md.convert_stream(io.BytesIO(data), stream_info=_INFO).markdown == _convert( + PptxConverter(), data + ) + without_fallback = MarkItDown(enable_builtins=False) + without_fallback.register_converter(converter) + with pytest.raises(FileConversionException) as aggregate: + without_fallback.convert_stream(io.BytesIO(data), stream_info=_INFO) + assert aggregate.value.attempts is not None + assert any( + attempt.exc_info and attempt.exc_info[1] is error + for attempt in aggregate.value.attempts + ) + + +def test_svg_only_images_use_inherited_resolution_and_ocr() -> None: + seen = [] + + def recognize(stream): + seen.append(stream.read()) + return OCRResult(text="SVG text") + + data = (_CORE_FILES / "test_svg_no_fallback.pptx").read_bytes() + result = _convert( + PptxConverterWithOCR(Mock(extract_text=Mock(side_effect=recognize))), data + ) + + assert len(seen) == 1 and b" None: + entry_point = Mock() + entry_point.load.return_value = _plugin + entry_points = Mock(return_value=[entry_point]) + monkeypatch.setattr(markitdown_module, "entry_points", entry_points) + monkeypatch.setattr(markitdown_module, "_plugins", None) + client = Mock() + response = Mock(choices=[Mock(message=Mock(content="Recognized_text"))]) + client.chat.completions.create.side_effect = ( + [response] + if caption_succeeds + else [Mock(choices=[Mock(message=Mock(content=None))]), response] + ) + md = MarkItDown( + enable_plugins=True, + llm_client=client, + llm_model="vision-model", + llm_prompt="Read the text", + ) + registered = [ + registration + for registration in md._converters + if isinstance(registration.converter, PptxConverterWithOCR) + ] + assert len(registered) == 1 and registered[0].priority == -1 + + result = md.convert_stream(io.BytesIO(_presentation()), stream_info=_INFO) + + expected_image = ( + "![Recognized_text](Picture1.jpg)" + if caption_succeeds + else "*[Image OCR]\nRecognized\\_text\n[End OCR]*" + ) + assert result.markdown == ( + "\n# Heading\n\n" + + expected_image + + "\n\n### Notes:\nSpeaker notes" + ) + entry_points.assert_called_once_with(group="markitdown.plugin") + assert client.chat.completions.create.call_count == (1 if caption_succeeds else 2) + for call in client.chat.completions.create.call_args_list: + request = call.kwargs + assert request["model"] == "vision-model" + content = request["messages"][0]["content"] + assert content[0]["text"] == "Read the text" + assert content[1]["image_url"]["url"] == ( + "data:image/png;base64," + base64.b64encode(_RED).decode("ascii") + ) + + +def test_native_caption_precedes_ocr_and_does_not_escape_existing_markdown( + monkeypatch: pytest.MonkeyPatch, +) -> None: + caption = Mock(return_value="**Caption** with_under & ") + monkeypatch.setattr(_pptx_converter, "llm_caption", caption) + service = _service() + data = _presentation() + options = {"llm_client": object(), "llm_model": "vision"} + + result = _convert(PptxConverterWithOCR(service), data, **options) + + assert result == _convert(PptxConverter(), data, **options) + assert "![**Caption** with_under & ](Picture1.jpg)" in result + service.extract_text.assert_not_called() + assert caption.call_count == 2 + + +def test_older_core_fails_explicitly(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delattr(_pptx_converter.PptxConverter, "_image_to_html") + + with pytest.raises(RuntimeError, match=r"markitdown>=0\.1\.8b3") as caught: + PptxConverterWithOCR() + assert "pip install --upgrade 'markitdown>=0.1.8b3'" in str(caught.value) diff --git a/packages/markitdown-ocr/tests/test_xlsx_converter.py b/packages/markitdown-ocr/tests/test_xlsx_converter.py index 4ab30c6000..113960dddb 100644 --- a/packages/markitdown-ocr/tests/test_xlsx_converter.py +++ b/packages/markitdown-ocr/tests/test_xlsx_converter.py @@ -6,7 +6,7 @@ OCR block format used by the converter: *[Image OCR] - MOCK_OCR_TEXT_12345 + MOCK\\_OCR\\_TEXT\\_12345 [End OCR]* Images are grouped at the end of each sheet under: @@ -30,8 +30,9 @@ TEST_DATA_DIR = Path(__file__).parent / "ocr_test_data" _MOCK_TEXT = "MOCK_OCR_TEXT_12345" -_OCR_BLOCK = f"*[Image OCR]\n{_MOCK_TEXT}\n[End OCR]*" -_IMG_SECTION = "### Images in this sheet:" + + +_OCR_BLOCK = "*[Image OCR] \nMOCK\\_OCR\\_TEXT\\_12345 \n[End OCR]*" class MockOCRService: @@ -66,20 +67,20 @@ def _convert(filename: str, ocr_service: MockOCRService) -> str: def test_xlsx_image_start(svc: MockOCRService) -> None: expected = ( - "## Sales Q1\n\n" + "## Sales Q1\n" "| Product | Sales |\n" "| --- | --- |\n" "| Widget A | 100 |\n" "| Widget B | 150 |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "## Forecast Q2\n\n" + f"{_OCR_BLOCK}\n\n" + "## Forecast Q2\n" "| Projected Sales | Unnamed: 1 |\n" "| --- | --- |\n" "| Widget A | 120 |\n" "| Widget B | 180 |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + f"{_OCR_BLOCK}" ) assert _convert("xlsx_image_start.xlsx", svc) == expected @@ -91,7 +92,7 @@ def test_xlsx_image_start(svc: MockOCRService) -> None: def test_xlsx_image_middle(svc: MockOCRService) -> None: expected = ( - "## Revenue\n\n" + "## Revenue\n" "| Q1 Report | Unnamed: 1 |\n" "| --- | --- |\n" "| NaN | NaN |\n" @@ -102,8 +103,8 @@ def test_xlsx_image_middle(svc: MockOCRService) -> None: "| NaN | NaN |\n" "| Profit Margin | 40% |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "## Expenses\n\n" + f"{_OCR_BLOCK}\n\n" + "## Expenses\n" "| Expense Breakdown | Unnamed: 1 |\n" "| --- | --- |\n" "| NaN | NaN |\n" @@ -114,7 +115,7 @@ def test_xlsx_image_middle(svc: MockOCRService) -> None: "| NaN | NaN |\n" "| Savings | $5,000 |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + f"{_OCR_BLOCK}" ) assert _convert("xlsx_image_middle.xlsx", svc) == expected @@ -126,7 +127,7 @@ def test_xlsx_image_middle(svc: MockOCRService) -> None: def test_xlsx_image_end(svc: MockOCRService) -> None: expected = ( - "## Sheet\n\n" + "## Sheet\n" "| Financial Summary | Unnamed: 1 |\n" "| --- | --- |\n" "| Total Revenue | $500,000 |\n" @@ -139,8 +140,8 @@ def test_xlsx_image_end(svc: MockOCRService) -> None: "| NaN | NaN |\n" "| Signature: | NaN |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "## Budget\n\n" + f"{_OCR_BLOCK}\n\n" + "## Budget\n" "| Budget Allocation | Unnamed: 1 |\n" "| --- | --- |\n" "| Marketing | $100,000 |\n" @@ -153,7 +154,7 @@ def test_xlsx_image_end(svc: MockOCRService) -> None: "| NaN | NaN |\n" "| Approved: | NaN |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + f"{_OCR_BLOCK}" ) assert _convert("xlsx_image_end.xlsx", svc) == expected @@ -165,7 +166,7 @@ def test_xlsx_image_end(svc: MockOCRService) -> None: def test_xlsx_multiple_images(svc: MockOCRService) -> None: expected = ( - "## Overview\n\n" + "## Overview\n" "| Dashboard |\n" "| --- |\n" "| Status: Active |\n" @@ -175,20 +176,20 @@ def test_xlsx_multiple_images(svc: MockOCRService) -> None: "| NaN |\n" "| Performance Summary |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "## Details\n\n" + f"{_OCR_BLOCK}\n\n" + f"{_OCR_BLOCK}\n\n" + "## Details\n" "| Detailed Metrics |\n" "| --- |\n" "| System Health |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "## Summary\n\n" + f"{_OCR_BLOCK}\n\n" + "## Summary\n" "| Quarter Summary |\n" "| --- |\n" "| Overall Performance |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + f"{_OCR_BLOCK}" ) assert _convert("xlsx_multiple_images.xlsx", svc) == expected @@ -200,7 +201,7 @@ def test_xlsx_multiple_images(svc: MockOCRService) -> None: def test_xlsx_complex_layout(svc: MockOCRService) -> None: expected = ( - "## Complex Report\n\n" + "## Complex Report\n" "| Annual Report 2024 | Unnamed: 1 |\n" "| --- | --- |\n" "| NaN | NaN |\n" @@ -210,17 +211,17 @@ def test_xlsx_complex_layout(svc: MockOCRService) -> None: "| NaN | NaN |\n" "| Total | 2200 |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "## Customers\n\n" + f"{_OCR_BLOCK}\n\n" + f"{_OCR_BLOCK}\n\n" + "## Customers\n" "| Customer Metrics | Unnamed: 1 |\n" "| --- | --- |\n" "| NaN | NaN |\n" "| New Customers | 250 |\n" "| Retention Rate | 92% |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*\n\n" - "## Regions\n\n" + f"{_OCR_BLOCK}\n\n" + "## Regions\n" "| Regional Breakdown | Unnamed: 1 |\n" "| --- | --- |\n" "| NaN | NaN |\n" @@ -228,7 +229,7 @@ def test_xlsx_complex_layout(svc: MockOCRService) -> None: "| North | $800K |\n" "| South | $600K |\n\n" "### Images in this sheet:\n\n" - "*[Image OCR]\nMOCK_OCR_TEXT_12345\n[End OCR]*" + f"{_OCR_BLOCK}" ) assert _convert("xlsx_complex_layout.xlsx", svc) == expected diff --git a/packages/markitdown-ocr/tests/test_xlsx_inheritance.py b/packages/markitdown-ocr/tests/test_xlsx_inheritance.py new file mode 100644 index 0000000000..f0e010a706 --- /dev/null +++ b/packages/markitdown-ocr/tests/test_xlsx_inheritance.py @@ -0,0 +1,333 @@ +"""XLSX OCR trips through native tables, repair, extraction and HTML conversion.""" + +import base64 +import inspect +import io +from typing import Any +from unittest.mock import Mock +import zipfile + +import openpyxl +from openpyxl.drawing.image import Image as SheetImage +from openpyxl.drawing.spreadsheet_drawing import ( + AbsoluteAnchor, + AnchorMarker, + OneCellAnchor, + TwoCellAnchor, +) +from PIL import Image +import pytest + +from markitdown import FileConversionException, MarkItDown, StreamInfo +from markitdown.converters import XlsxConverter +from markitdown.converters import _xlsx_converter +import markitdown._markitdown as markitdown_module +from markitdown_ocr import _plugin +from markitdown_ocr._ocr_service import OCRResult +from markitdown_ocr._xlsx_converter_with_ocr import XlsxConverterWithOCR + + +_INFO = StreamInfo(extension=".xlsx") + + +def _png(color: str) -> bytes: + stream = io.BytesIO() + Image.new("RGB", (2, 2), color).save(stream, "PNG") + return stream.getvalue() + + +_RED, _BLUE = _png("red"), _png("blue") + + +def _workbook(images: tuple[bytes, ...] = (_RED,)) -> bytes: + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.title = "Cells" + sheet.append(["Header_one", None]) + sheet.append([" & *value*", 12]) + for index, data in enumerate(images): + sheet.add_image(SheetImage(io.BytesIO(data)), f"AA{index + 3}") + last = workbook.create_sheet("Other") + last.append(["Last"]) + last.append(["Final"]) + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + return stream.getvalue() + + +def _service(text: str = "recognized") -> Mock: + return Mock(extract_text=Mock(side_effect=lambda stream: OCRResult(text=text))) + + +def _convert(converter: XlsxConverter, data: bytes, **kwargs: Any) -> str: + return converter.convert(io.BytesIO(data), _INFO, **kwargs).markdown + + +def test_ocr_is_a_thin_subclass_with_native_acceptance_and_signature() -> None: + assert issubclass(XlsxConverterWithOCR, XlsxConverter) + assert XlsxConverterWithOCR.accepts is XlsxConverter.accepts + assert inspect.signature(XlsxConverterWithOCR.convert) == inspect.signature( + XlsxConverter.convert + ) + assert not hasattr(XlsxConverterWithOCR, "_convert_standard") + assert not hasattr(XlsxConverterWithOCR, "_convert_with_ocr") + assert not hasattr(XlsxConverterWithOCR, "_extract_and_ocr_sheet_images") + assert not XlsxConverterWithOCR().accepts( + io.BytesIO(), StreamInfo(extension=".xls") + ) + + +def test_plugin_full_trip_uses_native_cells_and_original_image( + monkeypatch: pytest.MonkeyPatch, +) -> None: + entry_point = Mock() + entry_point.load.return_value = _plugin + monkeypatch.setattr( + markitdown_module, "entry_points", Mock(return_value=[entry_point]) + ) + monkeypatch.setattr(markitdown_module, "_plugins", None) + client = Mock() + client.chat.completions.create.return_value.choices = [ + Mock(message=Mock(content="Recognized_text")) + ] + md = MarkItDown( + enable_plugins=True, + llm_client=client, + llm_model="vision-model", + llm_prompt="Read the text", + ) + registered = [ + registration + for registration in md._converters + if isinstance(registration.converter, XlsxConverterWithOCR) + ] + assert len(registered) == 1 and registered[0].priority == -1 + result = md.convert_stream(io.BytesIO(_workbook((_RED, _RED))), stream_info=_INFO) + + assert result.markdown == ( + "## Cells\n" + "| Header\\_one | Unnamed: 1 |\n" + "| --- | --- |\n" + "| & \\*value\\* | 12 |\n\n" + "### Images in this sheet:\n\n" + "*[Image OCR]\nRecognized\\_text\n[End OCR]*\n\n" + "*[Image OCR]\nRecognized\\_text\n[End OCR]*\n\n" + "## Other\n| Last |\n| --- |\n| Final |" + ) + client.chat.completions.create.assert_called_once() + request = client.chat.completions.create.call_args.kwargs + assert request["model"] == "vision-model" + content = request["messages"][0]["content"] + assert content[0]["text"] == "Read the text" + assert content[1]["image_url"]["url"] == ( + "data:image/png;base64," + base64.b64encode(_RED).decode("ascii") + ) + + +@pytest.mark.parametrize("text", [None, "", " \n\t"]) +def test_no_service_or_blank_recognition_is_exactly_native(text: str | None) -> None: + service = None if text is None else _service(text) + data = _workbook() + options = {"escape_underscores": False, "heading_style": "underlined"} + assert _convert(XlsxConverterWithOCR(service), data, **options) == ( + _convert(XlsxConverter(), data, **options) + ) + + +def test_no_images_does_not_call_service() -> None: + service = _service() + data = _workbook(()) + assert _convert(XlsxConverterWithOCR(service), data) == _convert( + XlsxConverter(), data + ) + service.extract_text.assert_not_called() + + +def test_cache_is_document_local_and_per_call_service_overrides_work() -> None: + first = _service("first") + second = _service("second") + converter = XlsxConverterWithOCR(first) + data = _workbook((_RED,) * 4) + default = _convert(converter, data) + overridden = _convert(converter, data, ocr_service=second) + again = _convert(converter, data) + + assert default.count("*[Image OCR] \nfirst \n[End OCR]*") == 4 + assert overridden.count("*[Image OCR] \nsecond \n[End OCR]*") == 4 + assert again == default + assert first.extract_text.call_count == 2 + second.extract_text.assert_called_once() + + +def test_recognition_identity_and_blank_images_preserve_anchor_order() -> None: + calls = [] + + def recognize(stream): + data = stream.read() + calls.append(data) + return OCRResult(text="blue" if data == _BLUE else "") + + converter = XlsxConverterWithOCR(Mock(extract_text=Mock(side_effect=recognize))) + result = _convert(converter, _workbook((_RED, _BLUE, _RED, _BLUE))) + + assert calls == [_RED, _BLUE] + assert result.count("[Image OCR]") == 2 + assert "Image at " not in result + assert result.count("*[Image OCR] \nblue \n[End OCR]*") == 2 + + +def test_mixed_anchor_recognition_keeps_legacy_openpyxl_order() -> None: + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Native"]) + sheet.append(["Cells"]) + images = [ + ("one first", _RED, OneCellAnchor(_from=AnchorMarker(col=1, row=3))), + ("absolute", _BLUE, AbsoluteAnchor()), + ( + "two", + _png("green"), + TwoCellAnchor( + _from=AnchorMarker(col=0, row=1), to=AnchorMarker(col=4, row=4) + ), + ), + ("one second", _png("yellow"), OneCellAnchor(_from=AnchorMarker(col=3, row=7))), + ] + labels = {} + for label, data, anchor in images: + labels[data] = label + image = SheetImage(io.BytesIO(data)) + image.anchor = anchor + sheet.add_image(image) + output = io.BytesIO() + workbook.save(output) + workbook.close() + data = output.getvalue() + legacy = openpyxl.load_workbook(io.BytesIO(data)) + try: + legacy_order = [labels[image._data()] for image in legacy.active._images] + finally: + legacy.close() + assert legacy_order == ["absolute", "one first", "one second", "two"] + calls = [] + + def recognize(stream): + label = labels[stream.read()] + calls.append(label) + return OCRResult(text=label) + + result = _convert( + XlsxConverterWithOCR(Mock(extract_text=Mock(side_effect=recognize))), data + ) + assert calls == legacy_order + blocks = [f"*[Image OCR] \n{label} \n[End OCR]*" for label in legacy_order] + assert "\n\n".join(blocks) in result + + +def test_ocr_html_is_escaped_and_existing_html_options_are_forwarded( + monkeypatch: pytest.MonkeyPatch, +) -> None: + converter = XlsxConverterWithOCR(_service("A_B *literal*\r\n & value\rfinal")) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + result = _convert( + converter, _workbook(), escape_underscores=False, heading_style="underlined" + ) + + assert ( + "

[Image OCR]
A_B *literal*
<tag> & value" + "
final
[End OCR]

" + ) in convert_html.call_args_list[1].args[0] + assert ( + "*[Image OCR] \nA_B \\*literal\\* \n & value \nfinal \n[End OCR]*" + in result + ) + assert "| Header_one | Unnamed: 1 |" in result + assert all( + call.kwargs["escape_underscores"] is False + for call in convert_html.call_args_list + ) + + +def test_inherited_repairs_and_native_table_fixes_reach_ocr( + monkeypatch: pytest.MonkeyPatch, +) -> None: + data = _workbook((_RED, _RED)) + stream = io.BytesIO() + with zipfile.ZipFile(io.BytesIO(data)) as source: + with zipfile.ZipFile(stream, "w") as target: + for entry in source.infolist(): + content = source.read(entry) + if entry.filename == "xl/worksheets/sheet1.xml": + content = content.replace( + b"" not in result + assert result.count("[Image OCR]") == 2 + repair.assert_called_once() + assert calls == [{"sheet_name": None, "engine": "openpyxl"}] * 2 + service.extract_text.assert_called_once() + + +def test_reported_ocr_error_warns_once_and_keeps_native_output() -> None: + service = Mock( + extract_text=Mock( + return_value=OCRResult(text="ignored", error="quota exceeded") + ) + ) + data = _workbook((_RED, _RED)) + with pytest.warns(RuntimeWarning, match="quota exceeded") as warnings: + result = _convert(XlsxConverterWithOCR(service), data) + assert len(warnings) == 1 + assert result == _convert(XlsxConverter(), data) + service.extract_text.assert_called_once() + + +def test_thrown_ocr_errors_use_standard_dispatcher_fallback() -> None: + error = RuntimeError("custom service failed") + converter = XlsxConverterWithOCR(Mock(extract_text=Mock(side_effect=error))) + data = _workbook() + with pytest.raises(RuntimeError) as caught: + _convert(converter, data) + assert caught.value is error + + md = MarkItDown() + md.register_converter(converter, priority=-1) + assert md.convert_stream(io.BytesIO(data), stream_info=_INFO).markdown == _convert( + XlsxConverter(), data + ) + no_fallback = MarkItDown(enable_builtins=False) + no_fallback.register_converter(converter, priority=-1) + with pytest.raises(FileConversionException) as aggregate: + no_fallback.convert_stream(io.BytesIO(data), stream_info=_INFO) + assert aggregate.value.attempts is not None + assert any( + attempt.exc_info and attempt.exc_info[1] is error + for attempt in aggregate.value.attempts + ) + + +def test_older_core_fails_with_actionable_upgrade_message( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delattr(_xlsx_converter.XlsxConverter, "_image_to_html") + with pytest.raises(RuntimeError, match=r"markitdown>=0\.1\.8b3") as caught: + XlsxConverterWithOCR() + assert "pip install --upgrade 'markitdown>=0.1.8b3'" in str(caught.value) diff --git a/packages/markitdown/src/markitdown/converter_utils/_image.py b/packages/markitdown/src/markitdown/converter_utils/_image.py new file mode 100644 index 0000000000..0efef4bee3 --- /dev/null +++ b/packages/markitdown/src/markitdown/converter_utils/_image.py @@ -0,0 +1,19 @@ +"""Shared validation for Office image-rendering hooks.""" + +from typing import Optional + +from bs4 import BeautifulSoup, Doctype + + +def _parse_image_html(fragment: Optional[str]) -> Optional[BeautifulSoup]: + if fragment is not None and not isinstance(fragment, str): + raise TypeError("_image_to_html must return an HTML string or None") + if fragment is None or not fragment.strip(): + return None + + soup = BeautifulSoup(fragment, "html.parser") + if soup.find(["html", "head", "body"]) or any( + isinstance(node, Doctype) for node in soup.descendants + ): + raise ValueError("_image_to_html must return a fragment, not a document") + return soup diff --git a/packages/markitdown/src/markitdown/converter_utils/_xlsx_images.py b/packages/markitdown/src/markitdown/converter_utils/_xlsx_images.py new file mode 100644 index 0000000000..7eb0bb2776 --- /dev/null +++ b/packages/markitdown/src/markitdown/converter_utils/_xlsx_images.py @@ -0,0 +1,115 @@ +"""Read spreadsheet drawing images without reloading or resaving the workbook.""" + +import io +import mimetypes +import posixpath +import zipfile +from typing import Any, BinaryIO, Callable, Optional +from urllib.parse import unquote + +from defusedxml import ElementTree as ET + +from .._stream_info import StreamInfo +from ._image import _parse_image_html + + +_NS = { + "s": "http://schemas.openxmlformats.org/spreadsheetml/2006/main", + "r": "http://schemas.openxmlformats.org/officeDocument/2006/relationships", + "xdr": "http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing", + "a": "http://schemas.openxmlformats.org/drawingml/2006/main", +} +_REL_ID = "{" + _NS["r"] + "}id" +_EMBED = "{" + _NS["r"] + "}embed" + + +def _relationships( + archive: zipfile.ZipFile, part: str, kind: Optional[str] = None +) -> dict[str, str]: + directory, filename = posixpath.split(part) + rels = posixpath.join(directory, "_rels", filename + ".rels") + if rels not in archive.namelist(): + return {} + root = ET.fromstring(archive.read(rels)) + return { + rel.attrib["Id"]: posixpath.normpath( + posixpath.join(directory, unquote(rel.attrib["Target"])) + ).lstrip("/") + for rel in root + if rel.get("TargetMode") != "External" + and (kind is None or rel.attrib["Type"].endswith("/" + kind)) + } + + +class _XlsxImages: + def __init__(self, file_stream: BinaryIO): + self._sheets: dict[str, list[tuple[bytes, StreamInfo]]] = {} + with zipfile.ZipFile(file_stream) as archive: + workbook = _relationships(archive, "", "officeDocument") + workbook_part = next(iter(workbook.values())) + sheets = ET.fromstring(archive.read(workbook_part)) + sheet_parts = _relationships(archive, workbook_part) + content_types = ET.fromstring(archive.read("[Content_Types].xml")) + defaults = { + item.attrib["Extension"].lower(): item.attrib["ContentType"] + for item in content_types + if "Extension" in item.attrib + } + overrides = { + unquote(item.attrib["PartName"]).lstrip("/"): item.attrib["ContentType"] + for item in content_types + if "PartName" in item.attrib + } + for sheet in sheets.findall("s:sheets/s:sheet", _NS): + sheet_part = sheet_parts[sheet.attrib[_REL_ID]] + drawings = ET.fromstring(archive.read(sheet_part)).findall( + "s:drawing", _NS + ) + if not drawings: + continue + drawing_parts = _relationships(archive, sheet_part) + images = self._sheets.setdefault(sheet.attrib["name"], []) + for drawing in drawings: + drawing_part = drawing_parts[drawing.attrib[_REL_ID]] + image_parts = _relationships(archive, drawing_part) + drawing_root = ET.fromstring(archive.read(drawing_part)) + # Match openpyxl's image traversal, not its XML serialization order. + anchors = [ + anchor + for kind in ("absoluteAnchor", "oneCellAnchor", "twoCellAnchor") + for anchor in drawing_root.findall(f"xdr:{kind}", _NS) + ] + for anchor in anchors: + for blip in anchor.findall(".//a:blip", _NS): + relationship = blip.get(_EMBED) + if relationship is None: + # Linked images are not embedded package content. + continue + image_part = image_parts[relationship] + extension = posixpath.splitext(image_part)[1].lower() + info = StreamInfo( + mimetype=overrides.get(image_part) + or defaults.get(extension.lstrip(".")) + or mimetypes.guess_type(image_part)[0], + extension=extension, + filename=posixpath.basename(image_part), + ) + images.append((archive.read(image_part), info)) + + def to_html( + self, + sheet_name: str, + render: Callable[..., Optional[str]], + options: dict[str, Any], + ) -> str: + fragments = [] + for data, info in self._sheets.get(sheet_name, []): + with io.BytesIO(data) as image_stream: + fragment = render(image_stream, info, **options) + soup = _parse_image_html(fragment) + if soup is None: + continue + fragments.append(f"
{soup}
") + if not fragments: + return "" + return "

Images in this sheet:

" + "".join(fragments) diff --git a/packages/markitdown/src/markitdown/converter_utils/docx/_images.py b/packages/markitdown/src/markitdown/converter_utils/docx/_images.py index a873046709..99ee670fb4 100644 --- a/packages/markitdown/src/markitdown/converter_utils/docx/_images.py +++ b/packages/markitdown/src/markitdown/converter_utils/docx/_images.py @@ -4,12 +4,13 @@ from typing import Any, Callable, Optional from uuid import uuid4 -from bs4 import BeautifulSoup, Doctype, Tag +from bs4 import BeautifulSoup, Tag from bs4.builder import HTMLTreeBuilder from mammoth import html, images from mammoth.docx.files import InvalidFileReferenceError from ..._stream_info import StreamInfo +from .._image import _parse_image_html _BLOCK_ELEMENTS = HTMLTreeBuilder.DEFAULT_BLOCK_ELEMENTS | { @@ -73,16 +74,10 @@ def convert_image(self, image: Any) -> list[Any]: # Mammoth swallows this exception for missing document images, # but an error raised by the override must reach the dispatcher. raise RuntimeError("_image_to_html failed") from exc - if fragment is not None and not isinstance(fragment, str): - raise TypeError("_image_to_html must return an HTML string or None") - if fragment is None or not fragment.strip(): + soup = _parse_image_html(fragment) + if soup is None: return images.data_uri(image) - soup = BeautifulSoup(fragment, "html.parser") - if soup.find(["html", "head", "body"]) or any( - isinstance(node, Doctype) for node in soup.descendants - ): - raise ValueError("_image_to_html must return a fragment, not a document") key = str(len(self._fragments)) self._fragments[key] = soup return [html.element("img", {self._attribute: key})] diff --git a/packages/markitdown/src/markitdown/converters/_pptx_converter.py b/packages/markitdown/src/markitdown/converters/_pptx_converter.py index 9b1c64366b..35ed21d658 100644 --- a/packages/markitdown/src/markitdown/converters/_pptx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_pptx_converter.py @@ -5,11 +5,11 @@ import re import html -from typing import BinaryIO, Any -from operator import attrgetter +from typing import BinaryIO, Any, Optional from ._html_converter import HtmlConverter from ._llm_caption import llm_caption +from ..converter_utils._image import _parse_image_html from .._base_converter import DocumentConverter, DocumentConverterResult from .._stream_info import StreamInfo from .._exceptions import MissingDependencyException, MISSING_DEPENDENCY_MESSAGE @@ -93,80 +93,7 @@ def get_shape_content(shape, **kwargs): nonlocal md_content # Pictures if self._is_picture(shape): - # https://github.com/scanny/python-pptx/pull/512#issuecomment-1713100069 - - llm_description = "" - alt_text = "" - - # Resolve the image blob, handling SVG images that lack a - # rasterized fallback (shape.image raises in that case). - ( - image_blob, - image_content_type, - image_filename, - ) = self._get_image_info(shape) - - # Potentially generate a description using an LLM - llm_client = kwargs.get("llm_client") - llm_model = kwargs.get("llm_model") - if ( - llm_client is not None - and llm_model is not None - and image_blob is not None - ): - # Prepare a file_stream and stream_info for the image data - image_extension = None - if image_filename: - image_extension = os.path.splitext(image_filename)[1] - image_stream_info = StreamInfo( - mimetype=image_content_type, - extension=image_extension, - filename=image_filename, - ) - - image_stream = io.BytesIO(image_blob) - - # Caption the image - try: - llm_description = llm_caption( - image_stream, - image_stream_info, - client=llm_client, - model=llm_model, - prompt=kwargs.get("llm_prompt"), - ) - except Exception: - # Unable to generate a description - pass - - # Also grab any description embedded in the deck - try: - alt_text = shape._element._nvXxPr.cNvPr.attrib.get("descr", "") - except Exception: - # Unable to get alt text - pass - - # Prepare the alt, escaping any special characters - alt_text = ( - "\n".join( - text - for text in [llm_description, alt_text] - if text and text.strip() - ) - or shape.name - ) - alt_text = re.sub(r"[\r\n\[\]]", " ", alt_text) - alt_text = re.sub(r"\s+", " ", alt_text).strip() - - # If keep_data_uris is True, use base64 encoding for images - if kwargs.get("keep_data_uris", False) and image_blob is not None: - content_type = image_content_type or "image/png" - b64_string = base64.b64encode(image_blob).decode("utf-8") - md_content += f"\n![{alt_text}](data:{content_type};base64,{b64_string})\n" - else: - # A placeholder name - filename = re.sub(r"\W", "", shape.name) + ".jpg" - md_content += "\n![" + alt_text + "](" + filename + ")\n" + md_content += self._convert_picture_to_markdown(shape, **kwargs) # Tables if self._is_table(shape): @@ -221,6 +148,90 @@ def get_shape_content(shape, **kwargs): return DocumentConverterResult(markdown=md_content.strip()) + def _image_to_html( + self, + image_stream: BinaryIO, + stream_info: StreamInfo, + **kwargs: Any, + ) -> Optional[str]: + """Override to render an embedded image as an HTML fragment. + + The stream is borrowed, seekable, and positioned at zero; do not close + or retain it. StreamInfo describes the image, not the presentation. + Existing conversion options are forwarded through kwargs. + + Native LLM captions take precedence. Otherwise, return None or blank + text to retain the native image representation, or HTML with literal + text escaped. The fragment passes through HtmlConverter before being + placed at the picture's position in slide/group order. Hook failures + propagate through the normal conversion failure path. + """ + return None + + def _convert_picture_to_markdown(self, shape, **kwargs): + llm_description = "" + alt_text = "" + image_blob, image_content_type, image_filename = self._get_image_info(shape) + image_stream_info = StreamInfo( + mimetype=image_content_type, + extension=os.path.splitext(image_filename)[1] if image_filename else None, + filename=image_filename, + ) + + llm_client = kwargs.get("llm_client") + llm_model = kwargs.get("llm_model") + if llm_client is not None and llm_model is not None and image_blob is not None: + with io.BytesIO(image_blob) as image_stream: + try: + llm_description = llm_caption( + image_stream, + image_stream_info, + client=llm_client, + model=llm_model, + prompt=kwargs.get("llm_prompt"), + ) + except Exception: + # Preserve native caption failure fallback. + pass + + if ( + not llm_description + and image_blob is not None + and type(self)._image_to_html is not PptxConverter._image_to_html + ): + with io.BytesIO(image_blob) as image_stream: + fragment = self._image_to_html( + image_stream, image_stream_info, **kwargs + ) + soup = _parse_image_html(fragment) + if soup is not None: + return ( + "\n" + + self._html_converter.convert_string(str(soup), **kwargs).markdown + + "\n" + ) + + # Keep native caption/alt Markdown separate from custom image HTML. + try: + alt_text = shape._element._nvXxPr.cNvPr.attrib.get("descr", "") + except Exception: + pass + alt_text = ( + "\n".join( + text for text in [llm_description, alt_text] if text and text.strip() + ) + or shape.name + ) + alt_text = re.sub(r"[\r\n\[\]]", " ", alt_text) + alt_text = re.sub(r"\s+", " ", alt_text).strip() + + if kwargs.get("keep_data_uris", False) and image_blob is not None: + content_type = image_content_type or "image/png" + b64_string = base64.b64encode(image_blob).decode("utf-8") + return f"\n![{alt_text}](data:{content_type};base64,{b64_string})\n" + filename = re.sub(r"\W", "", shape.name) + ".jpg" + return "\n![" + alt_text + "](" + filename + ")\n" + def _find_svg_blip_part(self, shape): """Return the image part referenced by an ````, if any. diff --git a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py index 355dd8f8d7..9f794a3b77 100644 --- a/packages/markitdown/src/markitdown/converters/_xlsx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_xlsx_converter.py @@ -2,7 +2,8 @@ import re import sys import zipfile -from typing import BinaryIO, Any +from contextlib import contextmanager +from typing import BinaryIO, Any, Iterator, Optional from ._html_converter import HtmlConverter from .._base_converter import DocumentConverter, DocumentConverterResult from .._exceptions import MissingDependencyException, MISSING_DEPENDENCY_MESSAGE @@ -43,16 +44,24 @@ _SHOW_ZEROES_ATTRIBUTE = re.compile(rb"(?<=[\s])showZeroes(\s*=)") -def _read_xlsx_sheets(file_stream: BinaryIO) -> dict[str, Any]: +@contextmanager +def _read_xlsx_sheets( + file_stream: BinaryIO, +) -> Iterator[tuple[dict[str, Any], BinaryIO]]: start_pos = file_stream.tell() + repaired_stream = None try: - return pd.read_excel(file_stream, sheet_name=None, engine="openpyxl") - except TypeError as exc: - if "showZeroes" not in str(exc): - raise - - repaired_stream = _repair_sheetview_show_zeroes(file_stream, start_pos) - return pd.read_excel(repaired_stream, sheet_name=None, engine="openpyxl") + try: + sheets = pd.read_excel(file_stream, sheet_name=None, engine="openpyxl") + except TypeError as exc: + if "showZeroes" not in str(exc): + raise + repaired_stream = _repair_sheetview_show_zeroes(file_stream, start_pos) + sheets = pd.read_excel(repaired_stream, sheet_name=None, engine="openpyxl") + yield sheets, repaired_stream if repaired_stream is not None else file_stream + finally: + if repaired_stream is not None: + repaired_stream.close() def _rename_show_zeroes_attribute(data: bytes) -> bytes: @@ -131,20 +140,55 @@ def convert( _xlsx_dependency_exc_info[2] ) - sheets = _read_xlsx_sheets(file_stream) md_content = "" - for s in sheets: - md_content += f"## {s}\n" - html_content = sheets[s].to_html(index=False) - md_content += ( - self._html_converter.convert_string( - html_content, **kwargs - ).markdown.strip() - + "\n\n" - ) + with _read_xlsx_sheets(file_stream) as (sheets, workbook_stream): + images = None + if type(self)._image_to_html is not XlsxConverter._image_to_html: + from ..converter_utils._xlsx_images import _XlsxImages + + images = _XlsxImages(workbook_stream) + + for s in sheets: + md_content += f"## {s}\n" + html_content = sheets[s].to_html(index=False) + md_content += ( + self._html_converter.convert_string( + html_content, **kwargs + ).markdown.strip() + + "\n\n" + ) + if images is not None: + image_content = images.to_html(s, self._image_to_html, kwargs) + if image_content: + md_content += ( + self._html_converter.convert_string( + image_content, **kwargs + ).markdown.strip() + + "\n\n" + ) return DocumentConverterResult(markdown=md_content.strip()) + def _image_to_html( + self, + image_stream: BinaryIO, + stream_info: StreamInfo, + **kwargs: Any, + ) -> Optional[str]: + """Override to render an embedded image as an HTML fragment. + + The stream is borrowed, seekable, and positioned at zero; do not close + or retain it. StreamInfo describes the image, not the workbook. Existing + conversion options are forwarded through kwargs. + + Return None or blank text to retain the native representation (no image + output). Otherwise return HTML, escaping any literal text. Images appear + after their sheet's table, in the existing worksheet image order. Linked + images are not fetched. Hook failures propagate through the normal + conversion failure path. + """ + return None + class XlsConverter(DocumentConverter): """ diff --git a/packages/markitdown/tests/test_pptx_images.py b/packages/markitdown/tests/test_pptx_images.py new file mode 100644 index 0000000000..fa6d579aef --- /dev/null +++ b/packages/markitdown/tests/test_pptx_images.py @@ -0,0 +1,502 @@ +"""PPTX image hooks share native slide traversal and HTML rendering.""" + +import base64 +import inspect +import io +from pathlib import Path +from typing import Any, BinaryIO, Callable, Optional +from unittest.mock import Mock + +from bs4 import BeautifulSoup +from lxml import etree +from pptx import Presentation +from pptx.chart.data import CategoryChartData +from pptx.enum.chart import XL_CHART_TYPE +from pptx.enum.shapes import MSO_SHAPE_TYPE +from pptx.util import Inches +import pytest + +from markitdown import ( + FileConversionException, + MarkItDown, + MissingDependencyException, + StreamInfo, +) +from markitdown.converters import HtmlConverter, PptxConverter +from markitdown.converters import _pptx_converter + + +_PNG = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAM" + "BAQDJ/pLvAAAAAElFTkSuQmCC" +) +_GIF = base64.b64decode("R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBTAA7") +_INFO = StreamInfo(extension=".pptx") +_FILES = Path(__file__).parent / "test_files" + + +def _presentation( + images: tuple[bytes, ...] = (_PNG,), *, native_content: bool = False +) -> io.BytesIO: + presentation = Presentation() + slide = presentation.slides.add_slide(presentation.slide_layouts[5]) + slide.shapes.title.text = "Deck title" + slide.shapes.title.top = 0 + # Insert in reverse order to distinguish shape order from reading order. + for index in reversed(range(len(images))): + image = slide.shapes.add_picture( + io.BytesIO(images[index]), 0, Inches(index + 1), width=Inches(0.5) + ) + image.name = f"Picture {index + 1}" + image._element._nvXxPr.cNvPr.set("descr", f"Alt [{index + 1}]\r\n& text") + if native_content: + table = slide.shapes.add_table(2, 2, 0, Inches(4), Inches(4), Inches(1)).table + for cell, value in zip( + (table.cell(0, 0), table.cell(0, 1), table.cell(1, 0), table.cell(1, 1)), + ("Item", "Details", "A", "B & "), + ): + cell.text = value + chart_data = CategoryChartData() + chart_data.categories = ["Q1", "Q2"] + chart_data.add_series("Revenue", (10, 20)) + chart = slide.shapes.add_chart( + XL_CHART_TYPE.COLUMN_CLUSTERED, + 0, + Inches(5), + Inches(4), + Inches(1), + chart_data, + ).chart + chart.has_title = True + chart.chart_title.text_frame.text = "Sales" + slide.notes_slide.notes_text_frame.text = "Speaker notes" + next_slide = presentation.slides.add_slide(presentation.slide_layouts[5]) + next_slide.shapes.title.text = "" + next_slide.shapes.add_textbox(0, 0, Inches(2), Inches(1)).text = "Closing" + next_slide.notes_slide.notes_text_frame.text = " \n " + stream = io.BytesIO() + presentation.save(stream) + stream.seek(0) + return stream + + +class _ImageConverter(PptxConverter): + def __init__(self, render: Callable[..., Optional[str]]): + super().__init__() + self.render = render + + def _image_to_html( + self, image_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any + ) -> Optional[str]: + return self.render(image_stream, stream_info, **kwargs) + + +def test_hook_does_not_change_public_signature() -> None: + assert list(inspect.signature(PptxConverter.convert).parameters) == [ + "self", + "file_stream", + "stream_info", + "kwargs", + ] + assert list(inspect.signature(PptxConverter.__init__).parameters) == ["self"] + assert _ImageConverter.convert is PptxConverter.convert + + +@pytest.mark.parametrize("via_dispatcher", [False, True]) +def test_inherited_hook_receives_real_images_and_options_in_reading_order( + via_dispatcher: bool, +) -> None: + seen = [] + streams = [] + service = object() + + def render(stream: BinaryIO, info: StreamInfo, **kwargs: Any) -> str: + assert stream.tell() == 0 and stream.seekable() + data = stream.read() + stream.seek(0) + assert stream.read() == data + seen.append((data, info, kwargs)) + streams.append(stream) + return f"Image {len(seen)}" + + class InheritedImages(_ImageConverter): + pass + + converter = InheritedImages(render) + source = _presentation((_GIF, _PNG, _GIF)) + original = source.getvalue() + info = StreamInfo( + extension=".pptx", + filename="deck.pptx", + url="https://example.test/deck.pptx", + local_path="/deck.pptx", + ) + options: dict[str, Any] = {"ocr_service": service, "escape_underscores": False} + if via_dispatcher: + md = MarkItDown() + md.register_converter(converter, priority=-1) + result = md.convert_stream(source, stream_info=info, **options) + else: + source.seek(7) + result = converter.convert(source, info, **options) + + assert [entry[0] for entry in seen] == [_GIF, _PNG, _GIF] + assert [entry[1] for entry in seen] == [ + StreamInfo( + mimetype=f"image/{ext}", extension=f".{ext}", filename=f"image.{ext}" + ) + for ext in ("gif", "png", "gif") + ] + assert all(entry[2]["ocr_service"] is service for entry in seen) + assert all(entry[2]["escape_underscores"] is False for entry in seen) + assert result.markdown == ( + "\n# Deck title\n" + "\n**Image 1**\n\n**Image 2**\n\n**Image 3**" + ) + assert result.title is None + assert all(stream.closed for stream in streams) + assert not source.closed and source.getvalue() == original + + +@pytest.mark.parametrize( + ("fallback", "keep_data_uris"), [(None, False), ("", True), (" \r\n\t", False)] +) +def test_declining_hook_preserves_native_output_and_metadata( + fallback: Optional[str], keep_data_uris: bool +) -> None: + expected = PptxConverter().convert( + _presentation(native_content=True), _INFO, keep_data_uris=keep_data_uris + ) + render = Mock(return_value=fallback) + actual = _ImageConverter(render).convert( + _presentation(native_content=True), _INFO, keep_data_uris=keep_data_uris + ) + + assert actual.markdown == expected.markdown + assert actual.title == expected.title + render.assert_called_once() + + +@pytest.mark.parametrize("keep_data_uris", [False, True]) +def test_declining_hook_preserves_real_presentation(keep_data_uris: bool) -> None: + data = (_FILES / "test.pptx").read_bytes() + expected = PptxConverter().convert( + io.BytesIO(data), _INFO, keep_data_uris=keep_data_uris + ) + render = Mock(return_value=None) + + actual = _ImageConverter(render).convert( + io.BytesIO(data), _INFO, keep_data_uris=keep_data_uris + ) + + assert actual.markdown == expected.markdown + assert actual.title == expected.title + assert render.call_count > 0 + + +def test_native_converter_does_not_add_image_html_processing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class NativeImages(PptxConverter): + pass + + for converter in (PptxConverter(), NativeImages()): + convert_html = Mock(side_effect=AssertionError("unexpected HTML")) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + assert ( + "![Alt 1 & text](Picture1.jpg)" + in converter.convert(_presentation(), _INFO).markdown + ) + convert_html.assert_not_called() + + +def test_override_can_delegate_to_super() -> None: + class NativeImages(PptxConverter): + def _image_to_html(self, image_stream, stream_info, **kwargs): + return super()._image_to_html(image_stream, stream_info, **kwargs) + + assert ( + NativeImages().convert(_presentation(), _INFO).markdown + == PptxConverter().convert(_presentation(), _INFO).markdown + ) + + +def test_custom_fragment_uses_html_options_without_reprocessing_generated_images( + monkeypatch: pytest.MonkeyPatch, +) -> None: + fragment = ( + "

Image heading

A_B & <literal>
second

" + 'generated' + ) + render = Mock(return_value=fragment) + converter = _ImageConverter(render) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + options: dict[str, Any] = { + "escape_underscores": False, + "heading_style": "underlined", + } + + result = converter.convert(_presentation(), _INFO, **options) + + convert_html.assert_called_once_with( + str(BeautifulSoup(fragment, "html.parser")), **options + ) + expected = HtmlConverter().convert_string(fragment, **options).markdown + assert result.markdown == "\n# Deck title\n\n" + expected + render.assert_called_once() + + +def test_replacements_keep_native_table_chart_notes_and_slide_placement() -> None: + render = Mock(side_effect=["

first

", None, "

third

"]) + result = _ImageConverter(render).convert( + _presentation((_PNG, _GIF, _PNG), native_content=True), _INFO + ) + markdown = result.markdown + ordered = [ + "# Deck title", + "first", + "![Alt 2 & text](Picture2.jpg)", + "third", + "| Item | Details |", + "| A | B & |", + "### Chart: Sales", + "| Q1 | 10.0 |", + "### Notes:\nSpeaker notes", + "", + "Closing", + ] + assert [markdown.index(text) for text in ordered] == sorted( + markdown.index(text) for text in ordered + ) + assert markdown.count("### Notes:") == 1 + assert "\n# \n" not in markdown + + +def test_group_images_follow_native_negative_and_zero_coordinate_order() -> None: + presentation = Presentation() + slide = presentation.slides.add_slide(presentation.slide_layouts[6]) + group = slide.shapes.add_group_shape() + group.shapes.add_picture(io.BytesIO(_PNG), 0, 0, width=Inches(0.5)) + group.shapes.add_picture(io.BytesIO(_GIF), 0, -Inches(1), width=Inches(0.5)) + stream = io.BytesIO() + presentation.save(stream) + stream.seek(0) + seen = [] + + def render(image_stream, info, **kwargs): + seen.append(image_stream.read()) + return f"

image{len(seen)}

" + + result = _ImageConverter(render).convert(stream, _INFO) + + assert seen == [_GIF, _PNG] + assert result.markdown == "\n\nimage1\n\nimage2" + + +def test_picture_placeholder_invokes_hook() -> None: + presentation = Presentation() + slide = presentation.slides.add_slide(presentation.slide_layouts[8]) + placeholder = next( + shape for shape in slide.placeholders if hasattr(shape, "insert_picture") + ) + placeholder.insert_picture(io.BytesIO(_PNG)) + stream = io.BytesIO() + presentation.save(stream) + stream.seek(0) + render = Mock(return_value="

placeholder

") + + result = _ImageConverter(render).convert(stream, _INFO) + + assert result.markdown == "\n\nplaceholder" + render.assert_called_once() + + +def test_svg_without_raster_fallback_reaches_image_hook() -> None: + seen = [] + + def render(stream, info, **kwargs): + seen.append((stream.read(), info)) + return "

SVG content

" + + with (_FILES / "test_svg_no_fallback.pptx").open("rb") as stream: + result = _ImageConverter(render).convert(stream, _INFO) + + assert len(seen) == 1 and b" None: + presentation = Presentation(str(_FILES / "test_svg_no_fallback.pptx")) + picture = next( + shape + for slide in presentation.slides + for shape in slide.shapes + if PptxConverter()._is_picture(shape) + ) + nonvisual = picture._element.xpath("./p:nvPicPr/p:nvPr")[0] + etree.SubElement( + nonvisual, + "{http://schemas.openxmlformats.org/presentationml/2006/main}ph", + type="pic", + idx="1", + ) + stream = io.BytesIO() + presentation.save(stream) + stream.seek(0) + presentation = Presentation(stream) + placeholder = next( + shape + for slide in presentation.slides + for shape in slide.shapes + if shape.shape_type == MSO_SHAPE_TYPE.PLACEHOLDER + ) + with pytest.raises(ValueError, match="no embedded image"): + _ = placeholder.image + stream.seek(0) + render = Mock(return_value="

SVG placeholder

") + + result = _ImageConverter(render).convert(stream, _INFO) + + assert "SVG placeholder" in result.markdown + assert render.call_args.args[1].mimetype == "image/svg+xml" + render.assert_called_once() + + +def test_missing_image_bytes_keep_native_placeholder( + monkeypatch: pytest.MonkeyPatch, +) -> None: + render = Mock(side_effect=AssertionError("unexpected image")) + converter = _ImageConverter(render) + monkeypatch.setattr( + converter, "_get_image_info", Mock(return_value=(None, None, None)) + ) + + result = converter.convert(_presentation(), _INFO, keep_data_uris=True) + + assert "![Alt 1 & text](Picture1.jpg)" in result.markdown + render.assert_not_called() + + +@pytest.mark.parametrize("value", [False, 123, b"

not text

"]) +def test_non_string_fragments_fail_explicitly(value: Any) -> None: + with pytest.raises(TypeError, match="HTML string or None"): + _ImageConverter(Mock(return_value=value)).convert(_presentation(), _INFO) + + +@pytest.mark.parametrize( + "fragment", + [ + "document", + "document", + "document", + "

document

", + ], +) +def test_full_documents_fail_explicitly(fragment: str) -> None: + with pytest.raises(ValueError, match="fragment, not a document"): + _ImageConverter(Mock(return_value=fragment)).convert(_presentation(), _INFO) + + +def test_hook_errors_propagate_close_stream_and_use_dispatcher_fallback() -> None: + streams = [] + error = RuntimeError("image renderer failed") + + def render(stream, info, **kwargs): + streams.append(stream) + raise error + + converter = _ImageConverter(render) + with pytest.raises(RuntimeError) as caught: + converter.convert(_presentation(), _INFO) + assert caught.value is error + without_fallback = MarkItDown(enable_builtins=False) + without_fallback.register_converter(converter) + with pytest.raises(FileConversionException) as aggregate: + without_fallback.convert_stream(_presentation(), stream_info=_INFO) + assert aggregate.value.attempts is not None + assert any( + attempt.exc_info and attempt.exc_info[1] is error + for attempt in aggregate.value.attempts + ) + md = MarkItDown() + md.register_converter(converter, priority=-1) + assert ( + md.convert_stream(_presentation(), stream_info=_INFO).markdown + == PptxConverter().convert(_presentation(), _INFO).markdown + ) + assert streams and all(stream.closed for stream in streams) + + +@pytest.mark.parametrize("caption_result", [None, "", RuntimeError("caption failed")]) +def test_caption_failure_precedes_hook_with_fresh_image_stream( + monkeypatch: pytest.MonkeyPatch, caption_result: Any +) -> None: + calls = [] + client = object() + + def caption(stream, info, **kwargs): + assert stream.tell() == 0 and stream.read() == _PNG + assert info.mimetype == "image/png" and info.extension == ".png" + assert kwargs == {"client": client, "model": "vision", "prompt": "Describe"} + calls.append("caption") + if isinstance(caption_result, Exception): + raise caption_result + return caption_result + + def render(stream, info, **kwargs): + assert stream.tell() == 0 and stream.read() == _PNG + calls.append("hook") + return "

recognized

" + + monkeypatch.setattr(_pptx_converter, "llm_caption", caption) + result = _ImageConverter(render).convert( + _presentation(), + _INFO, + llm_client=client, + llm_model="vision", + llm_prompt="Describe", + ) + + assert calls == ["caption", "hook"] + assert "recognized" in result.markdown + + +def test_successful_caption_keeps_native_markdown_without_hook_or_html( + monkeypatch: pytest.MonkeyPatch, +) -> None: + caption = Mock(return_value="**Caption** [label]\nwith_under & ") + monkeypatch.setattr(_pptx_converter, "llm_caption", caption) + render = Mock(side_effect=AssertionError("caption must take precedence")) + converter = _ImageConverter(render) + convert_html = Mock(side_effect=AssertionError("caption is not image HTML")) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + result = converter.convert( + _presentation(), _INFO, llm_client=object(), llm_model="vision" + ) + + assert result.markdown == ( + "\n# Deck title\n\n" + "![**Caption** label with_under & Alt 1 & text](Picture1.jpg)" + ) + caption.assert_called_once() + render.assert_not_called() + convert_html.assert_not_called() + + +def test_missing_optional_dependencies_fail_before_hook( + monkeypatch: pytest.MonkeyPatch, +) -> None: + error = ImportError("python-pptx is unavailable") + monkeypatch.setattr( + _pptx_converter, "_dependency_exc_info", (ImportError, error, None) + ) + render = Mock() + with pytest.raises(MissingDependencyException) as caught: + _ImageConverter(render).convert(io.BytesIO(b""), _INFO) + assert caught.value.__cause__ is error + render.assert_not_called() diff --git a/packages/markitdown/tests/test_xlsx_images.py b/packages/markitdown/tests/test_xlsx_images.py new file mode 100644 index 0000000000..3dce30bfce --- /dev/null +++ b/packages/markitdown/tests/test_xlsx_images.py @@ -0,0 +1,466 @@ +"""Spreadsheet image hooks retain the native table pipeline and package bytes.""" + +import inspect +import io +from pathlib import Path +from typing import Any, BinaryIO, Callable, Optional +from unittest.mock import Mock +import zipfile +from xml.etree import ElementTree as ET + +from bs4 import BeautifulSoup +from defusedxml.common import EntitiesForbidden +import openpyxl +from openpyxl.drawing.image import Image as SheetImage +from openpyxl.drawing.spreadsheet_drawing import ( + AbsoluteAnchor, + AnchorMarker, + OneCellAnchor, + TwoCellAnchor, +) +from PIL import Image +import pytest + +from markitdown import MissingDependencyException, StreamInfo +from markitdown.converters import XlsConverter, XlsxConverter +from markitdown.converters import _xlsx_converter +from markitdown.converter_utils import _xlsx_images + + +_INFO = StreamInfo(extension=".xlsx") + + +def _png(color: str) -> bytes: + stream = io.BytesIO() + Image.new("RGB", (2, 2), color).save(stream, "PNG") + return stream.getvalue() + + +_RED, _BLUE = _png("red"), _png("blue") + + +def _workbook(*, images: bool = True) -> bytes: + workbook = openpyxl.Workbook() + first = workbook.active + first.title = "Second" + first.append(["Header_one", None]) + first.append([" & *value*", 12]) + last = workbook.create_sheet("First") + last.append(["Other"]) + last.append(["End"]) + if images: + for data, anchor in ( + (_RED, OneCellAnchor(_from=AnchorMarker(col=26, row=2))), + ( + _BLUE, + TwoCellAnchor( + _from=AnchorMarker(col=2, row=4), to=AnchorMarker(col=4, row=7) + ), + ), + (_RED, AbsoluteAnchor()), + ): + image = SheetImage(io.BytesIO(data)) + image.anchor = anchor + first.add_image(image) + last.add_image(SheetImage(io.BytesIO(_BLUE)), "Z10") + stream = io.BytesIO() + workbook.save(stream) + workbook.close() + return stream.getvalue() + + +def _rewrite(data: bytes, change: Callable[[str, bytes], bytes]) -> bytes: + output = io.BytesIO() + with zipfile.ZipFile(io.BytesIO(data)) as source: + with zipfile.ZipFile(output, "w") as target: + for entry in source.infolist(): + target.writestr(entry, change(entry.filename, source.read(entry))) + return output.getvalue() + + +class _ImageConverter(XlsxConverter): + def __init__(self, render: Callable[..., Optional[str]]): + super().__init__() + self.render = render + + def _image_to_html( + self, image_stream: BinaryIO, stream_info: StreamInfo, **kwargs: Any + ) -> Optional[str]: + return self.render(image_stream, stream_info, **kwargs) + + +def _convert(converter: XlsxConverter, data: bytes, **kwargs: Any) -> str: + return converter.convert(io.BytesIO(data), _INFO, **kwargs).markdown + + +def test_image_hook_keeps_native_public_signature_and_acceptance() -> None: + assert list(inspect.signature(XlsxConverter.convert).parameters) == [ + "self", + "file_stream", + "stream_info", + "kwargs", + ] + assert list(inspect.signature(XlsxConverter.__init__).parameters) == ["self"] + assert _ImageConverter.convert is XlsxConverter.convert + converter = XlsxConverter() + assert converter.accepts(io.BytesIO(), StreamInfo(extension=".XLSX")) + assert converter.accepts( + io.BytesIO(), + StreamInfo( + mimetype="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" + ), + ) + assert not converter.accepts(io.BytesIO(), StreamInfo(extension=".xls")) + + +def test_hook_receives_original_images_metadata_options_and_legacy_anchor_order() -> ( + None +): + seen = [] + streams = [] + service = object() + + def render(stream: BinaryIO, info: StreamInfo, **kwargs: Any) -> str: + assert stream.seekable() and stream.tell() == 0 + image = stream.read() + stream.seek(0) + assert stream.read() == image + seen.append((image, info, kwargs)) + streams.append(stream) + return f"

image_{len(seen)}

" + + class InheritedImages(_ImageConverter): + pass + + source = io.BytesIO(_workbook()) + original = source.getvalue() + source.seek(7) + info = StreamInfo( + extension=".xlsx", + filename="parent.xlsx", + local_path="/parent.xlsx", + url="https://example.test/parent.xlsx", + ) + result = InheritedImages(render).convert( + source, + info, + ocr_service=service, + heading_style="atx", + escape_underscores=False, + filename="parent-option.xlsx", + url="https://example.test/option.xlsx", + ) + + # Preserve openpyxl's absolute, one-cell, two-cell traversal, not ZIP order. + assert [(data, metadata) for data, metadata, _ in seen] == [ + ( + _RED, + StreamInfo(extension=".png", mimetype="image/png", filename="image3.png"), + ), + ( + _RED, + StreamInfo(extension=".png", mimetype="image/png", filename="image1.png"), + ), + ( + _BLUE, + StreamInfo(extension=".png", mimetype="image/png", filename="image2.png"), + ), + ( + _BLUE, + StreamInfo(extension=".png", mimetype="image/png", filename="image4.png"), + ), + ] + assert all(options["ocr_service"] is service for _, _, options in seen) + assert all(options["filename"] == "parent-option.xlsx" for _, _, options in seen) + expected_order = [ + "## Second", + "| & \\*value\\* | 12 |", + "### Images in this sheet:", + "**image_1**", + "**image_2**", + "**image_3**", + "## First", + "| End |", + "**image_4**", + ] + offsets = [result.markdown.index(text) for text in expected_order] + assert offsets == sorted(offsets) + assert "Image at " not in result.markdown + assert all(stream.closed for stream in streams) + assert not source.closed and source.getvalue() == original + + +@pytest.mark.parametrize( + ("template", "expected"), + [ + ("{}", "**one**\n\n**two**\n\n**three**"), + ("{}", "one\n\ntwo\n\nthree"), + ("{}
line", "one \nline\n\ntwo \nline\n\nthree \nline"), + ( + "

{}

paragraph

", + "one\n\nparagraph\n\ntwo\n\nparagraph\n\nthree\n\nparagraph", + ), + ], +) +def test_image_fragments_are_separate_valid_blocks( + monkeypatch: pytest.MonkeyPatch, template: str, expected: str +) -> None: + render = Mock( + side_effect=[template.format(text) for text in ("one", "two", "three", "four")] + ) + converter = _ImageConverter(render) + convert_html = Mock(wraps=converter._html_converter.convert_string) + monkeypatch.setattr(converter._html_converter, "convert_string", convert_html) + + result = _convert(converter, _workbook()) + + assert f"### Images in this sheet:\n\n{expected}\n\n## First\n" in result + soup = BeautifulSoup(convert_html.call_args_list[1].args[0], "html.parser") + assert [tag.name for tag in soup.find_all(recursive=False)] == [ + "h3", + "div", + "div", + "div", + ] + assert not soup.select("p div") + + +@pytest.mark.parametrize("fragment", [None, "", " \n\t"]) +def test_declining_hook_is_exactly_native(fragment: Optional[str]) -> None: + data = _workbook() + options = {"escape_underscores": False, "heading_style": "underlined"} + assert _convert(_ImageConverter(Mock(return_value=fragment)), data, **options) == ( + _convert(XlsxConverter(), data, **options) + ) + + +def test_default_converter_does_not_parse_images_or_change_native_output( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + _xlsx_images, "_XlsxImages", Mock(side_effect=AssertionError("extra parsing")) + ) + reader = Mock(wraps=_xlsx_converter.pd.read_excel) + monkeypatch.setattr(_xlsx_converter.pd, "read_excel", reader) + + class NativeSubclass(XlsxConverter): + pass + + expected = ( + "## Second\n" + "| Header\\_one | Unnamed: 1 |\n" + "| --- | --- |\n" + "| & \\*value\\* | 12 |\n\n" + "## First\n| Other |\n| --- |\n| End |" + ) + for converter in (XlsxConverter(), NativeSubclass()): + assert _convert(converter, _workbook()) == expected + assert reader.call_count == 2 + + +def test_image_hook_uses_repaired_package_and_repair_stream_is_closed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + data = _rewrite( + _workbook(), + lambda name, content: ( + content.replace(b" io.BytesIO: + stream = original_repair(*args) + repaired_streams.append(stream) + return stream + + original_images = _xlsx_images._XlsxImages + + def images(stream: BinaryIO): + assert stream is repaired_streams[-1] + with zipfile.ZipFile(stream) as archive: + sheet = archive.read("xl/worksheets/sheet1.xml") + assert b'showZeros="0"' in sheet and b"showZeroes" not in sheet + return original_images(stream) + + monkeypatch.setattr(_xlsx_converter, "_repair_sheetview_show_zeroes", repair) + monkeypatch.setattr(_xlsx_images, "_XlsxImages", images) + render = Mock(return_value="

recognized

") + result = _convert(_ImageConverter(render), data) + assert result.count("recognized") == 4 + assert render.call_count == 4 + assert all(stream.closed for stream in repaired_streams) + + +@pytest.mark.parametrize( + ("fragment", "error"), + [ + (17, TypeError), + (b"

bytes

", TypeError), + ("

text

", ValueError), + ("text", ValueError), + ("title", ValueError), + ], +) +def test_invalid_hook_results_are_rejected( + fragment: Any, error: type[Exception] +) -> None: + with pytest.raises(error, match="_image_to_html must return"): + _convert(_ImageConverter(Mock(return_value=fragment)), _workbook()) + + +def test_hook_failure_closes_image_stream_and_remains_visible() -> None: + streams = [] + error = RuntimeError("image failure") + + def render(stream: BinaryIO, *args: Any, **kwargs: Any) -> str: + streams.append(stream) + raise error + + source = io.BytesIO(_workbook()) + with pytest.raises(RuntimeError) as caught: + _ImageConverter(render).convert(source, _INFO) + assert caught.value is error + assert streams and all(stream.closed for stream in streams) + assert not source.closed + + +def test_native_table_failure_is_not_silently_skipped( + monkeypatch: pytest.MonkeyPatch, +) -> None: + error = TypeError("not the repairable error") + monkeypatch.setattr(_xlsx_converter.pd, "read_excel", Mock(side_effect=error)) + render = Mock(return_value="

recognized

") + with pytest.raises(TypeError) as caught: + _convert(_ImageConverter(render), _workbook()) + assert caught.value is error + render.assert_not_called() + + +def test_package_relationships_preserve_unusual_image_names_types_and_bytes() -> None: + data = _workbook() + svg = b'vector' + output = io.BytesIO() + with zipfile.ZipFile(io.BytesIO(data)) as source: + with zipfile.ZipFile(output, "w") as target: + for entry in source.infolist(): + content = source.read(entry) + name = entry.filename + if name == "xl/media/image1.png": + name, content = "xl/media/vector image.SVG", svg + elif name.endswith(".rels"): + content = content.replace( + b"/xl/media/image1.png", b"../media/vector%20image.SVG" + ) + elif name == "[Content_Types].xml": + content = content.replace( + b"", + b'', + ) + target.writestr(name, content) + render = Mock(return_value="

recognized

") + + result = _convert(_ImageConverter(render), output.getvalue()) + + assert result.count("recognized") == 4 + assert render.call_args_list[1].args[1] == StreamInfo( + mimetype="image/svg+xml", extension=".svg", filename="vector image.SVG" + ) + # The read-only table reader ignores drawings, including vector images. + captured = [] + + def capture(stream, info, **kwargs): + captured.append(stream.read()) + return None + + _convert(_ImageConverter(capture), output.getvalue()) + assert captured == [_RED, svg, _BLUE, _BLUE] + + +def test_repeated_relationships_and_grouped_images_are_not_dropped() -> None: + def group(name: str, content: bytes) -> bytes: + if name != "xl/drawings/drawing1.xml": + return content + root = ET.fromstring(content) + ns = _xlsx_images._NS + anchor = root.find("xdr:oneCellAnchor", ns) + assert anchor is not None + picture = anchor.find("xdr:pic", ns) + assert picture is not None + anchor.remove(picture) + group = ET.SubElement(anchor, "{" + ns["xdr"] + "}grpSp") + nonvisual = ET.SubElement(group, "{" + ns["xdr"] + "}nvGrpSpPr") + ET.SubElement( + nonvisual, "{" + ns["xdr"] + "}cNvPr", {"id": "20", "name": "Group"} + ) + ET.SubElement(nonvisual, "{" + ns["xdr"] + "}cNvGrpSpPr") + ET.SubElement(group, "{" + ns["xdr"] + "}grpSpPr") + group.append(picture) + duplicate = ET.fromstring(ET.tostring(picture)) + properties = duplicate.find("xdr:nvPicPr/xdr:cNvPr", ns) + assert properties is not None + properties.set("id", "21") + group.append(duplicate) + return ET.tostring(root) + + captured = [] + + def render(stream, info, **kwargs): + captured.append(stream.read()) + return "

recognized

" + + result = _convert(_ImageConverter(render), _rewrite(_workbook(), group)) + assert captured == [_RED, _RED, _RED, _BLUE, _BLUE] + assert "Image at " not in result + assert result.count("recognized") == 5 + + +def test_malformed_image_reference_does_not_become_silent_native_fallback() -> None: + def missing_reference(name: str, content: bytes) -> bytes: + if name != "xl/drawings/drawing1.xml": + return content + return content.replace(b'embed="rId1"', b'embed="missing"') + + with pytest.raises(KeyError): + _convert( + _ImageConverter(Mock(return_value="

recognized

")), + _rewrite(_workbook(), missing_reference), + ) + + +def test_embedded_drawing_entities_are_rejected() -> None: + def entity(name: str, content: bytes) -> bytes: + if name != "xl/drawings/drawing1.xml": + return content + return b']>' + content.replace( + b'name="Image 1"', b'name="&label;"' + ) + + render = Mock(return_value="

recognized

") + with pytest.raises(EntitiesForbidden): + _convert(_ImageConverter(render), _rewrite(_workbook(), entity)) + render.assert_not_called() + + +def test_missing_dependencies_and_legacy_xls_stay_separate( + monkeypatch: pytest.MonkeyPatch, +) -> None: + data = (Path(__file__).parent / "test_files" / "test.xls").read_bytes() + legacy = XlsConverter() + expected = legacy.convert(io.BytesIO(data), StreamInfo(extension=".xls")).markdown + dependency = ImportError("openpyxl unavailable") + monkeypatch.setattr( + _xlsx_converter, "_xlsx_dependency_exc_info", (ImportError, dependency, None) + ) + with pytest.raises(MissingDependencyException, match=r"\[xlsx\]"): + XlsxConverter().convert(io.BytesIO(), _INFO) + assert not hasattr(legacy, "_image_to_html") + assert legacy.accepts(io.BytesIO(), StreamInfo(extension=".XLS")) + assert ( + legacy.convert(io.BytesIO(data), StreamInfo(extension=".xls")).markdown + == expected + ) From a102641f049061314c5b314cf4444cbeab5b4236 Mon Sep 17 00:00:00 2001 From: afourney Date: Wed, 16 Sep 2026 07:50:15 -0700 Subject: [PATCH 4/6] Fix conditional check for llm_description Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../markitdown/src/markitdown/converters/_pptx_converter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/markitdown/src/markitdown/converters/_pptx_converter.py b/packages/markitdown/src/markitdown/converters/_pptx_converter.py index 35ed21d658..76cf7751bc 100644 --- a/packages/markitdown/src/markitdown/converters/_pptx_converter.py +++ b/packages/markitdown/src/markitdown/converters/_pptx_converter.py @@ -195,7 +195,7 @@ def _convert_picture_to_markdown(self, shape, **kwargs): pass if ( - not llm_description + (not llm_description or not llm_description.strip()) and image_blob is not None and type(self)._image_to_html is not PptxConverter._image_to_html ): From cf461a2392592ffa72069a7c653f51117bd0b1f1 Mon Sep 17 00:00:00 2001 From: afourney Date: Wed, 16 Sep 2026 07:54:34 -0700 Subject: [PATCH 5/6] Update relationships function calls for drawings and images Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../markitdown/src/markitdown/converter_utils/_xlsx_images.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/markitdown/src/markitdown/converter_utils/_xlsx_images.py b/packages/markitdown/src/markitdown/converter_utils/_xlsx_images.py index 7eb0bb2776..bc26378d9b 100644 --- a/packages/markitdown/src/markitdown/converter_utils/_xlsx_images.py +++ b/packages/markitdown/src/markitdown/converter_utils/_xlsx_images.py @@ -67,11 +67,11 @@ def __init__(self, file_stream: BinaryIO): ) if not drawings: continue - drawing_parts = _relationships(archive, sheet_part) + drawing_parts = _relationships(archive, sheet_part, "drawing") images = self._sheets.setdefault(sheet.attrib["name"], []) for drawing in drawings: drawing_part = drawing_parts[drawing.attrib[_REL_ID]] - image_parts = _relationships(archive, drawing_part) + image_parts = _relationships(archive, drawing_part, "image") drawing_root = ET.fromstring(archive.read(drawing_part)) # Match openpyxl's image traversal, not its XML serialization order. anchors = [ From e4fbb5bfe0792de0857a65bd2740267494d0c4cd Mon Sep 17 00:00:00 2001 From: Adam Fourney Date: Wed, 16 Sep 2026 09:48:40 -0700 Subject: [PATCH 6/6] Forward stream_info to LLMVisionOCRService --- .../_docx_converter_with_ocr.py | 5 +- .../src/markitdown_ocr/_ocr_service.py | 17 +++ .../_pptx_converter_with_ocr.py | 5 +- .../_xlsx_converter_with_ocr.py | 4 +- .../tests/test_docx_inheritance.py | 7 +- .../markitdown-ocr/tests/test_ocr_metadata.py | 134 ++++++++++++++++++ .../tests/test_pptx_inheritance.py | 38 ++++- .../tests/test_xlsx_inheritance.py | 14 +- 8 files changed, 204 insertions(+), 20 deletions(-) create mode 100644 packages/markitdown-ocr/tests/test_ocr_metadata.py diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py b/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py index e3090808b7..6f072489f4 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py @@ -8,7 +8,7 @@ from markitdown import DocumentConverterResult, StreamInfo from markitdown.converters import DocxConverter -from ._ocr_service import LLMVisionOCRService +from ._ocr_service import LLMVisionOCRService, _extract_text_with_metadata class DocxConverterWithOCR(DocxConverter): @@ -50,8 +50,7 @@ def _image_to_html( if key in cache: return cache[key] - # Preserve compatibility with services accepting only an image stream. - result = ocr_service.extract_text(image_stream) + result = _extract_text_with_metadata(ocr_service, image_stream, stream_info) if result.error: warn( f"DOCX image OCR failed: {result.error}. Keeping the native image.", diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_ocr_service.py b/packages/markitdown-ocr/src/markitdown_ocr/_ocr_service.py index 2885e1f47c..c42ca55a65 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_ocr_service.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_ocr_service.py @@ -4,6 +4,7 @@ """ import base64 +import inspect from typing import Any, BinaryIO from dataclasses import dataclass @@ -108,3 +109,19 @@ def extract_text( return OCRResult(text="", backend_used="llm_vision", error=str(e)) finally: image_stream.seek(0) + + +def _extract_text_with_metadata( + ocr_service: LLMVisionOCRService, + image_stream: BinaryIO, + stream_info: StreamInfo, +) -> OCRResult: + """Forward metadata when supported, retaining the legacy stream-only call.""" + extract_text = ocr_service.extract_text + try: + inspect.signature(extract_text).bind(image_stream, stream_info=stream_info) + except (TypeError, ValueError): + # Unsupported or uninspectable signatures keep the legacy invocation. + return extract_text(image_stream) + # Keep service errors outside the signature check; never retry an OCR call. + return extract_text(image_stream, stream_info=stream_info) diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py b/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py index b856d79981..124b143ef7 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_pptx_converter_with_ocr.py @@ -8,7 +8,7 @@ from markitdown import DocumentConverterResult, StreamInfo from markitdown.converters import PptxConverter -from ._ocr_service import LLMVisionOCRService +from ._ocr_service import LLMVisionOCRService, _extract_text_with_metadata class PptxConverterWithOCR(PptxConverter): @@ -49,8 +49,7 @@ def _image_to_html( if key in cache: return cache[key] - # Preserve compatibility with services accepting only an image stream. - result = ocr_service.extract_text(image_stream) + result = _extract_text_with_metadata(ocr_service, image_stream, stream_info) if result.error: warn( f"PPTX image OCR failed: {result.error}. Keeping the native image.", diff --git a/packages/markitdown-ocr/src/markitdown_ocr/_xlsx_converter_with_ocr.py b/packages/markitdown-ocr/src/markitdown_ocr/_xlsx_converter_with_ocr.py index 8ae36ad1e6..b272bcf663 100644 --- a/packages/markitdown-ocr/src/markitdown_ocr/_xlsx_converter_with_ocr.py +++ b/packages/markitdown-ocr/src/markitdown_ocr/_xlsx_converter_with_ocr.py @@ -8,7 +8,7 @@ from markitdown import DocumentConverterResult, StreamInfo from markitdown.converters import XlsxConverter -from ._ocr_service import LLMVisionOCRService +from ._ocr_service import LLMVisionOCRService, _extract_text_with_metadata class XlsxConverterWithOCR(XlsxConverter): @@ -49,7 +49,7 @@ def _image_to_html( if key in cache: return cache[key] - result = ocr_service.extract_text(image_stream) + result = _extract_text_with_metadata(ocr_service, image_stream, stream_info) if result.error: warn( f"XLSX image OCR failed: {result.error}. Keeping the native image.", diff --git a/packages/markitdown-ocr/tests/test_docx_inheritance.py b/packages/markitdown-ocr/tests/test_docx_inheritance.py index 3b485b5d24..3cbbf99ba2 100644 --- a/packages/markitdown-ocr/tests/test_docx_inheritance.py +++ b/packages/markitdown-ocr/tests/test_docx_inheritance.py @@ -4,7 +4,7 @@ import inspect import io from typing import Any -from unittest.mock import Mock +from unittest.mock import Mock, create_autospec from bs4 import BeautifulSoup from docx import Document @@ -63,7 +63,8 @@ def _document( def _service(text: str = "recognized") -> Mock: # A one-argument service must remain supported, without injected keywords. - return Mock(extract_text=Mock(side_effect=lambda stream: OCRResult(text=text))) + recognize = lambda stream: OCRResult(text=text) + return Mock(extract_text=create_autospec(recognize, side_effect=recognize)) def _convert(converter: DocxConverter, data: bytes, **kwargs: Any) -> str: @@ -169,7 +170,7 @@ def recognize(image_stream): calls.append(image) return OCRResult(text="blue" if image == _BLUE else "") - service = Mock(extract_text=Mock(side_effect=recognize)) + service = Mock(extract_text=create_autospec(recognize, side_effect=recognize)) result = _convert(DocxConverterWithOCR(service), stream.getvalue()) assert calls == [_BLUE, _RED] diff --git a/packages/markitdown-ocr/tests/test_ocr_metadata.py b/packages/markitdown-ocr/tests/test_ocr_metadata.py new file mode 100644 index 0000000000..de7c6a27f2 --- /dev/null +++ b/packages/markitdown-ocr/tests/test_ocr_metadata.py @@ -0,0 +1,134 @@ +"""Office image metadata reaches compatible services without retrying OCR.""" + +import inspect +import io +from typing import Any, BinaryIO +from unittest.mock import Mock + +import pytest + +from markitdown import StreamInfo +from markitdown_ocr._docx_converter_with_ocr import DocxConverterWithOCR +from markitdown_ocr._ocr_service import OCRResult, _extract_text_with_metadata +from markitdown_ocr._pptx_converter_with_ocr import PptxConverterWithOCR +from markitdown_ocr._xlsx_converter_with_ocr import XlsxConverterWithOCR + + +_Converter = DocxConverterWithOCR | PptxConverterWithOCR | XlsxConverterWithOCR +_CONVERTERS = [DocxConverterWithOCR, PptxConverterWithOCR, XlsxConverterWithOCR] +_SVG = b'recognized' +_INFO = StreamInfo(mimetype="image/svg+xml", extension=".svg", filename="drawing.svg") + + +@pytest.mark.parametrize("converter_type", _CONVERTERS) +def test_bound_service_receives_metadata_by_keyword( + converter_type: type[_Converter], +) -> None: + seen = [] + + class Service: + def extract_text( + self, + image_stream: BinaryIO, + prompt: str | None = None, + *, + stream_info: StreamInfo, + ) -> OCRResult: + assert prompt is None + assert image_stream.tell() == 0 + seen.append((image_stream.read(), stream_info)) + return OCRResult(text="recognized") + + with io.BytesIO(_SVG) as stream: + result = converter_type()._image_to_html(stream, _INFO, ocr_service=Service()) + assert not stream.closed + + assert seen == [(_SVG, _INFO)] + assert seen[0][1] is _INFO + assert result == "

[Image OCR]
recognized
[End OCR]

" + + +@pytest.mark.parametrize("converter_type", _CONVERTERS) +def test_service_accepting_kwargs_receives_only_image_metadata( + converter_type: type[_Converter], +) -> None: + seen = [] + + class Service: + def extract_text(self, image_stream: BinaryIO, **kwargs: Any) -> OCRResult: + seen.append(kwargs) + return OCRResult(text="recognized") + + with io.BytesIO(_SVG) as stream: + converter_type()._image_to_html( + stream, + _INFO, + ocr_service=Service(), + url="https://example.test/parent.pptx", + file_extension=".pptx", + ) + + assert seen == [{"stream_info": _INFO}] + + +@pytest.mark.parametrize("converter_type", _CONVERTERS) +def test_legacy_positional_only_service_remains_supported( + converter_type: type[_Converter], +) -> None: + seen = [] + + class Service: + def extract_text(self, image_stream: BinaryIO, /) -> OCRResult: + seen.append(image_stream.read()) + return OCRResult(text="recognized") + + with io.BytesIO(_SVG) as stream: + result = converter_type()._image_to_html(stream, _INFO, ocr_service=Service()) + + assert seen == [_SVG] + assert result == "

[Image OCR]
recognized
[End OCR]

" + + +@pytest.mark.parametrize("converter_type", _CONVERTERS) +def test_internal_type_error_is_not_retried( + converter_type: type[_Converter], +) -> None: + error = TypeError("service implementation failed") + seen = [] + + class Service: + def extract_text( + self, image_stream: BinaryIO, stream_info: StreamInfo | None = None + ) -> OCRResult: + seen.append(stream_info) + raise error + + with io.BytesIO(_SVG) as stream: + with pytest.raises(TypeError) as caught: + converter_type()._image_to_html(stream, _INFO, ocr_service=Service()) + + assert caught.value is error + assert seen == [_INFO] + + +@pytest.mark.parametrize("signature_error", [TypeError, ValueError]) +def test_uninspectable_service_retains_single_argument_invocation( + signature_error: type[Exception], +) -> None: + seen = [] + + class Extractor: + @property + def __signature__(self) -> inspect.Signature: + raise signature_error("signature unavailable") + + def __call__(self, image_stream: BinaryIO) -> OCRResult: + seen.append(image_stream.read()) + return OCRResult(text="recognized") + + service = Mock(extract_text=Extractor()) + with io.BytesIO(_SVG) as stream: + result = _extract_text_with_metadata(service, stream, _INFO) + + assert result.text == "recognized" + assert seen == [_SVG] diff --git a/packages/markitdown-ocr/tests/test_pptx_inheritance.py b/packages/markitdown-ocr/tests/test_pptx_inheritance.py index 8a9923f13a..6d1dbd6d51 100644 --- a/packages/markitdown-ocr/tests/test_pptx_inheritance.py +++ b/packages/markitdown-ocr/tests/test_pptx_inheritance.py @@ -5,7 +5,8 @@ import io from pathlib import Path from typing import Any -from unittest.mock import Mock +from unittest.mock import Mock, create_autospec +import zipfile from PIL import Image from pptx import Presentation @@ -17,7 +18,7 @@ from markitdown.converters import _pptx_converter import markitdown._markitdown as markitdown_module from markitdown_ocr import _plugin -from markitdown_ocr._ocr_service import OCRResult +from markitdown_ocr._ocr_service import LLMVisionOCRService, OCRResult from markitdown_ocr._pptx_converter_with_ocr import PptxConverterWithOCR @@ -53,7 +54,8 @@ def _presentation(images: tuple[bytes, ...] = (_RED,)) -> bytes: def _service(text: str = "recognized") -> Mock: - return Mock(extract_text=Mock(side_effect=lambda stream: OCRResult(text=text))) + recognize = lambda stream: OCRResult(text=text) + return Mock(extract_text=create_autospec(recognize, side_effect=recognize)) def _convert(converter: PptxConverter, data: bytes, **kwargs: Any) -> str: @@ -120,7 +122,7 @@ def recognize(stream): calls.append(image) return OCRResult(text="blue" if image == _BLUE else "") - service = Mock(extract_text=Mock(side_effect=recognize)) + service = Mock(extract_text=create_autospec(recognize, side_effect=recognize)) result = _convert( PptxConverterWithOCR(service), _presentation((_BLUE, _RED, _BLUE)) ) @@ -205,7 +207,10 @@ def recognize(stream): data = (_CORE_FILES / "test_svg_no_fallback.pptx").read_bytes() result = _convert( - PptxConverterWithOCR(Mock(extract_text=Mock(side_effect=recognize))), data + PptxConverterWithOCR( + Mock(extract_text=create_autospec(recognize, side_effect=recognize)) + ), + data, ) assert len(seen) == 1 and b" None: + data = (_CORE_FILES / "test_svg_no_fallback.pptx").read_bytes() + with zipfile.ZipFile(io.BytesIO(data)) as archive: + svg_parts = [name for name in archive.namelist() if name.endswith(".svg")] + assert len(svg_parts) == 1 + svg = archive.read(svg_parts[0]) + client = Mock() + client.chat.completions.create.return_value.choices = [ + Mock(message=Mock(content="SVG text")) + ] + service = LLMVisionOCRService(client, "vision-model") + + result = _convert(PptxConverterWithOCR(service), data) + + assert "*[Image OCR] \nSVG text \n[End OCR]*" in result + client.chat.completions.create.assert_called_once() + request = client.chat.completions.create.call_args.kwargs + assert request["model"] == "vision-model" + assert request["messages"][0]["content"][1]["image_url"]["url"] == ( + "data:image/svg+xml;base64," + base64.b64encode(svg).decode("ascii") + ) + + @pytest.mark.parametrize("caption_succeeds", [False, True]) def test_plugin_full_trip_with_mocked_model_only( monkeypatch: pytest.MonkeyPatch, caption_succeeds: bool diff --git a/packages/markitdown-ocr/tests/test_xlsx_inheritance.py b/packages/markitdown-ocr/tests/test_xlsx_inheritance.py index f0e010a706..98a104b84d 100644 --- a/packages/markitdown-ocr/tests/test_xlsx_inheritance.py +++ b/packages/markitdown-ocr/tests/test_xlsx_inheritance.py @@ -4,7 +4,7 @@ import inspect import io from typing import Any -from unittest.mock import Mock +from unittest.mock import Mock, create_autospec import zipfile import openpyxl @@ -57,7 +57,8 @@ def _workbook(images: tuple[bytes, ...] = (_RED,)) -> bytes: def _service(text: str = "recognized") -> Mock: - return Mock(extract_text=Mock(side_effect=lambda stream: OCRResult(text=text))) + recognize = lambda stream: OCRResult(text=text) + return Mock(extract_text=create_autospec(recognize, side_effect=recognize)) def _convert(converter: XlsxConverter, data: bytes, **kwargs: Any) -> str: @@ -168,7 +169,9 @@ def recognize(stream): calls.append(data) return OCRResult(text="blue" if data == _BLUE else "") - converter = XlsxConverterWithOCR(Mock(extract_text=Mock(side_effect=recognize))) + converter = XlsxConverterWithOCR( + Mock(extract_text=create_autospec(recognize, side_effect=recognize)) + ) result = _convert(converter, _workbook((_RED, _BLUE, _RED, _BLUE))) assert calls == [_RED, _BLUE] @@ -218,7 +221,10 @@ def recognize(stream): return OCRResult(text=label) result = _convert( - XlsxConverterWithOCR(Mock(extract_text=Mock(side_effect=recognize))), data + XlsxConverterWithOCR( + Mock(extract_text=create_autospec(recognize, side_effect=recognize)) + ), + data, ) assert calls == legacy_order blocks = [f"*[Image OCR] \n{label} \n[End OCR]*" for label in legacy_order]