From 9ad385ec5ebdc005cd3bddabdbcd94cc6a54d039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sertu=C4=9F=20Kaya?= Date: Mon, 9 Nov 2026 13:13:49 +0000 Subject: [PATCH 1/6] feat(hermes-memory): opt-in semantic recall with hybrid search --- python/hermes-memory/CHANGELOG.md | 18 + python/hermes-memory/README.md | 41 ++- python/hermes-memory/plugin.yaml | 2 +- python/hermes-memory/pyproject.toml | 2 +- .../src/upstash_hermes_memory/_search.py | 129 ++++++- .../src/upstash_hermes_memory/_version.py | 2 +- .../src/upstash_hermes_memory/chat_history.py | 6 +- .../src/upstash_hermes_memory/memory.py | 119 ++++++- .../src/upstash_hermes_memory/migrate.py | 203 +++++++++++ .../src/upstash_hermes_memory/provider.py | 48 ++- python/hermes-memory/tests/test_semantic.py | 314 ++++++++++++++++++ .../tests/test_semantic_redis.py | 117 +++++++ uv.lock | 2 +- 13 files changed, 963 insertions(+), 40 deletions(-) create mode 100644 python/hermes-memory/src/upstash_hermes_memory/migrate.py create mode 100644 python/hermes-memory/tests/test_semantic.py create mode 100644 python/hermes-memory/tests/test_semantic_redis.py diff --git a/python/hermes-memory/CHANGELOG.md b/python/hermes-memory/CHANGELOG.md index 0c4556b..50f9c2e 100644 --- a/python/hermes-memory/CHANGELOG.md +++ b/python/hermes-memory/CHANGELOG.md @@ -4,6 +4,24 @@ Written by hand: this package is not versioned by Changesets. To release, bump ` `pyproject.toml`, run `node scripts/sync-version.mjs` and `uv lock`, and add a `## ` section here. The release workflow publishes any version PyPI does not have yet. +## 0.3.0 + +### Minor Changes + +- Semantic recall, opt in with `UPSTASH_HERMES_SEMANTIC=1` (or `AgentMemory(semantic=True)`): the + memory `text` field becomes a Redis Search `HYBRID` field, and recall merges keyword (`$smart`) + and semantic (`$ann`) results with `$rrf`. Off by default; nothing changes without the flag. +- `python -m upstash_hermes_memory.migrate --to hybrid|keyword` rebuilds an existing shared index + either way, with clients stopped, and checks the counts afterwards. An existing index is never + dropped at runtime: with the flag on and an unmigrated index, recall stays keyword and warns. +- `AgentMemory.recall` takes `mode="auto"|"keyword"`; records carry `retrieval`. A failed hybrid + query falls back to keyword once. `min_score` applies to keyword results only. +- Automatic recall injects 3 memories in semantic mode (5 otherwise). +- `upstash_memory_search` results include `retrieval`. +- Fix: an empty result from a retriever query no longer sends `SEARCH.COUNT` with that query + (Search rejects `$ann` in COUNT); the missing-index check counts `{}` instead. +- Docs: the shared `agentkit` index is Python-only; the TS SDK creates its own indexes. + ## 0.2.2 ### Patch Changes diff --git a/python/hermes-memory/README.md b/python/hermes-memory/README.md index aa599f4..3e7cefc 100644 --- a/python/hermes-memory/README.md +++ b/python/hermes-memory/README.md @@ -7,6 +7,7 @@ your AI SDK app saves is recalled by your Hermes agent, and the other way round. ## What it does in Hermes - **Recall:** before each turn, the memories closest to the user's message are added to the prompt. + By default by keyword; with [semantic recall](#semantic-recall) also by meaning. - **Chat history:** after each turn, the turn is appended in place to the session's searchable transcript document, so a write stays the size of one turn however long the session runs. - **Mirroring:** facts the built-in `memory` tool adds are also saved to Redis, and when Hermes @@ -39,6 +40,43 @@ these): | `UPSTASH_HERMES_USER_ID` | Whose memories these are. Defaults to the gateway user, then the Hermes profile name, then `default`. | | `UPSTASH_HERMES_NAMESPACE` | Key namespace. Default `agentkit`, so memories sit at the TS SDK's default keys and are shared with AgentKit apps on the same database. | | `UPSTASH_HERMES_TELEMETRY` | Set to `1` to send telemetry. Off by default; see [Telemetry](#telemetry). | +| `UPSTASH_HERMES_SEMANTIC` | Set to `1` to recall memories by meaning as well as by words. Off by default; see [Semantic recall](#semantic-recall). | + +## Semantic recall + +Keyword recall (`$smart`: phrase, term and fuzzy matching) only finds a memory that shares words +with the message: "where is the assistant hosted" misses "Hermes runs on the ops Box". With +`UPSTASH_HERMES_SEMANTIC=1`, the memory `text` field is a `HYBRID` field, which Upstash Redis +Search embeds itself (no separate vector database), and recall merges a keyword query with a +semantic `$ann` query using `$rrf` +([Hybrid Search](https://upstash.com/docs/redis/search/hybrid-search)). + +- A **new** index is created that way. An **existing** index keeps working with keyword recall + (a warning says so) until you migrate it. Stop every client of the namespace (Hermes gateway, + desktop app, cron jobs, other profiles on it), then: + + ```bash + python -m upstash_hermes_memory.migrate --to hybrid --env-file ~/.hermes/.env + ``` + + For a directory (catalog) install, run it from the plugin's `src` directory with the Python + Hermes runs in. The index is dropped and created again over the same keys, which are not + touched, and the script checks that it holds as many memories and chats as before. +- **Rollback:** `--to keyword` restores the 0.2.x index; do that before installing 0.2.x again, + which fails on a hybrid index. Turning the variable off is not enough: an existing hybrid index + keeps embedding new memories, it is only no longer queried by meaning. +- Automatic recall injects up to **3** memories instead of 5: the semantic side has no relevance + threshold and always returns its limit, and its similarity scores did not separate related from + unrelated memories well enough in testing for a cutoff. `upstash_memory_search` keeps its limit. +- Results carry `retrieval: "hybrid"` or `"keyword"`. Hybrid scores are rank-fusion values that + only order one search's results; `min_score` applies to keyword results only. +- If a hybrid query fails, recall falls back to keyword search for that call (logged at most + every 10 minutes). +- Chat history stays keyword-only: a `HYBRID` field gets one embedding for its whole value, and a + chat document holds a whole session. +- Every memory in the index is embedded, whichever user or profile wrote it, and each recall + sends the message to Upstash Redis Search to be embedded. A hybrid query adds about 150 to + 300 ms to recall (keyword recall takes a few ms). ## Telemetry @@ -78,4 +116,5 @@ there. One session saves two facts; a fresh session started right after must nam calling any tool (checked in Hermes's `state.db`), so they can only come from the automatic recall; and a control session as another user must not know them. The script then checks the keys and transcripts in Redis (and, if `packages/sdk` is built, that the TS `AgentMemory` recalls them too) -before cleaning up. +before cleaning up. Tests that rebuild indexes (`test_semantic_redis.py`) need a database of their +own: never point `UPSTASH_REDIS_REST_*` at a database with data you care about when running them. diff --git a/python/hermes-memory/plugin.yaml b/python/hermes-memory/plugin.yaml index e4512ac..b1c0f05 100644 --- a/python/hermes-memory/plugin.yaml +++ b/python/hermes-memory/plugin.yaml @@ -3,7 +3,7 @@ # https://hermes-agent.nousresearch.com/docs/developer-guide/plugins/catalog-submission name: upstash # Copied from pyproject.toml by scripts/sync-version.mjs. -version: "0.2.2" +version: "0.3.0" description: "Upstash Redis memory provider: long-term memory, searchable chat history and recall on Redis Search, sharing keys with @upstash/agentkit-sdk." # Also declared in the catalog entry; older Hermes releases only enforce this one (catalog rule 14). requires_hermes: ">=0.19" diff --git a/python/hermes-memory/pyproject.toml b/python/hermes-memory/pyproject.toml index bfc2537..3f6f25e 100644 --- a/python/hermes-memory/pyproject.toml +++ b/python/hermes-memory/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "upstash-hermes-memory" -version = "0.2.2" +version = "0.3.0" description = "Upstash Redis memory provider for Hermes Agent: long-term memory, searchable chat history and recall on Redis Search, with no separate vector database." readme = "README.md" license = "MIT" diff --git a/python/hermes-memory/src/upstash_hermes_memory/_search.py b/python/hermes-memory/src/upstash_hermes_memory/_search.py index 2c21feb..cbc693d 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/_search.py +++ b/python/hermes-memory/src/upstash_hermes_memory/_search.py @@ -1,7 +1,8 @@ """A Redis Search index that is created on first use, shared by AgentMemory and ChatHistory. -The commands are the ones `ReactiveSearchIndex` sends from the TS SDK, argument for argument, so -an index created from either language is the same index. +A store with its own `prefix` sends the commands `ReactiveSearchIndex` sends from the TS SDK, +argument for argument, so such an index is the same from either language (unless `semantic` makes +`text` HYBRID, which the TS SDK does not do yet). The default shared index is Python-only. """ import json @@ -25,8 +26,11 @@ def dumps(value: Any) -> str: DEFAULT_NAMESPACE = "agentkit" -# The one index AgentMemory and ChatHistory share by default (the TS `SharedIndexSchema`, field -# for field and in the same order). Upstash's Free plan allows one search index per database. +# Retrievers ($rrf, $ann) must be the only key of the query object and cannot be counted. +RETRIEVERS = frozenset({"$rrf", "$ann"}) + +# The one index AgentMemory and ChatHistory share by default (Python only: the TS SDK gives each +# store its own index). Upstash's Free plan allows one search index per database. # `kind` ("memory" or "chat") keeps the two apart in every query. SHARED_SCHEMA: list[Field] = [ ("kind", "TEXT", True), @@ -38,31 +42,117 @@ def dumps(value: Any) -> str: ] -def shared_index(redis: Any, namespace: str) -> "SearchIndex": +def with_hybrid_text(schema: Sequence[Field]) -> list[Field]: + """`schema` with the memory `text` field as HYBRID (full-text plus an embedding for `$ann`). + + Only `text` changes: chat transcripts stay TEXT, since a HYBRID field gets one embedding for + its whole value and `userMessages` / `modelMessages` hold a whole session each. + """ + return [(f, "HYBRID" if f == "text" else t, nt) for f, t, nt in schema] + + +def shared_index(redis: Any, namespace: str, *, semantic: bool = False) -> "SearchIndex": """The shared index of a namespace, covering `:memory:` and `:chat:`. Both stores build this same definition, so whichever reads first creates it and the other's - EXISTSOK create is a no-op; the TS SDK builds it identically. + EXISTSOK create is a no-op. `semantic` only decides what a new index is created with: an + existing index with `text` as either TEXT or HYBRID is used as it is (see `SearchIndex.ensure`), + and changing it is a maintenance step (`python -m upstash_hermes_memory.migrate`). """ + hybrid = with_hybrid_text(SHARED_SCHEMA) return SearchIndex( redis, name=index_name_for(namespace), prefix=[f"{namespace}:memory:", f"{namespace}:chat:"], - schema=SHARED_SCHEMA, + schema=hybrid if semantic else SHARED_SCHEMA, + accepted=[SHARED_SCHEMA, hybrid], + ) + + +def _is_config_mismatch(error: Exception) -> bool: + return "different configuration" in str(error) + + +def describe(redis: Any, name: str) -> dict[str, Any] | None: + """SEARCH.DESCRIBE as {"type", "prefixes", "language", "schema": [Field...]}, or None if missing. + + Accepts the client's parsed `IndexDescription` as well as the raw reply, so it does not depend + on the client knowing every field type (upstash-redis 1.8 has no HYBRID enum member). + """ + raw = redis.execute(["SEARCH.DESCRIBE", name]) + if raw is None or raw == []: + return None + if isinstance(raw, list): + pairs = dict(zip(raw[0::2], raw[1::2])) + fields = [ + (f[0], str(f[1]).upper(), "NOTOKENIZE" in f[2:], "NOSTEM" in f[2:], "FROM" in f[2:]) + for f in pairs.get("schema") or [] + ] + data_type, prefixes, language = ( + pairs.get("type"), + pairs.get("prefixes"), + pairs.get("language"), + ) + else: + fields = [ + ( + n, + str(getattr(i.type, "value", i.type)).upper(), + i.no_tokenize, + i.no_stem, + bool(i.alias), + ) + for n, i in raw.schema.items() + ] + data_type, prefixes, language = raw.data_type, raw.prefixes, raw.language + return { + "type": str(getattr(data_type, "value", data_type) or "").upper(), + "prefixes": sorted(prefixes or []), + "language": str(getattr(language, "value", language) or "").lower(), + # NOSTEM and FROM are never set by this package, so their presence is a mismatch. + "schema": sorted(fields), + } + + +def matches( + description: dict[str, Any] | None, prefixes: Sequence[str], schema: Sequence[Field] +) -> bool: + """Whether a `describe()` result is exactly the index this package would create.""" + if description is None: + return False + return ( + description["type"] == "JSON" + and description["prefixes"] == sorted(prefixes) + and description["language"] in ("", "english") + and description["schema"] == sorted((f, t, nt, False, False) for f, t, nt in schema) ) class SearchIndex: def __init__( - self, redis: Any, *, name: str, prefix: str | Sequence[str], schema: Sequence[Field] + self, + redis: Any, + *, + name: str, + prefix: str | Sequence[str], + schema: Sequence[Field], + accepted: Sequence[Sequence[Field]] = (), ): self.redis = redis self.name = name self.prefixes = [prefix] if isinstance(prefix, str) else list(prefix) self.schema = list(schema) + # Other definitions an existing index may have and still be used as it is. + self.accepted = [list(s) for s in accepted if list(s) != self.schema] + # The definition the index actually has, known once `ensure()` has run. + self.actual_schema: list[Field] | None = None self._ready = False self._lock = threading.Lock() + def field_type(self, field: str) -> str | None: + """The type `field` has in the live index (after `ensure()`), e.g. "TEXT" or "HYBRID".""" + return next((t for f, t, _ in self.actual_schema or [] if f == field), None) + def create_command(self) -> list[str]: cmd = ["SEARCH.CREATE", self.name, "EXISTSOK", "ON", "JSON", "PREFIX"] cmd += [str(len(self.prefixes)), *self.prefixes] @@ -72,20 +162,37 @@ def create_command(self) -> list[str]: return cmd def ensure(self) -> None: - """Create the index if it does not exist yet. A no-op after the first call.""" + """Create the index if it does not exist yet. A no-op after the first call. + + An existing index whose definition differs only in a way this package knows (an `accepted` + schema, such as `text` TEXT vs HYBRID) is used as it is, never dropped: rebuilding an index + under running clients is unsafe, so it is left to `python -m upstash_hermes_memory.migrate`. + """ if self._ready: return with self._lock: if self._ready: return - self.redis.execute(self.create_command()) + try: + self.redis.execute(self.create_command()) + actual = self.schema + except Exception as error: + if not (self.accepted and _is_config_mismatch(error)): + raise + found = describe(self.redis, self.name) + actual = next((s for s in self.accepted if matches(found, self.prefixes, s)), None) + if actual is None: + raise self.redis.execute(["SEARCH.WAITINDEXING", self.name]) + self.actual_schema = list(actual) self._ready = True def query(self, filter: dict[str, Any], limit: int | None = None) -> list[Any]: self.ensure() rows = self._query(filter, limit) - if not rows and self._raw_count(filter) < 0: + # COUNT cannot take a retriever, so a retriever query checks the index with `{}` instead. + probe = {} if RETRIEVERS & set(filter) else filter + if not rows and self._raw_count(probe) < 0: # The Python client turns a missing index into [], so an empty result is ambiguous. # COUNT says -1 when the index is gone (another client dropped it): recreate, retry once. self._ready = False diff --git a/python/hermes-memory/src/upstash_hermes_memory/_version.py b/python/hermes-memory/src/upstash_hermes_memory/_version.py index 5228795..713288d 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/_version.py +++ b/python/hermes-memory/src/upstash_hermes_memory/_version.py @@ -1,2 +1,2 @@ # Generated by scripts/sync-version.mjs from pyproject.toml — do not edit by hand. -VERSION = "0.2.2" +VERSION = "0.3.0" diff --git a/python/hermes-memory/src/upstash_hermes_memory/chat_history.py b/python/hermes-memory/src/upstash_hermes_memory/chat_history.py index dbd43fd..a952bb4 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/chat_history.py +++ b/python/hermes-memory/src/upstash_hermes_memory/chat_history.py @@ -104,15 +104,19 @@ def __init__( index_name: str | None = None, ttl_seconds: int | None = None, min_score: float = 0, + semantic: bool = False, enable_telemetry: bool = False, ): + """`semantic` only matters on the shared index: it decides whether a new index gets the + memory `text` field as HYBRID, so memory and chats create it the same way. Chat search + itself stays keyword search ($smart).""" self.redis = redis self.ttl_seconds = ttl_seconds self.min_score = min_score if prefix is None and index_name is None: self.prefix = f"{namespace}:chat" self.kind: str | None = "chat" - self.index = shared_index(redis, namespace) + self.index = shared_index(redis, namespace, semantic=semantic) else: self.prefix = prefix or DEFAULT_PREFIX self.kind = None diff --git a/python/hermes-memory/src/upstash_hermes_memory/memory.py b/python/hermes-memory/src/upstash_hermes_memory/memory.py index f267133..128671e 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/memory.py +++ b/python/hermes-memory/src/upstash_hermes_memory/memory.py @@ -13,16 +13,33 @@ Passing `prefix` (or `index_name`) gives the store its own index over `:` instead. """ +import logging import time import uuid from dataclasses import dataclass, field -from typing import Any - -from ._search import DEFAULT_NAMESPACE, SearchIndex, dumps, index_name_for, shared_index +from typing import Any, Literal + +from upstash_redis.errors import UpstashError + +from ._search import ( + DEFAULT_NAMESPACE, + SearchIndex, + dumps, + index_name_for, + shared_index, + with_hybrid_text, +) from ._telemetry import add_telemetry +logger = logging.getLogger(__name__) + DEFAULT_PREFIX = f"{DEFAULT_NAMESPACE}:memory" +# How often a failing semantic query or an index without embeddings is logged, per store. +_WARN_EVERY_S = 600.0 + +RecallMode = Literal["auto", "keyword"] + # The document's own fields. Metadata is spread next to them, so it must not replace them: a # `userId` in metadata would move the memory into another user's searches. RESERVED_FIELDS = frozenset({"kind", "text", "userId", "createdAt"}) @@ -47,6 +64,10 @@ class MemoryRecord: created_at: int score: float | None = None metadata: dict[str, Any] = field(default_factory=dict) + # How a recalled record was found: "keyword" ($smart, BM25 score) or "hybrid" ($rrf of $smart + # and $ann; the score is a rank-fusion value, only comparable within one query). None for + # records that were not searched for (get, list, an empty query). + retrieval: str | None = None class AgentMemory: @@ -58,21 +79,32 @@ def __init__( prefix: str | None = None, index_name: str | None = None, min_score: float = 0, + semantic: bool = False, enable_telemetry: bool = False, ): + """`semantic=True` recalls with hybrid search (full-text plus meaning) when the index has + `text` as a HYBRID field, and creates a new index that way. An existing TEXT index keeps + working with keyword recall until it is migrated (`python -m upstash_hermes_memory.migrate`). + `min_score` applies to keyword results only; hybrid scores are not on the same scale. + """ self.redis = redis self.min_score = min_score + self.semantic = semantic self.shared = prefix is None and index_name is None + self._warned_at: dict[str, float] = {} if self.shared: self.prefix = f"{namespace}:memory" - self.index = shared_index(redis, namespace) + self.index = shared_index(redis, namespace, semantic=semantic) else: self.prefix = prefix or DEFAULT_PREFIX + text_schema = [("text", "TEXT", False), ("userId", "TEXT", True)] + hybrid_schema = with_hybrid_text(text_schema) self.index = SearchIndex( redis, name=index_name or index_name_for(self.prefix), prefix=f"{self.prefix}:", - schema=[("text", "TEXT", False), ("userId", "TEXT", True)], + schema=hybrid_schema if semantic else text_schema, + accepted=[text_schema, hybrid_schema], ) add_telemetry(redis, enabled=enable_telemetry) @@ -110,18 +142,70 @@ def add( self.redis.execute(["JSON.SET", self._key(user_id, id), "$", dumps(doc)]) return MemoryRecord(id=id, text=text, created_at=created_at, metadata=metadata or {}) - def recall(self, query: str, *, user_id: str, top_k: int = 5) -> list[MemoryRecord]: - """The `top_k` memories closest to `query` ($smart: full-text plus fuzzy matching).""" + def recall( + self, query: str, *, user_id: str, top_k: int = 5, mode: RecallMode = "auto" + ) -> list[MemoryRecord]: + """The `top_k` memories closest to `query`. + + `mode="auto"` uses hybrid search ($rrf of $smart and $ann) when this store is `semantic` + and the index has embeddings, else keyword search ($smart: full-text plus fuzzy matching). + A failed hybrid query falls back to keyword search once. `mode="keyword"` always uses + keyword search, for callers that need COUNT and QUERY to describe the same set. + """ + if mode not in ("auto", "keyword"): + raise ValueError(f"mode must be 'auto' or 'keyword' (got {mode!r})") _check_id(user_id, "user_id") - filter = self._filter(user_id) - has_query = bool(query and query.strip()) - if has_query: - filter["text"] = {"$smart": query} - rows = self._memories(self.index.query(filter, limit=top_k), user_id) - min_score = self.min_score if has_query else 0 - return [ - r for r in (self._record(row, user_id) for row in rows) if (r.score or 0) >= min_score - ] + base = self._filter(user_id) + if not (query and query.strip()): + rows = self._memories(self.index.query(base, limit=top_k), user_id) + return [self._record(row, user_id) for row in rows] + if mode == "auto" and self.semantic and self._hybrid_ready(): + hybrid = { + "$rrf": { + "retrievers": [ + {**base, "text": {"$smart": query}}, + { + "$ann": { + "field": "text", + "query": query, + "filter": self._filter(user_id), + } + }, + ] + } + } + try: + rows = self._memories(self.index.query(hybrid, limit=top_k), user_id) + except UpstashError as error: + self._warn( + "hybrid", "upstash memory hybrid recall failed, using keyword: %s", error + ) + else: + return [self._record(row, user_id, "hybrid") for row in rows] + rows = self._memories( + self.index.query({**base, "text": {"$smart": query}}, limit=top_k), user_id + ) + records = [self._record(row, user_id, "keyword") for row in rows] + return [r for r in records if (r.score or 0) >= self.min_score] + + def _hybrid_ready(self) -> bool: + """Whether the live index can answer `$ann` on `text` (it was created or migrated so).""" + self.index.ensure() + if self.index.field_type("text") == "HYBRID": + return True + self._warn( + "no-embeddings", + "upstash memory: semantic recall is on but index %s has no embeddings yet; using " + "keyword recall until it is migrated (python -m upstash_hermes_memory.migrate)", + self.index.name, + ) + return False + + def _warn(self, what: str, message: str, *args: Any) -> None: + now = time.monotonic() + if now - self._warned_at.get(what, -_WARN_EVERY_S) >= _WARN_EVERY_S: + self._warned_at[what] = now + logger.warning(message, *args) def list(self, *, user_id: str, limit: int = 100) -> list[MemoryRecord]: _check_id(user_id, "user_id") @@ -145,10 +229,11 @@ def forget(self, id: str, *, user_id: str) -> bool: _check_memory_id(id) return bool(self.redis.delete(self._key(user_id, id))) - def _record(self, row: Any, user_id: str) -> MemoryRecord: + def _record(self, row: Any, user_id: str, retrieval: str | None = None) -> MemoryRecord: id = row.key[len(self._key(user_id, "")) :] record = self._from_doc(id, row.data if isinstance(row.data, dict) else {}) record.score = row.score + record.retrieval = retrieval return record @staticmethod diff --git a/python/hermes-memory/src/upstash_hermes_memory/migrate.py b/python/hermes-memory/src/upstash_hermes_memory/migrate.py new file mode 100644 index 0000000..daf0819 --- /dev/null +++ b/python/hermes-memory/src/upstash_hermes_memory/migrate.py @@ -0,0 +1,203 @@ +"""Switch a namespace's shared search index between keyword and hybrid (semantic) recall. + + python -m upstash_hermes_memory.migrate --to hybrid # add embeddings to memory `text` + python -m upstash_hermes_memory.migrate --to keyword # back to the 0.2.x index (rollback) + +Stop every client of the namespace first (the Hermes gateway, the desktop app, cron jobs, other +profiles on the same namespace): the index is dropped and created again, so searches fail in +between, and a client that recreates the index during the gap would race this script. Documents +are never touched; only the index is rebuilt over the same keys, then checked against the counts +it had before. Running it again is safe: an index that is already the target is left alone. + +Credentials come from UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN in the environment, or +from `--env-file` (e.g. ~/.hermes/.env), which is read for those keys and the namespace only. +""" + +import argparse +import os +import sys +import time +from typing import Any + +from ._search import ( + DEFAULT_NAMESPACE, + SHARED_SCHEMA, + SearchIndex, + describe, + dumps, + index_name_for, + matches, + with_hybrid_text, +) + +_KEYS = ("UPSTASH_REDIS_REST_URL", "UPSTASH_REDIS_REST_TOKEN", "UPSTASH_HERMES_NAMESPACE") +_CHATS = {"kind": {"$eq": "chat"}} +_MEMORIES = {"$mustNot": [{"kind": {"$eq": "chat"}}]} + + +class MigrationError(Exception): + pass + + +def _read_env_file(path: str) -> dict[str, str]: + values: dict[str, str] = {} + with open(path) as file: + for line in file: + line = line.strip().removeprefix("export ").strip() + key, sep, value = line.partition("=") + if sep and key.strip() in _KEYS: + value = value.strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in "\"'": + value = value[1:-1] + values[key.strip()] = value + return values + + +def _counts(redis: Any, name: str) -> tuple[int, int]: + """(memories, chats) the index holds; -1 each when the index does not exist.""" + + def count(filter: dict[str, Any]) -> int: + result = redis.execute(["SEARCH.COUNT", name, dumps(filter)]) + return int(getattr(result, "count", result)) + + return count(_MEMORIES), count(_CHATS) + + +def migrate( + redis: Any, + *, + namespace: str, + to: str, + confirm: Any = lambda plan: True, + log: Any = print, + attempts: int = 3, +) -> str: + """Rebuild the shared index of `namespace` as `to` ("hybrid" or "keyword"). + + Returns "unchanged", "created" or "migrated". Raises MigrationError, without dropping anything, + when the live index is not one this package created, and after a failed rebuild with the + command that recovers it. + """ + if to not in ("hybrid", "keyword"): + raise ValueError("to must be 'hybrid' or 'keyword'") + keyword, hybrid = list(SHARED_SCHEMA), with_hybrid_text(SHARED_SCHEMA) + target, other = (hybrid, keyword) if to == "hybrid" else (keyword, hybrid) + name = index_name_for(namespace) + prefixes = [f"{namespace}:memory:", f"{namespace}:chat:"] + index = SearchIndex(redis, name=name, prefix=prefixes, schema=target) + + current = describe(redis, name) + if matches(current, prefixes, target): + log(f"Index {name} is already {to}; nothing to do.") + return "unchanged" + if current is None: + if not confirm(f"Index {name} does not exist. Create it as {to}?"): + raise MigrationError("cancelled") + _create(redis, index, prefixes, target, attempts, log) + log(f"Created index {name} ({to}).") + return "created" + if not matches(current, prefixes, other): + raise MigrationError( + f"Index {name} has a definition this package did not create ({current}); " + "refusing to drop it." + ) + + memories, chats = _counts(redis, name) + came_from = "keyword" if to == "hybrid" else "hybrid" + plan = ( + f"Rebuild index {name} from {came_from} to {to}: {memories} memories and {chats} chats " + f"will be indexed again{' and embedded' if to == 'hybrid' else ''}. Documents are not " + "touched. Every client of this namespace must be stopped. Continue?" + ) + if not confirm(plan): + raise MigrationError("cancelled") + + redis.execute(["SEARCH.DROP", name]) + try: + _create(redis, index, prefixes, target, attempts, log) + except MigrationError as error: + raise MigrationError( + f"{error}. The index is gone and searches fail until it exists again; documents are " + f"intact. Re-run this command (--to {to}), or restore the previous index with " + f"--to {came_from}." + ) from error + + after = _counts(redis, name) + if after != (memories, chats): + raise MigrationError( + f"Index {name} is {to} now, but holds {after[0]} memories and {after[1]} chats " + f"(before: {memories} and {chats}). Was a client still writing? Check, then restart." + ) + log(f"Index {name} is {to}: {memories} memories and {chats} chats indexed.") + return "migrated" + + +def _create( + redis: Any, index: SearchIndex, prefixes: list[str], target: list, attempts: int, log: Any +) -> None: + """CREATE then WAITINDEXING, retried; reconciles with DESCRIBE after an ambiguous failure.""" + last: Exception | None = None + for attempt in range(1, attempts + 1): + try: + redis.execute(index.create_command()) + except Exception as error: # noqa: BLE001 - a timeout may still have created it + last = error + log(f"Create attempt {attempt} failed: {error}") + found = describe(redis, index.name) + if matches(found, prefixes, target): + # 1 once every document is searchable; 0 means the index is missing (dropped again). + if redis.execute(["SEARCH.WAITINDEXING", index.name]) == 1: + return + last = MigrationError(f"index {index.name} disappeared while indexing") + elif found is not None: + raise MigrationError(f"index {index.name} was created with another definition: {found}") + time.sleep(min(2**attempt, 10)) + raise MigrationError(f"could not create index {index.name}: {last}") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="python -m upstash_hermes_memory.migrate", + description="Switch the shared memory/chat index between keyword and hybrid recall.", + ) + parser.add_argument("--to", required=True, choices=["hybrid", "keyword"]) + parser.add_argument("--namespace", help=f"key namespace (default: {DEFAULT_NAMESPACE})") + parser.add_argument("--env-file", help="read the Upstash credentials from this .env file") + parser.add_argument("--yes", action="store_true", help="do not ask for confirmation") + args = parser.parse_args(argv) + + env = {k: os.environ[k] for k in _KEYS if os.environ.get(k)} + if args.env_file: + env.update(_read_env_file(args.env_file)) + if not (env.get("UPSTASH_REDIS_REST_URL") and env.get("UPSTASH_REDIS_REST_TOKEN")): + print("UPSTASH_REDIS_REST_URL and UPSTASH_REDIS_REST_TOKEN are required.", file=sys.stderr) + return 2 + namespace = args.namespace or env.get("UPSTASH_HERMES_NAMESPACE") or DEFAULT_NAMESPACE + + from upstash_redis import Redis + + redis = Redis( + url=env["UPSTASH_REDIS_REST_URL"], + token=env["UPSTASH_REDIS_REST_TOKEN"], + allow_telemetry=False, + ) + + def confirm(plan: str) -> bool: + print(plan) + if args.yes: + return True + if not sys.stdin.isatty(): + print("Not a terminal: pass --yes to confirm.", file=sys.stderr) + return False + return input("Type yes to continue: ").strip().lower() == "yes" + + try: + migrate(redis, namespace=namespace, to=args.to, confirm=confirm) + except MigrationError as error: + print(f"Migration stopped: {error}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/python/hermes-memory/src/upstash_hermes_memory/provider.py b/python/hermes-memory/src/upstash_hermes_memory/provider.py index 3d03f5a..bd4b906 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/provider.py +++ b/python/hermes-memory/src/upstash_hermes_memory/provider.py @@ -20,6 +20,10 @@ works on Upstash's Free plan (one index per database). - UPSTASH_HERMES_TELEMETRY: set to `1` to send telemetry (off by default, as the plugin catalog requires). `UPSTASH_DISABLE_TELEMETRY` still turns it off. +- UPSTASH_HERMES_SEMANTIC: set to `1` to recall memories by meaning as well as by words (hybrid + search with Upstash Redis Search's managed embeddings). Off by default. A new index is created + with embeddings; an existing one keeps keyword recall until it is migrated with + `python -m upstash_hermes_memory.migrate --to hybrid` (Hermes stopped). """ import hashlib @@ -32,6 +36,8 @@ from ._search import DEFAULT_NAMESPACE from ._telemetry import OPT_IN_ENV, OPT_OUT_ENV, is_opt_in + +SEMANTIC_ENV = "UPSTASH_HERMES_SEMANTIC" from .chat_history import ChatHistory from .memory import AgentMemory, MemoryRecord @@ -53,6 +59,10 @@ def get_secret(name: str, default: str | None = None) -> str | None: # Hermes only lets the primary agent write; subagents, cron jobs and flushes read. _WRITING_CONTEXTS = {"", "primary"} _PREFETCH_TOP_K = 5 +# Hybrid search always returns its `LIMIT` (the $ann side has no relevance threshold, and its +# similarity scores do not separate related from unrelated memories well enough for a cutoff), so +# automatic recall injects fewer memories when it is on. The search tool's own limit is unchanged. +_PREFETCH_TOP_K_SEMANTIC = 3 _EXCERPT_CHARS = 300 # Built-in memory tool actions Hermes forwards (a patch arrives as a replace). _MIRRORED_ACTIONS = {"add", "replace", "remove"} @@ -79,7 +89,10 @@ def _mirror_id(target: str, text: str) -> str: } SEARCH_SCHEMA = { "name": "upstash_memory_search", - "description": "Search long-term memory for facts related to a query. Returns ids and text.", + "description": ( + "Search long-term memory for facts related to a query. Returns ids, text and a score; " + "scores rank results within one search only (`retrieval` says how they were found)." + ), "parameters": { "type": "object", "properties": { @@ -302,6 +315,15 @@ def get_config_schema(self) -> list[dict[str, Any]]: "required": False, "env_var": "UPSTASH_HERMES_USER_ID", }, + { + "key": "semantic", + "description": ( + "Recall memories by meaning as well as by words (1 to enable; an existing " + "index needs `python -m upstash_hermes_memory.migrate --to hybrid`)" + ), + "required": False, + "env_var": SEMANTIC_ENV, + }, ] def save_config(self, values: dict[str, Any], hermes_home: str) -> None: @@ -338,8 +360,15 @@ def initialize(self, session_id: str, **kwargs: Any) -> None: # Like the credentials, the namespace is read per Hermes profile: a multiplexed gateway # keeps each profile's .env out of os.environ. Memory and chats share its one index. namespace = get_secret("UPSTASH_HERMES_NAMESPACE", "") or DEFAULT_NAMESPACE - self._memory = AgentMemory(self._redis, namespace=namespace, enable_telemetry=telemetry) - self._chats = ChatHistory(self._redis, namespace=namespace, enable_telemetry=telemetry) + # Both stores get the same flag, so whichever creates the shared index creates it the same + # way. It is read per profile like the namespace, so profiles can differ. + semantic = is_opt_in(get_secret(SEMANTIC_ENV, "")) + self._memory = AgentMemory( + self._redis, namespace=namespace, semantic=semantic, enable_telemetry=telemetry + ) + self._chats = ChatHistory( + self._redis, namespace=namespace, semantic=semantic, enable_telemetry=telemetry + ) self._user_id = user_id self._session_id = self._session_key(session_id) self._write_enabled = (kwargs.get("agent_context") or "") in _WRITING_CONTEXTS @@ -362,7 +391,8 @@ def prefetch(self, query: str, *, session_id: str = "") -> str: if not self._memory or not query.strip(): return "" try: - hits = self._memory.recall(query, user_id=self._user_id, top_k=_PREFETCH_TOP_K) + top_k = _PREFETCH_TOP_K_SEMANTIC if self._memory.semantic else _PREFETCH_TOP_K + hits = self._memory.recall(query, user_id=self._user_id, top_k=top_k) except Exception as error: # noqa: BLE001 - a Redis outage must never break a turn logger.warning("upstash memory recall failed: %s", error) return "" @@ -554,7 +584,8 @@ def _all_matches(self, query: str) -> list[MemoryRecord]: ) if total <= 0: return [] - return self._memory.recall(query, user_id=self._user_id, top_k=total) + # Keyword mode: the COUNT above cannot take $ann, and both must describe the same set. + return self._memory.recall(query, user_id=self._user_id, top_k=total, mode="keyword") def _all_memories(self) -> list[MemoryRecord]: """Every memory of this user, for the substring match that search can't do.""" @@ -611,7 +642,12 @@ def _call(self, tool_name: str, args: dict[str, Any]) -> dict[str, Any]: user_id=self._user_id, top_k=int(args.get("limit") or 5), ) - return {"results": [{"id": h.id, "text": h.text, "score": h.score} for h in hits]} + return { + "results": [ + {"id": h.id, "text": h.text, "score": h.score, "retrieval": h.retrieval} + for h in hits + ] + } if tool_name == "upstash_memory_forget": if not self._write_enabled: raise RuntimeError("memory is read-only in this agent context") diff --git a/python/hermes-memory/tests/test_semantic.py b/python/hermes-memory/tests/test_semantic.py new file mode 100644 index 0000000..99ad338 --- /dev/null +++ b/python/hermes-memory/tests/test_semantic.py @@ -0,0 +1,314 @@ +"""Semantic (hybrid) recall and the index migration, against a scripted fake Redis.""" + +import json +import logging +from dataclasses import dataclass +from typing import Any + +import pytest +from upstash_hermes_memory import AgentMemory, ChatHistory, UpstashMemoryProvider +from upstash_hermes_memory import provider as provider_module +from upstash_hermes_memory._search import SHARED_SCHEMA, shared_index, with_hybrid_text +from upstash_hermes_memory.migrate import MigrationError, migrate +from upstash_redis.errors import UpstashError + +MISMATCH = "ERR Index agentkit already exists with a different configuration" + + +@dataclass +class Row: + key: str + score: float + data: dict[str, Any] + + +def describe_reply(schema, name="agentkit", prefixes=("agentkit:chat:", "agentkit:memory:")): + fields = [[f, t] + (["NOTOKENIZE"] if nt else []) for f, t, nt in schema] + return ["name", name, "type", "JSON", "prefixes", list(prefixes), "language", "english", + "schema", fields] # fmt: skip + + +class FakeRedis: + """Answers search commands from a small script; records every command it gets.""" + + def __init__(self, index_schema=None, rows=(), fail_hybrid=False, count=None): + self.index_schema = index_schema # None: no index + self.rows = list(rows) + self.fail_hybrid = fail_hybrid + self.count = count + self.commands: list[list[Any]] = [] + self._headers: dict[str, str] = {} + + def execute(self, command): + self.commands.append(command) + op = command[0] + if op == "SEARCH.CREATE": + schema = _schema_of(command) + if self.index_schema is not None and self.index_schema != schema: + raise UpstashError(MISMATCH) + self.index_schema = schema + return 1 + if op == "SEARCH.DESCRIBE": + return describe_reply(self.index_schema) if self.index_schema else None + if op == "SEARCH.DROP": + dropped, self.index_schema = self.index_schema is not None, None + return int(dropped) + if op == "SEARCH.WAITINDEXING": + return 1 if self.index_schema else 0 + if op == "SEARCH.QUERY": + if self.index_schema is None: + return [] + filter = json.loads(command[2]) + if "$rrf" in filter and self.fail_hybrid: + raise UpstashError("ERR dot metric only accepts unit-length vectors") + return list(self.rows) + if op == "SEARCH.COUNT": + if self.index_schema is None: + return -1 + if "$rrf" in json.loads(command[2]) or "$ann" in json.loads(command[2]): + raise UpstashError("ERR $ann cannot be used with SEARCH.COUNT") + return self.count if self.count is not None else len(self.rows) + return 1 + + def delete(self, key): + self.commands.append(["DEL", key]) + return 1 + + def expire(self, key, seconds): + return True + + def queries(self): + return [json.loads(c[2]) for c in self.commands if c[0] == "SEARCH.QUERY"] + + +def _schema_of(command): + i = command.index("SCHEMA") + 1 + out, rest = [], command[i:] + while rest: + field, type_ = rest[0], rest[1] + no_tokenize = len(rest) > 2 and rest[2] == "NOTOKENIZE" + out.append((field, type_, no_tokenize)) + rest = rest[3:] if no_tokenize else rest[2:] + return out + + +HYBRID = with_hybrid_text(SHARED_SCHEMA) +USER = {"userId": {"$eq": "u1"}, "$mustNot": [{"kind": {"$eq": "chat"}}]} + + +def memory_row(id, text, score=1.0): + return Row(f"agentkit:memory:u1:{id}", score, {"text": text, "userId": "u1", "createdAt": 1}) + + +# -- index creation ------------------------------------------------------------------------------ + + +def test_semantic_creates_text_as_the_only_hybrid_field(): + command = shared_index(FakeRedis(), "agentkit", semantic=True).create_command() + assert " ".join(command).endswith( + "SCHEMA kind TEXT NOTOKENIZE userId TEXT NOTOKENIZE text HYBRID sessionId TEXT NOTOKENIZE " + "userMessages TEXT modelMessages TEXT" + ) + + +def test_memory_and_chats_create_the_index_the_same_way(): + redis = FakeRedis() + memory = AgentMemory(redis, semantic=True) + chats = ChatHistory(redis, semantic=True) + assert memory.index.create_command() == chats.index.create_command() + + +@pytest.mark.parametrize("semantic, existing", [(True, SHARED_SCHEMA), (False, HYBRID)]) +def test_an_existing_index_of_the_other_kind_is_used_never_dropped(semantic, existing): + redis = FakeRedis(index_schema=list(existing)) + memory = AgentMemory(redis, semantic=semantic) + memory.index.ensure() + assert memory.index.actual_schema == list(existing) + assert not [c for c in redis.commands if c[0] == "SEARCH.DROP"] + + +def test_an_unknown_index_definition_still_fails(): + redis = FakeRedis(index_schema=SHARED_SCHEMA + [("extra", "TEXT", False)]) + with pytest.raises(UpstashError): + AgentMemory(redis, semantic=True).index.ensure() + assert not [c for c in redis.commands if c[0] == "SEARCH.DROP"] + + +# -- recall -------------------------------------------------------------------------------------- + + +def test_hybrid_recall_fuses_keyword_and_meaning_for_this_user_only(): + redis = FakeRedis(index_schema=list(HYBRID), rows=[memory_row("1", "Weeks start Monday", 0.03)]) + memory = AgentMemory(redis, semantic=True, min_score=5) + hits = memory.recall("what day does the week begin", user_id="u1", top_k=3) + assert [(h.text, h.retrieval) for h in hits] == [("Weeks start Monday", "hybrid")] + (query,) = redis.queries() + keyword, ann = query["$rrf"]["retrievers"] + assert keyword == {**USER, "text": {"$smart": "what day does the week begin"}} + assert ann == { + "$ann": {"field": "text", "query": "what day does the week begin", "filter": USER} + } + + +def test_semantic_on_a_keyword_index_recalls_by_keyword_and_says_so(caplog): + redis = FakeRedis(index_schema=list(SHARED_SCHEMA), rows=[memory_row("1", "x")]) + memory = AgentMemory(redis, semantic=True) + with caplog.at_level(logging.WARNING): + memory.recall("a", user_id="u1") + memory.recall("b", user_id="u1") + assert all("$rrf" not in q for q in redis.queries()) + assert [r.message for r in caplog.records].count(caplog.records[0].message) == 1 + assert "migrate" in caplog.records[0].message + + +def test_a_failed_hybrid_query_falls_back_to_keyword_once_with_min_score(): + rows = [memory_row("1", "strong", 3.0), memory_row("2", "weak", 0.2)] + redis = FakeRedis(index_schema=list(HYBRID), rows=rows, fail_hybrid=True) + memory = AgentMemory(redis, semantic=True, min_score=1) + hits = memory.recall("billing for big accounts", user_id="u1") + assert [(h.text, h.retrieval) for h in hits] == [("strong", "keyword")] + assert ["$rrf" in q for q in redis.queries()] == [True, False] + assert redis.queries()[1] == {**USER, "text": {"$smart": "billing for big accounts"}} + + +def test_keyword_mode_never_uses_a_retriever(): + redis = FakeRedis(index_schema=list(HYBRID), rows=[memory_row("1", "x")]) + AgentMemory(redis, semantic=True).recall("x", user_id="u1", mode="keyword") + assert all("$rrf" not in q for q in redis.queries()) + + +def test_an_unknown_mode_is_refused(): + with pytest.raises(ValueError): + AgentMemory(FakeRedis(), semantic=True).recall("x", user_id="u1", mode="vector") + + +def test_an_empty_hybrid_result_checks_the_index_without_counting_a_retriever(): + redis = FakeRedis(index_schema=list(HYBRID), rows=[]) + assert AgentMemory(redis, semantic=True).recall("nothing", user_id="u1") == [] + counts = [c[2] for c in redis.commands if c[0] == "SEARCH.COUNT"] + assert counts == ["{}"] + + +def test_semantic_off_keeps_the_0_2_commands(): + redis = FakeRedis(index_schema=list(SHARED_SCHEMA), rows=[memory_row("1", "x", 2.0)]) + hits = AgentMemory(redis).recall("x", user_id="u1") + assert redis.queries() == [{**USER, "text": {"$smart": "x"}}] + assert hits[0].retrieval == "keyword" + + +# -- provider ------------------------------------------------------------------------------------ + + +@pytest.fixture +def semantic_provider(monkeypatch): + env = {"UPSTASH_HERMES_SEMANTIC": "1", "UPSTASH_HERMES_USER_ID": "u1"} + monkeypatch.setattr(provider_module, "get_secret", lambda n, d=None: env.get(n, d)) + redis = FakeRedis( + index_schema=list(HYBRID), rows=[memory_row(str(i), f"m{i}") for i in range(5)] + ) + provider = UpstashMemoryProvider(redis) + provider.initialize("s1", platform="cli") + return provider, redis + + +def test_the_env_flag_turns_semantic_on_for_both_stores(semantic_provider): + provider, _ = semantic_provider + assert provider._memory.semantic + assert provider._chats.index.schema == provider._memory.index.schema == HYBRID + + +def test_prefetch_asks_for_three_memories_in_semantic_mode(semantic_provider): + provider, redis = semantic_provider + provider.prefetch("what do I prefer") + (limit,) = [c[c.index("LIMIT") + 1] for c in redis.commands if c[0] == "SEARCH.QUERY"] + assert limit == "3" + + +def test_the_search_tool_says_how_results_were_found(semantic_provider): + provider, _ = semantic_provider + result = json.loads(provider.handle_tool_call("upstash_memory_search", {"query": "m"})) + assert {r["retrieval"] for r in result["results"]} == {"hybrid"} + + +def test_mirror_deletion_matches_by_keyword_only(semantic_provider): + provider, redis = semantic_provider + provider._all_matches("old entry") + assert all("$rrf" not in q for q in redis.queries()) + + +# -- migration ----------------------------------------------------------------------------------- + + +def test_migration_rebuilds_a_keyword_index_and_checks_the_counts(): + redis = FakeRedis(index_schema=list(SHARED_SCHEMA), count=7) + assert migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) == "migrated" + ops = [c[0] for c in redis.commands] + assert ops.index("SEARCH.DROP") < ops.index("SEARCH.CREATE") < ops.index("SEARCH.WAITINDEXING") + assert redis.index_schema == list(HYBRID) + + +def test_migration_back_to_keyword_restores_the_0_2_index(): + redis = FakeRedis(index_schema=list(HYBRID), count=3) + migrate(redis, namespace="agentkit", to="keyword", log=lambda m: None) + assert redis.index_schema == list(SHARED_SCHEMA) + + +def test_migration_to_the_current_kind_does_nothing(): + redis = FakeRedis(index_schema=list(HYBRID)) + assert migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) == "unchanged" + assert "SEARCH.DROP" not in [c[0] for c in redis.commands] + + +def test_migration_refuses_an_index_it_did_not_create(): + redis = FakeRedis(index_schema=SHARED_SCHEMA + [("extra", "TEXT", False)]) + with pytest.raises(MigrationError, match="refusing"): + migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) + assert "SEARCH.DROP" not in [c[0] for c in redis.commands] + + +def test_a_declined_migration_drops_nothing(): + redis = FakeRedis(index_schema=list(SHARED_SCHEMA)) + with pytest.raises(MigrationError, match="cancelled"): + migrate(redis, namespace="agentkit", to="hybrid", confirm=lambda p: False, log=print) + assert "SEARCH.DROP" not in [c[0] for c in redis.commands] + + +def test_a_failed_rebuild_says_how_to_recover(monkeypatch): + monkeypatch.setattr("upstash_hermes_memory.migrate.time.sleep", lambda s: None) + + class NoCreate(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.CREATE": + self.commands.append(command) + raise TimeoutError("read timed out") + return super().execute(command) + + redis = NoCreate(index_schema=list(SHARED_SCHEMA)) + with pytest.raises(MigrationError, match="--to keyword"): + migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) + assert [c[0] for c in redis.commands].count("SEARCH.CREATE") == 3 + + +def test_a_timed_out_create_that_succeeded_is_not_repeated(): + class SlowCreate(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.CREATE": + super().execute(command) + raise TimeoutError("read timed out") + return super().execute(command) + + redis = SlowCreate(index_schema=list(SHARED_SCHEMA), count=2) + assert migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) == "migrated" + assert [c[0] for c in redis.commands].count("SEARCH.CREATE") == 1 + + +def test_a_count_change_during_the_rebuild_is_reported(): + class Writer(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.DROP": + self.count = 8 # a client kept writing + return super().execute(command) + + redis = Writer(index_schema=list(SHARED_SCHEMA), count=7) + with pytest.raises(MigrationError, match="still writing"): + migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) diff --git a/python/hermes-memory/tests/test_semantic_redis.py b/python/hermes-memory/tests/test_semantic_redis.py new file mode 100644 index 0000000..93fa8f9 --- /dev/null +++ b/python/hermes-memory/tests/test_semantic_redis.py @@ -0,0 +1,117 @@ +"""Semantic recall and the migration against a real Upstash Redis (skipped without secrets). + +Hybrid queries can fail intermittently on the server ("dot metric only accepts unit-length +vectors"); recall then falls back to keyword search, so assertions about meaning retry a few times +and only count results that were actually found by hybrid search. +""" + +import pytest +from upstash_hermes_memory import AgentMemory, ChatHistory +from upstash_hermes_memory._search import ( + SHARED_SCHEMA, + SearchIndex, + describe, + matches, + with_hybrid_text, +) +from upstash_hermes_memory.migrate import migrate + +FACTS = { + "week": "Weeks start Monday for him.", + "coffee": "His favorite coffee is a flat white from the place near the office.", + "drafts": "Customer-facing drafts on Slack go out as draft cards, never plain text.", + "invoices": "Invoices for enterprise customers are reviewed on the first business day.", + "host": "Hermes runs only on the ops Box since October; the Mac runs the desktop app.", + "dashes": "He writes without em dashes and leads with the answer.", + "churn": "The churn analysis runs monthly and he reviews it before publication.", + "keys": "1Password is his default credential store.", + "units": "Sizes are written in decimal MB and GB.", + "board": "Side ideas go to the hermes-queue Kanban board.", + "shell": "Shell snippets for him are written in fish.", + "mail": "Never delete or trash email; ask before sending.", +} + + +def hybrid_recall(memory, query, user_id="u1", top_k=3, tries=4): + for _ in range(tries): + hits = memory.recall(query, user_id=user_id, top_k=top_k) + if hits and hits[0].retrieval == "hybrid": + return hits + pytest.skip("hybrid search kept failing on the server; recall fell back to keyword") + + +@pytest.fixture +def semantic(redis, prefix): + memory = AgentMemory(redis, namespace=prefix, semantic=True, enable_telemetry=False) + memory.index.ensure() + yield memory + memory.index.drop() + + +def test_a_new_semantic_index_embeds_memory_text_only(semantic, prefix): + found = describe(semantic.redis, semantic.index.name) + prefixes = [f"{prefix}:memory:", f"{prefix}:chat:"] + assert matches(found, prefixes, with_hybrid_text(SHARED_SCHEMA)) + assert semantic.index.field_type("text") == "HYBRID" + assert semantic.index.field_type("userMessages") == "TEXT" + + +def ids(hits): + return [h.id for h in hits] + + +def test_recall_finds_a_memory_by_meaning_within_what_is_injected(semantic): + # Hermes injects the top 3 in semantic mode. Equal-weight $rrf can rank a keyword-only match + # (here "first business day" for "day") level with the best meaning match, so the contract + # tested is "among the injected 3", not "first". + for id, text in FACTS.items(): + semantic.add(text, user_id="u1", id=id) + semantic.index.wait_indexing() + assert "week" in ids(hybrid_recall(semantic, "what day does the week begin")) + assert "coffee" in ids(hybrid_recall(semantic, "what hot drink does he enjoy")) + assert "drafts" in ids(hybrid_recall(semantic, "how should a message to a client be sent")) + + +def test_hybrid_recall_never_crosses_users_or_returns_chats(semantic, redis, prefix): + semantic.add("Hermes runs on the ops Box.", user_id="u1", id="mine") + semantic.add("Hermes runs on a Raspberry Pi.", user_id="u2", id="theirs") + chats = ChatHistory(redis, namespace=prefix, semantic=True, enable_telemetry=False) + chats.save_chat("s1", [{"role": "user", "content": "where does hermes run"}], user_id="u1") + semantic.index.wait_indexing() + hits = hybrid_recall(semantic, "where is the assistant hosted", top_k=5) + assert [h.id for h in hits] == ["mine"] + + +def test_an_empty_store_recalls_nothing_without_errors(semantic): + assert semantic.recall("anything at all", user_id="nobody") == [] + + +def test_migration_round_trip_keeps_memories_and_chats(redis, prefix): + keyword = AgentMemory(redis, namespace=prefix, enable_telemetry=False) + chats = ChatHistory(redis, namespace=prefix, enable_telemetry=False) + keyword.index.ensure() + try: + for id, text in FACTS.items(): + keyword.add(text, user_id="u1", id=id) + chats.save_chat("s1", [{"role": "user", "content": "flat white plans"}], user_id="u1") + keyword.index.wait_indexing() + quiet = lambda message: None + + assert migrate(redis, namespace=prefix, to="hybrid", log=quiet) == "migrated" + semantic = AgentMemory(redis, namespace=prefix, semantic=True, enable_telemetry=False) + assert "coffee" in ids(hybrid_recall(semantic, "what hot drink does he enjoy")) + assert [h.session_id for h in chats.search_chats("flat white", user_id="u1")] == ["s1"] + assert migrate(redis, namespace=prefix, to="hybrid", log=quiet) == "unchanged" + + assert migrate(redis, namespace=prefix, to="keyword", log=quiet) == "migrated" + # What 0.2.2 does on first use: a plain EXISTSOK create of the TEXT index must succeed. + legacy = SearchIndex( + redis, + name=keyword.index.name, + prefix=[f"{prefix}:memory:", f"{prefix}:chat:"], + schema=SHARED_SCHEMA, + ) + redis.execute(legacy.create_command()) + assert len(AgentMemory(redis, namespace=prefix).list(user_id="u1")) == len(FACTS) + finally: + redis.execute(["SEARCH.DROP", keyword.index.name]) diff --git a/uv.lock b/uv.lock index ed7d5fd..08f3c48 100644 --- a/uv.lock +++ b/uv.lock @@ -178,7 +178,7 @@ wheels = [ [[package]] name = "upstash-hermes-memory" -version = "0.2.2" +version = "0.3.0" source = { editable = "python/hermes-memory" } dependencies = [ { name = "upstash-redis" }, From 49f5669bc20106bcf1403df38355298961493077 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sertu=C4=9F=20Kaya?= Date: Mon, 9 Nov 2026 13:24:41 +0000 Subject: [PATCH 2/6] fix(hermes-memory): resumable migration, transport fallback, alias and WAITINDEXING checks --- python/hermes-memory/README.md | 19 +- .../src/upstash_hermes_memory/_search.py | 27 +- .../src/upstash_hermes_memory/memory.py | 20 +- .../src/upstash_hermes_memory/migrate.py | 211 +++++++---- python/hermes-memory/tests/test_semantic.py | 342 ++++++++++++++---- .../tests/test_semantic_redis.py | 12 +- 6 files changed, 459 insertions(+), 172 deletions(-) diff --git a/python/hermes-memory/README.md b/python/hermes-memory/README.md index 3e7cefc..774d124 100644 --- a/python/hermes-memory/README.md +++ b/python/hermes-memory/README.md @@ -56,18 +56,21 @@ semantic `$ann` query using `$rrf` desktop app, cron jobs, other profiles on it), then: ```bash - python -m upstash_hermes_memory.migrate --to hybrid --env-file ~/.hermes/.env + # UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN set in the environment (the script reads no files) + python -m upstash_hermes_memory.migrate --to hybrid ``` - For a directory (catalog) install, run it from the plugin's `src` directory with the Python - Hermes runs in. The index is dropped and created again over the same keys, which are not - touched, and the script checks that it holds as many memories and chats as before. + For a directory (catalog) install, run it from the plugin's `src` directory with a Python that + has `upstash-redis`. The script waits for pending indexing, saves a baseline (counts and a + sample of memory keys) at `:migrate:baseline`, drops and recreates the index over + the same keys (which it never touches), and checks the result against the baseline. If it is + interrupted, running the same command again resumes and checks. - **Rollback:** `--to keyword` restores the 0.2.x index; do that before installing 0.2.x again, which fails on a hybrid index. Turning the variable off is not enough: an existing hybrid index keeps embedding new memories, it is only no longer queried by meaning. - Automatic recall injects up to **3** memories instead of 5: the semantic side has no relevance - threshold and always returns its limit, and its similarity scores did not separate related from - unrelated memories well enough in testing for a cutoff. `upstash_memory_search` keeps its limit. + threshold and returns up to its limit even for unrelated text, and in a small-store test its + similarity scores did not separate related from unrelated memories well enough for a cutoff. `upstash_memory_search` keeps its limit. - Results carry `retrieval: "hybrid"` or `"keyword"`. Hybrid scores are rank-fusion values that only order one search's results; `min_score` applies to keyword results only. - If a hybrid query fails, recall falls back to keyword search for that call (logged at most @@ -75,8 +78,8 @@ semantic `$ann` query using `$rrf` - Chat history stays keyword-only: a `HYBRID` field gets one embedding for its whole value, and a chat document holds a whole session. - Every memory in the index is embedded, whichever user or profile wrote it, and each recall - sends the message to Upstash Redis Search to be embedded. A hybrid query adds about 150 to - 300 ms to recall (keyword recall takes a few ms). + sends the message to Upstash Redis Search to be embedded. In a small-store test a hybrid query + took about 150 to 300 ms, against a few ms for keyword recall. ## Telemetry diff --git a/python/hermes-memory/src/upstash_hermes_memory/_search.py b/python/hermes-memory/src/upstash_hermes_memory/_search.py index cbc693d..cdd813a 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/_search.py +++ b/python/hermes-memory/src/upstash_hermes_memory/_search.py @@ -74,7 +74,9 @@ def _is_config_mismatch(error: Exception) -> bool: def describe(redis: Any, name: str) -> dict[str, Any] | None: - """SEARCH.DESCRIBE as {"type", "prefixes", "language", "schema": [Field...]}, or None if missing. + """SEARCH.DESCRIBE as {"name", "type", "prefixes", "language", "schema"}, or None if missing. + + `name` is the index DESCRIBE resolved, which differs from the argument for an alias. Accepts the client's parsed `IndexDescription` as well as the raw reply, so it does not depend on the client knowing every field type (upstash-redis 1.8 has no HYBRID enum member). @@ -88,6 +90,7 @@ def describe(redis: Any, name: str) -> dict[str, Any] | None: (f[0], str(f[1]).upper(), "NOTOKENIZE" in f[2:], "NOSTEM" in f[2:], "FROM" in f[2:]) for f in pairs.get("schema") or [] ] + found_name = pairs.get("name") data_type, prefixes, language = ( pairs.get("type"), pairs.get("prefixes"), @@ -104,8 +107,10 @@ def describe(redis: Any, name: str) -> dict[str, Any] | None: ) for n, i in raw.schema.items() ] + found_name = raw.name data_type, prefixes, language = raw.data_type, raw.prefixes, raw.language return { + "name": found_name, "type": str(getattr(data_type, "value", data_type) or "").upper(), "prefixes": sorted(prefixes or []), "language": str(getattr(language, "value", language) or "").lower(), @@ -115,13 +120,18 @@ def describe(redis: Any, name: str) -> dict[str, Any] | None: def matches( - description: dict[str, Any] | None, prefixes: Sequence[str], schema: Sequence[Field] + description: dict[str, Any] | None, + prefixes: Sequence[str], + schema: Sequence[Field], + name: str | None = None, ) -> bool: - """Whether a `describe()` result is exactly the index this package would create.""" + """Whether a `describe()` result is exactly the index this package would create (and, with + `name`, that index itself rather than an alias pointing at another one).""" if description is None: return False return ( - description["type"] == "JSON" + (name is None or description["name"] == name) + and description["type"] == "JSON" and description["prefixes"] == sorted(prefixes) and description["language"] in ("", "english") and description["schema"] == sorted((f, t, nt, False, False) for f, t, nt in schema) @@ -180,10 +190,15 @@ def ensure(self) -> None: if not (self.accepted and _is_config_mismatch(error)): raise found = describe(self.redis, self.name) - actual = next((s for s in self.accepted if matches(found, self.prefixes, s)), None) + actual = next( + (s for s in self.accepted if matches(found, self.prefixes, s, self.name)), None + ) if actual is None: raise - self.redis.execute(["SEARCH.WAITINDEXING", self.name]) + # 1 once pending updates are searchable; 0 means the index is gone (dropped right + # after the create): stay not-ready, so the next call creates it again. + if self.redis.execute(["SEARCH.WAITINDEXING", self.name]) != 1: + raise RuntimeError(f"search index {self.name} disappeared while it was created") self.actual_schema = list(actual) self._ready = True diff --git a/python/hermes-memory/src/upstash_hermes_memory/memory.py b/python/hermes-memory/src/upstash_hermes_memory/memory.py index 128671e..679d217 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/memory.py +++ b/python/hermes-memory/src/upstash_hermes_memory/memory.py @@ -1,7 +1,9 @@ """Python port of `AgentMemory` from `@upstash/agentkit-sdk` (packages/sdk/src/memory.ts). -Same keys, same documents, same index: a memory saved by the TS SDK is recalled here and the other -way round, as long as both sides use the same namespace (default `agentkit`). +Same keys and documents as the TS SDK: a memory saved by the TS SDK is recalled here and the other +way round, as long as both sides use the same namespace (default `agentkit`). The search index is +not shared: Python indexes memories and chats together in one index (`agentkit`), the TS SDK +creates its own (`agentkit_memory`), and both index the same keys. key :memory:: value JSON {"text", "userId", "createdAt" (epoch ms), ...metadata}, exactly what the TS SDK writes @@ -19,6 +21,7 @@ from dataclasses import dataclass, field from typing import Any, Literal +import httpx from upstash_redis.errors import UpstashError from ._search import ( @@ -40,6 +43,10 @@ RecallMode = Literal["auto", "keyword"] +# What a failed hybrid query can raise: a server error (e.g. the intermittent "dot metric only +# accepts unit-length vectors") or a transport failure. Anything else is a bug and propagates. +_FALLBACK_ERRORS = (UpstashError, httpx.HTTPError) + # The document's own fields. Metadata is spread next to them, so it must not replace them: a # `userId` in metadata would move the memory into another user's searches. RESERVED_FIELDS = frozenset({"kind", "text", "userId", "createdAt"}) @@ -176,7 +183,7 @@ def recall( } try: rows = self._memories(self.index.query(hybrid, limit=top_k), user_id) - except UpstashError as error: + except _FALLBACK_ERRORS as error: self._warn( "hybrid", "upstash memory hybrid recall failed, using keyword: %s", error ) @@ -185,6 +192,13 @@ def recall( rows = self._memories( self.index.query({**base, "text": {"$smart": query}}, limit=top_k), user_id ) + if not self.semantic and self.index.field_type("text") == "HYBRID": + self._warn( + "embeddings-unused", + "upstash memory: index %s has embeddings but semantic recall is off; new memories " + "are still embedded (migrate --to keyword to stop that)", + self.index.name, + ) records = [self._record(row, user_id, "keyword") for row in rows] return [r for r in records if (r.score or 0) >= self.min_score] diff --git a/python/hermes-memory/src/upstash_hermes_memory/migrate.py b/python/hermes-memory/src/upstash_hermes_memory/migrate.py index daf0819..3c3ddc4 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/migrate.py +++ b/python/hermes-memory/src/upstash_hermes_memory/migrate.py @@ -6,14 +6,19 @@ Stop every client of the namespace first (the Hermes gateway, the desktop app, cron jobs, other profiles on the same namespace): the index is dropped and created again, so searches fail in between, and a client that recreates the index during the gap would race this script. Documents -are never touched; only the index is rebuilt over the same keys, then checked against the counts -it had before. Running it again is safe: an index that is already the target is left alone. +are never touched; only the index is rebuilt over the same keys. -Credentials come from UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN in the environment, or -from `--env-file` (e.g. ~/.hermes/.env), which is read for those keys and the namespace only. +Before dropping anything the script waits for pending indexing, then saves what the index holds +(memory and chat counts and a sample of memory keys) at `:migrate:baseline`. After the +rebuild it checks the new index against that baseline and deletes it. If a run is interrupted, +running the same command again resumes: it finishes the rebuild and checks the saved baseline. + +Credentials come from UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN in the environment (and the +namespace from UPSTASH_HERMES_NAMESPACE or --namespace); the script reads no files. """ import argparse +import json import os import sys import time @@ -30,29 +35,15 @@ with_hybrid_text, ) -_KEYS = ("UPSTASH_REDIS_REST_URL", "UPSTASH_REDIS_REST_TOKEN", "UPSTASH_HERMES_NAMESPACE") _CHATS = {"kind": {"$eq": "chat"}} _MEMORIES = {"$mustNot": [{"kind": {"$eq": "chat"}}]} +_SAMPLE = 200 # memory keys checked one by one after the rebuild class MigrationError(Exception): pass -def _read_env_file(path: str) -> dict[str, str]: - values: dict[str, str] = {} - with open(path) as file: - for line in file: - line = line.strip().removeprefix("export ").strip() - key, sep, value = line.partition("=") - if sep and key.strip() in _KEYS: - value = value.strip() - if len(value) >= 2 and value[0] == value[-1] and value[0] in "\"'": - value = value[1:-1] - values[key.strip()] = value - return values - - def _counts(redis: Any, name: str) -> tuple[int, int]: """(memories, chats) the index holds; -1 each when the index does not exist.""" @@ -63,6 +54,28 @@ def count(filter: dict[str, Any]) -> int: return count(_MEMORIES), count(_CHATS) +def _memory_keys(redis: Any, name: str, limit: int) -> list[str]: + rows = redis.execute(["SEARCH.QUERY", name, dumps(_MEMORIES), "LIMIT", str(limit)]) or [] + return sorted(getattr(row, "key", None) or row[0] for row in rows) + + +def _state(redis: Any, name: str, prefixes: list[str], schemas: dict[str, list]) -> str: + """ "absent", the matching schema's name, "other", or "unknown" (DESCRIBE failed).""" + try: + found = describe(redis, name) + except Exception: # noqa: BLE001 - reported, never acted on + return "unknown" + if found is None: + return "absent" + return next((k for k, s in schemas.items() if matches(found, prefixes, s, name)), "other") + + +def _wait(redis: Any, name: str) -> None: + # 1 once every pending update is searchable; 0 means the index does not exist. + if redis.execute(["SEARCH.WAITINDEXING", name]) != 1: + raise MigrationError(f"index {name} was not found while waiting for indexing") + + def migrate( redis: Any, *, @@ -74,68 +87,119 @@ def migrate( ) -> str: """Rebuild the shared index of `namespace` as `to` ("hybrid" or "keyword"). - Returns "unchanged", "created" or "migrated". Raises MigrationError, without dropping anything, - when the live index is not one this package created, and after a failed rebuild with the - command that recovers it. + Returns "unchanged", "created", "migrated" or "resumed". Raises MigrationError, without + dropping anything, when the live index is not one this package created (or is an alias), and + after a failed step, saying what state the index is in. """ if to not in ("hybrid", "keyword"): raise ValueError("to must be 'hybrid' or 'keyword'") - keyword, hybrid = list(SHARED_SCHEMA), with_hybrid_text(SHARED_SCHEMA) - target, other = (hybrid, keyword) if to == "hybrid" else (keyword, hybrid) + came_from = "keyword" if to == "hybrid" else "hybrid" + schemas = {"keyword": list(SHARED_SCHEMA), "hybrid": with_hybrid_text(SHARED_SCHEMA)} name = index_name_for(namespace) prefixes = [f"{namespace}:memory:", f"{namespace}:chat:"] - index = SearchIndex(redis, name=name, prefix=prefixes, schema=target) + index = SearchIndex(redis, name=name, prefix=prefixes, schema=schemas[to]) + baseline_key = f"{namespace}:migrate:baseline" + raw = redis.execute(["GET", baseline_key]) + baseline = json.loads(raw) if raw else None + if baseline and baseline.get("to") != to: + raise MigrationError( + f"an interrupted migration to {baseline.get('to')} is pending ({baseline_key}); " + f"finish it with --to {baseline.get('to')} first" + ) + + state = _state(redis, name, prefixes, schemas) + if state == "unknown": + raise MigrationError(f"index {name} could not be read; nothing changed") + if state == "other": + raise MigrationError( + f"index {name} is not one this package created (or is an alias); refusing to drop it" + ) - current = describe(redis, name) - if matches(current, prefixes, target): - log(f"Index {name} is already {to}; nothing to do.") + if state == to: + _wait(redis, name) + if baseline: # a previous run rebuilt the index but did not finish checking it + _verify(redis, name, baseline, to) + redis.execute(["DEL", baseline_key]) + log(f"Index {name} is {to}; the interrupted migration is checked and complete.") + return "resumed" + memories, chats = _counts(redis, name) + log(f"Index {name} is already {to} ({memories} memories, {chats} chats); nothing to do.") return "unchanged" - if current is None: + + if state == "absent" and not baseline: if not confirm(f"Index {name} does not exist. Create it as {to}?"): raise MigrationError("cancelled") - _create(redis, index, prefixes, target, attempts, log) + _create(redis, index, prefixes, schemas[to], attempts, log) log(f"Created index {name} ({to}).") return "created" - if not matches(current, prefixes, other): - raise MigrationError( - f"Index {name} has a definition this package did not create ({current}); " - "refusing to drop it." - ) - memories, chats = _counts(redis, name) - came_from = "keyword" if to == "hybrid" else "hybrid" - plan = ( - f"Rebuild index {name} from {came_from} to {to}: {memories} memories and {chats} chats " - f"will be indexed again{' and embedded' if to == 'hybrid' else ''}. Documents are not " - "touched. Every client of this namespace must be stopped. Continue?" - ) - if not confirm(plan): - raise MigrationError("cancelled") + if state == came_from: + _wait(redis, name) # pending updates first, or the baseline undercounts + memories, chats = _counts(redis, name) + plan = ( + f"Rebuild index {name} from {came_from} to {to}: {memories} memories and {chats} " + f"chats will be indexed again{' and embedded' if to == 'hybrid' else ''}. Documents " + "are not touched. Every client of this namespace must be stopped. Continue?" + ) + if not confirm(plan): + raise MigrationError("cancelled") + baseline = { + "to": to, + "memories": memories, + "chats": chats, + "sample": _memory_keys(redis, name, _SAMPLE), + "at": int(time.time()), + } + redis.execute(["SET", baseline_key, dumps(baseline)]) + try: + redis.execute(["SEARCH.DROP", name]) + except Exception as error: # noqa: BLE001 - reconciled below + log(f"Drop failed: {error}") + state = _state(redis, name, prefixes, schemas) + if state == came_from: + redis.execute(["DEL", baseline_key]) + raise MigrationError(f"index {name} could not be dropped; nothing changed") + if state != "absent": + raise MigrationError(f"index {name} is {state} after the drop; re-run to resume") + # Absent with a baseline: just dropped, or an interrupted run stopped between drop and create. + assert baseline is not None - redis.execute(["SEARCH.DROP", name]) try: - _create(redis, index, prefixes, target, attempts, log) + _create(redis, index, prefixes, schemas[to], attempts, log) except MigrationError as error: + now = _state(redis, name, prefixes, schemas) raise MigrationError( - f"{error}. The index is gone and searches fail until it exists again; documents are " - f"intact. Re-run this command (--to {to}), or restore the previous index with " - f"--to {came_from}." + f"{error}. The index is {now}; documents are intact. Re-run this command (--to {to}) " + f"to resume, or restore the previous index with --to {came_from}." ) from error + _verify(redis, name, baseline, to) + redis.execute(["DEL", baseline_key]) + log(f"Index {name} is {to}: {baseline['memories']} memories and {baseline['chats']} chats.") + return "migrated" + +def _verify(redis: Any, name: str, baseline: dict[str, Any], to: str) -> None: after = _counts(redis, name) - if after != (memories, chats): + if after != (baseline["memories"], baseline["chats"]): raise MigrationError( - f"Index {name} is {to} now, but holds {after[0]} memories and {after[1]} chats " - f"(before: {memories} and {chats}). Was a client still writing? Check, then restart." + f"index {name} is {to} now but holds {after[0]} memories and {after[1]} chats " + f"(before: {baseline['memories']} and {baseline['chats']}). Was a client still " + "writing? Check, then re-run to verify again." ) - log(f"Index {name} is {to}: {memories} memories and {chats} chats indexed.") - return "migrated" + sample = baseline.get("sample") or [] + if sample: + found = set(_memory_keys(redis, name, max(after[0], len(sample)))) + missing = [k for k in sample if k not in found] + if missing: + raise MigrationError( + f"index {name} lost {len(missing)} sampled memories, e.g. {missing[0]}" + ) def _create( redis: Any, index: SearchIndex, prefixes: list[str], target: list, attempts: int, log: Any ) -> None: - """CREATE then WAITINDEXING, retried; reconciles with DESCRIBE after an ambiguous failure.""" + """CREATE then WAITINDEXING, retried; reconciles with DESCRIBE after any failed step.""" last: Exception | None = None for attempt in range(1, attempts + 1): try: @@ -143,14 +207,18 @@ def _create( except Exception as error: # noqa: BLE001 - a timeout may still have created it last = error log(f"Create attempt {attempt} failed: {error}") - found = describe(redis, index.name) - if matches(found, prefixes, target): - # 1 once every document is searchable; 0 means the index is missing (dropped again). - if redis.execute(["SEARCH.WAITINDEXING", index.name]) == 1: + try: + found = describe(redis, index.name) + if matches(found, prefixes, target, index.name): + _wait(redis, index.name) return - last = MigrationError(f"index {index.name} disappeared while indexing") - elif found is not None: - raise MigrationError(f"index {index.name} was created with another definition: {found}") + if found is not None: + raise MigrationError(f"index {index.name} exists with another definition: {found}") + except MigrationError: + raise + except Exception as error: # noqa: BLE001 - state unknown: retry the whole step + last = error + log(f"Checking the index failed: {error}") time.sleep(min(2**attempt, 10)) raise MigrationError(f"could not create index {index.name}: {last}") @@ -162,25 +230,19 @@ def main(argv: list[str] | None = None) -> int: ) parser.add_argument("--to", required=True, choices=["hybrid", "keyword"]) parser.add_argument("--namespace", help=f"key namespace (default: {DEFAULT_NAMESPACE})") - parser.add_argument("--env-file", help="read the Upstash credentials from this .env file") parser.add_argument("--yes", action="store_true", help="do not ask for confirmation") args = parser.parse_args(argv) - env = {k: os.environ[k] for k in _KEYS if os.environ.get(k)} - if args.env_file: - env.update(_read_env_file(args.env_file)) - if not (env.get("UPSTASH_REDIS_REST_URL") and env.get("UPSTASH_REDIS_REST_TOKEN")): + url = os.environ.get("UPSTASH_REDIS_REST_URL") + token = os.environ.get("UPSTASH_REDIS_REST_TOKEN") + if not (url and token): print("UPSTASH_REDIS_REST_URL and UPSTASH_REDIS_REST_TOKEN are required.", file=sys.stderr) return 2 - namespace = args.namespace or env.get("UPSTASH_HERMES_NAMESPACE") or DEFAULT_NAMESPACE + namespace = args.namespace or os.environ.get("UPSTASH_HERMES_NAMESPACE") or DEFAULT_NAMESPACE from upstash_redis import Redis - redis = Redis( - url=env["UPSTASH_REDIS_REST_URL"], - token=env["UPSTASH_REDIS_REST_TOKEN"], - allow_telemetry=False, - ) + redis = Redis(url=url, token=token, allow_telemetry=False) def confirm(plan: str) -> bool: print(plan) @@ -196,6 +258,9 @@ def confirm(plan: str) -> bool: except MigrationError as error: print(f"Migration stopped: {error}", file=sys.stderr) return 1 + except Exception as error: # noqa: BLE001 - a clean message instead of a traceback + print(f"Migration failed: {type(error).__name__}: {error}", file=sys.stderr) + return 1 return 0 diff --git a/python/hermes-memory/tests/test_semantic.py b/python/hermes-memory/tests/test_semantic.py index 99ad338..e269f87 100644 --- a/python/hermes-memory/tests/test_semantic.py +++ b/python/hermes-memory/tests/test_semantic.py @@ -5,14 +5,26 @@ from dataclasses import dataclass from typing import Any +import httpx import pytest from upstash_hermes_memory import AgentMemory, ChatHistory, UpstashMemoryProvider from upstash_hermes_memory import provider as provider_module -from upstash_hermes_memory._search import SHARED_SCHEMA, shared_index, with_hybrid_text +from upstash_hermes_memory._search import ( + SHARED_SCHEMA, + describe, + matches, + shared_index, + with_hybrid_text, +) from upstash_hermes_memory.migrate import MigrationError, migrate from upstash_redis.errors import UpstashError +from upstash_redis.search import deserialize_describe_response MISMATCH = "ERR Index agentkit already exists with a different configuration" +HYBRID = with_hybrid_text(SHARED_SCHEMA) +USER = {"userId": {"$eq": "u1"}, "$mustNot": [{"kind": {"$eq": "chat"}}]} +PREFIXES = ["agentkit:memory:", "agentkit:chat:"] +quiet = print # migration log lines show up in pytest output on failure only @dataclass @@ -22,6 +34,10 @@ class Row: data: dict[str, Any] +def memory_row(id, text="x", score=1.0): + return Row(f"agentkit:memory:u1:{id}", score, {"text": text, "userId": "u1", "createdAt": 1}) + + def describe_reply(schema, name="agentkit", prefixes=("agentkit:chat:", "agentkit:memory:")): fields = [[f, t] + (["NOTOKENIZE"] if nt else []) for f, t, nt in schema] return ["name", name, "type", "JSON", "prefixes", list(prefixes), "language", "english", @@ -29,13 +45,23 @@ def describe_reply(schema, name="agentkit", prefixes=("agentkit:chat:", "agentki class FakeRedis: - """Answers search commands from a small script; records every command it gets.""" - - def __init__(self, index_schema=None, rows=(), fail_hybrid=False, count=None): - self.index_schema = index_schema # None: no index - self.rows = list(rows) - self.fail_hybrid = fail_hybrid - self.count = count + """A small model of Upstash Redis Search, enough for the index lifecycle and recall. + + Memory rows and a chat count are the index contents; `pending` memories become searchable at + the next WAITINDEXING, as asynchronous indexing does. Every command is recorded. + """ + + def __init__(self, index_schema=None, memories=(), chats=0, *, parsed=False, alias_of=None): + self.index_schema = list(index_schema) if index_schema else None + self.memories = list(memories) + self.chats = chats + self.pending: list[Row] = [] + self.parsed = parsed # answer DESCRIBE like the client does (an IndexDescription) + self.alias_of = alias_of # DESCRIBE resolves the name to this index + self.hybrid_error: Exception | None = None + self.keyword_error: Exception | None = None + self.wait_result: int | None = None + self.kv: dict[str, str] = {} self.commands: list[list[Any]] = [] self._headers: dict[str, str] = {} @@ -49,25 +75,46 @@ def execute(self, command): self.index_schema = schema return 1 if op == "SEARCH.DESCRIBE": - return describe_reply(self.index_schema) if self.index_schema else None + if not self.index_schema: + return None + reply = describe_reply(self.index_schema, name=self.alias_of or command[1]) + return deserialize_describe_response(reply) if self.parsed else reply if op == "SEARCH.DROP": dropped, self.index_schema = self.index_schema is not None, None return int(dropped) if op == "SEARCH.WAITINDEXING": + if self.wait_result is not None: + return self.wait_result + self.memories += self.pending + self.pending = [] return 1 if self.index_schema else 0 if op == "SEARCH.QUERY": if self.index_schema is None: return [] filter = json.loads(command[2]) - if "$rrf" in filter and self.fail_hybrid: - raise UpstashError("ERR dot metric only accepts unit-length vectors") - return list(self.rows) + error = self.hybrid_error if "$rrf" in filter else self.keyword_error + if error: + raise error + limit = int(command[command.index("LIMIT") + 1]) if "LIMIT" in command else 10 + return self.memories[:limit] if op == "SEARCH.COUNT": if self.index_schema is None: return -1 - if "$rrf" in json.loads(command[2]) or "$ann" in json.loads(command[2]): + filter = json.loads(command[2]) + if {"$rrf", "$ann"} & set(filter): raise UpstashError("ERR $ann cannot be used with SEARCH.COUNT") - return self.count if self.count is not None else len(self.rows) + if filter == {"kind": {"$eq": "chat"}}: + return self.chats + if filter == {}: + return len(self.memories) + self.chats + return len(self.memories) + if op == "GET": + return self.kv.get(command[1]) + if op == "SET": + self.kv[command[1]] = command[2] + return "OK" + if op == "DEL": + return int(self.kv.pop(command[1], None) is not None) return 1 def delete(self, key): @@ -77,6 +124,9 @@ def delete(self, key): def expire(self, key, seconds): return True + def ops(self): + return [c[0] for c in self.commands] + def queries(self): return [json.loads(c[2]) for c in self.commands if c[0] == "SEARCH.QUERY"] @@ -92,12 +142,31 @@ def _schema_of(command): return out -HYBRID = with_hybrid_text(SHARED_SCHEMA) -USER = {"userId": {"$eq": "u1"}, "$mustNot": [{"kind": {"$eq": "chat"}}]} +@pytest.fixture(autouse=True) +def no_sleep(monkeypatch): + monkeypatch.setattr("upstash_hermes_memory.migrate.time.sleep", lambda s: None) -def memory_row(id, text, score=1.0): - return Row(f"agentkit:memory:u1:{id}", score, {"text": text, "userId": "u1", "createdAt": 1}) +# -- describe ------------------------------------------------------------------------------------ + + +@pytest.mark.parametrize("parsed", [False, True]) +def test_describe_reads_raw_and_client_parsed_replies_alike(parsed): + redis = FakeRedis(HYBRID, parsed=parsed) + found = describe(redis, "agentkit") + assert matches(found, PREFIXES, HYBRID, "agentkit") + assert not matches(found, PREFIXES, SHARED_SCHEMA, "agentkit") + + +def test_describe_sees_options_this_package_never_sets(): + reply = describe_reply(SHARED_SCHEMA) + reply[-1][2] = ["text", "TEXT", "NOSTEM"] + + class Raw(FakeRedis): + def execute(self, command): + return reply + + assert not matches(describe(Raw(), "agentkit"), PREFIXES, SHARED_SCHEMA) # -- index creation ------------------------------------------------------------------------------ @@ -113,32 +182,49 @@ def test_semantic_creates_text_as_the_only_hybrid_field(): def test_memory_and_chats_create_the_index_the_same_way(): redis = FakeRedis() - memory = AgentMemory(redis, semantic=True) - chats = ChatHistory(redis, semantic=True) - assert memory.index.create_command() == chats.index.create_command() + assert ( + AgentMemory(redis, semantic=True).index.create_command() + == ChatHistory(redis, semantic=True).index.create_command() + ) @pytest.mark.parametrize("semantic, existing", [(True, SHARED_SCHEMA), (False, HYBRID)]) -def test_an_existing_index_of_the_other_kind_is_used_never_dropped(semantic, existing): - redis = FakeRedis(index_schema=list(existing)) +@pytest.mark.parametrize("parsed", [False, True]) +def test_an_existing_index_of_the_other_kind_is_used_never_dropped(semantic, existing, parsed): + redis = FakeRedis(existing, parsed=parsed) memory = AgentMemory(redis, semantic=semantic) memory.index.ensure() assert memory.index.actual_schema == list(existing) - assert not [c for c in redis.commands if c[0] == "SEARCH.DROP"] + assert "SEARCH.DROP" not in redis.ops() def test_an_unknown_index_definition_still_fails(): - redis = FakeRedis(index_schema=SHARED_SCHEMA + [("extra", "TEXT", False)]) + redis = FakeRedis(SHARED_SCHEMA + [("extra", "TEXT", False)]) + with pytest.raises(UpstashError): + AgentMemory(redis, semantic=True).index.ensure() + assert "SEARCH.DROP" not in redis.ops() + + +def test_an_alias_to_another_index_is_not_accepted(): + redis = FakeRedis(SHARED_SCHEMA, alias_of="someone_else") with pytest.raises(UpstashError): AgentMemory(redis, semantic=True).index.ensure() - assert not [c for c in redis.commands if c[0] == "SEARCH.DROP"] + + +def test_an_index_that_vanishes_during_create_is_not_marked_ready(): + redis = FakeRedis() + redis.wait_result = 0 + memory = AgentMemory(redis, semantic=True) + with pytest.raises(RuntimeError): + memory.index.ensure() + assert not memory.index._ready # -- recall -------------------------------------------------------------------------------------- def test_hybrid_recall_fuses_keyword_and_meaning_for_this_user_only(): - redis = FakeRedis(index_schema=list(HYBRID), rows=[memory_row("1", "Weeks start Monday", 0.03)]) + redis = FakeRedis(HYBRID, [memory_row("1", "Weeks start Monday", 0.03)]) memory = AgentMemory(redis, semantic=True, min_score=5) hits = memory.recall("what day does the week begin", user_id="u1", top_k=3) assert [(h.text, h.retrieval) for h in hits] == [("Weeks start Monday", "hybrid")] @@ -150,20 +236,34 @@ def test_hybrid_recall_fuses_keyword_and_meaning_for_this_user_only(): } -def test_semantic_on_a_keyword_index_recalls_by_keyword_and_says_so(caplog): - redis = FakeRedis(index_schema=list(SHARED_SCHEMA), rows=[memory_row("1", "x")]) +def test_semantic_on_a_keyword_index_recalls_by_keyword_and_says_so_once(caplog): + redis = FakeRedis(SHARED_SCHEMA, [memory_row("1")]) memory = AgentMemory(redis, semantic=True) with caplog.at_level(logging.WARNING): memory.recall("a", user_id="u1") memory.recall("b", user_id="u1") assert all("$rrf" not in q for q in redis.queries()) - assert [r.message for r in caplog.records].count(caplog.records[0].message) == 1 - assert "migrate" in caplog.records[0].message + assert len(caplog.records) == 1 and "migrate" in caplog.records[0].message -def test_a_failed_hybrid_query_falls_back_to_keyword_once_with_min_score(): - rows = [memory_row("1", "strong", 3.0), memory_row("2", "weak", 0.2)] - redis = FakeRedis(index_schema=list(HYBRID), rows=rows, fail_hybrid=True) +def test_semantic_off_on_a_hybrid_index_says_embeddings_go_on(caplog): + redis = FakeRedis(HYBRID, [memory_row("1")]) + with caplog.at_level(logging.WARNING): + AgentMemory(redis).recall("a", user_id="u1") + assert "still embedded" in caplog.records[0].message + + +@pytest.mark.parametrize( + "error", + [ + UpstashError("ERR dot metric only accepts unit-length vectors"), + httpx.ConnectError("connection reset"), + httpx.ReadTimeout("read timed out"), + ], +) +def test_a_failed_hybrid_query_falls_back_to_keyword_once_with_min_score(error): + redis = FakeRedis(HYBRID, [memory_row("1", "strong", 3.0), memory_row("2", "weak", 0.2)]) + redis.hybrid_error = error memory = AgentMemory(redis, semantic=True, min_score=1) hits = memory.recall("billing for big accounts", user_id="u1") assert [(h.text, h.retrieval) for h in hits] == [("strong", "keyword")] @@ -171,8 +271,23 @@ def test_a_failed_hybrid_query_falls_back_to_keyword_once_with_min_score(): assert redis.queries()[1] == {**USER, "text": {"$smart": "billing for big accounts"}} +def test_a_failed_fallback_raises_for_the_caller_to_handle(): + redis = FakeRedis(HYBRID, [memory_row("1")]) + redis.hybrid_error = redis.keyword_error = httpx.ConnectError("down") + with pytest.raises(httpx.ConnectError): + AgentMemory(redis, semantic=True).recall("x", user_id="u1") + assert len(redis.queries()) == 2 # one hybrid attempt, one keyword attempt + + +def test_programming_errors_are_not_swallowed_by_the_fallback(): + redis = FakeRedis(HYBRID, [memory_row("1")]) + redis.hybrid_error = TypeError("bug") + with pytest.raises(TypeError): + AgentMemory(redis, semantic=True).recall("x", user_id="u1") + + def test_keyword_mode_never_uses_a_retriever(): - redis = FakeRedis(index_schema=list(HYBRID), rows=[memory_row("1", "x")]) + redis = FakeRedis(HYBRID, [memory_row("1")]) AgentMemory(redis, semantic=True).recall("x", user_id="u1", mode="keyword") assert all("$rrf" not in q for q in redis.queries()) @@ -183,14 +298,13 @@ def test_an_unknown_mode_is_refused(): def test_an_empty_hybrid_result_checks_the_index_without_counting_a_retriever(): - redis = FakeRedis(index_schema=list(HYBRID), rows=[]) + redis = FakeRedis(HYBRID) assert AgentMemory(redis, semantic=True).recall("nothing", user_id="u1") == [] - counts = [c[2] for c in redis.commands if c[0] == "SEARCH.COUNT"] - assert counts == ["{}"] + assert [c[2] for c in redis.commands if c[0] == "SEARCH.COUNT"] == ["{}"] def test_semantic_off_keeps_the_0_2_commands(): - redis = FakeRedis(index_schema=list(SHARED_SCHEMA), rows=[memory_row("1", "x", 2.0)]) + redis = FakeRedis(SHARED_SCHEMA, [memory_row("1", "x", 2.0)]) hits = AgentMemory(redis).recall("x", user_id="u1") assert redis.queries() == [{**USER, "text": {"$smart": "x"}}] assert hits[0].retrieval == "keyword" @@ -203,9 +317,7 @@ def test_semantic_off_keeps_the_0_2_commands(): def semantic_provider(monkeypatch): env = {"UPSTASH_HERMES_SEMANTIC": "1", "UPSTASH_HERMES_USER_ID": "u1"} monkeypatch.setattr(provider_module, "get_secret", lambda n, d=None: env.get(n, d)) - redis = FakeRedis( - index_schema=list(HYBRID), rows=[memory_row(str(i), f"m{i}") for i in range(5)] - ) + redis = FakeRedis(HYBRID, [memory_row(str(i), f"m{i}") for i in range(5)]) provider = UpstashMemoryProvider(redis) provider.initialize("s1", platform="cli") return provider, redis @@ -217,13 +329,20 @@ def test_the_env_flag_turns_semantic_on_for_both_stores(semantic_provider): assert provider._chats.index.schema == provider._memory.index.schema == HYBRID -def test_prefetch_asks_for_three_memories_in_semantic_mode(semantic_provider): +def test_prefetch_injects_three_memories_in_semantic_mode(semantic_provider): provider, redis = semantic_provider - provider.prefetch("what do I prefer") + block = provider.prefetch("what do I prefer") + assert block.count("\n- ") == 3 (limit,) = [c[c.index("LIMIT") + 1] for c in redis.commands if c[0] == "SEARCH.QUERY"] assert limit == "3" +def test_prefetch_survives_a_redis_outage(semantic_provider): + provider, redis = semantic_provider + redis.hybrid_error = redis.keyword_error = httpx.ConnectError("down") + assert provider.prefetch("anything") == "" + + def test_the_search_tool_says_how_results_were_found(semantic_provider): provider, _ = semantic_provider result = json.loads(provider.handle_tool_call("upstash_memory_search", {"query": "m"})) @@ -239,54 +358,91 @@ def test_mirror_deletion_matches_by_keyword_only(semantic_provider): # -- migration ----------------------------------------------------------------------------------- -def test_migration_rebuilds_a_keyword_index_and_checks_the_counts(): - redis = FakeRedis(index_schema=list(SHARED_SCHEMA), count=7) - assert migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) == "migrated" - ops = [c[0] for c in redis.commands] - assert ops.index("SEARCH.DROP") < ops.index("SEARCH.CREATE") < ops.index("SEARCH.WAITINDEXING") - assert redis.index_schema == list(HYBRID) +def migrated(redis, to="hybrid", **kwargs): + return migrate(redis, namespace="agentkit", to=to, log=quiet, **kwargs) + + +def test_migration_rebuilds_a_keyword_index_and_checks_it(): + redis = FakeRedis(SHARED_SCHEMA, [memory_row(i) for i in range(7)], chats=2) + assert migrated(redis) == "migrated" + ops = redis.ops() + assert ops.index("SET") < ops.index("SEARCH.DROP") < ops.index("SEARCH.CREATE") + assert redis.index_schema == HYBRID + assert redis.kv == {} # baseline removed once checked + + +def test_pending_updates_are_settled_before_the_baseline(): + redis = FakeRedis(SHARED_SCHEMA, [memory_row(1)], chats=1) + redis.pending = [memory_row(2)] # written just before the clients stopped + assert migrated(redis) == "migrated" + first_count = redis.ops().index("SEARCH.COUNT") + assert redis.ops().index("SEARCH.WAITINDEXING") < first_count def test_migration_back_to_keyword_restores_the_0_2_index(): - redis = FakeRedis(index_schema=list(HYBRID), count=3) - migrate(redis, namespace="agentkit", to="keyword", log=lambda m: None) + redis = FakeRedis(HYBRID, [memory_row(1)], chats=3) + assert migrated(redis, to="keyword") == "migrated" assert redis.index_schema == list(SHARED_SCHEMA) def test_migration_to_the_current_kind_does_nothing(): - redis = FakeRedis(index_schema=list(HYBRID)) - assert migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) == "unchanged" - assert "SEARCH.DROP" not in [c[0] for c in redis.commands] - - -def test_migration_refuses_an_index_it_did_not_create(): - redis = FakeRedis(index_schema=SHARED_SCHEMA + [("extra", "TEXT", False)]) + redis = FakeRedis(HYBRID, [memory_row(1)]) + assert migrated(redis) == "unchanged" + assert "SEARCH.DROP" not in redis.ops() + + +@pytest.mark.parametrize( + "redis", + [ + FakeRedis(SHARED_SCHEMA + [("extra", "TEXT", False)]), + FakeRedis(SHARED_SCHEMA, alias_of="agentkit_v2"), + ], +) +def test_migration_refuses_an_index_it_did_not_create_or_an_alias(redis): with pytest.raises(MigrationError, match="refusing"): - migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) - assert "SEARCH.DROP" not in [c[0] for c in redis.commands] + migrated(redis) + assert "SEARCH.DROP" not in redis.ops() def test_a_declined_migration_drops_nothing(): - redis = FakeRedis(index_schema=list(SHARED_SCHEMA)) + redis = FakeRedis(SHARED_SCHEMA) with pytest.raises(MigrationError, match="cancelled"): - migrate(redis, namespace="agentkit", to="hybrid", confirm=lambda p: False, log=print) - assert "SEARCH.DROP" not in [c[0] for c in redis.commands] + migrated(redis, confirm=lambda plan: False) + assert "SEARCH.DROP" not in redis.ops() and redis.kv == {} -def test_a_failed_rebuild_says_how_to_recover(monkeypatch): - monkeypatch.setattr("upstash_hermes_memory.migrate.time.sleep", lambda s: None) +def test_a_failed_drop_changes_nothing(): + class NoDrop(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.DROP": + raise httpx.ReadTimeout("timed out") + return super().execute(command) + redis = NoDrop(SHARED_SCHEMA, [memory_row(1)]) + with pytest.raises(MigrationError, match="nothing changed"): + migrated(redis) + assert redis.index_schema == list(SHARED_SCHEMA) and redis.kv == {} + + +def test_a_failed_rebuild_says_where_things_stand_and_resumes(): class NoCreate(FakeRedis): + broken = True + def execute(self, command): - if command[0] == "SEARCH.CREATE": + if command[0] == "SEARCH.CREATE" and self.broken: self.commands.append(command) - raise TimeoutError("read timed out") + raise httpx.ReadTimeout("read timed out") return super().execute(command) - redis = NoCreate(index_schema=list(SHARED_SCHEMA)) - with pytest.raises(MigrationError, match="--to keyword"): - migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) - assert [c[0] for c in redis.commands].count("SEARCH.CREATE") == 3 + redis = NoCreate(SHARED_SCHEMA, [memory_row(1), memory_row(2)], chats=1) + with pytest.raises(MigrationError, match="The index is absent.*--to keyword"): + migrated(redis) + assert redis.ops().count("SEARCH.CREATE") == 3 + assert "agentkit:migrate:baseline" in redis.kv + + redis.broken = False # the server is back: the same command resumes and checks the baseline + assert migrated(redis) == "migrated" + assert redis.index_schema == HYBRID and redis.kv == {} def test_a_timed_out_create_that_succeeded_is_not_repeated(): @@ -294,21 +450,47 @@ class SlowCreate(FakeRedis): def execute(self, command): if command[0] == "SEARCH.CREATE": super().execute(command) - raise TimeoutError("read timed out") + raise httpx.ReadTimeout("read timed out") return super().execute(command) - redis = SlowCreate(index_schema=list(SHARED_SCHEMA), count=2) - assert migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) == "migrated" - assert [c[0] for c in redis.commands].count("SEARCH.CREATE") == 1 + redis = SlowCreate(SHARED_SCHEMA, [memory_row(1)]) + assert migrated(redis) == "migrated" + assert redis.ops().count("SEARCH.CREATE") == 1 -def test_a_count_change_during_the_rebuild_is_reported(): +def test_a_count_change_is_reported_and_rechecked_on_the_next_run(): class Writer(FakeRedis): def execute(self, command): if command[0] == "SEARCH.DROP": - self.count = 8 # a client kept writing + self.memories.append(memory_row("late")) # a client kept writing return super().execute(command) - redis = Writer(index_schema=list(SHARED_SCHEMA), count=7) + redis = Writer(SHARED_SCHEMA, [memory_row(i) for i in range(3)]) + with pytest.raises(MigrationError, match="still writing"): + migrated(redis) + assert "agentkit:migrate:baseline" in redis.kv # kept: a rerun checks again, not "unchanged" with pytest.raises(MigrationError, match="still writing"): - migrate(redis, namespace="agentkit", to="hybrid", log=lambda m: None) + migrated(redis) + + redis.memories.pop() # the stray write is dealt with + assert migrated(redis) == "resumed" + assert redis.kv == {} + + +def test_lost_memories_are_caught_by_key_not_just_by_count(): + class Swap(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.DROP": + self.memories = [memory_row("other")] # same count, different documents + return super().execute(command) + + redis = Swap(SHARED_SCHEMA, [memory_row("mine")]) + with pytest.raises(MigrationError, match="lost 1 sampled"): + migrated(redis) + + +def test_a_pending_migration_in_the_other_direction_blocks_a_new_one(): + redis = FakeRedis(None) + redis.kv["agentkit:migrate:baseline"] = json.dumps({"to": "hybrid", "memories": 0, "chats": 0}) + with pytest.raises(MigrationError, match="--to hybrid first"): + migrated(redis, to="keyword") diff --git a/python/hermes-memory/tests/test_semantic_redis.py b/python/hermes-memory/tests/test_semantic_redis.py index 93fa8f9..8fb9c3a 100644 --- a/python/hermes-memory/tests/test_semantic_redis.py +++ b/python/hermes-memory/tests/test_semantic_redis.py @@ -33,9 +33,12 @@ def hybrid_recall(memory, query, user_id="u1", top_k=3, tries=4): + """Recall that was answered by hybrid search. Results that came back by keyword are the + fallback after a server error, so they are retried; no results at all is a failure.""" for _ in range(tries): hits = memory.recall(query, user_id=user_id, top_k=top_k) - if hits and hits[0].retrieval == "hybrid": + assert hits, f"no memories recalled for {query!r}" + if hits[0].retrieval == "hybrid": return hits pytest.skip("hybrid search kept failing on the server; recall fell back to keyword") @@ -99,8 +102,13 @@ def test_migration_round_trip_keeps_memories_and_chats(redis, prefix): assert migrate(redis, namespace=prefix, to="hybrid", log=quiet) == "migrated" semantic = AgentMemory(redis, namespace=prefix, semantic=True, enable_telemetry=False) - assert "coffee" in ids(hybrid_recall(semantic, "what hot drink does he enjoy")) + semantic.index.ensure() + assert semantic.index.field_type("text") == "HYBRID" + # Keyword search and chats keep working on the rebuilt index whatever the semantic side does. + keyword_hits = semantic.recall("flat white", user_id="u1", mode="keyword") + assert "coffee" in ids(keyword_hits) assert [h.session_id for h in chats.search_chats("flat white", user_id="u1")] == ["s1"] + assert redis.execute(["GET", f"{prefix}:migrate:baseline"]) is None assert migrate(redis, namespace=prefix, to="hybrid", log=quiet) == "unchanged" assert migrate(redis, namespace=prefix, to="keyword", log=quiet) == "migrated" From 349158f7d4f74fc3e4228a1d83a65b21e11be5ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sertu=C4=9F=20Kaya?= Date: Mon, 9 Nov 2026 13:30:23 +0000 Subject: [PATCH 3/6] fix(hermes-memory): allow rolling back an interrupted migration, page past 1000 results, recover in wait_indexing --- .../src/upstash_hermes_memory/_search.py | 10 ++- .../src/upstash_hermes_memory/migrate.py | 62 ++++++++++------- python/hermes-memory/tests/test_semantic.py | 68 +++++++++++++++++-- 3 files changed, 109 insertions(+), 31 deletions(-) diff --git a/python/hermes-memory/src/upstash_hermes_memory/_search.py b/python/hermes-memory/src/upstash_hermes_memory/_search.py index cdd813a..8e9015a 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/_search.py +++ b/python/hermes-memory/src/upstash_hermes_memory/_search.py @@ -236,9 +236,15 @@ def _raw_count(self, filter: dict[str, Any]) -> int: return int(getattr(result, "count", result)) def wait_indexing(self) -> None: - """Block until every write so far is searchable (and every delete has left the index).""" + """Block until every write so far is searchable (and every delete has left the index). + + WAITINDEXING answers 0 when the index is gone (another client dropped it): it is created + again (its documents are indexed by the create-time scan) and waited for once more. + """ self.ensure() - self.redis.execute(["SEARCH.WAITINDEXING", self.name]) + if self.redis.execute(["SEARCH.WAITINDEXING", self.name]) != 1: + self._ready = False + self.ensure() def drop(self) -> None: self.redis.execute(["SEARCH.DROP", self.name]) diff --git a/python/hermes-memory/src/upstash_hermes_memory/migrate.py b/python/hermes-memory/src/upstash_hermes_memory/migrate.py index 3c3ddc4..afae6f1 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/migrate.py +++ b/python/hermes-memory/src/upstash_hermes_memory/migrate.py @@ -38,6 +38,8 @@ _CHATS = {"kind": {"$eq": "chat"}} _MEMORIES = {"$mustNot": [{"kind": {"$eq": "chat"}}]} _SAMPLE = 200 # memory keys checked one by one after the rebuild +_PAGE = 1000 # SEARCH.QUERY's largest LIMIT +_MAX_OFFSET = 10_000 # and its largest OFFSET class MigrationError(Exception): @@ -54,9 +56,12 @@ def count(filter: dict[str, Any]) -> int: return count(_MEMORIES), count(_CHATS) -def _memory_keys(redis: Any, name: str, limit: int) -> list[str]: - rows = redis.execute(["SEARCH.QUERY", name, dumps(_MEMORIES), "LIMIT", str(limit)]) or [] - return sorted(getattr(row, "key", None) or row[0] for row in rows) +def _memory_keys(redis: Any, name: str, limit: int, offset: int = 0) -> list[str]: + cmd = ["SEARCH.QUERY", name, dumps(_MEMORIES), "LIMIT", str(limit), "NOCONTENT"] + if offset: + cmd += ["OFFSET", str(offset)] + rows = redis.execute(cmd) or [] + return [getattr(row, "key", None) or row[0] for row in rows] def _state(redis: Any, name: str, prefixes: list[str], schemas: dict[str, list]) -> str: @@ -101,11 +106,13 @@ def migrate( baseline_key = f"{namespace}:migrate:baseline" raw = redis.execute(["GET", baseline_key]) baseline = json.loads(raw) if raw else None - if baseline and baseline.get("to") != to: - raise MigrationError( - f"an interrupted migration to {baseline.get('to')} is pending ({baseline_key}); " - f"finish it with --to {baseline.get('to')} first" - ) + if baseline and to not in (baseline.get("to"), baseline.get("from")): + raise MigrationError(f"a migration is pending ({baseline_key}): {raw}") + if baseline and baseline["to"] != to: + # Rolling an interrupted migration back: same baseline, the other target. + log(f"Rolling the interrupted migration to {baseline['to']} back to {to}.") + baseline = {**baseline, "from": baseline["to"], "to": to} + redis.execute(["SET", baseline_key, dumps(baseline)]) state = _state(redis, name, prefixes, schemas) if state == "unknown": @@ -143,14 +150,16 @@ def migrate( ) if not confirm(plan): raise MigrationError("cancelled") - baseline = { - "to": to, - "memories": memories, - "chats": chats, - "sample": _memory_keys(redis, name, _SAMPLE), - "at": int(time.time()), - } - redis.execute(["SET", baseline_key, dumps(baseline)]) + if baseline is None: # a rollback keeps the baseline of the original index + baseline = { + "from": came_from, + "to": to, + "memories": memories, + "chats": chats, + "sample": sorted(_memory_keys(redis, name, _SAMPLE)), + "at": int(time.time()), + } + redis.execute(["SET", baseline_key, dumps(baseline)]) try: redis.execute(["SEARCH.DROP", name]) except Exception as error: # noqa: BLE001 - reconciled below @@ -186,14 +195,19 @@ def _verify(redis: Any, name: str, baseline: dict[str, Any], to: str) -> None: f"(before: {baseline['memories']} and {baseline['chats']}). Was a client still " "writing? Check, then re-run to verify again." ) - sample = baseline.get("sample") or [] - if sample: - found = set(_memory_keys(redis, name, max(after[0], len(sample)))) - missing = [k for k in sample if k not in found] - if missing: - raise MigrationError( - f"index {name} lost {len(missing)} sampled memories, e.g. {missing[0]}" - ) + missing = set(baseline.get("sample") or []) + offset = 0 + while missing and offset <= _MAX_OFFSET: + page = _memory_keys(redis, name, _PAGE, offset) + missing -= set(page) + if len(page) < _PAGE: + break + offset += _PAGE + if missing and offset <= _MAX_OFFSET: + raise MigrationError( + f"index {name} lost {len(missing)} sampled memories, e.g. {min(missing)}" + ) + # Past OFFSET's limit the remaining sample can't be listed; the counts above still matched. def _create( diff --git a/python/hermes-memory/tests/test_semantic.py b/python/hermes-memory/tests/test_semantic.py index e269f87..73421be 100644 --- a/python/hermes-memory/tests/test_semantic.py +++ b/python/hermes-memory/tests/test_semantic.py @@ -96,7 +96,10 @@ def execute(self, command): if error: raise error limit = int(command[command.index("LIMIT") + 1]) if "LIMIT" in command else 10 - return self.memories[:limit] + offset = int(command[command.index("OFFSET") + 1]) if "OFFSET" in command else 0 + if not 1 <= limit <= 1000 or not 0 <= offset <= 10_000: # the documented bounds + raise UpstashError("ERR LIMIT must be between 1 and 1000") + return self.memories[offset : offset + limit] if op == "SEARCH.COUNT": if self.index_schema is None: return -1 @@ -489,8 +492,63 @@ def execute(self, command): migrated(redis) -def test_a_pending_migration_in_the_other_direction_blocks_a_new_one(): +def test_a_failed_rebuild_can_be_rolled_back_with_the_command_it_suggests(): + class NoHybrid(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.CREATE" and "HYBRID" in command: + self.commands.append(command) + raise UpstashError("ERR embeddings unavailable") + return super().execute(command) + + redis = NoHybrid(SHARED_SCHEMA, [memory_row(1), memory_row(2)], chats=1) + with pytest.raises(MigrationError, match="--to keyword"): + migrated(redis) + assert redis.index_schema is None + assert migrated(redis, to="keyword") == "migrated" # checked against the original baseline + assert redis.index_schema == list(SHARED_SCHEMA) and redis.kv == {} + + +def test_a_rollback_after_a_rebuild_that_failed_its_check_keeps_the_original_baseline(): + class Writer(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.DROP" and self.index_schema == list(SHARED_SCHEMA): + self.memories.append(memory_row("late")) + return super().execute(command) + + redis = Writer(SHARED_SCHEMA, [memory_row(1)]) + with pytest.raises(MigrationError, match="still writing"): + migrated(redis) + redis.memories.pop() + assert migrated(redis, to="keyword") == "migrated" + assert redis.index_schema == list(SHARED_SCHEMA) + + +def test_migration_checks_stores_beyond_one_query_page(): + class Reorder(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.DROP": + self.memories.reverse() # the rebuilt index ranks documents differently + return super().execute(command) + + redis = Reorder(SHARED_SCHEMA, [memory_row(i) for i in range(2500)], chats=4) + assert migrated(redis) == "migrated" + offsets = [c[c.index("OFFSET") + 1] for c in redis.commands if "OFFSET" in c] + assert offsets == ["1000", "2000"] # paged to the sampled keys, within LIMIT's bound + + +def test_an_unrelated_pending_baseline_blocks_a_migration(): redis = FakeRedis(None) - redis.kv["agentkit:migrate:baseline"] = json.dumps({"to": "hybrid", "memories": 0, "chats": 0}) - with pytest.raises(MigrationError, match="--to hybrid first"): - migrated(redis, to="keyword") + redis.kv["agentkit:migrate:baseline"] = json.dumps( + {"from": "keyword", "to": "keyword", "memories": 0, "chats": 0} + ) + with pytest.raises(MigrationError, match="pending"): + migrated(redis) + + +def test_wait_indexing_recreates_an_index_that_was_dropped_under_it(): + redis = FakeRedis(HYBRID, [memory_row(1)]) + memory = AgentMemory(redis, semantic=True) + memory.index.ensure() + redis.index_schema = None # another client dropped it + memory.index.wait_indexing() + assert redis.index_schema == HYBRID and memory.index._ready From 75d569de15c336fddd0d193f52b8a265f89fbff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sertu=C4=9F=20Kaya?= Date: Mon, 9 Nov 2026 13:32:36 +0000 Subject: [PATCH 4/6] fix(hermes-memory): keep an inherited migration baseline when a rollback's drop fails --- .../src/upstash_hermes_memory/migrate.py | 8 +++++-- python/hermes-memory/tests/test_semantic.py | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/python/hermes-memory/src/upstash_hermes_memory/migrate.py b/python/hermes-memory/src/upstash_hermes_memory/migrate.py index afae6f1..e0d0744 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/migrate.py +++ b/python/hermes-memory/src/upstash_hermes_memory/migrate.py @@ -150,7 +150,8 @@ def migrate( ) if not confirm(plan): raise MigrationError("cancelled") - if baseline is None: # a rollback keeps the baseline of the original index + fresh = baseline is None # a rollback keeps the baseline of the original index + if fresh: baseline = { "from": came_from, "to": to, @@ -166,7 +167,10 @@ def migrate( log(f"Drop failed: {error}") state = _state(redis, name, prefixes, schemas) if state == came_from: - redis.execute(["DEL", baseline_key]) + # Only a baseline taken by this run goes: one from an interrupted migration still + # describes the documents that rebuild has to account for. + if fresh: + redis.execute(["DEL", baseline_key]) raise MigrationError(f"index {name} could not be dropped; nothing changed") if state != "absent": raise MigrationError(f"index {name} is {state} after the drop; re-run to resume") diff --git a/python/hermes-memory/tests/test_semantic.py b/python/hermes-memory/tests/test_semantic.py index 73421be..ef4f9a8 100644 --- a/python/hermes-memory/tests/test_semantic.py +++ b/python/hermes-memory/tests/test_semantic.py @@ -523,6 +523,30 @@ def execute(self, command): assert redis.index_schema == list(SHARED_SCHEMA) +def test_a_failed_drop_during_a_rollback_keeps_the_original_baseline(): + class Flaky(FakeRedis): + swap = True + drop_fails = False + + def execute(self, command): + if command[0] == "SEARCH.DROP": + if self.drop_fails: + raise httpx.ReadTimeout("timed out") + if self.swap: + self.memories = [memory_row("other")] # the hybrid rebuild loses a memory + return super().execute(command) + + redis = Flaky(SHARED_SCHEMA, [memory_row("mine")]) + with pytest.raises(MigrationError, match="lost 1 sampled"): + migrated(redis) + redis.drop_fails = True + with pytest.raises(MigrationError, match="nothing changed"): + migrated(redis, to="keyword") + assert "agentkit:migrate:baseline" in redis.kv + with pytest.raises(MigrationError, match="lost 1 sampled"): # not "unchanged" + migrated(redis) + + def test_migration_checks_stores_beyond_one_query_page(): class Reorder(FakeRedis): def execute(self, command): From bd3eec890cebdff104881384c2da921e0424c89e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sertu=C4=9F=20Kaya?= Date: Mon, 9 Nov 2026 13:34:53 +0000 Subject: [PATCH 5/6] fix(hermes-memory): report lost samples when the check listed every memory --- .../src/upstash_hermes_memory/migrate.py | 7 +++++-- python/hermes-memory/tests/test_semantic.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/python/hermes-memory/src/upstash_hermes_memory/migrate.py b/python/hermes-memory/src/upstash_hermes_memory/migrate.py index e0d0744..47abdcc 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/migrate.py +++ b/python/hermes-memory/src/upstash_hermes_memory/migrate.py @@ -200,14 +200,17 @@ def _verify(redis: Any, name: str, baseline: dict[str, Any], to: str) -> None: "writing? Check, then re-run to verify again." ) missing = set(baseline.get("sample") or []) - offset = 0 + offset, scanned = 0, 0 while missing and offset <= _MAX_OFFSET: page = _memory_keys(redis, name, _PAGE, offset) missing -= set(page) + scanned += len(page) if len(page) < _PAGE: break offset += _PAGE - if missing and offset <= _MAX_OFFSET: + # Every memory was listed (the index holds after[0]) or the last page came back short. + complete = scanned >= after[0] or offset <= _MAX_OFFSET + if missing and complete: raise MigrationError( f"index {name} lost {len(missing)} sampled memories, e.g. {min(missing)}" ) diff --git a/python/hermes-memory/tests/test_semantic.py b/python/hermes-memory/tests/test_semantic.py index ef4f9a8..12e118a 100644 --- a/python/hermes-memory/tests/test_semantic.py +++ b/python/hermes-memory/tests/test_semantic.py @@ -547,6 +547,23 @@ def execute(self, command): migrated(redis) +@pytest.mark.parametrize("size", [11_000, 11_001]) +def test_lost_samples_are_caught_at_the_paging_boundary(size): + class Swap(FakeRedis): + def execute(self, command): + if command[0] == "SEARCH.DROP": + self.memories.reverse() + self.memories[-1] = memory_row("other") # one sampled memory replaced + return super().execute(command) + + redis = Swap(SHARED_SCHEMA, [memory_row(i) for i in range(size)]) + if size == 11_000: # every memory can be listed: the loss must be reported + with pytest.raises(MigrationError, match="lost 1 sampled"): + migrated(redis) + else: # one memory is past OFFSET's limit, so the sample check stops; counts still match + assert migrated(redis) == "migrated" + + def test_migration_checks_stores_beyond_one_query_page(): class Reorder(FakeRedis): def execute(self, command): From cb2054779255bd5e77de2f7e302f08546c49567c Mon Sep 17 00:00:00 2001 From: CahidArda Date: Mon, 9 Nov 2026 14:49:15 +0000 Subject: [PATCH 6/6] feat(hermes-memory)!: make hybrid recall the only mode Memory text is always a HYBRID field and recall always merges $smart and $ann with $rrf. Remove the UPSTASH_HERMES_SEMANTIC flag, `semantic=` arguments, TEXT index support and the migrate command. An index from 0.2.x is refused with an error that says to run SEARCH.DROP; documents are kept and indexed again. Automatic recall injects 3 memories. Failed hybrid queries still fall back to keyword search for that call. --- CLAUDE.md | 3 +- python/hermes-memory/CHANGELOG.md | 12 +- python/hermes-memory/README.md | 36 +- .../src/upstash_hermes_memory/_search.py | 131 +----- .../src/upstash_hermes_memory/chat_history.py | 6 +- .../src/upstash_hermes_memory/memory.py | 56 +-- .../src/upstash_hermes_memory/migrate.py | 289 ------------- .../src/upstash_hermes_memory/provider.py | 34 +- python/hermes-memory/tests/test_layout.py | 4 +- python/hermes-memory/tests/test_redis.py | 5 +- python/hermes-memory/tests/test_semantic.py | 409 ++---------------- .../tests/test_semantic_redis.py | 60 +-- 12 files changed, 95 insertions(+), 950 deletions(-) delete mode 100644 python/hermes-memory/src/upstash_hermes_memory/migrate.py diff --git a/CLAUDE.md b/CLAUDE.md index 29378d2..496cf03 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,8 @@ starting work; it captures the non-obvious decisions and gotchas that aren't vis **Everything runs on Upstash Redis. No vector database, ever.** All "semantic"/similarity features use **Upstash Redis Search** with the **`$smart` fuzzy operator** — via `@upstash/redis`'s `redis.search` API, **not** RediSearch `FT.*` and **not** `@upstash/vector`. Matching is lexical/fuzzy (BM25), not -embeddings — keep that in mind when naming/among scoring. +embeddings — keep that in mind when naming/among scoring. The one exception is `upstash-hermes-memory`, whose memory +`text` is a Redis Search `HYBRID` field with managed embeddings (still no separate vector database). ## Packages diff --git a/python/hermes-memory/CHANGELOG.md b/python/hermes-memory/CHANGELOG.md index 50f9c2e..e5fc847 100644 --- a/python/hermes-memory/CHANGELOG.md +++ b/python/hermes-memory/CHANGELOG.md @@ -8,15 +8,13 @@ section here. The release workflow publishes any version PyPI does not have yet. ### Minor Changes -- Semantic recall, opt in with `UPSTASH_HERMES_SEMANTIC=1` (or `AgentMemory(semantic=True)`): the - memory `text` field becomes a Redis Search `HYBRID` field, and recall merges keyword (`$smart`) - and semantic (`$ann`) results with `$rrf`. Off by default; nothing changes without the flag. -- `python -m upstash_hermes_memory.migrate --to hybrid|keyword` rebuilds an existing shared index - either way, with clients stopped, and checks the counts afterwards. An existing index is never - dropped at runtime: with the flag on and an unmigrated index, recall stays keyword and warns. +- **Breaking:** recall is hybrid only. The memory `text` field is a Redis Search `HYBRID` field and + recall merges keyword (`$smart`) and semantic (`$ann`) results with `$rrf`. An index from 0.2.x + (`text` as `TEXT`) is refused with an error: stop every client and run `SEARCH.DROP `. + Documents are kept and indexed again on the next start. Stores with their own `prefix` change too. - `AgentMemory.recall` takes `mode="auto"|"keyword"`; records carry `retrieval`. A failed hybrid query falls back to keyword once. `min_score` applies to keyword results only. -- Automatic recall injects 3 memories in semantic mode (5 otherwise). +- Automatic recall injects 3 memories instead of 5. - `upstash_memory_search` results include `retrieval`. - Fix: an empty result from a retriever query no longer sends `SEARCH.COUNT` with that query (Search rejects `$ann` in COUNT); the missing-index check counts `{}` instead. diff --git a/python/hermes-memory/README.md b/python/hermes-memory/README.md index 774d124..41b6bf9 100644 --- a/python/hermes-memory/README.md +++ b/python/hermes-memory/README.md @@ -7,7 +7,7 @@ your AI SDK app saves is recalled by your Hermes agent, and the other way round. ## What it does in Hermes - **Recall:** before each turn, the memories closest to the user's message are added to the prompt. - By default by keyword; with [semantic recall](#semantic-recall) also by meaning. + By keyword and by meaning; see [Semantic recall](#semantic-recall). - **Chat history:** after each turn, the turn is appended in place to the session's searchable transcript document, so a write stays the size of one turn however long the session runs. - **Mirroring:** facts the built-in `memory` tool adds are also saved to Redis, and when Hermes @@ -40,35 +40,20 @@ these): | `UPSTASH_HERMES_USER_ID` | Whose memories these are. Defaults to the gateway user, then the Hermes profile name, then `default`. | | `UPSTASH_HERMES_NAMESPACE` | Key namespace. Default `agentkit`, so memories sit at the TS SDK's default keys and are shared with AgentKit apps on the same database. | | `UPSTASH_HERMES_TELEMETRY` | Set to `1` to send telemetry. Off by default; see [Telemetry](#telemetry). | -| `UPSTASH_HERMES_SEMANTIC` | Set to `1` to recall memories by meaning as well as by words. Off by default; see [Semantic recall](#semantic-recall). | ## Semantic recall Keyword recall (`$smart`: phrase, term and fuzzy matching) only finds a memory that shares words -with the message: "where is the assistant hosted" misses "Hermes runs on the ops Box". With -`UPSTASH_HERMES_SEMANTIC=1`, the memory `text` field is a `HYBRID` field, which Upstash Redis -Search embeds itself (no separate vector database), and recall merges a keyword query with a -semantic `$ann` query using `$rrf` +with the message: "where is the assistant hosted" misses "Hermes runs on the ops Box". So the +memory `text` field is a `HYBRID` field, which Upstash Redis Search embeds itself (no separate +vector database), and recall merges a keyword query with a semantic `$ann` query using `$rrf` ([Hybrid Search](https://upstash.com/docs/redis/search/hybrid-search)). -- A **new** index is created that way. An **existing** index keeps working with keyword recall - (a warning says so) until you migrate it. Stop every client of the namespace (Hermes gateway, - desktop app, cron jobs, other profiles on it), then: - - ```bash - # UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN set in the environment (the script reads no files) - python -m upstash_hermes_memory.migrate --to hybrid - ``` - - For a directory (catalog) install, run it from the plugin's `src` directory with a Python that - has `upstash-redis`. The script waits for pending indexing, saves a baseline (counts and a - sample of memory keys) at `:migrate:baseline`, drops and recreates the index over - the same keys (which it never touches), and checks the result against the baseline. If it is - interrupted, running the same command again resumes and checks. -- **Rollback:** `--to keyword` restores the 0.2.x index; do that before installing 0.2.x again, - which fails on a hybrid index. Turning the variable off is not enough: an existing hybrid index - keeps embedding new memories, it is only no longer queried by meaning. -- Automatic recall injects up to **3** memories instead of 5: the semantic side has no relevance +- **Upgrading from 0.2.x:** the old index has `text` as `TEXT` and is not supported. Recall fails + until you replace it: stop every client of the namespace (Hermes gateway, desktop app, cron jobs, + other profiles on it) and run `SEARCH.DROP agentkit` (or your namespace's index). Your memories + and chats are kept, and the next start creates the new index and indexes them again. +- Automatic recall injects up to **3** memories: the semantic side has no relevance threshold and returns up to its limit even for unrelated text, and in a small-store test its similarity scores did not separate related from unrelated memories well enough for a cutoff. `upstash_memory_search` keeps its limit. - Results carry `retrieval: "hybrid"` or `"keyword"`. Hybrid scores are rank-fusion values that @@ -119,5 +104,4 @@ there. One session saves two facts; a fresh session started right after must nam calling any tool (checked in Hermes's `state.db`), so they can only come from the automatic recall; and a control session as another user must not know them. The script then checks the keys and transcripts in Redis (and, if `packages/sdk` is built, that the TS `AgentMemory` recalls them too) -before cleaning up. Tests that rebuild indexes (`test_semantic_redis.py`) need a database of their -own: never point `UPSTASH_REDIS_REST_*` at a database with data you care about when running them. +before cleaning up. diff --git a/python/hermes-memory/src/upstash_hermes_memory/_search.py b/python/hermes-memory/src/upstash_hermes_memory/_search.py index 8e9015a..0b7442d 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/_search.py +++ b/python/hermes-memory/src/upstash_hermes_memory/_search.py @@ -1,8 +1,9 @@ """A Redis Search index that is created on first use, shared by AgentMemory and ChatHistory. A store with its own `prefix` sends the commands `ReactiveSearchIndex` sends from the TS SDK, -argument for argument, so such an index is the same from either language (unless `semantic` makes -`text` HYBRID, which the TS SDK does not do yet). The default shared index is Python-only. +argument for argument, except that memory `text` is HYBRID here (the TS SDK still creates it as +TEXT), so the two languages cannot share an index over the same name. The default shared index is +Python-only. """ import json @@ -31,41 +32,30 @@ def dumps(value: Any) -> str: # The one index AgentMemory and ChatHistory share by default (Python only: the TS SDK gives each # store its own index). Upstash's Free plan allows one search index per database. -# `kind` ("memory" or "chat") keeps the two apart in every query. +# `kind` ("memory" or "chat") keeps the two apart in every query. Only the memory `text` is HYBRID +# (full-text plus an embedding for `$ann`): chat transcripts stay TEXT, since a HYBRID field gets +# one embedding for its whole value and `userMessages` / `modelMessages` hold a whole session each. SHARED_SCHEMA: list[Field] = [ ("kind", "TEXT", True), ("userId", "TEXT", True), - ("text", "TEXT", False), + ("text", "HYBRID", False), ("sessionId", "TEXT", True), ("userMessages", "TEXT", False), ("modelMessages", "TEXT", False), ] -def with_hybrid_text(schema: Sequence[Field]) -> list[Field]: - """`schema` with the memory `text` field as HYBRID (full-text plus an embedding for `$ann`). - - Only `text` changes: chat transcripts stay TEXT, since a HYBRID field gets one embedding for - its whole value and `userMessages` / `modelMessages` hold a whole session each. - """ - return [(f, "HYBRID" if f == "text" else t, nt) for f, t, nt in schema] - - -def shared_index(redis: Any, namespace: str, *, semantic: bool = False) -> "SearchIndex": +def shared_index(redis: Any, namespace: str) -> "SearchIndex": """The shared index of a namespace, covering `:memory:` and `:chat:`. Both stores build this same definition, so whichever reads first creates it and the other's - EXISTSOK create is a no-op. `semantic` only decides what a new index is created with: an - existing index with `text` as either TEXT or HYBRID is used as it is (see `SearchIndex.ensure`), - and changing it is a maintenance step (`python -m upstash_hermes_memory.migrate`). + EXISTSOK create is a no-op. """ - hybrid = with_hybrid_text(SHARED_SCHEMA) return SearchIndex( redis, name=index_name_for(namespace), prefix=[f"{namespace}:memory:", f"{namespace}:chat:"], - schema=hybrid if semantic else SHARED_SCHEMA, - accepted=[SHARED_SCHEMA, hybrid], + schema=SHARED_SCHEMA, ) @@ -73,96 +63,17 @@ def _is_config_mismatch(error: Exception) -> bool: return "different configuration" in str(error) -def describe(redis: Any, name: str) -> dict[str, Any] | None: - """SEARCH.DESCRIBE as {"name", "type", "prefixes", "language", "schema"}, or None if missing. - - `name` is the index DESCRIBE resolved, which differs from the argument for an alias. - - Accepts the client's parsed `IndexDescription` as well as the raw reply, so it does not depend - on the client knowing every field type (upstash-redis 1.8 has no HYBRID enum member). - """ - raw = redis.execute(["SEARCH.DESCRIBE", name]) - if raw is None or raw == []: - return None - if isinstance(raw, list): - pairs = dict(zip(raw[0::2], raw[1::2])) - fields = [ - (f[0], str(f[1]).upper(), "NOTOKENIZE" in f[2:], "NOSTEM" in f[2:], "FROM" in f[2:]) - for f in pairs.get("schema") or [] - ] - found_name = pairs.get("name") - data_type, prefixes, language = ( - pairs.get("type"), - pairs.get("prefixes"), - pairs.get("language"), - ) - else: - fields = [ - ( - n, - str(getattr(i.type, "value", i.type)).upper(), - i.no_tokenize, - i.no_stem, - bool(i.alias), - ) - for n, i in raw.schema.items() - ] - found_name = raw.name - data_type, prefixes, language = raw.data_type, raw.prefixes, raw.language - return { - "name": found_name, - "type": str(getattr(data_type, "value", data_type) or "").upper(), - "prefixes": sorted(prefixes or []), - "language": str(getattr(language, "value", language) or "").lower(), - # NOSTEM and FROM are never set by this package, so their presence is a mismatch. - "schema": sorted(fields), - } - - -def matches( - description: dict[str, Any] | None, - prefixes: Sequence[str], - schema: Sequence[Field], - name: str | None = None, -) -> bool: - """Whether a `describe()` result is exactly the index this package would create (and, with - `name`, that index itself rather than an alias pointing at another one).""" - if description is None: - return False - return ( - (name is None or description["name"] == name) - and description["type"] == "JSON" - and description["prefixes"] == sorted(prefixes) - and description["language"] in ("", "english") - and description["schema"] == sorted((f, t, nt, False, False) for f, t, nt in schema) - ) - - class SearchIndex: def __init__( - self, - redis: Any, - *, - name: str, - prefix: str | Sequence[str], - schema: Sequence[Field], - accepted: Sequence[Sequence[Field]] = (), + self, redis: Any, *, name: str, prefix: str | Sequence[str], schema: Sequence[Field] ): self.redis = redis self.name = name self.prefixes = [prefix] if isinstance(prefix, str) else list(prefix) self.schema = list(schema) - # Other definitions an existing index may have and still be used as it is. - self.accepted = [list(s) for s in accepted if list(s) != self.schema] - # The definition the index actually has, known once `ensure()` has run. - self.actual_schema: list[Field] | None = None self._ready = False self._lock = threading.Lock() - def field_type(self, field: str) -> str | None: - """The type `field` has in the live index (after `ensure()`), e.g. "TEXT" or "HYBRID".""" - return next((t for f, t, _ in self.actual_schema or [] if f == field), None) - def create_command(self) -> list[str]: cmd = ["SEARCH.CREATE", self.name, "EXISTSOK", "ON", "JSON", "PREFIX"] cmd += [str(len(self.prefixes)), *self.prefixes] @@ -174,9 +85,8 @@ def create_command(self) -> list[str]: def ensure(self) -> None: """Create the index if it does not exist yet. A no-op after the first call. - An existing index whose definition differs only in a way this package knows (an `accepted` - schema, such as `text` TEXT vs HYBRID) is used as it is, never dropped: rebuilding an index - under running clients is unsafe, so it is left to `python -m upstash_hermes_memory.migrate`. + An index of this name with another definition (one from 0.2.x has `text` as TEXT) is never + dropped here, since other clients may be using it; the error says how to replace it. """ if self._ready: return @@ -185,21 +95,18 @@ def ensure(self) -> None: return try: self.redis.execute(self.create_command()) - actual = self.schema except Exception as error: - if not (self.accepted and _is_config_mismatch(error)): - raise - found = describe(self.redis, self.name) - actual = next( - (s for s in self.accepted if matches(found, self.prefixes, s, self.name)), None - ) - if actual is None: + if not _is_config_mismatch(error): raise + raise RuntimeError( + f"search index {self.name} exists with a different definition, for example " + f"from upstash-hermes-memory 0.2.x. Stop every client, run `SEARCH.DROP " + f"{self.name}` (documents are kept and indexed again) and start again." + ) from error # 1 once pending updates are searchable; 0 means the index is gone (dropped right # after the create): stay not-ready, so the next call creates it again. if self.redis.execute(["SEARCH.WAITINDEXING", self.name]) != 1: raise RuntimeError(f"search index {self.name} disappeared while it was created") - self.actual_schema = list(actual) self._ready = True def query(self, filter: dict[str, Any], limit: int | None = None) -> list[Any]: diff --git a/python/hermes-memory/src/upstash_hermes_memory/chat_history.py b/python/hermes-memory/src/upstash_hermes_memory/chat_history.py index a952bb4..dbd43fd 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/chat_history.py +++ b/python/hermes-memory/src/upstash_hermes_memory/chat_history.py @@ -104,19 +104,15 @@ def __init__( index_name: str | None = None, ttl_seconds: int | None = None, min_score: float = 0, - semantic: bool = False, enable_telemetry: bool = False, ): - """`semantic` only matters on the shared index: it decides whether a new index gets the - memory `text` field as HYBRID, so memory and chats create it the same way. Chat search - itself stays keyword search ($smart).""" self.redis = redis self.ttl_seconds = ttl_seconds self.min_score = min_score if prefix is None and index_name is None: self.prefix = f"{namespace}:chat" self.kind: str | None = "chat" - self.index = shared_index(redis, namespace, semantic=semantic) + self.index = shared_index(redis, namespace) else: self.prefix = prefix or DEFAULT_PREFIX self.kind = None diff --git a/python/hermes-memory/src/upstash_hermes_memory/memory.py b/python/hermes-memory/src/upstash_hermes_memory/memory.py index 679d217..3c383a2 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/memory.py +++ b/python/hermes-memory/src/upstash_hermes_memory/memory.py @@ -24,21 +24,14 @@ import httpx from upstash_redis.errors import UpstashError -from ._search import ( - DEFAULT_NAMESPACE, - SearchIndex, - dumps, - index_name_for, - shared_index, - with_hybrid_text, -) +from ._search import DEFAULT_NAMESPACE, SearchIndex, dumps, index_name_for, shared_index from ._telemetry import add_telemetry logger = logging.getLogger(__name__) DEFAULT_PREFIX = f"{DEFAULT_NAMESPACE}:memory" -# How often a failing semantic query or an index without embeddings is logged, per store. +# How often a failing hybrid query is logged, per store. _WARN_EVERY_S = 600.0 RecallMode = Literal["auto", "keyword"] @@ -86,32 +79,23 @@ def __init__( prefix: str | None = None, index_name: str | None = None, min_score: float = 0, - semantic: bool = False, enable_telemetry: bool = False, ): - """`semantic=True` recalls with hybrid search (full-text plus meaning) when the index has - `text` as a HYBRID field, and creates a new index that way. An existing TEXT index keeps - working with keyword recall until it is migrated (`python -m upstash_hermes_memory.migrate`). - `min_score` applies to keyword results only; hybrid scores are not on the same scale. - """ + """`min_score` applies to keyword results only; hybrid scores are not on the same scale.""" self.redis = redis self.min_score = min_score - self.semantic = semantic self.shared = prefix is None and index_name is None self._warned_at: dict[str, float] = {} if self.shared: self.prefix = f"{namespace}:memory" - self.index = shared_index(redis, namespace, semantic=semantic) + self.index = shared_index(redis, namespace) else: self.prefix = prefix or DEFAULT_PREFIX - text_schema = [("text", "TEXT", False), ("userId", "TEXT", True)] - hybrid_schema = with_hybrid_text(text_schema) self.index = SearchIndex( redis, name=index_name or index_name_for(self.prefix), prefix=f"{self.prefix}:", - schema=hybrid_schema if semantic else text_schema, - accepted=[text_schema, hybrid_schema], + schema=[("text", "HYBRID", False), ("userId", "TEXT", True)], ) add_telemetry(redis, enabled=enable_telemetry) @@ -154,10 +138,10 @@ def recall( ) -> list[MemoryRecord]: """The `top_k` memories closest to `query`. - `mode="auto"` uses hybrid search ($rrf of $smart and $ann) when this store is `semantic` - and the index has embeddings, else keyword search ($smart: full-text plus fuzzy matching). - A failed hybrid query falls back to keyword search once. `mode="keyword"` always uses - keyword search, for callers that need COUNT and QUERY to describe the same set. + `mode="auto"` uses hybrid search ($rrf of $smart and $ann). A failed hybrid query falls + back to keyword search ($smart: full-text plus fuzzy matching) once. `mode="keyword"` + always uses keyword search, for callers that need COUNT and QUERY to describe the same + set. """ if mode not in ("auto", "keyword"): raise ValueError(f"mode must be 'auto' or 'keyword' (got {mode!r})") @@ -166,7 +150,7 @@ def recall( if not (query and query.strip()): rows = self._memories(self.index.query(base, limit=top_k), user_id) return [self._record(row, user_id) for row in rows] - if mode == "auto" and self.semantic and self._hybrid_ready(): + if mode == "auto": hybrid = { "$rrf": { "retrievers": [ @@ -192,29 +176,9 @@ def recall( rows = self._memories( self.index.query({**base, "text": {"$smart": query}}, limit=top_k), user_id ) - if not self.semantic and self.index.field_type("text") == "HYBRID": - self._warn( - "embeddings-unused", - "upstash memory: index %s has embeddings but semantic recall is off; new memories " - "are still embedded (migrate --to keyword to stop that)", - self.index.name, - ) records = [self._record(row, user_id, "keyword") for row in rows] return [r for r in records if (r.score or 0) >= self.min_score] - def _hybrid_ready(self) -> bool: - """Whether the live index can answer `$ann` on `text` (it was created or migrated so).""" - self.index.ensure() - if self.index.field_type("text") == "HYBRID": - return True - self._warn( - "no-embeddings", - "upstash memory: semantic recall is on but index %s has no embeddings yet; using " - "keyword recall until it is migrated (python -m upstash_hermes_memory.migrate)", - self.index.name, - ) - return False - def _warn(self, what: str, message: str, *args: Any) -> None: now = time.monotonic() if now - self._warned_at.get(what, -_WARN_EVERY_S) >= _WARN_EVERY_S: diff --git a/python/hermes-memory/src/upstash_hermes_memory/migrate.py b/python/hermes-memory/src/upstash_hermes_memory/migrate.py deleted file mode 100644 index 47abdcc..0000000 --- a/python/hermes-memory/src/upstash_hermes_memory/migrate.py +++ /dev/null @@ -1,289 +0,0 @@ -"""Switch a namespace's shared search index between keyword and hybrid (semantic) recall. - - python -m upstash_hermes_memory.migrate --to hybrid # add embeddings to memory `text` - python -m upstash_hermes_memory.migrate --to keyword # back to the 0.2.x index (rollback) - -Stop every client of the namespace first (the Hermes gateway, the desktop app, cron jobs, other -profiles on the same namespace): the index is dropped and created again, so searches fail in -between, and a client that recreates the index during the gap would race this script. Documents -are never touched; only the index is rebuilt over the same keys. - -Before dropping anything the script waits for pending indexing, then saves what the index holds -(memory and chat counts and a sample of memory keys) at `:migrate:baseline`. After the -rebuild it checks the new index against that baseline and deletes it. If a run is interrupted, -running the same command again resumes: it finishes the rebuild and checks the saved baseline. - -Credentials come from UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN in the environment (and the -namespace from UPSTASH_HERMES_NAMESPACE or --namespace); the script reads no files. -""" - -import argparse -import json -import os -import sys -import time -from typing import Any - -from ._search import ( - DEFAULT_NAMESPACE, - SHARED_SCHEMA, - SearchIndex, - describe, - dumps, - index_name_for, - matches, - with_hybrid_text, -) - -_CHATS = {"kind": {"$eq": "chat"}} -_MEMORIES = {"$mustNot": [{"kind": {"$eq": "chat"}}]} -_SAMPLE = 200 # memory keys checked one by one after the rebuild -_PAGE = 1000 # SEARCH.QUERY's largest LIMIT -_MAX_OFFSET = 10_000 # and its largest OFFSET - - -class MigrationError(Exception): - pass - - -def _counts(redis: Any, name: str) -> tuple[int, int]: - """(memories, chats) the index holds; -1 each when the index does not exist.""" - - def count(filter: dict[str, Any]) -> int: - result = redis.execute(["SEARCH.COUNT", name, dumps(filter)]) - return int(getattr(result, "count", result)) - - return count(_MEMORIES), count(_CHATS) - - -def _memory_keys(redis: Any, name: str, limit: int, offset: int = 0) -> list[str]: - cmd = ["SEARCH.QUERY", name, dumps(_MEMORIES), "LIMIT", str(limit), "NOCONTENT"] - if offset: - cmd += ["OFFSET", str(offset)] - rows = redis.execute(cmd) or [] - return [getattr(row, "key", None) or row[0] for row in rows] - - -def _state(redis: Any, name: str, prefixes: list[str], schemas: dict[str, list]) -> str: - """ "absent", the matching schema's name, "other", or "unknown" (DESCRIBE failed).""" - try: - found = describe(redis, name) - except Exception: # noqa: BLE001 - reported, never acted on - return "unknown" - if found is None: - return "absent" - return next((k for k, s in schemas.items() if matches(found, prefixes, s, name)), "other") - - -def _wait(redis: Any, name: str) -> None: - # 1 once every pending update is searchable; 0 means the index does not exist. - if redis.execute(["SEARCH.WAITINDEXING", name]) != 1: - raise MigrationError(f"index {name} was not found while waiting for indexing") - - -def migrate( - redis: Any, - *, - namespace: str, - to: str, - confirm: Any = lambda plan: True, - log: Any = print, - attempts: int = 3, -) -> str: - """Rebuild the shared index of `namespace` as `to` ("hybrid" or "keyword"). - - Returns "unchanged", "created", "migrated" or "resumed". Raises MigrationError, without - dropping anything, when the live index is not one this package created (or is an alias), and - after a failed step, saying what state the index is in. - """ - if to not in ("hybrid", "keyword"): - raise ValueError("to must be 'hybrid' or 'keyword'") - came_from = "keyword" if to == "hybrid" else "hybrid" - schemas = {"keyword": list(SHARED_SCHEMA), "hybrid": with_hybrid_text(SHARED_SCHEMA)} - name = index_name_for(namespace) - prefixes = [f"{namespace}:memory:", f"{namespace}:chat:"] - index = SearchIndex(redis, name=name, prefix=prefixes, schema=schemas[to]) - baseline_key = f"{namespace}:migrate:baseline" - raw = redis.execute(["GET", baseline_key]) - baseline = json.loads(raw) if raw else None - if baseline and to not in (baseline.get("to"), baseline.get("from")): - raise MigrationError(f"a migration is pending ({baseline_key}): {raw}") - if baseline and baseline["to"] != to: - # Rolling an interrupted migration back: same baseline, the other target. - log(f"Rolling the interrupted migration to {baseline['to']} back to {to}.") - baseline = {**baseline, "from": baseline["to"], "to": to} - redis.execute(["SET", baseline_key, dumps(baseline)]) - - state = _state(redis, name, prefixes, schemas) - if state == "unknown": - raise MigrationError(f"index {name} could not be read; nothing changed") - if state == "other": - raise MigrationError( - f"index {name} is not one this package created (or is an alias); refusing to drop it" - ) - - if state == to: - _wait(redis, name) - if baseline: # a previous run rebuilt the index but did not finish checking it - _verify(redis, name, baseline, to) - redis.execute(["DEL", baseline_key]) - log(f"Index {name} is {to}; the interrupted migration is checked and complete.") - return "resumed" - memories, chats = _counts(redis, name) - log(f"Index {name} is already {to} ({memories} memories, {chats} chats); nothing to do.") - return "unchanged" - - if state == "absent" and not baseline: - if not confirm(f"Index {name} does not exist. Create it as {to}?"): - raise MigrationError("cancelled") - _create(redis, index, prefixes, schemas[to], attempts, log) - log(f"Created index {name} ({to}).") - return "created" - - if state == came_from: - _wait(redis, name) # pending updates first, or the baseline undercounts - memories, chats = _counts(redis, name) - plan = ( - f"Rebuild index {name} from {came_from} to {to}: {memories} memories and {chats} " - f"chats will be indexed again{' and embedded' if to == 'hybrid' else ''}. Documents " - "are not touched. Every client of this namespace must be stopped. Continue?" - ) - if not confirm(plan): - raise MigrationError("cancelled") - fresh = baseline is None # a rollback keeps the baseline of the original index - if fresh: - baseline = { - "from": came_from, - "to": to, - "memories": memories, - "chats": chats, - "sample": sorted(_memory_keys(redis, name, _SAMPLE)), - "at": int(time.time()), - } - redis.execute(["SET", baseline_key, dumps(baseline)]) - try: - redis.execute(["SEARCH.DROP", name]) - except Exception as error: # noqa: BLE001 - reconciled below - log(f"Drop failed: {error}") - state = _state(redis, name, prefixes, schemas) - if state == came_from: - # Only a baseline taken by this run goes: one from an interrupted migration still - # describes the documents that rebuild has to account for. - if fresh: - redis.execute(["DEL", baseline_key]) - raise MigrationError(f"index {name} could not be dropped; nothing changed") - if state != "absent": - raise MigrationError(f"index {name} is {state} after the drop; re-run to resume") - # Absent with a baseline: just dropped, or an interrupted run stopped between drop and create. - assert baseline is not None - - try: - _create(redis, index, prefixes, schemas[to], attempts, log) - except MigrationError as error: - now = _state(redis, name, prefixes, schemas) - raise MigrationError( - f"{error}. The index is {now}; documents are intact. Re-run this command (--to {to}) " - f"to resume, or restore the previous index with --to {came_from}." - ) from error - _verify(redis, name, baseline, to) - redis.execute(["DEL", baseline_key]) - log(f"Index {name} is {to}: {baseline['memories']} memories and {baseline['chats']} chats.") - return "migrated" - - -def _verify(redis: Any, name: str, baseline: dict[str, Any], to: str) -> None: - after = _counts(redis, name) - if after != (baseline["memories"], baseline["chats"]): - raise MigrationError( - f"index {name} is {to} now but holds {after[0]} memories and {after[1]} chats " - f"(before: {baseline['memories']} and {baseline['chats']}). Was a client still " - "writing? Check, then re-run to verify again." - ) - missing = set(baseline.get("sample") or []) - offset, scanned = 0, 0 - while missing and offset <= _MAX_OFFSET: - page = _memory_keys(redis, name, _PAGE, offset) - missing -= set(page) - scanned += len(page) - if len(page) < _PAGE: - break - offset += _PAGE - # Every memory was listed (the index holds after[0]) or the last page came back short. - complete = scanned >= after[0] or offset <= _MAX_OFFSET - if missing and complete: - raise MigrationError( - f"index {name} lost {len(missing)} sampled memories, e.g. {min(missing)}" - ) - # Past OFFSET's limit the remaining sample can't be listed; the counts above still matched. - - -def _create( - redis: Any, index: SearchIndex, prefixes: list[str], target: list, attempts: int, log: Any -) -> None: - """CREATE then WAITINDEXING, retried; reconciles with DESCRIBE after any failed step.""" - last: Exception | None = None - for attempt in range(1, attempts + 1): - try: - redis.execute(index.create_command()) - except Exception as error: # noqa: BLE001 - a timeout may still have created it - last = error - log(f"Create attempt {attempt} failed: {error}") - try: - found = describe(redis, index.name) - if matches(found, prefixes, target, index.name): - _wait(redis, index.name) - return - if found is not None: - raise MigrationError(f"index {index.name} exists with another definition: {found}") - except MigrationError: - raise - except Exception as error: # noqa: BLE001 - state unknown: retry the whole step - last = error - log(f"Checking the index failed: {error}") - time.sleep(min(2**attempt, 10)) - raise MigrationError(f"could not create index {index.name}: {last}") - - -def main(argv: list[str] | None = None) -> int: - parser = argparse.ArgumentParser( - prog="python -m upstash_hermes_memory.migrate", - description="Switch the shared memory/chat index between keyword and hybrid recall.", - ) - parser.add_argument("--to", required=True, choices=["hybrid", "keyword"]) - parser.add_argument("--namespace", help=f"key namespace (default: {DEFAULT_NAMESPACE})") - parser.add_argument("--yes", action="store_true", help="do not ask for confirmation") - args = parser.parse_args(argv) - - url = os.environ.get("UPSTASH_REDIS_REST_URL") - token = os.environ.get("UPSTASH_REDIS_REST_TOKEN") - if not (url and token): - print("UPSTASH_REDIS_REST_URL and UPSTASH_REDIS_REST_TOKEN are required.", file=sys.stderr) - return 2 - namespace = args.namespace or os.environ.get("UPSTASH_HERMES_NAMESPACE") or DEFAULT_NAMESPACE - - from upstash_redis import Redis - - redis = Redis(url=url, token=token, allow_telemetry=False) - - def confirm(plan: str) -> bool: - print(plan) - if args.yes: - return True - if not sys.stdin.isatty(): - print("Not a terminal: pass --yes to confirm.", file=sys.stderr) - return False - return input("Type yes to continue: ").strip().lower() == "yes" - - try: - migrate(redis, namespace=namespace, to=args.to, confirm=confirm) - except MigrationError as error: - print(f"Migration stopped: {error}", file=sys.stderr) - return 1 - except Exception as error: # noqa: BLE001 - a clean message instead of a traceback - print(f"Migration failed: {type(error).__name__}: {error}", file=sys.stderr) - return 1 - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/python/hermes-memory/src/upstash_hermes_memory/provider.py b/python/hermes-memory/src/upstash_hermes_memory/provider.py index bd4b906..8d3e058 100644 --- a/python/hermes-memory/src/upstash_hermes_memory/provider.py +++ b/python/hermes-memory/src/upstash_hermes_memory/provider.py @@ -20,10 +20,6 @@ works on Upstash's Free plan (one index per database). - UPSTASH_HERMES_TELEMETRY: set to `1` to send telemetry (off by default, as the plugin catalog requires). `UPSTASH_DISABLE_TELEMETRY` still turns it off. -- UPSTASH_HERMES_SEMANTIC: set to `1` to recall memories by meaning as well as by words (hybrid - search with Upstash Redis Search's managed embeddings). Off by default. A new index is created - with embeddings; an existing one keeps keyword recall until it is migrated with - `python -m upstash_hermes_memory.migrate --to hybrid` (Hermes stopped). """ import hashlib @@ -36,8 +32,6 @@ from ._search import DEFAULT_NAMESPACE from ._telemetry import OPT_IN_ENV, OPT_OUT_ENV, is_opt_in - -SEMANTIC_ENV = "UPSTASH_HERMES_SEMANTIC" from .chat_history import ChatHistory from .memory import AgentMemory, MemoryRecord @@ -58,11 +52,10 @@ def get_secret(name: str, default: str | None = None) -> str | None: # Hermes only lets the primary agent write; subagents, cron jobs and flushes read. _WRITING_CONTEXTS = {"", "primary"} -_PREFETCH_TOP_K = 5 # Hybrid search always returns its `LIMIT` (the $ann side has no relevance threshold, and its # similarity scores do not separate related from unrelated memories well enough for a cutoff), so -# automatic recall injects fewer memories when it is on. The search tool's own limit is unchanged. -_PREFETCH_TOP_K_SEMANTIC = 3 +# automatic recall injects only a few. The search tool has its own limit. +_PREFETCH_TOP_K = 3 _EXCERPT_CHARS = 300 # Built-in memory tool actions Hermes forwards (a patch arrives as a replace). _MIRRORED_ACTIONS = {"add", "replace", "remove"} @@ -315,15 +308,6 @@ def get_config_schema(self) -> list[dict[str, Any]]: "required": False, "env_var": "UPSTASH_HERMES_USER_ID", }, - { - "key": "semantic", - "description": ( - "Recall memories by meaning as well as by words (1 to enable; an existing " - "index needs `python -m upstash_hermes_memory.migrate --to hybrid`)" - ), - "required": False, - "env_var": SEMANTIC_ENV, - }, ] def save_config(self, values: dict[str, Any], hermes_home: str) -> None: @@ -360,15 +344,8 @@ def initialize(self, session_id: str, **kwargs: Any) -> None: # Like the credentials, the namespace is read per Hermes profile: a multiplexed gateway # keeps each profile's .env out of os.environ. Memory and chats share its one index. namespace = get_secret("UPSTASH_HERMES_NAMESPACE", "") or DEFAULT_NAMESPACE - # Both stores get the same flag, so whichever creates the shared index creates it the same - # way. It is read per profile like the namespace, so profiles can differ. - semantic = is_opt_in(get_secret(SEMANTIC_ENV, "")) - self._memory = AgentMemory( - self._redis, namespace=namespace, semantic=semantic, enable_telemetry=telemetry - ) - self._chats = ChatHistory( - self._redis, namespace=namespace, semantic=semantic, enable_telemetry=telemetry - ) + self._memory = AgentMemory(self._redis, namespace=namespace, enable_telemetry=telemetry) + self._chats = ChatHistory(self._redis, namespace=namespace, enable_telemetry=telemetry) self._user_id = user_id self._session_id = self._session_key(session_id) self._write_enabled = (kwargs.get("agent_context") or "") in _WRITING_CONTEXTS @@ -391,8 +368,7 @@ def prefetch(self, query: str, *, session_id: str = "") -> str: if not self._memory or not query.strip(): return "" try: - top_k = _PREFETCH_TOP_K_SEMANTIC if self._memory.semantic else _PREFETCH_TOP_K - hits = self._memory.recall(query, user_id=self._user_id, top_k=top_k) + hits = self._memory.recall(query, user_id=self._user_id, top_k=_PREFETCH_TOP_K) except Exception as error: # noqa: BLE001 - a Redis outage must never break a turn logger.warning("upstash memory recall failed: %s", error) return "" diff --git a/python/hermes-memory/tests/test_layout.py b/python/hermes-memory/tests/test_layout.py index 6ee5e30..051b461 100644 --- a/python/hermes-memory/tests/test_layout.py +++ b/python/hermes-memory/tests/test_layout.py @@ -28,7 +28,7 @@ def test_index_names_follow_the_ts_default(): SHARED_CREATE = ( "SEARCH.CREATE agentkit EXISTSOK ON JSON PREFIX 2 agentkit:memory: agentkit:chat: SCHEMA " - "kind TEXT NOTOKENIZE userId TEXT NOTOKENIZE text TEXT sessionId TEXT NOTOKENIZE " + "kind TEXT NOTOKENIZE userId TEXT NOTOKENIZE text HYBRID sessionId TEXT NOTOKENIZE " "userMessages TEXT modelMessages TEXT" ) @@ -45,7 +45,7 @@ def test_a_prefix_gives_a_store_its_own_index_like_the_ts_sdk(): memory = AgentMemory(Recorder(), prefix="agentkit:memory", enable_telemetry=False) assert " ".join(memory.index.create_command()) == ( "SEARCH.CREATE agentkit_memory EXISTSOK ON JSON PREFIX 1 agentkit:memory: " - "SCHEMA text TEXT userId TEXT NOTOKENIZE" + "SCHEMA text HYBRID userId TEXT NOTOKENIZE" ) chats = ChatHistory(Recorder(), prefix="agentkit:chat", enable_telemetry=False) assert " ".join(chats.index.create_command()) == ( diff --git a/python/hermes-memory/tests/test_redis.py b/python/hermes-memory/tests/test_redis.py index 327b243..9924bf0 100644 --- a/python/hermes-memory/tests/test_redis.py +++ b/python/hermes-memory/tests/test_redis.py @@ -41,7 +41,8 @@ def test_memory_lifecycle(memory): memory.add("Someone else likes coffee too", user_id="u2") memory.index.wait_indexing() - hits = memory.recall("coffee", user_id="u1") + # Keyword mode: hybrid search has no relevance threshold, so it would add the unrelated memory. + hits = memory.recall("coffee", user_id="u1", mode="keyword") assert [h.id for h in hits] == ["coffee"] assert hits[0].text == "Arda prefers dark roast coffee" assert hits[0].score > 0 @@ -55,7 +56,7 @@ def test_memory_lifecycle(memory): assert memory.get("coffee", user_id="u1") is None memory.index.wait_indexing() assert memory.count(user_id="u1") == 1 - assert memory.recall("coffee", user_id="u1") == [] + assert memory.recall("coffee", user_id="u1", mode="keyword") == [] def test_memory_reads_documents_written_by_the_ts_sdk(redis, memory): diff --git a/python/hermes-memory/tests/test_semantic.py b/python/hermes-memory/tests/test_semantic.py index 12e118a..40d24be 100644 --- a/python/hermes-memory/tests/test_semantic.py +++ b/python/hermes-memory/tests/test_semantic.py @@ -1,7 +1,6 @@ -"""Semantic (hybrid) recall and the index migration, against a scripted fake Redis.""" +"""Semantic (hybrid) recall, against a scripted fake Redis.""" import json -import logging from dataclasses import dataclass from typing import Any @@ -9,22 +8,13 @@ import pytest from upstash_hermes_memory import AgentMemory, ChatHistory, UpstashMemoryProvider from upstash_hermes_memory import provider as provider_module -from upstash_hermes_memory._search import ( - SHARED_SCHEMA, - describe, - matches, - shared_index, - with_hybrid_text, -) -from upstash_hermes_memory.migrate import MigrationError, migrate +from upstash_hermes_memory._search import SHARED_SCHEMA, shared_index from upstash_redis.errors import UpstashError -from upstash_redis.search import deserialize_describe_response MISMATCH = "ERR Index agentkit already exists with a different configuration" -HYBRID = with_hybrid_text(SHARED_SCHEMA) +HYBRID = SHARED_SCHEMA +OLD_TEXT_INDEX = [(f, "TEXT", nt) for f, _, nt in SHARED_SCHEMA] USER = {"userId": {"$eq": "u1"}, "$mustNot": [{"kind": {"$eq": "chat"}}]} -PREFIXES = ["agentkit:memory:", "agentkit:chat:"] -quiet = print # migration log lines show up in pytest output on failure only @dataclass @@ -38,30 +28,19 @@ def memory_row(id, text="x", score=1.0): return Row(f"agentkit:memory:u1:{id}", score, {"text": text, "userId": "u1", "createdAt": 1}) -def describe_reply(schema, name="agentkit", prefixes=("agentkit:chat:", "agentkit:memory:")): - fields = [[f, t] + (["NOTOKENIZE"] if nt else []) for f, t, nt in schema] - return ["name", name, "type", "JSON", "prefixes", list(prefixes), "language", "english", - "schema", fields] # fmt: skip - - class FakeRedis: """A small model of Upstash Redis Search, enough for the index lifecycle and recall. - Memory rows and a chat count are the index contents; `pending` memories become searchable at - the next WAITINDEXING, as asynchronous indexing does. Every command is recorded. + Memory rows and a chat count are the index contents. Every command is recorded. """ - def __init__(self, index_schema=None, memories=(), chats=0, *, parsed=False, alias_of=None): + def __init__(self, index_schema=None, memories=(), chats=0): self.index_schema = list(index_schema) if index_schema else None self.memories = list(memories) self.chats = chats - self.pending: list[Row] = [] - self.parsed = parsed # answer DESCRIBE like the client does (an IndexDescription) - self.alias_of = alias_of # DESCRIBE resolves the name to this index self.hybrid_error: Exception | None = None self.keyword_error: Exception | None = None self.wait_result: int | None = None - self.kv: dict[str, str] = {} self.commands: list[list[Any]] = [] self._headers: dict[str, str] = {} @@ -74,19 +53,12 @@ def execute(self, command): raise UpstashError(MISMATCH) self.index_schema = schema return 1 - if op == "SEARCH.DESCRIBE": - if not self.index_schema: - return None - reply = describe_reply(self.index_schema, name=self.alias_of or command[1]) - return deserialize_describe_response(reply) if self.parsed else reply if op == "SEARCH.DROP": dropped, self.index_schema = self.index_schema is not None, None return int(dropped) if op == "SEARCH.WAITINDEXING": if self.wait_result is not None: return self.wait_result - self.memories += self.pending - self.pending = [] return 1 if self.index_schema else 0 if op == "SEARCH.QUERY": if self.index_schema is None: @@ -96,10 +68,7 @@ def execute(self, command): if error: raise error limit = int(command[command.index("LIMIT") + 1]) if "LIMIT" in command else 10 - offset = int(command[command.index("OFFSET") + 1]) if "OFFSET" in command else 0 - if not 1 <= limit <= 1000 or not 0 <= offset <= 10_000: # the documented bounds - raise UpstashError("ERR LIMIT must be between 1 and 1000") - return self.memories[offset : offset + limit] + return self.memories[:limit] if op == "SEARCH.COUNT": if self.index_schema is None: return -1 @@ -111,13 +80,6 @@ def execute(self, command): if filter == {}: return len(self.memories) + self.chats return len(self.memories) - if op == "GET": - return self.kv.get(command[1]) - if op == "SET": - self.kv[command[1]] = command[2] - return "OK" - if op == "DEL": - return int(self.kv.pop(command[1], None) is not None) return 1 def delete(self, key): @@ -145,38 +107,11 @@ def _schema_of(command): return out -@pytest.fixture(autouse=True) -def no_sleep(monkeypatch): - monkeypatch.setattr("upstash_hermes_memory.migrate.time.sleep", lambda s: None) - - -# -- describe ------------------------------------------------------------------------------------ - - -@pytest.mark.parametrize("parsed", [False, True]) -def test_describe_reads_raw_and_client_parsed_replies_alike(parsed): - redis = FakeRedis(HYBRID, parsed=parsed) - found = describe(redis, "agentkit") - assert matches(found, PREFIXES, HYBRID, "agentkit") - assert not matches(found, PREFIXES, SHARED_SCHEMA, "agentkit") - - -def test_describe_sees_options_this_package_never_sets(): - reply = describe_reply(SHARED_SCHEMA) - reply[-1][2] = ["text", "TEXT", "NOSTEM"] - - class Raw(FakeRedis): - def execute(self, command): - return reply - - assert not matches(describe(Raw(), "agentkit"), PREFIXES, SHARED_SCHEMA) - - # -- index creation ------------------------------------------------------------------------------ -def test_semantic_creates_text_as_the_only_hybrid_field(): - command = shared_index(FakeRedis(), "agentkit", semantic=True).create_command() +def test_text_is_the_only_hybrid_field(): + command = shared_index(FakeRedis(), "agentkit").create_command() assert " ".join(command).endswith( "SCHEMA kind TEXT NOTOKENIZE userId TEXT NOTOKENIZE text HYBRID sessionId TEXT NOTOKENIZE " "userMessages TEXT modelMessages TEXT" @@ -185,39 +120,21 @@ def test_semantic_creates_text_as_the_only_hybrid_field(): def test_memory_and_chats_create_the_index_the_same_way(): redis = FakeRedis() - assert ( - AgentMemory(redis, semantic=True).index.create_command() - == ChatHistory(redis, semantic=True).index.create_command() - ) + assert AgentMemory(redis).index.create_command() == ChatHistory(redis).index.create_command() -@pytest.mark.parametrize("semantic, existing", [(True, SHARED_SCHEMA), (False, HYBRID)]) -@pytest.mark.parametrize("parsed", [False, True]) -def test_an_existing_index_of_the_other_kind_is_used_never_dropped(semantic, existing, parsed): - redis = FakeRedis(existing, parsed=parsed) - memory = AgentMemory(redis, semantic=semantic) - memory.index.ensure() - assert memory.index.actual_schema == list(existing) - assert "SEARCH.DROP" not in redis.ops() - - -def test_an_unknown_index_definition_still_fails(): - redis = FakeRedis(SHARED_SCHEMA + [("extra", "TEXT", False)]) - with pytest.raises(UpstashError): - AgentMemory(redis, semantic=True).index.ensure() +def test_a_0_2_text_index_is_refused_with_the_fix_and_never_dropped(): + redis = FakeRedis(OLD_TEXT_INDEX) + with pytest.raises(RuntimeError, match="SEARCH.DROP agentkit"): + AgentMemory(redis).recall("x", user_id="u1") assert "SEARCH.DROP" not in redis.ops() - - -def test_an_alias_to_another_index_is_not_accepted(): - redis = FakeRedis(SHARED_SCHEMA, alias_of="someone_else") - with pytest.raises(UpstashError): - AgentMemory(redis, semantic=True).index.ensure() + assert redis.queries() == [] # no pointless keyword fallback against the same refusal def test_an_index_that_vanishes_during_create_is_not_marked_ready(): redis = FakeRedis() redis.wait_result = 0 - memory = AgentMemory(redis, semantic=True) + memory = AgentMemory(redis) with pytest.raises(RuntimeError): memory.index.ensure() assert not memory.index._ready @@ -228,7 +145,7 @@ def test_an_index_that_vanishes_during_create_is_not_marked_ready(): def test_hybrid_recall_fuses_keyword_and_meaning_for_this_user_only(): redis = FakeRedis(HYBRID, [memory_row("1", "Weeks start Monday", 0.03)]) - memory = AgentMemory(redis, semantic=True, min_score=5) + memory = AgentMemory(redis, min_score=5) hits = memory.recall("what day does the week begin", user_id="u1", top_k=3) assert [(h.text, h.retrieval) for h in hits] == [("Weeks start Monday", "hybrid")] (query,) = redis.queries() @@ -239,23 +156,6 @@ def test_hybrid_recall_fuses_keyword_and_meaning_for_this_user_only(): } -def test_semantic_on_a_keyword_index_recalls_by_keyword_and_says_so_once(caplog): - redis = FakeRedis(SHARED_SCHEMA, [memory_row("1")]) - memory = AgentMemory(redis, semantic=True) - with caplog.at_level(logging.WARNING): - memory.recall("a", user_id="u1") - memory.recall("b", user_id="u1") - assert all("$rrf" not in q for q in redis.queries()) - assert len(caplog.records) == 1 and "migrate" in caplog.records[0].message - - -def test_semantic_off_on_a_hybrid_index_says_embeddings_go_on(caplog): - redis = FakeRedis(HYBRID, [memory_row("1")]) - with caplog.at_level(logging.WARNING): - AgentMemory(redis).recall("a", user_id="u1") - assert "still embedded" in caplog.records[0].message - - @pytest.mark.parametrize( "error", [ @@ -267,7 +167,7 @@ def test_semantic_off_on_a_hybrid_index_says_embeddings_go_on(caplog): def test_a_failed_hybrid_query_falls_back_to_keyword_once_with_min_score(error): redis = FakeRedis(HYBRID, [memory_row("1", "strong", 3.0), memory_row("2", "weak", 0.2)]) redis.hybrid_error = error - memory = AgentMemory(redis, semantic=True, min_score=1) + memory = AgentMemory(redis, min_score=1) hits = memory.recall("billing for big accounts", user_id="u1") assert [(h.text, h.retrieval) for h in hits] == [("strong", "keyword")] assert ["$rrf" in q for q in redis.queries()] == [True, False] @@ -278,7 +178,7 @@ def test_a_failed_fallback_raises_for_the_caller_to_handle(): redis = FakeRedis(HYBRID, [memory_row("1")]) redis.hybrid_error = redis.keyword_error = httpx.ConnectError("down") with pytest.raises(httpx.ConnectError): - AgentMemory(redis, semantic=True).recall("x", user_id="u1") + AgentMemory(redis).recall("x", user_id="u1") assert len(redis.queries()) == 2 # one hybrid attempt, one keyword attempt @@ -286,39 +186,32 @@ def test_programming_errors_are_not_swallowed_by_the_fallback(): redis = FakeRedis(HYBRID, [memory_row("1")]) redis.hybrid_error = TypeError("bug") with pytest.raises(TypeError): - AgentMemory(redis, semantic=True).recall("x", user_id="u1") + AgentMemory(redis).recall("x", user_id="u1") def test_keyword_mode_never_uses_a_retriever(): redis = FakeRedis(HYBRID, [memory_row("1")]) - AgentMemory(redis, semantic=True).recall("x", user_id="u1", mode="keyword") + AgentMemory(redis).recall("x", user_id="u1", mode="keyword") assert all("$rrf" not in q for q in redis.queries()) def test_an_unknown_mode_is_refused(): with pytest.raises(ValueError): - AgentMemory(FakeRedis(), semantic=True).recall("x", user_id="u1", mode="vector") + AgentMemory(FakeRedis()).recall("x", user_id="u1", mode="vector") def test_an_empty_hybrid_result_checks_the_index_without_counting_a_retriever(): redis = FakeRedis(HYBRID) - assert AgentMemory(redis, semantic=True).recall("nothing", user_id="u1") == [] + assert AgentMemory(redis).recall("nothing", user_id="u1") == [] assert [c[2] for c in redis.commands if c[0] == "SEARCH.COUNT"] == ["{}"] -def test_semantic_off_keeps_the_0_2_commands(): - redis = FakeRedis(SHARED_SCHEMA, [memory_row("1", "x", 2.0)]) - hits = AgentMemory(redis).recall("x", user_id="u1") - assert redis.queries() == [{**USER, "text": {"$smart": "x"}}] - assert hits[0].retrieval == "keyword" - - # -- provider ------------------------------------------------------------------------------------ @pytest.fixture -def semantic_provider(monkeypatch): - env = {"UPSTASH_HERMES_SEMANTIC": "1", "UPSTASH_HERMES_USER_ID": "u1"} +def hybrid_provider(monkeypatch): + env = {"UPSTASH_HERMES_USER_ID": "u1"} monkeypatch.setattr(provider_module, "get_secret", lambda n, d=None: env.get(n, d)) redis = FakeRedis(HYBRID, [memory_row(str(i), f"m{i}") for i in range(5)]) provider = UpstashMemoryProvider(redis) @@ -326,269 +219,35 @@ def semantic_provider(monkeypatch): return provider, redis -def test_the_env_flag_turns_semantic_on_for_both_stores(semantic_provider): - provider, _ = semantic_provider - assert provider._memory.semantic - assert provider._chats.index.schema == provider._memory.index.schema == HYBRID - - -def test_prefetch_injects_three_memories_in_semantic_mode(semantic_provider): - provider, redis = semantic_provider +def test_prefetch_injects_three_memories(hybrid_provider): + provider, redis = hybrid_provider block = provider.prefetch("what do I prefer") assert block.count("\n- ") == 3 (limit,) = [c[c.index("LIMIT") + 1] for c in redis.commands if c[0] == "SEARCH.QUERY"] assert limit == "3" -def test_prefetch_survives_a_redis_outage(semantic_provider): - provider, redis = semantic_provider +def test_prefetch_survives_a_redis_outage(hybrid_provider): + provider, redis = hybrid_provider redis.hybrid_error = redis.keyword_error = httpx.ConnectError("down") assert provider.prefetch("anything") == "" -def test_the_search_tool_says_how_results_were_found(semantic_provider): - provider, _ = semantic_provider +def test_the_search_tool_says_how_results_were_found(hybrid_provider): + provider, _ = hybrid_provider result = json.loads(provider.handle_tool_call("upstash_memory_search", {"query": "m"})) assert {r["retrieval"] for r in result["results"]} == {"hybrid"} -def test_mirror_deletion_matches_by_keyword_only(semantic_provider): - provider, redis = semantic_provider +def test_mirror_deletion_matches_by_keyword_only(hybrid_provider): + provider, redis = hybrid_provider provider._all_matches("old entry") assert all("$rrf" not in q for q in redis.queries()) -# -- migration ----------------------------------------------------------------------------------- - - -def migrated(redis, to="hybrid", **kwargs): - return migrate(redis, namespace="agentkit", to=to, log=quiet, **kwargs) - - -def test_migration_rebuilds_a_keyword_index_and_checks_it(): - redis = FakeRedis(SHARED_SCHEMA, [memory_row(i) for i in range(7)], chats=2) - assert migrated(redis) == "migrated" - ops = redis.ops() - assert ops.index("SET") < ops.index("SEARCH.DROP") < ops.index("SEARCH.CREATE") - assert redis.index_schema == HYBRID - assert redis.kv == {} # baseline removed once checked - - -def test_pending_updates_are_settled_before_the_baseline(): - redis = FakeRedis(SHARED_SCHEMA, [memory_row(1)], chats=1) - redis.pending = [memory_row(2)] # written just before the clients stopped - assert migrated(redis) == "migrated" - first_count = redis.ops().index("SEARCH.COUNT") - assert redis.ops().index("SEARCH.WAITINDEXING") < first_count - - -def test_migration_back_to_keyword_restores_the_0_2_index(): - redis = FakeRedis(HYBRID, [memory_row(1)], chats=3) - assert migrated(redis, to="keyword") == "migrated" - assert redis.index_schema == list(SHARED_SCHEMA) - - -def test_migration_to_the_current_kind_does_nothing(): - redis = FakeRedis(HYBRID, [memory_row(1)]) - assert migrated(redis) == "unchanged" - assert "SEARCH.DROP" not in redis.ops() - - -@pytest.mark.parametrize( - "redis", - [ - FakeRedis(SHARED_SCHEMA + [("extra", "TEXT", False)]), - FakeRedis(SHARED_SCHEMA, alias_of="agentkit_v2"), - ], -) -def test_migration_refuses_an_index_it_did_not_create_or_an_alias(redis): - with pytest.raises(MigrationError, match="refusing"): - migrated(redis) - assert "SEARCH.DROP" not in redis.ops() - - -def test_a_declined_migration_drops_nothing(): - redis = FakeRedis(SHARED_SCHEMA) - with pytest.raises(MigrationError, match="cancelled"): - migrated(redis, confirm=lambda plan: False) - assert "SEARCH.DROP" not in redis.ops() and redis.kv == {} - - -def test_a_failed_drop_changes_nothing(): - class NoDrop(FakeRedis): - def execute(self, command): - if command[0] == "SEARCH.DROP": - raise httpx.ReadTimeout("timed out") - return super().execute(command) - - redis = NoDrop(SHARED_SCHEMA, [memory_row(1)]) - with pytest.raises(MigrationError, match="nothing changed"): - migrated(redis) - assert redis.index_schema == list(SHARED_SCHEMA) and redis.kv == {} - - -def test_a_failed_rebuild_says_where_things_stand_and_resumes(): - class NoCreate(FakeRedis): - broken = True - - def execute(self, command): - if command[0] == "SEARCH.CREATE" and self.broken: - self.commands.append(command) - raise httpx.ReadTimeout("read timed out") - return super().execute(command) - - redis = NoCreate(SHARED_SCHEMA, [memory_row(1), memory_row(2)], chats=1) - with pytest.raises(MigrationError, match="The index is absent.*--to keyword"): - migrated(redis) - assert redis.ops().count("SEARCH.CREATE") == 3 - assert "agentkit:migrate:baseline" in redis.kv - - redis.broken = False # the server is back: the same command resumes and checks the baseline - assert migrated(redis) == "migrated" - assert redis.index_schema == HYBRID and redis.kv == {} - - -def test_a_timed_out_create_that_succeeded_is_not_repeated(): - class SlowCreate(FakeRedis): - def execute(self, command): - if command[0] == "SEARCH.CREATE": - super().execute(command) - raise httpx.ReadTimeout("read timed out") - return super().execute(command) - - redis = SlowCreate(SHARED_SCHEMA, [memory_row(1)]) - assert migrated(redis) == "migrated" - assert redis.ops().count("SEARCH.CREATE") == 1 - - -def test_a_count_change_is_reported_and_rechecked_on_the_next_run(): - class Writer(FakeRedis): - def execute(self, command): - if command[0] == "SEARCH.DROP": - self.memories.append(memory_row("late")) # a client kept writing - return super().execute(command) - - redis = Writer(SHARED_SCHEMA, [memory_row(i) for i in range(3)]) - with pytest.raises(MigrationError, match="still writing"): - migrated(redis) - assert "agentkit:migrate:baseline" in redis.kv # kept: a rerun checks again, not "unchanged" - with pytest.raises(MigrationError, match="still writing"): - migrated(redis) - - redis.memories.pop() # the stray write is dealt with - assert migrated(redis) == "resumed" - assert redis.kv == {} - - -def test_lost_memories_are_caught_by_key_not_just_by_count(): - class Swap(FakeRedis): - def execute(self, command): - if command[0] == "SEARCH.DROP": - self.memories = [memory_row("other")] # same count, different documents - return super().execute(command) - - redis = Swap(SHARED_SCHEMA, [memory_row("mine")]) - with pytest.raises(MigrationError, match="lost 1 sampled"): - migrated(redis) - - -def test_a_failed_rebuild_can_be_rolled_back_with_the_command_it_suggests(): - class NoHybrid(FakeRedis): - def execute(self, command): - if command[0] == "SEARCH.CREATE" and "HYBRID" in command: - self.commands.append(command) - raise UpstashError("ERR embeddings unavailable") - return super().execute(command) - - redis = NoHybrid(SHARED_SCHEMA, [memory_row(1), memory_row(2)], chats=1) - with pytest.raises(MigrationError, match="--to keyword"): - migrated(redis) - assert redis.index_schema is None - assert migrated(redis, to="keyword") == "migrated" # checked against the original baseline - assert redis.index_schema == list(SHARED_SCHEMA) and redis.kv == {} - - -def test_a_rollback_after_a_rebuild_that_failed_its_check_keeps_the_original_baseline(): - class Writer(FakeRedis): - def execute(self, command): - if command[0] == "SEARCH.DROP" and self.index_schema == list(SHARED_SCHEMA): - self.memories.append(memory_row("late")) - return super().execute(command) - - redis = Writer(SHARED_SCHEMA, [memory_row(1)]) - with pytest.raises(MigrationError, match="still writing"): - migrated(redis) - redis.memories.pop() - assert migrated(redis, to="keyword") == "migrated" - assert redis.index_schema == list(SHARED_SCHEMA) - - -def test_a_failed_drop_during_a_rollback_keeps_the_original_baseline(): - class Flaky(FakeRedis): - swap = True - drop_fails = False - - def execute(self, command): - if command[0] == "SEARCH.DROP": - if self.drop_fails: - raise httpx.ReadTimeout("timed out") - if self.swap: - self.memories = [memory_row("other")] # the hybrid rebuild loses a memory - return super().execute(command) - - redis = Flaky(SHARED_SCHEMA, [memory_row("mine")]) - with pytest.raises(MigrationError, match="lost 1 sampled"): - migrated(redis) - redis.drop_fails = True - with pytest.raises(MigrationError, match="nothing changed"): - migrated(redis, to="keyword") - assert "agentkit:migrate:baseline" in redis.kv - with pytest.raises(MigrationError, match="lost 1 sampled"): # not "unchanged" - migrated(redis) - - -@pytest.mark.parametrize("size", [11_000, 11_001]) -def test_lost_samples_are_caught_at_the_paging_boundary(size): - class Swap(FakeRedis): - def execute(self, command): - if command[0] == "SEARCH.DROP": - self.memories.reverse() - self.memories[-1] = memory_row("other") # one sampled memory replaced - return super().execute(command) - - redis = Swap(SHARED_SCHEMA, [memory_row(i) for i in range(size)]) - if size == 11_000: # every memory can be listed: the loss must be reported - with pytest.raises(MigrationError, match="lost 1 sampled"): - migrated(redis) - else: # one memory is past OFFSET's limit, so the sample check stops; counts still match - assert migrated(redis) == "migrated" - - -def test_migration_checks_stores_beyond_one_query_page(): - class Reorder(FakeRedis): - def execute(self, command): - if command[0] == "SEARCH.DROP": - self.memories.reverse() # the rebuilt index ranks documents differently - return super().execute(command) - - redis = Reorder(SHARED_SCHEMA, [memory_row(i) for i in range(2500)], chats=4) - assert migrated(redis) == "migrated" - offsets = [c[c.index("OFFSET") + 1] for c in redis.commands if "OFFSET" in c] - assert offsets == ["1000", "2000"] # paged to the sampled keys, within LIMIT's bound - - -def test_an_unrelated_pending_baseline_blocks_a_migration(): - redis = FakeRedis(None) - redis.kv["agentkit:migrate:baseline"] = json.dumps( - {"from": "keyword", "to": "keyword", "memories": 0, "chats": 0} - ) - with pytest.raises(MigrationError, match="pending"): - migrated(redis) - - def test_wait_indexing_recreates_an_index_that_was_dropped_under_it(): redis = FakeRedis(HYBRID, [memory_row(1)]) - memory = AgentMemory(redis, semantic=True) + memory = AgentMemory(redis) memory.index.ensure() redis.index_schema = None # another client dropped it memory.index.wait_indexing() diff --git a/python/hermes-memory/tests/test_semantic_redis.py b/python/hermes-memory/tests/test_semantic_redis.py index 8fb9c3a..b5d265b 100644 --- a/python/hermes-memory/tests/test_semantic_redis.py +++ b/python/hermes-memory/tests/test_semantic_redis.py @@ -1,4 +1,4 @@ -"""Semantic recall and the migration against a real Upstash Redis (skipped without secrets). +"""Semantic recall against a real Upstash Redis (skipped without secrets). Hybrid queries can fail intermittently on the server ("dot metric only accepts unit-length vectors"); recall then falls back to keyword search, so assertions about meaning retry a few times @@ -7,14 +7,6 @@ import pytest from upstash_hermes_memory import AgentMemory, ChatHistory -from upstash_hermes_memory._search import ( - SHARED_SCHEMA, - SearchIndex, - describe, - matches, - with_hybrid_text, -) -from upstash_hermes_memory.migrate import migrate FACTS = { "week": "Weeks start Monday for him.", @@ -45,26 +37,18 @@ def hybrid_recall(memory, query, user_id="u1", top_k=3, tries=4): @pytest.fixture def semantic(redis, prefix): - memory = AgentMemory(redis, namespace=prefix, semantic=True, enable_telemetry=False) + memory = AgentMemory(redis, namespace=prefix, enable_telemetry=False) memory.index.ensure() yield memory memory.index.drop() -def test_a_new_semantic_index_embeds_memory_text_only(semantic, prefix): - found = describe(semantic.redis, semantic.index.name) - prefixes = [f"{prefix}:memory:", f"{prefix}:chat:"] - assert matches(found, prefixes, with_hybrid_text(SHARED_SCHEMA)) - assert semantic.index.field_type("text") == "HYBRID" - assert semantic.index.field_type("userMessages") == "TEXT" - - def ids(hits): return [h.id for h in hits] def test_recall_finds_a_memory_by_meaning_within_what_is_injected(semantic): - # Hermes injects the top 3 in semantic mode. Equal-weight $rrf can rank a keyword-only match + # Hermes injects the top 3. Equal-weight $rrf can rank a keyword-only match # (here "first business day" for "day") level with the best meaning match, so the contract # tested is "among the injected 3", not "first". for id, text in FACTS.items(): @@ -78,7 +62,7 @@ def test_recall_finds_a_memory_by_meaning_within_what_is_injected(semantic): def test_hybrid_recall_never_crosses_users_or_returns_chats(semantic, redis, prefix): semantic.add("Hermes runs on the ops Box.", user_id="u1", id="mine") semantic.add("Hermes runs on a Raspberry Pi.", user_id="u2", id="theirs") - chats = ChatHistory(redis, namespace=prefix, semantic=True, enable_telemetry=False) + chats = ChatHistory(redis, namespace=prefix, enable_telemetry=False) chats.save_chat("s1", [{"role": "user", "content": "where does hermes run"}], user_id="u1") semantic.index.wait_indexing() hits = hybrid_recall(semantic, "where is the assistant hosted", top_k=5) @@ -87,39 +71,3 @@ def test_hybrid_recall_never_crosses_users_or_returns_chats(semantic, redis, pre def test_an_empty_store_recalls_nothing_without_errors(semantic): assert semantic.recall("anything at all", user_id="nobody") == [] - - -def test_migration_round_trip_keeps_memories_and_chats(redis, prefix): - keyword = AgentMemory(redis, namespace=prefix, enable_telemetry=False) - chats = ChatHistory(redis, namespace=prefix, enable_telemetry=False) - keyword.index.ensure() - try: - for id, text in FACTS.items(): - keyword.add(text, user_id="u1", id=id) - chats.save_chat("s1", [{"role": "user", "content": "flat white plans"}], user_id="u1") - keyword.index.wait_indexing() - quiet = lambda message: None - - assert migrate(redis, namespace=prefix, to="hybrid", log=quiet) == "migrated" - semantic = AgentMemory(redis, namespace=prefix, semantic=True, enable_telemetry=False) - semantic.index.ensure() - assert semantic.index.field_type("text") == "HYBRID" - # Keyword search and chats keep working on the rebuilt index whatever the semantic side does. - keyword_hits = semantic.recall("flat white", user_id="u1", mode="keyword") - assert "coffee" in ids(keyword_hits) - assert [h.session_id for h in chats.search_chats("flat white", user_id="u1")] == ["s1"] - assert redis.execute(["GET", f"{prefix}:migrate:baseline"]) is None - assert migrate(redis, namespace=prefix, to="hybrid", log=quiet) == "unchanged" - - assert migrate(redis, namespace=prefix, to="keyword", log=quiet) == "migrated" - # What 0.2.2 does on first use: a plain EXISTSOK create of the TEXT index must succeed. - legacy = SearchIndex( - redis, - name=keyword.index.name, - prefix=[f"{prefix}:memory:", f"{prefix}:chat:"], - schema=SHARED_SCHEMA, - ) - redis.execute(legacy.create_command()) - assert len(AgentMemory(redis, namespace=prefix).list(user_id="u1")) == len(FACTS) - finally: - redis.execute(["SEARCH.DROP", keyword.index.name])