Skip to content

fix(service-mode): add support for TXT and HTML chunking in service mode - #2368

Open
KyleZheng1284 wants to merge 6 commits into
NVIDIA:mainfrom
KyleZheng1284:fix/service-mode-text-chunking
Open

fix(service-mode): add support for TXT and HTML chunking in service mode#2368
KyleZheng1284 wants to merge 6 commits into
NVIDIA:mainfrom
KyleZheng1284:fix/service-mode-text-chunking

Conversation

@KyleZheng1284

@KyleZheng1284 KyleZheng1284 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

TXT and HTML ingestion did not work correctly in service mode because text chunking depended on transformers.AutoTokenizer. Service mode intentionally excludes transformers, PyTorch, GPU models, and model weights to keep the image lightweight.

The missing dependency was also hidden by the extraction actors, causing ingestion to return a misleading empty-result error.

What changed

  • Replaced AutoTokenizer with the lightweight tokenizers and huggingface-hub packages.
  • Reused the existing TXT/HTML extraction and chunking logic; only tokenizer loading changed.
  • Used the revision-pinned tokenizer associated with the default embedding model.
  • Bundled only tokenizer.json in the service image—no model weights, PyTorch, or transformers.
  • Enabled deterministic offline tokenizer loading at runtime.
  • Preserved per-document error isolation while allowing tokenizer configuration failures to surface clearly.
  • Added regression tests and updated the relevant documentation.

The chunker only requires encode() and decode(), so loading a full Transformers model stack doesn't seem to be necessary imo

Result

Service-mode deployments can now ingest and chunk TXT and HTML documents while keeping the production image lightweight. If the required tokenizer artifact is unavailable, ingestion reports a clear failure instead of silently returning zero rows.

Validation

  • 127 focused tokenizer, extraction, and service-pipeline tests passed.
  • The slim service image built successfully.
  • Offline TXT and HTML chunking passed inside the image without transformers.
  • Live hosted TXT ingestion completed successfully.
  • The indexed TXT content was retrieved through /v1/query.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date.

Signed-off-by: Kyle Zheng <126034466+KyleZheng1284@users.noreply.github.com>
@KyleZheng1284
KyleZheng1284 requested review from a team as code owners July 16, 2026 03:19
@KyleZheng1284
KyleZheng1284 requested a review from jperez999 July 16, 2026 03:19
@KyleZheng1284
KyleZheng1284 marked this pull request as draft July 16, 2026 03:20
@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Replaces transformers.AutoTokenizer with the lightweight tokenizers + huggingface-hub packages for TXT and HTML chunking in service mode, eliminating a missing-dependency failure that silently returned empty results. The tokenizer artifact is pre-cached at Docker build time and the service image runs fully offline via HF_HUB_OFFLINE=1.

  • tokenizer_provider.py (new): Defines a ChunkTokenizer protocol and an lru_cache-backed load_chunk_tokenizer that downloads only tokenizer.json from the pinned revision. get_hf_revision is called inside the try block, so a ValueError for an unregistered model is correctly wrapped into TokenizerUnavailableError.
  • Actor exception isolation: except TokenizerUnavailableError: raise is inserted before the pre-existing except Exception: continue, so configuration failures surface at the batch level while per-document parse errors remain isolated.
  • raw or bytes truthiness fix: Both actor files now use raw if raw is not None else text.encode("utf-8") to correctly handle an empty-bytes payload.

Confidence Score: 5/5

Safe to merge; the change is well-contained, comprehensively tested, and all issues from prior review rounds have been addressed.

The tokenizer loading is self-contained and deterministic: get_hf_revision is inside the try block so all failure modes funnel through TokenizerUnavailableError, the lru_cache is process-scoped without cross-worker interference, and the Dockerfile correctly pre-caches the artifact before enabling offline mode. The exception-isolation contract in the actors is unchanged and explicitly covered by new tests.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
nemo_retriever/src/nemo_retriever/common/modality/txt/tokenizer_provider.py New file: defines ChunkTokenizer protocol, _FastTokenizer adapter, and lru_cache-backed load_chunk_tokenizer; get_hf_revision is inside the try block so ValueError for unregistered models is correctly wrapped into TokenizerUnavailableError.
nemo_retriever/src/nemo_retriever/common/modality/txt/split.py Replaced AutoTokenizer with load_chunk_tokenizer; DEFAULT_TOKENIZER_MODEL_ID now references VL_EMBED_MODEL constant; docstring updated to match new type.
nemo_retriever/src/nemo_retriever/operators/extract/txt/ray_data.py Added except TokenizerUnavailableError: raise before the per-document except Exception: continue; fixed raw or bytes truthiness bug to raw if raw is not None else.
nemo_retriever/src/nemo_retriever/operators/extract/html/ray_data.py Same exception-isolation pattern as txt/ray_data.py; same truthiness fix for raw bytes payload.
Dockerfile Sets HF_HOME in the install stage to /opt/nemo-retriever/huggingface; pre-caches tokenizer.json at build time; enables HF_HUB_OFFLINE=1 only in the service stage; adds /opt/nemo-retriever to chown so the nemo user can read the cached artifact.
nemo_retriever/tests/test_tokenizer_provider.py New test file covering: happy-path tokenizer loading, offline chunking without transformers, actionable error surfacing, unregistered model wrapping, TokenizerUnavailableError propagation, and per-document error isolation for both TXT and HTML actors.
.github/workflows/reusable-docker-build-and-test.yml Adds a CI smoke test that runs inside the built service image before installing test dependencies; verifies transformers is absent, tokenizers is present, and offline TXT/HTML chunking succeeds.
nemo_retriever/pyproject.toml Added tokenizers>=0.21.1 and huggingface-hub>=0.34.0 to the service extra; transformers is intentionally absent from this extra.

Sequence Diagram

sequenceDiagram
    participant Build as DockerBuild
    participant TP as tokenizer_provider
    participant HF as huggingface_hub
    participant A as TxtSplitCPUActor
    participant S as txt_bytes_to_chunks_df

    Build->>TP: load_chunk_tokenizer(VL_EMBED_MODEL)
    TP->>HF: hf_hub_download(tokenizer.json, pinned revision)
    HF-->>TP: path to cached tokenizer.json
    TP-->>Build: _FastTokenizer stored in HF_HOME

    A->>S: txt_bytes_to_chunks_df(payload, path)
    S->>TP: load_chunk_tokenizer(model_id)
    TP-->>S: _FastTokenizer (lru_cache hit, offline)
    S-->>A: chunk DataFrame

    alt TokenizerUnavailableError raised
        TP-->>A: TokenizerUnavailableError
        A-->>A: re-raise, batch fails with clear error
    else per-document exception
        S-->>A: Exception caught
        A-->>A: continue to next document
    end
Loading

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile

Comment thread nemo_retriever/src/nemo_retriever/common/modality/txt/tokenizer_provider.py Outdated
Comment thread nemo_retriever/src/nemo_retriever/operators/extract/txt/ray_data.py Outdated
Comment thread nemo_retriever/pyproject.toml
KyleZheng1284 and others added 4 commits July 15, 2026 20:46
Preserve per-document batch isolation while surfacing typed tokenizer failures, wrap unregistered model errors, document the lightweight tokenizer contract, and make CLI help tests width-deterministic across platforms.
Assert the public command and option contract without depending on Typer's version-specific metavar delimiters.
@KyleZheng1284
KyleZheng1284 marked this pull request as ready for review July 16, 2026 18:06
…xt-chunking

# Conflicts:
#	nemo_retriever/tests/test_root_cli_workflow.py
#	nemo_retriever/tests/test_root_query_cli.py
#	nemo_retriever/uv.lock

@charlesbluca charlesbluca left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two Docker image consistency points from the Helm deployment review:

Comment thread Dockerfile
. /opt/retriever_runtime/bin/activate \
&& uv pip install -e "./nemo_retriever[service]"
&& uv pip install -e "./nemo_retriever[service]" \
&& python -c "from nemo_retriever.common.modality.txt.tokenizer_provider import load_chunk_tokenizer; load_chunk_tokenizer('nvidia/llama-nemotron-embed-vl-1b-v2')"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we restore the DOWNLOAD_LLAMA_TOKENIZER build-argument gate that the existing Docker build and release tooling still passes, and use it for a shared pre-cache step in both install and install-service-gpu? The step should resolve DEFAULT_TOKENIZER_MODEL_ID instead of hardcoding the model ID, keeping the cached artifact aligned with the runtime chunker and its pinned revision. Currently the historical Dockerfile gate has been removed, so the tooling knob is ignored, service downloads unconditionally, and service-gpu does not pre-cache the tokenizer.

Comment thread Dockerfile
FROM install AS service

ENV NEMO_RETRIEVER_SERVICE_CONFIG=/etc/nemo-retriever/retriever-service.yaml
ENV HF_HUB_OFFLINE=1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please keep HF_HUB_OFFLINE=1 scoped to the service image when adding the tokenizer pre-cache to service-gpu. The GPU image is expected to download local Hugging Face model weights at runtime, so it should reuse the pre-cached tokenizer without globally disabling Hub access.

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.

2 participants