feat: MCP tools, hooks, export, search threshold + fixes#493
feat: MCP tools, hooks, export, search threshold + fixes#493jphein wants to merge 47 commits intoMemPalace:mainfrom
Conversation
Float equality on mtime fails due to JSON round-trip precision loss, causing every file to be re-mined on each run. Use epsilon < 0.01. Also adds bulk_check_mined() for fetching all source_file/mtime pairs in paginated batches — turns 25K individual DB queries into ~5 fetches. Fixes MemPalace#475 Co-Authored-By: Claude Opus 4.6 <[email protected]>
…decls - Clamp tool_search limit to [1, 100] to prevent memory exhaustion - Replace hardcoded limit=10000 in status/taxonomy tools with paginated _fetch_all_metadata() helper (matches palace_graph.py pattern) - Remove duplicate _client_cache/_collection_cache declarations Fixes MemPalace#477, MemPalace#478, MemPalace#479 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Accumulate all chunks for a file into lists, then issue a single collection.upsert() (miner) or collection.add() (convo_miner) call. Reduces 125K-375K individual DB round-trips to ~25K batched calls. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Prevents false positives like Handler, Node, Service, Manager, Client being flagged as project/person entities in code-heavy directories. Fixes MemPalace#476 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Adds min_similarity parameter (L2 distance cutoff) to search_memories() and MCP tool_search (default 1.5). Filters out clearly irrelevant results instead of always returning top-N regardless of quality. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Updated STOP_BLOCK_REASON to instruct AI to use mempalace_diary_write and mempalace_add_drawer instead of generic "memory system" - Updated PRECOMPACT_BLOCK_REASON with same MCP tool instructions - Added _ingest_transcript() to mine Claude Code JSONL transcripts into the palace automatically on stop/precompact triggers - Transcript goes into a "sessions" wing via convo_miner Co-Authored-By: Claude Opus 4.6 <[email protected]>
Documents fork relationship, key files, development workflow, fork changes, upstream PRs, and integration details. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Mining: - Added _prepare_file() for thread-safe file processing (read/chunk/route) - mine() now supports --workers flag (default: min(8, cpu_count)) - Concurrent path: bulk mtime pre-fetch, parallel _prepare_file(), serialized ChromaDB writes in batches of 100. Sequential path unchanged (workers=1). Room routing: - Priority 1: exact folder match only (no substring) - Priority 2: exact filename match only - Content scan increased from 2KB to 5KB (full file if <10KB) - Keyword scoring uses word-boundary regex instead of substring count - Added 13 unit tests for detect_room covering all priority paths Co-Authored-By: Claude Opus 4.6 <[email protected]>
New exporter.py: paginates all drawers, groups by wing/room, writes browsable markdown tree with index.md table of contents. Each drawer becomes a blockquoted section with metadata table. Usage: mempalace export -o ./palace-export Also fixes test_cli.py for new --workers arg on mine subparser. Co-Authored-By: Claude Opus 4.6 <[email protected]>
… cache I7: Three new MCP tools — get_drawer, list_drawers (paginated), update_drawer (with WAL audit logging and input sanitization). I8: WAL file chmod(0o600) now only runs on file creation instead of every write call. I9: 5-second TTL metadata cache for status/wings/taxonomy tools. Eliminates redundant full-palace pagination when tools are called in quick succession. Co-Authored-By: Claude Opus 4.6 <[email protected]>
I6: Chunk size/overlap/min now configurable via ~/.mempalace/config.json instead of hardcoded constants. Wired through mine() → process_file() → chunk_text(). I11: Layer1.generate() capped at MAX_SCAN=2000 drawers (was unbounded). Reduces wake-up from 250+ ChromaDB round-trips to 4 max. I12: Extracted _build_where_filter() helper in searcher.py, replaced 5 duplicate where-filter blocks across searcher.py and layers.py. Co-Authored-By: Claude Opus 4.6 <[email protected]>
I10: 10 unit tests for chunk_text() covering boundaries, overlap, indices, empty/whitespace input, content preservation. I13: KG query_entity default direction aligned from "outgoing" to "both" to match the MCP schema default. I14: Plugin versions synced to 3.1.0 in both .claude-plugin/ and .codex-plugin/. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Float equality on mtime fails due to JSON round-trip precision loss, causing every file to be re-mined on each run. Use epsilon < 0.01. Also adds bulk_check_mined() for fetching all source_file/mtime pairs in paginated batches — turns 25K individual DB queries into ~5 fetches. Fixes MemPalace#475 Co-Authored-By: Claude Opus 4.6 <[email protected]>
…decls - Clamp tool_search limit to [1, 100] to prevent memory exhaustion - Replace hardcoded limit=10000 in status/taxonomy tools with paginated _fetch_all_metadata() helper (matches palace_graph.py pattern) - Remove duplicate _client_cache/_collection_cache declarations Fixes MemPalace#477, MemPalace#478, MemPalace#479 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Adds min_similarity parameter (L2 distance cutoff) to search_memories() and MCP tool_search (default 1.5). Filters out clearly irrelevant results instead of always returning top-N regardless of quality. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Updated STOP_BLOCK_REASON to instruct AI to use mempalace_diary_write and mempalace_add_drawer instead of generic "memory system" - Updated PRECOMPACT_BLOCK_REASON with same MCP tool instructions - Added _ingest_transcript() to mine Claude Code JSONL transcripts into the palace automatically on stop/precompact triggers - Transcript goes into a "sessions" wing via convo_miner Co-Authored-By: Claude Opus 4.6 <[email protected]>
Documents fork relationship, key files, development workflow, fork changes, upstream PRs, and integration details. Co-Authored-By: Claude Opus 4.6 <[email protected]>
New exporter.py: paginates all drawers, groups by wing/room, writes browsable markdown tree with index.md table of contents. Each drawer becomes a blockquoted section with metadata table. Usage: mempalace export -o ./palace-export Also fixes test_cli.py for new --workers arg on mine subparser. Co-Authored-By: Claude Opus 4.6 <[email protected]>
… cache I7: Three new MCP tools — get_drawer, list_drawers (paginated), update_drawer (with WAL audit logging and input sanitization). I8: WAL file chmod(0o600) now only runs on file creation instead of every write call. I9: 5-second TTL metadata cache for status/wings/taxonomy tools. Eliminates redundant full-palace pagination when tools are called in quick succession. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds MCP server enhancements, safer/cheaper metadata fetching, a new markdown exporter, and hook updates to better support MemPalace workflows (plus a few bug fixes around dedup/search limits).
Changes:
- Add markdown export command (
mempalace export) and exporter implementation + tests. - Extend MCP server tools (drawer CRUD helpers, pagination/caching, similarity threshold) and hook instructions/transcript ingest.
- Improve dedup correctness (epsilon mtime compare) and add a bulk prefetch helper for mined-file checks.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
mempalace/cli.py |
Adds export command + forwards workers arg into mining. |
mempalace/exporter.py |
New exporter that writes a wing/room markdown tree + index. |
tests/test_exporter.py |
New tests validating exporter structure/content. |
mempalace/mcp_server.py |
Pagination/cache helpers, search threshold, new MCP tools (get/list/update drawer), WAL chmod tweak. |
mempalace/searcher.py |
Adds min_similarity/distance filtering and returns distance in results. |
mempalace/hooks_cli.py |
Updates stop/precompact instructions + transcript auto-ingest logic. |
mempalace/palace.py |
Epsilon mtime comparison + bulk_check_mined() helper. |
tests/test_cli.py |
Updates mining CLI tests to include new workers arg. |
CLAUDE.md |
Adds project context and workflow notes for Claude Code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from .miner import mine | ||
|
|
||
| mine( | ||
| project_dir=args.dir, | ||
| palace_path=palace_path, | ||
| wing_override=args.wing, | ||
| agent=args.agent, | ||
| limit=args.limit, | ||
| dry_run=args.dry_run, | ||
| respect_gitignore=not args.no_gitignore, | ||
| include_ignored=include_ignored, | ||
| workers=args.workers, | ||
| ) |
There was a problem hiding this comment.
This is already working — mine() has the workers parameter (line 661). The PR diff may not show it since it was added in the concurrent mining commit.
| try: | ||
| palace_path = MempalaceConfig().palace_path | ||
| except Exception: | ||
| return |
There was a problem hiding this comment.
Fixed — changed to MempalaceConfig() (validate config loads) instead of assigning to unused palace_path.
|
|
||
| import chromadb | ||
| import yaml |
There was a problem hiding this comment.
Fixed — removed unused chromadb import from test_exporter.py.
| for wing in sorted(grouped): | ||
| wing_dir = os.path.join(output_dir, wing) | ||
| os.makedirs(wing_dir, exist_ok=True) | ||
| wing_drawer_count = 0 | ||
|
|
||
| rooms = grouped[wing] | ||
| for room in sorted(rooms): | ||
| drawers = rooms[room] | ||
| room_path = os.path.join(wing_dir, f"{room}.md") | ||
|
|
There was a problem hiding this comment.
Fixed — added _safe_path_component() that strips /\:*?"<>| and leading dots from wing/room metadata before using as directory/file names. Applied to both wing dirs and room filenames in the streaming exporter.
| "similarity": round(1 - dist, 3), | ||
| "distance": round(dist, 4), | ||
| } | ||
| ) | ||
|
|
||
| # Filter out results exceeding the distance threshold. | ||
| # ChromaDB default L2: lower distance = more similar. | ||
| # min_similarity=0.0 (default) disables filtering for backwards compat. | ||
| if min_similarity > 0.0: | ||
| hits = [h for h in hits if h["distance"] <= min_similarity] | ||
|
|
There was a problem hiding this comment.
Fixed — distance filtering now happens on the raw float before rounding, then the display values are rounded for output. Avoids borderline precision loss.
| "query": {"type": "string", "description": "What to search for"}, | ||
| "limit": {"type": "integer", "description": "Max results (default 5)"}, | ||
| "wing": {"type": "string", "description": "Filter by wing (optional)"}, |
There was a problem hiding this comment.
Fixed — added minimum: 1, maximum: 100 to the search limit schema and list_drawers limit/offset schemas.
| def tool_get_drawer(drawer_id: str): | ||
| """Fetch a single drawer by ID. Returns full content and metadata.""" | ||
| col = _get_collection() | ||
| if not col: | ||
| return _no_palace() | ||
| try: | ||
| result = col.get(ids=[drawer_id], include=["documents", "metadatas"]) | ||
| if not result["ids"]: | ||
| return {"error": f"Drawer not found: {drawer_id}"} | ||
| meta = result["metadatas"][0] | ||
| doc = result["documents"][0] | ||
| return { | ||
| "drawer_id": drawer_id, | ||
| "content": doc, | ||
| "wing": meta.get("wing", ""), | ||
| "room": meta.get("room", ""), | ||
| "metadata": meta, | ||
| } | ||
| except Exception as e: | ||
| return {"error": str(e)} | ||
|
|
||
|
|
||
| def tool_list_drawers(wing: str = None, room: str = None, limit: int = 20, offset: int = 0): | ||
| """List drawers with pagination. Optional wing/room filter.""" | ||
| limit = max(1, min(limit, 100)) | ||
| col = _get_collection() | ||
| if not col: | ||
| return _no_palace() | ||
| try: | ||
| where = None | ||
| conditions = [] | ||
| if wing: | ||
| conditions.append({"wing": wing}) | ||
| if room: | ||
| conditions.append({"room": room}) | ||
| if len(conditions) == 1: | ||
| where = conditions[0] | ||
| elif len(conditions) > 1: | ||
| where = {"$and": conditions} | ||
|
|
||
| kwargs = {"include": ["documents", "metadatas"], "limit": limit, "offset": offset} | ||
| if where: | ||
| kwargs["where"] = where | ||
| result = col.get(**kwargs) | ||
|
|
||
| drawers = [] | ||
| for i, did in enumerate(result["ids"]): | ||
| meta = result["metadatas"][i] | ||
| doc = result["documents"][i] | ||
| drawers.append( | ||
| { | ||
| "drawer_id": did, | ||
| "wing": meta.get("wing", ""), | ||
| "room": meta.get("room", ""), | ||
| "content_preview": doc[:200] + "..." if len(doc) > 200 else doc, | ||
| } | ||
| ) | ||
| return { | ||
| "drawers": drawers, | ||
| "count": len(drawers), | ||
| "offset": offset, | ||
| "limit": limit, | ||
| } | ||
| except Exception as e: | ||
| return {"error": str(e)} | ||
|
|
||
|
|
||
| def tool_update_drawer(drawer_id: str, content: str = None, wing: str = None, room: str = None): | ||
| """Update an existing drawer's content and/or metadata.""" | ||
| col = _get_collection() |
There was a problem hiding this comment.
Noted — will add MCP tool tests for get/list/update_drawer in a follow-up commit. The existing test_mcp_server.py covers the other tools; these three need coverage for basic CRUD and pagination.
| ```bash | ||
| source venv/bin/activate | ||
| python -m pytest tests/ -x -q # run tests (534 expected) | ||
| mempalace status # check palace state |
There was a problem hiding this comment.
Fixed — updated CLAUDE.md from 534 to 562 expected tests.
| def search_memories( | ||
| query: str, palace_path: str, wing: str = None, room: str = None, n_results: int = 5 | ||
| query: str, | ||
| palace_path: str, | ||
| wing: str = None, | ||
| room: str = None, | ||
| n_results: int = 5, | ||
| min_similarity: float = 0.0, | ||
| ) -> dict: |
There was a problem hiding this comment.
Agreed the naming is confusing. For now I've added comprehensive L2 distance documentation in the function docstring and MCP schema description. A full rename to max_distance would be a breaking change — happy to do it in a separate PR with a deprecation alias if you'd prefer.
| "limit": { | ||
| "type": "integer", | ||
| "description": "Max results per page (default 20, max 100)", | ||
| }, | ||
| "offset": { | ||
| "type": "integer", | ||
| "description": "Offset for pagination (default 0)", | ||
| }, |
There was a problem hiding this comment.
Fixed — added minimum/maximum bounds to list_drawers schema: limit min 1 max 100, offset min 0.
web3guru888
left a comment
web3guru888
left a comment
There was a problem hiding this comment.
Solid PR, jphein — this addresses several gaps I've hit in production use.
Similarity threshold — This is the feature I've wanted most. We implemented min_similarity filtering internally (threshold 0.72 for our use case) and it dramatically cuts noise on broad queries. One critical thing to document: is min_similarity filtering against L2 distance (higher = less similar) or cosine similarity (higher = more similar)? ChromaDB's default embedding function returns L2 by default, so a threshold of 1.5 is excluding scores above 1.5, not including. That's counterintuitive for users coming from cosine-similarity workflows and should be called out clearly in CLAUDE.md and/or the docstring. Easy fix but prevents a lot of confusion.
Hooks rewrite — Going through MCP tools (mempalace_diary_write, mempalace_add_drawer) instead of direct Python calls is the right architectural choice. Keeps hooks decoupled from internals and benefits automatically from validation, pagination, etc. The auto-ingest of Claude Code JSONL transcripts on compaction is clever — turns a normally lossy event into a structured memory commit.
New MCP tools — list_drawers with wing/room pagination fills a real gap. Right now you're flying blind unless you already know what to search for. get_drawer by ID is handy for follow-up fetches after a search result. Both straightforward, good additions.
export_palace at scale — Does this stream or paginate? If a palace has 10K–50K drawers (which is realistic for long-running integrations), a single export call risks timeout at the MCP layer. I'd suggest either: (a) returning a paginated export with page/page_size params, or (b) writing to a temp file on disk and returning the path. The exporter module design looks right for (b) if export_palace just wraps a file write.
Conflict with #484 — Both this PR and #484 touch mcp_server.py. Worth checking if there's overlap before merge — specifically around the search tool signature changes.
Mtime epsilon fix is a clean correctness improvement. No concerns there.
Overall: 👍 on direction. Threshold docs + export scaling are the two things worth addressing before merge.
Address web3guru888's review feedback across PRs MemPalace#492 and MemPalace#493: - palace.py: remove unused filepaths param from bulk_check_mined(), replace bare except with logger.warning for partial fetch visibility - miner.py: wrap future.result() in try/except so one file failure doesn't abort the entire concurrent mining run - exporter.py: stream drawers in batches instead of loading entire palace into memory — keeps memory bounded for large palaces - searcher.py: document min_similarity as L2 distance (not cosine) with typical range guidance in docstring Co-Authored-By: Claude Opus 4.6 <[email protected]>
# Conflicts: # mempalace/exporter.py # mempalace/palace.py
- Rename _build_where_filter → build_where_filter (public cross-module API) - Add float() cast + TypeError/ValueError handling in _is_already_mined - Add chunk_overlap validation (must be >= 0 and < chunk_size) - Batch convo_miner adds to 100 docs per call (avoid SQLite limits) - Stream miner writes as futures complete (bounded memory) - Remove unused palace_path in hooks_cli - Remove unused chromadb import in test_exporter - Sanitize wing/room as path components in exporter (prevent traversal) - Filter on raw distance before rounding in searcher - Clamp negative offset in tool_list_drawers - No-op early return + cache invalidation in tool_update_drawer - Add min/max schema bounds for search limit and list_drawers limit/offset - Update CLAUDE.md test count (534 → 562) - Improve chunk coverage test with position-unique tokens Co-Authored-By: Claude Opus 4.6 <[email protected]>
Stop and precompact hooks used bare `python3` which resolves to system Python in sessions outside the memorypalace project directory, causing `No module named mempalace` errors. Now uses the venv's Python with fallback to system python3. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Tested this branch ( ✓ MCP server — correctly fixed
✗ CLI
|
|
Thanks @psaghelyi — really valuable field test on a real 14.9K palace. Good catch on Your patch is correct and clean: replace I'd suggest @jphein roll this in before merge. Shipping the PR with MCP server fixed but |
The CLI `mempalace status` command still capped at 10,000 drawers after this PR's MCP server fix, because miner.status() retained the original `col.get(limit=10000, include=["metadatas"])` call. On palaces larger than 10K, it printed a wrong total and silently dropped every wing past the cutoff (tested on a 14,902-drawer / 17-wing palace: CLI reported "10000 drawers" and listed only 11 wings). Replace the single bounded call with the same paginated offset loop the new `_fetch_all_metadata()` helper uses in mcp_server.py (1,000-item batches), and report the true total via `col.count()`. Closes the CLI half of MemPalace#478.
|
@jphein — rolled the One hunk, mirrors the pagination pattern from your new |
Clean fix — mirrors the paginated offset loop from _fetch_all_metadata() in mcp_server.py. Verified by psaghelyi against 14,902-drawer palace. Closes the CLI half of MemPalace#478.
|
@psaghelyi — great catch and thanks for the thorough field test against your 14.9K palace. Merged your PR (#1) into Also merged latest |
|
@jphein — the cross-branch coordination with psaghelyi is exactly the right approach. The Merging |
Co-Authored-By: Claude Opus 4.6 <[email protected]>
…king MCP Stop hook no longer blocks Claude with MCP tool call instructions every 15 messages. Instead it saves a diary checkpoint directly via the Python API and shows a single-line terminal notification + desktop toast. Fixes MemPalace#554 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add hooks.silent_save and hooks.desktop_toast to config.json, readable via new mempalace_hook_settings MCP tool (get/set). Stop hook checks config to decide between silent direct save vs legacy blocking MCP. Restore STOP_BLOCK_REASON for legacy mode. Toast is opt-in via config. Co-Authored-By: Claude Opus 4.6 <[email protected]>
stderr from hook subprocesses doesn't reach the Claude Code terminal. Block with a one-liner notification after the direct save completes — save already happened, Claude just continues. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Claude Code shows all hook blocks as "Stop hook error:" with no info
level available. Return {} for truly invisible saves.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Hook saves directly, then blocks asking Claude to call mempalace_checkpoint_ack — a zero-param tool returning one line like "✦ Journal entry filed — 30 messages tucked into drawers". Replaces both the verbose MCP diary/drawer calls and the invisible silent mode with a single clean terminal line. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Claude Code labels all hook blocks as "Stop hook error:" with no way to customize. Go fully silent instead — save happens invisibly. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Stop hook now outputs {"systemMessage": "✦ N messages filed away"} which
Claude Code renders as a visible one-line terminal notification — no MCP
tool call needed. Also renames checkpoint_ack → memories_filed_away and
fixes MCP server to silently ignore all notifications/ methods per spec.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Copilot review caught that hook_stop() updated the last-save marker before _save_diary_direct() ran. If save failed, the marker would still advance and skip the next checkpoint. Move marker write after save confirms success. Also updates CLAUDE.md test count and hook docs. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Stop hook now extracts topic keywords from recent messages and displays them in the notification: "✦ 10 memories woven into the palace — hooks, notifications, MCP". Stopword filtering keeps only distinctive terms. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Rename min_similarity → max_distance (searcher + MCP schema), keep backwards compat alias in MCP tool handler - Fix ingest comment accuracy (async/best-effort, not guaranteed) - Add notification protocol tests (all notifications/* return None, unknown methods without id return None) - 578 tests passing Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Superseded by consolidated PR — all changes plus review feedback incorporated into a single clean PR from main. |
Summary
abs() < 0.01) instead of float==(fixes BUG: Float mtime comparison breaks file deduplication — every file re-mined on each run #475)min_similarityparameter in search, default 1.5 L2 distance in MCPmempalace_diary_write,mempalace_add_drawer), auto-ingest Claude Code JSONL transcriptsget_drawer,list_drawers(paginated),update_drawer(with WAL audit)mempalace export -o <dir>renders palace as browsable markdown filesSupersedes #483 and #484 (includes those fixes plus new features).
Test plan
mempalace export -o /tmp/testproduces correct markdown treeget_drawer,list_drawers,update_drawer) via MCP protocol🤖 Generated with Claude Code