Skip to content

feat: Phase 4 — memory_update, typed relations, hybrid RRF search - #6

Merged
hermanngeorge15 merged 42 commits into
mainfrom
feat/phase-04-update-relations-hybrid-search
Oct 25, 2026
Merged

hermanngeorge15 merged 42 commits into
mainfrom
feat/phase-04-update-relations-hybrid-search

Conversation

@hermanngeorge15

Copy link
Copy Markdown
Contributor

Closes #5.

Milestone v1.1 (ships as product v0.1.0), Phase 4. 42 commits, one per task, intended for a rebase merge so the per-plan history survives.

What changes

  • memory_update (MCP) and PATCH /api/memories/{id} (REST): merge-patch content, tags, scope and TTL. A content edit re-mirrors FTS and re-embeds in the same writer transaction; if Ollama is down, the row falls back to embedding_status = 0 and the sweep backfills it. Unknown and immutable fields are refused.
  • Relations: memory_link / memory_unlink with flat typed kinds. relates_to is symmetric (canonical storage, unlink from either end). Links cascade on forget and TTL sweep. expand_links adds up to 10 one-hop related summaries on search and list.
  • Migration 0003 with hygiene: 0001/0002 SQL frozen and checked against a v0.0.1 fixture database, a pre-migration backup that is safe under concurrent first open, and a friendly error when a newer database meets an older binary. The README gains an Upgrading section.
  • Hybrid search: RRF (k=60) over the keyword and semantic legs, with decay applied after fusion. With Ollama unreachable it degrades to keyword exactly as in v1.0, and the envelope is unchanged.
  • Forgiving keyword leg: the raw FTS5 query runs first, then one sanitized retry. Queries like sqlite-vec or auth: tokens no longer return 500, and search errors never expose SQLite text.

Behaviour changes callers can see

  • memory_list / GET /api/memories with expand_links=true and no limit → invalid_params / 400 (WR-04, option a). Pass limit (1–200).
  • Ordinary punctuation in a search query now returns results instead of a 400.

Verification

  • cargo fmt --all -- --check: clean
  • cargo clippy --workspace --all-targets --locked -- -D warnings: clean
  • cargo test --workspace --locked: 217 passed, 0 failed, 2 ignored. Both ignored tests predate this phase. I re-ran this locally at the PR head.
  • GSD phase re-verification: passed 28/28. It was 13/15 before the gap-closure plans 04-05 to 04-07.
  • No unwrap() added to production code.
  • Docs updated: README.md (Upgrading section, relations, hybrid search, query rules), rustdoc, and the MCP tool descriptions.
  • ADR: none. This repo keeps its decisions in .planning/ (04-CONTEXT.md and the STATE.md decisions).

Known, deferred (recorded in 04-REVIEW.md)

  • Errors from update, link and forget can still include raw SQLite text; redaction is applied only to search errors.
  • With Ollama down, a question without punctuation is matched as an AND of every word.
  • The pre-migration backup has no cross-process lock, which would need a new dependency. Instead, the backup is locked within a process and an open tolerates a concurrent upgrade.
  • IN-05 (updating an expired but not yet swept memory) and IN-06 (a snippet can split an emoji) are documented only.

🤖 Generated with Claude Code

hermanngeorge15 and others added 30 commits October 23, 2026 19:52
…ample

- WR-02: add MemoryError::Internal; all six writer-lock sites map poisoning
  to Internal("writer lock poisoned") -> REST 500 / MCP internal_error
- delete MemoryError::NotFound (zero producers) and the now-unconstructable
  ApiError::NotFound; not-found stays data (forget's Ok(false))
- CR-01: Store::exists takes Option<&str> and probes with NULL-safe
  `source IS ?1`; import no longer coerces None to "" (regression test)
- WR-01 partial: MAX_KNN_K = MAX_LIMIT * 4 keeps 4x oversample at every limit
- mapper tier tests on both transports; exhaustive arms, no catch-alls

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efusal

- sql/0003_relations.sql: directed memory_links (PK from/to/kind, FK
  ON DELETE CASCADE x2, CHECK from!=to, WITHOUT ROWID, idx_links_to)
- migrations.rs: append 0003, LATEST_SCHEMA_VERSION = 3 (+ chain test)
- SqliteStore::open: user_version probe before any write -> SchemaTooNew
  (names both versions + brew upgrade hint, DB byte-untouched); sibling
  online backup {db}.backup-pre-0003-{YYYYMMDD} before migrating a
  non-empty older DB, one per schema version; backup failure aborts open
- MemoryError::SchemaTooNew classified internal (startup-only) in both
  exhaustive mappers
- rusqlite gains only the `backup` feature; 0.39 pin unchanged (AR-03-01)
- tests/migration_hygiene.rs + committed binary fixture v0.0.1.db
  (generated once from tag-identical 0001+0002): freeze, backup, retention,
  fresh-no-backup, too-new refusal, backup-failure abort

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- MemoryPatch (double-Option merge-patch) + UpdatedMemory (flattened view +
  embedding_status, D-10) + validate_patch at the service seam (D-08)
- Store::update: one writer tx, UPDATE in place (never delete+insert),
  freshness bump folded into the same statement (D-09), same-tx vec_memories
  invalidation + re-embed, outage falls back to embedding_status = 0
- MemoryService::update: validate -> clock -> embed only when content is
  patched -> spawn_blocking; unknown id is Ok(None)
- serde_util::deserialize_explicit_null shared by both transports
- memory_update MCP tool; not-found mirrors memory_forget's status shape;
  tool schema advertises no `default: null` (null means "clear")
- tests/update.rs: re-embed kill-test, outage + sweep backfill, freshness
  bump, response shape, not-found, no-embed tags patch, validate-before-embed

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ansports

- UpdateBody REST DTO (serde-only, same explicit-null helper as UpdateArgs)
  + update_handler: 200 UpdatedMemory, 404 {"id","updated":false,
  "reason":"not_found"}; body-extractor rejections reported as 400
- PATCH route on /api/memories/{id} beside DELETE
- Matrix tests from raw JSON bodies: ttl_secs/tags/scope x absent/null/value
  on REST (through axum's Json extractor) and MCP (UpdateArgs ->
  MemoryPatch states + tool-level effects), plus a TestClock core matrix;
  ttl null survives a sweep past the old TTL
- content null/empty, out-of-bounds ttl, empty patch, malformed bodies:
  400 / invalid_params, never 500, never clamped
- README: memory_update section (merge-patch table, embedding_status,
  freshness note) and the PATCH endpoint

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…TL sweep (MCP-07)

- LinkKind closed enum (relates_to | supersedes | caused_by) with TryFrom
  returning InvalidArgument naming the allowed set (D-01); LinkOutcome
  { Linked, NotFound { missing_id } } so a missing endpoint is data (D-02)
- Store::link pre-checks both endpoints in the writer tx, then
  INSERT OR IGNORE: duplicates are idempotent Linked; never an FK 500
- Service rejects self-links at the seam before any store hop
- forget() and sweep_expired() gain belt-and-braces orphan-edge deletes
  beside the FK ON DELETE CASCADE (T-04-12)
- memory_link / memory_unlink MCP tools; POST /api/links and
  DELETE /api/links/{from_id}/{to_id}/{kind} (200/404 dual status, 400 on
  any bad body/kind/self-link)
- tests/relations.rs (10) + MCP (4) + REST (4, incl. a real-HTTP router test)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-07)

- MemoryView gains related: Option<Vec<RelatedLink>> with
  skip_serializing_if, so flag-less envelopes stay byte-identical to v1.0
  (golden-tested for search and list)
- RelatedLink {id, kind, direction, type, snippet, tags}: incoming edges
  read their natural reverse label (superseded_by, caused); snippet cut at
  160 chars on a char boundary with a trailing ellipsis (D-05)
- Store::related: one read-pool query per anchor following edges both
  ways, newest first, LIMIT MAX_EXPANDED_LINKS = 10 (D-07)
- Service expands AFTER truncation and AFTER spawn_bump(returned ids) on
  both search legs and list; neighbors are never bumped (D-06)
- expand_links (serde default false) on MCP search/list args and REST
  POST /api/search body + GET /api/memories query
- README: relations section, expansion shape, bounds table

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-fusion decay (SEARCH-04/05)

- Store::keyword_candidates: bm25-ordered keyword leg (ORDER BY bm25 ASC, id ASC),
  no decay term, oversampled limit*4 capped at MAX_KNN_K, same filters and
  map_fts_query_error routing as search; public Store::search untouched
- pub const RRF_K = 60.0 and pub fn rrf_fuse: 1-based ranks, duplicate ids sum,
  total_cmp descending with ascending-id tie-break
- Embed-OK arm runs knn_search + keyword_candidates concurrently (tokio::join!),
  propagates leg errors (keyword first), fuses, multiplies by pinned-aware decay
  once, truncates, bumps only returned ids, then expands
- SearchMode::Hybrid replaces the now-unproduced SearchMode::Semantic
- tests/hybrid.rs: hand-computed RRF, e2e fusion, both-leg oversample recall,
  SEARCH-05 golden (fresh beats stale at equal rank; pinned, once, multiplicative)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e and wire docs (SEARCH-04/05)

- tests/hybrid.rs: embed error and empty embed batch degrade to output
  byte-identical to a direct Store::search; malformed MATCH (unbalanced quote,
  dangling AND, bare '?') is InvalidQuery in hybrid exactly as in keyword mode;
  a broken FTS or vec leg propagates a Sqlite error instead of serving the other
  leg; 20 candidates / limit 5 bumps exactly the 5 returned rows; hybrid
  expansion never bumps neighbors
- MCP + REST in-process tests: "hybrid" with a live FakeEmbedder, "keyword"
  with a dead one, identical {search_mode, results} envelope; malformed query
  is invalid_params / 400 in hybrid mode
- memory_search tool description: 'hybrid'|'keyword', RRF k=60, keyword degrade,
  FTS5 query syntax, filter-after-cut bound (WR-01), returned-only bumps
- README: search-mode table, behavioral change "semantic" -> "hybrid" for the
  v0.1.0 release notes, bump scope and query-syntax notes
- agent-memory dev-dependency enables core's test-clock feature (test builds only)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… complete

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ace, review fixes)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…try, redacted search errors

Implements user decision option-a (2026-10-23) at the shared store seam:

- sqlite.rs: is_match_input_error (SQLITE_ERROR + FTS5 query-parse markers,
  incl. the step-time `no such column` class from hyphens/colons),
  sanitize_fts_query (word tokens quoted and OR-joined, None when tokenless)
  and match_with_sanitized_retry, used by both Store::search (keyword
  degrade) and Store::keyword_candidates (hybrid leg). SQL strings unchanged.
  The marker-only map_fts_query_error is removed.
- service.rs: SEARCH_STORAGE_FAILURE + redact_search_error (exhaustive);
  search() = search_unredacted().map_err(redact_search_error), so no SQLite
  text reaches any search response body (T-04-19).
- Old 400-pinning tests reversed in the same commit (hybrid.rs, fallback.rs,
  mcp.rs, handlers.rs, tests/rest.rs); sqlite-vec and a punctuated question
  recall the target memory in both modes.

Closes VERIFICATION truths #14/#15, REVIEW CR-01/CR-02 (T-04-15 revised).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… table, raw-first proofs

- sqlite.rs unit tests: all 9 candidate erroring probes (empty, `what is it?`,
  `"unbalanced`, `AND`, `sqlite-vec`, `auth: tokens`, `*`, `NEAR(a b, x)`,
  3000-deep parens) error in the bundled SQLite 3.51.3 and are classified;
  the 5 valid-raw probes run Ok; every sanitized fallback parses; classifier
  negatives (SQLITE_BUSY, `malformed JSON`, FromSqlConversionFailure);
  sanitize_fts_query table.
- hybrid.rs: shared corpus M1-M5 decision table (7 targeted + 4 tokenless rows)
  in both modes; structured_fts5_syntax_still_runs_raw (exact id sets);
  keyword-mode broken-FTS redaction to SEARCH_STORAGE_FAILURE.
- domain.rs: InvalidQuery documented as no longer produced by search.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…docs and STATE decision

- mcp.rs: memory_search description and SearchArgs.query schema text now say
  the query may be plain natural language or FTS5 syntax and is never
  rejected for its syntax; description test asserts the new wording and the
  absence of invalid_params. New in-process two-mode decision-table test over
  the shared corpus and a broken-FTS redaction test (internal_error, no
  SQLite text).
- handlers.rs: same two-mode table through search_handler (200 everywhere)
  and a broken-FTS 500 redaction test.
- tests/tools.rs: real-binary stdio case — sqlite-vec and a punctuated
  question both recall the stored memory in keyword mode.
- README: "never rejected" search bullet, REST storage-failure sentence,
  tools-table row, Behavioral changes item 3 rewritten (v0.0.1 400 and
  sqlite-vec 500 cases both answered now).
- STATE.md: 04-05 T-04-15 revised (option-a) decision entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- STATE: position Plan 5 of 7, executing gap closure (04-06 next), P05 metric,
  session; the option-a decision entry landed with the Task 3 commit
- ROADMAP: 04-05 checked, phase 04 progress 5/7 (In Progress, not complete)
- SEARCH-04 already complete from 04-04; REQUIREMENTS.md unchanged

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… forgotten row (CR-03)

- Store::insert_embedding(id, expected_content, vec) -> Result<bool>: a guarded
  claim (embedding_status = 0 AND content = ?2) runs first in the writer tx;
  no claim, no vector write
- sweep passes the exact embedded content back and logs backfilled +
  skipped_stale; SweepReport shape unchanged
- tests/backfill_race.rs: 5 store-level interleavings (happy path, embedded
  update, failed update, forget + rowid reuse, double sweep) and 2 gated
  service-level sweep tests (tokio Notify, no sleeps)
- README Lifecycle: an edited/deleted memory is never overwritten by an
  in-flight backfill

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… unlink from either end (WR-03)

- LinkKind::is_symmetric / canonical_endpoints (exhaustive matches)
- SqliteStore::link keeps the request-order endpoint pre-check, then inserts
  the canonical pair: relates_to as (min, max); directed kinds unchanged
- SqliteStore::unlink: one bound DELETE matching the requested orientation,
  plus the reversed one for symmetric kinds (also removes pre-fix dev rows)
- Store link/unlink docs; IN-05 (update on an expired-but-unswept row) and
  IN-06 (snippet may split a grapheme cluster) documented, no behaviour change
- MCP memory_link / memory_unlink descriptions and arg docs state symmetry;
  tests lock request-order echo on MCP and REST and the description text
- tests/relations.rs: 8 WR-03 tests (unit, both-ways, canonical row, either-end
  unlink, pre-fix backstop, directed kinds, request-order not-found)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…xpanded-list bound

- README Relations: relates_to symmetric (b->a after a->b is a no-op), unlink
  from either end, responses echo the request ids, relates_to direction
  carries no meaning, snippet may split a combined emoji/accent sequence,
  unlimited expanded memory_list note (pass a limit on large stores)
- README memory_update: an expired-but-unswept memory can still be updated
  and its TTL extended or cleared (IN-05)
- MAX_EXPANDED_LINKS doc: the 200 x 10 bound covers search and
  list-with-limit only; a list without a limit returns every row (WR-04 doc
  correction, behaviour change deferred)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… and REST (WR-02)

- #[serde(deny_unknown_fields)] on mcp::UpdateArgs and rest::UpdateBody only;
  the [03-01] decision stands for the v0.0.1 DTOs
- schema test now also pins additionalProperties == false
- stdio e2e: type / ttl_sec are refused as an rmcp tool error result
  (isError, 'failed to deserialize parameters: unknown field'), row untouched
- REST: PATCH with type/source/ttl_sec is a 400 naming the field, nothing written
- README memory_update note and STATE decision

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hermanngeorge15 and others added 12 commits October 23, 2026 22:06
…ody (IN-02)

- forget/update/unlink take Result<Path, PathRejection>, list takes
  Result<Query, QueryRejection>, mapped through extractor_rejection
- status tier preserved: 4xx parse failures are 400 {"error"}, a 5xx
  routing misconfiguration stays a 500
- real-HTTP test: 5 malformed requests are 400 application/json with an error
- README: every REST client error carries {"error": ...}

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…erant open (WR-01, IN-03, IN-01)

- busy_timeout set first on the writer (before the D-13 probe) and in apply_pragmas
- pre_migration_backup(src, path, expected_version) -> Option<PathBuf>: copy to a
  pid-qualified temp outside the retention prefix, verify the copy's user_version,
  rename into place, only then prune older same-version backups; a copy taken
  after a concurrent migration is discarded, never kept as pre-migration
- migrate_tolerating_concurrent_upgrade absorbs a to_latest failure only when a
  re-probe shows LATEST_SCHEMA_VERSION
- seam tests (discard, replace-then-prune, failed temp write, failed rename,
  closure seam, real lost-race errors) and a 10-round two-thread first-open
  smoke test on a temp copy of the v0.0.1 fixture
- README: Upgrading section (backup name, retention, restore, newer-schema refusal)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ackup gap-closure plan (summary)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… closure

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
257+ chained NOTs exceed SQLITE_FTS5_MAX_EXPR_DEPTH (256) and fail with
"fts5 expression tree is too large", which carries no "fts5: " prefix and
so fell through to a generic 500 / internal_error. Add it to
FTS5_QUERY_PARSE_MARKERS so it takes the single sanitized retry like every
other FTS5 parse error. The retry quotes each token and joins them with
OR, a flat node, so it cannot hit the same limit.

Tests, against the bundled SQLite: NOT x257 and x300 join the classifier
and fallback-parses probes; a boundary test pins 256 as accepted, 257 as
the classified depth error, and a 3000-NOT chain's fallback as parsing.
A two-mode test (hybrid with FakeEmbedder, keyword) shows the chain is
answered and the retry recalls the matching row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…at honour it

The README said every REST client error returns a JSON {"error"} body.
POST /api/memories and POST /api/search still use axum's bare Json<T>
extractor, so a body that fails to deserialize gets a plain-text 422/400/415.
List exactly which routes map malformed body, path and query input to a
400 {"error"}, and state that the two v0.0.1 body routes keep axum's
default rejection. Doc only: no behaviour change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t (WR-04)

Implements user decision option-a (reject, 2026-10-23) for Phase 4 review
finding WR-04.

- MemoryService::list rejects expand_links=true with no limit as the existing
  MemoryError::InvalidArgument, before any store hop. Range validation runs
  first. No new error variant, and neither transport mapper changes.
- MCP: invalid_params, which goes out over stdio as a JSON-RPC -32602 error.
  REST: 400 {"error": "invalid argument: limit is required when expand_links
  is true; pass a limit between 1 and 200"}.
- expand_links with a limit, a list without expand_links, and search are
  unchanged, so the 200 x 10 expansion bound now covers every expanded
  response.
- Tests at the service seam, over MCP stdio, over real REST HTTP, and an
  inline REST 400 test. Expansion fixtures that relied on unlimited expanded
  lists now pass a limit.
- The rule is now stated in the tool description, the MCP/REST list DTO
  docs, the MAX_EXPANDED_LINKS doc and the README (the curl example now
  passes a limit).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…REVIEW

The WR-04 row now reads Resolved (user decision option-a, reject;
2026-10-23). Its evidence cites the service.rs guard, the MCP and REST
mappers, the four new tests, the domain/README/tool-description docs, the
STATE.md:122 pointer and code commit 969e891. STATE.md is committed
separately by the orchestrator.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t limit

Records the quick-task plan and summary, the WR-04 decision (option a,
reject) in STATE.md, and the Quick Tasks Completed row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

Phase 4: memory_update, typed relations and hybrid RRF search (MCP-06, MCP-07, STORE-05, SEARCH-04, SEARCH-05)

1 participant