fix(ts-oss): isolate entity store from memory store by default (#4829)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: kartik-mem0 <kartik.labhshetwar@mem0.ai>
This commit is contained in:
@@ -26,7 +26,7 @@ describeIntegration("MemoryClient Integration — Batch Operations", () => {
|
||||
|
||||
beforeAll(async () => {
|
||||
cleanup = suppressTelemetryNoise();
|
||||
client = createTestClient();
|
||||
client = await createTestClient();
|
||||
memoryIds = await seedTestMemories(client, TEST_USER_ID);
|
||||
});
|
||||
|
||||
|
||||
@@ -26,9 +26,9 @@ describeIntegration("MemoryClient Integration — CRUD", () => {
|
||||
let cleanup: () => void;
|
||||
let memoryIds: string[] = [];
|
||||
|
||||
beforeAll(() => {
|
||||
beforeAll(async () => {
|
||||
cleanup = suppressTelemetryNoise();
|
||||
client = createTestClient();
|
||||
client = await createTestClient();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
|
||||
@@ -18,9 +18,13 @@ export const describeIntegration = API_KEY ? describe : describe.skip;
|
||||
* Create a MemoryClient with the real API key.
|
||||
* Call this inside beforeAll — not at module scope — so it only
|
||||
* runs when the suite is not skipped.
|
||||
*
|
||||
* Returns an initialized client ready for immediate use.
|
||||
*/
|
||||
export function createTestClient(): MemoryClient {
|
||||
return new MemoryClient({ apiKey: API_KEY! });
|
||||
export async function createTestClient(): Promise<MemoryClient> {
|
||||
const client = new MemoryClient({ apiKey: API_KEY! });
|
||||
await client.ping();
|
||||
return client;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -24,9 +24,9 @@ describeIntegration("MemoryClient Integration — Initialization", () => {
|
||||
let client: MemoryClient;
|
||||
let cleanup: () => void;
|
||||
|
||||
beforeAll(() => {
|
||||
beforeAll(async () => {
|
||||
cleanup = suppressTelemetryNoise();
|
||||
client = createTestClient();
|
||||
client = await createTestClient();
|
||||
});
|
||||
|
||||
afterAll(() => cleanup());
|
||||
|
||||
@@ -27,7 +27,7 @@ describeIntegration("MemoryClient Integration — Users & Project", () => {
|
||||
|
||||
beforeAll(async () => {
|
||||
cleanup = suppressTelemetryNoise();
|
||||
client = createTestClient();
|
||||
client = await createTestClient();
|
||||
await seedTestMemories(client, TEST_USER_ID);
|
||||
});
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@ describeIntegration("MemoryClient Integration — Search & History", () => {
|
||||
|
||||
beforeAll(async () => {
|
||||
cleanup = suppressTelemetryNoise();
|
||||
client = createTestClient();
|
||||
client = await createTestClient();
|
||||
memoryIds = await seedTestMemories(client, TEST_USER_ID);
|
||||
});
|
||||
|
||||
|
||||
@@ -22,7 +22,7 @@ export const DEFAULT_MEMORY_CONFIG: MemoryConfig = {
|
||||
config: {
|
||||
baseURL: "https://api.openai.com/v1",
|
||||
apiKey: process.env.OPENAI_API_KEY || "",
|
||||
model: "gpt-4.1-nano-2025-04-14",
|
||||
model: "gpt-5-mini",
|
||||
modelProperties: undefined,
|
||||
},
|
||||
},
|
||||
|
||||
@@ -18,7 +18,7 @@ export class AzureOpenAILLM implements LLM {
|
||||
endpoint: endpoint as string,
|
||||
...rest,
|
||||
});
|
||||
this.model = config.model || "gpt-4";
|
||||
this.model = config.model || "gpt-5-mini";
|
||||
}
|
||||
|
||||
async generateResponse(
|
||||
|
||||
@@ -11,7 +11,7 @@ export class OpenAILLM implements LLM {
|
||||
apiKey: config.apiKey,
|
||||
baseURL: config.baseURL,
|
||||
});
|
||||
this.model = config.model || "gpt-4.1-nano-2025-04-14";
|
||||
this.model = config.model || "gpt-5-mini";
|
||||
}
|
||||
|
||||
async generateResponse(
|
||||
|
||||
@@ -8,7 +8,7 @@ export class OpenAIStructuredLLM implements LLM {
|
||||
|
||||
constructor(config: LLMConfig) {
|
||||
this.openai = new OpenAI({ apiKey: config.apiKey });
|
||||
this.model = config.model || "gpt-4-turbo-preview";
|
||||
this.model = config.model || "gpt-5-mini";
|
||||
}
|
||||
|
||||
async generateResponse(
|
||||
|
||||
@@ -39,7 +39,7 @@ class AzureOpenAILLM(LLMBase):
|
||||
|
||||
# Model name should match the custom deployment name chosen for it.
|
||||
if not self.config.model:
|
||||
self.config.model = "gpt-4.1-nano-2025-04-14"
|
||||
self.config.model = "gpt-5-mini"
|
||||
|
||||
api_key = self.config.azure_kwargs.api_key or os.getenv("LLM_AZURE_OPENAI_API_KEY")
|
||||
azure_deployment = self.config.azure_kwargs.azure_deployment or os.getenv("LLM_AZURE_DEPLOYMENT")
|
||||
|
||||
@@ -18,7 +18,7 @@ class AzureOpenAIStructuredLLM(LLMBase):
|
||||
|
||||
# Model name should match the custom deployment name chosen for it.
|
||||
if not self.config.model:
|
||||
self.config.model = "gpt-4.1-nano-2025-04-14"
|
||||
self.config.model = "gpt-5-mini"
|
||||
|
||||
api_key = self.config.azure_kwargs.api_key or os.getenv("LLM_AZURE_OPENAI_API_KEY")
|
||||
azure_deployment = self.config.azure_kwargs.azure_deployment or os.getenv("LLM_AZURE_DEPLOYMENT")
|
||||
|
||||
@@ -16,7 +16,7 @@ class LiteLLM(LLMBase):
|
||||
super().__init__(config)
|
||||
|
||||
if not self.config.model:
|
||||
self.config.model = "gpt-4.1-nano-2025-04-14"
|
||||
self.config.model = "gpt-5-mini"
|
||||
|
||||
def _parse_response(self, response, tools):
|
||||
"""
|
||||
|
||||
+1
-1
@@ -36,7 +36,7 @@ class OpenAILLM(LLMBase):
|
||||
super().__init__(config)
|
||||
|
||||
if not self.config.model:
|
||||
self.config.model = "gpt-4.1-nano-2025-04-14"
|
||||
self.config.model = "gpt-5-mini"
|
||||
|
||||
if os.environ.get("OPENROUTER_API_KEY"): # Use OpenRouter
|
||||
self.client = OpenAI(
|
||||
|
||||
@@ -12,7 +12,7 @@ class OpenAIStructuredLLM(LLMBase):
|
||||
super().__init__(config)
|
||||
|
||||
if not self.config.model:
|
||||
self.config.model = "gpt-4o-2024-08-06"
|
||||
self.config.model = "gpt-5-mini"
|
||||
|
||||
api_key = self.config.api_key or os.getenv("OPENAI_API_KEY")
|
||||
base_url = self.config.openai_base_url or os.getenv("OPENAI_API_BASE") or "https://api.openai.com/v1"
|
||||
|
||||
+14
-3
@@ -32,15 +32,26 @@ def get_user_id():
|
||||
return "anonymous_user"
|
||||
|
||||
|
||||
def get_or_create_user_id(vector_store):
|
||||
"""Store user_id in vector store and return it."""
|
||||
def get_or_create_user_id(vector_store=None):
|
||||
"""Store user_id in vector store and return it.
|
||||
|
||||
If vector_store is None, simply returns the user_id from config.
|
||||
This ensures telemetry initialization never fails due to missing vector store.
|
||||
"""
|
||||
user_id = get_user_id()
|
||||
|
||||
# If no vector store provided, just return the user_id
|
||||
if vector_store is None:
|
||||
return user_id
|
||||
|
||||
# Try to get existing user_id from vector store
|
||||
try:
|
||||
existing = vector_store.get(vector_id=user_id)
|
||||
if existing and hasattr(existing, "payload") and existing.payload and "user_id" in existing.payload:
|
||||
return existing.payload["user_id"]
|
||||
stored_id = existing.payload["user_id"]
|
||||
# Ensure we never return None from vector store
|
||||
if stored_id is not None:
|
||||
return stored_id
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
||||
@@ -88,6 +88,12 @@ class AnonymousTelemetry:
|
||||
if self.posthog is None:
|
||||
return
|
||||
|
||||
# Determine distinct_id, skip if None to prevent crashes
|
||||
distinct_id = self.user_id if user_email is None else user_email
|
||||
if distinct_id is None:
|
||||
_logger.debug("Skipping telemetry event %r: no distinct_id available", event_name)
|
||||
return
|
||||
|
||||
if properties is None:
|
||||
properties = {}
|
||||
properties = {
|
||||
@@ -101,8 +107,10 @@ class AnonymousTelemetry:
|
||||
"machine": platform.machine(),
|
||||
**properties,
|
||||
}
|
||||
distinct_id = self.user_id if user_email is None else user_email
|
||||
try:
|
||||
self.posthog.capture(distinct_id=distinct_id, event=event_name, properties=properties)
|
||||
except Exception as e:
|
||||
_logger.debug("Failed to capture telemetry event %r: %s", event_name, e)
|
||||
|
||||
def close(self):
|
||||
if self.posthog is not None:
|
||||
@@ -158,9 +166,15 @@ atexit.register(client_telemetry.close)
|
||||
|
||||
|
||||
def capture_event(event_name, memory_instance, additional_data=None):
|
||||
"""Capture telemetry event for OSS Memory instances.
|
||||
|
||||
This function is designed to never raise exceptions - telemetry failures
|
||||
should not affect the main application flow.
|
||||
"""
|
||||
if not MEM0_TELEMETRY:
|
||||
return
|
||||
|
||||
try:
|
||||
oss_telemetry = _get_oss_telemetry()
|
||||
if oss_telemetry is None:
|
||||
return
|
||||
@@ -178,12 +192,20 @@ def capture_event(event_name, memory_instance, additional_data=None):
|
||||
event_data.update(additional_data)
|
||||
|
||||
oss_telemetry.capture_event(event_name, event_data)
|
||||
except Exception as e:
|
||||
_logger.debug("Failed to capture OSS telemetry event %r: %s", event_name, e)
|
||||
|
||||
|
||||
def capture_client_event(event_name, instance, additional_data=None):
|
||||
"""Capture telemetry event for hosted MemoryClient instances.
|
||||
|
||||
This function is designed to never raise exceptions - telemetry failures
|
||||
should not affect the main application flow.
|
||||
"""
|
||||
if not MEM0_TELEMETRY:
|
||||
return
|
||||
|
||||
try:
|
||||
event_data = {
|
||||
"function": f"{instance.__class__.__module__}.{instance.__class__.__name__}",
|
||||
}
|
||||
@@ -191,3 +213,5 @@ def capture_client_event(event_name, instance, additional_data=None):
|
||||
event_data.update(additional_data)
|
||||
|
||||
client_telemetry.capture_event(event_name, event_data, instance.user_email)
|
||||
except Exception as e:
|
||||
_logger.debug("Failed to capture client telemetry event %r: %s", event_name, e)
|
||||
|
||||
@@ -286,8 +286,8 @@ def test_init_with_env_vars(monkeypatch):
|
||||
http_client=None,
|
||||
default_headers=None,
|
||||
)
|
||||
# Should default to "gpt-4.1-nano-2025-04-14" if model is None
|
||||
assert llm.config.model == "gpt-4.1-nano-2025-04-14"
|
||||
# Should default to "gpt-5-mini" if model is None
|
||||
assert llm.config.model == "gpt-5-mini"
|
||||
|
||||
|
||||
def test_init_with_default_azure_credential(monkeypatch):
|
||||
|
||||
@@ -57,7 +57,7 @@ def test_init_with_default_credential(mock_credential, mock_token_provider, mock
|
||||
mock_token_provider.return_value = "token-provider"
|
||||
llm = AzureOpenAIStructuredLLM(config)
|
||||
# Should set default model if not provided
|
||||
assert llm.config.model == "gpt-4.1-nano-2025-04-14"
|
||||
assert llm.config.model == "gpt-5-mini"
|
||||
mock_credential.assert_called_once()
|
||||
mock_token_provider.assert_called_once_with(mock_credential.return_value, SCOPE)
|
||||
mock_azure_openai.assert_called_once()
|
||||
@@ -92,7 +92,7 @@ def test_init_with_placeholder_api_key_uses_default_credential(
|
||||
config = DummyConfig(model=None, azure_kwargs=DummyAzureKwargs(api_key="your-api-key"))
|
||||
mock_token_provider.return_value = "token-provider"
|
||||
llm = AzureOpenAIStructuredLLM(config)
|
||||
assert llm.config.model == "gpt-4.1-nano-2025-04-14"
|
||||
assert llm.config.model == "gpt-5-mini"
|
||||
mock_credential.assert_called_once()
|
||||
mock_token_provider.assert_called_once_with(mock_credential.return_value, SCOPE)
|
||||
mock_azure_openai.assert_called_once()
|
||||
|
||||
@@ -359,3 +359,63 @@ class TestTelemetryEnvVar:
|
||||
def test_env_var_parsing(self, value, expected):
|
||||
result = value.lower() in ("true", "1", "yes")
|
||||
assert result == expected
|
||||
|
||||
|
||||
class TestTelemetryNullUserIdHandling:
|
||||
"""Verify telemetry doesn't crash when user_id is None.
|
||||
|
||||
This is a regression test for the bug where Memory.from_config() crashed
|
||||
with AssertionError because distinct_id was None in PostHog.capture().
|
||||
"""
|
||||
|
||||
def test_capture_event_skips_when_user_id_is_none(self):
|
||||
"""AnonymousTelemetry.capture_event should not crash when user_id is None."""
|
||||
with patch.object(telemetry_module, "MEM0_TELEMETRY", True):
|
||||
with patch("mem0.memory.telemetry.Posthog") as mock_posthog_cls:
|
||||
at = telemetry_module.AnonymousTelemetry()
|
||||
at.user_id = None # Simulate the bug condition
|
||||
|
||||
# This should not raise, even with user_id=None
|
||||
at.capture_event("test.event", {"key": "value"})
|
||||
|
||||
# PostHog.capture should NOT have been called since user_id is None
|
||||
mock_posthog_cls.return_value.capture.assert_not_called()
|
||||
|
||||
def test_capture_event_does_not_crash_on_posthog_error(self):
|
||||
"""AnonymousTelemetry.capture_event should catch PostHog exceptions."""
|
||||
with patch.object(telemetry_module, "MEM0_TELEMETRY", True):
|
||||
with patch("mem0.memory.telemetry.Posthog") as mock_posthog_cls:
|
||||
with patch("mem0.memory.telemetry.get_or_create_user_id", return_value="test-user"):
|
||||
at = telemetry_module.AnonymousTelemetry()
|
||||
mock_posthog_cls.return_value.capture.side_effect = Exception("PostHog error")
|
||||
|
||||
# This should not raise, even when PostHog.capture fails
|
||||
at.capture_event("test.event", {"key": "value"})
|
||||
|
||||
def test_oss_capture_event_does_not_crash_memory_init(self):
|
||||
"""capture_event() should never raise, even if everything inside fails."""
|
||||
with patch.object(telemetry_module, "MEM0_TELEMETRY", True):
|
||||
# Make _get_oss_telemetry return a broken telemetry object
|
||||
mock_at = MagicMock()
|
||||
mock_at.capture_event.side_effect = Exception("Telemetry is broken")
|
||||
|
||||
with patch.object(telemetry_module, "_oss_telemetry_instance", mock_at):
|
||||
mock_memory = MagicMock()
|
||||
mock_memory.config.graph_store.config = None
|
||||
mock_memory.api_version = "v1"
|
||||
|
||||
# This should not raise, even when telemetry fails
|
||||
telemetry_module.capture_event("mem0.init", mock_memory)
|
||||
|
||||
def test_client_capture_event_does_not_crash(self):
|
||||
"""capture_client_event() should never raise exceptions."""
|
||||
with patch.object(telemetry_module, "MEM0_TELEMETRY", True):
|
||||
mock_client_telemetry = MagicMock()
|
||||
mock_client_telemetry.capture_event.side_effect = Exception("Client telemetry broken")
|
||||
|
||||
with patch.object(telemetry_module, "client_telemetry", mock_client_telemetry):
|
||||
mock_instance = MagicMock()
|
||||
mock_instance.user_email = "test@example.com"
|
||||
|
||||
# This should not raise
|
||||
telemetry_module.capture_client_event("test.event", mock_instance)
|
||||
|
||||
Reference in New Issue
Block a user