From bf57f0648552f30da7ebf33c680e33ada941406d Mon Sep 17 00:00:00 2001 From: phernandez Date: Tue, 1 Sep 2026 15:04:06 -0500 Subject: [PATCH 1/2] fix(ci): pin the uv interpreter so the matrix tests what it claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every uv invocation in CI resolved its interpreter from `.python-version` (3.14) instead of the version the job set up. The matrix rows labeled "Python 3.12" and "Python 3.13" therefore built and ran against 3.14 — we have had no coverage of either version despite reporting it, while `requires-python` declares `>=3.12`. Pinning `uv venv` alone is not enough. Every `just` recipe shells out to `uv run`, which re-resolves the interpreter and DELETES a `.venv` that disagrees, recreating it from the lockfile. That drops anything installed by the job's `uv pip install -e ".[dev,milvus,pdf]"` step, which is how a pinned 3.12 venv ends up failing typecheck on an unresolved `pymilvus`. Set UV_PYTHON at job level instead: it governs every uv call, including the ones inside just recipes, so the environment survives and the job runs the version its name claims. Covers all ten jobs that build a uv environment. claude.yml and release.yml had the same defect — release.yml was building the published artifact on 3.14 while declaring 3.12 — and consolidated-packages.yml's hermes job ran `uv python install 3.12`, which only downloads that interpreter without selecting it. Signed-off-by: phernandez --- .github/workflows/claude.yml | 9 +++++++ .github/workflows/consolidated-packages.yml | 5 ++++ .github/workflows/release.yml | 9 +++++++ .github/workflows/test.yml | 29 +++++++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 4e5d20606..76b3567b0 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -32,6 +32,11 @@ on: env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" +# Every job that builds a uv environment sets UV_PYTHON. Without it, `uv run` +# — which every `just` recipe shells out to — resolves its interpreter from +# .python-version (3.14) rather than the matrix version, and DELETES a .venv +# that disagrees, taking the pip-installed extras with it. That is why the +# 3.12/3.13 matrix rows silently tested 3.14 and why typecheck lost pymilvus. jobs: claude: if: | @@ -58,6 +63,10 @@ jobs: issues: write id-token: write actions: read # Required for Claude to read CI results on PRs + # Pin every uv call, including `uv run` inside just recipes. See top of file. + env: + UV_PYTHON: "3.12" + steps: - name: Checkout repository uses: actions/checkout@v6 diff --git a/.github/workflows/consolidated-packages.yml b/.github/workflows/consolidated-packages.yml index bbe7f4ee5..e49e45ab1 100644 --- a/.github/workflows/consolidated-packages.yml +++ b/.github/workflows/consolidated-packages.yml @@ -78,6 +78,11 @@ jobs: defaults: run: working-directory: integrations/hermes + # `uv python install 3.12` only downloads that interpreter; it does not + # select it. Without this, `just check` runs `uv run`, which resolves from + # .python-version (3.14) instead. + env: + UV_PYTHON: "3.12" steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e8a8c596a..fd65e387d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -8,6 +8,11 @@ on: env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" +# Every job that builds a uv environment sets UV_PYTHON. Without it, `uv run` +# — which every `just` recipe shells out to — resolves its interpreter from +# .python-version (3.14) rather than the matrix version, and DELETES a .venv +# that disagrees, taking the pip-installed extras with it. That is why the +# 3.12/3.13 matrix rows silently tested 3.14 and why typecheck lost pymilvus. jobs: release: runs-on: ubuntu-latest @@ -15,6 +20,10 @@ jobs: id-token: write contents: write + # Pin every uv call, including `uv run` inside just recipes. See top of file. + env: + UV_PYTHON: "3.12" + steps: - uses: actions/checkout@v6 with: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 80be1c9ea..5f46fbc24 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,6 +22,11 @@ env: # to main run the full suite with --testmon-noselect to refresh the baseline. BASIC_MEMORY_TESTMON_FLAGS: ${{ github.ref_name == 'main' && '--testmon-noselect' || '--testmon --testmon-forceselect' }} +# Every job that builds a uv environment sets UV_PYTHON. Without it, `uv run` +# — which every `just` recipe shells out to — resolves its interpreter from +# .python-version (3.14) rather than the matrix version, and DELETES a .venv +# that disagrees, taking the pip-installed extras with it. That is why the +# 3.12/3.13 matrix rows silently tested 3.14 and why typecheck lost pymilvus. jobs: changes: # Docs/workflow-only changes skip the entire test matrix while the workflow @@ -57,6 +62,10 @@ jobs: timeout-minutes: 20 runs-on: ubuntu-latest + # Pin every uv call, including `uv run` inside just recipes. See top of file. + env: + UV_PYTHON: "3.12" + steps: - uses: actions/checkout@v6 with: @@ -111,6 +120,10 @@ jobs: python-version: "3.12" runs-on: ${{ matrix.os }} + # Pin every uv call, including `uv run` inside just recipes. See top of file. + env: + UV_PYTHON: ${{ matrix.python-version }} + steps: - uses: actions/checkout@v6 with: @@ -172,6 +185,10 @@ jobs: python-version: "3.12" runs-on: ${{ matrix.os }} + # Pin every uv call, including `uv run` inside just recipes. See top of file. + env: + UV_PYTHON: ${{ matrix.python-version }} + steps: - uses: actions/checkout@v6 with: @@ -245,6 +262,8 @@ jobs: --health-retries 5 env: BASIC_MEMORY_TEST_POSTGRES_URL: postgresql://basic_memory_user:dev_password@127.0.0.1:5432/basic_memory_test + # Pin every uv call, including `uv run` inside just recipes. See top of file. + UV_PYTHON: ${{ matrix.python-version }} steps: - uses: actions/checkout@v6 @@ -317,6 +336,8 @@ jobs: --health-retries 5 env: BASIC_MEMORY_TEST_POSTGRES_URL: postgresql://basic_memory_user:dev_password@127.0.0.1:5432/basic_memory_test + # Pin every uv call, including `uv run` inside just recipes. See top of file. + UV_PYTHON: ${{ matrix.python-version }} steps: - uses: actions/checkout@v6 @@ -367,6 +388,10 @@ jobs: timeout-minutes: 45 runs-on: ubuntu-latest + # Pin every uv call, including `uv run` inside just recipes. See top of file. + env: + UV_PYTHON: "3.12" + steps: - uses: actions/checkout@v6 with: @@ -426,6 +451,10 @@ jobs: python-version: "3.12" runs-on: ${{ matrix.os }} + # Pin every uv call, including `uv run` inside just recipes. See top of file. + env: + UV_PYTHON: ${{ matrix.python-version }} + steps: - uses: actions/checkout@v6 From 780379352f5e8fb5ba1d7f89edd3fc95c125cebe Mon Sep 17 00:00:00 2001 From: phernandez Date: Tue, 1 Sep 2026 15:56:54 -0500 Subject: [PATCH 2/2] test(core): make the suite run on Python 3.12 and 3.13 Pinning UV_PYTHON made the 3.12 and 3.13 matrix rows run their declared interpreter for the first time, which surfaced seven pre-existing test bugs. Neither is a regression from that change; both are tests that only ever ran on 3.14. CPU-budget auto-tuning (6 tests, 3.12 only) `os.process_cpu_count` is new in 3.13. `_available_cpu_count` already handles its absence, but the tests did not: `monkeypatch.setattr` refuses to set an attribute that does not exist, so every test that pinned a CPU budget raised AttributeError on 3.12. Each of those tests pins `process_cpu_count` and `cpu_count` to the same value, so none of them cares which API reports the budget - they assert on the resolved number. Collapse both patches into a `pin_cpu_budget` fixture that passes `raising=False`, which keeps the preferred branch under test on 3.12 as well. monkeypatch deletes an attribute it created during teardown, so `os` is restored exactly as found on every interpreter (verified). That left the genuine 3.12 branch - `process_cpu_count` absent, budget from `os.cpu_count()` - covered by no test on any interpreter, since 3.13+ never takes it and 3.12 now forces the attribute to exist. Add two tests that delete the attribute to reproduce the 3.12 runtime anywhere: one asserting the fallback resolves the same knobs as the preferred API, one asserting an unreported budget leaves FastEmbed on its own defaults. `_available_cpu_count` and `_resolve_fastembed_runtime_knobs` are now fully covered on 3.12 and 3.14. Zero-argument super() under dataclass slots (1 test, 3.12 and 3.13) `dataclass(slots=True)` cannot add `__slots__` in place, so it rebuilds the class. Before CPython 3.14 (gh-90562) the rebuilt methods keep a `__class__` cell pointing at the discarded original, and zero-argument `super()` rejects `self` with "obj must be an instance or subtype of type". Name the class in the super() call so it is re-looked-up at call time and resolves to the rebuilt class on every supported interpreter. A repo-wide AST scan for slots dataclasses containing zero-argument super() found this as the only occurrence. Verified: tests/repository/test_openai_provider.py, tests/repository/test_fastembed_provider.py and tests/indexing/test_project_index_maintenance.py all pass (84 tests) on 3.12, 3.13 and 3.14. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez --- .../test_project_index_maintenance.py | 8 +- tests/repository/conftest.py | 30 +++++++ tests/repository/test_fastembed_provider.py | 11 ++- tests/repository/test_openai_provider.py | 80 +++++++++++++++---- 4 files changed, 105 insertions(+), 24 deletions(-) create mode 100644 tests/repository/conftest.py diff --git a/tests/indexing/test_project_index_maintenance.py b/tests/indexing/test_project_index_maintenance.py index 92a4b81a6..d625684a6 100644 --- a/tests/indexing/test_project_index_maintenance.py +++ b/tests/indexing/test_project_index_maintenance.py @@ -1312,7 +1312,13 @@ async def execute( ) -> FakeProjectIndexResult: if "UPDATE entity" in str(statement): raise RuntimeError("simulated intra-batch failure") - return await super().execute(statement, params) + # Constraint: `dataclass(slots=True)` cannot add __slots__ in place, so it + # rebuilds the class. Before CPython 3.14 (gh-90562) the rebuilt methods keep + # a `__class__` cell pointing at the discarded original, and zero-argument + # `super()` then rejects `self` with "obj must be an instance or subtype of + # type". Naming the class re-looks it up at call time, which resolves to the + # rebuilt class on every supported interpreter. + return await super(FailingUpdateProjectIndexSession, self).execute(statement, params) @pytest.mark.asyncio diff --git a/tests/repository/conftest.py b/tests/repository/conftest.py new file mode 100644 index 000000000..0ce7aa61f --- /dev/null +++ b/tests/repository/conftest.py @@ -0,0 +1,30 @@ +"""Shared fixtures for repository-layer tests.""" + +import os +from collections.abc import Callable + +import pytest + + +@pytest.fixture +def pin_cpu_budget(monkeypatch: pytest.MonkeyPatch) -> Callable[[int], None]: + """Return a helper that pins the CPU budget the embedding factory observes. + + ``_available_cpu_count`` prefers ``os.process_cpu_count`` and falls back to + ``os.cpu_count``. Both are pinned to the same value, so callers assert on the + resolved budget rather than on which API reported it. + + Constraint: ``os.process_cpu_count`` is new in Python 3.13 and + ``monkeypatch.setattr`` refuses to set an attribute that does not exist, so + 3.12 needs ``raising=False``. Creating it there is deliberate - it keeps the + preferred branch under test on every supported interpreter, and monkeypatch + deletes an attribute it created during teardown, so ``os`` is restored either + way. The genuine 3.12 fallback is covered separately by the tests that delete + ``os.process_cpu_count``. + """ + + def pin(cpu_count: int) -> None: + monkeypatch.setattr(os, "process_cpu_count", lambda: cpu_count, raising=False) + monkeypatch.setattr(os, "cpu_count", lambda: cpu_count) + + return pin diff --git a/tests/repository/test_fastembed_provider.py b/tests/repository/test_fastembed_provider.py index 703e32d5e..409ae21c9 100644 --- a/tests/repository/test_fastembed_provider.py +++ b/tests/repository/test_fastembed_provider.py @@ -598,7 +598,9 @@ async def test_fastembed_provider_fails_fast_without_cache_dir(monkeypatch): @pytest.mark.asyncio -async def test_factory_loads_native_model_once_across_repo_constructions(monkeypatch): +async def test_factory_loads_native_model_once_across_repo_constructions( + monkeypatch, pin_cpu_budget +): """The native ONNX model must load exactly once per process despite reuse (#872). Counting native model loads requires a stub TextEmbedding that increments a @@ -614,7 +616,6 @@ async def test_factory_loads_native_model_once_across_repo_constructions(monkeyp from typing import Any, cast from basic_memory.config import BasicMemoryConfig, DatabaseBackend, ProjectEntry - from basic_memory.repository import embedding_provider_factory as factory_module from basic_memory.repository.embedding_provider_factory import ( create_embedding_provider, reset_embedding_provider_cache, @@ -643,13 +644,11 @@ async def test_factory_loads_native_model_once_across_repo_constructions(monkeyp try: # First resolution under one CPU budget. - monkeypatch.setattr(factory_module.os, "process_cpu_count", lambda: 8) - monkeypatch.setattr(factory_module.os, "cpu_count", lambda: 8) + pin_cpu_budget(8) provider_first = create_embedding_provider(config) # CPU budget drifts (cgroup throttling) — used to force a second model load. - monkeypatch.setattr(factory_module.os, "process_cpu_count", lambda: 4) - monkeypatch.setattr(factory_module.os, "cpu_count", lambda: 4) + pin_cpu_budget(4) # Build several repositories the way per-request/per-sync code does. Each # one is injected with the cached provider rather than deriving its own. diff --git a/tests/repository/test_openai_provider.py b/tests/repository/test_openai_provider.py index cc3cf6888..9c6e5da2b 100644 --- a/tests/repository/test_openai_provider.py +++ b/tests/repository/test_openai_provider.py @@ -391,10 +391,11 @@ def test_openai_provider_reports_runtime_log_attrs(): } -def test_embedding_provider_factory_auto_tunes_fastembed_runtime_knobs_from_cpu_budget(monkeypatch): +def test_embedding_provider_factory_auto_tunes_fastembed_runtime_knobs_from_cpu_budget( + pin_cpu_budget, +): """Unset FastEmbed runtime knobs should resolve from available CPU budget.""" - monkeypatch.setattr(embedding_provider_factory_module.os, "process_cpu_count", lambda: 8) - monkeypatch.setattr(embedding_provider_factory_module.os, "cpu_count", lambda: 8) + pin_cpu_budget(8) config = BasicMemoryConfig( env="test", @@ -413,10 +414,9 @@ def test_embedding_provider_factory_auto_tunes_fastembed_runtime_knobs_from_cpu_ assert provider.parallel == 1 -def test_embedding_provider_factory_auto_tuning_caps_large_cpu_budgets(monkeypatch): +def test_embedding_provider_factory_auto_tuning_caps_large_cpu_budgets(pin_cpu_budget): """Large workers should still leave some headroom and stop at the thread cap.""" - monkeypatch.setattr(embedding_provider_factory_module.os, "process_cpu_count", lambda: 16) - monkeypatch.setattr(embedding_provider_factory_module.os, "cpu_count", lambda: 16) + pin_cpu_budget(16) config = BasicMemoryConfig( env="test", @@ -436,11 +436,10 @@ def test_embedding_provider_factory_auto_tuning_caps_large_cpu_budgets(monkeypat def test_embedding_provider_factory_auto_tuning_stays_conservative_on_small_cpu_budget( - monkeypatch, + pin_cpu_budget, ): """Small workers should not get an oversized FastEmbed runtime footprint.""" - monkeypatch.setattr(embedding_provider_factory_module.os, "process_cpu_count", lambda: 2) - monkeypatch.setattr(embedding_provider_factory_module.os, "cpu_count", lambda: 2) + pin_cpu_budget(2) config = BasicMemoryConfig( env="test", @@ -459,6 +458,56 @@ def test_embedding_provider_factory_auto_tuning_stays_conservative_on_small_cpu_ assert provider.parallel == 1 +def test_embedding_provider_factory_auto_tunes_without_process_cpu_count(monkeypatch): + """Auto-tuning must land on the same knobs when only os.cpu_count() exists. + + ``os.process_cpu_count`` is new in Python 3.13, so on 3.12 the factory always + takes the ``os.cpu_count()`` fallback. Deleting the attribute reproduces that + runtime on newer interpreters, where the branch is otherwise unreachable, and + asserts 3.12 resolves the identical 8-CPU budget as the preferred API does. + """ + monkeypatch.delattr(embedding_provider_factory_module.os, "process_cpu_count", raising=False) + monkeypatch.setattr(embedding_provider_factory_module.os, "cpu_count", lambda: 8) + + config = BasicMemoryConfig( + env="test", + projects={"test-project": "/tmp/basic-memory-test"}, + default_project="test-project", + semantic_search_enabled=True, + semantic_embedding_provider="fastembed", + semantic_embedding_threads=None, + semantic_embedding_parallel=None, + ) + + provider = create_embedding_provider(config) + + assert isinstance(provider, FastEmbedEmbeddingProvider) + assert provider.threads == 6 + assert provider.parallel == 1 + + +def test_embedding_provider_factory_leaves_knobs_unset_when_cpu_budget_is_unknown(monkeypatch): + """An unreported CPU budget must leave FastEmbed on its own runtime defaults.""" + monkeypatch.delattr(embedding_provider_factory_module.os, "process_cpu_count", raising=False) + monkeypatch.setattr(embedding_provider_factory_module.os, "cpu_count", lambda: None) + + config = BasicMemoryConfig( + env="test", + projects={"test-project": "/tmp/basic-memory-test"}, + default_project="test-project", + semantic_search_enabled=True, + semantic_embedding_provider="fastembed", + semantic_embedding_threads=None, + semantic_embedding_parallel=None, + ) + + provider = create_embedding_provider(config) + + assert isinstance(provider, FastEmbedEmbeddingProvider) + assert provider.threads is None + assert provider.parallel is None + + def test_embedding_provider_factory_reuses_provider_for_same_cache_key(): """Factory should reuse the same provider instance for identical config values.""" config_a = BasicMemoryConfig( @@ -484,10 +533,9 @@ def test_embedding_provider_factory_reuses_provider_for_same_cache_key(): assert provider_a is provider_b -def test_embedding_provider_factory_reuses_auto_tuned_provider_for_same_cpu_budget(monkeypatch): +def test_embedding_provider_factory_reuses_auto_tuned_provider_for_same_cpu_budget(pin_cpu_budget): """Auto-tuned FastEmbed providers should still reuse the process cache.""" - monkeypatch.setattr(embedding_provider_factory_module.os, "process_cpu_count", lambda: 8) - monkeypatch.setattr(embedding_provider_factory_module.os, "cpu_count", lambda: 8) + pin_cpu_budget(8) config_a = BasicMemoryConfig( env="test", @@ -685,7 +733,7 @@ def test_embedding_provider_factory_reuses_provider_when_only_thread_knobs_diffe assert provider_a is provider_b -def test_embedding_provider_factory_reuses_provider_when_cpu_budget_drifts(monkeypatch): +def test_embedding_provider_factory_reuses_provider_when_cpu_budget_drifts(pin_cpu_budget): """A drifting CPU budget between calls must not reload the model (#872). Simulates a container/cgroup where the auto-tuned thread count changes between @@ -702,13 +750,11 @@ def test_embedding_provider_factory_reuses_provider_when_cpu_budget_drifts(monke semantic_embedding_parallel=None, ) - monkeypatch.setattr(embedding_provider_factory_module.os, "process_cpu_count", lambda: 8) - monkeypatch.setattr(embedding_provider_factory_module.os, "cpu_count", lambda: 8) + pin_cpu_budget(8) provider_first = create_embedding_provider(config) # CPU budget shrinks (e.g. cgroup throttling) → auto-tuned thread count changes. - monkeypatch.setattr(embedding_provider_factory_module.os, "process_cpu_count", lambda: 4) - monkeypatch.setattr(embedding_provider_factory_module.os, "cpu_count", lambda: 4) + pin_cpu_budget(4) provider_second = create_embedding_provider(config) assert provider_first is provider_second