Repository navigation
fix(streaming): backport text-completion usage fix and e2e provider-flake tolerance to rc/1.103.0 - #43400
Conversation
…2628) * test(e2e): tolerate provider-side flakes on five full-suite cells Mistral OCR retries a provider-relayed 429 with backoff, the Vertex vision probe turns reasoning off so its 32 tokens go to the answer, the Vertex cache cell spaces eight never-seen prefixes 15s apart around Google's nondeterministic minimum-token rejection and prices the cached tokens instead of prompt_tokens, and the Azure content-policy cell resends the jailbreak prompt while Azure skips its filter * test(e2e): shorten the new helper docstrings * test(e2e): accept a relayed provider 429 on the rust OCR cells The gateway already retries a provider 429 three times per call and the Mistral key is shared across pipelines, so a throttle can hold across all four attempts of the OCR cell. After the bounded retries the cell now accepts the gateway's faithful relay of the provider's 429 (throttling_error, code 429) as its second expected outcome; the gateway's own 429 and any other error still fail the cell at once. * test(e2e): drop the harness unit tests, the live cells cover the helpers --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com> (cherry picked from commit 41ca465)
|
|
GitHub Actions
Waiting
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| case RateLimitedError() as outcome: | ||
| _assert_provider_rate_limit_relayed(model, outcome) |
There was a problem hiding this comment.
Rule Used: What: Flag any modifications to existing tests and verify they don't weaken test coverage or mask regressions. Why: Developers may alter tests to make failing code pass rather than fix the actual bug, hiding regressions. Good: ``` // Test updated t... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| for attempt in range(1, attempts): | ||
| match issue(): | ||
| case RateLimitedError(body=body, retry_after_seconds=retry_after) if PROVIDER_RATE_LIMIT_MARKER in body: | ||
| delay = retry_after or PROVIDER_RATE_LIMIT_BACKOFF_SECONDS * (1 << (attempt - 1)) |
There was a problem hiding this comment.
delay local lacks : Final, as do candidate and resp elsewhere in this change. The repository requires every variable to have that annotation; satisfy this requirement before merging.
Context Used: AGENTS.md (source)
This branch is waiting to be deployed
2 participants
TLDR
Problem this solves:
/v1/completionswithinclude_usagecan end in aMockValSererror instead of usageHow it solves it:
Usagebefore attaching itprompt_tokensUser Flow
Before: a developer streaming a legacy text completion with usage turned on gets an error where the usage should be
"stream": trueand"stream_options": {"include_usage": true}'MockValSer' object is not an instance of 'SchemaSerializer', and no usage arrivesAfter: the same request ends with real token counts
usagewith prompt, completion and total tokensRelevant issues
Backport of #42628 and #43047
Affected release
regression in v1.94.0-rc.1
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)The
MockValSerbug came in with #32255 (commit8a4942340e, first in v1.94.0-rc.1), which copies the OpenAI SDK'sCompletionUsageonto litellm's streamed response as is. The SDK defers building its pydantic serializers, so in a process where nothing has built that class yet, dumping the chunk fails. Run alone against the real OpenAI API,tests/local_testing/test_streaming.py::test_openai_stream_options_call_text_completionpasses at the parent of8a4942340e, fails at8a4942340e, fails on the rc/1.103.0 tip, and passes on this branch. In CircleCI it only fails when no earlier test on the same worker has built the class, which is why it looks flaky there (5 failures in the last 59 main runs)test_streaming_handler.pypasses on this branch (138 tests), and #43047's two new cases fail with its source change reverted. The e2e files touched by #42628 collect, andtest_e2e_http.pyplustest_batch_cleanup.pypass (71 tests)Adaptations from the main versions:
test_ocr_rust_e2e.py: rc's tests use theendpoints_clientfixture, so the rate-limit handling was applied to it instead of main'sproxyfixturetest_chat_completions_regression_e2e.py: rc has no Vertex or Azure chat test classes, so only the vision assertion change was keptType
🐛 Bug Fix
✅ Test
Caveats (if any)
Medium