diff --git a/mempalace/diary_ingest.py b/mempalace/diary_ingest.py index ef5625528f..e0d6b3c8c7 100644 --- a/mempalace/diary_ingest.py +++ b/mempalace/diary_ingest.py @@ -174,7 +174,6 @@ def ingest_diaries( _now = datetime.now(timezone.utc) now_iso = _now.isoformat() now_ts = _now.timestamp() - drawer_id = _diary_drawer_id(wing, date_str) entities = _extract_entities_for_metadata(text) source_file = str(diary_path) diff --git a/mempalace/searcher.py b/mempalace/searcher.py index db14c19ff9..683c35e0b2 100644 --- a/mempalace/searcher.py +++ b/mempalace/searcher.py @@ -745,6 +745,38 @@ def _apply_candidate_strategy( merger(hits, query, palace_path, wing, room, n_results, max_distance=max_distance) +def _open_drawers_or_error_dict(palace_path, collection_name): + """Open the drawers collection, distinguishing recoverable filesystem + states from unexpected errors. + + Returns the open collection on success or the user-facing "No palace + found" error dict if the palace directory genuinely doesn't exist + (PalaceNotFoundError). All other exceptions log the full chained + traceback via logger.exception and propagate with their original type + so callers can distinguish the failure mode. + + Used to be inlined in search_memories under a bare ``except Exception`` + that swallowed every error under the same misleading "No palace found" + message, hiding e.g. chromadb's KeyError('_type') on a corrupt + collection config under the same diagnostic. + """ + try: + return get_collection(palace_path, collection_name=collection_name, create=False) + except PalaceNotFoundError as e: + logger.error("No palace found at %s: %s", palace_path, e) + return { + "error": "No palace found", + "hint": "Run: mempalace init && mempalace mine ", + } + except Exception: + logger.exception( + "get_collection failed for palace=%s collection=%s", + palace_path, + collection_name, + ) + raise + + def search_memories( query: str, palace_path: str, @@ -808,14 +840,10 @@ def search_memories( collection_name=collection_name, ) - try: - drawers_col = get_collection(palace_path, collection_name=collection_name, create=False) - except Exception as e: - logger.error("No palace found at %s: %s", palace_path, e) - return { - "error": "No palace found", - "hint": "Run: mempalace init && mempalace mine ", - } + opened = _open_drawers_or_error_dict(palace_path, collection_name) + if isinstance(opened, dict): + return opened + drawers_col = opened where = build_where_filter(wing, room) diff --git a/tests/test_backfill_filed_at_ts.py b/tests/test_backfill_filed_at_ts.py index 1b6ff834c9..09d6f14075 100644 --- a/tests/test_backfill_filed_at_ts.py +++ b/tests/test_backfill_filed_at_ts.py @@ -1,9 +1,7 @@ """Tests for mempalace.backfill_filed_at_ts.""" from __future__ import annotations -import os import sqlite3 -import tempfile from datetime import datetime, timezone from pathlib import Path diff --git a/tests/test_classifier.py b/tests/test_classifier.py index 971b156035..3394abe8ad 100644 --- a/tests/test_classifier.py +++ b/tests/test_classifier.py @@ -15,7 +15,6 @@ from typing import Any from unittest.mock import patch -import pytest from mempalace.provenance.classifier import ( DEFAULT_ENDPOINT, diff --git a/tests/test_provenance.py b/tests/test_provenance.py index 0a228382b5..025eec9a2f 100644 --- a/tests/test_provenance.py +++ b/tests/test_provenance.py @@ -11,14 +11,12 @@ from __future__ import annotations -import pytest from mempalace.provenance import ( HEURISTIC_CONFIDENCE_QUOTE, HEURISTIC_CONFIDENCE_RELATION_ONLY, WING_LINEAGE_SCHEMA_DOC, ProvenanceCandidate, - ProvenanceRecord, extract_candidates, validate_candidate, ) diff --git a/tests/test_provenance_mining.py b/tests/test_provenance_mining.py index ad7dffbc26..138716c24b 100644 --- a/tests/test_provenance_mining.py +++ b/tests/test_provenance_mining.py @@ -17,10 +17,8 @@ from typing import Any -import pytest from mempalace.provenance.mining import ( - DEFAULT_CONFIDENCE_THRESHOLD, mine_chunk_for_provenance, _rewrite_speaker_to_source, ) diff --git a/tests/test_searcher.py b/tests/test_searcher.py index 721bb117e6..771254a27c 100644 --- a/tests/test_searcher.py +++ b/tests/test_searcher.py @@ -5,10 +5,12 @@ plus mock-based tests for error paths. """ +import logging from unittest.mock import MagicMock, patch import pytest +from mempalace.backends import PalaceNotFoundError from mempalace.searcher import SearchError, search, search_memories @@ -84,6 +86,51 @@ def test_search_memories_query_error(self): assert "error" in result assert "query failed" in result["error"] + def test_search_memories_palace_not_found_returns_error_dict(self): + """PalaceNotFoundError raised from get_collection (backend filesystem + race) returns the user-facing error dict, preserving the contract for + the common filesystem-missing case.""" + with patch( + "mempalace.searcher.get_collection", + side_effect=PalaceNotFoundError("missing palace"), + ): + result = search_memories("anything", "/nonexistent/palace") + assert isinstance(result, dict) + assert result.get("error") == "No palace found" + assert "mempalace init" in result.get("hint", "") + + def test_search_memories_unexpected_exception_propagates_with_chain(self, caplog): + """Non-filesystem errors (e.g. chromadb's KeyError('_type') from a + corrupt collection config) propagate to the caller with the original + exception type and chained traceback. Diagnostic logging fires via + logger.exception so the traceback always lands in mempalace logs even + if the caller swallows the exception silently.""" + original = KeyError("_type") + with patch( + "mempalace.searcher.get_collection", + side_effect=original, + ): + with caplog.at_level(logging.ERROR, logger="mempalace_mcp"): + with pytest.raises(KeyError) as exc_info: + search_memories("anything", "/some/palace") + # Original exception preserved (same type, same args). + assert exc_info.value.args == ("_type",) + # Chained context preserved (`__context__` / `__cause__` are the same + # instance, set by Python's implicit exception chaining when `raise` + # is used inside an except block). + assert exc_info.value.__context__ is original or exc_info.value is original + # Full traceback logged via logger.exception (level=ERROR plus + # exc_info=True), so callers that suppress the exception still leave a + # diagnostic trail naming the failure. + diagnostic_records = [ + r for r in caplog.records + if r.name == "mempalace_mcp" and r.exc_info is not None + ] + assert len(diagnostic_records) >= 1, ( + "Expected logger.exception() call from search_memories' get_collection " + "wrapper, got no records with exc_info set" + ) + def test_search_memories_vector_path_uses_explicit_collection_name(self): mock_col = MagicMock() mock_col.query.return_value = {