diff --git a/backend/management/services/model/resolver.py b/backend/management/services/model/resolver.py index e0a4459b59..58fb71968f 100644 --- a/backend/management/services/model/resolver.py +++ b/backend/management/services/model/resolver.py @@ -82,7 +82,7 @@ def create_embedding_model(model: dict) -> Any: # Vendor dispatch (DashScope/Siliconflow/Jina/OpenAI) is resolved by the # adapter registry; per-vendor request-body formatting lives in the - # embedding adapters. Built fresh (no gateway cache). Returns the adapter; + # embedding adapters. Built fresh. Returns the adapter; # callers use adapter.get_embeddings / adapter.dimension_check unchanged. return build_adapter_fresh(model_config, modality, slot, None) diff --git a/backend/services/model_gateway_service.py b/backend/services/model_gateway_service.py index 177f327ae3..f5776892da 100644 --- a/backend/services/model_gateway_service.py +++ b/backend/services/model_gateway_service.py @@ -174,7 +174,7 @@ def get_adapter_from_config( tenant_id: Optional[str] = None, **construct_extras: Any, ): - """Resolve and return the adapter for ``cfg`` (cached by the gateway).""" + """Resolve and return the adapter for ``cfg``.""" context = _config_to_context(cfg, modality, slot, tenant_id, **construct_extras) return get_gateway().get_adapter(context) @@ -186,11 +186,11 @@ def build_adapter_fresh( tenant_id: Optional[str] = None, **construct_extras: Any, ): - """Build a fresh adapter for ``cfg`` WITHOUT the gateway instance cache. + """Build a fresh adapter for ``cfg``. Used by per-call construction sites (e.g. voice streaming sessions) where vendor config carries per-request params (api_key, ws_url, voice, …) that - must not collide across tenants under a shared cache key. + must not collide across tenants. """ context = _config_to_context(cfg, modality, slot, tenant_id, **construct_extras) cls = get_registry().resolve(context.factory, modality) diff --git a/sdk/nexent/core/gateway/model_context.py b/sdk/nexent/core/gateway/model_context.py index 95e23724b7..700d091b9c 100644 --- a/sdk/nexent/core/gateway/model_context.py +++ b/sdk/nexent/core/gateway/model_context.py @@ -26,10 +26,6 @@ class ModelContext: observer: Any = None # cross-cutting: LLM/VLM/ModelEngine STT/TTS timeout_seconds: Optional[float] = None # cross-cutting: all HTTP-backed adapters - def cache_key(self) -> tuple: - return (self.tenant_id or "", self.modality, self.slot or "", - self.model_name, self.factory) - @dataclass class LLMContext(ModelContext): diff --git a/sdk/nexent/core/gateway/multimodal_gateway.py b/sdk/nexent/core/gateway/multimodal_gateway.py index 313519bb68..845d2ed11c 100644 --- a/sdk/nexent/core/gateway/multimodal_gateway.py +++ b/sdk/nexent/core/gateway/multimodal_gateway.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import Any, Dict, Tuple +from typing import Any from .multimodal_adapter import MultimodalAdapter from .model_context import ModelContext @@ -10,32 +10,28 @@ class MultimodalGateway: - """Resolve and cache :class:`MultimodalAdapter` instances by context.""" + """Resolve :class:`MultimodalAdapter` instances by context.""" def __init__(self, registry: AdapterRegistry = None) -> None: - """Initializes the gateway with a registry and empty cache. + """Initializes the gateway with a registry. Args: registry: The adapter registry to resolve from. Defaults to the process-wide singleton. """ self._registry = registry or get_registry() - self._adapter_cache: Dict[Tuple, MultimodalAdapter] = {} def get_adapter(self, context: ModelContext) -> MultimodalAdapter: - """Returns the adapter for ``context``, building and caching it once. + """Returns the adapter for ``context``. Args: context: The construction context identifying the desired model. Returns: - The cached or newly built adapter instance. + The newly built adapter instance. """ cls = self._registry.resolve(context.factory, context.modality) - key = context.cache_key() - if key not in self._adapter_cache: - self._adapter_cache[key] = cls(context) - return self._adapter_cache[key] + return cls(context) async def invoke(self, context: ModelContext, request: Any) -> Any: """Resolves the adapter for ``context`` and invokes it. @@ -72,18 +68,6 @@ async def health_check(self, context: ModelContext) -> bool: """ return await self.get_adapter(context).health_check() - def invalidate(self, context: ModelContext = None) -> None: - """Drops cached adapter instances. - - Args: - context: If provided, drops only that context's cached adapter. - If None, drops the entire cache. - """ - if context is None: - self._adapter_cache.clear() - else: - self._adapter_cache.pop(context.cache_key(), None) - _gateway: MultimodalGateway = None diff --git a/sdk/nexent/memory/__init__.py b/sdk/nexent/memory/__init__.py index 5b0c1c36d0..1d20257af5 100644 --- a/sdk/nexent/memory/__init__.py +++ b/sdk/nexent/memory/__init__.py @@ -3,7 +3,6 @@ from .embedding_model import ( EmbeddingModelInfo, get_embedding_client, - reset_embedding_client_cache, ) from .models import ( ExternalMemoryItem, @@ -96,7 +95,6 @@ # Embedding "EmbeddingModelInfo", "get_embedding_client", - "reset_embedding_client_cache", # Providers "BaseMemoryProvider", "SearchableMemoryProvider", diff --git a/sdk/nexent/memory/embedding_model.py b/sdk/nexent/memory/embedding_model.py index 4ae0553eb2..6c2ef19725 100644 --- a/sdk/nexent/memory/embedding_model.py +++ b/sdk/nexent/memory/embedding_model.py @@ -1,4 +1,4 @@ -"""Embedding model metadata and client cache for the memory system. +"""Embedding model metadata and client factory for the memory system. This module provides: @@ -10,11 +10,8 @@ mem_{model_repo}_{model_name}_{dimension} mem_{model_name}_{dimension} # when model_repo is absent -- ``get_embedding_client()``: a process-wide cache that reuses - ``OpenAICompatibleEmbeddingAdapter`` instances keyed by ``(model_name, dimension)``. - Creating an HTTP client per memory write would add unnecessary latency; - caching a single instance per model avoids that while keeping the SDK - layer stateless. +- ``get_embedding_client()``: builds an ``OpenAICompatibleEmbeddingAdapter`` + from the caller-supplied configuration. The SDK never talks to Elasticsearch directly. All vector writes go through the backend layer (``memory_index_service``). This module is therefore purely @@ -23,15 +20,12 @@ from __future__ import annotations -import logging import re from dataclasses import dataclass -from typing import List, Optional +from typing import Optional -from ..core.gateway.modality import OpenAICompatibleEmbeddingAdapter from ..core.gateway import EmbeddingContext - -logger = logging.getLogger("memory_embedding_model") +from ..core.gateway.modality import OpenAICompatibleEmbeddingAdapter def _sanitize_index_component(value: str) -> str: @@ -79,10 +73,8 @@ def get_index_name(self) -> str: # --------------------------------------------------------------------------- # -# Process-wide HTTP client cache # +# Embedding client factory # # --------------------------------------------------------------------------- # -# Key = "model_name:dimension", Value = OpenAICompatibleEmbeddingAdapter instance. -_embedding_client_cache: dict[str, OpenAICompatibleEmbeddingAdapter] = {} def get_embedding_client( @@ -93,11 +85,7 @@ def get_embedding_client( model_repo: Optional[str] = None, ssl_verify: bool = True, ) -> OpenAICompatibleEmbeddingAdapter: - """Return a cached ``OpenAICompatibleEmbeddingAdapter`` instance. - - Instances are cached by ``(model_repo, model_name, dimension)`` so that - repeated memory writes within the same process reuse the underlying HTTP - client and connection pool. + """Return an ``OpenAICompatibleEmbeddingAdapter`` instance. When ``model_repo`` is provided (e.g. ``"BAAI"``), the fully-qualified name ``"BAAI/bge-m3"`` is passed to the API. Some providers (e.g. @@ -113,35 +101,18 @@ def get_embedding_client( ssl_verify: Whether to verify SSL certificates. Returns: - A cached (or newly created) ``OpenAICompatibleEmbeddingAdapter`` instance. + A newly created ``OpenAICompatibleEmbeddingAdapter`` instance. """ - cache_key = f"{model_repo or ''}:{model_name}:{dimension}" - if cache_key not in _embedding_client_cache: - # Form the fully-qualified model name the API expects. - full_model_name = f"{model_repo}/{model_name}" if model_repo else model_name - _embedding_client_cache[cache_key] = OpenAICompatibleEmbeddingAdapter( - EmbeddingContext( - model_name=full_model_name, - base_url=base_url, - api_key=api_key, - modality="embedding", - factory="openai", - embedding_dim=dimension, - ssl_verify=ssl_verify, - ) - ) - logger.debug( - "Created and cached embedding client for model=%s dim=%d", - full_model_name, - dimension, + # Form the fully-qualified model name the API expects. + full_model_name = f"{model_repo}/{model_name}" if model_repo else model_name + return OpenAICompatibleEmbeddingAdapter( + EmbeddingContext( + model_name=full_model_name, + base_url=base_url, + api_key=api_key, + modality="embedding", + factory="openai", + embedding_dim=dimension, + ssl_verify=ssl_verify, ) - return _embedding_client_cache[cache_key] - - -def reset_embedding_client_cache() -> None: - """Clear all cached embedding client instances. - - Call this in test teardown to ensure test isolation. - """ - _embedding_client_cache.clear() - logger.debug("Cleared embedding client cache") + ) diff --git a/test/sdk/core/gateway/test_model_context.py b/test/sdk/core/gateway/test_model_context.py index 4bd521a875..41d1c45d61 100644 --- a/test/sdk/core/gateway/test_model_context.py +++ b/test/sdk/core/gateway/test_model_context.py @@ -3,19 +3,7 @@ from nexent.core.gateway.model_context import LLMContext, VLMContext -def test_cache_key_uses_empty_defaults(): - context = VLMContext( - model_name="qwen-vl-max", - base_url="https://api.example.com", - api_key="sk-key", - modality="vlm", - factory="openai", - ) - - assert context.cache_key() == ("", "vlm", "", "qwen-vl-max", "openai") - - -def test_cache_key_includes_tenant_and_slot(): +def test_context_carries_full_connection_config(): context = VLMContext( model_name="qwen-vl-max", base_url="https://api.example.com", @@ -24,9 +12,15 @@ def test_cache_key_includes_tenant_and_slot(): factory="openai", tenant_id="tenant-1", slot="vlm3", + ssl_verify=False, + timeout_seconds=12.5, ) - assert context.cache_key() == ("tenant-1", "vlm", "vlm3", "qwen-vl-max", "openai") + assert context.base_url == "https://api.example.com" + assert context.api_key == "sk-key" + assert context.ssl_verify is False + assert context.timeout_seconds == 12.5 + assert (context.tenant_id, context.slot) == ("tenant-1", "vlm3") def test_subclass_fields_are_independent(): @@ -41,4 +35,4 @@ def test_subclass_fields_are_independent(): ) assert llm.temperature == 0.2 assert llm.stream is True - assert not hasattr(llm, "capabilities") \ No newline at end of file + assert not hasattr(llm, "capabilities") diff --git a/test/sdk/core/gateway/test_multimodal_gateway.py b/test/sdk/core/gateway/test_multimodal_gateway.py index c15a38c4ee..b0c30d5166 100644 --- a/test/sdk/core/gateway/test_multimodal_gateway.py +++ b/test/sdk/core/gateway/test_multimodal_gateway.py @@ -1,4 +1,4 @@ -"""Unit tests for MultimodalGateway caching and delegation.""" +"""Unit tests for MultimodalGateway adapter construction and delegation.""" import pytest from nexent.core.gateway.model_context import VLMContext @@ -49,7 +49,7 @@ def _make_context(model_name="dummy-model"): ) -def test_get_adapter_builds_and_caches_by_context(): +def test_get_adapter_builds_a_new_instance_on_every_call(): gateway = MultimodalGateway(_make_registry()) context = _make_context() @@ -57,11 +57,12 @@ def test_get_adapter_builds_and_caches_by_context(): second = gateway.get_adapter(context) assert isinstance(first, _FakeAdapter) - assert first is second + assert first is not second assert first._context is context + assert second._context is context -def test_get_adapter_builds_separate_instance_for_different_key(): +def test_get_adapter_builds_separate_instances_for_different_models(): gateway = MultimodalGateway(_make_registry()) first = gateway.get_adapter(_make_context("model-a")) @@ -70,6 +71,29 @@ def test_get_adapter_builds_separate_instance_for_different_key(): assert first is not second +def test_get_adapter_does_not_share_endpoint_or_credentials(): + gateway = MultimodalGateway(_make_registry()) + + stale = gateway.get_adapter(_make_context()) + rotated_context = _make_context() + rotated_context.base_url = "https://rotated.example.com" + rotated_context.api_key = "sk-rotated" + rotated = gateway.get_adapter(rotated_context) + + assert stale is not rotated + assert rotated._context.base_url == "https://rotated.example.com" + assert rotated._context.api_key == "sk-rotated" + assert stale._context.api_key == "sk-key" + + +def test_gateway_keeps_no_adapter_cache(): + gateway = MultimodalGateway(_make_registry()) + gateway.get_adapter(_make_context()) + + assert not hasattr(gateway, "_adapter_cache") + assert not hasattr(gateway, "invalidate") + + def test_gateway_defaults_to_process_registry(): gateway = MultimodalGateway() context = VLMContext( @@ -103,24 +127,6 @@ async def test_health_check_delegates_to_adapter(gateway, context): assert await gateway.health_check(context) is True -def test_invalidate_single_context(gateway, context): - cached = gateway.get_adapter(context) - assert gateway.get_adapter(context) is cached - - gateway.invalidate(context) - assert gateway.get_adapter(context) is not cached - - -def test_invalidate_all_contexts(gateway, context): - other_context = _make_context("model-other") - first_cached = gateway.get_adapter(context) - other_cached = gateway.get_adapter(other_context) - - gateway.invalidate() - - assert gateway.get_adapter(context) is not first_cached - assert gateway.get_adapter(other_context) is not other_cached - def test_get_gateway_is_lazy_singleton(): from nexent.core.gateway import multimodal_gateway as gateway_module @@ -140,4 +146,4 @@ def gateway(): @pytest.fixture def context(): - return _make_context() \ No newline at end of file + return _make_context() diff --git a/test/sdk/memory/test_embedding_model.py b/test/sdk/memory/test_embedding_model.py index 890aa83919..ad0466c14a 100644 --- a/test/sdk/memory/test_embedding_model.py +++ b/test/sdk/memory/test_embedding_model.py @@ -1,15 +1,12 @@ -"""Tests for embedding model metadata and client cache.""" +"""Tests for embedding model metadata and the embedding client factory.""" from unittest.mock import MagicMock -import pytest - from nexent.core.gateway import EmbeddingContext from nexent.memory.embedding_model import ( EmbeddingModelInfo, _sanitize_index_component, get_embedding_client, - reset_embedding_client_cache, ) @@ -97,20 +94,14 @@ def test_ssl_verify_default_true(self): # --------------------------------------------------------------------------- # -# get_embedding_client / reset_embedding_client_cache # +# get_embedding_client # # --------------------------------------------------------------------------- # -class TestEmbeddingClientCache: - """Tests for the process-wide HTTP client cache.""" - - def setup_method(self): - reset_embedding_client_cache() - - def teardown_method(self): - reset_embedding_client_cache() +class TestEmbeddingClientFactory: + """Tests for the embedding client factory.""" - def test_cache_miss_creates_instance(self, mocker): - """First call with a given key must create and cache the client.""" + def test_builds_adapter_from_context(self, mocker): + """The adapter is constructed from an EmbeddingContext built out of the arguments.""" mock_init = mocker.patch( "nexent.memory.embedding_model.OpenAICompatibleEmbeddingAdapter" ) @@ -120,17 +111,15 @@ def test_cache_miss_creates_instance(self, mocker): client = get_embedding_client( model_name="text-embedding-3-small", dimension=1536, - base_url="https://api.openai.com/v1", + base_url="https://api.openai.com/v1/embeddings", api_key="sk-test", ) assert client is mock_instance - # The migrated client is constructed from an EmbeddingContext (one - # positional arg); dataclass equality verifies every field. mock_init.assert_called_once_with( EmbeddingContext( model_name="text-embedding-3-small", - base_url="https://api.openai.com/v1", + base_url="https://api.openai.com/v1/embeddings", api_key="sk-test", modality="embedding", factory="openai", @@ -139,138 +128,74 @@ def test_cache_miss_creates_instance(self, mocker): ) ) - def test_cache_hit_returns_same_instance(self, mocker): - """Subsequent calls with the same key must return the cached instance.""" - mock_init = mocker.patch( - "nexent.memory.embedding_model.OpenAICompatibleEmbeddingAdapter" - ) - mock_instance = MagicMock() - mock_init.return_value = mock_instance - - client1 = get_embedding_client( - model_name="text-embedding-3-small", - dimension=1536, - base_url="https://api.openai.com/v1", - api_key="sk-test", - ) - client2 = get_embedding_client( - model_name="text-embedding-3-small", - dimension=1536, - base_url="https://api.openai.com/v1", - api_key="sk-test", - ) - - # Only one instance should have been created - assert mock_init.call_count == 1 - # Both calls should return the same object - assert client1 is client2 - - def test_different_dimension_returns_different_instance(self, mocker): - """Different dimensions are separate cache entries.""" - mock_init = mocker.patch( - "nexent.memory.embedding_model.OpenAICompatibleEmbeddingAdapter" - ) - mock1 = MagicMock() - mock2 = MagicMock() - mock_init.side_effect = [mock1, mock2] - - c1 = get_embedding_client( - model_name="text-embedding-3-small", - dimension=1536, - base_url="https://api.openai.com/v1", - api_key="sk-test", - ) - c2 = get_embedding_client( - model_name="text-embedding-3-small", - dimension=256, - base_url="https://api.openai.com/v1", - api_key="sk-test", - ) - - assert mock_init.call_count == 2 - assert c1 is not c2 - - def test_model_repo_used_in_cache_key(self, mocker): - """Different model_repo values must produce separate cache entries. - - The cache key is ``(model_repo, model_name, dimension)`` so that - tenants using different embedding vendors (e.g. ``openai`` vs. - ``local``) get their own client instances. - """ + def test_model_repo_prefixes_model_name(self, mocker): + """model_repo is prepended so vendors such as SiliconFlow receive "BAAI/bge-m3".""" mock_init = mocker.patch( "nexent.memory.embedding_model.OpenAICompatibleEmbeddingAdapter" ) - mock_instance = MagicMock() - mock_init.return_value = mock_instance - # First call with a repo get_embedding_client( - model_name="text-embedding-3-small", - dimension=1536, - base_url="https://api.openai.com/v1", + model_name="bge-m3", + dimension=1024, + base_url="https://api.siliconflow.cn/v1/embeddings", api_key="sk-test", - model_repo="openai", - ) - # Second call with different repo but same model_name + dimension - get_embedding_client( - model_name="text-embedding-3-small", - dimension=1536, - base_url="https://different.example.com", - api_key="sk-other", - model_repo="other-repo", + model_repo="BAAI", ) - # Different repos must not collide — one instance per repo. - assert mock_init.call_count == 2 + context = mock_init.call_args[0][0] + assert context.model_name == "BAAI/bge-m3" + assert context.embedding_dim == 1024 - def test_same_model_repo_hits_cache(self, mocker): - """Same (model_repo, model_name, dimension) returns the cached client.""" + def test_every_call_builds_a_new_adapter(self, mocker): + """Identical arguments must not be served from a shared instance.""" mock_init = mocker.patch( "nexent.memory.embedding_model.OpenAICompatibleEmbeddingAdapter" ) - mock_instance = MagicMock() - mock_init.return_value = mock_instance + mock_init.side_effect = lambda context: MagicMock(context=context) - get_embedding_client( + first = get_embedding_client( model_name="text-embedding-3-small", dimension=1536, - base_url="https://api.openai.com/v1", + base_url="https://api.openai.com/v1/embeddings", api_key="sk-test", - model_repo="openai", ) - get_embedding_client( + second = get_embedding_client( model_name="text-embedding-3-small", dimension=1536, - base_url="https://api.openai.com/v1", + base_url="https://api.openai.com/v1/embeddings", api_key="sk-test", - model_repo="openai", ) - assert mock_init.call_count == 1 - - def test_reset_clears_cache(self, mocker): - """reset_embedding_client_cache() must empty the cache so the next call - creates a fresh instance.""" - mock_init = mocker.patch( - "nexent.memory.embedding_model.OpenAICompatibleEmbeddingAdapter" - ) - mock_instance = MagicMock() - mock_init.return_value = mock_instance + assert mock_init.call_count == 2 + assert first is not second - get_embedding_client( - model_name="text-embedding-3-small", - dimension=1536, - base_url="https://api.openai.com/v1", - api_key="sk-test", - ) - reset_embedding_client_cache() + def test_changed_endpoint_and_credentials_are_not_shared(self): + """Same repo/name/dimension with a different endpoint, key or TLS setting + must never reuse another caller's client. - # After reset a new instance must be created - get_embedding_client( - model_name="text-embedding-3-small", - dimension=1536, - base_url="https://api.openai.com/v1", - api_key="sk-test", + Real adapters are used because constructing one performs no I/O. + """ + tenant_a = get_embedding_client( + model_name="bge-m3", + dimension=1024, + base_url="https://api.siliconflow.cn/v1/embeddings", + api_key="sk-tenant-a", + model_repo="BAAI", + ssl_verify=True, ) - - assert mock_init.call_count == 2 + tenant_b = get_embedding_client( + model_name="bge-m3", + dimension=1024, + base_url="http://10.0.0.7:8000/v1/embeddings", + api_key="sk-tenant-b", + model_repo="BAAI", + ssl_verify=False, + ) + + assert tenant_a is not tenant_b + assert tenant_a._base_url == "https://api.siliconflow.cn/v1/embeddings" + assert tenant_a._headers["Authorization"] == "Bearer sk-tenant-a" + assert tenant_a._ssl_verify is True + assert tenant_b._base_url == "http://10.0.0.7:8000/v1/embeddings" + assert tenant_b._headers["Authorization"] == "Bearer sk-tenant-b" + assert tenant_b._ssl_verify is False