feat: silent stop hook — direct save instead of blocking MCP calls - #556
feat: silent stop hook — direct save instead of blocking MCP calls#556jphein wants to merge 30 commits into
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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
Documents fork relationship, key files, development workflow, fork changes, upstream PRs, and integration details. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Replace hardcoded venv path with a resolution chain: 1. MEMPALACE_PYTHON env var (user override) 2. Plugin root's own venv (development installs) 3. System python3 (pip/pipx installs) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Covers basic CRUD, filtering, pagination, negative offset clamping, not-found errors, and no-op update detection. Addresses review comment on PR MemPalace#493 requesting coverage for the new drawer tools. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Single source of truth for the limit ceiling (100) so operators can adjust without hunting through multiple clamp sites. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
|
This is a substantial quality-of-life improvement for agent-heavy workflows — and the architectural split between stop (silent) vs precompact (blocking) is the right model. What works well: The The Concern worth flagging: The
The change to PRECOMPACT_BLOCK_REASON text (more explicit enumerated instructions) is also an improvement — the numbered list format tends to produce more reliable tool-call sequencing than prose. We use a similar pattern in our integration — a lightweight checkpoint on every cycle, with thoroughness reserved for context boundary events. Glad to see this upstream. LGTM. |
There was a problem hiding this comment.
Pull request overview
This PR shifts the stop hook to perform a non-blocking checkpoint save (via direct Python calls) and adds supporting improvements across mining/search/MCP tooling and export.
Changes:
- Stop hook now saves a diary checkpoint directly (no blocking MCP tool-call instructions) and emits a concise notification.
- Mining/search improvements: room detection and chunking updates, optional similarity filtering in programmatic search, and parallel file preparation with batched upserts.
- New/expanded capabilities: markdown exporter + CLI command, additional MCP drawer CRUD/list tools, and broader test coverage.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| mempalace/hooks_cli.py | Implements direct diary checkpoint saving, notifications, and transcript ingest; stop hook no longer blocks. |
| tests/test_hooks_cli.py | Updates stop hook expectations and adds tests for extracting recent transcript messages. |
| mempalace/mcp_server.py | Adds metadata pagination/cache, search filtering, and drawer get/list/update tools + schemas. |
| tests/test_mcp_server.py | Adds coverage for new drawer read/list/update MCP tools. |
| mempalace/searcher.py | Introduces shared build_where_filter() and adds distance-threshold filtering to search_memories(). |
| mempalace/layers.py | Reuses build_where_filter() and caps L1 scanning volume. |
| mempalace/palace.py | Adds epsilon mtime comparison and bulk mined-metadata prefetch helper. |
| mempalace/miner.py | Improves room detection and chunking; adds parallel file prep path with batched upserts and a workers option. |
| tests/test_miner.py | Adds unit tests for detect_room() and chunk_text() behavior and constants. |
| mempalace/convo_miner.py | Batches drawer writes per file to reduce ChromaDB call overhead. |
| mempalace/config.py | Adds configurable chunk sizing/overlap/min size defaults. |
| mempalace/exporter.py | New exporter to write palace contents as a browsable markdown tree. |
| tests/test_exporter.py | Adds tests for exporter structure and markdown content. |
| mempalace/cli.py | Adds export command and plumbs --workers through mine. |
| tests/test_cli.py | Updates mine CLI tests to include workers argument. |
| mempalace/knowledge_graph.py | Changes default query direction to include both incoming and outgoing relationships. |
| mempalace/entity_detector.py | Expands STOPWORDS to reduce false-positive entity detection for common technical terms. |
| CLAUDE.md | Adds repo-specific dev notes and documents fork deltas. |
| .claude-plugin/plugin.json | Bumps plugin version to 3.1.0. |
| .codex-plugin/plugin.json | Bumps plugin version to 3.1.0. |
| .claude-plugin/hooks/mempal-stop-hook.sh | Improves Python resolution (env override/venv/system) for hook execution. |
| .claude-plugin/hooks/mempal-precompact-hook.sh | Improves Python resolution (env override/venv/system) for hook execution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Auto-ingest transcript before compaction (so conversation lands in palace) | ||
| if transcript_path: | ||
| _ingest_transcript(transcript_path) | ||
|
|
There was a problem hiding this comment.
This comment implies the transcript will “land in palace” before compaction, but _ingest_transcript() spawns an async subprocess and returns immediately. Either run the ingest synchronously here (if that guarantee is important) or adjust the comment/behavior to make it clear it’s only best-effort/background.
| def bulk_check_mined(collection) -> dict[str, float]: | ||
| """Pre-fetch source_file/source_mtime pairs for all documents in the collection. | ||
|
|
||
| Returns a dict mapping source_file -> source_mtime (as float) for every | ||
| document that has both fields. Callers can check membership and compare | ||
| mtimes locally instead of issuing one ChromaDB query per file. | ||
|
|
||
| Fetches the full collection in paginated batches (like palace_graph.py) | ||
| since a WHERE-IN filter on thousands of paths is not supported by ChromaDB. | ||
| """ | ||
| mined: dict[str, float] = {} | ||
| try: | ||
| total = collection.count() | ||
| offset = 0 | ||
| while offset < total: | ||
| batch = collection.get(limit=1000, offset=offset, include=["metadatas"]) |
There was a problem hiding this comment.
bulk_check_mined() paginates over the entire collection, which can be very expensive when the palace contains many wings/projects. Since mine() uses this only to skip re-mining for a single project, consider allowing an optional where-filter (e.g., wing) so callers can restrict the metadata scan to the relevant subset and avoid O(total_palace_drawers) work per mine run.
| def cmd_export(args): | ||
| from .exporter import export_palace | ||
|
|
||
| palace_path = os.path.expanduser(args.palace) if args.palace else MempalaceConfig().palace_path | ||
| output_dir = os.path.expanduser(args.output) | ||
|
|
||
| print(f"\n{'=' * 55}") | ||
| print(" MemPalace Export") | ||
| print(f"{'=' * 55}\n") | ||
| print(f" Palace: {palace_path}") | ||
| print(f" Output: {output_dir}\n") | ||
|
|
||
| export_palace(palace_path=palace_path, output_dir=output_dir) | ||
|
|
||
| print(f"\n{'=' * 55}\n") | ||
|
|
There was a problem hiding this comment.
The PR title/description focuses on the silent stop hook change, but this PR also introduces additional user-facing features/behavior (e.g., new export command, exporter module/tests, miner parallelism/workers, new MCP drawer tools). Please either update the PR description to cover these changes (and their motivation/risk) or split them into separate PRs to keep review scope manageable.
| ```bash | ||
| source venv/bin/activate | ||
| python -m pytest tests/ -x -q # run tests (573 expected) | ||
| mempalace status # check palace state | ||
| mempalace search "query" # test search | ||
| python -m mempalace.mcp_server # run MCP server standalone | ||
| ``` |
There was a problem hiding this comment.
This testing note says “573 expected”, but the PR description/test plan indicates 576 tests passing and the suite has grown (new exporter/miner/MCP tests). Please update the expected test count here to match current reality so it remains useful during development.
| - **Claude Code plugin**: installed at user scope via marketplace | ||
| - **MCP server**: global user scope — available in all projects | ||
| - **Stop hook**: fires every 15 messages, saves to palace via MCP tools + auto-ingests transcript | ||
| - **PreCompact hook**: emergency save before context compaction | ||
|
|
There was a problem hiding this comment.
This section says the stop hook “saves to palace via MCP tools”, but the stop hook now writes the diary checkpoint directly via Python (calling tool_diary_write without blocking) and only the precompact hook still instructs MCP tool usage. Please update this description so it matches the current hook behavior.
| # Save diary checkpoint directly (no MCP, no terminal clutter) | ||
| if transcript_path: | ||
| _save_diary_direct(transcript_path, session_id) | ||
| _ingest_transcript(transcript_path) | ||
|
|
There was a problem hiding this comment.
hook_stop() updates the last-save marker before these direct-save calls run. If _save_diary_direct() fails (or transcript ingest fails), the session may still be marked as “saved” and the next checkpoint will be skipped until the next interval. Consider having _save_diary_direct() return success/failure and only advancing the last-save point on success (or track last_attempt separately).
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
|
Superseded by consolidated PR — all changes plus review feedback incorporated into a single clean PR from main. |
Summary
✦ MemPalace Checkpoint saved — N messages archived) replaces 3-4 expanded MCP tool blocksnotify-sendfor users not watching the terminalFixes #554
Changes
hooks_cli.py: Add_extract_recent_messages(),_save_diary_direct(),_notify()helpers;hook_stopsaves silently and never blockstests/test_hooks_cli.py: Updated stop hook tests, added 3 new tests for_extract_recent_messagesnotify-sendfails silently on non-Linux)Test plan
pytest tests/ -v— 576 passedruff check— cleannotify-send🤖 Generated with Claude Code