Commit graph

5 commits

Author SHA1 Message Date
Gergő Magyar
7f7255aef8
fix(analyze): load VECTOR before the incremental writeback touches embedding rows (#2623) (#2624)
* 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>
2026-07-22 12:27:00 +01:00
Gergő Magyar
e46651e42c
fix(embeddings): create VECTOR index via conn.query, not the prepared path (#2114)
`gitnexus analyze` silently failed to create the LadybugDB VECTOR/HNSW index because `CALL CREATE_VECTOR_INDEX(...)` was run through the prepared `conn.prepare()` path, which rejects multi-statement procedures — degrading semantic search to exact-scan. Route index creation through `conn.query()` via a new adapter-owned `createVectorIndex` (mirrors `createFTSIndex`), make the previously-swallowed error visible (`{ err }` logging), add an in-process idempotency cache, and add real-`@ladybugdb/core` regression coverage.

Fixes #2114.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 07:59:37 +01:00
Gergő Magyar
2a0c97c178
fix: add platform-aware semantic fallback (#1150)
* fix: add platform-aware semantic fallback

Make VECTOR an optional capability so Windows analysis remains stable while semantic embeddings can fall back to exact scan when native vector indexing is unavailable.

Made-with: Cursor

* fix: remove stale vector pool import

Keep the merge with main lint-clean after VECTOR loading moved out of the read pool.
2026-04-28 12:21:25 +01:00
Gergő Magyar
ffa0510f9a
fix(lbug): prevent DuckDB extension install hangs (#1129)
Some checks are pending
CI / quality (push) Waiting to run
CI / tests (push) Waiting to run
CI / e2e (push) Waiting to run
CI / scope-parity (push) Waiting to run
CI / Save PR Metadata (push) Blocked by required conditions
CI / CI Gate (push) Blocked by required conditions
Release Candidate / Check if release candidate should run (push) Waiting to run
Release Candidate / ci (push) Blocked by required conditions
Release Candidate / Publish release candidate to npm (push) Blocked by required conditions
Release Candidate / Build & Push RC Docker images (push) Blocked by required conditions
* fix(lbug): bound DuckDB extension install via ExtensionManager (closes #1128)

`gitnexus analyze` could hang indefinitely (60% / 85% on Windows) when
DuckDB's `INSTALL fts` or `INSTALL VECTOR` was unable to reach
`extensions.duckdb.org`. The DuckDB driver's INSTALL is a synchronous
network call, so any blocked egress would block the Node event loop
forever.

Replace the ad-hoc, in-process INSTALL/LOAD scattered across
`lbug-adapter.ts` and `pool-adapter.ts` with a single
`ExtensionManager` that owns the lifecycle of optional DuckDB
extensions:

* `LOAD` is always tried first — per-connection, idempotent, no network.
* If `LOAD` fails and policy permits, INSTALL runs in a short-lived
  child Node process bounded by `GITNEXUS_LBUG_EXTENSION_INSTALL_TIMEOUT_MS`
  (default 15s). The parent loop keeps spinning; on timeout the child is
  killed with SIGKILL and the capability is flagged unavailable.
* Capabilities and install attempts are cached per process, so a single
  bounded install per extension covers every subsequent call.

Install policy is now an explicit, per-context decision:

* `auto` (default for analyze) — try LOAD, fall back to bounded INSTALL.
* `load-only` — used by `pool-adapter` (serve / MCP read paths) so user
  queries never block on a network install.
* `never` — operator escape hatch for offline / airgapped environments.

`createFTSIndex` and `createVectorIndex` now check the boolean return
value before issuing the index DDL, so missing extensions degrade BM25
and semantic search gracefully without ever throwing during analyze.

Tests:
- New unit suite for `ExtensionManager` covering LOAD-first behavior,
  all three policies, install caching, observability, and warn dedup.
- Existing vector-extension integration tests pass against the new
  boolean return type.
- Existing embedding-pipeline mocks updated to return `true`.

Docs: `gitnexus/README.md` documents `GITNEXUS_LBUG_EXTENSION_INSTALL`
and `GITNEXUS_LBUG_EXTENSION_INSTALL_TIMEOUT_MS` with examples for
offline and slow-network environments.

Made-with: Cursor

* fix(lbug): move DuckDB extension install child into script

Keep the bounded out-of-process INSTALL behavior, but replace the inline child code with a stable packaged ESM script. This makes the child process directly runnable and gives debuggable stack traces without source-vs-dist branching or a runtime transpiler.

Made-with: Cursor
2026-04-27 23:09:17 +01:00
Mr. WorldwideBrown
6d9ec1009e
fix: load VECTOR extension during DB init for semantic search (#782)
* fix: load VECTOR extension during DB init for semantic search

The VECTOR extension was only loaded inside the embedding generation

pipeline (createVectorIndex). On a fresh gitnexus serve session,

semantic and hybrid search failed because QUERY_VECTOR_INDEX was

unknown.

Now loads the VECTOR extension alongside FTS during database

initialization in both the single-connection and pool-based paths.

Fixes #766

* fix: reset vectorExtensionLoaded on DB close and retry paths

The vectorExtensionLoaded flag was not being reset in closeLbug() or
the busy-retry cleanup path in withLbugDb(). This caused the VECTOR
extension to not be re-loaded after a close+re-init cycle, breaking
semantic search on reconnection.

Also resets shared.ftsLoaded and shared.vectorLoaded in the pool
adapter closeOne() for external DB entries, preventing stale extension
state when the pool is re-opened.

Adds integration tests covering vector extension loading, idempotency,
and state reset on both close and busy-retry paths.

* fix: set ftsLoaded flag in initLbugWithDb to avoid redundant extension reloads

* fix: set shared.vectorLoaded flag in initLbugWithDb to avoid redundant reloads
2026-04-11 12:17:40 +01:00