Repository navigation
feat(core): remove semantic_search_enabled; semantic search is always on - #1696
Conversation
Semantic search already defaulted on whenever fastembed and sqlite-vec were importable, and both are core dependencies, so the flag only mattered for installs that turned it off by hand. Every check of it is gone: repository construction, the search reader factory, embedding scheduling on writes and index-file, MCP startup, bm reindex, the search and grep tool defaults, the watcher, readiness, and embedding status. Six knowledge routes lose the app_config dependency they only had for the flag, and EmbeddingStatus loses its always-true field. The #711 runtime fallback stays: when sqlite-vec cannot load, that repository drops to keyword-only search, and the error now points at the startup log instead of a config flag. Old configs and BASIC_MEMORY_SEMANTIC_SEARCH_ENABLED still load because the config model ignores unknown keys. Tests used the flag to skip the ONNX model. They now use a deterministic test embedder (tests/fake_embeddings.py) that keeps the real FastEmbed provider class and identity and swaps only its embed methods; tests marked semantic or real_embedder keep the real model. Tests that only covered flag-off behavior are removed; the rest set up real vector storage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…ses it test-int runs on its own in CI, where the tests package is not importable, so 'from tests.fake_embeddings' failed while loading its conftest. pytest's pythonpath already puts tests/ on the import path; import the module by that name and teach ty the same path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7506dce642
ℹ️ 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".
| await driver_connection.enable_load_extension(True) | ||
| await driver_connection.load_extension(sqlite_vec.loadable_path()) |
There was a problem hiding this comment.
On Python builds without sqlite3.Connection.enable_load_extension—the exact macOS fallback environment this change preserves—this helper raises AttributeError before the status assertions run. Running the focused suite on such a build produced five failures in test_project_service_embedding_status.py; probe the capability and skip or use a non-vec test setup rather than unconditionally loading the extension.
Useful? React with 👍 / 👎.
| # Semantic search is always on, so indexing embeds with the real model. | ||
| # The pristine HOME would otherwise put the model cache inside the temp | ||
| # profile and download it again for every test; share the real one. | ||
| FASTEMBED_CACHE_PATH=SHARED_FASTEMBED_CACHE, |
There was a problem hiding this comment.
When these subprocess tests run, SHARED_FASTEMBED_CACHE resolves from the developer or CI account's real profile before HOME is replaced, so the supposedly pristine test can download into—and FastEmbed's recovery path can delete corrupt subtrees from—the user's actual ~/.basic-memory/fastembed_cache. Use a session-scoped temporary cache shared by these tests instead, preserving download reuse without mutating state outside the isolated test directory.
AGENTS.md reference: AGENTS.md:L300-L303
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 400265b: the subprocess tests now share a module-scoped tmp_path_factory cache (one model download per module) and never touch the real profile's fastembed_cache.
…ess builds - The subprocess CLI tests share a module-scoped temporary FastEmbed cache instead of the runner's real ~/.basic-memory/fastembed_cache, so they cannot download into or prune a developer's actual profile. - The embedding-status helper loads sqlite-vec only when the driver can load extensions; on builds that cannot, search never created the vec0 table. Addresses Codex review on #1696. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 400265bb7f
ℹ️ 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".
| if app_config is None or not app_config.semantic_search_enabled: | ||
| return None | ||
| """Return the semantic vector sync backend for local indexing.""" | ||
| return dependencies.search_service |
There was a problem hiding this comment.
On SQLite runtimes where sqlite-vec cannot load, startup deliberately falls back to keyword-only operation, but this factory creates a fresh repository and now always supplies its search service as the vector-sync backend. run_project_index_coordinator subsequently awaits vector synchronization, whose _ensure_vector_tables() raises SemanticDependenciesMissingError, causing bm project add and bm project index to fail after the full-text rows were written instead of completing in keyword-only mode. Only provide the vector sync backend when the runtime capability probe succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e8f56b7. SearchService.sync_entity_vectors[_batch] now return the entities as skipped when the repository runs keyword-only (#711), so project indexing, the watcher and write follow-ups finish without vectors. The same commit makes readiness owe no embeddings on such hosts (otherwise bm status --wait ran to its timeout). Test: test_vector_sync_skips_entities_when_the_vector_runtime_is_unavailable (fails without the fix).
| return config.default_search_type | ||
|
|
||
| return "hybrid" if config.semantic_search_enabled else "text" | ||
| return "hybrid" |
There was a problem hiding this comment.
When SQLite extension loading is unsupported, initialization logs that search is falling back to keywords, but omitted search_type values are now always converted to hybrid. Per-request repositories are rebuilt with semantics enabled and their vector-table probe raises, so ordinary search_notes("...") calls return a semantic-unavailable error rather than keyword results; users must know to explicitly request search_type="text". The default mode needs to reflect the effective runtime capability, not merely the always-on configuration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e8f56b7: the default search type is hybrid only when semantic_runtime_available() (a cached sqlite-vec load probe, always true on Postgres); otherwise text. Test: test_default_search_type_is_text_when_the_vector_runtime_cannot_load.
…s always on With the flag gone, a host that cannot load sqlite-vec (#711) had no way to opt out, and three paths still assumed vectors: - project indexing and write follow-ups awaited vector sync, which raised once the repository had fallen back, failing `bm project add`/`index` after the full-text rows were written; - readiness counted every note as owed embeddings, parking the project in PENDING and running `bm status --wait` to its timeout; - `search_notes` defaulted omitted search types to hybrid, turning every plain search into a semantic-unavailable error. One cached probe, `semantic_runtime_available` (sqlite-vec loadable, or Postgres), now answers "can this host run vector search". Vector sync skips and reports entities as skipped when the repository runs keyword-only, readiness owes no embeddings there, and the search tool defaults to text there. Addresses Codex review on #1696. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
Semantic search is always on, so app and MCP tests now reach pgvector (write_note's similar-notes probe, vector sync). Production creates that storage at database initialization; the per-test schema reset drops it, so the Postgres engine fixture now recreates it for the configured provider's dimensions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75d5bbbc3f
ℹ️ 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".
| # A host that cannot load sqlite-vec runs keyword-only (#711); defaulting to | ||
| # hybrid there would turn every plain search into a semantic-unavailable error. | ||
| return "hybrid" if semantic_runtime_available(config) else "text" |
There was a problem hiding this comment.
When the MCP process runs on an extension-less SQLite host but the selected project is routed to Cloud (or through a custom factory), get_project_client() sends the request to the remote Postgres service while this check still probes the local SQLite runtime and returns text. Ordinary searches against a fully semantic-capable cloud project therefore silently lose hybrid retrieval; determine availability from the selected route, or default cloud/factory routes to hybrid.
AGENTS.md reference: AGENTS.md:L344-L350
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8d5c934: the default is hybrid when the local runtime can run vector search or when every selected project routes to Cloud (get_project_mode == CLOUD; Cloud runs Postgres). Test: test_default_search_type_is_hybrid_for_cloud_projects_on_a_keyword_only_host.
| @dataclass | ||
| class StubConfig: | ||
| semantic_search_enabled: bool = True | ||
| default_search_type: str | None = None | ||
| database_backend: DatabaseBackend = DatabaseBackend.SQLITE |
There was a problem hiding this comment.
On a supported Python build where sqlite3.Connection.enable_load_extension is absent, this SQLite stub invokes the real capability probe, so the test receives FTS instead of its asserted hybrid mode. Fresh evidence after the earlier extension-less helper fix: on Python 3.14.4, the focused command containing this test and test_an_unembedded_markdown_note_is_pending_embedding_work fails both cases, and the broader changed suites produce the same host-dependent failures; patch the probe to return true in tests that model an available semantic runtime while retaining the explicit fallback test.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8d5c934: the shared autouse fixture (tests/fake_embeddings.py) now also patches semantic_runtime.sqlite_vector_runtime_available to True, so tests that model an available runtime are host-independent. The explicit fallback tests still inject False at the call sites.
…y hosts - search_notes now picks its default from where the search runs: hybrid when the local runtime can run vector search or every selected project routes to Cloud (Postgres always can), text otherwise. Probing only the local SQLite runtime downgraded cloud projects on hosts without sqlite-vec. - The shared test fixture now models a vector-capable host (the sqlite-vec probe answers yes), so ordinary tests no longer depend on whether this Python can load extensions. Fallback tests still inject the opposite. - test-int's Postgres fixture recreates pgvector storage per test, as the unit fixture now does. Addresses Codex review on #1696. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d5c934781
ℹ️ 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".
|
|
||
| # A host that cannot load sqlite-vec runs keyword-only (#711): there is | ||
| # nothing to embed into, so indexing and writes must finish without vectors. | ||
| if not await self.repository.semantic_effectively_enabled(): |
There was a problem hiding this comment.
On a supported Python build that cannot load SQLite extensions, this check is reached too late to protect bm reindex --embeddings: _reindex() constructs a fresh repository with _semantic_enabled=True, and reindex_vectors() calls _purge_stale_search_rows() before this method and reconcile_vector_index() afterward; both invoke _ensure_vector_tables() and raise instead of honoring the keyword-only fallback. Check effective semantic availability around the whole reindex workflow so this maintenance command does not fail on the fallback environment.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b4f8f41: reindex_vectors() checks semantic_effectively_enabled() before purging or reconciling, and returns every entity as skipped on a keyword-only host. Test: test_reindex_vectors_is_a_noop_when_the_vector_runtime_is_unavailable.
| routed_to_cloud = bool(project_names) and all( | ||
| config.get_project_mode(name) == ProjectMode.CLOUD for name in project_names | ||
| ) | ||
| return "hybrid" if routed_to_cloud or semantic_runtime_available(config) else "text" |
There was a problem hiding this comment.
Fresh evidence after the prior cloud-routing fix: this still recognizes only per-project CLOUD mode, although get_client() routes factory injection and explicit --cloud ahead of project mode. On an extension-less local host, bm tool search-notes ... --cloud for a locally configured project—or a custom cloud client factory—therefore sends search_type="text" to the remote Postgres service and silently loses the intended hybrid default. Base this decision on the effective route, including factory and force-cloud routing.
AGENTS.md reference: AGENTS.md:L347-L350
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b4f8f41: new routes_off_host() in async_client follows get_client's precedence (injected factory, then --local/--cloud, then project mode); the search default uses it. Test: test_routes_off_host_follows_get_client_precedence.
| "real_embedder" | ||
| ): | ||
| return | ||
| monkeypatch.setattr(semantic_runtime, "sqlite_vector_runtime_available", lambda: True) |
There was a problem hiding this comment.
Fresh evidence after the earlier extension-host test fix: patching this preflight helper does not affect SQLiteSearchRepository._ensure_sqlite_vec_loaded(), which still disables semantics when the actual driver lacks enable_load_extension. On Python 3.14.4, the focused changed suites consequently fail seven tests, including both test_reindex_vectors cases and five embedding-status cases, because vector sync is skipped or the vec0 table was never created. The fixture must emulate the repository/vector-storage path too, or those tests must be capability-gated.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining this one. These tests exercise sqlite-vec storage itself (vec0 tables, reindex, embedding status), so a Python build that cannot load SQLite extensions cannot run them meaningfully, and emulating vec0 in a fixture would test the fake rather than the storage. Supported installs and CI load sqlite-vec; the keyword-only fallback on extension-less builds is covered by its own tests (vector sync skip, reindex no-op, readiness, search default).
…p vector reindex on keyword-only hosts - routes_off_host() answers "does this request leave this process" with get_client's precedence (injected factory, then --local/--cloud, then each project's mode). search_notes defaults to hybrid when the request runs remotely or the local runtime can run vector search. - reindex_vectors() returns every entity as skipped on a keyword-only host (#711) instead of failing while opening vector tables it cannot load. - Postgres tests that drop search_vector_chunks use CASCADE, since the pgvector table now exists in every test and references it. Addresses Codex review on #1696. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
…n path test-int's schema reset drops search_vector_chunks as well as the pgvector table, so creating only the pgvector table failed on its foreign key. Both conftests now run PostgresSearchRepository.init_search_index(), the same step database initialization takes, which creates the chunk manifest and the pgvector table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 496145d235
ℹ️ 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".
| es = info.embedding_status | ||
| if not es.semantic_search_enabled: | ||
| right.add_row("[green]●[/green] Semantic Search", "Disabled") | ||
| right.add_row("[green]●[/green] Semantic Search", "Enabled") |
There was a problem hiding this comment.
On a SQLite host that cannot load extensions—the supported keyword-only fallback—get_embedding_status() reports missing vector storage and recommends remediation, but this unconditional row still tells the user that semantic search is “Enabled.” Consequently, bm project info presents contradictory status and may direct the user to bm reindex --embeddings, which intentionally skips all work on that host. Expose the effective runtime availability in the status response and display semantic search as unavailable when the fallback is active.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8f40501: EmbeddingStatus gains vector_runtime_available. On a keyword-only host get_embedding_status() returns it False with no reindex recommendation, and bm project info shows 'Unavailable: sqlite-vec cannot load here, keyword-only'. Test: test_embedding_status_reports_keyword_only_host.
… info On a SQLite host that cannot load sqlite-vec (#711), get_embedding_status() now returns vector_runtime_available=False with no reindex recommendation, and bm project info shows semantic search as unavailable instead of Enabled next to advice to run a reindex that would skip every note. Addresses Codex review on #1696. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f40501078
ℹ️ 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".
| # Semantic search is always on. init_search_index() turns it off for this | ||
| # instance only when sqlite-vec cannot load (#711), falling back to keywords. | ||
| self._semantic_enabled = True |
There was a problem hiding this comment.
When sqlite_vec imports and the driver exposes enable_load_extension, but loading the binary fails—for example because the wheel is incompatible or extension loading is denied—this unconditional enablement sends startup through _ensure_vector_tables(). _ensure_sqlite_vec_loaded() lets the resulting sqlite3.OperationalError escape, while init_search_index() catches only SemanticDependenciesMissingError, so MCP/API startup aborts instead of using the documented keyword-only fallback, and the removed config flag no longer provides an escape hatch. Fresh evidence beyond the earlier AttributeError fix is that the new semantic_runtime probe explicitly classifies OperationalError as unavailable, but the repository path does not; translate the load failure into the typed dependency error as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a76c6ed: load_extension's sqlite3.OperationalError is converted to SemanticDependenciesMissingError, so init_search_index() takes the keyword-only fallback. Test: test_sqlite_vec_load_failure_falls_back_to_keyword_search (fails without the conversion).
| # False on a SQLite host that cannot load sqlite-vec, where search runs | ||
| # keyword-only (#711) and the counts below stay empty by design. | ||
| vector_runtime_available: bool = True |
There was a problem hiding this comment.
When the cloud/API server is upgraded before an installed CLI, this response no longer includes semantic_search_enabled, but older releases define that field as required and ProjectClient.get_info() validates the remote JSON as ProjectInfoResponse. Consequently, bm project info against the upgraded server fails validation instead of rendering status; retain the deprecated field during the compatibility window while adding vector_runtime_available for the new distinction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a76c6ed: EmbeddingStatus keeps semantic_search_enabled as a computed field mirroring vector_runtime_available, so older CLIs keep validating. Test: test_embedding_status_still_emits_the_deprecated_semantic_field.
… keep the legacy status field - A sqlite-vec binary that imports but will not load (incompatible wheel, loading denied) raised sqlite3.OperationalError past init_search_index() and aborted startup. It now becomes SemanticDependenciesMissingError and takes the keyword-only fallback (#711), which matters now that there is no config flag to turn semantic search off. - EmbeddingStatus keeps emitting semantic_search_enabled as a computed mirror of vector_runtime_available, so CLIs released before the flag was removed can still read project info from an upgraded server. Addresses Codex review on #1696. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a76c6ed7dd
ℹ️ 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".
| if is_memory_url: | ||
| query = resolved_query | ||
| effective_search_type = _search_type_for(search_type, query) | ||
| effective_search_type = _search_type_for(search_type, query, [active_project.name]) |
There was a problem hiding this comment.
Fresh evidence after the earlier routing fixes: when project_id selects a cloud project whose bare name matches a locally configured LOCAL project, get_project_client() correctly routes by UUID/workspace, but this passes only the bare active_project.name back through routes_off_host(), which finds the local entry and returns false. On an extension-less local host, an ordinary search sent to Cloud is therefore silently downgraded from hybrid to text; carry the already-resolved route/workspace into the default-mode decision instead of re-deriving it from the possibly colliding name.
AGENTS.md reference: AGENTS.md:L347-L349
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining. This needs three conditions at once: a cloud project selected by project_id whose bare name collides with a configured LOCAL project, on a host that cannot load sqlite-vec. The outcome is a working text search rather than hybrid, not a failure, and an explicit search_type still applies. Threading the resolved client route back into the default-mode choice would restructure search_notes for that case; not worth it in this PR.
| semantic_search_enabled: bool | ||
| # False on a SQLite host that cannot load sqlite-vec, where search runs | ||
| # keyword-only (#711) and the counts below stay empty by design. | ||
| vector_runtime_available: bool = True |
There was a problem hiding this comment.
When a newly upgraded CLI connects to an older Cloud/API server that emits only semantic_search_enabled: false, this new field is absent and defaults to true while the legacy input is ignored because it now names a computed field. Consequently ProjectInfoResponse.model_validate() reverses the server's status and bm project info reports semantic search as enabled; populate vector_runtime_available from the legacy field when the new field is missing so client-before-server rolling upgrades remain accurate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f4d95f7: a before-validator fills vector_runtime_available from semantic_search_enabled when only the legacy field is present. Test: test_embedding_status_reads_the_legacy_field_from_older_servers.
An upgraded CLI talking to a server released before the flag was removed receives only semantic_search_enabled. EmbeddingStatus now fills vector_runtime_available from it when the new field is missing, so bm project info does not report a disabled server as enabled during rolling upgrades. Addresses Codex review on #1696. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF Signed-off-by: phernandez <paul@basicmachines.co>
#1696 removed `semantic_search_enabled` from BasicMemoryConfig, but it was tested against a main that predated several milestone merges that still read the flag, so main failed static checks and most test jobs: - `index_project_and_report_readiness` (#1704) passed `embeddings=app_config.semantic_search_enabled`, which raised AttributeError on every `bm project add`. Semantic search is always on, so the add always runs the embedding pass. - test_db_reindex (#1704) parametrized the add on the flag; it now asserts the embedding pass runs. - test_deferred_embedding_resume (#1700) set the flag on its config. - test_config_env_overrides_not_persisted (#1701) used the flag as its example env-overridden boolean; it now uses `format_on_save`. Also makes #1704's cloud-prune test compare against str(Path) so it holds on Windows, where the path prints with backslashes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…gelog Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…ated alias #1696 adds vector_runtime_available and keeps emitting semantic_search_enabled as a computed alias (and accepts it from older servers), so the entry must not say the field was dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…gelog Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…ated alias #1696 adds vector_runtime_available and keeps emitting semantic_search_enabled as a computed alias (and accepts it from older servers), so the entry must not say the field was dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
1 participant
Why
Follow-up to #1691. Semantic search already defaulted on whenever
fastembedandsqlite-vecwere importable, and both are core dependencies, sosemantic_search_enabledonly mattered for installs that turned it off by hand. Keeping it meant a second code path at every write, search and indexing site. Removing it removes those branches.What changed
semantic_search_enabledis removed fromBasicMemoryConfig, along with its dependency-sniffing default and the reranker's "requires semantic" validation.create_search_reader(always composes the semantic stack)index-filebm reindex --embeddings(no longer refuses)search_notes(hybrid) andgrep(FTS by default, hybrid whensemantic=True)get_embedding_statusapp_configdependency they only had for the flag.EmbeddingStatusdrops its now always-truesemantic_search_enabledfield.Implementation details
init_search_index()turns semantic retrieval off for that repository and search continues keyword-only, so Claude Desktop's handshake still succeeds. The error for a vector/hybrid query in that state now reads "semantic search is unavailable" and points at the startup log, and the MCP help text explains the fix instead of telling users to set a flag.BasicMemoryConfigusesextra="ignore", soconfig.jsonfiles andBASIC_MEMORY_SEMANTIC_SEARCH_ENABLEDare ignored rather than rejected. New test:test_legacy_semantic_search_enabled_setting_is_ignored.tests/fake_embeddings.py, wired as an autouse fixture in both conftests): it keeps the realFastEmbedEmbeddingProviderclass, and therefore the configured embedding identity, and replaces onlyembed_query/embed_documentswith a unit-length hash vector. Tests markedsemanticor the newreal_embeddermarker keep the real model.test_project_add_indexing.pyrun a real install, so they embed with the real model; they share the FastEmbed cache instead of downloading into each pristine HOME.Testing
pytest tests test-int -n 8 -m "not semantic and not benchmark and not live"(SQLite): 8,593 passed, 66 skipped, 4m40s. The unit suite alone went from about 3m to 3.6m with the test embedder.tests/cli/test_project_add_indexing.pywith the real model: 2 passed in 35s.just fast-check: clean.Risks / Follow-ups
semantic_search_enabled: false(to avoid the model download or CPU use) now get semantic search; there is no off switch.config.semantic_search_enabledinapps/cloud/src/basic_memory_cloud/main.py:555andpgq/jobs/deps.py:591(feedingentrypoints.py:1723). Those reads must be removed in the Cloud pin bump that picks this up, along with the cloud tests that passsemantic_search_enabled=False.🤖 Generated with Claude Code
https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF