feat: add configurable multilingual embedding model support by NickShtefan · Pull Request #2 · NickShtefan/mempalace

NickShtefan · 2026-04-09T22:21:27Z

Summary

Centralizes embedding function selection through get_embedding_function() in config.py
Configurable via MEMPALACE_EMBEDDING_MODEL env var or embedding_model in config.json
Falls back to ChromaDB default (all-MiniLM-L6-v2) when no model configured or sentence-transformers not installed
Adds [multilingual] optional dependency: pip install mempalace[multilingual]
Reduces default chunk size from 800→450 to respect model token limits (Chunk size (800) exceeds embedding model token limit (256 tokens / ~512 chars) MemPalace/mempalace#390)

Motivation

The default all-MiniLM-L6-v2 model is English-centric. Non-English users get poor search quality. Tested with intfloat/multilingual-e5-base on 245 Russian blog posts — relevance scores improved from 0.19–0.40 to 0.70–0.77.

Usage

# Install with multilingual support
pip install mempalace[multilingual]

# Configure model (env var or config.json)
export MEMPALACE_EMBEDDING_MODEL=intfloat/multilingual-e5-base

# Or in ~/.mempalace/config.json:
# { "embedding_model": "intfloat/multilingual-e5-base" }

# Re-mine after changing model (embeddings are model-specific)
mempalace mine ~/my-project

Test plan

pytest tests/test_multilingual.py -v — all 11 tests pass
Default behavior unchanged (no config → ChromaDB default model)
Setting MEMPALACE_EMBEDDING_MODEL selects the specified model
Graceful fallback when sentence-transformers not installed
Search quality on non-English content with multilingual model

Closes MemPalace#231
Refs MemPalace#390

Centralizes embedding function selection via get_embedding_function() in config.py. Supports MEMPALACE_EMBEDDING_MODEL env var and embedding_model in config.json. Falls back to ChromaDB default when sentence-transformers is not installed. All ChromaDB collection access points now pass the configured embedding_function, ensuring consistent embeddings across mine, search, MCP server, layers, graph traversal, and CLI commands. Also reduces default chunk size from 800 to 450 to fit within embedding model token limits. Closes MemPalace#231 Refs MemPalace#390 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ut, tests Addresses the six Copilot review comments on the initial commit. 1) MemPalace#6 (critical) — mcp_server.py `_get_collection` bypassed ChromaBackend The MCP server creates its palace collection directly via `chromadb.PersistentClient.get_or_create_collection` in `_get_collection`, not through `ChromaBackend.get_collection`. That path was missing the `hnsw:num_threads=1` metadata, so the primary crash surface for MemPalace#974 and MemPalace#965 was untouched by the original patch. Fixed by passing `hnsw:num_threads=1` at the mcp_server create site too. Documented in a code comment that the setting is only honored at creation time — existing palaces created before this fix still need a `mempalace nuke` + re-mine to gain the protection. 2) MemPalace#3 — mine_global_lock over-serialized mines across unrelated palaces Replaced the single global lock file `mine_global.lock` with a per-palace lock keyed by `sha256(os.path.abspath(palace_path))` (`mine_palace_<hash>.lock`). Mines against the same palace still collapse to a single runner (the correctness boundary), but mines against *different* palaces are now free to run in parallel. `mine_global_lock` is kept as a backward-compatible alias for `mine_palace_lock` so any external callers that imported the previous name keep working. 3) #1 — hook_precompact swallowed OSError but not subprocess.TimeoutExpired `subprocess.run(..., timeout=60)` raises `TimeoutExpired` on slow palaces. The previous `except OSError` clause didn't catch it, so the hook could raise and fail to emit any JSON decision — leaving the harness without a block/passthrough signal. Fixed by catching `(OSError, subprocess.TimeoutExpired)` together and always falling through to the block decision so the hook reliably emits a response. 4) #2 + MemPalace#4 — tests - tests/test_hooks_cli.py: added `test_precompact_first_two_attempts_block`, `test_precompact_passes_through_after_cap`, and `test_precompact_counter_is_per_session` to lock in the MemPalace#955 deadlock fix. - tests/test_palace_locks.py (new): covers `mine_palace_lock` single-acquire, reuse-after-release, cross-process serialization on the same palace, non-interference across different palaces, path normalization, and the `mine_global_lock` back-compat alias. 5) MemPalace#5 — known limitation, documented but not auto-fixed Copilot suggested detecting collections missing `hnsw:num_threads=1` and calling `collection.modify(metadata=...)` to retrofit existing palaces. Verified against chromadb 1.5.7: `modify(metadata=...)` replaces metadata rather than merging, and re-passing `hnsw:space="cosine"` then raises `ValueError: Changing the distance function of a collection once it is created is not supported currently.` The HNSW runtime configuration (`configuration_json`) also does not expose `num_threads` in chromadb 1.5.x, so the flag appears to be read only at creation time. Rather than paper over the limitation with a best-effort `modify` that silently drops `hnsw:space`, documented in the mcp_server comment that pre-existing palaces need a `mempalace nuke` + re-mine to gain the protection. Fresh palaces are always protected. Testing - pytest tests/test_palace_locks.py tests/test_hooks_cli.py tests/test_backends.py tests/test_cli.py → **98 passed, 0 failed**. - Runtime validation with two concurrent `mempalace mine` calls: - Different palaces → both complete in parallel ✓ - Same palace → one completes, the other exits with "another `mine` is already running against <palace> — exiting cleanly." ✓

…parity) Third amendment to PR MemPalace#1555. Closes the remaining parity gaps between format_miner and the existing project miner that Aya's palace audit surfaced after amendment #2. ## What was broken A smoke test against the just-mined v6 palace confirmed: WING: wing_aya ROOM: documents 2904 drawers ← ALL in one room Tunnels for wing_aya: 0 ← no cross-wing links Cause: format_miner hardcoded `room = "documents"` for every drawer and never called `_compute_topic_tunnels_for_wing()` post-mine. miner.py calls `detect_room()` per drawer and computes tunnels after the loop. format_miner did neither. ## What this commit changes 1. Module-level imports of `detect_room`, `load_config`, and `_compute_topic_tunnels_for_wing` from `.miner`. Module-level (not lazy) so test seams `patch("mempalace.format_miner.detect_room", ...)` work — lazy imports inside a function don't expose attributes on the module object. 2. `mine_formats()` loads the project's `mempalace.yaml` via `load_config()` to get the rooms list. Falls back to a single "documents" room if no config exists. 3. The hardcoded `room = "documents"` line is replaced with: room = detect_room(filepath, text, rooms, format_path) Mirrors miner.py:904 exactly — folder-match → filename-match → content-keyword scoring → fallback "general". 4. After the per-file loop completes (in an `else` branch on the outer try, so it does NOT run on KeyboardInterrupt), call `_compute_topic_tunnels_for_wing(wing)` in try/except. Exact mirror of miner.py:1241-1249. Tunnel-compute failures must never fail a mine. 5. Import `sys` at module level (used by the tunnel-compute error path for `print(..., file=sys.stderr)`). ## Tests 5 new RED-first tests in `tests/test_format_miner.py`: - test_mine_formats_calls_load_config_for_rooms - test_mine_formats_calls_detect_room_per_file - test_mine_formats_uses_detected_room_in_drawer_metadata - test_mine_formats_calls_compute_topic_tunnels_after_loop - test_mine_formats_tunnel_failure_does_not_crash_mine All 5 failed against pre-commit code with AttributeError on missing module attributes (proof the bug existed). All 5 pass after the implementation. Total format_miner test count: 62. Total proposal test count (format_miner + line_numbers): 83. Full mempalace test suite: 2002 passed, 1 skipped (no regressions). ## Verification ruff check mempalace/format_miner.py tests/test_format_miner.py → All checks passed! ruff format --check ... → already formatted (pinned ruff 0.15.9) pytest tests/test_format_miner.py tests/test_line_numbers.py → 83 passed pytest -q (full suite) → 2002 passed, 1 skipped ## Not in this commit (deferred) Within-wing hallway primitives are a separate architectural addition (separate PR — being designed). The current `_compute_topic_tunnels_for_wing` matches miner.py exactly but inherits miner.py's gap: it computes tunnels from raw topic words rather than from hallway primitives. That refactor lands in its own PR after the hallway primitive ships.

…licing in hot paths Three medium-priority gemini-code-assist comments on PR MemPalace#1579 all flagged the same anti-pattern: slicing or splitting the entire ``content`` string to inspect a small prefix. On a 500MB file with 50K chunks, the original ``content[:offset]`` and ``content.split("\n")`` calls produce O(N²) work and severe memory churn. None of this was wrong semantically — it was wrong about *cost*. This commit replaces all three sites with bounded equivalents that read the original string in place: ## Changes 1. **``mempalace/miner.py::chunk_text``** — replace ``content[:start].count("\n")`` with ``content.count("\n", 0, start)``. The ``str.count`` method's bounds form counts on the original string without allocating a slice. Same result, O(start) instead of O(N) memory per chunk. 2. **``mempalace/miner.py::_try_frontmatter_date``** — replace the ``stripped.split("\n")`` + line-by-line scan with ``stripped.find("\n---", 3)`` to locate the closing delimiter, then slice the frontmatter region directly. Frontmatter is always near the start of the file, so the find walks at most a few KB even on huge files. The earlier implementation produced a list of every line in the file. 3. **``mempalace/miner.py::_try_content_body_date``** — two-part fix: (a) skip frontmatter via ``find("\n---", 3)`` (same pattern as #2); (b) replace ``stripped.split("\n")[:10]`` with ``stripped.split("\n", 10)[:10]`` — the ``maxsplit`` form caps work to scanning 10 newlines rather than splitting the entire file. ## Why no behavior change All three transformations are equivalence-preserving: - ``s[:n].count(ch) == s.count(ch, 0, n)`` — by definition. - ``s.split("\n")[i].strip() == "---"`` for some i ≤ N is equivalent to ``s.find("\n---") != -1`` (both detect "a line consisting of exactly ---") for our use case where the opening fence is already validated by ``stripped.startswith("---")``. - ``s.split("\n")[:10]`` produces the same first-10-elements as ``s.split("\n", 10)[:10]``; the difference is the latter stops after 10 newlines instead of scanning the rest. ## Verification pytest tests/test_miner.py::TestChunkTextLineRanges tests/test_miner.py::TestBuildDrawerMetadataLineRange tests/test_miner.py::TestExtractContentDate tests/test_closets.py::TestBuildClosetLines → 32 passed (same as PR MemPalace#1579's baseline; no test changes needed because behavior is identical, only memory/time profile changed) ruff check + ruff format --check (pinned 0.15.9) → All checks passed; 1 file already formatted

Adds _try_gemini_json parser to normalize.py for three layouts: 1. Gemini API contents format (~/.gemini/sessions/*.json): {"contents": [{"role": "user", "parts": [{"text": "..."}]}, ...]} 2. Messages-wrapper variant: {"messages": [{"role": "user", ...}, {"role": "model", ...}]} 3. Flat top-level list with role="model". This complements the existing _try_gemini_jsonl parser (which handles ~/.gemini/tmp/<hash>/chats/session-*.jsonl with session_metadata sentinel) — JSONL covers Gemini CLI runtime sessions, JSON covers exported / Studio-saved transcripts. ## Review feedback addressed (PR MemPalace#204) bgauryy review: - #1 Parser-precedence bug: _try_gemini_json runs *before* _try_claude_ai_json so the {"messages":[..., role=model, ...]} layout is no longer silently claimed by the Claude parser. The Gemini parser's has_model_role guard prevents false-positives against Claude / ChatGPT data. - #2 Layout 2a coverage: TestGeminiJson.test_messages_wrapper_format + test_messages_wrapper_does_not_get_claimed_by_claude pin the fix in place. - MemPalace#3 Test conflicts with current main: rebased onto develop; tests restructured into TestGeminiJson class. - MemPalace#4 tempfile/os.unlink → pytest tmp_path everywhere. - MemPalace#5 elif not text → else (the elif branch was dead). - MemPalace#6 Module docstring updated to mention Google AI Studio. Tests: 9 new cases in TestGeminiJson covering all three layouts, multi-part text joining, non-text part skipping, has_model_role disambiguation, dispatch-chain regression for review #1.

Five fixes from the Gemini Code Assist review on MemPalace#1632 — three real bugs, two cleanups, all consistent with the bash-3.2-compatibility contract documented in the original commit. Bug fixes (high) ---------------- 1. hooks/cursor/lib/common.sh — config.json kill-switch check used a `python3 - <<'PYEOF' ... PYEOF` heredoc inside a `$(...)` command substitution. The heredoc body contains parens which trips the macOS bash 3.2.57 parser bug. Replaced with a `python -c '...'` call passing the config path as argv[1]. Matches the pattern already used in mempal_parse_stdin in the same file. 2. hooks/cursor/install.sh — a relative `--install-dir` was written verbatim into hooks.json. Cursor invokes hook commands from its own working directory (typically the project root), so a relative command path would silently fail to launch the hook. Now resolved to an absolute path against `$PWD` before being baked in. 3. hooks/cursor/mempal_save_hook_cursor.sh — `MEMPAL_SAVE_INTERVAL=0` would crash bash on `$((NEXT % 0))` (division by zero). Extended the existing sanitiser case to coerce 0 to the default interval alongside empty / non-numeric values. Cleanups (medium) ----------------- 4. hooks/cursor/install.sh — the EMPTY_CHECK_PY temp file is now inlined as `python -c '...'`. Removes a small leak window (tmpfile would linger if the script were interrupted between mktemp and rm -f) and shortens the script. 5. hooks/cursor/install.sh — `mktemp -t prefix` has subtly different semantics on BSD (macOS) vs GNU mktemp. Switched to the portable absolute-template form `mktemp "${TMPDIR:-/tmp}/...XXXXXX"` which behaves identically on both. Regression tests ---------------- - tests/test_cursor_hooks_shell.py test_save_interval_zero_is_coerced_to_default — guards fix MemPalace#3. - tests/test_cursor_hooks_install.py — new TestInstallDirAbsolutePath class: test_relative_install_dir_is_absolutized_in_hooks_json — guards fix #2 against regression. test_absolute_install_dir_is_preserved_verbatim — guards that the relative-to-absolute resolution does not mangle paths that were already absolute. Verification ------------ - bash -n on all three edited scripts: clean. - uv run pytest tests/test_cursor_hooks_*.py tests/test_cursor_plugin_manifest.py: 132 passed (was 129; +3 regression tests). - uv run pytest tests/ --ignore=tests/benchmarks: 2399 passed, 3 skipped (pre-existing). - uv run ruff check . / ruff format --check .: clean. Co-authored-by: Cursor <cursoragent@cursor.com>

NickShtefan closed this Apr 9, 2026

Navigation Menu

feat: add configurable multilingual embedding model support - #2

feat: add configurable multilingual embedding model support#2
NickShtefan wants to merge 1 commit into
mainfrom
feat/multilingual-embedding

NickShtefan commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

NickShtefan commented Apr 9, 2026

Summary

Motivation

Usage

Test plan

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant