deleting providers should delete associated model configs

Author: nabinchha

src/data_designer/cli/controllers/provider_controller.py

@@ -5,7 +5,9 @@
 from pathlib import Path
 
 from data_designer.cli.forms.provider_builder import ProviderFormBuilder
+from data_designer.cli.repositories.model_repository import ModelRepository
 from data_designer.cli.repositories.provider_repository import ProviderRepository
+from data_designer.cli.services.model_service import ModelService
 from data_designer.cli.services.provider_service import ProviderService
 from data_designer.cli.ui import (
     confirm_action,
@@ -15,6 +17,7 @@
     print_header,
     print_info,
     print_success,
+    print_warning,
     select_with_arrows,
 )
 from data_designer.engine.model_provider import ModelProvider
@@ -27,6 +30,8 @@ def __init__(self, config_dir: Path):
         self.config_dir = config_dir
         self.repository = ProviderRepository(config_dir)
         self.service = ProviderService(self.repository)
+        self.model_repository = ModelRepository(config_dir)
+        self.model_service = ModelService(self.model_repository)
 
     def run(self) -> None:
         """Main entry point for provider configuration."""
@@ -182,14 +187,45 @@ def _handle_delete(self) -> None:
         if selected_name is None:
             return
 
+        # Check for associated models
+        associated_models = self.model_service.find_by_provider(selected_name)
+
         # Confirm deletion
         console.print()
-        if confirm_action(f"Delete provider '{selected_name}'?", default=False):
-            try:
-                self.service.delete(selected_name)
-                print_success(f"Provider '{selected_name}' deleted successfully")
-            except ValueError as e:
-                print_error(f"Failed to delete provider: {e}")
+
+        if associated_models:
+            # Notify user about associated models
+            model_count = len(associated_models)
+            model_aliases = ", ".join([f"'{m.alias}'" for m in associated_models])
+
+            print_warning(f"Provider '{selected_name}' has {model_count} associated model config(s): {model_aliases}")
+            console.print()
+
+            # Ask if user wants to delete provider and associated models
+            if confirm_action(
+                f"Delete provider '{selected_name}' and its {model_count} associated model config(s)?", default=False
+            ):
+                try:
+                    # Delete associated models first
+                    model_aliases_to_delete = [m.alias for m in associated_models]
+                    self.model_service.delete_by_aliases(model_aliases_to_delete)
+
+                    # Then delete the provider
+                    self.service.delete(selected_name)
+
+                    print_success(
+                        f"Provider '{selected_name}' and {model_count} associated model(s) deleted successfully"
+                    )
+                except ValueError as e:
+                    print_error(f"Failed to delete provider and associated models: {e}")
+        else:
+            # No associated models, proceed with simple deletion
+            if confirm_action(f"Delete provider '{selected_name}'?", default=False):
+                try:
+                    self.service.delete(selected_name)
+                    print_success(f"Provider '{selected_name}' deleted successfully")
+                except ValueError as e:
+                    print_error(f"Failed to delete provider: {e}")
 
     def _handle_delete_all(self) -> None:
         """Handle deleting all providers."""
@@ -198,20 +234,48 @@ def _handle_delete_all(self) -> None:
             print_error("No providers to delete")
             return
 
+        # Check for associated models across all providers
+        all_models = self.model_service.list_all()
+        provider_names_set = {p.name for p in providers}
+        associated_models = [m for m in all_models if m.provider in provider_names_set]
+
         # List providers to be deleted
         console.print()
         provider_count = len(providers)
         provider_names = ", ".join([f"'{p.name}'" for p in providers])
 
-        if confirm_action(
-            f"⚠️  Delete ALL ({provider_count}) provider(s): {provider_names}?\n   This action cannot be undone.",
-            default=False,
-        ):
-            try:
-                self.repository.delete()
-                print_success(f"All ({provider_count}) provider(s) deleted successfully")
-            except Exception as e:
-                print_error(f"Failed to delete all providers: {e}")
+        if associated_models:
+            model_count = len(associated_models)
+            print_warning(f"Deleting all providers will also affect {model_count} associated model config(s)")
+            console.print()
+
+            if confirm_action(
+                f"⚠️  Delete ALL ({provider_count}) provider(s): {provider_names} and {model_count} associated model(s)?\n   This action cannot be undone.",
+                default=False,
+            ):
+                try:
+                    # Delete all associated models first
+                    model_aliases_to_delete = [m.alias for m in associated_models]
+                    self.model_service.delete_by_aliases(model_aliases_to_delete)
+
+                    # Then delete all providers
+                    self.repository.delete()
+
+                    print_success(
+                        f"All ({provider_count}) provider(s) and {model_count} associated model(s) deleted successfully"
+                    )
+                except Exception as e:
+                    print_error(f"Failed to delete all providers and associated models: {e}")
+        else:
+            if confirm_action(
+                f"⚠️  Delete ALL ({provider_count}) provider(s): {provider_names}?\n   This action cannot be undone.",
+                default=False,
+            ):
+                try:
+                    self.repository.delete()
+                    print_success(f"All ({provider_count}) provider(s) deleted successfully")
+                except Exception as e:
+                    print_error(f"Failed to delete all providers: {e}")
 
     def _handle_change_default(self) -> None:
         """Handle changing the default provider."""

src/data_designer/cli/services/model_service.py

@@ -21,6 +21,18 @@ def get_by_alias(self, alias: str) -> ModelConfig | None:
         models = self.list_all()
         return next((m for m in models if m.alias == alias), None)
 
+    def find_by_provider(self, provider_name: str) -> list[ModelConfig]:
+        """Find all models associated with a provider.
+
+        Args:
+            provider_name: Name of the provider to search for
+
+        Returns:
+            List of models associated with the provider
+        """
+        models = self.list_all()
+        return [m for m in models if m.provider == provider_name]
+
     def add(self, model: ModelConfig) -> None:
         """Add a new model.
 
@@ -79,3 +91,26 @@ def delete(self, alias: str) -> None:
             self.repository.save(registry)
         else:
             self.repository.delete()
+
+    def delete_by_aliases(self, aliases: list[str]) -> None:
+        """Delete multiple models by their aliases.
+
+        Args:
+            aliases: List of model aliases to delete
+
+        Raises:
+            ValueError: If no models configured
+        """
+        if not aliases:
+            return
+
+        registry = self.repository.load()
+        if not registry:
+            raise ValueError("No models configured")
+
+        registry.model_configs = [m for m in registry.model_configs if m.alias not in aliases]
+
+        if registry.model_configs:
+            self.repository.save(registry)
+        else:
+            self.repository.delete()

tests/cli/services/test_model_service.py

@@ -35,3 +35,32 @@ def test_update(
 def test_delete(stub_model_service: ModelService, stub_model_configs: list[ModelConfig]):
     stub_model_service.delete("test-alias-1")
     assert stub_model_service.list_all() == stub_model_configs[1:]
+
+
+def test_find_by_provider(stub_model_service: ModelService, stub_model_configs: list[ModelConfig]):
+    # Both test models have provider="test-provider-1"
+    models = stub_model_service.find_by_provider("test-provider-1")
+    assert len(models) == 2
+    assert models == stub_model_configs
+
+    # Non-existent provider should return empty list
+    models = stub_model_service.find_by_provider("non-existent-provider")
+    assert models == []
+
+
+def test_delete_by_aliases(stub_model_service: ModelService, stub_model_configs: list[ModelConfig]):
+    # Delete both models
+    stub_model_service.delete_by_aliases(["test-alias-1", "test-alias-2"])
+    assert stub_model_service.list_all() == []
+
+
+def test_delete_by_aliases_partial(stub_model_service: ModelService, stub_model_configs: list[ModelConfig]):
+    # Delete only one model
+    stub_model_service.delete_by_aliases(["test-alias-1"])
+    assert stub_model_service.list_all() == stub_model_configs[1:]
+
+
+def test_delete_by_aliases_empty_list(stub_model_service: ModelService, stub_model_configs: list[ModelConfig]):
+    # Deleting empty list should do nothing
+    stub_model_service.delete_by_aliases([])
+    assert stub_model_service.list_all() == stub_model_configs

tests/cli/services/test_provider_service.py

@@ -1,8 +1,12 @@
 # SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 # SPDX-License-Identifier: Apache-2.0
 
+from pathlib import Path
+
+from data_designer.cli.repositories.model_repository import ModelConfigRegistry, ModelRepository
+from data_designer.cli.services.model_service import ModelService
 from data_designer.cli.services.provider_service import ProviderService
-from data_designer.config.models import ModelProvider
+from data_designer.config.models import ModelConfig, ModelProvider
 
 
 def test_list_all(stub_provider_service: ProviderService, stub_model_providers: list[ModelProvider]):
@@ -41,3 +45,44 @@ def test_set_default(stub_provider_service: ProviderService, stub_model_provider
 
 def test_get_default(stub_provider_service: ProviderService, stub_model_providers: list[ModelProvider]):
     assert stub_provider_service.get_default() == "test-provider-1"
+
+
+def test_delete_provider_with_associated_models(
+    tmp_path: Path, stub_model_providers: list[ModelProvider], stub_model_configs: list[ModelConfig]
+):
+    """Test integration: deleting a provider and its associated models."""
+    # Setup: Create provider and model services
+    provider_service = ProviderService(
+        ModelRepository(tmp_path)  # This should be ProviderRepository
+    )
+    model_service = ModelService(ModelRepository(tmp_path))
+
+    # Save provider and models
+    from data_designer.cli.repositories.provider_repository import ModelProviderRegistry, ProviderRepository
+
+    provider_repo = ProviderRepository(tmp_path)
+    provider_repo.save(ModelProviderRegistry(providers=stub_model_providers, default="test-provider-1"))
+
+    model_repo = ModelRepository(tmp_path)
+    model_repo.save(ModelConfigRegistry(model_configs=stub_model_configs))
+
+    # Verify initial state: 2 providers, 2 models (both associated with test-provider-1)
+    provider_service = ProviderService(provider_repo)
+    assert len(provider_service.list_all()) == 2
+    assert len(model_service.list_all()) == 2
+
+    # Find models associated with test-provider-1
+    associated_models = model_service.find_by_provider("test-provider-1")
+    assert len(associated_models) == 2
+
+    # Delete the associated models
+    model_aliases = [m.alias for m in associated_models]
+    model_service.delete_by_aliases(model_aliases)
+
+    # Delete the provider
+    provider_service.delete("test-provider-1")
+
+    # Verify final state: 1 provider, 0 models
+    assert len(provider_service.list_all()) == 1
+    assert len(model_service.list_all()) == 0
+    assert provider_service.get_by_name("test-provider-1") is None