Release v3.4.0 — promote develop to main - #1706
Merged
Merged
Conversation
Two bugs found in production use with a ChromaDB palace of 1200+ drawers ingested via mixed paths (bulk import + MCP tool calls): 1. searcher.py: filtered search (wing= or room=) crashes with "Error finding id" when the HNSW vector index is out of sync with the SQLite metadata store. The outer try/except swallowed the error as a search failure. Fix: inner try/except catches filter failures, retries unfiltered with n_results*15 (capped at 500), and post-filters by wing/room in Python. Degrades gracefully instead of returning an error. 2. mcp_server.py: diary_write requires 'entry' but add_drawer uses 'content', making it natural to pass content= by analogy. The mismatch returns a silent MCP -32000 error with no explanation. Fix: accept 'content' as an alias for 'entry' with a clear error message if neither is provided. Both bugs were diagnosed and patched in a live palace. This contributes the fixes upstream.
The scam-alert and Claude-Code-retention admonitions were the first content visitors saw on the repo page — louder than the project introduction. Moves both below the logo/title/badges so the project identity reads first, and softens the scam block (drops the H1 "CRITICAL SECURITY WARNING" + all-caps shouting + redundant emoji) to a single-paragraph CAUTION. All factual content preserved: impostor-domain warning, official sources, malware caveat, link to docs/HISTORY.md.
…aths The COCA content-word filter shipped in PR #1605 imported `_get_coca_filter` and `_candidate_entity_words` locally inside two hot paths: - `palace.build_closet_lines` — runs per source file during mine - `miner._extract_entities_for_metadata` — runs per drawer during mine Both imports are now at module top, where they're resolved once at import time instead of on every per-drawer call. Module-top imports also make the dependency graph visible to static analysis (pylint's C0415 was flagging the locals). No behavior change. The `_get_coca_filter()` call is unchanged — only the import statement moved. End-to-end mining produces identical chromadb output. Addresses the MEDIUM finding gemini-code-assist raised on PR #1605 review. Verification: full pytest 2258 passed / 3 skipped / coverage 85.35%. ruff check + format clean. Linux Py 3.9 / 3.11 / 3.13 via CI-matching `pip install -e ".[dev]"`: 2249 passed each. End-to-end mine of a test corpus produces the expected drawer + closet pointer.
…omic
Adds a curated list of multi-word product/system names ("Claude Code",
"GitHub Copilot", "Visual Studio Code", "GPT-4", …) and a compound
pre-pass that detects them atomically before the existing single-word
extraction runs. Without this, the regex-based detector decomposes
"Claude Code" into "Claude" + "Code" — and the COCA filter (shipped
in v3.3.6) then drops "Code" as a content word, leaving "Claude" alone
with the wrong attribution.
What ships
- mempalace/data/known_systems.json — 59 curated compounds covering
common AI assistants, IDEs, model names, cloud platforms, and
Office/Google apps. Each entry is multi-word or hyphenated;
single-word product names ("ChatGPT", "Cursor") have no
decomposition risk and stay handled by the existing regex.
- mempalace/entity_detector.py — _get_known_systems() (cached loader,
mirrors _get_coca_filter from Tier 2) and _apply_known_systems_prepass
which scans for each compound case-insensitively with word boundaries,
counts occurrences, and returns the masked text + count dict so the
subsequent single-word + multi-word loops don't re-decompose.
- mempalace/miner.py and mempalace/palace.py — same pre-pass wired
into _extract_entities_for_metadata (per-drawer tagger) and
build_closet_lines (closet pointer construction). Without these,
the new behavior would only apply at init-time and per-drawer
metadata would still decompose compounds.
How it interacts with Tier 2
Tier 2 (COCA filter) blocks single-word content nouns like "Code"
and "Brutal". Tier 3 protects multi-word product names so they
don't get decomposed in the first place. They complement each
other: the compound pre-pass runs FIRST and masks compounds out
of the text; the COCA filter then runs on the remaining
single-word candidates.
Behavior verification
Before this PR, mining a document containing "Claude Code wrote the
patch" three times emitted entities:
Claude;Claude Code;Code (Code filtered by COCA);
After this PR, the same document emits:
Claude Code
The standalone "Claude" no longer appears (it never actually appeared
alone in the source) and decomposition stops at the compound boundary.
Tests
Nine new tests in tests/test_entity_detector.py covering:
- "Claude Code" detected as atomic compound at extract_candidates
- "Claude" alone NOT in results when only mentioned as part of compound
- Case-insensitive compound matching (claude code, CLAUDE CODE, etc.)
- Single-word "Code" still filtered by COCA (no Tier 2 regression)
- Single-word real name "Aya" still detected (no regression on names)
- Multiple distinct compounds in one text both detected
- Unknown two-word phrase still detected via existing multi-word regex
- known_systems.json ships with expected schema (>=20 entries, all multi-token)
- known_systems.json contains expected high-value entries
Verification
Full pytest 2267 passed / 3 skipped on macOS, coverage 85.34%.
Linux Py 3.9 / 3.11 / 3.13 via CI-matching pip install -e ".[dev]":
2258 passed each. End-to-end mine of a compound-rich corpus
confirms chromadb entities metadata now shows compounds atomic
(Claude Code, GPT-4, GitHub Copilot, Visual Studio Code) with no
decomposition.
Addresses gemini-code-assist MEDIUM finding on PR #1613: the previous implementation of _apply_known_systems_prepass compiled a regex pattern for every compound on every call, repeating the work on every drawer mined and every closet built. With 59 compounds × N drawers, that's 59N re.compile() calls for a workload where the patterns never change. The fix moves compilation into _get_known_systems (already lru_cache'd to size=1), which now returns tuple[tuple[str, re.Pattern], ...] — pairs of (canonical name, pre-compiled case-insensitive word-bounded regex). _apply_known_systems_prepass consumes the cached tuple and does zero compilation in the hot path. Behavior is identical: same word boundaries, same case-insensitive matching, same longest-first ordering, same graceful-degrade on malformed json. All 74 entity_detector tests still pass on macOS plus the full 2258-test suite on Linux Py 3.9 / 3.11 / 3.13.
…below-header docs(readme): move scam/retention alerts below header, soften tone
perf(miner,palace): hoist COCA filter imports out of per-drawer hot paths
Resolves conflicts in miner.py and palace.py against the COCA-hoist work from #1612 just merged. Both files had local imports for _get_coca_filter / _candidate_entity_words in the function bodies; after #1612 those moved to module top. Resolution: hoist _apply_known_systems_prepass alongside _get_coca_filter at module top in both files, drop the now-empty local imports. No behavior change; just removes the local-import duplication that would have been left in place if the merge auto-resolved naively.
…mpound-matcher feat(entity): known-systems lexicon keeps multi-word product names atomic
Brings in the three commits that landed on main during the v3.3.6 release cycle but not on develop: - 62a555c fix(release): align ruff pin to 0.15.14 + hoist COCA imports out of hot paths (#1614) - a1cf052 Merge pull request #1614 from MemPalace/fix/release-3.3.6-ruff-pin-align - db1fbe8 Merge pull request #1610 from MemPalace/release/3.3.6 Resolves conflicts in miner.py and palace.py where develop's #1613 (known-systems lexicon, Tier 3) had added _apply_known_systems_prepass to module-top imports + the prepass call in build_closet_lines. Main side had only the COCA hoist from #1614. Resolution: keep develop's side (the prepass additions). Mirrors the v3.3.5 sync-back pattern (#1442).
chromadb <= 1.5.8 writes config_json_str = '{}' (empty JSON) when
creating collections. chromadb 1.5.9 introduced a strict _type check
in the collection config deserialization path -- its absence raises
KeyError: '_type' on palace open. Since the pin allows >=1.5.4,<2,
any upgrade pulls 1.5.9 and breaks every existing palace.
Add a fourth pre-open migration step (_fix_missing_collection_type)
that injects "_type": "CollectionConfigurationInternal" into
collections.config_json_str rows that lack it. Same lifecycle and
marker-file pattern as the existing _fix_blob_seq_ids.
Co-Authored-By: nautis <nautis@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review feedback: `with sqlite3.connect() as conn:` only manages transactions, it does not close the connection. An open connection before PersistentClient instantiation can leave WAL state. Use explicit `try...finally: conn.close()` matching the read-only helpers elsewhere in the module. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chore: sync main back to develop after v3.3.6 release
… for ChromaDB 1.5.x compatibility ChromaDB 1.5.x calls embedding_function.embed_query(input=...) via keyword argument during collection.query(). EmbeddinggemmaONNX lacked both embed_query and embed_documents methods, causing: TypeError: embed_query() got an unexpected keyword argument 'input' whenever semantic search was triggered. This patch adds the two methods required by the ChromaDB EF protocol, using (the ChromaDB kwarg name, noqa A002) so that palace search works correctly with the embeddinggemma model. Also downloads the companion ONNX file alongside the main model to prevent runtime InferenceSession failures. Fixes silent search failures when is set to embeddinggemma.
Bumps [ruff](https://github.com/astral-sh/ruff) from 0.15.14 to 0.15.15. - [Release notes](https://github.com/astral-sh/ruff/releases) - [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md) - [Commits](astral-sh/ruff@0.15.14...0.15.15) --- updated-dependencies: - dependency-name: ruff dependency-version: 0.15.15 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…has multiple parent_drawer_id mining passes
Under the additive-mining model (drawer history preserved across re-mines),
a single source_file can have multiple parent_drawer_id groups in the
palace, one per mining pass. Each group carries its own stored
source_mtime and normalize_version.
`file_already_mined` previously used `collection.get(where={"source_file": X},
limit=1)` in the `extract_mode is None` branch and only checked the single
returned row's metadata. ChromaDB's `get(..., limit=1)` has no ordering
guarantee across multiple matching rows, so the returned row was effectively
arbitrary. When ChromaDB happened to return a stale group (older mining
pass with a different stored mtime), the function returned False, the
additive miner concluded the file had changed, and wrote yet another
duplicate group of drawers for a file that had not actually changed since
the last successful mine.
Steady-state failure: for any source_file that has ever been edited (so
that the palace contains groups with different stored mtimes), each re-mine
has a probability of spuriously concluding "file changed" and writing
another duplicate group. Each spurious group compounds the problem because
its stored mtime can also trigger the next spurious re-mine. Storage grows
without bound; search results, hallway counts, and entity-frequency stats
become inflated proportionally.
The fix mirrors the paginated-iteration pattern already used in the
`extract_mode is not None` branch — iterate every drawer for the
source_file in 1000-row pages, short-circuit on the first matching group.
A correct group is one that passes the existing checks: normalize_version
not stale; if check_mtime, stored source_mtime within 0.001 seconds of the
current file mtime. The two branches (extract_mode is None vs set) collapse
into one loop that skips the extract_mode check when no extract_mode was
specified.
Trade-off: average-case cost rises from O(1) limit=1 query to O(N/1000)
paginated scan. For typical sources (1-3 groups) the cost is unchanged
because the loop short-circuits on the first matching group within the
first page. For pathological sources with thousands of groups, the cost
is O(number-of-pages-until-match) — still bounded, no longer flaky.
RED test pins the failure space deterministically
`test_file_already_mined_handles_multiple_groups_under_one_source_file`
uses a MockCollection that simulates two parent_drawer_id groups under one
source_file: a STALE group (older mtime) and a CURRENT group (matching
mtime). The mock returns the STALE group when called with limit=1
(worst-case ChromaDB ordering) and returns BOTH groups when called with
limit=1000 (what a correctly-iterating implementation must do). Test asserts
file_already_mined returns True even when limit=1 picks the stale group.
- Against pre-fix code: test FAILS (function returns False because
limit=1 picks stale group, mtime mismatch returns False)
- Against post-fix code: test PASSES (iteration finds the current group,
short-circuits to True)
Verified RED-then-GREEN locally. Existing 4 file_already_mined tests in
tests/test_miner.py continue to pass:
- test_file_already_mined_check_mtime
- test_file_already_mined_scopes_convo_extract_mode
- test_file_already_mined_extract_mode_paginates_large_sources
- test_file_already_mined_returns_false_for_stale_normalize_version
Verification
- macOS Python 3.12 (local) full pytest : 2268 passed, 0 failed
- Linux Python 3.9.25 (OrbStack) : 2260 passed, 0 failed
- Linux Python 3.11.15 (OrbStack) : 2261 passed, 0 failed
- Linux Python 3.13.13 (OrbStack) : 2261 passed, 0 failed
- ruff check + ruff format --check : all clean
Provenance
Surfaced during the per-query audit on the PR #1628 amendment cycle (the
search for every bare `where={"source_file": ...}` query in the repo).
One of six sites identified. The other five are legitimately file-global
in intent (closet purges, full-rebuild deletes, paginated mode-filtered
scans). This site is the one whose failure mode mirrors the cross-group
stitching pattern PR #1628 fixed at the searcher layer.
…mtime-groups fix(palace): file_already_mined iterates all groups when source_file has multiple parent_drawer_id mining passes
build(deps-dev): bump ruff from 0.15.14 to 0.15.15
fix(backends): repair missing _type in collection config (#1611)
…eights + tokenizer) The EmbeddinggemmaONNX lazy-load now fetches the ONNX external-weights file (model.onnx_data) in addition to the model graph and tokenizer, so a single warm-up issues 3 downloads, not 2. The lazy-load-once invariant is unchanged (InferenceSession and Tokenizer.from_file are still each built exactly once).
Windows exports of Claude Code JSONL sessions prepend a UTF-8 BOM (\xef\xbb\xbf). With encoding='utf-8', json.loads() raises JSONDecodeError on the first line, _try_claude_code_jsonl silently skips every line, and the file falls through as raw text — losing all structured message content. utf-8-sig strips the BOM transparently and is backward-compatible with BOM-free files on all platforms.
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 #681 affects closet_llm.py and cli.py. Replace with ASCII equivalents: [OK], [FAIL], [!], and hyphen.
_chunk_by_exchange stripped every line, joined them with single spaces, and silently dropped blank lines. That violated the verbatim-always principle stated in CLAUDE.md and contradicted the function's own docstring, which claimed 'The full AI response is preserved verbatim.' Concrete consequences before this change: - paragraph breaks fused: 'para1\n\npara2' → 'para1 para2' - list items fused: '1. a\n2. b' → '1. a 2. b' - code fences destroyed: indented code collapsed to a single line - search quality degraded because tokenization changed at ingest Fix is surgical: keep each line as-is, join on newline, trim only trailing newlines produced by the loop stopping at the next '>' turn. The fallback path _chunk_by_paragraph has a narrower version of the same bug (it strips each paragraph); that is out of scope here and left for a follow-up.
…st (#1579) _HNSW_BLOAT_GUARD set batch_size and sync_threshold to 50,000 to prevent link_lists.bin sparse-file bloat in pre-1.5.x Python chromadb (#344). chromadb >=1.5.4 Rust bindings do not exhibit that bloat. The 50k guard meant any mine under 50,000 drawers never triggered chromadb's _persist(), leaving index_metadata.pickle absent and link_lists.bin empty. quarantine_stale_hnsw then renamed the segment on every cold open after a 300s mtime gap, accumulating .drift-* directories indefinitely. Lower both thresholds to 2 (empirical Rust-side minimum; 1 is rejected with InvalidArgumentError) so any mine of 2+ drawers triggers a natural persist. Verified: batch_size=2 with 20k records produces link_lists.bin at 171 KB with zero sparse-file inflation. Existing palaces retain the old 50k thresholds in their collection metadata until the user runs repair --mode from-sqlite. Co-Authored-By: Tim Harmon <tim-harmon@users.noreply.github.com>
… data size chromadb pre-allocates data_level0.bin at index creation (~168 KB for 384-dim embeddings) regardless of record count, so the previous data-size-vs-floor heuristic in _segment_appears_healthy could not distinguish a single-record segment (sub-threshold, never persisted) from an interrupted persist. Restructure _segment_appears_healthy: when index_metadata.pickle is absent, check link_lists.bin instead of data_level0.bin size. Empty or absent link_lists + absent metadata = sub-threshold (never persisted). Non-empty link_lists + absent metadata = interrupted persist. Co-Authored-By: 0xKingVee9527 <0xWinner98@users.noreply.github.com>
…it reconnect (#1573) The _quarantined_paths gate fired once per palace per process and never re-armed after external in-place writes (closet_llm, mine, compress) that drift HNSW segments. The MCP server path (make_client static) had zero discard logic -- quarantine never re-armed even on inode change. Extend the discard guard in _client() from inode_changed-only to inode_changed or mtime_changed or mtime_appeared. Add a guarded discard in mcp_server._get_client() before make_client(), and an unconditional discard in tool_reconnect(). Remove dead _auto_repair / palace-daemon comment (does not exist in this codebase) and correct misleading _get_collection retry-path comments that overclaimed quarantine re-runs. Co-Authored-By: C-LaForest <C-LaForest@users.noreply.github.com>
Prevent redundant quarantine re-run when a fresh ChromaBackend instance opens a palace that was already quarantined by another instance in the same process. The mtime_appeared transition (cached 0.0 -> real mtime) now only triggers a discard if the instance previously tracked the path, distinguishing genuine file appearance from first-access default. Addresses gemini-code-assist review on PR #1602. Co-Authored-By: C-LaForest <C-LaForest@users.noreply.github.com>
Drawer/closet/triple IDs built by concatenating strings without a
delimiter before hashing can collide: hash((s1 + str(i1))) ==
hash((s2 + str(i2))) whenever s1+str(i1) == s2+str(i2). Concretely,
source_file="foo" + chunk_index=12 produces the same hash input as
source_file="foo1" + chunk_index=2 — both yield "foo12". Under
ChromaDB's primary-key constraint the second write silently
overwrites the first; one chunk's content is lost without surfacing
an error (the surrounding broad `except Exception:` blocks swallow
any backend complaint).
The defect class is "string + string concat fed to a hash without
a delimiter." Per the styleguide's partial-scope-key-migration rule,
every site sharing the pattern is a candidate and must be triaged —
not just the ones the original issue named.
FIX — 6 sites
- mempalace/miner.py:1253 drawer_id, add_drawer() back-compat
- mempalace/miner.py:1386 drawer_id, batched mine loop
- mempalace/miner.py:1416 drawer_ids, closet-build re-derivation
- mempalace/format_miner.py:643 drawer_id, format_miner batched loop
- mempalace/mcp_server.py:1136 drawer_id, tool_add_drawer
- mempalace/knowledge_graph.py:305 triple_id, KG triple insertion
MIGRATED FOR CONSISTENCY — 2 sites
- mempalace/convo_miner.py:87 sentinel_key — was `:`, now `|`
- mempalace/convo_miner.py:422 drawer_key — was `:`, now `|`
Pre-existing delimiter was `:`. Migrated because (a) source_file can
contain literal `:` on Windows paths (`C:\Users\…`) and URL-like
sources (`https://host:8080`), weakening `:` as a separator; (b) `|`
is reserved in Windows filenames so source_file cannot contain it,
making it strictly safer.
DELIMITER CHOICE
- `|` chosen over `:` (Windows / URL source_file edge cases)
- `|` chosen over `\x00` (NUL breaks log greppability + print()
debug + downstream string handling for marginal extra safety)
- Matches the established precedent in mempalace/diary_ingest.py
(lines 52, 76, 91, 98 — all already on `|`)
EXEMPT — audited and correct as-is
Single-input hashes (nothing to delimit):
- mempalace/miner.py:1432 closet_id (source_file only)
- mempalace/format_miner.py:559 sentinel_id (source_file only)
- mempalace/palace.py:433 lock filename (source_file only)
- mempalace/palace.py:629 palace_key (lock_key_source only)
- mempalace/diary_ingest.py:158 content_hash (text only)
- mempalace/hooks_cli.py:329 pidfile digest (joined cmd only)
- mempalace/sources/context.py:141 record digest (source_file only)
Already correctly delimited:
- mempalace/hallways.py:157 `f"{wing}::{a}::{b}"` (`::`)
- mempalace/palace_graph.py:454 `f"{a}↔{b}"` (`↔`)
- mempalace/diary_ingest.py:52,76,91,98 (`|` precedent)
Protected by composition (uniqueness guaranteed by the ID prefix,
not by the hash slice):
- mempalace/mcp_server.py:1635 entry_id is
`diary_{wing}_{strftime('%Y%m%d_%H%M%S%f')}_{sha[:12]}`.
Microsecond-resolution timestamp prefix supplies uniqueness;
the trailing hash is a content-discriminator, not the
write-time uniqueness guarantor.
NEW MODULES
- mempalace/ids.py — single source of truth for ID construction.
Five helpers (make_drawer_id_from_chunk, make_drawer_id_from_content,
make_convo_drawer_id, make_convo_sentinel_id, make_triple_id) plus
an ID_RECIPE = "v2" constant.
- mempalace/collision_scan.py — pre-mining defense. Runs immediately
before each batched ChromaDB upsert; raises CollisionError naming
the colliding (source_file, chunk_index) pairs if any proposed
drawer_id appears more than once with conflicting metadata across
the union of incoming and existing rows.
DETECTION
ChromaDB's primary-key constraint means past collisions were
silently resolved at upsert time (last-write-wins), erasing
evidence. A post-hoc audit cannot reconstruct what was lost from
palace state alone. Therefore:
- Pre-mining risk scan. Before each batched upsert, compute the
proposed drawer_ids for the incoming chunk set AND query existing
drawer_ids from the collection. If any proposed id appears more
than once in the union (incoming-vs-incoming or incoming-vs-
existing) with conflicting (source_file, chunk_index), abort the
mine with an actionable error naming the colliding pairs.
Collision is caught BEFORE it destroys data, which is the only
point at which palace state still carries the evidence.
- New metadata key: `"id_recipe": "v2"` on every drawer written
under the delimited recipe. Audits compare like-for-like;
drawers without `id_recipe` are treated as v1 legacy (undelimited
or `:`-delimited), not as collisions.
- Honest disclosure: palaces mined under any pre-v2 mempalace may
carry silent past collisions whose original content is
unrecoverable from palace state. Future library tier work will
give users a per-drawer audit + opt-in archival path.
TESTS
- tests/test_ids.py: 17 unit tests covering all 5 helpers, the
ID_RECIPE constant, the private `_delimited_sha256` helper, and
the four defect-class collision shapes (chunk_index boundary,
content boundary, extract_mode boundary, ISO datetime boundary).
RED collision tests confirm the v2 recipe breaks the defect class.
- tests/test_collision_scan.py: 10 tests covering clean batches,
idempotent re-mines, incoming-vs-incoming collisions, incoming-vs-
existing collisions, error-message quality, empty batches,
metadata without chunk_index, and ChromaDB backend errors
propagating cleanly.
- tests/test_convo_miner_unit.py: FakeCol gains a stub `get()` so
the pre-mining scan can probe an empty in-test collection.
BACKWARDS COMPATIBILITY
- Existing v1 drawers remain queryable; no rewrite of stored IDs.
- New mining passes write v2 drawers alongside v1 via PR #1628's
additive-mining model.
- No user action required; opt-in cleanup ships separately.
VERIFICATION
- macOS Python 3.12 (local): 2304 passed, 3 skipped, 0 failed,
coverage 85.44% (above 80% threshold).
- Linux Python 3.9/3.11/3.13 (OrbStack with full `pip install -e
'.[dev]'` flow): see PR body for per-version pass counts.
- `ruff check .` + `ruff format --check .`: clean.
- pylint score: ids.py 10.00/10, collision_scan.py 10.00/10; all
modified files >= 8.92 (no regression).
- bandit: clean on all new files; the one MEDIUM finding in
knowledge_graph.py is on lines 385/407 (pre-existing SQL string
construction in timeline queries), not in code touched by this PR.
- pre_push_check.sh: ALL CHECKS PASSED.
Refs: deferred from PR #1572 (Copilot finding #13). Styleguide audit
documented above: 8 fix/migration + 11 enumerated exempt sites.
test_load_config_uses_defaults_when_yaml_missing asserted the derived wing equals project_root.name. That only held when the random tempfile name had no separators; tempfile's alphabet includes '_', so once normalize_wing_name strips leading/trailing '_' (this PR), a name like 'tmpXXXX_' makes the derived wing diverge from the raw name. Compare against normalize_wing_name(project_root.name) — the actual contract — which is deterministic across platforms. (Surfaced as a test-windows failure on this PR, but it was cross-platform flaky.)
fix(mcp): repair diary_write content alias + restore -32602 diagnostic (fixes red develop)
Follow-up to the wing-name normalization (#1675). Palaces built before the rule filed drawers under leading/trailing-separator wing names (e.g. a Claude Code path-encoded dir `-home-user-proj` -> `_home_user_proj`); the new derivation strips those, so searches/diary reads under the new name miss the old memories — the history is split, not lost. `migrate_wing_names` (CLI: `mempalace migrate-wings [--dry-run] [--yes]`) re-keys the `wing` metadata field on drawers and closets to the normalized form, merging collisions. Design (verified against the codebase): - Drawer/closet IDs embed the wing as an opaque prefix that is never decoded back into a wing, and mining idempotency keys on `source_file`, so IDs are left untouched: closet ->drawer_id pointers stay valid and future mining still skips already-mined files. No risky extract-rebuild needed. - `topics_by_wing` registry keys are re-keyed (merging on collision). - Tunnels resolve via existing read-time normalization and need no rewrite. - Backend-agnostic (uses the get/update collection abstraction); idempotent; dry-run-able. The strip is applied by the migration itself, so it is correct regardless of whether the running build's normalize_wing_name already carries the #1675 change. Tests: pure planner coverage (strip, no-op, empty/non-string, collision) + hermetic backend integration (relabel, merge, dry-run, idempotency).
feat(migrate): mempalace migrate-wings — normalize legacy wing names (#1675 follow-up)
fix(config): strip leading/trailing separators in normalize_wing_name
Minor release. Highlights since v3.3.6: - feat: pluggable vector backends (qdrant, pgvector, sqlite_exact) - feat: Docker image for MCP server + CLI - feat: mempalace migrate-wings (normalize legacy wing names) - feat: known-systems lexicon for compound product names - fix: embeddinggemma works with ChromaDB 1.5.x (default-model search) - fix: drawer_id collision data-loss; WAL import side-effect / kill-switch - ci: PyPI Trusted Publishing pipeline
Enforced by test_readme_claims.py::TestVersionBadge — a sixth version location outside the version-guard 5-file set.
chore(release): 3.4.0
![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 introduces pluggable storage backends to MemPalace, adding support for sqlite_exact, qdrant, and pgvector alongside the default chroma backend, while also updating the CLI, MCP server, and conversation miner. Key feedback from the review highlights several critical issues: string-to-list conversion bugs in embedding_wrapper.py that split single strings into characters, an unsupported text_any match operator in the Qdrant backend, a type-checking bug in collision_scan.py that bypasses safety checks for non-Chroma backends, a missing tempfile import in migrate.py, and a performance bottleneck from repeated row.keys() calls in chroma.py.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Contributor
There was a problem hiding this comment.
Pull request overview
Promotes the codebase to v3.4.0 and lands the release set on main, including the new pluggable backend architecture (chroma/pgvector/qdrant/sqlite_exact), improved CLI/backend selection + mismatch protection, new migrations, Docker packaging, and release automation.
Changes:
- Bump version manifests to 3.4.0 and add a Trusted Publishing GitHub Actions workflow for PyPI releases.
- Add/expand pluggable backend support (registry + mismatch detection + lexical capability + embedding wrapper for explicit-vector backends), plus extensive conformance/regression tests.
- Add Docker (CPU + GPU) images and docs/runbooks; add recovery tooling for wing-name normalization.
Reviewed changes
Copilot reviewed 66 out of 67 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_sqlite_exact_backend.py | New conformance/regression coverage for sqlite_exact backend behavior (filters, persistence, mismatch, lexical, ranking). |
| tests/test_pgvector_backend.py | New fake-client + tests for pgvector backend contracts (filters, marker, isolation, live test). |
| tests/test_palace.py | Adds coverage for unknown backend selection surfacing as a CLI state message. |
| tests/test_miner.py | Adds regressions for status fast-path, sqlite tally correctness, and file_already_mined additive-mining behavior. |
| tests/test_migrate_wings.py | Tests for wing-name normalization migration planning + integration behavior. |
| tests/test_mcp_server.py | Adds MCP regressions for backend-specific status fields, retry-on-stale-index, reconnect semantics, WAL kill-switch safety, diary_write aliasing. |
| tests/test_ids.py | New tests pinning collision-safe ID recipe v2 and delimiter-based hashing behavior. |
| tests/test_hooks_cli.py | Verifies stop-hook diary checkpoints are discoverable under harness agent identities (#1693). |
| tests/test_hallways.py | Updates mocks to support hallways pagination fetch shape. |
| tests/test_hallways_pagination.py | Regression test ensuring hallways computation paginates instead of where-get on large wings (#1619). |
| tests/test_entity_detector.py | Adds Tier-3 compound lexicon tests (known multi-word systems like “Claude Code”, “GitHub Copilot”). |
| tests/test_embeddinggemma.py | Updates lazy-load expectations to include ONNX weights sidecar. |
| tests/test_dedup.py | Updates dedup tests to mock get_collection instead of ChromaBackend directly. |
| tests/test_convo_miner_unit.py | Adds regressions for preserving AI response newlines/blank lines and updates a test double for collision scans. |
| tests/test_config.py | Adds backend config/env resolution tests and backend persistence validation. |
| tests/test_collision_scan.py | New tests for pre-upsert collision detection behavior and error quality. |
| tests/test_cli.py | Adds coverage for --backend flag propagation and MCP command output including backend. |
| tests/test_clean_lone_surrogates.py | Adds chokepoint tests ensuring document sanitization prevents batch drops on lone surrogates. |
| tests/test_backend_conformance.py | Adds shared isolation conformance suite over local backends. |
| tests/conftest.py | Extends cache reset to include new MCP server cache fields. |
| tests/_backend_conformance.py | Shared assertion helpers for backend isolation contract (RFC 001). |
| README.md | Reorders security notices, adds Docker usage docs, documents backend selection, bumps version badge. |
| pyproject.toml | Bumps version, registers backend entry-points, adds pgvector extra, bumps ruff pin. |
| mempalace/version.py | Updates __version__ to 3.4.0. |
| mempalace/searcher.py | Refactors search to use backend-aware open helpers; adds lexical “union” via backend capability + better errors/fallbacks. |
| mempalace/palace.py | Adds backend resolution/mismatch protection, explicit-backend env, embedding wrapper for explicit-vector backends, and improves state messages. |
| mempalace/normalize.py | Uses utf-8-sig reading; hardens tool_use formatting when input is list. |
| mempalace/miner.py | Adds collision scanning + v2 ID recipe usage; improves entity extraction with known-systems prepass; adds status sqlite fast path. |
| mempalace/migrate.py | Adds wing-name migration planner + executor + topics_by_wing rekeying. |
| mempalace/knowledge_graph.py | Switches KG triple ID creation to centralized IDs helper. |
| mempalace/ids.py | New centralized ID construction helpers using a delimiter-based v2 recipe. |
| mempalace/hooks_cli.py | Fixes stop-hook diary checkpoint attribution via harness→agent mapping; threads agent_name through direct diary save. |
| mempalace/hallways.py | Switches hallway drawer fetch to paginated scan + client-side wing filter to avoid SQLite variable limits. |
| mempalace/format_miner.py | Switches to centralized drawer ID helper + collision scan; tags id_recipe in metadata. |
| mempalace/entity_detector.py | Adds known-systems lexicon loader + compound prepass to avoid decomposing product compounds. |
| mempalace/embedding.py | Ensures ONNX sidecar weights are downloaded; adds embed_query/embed_documents protocol methods. |
| mempalace/dedup.py | Routes dedup through palace.get_collection (backend-aware) instead of ChromaBackend directly. |
| mempalace/data/known_systems.json | Adds curated compound lexicon data file for Tier-3 entity detection. |
| mempalace/convo_miner.py | Switches convo IDs to centralized helpers; adds collision scan; preserves AI response line structure verbatim. |
| mempalace/config.py | Adds backend default/config/env accessors, qdrant/pgvector settings, backend persistence, and wing normalization stripping separators. |
| mempalace/collision_scan.py | Adds pre-upsert collision detection utility and CollisionError formatting. |
| mempalace/closet_llm.py | Normalizes output markers (ASCII-safe) for dry-run/ok/fail lines. |
| mempalace/cli.py | Adds global/subcommand --backend, persists backend on init, guards Chroma-only maintenance commands, adds migrate-wings command. |
| mempalace/backends/registry.py | Adds multi-backend detection helpers and registers built-in backends in registry. |
| mempalace/backends/embedding_wrapper.py | New wrapper to compute embeddings locally for explicit-vector backends. |
| mempalace/backends/base.py | Extends backend/collection contract: mismatch/capability errors, lexical search types, isolation contract docs, dict-compat shim for typed results. |
| mempalace/backends/init.py | Exposes new backends, types, and detection utilities. |
| docs/RELEASING.md | Adds PyPI Trusted Publishing runbook. |
| docs/recovery/wing-name-migration.md | Documents symptoms + recovery steps for legacy wing-name split after normalization. |
| Dockerfile.gpu | Adds CUDA multi-stage build for GPU embeddings via uv-managed environment. |
| Dockerfile | Adds CPU multi-stage build + uv lockfile install flow. |
| docker-entrypoint.sh | Adds entrypoint dispatch between MCP server and CLI. |
| docker-compose.yml | Adds compose configuration for stdio MCP server and persisted /data volume. |
| .github/workflows/publish.yml | Adds build + tag/manifest verification + Trusted Publishing gated PyPI publish workflow. |
| .github/workflows/docker-publish.yml | Adds GHCR docker publish workflow (multi-arch) + GPU Dockerfile build validation. |
| .dockerignore | Adds ignore rules to keep Docker build context minimal. |
| .codex-plugin/plugin.json | Version bump to 3.4.0. |
| .claude-plugin/plugin.json | Version bump to 3.4.0. |
| .claude-plugin/marketplace.json | Version bump to 3.4.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
EmbeddingCollection did _embed_texts(list(documents)). For ChromaDB's
OneOrMany shape, a bare str document splits into per-character 'docs'
(list("abc") -> ['a','b','c']), embedding each character and breaking
length alignment with ids/metadatas on explicit-vector backends
(pgvector, sqlite_exact). Normalize str -> [str] via _as_list() at all
four sites (add/upsert/update documents, query query_texts) and pass the
normalized list to the inner backend too. Addresses PR #1706 review
(Gemini + Copilot, HIGH).
GitHub doesn't reliably emit a release event when a draft tied to a pre-existing tag is published (e.g. publishing the long-standing v3.3.6 draft fired no run). Add a workflow_dispatch trigger with a 'tag' input so a maintainer can run the publish for any existing tag from the Actions tab or 'gh workflow run'. A resolve step picks the release tag or the input, format-validates it (^v[0-9][0-9A-Za-z.+-]*$) before any use as a git ref, and the existing on-main + version-match checks + the pypi approval gate apply unchanged to both paths.
Address Copilot review on #1708: - Check out refs/tags/<tag> instead of the bare name, so a same-named branch can't be resolved instead of the tag object (checkout prefers branches on ambiguous refs). - Tighten the dispatch-input validation to require vMAJOR.MINOR.PATCH with an optional -/+ suffix, rejecting loose values like v3 / v3foo.
ci(publish): add workflow_dispatch manual trigger
Address #1707 review: - _as_list also wraps a bare dict (single metadata) — list({'k':1}) -> ['k'] would drop the values — and returns list inputs as-is (no copy, Copilot perf note); other iterables are materialized once. - Normalize ids and metadatas (not just documents) in add/upsert/update so a scalar id/metadata stays length-aligned with documents/embeddings. - Widen query_texts annotation to list[str] | str to match the behavior. - Tests: bare-str ids + dict metadatas, dict wrapping, list-returned-as-is.
fix(backends): wrap bare-str OneOrMany inputs before embedding
…andoff The two-job split passed the wheel from the build job to the publish job via upload/download-artifact, which failed repeatedly with BlobNotFound on the same-run download (GitHub artifact-service flake) — the PyPI upload step never ran. Collapse into a single `publish` job that builds and publishes in the same workspace, removing the handoff entirely. The job keeps the `pypi` environment gate, OIDC (id-token: write), refs/tags checkout, and the on-main + version-manifest checks.
ci(publish): build + publish in one job (fix BlobNotFound artifact handoff)
milla-jovovich
approved these changes
Jun 6, 2026
milla-jovovich
left a comment
milla-jovovich
left a comment
Collaborator
There was a problem hiding this comment.
@igorls you have made many people very happy today with all this work. thank you⚡️
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.
Promotes
develop(now at 3.4.0) tomainto cut the v3.4.0 release. 83 commits since v3.3.6.After this merges, the release flow is:
main→ triggerspublish.yml→ approve thepypienv gate →3.4.0rc1on PyPI.pip install mempalace==3.4.0rc1in a clean env and build a palace to verify the published artifact end-to-end.Highlights since v3.3.6
mempalace migrate-wings— normalize legacy wing names (withdocs/recovery/wing-name-migration.md)-32602diagnosticpublish.yml) +docs/RELEASING.mdrunbookPre-flight: all 5 manifests + README badge agree at 3.4.0;
mempalace-mcpentry point aligned.