feat(backends): pgvector substrate via #665 cherry-pick + smoke tests#21
Merged
Conversation
…backend Phase 0 Task 0.2 of the pgvector + AGE migration plan. Decision: WAIT for upstream MemPalace#665 (skuznetsov's PostgreSQL backend) to merge, then cherry-pick — rather than fork-port. 1839 LOC of working code on top of the MemPalace#995/RFC 001 BaseBackend contract; ~51 LOC of moderate merge conflict (palace.py 32, test_backends.py 19, plus trivial README + uv.lock). pg_sorted_heap codepath is gated by extension availability so our apache/age:PG16 + pgvector substrate runs the fallback path which is exactly what we want. Plan-B triggers documented to revisit (4 weeks of maintainer silence, unresolved blocking review concern, or a Phase 1 blocker that requires patching MemPalace#665 internals). On any trigger, switch Phase 1 from Task 1.A.1 to Task 1.B.1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rebuild the stage 2 PostgreSQL backend on top of the RFC 001 backend contract merged in MemPalace#995 instead of carrying forward the stale Chroma-shaped adapter. The new backend implements BaseBackend/BaseCollection with PalaceRef, typed QueryResult/GetResult returns, registry discovery, and env/config backend selection while keeping Chroma as the default path. The PostgreSQL collection supports both pg_sorted_heap and pgvector. It prefers pg_sorted_heap when the extension is installed, falls back to pgvector when only vector is installed, writes rows with INSERT ... SELECT FROM unnest(...) and ON CONFLICT, preserves first-wins semantics for add() and last-wins semantics for upsert(), supports required metadata filters, returns optional embeddings, keeps exact count() separate from estimated_count(), and creates vector indexes lazily after the threshold. Text embedding reuses Chroma's default local embedding function so the postgres extra only adds psycopg2-binary instead of a second ML dependency stack. Add user-facing PostgreSQL backend documentation, a PGXS install helper for pg_sorted_heap/pgvector, README links, lockfile metadata for the postgres extra, and unit coverage for registry exposure, config/env routing, PostgreSQL validation, filter translation, batch upsert, typed result shapes, estimated count, and palace-level backend selection. Fork resolution notes (Jeffrey Hein, 2026-05-11): - README.md: kept fork README (per feedback_fork_readme_handling). - uv.lock: regenerated from auto-merged pyproject.toml so the fork's existing deps and MemPalace#665's psycopg2-binary extra land coherently. - mempalace/palace.py: took MemPalace#665's get_collection() rewrite + re-added `_DEFAULT_BACKEND = get_backend("chroma")` as a fork-only compatibility shim. mcp_server.py + tests still reference the module attribute (cache invalidation + monkeypatch); follow-up commits will migrate callers to the new abstraction. - tests/test_backends.py: took the union of import sets — fork-side HNSW helpers (_HNSW_MISSING_METADATA_DATA_FLOOR, _pin_hnsw_threads, _segment_appears_healthy, quarantine_invalid_hnsw_metadata, quarantine_stale_hnsw) plus MemPalace#665's new postgres helpers (_metadata_value, _parse_vector_literal, _vec_literal). (cherry picked from commit 83d5448)
…emPalace#665 Cherry-picking MemPalace#665 changed `palace.get_collection()` from `_DEFAULT_BACKEND.get_collection(palace_path, collection_name=...)` to `get_backend(backend_name).get_collection(palace=PalaceRef(...), collection_name=..., options=None)`. `get_configured_collection_name()` survives as a thin back-compat wrapper but the live path now reads `MempalaceConfig().collection_name` directly (env `MEMPALACE_COLLECTION_NAME` or file config). Updated fake_get_collection's signature to match the new contract, switched from monkeypatching the function to setting the env var, and asserts the PalaceRef + options arguments alongside the existing collection_name and create assertions. `palace._DEFAULT_BACKEND` is now aliased to `get_backend("chroma")` (see fork-only shim in palace.py); the monkeypatch on that attribute still routes the captured call because the default backend resolves to chroma. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After cherry-picking MemPalace#665, palace.get_collection() only resolved "use config default" when the caller passed exactly DEFAULT_COLLECTION_NAME. Fork-side callers (searcher.search_memories, convo_miner.process_file, sweeper.sweep_palace, sync.sync_palace, diary_ingest, etc.) pass collection_name=None explicitly per the fork's pre-MemPalace#665 convention, which bypassed the check and propagated None to chromadb. Result: 30 test failures in test_searcher.py with "argument 'name': 'NoneType' object cannot be converted to 'PyString'". Accepting both forms — None (fork) and DEFAULT_COLLECTION_NAME (MemPalace#665) — restores the green floor without disturbing MemPalace#665's structure. Eventual follow-up: migrate fork-side callers to omit the keyword or pass the sentinel, then tighten the type back to `str = DEFAULT_COLLECTION_NAME`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three pytest.skipif-gated integration tests for the postgres backend added by MemPalace#665. Activated by setting TEST_POSTGRES_DSN; skipped by default so the suite stays green on machines without a postgres at hand. Substrate is the mempalace-db container deployed to disks (see scratch/postgres-preflight-2026-05-10.md): postgresql://palace:<vault-pw>@disks.jphe.in:5433/mempalace_test Tests: - test_postgres_backend_is_registered — get_backend("postgres") returns PostgresBackend; singleton invariant holds across repeated calls. - test_postgres_backend_smoke — round-trip a single drawer through add/get; verifies metadata is preserved. - test_postgres_vector_distance_query — two drawers at distance 0 and distance √(384·0.64) from the query; assert L2 ordering puts 'near' before 'far'. Cleanup uses col.delete(ids=...) instead of backend.delete_collection (PostgresBackend doesn't expose that method — not part of the BaseBackend interface). Idempotent setup pattern (try-delete-before-add) means re-running the suite doesn't accumulate state. Plan reference: Phase 1 Task 1.A.1 Step 3 of docs/superpowers/plans/2026-05-10-pgvector-age-migration-impl.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`tests/test_corpus_origin_integration.py::test_no_internal_coordination_jargon_in_source_or_tests`
enforces a project convention that internal coordination jargon
("Phase N", "Task N.X.Y", etc.) doesn't leak into source or tests
(see `feedback_apply_naming_decision_actively.md` per the lint
test's error message). My initial smoke-test docstring used
"Phase 2's job" and "Phase 1 Task 1.A.1 Step 3" — both flagged.
Rewrote the docstring with feature-neutral language: "knowledge-graph
coverage lives in its own test module" instead of "Phase 2's job",
and dropped the plan-task reference in favor of citing the postgres
backend module + the BaseCollection contract directly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an optional PostgreSQL storage backend (pgvector / pg_sorted_heap) behind the RFC-001 BaseBackend/BaseCollection contract, plus configuration/env plumbing, docs, and smoke/integration tests.
Changes:
- Register and expose a new
postgresbackend (entry point + built-in registry) withPostgresBackend/PostgresCollection. - Extend configuration to support backend selection and PostgreSQL DSN via env/config, and update palace collection routing accordingly.
- Add docs, an extension install helper script, and new postgres-focused tests (unit + integration-smoke).
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_config_extra.py |
Adds coverage for env/config overrides (collection name, backend, DSN). |
tests/test_backends.py |
Adds postgres backend/unit tests + updates palace get_collection tests for new API. |
tests/test_backends_postgres.py |
Adds integration smoke tests gated by TEST_POSTGRES_DSN. |
scripts/install_pg_backend.sh |
Adds helper to build/install pg extensions and optionally run CREATE EXTENSION. |
pyproject.toml |
Registers postgres backend entry point and adds postgres optional dependency extra. |
mempalace/palace.py |
Routes get_collection() through backend resolution; adds _DEFAULT_BACKEND compat shim. |
mempalace/config.py |
Adds backend + DSN config/env support and backend name normalization. |
mempalace/backends/registry.py |
Registers postgres backend as a built-in backend. |
mempalace/backends/postgres.py |
Implements the optional PostgreSQL backend and collection contract. |
mempalace/backends/__init__.py |
Exports postgres backend/collection symbols from the package. |
docs/postgres_backend.md |
Documents installation and operation of the PostgreSQL backend. |
docs/internal/pgvector-665-decision.md |
Records the fork’s decision-making context for composing upstream PR MemPalace#665. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+433
to
+437
| limit_clause = self._sql.SQL("LIMIT %s") if limit else self._sql.SQL("") | ||
| offset_clause = self._sql.SQL("OFFSET %s") if offset else self._sql.SQL("") | ||
| if limit: | ||
| params.append(int(limit)) | ||
| if offset: |
Comment on lines
+116
to
+121
| def __init__(self, dsn: str, table_name: str = "mempalace_drawers"): | ||
| self.dsn = dsn | ||
| self.table_name = table_name | ||
| self._conn = None | ||
| self._vec_type: Optional[str] = None | ||
| self._table_am: Optional[str] = None |
Comment on lines
+258
to
+283
| if self._table_am == "sorted_heap": | ||
| cur.execute( | ||
| self._sql.SQL( | ||
| "INSERT INTO {} (wing, room, id, document, embedding, metadata) " | ||
| "SELECT wing, room, id, document, embedding_text::{}, metadata_text::jsonb " | ||
| "FROM unnest(" | ||
| "%s::text[], %s::text[], %s::text[], %s::text[], %s::text[], %s::text[]" | ||
| ") AS rows(wing, room, id, document, embedding_text, metadata_text) " | ||
| "{}" | ||
| ).format(self._table_id, self._vec_type_sql, conflict_clause), | ||
| (wings, rooms, doc_ids, row_documents, row_embeddings, row_metadatas), | ||
| ) | ||
| else: | ||
| cur.execute( | ||
| self._sql.SQL( | ||
| "INSERT INTO {} (id, wing, room, document, embedding, metadata) " | ||
| "SELECT id, wing, room, document, embedding_text::{}, metadata_text::jsonb " | ||
| "FROM unnest(" | ||
| "%s::text[], %s::text[], %s::text[], %s::text[], %s::text[], %s::text[]" | ||
| ") AS rows(id, wing, room, document, embedding_text, metadata_text) " | ||
| "{}" | ||
| ).format(self._table_id, self._vec_type_sql, conflict_clause), | ||
| (doc_ids, wings, rooms, row_documents, row_embeddings, row_metadatas), | ||
| ) | ||
|
|
||
| self._maybe_create_vector_index(inserted_rows=len(rows)) |
Comment on lines
+364
to
+373
| "ORDER BY embedding <=> %s::{} " | ||
| "LIMIT %s" | ||
| ).format( | ||
| self._vec_type_sql, | ||
| embedding_select, | ||
| self._table_id, | ||
| where_clause, | ||
| self._vec_type_sql, | ||
| ), | ||
| [embedding, *where_params, embedding, int(n_results)], |
Comment on lines
+9
to
+13
| The substrate the fork uses for these tests is the docker container | ||
| documented in `scratch/postgres-preflight-2026-05-10.md` — | ||
| `apache/age:release_PG16_1.6.0` with `postgresql-16-pgvector` apt-installed | ||
| on top. The AGE graph extension is loaded but not exercised by these | ||
| tests; knowledge-graph coverage lives in its own test module. |
Comment on lines
+85
to
+114
| def test_postgres_vector_distance_query(): | ||
| """A vector query returns rows ordered by L2 distance to the query embedding.""" | ||
| backend = get_backend("postgres") | ||
| palace = _palace_ref("smoke_test_palace") | ||
| collection_name = "smoke_test_distance" | ||
|
|
||
| col = backend.get_collection( | ||
| palace=palace, | ||
| collection_name=collection_name, | ||
| create=True, | ||
| options={"dsn": POSTGRES_DSN}, | ||
| ) | ||
|
|
||
| # Idempotent setup | ||
| try: | ||
| col.delete(ids=["near", "far"]) | ||
| except Exception: | ||
| pass | ||
|
|
||
| col.add( | ||
| ids=["near", "far"], | ||
| documents=["close to the query", "very different"], | ||
| embeddings=[[0.1] * 384, [0.9] * 384], | ||
| metadatas=[{"wing": "test"}, {"wing": "test"}], | ||
| ) | ||
|
|
||
| query_emb = [[0.1] * 384] | ||
| qres = col.query(query_embeddings=query_emb, n_results=2) | ||
| assert qres["ids"][0][0] == "near", "nearest neighbor should be 'near' drawer" | ||
| assert qres["ids"][0][1] == "far" |
Comment on lines
491
to
+494
| default_config = { | ||
| "palace_path": DEFAULT_PALACE_PATH, | ||
| "collection_name": DEFAULT_COLLECTION_NAME, | ||
| "backend": DEFAULT_BACKEND, |
6 tasks
Adds a new test-postgres job to .github/workflows/ci.yml. Runs in parallel with the existing test-linux / test-windows / test-macos / lint matrix; doesn't disturb their behavior. Service container: pgvector/pgvector:pg16 (public Docker Hub image, PG16 + pgvector pre-installed). Bound on localhost:5432 in the job network. Pre-test setup step creates the vector extension inside the mempalace_test database (idempotent IF NOT EXISTS). Test scope: tests/test_backends_postgres.py only. The full pytest suite is already exercised by test-linux without the postgres extra; running it again with TEST_POSTGRES_DSN set would double the suite time on every PR for the marginal coverage of three additional tests. The targeted job gives us the regression signal we want — postgres backend works end-to-end against a real database — without the cost. AGE is deliberately not in the CI image. The apache/age + pgvector combined image we deploy on the homelab (mempalace-db on disks) isn't needed in CI yet — no test in the repo exercises AGE-specific behavior. When the knowledge-graph layer lands and adds AGE tests, the CI image swap is a separate concern (push our mempalace-db:0.1 image to ghcr.io/jphein, or build inline in CI). Until then, pgvector-only is the cheapest correct substrate. The CREATE EXTENSION step uses a Python heredoc with the DSN passed via env var (PGURL) rather than inline psql — avoids the postgresql-client install on the runner and removes one shell-quoting surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 11, 2026
jphein
added a commit
that referenced
this pull request
May 13, 2026
The plan was written 2026-05-10. Two days later PR #21 (feat/pgvector-age-impl) merged + commit a3ee623 landed the AGE skeleton — together completing all of Phase 0, Phase 1, and Task 2.1. Status header at top of the plan now captures the reconciliation: Phase 0 (preflight) ✅ Done 2026-05-11 Phase 1 (pgvector backend) ✅ Done 2026-05-11 (1.A.1, 1.2, 1.3, 1.4, 1.5) Phase 2.1 (AGE skeleton) ✅ Done 2026-05-11 Phase 2.2 (add_triple) ❌ Not done — canonical next task Phase 2.3 (temporal) ❌ Phase 2.4 (kg_backend cfg) ❌ Phase 3 (migration tool) ❌ Phase 4 (benchmark) ❌ 37 of 92 checkboxes ticked within done-task ranges. 1.B.* (fork-port path) sub-tasks remain unticked because we took the 1.A path (wait for MemPalace#665 + cherry-pick). Evidence for each ✅ entry is in the new status header — file paths, line numbers, and the test/CI fixtures that demonstrate the work. The canonical next task is Phase 2.2: implement add_triple() with Cypher MERGE/CREATE on the existing KnowledgeGraphAGE skeleton. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 14, 2026
jphein
added a commit
that referenced
this pull request
May 16, 2026
…37) Four call sites in cli.py still used the legacy positional signature ``backend.get_collection(palace_path, collection_name)`` from before PR #21's MemPalace#665 cherry-pick. They worked because ChromaBackend.get_collection accepts both forms via ``_normalize_get_collection_args`` (the answer to issue #37 question (a): the compat shim exists and is intentional). Migrating for consistency with palace.get_collection and searcher.search_memories, which already use the new shape. Future backends (postgres, in-memory test fakes) may not carry the same positional compat surface; converging the codebase on the kwargs form keeps the migration runway short when one of those tightens. Touched sites (cli.py): - cmd_purge match-count probe (line ~1192) - cmd_drawers list-by-wing/room probe (~1282) - cmd_repair_status drawer count (~1492) - cmd_extract palace open (~1645) One test asserted the old positional call shape — updated to match the new PalaceRef call shape. 75/75 cli tests pass. Closes #37 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
jphein
added a commit
that referenced
this pull request
May 16, 2026
…37) (#91) * refactor(cli): migrate get_collection call sites to PalaceRef kwargs (#37) Four call sites in cli.py still used the legacy positional signature ``backend.get_collection(palace_path, collection_name)`` from before PR #21's MemPalace#665 cherry-pick. They worked because ChromaBackend.get_collection accepts both forms via ``_normalize_get_collection_args`` (the answer to issue #37 question (a): the compat shim exists and is intentional). Migrating for consistency with palace.get_collection and searcher.search_memories, which already use the new shape. Future backends (postgres, in-memory test fakes) may not carry the same positional compat surface; converging the codebase on the kwargs form keeps the migration runway short when one of those tightens. Touched sites (cli.py): - cmd_purge match-count probe (line ~1192) - cmd_drawers list-by-wing/room probe (~1282) - cmd_repair_status drawer count (~1492) - cmd_extract palace open (~1645) One test asserted the old positional call shape — updated to match the new PalaceRef call shape. 75/75 cli tests pass. Closes #37 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: ruff format (CI lint pass-through) --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
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
Fork-side composition on upstream MemPalace#665 (skuznetsov's PostgreSQL backend on the RFC 001
BaseBackendcontract). Decision: cherry-pick now rather than fork-port; full reasoning indocs/internal/pgvector-665-decision.md.End-to-end verified against a live
mempalace-dbcontainer running ondisks.jphe.in:5433(PG16 + pgvector 0.8.2 + AGE 1.6.0, builtFROM apache/age:release_PG16_1.6.0withpostgresql-16-pgvectorapt-installed on top).Numbers: 1828 → 1851 passing (+23: 20 from MemPalace#665, 3 from new smoke file). Zero regressions.
What's in this PR
Six commits, layered from foundation to smoke:
fbd8dbd—docs(internal): record #665 composition stance for pgvector backend— the decision doc + plan-B triggers.5e90c72—feat(backends): add optional PostgreSQL backend— cherry-pick of upstream Add optional PostgreSQL backend with pg_sorted_heap support MemPalace/mempalace#665 (author preserved as Sergey Kuznetsov). Resolved 4 conflicts:README.md(fork-wins perfeedback_fork_readme_handling)mempalace/palace.py— took Add optional PostgreSQL backend with pg_sorted_heap support MemPalace/mempalace#665'sget_collection()rewrite + re-added_DEFAULT_BACKEND = get_backend("chroma")as a fork-only compat shim. mcp_server.py + tests still reference the module attribute; migration is a follow-up.tests/test_backends.py— union of fork-side HNSW helpers and Add optional PostgreSQL backend with pg_sorted_heap support MemPalace/mempalace#665's new postgres helpersuv.lock— regenerated from the merged pyproject.toml941342b—test(backends): update palace.get_collection test for new API after #665—fake_get_collection's signature now acceptspalace=PalaceRef,options=; uses env var instead of monkeypatchingget_configured_collection_name.5c7f234—fix(palace): accept None as "use default collection" in get_collection— caught by 30 test_searcher failures: fork-side callers passcollection_name=Noneper the pre-Add optional PostgreSQL backend with pg_sorted_heap support MemPalace/mempalace#665 convention; Add optional PostgreSQL backend with pg_sorted_heap support MemPalace/mempalace#665 only resolvedDEFAULT_COLLECTION_NAME. Accept both during the transition.04c6294—test(backends/postgres): smoke test for BaseCollection conformance— threepytest.skipif-gated tests covering registry, drawer round-trip, vector distance ordering.1d5486f—test(backends/postgres): remove internal phase taxonomy from docstring— passes the project's no-internal-jargon lint test.Substrate
Phase 0 work (separate doc):
mempalace-dbservice added to/opt/mediaserver/docker-compose.ymlon disks:Password lives canonically in Vaultwarden (
mempalace-db-postgres) and/opt/mediaserver/.env(mode 0600). LAN-bound on disks LAN IP only.Test plan
TEST_POSTGRES_DSNset: 1851 passingPlan-B triggers (if MemPalace#665 stalls)
Follow-ups (separate PRs)
mcp_server.py:204-207, 1703+tests/test_mcp_server.py:1466off the_DEFAULT_BACKENDshim to the newget_backend("chroma")abstraction.mempalace/cli.py:1043, 1133, 1490callers that still usebackend.get_collection(palace_path, "mempalace_drawers")positional signature.scripts/dev-postgres-dsn.shhelper to pull from vault + URL-encode (right now requires 3 commands).🤖 Generated with Claude Code