Skip to content

feat(cli): wire export_palace() to CLI as mempalace export#1086

Open
jphein wants to merge 3 commits intoMemPalace:developfrom
jphein:pr/cmd-export-cli
Open

feat(cli): wire export_palace() to CLI as mempalace export#1086
jphein wants to merge 3 commits intoMemPalace:developfrom
jphein:pr/cmd-export-cli

Conversation

@jphein
Copy link
Copy Markdown
Collaborator

@jphein jphein commented Apr 21, 2026

Summary

The exporter module with export_palace() ships today but has no CLI entry — users have to from mempalace.exporter import export_palace to use it. This PR adds a thin cmd_export wrapper + argparse block + dispatch entry so users can:

mempalace export -o ~/my-palace-backup

What's in the diff

  • mempalace/cli.pycmd_export function (~10 lines) + argparse subparser + dispatch entry (+24/-0)
  • tests/test_cli.py — 2 tests mirroring the cmd_status pattern: default palace from MempalaceConfig, custom palace with expanduser() (+26/-0)

No behavior changes elsewhere

cmd_export calls export_palace(palace_path, output_dir) exactly as Python-API callers do today — same module, same function, same args. Just reachable from a shell.

Test plan

  • mempalace export --help renders a clean help block
  • ruff check clean on cli.py and tests/test_cli.py
  • pytest tests/test_cli.py — 44 passed (2 new)

No new runtime dependencies. No config changes.

Copilot AI review requested due to automatic review settings April 21, 2026 23:25
jphein added a commit to jphein/mempalace that referenced this pull request Apr 21, 2026
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.

Pull request overview

Adds a CLI entry point for the existing exporter.export_palace() API so users can run palace exports from the shell via mempalace export.

Changes:

  • Add cmd_export() wrapper that resolves palace/output paths and calls export_palace().
  • Register an export subcommand in argparse and add it to the command dispatch table.
  • Add unit tests for cmd_export using the same mocking pattern as other CLI command tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
mempalace/cli.py Introduces cmd_export, wires export into argparse, and adds it to the dispatch map.
tests/test_cli.py Adds tests covering cmd_export default/custom path resolution and invocation of export_palace.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_cli.py
Comment on lines +54 to +78
@patch("mempalace.cli.MempalaceConfig")
def test_cmd_export_default_palace(mock_config_cls):
mock_config_cls.return_value.palace_path = "/fake/palace"
args = argparse.Namespace(palace=None, output="/tmp/export_target")
mock_exporter = MagicMock()
with patch.dict("sys.modules", {"mempalace.exporter": mock_exporter}):
cmd_export(args)
mock_exporter.export_palace.assert_called_once_with(
palace_path="/fake/palace", output_dir="/tmp/export_target"
)


@patch("mempalace.cli.MempalaceConfig")
def test_cmd_export_custom_palace_and_output(mock_config_cls):
args = argparse.Namespace(palace="~/my_palace", output="~/export_target")
mock_exporter = MagicMock()
with patch.dict("sys.modules", {"mempalace.exporter": mock_exporter}):
cmd_export(args)
import os

expected_palace = os.path.expanduser("~/my_palace")
expected_output = os.path.expanduser("~/export_target")
mock_exporter.export_palace.assert_called_once_with(
palace_path=expected_palace, output_dir=expected_output
)
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

There are main() dispatch tests for most CLI subcommands in this file, but none for the newly added export subcommand. Adding a test_main_export_dispatches (e.g., patching sys.argv to ['mempalace','export','-o','/tmp/out'] and asserting cmd_export is called) would ensure the argparse wiring + dispatch entry stays covered.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done in b905d04 — added test_main_export_dispatches mirroring the test_main_<cmd>_dispatches pattern of the other subcommands. Asserts both -o /tmp/out short-form and --output /tmp/out2 long-form, and that the global --palace flag reaches cmd_export via args.palace. 45/45 tests/test_cli.py pass.

Comment thread mempalace/cli.py
Comment on lines +718 to +722
# export
p_export = sub.add_parser(
"export",
help="Export palace as browsable markdown files (one file per room)",
)
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

The CLI help epilog is built from this module's top-level docstring (via __doc__), but the "Commands" list at the top of cli.py doesn't mention the new export command. Please update the docstring (and any examples) so mempalace --help documents mempalace export consistently with the other commands.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in b905d04. Added mempalace export -o <dir> to the Commands list and an mempalace export -o ~/Desktop/palace-export line to Examples — the epilog is built from __doc__ so this is what mempalace --help actually shows users.

Comment thread mempalace/cli.py Outdated
p_export.add_argument(
"-o", "--output", required=True, help="Output directory for markdown files"
)
p_export.add_argument("--palace", help="Path to palace directory (default: from config)")
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

--palace is already a global option on the root parser (usable as mempalace --palace … <command>), but export also defines its own --palace flag. This makes export behave differently from the other subcommands and can be confusing. Consider removing the per-subcommand --palace and relying on the global flag for consistency, or (if you want the post-subcommand form) standardize it across subcommands.

Suggested change
p_export.add_argument("--palace", help="Path to palace directory (default: from config)")

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch — fixed in b905d04 by dropping the per-subcommand --palace from p_export. The global --palace flag on the root parser already reaches cmd_export via args.palace (verified by the new dispatch test), matching the pattern of mine/search/mcp/status/wake-up. Usage is now mempalace --palace <p> export -o <d>.

jphein added a commit to jphein/mempalace that referenced this pull request Apr 22, 2026
…emPalace#1094 rows + MemPalace#659 CLEAN

Three open-PR rows were missing (cmd_export, cmd_purge, None-metadata
boundary). MemPalace#659 moved from MERGEABLE to CLEAN after the 2026-04-22
rebase onto current develop cleared its stale-merge blocker. Open-PR
count: 4 → 7.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@igorls igorls added enhancement New feature or request area/cli CLI commands labels Apr 24, 2026
jphein added 2 commits April 24, 2026 14:25
The `exporter` module with `export_palace()` ships today but has no CLI
entry — users have to `from mempalace.exporter import export_palace` to
use it. This PR adds a thin `cmd_export` wrapper + argparse block +
dispatch entry so:

    mempalace export -o ~/my-palace-backup

calls the existing exporter. No behavior changes anywhere else.

- `cli.py`: +12 lines for `cmd_export` + +12 for argparse/dispatch
- `tests/test_cli.py`: +2 tests mirroring `cmd_status`'s pattern
  (default palace from config; custom palace + expanduser)
- Total diff: +24/-0 in `cli.py`, +26/-0 in `tests/test_cli.py`

Smoke-tested: `mempalace export --help` renders clean. Full `test_cli.py`
suite: 44 passed.
…xport

Three corrections from @copilot-pull-request-reviewer:

1. **Top-of-module docstring updated.** The "Commands" list and
   "Examples" block now mention `mempalace export -o <dir>` so
   `mempalace --help` documents the new subcommand consistently
   with the rest. The epilog is built from this docstring via
   __doc__, so a stale list is what users actually see.

2. **Per-subcommand --palace dropped.** Other subcommands
   (mine/search/mcp/status/wake-up) all read args.palace from
   the global root-parser flag; export's local duplicate broke
   that consistency. Replaced with a comment explaining the
   choice. Usage is now `mempalace --palace <p> export -o <d>`.
   (cmd_export already accesses args.palace, which still resolves
   correctly via the global parser.)

3. **test_main_export_dispatches added.** Mirrors the
   test_main_<cmd>_dispatches pattern used for every other
   subcommand. Asserts the argparse wiring delivers both the
   global --palace and the -o/--output to cmd_export, and locks
   in that --output (long form) is also accepted.

45/45 tests pass in tests/test_cli.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli CLI commands enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants