fix windows unicode separator crash in search output

[Formatted]

Added stdout-encoding-aware separator fallback in searcher.py so cp1252 consoles use ASCII - instead of Unicode ─.
Added regression test in test_searcher.py that simulates cp1252 stdout and verifies no crash.

Validation:

python3 -m ruff check mempalace/searcher.py tests/test_searcher.py
python3 -m pytest tests/test_searcher.py -v
python3 -m pytest tests/ -q (102 passed)

[bgauryy]

PR Review: fix windows unicode separator crash in search output

Executive Summary

Aspect	Value
PR Goal	Prevent UnicodeEncodeError on Windows cp1252 consoles when printing search result separators
Files Changed	2 (mempalace/searcher.py, tests/test_searcher.py)
Risk Level	🟢 LOW - cosmetic output fix, no data flow or API changes
Review Effort	1 - small, focused bugfix
Recommendation	💬 COMMENT

Affected Areas: search() CLI output in mempalace/searcher.py, test suite in tests/test_searcher.py

Business Impact: Fixes a crash on Windows terminals using cp1252 encoding when running mempalace search. Previously, the ─ (U+2500 BOX DRAWINGS LIGHT HORIZONTAL) character would raise UnicodeEncodeError on these consoles.

Flow Changes: None — the fix only changes the visual separator character for non-UTF-8 terminals. All data flows, return values, and API contracts are unchanged.

Ratings

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

PR Health

• Has clear description
• References ticket/issue (UnicodeEncodeError on Windows: box-drawing chars in searcher.py crash cp1252 console #47)
• Appropriate size
• Has relevant tests

High Priority Issues

🔗 #1: Same ─ crash exists in 5+ other modules — helper is not reusable

Location: mempalace/searcher.py:21 (_separator_line) | Confidence: ✅ HIGH

The _separator_line() helper is defined as a private function in searcher.py, but the exact same ─ character appears in print() calls across at least 5 other core modules — all of which will crash identically on cp1252 consoles:

File	Line	Code
mempalace/miner.py	281	print(f"{'─' * 55}\n")
mempalace/convo_miner.py	241	print(f"{'─' * 55}\n")
mempalace/split_mega_files.py	270, 285	print(f"{'─' * 60}\n")
mempalace/room_detector_local.py	202	print(f"\n{'─' * 55}")
mempalace/onboarding.py	59	print(f"\n{'─' * 58}")

Suggested fix: Move _separator_line() to a shared module (e.g. constants.py or a new output.py) and reuse it across all modules. This also allows consistent separator widths.

# In mempalace/constants.py (or similar shared location)
import sys

def safe_separator(width: int = 56) -> str:
    encoding = getattr(sys.stdout, "encoding", None) or "utf-8"
    try:
        "─".encode(encoding)
        return "─" * width
    except (UnicodeEncodeError, LookupError):
        return "-" * width

---

Medium Priority Issues

🎨 #2: Test placed in wrong class

Location: tests/test_searcher.py:46 | Confidence: ✅ HIGH

The new test test_cli_search_uses_ascii_separator_on_cp1252_stdout tests the CLI search() function, but is placed inside TestSearchMemories. The file's own docstring says "Tests the library-facing search interface (not the CLI print variant)."

- class TestSearchMemories:
-     ...
-     def test_cli_search_uses_ascii_separator_on_cp1252_stdout(self, monkeypatch):

+ class TestSearchCli:
+     def test_uses_ascii_separator_on_cp1252_stdout(self, monkeypatch):

---

🎨 #3: Overly broad exception catch

Location: mempalace/searcher.py:30 | Confidence: ⚠️ MED

except Exception catches everything including KeyboardInterrupt subclasses on some runtimes. Since this is encoding validation, the only expected failures are UnicodeEncodeError (char not representable) and LookupError (unknown encoding name). Being explicit improves readability and prevents accidentally masking unrelated errors.

- except Exception:
+ except (UnicodeEncodeError, LookupError):

---

Low Priority Issues

🎨 #4: No UTF-8 happy-path test

Location: tests/test_searcher.py | Confidence: ⚠️ MED

The test only verifies the cp1252 fallback path. A complementary assertion that UTF-8 terminals still produce the ─ separator would prevent regressions where the function always falls back to ASCII.

def test_cli_search_uses_unicode_separator_on_utf8_stdout(self, monkeypatch):
    # ... same fake client setup ...
    buf = io.BytesIO()
    fake_stdout = io.TextIOWrapper(buf, encoding="utf-8")
    monkeypatch.setattr("sys.stdout", fake_stdout)

    search("anything", "/tmp/fake-palace")
    fake_stdout.flush()
    output = buf.getvalue().decode("utf-8")

    assert "  " + ("─" * 56) in output

---

What's Good

• Correct root-cause fix: The encoding probe ("─".encode(encoding)) is the right approach — it checks capability at runtime rather than guessing by platform.
• Efficient pattern: separator = _separator_line() is computed once before the loop rather than per-iteration.
• Good test isolation: The test correctly monkeypatches both sys.stdout and the ChromaDB client, avoiding any need for a real palace.
• Clean diff: Minimal, surgical change to just the affected code.

---

Created by Octocode MCP https://octocode.ai 🔍🐙

[Formatted]

Thanks for the thorough review! All four issues have been addressed:

• Congratulations Milla #1: Extracted safe_separator() into a new shared output.py module and updated all 6 affected files (miner.py, entity_detector.py, onboarding.py, room_detector_local.py, split_mega_files.py, searcher.py)
• Integrating MemPalace with SoulForge's code intelligence system #2: Moved the CLI tests into a separate TestSearchCLI class
• feat: Hermes memory provider integration #3: Exception narrowed to (UnicodeEncodeError, LookupError) in output.py
• Using AAAK as language for agents #4: Added both a UTF-8 happy-path test and a full TestSafeSeparator suite in the new tests/test_output.py

[web3guru888]

Clean, well-scoped fix for a real Windows UX bug. Here's my full review:

Summary

This PR introduces mempalace/output.py — a new safe_separator() helper that probes sys.stdout.encoding at call time and returns either Unicode box-drawing characters (─) or plain hyphens (-) depending on what the active console can encode. Six modules (searcher.py, miner.py, onboarding.py, entity_detector.py, room_detector_local.py, split_mega_files.py) are updated to use the helper, replacing inline '─' * N expressions. Test coverage includes test_output.py (6 unit tests) and extended test_searcher.py (2 integration tests with fake chromadb client).

What's done well

The safe_separator() design is correct. Probing via "─".encode(encoding) is the right approach — it catches both "encoding doesn't support the codepoint" and "encoding name is unknown" (LookupError). The None fallback to "utf-8" is appropriate for environments where sys.stdout.encoding is unset (e.g., piped output, CI). The getattr guard for missing .encoding attribute handles redirected stdout cleanly.

The separator variable in searcher.py is computed once before the loop rather than once per result — that's a small but correct optimization. If the palette is 50 results, you don't re-probe encoding 50 times.

Test coverage is thorough and uses monkeypatch correctly. The _FakeCollection / _FakeClient mock pair in test_searcher.py is minimal and doesn't leak into other test classes.

Issues and gaps

1. searcher.py still has a '=' * 60 hardcoded Unicode-safe separator. Line 75 in the patch shows print(f"{'=' * 60}\n") — the equals sign is ASCII and won't crash, but it's inconsistent with the stated goal of "stdout-encoding-aware separators." Not a crash risk, just an inconsistency to note.

2. errors="strict" interaction with PR #400. PR #400 (merged or pending) sets sys.stdin.reconfigure(encoding="utf-8", errors="strict") on Windows. If sys.stdout has already been reconfigured to UTF-8 strict by PR #400, safe_separator() will always return ─ on those Windows systems — which is actually correct post-reconfigure. But the two PRs aren't coordinated: safe_separator() checks sys.stdout.encoding which may be utf-8 after PR #400's reconfigure runs, making the cp1252 fallback path dead code on patched systems. This isn't a bug — it's belt-and-suspenders — but worth a comment in output.py explaining the interaction.

3. split_mega_files.py hardcoded env var path. The patch touches split_mega_files.py where there's an unrelated LUMI_DIR = Path(os.environ.get("MEMPALACE_SOURCE_DIR", str(HOME / "Desktop/transcripts"))). This is pre-existing but the Desktop/transcripts default path looks like a development artifact — someone's local transcript directory baked into a shipping module. Worth flagging to maintainers even if out of scope for this PR.

4. encoding detection happens at print time, not at import time. This is intentional and correct for the MCP server case (stdout may be reconfigured after import). But it means the separator character can change mid-session if something reconfigures stdout between the first and last result print. Unlikely in practice, but worth noting in the docstring.

5. No coverage for split_mega_files.py changes. The new test_output.py tests cover safe_separator() in isolation, but the split_mega_files.py integration isn't directly tested. Minor gap given the function is covered unit-level.

Minor nits

• Docstring in output.py mentions only cp1252 but the issue applies to any encoding that lacks the U+2500 block (cp1251, cp950, etc.). Consider broadening the note to "legacy Windows codepages" for accuracy.
• The PR title is lowercase where the rest of the repo uses sentence case. Cosmetic.

Overall verdict

✅ Approve. The core fix is correct and well-tested. safe_separator() is the right abstraction — centralizing the probe means if future code adds more box-drawing characters, there's a clear place to extend. Address the '=' * 60 inconsistency and the split_mega_files.py stray default path in follow-up if needed. The errors="strict" interaction with PR #400 is worth a comment but not a blocker.

---

Reviewed by MemPalace-AGI — autonomous research system with perfect memory

[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.)

[Formatted]

Done — rebased onto develop just now. The three commits applied cleanly; conflicts were in searcher.py, miner.py, split_mega_files.py, and entity_detector.py where recently-merged functions and import additions overlapped, all resolved in favour of keeping both.