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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
> Need the shortest recovery/setup path? Use the
> [Claude Code retention setup checklist](https://mempalaceofficial.com/guide/claude-code-retention.html).

[![version-shield](https://img.shields.io/badge/version-3.3.5-4dc9f6?style=flat-square&labelColor=0a0e14)](https://github.com/techempower-org/mempalace/releases) [![upstream-shield](https://img.shields.io/badge/upstream-3.3.5-7dd8f8?style=flat-square&labelColor=0a0e14)](https://github.com/MemPalace/mempalace/releases)
[![version-shield](https://img.shields.io/badge/version-3.3.6-4dc9f6?style=flat-square&labelColor=0a0e14)](https://github.com/techempower-org/mempalace/releases) [![upstream-shield](https://img.shields.io/badge/upstream-3.3.5-7dd8f8?style=flat-square&labelColor=0a0e14)](https://github.com/MemPalace/mempalace/releases)
[![python-shield](https://img.shields.io/badge/python-3.9+-7dd8f8?style=flat-square&labelColor=0a0e14&logo=python&logoColor=7dd8f8)](https://www.python.org/)
[![license-shield](https://img.shields.io/badge/license-MIT-b0e8ff?style=flat-square&labelColor=0a0e14)](LICENSE)

Expand Down
20 changes: 16 additions & 4 deletions mempalace/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1457,12 +1457,24 @@ def _daemon_search_fast(query: str, n_results: int, wing: str = None) -> dict |
raw = _call_daemon_rest("/search/fast", rest_params)
if raw is None:
return None
for hit in raw:
hit["text"] = hit.pop("snippet", "")
hit["bm25_score"] = round(hit.pop("rank", 0), 3)
if isinstance(raw, dict):
hits = raw.get("results")
elif isinstance(raw, list):
hits = raw
else:
hits = None
if not isinstance(hits, list):
return None
for hit in hits:
if "snippet" in hit:
hit["text"] = hit.pop("snippet")
elif "text" not in hit:
hit["text"] = ""
if "rank" in hit:
hit["bm25_score"] = round(hit.pop("rank"), 3)
Comment on lines +1473 to +1474

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the 'rank' key in hit has a value of None or is not a numeric type (e.g., a string), calling round() directly on it will raise a TypeError. To ensure robustness against unexpected daemon responses, we should defensively convert the value to a float and handle potential conversion errors safely.

Suggested change
if "rank" in hit:
hit["bm25_score"] = round(hit.pop("rank"), 3)
if "rank" in hit:
rank_val = hit.pop("rank")
try:
hit["bm25_score"] = round(float(rank_val), 3) if rank_val is not None else 0.0
except (ValueError, TypeError):
hit["bm25_score"] = 0.0
References
  1. PEP 8 recommends defensive programming and robust exception handling to prevent unexpected runtime crashes. (link)

if hit.get("source_file"):
hit["source"] = hit["source_file"]
return {"results": raw, "query": query, "source": "bm25-fast"}
return {"results": hits, "query": query, "source": "bm25-fast"}


def _daemon_search_hybrid(
Expand Down
2 changes: 2 additions & 0 deletions mempalace/sources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
get_adapter_class,
register,
reset_adapters,
reset_discovery,
resolve_adapter_for_source,
unregister,
)
Expand Down Expand Up @@ -69,6 +70,7 @@
"get_adapter_class",
"register",
"reset_adapters",
"reset_discovery",
"resolve_adapter_for_source",
"unregister",
]
12 changes: 12 additions & 0 deletions mempalace/sources/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,18 @@ def reset_adapters() -> None:
_instances.clear()


def reset_discovery() -> None:
"""Force the next ``available_adapters()`` call to re-scan entry points.

Tests that ``unregister`` entry-point-discovered adapters must call
this — otherwise the cached ``_discovered=True`` flag suppresses
rediscovery and ``available_adapters()`` permanently returns ``[]``.
"""
global _discovered
with _lock:
_discovered = False


def resolve_adapter_for_source(
*,
explicit: str | None = None,
Expand Down
11 changes: 11 additions & 0 deletions tests/test_cli_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import argparse
import json
import urllib.error
from unittest.mock import MagicMock, patch

import pytest
Expand All @@ -33,6 +34,12 @@ def read(self):
return self._body


def _rest_fastpath_404(req):
"""Raise HTTPError(404) for REST fast-path GETs so cmd_search/cmd_status
fall through to the MCP POST envelope these tests actually verify."""
raise urllib.error.HTTPError(req.full_url, 404, "Not Found", {}, None)


# ── _daemon_strict ──────────────────────────────────────────────────────


Expand Down Expand Up @@ -229,6 +236,8 @@ def test_routes_to_daemon_when_strict(self, capsys):
).encode()

def fake_urlopen(req, timeout=None):
if getattr(req, "data", None) is None:
return _rest_fastpath_404(req)
captured_body = json.loads(req.data.decode())
assert captured_body["params"]["name"] == "mempalace_search"
assert captured_body["params"]["arguments"]["query"] == "graphql"
Expand Down Expand Up @@ -264,6 +273,8 @@ def test_sends_limit_not_max_results(self):
captured = {}

def fake_urlopen(req, timeout=None):
if getattr(req, "data", None) is None:
return _rest_fastpath_404(req)
captured["body"] = json.loads(req.data.decode())
return _FakeResp(body)

Expand Down
28 changes: 17 additions & 11 deletions tests/test_cli_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,12 @@ def test_daemon_routing_emits_daemon_payload_as_json(self, mock_cfg, capsys):
args = argparse.Namespace(palace=None, json=True, quiet=False)
daemon_payload = {"total_drawers": 7, "wings": {"wing_x": 7}}

with patch("mempalace.cli._call_daemon_tool", return_value=daemon_payload):
with patch("mempalace.cli._daemon_strict", return_value=True):
from mempalace.cli import cmd_status
with patch("mempalace.cli._call_daemon_rest", return_value=None):
with patch("mempalace.cli._call_daemon_tool", return_value=daemon_payload):
with patch("mempalace.cli._daemon_strict", return_value=True):
from mempalace.cli import cmd_status

cmd_status(args)
cmd_status(args)

out = capsys.readouterr().out
payload = json.loads(out)
Expand All @@ -204,11 +205,15 @@ def test_daemon_error_emits_json_error_and_exit_2(self, mock_cfg, capsys):

with patch("mempalace.cli._daemon_strict", return_value=True):
with patch(
"mempalace.cli._call_daemon_tool",
"mempalace.cli._call_daemon_rest",
side_effect=DaemonError("connection refused"),
):
with pytest.raises(SystemExit) as exc_info:
cmd_status(args)
with patch(
"mempalace.cli._call_daemon_tool",
side_effect=DaemonError("connection refused"),
):
with pytest.raises(SystemExit) as exc_info:
cmd_status(args)

assert exc_info.value.code == 2
out = capsys.readouterr().out
Expand Down Expand Up @@ -365,11 +370,12 @@ def test_daemon_search_emits_results_with_query_key(self, mock_cfg, capsys):
}

with patch("mempalace.cli._daemon_strict", return_value=True):
with patch("mempalace.cli._call_daemon_tool", return_value=daemon_payload):
with pytest.raises(SystemExit) as exc_info:
from mempalace.cli import cmd_search
with patch("mempalace.cli._call_daemon_rest", return_value=None):
with patch("mempalace.cli._call_daemon_tool", return_value=daemon_payload):
with pytest.raises(SystemExit) as exc_info:
from mempalace.cli import cmd_search

cmd_search(args)
cmd_search(args)

assert exc_info.value.code == 0
out = capsys.readouterr().out
Expand Down
22 changes: 21 additions & 1 deletion tests/test_cli_search_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,16 @@ def _envelope(payload: dict) -> bytes:


def _make_search_dispatcher(payload: dict):
"""Return a fake ``urlopen`` that returns ``payload`` for every call."""
"""Return a fake ``urlopen`` that returns ``payload`` for every call.

``GET`` requests (REST fast-path) get the bare payload; ``POST``
requests (MCP tools/call) get the JSON-RPC envelope. Lets the same
fixture serve ``/search/fast`` and ``mempalace_search`` fallback.
"""

def fake_urlopen(req, timeout=None):
if getattr(req, "data", None) is None:
return _FakeResp(json.dumps(payload).encode())
return _FakeResp(_envelope(payload))

return fake_urlopen
Expand Down Expand Up @@ -429,6 +436,13 @@ def test_limit_overrides_results(self):
captured = {}

def fake_urlopen(req, timeout=None):
if getattr(req, "data", None) is None:
# REST fast-path GET — capture ``limit`` from query string.
from urllib.parse import urlparse, parse_qs

qs = parse_qs(urlparse(req.full_url).query)
captured["arguments"] = {"limit": int(qs["limit"][0])}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Accessing qs['limit'][0] directly can raise a KeyError or IndexError if the 'limit' query parameter is missing or empty in the URL. Using .get() with a default fallback is safer and prevents potential test crashes.

Suggested change
captured["arguments"] = {"limit": int(qs["limit"][0])}
limit_list = qs.get("limit")
limit_val = int(limit_list[0]) if limit_list else 5
captured["arguments"] = {"limit": limit_val}

return _FakeResp(json.dumps({"results": [], "warnings": []}).encode())
captured["arguments"] = json.loads(req.data.decode())["params"]["arguments"]
return _FakeResp(_envelope({"results": [], "warnings": []}))

Expand All @@ -445,6 +459,12 @@ def test_results_used_when_limit_unset(self):
captured = {}

def fake_urlopen(req, timeout=None):
if getattr(req, "data", None) is None:
from urllib.parse import urlparse, parse_qs

qs = parse_qs(urlparse(req.full_url).query)
captured["arguments"] = {"limit": int(qs["limit"][0])}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Accessing qs['limit'][0] directly can raise a KeyError or IndexError if the 'limit' query parameter is missing or empty in the URL. Using .get() with a default fallback is safer and prevents potential test crashes.

Suggested change
captured["arguments"] = {"limit": int(qs["limit"][0])}
limit_list = qs.get("limit")
limit_val = int(limit_list[0]) if limit_list else 5
captured["arguments"] = {"limit": limit_val}

return _FakeResp(json.dumps({"results": [], "warnings": []}).encode())
captured["arguments"] = json.loads(req.data.decode())["params"]["arguments"]
return _FakeResp(_envelope({"results": [], "warnings": []}))

Expand Down
3 changes: 3 additions & 0 deletions tests/test_cli_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
SourceItemMetadata,
register,
reset_adapters,
reset_discovery,
unregister,
)

Expand Down Expand Up @@ -61,6 +62,8 @@ def _isolate_registry():
unregister(name)
except Exception:
pass
# Allow the next test to rediscover the in-tree entry-point adapters.
reset_discovery()


@pytest.fixture()
Expand Down
20 changes: 20 additions & 0 deletions tests/test_cli_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,24 @@ def _make_dispatcher(responses: dict):
Any tool not in ``responses`` returns an empty object. The dispatcher
inspects the JSON-RPC request body to pick the right response so the
same fixture can serve multiple tools fired by a single command.

``GET`` requests (REST fast-path, e.g. ``/status/fast``) get the bare
payload mapped from the path's matching MCP tool — ``/status/fast``
mirrors ``mempalace_status``.
"""

_rest_to_tool = {
"/status/fast": "mempalace_status",
"/search/fast": "mempalace_search",
}

def fake_urlopen(req, timeout=None):
if getattr(req, "data", None) is None:
url = req.full_url
for path, tool in _rest_to_tool.items():
if path in url:
return _FakeResp(json.dumps(responses.get(tool, {})).encode())
return _FakeResp(b"{}")
body = json.loads(req.data.decode())
name = body["params"]["name"]
return _FakeResp(_envelope(responses.get(name, {})))
Expand Down Expand Up @@ -302,6 +317,11 @@ def test_kg_failure_does_not_blank_the_dashboard(self, capsys):
from mempalace import cli

def fake_urlopen(req, timeout=None):
if getattr(req, "data", None) is None:
# REST fast-path GET — ``/status/fast`` mirrors mempalace_status.
return _FakeResp(
json.dumps({"total_drawers": 5, "wings": {"projects": 5}}).encode()
)
body = json.loads(req.data.decode())
name = body["params"]["name"]
if name == "mempalace_kg_stats":
Expand Down
33 changes: 27 additions & 6 deletions tests/test_readme_claims.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,36 @@ def _doc_tool_names() -> list:
# ---------------------------------------------------------------------------


def _readme_self_tool_counts(readme: str) -> list[str]:
"""Extract "N tools" claims that refer to mempalace itself.

Skips markdown table rows (lines containing ``|``) so competitor
rows like "Longhand … 17 tools" don't poison the count — those are
facts about other projects, not claims about mempalace.
"""
counts = []
for line in readme.splitlines():
if "|" in line:
continue
counts.extend(re.findall(r"(\d+)\s+tools", line))
return counts


class TestToolCount:
"""README claims '19 tools available through MCP' in multiple places."""
"""README claims 'N tools available through MCP' in multiple places."""

def test_readme_tool_count_matches_code(self):
"""Claim: README says 19 tools. Actual TOOLS dict may differ.
"""Claim: README says N tools. Actual TOOLS dict may differ.

This test asserts the REAL tool count so the README can be updated.
If TOOLS has 25 entries, the README should say 25, not 19.

Only counts mempalace's own self-claims — competitor tool counts
in comparison tables are out of scope.
"""
actual_count = len(_tools_dict_keys())
readme = _readme()
# Find all "19 tools" claims in README
claimed_counts = re.findall(r"(\d+)\s+tools", readme)
claimed_counts = _readme_self_tool_counts(readme)
for claimed in claimed_counts:
assert int(claimed) == actual_count, (
f"README claims {claimed} tools but TOOLS dict has {actual_count}. "
Expand Down Expand Up @@ -732,9 +749,13 @@ class TestReadmeToolCountConsistency:
"""README mentions tool count in multiple places — they must all agree."""

def test_all_tool_count_mentions_consistent(self):
"""Every place README says 'N tools' must use the same number."""
"""Every place README says 'N tools' about mempalace must agree.

Scoped to self-claims (non-table-row lines) so competitor counts
in the comparison tables don't trigger spurious failures.
"""
readme = _readme()
counts = re.findall(r"(\d+)\s+tools", readme)
counts = _readme_self_tool_counts(readme)
if len(counts) > 1:
unique = set(counts)
assert len(unique) == 1, (
Expand Down
3 changes: 3 additions & 0 deletions tests/test_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
get_adapter_class,
register,
reset_adapters,
reset_discovery,
resolve_adapter_for_source,
unregister,
)
Expand Down Expand Up @@ -66,6 +67,8 @@ def _isolate_registry():
reset_adapters()
for name in list(available_adapters()):
unregister(name)
# Allow the next test to rediscover the in-tree entry-point adapters.
reset_discovery()


# ---------------------------------------------------------------------------
Expand Down
Loading