Skip to content

Conversation

@lalalune
Copy link
Contributor

@lalalune lalalune commented Jun 18, 2025

This PR adds a bunch of testing coverage, action chaining, etc.

Summary by CodeRabbit

  • New Features

    • Introduced advanced knowledge management features including advanced filtered and sorted search, analytics, batch operations, and import/export in JSON, CSV, and Markdown formats.
    • Added new API endpoints for updating document metadata, bulk deletion, advanced search, analytics, batch operations, export, and import.
    • Enhanced attachment handling with support for direct file and URL attachments, multiple files, and automatic intent detection.
    • Expanded web interface capabilities with document listing, metadata updates, search, visual graph, upload, and delete functionality.
    • Added custom Cypress commands for knowledge panel interactions and file operations.
    • Added comprehensive UI component test pages and Cypress test suites covering UI components and plugin API endpoints.
    • Introduced new repositories for document and fragment management with PostgreSQL support.
    • Added detailed type definitions and configuration options for knowledge management features.
  • Bug Fixes

    • Improved error handling and validation for document updates, batch operations, and attachment processing.
    • Fixed handling of file uploads and metadata updates in API routes.
  • Documentation

    • Significantly expanded README with detailed advanced features, API usage, configuration, and testing instructions.
    • Removed obsolete plugin image requirements documentation.
  • Tests

    • Added extensive unit and end-to-end tests covering knowledge lifecycle, advanced features, action chaining, update operations, and UI components.
    • Introduced Vitest and Cypress configurations for improved test coverage and environment setup.
  • Chores

    • Updated dependencies, scripts, and configuration files for streamlined development, testing, and build workflows.
    • Expanded .gitignore to exclude additional environment, coverage, screenshots, and documentation files.

@coderabbitai
Copy link

coderabbitai bot commented Jun 18, 2025

Walkthrough

This update introduces advanced knowledge management features, including a new document and fragment schema, repositories, analytics, import/export, batch operations, and advanced search/filtering. It expands the API with new endpoints, enhances the service and actions, and adds comprehensive unit and end-to-end tests. UI components and configuration files are also updated for improved testing and integration.

Changes

File(s) / Group Change Summary
.gitignore Expanded ignore list to include additional files/directories for environment, coverage, screenshots, and docs.
README.md Significantly expanded to document new advanced features, APIs, usage examples, and testing instructions.
images/README.md Deleted image requirements documentation.
package.json Added dependencies/devDependencies and new scripts for linting and type checking.
tsconfig.json Added compiler options and exclude patterns for stricter and faster builds.
tsup.config.ts Marked @elizaos/plugin-sql as external.
vitest.config.ts New Vitest config for unit testing, coverage, and aliasing.
cypress.config.ts, cypress/e2e/*.cy.ts, cypress/support/* Added Cypress E2E config, tests, and custom commands for UI and knowledge plugin.
cypress/support/component-index.html, cypress/support/component.ts Added Cypress component testing support files and HTML template.
src/__tests__/e2e/*.test.ts Added new E2E tests for advanced features, attachment handling, startup loading, and knowledge lifecycle.
src/__tests__/unit/*.test.ts Added new unit tests for actions, advanced features, repositories, schema, update operations, and action chaining.
src/__tests__/unit/utils.test.ts Changed import path for looksLikeBase64.
src/actions.ts Enhanced processKnowledgeAction to handle attachments, added advanced search, analytics, and export actions.
src/config.ts Simplified and refactored model config validation, focusing on core settings.
src/ctx-embeddings.ts, src/provider.ts Switched to single quotes for strings and minor formatting; no logic changes.
src/docs-loader.ts Changed clientDocumentId generation and added metadata to loaded docs.
src/document-processor.ts Replaced dynamic provider rate limits with static defaults; simplified cache logic.
src/frontend/index.tsx Changed KnowledgeTab import from .tsx to .js.
src/frontend/test-components.html Added standalone HTML test page for UI components and React KnowledgeTab.
src/frontend/ui/* Minor formatting and JSX simplifications in badge, button, card, memory-graph components.
src/index.ts Refactored main plugin export to include actions, services, routes, tests, and schema; simplified init logic.
src/repositories/document-repository.ts New DocumentRepository class for CRUD and queries on documents.
src/repositories/fragment-repository.ts New FragmentRepository class for CRUD, embedding search, and queries on fragments.
src/repositories/index.ts New index module re-exporting repositories and types.
src/routes.ts Added/expanded API routes for update, bulk delete, advanced search, analytics, batch, export/import, and test components.
src/schema.ts New schema defining documents and knowledge_fragments tables and relations using Drizzle ORM.
src/service.ts Added feature flag, advanced search, batch operation, analytics, import/export methods, and improved metadata handling.
src/tests.ts Integrated new E2E tests and added tests for new tables and document loading.
src/types.ts Added comprehensive types for documents, fragments, batch ops, analytics, versioning, search options, export/import, and relationships.

Sequence Diagram(s)

Advanced Knowledge Search and Analytics Flow

sequenceDiagram
    participant User
    participant Frontend
    participant API (Routes)
    participant KnowledgeService
    participant Database

    User->>Frontend: Submits advanced search/analytics request
    Frontend->>API (Routes): Sends POST/GET to /knowledge/search/advanced or /analytics
    API (Routes)->>KnowledgeService: Calls advancedSearch()/getAnalytics()
    KnowledgeService->>Database: Query documents/fragments with filters/aggregation
    Database-->>KnowledgeService: Returns results/analytics data
    KnowledgeService-->>API (Routes): Returns structured results/analytics
    API (Routes)-->>Frontend: Responds with data
    Frontend-->>User: Displays search results/analytics
Loading

Batch Operation and Import/Export Flow

sequenceDiagram
    participant User
    participant Frontend
    participant API (Routes)
    participant KnowledgeService
    participant Database

    User->>Frontend: Initiates batch add/update/delete or import/export
    Frontend->>API (Routes): Sends POST to /knowledge/batch or /import/export
    API (Routes)->>KnowledgeService: Calls batchOperation()/importKnowledge()/exportKnowledge()
    KnowledgeService->>Database: Performs batch DB operations or queries
    Database-->>KnowledgeService: Returns operation results/data
    KnowledgeService-->>API (Routes): Returns summary or exported data
    API (Routes)-->>Frontend: Responds with operation result/data
    Frontend-->>User: Shows operation status or download
Loading

Poem

Hopping through code with a whisk of delight,
New knowledge features now take flight!
Advanced search, analytics, and batch galore,
Import, export, and tests—who could ask for more?
With every hop, the docs and schema grow—
This rabbit’s proud to see the knowledge flow!
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Post Copyable Unit Tests in Comment
  • Commit Unit Tests in branch 1.x-frontend-tests

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

🔭 Outside diff range comments (4)
package.json (1)

74-84: eslint is invoked but not declared in devDependencies.

The new lint:ci script relies on the eslint binary, yet eslint is not listed under devDependencies. CI will fail on a clean install.

"devDependencies": {
+  "eslint": "^9.1.0",
   "tsup": "8.5.0",
   ...
}
README.md (1)

618-636: index.ts will not exist in Vite build output

Vite emits transpiled JavaScript (e.g. index.[hash].js), never raw *.ts files.
Switching the fallback asset detection from .js to .ts will break the panel in production because the browser will 404.

- if (key.endsWith('.ts') || (value as any).file?.endsWith('.ts')) {
+ if (key.endsWith('.js') || (value as any).file?.endsWith('.js')) {

Likewise update the default jsFile var above.

src/actions.ts (1)

996-1013: Wrap per-case declarations to silence noSwitchDeclarations errors

Variables headers, rows etc. are declared directly inside case 'csv': without braces.
Many runtimes hoist those bindings across cases leading to Biome’s lint errors and potential shadowing bugs.

- case 'csv':
-   const headers = [...]
-   ...
+ case 'csv': {
+   const headers = [...]
+   ...
+   return [headers, ...rows].map(r => r.join(',')).join('\n')
+ }

Apply the same block‐scoping to every case with its own const/let.

src/routes.ts (1)

716-721: Serving .ts as application/javascript may break CSP / loaders

If you really are shipping TypeScript sources, browsers won’t execute them.
But if the files are transpiled your check should match .js, not .ts (same mis-extension as in README).

- if (assetPath.endsWith('.ts')) {
+ if (assetPath.endsWith('.js')) {
🧹 Nitpick comments (40)
.gitignore (1)

5-12: Suggestion: Enhance .gitignore for complete Cypress artifacts
It currently ignores cypress/screenshots but omits cypress/videos, which Cypress generates by default. Also, double-check that ignoring the entire docs folder is intentional, as it may hide important project documentation.

src/frontend/ui/badge.tsx (1)

20-21: Formatting consistency for JSX markup
This flattens the <span> element into a single line. For readability and consistency across UI components, align with your project's preferred JSX formatting (e.g., multi-line children).

src/frontend/ui/button.tsx (1)

22-26: Consistent formatting in component definitions
You’ve added a trailing comma after type and split the baseClasses string over two lines. To maintain uniform styling across UI components, standardize how you handle trailing commas and multi-line strings (e.g., match the Badge formatting conventions).

src/frontend/ui/card.tsx (1)

16-22: Optional: add displayName for easier React-DevTools inspection

Tiny DX win – assigning a displayName helps when debugging component trees.

 export function CardHeader({ children, className = '' }: CardProps) {
   return <div className={`flex flex-col space-y-1.5 p-6 ${className}`}>{children}</div>;
+}
+
+CardHeader.displayName = 'CardHeader';
 
 export function CardFooter({ children, className = '' }: CardProps) {
   return <div className={`flex items-center p-6 pt-0 ${className}`}>{children}</div>;
+}
+
+CardFooter.displayName = 'CardFooter';
@@
 export function CardDescription({ children, className = '' }: CardProps) {
   return <p className={`text-sm text-muted-foreground ${className}`}>{children}</p>;
 }
 
 export function CardContent({ children, className = '' }: CardProps) {
   return <div className={`p-6 pt-0 ${className}`}>{children}</div>;
 }
+
+CardDescription.displayName = 'CardDescription';
+CardContent.displayName    = 'CardContent';

Also applies to: 32-38

src/provider.ts (1)

28-35: Rename the map callback variable to avoid shadowing

The arrow-function parameter knowledge shadows the outer knowledge string, which is slightly confusing.

- firstFiveKnowledgeItems.map((knowledge) => `- ${knowledge.content.text}`).join('\n')
+ firstFiveKnowledgeItems.map((item) => `- ${item.content.text}`).join('\n')
cypress/e2e/simple-test.cy.ts (1)

3-5: Avoid network-flaky external dependencies in E2E tests

The test relies on https://example.cypress.io, which introduces a network dependency and potential flakiness (DNS issues, offline work, rate-limiting). Prefer hosting a minimal static fixture locally or stubbing the response with cy.intercept() to keep the test 100 % deterministic.

src/ctx-embeddings.ts (3)

58-63: Inconsistent quote style within the same literal block

Most prompt strings were migrated to single quotes, but PDF (line 62) still uses double quotes. This breaks the newly-introduced style consistency rule and will be flagged by eslint/prettier if enabled.

-  PDF: "You are a precision document augmentation tool. Your task is to expand a given PDF text chunk ...
+  PDF: 'You are a precision document augmentation tool. Your task is to expand a given PDF text chunk ...

292-296: Avoid re-assigning function parameters

minTokens and maxTokens are parameters and are mutated in-place. That pattern can be error-prone (TS-eslint no-param-reassign) and makes intent less clear.

-  if (chunkTokens > maxTokens * 0.7) {
-    maxTokens = Math.ceil(chunkTokens * 1.3);
-    minTokens = chunkTokens;
-  }
+  if (chunkTokens > maxTokens * 0.7) {
+    const adjustedMax = Math.ceil(chunkTokens * 1.3);
+    const adjustedMin = chunkTokens;
+    maxTokens = adjustedMax;
+    minTokens = adjustedMin;
+  }

(or assign to local let variables and keep parameters immutable).


345-350: contentType.includes() checks are case-sensitive

MIME strings can arrive in various casings (application/PDF, text/Markdown). Convert once to lower-case to avoid false negatives:

-  if (contentType) {
-    if (
-      contentType.includes('javascript') ||
+  if (contentType) {
+    const type = contentType.toLowerCase();
+    if (
+      type.includes('javascript') ||

Repeat for the other branches.

cypress.config.ts (1)

5-16: Hard-coded baseUrl and disabled chromeWebSecurity

  1. Hard-coding http://localhost:3000 ties the config to a single port. Make it configurable via CYPRESS_BASE_URL or process.env.PORT so parallel CI jobs can run without port collisions.
  2. chromeWebSecurity: false is powerful but disables same-origin protections. Double-check that it’s strictly required; otherwise leave it on for better parity with production.
package.json (1)

81-82: clean script is not cross-platform.

Using rm -rf breaks on Windows runners. Prefer rimraf, already a transitive dep of many toolchains.

- "clean": "rm -rf dist .turbo node_modules .turbo-tsconfig.json tsconfig.tsbuildinfo",
+ "clean": "rimraf dist .turbo node_modules .turbo-tsconfig.json tsconfig.tsbuildinfo",
src/__tests__/unit/fragment-repository.test.ts (1)

226-234: Large 1536-element arrays slow the test suite.

Cosine-similarity logic does not require production-sized embeddings. Switching to a shorter vector (e.g. length = 4) speeds execution while preserving behaviour.

No functional change, purely test-performance.

cypress/support/e2e.ts (2)

51-52: Remove useless empty export.

The file already imports ./commands; the extra export {} is redundant and flagged by Biome.

-// Prevent TypeScript errors
-export {};

55-59: Blindly suppressing all uncaught exceptions may hide real bugs.

Returning false for every uncaught:exception event prevents Cypress from signalling legitimate application errors. Consider filtering only the known React-dev warnings instead of blanket suppression.

Cypress.on('uncaught:exception', (err) => {
  if (err.message.includes('React')) return false; // ignore React warnings
});
src/__tests__/e2e/attachment-handling.test.ts (1)

32-34: Avoid any cast when calling service methods.

Casting service to any bypasses type-safety and may mask interface drift. Import the concrete KnowledgeService type or extend the service retrieval typing on IAgentRuntime.

const knowledgeService = service as KnowledgeService;
await knowledgeService.addKnowledge(testDoc);
src/__tests__/unit/schema.test.ts (2)

3-4: Remove unused imports to keep tests lint-clean

uuidv4 and the UUID type are not referenced anywhere in this file. Dead imports trigger no-unused-vars/no-unused-imports rules in most TS setups and slow incremental builds.

-import { v4 as uuidv4 } from 'uuid';
-import type { UUID } from '@elizaos/core';

31-63: Consider parameterising repetitive column-existence assertions

The next two blocks manually list 14+ columns each. If the schema evolves you’ll have to update three separate hard-coded lists, which is brittle and easy to forget.

A compact alternative:

const expectedDocCols = [
  'id','agentId','worldId','roomId','entityId','originalFilename',
  'contentType','content','fileSize','title','sourceUrl',
  'createdAt','updatedAt','metadata',
];

expectedDocCols.forEach((col) =>
  expect((documentsTable as any)[col]).toBeDefined()
);

Same pattern for knowledgeFragmentsTable. This keeps tests declarative and future-proof.

src/__tests__/e2e/knowledge-e2e.test.ts (1)

60-64: Avoid delete on process.env for perf & lint compliance

delete process.env.KNOWLEDGE_PATH triggers noDelete/performance warnings and de-opts the hidden class of the process.env object.
Assigning undefined achieves the same semantic effect without the overhead.

-        delete process.env.KNOWLEDGE_PATH;
+        process.env.KNOWLEDGE_PATH = undefined as any;
cypress/e2e/ui-components.cy.ts (1)

45-57: Minor: reduce DOM queries inside loops

Inside the variants / sizes loops Cypress queues a new command on every iteration, which can slow the suite.
Example optimisation:

cy.get(variants.map(v => `[data-testid="button-${v}"]`).join(','))
  .should('have.length', variants.length);

Optional but speeds up large UI matrices.

cypress/e2e/ui-components-simple.cy.ts (1)

27-33: Nit: use .then() instead of second query for click count

Querying click-count twice adds extra overhead. Capture the alias once:

cy.get('[data-testid="button-clickable"]').as('btn').click().click();
cy.get('[data-testid="click-count"]').should('contain', '2');

Small gain in speed & readability.

src/repositories/document-repository.ts (1)

91-98: Wrap destructive queries in try/catch to surface DB errors

delete() currently propagates raw driver errors upstream. A guarded wrapper lets the service layer distinguish “not found” from “DB unavailable”:

-const result = await this.db
-  .delete(documentsTable)
-  .where(eq(documentsTable.id, id))
-  .returning();
+let result: unknown[];
+try {
+  result = await this.db
+    .delete(documentsTable)
+    .where(eq(documentsTable.id, id))
+    .returning();
+} catch (err) {
+  // log & re-throw a repository-specific error
+  logger.error('Failed to delete document', err);
+  throw new RepositoryError('DELETE_DOCUMENT_FAILED', err);
+}

Helps the service layer produce meaningful 5xx vs 4xx responses.

src/__tests__/unit/document-repository.test.ts (1)

110-126: Add a positive test for findByRoom to close coverage gap

findByRoom is the only public method without a success-path assertion. A quick clone of the agent test with roomId would raise overall repository coverage to 100 %.

src/frontend/test-components.html (2)

224-241: Duplicate data-testid="table-row" values hinder reliable selectors

Multiple elements share the same data-testid making Cypress queries ambiguous and brittle. Give each row a unique id or rely on semantic selectors:

-<tr data-testid="table-row" …>
+<tr data-testid="table-row-1" …>-<tr data-testid="table-row" …>
+<tr data-testid="table-row-2" …>

Repeat for other duplicates (e.g. table-cell).


302-313: Guard against missing React global in module scope

When the page is served with strict CSP blocking inline scripts, the UMD bundles might not load, leaving ReactDOM undefined and throwing. A tiny guard prevents hard failure in CI environments:

-if (container) {
-  const root = ReactDOM.createRoot(container);
-  root.render(React.createElement(KnowledgeTab, { agentId: 'test-agent-123' }));
+if (container && window.ReactDOM?.createRoot) {
+  const root = window.ReactDOM.createRoot(container);
+  root.render(window.React.createElement(KnowledgeTab, { agentId: 'test-agent-123' }));
}

Keeps the test page usable even if CDN fetches fail.

src/__tests__/unit/action-chaining.test.ts (1)

421-440: Seed Math.random() to stabilise relevance-based ranking tests

Using raw Math.random() can create rare flaky failures when two documents receive identical relevance scores. Either seed a pseudo-random generator or sort deterministically:

-relevance: Math.random(),
+relevance: i / 10, // deterministic ascending relevance

Improves test repeatability across CI runs.

src/__tests__/unit/advanced-features.test.ts (2)

30-32: generateMockUuid may collide for suffixes ≥ 10 000

The helper always pads to 4 digits, so generateMockUuid(12345) yields the same value as generateMockUuid(2345).
Pad to 12 digits (like elsewhere) or stringify with a counter to avoid accidental ID collisions in larger suites.


346-433: Batch / import tests bypass the public API

These tests call mockKnowledgeService.batchOperation and importKnowledge directly instead of exercising the exported actions or HTTP layer.
That doesn’t protect you against wiring mistakes in the real handlers/routes. Consider adding end-to-end tests (or at least invoking the corresponding actions) so regressions in the glue code are caught.

cypress/support/commands.ts (2)

45-48: Dead code – blob and file are never used

The created Blob and File objects are immediately discarded.
Remove them to reduce noise and keep the command lean.


21-23: Alias should include @

cy.wait() expects the alias string to start with @. Enforce it or prepend automatically to avoid silent time-outs.

-cy.wait(alias, { timeout });
+cy.wait(alias.startsWith('@') ? alias : `@${alias}`, { timeout });
src/schema.ts (2)

28-34: Storing large binaries in text column is risky

Base-64 PDFs can exceed the Postgres text practical limits and bloat tables / WAL.
Prefer bytea (binary) or move file blobs to object storage and keep only a reference URL in the row.


33-34: updatedAt never updates

defaultNow() sets the timestamp only on insert. Add a trigger or update hook so modifications refresh updatedAt.

src/repositories/fragment-repository.ts (2)

112-122: Double-computing cosine similarity is wasteful

You already sort by similarity in SQL; recomputing it in JS for every row adds O(k·d) work.
Return the similarity from SQL (e.g., 1 - cosine_distance AS similarity) and reuse it.


90-93: Unbounded limit & low threshold invite heavy queries

Consider clamping limit (e.g., ≤ 100) and validating threshold ∈ [0,1] to protect the DB from expensive vector scans.

src/__tests__/unit/action.test.ts (1)

670-689: Re-using the same id in the loop reduces test isolation

Every iteration creates a Memory with the same UUID (73).
Use the loop index to generate distinct IDs to avoid accidental state leakage when future code relies on uniqueness.

src/__tests__/e2e/advanced-features-e2e.test.ts (1)

214-218: Imported doc is not removed during cleanup

neural-networks.txt isn’t matched by the current filename filters, leaving data behind for later tests. Add that filename (or a deterministic tag) to the cleanup predicate.

src/frontend/ui/memory-graph.tsx (1)

105-111: Heavy re-processing on every render

processGraphData runs inside useEffect and always triggers setGraphData, even when the derived structure is identical. Wrap it in useMemo (or diff check) to cut unnecessary state updates on large graphs.

src/__tests__/e2e/startup-loading.test.ts (1)

22-24: Avoid using project-root docs folder in tests

Writing directly to ${cwd}/docs risks clobbering real documentation. Use fs.mkdtempSync(path.join(os.tmpdir(), 'docs-')) for an isolated workspace.

src/config.ts (1)

24-33: Boolean env parsing is too strict

Only the literal string 'true' is accepted. Consider treating '1', 'yes', or upper-case variants as truthy for better DX.

src/tests.ts (1)

483-484: Prefer process.env.KEY = undefined over delete

delete process.env.* incurs a de-opt in V8 and triggers noDelete lint errors. Assign undefined instead.

-delete process.env.KNOWLEDGE_PATH;
+process.env.KNOWLEDGE_PATH = undefined;

Also applies to: 519-520, 1241-1242

src/service.ts (1)

904-915: Retrieving 10 000 fragments unbounded may exhaust memory

count: 10000 is an arbitrary hard-coded limit.
For large agents this could load millions of KB into RAM and block the event loop.
Consider:

  1. Stream results or paginate.
  2. Make the limit configurable (KNOWLEDGE_ANALYTICS_MAX_SAMPLES).
📜 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

📥 Commits

Reviewing files that changed from the base of the PR and between 64cc040 and 770abcb.

📒 Files selected for processing (47)
  • .gitignore (1 hunks)
  • README.md (4 hunks)
  • __tests__/action.test.ts (0 hunks)
  • cypress.config.ts (1 hunks)
  • cypress/e2e/simple-test.cy.ts (1 hunks)
  • cypress/e2e/ui-components-simple.cy.ts (1 hunks)
  • cypress/e2e/ui-components.cy.ts (1 hunks)
  • cypress/support/commands.ts (1 hunks)
  • cypress/support/e2e.ts (1 hunks)
  • images/README.md (0 hunks)
  • package.json (3 hunks)
  • src/__tests__/e2e/advanced-features-e2e.test.ts (1 hunks)
  • src/__tests__/e2e/attachment-handling.test.ts (1 hunks)
  • src/__tests__/e2e/knowledge-e2e.test.ts (1 hunks)
  • src/__tests__/e2e/startup-loading.test.ts (1 hunks)
  • src/__tests__/unit/action-chaining.test.ts (1 hunks)
  • src/__tests__/unit/action.test.ts (1 hunks)
  • src/__tests__/unit/advanced-features.test.ts (1 hunks)
  • src/__tests__/unit/document-repository.test.ts (1 hunks)
  • src/__tests__/unit/fragment-repository.test.ts (1 hunks)
  • src/__tests__/unit/schema.test.ts (1 hunks)
  • src/__tests__/unit/update-knowledge.test.ts (1 hunks)
  • src/__tests__/unit/utils.test.ts (1 hunks)
  • src/actions.ts (6 hunks)
  • src/config.ts (1 hunks)
  • src/ctx-embeddings.ts (11 hunks)
  • src/docs-loader.ts (2 hunks)
  • src/document-processor.ts (4 hunks)
  • src/frontend/index.tsx (1 hunks)
  • src/frontend/test-components.html (1 hunks)
  • src/frontend/ui/badge.tsx (1 hunks)
  • src/frontend/ui/button.tsx (1 hunks)
  • src/frontend/ui/card.tsx (2 hunks)
  • src/frontend/ui/memory-graph.tsx (1 hunks)
  • src/index.ts (1 hunks)
  • src/provider.ts (2 hunks)
  • src/repositories/document-repository.ts (1 hunks)
  • src/repositories/fragment-repository.ts (1 hunks)
  • src/repositories/index.ts (1 hunks)
  • src/routes.ts (8 hunks)
  • src/schema.ts (1 hunks)
  • src/service.ts (8 hunks)
  • src/tests.ts (33 hunks)
  • src/types.ts (4 hunks)
  • tsconfig.json (1 hunks)
  • tsup.config.ts (1 hunks)
  • vitest.config.ts (1 hunks)
💤 Files with no reviewable changes (2)
  • images/README.md
  • tests/action.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/__tests__/unit/fragment-repository.test.ts (2)
src/repositories/fragment-repository.ts (1)
  • FragmentRepository (20-212)
src/types.ts (1)
  • KnowledgeFragment (165-177)
src/__tests__/unit/schema.test.ts (1)
src/schema.ts (3)
  • documentsTable (18-43)
  • knowledgeFragmentsTable (48-81)
  • knowledgeSchema (100-103)
src/__tests__/e2e/startup-loading.test.ts (2)
src/service.ts (1)
  • KnowledgeService (44-1115)
src/docs-loader.ts (1)
  • loadDocsFromPath (42-135)
src/__tests__/e2e/advanced-features-e2e.test.ts (1)
src/service.ts (1)
  • KnowledgeService (44-1115)
src/__tests__/e2e/knowledge-e2e.test.ts (2)
src/service.ts (1)
  • KnowledgeService (44-1115)
src/docs-loader.ts (1)
  • loadDocsFromPath (42-135)
src/__tests__/unit/action.test.ts (2)
src/service.ts (1)
  • KnowledgeService (44-1115)
src/actions.ts (2)
  • processKnowledgeAction (21-315)
  • searchKnowledgeAction (320-461)
src/__tests__/unit/advanced-features.test.ts (2)
src/service.ts (1)
  • KnowledgeService (44-1115)
src/actions.ts (3)
  • advancedSearchAction (466-590)
  • knowledgeAnalyticsAction (595-687)
  • exportKnowledgeAction (692-787)
src/routes.ts (1)
src/service.ts (2)
  • KnowledgeService (44-1115)
  • batchOperation (825-894)
🪛 Biome (1.9.4)
src/__tests__/unit/fragment-repository.test.ts

[error] 146-146: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 168-168: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 192-192: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 213-213: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 239-239: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 269-269: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 361-361: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 378-378: Do not add then to an object.

(lint/suspicious/noThenProperty)


[error] 393-393: Do not add then to an object.

(lint/suspicious/noThenProperty)

cypress/support/e2e.ts

[error] 51-52: This empty export is useless because there's another export or import.

This import makes useless the empty export.

Safe fix: Remove this useless empty export.

(lint/complexity/noUselessEmptyExport)

src/__tests__/e2e/knowledge-e2e.test.ts

[error] 63-64: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

src/service.ts

[error] 852-852: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 1000-1000: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 1001-1007: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 1038-1038: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 1053-1053: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 1054-1054: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 1074-1074: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

src/tests.ts

[error] 483-483: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 519-519: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 1241-1241: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🪛 LanguageTool
README.md

[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...attachments** - Share links to documents and they'll be downloaded and processed - *...

(COMMA_COMPOUND_SENTENCE)

🔇 Additional comments (17)
src/frontend/index.tsx (1)

5-5: Verify TypeScript & bundler resolution for .js import
Confirm that your TS config (allowJs, moduleResolution) and bundler handle explicit .js extensions. Otherwise consider using an extension-agnostic import or pointing to the original .tsx source.

src/__tests__/unit/utils.test.ts (1)

2-2: Corrected import path
The import from '../../utils' now correctly resolves to src/utils.ts, fixing the previous module resolution error.

tsup.config.ts (1)

18-19: Externalising the SQL plugin is the right move

Marking @elizaos/plugin-sql as external keeps the final bundle slim and avoids pulling heavy DB drivers into the build. No issues spotted.

src/docs-loader.ts (1)

107-112: Verify metadata is part of AddKnowledgeOptions

metadata is newly added here. If the type in types.ts hasn’t been updated, TypeScript will complain. Please confirm the interface has been extended.

src/repositories/index.ts (1)

1-6: Clean central re-export

Makes downstream imports shorter and clearer. Looks good.

tsconfig.json (1)

19-22: skipLibCheck may hide real type errors

Setting skipLibCheck: true speeds up compilation but suppresses errors in dependency .d.ts files—those often surface legitimate issues. Consider enabling it only in CI cache misses or large monorepos, or add a comment explaining the trade-off.

vitest.config.ts (1)

8-9: Globbing pattern may not match any tests

'src/**/*.{test,spec}.{js,ts,jsx,tsx}' is interpreted literally by picomatch: only files ending with . {test,spec}.{js…} will match. The usual pattern is 'src/**/*.{test,spec}.{js,ts,jsx,tsx}' or two-brace form 'src/**/*.@(test|spec).@(js|ts|jsx|tsx)'.

Verify that vitest actually discovers your unit tests; otherwise adjust the pattern.

src/__tests__/unit/fragment-repository.test.ts (2)

82-88: create input violates repository contract.

FragmentRepository.create expects Omit<KnowledgeFragment, 'id' | 'createdAt'>, yet the test passes a full KnowledgeFragment (it includes id and createdAt). Type-checking is bypassed only because the value is stored in a variable first. Drop the forbidden fields to align with actual usage and avoid misleading tests.


48-76: Promise-like mock should return a real Promise, not an object with .then.

Adding a then property to a plain object triggers Biome’s noThenProperty rule and can mask async mistakes. Wrap the chain in Promise.resolve instead:

- mockChain.returning.mockImplementation(() => {
-   return Promise.resolve(mockChain._returnValue || []);
- });
+ mockChain.returning.mockImplementation(async () => mockChain._returnValue ?? []);

Same pattern can be applied to the various selectChain helpers below.

Likely an incorrect or invalid review comment.

cypress/support/e2e.ts (1)

24-49: Only types are declared for the custom commands; ensure implementations exist.

visitKnowledgePanel, uploadKnowledgeFile, searchKnowledge, and deleteDocument are typed here but never defined in this module. Verify that cypress/support/commands.ts actually adds these commands, otherwise runtime failures will occur.

src/__tests__/unit/update-knowledge.test.ts (1)

220-222: Test expectation allows silent failures.

deleteMemory of a non-existent document currently counts as “success”. If the desired contract is to report an error when the document is missing, this assertion should be 2 (only existing docs), not 3.

Please confirm the intended behaviour.

src/document-processor.ts (1)

564-566: Defensive null-check before concatenating context

llmResponse may already be a string; after the fallback you could still end up with undefined if the provider returns {}.
Consider:

const generatedContext = typeof llmResponse === 'string'
  ? llmResponse
  : (llmResponse as any)?.text ?? '';

Prevents getChunkWithContext from receiving undefined.

src/__tests__/e2e/knowledge-e2e.test.ts (1)

138-155: Potential race: ensure fragments cascaded before assertion

The service might delete the document async and queue fragment cleanup.
A short poll / await would make this test less flaky:

await retryAssert(async () => {
  const remaining = await service.getMemories({ tableName: 'knowledge', count: 100 });
  expect(remaining.some(f => (f.metadata as any)?.documentId === testDocument.id)).toBe(false);
}, 5, 200); // 5 tries, 200 ms apart

Consider adding such helper to the test utils.

src/__tests__/e2e/advanced-features-e2e.test.ts (1)

190-198: Verify empty-query behaviour

advancedSearch({ query: '' … }) may bypass embedding generation or throw, depending on implementation. Confirm it cannot return inconsistent similarity scores.

src/tests.ts (1)

169-177: metadata.type comparison may mismatch enum value

MemoryType.FRAGMENT / DOCUMENT are enums; stored metadata.type may be lower-case strings ('fragment', 'document'). The current filter could silently drop records in tests.

src/actions.ts (1)

290-297: results.length is always 0 for path/text uploads

When no attachments are present the results array isn’t populated, yet it’s still returned to callers.
Return a more meaningful object (e.g. undefined) or populate it with the single processed file/text to keep the schema stable.

src/types.ts (1)

336-355: Nice extensibility!

The batch-operation and analytics types look solid and future-proof.

Comment on lines 100 to 112
clientDocumentId: createUniqueUuid((service as any).runtime, `docs-${fileName}-${Date.now()}`) as UUID,
contentType,
originalFilename: fileName,
worldId: worldId || agentId,
content,
roomId: agentId,
entityId: agentId,
metadata: {
source: 'docs',
path: filePath,
loadedAt: new Date().toISOString(),
},
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

createUniqueUuid is given an object, producing corrupt IDs

createUniqueUuid expects a UUID seed string. Passing (service as any).runtime (an object) will stringify to [object Object], breaking UUID semantics and potentially violating FK constraints.

- clientDocumentId: createUniqueUuid((service as any).runtime, `docs-${fileName}-${Date.now()}`) as UUID,
+ const runtimeId = (service as any).runtime?.id ?? agentId;
+ clientDocumentId: createUniqueUuid(runtimeId, `docs-${fileName}-${Date.now()}`) as UUID,

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/docs-loader.ts around lines 100 to 112, the call to createUniqueUuid
incorrectly passes (service as any).runtime, which is an object, instead of a
UUID seed string. To fix this, extract or generate a proper UUID string from the
runtime object or use a valid string identifier as the seed argument for
createUniqueUuid to ensure it produces valid UUIDs and maintains FK integrity.

Comment on lines 7 to 9
export class DocumentRepository {
constructor(private db: PostgresJsDatabase<any>) {}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Parameterise repository for better type-safety

PostgresJsDatabase<any> throws away the table schema information, forcing all downstream rows to any. Drizzle already provides strongly-typed row inference – keep it:

-export class DocumentRepository {
-  constructor(private db: PostgresJsDatabase<any>) {}
+export class DocumentRepository<TSchema = typeof documentsTable> {
+  constructor(private readonly db: PostgresJsDatabase<TSchema>) {}

This preserves column names & narrows row in mapToDocument, surfacing compile-time errors instead of silent any.

📝 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.

Suggested change
export class DocumentRepository {
constructor(private db: PostgresJsDatabase<any>) {}
export class DocumentRepository<TSchema = typeof documentsTable> {
constructor(private readonly db: PostgresJsDatabase<TSchema>) {}
🤖 Prompt for AI Agents
In src/repositories/document-repository.ts around lines 7 to 9, the constructor
parameter uses PostgresJsDatabase<any>, which loses table schema type
information and results in rows typed as any. To fix this, parameterize the
DocumentRepository class and the db parameter with the appropriate table schema
type from Drizzle to preserve strong typing. This will ensure that downstream
row objects, such as in mapToDocument, have accurate types reflecting column
names and types, improving type safety and compile-time error detection.

Comment on lines 64 to 66
await new Promise((resolve) => setTimeout(resolve, 2000));
console.log('✓ Waited for startup document loading');
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fixed 2 s delay may still race on slow CI

Wait for service.getAnalytics().totalDocuments >= expected (with timeout) instead of setTimeout(2000).

🤖 Prompt for AI Agents
In src/__tests__/e2e/startup-loading.test.ts around lines 64 to 66, replace the
fixed 2-second delay using setTimeout with a loop or polling mechanism that
waits until service.getAnalytics().totalDocuments is greater than or equal to
the expected value, implementing a timeout to avoid infinite waiting. This
ensures the test waits for the actual condition rather than a fixed delay,
preventing race conditions on slow CI environments.

lalalune and others added 2 commits June 18, 2025 23:11
- KnowledgeService routes were failing because services were being registered with class name 'KnowledgeService'
  but retrieved with serviceType 'knowledge'
- Runtime service registration uses serviceName || className as the key, but getService() uses serviceName
- Added static serviceName property to match serviceType for:
  - KnowledgeService: serviceName = 'knowledge'
  - ShellService: serviceName = 'SHELL'
  - EnvManagerService: serviceName = 'ENV_MANAGER'
- This ensures service registration and retrieval use the same key

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (2)
src/config.ts (1)

42-44: Proper NaN guard implementation.

The code correctly addresses the past review comment about NaN handling by checking Number.isNaN() and providing safe default values. This prevents runtime issues when environment variables contain non-numeric values.

src/service.ts (1)

846-873: Fix switch clause variable declaration issue.

The memory variable is declared directly in the update case clause without block scoping, which can cause variable shadowing issues across switch cases.

Apply this diff to wrap the case clause in block scope:

-    case 'update':
-      if (!item.id || !item.metadata) throw new Error('Missing id or metadata for update');
-      // Update document metadata
-      const memory = await this.runtime.getMemoryById(item.id as UUID);
-      if (memory) {
-        await this.runtime.updateMemory({
-          ...memory,
-          id: memory.id!, // Ensure id is defined
-          metadata: { ...memory.metadata, ...item.metadata } as MemoryMetadata,
-        });
-        result = { updated: true };
-      } else {
-        throw new Error('Document not found');
-      }
-      break;
+    case 'update': {
+      if (!item.id || !item.metadata) throw new Error('Missing id or metadata for update');
+      // Update document metadata
+      const memory = await this.runtime.getMemoryById(item.id as UUID);
+      if (memory) {
+        await this.runtime.updateMemory({
+          ...memory,
+          id: memory.id!, // Ensure id is defined
+          metadata: { ...memory.metadata, ...item.metadata } as MemoryMetadata,
+        });
+        result = { updated: true };
+      } else {
+        throw new Error('Document not found');
+      }
+      break;
+    }
🧹 Nitpick comments (3)
cypress.config.ts (2)

25-27: Consider differentiating timeout values based on operation type.

All timeouts are set to 10 seconds, which may not be optimal for all scenarios. Consider if different operations need different timeout values:

  • Command timeout: For user interactions (current: 10s)
  • Request timeout: For API calls (might need shorter: 5-8s)
  • Response timeout: For server responses (might need longer: 15-20s)
-    defaultCommandTimeout: 10000,
-    requestTimeout: 10000,
-    responseTimeout: 10000,
+    defaultCommandTimeout: 10000,
+    requestTimeout: 8000,
+    responseTimeout: 15000,

17-29: Consider adding retry configuration for improved test stability.

The e2e configuration is comprehensive, but adding retry configuration could help with flaky tests common in e2e scenarios.

     e2e: {
       baseUrl: process.env.CYPRESS_baseUrl || 'http://localhost:3000',
       supportFile: 'cypress/support/e2e.ts',
       specPattern: 'cypress/e2e/**/*.cy.{js,jsx,ts,tsx}',
+      retries: {
+        runMode: 2,
+        openMode: 0,
+      },
       video: false,
src/tests.ts (1)

483-483: Replace delete operator with undefined assignment for better performance.

The delete operator can impact performance. Use undefined assignment instead as suggested by the static analysis tool.

Apply this diff to improve performance:

-        delete process.env.KNOWLEDGE_PATH;
+        process.env.KNOWLEDGE_PATH = undefined;

Apply the same pattern to all three occurrences.

Also applies to: 519-519, 1243-1243

📜 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

📥 Commits

Reviewing files that changed from the base of the PR and between 770abcb and 3ac4cac.

📒 Files selected for processing (25)
  • cypress.config.ts (1 hunks)
  • cypress/e2e/simple-test.cy.ts (1 hunks)
  • cypress/support/component-index.html (1 hunks)
  • cypress/support/component.ts (1 hunks)
  • cypress/tsconfig.json (1 hunks)
  • package.json (2 hunks)
  • src/__tests__/e2e/advanced-features-e2e.test.ts (1 hunks)
  • src/__tests__/e2e/attachment-handling.test.ts (1 hunks)
  • src/__tests__/e2e/startup-loading.test.ts (1 hunks)
  • src/__tests__/unit/action-chaining.test.ts (1 hunks)
  • src/__tests__/unit/action.test.ts (1 hunks)
  • src/__tests__/unit/advanced-features.test.ts (1 hunks)
  • src/__tests__/unit/update-knowledge.test.ts (1 hunks)
  • src/__tests__/unit/utils.test.ts (2 hunks)
  • src/actions.ts (6 hunks)
  • src/config.ts (1 hunks)
  • src/docs-loader.ts (2 hunks)
  • src/document-processor.ts (5 hunks)
  • src/frontend/ui/memory-graph.tsx (1 hunks)
  • src/index.ts (1 hunks)
  • src/repositories/document-repository.ts (1 hunks)
  • src/routes.ts (8 hunks)
  • src/service.ts (7 hunks)
  • src/tests.ts (33 hunks)
  • src/utils.ts (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • cypress/tsconfig.json
  • cypress/support/component-index.html
  • cypress/support/component.ts
🚧 Files skipped from review as they are similar to previous changes (14)
  • src/tests/unit/utils.test.ts
  • src/docs-loader.ts
  • src/tests/unit/update-knowledge.test.ts
  • src/tests/e2e/attachment-handling.test.ts
  • src/document-processor.ts
  • src/index.ts
  • cypress/e2e/simple-test.cy.ts
  • src/tests/unit/advanced-features.test.ts
  • package.json
  • src/tests/unit/action.test.ts
  • src/tests/e2e/advanced-features-e2e.test.ts
  • src/repositories/document-repository.ts
  • src/tests/e2e/startup-loading.test.ts
  • src/frontend/ui/memory-graph.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/service.ts

[error] 855-855: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

src/tests.ts

[error] 483-483: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 519-519: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 1243-1243: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🔇 Additional comments (21)
cypress.config.ts (4)

1-3: LGTM! Clean configuration structure.

The import and export structure follows Cypress best practices using the defineConfig function.


4-16: Component testing configuration looks solid.

The React + Vite setup is appropriate and the file paths follow Cypress conventions. The artifact management (screenshots enabled, videos disabled) strikes a good balance between debugging capability and storage efficiency.


18-18: Good practice using environment variable for base URL.

This allows for flexible configuration across different environments while providing a sensible default.


28-28: Document the reason for disabling Chrome web security.

Disabling chromeWebSecurity can introduce security risks and should be documented. Please ensure this is necessary for your cross-origin testing scenarios.

Consider adding a comment explaining why this is needed:

+    // Disabled to allow cross-origin requests in testing environment
     chromeWebSecurity: false,

If cross-origin testing isn't required, consider removing this setting for better security:

-    chromeWebSecurity: false,
src/tests.ts (3)

1122-1179: Well-structured test for new tables feature.

The test comprehensively covers the new tables functionality including document creation, fragment verification, and cascade delete operations. The test setup and cleanup are properly handled.


1182-1246: Comprehensive startup document loading test with proper cleanup.

The test properly creates a temporary directory, populates it with test files, and verifies document loading. The cleanup logic in the finally block ensures no test artifacts remain.


1248-1264: Clean integration of E2E tests.

The integration of external E2E test modules using their name and fn properties maintains clean separation of concerns while enabling comprehensive test coverage.

src/utils.ts (1)

423-424: Good stylistic improvement for operator precedence clarity.

Adding parentheses around the logical expression makes the operator precedence explicit and improves code readability without changing functionality.

src/__tests__/unit/action-chaining.test.ts (1)

1-594: Excellent comprehensive test suite for action chaining.

This test file demonstrates strong testing practices with:

  • Proper mocking of dependencies
  • Comprehensive coverage of success and failure scenarios
  • Multi-step action chain testing
  • Clear test organization and readable assertions
  • Good error propagation testing

The test coverage will help ensure reliable action chaining functionality.

src/config.ts (1)

22-105: Simplified configuration validation improves maintainability.

The rewrite from Zod-based validation to explicit parameter extraction with clear defaults simplifies the codebase while maintaining necessary validation. The approach is more direct and easier to debug.

src/actions.ts (4)

128-187: Robust attachment processing with proper error handling.

The attachment processing logic properly handles different attachment types (URL vs data), includes appropriate error handling for individual attachments, and maintains detailed results tracking. The metadata enrichment with source and message ID aids traceability.


474-603: Well-implemented advanced search action with natural language parsing.

The advanced search action demonstrates good natural language processing to extract filters, date ranges, and sorting preferences. The implementation properly handles various search criteria and provides structured results.


608-706: Comprehensive analytics action with clear formatting.

The analytics action provides valuable insights about the knowledge base with clear formatting. The response includes key metrics like document counts, storage size, and content type distribution in a user-friendly format.


157-157: Verify UUID generation function.

The code uses stringToUuid() to generate document IDs. Ensure this function produces valid UUIDs and handles collisions appropriately for production use.

#!/bin/bash
# Search for stringToUuid function implementation to verify UUID generation
ast-grep --pattern 'function stringToUuid($_) {
  $$$
}'

# Also search for its usage patterns
rg -A 3 "stringToUuid"
src/service.ts (3)

53-56: LGTM! Good implementation of feature flag pattern.

The feature flag approach allows for safe rollout of new table implementations while maintaining backward compatibility.


724-821: LGTM! Well-implemented advanced search functionality.

The advanced search method provides comprehensive filtering, sorting, and pagination capabilities with proper error handling and type safety.


904-957: LGTM! Robust analytics implementation.

The analytics method provides useful insights into the knowledge base with proper error handling and fallbacks for missing data.

src/routes.ts (4)

314-408: LGTM! Comprehensive update handler implementation.

The update handler properly supports both file replacement and metadata-only updates with appropriate cleanup and error handling.


920-967: LGTM! Efficient bulk delete implementation.

The bulk delete handler processes operations in parallel with proper error aggregation and detailed result reporting.


1175-1198: LGTM! Route paths are correctly configured.

The route paths are properly relative to the base mount path, avoiding the duplication issue mentioned in previous reviews.


617-617: Question: Serving TypeScript files instead of compiled JavaScript.

The code references .ts file extensions for frontend assets, which is unusual in production. Typically, TypeScript files should be compiled to JavaScript before serving.

Verify if this is intentional for development purposes or if these should be .js extensions for production builds:

#!/bin/bash
# Check if there are actual .ts files in the dist/assets directory
find . -name "dist" -type d -exec find {} -name "*.ts" \; 2>/dev/null
find . -name "dist" -type d -exec find {} -name "*.js" \; 2>/dev/null | head -10

# Check build configuration
fd -t f "vite.config" -x cat {}
fd -t f "tsconfig" -x cat {} | head -20

Also applies to: 632-632, 716-716

Comment on lines +711 to 803
export const exportKnowledgeAction: Action = {
name: 'EXPORT_KNOWLEDGE',
description: 'Export knowledge base to various formats',

similes: ['export knowledge', 'download knowledge', 'backup knowledge', 'save knowledge to file'],

examples: [
[
{
name: 'user',
content: {
text: 'Export my knowledge base as JSON',
},
},
{
name: 'assistant',
content: {
text: "I'll export your knowledge base as JSON.",
actions: ['EXPORT_KNOWLEDGE'],
},
},
],
],

validate: async (runtime: IAgentRuntime, message: Memory) => {
const text = message.content.text?.toLowerCase() || '';
const hasExportKeywords = ['export', 'download', 'backup', 'save'].some((k) =>
text.includes(k)
);
const hasKnowledgeWord = text.includes('knowledge');

const service = runtime.getService(KnowledgeService.serviceType);
return !!(service && hasExportKeywords && hasKnowledgeWord);
},

handler: async (
runtime: IAgentRuntime,
message: Memory,
state?: State,
options?: { [key: string]: unknown },
callback?: HandlerCallback
): Promise<ActionResult> => {
try {
const service = runtime.getService<KnowledgeService>(KnowledgeService.serviceType);
if (!service) {
throw new Error('Knowledge service not available');
}

const text = message.content.text || '';

// Detect format
let format: 'json' | 'csv' | 'markdown' = 'json';
if (text.includes('csv')) format = 'csv';
else if (text.includes('markdown') || text.includes('md')) format = 'markdown';

const exportData = await service.exportKnowledge({
format,
includeMetadata: true,
includeFragments: false,
});

// In a real implementation, this would save to a file or return a download link
// For now, we'll just return a preview
const preview = exportData.substring(0, 500) + (exportData.length > 500 ? '...' : '');

const response: Content = {
text: `✅ Knowledge base exported as ${format.toUpperCase()}. Size: ${(exportData.length / 1024).toFixed(2)} KB\n\nPreview:\n\`\`\`${format}\n${preview}\n\`\`\``,
};

if (callback) {
await callback(response);
}

return {
data: {
format,
size: exportData.length,
content: exportData,
},
text: response.text,
};
} catch (error) {
logger.error('Error in EXPORT_KNOWLEDGE:', error);
const errorResponse: Content = {
text: `Error exporting knowledge: ${error instanceof Error ? error.message : 'Unknown error'}`,
};
if (callback) {
await callback(errorResponse);
}
return { data: { error: String(error) }, text: errorResponse.text };
}
},
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Export functionality with security considerations.

The export action supports multiple formats and includes size information. Consider adding access controls and rate limiting for export operations in production environments to prevent data exfiltration or abuse.

Consider implementing:

  • Access control checks before allowing exports
  • Rate limiting to prevent abuse
  • File size limits for large knowledge bases
  • Audit logging for export operations
🤖 Prompt for AI Agents
In src/actions.ts from lines 711 to 803, the exportKnowledgeAction lacks
security measures. Add access control checks to verify user permissions before
proceeding with export. Implement rate limiting to restrict the frequency of
export requests. Enforce file size limits to prevent exporting excessively large
knowledge bases. Include audit logging to record export operations for
monitoring and compliance.

/**
* Documents table - stores the main document metadata and content
*/
export const documentsTable = pgTable(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is this supposed to work with plugin-mysql?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants