feat: palace maintenance — junk filter, purge, status pagination, repair (#478 #586 #587 #581) - #627
feat: palace maintenance — junk filter, purge, status pagination, repair (#478 #586 #587 #581)#627jphein wants to merge 1 commit into
Conversation
…air fix, chromadb upgrade - Add SKIP_PATTERNS and JUNK_FILE_SIZE to filter noise during mining (MemPalace#587) - Fix dry-run room=None crash with fallback to 'general' (MemPalace#586) - Fix status 10K ceiling with paginated offset loop (MemPalace#478) - Add mempalace purge command for targeted drawer removal - Fix repair to nuke palace dir for clean HNSW index rebuild - Upgrade chromadb pin to >=1.5.4 (MemPalace#581) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds “palace maintenance” improvements to MemPalace: filtering low-value junk files during mining, fixing --dry-run room counting, making status reporting accurate for large palaces via pagination, introducing a selective purge command that rebuilds the index, improving repair by fully nuking/recreating the palace directory, and upgrading ChromaDB for Python 3.13/3.14 compatibility.
Changes:
- Upgrade ChromaDB dependency to
chromadb>=1.5.4,<2. - Add scan-time junk filtering (suffix patterns + 500KB “junk” size cutoff) and fix dry-run room counting.
- Implement paginated status counting and add
mempalace purgeplus a saferrepairrebuild strategy.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
pyproject.toml |
Bumps ChromaDB to >=1.5.4,<2 to restore compatibility with newer Python versions. |
mempalace/miner.py |
Adds junk file filters, fixes dry-run room=None counting, and paginates status metadata fetching. |
mempalace/cli.py |
Introduces purge subcommand and updates repair to rebuild by deleting/recreating the palace directory and re-inserting drawers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| palace_path = palace_path.rstrip(os.sep) | ||
| print(" Rebuilding palace...") | ||
| shutil.rmtree(palace_path) | ||
| os.makedirs(palace_path, mode=0o700) |
There was a problem hiding this comment.
palace_path.rstrip(os.sep) can turn dangerous inputs into unexpected targets (e.g., Windows C:\\ → C:), and then shutil.rmtree(palace_path) will delete the wrong directory or fail in surprising ways. Consider normalizing with os.path.normpath()/Path(...).resolve() and explicitly refusing root/drive-root/empty paths before deleting; also surface the resolved path in the confirmation prompt.
| palace_path = palace_path.rstrip(os.sep) | ||
| backup_path = palace_path + ".backup" | ||
| if os.path.exists(backup_path): | ||
| shutil.rmtree(backup_path) | ||
| print(f" Backing up to {backup_path}...") |
There was a problem hiding this comment.
palace_path = palace_path.rstrip(os.sep) is now used on the destructive delete path later in this function; on Windows this can collapse C:\\ into C: (drive-relative) and on some inputs can produce an empty/unsafe path. Use a safer normalization (e.g., os.path.normpath / Path.resolve) and add guards to prevent deleting anything that isn’t a dedicated palace directory.
| # Count matching drawers | ||
| match_ids = set() | ||
| offset = 0 | ||
| while True: | ||
| batch = col.get(limit=10000, offset=offset, where=where, include=[]) | ||
| if not batch["ids"]: | ||
| break | ||
| match_ids.update(batch["ids"]) | ||
| offset += len(batch["ids"]) |
There was a problem hiding this comment.
Building match_ids as an in-memory set of every matching drawer can be very large for big palaces and can double the total scan work (first to collect IDs, then to re-scan everything to extract keepers). Since you already fetch metadatas during extraction, you can decide “purge vs keep” based on wing/room directly and avoid storing all IDs (and potentially avoid the first pass by counting matches from metadatas only).
| @@ -585,6 +727,7 @@ def main(): | |||
| "wake-up": cmd_wakeup, | |||
| "repair": cmd_repair, | |||
| "migrate": cmd_migrate, | |||
| "purge": cmd_purge, | |||
| "status": cmd_status, | |||
| } | |||
| dispatch[args.command](args) | |||
There was a problem hiding this comment.
The new purge subcommand is added to argparse/dispatch, but there’s no corresponding CLI test coverage (e.g., main dispatches purge, and cmd_purge resolves palace path / validates args / prompts as expected). Adding a minimal tests/test_cli.py case similar to the existing test_main_*_dispatches tests would prevent regressions.
| @@ -524,7 +545,11 @@ def scan_project( | |||
| continue | |||
| # Skip files exceeding size limit | |||
| try: | |||
| if filepath.stat().st_size > MAX_FILE_SIZE: | |||
| fsize = filepath.stat().st_size | |||
| if fsize > MAX_FILE_SIZE: | |||
| continue | |||
| # Skip suspiciously large text files (SQL dumps, generated JSON, etc.) | |||
| if not force_include and fsize > JUNK_FILE_SIZE: | |||
| continue | |||
There was a problem hiding this comment.
scan_project() now skips additional suffix-based junk files (SKIP_PATTERNS) and skips files over JUNK_FILE_SIZE unless explicitly included. There are already detailed scan_project tests in tests/test_miner.py, but none cover these new filters or the include-override behavior; adding tests would lock in the intended behavior and avoid surprises/regressions.
Summary
Palace maintenance features split from #562 per maintainer request.
SKIP_PATTERNS(suffix-based) andJUNK_FILE_SIZE(500KB) filter minified JS/CSS, bundles, lock files during mining (feat: default-exclude runtime-state files + per-file drawer cap to prevent ingestion noise #587)room_counts[room or "general"]on both sequential and concurrent paths (bug: mempalace mine --dry-run crashes with TypeError on files assigned room=None #586)limit=10000(BUG: Status/taxonomy MCP tools silently truncate at 10,000 drawers — wrong counts for large palaces #478)mempalace purge --wing X --room Yextracts keepers, nukes palace, rebuilds with clean HNSW indexTest plan
🤖 Generated with Claude Code