Skip to content

refactor(cli): migrate get_collection call sites to PalaceRef kwargs (#37)#91

Merged
jphein merged 2 commits into
mainfrom
refactor/cli-py-get-collection-kwargs-37
May 16, 2026
Merged

refactor(cli): migrate get_collection call sites to PalaceRef kwargs (#37)#91
jphein merged 2 commits into
mainfrom
refactor/cli-py-get-collection-kwargs-37

Conversation

@jphein

@jphein jphein commented May 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes the cli.py audit from #37. Four sites still used the legacy positional `backend.get_collection(palace_path, collection_name)` signature from before PR #21's MemPalace#665 cherry-pick. They worked silently because `ChromaBackend.get_collection` accepts both forms via `_normalize_get_collection_args` (intentional compat shim — issue #37 question (a)).

Migrated all four sites to the new kwargs form for consistency with `palace.get_collection` and `searcher.search_memories`. Future backends (postgres, test fakes) won't necessarily carry the same positional surface; converging now keeps the migration runway short when one tightens.

Sites touched

  • `cmd_purge` match-count probe
  • `cmd_drawers` list-by-wing/room probe
  • `cmd_repair_status` drawer count
  • `cmd_extract` palace open

Test plan

  • One test asserted the old positional call shape — updated to match
  • `bun test tests/test_cli.py` → 75/75 pass

Closes #37

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 16, 2026 15:16

Copilot AI left a comment

Copy link
Copy Markdown

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.

…37)

Four call sites in cli.py still used the legacy positional signature
``backend.get_collection(palace_path, collection_name)`` from before
PR #21's MemPalace#665 cherry-pick. They worked because ChromaBackend.get_collection
accepts both forms via ``_normalize_get_collection_args`` (the answer to
issue #37 question (a): the compat shim exists and is intentional).

Migrating for consistency with palace.get_collection and
searcher.search_memories, which already use the new shape. Future
backends (postgres, in-memory test fakes) may not carry the same
positional compat surface; converging the codebase on the kwargs form
keeps the migration runway short when one of those tightens.

Touched sites (cli.py):
  - cmd_purge match-count probe (line ~1192)
  - cmd_drawers list-by-wing/room probe (~1282)
  - cmd_repair_status drawer count (~1492)
  - cmd_extract palace open (~1645)

One test asserted the old positional call shape — updated to match the
new PalaceRef call shape. 75/75 cli tests pass.

Closes #37

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@jphein jphein force-pushed the refactor/cli-py-get-collection-kwargs-37 branch from cc34c55 to bda5cf5 Compare May 16, 2026 16:04
@jphein jphein merged commit c245d87 into main May 16, 2026
7 checks passed
@jphein jphein deleted the refactor/cli-py-get-collection-kwargs-37 branch May 16, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate cli.py call sites still using old positional ChromaBackend().get_collection(palace_path, ...) signature

2 participants