mirror of
https://github.com/VectifyAI/PageIndex.git
synced 2026-07-15 21:11:05 +02:00
fix: degrade to empty result on LLM retry exhaustion instead of aborting
llm_completion/llm_acompletion raised RuntimeError once retries were
exhausted, which propagated through the unprotected sync TOC-detection
chain (find_toc_pages -> toc_transformer -> toc_extractor -> ...) and
aborted the whole document index — a regression vs. the pre-SDK behavior
that returned "" and let callers degrade (extract_json('') -> {} ->
.get(default), falling back to no-TOC indexing).
Restore the empty-result contract ("" / ("", "error") with
return_finish_reason), now logged at WARNING so the failure is visible
rather than silent. The async gather sites keep return_exceptions=True to
absorb any non-LLM error. A single persistently-failing call no longer
blocks indexing the rest of the document.
This commit is contained in:
parent
d3ea9b9f2e
commit
9ad7c687e0
2 changed files with 54 additions and 4 deletions
|
|
@ -165,8 +165,17 @@ def llm_completion(model, prompt, chat_history=None, return_finish_reason=False)
|
|||
if i < max_retries - 1:
|
||||
time.sleep(1)
|
||||
else:
|
||||
logger.error('Max retries reached for prompt: ' + prompt)
|
||||
raise RuntimeError(f"LLM call failed after {max_retries} retries") from e
|
||||
# Degrade gracefully instead of aborting the whole index: a single
|
||||
# persistently-failing call returns an empty result so callers can
|
||||
# skip that step (extract_json('') -> {} -> .get(default)) and the
|
||||
# rest of the document still gets indexed. Logged at WARNING so the
|
||||
# failure is visible, not silent.
|
||||
logger.warning(
|
||||
"LLM completion failed after %d retries; degrading to an empty "
|
||||
"result so the caller can skip this step. Last error: %s",
|
||||
max_retries, e,
|
||||
)
|
||||
return ("", "error") if return_finish_reason else ""
|
||||
|
||||
|
||||
|
||||
|
|
@ -192,8 +201,16 @@ async def llm_acompletion(model, prompt):
|
|||
if i < max_retries - 1:
|
||||
await asyncio.sleep(1)
|
||||
else:
|
||||
logger.error('Max retries reached for prompt: ' + prompt)
|
||||
raise RuntimeError(f"LLM call failed after {max_retries} retries") from e
|
||||
# Degrade gracefully (see llm_completion): return an empty result
|
||||
# so the caller skips this step and the rest of the document still
|
||||
# indexes. The gather sites still keep return_exceptions=True to
|
||||
# absorb any non-LLM error. WARNING so it's visible, not silent.
|
||||
logger.warning(
|
||||
"Async LLM completion failed after %d retries; degrading to an "
|
||||
"empty result so the caller can skip this step. Last error: %s",
|
||||
max_retries, e,
|
||||
)
|
||||
return ""
|
||||
|
||||
|
||||
def extract_json(content):
|
||||
|
|
|
|||
|
|
@ -224,6 +224,39 @@ def test_llm_acompletion_passes_a_timeout_to_litellm(monkeypatch):
|
|||
assert "timeout" in seen and seen["timeout"] == get_llm_params()["timeout"]
|
||||
|
||||
|
||||
def test_llm_completion_degrades_to_empty_on_exhaustion(monkeypatch, caplog):
|
||||
# A persistently-failing LLM call must NOT abort the whole index: it returns
|
||||
# an empty result (logged at WARNING, not silent) so callers degrade
|
||||
# (extract_json('') -> {} -> .get(default)) and the rest still indexes.
|
||||
import logging
|
||||
|
||||
def boom(**kwargs):
|
||||
raise RuntimeError("provider down")
|
||||
|
||||
monkeypatch.setattr("litellm.completion", boom)
|
||||
monkeypatch.setattr("pageindex.index.utils.time.sleep", lambda *a: None) # skip retry backoff
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="pageindex.index.utils"):
|
||||
assert llm_completion("gpt-x", "hi") == ""
|
||||
assert llm_completion("gpt-x", "hi", return_finish_reason=True) == ("", "error")
|
||||
assert any("failed after" in r.message and "degrading" in r.message for r in caplog.records)
|
||||
|
||||
|
||||
def test_llm_acompletion_degrades_to_empty_on_exhaustion(monkeypatch):
|
||||
# Async counterpart: exhausted retries return "" instead of raising, so the
|
||||
# return_exceptions gathers see a plain empty result and callers degrade.
|
||||
async def boom(**kwargs):
|
||||
raise RuntimeError("provider down")
|
||||
|
||||
async def _instant_sleep(*a):
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("litellm.acompletion", boom)
|
||||
monkeypatch.setattr("pageindex.index.utils.asyncio.sleep", _instant_sleep) # skip retry backoff
|
||||
|
||||
assert asyncio.run(llm_acompletion("gpt-x", "hi")) == ""
|
||||
|
||||
|
||||
def test_llm_completion_holds_the_shared_semaphore(monkeypatch):
|
||||
# Sync litellm.completion calls must share the same process-wide cap as the
|
||||
# async path; otherwise concurrent indexing threads can exceed
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue