fix: prevent duplicate Task and Response display when using litellm with memory#613
Conversation
…ith memory - Add display_interaction parameter to LLM.get_response() method (default: True) - Update all internal display_interaction calls in LLM class to respect new parameter - Modify agent calls to pass display_interaction=False when using custom LLM instances - Maintains backward compatibility while fixing duplicate displays (fixes #612) Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
|
Caution Review failedThe pull request is closed. WalkthroughA new boolean parameter Changes
Sequence Diagram(s)sequenceDiagram
participant Agent
participant LLM
Agent->>LLM: get_response(..., display_interaction=False)
alt display_interaction == False
LLM-->>LLM: Skip interaction display
else display_interaction == True and verbose == True
LLM-->>LLM: Display interaction output
end
LLM-->>Agent: Return response
Assessment against linked issues
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ 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 (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![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 to help everyone quickly understand the changes.
This PR addresses an issue where using LiteLLM with memory in PraisonAI agents resulted in duplicate display of the agent's 'Task' and 'Response' in the console. The root cause was identified as both the Agent class and the LLM class independently calling the display_interaction() function when a custom LLM instance (like one configured with LiteLLM and memory) was used.
The solution implemented is a minimal fix that maintains backward compatibility. It involves adding a new boolean parameter, display_interaction, to the LLM.get_response() and LLM.get_response_async() methods, defaulting to True. The internal calls to display_interaction() within the LLM class are updated to respect this new parameter. Finally, the agent's calls to the LLM's get_response methods are modified to pass display_interaction=False when using a custom LLM, effectively preventing the LLM from displaying the interaction itself, leaving the display responsibility solely with the Agent.
Highlights
- Bug Fix: Fixes a bug causing duplicate 'Task' and 'Response' output when using LiteLLM with memory in agents.
- LLM Method Update: Adds a new
display_interactionboolean parameter (defaulting toTrue) to theLLM.get_response()andLLM.get_response_async()methods to control whether the LLM class itself displays the interaction. - Agent Integration: Modifies the Agent's calls to LLM methods to pass
display_interaction=Falsewhen using custom LLMs, ensuring only the Agent handles the display in these cases. - Backward Compatibility: The changes are backward compatible due to the new parameter defaulting to
True.
Changelog
Click here to see the changelog
- src/praisonai-agents/praisonaiagents/agent/agent.py
- Added
display_interaction=Falseto thellm.get_responsecall within_chat_completion(line 924). - Added
display_interaction=Falseto thellm.get_responsecall within_chat_completion(line 941). - Added
display_interaction=Falseto thellm.get_responsecall withinchat(line 1114). - Added
display_interaction=Falseto thellm.get_response_asynccall withinachat(line 1388).
- Added
- src/praisonai-agents/praisonaiagents/llm/llm.py
- Added
display_interaction: bool = Trueparameter to theget_responsemethod signature (line 293). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionfor reasoning/answer display (line 458). - Added
and display_interactioncondition to theelif verbosecheck before callingdisplay_interactionfor response display (line 466). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionafter tool calls (line 745). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionfor JSON/Pydantic output (line 760). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionwhen not self-reflecting (line 766). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionafter reflection attempts (line 928). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionfor JSON/Pydantic output inget_response_async(line 1382). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionwhen not self-reflecting inget_response_async(line 1388). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionin the baseresponsemethod (line 1718). - Added
and display_interactioncondition to theif verbosecheck before callingdisplay_interactionin the basearesponsemethod (line 1826).
- Added
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 Code Suggestions ✨No code suggestions found for the PR. |
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of duplicate Task and Response displays when using litellm with memory by introducing a display_interaction flag. The core logic change is sound, and making the default True ensures backward compatibility, which is excellent.
However, there are a few critical issues in llm.py where the new display_interaction parameter is used in the methods get_response_async, response, and aresponse but has not been added to their respective signatures. This will likely lead to NameError exceptions at runtime. Additionally, the docstring for get_response (and consequently for the other methods once the parameter is added) should be updated to reflect this new parameter.
Overall, the approach is good, but these critical points need to be addressed before merging.
Summary of Findings
- Missing
display_interactionparameter in method signatures: Thedisplay_interactionparameter is used withinget_response_async,response, andaresponsemethods inllm.py, but it has not been added to their respective method signatures. This is critical as it will likely causeNameErrorexceptions. - Docstring Update Needed for
get_response: The docstring for theget_responsemethod inllm.pyshould be updated to include information about the newdisplay_interactionparameter. This is important for maintainability and user understanding. - Docstring Updates for Other Affected Methods: Once the
display_interactionparameter is added to the signatures ofget_response_async,response, andaresponse, their docstrings will also need to be updated accordingly.
Merge Readiness
The pull request introduces a valuable fix for duplicate displays. However, due to the critical issues identified (missing parameters in several method signatures in llm.py), I recommend that these changes be addressed before merging. Once these are fixed, and the relevant docstrings are updated, the PR should be in good shape. As a reviewer, I am not authorized to approve pull requests, so please ensure further review and approval from authorized maintainers after addressing the feedback.
| self.chat_history.append({"role": "user", "content": original_prompt}) | ||
| self.chat_history.append({"role": "assistant", "content": response_text}) | ||
| if verbose: | ||
| if verbose and display_interaction: |
There was a problem hiding this comment.
The display_interaction variable is used in this conditional, but it doesn't appear to be defined as a parameter in the get_response_async method's signature (see full file, signature starts around line 950). It's also not explicitly extracted from **kwargs within the method body.
This will likely cause a NameError when get_response_async is called.
To ensure this method functions correctly and consistently with get_response, should display_interaction: bool = True be added to the get_response_async method signature? The PR description mentions that the change applies to 'related methods', which implies get_response_async should have this parameter explicitly.
If it were intended to be passed via kwargs (though less ideal for clarity), it would need to be accessed like display_interaction = kwargs.get('display_interaction', True) at the beginning of the method.
| response_text = response.choices[0].message.content.strip() | ||
|
|
||
| if verbose: | ||
| if verbose and display_interaction: |
There was a problem hiding this comment.
Similar to get_response_async, the display_interaction variable is used here in the response method, but it's not defined in its signature (see full file, signature starts around line 1638). This is likely to cause a NameError.
For consistency and correctness, should display_interaction: bool = True be added to the response method's signature? This would align with the changes made to get_response and the intent described in the PR.
| response_text = response.choices[0].message.content.strip() | ||
|
|
||
| if verbose: | ||
| if verbose and display_interaction: |
There was a problem hiding this comment.
The display_interaction variable is used here within the aresponse method, but it's missing from the method's signature (see full file, signature starts around line 1746). This will likely lead to a NameError at runtime.
To fix this and maintain consistency with get_response, could we add display_interaction: bool = True to the aresponse method's signature?
| agent_role: Optional[str] = None, | ||
| agent_tools: Optional[List[str]] = None, | ||
| execute_tool_fn: Optional[Callable] = None, | ||
| display_interaction: bool = True, |
There was a problem hiding this comment.
It's great that you've added the display_interaction parameter with a default value for backward compatibility!
To help users and maintainers understand this new parameter, could we update the docstring for get_response (lines 296-...) to include a description of display_interaction, its purpose, and its default value? This would improve the method's documentation and maintainability.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Async Method Missing Parameter
The display_interaction parameter was added to the get_response method signature (line 293) but omitted from its asynchronous counterpart, get_response_async. The agent code calls get_response_async with display_interaction=False (e.g., agent.py line 1388), which will cause a TypeError. Additionally, the get_response_async method body uses display_interaction in conditional checks (e.g., lines 1382, 1388), leading to a NameError as the variable is undefined within the method scope.
src/praisonai-agents/praisonaiagents/agent/agent.py#L1387-L1388
PraisonAI/src/praisonai-agents/praisonaiagents/agent/agent.py
Lines 1387 to 1388 in a90c306
src/praisonai-agents/praisonaiagents/llm/llm.py#L1381-L1388
PraisonAI/src/praisonai-agents/praisonaiagents/llm/llm.py
Lines 1381 to 1388 in a90c306
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 👎
…612-20250605_213542 fix: prevent duplicate Task and Response display when using litellm with memory
1 participant
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 both the Agent class and LLM class were independently calling
display_interaction()when using custom LLM instances with memory:Agent.chat()→_chat_completion()→llm.get_response()llm.get_response()callsdisplay_interaction()internallyAgent.chat()callsdisplay_interaction()againSolution
Implemented a minimal fix with backward compatibility:
display_interactionparameter toLLM.get_response()method (default:True)display_interaction()calls in LLM class to respect this parameterdisplay_interaction=Falsewhen using custom LLM instancesChanges
llm.py:293: Addeddisplay_interaction: bool = Trueparameterllm.py: Updated ~20 display calls to checkverbose and display_interactionagent.py:924, 941, 1114, 1388: Addeddisplay_interaction=Falseto LLM callsBackward Compatibility
This change maintains full backward compatibility as the new parameter defaults to
True.Generated with Claude Code
PR Type
Bug fix, Enhancement
Description
Prevent duplicate Task and Response display in agent-LLM interactions
display_interactionparameter toLLM.get_response()and related methodsdisplay_interaction=Falsewhen using custom LLMsMaintains backward compatibility with default display behavior
Changes walkthrough 📝
agent.py
Prevent duplicate display by disabling agent-side interaction displaysrc/praisonai-agents/praisonaiagents/agent/agent.py
display_interaction=Falseto LLMget_response()calls in fourlocations
handles it
llm.py
Add display_interaction flag and update display logic in LLMsrc/praisonai-agents/praisonaiagents/llm/llm.py
display_interactionparameter (default True) toget_response()and related methods
verboseanddisplay_interactionSummary by CodeRabbit