fix: increment retrieval_count in FactRetriever (closes #17899) - #24527
Closed
plhiggins wants to merge 1 commit into
Closed
fix: increment retrieval_count in FactRetriever (closes #17899)#24527plhiggins wants to merge 1 commit into
plhiggins wants to merge 1 commit into
Conversation
…reason The retrieval_count column in the facts table was never incremented during normal operation because FactRetriever bypasses store.search_facts() (the only method with the increment logic). All four retrieval methods — search(), probe(), related(), reason() — query the database directly via _fts_candidates or _score_facts_by_vector and never update retrieval_count. Add store.touch_facts() as a shared helper and call it in all five return paths across the four retrieval methods. This enables trust scoring and usage metrics to reflect actual access patterns. Closes #17899
alt-glitch
added
type/bug
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
retrieval_countcolumn in thefactstable is never incremented during normal operation, making it useless as a usage metric. All 44 facts in my store showretrieval_count = 0despite weeks of active use.Root Cause
Two search paths exist, but only
store.search_facts()increments the counter — and nothing calls it:store.search_facts()(store.py:191) — hasUPDATE facts SET retrieval_count = retrieval_count + 1. Nobody calls this method.FactRetriever.search/probe/related/reason()(retrieval.py) — the actual retrieval layer used byprefetch()andfact_storetool calls. Queries the DB directly, never increments the counter.Reported in #17899.
Fix
store.touch_facts(fact_ids)as a shared helper that incrementsretrieval_countfor given fact IDs (with lock, noop on empty list).touch_facts()in all 5 return paths across the 4 retrieval methods:search(),probe()(2 paths — bank-hit and direct),related(), andreason().The existing
search_facts()increment logic instore.pyis untouched — no behavior change for any code that does call it directly.Testing
touch_factsexists onMemoryStoreand is referenced in all 4 retrieval methods viainspect.getsource().Files Changed
plugins/memory/holographic/store.py— addtouch_facts()methodplugins/memory/holographic/retrieval.py— calltouch_facts()after scoring in all return paths