mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(router): keep semantic auto-routers registered across delete/update
Deleting or updating an auto-router deployment left a stale entry in the router's per-strategy registries (auto_routers/complexity_routers/quality_routers/adaptive_routers). The subsequent re-add raised '<name> already exists', which is silently swallowed when ignore_invalid_deployments=True, dropping the deployment from model_list entirely. This is why semantic routers kept disappearing from the models tab and returned 'Invalid model name' on call. Deregister the strategy router whenever a deployment is removed so it can be cleanly re-added. Also require the embedding model in the auto-router create/edit UI, since the backend rejects semantic routers without one; previously the field was optional in the UI so routers created without it silently failed to initialize. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
cd6e8cdf23
commit
68ba8a7b99
4 changed files with 80 additions and 6 deletions
|
|
@ -8212,6 +8212,7 @@ class Router:
|
|||
self._invalidate_model_group_info_cache()
|
||||
self._invalidate_access_groups_cache()
|
||||
self._update_deployment_indices_after_removal(model_id=deployment_id, removal_idx=removal_idx)
|
||||
self._deregister_strategy_router(model_name=deployment.model_name)
|
||||
|
||||
# if the model_id is not in router
|
||||
self.add_deployment(deployment=deployment)
|
||||
|
|
@ -8225,6 +8226,18 @@ class Router:
|
|||
else:
|
||||
raise e
|
||||
|
||||
def _deregister_strategy_router(self, model_name: Optional[str]) -> None:
|
||||
"""Drop any per-strategy router (auto/complexity/quality/adaptive) registered under
|
||||
model_name. Called whenever a deployment is removed so the same model_name can be
|
||||
cleanly re-added; otherwise re-adding raises "<name> already exists" (silently
|
||||
swallowed when ignore_invalid_deployments=True), dropping it from model_list."""
|
||||
if model_name is None:
|
||||
return
|
||||
self.auto_routers.pop(model_name, None)
|
||||
self.complexity_routers.pop(model_name, None)
|
||||
self.quality_routers.pop(model_name, None)
|
||||
self.adaptive_routers.pop(model_name, None)
|
||||
|
||||
def delete_deployment(self, id: str) -> Optional[Deployment]:
|
||||
"""
|
||||
Parameters:
|
||||
|
|
@ -8245,6 +8258,7 @@ class Router:
|
|||
self._invalidate_model_group_info_cache()
|
||||
self._invalidate_access_groups_cache()
|
||||
self._update_deployment_indices_after_removal(model_id=id, removal_idx=deployment_idx)
|
||||
self._deregister_strategy_router(model_name=item.get("model_name"))
|
||||
_budget_limiter = self._get_router_deployment_budget_limiter()
|
||||
if _budget_limiter is not None:
|
||||
_budget_limiter.unregister_deployment_budget(model_id=id)
|
||||
|
|
|
|||
|
|
@ -1854,6 +1854,57 @@ def test_init_auto_router_deployment_duplicate_model_name(mock_auto_router, mode
|
|||
router.init_auto_router_deployment(deployment)
|
||||
|
||||
|
||||
def _auto_router_deployment(config="/path/to/config", model_id="ar-id"):
|
||||
return Deployment(
|
||||
model_name="my-auto-router",
|
||||
litellm_params=LiteLLM_Params(
|
||||
model="auto_router/my-auto-router",
|
||||
auto_router_config_path=config,
|
||||
auto_router_default_model="gpt-5-mini",
|
||||
auto_router_embedding_model="text-embedding-3-small",
|
||||
),
|
||||
model_info=ModelInfo(id=model_id, db_model=True),
|
||||
)
|
||||
|
||||
|
||||
@patch("litellm.router_strategy.auto_router.auto_router.AutoRouter")
|
||||
def test_delete_deployment_deregisters_auto_router(mock_auto_router, model_list):
|
||||
"""Regression: deleting an auto-router deployment must drop it from ``auto_routers`` so the
|
||||
same model_name can be re-added. Previously the stale entry made the re-add raise
|
||||
'already exists', which (with ignore_invalid_deployments) silently dropped the deployment."""
|
||||
mock_auto_router.return_value = MagicMock()
|
||||
router = Router(model_list=model_list, ignore_invalid_deployments=True)
|
||||
|
||||
router.add_deployment(deployment=_auto_router_deployment())
|
||||
assert "my-auto-router" in router.auto_routers
|
||||
assert router.has_model_id("ar-id")
|
||||
|
||||
router.delete_deployment(id="ar-id")
|
||||
assert "my-auto-router" not in router.auto_routers
|
||||
|
||||
router.add_deployment(deployment=_auto_router_deployment())
|
||||
assert router.has_model_id("ar-id")
|
||||
assert "my-auto-router" in router.auto_routers
|
||||
assert any(m.get("model_name") == "my-auto-router" for m in router.model_list)
|
||||
|
||||
|
||||
@patch("litellm.router_strategy.auto_router.auto_router.AutoRouter")
|
||||
def test_upsert_auto_router_deployment_update_keeps_it_registered(mock_auto_router, model_list):
|
||||
"""Regression for the semantic router 'keeps disappearing' bug: upserting an auto-router
|
||||
deployment with changed params (as the periodic DB sync does) must keep it in ``model_list``
|
||||
and ``auto_routers`` instead of silently dropping it via an 'already exists' error."""
|
||||
mock_auto_router.return_value = MagicMock()
|
||||
router = Router(model_list=model_list, ignore_invalid_deployments=True)
|
||||
|
||||
router.upsert_deployment(deployment=_auto_router_deployment(config="/path/v1"))
|
||||
assert router.has_model_id("ar-id")
|
||||
|
||||
router.upsert_deployment(deployment=_auto_router_deployment(config="/path/v2"))
|
||||
assert router.has_model_id("ar-id")
|
||||
assert "my-auto-router" in router.auto_routers
|
||||
assert any(m.get("model_name") == "my-auto-router" for m in router.model_list)
|
||||
|
||||
|
||||
def test_generate_model_id_with_deployment_model_name(model_list):
|
||||
"""Test that _generate_model_id works correctly with deployment model_name and handles None values properly"""
|
||||
router = Router(model_list=model_list)
|
||||
|
|
|
|||
|
|
@ -147,6 +147,11 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({ form, handleOk, acc
|
|||
return;
|
||||
}
|
||||
|
||||
if (!currentFormValues.auto_router_embedding_model && !currentFormValues.custom_embedding_model) {
|
||||
NotificationManager.fromBackend("Please select an Embedding Model");
|
||||
return;
|
||||
}
|
||||
|
||||
form.setFieldsValue({
|
||||
custom_llm_provider: "auto_router",
|
||||
model: currentFormValues.auto_router_name,
|
||||
|
|
@ -330,15 +335,16 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({ form, handleOk, acc
|
|||
|
||||
{/* Auto Router Embedding Model */}
|
||||
<Form.Item
|
||||
rules={[{ required: routerType === "semantic", message: "Embedding model is required" }]}
|
||||
label="Embedding Model"
|
||||
name="auto_router_embedding_model"
|
||||
tooltip="Optional: Embedding model to use for semantic routing decisions"
|
||||
tooltip="Embedding model used to compute semantic similarity for routing decisions"
|
||||
labelCol={{ span: 10 }}
|
||||
labelAlign="left"
|
||||
>
|
||||
<AntdSelect
|
||||
value={form.getFieldValue("auto_router_embedding_model")}
|
||||
placeholder="Select an embedding model (optional)"
|
||||
placeholder="Select an embedding model"
|
||||
onChange={(value) => {
|
||||
setShowCustomEmbeddingModel(value === "custom");
|
||||
form.setFieldValue("auto_router_embedding_model", value);
|
||||
|
|
|
|||
|
|
@ -106,7 +106,7 @@ const EditAutoRouterModal: React.FC<EditAutoRouterModalProps> = ({
|
|||
...modelData.litellm_params,
|
||||
auto_router_config: JSON.stringify(routerConfig),
|
||||
auto_router_default_model: values.auto_router_default_model,
|
||||
auto_router_embedding_model: values.auto_router_embedding_model || undefined,
|
||||
auto_router_embedding_model: values.auto_router_embedding_model,
|
||||
};
|
||||
|
||||
// Prepare updated model_info
|
||||
|
|
@ -205,15 +205,18 @@ const EditAutoRouterModal: React.FC<EditAutoRouterModalProps> = ({
|
|||
</Form.Item>
|
||||
|
||||
{/* Embedding Model */}
|
||||
<Form.Item label="Embedding Model" name="auto_router_embedding_model">
|
||||
<Form.Item
|
||||
label="Embedding Model"
|
||||
name="auto_router_embedding_model"
|
||||
rules={[{ required: true, message: "Embedding model is required" }]}
|
||||
>
|
||||
<AntdSelect
|
||||
placeholder="Select an embedding model (optional)"
|
||||
placeholder="Select an embedding model"
|
||||
onChange={(value) => {
|
||||
setShowCustomEmbeddingModel(value === "custom");
|
||||
}}
|
||||
options={[...modelOptions, { value: "custom", label: "Enter custom model name" }]}
|
||||
showSearch={true}
|
||||
allowClear
|
||||
/>
|
||||
</Form.Item>
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue