Skip to content

feat(retrieval): opt-in llm_id for GoodMem's LLM answer (0.3.0) - #3

Merged
bashareid merged 1 commit into
mainfrom
feat/llm-post-processing-2026-09-29
Sep 29, 2026
Merged

bashareid merged 1 commit into
mainfrom
feat/llm-post-processing-2026-09-29

Conversation

@bashareid

Copy link
Copy Markdown
Collaborator

Summary

Adds an opt-in, developer-set llm_id to GoodMemToolkit, so GoodMem runs one of its LLMs over the retrieved chunks and returns a grounded answer beside the hits. It works like reranker_id: you set it at construction, and the model's tool goodmem_search(query, top_k) does not change.

  • Config: GoodMemToolkit(..., llm_id="<uuid>"). It is checked with the package's single id validator (require_uuid) at construction and again at use. A non-UUID raises GoodMemIdError and nothing is sent. llm_id="" is refused with a hint to pass llm_id=None, the same way reranker_id handles it.
  • Request: the id goes in the retrieval post-processor config (postProcessor.config.llm_id), next to reranker_id when both are set. If llm_id is unset, the request body is byte-for-byte unchanged (the test checks there is no postProcessor).
  • Response: goodmem_search returns the answer as abstractReply, which is also the tool result the model reads. GoodMemRetriever.query() puts it in each row's extra_info as goodmem_abstract_reply. CAMEL's retriever returns a plain list of rows, so the answer is repeated on every row. Both keys are present whenever an LLM is configured, set to None if the LLM failed, and absent otherwise. No new public type is added: RetrievalOutcome.abstract_reply already existed and its parser already handled the event, but nothing ever requested it.
  • Status contract: when the LLM fails, the hits are kept, partial is set, the statuses and a warning are surfaced, and nothing is raised. FEATURE_DISABLED and LLM_CAPABILITY_INFERRED remain informational.
  • Scores: unchanged. An LLM does not rerank. With a working reranker next to a missing LLM, the hits keep their reranker scores: the LLM's NOT_FOUND names llm_id, not the reranker, so the reranker-fallback heuristic does not trip.

What failed before, and how it behaves now

Measured against the local GoodMem server (OpenRouter qwen/qwen3-8b LLM). The "before" rows used the published camel-goodmem==0.2.1 from PyPI.

Case Before (0.2.1 / main) After (this branch)
GoodMemToolkit(llm_id=...) TypeError: GoodMemToolkit.__init__() got an unexpected keyword argument 'llm_id' Accepted. Sent as postProcessor.config.llm_id
Answer from the search No way to get one. Result keys were partial, query, resultSetId, results, statuses, success, totalResults, with no abstractReply abstractReply: "CAMEL is a framework for building communicative multi-agent systems, as indicated by the retrieved data. Its agents work together through role-playing and dialogue …". The hits are unchanged (scoreKind: vector, score == -rawScore)
Retriever No answer extra_info["goodmem_abstract_reply"] on every row
LLM id that does not exist Could not be requested partial: True, statuses [NOT_FOUND, SUMMARIZATION_FAILED], hits kept, abstractReply: None, no exception
Provider failing (OpenAI LLM out of credits) Could not be requested partial: True, [SUMMARIZATION_FAILED] with the provider's 429 in the message, hits kept
Reranker + missing LLM Could not be requested Hits keep scoreKind: reranker (score == rawScore), and partial is set
Malformed llm_id (../llms/<id>, "", <uuid>\n, …) Could not be passed GoodMemIdError naming llm_id. The recording server receives nothing
What the model sees goodmem_search(query, top_k) Same: {query, top_k}

On main, 47 of the 51 new offline tests fail, all at TypeError: ... unexpected keyword argument 'llm_id'. The other 4 are controls that check nothing changes when llm_id is unset.

Tests

Suite Before After
tests/test_goodmem_toolkit.py (real SDK over a mock transport) 86 101
tests/test_goodmem_ids.py (real SDK against a recording server) 380 416
tests/test_goodmem_live.py (live) 30 35
  • New offline fixtures are live NDJSON streams captured today for each case: a working LLM, a missing LLM, a provider 429, and a reranker next to a missing LLM.
  • New live tests: TestLiveLlm. They need GOODMEM_TEST_LLM_ID; GOODMEM_TEST_FAILING_LLM_ID and GOODMEM_TEST_RERANKER_ID are optional, and each test skips without its id.
  • Run locally, exactly as CI does:
    • ruff check and ruff format --check clean; mypy camel_goodmem clean.
    • Offline: 517 passed.
    • Live tests skip without credentials: 35 skipped.
    • Build, twine check and an import in a clean venv pass.
    • Floors job (uv --resolution lowest-direct on Python 3.10: camel-ai 0.2.79, goodmem 0.1.35, pydantic 2.11.0, mcp 1.3.0): 517 passed.
  • Live: 35/35 passed. Every space created was deleted, confirmed against a fresh inventory.

Release

The version goes to 0.3.0 (a minor bump for a new opt-in feature) in pyproject.toml and __version__. This repo does not release on merge: publish.yml publishes only when a v* tag is pushed. Merging this PR publishes nothing; release 0.3.0 is a separate, deliberate tag.

The README has a new "LLM answers" section (where the answer appears, that it is opt-in and developer-set, and what happens when the LLM fails), plus a "Changes in 0.3.0" table. The README is where this repo keeps its changelog.

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@bashareid
bashareid merged commit 7169900 into main Sep 29, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant