fix: prevent duplicate Task and Response display when using litellm with memory #617
Conversation
…ith memory - Add suppress_display parameter to LLM.get_response() method (default: False) - Update all display_interaction calls in LLM class to respect suppress_display flag - Agent class now passes suppress_display=True to prevent LLM duplicate displays - Agent handles display centrally to ensure single display per interaction - Maintains full backward compatibility while fixing issue #612 Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
|
Caution Review failedThe pull request is closed. """ WalkthroughThe changes introduce a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Agent
participant LLM
User->>Agent: chat(prompt, ...)
Agent->>LLM: get_response(prompt, ..., suppress_display=True)
LLM-->>Agent: response_text
Agent->>Agent: Append response to history
Agent->>Agent: If verbose, display_interaction()
Agent-->>User: response_text
Assessment against linked issues
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @MervinPraison, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request. This PR addresses an issue where using LiteLLM with memory in PraisonAI agents resulted in duplicate displays of the Task and Response interactions. The root cause was identified as both the internal LLM class methods and the Agent class attempting to handle the display of interactions, leading to redundancy when a custom LLM instance was used. The solution implemented is a minimal fix that maintains backward compatibility by introducing a suppress_display parameter to the LLM.get_response method. This parameter is used internally within the LLM class to prevent its own display calls when suppress_display is set to True. The Agent's chat method is updated to call get_response with suppress_display=True and then handles the display centrally, ensuring each interaction is shown only once.
Highlights
- Fix Duplicate Display: Resolves the issue of duplicate Task and Response output when using LiteLLM with memory by centralizing display logic in the Agent.
- Add
suppress_displayParameter: Introduces a new boolean parametersuppress_display(defaulting toFalse) to theLLM.get_responsemethod to control internal display behavior. - Centralize Agent Display: Modifies the Agent's
chatmethod to call the LLM withsuppress_display=Trueand adds a dedicateddisplay_interactioncall within the Agent to ensure single, consistent output. - Backward Compatibility: The changes are backward compatible as the new parameter defaults to
False, not affecting existing LLM usages that don't explicitly set it.
Changelog
- src/praisonai-agents/praisonaiagents/agent/agent.py
- Modified the
chatmethod to passsuppress_display=Truewhen callingself.llm.get_response(line 1112). - Added a new block to call
display_interactionwithin the Agent'schatmethod whenself.verboseis true, handling the display centrally after suppressing the LLM's internal display (lines 1118-1121).
- Modified the
- src/praisonai-agents/praisonaiagents/llm/llm.py
- Added the
suppress_display: bool = Falseparameter to theget_responsemethod signature (line 293). - Updated multiple internal calls to
display_interactionwithinget_responseto checkverbose and not suppress_displaybefore displaying (lines 458, 466, 669, 677, 722, 745).
- Added the
- test_duplicate_fix.py
- Added a new test file to specifically verify that the duplicate display issue is fixed when using an Agent with a custom LLM configuration and memory enabled.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the duplicate display issue when using litellm with memory. The introduction of the suppress_display flag in the LLM class and centralizing display logic in the Agent class is a clean and well-thought-out solution. The changes maintain backward compatibility, which is excellent.
The core logic for suppressing and handling display appears correct and aligns well with the problem description. The addition of a regression test is a good step, though it could be enhanced for better automation.
Summary of Findings
- Test Automation: The new test
test_duplicate_fix.pyrelies on manual observation. It should be converted into an automated test that asserts the correct display behavior, for example, by mockingdisplay_interactionand checking its call count. (Commented) - Test Verbosity Setting: The test
test_duplicate_fix.pymay not trigger the intended display logic because the agent'sverboseattribute is not explicitly set to a positive value. This is crucial for testing the scenario where the agent handles the display. (Commented as part of Test Automation) - Generation Time Calculation Nuance: The
generation_timedisplayed by theAgentclass might be slightly different from what theLLMclass would have displayed. The agent's calculation includes more overhead from thechatmethod's start. This is a minor detail and likely acceptable for display. (Not commented due to review settings - low severity) - PEP 8: Missing Newline at End of File: The new file
test_duplicate_fix.pyis missing a newline character at the end of the file. (Not commented due to review settings - low severity) - PEP 8: Constant Naming: In
test_duplicate_fix.py,llm_configis a module-level constant and should ideally be namedLLM_CONFIGaccording to PEP 8. (Not commented due to review settings - low severity)
Merge Readiness
The core fix in this pull request is well-implemented and effectively solves the duplicate display problem. However, the associated regression test (test_duplicate_fix.py) currently relies on manual verification. To ensure long-term maintainability and catch future regressions automatically, this test should be updated to include automated assertions. I have provided suggestions on how to achieve this.
I recommend addressing the test automation aspect before merging. As a reviewer, I am not authorized to approve pull requests, so please ensure other necessary reviews and approvals are obtained.
| print("Testing fix for duplicate display issue...") | ||
| result = agents.start() | ||
| print("Test completed successfully!") No newline at end of file |
There was a problem hiding this comment.
This test is a good start for verifying the fix, but it currently relies on manual observation of the output. For a robust regression test, it would be beneficial to automate the verification.
Consider the following improvements:
- Automated Assertion: Instead of
printstatements for manual verification, the test should assert that the display interaction occurs the correct number of times (e.g., once per task interaction in verbose mode).- You could use
unittest.mock.patchto mocksrc.praisonaiagents.main.display_interactionand then assert itscall_count. - Alternatively, if using a test runner like
pytest, you could use thecapsysfixture to capture stdout and assert its content.
- You could use
- Verbose Mode Activation: The agent-level display (handled in
agent.py) and the LLM-level display (handled inllm.py) are conditional on verbosity settings. To properly test the scenario where the agent displays and the LLM is suppressed, ensure that the agent's verbose mode is active.- Currently,
blog_agent.verbosewill be0by default. You might need to setblog_agent.verbose = 1(or another positive integer) in the test setup for the agent-leveldisplay_interactionto be called.
- Currently,
An automated test will provide more confidence in CI/CD pipelines that this bug, once fixed, does not reappear.
Example using unittest.mock:
import unittest
from unittest.mock import patch
# ... other imports from your test file ...
# Ensure blog_agent is set to be verbose for the agent-level display to trigger
blog_agent = Agent(
role="Blog Writer",
goal="Write a blog post about AI",
backstory="Expert at writing blog posts",
llm=llm_config,
verbose=1 # Activate verbose mode for the agent
)
# ... (rest of your setup for blog_task, agents) ...
class TestDuplicateDisplay(unittest.TestCase):
@patch('src.praisonaiagents.main.display_interaction')
def test_no_duplicate_display_with_memory(self, mock_display_interaction):
# Re-initialize agents and tasks here if they are not module-level
# or pass the pre-configured blog_agent and blog_task
current_agents = PraisonAIAgents(
agents=[blog_agent], # Use the verbose agent
tasks=[blog_task],
memory=True
)
print("Testing fix for duplicate display issue...")
result = current_agents.start()
print("Test completed.")
# Assert that display_interaction was called (e.g., once for a single task)
self.assertEqual(mock_display_interaction.call_count, 1)
# You could add more specific assertions about the arguments if needed
if __name__ == '__main__':
unittest.main()This would replace the current print-based execution at the end of your file.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
bugbot run verbose=True |
|
bugbot is starting with request id serverGenReqId_b824bb70-e926-43b8-bfcc-ad472114db24 |
There was a problem hiding this comment.
Bug: Async Method Missing Display Suppression
The suppress_display parameter was added to the synchronous LLM.get_response method but is missing from the asynchronous LLM.get_response_async method. This creates inconsistent display behavior between synchronous (agent.chat) and asynchronous (agent.achat) agent calls. The fix for duplicate displays, which involves the agent suppressing the LLM's display and handling it centrally, only works for synchronous operations. Asynchronous agent calls using get_response_async cannot suppress the LLM's display, leading to duplicate displays in async scenarios.
src/praisonai-agents/praisonaiagents/llm/llm.py#L292-L293
PraisonAI/src/praisonai-agents/praisonaiagents/llm/llm.py
Lines 292 to 293 in 3038178
src/praisonai-agents/praisonaiagents/agent/agent.py#L1111-L1121
PraisonAI/src/praisonai-agents/praisonaiagents/agent/agent.py
Lines 1111 to 1121 in 3038178
BugBot free trial expires on June 11, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
User description
Fixes #612
Summary
This PR fixes the duplicate Task and Response printing issue when using litellm with memory in PraisonAI agents.
Root Cause
The issue occurred because the LLM class was calling
display_interaction()multiple times internally while the Agent class also expected to handle display, causing duplicates when using custom LLM instances.Solution
Implemented a minimal fix with backward compatibility:
suppress_displayparameter toLLM.get_response()method (default:False)display_interaction()calls in LLM class to respect this parametersuppress_display=Truewhen using custom LLM instancesChanges
llm.py:293: Addedsuppress_display: bool = Falseparameterllm.py: Updated ~4 display calls to checkverbose and not suppress_displayagent.py:1112: Addedsuppress_display=Trueto LLM callsagent.py:1119-1121: Added centralized Agent-level displayBackward Compatibility
This change maintains full backward compatibility as the new parameter defaults to
False.Generated with Claude Code
PR Type
Bug fix, Tests
Description
Prevents duplicate Task and Response display with memory-enabled agents.
suppress_displayparameter toLLM.get_response()(default: False).suppress_display.suppress_display=Trueto LLM.Adds regression test to verify duplicate display fix.
Changes walkthrough 📝
llm.py
Add suppress_display to LLM.get_response and update display logicsrc/praisonai-agents/praisonaiagents/llm/llm.py
suppress_displayparameter toget_response().display_interactioncalls to checknot suppress_display.suppress_displaytoFalse.
agent.py
Centralize display in Agent and suppress LLM duplicate outputsrc/praisonai-agents/praisonaiagents/agent/agent.py
suppress_display=Trueto LLM when agent manages display.test_duplicate_fix.py
Add regression test for duplicate display issuetest_duplicate_fix.py
Summary by CodeRabbit