Refactor Office OCR converters to reuse core conversion pipelines - #2506
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The OCR package dependency permits older core versions that lack the newly required hook.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a reusable DOCX image-to-HTML hook and refactors DOCX OCR to use the core conversion pipeline.
Changes:
- Introduces image fragment replacement with native fallback.
- Simplifies DOCX OCR through inheritance and per-document caching.
- Adds comprehensive core and OCR integration tests.
File summaries
| File | Description |
|---|---|
packages/markitdown/src/markitdown/converters/_docx_converter.py |
Adds the image-rendering hook. |
packages/markitdown/src/markitdown/converter_utils/docx/_images.py |
Bridges image callbacks into generated HTML. |
packages/markitdown/tests/test_docx_images.py |
Tests hook behavior and HTML placement. |
packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py |
Refactors OCR onto the core converter. |
packages/markitdown-ocr/tests/test_docx_inheritance.py |
Tests inherited OCR conversion behavior. |
packages/markitdown-ocr/tests/test_docx_converter.py |
Updates OCR output expectations. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…, overridable by plugins.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
XLSX relationship validation can expose non-image package parts to OCR, and whitespace-only PPTX captions incorrectly suppress OCR fallback.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Office OCR calls discard image MIME metadata, causing formats such as SVG to be submitted with an incorrect media type.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/markitdown-ocr/src/markitdown_ocr/_docx_converter_with_ocr.py:54
- Discarding the image
StreamInfoforces the bundled OCR service to infer the MIME type from Pillow. DOCX can contain formats such as SVG that Pillow does not identify, causing valid SVG bytes to be submitted asdata:image/png. Please pass metadata to services that support it while preserving the documented one-argument custom-service compatibility.
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Balanced
afourney
changed the title
Lukas (L4XB)
added a commit
to L4XB/markitdown
that referenced
this pull request
Sep 17, 2026
microsoft#2506 turned `_read_xlsx_sheets` into a context manager and made `XlsxConverterWithOCR` a subclass of `XlsxConverter`. The fix now sits on that structure: both `read_excel` calls in `_read_xlsx_sheets` pass `dtype=object`, and the sheet loop in `XlsxConverter.convert` renders through `sheet_to_html`. `_xlsx_converter_with_ocr.py` is taken from main as is. It no longer reads or renders a sheet itself, so the fix reaches it through the parent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace the OCR plugin’s duplicated DOCX, PPTX, and XLSX conversion pipelines with thin subclasses of the core converters. Native parsing and formatting fixes now benefit OCR-enabled conversion automatically, rather than requiring parallel fixes in the plugin.
Each core Office converter exposes the same semi-private
_image_to_html(image_stream, stream_info, **kwargs)hook. Core handles image extraction and placement; the plugin supplies OCR text as escaped HTML for the shared HTML-to-Markdown renderer.image/svg+xml, while legacy one-argument services remain supported.Compatibility and output
Public constructor and conversion signatures are unchanged. Hooks that return
Noneor blank text retain native rendering.OCR markers are preserved, but recognized text now follows shared Markdown escaping and line-break handling. PPTX also inherits core’s real newlines and empty-title/notes handling; XLSX heading spacing follows core.
Bump core to
0.1.8b3and requiremarkitdown>=0.1.8b3in the OCR package so dependency resolution enforces hook availability.PDF and legacy
.xlsconversion are unchanged. SVG rasterization and broader HTML-table formatting improvements remain outside this PR.