GitNexus/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts
Gergő Magyar e69d3c49c4
fix(analyze): gate FTS-indexed DML before the incremental writeback (#2841) (#2854)
* fix(lbug): never report a drop that could not happen, and gate FTS-indexed DML

`CALL DROP_FTS_INDEX` is itself an FTS-extension function, so with the
extension unloaded it fails with `Catalog exception: function DROP_FTS_INDEX
is not defined`. `isBenignDropFtsIndexError` classifies that as "nothing to
drop" — correct when the index does not exist, wrong when it does: the drop
silently no-ops and the next write to that table dies at bind time with an
engine message that never mentions FTS (#2841).

The classifier stays pure (a message cannot tell you whether an index is
live). Instead `dropFTSIndex` settles liveness with a catalog read on the
ERROR path only and raises an FTS-named, remedy-bearing error when the index
is present but undroppable.

Adds `ensureFtsRowDmlSafe`, the FTS twin of `ensureEmbeddingRowDmlSafe`
(#2623): catalog first, load FTS with the analyze policy only when an index
actually gates DML. LadybugDB refuses that DML at BIND time — a DETACH DELETE
matching zero rows fails exactly as hard as one matching thousands — and the
indexes cannot be cleared in place, so a verdict is the only useful answer.

Both gates now share one `SHOW_INDEXES` read via `readIndexCatalogRows`, so
adding the FTS check costs no extra catalog round-trip.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* fix(analyze): escalate instead of crashing when FTS blocks incremental DML

The incremental writeback decided its write plan without ever asking whether
row-level DML was legal. On a DB carrying FTS indexes with an unloadable FTS
extension, `deleteNodesForFiles` then died mid-writeback:

    Binder exception: Trying to delete from an index on table File but its
    extension is not loaded.

with no mention of FTS anywhere in the run — the only install-capable load
happened in Phase 3, long after the writes (#2841).

The incremental branch now reads the index catalog once and derives both
extension verdicts before any DML. When FTS (or VECTOR) blocks in-place
writes, the run falls through to the existing wipe-and-bulk-COPY escalation
— the same answer #2623 gave for VECTOR, and the only one available, since
the indexes cannot be dropped without the extension.

Every blocked extension is named in the reason log, not just the first one
checked: a DB can carry both a vector index and FTS indexes, and reporting
half the cause is how this failure stayed mis-diagnosed.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* test(analyze): cover the FTS DML gate, both-blocked escalation, and the drop guard

New `incremental-index-extension-dml-gate.test.ts` drives the real
`runFullAnalysis` against a real mini-repo and a real LadybugDB:

  - a DB carrying FTS indexes with FTS made unloadable escalates to a full DB
    write, names FTS in the log, ends with zero FTS indexes, and still has the
    newly committed content in the graph (pre-fix: Binder exception, exit 1);
  - FTS available keeps the surgical plan and the indexes;
  - a DB that never carried FTS indexes is not escalated (the catalog-first
    check must not tax FTS-less machines);
  - FTS and VECTOR both blocked produce ONE escalation naming both.

`drop-fts-index-error-classification.test.ts` gains the two `dropFTSIndex`
cases the #2841 guard turns on: live index + unloaded extension rejects with
an FTS-named error, absent index still resolves. The existing classifier
assertions are unchanged — it stays pure.

The CLI e2e reproduces the reporter's exact journey (analyze with the
extension, remove it, touch a file, analyze again) and asserts exit 0 plus an
FTS-named reason. It skips visibly when the seeded extension cannot load on
the host, so it can never report a false red about the fix.

Mutation-verified: reverting the run-analyze gate fails the first scenario;
reverting the dropFTSIndex guard fails the live-index case.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* fix(lbug): make every catalog-gated path fail closed, and classify the drop remedy

Review findings on #2854 (two-engine, 17 lanes).

H3 — `ftsIndexExistsInCatalog` returned `false` when the catalog could not be
read, i.e. "index absent", so `dropFTSIndex` swallowed the error and the caller
proceeded as if the index were gone. That is the #2841 symptom the guard exists
to make loud, and it contradicted the contract `readIndexCatalogRows` states two
functions above. It now fails closed.

§6.A — `ensureFtsRowDmlSafe` keyed on `index_type === 'FTS'`, which answers
`undefined === 'FTS'` → false → *no gate* for a row whose shape cannot be read:
fail-open, in the gate whose only job is preventing an unsafe write, while the
VECTOR twin fails closed on the same input. Now only a positively-identified
non-FTS index is waved through. Deliberately NOT the twin's `!== 'HASH'`: that
is safe there only because it is scoped to the embedding table first, and this
gate is table-agnostic — `!== 'HASH'` would let the HNSW index gate FTS DML.

§5.A — `undefined` was overloaded: "caller passed nothing" and "caller tried and
could not prove anything" shared one value, so a failed shared read silently
became three reads and the two gates could decide from different snapshots. The
failed snapshot is now representable (`INDEX_CATALOG_UNREADABLE`), leaving one
unambiguous `??` in `resolveGateRows`.

§5.B — both gates regained the unconditional null-connection precondition the
refactor moved into the reader.

§5.G — the throw's remedy now routes through `diagnoseExtensionLoad`, like
`--repair-fts` and `ftsDegradedWarning`, so a missing runtime dependency is not
told to reinstall. The message stays path-free (#2374/#2375).

The dead positional row fallbacks are kept and marked `LADYBUGDB-CONTRACT`:
removing them would turn a proven-inert hedge into a fail-open gate if a future
engine returns unnamed tuples.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* fix(analyze): never undo an explicit wipe, stage extension-forced rebuilds, report honestly

Review findings on #2854 (two-engine, 17 lanes).

H1 (P1, both engines) — `analyze --drop-embeddings` was silently reverted. The
`--drop-embeddings` → `force` conversion sits inside the `embeddingCheckpoint`
branch, so without a checkpoint the run stays incremental and reaches the gate;
the flag then *deliberately* leaves `cachedEmbeddings` empty, which is exactly
the rescue's trigger, so every row the operator asked to destroy was read back
and restored, exit 0. Widening the rescue from `!embeddingRowDmlSafe` to
`extensionForcedRebuild` moved that latent bug onto the dominant path, because
every analyzed DB carries FTS indexes. Guarded on the flag itself — NOT on
`shouldLoadCache`, which is false in the meta-under-reports case the rescue
exists for and would have deleted the safeguard while fixing the wipe. The
`--drop-embeddings --embeddings` variant is covered by the same guard.

H2 — an extension-forced escalation wiped the LIVE index in place: `buildPath`
was frozen ~440 lines earlier while the run was still classified incremental,
so an interrupt or ENOSPC left no complete index, where main failed at bind time
with it intact. Extension-forced rebuilds now build into a staging file and
publish via the existing atomic swap; size-forced ones stay in place, since that
trigger is the repo's own churn rather than a machine condition.

H5 — the escalation log asserted a vector index "exists" and that the store
"carries FTS indexes" in exactly the case the catalog read proved nothing, while
the only truthful signal went to stderr rather than the IPC log. It now emits a
distinct unreadable-catalog cause, and "this index carries" (which pointed at
the vector index just named) reads "the graph store carries".

§5.D — the write-set cause was dropped whenever an extension cause co-occurred;
causes are appended now, not selected between.

§5.C — after an FTS-forced rebuild stamped lastCommit, a plain rerun on the same
commit hit the alreadyUpToDate fast path before Phase 3, so the CLI's "install
… then rerun" advice could never restore FTS. The fast path is now bypassed when
meta records FTS unavailable and the extension can load again, keyed on the
persisted capabilities stamp rather than new state.

§5.F (skip the escalation for a zero-change commit) is deliberately NOT
implemented: `deleteSpringAutoConfigurationSyntheticClasses` and
`deleteSpringAopEvidenceNodes` run unconditionally on the surgical branch and
bind against FTS-indexed `Class`/`CodeElement`, and a zero-row DETACH DELETE
fails at bind time exactly as hard as a large one — so the skip would restore
the original crash.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* perf(search): read the index catalog once per drop sweep, and state the real contract

Review findings on #2854.

H4 — on a machine where FTS cannot load and the DB carries no FTS index, the
gate correctly returned early without loading the extension, but the surgical
path still ran the full 20-entry drop sweep: every `CALL DROP_FTS_INDEX` raised
"function DROP_FTS_INDEX is not defined", and the new liveness guard then fired
a fresh catalog read per table — 20 reads every run, forever, for exactly the
offline/load-only population, contradicting the "healthy path costs nothing"
claim shipped with the guard. The sweep now reads the catalog once and skips
entirely when no FTS-typed index exists. An unreadable catalog runs the sweep,
so an unprovable catalog never skips real work.

H8 — the docstring still promised `dropFTSIndex` "tolerates" an unloadable
extension. Post-#2854 a live index plus an unloadable extension throws, and
safety rests on caller ordering discipline rather than the type system — which
is what would have talked the next caller out of that ordering.

GUARDRAILS — the "switching to a full DB write" sign described exactly one
trigger (write set >~50%). Since #2623 and #2841 an unloadable extension
escalates regardless of write-set size; documented with its recovery steps.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* test(analyze): cover the wipe guard, the staged rebuild, and the fail-closed branches

Review findings on #2854.

H1/H2 mutation-verified: removing `!options.dropEmbeddings` fails the new
drop-embeddings case ("expected true to be false"); disabling the staging
upgrade fails the staging case ("expected 0 to be greater than 0"), so both
assert behaviour rather than describe it.

Gate suite (7 cases): `--drop-embeddings` under an FTS-forced escalation ends at
zero embedding rows and logs no "Preserving"; the escalation is one-shot — a
third run on a healthy host returns to surgery and rebuilds every FTS index; an
extension-forced rebuild is observed building into `lbug.staging.*` and leaves
none behind; the rescue complement still preserves un-stamped rows when no wipe
was requested; the never-built case now asserts the commit reached the graph.

H6 — the both-blocked case hard-asserted `createVectorIndex()` while the suite
probed FTS only, so it went red on any FTS-yes/VECTOR-no host. VECTOR is probed
now and gates only that case, with a GITNEXUS_REQUIRE_VECTOR hard-fail.

H7 — the fail-closed branches had no coverage although the VECTOR twin's test
and interception technique were ready to copy: `ensureFtsRowDmlSafe` under an
unreadable catalog now proves it routes to the load, and `dropFTSIndex` proves
it rejects rather than silently tolerating. Plus a redaction case that forces a
real path-bearing load failure — under policy `never` the assertion would have
been vacuous, since that reason carries no path.

§5.E/§6.B — the suite is registered in the cross-platform matrix (its sibling
was; it wasn't, and GITNEXUS_REQUIRE_VECTOR is set only on that job) and moved
into the sequential lbug-db project per TESTING.md:68, verified not to drop it
from the sharded ubuntu job. A Windows shard weight is added as a labelled
estimate — the 8s floor would skew the split it exists to protect.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* refactor(analyze): make the FTS gate's fast path cheap, its claims provable, and its remedies classified

Cleanup review of the #2841 work (four parallel angles: reuse, simplification,
efficiency, altitude). Behaviour-preserving except where the previous behaviour
was wrong.

Correctness the review caught:

- The fast-path probe keyed on `capabilities.fts.status === 'unavailable'`,
  which collapses "extension unavailable" and "index build failed". A
  deterministic build failure (an un-tokenizable row, #2544) therefore bypassed
  `alreadyUpToDate` on EVERY subsequent run, re-analyzed the whole repo, failed
  the same way, and restamped — a permanent loop where the run used to be one
  `stat`. Phase 3 already computes the discriminator; it is now persisted as
  `fts.skipReason` and the probe only runs for `extension-unavailable`. Metas
  written before this carry no field and keep today's behaviour.

- `dropSearchFTSIndexes` skipped its sweep when no row read `index_type ===
  'FTS'`, while `ensureFtsRowDmlSafe` treats an unreadable type as "might be
  FTS". Opposite polarity, under a comment claiming they matched: a row-shape
  change would let the gate wave the surgical plan through while the sweep
  dropped nothing, putting DELETEs back on tables carrying live FTS indexes —
  #2589 again. The sweep now decides per configured index on identity, which
  is also strictly more precise. Its old justification (leftover indexes under
  other names) was unreachable — the loop only ever drops configured entries.

- `dropFTSIndex` threw "FTS index X on table Y exists" on the one path where
  the catalog could not be read — a fabricated claim, on a DB the same run had
  just shown carries no FTS index. Presence is now `present | absent |
  unverifiable` and the message says which.

- The remedy was hand-written for three of the four load-failure classes,
  discarding `missingFileRemedy`/`corruptFileRemedy`, so a corrupt extension
  file was told to retry an install — the misdirection #2383 fixed. Both the
  drop error and the escalation log now use the classified remedy.

Cost, measured on a 391 MB index (cold open ~1 s, SHOW_INDEXES ~4 ms):

- The probe opened the live index WRITABLE on the millisecond fast path,
  dragging in schema DDL, the cross-process write lock, sidecar reclaim and a
  CHECKPOINT on close. It is read-only now. That also closes an install trap:
  `doInitLbug`'s pre-load resolves the env policy on the writable branch, so an
  operator following our own `GITNEXUS_LBUG_EXTENSION_INSTALL=auto` advice paid
  a forked 15 s installer on every up-to-date run (memoized per process; the CLI
  is a fresh process each time). The read-only branch pins `load-only`.

- A failed staged rebuild orphaned a full index-sized copy until the next lock
  sweep; the failure path now reclaims it.

- The sweep re-read a catalog the run already held, defeating the invariant the
  snapshot type exists to enforce.

Structure: row-shape accessors have one home, so the LADYBUGDB-CONTRACT grep
claim is true by construction; staging now applies to both escalation causes,
since recoverability is a property of the wipe-then-COPY plan, not of the
trigger; `getExtensionCapability`/`getFtsCapability` replace hand-spelled
lookups where the seam allows.

Two lookups in run-analyze.ts deliberately keep the exported
`getExtensionCapabilities()` form: the #2383 tests stub that export, and an ESM
module mock does not intercept a helper's internal call — routing through it
silently degraded the classified remedy to generic text. Recorded in-comment.

Not taken, deliberately: extracting the escalation message and replacing the
snapshot protocol with a connection-scoped catalog memo (both sound, both
restructure code this PR just stabilised — they belong in their own change);
an extension registry (premature at two instances, and the FTS/VECTOR polarity
difference is exactly what it would have to parameterize back out).

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* test(analyze): pin both sides of the degraded-FTS fast-path bypass

`healDegradedFts` (§5.C) had zero coverage — three separate review angles
flagged it, and the cleanup pass then found it sat one conjunct away from a
permanent full-re-analyze loop. Both sides are pinned now:

- it re-analyzes past `alreadyUpToDate` when the stored meta says FTS is
  degraded and the extension loads again: run 1 analyzes with loads blocked
  (asserting the precondition — `status: 'unavailable'`, `skipReason:
  'extension-unavailable'` — rather than assuming it), then a same-commit
  clean-tree rerun rebuilds every FTS index without a file changing;
- it stands down when the degradation was a BUILD failure: the stored
  `skipReason` is rewritten to 'build-failed' and the rerun must take the fast
  path, because that rebuild would fail identically on every run forever.

The build-failed state is reached by rewriting the stamped discriminator, not
by provoking a real tokenizer failure: a genuine one needs a stored row the
native tokenizer rejects (#2544/#2546), which is neither portable across the CI
matrix nor deterministic, and §5.C reads only that field.

Also folds the first escalation case into the one-shot case. The claim that it
was fully subsumed did not hold on audit: `logs` containing 'FTS' was unique as
expected, but so was the duplicate-File-node row count — every other reader goes
through a Map keyed by path, which collapses a stale twin an appending rebuild
would leave. Both assertions moved rather than one being dropped.

Net suite runtime goes UP (two cycles removed, four added), against the
cross-platform-matrix argument that motivated the dedup — recorded here because
the shard weight is an estimate pending a real Windows measurement.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* test(search): keep the whole-module adapter mock in step with the row accessors

The cleanup pass moved the LadybugDB row-shape reads behind named accessors so
the column contract has one home. `fts-indexes.test.ts` mocks the entire adapter
module with a hand-written factory, which still exposed only the three exports
the file imported before — so `verifySearchFTSIndexes` failed with "No
`indexRowName` export is defined on the mock" while production was fine.

The added accessors mirror the real implementations rather than returning
stubs. A stub would have read `undefined` out of every catalog row and let the
suite pass for the wrong reason — the failure mode a whole-module mock invites
whenever the module under test grows an import.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* revert(analyze): drop the degraded-FTS auto-heal, fix the advice it existed to justify

§5.C's complaint was that the CLI tells users to "install the extension … then
rerun" when a rerun lands on the up-to-date fast path and rebuilds nothing. The
answer shipped for it was a probe that bypasses that fast path. Four independent
problems later, the sentence is cheaper to fix than to make true:

- it could not tell "extension was missing" from "index build failed" without a
  stamped discriminator, so a deterministic build failure (#2544/#2546)
  re-analyzed the entire repo on every invocation, forever, where the run used
  to be one `stat`;
- it opened the live index on the millisecond fast path — writable at first,
  dragging in DDL, the cross-process lock and a CHECKPOINT (~1 s on a 391 MB
  index), and even read-only it is a full open;
- `doInitLbug`'s pre-load resolves the env policy, so an operator following our
  own `GITNEXUS_LBUG_EXTENSION_INSTALL=auto` advice paid a forked 15 s installer
  per up-to-date run;
- and it turns the fast path into a full re-analysis whenever an index authored
  where FTS was unavailable is later read where it loads — a legitimate, common
  state, and the invariant `analyzer-identity-cli.test.ts` pins.

So: no probe. The degraded-search warning now points at `gitnexus analyze
--repair-fts`, which rebuilds the search indexes without re-parsing the repo,
instead of "then rerun". One line, no new failure modes, and it is what the
issue actually asked for.

`capabilities.fts.skipReason` stays in the meta stamp: it costs three lines,
makes the two degradation causes distinguishable for support, and is what any
future correct answer here would key on.

Also gates the H2 staging assertion on the production predicate. It asserted
staging unconditionally while the upgrade requires `posixSwap || windowsSwapOk`,
and `windowsSwapOk` is opt-in (#2614) — so it failed on the Windows matrix for a
reason unrelated to #2841. Registering this suite cross-platform is what exposed
it; the assertion now mirrors the condition it is testing.

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

* fix(analyze): never stage around a damaged index — escalate in place when the catalog is unreadable

CI caught this on ubuntu and macOS: `analyze-wal-checkpoint-failure` stopped
failing, which is worse than it sounds.

That test plants a directory at `.gitnexus/lbug.wal.checkpoint` so the
auto-checkpoint's rename target is blocked, and asserts analyze exits non-zero
with the `--wal-checkpoint-threshold` hint. But LadybugDB cannot open that path
at all, so `CALL SHOW_INDEXES()` now fails with `IO exception: … Is a
directory`. The catalog read returns UNREADABLE, both DML gates correctly fail
closed, both extension loads fail with the same IO error, and the run escalates
— and since the escalation stages, it built a fresh index at
`lbug.staging.<uuid>`, swapped it in, and exited 0.

The blocked path was never touched. The run "succeeded" while the damage sat
untouched on disk, waiting to break the next in-place writeback.

So the staging upgrade is now conditional on the catalog having been READ.
Staging exists to protect a healthy live index from a machine-level cause (an
extension that will not load); it must not be used to route around a damaged
one. When we are escalating out of ignorance, build in place so the underlying
IO fault lands on the failure path where the operator gets a diagnosis.

Verified against the real CLI, not just the suite: with a directory planted at
the checkpoint path, analyze now exits 1 and prints
`gitnexus analyze --wal-checkpoint-threshold 67108864`. The healthy
extension-forced case still stages (gate suite 6/6).

Refs #2841

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 09:44:44 +01:00

565 lines
27 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* Integration coverage for `deleteNodesForFiles` — the batched incremental
* delete introduced for #2409.
*
* The per-file predecessor issued a count + DETACH DELETE per node table per
* FILE (~13k single-row write transactions on a ~700-file write set); the
* batched variant chunks paths into `IN [...]` lists. These tests pin the
* contract the incremental writeback depends on:
*
* - exactly the requested files' rows are deleted, across a >1-chunk set
* - DETACH semantics: relationships touching deleted nodes go away,
* relationships between survivors stay
* - single quotes in paths are escaped, not injected
* - unknown paths are a no-op success (zero-match ≠ error)
* - onChunk progress reports cumulative file counts
* - CodeEmbedding rows ride along with their file's nodes (tri-review
* 4669518496 P2-1): node ids are label-first (`Function:<fp>:fn:1`), so
* the delete joins `e.nodeId = n.id` through the still-present nodes —
* deleted/quoted files' rows go, survivors' rows stay.
*/
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';
import { DELETE_FILES_CHUNK_SIZE } from '../../src/core/lbug/lbug-adapter.js';
import { EMBEDDING_TABLE_NAME, EMBEDDING_DIMS } from '../../src/core/lbug/schema.js';
const FILE_COUNT = DELETE_FILES_CHUNK_SIZE + 30; // crosses the chunk boundary
const KEEP_COUNT = 10;
const QUOTED_PATH = "src/we'ird.ts";
const filePath = (i: number): string => `src/f-${String(i).padStart(4, '0')}.ts`;
function buildFixtureGraph() {
const nodes: TestNodeInput[] = [];
const rels: TestRelInput[] = [];
for (let i = 0; i < FILE_COUNT; i++) {
const fp = i === 0 ? QUOTED_PATH : filePath(i);
nodes.push({ id: `File:${fp}`, label: 'File', name: path.basename(fp), filePath: fp });
nodes.push({
id: `Function:${fp}:fn${i}:1`,
label: 'Function',
name: `fn${i}`,
filePath: fp,
startLine: 1,
endLine: 3,
isExported: true,
});
rels.push({ sourceId: `File:${fp}`, targetId: `Function:${fp}:fn${i}:1`, type: 'CONTAINS' });
if (i > 0) {
// Every function calls the previous file's function — so deleting a
// file must DETACH-drop edges on both sides of the kept/deleted
// boundary while the survivor-to-survivor edges remain.
const prev = i === 1 ? QUOTED_PATH : filePath(i - 1);
rels.push({
sourceId: `Function:${fp}:fn${i}:1`,
targetId: `Function:${prev}:fn${i - 1}:1`,
type: 'CALLS',
});
}
}
return buildTestGraph(nodes, rels);
}
withTestLbugDB('delete-nodes-for-files', (handle) => {
describe('deleteNodesForFiles (batched incremental delete, #2409)', () => {
it('deletes exactly the requested files across chunks with DETACH semantics, quote escaping, embedding-row joins, and zero-match no-ops', async () => {
const { loadGraphToLbug, deleteNodesForFiles, executeQuery, executeWithReusedStatement } =
await import('../../src/core/lbug/lbug-adapter.js');
const { batchInsertEmbeddings } =
await import('../../src/core/embeddings/embedding-pipeline.js');
await loadGraphToLbug(buildFixtureGraph(), '/tmp/repo', path.dirname(handle.dbPath));
const count = async (cypher: string): Promise<number> => {
const rows = (await executeQuery(cypher)) as Array<{ c: number | bigint }>;
return Number(rows[0]?.c ?? 0);
};
expect(await count('MATCH (n:File) RETURN count(n) AS c')).toBe(FILE_COUNT);
expect(await count('MATCH (n:Function) RETURN count(n) AS c')).toBe(FILE_COUNT);
const callsBefore = await count(
`MATCH ()-[r:CodeRelation]->() WHERE r.type = 'CALLS' RETURN count(r) AS c`,
);
expect(callsBefore).toBe(FILE_COUNT - 1);
// Seed embedding rows through the real batchInsertEmbeddings for a
// to-be-deleted plain-path file, the quoted-path file, and a survivor.
// nodeIds are the fixture's REAL label-first node ids — the exact
// format the old bare-path `STARTS WITH` shape could never match
// (tri-review 4669518496 P2-1). Zero vectors: the CodeEmbedding table
// is plain schema (no VECTOR extension involved).
const SURVIVOR_PATH = filePath(FILE_COUNT - 1);
const survivorEmbeddingNodeId = `Function:${SURVIVOR_PATH}:fn${FILE_COUNT - 1}:1`;
const survivorFileEmbeddingNodeId = `File:${SURVIVOR_PATH}`;
const seededEmbeddingNodeIds = [
`Function:${filePath(1)}:fn1:1`, // deleted, plain path
`File:${filePath(1)}`, // deleted fallback File embedding, plain path
`Function:${QUOTED_PATH}:fn0:1`, // deleted, quoted path
`File:${QUOTED_PATH}`, // deleted fallback File embedding, quoted path
survivorEmbeddingNodeId, // survives the delete
survivorFileEmbeddingNodeId, // fallback File embedding also survives
];
await batchInsertEmbeddings(
executeWithReusedStatement,
seededEmbeddingNodeIds.map((nodeId) => ({
nodeId,
chunkIndex: 0,
startLine: 1,
endLine: 3,
embedding: new Array(EMBEDDING_DIMS).fill(0),
})),
);
expect(await count(`MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN count(e) AS c`)).toBe(
seededEmbeddingNodeIds.length,
);
// Delete everything except the last KEEP_COUNT files. Includes the
// quoted path (chunk 1), crosses into chunk 2, and appends a path with
// no rows at all — which must not fail the batch.
const toDelete: string[] = [QUOTED_PATH];
for (let i = 1; i < FILE_COUNT - KEEP_COUNT; i++) toDelete.push(filePath(i));
toDelete.push('src/never-existed.ts');
const chunkCalls: Array<[number, number]> = [];
await deleteNodesForFiles(toDelete, {
onChunk: (done, total) => chunkCalls.push([done, total]),
});
// Cumulative chunk progress: [200, 221] then [221, 221].
expect(chunkCalls).toEqual([
[DELETE_FILES_CHUNK_SIZE, toDelete.length],
[toDelete.length, toDelete.length],
]);
expect(await count('MATCH (n:File) RETURN count(n) AS c')).toBe(KEEP_COUNT);
expect(await count('MATCH (n:Function) RETURN count(n) AS c')).toBe(KEEP_COUNT);
// Quoted path really gone (escaping worked; nothing else was swept up).
expect(
await count(`MATCH (n:File) WHERE n.filePath = "${QUOTED_PATH}" RETURN count(n) AS c`),
).toBe(0);
// DETACH: the only CALLS edges left are between surviving functions —
// KEEP_COUNT survivors form a chain of KEEP_COUNT-1 edges; the edge from
// the first survivor into the deleted region is gone.
expect(
await count(`MATCH ()-[r:CodeRelation]->() WHERE r.type = 'CALLS' RETURN count(r) AS c`),
).toBe(KEEP_COUNT - 1);
// Survivors untouched.
expect(
await count(
`MATCH (n:File) WHERE n.filePath = '${filePath(FILE_COUNT - 1)}' RETURN count(n) AS c`,
),
).toBe(1);
// Embedding rows followed their files: ONLY the survivor's row remains
// — exact nodeId, not count-only, so a delete that swept the wrong rows
// (or none) cannot pass. The quoted-path row proves the join statement
// escapes list literals, not just the per-table deletes.
const embRows = (await executeQuery(
`MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN e.nodeId AS nodeId`,
)) as Array<{ nodeId: string }>;
expect(embRows.map((r) => String(r.nodeId)).sort()).toEqual(
[survivorEmbeddingNodeId, survivorFileEmbeddingNodeId].sort(),
);
// Zero-match batch (all paths already gone) is a clean no-op.
await expect(deleteNodesForFiles([QUOTED_PATH, filePath(1)])).resolves.toBeUndefined();
// …and it left the surviving embedding row alone.
expect(await count(`MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN count(e) AS c`)).toBe(2);
}, 120_000);
it('a File node without an embedding deletes cleanly and leaves other files embedding rows intact (FIX 4)', async () => {
const { deleteNodesForFiles, executeQuery } =
await import('../../src/core/lbug/lbug-adapter.js');
const count = async (cypher: string): Promise<number> => {
const rows = (await executeQuery(cypher)) as Array<{ c: number | bigint }>;
return Number(rows[0]?.c ?? 0);
};
// File can own fallback embeddings, but this fixture deliberately has
// none. The delete must still remove the node row without erroring, and
// embedding rows owned by OTHER files stay put.
const ASSET_PATH = 'src/assets-only.txt';
await executeQuery(
`CREATE (:File {id: 'File:${ASSET_PATH}', name: 'assets-only.txt', filePath: '${ASSET_PATH}'})`,
);
const embeddingsBefore = await count(
`MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN count(e) AS c`,
);
await expect(deleteNodesForFiles([ASSET_PATH])).resolves.toBeUndefined();
expect(
await count(`MATCH (n:File) WHERE n.filePath = '${ASSET_PATH}' RETURN count(n) AS c`),
).toBe(0);
expect(await count(`MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN count(e) AS c`)).toBe(
embeddingsBefore,
);
}, 120_000);
it('deleteNodesForFile removes a fallback embedding owned by the File node', async () => {
const { deleteNodesForFile, executeQuery, executeWithReusedStatement } =
await import('../../src/core/lbug/lbug-adapter.js');
const { batchInsertEmbeddings } =
await import('../../src/core/embeddings/embedding-pipeline.js');
const count = async (cypher: string): Promise<number> => {
const rows = (await executeQuery(cypher)) as Array<{ c: number | bigint }>;
return Number(rows[0]?.c ?? 0);
};
const filePath = 'docs/singular.md';
const nodeId = `File:${filePath}`;
await executeQuery(
`CREATE (:File {id: '${nodeId}', name: 'singular.md', filePath: '${filePath}'})`,
);
await batchInsertEmbeddings(executeWithReusedStatement, [
{
nodeId,
chunkIndex: 0,
startLine: 1,
endLine: 1,
embedding: new Array(EMBEDDING_DIMS).fill(0),
},
]);
await expect(deleteNodesForFile(filePath)).resolves.toEqual({ deletedNodes: 1 });
expect(
await count(
`MATCH (e:${EMBEDDING_TABLE_NAME}) WHERE e.nodeId = '${nodeId}' RETURN count(e) AS c`,
),
).toBe(0);
}, 120_000);
});
});
/**
* Missing-embedding-table tolerance (FIX 4): a DB created without
* EMBEDDING_SCHEMA raises `Binder exception: Table CodeEmbedding does not
* exist.` (probe-recorded on @ladybugdb/core 0.18.0) on the join-delete.
* deleteNodesForFiles must tolerate exactly that one case — warn and keep
* going — instead of bricking every incremental run until `--force`, while
* the node-table deletes still complete. Own withTestLbugDB block: the
* DROP TABLE would poison the sibling suite's shared DB.
*/
withTestLbugDB('delete-nodes-missing-embedding-table', () => {
describe('deleteNodesForFiles without a CodeEmbedding table (FIX 4)', () => {
it('resolves, still deletes the node rows, and later statements keep working', async () => {
const { deleteNodesForFiles, executeQuery } =
await import('../../src/core/lbug/lbug-adapter.js');
const count = async (cypher: string): Promise<number> => {
const rows = (await executeQuery(cypher)) as Array<{ c: number | bigint }>;
return Number(rows[0]?.c ?? 0);
};
await executeQuery(
`CREATE (:Function {id: 'Function:src/a.ts:fnA:1', name: 'fnA', filePath: 'src/a.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`,
);
await executeQuery(
`CREATE (:Function {id: 'Function:src/b.ts:fnB:1', name: 'fnB', filePath: 'src/b.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`,
);
// Build-variant DB without the embedding schema.
await executeQuery(`DROP TABLE ${EMBEDDING_TABLE_NAME}`);
await expect(deleteNodesForFiles(['src/a.ts'])).resolves.toBeUndefined();
// The node delete completed despite the tolerated missing-table warn…
expect(
await count(`MATCH (n:Function) WHERE n.filePath = 'src/a.ts' RETURN count(n) AS c`),
).toBe(0);
// …the untouched file survives…
expect(
await count(`MATCH (n:Function) WHERE n.filePath = 'src/b.ts' RETURN count(n) AS c`),
).toBe(1);
// …and the connection stays healthy for subsequent batches.
await expect(deleteNodesForFiles(['src/b.ts'])).resolves.toBeUndefined();
expect(await count(`MATCH (n:Function) RETURN count(n) AS c`)).toBe(0);
}, 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.
*
* The block also carries the FTS twins of the same seam (#2841) —
* `ensureFtsRowDmlSafe` and, through the public `dropFTSIndex`, the catalog
* read behind it. They live here rather than in a new block because they need
* the identical machinery: the policy-reopen helper above, and the
* SHOW_INDEXES interception below that is the only way to stage an unreadable
* catalog. Where the VECTOR gate may fall back to a load it does not strictly
* need, both FTS readers must fall CLOSED — see the cases themselves.
*/
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<void> => {
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<void> => {
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<number> => {
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<void> => {
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);
/**
* Run `run` with every `CALL SHOW_INDEXES()` on the writable connection
* forced to fail, passing every other statement through to the real engine
* and recording the SQL that was attempted.
*
* Forcing the read is the ONLY way to reach the gates' "could not prove
* anything" branch: `SHOW_INDEXES` is readable with no extension loaded, so
* a genuine unreadable catalog cannot be staged by configuration alone.
*/
const withUnreadableIndexCatalog = async (
run: (seen: readonly string[]) => Promise<void>,
): Promise<void> => {
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 run(seen);
} finally {
spy.mockRestore();
}
};
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');
await withUnreadableIndexCatalog(async (seen) => {
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);
});
}, 120_000);
/**
* The FTS twin of the case above (#2841). Same seam, opposite polarity:
* `ensureEmbeddingRowDmlSafe` may fall back to a load it does not strictly
* need, but `ensureFtsRowDmlSafe`'s only job is refusing an unsafe write, so
* an unprovable catalog must never be answered "no FTS index seen ⇒ safe".
*
* Both halves are asserted because either one alone is satisfiable by a
* broken gate: the verdict alone could come from a gate that never looked at
* the extension, and the LOAD alone could come from a gate that issued it
* and then returned `true` regardless.
*/
it('FTS twin: an unreadable catalog fails CLOSED — verdict blocked, and the FTS load is attempted (#2841)', async () => {
const { ensureFtsRowDmlSafe } = await import('../../src/core/lbug/lbug-adapter.js');
// Half 1 — `never` makes the extension provably unloadable on every host,
// so the verdict is deterministic: a fail-OPEN gate answers `true` here.
await reopenWithPolicy('never');
await withUnreadableIndexCatalog(async (seen) => {
await expect(ensureFtsRowDmlSafe()).resolves.toBe(false);
expect(seen.some((s) => s.includes('SHOW_INDEXES'))).toBe(true);
});
// Half 2 — refusing is not enough: the gate must TRY to make the write
// legal, or an unreadable catalog would escalate every run to a full
// rebuild on a machine where FTS loads perfectly.
//
// Staging that needs care. `initLbug` pre-loads FTS itself whenever the
// policy permits (lbug-adapter.ts), and the adapter latches the result,
// so a gate running after a successful init-time load issues nothing and
// the assertion would be unfalsifiable. So: reopen under `never` (init's
// pre-load is refused, latch stays clear), then relax the policy WITHOUT
// reopening — the gate resolves it from the environment at call time, so
// the LOAD that appears is unambiguously its own. `afterEach`'s
// reopenWithPolicy(undefined) restores the variable.
//
// Deliberately not asserting the verdict here: whether FTS actually
// loads is a property of the host; ATTEMPTING it is the contract.
await reopenWithPolicy('never');
process.env.GITNEXUS_LBUG_EXTENSION_INSTALL = 'load-only';
await withUnreadableIndexCatalog(async (seen) => {
await ensureFtsRowDmlSafe();
expect(seen.some((s) => /^\s*LOAD EXTENSION fts\b/i.test(s))).toBe(true);
// …and it did not charge the caller a VECTOR load it never needed.
expect(seen.some((s) => /^\s*LOAD EXTENSION vector\b/i.test(s))).toBe(false);
});
}, 120_000);
/**
* `ftsIndexExistsInCatalog` is private; `dropFTSIndex` is the public surface
* that consumes it, and the H3 fix lives entirely in its unreadable-catalog
* branch. With the extension unloaded the DROP always fails the same way
* (`Catalog exception: function DROP_FTS_INDEX is not defined`), so the
* catalog read is the ONLY thing deciding whether the caller is told — which
* makes the two calls below a controlled pair on one connection.
*/
it('FTS twin: dropFTSIndex rejects (not resolves) when the extension is unloaded and the catalog is unreadable (#2841 H3)', async () => {
await reopenWithPolicy('never');
const { dropFTSIndex } = await import('../../src/core/lbug/lbug-adapter.js');
// A name no index carries: with a READABLE catalog this is provably
// "nothing to drop", which is exactly what the unreadable read must NOT
// be allowed to imply.
const ABSENT_INDEX = 'delete_nodes_2841_absent_fts';
// Control — catalog readable, index provably absent ⇒ benign no-op.
await expect(dropFTSIndex('File', ABSENT_INDEX)).resolves.toBeUndefined();
// Same call, same connection, same engine error; only the catalog read
// changes. Before the fix this ALSO resolved silently — reporting an
// unprovable catalog as "index absent" and handing the caller a drop that
// never happened, one DML statement before the #2841 crash.
await withUnreadableIndexCatalog(async (seen) => {
// The message must claim only what the run can prove. This branch is
// reached when the catalog is UNREADABLE, and the only path into it
// (`ensureFtsRowDmlSafe` waved the surgical plan through because the
// catalog showed no FTS index, then a later read failed) is one where
// the same run already established the opposite of "exists" — so
// asserting existence here would state a fabricated fact while
// failing the run.
await expect(dropFTSIndex('File', ABSENT_INDEX)).rejects.toThrow(
/FTS index '.*' on table File could not be verified as absent/,
);
expect(seen.some((s) => s.includes('DROP_FTS_INDEX'))).toBe(true);
expect(seen.some((s) => s.includes('SHOW_INDEXES'))).toBe(true);
});
}, 120_000);
});
});