From 70699e10ebf70b3d0395d9951667402ba395bad6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 07:20:01 +0000 Subject: [PATCH] docs: v3.1 'Make It True' milestone design spec and phase plan Full-project review (2026-08-30) found a systematic gap between planning-doc claims and running code: dead orchestrator crate, silent BM25 outbox no-op, mislabeled Hybrid layer, an unrun LOCOMO adapter with an invented schema and substring scoring, and a 65s TOC p50 in the recorded perf baseline. This spec defines milestone v3.1 (Phases 54-58): integration truth, performance truth, honest benchmarks with a real LOCOMO schema and LLM-as-judge, repo shop window + local-first positioning, and GSD process rules requiring execution evidence for verification. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01R6oegDbEcoNgWHckPyZYCJ --- docs/plans/v3.1-make-it-true-plan.md | 410 +++++++++++++++++++++++++++ 1 file changed, 410 insertions(+) create mode 100644 docs/plans/v3.1-make-it-true-plan.md diff --git a/docs/plans/v3.1-make-it-true-plan.md b/docs/plans/v3.1-make-it-true-plan.md new file mode 100644 index 0000000..fdcd890 --- /dev/null +++ b/docs/plans/v3.1-make-it-true-plan.md @@ -0,0 +1,410 @@ +# v3.1 "Make It True" — Design Spec & Phase Plan + +**Date:** 2026-08-30 +**Status:** Proposed +**Milestone:** v3.1 (follows v3.0 Competitive Parity & Benchmarks) +**Origin:** Independent full-project review, 2026-08-30 (code-quality audit + benchmark-evidence audit) + +--- + +## Why This Milestone Exists + +v3.0 shipped its three phases (Retrieval Orchestrator, Simple CLI API, Benchmark +Suite) as merged PRs, and the codebase underneath is genuinely strong: ~1,200 +tests, a correct transactional outbox on RocksDB, real Tantivy/usearch/Candle +integration, production-grade LLM API client hygiene, and disciplined fail-open +design. The architecture thesis (temporal TOC + progressive disclosure, indexes +as accelerators) is sound and independently validated by research (MemWalker, +RAPTOR) and by the industry shift toward agentic retrieval. + +But a review found a systematic gap between **what the planning docs claim** +and **what actually runs**: + +| # | Claim | Reality | Evidence | +|---|-------|---------|----------| +| 1 | "Simple CLI wired to orchestrator" (v3.0 spec, Phase B) | `memory-orchestrator` is dead code — **zero crates depend on it**; `memory search` calls `RouteQuery` RPC directly | `crates/memory-cli/Cargo.toml` (no orchestrator dep); `crates/memory-cli/src/commands/search.rs:1` | +| 2 | LLM reranking capability | Only a mock (`.reverse()`) is tested; cross-encoder path logs a warning and falls back | `crates/memory-orchestrator/src/rerank.rs:78-86` | +| 3 | Outbox-driven BM25 incremental indexing | `find_grip_for_event` unconditionally returns `Ok(None)`; `IndexEvent` and `UpdateToc` outbox entries are silently dropped while reporting success | `crates/memory-indexing/src/bm25_updater.rs:52-93` | +| 4 | `Hybrid` retrieval layer | The `CrateLayer::Hybrid` arm delegates to BM25; real fusion lives in a separate code path | `crates/memory-service/src/retrieval.rs:600-604` vs `crates/memory-service/src/hybrid.rs:159-214` | +| 5 | "Publishable LOCOMO score" (v3.0 milestone goal) | Adapter never ran: invented dataset schema, dead download URL (404), substring scoring instead of LLM-judge, shared store cross-contamination, swallowed errors | `crates/memory-bench/src/locomo.rs`; `benchmarks/scripts/download-locomo.sh` | +| 6 | Benchmark metrics | `recall_at_5` is numerically identical to accuracy; `compression_ratio` measures file-**path** lengths, not content | `crates/memory-bench/src/main.rs`, scorer | +| 7 | "Answer 'what were we talking about last week' without scanning everything" | Recorded perf baseline shows TOC navigation **p50 ≈ 64.6s** and vector **p50 ≈ 7.2s** on a 240-event corpus; percentiles computed from 3 samples | `crates/e2e-tests/benchmarks/latest.json` | +| 8 | Phase 53 "17/17 verified, no gaps" | Every verification item was an existence check (struct/flag/unit test), none an execution result; no v3.0 retrospective exists | `.planning/phases/53-benchmark-suite/53-VERIFICATION.md`; `.planning/RETROSPECTIVE.md` | +| 9 | MIT-licensed, discoverable project | No root `README.md`, no `LICENSE` file, `Cargo.toml` repository URL points at wrong org (`spillwave` vs `SpillwaveSolutions`), positioning writeup never written | repo root; `Cargo.toml:31` | + +**Milestone goal:** close the claim/reality gap so every public statement about +Agent-Memory is executable, measured, and defensible — then open the shop +window. No new capabilities. "Make it true, then make it known." + +**Positioning shift (load-bearing):** v3.0 framed the project as "category +leader with best-in-class retrieval and a publishable LOCOMO score." That is +the wrong fight for a local-first solo project against funded hosted vendors +(Mem0, Zep, MemMachine). v3.1 repositions to the defensible wedge: + +> **The local-first memory for AI coding agents: zero-token passive capture +> via hooks, one memory across every CLI you use (Claude Code, OpenCode, +> Gemini, Copilot, Codex), with provenance for every claim.** + +A LOCOMO score becomes supporting evidence *if and only if* it is honestly +produced and favorable — never the headline. + +--- + +## Non-Goals + +- No new retrieval capabilities, layers, or index types +- No REST/HTTP endpoint, Python SDK, or UI (unchanged from v3.0 scope) +- No cross-encoder reranking implementation (extension point stays stubbed; + see Phase 54 for honest labeling) +- No new runtime adapters; Phase 57 *reduces* the supported-surface promise + +--- + +## Phase Overview (Sequential with one parallel track) + +| Phase | Name | Depends On | Risk It Retires | +|-------|------|------------|-----------------| +| 54 | Integration Truth | — | Shipped-but-unwired features; silent no-ops | +| 55 | Performance Truth | 54 | The 65-second TOC p50 contradicting the core value claim | +| 56 | Honest Benchmarks | 54, 55 | Indefensible benchmark story | +| 57 | Shop Window & Positioning | 54 (can start alongside 55/56) | Invisible/unlicensed public repo | +| 58 | Launch (side quest) | 55, 56, 57 | — | + +Process changes (verification rules, retrospective, adapter tiering) are part +of Phase 54 entry and Phase 57 exit — see "Process Changes" below. + +--- + +## Phase 54: Integration Truth + +**Goal:** every merged feature is reachable from a public entry point or is +explicitly removed/relabeled. No code path that silently succeeds while doing +nothing. + +### 54-01: Wire the orchestrator into the `memory` CLI + +The v3.0 spec (Phase B) says all CLI commands are "wired to +`memory-orchestrator`." Make that true: + +- Add `memory-orchestrator` as a dependency of `memory-cli` (or of + `memory-service` behind the `RouteQuery` handler — decide at plan time; + prefer the daemon side so gRPC callers benefit too). +- `memory search/context/recall` must flow: query expansion → fan-out via + `RetrievalExecutor` → RRF fusion → (optional) rerank → context builder. +- `memory recall` keeps its contract as `search --rerank=llm --top=10`. +- The LLM reranker gets a **real implementation** using the existing API + client infrastructure in `memory-toc/src/summarizer/api.rs` (both Anthropic + and OpenAI wire protocols already exist with backoff + secrecy). A rerank + prompt over top-k candidates returning an ordering. Mock stays for tests; + the real impl is selected when an API key is configured. +- Delete `CrossEncoderReranker` or make it return a hard + `Error::NotImplemented` — never a warn-and-fallback that masquerades as + reranking. + +**Acceptance (testable at PR time):** +- [ ] `cargo tree -i memory-orchestrator` lists at least one binary crate +- [ ] An e2e test drives `memory search` (or `RouteQuery`) and asserts the + orchestrator pipeline executed (e.g., via explainability payload naming + the fusion stage) +- [ ] `memory recall` with a configured mock endpoint produces a reranked + order different from RRF order in an integration test + +### 54-02: Fix the BM25 outbox no-op + +`crates/memory-indexing/src/bm25_updater.rs:88-93` returns `Ok(None)` +unconditionally; `process_entry` for `OutboxAction::IndexEvent` always +reports success while indexing nothing. + +- Decide: implement event→grip resolution, or delete the `IndexEvent` outbox + path and document that BM25 is populated exclusively by grip indexing. + Either is acceptable; **silent success is not.** +- Same decision for the skipped `UpdateToc` arm. + +**Acceptance:** +- [ ] An e2e test ingests an event, drains the outbox, and asserts the + document is findable via BM25 teleport — OR the `IndexEvent` action no + longer exists and `docs/ARCHITECTURE.md` describes the real indexing + path +- [ ] No code path in `memory-indexing` returns `Ok` after intentionally + doing nothing without a `tracing::warn!` and a counter metric + +### 54-03: Make the `Hybrid` layer hybrid + +`crates/memory-service/src/retrieval.rs:600-604` delegates `CrateLayer::Hybrid` +to BM25. Route it through the real weighted fusion in +`crates/memory-service/src/hybrid.rs`, or rename the layer so the fallback +chain and tier detection report what actually runs. + +**Acceptance:** +- [ ] Tier-2 (`Hybrid`) retrieval demonstrably merges BM25 + vector scores in + an integration test with diverging per-index rankings + +### 54-04: One RRF implementation + +Two divergent RRFs exist: `crates/memory-orchestrator/src/fusion.rs` (unweighted, +`f64`, well-tested, dead) and `crates/memory-service/src/hybrid.rs:159-214` +(weighted, `f32`, live). Keep one (the weighted live one), port the better +tests onto it, delete the other. + +**Acceptance:** +- [ ] `rg -l "reciprocal|rrf" crates/` shows one implementation site +- [ ] Ported tests: rank-fusion differs from any single input when scores + diverge; empty-input fail-open + +### 54-05: Honest daemon flags + +`memory-daemon start --background` logs "not yet implemented" and runs in +foreground (`crates/memory-daemon/src/commands.rs:488`). Either implement +double-fork daemonization (already on the Deferred list) or remove the flag +and exit non-zero with guidance. No flag that lies. + +### 54-06: Lock-poisoning policy + +27 production `.lock()/.read()/.write().unwrap()` sites (e.g. +`crates/memory-service/src/vector.rs:48,54,100,168,202`) will panic the daemon +on poisoning. Pick one policy and apply it workspace-wide: `parking_lot` +(no poisoning), or explicit `unwrap_or_else` recovery with a metric. + +**Phase 54 exit criteria:** all six plans merged; `task pr-precheck` green; +no `Ok(())`-while-doing-nothing paths remain (audited by grep + review). + +--- + +## Phase 55: Performance Truth + +**Goal:** the recorded perf numbers support — rather than contradict — the +core value claim, and the measurement methodology survives scrutiny. + +### 55-01: Root-cause the 65-second TOC p50 + +`crates/e2e-tests/benchmarks/latest.json` (2026-02-12, tier=medium, warm) +records `single.toc` p50 = 64,576ms and `single.vector` p50 = 7,171ms on a +240-event corpus, while BM25 sits at 245ms and route_query at 2.3ms. On the +project's own thesis (TOC navigation is the always-works foundation), this is +the number a critic leads with. Likely suspects, in order: + +1. Summarizer/LLM calls inside the benchmarked path (network-bound step being + measured as "TOC navigation") +2. Cold model load (Candle embedder init) attributed to the query step +3. Full-tree traversal without the documented stop conditions +4. A harness defect (measuring setup + query together) + +Instrument, attribute, fix or re-scope the measurement so "TOC navigation" +measures navigation. If the true cost is LLM summarization at ingest/rollup +time, report it there — that is a defensible async cost, not a query cost. + +**Acceptance:** +- [ ] `single.toc` warm p50 < 500ms on the medium corpus, OR a written + breakdown in `docs/benchmarks.md` attributing the time to steps that + are genuinely not on the query path +- [ ] Same treatment for `single.vector` (embedder init vs. query separated) + +### 55-02: Fix the percentile methodology + +p90/p99 from `samples: 3` are interpolation artifacts. + +- Iterations ≥ 30 per step (configurable); report p50/p90/p99 only when + sample count supports them, otherwise report min/median/max and say so +- Separate cold-start metrics from warm metrics structurally, not by label +- Regenerate `baseline.json` / `latest.json`; update regression thresholds + +**Phase 55 exit criteria:** re-baselined numbers committed; +`docs/benchmarks.md` states corpus size, sample counts, hardware, and +caveats; no recorded number we would decline to show a skeptic. + +--- + +## Phase 56: Honest Benchmarks + +**Goal:** a benchmark story that survives ten minutes of hostile review — or +no benchmark story at all. Never a number produced by methodology we would +not defend in public. + +### 56-01: Fix the custom harness + +- Delete `recall_at_5` (it is `accuracy` under another name) or implement a + real recall@k against labeled relevant-item sets +- Fix `compression_ratio` to compare token estimates of retrieved context vs + **file contents** of the setup corpus (it currently sums file-path string + lengths) +- Errors are load-bearing: `memory add`/`memory search` subprocess failures + must fail the run loudly (currently `let _ =` + unchecked `success`, so a + dead daemon reports accuracy 0.0 as a clean result) +- Per-test store isolation: each fixture runs against a fresh project store + (temp dir), no shared-store bleed between tests +- Grow fixtures from 4 tests / 60 lines to ≥ 25 tests covering temporal, + multisession, and compression categories with distractor sessions + +### 56-02: LOCOMO adapter v2 (real schema, real judge) + +The real dataset: `https://github.com/snap-research/locomo`, file +`data/locomo10.json`. Actual schema (verified 2026-08-30): top-level +`sample_id`, `conversation` (object with `session_N` arrays + +`session_N_date_time`, speaker names per turn, dialog ids), `qa` (array of +`{question, answer, category, evidence}` with integer category labels), plus +`observation`/`session_summary`/`event_summary` fields. The current adapter's +schema (`conversation_id`/`turns`/`questions`) is invented and will not parse +this file. + +- Rewrite `benchmarks/scripts/download-locomo.sh` to fetch from the GitHub + repo (current URL 404s); verify `LICENSE.txt` terms at download time before + publishing any score +- Rewrite `LocomoConversation` deserialization against `locomo10.json`; + ingest sessions with their real timestamps and speakers (this exercises the + temporal TOC the way it was designed to be used) +- **One isolated store per conversation** (`sample_id`-keyed temp project); + the current shared-store design leaks conversations 1..N-1 into + conversation N +- **Answer generation + LLM-as-judge scoring**, reusing the API client in + `memory-toc/src/summarizer/api.rs`: retrieve context → generate an answer → + judge against gold with a fixed rubric prompt. Substring containment over + raw retrieval JSON is not comparable to published LOCOMO numbers and must + not be labeled a LOCOMO score. Keep substring mode only as a fast smoke + metric, clearly named (`context_hit_rate`), never surfaced in comparisons +- Deterministic judge settings (temperature 0, pinned model id recorded in + `results.json`), per-category breakdown (single-hop, multi-hop, temporal, + open-domain), and token/latency accounting per question + +### 56-03: Run it, record it, gate it + +- Execute the full LOCOMO run; commit `benchmarks/results/locomo-.json` + plus a methodology writeup (`docs/benchmarks.md` section: dataset version, + judge model, prompts, isolation, caveats) +- CI: smoke path runs the adapter against a 1-conversation fixture with a + mock judge (asserts parse → ingest → retrieve → score pipeline executes); + full run stays manual +- **Decision gate (explicit, in the PR description):** + - Score competitive (within reach of published Mem0-class numbers) → + publish with full methodology, including the `--compare` table with + like-for-like caveats per row + - Score not competitive → publish methodology + honest number in + `docs/benchmarks.md` *without* comparison marketing, or hold entirely; + positioning (Phase 57) does not depend on it either way +- Fix the `--compare` table so incommensurable metrics never share a row: + each baseline row states its metric ("LOCOMO LLM-judge", "relative delta + vs OpenAI memory") — no unified "Accuracy" column across different + measurement regimes + +**Phase 56 exit criteria:** a committed `results.json` produced by an actual +run (verification is the artifact, not the code's existence); README/docs +contain no benchmark claim without a committed result behind it. + +--- + +## Phase 57: Shop Window & Positioning + +**Goal:** a stranger landing on the repo understands what it is, trusts it, +and can run it — and the project's public claims match Phases 54–56 reality. + +### 57-01: Repo hygiene + +- Root `README.md`: one-paragraph pitch (local-first wedge), architecture + diagram, 5-minute quickstart (daemon start → hook install → `memory search`), + honest status table (what's solid / what's experimental), link to docs +- `LICENSE` file (MIT, matching `Cargo.toml`); fix `workspace.package.repository` + to `https://github.com/SpillwaveSolutions/agent-memory` +- GitHub repo description + topics (`ai-agents`, `memory`, `rust`, + `claude-code`, `local-first`); enable Discussions +- Demo: asciinema or GIF of the drill-down navigation answering + "what were we talking about last week?" + +### 57-02: Positioning writeup (the unpaid v3.0 debt) + +`docs/positioning/agent-memory-vs-competition.md`: + +- Head-to-head vs Mem0 / Zep / MemMachine / Letta across: memory model, + capture cost (tokens), locality/privacy, cross-CLI reach, provenance, + evolution — leading with the three dimensions where Agent-Memory is + structurally different (passive zero-token capture, local-first, cross-CLI) +- Platform-risk section, addressed head-on: native memory in Claude + Code/ChatGPT is single-vendor and non-portable; Agent-Memory is the + cross-CLI layer. Say it before a commenter does +- Benchmark section conditional on the Phase 56 gate +- Publishable as a blog post with minor edits + +### 57-03: Scope trim — supported-surface tiering + +61K LOC, 6 runtime converters, and 5 CLI bats harnesses is an unsustainable +promise for a solo-maintained project. Declare tiers in README: + +- **Tier 1 (supported):** Claude Code + one second runtime (pick by actual + usage) +- **Tier 2 (best-effort/community):** remaining converters; their bats + suites move out of the required CI gate to a scheduled workflow +- Delete or archive the OpenCode converter stub (known gap OC-01–06) rather + than shipping empty methods + +**Phase 57 exit criteria:** GitHub landing page renders the new README; a +fresh-machine quickstart has been executed start-to-finish by following the +README verbatim (recorded as the verification artifact). + +--- + +## Phase 58: Launch (Side Quest) + +Not a GSD phase. After 55–57 land: + +- Blog post from the positioning writeup (Substack; the harness-engineering + meta-story — "my AI pipeline self-graded an unrun benchmark as 17/17 + passed, here's the fix" — is itself strong content for the audience) +- Show HN / r/rust / r/LocalLLaMA with the local-first framing and the demo +- Cut a v3.1.0 release via the existing release workflow (multi-platform + binaries already build) + +--- + +## Process Changes (bound to this milestone) + +These prevent the v3.0 failure mode from recurring. The failure was not code +quality — it was **verification by existence instead of execution**, which +let a never-run benchmark and a never-wired crate self-grade as "17/17, no +gaps." + +1. **Execution-evidence rule:** a VERIFICATION.md item for any run-dependent + requirement (benchmark, e2e behavior, perf number, quickstart) must cite a + committed artifact produced by actually running it (results file, CI run + link, transcript). Existence of structs/flags/tests verifies nothing but + existence. Add this rule to `.planning/config.json` / the GSD phase + template so the checker enforces it. +2. **Reachability rule:** a phase that adds a crate or subsystem is not + complete until `cargo tree -i ` shows a dependent binary, or the + phase explicitly declares it dormant with a tracking requirement. +3. **Write the v3.0 retrospective** (currently absent; latest entry is v2.5), + recording the above honestly, plus the human_verification items from + 53-VERIFICATION that were waved through. +4. **`human_verification` items are blockers by default** when they gate the + milestone's stated goal (the v3.0 goal *was* the unrun benchmark). + +--- + +## Sequencing, Estimates, Risks + +**Suggested plan counts** (GSD granularity, ~1 session/plan): +Phase 54: 6 plans · Phase 55: 2 plans · Phase 56: 3 plans · Phase 57: 3 plans +→ 14 plans + retrospective. At the historical cadence (v2.x: 10–22 sessions +per milestone) this is a normal-sized milestone, achievable in 2–4 weeks of +part-time effort. + +| Risk | Likelihood | Mitigation | +|------|------------|------------| +| TOC 65s is architectural (traversal cost), not a harness artifact | Medium | Stop conditions already specced (`max_nodes`, `beam_width`) — enforce them in the benchmarked path; worst case, re-scope the claim to "navigation + async summarization" honestly | +| LOCOMO score lands well below Mem0-class numbers | Medium | Phase 56 gate: publish methodology without comparison marketing, or hold; positioning does not depend on it | +| LLM-judge cost/flakiness in CI | High | Full run is manual + committed artifact; CI uses 1-conversation fixture with mock judge | +| Orchestrator wiring destabilizes the live RouteQuery path | Low-Med | Wire behind a config flag first, flip default after e2e parity; fail-open fan-out already designed for this | +| Scope trim (Phase 57-03) disappoints Tier-2 users | Low | Tiering ≠ removal; scheduled CI keeps Tier-2 from silently rotting | + +--- + +## Milestone Success Criteria + +- [ ] Every v3.0-claimed capability is reachable from a public entry point, + relabeled, or removed (Phase 54) +- [ ] No recorded perf number contradicts the core value claim without a + committed explanation (Phase 55) +- [ ] Every published benchmark claim has a committed, honestly-produced + results artifact behind it; no substring score labeled "LOCOMO" (Phase 56) +- [ ] Root README + LICENSE + positioning doc exist; quickstart verified on a + fresh machine (Phase 57) +- [ ] v3.0 retrospective written; execution-evidence and reachability rules + active in the GSD templates +- [ ] `task pr-precheck` green on every PR (unchanged standing rule)