Skip to content

fix(memory/holographic): sanitize hyphenated tokens in FTS5 queries - #14262

Open
ms-alan wants to merge 3 commits into
NousResearch:mainfrom
ms-alan:fix/14024-holographic-fts5-hyphen
Open

fix(memory/holographic): sanitize hyphenated tokens in FTS5 queries#14262
ms-alan wants to merge 3 commits into
NousResearch:mainfrom
ms-alan:fix/14024-holographic-fts5-hyphen

Conversation

@ms-alan

@ms-alan ms-alan commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Fixes 'no such column' errors when fact_store search receives hyphenated identifiers like 'pve-01' by wrapping bare hyphenated tokens in double quotes before passing to FTS5 MATCH.

Closes #14024

pander added 3 commits April 23, 2026 09:17
Fixes 'no such column' errors when fact_store search receives
hyphenated identifiers like 'pve-01' by wrapping bare hyphenated
tokens in double quotes before passing to FTS5 MATCH.

Closes NousResearch#14024
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels Apr 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing fix with #14033 for issue #14024 (FTS5 hyphen sanitization). Also touches error_classifier.py and gateway/run.py — may overlap with #14261/#14264 from same author.

@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing fix with #14033.

@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 isolating the hyphenated FTS5 failure. The identifier-retrieval problem still needs a fix on current main, but this branch predates the current shared query-normalization path.

Problems

  • plugins/memory/holographic/retrieval.py:503 now uses _sanitize_fts_query(). That helper removes - at lines 603-615, so pve-01 becomes pve01 before MATCH rather than remaining a quoted phrase.
  • plugins/memory/holographic/store.py:212-215 shares that helper for search_facts(). Updating only the retriever would leave the sibling direct-store path without a verified identifier-preserving contract.
  • The current tests only assert crash safety for hyphenated input (tests/plugins/memory/test_holographic_retrieval.py:58-72); they do not prove a stored pve-01 fact is retrieved.
  • The gateway and error-classifier commits are unrelated to this memory fix and overlap later main fixes (40dbfa0e3ce85dc0d63f1ebe1319d212c53d3511, 38e7bd8a08a9df450b7d8661778ecc47f25323c2).

Suggested changes

  • Integrate phrase preservation into the existing shared sanitizer while retaining its prose-query OR behavior.
  • Add retrieval and direct-store regressions for a fact containing pve-01.

This is an automated hermes-sweeper review.

@@ -496,7 +510,7 @@ def _fts_candidates(
# We need to join facts_fts with facts to get all columns

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.

Current main now normalizes this argument through _sanitize_fts_query() before MATCH, and that helper strips - from pve-01. Please integrate identifier phrase preservation into that existing shared sanitizer rather than applying this legacy raw-query wrapper alone.

Comment thread agent/error_classifier.py
@@ -773,6 +773,14 @@ def _classify_by_message(
should_fallback=True,

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 change is unrelated to holographic FTS5 handling and is superseded by current main's broader overload classification fix (38e7bd8a08a9df450b7d8661778ecc47f25323c2). It should not be included in a focused salvage of this memory fix.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 12, 2026
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 comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

[Bug]: fact_store search action fails with "no such column" error on hyphenated queries (FTS5 tokenization)

3 participants