Repository navigation
test(straiker): assert a saved api_version v1 with an sk_agt_ key routes to v3 - #44106
Conversation
…tes to v3 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit eea7cdb. Configure here.
dc449ec
into
litellm_straiker_v3_review_fixes
…tes to v3 (#44153) * test(straiker): assert a saved api_version v1 with an sk_agt_ key routes to v3 (#44106) Co-authored-by: yucheng <yucheng@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(straiker): ignore zombie workers when counting uvicorn children after a kill Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(straiker): describe the saved v1 cell by the v3 route it asserts Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: yucheng <yucheng@berri.ai>
TLDR
Problem this solves:
sk_agt_key saved withapi_version: v1to v3How it solves it:
/api/v3/detectand never to the v1 webhooklitellm_straiker_v3_review_fixes(a mirror of fix(guardrails): straiker v3 routes sk_agt_ keys to v3 and fails closed on a missing verdict #44011's tip fcebfa9) so it lands with itUser Flow
Before: a maintainer merging #44011 gets a red extensions suite on the next main run
integration-extensionson main after the mergetest_explicit_api_version_v1_overrides_key_prefixfails: the sink saw a v3 call, the test wanted the v1 webhookAfter: the suite describes the shipped behavior and stays green
integration-extensionson main after the mergetest_saved_api_version_v1_with_v3_key_routes_to_v3_not_the_v1_webhookpasses with the rest of the fileRelevant issues
Stacked on #44011, follow-up to #42781
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<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)Screenshots / Proof of Fix
Test-only change, so the proof is the integration rig from
tests/integration/README.md: a real proxy with two workers, real Postgres and Redis, the scripted upstream and a local Straiker sink, no mocks inside litellm. Legs are the #44011 merge base (63b6e7f) and the #44011 tip (fcebfa9) that this branch sits onBefore (63b6e7f)
uv run --no-sync python -m pytest tests/integration/observability/test_straiker_v3_platform.py::test_saved_api_version_v1_with_v3_key_routes_to_v3_not_the_v1_webhook -qFAILED ... 1 failed in 24.21s: the saved-v1 guardrail posted to/api/v1/detect/webhook, zero v3 calls at the sinkAfter (fcebfa9 plus this commit)
uv run --no-sync python -m pytest tests/integration/observability/test_straiker_v3_platform.py -q(run id ac42505a18e843c3bc37a225e3969aa6)36 passed in 78.75s, then a second run with the same selection36 passed in 66.44s, identical collected and passed sets, no skipsType
✅ Test
Link to Devin session: https://app.devin.ai/sessions/16f17e78d5b6462991838dcfb24e3249
Open in Devin Desktop: https://app.devin.ai/desktop/session/16f17e78d5b6462991838dcfb24e3249?variant=devin
Requested by: @yucheng-berri
Note
Low Risk
Test-only change; no production code or runtime behavior is modified.
Overview
Updates the Straiker integration test for the
straiker-v3-as-v1guardrail (sk_agt_key with savedapi_version: v1) so expectations match routing after #44011.Replaces
test_explicit_api_version_v1_overrides_key_prefix, which asserted the v1 webhook (litellmformat header), withtest_saved_api_version_v1_with_v3_key_routes_to_v3_not_the_v1_webhook. The new test checks allow and block paths: traffic goes to/api/v3/detect(including block verdict and no upstream call), and never to the v1 webhook for that key.Reviewed by Cursor Bugbot for commit eea7cdb. Bugbot is set up for automated code reviews on this repo. Configure here.