chore: Make semantic deps default, auto-backfill embeddings, and default search to semantic

[phernandez]

Summary

• move semantic dependencies (fastembed, sqlite-vec, openai) into the default package install
• update install flows (justfile, CI workflow) and semantic docs to match default dependency inclusion
• add Alembic trigger revision and migration hook that performs a one-time embedding backfill (equivalent to bm reindex --embeddings behavior) when the new revision is first applied
• default semantic_search_enabled based on semantic dependency availability (fastembed + sqlite_vec)
• make search_notes choose default mode dynamically (hybrid when semantic is enabled, otherwise text/FTS), while preserving explicit search_type="text" as FTS
• align CLI/ChatGPT/UI search wrappers with dynamic default behavior
• update tests and user-facing messages for the new semantic dependency model

Validation

• uv run ruff check src/basic_memory/db.py src/basic_memory/alembic/versions/i2c3d4e5f6g7_auto_backfill_semantic_embeddings.py tests/services/test_initialization.py
• uv run pytest tests/services/test_initialization.py -q
• uv run pytest tests/services/test_initialization.py tests/repository/test_fastembed_provider.py tests/repository/test_openai_provider.py tests/mcp/test_tool_search.py -q
• uv run ruff check src/basic_memory/config.py src/basic_memory/mcp/tools/search.py src/basic_memory/mcp/tools/chatgpt_tools.py src/basic_memory/mcp/tools/ui_sdk.py src/basic_memory/cli/commands/tool.py tests/test_config.py tests/mcp/test_tool_search.py tests/mcp/tools/test_chatgpt_tools.py
• uv run pyright src/basic_memory/config.py src/basic_memory/mcp/tools/search.py src/basic_memory/mcp/tools/chatgpt_tools.py src/basic_memory/mcp/tools/ui_sdk.py src/basic_memory/cli/commands/tool.py tests/mcp/test_tool_search.py tests/mcp/tools/test_chatgpt_tools.py
• uv run pytest tests/test_config.py tests/mcp/test_tool_search.py tests/mcp/tools/test_chatgpt_tools.py -q

[github-actions]

Claude finished @phernandez's task in 3m 35s —— View job

---

PR Review: Make semantic deps default, auto-backfill embeddings

Tasks:

• Gather context and understand the PR
• Review changed files
• Check code quality and standards
• Check testing and documentation
• Check architecture patterns
• Check security and performance
• Post final review

---

Overall this is a well-structured PR. The auto-backfill approach is clever and the literate comments in db.py follow CLAUDE.md guidelines nicely. A few issues need attention before merge.

Issues Found

High: Resource leak in run_migrations — engine never disposed

src/basic_memory/db.py:519-520

if _session_maker is None:
    _, session_maker = _create_engine_and_session(app_config.database_path, database_type)

The engine returned by _create_engine_and_session is discarded with _. Unlike the pre-upgrade path (lines 485-493) which correctly wraps in try/finally and calls temp_engine.dispose(), this post-upgrade path leaks the engine. This runs on every fresh startup (before the module-level engine is set). Fix this →

Medium: 3 Pyright errors in test file

Per the PR validation output, tests/mcp/test_tool_search.py has unresolved type errors:

• Line 618: Literal['entity'] not assignable to SearchItemType — Pyright treats SearchItemType.ENTITY as Literal['entity'] in strict mode. Use type=cast(SearchItemType, "entity") or restructure the SearchResult call to use the enum directly.
• Lines 820, 882: None not assignable to StubConfig — The StubContainer dataclass declares config: StubConfig | None = None but pyright complains about the assignment. Replace with config: Optional[StubConfig] = field(default=None) or restructure to avoid the dataclass pattern entirely.

Fix this →

Medium: Non-idiomatic Alembic revision ID

src/basic_memory/alembic/versions/i2c3d4e5f6g7_auto_backfill_semantic_embeddings.py

The revision ID i2c3d4e5f6g7 and its down_revision h1b2c3d4e5f6 both appear to be manually crafted sequential IDs rather than Alembic-generated hex strings. Confirm that h1b2c3d4e5f6 actually exists as the head revision, otherwise the migration chain will break on upgrade. Run alembic heads to verify.

Additionally, the hardcoded constant SEMANTIC_EMBEDDING_BACKFILL_REVISION = "i2c3d4e5f6g7" in db.py:47 tightly couples the runtime logic to this specific migration revision. If the revision ID ever changes (e.g., if the migration is regenerated), the backfill trigger silently breaks. A comment explaining this coupling risk would be valuable.

Medium: Sequential entity backfill — no progress feedback

src/basic_memory/db.py:131-132

for entity_id in entity_ids:
    await search_repository.sync_entity_vectors(entity_id)

For users upgrading with large knowledge bases (thousands of entities), this runs entity-by-entity with no progress indication visible to the user. The server will appear to hang during startup. Consider:

1. Logging progress every N entities (e.g., every 100)
2. Or documenting in the upgrade notes that startup may take longer on first upgrade

Low: openai as a core dependency

pyproject.toml:49

openai>=1.100.2 is now a mandatory install-time dependency, even for users who will never use OpenAI embeddings (i.e., everyone using the default FastEmbed provider). The OpenAI package adds significant transitive dependencies. Consider whether this should remain optional (pip install basic-memory[openai]) or whether the tradeoff of defaulting is deliberate.

Low: Stale docs in Quick Start

docs/semantic-search.md:76-80

The Quick Start section still lists step 3 as "Build vector embeddings for your existing content: bm reindex --embeddings". Since auto-backfill now handles this on upgrade, step 3 is no longer required for upgrading users. The reindex note at line 204 mentions it, but the Quick Start section (the most visible part) is misleading for existing users upgrading.

Low: run_migrations fully marked pragma: no cover despite new testable logic

src/basic_memory/db.py:472

The entire run_migrations function is excluded from coverage. While the tests in test_initialization.py test the component functions directly via monkeypatching, the integration of those components inside run_migrations is completely untested. The # pragma: no cover here seems overly broad given this function now contains meaningful orchestration logic.

---

Checklist Results

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md — literate comments, section headers, decision-point comments all present
• Python 3.12+ type annotations and async patterns
• SQLAlchemy 2.0 best practices
• FastAPI and Typer conventions followed
• 100-character line length maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests for new functions/methods — test_initialization.py adds comprehensive tests
• Integration tests for new MCP tools
• [~] Test coverage for edge cases — sequential backfill and run_migrations itself not covered
• [~] 100% test coverage — run_migrations is entirely pragma: no cover
• Documentation updated — semantic-search.md updated
• Minor stale content in Quick Start section (see above)

Basic Memory Architecture

• MCP tools follow atomic, composable design
• Database changes include Alembic migrations
• Preserves local-first architecture principles
• [~] Migration approach is non-standard — using Alembic migration as a "trigger" rather than for schema change. This works but is unconventional and should be documented
• Markdown file handling preserves integrity
• AI-human collaboration patterns followed

Security & Performance

• No hardcoded secrets or credentials
• Input validation for MCP tools
• Proper error handling and logging
• [~] Performance: sequential per-entity backfill with no progress feedback (see above)
• No sensitive data in logs or commits

Compatibility

• File path comparisons appear Windows compatible
• No emojis in console/log output (the ✓ in docs is display-only markdown, not log output)

---

Summary

The core implementation is solid and the approach to auto-backfilling on upgrade is well-considered. The main items to address before merge:

1. Fix the engine resource leak at db.py:519-520 (critical path on every startup)
2. Fix the 3 pyright type errors in test_tool_search.py
3. Verify the Alembic revision chain is intact (check h1b2c3d4e5f6 exists)
4. Consider adding progress logging in the sequential entity backfill loop

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23a35dc374

ℹ️ 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".

[chatgpt-codex-connector]

Prevent migration backfill from aborting on one bad entity

The new migration-time backfill calls sync_entity_vectors in a tight loop without per-entity error handling, so any single embedding failure (e.g., one malformed row, provider/auth issue, or transient embedding error) now raises out of run_migrations and can block process startup right after upgrade. This is a regression in robustness compared to the existing SearchService.reindex_vectors() path, which catches per-entity failures and continues; the migration hook should avoid turning one bad entity into a full migration failure.

Useful? React with 👍 / 👎.