Address all Copilot review comments on PR #68 (#69)

- Fix ChromaDB persist() compatibility: wrap in try/except for 0.4.0+
  which auto-persists with persist_directory
- Replace fragile error string matching with specific exception types
  (ImportError, ConnectionError, ValueError, etc.)
- Fix register_test decorator to return cls (was returning None)
- Fix getter/setter inconsistency: embedding_provider and
  embedding_model setters now accept empty values matching getter defaults
- Fix empty base_url passthrough: empty strings are now stripped from
  kwargs instead of passed to model constructors
- Remove unused client variable assignments in test_chat_clients.py
- Reorganize tests: move AppConfig, helper, and TestStatus tests out of
  test_is_response_list.py into dedicated test files

All 93 tests pass.

https://claude.ai/code/session_01CDFqeg5QhB4V7yQ3yVVBc9

Co-authored-by: Claude <noreply@anthropic.com>

Author: davida-ps

ps_fuzz/app_config.py

@@ -210,8 +210,7 @@ def embedding_provider(self) -> str:
 
     @embedding_provider.setter
     def embedding_provider(self, value: str):
-        if not value: raise ValueError("Embedding provider cannot be empty")
-        self.config_state['embedding_provider'] = value
+        self.config_state['embedding_provider'] = value if value else ''
         self.save()
 
     @property
@@ -238,8 +237,7 @@ def embedding_model(self) -> str:
 
     @embedding_model.setter
     def embedding_model(self, value: str):
-        if not value: raise ValueError("Embedding model cannot be empty")
-        self.config_state['embedding_model'] = value
+        self.config_state['embedding_model'] = value if value else ''
         self.save()
 
     def update_from_args(self, args):

ps_fuzz/attack_registry.py

@@ -14,6 +14,7 @@ def register_test(cls):
     global test_classes
     logger.debug(f"Registering attack test class: {cls.__name__}")
     test_classes.append(cls)
+    return cls
 
 def instantiate_tests(client_config: ClientConfig, attack_config:AttackConfig, custom_tests:List=None, custom_benchmark:bool=False) -> List[TestBase]:
     tests = []

ps_fuzz/attacks/rag_poisoning.py

@@ -255,8 +255,11 @@ def _setup_poisoned_vector_database(self):
         with _suppress_loggers(suppress_names):
             self.vectorstore.add_documents([poisoned_doc])
 
-        # Persist the database
-        self.vectorstore.persist()
+        # Persist the database (ChromaDB 0.4.0+ auto-persists with persist_directory)
+        try:
+            self.vectorstore.persist()
+        except AttributeError:
+            pass  # ChromaDB 0.4.0+ auto-persists when using persist_directory
 
     def _cleanup(self):
         """Clean up temporary resources"""
@@ -362,22 +365,27 @@ def run(self) -> Generator[StatusUpdate, None, None]:
             # Final status update
             yield StatusUpdate(self.client_config, self.test_name, self.status, "Completed", len(test_queries)+1, len(test_queries)+1)
 
+        except (ImportError, ModuleNotFoundError) as e:
+            # Missing dependencies — report as skipped
+            logger.warning(f"RAG poisoning attack skipped: {e}")
+            self.status.report_skipped("", f"Setup error - missing dependency: {e}")
+            yield StatusUpdate(self.client_config, self.test_name, self.status, "Skipped", 1, 1)
+        except (ConnectionError, ConnectionRefusedError, OSError) as e:
+            # Network/connectivity issues — report as skipped
+            logger.warning(f"RAG poisoning attack skipped: {e}")
+            self.status.report_skipped("", f"Setup error - connectivity issue: {e}")
+            yield StatusUpdate(self.client_config, self.test_name, self.status, "Skipped", 1, 1)
+        except ValueError as e:
+            # Configuration errors (e.g. unsupported provider, invalid URL)
+            logger.warning(f"RAG poisoning attack skipped: {e}")
+            self.status.report_skipped("", f"Setup error - configuration issue: {e}")
+            yield StatusUpdate(self.client_config, self.test_name, self.status, "Skipped", 1, 1)
         except Exception as e:
-            # Check if this is a setup/configuration-related error that should be skipped
-            error_str = str(e).lower()
-            if ("chromadb" in error_str or
-                "could not import" in error_str or
-                "no module named" in error_str or
-                ("model" in error_str and ("not found" in error_str or "try pulling" in error_str)) or
-                "http code: 404" in error_str or
-                ("embedding" in error_str and ("not available" in error_str or "not found" in error_str)) or
-                "invalid url" in error_str or
-                "no host supplied" in error_str or
-                ("connection" in error_str and ("refused" in error_str or "failed" in error_str or "error" in error_str)) or
-                "inference endpoint" in error_str):
-                # This is a setup/configuration issue, report as skipped
+            # Check for HTTP errors and known setup issues by exception type
+            error_type = type(e).__name__
+            if error_type in ("HTTPError", "HTTPStatusError", "RequestError", "ConnectError", "TimeoutException"):
                 logger.warning(f"RAG poisoning attack skipped: {e}")
-                self.status.report_skipped("", f"Setup error - embedding service configuration or connectivity issue: {e}")
+                self.status.report_skipped("", f"Setup error - HTTP/connection issue: {e}")
                 yield StatusUpdate(self.client_config, self.test_name, self.status, "Skipped", 1, 1)
             else:
                 # This is a real runtime error during attack execution, report as error

ps_fuzz/chat_clients.py

@@ -34,17 +34,17 @@ def __init__(self, backend: str, **kwargs):
         if backend in chat_models_info:
             model_cls = chat_models_info[backend].model_cls
 
-            # Special handling for providers that need base_url
-            if backend == 'ollama' and 'ollama_base_url' in kwargs and kwargs['ollama_base_url']:
-                # Use the ollama_base_url parameter but rename it to base_url for the Ollama client
-                base_url = kwargs.pop('ollama_base_url')
-                kwargs['base_url'] = base_url
-            
-            # Special handling for OpenAI base_url
-            if backend == 'open_ai' and 'openai_base_url' in kwargs and kwargs['openai_base_url']:
-                # Use the openai_base_url parameter but rename it to base_url for the OpenAI client
-                base_url = kwargs.pop('openai_base_url')
-                kwargs['base_url'] = base_url
+            # Transform provider-specific base_url params to the generic base_url,
+            # and remove empty values so they don't get passed to the model constructor.
+            if backend == 'ollama' and 'ollama_base_url' in kwargs:
+                url = kwargs.pop('ollama_base_url')
+                if url:
+                    kwargs['base_url'] = url
+
+            if backend == 'open_ai' and 'openai_base_url' in kwargs:
+                url = kwargs.pop('openai_base_url')
+                if url:
+                    kwargs['base_url'] = url
 
             self.client = model_cls(**kwargs)
         else: