Revert "fix: prevent duplicate Task and Response display when using litellm with memory"#614
Conversation
…itellm with memory"
|
Caution Review failedThe pull request is closed. WalkthroughThe changes remove the Changes
Poem
Suggested labels
📜 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:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
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 is a direct revert of a previous change (PR #613) which aimed to prevent duplicate Task and Response displays specifically when using litellm with memory. The previous fix introduced a display_interaction parameter and logic to control output display. This revert removes that parameter and the associated logic from the LLM methods and the calls to them in the agent code, effectively undoing the implementation of the fix from #613. The reason for the revert isn't detailed in the description, but it suggests the previous approach might not have been the correct or complete solution.
Highlights
- Revert Previous Fix: This PR reverts the changes introduced in PR #613, which attempted to prevent duplicate display issues when using litellm with memory.
- Removed Display Control Parameter: The
display_interactionparameter has been removed from the LLM'sget_responseandget_response_asyncmethods. - Simplified Display Logic: The logic controlling when interactions are displayed now relies solely on the
verboseflag, removing the check for thedisplay_interactionparameter.
Changelog
- src/praisonai-agents/praisonaiagents/agent/agent.py
- Removed the
display_interaction=Falseargument from calls to the LLM's response methods (_chat_completion,chat,achat).
- Removed the
- src/praisonai-agents/praisonaiagents/llm/llm.py
- Removed the
display_interactionparameter from theget_responseandget_response_asyncfunction signatures. - Removed the
and display_interactioncondition from severalif verbose and display_interaction:checks, simplifying the display logic.
- Removed the
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. ↩
There was a problem hiding this comment.
Code Review
This pull request successfully reverts the changes introduced in PR #613, which aimed to prevent duplicate Task and Response displays when using LiteLLM with memory. The changes correctly remove the display_interaction parameter and associated logic from the LLM class and the calls to the LLM from the Agent class. This aligns with the stated goal of restoring the previous verbose display behavior.
Summary of Findings
- Re-introduction of Duplicate Display Issue: This revert PR removes the logic that prevented duplicate Task/Response displays when using LiteLLM with memory. This action re-introduces the original issue that PR #613 aimed to fix. While this is the intended outcome of a revert, it's important to acknowledge that the duplicate display behavior will return.
Merge Readiness
The code changes correctly implement the revert of PR #613. The primary consequence is the re-introduction of the duplicate Task/Response display issue when using LiteLLM with memory, which is noted as a medium severity finding. Whether this PR is ready to merge depends on whether re-introducing this specific behavior is acceptable for the project at this time. Please confirm if the re-introduced duplicate display is an acceptable trade-off for reverting the previous fix. I am unable to approve this pull request; please have other reviewers assess and approve the changes before merging.
There was a problem hiding this comment.
Bug: Interaction Display Logic Error
The conditional logic for displaying interactions in the get_response method was incorrectly changed from elif verbose and display_interaction: to else:. This causes display_interaction to be called even when verbose=False, violating the intended behavior of only displaying output when verbose mode is enabled. The condition should be elif verbose: to maintain the correct check.
src/praisonai-agents/praisonaiagents/llm/llm.py#L456-L472
PraisonAI/src/praisonai-agents/praisonaiagents/llm/llm.py
Lines 456 to 472 in 31c054a
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 👎
…aude/issue-612-20250605_213542 Revert "fix: prevent duplicate Task and Response display when using litellm with memory"
User description
Reverts #613
PR Type
Bug fix
Description
Reverts previous fix that suppressed duplicate Task/Response display
Removes
display_interactionargument from agent and LLM callsRestores original verbose display behavior in LLM responses
Ensures interaction display logic matches pre-fix implementation
Changes walkthrough 📝
agent.py
Remove display_interaction argument from agent LLM callssrc/praisonai-agents/praisonaiagents/agent/agent.py
display_interaction=Falsefrom LLM call argumentsllm.py
Revert display_interaction logic in LLM response methodssrc/praisonai-agents/praisonaiagents/llm/llm.py
display_interactionparameter fromget_responseand relatedmethods
display_interactionverbosefor displaySummary by CodeRabbit
display_interactionsetting. Verbose outputs now rely solely on theverboseflag, simplifying user experience. No changes to core functionality or visible features.