From 32ae3d535dbde6714b04d4ec67871cdde237b4e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 20:15:22 +0000 Subject: [PATCH 1/2] Fix #104: scope _vec_qualifiers kNN to caller's candidate ids _vec_qualifiers issued an unfiltered global k=50 kNN against reflection_embeddings / semantic_embeddings. Both vec tables are global across projects, and retire only UPDATEs status without deleting the vector, so a caller's own candidates could be crowded out of the top-50 slice by other projects' or retired reflections -- and the returned rank was against the global neighbourhood, not the caller's competitors. Now the caller passes its already-computed candidate id list into _vec_qualifiers. We scale k with max(len(candidates), 50) and filter the kNN in Python by the candidate set, matching the sibling pattern established in ReflectionService._vec_ranks (reflection.py). vec_r / vec_s ranks are now dense over the caller's actual candidates, which means their RRF vec contribution reflects position among competitors rather than global neighbours. Empty candidate lists short-circuit without touching the DB. Existing 536 service tests and 84 hook tests pass unchanged. Added test_orphan_vector_row_does_not_leak_via_vec_gate as a regression that locks in the candidate scoping via a direct _vec_qualifiers call. Co-Authored-By: Claude Opus 4.7 --- better_memory/services/relevant.py | 45 ++++++++++++++++++++++++------ tests/services/test_relevant.py | 27 ++++++++++++++++++ 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/better_memory/services/relevant.py b/better_memory/services/relevant.py index 221b612..2c75477 100644 --- a/better_memory/services/relevant.py +++ b/better_memory/services/relevant.py @@ -105,8 +105,10 @@ def _vec_qualifiers( id_col: str, query_vector: list[float] | None, vec_floor: float, + candidate_ids: list[str], ) -> dict[str, int]: - """id -> vec rank for rows within the cosine floor. + """id -> vec rank for rows within the cosine floor, restricted to + ``candidate_ids``. Vectors are unit-norm, so cosine >= c <=> L2 distance <= sqrt(2*(1-c)). @@ -117,21 +119,31 @@ def _vec_qualifiers( L2 distance, NOT squared L2. The floor comparison below is against the plain (unsquared) distance only; the squared-distance branch that a defensive dual-check would need is dead code and has been omitted. + + sqlite-vec kNN accepts only ``embedding MATCH ? AND k = ?`` -- no + extra predicates -- so we fetch top-k and filter in Python, matching + ``ReflectionService._vec_ranks`` (reflection.py). ``k`` scales with the + candidate set (min 50) so the window is dense over what the caller + actually ranks, instead of being drowned by neighbours from other + projects or retired records (whose vectors persist -- retire only + UPDATEs status). """ - if conn is None or query_vector is None: + if conn is None or query_vector is None or not candidate_ids: return {} max_dist = math.sqrt(2.0 * (1.0 - vec_floor)) try: rows = conn.execute( f"SELECT {id_col}, distance FROM {table} " f"WHERE embedding MATCH ? AND k = ? ORDER BY distance", - (sqlite_vec.serialize_float32(query_vector), 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 row in rows: - if float(row[1]) <= max_dist: + if row[0] in wanted and float(row[1]) <= max_dist: out[row[0]] = len(out) return out @@ -213,8 +225,26 @@ def retrieve_relevant( bm = _bm25_qualifiers(conn, query) qvec = sync_embedder.embed_text(query) if sync_embedder is not None else None - vec_r = _vec_qualifiers(conn, "reflection_embeddings", "reflection_id", qvec, vec_floor) - vec_s = _vec_qualifiers(conn, "semantic_embeddings", "memory_id", qvec, vec_floor) + # Collect the actual candidate id set before the vec query so kNN is + # restricted to it (issue #104): the vec tables are global across + # projects and retained/retired rows, so an unbounded k=50 kNN can be + # entirely filled by neighbours the caller will never rank -- starving + # the evidence gate for the caller's own reflections/semantics. + refl_bucket_order = ["do", "dont"] + (["neutral"] if include_neutral else []) + refl_candidate_ids = [ + str(r.get("id")) + for bucket in refl_bucket_order + for r in (buckets.get(bucket) or []) + ] + sem_candidate_ids = [str(getattr(s, "id", "")) for s in (semantic or [])] + vec_r = _vec_qualifiers( + conn, "reflection_embeddings", "reflection_id", qvec, vec_floor, + refl_candidate_ids, + ) + vec_s = _vec_qualifiers( + conn, "semantic_embeddings", "memory_id", qvec, vec_floor, + sem_candidate_ids, + ) keywords = extract_keywords(query) # fallback evidence only fts_unavailable = conn is None @@ -255,8 +285,7 @@ def retrieve_relevant( rank_map: dict[tuple[str, str], int] = raw_rank_map or {} refl_candidates: list[dict] = [] - order = ["do", "dont"] + (["neutral"] if include_neutral else []) - for bucket in order: + for bucket in refl_bucket_order: for r in buckets.get(bucket, []) or []: r_id = str(r.get("id")) title = str(r.get("title") or "") diff --git a/tests/services/test_relevant.py b/tests/services/test_relevant.py index d7195a2..c22a17c 100644 --- a/tests/services/test_relevant.py +++ b/tests/services/test_relevant.py @@ -144,6 +144,33 @@ def test_vec_below_floor_does_not_qualify(self, conn): ) assert out == [] + def test_orphan_vector_row_does_not_leak_via_vec_gate(self, conn): + # Issue #104: the vec table is global and its rows persist across + # retire (retire only UPDATEs status; the vector stays). An + # "orphan" vector -- one whose metadata row was retired or deleted + # so backend.retrieve does not surface it as a candidate -- must + # not qualify anything through the vec leg. Prior to the fix, + # _vec_qualifiers issued an unfiltered kNN, so the orphan showed + # up in vec_r but happened to be harmless only because the RRF + # loop keys on the caller's candidate list -- this test locks in + # that the candidate scoping is now enforced at the vec-query + # layer too, so orphan ids never appear in vec_r at all. + from better_memory.services.relevant import _vec_qualifiers + emb = DirectedEmbedder("orphan trigger") + # Insert a vec row for a reflection id that has NO metadata row. + _embed_reflection(conn, "r-orphan", emb._vec("orphan trigger")) + qvec = emb._vec("orphan trigger") + # candidate_ids empty -> short-circuit + assert _vec_qualifiers( + conn, "reflection_embeddings", "reflection_id", + qvec, 0.55, [], + ) == {} + # candidate_ids restricts what returns from the kNN scan + assert _vec_qualifiers( + conn, "reflection_embeddings", "reflection_id", + qvec, 0.55, ["r-different"], + ) == {} + class TestWilsonRanking: def test_wilson_ranks_among_qualifiers(self, conn): From 950bf612f7a07395910f6bc289fb60e57b78db4d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 20:17:57 +0000 Subject: [PATCH 2/2] Make candidate_ids optional so protocol-completeness callers keep global scope SqliteBackend.relevance_ranks (sqlite.py:353, :359) intentionally wants the unfiltered rank map -- it's the "server-side gate" analogue used by retrieve_relevant's agentcore-mode branch (relevant.py:230-234 notes this callsite is never used from the sqlite branch, but it stays for protocol completeness). Making candidate_ids required broke pyright on those two calls. candidate_ids is now defaulted to None ("no scope filter, keep the existing global k=50 behaviour"). retrieve_relevant continues to pass its actual candidate list so its own vec scoping is enforced; [] still short-circuits explicitly. Regression test still passes. Co-Authored-By: Claude Opus 4.7 --- better_memory/services/relevant.py | 36 +++++++++++++++++------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/better_memory/services/relevant.py b/better_memory/services/relevant.py index 2c75477..5ee7335 100644 --- a/better_memory/services/relevant.py +++ b/better_memory/services/relevant.py @@ -105,10 +105,9 @@ def _vec_qualifiers( id_col: str, query_vector: list[float] | None, vec_floor: float, - candidate_ids: list[str], + candidate_ids: list[str] | None = None, ) -> dict[str, int]: - """id -> vec rank for rows within the cosine floor, restricted to - ``candidate_ids``. + """id -> vec rank for rows within the cosine floor. Vectors are unit-norm, so cosine >= c <=> L2 distance <= sqrt(2*(1-c)). @@ -120,30 +119,37 @@ def _vec_qualifiers( plain (unsquared) distance only; the squared-distance branch that a defensive dual-check would need is dead code and has been omitted. - sqlite-vec kNN accepts only ``embedding MATCH ? AND k = ?`` -- no - extra predicates -- so we fetch top-k and filter in Python, matching - ``ReflectionService._vec_ranks`` (reflection.py). ``k`` scales with the - candidate set (min 50) so the window is dense over what the caller - actually ranks, instead of being drowned by neighbours from other - projects or retired records (whose vectors persist -- retire only - UPDATEs status). + ``candidate_ids`` scopes the result to the caller's candidate set, + matching ``ReflectionService._vec_ranks`` (reflection.py) -- sqlite-vec + kNN accepts only ``embedding MATCH ? AND k = ?`` (no extra predicates), + so we fetch top-k and filter in Python. ``k`` scales with + ``max(len(candidate_ids), 50)`` so the window is dense over what the + caller actually ranks, instead of being drowned by neighbours from + other projects or retired records (whose vectors persist -- retire + only UPDATEs status). ``None`` preserves the global-scope behaviour + for callers (e.g. ``SqliteBackend.relevance_ranks``) that intentionally + want the unfiltered rank map; ``[]`` short-circuits to ``{}``. """ - if conn is None or query_vector is None or not candidate_ids: + if conn is None or query_vector is None: + return {} + if candidate_ids is not None and not candidate_ids: return {} max_dist = math.sqrt(2.0 * (1.0 - vec_floor)) + k = max(len(candidate_ids), 50) if candidate_ids is not None else 50 try: rows = conn.execute( f"SELECT {id_col}, distance FROM {table} " f"WHERE embedding MATCH ? AND k = ? ORDER BY distance", - (sqlite_vec.serialize_float32(query_vector), - max(len(candidate_ids), 50)), + (sqlite_vec.serialize_float32(query_vector), k), ).fetchall() except sqlite3.OperationalError: return {} - wanted = set(candidate_ids) + wanted: set[str] | None = set(candidate_ids) if candidate_ids is not None else None out: dict[str, int] = {} for row in rows: - if row[0] in wanted and float(row[1]) <= max_dist: + if wanted is not None and row[0] not in wanted: + continue + if float(row[1]) <= max_dist: out[row[0]] = len(out) return out