chore(release): 3.3.5 - #1434
Merged
Merged
Conversation
Chroma 1.5.x can return ``None`` inside the ``metadatas`` / ``documents`` lists of a query/get result for partially-flushed rows. The codebase already has a systemic None-guard pattern (merged #999, #1013, #1019) but three call sites were still unguarded: * ``mcp_server.tool_check_duplicate`` (``mcp_server.py:487-488``) — ``meta = results["metadatas"][0][i]`` followed by ``meta.get(...)`` raises ``AttributeError: 'NoneType' object has no attribute 'get'``. The broad ``except Exception`` wrapper (line 504) swallows it and returns an uninformative ``"Duplicate check failed"``. * ``layers.Layer1.generate`` (``layers.py:126``) — iterates ``zip(docs, metas)`` and calls ``meta.get(key)`` in the importance loop. A single None metadata blows up the entire wake-up render. * ``layers.Layer2.retrieve`` (``layers.py:224``) — same pattern, same crash path for the on-demand render. Apply the same ``meta = meta or {}`` / ``doc = doc or ""`` idiom used by the merged guards in the search path. Three-line additions, no behaviour change on well-formed results. Tests added: * ``test_check_duplicate_handles_none_metadata`` — mocks the collection query to return ``None`` for one metadata and document, asserts the call does not crash and the sentinel-rendered entry has wing/room "?" and empty content. * ``test_layer1_handles_none_metadata`` / ``_handles_none_document`` * ``test_layer2_handles_none_metadata`` Relationship to other open PRs: * **#1019** guarded ``searcher.py`` loops. This PR extends the same guard to the three call sites #1019 did not touch. * **#979** fixed ``tool_check_duplicate`` negative similarity but left the None-metadata path unguarded. * Does not overlap **#1013** (``Layer3.search_raw``) or **#999**.
On Windows, Python defaults sys.stdin/sys.stdout to the system codepage
(e.g. cp1251 on Russian locales, cp1252 on Western European), while MCP
JSON-RPC is always UTF-8. Non-ASCII payloads (Cyrillic, CJK, accented
European) get mis-decoded before reaching handlers, causing json.loads
to fail or tool handlers to receive garbled strings. Both surface to
the client as a generic MCP error -32000.
Reproduction:
1. On Windows with a non-Latin locale, call mempalace_add_drawer or
mempalace_kg_add with Cyrillic/CJK in content or KG object.
2. Server returns: MCP error -32000: Internal tool error.
3. Calling the handler directly from Python works fine -- the bug is
purely in the stdio transport.
Fix:
Reconfigure stdin/stdout to UTF-8 at the start of main(), after
_restore_stdout(). Uses errors="replace" defensively so a lone bad
byte cannot take down the server. Guarded by hasattr(reconfigure)
for exotic stream replacements.
This matches the behaviour of PYTHONUTF8=1 / python -X utf8 without
requiring users to set an env var.
The list_drawers response only included count (current page size) with no total field, making it impossible for callers to know when pagination is exhausted. A page returning count == limit is ambiguous — it could be the last exact-fit page or there could be more results. Add a total field that reports the full number of matching drawers. For unfiltered requests this uses col.count(); for filtered requests (wing/room) it uses a lightweight col.get(include=[]) to count matching IDs without fetching documents.
A triple with valid_to < valid_from satisfies neither of the temporal
filter clauses in query_entity():
valid_from <= as_of AND valid_to >= as_of
so the triple is invisible to every query — silently corrupt. Reject
at write time with a clear error instead of letting bad data pile up
in the SQLite store.
The guard only fires when both bounds are present; open intervals
(only valid_from or only valid_to) are still accepted, and same-day
intervals (valid_from == valid_to, point-in-time facts) are explicitly
allowed.
#976 protects `mempalace mine`, but MCP/direct backend writers still call ChromaCollection.add/upsert/update/delete without the palace lock. This moves the lock boundary to the Chroma backend seam so all Chroma writes share the same palace-level serialization, with a re-entrant guard for miner paths that already hold the lock. mine_palace_lock(palace_path) gains a per-thread re-entrant guard (threading.local + pid-tag against fork inheritance) so ChromaCollection write methods can take the lock without self-deadlocking when called from inside miner.mine()'s outer hold. ChromaCollection.__init__ accepts an optional palace_path; when set, add/upsert/update/delete wrap their underlying chromadb call with mine_palace_lock(palace_path). palace_path=None preserves the legacy no-lock behaviour for direct callers and tests. ChromaBackend's get_collection/create_collection pass palace_path through; mcp_server._get_collection forwards _config.palace_path so all MCP write tools inherit the wrapping. Tests: 5 new in tests/test_chroma_collection_lock.py covering opt-in, writer-blocks-during-mine, re-entrant-inside-mine, two-process serialization, and a source-level read-path-not-locked pin. Plus 1 new + 1 rewritten in tests/test_palace_locks.py for the re-entrant semantics. 52 passed in 1.01s including the existing test_backends.py regression suite. Refs #1161.
Mirror the pagination pattern PR #851 landed in miner.py:status(). A single drawers_col.get(limit=total, ...) on palaces larger than SQLite's SQLITE_MAX_VARIABLE_NUMBER (32766) crashes inside chromadb. Fetch drawers in batch_size=5000 chunks, stepping offset until the collection is drained. by_source aggregation semantics are preserved exactly — grouping, wing filter, meta capture all unchanged. Closes #1073. Related: #802, #850, #1016.
tool_kg_query (as_of), tool_kg_add (valid_from), and tool_kg_invalidate (ended) accepted any string and forwarded it to SQLite without format validation. Parameterized queries prevent SQL injection, but invalid date strings silently produce empty result sets — callers cannot distinguish "no fact at this time" from "your date format was unrecognized." This is especially painful for natural-language LLM callers that synthesize dates like "March 2026" or "Jan 2025". Add sanitize_iso_date() in config.py alongside the other input validators. It accepts YYYY, YYYY-MM, and YYYY-MM-DD forms; passes through None/empty; and raises ValueError with a field-named message on anything else. Call it from the three kg MCP tool wrappers before values reach the storage layer so the caller gets a clear error instead of a silent miss. Closes #1164
Per qodo-ai review on PR #1167: sanitize_iso_date() previously accepted YYYY and YYYY-MM, but KnowledgeGraph.query_entity() compares valid_from/ valid_to TEXT columns lexicographically against as_of. Lexicographic comparison treats '2026-01-01' as greater than '2026' (because '-' > end-of-string), so partial as_of values silently excluded valid facts — re-introducing the silent-empty-results problem this PR was meant to fix. Tighten _ISO_DATE_RE to require YYYY-MM-DD only. Update docstring and error message accordingly. Invert the two test cases that asserted partials were accepted.
Bumps [actions/configure-pages](https://github.com/actions/configure-pages) from 5 to 6. - [Release notes](https://github.com/actions/configure-pages/releases) - [Commits](actions/configure-pages@v5...v6) --- updated-dependencies: - dependency-name: actions/configure-pages dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Rollback cleanup was instantiating a fresh ChromaBackend, so the live backend that had opened the PersistentClient could keep file handles alive during restore. Close the active backend instance instead so rollback and CLI recovery can release Windows-safe locks before copying the backup back into place.
`mcp_server._get_collection` bypassed `ChromaBackend.get_collection` and called `client.get_collection` / `client.create_collection` without `embedding_function=`. ChromaDB 1.x does not persist the EF identity with the collection, so the MCP server's reopen silently bound chromadb's built-in `DefaultEmbeddingFunction` while the miner / Stop hook ingest path bound `mempalace.embedding.get_embedding_function()`. On bleeding-edge interpreters (python 3.14 + chromadb 1.5.x on Apple Silicon, per #1299) the default EF's lazy ONNX provider selection could SIGSEGV the host process on first `col.add()`, killing the MCP stdio server and leaving every subsequent tool call returning `Connection closed` until Claude Code was relaunched. Reads worked because `col.get(ids=...)` and metadata fetches don't invoke the EF; the auto-ingest path worked because mining routes through the backend abstraction. Diary writes were the consistent failure surface. Resolve the EF up front (matching `ChromaBackend._resolve_embedding_function`) and pass it into both reopen branches. Falls back to the chromadb default only if `mempalace.embedding.get_embedding_function` itself raises. Regression test patches the chromadb client class to capture `embedding_function=` on every `get_collection` / `create_collection` call from `_get_collection(create=True)` and `_get_collection()`, and fails if any call omits it. Follow-up to #1262 / #1289 (which fixed the metadata-mismatch SIGSEGV path); this addresses the EF-mismatch SIGSEGV path on the same surface.
- Resolve the EF inside the two reopen branches that actually call `client.get_collection` / `client.create_collection`, so warm-cache reads stay zero-cost (no `MempalaceConfig()` / `_resolve_providers` on every tool call). - Reuse `ChromaBackend._resolve_embedding_function()` instead of duplicating its try/except + log message + None-fallback. - Reword the inline + CHANGELOG explanation to clarify that ChromaDB 1.x persists the EF *identity* (its `name()`) but not the *instance/ configuration* — `mempalace.embedding` documents this and spoofs `name()` to `"default"` precisely so the identity check passes; the bug was the *provider list* (lazy ONNX selection) silently differing.
…ing-function fix(mcp_server): pass embedding_function= on collection reopen (#1299)
…ing pool
Default search behavior is unchanged. Opt-in candidate_strategy="union"
also pulls top-K BM25-only candidates from sqlite FTS5 and merges them
into the rerank pool, catching docs with strong BM25 signal that the
vector index didn't surface in the over-fetch window.
Motivation
----------
The current hybrid path gathers candidates from the vector index only
(n_results * 3 over-fetch), then BM25-reranks within them. When the
query embeds close to the wrong content semantically, the right doc
never enters the rerank pool — *no matter how wide the over-fetch*.
Tested on a ~6K-document mixed corpus (knowledge prose + short structured
records): at *30x* over-fetch (~5% of the corpus) the target doc still
didn't surface for narrative-shaped queries targeting terminology guides.
Wider over-fetch isn't the answer; widening the pool's *source* is.
Concrete failure mode: a narrative-shaped query embeds close to records
sharing the same operational vocabulary (other narrative entries in the
corpus). A terminology / style guide is BM25-strong for the query
(rare keywords the guide repeats) but vector-distant. Vector-only
candidates don't include it; BM25 never gets to rerank it. The hybrid
path produces 0.00 recall on a probe that pure BM25 alone scores 1.00 —
the hybrid is worse than its component on the same input.
Behavior change
---------------
* New parameter ``candidate_strategy: str = "vector"`` on ``search_memories``.
- ``"vector"`` (default): historical behavior, no change.
- ``"union"``: also fetch top ``n_results * 3`` candidates via the
existing ``_bm25_only_via_sqlite`` helper, dedupe by source_file,
merge into the rerank pool. BM25-only candidates carry
``distance=None`` so they're scored on BM25 contribution alone
(vec_sim coerces to 0).
* ``_hybrid_rank`` now handles ``distance=None`` explicitly, scoring
such candidates as vector-unknown (vec_sim=0) rather than treating
it as max-distance via shim.
* New strategies register via ``_CANDIDATE_MERGERS``; dispatch is in
``_apply_candidate_strategy`` so ``search_memories`` stays under the
C901 complexity ceiling.
Bench numbers (~6K-doc internal mixed corpus, recall@10, 5 probes spanning
policy-exception lookup, temporal-decay, style retrieval, set-difference,
and pattern-recognition):
baseline ("vector") "union"
policy-exception probe 0.00 0.50 +0.50
temporal-decay probe 0.17 0.50 +0.33
style-retrieval probe 0.00 1.00 +1.00 (PASSES)
set-difference probe 0.00–0.06 0.06–0.09 ~
pattern-recog probe 0.64 (stable) 0.50–0.71 variance, typ. +0.07
macro recall 0.16–0.17 0.51–0.56 +0.34 to +0.40
The pattern-recog variance points at a related issue worth a separate PR:
``_hybrid_rank`` computes BM25 IDF over the candidate set. Adding new
candidates re-normalizes BM25 for *existing* candidates non-monotonically.
Stable corpus-wide BM25 would remove this. Out of scope here.
Tests
-----
``tests/test_hybrid_candidate_union.py`` (6 tests, all pass):
- default behavior unchanged (explicit ``"vector"`` matches default)
- ``"union"`` surfaces a BM25-strong vector-distant doc
- ``"union"`` doesn't drop docs ``"vector"`` would have found
- empty-palace handling
- invalid ``candidate_strategy`` raises
- ``_hybrid_rank`` tolerates ``distance=None``
Existing ``test_hybrid_search.py`` (5) and ``test_searcher.py`` (27) pass.
Performance note
----------------
Each ``"union"`` query adds one sqlite open + FTS5 MATCH + metadata
fetch (via the existing ``_bm25_only_via_sqlite`` helper, which already
runs as the ``vector_disabled`` fallback path so the code is
well-trodden). Per-query overhead is small but unmeasured at corpus
scale. Default stays ``"vector"`` until a maintainer characterizes the
cost.
#1076) (#1077) Shell splits hook command on whitespace after variable expansion, breaking paths with spaces (e.g. C:\Users\Richard M on Windows). Wrapping the path in double quotes preserves the token boundary. Fixes the reported Stop/PreCompact pair in .claude-plugin/hooks/hooks.json and applies the same fix to .codex-plugin/hooks.json (SessionStart/Stop/ PreCompact), which carries the identical bug.
When the user removes ~/.mempalace/ (a strong "do not auto-capture"
signal), the next hook fire would silently recreate the entire dir
hierarchy and ingest existing transcripts:
1. _log() at hooks_cli.py:148 unconditionally calls
STATE_DIR.mkdir(parents=True, exist_ok=True), so the act of
writing the hook log line recreated ~/.mempalace/hook_state/
2. With no config file present, hook_stop_auto_save and
hook_precompact_auto_save defaulted to True (no override to read)
3. The full save path then ran, materializing palace/, wal/,
knowledge_graph.sqlite3, and N drawers from existing transcripts
in ~/.claude/projects/*.jsonl
All four entry points (hook_stop, hook_precompact, hook_session_start,
and _log itself) now check a new PALACE_ROOT = Path.home() / ".mempalace"
constant first and short-circuit (returning {} on stdout, never logging)
when the dir is absent. The user-removable directory is now a kill-switch.
Five unit tests in tests/test_hooks_cli.py cover: hook_stop /
hook_precompact / hook_session_start do not create the dir when absent;
_log() does not create it when absent; existing dir proceeds normally
(regression).
Caught in the wild on a downstream fork: ~146 drawers materialized in
under a second after a deliberate `rm -rf ~/.mempalace/`, into a planning
session that was explicitly not meant to be captured.
Both @igorls and the Qodo bot flagged that `_palace_root_exists()` used `Path.exists()`, which returns True for a regular file. A stray file at `~/.mempalace` would let the kill-switch be bypassed and crash later in `STATE_DIR.mkdir()` with NotADirectoryError. Switched to `Path.is_dir()`. Also fold `_log()`'s inline check through `_palace_root_exists()` so both kill-switch sites use the same predicate. New test pins the behavior: a regular file at the palace root path is treated as absent (hook short-circuits, _log does not crash, the stray file is left untouched).
…ad them (#1244) `cmd_compress` was writing AAAK-compressed drawers to a `mempalace_compressed` collection, but every read path (`palace.get_closets_collection`, `searcher.py`, `repair.py`) reads from `mempalace_closets`. Result: for non-mined palaces (or any palace where the user ran `mempalace compress` expecting to backfill the closet/index layer), the compressed output was silently invisible — written to a collection nothing else opens. Fix the writer rather than renaming the readers: "closets" is the user-visible feature name baked into the public API (`get_closets_collection`), the searcher hybrid path, repair/HNSW diagnostics, and docs. Renaming the readers would churn 15+ call sites and the README for no benefit. The compressed AAAK strings are exactly what closets are conceptually — compact pointers scanned by an LLM to locate the right drawer — so they belong in `mempalace_closets`. Tests: - Update `test_cmd_compress_stores_results` to assert the collection name passed to `get_or_create_collection` is `mempalace_closets`. - Add `test_cmd_compress_output_readable_via_get_closets_collection`: end-to-end with a real ChromaBackend, seed a drawer, run cmd_compress, then read back via the same `get_closets_collection` helper that palace.py / searcher use. Regression test for the wrong-collection bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…1314) `tool_kg_add` previously accepted only `valid_from` and `source_closet`, silently dropping `valid_to`, `source_file`, and `source_drawer_id` at the MCP boundary. Backfilling already-ended historical facts therefore collapsed to "still current," and adapter provenance never reached the SQLite layer even though `KnowledgeGraph.add_triple` already supported every column. `tool_kg_invalidate` returned the literal string `"today"` whenever the caller omitted `ended`, hiding the actual stamped date from anyone trying to verify what got persisted. Changes: - Extend `tool_kg_add` signature + MCP input_schema with `valid_to`, `source_file`, `source_drawer_id`; forward all of them to `_kg.add_triple` and to the WAL log. - Resolve `ended` to `date.today().isoformat()` in `tool_kg_invalidate` before logging / returning, so the response always reports the actual date stored in `valid_to`. - Add regression tests for valid_to round-trip, source_file / source_drawer_id provenance, and the resolved-ended-date contract. - Leave TODO(#1283) markers so the open ISO-8601 validation PR can drop `validate_iso_date` over `valid_from` / `valid_to` / `ended` cleanly. The underlying `KnowledgeGraph.add_triple` already accepted these kwargs (RFC 002 §5.5) — only the MCP edge needed wiring up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cmd_init was instantiating MempalaceConfig() unconditionally, ignoring args.palace and always writing the palace under ~/.mempalace. Mirror the env-var pattern used by mcp_server.py (and consistent with how cmd_mine / cmd_status / cmd_search resolve --palace) so every downstream read of cfg.palace_path inside cmd_init — Pass 0, cfg.init(), and the post-init mine — routes to the user-specified location. Adds tests/test_cli.py::test_cmd_init_honors_palace_flag covering the regression: asserts Pass 0 receives the --palace value (not ~/.mempalace) and that MEMPALACE_PALACE_PATH is set in os.environ. Closes #1313.
…event SIGSEGV on stale HNSW (#1121, #1132, #1263) PR #1173 wired quarantine_stale_hnsw into the static make_client() helper but not into the instance _client() method. As a result every non-MCP entry point (CLI mining, search, repair, status) — which all use get_collection / _get_or_create_collection / _client() — skipped the cold-start quarantine pass and could SIGSEGV on a stale HNSW segment left over from a partial flush, replicated palace, or crashed-mid-write. Refactor: extract the (_fix_blob_seq_ids + gated quarantine_stale_hnsw) pre-open pass into a single private static helper ChromaBackend._prepare_palace_for_open(). Both make_client() and _client() now route through it, so the _quarantined_paths once-per- palace-per-process gate is preserved (no runtime thrash on hot paths) and behaviour stays identical — the fix is purely about extending the existing protection to the path that was missing it. Tests: - test_client_quarantines_corrupt_segment_on_first_open mirrors the existing make_client test and verifies _client() actually renames a corrupt segment on first open. - test_client_quarantines_only_on_first_call_per_palace verifies the cache gate prevents re-running quarantine across repeated _client() calls — important because _client() is hit on every backend op. Closes #1121. Closes #1132. Closes #1263. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`tool_diary_write` stored the `agent` metadata verbatim after `sanitize_name` (which preserves case), while `tool_diary_read` filtered by exact match — so writing as "Claude" and reading as "claude" silently returned zero rows. Both endpoints now lowercase `agent_name` immediately after sanitization. The default per-agent wing slug is also stable across casings since it's derived from the same normalized form. Behavior change: entries written prior to this fix under mixed-case agent names will not match the new lowercase filter; documented under v3.3.5 in CHANGELOG with a `mempalace repair` pointer. Adds a regression test (`test_diary_read_case_insensitive_agent`) and updates the existing `test_diary_write_and_read` to assert the new lowercase agent identity. Closes #1243 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI requires ruff format --check on the whole touched file. Pre-existing drift, no logic change.
CI requires whole-file format on touched files; pre-existing drift only.
…n Windows Windows runs treated `/tmp/elsewhere/x.md` as relative because Windows absolute paths require a drive letter, so `_classify_drawer` routed `drawer_out_of_scope` to `no_source` instead of `out_of_scope` and `test_dry_run_classifies_correctly` failed on test-windows. `Path(tmp_dir) / "elsewhere" / "x.md"` is absolute on every platform and still lives outside the project root that the synced_world fixture exposes via `repo_path`, so the bucket assertions hold cross-platform.
feat(sync): add gitignore-aware drawer prune (#1252)
docs: clarify contributor git identity setup
fix(miner): use token-boundary matching in detect_room
…k subprocess state
… lock subprocess state
…sw-flush fix(mcp): retry tool_search once on Chroma "Error finding id" transient (#1315)
test_palace_locks.py and test_chroma_collection_lock.py spawned child processes with the ``fork`` start method on POSIX. Under Python 3.13 this deadlocks reliably enough to hang the Linux 3.13 and macOS CI jobs indefinitely while Linux 3.9 / 3.11 / Windows complete normally. Root cause: by the time these tests run, the pytest parent process is multi-threaded — chromadb and onnxruntime both spawn background threads on import. ``fork`` snapshots the parent's address space into the child without those threads, so any lock another thread held at fork time stays locked in the child forever. Python 3.13 widened the window where Python's own internal threads can be holding locks (hence the new DeprecationWarning that fired ten times in our local 3.13 run). macOS hits a related but distinct issue: Apple's CoreFoundation explicitly forbids fork-without-exec; once anything in the parent has loaded a CF-using library (ONNX, anything via Objective-C bridges) a forked child will silently hang the moment it touches the same library. Switching to ``spawn`` re-imports modules in the child (~0.5s overhead per Process — measurable but bounded), which is the standard fix for both classes of bug. Lock-file semantics are unchanged: ``spawn`` inherits ``os.environ`` (including monkeypatched ``HOME``), which is all these tests need from the parent. Locally on Python 3.13: all 14 lock tests pass in 6.58s.
fix(tests): use spawn instead of fork for lock-test subprocesses
Several tests opened sqlite3 connections without try/finally or context-manager cleanup, relying on a flat conn.close() after the work. Any assertion failure or exception between connect and close leaked the connection until GC, producing ``ResourceWarning: unclosed database`` in CI logs. On Python 3.13 / macOS the ResourceWarning isn't just noise: a leaked connection can hold a SQLite advisory lock long enough for later test setup to block on it, which appears to be the cause of recent intermittent CI hangs on those two runners. Wrap each affected ``conn = sqlite3.connect(...)`` block in ``contextlib.closing(...)`` so cleanup runs on the failure path too. Mirrors the try/finally pattern already used in production code (searcher.py, repair.py, backends/chroma.py). No behavior change — same operations, same assertions, just deterministic cleanup. All 162 affected tests pass locally.
chore(tests): wrap sqlite3 connections in contextlib.closing
Bumps version 3.3.4 → 3.3.5 across pyproject.toml, version.py, plugin manifests, README badge, and uv.lock. Flips CHANGELOG.md from ``[3.3.5] — unreleased`` to ``[3.3.5] — 2026-05-09`` and adds entries for the four PRs that landed after the bug-fix block was authored: - Bug Fixes: #1396 (tool_search retry on transient HNSW flush) - Documentation: #1385 (CONTRIBUTING git-identity guidance, closes #1317) - Internal: #1431 (test multiprocessing fork → spawn) - Internal: #1430 (test sqlite connection lifecycle via contextlib.closing) The four open issues remaining on the v3.3.5 milestone (#1266, #1253, #1092, #1082) have been moved to v3.4 — they form the concurrent-writer / HNSW corruption cluster that needs deeper work than this cycle could absorb.
…etimes # Conflicts: # tests/test_palace_locks.py
fix(kg): accept ISO datetimes for temporal inputs
The 3.3.4 release shipped 2026-05-01 (per GitHub release v3.3.4) but its CHANGELOG header was never flipped from ``unreleased`` to the release date. Backfill while we're already touching CHANGELOG for the 3.3.5 cut.
chore(release): 3.3.5
# Conflicts: # .claude-plugin/marketplace.json # .claude-plugin/plugin.json # .codex-plugin/plugin.json # CHANGELOG.md # README.md # mempalace/version.py # pyproject.toml # uv.lock
Contributor
There was a problem hiding this comment.
Pull request overview
Promotes develop → main for the 3.3.5 release, including the version bumps and a large batch of functional fixes, new tooling (notably sync), test hardening, and documentation updates (including uv-first install guidance).
Changes:
- Bump version strings to 3.3.5 (package metadata + plugin manifests + lockfile) and publish a new 3.3.5 CHANGELOG section.
- Add/extend multiple reliability and integrity fixes: per-palace write locking + holder identity, KG temporal validation/compatibility, safer migrations, exporter symlink hardening, paginated closet regeneration, diary ingest hashing, etc.
- Refresh docs to recommend
uvworkflows and document the new MCP tool surface (30 tools, includingmempalace_sync).
Reviewed changes
Copilot reviewed 82 out of 84 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website/reference/mcp-tools.md | Updates MCP tool count/return shapes and documents new mempalace_sync. |
| website/reference/contributing.md | Switches contributor setup/testing docs to uv-based commands. |
| website/reference/benchmarks.md | Updates benchmark setup instructions to uv sync. |
| website/guide/getting-started.md | Recommends uv tool install for CLI installation. |
| website/guide/gemini-cli.md | Updates Gemini CLI setup instructions to use uv. |
| uv.lock | Updates editable mempalace version to 3.3.5. |
| tools/save.md | Adds /save tool doc for saving Claude Code sessions into MemPalace. |
| tools/render_jsonl.py | Adds a stdlib script to render Claude Code JSONL transcripts as readable text. |
| tools/find_orphan_claude_jsonls.sh | Adds a script to locate likely Claude JSONL transcript copies in backup locations. |
| tools/backup_claude_jsonls.sh | Adds a script to rsync Claude JSONLs into a backup directory. |
| tests/test_sources.py | Uses contextlib.closing for sqlite connections to avoid leaks. |
| tests/test_searcher.py | Adds tests for collection name plumbing and hybrid distance clamping invariants. |
| tests/test_palace_locks.py | Switches multiprocessing to spawn and expands lock behavior coverage (normalization/reentrancy/identity). |
| tests/test_miner.py | Adds routing regression tests, skip-filename/over-chunk-cap tests, and PID-slot cleanup tests. |
| tests/test_migrate.py | Adds coverage for write/delete round-trip probing and rebuild-from-sqlite decisioning. |
| tests/test_layers.py | Adds guards/tests for None metadata/documents and clamps similarity in search output. |
| tests/test_knowledge_graph.py | Adds temporal validation and datetime-vs-date compatibility tests plus KG connection cleanup tests. |
| tests/test_hybrid_candidate_union.py | Adds coverage for candidate_strategy="union" hybrid retrieval behavior. |
| tests/test_hnsw_payload_health.py | Adds tests around HNSW payload health/quarantine heuristics. |
| tests/test_hnsw_capacity.py | Adjusts capacity-status behavior for unflushed metadata and expands BM25 fallback tests. |
| tests/test_fact_checker.py | Adds Windows UTF-8 stdio reconfiguration tests for fact checker. |
| tests/test_exporter.py | Adds tests ensuring exporter refuses to follow pre-placed symlinks. |
| tests/test_entity_registry.py | Adds tests for atomic registry save behavior. |
| tests/test_config.py | Adds tests for ISO temporal validation behavior. |
| tests/test_closets.py | Adds diary ingest state hashing regression tests and legacy hash backfill coverage. |
| tests/test_closet_llm.py | Adds retry/pagination tests for closet regeneration and JSON decode retry behavior. |
| tests/test_chroma_collection_lock.py | Adds tests that ChromaCollection writes are serialized by the per-palace write lock. |
| tests/conftest.py | Resets MCP caches without forcing mcp_server import; closes cached KG connections when present. |
| tests/benchmarks/test_memory_profile.py | Updates benchmark patching to use _get_kg() instead of a module singleton. |
| tests/benchmarks/test_mcp_bench.py | Updates benchmark patching to use _get_kg() instead of a module singleton. |
| README.md | Updates install guidance to uv tool install and bumps version badge to 3.3.5. |
| pyproject.toml | Bumps project version to 3.3.5. |
| mempalace/version.py | Bumps __version__ to 3.3.5. |
| mempalace/sync.py | Adds gitignore-aware pruning (sync_palace) with reporting and optional apply mode. |
| mempalace/room_detector_local.py | Minor variable naming tweak in os.walk loop. |
| mempalace/palace.py | Makes collection name configurable by default; improves logging; adds re-entrant per-palace write lock with holder identity. |
| mempalace/palace_graph.py | Replaces silent exception swallowing with debug logging in one hydration path. |
| mempalace/normalize.py | Preserves exception chaining when rethrowing IO errors. |
| mempalace/miner.py | Adds token-boundary room matching; skips lockfiles; adds max-chunks-per-file guard; improves exception summary + PID-slot cleanup. |
| mempalace/migrate.py | Adds write/delete round-trip probe to detect “readable but not writable” migrated collections. |
| mempalace/layers.py | Guards against None docs/metas and clamps similarity to non-negative values. |
| mempalace/knowledge_graph.py | Adds ISO temporal validation, interval inversion rejection, and date-only vs datetime comparison compatibility. |
| mempalace/instructions/init.md | Updates init instructions to prefer PATH-visible mempalace --version + uv tool install. |
| mempalace/hooks_cli.py | Adds fully-detached spawn options, per-target PID slots, and a palace-root kill switch. |
| mempalace/fact_checker.py | Adds Windows stdio UTF-8 reconfigure wrapper + debug logging on KG lookup failures. |
| mempalace/exporter.py | Adds symlink refusal + safer file opening (O_NOFOLLOW when available). |
| mempalace/entity_registry.py | Implements atomic save via temp file + fsync + replace (+ parent dir fsync best-effort). |
| mempalace/diary_ingest.py | Switches diary skip logic to hash-based detection and rebuilds closets on content edits. |
| mempalace/dialect.py | Minor unused variable renaming. |
| mempalace/dedup.py | Minor unused variable renaming. |
| mempalace/convo_miner.py | Adds debug logging when stale drawer purge fails. |
| mempalace/config.py | Adds ISO temporal sanitizers and cached configured collection name. |
| mempalace/closet_llm.py | Rejects non-http(s) endpoints; retries JSON decode errors; paginates drawer fetches. |
| mempalace/_stdio.py | Adds shared Windows UTF-8 stdio reconfiguration helper. |
| integrations/openclaw/SKILL.md | Updates setup instructions to recommend uv tool install. |
| examples/gemini_cli_setup.md | Updates setup instructions to uv workflows. |
| CONTRIBUTING.md | Updates setup/testing commands to uv and adds git identity guidance. |
| CLAUDE.md | Updates dev commands to uv run ... equivalents. |
| CHANGELOG.md | Adds 3.3.5 release notes section and resolves changelog conflicts. |
| benchmarks/README.md | Updates benchmark setup to uv sync. |
| benchmarks/HYBRID_MODE.md | Updates benchmark setup to uv sync. |
| benchmarks/BENCHMARKS.md | Updates benchmark setup to uv sync and uv run examples. |
| .python-version | Pins local dev Python version to 3.12. |
| .gitignore | Ignores .envrc. |
| .github/workflows/deploy-docs.yml | Bumps actions/configure-pages major version. |
| .codex-plugin/README.md | Updates install guidance to ensure scripts land on PATH (uv tool / pip -e). |
| .codex-plugin/plugin.json | Bumps plugin version to 3.3.5. |
| .codex-plugin/hooks.json | Quotes hook script paths for safer execution. |
| .claude-plugin/skills/mempalace/SKILL.md | Updates install guidance to recommend uv tool install. |
| .claude-plugin/README.md | Clarifies init installs via uv tool or pip. |
| .claude-plugin/plugin.json | Bumps plugin version to 3.3.5. |
| .claude-plugin/marketplace.json | Bumps marketplace version to 3.3.5. |
| .claude-plugin/hooks/hooks.json | Quotes hook script paths for safer execution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **`miner.detect_room` bidirectional substring matching caused systemic misrouting.** The priority-1 (path parts) and priority-2 (filename) checks used `c in part or part in c` against room names + keywords, so any token that was an unbounded substring of a room name (or vice versa) matched. Priority-1 iterates left-to-right and returns on first match, so `views/billing-page/src/Foo.test.tsx` routed to an `interviews` room because `"views" in "interviews"` matched before reaching `billing-page`. Both call sites now use a `_name_matches` helper that compares names as equal or as separator-bounded tokens of each other (split on `-`, `_`, `.`, `/`). (#1004, closes #1002) | ||
| - **`mempalace compress` crashed on large palaces.** `regenerate_closets` fetched all closet_llm drawers in a single `col.get()`, which trips `SQLITE_MAX_VARIABLE_NUMBER` on palaces above ~32k drawers. Mirrors the #851 fix in `miner.py`: drawer fetch is now paginated at `batch_size=5000`. Per-source aggregation works across batches, so the LLM regeneration call still groups chunks correctly. (#1073, #1107) | ||
| - **CLI and `fact_checker --stdin` mojibaked non-ASCII content on Windows.** Python defaults `sys.stdin`/`stdout`/`stderr` to the system ANSI codepage (cp1252/cp1251/cp950), so `mempalace search > out.txt` and piped fact_checker invocations corrupted Cyrillic / CJK drawer text at the process boundary. New `mempalace/_stdio.py` helper reconfigures all three streams to UTF-8 on `sys.platform == "win32"`, with per-stream `errors` policy: `surrogateescape` on stdin (preserves bad bytes from redirected files for the consumer's parser), `replace` on stdout/stderr (substitutes U+FFFD instead of `UnicodeEncodeError`-ing mid-print). With this, all three user-facing console_scripts (`mcp_server`, `hooks_cli`, `cli`/`fact_checker`) now reconfigure identically on Windows. (#1282) | ||
| - **MCP knowledge-graph tools forwarded malformed date strings to SQLite.** `tool_kg_query` (`as_of`), `tool_kg_add` (`valid_from`), and `tool_kg_invalidate` (`ended`) accepted any string and produced empty result sets on natural-language inputs like `"March 2026"` or `"yesterday"` — callers (especially LLM agents) could not distinguish "no fact at this time" from "your date format was unrecognized." New `sanitize_iso_date()` validator in `config.py` accepts `YYYY`, `YYYY-MM`, `YYYY-MM-DD` (and passes through `None`/`""`); all three tools call it before values reach the storage layer. **Behavior change:** previously-silent date typos now raise a clear `ValueError` naming the offending field; full ISO-8601 with time (`YYYY-MM-DDTHH:MM:SS`, timezone offsets) is not yet accepted — file an issue if you have a use case. (#1164, #1167) |
Member
Author
There was a problem hiding this comment.
Good catch — fixed in a5ec325. Updated the bullet to describe the shipped behavior: sanitize_iso_temporal() accepts YYYY-MM-DD and canonical UTC datetimes (...Z / +00:00 normalized), rejects partial dates and naive/non-UTC datetimes. The original entry was authored when only #1167 had landed; #1417 (closes #1374) merged on 2026-05-10 and inverted the accepted/rejected sets. Backwards-compat wrapper sanitize_iso_date() is mentioned, and the partial-date behavior change from 3.3.4 is now called out explicitly.
Comment on lines
+405
to
+410
| try: | ||
| ident = f"{os.getpid()} {' '.join(sys.argv[:3])}".strip() | ||
| lock_file.seek(_LOCK_SENTINEL_BYTES) | ||
| lock_file.truncate(_LOCK_SENTINEL_BYTES + len(ident.encode("utf-8"))) | ||
| lock_file.write(ident) | ||
| lock_file.flush() |
Member
Author
There was a problem hiding this comment.
Confirmed — pre-existing on develop, out of scope for the release merge but the analysis is correct. Filed as #1435 with a repro plan and suggested fix (open with encoding="utf-8" and broaden handler to (OSError, UnicodeError)). Won't block this release.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates MemPalace to version 3.3.5, focusing on reliability, data integrity, and new maintenance tools. Key features include a sync command for gitignore-aware drawer pruning, a from-sqlite repair mode for recovering corrupt HNSW indexes, and support for ISO-8601 datetimes in the knowledge graph. The update also hardens the MCP server with lazy KG caching and improved transient error recovery. Feedback suggests increasing the retry count for JSON decoding errors in the LLM client to match HTTP error handling for better resilience.
Comment on lines
+172
to
+174
| if attempt < 2: | ||
| time.sleep(2**attempt) | ||
| continue |
Contributor
There was a problem hiding this comment.
The retry logic for JSONDecodeError seems inconsistent with the logic for HTTPError. For HTTPError, you retry up to two times (when attempt < 3), but for JSONDecodeError, you only retry once (when attempt < 2). A JSONDecodeError can also be caused by transient issues like a truncated response, so it might be beneficial to have the same number of retries for both cases for better resilience. Consider changing this to if attempt < 3: for consistency.
Suggested change
| if attempt < 2: | |
| time.sleep(2**attempt) | |
| continue | |
| if attempt < 3: | |
| time.sleep(2**attempt) | |
| continue |
Member
Author
There was a problem hiding this comment.
Reading the code on develop, all three handlers use attempt < 2:
for attempt in range(3): # 0, 1, 2
try:
...
except json.JSONDecodeError:
if attempt < 2: ...; continue # retries on 0, 1
except urllib.error.HTTPError as e:
if e.code in (429, 503) and attempt < 2: ...; continue # same
except Exception as e:
if "rate" in str(e).lower() and attempt < 2: ...; continue # sameSo all three already retry on attempts 0 and 1 (= 2 retries) and give up on attempt 2 (= 3 total tries). The retry counts are consistent — looks like the suggested change to attempt < 3 was based on a misread of the loop bound. Closing as not-an-issue.
Copilot review on PR #1434 caught that the existing 3.3.5 entry described the validator as it was authored under #1167 — accepting ``YYYY``/``YYYY-MM``/``YYYY-MM-DD`` and rejecting ISO datetimes — but PR #1417 (closes #1374) merged into develop on 2026-05-10 and inverted that: ``sanitize_iso_temporal()`` now rejects partial dates and accepts canonical UTC datetimes (``YYYY-MM-DDTHH:MM:SSZ`` / ``+00:00``). ``sanitize_iso_date()`` is kept as a backwards-compat wrapper. Update the bullet to describe the *shipped* behavior, name both functions, list both accepted and rejected forms, and call out the 3.3.4 → 3.3.5 behavior change for partial-date inputs that now error. Reference both #1167 (original) and #1374/#1417 (the expansion).
milla-jovovich
approved these changes
May 10, 2026
milla-jovovich
left a comment
milla-jovovich
left a comment
Collaborator
There was a problem hiding this comment.
Looks good @igorls will send some additions later this week.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Promotes
developtomainfor the 3.3.5 release. Mirrors the #1232 (3.3.4) flow.What's in 3.3.5
Bug Fixes (10 entries — full text in CHANGELOG.md):
tool_searchretry-once on transient HNSW flush errorsmempalace repair --mode from-sqliterecovers palaces stuck onapply_logsEntityRegistry.save()valid_to < valid_fromrejected at write timesanitize_iso_date()compressfor large palacesChromaBackend.close_palace()releases the SQLite lockminer.detect_roomtoken-boundary matching (closesminer.detect_roomsubstring matching causes systemic misrouting #1002)Documentation:
CONTRIBUTING.mdgit-identity guidance (closes process: PR #385 landed with a placeholder git identity — consider backfilling attribution or adding a note for tool-generated commits #1317)Internal (test reliability):
multiprocessingfork → spawn(fixes Linux 3.13 / macOS CI hang)contextlib.closingConflicts resolved
Three classes of conflict, all resolved trivially:
pyproject.toml,mempalace/version.py,uv.lock,README.mdbadge, all three plugin manifests): main was at 3.3.4, develop at 3.3.5 — took develop.2026-05-01(from GitHub's UTCpublishedAt), main had2026-04-30(the maintainer's canonical local date). Kept main's date.Milestone state
v3.3.5milestone: 84 closed / 0 openv3.4now holds the 4 carry-overs from the concurrent-writer / HNSW corruption cluster (HNSW pickle corruption after long-running mine: 'unsupported opcode' on next read; repair/migrate cannot recover #1266, PreCompact hook: double-ingest race condition causes HNSW corruption (no palace-wide write lock) #1253, Concurrent writers (hooks + MCP server + CLI) corrupt the palace on chromadb 1.5.8 — sparse-file bloat + SIGSEGV #1092, MCP tool_search returns "Error finding id" when wing-scoped to a convos-mined wing (CLI works, unscoped MCP works) #1082)Test plan
tests/test_version_consistency.py→ passing (cross-checksversion.pyvspyproject.toml)