Add pluggable vector backends - #1679
Merged
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a pluggable storage backend system to MemPalace, adding support for Qdrant (an opt-in external service backend) and SQLite Exact (a local exact-vector correctness backend) alongside the default Chroma backend. It implements a core-side embedding wrapper for backends requiring explicit vectors, introduces backend detection and mismatch protection, and integrates these backends into the CLI, MCP server, and search workflows. A performance issue was identified in the Qdrant backend's lexical search implementation, where a successful query returning zero results would incorrectly trigger a fallback that scrolls the entire collection. A code suggestion was provided to track the success of the text filter query and avoid this unnecessary overhead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Member
Author
|
Looping in backend authors/operators from the earlier proposals and forks: @skuznetsov @dekoza @RobertoGEMartin @Anush008 @cschnatz @MohamedAbdallah-14 @kostadis @jphein We have a draft implementation of the pluggable vector backend work up in this PR. This is not a request to review every line of the PR; the most useful review would be focused on whether the backend contract and extension points are enough for the backends you proposed or operated. Context from the previous backend threads:
Specific questions where your input would help before we mark this ready for review:
Current shape: Chroma remains default; Any focused objections, missing contract pieces, or “this would break my backend” notes would be especially valuable while this is still draft. |
Member
Author
|
Quick local backend benchmark update, now with a real Qdrant instance running. Setup:
Results:
Interpretation:
The benchmark is intentionally quick and synthetic; we should treat it as a sanity check for backend shape, not a release-grade performance claim. |
igorls
marked this pull request as ready for review
June 3, 2026 07:31
igorls
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes MemPalace’s storage/search layer backend-neutral, keeping Chroma as the default while adding two first-party alternative storage backends (sqlite_exact for local exact-vector correctness and qdrant for opt-in Qdrant REST). It also extends CLI/MCP/config selection and adds a backend-level lexical-search capability to support hybrid search flows beyond Chroma-specific SQLite fallbacks.
Changes:
- Introduces pluggable backend selection/resolution (config/env/CLI/MCP + on-disk artifact detection) with mismatch protection.
- Adds new backends:
sqlite_exact(SQLite + exact cosine + FTS5 lexical) andqdrant(REST client + marker/namespace isolation + lexical fallback). - Refactors search and MCP server paths to use backend capabilities (including
lexical_search) and improves error reporting.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Bumps ruff dev dependency version. |
| pyproject.toml | Registers qdrant and sqlite_exact backends as entry points. |
| README.md | Documents backend selection and Qdrant privacy boundary notes. |
| mempalace/backends/base.py | Adds backend mismatch/capability errors and lexical search typed results/capability. |
| mempalace/backends/init.py | Exports new backends, detection helpers, and new error/types. |
| mempalace/backends/registry.py | Adds multi-backend artifact detection helpers and registers built-in backends. |
| mempalace/backends/embedding_wrapper.py | Adds collection wrapper to auto-embed for explicit-vector backends. |
| mempalace/backends/chroma.py | Adds lexical_search() capability (prefers sqlite FTS, fallback to scan). |
| mempalace/backends/sqlite_exact.py | Implements local SQLite exact-vector backend with filters and lexical search. |
| mempalace/backends/qdrant.py | Implements Qdrant REST backend with marker-based mismatch protection and lexical search. |
| mempalace/palace.py | Routes collection access through resolved backend + embedding wrapper; adds backend resolution helpers. |
| mempalace/searcher.py | Uses backend lexical capability for union candidate strategy; improves backend-specific error returns. |
| mempalace/mcp_server.py | Adds --backend flag and makes MCP collection caching backend-aware with better open errors. |
| mempalace/config.py | Adds backend + Qdrant config fields and persistence (set_backend). |
| mempalace/cli.py | Adds backend flags, persists backend on init, and guards Chroma-only maintenance commands. |
| mempalace/dedup.py | Switches dedup to use configured backend collection instead of hard-coded Chroma. |
| tests/test_sqlite_exact_backend.py | Adds coverage for sqlite_exact backend behavior, filters, lexical search, and mismatch protection. |
| tests/test_qdrant_backend.py | Adds fake-client and optional live coverage for qdrant backend operations and marker behavior. |
| tests/test_mcp_server.py | Extends status/reconnect behavior tests for non-Chroma backends and cache closing semantics. |
| tests/test_dedup.py | Updates dedup tests to patch get_collection() instead of ChromaBackend. |
| tests/test_config.py | Adds tests for backend and Qdrant config/env precedence and persistence. |
| tests/test_cli.py | Adds tests ensuring --backend flag propagates into args and env, including MCP command output. |
| tests/test_backends.py | Adds test ensuring Chroma lexical search uses sqlite FTS path (no full scan). |
| tests/conftest.py | Clears new MCP server cache fields between tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+236
to
+241
| try: | ||
| backend_name = resolve_backend_name(palace_path) | ||
| except BackendMismatchError as e: | ||
| emit(f"\n Backend mismatch at {palace_path}: {e}") | ||
| emit(" Select the matching backend or use a fresh palace directory.") | ||
| return None |
|
Thanks for looping me in, Igor — and for landing this. Answering from the hosted multi-tenant operator angle (the #697 Chroma-HttpClient / per-tenant case). Q5 (namespace isolation): Depends on the backend. Qdrant: yes, Q4 (marker / mismatch protection): Not strict enough for hosted palaces. The one contract ask (still open from #743):
That's the minimum for hosted multi-tenant operators to cite the spec as the basis for tenant isolation, rather than relying on per-backend implementation behavior. Q6 (what would block contributing our backend as a plugin): Two things, both small:
With those, I'd be happy to rebase #697 as a conformant |
3 tasks
Adds a second external storage backend (Postgres/pgvector) alongside Qdrant to prove the BaseBackend/BaseCollection contract generalizes across substrates (SQL + JSONB containment filters + pgvector `<=>` ranking vs Qdrant's REST/dict model), and addresses the review feedback on PR #1679. Backend (mempalace/backends/pgvector.py): - table-per-(namespace, palace, collection) isolation; advertises supports_namespace_isolation - JSONB filter pushdown for the containment subset, local-exact fallback for $or/$contains/comparisons/where_document - BM25 lexical search; marker-based mismatch protection - optional psycopg dependency (lazy import), in-memory fake for CI, live test gated on MEMPALACE_PGVECTOR_LIVE_URL - registered in registry/__init__/pyproject entry point + [pgvector] extra; MEMPALACE_PGVECTOR_DSN / MEMPALACE_PGVECTOR_NAMESPACE config; README docs Isolation contract (RFC 001): - PalaceRef/BaseBackend document the per-id MUST and the cross-namespace MUST, gated on the new supports_namespace_isolation capability token - runnable conformance suite (tests/_backend_conformance.py, tests/test_backend_conformance.py); qdrant + pgvector run it via their fakes Marker fail-loud guard: - qdrant and pgvector now refuse get_collection when local_path is None instead of silently opening a remote collection with no mismatch protection Review fixes: - palace._open_collection_or_explain handles unknown-backend KeyError as a CLI state message instead of an escaping stack trace - dedup.py docstring no longer claims "No API calls" unconditionally (false for remote backends)
Member
Author
|
Thanks @cschnatz — this was exactly the kind of feedback that improves the contract. Update on all your points: Q4 (marker / hosted palaces): Fixed the silent gap. Both server backends now refuse loudly when Isolation MUST + conformance suite (the #743 ask): Done.
It's gated on a new On "one backend doesn't prove generalization": agreed — so this PR now adds a second external backend, Net for your ChromaHttpBackend plugin: the conformance cases + capability token are in; the only remaining blocker (first-class no- |
igorls
changed the title
Member
Author
|
@milla-jovovich please check if its all good to merge or you suggest any other changes |
Comment on lines
+501
to
+513
| def table_dimension(self, table: str) -> Optional[int]: | ||
| try: | ||
| rows = self._execute( | ||
| "SELECT a.atttypmod FROM pg_attribute a " | ||
| "WHERE a.attrelid = %s::regclass AND a.attname = 'embedding'", | ||
| [_quote_identifier(table)], | ||
| fetch=True, | ||
| ) | ||
| except BackendError: | ||
| return None | ||
| if rows and rows[0] and rows[0][0] and int(rows[0][0]) > 0: | ||
| return int(rows[0][0]) | ||
| return None |
Comment on lines
+837
to
+844
| if row is None: | ||
| if not create: | ||
| raise CollectionNotInitializedError(palace_path) | ||
| handle.conn.execute( | ||
| "INSERT INTO collections(name, created_at) VALUES (?, ?)", | ||
| (collection_name, _utcnow()), | ||
| ) | ||
| handle.conn.commit() |
Comment on lines
+925
to
+927
| if row is None: | ||
| raise CollectionNotInitializedError(palace_path) | ||
| collection_id = int(row[0]) |
Comment on lines
+1569
to
+1574
| docs = [doc for _, doc, _ in ordered] | ||
| scores = _bm25_scores(query, docs) | ||
| hits = [ | ||
| LexicalHit(id=str(emb_id), document=doc, metadata=meta, score=float(score)) | ||
| for (emb_id, doc, meta), score in zip(ordered, scores) | ||
| if score > 0 |
Three findings from the Copilot review on ec5d1eb: - pgvector (real correctness bug): table_dimension() read the raw pg_attribute.atttypmod of the vector(n) column, which is not the bare dimension, so reopening a stored pgvector palace could raise a false DimensionMismatchError on the next same-dimension write. Now rounds through format_type(atttypid, atttypmod) (the type's own typmod_out), which yields the canonical vector(N) regardless of encoding or pgvector version. The live roundtrip test now closes + reopens and writes a same-dim vector to guard it. - chroma (real correctness bug): _lexical_search_via_sqlite() returned LexicalHit.id as the internal embeddings.id rowid instead of the public embeddings.embedding_id, so lexical_search -> get(ids=...) did not round-trip (broke hybrid-search id lookups). Now selects e.embedding_id and maps rowid -> public id. Existing FTS test schema updated to include embedding_id (real Chroma schema) and assert the public id; added an end-to-end round-trip test through a real ChromaBackend collection. - sqlite_exact (error-message quality): CollectionNotInitializedError was raised with palace_path instead of the collection name in get_collection and delete_collection, inconsistent with the other backends and line 287. Now names the collection; added a regression test. Earlier first-pass findings (palace.py unknown-backend KeyError, dedup.py docstring) were already fixed in ec5d1eb.
…ctor-backends-qdrant # Conflicts: # README.md
…sting Validated the backend against a real Postgres 18.4 + pgvector 0.8.2 instance (full live roundtrip incl. close/reopen + same-dim write). The reviewer's claim that a vector(n) column's atttypmod is dimension+4 does NOT reproduce: raw atttypmod equals the bare dimension on 0.8.x, so the original direct read was already correct. Keep format_type() anyway as the canonical, version-proof way to read the typmod, and correct the comment to reflect reality instead of asserting a bug that does not exist.
|
Thanks for landing this, Igor — the One follow-through and one question:
On #697 itself: I'll keep iterating our Chroma-HTTP backend on our side and circle back on upstreaming it as a conformant plugin once the no- |
Collaborator
|
Thanks for landing this, Igor — and for tagging the operators. We run a postgres + pgvector palace with an Apache AGE knowledge-graph layer in the same Postgres, behind a long-lived daemon (~400K verbatim drawers, Q1/Q6 — the contract held. A few places the contract currently allows something that bites at scale — field data, not objections to the merge: Q2 — Index lifecycle — the spec has the hook, the backends don't yet. RFC 001 (#743) already specifies Offer — conformance against a live substrate. The runnable suite is a great addition. Since CI exercises pgvector against the in-memory fake, the SQL-specific behavior isn't actually run — the real Net: the surface is sufficient to run a non-Chroma backend. The open question is whether it should also let a backend declare its scaling/maintenance characteristics — indexed-vs-scan lexical, distance metric, the maintenance hooks #743 already sketches — so callers and operators aren't leaning on per-backend behavior. |
This was referenced Jun 8, 2026
igorls
added a commit
that referenced
this pull request
Jun 8, 2026
- §7.4: pin canonical NAMESPACE_MEMPALACE to qdrant's shipped UUID (bensig block) - §2.1/§4.4: supports_namespace_isolation contract (cschnatz) - §5/§1.5: minimal Embedder protocol normative; nameless→unknown (kostadis, bensig) - §2.1/§10: backend-declared distance_metric; searcher.py added to cleanup - §7.3: observable/serializable run_maintenance; no-op-kind omission - §2.4/§8.2: multi-collection-per-palace; exact-vector lossless both ways - §3.3/§4.2/§9: env + versioning clarifications (bensig) - §10/§11/§13: reconcile with #1679; resolve §12 open questions; Status Draft→Accepted Follow-ups tracked: #1724 (embedder identity), #1725 (maintenance hooks), #1726 (searcher.py backend-neutrality).
This was referenced Jun 8, 2026
igorls
added a commit
that referenced
this pull request
Jun 8, 2026
… path (RFC 001, #1725) Adds the maintenance contract RFC 001 specifies but #1679 deferred, and gives pgvector an opt-in HNSW index path with concurrency-safe builds. - base.py: MaintenanceResult (status ran/already_running/noop + free-form stats), UnsupportedMaintenanceKindError, BaseBackend.maintenance_kinds ClassVar (reserved: analyze/compact/reindex; a backend with no analogue MUST omit, not no-op), and BaseCollection.maintenance_state()/run_maintenance() defaults. EmbeddingCollection forwards both (BaseCollection methods shadow __getattr__). - sqlite_exact: analyze (ANALYZE) + compact (VACUUM, autocommit + page stats); omits reindex (exact scan, no ANN index). maintenance_state reports row/page counts. - pgvector: reindex builds the optional HNSW index, serialized by a session-level pg_advisory_lock so concurrent daemon writers learn "already_running" instead of each stacking an ACCESS EXCLUSIVE build (the production wedge). It is opt-in: the default exact `<=>` scan is the 100%-recall path; an HNSW index trades exact recall for scale, so an operator invokes it deliberately. Also analyze; omits compact (autovacuum). Advertises supports_server_side_indexes. maintenance_state reports index presence. - qdrant/chroma: empty maintenance_kinds (qdrant self-optimizes; chroma maintenance is the separate repair CLI) — the faithful "omit" default. Tests: contract + sqlite (real, CI-runnable) + pgvector advisory-lock flow via a fake client (ran/noop/already_running, no live Postgres). Full suite green: 2488 passed, 82.47% coverage. Benchmark three-phase wiring is deferred — the existing benchmarks/ are task-benchmarks, not backend-comparison harnesses, so there is nothing to wire into yet. Closes #1725. Refs #743.
Collaborator
|
Following up on the conformance offer above — ran the suite against our live substrate today. Net result: the in-memory fake is faithful. Every arm the fake passes also passes on real Postgres — 15/15, no fake-passes-live-fails divergence found. Environment: PostgreSQL 16.10 (Debian) · pgvector 0.8.2 · AGE 1.6.0 + pg_trgm in the same server (our production stack) · psycopg 3.3.4 · Portable arms (mirrored 1:1 from Cross-namespace isolation (@cschnatz's arm): same DSN, two namespaces, real tables — no leakage through query/get/count/delete in either direction. Live-only arms the fake can't exercise:
Not portable (excluded, honestly): The runner is up as #1769 — same gate pattern as the qdrant live test ( |
jphein
added a commit
to techempower-org/mempalace
that referenced
this pull request
Jun 11, 2026
…nd surface Rebase adaptation onto current develop (348 commits, including the pluggable-backends follow-ups and wing-normalize MemPalace#1702): 1. Gate cmd_purge behind _maintenance_requires_chroma("purge"), the same guard cmd_migrate / cmd_repair / cmd_repair_status grew for pluggable backends. A palace on a non-chroma backend now gets the standard "purge is Chroma-only in this release (selected backend: ...)" message + SystemExit(2) instead of a misleading "No palace found". 2. Use MempalaceConfig().collection_name instead of hardcoding "mempalace_drawers", matching cmd_repair and the configured- collection convention develop adopted since this branch forked. The e2e purge test sets collection_name on the patched config, same shape as the existing cmd_repair tests. 76/76 in tests/test_cli.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
wangbooth
added a commit
to wangbooth/mempalace
that referenced
this pull request
Jun 12, 2026
* fix: filtered search fallback and diary_write content alias
Two bugs found in production use with a ChromaDB palace of 1200+ drawers
ingested via mixed paths (bulk import + MCP tool calls):
1. searcher.py: filtered search (wing= or room=) crashes with "Error finding
id" when the HNSW vector index is out of sync with the SQLite metadata
store. The outer try/except swallowed the error as a search failure.
Fix: inner try/except catches filter failures, retries unfiltered with
n_results*15 (capped at 500), and post-filters by wing/room in Python.
Degrades gracefully instead of returning an error.
2. mcp_server.py: diary_write requires 'entry' but add_drawer uses 'content',
making it natural to pass content= by analogy. The mismatch returns a
silent MCP -32000 error with no explanation.
Fix: accept 'content' as an alias for 'entry' with a clear error message
if neither is provided.
Both bugs were diagnosed and patched in a live palace. This contributes the
fixes upstream.
* feat(benchmarks): multilingual datasets + parity controls (embed model, num_ctx, language)
Enables shipping decisions for non-English users and fair comparison across
candidates whose Modelfile defaults disagree.
- --language / --languages: load dataset.{lang}.jsonl alongside the base
dataset.jsonl. CSV gains a language column. Synthesized candidate
entries let ad-hoc model tags run without editing candidates.yaml.
- --num-ctx: force Ollama options.num_ctx per request, overriding the
model's Modelfile default. Required for apples-to-apples VRAM/TPS
(qwen3:4b-q8 defaults to 32k = 9.7 GB resident; at 8k it's 5.6 GB).
- --embed-model: thread the semantic-similarity embedding model through
scoring. Default flips to embeddinggemma (was nomic-embed-text v1).
Reason: v1 cosine on EN<->PT-BR same-meaning pairs sits at ~0.607
(right at the 0.6 match threshold), so any phrasing drift collapses
to false-negative. embeddinggemma lands ~0.766 with 2.7x the
signal/noise spread. PT-BR memory_extraction recovered 0.15 -> 0.85
on the same outputs after the swap.
Datasets: 12 new files (pt-BR/es/zh x 4 tasks, 633 samples). Input text
translated; proper nouns and labels stay English so cross-lingual
scoring against the existing labels.jsonl works without re-translation.
* fix(benchmarks): validate --language input + correct --embed-endpoint defaulting
Addresses Copilot + gemini-code-assist review on #1483.
1. Path-traversal guard for --language. The value is interpolated into
the dataset filename (`dataset.{language}.jsonl`), so unvalidated
input could escape `task_dir`. Now:
- regex `^[A-Za-z][A-Za-z0-9]*(?:[_-][A-Za-z0-9]+)?$` accepts en,
pt-BR, zh-CN, fr_CA, etc. and rejects anything with path separators
or `..`
- belt-and-suspenders `Path.resolve().is_relative_to(task_dir)` check
before opening the file
2. --embed-endpoint now defaults to None and is resolved after parsing:
uses --endpoint when --llm-provider=ollama (so remote benchmark
runs score against the same host), else http://localhost:11434.
Help text now matches behavior. runner.py's CLI was also missing the
flag entirely — added so single-task runs honor remote endpoints.
* feat(benchmarks): add DE/FR/HI/IT/KO/RU datasets + --output-dir + translated labels
Adds 6 new language datasets (German, French, Hindi, Italian, Korean, Russian)
across all 4 benchmark tasks (calibration, entity_extraction, memory_extraction,
room_classification) — 630 samples total, same conventions as the existing
pt-BR/es/zh datasets: inputs translated, labels/ground-truth stay English
except where noted.
Changes:
- 24 new dataset.{de,fr,hi,it,ko,ru}.jsonl files across all 4 tasks
- labels.ko.jsonl for memory_extraction: Korean ground-truth so the scorer
compares Korean model output against Korean expected content instead of
English (fixes ~20pp score gap identified during testing — see report)
- runner.py: loads labels.{lang}.jsonl when present, falls back to labels.jsonl
- orchestrator.py: adds --output-dir (writes <dir>/<lang>/YYYY-MM-DD-<host>.csv
per language); --output single-file mode unchanged
- candidates.yaml: adds community tier (igorls classifier variants, heretic)
and local tier (gemma4:e4b)
- translate_datasets.py: script used to generate the translations via Ollama;
included so contributors can extend to new languages without manual work
- reports/2026-05-13-multilingual.md: 210-run benchmark report across
6 models × 7 languages × 5 tasks on RTX 3080 Laptop 8 GB
* fix(benchmarks): address PR #1503 review — file-handle bug, untranslated samples, KO labels
Addresses the review feedback from igorls, gemini-code-assist, and Copilot.
HIGH:
- orchestrator: --output single-file mode now shares ONE (fh, writer) across
all languages instead of opening N handles to the same path. The old code
caused interleaved buffer corruption: first language opened "w", subsequent
ones opened "a", and writes from independent file offsets could overwrite
each other. Verified with a multi-language --output smoke test (4 rows
written, all distinct).
- 19 untranslated/empty samples re-translated:
- dataset.de.jsonl: cal_017
- dataset.hi.jsonl entity_extraction: ent_020, ent_025, ent_032, ent_038
- dataset.hi.jsonl room_classification: rc_017, rc_026, rc_028, rc_040,
rc_064, rc_089, rc_091
- dataset.ko.jsonl room_classification: rc_027, rc_067
- dataset.it.jsonl room_classification: rc_029, rc_030, rc_031, rc_032,
rc_053 (previously empty strings)
- labels.ko.jsonl: restored all proper nouns to English (Doreth, Saela, Ivora,
Ren Solanke, Pol Krisat, Pell Halloran, Bramble, Hollowmounts Institute,
Wendelsea, Bridgewater Community Garden, Wends, Drukar, Aerwyn cycle,
Jaccard, Mason bee, Markdown). Also fixed mistranslation 유전자 사과
(genetic apple) → 재래종 사과 (heirloom apple).
MEDIUM:
- runner.py: refactored label-resolution one-liner into 3 readable lines
and added an info log when falling back to English ground truth, so
readers don't misread "score collapse" as model failure.
LOW:
- orchestrator: moved `import socket` to module top (PEP 8); removed
unused `out_path` from the unpacking tuple.
- translate_datasets.py: renamed loop variable `l` → `code` (ruff E741);
made the _translate_one fallback return path explicit instead of relying
on for-loop fall-through; added a privacy warning in the docstring
flagging that the default `kimi-k2.6:cloud` sends prose to a remote
endpoint and should not be used over real palace data.
- 2026-05-13-multilingual.md: converted analytical paragraphs from
Portuguese to English to match the existing repo convention.
* fix(benchmarks): default --num-ctx to 4096 for apples-to-apples comparison
Without an explicit num_ctx, each candidate ran at its Modelfile default
(32k for the Gemma4 variants, larger for qwen3), so VRAM and latency
weren't comparable across families — a 32k-default model pre-allocates
KV cache a 4k-default model doesn't. The flag's own docstring promised
"apples-to-apples" but defaulted to None, defeating the intent.
All current benchmark prompts fit comfortably under 4k tokens
(memory_extraction is the longest at ~500). Users with longer prompts
can still pass --num-ctx <larger>.
Adds a methodology note to the 2026-05-13 multilingual report so its
VRAM/latency numbers aren't conflated with future runs at the new default.
* feat(embedding): add embeddinggemma-300m ONNX as opt-in multilingual embedder
MemPalace's default embedder (all-MiniLM-L6-v2) is English-only-trained.
Cross-lingual cosine similarity on parallel-translated text averages 0.35
across DE/FR/HI/IT/KO/RU — vs 0.88 for embeddinggemma-300m ONNX (q8) with
the semantic-similarity prefix. RU is the worst at 0.17, meaning a Russian
memory and its identical English translation embed to nearly orthogonal
vectors. Multilingual users effectively cannot retrieve their own memories.
This commit adds embeddinggemma-300m as an opt-in alternative:
* New EmbeddinggemmaONNX class implementing ChromaDB's EF protocol.
Lazy-downloads model_quantized.onnx (~300 MB) via huggingface_hub on
first use; cached under ~/.cache/huggingface/. Applies the sim prefix,
runs onnxruntime inference, truncates to 384 dims via Matryoshka
(MRL), L2-normalizes.
* MRL truncation to 384d is intentional: matches MiniLM's vector width
so collection schemas don't change, and validation showed 384d MRL
actually outperforms full 768d on these similarity tasks (0.893 vs
0.881 avg) — known property of MRL training.
* MEMPALACE_EMBEDDING_MODEL env (default "minilm" for back-compat).
Switching models on an existing palace requires re-embedding —
ChromaDB rejects reads with a mismatched EF name. Run
`mempalace repair rebuild-index` after changing the value.
* New optional dep group: pip install mempalace[multilingual]
Adds huggingface_hub + tokenizers + numpy. Core deps unchanged.
ONNX q8 validated lossless vs the Ollama gguf benchmarked previously
(max delta 0.002 cos across 240 parallel pairs).
* feat(embedding): EF-mismatch error helper, offline tests, migration docs
Three follow-ups bundled for the embeddinggemma EF added in 51702e9:
1. Offline tests for EmbeddinggemmaONNX (10 tests, 0.08s, no network).
Mocks huggingface_hub.hf_hub_download, tokenizers.Tokenizer.from_file,
and onnxruntime.InferenceSession so CI never pulls the 300 MB model.
Guarded with pytest.importorskip so the file is skipped when the
multilingual extra isn't installed. Covers: stable name(), lazy-load
runs exactly once, output shape (n, 384) after MRL truncation, L2
normalization, sim prefix applied, dispatch from
get_embedding_function(model="embeddinggemma"), cache key separates
models, helpful ImportError when deps missing, env override.
2. Friendlier ChromaDB EF-name-mismatch error. Switching
MEMPALACE_EMBEDDING_MODEL on an existing palace previously surfaced
ChromaDB's bare "Embedding function conflict: new: X vs persisted: Y"
ValueError. Now ChromaBackend.get_collection() wraps that error and
points users at the two recovery paths: revert the env var, or run
`mempalace repair rebuild-index --palace <path>`. New
_explain_ef_mismatch helper + 3 tests (unit + end-to-end).
3. Docs: CHANGELOG [Unreleased] entry covers both the new EF and the
error wrapper. README Requirements section mentions the multilingual
extra and points at the embedding.py docstring for the migration note.
* feat(onboarding): multilingual embedder by default for new installs
Onboarding now asks the user once, on first run, whether to use the
multilingual embedding model. The default answer is yes — defaulting to
English-only made the recall promise effectively unreachable for any
non-English content (cross-lingual cos ~0.35 vs ~0.88 for the multilingual
model). The choice is written to config.json so subsequent runs pick the
right EF without re-prompting; existing installs that never set the env
var or ran onboarding stay on minilm for back-compat. MEMPALACE_EMBEDDING_MODEL
still overrides both.
Multilingual deps (huggingface_hub, tokenizers, numpy) move from the
[multilingual] extra into core. The extra is kept as a no-op alias so
existing install scripts keep working. The 300 MB ONNX model is still
lazy-downloaded on first use, not at install time.
`quick_setup` (the programmatic non-interactive path) grows an optional
`embedding_model` arg so tests and benchmark scripts can pick a model
without writing config.json by accident.
EmbeddinggemmaONNX's "missing deps" error now points at the right
recovery path (reinstall mempalace, since the deps are core) rather
than the obsolete pip install mempalace[multilingual] hint.
Tests: 9 new (3 _ask_embedding_model variants + 2 run_onboarding
persistence + 2 quick_setup + 2 set_embedding_model round-trips). The
existing 2 run_onboarding tests now patch _ask_embedding_model so they
don't print to stdout.
* feat: add hooks.auto_save config toggle and shorten block reasons
Add a clean opt-out for auto-save hook blocking (closes #494).
- New `hooks.auto_save` config option (default: true) in
~/.mempalace/config.json and MEMPALACE_HOOKS_AUTO_SAVE env var
- When disabled, stop and precompact hooks pass through without blocking
- Shorten block reason text from 6-line instructions to single-line
prompts — reduces UI noise while keeping tool names explicit
- Both Python (hooks_cli.py) and standalone shell scripts respect the
toggle via config file or env var
* fix: address review — enrich block reasons, clean up tests
- Add parenthetical hints to block reasons so AI knows what each tool
saves (session summary, quotes/decisions/code)
- Remove dead config file creation from test_stop_hook_disabled_by_config
- Add missing test for MEMPALACE_HOOKS_AUTO_SAVE=no env var
- Replace bare except: with except Exception: in shell scripts
* Fix: ruff format config, hooks_cli, and test file
* fix: correct precompact test assertion + ruff format tests
test_precompact_hook_enabled_by_default asserted
result["decision"] == "block", but hook_precompact has never emitted
decision — it mines synchronously and returns {}. Assertion was
copy-pasted from the stop-hook test. Fix to assert result == {} with
_mine_sync mocked so the test verifies the real contract (enabled →
mine + pass through) without actually mining.
Plus ruff format on 6 test files the CI pin flagged.
* fix(hooks): retarget auto_save toggle at silent-save path after #1021 rebase
* style: ruff format tests/test_hooks_cli.py with CI-pinned ruff 0.4.x
* fix(mine): validate FTS5 at end of mine (#1537)
Wires _validate_palace_fts5_after_mine into all three mine entry
points so corrupted-FTS5 palaces cannot silently exit 0 from any
of them:
- _mine_impl (mempalace/miner.py) — project file miner
- mine_convos (mempalace/convo_miner.py) — conversation exports
- mine_formats (mempalace/format_miner.py) — binary office documents
via --mode extract, introduced on develop by #1555 (3.3.6 release)
between this PR's open date and its rebase
cmd_mine surfaces MineValidationError as exit 1 + the same
print_sqlite_integrity_abort banner cmd_repair already prints,
appended with a mine-specific stderr note that hedges attribution
(quick_check cannot tell pre-existing corruption from corruption
this mine produced). 17 tests in tests/test_miner_fts5_validation.py
cover the helper, the three call sites, dry-run / KeyboardInterrupt
skip semantics, and the MineValidationError constructor invariants.
Closes #1537.
Co-authored-by: Caleb Wells <15988028+calebcwells@users.noreply.github.com>
* fix(mcp): clean lone surrogates before ChromaDB write (issue #1235)
MCP clients can emit lone surrogates (\udc00–\udfff) that
cause Python's str.encode('utf-8') to raise UnicodeEncodeError,
which bubbles up as -32000 Internal Error from ChromaDB.
Add _clean(text) helper that uses 'surrogatepass'/'replace' to
remove lone surrogates before the string reaches ChromaDB.
Apply it in tool_add_drawer and tool_diary_write, and use
'surrogatepass' error handler on the SHA256 hash inputs for
defensive idempotency.
* fix(mcp): address code review feedback (PR #1422)
- Fix _clean() docstring (paired surrogate description was misleading)
- Apply _clean() to source_file and added_by metadata fields
- Remove redundant surrogatepass from SHA256 hashes (content already cleaned)
- Apply _clean() to content in tool_check_duplicate
- Apply _clean() to new_doc in tool_update_drawer
- Apply _clean() to sanitized query in tool_search
Addresses feedback from gemini-code-assist[bot] on PR #1422.
* test(mcp): add lone-surrogate sanitisation tests (issue #1235)
Add tests/test_clean_lone_surrogates.py covering:
Unit tests (TestCleanLoneSurrogates, 11 cases):
- _clean() passes normal ASCII and CJK strings unchanged
- lone surrogates (high/low, single/multiple) are replaced with U+FFFD
- real emoji (\U0001f600 astral code points) pass through unchanged
- empty string, all-surrogate string, SHA-256-hash-after-clean
- the specific \udcad surrogate observed in WorkBuddy production logs
Integration tests (TestLoneSurrogateCleaning, 6 cases):
- tool_add_drawer: surrogate in content and in metadata fields
- tool_check_duplicate: surrogate in query
- tool_search: surrogate in search query
- tool_update_drawer: surrogate in updated content
- tool_diary_write: surrogate in diary entry
Fix test environment issue:
- conftest.py redirects HOME to a temp dir, causing chromadb's
ONNXMiniLM_L6_V2 to look for its ONNX model in the wrong location
and trigger a 79 MB network download on every run.
- Fix: at module import time, recover the real USERPROFILE from
conftest._original_env and patch ONNXMiniLM_L6_V2.DOWNLOAD_PATH
before any ChromaDB collection fixture is invoked.
* refactor(mcp): move lone-surrogate strip into shared sanitizers (#1235)
Push the surrogate-cleaning behaviour from per-call-site _clean() helpers
into sanitize_content, sanitize_kg_value, and sanitize_query so every
caller (existing and future) gets the fix automatically. New MCP tools
no longer need to remember to call a separate helper.
- Add strip_lone_surrogates() in mempalace/config.py as the single
regex-based implementation (one U+FFFD per surrogate, not three).
- Wire it into sanitize_content and sanitize_kg_value.
- Wire it into sanitize_query so embedding lookups can't crash either.
- Drop the _clean() helper from mcp_server.py and the per-site calls;
retain a direct strip_lone_surrogates() for source_file/added_by
metadata which doesn't route through any sanitizer.
- Move the ONNX model-cache patch out of the test module and into
conftest.py so it's session-scoped instead of duplicated locally.
- Update tests to assert one U+FFFD per surrogate and exercise the
sanitizer-level entry points directly.
* Merge origin/develop into feat/benchmark-multilingual
Resolves conflicts in CHANGELOG.md and pyproject.toml by combining
the multilingual-embedder additions (huggingface_hub/tokenizers/numpy
core deps, [multilingual] alias, Features section) with develop's
additions (python-dateutil core dep, [extract] extra, tunnel Bug
Fixes and Internal sections).
Prepares PR #1483 for merge into v3.3.6.
* docs(readme): move CAUTION/IMPORTANT alerts below header, soften tone
The scam-alert and Claude-Code-retention admonitions were the first
content visitors saw on the repo page — louder than the project
introduction. Moves both below the logo/title/badges so the project
identity reads first, and softens the scam block (drops the H1
"CRITICAL SECURITY WARNING" + all-caps shouting + redundant emoji)
to a single-paragraph CAUTION. All factual content preserved:
impostor-domain warning, official sources, malware caveat, link to
docs/HISTORY.md.
* chore(release): 3.3.6
Bumps version 3.3.5 → 3.3.6 across pyproject.toml, version.py, plugin
manifests (.claude-plugin/plugin.json, .claude-plugin/marketplace.json,
.codex-plugin/plugin.json), README badge, and uv.lock. Flips CHANGELOG.md
from ``[Unreleased]`` to ``[3.3.6] — 2026-05-24`` and backfills the
major user-facing entries that landed without changelog entries during
the cycle:
Features:
- #1555 office-document mining via --mode extract + virtual line numbers
- #1584 surgical closet pointers with date+line locators (Tier 6a)
- #1558 + #1560 within-wing hallways (entity co-occurrence graph)
- #1565 cross-wing tunnels auto-promoted from hallways
- #1578 Hebbian potentiation + Ebbinghaus decay on hallways/tunnels
- #1236 API-tool transcripts auto-route to wing_api
- #711 hooks.auto_save toggle for silent-mode sessions
- #1605 COCA content-word filter for entity detection
- #1557 case-insensitive entity matching at mine time
- #1483 multilingual embeddings (embeddinggemma-300m) by default
Bug Fixes (selected, user-visible):
- #1540 silent data loss in three unchunked upsert sites
- #1538 paragraph chunker oversized chunks
- #1554 per-file chunk cap too low for transcripts
- #1562 Windows hook subprocess/ChromaDB deadlock
- #1529 create_tunnel corrupted hyphenated wing names
- #1424 save-hook truncated hyphenated project folders
- #1383 KG cache duplicated graphs for symlinked/cased paths
- #1466 silent symlink skip now logged
- #1441 macOS stock-bash 3.2 hook compatibility
- #1500 / #1513 structured JSON-RPC errors on bad MCP input
- #1523 VACUUM + FTS5 rebuild after repair
- #1548 FTS5 validation at end of mine
- plus #1216, #1408, #1438, #1439, #1445, #1452, #1459, #1461, #1466,
#1470, #1477, #1485, #1500, #1513, #1528, #1532, #1543, #1546, #1585
Performance:
- #1474 convo miner pre-fetches mined-set
- #1487 rebuild_index progress callback
- #1530 MCP cold-start diagnostics + opt-in warmup
Lint passes (ruff 0.15.14); mempalace-mcp entry point alignment
verified per RELEASING.md.
* docs(changelog): move tunnel fixes back under Bug Fixes (PR #1609 gemini review)
The two pre-existing entries for #1467 (tunnels.json path) and #1468
(create_tunnel endpoint validation) were sitting at the bottom of the
[Unreleased] block before this release-prep PR. Inserting the new
Performance section between the freshly-backfilled Bug Fixes and these
two pre-existing entries put them under Performance, which is wrong —
they're bug fixes. Moves them back ahead of Performance.
* perf(miner,palace): hoist COCA filter imports out of per-drawer hot paths
The COCA content-word filter shipped in PR #1605 imported
`_get_coca_filter` and `_candidate_entity_words` locally inside two
hot paths:
- `palace.build_closet_lines` — runs per source file during mine
- `miner._extract_entities_for_metadata` — runs per drawer during mine
Both imports are now at module top, where they're resolved once at
import time instead of on every per-drawer call. Module-top imports
also make the dependency graph visible to static analysis (pylint's
C0415 was flagging the locals).
No behavior change. The `_get_coca_filter()` call is unchanged — only
the import statement moved. End-to-end mining produces identical
chromadb output. Addresses the MEDIUM finding gemini-code-assist
raised on PR #1605 review.
Verification: full pytest 2258 passed / 3 skipped / coverage 85.35%.
ruff check + format clean. Linux Py 3.9 / 3.11 / 3.13 via CI-matching
`pip install -e ".[dev]"`: 2249 passed each. End-to-end mine of a
test corpus produces the expected drawer + closet pointer.
* feat(entity): known-systems lexicon keeps multi-word product names atomic
Adds a curated list of multi-word product/system names ("Claude Code",
"GitHub Copilot", "Visual Studio Code", "GPT-4", …) and a compound
pre-pass that detects them atomically before the existing single-word
extraction runs. Without this, the regex-based detector decomposes
"Claude Code" into "Claude" + "Code" — and the COCA filter (shipped
in v3.3.6) then drops "Code" as a content word, leaving "Claude" alone
with the wrong attribution.
What ships
- mempalace/data/known_systems.json — 59 curated compounds covering
common AI assistants, IDEs, model names, cloud platforms, and
Office/Google apps. Each entry is multi-word or hyphenated;
single-word product names ("ChatGPT", "Cursor") have no
decomposition risk and stay handled by the existing regex.
- mempalace/entity_detector.py — _get_known_systems() (cached loader,
mirrors _get_coca_filter from Tier 2) and _apply_known_systems_prepass
which scans for each compound case-insensitively with word boundaries,
counts occurrences, and returns the masked text + count dict so the
subsequent single-word + multi-word loops don't re-decompose.
- mempalace/miner.py and mempalace/palace.py — same pre-pass wired
into _extract_entities_for_metadata (per-drawer tagger) and
build_closet_lines (closet pointer construction). Without these,
the new behavior would only apply at init-time and per-drawer
metadata would still decompose compounds.
How it interacts with Tier 2
Tier 2 (COCA filter) blocks single-word content nouns like "Code"
and "Brutal". Tier 3 protects multi-word product names so they
don't get decomposed in the first place. They complement each
other: the compound pre-pass runs FIRST and masks compounds out
of the text; the COCA filter then runs on the remaining
single-word candidates.
Behavior verification
Before this PR, mining a document containing "Claude Code wrote the
patch" three times emitted entities:
Claude;Claude Code;Code (Code filtered by COCA);
After this PR, the same document emits:
Claude Code
The standalone "Claude" no longer appears (it never actually appeared
alone in the source) and decomposition stops at the compound boundary.
Tests
Nine new tests in tests/test_entity_detector.py covering:
- "Claude Code" detected as atomic compound at extract_candidates
- "Claude" alone NOT in results when only mentioned as part of compound
- Case-insensitive compound matching (claude code, CLAUDE CODE, etc.)
- Single-word "Code" still filtered by COCA (no Tier 2 regression)
- Single-word real name "Aya" still detected (no regression on names)
- Multiple distinct compounds in one text both detected
- Unknown two-word phrase still detected via existing multi-word regex
- known_systems.json ships with expected schema (>=20 entries, all multi-token)
- known_systems.json contains expected high-value entries
Verification
Full pytest 2267 passed / 3 skipped on macOS, coverage 85.34%.
Linux Py 3.9 / 3.11 / 3.13 via CI-matching pip install -e ".[dev]":
2258 passed each. End-to-end mine of a compound-rich corpus
confirms chromadb entities metadata now shows compounds atomic
(Claude Code, GPT-4, GitHub Copilot, Visual Studio Code) with no
decomposition.
* fix(entity): precompile known-systems regex once in cached loader
Addresses gemini-code-assist MEDIUM finding on PR #1613: the previous
implementation of _apply_known_systems_prepass compiled a regex pattern
for every compound on every call, repeating the work on every drawer
mined and every closet built. With 59 compounds × N drawers, that's
59N re.compile() calls for a workload where the patterns never change.
The fix moves compilation into _get_known_systems (already lru_cache'd
to size=1), which now returns tuple[tuple[str, re.Pattern], ...] —
pairs of (canonical name, pre-compiled case-insensitive word-bounded
regex). _apply_known_systems_prepass consumes the cached tuple and
does zero compilation in the hot path.
Behavior is identical: same word boundaries, same case-insensitive
matching, same longest-first ordering, same graceful-degrade on
malformed json. All 74 entity_detector tests still pass on macOS plus
the full 2258-test suite on Linux Py 3.9 / 3.11 / 3.13.
* fix(release): align ruff pin to 0.15.14 + hoist COCA imports out of hot paths
Two release-blocking fixes for v3.3.6:
1. CI ruff pin drift
.github/workflows/ci.yml installed ruff==0.15.9 while pyproject.toml
[dev] extras and .pre-commit-config.yaml both pin 0.15.14. Ruff's
formatter output can change between minor versions, so a contributor
running `pip install -e ".[dev]"` and formatting locally with 0.15.14
would produce output the 0.15.9 lint job rejects. Same failure mode
that surfaced on PR #1579 (2026-05-22). Aligning CI to 0.15.14 keeps
the three pin sites in lock-step.
2. COCA filter imports inside per-drawer hot paths
PR #1605 (COCA content-word filter, shipping in 3.3.6) introduced
`from .entity_detector import _get_coca_filter` and
`from .palace import _candidate_entity_words` inside
_extract_entities_for_metadata (called per drawer) and
build_closet_lines (called per closet). Python caches module imports
so the runtime cost after the first call is small, but the import
machinery still runs Python bytecode every invocation — gemini
flagged this on the original PR. Hoisting to module-level removes
the per-call import overhead.
The hoist is identical to PR #1612, which targets develop. Folding
it into the release so 3.3.6 doesn't ship the perf regression that
3.3.7 would immediately have to fix.
Verification: ruff check + format clean on 0.15.14, full pytest
(2258 passed / 12 skipped) on Linux Py 3.9 / 3.11 / 3.13 via
`pip install -e ".[dev]"` (CI-matching).
* fix(backends): repair missing _type in collection config (#1611)
chromadb <= 1.5.8 writes config_json_str = '{}' (empty JSON) when
creating collections. chromadb 1.5.9 introduced a strict _type check
in the collection config deserialization path -- its absence raises
KeyError: '_type' on palace open. Since the pin allows >=1.5.4,<2,
any upgrade pulls 1.5.9 and breaks every existing palace.
Add a fourth pre-open migration step (_fix_missing_collection_type)
that injects "_type": "CollectionConfigurationInternal" into
collections.config_json_str rows that lack it. Same lifecycle and
marker-file pattern as the existing _fix_blob_seq_ids.
Co-Authored-By: nautis <nautis@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(backends): close sqlite connection before PersistentClient
Address review feedback: `with sqlite3.connect() as conn:` only
manages transactions, it does not close the connection. An open
connection before PersistentClient instantiation can leave WAL state.
Use explicit `try...finally: conn.close()` matching the read-only
helpers elsewhere in the module.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(embedding): add embed_query/embed_documents to EmbeddinggemmaONNX for ChromaDB 1.5.x compatibility
ChromaDB 1.5.x calls embedding_function.embed_query(input=...) via
keyword argument during collection.query(). EmbeddinggemmaONNX lacked
both embed_query and embed_documents methods, causing:
TypeError: embed_query() got an unexpected keyword argument 'input'
whenever semantic search was triggered.
This patch adds the two methods required by the ChromaDB EF protocol,
using (the ChromaDB kwarg name, noqa A002) so that palace
search works correctly with the embeddinggemma model.
Also downloads the companion ONNX file alongside the main model
to prevent runtime InferenceSession failures.
Fixes silent search failures when is set to
embeddinggemma.
* fix(mcp): retry stale-index transient in tool_search
* build(deps-dev): bump ruff from 0.15.14 to 0.15.15
Bumps [ruff](https://github.com/astral-sh/ruff) from 0.15.14 to 0.15.15.
- [Release notes](https://github.com/astral-sh/ruff/releases)
- [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md)
- [Commits](https://github.com/astral-sh/ruff/compare/0.15.14...0.15.15)
---
updated-dependencies:
- dependency-name: ruff
dependency-version: 0.15.15
dependency-type: direct:development
update-type: version-update:semver-patch
...
Signed-off-by: dependabot[bot] <support@github.com>
* fix(palace): file_already_mined iterates all groups when source_file has multiple parent_drawer_id mining passes
Under the additive-mining model (drawer history preserved across re-mines),
a single source_file can have multiple parent_drawer_id groups in the
palace, one per mining pass. Each group carries its own stored
source_mtime and normalize_version.
`file_already_mined` previously used `collection.get(where={"source_file": X},
limit=1)` in the `extract_mode is None` branch and only checked the single
returned row's metadata. ChromaDB's `get(..., limit=1)` has no ordering
guarantee across multiple matching rows, so the returned row was effectively
arbitrary. When ChromaDB happened to return a stale group (older mining
pass with a different stored mtime), the function returned False, the
additive miner concluded the file had changed, and wrote yet another
duplicate group of drawers for a file that had not actually changed since
the last successful mine.
Steady-state failure: for any source_file that has ever been edited (so
that the palace contains groups with different stored mtimes), each re-mine
has a probability of spuriously concluding "file changed" and writing
another duplicate group. Each spurious group compounds the problem because
its stored mtime can also trigger the next spurious re-mine. Storage grows
without bound; search results, hallway counts, and entity-frequency stats
become inflated proportionally.
The fix mirrors the paginated-iteration pattern already used in the
`extract_mode is not None` branch — iterate every drawer for the
source_file in 1000-row pages, short-circuit on the first matching group.
A correct group is one that passes the existing checks: normalize_version
not stale; if check_mtime, stored source_mtime within 0.001 seconds of the
current file mtime. The two branches (extract_mode is None vs set) collapse
into one loop that skips the extract_mode check when no extract_mode was
specified.
Trade-off: average-case cost rises from O(1) limit=1 query to O(N/1000)
paginated scan. For typical sources (1-3 groups) the cost is unchanged
because the loop short-circuits on the first matching group within the
first page. For pathological sources with thousands of groups, the cost
is O(number-of-pages-until-match) — still bounded, no longer flaky.
RED test pins the failure space deterministically
`test_file_already_mined_handles_multiple_groups_under_one_source_file`
uses a MockCollection that simulates two parent_drawer_id groups under one
source_file: a STALE group (older mtime) and a CURRENT group (matching
mtime). The mock returns the STALE group when called with limit=1
(worst-case ChromaDB ordering) and returns BOTH groups when called with
limit=1000 (what a correctly-iterating implementation must do). Test asserts
file_already_mined returns True even when limit=1 picks the stale group.
- Against pre-fix code: test FAILS (function returns False because
limit=1 picks stale group, mtime mismatch returns False)
- Against post-fix code: test PASSES (iteration finds the current group,
short-circuits to True)
Verified RED-then-GREEN locally. Existing 4 file_already_mined tests in
tests/test_miner.py continue to pass:
- test_file_already_mined_check_mtime
- test_file_already_mined_scopes_convo_extract_mode
- test_file_already_mined_extract_mode_paginates_large_sources
- test_file_already_mined_returns_false_for_stale_normalize_version
Verification
- macOS Python 3.12 (local) full pytest : 2268 passed, 0 failed
- Linux Python 3.9.25 (OrbStack) : 2260 passed, 0 failed
- Linux Python 3.11.15 (OrbStack) : 2261 passed, 0 failed
- Linux Python 3.13.13 (OrbStack) : 2261 passed, 0 failed
- ruff check + ruff format --check : all clean
Provenance
Surfaced during the per-query audit on the PR #1628 amendment cycle (the
search for every bare `where={"source_file": ...}` query in the repo).
One of six sites identified. The other five are legitimately file-global
in intent (closet purges, full-rebuild deletes, paginated mode-filtered
scans). This site is the one whose failure mode mirrors the cross-group
stitching pattern PR #1628 fixed at the searcher layer.
* test(embedding): expect 3 hf_hub_download calls (model + .onnx_data weights + tokenizer)
The EmbeddinggemmaONNX lazy-load now fetches the ONNX external-weights file
(model.onnx_data) in addition to the model graph and tokenizer, so a single
warm-up issues 3 downloads, not 2. The lazy-load-once invariant is unchanged
(InferenceSession and Tokenizer.from_file are still each built exactly once).
* fix(normalize): use utf-8-sig to handle BOM-prefixed transcript files
Windows exports of Claude Code JSONL sessions prepend a UTF-8 BOM
(\xef\xbb\xbf). With encoding='utf-8', json.loads() raises JSONDecodeError
on the first line, _try_claude_code_jsonl silently skips every line, and
the file falls through as raw text — losing all structured message content.
utf-8-sig strips the BOM transparently and is backward-compatible with
BOM-free files on all platforms.
* fix(closet_llm): replace non-ASCII symbols in progress output (#1034)
GBK consoles (Windows PowerShell/CMD default) cannot encode U+2713 (✓),
U+2717 (✗), and U+2014 (—). The same class of UnicodeEncodeError fixed
in miner.py via #681 affects closet_llm.py and cli.py.
Replace with ASCII equivalents: [OK], [FAIL], [!], and hyphen.
* fix(convo_miner): preserve blank lines and indentation in AI responses
_chunk_by_exchange stripped every line, joined them with single spaces, and
silently dropped blank lines. That violated the verbatim-always principle
stated in CLAUDE.md and contradicted the function's own docstring, which
claimed 'The full AI response is preserved verbatim.'
Concrete consequences before this change:
- paragraph breaks fused: 'para1\n\npara2' → 'para1 para2'
- list items fused: '1. a\n2. b' → '1. a 2. b'
- code fences destroyed: indented code collapsed to a single line
- search quality degraded because tokenization changed at ingest
Fix is surgical: keep each line as-is, join on newline, trim only
trailing newlines produced by the loop stopping at the next '>' turn.
The fallback path _chunk_by_paragraph has a narrower version of the
same bug (it strips each paragraph); that is out of scope here and left
for a follow-up.
* fix(backends): lower HNSW bloat-guard thresholds to fix sub-50k persist (#1579)
_HNSW_BLOAT_GUARD set batch_size and sync_threshold to 50,000 to
prevent link_lists.bin sparse-file bloat in pre-1.5.x Python chromadb
(#344). chromadb >=1.5.4 Rust bindings do not exhibit that bloat.
The 50k guard meant any mine under 50,000 drawers never triggered
chromadb's _persist(), leaving index_metadata.pickle absent and
link_lists.bin empty. quarantine_stale_hnsw then renamed the segment
on every cold open after a 300s mtime gap, accumulating .drift-*
directories indefinitely.
Lower both thresholds to 2 (empirical Rust-side minimum; 1 is rejected
with InvalidArgumentError) so any mine of 2+ drawers triggers a natural
persist. Verified: batch_size=2 with 20k records produces
link_lists.bin at 171 KB with zero sparse-file inflation.
Existing palaces retain the old 50k thresholds in their collection
metadata until the user runs repair --mode from-sqlite.
Co-Authored-By: Tim Harmon <tim-harmon@users.noreply.github.com>
* style(test): use _HNSW_MISSING_METADATA_DATA_FLOOR constant instead of magic 1024
* fix(backends): detect sub-threshold segments by link_lists state, not data size
chromadb pre-allocates data_level0.bin at index creation (~168 KB for
384-dim embeddings) regardless of record count, so the previous
data-size-vs-floor heuristic in _segment_appears_healthy could not
distinguish a single-record segment (sub-threshold, never persisted)
from an interrupted persist.
Restructure _segment_appears_healthy: when index_metadata.pickle is
absent, check link_lists.bin instead of data_level0.bin size. Empty or
absent link_lists + absent metadata = sub-threshold (never persisted).
Non-empty link_lists + absent metadata = interrupted persist.
Co-Authored-By: 0xKingVee9527 <0xWinner98@users.noreply.github.com>
* fix(backends): re-arm HNSW quarantine gate on mtime change and explicit reconnect (#1573)
The _quarantined_paths gate fired once per palace per process and never
re-armed after external in-place writes (closet_llm, mine, compress)
that drift HNSW segments. The MCP server path (make_client static) had
zero discard logic -- quarantine never re-armed even on inode change.
Extend the discard guard in _client() from inode_changed-only to
inode_changed or mtime_changed or mtime_appeared. Add a guarded
discard in mcp_server._get_client() before make_client(), and an
unconditional discard in tool_reconnect().
Remove dead _auto_repair / palace-daemon comment (does not exist in
this codebase) and correct misleading _get_collection retry-path
comments that overclaimed quarantine re-runs.
Co-Authored-By: C-LaForest <C-LaForest@users.noreply.github.com>
* fix(backends): guard mtime_appeared discard behind _freshness membership
Prevent redundant quarantine re-run when a fresh ChromaBackend instance
opens a palace that was already quarantined by another instance in the
same process. The mtime_appeared transition (cached 0.0 -> real mtime)
now only triggers a discard if the instance previously tracked the path,
distinguishing genuine file appearance from first-access default.
Addresses gemini-code-assist review on PR #1602.
Co-Authored-By: C-LaForest <C-LaForest@users.noreply.github.com>
* fix(ids): delimit hash inputs to prevent drawer_id collisions (#80)
Drawer/closet/triple IDs built by concatenating strings without a
delimiter before hashing can collide: hash((s1 + str(i1))) ==
hash((s2 + str(i2))) whenever s1+str(i1) == s2+str(i2). Concretely,
source_file="foo" + chunk_index=12 produces the same hash input as
source_file="foo1" + chunk_index=2 — both yield "foo12". Under
ChromaDB's primary-key constraint the second write silently
overwrites the first; one chunk's content is lost without surfacing
an error (the surrounding broad `except Exception:` blocks swallow
any backend complaint).
The defect class is "string + string concat fed to a hash without
a delimiter." Per the styleguide's partial-scope-key-migration rule,
every site sharing the pattern is a candidate and must be triaged —
not just the ones the original issue named.
FIX — 6 sites
- mempalace/miner.py:1253 drawer_id, add_drawer() back-compat
- mempalace/miner.py:1386 drawer_id, batched mine loop
- mempalace/miner.py:1416 drawer_ids, closet-build re-derivation
- mempalace/format_miner.py:643 drawer_id, format_miner batched loop
- mempalace/mcp_server.py:1136 drawer_id, tool_add_drawer
- mempalace/knowledge_graph.py:305 triple_id, KG triple insertion
MIGRATED FOR CONSISTENCY — 2 sites
- mempalace/convo_miner.py:87 sentinel_key — was `:`, now `|`
- mempalace/convo_miner.py:422 drawer_key — was `:`, now `|`
Pre-existing delimiter was `:`. Migrated because (a) source_file can
contain literal `:` on Windows paths (`C:\Users\…`) and URL-like
sources (`https://host:8080`), weakening `:` as a separator; (b) `|`
is reserved in Windows filenames so source_file cannot contain it,
making it strictly safer.
DELIMITER CHOICE
- `|` chosen over `:` (Windows / URL source_file edge cases)
- `|` chosen over `\x00` (NUL breaks log greppability + print()
debug + downstream string handling for marginal extra safety)
- Matches the established precedent in mempalace/diary_ingest.py
(lines 52, 76, 91, 98 — all already on `|`)
EXEMPT — audited and correct as-is
Single-input hashes (nothing to delimit):
- mempalace/miner.py:1432 closet_id (source_file only)
- mempalace/format_miner.py:559 sentinel_id (source_file only)
- mempalace/palace.py:433 lock filename (source_file only)
- mempalace/palace.py:629 palace_key (lock_key_source only)
- mempalace/diary_ingest.py:158 content_hash (text only)
- mempalace/hooks_cli.py:329 pidfile digest (joined cmd only)
- mempalace/sources/context.py:141 record digest (source_file only)
Already correctly delimited:
- mempalace/hallways.py:157 `f"{wing}::{a}::{b}"` (`::`)
- mempalace/palace_graph.py:454 `f"{a}↔{b}"` (`↔`)
- mempalace/diary_ingest.py:52,76,91,98 (`|` precedent)
Protected by composition (uniqueness guaranteed by the ID prefix,
not by the hash slice):
- mempalace/mcp_server.py:1635 entry_id is
`diary_{wing}_{strftime('%Y%m%d_%H%M%S%f')}_{sha[:12]}`.
Microsecond-resolution timestamp prefix supplies uniqueness;
the trailing hash is a content-discriminator, not the
write-time uniqueness guarantor.
NEW MODULES
- mempalace/ids.py — single source of truth for ID construction.
Five helpers (make_drawer_id_from_chunk, make_drawer_id_from_content,
make_convo_drawer_id, make_convo_sentinel_id, make_triple_id) plus
an ID_RECIPE = "v2" constant.
- mempalace/collision_scan.py — pre-mining defense. Runs immediately
before each batched ChromaDB upsert; raises CollisionError naming
the colliding (source_file, chunk_index) pairs if any proposed
drawer_id appears more than once with conflicting metadata across
the union of incoming and existing rows.
DETECTION
ChromaDB's primary-key constraint means past collisions were
silently resolved at upsert time (last-write-wins), erasing
evidence. A post-hoc audit cannot reconstruct what was lost from
palace state alone. Therefore:
- Pre-mining risk scan. Before each batched upsert, compute the
proposed drawer_ids for the incoming chunk set AND query existing
drawer_ids from the collection. If any proposed id appears more
than once in the union (incoming-vs-incoming or incoming-vs-
existing) with conflicting (source_file, chunk_index), abort the
mine with an actionable error naming the colliding pairs.
Collision is caught BEFORE it destroys data, which is the only
point at which palace state still carries the evidence.
- New metadata key: `"id_recipe": "v2"` on every drawer written
under the delimited recipe. Audits compare like-for-like;
drawers without `id_recipe` are treated as v1 legacy (undelimited
or `:`-delimited), not as collisions.
- Honest disclosure: palaces mined under any pre-v2 mempalace may
carry silent past collisions whose original content is
unrecoverable from palace state. Future library tier work will
give users a per-drawer audit + opt-in archival path.
TESTS
- tests/test_ids.py: 17 unit tests covering all 5 helpers, the
ID_RECIPE constant, the private `_delimited_sha256` helper, and
the four defect-class collision shapes (chunk_index boundary,
content boundary, extract_mode boundary, ISO datetime boundary).
RED collision tests confirm the v2 recipe breaks the defect class.
- tests/test_collision_scan.py: 10 tests covering clean batches,
idempotent re-mines, incoming-vs-incoming collisions, incoming-vs-
existing collisions, error-message quality, empty batches,
metadata without chunk_index, and ChromaDB backend errors
propagating cleanly.
- tests/test_convo_miner_unit.py: FakeCol gains a stub `get()` so
the pre-mining scan can probe an empty in-test collection.
BACKWARDS COMPATIBILITY
- Existing v1 drawers remain queryable; no rewrite of stored IDs.
- New mining passes write v2 drawers alongside v1 via PR #1628's
additive-mining model.
- No user action required; opt-in cleanup ships separately.
VERIFICATION
- macOS Python 3.12 (local): 2304 passed, 3 skipped, 0 failed,
coverage 85.44% (above 80% threshold).
- Linux Python 3.9/3.11/3.13 (OrbStack with full `pip install -e
'.[dev]'` flow): see PR body for per-version pass counts.
- `ruff check .` + `ruff format --check .`: clean.
- pylint score: ids.py 10.00/10, collision_scan.py 10.00/10; all
modified files >= 8.92 (no regression).
- bandit: clean on all new files; the one MEDIUM finding in
knowledge_graph.py is on lines 385/407 (pre-existing SQL string
construction in timeline queries), not in code touched by this PR.
- pre_push_check.sh: ALL CHECKS PASSED.
Refs: deferred from PR #1572 (Copilot finding #13). Styleguide audit
documented above: 8 fix/migration + 11 enumerated exempt sites.
* fix(backends): strip lone surrogates from documents at the ChromaDB chokepoint
#1235 sanitised lone UTF-16 surrogates for the MCP write tools, but the bulk
ingest paths (miner, convo_miner, sweeper, diary_ingest) build documents
without routing through sanitize_content() and reach ChromaCollection directly.
A single lone surrogate in document text raises UnicodeEncodeError inside
chromadb and aborts the whole add/upsert batch with a -32000 Internal Error,
silently dropping every other row in the same batch.
Complete the chokepoint: add _sanitize_documents_for_chromadb (mirror of
_sanitize_metadatas_for_chromadb) and apply it in add/upsert/update so the
backend guarantees UTF-8-safe documents regardless of caller. IDs and dedup are
unaffected (IDs are computed upstream); only illegal lone surrogates become
U+FFFD, matching the errors="replace" behaviour used elsewhere.
* fix(backends): keep single-string documents whole in surrogate sanitiser
Per Gemini review on #1673: chromadb accepts OneOrMany[Document], so a bare
str document was iterated character-by-character by the list comprehension,
splitting it into per-character documents (the silent corruption this method
exists to prevent). Handle isinstance(str) explicitly; add a regression test.
* fix(config): strip leading/trailing separators in normalize_wing_name
A path-encoded dirname like `-home-user-proj` produced a leading-underscore
slug (`_home_user_proj`) that sanitize_name — and therefore the MCP write
tools — reject, so the conversation miner filed transcripts into wings the
MCP could never write to. Strip leading/trailing `_` after collapsing
separators so the slug is valid. Adds tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(mcp): defer WAL setup so import no longer recreates ~/.mempalace (#1676)
The write-ahead-log directory was created at module scope in
mempalace/mcp_server.py, so importing the MCP server ran
`_WAL_DIR.mkdir(parents=True, exist_ok=True)` and recreated `~/.mempalace`
even after a user removed it to engage the documented kill-switch
(`hooks_cli._palace_root_exists()`, #1305). On every session start this
re-armed the autosave/mining hooks the user had disabled.
Move the WAL directory setup into a lazy `_ensure_wal()` called from the
write path (`_wal_log`). Importing the module no longer touches disk; the
directory is created on the first real write, when the palace is being
written to anyway. The WAL is intentionally not gated on
`_palace_root_exists()` (the ChromaDB/KG layer recreates the palace
regardless, so gating would only drop audit records); runtime kill-switch
enforcement for MCP writes is tracked in #504.
Add regression tests: a subprocess import asserts `~/.mempalace` is not
created, and a write test asserts the directory is created lazily with the
expected permissions.
Co-Authored-By: Grace Gettert <9805362+ggettert@users.noreply.github.com>
* feat: add pluggable vector backends
* fix: avoid qdrant lexical full scan on empty text hits
* fix(hallways): paginate drawer fetch to avoid SQLite variable overflow on large wings (#1619)
compute_hallways_for_wing fetched the whole wing in a single
col.get(where={"wing": wing}). ChromaDB binds one SQL variable per matched id,
so on a wing larger than SQLITE_MAX_VARIABLE_NUMBER (32766) the call raised
"too many SQL variables" inside chromadb. The exception was caught, so the mine
completed — but the wing's hallway graph silently never built, and the
cross-wing tunnels promoted from it were starved, on exactly the large wings
that benefit most from navigation. Confirmed threshold: a 42,062-drawer wing
crashed; a 29,629-drawer wing succeeded.
Replace the single where-get with the established pagination pattern: count()
+ get(limit=5000, offset=...) filtered to the wing client-side — matching
miner.status, palace.regenerate_closets, and palace_graph.build_graph, which
already paginate to dodge the same 32766 limit.
Tests:
- test_hallways_pagination: a collection whose where-get raises (simulating the
overflow) while count() + paginated get works — RED before, GREEN after.
- test_hallways: _fake_collection updated to the paginated API; existing
hallway tests are unchanged in behavior.
Closes #1619.
* Update tests/test_hallways_pagination.py
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* style(tests): format test_hallways_pagination.py with ruff (#1680)
The where-filter update to the pagination regression test left one list
comprehension past the line-length limit, so `ruff format --check .` failed
in CI while every test platform passed. Wrap the comprehension as ruff
format produces it — no logic change.
Restores the lint job to green.
* docs(hallways): correct col contract in compute_hallways_for_wing docstring (#1680)
The docstring still said col "must support .get(where=..., include=...)",
but this PR changed the fetch to count() + paginated
get(limit=, offset=, include=) filtered client-side, precisely to avoid the
get(where=...) path that overflows SQLite's variable limit on large wings.
Update the Args entry to describe the real contract so fake collections and
alternate backends implement the right shape.
Docstring only — no behavior change.
* fix(status): count drawers from sqlite instead of cold-loading the HNSW index
`mempalace status` opened the ChromaDB collection purely to tally drawers by
wing/room — and opening it cold-loads the HNSW vector index. On a 398k-drawer
palace that load costs ~60s of CPU on every invocation, even though the counts
live in chroma.sqlite3's relational tables (`repair-status` already reads them
in <1s; `status` was the outlier).
Read the wing/room histogram directly from chroma.sqlite3 via a new
`_sqlite_wing_room_counts` helper, falling back to the existing ChromaDB-client
path when the sqlite read is unavailable (missing DB, un-bootstrapped
collection, sustained writer lock, or an unexpected schema) — preserving the
state-specific guidance for absent/empty palaces.
Measured on a 398,315-drawer / 3.5GB palace: status CPU ~60s -> ~1s.
Review hardening:
- PRAGMA busy_timeout so a transient checkpoint lock is waited out rather than
instantly demoted to the slow path; a sustained lock still falls back.
- COALESCE over string/int/float so a numeric wing/room matches the ChromaDB
path instead of dropping to "?".
- Explicit `s.scope = 'METADATA'` so the segment join can't silently
double-count on a future ChromaDB layout.
Tests: exact-tally (anti fan-out), no-cold-load regression (proven failable by
reverting the fix), numeric-metadata, partial-metadata "?" bucketing,
locked-DB fallback, and collection-absent None routing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Fix crash when tool_use input is a list instead of dict
Some Claude Code JSONL transcripts have tool_use blocks where the
`input` field is a list rather than a dict (e.g. multi-content tool
calls). This causes an AttributeError on line 554 when code tries to
call `.get()` on the list.
Guard by normalizing list inputs to an empty dict, allowing mining
to proceed without losing other tool metadata.
Fixes: mining Claude Code conversations crashes with
AttributeError: 'list' object has no attribute 'get'
at normalize.py:554
* fix(hooks): file stop-hook diary checkpoints under the harness agent identity (#1693)
Stop-hook checkpoints were saved via _save_diary_direct with a hardcoded
agent_name="session-hook". tool_diary_read filters Chroma metadata by
agent, so mempalace_diary_read(agent_name="claude") never surfaced any
hook-saved checkpoint. Derive the diary identity from the harness
(claude-code -> claude, codex -> codex; an unknown harness keeps its own
name) and thread it through to tool_diary_write. Make agent_name a
required keyword argument so the identity is always explicit.
Co-Authored-By: YC-AIUSER <273917354+YC-AIUSER@users.noreply.github.com>
* feat(docker): add container image for MCP server and CLI
Add a multi-stage, uv-based Dockerfile producing a CPU image (with the
extract + spellcheck extras), plus a CUDA variant (Dockerfile.gpu) for
onnxruntime-gpu accelerated embeddings.
A single flexible entrypoint dispatches to the MCP stdio server (default)
or the mempalace CLI. All state -- palace, config, and the lazily
downloaded embedding model -- persists under /data via HOME, runs as a
non-root user, and is exposed as a volume.
Also add a docker-compose.yml for convenience, a GHCR publish workflow,
and a Docker section in the README.
* fix(docker): address gemini review on MR #1696
* feat: add pgvector backend + namespace-isolation conformance contract
Adds a second external storage backend (Postgres/pgvector) alongside Qdrant
to prove the BaseBackend/BaseCollection contract generalizes across substrates
(SQL + JSONB containment filters + pgvector `<=>` ranking vs Qdrant's REST/dict
model), and addresses the review feedback on PR #1679.
Backend (mempalace/backends/pgvector.py):
- table-per-(namespace, palace, collection) isolation; advertises
supports_namespace_isolation
- JSONB filter pushdown for the containment subset, local-exact fallback for
$or/$contains/comparisons/where_document
- BM25 lexical search; marker-based mismatch protection
- optional psycopg dependency (lazy import), in-memory fake for CI, live test
gated on MEMPALACE_PGVECTOR_LIVE_URL
- registered in registry/__init__/pyproject entry point + [pgvector] extra;
MEMPALACE_PGVECTOR_DSN / MEMPALACE_PGVECTOR_NAMESPACE config; README docs
Isolation contract (RFC 001):
- PalaceRef/BaseBackend document the per-id MUST and the cross-namespace MUST,
gated on the new supports_namespace_isolation capability token
- runnable conformance suite (tests/_backend_conformance.py,
tests/test_backend_conformance.py); qdrant + pgvector run it via their fakes
Marker fail-loud guard:
- qdrant and pgvector now refuse get_collection when local_path is None instead
of silently opening a remote collection with no mismatch protection
Review fixes:
- palace._open_collection_or_explain handles unknown-backend KeyError as a CLI
state message instead of an escaping stack trace
- dedup.py docstring no longer claims "No API calls" unconditionally (false for
remote backends)
* ci: add PyPI trusted-publishing workflow
Publish to PyPI on a published GitHub Release via Trusted Publishing
(OIDC — no stored token), gated by the `pypi` environment's manual
approval. The build job verifies the release tag is reachable from main
and matches mempalace/version.py before building the sdist + wheel; a
separate publish job holds the id-token scope and does the upload.
Documents the one-time setup (PyPI trusted publisher + `pypi`
environment) and the per-release runbook in docs/RELEASING.md.
* docs: bump version on develop, not directly on main
Address review on #1698: committing the version bump straight to main
bypasses branch protection and drifts develop behind. Bump on develop
first; it reaches main via the develop -> main merge.
* ci(docker): fix latest/main publishing, add arm64 + GPU build check
Review fixes for the Docker packaging PR:
- docker-publish: tie the `latest` tag to pushes on `main` (the release
branch). Previously it was gated on `is_default_branch`, but the
default branch is `develop` and the workflow never ran there, so
`latest` was never produced. main + `v*` tags publish; develop is
validated via the pull_request trigger but does not publish.
- docker-publish: publish multi-arch amd64+arm64 (Apple Silicon / ARM)
on real pushes via setup-qemu-action; PRs stay amd64-only for speed.
- docker-publish: only export the GHA cache on in-repo events (fork PRs
get a read-only cache, which just emits 403 noise).
- docker-publish: add a build-only job that validates Dockerfile.gpu
compiles so the CUDA variant can't silently rot.
- Dockerfile: correct the persistence comment — the default `minilm`
model caches under ~/.cache/chroma (ChromaDB S3), not
~/.cache/huggingface (that's the optional embeddinggemma model).
- docker-compose: drop the redundant MEMPALACE_PALACE_PATH override (it
duplicated the HOME=/data default); document overrides as examples.
* docs: clarify publish.yml trigger comment
Address review on #1698: a published GitHub Release may create the v* tag
or reuse an existing one. Reword the header so it no longer implies the tag
is always created at release time, and state the real guarantee — the
in-workflow checks make the pipeline self-contained (tag on main + matches
the version manifest), independent of version-guard.
* fix: address Copilot second-pass review on the pluggable-backend PR
Three findings from the Copilot review on ec5d1eb:
- pgvector (real correctness bug): table_dimension() read the raw
pg_attribute.atttypmod of the vector(n) column, which is not the bare
dimension, so reopening a stored pgvector palace could raise a false
DimensionMismatchError on the next same-dimension write. Now rounds through
format_type(atttypid, atttypmod) (the type's own typmod_out), which yields
the canonical vector(N) regardless of encoding or pgvector version. The live
roundtrip test now closes + reopens and writes a same-dim vector to guard it.
- chroma (real correctness bug): _lexical_search_via_sqlite() returned
LexicalHit.id as the internal embeddings.id rowid instead of the public
embeddings.embedding_id, so lexical_search -> get(ids=...) did not round-trip
(broke hybrid-search id lookups). Now selects e.embedding_id and maps rowid
-> public id. Existing FTS test schema updated to include embedding_id (real
Chroma schema) and assert the public id; added an end-to-end round-trip test
through a real ChromaBackend collection.
- sqlite_exact (error-message quality): CollectionNotInitializedError was
raised with palace_path instead of the collection name in get_collection and
delete_collection, inconsistent with the other backends and line 287. Now
names the collection; added a regression test.
Earlier first-pass findings (palace.py unknown-backend KeyError, dedup.py
docstring) were already fixed in ec5d1eb.
* docs: correct pgvector table_dimension comment after real-Postgres testing
Validated the backend against a real Postgres 18.4 + pgvector 0.8.2 instance
(full live roundtrip incl. close/reopen + same-dim write). The reviewer's
claim that a vector(n) column's atttypmod is dimension+4 does NOT reproduce:
raw atttypmod equals the bare dimension on 0.8.x, so the original direct read
was already correct. Keep format_type() anyway as the canonical, version-proof
way to read the typmod, and correct the comment to reflect reality instead of
asserting a bug that does not exist.
* fix(mcp): repair diary_write content alias + restore -32602 diagnostic
#1245 landed on develop with three defects that turned the branch red:
- tool_diary_write gave `entry` a default (`entry: str = None`), which
silently disabled the signature-based missing-parameter diagnostic
(-32602) — two TestParamShapeDiagnostics tests failed.
- the `content` alias was never added to the tool input schema, so the
dispatch arg-filter stripped `content` before the handler ever saw it,
so the alias never actually worked.
- the filtered-search fallback inlined into search_memories pushed it
over the C901 complexity ceiling (30 > 25), and mcp_server.py was left
unformatted.
Fix:
- restore `entry` as a required param (revives the -32602 diagnostic)
- add `content` to the diary_write schema and remap content->entry at
dispatch, before the handler, so a content-only call still satisfies
the required `entry` (entry wins if both are supplied)
- extract the fallback into _query_drawers_with_filter_fallback() so
search_memories drops back under the complexity ceiling
- ruff format mcp_server.py
- add regression tests for the content alias (content-only + both-supplied)
Keeps #1245's filtered-search recall fix intact; turns develop green.
* fix(mcp): address bot review on #1700
- searcher: read the unfiltered fallback result via _first_or_empty()
instead of raw["documents"][0], matching the codebase's QueryResult/dict
polymorphism helper and guarding the empty-result IndexError (Gemini).
- mcp_server: the content->entry remap now fills only when 'entry' is
absent or None, so an explicit (even "") entry wins over the alias —
the truthiness check could clobber an empty entry (Gemini/Copilot).
- mcp_server: diary_write schema now expresses the real contract —
agent_name required + anyOf(entry, content) — so schema-validating
clients can legally call with content only (Copilot).
- test: lock the explicit-empty-entry edge (content must not override).
* test(miner): compare default wing to normalized dirname, not raw name
test_load_config_uses_defaults_when_yaml_missing asserted the derived
wing equals project_root.name. That only held when the random tempfile
name had no separators; tempfile's alphabet includes '_', so once
normalize_wing_name strips leading/trailing '_' (this PR), a name like
'tmpXXXX_' makes the derived wing diverge from the raw name. Compare
against normalize_wing_name(project_root.name) — the actual contract —
which is deterministic across platforms. (Surfaced as a test-windows
failure on this PR, but it was cross-platform flaky.)
* feat(migrate): mempalace migrate-wings — normalize legacy wing names
Follow-up to the wing-name normalization (#1675). Palaces built before the
rule filed drawers under leading/trailing-separator wing names (e.g. a
Claude Code path-encoded dir `-home-user-proj` -> `_home_user_proj`); the
new derivation strips those, so searches/diary reads under the new name miss
the old memories — the history is split, not lost.
`migrate_wing_names` (CLI: `mempalace migrate-wings [--dry-run] [--yes]`)
re-keys the `wing` metadata field on drawers and closets to the normaliz…
igorls
added a commit
that referenced
this pull request
Aug 11, 2026
- §7.4: pin canonical NAMESPACE_MEMPALACE to qdrant's shipped UUID (bensig block) - §2.1/§4.4: supports_namespace_isolation contract (cschnatz) - §5/§1.5: minimal Embedder protocol normative; nameless→unknown (kostadis, bensig) - §2.1/§10: backend-declared distance_metric; searcher.py added to cleanup - §7.3: observable/serializable run_maintenance; no-op-kind omission - §2.4/§8.2: multi-collection-per-palace; exact-vector lossless both ways - §3.3/§4.2/§9: env + versioning clarifications (bensig) - §10/§11/§13: reconcile with #1679; resolve §12 open questions; Status Draft→Accepted Follow-ups tracked: #1724 (embedder identity), #1725 (maintenance hooks), #1726 (searcher.py backend-neutrality).
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
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
This draft makes the normal MemPalace storage/search paths backend-neutral and adds three first-party non-Chroma backends for the next release:
sqlite_exact: local SQLite + NumPy exact-vector backend with no new dependencyqdrant: opt-in Qdrant REST backend, defaulting to localhost, with no new Python dependencypgvector: opt-in Postgres backend (SQL + JSONB + the pgvector<=>operator), defaulting to a localhost DSN, with an optionalpsycopgdependencyChroma remains the default backend. Chroma repair/migrate/HNSW flows stay explicitly Chroma-only. The two external backends (
qdrantREST +pgvectorSQL) deliberately sit on different substrates so theBaseBackend/BaseCollectioncontract is exercised across paradigms rather than shaped around a single vendor.What changed
palace.get_collection()andBaseCollectioninterfaces.ChromaCollection.lexical_search().sqlite_exactwith persistence, exact cosine ranking, Chroma-compatible filter subset, FTS5 lexical search, and Python fallback.qdrantwith REST create/upsert/query/get/delete/count/health, metadata filters, lexical search fallback, namespace/palace isolation, remote-target marker checks, and opt-in live integration coverage.pgvectorwith table-per-(namespace, palace, collection) isolation, JSONB containment filter pushdown plus a local-exact fallback for$or/$contains/comparisons/where_document, pgvector<=>ranking, BM25 lexical search, marker-based mismatch protection, optionalpsycopgdependency (lazy import), an in-memory fake client for CI, and opt-in live integration coverage.PalaceRef/BaseBackendnow document the per-idMUST and the cross-namespaceMUST, gated on a newsupports_namespace_isolationcapability token. Added a runnable, backend-agnostic conformance suite (tests/_backend_conformance.py,tests/test_backend_conformance.py) that every backend exercises.qdrantandpgvectornow refuseget_collectionwhenlocal_path=Noneinstead of silently opening a remote collection with no protection (a first-class remote/no-local_pathmarker store is a tracked follow-up).dedup.pydocstring corrected for remote backends.Validation
uv run pytest tests/ -v --ignore=tests/benchmarks—2352 passed, 5 skipped, 1 warninguv run ruff check .— passeduv run ruff format --check .— passed82%total (CI gate80);pgvector.pyat68%, on par withqdrant.py(remainder is the live-only SQL/REST client).The Qdrant and pgvector live API roundtrips are gated by
MEMPALACE_QDRANT_LIVE_URL/MEMPALACE_PGVECTOR_LIVE_URL; the standard suite uses in-memory fake clients for deterministic local CI.