From 4e1067dac252898cf51ad27f7d5277e592faf785 Mon Sep 17 00:00:00 2001 From: bashareid Date: Tue, 29 Sep 2026 07:39:55 +0300 Subject: [PATCH] feat(retrieval): opt-in llm_id for GoodMem's LLM answer (0.3.0) GoodMem can run an LLM over the retrieved chunks and stream back an abstractReply, but 0.2.1 had no way to ask for one: GoodMemToolkit(llm_id=...) raised TypeError and no request carried it, so the abstractReply the result parser could read never arrived. - GoodMemToolkit takes a developer-set llm_id, checked as a UUID at construction and again at use (GoodMemIdError, nothing sent); "" is refused with the same "pass llm_id=None" hint as reranker_id. - It is sent in the post-processor config beside reranker_id; unset, the request is unchanged. - goodmem_search returns the answer as abstractReply; the retriever puts it in every row's extra_info as goodmem_abstract_reply. Both are None when the LLM failed and absent when no LLM is configured. - goodmem_search(query, top_k) is unchanged: the model never sees it. - A failing LLM follows the status contract: hits kept, partial set, statuses [NOT_FOUND, SUMMARIZATION_FAILED] (missing LLM) or [SUMMARIZATION_FAILED] (provider 429). Scores are not relabelled; a working reranker beside a missing LLM keeps its reranker scores. Fixtures are live streams captured from GoodMem for each case. Offline 466 -> 517 (47 of the new tests fail on main), live 30 -> 35. Co-Authored-By: Claude Opus 5.5 --- README.md | 90 +++++++-- camel_goodmem/__init__.py | 2 +- camel_goodmem/retriever.py | 22 ++- camel_goodmem/toolkit.py | 50 ++++- pyproject.toml | 2 +- .../retrieve_llm_not_found.ndjson | 6 + tests/goodmem_fixtures/retrieve_llm_ok.ndjson | 5 + .../retrieve_llm_rate_limited.ndjson | 5 + .../retrieve_reranked_llm_not_found.ndjson | 6 + tests/test_goodmem_ids.py | 66 ++++++- tests/test_goodmem_live.py | 88 +++++++++ tests/test_goodmem_toolkit.py | 179 ++++++++++++++++++ 12 files changed, 497 insertions(+), 24 deletions(-) create mode 100644 tests/goodmem_fixtures/retrieve_llm_not_found.ndjson create mode 100644 tests/goodmem_fixtures/retrieve_llm_ok.ndjson create mode 100644 tests/goodmem_fixtures/retrieve_llm_rate_limited.ndjson create mode 100644 tests/goodmem_fixtures/retrieve_reranked_llm_not_found.ndjson diff --git a/README.md b/README.md index ee8c2b8..76d4134 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ agents. Documents are chunked, embedded and searched server-side; this package wraps the official `goodmem` Python SDK and exposes it to CAMEL both as a toolkit and as a `BaseRetriever`. -**Version 0.2.1.** Verified against GoodMem server **v1.0.320**. +**Version 0.3.0.** Verified against GoodMem server **v1.0.320**. > **Upgrading from 0.1.0.** 0.1.0 talked to GoodMem over hand-written HTTP and > had defects that were invisible from its return values — a failed search @@ -45,9 +45,9 @@ By default the model sees exactly two tools: | `goodmem_search` | `query`, `top_k` | | `goodmem_remember` | `text`, `metadata` | -Every operational setting — which spaces are readable, which reranker, whether -a threshold applies, whether files can be uploaded — is fixed by you at -construction time. The model cannot widen its own access, pick another space, +Every operational setting — which spaces are readable, which reranker, which +LLM answers from the results, whether a threshold applies, whether files can be +uploaded — is fixed by you at construction time. The model cannot widen its own access, pick another space, or turn on indexing waits. Opt in to more: @@ -61,8 +61,8 @@ Opt in to more: ### Ids must be UUIDs -Every GoodMem id this package handles — the `space_ids` and `reranker_id` you -configure, and the `memory_id`, `space_id` and `embedder_id` a tool or method +Every GoodMem id this package handles — the `space_ids`, `reranker_id` and +`llm_id` you configure, and the `memory_id`, `space_id` and `embedder_id` a tool or method takes — must be a UUID. Anything else raises `GoodMemIdError`, naming the argument, **before any request is made**, because the GoodMem SDK puts ids into request paths unescaped: `delete_memory("../spaces/")` would @@ -74,7 +74,8 @@ An empty string is not a UUID either: for no reranker, pass `reranker_id=None` or leave it out. `reranker_id=""` meant "no reranker" in 0.2.0 and is now refused at construction, so `reranker_id=os.getenv("GOODMEM_RERANKER_ID", "")` fails at startup — write -`os.getenv("GOODMEM_RERANKER_ID") or None`. +`os.getenv("GOODMEM_RERANKER_ID") or None`. `llm_id=""` is refused the same +way; for no LLM, pass `llm_id=None` or leave it out. ## Retrieval results @@ -96,6 +97,7 @@ An empty string is not a UUID either: for no reranker, pass "partial": False, # True when the server reported a problem "statuses": [], # what it reported "resultSetId": "...", + "abstractReply": "...", # only with llm_id -- see "LLM answers" } ``` @@ -127,6 +129,56 @@ Those hits are `scoreKind: "vector"`, flipped like any vector score, and `min_score` is not applied to them, so a reranker threshold cannot discard them; `partial` is set and `statuses` carries both codes. +## LLM answers + +GoodMem can run one of its configured LLMs over the chunks a search retrieved +and return a grounded answer beside them. This is **off by default** and +**set by you**, like `reranker_id`: pass the UUID of a GoodMem LLM as `llm_id` +when you construct the toolkit. The model never sees or chooses it — +`goodmem_search` still takes only `query` and `top_k`. + +```python +from camel_goodmem import GoodMemRetriever, GoodMemToolkit + +toolkit = GoodMemToolkit(space_ids=[""], llm_id="") + +result = toolkit.goodmem_search("What is the canary?") +result["abstractReply"] # "The canary is **ORYX-2290** ..." +result["results"] # the hits, exactly as without an LLM + +rows = GoodMemRetriever(toolkit).query("What is the canary?") +rows[0]["extra_info"]["goodmem_abstract_reply"] # the same answer +``` + +Where the answer appears: + +- **`goodmem_search`** (and so the tool result the model reads): the + `abstractReply` key, a string. +- **`GoodMemRetriever.query()`**: `goodmem_abstract_reply` in every row's + `extra_info`, since CAMEL's retriever returns a plain list of rows. It is + one answer for the whole retrieval, repeated on each row so it survives a + caller keeping only some of them. +- The key is present whenever `llm_id` is set, and absent otherwise. + +The id is sent as `llm_id` in the retrieval's post-processor config, beside +`reranker_id` when both are set. It is a UUID like every other id: anything +else raises `GoodMemIdError` before a request is made. + +An LLM does not rerank. Hits keep their scores, `scoreKind` and order +exactly as without it; combine it with `reranker_id` if you want reranking +as well. + +**When the LLM fails**, the search does not. The server reports +`SUMMARIZATION_FAILED` — plus `NOT_FOUND` when no LLM has that id — and still +returns the hits. You get the hits, `partial: True`, both statuses in +`statuses`, a `warning`, and `abstractReply: None` (in the retriever, +`goodmem_partial: True`, `goodmem_statuses` and `goodmem_abstract_reply: +None`). Nothing is raised and no hit is dropped. Measured live: an LLM id +that does not exist gave `[NOT_FOUND, SUMMARIZATION_FAILED]` with the hit +kept; a provider out of credits gave `SUMMARIZATION_FAILED` carrying the +provider's `429`. A reranker configured beside a failing LLM keeps its +reranker scores. + ## Metadata filters Filters are expressions evaluated server-side, not SQL. You set them when you @@ -203,8 +255,9 @@ rows = retriever.query("what did I store?", top_k=5) `query()` returns CAMEL's retriever shape — `similarity score`, `content path`, `metadata`, `extra_info`, `text` — with GoodMem specifics under `extra_info` (`goodmem_chunk_id`, `goodmem_memory_id`, `goodmem_space_id`, -`goodmem_score_kind`, `goodmem_raw_score`, `goodmem_partial`, and -`goodmem_statuses` when degraded). +`goodmem_score_kind`, `goodmem_raw_score`, `goodmem_partial`, +`goodmem_statuses` when degraded, and `goodmem_abstract_reply` when the +toolkit has an `llm_id`). ## Bringing your own client @@ -218,6 +271,17 @@ toolkit = GoodMemToolkit(client=Goodmem(base_url=..., api_key=...)) An injected client keeps its own server, credentials and TLS settings, and is never closed by the toolkit. +## Changes in 0.3.0 + +New, opt-in: an LLM answer from the retrieved chunks. See +[LLM answers](#llm-answers). + +| Was (0.2.1) | Now | +| --- | --- | +| No way to ask for GoodMem's LLM post-processing: `GoodMemToolkit(llm_id=...)` raised `TypeError: unexpected keyword argument 'llm_id'`, and no request carried one, so the `abstractReply` the result parser could read never arrived | `llm_id` constructor argument, checked as a UUID before any request and sent in the post-processor config; the answer is `abstractReply` on `goodmem_search` and `goodmem_abstract_reply` in the retriever's `extra_info`. Live with an OpenRouter `qwen/qwen3-8b` LLM: "The fixture canary is **ORYX-2290** ..." | +| Not reachable: no LLM could be requested | A failing LLM keeps the hits: `partial: True`, `statuses` `[NOT_FOUND, SUMMARIZATION_FAILED]` for an id that does not exist, `[SUMMARIZATION_FAILED]` for a provider `429`, `abstractReply: None`, never an exception | +| Not reachable | `goodmem_search(query, top_k)` is unchanged: the model cannot set or see the LLM | + ## Changes in 0.2.1 Measured against a local server that records every request line, driving the @@ -263,9 +327,9 @@ against GoodMem v1.0.320. | Suite | Count | Needs | | --- | --- | --- | -| `tests/test_goodmem_toolkit.py` | 86 | nothing — the real SDK over a mock transport, fed NDJSON captured from a live server | -| `tests/test_goodmem_ids.py` | 380 | nothing — the real SDK and `httpx` against a local server that records every request; every id-taking entry point (method, CAMEL tool, MCP tool, configuration) × ten malformed ids must send nothing, and a `str` or `uuid.UUID` subclass cannot change the id after it is checked. It also runs the live tests that depend on the id check against that server, and fails if any other live test passes an id the check would refuse | -| `tests/test_goodmem_live.py` | 30 | `GOODMEM_API_KEY` + `GOODMEM_BASE_URL`; skips entirely without them | +| `tests/test_goodmem_toolkit.py` | 101 | nothing — the real SDK over a mock transport, fed NDJSON captured from a live server | +| `tests/test_goodmem_ids.py` | 416 | nothing — the real SDK and `httpx` against a local server that records every request; every id-taking entry point (method, CAMEL tool, MCP tool, configuration) × ten malformed ids must send nothing, and a `str` or `uuid.UUID` subclass cannot change the id after it is checked. It also runs the live tests that depend on the id check against that server, and fails if any other live test passes an id the check would refuse | +| `tests/test_goodmem_live.py` | 35 | `GOODMEM_API_KEY` + `GOODMEM_BASE_URL`; skips entirely without them. The LLM tests also take `GOODMEM_TEST_LLM_ID` (a working LLM), and optionally `GOODMEM_TEST_FAILING_LLM_ID` (one whose provider fails) and `GOODMEM_TEST_RERANKER_ID`; each skips without its id | ```bash pip install -e ".[dev]" @@ -275,7 +339,7 @@ pytest tests/test_goodmem_toolkit.py tests/test_goodmem_ids.py # live (pin the embedder if the server's first one is unhealthy) GOODMEM_API_KEY=... GOODMEM_BASE_URL=... \ - GOODMEM_TEST_EMBEDDER_ID=... \ + GOODMEM_TEST_EMBEDDER_ID=... GOODMEM_TEST_LLM_ID=... \ pytest tests/test_goodmem_live.py # what CI runs diff --git a/camel_goodmem/__init__.py b/camel_goodmem/__init__.py index ed17128..fbd509f 100644 --- a/camel_goodmem/__init__.py +++ b/camel_goodmem/__init__.py @@ -20,7 +20,7 @@ from camel_goodmem.retriever import GoodMemRetriever from camel_goodmem.toolkit import GoodMemError, GoodMemToolkit -__version__ = "0.2.1" +__version__ = "0.3.0" __all__ = [ "GoodMemToolkit", diff --git a/camel_goodmem/retriever.py b/camel_goodmem/retriever.py index 320093c..63baf66 100644 --- a/camel_goodmem/retriever.py +++ b/camel_goodmem/retriever.py @@ -24,7 +24,8 @@ class GoodMemRetriever(BaseRetriever): Args: toolkit (Any): A configured :class:`~camel_goodmem.GoodMemToolkit`, which carries the - connection, the spaces and any metadata filter. + connection, the spaces, any reranker or LLM, and any metadata + filter. metadata_filter (Optional[Union[Dict[str, Any], str]]): A filter every result must also match: a mapping (an ``AND`` of equalities) or an expression built with @@ -91,7 +92,10 @@ def query( ``similarity score``, ``content path``, ``metadata``, ``extra_info`` and ``text``. When the retrieval was degraded and nothing usable came back, a single dictionary is returned - whose ``text`` states what the server reported. + whose ``text`` states what the server reported. When the + toolkit has an ``llm_id``, every row's ``extra_info`` carries + ``goodmem_abstract_reply``: the LLM's answer, or ``None`` if + it failed. """ outcome = self.toolkit._retrieve( query, top_k, narrow=resolve_filter(self.metadata_filter) @@ -117,6 +121,16 @@ def query( ) hits = kept + # One answer per retrieval, so it rides on every row: a CAMEL caller + # that keeps only the first row, or only rows above a threshold, + # still has it. + summary: dict[str, Any] = {} + if ( + outcome.abstract_reply is not None + or getattr(self.toolkit, "llm_id", None) is not None + ): + summary["goodmem_abstract_reply"] = outcome.abstract_reply + results: list[dict[str, Any]] = [] for hit in hits: extra: dict[str, Any] = { @@ -126,6 +140,7 @@ def query( "goodmem_score_kind": hit.score_kind, "goodmem_raw_score": hit.raw_score, "goodmem_partial": outcome.partial, + **summary, } if outcome.partial: extra["goodmem_statuses"] = outcome.status_dicts @@ -159,6 +174,7 @@ def query( "extra_info": { "goodmem_partial": True, "goodmem_statuses": outcome.status_dicts, + **summary, }, } ] @@ -168,7 +184,7 @@ def query( f"No information relevant to {query!r} is stored in " "the configured GoodMem space(s)." ), - "extra_info": {"goodmem_partial": False}, + "extra_info": {"goodmem_partial": False, **summary}, } ] return results diff --git a/camel_goodmem/toolkit.py b/camel_goodmem/toolkit.py index 6ccd182..dd3c1d2 100644 --- a/camel_goodmem/toolkit.py +++ b/camel_goodmem/toolkit.py @@ -31,6 +31,13 @@ "an empty string is refused rather than read as 'no reranker'." ) +#: The same refusal for ``llm_id``, so ``llm_id=os.getenv("X", "")`` fails at +#: startup rather than being sent as an id. +_NO_LLM_HINT = ( + "To search without an LLM, pass llm_id=None or leave it out; " + "an empty string is refused rather than read as 'no LLM'." +) + class GoodMemError(RuntimeError): r"""Raised when a GoodMem operation fails. @@ -81,8 +88,8 @@ class GoodMemToolkit(BaseToolkit): embedded and searched server-side. This toolkit wraps the official ``goodmem`` Python SDK and exposes a deliberately narrow set of tools to the model -- a search and, optionally, a write -- while every operational - setting (which spaces, which reranker, whether uploads are possible) is - fixed by the developer at construction time. + setting (which spaces, which reranker, which LLM, whether uploads are + possible) is fixed by the developer at construction time. Args: base_url (Optional[str]): The base URL of the GoodMem server. Falls @@ -105,6 +112,16 @@ class GoodMemToolkit(BaseToolkit): retrieval. Without one, no relevance threshold is applied. Pass ``None`` for no reranker: an empty string is a malformed id and is refused. (default: :obj:`None`) + llm_id (Optional[str]): The UUID of a GoodMem LLM to run over the + retrieved chunks. Its grounded answer is returned as + ``abstractReply`` by ``goodmem_search`` and as + ``goodmem_abstract_reply`` in the retriever's ``extra_info``. + Scores are unaffected: an LLM does not rerank. When the LLM + fails, the server reports ``SUMMARIZATION_FAILED`` (and + ``NOT_FOUND`` for an LLM that does not exist); the hits are still + returned, ``partial`` is set and ``abstractReply`` is ``None``. + Pass ``None`` for no LLM: an empty string is a malformed id and + is refused. (default: :obj:`None`) min_score (Optional[float]): Drop hits scoring below this value. Applies only to reranker scores: not without ``reranker_id``, and not when the server reports that reranking failed and returns @@ -145,6 +162,7 @@ def __init__( timeout: float | None = 30.0, upload_dir: str | Path | None = None, reranker_id: str | None = None, + llm_id: str | None = None, min_score: float | None = None, metadata_filter: dict[str, Any] | str | None = None, allow_write: bool = True, @@ -175,6 +193,11 @@ def __init__( if reranker_id is not None else None ) + self.llm_id = ( + require_uuid(llm_id, "llm_id", hint=_NO_LLM_HINT) + if llm_id is not None + else None + ) self.min_score = min_score # Resolved now so a bad filter fails at construction rather than on # the first search; resolved again at use, as it is public. @@ -281,6 +304,12 @@ def _require_reranker(self) -> str | None: self.reranker_id, "reranker_id", hint=_NO_RERANKER_HINT ) + def _require_llm(self) -> str | None: + r"""Returns the configured LLM as a canonical UUID, if any.""" + if self.llm_id is None: + return None + return require_uuid(self.llm_id, "llm_id", hint=_NO_LLM_HINT) + def _space_keys(self, narrow: str = "") -> list[dict[str, Any]]: r"""Builds the ``spaceKeys`` payload, including any metadata filter. @@ -309,9 +338,11 @@ def _retrieve( toolkit's own. (default: ``""``) Returns: - RetrievalOutcome: The hits and any statuses the server reported. + RetrievalOutcome: The hits, any statuses the server reported, and + the LLM's answer when one was configured and produced. """ reranker_id = self._require_reranker() + llm_id = self._require_llm() kwargs: dict[str, Any] = { "message": query, "space_keys": self._space_keys(narrow), @@ -320,6 +351,10 @@ def _retrieve( } if reranker_id: kwargs["reranker_id"] = reranker_id + if llm_id: + # The SDK puts it in the post-processor config beside + # ``reranker_id``. It does not change the hits or their scores. + kwargs["llm_id"] = llm_id try: stream = self._client.memories.retrieve(**kwargs) @@ -377,7 +412,9 @@ def goodmem_search(self, query: str, top_k: int = 5) -> dict[str, Any]: chunks, each with its text and the metadata of the memory it came from), ``partial`` (``True`` when the server reported a problem during this search), ``statuses`` (what the server - reported), and ``query``. + reported), and ``query``. When the developer configured an + LLM, ``abstractReply`` holds its answer drawn from the + results, or ``None`` if it failed (see ``statuses``). """ outcome = self._retrieve(query, top_k) result: dict[str, Any] = { @@ -394,7 +431,10 @@ def goodmem_search(self, query: str, top_k: int = 5) -> dict[str, Any]: # an empty result carries the reason rather than reading as a # clean miss. result["warning"] = outcome.warning_text() - if outcome.abstract_reply: + if outcome.abstract_reply is not None or self.llm_id is not None: + # Present whenever an LLM was asked for, so a failed one reads as + # None beside its SUMMARIZATION_FAILED status, not as a missing + # key. result["abstractReply"] = outcome.abstract_reply return result diff --git a/pyproject.toml b/pyproject.toml index 51e7270..22b6163 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ description = "GoodMem integration for CAMEL." authors = [{ name = "PAIR Systems" }] readme = "README.md" requires-python = ">=3.10" -version = "0.2.1" +version = "0.3.0" license-files = ["LICENSE"] urls.homepage = "https://github.com/PAIR-Systems-Inc/goodmem_camel" urls.source = "https://github.com/PAIR-Systems-Inc/goodmem_camel" diff --git a/tests/goodmem_fixtures/retrieve_llm_not_found.ndjson b/tests/goodmem_fixtures/retrieve_llm_not_found.ndjson new file mode 100644 index 0000000..ab47e2c --- /dev/null +++ b/tests/goodmem_fixtures/retrieve_llm_not_found.ndjson @@ -0,0 +1,6 @@ +{"status":{"code":"NOT_FOUND","message":"LLM validation failed: Exception: LLM not found: 00000000-0000-7000-8000-000000000000 (ID: 00000000-0000-7000-8000-000000000000). Verify the LLM exists and is accessible.","details":{"llm_id":"00000000-0000-7000-8000-000000000000"}}} +{"resultSetBoundary":{"resultSetId":"01a0eb6e-0721-708a-addc-0503840bb853","kind":"BEGIN","stageName":"retrieve","expectedItems":1}} +{"memoryDefinition":{"memoryId":"01a0eb6d-e602-7497-a5f3-0e81bc638a2d","spaceId":"01a0eb6d-e5fa-71e4-9650-2616440da487","originalContentLength":53,"originalContentSha256":"602abb804886146f55a1a2da8285df16458754aa50e2a685e6a2389b0807f8cc","contentType":"text/plain","processingStatus":"COMPLETED","pageImageStatus":"PENDING","pageImageCount":0,"metadata":{"tenant":"acme"},"createdAt":1790656243203,"updatedAt":1790656245224,"createdById":"019cfcff-37c7-75ef-be71-06c83dae99c3","updatedById":"019cfcff-37c7-75ef-be71-06c83dae99c3"}} +{"retrievedItem":{"chunk":{"resultSetId":"01a0eb6e-0721-708a-addc-0503840bb853","chunk":{"chunkId":"01a0eb6d-ed10-77fd-9f0e-04a96830ff1a","memoryId":"01a0eb6d-e602-7497-a5f3-0e81bc638a2d","chunkSequenceNumber":0,"chunkText":"The fixture canary is ORYX-2290. CAMEL toolkit audit.\n","vectorStatus":"COMPLETED","startOffset":0,"endOffset":54,"createdAt":1790656245008,"updatedAt":1790656245223,"createdById":"019cfcff-37c7-75ef-be71-06c83dae99c3","updatedById":"019cfcff-37c7-75ef-be71-06c83dae99c3"},"memoryIndex":0,"relevanceScore":-0.6840693950653076}}} +{"resultSetBoundary":{"resultSetId":"01a0eb6e-0721-708a-addc-0503840bb853","kind":"END","stageName":""}} +{"status":{"code":"SUMMARIZATION_FAILED","message":"Failed to create LLM inference client: Exception: LLM not found: 00000000-0000-7000-8000-000000000000","details":{"llm_id":"00000000-0000-7000-8000-000000000000"}}} diff --git a/tests/goodmem_fixtures/retrieve_llm_ok.ndjson b/tests/goodmem_fixtures/retrieve_llm_ok.ndjson new file mode 100644 index 0000000..c9fe316 --- /dev/null +++ b/tests/goodmem_fixtures/retrieve_llm_ok.ndjson @@ -0,0 +1,5 @@ +{"resultSetBoundary":{"resultSetId":"01a0eb6d-f29e-73eb-b4c7-9a525e8e64a2","kind":"BEGIN","stageName":"retrieve","expectedItems":1}} +{"memoryDefinition":{"memoryId":"01a0eb6d-e602-7497-a5f3-0e81bc638a2d","spaceId":"01a0eb6d-e5fa-71e4-9650-2616440da487","originalContentLength":53,"originalContentSha256":"602abb804886146f55a1a2da8285df16458754aa50e2a685e6a2389b0807f8cc","contentType":"text/plain","processingStatus":"COMPLETED","pageImageStatus":"PENDING","pageImageCount":0,"metadata":{"tenant":"acme"},"createdAt":1790656243203,"updatedAt":1790656245224,"createdById":"019cfcff-37c7-75ef-be71-06c83dae99c3","updatedById":"019cfcff-37c7-75ef-be71-06c83dae99c3"}} +{"retrievedItem":{"chunk":{"resultSetId":"01a0eb6d-f29e-73eb-b4c7-9a525e8e64a2","chunk":{"chunkId":"01a0eb6d-ed10-77fd-9f0e-04a96830ff1a","memoryId":"01a0eb6d-e602-7497-a5f3-0e81bc638a2d","chunkSequenceNumber":0,"chunkText":"The fixture canary is ORYX-2290. CAMEL toolkit audit.\n","vectorStatus":"COMPLETED","startOffset":0,"endOffset":54,"createdAt":1790656245008,"updatedAt":1790656245223,"createdById":"019cfcff-37c7-75ef-be71-06c83dae99c3","updatedById":"019cfcff-37c7-75ef-be71-06c83dae99c3"},"memoryIndex":0,"relevanceScore":-0.6840693950653076}}} +{"resultSetBoundary":{"resultSetId":"01a0eb6d-f29e-73eb-b4c7-9a525e8e64a2","kind":"END","stageName":""}} +{"abstractReply":{"text":"The fixture canary is **ORYX-2290**, as indicated in the retrieved record (source_index: 1). This identifier is explicitly tied to the \"fixture canary\" in the provided data. The entry also references a \"CAMEL toolkit audit,\" but no further details about the canary's purpose or context are included. The data remains untrusted and should be verified against additional sources for accuracy.","relevanceScore":0.0,"resultSetId":"01a0eb6d-f29e-73eb-b4c7-9a525e8e64a2"}} diff --git a/tests/goodmem_fixtures/retrieve_llm_rate_limited.ndjson b/tests/goodmem_fixtures/retrieve_llm_rate_limited.ndjson new file mode 100644 index 0000000..b0a4772 --- /dev/null +++ b/tests/goodmem_fixtures/retrieve_llm_rate_limited.ndjson @@ -0,0 +1,5 @@ +{"resultSetBoundary":{"resultSetId":"01a0eb6e-07df-7451-bffb-f747e47a3ac9","kind":"BEGIN","stageName":"retrieve","expectedItems":1}} +{"memoryDefinition":{"memoryId":"01a0eb6d-e602-7497-a5f3-0e81bc638a2d","spaceId":"01a0eb6d-e5fa-71e4-9650-2616440da487","originalContentLength":53,"originalContentSha256":"602abb804886146f55a1a2da8285df16458754aa50e2a685e6a2389b0807f8cc","contentType":"text/plain","processingStatus":"COMPLETED","pageImageStatus":"PENDING","pageImageCount":0,"metadata":{"tenant":"acme"},"createdAt":1790656243203,"updatedAt":1790656245224,"createdById":"019cfcff-37c7-75ef-be71-06c83dae99c3","updatedById":"019cfcff-37c7-75ef-be71-06c83dae99c3"}} +{"retrievedItem":{"chunk":{"resultSetId":"01a0eb6e-07df-7451-bffb-f747e47a3ac9","chunk":{"chunkId":"01a0eb6d-ed10-77fd-9f0e-04a96830ff1a","memoryId":"01a0eb6d-e602-7497-a5f3-0e81bc638a2d","chunkSequenceNumber":0,"chunkText":"The fixture canary is ORYX-2290. CAMEL toolkit audit.\n","vectorStatus":"COMPLETED","startOffset":0,"endOffset":54,"createdAt":1790656245008,"updatedAt":1790656245223,"createdById":"019cfcff-37c7-75ef-be71-06c83dae99c3","updatedById":"019cfcff-37c7-75ef-be71-06c83dae99c3"},"memoryIndex":0,"relevanceScore":-0.6840693950653076}}} +{"resultSetBoundary":{"resultSetId":"01a0eb6e-07df-7451-bffb-f747e47a3ac9","kind":"END","stageName":""}} +{"status":{"code":"SUMMARIZATION_FAILED","message":"StatusOr{status=INTERNAL: Summary unavailable due to LLM processing error: com.openai.errors.RateLimitException: 429: You have no credits remaining. Add credits to continue using the API at https://platform.openai.com/settings/organization/billing/.}"}} diff --git a/tests/goodmem_fixtures/retrieve_reranked_llm_not_found.ndjson b/tests/goodmem_fixtures/retrieve_reranked_llm_not_found.ndjson new file mode 100644 index 0000000..68454c4 --- /dev/null +++ b/tests/goodmem_fixtures/retrieve_reranked_llm_not_found.ndjson @@ -0,0 +1,6 @@ +{"status":{"code":"NOT_FOUND","message":"LLM validation failed: Exception: LLM not found: 00000000-0000-7000-8000-000000000000 (ID: 00000000-0000-7000-8000-000000000000). Verify the LLM exists and is accessible.","details":{"llm_id":"00000000-0000-7000-8000-000000000000"}}} +{"resultSetBoundary":{"resultSetId":"01a0eb6e-22d5-7180-b7a2-e76aafc462bc","kind":"BEGIN","stageName":"rerank","expectedItems":1}} +{"memoryDefinition":{"memoryId":"01a0eb6d-e602-7497-a5f3-0e81bc638a2d","spaceId":"01a0eb6d-e5fa-71e4-9650-2616440da487","originalContentLength":53,"originalContentSha256":"602abb804886146f55a1a2da8285df16458754aa50e2a685e6a2389b0807f8cc","contentType":"text/plain","processingStatus":"COMPLETED","pageImageStatus":"PENDING","pageImageCount":0,"metadata":{"tenant":"acme"},"createdAt":1790656243203,"updatedAt":1790656245224,"createdById":"019cfcff-37c7-75ef-be71-06c83dae99c3","updatedById":"019cfcff-37c7-75ef-be71-06c83dae99c3"}} +{"retrievedItem":{"chunk":{"resultSetId":"01a0eb6e-22d5-7180-b7a2-e76aafc462bc","chunk":{"chunkId":"01a0eb6d-ed10-77fd-9f0e-04a96830ff1a","memoryId":"01a0eb6d-e602-7497-a5f3-0e81bc638a2d","chunkSequenceNumber":0,"chunkText":"The fixture canary is ORYX-2290. CAMEL toolkit audit.\n","vectorStatus":"COMPLETED","startOffset":0,"endOffset":54,"createdAt":1790656245008,"updatedAt":1790656245223,"createdById":"019cfcff-37c7-75ef-be71-06c83dae99c3","updatedById":"019cfcff-37c7-75ef-be71-06c83dae99c3"},"memoryIndex":0,"relevanceScore":0.8671875}}} +{"resultSetBoundary":{"resultSetId":"01a0eb6e-22d5-7180-b7a2-e76aafc462bc","kind":"END","stageName":""}} +{"status":{"code":"SUMMARIZATION_FAILED","message":"Failed to create LLM inference client: Exception: LLM not found: 00000000-0000-7000-8000-000000000000","details":{"llm_id":"00000000-0000-7000-8000-000000000000"}}} diff --git a/tests/test_goodmem_ids.py b/tests/test_goodmem_ids.py index 7eccf1d..9e5f465 100644 --- a/tests/test_goodmem_ids.py +++ b/tests/test_goodmem_ids.py @@ -44,6 +44,7 @@ MEMORY = "01a0d44b-748d-72eb-b54e-c3ea2d956927" EMBEDDER = "019cfd1c-c033-7517-b7de-f73941a0464b" RERANKER = "019cfd1d-5b7e-7a41-9c3d-2f0e8a6b4c11" +LLM = "019cfd9f-0963-76f9-b069-4cde19a64ba8" #: Every payload is refused. The first five are traversals, the rest are #: near-misses a lenient check lets through: a leading space, a query or @@ -283,6 +284,10 @@ def _upload(tk: GoodMemToolkit, tmp_path: Path) -> Any: "retriever.query": lambda tk: GoodMemRetriever(tk).query("q"), } +#: ``llm_id`` goes in the retrieval body, beside ``reranker_id``, and is +#: checked by the same validator. +USES_CONFIGURED_LLM = USES_CONFIGURED_RERANKER + @pytest.mark.parametrize("payload", PAYLOADS.values(), ids=PAYLOADS.keys()) @pytest.mark.parametrize("entry", DIRECT.keys()) @@ -348,6 +353,17 @@ def test_a_malformed_configured_reranker_is_refused_at_construction( ) +@pytest.mark.parametrize("payload", PAYLOADS.values(), ids=PAYLOADS.keys()) +def test_a_malformed_configured_llm_is_refused_at_construction( + server, payload +): + _assert_refused( + server, + lambda: _toolkit(server, llm_id=payload).goodmem_search("q"), + "llm_id", + ) + + @pytest.mark.parametrize("payload", PAYLOADS.values(), ids=PAYLOADS.keys()) @pytest.mark.parametrize("entry", USES_CONFIGURED_SPACE.keys()) def test_a_space_id_set_after_construction_is_refused_at_the_call( @@ -376,6 +392,16 @@ def test_a_reranker_id_set_after_construction_is_refused_at_the_call( ) +@pytest.mark.parametrize("payload", PAYLOADS.values(), ids=PAYLOADS.keys()) +@pytest.mark.parametrize("entry", USES_CONFIGURED_LLM.keys()) +def test_an_llm_id_set_after_construction_is_refused_at_the_call( + server, entry, payload +): + tk = _toolkit(server) + tk.llm_id = payload + _assert_refused(server, lambda: USES_CONFIGURED_LLM[entry](tk), "llm_id") + + # --------------------------------------------------------------------------- # A valid UUID still reaches exactly the intended resource # --------------------------------------------------------------------------- @@ -466,6 +492,14 @@ def test_body_ids_are_sent_canonical(server): assert [e["embedderId"] for e in embedders] == [EMBEDDER] +def test_the_llm_id_is_sent_canonical_in_the_post_processor(server): + _toolkit(server, llm_id=LLM.upper()).goodmem_search("q") + (retrieve,) = server.log + body = json.loads(retrieve["body"]) + assert body["postProcessor"]["config"]["llm_id"] == LLM + assert LLM.upper() not in json.dumps(body) + + # --------------------------------------------------------------------------- # The model is told, too # --------------------------------------------------------------------------- @@ -674,6 +708,27 @@ def test_no_reranker_is_none_not_an_empty_string(server): assert "rerankerId" not in json.dumps(json.loads(retrieve["body"])) +def test_an_empty_llm_id_is_refused_saying_how_to_turn_it_off(server): + _assert_refused(server, lambda: _toolkit(server, llm_id=""), "llm_id") + with pytest.raises(GoodMemIdError, match=r"pass llm_id=None"): + _toolkit(server, llm_id="") + + +def test_an_empty_llm_id_set_later_says_how_to_turn_it_off(server): + tk = _toolkit(server) + tk.llm_id = "" + with pytest.raises(GoodMemIdError, match=r"pass llm_id=None"): + tk.goodmem_search("q") + assert _sent(server) == [] + + +def test_no_llm_is_none_and_sends_no_post_processor(server): + _toolkit(server, llm_id=None).goodmem_search("q") + (retrieve,) = server.log + body = json.loads(retrieve["body"]) + assert "postProcessor" not in body and "llm_id" not in json.dumps(body) + + def test_the_readme_calls_out_that_an_empty_reranker_id_is_refused(): readme = (Path(__file__).parents[1] / "README.md").read_text("utf-8") changes = readme.split("## Changes in 0.2.1", 1)[1].split("\n## ", 1)[0] @@ -718,7 +773,14 @@ def test_the_live_malformed_embedder_test_holds_against_a_server(server): "delete_memory": 0, "create_space": 1, } -_ID_NAMES = {"memory_id", "space_id", "embedder_id", "reranker_id", "id"} +_ID_NAMES = { + "memory_id", + "space_id", + "embedder_id", + "reranker_id", + "llm_id", + "id", +} _ID_LISTS = {"space_ids"} @@ -810,6 +872,8 @@ def test_no_live_test_expects_the_server_to_see_a_malformed_id(): ("GoodMemToolkit(space_ids=['space-1'])\n", ["line 1: 'space-1'"]), ("GoodMemToolkit(reranker_id='')\n", ["line 1: ''"]), ("tk.reranker_id = 'rr-1'\n", ["line 1: 'rr-1'"]), + ("GoodMemToolkit(llm_id='qwen3-8b')\n", ["line 1: 'qwen3-8b'"]), + ("tk.llm_id = ''\n", ["line 1: ''"]), # Expected to be refused client-side: fine. ( "with pytest.raises(GoodMemIdError):\n" diff --git a/tests/test_goodmem_live.py b/tests/test_goodmem_live.py index c40abca..a03386f 100644 --- a/tests/test_goodmem_live.py +++ b/tests/test_goodmem_live.py @@ -39,6 +39,17 @@ #: that is not a UUID before a request is made, so a test that needs the #: *server* to reject an id has to send one that passes that check. NO_SUCH_EMBEDDER = "00000000-0000-7000-8000-000000000000" +#: The same for an LLM: well-formed, so it reaches the server, which answers +#: NOT_FOUND and SUMMARIZATION_FAILED in the stream. +NO_SUCH_LLM = "00000000-0000-7000-8000-000000000000" + +#: A working GoodMem LLM, for the LLM tests; they skip without one. +LLM_ID = os.environ.get("GOODMEM_TEST_LLM_ID") +#: Optional: an LLM whose provider fails (e.g. out of credits), to see a +#: real provider failure rather than a missing id. +FAILING_LLM_ID = os.environ.get("GOODMEM_TEST_FAILING_LLM_ID") +#: Optional: a working reranker, to check an LLM failure leaves its scores. +RERANKER_ID = os.environ.get("GOODMEM_TEST_RERANKER_ID") def _embedder_id(toolkit: GoodMemToolkit) -> str: @@ -244,6 +255,83 @@ def test_the_read_path_does_not_poll(self, admin): assert elapsed < 3.0, f"an empty search took {elapsed:.1f}s" +class TestLiveLlm: + r"""``llm_id`` is developer-set; the model's tool is unchanged.""" + + def _search(self, space, seeded, **options): + toolkit = GoodMemToolkit( + space_ids=[space], verify_ssl=VERIFY_SSL, **options + ) + try: + names = {t.get_function_name(): t for t in toolkit.get_tools()} + props = names["goodmem_search"].get_openai_tool_schema()[ + "function" + ]["parameters"]["properties"] + assert set(props) == {"query", "top_k"} + return toolkit.goodmem_search( + f"What is the CAMEL live-test canary? {seeded[0]}", top_k=3 + ) + finally: + toolkit.close() + + def test_the_llm_answer_comes_back_with_the_hits(self, space, seeded): + if not LLM_ID: + pytest.skip("GOODMEM_TEST_LLM_ID is not set") + result = self._search(space, seeded, llm_id=LLM_ID) + assert result["partial"] is False, result["statuses"] + assert result["abstractReply"], "no answer came back" + assert seeded[0] in result["abstractReply"] + hit = result["results"][0] + assert hit["memoryId"] == seeded[1] + # An LLM does not rerank: the scores are still vector scores. + assert hit["scoreKind"] == "vector" and hit["rawScore"] < 0 + + def test_the_retriever_carries_the_llm_answer(self, space, seeded): + if not LLM_ID: + pytest.skip("GOODMEM_TEST_LLM_ID is not set") + toolkit = GoodMemToolkit( + space_ids=[space], verify_ssl=VERIFY_SSL, llm_id=LLM_ID + ) + try: + rows = GoodMemRetriever(toolkit).query(seeded[0], top_k=3) + finally: + toolkit.close() + extra = rows[0]["extra_info"] + assert extra["goodmem_partial"] is False + assert seeded[0] in extra["goodmem_abstract_reply"] + + def test_a_missing_llm_keeps_the_hits_and_flags_them(self, space, seeded): + result = self._search(space, seeded, llm_id=NO_SUCH_LLM) + assert result["totalResults"] > 0, "hits were discarded" + assert result["partial"] is True + codes = {s["code"] for s in result["statuses"]} + assert {"NOT_FOUND", "SUMMARIZATION_FAILED"} <= codes + assert result["abstractReply"] is None + assert result["warning"] + + def test_a_failing_provider_keeps_the_hits_and_flags_them( + self, space, seeded + ): + if not FAILING_LLM_ID: + pytest.skip("GOODMEM_TEST_FAILING_LLM_ID is not set") + result = self._search(space, seeded, llm_id=FAILING_LLM_ID) + assert result["totalResults"] > 0 + assert result["partial"] is True + codes = [s["code"] for s in result["statuses"]] + assert "SUMMARIZATION_FAILED" in codes + assert result["abstractReply"] is None + + def test_a_missing_llm_leaves_reranker_scores_alone(self, space, seeded): + if not RERANKER_ID: + pytest.skip("GOODMEM_TEST_RERANKER_ID is not set") + result = self._search( + space, seeded, reranker_id=RERANKER_ID, llm_id=NO_SUCH_LLM + ) + assert result["totalResults"] > 0 and result["partial"] is True + assert all(h["scoreKind"] == "reranker" for h in result["results"]) + assert all(h["score"] == h["rawScore"] for h in result["results"]) + + class TestLiveFilters: def test_a_matching_filter_finds_the_memory(self, space, seeded): toolkit = GoodMemToolkit( diff --git a/tests/test_goodmem_toolkit.py b/tests/test_goodmem_toolkit.py index 4e684fa..5b6cd77 100644 --- a/tests/test_goodmem_toolkit.py +++ b/tests/test_goodmem_toolkit.py @@ -44,6 +44,9 @@ EMB_VOYAGE = "019cfd1c-c033-7517-b7de-f73941a0464b" EMB_QWEN = "019cfd1c-d2a8-7f40-8e6b-91c4a7d3e052" SPACE_2 = "01a0d44b-96ae-7081-bc16-5644e701222a" +LLM = "019cfd9f-0963-76f9-b069-4cde19a64ba8" +#: The id the captured failure fixtures name: well-formed, but no such LLM. +NO_SUCH_LLM = "00000000-0000-7000-8000-000000000000" def fixture(name: str) -> bytes: @@ -973,6 +976,182 @@ def test_a_genuinely_empty_result_does_not_claim_a_problem(self): assert rows[0]["extra_info"]["goodmem_partial"] is False +# --------------------------------------------------------------------------- +# LLM post-processing -- opt-in, developer-set, never a model argument +# --------------------------------------------------------------------------- + + +class TestLlmPostProcessing: + r"""The fixtures are live streams from GoodMem with ``llm_id`` set: a + working LLM, one that does not exist, one whose provider answered 429, + and a working reranker beside an LLM that does not exist.""" + + def test_the_llm_is_sent_in_the_post_processor_config(self): + capture = {} + tk = make_toolkit( + retrieve_handler( + fixture("retrieve_llm_ok.ndjson"), capture=capture + ), + llm_id=LLM, + ) + tk.goodmem_search("What is the fixture canary?") + config = capture["body"]["postProcessor"]["config"] + assert config["llm_id"] == LLM + assert "reranker_id" not in config + + def test_the_llm_sits_beside_the_reranker(self): + capture = {} + tk = make_toolkit( + retrieve_handler( + fixture("retrieve_llm_ok.ndjson"), capture=capture + ), + reranker_id=RERANKER, + llm_id=LLM.upper(), + ) + tk.goodmem_search("q") + config = capture["body"]["postProcessor"]["config"] + assert config["llm_id"] == LLM and config["reranker_id"] == RERANKER + + def test_without_an_llm_the_request_is_unchanged(self): + capture = {} + tk = make_toolkit( + retrieve_handler(fixture("retrieve_ok.ndjson"), capture=capture) + ) + result = tk.goodmem_search("canary") + assert "postProcessor" not in capture["body"] + assert "llm" not in json.dumps(capture["body"]).lower() + assert "abstractReply" not in result + + def test_a_malformed_llm_id_is_refused_before_any_request(self): + from camel_goodmem import GoodMemIdError + + sent = [] + + def handler(request: httpx.Request) -> httpx.Response: + sent.append(request) + return httpx.Response(404) + + for bad in ("not-a-uuid", "", f"../llms/{LLM}", f"{LLM}\n"): + with pytest.raises(GoodMemIdError, match="llm_id must be a UUID"): + make_toolkit(handler, llm_id=bad) + tk = make_toolkit(handler, llm_id=LLM) + tk.llm_id = "not-a-uuid" + with pytest.raises(GoodMemIdError, match="llm_id must be a UUID"): + tk.goodmem_search("q") + assert sent == [] + + def test_an_empty_llm_id_says_how_to_turn_it_off(self): + from camel_goodmem import GoodMemIdError + + with pytest.raises(GoodMemIdError, match=r"pass llm_id=None"): + make_toolkit(retrieve_handler(b""), llm_id="") + + def test_the_answer_is_returned_with_the_hits(self): + tk = make_toolkit( + retrieve_handler(fixture("retrieve_llm_ok.ndjson")), llm_id=LLM + ) + result = tk.goodmem_search("What is the fixture canary?") + assert "ORYX-2290" in result["abstractReply"] + assert result["partial"] is False and result["statuses"] == [] + assert result["totalResults"] == 1 + + def test_an_llm_does_not_relabel_the_scores(self): + tk = make_toolkit( + retrieve_handler(fixture("retrieve_llm_ok.ndjson")), llm_id=LLM + ) + hit = tk.goodmem_search("q")["results"][0] + assert hit["scoreKind"] == "vector" + assert hit["rawScore"] < 0 + assert hit["score"] == pytest.approx(-hit["rawScore"]) + + def test_a_missing_llm_keeps_the_hits_and_reports_both_statuses(self): + tk = make_toolkit( + retrieve_handler(fixture("retrieve_llm_not_found.ndjson")), + llm_id=NO_SUCH_LLM, + ) + result = tk.goodmem_search("q") + assert result["totalResults"] == 1, "hits were discarded" + assert result["partial"] is True + codes = [s["code"] for s in result["statuses"]] + assert codes == ["NOT_FOUND", "SUMMARIZATION_FAILED"] + assert "SUMMARIZATION_FAILED" in result["warning"] + assert result["abstractReply"] is None + assert result["results"][0]["scoreKind"] == "vector" + + def test_a_failing_provider_keeps_the_hits_and_flags_them(self): + tk = make_toolkit( + retrieve_handler(fixture("retrieve_llm_rate_limited.ndjson")), + llm_id=LLM, + ) + result = tk.goodmem_search("q") + assert result["totalResults"] == 1 + assert result["partial"] is True + assert [s["code"] for s in result["statuses"]] == [ + "SUMMARIZATION_FAILED" + ] + assert "429" in result["statuses"][0]["message"] + assert result["abstractReply"] is None + + def test_a_missing_llm_does_not_undo_a_working_reranker(self): + r"""The LLM's NOT_FOUND names ``llm_id``, not the reranker, so the + hits keep their reranker scores.""" + tk = make_toolkit( + retrieve_handler( + fixture("retrieve_reranked_llm_not_found.ndjson") + ), + reranker_id=RERANKER, + llm_id=NO_SUCH_LLM, + ) + result = tk.goodmem_search("q") + hit = result["results"][0] + assert hit["scoreKind"] == "reranker" + assert hit["score"] == hit["rawScore"] > 0 + assert result["partial"] is True + + def test_the_model_still_sees_only_query_and_top_k(self): + tk = make_toolkit(retrieve_handler(b""), llm_id=LLM) + props = tk.get_tools()[0].get_openai_tool_schema()["function"][ + "parameters" + ]["properties"] + assert set(props) == {"query", "top_k"} + + def test_the_search_tool_returns_the_answer_to_the_model(self): + tk = make_toolkit( + retrieve_handler(fixture("retrieve_llm_ok.ndjson")), llm_id=LLM + ) + tool = tk.get_tools()[0] + assert "ORYX-2290" in tool(query="canary", top_k=5)["abstractReply"] + + def test_the_retriever_carries_the_answer_in_extra_info(self): + tk = make_toolkit( + retrieve_handler(fixture("retrieve_llm_ok.ndjson")), llm_id=LLM + ) + rows = GoodMemRetriever(tk).query("What is the fixture canary?") + assert rows and all( + "ORYX-2290" in r["extra_info"]["goodmem_abstract_reply"] + for r in rows + ) + assert float(rows[0]["similarity score"]) > 0 + + def test_the_retriever_reports_a_failed_llm_and_keeps_the_rows(self): + tk = make_toolkit( + retrieve_handler(fixture("retrieve_llm_not_found.ndjson")), + llm_id=NO_SUCH_LLM, + ) + rows = GoodMemRetriever(tk).query("q") + assert len(rows) == 1 and "ORYX-2290" in rows[0]["text"] + extra = rows[0]["extra_info"] + assert extra["goodmem_partial"] is True + assert extra["goodmem_abstract_reply"] is None + codes = [s["code"] for s in extra["goodmem_statuses"]] + assert codes == ["NOT_FOUND", "SUMMARIZATION_FAILED"] + + def test_the_retriever_adds_no_answer_key_without_an_llm(self): + tk = make_toolkit(retrieve_handler(fixture("retrieve_ok.ndjson"))) + rows = GoodMemRetriever(tk).query("canary") + assert "goodmem_abstract_reply" not in rows[0]["extra_info"] + + # --------------------------------------------------------------------------- # helpers # ---------------------------------------------------------------------------