Repository navigation
Remove Ollama and all client-side embeddings - #132
Merged
Merged
Conversation
Clean-break removal of the ollama embeddings backend, local vector search (vec0 tables), and the SyncEmbedder worker machinery. Approved section-by-section; single branch, squash-merge PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
11 tasks, inside-out strip: FTS pin first, wiring severance, config knobs, per-service strips, package deletion with #96 verdict, migration 0015, docs sync, final sweep. Suite + Pyright green per task. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verifies observation_fts/observation_trigram_fts population is unconditional (no embeddings_backend gating anywhere under db/), and pins observe -> FTS row -> BM25-evidenced ObservationService.retrieve() end to end with embedder=None, ahead of the Ollama/embeddings removal.
…OG diag Delete Config.ollama_host, embed_model, embeddings_backend, and context_vec_floor along with their env-var resolution and validation (OLLAMA_HOST, EMBED_MODEL, BETTER_MEMORY_EMBEDDINGS_BACKEND, BETTER_MEMORY_CONTEXT_VEC_FLOOR). embeddings/ollama.py (deleted wholesale in a later task) now hardcodes its host/model defaults instead of reading them off Config. contextual_inject.py drops its now-dead vec_floor=cfg.context_vec_floor pass-through, relying on retrieve_relevant's own default. _diag.py's BETTER_MEMORY_EMBED_LOG emergency override is removed; diagnostic logging is gated solely by Config.diag_logging now. Updated tests to assert the fields are gone (AttributeError) and trimmed the e2e/e2e_meta env contracts and fixtures that referenced the deleted knobs.
…n synthesis Strips all Ollama-backed embedding machinery from services/reflection.py: sync_embedder ctor params, the #97 embed_tasks/post-commit batched-embed pass in apply_decision, _heal_missing_embeddings, _vec_ranks, and the vector leg of _fuse_by_relevance (BM25 + Wilson-prior RRF remains). Updates direct constructor call sites (storage/sqlite.py, ui/app.py) and tests asserting on the now-removed _sync_embedder attribute.
_apply_merge's cleanup DELETE was the last write reference to reflection_embeddings anywhere in better_memory/ after Task 5 removed all embedding writes from reflection.py. Task 9's migration drops that table, which would turn this dead-code line into an OperationalError on every merge action inside apply_decision's SAVEPOINT. Removed it and tightened the now-orphaned comment.
ObservationService no longer accepts an embedder or writes vec0 rows; retrieval always resolves to trigram-FTS5 BM25. retrieve_relevant drops its sync_embedder/vec_floor params and vec cosine leg entirely, keeping the BM25 gate, keyword-hit fallback, and agentcore relevance_ranks branch byte-identical. _vec_qualifiers stays in relevant.py (unused by retrieve_relevant now) because SqliteBackend.relevance_ranks still imports it for its own vec leg -- that cleanup is Task 7's job. Moved tests/services/_embedding_fakes.py to tests/embeddings/ (its only remaining consumer after stripping test_relevant.py's vec-gate cases), dropping the now-fully-unused DirectedEmbedder.
…mbing Removes the vec0/sqlite-vec leg from search/hybrid.py (query_vector, second_source) so hybrid_search is always word-FTS5 + trigram-FTS5 BM25 fused via RRF. Deletes the now-unreachable _vec_qualifiers helper in services/relevant.py and the vector leg + query embed it fed inside SqliteBackend.relevance_ranks, along with the dead embedder/sync_embedder constructor params on SqliteBackend and build_backend and their remaining callers (contextual_inject/session_bootstrap hooks, ui/app.py).
Removes the SyncEmbedder worker bridge behind #96.
Local vector search is fully removed (remove-ollama-embeddings Tasks 1-8); sqlite-vec's module is still loaded by db/connection.py, which is what makes the DROP TABLE IF EXISTS statements legal. Drops observation_embeddings, reflection_embeddings, and semantic_embeddings. Also removes RetentionService._prune's dead DELETE FROM observation_embeddings step (and the now-inapplicable savepoint/rollback test for it) — the plan's file list didn't cover retention.py, but its prune path still wrote to the vec0 table on every archived-row prune and would have raised "no such table" the moment 0018 applied. tests/db/test_schema.py: flipped the two vec-table-exists assertions to vec-table-absent, and added a migration test that seeds all three vec0 tables before applying 0018 and checks both the drop and rerun idempotency.
… docs Sweeps every website/*.md, README.md, and docs/agentcore-ui/*.md (plus pyproject.toml's description) for Ollama/embeddings/vec0/vector-search claims left over from the sqlite embedder removal, and rewrites them against the current retrieval path: sqlite = BM25 over FTS5 (word + trigram, RRF-fused) plus keyword-hit fallback; agentcore = server-side relevance_ranks semantic search. gaps/*.md get one-line corrections only, per the docs guardrail; docs/superpowers/** is untouched.
…rrections Section 1 of edit-aws-extracted-reflection-text.md and the intro of retention-superseded-by-event-expiry.md were corrected in the prior commit to say the vec0 embedding tables are dropped, but section 2 of the first file and a later paragraph of the second still described the opposite (present-tense vec0 writes / drops). Same one-line bracketed-correction treatment applied at each remaining spot.
…sweep Task 11 remnant grep (ollama/embed patterns, sqlite-vec/sqlite_vec, and a broad case-insensitive "vec" sweep) turned up docstrings and comments that still described a three-leg BM25/vector-cosine/keyword scorer, a second_source="trigram"/"vec0" parameter, and a vector leg on StorageBackend.relevance_ranks/SqliteBackend -- none of which exist any more after the Ollama embeddings removal. Reworded each to describe the real two-leg story (BM25 qualifiers with a keyword-hit fallback, or the backend's relevance_ranks in agentcore mode) or the BM25-only implementation, without touching known survivors (sqlite-vec import/load in db/connection.py, migration 0018 and its test, and the historically-accurate absence comments in test_contextual_inject.py / test_server_backend_dispatch.py etc. that Task 2/6/7 already covered). Full pytest suite (1734 passed, 19 skipped) and pyright (0 errors) both green.
… sweep Review's broader sweep with bare embed|embedding|embedder patterns (not just the ollama/vec-specific ones from the first pass) found more docstrings/comments describing functionality that no longer exists: ObservationService.create's docstring claimed it still writes an "embedding" row; a test comment claimed inserting observations makes "embeddings exist" (it's FTS rows); test_env_helper_contract.py's smoke docstring said the hermetic env sets "sqlite embeddings" (no such concept remains) alongside "poisoned OLLAMA_HOST" (still real, reworded to explain why); _diag.step's docstring example referenced a self._embedder.embed call that no longer exists anywhere; and contextual_inject.py's module docstring/inline comment described the PreToolUse latch as short-circuiting "DB/embedder work" when there is no embedder work to short-circuit any more. Also reworded test_migration_0011_trigram_fts.py's module docstring, which called trigram FTS5 a feature "for the sqlite embeddings backend" -- a backend concept that was deleted along with embeddings_backend config. All changes are docstring/comment-only (no behavior lines touched). Pyright clean; targeted pytest on the touched/covering test files green (68 passed).
…view remove-ollama-embeddings left several docs and test docstrings still describing the deleted vector/embedding subsystem (BM25 + vector RRF, hybrid FTS5 + sqlite-vec, ChatCompleter-wraps-Ollama, stale env pins). Reword to the real word+trigram FTS5 BM25 RRF-fused retrieval story (agentcore: server-side relevance_ranks), and add a direct executescript rerun-idempotency check for migration 0018 independent of the migration ledger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟢 Claude BugBot Analysis
This PR is a large, mechanical removal of the Ollama embedding subsystem and local vector search (sqlite-vec), replacing vector-based relevance with pure FTS5 word+trigram BM25 fusion. Reviewed all changed source files (config, hooks, mcp server/tools, search/hybrid, services/observation, reflection, relevant, retention, semantic, storage backends, UI app) and traced parameter removals (embedder/sync_embedder/query_vector/vec_floor) through every call site; the removal is internally consistent with no dangling references, no orphaned imports causing runtime errors, and no logic regressions in the surviving BM25/trigram fusion or transaction/savepoint handling.
No bugs were detected in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Clean-break removal of the Ollama embedding subsystem and all client-side vector search, per the approved design spec (
docs/superpowers/specs/2026-09-01-remove-ollama-embeddings-design.md).better_memory/embeddings/(OllamaEmbedder, SyncEmbedder),async_bridge.py, the backfill CLI, all embed write paths, vector retrieval legs (_vec_qualifiers,_vec_ranks, heal/backfill machinery, the SQLite write lock held open across blocking embeds → cross-process "database is locked" (semantic.py:101) #97embed_taskspipeline), and theollama_host/embed_model/embeddings_backend/context_vec_floorconfig knobs. Env varsOLLAMA_HOST,EMBED_MODEL,BETTER_MEMORY_EMBEDDINGS_BACKEND,BETTER_MEMORY_EMBED_LOGare now silently unread.relevance_ranks(unchanged, byte-identical).observation_embeddings,reflection_embeddings,semantic_embeddings). Thesqlite_vecmodule load indb/connection.pyis deliberately retained this release so the DROPs are legal on existing DBs; removing the dependency (and the now-deadhttpxpin) is tracked in Remove sqlite-vec dependency and connection load #131.retention.py's_prunestill deleted fromobservation_embeddingsand would have failed post-migration.tests/services/test_fts_substrate.py); migration 0018 drop + idempotency test.Docs synced: website (configuration/architecture/index/mcp-tools/contributing/agentcore-setup), README, packaged
better_memory/skills/*.md,docs/hooks-setup.md, living agentcore-ui docs, pyproject description. Historical specs/plans untouched.Fixes #96 — the blocking
thread.joinevent-loop freeze lived in the deletedSyncEmbedder/async_bridgeworker machinery (verified sole caller chain before deletion).Test plan
🤖 Generated with Claude Code