Skip to content

fix: correct display_generating logic to only show when stream=False AND verbose=True #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MervinPraison merged 2 commits into from
Jul 22, 2025
Merged

fix: correct display_generating logic to only show when stream=False AND verbose=True #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
970ec4c
fix: correct display_generating logic to only show when stream=False …
github-actions[bot] Jul 22, 2025
c6f0e20
fix: correct hardcoded True to stream variable in display_generating …
github-actions[bot] Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 13 additions & 22 deletions src/praisonai-agents/praisonaiagents/agent/agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,7 @@ def _process_stream_response(self, messages, temperature, start_time, formatted_
tools=formatted_tools,
start_time=start_time,
console=self.console,
display_fn=display_generating,
display_fn=display_generating if (not stream and self.verbose) else None, # stream is True in this context
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix undefined variable stream and simplify logic

The variable stream is not defined in this method's scope, causing a runtime error. Since _process_stream_response is specifically for handling streaming responses, the display should always be None during streaming (as per the PR objective).

-            display_fn=display_generating if (not stream and self.verbose) else None,  # stream is True in this context
+            display_fn=None,  # No display during streaming
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
display_fn=display_generating if (not stream and self.verbose) else None, # stream is True in this context
display_fn=None, # No display during streaming
🧰 Tools
🪛 Ruff (0.12.2)

1073-1073: Undefined name stream

(F821)

🤖 Prompt for AI Agents 
In src/praisonai-agents/praisonaiagents/agent/agent.py at line 1073, the
variable `stream` is undefined causing a runtime error. Since this method
handles streaming responses, remove the undefined `stream` variable from the
condition and set `display_fn` to None directly to simplify the logic and align
with the streaming context.

reasoning_steps=reasoning_steps
)

Expand Down Expand Up @@ -1110,7 +1110,7 @@ def _chat_completion(self, messages, temperature=0.2, tools=None, stream=True, r
)
else:
# Non-streaming with custom LLM - add display functionality for verbose mode
if (stream or self.verbose) and self.console:
if (not stream and self.verbose) and self.console:
# Show "Generating..." display for verbose mode like OpenAI path
with Live(
display_generating("", start_time),
Expand Down Expand Up @@ -1169,7 +1169,7 @@ def _chat_completion(self, messages, temperature=0.2, tools=None, stream=True, r
execute_tool_fn=self.execute_tool,
stream=stream,
console=self.console if (self.verbose or stream) else None,
display_fn=display_generating if (stream or self.verbose) else None,
display_fn=display_generating if (not stream and self.verbose) else None,
reasoning_steps=reasoning_steps,
verbose=self.verbose,
max_iterations=10
Expand Down Expand Up @@ -1921,25 +1921,16 @@ async def _achat_completion(self, response, tools, reasoning_steps=False):
chunks = []
start_time = time.time()

with Live(
display_generating("", start_time),
console=self.console,
refresh_per_second=4,
transient=True,
vertical_overflow="ellipsis",
auto_refresh=True
) as live:
async for chunk in final_response:
chunks.append(chunk)
if chunk.choices[0].delta.content:
full_response_text += chunk.choices[0].delta.content
live.update(display_generating(full_response_text, start_time))

if reasoning_steps and hasattr(chunk.choices[0].delta, "reasoning_content"):
rc = chunk.choices[0].delta.reasoning_content
if rc:
reasoning_content += rc
live.update(display_generating(f"{full_response_text}\n[Reasoning: {reasoning_content}]", start_time))
# Process stream without display_generating since streaming is active
async for chunk in final_response:
chunks.append(chunk)
if chunk.choices[0].delta.content:
full_response_text += chunk.choices[0].delta.content

if reasoning_steps and hasattr(chunk.choices[0].delta, "reasoning_content"):
rc = chunk.choices[0].delta.reasoning_content
if rc:
reasoning_content += rc

self.console.print()

Expand Down
16 changes: 8 additions & 8 deletions test_comprehensive_display_fix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ def test_display_logic():
test_cases = [
{"stream": False, "verbose": False, "expected": False, "description": "No display (stream=False, verbose=False)"},
{"stream": False, "verbose": True, "expected": True, "description": "Display in verbose mode (stream=False, verbose=True) - MAIN FIX"},
{"stream": True, "verbose": False, "expected": True, "description": "Display in stream mode (stream=True, verbose=False)"},
{"stream": True, "verbose": True, "expected": True, "description": "Display in both modes (stream=True, verbose=True)"},
{"stream": True, "verbose": False, "expected": False, "description": "No display when streaming (stream=True, verbose=False)"},
{"stream": True, "verbose": True, "expected": False, "description": "No display when streaming (stream=True, verbose=True)"},
]

print(f"{'Description':<55} {'Stream':<8} {'Verbose':<8} {'Expected':<8} {'Result':<8} {'Status'}")
Expand All @@ -28,7 +28,7 @@ def test_display_logic():
all_passed = True
for case in test_cases:
# Test the actual logic used in the fix
result = (case["stream"] or case["verbose"])
result = (not case["stream"] and case["verbose"])
expected = case["expected"]
status = "✅ PASS" if result == expected else "❌ FAIL"

Expand Down Expand Up @@ -61,11 +61,11 @@ def test_agent_paths():
content = f.read()

# Check for OpenAI path fix
openai_fix = "display_fn=display_generating if (stream or self.verbose) else None"
openai_fix = "display_fn=display_generating if (not stream and self.verbose) else None"
has_openai_fix = openai_fix in content

# Check for custom LLM path fix
custom_llm_fix = "if (stream or self.verbose) and self.console:"
custom_llm_fix = "if (not stream and self.verbose) and self.console:"
has_custom_fix = custom_llm_fix in content

print(f"OpenAI path fix present: {'✅ YES' if has_openai_fix else '❌ NO'}")
Expand All @@ -84,14 +84,14 @@ def test_backward_compatibility():

# Test cases that should maintain existing behavior
scenarios = [
{"name": "Default streaming behavior", "stream": True, "verbose": True, "should_display": True},
{"name": "Default streaming behavior", "stream": True, "verbose": True, "should_display": False},
{"name": "Non-verbose non-streaming", "stream": False, "verbose": False, "should_display": False},
{"name": "Streaming with verbose off", "stream": True, "verbose": False, "should_display": True},
{"name": "Streaming with verbose off", "stream": True, "verbose": False, "should_display": False},
]

all_compat = True
for scenario in scenarios:
result = (scenario["stream"] or scenario["verbose"])
result = (not scenario["stream"] and scenario["verbose"])
expected = scenario["should_display"]
status = "✅ COMPATIBLE" if result == expected else "❌ INCOMPATIBLE"

Expand Down
12 changes: 6 additions & 6 deletions test_logic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ def original_logic(stream, verbose):

# Fixed logic
def fixed_logic(stream, verbose):
return (stream or verbose)
return (not stream and verbose)

print("=== Testing display_generating logic fix ===")

# Test cases
test_cases = [
{"stream": False, "verbose": False, "expected_display": False, "description": "No verbose, no stream"},
{"stream": False, "verbose": True, "expected_display": True, "description": "Verbose but no stream (user's case)"},
{"stream": True, "verbose": False, "expected_display": True, "description": "Stream but no verbose"},
{"stream": True, "verbose": True, "expected_display": True, "description": "Both stream and verbose"},
{"stream": True, "verbose": False, "expected_display": False, "description": "Stream but no verbose (no display when streaming)"},
{"stream": True, "verbose": True, "expected_display": False, "description": "Both stream and verbose (no display when streaming)"},
]

print(f"{'Description':<35} {'Stream':<8} {'Verbose':<8} {'Original':<10} {'Fixed':<8} {'Expected':<8} {'Status'}")
Expand All @@ -39,10 +39,10 @@ def fixed_logic(stream, verbose):
print("-" * 80)
if all_passed:
print("✅ All tests PASSED! The fix should work correctly.")
print("✅ display_generating will now be called when verbose=True, even with stream=False")
print("✅ display_generating will now ONLY be called when stream=False AND verbose=True")
else:
print("❌ Some tests FAILED!")

print("\n=== Key Fix ===")
print("Before: display_fn=display_generating if stream else None")
print("After: display_fn=display_generating if (stream or verbose) else None")
print("Before: display_fn=display_generating if (stream or verbose) else None")
print("After: display_fn=display_generating if (not stream and verbose) else None")