Compare commits

...
Author SHA1 Message Date
praisonai-triage-agent[bot]andMervin Praison 1d2b68d646 fix: architectural flaws in LLM response guards identified by reviewers
- Fix openai_client.py: Guards in Responses API were defeated by exception handlers
  - Move empty response checks to prevent fallback on filtered content
  - Add content=None validation per Gemini review
  - Ensure ValueError propagates for empty/filtered responses

- Fix guardrails.py: Guards were converted to passed=True by fail-safe handler
  - Return GuardrailResult(passed=False) instead of raising ValueError
  - Add content=None validation per Gemini review
  - Properly block filtered content instead of passing it through

- Fix code_agent.py: Add missing content=None checks per Gemini review
  - Prevent TypeError when message.content is None
  - Maintain str return type contract

- Fix rag.py: Add missing content=None checks per Gemini review
  - Ensure RAGResult always gets string for answer field
  - Handle None content gracefully

Addresses critical architectural issues identified by:
- Greptile P1: Exception handlers defeating guards
- Gemini High/Medium Priority: Missing content=None validation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Mervin Praison <[email protected]>
2026-05-18 09:26:02 +00:00
Jonathan Hill 4e1c36c868 fix: guard against empty/filtered LLM responses in core LLM paths
Add null checks before accessing choices[0].message in four files:
- openai_client.py: sync/async tool-call paths and responses-API path
- guardrails.py: check/aguard result extraction
- rag.py: query_rag and aquery_rag answer extraction
- code_agent.py: generate/review/explain/refactor/fix methods
2026-05-18 04:11:09 -05:00
5 changed files with 122 additions and 9 deletions

No files matched your search

@@ -164,7 +164,8 @@ Follow best practices and coding standards."""
messages=messages,
**{k: v for k, v in kwargs.items() if k != "model"}
)
if not response.choices or response.choices[0].message is None or response.choices[0].message.content is None:
raise ValueError("LLM returned empty or filtered response")
code = response.choices[0].message.content
self._log("Code generated successfully", "green")
return code
@@ -204,7 +205,8 @@ Follow best practices and coding standards."""
messages=messages,
**{k: v for k, v in kwargs.items() if k != "model"}
)
if not response.choices or response.choices[0].message is None or response.choices[0].message.content is None:
raise ValueError("LLM returned empty or filtered response")
code = response.choices[0].message.content
self._log("Code generated successfully", "green")
return code
@@ -365,6 +367,8 @@ Provide clear, actionable feedback."""
**{k: v for k, v in kwargs.items() if k != "model"}
)
if not response.choices or response.choices[0].message is None or response.choices[0].message.content is None:
raise ValueError("LLM returned empty or filtered response")
review = response.choices[0].message.content
self._log("Review completed", "green")
return review
@@ -405,6 +409,8 @@ Include:
**{k: v for k, v in kwargs.items() if k != "model"}
)
if not response.choices or response.choices[0].message is None or response.choices[0].message.content is None:
raise ValueError("LLM returned empty or filtered response")
explanation = response.choices[0].message.content
self._log("Explanation completed", "green")
return explanation
@@ -451,6 +457,8 @@ Only output the refactored code."""
**{k: v for k, v in kwargs.items() if k != "model"}
)
if not response.choices or response.choices[0].message is None or response.choices[0].message.content is None:
raise ValueError("LLM returned empty or filtered response")
refactored = response.choices[0].message.content
self._log("Refactoring completed", "green")
return refactored
@@ -495,6 +503,8 @@ Ensure the fix:
**{k: v for k, v in kwargs.items() if k != "model"}
)
if not response.choices or response.choices[0].message is None or response.choices[0].message.content is None:
raise ValueError("LLM returned empty or filtered response")
fixed = response.choices[0].message.content
self._log("Fix completed", "green")
return fixed
@@ -907,7 +907,9 @@ class OpenAIClient:
final_response = self._responses_to_chat_completion(raw)
# Display the response text if display_fn provided
response_text = final_response.choices[0].message.content or ""
if not final_response.choices or final_response.choices[0].message is None or final_response.choices[0].message.content is None:
raise ValueError("LLM returned empty or filtered response")
response_text = final_response.choices[0].message.content
if display_fn and response_text:
from rich.markdown import Markdown
console.print(Markdown(response_text))
@@ -931,6 +933,9 @@ class OpenAIClient:
return final_response
except Exception as e:
# Don't fall back for empty/filtered response errors - they should propagate
if "LLM returned empty or filtered response" in str(e):
raise e
self.logger.warning(f"Responses API streaming failed, falling back: {e}")
# Fall through to Chat Completions streaming
@@ -1167,7 +1172,9 @@ class OpenAIClient:
final_response = self._responses_to_chat_completion(raw)
# Display the response text if display_fn provided
response_text = final_response.choices[0].message.content or ""
if not final_response.choices or final_response.choices[0].message is None or final_response.choices[0].message.content is None:
raise ValueError("LLM returned empty or filtered response")
response_text = final_response.choices[0].message.content
if display_fn and response_text:
from rich.markdown import Markdown
console.print(Markdown(response_text))
@@ -1196,6 +1203,9 @@ class OpenAIClient:
return final_response
except Exception as e:
# Don't fall back for empty/filtered response errors - they should propagate
if "LLM returned empty or filtered response" in str(e):
raise e
self.logger.warning(f"Responses API async streaming failed, falling back: {e}")
# Fall through to Chat Completions streaming
@@ -1638,8 +1648,10 @@ class OpenAIClient:
)
# Check for tool calls
if not final_response.choices or final_response.choices[0].message is None:
raise ValueError("LLM returned empty or filtered response")
tool_calls = getattr(final_response.choices[0].message, 'tool_calls', None)
# Emit llm_content for intermediate narrative display
# (gpt-4.1+ models produce text alongside tool calls)
response_content = getattr(final_response.choices[0].message, 'content', None)
@@ -1835,10 +1847,12 @@ class OpenAIClient:
if not final_response:
return None
# Check for tool calls
if not final_response.choices or final_response.choices[0].message is None:
raise ValueError("LLM returned empty or filtered response")
tool_calls = getattr(final_response.choices[0].message, 'tool_calls', None)
# Emit llm_content for intermediate narrative display
# (gpt-4.1+ models produce text alongside tool calls)
response_content = getattr(final_response.choices[0].message, 'content', None)
@@ -2037,6 +2051,8 @@ class OpenAIClient:
return
# Check for tool calls
if not final_response.choices or final_response.choices[0].message is None:
raise ValueError("LLM returned empty or filtered response")
tool_calls = getattr(final_response.choices[0].message, 'tool_calls', None)
if tool_calls and execute_tool_fn:
@@ -86,6 +86,16 @@ Only respond with the JSON, no other text."""
)
import json
if not response.choices or response.choices[0].message is None or response.choices[0].message.content is None:
# Empty/filtered response should fail the guardrail, not raise
return GuardrailResult(
passed=False,
violations=["LLM returned empty or filtered response"],
modified_content=None,
original_content=content,
guardrail_name=guardrail_name,
metadata=metadata or {},
)
result_text = response.choices[0].message.content
result_data = json.loads(result_text)
@@ -160,6 +170,16 @@ Only respond with the JSON, no other text."""
)
import json
if not response.choices or response.choices[0].message is None or response.choices[0].message.content is None:
# Empty/filtered response should fail the guardrail, not raise
return GuardrailResult(
passed=False,
violations=["LLM returned empty or filtered response"],
modified_content=None,
original_content=content,
guardrail_name=guardrail_name,
metadata=metadata or {},
)
result_text = response.choices[0].message.content
result_data = json.loads(result_text)
+2 -2
View File
@@ -103,7 +103,7 @@ def rag_query(
**kwargs
)
answer = response.choices[0].message.content if response.choices else ""
answer = (response.choices[0].message.content if response.choices and response.choices[0].message is not None and response.choices[0].message.content is not None else "") or ""
usage = None
if hasattr(response, 'usage') and response.usage:
@@ -188,7 +188,7 @@ async def arag_query(
**kwargs
)
answer = response.choices[0].message.content if response.choices else ""
answer = (response.choices[0].message.content if response.choices and response.choices[0].message is not None and response.choices[0].message.content is not None else "") or ""
usage = None
if hasattr(response, 'usage') and response.usage:
+67
View File
@@ -0,0 +1,67 @@
#!/usr/bin/env python3
"""
Test script to verify that empty/filtered LLM response fixes work correctly.
"""
import sys
import os
# Add src paths to Python path
sys.path.insert(0, os.path.join(os.path.dirname(__file__), 'src', 'praisonai-agents'))
sys.path.insert(0, os.path.join(os.path.dirname(__file__), 'src', 'praisonai'))
def test_empty_response_mock():
"""Test that our fixes handle empty responses correctly"""
# Create a mock response object that simulates empty/filtered responses
class MockChoice:
def __init__(self, message=None):
self.message = message
class MockMessage:
def __init__(self, content=None):
self.content = content
class MockResponse:
def __init__(self, choices=None):
self.choices = choices or []
# Test case 1: Empty choices
empty_choices_response = MockResponse(choices=[])
# Test case 2: None message
none_message_response = MockResponse(choices=[MockChoice(message=None)])
# Test case 3: None content
none_content_response = MockResponse(choices=[MockChoice(message=MockMessage(content=None))])
# Test case 4: Valid response
valid_response = MockResponse(choices=[MockChoice(message=MockMessage(content="Valid response"))])
# Test the RAG fix
def test_rag_answer_extraction(response):
answer = (response.choices[0].message.content if response.choices and response.choices[0].message is not None and response.choices[0].message.content is not None else "") or ""
return answer
print("Testing RAG answer extraction...")
assert test_rag_answer_extraction(empty_choices_response) == ""
assert test_rag_answer_extraction(none_message_response) == ""
assert test_rag_answer_extraction(none_content_response) == ""
assert test_rag_answer_extraction(valid_response) == "Valid response"
print("✅ RAG fixes work correctly")
# Test the guard validation
def test_guard_validation(response):
return not response.choices or response.choices[0].message is None or response.choices[0].message.content is None
print("Testing guard validation...")
assert test_guard_validation(empty_choices_response) == True
assert test_guard_validation(none_message_response) == True
assert test_guard_validation(none_content_response) == True
assert test_guard_validation(valid_response) == False
print("✅ Guard validation works correctly")
print("All tests passed! ✅")
if __name__ == "__main__":
test_empty_response_mock()