fix(augment): add CONTAINS fallback when FTS indexes unavailable (#1476)

* 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 <gergomagyar@icloud.com>
This commit is contained in:
Antheurus 2026-05-11 22:39:10 +07:00 • committed by GitHub
parent 0b877ce869
commit cd2b0570a9
2 changed files with 113 additions and 4 deletions

View file

@ -86,6 +86,9 @@ async function findRepoForCwd(cwd: string): Promise<{
export async function augment(pattern: string, cwd?: string): Promise<string> {
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<string> {
}
// 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<string> {
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<string> {
}
}
// 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

View file

@ -57,6 +57,7 @@ vi.mock('../../src/storage/repo-manager.js', () => ({
}));
let augment: (pattern: string, cwd?: string) => Promise<string>;
let augmentNoFts: (pattern: string, cwd?: string) => Promise<string>;
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<typeof vi.fn>).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;
},
},
);