Files
shared-memory/docs/memory-api-improvements.md
shadowdaoandClaude Opus 5 6e9628b073 docs: record the memory API decisions, including the one declined
Keeps the reasoning behind PR #19 next to the code, since none of it is
recoverable from the diff: why memory_get stopped returning the embedding
and tsvector, why memory_patch refuses ambiguous matches rather than
picking one, and why memory_append was dropped as redundant with patch.

Also records P3 (mechanising file->memory mirroring) as DECLINED with its
reasoning and, more usefully, the condition that would reopen it — the
mirror going stale again now that patching is cheap. The evidence we had
pointed at edit cost, which P2 fixed; if drift recurs the cause was
attention instead, and the answer is probably to remove the duplication
rather than build a drift detector for it.

Notes two traps for anyone extending this: content_tsv is a generated
column so full-text search cannot rot after a patch (only the embedding
needs recomputing), and the obvious "does search find the patched text"
acceptance check therefore passes on an implementation that skips
re-embedding entirely.

The brief previously lived outside the repo. Moved rather than copied —
two hand-maintained copies is the exact drift problem described in the
document.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-11 15:10:36 -07:00

13 KiB
Raw Permalink Blame History

Decision record: memory read payloads and patch-style updates

Status: P1 shipped · P2 shipped · P3 declined Originated: 2026-08-11, from live use of the deployed server Revised: 2026-08-11, against source Closed: 2026-08-11 — implemented in feat/memory-patch-and-lean-get (PR #19)

This started as a proposal written from black-box observation of the deployed server. It is kept as a decision record because the reasoning behind what was built — and behind what was deliberately not built — is not recoverable from the diff.


0. Orientation

Tools are defined internally with dots (memory.get) and surfaced to clients with underscores (memory_get). Don't let it confuse a grep.

Data model

  • Memories have id (uuid), content, tags[], scope (project | user), projectId, visibility, version, lastEditedBy, createdAt, updatedAt, deletedAt, plus userId, embedding, and contentTsv (apps/web/lib/db/schema.ts).
  • scope: project requires project; scope: user requires it omitted.
  • version is an optimistic-locking token, bumped on every successful update. A stale version returns a concurrent-edit error.
  • Content limit is 64,000 chars (packages/schemas/src/index.ts).
  • Shared projects allow anyone with rw access to edit any memory — hence the locking.

1. P1 — memory_get returned the embedding and the tsvector SHIPPED

Problem. memory_get used a bare select() and returned the raw DB row, including the embedding vector and the contentTsv lexeme index. Both are Postgres retrieval internals with zero value to a model consumer. memory_list and memory_search already projected an explicit 9-field shape — memory_get was the only outlier.

Measured against the live server on a ~13k-char memory:

chars share
content 27,662 44.1%
contentTsv 29,912 47.7%
embedding 4,688 7.5%
other 12 fields 433 0.7%
total 62,695

Two distinct failure shapes, which the original draft had flattened together:

  • embedding is a fixed ~4,690-char tax on every read — 384 dims regardless of content length. Nearly invisible on large memories (7.5%), dominant on small ones (~58% of an ~8k response). Most memories are small.
  • contentTsv scales super-linearly with content (frequent lexemes accumulate long position lists, ~21 chars/entry) and was the largest single component of the large payload — larger than the content itself.

This was a correctness problem, not an efficiency nit. Fetching that memory exceeded the MCP tool-output cap and spilled to a file, even though content alone is comfortably under the limit. memory_get was unusable on exactly the large living documents P2 exists to serve. At the 64,000-char content ceiling a response would land near 140,000 characters, under half of it content.

Shipped: memory_get returns the same 9 fields as memory_list / memory_search. userId is still selected for the authorization check and stripped before responding.

Rejected: an include: ("embedding" | "tsv")[] opt-in. The original draft proposed gating the fields behind a flag in case some caller needed them. No caller can: no MCP client can consume a 384-float vector or a lexeme index, and the Web UI never goes through the MCP tools (it reads via lib/memories.ts and writes via lib/memory-actions.ts). The parameter would have been dead on arrival.


2. P2 — patch-style updates SHIPPED

Problem. memory_update accepted only full replacement. Adding four lines to a 13,000-char living document meant reproducing the entire document.

The evidence this was a real blocker. The WHP roadmap mirror was three weeks and two shipped releases out of date. The agent that noticed declined to fix it, on the grounds that hand-reproducing 13k characters of shared team history to add one entry risked silently dropping some of it — a worse outcome than leaving it stale.

That is the failure mode this was designed against: when the only safe way to make a small edit is expensive, the edit doesn't happen.

Shipped: memory_patch(id, old_string, new_string, version?)

condition behaviour
old_string absent error — never a silent no-op
old_string matches >1 error naming the count — ambiguity never resolves arbitrarily
matches exactly once replace, bump version, re-embed
stale version concurrent-edit error

Both failure modes refuse rather than clobber. That is the property that makes the operation safe to hand to an agent editing a shared document it cannot afford to corrupt. Semantics live in lib/memory-patch.ts as a pure function, free of DB and auth, so both surfaces share them.

Locking came nearly free. memory.update already computed expectedVersion = version ?? existing.version, falling back to the version read in the same handler. The CAS therefore already guarded the server-side read-modify-write; patch copies that shape and is race-safe even when the caller omits version. No explicit transaction was required.

Rejected: memory_append with a section parameter. Proposed as sugar for heading-structured logs. Patch already covers that case exactly — memory_patch(id, "## RECENTLY SHIPPED", "## RECENTLY SHIPPED\n- entry") — and does so with the uniqueness guarantee: if the heading appears twice you get an error instead of an arbitrary insert. Implementing section would have meant defining heading-match semantics, insert position within a section, and duplicate-heading behaviour, for something patch handles for free.


3. P3 — deriving mirrors rather than relying on convention DECLINED

The problem as stated. Some memories mirror local files. The sync is enforced only by a note in the file's own header: "MIRRORED to shared-memory MCP … when you update this file, also memory_update that record." That depends on whoever edits the file noticing the note and performing a second write. It went three weeks without one.

Proposed shapes were: a server-side memory_sync_from_file, a staleness signal via sourcePath + content hash, or leaving it manual but cheap via P2.

Declined, 2026-08-11. Three reasons, in order of weight:

  1. The cause we have evidence for is now fixed. The evidence was specific: an agent noticed the drift and declined to fix it because the edit was expensive and risky. That is a cost failure, not an attention failure. P2 makes that edit a single call. We have direct evidence for the cost cause and none yet for any other.

  2. It would likely be a mechanism for N=1. One mirror is known to exist. A sourcePath column, hash computation, and staleness plumbing is real schema-and-sync work; building it to police a single document is disproportionate.

  3. If drift recurs, the better fix probably isn't sync machinery. P3 assumes the mirror should exist and be kept honest. That assumption deserves scrutiny first: the memory is a condensed prose rendition of a file that lives in a container, existing separately only because the memory is cross-machine and the file is not. Two hand-maintained sources of truth plus a drift detector is strictly more machinery than one source of truth. The cheaper answer would be to remove the duplication — make the memory canonical and drop the file, or generate one from the other.

What would reopen this: the roadmap going stale again now that patching is cheap. That is the clean experiment and it costs nothing to run. If it drifts again, the cause was attention rather than cost, and option 2 above — a staleness signal that makes drift visible rather than trying to fix it automatically — becomes worth its weight.

Note for whoever picks this up: the mirror is not a byte copy of its local file. It is a condensed prose rendition with different headings and no wiki-links. A naive file-sync would destroy its established form. Any solution has to preserve that distinction or deliberately abandon it.


4. Structural work this depended on SHIPPED

Neither of these was in the original proposal; both were found once the source was available.

4.1 The write path was duplicated

updateMemoryAction (Web UI) and the memory.update MCP handler each reimplemented authorize → mutate → re-embed → CAS → audit. Neither delegated to a shared helper, and they had drifted: memory.delete over MCP skipped the project ACL whenever the caller authored the row, so a memory written while a share was rw stayed deletable by its author after an owner downgraded that share to ro. memory.update and the entire Web UI always checked.

Both surfaces now route through lib/memory-mutations.ts. Authoring a row grants no standing write privilege on any path — the project ACL is the authority, not the byline. This was a behaviour change, shipped deliberately, and is covered by a test that fails against the old code.

The one genuine difference between the surfaces is injected as a ProjectResolver: MCP refuses an unknown project key (call project.identify first) so an agent cannot spawn near-miss projects off a typo, while the Web UI creates one, because a person typing a name into a form means to.

4.2 There was no test infrastructure

No vitest, no jest, no test files, no test script. Added vitest, with integration tests running against a real Postgres rather than a mocked DB. Setup is documented in the README.


5. Invariants — preserve these

  • Optimistic locking. version must keep working, and every new mutating primitive must accept it. Shared projects have concurrent editors.
  • Scope/project rules. scope: projectproject required and must already exist on the MCP path; scope: userproject omitted.
  • Re-embedding on content change — and note what this does NOT cover. content_tsv is GENERATED ALWAYS AS (to_tsvector('english', coalesce(content, ''))) STORED (apps/web/drizzle/0000_init.sql), so Postgres maintains it and full-text search cannot rot. Only embedding requires an explicit recompute. Any new mutation path must re-embed, or semantic retrieval degrades silently while FTS keeps working — which is exactly what makes the failure hard to notice.
  • Stable id across edits. Never implement an edit as delete + recreate.
  • 64,000-char content limit enforced after a patch is applied, not just on the incoming fragment.
  • Audit trail. Partial edits record match offset and length delta, not just the changed field names.

6. Acceptance checks

P1 — verified in tests; the payload figures need a deploy to confirm.

  • Response contains neither embedding nor contentTsv.
  • Field set matches memory_list / memory_search exactly (9, down from 14).
  • Large specimen drops 62,695 → ~28,100 chars (55%); small specimen ~75%.
  • memory_get on aaea192c-edce-4372-b011-5113a02dea16 returns inline instead of spilling to a file. This is the check that matters — it is the difference between the tool working and not working on large memories.

P2 — all verified against a real Postgres.

  • Unique old_string → applied; version incremented by exactly 1.
  • Absent old_string → error; content byte-identical afterward.
  • old_string occurring twice → error naming the count; content unchanged.
  • Stale version → concurrent-edit error; content unchanged.
  • Content exceeding 64,000 chars post-patch → rejected.
  • Re-embedding — see the caveat below.

A trap in the re-embedding check. The original draft proposed "after a patch, memory_search finds text introduced by that patch." That test does not work. Search fuses three rankers via RRF, and because content_tsv is a generated column the FTS ranker finds the literal inserted text even if the patch skipped re-embedding entirely — it would pass on a broken implementation. The shipped test asserts the stored vector changed, using a deterministic per-text embedder stub. A separate test pins that content_tsv updates itself, documenting why the naive check is misleading.


7. Test specimen

Memory aaea192c-edce-4372-b011-5113a02dea16 (project cloud-hosting-platform/whp, tags roadmap / progress-tracker / planning) is a good real-world subject: ~13k chars, heading-structured, and its local counterpart is /home/claude/.claude/projects/-workspace/memory/project_roadmap.md.

It is live team data. Check its updatedAt and version before using it as a fixture, and prefer a scratch memory for destructive tests.