From fcab1e2e8207a469ae4996b5a75ae346d98d4b7c Mon Sep 17 00:00:00 2001 From: Antheurus <91123607+Antheurus@users.noreply.github.com> Date: Mon, 11 May 2026 22:39:10 +0700 Subject: [PATCH] fix(augment): add CONTAINS fallback when FTS indexes unavailable (#1476) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(augment): add CONTAINS fallback when FTS indexes unavailable When the MCP server holds the KuzuDB write lock, the augment CLI opens the DB read-only. FTS indexes cannot be created in read-only mode, so searchFTSFromLbug returns ftsAvailable=false and an empty results array. The existing early-return path silently produced no enrichment. Add a Cypher name CONTAINS fallback that fires only when ftsAvailable is false and BM25 produced no symbol matches. This covers the read-only DB case (concurrent MCP server) and the first-run case (indexes not yet built). The fallback is wrapped in .catch(() => []) and cannot throw. When FTS indexes exist, this branch is never reached — behaviour is unchanged for users without a concurrent MCP server. * fix(augment): guard against CONTAINS '' and add no-FTS test coverage Blocker 1 — CONTAINS '' on whitespace-leading patterns: pattern.split(/\s+/)[0] returns "" when the input has leading whitespace (e.g. " ".split(/\s+/) → ["", ""]). In Kuzu, CONTAINS '' matches every node with a name property, injecting arbitrary graph nodes into LLM context. Fix: trim() before split, then guard on !firstWord || firstWord.length < 2. No behaviour change for normal non-empty patterns. Blocker 2 — zero test coverage on the FTS-unavailable code path: The new CONTAINS fallback block (engine.ts lines 146-166) was exercised by no existing test — all existing tests run with FTS indexes built. A second withTestLbugDB fixture is added with no ftsIndexes, forcing searchFTSFromLbug to return ftsAvailable: false, and asserts: 1. augment('login', ...) returns non-empty enrichment (fallback works) 2. augment(' ', ...) returns '' (CONTAINS '' guard holds) 3. augment('nxyz_notfound', ...) returns '' (no matching nodes) 4. executeQuery throwing returns '' (.catch(() => []) path) * fix(augment): extend CONTAINS '' guard to FTS happy path and consolidate The same split(/\s+/)[0] bug existed at line 125 (BM25 symbol filter, FTS-available path) — a leading-whitespace pattern produced CONTAINS '' there too, matching every node in BM25-matched files. Fix: hoist patternFirstWord computation with trim() and the length guard to the top of augment(), before any DB interaction. Both CONTAINS sites (BM25 symbol filter and CONTAINS fallback) now use the single pre-validated value. No behaviour change for normal patterns; the guard fires once for all callers instead of being duplicated. Also tighten the whitespace test in the no-FTS suite from 3 spaces to 4 spaces so it unambiguously exercises the patternFirstWord guard rather than straddling the outer pattern.length < 3 boundary. * test(augment): negative-safety test for ftsAvailable=true gate Asserts the CONTAINS fallback does NOT fire when FTS is available but BM25 returns zero results. Pins the safety property promised by the PR description: behavior is unchanged for users without the read-only-DB condition. If anyone later loosens the gate to `symbolMatches.length === 0` alone, this test fails. --------- Co-authored-by: Gergő Magyar --- gitnexus/src/core/augmentation/engine.ts | 32 ++++++- .../test/integration/augmentation.test.ts | 85 +++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/gitnexus/src/core/augmentation/engine.ts b/gitnexus/src/core/augmentation/engine.ts index 42087e4b0..81e41077e 100644 --- a/gitnexus/src/core/augmentation/engine.ts +++ b/gitnexus/src/core/augmentation/engine.ts @@ -86,6 +86,9 @@ async function findRepoForCwd(cwd: string): Promise<{ export async function augment(pattern: string, cwd?: string): Promise { if (!pattern || pattern.length < 3) return ''; + const patternFirstWord = pattern.trim().replace(/'/g, "''").split(/\s+/)[0]; + if (!patternFirstWord || patternFirstWord.length < 2) return ''; + const workDir = cwd || process.cwd(); try { @@ -104,9 +107,7 @@ export async function augment(pattern: string, cwd?: string): Promise { } // Step 1: BM25 search (fast, no embeddings) - const { results: bm25Results } = await searchFTSFromLbug(pattern, 10, repoId); - - if (bm25Results.length === 0) return ''; + const { results: bm25Results, ftsAvailable } = await searchFTSFromLbug(pattern, 10, repoId); // Step 2: Map BM25 file results to symbols const symbolMatches: Array<{ @@ -124,7 +125,7 @@ export async function augment(pattern: string, cwd?: string): Promise { repoId, ` MATCH (n) WHERE n.filePath = '${escaped}' - AND n.name CONTAINS '${pattern.replace(/'/g, "''").split(/\s+/)[0]}' + AND n.name CONTAINS '${patternFirstWord}' RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath LIMIT 3 `, @@ -143,6 +144,29 @@ export async function augment(pattern: string, cwd?: string): Promise { } } + // When FTS indexes are unavailable (read-only DB, first run before indexes are built), + // fall back to a direct name CONTAINS query so enrichment still works. + if (symbolMatches.length === 0 && !ftsAvailable) { + const fallbackRows = await executeQuery( + repoId, + ` + MATCH (n) + WHERE n.name CONTAINS '${patternFirstWord}' + RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath + LIMIT 5 + `, + ).catch(() => []); + for (const sym of fallbackRows) { + symbolMatches.push({ + nodeId: sym.id || sym[0], + name: sym.name || sym[1], + type: sym.type || sym[2], + filePath: sym.filePath || sym[3], + score: 1.0, + }); + } + } + if (symbolMatches.length === 0) return ''; // Step 3: Batch-fetch callers/callees/processes/cohesion for top matches diff --git a/gitnexus/test/integration/augmentation.test.ts b/gitnexus/test/integration/augmentation.test.ts index aed09d61f..60c4170fd 100644 --- a/gitnexus/test/integration/augmentation.test.ts +++ b/gitnexus/test/integration/augmentation.test.ts @@ -57,6 +57,7 @@ vi.mock('../../src/storage/repo-manager.js', () => ({ })); let augment: (pattern: string, cwd?: string) => Promise; +let augmentNoFts: (pattern: string, cwd?: string) => Promise; withTestLbugDB( 'augment', @@ -106,6 +107,28 @@ withTestLbugDB( const result = await augment('日本語テスト', handle.dbPath); expect(typeof result).toBe('string'); }); + + // ─── Negative-safety: fallback must stay gated on !ftsAvailable ─── + // + // When FTS is available but happens to return zero BM25 hits, the + // CONTAINS fallback must NOT fire — preserving the original early-return + // semantics. If anyone later loosens the gate to `symbolMatches.length + // === 0` alone, this test fails. + + it('does NOT fire CONTAINS fallback when FTS is available but BM25 returns empty', async () => { + const bm25 = await import('../../src/core/search/bm25-index.js'); + const spy = vi + .spyOn(bm25, 'searchFTSFromLbug') + .mockResolvedValue({ results: [], ftsAvailable: true }); + try { + // 'login' WOULD match a graph node via CONTAINS, but FTS is available + // and empty → fallback gate must hold → result must be ''. + const result = await augment('login', handle.dbPath); + expect(result).toBe(''); + } finally { + spy.mockRestore(); + } + }); }); }, { @@ -131,3 +154,65 @@ withTestLbugDB( }, }, ); + +// ─── FTS-unavailable suite: exercises the CONTAINS fallback branch ──────────── +// +// No ftsIndexes → searchFTSFromLbug returns ftsAvailable: false → fallback fires. +// Same seed data so 'login' still exists as a graph node. + +withTestLbugDB( + 'augment-no-fts', + (handle) => { + describe('augment() — FTS indexes unavailable (CONTAINS fallback)', () => { + it('falls back to CONTAINS query and returns enrichment when FTS is unavailable', async () => { + const result = await augmentNoFts('login', handle.dbPath); + + expect(result.length).toBeGreaterThan(0); + expect(result).toContain('[GitNexus]'); + }); + + it("returns empty string for whitespace-only pattern (CONTAINS '' guard)", async () => { + const result = await augmentNoFts(' ', handle.dbPath); + expect(result).toBe(''); + }); + + it('returns empty string when no nodes match the CONTAINS query', async () => { + const result = await augmentNoFts('nxyz_notfound', handle.dbPath); + expect(result).toBe(''); + }); + + it('returns empty string when fallback CONTAINS query throws', async () => { + const poolAdapter = await import('../../src/core/lbug/pool-adapter.js'); + const spy = vi + .spyOn(poolAdapter, 'executeQuery') + .mockRejectedValue(new Error('simulated DB error')); + try { + const result = await augmentNoFts('login', handle.dbPath); + expect(result).toBe(''); + } finally { + spy.mockRestore(); + } + }); + }); + }, + { + seed: AUGMENT_SEED_DATA, + // Intentionally no ftsIndexes — forces searchFTSFromLbug to return ftsAvailable: false + poolAdapter: true, + afterSetup: async (handle) => { + const { listRegisteredRepos } = await import('../../src/storage/repo-manager.js'); + (listRegisteredRepos as ReturnType).mockResolvedValue([ + { + name: handle.repoId, + path: handle.dbPath, + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'abc123', + }, + ]); + + const engine = await import('../../src/core/augmentation/engine.js'); + augmentNoFts = engine.augment; + }, + }, +);