Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions plugins/memory/holographic/retrieval.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@

from __future__ import annotations

import logging
import math
from datetime import datetime, timezone
from typing import TYPE_CHECKING

logger = logging.getLogger(__name__)

if TYPE_CHECKING:
from .store import MemoryStore

Expand Down Expand Up @@ -106,6 +109,20 @@ def search(
# Sort by score descending, return top limit
scored.sort(key=lambda x: x["score"], reverse=True)
results = scored[:limit]

# Increment retrieval_count for returned facts (fixes #17899)
if results:
try:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please route this through a locked MemoryStore helper rather than writing _conn directly. MemoryStore deliberately shares one SQLite connection and one _lock for serialized access (store.py:101-112); this new write bypasses that contract and duplicates the existing counter SQL in search_facts().

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teknium1 I've implemented this with the locked MemoryStore helper. Might wanna take a look
And following your suggestion, I've wired all the retrieval paths to increment the count.

#73901

ids = [r["fact_id"] for r in results]
placeholders = ",".join("?" * len(ids))
self.store._conn.execute(
f"UPDATE facts SET retrieval_count = retrieval_count + 1 WHERE fact_id IN ({placeholders})",
ids,
)
self.store._conn.commit()
except Exception:
logger.debug("Failed to increment retrieval_count", exc_info=True)

# Strip raw HRR bytes — callers expect JSON-serializable dicts
for fact in results:
fact.pop("hrr_vector", None)
Expand Down
70 changes: 70 additions & 0 deletions tests/plugins/memory/test_retrieval_count.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
"""Tests for holographic memory retrieval_count increment (#17899).

retrieval_count was never incremented because FactRetriever.search()
bypassed store.search_facts(), the only place that bumped the counter.
"""

import pytest

from plugins.memory.holographic.store import MemoryStore
from plugins.memory.holographic.retrieval import FactRetriever


@pytest.fixture
def store(tmp_path):
db = str(tmp_path / "test_memory.db")
return MemoryStore(db_path=db, default_trust=0.5, hrr_dim=128)


@pytest.fixture
def retriever(store):
return FactRetriever(store=store, hrr_dim=128, hrr_weight=0.0)


class TestRetrievalCount:
"""retrieval_count should be incremented when facts are retrieved."""

def test_search_increments_count(self, store, retriever):
"""After searching, retrieval_count should go from 0 to >= 1."""
fact_id = store.add_fact("Python is a programming language")

# Verify starting at 0
row = store._conn.execute(
"SELECT retrieval_count FROM facts WHERE fact_id = ?", (fact_id,)
).fetchone()
assert row["retrieval_count"] == 0

# Search and find it
results = retriever.search("Python", min_trust=0.0)
assert len(results) >= 1

# Verify incremented
row = store._conn.execute(
"SELECT retrieval_count FROM facts WHERE fact_id = ?", (fact_id,)
).fetchone()
assert row["retrieval_count"] >= 1

def test_multiple_searches_accumulate(self, store, retriever):
"""Multiple searches should keep incrementing."""
store.add_fact("Rust is memory safe")

retriever.search("Rust", min_trust=0.0)
retriever.search("Rust", min_trust=0.0)
retriever.search("Rust", min_trust=0.0)

row = store._conn.execute(
"SELECT retrieval_count FROM facts WHERE content LIKE '%Rust%'"
).fetchone()
assert row["retrieval_count"] >= 3

def test_unrelated_search_no_increment(self, store, retriever):
"""Searching for something else shouldn't increment unrelated facts."""
fact_id = store.add_fact("Haskell is purely functional")
store.add_fact("Rust is memory safe")

retriever.search("Rust", min_trust=0.0)

row = store._conn.execute(
"SELECT retrieval_count FROM facts WHERE fact_id = ?", (fact_id,)
).fetchone()
assert row["retrieval_count"] == 0
Loading