From e46651e42c321940984bec0e3d4849c4d2b73a58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Wed, 10 Jun 2026 07:59:37 +0100 Subject: [PATCH] fix(embeddings): create VECTOR index via conn.query, not the prepared path (#2114) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gitnexus analyze` silently failed to create the LadybugDB VECTOR/HNSW index because `CALL CREATE_VECTOR_INDEX(...)` was run through the prepared `conn.prepare()` path, which rejects multi-statement procedures — degrading semantic search to exact-scan. Route index creation through `conn.query()` via a new adapter-owned `createVectorIndex` (mirrors `createFTSIndex`), make the previously-swallowed error visible (`{ err }` logging), add an in-process idempotency cache, and add real-`@ladybugdb/core` regression coverage. Fixes #2114. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/core/embeddings/embedding-pipeline.ts | 53 ++++---- gitnexus/src/core/lbug/lbug-adapter.ts | 54 ++++++++ .../integration/lbug-vector-extension.test.ts | 118 +++++++++++++++++- gitnexus/test/unit/embedding-pipeline.test.ts | 61 ++++++++- 4 files changed, 257 insertions(+), 29 deletions(-) diff --git a/gitnexus/src/core/embeddings/embedding-pipeline.ts b/gitnexus/src/core/embeddings/embedding-pipeline.ts index e394659f4..0405cd47c 100644 --- a/gitnexus/src/core/embeddings/embedding-pipeline.ts +++ b/gitnexus/src/core/embeddings/embedding-pipeline.ts @@ -36,13 +36,8 @@ import { } from './types.js'; import { resolveEmbeddingConfig } from './config.js'; import { rankExactEmbeddingRows, type ExactEmbeddingRow } from './exact-search.js'; -import { - EMBEDDING_TABLE_NAME, - EMBEDDING_INDEX_NAME, - CREATE_VECTOR_INDEX_QUERY, - STALE_HASH_SENTINEL, -} from '../lbug/schema.js'; -import { loadVectorExtension } from '../lbug/lbug-adapter.js'; +import { EMBEDDING_TABLE_NAME, EMBEDDING_INDEX_NAME, STALE_HASH_SENTINEL } from '../lbug/schema.js'; +import { loadVectorExtension, createVectorIndex } from '../lbug/lbug-adapter.js'; import type { ExtensionInstallPolicy } from '../lbug/extension-loader.js'; import { getExactScanLimit } from '../platform/capabilities.js'; import { logger } from '../logger.js'; @@ -215,24 +210,36 @@ export const batchInsertEmbeddings = async ( }; /** - * Create the vector index for semantic search - - * Now indexes the separate CodeEmbedding table. - * Delegates extension loading to lbug-adapter's loadVectorExtension(), - * which owns the VECTOR extension lifecycle and state tracking. - + * Create the vector index for semantic search (indexes the CodeEmbedding table). + * + * Keeps the embedding-specific extension-install policy gate here + * (ensureVectorExtensionAvailable → resolveEmbeddingInstallPolicy, default + * `auto` for the analyze write path), then delegates the actual + * `CALL CREATE_VECTOR_INDEX(...)` to the adapter, which runs it through the + * unprepared `conn.query()` path. It must NOT go through the injected + * `executeQuery` (prepared `conn.prepare()`): LadybugDB cannot prepare that + * procedure and fails with "We do not support prepare multiple statements" — + * the silent degrade in #2114. */ -const createVectorIndex = async ( - executeQuery: (cypher: string) => Promise, -): Promise => { +const buildVectorIndex = async (): Promise => { + // This pre-check applies the embedding-specific install policy + // (resolveEmbeddingInstallPolicy, default `auto` for analyze) before reaching + // the adapter. The adapter's createVectorIndex() calls loadVectorExtension() + // again, but that's a no-op here: once this gate loads VECTOR the module-level + // `vectorExtensionLoaded` flag is set, so the adapter's second call + // short-circuits without re-resolving the policy — no double install. if (!(await ensureVectorExtensionAvailable())) return false; try { - await executeQuery(CREATE_VECTOR_INDEX_QUERY); - return true; + return await createVectorIndex(); } catch (error) { - if (isDev) { - logger.warn({ error }, 'Vector index creation warning:'); - } + // Surface this even outside dev: it silently downgrades a user-requested + // feature (semantic search) to exact scan. Log under `err` so pino's + // standard serializer captures the message/stack — logging under `error` + // serialized an Error to `{}` (the empty `{"error":{}}` reported in #2114). + logger.warn( + { err: error }, + 'Vector index creation failed; semantic search will use exact-scan fallback', + ); return false; } }; @@ -383,7 +390,7 @@ export const runEmbeddingPipeline = async ( // Ensure the vector index exists even when no new nodes need embedding. // A prior crash or first-time incremental run may have left CodeEmbedding // rows without ever reaching index creation. - const vectorIndexReady = await createVectorIndex(executeQuery); + const vectorIndexReady = await buildVectorIndex(); onProgress({ phase: 'ready', @@ -544,7 +551,7 @@ export const runEmbeddingPipeline = async ( logger.info('📇 Creating vector index...'); } - const vectorIndexReady = await createVectorIndex(executeQuery); + const vectorIndexReady = await buildVectorIndex(); onProgress({ phase: 'ready', diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 101783bcb..dea1a4e98 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -14,6 +14,7 @@ import { REL_TABLE_NAME, SCHEMA_QUERIES, EMBEDDING_TABLE_NAME, + CREATE_VECTOR_INDEX_QUERY, STALE_HASH_SENTINEL, NodeTableName, } from './schema.js'; @@ -171,6 +172,11 @@ let currentDbPath: string | null = null; let currentDbReadOnly = false; let ftsLoaded = false; let vectorExtensionLoaded = false; +// In-process guard so a repeated createVectorIndex() within one connection +// lifetime skips the DB round-trip (mirrors ensuredFTSIndexes). Reset wherever +// vectorExtensionLoaded resets, so it can never stay true against a swapped or +// closed connection. +let vectorIndexEnsured = false; /** * In-process cache of FTS indexes observed against the current singleton @@ -603,6 +609,7 @@ const resetOpenConnectionState = (): void => { currentDbPath = null; ftsLoaded = false; vectorExtensionLoaded = false; + vectorIndexEnsured = false; ensuredFTSIndexes.clear(); }; @@ -690,6 +697,7 @@ export const withLbugDb = async ( currentDbPath = null; ftsLoaded = false; vectorExtensionLoaded = false; + vectorIndexEnsured = false; ensuredFTSIndexes.clear(); }); // Sleep outside the lock — no need to block others while waiting @@ -716,6 +724,7 @@ const doInitLbug = async (dbPath: string, readOnly: boolean = false) => { currentDbPath = null; ftsLoaded = false; vectorExtensionLoaded = false; + vectorIndexEnsured = false; ensuredFTSIndexes.clear(); } @@ -1671,6 +1680,7 @@ export const closeLbug = async (): Promise => { currentDbPath = null; ftsLoaded = false; vectorExtensionLoaded = false; + vectorIndexEnsured = false; ensuredFTSIndexes.clear(); }; @@ -1938,6 +1948,50 @@ export const createFTSIndex = async ( } }; +/** + * Create the HNSW vector index on the CodeEmbedding table. + * + * MUST run via `conn.query()` (here through `queryAndDrain`), NOT through the + * prepared `executeQuery`/`conn.prepare()` path: `CALL CREATE_VECTOR_INDEX(...)` + * compiles to multiple statements, which LadybugDB cannot prepare — it fails + * with "Connection Exception: We do not support prepare multiple statements." + * Routing index creation through `executeQuery` (prepared) is exactly what + * broke vector-index creation during `analyze` (#2114; the singleton + * `executeQuery` was switched to the prepared path in #1655 while FTS index + * creation kept using `conn.query()`, which is why FTS survived and VECTOR did + * not). Mirrors `createFTSIndex` above. + * + * Returns `true` on success (or when the index already exists — idempotent so + * incremental re-runs don't spuriously downgrade to exact scan), `false` when + * the VECTOR extension is unavailable or the connection is read-only. Any other + * failure propagates so the caller can log it. + */ +export const createVectorIndex = async (): Promise => { + if (!conn) { + throw new Error('LadybugDB not initialized. Call initLbug first.'); + } + // Already built on this connection — skip the round-trip (mirrors createFTSIndex). + if (vectorIndexEnsured) return true; + if (!(await loadVectorExtension())) { + return false; + } + try { + await queryAndDrain(conn, CREATE_VECTOR_INDEX_QUERY); + vectorIndexEnsured = true; + return true; + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + // Idempotent: a prior analyze already built the HNSW index. + if (msg.includes('already exists')) { + vectorIndexEnsured = true; + return true; + } + // Read-only DB (e.g. the MCP query pool): writable analyze owns creation. + if (isReadOnlyDbError(e)) return false; + throw e; + } +}; + /** * Lazy-create an FTS index, caching the fact in-process. * diff --git a/gitnexus/test/integration/lbug-vector-extension.test.ts b/gitnexus/test/integration/lbug-vector-extension.test.ts index 5eb966189..ba436f183 100644 --- a/gitnexus/test/integration/lbug-vector-extension.test.ts +++ b/gitnexus/test/integration/lbug-vector-extension.test.ts @@ -7,7 +7,7 @@ * Follows existing lbug integration test patterns (lbug-core-adapter, * lbug-lock-retry). */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeAll, beforeEach } from 'vitest'; import { withTestLbugDB } from '../helpers/test-indexed-db.js'; withTestLbugDB('vector-extension', (handle) => { @@ -73,3 +73,119 @@ withTestLbugDB('vector-extension', (handle) => { }); }); }); + +/** + * Regression: VECTOR/HNSW index creation during analyze (#2114). + * + * `CALL CREATE_VECTOR_INDEX(...)` compiles to multiple statements, which + * LadybugDB cannot run through `conn.prepare()`. Routing it through the + * prepared `executeQuery` path (as #1655 inadvertently did when it switched the + * singleton `executeQuery` from `conn.query()` to `conn.prepare()`) makes it + * throw "We do not support prepare multiple statements", which `analyze` + * swallowed and silently downgraded to exact-scan. The fix gives the adapter a + * `createVectorIndex()` that runs the procedure via `conn.query()` (like + * `createFTSIndex`). These tests exercise the real adapter against a real + * LadybugDB so a revert to the prepared path fails loudly. + */ +withTestLbugDB('vector-index-creation', () => { + // VECTOR is platform-sensitive (skipped on win32 / unsupported platforms, + // and when it cannot be installed offline). Probe once, skip the suite if + // unavailable — mirrors the FTS-skip convention in withTestLbugDB. + let vectorAvailable = false; + let skipWarned = false; + beforeAll(async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const { resolveAnalyzeInstallPolicy } = await import('../../src/core/lbug/extension-loader.js'); + // Mirror the analyze write path (`auto`: LOAD-first, then one bounded + // INSTALL) so this suite runs wherever analyze would have vector support. + vectorAvailable = await adapter.loadVectorExtension(undefined, { + policy: resolveAnalyzeInstallPolicy(), + }); + }); + beforeEach((ctx) => { + if (!vectorAvailable) { + if (!skipWarned) { + skipWarned = true; + console.warn( + '[withTestLbugDB(vector-index-creation)] Skipping — the LadybugDB VECTOR ' + + 'extension is unavailable (unsupported platform or could not be installed).', + ); + } + ctx.skip(); + } + }); + + describe('createVectorIndex', () => { + it('creates the HNSW index via conn.query (the prepared path cannot)', async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + const created = await adapter.createVectorIndex(); + expect(created).toBe(true); + + const rows = await adapter.executeQuery('CALL SHOW_INDEXES() RETURN *'); + const idx = rows.find((r: any) => r.index_name === 'code_embedding_idx'); + expect(idx).toBeDefined(); + expect(idx.index_type).toBe('HNSW'); + }); + + it('is idempotent — a second call returns true so incremental re-runs do not downgrade to exact scan', async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + await adapter.createVectorIndex(); + await expect(adapter.createVectorIndex()).resolves.toBe(true); + + // No duplicate index created by the repeat call. + const rows = await adapter.executeQuery('CALL SHOW_INDEXES() RETURN *'); + const matches = rows.filter((r: any) => r.index_name === 'code_embedding_idx'); + expect(matches).toHaveLength(1); + }); + }); +}); + +/** + * Regression for the #2114 root cause: the prepared `executeQuery` path cannot + * create the index. This lives in its OWN suite (a fresh, index-free DB) on + * purpose — in the `vector-index-creation` suite above the index already exists + * by the time this would run, so `conn.prepare()` fails with "index already + * exists" instead of the multi-statement rejection we want to pin. With no index + * present, `CALL CREATE_VECTOR_INDEX(...)` (which compiles to multiple + * statements) is rejected by `conn.prepare()` with "We do not support prepare + * multiple statements" — the exact failure that silently downgraded analyze to + * exact-scan, and why `createVectorIndex` must use `conn.query()` instead. + */ +withTestLbugDB('vector-index-prepare-rejects', () => { + let vectorAvailable = false; + let skipWarned = false; + beforeAll(async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const { resolveAnalyzeInstallPolicy } = await import('../../src/core/lbug/extension-loader.js'); + vectorAvailable = await adapter.loadVectorExtension(undefined, { + policy: resolveAnalyzeInstallPolicy(), + }); + }); + beforeEach((ctx) => { + if (!vectorAvailable) { + if (!skipWarned) { + skipWarned = true; + console.warn( + '[withTestLbugDB(vector-index-prepare-rejects)] Skipping — the LadybugDB VECTOR ' + + 'extension is unavailable (unsupported platform or could not be installed).', + ); + } + ctx.skip(); + } + }); + + it('the prepared executeQuery path rejects CREATE_VECTOR_INDEX (#2114 root cause)', async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const { CREATE_VECTOR_INDEX_QUERY } = await import('../../src/core/lbug/schema.js'); + + // executeQuery -> executePrepared -> conn.prepare(): the multi-statement + // CREATE_VECTOR_INDEX procedure cannot be prepared. Anchored to the specific + // error so the test can only pass for the #2114 reason — not for an + // unrelated throw (e.g. a missing table or an already-existing index). + await expect(adapter.executeQuery(CREATE_VECTOR_INDEX_QUERY)).rejects.toThrow( + /prepare multiple statements/i, + ); + }); +}); diff --git a/gitnexus/test/unit/embedding-pipeline.test.ts b/gitnexus/test/unit/embedding-pipeline.test.ts index f5a9f5ae2..91f182db2 100644 --- a/gitnexus/test/unit/embedding-pipeline.test.ts +++ b/gitnexus/test/unit/embedding-pipeline.test.ts @@ -182,6 +182,11 @@ describe('runEmbeddingPipeline incremental filter', () => { let queryCalls: string[]; let stmtCalls: Array<{ cypher: string; params: Array> }>; let progressUpdates: EmbeddingProgress[]; + // Spy for the adapter's createVectorIndex (the pipeline delegates index + // creation to it via conn.query — see #2114). Captured so tests can assert + // it was invoked instead of asserting CREATE_VECTOR_INDEX flowed through the + // injected (prepared) executeQuery, which it must NOT. + let vectorIndexMock: ReturnType; // Helper node const makeNode = (overrides: Partial = {}): EmbeddableNode => ({ @@ -215,9 +220,12 @@ describe('runEmbeddingPipeline incremental filter', () => { isEmbedderReady: vi.fn().mockReturnValue(true), })); - // Mock loadVectorExtension (avoids needing the native lbug module) + // Mock the adapter (avoids needing the native lbug module). The pipeline + // imports both loadVectorExtension and createVectorIndex from here. + vectorIndexMock = vi.fn().mockResolvedValue(true); vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ loadVectorExtension: vi.fn().mockResolvedValue(true), + createVectorIndex: vectorIndexMock, })); }; @@ -347,6 +355,7 @@ describe('runEmbeddingPipeline incremental filter', () => { })); vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ loadVectorExtension: vi.fn().mockResolvedValue(true), + createVectorIndex: vi.fn().mockResolvedValue(true), })); const executeQuery = vi.fn().mockImplementation(async (cypher: string) => { @@ -486,7 +495,7 @@ describe('runEmbeddingPipeline incremental filter', () => { const { runEmbeddingPipeline } = await import('../../src/core/embeddings/embedding-pipeline.js'); - await runEmbeddingPipeline( + const result = await runEmbeddingPipeline( executeQuery, executeWithReusedStatement, onProgress, @@ -496,9 +505,13 @@ describe('runEmbeddingPipeline incremental filter', () => { existingEmbeddings, ); - // The CREATE_VECTOR_INDEX query should have been called via executeQuery - const vectorIndexCalls = queryCalls.filter((c) => c.includes('CREATE_VECTOR_INDEX')); - expect(vectorIndexCalls.length).toBeGreaterThanOrEqual(1); + // Index creation must go through the adapter's createVectorIndex (conn.query), + // NOT the injected/prepared executeQuery — CALL CREATE_VECTOR_INDEX cannot be + // prepared (#2114). It must still run on the zero-nodes-to-embed branch. + expect(vectorIndexMock).toHaveBeenCalledTimes(1); + expect(queryCalls.some((c) => c.includes('CREATE_VECTOR_INDEX'))).toBe(false); + expect(result.vectorIndexReady).toBe(true); + expect(result.semanticMode).toBe('vector-index'); }); it('stores embeddings with exact-scan fallback when VECTOR is unavailable', async () => { @@ -515,6 +528,7 @@ describe('runEmbeddingPipeline incremental filter', () => { })); vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ loadVectorExtension: vi.fn().mockResolvedValue(false), + createVectorIndex: vi.fn().mockResolvedValue(false), })); const node = makeNode(); @@ -531,6 +545,41 @@ describe('runEmbeddingPipeline incremental filter', () => { expect(progressUpdates.at(-1)?.phase).toBe('ready'); }); + it('degrades to exact-scan (without throwing) when vector index creation fails', async () => { + vi.doMock('../../src/core/embeddings/embedder.js', () => ({ + initEmbedder: vi.fn().mockResolvedValue(undefined), + embedBatch: vi + .fn() + .mockImplementation((texts: string[]) => + Promise.resolve(texts.map(() => new Float32Array(384))), + ), + embedText: vi.fn().mockResolvedValue(new Float32Array(384)), + embeddingToArray: vi.fn().mockImplementation((emb: Float32Array) => Array.from(emb)), + isEmbedderReady: vi.fn().mockReturnValue(true), + })); + // VECTOR loads, but the adapter's createVectorIndex throws (e.g. a DB error + // during HNSW build). The pipeline wrapper must swallow it, log, and fall + // back to exact-scan rather than failing the whole analyze run (#2114). + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ + loadVectorExtension: vi.fn().mockResolvedValue(true), + createVectorIndex: vi.fn().mockRejectedValue(new Error('HNSW build failed')), + })); + + const node = makeNode(); + const executeQuery = mockExecuteQuery([node]); + const executeWithReusedStatement = mockExecuteWithReusedStatement(); + const { runEmbeddingPipeline } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + + const result = await runEmbeddingPipeline(executeQuery, executeWithReusedStatement, onProgress); + + expect(result.vectorIndexReady).toBe(false); + expect(result.semanticMode).toBe('exact-scan'); + // Embeddings were still persisted and the pipeline completed normally. + expect(stmtCalls.some((call) => call.cypher.includes('CREATE'))).toBe(true); + expect(progressUpdates.at(-1)?.phase).toBe('ready'); + }); + it('does not inject preceding context when overlap is disabled', async () => { const embedBatchSpy = vi .fn() @@ -546,6 +595,7 @@ describe('runEmbeddingPipeline incremental filter', () => { })); vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ loadVectorExtension: vi.fn().mockResolvedValue(true), + createVectorIndex: vi.fn().mockResolvedValue(true), })); const node = makeNode({ @@ -600,6 +650,7 @@ describe('runEmbeddingPipeline incremental filter', () => { })); vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ loadVectorExtension: vi.fn().mockResolvedValue(true), + createVectorIndex: vi.fn().mockResolvedValue(true), })); const node = makeNode({