Skip to content

fix(cli): cmd_compress writes to mempalace_closets (#1244)#1319

Merged
igorls merged 3 commits intodevelopfrom
fix/1244-closets-collection-name
May 3, 2026
Merged

fix(cli): cmd_compress writes to mempalace_closets (#1244)#1319
igorls merged 3 commits intodevelopfrom
fix/1244-closets-collection-name

Conversation

@igorls
Copy link
Copy Markdown
Member

@igorls igorls commented May 3, 2026

Closes #1244.

Problem

cmd_compress (in mempalace/cli.py) was writing AAAK-compressed drawers to a collection named mempalace_compressed, but every read path in the codebase reads from mempalace_closets:

  • mempalace/palace.py::get_closets_collection (the canonical accessor)
  • mempalace/searcher.py (hybrid search)
  • mempalace/repair.py (HNSW capacity check)
  • mempalace/closet_llm.py, mempalace/diary_ingest.py, mempalace/miner.py

So for any user running mempalace compress to backfill the closet/index layer on a non-mined palace, the compressed output was silently written to a dead collection nothing else opens — invisible to search, never scanned by the LLM.

Fix

Update the writer in cmd_compress to use mempalace_closets (matching the readers), instead of renaming the 15+ readers. Rationale:

  • "Closets" is the user-visible feature name in docs, the public API (get_closets_collection), README claims tests, and architectural diagrams.
  • The compressed AAAK strings are exactly what closets are conceptually — compact pointers an LLM scans to locate the right drawer.
  • Single-site change; no public API churn.

Tests

  • Updated test_cmd_compress_stores_results to assert the collection name passed to get_or_create_collection is mempalace_closets.
  • Added test_cmd_compress_output_readable_via_get_closets_collection — end-to-end regression with a real ChromaBackend: seeds a drawer, runs cmd_compress, then reads back through the same get_closets_collection helper that palace.py / searcher.py use. Fails on the old code, passes on the fix.

Test plan

  • python -m pytest tests/ -v --ignore=tests/benchmarks -x -k "compress or closet" — 94 passed
  • python -m pytest tests/test_cli.py -v — 54 passed
  • ruff check . — All checks passed
  • ruff format --check — only pre-existing format drift on develop, no new issues introduced by this PR

Generated with Claude Code

…ad them (#1244)

`cmd_compress` was writing AAAK-compressed drawers to a `mempalace_compressed`
collection, but every read path (`palace.get_closets_collection`,
`searcher.py`, `repair.py`) reads from `mempalace_closets`. Result: for
non-mined palaces (or any palace where the user ran `mempalace compress`
expecting to backfill the closet/index layer), the compressed output was
silently invisible — written to a collection nothing else opens.

Fix the writer rather than renaming the readers: "closets" is the
user-visible feature name baked into the public API
(`get_closets_collection`), the searcher hybrid path, repair/HNSW
diagnostics, and docs. Renaming the readers would churn 15+ call sites
and the README for no benefit. The compressed AAAK strings are exactly
what closets are conceptually — compact pointers scanned by an LLM to
locate the right drawer — so they belong in `mempalace_closets`.

Tests:
- Update `test_cmd_compress_stores_results` to assert the collection
  name passed to `get_or_create_collection` is `mempalace_closets`.
- Add `test_cmd_compress_output_readable_via_get_closets_collection`:
  end-to-end with a real ChromaBackend, seed a drawer, run cmd_compress,
  then read back via the same `get_closets_collection` helper that
  palace.py / searcher use. Regression test for the wrong-collection
  bug.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@igorls igorls requested a review from milla-jovovich as a code owner May 3, 2026 01:54
Copilot AI review requested due to automatic review settings May 3, 2026 01:54
@igorls igorls requested a review from bensig as a code owner May 3, 2026 01:54
@igorls igorls added this to the v3.3.5 milestone May 3, 2026
@igorls igorls added bug Something isn't working area/cli CLI commands storage labels May 3, 2026
igorls added 2 commits May 2, 2026 22:58
CI requires ruff format --check on the whole touched file. Pre-existing drift, no logic change.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@igorls igorls merged commit 339924a into develop May 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli CLI commands bug Something isn't working storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Closets not backfilled for non-mined palaces — mempalace compress writes to wrong collection (_compressed vs _closets)

2 participants