fix(memory/holographic): record retrieval_count on the live read path

plugins/memory/holographic/retrieval.py

@@ -119,6 +119,7 @@ def search(
         # Strip raw HRR bytes — callers expect JSON-serializable dicts
         for fact in results:
             fact.pop("hrr_vector", None)
+        self.store.record_retrievals([f["fact_id"] for f in results])
         return results
 
     def probe(
@@ -199,7 +200,9 @@ def probe(
             scored.append(fact)
 
         scored.sort(key=lambda x: x["score"], reverse=True)
-        return scored[:limit]
+        results = scored[:limit]
+        self.store.record_retrievals([f["fact_id"] for f in results])
+        return results
 
     def related(
         self,
@@ -269,7 +272,9 @@ def related(
             scored.append(fact)
 
         scored.sort(key=lambda x: x["score"], reverse=True)
-        return scored[:limit]
+        results = scored[:limit]
+        self.store.record_retrievals([f["fact_id"] for f in results])
+        return results
 
     def reason(
         self,
@@ -347,7 +352,9 @@ def reason(
             scored.append(fact)
 
         scored.sort(key=lambda x: x["score"], reverse=True)
-        return scored[:limit]
+        results = scored[:limit]
+        self.store.record_retrievals([f["fact_id"] for f in results])
+        return results
 
     def contradict(
         self,
@@ -490,7 +497,9 @@ def _score_facts_by_vector(
             scored.append(fact)
 
         scored.sort(key=lambda x: x["score"], reverse=True)
-        return scored[:limit]
+        results = scored[:limit]
+        self.store.record_retrievals([f["fact_id"] for f in results])
+        return results
 
     def _fts_candidates(
         self,

plugins/memory/holographic/store.py

@@ -277,16 +277,25 @@ def search_facts(
             rows = self._conn.execute(sql, params).fetchall()
             results = [self._row_to_dict(r) for r in rows]
 
-            if results:
-                ids = [r["fact_id"] for r in results]
-                placeholders = ",".join("?" * len(ids))
-                self._conn.execute(
-                    f"UPDATE facts SET retrieval_count = retrieval_count + 1 WHERE fact_id IN ({placeholders})",
-                    ids,
-                )
-                self._conn.commit()
+        self.record_retrievals([r["fact_id"] for r in results])
+        return results
 
-            return results
+    def record_retrievals(self, fact_ids: list[int]) -> None:
+        """Increment retrieval_count for the given facts.
+
+        Retrieval bookkeeping lives here so every read path can share it.
+        Callers pass the ids they actually returned to the caller, so the
+        counter reflects facts surfaced rather than rows scanned.
+        """
+        if not fact_ids:
+            return
+        with self._lock:
+            placeholders = ",".join("?" * len(fact_ids))
+            self._conn.execute(
+                f"UPDATE facts SET retrieval_count = retrieval_count + 1 WHERE fact_id IN ({placeholders})",
+                fact_ids,
+            )
+            self._conn.commit()
 
     def update_fact(
         self,
@@ -397,7 +406,10 @@ def list_facts(
                 LIMIT ?
             """
             rows = self._conn.execute(sql, params).fetchall()
-            return [self._row_to_dict(r) for r in rows]
+            facts = [self._row_to_dict(r) for r in rows]
+        # Outside the read lock: record_retrievals takes the same RLock and commits.
+        self.record_retrievals([f["fact_id"] for f in facts])
+        return facts
 
     def record_feedback(self, fact_id: int, helpful: bool) -> dict:
         """Record user feedback and adjust trust asymmetrically.

tests/plugins/memory/test_holographic_retrieval.py

@@ -196,6 +196,14 @@ def test_search_results_bit_identical_to_unhoisted(hoisted_retriever):
     for fact in old_results:
         fact.pop("hrr_vector", None)
 
+    # ``retrieval_count`` is bookkeeping, not scoring: the reference rows are
+    # re-read from the DB *after* search() has already recorded its retrievals,
+    # so they carry the incremented value while the returned dicts hold the
+    # pre-increment snapshot. Compare everything else — the hoist parity this
+    # test exists to prove is about the scored output.
+    for fact in (*new_results, *old_results):
+        fact.pop("retrieval_count", None)
+
     assert new_results == old_results
 
 
@@ -238,3 +246,153 @@ def test_search_without_vectors_never_encodes(hoisted_retriever, monkeypatch):
         f"encode_text called {len(calls)}x with zero vector candidates — "
         "lazy hoist regressed to eager"
     )
+# ---------------------------------------------------------------------------
+# retrieval_count bookkeeping — the retriever is the live read path, so it
+# must record retrievals too. Previously only MemoryStore.search_facts (which
+# has no callers) incremented, leaving the counter permanently zero.
+# ---------------------------------------------------------------------------
+
+def _counts(store):
+    return dict(
+        store._conn.execute("SELECT fact_id, retrieval_count FROM facts").fetchall()
+    )
+
+
+def test_search_increments_retrieval_count(retriever_with_facts):
+    """search() bumps the counter for facts it actually returns."""
+    store = retriever_with_facts.store
+    assert set(_counts(store).values()) == {0}
+
+    results = retriever_with_facts.search("compaction")
+    assert len(results) >= 1
+
+    after = _counts(store)
+    for fact in results:
+        assert after[fact["fact_id"]] == 1
+    # Facts that were not returned stay untouched
+    returned = {f["fact_id"] for f in results}
+    assert all(c == 0 for fid, c in after.items() if fid not in returned)
+
+
+def test_probe_increments_retrieval_count(retriever_with_facts):
+    """probe() shares the same bookkeeping path as search()."""
+    store = retriever_with_facts.store
+    results = retriever_with_facts.probe("compaction")
+
+    after = _counts(store)
+    for fact in results:
+        assert after[fact["fact_id"]] >= 1
+
+
+def test_zero_hit_search_records_nothing(retriever_with_facts):
+    """An empty result set must not touch any counter."""
+    store = retriever_with_facts.store
+    before = _counts(store)
+    retriever_with_facts.search("zzzznomatchzzzz")
+    assert _counts(store) == before
+
+
+def test_record_retrievals_is_idempotent_per_call(retriever_with_facts):
+    """Two searches produce two increments — the counter accumulates."""
+    store = retriever_with_facts.store
+    first = retriever_with_facts.search("compaction")
+    fact_id = first[0]["fact_id"]
+    assert _counts(store)[fact_id] == 1
+    retriever_with_facts.search("compaction")
+    assert _counts(store)[fact_id] == 2
+
+
+def test_record_retrievals_empty_list_is_noop(retriever_with_facts):
+    """Guard clause: empty id list must not issue a malformed UPDATE."""
+    store = retriever_with_facts.store
+    before = _counts(store)
+    store.record_retrievals([])
+    assert _counts(store) == before
+
+
+def test_related_increments_retrieval_count(retriever_with_facts):
+    """related() returns facts to the caller, so it counts them too."""
+    store = retriever_with_facts.store
+    results = retriever_with_facts.related("deployment")
+    assert len(results) >= 1
+
+    after = _counts(store)
+    for fact in results:
+        assert after[fact["fact_id"]] >= 1
+    returned = {f["fact_id"] for f in results}
+    assert all(c == 0 for fid, c in after.items() if fid not in returned)
+
+
+def test_reason_increments_retrieval_count(retriever_with_facts):
+    """reason() is a live fact_store read path — its results count."""
+    store = retriever_with_facts.store
+    results = retriever_with_facts.reason(["deployment", "migration"])
+    assert len(results) >= 1
+
+    after = _counts(store)
+    for fact in results:
+        assert after[fact["fact_id"]] >= 1
+
+
+def test_probe_category_bank_branch_increments(retriever_with_facts):
+    """The category-bank branch of probe() returns via _score_facts_by_vector.
+
+    That helper has its own return path, which was previously uncounted even
+    though probe()'s direct-scoring branch was fixed.
+    """
+    store = retriever_with_facts.store
+    results = retriever_with_facts.probe("compaction", category="tool")
+    assert len(results) >= 1
+
+    after = _counts(store)
+    for fact in results:
+        assert after[fact["fact_id"]] >= 1
+
+
+def test_score_facts_by_vector_increments_directly(retriever_with_facts):
+    """_score_facts_by_vector counts whatever it hands back."""
+    import numpy as np
+
+    from plugins.memory.holographic import holographic as hrr
+
+    store = retriever_with_facts.store
+    target = hrr.encode_text("compaction", retriever_with_facts.hrr_dim)
+    results = retriever_with_facts._score_facts_by_vector(target, limit=2)
+    assert len(results) >= 1
+
+    after = _counts(store)
+    for fact in results:
+        assert after[fact["fact_id"]] >= 1
+    assert isinstance(target, np.ndarray)
+
+
+def test_list_facts_increments_retrieval_count(retriever_with_facts):
+    """fact_store(action='list') surfaces facts, so list_facts() counts them."""
+    store = retriever_with_facts.store
+    assert set(_counts(store).values()) == {0}
+
+    facts = store.list_facts()
+    assert len(facts) == 3
+
+    after = _counts(store)
+    for fact in facts:
+        assert after[fact["fact_id"]] == 1
+
+
+def test_list_facts_respects_limit_when_counting(retriever_with_facts):
+    """Only the truncated result set is counted, not every scanned row."""
+    store = retriever_with_facts.store
+    facts = store.list_facts(limit=1)
+    assert len(facts) == 1
+
+    after = _counts(store)
+    assert after[facts[0]["fact_id"]] == 1
+    assert sum(after.values()) == 1
+
+
+def test_list_facts_empty_result_records_nothing(retriever_with_facts):
+    """A filter that matches nothing must leave every counter alone."""
+    store = retriever_with_facts.store
+    before = _counts(store)
+    assert store.list_facts(min_trust=99.0) == []
+    assert _counts(store) == before