fix(indexing): keep one failing document from failing its batch (#15563) to release v4.8

Merged
DA
3d ago
+421-107

Cherry-pick of #15563 to release v4.8. Prerequisite, not one of the four requested backports: #16178 needs the force_update flag and the per-document retry this introduces, and neither exists on this branch.

Upstream commit: 4194b87e4c8468b362628d9ede4cc17346793b1a

This branch is further from main than v4.9, so this needed more adaptation than the v4.9 twin (#16300). Please read the resolution before approving.

Document index. The branch predates the consolidation (#15336), so the pipeline writes to a list of backends: the ported index_doc_batch_with_handler passes document_indices=document_indices, index_doc_batch keeps this branch's primary_doc_idx_insertion_records and its asserts, and _maybe_push_documents is wrapped in the DocumentPushFailure the fix adds.

LLM API. The branch predates GenerationRequest / GenerationContext entirely, so upstream's version of the touched contextual-RAG code does not compile here:

  • _summarize_document, the helper upstream adds to replace two duplicated inline blocks, is rewritten against this branch's API (llm_generation_span + llm.invoke(prompt, max_tokens=...) + record_llm_response + llm_response_to_string). Both call sites in add_document_summaries and add_chunk_summaries now use it, as upstream does.
  • The chunk-context call keeps this branch's process_with_prompt_cache / llm_generation_span form, with the swallowing except LLMRateLimitError / except Exception removed — that removal is the substance of the fix.
  • Dropped the now-unused from onyx.llm.multi_llm import LLMRateLimitError.

Tests. test_contextual_rag_raises_when_a_call_fails is adapted: upstream's fake LLM picks the failing call from kwargs["context"].flow and returns AssistantMessage, neither of which exists here. This branch passes the flow to llm_generation_span rather than to invoke, so the fake tells the two calls apart by the prompt — only the document summary renders DOCUMENT_SUMMARY_PROMPT — and returns ModelResponse, as the other tests in the file do. Both parametrised cases still assert the exception escapes add_contextual_summaries, which is the behaviour under test. The three index_doc_batch_with_handler calls pass document_indices=[MagicMock()].

Behaviour note for the release manager: after this, a failed contextual-RAG summary or chunk-context call fails the document instead of indexing it without context. The batch handler then reports it per document.

Verified: tests/unit/onyx/background and tests/unit/onyx/indexing pass (303 passed, 6 skipped); ruff check clean on every file this touches.

Stacked backport. Merge in order: #16296 (#16115) → #16298 (#16261) → this PR → #16178 → #16153.

Summary by cubic

Cherry-picks #15563 onto release/v4.8 so one failing document (for example a failed contextual-RAG LLM call) no longer fails its entire batch. The batch now retries per document, and only documents whose prepare() already committed skip the change gates via force_update.

This is a prerequisite backport, not one of the four requested: #16178 needs the force_update flag and per-document retry this introduces. The branch is further from main than v4.9, so this port needs more adaptation than the v4.9 twin (#16300) and adapts to the pre-GenerationRequest/GenerationContext and pre-consolidated doc-index APIs.

Backport notes

  • The pipeline writes to a list of backends and wraps _maybe_push_documents in the new DocumentPushFailure, which the handler does not retry per document.
  • _summarize_document replaces two duplicated inline blocks against this branch's llm_generation_span + llm.invoke API; the chunk-context call keeps process_with_prompt_cache with the swallowing except LLMRateLimitError/except Exception removed.
  • Removing that swallowing is the substance of the fix: a failed summary or chunk-context call now fails the document instead of indexing it without context, and the handler reports it per document.
  • The adapted test distinguishes the two calls by prompt since the flow goes to llm_generation_span here; both parametrised cases still assert the exception escapes.
  • Verified: tests/unit/onyx/background and tests/unit/onyx/indexing pass (303 passed, 6 skipped); ruff check clean.

Merge order