From a287d3b248ce6298d979c7da3a3fc9848033fc9c Mon Sep 17 00:00:00 2001 From: Luis Novo Date: Sat, 25 Oct 2025 08:48:18 -0300 Subject: [PATCH] refactor: optimize duplicate model validation and improve error handling (#219) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: prevent duplicate model names under same provider Implement case-insensitive validation to prevent users from creating duplicate model names under the same provider. This validation is implemented both in the backend API and the frontend UI. Changes: - Backend: Add duplicate check in create_model endpoint (case-insensitive) - Frontend: Add client-side validation in AddModelForm - Frontend: Improve error message display from backend - Tests: Add unit tests for duplicate model validation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * refactor: optimize duplicate model validation and improve error handling - Replace O(n) model iteration with efficient SurrealDB query for duplicate check - Improve error message to include model name and provider for better UX - Remove frontend duplicate validation (backend-only enforcement) - Fix test authentication by setting OPEN_NOTEBOOK_PASSWORD before imports - Update test mocking to use repo_query instead of Model.get_all() - Add pytest fixture for TestClient to ensure proper test isolation All 11 tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * remove unnecessary package * fix: replace any with unknown type in error handler - Change error type from 'any' to 'unknown' to satisfy ESLint - Add proper type assertion for error object structure - Maintains same runtime behavior with better type safety --------- Co-authored-by: Claude --- api/routers/models.py | 20 +++- .../models/components/AddModelForm.tsx | 13 ++- frontend/src/lib/hooks/use-models.ts | 5 +- pyproject.toml | 1 - tests/conftest.py | 10 +- tests/test_models_api.py | 95 ++++++++++++++++--- uv.lock | 26 +++-- 7 files changed, 130 insertions(+), 40 deletions(-) diff --git a/api/routers/models.py b/api/routers/models.py index ff2eb28..853dd98 100644 --- a/api/routers/models.py +++ b/api/routers/models.py @@ -67,17 +67,29 @@ async def create_model(model_data: ModelCreate): valid_types = ["language", "embedding", "text_to_speech", "speech_to_text"] if model_data.type not in valid_types: raise HTTPException( - status_code=400, + status_code=400, detail=f"Invalid model type. Must be one of: {valid_types}" ) - + + # Check for duplicate model name under the same provider (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()} + ) + if existing: + raise HTTPException( + status_code=400, + detail=f"Model '{model_data.name}' already exists for provider '{model_data.provider}'" + ) + new_model = Model( name=model_data.name, provider=model_data.provider, type=model_data.type, ) await new_model.save() - + return ModelResponse( id=new_model.id or "", name=new_model.name, @@ -86,6 +98,8 @@ async def create_model(model_data: ModelCreate): created=str(new_model.created), updated=str(new_model.updated), ) + except HTTPException: + raise except InvalidInputError as e: raise HTTPException(status_code=400, detail=str(e)) except Exception as e: diff --git a/frontend/src/app/(dashboard)/models/components/AddModelForm.tsx b/frontend/src/app/(dashboard)/models/components/AddModelForm.tsx index 0d2b21b..a287a21 100644 --- a/frontend/src/app/(dashboard)/models/components/AddModelForm.tsx +++ b/frontend/src/app/(dashboard)/models/components/AddModelForm.tsx @@ -26,7 +26,7 @@ export function AddModelForm({ modelType, providers }: AddModelFormProps) { }) // Get available providers that support this model type - const availableProviders = providers.available.filter(provider => + const availableProviders = providers.available.filter(provider => providers.supported_types[provider]?.includes(modelType) ) @@ -63,8 +63,15 @@ export function AddModelForm({ modelType, providers }: AddModelFormProps) { ) } + const handleOpenChange = (isOpen: boolean) => { + setOpen(isOpen) + if (!isOpen) { + reset() + } + } + return ( - +