Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 ``~/.basic-memory``.

Works cross-platform: ``Path.home()`` returns ``%USERPROFILE%`` on
Windows and ``$HOME`` on macOS/Linux.
"""
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


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.environ.get("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
26 changes: 20 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,11 @@
_FASTEMBED_MAX_THREADS = 8


def _resolve_cache_dir(app_config: BasicMemoryConfig) -> str:
"""Resolve the effective FastEmbed cache dir for this config."""
return app_config.semantic_embedding_cache_dir or 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 +66,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 +113,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
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
94 changes: 94 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,50 @@ 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_tmp(self, config_home, monkeypatch):
"""Regression guard for #741: default must not point at /tmp/fastembed_cache."""
monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False)
monkeypatch.delenv("FASTEMBED_CACHE_PATH", raising=False)

resolved = default_fastembed_cache_dir()

assert "/tmp/fastembed_cache" not in resolved
assert not resolved.startswith(tempfile.gettempdir())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Remove brittle temp-root assertion

This assertion will fail in common test environments because config_home points HOME at tmp_path, and tmp_path is normally created under tempfile.gettempdir(). In that case a correct resolved cache path like <tmp_path>/.basic-memory/fastembed_cache still starts with the temp root, so this check reports a false failure even though the code no longer uses FastEmbed’s /tmp/fastembed_cache default.

Useful? React with 👍 / 👎.



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

Expand Down
Loading