-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
feat: add adapter-qdrant #2322
feat: add adapter-qdrant #2322
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @oxf71! Welcome to the elizaOS community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now an elizaOS contributor!
@coderabbitai review |
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces a new Qdrant database adapter for the project, expanding the database integration capabilities. Changes include adding configuration parameters for Qdrant in the Changes
Possibly related PRs
Suggested labels
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/adapter-qdrant/src/index.ts (1)
143-179
: Enhance caching strategy with invalidation and size limits.An in-memory cache without eviction policy can lead to uncontrolled memory growth. Consider implementing cache invalidation and setting size limits.
.env.example (1)
554-559
: Add documentation comments for Qdrant configuration.While the variables and default values are appropriate, adding brief documentation comments would help users understand:
- The expected format for QDRANT_URL
- Whether QDRANT_KEY is optional
- Why these specific default values were chosen
# Qdrant -QDRANT_URL= -QDRANT_KEY= -QDRANT_PORT=443 -QDRANT_VECTOR_SIZE=1536 +# URL of your Qdrant instance (e.g., https://your-instance.qdrant.tech) +QDRANT_URL= +# API key for authentication (optional for local instances) +QDRANT_KEY= +# Qdrant service port (default: 443 for cloud, typically 6333 for local) +QDRANT_PORT=443 +# Vector size matching your embedding model (default: 1536 for OpenAI embeddings) +QDRANT_VECTOR_SIZE=1536
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.env.example
(1 hunks)agent/package.json
(1 hunks)agent/src/index.ts
(2 hunks)packages/adapter-qdrant/.npmignore
(1 hunks)packages/adapter-qdrant/eslint.config.mjs
(1 hunks)packages/adapter-qdrant/package.json
(1 hunks)packages/adapter-qdrant/src/index.ts
(1 hunks)packages/adapter-qdrant/tsconfig.json
(1 hunks)packages/adapter-qdrant/tsup.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/adapter-qdrant/tsconfig.json
- packages/adapter-qdrant/.npmignore
- packages/adapter-qdrant/tsup.config.ts
- packages/adapter-qdrant/eslint.config.mjs
🔇 Additional comments (2)
packages/adapter-qdrant/package.json (1)
1-30
: LGTM!The
package.json
is correctly configured with appropriate dependencies and scripts.agent/package.json (1)
26-26
: LGTM! Qdrant adapter dependency added correctly.The dependency is properly added using workspace syntax and maintains alphabetical ordering.
Head branch was pushed to by a user without write access
@coderabbitai review |
@wtfsayo i'll review the changes in this pr that adds a new qdrant adapter. ✅ Actions performedReview triggered.
|
@oxf71 can you share demo if possible; Also please allow us to push updates to PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/adapter-qdrant/src/index.ts (1)
41-58
: Use descriptive variable name inpreprocess
methodRenaming the variable
c
toprocessedContent
enhances code readability and clarity.Apply this diff:
- const c = content + const processedContent = content ... - return c + return processedContent
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.example
(1 hunks)agent/package.json
(1 hunks)agent/src/index.ts
(2 hunks)packages/adapter-qdrant/src/index.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- agent/package.json
- .env.example
🔇 Additional comments (2)
agent/src/index.ts (2)
3-3
: Import statement is correctThe import of
QdrantDatabaseAdapter
is properly added.
574-587
: Database initialization logic for Qdrant is accurateThe added conditional check for environment variables and initialization of
QdrantDatabaseAdapter
are correctly implemented.
…qdrant # Conflicts: # agent/package.json
We are using the "trump character". The "knowledge" field needs to be changed to a path format. After testing, we found that there is a difference: the implemented knowledge method is only invoked when it is set to the path format. pnpm run dev --characters="characters/trump.character.json"
|
Head branch was pushed to by a user without write access
…qdrant # Conflicts: # agent/package.json
hey @oxf71 can you share a demo of it working? Personally superexcited for this |
also can't resolve conflict or/and push change to this branch myself |
Relates to
Add qdrant adapter into Eliza system.
Risks
Low
Main implementation risk is that the Adapter doesn't work correctly.
Background
What does this PR do?
Add a new adapter for qdrant.
What kind of change is this?
Features (non-breaking change which adds functionality).
Documentation changes needed?
Testing
Where should a reviewer start?
Detailed testing steps
Discord username
Summary by CodeRabbit
New Features
Dependencies
@elizaos/adapter-qdrant
package@qdrant/js-client-rest
dependencyConfiguration