Repository navigation
fix(indexer): honor preset parsing_strategy, lazy pools, pymupdf markdown, parse timeout #582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
b9881a1
8fdf4a6
7fcdc30
9eee08c
a393336
0c644ae
8d5fdf0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,6 @@ | |
| import pymupdf4llm | ||
|
|
||
| from ....models.document import Document, DocumentType, ImageBlock, ProcessedDocument, TextBlock | ||
| from ...image_preprocessor import extract_data_uri_image_blocks | ||
| from ..document_parser import DocumentParser | ||
| from ..registry import parser_registry | ||
|
|
||
|
|
@@ -48,29 +47,25 @@ def _extract_text(raw: bytes) -> tuple[list[str], list[ImageBlock]]: | |
|
|
||
|
|
||
| def _extract_markdown(raw: bytes) -> tuple[list[str], list[ImageBlock]]: | ||
| """Return Markdown per page + ``ImageBlock``s built from embedded data URIs. | ||
|
|
||
| ``embed_images=True`` makes ``pymupdf4llm`` write images as base64 | ||
| data URIs in-line. We decode each ref into an ``ImageBlock`` and | ||
| leave the ref in the page text untouched so the caption stage can | ||
| substitute later via ``ImageBlock.metadata['markdown_ref']``. | ||
| """Return structured Markdown per page (no images). | ||
|
|
||
| pymupdf is the lightweight, no-VLM backend. ``pymupdf4llm`` preserves | ||
| document structure (headings, lists, tables) — which the markdown-aware | ||
| chunker needs to cut on real boundaries instead of mid-sentence — while | ||
| ``embed_images=False`` keeps base64 image data out of the text. That keeps | ||
| chunks small (no Milvus gRPC overflow) and skips image rendering entirely | ||
| (fast). Image-aware parsing is marker/docling's job, so no ``ImageBlock``s | ||
| are produced here. | ||
| """ | ||
| with pymupdf.open(stream=raw, filetype="pdf") as doc: | ||
| chunks = pymupdf4llm.to_markdown( | ||
| doc, | ||
| page_chunks=True, | ||
| embed_images=True, | ||
| embed_images=False, | ||
| write_images=False, | ||
| dpi=300, | ||
| ) | ||
| pages: list[str] = [] | ||
| images: list[ImageBlock] = [] | ||
| for i, chunk in enumerate(chunks, start=1): | ||
| text = (chunk.get("text") or "").strip() | ||
| pages.append(text) | ||
| if text: | ||
| images.extend(extract_data_uri_image_blocks(text, page_number=i)) | ||
| return pages, images | ||
| pages = [(chunk.get("text") or "").strip() for chunk in chunks] | ||
| return pages, [] | ||
|
Comment on lines
+67
to
+68
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue with this implementation is that we're dropping embedded images, even though this parser is capable of handling them. Do we really want to do that?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since pymupdf image cropping quality has not been validated and compared to other advanced parsers like (docling & marker), we've decided to remove the embedded images until that validation. |
||
|
|
||
|
|
||
| @parser_registry.register("pymupdf") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| """The pooled PDF loaders must create their Ray pool lazily via | ||
| ``get_or_create_actor`` rather than assuming bootstrap already created it. | ||
|
|
||
| Bootstrap only pre-warms the *globally* configured PDF backend, but a per-preset | ||
| ``parsing_strategy`` can select any backend at runtime (#569). A get-only | ||
| ``ray.get_actor`` then fails with "Failed to look up actor 'DoclingPool'" | ||
| (#575); lazy creation makes whichever backend a preset picks work on first use. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import pytest | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("module_name", "loader_name", "pool_name", "pool_attr"), | ||
| [ | ||
| ("services.workers.parsers.docling_workers", "DoclingLoader", "DoclingPool", "DoclingPool"), | ||
| ("services.workers.parsers.marker_workers", "MarkerLoader", "MarkerPool", "MarkerPool"), | ||
| ], | ||
| ) | ||
| def test_pool_loader_lazily_creates_its_pool(monkeypatch, module_name, loader_name, pool_name, pool_attr): | ||
| import importlib | ||
|
|
||
| import services.workers.bootstrap as bootstrap | ||
|
|
||
| module = importlib.import_module(module_name) | ||
| Loader = getattr(module, loader_name) | ||
| PoolCls = getattr(module, pool_attr) | ||
|
|
||
| calls: list[tuple] = [] | ||
|
|
||
| def fake_get_or_create_actor(name, cls, **options): | ||
| calls.append((name, cls, options)) | ||
| return "pool-handle" | ||
|
|
||
| monkeypatch.setattr(bootstrap, "get_or_create_actor", fake_get_or_create_actor) | ||
| # A get-only ray.get_actor must NOT be used anymore — fail loudly if it is. | ||
| monkeypatch.setattr(module.ray, "get_actor", lambda *a, **k: pytest.fail("loader used get-only ray.get_actor")) | ||
|
|
||
| loader = Loader() | ||
|
|
||
| assert loader.worker == "pool-handle" | ||
| assert calls == [(pool_name, PoolCls, {"lifetime": "detached"})] |
Uh oh!
There was an error while loading. Please reload this page.