mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
* feat(lbug): add ensureEmbeddingRowDmlSafe VECTOR gate for embedding-row DML LadybugDB refuses every mutation of a table carrying an HNSW index while the VECTOR extension is not loaded on that connection: DELETE and CREATE raise a Binder exception, DROP TABLE is refused while the index references it, and SET segfaults the process. Dropping the index is not an available recovery either — CALL DROP_VECTOR_INDEX is itself a VECTOR-extension function and is undefined in exactly that state. Add a single primitive that loads VECTOR under the analyze install policy and, only when that fails, reads CALL SHOW_INDEXES (which works without the extension) to decide whether an index actually exists to trip over. No call sites yet. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the #2623 VECTOR gate for embedding-row DML Three cases: no index + VECTOR unavailable stays safe (no needless escalation); index present + VECTOR unavailable is reported blocked AND the raw deleteNodesForFiles genuinely throws 'extension is not loaded' (proving the hazard is real, not theoretical); index present + VECTOR loadable is safe, the delete works, and the HNSW index survives — the invariant run-analyze relies on when it keeps the index across a surgical incremental run. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): load VECTOR before the incremental writeback touches embedding rows Incremental analyze died on every content change once a repo had built code_embedding_idx: Analysis failed: Binder exception: Trying to delete from an index on table CodeEmbedding but its extension is not loaded. The surgical writeback's first statement is deleteNodesForFiles' CodeEmbedding join-delete, but nothing on that path loaded VECTOR until Phase 4 — so the engine refused the delete. This is an ordering defect, not an environment one: it reproduces on machines where VECTOR loads fine. The dirty-flag recovery then forced a full rebuild on the next run, which is why it read as 'just slow'. Call ensureEmbeddingRowDmlSafe() once, before the escalation gate and before any row is touched — the same 'index lifecycle before row DML' seam dropSearchFTSIndexes occupies for FTS (#2589). Unconditional, because a DB carrying the index from an earlier --embeddings run hits the same wall on a plain incremental run. When VECTOR truly cannot load the table is immutable (the index cannot be dropped without the extension either), so the run falls through to the existing wipe-and-COPY escalation with a message naming cause, consequence and remedy. Fixes #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): pin the #2623 VECTOR-before-embedding-DML ordering end-to-end Sibling of the #2589 FTS drop-before-delete suite, same shape: drive the real runFullAnalysis incremental path over a real git repo and a real LadybugDB, seed real embedding rows, build the HNSW index, then assert the index state at the exact moment deleteNodesForFiles is invoked. Both cases were confirmed to discriminate — with the run-analyze change reverted they fail with the reported 'Trying to delete from an index on table CodeEmbedding but its extension is not loaded', and pass with it: - surgical path: the run completes, the index is still present AND extension_loaded at delete time, exactly one row per nodeId survives, and the untouched file's rows are preserved - blocked path: with GITNEXUS_LBUG_EXTENSION_INSTALL=never the run escalates to a full DB write and says so, instead of crashing Also applies prettier's reindent to the run-analyze log ternary. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(lbug): cite the pinned LadybugDB version in the #2623 probe note The probe matrix behind ensureEmbeddingRowDmlSafe was first recorded on 0.18.0, but gitnexus/package-lock.json pins 0.18.2 (#2587). Re-ran every case on 0.18.2: refused DELETE, refused CREATE, SIGSEGV on SET, DROP_VECTOR_INDEX undefined, DROP TABLE refused, SHOW_INDEXES readable with extension_loaded intact. Identical on both, so the design is unchanged — only the citation was wrong. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): preserve embeddings across the VECTOR-blocked rebuild, and check the catalog before loading Three follow-ups from reviewing the fix itself. 1. Data loss on the blocked path. Escalating wipes the DB files, and Phase 3.5 restores embedding rows from cachedEmbeddings — which deriveEmbeddingMode only populates when meta.stats.embeddings > 0. A DB holding embedding rows that its meta does not account for therefore had every vector destroyed silently by a rebuild it never asked for. Probe on a 3-file repo: 3 rows before, 0 after, no warning. Read the rows before escalating (a plain MATCH, no extension needed) so the existing restore has something to restore, and say so in the log. The blocked-path test now asserts the seeded rows survive exactly once, and that assertion fails without this rescue. 2. Catalog before extension. ensureEmbeddingRowDmlSafe loaded VECTOR first and only read SHOW_INDEXES on failure, so every incremental analyze on a machine without VECTOR paid a bounded out-of-process INSTALL attempt plus an 'extension unavailable' warning — including repos that never built an embedding index and can never hit this bug. One local catalog read settles that case first; the load is attempted only when an index actually gates DML, or when the catalog cannot be read. 3. Dead branch. targetConn is always the module singleton there, so the isSharedSingletonConn ternary could never take its second arm. Collapsed to withConnLock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(doctor): live-probe the VECTOR extension instead of printing the static platform capability Review finding on #2624 (MEDIUM), and exactly what #2623's reporter hit: doctor printed 'VECTOR index: available' — derived from a static platform check — while every incremental analyze on the same machine was dying on an unloaded VECTOR extension. The FTS line was switched to a live LOAD probe for the identical contradiction under #2374; VECTOR now gets the same treatment. probeVectorExtensionLoad shares the FTS probe's implementation (bounded, offline-safe, never runs the installer) and doctor's semantic-mode line now follows the probe, not the platform: without a loadable extension the vector index can be neither built nor queried, so search really is on exact scan. The load-error classifier's remedies are label-parameterized so the VECTOR row stops dispensing FTS-specific advice — 'run analyze --repair-fts' repairs FTS indexes only and was actively wrong for a missing vector extension. Default label stays 'FTS'; every existing caller and pinned remedy string is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lbug): remove the stale Windows VECTOR gate — the extension ships for win_amd64 The codebase categorically refused VECTOR on Windows (platform !== 'win32' in isVectorExtensionSupportedByPlatform, plus a hard early-return in loadVectorExtension) on the strength of an early-era report that in-process INSTALL VECTOR could SIGSEGV (#1365). That belief is stale, verified directly: - the extension server hosts win_amd64 VECTOR artifacts for every 0.18.x extension version — v0.18.0 and v0.18.1 both serve a real 14 MB PE32+ DLL (curl-probed; 'file' confirms PE32+ x86-64) - the pinned 0.18.2 core resolves its extension directory to 0.18.1 (strace-verified LOAD open()), so the pinned version's Windows artifact exists too - INSTALL now runs in a spawned child (installDuckDbExtensionOutOfProcess), so even a crashing installer kills only the child and degrades to unavailable — the original hazard cannot reach the parent process any more Windows now takes the same runtime path as every other OS: try LOAD, install out-of-process when policy allows, degrade to exact scan when it truly fails. The MCP semantic-search lane loses its static platform gate too — it always attempts the vector index and falls back to the exact scan on runtime failure, with a once-per-backend diagnostic naming the real error instead of a platform-policy message. isVectorExtensionSupportedByPlatform is deleted; getRuntimeCapabilities reports the platform capability as available everywhere and defers machine truth to the live probe. Windows CI is the enforcement: the vector suites skip visibly only when the extension genuinely cannot load, so green Windows lanes now actually exercise VECTOR instead of silently skipping by policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the catalog-read-failure fallback in ensureEmbeddingRowDmlSafe Review finding on #2624 (LOW): the one branch where the gate cannot cheaply prove safety — SHOW_INDEXES itself erroring — was exercised only by inference. Force it with a Connection.prototype.query spy over the real DB: the catalog read fails, and the gate must fall through to actually attempting the extension load (asserted via the recorded statement stream) rather than guessing, returning true here because the extension is loadable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): load VECTOR on the pool's shared Database so the semantic vector lane actually works Review finding on #2624 (MEDIUM): extension load scope is per-Database (probe-verified — LOAD on one connection enables QUERY_VECTOR_INDEX on every connection of the same Database), and the pool pre-warm loaded only FTS. So LocalBackend's vector lane has ALWAYS raised 'Catalog exception: function QUERY_VECTOR_INDEX is not defined' through the pool and silently fallen back to the exact scan — repos above the 10k exact-scan cap got empty semantic results. The serve path was unaffected (the embedding pipeline loads the extension itself). Mirror the FTS line at BOTH load sites — doInitLbug's pre-warm and initLbugWithDb's external-Database adoption — under the same load-only contract (the read pool never triggers a network install), tracked by a new SharedDB.vectorLoaded flag reset where ftsLoaded resets. The new pool test is discriminating and deliberately closes the writable core adapter before the pool opens: a shared/injected Database would inherit the VECTOR load from test seeding and pass either way, so the case forces the pool onto its OWN fresh read-only Database where only the pre-warm can make the lane legal. Verified: fails at the pre-fix tree with the exact Catalog exception, passes with the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run the #2623 ordering suite on Windows/macOS and pre-install VECTOR alongside FTS Two review findings on #2624, both landing in existing seams: - scripts/cross-platform-tests.ts gains incremental-vector-extension-ordering .test.ts: the win32 VECTOR gate is gone in this PR, so the #2623 drop-ordering + blocked-path escalation must be proven on the windows-latest native addon, not just Ubuntu. (The review's claim that lbug-delete-nodes-for-files.test.ts was also missing was wrong — it has been on the roster since #2409.) - scripts/ensure-fts.ts now pre-installs VECTOR under the same best-effort auto-policy contract, so every sharded CI process LOADs from ~/.lbdb instead of racing its own bounded out-of-process INSTALL; the workflow's extension cache already covers it (path is the whole extension dir — key kept for cache continuity). The cross-platform job sets GITNEXUS_REQUIRE_VECTOR=1 beside GITNEXUS_REQUIRE_FTS so a genuinely unavailable VECTOR is a loud failure, never a silent skip. Windows/macOS cannot be executed locally; the PR's CI lanes are the proof for this commit. Linux smoke: ensure-fts.ts reports both extensions ready; all 79 roster entries resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(pool): register loadVectorExtension in the pool unit-suite mocks The pool adapter's new loadVectorExtension import surfaced in four suites that mock lbug-adapter.js with explicit factories (vitest fails loudly on a missing mocked export). Register the export in each — resolving false where the suite's world assumes no vector, true where it mirrors FTS — and extend lbug-pool-fts-load.test.ts, the suite that owns pre-warm extension loading, with the vector pair: successful load cached per shared Database, failed load retried on the next open, both pinned to policy load-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): use POSIX literals for graph paths in the #2623 ordering suite First Windows CI run of this suite (it joined the cross-platform roster this PR) failed with 'Parser exception: Invalid input <MATCH (n:Function) WHERE n.filePath = '>' — path.join produces backslashes on Windows, and a backslash inside the seed helper's single-quoted Cypher literal breaks the parser. The graph stores repo-relative filePaths with forward slashes on every OS, so graph-side paths are POSIX literals now (the incremental-orchestration convention); path.join stays only for real filesystem access. The same Windows lane also proved the substance this suite exists for: lbug-vector-extension passed 7/7 on windows-latest — the extension installed, loaded, and built a real HNSW index there — and the pool vector-lane and DML gate suites passed too. This commit fixes the harness, not the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <abhigyan1.patwari@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
121 lines
4.1 KiB
TypeScript
121 lines
4.1 KiB
TypeScript
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
|
|
|
/**
|
|
* Unit coverage for probeFtsExtensionLoad (#2374, PR #2375): the doctor FTS
|
|
* probe's outcomes without the real native module or network. @ladybugdb/core
|
|
* is mocked so query behavior (resolve / reject / never-settle) is controlled
|
|
* per test, and the timeout is exercised with a tiny injected budget.
|
|
*/
|
|
|
|
const h = vi.hoisted(() => ({
|
|
query: vi.fn<(sql: string) => Promise<unknown>>(),
|
|
connCtor: vi.fn<() => void>(),
|
|
connClose: vi.fn<() => Promise<void>>(async () => undefined),
|
|
dbClose: vi.fn<() => Promise<void>>(async () => undefined),
|
|
}));
|
|
|
|
vi.mock('@ladybugdb/core', () => {
|
|
class Database {
|
|
constructor(_path: string) {}
|
|
close = h.dbClose;
|
|
}
|
|
class Connection {
|
|
constructor(_db: unknown) {
|
|
h.connCtor();
|
|
}
|
|
query = h.query;
|
|
close = h.connClose;
|
|
}
|
|
return { default: { Database, Connection } };
|
|
});
|
|
|
|
import {
|
|
probeFtsExtensionLoad,
|
|
probeVectorExtensionLoad,
|
|
} from '../../src/core/lbug/native-check.js';
|
|
|
|
const closeable = () => ({ close: vi.fn() });
|
|
|
|
beforeEach(() => {
|
|
h.query.mockReset();
|
|
h.connCtor.mockReset();
|
|
h.connClose.mockClear();
|
|
h.dbClose.mockClear();
|
|
});
|
|
|
|
describe('probeFtsExtensionLoad (#2374)', () => {
|
|
it('reports loaded and closes every result when LOAD succeeds (array result)', async () => {
|
|
const results = [closeable(), closeable()];
|
|
h.query.mockResolvedValue(results);
|
|
|
|
await expect(probeFtsExtensionLoad()).resolves.toEqual({ loaded: true });
|
|
|
|
expect(results.map((r) => r.close.mock.calls.length)).toEqual([1, 1]);
|
|
expect(h.connClose).toHaveBeenCalled();
|
|
expect(h.dbClose).toHaveBeenCalled();
|
|
});
|
|
|
|
it('reports loaded for a single (non-array) result', async () => {
|
|
h.query.mockResolvedValue(closeable());
|
|
await expect(probeFtsExtensionLoad()).resolves.toEqual({ loaded: true });
|
|
});
|
|
|
|
it('reports the collapsed reason when LOAD fails', async () => {
|
|
h.query.mockRejectedValue(new Error('IO exception:\n invalid ELF header'));
|
|
await expect(probeFtsExtensionLoad()).resolves.toMatchObject({
|
|
loaded: false,
|
|
reason: 'IO exception: invalid ELF header',
|
|
});
|
|
});
|
|
|
|
it('times out instead of hanging when the native call never settles', async () => {
|
|
h.query.mockReturnValue(new Promise<unknown>(() => undefined));
|
|
await expect(probeFtsExtensionLoad(20)).resolves.toMatchObject({
|
|
loaded: false,
|
|
reason: expect.stringContaining('timed out'),
|
|
});
|
|
});
|
|
|
|
it('still closes the db when the Connection ctor throws', async () => {
|
|
h.connCtor.mockImplementation(() => {
|
|
throw new Error('connection ctor failed');
|
|
});
|
|
await expect(probeFtsExtensionLoad()).resolves.toMatchObject({ loaded: false });
|
|
expect(h.dbClose).toHaveBeenCalled();
|
|
});
|
|
|
|
it('reports loaded even when a result close() throws', async () => {
|
|
h.query.mockResolvedValue({
|
|
close: () => {
|
|
throw new Error('close boom');
|
|
},
|
|
});
|
|
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<unknown>(() => undefined));
|
|
await expect(probeVectorExtensionLoad(20)).resolves.toMatchObject({
|
|
loaded: false,
|
|
reason: expect.stringContaining('timed out'),
|
|
});
|
|
});
|
|
});
|