Skip to content

fix(state): honor AND/NOT operators in the CJK LIKE fallback - #64559

Open
solyanviktor-star wants to merge 1 commit into
NousResearch:mainfrom
solyanviktor-star:fix/cjk-like-fallback-boolean
Open

fix(state): honor AND/NOT operators in the CJK LIKE fallback#64559
solyanviktor-star wants to merge 1 commit into
NousResearch:mainfrom
solyanviktor-star:fix/cjk-like-fallback-boolean

Conversation

@solyanviktor-star

Copy link
Copy Markdown
Contributor

What does this PR do?

SessionDB.search_messages routes short-token CJK queries (any non-operator CJK token < 3 chars) to a LIKE-based fallback, because the trigram FTS5 index can't match them. That fallback joins every non-operator token with OR, regardless of the operators the query actually used:

non_op_tokens = [t for t in raw_query.split() if t.upper() not in {"AND", "OR", "NOT"}] or [raw_query]
...
like_where = [f"({' OR '.join(token_clauses)})"]

The trigram path forwards AND/OR/NOT verbatim to FTS5, so the same query silently flips meaning depending on token length:

  • 报告 NOT 草稿 ("report NOT draft") — the NOT term becomes a positive OR branch, so the search returns exactly the drafts it asked to exclude;
  • 广西 AND 桂林 degrades to 广西 OR 桂林 and matches messages containing either term;
  • implicit adjacency (广西 桂林, which FTS5 treats as AND) also degrades to OR.

The fix groups tokens into OR-separated buckets of positive and NOT-negated terms: whitespace/AND joins conjunctively within a bucket, OR splits buckets, NOT negates the following term. That matches FTS5 operator precedence for these flat queries (OR binds loosest), so the LIKE fallback now agrees with the trigram path. Two supporting details:

  • the per-term LIKE now COALESCEs the nullable tool_name/tool_calls columns — without that, a negated term evaluates to NULL (= non-matching) on every row without tool metadata, and NOT would wrongly exclude all plain chat messages;
  • the instr() snippet anchor uses the first positive term instead of the first token, so excluded terms don't drive snippet positioning.

The OR behavior introduced for #20494 is preserved — its regression tests (test_cjk_or_combined_short_tokens_returns_results, test_cjk_short_token_or_query_preserves_filters) pass unchanged.

Related Issue

No existing issue — found while comparing the operator handling of the trigram path (quotes tokens, keeps operators) against the LIKE fallback right below it.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_state.py: replace the flat OR-join in the LIKE fallback with OR-separated conjunctive groups honoring AND/NOT; COALESCE nullable columns inside negations; snippet anchor = first positive term.
  • tests/test_hermes_state.py: four regression tests in TestCJKSearchFallbackNOT excludes, AND requires both terms, implicit adjacency is conjunctive, and A AND B OR C groups as (A AND B) OR C. All four fail before the fix.

How to Test

  1. python -m pytest tests/test_hermes_state.py -q — 370 passed (366 pre-existing + 4 new).
  2. Revert the hermes_state.py hunks and re-run: the four new tests fail (e.g. 报告 NOT 草稿 returns the draft message).
  3. Manual: create two messages, one with both terms and one with only the first, then search_messages("报告 NOT 草稿") — only the non-draft message must come back.

Checklist

Code

Documentation & Housekeeping

  • Docs — N/A (inline comments updated to describe the boolean semantics)
  • cli-config.yaml.example — N/A
  • CONTRIBUTING.md/AGENTS.md — N/A
  • Cross-platform impact considered: pure SQL/string handling
  • Tool descriptions/schemas — N/A

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 14, 2026
@solyanviktor-star
solyanviktor-star force-pushed the fix/cjk-like-fallback-boolean branch from cd76275 to a4b24dd Compare July 14, 2026 19:52

@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 tracing the short-token fallback and adding focused regression coverage. Current main still removes boolean operators and OR-joins the remaining terms in hermes_state.py:5068-5080, so the underlying bug is real.

Problems

  • hermes_state.py:4389 does not fully match FTS5 precedence. SQLite FTS5 gives implicit AND tighter binding than NOT: 报告 NOT 草稿 初版 means 报告 NOT (草稿 AND 初版). The proposed group emits 报告 AND NOT 草稿 AND 初版 instead. See SQLite FTS5 boolean operators.
  • The new tests at tests/test_hermes_state.py:1688-1734 cover simple NOT and AND, but not this NOT-plus-implicit-AND case.

Suggested changes

  • Parse implicit-AND runs before applying NOT, then add a short-token regression for 报告 NOT 草稿 初版 that distinguishes the two expressions.

Automated hermes-sweeper review.

Comment thread hermes_state.py Outdated
group_clauses = []
for positives, negatives in groups:
parts = [_like_term(t) for t in positives]
parts += [f"NOT {_like_term(t)}" for t in negatives]

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 bucket representation loses FTS5 precedence for 报告 NOT 草稿 初版: FTS5 treats whitespace as a tighter implicit AND (报告 NOT (草稿 AND 初版)), while this produces 报告 AND NOT 草稿 AND 初版. Parse implicit-AND runs before applying NOT and add a regression for this case.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@solyanviktor-star

Copy link
Copy Markdown
Contributor Author

Addressed in the follow-up commit. I verified the precedence against live SQLite FTS5 before reworking the parser:

  • aa NOT bb cc matches like aa NOT (bb AND cc) — implicit AND binds tighter than NOT, exactly as the review states;
  • aa NOT bb AND cc matches like (aa NOT bb) AND cc — explicit AND binds looser than NOT and terminates the negated run;
  • aa NOT bb NOT cc is left-associative double exclusion.

The fallback now parses NOT as opening a conjunctive negated run that implicit adjacency extends and explicit AND/OR terminates, and emits each run as NOT (term AND term ...) (with the existing COALESCE guards inside the run).

Tests: test_cjk_like_fallback_not_binds_looser_than_implicit_and covers the exact review query 报告 NOT 草稿 初版 with a document set that distinguishes the two expressions (a message containing 草稿 but not the full run must survive; it fails against the previous emit — verified by stashing the diff). Also added test_cjk_like_fallback_explicit_and_terminates_not_run to lock the 报告 NOT 草稿 AND 初版 reading. tests/test_hermes_state.py: 309 passed.

@solyanviktor-star
solyanviktor-star force-pushed the fix/cjk-like-fallback-boolean branch from ee1517f to 682f84a Compare July 23, 2026 19:44
@solyanviktor-star

Copy link
Copy Markdown
Contributor Author

Rebased onto current main to resolve the conflict introduced by the messages_fts_cjk bigram index (#65544) and the schema-v23 FTS rework (#65798).

Notes on the rebase:

  • The premise still holds on main: the LIKE fallback still OR-joins every non-operator token. The bigram route passes AND/OR/NOT through to FTS5 correctly, so the broken path is now narrower — it serves DBs where the bigram index is absent or its backfill is pending, role='tool' queries (tool rows are excluded from the cjk index), and queries with a lone 1-char CJK run.
  • The fix is unchanged in substance (OR-separated conjunctive buckets matching FTS5 precedence, including the NOT-run parsing from the previous review round) and now preserves main's (m.active = 1 OR m.compacted = 1) visibility rule (fix(compression): eliminate session duplication -- adopt in-place compaction like Claude Code and Codex #38763) inside the rebuilt clause.
  • Regression tests now set _fts_cjk_available = False — on current main the short-CJK test queries would otherwise route to the bigram index and stop exercising the LIKE path. All six tests fail without the fix and pass with it; the full tests/test_hermes_state.py suite passes (415 tests).

@solyanviktor-star

Copy link
Copy Markdown
Contributor Author

The slice-4 CI failure (tests/gateway/test_config.py::TestApiServerEnvOverride::test_env_key_does_not_reenable_explicitly_disabled_api_server) is unrelated to this change — it's the current main breakage from the API-server key-strength validation, with fixes already open in #70273 / #70274. Will re-run CI once one of those lands. All hermes_state tests pass on this branch (415/415 locally).

@solyanviktor-star
solyanviktor-star force-pushed the fix/cjk-like-fallback-boolean branch from 682f84a to fbe6081 Compare July 23, 2026 19:57
The LIKE fallback OR-joined every non-operator token, so "报告 NOT 草稿"
returned exactly the drafts it was asked to exclude and AND degraded to
OR. Group tokens into OR-separated conjunctive buckets that mirror FTS5
semantics, including operator precedence verified against live FTS5:
implicit AND binds tighter than NOT ("a NOT b c" == "a NOT (b AND c)")
while explicit AND binds looser ("a NOT b AND c" == "(a NOT b) AND c").
NULL tool columns are COALESCEd inside negated terms so NOT does not
silently drop ordinary messages, and the snippet anchor is the first
positive token.

Rebased onto current main after the messages_fts_cjk bigram index
(NousResearch#65544) landed: the bigram route passes operators through to FTS5
correctly, but the LIKE fallback it retains — for DBs where the index
is absent or its backfill is pending, role='tool' queries, and lone
1-char CJK runs — still OR-joined everything. The regression tests now
set _fts_cjk_available = False so they keep exercising that path; all
six fail without the fix.
@solyanviktor-star
solyanviktor-star force-pushed the fix/cjk-like-fallback-boolean branch from fbe6081 to 8e435f6 Compare July 29, 2026 16:54
@solyanviktor-star

Copy link
Copy Markdown
Contributor Author

Rebased again onto current main — the read-path split (6623ee9, per-thread read-only connections) moved this query off self._lock/self._conn, which collided with the snippet-anchor line. The LIKE query now runs through self._read_ctx() like its neighbours; the fix itself is unchanged.

Premise still holds on today's main: the CJK LIKE fallback continues to OR-join every non-operator token, so 报告 NOT 草稿 still returns exactly the drafts it was asked to exclude. All six regression tests fail without the fix (verified by reverting hermes_state.py to main) and the full tests/test_hermes_state.py passes with it — 485 passed, 1 skipped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants