Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 47 additions & 16 deletions src/basic_memory/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,44 @@ def _default_semantic_search_enabled() -> bool:
)


def resolve_data_dir() -> Path:
"""Resolve the Basic Memory data directory.

Single source of truth for the per-user state directory. Honors
``BASIC_MEMORY_CONFIG_DIR`` so each process/worktree can isolate config
and database state; otherwise falls back to ``<user home>/.basic-memory``.

Cross-platform: ``Path.home()`` reads ``$HOME`` on POSIX and
``%USERPROFILE%`` on Windows, so there's no need to check ``$HOME``
explicitly here.
"""
if config_dir := os.getenv("BASIC_MEMORY_CONFIG_DIR"):
return Path(config_dir)
return Path.home() / DATA_DIR_NAME


def default_fastembed_cache_dir() -> str:
"""Return the default cache directory used for FastEmbed model artifacts.

Resolution order:
1. ``FASTEMBED_CACHE_PATH`` env var — honors FastEmbed's own convention
so users who already configure it through the environment keep working.
2. ``<basic-memory data dir>/fastembed_cache`` — the same stable,
user-writable directory Basic Memory already uses for config and
the default SQLite database. Honors ``BASIC_MEMORY_CONFIG_DIR``.

Why not ``tempfile.gettempdir()``?
FastEmbed's own default is ``<system tmp>/fastembed_cache``, which is
ephemeral in many sandboxed MCP runtimes (e.g. Codex CLI wipes /tmp
between invocations). The model then disappears and every subsequent
ONNX load raises ``NO_SUCHFILE``. Persisting the cache under the
per-user data directory works identically on macOS, Linux, and Windows.
"""
if env_override := os.getenv("FASTEMBED_CACHE_PATH"):
return env_override
return str(resolve_data_dir() / "fastembed_cache")


@dataclass
class ProjectConfig:
"""Configuration for a specific basic-memory project."""
Expand Down Expand Up @@ -222,7 +260,13 @@ def __init__(self, **data: Any) -> None: ...
)
semantic_embedding_cache_dir: str | None = Field(
default=None,
description="Optional cache directory for FastEmbed model artifacts.",
description=(
"Optional override for the FastEmbed model cache directory. "
"When unset, Basic Memory resolves this at runtime to "
"<basic-memory data dir>/fastembed_cache (or FASTEMBED_CACHE_PATH "
"when that env var is set) so the model persists across runs "
"without hardcoding a path into config.json."
),
)
semantic_embedding_threads: int | None = Field(
default=None,
Expand Down Expand Up @@ -709,11 +753,7 @@ def ensure_project_paths_exists(self) -> "BasicMemoryConfig": # pragma: no cove
@property
def data_dir_path(self) -> Path:
"""Get app state directory for config and default SQLite database."""
if config_dir := os.getenv("BASIC_MEMORY_CONFIG_DIR"):
return Path(config_dir)

home = os.getenv("HOME", Path.home())
return Path(home) / DATA_DIR_NAME
return resolve_data_dir()


# Module-level cache for configuration
Expand All @@ -731,16 +771,7 @@ class ConfigManager:

def __init__(self) -> None:
"""Initialize the configuration manager."""
home = os.getenv("HOME", Path.home())
if isinstance(home, str):
home = Path(home)

# Allow override via environment variable
if config_dir := os.getenv("BASIC_MEMORY_CONFIG_DIR"):
self.config_dir = Path(config_dir)
else:
self.config_dir = home / DATA_DIR_NAME

self.config_dir = resolve_data_dir()
self.config_file = self.config_dir / CONFIG_FILE_NAME

# Ensure config directory exists
Expand Down
35 changes: 29 additions & 6 deletions src/basic_memory/repository/embedding_provider_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import os
from threading import Lock

from basic_memory.config import BasicMemoryConfig
from basic_memory.config import BasicMemoryConfig, default_fastembed_cache_dir
from basic_memory.repository.embedding_provider import EmbeddingProvider

type ProviderCacheKey = tuple[
Expand All @@ -12,7 +12,7 @@
int | None,
int,
int,
str | None,
str,
int | None,
int | None,
]
Expand All @@ -22,6 +22,20 @@
_FASTEMBED_MAX_THREADS = 8


def _resolve_cache_dir(app_config: BasicMemoryConfig) -> str:
"""Resolve the effective FastEmbed cache dir for this config.

Uses an explicit ``is not None`` check — an empty string override from
config or ``BASIC_MEMORY_SEMANTIC_EMBEDDING_CACHE_DIR`` is an invalid
path, not a request to fall back to the default, and FastEmbed's error
message is clearer than silently swapping in a different directory.
"""
configured = app_config.semantic_embedding_cache_dir
if configured is not None:
return configured
return default_fastembed_cache_dir()


def _available_cpu_count() -> int | None:
"""Return the CPU budget available to this process when the runtime exposes it."""
process_cpu_count = getattr(os, "process_cpu_count", None)
Expand Down Expand Up @@ -61,15 +75,20 @@ def _resolve_fastembed_runtime_knobs(


def _provider_cache_key(app_config: BasicMemoryConfig) -> ProviderCacheKey:
"""Build a stable cache key from provider-relevant semantic embedding config."""
"""Build a stable cache key from provider-relevant semantic embedding config.

Uses the *resolved* cache dir — not the raw config field — so different
FASTEMBED_CACHE_PATH values produce distinct cache keys even when the
config field itself is unset.
"""
resolved_threads, resolved_parallel = _resolve_fastembed_runtime_knobs(app_config)
return (
app_config.semantic_embedding_provider.strip().lower(),
app_config.semantic_embedding_model,
app_config.semantic_embedding_dimensions,
app_config.semantic_embedding_batch_size,
app_config.semantic_embedding_request_concurrency,
app_config.semantic_embedding_cache_dir,
_resolve_cache_dir(app_config),
resolved_threads,
resolved_parallel,
)
Expand Down Expand Up @@ -103,8 +122,12 @@ def create_embedding_provider(app_config: BasicMemoryConfig) -> EmbeddingProvide
from basic_memory.repository.fastembed_provider import FastEmbedEmbeddingProvider

resolved_threads, resolved_parallel = _resolve_fastembed_runtime_knobs(app_config)
if app_config.semantic_embedding_cache_dir is not None:
extra_kwargs["cache_dir"] = app_config.semantic_embedding_cache_dir
# Trigger: cache_dir is resolved rather than passed through directly.
# Why: FastEmbed's own default caches to <system tmp>/fastembed_cache,
# which disappears in sandboxed MCP runtimes (e.g. Codex CLI). See #741.
# Outcome: always pass an explicit, user-writable cache dir so the ONNX
# model persists across runs.
extra_kwargs["cache_dir"] = _resolve_cache_dir(app_config)
if resolved_threads is not None:
extra_kwargs["threads"] = resolved_threads
if resolved_parallel is not None:
Expand Down
6 changes: 6 additions & 0 deletions test-int/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,13 @@ async def test_project(config_home, engine_factory) -> Project:

@pytest.fixture
def config_home(tmp_path, monkeypatch) -> Path:
# Patch both HOME and USERPROFILE so Path.home() returns the test dir on
# every platform — Path.home() reads HOME on POSIX and USERPROFILE on
# Windows, and ConfigManager.data_dir_path now goes through Path.home()
# via resolve_data_dir(). Must mirror tests/conftest.py:config_home.
monkeypatch.setenv("HOME", str(tmp_path))
if os.name == "nt":
monkeypatch.setenv("USERPROFILE", str(tmp_path))
# Set BASIC_MEMORY_HOME to the test directory
monkeypatch.setenv("BASIC_MEMORY_HOME", str(tmp_path / "basic-memory"))
return tmp_path
Expand Down
61 changes: 61 additions & 0 deletions tests/repository/test_openai_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ def test_embedding_provider_factory_uses_provider_defaults_when_dimensions_not_s

def test_embedding_provider_factory_forwards_fastembed_runtime_knobs():
"""Factory should forward FastEmbed runtime tuning config fields."""
reset_embedding_provider_cache()
config = BasicMemoryConfig(
env="test",
projects={"test-project": "/tmp/basic-memory-test"},
Expand All @@ -265,6 +266,66 @@ def test_embedding_provider_factory_forwards_fastembed_runtime_knobs():
assert provider.parallel == 2


def test_embedding_provider_factory_uses_default_cache_dir_when_unset(config_home, monkeypatch):
"""Factory should pass the data-dir-relative default when cache_dir is None.

Legacy configs that carry an explicit ``semantic_embedding_cache_dir: null``
must still get a user-writable cache path rather than letting FastEmbed fall
back to ``<tmp>/fastembed_cache``. See #741.
"""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)
reset_embedding_provider_cache()

config = BasicMemoryConfig(
env="test",
projects={"test-project": str(config_home / "project")},
default_project="test-project",
semantic_search_enabled=True,
semantic_embedding_provider="fastembed",
semantic_embedding_cache_dir=None,
)

provider = create_embedding_provider(config)
assert isinstance(provider, FastEmbedEmbeddingProvider)
expected = str(config_home / ".basic-memory" / "fastembed_cache")
assert provider.cache_dir == expected


def test_embedding_provider_factory_cache_key_reflects_resolved_cache_dir(
config_home, tmp_path, monkeypatch
):
"""Changing FASTEMBED_CACHE_PATH must yield a distinct cached provider.

The provider cache key uses the *resolved* cache dir rather than the raw
(nullable) config field, so env-driven path changes invalidate the cache
instead of silently returning a stale provider pointing at the old path.
"""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)
reset_embedding_provider_cache()

base_kwargs = dict(
env="test",
projects={"test-project": str(config_home / "project")},
default_project="test-project",
semantic_search_enabled=True,
semantic_embedding_provider="fastembed",
semantic_embedding_cache_dir=None,
)

provider_a = create_embedding_provider(BasicMemoryConfig(**base_kwargs))
assert isinstance(provider_a, FastEmbedEmbeddingProvider)

monkeypatch.setenv("FASTEMBED_CACHE_PATH", str(tmp_path / "alt-cache"))
provider_b = create_embedding_provider(BasicMemoryConfig(**base_kwargs))

assert isinstance(provider_b, FastEmbedEmbeddingProvider)
assert provider_b is not provider_a
assert provider_a.cache_dir == str(config_home / ".basic-memory" / "fastembed_cache")
assert provider_b.cache_dir == str(tmp_path / "alt-cache")


def test_fastembed_provider_reports_runtime_log_attrs():
"""FastEmbed should expose the resolved runtime knobs for batch startup logs."""
provider = FastEmbedEmbeddingProvider(batch_size=128, threads=4, parallel=2)
Expand Down
109 changes: 109 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
ConfigManager,
ProjectEntry,
ProjectMode,
default_fastembed_cache_dir,
resolve_data_dir,
)
from pathlib import Path

Expand Down Expand Up @@ -129,6 +131,54 @@ def test_app_database_path_defaults_to_home_data_dir(self, config_home, monkeypa
assert config.data_dir_path == config_home / ".basic-memory"
assert config.app_database_path == config_home / ".basic-memory" / "memory.db"

def test_semantic_embedding_cache_dir_field_stays_none_by_default(
self, config_home, monkeypatch
):
"""The raw config field stays None so it isn't persisted into config.json.

Resolution to a concrete path happens in embedding_provider_factory at
provider construction time, so ``BASIC_MEMORY_CONFIG_DIR`` and
``FASTEMBED_CACHE_PATH`` changes take effect on every run instead of
being frozen by the first save. See #741.
"""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)

config = BasicMemoryConfig()

assert config.semantic_embedding_cache_dir is None

def test_semantic_embedding_cache_dir_not_persisted_in_model_dump(
self, config_home, monkeypatch
):
"""model_dump must not bake a resolved cache path into config.json.

Regression guard for #741: persisting the default would freeze stale
paths when users later change BASIC_MEMORY_CONFIG_DIR or
FASTEMBED_CACHE_PATH.
"""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)

dumped = BasicMemoryConfig().model_dump(mode="json")

assert dumped["semantic_embedding_cache_dir"] is None

def test_semantic_embedding_cache_dir_explicit_user_value_preserved(
self, config_home, monkeypatch
):
"""An explicit user override still round-trips through model_dump."""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)

config = BasicMemoryConfig(semantic_embedding_cache_dir="/custom/explicit/path")

assert config.semantic_embedding_cache_dir == "/custom/explicit/path"
assert (
config.model_dump(mode="json")["semantic_embedding_cache_dir"]
== "/custom/explicit/path"
)

def test_explicit_default_project_preserved(self, config_home, monkeypatch):
"""Test that a valid explicit default_project is not overwritten by model_post_init."""
monkeypatch.delenv("BASIC_MEMORY_HOME", raising=False)
Expand Down Expand Up @@ -252,6 +302,65 @@ def test_config_file_without_default_project_key(self, config_home, monkeypatch)
assert loaded.default_project == "work"


class TestDataDirHelpers:
"""Module-level helpers that resolve the Basic Memory data directory."""

def test_resolve_data_dir_defaults_to_home_dot_basic_memory(self, config_home, monkeypatch):
"""Without BASIC_MEMORY_CONFIG_DIR, resolver returns ~/.basic-memory."""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)

assert resolve_data_dir() == config_home / ".basic-memory"

def test_resolve_data_dir_honors_config_dir_env(self, tmp_path, monkeypatch):
"""BASIC_MEMORY_CONFIG_DIR overrides the default location."""
custom = tmp_path / "elsewhere"
monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(custom))

assert resolve_data_dir() == custom

def test_default_fastembed_cache_dir_uses_data_dir(self, config_home, monkeypatch):
"""Default cache path is a subdir of the Basic Memory data dir."""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)

assert default_fastembed_cache_dir() == str(
config_home / ".basic-memory" / "fastembed_cache"
)

def test_default_fastembed_cache_dir_env_override(self, tmp_path, monkeypatch):
"""FASTEMBED_CACHE_PATH is preferred when set."""
custom = tmp_path / "custom-cache"
monkeypatch.setenv("FASTEMBED_CACHE_PATH", str(custom))
monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(tmp_path / "state"))

assert default_fastembed_cache_dir() == str(custom)

def test_default_fastembed_cache_dir_never_falls_back_to_fastembed_tmp_default(
self, config_home, monkeypatch
):
"""Regression guard for #741.

FastEmbed's own fallback when ``cache_dir`` is ``None`` is
``<system tmp>/fastembed_cache`` — the exact path that disappears in
sandboxed MCP runtimes (Codex CLI). Ensure Basic Memory's resolver
never lands on that path.

Compared as exact paths rather than ``startswith(tempfile.gettempdir())``
because the test runner itself can legitimately live under ``/tmp``
(pytest's ``tmp_path`` does on Linux CI), and that's not the bug we
care about here.
"""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)

resolved = Path(default_fastembed_cache_dir())

# Must equal the data-dir-relative default.
assert resolved == config_home / ".basic-memory" / "fastembed_cache"
# And must not equal FastEmbed's own <tempdir>/fastembed_cache fallback.
assert resolved != Path(tempfile.gettempdir()) / "fastembed_cache"


class TestConfigManager:
"""Test ConfigManager functionality."""

Expand Down
Loading