Leads 233 retry api call incase of error#173
Merged
asamal4 merged 3 commits intolightspeed-core:mainfrom Feb 28, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds configurable retry-on-429 behavior (default 3) to the API client, centralizes client configuration via APIConfig, integrates disk-cache checks/updates into query flow, updates constructor signature to accept APIConfig, and updates tests/fixtures to validate retry detection, retry flows, and exhaustion handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as APIClient
participant Config as APIConfig
participant Cache as Disk Cache
participant Decorator as Retry Decorator
participant API as HTTP API
Client->>Config: read api_base, version, timeout, num_retries
Client->>Cache: init/check (if cache_enabled)
Client->>Decorator: call query via retry wrapper
Decorator->>Cache: check cache for request (if applicable)
alt cached
Cache-->>Decorator: return cached response
Decorator-->>Client: return APIResponse
else not cached
Decorator->>API: POST request
alt 429 Too Many Requests
API-->>Decorator: 429 response
Decorator->>Decorator: backoff & retry (up to num_retries)
Decorator->>API: retry POST request
API-->>Decorator: success (200) or final 429
else Other HTTP Error
API-->>Decorator: non-429 error
Decorator-->>Client: raise APIError
else Success
API-->>Decorator: 200 response
end
Decorator->>Cache: update cache (if enabled)
Decorator-->>Client: return APIResponse
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lightspeed_evaluation/core/api/client.py`:
- Around line 64-67: The configured retry_attempts is being passed directly to
tenacity.stop_after_attempt which treats the value as total attempts (initial
call + retries), causing an off-by-one; update the retry policy in the
retry(...) call to stop after self.config.retry_attempts + 1 attempts (i.e.,
convert configured retries to total attempts) so the semantics match the
retry_attempts name, and adjust any related tests/assertions accordingly;
reference the retry(...) invocation and the _is_too_many_requests_error
predicate and use self.config.retry_attempts when computing the +1.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
config/system.yamlsrc/lightspeed_evaluation/core/api/client.pysrc/lightspeed_evaluation/core/constants.pysrc/lightspeed_evaluation/core/models/system.pytests/unit/core/api/conftest.pytests/unit/core/api/test_client.py
xmican10
force-pushed
the
LEADS-233-retry-api-call-incase-of-error
branch
from
ca74a80 to
5d322d4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/core/api/test_client.py (1)
517-608: Neutralize tenacity backoff delays in retry tests for faster, more deterministic execution.The retry decorator applies
wait_exponential(multiplier=1, min=4, max=100)to decorated methods (lines 60–61 in client.py), causing real sleep delays (minimum 4 seconds per retry) when tests trigger 429 error paths. Patch tenacity's wait strategy towait_none()in the three affected tests to eliminate these delays while preserving retry count assertions:
test_standard_query_retries_on_429_then_succeedstest_streaming_query_retries_on_429_then_succeedstest_query_raises_api_error_after_max_retriesExample:
mocker.patch("lightspeed_evaluation.core.api.client.wait_exponential", return_value=tenacity.wait_none())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/api/test_client.py` around lines 517 - 608, The retry backoff in APIClient (decorated with wait_exponential) causes real delays in the three tests; patch the wait strategy to tenacity.wait_none() at the start of each test to neutralize sleeping: in test_standard_query_retries_on_429_then_succeeds, test_streaming_query_retries_on_429_then_succeeds, and test_query_raises_api_error_after_max_retries use mocker.patch("lightspeed_evaluation.core.api.client.wait_exponential", return_value=tenacity.wait_none()) so the decorated methods (the retry behavior on 429) still run but without real backoff delays, keeping retry count assertions intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lightspeed_evaluation/core/api/client.py`:
- Line 66: The inline trailing comment on the call to stop_after_attempt in the
client retry configuration (the line containing
stop=stop_after_attempt(self.config.retry_attempts + 1)) exceeds
line-length/Black rules; move the explanatory comment to its own preceding line
or shorten it so the call line fits the formatter (for example place a
standalone comment like "# +1 to account for the initial attempt" above the
stop=... expression) and ensure you keep the exact expression
stop=stop_after_attempt(self.config.retry_attempts + 1) unchanged.
---
Nitpick comments:
In `@tests/unit/core/api/test_client.py`:
- Around line 517-608: The retry backoff in APIClient (decorated with
wait_exponential) causes real delays in the three tests; patch the wait strategy
to tenacity.wait_none() at the start of each test to neutralize sleeping: in
test_standard_query_retries_on_429_then_succeeds,
test_streaming_query_retries_on_429_then_succeeds, and
test_query_raises_api_error_after_max_retries use
mocker.patch("lightspeed_evaluation.core.api.client.wait_exponential",
return_value=tenacity.wait_none()) so the decorated methods (the retry behavior
on 429) still run but without real backoff delays, keeping retry count
assertions intact.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lightspeed_evaluation/core/api/client.pytests/unit/core/api/test_client.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/core/api/test_client.py`:
- Around line 517-520: The docstring for the test function
test_standard_query_retries_on_429_then_succeeds contains a typo: it uses "than"
where it should be "then"; update the docstring in
test_standard_query_retries_on_429_then_succeeds (and the other similar
docstring around line 550) to replace "than" with "then" so the sentence reads
"...retries on 429 error and then succeeds on retry."
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lightspeed_evaluation/core/api/client.pytests/unit/core/api/test_client.py
…ng configs defined in conftest
xmican10
force-pushed
the
LEADS-233-retry-api-call-incase-of-error
branch
from
e7bf583 to
69b86c3
Compare
asamal4
reviewed
Feb 27, 2026
Collaborator
asamal4
left a comment
asamal4
left a comment
There was a problem hiding this comment.
Thank you !!
some minor comments..
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
tests/unit/core/api/test_client.py (1)
547-550:⚠️ Potential issue | 🟡 MinorFix the remaining docstring typo.
Line 550 still uses “than” where “then” is intended.
✏️ Proposed fix
- """Test streaming query retries on 429 error and than succeeds on retry.""" + """Test streaming query retries on 429 error and then succeeds on retry."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/api/test_client.py` around lines 547 - 550, Fix the docstring typo in the test_streaming_query_retries_on_429_then_succeeds function: change the word "than" to "then" so the docstring reads "retries on 429 error and then succeeds on retry." Ensure the updated text is in the function's docstring line.src/lightspeed_evaluation/core/api/client.py (1)
123-136:⚠️ Potential issue | 🟠 MajorCache lookups can return incorrect responses across conversations/endpoints.
Line 123 and Line 134 now enable cache read/write for all queries, but the cache key does not include
conversation_id,endpoint_type, or API version. This can serve stale/wrong responses for follow-up turns or different endpoint modes sharing the same cache directory.💡 Proposed fix
def _get_cache_key(self, request: APIRequest) -> str: """Get cache key for the query.""" # Note, python hash is initialized randomly so can't be used here request_dict = request.model_dump() - keys_to_hash = [ - "query", - "provider", - "model", - "no_tools", - "system_prompt", - "attachments", - ] - str_request = ",".join([str(request_dict[k]) for k in keys_to_hash]) + key_payload = { + "query": request_dict.get("query"), + "provider": request_dict.get("provider"), + "model": request_dict.get("model"), + "no_tools": request_dict.get("no_tools"), + "system_prompt": request_dict.get("system_prompt"), + "attachments": request_dict.get("attachments"), + "conversation_id": request_dict.get("conversation_id"), + "endpoint_type": self.config.endpoint_type, + "api_version": self.config.version, + } + str_request = json.dumps(key_payload, sort_keys=True, default=str) return hashlib.sha256(str_request.encode()).hexdigest()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightspeed_evaluation/core/api/client.py` around lines 123 - 136, The cache lookup/write in methods _get_cached_response and _add_response_to_cache currently uses a key that can collide across conversations and endpoint modes; update the cache key generation to incorporate unique request context such as api_request.conversation_id (or conversation_id if separate), self.config.endpoint_type, and the API version (e.g., self.config.api_version or api_request.api_version) so that cached entries are namespaced by conversation and endpoint/version, and use that enhanced key both when reading (in _get_cached_response) and writing (in _add_response_to_cache) to prevent returning stale or cross-conversation responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/lightspeed_evaluation/core/api/client.py`:
- Around line 123-136: The cache lookup/write in methods _get_cached_response
and _add_response_to_cache currently uses a key that can collide across
conversations and endpoint modes; update the cache key generation to incorporate
unique request context such as api_request.conversation_id (or conversation_id
if separate), self.config.endpoint_type, and the API version (e.g.,
self.config.api_version or api_request.api_version) so that cached entries are
namespaced by conversation and endpoint/version, and use that enhanced key both
when reading (in _get_cached_response) and writing (in _add_response_to_cache)
to prevent returning stale or cross-conversation responses.
In `@tests/unit/core/api/test_client.py`:
- Around line 547-550: Fix the docstring typo in the
test_streaming_query_retries_on_429_then_succeeds function: change the word
"than" to "then" so the docstring reads "retries on 429 error and then succeeds
on retry." Ensure the updated text is in the function's docstring line.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
config/system.yamlsrc/lightspeed_evaluation/core/api/client.pysrc/lightspeed_evaluation/core/constants.pysrc/lightspeed_evaluation/core/models/system.pytests/unit/core/api/conftest.pytests/unit/core/api/test_client.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lightspeed_evaluation/core/models/system.py
xmican10
force-pushed
the
LEADS-233-retry-api-call-incase-of-error
branch
from
be31217 to
a1dfbbc
Compare
asamal4
approved these changes
Feb 28, 2026
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
3 participants
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.
Description
/streamingand/queryendpoints and retried, using tenacity library and its exponential waitretry_attemptsparameter was added tosystem.yamlfor handling number of retries, default set to 10retry_attemptsare reached and 429 persists,APIErroris rased with a message specifying that the number of retries was reachedconftest.pyAPIConfigsType of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit