jason/mempalace
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
853 Commits 2 Branches 0 Tags
main
Commit Graph

4 Commits

Author SHA1 Message Date
eldar702 4949aab68b fix: guard None metadata/doc in tool_check_duplicate and Layer1/Layer2 
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**.
 2026-04-19 11:13:50 +03:00
Sergey Kuznetsov ae5196bc8d Мempalace backend seam (#413) 
* refactor: add stage-1 backend abstraction seam

Introduce the first upstreamable storage seam for MemPalace without
bringing in the PostgreSQL spike or any benchmark artifacts.

This change adds a small backend package with:
- BaseCollection as the minimal collection contract
- ChromaBackend/ChromaCollection as the default implementation

It then routes the main runtime collection consumers through that seam:
- palace.py
- searcher.py
- layers.py
- palace_graph.py
- mcp_server.py
- miner.status()

Behavioral constraints kept for stage 1:
- ChromaDB remains the only backend and the default path
- no config/env backend selection yet
- no PostgreSQL code
- no benchmark or research files
- existing tests stay unchanged

Important compatibility details:
- read paths now call the seam with create=False so they still surface
  the existing 'no palace found' behavior instead of silently creating
  empty collections
- write paths keep create=True semantics through palace.get_collection()
- layers/searcher retain a chromadb module attribute so the existing
  mock-based tests can keep patching PersistentClient unchanged
- ChromaBackend only creates palace directories on create=True, which
  preserves mocked read-path tests that use fake read-only paths

Verification:
- python3 -m py_compile mempalace/backends/__init__.py mempalace/backends/base.py mempalace/backends/chroma.py mempalace/palace.py mempalace/searcher.py mempalace/layers.py mempalace/palace_graph.py mempalace/mcp_server.py mempalace/miner.py
- pytest -q  # 529 passed, 106 deselected

* refactor: clean up stage-1 seam compatibility shims

Tighten the stage-1 backend abstraction branch after review.

This follow-up does three small things:
- keep the chromadb compatibility hook in searcher.py and layers.py,
  but express it through the backends.chroma module so it no longer
  reads like an accidental unused import
- fix the palace_graph.py helper alias to avoid the local name collision
  flagged by ruff (imported helper vs local _get_collection wrapper)
- preserve the existing mock-based test patch points unchanged while
  keeping the new backend seam intact

Why this matters:
- the direct  form looked like a
  dead import in review, even though it was intentionally preserving the
  existing test seam ( and
  )
- palace_graph.py had a real lint issue ( redefinition) that was
  small but worth fixing before a public PR

Verification:
- /opt/homebrew/bin/ruff check mempalace/backends/__init__.py mempalace/backends/base.py mempalace/backends/chroma.py mempalace/palace.py mempalace/searcher.py mempalace/layers.py mempalace/palace_graph.py mempalace/mcp_server.py mempalace/miner.py
- pytest -q tests/test_layers.py tests/test_searcher.py
- pytest -q  # 529 passed, 106 deselected

* docs: explain backend shim imports in search paths

Add short code comments in searcher.py and layers.py explaining why the
module-level `chromadb` alias remains after the stage-1 backend seam
refactor.

The alias is intentional: it preserves the existing mock patch points used
by the current test suite (`mempalace.searcher.chromadb.PersistentClient`
and `mempalace.layers.chromadb.PersistentClient`) while the runtime logic
now flows through the backend abstraction.

This keeps the public PR easier to review because the apparent "unused
import" now has an explicit reason next to it.

Verification:
- /opt/homebrew/bin/ruff check mempalace/searcher.py mempalace/layers.py
- pytest -q tests/test_layers.py tests/test_searcher.py

* refactor: reuse a default backend instance in palace helper

Tighten the stage-1 backend seam by promoting the default Chroma backend
adapter to a module-level singleton in `mempalace/palace.py`.

This keeps the stage-1 scope unchanged — Chroma is still the only backend
wired in this branch — but avoids constructing a fresh `ChromaBackend()`
object on every `get_collection()` call. The backend is stateless today,
so this is a readability/cleanup change rather than a behavioral one.

Why this helps:
- makes `palace.get_collection()` read like a real default factory instead
  of an inline constructor call
- keeps the stage-1 branch a little cleaner before opening the public PR
- does not widen the backend surface or change any config/runtime behavior

Verification:
- python3 -m py_compile mempalace/palace.py
- pytest -q tests/test_miner.py tests/test_layers.py tests/test_searcher.py
- pytest -q  # 529 passed, 106 deselected

* fix: harden read-only seam behavior and update seam tests

Preserve the stage-1 backend abstraction while closing the real read-path
regression surfaced in PR review.

What changed:
- make ChromaBackend.get_collection(create=False) fail fast when the palace
  directory does not exist instead of letting PersistentClient create it as a
  side effect
- update miner.status() to call get_collection(..., create=False) so status
  keeps the historical 'No palace found' behavior
- remove the temporary chromadb shim aliases from layers.py and searcher.py
  now that the tests patch the seam directly
- add focused tests for the new backends package, including ChromaCollection
  delegation and ChromaBackend create=True/create=False behavior
- retarget layer/searcher tests to patch the backend seam instead of patching
  chromadb.PersistentClient inside production modules
- add a regression test that status() does not create an empty palace when the
  target path is missing

Verification:
- ruff check .
- uv run pytest -q
- uv run pytest -q tests/test_backends.py tests/test_cli.py tests/test_mcp_server.py tests/test_layers.py tests/test_searcher.py tests/test_miner.py

Notes:
- the separate benchmark/slow/stress layer was started as a soak but not used
  as the merge gate for this PR branch

* refactor: drop duplicate mcp collection cache declaration

Remove a redundant `_collection_cache = None` assignment in
`mempalace/mcp_server.py` left over after the stage-1 backend seam refactor.

This does not change behavior; it only trims review noise in the MCP server
module after the read-path hardening pass.

Verification:
- ruff check mempalace/mcp_server.py
- uv run pytest -q tests/test_mcp_server.py

---------

Co-authored-by: Sergey Kuznetsov <sergey@iterudit.com>
 2026-04-11 16:16:49 -07:00
Tal Muskal e24d8ca733 test: expand coverage to 70%, fix mcp_server CI crash (threshold 60%) 
Add/expand tests for normalize (39%→97%), searcher (39%→100%),
layers (28%→97%), split_mega_files (34%→72%).

Fix mcp_server.py parse_args→parse_known_args to prevent SystemExit
when imported during pytest (CI was crashing on all test jobs).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
 2026-04-08 21:07:03 +03:00
Tal Muskal 03e9b57108 test: add comprehensive test coverage (35% → 58%, threshold 50%) 
Add 180+ new tests across 10 test files covering previously untested modules:
- instructions_cli (0% → 100%), hooks_cli (73% → 96%), spellcheck (28% → 84%)
- palace_graph (9% → 91%), general_extractor (0% → 92%), entity_detector (0% → 69%)
- entity_registry (0% → 70%), room_detector_local (0% → 55%), layers (0% → 28%)
- onboarding (0% → 36%)

Also fixes Windows encoding bug in onboarding.py (write_text without encoding="utf-8").

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
 2026-04-08 20:54:56 +03:00