Conversation
- repair.py: wrap upsert loop in try/except; restore from backup on failure instead of leaving a partially rebuilt collection - migrate.py: replace non-atomic rmtree+move with rename-aside swap so a crash between the two calls does not destroy both copies - cli.py: use offset += len(batch["ids"]) with empty-batch guard instead of fixed offset += batch_size to prevent skipping drawers
agent_name and entry are validated via sanitize_name/sanitize_content, but topic is stored raw into ChromaDB metadata. Apply the same sanitize_name guard to reject null bytes, path traversal, and oversized payloads.
- repair.py: define backup_path before the conditional block so it is always in scope when the except handler references it - migrate.py: restore old palace from .old if both os.rename and shutil.move fail during the swap step
Bumps [actions/deploy-pages](https://github.com/actions/deploy-pages) from 4 to 5. - [Release notes](https://github.com/actions/deploy-pages/releases) - [Commits](actions/deploy-pages@v4...v5) --- updated-dependencies: - dependency-name: actions/deploy-pages dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/upload-pages-artifact](https://github.com/actions/upload-pages-artifact) from 3 to 5. - [Release notes](https://github.com/actions/upload-pages-artifact/releases) - [Commits](actions/upload-pages-artifact@v3...v5) --- updated-dependencies: - dependency-name: actions/upload-pages-artifact dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Fixes four issues causing silent hook failures: 1. **Relative paths** → Absolute paths (/absolute/path/to/hooks/...) Claude Code resolves hooks from working directory, not repo root. 2. **Wrong matcher** → Stop uses *, PreCompact has no matcher PreCompact doesn't use matcher (only Stop hooks do). 3. **Missing timeout** → Added timeout: 30 to both hooks Matches hooks/README.md specification. 4. **Ambiguous target** → Specified ~/.claude/settings.local.json Clarified global vs project-scoped config. Also added executable chmod instructions and path replacement note. Fixes #1037
shutil.move() can partially create palace_path before raising, which would trip a bare os.replace(stale_path, palace_path) rollback (dest exists). - Switch the primary swap to os.replace so same-filesystem moves stay atomic - Branch on errno.EXDEV before falling back to shutil.move, so real errors (permissions, EIO) surface instead of silently attempting a slow copy - Extract rollback into _restore_stale_palace which clears any partial destination and, if the restore itself fails, logs both stale_path and palace_path so the operator can recover by hand Adds three regression tests covering clean rollback, partial-copy cleanup, and logged failure on rollback-failure. Flagged by the Qodo reviewer on #935.
Enable setup-python's built-in pip cache on all CI jobs to avoid re-downloading ~300 MB of dependencies (chromadb, onnxruntime, hnswlib) on every run. Bump macOS and Windows from Python 3.9 to 3.11 -- Linux matrix already covers 3.9 compatibility, and 3.11 is faster on these platforms.
~/.mempalace/tunnels.json (introduced in #790) was created via plain open(..., "w") with no chmod, and its parent dir via os.makedirs() without mode=0o700. On Linux with default umask 022 both end up world-readable (0o644 / 0o755). Tunnels reveal cross-wing connections — which projects, people, and rooms the user has explicitly linked — so they are sensitive metadata that should not be readable by other local users on shared systems. Apply the same 0o700 / 0o600 pattern that #814 established for the other sensitive palace files. Chmod calls are wrapped in try/except (OSError, NotImplementedError) for Windows / unsupported-filesystem compatibility. Closes #1165
The miner upserted one drawer per ChromaDB call, paying tokenizer + ONNX session setup per chunk. The embedding device was CPU-only because no EmbeddingFunction was ever wired through the backend. Two changes, each a speedup in its own right; stacked they give ~10x end-to-end on a medium corpus (20 files, 568 drawers): 1. Batched upsert. `process_file` and `_file_chunks_locked` now collect all chunks of a file into a single `collection.upsert(...)` so the embedding model runs one forward pass per file instead of N. 2. Hardware-accelerated embedding function. New `mempalace/embedding.py` wraps `ONNXMiniLM_L6_V2` with configurable `preferred_providers`. `MEMPALACE_EMBEDDING_DEVICE` (or `embedding_device` in config.json) selects auto / cpu / cuda / coreml / dml. Unavailable accelerators log a warning and fall back to CPU. The factory subclasses `ONNXMiniLM_L6_V2` and spoofs its `name()` to `"default"` so the persisted EF identity matches existing palaces created with ChromaDB's bare `DefaultEmbeddingFunction` -- same model, same 384-dim vectors, no rebuild needed when turning GPU on. `ChromaBackend.get_collection` / `create_collection` now pass the resolved EF on every call so miner writes and searcher reads agree. Benchmarks (i9-12900KF + RTX 3090, medium scenario, 568 drawers): per-chunk + CPU 19.77s · 29 drw/s (baseline) batched + CPU 8.07s · 70 drw/s (2.4x) batched + CUDA 2.15s · 264 drw/s (9.2x) Reproducible via `benchmarks/mine_bench.py`. Install paths: pip install mempalace[gpu] # NVIDIA CUDA pip install mempalace[dml] # DirectML (Windows) pip install mempalace[coreml] # macOS Neural Engine Mine header now prints `Device: cpu|cuda|...` so users can confirm the accelerator engaged.
Agent-Logs-Url: https://github.com/MemPalace/mempalace/sessions/3213a67a-6871-4bb2-9ae0-23fa11001a22 Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MemPalace/mempalace/sessions/3213a67a-6871-4bb2-9ae0-23fa11001a22 Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MemPalace/mempalace/sessions/3213a67a-6871-4bb2-9ae0-23fa11001a22 Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MemPalace/mempalace/sessions/3213a67a-6871-4bb2-9ae0-23fa11001a22 Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MemPalace/mempalace/sessions/3213a67a-6871-4bb2-9ae0-23fa11001a22 Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
perf(mining): batch per-chunk upserts + optional GPU acceleration
When two wings have one or more confirmed TOPIC labels in common, the miner now drops a symmetric tunnel between them at mine time so the palace graph reflects shared themes (frameworks, vendors, recurring concepts). - llm_refine: TOPIC label routes to a dedicated `topics` bucket so the signal survives confirmation instead of getting collapsed into `uncertain` and dropped. - entity_detector / project_scanner: bucket plumbed through the detection pipeline; `confirm_entities` returns confirmed topics alongside people/projects. - miner.add_to_known_entities: optional `wing` parameter records the confirmed topics under `topics_by_wing` in `~/.mempalace/known_entities.json`. Wing names do NOT leak into the flat known-name set used by drawer-tagging. - palace_graph: `compute_topic_tunnels` and `topic_tunnels_for_wing` create symmetric tunnels via the existing `create_tunnel` API so they share dedup and persistence with explicit tunnels. - miner.mine: post-file-loop pass calls `topic_tunnels_for_wing` for the freshly-mined wing. Failures are logged but never abort the mine. - config: `topic_tunnel_min_count` knob (env `MEMPALACE_TOPIC_TUNNEL_MIN_COUNT` or `~/.mempalace/config.json`), default 1. Tests cover topic persistence through init->mine, tunnel creation when wings share a topic, no tunnel below threshold, cross-wing tunnel retrieval via `list_tunnels`, dedup on recompute, case-insensitive overlap, and the end-to-end mine-time wiring. Out of scope for this PR (called out in the PR body): manifest- dependency overlap, per-topic allow/deny lists, search-result surfacing.
… field
Previously a cross-wing topic tunnel for "Angular" stored the room as
"Angular" — colliding with a wing's literal folder-derived "Angular" room
at follow_tunnels/list_tunnels read time, and exposing raw topic strings
(which may contain characters rejected by sanitize_name) to the MCP
surface.
Topic tunnels now store their room as "topic:<original-casing>" and carry
kind="topic" on the stored dict. Explicit tunnels get kind="explicit"
(default). follow_tunnels("wing", "Angular") on a literal Angular room
no longer surfaces topic connections for the same name, and any LLM
scanning list_tunnels has a visible discriminator.
feat(graph): cross-wing tunnels by shared topics (#1180)
chore: add OpenArena owner claim verification file
Three tightly-coupled search-quality fixes for v3.3.3: 1. CLI `mempalace search` now routes through the same `_hybrid_rank` the MCP path already used. Drawers whose text contains every query term but embed as file-tree noise (directory listings, diffs, log fragments) were scoring cosine distance >= 1.0 — the display formula `max(0, 1 - dist)` then floored every result to `Match: 0.0`, with no way for the user to tell a lexical match from a total miss. BM25 catches these cleanly; the display surfaces both `cosine=` and `bm25=` so users see which component is firing. 2. Legacy-palace distance-metric warning. Palaces created before `hnsw:space=cosine` was consistently set silently use ChromaDB's default L2 metric, which breaks the cosine-similarity formula (L2 distances routinely exceed 1.0 on normalized 384-dim vectors). The search path now detects this at query time and prints a one-line notice pointing at `mempalace repair`. Only fires for legacy palaces; new palaces already set cosine correctly. 3. Invariant tests pinning `hnsw:space=cosine` on every collection- creation path — legacy `get_or_create_collection`, legacy `create_collection`, RFC 001 `get_collection(create=True)`, the public `palace.get_collection`, and a round-trip through reopen. Locks down the correctness that new-user palaces already have so a future refactor can't silently regress it. Also adds a `metadata` property to `ChromaCollection` so callers can read the underlying hnsw:space without reaching into `_collection`. Tests: - New regression: simulate three candidates at distance 1.5 (cosine=0), one containing query terms — must rank first with non-zero bm25. - New: legacy metric (empty or non-cosine) produces stderr warning. - New: correctly-configured palace produces no warning. - New: all five creation paths pin cosine metadata. All existing tests still pass.
`test_fresh_palace_via_full_stack_gets_cosine` used `tempfile.Temporary-
Directory()` as a context manager, which tries to delete the temp path
on exit. On Windows, ChromaDB still holds SQLite file handles to
`chroma.sqlite3` when the context closes, producing:
PermissionError: [WinError 32] The process cannot access the file
because it is being used by another process: '...\\chroma.sqlite3'
NotADirectoryError: [WinError 267] The directory name is invalid
Other tests in the same file use pytest's `tmp_path` fixture, which
defers cleanup to session end (when the process is exiting and the
file-lock contention is moot). Align this one with the rest of the
file.
CLAUDE.md already documents the 80% Windows coverage allowance due to
"ChromaDB file lock cleanup" — the fix is to stop fighting the lock.
fix(search): CLI hybrid rerank, legacy-metric warning, invariant tests (3.3.4)
`mempalace init` now ends with a `Mine this directory now? [Y/n]` prompt and runs `mine()` in-process when accepted; `--yes` skips the prompt and auto-mines for non-interactive callers. Declining prints the resume command. Removes the "remember to type the next command" friction since rooms + entities just got set up. `mempalace mine` now wraps its main loop in `try / except KeyboardInterrupt` and prints `files_processed`, `drawers_filed`, and `last_file` before exiting with code 130 on Ctrl-C. Re-mining is safe because deterministic drawer IDs make the upsert idempotent. The hooks PID lock at `~/.mempalace/hook_state/mine.pid` is now actively removed in a `finally` when its entry points at us, on clean exit, error, or interrupt — preventing the next hook fire from briefly waiting on a stale PID. Closes #1181, #1182.
…ore mine prompt
Reviewer feedback on the previous commit flagged two real problems:
1. Overloading --yes to also auto-mine was a silent behaviour change for
scripted callers. Today --yes only auto-accepts entities — making it
ALSO trigger a multi-minute ChromaDB write breaks every script that
currently runs `mempalace init --yes <dir>` for the fast non-interactive
entity path. Add a separate `--auto-mine` flag instead. Combinations:
mempalace init --yes <dir> # entities auto, STILL prompt mine
mempalace init --auto-mine <dir> # prompt entities, skip mine prompt
mempalace init --yes --auto-mine <dir> # fully non-interactive
--yes behaviour is now identical to pre-PR.
2. The mine prompt was firing without telling the user how big the job
was. On a real corpus mine takes minutes-to-tens-of-minutes; hitting
Enter on default-Y with no size cue is a footgun. Show a one-line
estimate computed from scan_project (the same walk we hand into mine)
BEFORE the prompt:
~423 files (~12 MB) would be mined into this palace.
Mine this directory now? [Y/n]
The estimate uses a single corpus walk: scan_project's output is
passed into mine() via a new optional files= kwarg, so we never walk
the tree twice.
Tests: replaced the old "--yes auto-mines" assertion with a regression
guard that --yes alone STILL prompts; added coverage for --auto-mine
alone, --yes --auto-mine together, and the pre-prompt estimate line.
The "Skipped. Run mempalace mine <dir>" hint after declining the init prompt and the "Re-run mempalace mine <dir> to resume" hint after a Ctrl-C interruption both interpolated project_dir without shell-quoting. A path containing spaces or metacharacters produced a copy-paste-broken command. Both spots now use shlex.quote(project_dir). Adds regression tests covering each hint with a path that contains a space.
The pre-existing test_maybe_run_mine_prompt_declined_prints_hint
asserted the bare unquoted form `mempalace mine {tmp_path}`. After
the production code switched to shlex.quote on the resume hint, this
passed on Linux/macOS (POSIX paths have no characters that trigger
quoting) but failed on Windows where backslashes always get wrapped
in single quotes.
Mirror the production code in the assertion via shlex.quote so it's
portable across platforms; do the same for the two new
spaces-in-path tests for consistency.
fix(hooks): pass --mode convos when mining Claude Code transcript dirs
…review) Address Copilot review on #1231: 1. Stop double-mining the transcript on the Python side. ``_get_mine_targets`` now returns only the ``MEMPAL_DIR`` projects target — the convos target for the transcript dir is dropped because ``_ingest_transcript`` already handles it on every hook fire. The duplicate spawn was using ``sys.executable`` (vs ``_mempalace_python()``) and a different ``--wing``, so each Stop/PreCompact event was writing the same transcript into two wings under asymmetric interpreters and overwriting the single ``_MINE_PID_FILE`` lock. 2. ``_maybe_auto_ingest`` and ``_mine_sync`` now spawn via ``_mempalace_python()`` so the resolved interpreter matches the venv that owns mempalace (matters under GUI-launched harnesses where ``sys.executable`` may resolve to a system Python without chromadb). 3. Replace ``eval $(...)`` in both shell hooks with a ``mapfile``-based reader. Sanitized values are still emitted by the same Python parser, but the shell now does plain variable assignment instead of executing the parser's stdout — smaller blast radius if the sanitizer is ever bypassed. 4. Mirror ``_validate_transcript_path`` in the shell hooks via a ``is_valid_transcript_path`` helper — extension + traversal-segment rejection, parity with the Python validator. The convos mine in each shell hook is now gated on the validator instead of bare ``-f``. 5. Tighten the ``..`` traversal test that previously exercised the suffix gate by mistake (``../../etc/passwd`` lacks ``.json[l]``). Use ``.jsonl`` paths with traversal segments to actually hit the ``..`` rejection branch. 6. README: add a one-liner pointing at ``mempalace sweep`` for users who want per-message recall on top of the file-level chunks the hooks produce. The sweeper was undiscoverable previously. Tests: 1418 passed, 1 skipped (full suite minus benchmarks).
`bash` on the Windows CI runner resolves to `wsl.exe` which fails with "Windows Subsystem for Linux has no installed distributions." The shell hooks themselves are POSIX-only — Windows users use the Python entry point — so the bash-subprocess exercise is non-applicable on win32. The static-grep validator tests still run on every platform, so the shell-side validation is still asserted under Windows; only the live bash invocation is skipped.
fix(hooks): always mine the active transcript as convos, additive to MEMPAL_DIR
Sets `hnsw:batch_size` and `hnsw:sync_threshold` to 50_000 at every collection-creation call site: * `mempalace/backends/chroma.py` — `get_collection(create=True)` and the legacy `create_collection()` path. Preserves existing `hnsw:space`, `hnsw:num_threads=1` (race fix from #976), and `**ef_kwargs` (embedding-function plumbing from a4868a3). * `mempalace/mcp_server.py` — the direct `client.get_or_create_collection` path used when a palace is first opened by the MCP server. Without this third site, MCP-bootstrapped palaces would skip the guard and could still trigger the original bloat. Without these defaults, mining ~10K+ drawers triggers ~30 HNSW index resizes and hundreds of persistDirty() calls. persistDirty uses relative seek positioning in link_lists.bin; accumulated seek drift across resize cycles causes the OS to extend the sparse file with zero-filled regions, each cycle compounding the next. Result: link_lists.bin grows into hundreds of GB sparse, after which `status`, `search`, and `repair` all segfault and the palace is unrecoverable. Empirical: rebuilt a palace from scratch on 39,792 drawers across 5 wings with this fix applied. Final palace 376 MB, link_lists.bin stays at 0 bytes across both Chroma collection dirs, status and search both return cleanly. Same workload without the fix bloated the palace to 565 GB sparse (30 GB on disk) and segfaulted at ~15K drawers. Migration note: chromadb 1.5.x exposes a `collection.modify(configuration={"hnsw": {...}})` retrofit path for already-created collections (`UpdateHNSWConfiguration`), but this PR doesn't pursue it — by the time link_lists.bin has bloated the index is already corrupt and the only known recovery is a fresh mine. Tests assert both keys land on the persisted collection metadata in both `ChromaBackend` code paths, which also covers the #1161 "config silently dropped" concern at CI time. A separate smoke test was used to verify the metadata round-trips through `chromadb.PersistentClient` reopen on chromadb 1.5.8. Closes #344 Supersedes #346 Co-authored-by: robot-rocket-science <robot-rocket-science@users.noreply.github.com>
The BLOB-seq_id migration shim (PR #664) ran int.from_bytes(..., 'big') over every BLOB in max_seq_id, including chromadb 1.5.x's own native format (b'\x11\x11' + 6 ASCII digits). That conversion yields a ~1.23e18 integer that silently suppresses every subsequent embeddings_queue write for the affected segment (queue filter is seq_id > start), causing silent drawer-write drops after a 1.5.x upgrade. Two-part fix: 1. Shim narrowing (mempalace/backends/chroma.py) - Drop max_seq_id from the shim loop. chromadb owns that column's format; we don't reinterpret it. - Defense-in-depth: skip rows in embeddings whose seq_id BLOB has the sysdb-10 b'\x11\x11' prefix rather than misconvert. 2. Recovery command (mempalace/repair.py, mempalace/cli.py) - mempalace repair --mode max-seq-id [--segment <uuid>] [--from-sidecar <path>] [--dry-run] [--yes] [--no-backup] - Detects poisoned rows via threshold (seq_id > 2**53). - Default heuristic: MAX(embeddings.seq_id) over the collection owning the poisoned segment. Matches METADATA max exactly; VECTOR segments get a few seq_ids ahead (queue skips an already-indexed window — an acceptable loss vs. resetting to 0 and re-processing everything). - --from-sidecar copies clean values from a pre-corruption sqlite db. - Backs up chroma.sqlite3, closes chroma handles, atomic UPDATEs, post-repair verification that raises MaxSeqIdVerificationError if any row is still above threshold. Tests: 8 new in tests/test_repair.py (detection, heuristic, sidecar, dry-run, segment filter, no-op, backup, rollback-on-verify-failure). 3 new in tests/test_backends.py (max_seq_id untouched by shim, sysdb-10 prefix skipped in embeddings, legacy big-endian u64 BLOBs still convert). Full suite: 1103 passed.
The narrowed _fix_blob_seq_ids returned early when safe_rows was empty, but #1177's marker contract requires the marker to be written on every successful pass — even no-op — so subsequent opens skip the sqlite3 connection entirely. Without this, palaces that have no genuine 0.6.x BLOBs but DO have sysdb-10-prefixed rows would re-open sqlite3 on every call, defeating the #1090 corruption guard. Restructured the conditional so the marker write is unconditional after a successful sqlite scan, regardless of whether any rows were updated. Surfaced by test_fix_blob_seq_ids_writes_marker_when_already_integer during the develop-rebase of this PR. The author's branch predates the marker contract from #1177 (merged 2026-04-26), so this is a rebase-edge fix-up rather than a logic change to their narrowing behaviour.
…henated dirs (#1194) `init` was recording `topics_by_wing[<raw-dirname>]` while `mempalace.yaml` got the lower-cased separator-collapsed slug. At mine time the miner read the slug from the yaml and missed the registry key, so `_compute_topic_tunnels_for_wing` returned 0 silently for every project whose folder contained a `-` or a space — the most common shape in the wild. Extracted the rule into `config.normalize_wing_name()` and routed both `cli.cmd_init` (registry write) and `room_detector_local.detect_rooms_local` (yaml write) through it. Added a regression test in `test_cli.py` asserting the registry call uses the normalized slug, plus four direct unit tests for the helper. Refs #1180. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…1194) Two follow-ups against the review on this PR: 1. ``miner.load_config`` no-yaml fallback was returning the raw dirname as the wing, while ``cmd_init`` writes ``topics_by_wing`` under the normalized slug. A hyphenated project mined without a ``mempalace.yaml`` file silently lost every topic tunnel — same key-miss class as #1194, just down the no-yaml branch (raised by Qodo on this PR). 2. ``convo_miner`` was applying the lower/replace rule inline at one call site. Now folded through ``normalize_wing_name`` so all wing-slug producers — ``cmd_init``, ``room_detector_local``, ``miner.load_config`` fallback, ``convo_miner`` — share a single source of truth. No behavior change for any input; pure consolidation. Added ``test_load_config_no_yaml_normalizes_hyphenated_wing`` to lock the fallback path to the normalized slug — fails on develop without the miner change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cmd_init now invokes ``_run_pass_zero`` unconditionally (#1221, #1223 landed on develop after this PR's branch point). The pass reads sample content via ``builtins.open``; with that mocked to MagicMock, the downstream ``"\\n\\n".join(samples)`` in ``corpus_origin.detect_origin_heuristic`` raises ``TypeError: expected str instance, MagicMock found``. This test only cares about the wing-slug write to the registry, so stub the pass-zero call directly rather than try to satisfy its full sample-gathering contract.
``def _normalize_wing(wing: str | None) -> str | None`` uses PEP 604 union syntax which requires Python 3.10+ at runtime. The project still declares ``python_requires=">=3.9"`` and CI runs the test-linux (3.9) matrix, where every test in ``tests/test_palace_graph*`` errors out before collection with ``TypeError: unsupported operand type(s) for |``. Added ``from __future__ import annotations`` so all annotations in this module are evaluated lazily as strings — the union syntax is then accepted on 3.9 without needing to rewrite to ``Optional[str]``. Surfaced after rebasing this PR onto current develop.
…unnels fix(graph): normalize wing slug at init so topic tunnels fire for hyphenated dirs (#1194)
…tunnels fix(tunnels): normalize wing names in topic tunnel lookup for hyphenated dirs
fix: narrow `_fix_blob_seq_ids` + add `repair --mode max-seq-id`
fix: prevent HNSW index bloat from resize+persist cycles
Bumps every version source from 3.3.3 to 3.3.4: - pyproject.toml - mempalace/version.py (canonical) - .claude-plugin/plugin.json - .claude-plugin/marketplace.json - .codex-plugin/plugin.json - README.md badge Dates the CHANGELOG section and adds entries for the bug fixes that landed this cycle (#1135, #1191, #1230, #1231) plus expands the #1194 entry to credit the lookup-side recovery path from #1197. Pre-tag verification: - 1441 passed, 1 skipped (full suite minus benchmarks, all platforms) - ruff check + format clean - 44/44 in test_version_consistency + test_readme_claims (6-file sync) - JPH invariant: pyproject.toml + .claude-plugin/plugin.json both reference mempalace-mcp - Wheel build + fresh-venv install: mempalace --version reports 3.3.4, mempalace-mcp --help works (catches the v3.3.2-class regression)
Contributor
There was a problem hiding this comment.
Pull request overview
Release prep for v3.3.4, syncing version sources and updating release notes while bundling a large set of reliability/features work around mining, search, hooks, tunnels, repair, and MCP safety.
Changes:
- Bump package/version references to 3.3.4 and date/update the CHANGELOG for the release.
- Improve operational robustness: per-palace mine locking, HNSW divergence probing + sqlite fallback, safer migration rollback, hook behavior fixes/validation, and search hybrid rerank + legacy-metric warning.
- Add/expand features and supporting tests: cross-wing topic tunnels, embedding-device selection (GPU/CoreML/DML), corpus-origin detection scaffolding/tests, plus several new invariants/regression suites.
Reviewed changes
Copilot reviewed 60 out of 61 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_searcher.py | Adds regression tests for BM25 None docs + CLI hybrid rerank + legacy-metric warning. |
| tests/test_save_hook_mines.py | Tightens shell hook transcript mining expectations and validates transcript-path guards. |
| tests/test_repair.py | Adds truncation-safety guard tests and max-seq-id repair coverage. |
| tests/test_project_scanner.py | Updates detected-entity dict shape to always include topics. |
| tests/test_palace_locks.py | New cross-process tests for per-palace non-blocking mine lock + back-compat alias. |
| tests/test_palace_graph_tunnels.py | Adds permissions regression + topic tunnels + wing normalization tests for tunnel helpers. |
| tests/test_palace_graph.py | Ensures graph build skips None metadata safely. |
| tests/test_miner.py | Adds wing normalization test, bounded upsert batching tests, topic tunnel E2E tests, and Ctrl‑C handling tests. |
| tests/test_migrate.py | Adds tests for robust rollback helper when migration swap fails. |
| tests/test_llm_refine.py | Updates expectations for TOPIC routing into a dedicated topics bucket. |
| tests/test_llm_client.py | Adds tests for provider endpoint “external service” heuristic + Tailscale CGNAT handling. |
| tests/test_known_entities_registry.py | Adds topics_by_wing persistence/behavior tests for tunnel signal source. |
| tests/test_hooks_cli.py | Updates hook CLI tests for new mine-target semantics and transcript validation. |
| tests/test_hnsw_capacity.py | New suite for HNSW capacity probe + BM25-only sqlite fallback + status output behavior. |
| tests/test_entity_detector.py | Updates detector empty/missing-file shape to include topics. |
| tests/test_embedding.py | New tests for embedding provider resolution, warning behavior, and EF caching. |
| tests/test_corpus_origin.py | New TDD suite for corpus-origin heuristic + LLM-confirmation parsing behavior. |
| tests/test_convo_miner_unit.py | Adds test coverage for bounded upsert batching in convo mining. |
| tests/test_config.py | Adds config tests for embedding device normalization and wing normalization helper. |
| tests/test_collection_metric_invariant.py | New invariant tests ensuring all collection creation paths set hnsw:space=cosine. |
| tests/test_cli.py | Adds tests for wing normalization at init and init→mine prompt/flags behavior. |
| tests/conftest.py | Resets ChromaBackend quarantine state between tests to avoid leakage. |
| pyproject.toml | Bumps version to 3.3.4 and adds optional deps for GPU/CoreML/DML onnxruntime variants. |
| openarena-claim.txt | Adds OpenArena owner-claim verification token. |
| mempalace/version.py | Bumps canonical __version__ to 3.3.4. |
| mempalace/searcher.py | Adds None-safe tokenization, CLI hybrid BM25 rerank, legacy metric warning, and sqlite BM25 fallback. |
| mempalace/room_detector_local.py | Routes wing slug creation through normalize_wing_name. |
| mempalace/project_scanner.py | Adds topics bucket to detected shape and threads corpus-origin context into discovery/refine. |
| mempalace/palace_graph.py | Adds wing normalization in tunnel helpers, logging warnings, topic tunnels, and restricts tunnels.json permissions. |
| mempalace/palace.py | Introduces mine_palace_lock non-blocking per-palace lock + mine_global_lock alias. |
| mempalace/migrate.py | Makes palace swap safer (rename aside + rollback helper) with EXDEV handling. |
| mempalace/mcp_server.py | Adds HNSW probe-driven vector-disable flag, sqlite status fallback, and threads HNSW tuning into collection creation. |
| mempalace/llm_refine.py | Routes TOPICs into topics bucket and adds corpus-origin prompt preamble support. |
| mempalace/llm_client.py | Adds local-vs-external endpoint heuristic used for privacy warnings. |
| mempalace/hooks_cli.py | Refactors auto-ingest into explicit (dir, mode) targets and uses the correct interpreter for spawned mines. |
| mempalace/entity_detector.py | Adds topics bucket, corpus-origin persona reclassification, and updates confirm_entities to return topics. |
| mempalace/embedding.py | New embedding function factory supporting hardware acceleration with provider-based caching. |
| mempalace/convo_miner.py | Adds bounded upsert batching and wing normalization for convo mines. |
| mempalace/config.py | Adds normalize_wing_name, embedding_device, and topic-tunnel threshold config plumbing. |
| hooks/mempal_save_hook.sh | Makes transcript mining explicit (--mode convos), makes MEMPAL_DIR additive, adds transcript validator, removes eval-based parsing. |
| hooks/mempal_precompact_hook.sh | Same as save hook: transcript parsing/validation and dual-target mining with correct modes. |
| hooks/README.md | Documents new additive MEMPAL_DIR semantics and always-on transcript mining. |
| examples/HOOKS_TUTORIAL.md | Updates Claude Code hook config guidance and executable-path expectations. |
| benchmarks/mine_bench.py | New benchmark script comparing unbatched vs batched mining and device impact. |
| README.md | Updates version badge and adds note about periodic sweep for per-message recall. |
| CLAUDE.md | Clarifies “local-first” stance as “no external API by default” with BYOK support. |
| CHANGELOG.md | Adds dated 3.3.4 section with added items and bug-fix notes. |
| .github/workflows/version-guard.yml | Updates checkout action major version reference. |
| .github/workflows/deploy-docs.yml | Updates checkout/pages actions major versions. |
| .github/workflows/ci.yml | Adds pip caching and updates action versions + windows/macos python versions. |
| .codex-plugin/plugin.json | Bumps plugin version to 3.3.4. |
| .claude-plugin/plugin.json | Bumps plugin version to 3.3.4. |
| .claude-plugin/marketplace.json | Bumps marketplace version to 3.3.4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
333
to
+360
| @@ -281,33 +344,215 @@ def search(query: str, palace_path: str, wing: str = None, room: str = None, n_r | |||
| print(f" Room: {room}") | |||
| print(f"{'=' * 60}\n") | |||
|
|
|||
| for i, (doc, meta, dist) in enumerate(zip(docs, metas, dists), 1): | |||
| similarity = round(max(0.0, 1 - dist), 3) | |||
| meta = meta or {} | |||
| for i, hit in enumerate(hits, 1): | |||
| vec_sim = round(max(0.0, 1 - hit["distance"]), 3) | |||
| bm25 = hit.get("bm25_score", 0.0) | |||
| meta = hit["metadata"] | |||
| source = Path(meta.get("source_file", "?")).name | |||
| wing_name = meta.get("wing", "?") | |||
| room_name = meta.get("room", "?") | |||
|
|
|||
| print(f" [{i}] {wing_name} / {room_name}") | |||
| print(f" Source: {source}") | |||
| print(f" Match: {similarity}") | |||
| print(f" Match: cosine={vec_sim} bm25={bm25}") | |||
| print() | |||
| # Print the verbatim text, indented | |||
| for line in doc.strip().split("\n"): | |||
| for line in hit["text"].strip().split("\n"): | |||
There was a problem hiding this comment.
search() now tolerates None docs during BM25 scoring, but it can still crash when printing results: hit["text"].strip() will raise if Chroma returns a None document (the same production scenario mentioned in the new BM25 regression tests). Normalize doc to an empty string when building hits (or when printing) so reranked results are always renderable.
Comment on lines
+124
to
+130
| _vector_disabled = False | ||
| _vector_disabled_reason = "" | ||
| # Optional[dict] (not ``dict | None``) keeps Python 3.9 import-time | ||
| # parsing happy — PEP 604 unions in annotations only became unconditional | ||
| # at module-eval time in 3.10. | ||
| _vector_capacity_status = None # type: Optional[dict] | ||
|
|
There was a problem hiding this comment.
_vector_capacity_status = None # type: Optional[dict] uses Optional without importing it. Ruff/pyflakes typically flags undefined names inside type comments, and type checkers will too. Either import Optional from typing at the top of the module, or switch to a real annotation that doesn’t require a type comment.
Comment on lines
427
to
435
| Returns: | ||
| { | ||
| "people": [...entity dicts...], | ||
| "projects": [...entity dicts...], | ||
| "uncertain":[...entity dicts...], | ||
| # Only present when corpus_origin reclassifies at least one | ||
| # candidate as an agent persona: | ||
| "agent_personas": [...entity dicts...], | ||
| } |
There was a problem hiding this comment.
detect_entities now always returns a topics bucket (even in the empty-candidates path), but the docstring’s documented return shape doesn’t mention it. Please update the Returns section to include "topics": [...] so downstream callers/tests relying on the stable dict shape have an accurate contract.
Comment on lines
+39
to
+50
| def _normalize_wing(wing: str | None) -> str | None: | ||
| """Normalize a wing name for consistent lookup. | ||
|
|
||
| ``init`` stores wing names with hyphens and spaces replaced by underscores | ||
| (e.g. ``mempalace_public``). Callers that pass the raw directory name | ||
| (``mempalace-public``) would silently miss. This helper aligns the lookup | ||
| key with the stored metadata. | ||
| """ | ||
| if wing is None: | ||
| return None | ||
| return wing.lower().replace(" ", "_").replace("-", "_") | ||
|
|
There was a problem hiding this comment.
palace_graph._normalize_wing() duplicates the normalization rule that was just centralized as config.normalize_wing_name(). Keeping two implementations risks future drift (and _normalize_wing also doesn’t strip whitespace). Prefer calling the shared helper (and handling None/empty) so init/mine/tunnel lookup all stay aligned by construction.
Comment on lines
634
to
+640
| def tool_check_duplicate(content: str, threshold: float = 0.9): | ||
| col = _get_collection() | ||
| if not col: | ||
| return _no_palace() | ||
| if _vector_disabled: | ||
| # Without a usable HNSW we can't compute cosine similarity for | ||
| # near-duplicate detection. Report the limitation rather than |
There was a problem hiding this comment.
In the #1222 divergence mode, opening a Chroma PersistentClient/collection can segfault. tool_check_duplicate() calls _get_collection() before checking _vector_disabled, so it can still crash the server in exactly the scenario this flag is meant to protect. Probe/check _vector_disabled first (via _refresh_vector_disabled_flag()) and short-circuit without touching chromadb when vector search is disabled.
Member
Author
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/MemPalace/mempalace/sessions/01a1089d-da46-4dc8-85e8-d7e50763dd58 Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
Contributor
Applied the requested review-thread changes in 4cfbd60. Focused regression tests, ruff checks, Code Review, and CodeQL all pass. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The fix landed this cycle and is documented under 3.3.4. The 3.3.0 Bug Fixes block is shipped history and shouldn't grow new entries retroactively.
The PR documenting the fix is #1232; referencing it from inside its own changelog entry is circular.
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.
Release prep for v3.3.4. Bumps every version source from 3.3.3 to 3.3.4, dates the CHANGELOG, and adds entries for the bug fixes that landed this cycle.
Version sync
Six hand-edited locations +
uv.lockauto-synced frompyproject.toml:pyproject.tomlmempalace/version.py(canonical).claude-plugin/plugin.json.claude-plugin/marketplace.json.codex-plugin/plugin.jsonREADME.mdbadgeuv.lock(auto-synced from pyproject.toml)test_version_consistency+test_readme_claimscover the 6-file invariant — both green.CHANGELOG additions this cycle
The pre-existing 3.3.4 draft already documented the larger items (init/mine prompt, topic tunnels, corpus-origin, CLI search rerank, graceful Ctrl-C, idempotent init). This PR adds Bug Fixes entries for what landed since:
normalize_wing_name; lookup side: tunnel helpers normalize incoming wing names so existing palaces with raw-name keys recover).hnsw:batch_size+hnsw:sync_threshold). Empirical: 30 GB → 376 MB on a 39,792-drawer palace, supersedes fix: prevent HNSW index bloat from resize+persist cycles #346._fix_blob_seq_ids+ addrepair --mode max-seq-id#1135 — narrowed_fix_blob_seq_idsshim + newmempalace repair --mode max-seq-idto un-poison existing palaces, either from a sidecar DB or heuristically.--mode convosregardless ofMEMPAL_DIR, and shell hooks gained the same_validate_transcript_pathrejection logic the Python entry point already had.Pre-tag verification
ruff check+ruff format --checkcleantest_version_consistency+test_readme_claimspyproject.tomland.claude-plugin/plugin.jsonboth referencemempalace-mcpmempalace --version→3.3.4,mempalace-mcp --helpworks (catches the v3.3.2-class regression class)After merge
Standard release flow:
v3.3.4on main (not develop) — GitHub Releases shows it as Latestpython -m build→twine check→twine uploadgh release create v3.3.4 --notes-from-tag --latestpip install mempalace==3.3.4,mempalace-mcp --help