Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion mempalace/diary_ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
44 changes: 36 additions & 8 deletions mempalace/searcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <dir> && mempalace mine <dir>",
}
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,
Expand Down Expand Up @@ -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 <dir> && mempalace mine <dir>",
}
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)

Expand Down
2 changes: 0 additions & 2 deletions tests/test_backfill_filed_at_ts.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down
1 change: 0 additions & 1 deletion tests/test_classifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from typing import Any
from unittest.mock import patch

import pytest

from mempalace.provenance.classifier import (
DEFAULT_ENDPOINT,
Expand Down
2 changes: 0 additions & 2 deletions tests/test_provenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
2 changes: 0 additions & 2 deletions tests/test_provenance_mining.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
47 changes: 47 additions & 0 deletions tests/test_searcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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 = {
Expand Down
Loading