fix: expand ~ in split command directory argument by brookewhatnall · Pull Request #361 · MemPalace/mempalace

brookewhatnall · 2026-04-09T11:13:24Z

Problem

mempalace split ~/some/dir silently finds no files because Path("~/foo") does not expand the tilde — it treats ~ as a literal directory name.

Root cause

In split_mega_files.py:

# before
src_dir = Path(args.source) if args.source else LUMI_DIR

Path does not call os.path.expanduser automatically, so any ~-prefixed path is never resolved to the real home directory.

Fix

Two one-line changes:

mempalace/split_mega_files.py (root cause):

src_dir = Path(args.source).expanduser().resolve() if args.source else LUMI_DIR

mempalace/cli.py (defensive fix at the CLI boundary):

argv = ["--source", str(Path(args.dir).expanduser().resolve())]

.resolve() is added alongside .expanduser() so relative paths (e.g. mempalace split .) also work correctly.

Reproduction

# Before — finds nothing, no error
mempalace split ~/Desktop/transcripts

# After — works as expected
mempalace split ~/Desktop/transcripts

Path("~/foo") does not expand tilde on its own, causing `mempalace split ~/some/dir` to silently find no files. Fix by calling .expanduser().resolve() in both places the path is constructed: cmd_split in cli.py (defensive, at the CLI boundary) and main() in split_mega_files.py (the root cause). Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

bgauryy · 2026-04-09T17:59:36Z

PR Review: fix: expand ~ in split command directory argument

Executive Summary

Aspect	Value
PR Goal	Fix silent failure where `mempalace split ~/dir` finds no files because `Path("~/foo")` treats `~` as a literal directory name
Files Changed	2
Risk Level	LOW — bug fix for path handling, no auth/API/data changes
Review Mode	Quick
Review Effort	1 — trivial
Recommendation	REQUEST_CHANGES

Affected Areas: mempalace/cli.py (cmd_split), mempalace/split_mega_files.py (main)

Business Impact: Users passing ~-prefixed paths to mempalace split will now see their files processed instead of a silent no-op

Flow Changes: cmd_split now normalizes the --source path before forwarding to split_mega_files.main(). split_mega_files.main() also normalizes on its own side for direct-invocation safety.

Ratings

Aspect	Score
Correctness	3/5
Security	5/5
Performance	5/5
Maintainability	4/5

PR Health

Has clear description — well-structured Problem / Root Cause / Fix sections
References ticket/issue (if applicable) — N/A
Appropriate size — 3 additions, 2 deletions
Has relevant tests (if applicable) — no test coverage for the fix

Guidelines Compliance

Source	Rule	Status
(none loaded)	—	N/A

High Priority Issues

(Must fix before merge)

[Bug] #1: Incomplete fix — `--output-dir` argument is not tilde-expanded

Location: mempalace/cli.py:133 + mempalace/split_mega_files.py:197 | Confidence: HIGH

The PR fixes tilde expansion for --source but the same bug exists for --output-dir. In cli.py, args.output_dir is passed through without expansion. In split_mega_files.py, the value is wrapped with Path(output_dir) which does not call expanduser(). Running mempalace split ~/chats --output-dir ~/output will expand the source correctly but the output directory will remain a literal ~/output, causing a FileNotFoundError or writing to the wrong location.

cli.py — apply same treatment:

     if args.output_dir:
-        argv += ["--output-dir", args.output_dir]
+        argv += ["--output-dir", str(Path(args.output_dir).expanduser().resolve())]

split_mega_files.py — defensive expansion:

-    out_dir = Path(output_dir) if output_dir else path.parent
+    out_dir = Path(output_dir).expanduser().resolve() if output_dir else path.parent

[Bug] #2: Incomplete fix — `--file` argument is not tilde-expanded

Location: mempalace/split_mega_files.py:270 | Confidence: HIGH

When split_mega_files.py is invoked directly with --file ~/some/file.txt, the path is wrapped as Path(args.file) without expanduser(). Same class of bug being fixed for --source.

     if args.file:
-        files = [Path(args.file)]
+        files = [Path(args.file).expanduser().resolve()]

Note: --file is not exposed through cli.py's cmd_split wrapper, so this only affects direct invocation of split_mega_files.py. Still worth fixing for consistency since the PR's intent is to fix tilde handling in this module.

Medium Priority Issues

(Should fix, not blocking)

(None)

Low Priority Issues

(Nice to have)

[Code Quality] #3: Redundant double expansion of `--source` path

Location: mempalace/cli.py:135 + mempalace/split_mega_files.py:260 | Confidence: MED

The tilde is expanded in both cli.py (before passing to split_mega_files) and in split_mega_files.py (after parsing argv). When called through cli.py, the second expansion is a harmless no-op. This is a reasonable defense-in-depth pattern since split_mega_files.py can be invoked directly — but it would be clearer to add a brief code comment in split_mega_files.py explaining why it re-expands (e.g., "also expand here for standalone invocation").

[Code Quality] #4: Style inconsistency with existing tilde expansion pattern

Location: mempalace/cli.py:130 vs mempalace/cli.py:135 | Confidence: MED

The nearby cmd_wakeup function uses os.path.expanduser(args.palace) while this PR uses Path(args.dir).expanduser().resolve(). Both work correctly but create two different tilde-expansion patterns in the same file. The pathlib approach in this PR is more modern and also adds resolve() for symlink normalization — consider updating cmd_wakeup to match in a follow-up.

Flow Impact Analysis

User CLI invocation
    │
    ▼
cmd_split(args)          ← cli.py: expands args.dir (PR fix)
    │                       but NOT args.output_dir (bug #1)
    │ builds argv list
    ▼
split_mega_files.main()  ← split_mega_files.py: re-expands --source (PR fix)
    │                       but NOT --output-dir (bug #1)
    │                       and NOT --file (bug #2)
    ▼
src_dir.glob("*.txt")    ← operates on resolved Path — works correctly after fix
    │
    ▼
split_file(f, output_dir) ← output_dir may still contain unexpanded ~
    │
    ▼
out_dir / name            ← Path join with literal ~ → wrong path

Created by Octocode MCP https://octocode.ai

web3guru888

Clean fix — tilde expansion is one of those silent failures that's really hard to debug when it bites you. Good call resolving in both the CLI entry point and the underlying split_mega_files.py.

One thought: you might want to add a quick test case (test_split_tilde_expansion or similar) to prevent regression, since the Path("~/foo") behavior varies subtly across platforms.

🔭 Reviewed as part of the MemPalace-AGI integration project — autonomous research with perfect memory. Community interaction updates are posted regularly on the dashboard.

bensig

Code review + security audit clean.

…n conflicts Merge upstream/develop: MemPalace#177, MemPalace#361, MemPalace#449, MemPalace#450 (benchmarks, tilde expand, duplicate cache vars, test fixture cleanup). Merge upstream/main: MemPalace#666 (z3tz3r0's block reason disambiguation). Conflict resolution: keep our scoped "For THIS save" phrasing instead of MemPalace#666's absolute "Do NOT write to auto-memory" — both memory systems are used in tandem. Also drop "AAAK-compressed" from diary instructions since diary_write is plain text, not AAAK dialect. Co-Authored-By: Claude Opus 4.6 <[email protected]>

…iterdir OSError) - Expand ~ in split command dir/output-dir args (upstream MemPalace#361) - Use yield+close in KG test fixture to prevent SQLite leaks (MemPalace#450) - Use char/3.8 token estimate for consistency (MemPalace#609) - Handle OSError from iterdir on broken symlinks/reparse points (MemPalace#558) Closes DOT-488.

Port of upstream dc14347 (PR MemPalace#361) — Path("~/foo") does not expand the tilde on its own, causing `mempalace split ~/some/dir` to silently find no files because src_dir.glob("*.txt") never resolves into the user's home. Fix in both places the path is constructed: cmd_split in cli.py (defensive, at the CLI boundary, before handing argv to the inner parser) and main() in split_mega_files.py (the root cause when invoked directly). Upstream: MemPalace@dc14347 Co-authored-by: Brooke Whatnall <[email protected]> Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

web3guru888 reviewed Apr 10, 2026

View reviewed changes

bensig changed the base branch from main to develop April 11, 2026 22:22

bensig requested review from bensig and milla-jovovich as code owners April 11, 2026 22:22

bensig approved these changes Apr 12, 2026

View reviewed changes

bensig merged commit dc14347 into MemPalace:develop Apr 12, 2026

ichoosetoaccept mentioned this pull request Apr 13, 2026

fix: port small upstream fixes (split ~, KG fixture, token estimate, iterdir OSError) detailobsessed/mempalace#51

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: expand ~ in split command directory argument#361

fix: expand ~ in split command directory argument#361
bensig merged 1 commit intoMemPalace:developfrom
brookewhatnall:fix/split-dir-tilde-expansion

brookewhatnall commented Apr 9, 2026

Uh oh!

bgauryy commented Apr 9, 2026

Uh oh!

web3guru888 left a comment

Uh oh!

bensig left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

brookewhatnall commented Apr 9, 2026

Problem

Root cause

Fix

Reproduction

Uh oh!

bgauryy commented Apr 9, 2026

PR Review: fix: expand ~ in split command directory argument

Executive Summary

Ratings

PR Health

Guidelines Compliance

High Priority Issues

[Bug] #1: Incomplete fix — --output-dir argument is not tilde-expanded

[Bug] #2: Incomplete fix — --file argument is not tilde-expanded

Medium Priority Issues

Low Priority Issues

[Code Quality] #3: Redundant double expansion of --source path

[Code Quality] #4: Style inconsistency with existing tilde expansion pattern

Flow Impact Analysis

Uh oh!

web3guru888 left a comment

Choose a reason for hiding this comment

Uh oh!

bensig left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[Bug] #1: Incomplete fix — `--output-dir` argument is not tilde-expanded

[Bug] #2: Incomplete fix — `--file` argument is not tilde-expanded

[Code Quality] #3: Redundant double expansion of `--source` path