fix(postgres): embed through get_embedding_function(), not chromadb's default (#413)

[jphein]

What

backends/postgres.py::_embed now delegates to mempalace.embedding.get_embedding_function() instead of constructing its own chromadb.utils.embedding_functions.DefaultEmbeddingFunction().

Why — three defects compounding in one eight-line function

Every embedding-layer improvement in the project lands inside that resolver, so this backend silently missed all of them.

1. MEMPALACE_EMBEDDING_MODEL was ignored on this backend's write path (_prepare_write_inputs) and query path. The config layer resolves correctly — describe_device() reports the endpoint and get_embedding_function() returns OpenAICompatEmbeddingFunction — but _embed never asked, so every vector was computed by a local CPU embedder. Measured on two long-running installs: the configured remote embedding server served 9 texts in 18.3 hours of near-continuous mining. Same class of failure as MemPalace#2324; the external-embedding-API support added in MemPalace#1559 never reached this backend.

2. The ORT intra_op_num_threads cap from MemPalace#1068 was bypassed. That fix lives in _build_ef_class/_MempalaceONNX, wired up only inside the resolver. Measured on a 48-core host: instantiating one such embedder adds ~49 OS threads; daemon CPU averaged 210-265% over 18 hours and was sampled at 2550% mid-mine.

3. DefaultEmbeddingFunction rebuilt the ONNX session on every call. In chromadb 1.5.9 it is not ONNXMiniLM_L6_V2 — it is a separate class in chromadb/api/types.py whose entire __call__ body is return ONNXMiniLM_L6_V2()(input), and ONNXMiniLM_L6_V2.model is a per-instance cached_property. So the module-global _embedder cached an object that held no model.

via DefaultEmbeddingFunction :  0.706s / 0.711s / 0.713s   <- flat, no warm-up ever
via a reused instance        :  1.257s / 0.532s / 0.539s

At DRAWER_UPSERT_BATCH_SIZE = 1000 that is one full session rebuild per 1000 chunks, each spawning and discarding a ~47-thread pool. Per-batch embed time after delegating: 0.706s → 0.016s.

The stated rationale still holds

The old docstring said: reuse Chroma's default local embedding function so the PostgreSQL backend matches the zero-API embedding model already used by the default backend without adding a second ML dependency stack. Delegating satisfies that completely — with no embedding configuration set, the resolver still returns a local ONNX MiniLM embedder. No new dependency, no change to default behaviour. It diverges only where an operator has explicitly configured something else, which is the entire point of MEMPALACE_EMBEDDING_MODEL.

Two deliberate design calls

Not declaring requires_explicit_embeddings. It is architecturally tidier and the other backends do it, but it only takes effect at palace.get_collection() — and mcp_server._get_collection_postgres() calls PostgresBackend.get_collection directly, while convo_miner.mine_sessions() imports mcp_server._get_collection. Both bypass the wrap point, so the capability alone (with _embed raising) would break the MCP server and the miner. Delegating inside _embed fixes every route, wrapped or not, and cannot break a caller that already worked.

Reusing embedding_wrapper._embed_texts rather than re-deriving the conversion. The postgres vector literal formats with %f, and np.float32 scalars do not survive it — that subtlety is documented at length in _embed_texts and should have exactly one home rather than a second copy that can drift.

The new failure mode this change makes reachable, and the guard for it

Before this change _embed could only ever produce EMBEDDING_DIM (384) vectors. Routing through the resolver means an operator can now configure a 768-dim model against a vector(384) column. pgvector's own error names neither the model nor the configuration that selected it, so _warn_on_dimension_mismatch names the model, the produced width, and the column width — once, on the batch that discovers it. Postgres stays the authority on whether the write is legal; this adds no second place that can refuse one.

(Vector-space safety, per the issue: on 500 real stored documents, chromadb's DefaultEmbeddingFunction vs an openai-compat endpoint serving the same all-MiniLM-L6-v2 gave min cosine 0.999999488, k-NN top-20 id-set overlap mean 0.997 — identical space, no re-embedding required there. That will not hold for every configured endpoint, which is what MemPalace#1561/MemPalace#1724 identity enforcement is for.)

Blast radius

mempalace/backends/postgres.py only. The module-global _embedder is deleted rather than moved — get_embedding_function() already caches under a lock.

Tests

tests/test_postgres_embedding_resolver.py — 6 cases:

• the embedder is constructed once across three _embed calls (defect 3 — this is the 0.706s-flat measurement, expressed structurally)
• DefaultEmbeddingFunction is never constructed directly
• the configured embedding function is the one that actually runs (defect 1)
• plain Python floats come back, not numpy scalars
• an empty batch never triggers a model load
• the dimension mismatch is named exactly once, not per batch

All watched failing against main first, and re-verified red after the harness was finalized (4 of 6 red; the float-conversion and empty-batch cases are existing-behaviour guards).

One harness note worth flagging for future test authors: conftest's autouse _stable_embedding_function_for_tests replaces both embedding_wrapper._embed_texts and embedding.get_embedding_function for every module outside its opt-out list. Those stubs sit exactly where this module's subject does and would have silently hidden every assertion here (they did, on the first run). The fixture captures the real functions at import time — collection runs before autouse fixtures — and restores them for this module only. No native ONNX is loaded either way: every test supplies its own embedder.

Full suite: 6821 passed, 82 skipped, 115 deselected. (5 pre-existing test_init_filters_sys_path_from_leaked_pythonpath failures reproduce on an unmodified branch in any git worktree — #454, unrelated.)

ruff check and ruff format --check clean; scripts/check-docs.sh green after all three renderers. Rebased onto main after #453 and #464 landed.

Arms #468

Delegating is what makes an embedder swap possible on this backend, and #468 covers the gap that leaves: postgres.py is the only vector backend with no stored embedder identity, so a swap to a same-width model after this lands is silent — the dimension guard added here catches a width change, and nothing catches a model change at the same width.

Production behaviour is unchanged by this PR: MEMPALACE_EMBEDDING_MODEL is unset in the environment, so the resolver returns the same local ONNX MiniLM embedder the old code constructed directly. The identity work belongs in #468 with MemPalace#1561/MemPalace#1724, not here.

Part of #413
Arms #468

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Warning

Review limit reached

Next included review available in 32 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a94ab069-9d7f-4d14-b5c1-e5c5bc2dcf12

📥 Commits

Reviewing files that changed from the base of the PR and between d8564fc and d6ad8e9.

📒 Files selected for processing (7)

• CLAUDE.md
• FORK_CHANGELOG.md
• README.md
• docs/fork-changes.yaml
• mempalace/backends/postgres.py
• tests/test_postgres_embedding_resolver.py
• website/public/llms-full.txt

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}