Address PR feedback: Add _is_gemini_model helper, improve error handling, and add comprehensive tests

Co-Authored-By: Joe Moura <joao@crewai.com>

src/crewai/agents/crew_agent_executor.py

@@ -216,14 +216,20 @@ class CrewAgentExecutor(CrewAgentExecutorMixin):
             raise e
 
         if answer is None:
+            error_msg = "Invalid response from LLM call - None response received"
             self._printer.print(
-                content="Received None response from LLM call.",
+                content=error_msg,
                 color="red",
             )
-            raise ValueError("Invalid response from LLM call - None.")
+            raise ValueError(error_msg)
             
         # Empty string responses are allowed for Gemini models with HTML templates
         # They will be handled at the LLM class level
+        if answer == "":
+            self._printer.print(
+                content="Received empty string response - checking if using Gemini with HTML templates",
+                color="yellow"
+            )
 
         return answer
 

src/crewai/llm.py

@@ -215,6 +215,7 @@ class LLM:
         self.additional_params = kwargs
         self.is_anthropic = self._is_anthropic_model(model)
         self.stream = stream
+        self.logger = logging.getLogger(__name__)
 
         litellm.drop_params = True
 
@@ -240,6 +241,15 @@ class LLM:
         """
         ANTHROPIC_PREFIXES = ("anthropic/", "claude-", "claude/")
         return any(prefix in model.lower() for prefix in ANTHROPIC_PREFIXES)
+        
+    def _is_gemini_model(self) -> bool:
+        """Helper to check if current model is based on Gemini.
+        
+        Returns:
+            bool: True if the model is Gemini or using OpenRouter, False otherwise.
+        """
+        model_name = str(self.model or "").lower()
+        return "gemini" in model_name or "openrouter" in str(self.base_url or self.api_base or "").lower()
 
     def _prepare_completion_params(
         self,
@@ -583,9 +593,10 @@ class LLM:
         # --- 2.1) Special handling for Gemini models that might return empty content
         # For OpenRouter with Gemini models, sometimes valid responses have empty content
         # when HTML templates are used, but the response object is still valid
-        if text_response == "" and self.model and ("gemini" in self.model.lower() or "openrouter" in str(self.base_url or self.api_base or "").lower()):
+        if text_response == "" and self._is_gemini_model():
             # Instead of rejecting empty responses for Gemini, return a placeholder
-            text_response = "Response processed successfully. Please check your HTML template if you expected different content."
+            self.logger.warning("Empty content received from Gemini model with HTML template")
+            text_response = "Response processed successfully. Empty content received - this is expected behavior when using HTML templates with Gemini models."
 
         # --- 3) Handle callbacks with usage info
         if callbacks and len(callbacks) > 0:

tests/test_gemini_html_template.py

@@ -51,3 +51,79 @@ def test_openrouter_gemini_empty_response_handling():
         # Verify that our fix works - empty string should be replaced with placeholder
         assert "Response processed successfully" in result
         assert "HTML template" in result
+
+
+def test_gemini_none_response_handling():
+    """Test that None responses are properly handled."""
+    llm = LLM(model="gemini/gemini-pro")
+    
+    # Create a mock response with None content
+    mock_response = MagicMock()
+    mock_response.choices = [MagicMock()]
+    mock_response.choices[0].message = MagicMock()
+    mock_response.choices[0].message.content = None
+    
+    # Mock litellm.completion to return our mock response
+    with patch('litellm.completion', return_value=mock_response):
+        # Call the non-streaming response handler directly
+        # None content should be converted to empty string and then handled
+        result = llm._handle_non_streaming_response({"model": "gemini/gemini-pro"})
+        
+        # Verify that our fix works - None should be converted to empty string
+        # and then handled as an empty string for Gemini models
+        assert "Response processed successfully" in result
+        assert "HTML template" in result
+
+
+@pytest.mark.parametrize("model_name,base_url", [
+    ("gemini/gemini-pro", None),
+    ("gemini-pro", None),
+    ("google/gemini-pro", None),
+    ("openrouter/google/gemini-pro", "https://openrouter.ai/api/v1"),
+    ("openrouter/gemini-pro", "https://openrouter.ai/api/v1"),
+])
+def test_various_gemini_configurations(model_name, base_url):
+    """Test different Gemini model configurations with the _is_gemini_model helper."""
+    # Create a mock LLM instance with the specified model and base URL
+    llm = LLM(model=model_name, base_url=base_url)
+    
+    # Verify that _is_gemini_model correctly identifies all these configurations
+    assert llm._is_gemini_model() is True
+    
+    # Create a mock response with empty content
+    mock_response = MagicMock()
+    mock_response.choices = [MagicMock()]
+    mock_response.choices[0].message = MagicMock()
+    mock_response.choices[0].message.content = ""
+    
+    # Mock litellm.completion to return our mock response
+    with patch('litellm.completion', return_value=mock_response):
+        # Call the non-streaming response handler directly
+        result = llm._handle_non_streaming_response({"model": model_name})
+        
+        # Verify that our fix works for all Gemini configurations
+        assert "Response processed successfully" in result
+        assert "HTML template" in result
+
+
+def test_non_gemini_model():
+    """Test that non-Gemini models don't get special handling for empty responses."""
+    # Create a mock LLM instance with a non-Gemini model
+    llm = LLM(model="gpt-4")
+    
+    # Verify that _is_gemini_model correctly identifies this as not a Gemini model
+    assert llm._is_gemini_model() is False
+    
+    # Create a mock response with empty content
+    mock_response = MagicMock()
+    mock_response.choices = [MagicMock()]
+    mock_response.choices[0].message = MagicMock()
+    mock_response.choices[0].message.content = ""
+    
+    # Mock litellm.completion to return our mock response
+    with patch('litellm.completion', return_value=mock_response):
+        # Call the non-streaming response handler directly
+        result = llm._handle_non_streaming_response({"model": "gpt-4"})
+        
+        # Verify that non-Gemini models just return the empty string
+        assert result == ""