Skip to content

feat: support Chroma HttpClient + per-tenant collection prefix#697

Closed
cschnatz wants to merge 2 commits into
MemPalace:developfrom
cschnatz:upstream-pr/httpclient-collection-prefix
Closed

feat: support Chroma HttpClient + per-tenant collection prefix#697
cschnatz wants to merge 2 commits into
MemPalace:developfrom
cschnatz:upstream-pr/httpclient-collection-prefix

Conversation

@cschnatz

Copy link
Copy Markdown

What does this PR do?

Replaces all chromadb.PersistentClient(path=palace_path) call sites with a shared chromadb.HttpClient(host, port) via get_chroma_client(). Adds get_collection_name() with an optional prefix for per-tenant isolation.

Why?

Two use cases:

  1. Self-hosted with Chroma Server: Running MemPalace in Docker/containers with a Chroma Server instead of PersistentClient. PersistentClient has known production warnings in the Chroma docs.
  2. Multi-tenant hosting: Setting MEMPALACE_COLLECTION_PREFIX=tenant_<uuid> per request isolates tenants within a shared Chroma instance. I built a hosted MemPalace service (mempalace.cloud) using this exact pattern.

New config

Property Env Var Default Purpose
chroma_http_host MEMPALACE_CHROMA_HOST "localhost" Chroma Server host
chroma_http_port MEMPALACE_CHROMA_PORT 8000 Chroma Server port
collection_prefix MEMPALACE_COLLECTION_PREFIX "" Per-tenant prefix

When no env vars are set, behavior is identical to before. Existing setups unaffected.

Files changed (8)

config.py, cli.py, layers.py, mcp_server.py, miner.py, palace.py, palace_graph.py, searcher.py — +97/-40 lines

How to test

# Start a Chroma Server
docker run -p 8000:8000 chromadb/chroma

# Point MemPalace at it
export MEMPALACE_CHROMA_HOST=localhost
export MEMPALACE_CHROMA_PORT=8000

# Test multi-tenant prefix
export MEMPALACE_COLLECTION_PREFIX=test_tenant_1
mempalace mine ~/some-project
mempalace search "anything"

Context

This came out of building mempalace.cloud, a hosted MemPalace service. The patches have been running in production since 2026-04-09. Happy to iterate on feedback.

Checklist

  • No hardcoded paths
  • Backward compatible (no env vars = same behavior as before)
  • 8 files changed, all PersistentClient call sites covered

z3tz3r0 and others added 2 commits April 11, 2026 23:06
…mPalace#666)

Replace "your memory system" with explicit MemPalace references and
tool names (mempalace_diary_write, mempalace_add_drawer, mempalace_kg_add)
in stop and precompact hook block reasons. This prevents Claude Code from
misinterpreting the hook as a native auto-memory save instruction.

Updated in both Python (hooks_cli.py) and standalone shell scripts.

Also fix CONTRIBUTING.md Getting Started to show the fork-first workflow,
matching the PR Guidelines section.
Replace all 14 `chromadb.PersistentClient(path=palace_path)` call sites
with a shared `chromadb.HttpClient(host, port)` via `get_chroma_client()`.
Add `get_collection_name()` that prepends an optional collection prefix
for per-tenant isolation.

This enables two deployment modes:
- **Self-hosted (default):** Set MEMPALACE_CHROMA_HOST/PORT to point at a
  Chroma Server instead of using local PersistentClient. Useful for
  multi-instance or container deployments.
- **Multi-tenant hosting:** Set MEMPALACE_COLLECTION_PREFIX per request
  (e.g., `tenant_<uuid>`) to isolate tenants within a shared Chroma Server.

New config properties (env or config file):
- `chroma_http_host` (MEMPALACE_CHROMA_HOST, default: "localhost")
- `chroma_http_port` (MEMPALACE_CHROMA_PORT, default: 8000)
- `collection_prefix` (MEMPALACE_COLLECTION_PREFIX, default: "")

When no env vars are set, behavior is identical to before (localhost:8000,
unprefixed collection names). Existing single-user setups are unaffected.

Files changed: config.py, cli.py, layers.py, mcp_server.py, miner.py,
palace.py, palace_graph.py, searcher.py (8 files, +97/-40)
@bensig

bensig commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

hey @cschnatz — thanks for this and for the email. we've created #737 to define a formal plugin spec for storage backends. once that's drafted you can adapt this PR to conform to it. the HttpClient + per-tenant prefix work is exactly the kind of thing we want to support — just want to make sure all backend PRs (#665, #574, #700, yours) build to the same contract. will keep you posted.

igorls added a commit that referenced this pull request Apr 12, 2026
Formalizes the BaseCollection/BaseBackend contract introduced as a seam
in #413 into an interchangeability spec that third-party backends can
build to. Driven by six in-flight backend PRs (#574, #643, #665, #697,
#700, #381) each implementing the interface differently.

Key decisions captured: entry-point distribution, typed QueryResult/
GetResult replacing Chroma dict shape, daemon-first multi-palace model
via PalaceRef, required where-clause subset (incl. $contains),
mandatory embedder injection with model-identity validation, capability
tokens, shared pytest conformance suite, and a backend-neutral
migrate/verify CLI.
@igorls igorls changed the base branch from main to develop April 13, 2026 04:46
@igorls igorls self-requested a review as a code owner April 13, 2026 04:46
@igorls igorls added area/cli CLI commands area/hooks Claude Code hook scripts (Stop, PreCompact, SessionStart) area/mcp MCP server and tools area/mining File and conversation mining area/search Search and retrieval enhancement New feature or request storage labels Apr 14, 2026
igorls added a commit that referenced this pull request Apr 18, 2026
…nd registry (RFC 001 §10)

Advances RFC 001 §10 cleanup so backend-author PRs (#574 LanceDB, #665 Postgres,
#700 Qdrant, #697 hosted, #643 PalaceStore, #381 Qdrant) have a stable target
to align against.

Scope (this PR):

- Typed QueryResult / GetResult dataclasses replace Chroma's dict shape at
  the BaseCollection boundary (§1.3). A transitional _DictCompatMixin keeps
  existing callers working while the attribute-access migration proceeds.
- BaseCollection is now kwargs-only across add/upsert/query/get/delete/update
  with ABC defaults for estimated_count/close/health and a non-atomic default
  update() (§1.1–1.2).
- PalaceRef replaces raw path strings at the backend boundary (§2.2).
- BaseBackend ABC with get_collection/close_palace/close/health/detect (§2.3).
- mempalace.backends entry-point group + in-tree registry with
  resolve_backend_for_palace priority order matching §3.2–3.3.
- ChromaCollection normalizes chroma returns into typed results; unknown
  where-clause operators raise UnsupportedFilterError (no silent drop, §1.4).
- ChromaBackend absorbs the inode/mtime client-cache freshness check
  previously duplicated in mcp_server._get_client() (§10 + PR #757).
- searcher.py migrated to typed-attribute access as the reference call
  site; remaining callers land in a follow-up.
- pyproject: chroma registered via [project.entry-points."mempalace.backends"].

Out of scope (explicit follow-ups):

- Full caller migration off the dict-compat shim across palace.py,
  mcp_server.py, miner.py, convo_miner.py, dedup.py, repair.py, exporter.py,
  palace_graph.py, cli.py, closet_llm.py.
- Embedder injection + three-state EmbedderIdentityMismatchError check (§1.5).
- maintenance_state() / run_maintenance() benchmark hooks (§7.3).
- AbstractBackendContractSuite full coverage (§7.1–7.2).
- mempalace migrate / mempalace verify CLI rewrites through BaseCollection (§8).

Tests: 970 passed (up from 967 on develop); new coverage for typed results,
empty-result outer-shape preservation, \$regex rejection, registry lookup,
priority resolver, and PalaceRef-kwarg ChromaBackend.get_collection.

Refs: #743 (RFC 001), #989 (RFC 002 tracking issue).
@igorls

igorls commented May 8, 2026

Copy link
Copy Markdown
Member

Hi, thanks for the contribution.

This PR has merge conflicts with develop, and the branch has not been updated in over 7 days, which puts it before our most recent release. The conflicts are likely against work that landed in that release.

Could you rebase onto develop so we can take another look?

If this change is no longer relevant, feel free to close the PR.

(This message is part of a periodic backlog pass, sent to all open PRs that match this state.)

@igorls igorls added the needs-rebase PR has merge conflicts with develop and needs rebase label May 8, 2026
@cschnatz

cschnatz commented May 8, 2026

Copy link
Copy Markdown
Author

Thanks @igorls — closing this to reduce review queue noise. The work in this PR is fully covered by RFC #737 / #743: collection_prefix becomes PalaceRef.namespace, and the HttpClient variant fits naturally as either a ChromaBackend config option or a separate mempalace-chroma-server plugin per the spec.

Will reopen this PR or file a conformant plugin PR once the storage backend plugin spec stabilizes. The implementation has been running in production at mempalace.cloud since 2026-04-09, so I can contribute the conformance test subclass and migration patterns from real multi-tenant load when the time comes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli CLI commands area/hooks Claude Code hook scripts (Stop, PreCompact, SessionStart) area/mcp MCP server and tools area/mining File and conversation mining area/search Search and retrieval enhancement New feature or request needs-rebase PR has merge conflicts with develop and needs rebase storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants