Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ def build_graph(
detect_kwargs["inference_batch_size"] = int(extract_params.inference_batch_size)

ocr_kwargs: dict[str, Any] = {}
if extract_params.method in ("pdfium_hybrid", "ocr") and extract_params.extract_text:
if extract_params.extract_text:
ocr_kwargs["extract_text"] = True
Comment thread
greptile-apps[bot] marked this conversation as resolved.
if extract_params.extract_tables:
ocr_kwargs["extract_tables"] = True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def _parse_mode_enabled(extract_params: ExtractParams) -> bool:


def _ocr_stage_needed(extract_params: ExtractParams) -> bool:
if extract_params.method in ("pdfium_hybrid", "ocr") and extract_params.extract_text:
if extract_params.extract_text:
return True
if extract_params.extract_tables:
# OCR is always needed for table crops: either to produce pseudo-markdown
Expand Down Expand Up @@ -481,7 +481,7 @@ def _run_detection_pipeline(self, batch_df: pd.DataFrame) -> pd.DataFrame:
}
if ocr_lang is not None:
ocr_kwargs["ocr_lang"] = ocr_lang
if extract_params.method in ("pdfium_hybrid", "ocr") and extract_params.extract_text:
if extract_params.extract_text:
ocr_kwargs["extract_text"] = True
if extract_params.extract_tables:
ocr_kwargs["extract_tables"] = True
Expand Down
120 changes: 120 additions & 0 deletions nemo_retriever/tests/test_ocr_version_selection.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,123 @@ def _fake_resolve(operator_class, resources, operator_kwargs=None):
table_kwargs = next(kwargs for name, kwargs in captured_kwargs if name == "TableStructureActor")
assert table_kwargs.get("ocr_version") == "v2"
assert table_kwargs.get("ocr_lang") == "english"


# ---------------------------------------------------------------------------
# Regression tests for Issue #2443 — default pipeline skips OCR extract_text
# ---------------------------------------------------------------------------


def test_default_pdfium_method_includes_ocr_when_extract_text_true() -> None:
"""Default method='pdfium' must still include OCRActor when extract_text=True.

Regression test for https://github.com/NVIDIA/NeMo-Retriever/issues/2443.
Previously the OCR stage was only appended when method was 'pdfium_hybrid'
or 'ocr', causing the default pipeline to skip text extraction for scanned
images and PDFs.
"""
graph = build_graph(
extract_params=ExtractParams(
method="pdfium",
extract_text=True,
extract_tables=False,
extract_charts=False,
extract_infographics=False,
),
embed_params=EmbedParams(
model_name="nvidia/llama-nemotron-embed-1b-v2",
embed_invoke_url="http://embed.example/v1",
),
)

nodes = _linear_nodes(graph)
classes = [node.operator_class for node in nodes]
assert OCRActor in classes
ocr_node = next(node for node in nodes if node.operator_class is OCRActor)
assert ocr_node.operator_kwargs.get("extract_text") is True


def test_default_pdfium_method_excludes_ocr_when_extract_text_false() -> None:
"""method='pdfium' with extract_text=False must not include OCRActor."""
graph = build_graph(
extract_params=ExtractParams(
method="pdfium",
extract_text=False,
extract_tables=False,
extract_charts=False,
extract_infographics=False,
),
embed_params=EmbedParams(
model_name="nvidia/llama-nemotron-embed-1b-v2",
embed_invoke_url="http://embed.example/v1",
),
)

nodes = _linear_nodes(graph)
classes = [node.operator_class for node in nodes]
assert OCRActor not in classes


def test_ocr_stage_needed_true_for_default_method_extract_text() -> None:
"""_ocr_stage_needed must return True when extract_text=True regardless of method."""
from nemo_retriever.operators.graph_ops.multi_type_extract_operator import _ocr_stage_needed

params = ExtractParams(method="pdfium", extract_text=True)
assert _ocr_stage_needed(params) is True


def test_ocr_stage_needed_false_when_extract_text_disabled() -> None:
"""_ocr_stage_needed must return False when extract_text=False and no other OCR flags."""
from nemo_retriever.operators.graph_ops.multi_type_extract_operator import _ocr_stage_needed

params = ExtractParams(
method="pdfium",
extract_text=False,
extract_tables=False,
extract_charts=False,
extract_infographics=False,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 New test lacks annotations

The new test_detection_pipeline_includes_ocr_for_default_method function leaves monkeypatch untyped and omits its -> None return annotation, weakening static checking and violating the repository's public-function annotation standard.

Rule Used: All public functions, methods, and class attribute... (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: nemo_retriever/tests/test_ocr_version_selection.py
Line: 410

Comment:
**New test lacks annotations**

The new `test_detection_pipeline_includes_ocr_for_default_method` function leaves `monkeypatch` untyped and omits its `-> None` return annotation, weakening static checking and violating the repository's public-function annotation standard.

**Rule Used:** All public functions, methods, and class attribute... ([source](.greptile))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

)
assert _ocr_stage_needed(params) is False


def test_detection_pipeline_includes_ocr_for_default_method(monkeypatch):
"""In-process detection pipeline must include OCR when method='pdfium' and extract_text=True."""
from nemo_retriever.operators.graph_ops.multi_type_extract_operator import MultiTypeExtractCPUActor
from nemo_retriever.common.ray_resource_hueristics import Resources
import pandas as pd

calls = []

class _IdentityStage:
def __init__(self, **kwargs):
self.kwargs = kwargs

def run(self, data):
return data

def _fake_resolve(operator_class, resources, operator_kwargs=None):
calls.append(operator_class.__name__)
return _IdentityStage

monkeypatch.setattr(
"nemo_retriever.operators.graph_ops.multi_type_extract_operator.resolve_operator_class",
_fake_resolve,
)
monkeypatch.setattr(
"nemo_retriever.common.ray_resource_hueristics.gather_local_resources",
lambda: Resources(cpu_count=8, gpu_count=1),
)

op = MultiTypeExtractCPUActor(
extraction_mode="image",
extract_params=ExtractParams(
method="pdfium",
extract_text=True,
extract_tables=False,
extract_charts=False,
extract_infographics=False,
),
)

op._run_detection_pipeline(pd.DataFrame({"page_image": ["x"]}))
assert "OCRActor" in calls