Skip to content

fix(core): load sqlite-vec for embedding-status query - #901

Merged
phernandez merged 3 commits into
mainfrom
fix/sqlite-vec-status
Jun 7, 2026
Merged

fix(core): load sqlite-vec for embedding-status query#901
phernandez merged 3 commits into
mainfrom
fix/sqlite-vec-status

Conversation

@phernandez

Copy link
Copy Markdown
Member

Summary

After a successful bm reindex --embeddings, bm project info still reported "sqlite-vec is unavailable", showed "Indexed 0/N" and "Chunks 0", and recommended an unnecessary reindex (#658). Vector search itself worked fine — only the status report was wrong.

Root cause: ProjectService.get_embedding_status() runs the vec0 count queries (the search_vector_chunks JOIN search_vector_embeddings) via ProjectRepository.execute_query, which opens a bare pooled session that never loads the sqlite-vec extension. SQLite then raises no such module: vec0, and the except SAOperationalError block mapped that to vector_tables_exist=False + reindex_recommended=True — the false "unavailable". This is SQLite-only; the Postgres branch is unaffected.

What changed

  • repository/project_repository.py: added scalar_vec_query(), which opens a scoped session, loads sqlite-vec on it via the existing _load_sqlite_vec_on_session loader (the same one used by project delete), then runs a scalar COUNT query. Returns None only when sqlite-vec genuinely cannot be loaded on this Python build (e.g. python.org macOS without enable_load_extension).
  • services/project_service.py: the SQLite embeddings/orphan JOIN counts now go through scalar_vec_query (consolidated in a small _vec_scalar closure). When it returns None the code raises the canonical no such module: vec0 so the existing except block still emits the true "sqlite-vec unavailable" message — now only for the genuinely-missing-dependency case, not the normal path. The Postgres branch is unchanged (still uses execute_query).
  • tests/services/test_project_service_embedding_status.py: updated the "unavailable" unit test to simulate the failure via scalar_vec_query returning None (the new signal), since the JOIN no longer flows through execute_query.

Testing

uv run ruff check src/basic_memory/services/project_service.py src/basic_memory/repository/project_repository.py tests/services/test_project_service_embedding_status.py test-int/semantic/test_embedding_status_vec0.py
# All checks passed!

uv run ty check src tests test-int
# All checks passed!

uv run pytest tests/services/test_project_service_embedding_status.py test-int/semantic/test_embedding_status_vec0.py
# 9 passed

uv run pytest tests/services/ tests/repository/test_project_repository.py test-int/semantic/test_embedding_status_vec0.py
# 354 passed, 3 skipped

New integration test test-int/semantic/test_embedding_status_vec0.py builds a real vec0 virtual table, writes a real embedding into it via the search repository, disposes the connection pool (so the vec-loaded connection is evicted), then calls get_embedding_status through a fresh ProjectRepository that never loaded the extension — the exact #658 condition. It asserts vector_tables_exist=True, reindex_recommended=False, and correct Indexed/Chunks/Embeddings counts. Verified the test fails on the pre-fix code (reports vector_tables_exist=False + "sqlite-vec is unavailable") and passes with the fix.

Risk / validation

  • SQLite-only behavior change; the Postgres path is byte-for-byte unchanged (still routes through execute_query).
  • The "genuinely unavailable" degradation is preserved: when sqlite-vec can't load at all, scalar_vec_query returns None and the code re-raises no such module: vec0 into the same except block, so the existing graceful "install/update basic-memory" message still appears.
  • One extra short-lived session per status query that loads the extension; status is an on-demand project info call, not a hot path.

Closes #658

🤖 Generated with Claude Code

After a successful 'bm reindex --embeddings', 'bm project info' still reported "sqlite-vec is unavailable", "Indexed 0/N", and "Chunks 0" and recommended an unnecessary reindex. get_embedding_status() ran the vec0 JOIN count queries on a bare pooled ProjectRepository session that never loaded the sqlite-vec extension, so SQLite raised "no such module: vec0", which the except block mis-reported as unavailable.

Add ProjectRepository.scalar_vec_query, which loads sqlite-vec on the session it opens (reusing the project-delete loader) before running a scalar vec0 count, and route the SQLite embeddings/orphan JOIN queries through it. The except block now only handles the genuinely-missing-dependency case (e.g. python.org builds without enable_load_extension). Adds a real-vec0 integration regression test and updates the unit test to simulate sqlite-vec failing to load via scalar_vec_query returning None.

Closes #658

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66ca306bb3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

project_id=project_id,
app_config=app_config,
)
await search_repo._ensure_vector_tables()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip real vec0 test when extension loading is unavailable

On SQLite environments where the Python build exposes sqlite_vec but cannot enable/load SQLite extensions, this new regression test fails at setup before it reaches the status fallback path (I reproduced this with uv run pytest test-int/semantic/test_embedding_status_vec0.py -q, which raises AttributeError: 'sqlite3.Connection' object has no attribute 'enable_load_extension'). Since the codebase explicitly supports degrading in that scenario, the test should probe the capability and skip when real vec0 tables cannot be created rather than making the suite fail on those interpreters.

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @phernandez's task in 2m 11s —— View job


PR Review: fix(core): load sqlite-vec for embedding-status query

Tasks:

  • Gather context and understand the PR
  • Read changed files in detail
  • Verify fix correctness and architecture fit
  • Check test coverage and quality
  • Evaluate Codex P2 suggestion
  • Run team checklist
  • Post final review

Summary

The root cause diagnosis and fix are correct. ProjectRepository.execute_query uses a bare pooled session that never loads the sqlite-vec extension; the new scalar_vec_query opens a fresh scoped session, loads the extension via the existing _load_sqlite_vec_on_session helper, then runs the COUNT. The Postgres path is unchanged. The integration test faithfully reproduces the #658 failure condition by disposing the connection pool after setup, forcing get_embedding_status onto a brand-new connection — exactly the real-world state after bm reindex --embeddings.


Issues

P2 — Integration test will fail on Python builds without enable_load_extension (confirms the Codex finding)

test_embedding_status_vec0.py:74 calls search_repo._ensure_vector_tables(), which internally calls _load_sqlite_vec_on_session. When the Python build exposes sqlite3 but lacks enable_load_extension (python.org macOS, some Windows builds), that loader returns False, and the subsequent _write_embeddings call will fail because the vec0 table was never created. The test should probe capability first and skip when the Python build cannot load extensions.

A minimal guard to add after the Postgres skip at line 57:

# Probe: skip on Python builds without SQLite extension loading support
import sqlite3 as _sqlite3
_probe = _sqlite3.connect(":memory:")
if not hasattr(_probe, "enable_load_extension"):
    _probe.close()
    pytest.skip("Python build does not support SQLite extension loading — cannot create real vec0 tables")
_probe.close()

Fix this →


Minor — Missing type annotation on scalar_vec_query.query parameter

project_repository.py:261 — CLAUDE.md requires full type annotations on Python 3.12+. The query parameter is untyped; the rest of the codebase passes text(...) objects, so sqlalchemy.sql.elements.ClauseElement (or the more precise TextClause) fits:

from sqlalchemy.sql.elements import ClauseElement

async def scalar_vec_query(self, query: ClauseElement, params: Optional[dict] = None) -> Optional[int]:

Fix this →


What's working well

  • Fix is minimal and targeted — only the two queries that actually read search_vector_embeddings are rerouted through scalar_vec_query; all other execute_query usage is untouched.
  • Correct reuse of _load_sqlite_vec_on_session — the same loader used for project delete, avoiding drift between two vec-loading paths.
  • Degradation path preserved — when scalar_vec_query returns None (genuine missing dependency), the code re-raises the canonical SAOperationalError("no such module: vec0") so the existing except block still emits the helpful "install/update basic-memory" message.
  • Postgres unchanged_vec_scalar correctly routes Postgres through execute_query and SQLite through scalar_vec_query. The guard if is_postgres or "no such module: vec0" not in str(exc).lower(): raise still re-raises non-vec0 errors.
  • Integration test design — disposing the engine pool after setup to evict the vec-loaded connection is the right way to reproduce the real process-restart condition from [BUG] project info reports "sqlite-vec is unavailable" after successful reindex #658.
  • Literate comments — decision-point comments (Trigger / Why / Outcome) on the new scalar_vec_query method and _vec_scalar closure follow project conventions.

Team Checklist

Code Quality & Standards

  • Follows Basic Memory's coding conventions in CLAUDE.md — literate comments, section headers, fail-fast
  • [~] Python 3.12+ type annotations — query parameter in scalar_vec_query missing annotation
  • SQLAlchemy 2.0 best practices — scoped_session, async, correct use of execute/scalar
  • 100-character line length limit maintained
  • No commented-out code blocks

Testing & Documentation

  • Unit tests for new functions/methods — scalar_vec_query covered via service tests; "unavailable" path updated correctly
  • Integration tests — new test_embedding_status_vec0.py covers the exact [BUG] project info reports "sqlite-vec is unavailable" after successful reindex #658 regression path
  • [~] Test coverage for edge cases — integration test is missing the skip guard for builds without extension loading (P2 above)
  • No doc/CLAUDE.md changes required

Basic Memory Architecture

  • No schema changes, no Alembic migration needed
  • Preserves local-first architecture
  • Postgres path byte-for-byte unchanged

Security & Performance

  • No secrets or credentials
  • Error handling and logging correct — degradation preserved for genuine missing-dependency case
  • Performance acceptable — scalar_vec_query adds one short-lived session; project info is an on-demand call, not a hot path

Compatibility

  • No emoji or unicode characters in log output

Verdict: The fix is correct and the approach is solid. The P2 test skip guard is the only change I'd request before merge; the type annotation is a minor cleanup. Once the skip guard is added, the suite will be reliable across all Python builds.

Windows SQLite Integration (job 79984353954) and both review bots flagged
test-int/semantic/test_embedding_status_vec0.py: it builds a REAL vec0 table
during setup, which fails on Python builds lacking enable_load_extension
(python.org macOS / some Windows interpreters) with AttributeError. The
codebase already degrades gracefully there, so probe the capability and
pytest.skip when extension loading is unavailable — an environment-capability
skip, not silencing a bug.

Also addresses claude-review: add the missing Executable type annotation on
ProjectRepository.scalar_vec_query's 'query' parameter (matches the type the
base Repository.execute_query already uses); ty check passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @phernandez's task in 1m 21s —— View job


PR Review: fix(core): load sqlite-vec for embedding-status query

Tasks:

  • Gather context and understand the PR
  • Read changed files to verify P2 and annotation fixes
  • Run team checklist
  • Post final review

Follow-up Review (commit 0bdf1e1)

Both issues raised in the previous review have been addressed:

P2 resolved — skip guard added (test-int/semantic/test_embedding_status_vec0.py:65-72)

The capability probe is correct:

_probe = sqlite3.connect(":memory:")
if not hasattr(_probe, "enable_load_extension"):
    _probe.close()
    pytest.skip(
        "Python build does not support SQLite extension loading — "
        "cannot create real vec0 tables"
    )
_probe.close()

It's placed after the Postgres skip, has a proper literate comment explaining Trigger/Why/Outcome, and uses the right attribute check. The suite will now skip cleanly on python.org macOS and Windows builds that lack enable_load_extension instead of failing with AttributeError.

Minor annotation resolved (project_repository.py:261-262)

Executable is now imported from sqlalchemy (line 8) and the signature reads:

async def scalar_vec_query(self, query: Executable, params: Optional[dict] = None) -> Optional[int]:

This satisfies the CLAUDE.md requirement for full Python 3.12+ type annotations.


Team Checklist

Code Quality & Standards

  • Follows Basic Memory's coding conventions — literate Trigger/Why/Outcome comments throughout
  • Python 3.12+ type annotations — Executable annotation on scalar_vec_query; all other new code fully annotated
  • SQLAlchemy 2.0 best practices — scoped_session, async, correct execute/scalar usage
  • 100-character line length maintained
  • No commented-out code blocks

Testing & Documentation

  • Unit tests — scalar_vec_query covered via test_embedding_status_handles_sqlite_vec_unavailable (mocks scalar_vec_query returning None)
  • Integration test — test_embedding_status_vec0.py faithfully reproduces the exact [BUG] project info reports "sqlite-vec is unavailable" after successful reindex #658 failure condition (dispose pool → fresh connection → no extension loaded)
  • Test coverage for edge cases — capability skip guard added; "genuinely unavailable" degradation path covered by unit test
  • 100% coverage maintained — no new untested paths
  • No doc/CLAUDE.md changes needed

Basic Memory Architecture

  • No schema changes, no Alembic migration needed
  • Preserves local-first architecture
  • Postgres path byte-for-byte unchanged — _vec_scalar routes Postgres through execute_query, SQLite through scalar_vec_query

Security & Performance

  • No secrets or credentials
  • Error handling correct — genuine missing-dependency degradation preserved via None → re-raise path
  • Performance acceptable — one short-lived session per project info call (on-demand, not hot path)

Compatibility

  • No emoji or unicode in log output
  • No file path comparison issues

Verdict

All review items resolved. The fix is correct and minimal, both issues from the previous review have been addressed, and the integration test is now robust across Python build variants. Ready to merge.

…ftest

test-int/semantic/conftest.py spins up a pgvector testcontainer for its benchmark fixtures, so the new vec0 regression test triggered Docker (ryuk) during setup and ERRORed on the Windows SQLite-only CI job (no Docker). Move it to test-int/ so it uses the root SQLite engine_factory (no Docker); the enable_load_extension skip-probe still covers python.org/Windows builds without extension loading.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @phernandez's task in 1m 40s —— View job


PR Review: fix(core): load sqlite-vec for embedding-status query

Tasks:

  • Gather context and understand the PR
  • Read changed files in detail
  • Verify latest commit changes (test file relocation)
  • Run team checklist
  • Post final review

Follow-up Review (commit a5286b7)

This commit is a pure file move — zero content changes, 0 insertions / 0 deletions:

test-int/{semantic => }/test_embedding_status_vec0.py

Why the move is correct:

test-int/semantic/conftest.py imports testcontainers.postgres and spins up a pgvector Docker container for its benchmark fixtures. pytest collects conftest.py files for every test in that directory, so placing the vec0 regression test there caused Docker/Ryuk to be invoked even though the test itself is pure SQLite — ERRORing on the Windows SQLite-only CI job where Docker is unavailable.

Moving the file to test-int/ gives it the root conftest.py fixtures (engine_factory, test_project, config_manager) which use a plain SQLite filesystem engine — no containers, no Docker. I confirmed both the root conftest exports all three fixtures (lines 256, 319, 389 of test-int/conftest.py) and the semantic conftest has no relevant fixtures the test needed.

The enable_load_extension skip probe added in 0bdf1e1 still covers Python builds without extension loading support (e.g., python.org macOS / Windows interpreters), so the CI matrix is safe across all build variants.


What's working well across all three commits

  • Root cause fix is minimal and targeted — only the two queries that read search_vector_embeddings are rerouted through scalar_vec_query; all other execute_query usage is untouched
  • Correct reuse of _load_sqlite_vec_on_session — same loader as project delete, no drift between vec-loading paths
  • Executable type annotation on scalar_vec_query.query parameter satisfies the CLAUDE.md full-annotation requirement
  • Degradation path preservedscalar_vec_query returning None re-raises SAOperationalError("no such module: vec0") into the existing except block
  • Postgres path unchanged_vec_scalar routes Postgres through execute_query, SQLite through scalar_vec_query
  • Integration test design — dispose pool after setup forces get_embedding_status onto a brand-new connection with no extension preloaded, exactly reproducing the [BUG] project info reports "sqlite-vec is unavailable" after successful reindex #658 condition
  • CI fix is clean — moving the test rather than adding Docker skips is the right call

Team Checklist

Code Quality & Standards

  • Follows Basic Memory's coding conventions — literate Trigger/Why/Outcome comments throughout
  • Python 3.12+ type annotations — Executable annotation on scalar_vec_query; all new code fully annotated
  • SQLAlchemy 2.0 best practices — scoped_session, async, correct execute/scalar usage
  • 100-character line length maintained
  • No commented-out code blocks

Testing & Documentation

  • Unit tests — scalar_vec_query covered via test_embedding_status_handles_sqlite_vec_unavailable (mocks returning None)
  • Integration test — test_embedding_status_vec0.py faithfully reproduces the exact [BUG] project info reports "sqlite-vec is unavailable" after successful reindex #658 regression condition (dispose pool → fresh connection → no extension loaded)
  • Test coverage for edge cases — capability skip guard added; "genuinely unavailable" degradation path covered by unit test
  • 100% coverage maintained — no new untested paths
  • No doc/CLAUDE.md changes needed

Basic Memory Architecture

  • No schema changes, no Alembic migration needed
  • Preserves local-first architecture
  • Postgres path byte-for-byte unchanged

Security & Performance

  • No secrets or credentials
  • Error handling correct — genuine missing-dependency degradation preserved
  • Performance acceptable — one short-lived session per project info call (on-demand, not hot path)

Compatibility

  • No emoji or unicode in log output
  • Test relocated out of Docker-dependent conftest — Windows CI no longer errors

Verdict: All three commits are correct. The file relocation in a5286b7 is exactly the right fix for the Windows CI error — no content changes, just moving the test to the conftest that matches its runtime requirements. Ready to merge.

@phernandez
phernandez merged commit 271c883 into main Jun 7, 2026
40 of 41 checks passed
@phernandez
phernandez deleted the fix/sqlite-vec-status branch June 7, 2026 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] project info reports "sqlite-vec is unavailable" after successful reindex

1 participant