diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index d9aa33a4e..c290fa29a 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -46,7 +46,7 @@ jobs: with: path: ~/.lbdb/extension key: lbug-fts-${{ runner.os }}-${{ hashFiles('gitnexus/package-lock.json') }} - - name: Ensure FTS extension installed + - name: Ensure FTS + VECTOR extensions installed run: npx tsx scripts/ensure-fts.ts working-directory: gitnexus - name: Run sharded tests with coverage (blob) @@ -205,6 +205,10 @@ jobs: # tsx-on-source path in CI (both entry points stay covered). env: GITNEXUS_REQUIRE_FTS: '1' + # #2623: the win32 VECTOR gate is gone, so the vector suites genuinely + # run here — require the extension so an unavailable VECTOR is a loud + # failure, never a silent skip (same contract as GITNEXUS_REQUIRE_FTS). + GITNEXUS_REQUIRE_VECTOR: '1' GITNEXUS_E2E_CLI: dist # #2449: hosted Windows runners intermittently push the busiest shard past # the default 15-minute watchdog. 20 minutes restores real headroom while @@ -219,19 +223,21 @@ jobs: - uses: ./.github/actions/setup-gitnexus with: build: 'true' - # Warm-cache the installed LadybugDB FTS extension (~/.lbdb/extension) per - # OS + lockfile so a warm run skips the network install entirely, and the - # parallel shards share one download across runs. Pure reliability/speed: - # on a cache miss the tests self-install FTS on demand (see - # test/helpers/fts-availability.ts), so a miss just falls back to install — - # never a correctness dependency. Keyed by lockfile hash so a LadybugDB - # version bump re-installs; per-OS because the extension is a native binary. + # Warm-cache the installed LadybugDB FTS + VECTOR extensions + # (~/.lbdb/extension) per OS + lockfile so a warm run skips the network + # install entirely, and the parallel shards share one download across + # runs. Pure reliability/speed: on a cache miss the tests self-install on + # demand (see test/helpers/fts-availability.ts), so a miss just falls + # back to install — never a correctness dependency. Keyed by lockfile + # hash so a LadybugDB version bump re-installs; per-OS because the + # extensions are native binaries. (Key name kept as lbug-fts for cache + # continuity — the path covers every extension in the shared home.) - name: Cache LadybugDB FTS extension uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v5 with: path: ~/.lbdb/extension key: lbug-fts-${{ runner.os }}-${{ hashFiles('gitnexus/package-lock.json') }} - - name: Ensure FTS extension installed + - name: Ensure FTS + VECTOR extensions installed run: npx tsx scripts/ensure-fts.ts working-directory: gitnexus - name: Run platform-sensitive tests diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index 5d6b177f5..0ccd6beb8 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -117,6 +117,12 @@ const LBUG_NATIVE = [ // to a live native DB, rm-then-rename over an existing parked copy) before // any open — rename semantics are exactly what differs on Windows. 'test/unit/incremental-dirty-recovery.test.ts', + // #2623: the incremental writeback must load VECTOR before the CodeEmbedding + // join-delete, and the blocked path must escalate instead of crashing. The + // win32 VECTOR gate was removed in the same PR, so this ordering must be + // proven on the windows-latest native addon, not just Ubuntu. Budget: ~25s + // on Linux → expect ~2min on the slowest Windows shard. + 'test/unit/incremental-vector-extension-ordering.test.ts', ]; // Process spawning and CLI tests — exercise child_process with real diff --git a/gitnexus/scripts/ensure-fts.ts b/gitnexus/scripts/ensure-fts.ts index a3611de1b..94f781374 100644 --- a/gitnexus/scripts/ensure-fts.ts +++ b/gitnexus/scripts/ensure-fts.ts @@ -1,6 +1,6 @@ /** - * Install the LadybugDB FTS extension into the shared home (~/.lbdb) up front, so - * every test in a sharded CI run finds it regardless of which shard it lands in. + * Install the LadybugDB FTS and VECTOR extensions into the shared home (~/.lbdb) + * up front, so every test in a sharded CI run finds them regardless of shard. * * FTS-dependent tests split two ways: the LOAD-path gate (skipUnlessFtsAvailable) * self-installs on miss, but the FILE-path gate (requireFtsResourceOrSkip, e.g. @@ -17,13 +17,24 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { initLbug, loadFTSExtension, closeLbug } from '../src/core/lbug/lbug-adapter.js'; +import { + initLbug, + loadFTSExtension, + loadVectorExtension, + closeLbug, +} from '../src/core/lbug/lbug-adapter.js'; const dir = mkdtempSync(join(tmpdir(), 'gn-ensure-fts-')); try { await initLbug(join(dir, 'ensure-fts.lbug')); const ok = await loadFTSExtension(undefined, { policy: 'auto' }); console.log(ok ? 'FTS extension ready.' : 'FTS extension unavailable (continuing).'); + // VECTOR rides the same pre-install (#2623): the win32 gate is gone, so the + // vector suites genuinely run on Windows/macOS — installing once here means + // every sharded test process LOADs from ~/.lbdb instead of racing its own + // out-of-process INSTALL (bounded 15s each when the server is unreachable). + const vec = await loadVectorExtension(undefined, { policy: 'auto' }); + console.log(vec ? 'VECTOR extension ready.' : 'VECTOR extension unavailable (continuing).'); } catch (err) { console.warn(`ensure-fts: skipped (${err instanceof Error ? err.message : String(err)})`); } finally { diff --git a/gitnexus/src/cli/doctor.ts b/gitnexus/src/cli/doctor.ts index ce9811de6..7ec8f30f5 100644 --- a/gitnexus/src/cli/doctor.ts +++ b/gitnexus/src/cli/doctor.ts @@ -12,7 +12,11 @@ import { type EmbeddingRuntimeResolution, } from '../core/embeddings/runtime-install.js'; import { cudaRedirectDoctorStatus } from '../core/embeddings/onnxruntime-node-resolver.js'; -import { checkLbugNative, probeFtsExtensionLoad } from '../core/lbug/native-check.js'; +import { + checkLbugNative, + probeFtsExtensionLoad, + probeVectorExtensionLoad, +} from '../core/lbug/native-check.js'; import { getOsPageSize, isPageSizeAwareLadybug } from '../core/lbug/lbug-config.js'; import { diagnoseExtensionLoad } from '../core/lbug/extension-load-error.js'; import { getExtensionInstallPolicy } from '../core/lbug/extension-loader.js'; @@ -195,8 +199,32 @@ export const doctorCommand = async () => { console.log(` ${padDisplayEnd('', 18)}${remedy}`); } } - console.log(` ${label('doctor.labels.vectorIndex', 18)}${capabilities.vector}`); - console.log(` ${label('doctor.labels.semanticMode', 18)}${capabilities.semanticMode}`); + // Live LOAD probe for VECTOR too (#2623). The static capability is just + // `platform !== 'win32'`, so it printed "available" on the very machines + // where analyze was failing to load the extension — the same contradiction + // #2374 fixed for FTS above, and exactly what #2623's reporter saw while + // every incremental analyze died on an unloaded VECTOR extension. + const vectorProbe = nativeCheck.ok + ? await probeVectorExtensionLoad() + : { loaded: false, reason: 'LadybugDB native module (lbugjs.node) failed to load' }; + console.log( + ` ${label('doctor.labels.vectorIndex', 18)}${vectorProbe.loaded ? 'available' : 'unavailable'}`, + ); + if (!vectorProbe.loaded && vectorProbe.reason) { + console.log(` ${padDisplayEnd('', 18)}${vectorProbe.reason}`); + const { kind, remedy } = diagnoseExtensionLoad(vectorProbe.reason, 'VECTOR'); + if (kind !== 'unknown') { + console.log(` ${padDisplayEnd('', 18)}${remedy}`); + } + } + // Semantic mode follows the probe, not the platform: without a loadable + // VECTOR extension the index can be neither built nor queried, so search is + // really on exact scan no matter what the platform would allow. + console.log( + ` ${label('doctor.labels.semanticMode', 18)}${ + vectorProbe.loaded ? capabilities.semanticMode : 'exact-scan' + }`, + ); // Surface the optional-extension install policy so offline users can see // whether analyze/query will reach the network (extension.ladybugdb.com). // Literal label (like the 'native' line) to avoid adding i18n keys. diff --git a/gitnexus/src/core/lbug/extension-load-error.ts b/gitnexus/src/core/lbug/extension-load-error.ts index 67886fe96..712730164 100644 --- a/gitnexus/src/core/lbug/extension-load-error.ts +++ b/gitnexus/src/core/lbug/extension-load-error.ts @@ -101,18 +101,24 @@ const POSIX_MISSING_DEPENDENCY_SIGNATURES: readonly RegExp[] = [ * display language — the only localized part is the OS-error tail after it. So * it is the language-independent fallback signal once the specific tails miss: a * French/German/Japanese Windows 126 has a localized tail we cannot enumerate, - * but it still carries this wrapper. See HEDGED_LOAD_FAILURE_REMEDY. + * but it still carries this wrapper. See hedgedLoadFailureRemedy. */ const LOAD_FAILURE_WRAPPER = /failed to load library/i; -const MISSING_FILE_REMEDY = - 'The FTS extension is not installed. Re-run with network access and ' + - 'GITNEXUS_LBUG_EXTENSION_INSTALL=auto (or `gitnexus analyze --repair-fts`) to download it.'; +// Remedies are label-parameterized (#2623 follow-up): doctor now live-probes +// VECTOR through the same classifier, and FTS-specific advice (`--repair-fts` +// repairs FTS indexes only) must not be dispensed for other extensions. +const repairFtsHint = (label: string, lead: string): string => + label === 'FTS' ? ` (${lead}\`gitnexus analyze --repair-fts\`)` : ''; -const CORRUPT_FILE_REMEDY = - 'The FTS extension file is present but unreadable (corrupt, truncated, or built for another ' + - 'platform). Re-download it with network access and GITNEXUS_LBUG_EXTENSION_INSTALL=auto ' + - '(`gitnexus analyze --repair-fts`).'; +const missingFileRemedy = (label: string): string => + `The ${label} extension is not installed. Re-run with network access and ` + + `GITNEXUS_LBUG_EXTENSION_INSTALL=auto${repairFtsHint(label, 'or ')} to download it.`; + +const corruptFileRemedy = (label: string): string => + `The ${label} extension file is present but unreadable (corrupt, truncated, or built for another ` + + `platform). Re-download it with network access and ` + + `GITNEXUS_LBUG_EXTENSION_INSTALL=auto${repairFtsHint(label, '')}.`; // Single source of truth for the VC++ runtime-install pointer, shared by the // Windows-126 and structural missing-dependency remedies so the name/URL cannot @@ -122,15 +128,15 @@ const VC_REDIST_INSTALL_HINT = 'https://aka.ms/vs/17/release/vc_redist.x64.exe'; // MSVC-first per DuckDB's canonical answer for this exact error; OpenSSL second. -const WINDOWS_MISSING_DEPENDENCY_REMEDY = - 'The FTS extension is present but a required runtime library is missing (Windows error 126). ' + +const windowsMissingDependencyRemedy = (label: string): string => + `The ${label} extension is present but a required runtime library is missing (Windows error 126). ` + 'Reinstalling the extension will NOT help. Install ' + VC_REDIST_INSTALL_HINT + '; if the error persists, the extension also needs OpenSSL 3 ' + '(libcrypto-3-x64.dll / libssl-3-x64.dll) on the DLL search path.'; -const POSIX_MISSING_DEPENDENCY_REMEDY = - 'The FTS extension is present but a shared library it depends on could not be loaded (named in ' + +const posixMissingDependencyRemedy = (label: string): string => + `The ${label} extension is present but a shared library it depends on could not be loaded (named in ` + 'the error above). Reinstalling the extension will NOT help — install that library or add it to ' + 'your loader search path.'; @@ -140,16 +146,18 @@ const POSIX_MISSING_DEPENDENCY_REMEDY = // branches — rather than confidently prescribing the wrong single fix. The clean // long-term fix is upstream: have LadybugDB include the numeric GetLastError/errno // in the message (as it already does elsewhere), so this becomes a code match. -const HEDGED_LOAD_FAILURE_REMEDY = - 'The FTS extension file was found but could not be loaded — see the "Error:" text above (shown ' + +const hedgedLoadFailureRemedy = (label: string): string => + `The ${label} extension file was found but could not be loaded — see the "Error:" text above (shown ` + "in your system's language). Reinstalling usually will not help. If it names a missing module or " + 'library, install the required runtime (on Windows: the Microsoft Visual C++ 2015-2022 ' + - 'Redistributable x64 and OpenSSL 3); if it names a corrupt or invalid file, run ' + - '`gitnexus analyze --repair-fts` to re-download.'; + 'Redistributable x64 and OpenSSL 3); if it names a corrupt or invalid file, ' + + (label === 'FTS' + ? 'run `gitnexus analyze --repair-fts` to re-download.' + : 're-run analyze with network access and GITNEXUS_LBUG_EXTENSION_INSTALL=auto to re-download.'); -const UNKNOWN_REMEDY = - 'The FTS extension failed to load for an unrecognized reason. Run `gitnexus doctor` for live ' + - 'FTS status and verify the extension file and platform.'; +const unknownRemedy = (label: string): string => + `The ${label} extension failed to load for an unrecognized reason. Run \`gitnexus doctor\` for live ` + + `${label} status and verify the extension file and platform.`; const matchesAny = (reason: string, signatures: readonly RegExp[]): boolean => signatures.some((re) => re.test(reason)); @@ -162,19 +170,20 @@ const matchesAny = (reason: string, signatures: readonly RegExp[]): boolean => */ export function classifyExtensionLoadError( reason: string | undefined | null, + label: string = 'FTS', ): ExtensionLoadDiagnosis { const text = reason ?? ''; if (matchesAny(text, MISSING_FILE_SIGNATURES)) { - return { kind: 'missing_file', remedy: MISSING_FILE_REMEDY }; + return { kind: 'missing_file', remedy: missingFileRemedy(label) }; } if (matchesAny(text, FILE_CORRUPTION_SIGNATURES)) { - return { kind: 'corrupt_file', remedy: CORRUPT_FILE_REMEDY }; + return { kind: 'corrupt_file', remedy: corruptFileRemedy(label) }; } if (matchesAny(text, WINDOWS_MISSING_DEPENDENCY_SIGNATURES)) { - return { kind: 'missing_dependency', remedy: WINDOWS_MISSING_DEPENDENCY_REMEDY }; + return { kind: 'missing_dependency', remedy: windowsMissingDependencyRemedy(label) }; } if (matchesAny(text, POSIX_MISSING_DEPENDENCY_SIGNATURES)) { - return { kind: 'missing_dependency', remedy: POSIX_MISSING_DEPENDENCY_REMEDY }; + return { kind: 'missing_dependency', remedy: posixMissingDependencyRemedy(label) }; } // Language-independent fallback: the extension demonstrably failed to load // (lbug's English wrapper is present) but the localized OS tail matched no @@ -182,9 +191,9 @@ export function classifyExtensionLoadError( // remedy — strictly better than the generic `unknown` for non-English hosts, // and it never prescribes the wrong fix. if (LOAD_FAILURE_WRAPPER.test(text)) { - return { kind: 'missing_dependency', remedy: HEDGED_LOAD_FAILURE_REMEDY }; + return { kind: 'missing_dependency', remedy: hedgedLoadFailureRemedy(label) }; } - return { kind: 'unknown', remedy: UNKNOWN_REMEDY }; + return { kind: 'unknown', remedy: unknownRemedy(label) }; } // ── Language-independent structural layer ──────────────────────────────────── @@ -192,8 +201,8 @@ export function classifyExtensionLoadError( /** Well-formedness of the extension binary for the host platform + arch. */ export type ExtensionBinaryState = 'absent' | 'corrupt' | 'valid' | 'indeterminate'; -const STRUCTURAL_MISSING_DEPENDENCY_REMEDY = - 'The FTS extension file is valid, so the failure is a missing or incompatible runtime dependency, ' + +const structuralMissingDependencyRemedy = (label: string): string => + `The ${label} extension file is valid, so the failure is a missing or incompatible runtime dependency, ` + 'not the extension itself — reinstalling will NOT help. On Windows, install ' + VC_REDIST_INSTALL_HINT + ' and ensure OpenSSL 3 is available; on Linux/macOS install the shared library named in the error above.'; @@ -332,13 +341,16 @@ export function inspectExtensionBinary( * classifier (which still carries the language-independent hedged fallback). This * is the entry point every surface should call. */ -export function diagnoseExtensionLoad(reason: string | undefined | null): ExtensionLoadDiagnosis { +export function diagnoseExtensionLoad( + reason: string | undefined | null, + label: string = 'FTS', +): ExtensionLoadDiagnosis { const text = reason ?? ''; - const stringResult = classifyExtensionLoadError(text); + const stringResult = classifyExtensionLoadError(text, label); const fileState = inspectExtensionBinary(extractExtensionPath(text)); if (fileState === 'corrupt') { - return { kind: 'corrupt_file', remedy: CORRUPT_FILE_REMEDY }; + return { kind: 'corrupt_file', remedy: corruptFileRemedy(label) }; } if (fileState === 'valid') { // The structural probe only inspects the first BINARY_HEADER_BYTES, so a file @@ -357,7 +369,7 @@ export function diagnoseExtensionLoad(reason: string | undefined | null): Extens const remedy = stringResult.kind === 'missing_dependency' ? stringResult.remedy - : STRUCTURAL_MISSING_DEPENDENCY_REMEDY; + : structuralMissingDependencyRemedy(label); return { kind: 'missing_dependency', remedy }; } // 'absent' or 'indeterminate' → no positive structural evidence, so defer to the diff --git a/gitnexus/src/core/lbug/extension-loader.ts b/gitnexus/src/core/lbug/extension-loader.ts index c1705336a..b6166a1aa 100644 --- a/gitnexus/src/core/lbug/extension-loader.ts +++ b/gitnexus/src/core/lbug/extension-loader.ts @@ -323,7 +323,7 @@ export class ExtensionManager { name, loaded: false, reason, - diagnosis: diagnoseExtensionLoad(reason), + diagnosis: diagnoseExtensionLoad(reason, label), }); const key = `${name}:${reason}`; if (this.warnedKeys.has(key)) return; diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 0be12b947..2319507ea 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -23,7 +23,11 @@ import { streamAllCSVsToDisk, type StreamedCSVResult } from './csv-generator.js' import type { PdgEmitManifest } from './pdg-emit-sink.js'; import { getNodeLabel as deriveNodeLabel, type WriteStreamFactory } from './rel-pair-routing.js'; import { EMBEDDABLE_LABELS, type CachedEmbedding } from '../embeddings/types.js'; -import { extensionManager, type ExtensionEnsureOptions } from './extension-loader.js'; +import { + extensionManager, + resolveAnalyzeInstallPolicy, + type ExtensionEnsureOptions, +} from './extension-loader.js'; import { classifyDeleteAllError, closeLbugConnection, @@ -51,7 +55,6 @@ import { renameFailureMessage, shadowSidecarRecoveryMessage, } from './sidecar-recovery.js'; -import { isVectorExtensionSupportedByPlatform } from '../platform/capabilities.js'; import { logger } from '../logger.js'; // --------------------------------------------------------------------------- @@ -2696,14 +2699,16 @@ export const loadVectorExtension = async ( ): Promise => { const useModuleState = targetConn === undefined; if (useModuleState && vectorExtensionLoaded) return true; - // INSTALL VECTOR crashes with SIGSEGV on Windows: the KuzuDB native extension - // installer has an unhandled error path on Windows that raises a fatal signal - // that JS try/catch cannot intercept. Skip loading — vector/embedding search - // is unavailable but all graph index queries still work. Do NOT set - // vectorExtensionLoaded here: the flag means "successfully loaded", and a - // subsequent call would otherwise short-circuit to `return true` at the top. - if (process.platform === 'win32') return false; - if (!isVectorExtensionSupportedByPlatform()) return false; + // No platform gate. Windows was hard-refused here for years on the strength + // of an early-era report that in-process INSTALL VECTOR could SIGSEGV + // (#1365) — but the extension server ships win_amd64 VECTOR artifacts for + // every 0.18.x extension version (probed live: v0.18.0 and v0.18.1 both + // serve a real PE32+ DLL; the pinned 0.18.2 core resolves its extension + // directory to 0.18.1, strace-verified), and INSTALL now runs in a spawned + // child process (installDuckDbExtensionOutOfProcess), so even a crashing + // installer kills only the child and degrades to `false` here. LOAD of a + // present extension file is an ordinary in-process load whose failures + // surface as catchable errors, exactly like FTS. const c: lbug.Connection | null = targetConn ?? conn; if (!c) { @@ -2812,6 +2817,78 @@ export const createVectorIndex = async (): Promise => { } }; +/** + * Make DML against {@link EMBEDDING_TABLE_NAME} legal on the writable + * connection when it can be, and report whether it is. + * + * LadybugDB refuses EVERY mutation of a table carrying an HNSW index while + * the VECTOR extension is not loaded on that connection: `DELETE` fails with + * "Trying to delete from an index on table CodeEmbedding but its extension is + * not loaded", `CREATE` with the matching "insert into an index" variant, + * `DROP TABLE` is refused while the index references it, and `SET` — even on + * a NON-indexed property — segfaults the process outright. Probed against + * @ladybugdb/core 0.18.2 (the lockfile-pinned version) and 0.18.0 — every + * result identical on both (#2623). + * + * Dropping the index is NOT an available recovery: `CALL DROP_VECTOR_INDEX` + * is itself a VECTOR-extension function and resolves to "Catalog exception: + * function DROP_VECTOR_INDEX is not defined" in exactly the state it would + * need to rescue. Loading the extension is the only in-place repair, which is + * why this returns a verdict instead of attempting a fixup. + * + * `true` = embedding-row DML is safe: either VECTOR is now loaded, or the + * table carries no index to trip over. `false` = genuinely blocked (index + * present, extension unloadable); the analyze orchestrator answers that by + * escalating to the wipe-and-rebuild write plan instead of failing + * mid-writeback. + * + * Cheap by construction: one local `SHOW_INDEXES` read settles the common + * "this repo never built an embedding index" case without touching the + * extension machinery at all, so a VECTOR-less machine is not charged a + * bounded INSTALL attempt on every incremental analyze. `SHOW_INDEXES` is + * readable WITHOUT the extension and reports `extension_loaded` per index, so + * no error-string sniffing is needed; it runs through the unprepared + * `conn.query()` path like every other `CALL` procedure here (#2114). + */ +export const ensureEmbeddingRowDmlSafe = async (): Promise => { + const targetConn = conn; + if (!targetConn) { + throw new Error('LadybugDB not initialized. Call initLbug first.'); + } + // Catalog FIRST. The overwhelmingly common case on a repo that never enabled + // embeddings is "no index at all", and that is provable with one local read + // — no extension needed. Loading first would make every incremental analyze + // on a VECTOR-less machine pay a bounded out-of-process INSTALL attempt (the + // `auto` policy) plus an "extension unavailable" warning, for a repo that + // can never hit this hazard. + let indexRows: any[] | undefined; + try { + indexRows = await withConnLock(async () => + readQueryRows(await targetConn.query('CALL SHOW_INDEXES() RETURN *')), + ); + } catch (err) { + // Fall through to the load attempt: unable to prove the index is absent, + // so the extension is the only thing that can make DML safe. + logger.warn( + { err }, + `Could not read the index catalog to check for a ${EMBEDDING_TABLE_NAME} vector index; ` + + 'falling back to loading the VECTOR extension.', + ); + } + // Any non-HASH index on the embedding table gates DML. Keyed on index TYPE, + // not name, so an index built under a different name still counts; the + // implicit primary-key HASH index is engine-internal and never gates. + const indexGatesDml = + indexRows === undefined || + indexRows.some((row) => { + const table = row?.table_name ?? row?.[0]; + if (table !== EMBEDDING_TABLE_NAME) return false; + return (row?.index_type ?? row?.[2]) !== 'HASH'; + }); + if (!indexGatesDml) return true; + return await loadVectorExtension(undefined, { policy: resolveAnalyzeInstallPolicy() }); +}; + /** * Lazy-create an FTS index, caching the fact in-process. * diff --git a/gitnexus/src/core/lbug/native-check.ts b/gitnexus/src/core/lbug/native-check.ts index accf43a52..a9971a5e9 100644 --- a/gitnexus/src/core/lbug/native-check.ts +++ b/gitnexus/src/core/lbug/native-check.ts @@ -96,6 +96,9 @@ export interface FtsProbeResult { reason?: string; } +/** Same shape for every optional extension; `FtsProbeResult` is the legacy name. */ +export type ExtensionProbeResult = FtsProbeResult; + const DEFAULT_FTS_PROBE_TIMEOUT_MS = 10_000; /** A LadybugDB query result exposes a synchronous `close()`. */ @@ -136,8 +139,39 @@ const closeProbeResults = (result: unknown): void => { export async function probeFtsExtensionLoad( timeoutMs: number = DEFAULT_FTS_PROBE_TIMEOUT_MS, ): Promise { + return await probeExtensionLoad('fts', timeoutMs); +} + +/** + * Live-probe `LOAD EXTENSION vector`, the VECTOR counterpart of the FTS probe. + * + * Needed for the same reason #2374 needed the FTS one, and reported the same + * way: #2623's reporter saw `doctor` print `VECTOR index: available` while + * every incremental `analyze` was dying because the extension had not loaded. + * `doctor` derived that line from a static platform capability, so it read + * "available" no matter what the extension file was doing. + * + * Probes for real on every platform, Windows included: the extension server + * ships win_amd64 VECTOR artifacts for every 0.18.x extension version (the + * old blanket Windows refusal was stale, #1365-era). LOAD never touches the + * network and never invokes the installer, so this probe is exactly as safe + * as the FTS one above. + */ +export async function probeVectorExtensionLoad( + timeoutMs: number = DEFAULT_FTS_PROBE_TIMEOUT_MS, +): Promise { + return await probeExtensionLoad('vector', timeoutMs); +} + +/** + * Shared LOAD probe. `extension` is a fixed internal literal, never user input. + */ +async function probeExtensionLoad( + extension: 'fts' | 'vector', + timeoutMs: number, +): Promise { let timer: ReturnType | undefined; - const timeout = new Promise((resolve) => { + const timeout = new Promise((resolve) => { timer = setTimeout( () => resolve({ @@ -148,7 +182,7 @@ export async function probeFtsExtensionLoad( ); }); - const probe = (async (): Promise => { + const probe = (async (): Promise => { try { const { default: lbug } = await import('@ladybugdb/core'); const db = new lbug.Database(':memory:'); @@ -156,7 +190,7 @@ export async function probeFtsExtensionLoad( try { const conn = new lbug.Connection(db); try { - const result = await conn.query('LOAD EXTENSION fts'); + const result = await conn.query(`LOAD EXTENSION ${extension}`); closeProbeResults(result); return { loaded: true }; } finally { diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index a53ca0c92..d88976fa0 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -17,7 +17,7 @@ import fs from 'fs/promises'; import lbug from '@ladybugdb/core'; -import { isReadOnlyDbError, loadFTSExtension } from './lbug-adapter.js'; +import { isReadOnlyDbError, loadFTSExtension, loadVectorExtension } from './lbug-adapter.js'; import { closeQueryResults } from './query-result-utils.js'; import { createLbugDatabase, @@ -126,6 +126,14 @@ interface SharedDB { db: lbug.Database; refCount: number; ftsLoaded: boolean; + /** VECTOR loaded on this Database. Extension load scope is per-Database + * (probe-verified on @ladybugdb/core 0.18.x): loading on any one + * connection enables QUERY_VECTOR_INDEX on every connection of the same + * Database. Without this load the pool's vector lane raised a Catalog + * exception on every semantic query and silently fell back to the exact + * scan (#2623 follow-up). Optional with `?? false` semantics so the + * construction sites stay minimal. */ + vectorLoaded?: boolean; /** File identity at open — used to detect reuse of a shared read-only handle * whose on-disk index was rebuilt/swapped since it opened (only reachable * when a second pool consumer shares this dbPath; #2614 F2). */ @@ -358,6 +366,7 @@ function closeOne(repoId: string): void { // for the same dbPath reuse it instead of hitting a file lock. shared.refCount = 0; shared.ftsLoaded = false; + shared.vectorLoaded = false; } else { shared.db.close().catch(() => {}); dbCache.delete(entry.dbPath); @@ -810,6 +819,13 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { if (!shared.ftsLoaded) { shared.ftsLoaded = await loadFTSExtension(available[0], { policy: 'load-only' }); } + // VECTOR too — extension load scope is per-Database, so this one load + // makes QUERY_VECTOR_INDEX legal on every pooled connection. Same + // load-only contract as FTS above; on failure the semantic-query lane + // falls back to the exact scan with its own diagnostic (#2623 follow-up). + if (!shared.vectorLoaded) { + shared.vectorLoaded = await loadVectorExtension(available[0], { policy: 'load-only' }); + } // Register pool entry only after all connections are pre-warmed and FTS is // loaded. Concurrent executeQuery calls see either "not initialized" @@ -880,6 +896,11 @@ export async function initLbugWithDb( if (!shared.ftsLoaded) { shared.ftsLoaded = await loadFTSExtension(available[0], { policy: 'load-only' }); } + // VECTOR too — same per-Database scope and load-only contract as the + // doInitLbug site above (#2623 follow-up). + if (!shared.vectorLoaded) { + shared.vectorLoaded = await loadVectorExtension(available[0], { policy: 'load-only' }); + } pool.set(repoId, { db: existingDb, diff --git a/gitnexus/src/core/platform/capabilities.ts b/gitnexus/src/core/platform/capabilities.ts index 6bfe459e4..ff2195e22 100644 --- a/gitnexus/src/core/platform/capabilities.ts +++ b/gitnexus/src/core/platform/capabilities.ts @@ -86,23 +86,24 @@ export const getRuntimeFingerprint = (): RuntimeFingerprint => ({ onnxruntime: packageVersion('onnxruntime-node'), }); -export const isVectorExtensionSupportedByPlatform = ( - platform: NodeJS.Platform = process.platform, -): boolean => platform !== 'win32'; - export const getRuntimeCapabilities = (): RuntimeCapabilities => { - const vector = isVectorExtensionSupportedByPlatform() ? 'available' : 'unavailable'; const exactScanLimit = getExactScanLimit(); + // Static PLATFORM capability only. LadybugDB ships the VECTOR extension for + // every platform gitnexus supports — the extension server hosts win_amd64 + // artifacts for every 0.18.x extension version (probed: v0.18.0 and v0.18.1 + // both return a real 14 MB PE32+ DLL; the pinned 0.18.2 core resolves its + // extension directory to 0.18.1, strace-verified), so the old + // `platform !== 'win32'` gate was stale (#1365-era). Whether the extension + // actually LOADS on a given machine is a runtime question — doctor answers + // it with probeVectorExtensionLoad, and analyze/query degrade to exact scan + // when the load fails. return { graph: 'available', fts: 'available', - vector, - semanticMode: vector === 'available' ? 'vector-index' : 'exact-scan', + vector: 'available', + semanticMode: 'vector-index', exactScanLimit, - reason: - vector === 'unavailable' - ? 'LadybugDB VECTOR is disabled on this platform; semantic search uses exact scan when embeddings exist.' - : undefined, + reason: undefined, }; }; diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index cd3c9a458..b404866a5 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -25,6 +25,7 @@ import { closeLbugBeforeExit, loadCachedEmbeddings, deleteNodesForFiles, + ensureEmbeddingRowDmlSafe, deleteAllCommunitiesAndProcesses, deleteAllInterprocTaintPaths, deleteAllCallSummaries, @@ -1686,7 +1687,44 @@ export async function runFullAnalysis( // DB write plan changes here; fileHashes/meta bookkeeping is identical. // Thresholds + the AND-gate live in incremental/escalation-gate.ts. const writeFraction = effectiveWriteSet.size / Math.max(1, allFilePaths.length); + // VECTOR gate (#2623) — load the extension BEFORE a single embedding row + // is touched. `deleteNodesForFiles` below opens with the CodeEmbedding + // join-delete, and LadybugDB refuses all DML on a table carrying its HNSW + // index unless VECTOR is loaded on this connection; nothing else on this + // path loads it until Phase 4, so every incremental run over a DB that + // already built `code_embedding_idx` died here. Same seam the FTS drop + // occupies at the head of this branch (#2589): index lifecycle first, + // then rows. UNCONDITIONAL — not gated on `shouldGenerateEmbeddings` — + // because a DB carrying the index from an earlier `--embeddings` run hits + // the identical wall on a plain incremental run. + // + // When VECTOR genuinely cannot load, the table is immutable (the index + // cannot be dropped without the extension either), so surgery is + // impossible: fall through to the escalation valve's wipe-and-COPY plan, + // which rebuilds the DB files outright and needs no embedding-row DML. + const embeddingRowDmlSafe = await ensureEmbeddingRowDmlSafe(); + if (!embeddingRowDmlSafe && cachedEmbeddings.length === 0) { + // The escalation below WIPES the DB files, and Phase 3.5 restores + // embedding rows from `cachedEmbeddings` — which is only populated when + // `deriveEmbeddingMode` saw `meta.stats.embeddings > 0`. A DB whose meta + // under-reports its embeddings (meta restored from an older run, or a + // count that never got stamped) would therefore have every vector + // silently destroyed by a rebuild it did not ask for. Read them now, + // while the DB is still intact — a plain MATCH, which needs no VECTOR + // extension. Rows whose owning node is gone are dropped by Phase 3.5's + // live-graph filter, exactly as on any other wiped path. + const rescued = await loadCachedEmbeddings(); + if (rescued.embeddings.length > 0) { + cachedEmbeddings = rescued.embeddings; + cachedEmbeddingNodeIds = rescued.embeddingNodeIds; + log( + `Preserving ${rescued.embeddings.length} embedding row(s) across the forced rebuild ` + + `(the index metadata did not account for them).`, + ); + } + } if ( + !embeddingRowDmlSafe || shouldEscalateIncrementalWrite( filesToDelete.length, effectiveWriteSet.size, @@ -1695,13 +1733,20 @@ export async function runFullAnalysis( ) { escalatedFullWrite = true; log( - `Incremental: effective write set covers ${effectiveWriteSet.size}/${allFilePaths.length} ` + - // Display clamp only (predicate unchanged): BFS-found deleted - // importers can push the numerator past the CURRENT file list, so - // the raw fraction can exceed 1 — see the population-mismatch note - // on shouldEscalateIncrementalWrite (tri-review 4669518496). - `files (${Math.min(100, Math.round(writeFraction * 100))}%) — switching to a full DB write ` + - `(wipe + bulk COPY) for this run; file-level incremental bookkeeping is unaffected.`, + !embeddingRowDmlSafe + ? `Incremental: the ${EMBEDDING_TABLE_NAME} vector index exists but the VECTOR ` + + `extension could not be loaded, so embedding rows cannot be rewritten in place — ` + + `switching to a full DB write (wipe + bulk COPY) for this run. Semantic search ` + + `falls back to exact scan until VECTOR is available; run \`gitnexus doctor\` for ` + + `live extension status, or set GITNEXUS_LBUG_EXTENSION_INSTALL=auto to allow one ` + + `bounded install attempt.` + : `Incremental: effective write set covers ${effectiveWriteSet.size}/${allFilePaths.length} ` + + // Display clamp only (predicate unchanged): BFS-found deleted + // importers can push the numerator past the CURRENT file list, so + // the raw fraction can exceed 1 — see the population-mismatch note + // on shouldEscalateIncrementalWrite (tri-review 4669518496). + `files (${Math.min(100, Math.round(writeFraction * 100))}%) — switching to a full DB write ` + + `(wipe + bulk COPY) for this run; file-level incremental bookkeeping is unaffected.`, ); // toWriteCount: 0 is the established full-path dirty-flag sentinel; // the real counters ride along for crash diagnostics. @@ -2067,8 +2112,8 @@ export async function runFullAnalysis( // the case a naive gate would leave index-less again. // buildVectorIndex carries its own extension-policy gate and // warn-on-failure; the boolean feeds semanticMode so the finalize stamp - // reflects the DB's ACTUAL state even when recreation fails (win32 / - // extension unavailable → 'exact-scan'). + // reflects the DB's ACTUAL state even when recreation fails (extension + // unavailable → 'exact-scan'). const dbWasWiped = !isIncremental || escalatedFullWrite; if (restoredEmbeddingCount > 0 && dbWasWiped && embeddingSkipped) { // Re-import at the seam rather than thread a mutable capture from diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 2ed048fb9..7cc72eccb 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -61,10 +61,7 @@ import { type ExactEmbeddingRow, } from '../../core/embeddings/exact-search.js'; import { EMBEDDING_TABLE_NAME, EMBEDDING_INDEX_NAME } from '../../core/lbug/schema.js'; -import { - getExactScanLimit, - isVectorExtensionSupportedByPlatform, -} from '../../core/platform/capabilities.js'; +import { getExactScanLimit } from '../../core/platform/capabilities.js'; import { PhaseTimer } from '../../core/search/phase-timer.js'; import { ftsDegradedWarning } from '../../core/search/fts-indexes.js'; import { @@ -2419,10 +2416,16 @@ export class LocalBackend { string, { distance: number; chunkIndex: number; startLine: number; endLine: number } >(); - if (isVectorExtensionSupportedByPlatform()) { - try { - bestChunks = await collectBestChunks(limit, async (fetchLimit) => { - const vectorQuery = ` + // Always TRY the vector lane — no platform gate. LadybugDB ships the + // VECTOR extension for every supported platform, Windows included + // (#2623 follow-up; the old `platform !== 'win32'` gate was stale), so + // whether the index is queryable is a per-machine runtime fact. The + // catch below is the fallback: any failure (extension unloadable, index + // absent, older DB) degrades to the exact scan with a once-per-backend + // diagnostic instead of being silently swallowed. + try { + bestChunks = await collectBestChunks(limit, async (fetchLimit) => { + const vectorQuery = ` CALL QUERY_VECTOR_INDEX('${EMBEDDING_TABLE_NAME}', '${EMBEDDING_INDEX_NAME}', CAST(${queryVecStr} AS FLOAT[${dims}]), ${fetchLimit}) YIELD node AS emb, distance @@ -2433,27 +2436,27 @@ export class LocalBackend { ORDER BY distance `; - const embResults = await executeQuery(repo.lbugPath, vectorQuery); - return embResults.map((row) => ({ - nodeId: row.nodeId ?? row[0], - chunkIndex: row.chunkIndex ?? row[1] ?? 0, - startLine: row.startLine ?? row[2] ?? 0, - endLine: row.endLine ?? row[3] ?? 0, - distance: row.distance ?? row[4], - })); - }); - } catch { - bestChunks = new Map(); + const embResults = await executeQuery(repo.lbugPath, vectorQuery); + return embResults.map((row) => ({ + nodeId: row.nodeId ?? row[0], + chunkIndex: row.chunkIndex ?? row[1] ?? 0, + startLine: row.startLine ?? row[2] ?? 0, + endLine: row.endLine ?? row[3] ?? 0, + distance: row.distance ?? row[4], + })); + }); + } catch (err) { + bestChunks = new Map(); + if (!this.warnedVectorUnsupported) { + // Rare diagnostic: surface why semantic search fell back to the + // exact scan. Emitted once per `LocalBackend` instance lifetime to + // avoid noisy stderr on hot semantic-search paths (DoD §2.8). + this.warnedVectorUnsupported = true; + logger.warn( + { err }, + 'GitNexus [query:vector]: vector index query failed; using exact scan fallback', + ); } - } else if (!this.warnedVectorUnsupported) { - // Rare diagnostic: surface why we fell back to the exact scan path so - // operators can see at a glance that VECTOR is disabled by platform - // policy. Emitted once per `LocalBackend` instance lifetime to avoid - // noisy stderr on hot semantic-search paths (DoD §2.8). - this.warnedVectorUnsupported = true; - logger.warn( - 'GitNexus [query:vector]: VECTOR extension not supported on this platform; using exact scan fallback', - ); } if (bestChunks.size === 0) { diff --git a/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts b/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts index 736dba3f5..d8e037ba3 100644 --- a/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts +++ b/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts @@ -18,7 +18,7 @@ * the delete joins `e.nodeId = n.id` through the still-present nodes — * deleted/quoted files' rows go, survivors' rows stay. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; import path from 'path'; import { withTestLbugDB } from '../helpers/test-indexed-db.js'; import { buildTestGraph, type TestNodeInput, type TestRelInput } from '../helpers/test-graph.js'; @@ -276,3 +276,179 @@ withTestLbugDB('delete-nodes-missing-embedding-table', () => { }, 120_000); }); }); + +/** + * VECTOR-extension gate for embedding-row DML (#2623). + * + * LadybugDB refuses EVERY mutation of a table carrying an HNSW index while + * the VECTOR extension is not loaded on the connection. The surgical + * incremental writeback's FIRST statement is `deleteNodesForFiles`' embedding + * join-delete, and nothing on that path loaded VECTOR until Phase 4 — so an + * incremental analyze over a DB that already had `code_embedding_idx` died + * with "Trying to delete from an index on table CodeEmbedding but its + * extension is not loaded". + * + * `ensureEmbeddingRowDmlSafe` is the seam that answers "is embedding-row DML + * legal right now?" before a single row is touched. Own withTestLbugDB block: + * these cases close and reopen the DB under a different extension-install + * policy, which would wreck the sibling suites' shared connection. + */ +withTestLbugDB('embedding-row-dml-vector-gate', (handle) => { + describe('ensureEmbeddingRowDmlSafe (#2623)', () => { + const FILE_A = 'src/gate-a.ts'; + const FILE_B = 'src/gate-b.ts'; + const nodeIdFor = (fp: string): string => `Function:${fp}:fn:1`; + + /** Reopen the singleton connection under an explicit install policy. */ + const reopenWithPolicy = async (policy: string | undefined): Promise => { + const { initLbug, closeLbug } = await import('../../src/core/lbug/lbug-adapter.js'); + await closeLbug(); + if (policy === undefined) delete process.env.GITNEXUS_LBUG_EXTENSION_INSTALL; + else process.env.GITNEXUS_LBUG_EXTENSION_INSTALL = policy; + await initLbug(handle.dbPath); + }; + + const seedTwoFilesWithEmbeddings = async (): Promise => { + const { executeQuery, executeWithReusedStatement } = + await import('../../src/core/lbug/lbug-adapter.js'); + const { batchInsertEmbeddings } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + for (const fp of [FILE_A, FILE_B]) { + await executeQuery( + `CREATE (:Function {id: '${nodeIdFor(fp)}', name: 'fn', filePath: '${fp}', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + ); + } + await batchInsertEmbeddings( + executeWithReusedStatement, + [FILE_A, FILE_B].map((fp) => ({ + nodeId: nodeIdFor(fp), + chunkIndex: 0, + startLine: 1, + endLine: 3, + embedding: new Array(EMBEDDING_DIMS).fill(0.1), + contentHash: `hash-${fp}`, + })), + ); + }; + + const embeddingCountFor = async (fp: string): Promise => { + const { executeQuery } = await import('../../src/core/lbug/lbug-adapter.js'); + const rows = (await executeQuery( + `MATCH (e:${EMBEDDING_TABLE_NAME}) WHERE e.nodeId = '${nodeIdFor(fp)}' RETURN count(e) AS c`, + )) as Array<{ c: number | bigint }>; + return Number(rows[0]?.c ?? 0); + }; + + const clearSeed = async (): Promise => { + const { executeQuery } = await import('../../src/core/lbug/lbug-adapter.js'); + await executeQuery(`MATCH (e:${EMBEDDING_TABLE_NAME}) DELETE e`); + await executeQuery(`MATCH (n:Function) DETACH DELETE n`); + }; + + afterEach(async () => { + await reopenWithPolicy(undefined); + // Teardown deletes embedding rows, so it is itself subject to #2623 once + // a case has built the index — load VECTOR before clearing. + const { ensureEmbeddingRowDmlSafe } = await import('../../src/core/lbug/lbug-adapter.js'); + await ensureEmbeddingRowDmlSafe(); + await clearSeed(); + }); + + it('no vector index + VECTOR unavailable → safe, and the delete still works', async () => { + await seedTwoFilesWithEmbeddings(); + await reopenWithPolicy('never'); + const { ensureEmbeddingRowDmlSafe, deleteNodesForFiles } = + await import('../../src/core/lbug/lbug-adapter.js'); + + // No HNSW index was ever built, so there is nothing to gate on — the + // degraded path must NOT escalate needlessly. + await expect(ensureEmbeddingRowDmlSafe()).resolves.toBe(true); + await expect(deleteNodesForFiles([FILE_A])).resolves.toBeUndefined(); + expect(await embeddingCountFor(FILE_A)).toBe(0); + expect(await embeddingCountFor(FILE_B)).toBe(1); + }, 120_000); + + it('vector index present + VECTOR unavailable → blocked, and the raw delete throws', async () => { + await seedTwoFilesWithEmbeddings(); + const { createVectorIndex } = await import('../../src/core/lbug/lbug-adapter.js'); + const built = await createVectorIndex(); + if (!built) return; // VECTOR not installable here — nothing to assert. + + await reopenWithPolicy('never'); + const { ensureEmbeddingRowDmlSafe, deleteNodesForFiles } = + await import('../../src/core/lbug/lbug-adapter.js'); + + // The gate must SEE the hazard… + await expect(ensureEmbeddingRowDmlSafe()).resolves.toBe(false); + // …and the hazard must be real: this is the exact #2623 failure. + await expect(deleteNodesForFiles([FILE_A])).rejects.toThrow(/extension is not loaded/); + // Nothing was destroyed by the refused statement. + expect(await embeddingCountFor(FILE_A)).toBe(1); + }, 120_000); + + it('vector index present + VECTOR loadable → safe, delete works, index survives', async () => { + await seedTwoFilesWithEmbeddings(); + const { createVectorIndex } = await import('../../src/core/lbug/lbug-adapter.js'); + const built = await createVectorIndex(); + if (!built) return; // VECTOR not installable here — nothing to assert. + + // Reopen so the in-process "already loaded" latch cannot mask a missing + // load — this is the state a second `analyze` run actually starts from. + await reopenWithPolicy(undefined); + const { ensureEmbeddingRowDmlSafe, deleteNodesForFiles, executeQuery } = + await import('../../src/core/lbug/lbug-adapter.js'); + + await expect(ensureEmbeddingRowDmlSafe()).resolves.toBe(true); + await expect(deleteNodesForFiles([FILE_A])).resolves.toBeUndefined(); + expect(await embeddingCountFor(FILE_A)).toBe(0); + expect(await embeddingCountFor(FILE_B)).toBe(1); + + // The surgical path KEEPS its index (run-analyze relies on HNSW + // self-maintaining across insert/delete) — it must still be there. + const indexes = (await executeQuery('CALL SHOW_INDEXES() RETURN *')) as Array<{ + table_name?: string; + index_type?: string; + }>; + expect( + indexes.some((r) => r.table_name === EMBEDDING_TABLE_NAME && r.index_type === 'HNSW'), + ).toBe(true); + }, 120_000); + + it('catalog read fails → falls back to attempting the extension load (fail-safe)', async () => { + // The one branch where the gate cannot cheaply prove safety: SHOW_INDEXES + // itself errors. It must fall through to loadVectorExtension — in this + // environment the extension IS loadable, so the verdict is still `true` + // and DML proceeds safely despite the unreadable catalog. + await seedTwoFilesWithEmbeddings(); + // Reopen so the module-level "already loaded" latch cannot let + // loadVectorExtension return true without issuing a LOAD statement. + await reopenWithPolicy('load-only'); + const { ensureEmbeddingRowDmlSafe } = await import('../../src/core/lbug/lbug-adapter.js'); + const { default: lbug } = await import('@ladybugdb/core'); + + const originalQuery = lbug.Connection.prototype.query; + const seen: string[] = []; + const spy = vi.spyOn(lbug.Connection.prototype, 'query').mockImplementation(function ( + this: unknown, + sql: string, + ...rest: unknown[] + ) { + seen.push(sql); + if (sql.includes('SHOW_INDEXES')) { + return Promise.reject(new Error('Catalog exception: forced by test')); + } + return originalQuery.call(this, sql, ...rest); + }); + + try { + await expect(ensureEmbeddingRowDmlSafe()).resolves.toBe(true); + // The catalog read was attempted and failed… + expect(seen.some((s) => s.includes('SHOW_INDEXES'))).toBe(true); + // …and the fallback really attempted the LOAD instead of guessing. + expect(seen.some((s) => s.toUpperCase().includes('LOAD'))).toBe(true); + } finally { + spy.mockRestore(); + } + }, 120_000); + }); +}); diff --git a/gitnexus/test/integration/lbug-pool.test.ts b/gitnexus/test/integration/lbug-pool.test.ts index 561772a75..083974674 100644 --- a/gitnexus/test/integration/lbug-pool.test.ts +++ b/gitnexus/test/integration/lbug-pool.test.ts @@ -315,3 +315,91 @@ withTestLbugDB( poolAdapter: true, }, ); + +/** + * Pool vector lane (#2623 follow-up). + * + * Extension load scope is per-Database, and the pool pre-warm historically + * loaded only FTS — so `CALL QUERY_VECTOR_INDEX` through the pool ALWAYS + * raised `Catalog exception: function QUERY_VECTOR_INDEX is not defined` and + * LocalBackend's semantic lane silently exact-scanned. This block pins that + * the pool's shared Database really can serve the vector lane: rows and the + * HNSW index are built through the core adapter first (the state `analyze + * --embeddings` leaves behind), then the pool opens and must answer a vector + * query. Own withTestLbugDB block: the vector index would leak into the + * sibling suites' shared fixture expectations. + */ +withTestLbugDB( + 'lbug-pool-vector-lane', + (handle) => { + describe('pool vector lane (#2623 follow-up)', () => { + afterEach(async () => { + try { + await closeLbug('vec-repo'); + } catch { + /* best-effort */ + } + }); + + it('QUERY_VECTOR_INDEX works through the pool once the pre-warm loads VECTOR', async (ctx) => { + const core = await import('../../src/core/lbug/lbug-adapter.js'); + const { batchInsertEmbeddings } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + const { EMBEDDING_TABLE_NAME, EMBEDDING_INDEX_NAME, EMBEDDING_DIMS } = + await import('../../src/core/lbug/schema.js'); + + // Seed one embedding row for the fixture Function through the CORE + // adapter (writable), then build the HNSW index — skip visibly when + // VECTOR is unavailable in this environment, matching the + // lbug-vector-extension suite convention. + const embedding = new Array(EMBEDDING_DIMS).fill(0); + embedding[0] = 1; + await batchInsertEmbeddings(core.executeWithReusedStatement, [ + { + nodeId: 'func:vec', + chunkIndex: 0, + startLine: 1, + endLine: 3, + embedding, + contentHash: 'vec-hash', + }, + ]); + const indexBuilt = await core.createVectorIndex(); + if (!indexBuilt) { + console.warn('[lbug-pool-vector-lane] Skipping — VECTOR unavailable.'); + ctx.skip(); + return; + } + + // Close the writable core adapter so the pool opens its OWN read-only + // Database. This is what makes the case discriminating: extension + // loads are per-Database, so a shared/injected Database would inherit + // the VECTOR load from createVectorIndex above and pass even without + // the pre-warm fix. A fresh Database has nothing loaded — only the + // pool's own pre-warm can make the vector lane legal. + await core.closeLbug(); + + // The regression: through the POOL, the vector lane must work without + // any caller loading the extension. Pre-fix this rejects with + // "Catalog exception: function QUERY_VECTOR_INDEX is not defined". + await initLbug('vec-repo', handle.dbPath); + const vec = `CAST([${embedding.join(',')}] AS FLOAT[${EMBEDDING_DIMS}])`; + const rows = (await executeQuery( + 'vec-repo', + `CALL QUERY_VECTOR_INDEX('${EMBEDDING_TABLE_NAME}', '${EMBEDDING_INDEX_NAME}', ${vec}, 1) + YIELD node AS emb, distance + RETURN emb.nodeId AS nodeId, distance`, + )) as Array<{ nodeId: string; distance: number }>; + + expect(rows.length).toBe(1); + expect(String(rows[0].nodeId)).toBe('func:vec'); + expect(Number(rows[0].distance)).toBeLessThan(1e-6); + }, 120_000); + }); + }, + { + seed: [ + `CREATE (fn:Function {id: 'func:vec', name: 'vec', filePath: 'src/vec.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + ], + }, +); diff --git a/gitnexus/test/integration/lbug-vector-extension.test.ts b/gitnexus/test/integration/lbug-vector-extension.test.ts index ba436f183..72097089e 100644 --- a/gitnexus/test/integration/lbug-vector-extension.test.ts +++ b/gitnexus/test/integration/lbug-vector-extension.test.ts @@ -88,9 +88,11 @@ withTestLbugDB('vector-extension', (handle) => { * 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. + // VECTOR is environment-sensitive (skipped when the extension cannot be + // loaded or installed — offline machines without a pre-installed file). + // Probe once, skip the suite if unavailable — mirrors the FTS-skip + // convention in withTestLbugDB. No platform is categorically excluded: + // win_amd64 artifacts ship for every 0.18.x extension version (#2623). let vectorAvailable = false; let skipWarned = false; beforeAll(async () => { diff --git a/gitnexus/test/unit/calltool-dispatch-id-bridge.test.ts b/gitnexus/test/unit/calltool-dispatch-id-bridge.test.ts index ca009c542..0390caab6 100644 --- a/gitnexus/test/unit/calltool-dispatch-id-bridge.test.ts +++ b/gitnexus/test/unit/calltool-dispatch-id-bridge.test.ts @@ -18,7 +18,7 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -const { lbugMocks, platformMocks } = vi.hoisted(() => ({ +const { lbugMocks } = vi.hoisted(() => ({ lbugMocks: { initLbug: vi.fn().mockResolvedValue(undefined), executeQuery: vi.fn().mockResolvedValue([]), @@ -26,9 +26,6 @@ const { lbugMocks, platformMocks } = vi.hoisted(() => ({ closeLbug: vi.fn().mockResolvedValue(undefined), isLbugReady: vi.fn().mockReturnValue(true), }, - platformMocks: { - isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), - }, })); vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { @@ -66,14 +63,6 @@ vi.mock('../../src/storage/git.js', async (importOriginal) => { }; }); -vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - isVectorExtensionSupportedByPlatform: platformMocks.isVectorExtensionSupportedByPlatform, - }; -}); - vi.mock('../../src/core/search/bm25-index.js', () => ({ searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }), })); @@ -145,7 +134,6 @@ describe('LocalBackend PDG impact — resolved-callee-id bridge (U6)', () => { beforeEach(async () => { vi.clearAllMocks(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); // READY PDG layer so the dispatch reaches the mode-dispatch / bridge surface. vi.mocked(loadMeta).mockResolvedValue({ pdg: { maxCdgEdgesPerFunction: 0, maxReachingDefEdgesPerFunction: 0 }, diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index eb0b94e26..18206ff80 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -17,7 +17,7 @@ import path from 'path'; // local-backend.ts imports from core/lbug/pool-adapter.js; the mcp/core/lbug-adapter.js // re-exports from the same module, so we mock the canonical source. // vi.hoisted runs before vi.mock hoisting, making the fns available to both factories. -const { lbugMocks, platformMocks } = vi.hoisted(() => ({ +const { lbugMocks } = vi.hoisted(() => ({ lbugMocks: { initLbug: vi.fn().mockResolvedValue(undefined), executeQuery: vi.fn().mockResolvedValue([]), @@ -25,9 +25,6 @@ const { lbugMocks, platformMocks } = vi.hoisted(() => ({ closeLbug: vi.fn().mockResolvedValue(undefined), isLbugReady: vi.fn().mockReturnValue(true), }, - platformMocks: { - isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), - }, })); vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { @@ -76,14 +73,6 @@ vi.mock('../../src/storage/git.js', async (importOriginal) => { }; }); -vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - isVectorExtensionSupportedByPlatform: platformMocks.isVectorExtensionSupportedByPlatform, - }; -}); - // Also mock the search modules to avoid loading onnxruntime vi.mock('../../src/core/search/bm25-index.js', () => ({ searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }), @@ -300,7 +289,6 @@ describe('LocalBackend.callTool', () => { beforeEach(async () => { vi.clearAllMocks(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); backend = new LocalBackend(); setupSingleRepo(); await backend.init(); @@ -549,11 +537,18 @@ describe('LocalBackend.callTool', () => { } }); - it('skips vector index query when VECTOR is unsupported by the platform', async () => { + it('falls back to the exact scan with a once-per-backend warning when the vector index query fails', async () => { + // The platform gate is gone (#2623 follow-up): the vector lane is always + // ATTEMPTED, and a runtime failure (extension unloadable, index absent) is + // what routes semantic search onto the exact scan. const cap = _captureLogger(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(false); (executeQuery as any).mockImplementation(async (_repoId: string, cypher: string) => { if (cypher.includes('COUNT(*) AS cnt')) return [{ cnt: 1 }]; + if (cypher.includes('QUERY_VECTOR_INDEX')) { + throw new Error( + 'Binder exception: Trying to read from an index on table CodeEmbedding but its extension is not loaded.', + ); + } if (cypher.includes('MATCH (e:CodeEmbedding)')) return []; return []; }); @@ -565,7 +560,9 @@ describe('LocalBackend.callTool', () => { const queries = (executeQuery as any).mock.calls.map( ([, cypher]: [string, string]) => cypher, ); - expect(queries.some((cypher: string) => cypher.includes('QUERY_VECTOR_INDEX'))).toBe(false); + // The vector lane was attempted… + expect(queries.some((cypher: string) => cypher.includes('QUERY_VECTOR_INDEX'))).toBe(true); + // …and its failure routed the query onto the exact scan. expect( queries.some( (cypher: string) => @@ -578,7 +575,7 @@ describe('LocalBackend.callTool', () => { .records() .some((r) => String(r.msg ?? '').includes( - 'GitNexus [query:vector]: VECTOR extension not supported on this platform', + 'GitNexus [query:vector]: vector index query failed; using exact scan fallback', ), ), ).toBe(true); @@ -588,7 +585,6 @@ describe('LocalBackend.callTool', () => { }); it('issues vector index query when VECTOR is supported by the platform', async () => { - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); (executeQuery as any).mockImplementation(async (_repoId: string, cypher: string) => { if (cypher.includes('COUNT(*) AS cnt')) return [{ cnt: 1 }]; return []; @@ -605,7 +601,6 @@ describe('LocalBackend.callTool', () => { }); it('threads GITNEXUS_VECTOR_MAX_DISTANCE into the vector index WHERE clause', async () => { - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); vi.mocked(executeQuery).mockImplementation(async (_repoId: string, cypher: string) => { if (cypher.includes('COUNT(*) AS cnt')) return [{ cnt: 1 }]; return []; @@ -1924,7 +1919,6 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { beforeEach(async () => { vi.clearAllMocks(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); // U2: stamp a READY PDG layer (both caps) so the layer-presence probe in // `_impactImpl` falls THROUGH to the mode-dispatch surface these tests pin // (the `_runImpactPDG` delegate / the ambiguous fan-out under `mode:'pdg'`). @@ -3338,7 +3332,6 @@ describe('LocalBackend.listReposPage / callTool list_repos pagination (#2119)', beforeEach(async () => { vi.clearAllMocks(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); backend = new LocalBackend(); }); diff --git a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts index 25db417c5..6827376f1 100644 --- a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts +++ b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts @@ -92,8 +92,9 @@ describe('partitionManifestWindows (issue #2189 windowed resolution)', () => { // ── Surface 2: real-pool residency bound through syncGroup ─────────────────── -const { loadFTSExtensionMock, openCounter } = vi.hoisted(() => ({ +const { loadFTSExtensionMock, loadVectorExtensionMock, openCounter } = vi.hoisted(() => ({ loadFTSExtensionMock: vi.fn(), + loadVectorExtensionMock: vi.fn().mockResolvedValue(false), openCounter: { live: 0, peak: 0 }, })); @@ -122,6 +123,7 @@ vi.mock('@ladybugdb/core', () => ({ vi.mock('../../../src/core/lbug/lbug-adapter.js', () => ({ isReadOnlyDbError: vi.fn(() => false), loadFTSExtension: loadFTSExtensionMock, + loadVectorExtension: loadVectorExtensionMock, })); vi.mock('../../../src/core/lbug/lbug-config.js', () => ({ diff --git a/gitnexus/test/unit/incremental-orchestration.test.ts b/gitnexus/test/unit/incremental-orchestration.test.ts index b74efcb47..fda37c3b5 100644 --- a/gitnexus/test/unit/incremental-orchestration.test.ts +++ b/gitnexus/test/unit/incremental-orchestration.test.ts @@ -931,8 +931,9 @@ describe('runFullAnalysis — incremental orchestration', () => { * and boot a real embedder in CI. This run stays preserve-only (no force). * * Skip-gated on VECTOR availability (the lbug-vector-extension.test.ts - * pattern): hard-false on win32; statically linked on linux-x64, so the - * assertions genuinely run in CI — and on win32 the honest stamp is + * pattern): skipped only where the extension genuinely cannot load — + * no platform is categorically excluded any more (#2623 follow-up) — and + * where it cannot, the honest stamp is * 'exact-scan', which the unit-level wiring pin in * run-analyze-fts-repair.test.ts covers platform-independently. */ diff --git a/gitnexus/test/unit/incremental-vector-extension-ordering.test.ts b/gitnexus/test/unit/incremental-vector-extension-ordering.test.ts new file mode 100644 index 000000000..749a89697 --- /dev/null +++ b/gitnexus/test/unit/incremental-vector-extension-ordering.test.ts @@ -0,0 +1,267 @@ +/** + * #2623: the incremental writeback must have the VECTOR extension loaded + * BEFORE `deleteNodesForFiles` runs — its very first statement is the + * `CodeEmbedding` join-delete, and LadybugDB refuses every mutation of a + * table carrying an HNSW index while the extension is unloaded: + * + * Binder exception: Trying to delete from an index on table CodeEmbedding + * but its extension is not loaded. + * + * Nothing on that path loaded VECTOR until Phase 4, so any repo that had + * built `code_embedding_idx` crashed on the next content change — on machines + * where VECTOR loads perfectly well. This is the sibling of the #2589 FTS + * drop-before-delete ordering test and deliberately mirrors its shape: drive + * the real `runFullAnalysis` incremental path against a real git repo and a + * real LadybugDB, and assert the index state at the exact moment + * `deleteNodesForFiles` is invoked. + */ +import { readFile, writeFile } from 'fs/promises'; +import { execSync } from 'child_process'; +import path from 'path'; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { setupMiniRepo } from '../helpers/mini-repo.js'; +import { seedEmbeddingsForFiles, stampEmbeddingCount } from '../helpers/embedding-seed.js'; +import { getStoragePaths } from '../../src/storage/repo-manager.js'; +import { createTempDir } from '../helpers/test-db.js'; +import { EMBEDDING_TABLE_NAME } from '../../src/core/lbug/schema.js'; +import { resolveAnalyzeInstallPolicy } from '../../src/core/lbug/extension-loader.js'; + +const vectorMustBeAvailable = process.env.GITNEXUS_REQUIRE_VECTOR === '1'; + +const commitAll = (cwd: string, message: string): void => { + execSync('git -c user.name=test -c user.email=t@t -c commit.gpgsign=false add -A', { + cwd, + stdio: 'pipe', + }); + execSync( + `git -c user.name=test -c user.email=t@t -c commit.gpgsign=false commit -q -m "${message}"`, + { cwd, stdio: 'pipe' }, + ); +}; + +describe('runFullAnalysis incremental writeback — VECTOR loaded before embedding-row DML (#2623)', () => { + let vectorAvailable = true; + let skipWarned = false; + + beforeAll(async () => { + const lbugAdapter = await import('../../src/core/lbug/lbug-adapter.js'); + // Cheap standalone probe, matching the #2589 suite's convention: settle + // availability once, up front, not inside the expensive test body. + const probe = await createTempDir('gitnexus-2623-vector-probe-'); + try { + await lbugAdapter.initLbug(probe.dbPath); + vectorAvailable = await lbugAdapter.loadVectorExtension(undefined, { + policy: resolveAnalyzeInstallPolicy(), + }); + } finally { + await lbugAdapter.closeLbug(); + await probe.cleanup(); + } + }, 120_000); + + // Skip VISIBLY: a silent `return` would report a false pass and hide an + // ordering regression in exactly the environments least likely to notice. + beforeEach((ctx) => { + if (!vectorAvailable) { + if (vectorMustBeAvailable) { + throw new Error( + 'GITNEXUS_REQUIRE_VECTOR=1 but the VECTOR extension is unavailable — cannot verify the #2623 ordering fix.', + ); + } + if (!skipWarned) { + skipWarned = true; + console.warn( + '[incremental-vector-extension-ordering] Skipping — the LadybugDB VECTOR extension is unavailable.', + ); + } + ctx.skip(); + } + }); + + afterEach(() => { + vi.doUnmock('../../src/core/lbug/lbug-adapter.js'); + vi.resetModules(); + }); + + it('completes the surgical incremental run with the HNSW index present, and VECTOR is loaded by the time deleteNodesForFiles runs', async () => { + const lbugAdapter = await import('../../src/core/lbug/lbug-adapter.js'); + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + const repo = await setupMiniRepo('gitnexus-2623-vector-order-'); + try { + // First run: full rebuild, real graph. + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + + // Seed real embedding rows for two files, then build the HNSW index — + // the state a prior `analyze --embeddings` leaves behind. Zero vectors + // need no extension for the TABLE; only the index is extension-gated. + // POSIX literals, NOT path.join: the graph stores repo-relative + // filePaths with forward slashes on every OS, and a Windows backslash + // inside the seed helper's single-quoted Cypher literal is a parser + // error ("Invalid input <... n.filePath = '>"). path.join stays only + // for real filesystem access below. + const changedFile = 'src/handler.ts'; + const untouchedFile = 'src/validator.ts'; + const seeded = await seedEmbeddingsForFiles(repo.dbPath, [changedFile, untouchedFile], 2); + const changedIds = seeded.get(changedFile) ?? []; + const untouchedIds = seeded.get(untouchedFile) ?? []; + expect(changedIds.length).toBeGreaterThan(0); + expect(untouchedIds.length).toBeGreaterThan(0); + + const { lbugPath, storagePath } = getStoragePaths(repo.dbPath); + // Without this, deriveEmbeddingMode sees a repo with no embeddings, the + // Phase 3.5 restore never engages, and rows deleted by the importer-BFS + // write-set expansion simply never come back — which would make the + // preservation assertion below measure the wrong thing. + await stampEmbeddingCount(storagePath, changedIds.length + untouchedIds.length); + + const readEmbeddingIndexRows = async (): Promise>> => { + const rows = (await lbugAdapter.executeQuery('CALL SHOW_INDEXES() RETURN *')) as Array< + Record + >; + return rows.filter((r) => r.table_name === EMBEDDING_TABLE_NAME && r.index_type !== 'HASH'); + }; + + await lbugAdapter.initLbug(lbugPath); + const indexBuilt = await lbugAdapter.createVectorIndex(); + const indexRowsBefore = await readEmbeddingIndexRows(); + await lbugAdapter.closeLbug(); + + // The beforeEach gate already proved VECTOR loads here, so a failure to + // build the index is a real bug, not an environment gap. + expect(indexBuilt).toBe(true); + expect(indexRowsBefore.length).toBeGreaterThan(0); + + // Record the index's extension state at the exact moment the embedding + // join-delete is about to run. Pre-fix this is `false` and the run then + // throws; post-fix the gate has already loaded VECTOR. + let embeddingIndexAtDeleteTime: Array> | undefined; + const originalDeleteNodesForFiles = lbugAdapter.deleteNodesForFiles; + vi.spyOn(lbugAdapter, 'deleteNodesForFiles').mockImplementation(async (filePaths, opts) => { + embeddingIndexAtDeleteTime = await readEmbeddingIndexRows(); + return originalDeleteNodesForFiles(filePaths, opts); + }); + + // One-file change keeps this well under the 50-file escalation + // threshold on a 7-file repo, so it takes the surgical branch. + const handlerPath = path.join(repo.dbPath, changedFile); + await writeFile( + handlerPath, + (await readFile(handlerPath, 'utf-8')) + '\n// #2623 ordering-test touch\n', + 'utf-8', + ); + commitAll(repo.dbPath, '#2623 ordering touch'); + + // THE regression: before the fix this rejects with + // "Trying to delete from an index on table CodeEmbedding". + await expect( + runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }), + ).resolves.toBeDefined(); + + // Ordering proof: the index was still there AND its extension was + // loaded when the delete ran — the fix loads VECTOR rather than + // dropping the index (run-analyze relies on HNSW self-maintaining + // across a surgical run). + expect(embeddingIndexAtDeleteTime).toBeDefined(); + expect(embeddingIndexAtDeleteTime!.length).toBeGreaterThan(0); + for (const row of embeddingIndexAtDeleteTime!) { + expect(row.extension_loaded).toBe(true); + } + + // Data outcome: the untouched file's rows survive, and nothing is + // duplicated. (The changed file's rows are removed by the join-delete + // and restored by Phase 3.5, so their count must stay at exactly one + // per nodeId — a PK duplicate would mean the delete silently no-op'd.) + await lbugAdapter.initLbug(lbugPath); + try { + const perNode = (await lbugAdapter.executeQuery( + `MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN e.nodeId AS nodeId, count(e) AS c`, + )) as Array<{ nodeId: string; c: number | bigint }>; + const counts = new Map(perNode.map((r) => [String(r.nodeId), Number(r.c)])); + for (const id of untouchedIds) { + expect(counts.get(id)).toBe(1); + } + for (const [, c] of counts) { + expect(c).toBe(1); + } + // The index is still there — the surgical path keeps it. + expect((await readEmbeddingIndexRows()).length).toBeGreaterThan(0); + } finally { + await lbugAdapter.closeLbug(); + } + } finally { + await repo.cleanup(); + } + }, 300_000); + + it('escalates to a full DB write instead of crashing when VECTOR cannot be loaded', async () => { + const lbugAdapter = await import('../../src/core/lbug/lbug-adapter.js'); + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + const repo = await setupMiniRepo('gitnexus-2623-vector-blocked-'); + const previousPolicy = process.env.GITNEXUS_LBUG_EXTENSION_INSTALL; + try { + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + // POSIX literal for the graph-side path (see the note in the first case). + const seeded = await seedEmbeddingsForFiles(repo.dbPath, ['src/handler.ts'], 2); + const seededIds = seeded.get('src/handler.ts') ?? []; + expect(seededIds.length).toBeGreaterThan(0); + // Deliberately NOT stampEmbeddingCount: this pins the case where the DB + // holds embedding rows that meta does not account for. The escalation + // wipes the DB, so without an explicit rescue read those rows would be + // destroyed silently — the run would still "succeed" and the loss would + // be invisible. + + const { lbugPath } = getStoragePaths(repo.dbPath); + await lbugAdapter.initLbug(lbugPath); + const indexBuilt = await lbugAdapter.createVectorIndex(); + await lbugAdapter.closeLbug(); + expect(indexBuilt).toBe(true); + + const handlerPath = path.join(repo.dbPath, 'src', 'handler.ts'); + await writeFile( + handlerPath, + (await readFile(handlerPath, 'utf-8')) + '\n// #2623 blocked-path touch\n', + 'utf-8', + ); + commitAll(repo.dbPath, '#2623 blocked touch'); + + // VECTOR becomes unloadable for this run. The table is now immutable + // (the index cannot be dropped without the extension either), so the + // run must abandon surgery rather than fail mid-writeback. + process.env.GITNEXUS_LBUG_EXTENSION_INSTALL = 'never'; + const logs: string[] = []; + await expect( + runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {}, onLog: (m: string) => logs.push(m) }, + ), + ).resolves.toBeDefined(); + + expect(logs.some((m) => m.includes('full DB write'))).toBe(true); + expect(logs.some((m) => m.includes('VECTOR'))).toBe(true); + + // The forced rebuild must NOT eat the embeddings it never asked to touch. + await lbugAdapter.initLbug(lbugPath); + try { + const surviving = (await lbugAdapter.executeQuery( + `MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN e.nodeId AS nodeId`, + )) as Array<{ nodeId: string }>; + const survivingIds = new Set(surviving.map((r) => String(r.nodeId))); + for (const id of seededIds) { + expect(survivingIds.has(id)).toBe(true); + } + // …and exactly once each — the restore must not double-insert. + expect(surviving.length).toBe(survivingIds.size); + } finally { + await lbugAdapter.closeLbug(); + } + expect(logs.some((m) => m.includes('Preserving'))).toBe(true); + } finally { + if (previousPolicy === undefined) delete process.env.GITNEXUS_LBUG_EXTENSION_INSTALL; + else process.env.GITNEXUS_LBUG_EXTENSION_INSTALL = previousPolicy; + await repo.cleanup(); + } + }, 300_000); +}); diff --git a/gitnexus/test/unit/lbug-pool-fts-load.test.ts b/gitnexus/test/unit/lbug-pool-fts-load.test.ts index d62735c27..7f5b6ff87 100644 --- a/gitnexus/test/unit/lbug-pool-fts-load.test.ts +++ b/gitnexus/test/unit/lbug-pool-fts-load.test.ts @@ -1,7 +1,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; -const { loadFTSExtensionMock } = vi.hoisted(() => ({ +const { loadFTSExtensionMock, loadVectorExtensionMock } = vi.hoisted(() => ({ loadFTSExtensionMock: vi.fn(), + loadVectorExtensionMock: vi.fn(), })); vi.mock('@ladybugdb/core', () => ({ @@ -16,6 +17,7 @@ vi.mock('@ladybugdb/core', () => ({ vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ isReadOnlyDbError: vi.fn(() => false), loadFTSExtension: loadFTSExtensionMock, + loadVectorExtension: loadVectorExtensionMock, })); vi.mock('../../src/core/lbug/lbug-config.js', () => ({ @@ -31,10 +33,13 @@ describe('read-pool FTS loading', () => { afterEach(async () => { await closeLbug().catch(() => {}); loadFTSExtensionMock.mockReset(); + loadVectorExtensionMock.mockReset(); + loadVectorExtensionMock.mockResolvedValue(false); }); it('loads FTS with load-only policy and caches a successful load', async () => { loadFTSExtensionMock.mockResolvedValue(true); + loadVectorExtensionMock.mockResolvedValue(true); const db = {} as any; await initLbugWithDb('repo-a', db, '/tmp/shared-fts-db'); @@ -46,6 +51,7 @@ describe('read-pool FTS loading', () => { it('does not fake a successful load when FTS is unavailable', async () => { loadFTSExtensionMock.mockResolvedValue(false); + loadVectorExtensionMock.mockResolvedValue(false); const db = {} as any; await initLbugWithDb('repo-a', db, '/tmp/shared-fts-db'); @@ -59,4 +65,29 @@ describe('read-pool FTS loading', () => { policy: 'load-only', }); }); + + it('loads VECTOR with load-only policy and caches a successful load (#2623 follow-up)', async () => { + loadFTSExtensionMock.mockResolvedValue(true); + loadVectorExtensionMock.mockResolvedValue(true); + const db = {} as any; + + await initLbugWithDb('repo-a', db, '/tmp/shared-vec-db'); + await initLbugWithDb('repo-b', db, '/tmp/shared-vec-db'); + + expect(loadVectorExtensionMock).toHaveBeenCalledTimes(1); + expect(loadVectorExtensionMock).toHaveBeenCalledWith(expect.anything(), { + policy: 'load-only', + }); + }); + + it('retries the VECTOR load on the next open when it was unavailable', async () => { + loadFTSExtensionMock.mockResolvedValue(true); + loadVectorExtensionMock.mockResolvedValue(false); + const db = {} as any; + + await initLbugWithDb('repo-a', db, '/tmp/shared-vec-db'); + await initLbugWithDb('repo-b', db, '/tmp/shared-vec-db'); + + expect(loadVectorExtensionMock).toHaveBeenCalledTimes(2); + }); }); diff --git a/gitnexus/test/unit/lbug-pool-pinning.test.ts b/gitnexus/test/unit/lbug-pool-pinning.test.ts index 4ce154023..125c803e0 100644 --- a/gitnexus/test/unit/lbug-pool-pinning.test.ts +++ b/gitnexus/test/unit/lbug-pool-pinning.test.ts @@ -14,8 +14,9 @@ import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; // repo resident through deferred manifest/workspace resolution. Pinning makes // that resident set survive automatic (LRU + idle) eviction. -const { loadFTSExtensionMock } = vi.hoisted(() => ({ +const { loadFTSExtensionMock, loadVectorExtensionMock } = vi.hoisted(() => ({ loadFTSExtensionMock: vi.fn(), + loadVectorExtensionMock: vi.fn().mockResolvedValue(false), })); vi.mock('@ladybugdb/core', () => ({ @@ -36,6 +37,7 @@ vi.mock('@ladybugdb/core', () => ({ vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ isReadOnlyDbError: vi.fn(() => false), loadFTSExtension: loadFTSExtensionMock, + loadVectorExtension: loadVectorExtensionMock, })); vi.mock('../../src/core/lbug/lbug-config.js', () => ({ diff --git a/gitnexus/test/unit/mcp-wal-feedback.test.ts b/gitnexus/test/unit/mcp-wal-feedback.test.ts index 710d81369..65515a987 100644 --- a/gitnexus/test/unit/mcp-wal-feedback.test.ts +++ b/gitnexus/test/unit/mcp-wal-feedback.test.ts @@ -3,7 +3,7 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { lbugMocks, platformMocks, repoMocks } = vi.hoisted(() => ({ +const { lbugMocks, repoMocks } = vi.hoisted(() => ({ lbugMocks: { initLbug: vi.fn().mockResolvedValue(undefined), executeQuery: vi.fn(), @@ -11,9 +11,6 @@ const { lbugMocks, platformMocks, repoMocks } = vi.hoisted(() => ({ closeLbug: vi.fn().mockResolvedValue(undefined), isLbugReady: vi.fn().mockReturnValue(true), }, - platformMocks: { - isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), - }, repoMocks: { listRegisteredRepos: vi.fn(), }, @@ -40,14 +37,6 @@ vi.mock('../../src/core/git-staleness.js', () => ({ checkCwdMatch: vi.fn().mockResolvedValue({ match: 'none' }), })); -vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - isVectorExtensionSupportedByPlatform: platformMocks.isVectorExtensionSupportedByPlatform, - }; -}); - vi.mock('../../src/core/search/bm25-index.js', () => ({ searchFTSFromLbug: vi.fn().mockResolvedValue([]), })); diff --git a/gitnexus/test/unit/native-check-probe.test.ts b/gitnexus/test/unit/native-check-probe.test.ts index 3593977ab..8d18663aa 100644 --- a/gitnexus/test/unit/native-check-probe.test.ts +++ b/gitnexus/test/unit/native-check-probe.test.ts @@ -29,7 +29,10 @@ vi.mock('@ladybugdb/core', () => { return { default: { Database, Connection } }; }); -import { probeFtsExtensionLoad } from '../../src/core/lbug/native-check.js'; +import { + probeFtsExtensionLoad, + probeVectorExtensionLoad, +} from '../../src/core/lbug/native-check.js'; const closeable = () => ({ close: vi.fn() }); @@ -90,3 +93,29 @@ describe('probeFtsExtensionLoad (#2374)', () => { await expect(probeFtsExtensionLoad()).resolves.toEqual({ loaded: true }); }); }); + +describe('probeVectorExtensionLoad (#2623 follow-up)', () => { + it('issues LOAD EXTENSION vector and reports loaded on success — no platform short-circuit', async () => { + h.query.mockResolvedValue(closeable()); + await expect(probeVectorExtensionLoad()).resolves.toEqual({ loaded: true }); + // The probe must really attempt the LOAD (the old code refused Windows + // before ever touching the engine; the artifact ships for win_amd64 too). + expect(h.query).toHaveBeenCalledWith('LOAD EXTENSION vector'); + }); + + it('reports the collapsed reason when LOAD fails', async () => { + h.query.mockRejectedValue(new Error('IO exception:\n extension file not found')); + await expect(probeVectorExtensionLoad()).resolves.toMatchObject({ + loaded: false, + reason: 'IO exception: extension file not found', + }); + }); + + it('times out instead of hanging when the native call never settles', async () => { + h.query.mockReturnValue(new Promise(() => undefined)); + await expect(probeVectorExtensionLoad(20)).resolves.toMatchObject({ + loaded: false, + reason: expect.stringContaining('timed out'), + }); + }); +}); diff --git a/gitnexus/test/unit/platform-capabilities.test.ts b/gitnexus/test/unit/platform-capabilities.test.ts index 3b58d9d16..7497eb1a7 100644 --- a/gitnexus/test/unit/platform-capabilities.test.ts +++ b/gitnexus/test/unit/platform-capabilities.test.ts @@ -1,17 +1,19 @@ import { describe, expect, it } from 'vitest'; import { + getRuntimeCapabilities, getRuntimeFingerprint, - isVectorExtensionSupportedByPlatform, } from '../../src/core/platform/capabilities.js'; describe('platform capabilities', () => { - it('keeps Ladybug VECTOR disabled by default on Windows', () => { - expect(isVectorExtensionSupportedByPlatform('win32')).toBe(false); - }); - - it('allows VECTOR probing on Linux and macOS', () => { - expect(isVectorExtensionSupportedByPlatform('linux')).toBe(true); - expect(isVectorExtensionSupportedByPlatform('darwin')).toBe(true); + it('reports VECTOR as platform-available everywhere, Windows included (#2623 follow-up)', () => { + // LadybugDB ships win_amd64 VECTOR artifacts for every 0.18.x extension + // version, so no platform is categorically excluded any more. Whether the + // extension actually LOADS on a machine is a runtime question answered by + // probeVectorExtensionLoad (doctor) and loadVectorExtension (analyze). + const caps = getRuntimeCapabilities(); + expect(caps.vector).toBe('available'); + expect(caps.semanticMode).toBe('vector-index'); + expect(caps.reason).toBeUndefined(); }); it('resolves the LadybugDB version even though @ladybugdb/core exports omit ./package.json (#2374)', () => { diff --git a/gitnexus/test/unit/pool-wal-recovery.test.ts b/gitnexus/test/unit/pool-wal-recovery.test.ts index 77ccea84d..ca7517878 100644 --- a/gitnexus/test/unit/pool-wal-recovery.test.ts +++ b/gitnexus/test/unit/pool-wal-recovery.test.ts @@ -31,6 +31,7 @@ vi.mock('@ladybugdb/core', () => ({ vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ loadFTSExtension: vi.fn().mockResolvedValue(true), + loadVectorExtension: vi.fn().mockResolvedValue(true), })); vi.mock('../../src/core/lbug/lbug-config.js', () => ({ diff --git a/gitnexus/test/unit/trace-bfs.test.ts b/gitnexus/test/unit/trace-bfs.test.ts index 33ab72bff..1f3a97baf 100644 --- a/gitnexus/test/unit/trace-bfs.test.ts +++ b/gitnexus/test/unit/trace-bfs.test.ts @@ -6,7 +6,7 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -const { lbugMocks, platformMocks } = vi.hoisted(() => ({ +const { lbugMocks } = vi.hoisted(() => ({ lbugMocks: { initLbug: vi.fn().mockResolvedValue(undefined), executeQuery: vi.fn().mockResolvedValue([]), @@ -14,9 +14,6 @@ const { lbugMocks, platformMocks } = vi.hoisted(() => ({ closeLbug: vi.fn().mockResolvedValue(undefined), isLbugReady: vi.fn().mockReturnValue(true), }, - platformMocks: { - isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), - }, })); vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { @@ -59,11 +56,6 @@ vi.mock('../../src/storage/git.js', async (importOriginal) => { return { ...actual, getGitRoot: vi.fn().mockReturnValue(null) }; }); -vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, ...platformMocks }; -}); - vi.mock('../../src/core/search/bm25-index.js', () => ({ searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }), }));