fix(gemini): avoid duplicate model route for full api_base#26980
fix(gemini): avoid duplicate model route for full api_base#26980StatPan wants to merge 6 commits into
Conversation
…ags. Add tests to validate the condition improve the conditional readability by naming the plain-tag check explicitly.
…/litellm into litellm_auth_bypass_tag_based_routing
…_based_routing add test(tag-routing): prevent header regex bypass for strict plain t…
|
|
|
Note: the initial branch-guard check is failing because this repo currently rejects external fork PRs to I'm leaving the PR open since the code/tests are here and the rest of CI is running, but if maintainers want this retargeted another way I can adjust quickly. |
|
For clarity on the current failing checks:
These two failures do not appear to be caused by the Gemini fix itself. I'm watching the provider test jobs separately for actual code-level regressions. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a duplicate-route bug in Confidence Score: 4/5Safe to merge; the core Gemini URL-construction fix is correct and well-covered by new tests. No P0/P1 findings. One P2 edge case in the URL-construction logic and one P2 noting the tag-routing change is functionally a no-op. Both are non-blocking. litellm/llms/vertex_ai/vertex_llm_base.py — edge case where
|
| Filename | Overview |
|---|---|
| litellm/llms/vertex_ai/vertex_llm_base.py | Core bug fix: adds three-way branching to avoid double-appending /models/{model}:{endpoint} when api_base already carries the full route. One unhandled edge case remains: an api_base ending with /models/{model}:{different_endpoint} falls through to the else branch and produces a broken double-path URL. |
| litellm/router_strategy/tag_based_routing.py | Cosmetic refactor: replaces bool(deployment_tags) with an explicit is not None and len(...) > 0 guard. Functionally identical to the original for all valid deployment_tags values. |
| tests/test_litellm/llms/vertex_ai/test_vertex_llm_base.py | Adds three parametrized cases covering full-route, model-only, and plain-base api_base inputs; strengthens an existing test by replacing a print with a proper assert; removes unused imports. |
| tests/test_litellm/router_strategy/test_router_tag_routing.py | Adds a new test verifying that strict-tag mode (match_any=False) blocks regex-based routing when request_tags is None. Correct and well-named, though the underlying production code change is a no-op. |
Reviews (1): Last reviewed commit: "test(gemini): cover full-route custom ap..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if api_base.endswith(f"/models/{model}:{endpoint}"): | ||
| url = api_base | ||
| elif api_base.endswith(f"/models/{model}"): | ||
| url = f"{api_base}:{endpoint}" | ||
| else: | ||
| url = "{}/models/{}:{}".format( | ||
| api_base.rstrip("/"), model, endpoint | ||
| ) |
There was a problem hiding this comment.
api_base ends with a different endpoint suffix
If api_base already contains a full route but with a different endpoint — e.g. https://proxy.example.com/.../models/gemini-2.5-flash-lite:streamGenerateContent while the current call has endpoint="generateContent" — neither condition 1 nor 2 matches. The else branch then appends /models/gemini-2.5-flash-lite:generateContent to the existing full URL, producing a doubly-routed, broken URL. Admittedly an unlikely proxy configuration, but it may silently produce a 404 instead of a clear error.
| deployment_has_plain_tags = deployment_tags is not None and len(deployment_tags) > 0 | ||
| strict_tag_check_failed = not match_any and deployment_has_plain_tags |
There was a problem hiding this comment.
bool(deployment_tags) and deployment_tags is not None and len(deployment_tags) > 0 produce identical results for all possible values of deployment_tags (None, [], or a non-empty list). The accompanying test test_strict_tag_routing_without_request_tags_blocks_header_regex_fallback also passes against the original code, so this change does not fix any behavioral bug — it only clarifies intent. If not intentional, the PR description is slightly misleading.
|
I found and fixed the Vertex AI test failure. Root cause: while cleaning up the test file, I accidentally changed the expected value in I've pushed a follow-up commit restoring the original expectation and re-ran the affected tests locally:
|
|
Documenting the remaining blocker clearly: This PR is now down to a single failing required check: Root cause is repository contribution routing policy, not the Gemini fix itself:
The workflow only allows PRs into So at this point the code/tests are green; the remaining blocker is that this change likely needs to be retargeted through the repo's OSS staging flow rather than submitted directly to |
|
I've re-opened the same fix through the OSS staging path here: #26982 Reason for replacement: #26980 is code/test green but blocked by the repo's Keeping #26980 as the routing/reference record and moving the actionable merge path to #26982 ( |
4 participants
Summary
/models/{model}:{endpoint}route when Geminiapi_basealready includes the full routeapi_basevalues that already end with/models/{model}by appending only:{endpoint}api_basecasesContext
This is a follow-up/replacement for #24786, which has been inactive since 2026-03-30.
The bug is still affecting downstream integrations in OpenHands:
Tracking issue:
Test Plan
uv run pytest tests/test_litellm/llms/vertex_ai/test_vertex_llm_base.py -k 'custom_proxy or full_route or api_base'uv run ruff check litellm/llms/vertex_ai/vertex_llm_base.py tests/test_litellm/llms/vertex_ai/test_vertex_llm_base.py