fix: replace Unicode checkmark with ASCII for Windows encoding (#535) by jphein · Pull Request #681 · MemPalace/mempalace

jphein · 2026-04-12T07:08:01Z

Summary

Replace Unicode ✓ (U+2713) with ASCII + in convo_miner.py and split_mega_files.py
Windows terminals using cp1251/cp1252 crash on the Unicode character during mining progress output

Files changed

mempalace/convo_miner.py — line 359
mempalace/split_mega_files.py — line 227

Test plan

Existing tests pass
Verify mining progress output displays correctly on Windows

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR aims to prevent Windows console encoding crashes during progress output by replacing the Unicode checkmark (✓) with an ASCII alternative in mining/splitting scripts.

Changes:

Replace ✓ with + in convo_miner.py progress output.
Replace ✓ with + in split_mega_files.py per-chunk write output.

Reviewed changes

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

File	Description
mempalace/convo_miner.py	Swaps the per-file progress marker from Unicode to ASCII.
mempalace/split_mega_files.py	Swaps the per-chunk success marker from Unicode to ASCII.

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

Copilot · 2026-04-12T07:10:48Z


        total_drawers += drawers_added
-        print(f"  ✓ [{i:4}/{len(files)}] {filepath.name[:50]:50} +{drawers_added}")
+        print(f"  + [{i:4}/{len(files)}] {filepath.name[:50]:50} +{drawers_added}")


This change removes the checkmark here, but the same Unicode ✓ progress marker still exists in mempalace/miner.py (used by the main mempalace mine flow). If the goal is to avoid Windows cp1251/cp1252 UnicodeEncodeErrors during progress output, that remaining print will still crash—please update it (or centralize the status marker) so all mining commands are consistent.

Fixed — added miner.py checkmark and all box-drawing/arrow chars (─, →) across both files. 5044464

Copilot · 2026-04-12T07:10:48Z

            print(f"  [{i + 1}/{len(boundaries) - 1}] {name}  ({len(chunk)} lines)")
        else:
            out_path.write_text("".join(chunk), encoding="utf-8")
-            print(f"  ✓ {name}  ({len(chunk)} lines)")
+            print(f"  + {name}  ({len(chunk)} lines)")



Replacing ✓ with '+' helps, but this script still prints other non-ASCII characters (e.g., the box-drawing '─' separators and the '→' arrow). On Windows consoles/codepages like cp1251/cp1252 those can also raise UnicodeEncodeError, so the encoding crash may persist even after this change. Consider switching those to ASCII equivalents (e.g., '-' and '->') or explicitly ensuring stdout is UTF-8 / using a safe write strategy.

Fixed — replaced ─ with - and → with -> in split_mega_files.py. All non-ASCII progress markers are now ASCII. 5044464

…emPalace#681-684 new Co-Authored-By: Claude Opus 4.6 <[email protected]>

…ged, add MemPalace#681, bump test count to 715 Co-Authored-By: Claude Opus 4.6 <[email protected]>

…Palace#535) Windows terminals using cp1251/cp1252 crash on the Unicode ✓ (U+2713) in progress output. Replace with ASCII + in convo_miner.py and split_mega_files.py. Co-Authored-By: Claude Opus 4.6 <[email protected]>

Also fix miner.py checkmark and box-drawing/arrow chars (─, →) in both miner.py and split_mega_files.py that would crash on cp1251/cp1252. Co-Authored-By: Claude Opus 4.6 <[email protected]>

fork-direction.md and TODO-fork-improvements.md were scattered strategic thinking that belongs in the front-door README: competitive landscape, roadmap (P0-P6), and open problems. Merges them in and deletes the standalone files. PR-status tables (README + CLAUDE.md) were stale after today's rebases of MemPalace#659, MemPalace#660, MemPalace#661, MemPalace#673, MemPalace#681 — updated to reflect current mergeable state. MemPalace#673 specifically noted as cleanly rebased against MemPalace#863. test_readme_claims.py skips MCP-tool-table and dialect-reference checks when absent — our slimmed fork README doesn't reproduce upstream's tool table structure. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

jphein · 2026-04-18T16:56:24Z

Friendly ping on review — this is a one-character change swapping the Unicode ✓ for ASCII to fix Windows GBK / cp1252 encoding errors (#535). Clean merge against current develop. Happy to rebase if needed.

bensig · 2026-04-19T06:20:34Z

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

…emPalace#1036 Overnight/morning: - MemPalace#681, MemPalace#1000, MemPalace#1023 merged — moved from "Still ahead" to "Merged upstream (post-v3.3.1)" - bensig reviewed MemPalace#659 (wing_ prefix + agent filter) and MemPalace#1021 (silent_guard default) — both addressed on their PR branches - MemPalace#673 needed re-rebase after overnight develop merges; done - MemPalace#1036 filed: paginate miner.status(), closes upstream MemPalace#802 and MemPalace#1015 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Version bumps across pyproject.toml, mempalace/version.py, README badge, uv.lock, and plugin manifests (.claude-plugin/*, .codex-plugin/*). CHANGELOG aligned with main (post-3.3.1) and a new [3.3.2] section added covering the 11 PRs merged on develop since v3.3.1 — silent-transcript-drop fix + tandem sweeper (#998), None-metadata guards (#999, #1013), chromadb ≥1.5.4 for Py 3.13/3.14 (#1010), Windows Unicode (#681), HNSW quarantine recovery (#1000), PID stacking guard (#1023), doc-path cleanup (#996, #1012), and RFC 001/002 internal scaffolding (#995, #1014, #990).

Non-ASCII glyphs (regression of the #681 class of Windows UnicodeEncodeError): - mempalace/cli.py: "✗" → "ERROR:", "⚠" → "WARNING:", em dash → "-" - mempalace/sweeper.py: "⚠" → "WARNING:" Backend arg validation: - mempalace/backends/chroma.py: `_normalize_get_collection_args` now raises TypeError on unexpected trailing positional args instead of silently dropping them — surfaces call-site bugs early. Docs site: - website/.vitepress/config.mts: gate Google Analytics scripts behind MEMPALACE_DOCS_GA_ID env var (default off). Self-hosters no longer get GA injected unconditionally. Landing page SPA hygiene: - website/.vitepress/theme/landing/useLandingEffects.js: collect all IntersectionObserver disconnects and removeEventListener thunks in a shared `cleanups` registry; drain it in `onBeforeUnmount` so observers and form/replay listeners don't leak across SPA navigations.

Resolutions: - `.claude-plugin/.mcp.json`, `plugin.json` — adopt upstream's `mempalace-mcp` console-script command (added via upstream MemPalace#340 for pipx/uv). Run `pip install -e .` in plugin venv after merge to install the entry point. - `.claude-plugin/hooks/*.sh`, `hooks/*.sh` — adopt upstream's console-command resolution order (`mempalace` script → `python3 -m mempalace` → `python`). `MEMPAL_PYTHON` override still works inside `hooks_cli.py`. - `mempalace/hooks_cli.py`, `tests/test_hooks_cli.py` — keep fork's `_mempalace_python()` helper (fork-ahead #4); upstream only had `sys.executable`, which loses MEMPAL_PYTHON override. - `mempalace/miner.py` — keep fork's concurrent mining path (fork-ahead #1), apply upstream's unicode-`✓` → ASCII-`+` fix (MemPalace#681) to both paths. - `mempalace/backends/chroma.py` — take upstream's refined `quarantine_stale_hnsw` docstring (it's the version merged via our own MemPalace#1000). Brought in: 33 upstream commits including Belarusian/Chinese/German/Spanish/French entity detection, console-script entry points, hook plugin-root space quoting, and v3.3.2 tag (which contains our MemPalace#681/MemPalace#1000/MemPalace#1023). Tests: 1096 passed, 106 deselected (benchmarks). Ruff clean.

…#1023 merged, MemPalace#673 rebased - Bump fork-ahead section header to "after v3.3.2" - Strike #11 (quarantine_stale_hnsw) and MemPalace#18 (PID file guard) as merged-into-upstream-via-v3.3.2, keep entries for traceability - Add v3.3.2-shipped items to "Merged into upstream (post-v3.3.1)" - Rebuild PR table: 10 merged / 7 open / 7 closed; add MemPalace#1024 row, reclassify MemPalace#681/MemPalace#1000/MemPalace#1023 as merged, note MemPalace#673 rebased 2026-04-21 - Annotate MemPalace#661 status with the GitHub review-state machine caveat (CHANGES_REQUESTED persists until reviewer dismisses, not owed) - Bump test count 1063 → 1096 post-merge

…unts - Upstream released v3.3.2 on 2026-04-21 (our MemPalace#681/MemPalace#1000/MemPalace#1023) - Drawer count 152,682 → 165,632; wings 23 → 28; tests 1063 → 1096 - MemPalace#673 now MERGEABLE after fresh rebase + squash to 1 commit - MemPalace#661 status clarified: CHANGES_REQUESTED persists until reviewer dismisses, not an outstanding owe - Merged-upstream section split out v3.3.2 release group

…ace#1034) GBK consoles (Windows PowerShell/CMD default) cannot encode U+2713 (✓), U+2717 (✗), and U+2014 (—). The same class of UnicodeEncodeError fixed in miner.py via MemPalace#681 affects closet_llm.py and cli.py. Replace with ASCII equivalents: [OK], [FAIL], [!], and hyphen.

jphein requested review from bensig and milla-jovovich as code owners April 12, 2026 07:08

Copilot AI review requested due to automatic review settings April 12, 2026 07:08

Copilot started reviewing on behalf of jphein April 12, 2026 07:08 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

jphein added a commit to jphein/mempalace that referenced this pull request Apr 12, 2026

docs: update PR table — MemPalace#626 closed, MemPalace#664 merged, M…

7870ddf

…emPalace#681-684 new Co-Authored-By: Claude Opus 4.6 <[email protected]>

jphein added a commit to jphein/mempalace that referenced this pull request Apr 12, 2026

docs: update PR table — MemPalace#682/MemPalace#683/MemPalace#684 mer…

ee45216

…ged, add MemPalace#681, bump test count to 715 Co-Authored-By: Claude Opus 4.6 <[email protected]>

jphein force-pushed the fix/unicode-checkmark branch from 5044464 to d092918 Compare April 13, 2026 14:22

jphein mentioned this pull request Apr 13, 2026

fix: sync version.py with pyproject.toml and update stale org URLs (#771, #787) #794

Closed

igorls added area/i18n Multilingual, Unicode, non-English embeddings area/mining File and conversation mining area/windows Windows-specific bugs and compatibility bug Something isn't working labels Apr 14, 2026

jphein and others added 2 commits April 16, 2026 08:59

fix: replace all non-ASCII progress markers for Windows encoding

15ea385

Also fix miner.py checkmark and box-drawing/arrow chars (─, →) in both miner.py and split_mega_files.py that would crash on cp1251/cp1252. Co-Authored-By: Claude Opus 4.6 <[email protected]>

jphein force-pushed the fix/unicode-checkmark branch from 586d302 to 15ea385 Compare April 16, 2026 16:01

bensig merged commit 62439e1 into MemPalace:develop Apr 19, 2026
6 checks passed

jphein mentioned this pull request Apr 19, 2026

[Windows] UnicodeEncodeError when running mempalace mine on GBK console #1034

Closed

igorls mentioned this pull request Apr 19, 2026

release: v3.3.2 #1041

Merged

8 tasks

igorls mentioned this pull request Apr 19, 2026

fix: address Copilot review on release/3.3.2 #1045

Merged

3 tasks

jphein mentioned this pull request Apr 21, 2026

mempalace status crashes on palaces >~32K drawers (SQLite variable limit) #1015

Closed

messelink mentioned this pull request Apr 22, 2026

release/3.3.2 fixes not back-merged into develop — non-ASCII regression and backend hardening missing #1095

Closed

arnoldwender mentioned this pull request Apr 22, 2026

fix(closet_llm): replace non-ASCII symbols in progress output (#1034) #1100

Open

2 tasks

dylanneve1 mentioned this pull request Apr 23, 2026

fix(mempalace): require >= 3.3.2 for PID guard + HNSW crash fix dylanneve1/talon#83

Closed

4 tasks

jphein deleted the fix/unicode-checkmark branch April 25, 2026 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: replace Unicode checkmark with ASCII for Windows encoding (#535)#681

fix: replace Unicode checkmark with ASCII for Windows encoding (#535)#681
bensig merged 2 commits intoMemPalace:developfrom
jphein:fix/unicode-checkmark

jphein commented Apr 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 12, 2026

Uh oh!

jphein Apr 12, 2026

Uh oh!

Copilot AI Apr 12, 2026

Uh oh!

jphein Apr 12, 2026

Uh oh!

jphein commented Apr 18, 2026

Uh oh!

bensig commented Apr 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jphein commented Apr 12, 2026

Summary

Files changed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein commented Apr 18, 2026

Uh oh!

bensig commented Apr 19, 2026

Code review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants