feat: retry + timeout + uniform errors for LLM calls (closes #28)#34
Merged
Conversation
LLMClient issued a single attempt per call, so a provider 429 / timeout / connection blip propagated as a raw SDK exception. Centralize resilient outbound handling for every LLMClient-based agent (report, ontology, tools). - _request_with_retries: retry transient provider errors (RateLimitError, APITimeoutError, APIConnectionError, InternalServerError) with exponential backoff; non-transient errors (bad request, auth) raise immediately. - Per-request timeout passed to the OpenAI client (primary + fallback). - Order: primary (with retries) -> backup model (with retries) -> give up. - LLMError: a single, actionable, user-facing failure surface raised when all attempts are exhausted. Subclasses RuntimeError and embeds the original detail + model, so existing catchers and the fallback tests still work. - Config: LLM_MAX_RETRIES, LLM_RETRY_INITIAL_DELAY, LLM_RETRY_MAX_DELAY, LLM_TIMEOUT; documented in .env.example. - tests/test_llm_retry.py: retry-then-succeed, exhausted->LLMError with actionable message, non-transient not retried, transient-then-fallback, timeout class membership. Note: HTTP-side rate limiting (incoming) is already wired via flask-limiter on the routes; this covers the outbound provider direction. Stacked on #20/#15. Co-Authored-By: PRATHAMESH75 <prathamesh290504@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #28. Adds resilient outbound handling for LLM provider calls — retry with backoff, per-request timeout, and a single actionable error surface — for every
LLMClient-based agent (report, ontology, tools).Changes
_request_with_retries— retries transient provider errors (RateLimitError,APITimeoutError,APIConnectionError,InternalServerError) with exponential backoff; non-transient errors (bad request, auth) raise immediately (no pointless retries).LLMError— one user-facing, actionable failure surface raised when everything is exhausted. SubclassesRuntimeErrorand embeds the original detail + model name, so existingexcept Exceptioncatchers and the Per-task model/provider config + fallback #15 fallback tests keep working unchanged.LLM_MAX_RETRIES,LLM_RETRY_INITIAL_DELAY,LLM_RETRY_MAX_DELAY,LLM_TIMEOUT(documented in.env.example).Acceptance criteria
LLMErrorwith retry/provider guidance)Testing
tests/test_llm_retry.py(5 tests): retry-then-succeed, exhausted→LLMErrorwith actionable message, non-transient not retried, transient-on-primary→fallback succeeds, timeout class membership.Scope note
The two raw-OpenAI generators (
oasis_profile_generator,simulation_config_generator) keep their own existing retry loops; this PR centralizes resilience for the sharedLLMClientpath. Folding those ontoLLMClientis a reasonable follow-up.🤖 Generated with Claude Code