Skip to content

feat(skill): memory-enhanced-retrieval — entity-indexed two-stage retrieval - #49542

Draft
luxuguang-leo wants to merge 1 commit into
NousResearch:mainfrom
luxuguang-leo:feat/memory-enhanced-retrieval
Draft

feat(skill): memory-enhanced-retrieval — entity-indexed two-stage retrieval#49542
luxuguang-leo wants to merge 1 commit into
NousResearch:mainfrom
luxuguang-leo:feat/memory-enhanced-retrieval

Conversation

@luxuguang-leo

Copy link
Copy Markdown
Contributor

Summary

Adds memory-enhanced-retrieval as an optional skill under research/. This skill implements entity-indexed two-stage retrieval to improve multi-hop and cross-session recall in Hermes' session search.

Background

Based on A/B testing against MRAgent (ICML 2026, "Memory is Reconstructed, Not Retrieved") on 10 real-world conversation scenarios:

Metric Proposed MRAgent Flat RAG
Correct+Partial 100% 30% 40%
Avg time 12s 33s 4s
LLM calls 1 3 2
Stable 10/10 5/10 5/10

Design

  • Sidecar architecture: jieba + SQLite, completely independent from Hermes' state.db
  • Two-stage retrieval: first pass via entity index, second pass expands with newly discovered entities
  • Zero core changes: skill-only, hermes skills remove for full rollback
  • ~150 lines total, MIT licensed

Full report: https://github.com/luxuguang-leo/hermes-memory-ab-test

Entity-indexed two-stage retrieval for Hermes. Uses jieba + SQLite as a
sidecar to improve multi-hop and cross-session recall. Based on A/B testing
against MRAgent (ICML 2026).

Results: 100% correct+partial (vs MRAgent 30%), 12s avg (vs 33s).
Zero Hermes core changes. Full report and data at
https://github.com/luxuguang-leo/hermes-memory-ab-test
@alt-glitch alt-glitch added type/feature New feature or request tool/skills Skills system (list, view, manage) tool/memory Memory tool and memory providers P3 Low — cosmetic, nice to have labels Jun 20, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing a skill-only approach. The current patch does not yet implement the retrieval behavior it describes.

Problems

  • optional-skills/research/hermes-memory-ab-test/scripts/build_index.py:32-48 only extracts and writes entities. It contains no index query, second retrieval pass, merge, or invocation of session_search; current main discovery instead calls db.search_messages() at tools/session_search_tool.py:513.
  • build_index() clears entities at scripts/build_index.py:35, while each --add call passes a one-item list at line 48. Repeated indexing therefore removes all prior turns.
  • The default path at scripts/build_index.py:44 hardcodes ~/.hermes, conflicting with the profile-safe state-path rule in AGENTS.md:1169-1178.

Suggested changes

  • Implement and document a complete standalone two-stage retrieval command, or re-scope the skill so it does not claim to improve session_search.
  • Make indexing incremental and profile-aware, then add tests for multi-turn retention and retrieval.
  • Add the standard runnable procedure, pitfalls, verification, and script tests required for an optional skill.

Automated hermes-sweeper review.


# Memory Enhanced Retrieval

Improves `session_search` for multi-hop and cross-session queries using entity

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch does not currently improve session_search: the only script writes an index and has no query, two-stage retrieval, result merge, or invocation of the session-search path. Please either provide a complete standalone retrieval workflow here or re-scope this claim.

def build_index(conversation, db_path):
conn = sqlite3.connect(db_path)
conn.execute("CREATE TABLE IF NOT EXISTS entities (entity TEXT, turn_idx INT, content TEXT, tag TEXT)")
conn.execute("DELETE FROM entities")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deletes all indexed rows on every call. Because the CLI passes one --add value as a one-turn conversation at line 48, each subsequent indexing operation drops all prior conversation history. Use an incremental/idempotent update keyed by source message instead.


if __name__ == "__main__":
p = argparse.ArgumentParser()
p.add_argument("--db", default=str(Path.home()/".hermes/entity_index/index.db"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hardcodes the default Hermes home and bypasses named-profile isolation. Resolve this from the profile-aware Hermes home mechanism (or at least HERMES_HOME with the normal fallback) and ensure the parent directory exists before opening SQLite.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 14, 2026
@luxuguang-leo
luxuguang-leo marked this pull request as draft July 28, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/memory Memory tool and memory providers tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants