Skip to content

fix: shared batch length check in BaseDetector (follow-up to #113) - #122

Open
YassinNouh21 wants to merge 1 commit into
KRLabsOrg:mainfrom
YassinNouh21:fix/shared-batch-length-check
Open

YassinNouh21 wants to merge 1 commit into
KRLabsOrg:mainfrom
YassinNouh21:fix/shared-batch-length-check

Conversation

@YassinNouh21

Copy link
Copy Markdown

Summary

This is a follow-up to #113. @adaamko asked for it when closing #114:

  1. A shared length check in BaseDetector that every detector calls; RAGFactCheckerDetector.predict_prompt_batch (rag_fact_checker.py:119) still zips silently.

This PR:

  • Adds BaseDetector._validate_batch_lengths(prompts, answers). It raises ValueError when the two lengths differ. The message keeps the old text and adds both lengths: Number of prompts must match number of answers, got 2 prompts and 1 answers.
  • Replaces the inline length checks in TransformerDetector, LLMDetector and RAGFactCheckerDetector with one call to the helper. The error type and the old message stay the same, so the perf: true batched inference for TransformerDetector (fixes #23) #113 tests pass without edits.
  • Makes the zip at rag_fact_checker.py:119 strict.

About rag_fact_checker.py:119

RAGFactCheckerDetector.predict_prompt_batch already rejected a prompts/answers mismatch before this PR. The silent zip at line 119 pairs answers with the results from self.rag.detect_hallucinations_batch(...). If the backend returns fewer results than answers, zip drops the extra answers with no error, and the caller gets a shorter list. With strict=True, this case now raises ValueError. The project requires Python 3.11 or later, so strict=True is available.

Not in this PR:

  • RAGFactChecker.detect_hallucinations_batch (lettucedetect/ragfactchecker.py:358) also zips contexts and answers with no length check. The detector path is safe now, because the detector checks the lengths first. A direct caller of RAGFactChecker can still get a shorter list. I can send a separate small PR for this if you want it.
  • Idea 2 from the same comment (sort-by-length micro-batching).

Related issue

Follow-up to #113. Refs #114 and #23.

Type of change

  • Bug fix
  • Feature
  • Documentation
  • Tests
  • Refactor or maintenance

Testing

  • ruff format --check lettucedetect/ lettucedetect_api/ tests/
  • ruff check lettucedetect/ lettucedetect_api/ tests/ --extend-exclude lettucedetect/integrations/
  • python -m pytest: 218 passed, with HF_HUB_OFFLINE=1 TRANSFORMERS_OFFLINE=1.
  • Other: I ran the new RAG tests against the old rag_fact_checker.py. test_short_backend_result_raises_instead_of_truncating fails there with DID NOT RAISE ValueError. It passes with this change.

New tests:

  • tests/test_inference_pytest.py::TestBatchLengthValidation: the helper raises with both lengths in the message. It accepts equal lengths, also two empty lists.
  • tests/test_rag_fact_checker_pytest.py: this file is new, because the RAG detector had no tests. The backend is a MagicMock, so the tests need no API key and no network. The tests cover three cases:
    1. A length mismatch raises before the backend gets a request.
    2. Equal lengths give one result for each pair, in input order, for spans and for tokens.
    3. A short backend result raises.

The transformer and LLM detectors keep their #113 tests (TestPredictPromptBatchLengthValidation and TestPredictPromptBatchLengthValidationLLM). These tests pass without changes.

Checklist

  • I kept the PR focused on one change.
  • I added or updated tests/docs when needed.
  • I checked that no secrets, API keys, or credentials are included.

Rights & sign-off (required)

  • I certify that I have the right to submit this code and that it may be
    distributed under the repository's MIT license
    (see CONTRIBUTING).

Add BaseDetector._validate_batch_lengths(prompts, answers). It raises
ValueError when the two lengths differ. The message shows both lengths.
TransformerDetector, LLMDetector and RAGFactCheckerDetector now call
the helper instead of an inline check. The old message text stays as a
prefix, so the KRLabsOrg#113 tests pass without edits.

RAGFactCheckerDetector.predict_prompt_batch now zips the answers and
the backend results with strict=True. Before this change, a short
backend result dropped answers with no error.

Add tests for the helper and a new test file for the RAG detector.

Follow-up to KRLabsOrg#113, refs KRLabsOrg#114.

Signed-off-by: yassinnouh21 <yassinnouh21@gmail.com>

This branch has not been deployed

No deployments
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.

1 participant