Skip to content

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

Open
spfcraze wants to merge 1 commit into
mainfrom
fix/holographic-query-hoist
Open

perf(memory): hoist loop-invariant HRR encodes out of retrieval loops#20
spfcraze wants to merge 1 commit into
mainfrom
fix/holographic-query-hoist

Conversation

@spfcraze

@spfcraze spfcraze commented Aug 1, 2026

Copy link
Copy Markdown
Owner

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):

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

FactRetriever.search() re-encoded the query vector once per candidate,
related() re-encoded both role atoms once per fact row, and probe()
re-encoded the role-content atom once per row. All three encoders are
deterministic (SHA-256 counter blocks), so the hoisted vectors are
bit-identical to the per-iteration values they replace.

Measured (300-fact store, dim=1024, median of 30 calls): 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; dominated by per-fact content encoding,
which is inherent to the algorithm and unchanged).

Tests: call-count regression tests for each hoist plus a bit-exact
parity test of search() against the pre-fix per-candidate loop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant