feat: multi-encoder RRF retrieval (RESEARCH FEATURE, default off) - #85
Merged
Merged
Conversation
jphein
changed the title
Collaborator
Author
|
Important eval-scope clarification (caught after PR open): The end-to-end "−0.0008 MRR through What that means for the result:
The absolute MRR numbers (0.4042 baseline) should NOT be quoted as production retrieval quality. They reflect a 2k-chunk code-only corpus. A daemon-palace eval would need:
Flag stays off in prod regardless — the structural finding is unchanged. The code is still useful as a reproducible research artifact and as the RRF math substrate for any future fusion experiment. |
RESEARCH FEATURE supporting code — no behavior change in production. Add ``mempalace/rrf.py`` with three functions: * ``rrf_scores(rank_lists, key=, k=60)`` — Cormack-2009 RRF score aggregation across N ranked lists. * ``rrf_fuse(rank_lists, ...)`` — score + sort + return fused triples (identity, score, representative). Configurable representative selector for callers that want metadata preference rules. * ``explain_fusion(...)`` — diagnostic: per-identity rank table across lists, sorted by RRF score. Pure module, no deps. Used by the multi-encoder retrieval glue (follow-up commit) which is gated behind ``PALACE_USE_MULTI_ENCODER_RRF``. Refs #82. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ch feature) RESEARCH FEATURE — default off, gated by ``PALACE_USE_MULTI_ENCODER_RRF=1``. Refs #82. Add ``mempalace/multi_encoder.py``: read an encoder roster from env, for each encoder encode the query → query its bound palace → RRF-fuse the rank lists into a chromadb-shaped result dict. Downstream code (closet boost, hybrid rerank, BM25 fallback) is unchanged because the fused result has the same shape as a single chromadb query returns. Encoders: * ``default`` — built-in ONNX MiniLM, no model path needed. * Any other name — local ``SentenceTransformer`` directory passed via ``PALACE_RRF_ENCODER_PATHS``. Each encoder may have its own palace via ``PALACE_RRF_PALACES``. The production shape is "N parallel mines, one per encoder" because FT-encoded queries against ONNX-encoded drawer vectors are noise. When an encoder has no palace, the call falls back to the call-site palace with a one-shot warning (useful only for single-palace benchmarks). Defensive: one bad encoder palace or loader doesn't sink the whole query — that encoder's contribution is skipped with a warning; remaining encoders fuse normally. When every encoder fails, the caller receives an empty result and the existing BM25-fallback path fills in. ``mempalace/searcher.py`` gets a single hook at the ``drawers_col.query(...)`` call site: when ``is_enabled()`` is true, route through ``fused_query``. Default path is byte-identical to before. Hook is inside the existing try/except so a fan-out failure falls through to the existing SQLite/BM25 fallback rather than hard-failing. Tunables: * ``PALACE_RRF_K=60`` — Cormack smoothing constant. * ``PALACE_RRF_OVERFETCH=3`` — per-encoder pool multiplier. Costs: query latency ≈Nx; storage ≈Nx (one palace per encoder); ingest ≈Nx. These costs make this a research lever, not a flip-the-default candidate without further evaluation. Tests cover env parsing, RRF promotion of consensus winners, collapse of duplicate (source_file, chunk_index) across encoder palaces, one-encoder-fails-gracefully, all-encoders-fail-empty, overfetch tunable, and the env-flag hook through ``search_memories`` end-to-end. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… RRF Add ``scripts/eval_multi_encoder_rrf.py``: mine the same corpus once per encoder into temp palaces, then run a probe set through the production ``search_memories`` path twice — once with ``PALACE_USE_MULTI_ENCODER_RRF`` unset (single-encoder baseline) and once with it on. Report MRR@K, Recall@5, Recall@10, plus deltas. Differs from ``scripts/verify_rrf_ftcode5k.py`` by exercising the real production code path (not a surrogate min-rank fusion), so the numbers it produces are what we'd actually ship. Defaults to the 3-encoder roster from issue #82 (default + FT-Code-1000 + FT-Code-5000) but accepts arbitrary ``--encoders`` / ``--model-paths`` so 2-way and other rosters are one-flag changes. Add ``docs/research/2026-05-15-multi-encoder-rrf.md`` explaining why this lever exists, what this PR ships vs. doesn't, the operator surface, costs, and open questions. Refs #82. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The built-in chromadb ONNX MiniLM embedding function returns
``[np.ndarray(dtype=float32), ...]`` from a batched call, and the
shape ``list(vecs[0])`` produces is a Python list whose elements
are ``numpy.float32`` scalars.
chromadb's ``collection.query(query_embeddings=...)`` validates the
input shape against ``list[float] | list[list[float]] | numpy
array``. A list of numpy scalars is the one shape it doesn't
recognize and raises:
Expected embeddings to be a list of floats or ints, a list of
lists, a numpy array, or a list of numpy arrays, got
[[np.float32(-0.0807703), …]]
The first end-to-end eval against the n=200 git-derived probe set
silently skipped the default encoder for every query because of
this — the FT-Code SentenceTransformers happened to be fine since
they go through ``.tolist()`` already.
Fix: prefer ``ndarray.tolist()`` (the canonical numpy → Python
floats conversion), fall back to ``[float(x) for x in vec]`` for
EF implementations that return plain lists. Regression test stubs
the EF with the chromadb-shaped numpy output and asserts the
encoder yields plain ``float`` elements.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…pipeline End-to-end eval of multi-encoder RRF against the n=200 git-derived probe set, routed through ``search_memories`` (closet boost + hybrid BM25 rerank already on). Headline: | Path | MRR | R@5 | R@10 | qps | |-------------------------------------|-------:|------:|------:|-----:| | Single-encoder baseline (default) | 0.4042 | 46.5% | 49.0% | 3.85 | | 3-way RRF (default + ft-1k + ft-5k) | 0.4033 | 45.5% | 49.5% | 2.78 | | Δ | −0.0008 | −1.00 pp | +0.50 pp | 0.72x | Per-probe breakdown: * 2 rescued, 1 regressed * 5 better rank, 10 worse rank * 82 tied hits, 100 tied misses The +0.0841 raw-vector lift from issue #82 (and nakata-app's MemPalace#1384) does not survive the hybrid pipeline. Most likely explanation: the encoder-orthogonality signal that RRF exploits on raw vector retrieval is already largely captured by the production path's closet boost + BM25 rerank — whatever the encoders disagree on, BM25 catches as verbatim terminology match. Same shape of finding as the HyDE diagnosis (familiar.realm.watch#6): retrieval techniques targeting vocabulary-bridging produce smaller wins on top of an already-doing-vocabulary-bridging hybrid path than they do on raw vector retrieval. Persisted full per-probe JSON to ``docs/research/2026-05-15-rrf-eval-3way.json`` for reproducibility and follow-up analysis. Implication for the feature flag: leave default off; do not flip in production. The code lands as a research artifact — reproducible, inspectable, gated — but the measured lift on real call paths doesn't justify 3x query latency or Nx storage. Refs #82. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three small lint fixes so #85 can merge clean on the rebased branch: - scripts/verify_rrf_ftcode5k.py: f-string with no placeholders → plain string (F541) - scripts/chunk_strategy_ablation.py + scripts/verify_rrf_ftcode5k.py: ruff format trailing-comma adjustments - tests/test_multi_encoder.py: the `type(x) is float` check was deliberate (isinstance accepts np.float32 because numpy scalars subclass float — that defeats the coercion test). Reworked to collect type names into a set and compare strings, dodging E721 without losing the test's structural intent. All RRF tests pass (40/40 across test_rrf.py + test_multi_encoder.py). Co-Authored-By: jphein <jphein@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
jphein
force-pushed
the
feat/multi-encoder-rrf
branch
from
May 16, 2026 21:48
e307ee7 to
d8f81c2
Compare
jphein
added a commit
that referenced
this pull request
May 22, 2026
…tion (#101) * feat(rrf): pure Reciprocal Rank Fusion math + tests RESEARCH FEATURE supporting code — no behavior change in production. Add ``mempalace/rrf.py`` with three functions: * ``rrf_scores(rank_lists, key=, k=60)`` — Cormack-2009 RRF score aggregation across N ranked lists. * ``rrf_fuse(rank_lists, ...)`` — score + sort + return fused triples (identity, score, representative). Configurable representative selector for callers that want metadata preference rules. * ``explain_fusion(...)`` — diagnostic: per-identity rank table across lists, sorted by RRF score. Pure module, no deps. Used by the multi-encoder retrieval glue (follow-up commit) which is gated behind ``PALACE_USE_MULTI_ENCODER_RRF``. Refs #82. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(searcher,multi_encoder): query-time N-encoder RRF fusion (research feature) RESEARCH FEATURE — default off, gated by ``PALACE_USE_MULTI_ENCODER_RRF=1``. Refs #82. Add ``mempalace/multi_encoder.py``: read an encoder roster from env, for each encoder encode the query → query its bound palace → RRF-fuse the rank lists into a chromadb-shaped result dict. Downstream code (closet boost, hybrid rerank, BM25 fallback) is unchanged because the fused result has the same shape as a single chromadb query returns. Encoders: * ``default`` — built-in ONNX MiniLM, no model path needed. * Any other name — local ``SentenceTransformer`` directory passed via ``PALACE_RRF_ENCODER_PATHS``. Each encoder may have its own palace via ``PALACE_RRF_PALACES``. The production shape is "N parallel mines, one per encoder" because FT-encoded queries against ONNX-encoded drawer vectors are noise. When an encoder has no palace, the call falls back to the call-site palace with a one-shot warning (useful only for single-palace benchmarks). Defensive: one bad encoder palace or loader doesn't sink the whole query — that encoder's contribution is skipped with a warning; remaining encoders fuse normally. When every encoder fails, the caller receives an empty result and the existing BM25-fallback path fills in. ``mempalace/searcher.py`` gets a single hook at the ``drawers_col.query(...)`` call site: when ``is_enabled()`` is true, route through ``fused_query``. Default path is byte-identical to before. Hook is inside the existing try/except so a fan-out failure falls through to the existing SQLite/BM25 fallback rather than hard-failing. Tunables: * ``PALACE_RRF_K=60`` — Cormack smoothing constant. * ``PALACE_RRF_OVERFETCH=3`` — per-encoder pool multiplier. Costs: query latency ≈Nx; storage ≈Nx (one palace per encoder); ingest ≈Nx. These costs make this a research lever, not a flip-the-default candidate without further evaluation. Tests cover env parsing, RRF promotion of consensus winners, collapse of duplicate (source_file, chunk_index) across encoder palaces, one-encoder-fails-gracefully, all-encoders-fail-empty, overfetch tunable, and the env-flag hook through ``search_memories`` end-to-end. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(eval): end-to-end probe runner + research note for multi-encoder RRF Add ``scripts/eval_multi_encoder_rrf.py``: mine the same corpus once per encoder into temp palaces, then run a probe set through the production ``search_memories`` path twice — once with ``PALACE_USE_MULTI_ENCODER_RRF`` unset (single-encoder baseline) and once with it on. Report MRR@K, Recall@5, Recall@10, plus deltas. Differs from ``scripts/verify_rrf_ftcode5k.py`` by exercising the real production code path (not a surrogate min-rank fusion), so the numbers it produces are what we'd actually ship. Defaults to the 3-encoder roster from issue #82 (default + FT-Code-1000 + FT-Code-5000) but accepts arbitrary ``--encoders`` / ``--model-paths`` so 2-way and other rosters are one-flag changes. Add ``docs/research/2026-05-15-multi-encoder-rrf.md`` explaining why this lever exists, what this PR ships vs. doesn't, the operator surface, costs, and open questions. Refs #82. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(multi_encoder): coerce ONNX EF numpy output to plain floats The built-in chromadb ONNX MiniLM embedding function returns ``[np.ndarray(dtype=float32), ...]`` from a batched call, and the shape ``list(vecs[0])`` produces is a Python list whose elements are ``numpy.float32`` scalars. chromadb's ``collection.query(query_embeddings=...)`` validates the input shape against ``list[float] | list[list[float]] | numpy array``. A list of numpy scalars is the one shape it doesn't recognize and raises: Expected embeddings to be a list of floats or ints, a list of lists, a numpy array, or a list of numpy arrays, got [[np.float32(-0.0807703), …]] The first end-to-end eval against the n=200 git-derived probe set silently skipped the default encoder for every query because of this — the FT-Code SentenceTransformers happened to be fine since they go through ``.tolist()`` already. Fix: prefer ``ndarray.tolist()`` (the canonical numpy → Python floats conversion), fall back to ``[float(x) for x in vec]`` for EF implementations that return plain lists. Regression test stubs the EF with the chromadb-shaped numpy output and asserts the encoder yields plain ``float`` elements. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(research): record n=200 eval — 3-way RRF is flat through hybrid pipeline End-to-end eval of multi-encoder RRF against the n=200 git-derived probe set, routed through ``search_memories`` (closet boost + hybrid BM25 rerank already on). Headline: | Path | MRR | R@5 | R@10 | qps | |-------------------------------------|-------:|------:|------:|-----:| | Single-encoder baseline (default) | 0.4042 | 46.5% | 49.0% | 3.85 | | 3-way RRF (default + ft-1k + ft-5k) | 0.4033 | 45.5% | 49.5% | 2.78 | | Δ | −0.0008 | −1.00 pp | +0.50 pp | 0.72x | Per-probe breakdown: * 2 rescued, 1 regressed * 5 better rank, 10 worse rank * 82 tied hits, 100 tied misses The +0.0841 raw-vector lift from issue #82 (and nakata-app's MemPalace#1384) does not survive the hybrid pipeline. Most likely explanation: the encoder-orthogonality signal that RRF exploits on raw vector retrieval is already largely captured by the production path's closet boost + BM25 rerank — whatever the encoders disagree on, BM25 catches as verbatim terminology match. Same shape of finding as the HyDE diagnosis (familiar.realm.watch#6): retrieval techniques targeting vocabulary-bridging produce smaller wins on top of an already-doing-vocabulary-bridging hybrid path than they do on raw vector retrieval. Persisted full per-probe JSON to ``docs/research/2026-05-15-rrf-eval-3way.json`` for reproducibility and follow-up analysis. Implication for the feature flag: leave default off; do not flip in production. The code lands as a research artifact — reproducible, inspectable, gated — but the measured lift on real call paths doesn't justify 3x query latency or Nx storage. Refs #82. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(rrf): ruff lint passthrough — drop stray f-string, dodge E721 Three small lint fixes so #85 can merge clean on the rebased branch: - scripts/verify_rrf_ftcode5k.py: f-string with no placeholders → plain string (F541) - scripts/chunk_strategy_ablation.py + scripts/verify_rrf_ftcode5k.py: ruff format trailing-comma adjustments - tests/test_multi_encoder.py: the `type(x) is float` check was deliberate (isinstance accepts np.float32 because numpy scalars subclass float — that defeats the coercion test). Reworked to collect type names into a set and compare strings, dodging E721 without losing the test's structural intent. All RRF tests pass (40/40 across test_rrf.py + test_multi_encoder.py). Co-Authored-By: jphein <jphein@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(kg-age): fill API gaps — add_entity, invalidate, query_entity, query_relationship, timeline, seed_from_entity_facts Brings KnowledgeGraphAGE to API parity with the SQLite KnowledgeGraph. Previously only had: add_triple, query_triples, stats, clear. The 5 missing methods make AGE a drop-in replacement for the SQLite backend without requiring callsite changes. All methods mirror SQLite semantics exactly: - add_entity: MERGE pattern with type + properties (no ON CREATE SET in AGE — last-write-wins semantics, acceptable for write-through use) - invalidate: SET valid_to on every active matching triple, with inverted-interval guard - query_entity: outgoing/incoming/both direction filter + as_of temporal filter - query_relationship: filter by predicate + as_of - timeline: chronological ORDER BY, default limit 100 - seed_from_entity_facts: bulk-load from ENTITY_FACTS dict - _entity_id: name → canonical id helper, matches SQLite KG derivation All built on the existing _run_cypher infrastructure with the inlined- parameter approach that works around AGE's prepared-statement incompatibility. AGE Cypher dialect gaps respected: - No ON CREATE SET → unconditional SET on MERGE - No multi-column RETURN inside cypher() — works here because each method's RETURN uses AS-aliased columns that _extract_return_aliases parses - No list literals — not used anywhere in these implementations Smoke-tested end-to-end against sme_lme_bench: - add 3 triples (Atakan -[works_on]-> adaptmem/mempalace-PRs, FT-300 -[trained_by]-> Atakan) - query_entity('Atakan', 'outgoing') → 2 results, both current=True - query_entity('Atakan', 'incoming') → 1 result (FT-300 trained_by) - invalidate('Atakan', 'works_on', 'mempalace-PRs') → 1 affected, then re-query shows valid_to=2026-05-17, current=False - timeline() → 3 rows ordered by valid_from (NULL at end) - stats → entities=4, triples=3, current_facts=2, expired_facts=1, relationship_types=['trained_by', 'works_on'] Phase 1 of the larger AGE-integration plan toward "agent walks the palace" via real Cypher traversal. * feat(kg-age): write-through middleware — entities populate AGE on every drawer write Phase 2 of /goal: AGE integration. Every PostgresCollection upsert/add now optionally fires a KG write-through hook that extracts entities from the document and creates (drawer_id -[mentions]-> entity) triples in AGE. Plumbing: - PostgresCollection._insert_rows: after the row commits, calls self._kg_writethrough(drawer_id, document, metadata) if registered. Hook errors caught + logged, never raised — KG enrichment is opportunistic, never blocks writes. - PostgresCollection.set_kg_writethrough(hook): registration API. Default (no hook) is zero overhead — vector-only behavior unchanged. New module mempalace/kg_writethrough.py: - make_age_writethrough(kg, extractor) — canonical hook factory. For each drawer, extracts entities, caps at max_entities_per_drawer (default 100), creates one add_triple per entity. - make_null_writethrough() — no-op for tests / disabling. - make_writethrough_from_env() — env-var-driven config: MEMPALACE_KG_WRITETHROUGH=1 + MEMPALACE_KG_EXTRACTOR=regex|null - _builtin_regex_extractor — fallback when SME's regex extractor isn't importable. Captures capitalized proper nouns + hyphenated identifiers + version strings. Extractor is pluggable: any callable matching (text) -> list[Entity] where Entity has .name works. Tested with sme.extractors.regex.extract. Smoke test on sme_lme_bench: - Fresh AGE graph + fresh PostgresCollection + registered hook - coll.upsert(2 drawers about Atakan/FT-300/AGE/mempalace-Phase-2) - KG state after: 10 entities, 8 mentions edges, all current - d1 → mentions → {atakan, longmemeval, apache, nakata-app, ft-300} - d2 → mentions → {age, postgrescollection, phase-2} Phase 2 done. Next: Phase 3 — palace structure (Wing/Room/Drawer) as AGE nodes so /graph queries become Cypher MATCH on real graph structure. * feat(palace-age): Wing/Room/Drawer structure as native AGE nodes Phase 3 of /goal: AGE integration. Mirrors mempalace.palace_graph's SQL-aggregation pattern into AGE so Cypher MATCH can walk the palace structure natively — no SQL aggregation per query. New module mempalace/palace_graph_age.py: - populate_from_postgres(kg, dsn, table_name, skip_drawers, skip_tunnels): Reads drawer table, builds Wing/Room/Drawer/SHARED_VIA in AGE. Idempotent via MERGE. Three-pass design (wings → rooms+contains → drawers+contains → tunnels) so skip_drawers can give a fast "high-level palace map" without per-drawer cost on huge palaces. - walk_wing(kg, wing, depth): Returns a structured walk of the wing's contents. depth=1 returns rooms; depth=2 adds drawers; depth=3 joins MENTIONS edges back to entities from the kg_writethrough layer. This is the "agent walks the palace" primitive. - list_wings, list_rooms_in_wing, list_drawers_in_room, tunnels_from_wing: read-side helpers, idiomatic Cypher patterns ready for MCP tool wiring in Phase 6. Cypher schema: Wing -[:CONTAINS]-> Room -[:CONTAINS]-> Drawer -[:MENTIONS]-> Entity Wing -[:SHARED_VIA {via_room}]- Wing (tunnels: rooms in multiple wings) The CONTAINS + MENTIONS edges connect the palace structure (Phase 3) to the KG triples (Phase 1) and the kg_writethrough layer (Phase 2) into one unified graph that an agent or RLM can navigate. Smoke-test on sme_lme_bench (5344 drawers across 2 wings: code/docs): - populate(skip_drawers=True) → wings=2, rooms=237, contains_edges=238, shared_via_edges=1 (one tunnel — the 'cli' room appears in both wings) - list_wings() → ['code', 'docs'] - list_rooms_in_wing('code')[:10] → ['__init__', '__main__', '_stdio', 'base', 'basic_mining', ...] - tunnels_from_wing('code') → [{'to_wing': 'docs', 'via_room': 'cli'}] Next: Phase 4 — backfill production palace (274K drawers) AGE graph. * feat(kg-age): add_mention + backfill_age + edge-type-union walk fix Builds out Phase 4 (backfill script) and tightens Phases 1-3: KnowledgeGraphAGE.add_mention(): New method that connects Drawer nodes (palace structure) to Entity nodes via :MENTIONS edges, distinct from add_triple's Entity-RELATION-Entity pattern. Lets the write-through layer (Phase 2) populate the kg in the same shape the read-side walk (walk_wing, MCP tools) expects. AGE 1.6.0 dialect gaps documented + worked around: - No SET on edge properties inline (errors at '=') - No ON CREATE SET - No coalesce() in SET - No edge-type union [:A|B] in MATCH CREATE-always edge semantics matches the SQLite KG's triples-table behavior (no upsert on triples; multiple add_triple calls create parallel rows). Aggregate at read time via count(r). palace_graph_age.walk_wing(): Fixed to use :RELATION + property filter instead of [:MENTIONS|RELATION] union — AGE doesn't parse the union syntax. kg_writethrough.make_age_writethrough(): Switched from add_triple to add_mention so write-through populates the Drawer-MENTIONS-Entity graph the read side expects. backfill_age.py (Phase 4): CLI + library entry point for one-shot AGE-graph population from existing drawer table. Restartable via mempalace_kg_backfill_state checkpoint table. Stream-based via psycopg2 named cursor (bounded memory). Configurable scope (--wing, --skip-palace, --skip-entities, --extractor). Default extractor is SME's regex extractor (with builtin fallback when SME isn't importable). Smoke test on sme_lme_bench: - populate_from_postgres(skip_drawers=True) → 2 wings, 237 rooms, 238 CONTAINS, 1 SHARED_VIA tunnel - backfill (docs wing only, max 20 entities/drawer) → 1181 drawers processed → 6015 entities + 13721 mentions in 5.85 min (~3.4 drawers/s; projects to ~22h for the prod 274K palace) - add_mention parallel-edge test: 3 calls (1 duplicate) → 2 edges for Atakan, 1 for adaptmem; aggregate count(r) recovers true count * feat(mcp): mempalace_walk_palace — agent walks the palace via AGE Cypher Phase 6 of /goal: AGE integration. The "agent walks into the palace finding wings, rooms, drawers" metaphor becomes a real MCP tool over the unified palace+entity graph (Wing → Room → Drawer → MENTIONS → Entity) built in Phases 1-4. Three traversal modes, exactly one anchor required: start_wing="memorypalace" → walks DOWN the structure depth=1: rooms in this wing depth=2: + drawers in those rooms depth=3: + entities those drawers mention start_room="problems" → walks DOWN from a room (across all wings) depth=1: drawers in this room (any wing) depth=2: + entities mentioned start_entity="pgvector" → walks UP from an entity (inverse walk) depth=1: drawers that mention this entity depth=2: + the rooms+wings containing those drawers Returns: { "start": {"wing": ..., "room": ..., "entity": ...}, "depth": N, "walk": [{wing, room, drawer, entity}, ...], "stats": {wings_touched, rooms_touched, drawers_touched, entities_touched} } Requires MEMPALACE_BACKEND=postgres and the AGE graph populated via mempalace.kg_writethrough or mempalace.backfill_age. Smoke-tested on sme_lme_bench: - walk_palace(start_entity='pgvector', depth=2) returns the 3 drawers mentioning it (postgres.py, CHANGELOG.md, BENCHMARKS.md), plus their containing rooms+wings (postgres+code, CHANGELOG+docs, BENCHMARKS+docs) - walk_palace(start_room='postgres', depth=2) returns the postgres.py drawer plus its 3 mentioned entities (pgvector, hnsw, Apache AGE) - walk_palace(start_wing='docs', depth=3) walks into the docs wing's rooms, drawers, and entity layer This completes the 6-phase plan that started from today's spike result (+9pp R@5 from AGE entity-overlap fused with vector). The metaphor of the AI walking the palace is now real Cypher traversal — anyone with the MCP tool can ask "where does pgvector get discussed?" or "what's in the memorypalace wing?" and get a structured answer. * docs: add 2026-05-17 AGE-integration phase changelog entries Documents the 5 fork-side commits in feat/age-kg-parity branch in the canonical fork-changes.yaml + regenerated FORK_CHANGELOG.md. Each phase gets a full body explaining what landed, why it matters, what was tested, and which files changed: - Phase 1 (ff7187d): KnowledgeGraphAGE API parity - Phase 2 (3321d83): write-through middleware in PostgresCollection - Phase 3 (ff583c0): palace structure as native AGE nodes - Phase 4 (b3f0206): add_mention + backfill_age + walk_wing fix - Phase 6 (8022ecb): mempalace_walk_palace MCP tool Phase 5 (search-side fusion) lives on the palace-daemon repo; its changelog gets a parallel entry there. Note: render-docs.py regenerated FORK_CHANGELOG.md from fork-changes.yaml per the file's own instructions. * chore(gitignore): exclude benchmarks/data/ (LongMemEval download artifacts) The LongMemEval cleaned datasets (longmemeval_s_cleaned.json ~261MB, longmemeval_oracle.json ~15MB) get pulled per-environment when running benchmarks/longmemeval_bench.py. They should never be committed — git LFS or off-band data hosting is the right home, and the download instructions live in the bench script docstring. * chore(ci): add .gemini/config.yaml — swap Copilot Code Review for Gemini Code Assist Copilot Code Review has been failing 10+ times in last 20 runs since weekly Copilot credit ran out. Gemini Code Assist is free for public repos (which all three techempower-org forks are) and already actively reviews on MemPalace upstream (saw it on PR MemPalace#1525). Config is tuned for an active research fork: - MEDIUM severity threshold — filter low-severity noise - summary + code_review on PR open; help disabled - have_fun: false — no emoji in review comments - Ignore patterns: benchmark result JSONs, generated FORK_CHANGELOG.md (yaml is source of truth), v33x snapshot dirs, dashboards, lock files, build artifacts Install Gemini Code Assist at https://github.com/apps/gemini-code-assist on techempower-org org; config picks up automatically. Disable Copilot Code Review in repo Settings → Code & automation → Copilot review to stop the failure notification flood. AGENTS.md (already in repo at /AGENTS.md) provides project context to Gemini for code review — no separate GEMINI.md needed. * docs(changelog,readme): surface 2026-05-17 AGE-integration phases + upstream/develop merge CHANGELOG.md: new [Unreleased] 2026-05-17 section above the existing 2026-05-14/15 cutover section. Documents the 6-phase AGE integration (commits ff7187d, 3321d83, ff583c0, b3f0206, 8022ecb) with the +9pp R@5 spike result that motivated it. Plus the upstream/develop merge (6058489 / 342a59f) with the 12-file conflict resolution summary. README.md: new "Walking the palace — Apache AGE integration (2026-05-17)" section above the per-axis ship-list. Six-phase table, unified graph schema, walk_palace primitive shape, AGE Cypher dialect gap inventory, cross-fork verification note from nakata-app's independent audit. Links PR #101 (this fork) and palace-daemon PR #25 (read-side fusion). * fix(age-kg): address Gemini-flagged SQLi/perf in PR #101 review Four high-severity issues from the Gemini Code Assist review of `#101` (2026-05-17): knowledge_graph_age: - Single-pass `re.sub` param inlining via `_inline_cypher_params` replaces the length-sorted `str.replace` loop. Values containing `$other_key` no longer trigger a recursive substitution. - Outer dollar-quote tagged `$mp_age_q$...$mp_age_q$` instead of bare `$$`; `_cypher_literal` rejects any string containing the tag so a sanitized-but-adversarial value can't close the SQL boundary. - New `commit=False` kwarg on `_run_cypher` / `_cypher_scalar` / `add_mention` plus a `kg.commit()` helper for batched writes. backfill_age: - `_validate_pg_identifier()` whitelists `[A-Za-z_][A-Za-z0-9_]*` before the table name is f-stringed into the SQL. - Drawer-scan loop now batches `commit_every=100` (CLI flag `--commit-every`) instead of committing per drawer; checkpoint marks are written only after the matching KG commit succeeds. Tests: 163 passing in `tests/ -k 'age or kg or knowledge_graph'`, 109 passing in `-k 'backfill or kg or knowledge_graph or writethrough'`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(age-kg): address Gemini-flagged mediums in PR #101 review Three medium-priority findings from the Gemini Code Assist review of ``#101`` (2026-05-17), follow-up to 551821b which addressed the security-critical / high-priority items. - **multi_encoder.py:_doc_key**: switched from ``hash(doc)`` to ``hashlib.sha256(doc.encode()).hexdigest()[:16]``. Python's built-in ``hash()`` randomizes per-process (PYTHONHASHSEED), so palace-daemon and the MCP server would assign different chunk identities to the same content, breaking RRF fusion. 16 hex chars = 64 bits, plenty of namespace for chunk-collision safety. - **mcp_server.tool_walk_palace**: now uses ``_get_kg()`` (the cached per-DSN factory) instead of directly instantiating ``KnowledgeGraphAGE(dsn)``. Each direct call opened a fresh postgres connection; with the cache, walk_palace reuses the daemon-shared connection. Added an explicit ``kg_backend == "age"`` guard so wrong-backend invocations fail cleanly instead of silently returning a sqlite KG. - **backends/postgres._insert_rows**: stopped re-parsing ``json.dumps(metadata)`` back into a dict for the KG write-through hook. Now tracks a parallel ``metadata_by_id`` map in the row-build loop that holds the *post-pop* metadata (wing/room already removed) — matches the prior contract byte-identically while skipping the round-trip through ``json.loads``. Tests: 190 passing in tests/ -k 'age or kg or knowledge_graph or multi_encoder or writethrough'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: complete ruff format pass — missed mcp_server.py in 082a890 * fix(age-kg): remove 'Phase N' jargon + add mempalace_walk_palace docs Closes the two fork-specific CI failures on PR #101: 1. ``test_no_internal_coordination_jargon_in_source_or_tests`` The fork's source/tests lint test forbids 'Phase N' / 'Task N.X.Y' internal-coordination references; three leaks remained: - ``mempalace/kg_writethrough.py:3`` — module docstring - ``mempalace/palace_graph_age.py:3, 236`` — module docstring + comment - ``mempalace/backends/postgres.py:295`` — KG write-through comment Replaced with feature-named phrasing ("inline KG enrichment", "AGE-integration inline enrichment"). Per feedback_no_phase_jargon_in_source.md / feedback_apply_naming_decision_actively.md. 2. ``test_no_undocumented_tools`` ``mempalace_walk_palace`` was wired into TOOLS but not yet in ``website/reference/mcp-tools.md``. Added a Navigation Tools section covering the three start-anchor modes (wing/room/entity), the depth and limit knobs, and the AGE-backend prerequisite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(age-kg): unit-level coverage for AGE-KG modules — no postgres required 53 tests targeting the pure-function and mock-friendly slices of the four AGE-KG modules. Closes the coverage gap that was blocking PR #101 merge (was 76.97%, target 79%). Coverage delta (module-level, isolated): - kg_writethrough.py 0% → 97% - palace_graph_age.py 0% → 30% (populate_from_postgres needs live AGE) - backfill_age.py 0% → 29% (backfill() needs live AGE) - knowledge_graph_age.py 20% → 33% (write/query class methods need live AGE) What's tested (all without DSN-dependent paths): - _cypher_literal: ValueError on outer dollar-quote tag injection; json.dumps fallback for non-string values. - _inline_cypher_params: named-placeholder substitution; unknown placeholders pass through; values containing $other_key are NOT recursively replaced (Gemini PR #101 fix regression guard); identity on no-placeholder cypher. - KnowledgeGraphAGE class helpers (via __new__, no postgres): _entity_id, _unwrap_agtype, _extract_return_aliases. - KnowledgeGraphAGE __init__/close/clear with a fake psycopg2 module + recording-cursor stand-in: bootstrap SQL is right, graph create is skipped when present, close+context-manager work, clear drop_graph fires only when graph exists. - kg_writethrough: _builtin_regex_extractor (proper nouns, tech idents, version strings, repeated-count via Counter, empty input); make_null_writethrough; make_age_writethrough (call-through to add_mention, swallows extractor and KG errors, respects max_entities_per_drawer); make_writethrough_from_env (disabled, requires-kg guard, null branch, unknown-extractor error, regex default). - backfill_age: _validate_pg_identifier (positive + 5 negative cases + non-string rejection); _get_extractor (regex fallback shape + unknown-name error); main() CLI argparse paths (missing --dsn exits non-zero, invalid table name rejected). - palace_graph_age: walk_wing parameter wiring, list_wings, list_rooms_in_wing, list_drawers_in_room, tunnels_from_wing (all with a mock KG that returns shaped row tuples). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: collapse implicit string concat in backfill_age ValueError (ruff 0.15.9) * test(age-kg): close the last coverage gap — 6 more checkpoint/empty-extractor tests Local coverage was 78.99% (one shy of 79%). Added tests for: - kg_writethrough.make_age_writethrough early-return when the extractor returns no entities (covers the previously-uncovered line 104). - backfill_age._ensure_checkpoint_table / _checkpoint_done / _checkpoint_mark / _checkpoint_clear via a recording-cursor connection mock — covers ~30 lines that previously needed live postgres. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: jphein <jphein@users.noreply.github.com>
This was referenced May 28, 2026
jphein
added a commit
that referenced
this pull request
May 28, 2026
…o mempalace package (#290) PR #279 wired canonical-predicate mapping into ``kg_triple_worker`` via a bare top-level ``from kg_canonical_writepass import …`` that relied on palace-daemon's source being on ``PYTHONPATH``. That assumption collides with ``mempalace/__init__.py:_strip_leaked_pythonpath_from_sys_path()``, which removes PYTHONPATH-derived entries from ``sys.path`` for ABI hygiene **before** the worker's top-level import runs. The result was a silent ``ImportError`` that fell through to the identity stub: the PALACE_KG_CANONICAL_MAPPING flag could be flipped on, yet ``map_for_write`` would still pass the raw predicate through and no ``raw_relation_type`` ever landed on RELATION edges. A ``.pth`` workaround patched familiar but the underlying coupling remained. This change ports the three canonical-mapping modules into the ``mempalace`` package itself so the import becomes package-relative and is no longer affected by the PYTHONPATH strip: * ``mempalace/kg_predicate_norm.py`` — surface-form normalizer * ``mempalace/kg_canonical_vocab.py`` — ``CanonicalMapper`` (incl. the batched ``map_predicates`` API from palace-daemon #85) * ``mempalace/kg_canonical_writepass.py`` — env-gated ``map_for_write`` ``kg_triple_worker.py`` now does ``from .kg_canonical_writepass import MappedPredicate, map_for_write``. The ``try/except ImportError`` stub stays as defense-in-depth for mid-migration deploys but is no longer the only path that resolves in clean environments. A new regression test ``test_canonical_mapping_import_survives_pythonpath_strip`` spawns a subprocess with ``PYTHONPATH`` explicitly cleared and asserts that ``kg_triple_worker.MappedPredicate is kg_canonical_writepass.MappedPredicate`` and that ``mapping_enabled()`` returns ``True`` under the flag — i.e. the worker really is using the real implementation, not the identity stub. Test count: 3572 → 3629 (+57: 56 ported from palace-daemon + 1 new regression). Full suite passes (one pre-existing ``test_init_filters_sys_path_from_leaked_pythonpath`` flake is unrelated; verified on clean ``main``). A follow-up issue will be filed against ``techempower-org/palace-daemon`` to convert its top-level ``kg_canonical_*`` / ``kg_predicate_norm`` copies into shims that re-export from ``mempalace.*``, so there is a single source of truth. The ``.pth`` workaround on familiar can be removed once both PRs land. Closes #281 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
RESEARCH FEATURE — default off, gated behind
PALACE_USE_MULTI_ENCODER_RRF=1.Implements query-time fusion of N encoder-bound vector indexes via Reciprocal Rank Fusion (Cormack 2009, k=60). The motivating result was the +0.0841 MRR lift measured in issue #82 when fusing default ONNX MiniLM + adaptmem FT-Code-1000 + FT-Code-5000 against the n=200 git-derived probe set on a raw chromadb-vector path.
This PR ships the production code path AND a full end-to-end eval — and the eval finding is important: the raw-vector lift does not survive the existing hybrid pipeline. The PR still lands the code as a research artifact (reproducible, inspectable, gated) but the eval says do not flip the flag on in production.
What this PR ships
mempalace/rrf.py— pure RRF math (no deps).rrf_scores,rrf_fuse,explain_fusion. 13 unit tests.mempalace/multi_encoder.py— encoder roster from env, fan-out to N encoder-bound palaces, RRF fusion, returns a chromadb-shaped result so downstream code (closet boost, hybrid rerank, BM25 fallback) is byte-identical. Defensive — per-encoder failures are logged and skipped, the remaining encoders fuse normally; all-fail returns empty so the existing BM25-fallback path catches.mempalace/searcher.py— one hook at the existingdrawers_col.query(...)call site. Default path is unchanged.scripts/eval_multi_encoder_rrf.py— eval harness that mines one temp palace per encoder, then runs probes through the productionsearch_memoriespath with the env flag off vs on.docs/research/2026-05-15-multi-encoder-rrf.md— design, findings, open questions.Plus a regression fix: chromadb's ONNX EF returns
numpy.ndarray; chromadb'squery(query_embeddings=...)rejects a list-of-numpy-scalars shape. Coerce via.tolist(). First eval run silently skipped the default encoder on every probe because of this — the regression test intest_multi_encoder.py::test_default_encoder_returns_plain_floatsasserts the shape exactly.Operator surface
Before / after (n=200 git-derived probes, through
search_memories)Per-probe (200): 2 rescued, 1 regressed, 5 better rank, 10 worse rank, 82 tied hits, 100 tied misses.
Issue #82's +0.0841 raw-vector lift does not survive the production pipeline. The closet boost + hybrid BM25 rerank that runs on top of vector retrieval is already capturing most of the encoder-orthogonality signal that 3-way RRF exploits on a raw vector path. Tools that improve raw vector retrieval do not automatically improve
search_memoriesretrieval — same shape of finding as the HyDE diagnosis at familiar.realm.watch#6.Full per-probe JSON at
docs/research/2026-05-15-rrf-eval-3way.json.Cost (per query, with flag on)
How to enable / how to verify
What this PR does NOT ship
Test plan
pytest tests/test_rrf.py— 13 passpytest tests/test_multi_encoder.py— 27 passpytest tests/test_searcher.py tests/test_hybrid_search.py tests/test_hybrid_candidate_union.py— 47 pass, no regressionsdocs/research/2026-05-15-rrf-eval-3way.jsonruff checkandruff format --checkpassRefs techempower-org/mempalace#82.
JP — please review + merge; not merging this myself per the work brief.
🤖 Generated with Claude Code