Skip to content

camel-goodmem 0.2.0: official SDK, retrieval status contract, native retriever - #1

Merged
bashareid merged 3 commits into
mainfrom
audit/0.2.0-sdk-rewrite
Sep 24, 2026
Merged

bashareid merged 3 commits into
mainfrom
audit/0.2.0-sdk-rewrite

Conversation

@bashareid

Copy link
Copy Markdown
Collaborator

Audit and rewrite of camel-goodmem against the P1–P38 checklist.

Everything below was reproduced against the published wheel — camel-goodmem 0.1.0 on PyPI, verified byte-identical to b12a805 — running live against GoodMem server v1.0.320, not inferred from reading the source.

The headline

goodmem_update_space sent publicRead. The server removed that field and answers:

HTTP 400 {"error":"Invalid JSON request body: Unrecognized field \"publicRead\"
          (class com.goodmem.rest.dto.UpdateSpaceRequest)"}

metadata_filter was a raw filter expression supplied by the model. Live, a model could widen its own scope — the intended tenant = 'acme' filter returned 1 hit, a self-widened one returned 2 — and an ordinary apostrophe in a value was a 400.

A search with an unavailable reranker returned success: true with no indication of trouble, while the server had sent NOT_FOUND, FEATURE_DISABLED and RERANKING_FAILED.

A PDF's content came back as raw bytes, which is not JSON-serialisable — that tool result could never reach a model.

The 71 existing tests passed against every one of these. They assigned a MagicMock to the toolkit's _session, so they asserted that the code calls the methods it calls. There was no CI: only a publish workflow, so 0.1.0 shipped without a test ever running.

Measured before/after

Scenario 0.1.0 (published) 0.2.0
update_space(public_read=True) HTTP 400, Unrecognized field "publicRead" argument does not exist
Model-supplied filter widened its own scope, 1 hit → 2; apostrophe → 400 developer-set, escaped; injection payload → 0 hits
Search, broken reranker success=true, no status partial=true + the server's own messages
PDF content raw bytes, not JSON-serialisable base64 str, byte-identical
file_path=/etc/hostname read and uploaded byte-identical refused, as are .. and symlink escapes
Empty-space search 11.63 s 0.29 s
Retrieval tool surface 13 model arguments; delete_space always exposed goodmem_search(query, top_k)
Score raw -0.6236 score=+0.6236, rawScore, scoreKind
Rejected create 400 Client Error: Bad Request the server's field-level message + status
Listings first page only paginated
Timeouts 0 of 12 calls on the client
Retriever none GoodMemRetriever(BaseRetriever)

Full transcript: camel-goodmem-before-after.txt in the audit notes.

Tests

  • 69 offline — the real SDK over an httpx mock transport, fed NDJSON captured from v1.0.320. A guard test asserts the fixtures are server bytes.
  • 29 live — skip entirely without credentials, which is also the check that no key is baked in. Teardown verified against a fresh server inventory.
  • CI — lint, types, offline suite, skip check, clean-environment install, plus gates for a committed key and disabled TLS.

Deliberately not done

  • No AgentMemory implementation: CAMEL's AgentMemory is chat history with a context-window policy, GoodMem is a document store with server-side embedding. Implementing it would fake one side of the contract.
  • GoodMemToolkit moved from camel_goodmem.goodmem_toolkit to camel_goodmem.toolkit; the package-level import from camel_goodmem import GoodMemToolkit is unchanged.

🤖 Generated with Claude Code

bashareid and others added 3 commits September 24, 2026 20:31
…retriever

Rewrites the package against the P1-P38 checklist. Every defect below was
reproduced against the PUBLISHED wheel (camel-goodmem 0.1.0 on PyPI, verified
byte-identical to b12a805) running live against GoodMem server v1.0.320.

Reproduced and fixed:

- P1  hand-rolled `requests` client -> official `goodmem` SDK
- P2  `publicRead` was sent by goodmem_update_space, and the server answers
      `400 Unrecognized field "publicRead"`. The SDK's own UpdateSpaceRequest
      has no such field; the argument is gone
- P3  a malformed NDJSON line was swallowed and the search reported success;
      now what arrived is kept and reported as MALFORMED_STREAM
- P4  `status` events had no branch in the parser: a search with a broken
      reranker returned success=true with no status, while the server had sent
      NOT_FOUND, FEATURE_DISABLED and RERANKING_FAILED. Now the Q1/Q3/Q4a/Q4b
      contract, with `partial` + `statuses`
- P5  `wait_for_indexing` defaulted on and was model-controllable: 11.63s on an
      empty space, 0.33s without. The read path no longer polls
- P6  listings returned page one; the server's `nextToken` was never read
- P7  the server's field-level errors were replaced by "400 Client Error"
- P10 `file_path` was a model-facing argument with no restriction and read
      /etc/hostname byte-identical. Uploads now require an `upload_dir`
- P12 a failed content fetch set `contentError` and left `success: true`
- P16 a PDF's content came back as raw `bytes` -- not JSON-serialisable, so
      that tool result could never reach a model. Now text as text, anything
      else base64
- P15/P30 chunks and memories were two arrays joined by positional
      `memoryIndex`; now joined by UUID, de-duplicated by chunk id
- P17 none of the 12 HTTP calls carried a timeout
- P19/P34 `metadata_filter` was a raw expression string supplied by the MODEL.
      Live, the model widened its own scope (1 hit -> 2) and an ordinary
      apostrophe in a value was a 400. Filters are now developer-set and built
      by `camel_goodmem.filters`, with the escaping the server accepts and
      casts matching the stored type
- P28 retrieval showed the model 13 arguments including `poll_interval` and
      `llm_temperature`; `delete_space` and `update_space` were always in the
      toolset. The model now sees `goodmem_search(query, top_k)` plus an
      optional write; admin, delete and upload are opt-in
- P29 the threshold was documented "Minimum score (0-1)" and raw negative
      vector scores were passed through. Scores are oriented higher-is-better
      with rawScore/scoreKind kept, reranker scores are never negated, and a
      threshold is reranker-only and warns when it empties the result
- P21/P36 the API key was a public attribute; it is now private and absent
      from repr
- P32 reusing a space name silently accepted a different embedder. Reuse now
      requires a match; a mismatch is an error naming both
- P35 there was no retriever, so GoodMem could not be used with CAMEL's RAG
      paths. Adds GoodMemRetriever(BaseRetriever)

Tests: 71 pre-existing tests passed against every defect above; they assigned
a MagicMock to the toolkit's `_session`. Replaced with 69 offline (the real SDK
over a mock transport, fed NDJSON captured from v1.0.320) plus 29 live, which
skip without credentials and verify teardown against a server inventory.

CI added: this repo had only a publish workflow, so 0.1.0 shipped without any
test ever running. CI now runs lint, types, the offline suite, the skip check,
a clean-environment install, and gates for a committed key and disabled TLS.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
camel-ai's BaseToolkit does `from mcp.server import FastMCP`, which mcp 2.x
moved, and camel-ai only requires `mcp>=1.3.0`. So `pip install
camel-goodmem==0.1.0` into a clean environment today resolves mcp 2.2.0 and
raises ImportError on `import camel_goodmem` -- the published package is
unusable for any new user, independently of every other defect in this branch.

Verified: a clean venv with 0.1.0 fails to import; the same venv with this
branch resolves mcp 1.30.0 and imports. CI's clean-environment install step
covers it from here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shipped example still called the 0.1.0 method names
(goodmem_list_embedders, goodmem_create_space), handed every tool to
every agent and let the model pick the space, and asked the model to
compose a raw CAST(val(...)) filter string -- the exact P28 and P34 shapes
the rewrite removed. It also never deleted the three spaces it created.

Now: one admin toolkit for setup and teardown, a toolkit scoped per space
for each agent, scenario 3 uses a developer-set metadata_filter, and
cleanup verifies against a fresh listing. Every call is checked against
the real method surface.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@bashareid
bashareid merged commit 14409c3 into main Sep 24, 2026
7 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