-
Notifications
You must be signed in to change notification settings - Fork 10
feat: Add static provider for listing available documents #45
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
Conversation
WalkthroughThe pull request introduces a new documents provider to the knowledge plugin, enabling agents to discover and list available documents from the knowledge base. The version is bumped to 1.5.13, and the new provider is exported through the plugin's public API. Changes
Sequence DiagramsequenceDiagram
participant Agent
participant Provider as documentsProvider
participant Runtime as IAgentRuntime
participant KnowledgeService as Knowledge Service
Agent->>Provider: get(runtime)
Provider->>Runtime: access knowledge service
alt Knowledge Service exists
Provider->>KnowledgeService: query documents table
KnowledgeService-->>Provider: return memories
Provider->>Provider: filter for documents<br/>(exclude fragments)
Provider->>Provider: format metadata<br/>(filename, type, size, source)
Provider-->>Agent: return formatted list +<br/>count + text
else No Knowledge Service
Provider-->>Agent: return empty results
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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: 2
🧹 Nitpick comments (3)
src/documents-provider.ts (3)
38-38: Consider making the document limit configurable.The hardcoded limit of 100 documents might be insufficient for knowledge bases with many documents. Consider making this configurable or implementing pagination.
42-44: Potentially redundant filtering.Since you're already querying the
documentstable, filtering byMemoryType.DOCUMENTmight be redundant. However, this defensive approach ensures data integrity if the table contains mixed types.
55-89: Consider defining a proper type for document metadata.The use of
as anyon line 57 (and lines 98-102) bypasses type checking. While this may be necessary due to flexible metadata structures, consider defining an interface for expected document metadata fields to improve type safety.Example:
interface DocumentMetadata { filename?: string; title?: string; fileExt?: string; fileType?: string; source?: string; fileSize?: number; }However, the formatting logic itself is well-structured with appropriate fallbacks and the file size calculation is correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
package.json(1 hunks)src/documents-provider.ts(1 hunks)src/index.ts(3 hunks)
🔇 Additional comments (6)
package.json (1)
4-4: LGTM! Appropriate version bump.The minor version increment from 1.5.12 to 1.5.13 is appropriate for adding the new documents provider feature.
src/index.ts (3)
10-10: LGTM! Clean import.The documentsProvider is properly imported for use in the plugin.
77-77: LGTM! Provider correctly registered.The documentsProvider is appropriately added to the providers array alongside the existing knowledgeProvider, making it available to all plugin configurations.
149-149: LGTM! Provider properly exported.The documentsProvider is correctly exported from the module's public API, enabling external consumers to access it.
src/documents-provider.ts (2)
16-20: LGTM! Clear provider metadata.The provider name, description, and static flag are well-defined. Setting
dynamic: falseis appropriate since the available documents don't change based on individual messages.
35-39: No issues identified. The usage is semantically correct and follows the established codebase pattern.The search results confirm that using
runtime.agentIdasroomIdis the standard pattern throughout the entire codebase. This pattern is consistently applied in:
- Memory creation and retrieval across all document operations
- Service layer (defaulting
roomIdtoagentIdwhen not specified)- Route handlers for document fetching and searching
- Test suites for memory verification
- Document processing pipeline
The
roomIdparameter semantically represents the agent scope for memories—not a chat room. The consistent application and the service layer's implementation (which defaultsroomIdtoagentId) demonstrate this is intentional design, not an error.Likely an incorrect or invalid review comment.
| @@ -0,0 +1,121 @@ | |||
| import type { IAgentRuntime, Memory, Provider } from '@elizaos/core'; | |||
| import { addHeader, logger, MemoryType } from '@elizaos/core'; | |||
| import { KnowledgeService } from './service.ts'; | |||
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.
Remove file extension from import statement.
The .ts extension in the import path may cause module resolution issues in certain build configurations. TypeScript imports should typically omit the file extension.
Apply this diff:
-import { KnowledgeService } from './service.ts';
+import { KnowledgeService } from './service';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { KnowledgeService } from './service.ts'; | |
| import { KnowledgeService } from './service'; |
🤖 Prompt for AI Agents
In src/documents-provider.ts around line 3, the import uses a '.ts' extension
which can break some module resolvers; remove the file extension so the line
reads import { KnowledgeService } from './service'; and save; ensure the
relative path still points to the same module and update any other imports
following this project convention if needed.
| if (!knowledgeService) { | ||
| logger.warn('Knowledge service not available for documents provider'); | ||
| return { | ||
| data: { documents: [] }, | ||
| values: { documents: '' }, | ||
| text: '', | ||
| }; | ||
| } |
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.
Inconsistent error handling in return values.
When the knowledge service is unavailable (lines 25-32), the return object doesn't include an error field in data, but when an exception occurs (lines 112-119), it does. This inconsistency may confuse consumers trying to distinguish between "no documents available" and "error occurred."
Consider adding an error field when the service is unavailable:
if (!knowledgeService) {
logger.warn('Knowledge service not available for documents provider');
return {
- data: { documents: [] },
+ data: { documents: [], error: 'Knowledge service not available' },
values: { documents: '' },
text: '',
};
}Also applies to: 112-119
🤖 Prompt for AI Agents
In src/documents-provider.ts around lines 25-32 (and similarly ensure
consistency with lines 112-119), the return value when the knowledge service is
unavailable lacks the data.error field that the exception path sets; update the
early-return object to include a data.error string (e.g. 'Knowledge service not
available') and keep the same shape as the error-return in the catch block so
consumers can reliably detect an error versus an empty result; make the message
explicit and ensure values and text remain as currently returned.
Note
Adds a static
documentsProviderthat lists available knowledge documents, integrates it into plugin providers/exports, and bumps version to 1.5.13.documentsProvider(src/documents-provider.ts) to list available knowledge documents viaKnowledgeService.getMemories, filtering byMemoryType.DOCUMENTand formatting summary output.src/index.ts) by adding toproviders: [knowledgeProvider, documentsProvider]and export it.package.jsonversion from1.5.12to1.5.13.Written by Cursor Bugbot for commit 15d97c6. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Chores