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
This commit is contained in:
parent
b8203db380
commit
3c053c6eed
2 changed files with 28 additions and 6 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue