Skip to content

feat: add MemPalace memory provider plugin - #12203

Closed
Jessica-lol wants to merge 1 commit into
NousResearch:mainfrom
Jessica-lol:feat/mempalace-provider-pr
Closed

feat: add MemPalace memory provider plugin#12203
Jessica-lol wants to merge 1 commit into
NousResearch:mainfrom
Jessica-lol:feat/mempalace-provider-pr

Conversation

@Jessica-lol

Copy link
Copy Markdown

Summary

  • add a modular MemPalace memory provider plugin with config parsing, collection naming, room scoping, structured metadata, tools, and lifecycle hooks
  • add plugin test coverage for foundation behavior, loader integration, module layout, and end-to-end memory flows
  • deduplicate overlapping prefetch memory lines across providers to reduce repeated context when builtin memory and MemPalace return the same fact
  • document installation, configuration, verification, and reviewer quick start in the plugin README

Testing

  • pytest tests/agent/test_memory_provider.py tests/plugins/test_mempalace_v2_foundation.py tests/plugins/test_mempalace_module_layout.py tests/plugins/test_mempalace_plugin_loader.py tests/plugins/test_mempalace_e2e.py -q

Notes

  • branch pushed from fork: Jessica-lol/hermes-agent
  • excluded unrelated local change in scripts/whatsapp-bridge/package-lock.json from this PR

- add modular MemPalace provider implementation and tool bindings
- add plugin tests for foundation, loader, module layout, and e2e flows
- deduplicate overlapping memory prefetch lines across providers
- document setup and reviewer quick start in plugin README
@eugeneyvt

eugeneyvt commented Apr 18, 2026

Copy link
Copy Markdown

I’ve been working on a parallel MemPalace integration in my fork and wanted to share the main architectural difference here before opening a duplicate PR.

This PR appears to take a Hermes-owned provider approach, while my branch takes a lower-coupling approach:

  • MCP-first for runtime tools
  • CLI/hook-backed for lifecycle operations like session-start, stop, precompact, mining, and wake-up

The main reason for that design is maintenance. Hermes can rely on MemPalace’s public runtime surfaces instead of depending as much on MemPalace’s internal Python implementation. In practice that should make upstream MemPalace changes easier to absorb over time, as long as the CLI/MCP contracts stay stable.

I’m not saying that makes this PR wrong. The tradeoff seems more like:

  • native Hermes-side implementation: easier to review and control entirely inside Hermes
  • CLI/MCP contract integration: less coupling and potentially lower long-term maintenance cost

A few concrete things I ended up solving in that branch:

  • programmatic MCP calls from the provider layer
  • curated tool exposure with config overrides
  • scoped wake-up behavior so first-turn wake-up uses the Hermes conversation wing instead of global palace wake-up

Branch for reference:

https://github.com/eugeneyvt/hermes-agent/tree/integrate/mempalace-20260418

@alt-glitch alt-glitch added type/feature New feature or request comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers P3 Low — cosmetic, nice to have labels Apr 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Note: prior MemPalace PRs exist \u2014 #5671 (open), #9761 (open), #6871 (closed). Please coordinate with those authors to avoid redundant review effort.

@Motokiyo

Copy link
Copy Markdown

Bug report: embedding dimension mismatch on existing MemPalace collections (cross-posted from #5671 per @alt-glitch's triage)

Hi @Jessica-lol — I cherry-picked this PR locally to integrate Hermes with my existing MemPalace install and hit a blocker worth reporting. TL;DR: the plugin opens the Chroma collection without specifying an embedding_function, so it falls back to all-MiniLM-L6-v2 (384-dim) and breaks any pre-existing collection created by the upstream MemPalace CLI/MCP server (which uses OllamaEmbeddingFunction(model="bge-m3"), 1024-dim).

Symptom (every conversational turn):

WARNING plugins.memory.mempalace: MemPalace sync_turn failed:
Collection expecting embedding with dimension of 1024, got 384

Result: _do_sync_turn silently fails, no drawer is filed automatically. Explicit mempalace_kg_add and mempalace_diary_write tool calls still work because they go through the upstream MemPalace package (which loads its own EF), so the bug is invisible until you tail the logs and notice that automatic turn persistence is dead.

Root causeplugins/memory/mempalace/__init__.py (the get_or_create_collection call has no embedding_function argument):

self._chroma_client = chromadb.PersistentClient(path=str(self._palace_path))
self._chroma_collection = self._chroma_client.get_or_create_collection(
    "mempalace_drawers"   # no embedding_function → Chroma default = MiniLM 384
)

Compare with the upstream MemPalace package (mempalace/mcp_server.py, miner.py, searcher.py):

from mempalace.ollama_embedding import OllamaEmbeddingFunction
ef = OllamaEmbeddingFunction(model="bge-m3")
col = client.get_or_create_collection("mempalace_drawers", embedding_function=ef)

Fix — 3-line patch:

from mempalace.ollama_embedding import OllamaEmbeddingFunction
ef = OllamaEmbeddingFunction(model="bge-m3")
self._chroma_client = chromadb.PersistentClient(path=str(self._palace_path))
self._chroma_collection = self._chroma_client.get_or_create_collection(
    "mempalace_drawers",
    embedding_function=ef,
)

Tested locally (Hermes gateway + MemPalace 1024-dim collection, ~68k drawers). After patch + gateway restart, a Telegram turn produced the expected log:

INFO plugins.memory.mempalace: MemPalace: filed drawer wing_general/test-post-fix/hall_events (drawer_wing_general_test-post-fix_ffbeed9fef0bb6d4b38c4004)

Sync now works as designed. Same bug exists in #9761.

Open design question — should the plugin fall back gracefully if mempalace.ollama_embedding import fails (e.g. user runs Hermes without the upstream MemPalace package installed, or without Ollama running)? The current ChromaDB init is already wrapped in try/except that disables palace writes — adding a try-import for the EF and logging "MemPalace EF unavailable, palace writes disabled" would keep the same shape. Happy to open a follow-up PR if you'd like.

@Motokiyo

Motokiyo commented Apr 26, 2026

Copy link
Copy Markdown

Quick follow-up after 5+ hours in prod (Telegram Précepteur, Hermes gateway on macOS launchd):

Stability: zero sync_turn failed warnings since gateway restart at 14:14:59 UTC+2 — the warnings I was still seeing at 03:17–05:10 turned out to be the patched code not yet propagated (Python module cache; the gateway needed a full restart, not just a config reload).

Volume validated:

  • ~30+ drawers auto-filed across 3 wings (wing_ai_research, wing_eiffel, wing_dev)
  • ~10 conversational turns with explicit tool calls (mempalace_kg_add, mempalace_kg_query, mempalace_search) all succeed
  • 1024-dim collection (~68k pre-existing drawers from upstream MemPalace CLI) reads and writes consistently

On the open design question (graceful fallback if mempalace.ollama_embedding import fails) — I'd lean toward yes, mirroring the existing ChromaDB try/except. Sketch:

try:
    import chromadb
    try:
        from mempalace.ollama_embedding import OllamaEmbeddingFunction
        ef = OllamaEmbeddingFunction(model="bge-m3")
    except (ImportError, Exception) as ef_exc:
        logger.warning(
            "MemPalace: OllamaEmbeddingFunction unavailable (%s) — palace writes disabled "
            "(install upstream `mempalace` package and ensure Ollama is running with bge-m3 pulled)",
            ef_exc,
        )
        ef = None
    self._chroma_client = chromadb.PersistentClient(path=str(self._palace_path))
    if ef is not None:
        self._chroma_collection = self._chroma_client.get_or_create_collection(
            "mempalace_drawers", embedding_function=ef,
        )
    else:
        self._chroma_collection = None
except Exception as e:
    ...

This way pip install hermes-agent works without mempalace + Ollama (writes disabled, plugin still loads), while the full stack gets the right EF. Happy to send a follow-up PR if you want.

@hmcp22

hmcp22 commented May 13, 2026

Copy link
Copy Markdown

@Jessica-lol heads up — Bartok9 picked this up and has a working salvage at #21017 with the import guard and test skip markers applied. might be worth coordinating there since that one's past review and just blocked on pre-existing CI noise

Bartok9 added a commit to Bartok9/hermes-agent that referenced this pull request May 15, 2026
…alace package

- Wrap top-level mempalace imports in try/except at module level so the
  plugin loads gracefully when mempalace is not installed
- Add _MEMPALACE_AVAILABLE guard; initialize() raises RuntimeError with
  clear install instructions when mempalace is absent
- Add @_requires_mempalace skip markers on tests that need the actual
  mempalace package (test_initialize_*) — CI passes without mempalace
  installed, tests run when it is available

Salvage of NousResearch#12203 by @Jessica-lol — rebased onto current main.
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Thanks for the contribution!

Per the updated CONTRIBUTING.md, new memory providers are no longer accepted as in-tree additions to plugins/memory/:

Memory Providers: CLOSED to new in-tree additions
PRs adding to plugins/memory/ will be closed. Publish as standalone plugin into ~/.hermes/plugins/ or via pip entry point. Must implement MemoryProvider ABC (sync_turn, prefetch, shutdown, optional post_setup).

Closing this in line with that policy. The path forward is to publish it as a standalone plugin so users can install it directly without touching the Hermes source tree. Once it's published, a small docs PR adding it to the Community plugins section of the README is welcome.

Sorry for the bump — appreciate the time you put into this.

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

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have tool/memory Memory tool and memory providers type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants