Skip to content

[memory] Align the Hindsight adapter with a real deployment #302

Open

Description

@yusiwen

Context

The Hindsight adapter (agent/memory.go) was written in June against the API as it was then, and has
never been checked against a deployment. Its decoders read content and memory_units; the current
service returns text and items (see hindsight.vectorize.io).
Against a current server, Recall therefore decodes an empty list — a run would report "no memories"
rather than an error, which is the worst of the three outcomes.

Forget returns ErrMemoryDeleteUnsupported without attempting anything, types.Memory.ID is never
filled in by the adapter, and auth() falls back to OPENAI_API_KEY when no memory key is set — so a
user whose only key is OpenAI's sends it to the memory endpoint as a bearer token.

The endpoint itself is no longer implicit: base_url is required and the bank comes from config
(PR #301). What remains is the contract behind that endpoint.

What to build

  • Probe before changing anything. A gated command (or an env-gated test, like the recall benchmark)
    that calls retain, recall and list against a real deployment and prints what came back — paths,
    status, and the field names in the response. The fix has to be aimed at a response, not at a docs page.
  • Align the decoders to what the probe saw: recall results and the listing envelope. A response in
    an unknown shape must be an error, not an empty list: "the server answered something I cannot
    read" and "there are no memories" are different facts.
  • Carry the id. types.Memory.ID is what Forget takes; fill it from the service's memory id so a
    listing can name what it would delete.
  • Make Forget real or explicitly read-only. Hindsight curates with PATCH /memories/{id}
    (invalidate), so either map Forget to that and report what happened, or keep returning
    ErrMemoryDeleteUnsupported and say in the docs that a Hindsight store is append-and-invalidate.
  • Drop the OPENAI_API_KEY fallback. The adapter reads its own credential (the variable the section
    names, else HINDSIGHT_API_KEY), and a missing key is an error before the first request.

Acceptance

  • The gated probe run against a real deployment reports retain/recall/list succeeding, with the response
    shape recorded in the issue's closing comment.
  • A stub serving the current shape yields a non-empty Recall with ids; a stub serving a 200 with an
    unknown shape yields an error that names the shape, not an empty list.
  • Forget(id) either removes/invalidates exactly that fact or returns ErrMemoryDeleteUnsupported —
    never nil on a delete that did nothing.
  • With HINDSIGHT_API_KEY unset and OPENAI_API_KEY set, a request fails with "no memory credential"
    before anything is sent.
  • go test ./... -count=1, -race, go vet, gofmt, staticcheck clean.

Out of scope

Vector search over memory (B3, delivered), the local store (B2), and the rerank stage. This issue is the
remote adapter's contract only.

Where it came from

The remaining slice of #293, split out when that issue was closed: the rest of its work (the config
section, the local store, the composition root and the two commands) is delivered and linked there.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions