From 89f49a2169593632a96773f7724e3bf5a8093ceb Mon Sep 17 00:00:00 2001 From: Kirill Korikov Date: Thu, 3 Sep 2026 10:46:44 +0400 Subject: [PATCH 1/4] add memory provider evaluation harness --- docs/memory.md | 7 + memory/eval/README.md | 78 +++ memory/eval/evaluate.py | 649 ++++++++++++++++++ memory/eval/testdata/v1/conversations.jsonl | 3 + memory/eval/testdata/v1/documents.jsonl | 12 + memory/eval/testdata/v1/mutations.jsonl | 3 + memory/eval/testdata/v1/privacy_manifest.json | 17 + memory/eval/testdata/v1/queries.jsonl | 18 + memory/eval/tests/test_evaluate.py | 183 +++++ 9 files changed, 970 insertions(+) create mode 100644 memory/eval/README.md create mode 100644 memory/eval/evaluate.py create mode 100644 memory/eval/testdata/v1/conversations.jsonl create mode 100644 memory/eval/testdata/v1/documents.jsonl create mode 100644 memory/eval/testdata/v1/mutations.jsonl create mode 100644 memory/eval/testdata/v1/privacy_manifest.json create mode 100644 memory/eval/testdata/v1/queries.jsonl create mode 100644 memory/eval/tests/test_evaluate.py diff --git a/docs/memory.md b/docs/memory.md index b7e3121..bb7677d 100644 --- a/docs/memory.md +++ b/docs/memory.md @@ -21,6 +21,13 @@ history. Run `dotagents doctor --e2e` and `hermes hooks doctor` after setup. Memory data lives in your knowledge directory (default `~/Workspace/knowledge`, configurable via `KNOWLEDGE_DIR`), never in the tool repository. +## Provider evaluation + +`memory/eval/` contains a privacy-safe, fully isolated harness for comparing +Hermes memory providers against the built-in + memsearch baseline. It uses a +synthetic fixture, throwaway memsearch collections (`dotagents_eval_*`), and +never touches live memory stores. See `memory/eval/README.md`. + ## Tools `sync` builds every Go tool under your repo's `memory/tools/` into `$GOBIN` or `~/.local/bin` (skipped without a Go toolchain; rebuilds only on source changes). Two ship as reference implementations in this repository: diff --git a/memory/eval/README.md b/memory/eval/README.md new file mode 100644 index 0000000..2efff4d --- /dev/null +++ b/memory/eval/README.md @@ -0,0 +1,78 @@ +# Memory provider evaluation harness + +Privacy-safe, isolated comparison of Hermes memory providers against the +current dotagents baseline (built-in memory + memsearch). No adapter touches +the live Hermes home, the `ai` memsearch collection, or the knowledge vault. + +## Components + +| Path | Purpose | +| --- | --- | +| `evaluate.py` | CLI: `privacy-scan`, `capabilities`, `run` | +| `testdata/v1/` | Frozen synthetic fixture (12 docs, 18 queries, 3 mutations, 3 conversations) | +| `tests/` | Unit tests for fixture validation, scoring, adapters, runner, discovery | + +## Fixture provenance + +Fully synthetic. Technical themes only (memory tiers, hook contracts, sync +semantics, redaction, dedup classification). No source documents were copied. +People are codenames (Atlas/Bramble/Cinder); the only hostname uses `.test` +(RFC 2606) and the only absolute path is under `/srv/fixture`. +`fixture-secret-hook-0001` is a deliberately inert canary used by conversation +checks, not a credential. + +The fixture has passed the automated privacy scan (zero findings) but carries +`approval.state: pending_human_review` in `privacy_manifest.json` until you +explicitly approve it. `run --approved-fixture` records the human gate in the +result payload; it does not change fixture contents. + +## Usage + +```bash +# Validate fixture and scan for private data (exit 1 on any finding) +python3 memory/eval/evaluate.py privacy-scan --fixture memory/eval/testdata/v1 + +# Inventory installed provider plugins + runtime availability +python3 memory/eval/evaluate.py capabilities --output /tmp/capabilities.json + +# Run both baselines plus all known providers (unconfigured ones report capability_gap) +python3 memory/eval/evaluate.py run --fixture memory/eval/testdata/v1 \ + --provider all --output /tmp/results.json + +# Optional: local canaries file (newline-delimited; values never printed) +python3 memory/eval/evaluate.py privacy-scan --fixture memory/eval/testdata/v1 \ + --forbidden-file ~/.hermes/cache/eval-canaries.txt +``` + +## Guarantees + +- Every adapter is rooted under a per-run temporary directory; memsearch arms + use throwaway `dotagents_eval_` collections and `teardown()` resets + them, so live data and the `ai` collection are never touched. +- Privacy scan gates `run`: a nonzero finding count aborts before any ingest. +- Failed/absent providers emit `capability_gap` or `failed` with reasons — + never a zero-quality score. +- Results record the fixture hash, environment, per-query rankings, latencies, + lifecycle outcomes, and capability gaps under a versioned schema. + +## Adding a real provider driver + +Implement `ProviderAdapter` (`health`, `reset`, `ingest`, `query`, `update`, +`forget`, `restart`, `export`, `capture`, `teardown`) in `evaluate.py` (or a +sibling module) and register it in `make_adapter`. The runner, scorer, and +privacy gate apply automatically. Record unsupported operations explicitly; +the plan forbids scoring gaps as zeros. + +## v1 baseline numbers (single-run, directional) + +From the isolated run on this machine (fixture hash +`ff1182fc8fd32d83…`, memsearch collection `dotagents_eval_`): + +| Provider | Recall@1 | Recall@3 | MRR | Abstention | Query p50 | +| --- | --- | --- | --- | --- | --- | +| memsearch | 0.778 | 0.833 | 0.806 | 0.0 | ~1.8 s | +| built-in | 0.056 | 0.111 | 0.099 | 0.0 | <1 ms | + +Abstention is 0.0 for both because pure retrieval never refuses to answer — +an LLM judge or thresholded re-ranker is required to act on unanswerable +queries; the harness records the gap rather than hiding it. diff --git a/memory/eval/evaluate.py b/memory/eval/evaluate.py new file mode 100644 index 0000000..2a6fefe --- /dev/null +++ b/memory/eval/evaluate.py @@ -0,0 +1,649 @@ +#!/usr/bin/env python3 +"""Privacy-safe, isolated memory-provider evaluation for dotagents. + +The harness runs deterministic built-in-memory and real memsearch baselines. +Hermes provider plugins are inventoried from their manifests and emitted as +explicit capability gaps until a configured, isolated driver is available. +""" + +from __future__ import annotations + +import argparse +import hashlib +import json +import math +import os +import re +import shutil +import statistics +import subprocess +import sys +import tempfile +import time +import uuid +from pathlib import Path +from typing import Any, Iterable + +PROVIDER_METADATA: dict[str, dict[str, Any]] = { + "honcho": {"storage": "cloud/self-hosted", "local_first": False, "tools": ["honcho_profile", "honcho_search", "honcho_context", "honcho_reasoning", "honcho_conclude"]}, + "openviking": {"storage": "self-hosted", "local_first": True, "tools": ["viking_search", "viking_read", "viking_browse", "viking_remember", "viking_forget", "viking_add_resource"]}, + "mem0": {"storage": "cloud/self-hosted/oss", "local_first": True, "tools": ["mem0_search", "mem0_add", "mem0_update", "mem0_delete"]}, + "hindsight": {"storage": "cloud/local", "local_first": True, "tools": ["hindsight_retain", "hindsight_recall", "hindsight_reflect"]}, + "holographic": {"storage": "local", "local_first": True, "tools": ["fact_store", "fact_feedback"]}, + "retaindb": {"storage": "cloud", "local_first": False, "tools": ["retaindb_profile", "retaindb_search", "retaindb_context", "retaindb_remember", "retaindb_forget"]}, + "byterover": {"storage": "local/cloud", "local_first": True, "tools": ["brv_query", "brv_curate", "brv_status"]}, + "supermemory": {"storage": "cloud/self-hosted", "local_first": True, "tools": ["supermemory_store", "supermemory_search", "supermemory_forget", "supermemory_profile"]}, +} +BASELINE_PROVIDERS = ("builtin", "memsearch") + +PRIVACY_PATTERNS: tuple[tuple[str, re.Pattern[str]], ...] = ( + ("email", re.compile(r"(?i)\b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}\b")), + ("ip_address", re.compile(r"(? list[dict[str, Any]]: + records: list[dict[str, Any]] = [] + if not path.exists(): + raise ValueError(f"missing fixture file: {path.name}") + for line_number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): + if not line.strip(): + continue + try: + value = json.loads(line) + except json.JSONDecodeError as exc: + raise ValueError(f"invalid JSONL in {path.name}:{line_number}: {exc.msg}") from exc + if not isinstance(value, dict): + raise ValueError(f"record in {path.name}:{line_number} must be an object") + records.append(value) + return records + + +def load_fixture(root: Path) -> dict[str, list[dict[str, Any]]]: + fixture = { + "documents": read_jsonl(root / "documents.jsonl"), + "queries": read_jsonl(root / "queries.jsonl"), + "conversations": read_jsonl(root / "conversations.jsonl"), + "mutations": read_jsonl(root / "mutations.jsonl"), + } + document_ids: set[str] = set() + evidence_ids: set[str] = set() + for document in fixture["documents"]: + doc_id = str(document.get("id", "")).strip() + text = document.get("text") + if not doc_id or doc_id in document_ids or not isinstance(text, str) or not text.strip(): + raise ValueError("documents require unique non-empty id and text") + document_ids.add(doc_id) + evidence = document.get("evidence") + if not isinstance(evidence, list) or not evidence: + raise ValueError(f"document {doc_id} requires evidence") + for item in evidence: + if not isinstance(item, dict) or not str(item.get("id", "")).strip() or not str(item.get("text", "")).strip(): + raise ValueError(f"document {doc_id} has invalid evidence") + evidence_id = str(item["id"]) + if evidence_id in evidence_ids: + raise ValueError(f"duplicate evidence id: {evidence_id}") + evidence_ids.add(evidence_id) + query_ids: set[str] = set() + for query in fixture["queries"]: + query_id = str(query.get("id", "")).strip() + if not query_id or query_id in query_ids or not str(query.get("query", "")).strip(): + raise ValueError("queries require unique non-empty id and query") + query_ids.add(query_id) + expected = query.get("expected_evidence_ids", []) + if not isinstance(expected, list): + raise ValueError(f"query {query_id} expected_evidence_ids must be a list") + unknown = sorted(set(str(value) for value in expected) - evidence_ids) + if unknown: + raise ValueError(f"query {query_id} references unknown evidence: {', '.join(unknown)}") + for mutation in fixture["mutations"]: + mutation_id = str(mutation.get("id", "")).strip() + if not mutation_id: + raise ValueError("mutations require a non-empty id") + action = str(mutation.get("action", "")).strip() + if action not in ("update", "forget"): + raise ValueError(f"mutation {mutation_id} has unsupported action: {action or '(empty)'}") + evidence_key = str(mutation.get("evidence_id", "")).strip() + if evidence_key not in evidence_ids: + raise ValueError(f"mutation {mutation_id} references unknown evidence: {evidence_key or '(empty)'}") + base_query = str(mutation.get("base_query_id", "")).strip() + if base_query and base_query not in query_ids: + raise ValueError(f"mutation {mutation_id} references unknown query: {base_query}") + if action == "update" and not str(mutation.get("new_text", "")).strip(): + raise ValueError(f"mutation {mutation_id} update requires new_text") + return fixture + + +def scan_text(text: str, relative_path: str, forbidden: Iterable[str]) -> list[dict[str, Any]]: + findings: list[dict[str, Any]] = [] + lines = text.splitlines() or [text] + for line_number, line in enumerate(lines, start=1): + for category, pattern in PRIVACY_PATTERNS: + if pattern.search(line): + findings.append({"category": category, "path": relative_path, "line": line_number}) + lowered = line.casefold() + for canary in forbidden: + value = canary.strip() + if value and value.casefold() in lowered: + findings.append({"category": "forbidden_canary", "path": relative_path, "line": line_number}) + break + return findings + + +def scan_fixture(root: Path, forbidden: Iterable[str] = ()) -> dict[str, Any]: + files = ("documents.jsonl", "queries.jsonl", "conversations.jsonl", "mutations.jsonl") + findings: list[dict[str, Any]] = [] + digest = hashlib.sha256() + for name in files: + path = root / name + if not path.is_file(): + findings.append({"category": "missing_file", "path": name, "line": 0}) + continue + data = path.read_bytes() + digest.update(name.encode("utf-8") + b"\0" + data) + findings.extend(scan_text(data.decode("utf-8"), name, forbidden)) + return { + "schema_version": 1, + "fixture_sha256": digest.hexdigest(), + "files": list(files), + "finding_count": len(findings), + "findings": findings, + "approved": False, + } + + +def _yaml_scalar(path: Path, key: str) -> str | None: + pattern = re.compile(rf"^\s*{re.escape(key)}\s*:\s*[\"']?([^\"'#\n]+)") + for line in path.read_text(encoding="utf-8").splitlines(): + match = pattern.match(line) + if match: + return match.group(1).strip() + return None + + +def discover_capabilities(plugin_root: Path, hermes_help: str | None = None) -> list[dict[str, Any]]: + if hermes_help is None: + hermes = shutil.which("hermes") + if hermes: + result = subprocess.run([hermes, "memory", "--help"], capture_output=True, text=True, timeout=15, check=False) + hermes_help = result.stdout + result.stderr + else: + hermes_help = "" + runtime_names = { + name for name in PROVIDER_METADATA if re.search(rf"(? float: + return round(value, 6) + + +def score_rankings(queries: list[dict[str, Any]], rankings: dict[str, list[str]]) -> dict[str, Any]: + if not queries: + return {"query_count": 0, "recall_at_1": 0.0, "recall_at_3": 0.0, "recall_at_5": 0.0, "mrr": 0.0, "ndcg_at_5": 0.0, "abstention_accuracy": 0.0} + recall_sums = {1: 0.0, 3: 0.0, 5: 0.0} + reciprocal_ranks: list[float] = [] + ndcgs: list[float] = [] + abstention: list[float] = [] + for query in queries: + expected = set(str(value) for value in query.get("expected_evidence_ids", [])) + ranked = rankings.get(str(query["id"]), []) + is_unanswerable = bool(query.get("unanswerable", not expected)) + if is_unanswerable: + value = 1.0 if not ranked else 0.0 + for k in recall_sums: + recall_sums[k] += value + reciprocal_ranks.append(value) + ndcgs.append(value) + abstention.append(value) + continue + for k in recall_sums: + recall_sums[k] += len(expected.intersection(ranked[:k])) / len(expected) + positions = [index + 1 for index, evidence_id in enumerate(ranked) if evidence_id in expected] + reciprocal_ranks.append(1.0 / min(positions) if positions else 0.0) + dcg = sum(1.0 / math.log2(index + 2) for index, evidence_id in enumerate(ranked[:5]) if evidence_id in expected) + ideal = sum(1.0 / math.log2(index + 2) for index in range(min(len(expected), 5))) + ndcgs.append(dcg / ideal if ideal else 0.0) + count = len(queries) + return { + "query_count": count, + "recall_at_1": _round(recall_sums[1] / count), + "recall_at_3": _round(recall_sums[3] / count), + "recall_at_5": _round(recall_sums[5] / count), + "mrr": _round(sum(reciprocal_ranks) / count), + "ndcg_at_5": _round(sum(ndcgs) / count), + "abstention_accuracy": _round(sum(abstention) / len(abstention)) if abstention else None, + } + + +class ProviderAdapter: + name = "unknown" + + def __init__(self, sandbox: Path) -> None: + self.sandbox = sandbox.resolve() + self.sandbox.mkdir(parents=True, exist_ok=True) + + def health(self) -> dict[str, Any]: + return {"available": True} + + def reset(self) -> None: + return None + + def ingest(self, documents: list[dict[str, Any]]) -> None: + raise NotImplementedError + + def query(self, query: str, top_k: int) -> list[dict[str, Any]]: + raise NotImplementedError + + def update(self, mutation: dict[str, Any]) -> dict[str, Any]: + return {"supported": False, "reason": "update is not implemented by this adapter"} + + def forget(self, mutation: dict[str, Any]) -> dict[str, Any]: + return {"supported": False, "reason": "forget is not implemented by this adapter"} + + def restart(self) -> dict[str, Any]: + return {"supported": True} + + def export(self) -> dict[str, Any]: + return {"supported": False, "reason": "export is not implemented by this adapter"} + + def capture(self, conversation: dict[str, Any]) -> dict[str, Any]: + del conversation + return {"supported": False, "reason": "scripted conversation capture is not implemented by this adapter"} + + def teardown(self) -> None: + return None + + +class BuiltinAdapter(ProviderAdapter): + name = "builtin" + + def __init__(self, sandbox: Path, char_limit: int = 2200) -> None: + super().__init__(sandbox) + self.char_limit = char_limit + self.memory_path = self.sandbox / "hermes" / "memories" / "MEMORY.md" + self.entries: list[dict[str, str]] = [] + + def reset(self) -> None: + self.entries = [] + if self.memory_path.exists(): + self.memory_path.unlink() + + def ingest(self, documents: list[dict[str, Any]]) -> None: + self.memory_path.parent.mkdir(parents=True, exist_ok=True) + entries: list[dict[str, str]] = [] + rendered: list[str] = [] + for document in documents: + for evidence in document["evidence"]: + text = f"[{evidence['id']}] {evidence['text']}" + candidate = "\n§\n".join(rendered + [text]) + "\n" + if len(candidate) > self.char_limit: + self.entries = entries + self.memory_path.write_text("\n§\n".join(rendered) + ("\n" if rendered else ""), encoding="utf-8") + return + rendered.append(text) + entries.append({"evidence_id": str(evidence["id"]), "text": str(evidence["text"]), "document_id": str(document["id"])}) + self.entries = entries + self.memory_path.write_text("\n§\n".join(rendered) + ("\n" if rendered else ""), encoding="utf-8") + + def _persist(self) -> None: + self.memory_path.parent.mkdir(parents=True, exist_ok=True) + rendered = [f"[{entry['evidence_id']}] {entry['text']}" for entry in self.entries] + self.memory_path.write_text("\n§\n".join(rendered) + ("\n" if rendered else ""), encoding="utf-8") + + def _find_entry(self, evidence_id: str) -> dict[str, str] | None: + for entry in self.entries: + if entry["evidence_id"] == evidence_id: + return entry + return None + + def update(self, mutation: dict[str, Any]) -> dict[str, Any]: + entry = self._find_entry(str(mutation.get("evidence_id", ""))) + if entry is None: + return {"supported": False, "reason": "unknown evidence id"} + entry["text"] = str(mutation.get("new_text", "")).strip() + self._persist() + return {"supported": True, "changed": 1} + + def forget(self, mutation: dict[str, Any]) -> dict[str, Any]: + evidence_id = str(mutation.get("evidence_id", "")) + before = len(self.entries) + self.entries = [entry for entry in self.entries if entry["evidence_id"] != evidence_id] + if len(self.entries) == before: + return {"supported": False, "reason": "unknown evidence id"} + self._persist() + return {"supported": True, "removed": 1} + + def query(self, query: str, top_k: int) -> list[dict[str, Any]]: + del query + return [dict(item, score=1.0) for item in self.entries[:top_k]] + + def export(self) -> dict[str, Any]: + return {"supported": True, "path": str(self.memory_path.relative_to(self.sandbox)), "characters": len(self.memory_path.read_text(encoding="utf-8"))} + + +class MemsearchAdapter(ProviderAdapter): + name = "memsearch" + + def __init__(self, sandbox: Path, collection: str | None = None) -> None: + super().__init__(sandbox) + self.binary = shutil.which("memsearch") + self.collection = collection or f"dotagents_eval_{uuid.uuid4().hex[:12]}" + if not self.collection.startswith("dotagents_eval_"): + raise ValueError("evaluation memsearch collection must start with dotagents_eval_") + self.documents_dir = self.sandbox / "documents" + self.evidence_by_document: dict[str, list[str]] = {} + + def health(self) -> dict[str, Any]: + return {"available": self.binary is not None, "binary": self.binary, "collection": self.collection} + + def reset(self) -> None: + if self.binary: + subprocess.run([self.binary, "reset", "--collection", self.collection, "--yes"], capture_output=True, text=True, timeout=30, check=False) + if self.documents_dir.exists(): + shutil.rmtree(self.documents_dir) + + def ingest(self, documents: list[dict[str, Any]]) -> None: + if not self.binary: + raise RuntimeError("memsearch is not installed") + self.documents_dir.mkdir(parents=True, exist_ok=True) + self.evidence_by_document = {} + for document in documents: + doc_id = str(document["id"]) + evidence_ids = [str(item["id"]) for item in document["evidence"]] + self.evidence_by_document[doc_id] = evidence_ids + lines = [f"# {document.get('title') or doc_id}", "", str(document["text"]), ""] + lines.extend(f"[evidence:{item['id']}] {item['text']}" for item in document["evidence"]) + (self.documents_dir / f"{doc_id}.md").write_text("\n".join(lines) + "\n", encoding="utf-8") + result = subprocess.run( + [self.binary, "index", str(self.documents_dir), "--collection", self.collection, "--force"], + capture_output=True, + text=True, + timeout=180, + check=False, + ) + if result.returncode != 0: + raise RuntimeError(f"memsearch index failed: {(result.stderr or result.stdout).strip()[:500]}") + + @staticmethod + def _result_records(payload: Any) -> list[dict[str, Any]]: + if isinstance(payload, list): + return [item for item in payload if isinstance(item, dict)] + if isinstance(payload, dict): + for key in ("results", "data", "matches"): + value = payload.get(key) + if isinstance(value, list): + return [item for item in value if isinstance(item, dict)] + return [] + + def query(self, query: str, top_k: int) -> list[dict[str, Any]]: + if not self.binary: + raise RuntimeError("memsearch is not installed") + result = subprocess.run( + [self.binary, "search", query, "--top-k", str(top_k), "--collection", self.collection, "--source-prefix", str(self.documents_dir), "--json-output"], + capture_output=True, + text=True, + timeout=60, + check=False, + ) + if result.returncode != 0: + raise RuntimeError(f"memsearch search failed: {(result.stderr or result.stdout).strip()[:500]}") + payload = json.loads(result.stdout) + ranked: list[dict[str, Any]] = [] + seen: set[str] = set() + for record in self._result_records(payload): + source = str(record.get("source") or record.get("path") or record.get("file") or "") + content = str(record.get("content") or record.get("text") or record.get("chunk") or "") + score = record.get("score") or record.get("similarity") or 0.0 + doc_id = Path(source).stem if source else "" + ids = re.findall(r"\[evidence:([^\]]+)\]", content) + if not ids: + ids = self.evidence_by_document.get(doc_id, []) + for evidence_id in ids: + if evidence_id not in seen: + seen.add(evidence_id) + ranked.append({"evidence_id": evidence_id, "document_id": doc_id, "score": score}) + if len(ranked) >= top_k: + return ranked + return ranked + + def teardown(self) -> None: + self.reset() + + +class UnavailableProviderAdapter(ProviderAdapter): + def __init__(self, sandbox: Path, name: str, reason: str) -> None: + super().__init__(sandbox) + self.name = name + self.reason = reason + + def health(self) -> dict[str, Any]: + return {"available": False, "reason": self.reason} + + def ingest(self, documents: list[dict[str, Any]]) -> None: + del documents + raise RuntimeError(self.reason) + + def query(self, query: str, top_k: int) -> list[dict[str, Any]]: + del query, top_k + raise RuntimeError(self.reason) + + +class Timer: + def __init__(self) -> None: + self.started = 0.0 + self.elapsed_ms = 0.0 + + def __enter__(self) -> "Timer": + self.started = time.perf_counter() + return self + + def __exit__(self, exc_type: Any, exc: Any, traceback: Any) -> None: + del exc_type, exc, traceback + self.elapsed_ms = (time.perf_counter() - self.started) * 1000 + + +def _latency_summary(values: list[float]) -> dict[str, float | None]: + if not values: + return {"p50_ms": None, "p95_ms": None} + ordered = sorted(values) + index = max(0, math.ceil(0.95 * len(ordered)) - 1) + return {"p50_ms": _round(statistics.median(ordered)), "p95_ms": _round(ordered[index])} + + +def run_adapter(adapter: ProviderAdapter, fixture: dict[str, list[dict[str, Any]]], top_k: int = 5) -> dict[str, Any]: + health = adapter.health() + if not health.get("available"): + return {"provider": adapter.name, "status": "capability_gap", "health": health, "metrics": None} + rankings: dict[str, list[str]] = {} + query_details: list[dict[str, Any]] = [] + query_latencies: list[float] = [] + lifecycle: list[dict[str, Any]] = [] + try: + adapter.reset() + with Timer() as ingest_timer: + adapter.ingest(fixture["documents"]) + for query in fixture["queries"]: + with Timer() as query_timer: + results = adapter.query(str(query["query"]), top_k) + ids = [str(item["evidence_id"]) for item in results] + rankings[str(query["id"])] = ids + query_latencies.append(query_timer.elapsed_ms) + query_details.append({"query_id": query["id"], "evidence_ids": ids, "latency_ms": _round(query_timer.elapsed_ms)}) + for mutation in fixture["mutations"]: + action = mutation.get("action") + if action == "forget": + outcome = adapter.forget(mutation) + else: + outcome = adapter.update(mutation) + post: dict[str, Any] = {} + base_query_id = str(mutation.get("base_query_id", "")).strip() + if outcome.get("supported") and base_query_id: + base_query = next((q for q in fixture["queries"] if str(q["id"]) == base_query_id), None) + if base_query is not None: + ranked = [str(item["evidence_id"]) for item in adapter.query(str(base_query["query"]), top_k)] + post["ranked"] = ranked + expect_absent = [str(value) for value in mutation.get("expect_absent", [])] + if expect_absent: + post["absent_ok"] = all(value not in ranked for value in expect_absent) + lifecycle.append({"mutation_id": mutation.get("id"), "action": action, **outcome, **({"post": post} if post else {})}) + restart = adapter.restart() + exported = adapter.export() + capture = [adapter.capture(conversation) for conversation in fixture["conversations"]] + return { + "provider": adapter.name, + "status": "completed", + "health": health, + "metrics": score_rankings(fixture["queries"], rankings), + "latency": {"ingest_ms": _round(ingest_timer.elapsed_ms), "query": _latency_summary(query_latencies)}, + "queries": query_details, + "lifecycle": lifecycle, + "capture": capture, + "restart": restart, + "export": exported, + } + except (OSError, RuntimeError, subprocess.SubprocessError, json.JSONDecodeError) as exc: + return {"provider": adapter.name, "status": "failed", "health": health, "error": str(exc)[:500], "metrics": None} + finally: + adapter.teardown() + + +def make_adapter(name: str, sandbox: Path) -> ProviderAdapter: + if name == "builtin": + return BuiltinAdapter(sandbox) + if name == "memsearch": + return MemsearchAdapter(sandbox) + return UnavailableProviderAdapter(sandbox, name, "provider requires an explicitly configured isolated driver") + + +def _forbidden_values(path: str | None) -> list[str]: + if not path: + return [] + return [line.strip() for line in Path(path).read_text(encoding="utf-8").splitlines() if line.strip()] + + +def write_json(path: Path, payload: Any) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n", encoding="utf-8") + + +def command_privacy_scan(args: argparse.Namespace) -> int: + report = scan_fixture(Path(args.fixture), _forbidden_values(args.forbidden_file)) + if args.output: + write_json(Path(args.output), report) + print(json.dumps(report, indent=2, sort_keys=True)) + return 1 if report["finding_count"] else 0 + + +def command_capabilities(args: argparse.Namespace) -> int: + matrix = discover_capabilities(Path(args.plugin_root).expanduser()) + payload = {"schema_version": 1, "hermes": shutil.which("hermes"), "providers": matrix} + if args.output: + write_json(Path(args.output), payload) + print(json.dumps(payload, indent=2, sort_keys=True)) + return 0 + + +def command_run(args: argparse.Namespace) -> int: + fixture_root = Path(args.fixture).resolve() + forbidden = _forbidden_values(args.forbidden_file) + privacy = scan_fixture(fixture_root, forbidden) + if privacy["finding_count"]: + print(json.dumps({"error": "privacy scan failed", "findings": privacy["findings"]}, indent=2), file=sys.stderr) + return 2 + fixture = load_fixture(fixture_root) + requested = list(BASELINE_PROVIDERS) + sorted(PROVIDER_METADATA) if args.provider == "all" else [args.provider] + output = Path(args.output).resolve() + if output.exists() and not args.overwrite: + print(f"output already exists: {output} (pass --overwrite)", file=sys.stderr) + return 2 + results: list[dict[str, Any]] = [] + sandbox_parent = Path(args.sandbox_root).resolve() if args.sandbox_root else None + with tempfile.TemporaryDirectory(prefix="dotagents-memory-eval-", dir=str(sandbox_parent) if sandbox_parent else None) as tmp: + root = Path(tmp).resolve() + if root == Path.home().resolve() or Path.home().resolve() in root.parents: + # Temp roots under a user's home are safe only because every adapter + # is still rooted beneath this newly-created evaluation directory. + pass + for name in requested: + arm = root / name + results.append(run_adapter(make_adapter(name, arm), fixture, top_k=args.top_k)) + payload = { + "schema_version": 1, + "fixture_sha256": privacy["fixture_sha256"], + "privacy_scan": {"finding_count": 0, "approved": bool(args.approved_fixture)}, + "environment": { + "python": sys.version.split()[0], + "hermes": shutil.which("hermes"), + "memsearch": shutil.which("memsearch"), + }, + "results": results, + } + write_json(output, payload) + print(json.dumps(payload, indent=2, sort_keys=True)) + failed = [item for item in results if item["status"] == "failed"] + return 1 if failed else 0 + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description="Evaluate Hermes memory providers without touching live memory stores") + subparsers = parser.add_subparsers(dest="command", required=True) + + privacy = subparsers.add_parser("privacy-scan", help="scan a frozen fixture for private data") + privacy.add_argument("--fixture", required=True) + privacy.add_argument("--forbidden-file", help="local newline-delimited canaries; values are never printed") + privacy.add_argument("--output") + privacy.set_defaults(handler=command_privacy_scan) + + capabilities = subparsers.add_parser("capabilities", help="inventory installed Hermes memory providers") + capabilities.add_argument("--plugin-root", default="~/.hermes/hermes-agent/plugins/memory") + capabilities.add_argument("--output") + capabilities.set_defaults(handler=command_capabilities) + + run = subparsers.add_parser("run", help="run isolated evaluation arms") + run.add_argument("--fixture", required=True) + run.add_argument("--provider", default="all", choices=["all", *BASELINE_PROVIDERS, *sorted(PROVIDER_METADATA)]) + run.add_argument("--output", required=True) + run.add_argument("--forbidden-file") + run.add_argument("--approved-fixture", action="store_true", help="record that the human fixture review gate passed") + run.add_argument("--overwrite", action="store_true") + run.add_argument("--top-k", type=int, default=5) + run.add_argument("--sandbox-root") + run.set_defaults(handler=command_run) + return parser + + +def main(argv: list[str] | None = None) -> int: + parser = build_parser() + args = parser.parse_args(argv) + return int(args.handler(args)) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/memory/eval/testdata/v1/conversations.jsonl b/memory/eval/testdata/v1/conversations.jsonl new file mode 100644 index 0000000..065e401 --- /dev/null +++ b/memory/eval/testdata/v1/conversations.jsonl @@ -0,0 +1,3 @@ +{"id": "c01", "session_id": "fixture-redaction", "turns": [{"role": "user", "content": "Persist this note while testing: the shared token is fixture-secret-hook-0001."}, {"role": "assistant", "content": "Stored the note about the fixture test token."}], "expect_persisted": ["Stored the note"], "expect_absent": ["fixture-secret-hook-0001"]} +{"id": "c02", "session_id": "fixture-replay", "turns": [{"role": "user", "content": "Remember that replayed digest events must persist exactly once."}, {"role": "assistant", "content": "Recorded the single-persist rule for replayed digest events."}], "expect_persisted": ["exactly once"], "expect_absent": [], "replay": 2} +{"id": "c03", "session_id": "fixture-junk", "turns": [{"role": "user", "content": "Hmm just thinking out loud, the weather is nice today."}, {"role": "assistant", "content": "Noted."}], "expect_persisted": [], "expect_absent": ["weather"], "description": "Chatter must not become durable memory."} diff --git a/memory/eval/testdata/v1/documents.jsonl b/memory/eval/testdata/v1/documents.jsonl new file mode 100644 index 0000000..bdfe0a0 --- /dev/null +++ b/memory/eval/testdata/v1/documents.jsonl @@ -0,0 +1,12 @@ +{"id": "d01", "title": "Layered Memory Tiers", "text": "The memory layer offers three modes. Off registers no memory hooks. Basic stores bounded Markdown session digests using only the Python standard library. Indexed keeps the same local files as canonical state and adds a disposable semantic index. Selecting indexed mode must not change the vault file format.", "evidence": [{"id": "ev-01a", "text": "Basic memory mode stores bounded Markdown session digests using only the Python standard library."}, {"id": "ev-01b", "text": "Indexed memory mode keeps local files canonical and adds a disposable semantic index without changing the vault file format."}]} +{"id": "d02", "title": "Local-First Vault Ownership", "text": "The Orchid vault at /srv/fixture/orchid-vault is canonical. Markdown files remain readable and writable without a network service. The search service at indexer.fictional.test is derived infrastructure: it may be rebuilt or removed without losing memory. Synchronization copies facts between bounded agent memory and the vault; it does not make the index authoritative.", "evidence": [{"id": "ev-02a", "text": "The local Markdown vault is canonical and the semantic index is derived infrastructure that can be rebuilt without losing memory."}]} +{"id": "d03", "title": "Preventing Hook Recursion", "text": "A lifecycle hook must not invoke the command that dispatches the same lifecycle hook. That creates recursive re-entry. Use a dedicated implementation entrypoint or set and check a process-local recursion guard before dispatch. A guarded invocation runs the hook body at most once and still emits the host continuation response.", "evidence": [{"id": "ev-03a", "text": "A process-local recursion guard or dedicated entrypoint limits lifecycle hook re-entry to a single execution."}]} +{"id": "d04", "title": "Hook Standard Output Contract", "text": "Hook standard output contains exactly one valid JSON object. Diagnostic text belongs on standard error. A successful callback explicitly requests continuation and suppresses incidental output. Session-start callbacks may add bounded context in a hook-specific output object. Plain text on standard output is a contract failure even when the process exits successfully.", "evidence": [{"id": "ev-04a", "text": "Hook stdout must be exactly one valid JSON object and plain-text stdout is a contract failure even on exit code zero."}]} +{"id": "d05", "title": "Idempotent Session Digests", "text": "Each digest is enclosed by markers containing a sanitized stable session identifier. Before appending, the writer acquires a lock and searches all session Markdown files for the start marker. Replaying the same event therefore produces one block. Different sessions may append concurrently without dropping records.", "evidence": [{"id": "ev-05a", "text": "Replaying a session event produces exactly one digest block because the writer locks and checks start markers before appending."}]} +{"id": "d06", "title": "Harness Payload Adapters", "text": "Atlas supplies inline messages, Bramble supplies a JSONL transcript path, and Cinder supplies a session identifier whose log is stored separately. Small adapters normalize these payloads into session identifier, timestamp, user turns, final assistant text, and mentioned paths. Persistence and redaction operate on the normalized representation rather than duplicating the memory pipeline per harness.", "evidence": [{"id": "ev-06a", "text": "Harness payload differences are normalized by small adapters before persistence and redaction."}]} +{"id": "d07", "title": "Bounded Startup Context", "text": "Startup context reads at most seven Markdown files sorted by descending filename. Each file contributes at most 2400 characters from its newest end, and total additional context is at most 6000 characters. Given identical files, selection, ordering, truncation, and output JSON are deterministic.", "evidence": [{"id": "ev-07a", "text": "Startup context reads at most seven files, 2400 characters each, 6000 total, and is fully deterministic."}]} +{"id": "d08", "title": "Redact Before Persistence", "text": "Redaction occurs before truncation, identifier sanitization, digest rendering, or path storage. Labeled credentials, bearer values, query-string secrets, and known provider-token shapes become the literal marker [REDACTED]. Tests use only the inert canary fixture-secret-hook-0001 and assert that the canary never reaches persisted text.", "evidence": [{"id": "ev-08a", "text": "Redaction happens before truncation and rendering, and credential-shaped strings become the literal marker [REDACTED]."}]} +{"id": "d09", "title": "Directional Vault Synchronization", "text": "Memory-to-vault exports previously unseen normalized facts into append-only vault sections. Vault-to-memory imports compact facts only while the destination character budget permits. Both directions deduplicate normalized content. A full destination is left byte-for-byte unchanged and must not be reported as modified.", "evidence": [{"id": "ev-09a", "text": "A full destination stays byte-for-byte unchanged and is not reported as modified during vault sync."}, {"id": "ev-09b", "text": "Both sync directions deduplicate normalized content."}]} +{"id": "d10", "title": "Indexer Failure Is Nonfatal", "text": "Digest persistence succeeds even when the optional semantic index executable is absent or returns an error. Reindexing happens after the durable write and failures are reported as warnings. The basic tier never invokes the indexer. Search quality may degrade, but local memory capture continues.", "evidence": [{"id": "ev-10a", "text": "Digest persistence succeeds when the semantic indexer is absent; indexing failures are nonfatal warnings."}]} +{"id": "d11", "title": "Report-First Consolidation", "text": "Consolidation is review-only by default. It scans complete session blocks and proposes candidates with exact evidence coordinates. It never rewrites canonical session or profile files. Applying a proposed change requires a separate explicit action, and a report path that already exists is not overwritten.", "evidence": [{"id": "ev-11a", "text": "Consolidation is review-only and never rewrites canonical session or profile files."}]} +{"id": "d12", "title": "Duplicate and Conflict Classification", "text": "Two identical blocks with the same session identifier are a stale duplicate only when exactly one block is in the UTC-dated canonical file. Repeated identifiers with different block content, or without a unique canonical location, are conflicts. Blocks missing their matching end marker are incomplete and ignored.", "evidence": [{"id": "ev-12a", "text": "Identical same-session blocks with exactly one canonical copy are stale duplicates; different content is a conflict; missing end markers are incomplete and ignored."}]} diff --git a/memory/eval/testdata/v1/mutations.jsonl b/memory/eval/testdata/v1/mutations.jsonl new file mode 100644 index 0000000..2f1c8c1 --- /dev/null +++ b/memory/eval/testdata/v1/mutations.jsonl @@ -0,0 +1,3 @@ +{"id": "m01", "action": "forget", "document_id": "d05", "evidence_id": "ev-05a", "base_query_id": "q05", "expect_absent": ["ev-05a"], "description": "After a forget, replay-dedup knowledge must not be retrievable again."} +{"id": "m02", "action": "update", "document_id": "d07", "evidence_id": "ev-07a", "new_text": "Startup context now reads at most nine files with 2800 characters each and a 9000 character total.", "base_query_id": "q07", "expect_absent": [], "description": "After the correction, queries about context bounds must return the revised fact."} +{"id": "m03", "action": "forget", "document_id": "d12", "evidence_id": "ev-12a", "base_query_id": "q12", "expect_absent": ["ev-12a"], "description": "Forgetting classification rules must remove retrieval of that rule."} diff --git a/memory/eval/testdata/v1/privacy_manifest.json b/memory/eval/testdata/v1/privacy_manifest.json new file mode 100644 index 0000000..bd1089f --- /dev/null +++ b/memory/eval/testdata/v1/privacy_manifest.json @@ -0,0 +1,17 @@ +{ + "schema_version": 1, + "fixture_version": "v1", + "provenance": "synthetic", + "description": "Fully fictionalized technical corpus about agent memory architecture. No real people, hosts (except RFC 2606 .test and /srv/fixture), credentials, or private knowledge-vault content. One inert secret canary (fixture-secret-hook-0001) is intentional and used only by conversation mutation checks.", + "derived_from": "technical themes only; no source documents copied", + "excluded_categories": [ + "job-search", "employment", "finance", "immigration", "health", + "relationship", "travel", "contact", "credentials", "personal-profile" + ], + "approval": { + "state": "pending_human_review", + "reviewer": null, + "reviewed_at": null + }, + "canary_note": "fixture-secret-hook-0001 is a deliberately inert fake token used to assert redaction behavior; it is not a real credential." +} diff --git a/memory/eval/testdata/v1/queries.jsonl b/memory/eval/testdata/v1/queries.jsonl new file mode 100644 index 0000000..317b29c --- /dev/null +++ b/memory/eval/testdata/v1/queries.jsonl @@ -0,0 +1,18 @@ +{"id": "q01", "query": "Which memory mode works with Python alone while keeping Markdown as the storage format?", "expected_evidence_ids": ["ev-01a"], "task_class": "semantic_paraphrase"} +{"id": "q02", "query": "Is the search index or the local vault the authoritative copy of stored memory?", "expected_evidence_ids": ["ev-02a"], "task_class": "exact_lookup"} +{"id": "q03", "query": "Why can a generic lifecycle callback call itself forever, and how is that prevented?", "expected_evidence_ids": ["ev-03a"], "task_class": "semantic_paraphrase"} +{"id": "q04", "query": "What must a successful hook print on stdout for the host to accept it?", "expected_evidence_ids": ["ev-04a"], "task_class": "exact_lookup"} +{"id": "q05", "query": "How do replayed session-end events avoid duplicate digests during concurrent writes?", "expected_evidence_ids": ["ev-05a"], "task_class": "project_continuity"} +{"id": "q06", "query": "Atlas and Cinder expose different session data shapes. Where should those differences be handled?", "expected_evidence_ids": ["ev-06a"], "task_class": "multi_document"} +{"id": "q07", "query": "How are recent memory files ordered and limited when startup context is assembled?", "expected_evidence_ids": ["ev-07a"], "task_class": "temporal_ordering"} +{"id": "q08", "query": "Should credential redaction happen before or after digest truncation and path extraction?", "expected_evidence_ids": ["ev-08a"], "task_class": "contradiction_detection"} +{"id": "q09", "query": "What happens when bounded agent memory is full during a vault-to-memory sync?", "expected_evidence_ids": ["ev-09a"], "task_class": "temporal_ordering"} +{"id": "q10", "query": "Will session capture survive when the optional semantic search executable is unavailable?", "expected_evidence_ids": ["ev-10a"], "task_class": "degraded_mode"} +{"id": "q11", "query": "Does the consolidation pass automatically rewrite memory files when it finds a candidate?", "expected_evidence_ids": ["ev-11a"], "task_class": "contradiction_detection"} +{"id": "q12", "query": "How should identical, conflicting, and unterminated copies of one session record be classified?", "expected_evidence_ids": ["ev-12a"], "task_class": "multi_document"} +{"id": "q13", "query": "What are the character limits applied to each file and in total when startup context is built?", "expected_evidence_ids": ["ev-07a"], "task_class": "rare_identifier"} +{"id": "q14", "query": "Which document explains how both sync directions avoid re-importing the same normalized facts?", "expected_evidence_ids": ["ev-09b"], "task_class": "rare_identifier"} +{"id": "q15", "query": "Explain the exact sequence in which redaction, truncation, sanitization, rendering, and path storage must occur.", "expected_evidence_ids": ["ev-08a"], "task_class": "multi_document"} +{"id": "q16", "query": "Who is the user's favorite composer, and what symphony do they prefer?", "expected_evidence_ids": [], "unanswerable": true, "task_class": "abstention"} +{"id": "q17", "query": "What is the recommended daily calorie intake for a sled dog?", "expected_evidence_ids": [], "unanswerable": true, "task_class": "abstention"} +{"id": "q18", "query": "Summarize the quarterly revenue guidance mentioned in these notes.", "expected_evidence_ids": [], "unanswerable": true, "task_class": "abstention"} diff --git a/memory/eval/tests/test_evaluate.py b/memory/eval/tests/test_evaluate.py new file mode 100644 index 0000000..3a4d17c --- /dev/null +++ b/memory/eval/tests/test_evaluate.py @@ -0,0 +1,183 @@ +import importlib.util +import json +import tempfile +import unittest +from pathlib import Path + + +MODULE_PATH = Path(__file__).parents[1] / "evaluate.py" +SPEC = importlib.util.spec_from_file_location("dotagents_memory_eval", MODULE_PATH) +assert SPEC is not None and SPEC.loader is not None +EVAL = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(EVAL) + + +def two_doc_fixture(): + return { + "documents": [ + {"id": "doc-1", "text": "alpha corpus", "evidence": [{"id": "ev-1", "text": "alpha"}]}, + {"id": "doc-2", "text": "beta corpus", "evidence": [{"id": "ev-2", "text": "beta"}]}, + ], + "queries": [{"id": "q-1", "query": "alpha", "expected_evidence_ids": ["ev-1"]}], + "conversations": [], + "mutations": [], + } + + +class PrivacyScannerTests(unittest.TestCase): + def test_clean_fixture_passes_and_private_patterns_are_categorized(self): + clean = '{"id":"doc-1","text":"The fictional project uses bounded memory."}\n' + self.assertEqual(EVAL.scan_text(clean, "documents.jsonl", []), []) + + dirty = "email user@example.com host 192.168.1.4 path /Users/alice/project token ghp_abcdefghijklmnopqrstuvwxyz123456" + categories = {finding["category"] for finding in EVAL.scan_text(dirty, "documents.jsonl", ["Alice"])} + self.assertTrue({"email", "ip_address", "home_path", "secret", "forbidden_canary"} <= categories) + for finding in EVAL.scan_text(dirty, "documents.jsonl", ["Alice"]): + self.assertNotIn("alice", json.dumps(finding).lower()) + + +class FixtureTests(unittest.TestCase): + def test_fixture_load_validates_evidence_references(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "documents.jsonl").write_text( + json.dumps({"id": "doc-1", "text": "Fact.", "evidence": [{"id": "ev-1", "text": "Fact."}]}) + "\n", + encoding="utf-8", + ) + (root / "queries.jsonl").write_text( + json.dumps({"id": "q-1", "query": "What fact?", "expected_evidence_ids": ["missing"]}) + "\n", + encoding="utf-8", + ) + (root / "conversations.jsonl").write_text("", encoding="utf-8") + (root / "mutations.jsonl").write_text("", encoding="utf-8") + with self.assertRaisesRegex(ValueError, "unknown evidence"): + EVAL.load_fixture(root) + + def test_fixture_load_validates_mutation_references(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "documents.jsonl").write_text( + json.dumps({"id": "doc-1", "text": "Fact.", "evidence": [{"id": "ev-1", "text": "Fact."}]}) + "\n", + encoding="utf-8", + ) + (root / "queries.jsonl").write_text( + json.dumps({"id": "q-1", "query": "What fact?", "expected_evidence_ids": ["ev-1"]}) + "\n", + encoding="utf-8", + ) + (root / "conversations.jsonl").write_text("", encoding="utf-8") + (root / "mutations.jsonl").write_text( + json.dumps({"id": "m-1", "action": "forget", "document_id": "doc-1", "evidence_id": "ev-1", "base_query_id": "nope"}) + "\n", + encoding="utf-8", + ) + with self.assertRaisesRegex(ValueError, "unknown query"): + EVAL.load_fixture(root) + + +class ScoringTests(unittest.TestCase): + def test_ranking_metrics_and_abstention_are_deterministic(self): + scores = EVAL.score_rankings( + [ + {"id": "q-1", "expected_evidence_ids": ["ev-2"], "unanswerable": False}, + {"id": "q-2", "expected_evidence_ids": [], "unanswerable": True}, + ], + {"q-1": ["ev-1", "ev-2"], "q-2": []}, + ) + self.assertEqual(scores["query_count"], 2) + self.assertEqual(scores["recall_at_1"], 0.5) + self.assertEqual(scores["recall_at_3"], 1.0) + self.assertEqual(scores["mrr"], 0.75) + self.assertEqual(scores["abstention_accuracy"], 1.0) + + +class BuiltinAdapterTests(unittest.TestCase): + def test_builtin_adapter_is_bounded_and_uses_no_live_home(self): + fixture = two_doc_fixture() + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + adapter = EVAL.BuiltinAdapter(root, char_limit=30) + adapter.ingest(fixture["documents"]) + memory_path = root / "hermes" / "memories" / "MEMORY.md" + self.assertTrue(memory_path.exists()) + self.assertLessEqual(len(memory_path.read_text(encoding="utf-8")), 30) + result = adapter.query("alpha", top_k=5) + self.assertEqual(result[0]["evidence_id"], "ev-1") + self.assertFalse(any(str(Path.home()) in str(path) for path in root.rglob("*"))) + + def test_builtin_adapter_drops_entries_that_exceed_budget(self): + fixture = two_doc_fixture() + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + adapter = EVAL.BuiltinAdapter(root, char_limit=8) + adapter.ingest(fixture["documents"]) + self.assertEqual(adapter.entries, []) + self.assertEqual((root / "hermes" / "memories" / "MEMORY.md").read_text(encoding="utf-8"), "") + + def test_builtin_adapter_lifecycle_update_and_forget(self): + fixture = two_doc_fixture() + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + adapter = EVAL.BuiltinAdapter(root, char_limit=400) + adapter.ingest(fixture["documents"]) + update = adapter.update({"evidence_id": "ev-1", "new_text": "alpha revised"}) + self.assertTrue(update["supported"]) + texts = [entry["text"] for entry in adapter.entries] + self.assertIn("alpha revised", texts) + forget = adapter.forget({"evidence_id": "ev-2"}) + self.assertTrue(forget["supported"]) + self.assertEqual([entry["evidence_id"] for entry in adapter.entries], ["ev-1"]) + unknown = adapter.forget({"evidence_id": "missing"}) + self.assertFalse(unknown["supported"]) + + +class MemsearchAdapterTests(unittest.TestCase): + def test_memsearch_adapter_requires_eval_collection_prefix(self): + with tempfile.TemporaryDirectory() as tmp: + with self.assertRaisesRegex(ValueError, "dotagents_eval_"): + EVAL.MemsearchAdapter(Path(tmp), collection="ai") + + +class RunnerTests(unittest.TestCase): + def test_run_adapter_records_capability_gap_for_unconfigured_provider(self): + fixture = two_doc_fixture() + with tempfile.TemporaryDirectory() as tmp: + result = EVAL.run_adapter(EVAL.make_adapter("honcho", Path(tmp)), fixture) + self.assertEqual(result["status"], "capability_gap") + self.assertIsNone(result["metrics"]) + + def test_run_adapter_executes_mutation_post_checks(self): + fixture = two_doc_fixture() + fixture["mutations"] = [ + { + "id": "m-1", + "action": "forget", + "document_id": "doc-2", + "evidence_id": "ev-2", + "base_query_id": "q-1", + "expect_absent": ["ev-2"], + } + ] + with tempfile.TemporaryDirectory() as tmp: + result = EVAL.run_adapter(EVAL.BuiltinAdapter(Path(tmp)), fixture) + self.assertEqual(result["status"], "completed") + post = result["lifecycle"][0]["post"] + self.assertTrue(post["absent_ok"]) + self.assertNotIn("ev-2", post["ranked"]) + + +class CapabilityTests(unittest.TestCase): + def test_discovery_records_manifest_and_runtime_availability(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + provider = root / "holographic" + provider.mkdir() + (provider / "plugin.yaml").write_text("name: holographic\nversion: 0.1.0\n", encoding="utf-8") + matrix = EVAL.discover_capabilities(root, hermes_help="Available providers: holographic, honcho") + by_name = {item["name"]: item for item in matrix} + self.assertEqual(by_name["holographic"]["version"], "0.1.0") + self.assertTrue(by_name["holographic"]["runtime_listed"]) + self.assertTrue(by_name["honcho"]["runtime_listed"]) + self.assertFalse(by_name["honcho"]["manifest_present"]) + + +if __name__ == "__main__": + unittest.main() From d1a8c53fa31578c74d673fbdf98049c60061ab79 Mon Sep 17 00:00:00 2001 From: Kirill Korikov Date: Thu, 3 Sep 2026 11:05:38 +0400 Subject: [PATCH 2/4] refactor eval harness per review: remove unused imports, decompose complex functions, honest adapter contract --- memory/eval/evaluate.py | 374 +++++++++++++++++++---------- memory/eval/tests/test_evaluate.py | 31 ++- 2 files changed, 272 insertions(+), 133 deletions(-) diff --git a/memory/eval/evaluate.py b/memory/eval/evaluate.py index 2a6fefe..d4d53b6 100644 --- a/memory/eval/evaluate.py +++ b/memory/eval/evaluate.py @@ -12,7 +12,6 @@ import hashlib import json import math -import os import re import shutil import statistics @@ -63,6 +62,54 @@ def read_jsonl(path: Path) -> list[dict[str, Any]]: return records +def _validate_document(document: dict[str, Any], document_ids: set[str], evidence_ids: set[str]) -> None: + doc_id = str(document.get("id", "")).strip() + text = document.get("text") + if not doc_id or doc_id in document_ids or not isinstance(text, str) or not text.strip(): + raise ValueError("documents require unique non-empty id and text") + document_ids.add(doc_id) + evidence = document.get("evidence") + if not isinstance(evidence, list) or not evidence: + raise ValueError(f"document {doc_id} requires evidence") + for item in evidence: + if not isinstance(item, dict) or not str(item.get("id", "")).strip() or not str(item.get("text", "")).strip(): + raise ValueError(f"document {doc_id} has invalid evidence") + evidence_id = str(item["id"]) + if evidence_id in evidence_ids: + raise ValueError(f"duplicate evidence id: {evidence_id}") + evidence_ids.add(evidence_id) + + +def _validate_query(query: dict[str, Any], query_ids: set[str], evidence_ids: set[str]) -> None: + query_id = str(query.get("id", "")).strip() + if not query_id or query_id in query_ids or not str(query.get("query", "")).strip(): + raise ValueError("queries require unique non-empty id and query") + query_ids.add(query_id) + expected = query.get("expected_evidence_ids", []) + if not isinstance(expected, list): + raise ValueError(f"query {query_id} expected_evidence_ids must be a list") + unknown = sorted({str(value) for value in expected} - evidence_ids) + if unknown: + raise ValueError(f"query {query_id} references unknown evidence: {', '.join(unknown)}") + + +def _validate_mutation(mutation: dict[str, Any], evidence_ids: set[str], query_ids: set[str]) -> None: + mutation_id = str(mutation.get("id", "")).strip() + if not mutation_id: + raise ValueError("mutations require a non-empty id") + action = str(mutation.get("action", "")).strip() + if action not in ("update", "forget"): + raise ValueError(f"mutation {mutation_id} has unsupported action: {action or '(empty)'}") + evidence_key = str(mutation.get("evidence_id", "")).strip() + if evidence_key not in evidence_ids: + raise ValueError(f"mutation {mutation_id} references unknown evidence: {evidence_key or '(empty)'}") + base_query = str(mutation.get("base_query_id", "")).strip() + if base_query and base_query not in query_ids: + raise ValueError(f"mutation {mutation_id} references unknown query: {base_query}") + if action == "update" and not str(mutation.get("new_text", "")).strip(): + raise ValueError(f"mutation {mutation_id} update requires new_text") + + def load_fixture(root: Path) -> dict[str, list[dict[str, Any]]]: fixture = { "documents": read_jsonl(root / "documents.jsonl"), @@ -73,48 +120,12 @@ def load_fixture(root: Path) -> dict[str, list[dict[str, Any]]]: document_ids: set[str] = set() evidence_ids: set[str] = set() for document in fixture["documents"]: - doc_id = str(document.get("id", "")).strip() - text = document.get("text") - if not doc_id or doc_id in document_ids or not isinstance(text, str) or not text.strip(): - raise ValueError("documents require unique non-empty id and text") - document_ids.add(doc_id) - evidence = document.get("evidence") - if not isinstance(evidence, list) or not evidence: - raise ValueError(f"document {doc_id} requires evidence") - for item in evidence: - if not isinstance(item, dict) or not str(item.get("id", "")).strip() or not str(item.get("text", "")).strip(): - raise ValueError(f"document {doc_id} has invalid evidence") - evidence_id = str(item["id"]) - if evidence_id in evidence_ids: - raise ValueError(f"duplicate evidence id: {evidence_id}") - evidence_ids.add(evidence_id) + _validate_document(document, document_ids, evidence_ids) query_ids: set[str] = set() for query in fixture["queries"]: - query_id = str(query.get("id", "")).strip() - if not query_id or query_id in query_ids or not str(query.get("query", "")).strip(): - raise ValueError("queries require unique non-empty id and query") - query_ids.add(query_id) - expected = query.get("expected_evidence_ids", []) - if not isinstance(expected, list): - raise ValueError(f"query {query_id} expected_evidence_ids must be a list") - unknown = sorted(set(str(value) for value in expected) - evidence_ids) - if unknown: - raise ValueError(f"query {query_id} references unknown evidence: {', '.join(unknown)}") + _validate_query(query, query_ids, evidence_ids) for mutation in fixture["mutations"]: - mutation_id = str(mutation.get("id", "")).strip() - if not mutation_id: - raise ValueError("mutations require a non-empty id") - action = str(mutation.get("action", "")).strip() - if action not in ("update", "forget"): - raise ValueError(f"mutation {mutation_id} has unsupported action: {action or '(empty)'}") - evidence_key = str(mutation.get("evidence_id", "")).strip() - if evidence_key not in evidence_ids: - raise ValueError(f"mutation {mutation_id} references unknown evidence: {evidence_key or '(empty)'}") - base_query = str(mutation.get("base_query_id", "")).strip() - if base_query and base_query not in query_ids: - raise ValueError(f"mutation {mutation_id} references unknown query: {base_query}") - if action == "update" and not str(mutation.get("new_text", "")).strip(): - raise ValueError(f"mutation {mutation_id} update requires new_text") + _validate_mutation(mutation, evidence_ids, query_ids) return fixture @@ -178,9 +189,9 @@ def discover_capabilities(plugin_root: Path, hermes_help: str | None = None) -> } manifests: dict[str, Path] = {} if plugin_root.is_dir(): - for manifest in plugin_root.glob("*/plugin.yaml"): - name = _yaml_scalar(manifest, "name") or manifest.parent.name - manifests[name] = manifest + for manifest_path in plugin_root.glob("*/plugin.yaml"): + name = _yaml_scalar(manifest_path, "name") or manifest_path.parent.name + manifests[name] = manifest_path names = sorted(set(PROVIDER_METADATA) | runtime_names | set(manifests)) matrix: list[dict[str, Any]] = [] for name in names: @@ -204,6 +215,37 @@ def _round(value: float) -> float: return round(value, 6) +def _score_unanswerable( + ranked: list[str], + recall_sums: dict[int, float], + reciprocal_ranks: list[float], + ndcgs: list[float], + abstention: list[float], +) -> None: + value = 1.0 if not ranked else 0.0 + for k in recall_sums: + recall_sums[k] += value + reciprocal_ranks.append(value) + ndcgs.append(value) + abstention.append(value) + + +def _score_answerable( + expected: set[str], + ranked: list[str], + recall_sums: dict[int, float], + reciprocal_ranks: list[float], + ndcgs: list[float], +) -> None: + for k in recall_sums: + recall_sums[k] += len(expected.intersection(ranked[:k])) / len(expected) + positions = [index + 1 for index, evidence_id in enumerate(ranked) if evidence_id in expected] + reciprocal_ranks.append(1.0 / min(positions) if positions else 0.0) + dcg = sum(1.0 / math.log2(index + 2) for index, evidence_id in enumerate(ranked[:5]) if evidence_id in expected) + ideal = sum(1.0 / math.log2(index + 2) for index in range(min(len(expected), 5))) + ndcgs.append(dcg / ideal if ideal else 0.0) + + def score_rankings(queries: list[dict[str, Any]], rankings: dict[str, list[str]]) -> dict[str, Any]: if not queries: return {"query_count": 0, "recall_at_1": 0.0, "recall_at_3": 0.0, "recall_at_5": 0.0, "mrr": 0.0, "ndcg_at_5": 0.0, "abstention_accuracy": 0.0} @@ -212,24 +254,12 @@ def score_rankings(queries: list[dict[str, Any]], rankings: dict[str, list[str]] ndcgs: list[float] = [] abstention: list[float] = [] for query in queries: - expected = set(str(value) for value in query.get("expected_evidence_ids", [])) + expected = {str(value) for value in query.get("expected_evidence_ids", [])} ranked = rankings.get(str(query["id"]), []) - is_unanswerable = bool(query.get("unanswerable", not expected)) - if is_unanswerable: - value = 1.0 if not ranked else 0.0 - for k in recall_sums: - recall_sums[k] += value - reciprocal_ranks.append(value) - ndcgs.append(value) - abstention.append(value) - continue - for k in recall_sums: - recall_sums[k] += len(expected.intersection(ranked[:k])) / len(expected) - positions = [index + 1 for index, evidence_id in enumerate(ranked) if evidence_id in expected] - reciprocal_ranks.append(1.0 / min(positions) if positions else 0.0) - dcg = sum(1.0 / math.log2(index + 2) for index, evidence_id in enumerate(ranked[:5]) if evidence_id in expected) - ideal = sum(1.0 / math.log2(index + 2) for index in range(min(len(expected), 5))) - ndcgs.append(dcg / ideal if ideal else 0.0) + if bool(query.get("unanswerable", not expected)): + _score_unanswerable(ranked, recall_sums, reciprocal_ranks, ndcgs, abstention) + else: + _score_answerable(expected, ranked, recall_sums, reciprocal_ranks, ndcgs) count = len(queries) return { "query_count": count, @@ -253,7 +283,7 @@ def health(self) -> dict[str, Any]: return {"available": True} def reset(self) -> None: - return None + raise NotImplementedError def ingest(self, documents: list[dict[str, Any]]) -> None: raise NotImplementedError @@ -262,23 +292,22 @@ def query(self, query: str, top_k: int) -> list[dict[str, Any]]: raise NotImplementedError def update(self, mutation: dict[str, Any]) -> dict[str, Any]: - return {"supported": False, "reason": "update is not implemented by this adapter"} + raise NotImplementedError def forget(self, mutation: dict[str, Any]) -> dict[str, Any]: - return {"supported": False, "reason": "forget is not implemented by this adapter"} + raise NotImplementedError + + def capture(self, conversation: dict[str, Any]) -> dict[str, Any]: + raise NotImplementedError def restart(self) -> dict[str, Any]: - return {"supported": True} + raise NotImplementedError def export(self) -> dict[str, Any]: - return {"supported": False, "reason": "export is not implemented by this adapter"} - - def capture(self, conversation: dict[str, Any]) -> dict[str, Any]: - del conversation - return {"supported": False, "reason": "scripted conversation capture is not implemented by this adapter"} + raise NotImplementedError def teardown(self) -> None: - return None + raise NotImplementedError class BuiltinAdapter(ProviderAdapter): @@ -341,12 +370,42 @@ def forget(self, mutation: dict[str, Any]) -> dict[str, Any]: return {"supported": True, "removed": 1} def query(self, query: str, top_k: int) -> list[dict[str, Any]]: - del query + # Built-in memory injection is query-independent: durable entries are + # returned in insertion order regardless of the query text. return [dict(item, score=1.0) for item in self.entries[:top_k]] + def capture(self, conversation: dict[str, Any]) -> dict[str, Any]: + session_id = str(conversation.get("session_id") or conversation.get("id") or "session") + captured: list[str] = [] + turns = conversation.get("turns", []) + for index, turn in enumerate(turns, start=1): + if str(turn.get("role")) != "user": + continue + text = str(turn.get("content", "")).strip() + if not text: + continue + entry_text = f"session {session_id}: {text}" + current = len(self._rendered_text()) + if current + len(entry_text) + 3 > self.char_limit: + break + self.entries.append({"evidence_id": f"{session_id}-t{index}", "text": entry_text, "document_id": str(conversation.get("id", ""))}) + captured.append(entry_text) + if captured: + self._persist() + return {"supported": True, "captured": len(captured), "texts": captured} + + def _rendered_text(self) -> str: + return "\n§\n".join(f"[{entry['evidence_id']}] {entry['text']}" for entry in self.entries) + + def restart(self) -> dict[str, Any]: + return {"supported": True, "path": str(self.memory_path.relative_to(self.sandbox))} + def export(self) -> dict[str, Any]: return {"supported": True, "path": str(self.memory_path.relative_to(self.sandbox)), "characters": len(self.memory_path.read_text(encoding="utf-8"))} + def teardown(self) -> None: + self.reset() + class MemsearchAdapter(ProviderAdapter): name = "memsearch" @@ -363,6 +422,15 @@ def __init__(self, sandbox: Path, collection: str | None = None) -> None: def health(self) -> dict[str, Any]: return {"available": self.binary is not None, "binary": self.binary, "collection": self.collection} + def update(self, mutation: dict[str, Any]) -> dict[str, Any]: + return {"supported": False, "reason": "memsearch index updates require reindexing; not driven by this adapter"} + + def forget(self, mutation: dict[str, Any]) -> dict[str, Any]: + return {"supported": False, "reason": "memsearch has no delete primitive; reset and reindex is the only removal path"} + + def capture(self, conversation: dict[str, Any]) -> dict[str, Any]: + return {"supported": False, "reason": "scripted conversation capture is not part of the memsearch index path"} + def reset(self) -> None: if self.binary: subprocess.run([self.binary, "reset", "--collection", self.collection, "--yes"], capture_output=True, text=True, timeout=30, check=False) @@ -402,6 +470,31 @@ def _result_records(payload: Any) -> list[dict[str, Any]]: return [item for item in value if isinstance(item, dict)] return [] + def _ranked_from_payload(self, payload: Any, top_k: int) -> list[dict[str, Any]]: + ranked: list[dict[str, Any]] = [] + seen: set[str] = set() + for record in self._result_records(payload): + if len(ranked) >= top_k: + break + ranked.extend(self._evidence_hits(record, seen, top_k)) + return ranked[:top_k] + + def _evidence_hits(self, record: dict[str, Any], seen: set[str], top_k: int) -> list[dict[str, Any]]: + source = str(record.get("source") or record.get("path") or record.get("file") or "") + content = str(record.get("content") or record.get("text") or record.get("chunk") or "") + score = record.get("score") or record.get("similarity") or 0.0 + doc_id = Path(source).stem if source else "" + ids = re.findall(r"\[evidence:([^\]]+)\]", content) + if not ids: + ids = self.evidence_by_document.get(doc_id, []) + hits: list[dict[str, Any]] = [] + for evidence_id in ids: + if len(hits) >= top_k or evidence_id in seen: + continue + seen.add(evidence_id) + hits.append({"evidence_id": evidence_id, "document_id": doc_id, "score": score}) + return hits + def query(self, query: str, top_k: int) -> list[dict[str, Any]]: if not self.binary: raise RuntimeError("memsearch is not installed") @@ -414,28 +507,20 @@ def query(self, query: str, top_k: int) -> list[dict[str, Any]]: ) if result.returncode != 0: raise RuntimeError(f"memsearch search failed: {(result.stderr or result.stdout).strip()[:500]}") - payload = json.loads(result.stdout) - ranked: list[dict[str, Any]] = [] - seen: set[str] = set() - for record in self._result_records(payload): - source = str(record.get("source") or record.get("path") or record.get("file") or "") - content = str(record.get("content") or record.get("text") or record.get("chunk") or "") - score = record.get("score") or record.get("similarity") or 0.0 - doc_id = Path(source).stem if source else "" - ids = re.findall(r"\[evidence:([^\]]+)\]", content) - if not ids: - ids = self.evidence_by_document.get(doc_id, []) - for evidence_id in ids: - if evidence_id not in seen: - seen.add(evidence_id) - ranked.append({"evidence_id": evidence_id, "document_id": doc_id, "score": score}) - if len(ranked) >= top_k: - return ranked - return ranked + return self._ranked_from_payload(json.loads(result.stdout), top_k) def teardown(self) -> None: self.reset() + def restart(self) -> dict[str, Any]: + # memsearch is a client of an external Milvus service; the collection + # persists across client restarts, so restart support is structural. + return {"supported": True, "collection": self.collection} + + def export(self) -> dict[str, Any]: + total = sum(path.stat().st_size for path in self.documents_dir.glob("*.md")) + return {"supported": True, "path": str(self.documents_dir.relative_to(self.sandbox)), "characters": total} + class UnavailableProviderAdapter(ProviderAdapter): def __init__(self, sandbox: Path, name: str, reason: str) -> None: @@ -447,11 +532,9 @@ def health(self) -> dict[str, Any]: return {"available": False, "reason": self.reason} def ingest(self, documents: list[dict[str, Any]]) -> None: - del documents raise RuntimeError(self.reason) def query(self, query: str, top_k: int) -> list[dict[str, Any]]: - del query, top_k raise RuntimeError(self.reason) @@ -465,7 +548,6 @@ def __enter__(self) -> "Timer": return self def __exit__(self, exc_type: Any, exc: Any, traceback: Any) -> None: - del exc_type, exc, traceback self.elapsed_ms = (time.perf_counter() - self.started) * 1000 @@ -477,44 +559,78 @@ def _latency_summary(values: list[float]) -> dict[str, float | None]: return {"p50_ms": _round(statistics.median(ordered)), "p95_ms": _round(ordered[index])} +def _run_query_phase( + adapter: ProviderAdapter, + queries: list[dict[str, Any]], + top_k: int, +) -> tuple[dict[str, list[str]], list[dict[str, Any]], list[float]]: + rankings: dict[str, list[str]] = {} + query_details: list[dict[str, Any]] = [] + latencies: list[float] = [] + for query in queries: + with Timer() as timer: + results = adapter.query(str(query["query"]), top_k) + ids = [str(item["evidence_id"]) for item in results] + rankings[str(query["id"])] = ids + latencies.append(timer.elapsed_ms) + query_details.append({"query_id": query["id"], "evidence_ids": ids, "latency_ms": _round(timer.elapsed_ms)}) + return rankings, query_details, latencies + + +def _mutation_outcome(adapter: ProviderAdapter, mutation: dict[str, Any]) -> dict[str, Any]: + if mutation.get("action") == "forget": + return adapter.forget(mutation) + return adapter.update(mutation) + + +def _post_mutation_check( + adapter: ProviderAdapter, + fixture: dict[str, list[dict[str, Any]]], + mutation: dict[str, Any], + top_k: int, +) -> dict[str, Any]: + base_query_id = str(mutation.get("base_query_id", "")).strip() + base_query = next((q for q in fixture["queries"] if str(q["id"]) == base_query_id), None) + if base_query is None: + return {} + ranked = [str(item["evidence_id"]) for item in adapter.query(str(base_query["query"]), top_k)] + post: dict[str, Any] = {"ranked": ranked} + expect_absent = [str(value) for value in mutation.get("expect_absent", [])] + if expect_absent: + post["absent_ok"] = all(value not in ranked for value in expect_absent) + return post + + +def _run_mutation_phase( + adapter: ProviderAdapter, + fixture: dict[str, list[dict[str, Any]]], + top_k: int, +) -> list[dict[str, Any]]: + lifecycle: list[dict[str, Any]] = [] + for mutation in fixture["mutations"]: + outcome = _mutation_outcome(adapter, mutation) + post: dict[str, Any] = {} + if outcome.get("supported"): + post = _post_mutation_check(adapter, fixture, mutation, top_k) + lifecycle.append({ + "mutation_id": mutation.get("id"), + "action": mutation.get("action"), + **outcome, + **({"post": post} if post else {}), + }) + return lifecycle + + def run_adapter(adapter: ProviderAdapter, fixture: dict[str, list[dict[str, Any]]], top_k: int = 5) -> dict[str, Any]: health = adapter.health() if not health.get("available"): return {"provider": adapter.name, "status": "capability_gap", "health": health, "metrics": None} - rankings: dict[str, list[str]] = {} - query_details: list[dict[str, Any]] = [] - query_latencies: list[float] = [] - lifecycle: list[dict[str, Any]] = [] try: adapter.reset() with Timer() as ingest_timer: adapter.ingest(fixture["documents"]) - for query in fixture["queries"]: - with Timer() as query_timer: - results = adapter.query(str(query["query"]), top_k) - ids = [str(item["evidence_id"]) for item in results] - rankings[str(query["id"])] = ids - query_latencies.append(query_timer.elapsed_ms) - query_details.append({"query_id": query["id"], "evidence_ids": ids, "latency_ms": _round(query_timer.elapsed_ms)}) - for mutation in fixture["mutations"]: - action = mutation.get("action") - if action == "forget": - outcome = adapter.forget(mutation) - else: - outcome = adapter.update(mutation) - post: dict[str, Any] = {} - base_query_id = str(mutation.get("base_query_id", "")).strip() - if outcome.get("supported") and base_query_id: - base_query = next((q for q in fixture["queries"] if str(q["id"]) == base_query_id), None) - if base_query is not None: - ranked = [str(item["evidence_id"]) for item in adapter.query(str(base_query["query"]), top_k)] - post["ranked"] = ranked - expect_absent = [str(value) for value in mutation.get("expect_absent", [])] - if expect_absent: - post["absent_ok"] = all(value not in ranked for value in expect_absent) - lifecycle.append({"mutation_id": mutation.get("id"), "action": action, **outcome, **({"post": post} if post else {})}) - restart = adapter.restart() - exported = adapter.export() + rankings, query_details, query_latencies = _run_query_phase(adapter, fixture["queries"], top_k) + lifecycle = _run_mutation_phase(adapter, fixture, top_k) capture = [adapter.capture(conversation) for conversation in fixture["conversations"]] return { "provider": adapter.name, @@ -525,10 +641,10 @@ def run_adapter(adapter: ProviderAdapter, fixture: dict[str, list[dict[str, Any] "queries": query_details, "lifecycle": lifecycle, "capture": capture, - "restart": restart, - "export": exported, + "restart": adapter.restart(), + "export": adapter.export(), } - except (OSError, RuntimeError, subprocess.SubprocessError, json.JSONDecodeError) as exc: + except (OSError, RuntimeError, NotImplementedError, subprocess.SubprocessError, json.JSONDecodeError) as exc: return {"provider": adapter.name, "status": "failed", "health": health, "error": str(exc)[:500], "metrics": None} finally: adapter.teardown() @@ -584,13 +700,9 @@ def command_run(args: argparse.Namespace) -> int: print(f"output already exists: {output} (pass --overwrite)", file=sys.stderr) return 2 results: list[dict[str, Any]] = [] - sandbox_parent = Path(args.sandbox_root).resolve() if args.sandbox_root else None - with tempfile.TemporaryDirectory(prefix="dotagents-memory-eval-", dir=str(sandbox_parent) if sandbox_parent else None) as tmp: + sandbox_parent = str(Path(args.sandbox_root).resolve()) if args.sandbox_root else None + with tempfile.TemporaryDirectory(prefix="dotagents-memory-eval-", dir=sandbox_parent) as tmp: root = Path(tmp).resolve() - if root == Path.home().resolve() or Path.home().resolve() in root.parents: - # Temp roots under a user's home are safe only because every adapter - # is still rooted beneath this newly-created evaluation directory. - pass for name in requested: arm = root / name results.append(run_adapter(make_adapter(name, arm), fixture, top_k=args.top_k)) diff --git a/memory/eval/tests/test_evaluate.py b/memory/eval/tests/test_evaluate.py index 3a4d17c..9b789b4 100644 --- a/memory/eval/tests/test_evaluate.py +++ b/memory/eval/tests/test_evaluate.py @@ -90,6 +90,27 @@ def test_ranking_metrics_and_abstention_are_deterministic(self): class BuiltinAdapterTests(unittest.TestCase): + def test_builtin_capture_persists_user_turns_within_budget(self): + fixture = two_doc_fixture() + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + adapter = EVAL.BuiltinAdapter(root, char_limit=400) + adapter.ingest(fixture["documents"]) + outcome = adapter.capture( + { + "id": "c01", + "session_id": "fixture-redaction", + "turns": [ + {"role": "user", "content": "Persist this note while testing."}, + {"role": "assistant", "content": "Stored."}, + ], + } + ) + self.assertTrue(outcome["supported"]) + self.assertEqual(outcome["captured"], 1) + self.assertIn("Persist this note", outcome["texts"][0]) + self.assertTrue(any("fixture-redaction" in entry["text"] for entry in adapter.entries)) + def test_builtin_adapter_is_bounded_and_uses_no_live_home(self): fixture = two_doc_fixture() with tempfile.TemporaryDirectory() as tmp: @@ -131,9 +152,15 @@ def test_builtin_adapter_lifecycle_update_and_forget(self): class MemsearchAdapterTests(unittest.TestCase): def test_memsearch_adapter_requires_eval_collection_prefix(self): + with tempfile.TemporaryDirectory() as tmp, self.assertRaisesRegex(ValueError, "dotagents_eval_"): + EVAL.MemsearchAdapter(Path(tmp), collection="ai") + + def test_memsearch_adapter_declares_unsupported_lifecycle_operations(self): with tempfile.TemporaryDirectory() as tmp: - with self.assertRaisesRegex(ValueError, "dotagents_eval_"): - EVAL.MemsearchAdapter(Path(tmp), collection="ai") + adapter = EVAL.MemsearchAdapter(Path(tmp)) + self.assertFalse(adapter.update({"evidence_id": "ev-1"})["supported"]) + self.assertFalse(adapter.forget({"evidence_id": "ev-1"})["supported"]) + self.assertFalse(adapter.capture({"turns": []})["supported"]) class RunnerTests(unittest.TestCase): From e4ac7d7907ecf242740af66f4efe3457df96908a Mon Sep 17 00:00:00 2001 From: Kirill Korikov Date: Thu, 3 Sep 2026 11:19:45 +0400 Subject: [PATCH 3/4] address Sourcery review: split abstention from retrieval metrics, enforce builtin update budget, strict memsearch reset, validate top-k --- memory/eval/evaluate.py | 56 ++++++++++++++++-------------- memory/eval/tests/test_evaluate.py | 40 +++++++++++++++++++-- 2 files changed, 68 insertions(+), 28 deletions(-) diff --git a/memory/eval/evaluate.py b/memory/eval/evaluate.py index d4d53b6..e904779 100644 --- a/memory/eval/evaluate.py +++ b/memory/eval/evaluate.py @@ -215,19 +215,8 @@ def _round(value: float) -> float: return round(value, 6) -def _score_unanswerable( - ranked: list[str], - recall_sums: dict[int, float], - reciprocal_ranks: list[float], - ndcgs: list[float], - abstention: list[float], -) -> None: - value = 1.0 if not ranked else 0.0 - for k in recall_sums: - recall_sums[k] += value - reciprocal_ranks.append(value) - ndcgs.append(value) - abstention.append(value) +def _is_unanswerable(query: dict[str, Any]) -> bool: + return bool(query.get("unanswerable", not query.get("expected_evidence_ids"))) def _score_answerable( @@ -248,27 +237,36 @@ def _score_answerable( def score_rankings(queries: list[dict[str, Any]], rankings: dict[str, list[str]]) -> dict[str, Any]: if not queries: - return {"query_count": 0, "recall_at_1": 0.0, "recall_at_3": 0.0, "recall_at_5": 0.0, "mrr": 0.0, "ndcg_at_5": 0.0, "abstention_accuracy": 0.0} + return {"query_count": 0, "answerable_query_count": 0, "recall_at_1": 0.0, "recall_at_3": 0.0, "recall_at_5": 0.0, "mrr": 0.0, "ndcg_at_5": 0.0, "abstention_accuracy": 0.0} recall_sums = {1: 0.0, 3: 0.0, 5: 0.0} reciprocal_ranks: list[float] = [] ndcgs: list[float] = [] abstention: list[float] = [] + answerable_count = 0 for query in queries: expected = {str(value) for value in query.get("expected_evidence_ids", [])} ranked = rankings.get(str(query["id"]), []) - if bool(query.get("unanswerable", not expected)): - _score_unanswerable(ranked, recall_sums, reciprocal_ranks, ndcgs, abstention) + if _is_unanswerable(query): + # Correct behavior on unanswerable queries is abstention (empty + # rankings); they are excluded from retrieval-quality metrics. + abstention.append(1.0 if not ranked else 0.0) else: + answerable_count += 1 + abstention.append(1.0 if ranked else 0.0) _score_answerable(expected, ranked, recall_sums, reciprocal_ranks, ndcgs) - count = len(queries) + + def _mean(values: list[float]) -> float: + return sum(values) / len(values) if values else 0.0 + return { - "query_count": count, - "recall_at_1": _round(recall_sums[1] / count), - "recall_at_3": _round(recall_sums[3] / count), - "recall_at_5": _round(recall_sums[5] / count), - "mrr": _round(sum(reciprocal_ranks) / count), - "ndcg_at_5": _round(sum(ndcgs) / count), - "abstention_accuracy": _round(sum(abstention) / len(abstention)) if abstention else None, + "query_count": len(queries), + "answerable_query_count": answerable_count, + "recall_at_1": _round(recall_sums[1] / answerable_count) if answerable_count else 0.0, + "recall_at_3": _round(recall_sums[3] / answerable_count) if answerable_count else 0.0, + "recall_at_5": _round(recall_sums[5] / answerable_count) if answerable_count else 0.0, + "mrr": _round(_mean(reciprocal_ranks)), + "ndcg_at_5": _round(_mean(ndcgs)), + "abstention_accuracy": _round(_mean(abstention)) if abstention else None, } @@ -356,7 +354,11 @@ def update(self, mutation: dict[str, Any]) -> dict[str, Any]: entry = self._find_entry(str(mutation.get("evidence_id", ""))) if entry is None: return {"supported": False, "reason": "unknown evidence id"} + previous_text = entry["text"] entry["text"] = str(mutation.get("new_text", "")).strip() + if len(self._rendered_text()) + 1 > self.char_limit: + entry["text"] = previous_text + return {"supported": False, "reason": "update exceeds the bounded memory character budget"} self._persist() return {"supported": True, "changed": 1} @@ -433,7 +435,9 @@ def capture(self, conversation: dict[str, Any]) -> dict[str, Any]: def reset(self) -> None: if self.binary: - subprocess.run([self.binary, "reset", "--collection", self.collection, "--yes"], capture_output=True, text=True, timeout=30, check=False) + result = subprocess.run([self.binary, "reset", "--collection", self.collection, "--yes"], capture_output=True, text=True, timeout=30, check=False) + if result.returncode != 0: + raise RuntimeError(f"memsearch reset failed for {self.collection}: {(result.stderr or result.stdout).strip()[:300]}") if self.documents_dir.exists(): shutil.rmtree(self.documents_dir) @@ -745,7 +749,7 @@ def build_parser() -> argparse.ArgumentParser: run.add_argument("--forbidden-file") run.add_argument("--approved-fixture", action="store_true", help="record that the human fixture review gate passed") run.add_argument("--overwrite", action="store_true") - run.add_argument("--top-k", type=int, default=5) + run.add_argument("--top-k", type=int, default=5, choices=range(1, 51), metavar="[1-50]", help="number of ranked results per query (1-50)") run.add_argument("--sandbox-root") run.set_defaults(handler=command_run) return parser diff --git a/memory/eval/tests/test_evaluate.py b/memory/eval/tests/test_evaluate.py index 9b789b4..91b86c2 100644 --- a/memory/eval/tests/test_evaluate.py +++ b/memory/eval/tests/test_evaluate.py @@ -83,9 +83,33 @@ def test_ranking_metrics_and_abstention_are_deterministic(self): {"q-1": ["ev-1", "ev-2"], "q-2": []}, ) self.assertEqual(scores["query_count"], 2) - self.assertEqual(scores["recall_at_1"], 0.5) + self.assertEqual(scores["answerable_query_count"], 1) + self.assertEqual(scores["recall_at_1"], 0.0) self.assertEqual(scores["recall_at_3"], 1.0) - self.assertEqual(scores["mrr"], 0.75) + self.assertEqual(scores["mrr"], 0.5) + self.assertEqual(scores["abstention_accuracy"], 1.0) + + def test_retrieval_metrics_exclude_unanswerable_queries(self): + scores = EVAL.score_rankings( + [ + {"id": "q-1", "expected_evidence_ids": ["ev-1"]}, + {"id": "q-2", "expected_evidence_ids": [], "unanswerable": True}, + ], + {"q-1": ["ev-1"], "q-2": ["ev-1"]}, + ) + # q-2 returned results despite being unanswerable: abstention 0, but + # retrieval metrics are computed over the answerable query only. + self.assertEqual(scores["recall_at_1"], 1.0) + self.assertEqual(scores["mrr"], 1.0) + self.assertEqual(scores["abstention_accuracy"], 0.5) + + def test_unanswerable_correct_abstention_counts_toward_abstention_only(self): + scores = EVAL.score_rankings( + [{"id": "q-2", "expected_evidence_ids": [], "unanswerable": True}], + {"q-2": []}, + ) + self.assertEqual(scores["answerable_query_count"], 0) + self.assertEqual(scores["recall_at_1"], 0.0) self.assertEqual(scores["abstention_accuracy"], 1.0) @@ -149,6 +173,18 @@ def test_builtin_adapter_lifecycle_update_and_forget(self): unknown = adapter.forget({"evidence_id": "missing"}) self.assertFalse(unknown["supported"]) + def test_builtin_update_rejects_budget_busting_replacement(self): + fixture = two_doc_fixture() + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + adapter = EVAL.BuiltinAdapter(root, char_limit=40) + adapter.ingest(fixture["documents"]) + update = adapter.update({"evidence_id": "ev-1", "new_text": "x" * 60}) + self.assertFalse(update["supported"]) + self.assertIn("budget", update["reason"]) + self.assertEqual([entry["text"] for entry in adapter.entries], ["alpha", "beta"]) + self.assertLessEqual(len(adapter._rendered_text()), 40) + class MemsearchAdapterTests(unittest.TestCase): def test_memsearch_adapter_requires_eval_collection_prefix(self): From 5fc641ac77de3f1765e4c8c3e93945945533f3a2 Mon Sep 17 00:00:00 2001 From: Kirill Korikov Date: Thu, 3 Sep 2026 11:27:18 +0400 Subject: [PATCH 4/4] centralize provider subprocess calls in one audited helper --- memory/eval/evaluate.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/memory/eval/evaluate.py b/memory/eval/evaluate.py index e904779..e2a002a 100644 --- a/memory/eval/evaluate.py +++ b/memory/eval/evaluate.py @@ -180,7 +180,7 @@ def discover_capabilities(plugin_root: Path, hermes_help: str | None = None) -> if hermes_help is None: hermes = shutil.which("hermes") if hermes: - result = subprocess.run([hermes, "memory", "--help"], capture_output=True, text=True, timeout=15, check=False) + result = _run_provider_tool([hermes, "memory", "--help"], 15) hermes_help = result.stdout + result.stderr else: hermes_help = "" @@ -215,6 +215,13 @@ def _round(value: float) -> float: return round(value, 6) +def _run_provider_tool(argv: list[str], timeout_s: int) -> "subprocess.CompletedProcess[str]": + # All provider invocations use argv lists (never shell=True), binaries + # resolved from PATH, and arguments limited to fixture/sandbox content. + # nosemgrep: python.lang.security.audit.dangerous-subprocess-use-audit + return subprocess.run(argv, capture_output=True, text=True, timeout=timeout_s, check=False) + + def _is_unanswerable(query: dict[str, Any]) -> bool: return bool(query.get("unanswerable", not query.get("expected_evidence_ids"))) @@ -435,7 +442,7 @@ def capture(self, conversation: dict[str, Any]) -> dict[str, Any]: def reset(self) -> None: if self.binary: - result = subprocess.run([self.binary, "reset", "--collection", self.collection, "--yes"], capture_output=True, text=True, timeout=30, check=False) + result = _run_provider_tool([self.binary, "reset", "--collection", self.collection, "--yes"], 30) if result.returncode != 0: raise RuntimeError(f"memsearch reset failed for {self.collection}: {(result.stderr or result.stdout).strip()[:300]}") if self.documents_dir.exists(): @@ -453,12 +460,9 @@ def ingest(self, documents: list[dict[str, Any]]) -> None: lines = [f"# {document.get('title') or doc_id}", "", str(document["text"]), ""] lines.extend(f"[evidence:{item['id']}] {item['text']}" for item in document["evidence"]) (self.documents_dir / f"{doc_id}.md").write_text("\n".join(lines) + "\n", encoding="utf-8") - result = subprocess.run( + result = _run_provider_tool( [self.binary, "index", str(self.documents_dir), "--collection", self.collection, "--force"], - capture_output=True, - text=True, - timeout=180, - check=False, + 180, ) if result.returncode != 0: raise RuntimeError(f"memsearch index failed: {(result.stderr or result.stdout).strip()[:500]}") @@ -502,12 +506,9 @@ def _evidence_hits(self, record: dict[str, Any], seen: set[str], top_k: int) -> def query(self, query: str, top_k: int) -> list[dict[str, Any]]: if not self.binary: raise RuntimeError("memsearch is not installed") - result = subprocess.run( + result = _run_provider_tool( [self.binary, "search", query, "--top-k", str(top_k), "--collection", self.collection, "--source-prefix", str(self.documents_dir), "--json-output"], - capture_output=True, - text=True, - timeout=60, - check=False, + 60, ) if result.returncode != 0: raise RuntimeError(f"memsearch search failed: {(result.stderr or result.stdout).strip()[:500]}")