Skip to content

fix(memory): recall facts when an LLM search query contains FTS5 metasyntax - #43490

Closed
ly-wang19 wants to merge 1 commit into
NousResearch:mainfrom
ly-wang19:fix/memory-search-fts5-metasyntax
Closed

fix(memory): recall facts when an LLM search query contains FTS5 metasyntax#43490
ly-wang19 wants to merge 1 commit into
NousResearch:mainfrom
ly-wang19:fix/memory-search-fts5-metasyntax

Conversation

@ly-wang19

Copy link
Copy Markdown
Contributor

Summary

FactRetriever.search() is the path behind the LLM-facing memory search action in the holographic memory plugin (_handle_fact_actionretriever.search). It passes the raw query straight to an FTS5 MATCH in _fts_candidates():

where_clauses = ["facts_fts MATCH ?"]
params.append(query)          # raw LLM query
...
try:
    rows = conn.execute(sql, params).fetchall()
except Exception:
    return []                 # ← silently drops ALL results

LLM-issued search queries routinely contain FTS5 metasyntax — a key:value colon (project:hermes deadline), a stray double-quote, a bare AND/OR/NEAR, or a parenthesis. FTS5 MATCH raises sqlite3.OperationalError on those, so the fetch returned [] and the agent recalled no memories even when relevant facts existed — a silent correctness loss on entirely normal input.

Fix

Before giving up, retry the MATCH with each token quoted as a literal phrase:

except Exception:
    safe_query = _fts5_safe_query(query)   # 'project:hermes deadline' -> '"project:hermes" "deadline"'
    ...

The raw query is tried first, so valid FTS5 (e.g. term* prefix queries) is preserved; only a rejected query falls back to the quoted retry, which degrades to a literal-phrase match instead of returning nothing.

This applies the same try-raw-then-quote approach as #43435 (which fixes the crash in MemoryStore.search_facts, the direct-API sibling). Here the symptom is silent empty results rather than a crash, because _fts_candidates already swallowed the error into [].

Note: _fts5_safe_query is duplicated from #43435's store.py helper to keep this PR self-contained. If both land, the two copies can be hoisted into one shared util.

Tests

New tests/plugins/memory/test_holographic_retrieval_fts5.py (12 tests):

  • colon / dangling-AND / paren / NEAR( / stray-quote queries no longer return empty;
  • test_colon_query_still_recalls_matching_fact and the boolean-operator case fail without the retry (they return []) and pass with it;
  • plain and term* prefix queries are unaffected (raw is tried first);
  • _fts5_safe_query unit coverage.

…syntax

FactRetriever.search() — the LLM-facing memory `search` action — passes the raw
query straight to an FTS5 MATCH in _fts_candidates(). LLM queries routinely
contain FTS5 metasyntax (a `key:value` colon, a stray double-quote, a bare
AND/OR/NEAR, or a parenthesis); MATCH raises sqlite3.OperationalError on those,
and the candidate fetch swallowed the error and returned [] — so the agent
recalled NO memories even when relevant facts existed.

Retry the MATCH with each token quoted as a literal phrase (_fts5_safe_query)
before giving up. The raw query is tried first so valid FTS5 (e.g. `term*`
prefix queries) is preserved; only a rejected query falls back to the quoted
retry, which degrades to a literal-phrase match instead of returning nothing.

Adds tests for the colon/boolean/paren/NEAR metasyntax cases (the colon and
dangling-operator cases fail without the retry) plus the helper.
@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: LGTM

The retry-with-literal-quotes approach is sound — each whitespace-delimited token gets wrapped in double quotes (with embedded quotes doubled per FTS5 phrase syntax), turning malformed queries into safe phrase matches.

  • _fts5_safe_query handles edge cases correctly: empty input returns "", embedded " is doubled to "".
  • The params[0] = safe_query in-place mutation is safe since params is a freshly-built local list inside _fts_candidates.
  • Tests cover the key failure modes: colon syntax, stray quotes, trailing boolean operators, unbalanced parens, NEAR with no args, and mixed content+operators.
  • The prefix query test (pyth*) confirms valid FTS5 syntax is unaffected (raw MATCH tried first).

Small note: the function splits on whitespace, so multi-word phrases with spaces inside quotes (e.g., "exact phrase") from a savvy LLM would be split into individual tokens and lose the phrase semantics. This is acceptable for the stated use case (LLM-emitted queries that are typically keyword lists, not crafted FTS5).

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels Jun 10, 2026

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ Approved

The bug: _fts_candidates in holographic/retrieval.py was swallowing FTS5 sqlite3.OperationalError and silently returning [] — so the agent received no memories even when relevant facts existed.

The fix: Replace the silent-return-empty fallback with a retry using _fts5_safe_query (same token-quoting approach as PR #43435 which fixes the same pattern in store.py). The inner try/except on the retry is correct — guarantees empty return only after two real attempts fail.

Note: PR #43435 applies the same pattern to store.py's search_facts — different file, no conflict, both are wanted.

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression analysis. This has already been implemented on current main through a broader pre-MATCH sanitizer rather than a retry-after-error path.

  • plugins/memory/holographic/retrieval.py:503 now passes self._sanitize_fts_query(query) to facts_fts MATCH.
  • plugins/memory/holographic/retrieval.py:585-619 strips FTS5 metasyntax, quotes literal tokens, and OR-joins them before SQLite sees the query.
  • tests/plugins/memory/test_holographic_retrieval.py:58-72 covers FTS5-special-character sanitizer inputs.
  • Commit cb6d6d46ab6b20b173c8215a1f066b53847e9ee5 shipped this retrieval-side behavior; 638d2e7bfcad1be6e779c0d1af95481a6e92d811 applies it to the direct search_facts sibling as well.

Automated hermes-sweeper review.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants