diff --git a/gitnexus/bench/scope-capture/baselines.json b/gitnexus/bench/scope-capture/baselines.json index d4e86debb..d1413db56 100644 --- a/gitnexus/bench/scope-capture/baselines.json +++ b/gitnexus/bench/scope-capture/baselines.json @@ -89,7 +89,7 @@ "_rebaselined": "#1919 review CF3 fix: extended kotlin-local-property-owner (init/accessor destructuring) + new dart-accessor-owner fixture (getter/setter ownership). Fingerprint-only corpus drift; scaling ~1.0." }, "java": { - "fingerprint": "975b68aaac6d06094260fb0c67f9b1bc03692ba7220669d192aca9dccd5fc0ca", + "fingerprint": "85fc7af9c3c1bceac76cb4f27214410b04967682a2eaa7e468e26efd1f4e2537", "scaling_budget": 1.5, "_rebaselined_callable_flow_2522_review": "PR #2522 review hardening: callable operands retain expression/qualified identity and formals retain signature metadata; same-name lexical regions use an O(ancestor-depth) ID-set lookup. Prior d5c59d7dc9e206637515d5aea1163f7c1cdd76410c38c5fe6143d13d19677d6a -> 004a3592998dca1193bd1429a8284513725de7764f2a3eceedaaa984cfd763b4; scaling 0.992 < 1.5.", "_rebaselined_callable_flow_2522_followup": "PR #2522 follow-up: Java method-reference/SAM callable flow facts with invocation-result suppression. Prior 062d754764aaa8a6772fb90875c710502a63e3e7a300e633942381ed914faada -> d5c59d7dc9e206637515d5aea1163f7c1cdd76410c38c5fe6143d13d19677d6a; scaling 1.074 < 1.5.", @@ -97,7 +97,8 @@ "_note": "#1928 / #2045: F35 adds qualified + qualified-generic constructor query captures (`new pkg.Foo()`, `new a.b.Foo()`, `new pkg.Box()`); F38 synthesizes `@reference.call.constructor` on `super(...)`/`this(...)` explicit_constructor_invocation nodes; F41 generic-aware stripQualifier in interpret (type-binding normalization). + java-qualified-constructor and java-explicit-constructor fixtures. Pure capture-additive + fixture-corpus drift; scaling stays linear (~1.06).", "_rebaselined_2522_review_fixes": "PR #2522 review fixes: get/test dropped from callableProtocolMethods. Prior 004a3592998dca1193bd1429a8284513725de7764f2a3eceedaaa984cfd763b4 -> f3b4f4b6610e07c3ac90deb1c53d3572b6ad55a36e5d7134984876d30031ff67; scaling ratio re-verified within budget.", "_rebaselined_2550_instance_model": "PR #2549 (#2550): anonymous class bodies emit synthesized @declaration.class/@declaration.name (Worker$N), an @reference.inherits to the constructed type, and receiver @type-binding.* captures; six new java-* fixtures joined the corpus. Prior f3b4f4b6610e07c3ac90deb1c53d3572b6ad55a36e5d7134984876d30031ff67 -> d79c3b92acfc866094981499b977388ca14f90839bca0c040342ab1cec00aa90; scaling 1.058 < 1.5.", - "_rebaselined_2555_enum_constant_bodies": "PR for #2555: enum constant bodies emit synthesized E$N classes + @reference.inherits to the host enum; anonymous naming follows JLS 13.1 immediately-enclosing-type chains INCLUDING anonymous enclosing types (NestHost$1$1, N$1$1); six new java-* fixtures joined the corpus. Prior d79c3b92acfc866094981499b977388ca14f90839bca0c040342ab1cec00aa90 -> 975b68aaac6d06094260fb0c67f9b1bc03692ba7220669d192aca9dccd5fc0ca; scaling 1.05 < 1.5." + "_rebaselined_2555_enum_constant_bodies": "PR for #2555: enum constant bodies emit synthesized E$N classes + @reference.inherits to the host enum; anonymous naming follows JLS 13.1 immediately-enclosing-type chains INCLUDING anonymous enclosing types (NestHost$1$1, N$1$1); six new java-* fixtures joined the corpus. Prior d79c3b92acfc866094981499b977388ca14f90839bca0c040342ab1cec00aa90 -> 975b68aaac6d06094260fb0c67f9b1bc03692ba7220669d192aca9dccd5fc0ca; scaling 1.05 < 1.5.", + "_rebaselined_2564_record_capture": "PR for #2564: JAVA_QUERIES gained a (record_declaration name: (identifier) @name) @definition.record capture, previously entirely missing (record_declaration had no structure-phase capture at all, unlike class/interface/enum) - a record's methods existed as ownerless Method nodes with no HAS_METHOD edge. Two new java-* fixtures (java-record-methods, java-new-expr-chain-call) joined the corpus. Prior 975b68aaac6d06094260fb0c67f9b1bc03692ba7220669d192aca9dccd5fc0ca -> 85fc7af9c3c1bceac76cb4f27214410b04967682a2eaa7e468e26efd1f4e2537; scaling 1.059 < 1.5." }, "typescript": { "fingerprint": "3280b13d3f9378ab23eee31c2edc779b5a9ae1e7bb510c23a24855b44406d2f4", diff --git a/gitnexus/src/core/ingestion/languages/java/captures.ts b/gitnexus/src/core/ingestion/languages/java/captures.ts index c4da21212..3adb036c8 100644 --- a/gitnexus/src/core/ingestion/languages/java/captures.ts +++ b/gitnexus/src/core/ingestion/languages/java/captures.ts @@ -154,6 +154,29 @@ export function emitJavaScopeCaptures( continue; } + // Normalize a `new`-expression receiver to its constructed type's simple + // name: `new Local().inner()` binds the WHOLE `object_creation_expression` + // as `@reference.receiver`, so its raw text is `"new Local()"` — a string + // that can never match a scope binding, so the compound-receiver resolver + // silently falls through to name-only fallback resolution and picks the + // wrong same-named method on a collision (#2564). Rewriting the text to + // just `Local` lets Case 2 (class-name / static receiver) in + // receiver-bound-calls.ts resolve it via its normal MRO walk. Mirrors the + // established `normalizePhpReceiver` precedent (php/captures.ts) — a + // language-local capture rewrite, no shared-pipeline change. + if (grouped['@reference.receiver'] !== undefined) { + const receiverNode = nodeIfType(nodeMap['@reference.receiver'], 'object_creation_expression'); + const typeNode = receiverNode?.childForFieldName('type'); + const simpleName = typeNode ? javaBaseSimpleNameOf(typeNode) : undefined; + if (simpleName !== undefined) { + grouped['@reference.receiver'] = syntheticCapture( + '@reference.receiver', + receiverNode!, + simpleName, + ); + } + } + // Filter read.member when it's a child of method_invocation or assignment. // `@reference.read.member` is captured directly on the `field_access` node. if (grouped['@reference.read.member'] !== undefined) { diff --git a/gitnexus/src/core/ingestion/tree-sitter-queries.ts b/gitnexus/src/core/ingestion/tree-sitter-queries.ts index d1b0359aa..3261913ba 100644 --- a/gitnexus/src/core/ingestion/tree-sitter-queries.ts +++ b/gitnexus/src/core/ingestion/tree-sitter-queries.ts @@ -743,10 +743,11 @@ export const PYTHON_QUERIES = ` // Java queries - works with tree-sitter-java export const JAVA_QUERIES = ` -; Classes, Interfaces, Enums, Annotations +; Classes, Interfaces, Enums, Records, Annotations (class_declaration name: (identifier) @name) @definition.class (interface_declaration name: (identifier) @name) @definition.interface (enum_declaration name: (identifier) @name) @definition.enum +(record_declaration name: (identifier) @name) @definition.record (annotation_type_declaration name: (identifier) @name) @definition.annotation ; Anonymous class bodies: new Runnable() { ... } — no @name capture; the diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 51af63b03..0be12b947 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -2904,7 +2904,30 @@ export const queryFTS = async ( }; /** - * Drop an FTS index + * True for the two benign "nothing to drop" `DROP_FTS_INDEX` failures — + * both catalog/binder exceptions, LadybugDB's classes for "this name isn't + * bound to anything right now" (probe-verified end-to-end through + * `dropFTSIndex`'s real `conn.query()` path against @ladybugdb/core + * 0.18.x): the named index was never created (`Binder exception: Table + * doesn't have an index with name .`), or the FTS extension/function + * isn't registered at all (`Catalog exception: function DROP_FTS_INDEX is + * not defined...`). A real engine failure — e.g. the `Runtime exception: + * FTS index '' is inconsistent: ...` class from #2589 — is a + * DIFFERENT exception class (an execution-time failure, not a catalog/bind + * lookup miss), so this returns false for it. Anchored to the START of the + * message (not a bare substring search): every probed LadybugDB error leads + * with its exception class, and anchoring means a future message that merely + * mentions "Binder exception" or "Catalog exception" further in in the body + * of an otherwise-genuine failure can't be misclassified as benign. Pure + * string logic so it is unit-testable without a native LadybugDB connection. + */ +export const isBenignDropFtsIndexError = (message: string): boolean => + message.startsWith('Binder exception:') || message.startsWith('Catalog exception:'); + +/** + * Drop an FTS index. Tolerates only {@link isBenignDropFtsIndexError} — + * anything else rethrows instead of being silently masked, which previously + * let a corrupted index persist across analyze runs undetected. */ export const dropFTSIndex = async (tableName: string, indexName: string): Promise => { if (!conn) { @@ -2913,8 +2936,11 @@ export const dropFTSIndex = async (tableName: string, indexName: string): Promis try { await queryAndDrain(conn, `CALL DROP_FTS_INDEX('${tableName}', '${indexName}')`); - } catch { - // Index may not exist + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); + if (!isBenignDropFtsIndexError(msg)) { + throw e; + } } finally { ensuredFTSIndexes.delete(ftsIndexKey(tableName, indexName)); } diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index c71a716d8..9d5e7a811 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -39,6 +39,7 @@ import { escapeCypherString } from './lbug/cypher-escape.js'; import { buildSearchIndexesOrDegrade, createSearchFTSIndexes, + dropSearchFTSIndexes, initialiseSearchFTSStemmer, verifySearchFTSIndexes, } from './search/fts-indexes.js'; @@ -1663,7 +1664,20 @@ export async function runFullAnalysis( progress('lbug', pct, msg); }); } else { - // 1a. Remove the write set's existing rows — batched (#2409): one + // 1a. Drop every FTS index before touching a single row (#2589). + // `deleteNodesForFiles` below DETACH DELETEs rows out of tables + // that otherwise still carry the FTS index built at the end of + // the PREVIOUS analyze run — Phase 3 doesn't drop+rebuild it + // until well after this delete completes. LadybugDB's FTS + // extension is not proven to survive DML against an indexed + // table (its own docs never demonstrate it), and that ordering + // is exactly what produced "FTS index 'file_fts' is + // inconsistent: term is missing during delete". Dropping first + // removes the hazard outright; Phase 3's createSearchFTSIndexes + // rebuilds every index from the final row set regardless, so + // this is a no-op on its own drop step there. + await dropSearchFTSIndexes(); + // 1b. Remove the write set's existing rows — batched (#2409): one // DETACH DELETE per table per 200-file chunk. The former per-file // loop issued a count + delete per table per FILE — ~13k // single-row write transactions on a ~700-file write set — which diff --git a/gitnexus/src/core/search/fts-indexes.ts b/gitnexus/src/core/search/fts-indexes.ts index dfc4f2eeb..1207861b9 100644 --- a/gitnexus/src/core/search/fts-indexes.ts +++ b/gitnexus/src/core/search/fts-indexes.ts @@ -121,6 +121,20 @@ export function getSearchFTSStemmer(): string { return resolvedStemmer ?? resolveFTSStemmer(); } +/** + * Drop every configured FTS index (no-op per index when absent or unloadable + * — `dropFTSIndex` tolerates both). Callable ahead of any DML that mutates an + * FTS-indexed table's rows: LadybugDB's FTS extension is not proven to + * survive a DETACH DELETE against a table that still carries a live index + * from a prior run (#2589) — dropping first removes that hazard entirely, + * regardless of whether it also fixed a specific native inconsistency. + */ +export async function dropSearchFTSIndexes(): Promise { + for (const { table, indexName } of FTS_INDEXES) { + await dropFTSIndex(table, indexName); + } +} + export async function createSearchFTSIndexes( options?: CreateSearchFTSIndexesOptions, ): Promise { diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 568d288bd..e34432a7a 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -429,8 +429,16 @@ export interface RepoMeta { * `E.hook` to `E$1.hook`, and nested-host anonymous names re-key * (`EnumWrap$1` → `EnumWrap$Mode$1`). Same contract as v8: identities move * on unchanged files; force a full re-analyze. + * v10: Java `record_declaration` now emits a first-class `Record` graph node + * (#2564): a record's container node was previously never created (JAVA_QUERIES + * had no capture for it), so its methods existed as ownerless Method nodes + * with no `HAS_METHOD` edge. The incremental write set only covers changed + * files — a top-up against a pre-v10 index would keep silently omitting the + * `Record` node and its `HAS_METHOD` edges for every unchanged record file + * (same v7 contract: new nodes/edges the incremental path would otherwise + * never backfill); force a full re-analyze instead. */ -export const INCREMENTAL_SCHEMA_VERSION = 9; +export const INCREMENTAL_SCHEMA_VERSION = 10; export interface IndexedRepo { repoPath: string; diff --git a/gitnexus/test/fixtures/lang-resolution/java-new-expr-chain-call/LocalChain.java b/gitnexus/test/fixtures/lang-resolution/java-new-expr-chain-call/LocalChain.java new file mode 100644 index 000000000..599f8d5ed --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/java-new-expr-chain-call/LocalChain.java @@ -0,0 +1,12 @@ +package probe; + +public class LocalChain { + void m() { + class Local { + void inner() { + System.out.println("right target"); + } + } + new Local().inner(); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/java-new-expr-chain-call/Other.java b/gitnexus/test/fixtures/lang-resolution/java-new-expr-chain-call/Other.java new file mode 100644 index 000000000..2b722fc3d --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/java-new-expr-chain-call/Other.java @@ -0,0 +1,7 @@ +package probe; + +class Other { + void inner() { + System.out.println("wrong target"); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/java-record-methods/Point.java b/gitnexus/test/fixtures/lang-resolution/java-record-methods/Point.java new file mode 100644 index 000000000..f4471a1e1 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/java-record-methods/Point.java @@ -0,0 +1,11 @@ +package probe; + +public record Point(int x, int y) { + public int sum() { + return x + y; + } + + public int scaled(int factor) { + return sum() * factor; + } +} diff --git a/gitnexus/test/integration/resolvers/java.test.ts b/gitnexus/test/integration/resolvers/java.test.ts index 66bc820dd..c7e0545c0 100644 --- a/gitnexus/test/integration/resolvers/java.test.ts +++ b/gitnexus/test/integration/resolvers/java.test.ts @@ -1174,6 +1174,75 @@ describe('Java chained method call resolution', () => { }); }); +// --------------------------------------------------------------------------- +// Chained call on a new-expression receiver: new Local().inner() +// The receiver of inner() is an object_creation_expression, not a variable. +// Regression test for #2564: without treating `new Local()` as a typed +// receiver, the call falls back to name-only resolution and can pick an +// unrelated same-named method (Other.inner) instead of Local.inner. +// --------------------------------------------------------------------------- + +describe('Java chained call on a new-expression receiver (#2564)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'java-new-expr-chain-call'), () => {}); + }, 60000); + + it('detects LocalChain and Other classes', () => { + const classes = getNodesByLabel(result, 'Class'); + expect(classes).toContain('LocalChain'); + expect(classes).toContain('Other'); + }); + + it('resolves new Local().inner() to the local Local#inner, NOT Other#inner', () => { + const calls = getRelationships(result, 'CALLS'); + const localInner = calls.find( + (c) => + c.target === 'inner' && c.source === 'm' && c.targetFilePath.includes('LocalChain.java'), + ); + const otherInner = calls.find( + (c) => c.target === 'inner' && c.source === 'm' && c.targetFilePath.includes('Other.java'), + ); + expect(localInner).toBeDefined(); + expect(otherInner).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// Java record: container node + HAS_METHOD edges +// Regression test for #2564: JAVA_QUERIES previously had no @definition.record +// capture, so a record never got a Class/Record graph node — its methods +// existed as ownerless orphans with no HAS_METHOD edge. +// --------------------------------------------------------------------------- + +describe('Java record method resolution (#2564)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'java-record-methods'), () => {}); + }, 60000); + + it('detects a Record node for Point', () => { + const records = getNodesByLabel(result, 'Record'); + expect(records).toContain('Point'); + }); + + it('emits HAS_METHOD edges linking sum and scaled to Point', () => { + const hasMethod = getRelationships(result, 'HAS_METHOD'); + const sumEdge = hasMethod.find((e) => e.source === 'Point' && e.target === 'sum'); + const scaledEdge = hasMethod.find((e) => e.source === 'Point' && e.target === 'scaled'); + expect(sumEdge).toBeDefined(); + expect(scaledEdge).toBeDefined(); + }); + + it('resolves scaled() calling sum() via a CALLS edge', () => { + const calls = getRelationships(result, 'CALLS'); + const sumCall = calls.find((c) => c.target === 'sum' && c.source === 'scaled'); + expect(sumCall).toBeDefined(); + }); +}); + // --------------------------------------------------------------------------- // Java 16+ instanceof pattern variable: `if (obj instanceof User user)` // Phase 5.2: extractPatternBinding on instanceof_expression binds user → User. diff --git a/gitnexus/test/unit/call-summary-schema-version.test.ts b/gitnexus/test/unit/call-summary-schema-version.test.ts index 8e7e3b5ce..a8240fa0f 100644 --- a/gitnexus/test/unit/call-summary-schema-version.test.ts +++ b/gitnexus/test/unit/call-summary-schema-version.test.ts @@ -73,8 +73,8 @@ describe('CALL_SUMMARY relation-type exclusion (U-C1)', () => { }); describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { - it('INCREMENTAL_SCHEMA_VERSION is bumped to 9 (Java enum-constant-body + JLS-naming re-index window)', () => { - expect(INCREMENTAL_SCHEMA_VERSION).toBe(9); + it('INCREMENTAL_SCHEMA_VERSION is bumped to 10 (Java record container-node re-index window)', () => { + expect(INCREMENTAL_SCHEMA_VERSION).toBe(10); }); it('a pre-current stamp fails the `=== INCREMENTAL_SCHEMA_VERSION` reuse gate → forces full re-analyze', () => { @@ -108,7 +108,11 @@ describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { // topmost-anchored `EnumWrap$1`-style ids would be stranded alongside // the re-keyed ones on unchanged files → must NOT reuse. expect(passesReuseGate(8)).toBe(false); + // A pre-v10 (v9) index predates the Java record container-node fix + // (#2564) — a record's methods would keep being ownerless Method nodes + // with no HAS_METHOD edge on unchanged files → must NOT reuse. + expect(passesReuseGate(9)).toBe(false); // A current-version stamp passes the gate (incremental top-up eligible). - expect(passesReuseGate(9)).toBe(true); + expect(passesReuseGate(10)).toBe(true); }); }); diff --git a/gitnexus/test/unit/drop-fts-index-error-classification.test.ts b/gitnexus/test/unit/drop-fts-index-error-classification.test.ts new file mode 100644 index 000000000..74b43dc38 --- /dev/null +++ b/gitnexus/test/unit/drop-fts-index-error-classification.test.ts @@ -0,0 +1,66 @@ +/** + * #2589: `dropFTSIndex` must tolerate only benign "nothing to drop" + * `DROP_FTS_INDEX` failures and rethrow everything else — previously it + * swallowed every error unconditionally, which could mask a genuinely + * corrupted FTS index across analyze runs. + * + * `isBenignDropFtsIndexError` is pure string logic (no native connection + * needed), so the classification itself is unit-tested directly, including + * against the exact reported #2589 error text — a native repro of that + * specific engine failure was not achieved during investigation, but the + * classifier's behavior for it is still provable from the message alone. + */ +import { describe, expect, it } from 'vitest'; +import { isBenignDropFtsIndexError, dropFTSIndex } from '../../src/core/lbug/lbug-adapter.js'; +import { withTestLbugDB } from '../helpers/test-indexed-db.js'; + +describe('isBenignDropFtsIndexError', () => { + it('is true for the FTS-extension/function-not-registered catalog error (probe-verified text)', () => { + expect( + isBenignDropFtsIndexError( + "Catalog exception: function DROP_FTS_INDEX is not defined. This function exists in the FTS extension. You can install and load the extension by running 'INSTALL FTS; LOAD EXTENSION FTS;'.", + ), + ).toBe(true); + }); + + it('is true for the index-never-created binder error (probe-verified against the real dropFTSIndex path)', () => { + expect( + isBenignDropFtsIndexError( + "Binder exception: Table File doesn't have an index with name file_fts.", + ), + ).toBe(true); + }); + + it('is false for the #2589 runtime inconsistency error (must surface, not be swallowed)', () => { + expect( + isBenignDropFtsIndexError( + "Runtime exception: FTS index 'file_fts' is inconsistent: term 'wiki' is missing during delete.", + ), + ).toBe(false); + }); + + it('is false for an unrelated failure', () => { + expect(isBenignDropFtsIndexError('Connection Exception: database is closed')).toBe(false); + }); + + it('is false for a genuine failure that merely mentions "Binder exception" mid-message (anchored, not a bare substring match)', () => { + expect( + isBenignDropFtsIndexError( + 'Runtime exception: internal state corrupted while processing Binder exception: recovery failed.', + ), + ).toBe(false); + }); +}); + +withTestLbugDB('drop-fts-index-benign-cases', (handle) => { + describe('dropFTSIndex end-to-end benign cases (#2589)', () => { + it('resolves cleanly when the named index was never created', async () => { + void handle; + const { executeQuery } = await import('../../src/core/lbug/lbug-adapter.js'); + await executeQuery( + `CREATE NODE TABLE IF NOT EXISTS DropProbe (id STRING PRIMARY KEY, content STRING)`, + ); + await expect(dropFTSIndex('DropProbe', 'drop_probe_never_created')).resolves.toBeUndefined(); + }, 120_000); + }); +}); diff --git a/gitnexus/test/unit/incremental-fts-drop-ordering.test.ts b/gitnexus/test/unit/incremental-fts-drop-ordering.test.ts new file mode 100644 index 000000000..e2f5c3c87 --- /dev/null +++ b/gitnexus/test/unit/incremental-fts-drop-ordering.test.ts @@ -0,0 +1,138 @@ +/** + * #2589: the incremental writeback must drop every FTS index BEFORE + * `deleteNodesForFiles` runs its batched DETACH DELETE — not only in + * Phase 3, after the delete already ran against a table still carrying the + * PREVIOUS run's index. This drives the real `runFullAnalysis` incremental + * path (real git repo, real LadybugDB, real FTS extension) and asserts, + * at the moment `deleteNodesForFiles` is invoked, that `SHOW_INDEXES()` + * already reports every FTS index absent — proving the drop-before-delete + * ordering end-to-end rather than only unit-testing the call sequence. + */ +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 { getStoragePaths } from '../../src/storage/repo-manager.js'; +import { FTS_INDEXES } from '../../src/core/search/fts-schema.js'; +import { createTempDir } from '../helpers/test-db.js'; +import { resolveAnalyzeInstallPolicy } from '../../src/core/lbug/extension-loader.js'; + +const ftsMustBeAvailable = process.env.GITNEXUS_REQUIRE_FTS === '1'; + +describe('runFullAnalysis incremental writeback — FTS drop-before-delete ordering (#2589)', () => { + let ftsAvailable = true; + let skipWarned = false; + + beforeAll(async () => { + const lbugAdapter = await import('../../src/core/lbug/lbug-adapter.js'); + // Cheap standalone probe — matches the withTestLbugDB/lbug-vector-extension + // convention of checking availability once, up front, rather than deep + // inside the (expensive) test body. + const probe = await createTempDir('gitnexus-2589-fts-probe-'); + try { + await lbugAdapter.initLbug(probe.dbPath); + ftsAvailable = await lbugAdapter.loadFTSExtension(undefined, { + policy: resolveAnalyzeInstallPolicy(), + }); + } finally { + await lbugAdapter.closeLbug(); + await probe.cleanup(); + } + }, 120_000); + + // Skip VISIBLY (ctx.skip() marks the test as skipped, not passed) when the + // extension is unavailable — silently `return`ing from inside `it()` would + // report a false pass and hide a regression in the drop-before-delete + // ordering in exactly the environments least likely to have a human notice. + beforeEach((ctx) => { + if (!ftsAvailable) { + if (ftsMustBeAvailable) { + throw new Error( + 'GITNEXUS_REQUIRE_FTS=1 but the FTS extension is unavailable — cannot verify the #2589 ordering fix.', + ); + } + if (!skipWarned) { + skipWarned = true; + console.warn( + '[incremental-fts-drop-ordering] Skipping — the LadybugDB FTS extension is unavailable.', + ); + } + ctx.skip(); + } + }); + + afterEach(() => { + vi.doUnmock('../../src/core/lbug/lbug-adapter.js'); + vi.resetModules(); + }); + + it('SHOW_INDEXES() reports every FTS index absent 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-2589-fts-order-'); + try { + // First run: full rebuild, builds every FTS index for real. + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + + // runFullAnalysis closes its own connection on return — open a fresh + // one just to probe SHOW_INDEXES(), then close it before the second + // run opens its own (LadybugDB is single-writer/single-connection). + const { lbugPath } = getStoragePaths(repo.dbPath); + await lbugAdapter.initLbug(lbugPath); + const showIndexNames = async (): Promise => { + const rows = (await lbugAdapter.executeQuery('CALL SHOW_INDEXES() RETURN *')) as Array< + Record + >; + return rows.map((r) => r.index_name).filter((n): n is string => typeof n === 'string'); + }; + const beforeChange = await showIndexNames(); + await lbugAdapter.closeLbug(); + + // Hard assertion, not a soft skip: the beforeEach gate already proved + // the extension loads, so every index failing to build here is a real + // bug in the full-rebuild FTS phase, not an environment gap. + for (const { indexName } of FTS_INDEXES) { + expect(beforeChange).toContain(indexName); + } + + // Spy on the real deleteNodesForFiles, recording the FTS index list at + // the exact moment it's invoked (before it does anything), then + // delegating to the real implementation so the run completes normally. + let indexNamesAtDeleteTime: string[] | undefined; + const originalDeleteNodesForFiles = lbugAdapter.deleteNodesForFiles; + vi.spyOn(lbugAdapter, 'deleteNodesForFiles').mockImplementation(async (filePaths, opts) => { + indexNamesAtDeleteTime = await showIndexNames(); + return originalDeleteNodesForFiles(filePaths, opts); + }); + + // Small change to a single file — stays well under the escalation + // threshold (50 files) on this 7-file mini-repo, so it takes the + // non-escalated (surgical) incremental branch this test targets. + const handlerPath = path.join(repo.dbPath, 'src', 'handler.ts'); + await writeFile( + handlerPath, + (await readFile(handlerPath, 'utf-8')) + '\n// #2589 ordering-test touch\n', + 'utf-8', + ); + execSync('git -c user.name=test -c user.email=t@t -c commit.gpgsign=false add -A', { + cwd: repo.dbPath, + stdio: 'pipe', + }); + execSync( + 'git -c user.name=test -c user.email=t@t -c commit.gpgsign=false commit -q -m "#2589 ordering touch"', + { cwd: repo.dbPath, stdio: 'pipe' }, + ); + + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + + expect(indexNamesAtDeleteTime).toBeDefined(); + for (const { indexName } of FTS_INDEXES) { + expect(indexNamesAtDeleteTime).not.toContain(indexName); + } + } finally { + await repo.cleanup(); + } + }, 300_000); +});