Skip to content

fix(router): register model info under responses/-stripped variant - #27531

Merged
oss-pr-review-agent-shin[bot] merged 2 commits into
BerriAI:shin_agent_oss_staging_05_09_2026from
qiniu:fix/router-register-model-cost-responses-stripped
May 9, 2026
Merged

fix(router): register model info under responses/-stripped variant#27531
oss-pr-review-agent-shin[bot] merged 2 commits into
BerriAI:shin_agent_oss_staging_05_09_2026from
qiniu:fix/router-register-model-cost-responses-stripped

Conversation

@krisxia0506

Copy link
Copy Markdown
Contributor

Relevant issues

When the Router registers a deployment into litellm.model_cost, the downstream responses_api_bridge_check() strips the responses/ prefix from the model name. If a deployment is only registered under azure/responses/..., lookups for azure/... miss, which can affect flags like supports_native_streaming (e.g. falsely triggering fake streaming).

Linear ticket

N/A

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Screenshots / Proof of Fix

Unit tests covering both registration paths:

  • test_responses_prefix_stripped_alias_registered_for_model_list
  • test_responses_prefix_stripped_alias_registered_for_add_deployment
proof-of-fix

Type

🐛 Bug Fix
✅ Test

Changes

Bug Fix

  • _create_deployment: after registering the shared backend key, if the deployment's litellm_params.model contains responses/, additionally register an alias with that prefix stripped, so post-bridge lookups resolve correctly.
  • add_deployment: on top of the existing model_id-level custom pricing registration, mirror the OLD MODEL REGISTRATION and the stripped alias from _create_deployment, so runtime-added deployments match the behavior of YAML-bootstrapped ones.

Affected files

  • litellm/router.py
  • tests/test_litellm/test_router_model_cost_isolation.py

yuneng-berri and others added 2 commits May 7, 2026 18:05
- _create_deployment: register shared model_info under stripped backend key when
  litellm_params.model contains responses/ (matches post-bridge lookup names).
- add_deployment: mirror OLD MODEL REGISTRATION + stripped alias inside model_id
  block so runtime-loaded deployments match YAML bootstrap.
- Tests for model_list init and add_deployment paths.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codspeed-hq

codspeed-hq Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing qiniu:fix/router-register-model-cost-responses-stripped (73eefc8) with main (fa81017)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a lookup miss in litellm.model_cost for deployments whose litellm_params.model starts with "responses/": because responses_api_bridge_check() strips that prefix before performing model-info lookups, only registering azure/responses/gpt-4o left azure/gpt-4o unresolvable, which could falsely suppress flags like supports_native_streaming.

  • _create_deployment: after building the shared backend alias in litellm.model_cost, a second entry without the responses/ segment is now also registered.
  • add_deployment: mirrors the same dual-registration so dynamically added deployments (e.g. from DB) match the YAML-bootstrapped path; _model_info_dict construction is moved up to be shared between both registration calls.
  • Tests: two new mock-only tests cover both registration paths, each verifying both the prefixed and stripped keys exist in litellm.model_cost with the expected field values.

Confidence Score: 4/5

Safe to merge; the fix is narrow and well-tested, with only a minor guard condition that could be tightened.

The core logic is correct and both changed code paths are covered by the new tests. The only concern is the guard "responses/" in _model_name operating on the fully-constructed provider/model string instead of the raw litellm_params.model field — a model whose name incidentally contains "responses/" mid-string would produce a spurious stripped alias in litellm.model_cost. This is an edge case in practice but the fix (using startswith on the raw field and capping replace to count=1) is straightforward.

The two alias-registration blocks in litellm/router.py (around lines 7079–7083 and 7787–7791) both share the same broad guard condition and are worth a second look before merging.

Important Files Changed

Filename Overview
litellm/router.py Registers a stripped alias (without responses/ prefix) in litellm.model_cost for both _create_deployment and add_deployment. The guard condition "responses/" in _model_name is broader than intended — it operates on the fully-formed provider/model string rather than on litellm_params.model directly, which could cause false-positive alias creation for non-responses-API models that happen to contain that substring.
tests/test_litellm/test_router_model_cost_isolation.py Adds two new unit tests verifying that the stripped alias is registered for both model-list and add_deployment paths. Tests use fake API keys and only inspect litellm.model_cost in memory — no real network calls are made, consistent with the mock-only rule for this test directory.

Reviews (1): Last reviewed commit: "fix(router): register model info under r..." | Re-trigger Greptile

Comment thread litellm/router.py
Comment on lines +7079 to 7085
_backend_alias_cost = {_model_name: _shared_model_info}
if "responses/" in _model_name:
_stripped_model_name = _model_name.replace("responses/", "")
_backend_alias_cost[_stripped_model_name] = _shared_model_info
litellm.register_model(model_cost=_backend_alias_cost)

## Check if LLM Deployment is allowed for this deployment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The check "responses/" in _model_name operates on the fully-constructed provider/model string, so a model named something like azure/custom-responses/v2 (where "responses/" is not a prefix of the model field) would also trigger the alias stripping and produce azure/custom-v2 — a potentially incorrect or conflicting key. A tighter guard that checks only the litellm_params.model field (before prepending the provider) avoids this false-positive. str.replace without count=1 would also replace multiple occurrences of "responses/" if they exist.

Suggested change
_backend_alias_cost = {_model_name: _shared_model_info}
if "responses/" in _model_name:
_stripped_model_name = _model_name.replace("responses/", "")
_backend_alias_cost[_stripped_model_name] = _shared_model_info
litellm.register_model(model_cost=_backend_alias_cost)
## Check if LLM Deployment is allowed for this deployment
_backend_alias_cost = {_model_name: _shared_model_info}
if deployment.litellm_params.model.startswith("responses/"):
_stripped_model_name = _model_name.replace("responses/", "", 1)
_backend_alias_cost[_stripped_model_name] = _shared_model_info
litellm.register_model(model_cost=_backend_alias_cost)
## Check if LLM Deployment is allowed for this deployment

Comment thread litellm/router.py
Comment on lines +7787 to 7793
_backend_alias_cost = {_model_name: _shared_model_info}
if "responses/" in _model_name:
_stripped_model_name = _model_name.replace("responses/", "")
_backend_alias_cost[_stripped_model_name] = _shared_model_info
litellm.register_model(model_cost=_backend_alias_cost)

# add to model names

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Same overly-broad guard in add_deployment: "responses/" in _model_name can match a model whose name happens to contain that substring in a non-prefix position. Checking deployment.litellm_params.model.startswith("responses/") is the precise equivalent of what responses_api_bridge_check actually tests, and passing count=1 to replace keeps the replacement bounded.

Suggested change
_backend_alias_cost = {_model_name: _shared_model_info}
if "responses/" in _model_name:
_stripped_model_name = _model_name.replace("responses/", "")
_backend_alias_cost[_stripped_model_name] = _shared_model_info
litellm.register_model(model_cost=_backend_alias_cost)
# add to model names
_backend_alias_cost = {_model_name: _shared_model_info}
if deployment.litellm_params.model.startswith("responses/"):
_stripped_model_name = _model_name.replace("responses/", "", 1)
_backend_alias_cost[_stripped_model_name] = _shared_model_info
litellm.register_model(model_cost=_backend_alias_cost)
# add to model names

@codecov

codecov Bot commented May 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@oss-pr-review-agent-shin
oss-pr-review-agent-shin Bot changed the base branch from main to shin_agent_oss_staging_05_09_2026 May 9, 2026 20:30
@oss-pr-review-agent-shin
oss-pr-review-agent-shin Bot merged commit d1d2400 into BerriAI:shin_agent_oss_staging_05_09_2026 May 9, 2026
50 of 51 checks passed
@oss-pr-review-agent-shin

Copy link
Copy Markdown
Contributor

🤖 litellm-agent: Squash-merged into staging branch shin_agent_oss_staging_05_09_2026. Staging PR: #27549


Triage Summary
Gathered PR data only — the triage LLM step did not produce a valid report, so failing-check classification and prior-signal reconciliation were skipped. 101 line(s) across 2 file(s) (+91/-10).

101 lines across 2 files (+91 / -10)

Merge Confidence: 5/5 ✅ READY
Ready to ship.

All checks green. Greptile 4/5, no blocking pattern findings, no CircleCI runs (OSS-typical).

fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants