fix: resolve Memory import error with litellm compatibility #610
Conversation
- Add LITELLM_AVAILABLE flag for proper dependency detection - Implement litellm → openai fallback pattern for all embedding operations - Update search_short_term, store_long_term, search_long_term embedding calls - Update calculate_quality_metrics to use litellm → openai fallback - Add graceful degradation when neither litellm nor openai is available - Fixes ImportError: cannot import name 'Memory' from 'praisonaiagents.memory' Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
- Export Memory class from main praisonaiagents package - Add Memory to __all__ list for proper package API - Create comprehensive test suite to verify all import scenarios - Tests original failing import from GitHub issue - Tests direct Memory imports and instantiation Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
|
Warning Rate limit exceeded@MervinPraison has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThe changes add and test explicit export of the Changes
Sequence Diagram(s)sequenceDiagram
participant TestScript
participant PraisonAIAgents
participant Memory
participant Session
TestScript->>PraisonAIAgents: import Memory (via package root)
Note right of PraisonAIAgents: __init__.py exports Memory
TestScript->>Memory: instantiate Memory(config)
Memory->>Memory: initialize (with litellm/OpenAI check)
TestScript->>Session: import Session
Session->>Memory: uses Memory internally
Assessment against linked issues
Poem
✨ 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 aims to resolve an ImportError specifically related to the Memory class within the praisonaiagents.memory module, which was reported in issue #605. The core problem seems to stem from dependency handling, particularly around litellm and openai.
The changes introduce a more robust dependency check and a fallback mechanism for LLM and embedding calls, prioritizing litellm if available, then falling back to the standard openai client, and finally logging a warning if neither is present. Additionally, the Memory class is explicitly exported from the main package __init__.py for easier access. New comprehensive tests have been added to verify that various import paths for Memory and related components (like Session and the main agents classes) now work correctly, including the specific failing import chain from the original issue.
Highlights
- Fixes ImportError: Resolves the reported
ImportErrorwhen importing theMemoryclass, addressing issue #605. - LiteLLM/OpenAI Compatibility: Implements a fallback mechanism for LLM completion and embedding calls within the
Memoryclass, attempting to uselitellmfirst, then falling back to the standardopenaiclient iflitellmis not available. - Dependency Handling: Adds a
LITELLM_AVAILABLEflag to explicitly check for the presence of thelitellmlibrary. - API Export: Adds the
Memoryclass to the main__all__export in__init__.pyfor easier top-level import. - Comprehensive Testing: Introduces new test scripts (
test_comprehensive_import.py,test_memory_import.py,test_session_import.py) to specifically validate various import scenarios and basicMemoryinstantiation, ensuring the fix is effective and prevents regressions.
Changelog
Click here to see the changelog
- src/praisonai-agents/praisonaiagents/init.py
- Imports the
Memoryclass frompraisonaiagents.memory.memory(line 15). - Adds
Memoryto the__all__list to make it available for direct import from the package root (line 47).
- Imports the
- src/praisonai-agents/praisonaiagents/memory/memory.py
- Adds a
try...except ImportErrorblock to check forlitellmand set aLITELLM_AVAILABLEflag (lines 32-36). - Modifies
search_short_termto uselitellm.embeddingifLITELLM_AVAILABLEis True, otherwise fall back toopenai.OpenAI().embeddings.createifOPENAI_AVAILABLEis True, logging a warning if neither is available (lines 349-370). - Modifies
store_long_termto uselitellm.embeddingifLITELLM_AVAILABLEis True, otherwise fall back toopenai.OpenAI().embeddings.createifOPENAI_AVAILABLEis True, logging a warning if neither is available (lines 487-519). - Modifies
search_long_termto uselitellm.embeddingifLITELLM_AVAILABLEis True, otherwise fall back toopenai.OpenAI().embeddings.createifOPENAI_AVAILABLEis True, logging a warning if neither is available (lines 570-591). - Modifies
calculate_quality_metricsto uselitellm.completionifLITELLM_AVAILABLEis True, otherwise fall back toopenai.OpenAI().chat.completions.createifOPENAI_AVAILABLEis True, logging an error and returning default metrics if neither is available (lines 966-1003).
- Adds a
- test_comprehensive_import.py
- Adds a new test file to perform comprehensive import checks.
- Includes tests for the original failing import chain (
praisonaiagents.agents.agents), directMemoryimport (praisonaiagents.memory),Memoryimport from the package root (praisonaiagents),Sessionimport (praisonaiagents.session), and basicMemoryinstantiation and operation.
- test_memory_import.py
- Adds a new test file specifically to verify the direct import of
praisonaiagents.memory.Memoryand basic instantiation.
- Adds a new test file specifically to verify the direct import of
- test_session_import.py
- Adds a new test file to verify the import of
praisonaiagents.session.Sessionand the original failing import chain (praisonaiagents.agents.agents).
- Adds a new test file to verify the import of
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.
Import paths now clear,
Memory loads without a fear,
Dependencies fixed.
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. ↩
…comprehensive_import.py
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
|
@claude verify if this implmentation to fix the error is correct. Verify that it is backward compatible |
|
Claude finished @MervinPraison's task in 1m 39s —— View job I'll verify the implementation for correctness and backward compatibility. Todo List:
✅ Implementation Analysis: CORRECT and BACKWARD COMPATIBLESummaryThe implementation successfully fixes the ImportError and maintains full backward compatibility. The litellm integration is properly implemented with robust fallback handling. Key Findings✅ Import Fix (Issue #605 Resolved)
✅ Litellm Integration (Excellent Implementation)
✅ Backward Compatibility (Fully Maintained)
✅ Code Quality (Professional Implementation)
Technical VerificationEmbedding Operations (4 locations)# Pattern used consistently:
if LITELLM_AVAILABLE:
import litellm
response = litellm.embedding(model="text-embedding-3-small", input=query)
embedding = response.data[0]["embedding"]
elif OPENAI_AVAILABLE:
from openai import OpenAI
client = OpenAI()
response = client.embeddings.create(input=query, model="text-embedding-3-small")
embedding = response.data[0].embedding
else:
# Graceful warning and fallbackLLM Completion Operations# Quality metrics calculation with proper JSON handling:
if LITELLM_AVAILABLE:
response = litellm.completion(model=model_name, messages=[...], response_format={"type": "json_object"})
elif OPENAI_AVAILABLE:
response = client.chat.completions.create(model=llm, messages=[...], response_format={"type": "json_object"})
# Both paths use: response.choices[0].message.contentTest Coverage AssessmentComprehensive Test Suite
Minor Observations (Non-blocking)
Security & Performance
Verdict: APPROVE ✅This implementation:
The litellm integration is implemented correctly with proper fallback mechanisms, and the Memory export fix directly addresses the ImportError. This is a solid, professional implementation that enhances the codebase without breaking existing functionality. |
CI Feedback 🧐(Feedback updated until commit 8c85de5)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
There was a problem hiding this comment.
Code Review
This pull request successfully addresses the Memory import error by updating __init__.py and introduces litellm compatibility for embedding and LLM calls, which is a valuable enhancement. The addition of new test scripts is also commendable for verifying these changes.
However, there's a critical issue in how the litellm -> openai fallback logic is implemented: if litellm is available but its API call fails (e.g., network error, API key issue), the current code does not fall back to openai. Instead, the exception is caught by an outer handler, preventing the intended fallback and potentially misattributing the error. This affects all methods where this pattern is used (search_short_term, store_long_term, search_long_term, and calculate_quality_metrics).
Additionally, there are some suggestions for improving test maintainability by making sys.path modifications more portable.
Addressing the critical fallback logic is essential before merging.
Summary of Findings
- Incorrect Fallback Logic: The fallback from
litellmtoopenai(for both embeddings and completions) does not function as intended iflitellmis available but its API call fails. The system currently doesn't attempt theopenaifallback in such scenarios. This is a critical issue affecting reliability. - Hardcoded Paths in Test Scripts: Test scripts (
test_comprehensive_import.py,test_memory_import.py,test_session_import.py) use hardcoded absolute paths forsys.pathmodification, which impacts portability and local development. (Severity: Medium) - Code Duplication for Embedding Logic: The logic for selecting and calling embedding providers (
litellmoropenai) is duplicated acrosssearch_short_term,store_long_term, andsearch_long_termmethods inmemory.py. Refactoring this into a helper method would improve maintainability. (Severity: Medium - Not commented due to review settings) - Test Scripting Style: The new test scripts use
print()for assertions/feedback andsys.exit()for status codes. While functional, adopting a standard testing framework likepytestorunittestwould offer more structured test organization, execution, and reporting. (Severity: Low - Not commented due to review settings)
Merge Readiness
This pull request makes good progress in resolving the import issue and adding litellm support. However, due to the critical issue identified with the litellm to openai fallback logic, I recommend that these changes not be merged until this is addressed. The fallback mechanism needs to be robust to ensure that if litellm is configured but fails during an API call, the system correctly attempts to use openai as intended. Additionally, addressing the medium-severity issue regarding test path portability would improve the overall quality. I am unable to approve pull requests directly; please ensure these critical issues are resolved and further review is conducted before merging.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ BugBot reviewed your changes and found no bugs!
BugBot free trial expires on June 9, 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 #605
Summary
Resolves the ImportError when importing Memory class from praisonaiagents.memory by implementing proper litellm compatibility with fallback handling.
Changes
Testing
Generated with Claude Code
PR Type
Bug fix, Enhancement, Tests
Description
Fix ImportError for
Memoryby adding it to package exports.Implement robust embedding backend selection: litellm → openai → fallback.
Refactor all embedding and LLM calls for litellm compatibility.
Add comprehensive tests for all import and instantiation scenarios.
Changes walkthrough 📝
__init__.py
Export Memory from package root and update __all__.src/praisonai-agents/praisonaiagents/init.py
Memoryto package imports and__all__for public API.Memoryis accessible from package root.memory.py
Add litellm compatibility and robust backend fallback forembeddings/LLM.src/praisonai-agents/praisonaiagents/memory/memory.py
LITELLM_AVAILABLEflag).openai, else warn/fail gracefully.
logic.
test_comprehensive_import.py
Add comprehensive test for all import scenarios.test_comprehensive_import.py
failing import.
test_memory_import.py
Add direct Memory import and instantiation test.test_memory_import.py
test_session_import.py
Add Session and related import chain test.test_session_import.py
Summary by CodeRabbit
litellmlibrary as an alternative to OpenAI for embedding and completion tasks, with automatic fallback and improved logging.MemoryandSession, ensuring smoother integration and troubleshooting.Memoryclass directly accessible from the main package.