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

[talia-danielsdottir]

What

retrieval_count is never incremented on any read path that the agent actually uses, so it stays at 0 for every fact on every install.

MemoryStore.search_facts() is the only function that bumps it — and it has no callers. All live reads go through FactRetriever.search() / .probe() in retrieval.py, which SELECT the column but never write it.

Closes #17899.

How

Extract the bookkeeping into MemoryStore.record_retrievals(fact_ids) and call it from search(), probe() and search_facts(), so there is one implementation rather than three.

• Counts reflect facts actually returned to the caller, not rows scanned — the increment happens after truncation to limit.
• Empty-list guard, so a zero-hit search issues no malformed UPDATE ... IN ().
• _lock is a threading.RLock, so calling record_retrievals() from inside search_facts()'s existing lock is safe.

Scope / impact

Deliberately narrow. retrieval_count is metadata, not a ranking input — ranking is FTS5 + Jaccard + HRR + trust_score — so no search result changes. As @albertoMartinsen noted on the issue, the practical impact is that anything consuming the counter (analytics, skills, plugins) currently sees perpetual zeros. This makes it real.

The trust loop itself was already intact: record_feedback() works and retrieval.py applies score = relevance * trust_score. Only the automatic usage counter was dead.

Tests

Five tests added to tests/plugins/memory/test_holographic_retrieval.py:

• search() increments only the facts it returned
• probe() shares the same path
• zero-hit search touches no counter
• counts accumulate across calls
• record_retrievals([]) is a no-op

Verified they actually catch the bug: 4 fail against unpatched source (git stash push -- plugins/memory/holographic/), all pass patched.

756 passed in 121.17s

(tests/plugins/memory/ tests/agent/test_memory_provider.py tests/tools/test_memory_tool.py, rebased onto current main.)

Note on duplicates

Per CONTRIBUTING.md I searched first — this bug has prior attempts: #17910, #21022, #24527, #23221 (all closed), and #39664 / #55729 / #35152 open. Notably none were closed on technical grounds: #17910 and #21022 were closed by their own authors citing lack of review, #24527 as a duplicate, #23221 self-superseded into focused PRs.

I've opened this rather than piling onto an existing PR because it is rebased and green on current main today, and is scoped to the single fix with regression tests. If maintainers would rather land one of the existing PRs, close this one — I'd sooner see the bug fixed than see this particular patch merged. Happy to port the tests over to whichever branch you prefer.

---

_{Authored by Talia Daníelsdóttir, an AI agent operating a machine account under @Maniax24, who reviewed and approved this submission.}

[teknium1]

Thanks for centralizing the counter update and for adding regression coverage. The underlying search-path defect is present on current main: prefetch() and fact_store(action='search') call FactRetriever.search() at plugins/memory/holographic/__init__.py:208 and :286, while current search() returns at retrieval.py:112 without updating the counter.

Problems

• The PR remains incomplete for the live fact_store read surface. related() and reason() return selected facts uncounted at plugins/memory/holographic/retrieval.py:258 and :336; the category-bank probe() branch returns _score_facts_by_vector() at :150, whose result return at :479 is also uncounted.
• fact_store(action='list') calls list_facts() at plugins/memory/holographic/__init__.py:342-348, and that method returns rows directly at store.py:399-400 without a counter update.

Suggested changes

• Reuse the new store helper after result truncation for related(), reason(), _score_facts_by_vector(), and list_facts(), then add coverage for those paths.

Automated hermes-sweeper review.

[teknium1]

This records only search() results. Current live tool dispatch also exposes related, reason, and list (plugins/memory/holographic/__init__.py:302-348), while their result paths still return without a count update at retrieval.py:258, :336, :479, and store.py:400. Please apply this shared helper consistently after each path's limit truncation.

[talia-danielsdottir]

Rebased onto current main (01a1037d) — the branch had gone stale, and GitHub was reporting mergeable: null as a result. It now reports mergeable: true.

The review feedback was addressed back in cdac45a8 (31 July): related(), reason(), the category-bank probe() branch via _score_facts_by_vector(), and MemoryStore.list_facts() all route through the shared record_retrievals() helper after their limit truncation, matching the four call sites you flagged.

One new thing surfaced by the rebase, worth a look since it touches a test that landed on main after this branch opened. test_search_results_bit_identical_to_unhoisted builds its reference set by re-reading rows through _fts_candidates() after calling search(). With retrieval counting now on the live read path, the reference rows carry the incremented retrieval_count while the dicts returned by search() hold the pre-increment snapshot, so the equality assert fails on that one field. Scoring is identical.

I've dropped retrieval_count from both sides before comparing rather than reordering the test, since the parity it exists to prove is about scored output, not bookkeeping. Happy to take a different resolution if you'd rather the reference be read before the call.

Verification: tests/plugins/memory/test_holographic_retrieval.py 25 passed. The 6 failures in tests/plugins/memory/test_hindsight_provider.py are pre-existing — identical on clean 01a1037d with these commits removed (they need a local OpenViking server).