fix: pass num_retries to streaming completion path#9460
Open
saivedant169 wants to merge 2 commits intostanfordnlp:mainfrom
Open
fix: pass num_retries to streaming completion path#9460saivedant169 wants to merge 2 commits intostanfordnlp:mainfrom
saivedant169 wants to merge 2 commits intostanfordnlp:mainfrom
Conversation
The streaming path via _get_stream_completion_fn called litellm.acompletion(stream=True) without num_retries or retry_strategy, so rate limit errors (429) crashed immediately instead of retrying with exponential backoff. Thread num_retries through _get_stream_completion_fn and pass it along with retry_strategy to the litellm.acompletion call, matching the non-streaming path behavior. Also fix alitellm_completion to pass headers to the streaming path (previously missing, inconsistent with litellm_completion). Fixes stanfordnlp#9459
isaacbmiller
requested changes
Mar 16, 2026
dspy/clients/lm.py
Outdated
| def _get_stream_completion_fn( | ||
| request: dict[str, Any], | ||
| cache_kwargs: dict[str, Any], | ||
| num_retries: int = 0, |
Collaborator
There was a problem hiding this comment.
Let's have num_retries as a required function parameter
dspy/clients/lm.py
Outdated
| headers = request.pop("headers", None) | ||
| stream_completion = _get_stream_completion_fn(request, cache, sync=False) | ||
| stream_completion = _get_stream_completion_fn( | ||
| request, cache, num_retries=num_retries, sync=False, headers=_add_dspy_identifier_to_headers(headers) |
Collaborator
There was a problem hiding this comment.
Make the syntax should match lm:355
dspy/clients/lm.py
Outdated
| messages: list[dict[str, Any]] | None = None, | ||
| **kwargs | ||
| ): | ||
| def forward(self, prompt: str | None = None, messages: list[dict[str, Any]] | None = None, **kwargs): |
Collaborator
There was a problem hiding this comment.
undo change unless necessary for ruff
…evert formatting - Make num_retries a required parameter in _get_stream_completion_fn - Match alitellm_completion syntax with litellm_completion (line 355 style) - Revert unrelated ruff formatting change on forward() signature
Author
|
Hey @isaacbmiller , wanted to see if there's anything else you need from me on this. Happy to make changes if something's off. |
Author
|
Hey @isaacbmiller, all three items from your review are in. num_retries is required, the async caller matches the sync style, and the formatting is reverted. Let me know if anything else needs changing. |
Author
ForgeArena ReviewDecision: PASS (score: 89/100) Evaluation
Risk Signals
Policy
approved by ForgeArena - the merge gate for AI-generated code |
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
2 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.
Fixes #9459
Description
The streaming completion path via
_get_stream_completion_fncalledlitellm.acompletion(stream=True)withoutnum_retriesorretry_strategy, so rate limit errors (429) crashed immediately instead of retrying with exponential backoff. The non-streaming path correctly passes both parameters.Changes
num_retriesparameter to_get_stream_completion_fn()num_retriesandretry_strategy="exponential_backoff_retry"tolitellm.acompletion(stream=True)inside the stream closurelitellm_completion()caller to forwardnum_retriesalitellm_completion()caller to forwardnum_retriesandheaders(headers were also previously missing from the streaming path inconsistent with the sync caller and the non-streaming path)Before vs After
Testing
ruff checkandruff formatpass clean.