From 3c053c6eedb10306b3fb1d0f4819a345913cdb66 Mon Sep 17 00:00:00 2001 From: LUIS NOVO Date: Fri, 9 Jan 2026 20:00:20 -0300 Subject: [PATCH] fix: include type in model duplicate check The model uniqueness constraint now considers (provider, name, type) instead of just (provider, name). This allows users to add the same model name for different purposes (e.g., language vs embedding). Fixes #391 --- api/routers/models.py | 8 ++++---- tests/test_models_api.py | 26 ++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/api/routers/models.py b/api/routers/models.py index 7b2b31c..6574dac 100644 --- a/api/routers/models.py +++ b/api/routers/models.py @@ -98,16 +98,16 @@ async def create_model(model_data: ModelCreate): detail=f"Invalid model type. Must be one of: {valid_types}" ) - # Check for duplicate model name under the same provider (case-insensitive) + # Check for duplicate model name under the same provider and type (case-insensitive) from open_notebook.database.repository import repo_query existing = await repo_query( - "SELECT * FROM model WHERE string::lowercase(provider) = $provider AND string::lowercase(name) = $name LIMIT 1", - {"provider": model_data.provider.lower(), "name": model_data.name.lower()} + "SELECT * FROM model WHERE string::lowercase(provider) = $provider AND string::lowercase(name) = $name AND string::lowercase(type) = $type LIMIT 1", + {"provider": model_data.provider.lower(), "name": model_data.name.lower(), "type": model_data.type.lower()} ) if existing: raise HTTPException( status_code=400, - detail=f"Model '{model_data.name}' already exists for provider '{model_data.provider}'" + detail=f"Model '{model_data.name}' already exists for provider '{model_data.provider}' with type '{model_data.type}'" ) new_model = Model( diff --git a/tests/test_models_api.py b/tests/test_models_api.py index 60319ce..b0ef456 100644 --- a/tests/test_models_api.py +++ b/tests/test_models_api.py @@ -41,7 +41,7 @@ class TestModelCreation: assert response.status_code == 400 assert ( response.json()["detail"] - == "Model 'gpt-4' already exists for provider 'openai'" + == "Model 'gpt-4' already exists for provider 'openai' with type 'language'" ) @pytest.mark.asyncio @@ -70,7 +70,7 @@ class TestModelCreation: assert response.status_code == 400 assert ( response.json()["detail"] - == "Model 'GPT-4' already exists for provider 'OpenAI'" + == "Model 'GPT-4' already exists for provider 'OpenAI' with type 'language'" ) @pytest.mark.asyncio @@ -95,6 +95,28 @@ class TestModelCreation: # Should succeed because provider is different assert response.status_code == 200 + @pytest.mark.asyncio + @patch("open_notebook.database.repository.repo_query") + async def test_create_same_model_name_different_type( + self, mock_repo_query, client + ): + """Test that creating a model with same name but different type is allowed.""" + from open_notebook.ai.models import Model + + # Mock repo_query to return empty (no duplicate found for different type) + mock_repo_query.return_value = [] + + # Patch the save method on the Model class + with patch.object(Model, "save", new_callable=AsyncMock) as mock_save: + # Attempt to create same model name with different type (embedding instead of language) + response = client.post( + "/api/models", + json={"name": "gpt-4", "provider": "openai", "type": "embedding"}, + ) + + # Should succeed because type is different + assert response.status_code == 200 + class TestModelsProviderAvailability: """Test suite for Models Provider Availability endpoint."""