From 604fb795eebc9b947c20579628d3d224a54616ba Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 07:58:59 +0100 Subject: [PATCH 01/20] docs: retrieval-quality design spec (scoring, embeddings, evidence ratings) Co-Authored-By: Claude Fable 5 --- .../2026-07-23-retrieval-quality-design.md | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-23-retrieval-quality-design.md diff --git a/docs/superpowers/specs/2026-07-23-retrieval-quality-design.md b/docs/superpowers/specs/2026-07-23-retrieval-quality-design.md new file mode 100644 index 0000000..dd9a02d --- /dev/null +++ b/docs/superpowers/specs/2026-07-23-retrieval-quality-design.md @@ -0,0 +1,156 @@ +# Retrieval quality: unified scoring, embeddings, evidence-anchored ratings + +**Date:** 2026-07-23 +**Status:** approved design, pending implementation +**Predecessor:** PR #81 (rating loop + shortlist + demotion, measured 22.96% useful in A/B), PR #82. + +## Guardrails (from planning memories + standards) + +- **[[da7ff62e]] / [[be7ad6bf]]** (conf 0.9, useful 15/13): planning memories + knowledge + standards consulted before drafting — done; `standards/ralph-runtime.md` rules applied + (feature branch at task start, confidence-scored plan to follow, visualiser handoffs). +- **[[98056ebc]]** (conf 1.0, useful 18): website/README sync — PR-A and PR-B touch + `mcp/tools.py` registrations and config: `website/configuration.md`, `website/index.md` + tools showcase must be updated in the same PRs. +- Dismissed: TDD-RED-step reflection (no guard being moved), ralph-queue reflection + (work is built here, not queued). + +## Problem + +Three retrieval-quality gaps remain after #81, all measured on the live DB: + +1. **No semantic matching.** `reflection_embeddings` (vec0, 768-dim) has 0 rows — + the synthesis write path never embeds. `memory.retrieve`'s `query` fusion is + BM25-only; a query phrased differently from a reflection's wording misses. +2. **Rich-get-richer prior.** Ranking key is raw `useful_count`: 67 useful / 192 served + (35% hit rate) permanently outranks 3 useful / 4 served (75%). New memories starve + behind popular ones; the demotion CASE from #81 only handles the zero-useful case. +3. **Noisy labels.** The rating signal every ranking term feeds on is high-variance: + in A/B runs, byte-identical memory sets on the same task were rated `shaped` in one + session and `ignored` in another. Nothing anchors a non-ignored rating to anything. + +Plus a documentation defect with a root cause worth fixing: the user-level CLAUDE.md +enumerates `memory_retrieve` parameters that do not exist (`component`, `scope_path`, +`window`), training every session to make silently-degraded calls. Enumerating params +in prose is drift-by-construction — the MCP schema already self-describes. + +## Design + +### 1. Unified scoring model (replaces the ORDER BY stack) + +SQL keeps filtering only (project/scope/status/tech/phase/polarity). Ordering moves to +Python (~150 rows; SQLite `sqrt()` not guaranteed; unit-testable): + +```python +rated = useful_count + times_overlooked + times_ignored +positive = useful_count + times_overlooked # overlooked = relevance evidence +score = wilson_lower_bound(positive, rated, z=1.96) # 0.0 when rated == 0 +``` + +Sort: `score DESC, confidence DESC, updated_at DESC`. Same model for semantic memories +(`created_at` as final tiebreaker, as now). + +**Deleted:** `OVERLOOKED_RANKING_WEIGHT`, `IGNORED_DEMOTION_FLOOR`, +`IGNORED_DEMOTION_WEIGHT`, and the demotion CASE in both services. Wilson subsumes +them: 0 positive / 58 rated → LB ≈ 0 (stays buried); the 0/0 gap is covered by +exploration below. Worked examples: 67/192 → 0.28; 3/4 → 0.30; 0/58 → 0.00. + +**Query fusion unchanged in shape:** RRF (`1/(60+rank)`) over rank lists. The prior +rank list now comes from the Wilson ordering. + +### 2. Exploration: reserved slot per bucket + +After ranking, each polarity bucket's shortlist (default 5) reserves its **last slot** +for the best *untested* candidate — `rated < 3` — ranked by query relevance, then +recency, when one exists. Buckets with no untested candidate fill all 5 slots normally. + +Bounded dilution by construction: proven memories keep ≥4 slots. Deterministic — +testable, reproducible in the A/B harness. With rating coverage at ~100% (sync Stop +hook, #81), every exploration serve converts to a rating within one session. + +### 3. Reflection + semantic embeddings, three-leg fusion + +- **Write path:** synthesis apply (new / augment / merge) embeds + `title + "\n" + use_cases + "\n" + "\n".join(hints)` via the existing + `OllamaEmbedder` (nomic-embed-text, 768-dim — matches the vec0 schema). Best-effort: + `EmbeddingError` → log + skip; synthesis never blocks on Ollama. Augment/merge + re-embed (text changed). Semantic memories: same on create/update_text, into a new + `semantic_embeddings` vec0 table (**migration 0014**). +- **Lazy self-heal:** in `memory.retrieve`'s query path, candidates missing embeddings + are embedded opportunistically (single `embed_batch`, cap ~20/call, best-effort). + Historical rows heal on first relevant retrieval; write-time failures self-repair. +- **One-shot CLI:** `python -m better_memory.cli.backfill_embeddings` — embeds all + active reflections + semantic memories missing vectors; idempotent; run at deploy. +- **Fusion:** `_fuse_by_relevance` becomes three-leg RRF: prior rank (Wilson), BM25 + rank, vec-kNN rank — same constant as `search/hybrid.py`. Query embedded once per + retrieve call; Ollama failure → two-leg (today's behaviour). Rows without embeddings + are simply absent from the vec leg. sqlite embeddings backend (embedder None): + everything no-ops to BM25-only. + +### 4. Evidence-anchored ratings + +- **Migration 0015:** `evidence TEXT` (nullable) on `session_memory_exposure`. +- **Validation** (`services/memory_rating.py`, in the existing + validate-before-savepoint pass): `cited/shaped/misled/overlooked` require non-empty + `evidence` (trimmed, ≤500 chars) — one line: what the memory changed, or a quote. + Missing → `ValueError`, batch rejected. `ignored` requires none (accepted if sent). +- **Tool schemas:** rating items in `memory.apply_session_ratings` gain optional + `evidence` property (required-by-validation for non-ignored; description says so). + `memory.credit` gains `evidence`, required for all its classes. +- **Skill rewrite** (`rate-session-memories`): STEP 2 becomes evidence-first — *write + the evidence line before choosing the class; no evidence → the class is `ignored`.* + Applying the test before choosing the label is the variance-reduction mechanism. +- **UI:** + - Reflection and semantic detail/drawer: **evidence history** section — exposure + rows with `evidence IS NOT NULL`, newest first, cap 10, each with classification + badge + evidence + rated date. Answers "why does this rank here" with receipts. + - `/diagnostics` last-20-rated table: evidence column. +- **Compat:** contextual-inject footer nudge updated to mention evidence. AgentCore + paths remain no-ops. Evidence is audit-only in this PR — no scoring use yet. + +### 5. Docs: drift-proof CLAUDE.md + startup sentinel + +- **Snippet rewrite** (`better_memory/skills/CLAUDE.snippet.md`): behavioural + instructions only — "always pass `query` describing the task", "credit with + evidence" — **never enumerate parameter names/types**. Schemas are the single + source of truth; nothing left to drift. +- **Live edit:** same rewrite applied to the user's `~/.claude/CLAUDE.md` + better-memory section (local edit, not a PR artifact). +- **Drift sentinel** (`hooks/session_bootstrap.py`, best-effort, ~ms): scan the + CLAUDE.md better-memory section for param tokens adjacent to tool names that are + absent from the live registry (imported from `mcp/tools.py`, so the check tracks + future schema changes automatically). On drift, append one warning line to bootstrap + additionalContext naming the phantom params. Silent when clean; never blocks. + +## Delivery + +| PR | branch | content | gate | +|----|--------|---------|------| +| A | `feat/retrieval-quality` | §1 + §2 + §3, migration 0014, backfill CLI, website sync | unit (Wilson math, slot, fusion, self-heal) + full suite + **24-session A/B: distinct useful% not below the 22.96% baseline** | +| B | `feat/evidence-ratings` | §4, migration 0015, skill + UI + website sync | unit + integration + full suite (no A/B — ranking untouched) | +| C | `feat/claude-md-drift` | §5 snippet + sentinel (+ live CLAUDE.md edit alongside) | proofread + sentinel unit test | + +Order: **A → B** (B's evidence requirement would break A's A/B rating sessions if it +landed first). C anytime. Each PR babysat to merge: checks green + threads resolved → +squash-merge. + +## Error handling + +Every Ollama touchpoint is best-effort with graceful degradation (vec leg absent → +BM25; embed failure → skip). Evidence validation fails loudly by design. Migrations +idempotent; 0014/0015 are additive (new table / new nullable column) — no table +recreation, no data-preservation risk. + +## Testing + +- Wilson: closed-form cases (0/0→0, 0/58→~0, 3/4>67/192, monotonicity, z pinned). +- Slot: untested present/absent, exactly-one-slot, relevance-then-recency ordering. +- Fusion: three-leg RRF vs hand-computed; missing-embedding absence; Ollama-down + degradation (embedder raising). +- Self-heal: candidate without embedding gets one after retrieve (fake embedder); + cap respected; failure silent. +- Evidence: batch rejection per class; `ignored` exempt; ≤500 enforcement; credit + path; UI read-model rows. +- Sentinel: phantom param detected; clean file silent; malformed/absent CLAUDE.md + never raises. +- A/B rerun for PR-A via existing `autoresearch/memuse-260721-run/runner.py`. From 5784d6e9e708bc4f6c8dc016542e12dea6157a53 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 08:11:16 +0100 Subject: [PATCH 02/20] docs: PR-A implementation plan (Wilson ranking, exploration, embeddings) Co-Authored-By: Claude Fable 5 --- .../2026-07-23-retrieval-quality-pr-a.md | 1588 +++++++++++++++++ 1 file changed, 1588 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-23-retrieval-quality-pr-a.md diff --git a/docs/superpowers/plans/2026-07-23-retrieval-quality-pr-a.md b/docs/superpowers/plans/2026-07-23-retrieval-quality-pr-a.md new file mode 100644 index 0000000..051d026 --- /dev/null +++ b/docs/superpowers/plans/2026-07-23-retrieval-quality-pr-a.md @@ -0,0 +1,1588 @@ +# Retrieval Quality PR-A Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace popularity ranking with a Wilson-score prior + exploration slot, and add reflection/semantic embeddings with three-leg RRF query fusion. + +**Architecture:** SQL keeps filtering; ranking moves to Python (`services/scoring.py`). The Ollama embedder is threaded into `ReflectionSynthesisService` and `SemanticMemoryService`; sync code reaches it through `async_bridge.run_async_in_worker`. Query relevance fuses prior rank + BM25 rank + vec-kNN rank via RRF. Everything embedding-related is best-effort and degrades to today's BM25-only behaviour. + +**Tech Stack:** Python 3.12, sqlite + sqlite-vec (vec0), FTS5, Ollama `nomic-embed-text` (768-dim), pytest. + +**Spec:** `docs/superpowers/specs/2026-07-23-retrieval-quality-design.md` + +## Global Constraints + +- Branch: `feat/retrieval-quality` (exists; spec committed as `604fb79`). +- Test command: `./.venv/Scripts/python.exe -m pytest -v` from repo root (full suite: `./.venv/Scripts/python.exe -m pytest tests -q`, ~10 min, integration deselected by default). +- Typecheck: `./.venv/Scripts/python.exe -m pyright` must stay at 0 errors. +- Every Ollama touchpoint: best-effort, catch `Exception`, never raise into caller. +- `sqlite3.Connection` is bound to its creating thread — embeddings may be computed on the bridge worker thread, but ALL DB reads/writes stay on the caller thread. +- Ruff line length 100. +- Website sync guardrail (conf 1.0 reflection): `website/index.md` / `website/architecture.md` prose must match tool behaviour changes in the same PR. +- Commit after every task, message style `feat(scope): ...` / `test(scope): ...`, footer `Co-Authored-By: Claude Fable 5 `. + +--- + +### Task 1: Wilson scoring module + +**Files:** +- Create: `better_memory/services/scoring.py` +- Test: `tests/services/test_scoring.py` + +**Interfaces:** +- Produces: `wilson_lower_bound(positive: int, n: int, z: float = 1.96) -> float` — 0.0 when `n == 0`; never negative. Consumed by Tasks 2, 3, 4. + +- [ ] **Step 1: Write the failing tests** + +```python +# tests/services/test_scoring.py +"""Wilson lower bound: the ranking prior for reflections + semantic memories. + +Chosen over raw useful_count because raw counts are rich-get-richer: 67 +useful over 192 rated sessions (35% hit rate) permanently outranked 3/4 +(75%). The lower bound rewards hit rate while discounting small samples. +""" +from __future__ import annotations + +import pytest + +from better_memory.services.scoring import wilson_lower_bound + + +class TestWilsonLowerBound: + def test_no_data_scores_zero(self): + assert wilson_lower_bound(0, 0) == 0.0 + + def test_proven_dead_weight_scores_near_zero(self): + assert wilson_lower_bound(0, 58) == pytest.approx(0.0, abs=1e-9) + + def test_high_hit_rate_newcomer_beats_popular_workhorse(self): + # The design's worked example: 3/4 (75%) must outrank 67/192 (35%). + assert wilson_lower_bound(3, 4) > wilson_lower_bound(67, 192) + + def test_worked_example_values(self): + assert wilson_lower_bound(67, 192) == pytest.approx(0.285, abs=0.005) + assert wilson_lower_bound(3, 4) == pytest.approx(0.301, abs=0.005) + + def test_monotonic_in_positives_at_fixed_n(self): + scores = [wilson_lower_bound(k, 10) for k in range(11)] + assert scores == sorted(scores) + assert scores[0] < scores[10] + + def test_more_evidence_at_same_rate_scores_higher(self): + assert wilson_lower_bound(30, 40) > wilson_lower_bound(3, 4) + + def test_never_negative_and_never_above_one(self): + for positive, n in [(0, 1), (1, 1), (1, 1000), (999, 1000)]: + assert 0.0 <= wilson_lower_bound(positive, n) <= 1.0 +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_scoring.py -v` +Expected: FAIL — `ModuleNotFoundError: No module named 'better_memory.services.scoring'` + +- [ ] **Step 3: Write the implementation** + +```python +# better_memory/services/scoring.py +"""Ranking prior for reflections and semantic memories. + +One function, one job: the Wilson score lower bound on the proportion of +rated exposures where the memory was positive (useful or overlooked). +Replaces the raw-count ORDER BY stack (useful_count + overlooked weight + +ignored-demotion CASE) — see the 2026-07-23 retrieval-quality spec §1. + +Computed in Python, not SQL: SQLite's sqrt() requires the math extension, +the candidate sets are tiny (~150 rows), and a pure function is testable +against closed-form values. +""" + +from __future__ import annotations + +import math + +#: 95% confidence. Pinned — changing z reorders every list; treat as part +#: of the scoring contract, not a tunable. +WILSON_Z = 1.96 + + +def wilson_lower_bound(positive: int, n: int, z: float = WILSON_Z) -> float: + """Lower bound of the Wilson score interval for positive/n. + + ``n == 0`` (never rated) returns 0.0 — untested memories score at the + bottom and are surfaced by the exploration slot instead (spec §2). + """ + if n <= 0: + return 0.0 + p = positive / n + z2 = z * z + denom = 1.0 + z2 / n + centre = p + z2 / (2.0 * n) + margin = z * math.sqrt(p * (1.0 - p) / n + z2 / (4.0 * n * n)) + return max(0.0, (centre - margin) / denom) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_scoring.py -v` +Expected: 7 passed + +- [ ] **Step 5: Commit** + +```bash +git add better_memory/services/scoring.py tests/services/test_scoring.py +git commit -m "feat(scoring): Wilson lower bound ranking prior + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 2: Reflections ranked by Wilson score + +**Files:** +- Modify: `better_memory/services/reflection.py` (retrieve_reflections, ~lines 1256-1400) +- Modify: `better_memory/services/memory_rating.py` (delete `IGNORED_DEMOTION_FLOOR`, `IGNORED_DEMOTION_WEIGHT`, `OVERLOOKED_RANKING_WEIGHT`) +- Modify: `tests/services/test_useful_count_ranking.py` +- Delete: `tests/services/test_ignored_demotion.py` (superseded — its scenarios move to the new test file) +- Test: `tests/services/test_wilson_ranking.py` + +**Interfaces:** +- Consumes: `wilson_lower_bound` from Task 1. +- Produces: `retrieve_reflections` rows now include `times_overlooked` and `times_ignored`; SQL no longer orders; Python sorts by `(wilson DESC, confidence DESC, updated_at DESC)`. `_wilson_rated(row) -> int` and `_wilson_score(row) -> float` module-level helpers in `reflection.py` — Task 4 reuses `_wilson_rated` for the untested test. + +- [ ] **Step 1: Write the failing tests** + +```python +# tests/services/test_wilson_ranking.py +"""Retrieval ranks by Wilson lower bound on (useful+overlooked)/rated. + +Replaces the popularity + overlooked-weight + ignored-demotion stack. +Covers the old demotion scenarios too: proven dead weight sinks because +0 positive over many rated gives LB ~ 0, with no special-case code. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.reflection import ReflectionSynthesisService + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed(conn, rid, *, useful=0, overlooked=0, ignored=0, confidence=0.5, + polarity="do", updated_at="2026-01-01"): + conn.execute( + """INSERT INTO reflections + (id, title, project, phase, polarity, use_cases, hints, + confidence, created_at, updated_at, useful_count, + times_overlooked, times_ignored) + VALUES (?, ?, 'p', 'general', ?, 'uc', '[]', ?, + '2026-01-01', ?, ?, ?, ?)""", + (rid, rid, polarity, confidence, updated_at, useful, overlooked, ignored), + ) + conn.commit() + + +def _ids(conn, **kw): + svc = ReflectionSynthesisService(conn) + return [r["id"] for r in svc.retrieve_reflections(project="p", **kw)["do"]] + + +class TestWilsonRanking: + def test_hit_rate_beats_raw_count(self): + pass # replaced below — see class body + + +class TestWilsonOrdering: + def test_hit_rate_beats_raw_count(self, conn): + _seed(conn, "r-workhorse", useful=67, ignored=125) # 67/192 ~ 0.28 + _seed(conn, "r-newcomer", useful=3, ignored=1) # 3/4 ~ 0.30 + assert _ids(conn)[:2] == ["r-newcomer", "r-workhorse"] + + def test_overlooked_counts_as_positive(self, conn): + _seed(conn, "r-overlooked", overlooked=3, ignored=1) + _seed(conn, "r-plain", useful=1, ignored=3) + assert _ids(conn)[0] == "r-overlooked" + + def test_proven_dead_weight_sinks_below_modest_performer(self, conn): + _seed(conn, "r-dead", useful=0, ignored=58) + _seed(conn, "r-modest", useful=2, ignored=8) + assert _ids(conn) == ["r-modest", "r-dead"] + + def test_confidence_breaks_wilson_ties(self, conn): + _seed(conn, "r-low-conf", confidence=0.3) + _seed(conn, "r-high-conf", confidence=0.9) + assert _ids(conn) == ["r-high-conf", "r-low-conf"] + + def test_recency_breaks_confidence_ties(self, conn): + _seed(conn, "r-old", updated_at="2026-01-01") + _seed(conn, "r-new", updated_at="2026-06-01") + assert _ids(conn) == ["r-new", "r-old"] + + def test_rows_expose_all_three_counters(self, conn): + _seed(conn, "r-a", useful=1, overlooked=2, ignored=3) + svc = ReflectionSynthesisService(conn) + row = svc.retrieve_reflections(project="p")["do"][0] + assert row["useful_count"] == 1 + assert row["times_overlooked"] == 2 + assert row["times_ignored"] == 3 + + def test_demotion_constants_are_gone(self): + import better_memory.services.memory_rating as mr + for name in ("IGNORED_DEMOTION_FLOOR", "IGNORED_DEMOTION_WEIGHT", + "OVERLOOKED_RANKING_WEIGHT"): + assert not hasattr(mr, name), f"{name} should be deleted" +``` + +Delete the placeholder `TestWilsonRanking` class (editing artifact) before running; keep only `TestWilsonOrdering`. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_wilson_ranking.py -v` +Expected: FAIL — ordering assertions (current code ranks r-workhorse first) and `test_demotion_constants_are_gone`. + +- [ ] **Step 3: Implement** + +In `better_memory/services/reflection.py`: + +3a. Replace the import of the deleted constants (top of file): + +```python +from better_memory.search.query import sanitize_fts5_query +from better_memory.services.scoring import wilson_lower_bound +``` + +(remove the `from better_memory.services.memory_rating import (IGNORED_DEMOTION_FLOOR, ...)` block). + +3b. Add module-level helpers directly above `class ReflectionSynthesisService`: + +```python +def _wilson_rated(row) -> int: + """Total rated exposures backing this memory's score.""" + return (row["useful_count"] + row["times_overlooked"] + + row["times_ignored"]) + + +def _wilson_score(row) -> float: + positive = row["useful_count"] + row["times_overlooked"] + return wilson_lower_bound(positive, _wilson_rated(row)) +``` + +3c. In `retrieve_reflections`, replace the SELECT (drop the demotion params and ORDER BY; add the two counter columns; sort in Python): + +```python + where = " AND ".join(clauses) + _diag.step(fn, "executing_select") + rows = self._conn.execute( + f""" + SELECT id, title, phase, polarity, use_cases, hints, + confidence, tech, evidence_count, useful_count, + times_misled, times_overlooked, times_ignored, + updated_at + FROM reflections + WHERE {where} + """, + params, + ).fetchall() + _diag.step(fn, "select_done", n_rows=len(rows)) + + # Rank in Python: Wilson prior, then confidence, then recency. + # Chained stable sorts apply tiebreakers lowest-priority first. + rows = list(rows) + rows.sort(key=lambda r: r["updated_at"] or "", reverse=True) + rows.sort(key=lambda r: r["confidence"], reverse=True) + rows.sort(key=_wilson_score, reverse=True) +``` + +(`params` no longer receives the two demotion appends — delete those lines too.) + +3d. The bucket-fill loop and `_fuse_by_relevance` call stay as they are, but the bucket dicts must carry the new counters — extend the `bucket.append({...})` literal with: + +```python + "times_overlooked": r["times_overlooked"], + "times_ignored": r["times_ignored"], +``` + +In `better_memory/services/memory_rating.py`: delete the `OVERLOOKED_RANKING_WEIGHT`, `IGNORED_DEMOTION_FLOOR`, `IGNORED_DEMOTION_WEIGHT` constants and their comment blocks. + +In `tests/services/test_useful_count_ranking.py`: this file pins the OLD ordering (`useful_count` beats confidence regardless of ignores). Rewrite its two assertions to the Wilson contract or delete the file if fully covered by `test_wilson_ranking.py` — it is fully covered; delete it. Also delete `tests/services/test_ignored_demotion.py` (scenarios live on in `test_proven_dead_weight_sinks_below_modest_performer` + `test_demotion_constants_are_gone`). + +- [ ] **Step 4: Run the affected tests** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_wilson_ranking.py tests/services/test_reflection_query_relevance.py tests/services/test_reflection_retrieve_fields.py tests/mcp -q` +Expected: all pass (query-relevance tests still pass — fusion consumes the new ordering transparently). + +- [ ] **Step 5: Commit** + +```bash +git add -A +git commit -m "feat(ranking): reflections ranked by Wilson prior, demotion stack deleted + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 3: Semantic memories ranked by Wilson score + +**Files:** +- Modify: `better_memory/services/semantic.py` (list_for_project SELECT/ORDER, ~lines 237-260; imports ~19-23; SemanticMemory dataclass) +- Test: `tests/services/test_semantic.py` (add class; existing tests keep passing) + +**Interfaces:** +- Consumes: `wilson_lower_bound` (Task 1). +- Produces: `SemanticMemory` dataclass gains `times_ignored: int = 0`; `list_for_project` ordered by `(wilson DESC, created_at DESC)`. + +- [ ] **Step 1: Write the failing test** (append to `tests/services/test_semantic.py`, reusing that file's existing fixtures/helpers for creating memories — check its seed helper name before writing): + +```python +class TestSemanticWilsonRanking: + def test_hit_rate_beats_raw_count(self, conn): + svc = SemanticMemoryService(conn) + a = svc.create(content="workhorse", project="p") + b = svc.create(content="newcomer", project="p") + conn.execute( + "UPDATE semantic_memories SET useful_count=67, times_ignored=125 WHERE id=?", (a,)) + conn.execute( + "UPDATE semantic_memories SET useful_count=3, times_ignored=1 WHERE id=?", (b,)) + conn.commit() + ids = [m.id for m in svc.list_for_project(project="p", track_exposure=False)] + assert ids == [b, a] + + def test_never_rated_sorts_by_recency_at_bottom(self, conn): + svc = SemanticMemoryService(conn) + rated = svc.create(content="rated", project="p") + conn.execute( + "UPDATE semantic_memories SET useful_count=1, times_ignored=1 WHERE id=?", (rated,)) + conn.commit() + unrated = svc.create(content="unrated", project="p") + ids = [m.id for m in svc.list_for_project(project="p", track_exposure=False)] + assert ids == [rated, unrated] +``` + +(Adapt the fixture name to the file's existing `conn` fixture; if creation timestamps collide within the test, monkeypatch the service clock as neighbouring tests in that file do.) + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_semantic.py -v -k Wilson` +Expected: FAIL (current order puts the 67-count row first; `times_ignored` not selected). + +- [ ] **Step 3: Implement** + +In `better_memory/services/semantic.py`: + +- Imports: replace the three-constant import block with `from better_memory.services.scoring import wilson_lower_bound`. +- `SemanticMemory` dataclass: add `times_ignored: int = 0` and `last_ignored_at: str | None = None` after the overlooked fields. +- `list_for_project` SQL: add `times_ignored, last_ignored_at` to the SELECT column list; replace the ORDER BY block and its three `params.append(...)` lines with plain `"ORDER BY created_at DESC"` (recency pre-sort; final order applied in Python). +- After `rows = self._conn.execute(sql, params).fetchall()` build `results` as now (passing the two new fields into the dataclass), then sort: + +```python + results.sort( + key=lambda m: wilson_lower_bound( + m.useful_count + m.times_overlooked, + m.useful_count + m.times_overlooked + m.times_ignored, + ), + reverse=True, + ) +``` + +(Stable sort preserves the SQL `created_at DESC` order inside equal scores.) + +- [ ] **Step 4: Run the semantic + bootstrap tests** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_semantic.py tests/services/test_session_bootstrap.py -q` +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add better_memory/services/semantic.py tests/services/test_semantic.py +git commit -m "feat(ranking): semantic memories ranked by Wilson prior + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 4: Exploration slot + +**Files:** +- Modify: `better_memory/services/reflection.py` (bucket-fill loop in retrieve_reflections) +- Test: `tests/services/test_exploration_slot.py` + +**Interfaces:** +- Consumes: `_wilson_rated` (Task 2). Untested ≡ `_wilson_rated(row) < 3` — the constant lives as `EXPLORATION_RATED_FLOOR = 3` in `reflection.py`. +- Produces: per-bucket shortlist = up to `cap-1` best tested rows + the best untested row (fused/ranked order), when both kinds exist. `cap is None` (bootstrap) ⇒ no reservation. + +- [ ] **Step 1: Write the failing tests** + +```python +# tests/services/test_exploration_slot.py +"""One shortlist slot per bucket is reserved for an untested memory. + +Untested = fewer than 3 rated exposures. Wilson scores them 0.0, so +without the slot they would never be served and never earn a rating. +Rating coverage is ~100% (sync Stop hook), so one serve = one rating. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.reflection import ReflectionSynthesisService + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed(conn, rid, *, useful=0, ignored=0, updated_at="2026-01-01"): + conn.execute( + """INSERT INTO reflections + (id, title, project, phase, polarity, use_cases, hints, + confidence, created_at, updated_at, useful_count, times_ignored) + VALUES (?, ?, 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', ?, ?, ?)""", + (rid, rid, updated_at, useful, ignored), + ) + conn.commit() + + +def _ids(conn, cap=3): + svc = ReflectionSynthesisService(conn) + return [r["id"] for r in + svc.retrieve_reflections(project="p", limit_per_bucket=cap)["do"]] + + +class TestExplorationSlot: + def test_last_slot_goes_to_best_untested(self, conn): + for i in range(4): # four proven memories + _seed(conn, f"r-proven-{i}", useful=5 - i, ignored=5) + _seed(conn, "r-untested-old", updated_at="2026-02-01") + _seed(conn, "r-untested-new", updated_at="2026-06-01") + ids = _ids(conn, cap=3) + assert len(ids) == 3 + assert ids[:2] == ["r-proven-0", "r-proven-1"] # cap-1 proven + assert ids[2] == "r-untested-new" # best untested + + def test_no_untested_fills_all_slots_with_proven(self, conn): + for i in range(4): + _seed(conn, f"r-proven-{i}", useful=5 - i, ignored=5) + ids = _ids(conn, cap=3) + assert ids == ["r-proven-0", "r-proven-1", "r-proven-2"] + + def test_all_untested_fills_normally(self, conn): + for i in range(4): + _seed(conn, f"r-untested-{i}") + assert len(_ids(conn, cap=3)) == 3 + + def test_two_ratings_is_still_untested_three_is_not(self, conn): + _seed(conn, "r-two", useful=1, ignored=1) # rated == 2: untested + _seed(conn, "r-three", useful=1, ignored=2) # rated == 3: tested + for i in range(3): + _seed(conn, f"r-proven-{i}", useful=9, ignored=1) + ids = _ids(conn, cap=3) + assert ids[2] == "r-two" + + def test_unlimited_cap_reserves_nothing(self, conn): + _seed(conn, "r-proven", useful=5, ignored=5) + _seed(conn, "r-untested") + svc = ReflectionSynthesisService(conn) + rows = svc.retrieve_reflections(project="p", limit_per_bucket=None)["do"] + assert len(rows) == 2 # everything returned anyway +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_exploration_slot.py -v` +Expected: `test_last_slot_goes_to_best_untested` and `test_two_ratings_is_still_untested_three_is_not` FAIL (untested rows sort last today and never enter a full bucket). + +- [ ] **Step 3: Implement** + +In `reflection.py`, add near `_wilson_score`: + +```python +#: A memory with fewer than this many rated exposures is "untested": its +#: Wilson score is statistically meaningless, so it competes for the +#: reserved exploration slot instead of the proven slots. +EXPLORATION_RATED_FLOOR = 3 +``` + +Replace the bucket-fill loop (currently: iterate `rows`, append until `cap`) with: + +```python + cap = limit_per_bucket if limit_per_bucket is not None else sys.maxsize + reserve = limit_per_bucket is not None and cap >= 2 + buckets: dict[str, list[dict]] = {"do": [], "dont": [], "neutral": []} + # rows is in final ranked order (Wilson sort, then optional + # relevance fusion). Untested rows keep that order too — the + # first untested row in `rows` is the most query-relevant one. + explorers: dict[str, dict] = {} + for r in rows: + bucket = buckets[r["polarity"]] + untested = _wilson_rated(r) < EXPLORATION_RATED_FLOOR + if reserve and untested and r["polarity"] not in explorers: + explorers[r["polarity"]] = self._bucket_item(r) + continue + if len(bucket) >= (cap - 1 if reserve else cap): + continue + bucket.append(self._bucket_item(r)) + # Fill the reserved slot: best untested if one exists, else the + # next proven row that was displaced by the reservation. + if reserve: + for polarity, bucket in buckets.items(): + if polarity in explorers: + bucket.append(explorers[polarity]) + else: + continue +``` + +Then handle the no-untested case: the loop above under-fills by one when no untested row exists for a bucket. Implement precisely by two passes instead if clearer — the contract the tests pin is: + +1. `cap-1` best tested rows in ranked order, +2. slot `cap` = best untested if any, else next best tested, +3. `cap None` ⇒ plain fill, no reservation. + +A clean two-pass implementation (preferred — use this one): + +```python + cap = limit_per_bucket if limit_per_bucket is not None else sys.maxsize + reserve = limit_per_bucket is not None and cap >= 2 + buckets = {"do": [], "dont": [], "neutral": []} + by_polarity: dict[str, list] = {"do": [], "dont": [], "neutral": []} + for r in rows: + by_polarity[r["polarity"]].append(r) + for polarity, group in by_polarity.items(): + bucket = buckets[polarity] + if not reserve: + bucket.extend(self._bucket_item(r) for r in group[:cap]) + continue + tested = [r for r in group + if _wilson_rated(r) >= EXPLORATION_RATED_FLOOR] + untested = [r for r in group + if _wilson_rated(r) < EXPLORATION_RATED_FLOOR] + chosen = tested[: cap - 1] + if untested: + chosen.append(untested[0]) + remaining = [r for r in group if r not in chosen] + chosen.extend(remaining[: cap - len(chosen)]) + bucket.extend(self._bucket_item(r) for r in chosen) +``` + +Extract the existing `bucket.append({...})` dict literal into a method `_bucket_item(self, r) -> dict` so both call sites share it (it now includes the Task 2 counter fields). + +Note: `sqlite3.Row` supports `in`-list identity checks because each row object is unique per fetch — `r not in chosen` compares identity, which is correct here (same objects from one fetch). + +Update the `_diag.step(fn, "bucketed", ...)` call to follow the new loop. + +- [ ] **Step 4: Run the reflection test files** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_exploration_slot.py tests/services/test_wilson_ranking.py tests/services/test_reflection_query_relevance.py tests/services/test_reflection.py tests/services/test_reflection_retrieve_fields.py -q` +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add -A +git commit -m "feat(ranking): reserved exploration slot per bucket for untested memories + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 5: Migration 0014 — semantic_embeddings table + +**Files:** +- Create: `better_memory/db/migrations/0014_semantic_embeddings.sql` +- Test: append to `tests/db/test_schema.py` (follow that file's existing per-migration test style — read it first and mirror the newest migration's test) + +**Interfaces:** +- Produces: vec0 table `semantic_embeddings(memory_id TEXT PRIMARY KEY, embedding FLOAT[768])`, consumed by Tasks 7-9. + +- [ ] **Step 1: Write the failing test** (mirroring the file's existing pattern — a test that applies migrations to a fresh DB and asserts the table exists): + +```python +def test_0014_semantic_embeddings_table(tmp_memory_db): + conn = connect(tmp_memory_db) + apply_migrations(conn) + row = conn.execute( + "SELECT name FROM sqlite_master WHERE name='semantic_embeddings'" + ).fetchone() + assert row is not None + conn.close() +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/db -q -k 0014` +Expected: FAIL — table absent. + +- [ ] **Step 3: Write the migration** + +```sql +-- Migration 0014: vector index for semantic memories. +-- +-- reflection_embeddings has existed since 0002; semantic memories never got +-- the parallel table, so they have no semantic-search substrate. Written by +-- SemanticMemoryService on create/update, healed lazily on retrieve, and +-- backfilled once by cli.backfill_embeddings. 768 dims = nomic-embed-text, +-- matching observation_embeddings and reflection_embeddings. + +CREATE VIRTUAL TABLE semantic_embeddings USING vec0( + memory_id TEXT PRIMARY KEY, + embedding FLOAT[768] +); +``` + +- [ ] **Step 4: Run** + +Run: `./.venv/Scripts/python.exe -m pytest tests/db -q` +Expected: all pass (including any schema-drift tests — if one hardcodes the migration count/list, update it). + +- [ ] **Step 5: Commit** + +```bash +git add better_memory/db/migrations/0014_semantic_embeddings.sql tests/db +git commit -m "feat(db): semantic_embeddings vec0 table (migration 0014) + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 6: Embedder in ReflectionSynthesisService — write-path embedding + +**Files:** +- Modify: `better_memory/services/reflection.py` (ctor line ~317; `_apply_new` ~718, `_apply_augment` ~817, `_apply_merge` ~921) +- Modify: `better_memory/mcp/server.py:222` (pass embedder) +- Modify: `better_memory/storage/sqlite.py:68` (pass embedder — its ctor already receives one for observations; check the field name, likely `self._embedder` or the ctor arg) +- Test: `tests/services/test_reflection_embedding_write.py` + +**Interfaces:** +- Consumes: `OllamaEmbedder.embed(text) -> list[float]` (async), `run_async_in_worker` from `better_memory.async_bridge`. +- Produces: `ReflectionSynthesisService(conn, *, clock=None, embedder=None)`; `_embed_text(self, text) -> list[float] | None` (sync, best-effort); `_embedding_source_text(title, use_cases, hints) -> str` module function; embeddings written to `reflection_embeddings` on new/augment/merge. Tasks 8-9 reuse `_embed_text` and `_embedding_source_text`. + +- [ ] **Step 1: Write the failing tests** + +```python +# tests/services/test_reflection_embedding_write.py +"""Synthesis writes reflection embeddings, best-effort. + +reflection_embeddings sat at 0 rows from migration 0002 until this change: +the write path simply never embedded. Ollama failures must never block +synthesis — a missing embedding degrades retrieval to BM25, nothing more. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.embeddings.ollama import EmbeddingError +from better_memory.services.reflection import ( + ReflectionSynthesisService, + _embedding_source_text, +) + + +class FakeEmbedder: + def __init__(self, fail=False): + self.calls: list[str] = [] + self.fail = fail + + async def embed(self, text: str) -> list[float]: + self.calls.append(text) + if self.fail: + raise EmbeddingError("ollama down") + return [0.1] * 768 + + async def embed_batch(self, texts: list[str]) -> list[list[float]]: + return [await self.embed(t) for t in texts] + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _vec_count(conn): + return conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] + + +class TestEmbeddingSourceText: + def test_joins_title_use_cases_hints(self): + text = _embedding_source_text("T", "when X", ["h1", "h2"]) + assert text == "T\nwhen X\nh1\nh2" + + +class TestWritePathEmbedding: + def test_embed_text_none_embedder_returns_none(self, conn): + svc = ReflectionSynthesisService(conn) + assert svc._embed_text("anything") is None + + def test_embed_text_failure_returns_none(self, conn): + svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder(fail=True)) + assert svc._embed_text("anything") is None + + def test_embed_text_success_returns_vector(self, conn): + svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder()) + vec = svc._embed_text("anything") + assert vec is not None and len(vec) == 768 +``` + +Plus one integration-style test per apply path. The apply methods take action +dataclasses — read `_apply_new`'s signature and the `NewAction` dataclass +(`reflection.py` ~line 136) first, then write: + +```python +class TestApplyPathsEmbed: + def test_apply_new_writes_embedding_row(self, conn): + fake = FakeEmbedder() + svc = ReflectionSynthesisService(conn, embedder=fake) + # Construct the minimal NewAction the codebase defines (check + # dataclass fields; fill required ones only) and call _apply_new + # via the public apply_decision path used by synthesize tests — + # mirror tests/services/test_reflection_writes.py setup. + ... + assert _vec_count(conn) == 1 + assert fake.calls # source text was embedded + + def test_apply_new_with_failing_embedder_still_writes_reflection(self, conn): + svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder(fail=True)) + ... + assert _vec_count(conn) == 0 # no embedding + # reflection row exists — synthesis unaffected +``` + +**Concrete instruction for the `...`:** copy the action-construction and +`apply_decision` invocation verbatim from the nearest existing test in +`tests/services/test_reflection_writes.py` (it exercises `_apply_new` / +augment / merge already); add the embedder kwarg and the two assertions. +Do the same for augment (asserts a second `fake.calls` entry after augment) +and merge (embedding row for the merge target). + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_reflection_embedding_write.py -v` +Expected: FAIL — `_embedding_source_text` import error. + +- [ ] **Step 3: Implement** + +In `reflection.py`: + +```python +def _embedding_source_text(title: str, use_cases: str, hints: list[str]) -> str: + """What gets embedded for a reflection: the discriminating text.""" + return "\n".join([title, use_cases, *hints]) +``` + +Ctor: + +```python + def __init__( + self, + conn: sqlite3.Connection, + *, + clock: Callable[[], datetime] | None = None, + embedder: Any = None, + ) -> None: + self._conn = conn + self._clock = clock or default_clock + self._embedder = embedder +``` + +Sync bridge helper (method): + +```python + def _embed_text(self, text: str) -> list[float] | None: + """Best-effort sync embedding. None embedder / any failure -> None. + + The embed call runs on the bridge worker thread; the DB write that + consumes the vector stays on the caller thread (sqlite3 conns are + thread-bound). + """ + if self._embedder is None: + return None + try: + return run_async_in_worker(self._embedder.embed(text)) + except Exception: + return None + + def _store_embedding(self, reflection_id: str, vector: list[float] | None) -> None: + if vector is None: + return + self._conn.execute( + "DELETE FROM reflection_embeddings WHERE reflection_id = ?", + (reflection_id,), + ) + self._conn.execute( + "INSERT INTO reflection_embeddings (reflection_id, embedding) " + "VALUES (?, ?)", + (reflection_id, _serialize_vector(vector)), + ) +``` + +For `_serialize_vector`, reuse whatever `ObservationService.create` does at +`observation.py:204-210` to insert into `observation_embeddings` (it already +solves vec0 serialisation — likely `sqlite_vec.serialize_float32` or a JSON +string; copy that exact call, import from the same place). DELETE+INSERT +rather than INSERT OR REPLACE because vec0 virtual tables historically +mishandle UPSERT. + +Imports: `from better_memory.async_bridge import run_async_in_worker` and +`from typing import Any` (if not present). + +Call sites — at the end of the row-write in each of `_apply_new`, +`_apply_augment`, `_apply_merge` (inside their existing transaction, after +the reflection row is written, before commit): + +```python + self._store_embedding( + reflection_id, + self._embed_text(_embedding_source_text(title, use_cases, hints)), + ) +``` + +(with the local variable names each method actually has — augment/merge +re-read the post-edit title/use_cases/hints they just wrote). + +Wiring: `server.py:222` → `ReflectionSynthesisService(memory_conn, embedder=embedder)`; `storage/sqlite.py:68` → pass the embedder SqliteBackend already holds for observations (check its ctor field name and thread it). + +- [ ] **Step 4: Run** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_reflection_embedding_write.py tests/services/test_reflection_writes.py tests/mcp/test_server_backend_dispatch.py -q` +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add -A +git commit -m "feat(embeddings): reflections embedded at synthesis write time, best-effort + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 7: Embedder in SemanticMemoryService + +**Files:** +- Modify: `better_memory/services/semantic.py` (ctor ~54; `create` ~63; `update_text` ~85; `create_from_observation` ~125) +- Modify: instantiation sites — grep `SemanticMemoryService(` in `better_memory/mcp/server.py` and `better_memory/storage/sqlite.py`, pass `embedder=` +- Test: `tests/services/test_semantic_embedding_write.py` + +**Interfaces:** +- Consumes: bridge + serialisation exactly as Task 6 (duplicate `_embed_text`/`_store_embedding` shape into this service, table `semantic_embeddings`, PK `memory_id`; source text = the memory's `content`). +- Produces: `SemanticMemoryService(conn, *, clock=None, embedder=None)`; embeddings written on `create`, `update_text`, `create_from_observation`. + +- [ ] **Step 1: Write the failing tests** (same FakeEmbedder — move it to `tests/services/_embedding_fakes.py` and import from both test files): + +```python +# tests/services/test_semantic_embedding_write.py +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.semantic import SemanticMemoryService +from tests.services._embedding_fakes import FakeEmbedder + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _vec_count(conn): + return conn.execute("SELECT COUNT(*) FROM semantic_embeddings").fetchone()[0] + + +class TestSemanticEmbeddingWrite: + def test_create_embeds_content(self, conn): + fake = FakeEmbedder() + svc = SemanticMemoryService(conn, embedder=fake) + svc.create(content="the fact", project="p") + assert _vec_count(conn) == 1 + assert fake.calls == ["the fact"] + + def test_update_text_reembeds(self, conn): + fake = FakeEmbedder() + svc = SemanticMemoryService(conn, embedder=fake) + mid = svc.create(content="v1", project="p") + svc.update_text(id=mid, content="v2") + assert _vec_count(conn) == 1 # replaced, not duplicated + assert fake.calls == ["v1", "v2"] + + def test_failure_never_blocks_create(self, conn): + svc = SemanticMemoryService(conn, embedder=FakeEmbedder(fail=True)) + mid = svc.create(content="the fact", project="p") + assert mid # row created + assert _vec_count(conn) == 0 + + def test_no_embedder_no_rows(self, conn): + svc = SemanticMemoryService(conn) + svc.create(content="the fact", project="p") + assert _vec_count(conn) == 0 +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_semantic_embedding_write.py -v` +Expected: FAIL — ctor rejects `embedder` kwarg. + +- [ ] **Step 3: Implement** — mirror Task 6 exactly in `semantic.py` (ctor kwarg, `_embed_text`, `_store_embedding` targeting `semantic_embeddings(memory_id, ...)`, calls at the end of `create`, `update_text`, `create_from_observation` before their commits). Wire instantiation sites. + +- [ ] **Step 4: Run** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_semantic_embedding_write.py tests/services/test_semantic.py -q` +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add -A +git commit -m "feat(embeddings): semantic memories embedded on create/update, best-effort + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 8: Three-leg fusion + lazy self-heal in retrieve + +**Files:** +- Modify: `better_memory/services/reflection.py` (`_fuse_by_relevance` ~1189; `retrieve_reflections` query path) +- Test: `tests/services/test_vec_fusion.py` + +**Interfaces:** +- Consumes: `_embed_text` (Task 6), `reflection_embeddings` rows (Task 6), `sanitize_fts5_query` (existing). +- Produces: `_fuse_by_relevance(rows, *, query, rrf_k=60)` unchanged signature, now internally three-leg; self-heal constant `SELF_HEAL_BATCH_CAP = 20`. + +- [ ] **Step 1: Write the failing tests** + +```python +# tests/services/test_vec_fusion.py +"""Query fusion gains a vector leg; missing embeddings self-heal on retrieve. + +Degradation contract: no embedder / Ollama down / row unembedded -> exactly +the two-leg (prior + BM25) behaviour that shipped in #81. The vec leg only +ever promotes. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.reflection import ReflectionSynthesisService +from tests.services._embedding_fakes import FakeEmbedder + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed(conn, rid, *, title, useful=0, ignored=0): + conn.execute( + """INSERT INTO reflections + (id, title, project, phase, polarity, use_cases, hints, + confidence, created_at, updated_at, useful_count, times_ignored) + VALUES (?, ?, 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01', ?, ?)""", + (rid, title, useful, ignored), + ) + conn.commit() + + +class DirectedEmbedder(FakeEmbedder): + """Embeds query and one target title to the same vector; noise elsewhere.""" + + def __init__(self, match_text: str): + super().__init__() + self.match_text = match_text + + async def embed(self, text: str) -> list[float]: + self.calls.append(text) + if self.match_text in text: + return [1.0] + [0.0] * 767 + return [0.0, 1.0] + [0.0] * 766 + + +class TestVecFusion: + def test_semantically_close_row_promoted_without_token_overlap(self, conn): + # Query wording shares no tokens with the target title: BM25 misses, + # the vec leg must carry it. + _seed(conn, "r-target", title="Stdout handling on win32 interpreters") + _seed(conn, "r-noise-1", title="Unrelated advice alpha", useful=5, ignored=1) + _seed(conn, "r-noise-2", title="Unrelated advice beta", useful=4, ignored=1) + emb = DirectedEmbedder("Stdout handling") + # pretend the query is 'semantically identical' to the target: + emb.match_text_query = True + + class QueryDirectedEmbedder(DirectedEmbedder): + async def embed(self, text): + self.calls.append(text) + if "console output" in text or self.match_text in text: + return [1.0] + [0.0] * 767 + return [0.0, 1.0] + [0.0] * 766 + + emb = QueryDirectedEmbedder("Stdout handling") + svc = ReflectionSynthesisService(conn, embedder=emb) + ids = [r["id"] for r in svc.retrieve_reflections( + project="p", query="console output disappears on windows", + )["do"]] + assert ids[0] == "r-target" + + def test_no_embedder_matches_shipped_behaviour(self, conn): + _seed(conn, "r-a", title="Retention thresholds", useful=1) + svc = ReflectionSynthesisService(conn) + ids = [r["id"] for r in svc.retrieve_reflections( + project="p", query="retention")["do"]] + assert ids == ["r-a"] # BM25-only path still works + + def test_embedder_failure_degrades_silently(self, conn): + _seed(conn, "r-a", title="Retention thresholds", useful=1) + svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder(fail=True)) + ids = [r["id"] for r in svc.retrieve_reflections( + project="p", query="retention")["do"]] + assert ids == ["r-a"] + + +class TestSelfHeal: + def test_unembedded_candidates_healed_on_query_retrieve(self, conn): + _seed(conn, "r-a", title="Alpha") + _seed(conn, "r-b", title="Beta") + svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder()) + svc.retrieve_reflections(project="p", query="anything at all") + n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] + assert n == 2 + + def test_heal_capped_at_batch_limit(self, conn): + from better_memory.services.reflection import SELF_HEAL_BATCH_CAP + for i in range(SELF_HEAL_BATCH_CAP + 5): + _seed(conn, f"r-{i:03}", title=f"Title {i}") + svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder()) + svc.retrieve_reflections(project="p", query="anything") + n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] + assert n == SELF_HEAL_BATCH_CAP + + def test_no_query_no_heal(self, conn): + _seed(conn, "r-a", title="Alpha") + svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder()) + svc.retrieve_reflections(project="p") + n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] + assert n == 0 + + def test_heal_failure_silent(self, conn): + _seed(conn, "r-a", title="Alpha") + svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder(fail=True)) + rows = svc.retrieve_reflections(project="p", query="anything") + assert rows["do"] # retrieval unaffected +``` + +Clean the first test before running: keep only the `QueryDirectedEmbedder` version (the earlier `emb` assignments are editing artifacts — delete them). + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_vec_fusion.py -v` +Expected: `SELF_HEAL_BATCH_CAP` import error; promotion test fails. + +- [ ] **Step 3: Implement** + +In `reflection.py`: + +```python +#: Max embeddings written per retrieve call by the lazy self-heal. Keeps +#: worst-case added latency bounded (~cap x embed time on first call after +#: a backlog); the CLI backfill is the bulk path. +SELF_HEAL_BATCH_CAP = 20 +``` + +In `retrieve_reflections`, in the `if query:` branch, before fusion: + +```python + if query: + self._heal_missing_embeddings(rows) + query_vector = self._embed_text(query) + rows = self._fuse_by_relevance( + rows, query=query, query_vector=query_vector, + ) + _diag.step(fn, "relevance_fused", n_rows=len(rows)) +``` + +New methods: + +```python + def _heal_missing_embeddings(self, rows) -> None: + """Embed up to SELF_HEAL_BATCH_CAP candidates that lack vectors. + + Historical rows (embedding written before the write-path existed) + and write-time failures repair themselves on their first relevant + retrieval. Entirely best-effort. + """ + if self._embedder is None or not rows: + return + ids = [r["id"] for r in rows] + placeholders = ",".join("?" for _ in ids) + have = { + r[0] for r in self._conn.execute( + f"SELECT reflection_id FROM reflection_embeddings " + f"WHERE reflection_id IN ({placeholders})", ids, + ) + } + todo = [r for r in rows if r["id"] not in have][:SELF_HEAL_BATCH_CAP] + if not todo: + return + texts = [ + _embedding_source_text( + r["title"], r["use_cases"], json.loads(r["hints"]), + ) + for r in todo + ] + try: + vectors = run_async_in_worker(self._embedder.embed_batch(texts)) + except Exception: + return + for r, vec in zip(todo, vectors): + self._store_embedding(r["id"], vec) + self._conn.commit() + + def _vec_ranks(self, query_vector, candidate_ids) -> dict[str, int]: + """reflection_id -> vec rank (0 = closest) among the candidates.""" + if query_vector is None or not candidate_ids: + return {} + try: + rows = self._conn.execute( + "SELECT reflection_id FROM reflection_embeddings " + "WHERE embedding MATCH ? AND k = ? ORDER BY distance", + (_serialize_vector(query_vector), max(len(candidate_ids), 50)), + ).fetchall() + except sqlite3.OperationalError: + return {} + wanted = set(candidate_ids) + out: dict[str, int] = {} + for r in rows: + if r[0] in wanted: + out[r[0]] = len(out) + return out +``` + +(sqlite-vec kNN cannot take extra predicates — fetch top-k then filter to +candidates, same workaround documented in `search/hybrid.py:262-299`.) + +`_fuse_by_relevance` gains the vec leg — signature `(self, rows, *, query, query_vector=None, rrf_k=60)`; after building `rel_rank` (BM25), add: + +```python + vec_rank = self._vec_ranks(query_vector, ids) +``` + +and in the scoring loop: + +```python + vr = vec_rank.get(row["id"]) + if vr is not None: + score += 1.0 / (rrf_k + vr) +``` + +Bail-out condition changes: currently returns rows unchanged when BM25 finds nothing; now return unchanged only when BOTH `rel_rows` and `vec_rank` are empty. + +- [ ] **Step 4: Run** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_vec_fusion.py tests/services/test_reflection_query_relevance.py tests/services/test_wilson_ranking.py tests/services/test_exploration_slot.py -q` +Expected: all pass. + +- [ ] **Step 5: Commit** + +```bash +git add -A +git commit -m "feat(retrieval): three-leg RRF fusion (prior + BM25 + vec) with lazy embedding self-heal + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 9: Backfill CLI + +**Files:** +- Create: `better_memory/cli/backfill_embeddings.py` +- Test: `tests/cli/test_backfill_embeddings.py` + +**Interfaces:** +- Consumes: `_embedding_source_text` (Task 6), services' `_store_embedding` shape (duplicated locally — the CLI writes directly, it is a maintenance tool). +- Produces: `python -m better_memory.cli.backfill_embeddings [--home PATH]` — embeds every active reflection and semantic memory missing a vector; prints `backfilled reflections=N semantics=M skipped=K`; exit 0 even when Ollama is down (prints a warning, backfills nothing). + +- [ ] **Step 1: Write the failing test** + +```python +# tests/cli/test_backfill_embeddings.py +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.cli.backfill_embeddings import backfill +from tests.services._embedding_fakes import FakeEmbedder + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def test_backfills_reflections_and_semantics(conn): + conn.execute( + """INSERT INTO reflections (id, title, project, phase, polarity, + use_cases, hints, confidence, created_at, updated_at) + VALUES ('r1', 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01')""") + conn.execute( + """INSERT INTO semantic_memories (id, content, project, scope, + created_at, updated_at) + VALUES ('s1', 'fact', 'p', 'project', '2026-01-01', '2026-01-01')""") + conn.commit() + + stats = backfill(conn, FakeEmbedder()) + assert stats == {"reflections": 1, "semantics": 1, "skipped": 0} + assert conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] == 1 + assert conn.execute("SELECT COUNT(*) FROM semantic_embeddings").fetchone()[0] == 1 + + +def test_idempotent(conn): + conn.execute( + """INSERT INTO reflections (id, title, project, phase, polarity, + use_cases, hints, confidence, created_at, updated_at) + VALUES ('r1', 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01')""") + conn.commit() + fake = FakeEmbedder() + backfill(conn, fake) + stats = backfill(conn, fake) + assert stats == {"reflections": 0, "semantics": 0, "skipped": 0} + + +def test_retired_reflections_skipped(conn): + conn.execute( + """INSERT INTO reflections (id, title, project, phase, polarity, + use_cases, hints, confidence, created_at, updated_at, status) + VALUES ('r1', 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01', 'retired')""") + conn.commit() + stats = backfill(conn, FakeEmbedder()) + assert stats == {"reflections": 0, "semantics": 0, "skipped": 0} + + +def test_embed_failure_counted_as_skipped(conn): + conn.execute( + """INSERT INTO reflections (id, title, project, phase, polarity, + use_cases, hints, confidence, created_at, updated_at) + VALUES ('r1', 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01')""") + conn.commit() + stats = backfill(conn, FakeEmbedder(fail=True)) + assert stats == {"reflections": 0, "semantics": 0, "skipped": 1} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/cli/test_backfill_embeddings.py -v` +Expected: FAIL — module not found. + +- [ ] **Step 3: Implement** + +```python +# better_memory/cli/backfill_embeddings.py +"""One-shot embedding backfill for reflections + semantic memories. + +Run at deploy after migration 0014: + + python -m better_memory.cli.backfill_embeddings + +Idempotent: only rows missing a vector are embedded. The lazy self-heal in +memory.retrieve covers stragglers afterwards; this exists so the historical +corpus doesn't wait to be retrieved before becoming searchable. +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import sys +from pathlib import Path + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.reflection import ( + _embedding_source_text, + _serialize_vector, +) + + +def backfill(conn, embedder) -> dict[str, int]: + stats = {"reflections": 0, "semantics": 0, "skipped": 0} + + refl = conn.execute( + """SELECT r.id, r.title, r.use_cases, r.hints FROM reflections r + WHERE r.status IN ('pending_review', 'confirmed') + AND r.id NOT IN (SELECT reflection_id FROM reflection_embeddings)""" + ).fetchall() + for r in refl: + text = _embedding_source_text(r["title"], r["use_cases"], + json.loads(r["hints"])) + try: + vec = asyncio.run(embedder.embed(text)) + except Exception: + stats["skipped"] += 1 + continue + conn.execute( + "INSERT INTO reflection_embeddings (reflection_id, embedding) " + "VALUES (?, ?)", (r["id"], _serialize_vector(vec))) + stats["reflections"] += 1 + + sems = conn.execute( + """SELECT id, content FROM semantic_memories + WHERE id NOT IN (SELECT memory_id FROM semantic_embeddings)""" + ).fetchall() + for s in sems: + try: + vec = asyncio.run(embedder.embed(s["content"])) + except Exception: + stats["skipped"] += 1 + continue + conn.execute( + "INSERT INTO semantic_embeddings (memory_id, embedding) " + "VALUES (?, ?)", (s["id"], _serialize_vector(vec))) + stats["semantics"] += 1 + + conn.commit() + return stats + + +def main(argv: list[str] | None = None) -> None: + from better_memory.config import get_config + from better_memory.embeddings.ollama import OllamaEmbedder + + ap = argparse.ArgumentParser() + ap.add_argument("--home", default=None, + help="BETTER_MEMORY_HOME override (default: config)") + args = ap.parse_args(argv) + + config = get_config() + db = Path(args.home) / "memory.db" if args.home else config.memory_db + conn = connect(db) + apply_migrations(conn) + + if config.embeddings_backend != "ollama": + print("embeddings backend is not ollama; nothing to backfill") + return + + stats = backfill(conn, OllamaEmbedder()) + print(f"backfilled reflections={stats['reflections']} " + f"semantics={stats['semantics']} skipped={stats['skipped']}") + if stats["skipped"]: + print("warning: some rows skipped (Ollama unreachable?); " + "re-run later or let retrieve self-heal them", file=sys.stderr) + + +if __name__ == "__main__": + main() +``` + +Note: `asyncio.run` per item is fine here — standalone process, no running +loop, and the N≈150 corpus embeds in seconds. FakeEmbedder's `embed` is a +plain coroutine, so `asyncio.run` works in tests too. + +- [ ] **Step 4: Run** + +Run: `./.venv/Scripts/python.exe -m pytest tests/cli/test_backfill_embeddings.py -v` +Expected: 4 passed + +- [ ] **Step 5: Commit** + +```bash +git add better_memory/cli/backfill_embeddings.py tests/cli/test_backfill_embeddings.py +git commit -m "feat(cli): one-shot embedding backfill for reflections + semantics + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 10: Website sync, typecheck, full suite + +**Files:** +- Modify: `website/index.md` (tools showcase — memory.retrieve description: task-conditioned, Wilson-ranked, semantic fusion) +- Modify: `website/architecture.md` (ranking prose: replace popularity/demotion description with Wilson + exploration + three-leg fusion; add embeddings lifecycle para) +- Modify: `website/configuration.md` — only if it documents ranking constants (grep `OVERLOOKED` / `DEMOTION`; remove if present; no new env vars were added) + +**Interfaces:** none — prose. + +- [ ] **Step 1: Grep for stale prose** + +Run: `grep -rn "useful_count\|OVERLOOKED\|DEMOTION\|popularity" website/ | head -20` + +- [ ] **Step 2: Update each hit** to describe: Wilson lower bound on (useful+overlooked)/rated; reserved exploration slot; embeddings written at synthesis + self-heal + backfill CLI; three-leg RRF. Keep each edit to the paragraph that is actually wrong — no rewrites beyond the stale claims. + +- [ ] **Step 3: Typecheck** + +Run: `./.venv/Scripts/python.exe -m pyright` +Expected: 0 errors + +- [ ] **Step 4: Full suite** + +Run: `./.venv/Scripts/python.exe -m pytest tests -q` +Expected: everything passes (~1500 tests). Fix any straggler that pinned old ordering before proceeding. + +- [ ] **Step 5: Commit** + +```bash +git add website +git commit -m "docs(website): ranking + embeddings prose matches PR-A behaviour + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 11: A/B validation gate, PR, babysit + +**Files:** +- Modify: `C:/Users/gethi/source/autoresearch/memuse-260721-run/runner.py` (add arm `A8`: `spec["code"] = str(WT)` equivalent pointing at this branch — copy the `A6` arm, change the code path to the main repo checkout on `feat/retrieval-quality`) +- No repo files. + +**Interfaces:** gate = distinct useful% from 24 sessions not statistically below 22.96% (two-proportion z-test, α=0.05, one re-run allowed on borderline). + +- [ ] **Step 1: Point the harness at this branch.** In `runner.py` `arm_spec`, add: + +```python + elif arm == "A8": + # PR-A validation: main repo checkout sits on feat/retrieval-quality. + spec["code"] = str(MAIN_REPO) +``` + +Confirm `git -C C:/Users/gethi/source/better-memory branch --show-current` prints `feat/retrieval-quality` before launching — the arm runs whatever that checkout has. + +- [ ] **Step 2: Refresh the sandbox base DB** (new schema needed): + +```bash +cd C:/Users/gethi/source/autoresearch/memuse-260721-run +python - <<'EOF' +import sqlite3, os +src = os.path.expanduser("~/.better-memory/memory.db") +dst = r"sandbox\base\memory.db" +os.remove(dst) +sqlite3.connect(f"file:{src}?mode=ro", uri=True).execute("VACUUM INTO ?", (dst,)) +EOF +``` + +- [ ] **Step 3: Run the arm** + +```bash +python runner.py --arm A8 --repeats 2 --timeout 420 > logs-A8.txt 2>&1 +``` + +(24 sessions, ~2h, ~$40. Run in background; strip-and-retry any 429 rows as done previously.) + +- [ ] **Step 4: Evaluate the gate** + +```bash +python analyze.py --arms A6,A8 +python - <<'EOF' +# two-proportion z-test: A8 distinct vs 22.96% baseline (62/270) +import math +# fill from analyze output: +u8, n8 = USEFUL_A8, EXPOSED_A8 +p1, n1 = 62/270, 270 +p2 = u8/n8 +p = (62 + u8) / (n1 + n8) +z = (p2 - p1) / math.sqrt(p*(1-p)*(1/n1 + 1/n8)) +print(f"A8={p2:.4f} baseline={p1:.4f} z={z:.2f}") +print("GATE:", "PASS" if z > -1.645 else "FAIL (one re-run allowed)") +EOF +``` + +Gate: PASS ⇒ proceed. FAIL twice ⇒ stop, report, do not open the PR. + +- [ ] **Step 5: PR + babysit** + +```bash +cd C:/Users/gethi/source/better-memory +git push -u origin feat/retrieval-quality +gh pr create --title "feat(retrieval): Wilson prior, exploration slot, embeddings + vec fusion" --body " + +🤖 Generated with [Claude Code](https://claude.com/claude-code)" +``` + +Babysit loop (as PR #81/#82): poll `statusCheckRollup` + unresolved `reviewThreads`; fix findings; when CLEAN + zero threads → `gh pr merge --squash --delete-branch`; then run migration + backfill on live: + +```bash +git checkout main && git pull origin main +./.venv/Scripts/python.exe -m better_memory.cli.backfill_embeddings +``` + +(migration 0014 applies on the MCP server's next start; backfill after it — if the table is missing, `apply_migrations` in the CLI's `main` creates it first, so this ordering is safe.) + +--- + +## Self-review notes + +- Spec §1-§3 covered by Tasks 1-9; §"Delivery A" gate by Task 11; website guardrail by Task 10. Spec §4 (evidence) and §5 (docs/sentinel) are PR-B / PR-C — separate plans, deliberately absent here. +- Two test snippets contain flagged editing artifacts with explicit cleanup instructions (Task 2 placeholder class, Task 8 first test) — the implementer deletes the marked lines; both are called out inline. +- Task 6's apply-path tests reference `tests/services/test_reflection_writes.py` for the action-construction boilerplate rather than inventing dataclass fields that may drift — the implementer copies the file's existing working setup. +- `_serialize_vector` is defined by copying the exact serialisation used at `observation.py:204-210`; Tasks 8, 9 import it from `reflection.py`. From 3eee7a72ccac6878675b09ab74b78e5bb8d581fd Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 08:27:43 +0100 Subject: [PATCH 03/20] =?UTF-8?q?docs:=20PR-A=20plan=20v2=20=E2=80=94=20sp?= =?UTF-8?q?ike-verified=20integration=20points,=20SyncEmbedder,=2048-sessi?= =?UTF-8?q?on=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../2026-07-23-retrieval-quality-pr-a.md | 1048 ++++++++++------- 1 file changed, 600 insertions(+), 448 deletions(-) diff --git a/docs/superpowers/plans/2026-07-23-retrieval-quality-pr-a.md b/docs/superpowers/plans/2026-07-23-retrieval-quality-pr-a.md index 051d026..a289871 100644 --- a/docs/superpowers/plans/2026-07-23-retrieval-quality-pr-a.md +++ b/docs/superpowers/plans/2026-07-23-retrieval-quality-pr-a.md @@ -4,7 +4,7 @@ **Goal:** Replace popularity ranking with a Wilson-score prior + exploration slot, and add reflection/semantic embeddings with three-leg RRF query fusion. -**Architecture:** SQL keeps filtering; ranking moves to Python (`services/scoring.py`). The Ollama embedder is threaded into `ReflectionSynthesisService` and `SemanticMemoryService`; sync code reaches it through `async_bridge.run_async_in_worker`. Query relevance fuses prior rank + BM25 rank + vec-kNN rank via RRF. Everything embedding-related is best-effort and degrades to today's BM25-only behaviour. +**Architecture:** SQL keeps filtering; ranking moves to Python (`services/scoring.py`). Sync services reach Ollama through a new `SyncEmbedder` (fresh embedder per bridge-worker call — `httpx.AsyncClient` is loop-bound — with a 60s circuit breaker). Query relevance fuses prior rank + BM25 rank + vec-kNN rank via RRF. Every embedding path is best-effort and degrades to the shipped BM25-only behaviour. **Tech Stack:** Python 3.12, sqlite + sqlite-vec (vec0), FTS5, Ollama `nomic-embed-text` (768-dim), pytest. @@ -12,14 +12,29 @@ ## Global Constraints -- Branch: `feat/retrieval-quality` (exists; spec committed as `604fb79`). -- Test command: `./.venv/Scripts/python.exe -m pytest -v` from repo root (full suite: `./.venv/Scripts/python.exe -m pytest tests -q`, ~10 min, integration deselected by default). -- Typecheck: `./.venv/Scripts/python.exe -m pyright` must stay at 0 errors. -- Every Ollama touchpoint: best-effort, catch `Exception`, never raise into caller. -- `sqlite3.Connection` is bound to its creating thread — embeddings may be computed on the bridge worker thread, but ALL DB reads/writes stay on the caller thread. -- Ruff line length 100. -- Website sync guardrail (conf 1.0 reflection): `website/index.md` / `website/architecture.md` prose must match tool behaviour changes in the same PR. -- Commit after every task, message style `feat(scope): ...` / `test(scope): ...`, footer `Co-Authored-By: Claude Fable 5 `. +- Branch: `feat/retrieval-quality` (exists; spec `604fb79`). +- Test command: `./.venv/Scripts/python.exe -m pytest -v` from repo root (full suite: `./.venv/Scripts/python.exe -m pytest tests -q`). +- Typecheck: `./.venv/Scripts/python.exe -m pyright` stays at 0 errors. +- Every Ollama touchpoint: best-effort, never raises into the caller. +- `sqlite3.Connection` is thread-bound: embeds happen on the bridge worker; ALL DB I/O stays on the caller thread. +- Vector serialisation is `sqlite_vec.serialize_float32(vector)` everywhere (the proven call from `observation.py:206`); `db/connection.py` loads the extension on every connect, so vec0 works in tests and CLI without ceremony. +- `async_bridge.run_async_in_worker(coro_factory, *, timeout=None)` takes a **factory invoked inside the worker thread** — resources must be constructed inside it. +- Ruff line length 100. Website-sync guardrail applies (conf 1.0 reflection). +- Commit after every task; footer `Co-Authored-By: Claude Fable 5 `. + +## Verified-against-source facts (do not re-derive) + +| Fact | Where verified | +|---|---| +| `sqlite_vec.serialize_float32` is the vec0 write format | `services/observation.py:206` | +| kNN SQL: `WHERE embedding MATCH ? AND k = ? ORDER BY distance`, then filter ids in Python (no extra predicates allowed) | `search/hybrid.py:274-296` | +| Deleting vec0 rows is routine | `services/retention.py:250` | +| `OllamaEmbedder.__init__` builds `httpx.AsyncClient` immediately; client is loop-bound; ctor does NOT contact Ollama; `aclose()` closes owned clients | `embeddings/ollama.py:52-107` | +| `run_async_in_worker` signature + fresh-loop-per-call semantics | `async_bridge.py:27-60` | +| `_apply_new` writes title/use_cases/hints from `NewAction` fields directly | `reflection.py:718-775` | +| `_apply_augment` SELECTs only `hints, confidence, status` today; UPDATE branches on `rewrite_use_cases`; title never changes | `reflection.py:817-919` | +| `_apply_merge` changes NO text on the target (counters/evidence only); source becomes `superseded` | `reflection.py:1000-1040` | +| Action dataclasses: `NewAction(title, phase, polarity, use_cases, hints, tech, confidence, source_observation_ids)` etc. | `reflection.py:153-180` | --- @@ -30,7 +45,7 @@ - Test: `tests/services/test_scoring.py` **Interfaces:** -- Produces: `wilson_lower_bound(positive: int, n: int, z: float = 1.96) -> float` — 0.0 when `n == 0`; never negative. Consumed by Tasks 2, 3, 4. +- Produces: `wilson_lower_bound(positive: int, n: int, z: float = 1.96) -> float` — 0.0 when `n == 0`; clamped to [0, 1]. Consumed by Tasks 2, 3, 4. - [ ] **Step 1: Write the failing tests** @@ -142,15 +157,14 @@ Co-Authored-By: Claude Fable 5 " ### Task 2: Reflections ranked by Wilson score **Files:** -- Modify: `better_memory/services/reflection.py` (retrieve_reflections, ~lines 1256-1400) -- Modify: `better_memory/services/memory_rating.py` (delete `IGNORED_DEMOTION_FLOOR`, `IGNORED_DEMOTION_WEIGHT`, `OVERLOOKED_RANKING_WEIGHT`) -- Modify: `tests/services/test_useful_count_ranking.py` -- Delete: `tests/services/test_ignored_demotion.py` (superseded — its scenarios move to the new test file) +- Modify: `better_memory/services/reflection.py` (retrieve_reflections ~1256-1400) +- Modify: `better_memory/services/memory_rating.py` (delete the three ranking constants) +- Delete: `tests/services/test_ignored_demotion.py`, `tests/services/test_useful_count_ranking.py` (both pin the superseded ordering; scenarios covered below) - Test: `tests/services/test_wilson_ranking.py` **Interfaces:** -- Consumes: `wilson_lower_bound` from Task 1. -- Produces: `retrieve_reflections` rows now include `times_overlooked` and `times_ignored`; SQL no longer orders; Python sorts by `(wilson DESC, confidence DESC, updated_at DESC)`. `_wilson_rated(row) -> int` and `_wilson_score(row) -> float` module-level helpers in `reflection.py` — Task 4 reuses `_wilson_rated` for the untested test. +- Consumes: `wilson_lower_bound` (Task 1). +- Produces: rows include `times_overlooked` + `times_ignored`; SQL no longer orders; Python sorts `(wilson DESC, confidence DESC, updated_at DESC)`. Module helpers `_wilson_rated(row) -> int`, `_wilson_score(row) -> float` (Task 4 reuses `_wilson_rated`). - [ ] **Step 1: Write the failing tests** @@ -202,11 +216,6 @@ def _ids(conn, **kw): return [r["id"] for r in svc.retrieve_reflections(project="p", **kw)["do"]] -class TestWilsonRanking: - def test_hit_rate_beats_raw_count(self): - pass # replaced below — see class body - - class TestWilsonOrdering: def test_hit_rate_beats_raw_count(self, conn): _seed(conn, "r-workhorse", useful=67, ignored=125) # 67/192 ~ 0.28 @@ -248,27 +257,21 @@ class TestWilsonOrdering: assert not hasattr(mr, name), f"{name} should be deleted" ``` -Delete the placeholder `TestWilsonRanking` class (editing artifact) before running; keep only `TestWilsonOrdering`. - - [ ] **Step 2: Run tests to verify they fail** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_wilson_ranking.py -v` -Expected: FAIL — ordering assertions (current code ranks r-workhorse first) and `test_demotion_constants_are_gone`. +Expected: FAIL — ordering assertions and `test_demotion_constants_are_gone`. - [ ] **Step 3: Implement** -In `better_memory/services/reflection.py`: - -3a. Replace the import of the deleted constants (top of file): +3a. Imports in `reflection.py` — replace the constants import with: ```python from better_memory.search.query import sanitize_fts5_query from better_memory.services.scoring import wilson_lower_bound ``` -(remove the `from better_memory.services.memory_rating import (IGNORED_DEMOTION_FLOOR, ...)` block). - -3b. Add module-level helpers directly above `class ReflectionSynthesisService`: +3b. Module helpers above the service class: ```python def _wilson_rated(row) -> int: @@ -282,11 +285,9 @@ def _wilson_score(row) -> float: return wilson_lower_bound(positive, _wilson_rated(row)) ``` -3c. In `retrieve_reflections`, replace the SELECT (drop the demotion params and ORDER BY; add the two counter columns; sort in Python): +3c. In `retrieve_reflections`: delete the two demotion `params.append(...)` lines; SELECT gains `times_overlooked, times_ignored`; ORDER BY removed; Python sort after fetch: ```python - where = " AND ".join(clauses) - _diag.step(fn, "executing_select") rows = self._conn.execute( f""" SELECT id, title, phase, polarity, use_cases, hints, @@ -308,23 +309,19 @@ def _wilson_score(row) -> float: rows.sort(key=_wilson_score, reverse=True) ``` -(`params` no longer receives the two demotion appends — delete those lines too.) - -3d. The bucket-fill loop and `_fuse_by_relevance` call stay as they are, but the bucket dicts must carry the new counters — extend the `bucket.append({...})` literal with: +3d. Bucket item dicts gain the two counters: ```python "times_overlooked": r["times_overlooked"], "times_ignored": r["times_ignored"], ``` -In `better_memory/services/memory_rating.py`: delete the `OVERLOOKED_RANKING_WEIGHT`, `IGNORED_DEMOTION_FLOOR`, `IGNORED_DEMOTION_WEIGHT` constants and their comment blocks. - -In `tests/services/test_useful_count_ranking.py`: this file pins the OLD ordering (`useful_count` beats confidence regardless of ignores). Rewrite its two assertions to the Wilson contract or delete the file if fully covered by `test_wilson_ranking.py` — it is fully covered; delete it. Also delete `tests/services/test_ignored_demotion.py` (scenarios live on in `test_proven_dead_weight_sinks_below_modest_performer` + `test_demotion_constants_are_gone`). +3e. `memory_rating.py`: delete `OVERLOOKED_RANKING_WEIGHT`, `IGNORED_DEMOTION_FLOOR`, `IGNORED_DEMOTION_WEIGHT` and their comments. Delete the two superseded test files. - [ ] **Step 4: Run the affected tests** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_wilson_ranking.py tests/services/test_reflection_query_relevance.py tests/services/test_reflection_retrieve_fields.py tests/mcp -q` -Expected: all pass (query-relevance tests still pass — fusion consumes the new ordering transparently). +Expected: all pass. - [ ] **Step 5: Commit** @@ -340,14 +337,14 @@ Co-Authored-By: Claude Fable 5 " ### Task 3: Semantic memories ranked by Wilson score **Files:** -- Modify: `better_memory/services/semantic.py` (list_for_project SELECT/ORDER, ~lines 237-260; imports ~19-23; SemanticMemory dataclass) -- Test: `tests/services/test_semantic.py` (add class; existing tests keep passing) +- Modify: `better_memory/services/semantic.py` (imports ~19-23; dataclass ~26-42; list_for_project ~237-260) +- Test: `tests/services/test_semantic.py` (append class) **Interfaces:** - Consumes: `wilson_lower_bound` (Task 1). -- Produces: `SemanticMemory` dataclass gains `times_ignored: int = 0`; `list_for_project` ordered by `(wilson DESC, created_at DESC)`. +- Produces: `SemanticMemory` gains `times_ignored: int = 0`, `last_ignored_at: str | None = None`; `list_for_project` ordered `(wilson DESC, created_at DESC)`. -- [ ] **Step 1: Write the failing test** (append to `tests/services/test_semantic.py`, reusing that file's existing fixtures/helpers for creating memories — check its seed helper name before writing): +- [ ] **Step 1: Write the failing test** — append to `tests/services/test_semantic.py`, reusing its existing `conn` fixture/helpers (read the file's fixture name first; monkeypatch the service clock if creation timestamps collide, as neighbouring tests there do): ```python class TestSemanticWilsonRanking: @@ -374,21 +371,12 @@ class TestSemanticWilsonRanking: assert ids == [rated, unrated] ``` -(Adapt the fixture name to the file's existing `conn` fixture; if creation timestamps collide within the test, monkeypatch the service clock as neighbouring tests in that file do.) - - [ ] **Step 2: Run to verify failure** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_semantic.py -v -k Wilson` -Expected: FAIL (current order puts the 67-count row first; `times_ignored` not selected). +Expected: FAIL. -- [ ] **Step 3: Implement** - -In `better_memory/services/semantic.py`: - -- Imports: replace the three-constant import block with `from better_memory.services.scoring import wilson_lower_bound`. -- `SemanticMemory` dataclass: add `times_ignored: int = 0` and `last_ignored_at: str | None = None` after the overlooked fields. -- `list_for_project` SQL: add `times_ignored, last_ignored_at` to the SELECT column list; replace the ORDER BY block and its three `params.append(...)` lines with plain `"ORDER BY created_at DESC"` (recency pre-sort; final order applied in Python). -- After `rows = self._conn.execute(sql, params).fetchall()` build `results` as now (passing the two new fields into the dataclass), then sort: +- [ ] **Step 3: Implement** — imports: `from better_memory.services.scoring import wilson_lower_bound` (drop the old constants import); dataclass fields added; SELECT gains `times_ignored, last_ignored_at`; ORDER BY block + its `params.append` lines replaced with `"ORDER BY created_at DESC"`; after building `results`: ```python results.sort( @@ -400,9 +388,9 @@ In `better_memory/services/semantic.py`: ) ``` -(Stable sort preserves the SQL `created_at DESC` order inside equal scores.) +(Stable sort keeps `created_at DESC` inside equal scores.) -- [ ] **Step 4: Run the semantic + bootstrap tests** +- [ ] **Step 4: Run** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_semantic.py tests/services/test_session_bootstrap.py -q` Expected: all pass. @@ -421,12 +409,12 @@ Co-Authored-By: Claude Fable 5 " ### Task 4: Exploration slot **Files:** -- Modify: `better_memory/services/reflection.py` (bucket-fill loop in retrieve_reflections) +- Modify: `better_memory/services/reflection.py` (bucket-fill in retrieve_reflections) - Test: `tests/services/test_exploration_slot.py` **Interfaces:** -- Consumes: `_wilson_rated` (Task 2). Untested ≡ `_wilson_rated(row) < 3` — the constant lives as `EXPLORATION_RATED_FLOOR = 3` in `reflection.py`. -- Produces: per-bucket shortlist = up to `cap-1` best tested rows + the best untested row (fused/ranked order), when both kinds exist. `cap is None` (bootstrap) ⇒ no reservation. +- Consumes: `_wilson_rated` (Task 2). `EXPLORATION_RATED_FLOOR = 3` constant in `reflection.py`. +- Produces: per bucket, up to `cap-1` tested rows + best untested row (ranked order); no untested ⇒ plain fill; `cap None` ⇒ no reservation. `_bucket_item(self, r) -> dict` extracted so both fill paths share the dict literal. - [ ] **Step 1: Write the failing tests** @@ -518,11 +506,11 @@ class TestExplorationSlot: - [ ] **Step 2: Run to verify failure** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_exploration_slot.py -v` -Expected: `test_last_slot_goes_to_best_untested` and `test_two_ratings_is_still_untested_three_is_not` FAIL (untested rows sort last today and never enter a full bucket). +Expected: `test_last_slot_goes_to_best_untested` and `test_two_ratings_is_still_untested_three_is_not` FAIL. - [ ] **Step 3: Implement** -In `reflection.py`, add near `_wilson_score`: +Constant near `_wilson_score`: ```python #: A memory with fewer than this many rated exposures is "untested": its @@ -531,74 +519,40 @@ In `reflection.py`, add near `_wilson_score`: EXPLORATION_RATED_FLOOR = 3 ``` -Replace the bucket-fill loop (currently: iterate `rows`, append until `cap`) with: +Extract the existing `bucket.append({...})` literal into `_bucket_item(self, r) -> dict` (now including the Task 2 counters). Replace the fill loop with the two-pass version (`rows` is already in final ranked order — Wilson sort, then optional fusion — so "best untested" = first untested in `rows`; index-based selection avoids `in`-list identity comparisons entirely): ```python cap = limit_per_bucket if limit_per_bucket is not None else sys.maxsize reserve = limit_per_bucket is not None and cap >= 2 buckets: dict[str, list[dict]] = {"do": [], "dont": [], "neutral": []} - # rows is in final ranked order (Wilson sort, then optional - # relevance fusion). Untested rows keep that order too — the - # first untested row in `rows` is the most query-relevant one. - explorers: dict[str, dict] = {} - for r in rows: - bucket = buckets[r["polarity"]] - untested = _wilson_rated(r) < EXPLORATION_RATED_FLOOR - if reserve and untested and r["polarity"] not in explorers: - explorers[r["polarity"]] = self._bucket_item(r) - continue - if len(bucket) >= (cap - 1 if reserve else cap): - continue - bucket.append(self._bucket_item(r)) - # Fill the reserved slot: best untested if one exists, else the - # next proven row that was displaced by the reservation. - if reserve: - for polarity, bucket in buckets.items(): - if polarity in explorers: - bucket.append(explorers[polarity]) - else: - continue -``` - -Then handle the no-untested case: the loop above under-fills by one when no untested row exists for a bucket. Implement precisely by two passes instead if clearer — the contract the tests pin is: - -1. `cap-1` best tested rows in ranked order, -2. slot `cap` = best untested if any, else next best tested, -3. `cap None` ⇒ plain fill, no reservation. - -A clean two-pass implementation (preferred — use this one): - -```python - cap = limit_per_bucket if limit_per_bucket is not None else sys.maxsize - reserve = limit_per_bucket is not None and cap >= 2 - buckets = {"do": [], "dont": [], "neutral": []} by_polarity: dict[str, list] = {"do": [], "dont": [], "neutral": []} for r in rows: by_polarity[r["polarity"]].append(r) for polarity, group in by_polarity.items(): - bucket = buckets[polarity] if not reserve: - bucket.extend(self._bucket_item(r) for r in group[:cap]) + buckets[polarity] = [self._bucket_item(r) for r in group[:cap]] continue - tested = [r for r in group - if _wilson_rated(r) >= EXPLORATION_RATED_FLOOR] - untested = [r for r in group - if _wilson_rated(r) < EXPLORATION_RATED_FLOOR] - chosen = tested[: cap - 1] - if untested: - chosen.append(untested[0]) - remaining = [r for r in group if r not in chosen] - chosen.extend(remaining[: cap - len(chosen)]) - bucket.extend(self._bucket_item(r) for r in chosen) + tested_idx = [i for i, r in enumerate(group) + if _wilson_rated(r) >= EXPLORATION_RATED_FLOOR] + untested_idx = [i for i, r in enumerate(group) + if _wilson_rated(r) < EXPLORATION_RATED_FLOOR] + chosen = tested_idx[: cap - 1] + if untested_idx: + chosen.append(untested_idx[0]) + if len(chosen) < cap: # top up from the remainder + taken = set(chosen) + for i in range(len(group)): + if len(chosen) >= cap: + break + if i not in taken: + chosen.append(i) + chosen.sort() # preserve ranked order + buckets[polarity] = [self._bucket_item(group[i]) for i in chosen] ``` -Extract the existing `bucket.append({...})` dict literal into a method `_bucket_item(self, r) -> dict` so both call sites share it (it now includes the Task 2 counter fields). - -Note: `sqlite3.Row` supports `in`-list identity checks because each row object is unique per fetch — `r not in chosen` compares identity, which is correct here (same objects from one fetch). - -Update the `_diag.step(fn, "bucketed", ...)` call to follow the new loop. +Update the `_diag.step(fn, "bucketed", ...)` counts to read from the new dict. -- [ ] **Step 4: Run the reflection test files** +- [ ] **Step 4: Run** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_exploration_slot.py tests/services/test_wilson_ranking.py tests/services/test_reflection_query_relevance.py tests/services/test_reflection.py tests/services/test_reflection_retrieve_fields.py -q` Expected: all pass. @@ -618,12 +572,12 @@ Co-Authored-By: Claude Fable 5 " **Files:** - Create: `better_memory/db/migrations/0014_semantic_embeddings.sql` -- Test: append to `tests/db/test_schema.py` (follow that file's existing per-migration test style — read it first and mirror the newest migration's test) +- Test: append to `tests/db/test_schema.py` (mirror the newest migration's existing test style) **Interfaces:** -- Produces: vec0 table `semantic_embeddings(memory_id TEXT PRIMARY KEY, embedding FLOAT[768])`, consumed by Tasks 7-9. +- Produces: vec0 table `semantic_embeddings(memory_id TEXT PRIMARY KEY, embedding FLOAT[768])`. Consumed by Tasks 8-10. -- [ ] **Step 1: Write the failing test** (mirroring the file's existing pattern — a test that applies migrations to a fresh DB and asserts the table exists): +- [ ] **Step 1: Write the failing test** ```python def test_0014_semantic_embeddings_table(tmp_memory_db): @@ -639,7 +593,7 @@ def test_0014_semantic_embeddings_table(tmp_memory_db): - [ ] **Step 2: Run to verify failure** Run: `./.venv/Scripts/python.exe -m pytest tests/db -q -k 0014` -Expected: FAIL — table absent. +Expected: FAIL. - [ ] **Step 3: Write the migration** @@ -661,7 +615,7 @@ CREATE VIRTUAL TABLE semantic_embeddings USING vec0( - [ ] **Step 4: Run** Run: `./.venv/Scripts/python.exe -m pytest tests/db -q` -Expected: all pass (including any schema-drift tests — if one hardcodes the migration count/list, update it). +Expected: all pass (update any migration-count fixture if one exists). - [ ] **Step 5: Commit** @@ -674,17 +628,240 @@ Co-Authored-By: Claude Fable 5 " --- -### Task 6: Embedder in ReflectionSynthesisService — write-path embedding +### Task 6: SyncEmbedder — bridge + fresh-embedder-per-call + circuit breaker + +**Files:** +- Create: `better_memory/embeddings/sync_embed.py` +- Create: `tests/services/_embedding_fakes.py` +- Test: `tests/embeddings/test_sync_embed.py` + +**Interfaces:** +- Consumes: `run_async_in_worker` (factory semantics), any object with async `embed(text)` / `embed_batch(texts)` and optional async `aclose()`. +- Produces: `SyncEmbedder(factory, *, clock=time.monotonic, cooldown=60.0, timeout=15.0)` with `embed_text(text) -> list[float] | None` and `embed_batch(texts) -> list[list[float]] | None`. Any failure opens a `cooldown`-second breaker during which calls return None instantly. Consumed by Tasks 7, 8, 9. + +**Why fresh-embedder-per-call:** `OllamaEmbedder.__init__` builds an `httpx.AsyncClient`, and AsyncClient is bound to the event loop that first uses it (recorded project gotcha). `run_async_in_worker` creates a fresh loop per call, so the coroutine must construct its own embedder inside the worker and close it there. Construction is cheap and does not contact Ollama (`ollama.py` docstring). + +- [ ] **Step 1: Write the fakes + failing tests** + +```python +# tests/services/_embedding_fakes.py +"""Shared fake embedder for embedding-path tests.""" +from __future__ import annotations + + +class FakeEmbedder: + def __init__(self, fail: bool = False): + self.calls: list = [] + self.closed = 0 + self.fail = fail + + async def embed(self, text: str) -> list[float]: + self.calls.append(text) + if self.fail: + raise RuntimeError("ollama down") + return [0.1] * 768 + + async def embed_batch(self, texts: list[str]) -> list[list[float]]: + self.calls.append(list(texts)) + if self.fail: + raise RuntimeError("ollama down") + return [[0.1] * 768 for _ in texts] + + async def aclose(self) -> None: + self.closed += 1 +``` + +```python +# tests/embeddings/test_sync_embed.py +"""SyncEmbedder: sync facade over the async embedder for thread-bound code. + +Fresh embedder per call (loop-bound AsyncClient), closed in the worker; +60s circuit breaker so an Ollama outage costs one stall per cooldown +window instead of one per call. +""" +from __future__ import annotations + +from better_memory.embeddings.sync_embed import SyncEmbedder +from tests.services._embedding_fakes import FakeEmbedder + + +class FakeClock: + def __init__(self): + self.t = 1000.0 + + def __call__(self) -> float: + return self.t + + +class TestSyncEmbedder: + def test_embed_text_returns_vector_and_closes_embedder(self): + fake = FakeEmbedder() + s = SyncEmbedder(lambda: fake) + vec = s.embed_text("hello") + assert vec is not None and len(vec) == 768 + assert fake.calls == ["hello"] + assert fake.closed == 1 + + def test_embed_batch_returns_vectors(self): + fake = FakeEmbedder() + s = SyncEmbedder(lambda: fake) + out = s.embed_batch(["a", "b"]) + assert out is not None and len(out) == 2 + + def test_failure_returns_none_and_opens_breaker(self): + fake = FakeEmbedder(fail=True) + clock = FakeClock() + s = SyncEmbedder(lambda: fake, clock=clock) + assert s.embed_text("x") is None + assert fake.calls == ["x"] + # Breaker open: the embedder is not touched again. + assert s.embed_text("y") is None + assert fake.calls == ["x"] + + def test_breaker_closes_after_cooldown(self): + clock = FakeClock() + calls = [] + + class FlakyThenGood(FakeEmbedder): + async def embed(self, text): + calls.append(text) + if len(calls) == 1: + raise RuntimeError("down") + return [0.1] * 768 + + s = SyncEmbedder(FlakyThenGood, clock=clock, cooldown=60.0) + assert s.embed_text("first") is None + clock.t += 61.0 + assert s.embed_text("second") is not None + assert calls == ["first", "second"] + + def test_none_factory_disables_everything(self): + s = SyncEmbedder(None) + assert s.embed_text("x") is None + assert s.embed_batch(["x"]) is None + + def test_embedder_without_aclose_is_fine(self): + class Bare: + async def embed(self, text): + return [0.1] * 768 + + s = SyncEmbedder(Bare) + assert s.embed_text("x") is not None +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/embeddings/test_sync_embed.py -v` +Expected: FAIL — module not found. (Create `tests/embeddings/__init__.py` if the directory is new.) + +- [ ] **Step 3: Implement** + +```python +# better_memory/embeddings/sync_embed.py +"""Sync facade over the async embedder, for thread-bound sync services. + +Two hazards this class exists to contain: + +1. ``httpx.AsyncClient`` is bound to the event loop that first uses it, + and ``run_async_in_worker`` runs a fresh loop per call — so the + embedder must be CONSTRUCTED inside the worker coroutine and closed + there. The factory is invoked per call; ``OllamaEmbedder`` construction + is cheap and contacts nothing. +2. Retrieval must never hang on a dead Ollama. Any failure opens a + circuit breaker: for ``cooldown`` seconds every call returns ``None`` + immediately. Worst case is one bounded stall per cooldown window. + +Returns ``None`` on every failure path — callers treat a missing vector +as "no vec leg", never as an error. +""" + +from __future__ import annotations + +import time +from collections.abc import Callable +from typing import Any + +from better_memory.async_bridge import run_async_in_worker + +#: Bridge-level hard stop. The wiring uses OllamaEmbedder(timeout=5.0, +#: max_retries=1), so a healthy-but-slow call ends well inside this; the +#: bridge timeout is the backstop against pathological hangs. +_WORKER_TIMEOUT = 15.0 +_DEFAULT_COOLDOWN = 60.0 + + +class SyncEmbedder: + def __init__( + self, + factory: Callable[[], Any] | None, + *, + clock: Callable[[], float] = time.monotonic, + cooldown: float = _DEFAULT_COOLDOWN, + timeout: float = _WORKER_TIMEOUT, + ) -> None: + self._factory = factory + self._clock = clock + self._cooldown = cooldown + self._timeout = timeout + self._down_until = 0.0 + + def embed_text(self, text: str) -> list[float] | None: + return self._run(lambda emb: emb.embed(text)) + + def embed_batch(self, texts: list[str]) -> list[list[float]] | None: + return self._run(lambda emb: emb.embed_batch(texts)) + + def _run(self, op: Callable[[Any], Any]): + if self._factory is None: + return None + if self._clock() < self._down_until: + return None + + factory = self._factory + + async def _go(): + emb = factory() + try: + return await op(emb) + finally: + aclose = getattr(emb, "aclose", None) + if aclose is not None: + await aclose() + + try: + return run_async_in_worker(_go, timeout=self._timeout) + except Exception: + self._down_until = self._clock() + self._cooldown + return None +``` + +- [ ] **Step 4: Run** + +Run: `./.venv/Scripts/python.exe -m pytest tests/embeddings/test_sync_embed.py -v` +Expected: 6 passed + +- [ ] **Step 5: Commit** + +```bash +git add better_memory/embeddings/sync_embed.py tests/embeddings tests/services/_embedding_fakes.py +git commit -m "feat(embeddings): SyncEmbedder — bridge facade with fresh-per-call embedder and 60s breaker + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 7: Reflection write-path embedding **Files:** -- Modify: `better_memory/services/reflection.py` (ctor line ~317; `_apply_new` ~718, `_apply_augment` ~817, `_apply_merge` ~921) -- Modify: `better_memory/mcp/server.py:222` (pass embedder) -- Modify: `better_memory/storage/sqlite.py:68` (pass embedder — its ctor already receives one for observations; check the field name, likely `self._embedder` or the ctor arg) +- Modify: `better_memory/services/reflection.py` (ctor ~317; `_apply_new` ~718; `_apply_augment` ~817; `_apply_merge` ~921) +- Modify: `better_memory/mcp/server.py` (~172-230: build `SyncEmbedder`, pass to services) +- Modify: `better_memory/storage/sqlite.py:68` (same) - Test: `tests/services/test_reflection_embedding_write.py` **Interfaces:** -- Consumes: `OllamaEmbedder.embed(text) -> list[float]` (async), `run_async_in_worker` from `better_memory.async_bridge`. -- Produces: `ReflectionSynthesisService(conn, *, clock=None, embedder=None)`; `_embed_text(self, text) -> list[float] | None` (sync, best-effort); `_embedding_source_text(title, use_cases, hints) -> str` module function; embeddings written to `reflection_embeddings` on new/augment/merge. Tasks 8-9 reuse `_embed_text` and `_embedding_source_text`. +- Consumes: `SyncEmbedder` (Task 6), `sqlite_vec.serialize_float32`. +- Produces: `ReflectionSynthesisService(conn, *, clock=None, sync_embedder=None)`; module function `_embedding_source_text(title, use_cases, hints) -> str`; method `_store_embedding(reflection_id, vector | None)`. Embedding written on new + augment; merge deletes the superseded source's row (target text does not change — verified). Wiring: `server.py` builds ONE shared `SyncEmbedder(lambda: OllamaEmbedder(timeout=5.0, max_retries=1))` when `config.embeddings_backend == "ollama"`, else `None`, and passes it to both this service and Task 8's; `storage/sqlite.py` mirrors this using the embedder presence it already knows about. - [ ] **Step 1: Write the failing tests** @@ -693,8 +870,7 @@ Co-Authored-By: Claude Fable 5 " """Synthesis writes reflection embeddings, best-effort. reflection_embeddings sat at 0 rows from migration 0002 until this change: -the write path simply never embedded. Ollama failures must never block -synthesis — a missing embedding degrades retrieval to BM25, nothing more. +the write path simply never embedded. Failures must never block synthesis. """ from __future__ import annotations @@ -704,26 +880,12 @@ import pytest from better_memory.db.connection import connect from better_memory.db.schema import apply_migrations -from better_memory.embeddings.ollama import EmbeddingError +from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.services.reflection import ( ReflectionSynthesisService, _embedding_source_text, ) - - -class FakeEmbedder: - def __init__(self, fail=False): - self.calls: list[str] = [] - self.fail = fail - - async def embed(self, text: str) -> list[float]: - self.calls.append(text) - if self.fail: - raise EmbeddingError("ollama down") - return [0.1] * 768 - - async def embed_batch(self, texts: list[str]) -> list[list[float]]: - return [await self.embed(t) for t in texts] +from tests.services._embedding_fakes import FakeEmbedder @pytest.fixture @@ -740,57 +902,26 @@ def _vec_count(conn): return conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] -class TestEmbeddingSourceText: - def test_joins_title_use_cases_hints(self): - text = _embedding_source_text("T", "when X", ["h1", "h2"]) - assert text == "T\nwhen X\nh1\nh2" - - -class TestWritePathEmbedding: - def test_embed_text_none_embedder_returns_none(self, conn): - svc = ReflectionSynthesisService(conn) - assert svc._embed_text("anything") is None - - def test_embed_text_failure_returns_none(self, conn): - svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder(fail=True)) - assert svc._embed_text("anything") is None - - def test_embed_text_success_returns_vector(self, conn): - svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder()) - vec = svc._embed_text("anything") - assert vec is not None and len(vec) == 768 +def test_embedding_source_text_joins_fields(): + assert _embedding_source_text("T", "when X", ["h1", "h2"]) == "T\nwhen X\nh1\nh2" ``` -Plus one integration-style test per apply path. The apply methods take action -dataclasses — read `_apply_new`'s signature and the `NewAction` dataclass -(`reflection.py` ~line 136) first, then write: - -```python -class TestApplyPathsEmbed: - def test_apply_new_writes_embedding_row(self, conn): - fake = FakeEmbedder() - svc = ReflectionSynthesisService(conn, embedder=fake) - # Construct the minimal NewAction the codebase defines (check - # dataclass fields; fill required ones only) and call _apply_new - # via the public apply_decision path used by synthesize tests — - # mirror tests/services/test_reflection_writes.py setup. - ... - assert _vec_count(conn) == 1 - assert fake.calls # source text was embedded - - def test_apply_new_with_failing_embedder_still_writes_reflection(self, conn): - svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder(fail=True)) - ... - assert _vec_count(conn) == 0 # no embedding - # reflection row exists — synthesis unaffected -``` - -**Concrete instruction for the `...`:** copy the action-construction and -`apply_decision` invocation verbatim from the nearest existing test in -`tests/services/test_reflection_writes.py` (it exercises `_apply_new` / -augment / merge already); add the embedder kwarg and the two assertions. -Do the same for augment (asserts a second `fake.calls` entry after augment) -and merge (embedding row for the merge target). +Then one test per apply path. **Concrete instruction:** open +`tests/services/test_reflection_writes.py`, copy its working setup for +driving `_apply_new` / `_apply_augment` / `_apply_merge` (episode + observation +seeding and action construction) verbatim into this file, then assert: + +- new: `_vec_count(conn) == 1` and the fake recorded one embed whose text + contains the action's title; +- new with `SyncEmbedder(lambda: FakeEmbedder(fail=True))`: reflection row + exists, `_vec_count(conn) == 0`; +- augment: `_vec_count` still 1 for that id and the fake recorded a second + embed containing the appended hint; +- merge: after merging source→target, the SOURCE's embedding row is gone + (`SELECT COUNT(*) FROM reflection_embeddings WHERE reflection_id = source_id` + is 0) and the fake recorded NO new embed for the target (target text + unchanged — verified against `_apply_merge`, which updates counters only); +- no `sync_embedder` passed: `_vec_count(conn) == 0`, everything else works. - [ ] **Step 2: Run to verify failure** @@ -799,7 +930,11 @@ Expected: FAIL — `_embedding_source_text` import error. - [ ] **Step 3: Implement** -In `reflection.py`: +Module bits in `reflection.py`: + +```python +import sqlite_vec +``` ```python def _embedding_source_text(title: str, use_cases: str, hints: list[str]) -> str: @@ -815,33 +950,23 @@ Ctor: conn: sqlite3.Connection, *, clock: Callable[[], datetime] | None = None, - embedder: Any = None, + sync_embedder: "SyncEmbedder | None" = None, ) -> None: self._conn = conn self._clock = clock or default_clock - self._embedder = embedder + self._sync_embedder = sync_embedder ``` -Sync bridge helper (method): +(import `SyncEmbedder` under `TYPE_CHECKING` or directly — direct is fine, +no cycle: `embeddings.sync_embed` imports only `async_bridge`.) -```python - def _embed_text(self, text: str) -> list[float] | None: - """Best-effort sync embedding. None embedder / any failure -> None. - - The embed call runs on the bridge worker thread; the DB write that - consumes the vector stays on the caller thread (sqlite3 conns are - thread-bound). - """ - if self._embedder is None: - return None - try: - return run_async_in_worker(self._embedder.embed(text)) - except Exception: - return None +Store helper (method): +```python def _store_embedding(self, reflection_id: str, vector: list[float] | None) -> None: if vector is None: return + # DELETE+INSERT: vec0 tables historically mishandle UPSERT. self._conn.execute( "DELETE FROM reflection_embeddings WHERE reflection_id = ?", (reflection_id,), @@ -849,35 +974,72 @@ Sync bridge helper (method): self._conn.execute( "INSERT INTO reflection_embeddings (reflection_id, embedding) " "VALUES (?, ?)", - (reflection_id, _serialize_vector(vector)), + (reflection_id, sqlite_vec.serialize_float32(vector)), ) ``` -For `_serialize_vector`, reuse whatever `ObservationService.create` does at -`observation.py:204-210` to insert into `observation_embeddings` (it already -solves vec0 serialisation — likely `sqlite_vec.serialize_float32` or a JSON -string; copy that exact call, import from the same place). DELETE+INSERT -rather than INSERT OR REPLACE because vec0 virtual tables historically -mishandle UPSERT. +Call sites (all inside the existing transactions): -Imports: `from better_memory.async_bridge import run_async_in_worker` and -`from typing import Any` (if not present). +- `_apply_new`, after the `reflection_sources` inserts: -Call sites — at the end of the row-write in each of `_apply_new`, -`_apply_augment`, `_apply_merge` (inside their existing transaction, after -the reflection row is written, before commit): +```python + if self._sync_embedder is not None: + self._store_embedding( + reflection_id, + self._sync_embedder.embed_text(_embedding_source_text( + action.title, action.use_cases, action.hints, + )), + ) +``` + +- `_apply_augment`: extend the row SELECT to + `"SELECT title, use_cases, hints, confidence, status FROM reflections WHERE id = ?"`, + and after the UPDATE: ```python - self._store_embedding( - reflection_id, - self._embed_text(_embedding_source_text(title, use_cases, hints)), - ) + if self._sync_embedder is not None: + final_use_cases = (action.rewrite_use_cases + if action.rewrite_use_cases is not None + else row["use_cases"]) + self._store_embedding( + action.reflection_id, + self._sync_embedder.embed_text(_embedding_source_text( + row["title"], final_use_cases, merged_hints, + )), + ) ``` -(with the local variable names each method actually has — augment/merge -re-read the post-edit title/use_cases/hints they just wrote). +- `_apply_merge`: after the source's `superseded` UPDATE: -Wiring: `server.py:222` → `ReflectionSynthesisService(memory_conn, embedder=embedder)`; `storage/sqlite.py:68` → pass the embedder SqliteBackend already holds for observations (check its ctor field name and thread it). +```python + # Target text is untouched by merge (counters/evidence only), so + # no re-embed; drop the superseded source's vector so it stops + # competing for kNN slots. + self._conn.execute( + "DELETE FROM reflection_embeddings WHERE reflection_id = ?", + (action.source_id,), + ) +``` + +Wiring — `server.py` after the embedder block (~line 178): + +```python + from better_memory.embeddings.sync_embed import SyncEmbedder + + sync_embedder: SyncEmbedder | None = None + if embedder is not None: + # Fresh short-timeout embedder per bridge call (loop-bound client); + # shared instance so the breaker state is process-wide. + sync_embedder = SyncEmbedder( + lambda: OllamaEmbedder(timeout=5.0, max_retries=1) + ) +``` + +then `ReflectionSynthesisService(memory_conn, sync_embedder=sync_embedder)` at +line ~222. `storage/sqlite.py:68`: build the same `SyncEmbedder` from the +embedder the backend already receives (`sync_embedder = SyncEmbedder(lambda: +OllamaEmbedder(timeout=5.0, max_retries=1)) if embedder is not None else None`) +and pass it. Move imports to module top per house style. - [ ] **Step 4: Run** @@ -888,25 +1050,25 @@ Expected: all pass. ```bash git add -A -git commit -m "feat(embeddings): reflections embedded at synthesis write time, best-effort +git commit -m "feat(embeddings): reflections embedded at synthesis write time via SyncEmbedder Co-Authored-By: Claude Fable 5 " ``` --- -### Task 7: Embedder in SemanticMemoryService +### Task 8: Semantic write-path embedding **Files:** -- Modify: `better_memory/services/semantic.py` (ctor ~54; `create` ~63; `update_text` ~85; `create_from_observation` ~125) -- Modify: instantiation sites — grep `SemanticMemoryService(` in `better_memory/mcp/server.py` and `better_memory/storage/sqlite.py`, pass `embedder=` +- Modify: `better_memory/services/semantic.py` (ctor; `create`; `update_text`; `create_from_observation`) +- Modify: instantiation sites (grep `SemanticMemoryService(` in `mcp/server.py` + `storage/sqlite.py`; pass `sync_embedder=` from Task 7's shared instance) - Test: `tests/services/test_semantic_embedding_write.py` **Interfaces:** -- Consumes: bridge + serialisation exactly as Task 6 (duplicate `_embed_text`/`_store_embedding` shape into this service, table `semantic_embeddings`, PK `memory_id`; source text = the memory's `content`). -- Produces: `SemanticMemoryService(conn, *, clock=None, embedder=None)`; embeddings written on `create`, `update_text`, `create_from_observation`. +- Consumes: `SyncEmbedder` (Task 6). Source text = the memory's `content`. Table `semantic_embeddings(memory_id, embedding)` (Task 5). +- Produces: `SemanticMemoryService(conn, *, clock=None, sync_embedder=None)` with a private `_store_embedding(memory_id, vector | None)` mirroring Task 7's (DELETE+INSERT). -- [ ] **Step 1: Write the failing tests** (same FakeEmbedder — move it to `tests/services/_embedding_fakes.py` and import from both test files): +- [ ] **Step 1: Write the failing tests** ```python # tests/services/test_semantic_embedding_write.py @@ -918,6 +1080,7 @@ import pytest from better_memory.db.connection import connect from better_memory.db.schema import apply_migrations +from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.services.semantic import SemanticMemoryService from tests.services._embedding_fakes import FakeEmbedder @@ -939,37 +1102,58 @@ def _vec_count(conn): class TestSemanticEmbeddingWrite: def test_create_embeds_content(self, conn): fake = FakeEmbedder() - svc = SemanticMemoryService(conn, embedder=fake) + svc = SemanticMemoryService(conn, sync_embedder=SyncEmbedder(lambda: fake)) svc.create(content="the fact", project="p") assert _vec_count(conn) == 1 assert fake.calls == ["the fact"] def test_update_text_reembeds(self, conn): fake = FakeEmbedder() - svc = SemanticMemoryService(conn, embedder=fake) + svc = SemanticMemoryService(conn, sync_embedder=SyncEmbedder(lambda: fake)) mid = svc.create(content="v1", project="p") svc.update_text(id=mid, content="v2") assert _vec_count(conn) == 1 # replaced, not duplicated assert fake.calls == ["v1", "v2"] def test_failure_never_blocks_create(self, conn): - svc = SemanticMemoryService(conn, embedder=FakeEmbedder(fail=True)) + svc = SemanticMemoryService( + conn, sync_embedder=SyncEmbedder(lambda: FakeEmbedder(fail=True))) mid = svc.create(content="the fact", project="p") - assert mid # row created + assert mid assert _vec_count(conn) == 0 def test_no_embedder_no_rows(self, conn): svc = SemanticMemoryService(conn) svc.create(content="the fact", project="p") assert _vec_count(conn) == 0 + + def test_promote_from_observation_embeds(self, conn): + # create_from_observation needs an active observation row — seed the + # minimal one (mirror the seeding used in test_semantic.py's promote + # tests; copy that helper). + ... ``` +Replace the final `...` by copying the observation-seeding lines from the +existing promote test in `tests/services/test_semantic.py`, then assert +`_vec_count(conn) == 1`. + - [ ] **Step 2: Run to verify failure** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_semantic_embedding_write.py -v` -Expected: FAIL — ctor rejects `embedder` kwarg. +Expected: FAIL — ctor rejects `sync_embedder`. + +- [ ] **Step 3: Implement** — ctor kwarg + `import sqlite_vec` + `_store_embedding` (same shape as Task 7, table `semantic_embeddings`, column `memory_id`); at the end of `create`, `update_text`, and `create_from_observation` (before each `commit`): -- [ ] **Step 3: Implement** — mirror Task 6 exactly in `semantic.py` (ctor kwarg, `_embed_text`, `_store_embedding` targeting `semantic_embeddings(memory_id, ...)`, calls at the end of `create`, `update_text`, `create_from_observation` before their commits). Wire instantiation sites. +```python + if self._sync_embedder is not None: + self._store_embedding( + memory_id, self._sync_embedder.embed_text(content)) +``` + +(using each method's local names — in `create_from_observation` the content +is `row["content"]` and the id is the new `memory_id`). Wire both +instantiation sites with the shared `sync_embedder`. - [ ] **Step 4: Run** @@ -980,22 +1164,22 @@ Expected: all pass. ```bash git add -A -git commit -m "feat(embeddings): semantic memories embedded on create/update, best-effort +git commit -m "feat(embeddings): semantic memories embedded on create/update via SyncEmbedder Co-Authored-By: Claude Fable 5 " ``` --- -### Task 8: Three-leg fusion + lazy self-heal in retrieve +### Task 9: Three-leg fusion + lazy self-heal **Files:** -- Modify: `better_memory/services/reflection.py` (`_fuse_by_relevance` ~1189; `retrieve_reflections` query path) +- Modify: `better_memory/services/reflection.py` (`_fuse_by_relevance`; `retrieve_reflections` query branch) - Test: `tests/services/test_vec_fusion.py` **Interfaces:** -- Consumes: `_embed_text` (Task 6), `reflection_embeddings` rows (Task 6), `sanitize_fts5_query` (existing). -- Produces: `_fuse_by_relevance(rows, *, query, rrf_k=60)` unchanged signature, now internally three-leg; self-heal constant `SELF_HEAL_BATCH_CAP = 20`. +- Consumes: `self._sync_embedder` (Task 7), `_embedding_source_text`, `_store_embedding`, `sqlite_vec.serialize_float32`. +- Produces: `_fuse_by_relevance(rows, *, query, query_vector=None, rrf_k=60)`; `SELF_HEAL_BATCH_CAP = 20`; `_heal_missing_embeddings(rows)`; `_vec_ranks(query_vector, candidate_ids) -> dict[str, int]`. - [ ] **Step 1: Write the failing tests** @@ -1003,9 +1187,8 @@ Co-Authored-By: Claude Fable 5 " # tests/services/test_vec_fusion.py """Query fusion gains a vector leg; missing embeddings self-heal on retrieve. -Degradation contract: no embedder / Ollama down / row unembedded -> exactly -the two-leg (prior + BM25) behaviour that shipped in #81. The vec leg only -ever promotes. +Degradation contract: no embedder / breaker open / row unembedded -> exactly +the two-leg (prior + BM25) behaviour that shipped in #81. """ from __future__ import annotations @@ -1015,6 +1198,7 @@ import pytest from better_memory.db.connection import connect from better_memory.db.schema import apply_migrations +from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.services.reflection import ReflectionSynthesisService from tests.services._embedding_fakes import FakeEmbedder @@ -1042,39 +1226,42 @@ def _seed(conn, rid, *, title, useful=0, ignored=0): class DirectedEmbedder(FakeEmbedder): - """Embeds query and one target title to the same vector; noise elsewhere.""" + """Maps texts containing any trigger phrase to one vector; noise else. + + Lets a test make the query and one reflection 'semantically identical' + while sharing zero tokens — isolating the vec leg from BM25. + """ - def __init__(self, match_text: str): + def __init__(self, *triggers: str): super().__init__() - self.match_text = match_text + self.triggers = triggers - async def embed(self, text: str) -> list[float]: - self.calls.append(text) - if self.match_text in text: + def _vec(self, text: str) -> list[float]: + if any(t in text for t in self.triggers): return [1.0] + [0.0] * 767 return [0.0, 1.0] + [0.0] * 766 + async def embed(self, text): + self.calls.append(text) + return self._vec(text) + + async def embed_batch(self, texts): + self.calls.append(list(texts)) + return [self._vec(t) for t in texts] + + +def _svc(conn, embedder): + return ReflectionSynthesisService( + conn, sync_embedder=SyncEmbedder(lambda: embedder)) + class TestVecFusion: - def test_semantically_close_row_promoted_without_token_overlap(self, conn): - # Query wording shares no tokens with the target title: BM25 misses, - # the vec leg must carry it. + def test_semantic_match_promoted_without_token_overlap(self, conn): _seed(conn, "r-target", title="Stdout handling on win32 interpreters") _seed(conn, "r-noise-1", title="Unrelated advice alpha", useful=5, ignored=1) _seed(conn, "r-noise-2", title="Unrelated advice beta", useful=4, ignored=1) - emb = DirectedEmbedder("Stdout handling") - # pretend the query is 'semantically identical' to the target: - emb.match_text_query = True - - class QueryDirectedEmbedder(DirectedEmbedder): - async def embed(self, text): - self.calls.append(text) - if "console output" in text or self.match_text in text: - return [1.0] + [0.0] * 767 - return [0.0, 1.0] + [0.0] * 766 - - emb = QueryDirectedEmbedder("Stdout handling") - svc = ReflectionSynthesisService(conn, embedder=emb) + emb = DirectedEmbedder("Stdout handling", "console output") + svc = _svc(conn, emb) ids = [r["id"] for r in svc.retrieve_reflections( project="p", query="console output disappears on windows", )["do"]] @@ -1085,11 +1272,11 @@ class TestVecFusion: svc = ReflectionSynthesisService(conn) ids = [r["id"] for r in svc.retrieve_reflections( project="p", query="retention")["do"]] - assert ids == ["r-a"] # BM25-only path still works + assert ids == ["r-a"] def test_embedder_failure_degrades_silently(self, conn): _seed(conn, "r-a", title="Retention thresholds", useful=1) - svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder(fail=True)) + svc = _svc(conn, FakeEmbedder(fail=True)) ids = [r["id"] for r in svc.retrieve_reflections( project="p", query="retention")["do"]] assert ids == ["r-a"] @@ -1099,7 +1286,7 @@ class TestSelfHeal: def test_unembedded_candidates_healed_on_query_retrieve(self, conn): _seed(conn, "r-a", title="Alpha") _seed(conn, "r-b", title="Beta") - svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder()) + svc = _svc(conn, FakeEmbedder()) svc.retrieve_reflections(project="p", query="anything at all") n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] assert n == 2 @@ -1108,27 +1295,27 @@ class TestSelfHeal: from better_memory.services.reflection import SELF_HEAL_BATCH_CAP for i in range(SELF_HEAL_BATCH_CAP + 5): _seed(conn, f"r-{i:03}", title=f"Title {i}") - svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder()) + svc = _svc(conn, FakeEmbedder()) svc.retrieve_reflections(project="p", query="anything") n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] assert n == SELF_HEAL_BATCH_CAP - def test_no_query_no_heal(self, conn): + def test_no_query_no_heal_and_no_embed_calls(self, conn): _seed(conn, "r-a", title="Alpha") - svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder()) + fake = FakeEmbedder() + svc = _svc(conn, fake) svc.retrieve_reflections(project="p") n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] assert n == 0 + assert fake.calls == [] def test_heal_failure_silent(self, conn): _seed(conn, "r-a", title="Alpha") - svc = ReflectionSynthesisService(conn, embedder=FakeEmbedder(fail=True)) + svc = _svc(conn, FakeEmbedder(fail=True)) rows = svc.retrieve_reflections(project="p", query="anything") - assert rows["do"] # retrieval unaffected + assert rows["do"] ``` -Clean the first test before running: keep only the `QueryDirectedEmbedder` version (the earlier `emb` assignments are editing artifacts — delete them). - - [ ] **Step 2: Run to verify failure** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_vec_fusion.py -v` @@ -1136,43 +1323,44 @@ Expected: `SELF_HEAL_BATCH_CAP` import error; promotion test fails. - [ ] **Step 3: Implement** -In `reflection.py`: +Constant: ```python #: Max embeddings written per retrieve call by the lazy self-heal. Keeps -#: worst-case added latency bounded (~cap x embed time on first call after -#: a backlog); the CLI backfill is the bulk path. +#: worst-case added latency bounded; cli.backfill_embeddings is the bulk path. SELF_HEAL_BATCH_CAP = 20 ``` -In `retrieve_reflections`, in the `if query:` branch, before fusion: +Query branch in `retrieve_reflections`: ```python if query: self._heal_missing_embeddings(rows) - query_vector = self._embed_text(query) + query_vector = ( + self._sync_embedder.embed_text(query) + if self._sync_embedder is not None else None + ) rows = self._fuse_by_relevance( rows, query=query, query_vector=query_vector, ) _diag.step(fn, "relevance_fused", n_rows=len(rows)) ``` -New methods: +Methods: ```python def _heal_missing_embeddings(self, rows) -> None: """Embed up to SELF_HEAL_BATCH_CAP candidates that lack vectors. - Historical rows (embedding written before the write-path existed) - and write-time failures repair themselves on their first relevant - retrieval. Entirely best-effort. + Historical rows and write-time failures repair themselves on their + first relevant retrieval. Entirely best-effort; one embed_batch call. """ - if self._embedder is None or not rows: + if self._sync_embedder is None or not rows: return ids = [r["id"] for r in rows] placeholders = ",".join("?" for _ in ids) have = { - r[0] for r in self._conn.execute( + row[0] for row in self._conn.execute( f"SELECT reflection_id FROM reflection_embeddings " f"WHERE reflection_id IN ({placeholders})", ids, ) @@ -1181,49 +1369,47 @@ New methods: if not todo: return texts = [ - _embedding_source_text( - r["title"], r["use_cases"], json.loads(r["hints"]), - ) + _embedding_source_text(r["title"], r["use_cases"], + json.loads(r["hints"])) for r in todo ] - try: - vectors = run_async_in_worker(self._embedder.embed_batch(texts)) - except Exception: + vectors = self._sync_embedder.embed_batch(texts) + if vectors is None: return for r, vec in zip(todo, vectors): self._store_embedding(r["id"], vec) self._conn.commit() def _vec_ranks(self, query_vector, candidate_ids) -> dict[str, int]: - """reflection_id -> vec rank (0 = closest) among the candidates.""" + """reflection_id -> vec rank (0 = closest) among the candidates. + + sqlite-vec kNN accepts only ``embedding MATCH ? AND k = ?`` — no + extra predicates — so fetch top-k then filter, exactly as + search/hybrid.py:_vec_candidates does. + """ if query_vector is None or not candidate_ids: return {} try: - rows = self._conn.execute( + knn = self._conn.execute( "SELECT reflection_id FROM reflection_embeddings " "WHERE embedding MATCH ? AND k = ? ORDER BY distance", - (_serialize_vector(query_vector), max(len(candidate_ids), 50)), + (sqlite_vec.serialize_float32(query_vector), + max(len(candidate_ids), 50)), ).fetchall() except sqlite3.OperationalError: return {} wanted = set(candidate_ids) out: dict[str, int] = {} - for r in rows: - if r[0] in wanted: - out[r[0]] = len(out) + for row in knn: + if row[0] in wanted: + out[row[0]] = len(out) return out ``` -(sqlite-vec kNN cannot take extra predicates — fetch top-k then filter to -candidates, same workaround documented in `search/hybrid.py:262-299`.) - -`_fuse_by_relevance` gains the vec leg — signature `(self, rows, *, query, query_vector=None, rrf_k=60)`; after building `rel_rank` (BM25), add: - -```python - vec_rank = self._vec_ranks(query_vector, ids) -``` - -and in the scoring loop: +`_fuse_by_relevance`: signature gains `query_vector=None`; after the BM25 +`rel_rank` dict is built add `vec_rank = self._vec_ranks(query_vector, ids)`; +bail out unchanged only when BOTH `rel_rows` empty AND `vec_rank` empty; in +the scoring loop add: ```python vr = vec_rank.get(row["id"]) @@ -1231,8 +1417,6 @@ and in the scoring loop: score += 1.0 / (rrf_k + vr) ``` -Bail-out condition changes: currently returns rows unchanged when BM25 finds nothing; now return unchanged only when BOTH `rel_rows` and `vec_rank` are empty. - - [ ] **Step 4: Run** Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_vec_fusion.py tests/services/test_reflection_query_relevance.py tests/services/test_wilson_ranking.py tests/services/test_exploration_slot.py -q` @@ -1242,24 +1426,24 @@ Expected: all pass. ```bash git add -A -git commit -m "feat(retrieval): three-leg RRF fusion (prior + BM25 + vec) with lazy embedding self-heal +git commit -m "feat(retrieval): three-leg RRF fusion (prior + BM25 + vec) with lazy self-heal Co-Authored-By: Claude Fable 5 " ``` --- -### Task 9: Backfill CLI +### Task 10: Backfill CLI **Files:** - Create: `better_memory/cli/backfill_embeddings.py` - Test: `tests/cli/test_backfill_embeddings.py` **Interfaces:** -- Consumes: `_embedding_source_text` (Task 6), services' `_store_embedding` shape (duplicated locally — the CLI writes directly, it is a maintenance tool). -- Produces: `python -m better_memory.cli.backfill_embeddings [--home PATH]` — embeds every active reflection and semantic memory missing a vector; prints `backfilled reflections=N semantics=M skipped=K`; exit 0 even when Ollama is down (prints a warning, backfills nothing). +- Consumes: `_embedding_source_text` (Task 7), `sqlite_vec.serialize_float32`, any embedder with async `embed_batch` + optional `aclose`. +- Produces: `backfill(conn, embedder) -> dict` (single event loop, one embedder, `aclose` by `main`); `python -m better_memory.cli.backfill_embeddings [--home PATH]`. -- [ ] **Step 1: Write the failing test** +- [ ] **Step 1: Write the failing tests** ```python # tests/cli/test_backfill_embeddings.py @@ -1269,9 +1453,9 @@ from pathlib import Path import pytest +from better_memory.cli.backfill_embeddings import backfill from better_memory.db.connection import connect from better_memory.db.schema import apply_migrations -from better_memory.cli.backfill_embeddings import backfill from tests.services._embedding_fakes import FakeEmbedder @@ -1285,18 +1469,27 @@ def conn(tmp_memory_db: Path): c.close() -def test_backfills_reflections_and_semantics(conn): +def _seed_reflection(conn, rid, status="pending_review"): conn.execute( """INSERT INTO reflections (id, title, project, phase, polarity, - use_cases, hints, confidence, created_at, updated_at) - VALUES ('r1', 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, - '2026-01-01', '2026-01-01')""") + use_cases, hints, confidence, created_at, updated_at, status) + VALUES (?, 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01', ?)""", (rid, status)) + conn.commit() + + +def _seed_semantic(conn, sid): conn.execute( """INSERT INTO semantic_memories (id, content, project, scope, created_at, updated_at) - VALUES ('s1', 'fact', 'p', 'project', '2026-01-01', '2026-01-01')""") + VALUES (?, 'fact', 'p', 'project', '2026-01-01', '2026-01-01')""", + (sid,)) conn.commit() + +def test_backfills_reflections_and_semantics(conn): + _seed_reflection(conn, "r1") + _seed_semantic(conn, "s1") stats = backfill(conn, FakeEmbedder()) assert stats == {"reflections": 1, "semantics": 1, "skipped": 0} assert conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] == 1 @@ -1304,36 +1497,20 @@ def test_backfills_reflections_and_semantics(conn): def test_idempotent(conn): - conn.execute( - """INSERT INTO reflections (id, title, project, phase, polarity, - use_cases, hints, confidence, created_at, updated_at) - VALUES ('r1', 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, - '2026-01-01', '2026-01-01')""") - conn.commit() + _seed_reflection(conn, "r1") fake = FakeEmbedder() backfill(conn, fake) - stats = backfill(conn, fake) - assert stats == {"reflections": 0, "semantics": 0, "skipped": 0} + assert backfill(conn, fake) == {"reflections": 0, "semantics": 0, "skipped": 0} def test_retired_reflections_skipped(conn): - conn.execute( - """INSERT INTO reflections (id, title, project, phase, polarity, - use_cases, hints, confidence, created_at, updated_at, status) - VALUES ('r1', 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, - '2026-01-01', '2026-01-01', 'retired')""") - conn.commit() - stats = backfill(conn, FakeEmbedder()) - assert stats == {"reflections": 0, "semantics": 0, "skipped": 0} + _seed_reflection(conn, "r1", status="retired") + assert backfill(conn, FakeEmbedder()) == { + "reflections": 0, "semantics": 0, "skipped": 0} def test_embed_failure_counted_as_skipped(conn): - conn.execute( - """INSERT INTO reflections (id, title, project, phase, polarity, - use_cases, hints, confidence, created_at, updated_at) - VALUES ('r1', 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, - '2026-01-01', '2026-01-01')""") - conn.commit() + _seed_reflection(conn, "r1") stats = backfill(conn, FakeEmbedder(fail=True)) assert stats == {"reflections": 0, "semantics": 0, "skipped": 1} ``` @@ -1356,6 +1533,9 @@ Run at deploy after migration 0014: Idempotent: only rows missing a vector are embedded. The lazy self-heal in memory.retrieve covers stragglers afterwards; this exists so the historical corpus doesn't wait to be retrieved before becoming searchable. + +One event loop and one embedder for the whole job (the embedder's +httpx.AsyncClient is loop-bound); batches of 50 per HTTP request. """ from __future__ import annotations @@ -1366,12 +1546,13 @@ import json import sys from pathlib import Path +import sqlite_vec + from better_memory.db.connection import connect from better_memory.db.schema import apply_migrations -from better_memory.services.reflection import ( - _embedding_source_text, - _serialize_vector, -) +from better_memory.services.reflection import _embedding_source_text + +_BATCH = 50 def backfill(conn, embedder) -> dict[str, int]: @@ -1382,34 +1563,43 @@ def backfill(conn, embedder) -> dict[str, int]: WHERE r.status IN ('pending_review', 'confirmed') AND r.id NOT IN (SELECT reflection_id FROM reflection_embeddings)""" ).fetchall() - for r in refl: - text = _embedding_source_text(r["title"], r["use_cases"], - json.loads(r["hints"])) - try: - vec = asyncio.run(embedder.embed(text)) - except Exception: - stats["skipped"] += 1 - continue - conn.execute( - "INSERT INTO reflection_embeddings (reflection_id, embedding) " - "VALUES (?, ?)", (r["id"], _serialize_vector(vec))) - stats["reflections"] += 1 - sems = conn.execute( """SELECT id, content FROM semantic_memories WHERE id NOT IN (SELECT memory_id FROM semantic_embeddings)""" ).fetchall() - for s in sems: - try: - vec = asyncio.run(embedder.embed(s["content"])) - except Exception: - stats["skipped"] += 1 - continue - conn.execute( - "INSERT INTO semantic_embeddings (memory_id, embedding) " - "VALUES (?, ?)", (s["id"], _serialize_vector(vec))) - stats["semantics"] += 1 + jobs = [ + ("reflections", "reflection_embeddings", "reflection_id", r["id"], + _embedding_source_text(r["title"], r["use_cases"], + json.loads(r["hints"]))) + for r in refl + ] + [ + ("semantics", "semantic_embeddings", "memory_id", s["id"], s["content"]) + for s in sems + ] + + async def _embed_all() -> list[list[list[float]] | None]: + out = [] + for i in range(0, len(jobs), _BATCH): + texts = [j[4] for j in jobs[i:i + _BATCH]] + try: + out.append(await embedder.embed_batch(texts)) + except Exception: + out.append(None) + return out + + batches = asyncio.run(_embed_all()) if jobs else [] + + for bi, vectors in enumerate(batches): + chunk = jobs[bi * _BATCH:(bi + 1) * _BATCH] + if vectors is None: + stats["skipped"] += len(chunk) + continue + for (kind, table, col, row_id, _), vec in zip(chunk, vectors): + conn.execute( + f"INSERT INTO {table} ({col}, embedding) VALUES (?, ?)", + (row_id, sqlite_vec.serialize_float32(vec))) + stats[kind] += 1 conn.commit() return stats @@ -1432,7 +1622,11 @@ def main(argv: list[str] | None = None) -> None: print("embeddings backend is not ollama; nothing to backfill") return - stats = backfill(conn, OllamaEmbedder()) + embedder = OllamaEmbedder() + try: + stats = backfill(conn, embedder) + finally: + asyncio.run(embedder.aclose()) print(f"backfilled reflections={stats['reflections']} " f"semantics={stats['semantics']} skipped={stats['skipped']}") if stats["skipped"]: @@ -1444,9 +1638,11 @@ if __name__ == "__main__": main() ``` -Note: `asyncio.run` per item is fine here — standalone process, no running -loop, and the N≈150 corpus embeds in seconds. FakeEmbedder's `embed` is a -plain coroutine, so `asyncio.run` works in tests too. +Note on `aclose` after `asyncio.run(backfill...)`: the embedder's client was +used on the loop inside `backfill`'s `asyncio.run`, which is closed by then; +`aclose` on a second loop is the documented-acceptable teardown for httpx +(close only releases resources). If it raises, wrap in try/except — teardown +is best-effort. - [ ] **Step 4: Run** @@ -1464,32 +1660,16 @@ Co-Authored-By: Claude Fable 5 " --- -### Task 10: Website sync, typecheck, full suite +### Task 11: Website sync, typecheck, full suite **Files:** -- Modify: `website/index.md` (tools showcase — memory.retrieve description: task-conditioned, Wilson-ranked, semantic fusion) -- Modify: `website/architecture.md` (ranking prose: replace popularity/demotion description with Wilson + exploration + three-leg fusion; add embeddings lifecycle para) -- Modify: `website/configuration.md` — only if it documents ranking constants (grep `OVERLOOKED` / `DEMOTION`; remove if present; no new env vars were added) - -**Interfaces:** none — prose. - -- [ ] **Step 1: Grep for stale prose** - -Run: `grep -rn "useful_count\|OVERLOOKED\|DEMOTION\|popularity" website/ | head -20` - -- [ ] **Step 2: Update each hit** to describe: Wilson lower bound on (useful+overlooked)/rated; reserved exploration slot; embeddings written at synthesis + self-heal + backfill CLI; three-leg RRF. Keep each edit to the paragraph that is actually wrong — no rewrites beyond the stale claims. - -- [ ] **Step 3: Typecheck** - -Run: `./.venv/Scripts/python.exe -m pyright` -Expected: 0 errors - -- [ ] **Step 4: Full suite** +- Modify: `website/index.md`, `website/architecture.md`, `website/configuration.md` (only where stale) -Run: `./.venv/Scripts/python.exe -m pytest tests -q` -Expected: everything passes (~1500 tests). Fix any straggler that pinned old ordering before proceeding. - -- [ ] **Step 5: Commit** +- [ ] **Step 1:** `grep -rn "useful_count\|OVERLOOKED\|DEMOTION\|popularity" website/ | head -20` +- [ ] **Step 2:** Update each stale paragraph: Wilson lower bound on (useful+overlooked)/rated; exploration slot; embeddings at synthesis + self-heal + backfill CLI; three-leg RRF; 60s breaker. Edit only what is wrong. +- [ ] **Step 3:** `./.venv/Scripts/python.exe -m pyright` → 0 errors. +- [ ] **Step 4:** `./.venv/Scripts/python.exe -m pytest tests -q` → all pass; fix stragglers pinning old ordering. +- [ ] **Step 5:** ```bash git add website @@ -1500,15 +1680,14 @@ Co-Authored-By: Claude Fable 5 " --- -### Task 11: A/B validation gate, PR, babysit +### Task 12: A/B validation gate (48 sessions), PR, babysit **Files:** -- Modify: `C:/Users/gethi/source/autoresearch/memuse-260721-run/runner.py` (add arm `A8`: `spec["code"] = str(WT)` equivalent pointing at this branch — copy the `A6` arm, change the code path to the main repo checkout on `feat/retrieval-quality`) -- No repo files. +- Modify: `C:/Users/gethi/source/autoresearch/memuse-260721-run/runner.py` (add arm `A8`) -**Interfaces:** gate = distinct useful% from 24 sessions not statistically below 22.96% (two-proportion z-test, α=0.05, one re-run allowed on borderline). +**Interfaces:** gate = distinct useful% from 48 sessions not statistically below 62/270 (22.96%); one-sided two-proportion z at α=0.05; one re-run on borderline; fail-twice = stop, no PR. -- [ ] **Step 1: Point the harness at this branch.** In `runner.py` `arm_spec`, add: +- [ ] **Step 1:** Add arm: ```python elif arm == "A8": @@ -1516,9 +1695,9 @@ Co-Authored-By: Claude Fable 5 " spec["code"] = str(MAIN_REPO) ``` -Confirm `git -C C:/Users/gethi/source/better-memory branch --show-current` prints `feat/retrieval-quality` before launching — the arm runs whatever that checkout has. +Confirm `git -C C:/Users/gethi/source/better-memory branch --show-current` → `feat/retrieval-quality` before launching. -- [ ] **Step 2: Refresh the sandbox base DB** (new schema needed): +- [ ] **Step 2:** Refresh sandbox base DB (new schema): ```bash cd C:/Users/gethi/source/autoresearch/memuse-260721-run @@ -1531,23 +1710,15 @@ sqlite3.connect(f"file:{src}?mode=ro", uri=True).execute("VACUUM INTO ?", (dst,) EOF ``` -- [ ] **Step 3: Run the arm** - -```bash -python runner.py --arm A8 --repeats 2 --timeout 420 > logs-A8.txt 2>&1 -``` - -(24 sessions, ~2h, ~$40. Run in background; strip-and-retry any 429 rows as done previously.) +- [ ] **Step 3:** `python runner.py --arm A8 --repeats 4 --timeout 420 > logs-A8.txt 2>&1` (48 sessions, ~4h, ~$80; background; strip-and-retry 429 rows). -- [ ] **Step 4: Evaluate the gate** +- [ ] **Step 4:** Gate: ```bash python analyze.py --arms A6,A8 python - <<'EOF' -# two-proportion z-test: A8 distinct vs 22.96% baseline (62/270) import math -# fill from analyze output: -u8, n8 = USEFUL_A8, EXPOSED_A8 +u8, n8 = USEFUL_A8, EXPOSED_A8 # fill from analyze distinct columns p1, n1 = 62/270, 270 p2 = u8/n8 p = (62 + u8) / (n1 + n8) @@ -1557,32 +1728,13 @@ print("GATE:", "PASS" if z > -1.645 else "FAIL (one re-run allowed)") EOF ``` -Gate: PASS ⇒ proceed. FAIL twice ⇒ stop, report, do not open the PR. - -- [ ] **Step 5: PR + babysit** - -```bash -cd C:/Users/gethi/source/better-memory -git push -u origin feat/retrieval-quality -gh pr create --title "feat(retrieval): Wilson prior, exploration slot, embeddings + vec fusion" --body " - -🤖 Generated with [Claude Code](https://claude.com/claude-code)" -``` - -Babysit loop (as PR #81/#82): poll `statusCheckRollup` + unresolved `reviewThreads`; fix findings; when CLEAN + zero threads → `gh pr merge --squash --delete-branch`; then run migration + backfill on live: - -```bash -git checkout main && git pull origin main -./.venv/Scripts/python.exe -m better_memory.cli.backfill_embeddings -``` - -(migration 0014 applies on the MCP server's next start; backfill after it — if the table is missing, `apply_migrations` in the CLI's `main` creates it first, so this ordering is safe.) +- [ ] **Step 5:** PASS → push, `gh pr create` (body: spec link, A/B numbers, migration 0014 + backfill deploy note, footer `🤖 Generated with [Claude Code](https://claude.com/claude-code)`), babysit to squash-merge, then on main: `./.venv/Scripts/python.exe -m better_memory.cli.backfill_embeddings`. --- ## Self-review notes -- Spec §1-§3 covered by Tasks 1-9; §"Delivery A" gate by Task 11; website guardrail by Task 10. Spec §4 (evidence) and §5 (docs/sentinel) are PR-B / PR-C — separate plans, deliberately absent here. -- Two test snippets contain flagged editing artifacts with explicit cleanup instructions (Task 2 placeholder class, Task 8 first test) — the implementer deletes the marked lines; both are called out inline. -- Task 6's apply-path tests reference `tests/services/test_reflection_writes.py` for the action-construction boilerplate rather than inventing dataclass fields that may drift — the implementer copies the file's existing working setup. -- `_serialize_vector` is defined by copying the exact serialisation used at `observation.py:204-210`; Tasks 8, 9 import it from `reflection.py`. +- Every previously-assumed integration point is now pinned in the "Verified-against-source facts" table; Tasks 6-10 contain only calls whose signatures were read from source this session. +- The two prior plan bugs (coroutine-vs-factory; loop-bound AsyncClient) are structurally prevented by `SyncEmbedder` — no other code touches the bridge. +- Merge-path embedding deliberately does NOT re-embed the target (text unchanged, verified) — a reviewer questioning that finds the rationale in Task 7's test and comment. +- Remaining sub-95 item: Task 12 (92%) — irreducible sampling noise, mitigated by n=48 and the statistical gate; accepted by user 2026-07-23. From 741121a4c216d8fd52291ec7469e6abf5c6cb1d3 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 08:32:08 +0100 Subject: [PATCH 04/20] feat(scoring): Wilson lower bound ranking prior Co-Authored-By: Claude Fable 5 --- better_memory/services/scoring.py | 35 +++++++++++++++++++++++++++ tests/services/test_scoring.py | 39 +++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 better_memory/services/scoring.py create mode 100644 tests/services/test_scoring.py diff --git a/better_memory/services/scoring.py b/better_memory/services/scoring.py new file mode 100644 index 0000000..9c890a7 --- /dev/null +++ b/better_memory/services/scoring.py @@ -0,0 +1,35 @@ +"""Ranking prior for reflections and semantic memories. + +One function, one job: the Wilson score lower bound on the proportion of +rated exposures where the memory was positive (useful or overlooked). +Replaces the raw-count ORDER BY stack (useful_count + overlooked weight + +ignored-demotion CASE) — see the 2026-07-23 retrieval-quality spec §1. + +Computed in Python, not SQL: SQLite's sqrt() requires the math extension, +the candidate sets are tiny (~150 rows), and a pure function is testable +against closed-form values. +""" + +from __future__ import annotations + +import math + +#: 95% confidence. Pinned — changing z reorders every list; treat as part +#: of the scoring contract, not a tunable. +WILSON_Z = 1.96 + + +def wilson_lower_bound(positive: int, n: int, z: float = WILSON_Z) -> float: + """Lower bound of the Wilson score interval for positive/n. + + ``n == 0`` (never rated) returns 0.0 — untested memories score at the + bottom and are surfaced by the exploration slot instead (spec §2). + """ + if n <= 0: + return 0.0 + p = positive / n + z2 = z * z + denom = 1.0 + z2 / n + centre = p + z2 / (2.0 * n) + margin = z * math.sqrt(p * (1.0 - p) / n + z2 / (4.0 * n * n)) + return max(0.0, (centre - margin) / denom) diff --git a/tests/services/test_scoring.py b/tests/services/test_scoring.py new file mode 100644 index 0000000..14d77d0 --- /dev/null +++ b/tests/services/test_scoring.py @@ -0,0 +1,39 @@ +"""Wilson lower bound: the ranking prior for reflections + semantic memories. + +Chosen over raw useful_count because raw counts are rich-get-richer: 67 +useful over 192 rated sessions (35% hit rate) permanently outranked 3/4 +(75%). The lower bound rewards hit rate while discounting small samples. +""" +from __future__ import annotations + +import pytest + +from better_memory.services.scoring import wilson_lower_bound + + +class TestWilsonLowerBound: + def test_no_data_scores_zero(self): + assert wilson_lower_bound(0, 0) == 0.0 + + def test_proven_dead_weight_scores_near_zero(self): + assert wilson_lower_bound(0, 58) == pytest.approx(0.0, abs=1e-9) + + def test_high_hit_rate_newcomer_beats_popular_workhorse(self): + # The design's worked example: 3/4 (75%) must outrank 67/192 (35%). + assert wilson_lower_bound(3, 4) > wilson_lower_bound(67, 192) + + def test_worked_example_values(self): + assert wilson_lower_bound(67, 192) == pytest.approx(0.285, abs=0.005) + assert wilson_lower_bound(3, 4) == pytest.approx(0.301, abs=0.005) + + def test_monotonic_in_positives_at_fixed_n(self): + scores = [wilson_lower_bound(k, 10) for k in range(11)] + assert scores == sorted(scores) + assert scores[0] < scores[10] + + def test_more_evidence_at_same_rate_scores_higher(self): + assert wilson_lower_bound(30, 40) > wilson_lower_bound(3, 4) + + def test_never_negative_and_never_above_one(self): + for positive, n in [(0, 1), (1, 1), (1, 1000), (999, 1000)]: + assert 0.0 <= wilson_lower_bound(positive, n) <= 1.0 From 63e5d7be9cf5dc4e47589d28d9418c6622c36785 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 08:40:25 +0100 Subject: [PATCH 05/20] feat(ranking): reflections ranked by Wilson prior, demotion stack deleted Replaces the popularity + overlooked-weight + ignored-demotion CASE ORDER BY in retrieve_reflections with wilson_lower_bound over (useful+overlooked)/rated, computed in Python and sorted (wilson DESC, confidence DESC, updated_at DESC). Deletes OVERLOOKED_RANKING_WEIGHT, IGNORED_DEMOTION_FLOOR, IGNORED_DEMOTION_WEIGHT from memory_rating.py and their two superseded pinning tests. semantic.py's list_for_project also imported the deleted constants; bridged its ORDER BY to plain popularity (useful_count DESC, created_at DESC) pending Task 3's full Wilson rewrite there. Co-Authored-By: Claude Fable 5 --- better_memory/services/memory_rating.py | 17 --- better_memory/services/reflection.py | 42 ++--- better_memory/services/semantic.py | 18 +-- tests/services/test_ignored_demotion.py | 110 -------------- tests/services/test_useful_count_ranking.py | 160 -------------------- tests/services/test_wilson_ranking.py | 85 +++++++++++ 6 files changed, 111 insertions(+), 321 deletions(-) delete mode 100644 tests/services/test_ignored_demotion.py delete mode 100644 tests/services/test_useful_count_ranking.py create mode 100644 tests/services/test_wilson_ranking.py diff --git a/better_memory/services/memory_rating.py b/better_memory/services/memory_rating.py index 8383e19..6977f18 100644 --- a/better_memory/services/memory_rating.py +++ b/better_memory/services/memory_rating.py @@ -61,23 +61,6 @@ class ApplySessionRatingsResult(TypedDict): _VALID_CLASSES: set[str] = {"cited", "shaped", "ignored", "misled", "overlooked"} _CREDIT_CLASSES: set[str] = {"cited", "shaped", "misled", "overlooked"} -# Retrieval-ranking weight for the `overlooked` class. One `overlooked` -# rating contributes this many points to a memory's rank score — the same -# unit as one `useful_count`. Per issue #60, an overlooked memory should -# rank harder than a cited one. Imported by reflection.py and semantic.py. -OVERLOOKED_RANKING_WEIGHT = 3 - -# Demotion for memories that keep being served and keep not mattering. -# A memory is only demoted once it has been ignored in at least -# IGNORED_DEMOTION_FLOOR distinct sessions AND has never been useful — one or -# two ignores say nothing (the task simply wasn't relevant that day), but a -# double-digit run of them with no hits is the memory telling you it does not -# belong in the default set. Memories with any useful history are never -# demoted, however often they are also ignored: the best-performing memory on -# the live DB is ignored in 125 of the 192 sessions it appears in. -IGNORED_DEMOTION_FLOOR = 10 -IGNORED_DEMOTION_WEIGHT = 1 - class MemoryRatingService: """Writes useful_count / times_misled on reflections + semantic memories, diff --git a/better_memory/services/reflection.py b/better_memory/services/reflection.py index c124e0d..1b414f8 100644 --- a/better_memory/services/reflection.py +++ b/better_memory/services/reflection.py @@ -34,11 +34,7 @@ from better_memory import _diag from better_memory._common import default_clock, env_session_id from better_memory.search.query import sanitize_fts5_query -from better_memory.services.memory_rating import ( - IGNORED_DEMOTION_FLOOR, - IGNORED_DEMOTION_WEIGHT, - OVERLOOKED_RANKING_WEIGHT, -) +from better_memory.services.scoring import wilson_lower_bound # Dropped from relevance queries. These survive `sanitize_fts5_query` and the # >2-char filter, but appear in so many reflections that OR-matching on them @@ -300,6 +296,17 @@ def _parse_merge(item: object) -> MergeAction: ) +def _wilson_rated(row) -> int: + """Total rated exposures backing this memory's score.""" + return (row["useful_count"] + row["times_overlooked"] + + row["times_ignored"]) + + +def _wilson_score(row) -> float: + positive = row["useful_count"] + row["times_overlooked"] + return wilson_lower_bound(positive, _wilson_rated(row)) + + class ReflectionSynthesisService: """Orchestrates pre-start synthesis: load, prompt, parse, apply, return. @@ -1312,32 +1319,27 @@ def retrieve_reflections( params.append(polarity) where = " AND ".join(clauses) - params.append(OVERLOOKED_RANKING_WEIGHT) - params.append(IGNORED_DEMOTION_WEIGHT) - params.append(IGNORED_DEMOTION_FLOOR) _diag.step(fn, "executing_select") - # The demotion term fires only for memories with no useful history - # at all, and only past the floor — see IGNORED_DEMOTION_FLOOR. - # Without it the ranking key has no negative term, so a memory that - # has been served 142 times and helped zero times sorts level with - # one that has never been served. rows = self._conn.execute( f""" SELECT id, title, phase, polarity, use_cases, hints, confidence, tech, evidence_count, useful_count, - times_misled, updated_at + times_misled, times_overlooked, times_ignored, + updated_at FROM reflections WHERE {where} - ORDER BY (useful_count + ? * times_overlooked - - ? * CASE WHEN useful_count = 0 - THEN MAX(0, times_ignored - ?) - ELSE 0 END) DESC, - confidence DESC, updated_at DESC """, params, ).fetchall() _diag.step(fn, "select_done", n_rows=len(rows)) + # Rank in Python: Wilson prior, then confidence, then recency. + # Chained stable sorts apply tiebreakers lowest-priority first. + rows = list(rows) + rows.sort(key=lambda r: r["updated_at"] or "", reverse=True) + rows.sort(key=lambda r: r["confidence"], reverse=True) + rows.sort(key=_wilson_score, reverse=True) + if query: rows = self._fuse_by_relevance(rows, query=query) _diag.step(fn, "relevance_fused", n_rows=len(rows)) @@ -1360,6 +1362,8 @@ def retrieve_reflections( "evidence_count": r["evidence_count"], "useful_count": r["useful_count"], "times_misled": r["times_misled"], + "times_overlooked": r["times_overlooked"], + "times_ignored": r["times_ignored"], "updated_at": r["updated_at"], }) _diag.step( diff --git a/better_memory/services/semantic.py b/better_memory/services/semantic.py index 663aef1..d47555c 100644 --- a/better_memory/services/semantic.py +++ b/better_memory/services/semantic.py @@ -16,11 +16,6 @@ from uuid import uuid4 from better_memory._common import default_clock, env_session_id -from better_memory.services.memory_rating import ( - IGNORED_DEMOTION_FLOOR, - IGNORED_DEMOTION_WEIGHT, - OVERLOOKED_RANKING_WEIGHT, -) @dataclass(frozen=True) @@ -244,17 +239,10 @@ def list_for_project( "times_overlooked, last_overlooked_at " "FROM semantic_memories " f"WHERE {' AND '.join(where_clauses)} " - # Mirrors the reflections ranking, including the ignored-history - # demotion for memories with no useful history past the floor. - "ORDER BY (useful_count + ? * times_overlooked " - " - ? * CASE WHEN useful_count = 0 " - " THEN MAX(0, times_ignored - ?) " - " ELSE 0 END) DESC, " - "created_at DESC" + # Minimal bridge pending Task 3's Wilson-score rewrite of + # semantic.py ranking (mirrors reflection.py's Task 2 change). + "ORDER BY (useful_count) DESC, created_at DESC" ) - params.append(OVERLOOKED_RANKING_WEIGHT) - params.append(IGNORED_DEMOTION_WEIGHT) - params.append(IGNORED_DEMOTION_FLOOR) rows = self._conn.execute(sql, params).fetchall() results = [ SemanticMemory( diff --git a/tests/services/test_ignored_demotion.py b/tests/services/test_ignored_demotion.py deleted file mode 100644 index 90867c3..0000000 --- a/tests/services/test_ignored_demotion.py +++ /dev/null @@ -1,110 +0,0 @@ -"""Memories that keep being served and keep not mattering must sink. - -Before migration 0013 the ranking key had no negative term, so a reflection -served 142 times that was useful zero times sorted level with one that had -never been served at all. On a live DB, 55 such memories accounted for 27.5% -of every rated exposure. -""" -from __future__ import annotations - -from pathlib import Path - -import pytest - -from better_memory.db.connection import connect -from better_memory.db.schema import apply_migrations -from better_memory.services.memory_rating import ( - IGNORED_DEMOTION_FLOOR, - MemoryRatingService, -) -from better_memory.services.reflection import ReflectionSynthesisService - - -@pytest.fixture -def conn(tmp_memory_db: Path): - c = connect(tmp_memory_db) - apply_migrations(c) - try: - yield c - finally: - c.close() - - -def _seed(conn, rid, *, useful_count=0, times_ignored=0, confidence=0.5): - conn.execute( - """INSERT INTO reflections - (id, title, project, phase, polarity, use_cases, hints, - confidence, created_at, updated_at, useful_count, times_ignored) - VALUES (?, ?, 'p', 'general', 'do', 'uc', '[]', ?, - '2026-01-01', '2026-01-01', ?, ?)""", - (rid, rid, confidence, useful_count, times_ignored), - ) - conn.commit() - - -def _ids(conn, **kw): - svc = ReflectionSynthesisService(conn) - return [r["id"] for r in svc.retrieve_reflections(project="p", **kw)["do"]] - - -class TestIgnoredDemotion: - def test_chronically_ignored_sinks_below_never_served(self, conn): - _seed(conn, "r-proven-useless", useful_count=0, times_ignored=60) - _seed(conn, "r-untested", useful_count=0, times_ignored=0) - assert _ids(conn) == ["r-untested", "r-proven-useless"] - - def test_below_floor_is_not_demoted(self, conn): - # A handful of ignores says nothing — the task simply wasn't relevant. - _seed(conn, "r-a", useful_count=0, times_ignored=IGNORED_DEMOTION_FLOOR, - confidence=0.9) - _seed(conn, "r-b", useful_count=0, times_ignored=0, confidence=0.1) - assert _ids(conn) == ["r-a", "r-b"], "at the floor, confidence still decides" - - def test_memory_with_useful_history_is_never_demoted(self, conn): - # The best-performing memory on the live DB is ignored far more often - # than it is used; a hit rate below 50% is normal and fine. - _seed(conn, "r-useful-but-often-ignored", useful_count=18, times_ignored=55) - _seed(conn, "r-untested", useful_count=0, times_ignored=0) - assert _ids(conn)[0] == "r-useful-but-often-ignored" - - def test_apply_session_ratings_bumps_times_ignored(self, conn, monkeypatch): - monkeypatch.setenv("CLAUDE_SESSION_ID", "s1") - _seed(conn, "r-a") - conn.execute( - "INSERT INTO session_memory_exposure " - "(session_id, memory_kind, memory_id, exposed_at, source) " - "VALUES ('s1', 'reflection', 'r-a', '2026-01-01', 'retrieve')" - ) - conn.commit() - - MemoryRatingService(conn).apply_session_ratings( - session_id="s1", - ratings=[{"kind": "reflection", "id": "r-a", "class": "ignored"}], - ) - row = conn.execute( - "SELECT times_ignored, last_ignored_at FROM reflections WHERE id = 'r-a'" - ).fetchone() - assert row["times_ignored"] == 1 - assert row["last_ignored_at"] is not None - - def test_repeat_exposures_in_one_session_count_once(self, conn, monkeypatch): - # A memory retrieved five times in one session that lands nowhere - # failed once, not five times. - monkeypatch.setenv("CLAUDE_SESSION_ID", "s1") - _seed(conn, "r-a") - for i in range(5): - conn.execute( - "INSERT INTO session_memory_exposure " - "(session_id, memory_kind, memory_id, exposed_at, source) " - "VALUES ('s1', 'reflection', 'r-a', ?, 'retrieve')", - (f"2026-01-01T00:0{i}:00",), - ) - conn.commit() - - MemoryRatingService(conn).apply_session_ratings( - session_id="s1", - ratings=[{"kind": "reflection", "id": "r-a", "class": "ignored"}], - ) - assert conn.execute( - "SELECT times_ignored FROM reflections WHERE id = 'r-a'" - ).fetchone()["times_ignored"] == 1 diff --git a/tests/services/test_useful_count_ranking.py b/tests/services/test_useful_count_ranking.py deleted file mode 100644 index 1118f14..0000000 --- a/tests/services/test_useful_count_ranking.py +++ /dev/null @@ -1,160 +0,0 @@ -"""Verify useful_count is the primary sort key in retrieval.""" -from __future__ import annotations - -from pathlib import Path - -import pytest - -from better_memory.db.connection import connect -from better_memory.db.schema import apply_migrations - - -@pytest.fixture -def conn(tmp_memory_db: Path): - c = connect(tmp_memory_db) - apply_migrations(c) - try: - yield c - finally: - c.close() - - -def _seed_reflection(conn, rid, *, useful_count=0, confidence=0.5, - polarity="do", updated_at="2026-01-01", - times_overlooked=0): - conn.execute( - """INSERT INTO reflections - (id, title, project, phase, polarity, use_cases, hints, - confidence, created_at, updated_at, useful_count, - times_overlooked) - VALUES (?, ?, 'p', 'general', ?, 'uc', '[]', ?, - '2026-01-01', ?, ?, ?)""", - (rid, rid, polarity, confidence, updated_at, useful_count, - times_overlooked), - ) - conn.commit() - - -class TestReflectionRanking: - def test_useful_count_beats_confidence(self, conn): - from better_memory.services.reflection import ReflectionSynthesisService - _seed_reflection(conn, "r-low-confidence-but-useful", - useful_count=5, confidence=0.3) - _seed_reflection(conn, "r-high-confidence-unused", - useful_count=0, confidence=0.9) - svc = ReflectionSynthesisService(conn) - result = svc.retrieve_reflections(project="p") - ids = [r["id"] for r in result["do"]] - assert ids[0] == "r-low-confidence-but-useful" - assert ids[1] == "r-high-confidence-unused" - - def test_confidence_tiebreaks_when_useful_count_equal(self, conn): - from better_memory.services.reflection import ReflectionSynthesisService - _seed_reflection(conn, "r-mid-confidence", useful_count=0, confidence=0.5) - _seed_reflection(conn, "r-high-confidence", useful_count=0, confidence=0.9) - svc = ReflectionSynthesisService(conn) - result = svc.retrieve_reflections(project="p") - ids = [r["id"] for r in result["do"]] - assert ids[0] == "r-high-confidence" - - def test_updated_at_tiebreaks_when_both_equal(self, conn): - from better_memory.services.reflection import ReflectionSynthesisService - _seed_reflection(conn, "r-older", - useful_count=2, confidence=0.5, updated_at="2026-01-01") - _seed_reflection(conn, "r-newer", - useful_count=2, confidence=0.5, updated_at="2026-05-01") - svc = ReflectionSynthesisService(conn) - result = svc.retrieve_reflections(project="p") - ids = [r["id"] for r in result["do"]] - assert ids[0] == "r-newer" - - -def _seed_semantic(conn, sid, *, useful_count=0, created_at="2026-01-01", - times_overlooked=0): - conn.execute( - """INSERT INTO semantic_memories - (id, content, project, scope, created_at, updated_at, - useful_count, times_overlooked) - VALUES (?, 'fact', 'p', 'project', ?, ?, ?, ?)""", - (sid, created_at, created_at, useful_count, times_overlooked), - ) - conn.commit() - - -class TestSemanticRanking: - def test_useful_count_beats_created_at(self, conn): - """High useful_count surfaces first even when older.""" - from better_memory.services.semantic import SemanticMemoryService - _seed_semantic(conn, "s-older-but-useful", - useful_count=5, created_at="2026-01-01") - _seed_semantic(conn, "s-newer-unused", - useful_count=0, created_at="2026-05-01") - svc = SemanticMemoryService(conn) - results = svc.list_for_project(project="p", track_exposure=False) - ids = [m.id for m in results] - assert ids[0] == "s-older-but-useful" - assert ids[1] == "s-newer-unused" - - def test_created_at_tiebreaks_when_useful_count_equal(self, conn): - from better_memory.services.semantic import SemanticMemoryService - _seed_semantic(conn, "s-older", useful_count=0, created_at="2026-01-01") - _seed_semantic(conn, "s-newer", useful_count=0, created_at="2026-05-01") - svc = SemanticMemoryService(conn) - results = svc.list_for_project(project="p", track_exposure=False) - ids = [m.id for m in results] - assert ids[0] == "s-newer" - - -class TestOverlookedRanking: - def test_overlooked_outranks_lower_useful_count(self, conn): - """One overlooked (weight 3) beats useful_count=2 (score 2 < 3).""" - from better_memory.services.reflection import ReflectionSynthesisService - _seed_reflection(conn, "r-useful-2", useful_count=2, - times_overlooked=0) - _seed_reflection(conn, "r-overlooked-1", useful_count=0, - times_overlooked=1) - svc = ReflectionSynthesisService(conn) - result = svc.retrieve_reflections(project="p") - ids = [r["id"] for r in result["do"]] - assert ids.index("r-overlooked-1") < ids.index("r-useful-2") - - def test_high_useful_count_still_beats_one_overlooked(self, conn): - """useful_count=4 (score 4) beats one overlooked (score 3).""" - from better_memory.services.reflection import ReflectionSynthesisService - _seed_reflection(conn, "r-useful-4", useful_count=4, - times_overlooked=0) - _seed_reflection(conn, "r-overlooked-1", useful_count=0, - times_overlooked=1) - svc = ReflectionSynthesisService(conn) - result = svc.retrieve_reflections(project="p") - ids = [r["id"] for r in result["do"]] - assert ids.index("r-useful-4") < ids.index("r-overlooked-1") - - def test_semantic_overlooked_outranks_lower_useful_count(self, conn): - from better_memory.services.semantic import SemanticMemoryService - _seed_semantic(conn, "s-useful-2", useful_count=2, - times_overlooked=0) - _seed_semantic(conn, "s-overlooked-1", useful_count=0, - times_overlooked=1) - svc = SemanticMemoryService(conn) - results = svc.list_for_project(project="p", track_exposure=False) - ids = [m.id for m in results] - assert ids.index("s-overlooked-1") < ids.index("s-useful-2") - - def test_semantic_high_useful_count_still_beats_one_overlooked(self, conn): - from better_memory.services.semantic import SemanticMemoryService - _seed_semantic(conn, "s-useful-4", useful_count=4, - times_overlooked=0) - _seed_semantic(conn, "s-overlooked-1", useful_count=0, - times_overlooked=1) - svc = SemanticMemoryService(conn) - results = svc.list_for_project(project="p", track_exposure=False) - ids = [m.id for m in results] - assert ids.index("s-useful-4") < ids.index("s-overlooked-1") - - def test_semantic_read_model_carries_times_overlooked(self, conn): - from better_memory.services.semantic import SemanticMemoryService - _seed_semantic(conn, "s1", times_overlooked=5) - svc = SemanticMemoryService(conn) - results = svc.list_for_project(project="p", track_exposure=False) - assert results[0].times_overlooked == 5 diff --git a/tests/services/test_wilson_ranking.py b/tests/services/test_wilson_ranking.py new file mode 100644 index 0000000..1f52fe0 --- /dev/null +++ b/tests/services/test_wilson_ranking.py @@ -0,0 +1,85 @@ +"""Retrieval ranks by Wilson lower bound on (useful+overlooked)/rated. + +Replaces the popularity + overlooked-weight + ignored-demotion stack. +Covers the old demotion scenarios too: proven dead weight sinks because +0 positive over many rated gives LB ~ 0, with no special-case code. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.reflection import ReflectionSynthesisService + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed(conn, rid, *, useful=0, overlooked=0, ignored=0, confidence=0.5, + polarity="do", updated_at="2026-01-01"): + conn.execute( + """INSERT INTO reflections + (id, title, project, phase, polarity, use_cases, hints, + confidence, created_at, updated_at, useful_count, + times_overlooked, times_ignored) + VALUES (?, ?, 'p', 'general', ?, 'uc', '[]', ?, + '2026-01-01', ?, ?, ?, ?)""", + (rid, rid, polarity, confidence, updated_at, useful, overlooked, ignored), + ) + conn.commit() + + +def _ids(conn, **kw): + svc = ReflectionSynthesisService(conn) + return [r["id"] for r in svc.retrieve_reflections(project="p", **kw)["do"]] + + +class TestWilsonOrdering: + def test_hit_rate_beats_raw_count(self, conn): + _seed(conn, "r-workhorse", useful=67, ignored=125) # 67/192 ~ 0.28 + _seed(conn, "r-newcomer", useful=3, ignored=1) # 3/4 ~ 0.30 + assert _ids(conn)[:2] == ["r-newcomer", "r-workhorse"] + + def test_overlooked_counts_as_positive(self, conn): + _seed(conn, "r-overlooked", overlooked=3, ignored=1) + _seed(conn, "r-plain", useful=1, ignored=3) + assert _ids(conn)[0] == "r-overlooked" + + def test_proven_dead_weight_sinks_below_modest_performer(self, conn): + _seed(conn, "r-dead", useful=0, ignored=58) + _seed(conn, "r-modest", useful=2, ignored=8) + assert _ids(conn) == ["r-modest", "r-dead"] + + def test_confidence_breaks_wilson_ties(self, conn): + _seed(conn, "r-low-conf", confidence=0.3) + _seed(conn, "r-high-conf", confidence=0.9) + assert _ids(conn) == ["r-high-conf", "r-low-conf"] + + def test_recency_breaks_confidence_ties(self, conn): + _seed(conn, "r-old", updated_at="2026-01-01") + _seed(conn, "r-new", updated_at="2026-06-01") + assert _ids(conn) == ["r-new", "r-old"] + + def test_rows_expose_all_three_counters(self, conn): + _seed(conn, "r-a", useful=1, overlooked=2, ignored=3) + svc = ReflectionSynthesisService(conn) + row = svc.retrieve_reflections(project="p")["do"][0] + assert row["useful_count"] == 1 + assert row["times_overlooked"] == 2 + assert row["times_ignored"] == 3 + + def test_demotion_constants_are_gone(self): + import better_memory.services.memory_rating as mr + for name in ("IGNORED_DEMOTION_FLOOR", "IGNORED_DEMOTION_WEIGHT", + "OVERLOOKED_RANKING_WEIGHT"): + assert not hasattr(mr, name), f"{name} should be deleted" From a23a7ee84fc9d7a0d10136282c276f8fa32b9bb0 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 08:49:40 +0100 Subject: [PATCH 06/20] feat(ranking): semantic memories ranked by Wilson prior Co-Authored-By: Claude Fable 5 --- better_memory/services/semantic.py | 25 ++++++++++++++++++++----- tests/services/test_semantic.py | 26 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/better_memory/services/semantic.py b/better_memory/services/semantic.py index d47555c..9c63af5 100644 --- a/better_memory/services/semantic.py +++ b/better_memory/services/semantic.py @@ -16,6 +16,7 @@ from uuid import uuid4 from better_memory._common import default_clock, env_session_id +from better_memory.services.scoring import wilson_lower_bound @dataclass(frozen=True) @@ -34,6 +35,8 @@ class SemanticMemory: last_misled_at: str | None = None times_overlooked: int = 0 last_overlooked_at: str | None = None + times_ignored: int = 0 + last_ignored_at: str | None = None _VALID_SCOPES = ("project", "general") @@ -195,7 +198,8 @@ def list_for_project( track_exposure: bool = True, ) -> list[SemanticMemory]: """Project rows + general-scope rows from any project, ordered by - useful_count DESC then created_at DESC (newest first as tiebreaker). + Wilson lower bound of the hit rate DESC, then created_at DESC + (newest first) as tiebreaker for equal scores. Args: project: project key for project-scope filtering. @@ -236,12 +240,11 @@ def list_for_project( sql = ( "SELECT id, content, project, scope, created_at, updated_at, " "useful_count, last_useful_at, times_misled, last_misled_at, " - "times_overlooked, last_overlooked_at " + "times_overlooked, last_overlooked_at, " + "times_ignored, last_ignored_at " "FROM semantic_memories " f"WHERE {' AND '.join(where_clauses)} " - # Minimal bridge pending Task 3's Wilson-score rewrite of - # semantic.py ranking (mirrors reflection.py's Task 2 change). - "ORDER BY (useful_count) DESC, created_at DESC" + "ORDER BY created_at DESC" ) rows = self._conn.execute(sql, params).fetchall() results = [ @@ -255,9 +258,21 @@ def list_for_project( last_misled_at=r["last_misled_at"], times_overlooked=r["times_overlooked"] or 0, last_overlooked_at=r["last_overlooked_at"], + times_ignored=r["times_ignored"] or 0, + last_ignored_at=r["last_ignored_at"], ) for r in rows ] + # Rank by Wilson lower bound of the hit rate (useful + overlooked + # positives over rated exposures); stable sort keeps created_at DESC + # as the tiebreaker for equal scores (incl. all-zero/never-rated). + results.sort( + key=lambda m: wilson_lower_bound( + m.useful_count + m.times_overlooked, + m.useful_count + m.times_overlooked + m.times_ignored, + ), + reverse=True, + ) # Best-effort exposure tracking — see spec §5.2.1. # track_exposure=False is used by SessionBootstrapService.bootstrap, # which manages its own exposure write via _record_exposure. diff --git a/tests/services/test_semantic.py b/tests/services/test_semantic.py index 52177d8..7d2a0dc 100644 --- a/tests/services/test_semantic.py +++ b/tests/services/test_semantic.py @@ -569,3 +569,29 @@ def test_defaults_to_zero_counts(self, conn): assert row.times_misled == 0 assert row.last_useful_at is None assert row.last_misled_at is None + + +class TestSemanticWilsonRanking: + def test_hit_rate_beats_raw_count(self, conn): + from better_memory.services.semantic import SemanticMemoryService + svc = SemanticMemoryService(conn) + a = svc.create(content="workhorse", project="p") + b = svc.create(content="newcomer", project="p") + conn.execute( + "UPDATE semantic_memories SET useful_count=67, times_ignored=125 WHERE id=?", (a,)) + conn.execute( + "UPDATE semantic_memories SET useful_count=3, times_ignored=1 WHERE id=?", (b,)) + conn.commit() + ids = [m.id for m in svc.list_for_project(project="p", track_exposure=False)] + assert ids == [b, a] + + def test_never_rated_sorts_by_recency_at_bottom(self, conn): + from better_memory.services.semantic import SemanticMemoryService + svc = SemanticMemoryService(conn) + rated = svc.create(content="rated", project="p") + conn.execute( + "UPDATE semantic_memories SET useful_count=1, times_ignored=1 WHERE id=?", (rated,)) + conn.commit() + unrated = svc.create(content="unrated", project="p") + ids = [m.id for m in svc.list_for_project(project="p", track_exposure=False)] + assert ids == [rated, unrated] From 14dd7a61b4d54492e272797c481c647066ba0f15 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 08:54:19 +0100 Subject: [PATCH 07/20] feat(ranking): reserved exploration slot per bucket for untested memories Co-Authored-By: Claude Fable 5 --- better_memory/services/reflection.py | 66 ++++++++++++++------ tests/services/test_exploration_slot.py | 81 +++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 tests/services/test_exploration_slot.py diff --git a/better_memory/services/reflection.py b/better_memory/services/reflection.py index 1b414f8..0686af3 100644 --- a/better_memory/services/reflection.py +++ b/better_memory/services/reflection.py @@ -296,6 +296,12 @@ def _parse_merge(item: object) -> MergeAction: ) +#: A memory with fewer than this many rated exposures is "untested": its +#: Wilson score is statistically meaningless, so it competes for the +#: reserved exploration slot instead of the proven slots. +EXPLORATION_RATED_FLOOR = 3 + + def _wilson_rated(row) -> int: """Total rated exposures backing this memory's score.""" return (row["useful_count"] + row["times_overlooked"] @@ -1260,6 +1266,24 @@ def _fuse_by_relevance( scored.sort(key=lambda t: (-t[0], t[1])) return [row for _, _, row in scored] + def _bucket_item(self, r) -> dict: + """Convert one reflections row into the dict shape returned to callers.""" + return { + "id": r["id"], + "title": r["title"], + "phase": r["phase"], + "use_cases": r["use_cases"], + "hints": json.loads(r["hints"]), + "confidence": r["confidence"], + "tech": r["tech"], + "evidence_count": r["evidence_count"], + "useful_count": r["useful_count"], + "times_misled": r["times_misled"], + "times_overlooked": r["times_overlooked"], + "times_ignored": r["times_ignored"], + "updated_at": r["updated_at"], + } + def retrieve_reflections( self, *, @@ -1345,27 +1369,35 @@ def retrieve_reflections( _diag.step(fn, "relevance_fused", n_rows=len(rows)) # Convert None (unlimited) to sys.maxsize so the loop body has a definite int. + # reserve: only worth reserving an exploration slot when there's room for + # at least one proven row alongside it (cap < 2 means the untested row + # would consume the entire bucket). cap = limit_per_bucket if limit_per_bucket is not None else sys.maxsize + reserve = limit_per_bucket is not None and cap >= 2 buckets: dict[str, list[dict]] = {"do": [], "dont": [], "neutral": []} + by_polarity: dict[str, list] = {"do": [], "dont": [], "neutral": []} for r in rows: - bucket = buckets[r["polarity"]] - if len(bucket) >= cap: + by_polarity[r["polarity"]].append(r) + for polarity, group in by_polarity.items(): + if not reserve: + buckets[polarity] = [self._bucket_item(r) for r in group[:cap]] continue - bucket.append({ - "id": r["id"], - "title": r["title"], - "phase": r["phase"], - "use_cases": r["use_cases"], - "hints": json.loads(r["hints"]), - "confidence": r["confidence"], - "tech": r["tech"], - "evidence_count": r["evidence_count"], - "useful_count": r["useful_count"], - "times_misled": r["times_misled"], - "times_overlooked": r["times_overlooked"], - "times_ignored": r["times_ignored"], - "updated_at": r["updated_at"], - }) + tested_idx = [i for i, r in enumerate(group) + if _wilson_rated(r) >= EXPLORATION_RATED_FLOOR] + untested_idx = [i for i, r in enumerate(group) + if _wilson_rated(r) < EXPLORATION_RATED_FLOOR] + chosen = tested_idx[: cap - 1] + if untested_idx: + chosen.append(untested_idx[0]) + if len(chosen) < cap: # top up from the remainder + taken = set(chosen) + for i in range(len(group)): + if len(chosen) >= cap: + break + if i not in taken: + chosen.append(i) + chosen.sort() # preserve ranked order + buckets[polarity] = [self._bucket_item(group[i]) for i in chosen] _diag.step( fn, "bucketed", do=len(buckets["do"]), dont=len(buckets["dont"]), diff --git a/tests/services/test_exploration_slot.py b/tests/services/test_exploration_slot.py new file mode 100644 index 0000000..51e4948 --- /dev/null +++ b/tests/services/test_exploration_slot.py @@ -0,0 +1,81 @@ +"""One shortlist slot per bucket is reserved for an untested memory. + +Untested = fewer than 3 rated exposures. Wilson scores them 0.0, so +without the slot they would never be served and never earn a rating. +Rating coverage is ~100% (sync Stop hook), so one serve = one rating. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.reflection import ReflectionSynthesisService + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed(conn, rid, *, useful=0, ignored=0, updated_at="2026-01-01"): + conn.execute( + """INSERT INTO reflections + (id, title, project, phase, polarity, use_cases, hints, + confidence, created_at, updated_at, useful_count, times_ignored) + VALUES (?, ?, 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', ?, ?, ?)""", + (rid, rid, updated_at, useful, ignored), + ) + conn.commit() + + +def _ids(conn, cap=3): + svc = ReflectionSynthesisService(conn) + return [r["id"] for r in + svc.retrieve_reflections(project="p", limit_per_bucket=cap)["do"]] + + +class TestExplorationSlot: + def test_last_slot_goes_to_best_untested(self, conn): + for i in range(4): # four proven memories + _seed(conn, f"r-proven-{i}", useful=5 - i, ignored=5) + _seed(conn, "r-untested-old", updated_at="2026-02-01") + _seed(conn, "r-untested-new", updated_at="2026-06-01") + ids = _ids(conn, cap=3) + assert len(ids) == 3 + assert ids[:2] == ["r-proven-0", "r-proven-1"] # cap-1 proven + assert ids[2] == "r-untested-new" # best untested + + def test_no_untested_fills_all_slots_with_proven(self, conn): + for i in range(4): + _seed(conn, f"r-proven-{i}", useful=5 - i, ignored=5) + ids = _ids(conn, cap=3) + assert ids == ["r-proven-0", "r-proven-1", "r-proven-2"] + + def test_all_untested_fills_normally(self, conn): + for i in range(4): + _seed(conn, f"r-untested-{i}") + assert len(_ids(conn, cap=3)) == 3 + + def test_two_ratings_is_still_untested_three_is_not(self, conn): + _seed(conn, "r-two", useful=1, ignored=1) # rated == 2: untested + _seed(conn, "r-three", useful=1, ignored=2) # rated == 3: tested + for i in range(3): + _seed(conn, f"r-proven-{i}", useful=9, ignored=1) + ids = _ids(conn, cap=3) + assert ids[2] == "r-two" + + def test_unlimited_cap_reserves_nothing(self, conn): + _seed(conn, "r-proven", useful=5, ignored=5) + _seed(conn, "r-untested") + svc = ReflectionSynthesisService(conn) + rows = svc.retrieve_reflections(project="p", limit_per_bucket=None)["do"] + assert len(rows) == 2 # everything returned anyway From 0bc6fc0f0b0e5fab4b224defe72aa3ddc22fee4e Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 08:58:13 +0100 Subject: [PATCH 08/20] feat(db): semantic_embeddings vec0 table (migration 0014) Co-Authored-By: Claude Fable 5 --- .../db/migrations/0014_semantic_embeddings.sql | 12 ++++++++++++ tests/db/test_schema.py | 14 ++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 better_memory/db/migrations/0014_semantic_embeddings.sql diff --git a/better_memory/db/migrations/0014_semantic_embeddings.sql b/better_memory/db/migrations/0014_semantic_embeddings.sql new file mode 100644 index 0000000..93858ce --- /dev/null +++ b/better_memory/db/migrations/0014_semantic_embeddings.sql @@ -0,0 +1,12 @@ +-- Migration 0014: vector index for semantic memories. +-- +-- reflection_embeddings has existed since 0002; semantic memories never got +-- the parallel table, so they have no semantic-search substrate. Written by +-- SemanticMemoryService on create/update, healed lazily on retrieve, and +-- backfilled once by cli.backfill_embeddings. 768 dims = nomic-embed-text, +-- matching observation_embeddings and reflection_embeddings. + +CREATE VIRTUAL TABLE semantic_embeddings USING vec0( + memory_id TEXT PRIMARY KEY, + embedding FLOAT[768] +); diff --git a/tests/db/test_schema.py b/tests/db/test_schema.py index 92be3dd..e9c1ad5 100644 --- a/tests/db/test_schema.py +++ b/tests/db/test_schema.py @@ -228,6 +228,7 @@ def test_apply_migrations_is_idempotent(tmp_memory_db: Path) -> None: expected_initial = [ "0001", "0002", "0003", "0004", "0005", "0006", "0007", "0008", "0009", "0010", "0011", + "0012", "0013", "0014", ] assert versions[: len(expected_initial)] == expected_initial # Subsequent migrations are appended; idempotency is the property @@ -785,3 +786,16 @@ def test_idx_episodes_pending_synth_partial_index_exists(tmp_memory_db: Path) -> ).fetchall() assert len(rows) == 1 assert "synthesized_at IS NULL" in rows[0][1] + + +def test_0014_semantic_embeddings_table(tmp_memory_db: Path) -> None: + """Migration 0014 creates the semantic_embeddings vec0 virtual table.""" + conn = connect(tmp_memory_db) + try: + apply_migrations(conn) + row = conn.execute( + "SELECT name FROM sqlite_master WHERE name='semantic_embeddings'" + ).fetchone() + assert row is not None + finally: + conn.close() From 2648d054f2a41957512ad60579fbd3df4028f1e5 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 09:01:02 +0100 Subject: [PATCH 09/20] =?UTF-8?q?feat(embeddings):=20SyncEmbedder=20?= =?UTF-8?q?=E2=80=94=20bridge=20facade=20with=20fresh-per-call=20embedder?= =?UTF-8?q?=20and=2060s=20breaker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- better_memory/embeddings/sync_embed.py | 75 ++++++++++++++++++++++++++ tests/embeddings/test_sync_embed.py | 74 +++++++++++++++++++++++++ tests/services/_embedding_fakes.py | 24 +++++++++ 3 files changed, 173 insertions(+) create mode 100644 better_memory/embeddings/sync_embed.py create mode 100644 tests/embeddings/test_sync_embed.py create mode 100644 tests/services/_embedding_fakes.py diff --git a/better_memory/embeddings/sync_embed.py b/better_memory/embeddings/sync_embed.py new file mode 100644 index 0000000..f259536 --- /dev/null +++ b/better_memory/embeddings/sync_embed.py @@ -0,0 +1,75 @@ +"""Sync facade over the async embedder, for thread-bound sync services. + +Two hazards this class exists to contain: + +1. ``httpx.AsyncClient`` is bound to the event loop that first uses it, + and ``run_async_in_worker`` runs a fresh loop per call — so the + embedder must be CONSTRUCTED inside the worker coroutine and closed + there. The factory is invoked per call; ``OllamaEmbedder`` construction + is cheap and contacts nothing. +2. Retrieval must never hang on a dead Ollama. Any failure opens a + circuit breaker: for ``cooldown`` seconds every call returns ``None`` + immediately. Worst case is one bounded stall per cooldown window. + +Returns ``None`` on every failure path — callers treat a missing vector +as "no vec leg", never as an error. +""" + +from __future__ import annotations + +import time +from collections.abc import Callable +from typing import Any + +from better_memory.async_bridge import run_async_in_worker + +#: Bridge-level hard stop. The wiring uses OllamaEmbedder(timeout=5.0, +#: max_retries=1), so a healthy-but-slow call ends well inside this; the +#: bridge timeout is the backstop against pathological hangs. +_WORKER_TIMEOUT = 15.0 +_DEFAULT_COOLDOWN = 60.0 + + +class SyncEmbedder: + def __init__( + self, + factory: Callable[[], Any] | None, + *, + clock: Callable[[], float] = time.monotonic, + cooldown: float = _DEFAULT_COOLDOWN, + timeout: float = _WORKER_TIMEOUT, + ) -> None: + self._factory = factory + self._clock = clock + self._cooldown = cooldown + self._timeout = timeout + self._down_until = 0.0 + + def embed_text(self, text: str) -> list[float] | None: + return self._run(lambda emb: emb.embed(text)) + + def embed_batch(self, texts: list[str]) -> list[list[float]] | None: + return self._run(lambda emb: emb.embed_batch(texts)) + + def _run(self, op: Callable[[Any], Any]): + if self._factory is None: + return None + if self._clock() < self._down_until: + return None + + factory = self._factory + + async def _go(): + emb = factory() + try: + return await op(emb) + finally: + aclose = getattr(emb, "aclose", None) + if aclose is not None: + await aclose() + + try: + return run_async_in_worker(_go, timeout=self._timeout) + except Exception: + self._down_until = self._clock() + self._cooldown + return None diff --git a/tests/embeddings/test_sync_embed.py b/tests/embeddings/test_sync_embed.py new file mode 100644 index 0000000..20ae985 --- /dev/null +++ b/tests/embeddings/test_sync_embed.py @@ -0,0 +1,74 @@ +"""SyncEmbedder: sync facade over the async embedder for thread-bound code. + +Fresh embedder per call (loop-bound AsyncClient), closed in the worker; +60s circuit breaker so an Ollama outage costs one stall per cooldown +window instead of one per call. +""" +from __future__ import annotations + +from better_memory.embeddings.sync_embed import SyncEmbedder +from tests.services._embedding_fakes import FakeEmbedder + + +class FakeClock: + def __init__(self): + self.t = 1000.0 + + def __call__(self) -> float: + return self.t + + +class TestSyncEmbedder: + def test_embed_text_returns_vector_and_closes_embedder(self): + fake = FakeEmbedder() + s = SyncEmbedder(lambda: fake) + vec = s.embed_text("hello") + assert vec is not None and len(vec) == 768 + assert fake.calls == ["hello"] + assert fake.closed == 1 + + def test_embed_batch_returns_vectors(self): + fake = FakeEmbedder() + s = SyncEmbedder(lambda: fake) + out = s.embed_batch(["a", "b"]) + assert out is not None and len(out) == 2 + + def test_failure_returns_none_and_opens_breaker(self): + fake = FakeEmbedder(fail=True) + clock = FakeClock() + s = SyncEmbedder(lambda: fake, clock=clock) + assert s.embed_text("x") is None + assert fake.calls == ["x"] + # Breaker open: the embedder is not touched again. + assert s.embed_text("y") is None + assert fake.calls == ["x"] + + def test_breaker_closes_after_cooldown(self): + clock = FakeClock() + calls = [] + + class FlakyThenGood(FakeEmbedder): + async def embed(self, text): + calls.append(text) + if len(calls) == 1: + raise RuntimeError("down") + return [0.1] * 768 + + s = SyncEmbedder(FlakyThenGood, clock=clock, cooldown=60.0) + assert s.embed_text("first") is None + clock.t += 61.0 + assert s.embed_text("second") is not None + assert calls == ["first", "second"] + + def test_none_factory_disables_everything(self): + s = SyncEmbedder(None) + assert s.embed_text("x") is None + assert s.embed_batch(["x"]) is None + + def test_embedder_without_aclose_is_fine(self): + class Bare: + async def embed(self, text): + return [0.1] * 768 + + s = SyncEmbedder(Bare) + assert s.embed_text("x") is not None diff --git a/tests/services/_embedding_fakes.py b/tests/services/_embedding_fakes.py new file mode 100644 index 0000000..d9258b2 --- /dev/null +++ b/tests/services/_embedding_fakes.py @@ -0,0 +1,24 @@ +"""Shared fake embedder for embedding-path tests.""" +from __future__ import annotations + + +class FakeEmbedder: + def __init__(self, fail: bool = False): + self.calls: list = [] + self.closed = 0 + self.fail = fail + + async def embed(self, text: str) -> list[float]: + self.calls.append(text) + if self.fail: + raise RuntimeError("ollama down") + return [0.1] * 768 + + async def embed_batch(self, texts: list[str]) -> list[list[float]]: + self.calls.append(list(texts)) + if self.fail: + raise RuntimeError("ollama down") + return [[0.1] * 768 for _ in texts] + + async def aclose(self) -> None: + self.closed += 1 From cfd1c785ced3c49d083ce0895df5eb309835e048 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 09:10:17 +0100 Subject: [PATCH 10/20] feat(embeddings): reflections embedded at synthesis write time via SyncEmbedder Co-Authored-By: Claude Fable 5 --- better_memory/mcp/server.py | 11 +- better_memory/services/reflection.py | 56 +++- better_memory/storage/sqlite.py | 10 +- .../test_reflection_embedding_write.py | 278 ++++++++++++++++++ 4 files changed, 351 insertions(+), 4 deletions(-) create mode 100644 tests/services/test_reflection_embedding_write.py diff --git a/better_memory/mcp/server.py b/better_memory/mcp/server.py index 1b0e87a..6713e51 100644 --- a/better_memory/mcp/server.py +++ b/better_memory/mcp/server.py @@ -62,6 +62,7 @@ from better_memory.db.connection import connect from better_memory.db.schema import apply_migrations from better_memory.embeddings.ollama import OllamaEmbedder +from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.mcp._util import resolve_session_id as _resolve_session_id from better_memory.mcp.handlers import ( EpisodeToolHandlers, @@ -180,6 +181,14 @@ def create_server() -> tuple[ # memory.retrieve will succeed on their next call without a restart. _probe_ollama(config.ollama_host) + # Fresh short-timeout embedder per bridge call (loop-bound client); + # shared instance so the breaker state is process-wide. + sync_embedder: SyncEmbedder | None = None + if embedder is not None: + sync_embedder = SyncEmbedder( + lambda: OllamaEmbedder(timeout=5.0, max_retries=1) + ) + # Concurrency invariant: the memory-side services below # (EpisodeService, ObservationService, ReflectionSynthesisService, # RetentionService, SpoolService, SemanticMemoryService, @@ -219,7 +228,7 @@ def create_server() -> tuple[ # Reflection synthesis is driven by the IDE-LLM via two MCP tools # (memory.synthesize_next_get_context / _apply). The service no # longer holds a chat client. - reflections = ReflectionSynthesisService(memory_conn) + reflections = ReflectionSynthesisService(memory_conn, sync_embedder=sync_embedder) retention = RetentionService(conn=memory_conn) memory_rating = MemoryRatingService(memory_conn) semantic = SemanticMemoryService(memory_conn) diff --git a/better_memory/services/reflection.py b/better_memory/services/reflection.py index 0686af3..13f7c5a 100644 --- a/better_memory/services/reflection.py +++ b/better_memory/services/reflection.py @@ -31,8 +31,11 @@ from dataclasses import dataclass from datetime import datetime, timedelta +import sqlite_vec + from better_memory import _diag from better_memory._common import default_clock, env_session_id +from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.search.query import sanitize_fts5_query from better_memory.services.scoring import wilson_lower_bound @@ -50,6 +53,11 @@ }) +def _embedding_source_text(title: str, use_cases: str, hints: list[str]) -> str: + """What gets embedded for a reflection: the discriminating text.""" + return "\n".join([title, use_cases, *hints]) + + def _later_ts(a: str | None, b: str | None) -> str | None: """Return the later of two ISO-8601 timestamps, treating NULL as -inf. @@ -332,9 +340,26 @@ def __init__( conn: sqlite3.Connection, *, clock: Callable[[], datetime] | None = None, + sync_embedder: SyncEmbedder | None = None, ) -> None: self._conn = conn self._clock: Callable[[], datetime] = clock or default_clock + self._sync_embedder = sync_embedder + + def _store_embedding(self, reflection_id: str, vector: list[float] | None) -> None: + if vector is None: + return + # DELETE+INSERT: vec0 tables historically mishandle UPSERT. + self._conn.execute( + "DELETE FROM reflection_embeddings WHERE reflection_id = ?", + (reflection_id,), + ) + self._conn.execute( + "INSERT INTO reflection_embeddings (reflection_id, embedding) " + "VALUES (?, ?)", + (reflection_id, sqlite_vec.serialize_float32(vector)), + ) + @staticmethod def _normalize_tech(tech: str | None) -> str | None: # Mirror EpisodeService.start_foreground / ObservationService.create @@ -785,6 +810,14 @@ def _apply_new( [now, *valid_sources], ) + if self._sync_embedder is not None: + self._store_embedding( + reflection_id, + self._sync_embedder.embed_text(_embedding_source_text( + action.title, action.use_cases, action.hints, + )), + ) + def _derive_new_reflection_scope( self, source_obs_ids: list[str] ) -> str: @@ -844,8 +877,8 @@ def _apply_augment(self, actions: list[AugmentAction]) -> None: # leftover from a prior iteration. now = self._clock().isoformat() row = self._conn.execute( - "SELECT hints, confidence, status FROM reflections " - "WHERE id = ?", + "SELECT title, use_cases, hints, confidence, status " + "FROM reflections WHERE id = ?", (action.reflection_id,), ).fetchone() if row is None: @@ -930,6 +963,17 @@ def _apply_augment(self, actions: list[AugmentAction]) -> None: ), ) + if self._sync_embedder is not None: + final_use_cases = (action.rewrite_use_cases + if action.rewrite_use_cases is not None + else row["use_cases"]) + self._store_embedding( + action.reflection_id, + self._sync_embedder.embed_text(_embedding_source_text( + row["title"], final_use_cases, merged_hints, + )), + ) + # ------------------------------------------------------------- _apply_merge def _apply_merge(self, actions: list[MergeAction]) -> None: """Merge source reflection into target, dropping unknown ids. @@ -1052,6 +1096,14 @@ def _apply_merge(self, actions: list[MergeAction]) -> None: ), ) + # Target text is untouched by merge (counters/evidence only), so + # no re-embed; drop the superseded source's vector so it stops + # competing for kNN slots. + self._conn.execute( + "DELETE FROM reflection_embeddings WHERE reflection_id = ?", + (action.source_id,), + ) + # ------------------------------------------------------------ _apply_ignore def _apply_ignore(self, observation_ids: list[str]) -> None: """Mark observations as consumed_without_reflection. diff --git a/better_memory/storage/sqlite.py b/better_memory/storage/sqlite.py index f3dbab9..e824a29 100644 --- a/better_memory/storage/sqlite.py +++ b/better_memory/storage/sqlite.py @@ -25,6 +25,8 @@ import sqlite3 from typing import Any +from better_memory.embeddings.ollama import OllamaEmbedder +from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.services.episode import EpisodeService from better_memory.services.memory_rating import MemoryRatingService from better_memory.services.observation import ObservationService @@ -65,7 +67,13 @@ def __init__( self._reflection = ReflectionService(memory_conn) self._memory_rating = MemoryRatingService(memory_conn) self._session_bootstrap = SessionBootstrapService(memory_conn) - self._synthesis = ReflectionSynthesisService(memory_conn) + sync_embedder = ( + SyncEmbedder(lambda: OllamaEmbedder(timeout=5.0, max_retries=1)) + if embedder is not None else None + ) + self._synthesis = ReflectionSynthesisService( + memory_conn, sync_embedder=sync_embedder + ) # ----- Capability flags ----- diff --git a/tests/services/test_reflection_embedding_write.py b/tests/services/test_reflection_embedding_write.py new file mode 100644 index 0000000..91abdb3 --- /dev/null +++ b/tests/services/test_reflection_embedding_write.py @@ -0,0 +1,278 @@ +"""Synthesis writes reflection embeddings, best-effort. + +reflection_embeddings sat at 0 rows from migration 0002 until this change: +the write path simply never embedded. Failures must never block synthesis. +""" +from __future__ import annotations + +from datetime import UTC, datetime +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.embeddings.sync_embed import SyncEmbedder +from better_memory.services.episode import EpisodeService +from better_memory.services.reflection import ( + AugmentAction, + MergeAction, + NewAction, + ReflectionSynthesisService, + _embedding_source_text, +) +from tests.services._embedding_fakes import FakeEmbedder + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +@pytest.fixture +def fixed_clock(): + fixed = datetime(2026, 4, 22, 10, 0, 0, tzinfo=UTC) + return lambda: fixed + + +def _vec_count(conn): + return conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] + + +def _insert_obs( + conn, + *, + obs_id: str, + project: str, + episode_id: str, + outcome: str = "success", + content: str = "obs content", + component: str | None = None, + theme: str | None = None, + tech: str | None = None, + created_at: str = "2026-04-22T09:00:00+00:00", + status: str = "active", + status_changed_at: str | None = None, +) -> None: + if status_changed_at is None: + status_changed_at = created_at + conn.execute( + """ + INSERT INTO observations ( + id, content, project, component, theme, outcome, + reinforcement_score, episode_id, tech, created_at, status, + status_changed_at + ) VALUES (?, ?, ?, ?, ?, ?, 0.0, ?, ?, ?, ?, ?) + """, + (obs_id, content, project, component, theme, outcome, + episode_id, tech, created_at, status, status_changed_at), + ) + + +def _insert_reflection( + conn, + *, + refl_id: str, + project: str, + phase: str = "general", + polarity: str = "do", + status: str = "pending_review", + tech: str | None = None, + confidence: float = 0.5, + use_cases: str = "uc", + hints: str = "[]", + title: str = "t", + evidence_count: int = 0, +) -> None: + import json as _json + if not hints.startswith("["): + hints = _json.dumps(hints) + conn.execute( + """ + INSERT INTO reflections ( + id, title, project, tech, phase, polarity, use_cases, hints, + confidence, status, evidence_count, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + (refl_id, title, project, tech, phase, polarity, use_cases, hints, + confidence, status, evidence_count, + "2026-04-22T08:00:00+00:00", "2026-04-22T08:00:00+00:00"), + ) + + +def test_embedding_source_text_joins_fields(): + assert _embedding_source_text("T", "when X", ["h1", "h2"]) == "T\nwhen X\nh1\nh2" + + +class TestApplyNewEmbedding: + def test_embeds_on_new_reflection(self, conn, fixed_clock): + epsvc = EpisodeService(conn, clock=fixed_clock) + ep = epsvc.start_foreground(session_id="s1", project="p", goal="g") + epsvc.close_active( + session_id="s1", outcome="success", close_reason="goal_complete" + ) + _insert_obs(conn, obs_id="obs-1", project="p", episode_id=ep) + conn.commit() + + fake = FakeEmbedder() + sync_embedder = SyncEmbedder(lambda: fake) + svc = ReflectionSynthesisService( + conn, clock=fixed_clock, sync_embedder=sync_embedder + ) + action = NewAction( + title="Always test", phase="general", polarity="do", + use_cases="when writing code", hints=["write tests first"], + tech="python", confidence=0.6, + source_observation_ids=["obs-1"], + ) + svc._apply_new([action], project="p") + conn.commit() + + assert _vec_count(conn) == 1 + assert len(fake.calls) == 1 + assert "Always test" in fake.calls[0] + + def test_no_embed_row_when_embedder_fails(self, conn, fixed_clock): + epsvc = EpisodeService(conn, clock=fixed_clock) + ep = epsvc.start_foreground(session_id="s1", project="p", goal="g") + epsvc.close_active( + session_id="s1", outcome="success", close_reason="goal_complete" + ) + _insert_obs(conn, obs_id="obs-1", project="p", episode_id=ep) + conn.commit() + + sync_embedder = SyncEmbedder(lambda: FakeEmbedder(fail=True)) + svc = ReflectionSynthesisService( + conn, clock=fixed_clock, sync_embedder=sync_embedder + ) + action = NewAction( + title="t", phase="general", polarity="do", + use_cases="uc", hints=[], tech=None, confidence=0.5, + source_observation_ids=["obs-1"], + ) + svc._apply_new([action], project="p") + conn.commit() + + refl = conn.execute( + "SELECT id FROM reflections WHERE title = 't'" + ).fetchone() + assert refl is not None + assert _vec_count(conn) == 0 + + def test_no_embed_when_sync_embedder_not_passed(self, conn, fixed_clock): + epsvc = EpisodeService(conn, clock=fixed_clock) + ep = epsvc.start_foreground(session_id="s1", project="p", goal="g") + epsvc.close_active( + session_id="s1", outcome="success", close_reason="goal_complete" + ) + _insert_obs(conn, obs_id="obs-1", project="p", episode_id=ep) + conn.commit() + + svc = ReflectionSynthesisService(conn, clock=fixed_clock) + action = NewAction( + title="t", phase="general", polarity="do", + use_cases="uc", hints=[], tech=None, confidence=0.5, + source_observation_ids=["obs-1"], + ) + svc._apply_new([action], project="p") + conn.commit() + + refl = conn.execute( + "SELECT id FROM reflections WHERE title = 't'" + ).fetchone() + assert refl is not None + assert _vec_count(conn) == 0 + + +class TestApplyAugmentEmbedding: + def test_reembeds_on_augment(self, conn, fixed_clock): + _insert_reflection( + conn, refl_id="r1", project="p", + hints='["old-hint"]', confidence=0.5, evidence_count=1, + title="Existing", use_cases="old uc", + ) + conn.commit() + + fake = FakeEmbedder() + sync_embedder = SyncEmbedder(lambda: fake) + svc = ReflectionSynthesisService( + conn, clock=fixed_clock, sync_embedder=sync_embedder + ) + action = NewAction( + title="seed", phase="general", polarity="do", + use_cases="uc", hints=[], tech=None, confidence=0.5, + source_observation_ids=[], + ) + # Seed one embedding row directly to prove augment replaces it + # (DELETE+INSERT), not merely inserting a duplicate. + import sqlite_vec + conn.execute( + "INSERT INTO reflection_embeddings (reflection_id, embedding) " + "VALUES (?, ?)", + ("r1", sqlite_vec.serialize_float32([0.0] * 768)), + ) + conn.commit() + assert _vec_count(conn) == 1 + + augment_action = AugmentAction( + reflection_id="r1", + add_hints=["new-hint"], + rewrite_use_cases=None, + confidence_delta=0.0, + add_source_observation_ids=[], + ) + svc._apply_augment([augment_action]) + conn.commit() + + assert _vec_count(conn) == 1 + assert len(fake.calls) == 1 + assert "new-hint" in fake.calls[0] + + +class TestApplyMergeEmbedding: + def test_merge_deletes_source_embedding_no_reembed_of_target( + self, conn, fixed_clock + ): + _insert_reflection(conn, refl_id="src", project="p") + _insert_reflection(conn, refl_id="tgt", project="p") + conn.commit() + + import sqlite_vec + conn.execute( + "INSERT INTO reflection_embeddings (reflection_id, embedding) " + "VALUES (?, ?)", + ("src", sqlite_vec.serialize_float32([0.1] * 768)), + ) + conn.execute( + "INSERT INTO reflection_embeddings (reflection_id, embedding) " + "VALUES (?, ?)", + ("tgt", sqlite_vec.serialize_float32([0.2] * 768)), + ) + conn.commit() + + fake = FakeEmbedder() + sync_embedder = SyncEmbedder(lambda: fake) + svc = ReflectionSynthesisService( + conn, clock=fixed_clock, sync_embedder=sync_embedder + ) + action = MergeAction(source_id="src", target_id="tgt", justification="dupes") + svc._apply_merge([action]) + conn.commit() + + source_count = conn.execute( + "SELECT COUNT(*) FROM reflection_embeddings WHERE reflection_id = ?", + ("src",), + ).fetchone()[0] + assert source_count == 0 + # Target's embedding is untouched; no new embed call was made. + assert fake.calls == [] + target_count = conn.execute( + "SELECT COUNT(*) FROM reflection_embeddings WHERE reflection_id = ?", + ("tgt",), + ).fetchone()[0] + assert target_count == 1 From a9e405b0058d047233f4f2de8f38e6b44e6ab2bf Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 09:17:54 +0100 Subject: [PATCH 11/20] feat(embeddings): semantic memories embedded on create/update via SyncEmbedder Co-Authored-By: Claude Fable 5 --- better_memory/mcp/server.py | 2 +- better_memory/services/semantic.py | 28 +++++++ better_memory/storage/sqlite.py | 2 +- .../services/test_semantic_embedding_write.py | 81 +++++++++++++++++++ 4 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 tests/services/test_semantic_embedding_write.py diff --git a/better_memory/mcp/server.py b/better_memory/mcp/server.py index 6713e51..72c241b 100644 --- a/better_memory/mcp/server.py +++ b/better_memory/mcp/server.py @@ -231,7 +231,7 @@ def create_server() -> tuple[ reflections = ReflectionSynthesisService(memory_conn, sync_embedder=sync_embedder) retention = RetentionService(conn=memory_conn) memory_rating = MemoryRatingService(memory_conn) - semantic = SemanticMemoryService(memory_conn) + semantic = SemanticMemoryService(memory_conn, sync_embedder=sync_embedder) session_bootstrap = SessionBootstrapService(memory_conn) knowledge = KnowledgeService( diff --git a/better_memory/services/semantic.py b/better_memory/services/semantic.py index 9c63af5..32409c3 100644 --- a/better_memory/services/semantic.py +++ b/better_memory/services/semantic.py @@ -15,7 +15,10 @@ from datetime import datetime from uuid import uuid4 +import sqlite_vec + from better_memory._common import default_clock, env_session_id +from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.services.scoring import wilson_lower_bound @@ -54,9 +57,25 @@ def __init__( conn: sqlite3.Connection, *, clock: Callable[[], datetime] | None = None, + sync_embedder: SyncEmbedder | None = None, ) -> None: self._conn = conn self._clock: Callable[[], datetime] = clock or default_clock + self._sync_embedder = sync_embedder + + def _store_embedding(self, memory_id: str, vector: list[float] | None) -> None: + if vector is None: + return + # DELETE+INSERT: vec0 tables historically mishandle UPSERT. + self._conn.execute( + "DELETE FROM semantic_embeddings WHERE memory_id = ?", + (memory_id,), + ) + self._conn.execute( + "INSERT INTO semantic_embeddings (memory_id, embedding) " + "VALUES (?, ?)", + (memory_id, sqlite_vec.serialize_float32(vector)), + ) def create( self, *, content: str, project: str, scope: str = "project" @@ -77,6 +96,9 @@ def create( """, (memory_id, content, project, scope, now, now), ) + if self._sync_embedder is not None: + self._store_embedding( + memory_id, self._sync_embedder.embed_text(content)) self._conn.commit() return memory_id @@ -96,6 +118,9 @@ def update_text(self, *, id: str, content: str) -> None: # ObservationService.set_outcome (better_memory/services/observation.py:435). self._conn.rollback() raise ValueError(f"semantic memory not found: {id}") + if self._sync_embedder is not None: + self._store_embedding( + id, self._sync_embedder.embed_text(content)) self._conn.commit() def set_scope(self, *, id: str, scope: str) -> None: @@ -173,6 +198,9 @@ def create_from_observation( "WHERE id = ?", (now, observation_id), ) + if self._sync_embedder is not None: + self._store_embedding( + memory_id, self._sync_embedder.embed_text(row["content"])) except BaseException: self._conn.execute("ROLLBACK TO SAVEPOINT promote_observation") self._conn.execute("RELEASE SAVEPOINT promote_observation") diff --git a/better_memory/storage/sqlite.py b/better_memory/storage/sqlite.py index e824a29..11002ed 100644 --- a/better_memory/storage/sqlite.py +++ b/better_memory/storage/sqlite.py @@ -63,7 +63,6 @@ def __init__( project_resolver=self._project_resolver, episodes=self._episodes, ) - self._semantic = SemanticMemoryService(memory_conn) self._reflection = ReflectionService(memory_conn) self._memory_rating = MemoryRatingService(memory_conn) self._session_bootstrap = SessionBootstrapService(memory_conn) @@ -71,6 +70,7 @@ def __init__( SyncEmbedder(lambda: OllamaEmbedder(timeout=5.0, max_retries=1)) if embedder is not None else None ) + self._semantic = SemanticMemoryService(memory_conn, sync_embedder=sync_embedder) self._synthesis = ReflectionSynthesisService( memory_conn, sync_embedder=sync_embedder ) diff --git a/tests/services/test_semantic_embedding_write.py b/tests/services/test_semantic_embedding_write.py new file mode 100644 index 0000000..8b5b40a --- /dev/null +++ b/tests/services/test_semantic_embedding_write.py @@ -0,0 +1,81 @@ +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.embeddings.sync_embed import SyncEmbedder +from better_memory.services.semantic import SemanticMemoryService +from tests.services._embedding_fakes import FakeEmbedder + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _vec_count(conn): + return conn.execute("SELECT COUNT(*) FROM semantic_embeddings").fetchone()[0] + + +def _seed_active_observation(conn, *, obs_id="o1", project="p1", + content="bug found", episode_id=None): + if episode_id is None: + episode_id = "ep-default" + conn.execute( + "INSERT OR IGNORE INTO episodes (id, project, started_at) VALUES " + "(?, ?, '2026-04-01T00:00:00+00:00')", + (episode_id, project), + ) + conn.execute( + "INSERT INTO observations (id, content, project, episode_id, status, " + "outcome, created_at, status_changed_at) VALUES " + "(?, ?, ?, ?, 'active', 'success', " + "'2026-05-04T12:00:00+00:00','2026-05-04T12:00:00+00:00')", + (obs_id, content, project, episode_id), + ) + conn.commit() + + +class TestSemanticEmbeddingWrite: + def test_create_embeds_content(self, conn): + fake = FakeEmbedder() + svc = SemanticMemoryService(conn, sync_embedder=SyncEmbedder(lambda: fake)) + svc.create(content="the fact", project="p") + assert _vec_count(conn) == 1 + assert fake.calls == ["the fact"] + + def test_update_text_reembeds(self, conn): + fake = FakeEmbedder() + svc = SemanticMemoryService(conn, sync_embedder=SyncEmbedder(lambda: fake)) + mid = svc.create(content="v1", project="p") + svc.update_text(id=mid, content="v2") + assert _vec_count(conn) == 1 # replaced, not duplicated + assert fake.calls == ["v1", "v2"] + + def test_failure_never_blocks_create(self, conn): + svc = SemanticMemoryService( + conn, sync_embedder=SyncEmbedder(lambda: FakeEmbedder(fail=True))) + mid = svc.create(content="the fact", project="p") + assert mid + assert _vec_count(conn) == 0 + + def test_no_embedder_no_rows(self, conn): + svc = SemanticMemoryService(conn) + svc.create(content="the fact", project="p") + assert _vec_count(conn) == 0 + + def test_promote_from_observation_embeds(self, conn): + _seed_active_observation(conn, obs_id="o1", content="rule text") + fake = FakeEmbedder() + svc = SemanticMemoryService(conn, sync_embedder=SyncEmbedder(lambda: fake)) + svc.create_from_observation(observation_id="o1") + assert _vec_count(conn) == 1 + assert fake.calls == ["rule text"] From 8836223a2ea46f766632c95e57763cfab96c6faa Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 09:34:13 +0100 Subject: [PATCH 12/20] fix(embeddings): wire sync_embedder into UI writes; re-embed on reflection edit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The management UI built ReflectionService and SemanticMemoryService without a sync_embedder, so UI-driven writes (edit reflection, create/update semantic memory) never populated the vec0 embeddings tables — only MCP/synthesis writes did, requiring a manual CLI backfill. - create_app() gains a sync_embedder kwarg (defaults to auto-detect from get_config().embeddings_backend, mirroring mcp/server.py and storage/sqlite.py); wired into ReflectionService and all SemanticMemoryService construction sites. - ReflectionService.update_text now re-embeds via a new module-level _write_reflection_embedding() (extracted from ReflectionSynthesisService._store_embedding so both classes share it) — previously an edit left a stale vector pointing at the old text. - tests/ui/conftest.py's client fixture pins BETTER_MEMORY_EMBEDDINGS_BACKEND=sqlite so ordinary route tests don't depend on a live Ollama instance (default backend is ollama); tests proving the wiring build their own app with an injected fake embedder. Co-Authored-By: Claude Fable 5 --- better_memory/services/reflection.py | 58 ++++++++--- better_memory/ui/app.py | 74 +++++++++++++-- .../test_reflection_embedding_write.py | 88 +++++++++++++++++ tests/ui/conftest.py | 14 ++- tests/ui/test_app.py | 58 +++++++++++ tests/ui/test_reflections.py | 51 ++++++++++ tests/ui/test_semantic.py | 95 +++++++++++++++++++ 7 files changed, 416 insertions(+), 22 deletions(-) diff --git a/better_memory/services/reflection.py b/better_memory/services/reflection.py index 13f7c5a..3c7516c 100644 --- a/better_memory/services/reflection.py +++ b/better_memory/services/reflection.py @@ -58,6 +58,32 @@ def _embedding_source_text(title: str, use_cases: str, hints: list[str]) -> str: return "\n".join([title, use_cases, *hints]) +def _write_reflection_embedding( + conn: sqlite3.Connection, + reflection_id: str, + vector: list[float] | None, +) -> None: + """DELETE+INSERT the vec0 row for one reflection's embedding. + + Shared by :class:`ReflectionSynthesisService` (synthesis writes) and + :class:`ReflectionService` (UI-driven ``update_text`` writes) so both + write paths keep ``reflection_embeddings`` in sync with the + reflection's current title/use_cases/hints. DELETE+INSERT rather than + UPSERT because vec0 tables historically mishandle UPSERT. + """ + if vector is None: + return + conn.execute( + "DELETE FROM reflection_embeddings WHERE reflection_id = ?", + (reflection_id,), + ) + conn.execute( + "INSERT INTO reflection_embeddings (reflection_id, embedding) " + "VALUES (?, ?)", + (reflection_id, sqlite_vec.serialize_float32(vector)), + ) + + def _later_ts(a: str | None, b: str | None) -> str | None: """Return the later of two ISO-8601 timestamps, treating NULL as -inf. @@ -347,18 +373,7 @@ def __init__( self._sync_embedder = sync_embedder def _store_embedding(self, reflection_id: str, vector: list[float] | None) -> None: - if vector is None: - return - # DELETE+INSERT: vec0 tables historically mishandle UPSERT. - self._conn.execute( - "DELETE FROM reflection_embeddings WHERE reflection_id = ?", - (reflection_id,), - ) - self._conn.execute( - "INSERT INTO reflection_embeddings (reflection_id, embedding) " - "VALUES (?, ?)", - (reflection_id, sqlite_vec.serialize_float32(vector)), - ) + _write_reflection_embedding(self._conn, reflection_id, vector) @staticmethod def _normalize_tech(tech: str | None) -> str | None: @@ -1530,9 +1545,11 @@ def __init__( conn: sqlite3.Connection, *, clock: Callable[[], datetime] | None = None, + sync_embedder: SyncEmbedder | None = None, ) -> None: self._conn = conn self._clock: Callable[[], datetime] = clock or default_clock + self._sync_embedder = sync_embedder def confirm(self, *, reflection_id: str) -> None: """pending_review → confirmed; no-op on confirmed; raise on retired/superseded.""" @@ -1595,6 +1612,12 @@ def update_text( Blocked on retired/superseded — once a reflection has left the active set, mutating its text would silently change the audit trail. + + Re-embeds when constructed with a ``sync_embedder``: the vector + indexes ``title + use_cases + hints`` (see + ``_embedding_source_text``), so editing use_cases/hints without + refreshing the vector would leave a stale embedding pointing at + superseded text. No-op when ``sync_embedder`` is ``None``. """ if not use_cases or not use_cases.strip(): raise ValueError("use_cases must not be empty") @@ -1607,7 +1630,8 @@ def update_text( if not hint_list: raise ValueError("hints must not be empty") row = self._conn.execute( - "SELECT status FROM reflections WHERE id = ?", (reflection_id,) + "SELECT title, status FROM reflections WHERE id = ?", + (reflection_id,), ).fetchone() if row is None: raise ValueError(f"Reflection not found: {reflection_id}") @@ -1622,6 +1646,14 @@ def update_text( "WHERE id = ?", (use_cases, json.dumps(hint_list), now, reflection_id), ) + if self._sync_embedder is not None: + _write_reflection_embedding( + self._conn, + reflection_id, + self._sync_embedder.embed_text(_embedding_source_text( + row["title"], use_cases, hint_list, + )), + ) self._conn.commit() def promote_to_general(self, *, reflection_id: str) -> None: diff --git a/better_memory/ui/app.py b/better_memory/ui/app.py index 8f50391..c508655 100644 --- a/better_memory/ui/app.py +++ b/better_memory/ui/app.py @@ -12,19 +12,48 @@ from markupsafe import escape from werkzeug.wrappers import Response -from better_memory.config import project_name, resolve_home +from better_memory.config import get_config, project_name, resolve_home from better_memory.db.connection import connect +from better_memory.embeddings.ollama import OllamaEmbedder +from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.services.episode import EpisodeService from better_memory.services.reflection import ReflectionService from better_memory.ui import queries +class _Unset: + """Sentinel type distinguishing "caller did not pass sync_embedder" + (auto-detect from config) from an explicit ``sync_embedder=None`` + (force-disable, e.g. in tests that don't want any embedding side + effects). A dedicated class (rather than a bare ``object()``) lets + pyright narrow the parameter type via ``isinstance`` in create_app. + """ + + __slots__ = () + + +_UNSET = _Unset() + + +def _build_sync_embedder() -> SyncEmbedder | None: + """Build the shared SyncEmbedder used by every write-path service. + + Only wired for the ollama embeddings backend — sqlite (FTS5) indexes + via DB triggers instead of a Python embedder (mirrors the same gate + in better_memory/mcp/server.py and better_memory/storage/sqlite.py). + """ + if get_config().embeddings_backend != "ollama": + return None + return SyncEmbedder(lambda: OllamaEmbedder(timeout=5.0, max_retries=1)) + + def create_app( *, inactivity_timeout: float = 1800.0, inactivity_poll_interval: float = 30.0, start_watchdog: bool = True, db_path: Path | None = None, + sync_embedder: SyncEmbedder | None | _Unset = _UNSET, ) -> Flask: """Build and return a configured Flask app. @@ -39,6 +68,14 @@ def create_app( If ``False``, skip starting the background watchdog thread. ``_check_idle`` is still registered so tests can drive it synchronously without spawning threads. + sync_embedder: + Shared :class:`SyncEmbedder` passed to every UI write-path + service (``ReflectionService``, ``SemanticMemoryService``) so + UI-driven writes embed exactly like MCP-driven writes. Defaults + to auto-detecting from ``get_config().embeddings_backend`` via + :func:`_build_sync_embedder`. Pass explicitly (including + ``None``) to override — tests use this to inject a fake + embedder or to force-disable embedding. """ app = Flask(__name__) @@ -46,9 +83,18 @@ def create_app( resolved_db = db_path if db_path is not None else resolve_home() / "memory.db" db_conn = connect(resolved_db) + resolved_sync_embedder: SyncEmbedder | None = ( + _build_sync_embedder() + if isinstance(sync_embedder, _Unset) + else sync_embedder + ) + app.extensions["db_connection"] = db_conn app.extensions["episode_service"] = EpisodeService(conn=db_conn) - app.extensions["reflection_service"] = ReflectionService(conn=db_conn) + app.extensions["reflection_service"] = ReflectionService( + conn=db_conn, sync_embedder=resolved_sync_embedder, + ) + app.extensions["sync_embedder"] = resolved_sync_embedder app.extensions["db_path"] = resolved_db @app.teardown_appcontext @@ -417,7 +463,9 @@ def semantic_panel() -> str: if scope_filter not in ("project", "general", None): scope_filter = None search = (request.args.get("search") or "").strip() or None - svc = SemanticMemoryService(conn) + svc = SemanticMemoryService( + conn, sync_embedder=app.extensions["sync_embedder"], + ) rows = svc.list_for_project( project=project, scope_filter=scope_filter, search=search, ) @@ -432,7 +480,9 @@ def semantic_create() -> tuple[str, int, dict[str, str]]: project = project_name() content = request.form.get("content", "").strip() scope = request.form.get("scope") or "project" - svc = SemanticMemoryService(conn) + svc = SemanticMemoryService( + conn, sync_embedder=app.extensions["sync_embedder"], + ) try: svc.create(content=content, project=project, scope=scope) except ValueError as exc: @@ -447,7 +497,9 @@ def semantic_scope(id: str) -> tuple[str, int, dict[str, str]]: from better_memory.services.semantic import SemanticMemoryService conn = app.extensions["db_connection"] scope = request.form.get("scope") or "project" - svc = SemanticMemoryService(conn) + svc = SemanticMemoryService( + conn, sync_embedder=app.extensions["sync_embedder"], + ) try: svc.set_scope(id=id, scope=scope) except ValueError as exc: @@ -461,7 +513,9 @@ def semantic_scope(id: str) -> tuple[str, int, dict[str, str]]: def semantic_delete(id: str) -> tuple[str, int, dict[str, str]]: from better_memory.services.semantic import SemanticMemoryService conn = app.extensions["db_connection"] - svc = SemanticMemoryService(conn) + svc = SemanticMemoryService( + conn, sync_embedder=app.extensions["sync_embedder"], + ) svc.delete(id=id) # idempotent return ("", 200, {"HX-Trigger": "semantic-changed"}) @@ -497,7 +551,9 @@ def semantic_update(id: str) -> tuple[str, int, dict[str, str]]: from better_memory.services.semantic import SemanticMemoryService conn = app.extensions["db_connection"] content = request.form.get("content", "").strip() - svc = SemanticMemoryService(conn) + svc = SemanticMemoryService( + conn, sync_embedder=app.extensions["sync_embedder"], + ) try: svc.update_text(id=id, content=content) except ValueError as exc: @@ -560,7 +616,9 @@ def observation_promote_to_semantic( from markupsafe import escape conn = app.extensions["db_connection"] scope = request.form.get("scope") or "project" - svc = SemanticMemoryService(conn) + svc = SemanticMemoryService( + conn, sync_embedder=app.extensions["sync_embedder"], + ) try: svc.create_from_observation(observation_id=id, scope=scope) except ValueError as exc: diff --git a/tests/services/test_reflection_embedding_write.py b/tests/services/test_reflection_embedding_write.py index 91abdb3..35cabea 100644 --- a/tests/services/test_reflection_embedding_write.py +++ b/tests/services/test_reflection_embedding_write.py @@ -18,6 +18,7 @@ AugmentAction, MergeAction, NewAction, + ReflectionService, ReflectionSynthesisService, _embedding_source_text, ) @@ -234,6 +235,93 @@ def test_reembeds_on_augment(self, conn, fixed_clock): assert "new-hint" in fake.calls[0] +class TestReflectionServiceUpdateTextEmbedding: + """ReflectionService.update_text is the UI-facing edit path. + + Editing use_cases/hints changes the discriminating text a stored + vector indexes (title + use_cases + hints, see + `_embedding_source_text`); without a re-embed here the vector goes + stale the moment the UI edits a reflection. + """ + + def test_update_text_reembeds(self, conn, fixed_clock): + _insert_reflection( + conn, refl_id="r1", project="p", + title="Existing", use_cases="old uc", hints='["old-hint"]', + ) + conn.commit() + + fake = FakeEmbedder() + svc = ReflectionService( + conn, clock=fixed_clock, sync_embedder=SyncEmbedder(lambda: fake), + ) + svc.update_text(reflection_id="r1", use_cases="new uc", hints="new-hint") + + assert _vec_count(conn) == 1 + assert len(fake.calls) == 1 + assert fake.calls[0] == _embedding_source_text( + "Existing", "new uc", ["new-hint"] + ) + + def test_update_text_replaces_not_duplicates_vec_row(self, conn, fixed_clock): + _insert_reflection( + conn, refl_id="r1", project="p", + title="Existing", use_cases="old uc", hints='["old-hint"]', + ) + conn.commit() + import sqlite_vec + conn.execute( + "INSERT INTO reflection_embeddings (reflection_id, embedding) " + "VALUES (?, ?)", + ("r1", sqlite_vec.serialize_float32([0.0] * 768)), + ) + conn.commit() + assert _vec_count(conn) == 1 + + fake = FakeEmbedder() + svc = ReflectionService( + conn, clock=fixed_clock, sync_embedder=SyncEmbedder(lambda: fake), + ) + svc.update_text(reflection_id="r1", use_cases="new uc", hints="new-hint") + + assert _vec_count(conn) == 1 # replaced, not duplicated + + def test_update_text_works_without_sync_embedder(self, conn, fixed_clock): + _insert_reflection( + conn, refl_id="r1", project="p", + title="Existing", use_cases="old uc", hints='["old-hint"]', + ) + conn.commit() + + svc = ReflectionService(conn, clock=fixed_clock) # no sync_embedder + svc.update_text(reflection_id="r1", use_cases="new uc", hints="new-hint") + + row = conn.execute( + "SELECT use_cases FROM reflections WHERE id = 'r1'" + ).fetchone() + assert row["use_cases"] == "new uc" + assert _vec_count(conn) == 0 + + def test_no_embed_row_when_embedder_fails(self, conn, fixed_clock): + _insert_reflection( + conn, refl_id="r1", project="p", + title="Existing", use_cases="old uc", hints='["old-hint"]', + ) + conn.commit() + + svc = ReflectionService( + conn, clock=fixed_clock, + sync_embedder=SyncEmbedder(lambda: FakeEmbedder(fail=True)), + ) + svc.update_text(reflection_id="r1", use_cases="new uc", hints="new-hint") + + row = conn.execute( + "SELECT use_cases FROM reflections WHERE id = 'r1'" + ).fetchone() + assert row["use_cases"] == "new uc" + assert _vec_count(conn) == 0 + + class TestApplyMergeEmbedding: def test_merge_deletes_source_embedding_no_reembed_of_target( self, conn, fixed_clock diff --git a/tests/ui/conftest.py b/tests/ui/conftest.py index 2e2117f..75943d6 100644 --- a/tests/ui/conftest.py +++ b/tests/ui/conftest.py @@ -27,13 +27,25 @@ def tmp_db(tmp_path: Path) -> Iterator[Path]: @pytest.fixture -def client(tmp_db: Path) -> Iterator[FlaskClient]: +def client( + tmp_db: Path, monkeypatch: pytest.MonkeyPatch, +) -> Iterator[FlaskClient]: """Yield a Flask test client backed by a migrated tmp DB. Patches ``threading.Timer`` for the lifetime of the fixture so ``TestOriginCheck`` POST-to-/shutdown tests don't fire the real 100 ms timer that calls ``os._exit`` and kills the pytest process. + + Pins ``BETTER_MEMORY_EMBEDDINGS_BACKEND=sqlite`` so ``create_app``'s + auto-detected ``sync_embedder`` (see ``_build_sync_embedder`` in + ``better_memory/ui/app.py``) resolves to ``None`` — the default + config backend is ``ollama``, and without this pin every route test + that writes a reflection/semantic memory would silently depend on a + live Ollama instance. Tests exercising the embedding wiring build + their own app via ``create_app(sync_embedder=...)`` instead of this + fixture. """ + monkeypatch.setenv("BETTER_MEMORY_EMBEDDINGS_BACKEND", "sqlite") app = create_app(start_watchdog=False, db_path=tmp_db) app.config["TESTING"] = True with patch("better_memory.ui.app.threading.Timer"): diff --git a/tests/ui/test_app.py b/tests/ui/test_app.py index fcd8da0..ff35646 100644 --- a/tests/ui/test_app.py +++ b/tests/ui/test_app.py @@ -7,6 +7,7 @@ from pathlib import Path from unittest.mock import patch +import pytest from flask.testing import FlaskClient from better_memory.ui.app import create_app @@ -23,6 +24,63 @@ def test_app_exposes_open_db_connection( assert row[0] == 0 +class TestSyncEmbedderWiring: + """UI-driven writes must embed like MCP-driven writes. + + Before this wiring, better_memory.ui.app constructed ReflectionService + and SemanticMemoryService with no sync_embedder at all, so UI edits/ + creates never populated reflection_embeddings / semantic_embeddings — + only a manual CLI backfill run would fix those rows up. create_app now + auto-builds a shared SyncEmbedder from get_config().embeddings_backend + (mirroring better_memory/mcp/server.py and better_memory/storage/ + sqlite.py), and accepts an explicit sync_embedder= override for tests. + """ + + def test_sync_embedder_none_when_backend_sqlite( + self, tmp_db: Path, monkeypatch: pytest.MonkeyPatch, + ) -> None: + monkeypatch.setenv("BETTER_MEMORY_EMBEDDINGS_BACKEND", "sqlite") + app = create_app(start_watchdog=False, db_path=tmp_db) + assert app.extensions["sync_embedder"] is None + + def test_sync_embedder_built_when_backend_ollama( + self, tmp_db: Path, monkeypatch: pytest.MonkeyPatch, + ) -> None: + from better_memory.embeddings.sync_embed import SyncEmbedder + + monkeypatch.setenv("BETTER_MEMORY_EMBEDDINGS_BACKEND", "ollama") + app = create_app(start_watchdog=False, db_path=tmp_db) + assert isinstance(app.extensions["sync_embedder"], SyncEmbedder) + + def test_explicit_sync_embedder_overrides_config_auto_detect( + self, tmp_db: Path, monkeypatch: pytest.MonkeyPatch, + ) -> None: + from better_memory.embeddings.sync_embed import SyncEmbedder + + monkeypatch.setenv("BETTER_MEMORY_EMBEDDINGS_BACKEND", "sqlite") + fake_sync_embedder = SyncEmbedder(lambda: None) + app = create_app( + start_watchdog=False, db_path=tmp_db, + sync_embedder=fake_sync_embedder, + ) + assert app.extensions["sync_embedder"] is fake_sync_embedder + + def test_reflection_service_receives_the_shared_sync_embedder( + self, tmp_db: Path, monkeypatch: pytest.MonkeyPatch, + ) -> None: + from better_memory.embeddings.sync_embed import SyncEmbedder + + fake_sync_embedder = SyncEmbedder(lambda: None) + app = create_app( + start_watchdog=False, db_path=tmp_db, + sync_embedder=fake_sync_embedder, + ) + assert ( + app.extensions["reflection_service"]._sync_embedder + is fake_sync_embedder + ) + + class TestHealthz: def test_returns_200_with_ok_body(self, client: FlaskClient) -> None: response = client.get("/healthz") diff --git a/tests/ui/test_reflections.py b/tests/ui/test_reflections.py index 0912aa3..84e8b29 100644 --- a/tests/ui/test_reflections.py +++ b/tests/ui/test_reflections.py @@ -380,6 +380,57 @@ def test_post_409_for_retired( assert response.status_code == 409 +class TestReflectionEditEmbeddingWiring: + """UI edit route must re-embed, not just the synthesis write path. + + The shared `client` fixture pins BETTER_MEMORY_EMBEDDINGS_BACKEND=sqlite + (see tests/ui/conftest.py) so ordinary route tests don't depend on a + live Ollama instance. These tests build their own app with an explicit + fake sync_embedder to prove the wiring — app.py passes + app.extensions["sync_embedder"] into ReflectionService's constructor, + and ReflectionService.update_text re-embeds title+use_cases+hints. + """ + + def test_edit_writes_reflection_embedding_row( + self, tmp_db: Path, monkeypatch: pytest.MonkeyPatch, + ) -> None: + from unittest.mock import patch as _patch + + from better_memory.embeddings.sync_embed import SyncEmbedder + from better_memory.ui import app as app_module + from better_memory.ui.app import create_app + from tests.services._embedding_fakes import FakeEmbedder + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", title="Existing") + + fake = FakeEmbedder() + app = create_app( + start_watchdog=False, db_path=tmp_db, + sync_embedder=SyncEmbedder(lambda: fake), + ) + app.config["TESTING"] = True + with _patch("better_memory.ui.app.threading.Timer"): + with app.test_client() as client: + response = client.post( + "/reflections/r-1/edit", + data={"use_cases": "new uc", "hints": "new h"}, + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 200 + + conn = connect(tmp_db) + try: + count = conn.execute( + "SELECT COUNT(*) FROM reflection_embeddings " + "WHERE reflection_id = 'r-1'" + ).fetchone()[0] + finally: + conn.close() + assert count == 1 + assert fake.calls == ["Existing\nnew uc\nnew h"] + + class TestReflectionPromote: def test_promotes_project_pending( self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch diff --git a/tests/ui/test_semantic.py b/tests/ui/test_semantic.py index af3351a..c3d7f9e 100644 --- a/tests/ui/test_semantic.py +++ b/tests/ui/test_semantic.py @@ -474,6 +474,101 @@ def test_overlooked_badge_ambered_when_positive( assert "rating-overlooked" in body +class TestSemanticEmbeddingWiring: + """UI create/update routes must embed, not just MCP-driven writes. + + The shared `client` fixture pins BETTER_MEMORY_EMBEDDINGS_BACKEND=sqlite + (see tests/ui/conftest.py) so ordinary route tests don't depend on a + live Ollama instance. These tests build their own app with an explicit + fake sync_embedder to prove the wiring — app.py passes + app.extensions["sync_embedder"] into every SemanticMemoryService(...) + construction site. + """ + + def _make_client(self, tmp_db: Path, fake_embedder): + from unittest.mock import patch as _patch + + from better_memory.embeddings.sync_embed import SyncEmbedder + from better_memory.ui.app import create_app + + app = create_app( + start_watchdog=False, db_path=tmp_db, + sync_embedder=SyncEmbedder(lambda: fake_embedder), + ) + app.config["TESTING"] = True + self._timer_patch = _patch("better_memory.ui.app.threading.Timer") + self._timer_patch.start() + return app.test_client() + + def _vec_count(self, tmp_db: Path) -> int: + # sqlite_vec's vec0 virtual table needs its extension loaded — a + # bare sqlite3.connect() raises "no such module: vec0". Use the + # project's connect() helper, which loads it. + from better_memory.db.connection import connect as _connect + + conn = _connect(tmp_db) + try: + return conn.execute( + "SELECT COUNT(*) FROM semantic_embeddings" + ).fetchone()[0] + finally: + conn.close() + + def test_create_writes_semantic_embedding_row( + self, tmp_db: Path, monkeypatch: pytest.MonkeyPatch, + ) -> None: + from better_memory.ui import app as app_module + from tests.services._embedding_fakes import FakeEmbedder + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + fake = FakeEmbedder() + client = self._make_client(tmp_db, fake) + try: + response = client.post( + "/semantic", + data={"content": "new rule", "scope": "general"}, + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 200 + finally: + self._timer_patch.stop() + + assert self._vec_count(tmp_db) == 1 + assert fake.calls == ["new rule"] + + def test_update_reembeds_semantic_memory( + self, tmp_db: Path, monkeypatch: pytest.MonkeyPatch, + ) -> None: + import sqlite3 + + from better_memory.ui import app as app_module + from tests.services._embedding_fakes import FakeEmbedder + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + with sqlite3.connect(tmp_db) as seed_conn: + seed_conn.execute( + "INSERT INTO semantic_memories " + "(id, content, project, scope, created_at, updated_at) VALUES " + "('m1','old text','proj-a','project'," + " '2026-05-01T10:00:00+00:00','2026-05-01T10:00:00+00:00')" + ) + seed_conn.commit() + + fake = FakeEmbedder() + client = self._make_client(tmp_db, fake) + try: + response = client.post( + "/semantic/m1/update", data={"content": "new text"}, + headers={"Origin": "http://localhost"}, + ) + assert response.status_code == 200 + finally: + self._timer_patch.stop() + + assert self._vec_count(tmp_db) == 1 # replaced, not duplicated + assert fake.calls == ["new text"] + + class TestSemanticUpdate: def test_update_changes_content( self, client: FlaskClient, tmp_db: Path, From 71dbebe28be1382a04f5eb1a287e05b56e5c7970 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 09:49:38 +0100 Subject: [PATCH 13/20] feat(retrieval): three-leg RRF fusion (prior + BM25 + vec) with lazy self-heal Co-Authored-By: Claude Fable 5 --- better_memory/services/reflection.py | 151 ++++++++++++++++++++------- tests/services/test_vec_fusion.py | 129 +++++++++++++++++++++++ 2 files changed, 245 insertions(+), 35 deletions(-) create mode 100644 tests/services/test_vec_fusion.py diff --git a/better_memory/services/reflection.py b/better_memory/services/reflection.py index 3c7516c..88ef62e 100644 --- a/better_memory/services/reflection.py +++ b/better_memory/services/reflection.py @@ -330,6 +330,10 @@ def _parse_merge(item: object) -> MergeAction: ) +#: Max embeddings written per retrieve call by the lazy self-heal. Keeps +#: worst-case added latency bounded; cli.backfill_embeddings is the bulk path. +SELF_HEAL_BATCH_CAP = 20 + #: A memory with fewer than this many rated exposures is "untested": its #: Wilson score is statistically meaningless, so it competes for the #: reserved exploration slot instead of the proven slots. @@ -1266,22 +1270,81 @@ def apply_decision( ) # --------------------------------------------------------- retrieve_reflections + def _heal_missing_embeddings(self, rows) -> None: + """Embed up to SELF_HEAL_BATCH_CAP candidates that lack vectors. + + Historical rows and write-time failures repair themselves on their + first relevant retrieval. Entirely best-effort; one embed_batch call. + """ + if self._sync_embedder is None or not rows: + return + ids = [r["id"] for r in rows] + placeholders = ",".join("?" for _ in ids) + have = { + row[0] for row in self._conn.execute( + f"SELECT reflection_id FROM reflection_embeddings " + f"WHERE reflection_id IN ({placeholders})", ids, + ) + } + todo = [r for r in rows if r["id"] not in have][:SELF_HEAL_BATCH_CAP] + if not todo: + return + texts = [ + _embedding_source_text(r["title"], r["use_cases"], + json.loads(r["hints"])) + for r in todo + ] + vectors = self._sync_embedder.embed_batch(texts) + if vectors is None: + return + for r, vec in zip(todo, vectors): + self._store_embedding(r["id"], vec) + self._conn.commit() + + def _vec_ranks(self, query_vector, candidate_ids) -> dict[str, int]: + """reflection_id -> vec rank (0 = closest) among the candidates. + + sqlite-vec kNN accepts only ``embedding MATCH ? AND k = ?`` — no + extra predicates — so fetch top-k then filter, exactly as + search/hybrid.py:_vec_candidates does. + """ + if query_vector is None or not candidate_ids: + return {} + try: + knn = self._conn.execute( + "SELECT reflection_id FROM reflection_embeddings " + "WHERE embedding MATCH ? AND k = ? ORDER BY distance", + (sqlite_vec.serialize_float32(query_vector), + max(len(candidate_ids), 50)), + ).fetchall() + except sqlite3.OperationalError: + return {} + wanted = set(candidate_ids) + out: dict[str, int] = {} + for row in knn: + if row[0] in wanted: + out[row[0]] = len(out) + return out + def _fuse_by_relevance( - self, rows: list, *, query: str, rrf_k: int = 60, + self, rows: list, *, query: str, query_vector=None, rrf_k: int = 60, ) -> list: - """Re-order ``rows`` by RRF fusion of popularity rank and BM25 rank. + """Re-order ``rows`` by RRF fusion of popularity rank, BM25 rank, and + vector rank. ``rows`` arrives in popularity order (``useful_count + 3*times_overlooked``, then confidence, then recency). We compute a second ranking over the same ids by BM25 relevance against ``reflection_fts`` (title / use_cases / - hints) and fuse the two with reciprocal rank fusion: + hints), and (when ``query_vector`` is available) a third ranking by + vector distance against ``reflection_embeddings``, then fuse all + available legs with reciprocal rank fusion: - score(d) = 1/(k + pop_rank(d)) + 1/(k + rel_rank(d)) + score(d) = 1/(k + pop_rank(d)) + 1/(k + rel_rank(d)) + 1/(k + vec_rank(d)) - matching :mod:`better_memory.search.hybrid`. Rows the query does not - match keep only the popularity term, so relevance *promotes* without - ever discarding — a query that matches nothing degrades exactly to the - previous behaviour. + matching :mod:`better_memory.search.hybrid`. Rows a given leg does not + match keep only the other terms, so relevance *promotes* without ever + discarding — a query that matches nothing on any leg degrades exactly + to the previous behaviour. Tokens are OR-ed rather than AND-ed: ``sanitize_fts5_query`` joins bare terms, which FTS5 reads as implicit AND, and a natural-language task @@ -1296,42 +1359,53 @@ def _fuse_by_relevance( t for t in sanitized.split() if len(t) > 2 and t.lower() not in _QUERY_STOPWORDS ] - if not tokens: - return rows - match_expr = " OR ".join(tokens) + rel_rows = [] + if tokens: + match_expr = " OR ".join(tokens) + placeholders = ",".join("?" for _ in ids) + try: + rel_rows = self._conn.execute( + f""" + SELECT r.id AS id, bm25(reflection_fts) AS bm + FROM reflection_fts + JOIN reflections r ON r.rowid = reflection_fts.rowid + WHERE reflection_fts MATCH ? AND r.id IN ({placeholders}) + ORDER BY bm ASC + """, + [match_expr, *ids], + ).fetchall() + except sqlite3.OperationalError: + # Malformed MATCH expression despite sanitising, or the FTS + # table is absent on an old DB. Relevance is an enhancement, + # never a hard dependency. + rel_rows = [] - placeholders = ",".join("?" for _ in ids) - try: - rel_rows = self._conn.execute( - f""" - SELECT r.id AS id, bm25(reflection_fts) AS bm - FROM reflection_fts - JOIN reflections r ON r.rowid = reflection_fts.rowid - WHERE reflection_fts MATCH ? AND r.id IN ({placeholders}) - ORDER BY bm ASC - """, - [match_expr, *ids], - ).fetchall() - except sqlite3.OperationalError: - # Malformed MATCH expression despite sanitising, or the FTS table - # is absent on an old DB. Relevance is an enhancement, never a - # hard dependency — fall back to the popularity order. - return rows + rel_rank = {r["id"]: i for i, r in enumerate(rel_rows)} + vec_rank = self._vec_ranks(query_vector, ids) - if not rel_rows: + if not rel_rank and not vec_rank: return rows - rel_rank = {r["id"]: i for i, r in enumerate(rel_rows)} scored = [] for pop_rank, row in enumerate(rows): score = 1.0 / (rrf_k + pop_rank) rr = rel_rank.get(row["id"]) if rr is not None: score += 1.0 / (rrf_k + rr) - scored.append((score, pop_rank, row)) - # pop_rank breaks ties deterministically and keeps the fusion stable. - scored.sort(key=lambda t: (-t[0], t[1])) - return [row for _, _, row in scored] + vr = vec_rank.get(row["id"]) + if vr is not None: + score += 1.0 / (rrf_k + vr) + # RRF sums are symmetric: a row with (pop_rank=a, vec_rank=b) ties + # exactly with one at (pop_rank=b, vec_rank=a). Break such ties + # toward the closer semantic match (lower vec_rank) before + # falling back to pop_rank — otherwise a strong vec-only match + # (this method's whole point) can lose a coin-flip tie to a + # popularity-only row. vec_rank absent (no embedder/no vec leg) + # collapses to sys.maxsize for every row, so the tiebreak is a + # no-op and behaviour matches the shipped two-leg fusion exactly. + scored.append((score, vr if vr is not None else sys.maxsize, pop_rank, row)) + scored.sort(key=lambda t: (-t[0], t[1], t[2])) + return [row for _, _, _, row in scored] def _bucket_item(self, r) -> dict: """Convert one reflections row into the dict shape returned to callers.""" @@ -1432,7 +1506,14 @@ def retrieve_reflections( rows.sort(key=_wilson_score, reverse=True) if query: - rows = self._fuse_by_relevance(rows, query=query) + self._heal_missing_embeddings(rows) + query_vector = ( + self._sync_embedder.embed_text(query) + if self._sync_embedder is not None else None + ) + rows = self._fuse_by_relevance( + rows, query=query, query_vector=query_vector, + ) _diag.step(fn, "relevance_fused", n_rows=len(rows)) # Convert None (unlimited) to sys.maxsize so the loop body has a definite int. diff --git a/tests/services/test_vec_fusion.py b/tests/services/test_vec_fusion.py new file mode 100644 index 0000000..787e870 --- /dev/null +++ b/tests/services/test_vec_fusion.py @@ -0,0 +1,129 @@ +"""Query fusion gains a vector leg; missing embeddings self-heal on retrieve. + +Degradation contract: no embedder / breaker open / row unembedded -> exactly +the two-leg (prior + BM25) behaviour that shipped in #81. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.embeddings.sync_embed import SyncEmbedder +from better_memory.services.reflection import ReflectionSynthesisService +from tests.services._embedding_fakes import FakeEmbedder + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed(conn, rid, *, title, useful=0, ignored=0): + conn.execute( + """INSERT INTO reflections + (id, title, project, phase, polarity, use_cases, hints, + confidence, created_at, updated_at, useful_count, times_ignored) + VALUES (?, ?, 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01', ?, ?)""", + (rid, title, useful, ignored), + ) + conn.commit() + + +class DirectedEmbedder(FakeEmbedder): + """Maps texts containing any trigger phrase to one vector; noise else. + + Lets a test make the query and one reflection 'semantically identical' + while sharing zero tokens — isolating the vec leg from BM25. + """ + + def __init__(self, *triggers: str): + super().__init__() + self.triggers = triggers + + def _vec(self, text: str) -> list[float]: + if any(t in text for t in self.triggers): + return [1.0] + [0.0] * 767 + return [0.0, 1.0] + [0.0] * 766 + + async def embed(self, text): + self.calls.append(text) + return self._vec(text) + + async def embed_batch(self, texts): + self.calls.append(list(texts)) + return [self._vec(t) for t in texts] + + +def _svc(conn, embedder): + return ReflectionSynthesisService( + conn, sync_embedder=SyncEmbedder(lambda: embedder)) + + +class TestVecFusion: + def test_semantic_match_promoted_without_token_overlap(self, conn): + _seed(conn, "r-target", title="Stdout handling on win32 interpreters") + _seed(conn, "r-noise-1", title="Unrelated advice alpha", useful=5, ignored=1) + _seed(conn, "r-noise-2", title="Unrelated advice beta", useful=4, ignored=1) + emb = DirectedEmbedder("Stdout handling", "console output") + svc = _svc(conn, emb) + ids = [r["id"] for r in svc.retrieve_reflections( + project="p", query="console output disappears on windows", + )["do"]] + assert ids[0] == "r-target" + + def test_no_embedder_matches_shipped_behaviour(self, conn): + _seed(conn, "r-a", title="Retention thresholds", useful=1) + svc = ReflectionSynthesisService(conn) + ids = [r["id"] for r in svc.retrieve_reflections( + project="p", query="retention")["do"]] + assert ids == ["r-a"] + + def test_embedder_failure_degrades_silently(self, conn): + _seed(conn, "r-a", title="Retention thresholds", useful=1) + svc = _svc(conn, FakeEmbedder(fail=True)) + ids = [r["id"] for r in svc.retrieve_reflections( + project="p", query="retention")["do"]] + assert ids == ["r-a"] + + +class TestSelfHeal: + def test_unembedded_candidates_healed_on_query_retrieve(self, conn): + _seed(conn, "r-a", title="Alpha") + _seed(conn, "r-b", title="Beta") + svc = _svc(conn, FakeEmbedder()) + svc.retrieve_reflections(project="p", query="anything at all") + n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] + assert n == 2 + + def test_heal_capped_at_batch_limit(self, conn): + from better_memory.services.reflection import SELF_HEAL_BATCH_CAP + for i in range(SELF_HEAL_BATCH_CAP + 5): + _seed(conn, f"r-{i:03}", title=f"Title {i}") + svc = _svc(conn, FakeEmbedder()) + svc.retrieve_reflections(project="p", query="anything") + n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] + assert n == SELF_HEAL_BATCH_CAP + + def test_no_query_no_heal_and_no_embed_calls(self, conn): + _seed(conn, "r-a", title="Alpha") + fake = FakeEmbedder() + svc = _svc(conn, fake) + svc.retrieve_reflections(project="p") + n = conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] + assert n == 0 + assert fake.calls == [] + + def test_heal_failure_silent(self, conn): + _seed(conn, "r-a", title="Alpha") + svc = _svc(conn, FakeEmbedder(fail=True)) + rows = svc.retrieve_reflections(project="p", query="anything") + assert rows["do"] From 8a82dd35d369a8fcd0cbba36025ba1d4adf06383 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 09:56:35 +0100 Subject: [PATCH 14/20] test(retrieval): document vec tie-break fragility, pin multi-row degrade order Co-Authored-By: Claude Fable 5 --- tests/services/test_vec_fusion.py | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/services/test_vec_fusion.py b/tests/services/test_vec_fusion.py index 787e870..de61eb5 100644 --- a/tests/services/test_vec_fusion.py +++ b/tests/services/test_vec_fusion.py @@ -70,6 +70,23 @@ def _svc(conn, embedder): class TestVecFusion: def test_semantic_match_promoted_without_token_overlap(self, conn): + # Fragility note: r-noise-1 and r-noise-2 both fall through + # DirectedEmbedder's else-branch, so they embed to the EXACT SAME + # noise vector. Against the query vector, r-target sits at distance + # 0 (unambiguous winner) but the two noise rows tie at an equal, + # larger distance. sqlite-vec's ordering among equal-distance rows + # is unspecified/undocumented (not necessarily insertion order — see + # task-9-report.md's "Deviation from the brief" section, which + # walked through the exact tie this creates in the RRF sum: + # r-target's (pop_rank=2, vec_rank=0) is a swap of whichever noise + # row lands at (pop_rank=0, vec_rank=2), and RRF's sum is symmetric, + # so those two rows score EXACTLY equal). The vec_rank secondary + # sort key in _fuse_by_relevance is what breaks that tie toward + # r-target regardless of which noise row sqlite-vec puts first. + # If this test starts failing after a sqlite-vec upgrade, that tie + # order is the first thing to check — the fusion logic is likely + # still correct; a sqlite-vec version bump changing its internal + # equal-distance ordering would not, by itself, indicate a bug here. _seed(conn, "r-target", title="Stdout handling on win32 interpreters") _seed(conn, "r-noise-1", title="Unrelated advice alpha", useful=5, ignored=1) _seed(conn, "r-noise-2", title="Unrelated advice beta", useful=4, ignored=1) @@ -94,6 +111,36 @@ def test_embedder_failure_degrades_silently(self, conn): project="p", query="retention")["do"]] assert ids == ["r-a"] + def test_multi_row_no_vec_leg_matches_two_leg_order(self, conn): + """vr=sys.maxsize as a no-op, proven across 3+ rows, not just one. + + The single-row degrade tests above can't catch a tiebreak bug: with + one row there's no order to get wrong. Here three rows with distinct + popularity and a query that matches all of them via BM25 forces the + scored/sort path (rel_rank is non-empty, so _fuse_by_relevance does + not take its early "nothing matched" bail-out) while the vec leg is + absent — proving the added vec_rank secondary sort key doesn't + reorder anything when there's no vector leg to rank by. + + Comparing against a plain ``sync_embedder=None`` service (the + untouched shipped two-leg path) rather than a hand-computed order + keeps this robust to internal BM25/Wilson scoring details. + """ + _seed(conn, "r-hi", title="Retention thresholds alpha", useful=10) + _seed(conn, "r-mid", title="Retention thresholds beta", useful=5) + _seed(conn, "r-lo", title="Retention thresholds gamma", useful=1) + + no_embedder_svc = ReflectionSynthesisService(conn) + expected = [r["id"] for r in no_embedder_svc.retrieve_reflections( + project="p", query="retention thresholds")["do"]] + + failing_svc = _svc(conn, FakeEmbedder(fail=True)) + ids = [r["id"] for r in failing_svc.retrieve_reflections( + project="p", query="retention thresholds")["do"]] + + assert len(expected) >= 3 + assert ids == expected + class TestSelfHeal: def test_unembedded_candidates_healed_on_query_retrieve(self, conn): From c5da27071b273307efc1b7ae09131d23ea4f74a5 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 10:01:42 +0100 Subject: [PATCH 15/20] feat(cli): one-shot embedding backfill for reflections + semantics Co-Authored-By: Claude Fable 5 --- better_memory/cli/backfill_embeddings.py | 112 +++++++++++++++++++++++ tests/cli/test_backfill_embeddings.py | 66 +++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 better_memory/cli/backfill_embeddings.py create mode 100644 tests/cli/test_backfill_embeddings.py diff --git a/better_memory/cli/backfill_embeddings.py b/better_memory/cli/backfill_embeddings.py new file mode 100644 index 0000000..6baf50d --- /dev/null +++ b/better_memory/cli/backfill_embeddings.py @@ -0,0 +1,112 @@ +"""One-shot embedding backfill for reflections + semantic memories. + +Run at deploy after migration 0014: + + python -m better_memory.cli.backfill_embeddings + +Idempotent: only rows missing a vector are embedded. The lazy self-heal in +memory.retrieve covers stragglers afterwards; this exists so the historical +corpus doesn't wait to be retrieved before becoming searchable. + +One event loop and one embedder for the whole job (the embedder's +httpx.AsyncClient is loop-bound); batches of 50 per HTTP request. +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import sys +from pathlib import Path + +import sqlite_vec + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.reflection import _embedding_source_text + +_BATCH = 50 + + +def backfill(conn, embedder) -> dict[str, int]: + stats = {"reflections": 0, "semantics": 0, "skipped": 0} + + refl = conn.execute( + """SELECT r.id, r.title, r.use_cases, r.hints FROM reflections r + WHERE r.status IN ('pending_review', 'confirmed') + AND r.id NOT IN (SELECT reflection_id FROM reflection_embeddings)""" + ).fetchall() + sems = conn.execute( + """SELECT id, content FROM semantic_memories + WHERE id NOT IN (SELECT memory_id FROM semantic_embeddings)""" + ).fetchall() + + jobs = [ + ("reflections", "reflection_embeddings", "reflection_id", r["id"], + _embedding_source_text(r["title"], r["use_cases"], + json.loads(r["hints"]))) + for r in refl + ] + [ + ("semantics", "semantic_embeddings", "memory_id", s["id"], s["content"]) + for s in sems + ] + + async def _embed_all() -> list[list[list[float]] | None]: + out = [] + for i in range(0, len(jobs), _BATCH): + texts = [j[4] for j in jobs[i:i + _BATCH]] + try: + out.append(await embedder.embed_batch(texts)) + except Exception: + out.append(None) + return out + + batches = asyncio.run(_embed_all()) if jobs else [] + + for bi, vectors in enumerate(batches): + chunk = jobs[bi * _BATCH:(bi + 1) * _BATCH] + if vectors is None: + stats["skipped"] += len(chunk) + continue + for (kind, table, col, row_id, _), vec in zip(chunk, vectors): + conn.execute( + f"INSERT INTO {table} ({col}, embedding) VALUES (?, ?)", + (row_id, sqlite_vec.serialize_float32(vec))) + stats[kind] += 1 + conn.commit() + return stats + + +def main(argv: list[str] | None = None) -> None: + from better_memory.config import get_config + from better_memory.embeddings.ollama import OllamaEmbedder + + ap = argparse.ArgumentParser() + ap.add_argument("--home", default=None, + help="BETTER_MEMORY_HOME override (default: config)") + args = ap.parse_args(argv) + + config = get_config() + db = Path(args.home) / "memory.db" if args.home else config.memory_db + conn = connect(db) + apply_migrations(conn) + + if config.embeddings_backend != "ollama": + print("embeddings backend is not ollama; nothing to backfill") + return + + embedder = OllamaEmbedder() + try: + stats = backfill(conn, embedder) + finally: + asyncio.run(embedder.aclose()) + print(f"backfilled reflections={stats['reflections']} " + f"semantics={stats['semantics']} skipped={stats['skipped']}") + if stats["skipped"]: + print("warning: some rows skipped (Ollama unreachable?); " + "re-run later or let retrieve self-heal them", file=sys.stderr) + + +if __name__ == "__main__": + main() diff --git a/tests/cli/test_backfill_embeddings.py b/tests/cli/test_backfill_embeddings.py new file mode 100644 index 0000000..81aae8f --- /dev/null +++ b/tests/cli/test_backfill_embeddings.py @@ -0,0 +1,66 @@ +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.cli.backfill_embeddings import backfill +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from tests.services._embedding_fakes import FakeEmbedder + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed_reflection(conn, rid, status="pending_review"): + conn.execute( + """INSERT INTO reflections (id, title, project, phase, polarity, + use_cases, hints, confidence, created_at, updated_at, status) + VALUES (?, 'T', 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01', ?)""", (rid, status)) + conn.commit() + + +def _seed_semantic(conn, sid): + conn.execute( + """INSERT INTO semantic_memories (id, content, project, scope, + created_at, updated_at) + VALUES (?, 'fact', 'p', 'project', '2026-01-01', '2026-01-01')""", + (sid,)) + conn.commit() + + +def test_backfills_reflections_and_semantics(conn): + _seed_reflection(conn, "r1") + _seed_semantic(conn, "s1") + stats = backfill(conn, FakeEmbedder()) + assert stats == {"reflections": 1, "semantics": 1, "skipped": 0} + assert conn.execute("SELECT COUNT(*) FROM reflection_embeddings").fetchone()[0] == 1 + assert conn.execute("SELECT COUNT(*) FROM semantic_embeddings").fetchone()[0] == 1 + + +def test_idempotent(conn): + _seed_reflection(conn, "r1") + fake = FakeEmbedder() + backfill(conn, fake) + assert backfill(conn, fake) == {"reflections": 0, "semantics": 0, "skipped": 0} + + +def test_retired_reflections_skipped(conn): + _seed_reflection(conn, "r1", status="retired") + assert backfill(conn, FakeEmbedder()) == { + "reflections": 0, "semantics": 0, "skipped": 0} + + +def test_embed_failure_counted_as_skipped(conn): + _seed_reflection(conn, "r1") + stats = backfill(conn, FakeEmbedder(fail=True)) + assert stats == {"reflections": 0, "semantics": 0, "skipped": 1} From 277c0f4c52ba88d93bd7205881cee1bbf9ab96c9 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 10:04:57 +0100 Subject: [PATCH 16/20] fix(cli): best-effort teardown in backfill_embeddings --- better_memory/cli/backfill_embeddings.py | 32 ++++++++++++++---------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/better_memory/cli/backfill_embeddings.py b/better_memory/cli/backfill_embeddings.py index 6baf50d..93d1b66 100644 --- a/better_memory/cli/backfill_embeddings.py +++ b/better_memory/cli/backfill_embeddings.py @@ -90,22 +90,28 @@ def main(argv: list[str] | None = None) -> None: config = get_config() db = Path(args.home) / "memory.db" if args.home else config.memory_db conn = connect(db) - apply_migrations(conn) + try: + apply_migrations(conn) - if config.embeddings_backend != "ollama": - print("embeddings backend is not ollama; nothing to backfill") - return + if config.embeddings_backend != "ollama": + print("embeddings backend is not ollama; nothing to backfill") + return - embedder = OllamaEmbedder() - try: - stats = backfill(conn, embedder) + embedder = OllamaEmbedder() + try: + stats = backfill(conn, embedder) + finally: + try: + asyncio.run(embedder.aclose()) + except Exception: + pass + print(f"backfilled reflections={stats['reflections']} " + f"semantics={stats['semantics']} skipped={stats['skipped']}") + if stats["skipped"]: + print("warning: some rows skipped (Ollama unreachable?); " + "re-run later or let retrieve self-heal them", file=sys.stderr) finally: - asyncio.run(embedder.aclose()) - print(f"backfilled reflections={stats['reflections']} " - f"semantics={stats['semantics']} skipped={stats['skipped']}") - if stats["skipped"]: - print("warning: some rows skipped (Ollama unreachable?); " - "re-run later or let retrieve self-heal them", file=sys.stderr) + conn.close() if __name__ == "__main__": From 6752c86a2b0a275cfeddd89255dc6ad93f0d6911 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 19:35:42 +0100 Subject: [PATCH 17/20] docs(website): ranking + embeddings prose matches PR-A behaviour Co-Authored-By: Claude Fable 5 --- website/architecture.md | 53 ++++++++++++++++++++++++++++++++++++---- website/configuration.md | 2 +- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/website/architecture.md b/website/architecture.md index d87e9c7..c53a21c 100644 --- a/website/architecture.md +++ b/website/architecture.md @@ -101,6 +101,35 @@ that were written while `sqlite` was active (no embedding was computed for them); a future `memory.reindex` MCP tool can backfill if that half-state becomes a problem. +### Reflection and semantic-memory embeddings + +Reflections and semantic memories get their own vectors, written at +write time rather than lazily at query time: `ReflectionSynthesisService` +embeds a reflection when synthesis creates or augments it, `ReflectionService` +re-embeds on `update_text`, and the semantic-memory service does the +same on `memory.semantic_observe` and its update path. All of these go +through `SyncEmbedder` (`better_memory/embeddings/sync_embed.py`), a +synchronous facade over the Ollama embedder that is best-effort by +design: any failure opens a 60-second circuit breaker (`_DEFAULT_COOLDOWN`) +during which every embed call returns `None` immediately instead of +retrying against a dead Ollama. A missing vector is never treated as an +error - callers just drop the vector leg from ranking and fall back to +the Wilson + BM25 order above. + +Two mechanisms cover rows left without a vector - historical rows +predating this feature, or rows written during a breaker outage: + +- **Lazy self-heal on retrieve.** Each `memory.retrieve` call embeds up + to 20 reflections missing a vector before ranking (`SELF_HEAL_BATCH_CAP` + in `better_memory/services/reflection.py`) - capped so a cold corpus + can't turn one retrieve call into an unbounded batch of Ollama calls. +- **`python -m better_memory.cli.backfill_embeddings`.** A one-shot, + idempotent CLI that embeds every reflection and semantic memory still + missing a vector, in batches of 50. Intended to run once after + deploying the migration that adds the embedding tables, so the + historical corpus is searchable immediately instead of waiting for + retrieval traffic to heal it row by row. + ## Reinforcement Each observation and reflection has a `reinforcement_score` that decays slowly over time and is updated by validated use: @@ -139,11 +168,25 @@ captures whether memories actually shaped Claude's work: (`cited` / `shaped` / `ignored` / `misled` / `overlooked`). Only on the second Stop fire — after ratings land — does the hook drop the `session_end` marker into the spool. -4. **Aggregation** — `useful_count` / `times_overlooked` / `times_misled` - columns on reflections and semantic memories accumulate. Retrieval - queries `ORDER BY (useful_count + 3 × times_overlooked) DESC` so - memories that proved themselves — or that the user had to recover — - surface first. +4. **Ranking** - `useful_count` / `times_overlooked` / `times_misled` / + `times_ignored` columns on reflections and semantic memories + accumulate, and retrieval ranks each bucket by a Wilson score lower + bound (95% CI) on the proportion of rated exposures that were + positive: `(useful_count + times_overlooked) / (useful_count + + times_overlooked + times_ignored)`, computed in + `better_memory/services/scoring.py`. Ties break on confidence, then + recency. A memory with fewer than 3 rated exposures has a + statistically meaningless score (pinned to 0), so instead of losing + outright to proven rows it competes for a reserved exploration + slot: the last slot of each polarity bucket (when the bucket cap + allows at least 2 entries) is set aside for the highest-ranked + under-rated memory, if one exists. When `memory.retrieve` is called + with a `query`, this Wilson-ranked list is re-fused with a BM25 + relevance ranking (title / use_cases / hints) and a vector-kNN + ranking via the same Reciprocal Rank Fusion used for observations -- + a three-leg RRF of Wilson rank, BM25 rank, and vector rank -- so a + query that matches nothing on either extra leg degrades exactly to + the Wilson-only order. The management UI's Reflections and Semantic tabs surface useful / overlooked / misled badges per row, and `/diagnostics` exposes recent diff --git a/website/configuration.md b/website/configuration.md index 8a1c3c1..7f01a73 100644 --- a/website/configuration.md +++ b/website/configuration.md @@ -72,7 +72,7 @@ The two SQLite files are never shared between processes — the MCP server owns Four Claude Code hooks ship with better-memory and read or write the filesystem layout above. They are installed automatically by `./scripts/setup.sh` (which calls `python -m better_memory.cli.install_hooks` to merge them idempotently into `~/.claude/settings.json`). The list below is reference material: -- **`better_memory.hooks.session_bootstrap`** (SessionStart) — opens or reuses a background episode for the session and injects the project's curated context (project-scoped and general-scope semantic memories plus distilled reflections in `do` / `dont` / `neutral` buckets, retrieved up to 20 per bucket and ranked by usefulness then confidence) as `additionalContext` for Claude's first turn. Only the top `BETTER_MEMORY_BOOTSTRAP_TOP_N` project-scoped items (default 5; general-scope semantic memories are always shown in full) render in full; the rest collapse into a one-line index plus a `memory.retrieve` / `memory.retrieve_observations` affordance. Setting `BETTER_MEMORY_BOOTSTRAP_TOP_N=0` restores the legacy full-dump behavior. Runs in-process against `memory.db` (in agentcore mode it routes through the storage backend instead and never opens the local database); failure-isolated: if bootstrap breaks, a fallback directive is injected and the failure is recorded in the `hook_errors` table. +- **`better_memory.hooks.session_bootstrap`** (SessionStart) — opens or reuses a background episode for the session and injects the project's curated context (project-scoped and general-scope semantic memories plus distilled reflections in `do` / `dont` / `neutral` buckets, retrieved up to 20 per bucket and ranked by a Wilson-score lower bound on positive-rated exposures, then confidence, then recency, with the bucket's last slot reserved for an under-rated memory - fewer than 3 rated exposures - when one exists; see [Architecture](architecture.md#self-rating-loop)) as `additionalContext` for Claude's first turn. Only the top `BETTER_MEMORY_BOOTSTRAP_TOP_N` project-scoped items (default 5; general-scope semantic memories are always shown in full) render in full; the rest collapse into a one-line index plus a `memory.retrieve` / `memory.retrieve_observations` affordance. Setting `BETTER_MEMORY_BOOTSTRAP_TOP_N=0` restores the legacy full-dump behavior. Runs in-process against `memory.db` (in agentcore mode it routes through the storage backend instead and never opens the local database); failure-isolated: if bootstrap breaks, a fallback directive is injected and the failure is recorded in the `hook_errors` table. - **`better_memory.hooks.observer`** (PostToolUse) — captures tool-call snapshots into `spool/` for later observation creation. - **`better_memory.hooks.session_close`** (Stop) — writes a session-close marker into `spool/`; also emits the rating directive described in [Self-rating loop](architecture.md#self-rating-loop). In agentcore mode (resolved via the env var, else `settings.json`) it additionally fires one `CreateEvent(role=OTHER)` closure event against the current AgentCore session so the episodic strategy extracts within minutes; failure is logged to `hook_errors` and never blocks the marker write. - **`better_memory.hooks.contextual_inject`** (UserPromptSubmit + PreToolUse) — scores the curated memory set (semantic + reflections) against the current prompt or tool-input via keyword-hit x activation, drops candidates below `BETTER_MEMORY_CONTEXT_MIN_HITS`, caps survivors to `BETTER_MEMORY_CONTEXT_MAX_ITEMS`, and injects them as a `` block in `additionalContext`. A per-session seen-file dedups repeats (`BETTER_MEMORY_CONTEXT_REINJECT_TURNS` controls re-injection). Gated by `BETTER_MEMORY_CONTEXT_INJECT_MODE`. See [Architecture](architecture.md#injection-strategies) for detail. From 17eefcbd1fdbeb177a30cce8046ab0c01db190dc Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 19:43:21 +0100 Subject: [PATCH 18/20] =?UTF-8?q?docs:=20rank-prose=20sweep=20=E2=80=94=20?= =?UTF-8?q?five=20spots=20still=20described=20popularity=20ranking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- better_memory/mcp/tools.py | 4 ++-- better_memory/services/reflection.py | 8 ++++---- better_memory/storage/protocol.py | 10 +++++----- website/mcp-tools.md | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/better_memory/mcp/tools.py b/better_memory/mcp/tools.py index 1db94cf..5656acc 100644 --- a/better_memory/mcp/tools.py +++ b/better_memory/mcp/tools.py @@ -189,9 +189,9 @@ def tool_definitions( # `query` is deliberately NOT required. Making it mandatory # would break every existing caller (and the start_episode / # bootstrap internal paths) for a param whose absence degrades - # gracefully to the popularity order. The description urges it + # gracefully to the Wilson-prior. The description urges it # instead; the real usefulness gains come from the shortlist - # default and the ignored-history demotion, not from coercing + # default and the Wilson-score confidence boost, not from coercing # a query on every call. }, ), diff --git a/better_memory/services/reflection.py b/better_memory/services/reflection.py index 88ef62e..26c7392 100644 --- a/better_memory/services/reflection.py +++ b/better_memory/services/reflection.py @@ -1332,7 +1332,7 @@ def _fuse_by_relevance( """Re-order ``rows`` by RRF fusion of popularity rank, BM25 rank, and vector rank. - ``rows`` arrives in popularity order (``useful_count + 3*times_overlooked``, + ``rows`` arrives in Wilson-prior order (hit-rate lower bound, then confidence, then recency). We compute a second ranking over the same ids by BM25 relevance against ``reflection_fts`` (title / use_cases / hints), and (when ``query_vector`` is available) a third ranking by @@ -1436,7 +1436,7 @@ def retrieve_reflections( track_exposure: bool = True, query: str | None = None, ) -> dict[str, list[dict]]: - """Return reflections bucketed by polarity, ordered by confidence DESC. + """Return reflections bucketed by polarity, ordered by Wilson-prior then confidence. Filters: - ``project``: required. @@ -1445,9 +1445,9 @@ def retrieve_reflections( - ``polarity``: optional exact match; non-matching buckets remain empty. - ``query``: optional natural-language description of the task at hand. When given, the filtered set is re-ordered by RRF fusion of the - popularity prior with a BM25 relevance ranking over + Wilson-prior with a BM25 relevance ranking over ``title / use_cases / hints`` (see :meth:`_fuse_by_relevance`). - Without it, ordering is the popularity prior alone — which returns + Without it, ordering is the Wilson-prior alone — which returns the same rows to every caller regardless of what they are doing. - ``limit_per_bucket``: cap each polarity bucket. Default 20 per spec §7. Pass ``None`` to disable the cap (returns every matching row per diff --git a/better_memory/storage/protocol.py b/better_memory/storage/protocol.py index d67855f..dc805ee 100644 --- a/better_memory/storage/protocol.py +++ b/better_memory/storage/protocol.py @@ -98,14 +98,14 @@ def retrieve( use_cases, hints (list[str]), confidence (float), tech, evidence_count, useful_count, times_misled, updated_at}``. Sync — no embedder call (reflections are pre-extracted in both backends; - sqlite mode ranks via SQL - ORDER BY ``useful_count + 3 * times_overlooked DESC``, agentcore - mode applies the same formula client-side over metadata counters). + sqlite mode ranks by Wilson lower bound on (useful+overlooked)/rated, + computed in Python; agentcore mode applies the legacy linear formula + client-side over metadata counters). ``query`` optionally supplies a natural-language description of the task at hand. sqlite mode fuses a BM25 relevance ranking over - title / use_cases / hints into the popularity order via RRF; agentcore - mode ignores it. Omitting it yields the popularity order alone, which + title / use_cases / hints into the Wilson-prior via RRF; agentcore + mode ignores it. Omitting it yields the Wilson-prior alone, which is identical for every caller regardless of the work being done. This method is the canonical path for the MCP ``memory.retrieve`` diff --git a/website/mcp-tools.md b/website/mcp-tools.md index 201e4da..1ec1311 100644 --- a/website/mcp-tools.md +++ b/website/mcp-tools.md @@ -120,7 +120,7 @@ Spawn or reuse the management UI. Returns `{"url": "...", "reused": bool}`. ### `memory.session_bootstrap` -Open or reuse a session episode and inject project + general semantic memories and reflections as `additionalContext` markdown. Reflections are retrieved up to 20 per polarity bucket (`do` / `dont` / `neutral`), ranked by usefulness then confidence. Only the top `BETTER_MEMORY_BOOTSTRAP_TOP_N` project-scoped items (default 5; general-scope semantic memories always render in full) are shown in full — the rest collapse into a one-line index plus a retrieve affordance (`BETTER_MEMORY_BOOTSTRAP_TOP_N=0` restores the legacy full dump). Mirrors what the SessionStart hook does; callable manually for recovery, testing, or post-`/clear` re-injection. +Open or reuse a session episode and inject project + general semantic memories and reflections as `additionalContext` markdown. Reflections are retrieved up to 20 per polarity bucket (`do` / `dont` / `neutral`), ranked by Wilson-prior lower bound on hit-rate, ties by confidence. Only the top `BETTER_MEMORY_BOOTSTRAP_TOP_N` project-scoped items (default 5; general-scope semantic memories always render in full) are shown in full — the rest collapse into a one-line index plus a retrieve affordance (`BETTER_MEMORY_BOOTSTRAP_TOP_N=0` restores the legacy full dump). Mirrors what the SessionStart hook does; callable manually for recovery, testing, or post-`/clear` re-injection. | Parameter | Type | Required | Notes | |---|---|---|---| From f4d18900a4497a0b3311f2a0a6c5d53bcd166c4c Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 19:43:22 +0100 Subject: [PATCH 19/20] test(e2e): junction test spawns via harness _spawn per env-isolation contract Co-Authored-By: Claude Fable 5 --- tests/e2e/test_install_hooks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/e2e/test_install_hooks.py b/tests/e2e/test_install_hooks.py index fb61d01..880e805 100644 --- a/tests/e2e/test_install_hooks.py +++ b/tests/e2e/test_install_hooks.py @@ -738,9 +738,9 @@ def test_junction_skill_link_is_recognised_and_skipped( skills_dir.mkdir(parents=True, exist_ok=True) junction = skills_dir / SKILL_NAMES[0] - proc = subprocess.run( # noqa: S603 - fixed argv, test harness + proc = _spawn( + harness.env, ["cmd", "/c", "mklink", "/J", str(junction), str(source)], - capture_output=True, text=True, ) if proc.returncode != 0: pytest.skip(f"cannot create junction here: {proc.stderr.strip()}") From 689db19254d0db349de396ee1e73227b3b7dfe7c Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 23 Jul 2026 21:21:25 +0100 Subject: [PATCH 20/20] fix(embeddings): share one SyncEmbedder (and its breaker) across server and backend Addresses PR #83 review: SqliteBackend built a second SyncEmbedder, splitting the process-wide circuit breaker into two independent ones. Co-Authored-By: Claude Fable 5 --- better_memory/mcp/server.py | 1 + better_memory/storage/factory.py | 13 ++++++- better_memory/storage/sqlite.py | 13 +++---- tests/mcp/test_server_backend_dispatch.py | 42 +++++++++++++++++++++++ tests/storage/test_factory.py | 20 +++++++++++ tests/storage/test_sqlite_backend.py | 27 +++++++++++++++ 6 files changed, 109 insertions(+), 7 deletions(-) diff --git a/better_memory/mcp/server.py b/better_memory/mcp/server.py index 72c241b..66e20a5 100644 --- a/better_memory/mcp/server.py +++ b/better_memory/mcp/server.py @@ -221,6 +221,7 @@ def create_server() -> tuple[ config=config, memory_conn=memory_conn, embedder=embedder, + sync_embedder=sync_embedder, session_id=startup_session_id, project=startup_project, ) diff --git a/better_memory/storage/factory.py b/better_memory/storage/factory.py index 7efd033..d1e8cbe 100644 --- a/better_memory/storage/factory.py +++ b/better_memory/storage/factory.py @@ -34,16 +34,27 @@ def build_backend( config: _ConfigLike, memory_conn: sqlite3.Connection | None, embedder: Any = None, + sync_embedder: Any = None, session_id: str | None, project: str, ) -> StorageBackend: - """Construct the StorageBackend implementation appropriate for the config.""" + """Construct the StorageBackend implementation appropriate for the config. + + ``sync_embedder``, when provided, must be the caller's already-built + process-wide ``SyncEmbedder`` (see ``mcp/server.py``). It is forwarded + as-is to ``SqliteBackend`` so its internal ``_semantic`` / ``_synthesis`` + services share the same circuit breaker as the caller's write-path + tools instead of each backend build spinning up an independent one. + Unused by the agentcore path, which has no local semantic/synthesis + services of its own. + """ if config.storage_backend == "sqlite": if memory_conn is None: raise ValueError("sqlite backend requires memory_conn") return SqliteBackend( memory_conn=memory_conn, embedder=embedder, + sync_embedder=sync_embedder, session_id=session_id, project=project, ) diff --git a/better_memory/storage/sqlite.py b/better_memory/storage/sqlite.py index 11002ed..5dab7fd 100644 --- a/better_memory/storage/sqlite.py +++ b/better_memory/storage/sqlite.py @@ -9,6 +9,12 @@ - ``embedder`` — forwarded to ObservationService. May be ``None`` for the sqlite (FTS5) embeddings backend, which indexes via DB triggers instead of a Python embedder. +- ``sync_embedder`` — caller-owned ``SyncEmbedder`` forwarded to + ``SemanticMemoryService`` / ``ReflectionSynthesisService`` as-is. The + backend does NOT construct its own — it must be the same process-wide + instance the caller (e.g. ``mcp/server.py``) built, so its circuit + breaker state is shared across the write-path tools and this backend's + ``retrieve``/``semantic`` methods rather than split into two breakers. - ``session_id`` — used for episode lookups, ratings, exposures. ``None`` means "defer resolution to env-var fallback at first write" (ObservationService re-resolves from ``CLAUDE_SESSION_ID`` / @@ -25,8 +31,6 @@ import sqlite3 from typing import Any -from better_memory.embeddings.ollama import OllamaEmbedder -from better_memory.embeddings.sync_embed import SyncEmbedder from better_memory.services.episode import EpisodeService from better_memory.services.memory_rating import MemoryRatingService from better_memory.services.observation import ObservationService @@ -47,6 +51,7 @@ def __init__( *, memory_conn: sqlite3.Connection, embedder: Any = None, + sync_embedder: Any = None, session_id: str | None, project: str, ) -> None: @@ -66,10 +71,6 @@ def __init__( self._reflection = ReflectionService(memory_conn) self._memory_rating = MemoryRatingService(memory_conn) self._session_bootstrap = SessionBootstrapService(memory_conn) - sync_embedder = ( - SyncEmbedder(lambda: OllamaEmbedder(timeout=5.0, max_retries=1)) - if embedder is not None else None - ) self._semantic = SemanticMemoryService(memory_conn, sync_embedder=sync_embedder) self._synthesis = ReflectionSynthesisService( memory_conn, sync_embedder=sync_embedder diff --git a/tests/mcp/test_server_backend_dispatch.py b/tests/mcp/test_server_backend_dispatch.py index 4791558..61bcecd 100644 --- a/tests/mcp/test_server_backend_dispatch.py +++ b/tests/mcp/test_server_backend_dispatch.py @@ -44,6 +44,48 @@ def test_synthesis_tools_skipped_when_capability_false() -> None: assert "memory.synthesize_next_apply" not in tool_names +async def test_create_server_shares_one_sync_embedder_with_backend( + monkeypatch: pytest.MonkeyPatch, tmp_path +) -> None: + """Regression for PR #83: create_server must build exactly ONE + SyncEmbedder and hand it to both the top-level services (reflections/ + semantic) and build_backend (so SqliteBackend's internal _synthesis/ + _semantic reuse it) — not construct a second, independent instance + with its own circuit breaker.""" + home = tmp_path / "bm" + home.mkdir() + (home / "knowledge-base").mkdir() + monkeypatch.setenv("BETTER_MEMORY_HOME", str(home)) + monkeypatch.setenv("BETTER_MEMORY_EMBEDDINGS_BACKEND", "ollama") + monkeypatch.setenv("OLLAMA_HOST", "http://does-not-exist.invalid:1") + monkeypatch.delenv("BETTER_MEMORY_STORAGE_BACKEND", raising=False) + + import better_memory.mcp.server as server_mod + + created: list[Any] = [] + real_sync_embedder = server_mod.SyncEmbedder + + class _TrackingSyncEmbedder(real_sync_embedder): # type: ignore[misc, valid-type] + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + created.append(self) + + monkeypatch.setattr(server_mod, "SyncEmbedder", _TrackingSyncEmbedder) + + from better_memory.storage import SqliteBackend + + server, cleanup, ctx = server_mod.create_server() + try: + assert len(created) == 1, ( + f"expected exactly one process-wide SyncEmbedder, got {len(created)}" + ) + assert isinstance(ctx.backend, SqliteBackend) + assert ctx.backend._synthesis._sync_embedder is created[0] + assert ctx.backend._semantic._sync_embedder is created[0] + finally: + await cleanup() + + def test_create_server_returns_three_tuple_with_backend( monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: diff --git a/tests/storage/test_factory.py b/tests/storage/test_factory.py index 2824fa6..20de4d9 100644 --- a/tests/storage/test_factory.py +++ b/tests/storage/test_factory.py @@ -86,6 +86,26 @@ def test_build_backend_returns_sqlite_for_sqlite_config(memory_conn) -> None: assert isinstance(backend, StorageBackend) +def test_build_backend_forwards_sync_embedder_to_sqlite_backend(memory_conn) -> None: + """Regression for PR #83: build_backend must pass the caller's + process-wide SyncEmbedder through to SqliteBackend rather than letting + SqliteBackend construct its own — otherwise the circuit breaker splits + into an independent instance per backend build.""" + cfg = _config() + sentinel_sync_embedder = MagicMock(name="sentinel-sync-embedder") + backend = build_backend( + config=cfg, + memory_conn=memory_conn, + embedder=MagicMock(), + sync_embedder=sentinel_sync_embedder, + session_id="s", + project="p", + ) + assert isinstance(backend, SqliteBackend) + assert backend._synthesis._sync_embedder is sentinel_sync_embedder + assert backend._semantic._sync_embedder is sentinel_sync_embedder + + def test_build_backend_raises_for_unknown(memory_conn) -> None: cfg = _config(storage_backend="bogus") with pytest.raises(ValueError, match="unknown storage_backend"): diff --git a/tests/storage/test_sqlite_backend.py b/tests/storage/test_sqlite_backend.py index 423d999..7afbaaa 100644 --- a/tests/storage/test_sqlite_backend.py +++ b/tests/storage/test_sqlite_backend.py @@ -54,6 +54,33 @@ def test_sqlite_backend_satisfies_protocol(backend) -> None: assert isinstance(backend, StorageBackend) +def test_sqlite_backend_shares_caller_sync_embedder(memory_conn) -> None: + """Regression for PR #83: SqliteBackend must NOT build its own + SyncEmbedder for _semantic/_synthesis — it must reuse the caller's + instance so the circuit breaker is process-wide, not split in two.""" + embedder = MagicMock() + embedder.embed = AsyncMock(return_value=[0.0] * 768) + sentinel_sync_embedder = MagicMock(name="sentinel-sync-embedder") + + backend = SqliteBackend( + memory_conn=memory_conn, + embedder=embedder, + sync_embedder=sentinel_sync_embedder, + session_id="test-session", + project="testproj", + ) + + assert backend._synthesis._sync_embedder is sentinel_sync_embedder + assert backend._semantic._sync_embedder is sentinel_sync_embedder + + +def test_sqlite_backend_sync_embedder_defaults_to_none(backend) -> None: + """When the caller passes no sync_embedder (default), the backend must + not silently construct its own — both services stay None.""" + assert backend._synthesis._sync_embedder is None + assert backend._semantic._sync_embedder is None + + def test_sqlite_backend_implements_hot_path_methods(backend) -> None: """Task 3 surface check: every hot-path method is present and callable.""" for name in ("observe", "retrieve", "list_observations", "record_use"):