Conversation
Previously, `StreamRetryState::reset()` was called only when a streaming cycle completed with `Event::Finished`. This meant a stream that emitted content and then hit a rate limit mid-response — before ever reaching `Finished` — would permanently consume the retry budget. On a second identical mid-stream failure, the budget would be exhausted and jp would hard-fail even though each attempt successfully produced output. The fix introduces a `received_provider_event` flag in the turn loop's inner streaming cycle. The retry counter now resets on the very first successful event in a new streaming cycle, ensuring partial-but-retried streams don't accumulate against the budget. A new integration test (`test_retry_counter_resets_on_successful_event`) verifies the scenario with `max_retries = 1` and two mid-stream failures followed by success. On TTY terminals, retry notifications now also overwrite in-place using `\r\x1b[K` rather than printing a new line per attempt. The notification line is erased when the first successful event arrives or when a fatal error is printed, preventing stale status text from lingering on screen. Signed-off-by: Jean Mertz <git@jeanmertz.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
1 participant
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.
Previously,
StreamRetryState::reset()was called only when a streaming cycle completed withEvent::Finished. This meant a stream that emitted content and then hit a rate limit mid-response — before ever reachingFinished— would permanently consume the retry budget. On a second identical mid-stream failure, the budget would be exhausted and jp would hard-fail even though each attempt successfully produced output.The fix introduces a
received_provider_eventflag in the turn loop's inner streaming cycle. The retry counter now resets on the very first successful event in a new streaming cycle, ensuring partial-but-retried streams don't accumulate against the budget. A new integration test (test_retry_counter_resets_on_successful_event) verifies the scenario withmax_retries = 1and two mid-stream failures followed by success.On TTY terminals, retry notifications now also overwrite in-place using
\r\x1b[Krather than printing a new line per attempt. The notification line is erased when the first successful event arrives or when a fatal error is printed, preventing stale status text from lingering on screen.