feat: add ability to extract images for OpenDataLoader PDF integration to better support multimodal pipelines

[maxdswain]

Related Issues

• fixes Add image extraction support to OpenDataLoader PDF integration #3921

Proposed Changes:

Add an optional extract_images parameter that extracts images and returns them as Haystack Documents.

How did you test it?

Added unit and integration tests.

Notes for the reviewer

I modelled my changes off of existing Jina/Cohere integrations that support image-based documents using the Document.meta["file_path"]. This allows this component to be readily used in multimodal RAG pipelines with these components.

Checklist

• I have read the contributors guidelines and the code of conduct
• I have updated the related issue with new insights and changes
• I added unit tests and updated the docstrings
• I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test:.

[github-actions]

Hi @maxdswain, thanks for your interest in contributing to Haystack! 🙏

⚠️ You currently have 3 open pull requests in this repository (#3870, #3839 and this one). Our review capacity is limited, so please hold off opening more PRs until we've had a chance to review your first 2 open PRs. This helps us give each contribution the attention it deserves. Thank you!

This is an automated message to help us keep the review queue healthy.

[github-actions]

Coverage report (opendataloader_pdf)

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
integrations/opendataloader_pdf/src/haystack_integrations/components/converters/opendataloader_pdf
converter.py					188
Project Total

This report was generated by python-coverage-comment-action

[davidsbatista]

OpenDataLoader derives extracted image filenames from the staged PDF's stem (confirmed by this PR's own test fixture: f"{pdf_path.stem}_image_1.png").

Since the stem is always document_0, document_1, … regardless of which call produced it, two separate run() calls on the same converter with the same persistent image_output_dir write to the same image filename, and the second call silently overwrites the first.

converter = OpenDataLoaderConverter(extract_images=True, image_output_dir="images")

 result_a = converter.run(sources=["report_a.pdf"])
# writes images/document_0_image_1.png
# result_a["image_documents"][0].meta["file_path"] == "images/document_0_image_1.png"

result_b = converter.run(sources=["report_b.pdf"])
# writes images/document_0_image_1.png again -> overwrites report_a's image on disk
# result_a's already-returned Document now points at report_b's image content

This isn't an edge case — looping run() over a document collection into one shared, persistent image_output_dir is exactly the use case the docstring describes ("Persistent directory for extracted image files").

Suggestion: make the staged filename unique per call, e.g. a call-scoped uuid or a counter that doesn't reset:

import uuid

for index, (source, user_meta) in enumerate(zip(sources, metadata, strict=True)):
    staged_path = input_dir / f"document_{uuid.uuid4().hex}_{index}.pdf"

[davidsbatista]

Even with unique filenames, the before/after directory-diffing in run() (converter.py:233-235, 259-264) isn't isolated per call:

if self.image_output_dir is not None:
    self.image_output_dir.mkdir(parents=True, exist_ok=True)
    image_files_before = self._file_state(self.image_output_dir)
...
if self.image_output_dir is not None:
    image_files_after = self._file_state(self.image_output_dir)
    image_documents = [
        Document(meta={"file_path": str(path)})
        for path, state in image_files_after.items()
        if image_files_before.get(path) != state
    ]

If the same converter (or two converters pointed at the same directory) is invoked concurrently — e.g. a shared pipeline instance serving concurrent requests both calls snapshot "before" around the same time, both write, and both "after" diffs can pick up files written by the other call.

Suggested fix: don't rely on filesystem diffing at all.

Write each call's images into a call-scoped subdirectory (e.g. image_output_dir/<uuid>/) and just list that subdirectory's contents afterward:

import uuid

call_dir = self.image_output_dir / uuid.uuid4().hex
call_dir.mkdir(parents=True)
conversion_kwargs.update(image_output="external", image_dir=str(call_dir))
...
image_documents = [Document(meta={"file_path": str(p)}) for p in call_dir.rglob("*") if p.is_file()]

This removes the race entirely (no shared "before" state to corrupt) and also fixes the filename-collision issue, since filenames are now scoped per call by construction.

[davidsbatista]

Efficiency: full directory scan on every run() call

_file_state (converter.py:191) does a recursive rglob("*") + stat() over the entire image_output_dir before and after every run() call:

@staticmethod
def _file_state(directory: Path) -> dict[Path, tuple[int, int, int]]:
    state: dict[Path, tuple[int, int, int]] = {}
    for path in directory.rglob("*"):
        if path.is_file():
            stat = path.stat()
            state[path] = (stat.st_size, stat.st_mtime_ns, stat.st_ctime_ns)
    return state

Since image_output_dir is designed to accumulate images persistently across many calls, this scan's cost grows with the total number of images ever written to it, not with the size of the current batch — so processing a large document collection incrementally degrades to roughly O(n²) scanning overall.

Suggested fix: same as the concurrency issue above, a call-scoped subdirectory means each call only ever lists its own (small, freshly-created) directory, making the cost O(images in this batch) instead of O(images ever written).

[davidsbatista]

Integration tests missing

The PR description says "Added unit and integration tests," buttest_real_pdf_conversion wasn't extended to cover image extraction.

Worth adding a real-PDF integration test to confirm the image_output/image_dir kwargs actually work against the real library:

  @pytest.mark.integration
  def test_real_pdf_conversion_extracts_images(tmp_path):
      pdf_file = Path(__file__).parent / "test_files" / "pdf_with_image.pdf"
      image_output_dir = tmp_path / "images"
      converter = OpenDataLoaderConverter(extract_images=True, image_output_dir=image_output_dir)

      result = converter.run(sources=[pdf_file])

      assert result["image_documents"]
      assert Path(result["image_documents"][0].meta["file_path"]).exists()

[maxdswain]

OpenDataLoader derives extracted image filenames from the staged PDF's stem (confirmed by this PR's own test fixture: f"{pdf_path.stem}_image_1.png").

Since the stem is always document_0, document_1, … regardless of which call produced it, two separate run() calls on the same converter with the same persistent image_output_dir write to the same image filename, and the second call silently overwrites the first.

converter = OpenDataLoaderConverter(extract_images=True, image_output_dir="images")

 result_a = converter.run(sources=["report_a.pdf"])
# writes images/document_0_image_1.png
# result_a["image_documents"][0].meta["file_path"] == "images/document_0_image_1.png"

result_b = converter.run(sources=["report_b.pdf"])
# writes images/document_0_image_1.png again -> overwrites report_a's image on disk
# result_a's already-returned Document now points at report_b's image content

This isn't an edge case — looping run() over a document collection into one shared, persistent image_output_dir is exactly the use case the docstring describes ("Persistent directory for extracted image files").

Suggestion: make the staged filename unique per call, e.g. a call-scoped uuid or a counter that doesn't reset:

import uuid

for index, (source, user_meta) in enumerate(zip(sources, metadata, strict=True)):
    staged_path = input_dir / f"document_{uuid.uuid4().hex}_{index}.pdf"

This issue is also fixed by the suggested changes below to incorporate a uuid into the path as directory name, so I haven't implemented this exact fix to add uuid's per file.

[maxdswain]

I've added an integration tests that uses the PDF from the Azure Form Recognizer integration and should have addressed all of your comments now @davidsbatista.

[davidsbatista]

We need to mention the per-call subdirectory layout now on the docstring

[davidsbatista]

looks good, just one final adjustment of the docstring

[maxdswain]

@davidsbatista Docstring should now be updated.