Skip to content

feat(retrieval): add GLMOCRLoader and GLMOCRPDFLoader with structured OCR output via OCRResult#1224

Open
contactvishwav wants to merge 10 commits into
RailtownAI:mainfrom
contactvishwav:fix-issue-1175
Open

feat(retrieval): add GLMOCRLoader and GLMOCRPDFLoader with structured OCR output via OCRResult#1224
contactvishwav wants to merge 10 commits into
RailtownAI:mainfrom
contactvishwav:fix-issue-1175

Conversation

@contactvishwav

Copy link
Copy Markdown

What does this PR do?

Adds GLMOCRLoader and GLMOCRPDFLoader to the retrieval loaders pipeline, enabling structured OCR extraction via Zhipu's GLM-OCR multimodal model. GLMOCRLoader processes image files (.bmp, .jpeg, .jpg, .png, .tif, .tiff, .webp) and GLMOCRPDFLoader sends raw PDF bytes directly to the API — bypassing rasterization entirely — to preserve layout-aware output in a single round-trip. Both loaders support two deployment modes: cloud (delegates to the glmocr SDK, blocking call offloaded via asyncio.to_thread) and local (POSTs to a self-hosted vLLM/Ollama endpoint using httpx.AsyncClient). A new OCRResult dataclass (models.py) carries the structured response — markdown: str, bboxes: list[dict], tables: list[dict] — and is exported from railtracks.retrieval. BaseOCRLoader gains an optional _ocr_image_structured() method that returns OCRResult, leaving the existing _ocr_image() -> str abstract contract intact for all current subclasses. The glmocr SDK and httpx are declared as a new [glm] optional extra in pyproject.toml; pillow is co-listed since GLMOCRLoader._stream_image() uses PIL for image encoding.

Why was this PR needed?

BaseOCRLoader._ocr_image() returns a flat str, discarding everything the OCR engine knows about document structure. GLM-OCR's 0.9B multimodal model produces markdown-formatted text with heading hierarchy, bounding-box annotations per text region, and extracted table data — none of which survived the str return type. Retaining that structure in OCRResult means downstream chunkers can make layout-aware decisions (split at headings, treat table cells as atomic units) rather than operating on a stripped plaintext blob. This is the core ask of issue #1175: surface structured OCR output as a first-class type in the retrieval pipeline.

What are the relevant issue numbers?

Closes #1175

Design decisions made

1. Extended BaseOCRLoader with _ocr_image_structured() rather than a parallel loader hierarchy.

The maintainer's stated preference (option ii in #1175) was to add _ocr_image_structured(image) -> OCRResult as a non-abstract method on BaseOCRLoader with a default implementation that delegates to _ocr_image() and wraps the result in OCRResult(markdown=text). This means PyPDFOCRLoader and any other existing subclass that only implements _ocr_image() continues to work without modification. GLMOCRLoader inverts the delegation: _ocr_image_structured() is the real implementation (GLM-OCR natively returns structured output), and _ocr_image() satisfies the abstract contract by calling result.to_text(). No breaking changes to the existing str return contract.

2. Followed PyPDFOCRLoader patterns exactly.

glm_ocr_loader.py mirrors pdf_ocr_loader.py in every structural respect: a module-level try/except ImportError guard that re-raises with a pip install "railtracks[glm]" hint; asyncio.to_thread for all blocking I/O (image encoding, SDK calls, path.read_bytes); eager parameter validation in __init__ (invalid mode, missing endpoint for local mode, invalid breakdown_strategy) so errors surface at construction time rather than mid-stream; a BreakdownStrategy = Literal["page", "document"] alias at module level; and a _stream_dir() helper extracted from astream() to keep McCabe complexity under the project's limit of 10. GLMOCRLoader and GLMOCRPDFLoader are not re-exported from loaders/__init__.py, matching PyPDFOCRLoader's precedent — users import directly from railtracks.retrieval.loaders.glm_ocr_loader.

3. [glm] is a standalone extra, not folded into [retrieval].

glmocr is a cloud SDK requiring API credentials, unlike the local tooling in [ocr] (Tesseract, pypdfium2). Adding it to [retrieval] would force the SDK on all users who install the comprehensive retrieval stack regardless of whether they use GLM-OCR. It is available as pip install "railtracks[glm]" and can be promoted to [retrieval] later if it becomes a default.

Known limitations / TODOs

GLM-OCR's JSON response schema is not yet fully published in the official SDK documentation. Five methods contain stub mappings with # TODO comments that must be updated once the live schema is confirmed:

  • _parse_glmocr_response() — assumes raw["markdown"], raw.get("bboxes", []), raw.get("tables", []). Actual field names may differ.
  • _call_cloud() — assumes glmocr.ocr(image_bytes: bytes) -> dict. Positional argument and return type need verification.
  • _call_local() — assumes a JSON body of {"image": <base64-PNG>, "format": "markdown"}. Endpoint contract is a best-guess based on common vLLM patterns.
  • _call_cloud_pdf() — assumes glmocr.ocr(pdf_bytes, format="pdf"). It is not yet confirmed whether the SDK accepts raw bytes or requires base64 encoding for PDF input.
  • _call_local_pdf() — mirrors _call_local() with {"pdf": <base64>, "format": "pdf"}.

The loader is complete end-to-end and all unit tests pass against a mock SDK. Real API call verification is blocked on SDK documentation and is the only outstanding item before this feature can be considered production-ready.

Does this PR meet the acceptance criteria?

  • Tests added for new/changed behavior — 26 unit tests in test_glm_ocr_loader.py covering init validation, error conditions, cloud-mode OCR paths, page/document breakdown strategies, OCRResult dataclass contract, GLMOCRPDFLoader error paths, and the ImportError install-hint guard
  • All tests passing — 1639 passed, 5 skipped, 0 failed
  • Follows project style guide — ruff check and ruff format --check both clean across all changed files
  • No breaking changes introduced — existing _ocr_image() -> str abstract contract is preserved; PyPDFOCRLoader is unchanged; OCRResult is additive
  • GLM-OCR API response schema stubs require verification against the live SDK once documentation is published (five # TODO comments in glm_ocr_loader.py)

Files changed

File Change
packages/railtracks/src/railtracks/retrieval/models.py Added OCRResult dataclass with markdown, bboxes, tables fields and to_text()
packages/railtracks/src/railtracks/retrieval/__init__.py Exported OCRResult from .models and added to __all__
packages/railtracks/src/railtracks/retrieval/loaders/base_ocr.py Added non-abstract _ocr_image_structured(image) -> OCRResult with default delegation to _ocr_image()
packages/railtracks/src/railtracks/retrieval/loaders/glm_ocr_loader.py New file — GLMOCRLoader and GLMOCRPDFLoader implementations (405 lines)
packages/railtracks/pyproject.toml Added [glm] optional extra: glmocr >= 0.1.0, httpx >= 0.23.0, pillow >= 10.0.0
packages/railtracks/tests/unit_tests/retrieval/loaders/test_glm_ocr_loader.py New file — 26 unit tests across 8 test classes (314 lines)

@contactvishwav

Copy link
Copy Markdown
Author

Hi @Amir-R25 — this is my first contribution to railtracks. I've implemented the GLMOCRLoader requested in #1175, following the PyPDFOCRLoader pattern and your stated recommendation (option ii) to extend BaseOCRLoader with
_ocr_image_structured() -> OCRResult while keeping the existing _ocr_image() -> str contract intact for backward compatibility.

One honest note: GLM-OCR's API response schema isn't fully published yet, so _parse_glmocr_response() and the call* methods contain documented stubs with TODO comments. Happy to update those once the schema is confirmed, or if you have access to the live SDK response format.

Would appreciate a review when you have time!

@soulFood5632 soulFood5632 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hello thanks for the PR. I appreciate you taking the time to go through this issue.

However, there are a couple of tweaks that will be required. I will be taking a deeper dive on the PR and ensuring functionality once you have addressed the concerns I laid out.

Please let me know if you have any questions or concerns with any of the above comments.



def _parse_glmocr_response(raw: dict) -> OCRResult:
# TODO: Confirm actual GLM-OCR JSON response schema once SDK docs are

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you can please remove this loose TODO. I would rather just create an issue if and when the schema changes. We should also for version lock the glmocr library to prevent issues.

"""
...

async def _ocr_image_structured(self, image: Image) -> OCRResult:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking through this logic I don't mind a new external facing abstraction for OCR result. I would like to confirm the design with @Amir-R25 or @Pooria90. I just want to make sure the design is general enough to meet requirements of future work since we will want to maintain backwards compatibility.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Happy to wait for @Amir-R25 and @Pooria90's input before making any changes here.

is an engine-specific dict representing one table.
"""

markdown: str

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I want to hear @Pooria90 or @Amir-R25's thoughts on this new abstraction.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Happy to wait for @Amir-R25 and @Pooria90's input before making any changes here.

Encodes the image as PNG bytes in the calling thread, then offloads
the blocking network call to a worker thread.

# TODO: Replace with confirmed SDK signature once glmocr API docs are

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here, lets remove the loose Todo and again create an issue as required.

# TODO: Replace with confirmed SDK signature once glmocr API docs are
# finalised. Current stub: glmocr.ocr(image_bytes) -> dict
"""
import io

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you can can you please get rid of this local import and use the top level as per PEP guidelines


async def _stream_image(self, path: Path) -> AsyncGenerator[Document, None]:
"""Yield a single Document from one image file."""
from PIL import Image as PILImage

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here, please avoid local imports

# TODO: Confirm local endpoint schema for PDF input. Current stub
# mirrors the image path with "pdf" as the key and format flag.
"""
import base64

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please avoid local imports here

async def _call_local_pdf(self, pdf_bytes: bytes) -> OCRResult:
"""POST PDF bytes (base64-encoded) to the local endpoint.

# TODO: Confirm local endpoint schema for PDF input. Current stub

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

get rid of loose todo

async def _call_cloud_pdf(self, pdf_bytes: bytes) -> OCRResult:
"""Send PDF bytes to the Zhipu cloud API.

# TODO: Confirm whether the SDK accepts raw bytes or requires base64,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please get rid of loose todo. either answer the question in your code or place an issue for it

Comment thread .gitignore Outdated

# Ignore Files copied during release workflow for PyPI packaging (for Flit)
packages/railtracks/docs/ No newline at end of file
packages/railtracks/docs/scripts/repro_issue_1175.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should not be included in the gitignore please revert

@contactvishwav

Copy link
Copy Markdown
Author

Hi @soulFood5632 — thanks for the thorough review! I've addressed all the requested changes:

  • Removed all loose TODO comments from glm_ocr_loader.py — the stub implementations now read as the intended behavior (38b85e7)
  • Moved all local imports (io, base64, PIL) to module level per PEP 8 (027f854)
  • Version locked glmocr in pyproject.toml (dfa8b83)
  • Reverted the unintended .gitignore change (e424d33)

On the _ocr_image_structured() and OCRResult design questions — happy to wait for @Amir-R25 and @Pooria90's input before making any changes there. Let me know if anything else needs adjusting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature][Retrieval] GLM-OCR

2 participants