Skip to content

BZ-45257: fix: migrated to cloud hosted mem0 - #235

Merged
murdore merged 1 commit into
juspay:releasefrom
cmd-err:BZ-45257-cloud-hosted-mem-0-support
Nov 19, 2025
Merged

murdore merged 1 commit into
juspay:releasefrom
cmd-err:BZ-45257-cloud-hosted-mem-0-support

Conversation

@cmd-err

@cmd-err cmd-err commented Nov 12, 2025 •

Copy link
Copy Markdown
Contributor

Pull Request

Description

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🧹 Code refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🧪 Test coverage improvement
  • 🔧 Build/CI configuration change

Related Issues

  • Fixes #
  • Related to #

Changes Made

AI Provider Impact

  • OpenAI
  • Anthropic
  • Google AI/Vertex
  • AWS Bedrock
  • Azure OpenAI
  • Hugging Face
  • Ollama
  • Mistral
  • All providers
  • No provider-specific changes

Component Impact

  • CLI
  • SDK
  • MCP Integration
  • Streaming
  • Tool Calling
  • Configuration
  • Documentation
  • Tests

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • E2E tests added/updated
  • Manual testing performed
  • All existing tests pass

Test Environment

  • OS:
  • Node.js version:
  • Package manager:

Performance Impact

  • No performance impact
  • Performance improvement
  • Minor performance impact (acceptable)
  • Significant performance impact (needs discussion)

Breaking Changes

Screenshots/Demo

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Additional Notes

Summary by CodeRabbit

  • New Features

    • Memory now integrates with the Mem0 cloud API for remote memory handling.
  • Improvements

    • Memory setup simplified to an API-key-only configuration.
    • Better cleanup of memory resources, processes, and error paths.
    • Local memory fallback removed — initialization skips when no API key provided.
  • Bug Fixes

    • Updated memory interactions and compatibility with the cloud memory service.

Copilot AI review requested due to automatic review settings November 12, 2025 04:14
@coderabbitai

coderabbitai Bot commented Nov 12, 2025 •

Copy link
Copy Markdown

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Migrates Mem0 integration from an OSS client to the Mem0 cloud API client (MemoryClient), replaces MemoryConfig with Mem0Config (API-key based), updates NeuroLink memory calls and helpers, adjusts example scripts and package overrides, and removes the prior Mem0Memory public interface and some internal defaults.

Changes

Cohort / File(s) Summary
Package overrides
package.json
Added/adjusted pnpm overrides (glob range fix) and fixed trailing-comma JSON syntax.
Example: real memory test
scripts/examples/real-memory-test.js
Switched example to API-key based Mem0 config, updated userId usage, added explicit cleanupResources, exported realMemoryTest.
Mem0 initializer & config
src/lib/memory/mem0Initializer.ts
Replaced OSS Memory client with cloud MemoryClient, added exported Mem0Config { apiKey }, initializer returns `MemoryClient
NeuroLink memory integration
src/lib/neurolink.ts
Replaced Mem0 types with MemoryClient and Mem0Config, added private helpers extractMemoryContext and storeConversationTurn, updated calls to use new memory API shapes (user_id, array results) and static initializer import.
Conversation types
src/lib/types/conversation.ts
Replaced MemoryConfig reference with exported Mem0Config; updated ConversationMemoryConfig.mem0Config type.
Utilities: removed Mem0 interface
src/lib/types/utilities.ts
Removed exported Mem0Memory interface and its method declarations from public types.
MCP defaults
src/lib/mcp/mcpClientFactory.ts
Removed default capability keys (tools, resources, prompts) from DEFAULT_CAPABILITIES.

Sequence Diagram(s)

sequenceDiagram
  participant NL as NeuroLink
  participant MI as mem0Initializer
  participant MC as MemoryClient (mem0 cloud)

  rect rgba(135,206,235,0.12)
    NL->>MI: ensureMem0Ready(mem0Config)
    MI-->>NL: MemoryClient (if apiKey present) / null
  end

  alt Mem0 available
    NL->>MC: search(user_id, query)
    MC-->>NL: [results] 
    NL->>NL: extractMemoryContext(results)
    NL->>MC: add({...}, { user_id })
    MC-->>NL: add response
  else Mem0 disabled
    NL-->>NL: skip memory flow
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas to focus:
    • src/lib/memory/mem0Initializer.ts — new client initialization, API-key gating, removed fallback.
    • src/lib/neurolink.ts — updated flows, helpers, and memory call signatures.
    • src/lib/types/utilities.ts & src/lib/types/conversation.ts — public type removals/changes and propagation.
    • scripts/examples/real-memory-test.js — exported function and cleanup behavior.

Possibly related PRs

Suggested reviewers

  • murdore

Poem

🐰 From burrowed OSS to cloud-lit air,
I hop with keys and tidy care,
I store a thought, then softly hum,
Mem0: API — here I come! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'BZ-45257: fix: migrated to cloud hosted mem0' accurately describes the main changeset—migration from OSS-based Mem0 to cloud-hosted Mem0 API, as evidenced by widespread changes across memory initialization, configuration types, and usage throughout the codebase.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the mem0 integration from a self-hosted OSS solution to the cloud-hosted Mem0 API. This is a significant architectural change that simplifies deployment by removing the need for local vector stores and embedders while introducing a dependency on Mem0's cloud service.

Key Changes:

  • Replaced mem0ai/oss Memory class with mem0ai MemoryClient for cloud API
  • Updated type definitions to match cloud API response structures and method signatures
  • Migrated from self-hosted configuration (vector stores, embedders, LLM) to simple API key authentication
  • Fixed role assignment for AI responses from "system" to "assistant" in conversation storage

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/lib/types/utilities.ts Updated Mem0Memory interface with cloud API method signatures and return types
src/lib/types/conversation.ts Changed import from mem0ai/oss MemoryConfig to custom Mem0Config
src/lib/neurolink.ts Updated mem0 integration calls to use cloud API parameters and corrected message roles
src/lib/memory/mem0Initializer.ts Replaced Memory class with MemoryClient and simplified configuration to API key only
scripts/examples/real-memory-test.js Simplified configuration from complex self-hosted setup to cloud API key
package.json Pinned mem0ai version to exact 2.1.38
pnpm-lock.yaml Updated lock file to reflect exact version
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/types/utilities.ts Outdated
Comment thread src/lib/types/utilities.ts Outdated
Comment thread src/lib/types/utilities.ts Outdated
Comment thread scripts/examples/real-memory-test.js Outdated
Comment thread src/lib/types/utilities.ts Outdated
Comment thread src/lib/neurolink.ts
Comment thread src/lib/neurolink.ts Outdated
Comment thread src/lib/neurolink.ts Outdated
Comment thread package.json Outdated
Comment thread src/lib/neurolink.ts Outdated
@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch from 07933fe to b728a49 Compare November 12, 2025 04:22
Copilot AI review requested due to automatic review settings November 12, 2025 04:29
@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch from b728a49 to 75d5130 Compare November 12, 2025 04:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/neurolink.ts Outdated
Comment thread src/lib/neurolink.ts
Comment thread scripts/examples/real-memory-test.js Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
scripts/examples/real-memory-test.js (3)

53-53: Do not log secrets; mask MEM0 API key in debug output.

debugLog("Memory Configuration", conversationMemory) will print mem0Config.apiKey if set. Mask before logging to avoid secret leakage (even in examples).

Apply this minimal change:

-  debugLog("Memory Configuration", conversationMemory);
+  const safeMemoryConfig = {
+    ...conversationMemory,
+    mem0Config: conversationMemory.mem0Config
+      ? { ...conversationMemory.mem0Config, apiKey: (process.env.MEM0_API_KEY ? process.env.MEM0_API_KEY.slice(0,2) + "****" + process.env.MEM0_API_KEY.slice(-2) : "") }
+      : undefined,
+  };
+  debugLog("Memory Configuration", safeMemoryConfig);

Optionally extract a small mask helper for reuse.


505-507: Fix stray replacement character in console label.

"� Stack Trace" likely results from a bad emoji encoding. Replace with a plain label to avoid garbled console output.

-      console.log("\n� Stack Trace (first 10 lines):");
+      console.log("\nStack Trace (first 10 lines):");

61-65: Standardize provider configuration and environment variable; align with NeuroLink's expected credentials.

The code configures providers.google with GEMINI_API_KEY, but NeuroLink expects provider name "google-ai" (or "vertex") with env var GOOGLE_AI_API_KEY. This mismatch contradicts your troubleshooting steps at lines 512–520, which correctly reference GOOGLE_AI_API_KEY and GOOGLE_VERTEX_PROJECT.

Change line 62–63 to use the correct provider name and env var:

-        google: {
-          apiKey: process.env.GEMINI_API_KEY
+        "google-ai": {
+          apiKey: process.env.GOOGLE_AI_API_KEY

Alternatively, if you intend to use Vertex, align with the provider: "vertex" logic already present elsewhere in the file (line 81) and configure with appropriate Vertex credentials.

♻️ Duplicate comments (2)
package.json (1)

188-189: Exact pin for mem0ai may hinder security/patch updates.

Consider a tilde range to allow patch fixes, or document why exact pin is required for the cloud API migration.

#!/bin/bash
# Inspect mem0 usage to detect API-surface assumptions that require exact pin
rg -nP "from\s+\"mem0ai\"|MemoryClient|\.search\(|\.add\(|\.get\(|\.update\(|\.getAll\(" -C2
src/lib/neurolink.ts (1)

145-146: Static import of initializeMem0: verify package side effects at startup.

Good move for clarity; just ensure mem0ai has no heavy side effects on import that affect startup time.

#!/bin/bash
# Rough check for import cost hotspots in mem0ai (string search heuristic)
rg -n "global\\.|process\\.on\\(|setInterval\\(|http\\.create" "$(pnpm root)/mem0ai" 2>/dev/null || true
🧹 Nitpick comments (3)
src/lib/neurolink.ts (3)

247-264: Guard against empty API key before initializing mem0.

Add a quick check to skip initialization when this.mem0Config?.apiKey is missing/blank to avoid redundant attempts.

   if (!this.mem0Config) {
     this.mem0Instance = null;
     return null;
   }
-  this.mem0Instance = await initializeMem0(this.mem0Config);
+  if (!this.mem0Config.apiKey || this.mem0Config.apiKey.trim() === "") {
+    this.mem0Instance = null;
+    return null;
+  }
+  this.mem0Instance = await initializeMem0(this.mem0Config);
   return this.mem0Instance;

592-597: Memory-context helpers are clean; minor prompt wording tweak optional.

Helpers look good. Optionally make the header explicit to the model: “Relevant memories from past conversations:” to reduce ambiguity.

-    return `Context from previous conversations:
+    return `Relevant memories from past conversations:
 
 ${memoryContext}
 
 Current user's request: ${currentInput}`;

Also applies to: 600-605, 608-626


1644-1647: Avoid mutating caller-provided options.input.text.

Mutating options can surprise callers. Build a derived prompt and pass it forward without changing the original object.

-            options.input.text = this.formatMemoryContext(
-              memoryContext,
-              options.input.text,
-            );
+            const promptWithMemory = this.formatMemoryContext(
+              memoryContext,
+              options.input.text,
+            );
+            // Use promptWithMemory when constructing textOptions later instead of mutating options.input.text

Outside this hunk, ensure baseOptions.prompt uses promptWithMemory if set.

Also applies to: 2667-2670

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60c35e0 and 75d5130.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • package.json (1 hunks)
  • scripts/examples/real-memory-test.js (9 hunks)
  • src/lib/memory/mem0Initializer.ts (2 hunks)
  • src/lib/neurolink.ts (7 hunks)
  • src/lib/types/conversation.ts (2 hunks)
  • src/lib/types/utilities.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-17T17:55:15.261Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 173
File: src/lib/index.ts:16-16
Timestamp: 2025-09-17T17:55:15.261Z
Learning: In src/lib/types/providers.ts, ProviderConfig was renamed to AIModelProviderConfig to deduplicate type names, as there was an existing ProviderConfig type that better suited the "ProviderConfig" name. This was an intentional breaking change for better type organization.

Applied to files:

  • src/lib/types/conversation.ts
  • src/lib/types/utilities.ts
📚 Learning: 2025-09-02T13:50:42.770Z
Learnt from: YasmeenOgo
Repo: juspay/neurolink PR: 145
File: src/lib/core/types.ts:0-0
Timestamp: 2025-09-02T13:50:42.770Z
Learning: The APIVersions enum in src/lib/core/types.ts now contains comprehensive API version constants for all major AI providers: Azure OpenAI (latest, stable, legacy), OpenAI (current, beta), Google AI (current, beta), and Anthropic (current). This centralization helps avoid API version drift across the codebase.

Applied to files:

  • src/lib/types/utilities.ts
📚 Learning: 2025-09-24T07:26:41.988Z
Learnt from: amreetkhuntia
Repo: juspay/neurolink PR: 185
File: src/lib/evaluation/prompts.ts:86-101
Timestamp: 2025-09-24T07:26:41.988Z
Learning: In the neurolink codebase, maintainer amreetkhuntia consistently prefers to keep template literal indentation in LLM prompts (including evaluation prompts in src/lib/evaluation/prompts.ts) for readability, even when it results in extra whitespace in the output, as LLMs can parse and understand the content correctly.

Applied to files:

  • src/lib/neurolink.ts
📚 Learning: 2025-09-24T06:42:06.088Z
Learnt from: amreetkhuntia
Repo: juspay/neurolink PR: 185
File: src/lib/evaluation/contextBuilder.ts:79-85
Timestamp: 2025-09-24T06:42:06.088Z
Learning: In the NeuroLink codebase, using `(options.prompt || [])` pattern for handling potentially undefined prompt arrays is the preferred approach over extracting to a normalized variable when building conversation history in the ContextBuilder class.

Applied to files:

  • src/lib/neurolink.ts
🧬 Code graph analysis (3)
src/lib/types/conversation.ts (1)
src/lib/memory/mem0Initializer.ts (1)
  • Mem0Config (13-15)
src/lib/memory/mem0Initializer.ts (2)
scripts/examples/real-memory-test.js (1)
  • mem0Config (43-45)
src/lib/types/utilities.ts (1)
  • Mem0Memory (214-271)
src/lib/neurolink.ts (2)
src/lib/memory/mem0Initializer.ts (1)
  • Mem0Config (13-15)
src/lib/types/utilities.ts (1)
  • Mem0Memory (214-271)
🔇 Additional comments (3)
src/lib/types/conversation.ts (1)

6-6: Type alignment with Mem0 cloud config looks good.

Import path and mem0Config?: Mem0Config are consistent with the initializer.

Also applies to: 39-41

src/lib/types/utilities.ts (1)

212-271: Mem0Memory types verified; all call sites compatible.

Search and add methods at call sites use correct user_id parameters. Removed methods (history, reset) are not referenced. The update method is unused in the codebase, so the signature change poses no migration risk. Date fields as ISO 8601 strings align with cloud API expectations.

src/lib/neurolink.ts (1)

1636-1638: Mem0 v2.1.38 API usage verified across all flagged sections.

All search() calls correctly use user_id and limit parameters, and the add() method is called with valid parameters: user_id, metadata, infer (default true), and async_mode. Implementation is consistent with the confirmed API.

Comment thread src/lib/memory/mem0Initializer.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60c35e0 and 75d5130.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • package.json (1 hunks)
  • scripts/examples/real-memory-test.js (9 hunks)
  • src/lib/memory/mem0Initializer.ts (2 hunks)
  • src/lib/neurolink.ts (7 hunks)
  • src/lib/types/conversation.ts (2 hunks)
  • src/lib/types/utilities.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-09-17T17:55:15.261Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 173
File: src/lib/index.ts:16-16
Timestamp: 2025-09-17T17:55:15.261Z
Learning: In src/lib/types/providers.ts, ProviderConfig was renamed to AIModelProviderConfig to deduplicate type names, as there was an existing ProviderConfig type that better suited the "ProviderConfig" name. This was an intentional breaking change for better type organization.

Applied to files:

  • src/lib/types/conversation.ts
  • src/lib/neurolink.ts
  • src/lib/types/utilities.ts
  • src/lib/memory/mem0Initializer.ts
📚 Learning: 2025-11-04T22:14:18.719Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 0
File: :0-0
Timestamp: 2025-11-04T22:14:18.719Z
Learning: In the juspay/neurolink repository, do not flag existing type or interface definitions located outside src/lib/types/ - these are part of a phased migration plan and will be addressed in upcoming PRs. Only enforce type centralization rules on new code going forward.

Applied to files:

  • src/lib/neurolink.ts
📚 Learning: 2025-09-17T18:14:34.960Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 173
File: src/lib/types/index.ts:58-62
Timestamp: 2025-09-17T18:14:34.960Z
Learning: RajuSudhar explained that in the Neurolink codebase, there are multiple ProviderConfig types causing inconsistency. One existing ProviderConfig type better suited the "ProviderConfig" name, so they renamed the less-suitable one to AIModelProviderConfig to free up the name. Adding backward compatibility aliases would worsen naming inconsistency rather than help. The remaining duplicates will be systematically deduplicated in the 07-Types-Module.md TODO as part of their phased refactor approach.

Applied to files:

  • src/lib/neurolink.ts
📚 Learning: 2025-09-24T07:26:41.988Z
Learnt from: amreetkhuntia
Repo: juspay/neurolink PR: 185
File: src/lib/evaluation/prompts.ts:86-101
Timestamp: 2025-09-24T07:26:41.988Z
Learning: In the neurolink codebase, maintainer amreetkhuntia consistently prefers to keep template literal indentation in LLM prompts (including evaluation prompts in src/lib/evaluation/prompts.ts) for readability, even when it results in extra whitespace in the output, as LLMs can parse and understand the content correctly.

Applied to files:

  • src/lib/neurolink.ts
📚 Learning: 2025-09-24T06:42:06.088Z
Learnt from: amreetkhuntia
Repo: juspay/neurolink PR: 185
File: src/lib/evaluation/contextBuilder.ts:79-85
Timestamp: 2025-09-24T06:42:06.088Z
Learning: In the NeuroLink codebase, using `(options.prompt || [])` pattern for handling potentially undefined prompt arrays is the preferred approach over extracting to a normalized variable when building conversation history in the ContextBuilder class.

Applied to files:

  • src/lib/neurolink.ts
📚 Learning: 2025-09-02T13:50:42.770Z
Learnt from: YasmeenOgo
Repo: juspay/neurolink PR: 145
File: src/lib/core/types.ts:0-0
Timestamp: 2025-09-02T13:50:42.770Z
Learning: The APIVersions enum in src/lib/core/types.ts now contains comprehensive API version constants for all major AI providers: Azure OpenAI (latest, stable, legacy), OpenAI (current, beta), Google AI (current, beta), and Anthropic (current). This centralization helps avoid API version drift across the codebase.

Applied to files:

  • src/lib/types/utilities.ts
🧬 Code graph analysis (3)
src/lib/types/conversation.ts (1)
src/lib/memory/mem0Initializer.ts (1)
  • Mem0Config (13-15)
src/lib/neurolink.ts (2)
src/lib/memory/mem0Initializer.ts (1)
  • Mem0Config (13-15)
src/lib/types/utilities.ts (1)
  • Mem0Memory (214-271)
src/lib/memory/mem0Initializer.ts (2)
scripts/examples/real-memory-test.js (1)
  • mem0Config (43-45)
src/lib/types/utilities.ts (1)
  • Mem0Memory (214-271)
🔇 Additional comments (2)
src/lib/neurolink.ts (1)

1635-1647: Mem0 client usage matches cloud API.

Lines 1635-1645 and 1834-1847: The mem0.search / mem0.add calls mirror the Mem0 MemoryClient contract (query plus options containing user_id, limit, and metadata), so the cloud client swap should behave identically at runtime. (deepwiki.com)

Also applies to: 1834-1847

src/lib/memory/mem0Initializer.ts (1)

26-59: MemoryClient initialization looks solid.

Lines 27-59: Instantiating MemoryClient with the API key and falling back to a no-op shim still satisfies the documented add/search/getAll surface, so consumers keep the same contract even when the cloud client cannot boot. (deepwiki.com)

Comment thread scripts/examples/real-memory-test.js
Comment thread src/lib/types/utilities.ts Outdated
Comment thread src/lib/types/utilities.ts Outdated
Comment thread src/lib/types/utilities.ts Outdated
@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch from 75d5130 to 75e7f6a Compare November 12, 2025 06:16
Copilot AI review requested due to automatic review settings November 17, 2025 04:26
@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch from 75e7f6a to c6478f4 Compare November 17, 2025 04:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/neurolink.ts
@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch from c6478f4 to 8a53aca Compare November 17, 2025 04:58
Copilot AI review requested due to automatic review settings November 17, 2025 07:06
@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch from 8a53aca to b695faa Compare November 17, 2025 07:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch 3 times, most recently from 85cd34c to 3097698 Compare November 18, 2025 03:20
Copilot AI review requested due to automatic review settings November 18, 2025 03:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/mcp/mcpClientFactory.ts
@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch from 3097698 to 2526a87 Compare November 18, 2025 03:41
@cmd-err

cmd-err commented Nov 18, 2025

Copy link
Copy Markdown
Contributor Author

@coderabbitai please review

@coderabbitai

coderabbitai Bot commented Nov 18, 2025

Copy link
Copy Markdown

@cmd-err: I'll review the pull request for the migration to cloud-hosted mem0.


🧠 Learnings used
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 0
File: :0-0
Timestamp: 2025-11-04T22:14:18.719Z
Learning: In the juspay/neurolink repository, do not flag existing type or interface definitions located outside src/lib/types/ - these are part of a phased migration plan and will be addressed in upcoming PRs. Only enforce type centralization rules on new code going forward.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (3)
scripts/examples/real-memory-test.js (2)

77-88: User IDs switched to rollout/revert are fine as isolated test identities.

The various userId changes to "rollout" and "revert" across steps keep Alice/Bob separated for mem0 and match your earlier note that these are just throwaway test IDs. Functionally, they still exercise per-user isolation and cross-session continuity as intended.

Also applies to: 123-135, 183-194, 227-238, 267-278, 311-323, 375-387


512-520: Env var debugging guidance now matches actual variables.

The troubleshooting steps correctly reference MEM0_API_KEY, GEMINI_API_KEY, and a grep that includes MEM0, GEMINI, VERTEX, and AI, which is consistent with how the script reads environment variables.

src/lib/neurolink.ts (1)

607-623: Only user prompts are persisted to Mem0; consider optionally including AI responses as well.

Right now:

  • storeConversationTurn builds conversationTurn with just { role: "user", content: userContent }.
  • In generate() you call it with originalPrompt and metadata (provider/model/etc.), but not the AI output.
  • In stream(), the AI response is only present in the metadata (aiResponse), not as a separate message in the turn array.

If the goal is for Mem0 to capture full conversation semantics (user + assistant), it may be worth extending the helper to accept an optional assistant message so you can store both when available, while still allowing “user-only” storage when you care purely about user facts.

For example:

-  private async storeConversationTurn(
-    mem0: MemoryClient,
-    userContent: string,
-    userId: string,
-    metadata: Record<string, unknown>,
-  ): Promise<void> {
-    // Store user message only, reducing latency in mem0
-    const conversationTurn = [{ role: "user" as const, content: userContent }];
+  private async storeConversationTurn(
+    mem0: MemoryClient,
+    userContent: string,
+    userId: string,
+    metadata: Record<string, unknown>,
+    assistantContent?: string,
+  ): Promise<void> {
+    const conversationTurn: Array<{ role: "user" | "assistant"; content: string }> = [
+      { role: "user", content: userContent },
+    ];
+    if (assistantContent && assistantContent.trim()) {
+      conversationTurn.push({ role: "assistant", content: assistantContent.trim() });
+    }
@@
-    await mem0.add(conversationTurn, {
+    await mem0.add(conversationTurn, {
       user_id: userId,
       metadata,
       infer: true,
       async_mode: true,
     });

And then:

-            await this.storeConversationTurn(
-              mem0,
-              originalPrompt,
-              options.context?.userId as string,
-              {
-                timestamp: new Date().toISOString(),
-                provider: generateResult.provider,
-                model: generateResult.model,
-                type: "conversation_turn",
-              },
-            );
+            await this.storeConversationTurn(
+              mem0,
+              originalPrompt,
+              options.context?.userId as string,
+              {
+                timestamp: new Date().toISOString(),
+                provider: generateResult.provider,
+                model: generateResult.model,
+                type: "conversation_turn",
+              },
+              generateResult.content || undefined,
+            );
@@
-                  await self.storeConversationTurn(
-                    mem0,
-                    originalPrompt,
-                    enhancedOptions.context?.userId as string,
-                    {
-                      timestamp: new Date().toISOString(),
-                      type: "conversation_turn_stream",
-                      userMessage: originalPrompt,
-                      aiResponse: accumulatedContent.trim(),
-                    },
-                  );
+                  await self.storeConversationTurn(
+                    mem0,
+                    originalPrompt,
+                    enhancedOptions.context?.userId as string,
+                    {
+                      timestamp: new Date().toISOString(),
+                      type: "conversation_turn_stream",
+                    },
+                    accumulatedContent.trim(),
+                  );

If you intentionally want Mem0 to only index user-side content (and keep AI output purely in metadata), then the current implementation is fine—just worth confirming that this matches your long-term retrieval expectations.

Also applies to: 1621-1650, 1822-1849, 2643-2673, 2761-2787

🧹 Nitpick comments (1)
scripts/examples/real-memory-test.js (1)

42-51: Consider failing fast when MEM0_API_KEY is missing for this test.

Right now mem0Enabled: true but mem0Config.apiKey can be "" (via process.env.MEM0_API_KEY || ""), so NeuroLink will just disable mem0 internally and the script still reports a “successful” run without actually exercising cloud memory.

For a script explicitly meant to validate real mem0 behavior, you might make the key mandatory and exit early with a clear message, e.g.:

   const mem0Config = {
-    apiKey: process.env.MEM0_API_KEY || "",
+    apiKey: process.env.MEM0_API_KEY || "",
   };
 
-   const conversationMemory = {
+  if (!mem0Config.apiKey) {
+    console.error("MEM0_API_KEY is not set. Please export it before running real-memory-test.");
+    process.exit(1);
+  }
+
+  const conversationMemory = {
     enabled: true,
     mem0Enabled: true,
     mem0Config
   }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 75d5130 and 2526a87.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (7)
  • package.json (1 hunks)
  • scripts/examples/real-memory-test.js (9 hunks)
  • src/lib/mcp/mcpClientFactory.ts (0 hunks)
  • src/lib/memory/mem0Initializer.ts (1 hunks)
  • src/lib/neurolink.ts (9 hunks)
  • src/lib/types/conversation.ts (2 hunks)
  • src/lib/types/utilities.ts (0 hunks)
💤 Files with no reviewable changes (2)
  • src/lib/mcp/mcpClientFactory.ts
  • src/lib/types/utilities.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 0
File: :0-0
Timestamp: 2025-11-04T22:14:18.719Z
Learning: In the juspay/neurolink repository, do not flag existing type or interface definitions located outside src/lib/types/ - these are part of a phased migration plan and will be addressed in upcoming PRs. Only enforce type centralization rules on new code going forward.
📚 Learning: 2025-09-17T17:55:15.261Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 173
File: src/lib/index.ts:16-16
Timestamp: 2025-09-17T17:55:15.261Z
Learning: In src/lib/types/providers.ts, ProviderConfig was renamed to AIModelProviderConfig to deduplicate type names, as there was an existing ProviderConfig type that better suited the "ProviderConfig" name. This was an intentional breaking change for better type organization.

Applied to files:

  • src/lib/types/conversation.ts
  • src/lib/neurolink.ts
  • src/lib/memory/mem0Initializer.ts
📚 Learning: 2025-11-04T22:14:18.719Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 0
File: :0-0
Timestamp: 2025-11-04T22:14:18.719Z
Learning: In the juspay/neurolink repository, do not flag existing type or interface definitions located outside src/lib/types/ - these are part of a phased migration plan and will be addressed in upcoming PRs. Only enforce type centralization rules on new code going forward.

Applied to files:

  • src/lib/neurolink.ts
📚 Learning: 2025-09-24T07:26:41.988Z
Learnt from: amreetkhuntia
Repo: juspay/neurolink PR: 185
File: src/lib/evaluation/prompts.ts:86-101
Timestamp: 2025-09-24T07:26:41.988Z
Learning: In the neurolink codebase, maintainer amreetkhuntia consistently prefers to keep template literal indentation in LLM prompts (including evaluation prompts in src/lib/evaluation/prompts.ts) for readability, even when it results in extra whitespace in the output, as LLMs can parse and understand the content correctly.

Applied to files:

  • src/lib/neurolink.ts
📚 Learning: 2025-09-17T18:14:34.960Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 173
File: src/lib/types/index.ts:58-62
Timestamp: 2025-09-17T18:14:34.960Z
Learning: RajuSudhar explained that in the Neurolink codebase, there are multiple ProviderConfig types causing inconsistency. One existing ProviderConfig type better suited the "ProviderConfig" name, so they renamed the less-suitable one to AIModelProviderConfig to free up the name. Adding backward compatibility aliases would worsen naming inconsistency rather than help. The remaining duplicates will be systematically deduplicated in the 07-Types-Module.md TODO as part of their phased refactor approach.

Applied to files:

  • src/lib/neurolink.ts
📚 Learning: 2025-09-24T06:42:06.088Z
Learnt from: amreetkhuntia
Repo: juspay/neurolink PR: 185
File: src/lib/evaluation/contextBuilder.ts:79-85
Timestamp: 2025-09-24T06:42:06.088Z
Learning: In the NeuroLink codebase, using `(options.prompt || [])` pattern for handling potentially undefined prompt arrays is the preferred approach over extracting to a normalized variable when building conversation history in the ContextBuilder class.

Applied to files:

  • src/lib/neurolink.ts
🔇 Additional comments (4)
src/lib/types/conversation.ts (1)

6-7: Mem0Config integration into ConversationMemoryConfig looks correct.

The Mem0Config import path and the optional mem0Config?: Mem0Config field align with the new initializer and lazy mem0 wiring in NeuroLink. No issues from a typing or configuration perspective.

Also applies to: 39-44

src/lib/memory/mem0Initializer.ts (1)

6-14: Mem0 cloud initializer behavior is clean and matches the lazy-init pattern.

The Mem0Config shape, API-key guard, and initializeMem0 implementation (returning MemoryClient | null with clear logging and no fallback object) align well with the updated Neurolink mem0 usage and avoid silent pseudo-success states.

Also applies to: 19-50

src/lib/neurolink.ts (2)

27-27: Mem0 lazy initialization and config wiring look sound.

The combination of mem0Instance?: MemoryClient | null, optional mem0Config?: Mem0Config, initializeMem0Config() gated on conversationMemory.mem0Enabled, and ensureMem0Ready() caching both success and null gives you a predictable, one-time Mem0 client setup without impacting constructor cost. Call sites correctly treat null as “mem0 disabled/not available.”

Also applies to: 145-146, 228-242, 247-264


587-605: Memory context formatting and extraction helpers are straightforward and reusable.

formatMemoryContext and extractMemoryContext give you a clear, shared path for turning Mem0 search results into a prompt prefix, and they’re used consistently in both generate() and stream(). The simple newline-joined format keeps the injected context readable for models without overcomplicating the structure.

Copilot AI review requested due to automatic review settings November 19, 2025 07:06
@cmd-err
cmd-err force-pushed the BZ-45257-cloud-hosted-mem-0-support branch from 2526a87 to 7a0a24c Compare November 19, 2025 07:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)

src/lib/neurolink.ts:2727

  • The indentation of this statement suggests that it is controlled by this statement, while in fact it is not.
        if (this.enableOrchestration && !options.provider && !options.model) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/neurolink.ts
Comment thread scripts/examples/real-memory-test.js Outdated
Comment thread scripts/examples/real-memory-test.js Outdated
Comment thread scripts/examples/real-memory-test.js Outdated
Comment thread src/lib/neurolink.ts Outdated
Migrate from mem0ai/oss (self-hosted) to mem0ai cloud API to resolve production ESM compatibility issues

1. **Production ESM Errors**: Fixed '__filename is not defined' errors caused by CommonJS dependencies (Qdrant, SQLite) in ESM environment
2. **Type System Complexity**: Removed custom type definitions in favor of official MemoryClient types from mem0ai package
3. **Maintenance Burden**: Eliminated fallback memory implementation and unnecessary abstractions

- Removed custom Mem0Memory type definition from utilities.ts
- Import MemoryClient directly from mem0ai package where needed
- Updated all type signatures to use MemoryClient

- Replaced MemoryConfig (OSS) with simple Mem0Config (apiKey only)
- Updated mem0Initializer.ts to use MemoryClient constructor
- Added API key guard to skip initialization gracefully
- Removed fallback memory implementation (return null on failure)

- search(): Changed userId parameter to user_id
- add(): Updated to accept message array with user_id parameter
- Removed async_mode from add() calls (moved to metadata)
- Updated return type handling (array instead of { results: [] })

- Added helper methods to reduce duplication:
  - extractMemoryContext(): Extract memory strings from search results
  - storeConversationTurn(): Store user/assistant message pairs
  - formatMemoryContext(): Format memory context for prompts
- Pinned mem0ai to exact version 2.1.38 for stability
- Updated test file with correct environment variable documentation

Comprehensive testing with real-memory-test.js verified:
- ✅ Memory storage (Alice: JavaScript/React/TechCorp)
- ✅ Memory retrieval across sessions
- ✅ User isolation (Alice: rollout vs Bob: revert)
- ✅ Cross-session continuity (same userId, different sessionId)
- ✅ Streaming with personalized memory context
- ✅ All builds passing with no bundling errors

1. **Zero Native Dependencies**: Pure HTTP client eliminates ESM compatibility issues
2. **Simplified Maintenance**: Direct type imports auto-update with package
3. **Better Type Safety**: Official types provide accurate IDE support
4. **Cleaner Codebase**: 145 lines removed, 112 lines added (net -33 lines)
5. **Production Ready**: Cloud-hosted infrastructure handles scaling
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.

3 participants