Fix: Not raising exception when the get_captions method fails

[Ahmath-Gadji]

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling during PDF processing to gracefully manage captioning errors through logging instead of failures
  • Enhanced result collection and mapping for PDF content extraction operations

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The change modifies error handling in the PDF marker component. It now constructs a result dictionary from keys and results, returns this mapping on successful captioning, and logs captioning errors while returning an empty dictionary instead of re-raising exceptions. Asyncio cancellation errors are still re-raised as before.

Changes

Cohort / File(s)	Summary
PDF Marker Error Handling openrag/components/indexer/loaders/pdf_loaders/marker.py	Modified exception handling for captioning operations: logs exceptions with error messages and returns empty dict instead of re-raising; early constructs result dictionary via zipping keys and results; preserves asyncio.CancelledError re-raise behavior

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A marker hops through PDFs with grace,
No longer crashing when errors embrace,
Now logging whispers instead of loud screams,
Empty dicts returned—resilient dreams!
The rabbit rejoices, control flows so clean,
The finest PDF handler e'er seen! 📄✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: preventing exception raising when get_captions method fails, which aligns with the code modifications that change error handling from re-raising to returning an empty dict.

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

openrag/components/indexer/loaders/pdf_loaders/marker.py (1)

262-266: Consider using strict=True in zip() and removing the assertion.

The project requires Python 3.12+ (per pyproject.toml), which fully supports the strict parameter. The assertion at line 264 could raise AssertionError if lengths mismatch, but this would be caught by the exception handler below (line 272-274) and silently return an empty dict. Using zip(keys, results, strict=True) is more Pythonic and raises ValueError instead, which is semantically more appropriate for a runtime data mismatch.

Proposed refactor

         try:
             results = await tqdm.gather(*tasks, desc="Captioning images")
-            assert len(keys) == len(results), "Mismatch between keys and results count"
-            result_dict = dict(zip(keys, results))
+            result_dict = dict(zip(keys, results, strict=True))
             return result_dict

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 397bb7b and ee56c98.

📒 Files selected for processing (1)

• openrag/components/indexer/loaders/pdf_loaders/marker.py

🧰 Additional context used

🪛 Ruff (0.14.10)

openrag/components/indexer/loaders/pdf_loaders/marker.py

265-265: zip() without an explicit strict= parameter

Add explicit value for parameter strict=

(B905)

---

266-266: Consider moving this statement to an else block

(TRY300)

🔇 Additional comments (1)

openrag/components/indexer/loaders/pdf_loaders/marker.py (1)

273-275: Approve the fault-tolerant error handling.

The exception handler correctly logs failures and returns an empty dict, allowing PDF processing to continue when image captioning fails. The caller at lines 225-228 handles this gracefully by simply skipping the caption replacement loop when the dict is empty.

The design is intentional and appropriate for a non-critical feature. Note that asyncio.CancelledError is explicitly re-raised on line 267, showing deliberate distinction between cancellation (which should propagate) and other errors (which are absorbed).