Skip to content

fix(store): apply BM25 scope to the full match set, not a top-N window - #953

Open
Mr-Beasley wants to merge 1 commit into
tobi:mainfrom
Mr-Beasley:fix/fts-scoped-exact-922
Open

Mr-Beasley wants to merge 1 commit into
tobi:mainfrom
Mr-Beasley:fix/fts-scoped-exact-922

Conversation

@Mr-Beasley

Copy link
Copy Markdown

Fixes #922.

The bug

searchFTS takes a global top limit * 10 from FTS5, then applies the collection or metadata filter. If stronger out-of-scope matches fill that window, a scoped search returns [] even though the scope has matches. Multi-collection scopes inherit this through the #775 fan-out, because each leg runs the same windowed path.

Seen on a real index (4,332 docs, 7 collections): one collection of ~300 long podcast transcripts returned 0 for qmd search churn -c <collection> against 71 true matches. Its best hit ranked 638th of 1,030 index-wide, because BM25 length normalisation puts long documents below short notes in other collections. qmd search uses a window of 500, qmd query/MCP 200, so both missed it.

The change

When a collection or metadata filter is present, the CTE is MATERIALIZED with no inner LIMIT. The filter, ORDER BY and LIMIT run in the outer query over the complete match set, which has at most one row per matching document. Without a filter, the query is unchanged and keeps its inner LIMIT.

MATERIALIZED matters. Without it the planner can flatten the CTE and fold the scope back into the MATCH, which gives the slow per-rowid plan described in #918.

This is the approach from #918 by @fxstein. That PR has conflicted since #910 added the metadata filter path to searchFTS, which has the same truncation. This PR applies the approach to both paths on current main.

Tests

Both return [] on main and pass with this change. store, metadata-search, metadata-store, structured-search, multi-collection-filter, store-cjk-fts and store-fts-separator-queries: 397/397 pass. Oxlint is clean on the changed files. tsc --noEmit gives the same 117 errors on main and on this branch, so nothing new.

Plan and latency

Measured read-only on the 4,332-doc index above (SQLite 3.46.1). The scoped plan is MATERIALIZE fts_matches / SCAN documents_fts VIRTUAL TABLE INDEX 0:M3, the same MATCH plan as unscoped search, not the per-rowid 0:=M3.

Query Before After
churn*, scoped to transcripts 0 rows, 1.1 ms 50 rows, 12.1 ms
renewal*, scoped to transcripts 0 rows, 1.9 ms 12 rows (all), 2.7 ms
the*, scoped, near-every-doc match 50 rows, 31.8 ms 50 rows, 35.2 ms

🤖 Generated with Claude Code

searchFTS took a global top `limit * 10` from FTS5 and only then applied
the collection or metadata filter. When stronger out-of-scope matches
filled that window, scoped search returned [] although the scope held
matches (tobi#922). Multi-collection scopes inherited it through the tobi#775
per-collection fan-out.

Scoped queries now MATERIALIZE the complete MATCH set and filter, order
and limit in the outer query. MATERIALIZED stops the planner flattening
the CTE into a per-rowid probe. Unscoped queries keep the inner LIMIT.
Approach from tobi#918, carried onto the current store (metadata filter).

Fixes tobi#922

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@OldEricJi

Copy link
Copy Markdown

Independent synthetic reproduction against pinned main 04e4dbd8245c527a88f1a8f0bda547aef9ca81fb (Node 24.15.0, Linux). No real corpus, credentials, or models are involved. I have not tested this PR branch, so this is supporting evidence for the baseline defect, not a claim that this PR passes the reproducer.

We independently observed the same FTS starvation: 600 stronger out-of-scope documents plus 2 weaker eligible documents. search -n 5 --filter ... returned [], while diagnostic --all with the same filter returned both. We agree this belongs in the retrieval engine rather than an application over-fetch loop.

A separate isolated candidate fix moves active/collection/metadata predicates inside an FTS-driven CTE before LIMIT (CROSS JOIN keeps FTS first; bodies are read after selection). It returned the two eligible documents. This is an alternative experiment, not a request to replace the MATERIALIZED approach in this PR. EXPLAIN showed documents_fts VIRTUAL TABLE INDEX 0:M3, followed by document PK lookups. Small in-memory timings do not establish production performance.

The attached diagnostic covers collection/metadata starvation, multiple collections, inactive rows, invalid/missing extraction, and lex-only structured search. It uses 10,066 synthetic documents. It asserts the desired behavior and is expected to fail on the pinned baseline. Run from a checkout with tsx/dependencies available, with QMD_CONFIG_DIR and XDG_CACHE_HOME set to fresh temporary directories:

QMD_UPSTREAM_DIR="$PWD" node --import tsx scope-probe.mjs
Self-contained diagnostic (scope-probe.mjs)
import assert from 'node:assert/strict';
import { performance } from 'node:perf_hooks';
import { pathToFileURL } from 'node:url';
import { resolve } from 'node:path';
const root=process.env.QMD_UPSTREAM_DIR;
if(!root) throw new Error('Set QMD_UPSTREAM_DIR to the isolated, pinned QMD checkout; never the global install.');
const { createStore, searchFTS, structuredSearch, insertContent, insertDocument, hashContent } = await import(pathToFileURL(resolve(root,'src/store.ts')).href);
const { replaceDocumentMetadata } = await import(pathToFileURL(resolve(root,'src/metadata-store.ts')).href);
const { METADATA_EXTRACTION_VERSION } = await import(pathToFileURL(resolve(root,'src/metadata.ts')).href);
const store=createStore(':memory:');
async function doc(collection,path,body,metadata,active=true) {
  const hash=await hashContent(body), now='2026-09-16T00:00:00Z';
  insertContent(store.db,hash,body,now);
  const id=insertDocument(store.db,collection,path,path,hash,now,now);
  if(metadata!==undefined) replaceDocumentMetadata(store.db,id,{metadata,extractionVersion:METADATA_EXTRACTION_VERSION});
  if(!active) store.db.prepare('UPDATE documents SET active=0 WHERE id=?').run(id);
  return id;
}
try {
  store.db.exec('BEGIN');
  for(let i=0;i<10000;i++) await doc('noise',`noise${i}.md`,`检索目标 authentication noise${i}`,{priority:100});
  for(let i=0;i<2;i++) await doc('target',`early${i}.md`,`检索目标 authentication ${'周围的风吹过树叶。'.repeat(80)}`,{priority:i+1,status:'published'});
  await doc('target','pending.md','检索目标 authentication');
  const stale=await doc('target','stale.md','检索目标 authentication',{});
  const errored=await doc('target','errored.md','检索目标 authentication',{});
  replaceDocumentMetadata(store.db,stale,{metadata:{},extractionVersion:METADATA_EXTRACTION_VERSION-1});
  replaceDocumentMetadata(store.db,errored,{metadata:{},error:'fixture failure',extractionVersion:METADATA_EXTRACTION_VERSION});
  for(let i=0;i<60;i++) await doc('target',`inactive${i}.md`,'inactivequery',{},false);
  await doc('target','active.md',`inactivequery ${'background '.repeat(80)}`,{});
  store.db.exec('COMMIT');
  const filter={operator:'and',operands:[{key:'priority',operator:'lt',value:3},{key:'status',operator:'eq',value:'published'}]};
  const start=performance.now();
  const result=searchFTS(store.db,'检索目标',5,'target',filter);
  const filteredMs=performance.now()-start;
  assert.equal(result.length,2);
  assert.ok(result.every(row=>row.collectionName==='target'&&row.metadata.priority<3));
  assert.equal(searchFTS(store.db,'authentication',5,'target').length,5);
  assert.equal(searchFTS(store.db,'authentication',5,['noise','target'],filter).length,2);
  assert.equal(searchFTS(store.db,'inactivequery',1)[0].displayPath,'target/active.md');
  assert.equal(searchFTS(store.db,'missingquery',5,'target',filter).length,0);
  assert.equal(searchFTS(store.db,'检索目标',5,'target',{key:'missing',operator:'exists',value:false}).length,2);
  let plan=[];
  const prepare=store.db.prepare.bind(store.db);
  store.db.prepare=(sql)=>{
    const statement=prepare(sql);
    if(sql.includes('WITH fts_matches')) return {all:(...params)=>{plan=prepare('EXPLAIN QUERY PLAN '+sql).all(...params);return statement.all(...params);}};
    return statement;
  };
  searchFTS(store.db,'检索目标',5,'target',filter);
  const filteredPlan=plan;
  const wideStart=performance.now(); const wide=searchFTS(store.db,'authentication',5); const wideMs=performance.now()-wideStart;
  assert.equal(wide.length,5);
  const widePlan=plan;
  const structured=await structuredSearch(store,[{type:'lex',query:'authentication'}],{collections:['target'],filter,skipRerank:true,limit:5});
  assert.equal(structured.length,2,'lex-only structured search must preserve scope');
  console.log(JSON.stringify({checks:8,documents:10066,structuredLex:structured.length,filteredMs:Math.round(filteredMs*100)/100,wideMs:Math.round(wideMs*100)/100,filteredPlan,widePlan},null,2));
} finally {store.close();}
Independent candidate diff, for comparison only
diff --git a/src/store.ts b/src/store.ts
index 3a45bbc..ce286e5 100644
--- a/src/store.ts
+++ b/src/store.ts
@@ -4090,41 +4090,18 @@ export function searchFTS(db: Database, query: string, limit: number = 20, colle
   const ftsQuery = buildFTS5Query(query);
   if (!ftsQuery) return [];
 
-  // Use a CTE to force FTS5 to run first, then filter by collection.
-  // Without the CTE, SQLite's query planner combines FTS5 MATCH with the
-  // collection filter in a single WHERE clause, which can cause it to
-  // abandon the FTS5 index and fall back to a full scan — turning an 8ms
-  // query into a 17-second query on large collections.
+  // Rank within the eligible corpus, not a truncated global candidate window.
+  // CROSS JOIN keeps FTS5 as the driving table even for selective filters;
+  // load document bodies only after choosing the eligible top-k.
   const params: (string | number)[] = [ftsQuery];
-
-  // When filtering by collection or metadata, fetch extra candidates from the
-  // FTS index since some will be filtered out. Without a filter we can fetch
-  // exactly the requested limit. Selective filters remain best-effort: an
-  // eligible document outside this candidate window is missed (same
-  // completeness contract as collection filtering).
-  const ftsLimit = (collectionFilter || filter) ? limit * 10 : limit;
-
   let sql = `
     WITH fts_matches AS (
-      SELECT rowid, bm25(documents_fts, 1.5, 4.0, 1.0) as bm25_score
+      SELECT documents_fts.rowid, bm25(documents_fts, 1.5, 4.0, 1.0) as bm25_score
       FROM documents_fts
+      CROSS JOIN documents d ON d.id = documents_fts.rowid
+      LEFT JOIN document_metadata dm ON dm.document_id = d.id
       WHERE documents_fts MATCH ?
-      ORDER BY bm25_score ASC
-      LIMIT ${ftsLimit}
-    )
-    SELECT
-      'qmd://' || d.collection || '/' || d.path as filepath,
-      d.collection || '/' || d.path as display_path,
-      d.title,
-      content.doc as body,
-      d.hash,
-      fm.bm25_score,
-      dm.metadata_json
-    FROM fts_matches fm
-    JOIN documents d ON d.id = fm.rowid
-    JOIN content ON content.hash = d.hash
-    LEFT JOIN document_metadata dm ON dm.document_id = d.id
-    WHERE d.active = 1
+      AND d.active = 1
   `;
 
   if (collectionFilter) {
@@ -4140,8 +4117,21 @@ export function searchFTS(db: Database, query: string, limit: number = 20, colle
     params.push(...compiledFilter.params);
   }
 
-  // bm25 lower is better; sort ascending.
-  sql += ` ORDER BY fm.bm25_score ASC LIMIT ?`;
+  sql += ` ORDER BY bm25_score ASC LIMIT ?
+    )
+    SELECT
+      'qmd://' || d.collection || '/' || d.path as filepath,
+      d.collection || '/' || d.path as display_path,
+      d.title,
+      content.doc as body,
+      d.hash,
+      fm.bm25_score,
+      dm.metadata_json
+    FROM fts_matches fm
+    CROSS JOIN documents d ON d.id = fm.rowid
+    JOIN content ON content.hash = d.hash
+    LEFT JOIN document_metadata dm ON dm.document_id = d.id
+    ORDER BY fm.bm25_score ASC`;
   params.push(limit);
 
   const rows = db.prepare(sql).all(...params) as { filepath: string; display_path: string; title: string; body: string; hash: string; bm25_score: number; metadata_json: string | null }[];

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Post-#775: collection-scoped BM25 still truncates global FTS results before collection filtering

2 participants