perf(memory): hoist loop-invariant HRR encodes out of retrieval loops

[spfcraze]

What does this PR do?

Hoist loop-invariant HRR vector encodes out of FactRetriever hot loops. search() re-encoded the query vector once per candidate (30x redundant work per user message), related() re-encoded both role atoms per fact row (601 encode_atom calls where 3 suffice), and probe() re-encoded the role-content atom per row. All encoders are deterministic (SHA-256 counter blocks), so hoisting is bit-identical.

Related Issue

No direct issue — discovered via code review and reproduced live (see below).
Related PRs reviewed during the duplicate check (none covers this change):

• Feat/hybrid skill retrieval #64528 [open] Feat/hybrid skill retrieval
• fix(memory/holographic): wire retrieval_count into all retrieval paths via locked store helper #73901 [open] fix(memory/holographic): wire retrieval_count into all retrieval paths via locked store helper
• feat(skill): memory-enhanced-retrieval — entity-indexed two-stage retrieval #49542 [open] feat(skill): memory-enhanced-retrieval — entity-indexed two-stage retrieval
• feat(honcho): expose representation retrieval knobs #72076 [open] feat(honcho): expose representation retrieval knobs
• fix(agent): respect permitted web retrieval guidance #39905 [open] fix(agent): respect permitted web retrieval guidance
• feat(skills): add targeted skill_view retrieval #49503 [open] feat(skills): add targeted skill_view retrieval
• fix: prefer holographic entity graph retrieval #26758 [open] fix: prefer holographic entity graph retrieval
• fix: harden retrieval routing and runtime hints #29143 [open] fix: harden retrieval routing and runtime hints
• …and 29 more reviewed in the sweep

Changes Made

• fix/holographic-query-hoist — 2 file(s) changed vs base:
  • plugins/memory/holographic/retrieval.py
  • tests/plugins/memory/test_holographic_retrieval.py

plugins/memory/holographic/retrieval.py: 3 hoists (search query_vec, probe role_content, related role_entity+role_content), ~10 lines moved, zero behavior change. tests/plugins/memory/test_holographic_retrieval.py: +5 tests — encode determinism, call-count regression tests for each hoist (search: encode_text==1; related: role atoms==2; probe: role_content==1), and a bit-exact parity test of search() against the pre-fix per-candidate loop.

How to Test

Measured on a 300-fact store (dim=1024, median of 30 calls, repo venv): search() 11.62 -> 1.46 ms/call (8.0x; encode_text 30 -> 1 per call); related() 63.08 -> 16.17 ms/call (3.9x; encode_atom 601 -> 3 per call); probe() 431.93 -> 389.36 ms/call (1.1x — probe is dominated by per-fact CONTENT encoding, inherent to the algorithm and intentionally unchanged; only its redundant role-atom re-encode was hoisted).

Validation completed (recorded by prp):

1. Sabotage check: pre-fix code fails the regression tests (3 failed), with the fix all pass (12 passed, 0 failed) — target tests/plugins/memory/test_holographic_retrieval.py.
2. Suite tests/plugins/memory/: branch 233 passed / 0 failed vs baseline 228 passed / 0 failed — zero branch-only failures.
3. Targeted fullcheck: tests/plugins/memory/ 233 passed vs 228 baseline, zero branch-only failures. Sabotage revert-verified: the 3 call-count regression tests fail on pre-fix main, 12/12 pass with the fix. Full repo-wide suite NOT run locally for this PR (skipped by decision; CI owns full-suite validation).
4. Duplicate check: 37 potential matches reviewed — none covers this change.
5. The full repo-wide suite was not run for this change; GitHub CI owns full-suite validation.

Logs

Sabotage verification output:

# base leg (pre-fix code + branch tests):
#   tests: 9 passed, 3 failed
# head leg (with fix):
#   tests: 12 passed, 0 failed

[teknium1]

Thanks for the focused performance cleanup. The current main premise is valid: search() encodes at plugins/memory/holographic/retrieval.py:86 per HRR candidate, while probe() and related() repeat role-atom encodes at lines 183 and 246-247.

Problems

• plugins/memory/holographic/retrieval.py:76 eagerly encodes the query whenever hrr_weight > 0, even if no candidate has an HRR vector. Current main only encodes beneath fact.get("hrr_vector") (plugins/memory/holographic/retrieval.py:84-86). This matters for migrated databases: MemoryStore._init_db() can add hrr_vector without backfilling existing facts (plugins/memory/holographic/store.py:178-182).

Suggested changes

• Guard the hoisted query encode on the presence of at least one candidate HRR vector, and add a NULL-vector candidate regression test asserting encode_text is not called.

Automated hermes-sweeper review.

[teknium1]

Please defer this encode unless at least one candidate has hrr_vector. Current main only calls encode_text in that branch; migrated stores can contain FTS candidates with NULL vectors because _init_db() adds the column without backfilling existing facts.

[spfcraze]

Fixed in da9e682 (amended, force-pushed). The hoist is now lazy: query_vec starts as None and the encode happens on the first candidate that actually carries an HRR vector, so a store whose facts were never backfilled (the _init_db migration case at store.py:178-182) never pays for an encode nothing will use — matching pre-fix behavior for that population exactly.

Regression test added as suggested: test_search_without_vectors_never_encodes nulls every candidate's hrr_vector, runs search(), and asserts encode_text was never called (results still return via the neutral hrr_sim=0.5 path).

Hot case unchanged: vector-bearing stores still encode exactly once per search() (call-count test: 30 candidates → 1 encode), and the benchmark is flat (1.51 ms/call vs 1.46 ms pre-amend — noise). Re-verified after the amend: 13/13 in the retrieval suite, sabotage (base 3 fail / head 13 pass), and zero branch-only failures vs baseline in tests/plugins/memory/.

[kshitijk4poor]

Thanks @spfcraze — hoists verified sound (deterministic encodes, parity-pinned) and salvaged into #76881 with your authorship preserved via cherry-pick, plus one test fix: the MemoryStore(":memory:") fixture actually created a literal ./:memory: file that leaked state across runs. Closing in favor of the salvage.