fix: Windows compatibility — room routing, Unicode output, test cleanup

[roip]

Summary

• Room routing fix: detect_room() used bidirectional substring matching (part in room_name or room_name in part) which caused short directory names like ml/ to incorrectly match room names containing that substring (e.g. �isualizeml). Changed to exact match against room names and keywords.
• Unicode console fix: Added mempalace/compat.py with _stdout_supports() helper that detects whether stdout can encode Unicode characters. On Windows with cp1252 console encoding, print() crashes on characters like checkmarks and bullets. Progress indicators now fall back to ASCII (+ / # / .) when needed.
• Test cleanup (Windows): ChromaDB holds file locks on data_level0.bin on Windows. Tests that create PersistentClient now properly release references and use _force_cleanup() with retry logic before shutil.rmtree.
• Regression test: Added est_detect_room_exact_match_no_substring to prevent the substring routing bug from recurring.

Test plan

• Full test suite passes on Windows (Python 3.13.6): 118 passed, 0 failed
• Ruff lint: all checks passed
• Manual smoke test: mempalace mine --dry-run routes files correctly
• Regression test covers the exact scenario that triggered the bug

[bgauryy]

PR Review: fix: Windows compatibility — room routing, Unicode output, test cleanup

Executive Summary

Aspect	Value
PR Goal	Fix Windows compatibility: replace hardcoded Unicode with encoding-aware fallbacks, tighten room routing from substring to exact match, add .mpignore support, harden test cleanup
Files Changed	7
Risk Level	MEDIUM — Room routing semantic change affects all users; .mpignore always-on is a new behavioral contract
Review Mode	Full
Review Effort	3/5
Recommendation	COMMENT

Affected Areas: mempalace/compat.py (new), mempalace/miner.py (room routing + .mpignore), mempalace/convo_miner.py, mempalace/split_mega_files.py, mempalace/entity_detector.py (Unicode consumers), tests/test_miner.py, tests/test_convo_miner.py

Business Impact: Room routing change may reassign files to different rooms for existing users who relied on substring folder/filename matching. .mpignore introduces a new always-on ignore mechanism that filters files even when --no-gitignore is set.

Flow Changes: detect_room() Priority 1 and 2 now require exact path-segment/stem match against room name or keywords (was bidirectional substring). scan_project() now loads .mpignore matchers unconditionally, merged with .gitignore matchers when respect_gitignore=True, or used alone when respect_gitignore=False.

Ratings

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

PR Health

• Has clear description
• References ticket/issue (if applicable) — no issue linked
• Appropriate size (195 additions, 43 deletions across 7 files)
• Has relevant tests (new tests for .mpignore and detect_room exact match)

---

Medium Priority Issues

(Should fix or explicitly acknowledge before merge)

[Flow Impact / Bug] #1: detect_room exact match may silently break existing room routing

Location: mempalace/miner.py:315-329 | Confidence: HIGH

Priority 1 (folder matching) changed from any(part == c or c in part or part in c for c in candidates) to any(part == c for c in candidates). Priority 2 (filename matching) changed from room["name"].lower() in filename or filename in room["name"].lower() to any(filename == c for c in candidates). This means a file at backend_notes/readme.md no longer matches room backend via substring — only exact folder name or keyword matches work. The regression test covers the ml/ → visualizeml false positive, but the tightening also removes legitimate fuzzy matches that users may depend on.

Callers affected: process_file() at miner.py:~499 and mine() at miner.py:~654 — both pass through to detect_room. room_detector_local.py indirectly affected via scan_project.

  # Priority 1: consider a middle ground — exact OR "part is a whole-word
  # prefix of candidate" to avoid substring false positives while keeping
  # reasonable fuzzy matches
  path_parts = relative.replace("\\", "/").split("/")
  for part in path_parts[:-1]:
      for room in rooms:
          candidates = [room["name"].lower()] + [k.lower() for k in room.get("keywords", [])]
-         if any(part == c for c in candidates):
+         if any(part == c or part in c.split("-") for c in candidates):
              return room["name"]

Alternatively, document this as an intentional breaking change and advise users to add keywords in their room config for previously fuzzy-matched folders.

---

[Security / Error Handling] #2: _stdout_supports uncaught TypeError on exotic stdout

Location: mempalace/compat.py:7-12 | Confidence: MED

Only UnicodeEncodeError and LookupError are caught. If sys.stdout.encoding is not a valid string (e.g., mocked, replaced, or a broken pipe scenario), chars.encode(encoding) raises TypeError, which would crash the import of compat.py and cascade to every module importing CHECKMARK.

  def _stdout_supports(chars: str) -> bool:
      encoding = getattr(sys.stdout, "encoding", "ascii") or "ascii"
      try:
          chars.encode(encoding)
          return True
-     except (UnicodeEncodeError, LookupError):
+     except (UnicodeEncodeError, LookupError, TypeError):
          return False

---

[Architecture / Flow] #3: .mpignore always-on not surfaced to users when --no-gitignore

Location: mempalace/miner.py:565-569 | Confidence: HIGH

When respect_gitignore=False, .mpignore matchers are still active: all_matchers = active_mp_matchers. This is intentional per the test test_mpignore_active_even_with_no_gitignore, but the user-facing output from mine() only says .gitignore: DISABLED — giving the impression that all ignore files are off. Callers of scan_project(..., respect_gitignore=False) (e.g., room_detector_local.py) may also be surprised.

  if not respect_gitignore:
-     print("  .gitignore: DISABLED")
+     print("  .gitignore: DISABLED  (.mpignore still active)")

---

Low Priority Issues

(Nice to have)

[Quality / Duplicate] #4: _force_cleanup duplicated across two test files

Location: tests/test_miner.py:11-23 and tests/test_convo_miner.py:9-21 | Confidence: HIGH

Identical helper with minor whitespace drift. tests/conftest.py exists and is the natural home for shared test utilities. Duplication risks divergence over time.

- # In tests/test_miner.py and tests/test_convo_miner.py:
- def _force_cleanup(path): ...
+ # In tests/conftest.py:
+ def force_cleanup_tempdir(path):
+     """Best-effort temp dir removal; ChromaDB may hold file locks on Windows."""
+     try:
+         shutil.rmtree(path)
+     except PermissionError:
+         if sys.platform == "win32":
+             gc.collect()
+             import time
+             time.sleep(0.5)
+             shutil.rmtree(path, ignore_errors=True)
+         else:
+             raise

Then import from conftest in both test modules.

---

[Quality] #5: Inline import inconsistency in entity_detector

Location: mempalace/entity_detector.py:707 | Confidence: MED

_stdout_supports is imported inline inside _print_entity_list(), while convo_miner.py, miner.py, and split_mega_files.py all use top-level imports from compat. Unless there is a circular import concern, this should be hoisted to module top for consistency.

+ from .compat import _stdout_supports
+
  def _print_entity_list(entities: list, label: str):
-     from .compat import _stdout_supports
      print(f"\n  {label}:")

---

[Quality] #6: is_gitignored docstring drift

Location: mempalace/miner.py:214-221 | Confidence: HIGH

is_gitignored() now receives matchers from both .gitignore and .mpignore, but its docstring and name only reference .gitignore. Consider renaming to is_ignored() or updating the docstring to reflect that matchers may originate from either ignore file.

---

[Quality] #7: _prune_matchers could use is_relative_to for readability

Location: mempalace/miner.py:528-533 | Confidence: MED

root == m.base_dir or m.base_dir in root.parents is correct but less readable than root.is_relative_to(m.base_dir) (available since Python 3.9). If the project's minimum Python version allows it, consider the more expressive form.

---

Flow Impact Analysis

scan_project()
├── loads .gitignore matchers (when respect_gitignore=True)
├── loads .mpignore matchers (ALWAYS)
├── combines into all_matchers
├── filters dirs & files
└── returns file list
    ├── mine() → process_file() → detect_room()  [exact match now]
    ├── detect_rooms_local() via room_detector_local.py
    └── tests via scanned_files() helper

detect_room(filepath, content, rooms, project_path)
├── Priority 1: folder path == room name/keyword  [was substring, now exact]
├── Priority 2: filename stem == room name/keyword [was substring, now exact]
├── Priority 3: keyword scoring from content       [unchanged]
└── Priority 4: fallback to "general"              [unchanged]

Blast radius: 2 direct call sites for detect_room in miner.py. scan_project is called from mine(), detect_rooms_local(), and test helpers. .mpignore always-on affects all scan_project consumers regardless of respect_gitignore flag.

---

Created by Octocode MCP https://octocode.ai

[web3guru888]

Solid Windows compat work. The room routing bug fix is the most important change here — the substring matching issue would affect everyone, not just Windows.

Room routing fix (important for all platforms):
The old part in room_name or room_name in part was genuinely broken: a folder named ml/ matching visualizeml is a real bug. The new part == c or part in c.split("-") fixes it while still allowing hyphen-component matching (e.g., backend matches keyword backend-api). The regression test test_detect_room_exact_match_no_substring is well-structured — covers the bug, the fix, and the edge case.

Unicode console compat:
_stdout_supports() checking encoding before printing is the right approach. Falling back to ASCII (+, #, .) is pragmatic. This is a common pain point on Windows cp1252 consoles.

Test cleanup:
force_cleanup_tempdir() with gc.collect() + retry is a known workaround for ChromaDB's file lock issue on Windows. Using try/finally blocks instead of bare shutil.rmtree at the end makes the test suite more robust.

⚠️ Bug in split_mega_files.py:
There's a merge/rebase artifact in this file — the diff shows out_path.write_text("".join(chunk), encoding="utf-8") and the _CHECKMARK print line duplicated three extra times in the main() function, inserted into completely wrong positions (inside else branches for the file iteration, the backup section, and the summary section). This will cause SyntaxError or incorrect behavior at runtime. Looks like the checkmark replacement got applied to the wrong hunks. The fix in the split_file() function itself (line ~225) is correct — just the main() function needs cleanup.

.mpignore in SKIP_FILES:
I notice you also added .mpignore to SKIP_FILES — heads-up that #379 introduces .mempalaceignore for the same purpose. Might want to coordinate on naming.

The room routing fix and Unicode compat are clean and well-tested. Just needs the split_mega_files.py artifact fixed before merge.

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

[roip]

Thanks for the review @web3guru888 — good catch on the split_mega_files.py merge artifact. Fixed in 7024af9: the three spurious write_text/_CHECKMARK insertions in main() are removed. The correct usage in split_file() (line ~227) is untouched.

Re .mpignore vs .mempalaceignore (#379): aware of the overlap. We went with .mpignore for brevity (same pattern as .npmignore, .dockerignore). Happy to support both filenames or defer to maintainer preference on the canonical name — it's a one-line change in load_ignore_matcher().

[igorls]

Hi, thanks for the contribution.

This PR has merge conflicts with develop, and the branch has not been updated in over 7 days, which puts it before our most recent release. The conflicts are likely against work that landed in that release.

Could you rebase onto develop so we can take another look?

If this change is no longer relevant, feel free to close the PR.

(This message is part of a periodic backlog pass, sent to all open PRs that match this state.)