From 288b96f3e5be6dd83105d7393e9888cc23ab191e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Tue, 9 Jun 2026 08:46:46 +0100 Subject: [PATCH 1/2] fix: batch query enrichment, bake FTS extension into CLI image, add FTS memory repro (#2108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(query): batch per-symbol process/cohesion/content lookups (N+1 -> 2-3) Port of the local-backend query-batching from gitnexus-enterprise PR #222 into the OSS local MCP backend. The query tool traced each matched symbol to its processes + cohesion (+ content) with up to 3N sequential pool round-trips; batch them into 2-3 'WHERE n.id IN $nodeIds' queries keyed back to each symbol by a prepended 'n.id AS nodeId' column. Output is identical: the aggregation loop is unchanged, iterates merged in the same order, and reads pre-fetched maps instead of issuing a query per symbol. Adaptations over a blind cherry-pick (would otherwise change output): - per-nodeId first-row community pick replaces the per-symbol LIMIT 1, so each symbol keeps its own community (not one for the whole batch); - batched rows regrouped to the originating merged item by nodeId so the JS-side RRF item.score still drives process ranking; - positional fallbacks shift +1 (process row[1..6], cohesion [1]/[2], content [1]); CodeRelation{type:...} relation form kept; IN-list chunked at 100 like the impact path. Adds a regression test asserting per-node community/content association (func:login keeps comm:auth; func:validate inherits no community). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(docker): bake LadybugDB FTS extension into the CLI/serve image The container runs `serve` under the default `load-only` extension policy (the read pool pins {policy:'load-only'}), so a runtime LOAD EXTENSION fts never INSTALLs. Dockerfile.cli copied the extension installer but never ran it, so the runtime user's HOME had no FTS extension: keyword search silently degraded (no FTS indexes written, ranking falls back to vector-only with only a warning field). Same class of footgun fixed for the Hub image in gitnexus-enterprise PR #222. Run install-duckdb-extension.mjs as the `node` user with the runtime HOME so INSTALL fts materializes the extension under $HOME/.lbdb/extension where the runtime LOAD resolves it offline. Pin ENV HOME=/home/node because Docker does not derive HOME from USER — without it the build-install and runtime-load would resolve different paths. Verified locally: INSTALL lands in $HOME/.lbdb/extension/0.17.0 and a fresh offline load-only `LOAD EXTENSION fts` resolves it. Dockerfile.web is unaffected (static frontend, no @ladybugdb backend). Co-Authored-By: Claude Opus 4.8 (1M context) * test(lbug): FTS evict->reload RSS repro + inert pool RSS tracing Settles the gitnexus-enterprise PR #222 root-cause hypothesis for OSS: does re-running LOAD EXTENSION fts on every pool evict->reload strand the native FTS arena (unbounded RSS growth in long-lived MCP serve), or does db.close() reclaim it (bounded by MAX_POOL_SIZE)? Static read could not decide — the native lbugjs.node binary documents no close->extension-unload contract. Adds gitnexus/scripts/bench/fts-evict-reload-rss.mjs: a NATIVE mode that reproduces the exact native sequence doInitLbug()+closeOne() perform (open Database -> Connection -> LOAD EXTENSION fts -> QUERY_FTS_INDEX -> close) across K self-built FTS fixtures, and a --via-pool mode that drives the real compiled pool (initLbug/executeParameterized/closeLbug) against an existing analyzed repo. Plus a behavior-neutral GITNEXUS_POOL_RSS_TRACE=1 stderr trace on pool init/close (stdout reserved for MCP JSON-RPC; single env read when disabled). RESULT (native, 24 and 40 cycles x 6 fixtures, --expose-gc): PLATEAU. RSS warms up to ~400 MB then flattens (40-cycle: +36 MB over cycles 1-10, +3 MB over 30-40; decelerating), not the linear climb a per-reload arena leak would produce (240 reloads x stranded arena = multi-GB). db.close() reclaims the FTS arena. The unbounded-leak hypothesis is NOT reproduced for the OSS path: the pool's LRU eviction + close-on-evict BOUNDS the footprint, which is exactly the protection the enterprise Hub supervisor lacked (it opened bridge DBs in-process without eviction -> 15 GB). => plan U4 (worker/process isolation) is NOT justified by this evidence; U1 + U2 are the only OSS-shared changes. Caveat: small fixtures + awaited close; a --via-pool run against a large analyzed repo over a long session is the production-faithful follow-up (instrumentation is in place for it). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(review): apply ce-code-review autofix feedback (#222 migration) Adversarial review found the U3 bench PLATEAU->no-leak conclusion was over-claimed from a 600-row fixture: a size-proportional FTS-arena leak would be sub-threshold at that scale. Strengthen the bench and make its verdict honest: - scale the fixture (--rows, UNWIND batch insert), probe ALL 5 FTS indexes in --via-pool (not 2 of 5), add a --no-await-close variant (the pool fire-and-forget close shape), and replace the absolute-delta gate with a SLOPE-DECELERATION 3-way verdict (PLATEAU / CLIMB / INCONCLUSIVE) plus step-discontinuity detection. At production-representative scale the synthetic runs are noisy/INCONCLUSIVE (deceleration argues against an UNBOUNDED leak but does not prove bounded), so plan U4 stays GATED on a --via-pool run against a real large analyzed repo -- not closed. - Dockerfile.cli: source the scratch-DB size from ENV GITNEXUS_LBUG_MAX_DB_SIZE (single source of truth) and add a build-time verify-only LOAD gate that fails the build on a HOME/extension-dir mismatch instead of silently degrading runtime keyword search. - install-duckdb-extension.mjs: additive verify-only mode (LOAD-only in a fresh process) + robust size parse; back-compatible with the runtime positional-size caller (validated). - tests: wire func:validate into a second process (proc:beta-flow) so the batched STEP_IN_PROCESS row[1..6] positional shift is exercised by a genuine multi-process symbol, and assert process ranking. No blast radius (75 seed-consuming tests pass). - pool-adapter.ts: trim the traceRss narrated-code comment (DoD 2.3). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(bench): classify a sustained sub-floor RSS slope as INCONCLUSIVE, not PLATEAU Tri-review P2: the FTS evict->reload verdict short-circuited to PLATEAU whenever secondHalfSlope < SUSTAIN_FLOOR, BEFORE the deceleration check — so a sustained (non-decelerating) linear leak below 0.5 MB/cycle was labeled PLATEAU ("no leak"), the label that would wrongly close plan U4. Extract median/slopeMbPerCycle/classifyVerdict into a pure, side-effect-free fts-rss-verdict.mjs (zero imports) so it is unit-testable without loading the native addon or running the bench, and fix the classifier: - epsilon-first gate: a truly flat tail (< 0.1 MB/cycle) is PLATEAU regardless of decelRatio (guards against over-correcting a real negative into INCONCLUSIVE); - a sustained sub-floor positive slope (>= epsilon, < floor, decelRatio >= 0.6) is INCONCLUSIVE — a slow creep RSS cannot distinguish from noise at this scale, so the honest label is "not resolved", never a clean PLATEAU; - the noise floor now scales with the WORKING-SET growth (peak-baseline), not the pre-DB baseline RSS (which is interpreter/addon overhead, larger in --via-pool mode, and would inflate the floor and HIDE leaks). Reconcile the stale "per-row-relative delta floor" docstring; add floor + decelRatio to the MACHINE line. New fts-rss-verdict.test.ts pins all label boundaries (flat->PLATEAU, sustained-sub-floor->INCONCLUSIVE, decelerated->PLATEAU, sustained-linear->CLIMB, step->INCONCLUSIVE, working-set floor, no import side effects). U1 does NOT add detection power for sub-floor leaks (RSS cannot attribute that magnitude) — it stops the false PLATEAU and routes that regime to the --via-pool run. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(query): signal partial/warning on a real enrichment failure (not benign missing-table) Tri-review P2: when a batched enrichment query (process/cohesion/content) threw, it was caught + logged and the chunk's symbols silently fell back to `definitions` with no signal — the caller could not tell "genuinely standalone" from "enrichment failed". Track an `enrichmentDegraded` flag in the three enrichment catch blocks and, at response build, compose a single `warning` (FTS-missing and/or the enrichment message, so neither overwrites the other) plus `partial: true`. Both fields are omitted on the clean path, so the success-path response shape is byte-identical. Crucially, the flag fires ONLY for a REAL failure (timeout / lock / native fault), NOT the benign "no Process/Community table" prepare error — a repo analyzed without processes/communities is a normal config, and firing `partial` on every such query would desensitize callers (isBenignMissingTableError gates it). New unit test test/unit/query-degraded-signal.test.ts (vi.mock pool-adapter, override hybrid search to feed one matched symbol, route STEP_IN_PROCESS -> throw): real failure -> warning+partial+symbol still returned; benign missing-table -> no signal; FTS-missing + enrichment failure -> both messages in one warning. Plus a success-path no-warning/no-partial assertion in the calltool integration test. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- Dockerfile.cli | 22 ++ .../scripts/bench/fts-evict-reload-rss.mjs | 374 ++++++++++++++++++ gitnexus/scripts/bench/fts-rss-verdict.mjs | 105 +++++ gitnexus/scripts/install-duckdb-extension.mjs | 28 +- gitnexus/src/core/lbug/pool-adapter.ts | 17 + gitnexus/src/mcp/local/local-backend.ts | 216 +++++++--- gitnexus/test/fixtures/local-backend-seed.ts | 6 + .../local-backend-calltool.test.ts | 72 ++++ gitnexus/test/unit/fts-rss-verdict.test.ts | 76 ++++ .../test/unit/query-degraded-signal.test.ts | 120 ++++++ 10 files changed, 965 insertions(+), 71 deletions(-) create mode 100644 gitnexus/scripts/bench/fts-evict-reload-rss.mjs create mode 100644 gitnexus/scripts/bench/fts-rss-verdict.mjs create mode 100644 gitnexus/test/unit/fts-rss-verdict.test.ts create mode 100644 gitnexus/test/unit/query-degraded-signal.test.ts diff --git a/Dockerfile.cli b/Dockerfile.cli index 3275c8f7e..cfba9bac4 100644 --- a/Dockerfile.cli +++ b/Dockerfile.cli @@ -67,6 +67,28 @@ COPY --from=builder --chown=node:node /app/gitnexus/vendor ./gitnexus/vendor # unreachable from $PATH. RUN ln -s /app/gitnexus/dist/cli/index.js /usr/local/bin/gitnexus +# Bake the LadybugDB FTS extension into the image so BM25 keyword search works +# at runtime. The server runs the default `load-only` extension policy (the read +# pool pins `{ policy: 'load-only' }`), so a runtime `LOAD EXTENSION fts` never +# INSTALLs — the extension must already exist in the runtime user's HOME +# extension dir, or every keyword search silently degrades (no FTS indexes are +# written and ranking falls back to vector-only with only a `warning` field). +# Run the installer as the `node` user with the SAME HOME the server runs under, +# so `INSTALL fts` materializes the extension under `$HOME/.lbdb/extension` where +# the runtime `LOAD` resolves it offline. `ENV HOME` is pinned because Docker +# does not derive HOME from `USER`, so without it build-install and runtime-load +# would resolve different paths. Requires network egress for the one-time +# INSTALL; the build fails loudly if it cannot fetch the extension. The DB-size +# default comes from GITNEXUS_LBUG_MAX_DB_SIZE (single source of truth, matches +# the runtime) — it only sizes the throwaway scratch DB used to run INSTALL. +# The second `--verify-only` step re-LOADs the extension in a FRESH process +# under the same HOME, so a HOME/extension-dir mismatch fails the build here +# rather than silently degrading keyword search to vector-only at runtime. +ENV HOME=/home/node \ + GITNEXUS_LBUG_MAX_DB_SIZE=17179869184 +RUN su node -s /bin/sh -c "HOME=/home/node node /app/gitnexus/scripts/install-duckdb-extension.mjs fts" \ + && su node -s /bin/sh -c "HOME=/home/node node /app/gitnexus/scripts/install-duckdb-extension.mjs fts --verify-only" + USER node # The web UI defaults to http://localhost:4747 - keep that contract. diff --git a/gitnexus/scripts/bench/fts-evict-reload-rss.mjs b/gitnexus/scripts/bench/fts-evict-reload-rss.mjs new file mode 100644 index 000000000..30d3f359a --- /dev/null +++ b/gitnexus/scripts/bench/fts-evict-reload-rss.mjs @@ -0,0 +1,374 @@ +#!/usr/bin/env node +// FTS evict→reload RSS repro (gitnexus-enterprise PR #222 / local U3). +// +// Settles ONE empirical question that no static read can answer: when a +// LadybugDB database that has `LOAD EXTENSION fts` applied is closed and a +// fresh one is opened + re-LOADed (the pool's evict→reload cycle), does the +// native FTS arena get reclaimed by `db.close()` — or is it stranded, so RSS +// climbs without bound over a long-lived MCP `serve` session? +// +// • PLATEAU across cycles → db.close() reclaims the FTS arena; the OSS pool's +// footprint is bounded by MAX_POOL_SIZE (~5 live arenas). No unbounded leak; +// the #222 worker-isolation rewrite (plan U4) is NOT justified for OSS. +// • MONOTONIC CLIMB → the FTS arena is stranded per reopen; the user's +// hypothesis holds and U4 (route FTS reads through a reclaimable worker) is +// justified. +// +// SCOPE OF THE VERDICT (read before citing it). A per-reload FTS-arena leak +// would be PROPORTIONAL to the index size. A small fixture therefore produces a +// small per-cycle increment that an absolute threshold can read as PLATEAU even +// when a production-scale graph would leak visibly. So: +// - `--rows` controls fixture size; run it LARGE (tens of thousands) before +// concluding "no leak". The default is deliberately not tiny. +// - The verdict (in fts-rss-verdict.mjs) keys on slope DECELERATION, not total +// delta, with a noise floor that scales with the working-set growth +// (peak−baseline) so sensitivity tracks fixture/arena size — NOT the pre-DB +// baseline RSS. A sustained sub-floor positive slope is INCONCLUSIVE (a slow +// creep RSS can't distinguish from noise), never a clean PLATEAU. +// - The PLATEAU verdict is only valid for the corpus size it was run at; the +// output states that size. The production-faithful confirmation is a +// `--via-pool` run against a real large analyzed repo over a long session. +// +// Two modes: +// (default) NATIVE — reproduces the native sequence doInitLbug()+closeOne() +// perform (open Database → new Connection → LOAD EXTENSION fts → +// QUERY_FTS_INDEX → close), against K self-built FTS fixtures, with no +// gitnexus build required. `--no-await-close` mirrors the pool's +// fire-and-forget close instead of awaiting (the production close shape). +// --via-pool — drives the REAL gitnexus pool from compiled dist +// (initLbug → executeParameterized → closeLbug) against an existing analyzed +// repo, exercising the production path + the GITNEXUS_POOL_RSS_TRACE +// instrumentation. Probes ALL FTS indexes the repo has. Forces an explicit +// close+reinit each cycle. Run `node scripts/build.js` first so the dist +// reflects the current pool-adapter (incl. the RSS trace). +// +// Run with --expose-gc so RSS excludes V8-heap noise: +// node --expose-gc gitnexus/scripts/bench/fts-evict-reload-rss.mjs +// node --expose-gc gitnexus/scripts/bench/fts-evict-reload-rss.mjs --rows 40000 --cycles 30 +// GITNEXUS_POOL_RSS_TRACE=1 node --expose-gc \ +// gitnexus/scripts/bench/fts-evict-reload-rss.mjs --via-pool /path/to/repo/.gitnexus/lbug +// +// Flags by mode: --rows/--repos/--read-write/--no-await-close apply to NATIVE +// only; --cycles applies to both. VIA-POOL warns when a NATIVE-only flag is set. +// +// Memory benches are noisy. Default is 24 cycles; trust the TREND (slope / +// first-third vs last-third), never a single delta. A flat trend at a LARGE +// fixture is a real NEGATIVE result (no unbounded leak), not a failed run. + +import { createRequire } from 'node:module'; +import os from 'node:os'; +import path from 'node:path'; +import fs from 'node:fs'; +// Pure verdict classifier (median, slopeMbPerCycle, classifyVerdict) lives in a +// side-effect-free sibling module so it is unit-testable without loading the +// native addon or running this bench. See fts-rss-verdict.mjs. +import { classifyVerdict, median, slopeMbPerCycle } from './fts-rss-verdict.mjs'; + +const require = createRequire(import.meta.url); +const lbugModule = require('@ladybugdb/core'); +const lbug = lbugModule.default ?? lbugModule; + +const LBUG_MAX_DB_SIZE = 16 * 1024 * 1024 * 1024; + +// ── args ────────────────────────────────────────────────────────────────── +function argVal(flag, dflt) { + const i = process.argv.indexOf(flag); + return i >= 0 && process.argv[i + 1] ? process.argv[i + 1] : dflt; +} +const CYCLES = Math.max(6, parseInt(argVal('--cycles', '24'), 10) || 24); +const REPOS = Math.max(1, parseInt(argVal('--repos', '6'), 10) || 6); // >5 mirrors LRU thrash +// Fixture size. Default is large enough that a size-proportional leak would be +// visible across cycles; raise it further before trusting a PLATEAU verdict. +const ROWS = Math.max(100, parseInt(argVal('--rows', '8000'), 10) || 8000); +const VIA_POOL = argVal('--via-pool', null); +const READONLY = !process.argv.includes('--read-write'); +const AWAIT_CLOSE = !process.argv.includes('--no-await-close'); + +if (VIA_POOL) { + // These flags are consumed only by NATIVE mode; warn rather than ignore + // silently so a VIA-POOL run is not misread as honoring them. + const ignored = ['--rows', '--repos', '--read-write', '--no-await-close'].filter((f) => + process.argv.includes(f), + ); + if (ignored.length) { + console.error( + `[fts-rss] NOTE: ${ignored.join(', ')} apply to NATIVE mode only; ignored in --via-pool.`, + ); + } +} + +if (typeof global.gc !== 'function') { + console.error( + '[fts-rss] WARNING: run with --expose-gc for clean RSS samples ' + + '(`node --expose-gc `). Continuing without forced GC — results are noisier.', + ); +} + +const gc = () => { + if (typeof global.gc === 'function') { + global.gc(); + global.gc(); + } +}; +const rssMb = () => Math.round(process.memoryUsage().rss / (1024 * 1024)); +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); + +// ── fixture: a minimal FTS-bearing .lbug ──────────────────────────────────── +const WORDS = [ + 'login auth session token user password validate verify credential', + 'parse tree syntax node grammar lexer token ast traversal visitor', + 'graph query cypher match relation node edge pattern aggregate index', + 'memory pool buffer arena allocate reclaim evict cache resident heap', + 'search rank score bm25 fts index stem porter keyword document corpus', + 'worker fork process spawn kill reclaim isolate native binding addon', +]; + +function buildFixture(dir) { + fs.mkdirSync(dir, { recursive: true }); + const dbPath = path.join(dir, 'fixture.lbug'); + const db = new lbug.Database(dbPath, 0, false, false, LBUG_MAX_DB_SIZE); + const conn = new lbug.Connection(db); + return (async () => { + await conn.query('LOAD EXTENSION fts'); + await conn.query( + 'CREATE NODE TABLE Doc(id STRING, name STRING, content STRING, PRIMARY KEY(id))', + ); + // Batch-insert via UNWIND so large fixtures (`--rows`) build in seconds + // instead of one round-trip per row. The fixture size drives the per-arena + // FTS allocation, which is what makes a size-proportional leak observable. + const rows = []; + for (let i = 0; i < ROWS; i++) { + const w = WORDS[i % WORDS.length]; + const name = `sym_${i}`; + const content = `${w} ${name} block number ${i} ${WORDS[(i + 3) % WORDS.length]}`; + rows.push({ id: `doc:${i}`, name, content }); + } + const INSERT_CHUNK = 2000; + for (let i = 0; i < rows.length; i += INSERT_CHUNK) { + const chunk = rows.slice(i, i + INSERT_CHUNK); + const stmt = await conn.prepare( + 'UNWIND $rows AS r CREATE (:Doc {id: r.id, name: r.name, content: r.content})', + ); + await conn.execute(stmt, { rows: chunk }); + } + await conn.query( + "CALL CREATE_FTS_INDEX('Doc', 'doc_fts', ['name', 'content'], stemmer := 'porter')", + ); + await conn.close(); + await db.close(); + return dbPath; + })(); +} + +const QUERIES = ['login token', 'parse node', 'memory arena', 'search index', 'worker reclaim']; + +// ── NATIVE mode ───────────────────────────────────────────────────────────── +async function runNative() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'fts-rss-')); + console.error( + `[fts-rss] NATIVE: ${REPOS} fixtures × ${ROWS} rows × ${CYCLES} cycles ` + + `(readOnly=${READONLY}, awaitClose=${AWAIT_CLOSE})`, + ); + console.error(`[fts-rss] building ${REPOS} FTS fixture(s) under ${root} …`); + + const srcDb = await buildFixture(path.join(root, 'src')); + const repoPaths = []; + for (let k = 0; k < REPOS; k++) { + const dst = path.join(root, `repo-${k}`); + fs.cpSync(path.dirname(srcDb), dst, { recursive: true }); + repoPaths.push(path.join(dst, 'fixture.lbug')); + } + + // Mirror the pool's evict→reload: each visit opens a FRESH Database, makes a + // Connection, LOADs fts, runs an FTS query, then closes — no caching, so every + // visit is a reload. K>5 amplifies the LRU-thrash signal the pool would see. + const series = []; + gc(); + await sleep(50); + const baseline = rssMb(); + console.error(`[fts-rss] baseline RSS=${baseline}MB`); + + for (let cycle = 0; cycle < CYCLES; cycle++) { + for (let k = 0; k < REPOS; k++) { + const db = new lbug.Database(repoPaths[k], 0, false, READONLY, LBUG_MAX_DB_SIZE); + const conn = new lbug.Connection(db); + try { + await conn.query('LOAD EXTENSION fts'); // the per-reload re-LOAD under test + const q = QUERIES[(cycle + k) % QUERIES.length]; + const res = await conn.query( + `CALL QUERY_FTS_INDEX('Doc', 'doc_fts', '${q}') RETURN node.id AS id, score ORDER BY score DESC LIMIT 20`, + ); + // Drain so the query actually materializes results. + if (res && typeof res.getAll === 'function') await res.getAll(); + } catch (e) { + console.error(`[fts-rss] query error (cycle ${cycle}, repo ${k}): ${e?.message || e}`); + } finally { + // AWAIT_CLOSE (default) is the best case for reclamation. --no-await-close + // mirrors the pool's fire-and-forget close (closeOne: db.close().catch()) + // so a leak that only manifests without awaiting is not hidden. + if (AWAIT_CLOSE) { + try { + await conn.close(); + await db.close(); + } catch { + /* ignore */ + } + } else { + conn.close().catch(() => {}); + db.close().catch(() => {}); + } + } + } + gc(); + // Longer settle when not awaiting close, so fire-and-forget native teardown + // has a chance to complete before the RSS sample (avoids a false PLATEAU). + await sleep(AWAIT_CLOSE ? 20 : 200); + const rss = rssMb(); + series.push(rss); + console.error(`[fts-rss] cycle ${String(cycle + 1).padStart(3)}/${CYCLES} rssMB=${rss}`); + } + + fs.rmSync(root, { recursive: true, force: true }); + return { baseline, series, corpus: `${REPOS}×${ROWS} rows, native, awaitClose=${AWAIT_CLOSE}` }; +} + +// ── VIA-POOL mode (real gitnexus pool from compiled dist) ─────────────────── +async function runViaPool(lbugPath) { + if (!fs.existsSync(lbugPath)) { + console.error(`[fts-rss] --via-pool path not found: ${lbugPath}`); + process.exit(2); + } + // Compiled dist is required (the pool pulls the native addon + many modules). + const distUrl = new URL('../../dist/core/lbug/pool-adapter.js', import.meta.url); + let pool; + try { + pool = await import(distUrl.href); + } catch (e) { + console.error( + `[fts-rss] could not import compiled pool-adapter (${e?.message}). ` + + `Run \`node scripts/build.js\` first, or use NATIVE mode.`, + ); + process.exit(2); + } + const { initLbug, executeParameterized, closeLbug } = pool; + console.error( + `[fts-rss] VIA-POOL on ${lbugPath} × ${CYCLES} cycles ` + + `(explicit closeLbug+initLbug per cycle = forced evict→reload)`, + ); + + // Probe ALL FTS indexes the analyzed graph carries (mirrors fts-schema.ts + // FTS_INDEXES) so the per-cycle FTS arena load matches production, not a + // 2-of-5 subset that would understate it. + const FTS_INDEXES = [ + { table: 'File', indexName: 'file_fts' }, + { table: 'Function', indexName: 'function_fts' }, + { table: 'Class', indexName: 'class_fts' }, + { table: 'Method', indexName: 'method_fts' }, + { table: 'Interface', indexName: 'interface_fts' }, + ]; + + const series = []; + gc(); + const baseline = rssMb(); + console.error(`[fts-rss] baseline RSS=${baseline}MB`); + + for (let cycle = 0; cycle < CYCLES; cycle++) { + try { + await initLbug(lbugPath, lbugPath); + const q = QUERIES[cycle % QUERIES.length]; + for (const { table, indexName } of FTS_INDEXES) { + await executeParameterized( + lbugPath, + `CALL QUERY_FTS_INDEX('${table}', '${indexName}', $q) RETURN node.id AS id, score ORDER BY score DESC LIMIT 20`, + { q }, + ).catch(() => []); // index may not exist for this graph — that's fine + } + await closeLbug(lbugPath); // force eviction → next cycle reopens + re-LOADs fts + } catch (e) { + console.error(`[fts-rss] pool cycle ${cycle} error: ${e?.message || e}`); + } + gc(); + // closeLbug fires a fire-and-forget native close (pool closeOne: + // db.close().catch()), so settle longer than NATIVE's awaited close to let + // native teardown finish before sampling — else a real leak reads PLATEAU. + await sleep(200); + const rss = rssMb(); + series.push(rss); + console.error(`[fts-rss] cycle ${String(cycle + 1).padStart(3)}/${CYCLES} rssMB=${rss}`); + } + await closeLbug().catch(() => {}); + return { baseline, series, corpus: `via-pool ${path.basename(path.dirname(lbugPath))}` }; +} + +// ── verdict ───────────────────────────────────────────────────────────────── +function verdict({ baseline, series, corpus }) { + const third = Math.max(1, Math.floor(series.length / 3)); + const firstMed = median(series.slice(0, third)); + const lastMed = median(series.slice(-third)); + const delta = lastMed - firstMed; + const slope = slopeMbPerCycle(series); + + // All label logic lives in the pure, unit-tested classifier (fts-rss-verdict.mjs): + // epsilon-first flat→PLATEAU, decelerated→PLATEAU, sustained-sub-floor→INCONCLUSIVE, + // ≥floor sustained→CLIMB, step→INCONCLUSIVE; floor scales with the working-set + // growth (peak−baseline), not the pre-DB baseline RSS. + const { + verdict: label, + firstHalfSlope, + secondHalfSlope, + decelRatio, + floor, + stepDiscontinuity, + maxJump, + peak, + } = classifyVerdict(series, baseline); + + console.log('\n==================== FTS evict→reload RSS verdict ===================='); + console.log(`corpus: ${corpus}`); + console.log(`samples (MB): ${series.join(' ')}`); + console.log( + `baseline=${baseline} firstThirdMed=${firstMed} lastThirdMed=${lastMed} delta=${delta}MB ` + + `peak=${peak} overallSlope=${slope.toFixed(2)} firstHalfSlope=${firstHalfSlope.toFixed(2)} ` + + `secondHalfSlope=${secondHalfSlope.toFixed(2)}MB/cycle floor=${floor.toFixed(2)} decelRatio=${decelRatio.toFixed(2)} ` + + `maxJump=${maxJump}MB step=${stepDiscontinuity} cycles=${series.length}`, + ); + if (label === 'CLIMB') { + console.log( + 'VERDICT: CLIMB — the per-cycle increment is SUSTAINED (second-half slope ≈ first-half),\n' + + ' i.e. RSS rises ~linearly with no decay. The native FTS arena is NOT reclaimed\n' + + ' by db.close(); the leak is real over a long-lived session.\n' + + ' → plan U4 (worker/process isolation of the FTS read path) is JUSTIFIED.', + ); + } else if (label === 'PLATEAU') { + console.log( + `VERDICT: PLATEAU at this corpus (${corpus}) — the per-cycle increment DECAYS to flat\n` + + ' (second-half slope below the noise floor). db.close() reclaims the FTS arena;\n' + + ' footprint is bounded (and the pool further caps it at MAX_POOL_SIZE). No\n' + + ' unbounded leak. Caveat: synthetic fixture — confirm with a --via-pool run\n' + + ' against a real large analyzed repo before fully closing plan U4.', + ); + } else { + console.log( + `VERDICT: INCONCLUSIVE at this corpus (${corpus}) — the run is noisy (step discontinuity)\n` + + ' or still decelerating without reaching flat, so neither a clean PLATEAU nor a\n' + + ' sustained linear CLIMB can be asserted. NATIVE synthetic runs do not resolve\n' + + ' this reliably at scale. The definitive test is a --via-pool run against a real\n' + + ' large analyzed repo over many cycles (with GITNEXUS_POOL_RSS_TRACE=1). Plan U4\n' + + ' stays GATED — neither closed nor built on this evidence.', + ); + } + console.log( + `MACHINE: ${JSON.stringify({ mode: VIA_POOL ? 'via-pool' : 'native', corpus, baseline, firstMed, lastMed, delta, overallSlope: Number(slope.toFixed(3)), firstHalfSlope: Number(firstHalfSlope.toFixed(3)), secondHalfSlope: Number(secondHalfSlope.toFixed(3)), floor: Number(floor.toFixed(3)), decelRatio: Number(decelRatio.toFixed(3)), maxJump, stepDiscontinuity, peak, cycles: series.length, verdict: label })}`, + ); + console.log('=====================================================================\n'); +} + +// ── main ──────────────────────────────────────────────────────────────────── +(async () => { + const result = VIA_POOL ? await runViaPool(VIA_POOL) : await runNative(); + verdict(result); + process.exit(0); +})().catch((e) => { + console.error('[fts-rss] fatal:', e?.stack || e); + process.exit(1); +}); diff --git a/gitnexus/scripts/bench/fts-rss-verdict.mjs b/gitnexus/scripts/bench/fts-rss-verdict.mjs new file mode 100644 index 000000000..b198234ee --- /dev/null +++ b/gitnexus/scripts/bench/fts-rss-verdict.mjs @@ -0,0 +1,105 @@ +// Pure, side-effect-free verdict classifier for the FTS evict→reload RSS bench +// (fts-evict-reload-rss.mjs). Extracted so it can be unit-tested WITHOUT importing +// the native LadybugDB addon or running the bench — this module has zero imports +// and zero module-scope side effects. Do not add imports or top-level statements. +// +// The discriminant between a real leak and allocator warmup is SLOPE DECELERATION, +// not total delta. A true per-reload leak (stranded FTS arena) rises ~linearly: +// the second-half slope stays ≈ the first-half slope. Allocator working-set warmup +// rises then flattens: the second-half slope decays to a fraction of the first. +// +// Thresholds: +// EPSILON (~0.1 MB/cycle) — below this the tail is effectively flat (no leak). +// SUSTAIN_FLOOR (0.5 MB/cycle) — the base noise floor. +// The floor SCALES with the working-set growth (peak − baseline), NOT the pre-DB +// `baseline` RSS: baseline is interpreter/addon overhead (and is LARGER in +// --via-pool mode), so a baseline-keyed floor would inflate and HIDE leaks. A +// bigger fixture has a bigger arena and bigger per-cycle noise, so the floor +// rises with the working set: floor = SUSTAIN_FLOOR · max(1, (peak−baseline)/REF). + +export const EPSILON_MB_PER_CYCLE = 0.1; +export const SUSTAIN_FLOOR = 0.5; +// Reference working-set (MB) at which the floor equals SUSTAIN_FLOOR; the floor +// scales up linearly for larger arenas. ~200 MB ≈ a small FTS fixture's footprint. +export const FLOOR_REF_WORKINGSET_MB = 200; + +export function median(xs) { + const s = [...xs].sort((a, b) => a - b); + const m = Math.floor(s.length / 2); + return s.length % 2 ? s[m] : Math.round((s[m - 1] + s[m]) / 2); +} + +export function slopeMbPerCycle(series) { + // Least-squares slope of rss vs cycle index. + const n = series.length; + if (n < 2) return 0; + const xs = series.map((_, i) => i); + const xMean = xs.reduce((a, b) => a + b, 0) / n; + const yMean = series.reduce((a, b) => a + b, 0) / n; + let num = 0; + let den = 0; + for (let i = 0; i < n; i++) { + num += (xs[i] - xMean) * (series[i] - yMean); + den += (xs[i] - xMean) ** 2; + } + return den === 0 ? 0 : num / den; +} + +/** + * Classify an RSS-per-cycle series into PLATEAU / CLIMB / INCONCLUSIVE. + * Pure: no I/O, no globals. `baseline` is the pre-DB RSS; `peak` defaults to the + * series max. Returns the label plus the diagnostics the bench prints. + */ +export function classifyVerdict(series, baseline, peak = Math.max(...series)) { + const cycles = series.length; + const half = Math.max(1, Math.floor(cycles / 2)); + const firstHalfSlope = slopeMbPerCycle(series.slice(0, half)); + const secondHalfSlope = slopeMbPerCycle(series.slice(-half)); + const decelRatio = secondHalfSlope / Math.max(firstHalfSlope, 1e-9); + + // Step discontinuity: a single cycle-to-cycle jump far larger than the typical + // per-cycle delta — a one-time allocator/arena reservation (then flat), not a + // per-reload leak, but a noisy run we won't claim a clean result on. + const deltas = series.slice(1).map((v, i) => v - series[i]); + const absDeltas = deltas.map(Math.abs).sort((a, b) => a - b); + const medAbsDelta = absDeltas.length ? absDeltas[Math.floor(absDeltas.length / 2)] : 0; + const maxJump = deltas.length ? Math.max(...deltas) : 0; + const stepDiscontinuity = maxJump > Math.max(30, 5 * Math.max(medAbsDelta, 1)); + + // Working-set-scaled floor (see header). Guard against a negative working set. + const workingSet = Math.max(0, peak - baseline); + const floor = SUSTAIN_FLOOR * Math.max(1, workingSet / FLOOR_REF_WORKINGSET_MB); + + const SUSTAINED = 0.6; // decelRatio at/above which the tail is "not decaying" + let verdict; + if (stepDiscontinuity) { + verdict = 'INCONCLUSIVE'; + } else if (secondHalfSlope < EPSILON_MB_PER_CYCLE) { + // Effectively flat — no leak, regardless of decelRatio (a flat-from-start run + // has decelRatio ≈ 1 but is still PLATEAU). This gate is what keeps a true + // negative from being over-corrected into INCONCLUSIVE. + verdict = 'PLATEAU'; + } else if (secondHalfSlope >= floor) { + // Tail is still substantial: sustained → real leak; decelerating → unresolved. + verdict = decelRatio >= SUSTAINED ? 'CLIMB' : 'INCONCLUSIVE'; + } else if (decelRatio < SUSTAINED) { + // Below the floor AND decelerating — warmup converged toward flat → PLATEAU. + verdict = 'PLATEAU'; + } else { + // Below the floor but SUSTAINED — a slow steady creep RSS can't distinguish + // from noise at this scale. The honest label is "not resolved", NEVER a clean + // PLATEAU ("no leak"). This is the headline tri-review fix. + verdict = 'INCONCLUSIVE'; + } + + return { + verdict, + firstHalfSlope, + secondHalfSlope, + decelRatio, + floor, + stepDiscontinuity, + maxJump, + peak, + }; +} diff --git a/gitnexus/scripts/install-duckdb-extension.mjs b/gitnexus/scripts/install-duckdb-extension.mjs index 2bc65a05e..7492e084f 100644 --- a/gitnexus/scripts/install-duckdb-extension.mjs +++ b/gitnexus/scripts/install-duckdb-extension.mjs @@ -14,7 +14,7 @@ function parseLbugMaxDbSize(raw) { return Math.floor(parsed); } -async function installDuckDbExtension(extensionName) { +async function installDuckDbExtension(extensionName, verifyOnly = false) { if (!extensionName || !EXTENSION_NAME_PATTERN.test(extensionName)) { throw new Error(`Invalid DuckDB extension name: ${extensionName ?? ''}`); } @@ -22,9 +22,11 @@ async function installDuckDbExtension(extensionName) { const require = createRequire(import.meta.url); const lbugModule = require('@ladybugdb/core'); const lbug = lbugModule.default ?? lbugModule; - const lbugMaxDbSize = parseLbugMaxDbSize( - process.argv[3] ?? process.env.GITNEXUS_LBUG_MAX_DB_SIZE, - ); + // argv[3] is the optional positional size; ignore it when it is actually a + // flag token (e.g. `--verify-only`) and fall back to the env default. + const sizeArg = + process.argv[3] && !process.argv[3].startsWith('--') ? process.argv[3] : undefined; + const lbugMaxDbSize = parseLbugMaxDbSize(sizeArg ?? process.env.GITNEXUS_LBUG_MAX_DB_SIZE); const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-ext-install-')); const dbPath = path.join(tmpDir, 'install.lbug'); @@ -34,7 +36,18 @@ async function installDuckDbExtension(extensionName) { try { db = new lbug.Database(dbPath, 0, false, false, lbugMaxDbSize); conn = new lbug.Connection(db); - await conn.query(`INSTALL ${extensionName}`); + if (verifyOnly) { + // Prove a previously-baked extension is resolvable by a FRESH process + // under the current HOME (the runtime `LOAD EXTENSION` path) — no INSTALL, + // no network. Used as a Docker build-time gate so a HOME/extension-dir + // mismatch fails the build instead of silently degrading search at runtime. + await conn.query(`LOAD EXTENSION ${extensionName}`); + console.log( + `[install-ext] LOAD-only verify OK for '${extensionName}' (HOME=${process.env.HOME})`, + ); + } else { + await conn.query(`INSTALL ${extensionName}`); + } } finally { if (conn) await conn.close().catch(() => {}); if (db) await db.close().catch(() => {}); @@ -42,7 +55,10 @@ async function installDuckDbExtension(extensionName) { } } -installDuckDbExtension(process.argv[2] ?? process.env.GITNEXUS_LBUG_EXTENSION_NAME).catch((err) => { +installDuckDbExtension( + process.argv[2] ?? process.env.GITNEXUS_LBUG_EXTENSION_NAME, + process.argv.includes('--verify-only'), +).catch((err) => { console.error(err instanceof Error ? (err.stack ?? err.message) : String(err)); process.exitCode = 1; }); diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index 030688d14..e5338e3d0 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -103,6 +103,19 @@ const IDLE_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes /** Max connections per repo (caps concurrent queries per repo) */ const MAX_CONNS_PER_REPO = 8; +// Behavior-neutral RSS tracing for the FTS evict→reload memory repro +// (gitnexus/scripts/bench/fts-evict-reload-rss.mjs). Two invariants keep it safe +// in the pool init/close hot path: it writes ONLY to stderr (stdout is the MCP +// JSON-RPC channel), and the GITNEXUS_POOL_RSS_TRACE gate makes it a no-op — one +// env-var compare per call, nothing else — unless a harness explicitly enables it. +function traceRss(event: 'init' | 'close', repoId: string): void { + if (process.env.GITNEXUS_POOL_RSS_TRACE !== '1') return; + const rssMb = Math.round(process.memoryUsage().rss / (1024 * 1024)); + process.stderr.write( + `[pool-rss] ${event} repo=${repoId} pool=${pool.size} dbCache=${dbCache.size} rssMB=${rssMb}\n`, + ); +} + let idleTimer: ReturnType | null = null; // Stdout-capture state lives in `gitnexus/src/mcp/stdio-capture.ts` — a leaf @@ -240,6 +253,8 @@ function closeOne(repoId: string): void { // Isolate listener failures — teardown must complete. } } + + traceRss('close', repoId); } /** @@ -611,6 +626,7 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { closed: false, }); ensureIdleTimer(); + traceRss('init', repoId); } /** @@ -673,6 +689,7 @@ export async function initLbugWithDb( closed: false, }); ensureIdleTimer(); + traceRss('init', repoId); } /** diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 77411b107..ff566ddd2 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -177,6 +177,21 @@ function logQueryError(context: string, err: unknown): void { logger.error({ context, err: msg }, 'GitNexus query failed'); } +/** + * A "missing table/label/relation" prepare error is benign for the query tool's + * best-effort enrichment: a repo analyzed without processes or communities simply + * has no `Process`/`Community` tables, so the `STEP_IN_PROCESS` / `MEMBER_OF` + * enrichment queries fail to prepare. That is a normal configuration, NOT a + * degraded result — it must not raise the `partial` flag (which callers would + * then learn to ignore). Real failures (timeouts, locks, native faults) do. + */ +function isBenignMissingTableError(err: unknown): boolean { + const msg = err instanceof Error ? err.message : String(err ?? ''); + return /does not exist|no such (table|label|rel)|unknown (table|label)|not (defined|found)/i.test( + msg, + ); +} + const isReadOnlyDbError = (err: unknown): boolean => { // Walk the `cause` chain (bounded) so a wrapped read-only error (e.g. the // pool adapter's `{ cause }` wrapper) is still detected here — this is the @@ -1112,6 +1127,112 @@ export class LocalBackend { >(); const definitions: any[] = []; // standalone symbols not in any process + // Batch-fetch process participation, cohesion, and (optionally) content for + // ALL matched symbols in 2-3 graph queries instead of 2-3 *per symbol*. The + // previous per-symbol loop issued up to 3N sequential pool round-trips + // (searchLimit symbols × {STEP_IN_PROCESS, MEMBER_OF, content}); on a warm + // repo the IPC + query-setup overhead of those round-trips dominated query + // latency. Collapsing to `WHERE n.id IN $nodeIds` preserves identical output + // (the aggregation loop below is unchanged) while cutting the round-trips. + // Array params bind through the pool exactly as bm25Search's + // `WHERE n.id IN $nodeIds` already does. (Ported from gitnexus-enterprise + // PR #222 — N+1 → 2-3 batched queries.) + const nodeIds = merged.map(([, m]) => m.data?.nodeId).filter((id): id is string => !!id); + + const processRowsByNode = new Map(); + const cohesionByNode = new Map(); + const contentByNode = new Map(); + // Set when a batched enrichment query throws a REAL failure (timeout, lock, + // native fault) — NOT the benign "no Process/Community table" case, which is + // a normal config (a repo analyzed without processes/communities) and must + // not raise a `partial` flag callers would learn to ignore. See + // isBenignMissingTableError + the response build below. + let enrichmentDegraded = false; + + // Chunk the IN-list like the impact path (CHUNK_SIZE=100) so a large result + // set never builds an unbounded `IN` parameter. Default batch is + // processLimit*maxSymbolsPerProcess (≤ one chunk), but chunk for robustness. + const QUERY_CHUNK_SIZE = 100; + for (let i = 0; i < nodeIds.length; i += QUERY_CHUNK_SIZE) { + const ids = nodeIds.slice(i, i + QUERY_CHUNK_SIZE); + + // Processes each symbol participates in. `n.id AS nodeId` is prepended as + // column 0 so rows from many symbols can be re-associated to their symbol. + try { + const rows = await executeParameterized( + repo.lbugPath, + ` + MATCH (n)-[r:CodeRelation {type: 'STEP_IN_PROCESS'}]->(p:Process) + WHERE n.id IN $nodeIds + RETURN n.id AS nodeId, p.id AS pid, p.label AS label, p.heuristicLabel AS heuristicLabel, p.processType AS processType, p.stepCount AS stepCount, r.step AS step + `, + { nodeIds: ids }, + ); + for (const row of rows) { + const nid = row.nodeId ?? row[0]; + let list = processRowsByNode.get(nid); + if (!list) processRowsByNode.set(nid, (list = [])); + list.push(row); + } + } catch (e) { + logQueryError('query:process-lookup', e); + if (!isBenignMissingTableError(e)) enrichmentDegraded = true; + } + + // Cluster membership + cohesion. Keep the FIRST community row per node to + // mirror the prior per-symbol `LIMIT 1` (each symbol keeps ITS community, + // not one community for the whole batch). + try { + const rows = await executeParameterized( + repo.lbugPath, + ` + MATCH (n)-[:CodeRelation {type: 'MEMBER_OF'}]->(c:Community) + WHERE n.id IN $nodeIds + RETURN n.id AS nodeId, c.cohesion AS cohesion, c.heuristicLabel AS module + `, + { nodeIds: ids }, + ); + for (const row of rows) { + const nid = row.nodeId ?? row[0]; + if (!cohesionByNode.has(nid)) { + cohesionByNode.set(nid, { + cohesion: (row.cohesion ?? row[1]) || 0, + module: row.module ?? row[2], + }); + } + } + } catch (e) { + logQueryError('query:cluster-info', e); + if (!isBenignMissingTableError(e)) enrichmentDegraded = true; + } + + // Optionally fetch content for every matched symbol. + if (includeContent) { + try { + const rows = await executeParameterized( + repo.lbugPath, + ` + MATCH (n) + WHERE n.id IN $nodeIds + RETURN n.id AS nodeId, n.content AS content + `, + { nodeIds: ids }, + ); + for (const row of rows) { + const nid = row.nodeId ?? row[0]; + contentByNode.set(nid, row.content ?? row[1]); + } + } catch (e) { + logQueryError('query:content-fetch', e); + if (!isBenignMissingTableError(e)) enrichmentDegraded = true; + } + } + } + + // Aggregation is unchanged from the per-symbol version — it now reads the + // pre-fetched maps instead of issuing a query per symbol. Iterating `merged` + // in the same (sorted) order preserves processMap insertion order, the + // definitions order, and the item.score association exactly. for (const [_, item] of merged) { const sym = item.data; if (!sym.nodeId) { @@ -1124,61 +1245,11 @@ export class LocalBackend { continue; } - // Find processes this symbol participates in - let processRows: any[] = []; - try { - processRows = await executeParameterized( - repo.lbugPath, - ` - MATCH (n {id: $nodeId})-[r:CodeRelation {type: 'STEP_IN_PROCESS'}]->(p:Process) - RETURN p.id AS pid, p.label AS label, p.heuristicLabel AS heuristicLabel, p.processType AS processType, p.stepCount AS stepCount, r.step AS step - `, - { nodeId: sym.nodeId }, - ); - } catch (e) { - logQueryError('query:process-lookup', e); - } - - // Get cluster membership + cohesion (cohesion used as internal ranking signal) - let cohesion = 0; - let module: string | undefined; - try { - const cohesionRows = await executeParameterized( - repo.lbugPath, - ` - MATCH (n {id: $nodeId})-[:CodeRelation {type: 'MEMBER_OF'}]->(c:Community) - RETURN c.cohesion AS cohesion, c.heuristicLabel AS module - LIMIT 1 - `, - { nodeId: sym.nodeId }, - ); - if (cohesionRows.length > 0) { - cohesion = (cohesionRows[0].cohesion ?? cohesionRows[0][0]) || 0; - module = cohesionRows[0].module ?? cohesionRows[0][1]; - } - } catch (e) { - logQueryError('query:cluster-info', e); - } - - // Optionally fetch content - let content: string | undefined; - if (includeContent) { - try { - const contentRows = await executeParameterized( - repo.lbugPath, - ` - MATCH (n {id: $nodeId}) - RETURN n.content AS content - `, - { nodeId: sym.nodeId }, - ); - if (contentRows.length > 0) { - content = contentRows[0].content ?? contentRows[0][0]; - } - } catch (e) { - logQueryError('query:content-fetch', e); - } - } + const processRows = processRowsByNode.get(sym.nodeId) ?? []; + const coh = cohesionByNode.get(sym.nodeId); + const cohesion = coh?.cohesion ?? 0; + const module = coh?.module; + const content = includeContent ? contentByNode.get(sym.nodeId) : undefined; const symbolEntry = { id: sym.nodeId, @@ -1197,12 +1268,13 @@ export class LocalBackend { } else { // Add to each process it belongs to for (const row of processRows) { - const pid = row.pid ?? row[0]; - const label = row.label ?? row[1]; - const hLabel = row.heuristicLabel ?? row[2]; - const pType = row.processType ?? row[3]; - const stepCount = row.stepCount ?? row[4]; - const step = row.step ?? row[5]; + // Positional fallbacks shift +1 because `n.id AS nodeId` is column 0. + const pid = row.pid ?? row[1]; + const label = row.label ?? row[2]; + const hLabel = row.heuristicLabel ?? row[3]; + const pType = row.processType ?? row[4]; + const stepCount = row.stepCount ?? row[5]; + const step = row.step ?? row[6]; if (!processMap.has(pid)) { processMap.set(pid, { @@ -1276,15 +1348,29 @@ export class LocalBackend { const timing = timer.summary(); logQueryTiming(searchQuery, timing); + // Compose a single `warning` from all degraded conditions (FTS-missing + // and/or a real enrichment failure) so neither overwrites the other, and + // flag `partial` when enrichment was lost. Both are omitted on the clean + // path, leaving the success-path response shape byte-identical. + const warnings: string[] = []; + if (!ftsUsed) { + warnings.push( + 'FTS indexes missing — keyword search degraded. Run: gitnexus analyze --repair-fts (or gitnexus analyze --force) to rebuild indexes.', + ); + } + if (enrichmentDegraded) { + warnings.push( + 'Symbol enrichment partially failed — some process/cohesion/content data may be missing from these results (see server logs).', + ); + } + return { processes, process_symbols: dedupedSymbols, definitions: definitions.slice(0, 20), // cap standalone definitions timing, - ...(!ftsUsed && { - warning: - 'FTS indexes missing — keyword search degraded. Run: gitnexus analyze --repair-fts (or gitnexus analyze --force) to rebuild indexes.', - }), + ...(warnings.length > 0 && { warning: warnings.join(' ') }), + ...(enrichmentDegraded && { partial: true }), }; } diff --git a/gitnexus/test/fixtures/local-backend-seed.ts b/gitnexus/test/fixtures/local-backend-seed.ts index 3f299046e..4878348b2 100644 --- a/gitnexus/test/fixtures/local-backend-seed.ts +++ b/gitnexus/test/fixtures/local-backend-seed.ts @@ -35,6 +35,12 @@ export const LOCAL_BACKEND_SEED_DATA = [ CREATE (a)-[:CodeRelation {type: 'STEP_IN_PROCESS', confidence: 1.0, reason: '', step: 1}]->(p)`, `MATCH (a:Function), (p:Process) WHERE a.id = 'func:validate' AND p.id = 'proc:login-flow' CREATE (a)-[:CodeRelation {type: 'STEP_IN_PROCESS', confidence: 1.0, reason: '', step: 2}]->(p)`, + // func:validate is the terminalId of proc:beta-flow too — wiring its second + // STEP_IN_PROCESS edge makes it a genuine MULTI-process symbol, which the + // batched-query test uses to exercise the full row[1..6] positional shift + // (a single-process symbol can't expose an off-by-one in those fallbacks). + `MATCH (a:Function), (p:Process) WHERE a.id = 'func:validate' AND p.id = 'proc:beta-flow' + CREATE (a)-[:CodeRelation {type: 'STEP_IN_PROCESS', confidence: 1.0, reason: '', step: 3}]->(p)`, `MATCH (h:Function), (t:Tool) WHERE h.id = 'func:alpha' AND t.id = 'Tool:alpha' CREATE (h)-[:CodeRelation {type: 'HANDLES_TOOL', confidence: 1.0, reason: 'tool-definition', step: 0}]->(t)`, `MATCH (h:Function), (t:Tool) WHERE h.id = 'func:beta' AND t.id = 'Tool:beta' diff --git a/gitnexus/test/integration/local-backend-calltool.test.ts b/gitnexus/test/integration/local-backend-calltool.test.ts index e2640deab..eed46b18d 100644 --- a/gitnexus/test/integration/local-backend-calltool.test.ts +++ b/gitnexus/test/integration/local-backend-calltool.test.ts @@ -111,6 +111,78 @@ withTestLbugDB( // At least one of the search phases must have fired for any // non-error response — bm25 and/or vector always runs. expect(result.timing.bm25 ?? result.timing.vector).toBeGreaterThanOrEqual(0); + + // Success path (FTS present + Process/Community tables exist): no degraded + // signal. Guards R6 — the response shape stays byte-identical when nothing + // fails (the `warning`/`partial` fields appear only on degradation). + expect(result).not.toHaveProperty('warning'); + expect(result).not.toHaveProperty('partial'); + }); + + // PR #222 port: the query tool batches per-symbol process/cohesion/content + // lookups (N+1 → 2-3 `WHERE n.id IN $nodeIds` queries). These assertions + // guard the batch-adaptation hazards that a naive cherry-pick would break: + // (1) each symbol keeps ITS OWN community (the per-node first-row pick that + // replaced the per-symbol `LIMIT 1`), and (2) content maps to the right + // node — both depend on the +1 positional-index shift after prepending + // `n.id AS nodeId`. func:login is MEMBER_OF comm:auth ("Authentication"); + // func:validate has no community, so it must NOT inherit login's. + it('query batches per-symbol enrichment without cross-assigning community/content', async () => { + const findSym = (res: any, id: string) => + (res.process_symbols ?? []).find((s: any) => s.id === id) ?? + (res.definitions ?? []).find((s: any) => s.id === id); + + const loginRes = await backend.callTool('query', { + query: 'login', + include_content: true, + }); + expect(loginRes).not.toHaveProperty('error'); + const login = findSym(loginRes, 'func:login'); + expect(login).toBeDefined(); + // Community correctly associated to its own node (not dropped, not leaked). + expect(login.module).toBe('Authentication'); + // Content correctly mapped to its own node (positional [1] after nodeId). + expect(login.content).toBe('function login() {}'); + + const validateRes = await backend.callTool('query', { + query: 'validate', + include_content: true, + }); + expect(validateRes).not.toHaveProperty('error'); + const validate = findSym(validateRes, 'func:validate'); + expect(validate).toBeDefined(); + // validate has no MEMBER_OF edge — a flat batched `LIMIT 1` would have + // leaked some other node's community onto it. It must have none. + expect(validate.module).toBeUndefined(); + expect(validate.content).toBe('function validate() {}'); + }); + + // PR #222 port: a symbol in MULTIPLE processes is what fully exercises the + // +1 positional shift in the batched STEP_IN_PROCESS aggregation — with a + // single process row, `row.pid ?? row[1]` succeeds whether the shift is + // right or wrong. func:validate is a step in BOTH proc:login-flow (step 2) + // and proc:beta-flow (step 3), so both rows for the one node must be parsed + // (pid=row[1], step=row[6]); an off-by-one would drop a process or mis-pair + // pid↔step. Also pins process ranking (totalScore via the regroup-by-nodeId). + it('query batches a multi-process symbol and ranks processes (positional shift across rows)', async () => { + const res = await backend.callTool('query', { query: 'validate' }); + expect(res).not.toHaveProperty('error'); + const processIds = (res.processes ?? []).map((p: any) => p.id); + // Both of validate's processes must appear — both STEP_IN_PROCESS rows + // were parsed and grouped by the correct pid (row[1]). + expect(processIds).toContain('proc:login-flow'); + expect(processIds).toContain('proc:beta-flow'); + + // process_symbols dedups by id, so validate appears once carrying the + // pid+step of its top-ranked process — they must come from the SAME + // shifted row: login-flow⇒step 2, beta-flow⇒step 3. + const v = (res.process_symbols ?? []).find((s: any) => s.id === 'func:validate'); + expect(v).toBeDefined(); + expect(v.step_index).toBe(v.process_id === 'proc:beta-flow' ? 3 : 2); + + // Ranking: 'login' surfaces proc:login-flow as the top process. + const loginRes = await backend.callTool('query', { query: 'login' }); + expect((loginRes.processes ?? [])[0]?.id).toBe('proc:login-flow'); }); it('tool_map returns per-tool flows without cross-attributing same-file tools', async () => { diff --git a/gitnexus/test/unit/fts-rss-verdict.test.ts b/gitnexus/test/unit/fts-rss-verdict.test.ts new file mode 100644 index 000000000..db4e644eb --- /dev/null +++ b/gitnexus/test/unit/fts-rss-verdict.test.ts @@ -0,0 +1,76 @@ +import { describe, it, expect } from 'vitest'; +// Pure, side-effect-free classifier extracted from the FTS evict→reload bench. +// Importing it must NOT load @ladybugdb/core, build fixtures, or run the bench +// (the module has zero imports and zero module-scope side effects). If this +// import ever pulled in the bench's native-addon require, this test file would +// be slow / fail to load — so the import succeeding cheaply IS the no-side-effect +// guard (R3). +import { classifyVerdict, median, slopeMbPerCycle } from '../../scripts/bench/fts-rss-verdict.mjs'; + +// Build an exactly-linear series: start, start+slope, start+2·slope, … +const linear = (start: number, slope: number, n: number): number[] => + Array.from({ length: n }, (_, i) => start + slope * i); + +describe('classifyVerdict (FTS evict→reload bench)', () => { + it('pure helpers behave', () => { + expect(typeof classifyVerdict).toBe('function'); + expect(median([3, 1, 2])).toBe(2); + // Least-squares slope of an exact linear series equals its step. + expect(slopeMbPerCycle(linear(100, 2, 10))).toBeCloseTo(2, 6); + }); + + it('truly flat run → PLATEAU (over-correction guard, R1)', () => { + // No leak, no warmup: first ≈ second ≈ 0. Must stay PLATEAU, NOT INCONCLUSIVE. + const series = [200, 200, 201, 200, 200, 201, 200, 200, 200, 201, 200, 200]; + const r = classifyVerdict(series, 190); + expect(r.verdict).toBe('PLATEAU'); + }); + + it('sustained sub-floor positive slope → INCONCLUSIVE (the headline fix, R1)', () => { + // ~0.4 MB/cycle, sustained (first ≈ second slope), below the absolute floor. + // The OLD logic labeled this PLATEAU ("no leak"); it must now be INCONCLUSIVE. + const series = linear(200, 0.4, 20); // peak ~207.6, baseline 190 → small WS → floor stays 0.5 + const r = classifyVerdict(series, 190); + expect(r.secondHalfSlope).toBeGreaterThan(0.1); // above EPSILON + expect(r.secondHalfSlope).toBeLessThan(r.floor); // below the floor + expect(r.decelRatio).toBeGreaterThanOrEqual(0.6); // sustained, not decelerating + expect(r.verdict).toBe('INCONCLUSIVE'); + }); + + it('decelerated-to-flat run → PLATEAU', () => { + // Climbs then flattens: tail slope ≈ 0 (below EPSILON). + const series = [200, 210, 218, 224, 228, 230, 231, 231, 231, 231, 231, 231, 231, 231, 231, 231]; + const r = classifyVerdict(series, 190); + expect(r.verdict).toBe('PLATEAU'); + }); + + it('sustained linear above the floor → CLIMB', () => { + const series = linear(200, 3, 20); // 3 MB/cycle sustained + const r = classifyVerdict(series, 190); + expect(r.decelRatio).toBeGreaterThanOrEqual(0.6); + expect(r.secondHalfSlope).toBeGreaterThanOrEqual(r.floor); + expect(r.verdict).toBe('CLIMB'); + }); + + it('step discontinuity → INCONCLUSIVE (existing guard preserved)', () => { + const series = [200, 201, 202, 203, 204, 205, 265, 266, 267, 268, 269, 270]; + const r = classifyVerdict(series, 190); + expect(r.stepDiscontinuity).toBe(true); + expect(r.verdict).toBe('INCONCLUSIVE'); + }); + + it('floor scales with working-set growth, not baseline RSS (R2)', () => { + // Identical 0.8 MB/cycle sustained tail, two different working sets. + const series = linear(600, 0.8, 12); // peak ~608.8 + // Small working set (baseline near the series) → low floor → 0.8 clears it → CLIMB. + const small = classifyVerdict(series, 590); // peak-baseline ~18.8 → floor 0.5 + expect(small.secondHalfSlope).toBeGreaterThanOrEqual(small.floor); + expect(small.verdict).toBe('CLIMB'); + // Large working set (low baseline) → floor rises with arena size → 0.8 is now + // sub-floor → the sustained-but-small slope is unresolved, not a clean CLIMB. + const large = classifyVerdict(series, 0); // peak-baseline ~608 → floor ~1.5 + expect(large.floor).toBeGreaterThan(small.floor); + expect(large.secondHalfSlope).toBeLessThan(large.floor); + expect(large.verdict).toBe('INCONCLUSIVE'); + }); +}); diff --git a/gitnexus/test/unit/query-degraded-signal.test.ts b/gitnexus/test/unit/query-degraded-signal.test.ts new file mode 100644 index 000000000..f7022efa2 --- /dev/null +++ b/gitnexus/test/unit/query-degraded-signal.test.ts @@ -0,0 +1,120 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Mock the pool adapter (and its re-export shim) so executeParameterized is fully +// controllable — the proven seam from impact-batching-grouping.test.ts. This is a +// UNIT test: the integration suite runs the real executeParameterized against a +// real DB, so it cannot make ONE enrichment query throw while the rest succeed. +const executeParameterizedMock = vi.fn(); + +vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + initLbug: vi.fn(), + executeParameterized: (...args: any[]) => executeParameterizedMock(...args), + closeLbug: vi.fn(), + isLbugReady: vi.fn().mockReturnValue(true), + }; +}); +vi.mock('../../src/mcp/core/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + initLbug: vi.fn(), + executeParameterized: (...args: any[]) => executeParameterizedMock(...args), + closeLbug: vi.fn(), + isLbugReady: vi.fn().mockReturnValue(true), + }; +}); + +import { LocalBackend } from '../../src/mcp/local/local-backend'; + +// A backend whose hybrid search yields exactly one matched symbol, so the +// enrichment chunk loop runs and can be made to fail. `ftsUsed` is parameterized +// so we can exercise the FTS-missing + enrichment-degraded composition. +function makeBackend(ftsUsed = true): LocalBackend { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo1', + name: 'repo1', + repoPath: '/tmp/repo', + storagePath: '/tmp/repo/.gitnexus', + lbugPath: '/tmp/repo/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + const sym = { + nodeId: 'func:x', + name: 'x', + type: 'Function', + filePath: 'f.ts', + startLine: 1, + endLine: 2, + }; + (backend as any).bm25Search = vi.fn().mockResolvedValue({ results: [sym], ftsUsed }); + (backend as any).semanticSearch = vi.fn().mockResolvedValue([]); + return { backend, repoHandle } as any; +} + +const runQuery = (b: any, params: any = { query: 'x' }) => + (b.backend as any).query(b.repoHandle, params); + +describe('query: degraded-enrichment signal', () => { + beforeEach(() => vi.clearAllMocks()); + + it('a REAL enrichment failure surfaces warning + partial, and still returns the symbol', async () => { + const b = makeBackend(true); + executeParameterizedMock.mockImplementation(async (_repo: string, query: string) => { + if (query.includes('STEP_IN_PROCESS')) + throw new Error('Query execution timed out after 30000ms'); + return []; // MEMBER_OF / content succeed (empty) + }); + + const result = await runQuery(b); + + expect(result).not.toHaveProperty('error'); + expect(result.partial).toBe(true); + expect(typeof result.warning).toBe('string'); + expect(result.warning.toLowerCase()).toContain('enrichment'); + // The matched symbol still comes back (degraded to definitions, not dropped). + expect(result.definitions.map((d: any) => d.id)).toContain('func:x'); + }); + + it('a BENIGN missing-table error does NOT trip the signal', async () => { + const b = makeBackend(true); + executeParameterizedMock.mockImplementation(async (_repo: string, query: string) => { + // A repo analyzed without processes/communities: prepare fails because the + // table/label does not exist. This is normal, not degraded. + if (query.includes('STEP_IN_PROCESS') || query.includes('MEMBER_OF')) + throw new Error('Binder exception: Table Process does not exist.'); + return []; + }); + + const result = await runQuery(b); + + expect(result).not.toHaveProperty('error'); + expect(result.partial).toBeUndefined(); + expect(result.warning).toBeUndefined(); // ftsUsed=true and no real failure + expect(result.definitions.map((d: any) => d.id)).toContain('func:x'); + }); + + it('composes the FTS-missing warning with the enrichment-degraded message', async () => { + const b = makeBackend(false); // FTS unavailable + executeParameterizedMock.mockImplementation(async (_repo: string, query: string) => { + if (query.includes('STEP_IN_PROCESS')) + throw new Error('Query execution timed out after 30000ms'); + return []; + }); + + const result = await runQuery(b); + + expect(result.partial).toBe(true); + expect(typeof result.warning).toBe('string'); + // Both messages present in the single composed warning — neither overwrites the other. + expect(result.warning).toMatch(/FTS indexes missing|repair-fts/i); + expect(result.warning.toLowerCase()).toContain('enrichment'); + }); +}); From f1151660b9e0c73a80d963dcc97d333176ba6a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Tue, 9 Jun 2026 09:04:01 +0100 Subject: [PATCH 2/2] fix(install): graceful Kotlin optional-grammar install + accurate toolchain docs (#2110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(install): document Kotlin optional-grammar toolchain behavior + graceful install probe tree-sitter-kotlin is a third-party npm optionalDependency that ships source-only (no upstream prebuilds) and compiles its native binding via node-gyp at install. It was the only optional grammar without a GitNexus install-time probe, and the README's GITNEXUS_SKIP_OPTIONAL_GRAMMARS "no toolchain needed" note omitted Kotlin entirely. This adds a fail-soft probe (mirroring the Swift one) that warns clearly and always exits 0 so install never breaks, wires it into postinstall, and corrects the optional-grammar docs in README.md and .devcontainer/README.md. Shipping prebuilt .node binaries (the literal request) needs an upstream/CI build matrix and is intentionally left as follow-up. Refs #2107 Co-Authored-By: Claude Opus 4.8 (1M context) * fix: address PR #2110 tri-review findings (Kotlin optional-grammar install) Addresses the four P2 findings from the PR #2110 tri-review: - F1: docs no longer imply GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1 skips Kotlin's toolchain. npm compiles tree-sitter-kotlin via its own node-gyp-build step regardless of that variable; point to `npm install --omit=optional` as the real lever (README.md + .devcontainer/README.md). - F2: the install probe now surfaces its "Kotlin unavailable" guidance on the dir-absent branch — the dominant toolchain-less case, where npm prunes the failed optional dependency so the package dir is gone at postinstall. Gated on npm_config_omit so a deliberate `--omit=optional` stays silent. Still never throws or exits non-zero. - F3: add a behavioral test that executes the probe across its skip / dir-absent-warn / dir-absent-omit-silent paths and asserts exit code 0 (guards the postinstall "never exit non-zero" invariant a static assertion cannot). - F4: reframe prebuilt Kotlin as deferred Swift-parity follow-up — GitNexus already vendors its own self-built Swift prebuilds and could do the same for Kotlin — tracked in #2107, not an upstream-only blocker. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .devcontainer/README.md | 2 +- README.md | 6 +- gitnexus/package.json | 2 +- gitnexus/scripts/build-tree-sitter-kotlin.cjs | 74 +++++++++++ .../build-tree-sitter-kotlin-probe.test.ts | 115 ++++++++++++++++++ gitnexus/test/unit/cli-commands.test.ts | 10 ++ 6 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 gitnexus/scripts/build-tree-sitter-kotlin.cjs create mode 100644 gitnexus/test/unit/build-tree-sitter-kotlin-probe.test.ts diff --git a/.devcontainer/README.md b/.devcontainer/README.md index 8817c1773..80642a121 100644 --- a/.devcontainer/README.md +++ b/.devcontainer/README.md @@ -310,7 +310,7 @@ VS Code's Ports panel shows forwarded ports once their listener starts. - **LadybugDB integration tests may fail in containers** (file-locking, `AGENTS.md` § Testing). Default to `npm run test:unit` inside the container; run integration tests on the host. Tracking issue: documented as a known limitation. - **Single-writer LadybugDB constraint** (`GUARDRAILS.md` § LadybugDB lock). Don't run `gitnexus analyze` on the host and inside the container against the same `.gitnexus/` directory simultaneously — the second writer will get `database busy`. -- **Native grammar builds add ~30s to first install.** Tree-sitter Dart/Proto/Swift grammars build during `gitnexus`'s `postinstall`. To skip them (loses parsing for those three languages), set `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1` in your shell or add it to `remoteEnv` and rebuild. +- **Native grammar builds add ~30s to first install.** Tree-sitter Dart/Proto/Swift grammars build during `gitnexus`'s `postinstall`; the `tree-sitter-kotlin` optional dependency (third-party npm package, source-only — no upstream prebuilds) compiles its native binding earlier, during npm's own dependency install, and is then probed by `build-tree-sitter-kotlin.cjs`. Set `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1` (in your shell or `remoteEnv`, then rebuild) to skip the Dart/Proto/Swift builds and silence the Kotlin probe — but npm still compiles `tree-sitter-kotlin` unless you also pass `--omit=optional`. Each loses parsing for the affected language(s); the install still succeeds. - **`tree-sitter-kotlin` warnings on install** are expected (per `AGENTS.md`). Ignore them. - **`.mcp.json` works inside the container**: `npx -y gitnexus@latest mcp` resolves cleanly because npm registry is reachable and the workspace bind mount exposes the same `.mcp.json` the host sees. - **Husky pre-commit fires inside the container** without extra setup. The root `npm install` (run automatically in `postCreateCommand`) installs the hook via `package.json` `prepare`. diff --git a/README.md b/README.md index 2df823d9a..44876243c 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,9 @@ That's it. This indexes the codebase, installs agent skills, registers Claude Co To configure MCP for your editor, run `npx gitnexus setup` once — or set it up manually below. -> **Faster install (no C++ toolchain needed):** set `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1` before `npm install -g gitnexus` to skip vendored grammar materialize/build (`tree-sitter-dart`, `tree-sitter-proto`, `tree-sitter-swift`). Dart/Proto/Swift files won't be parsed, but install completes in seconds without `python3`/`make`/`g++`. Strict `=1` only — any other value falls through to the rebuild. +> **Faster install (no C++ toolchain needed):** set `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1` before `npm install -g gitnexus` to skip the vendored grammar materialize/build for `tree-sitter-dart`, `tree-sitter-proto`, and `tree-sitter-swift` — those three won't be parsed, but install completes in seconds without `python3`/`make`/`g++`. Strict `=1` only — any other value falls through to the rebuild. This variable does **not** control `tree-sitter-kotlin` (a third-party npm `optionalDependency` that npm compiles via its own `node-gyp-build` step regardless); to skip the Kotlin compile too, add `npm install --omit=optional` — which also drops the `node-gyp-build`/`node-addon-api` build deps and so disables the vendored builds as well. See the `tree-sitter-kotlin` note below. +> +> **About `tree-sitter-kotlin`:** unlike the vendored grammars, Kotlin support comes from a third-party npm `optionalDependency` that ships **source only** (no upstream prebuilt binaries) and compiles via node-gyp at install time. On a host without a C/C++ toolchain its native build soft-fails: npm skips the optional dependency, the `gitnexus` install still **succeeds**, and only Kotlin (`.kt`/`.kts`) parsing is unavailable. An install-time probe surfaces a single clear warning when the binding is missing (suppressed only if you opted out with `--omit=optional`), instead of leaving raw node-gyp output as the only signal, and it honors `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1`. (GitNexus does **not** yet ship prebuilt Kotlin binaries. GitNexus already vendors its own self-built Swift prebuilds and could do the same for Kotlin — that's deferred Swift-parity follow-up work tracked in [#2107](https://github.com/abhigyanpatwari/GitNexus/issues/2107), not an upstream blocker.) ### MCP Setup @@ -328,7 +330,7 @@ Most `analyze` knobs are also CLI flags (`--workers`, `--worker-timeout`, `--max | `GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD`| `max(3, poolSize)` | Per-slot consecutive deaths before the pool's circuit breaker trips. After tripping, every subsequent dispatch rejects until a fresh pool is created. | Hosts where a SIGSEGV-prone native grammar should trip the breaker sooner; CI runners that should fail loudly. | | `GITNEXUS_CHUNK_BYTE_BUDGET` | `2097152` (2 MB) | Chunk boundary used for cache-key composition and dispatch. Smaller = finer-grained cache hits but more dispatch overhead. | Tuning incremental-analyze cache behavior on monorepos. | | `GITNEXUS_NO_GITIGNORE` | unset | When set, skips `.gitignore` parsing. `.gitnexusignore` is still honored. | Indexing a repo whose `.gitignore` excludes files you actually want indexed (e.g., generated code committed for cross-repo lookup). | -| `GITNEXUS_SKIP_OPTIONAL_GRAMMARS` | unset | When `=1` strictly, skips vendored grammar materialize/build for `tree-sitter-dart`, `tree-sitter-proto`, and `tree-sitter-swift` at install time. | Installing on a host without a C++ toolchain or where Swift prebuilds don't match; you're willing to skip Dart/Proto/Swift parsing. | +| `GITNEXUS_SKIP_OPTIONAL_GRAMMARS` | unset | When `=1` strictly, skips the vendored grammar materialize/build for `tree-sitter-dart`, `tree-sitter-proto`, and `tree-sitter-swift` at install time, and silences GitNexus's `tree-sitter-kotlin` probe. It does **not** stop npm from compiling `tree-sitter-kotlin` (a third-party `optionalDependency` with its own `node-gyp-build` step) — use `npm install --omit=optional` to skip that compile too. Without a toolchain the Kotlin build soft-fails, npm skips it, the install still succeeds, and only Kotlin parsing is lost. | Installing on a host without a C++ toolchain or where Swift prebuilds don't match; willing to skip Dart/Proto/Swift parsing (and, with `--omit=optional`, Kotlin). | #### Publishing to understand-quickly (opt-in) diff --git a/gitnexus/package.json b/gitnexus/package.json index cd065cef0..293b651ed 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -49,7 +49,7 @@ "test:watch": "vitest", "test:coverage": "vitest run --coverage", "test:cross-platform": "tsx scripts/run-cross-platform.ts", - "postinstall": "node scripts/materialize-vendor-grammars.cjs && node scripts/build-tree-sitter-dart.cjs && node scripts/build-tree-sitter-proto.cjs && node scripts/build-tree-sitter-swift.cjs", + "postinstall": "node scripts/materialize-vendor-grammars.cjs && node scripts/build-tree-sitter-dart.cjs && node scripts/build-tree-sitter-proto.cjs && node scripts/build-tree-sitter-swift.cjs && node scripts/build-tree-sitter-kotlin.cjs", "prepare": "node scripts/build.js", "prepack": "node scripts/build.js" }, diff --git a/gitnexus/scripts/build-tree-sitter-kotlin.cjs b/gitnexus/scripts/build-tree-sitter-kotlin.cjs new file mode 100644 index 000000000..e21a93766 --- /dev/null +++ b/gitnexus/scripts/build-tree-sitter-kotlin.cjs @@ -0,0 +1,74 @@ +#!/usr/bin/env node +/** + * Probe tree-sitter-kotlin native-binding availability at install time. + * + * Unlike Dart/Proto/Swift (vendored under vendor/ and materialized into + * node_modules/ at postinstall), tree-sitter-kotlin is a third-party npm + * `optionalDependency`. It ships SOURCE ONLY — no upstream `prebuilds/` dir — + * and its own `install` script runs `node-gyp-build`, which compiles the + * native binding from source via node-gyp. On a host without a C/C++ toolchain + * that build soft-fails: npm skips the optional dependency and the `gitnexus` + * install still succeeds. This probe surfaces a single, friendly install-time + * warning when the Kotlin binding is unavailable — whether npm pruned the + * optional dependency after a toolchain-less build failure (its dir is gone, + * which is the common case) or the dir survives but the binding won't load — + * instead of leaving a raw node-gyp error or a first-use runtime failure as the + * only signal. A deliberate opt-out (`--omit=optional`) stays silent. The probe + * does not copy, register, or mutate anything; the runtime require() path in + * parser-loader does the actual load. This probe MUST NEVER throw or exit + * non-zero — it must never break `gitnexus` install. + */ +const fs = require('fs'); +const path = require('path'); + +if (process.env.GITNEXUS_SKIP_OPTIONAL_GRAMMARS === '1') { + console.warn( + '[tree-sitter-kotlin] Skipping native-binding probe (GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1).', + ); + process.exit(0); +} + +const kotlinDir = path.join(__dirname, '..', 'node_modules', 'tree-sitter-kotlin'); + +// `--omit=optional` / `--no-optional` / `.npmrc omit=optional` surface to +// lifecycle scripts as `npm_config_omit` containing `optional` (a comma- or +// space-separated list, e.g. `dev,optional`). That is a deliberate opt-out, so +// an absent package for that reason should stay silent. Any OTHER absence means +// npm attempted the optional dependency's native build and pruned the package +// after it soft-failed (the toolchain-less case) — exactly when the guidance +// below is worth surfacing. +const omitsOptional = /(^|[,\s])optional([,\s]|$)/.test(process.env.npm_config_omit || ''); + +function warnKotlinUnavailable(err) { + if (err) { + console.warn('[tree-sitter-kotlin] Native-binding probe failed:', err.message); + } + console.warn( + '[tree-sitter-kotlin] Kotlin (.kt/.kts) parsing will be unavailable. Non-Kotlin functionality is unaffected.', + ); + console.warn( + '[tree-sitter-kotlin] This is expected on hosts without a C/C++ toolchain: tree-sitter-kotlin ships source only (no upstream prebuilt binaries) and compiles via node-gyp at install. Set GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1 to skip this probe.', + ); +} + +try { + if (!fs.existsSync(path.join(kotlinDir, 'bindings', 'node', 'index.js'))) { + // The package never materialized. If the user deliberately omitted optional + // dependencies, stay silent — they opted out. Otherwise npm pruned the + // package after its native build soft-failed (no toolchain), and this is the + // dominant real-world failure case: surface the guidance the raw node-gyp + // error would otherwise be the only signal of. + if (!omitsOptional) { + warnKotlinUnavailable(); + } + process.exit(0); + } + + const nodeGypBuild = require('node-gyp-build'); + nodeGypBuild(kotlinDir); +} catch (err) { + // The package is present but its native binding can't be loaded (e.g. the dir + // survived with --ignore-scripts, or a partial/ABI-mismatched build). + warnKotlinUnavailable(err); + process.exit(0); +} diff --git a/gitnexus/test/unit/build-tree-sitter-kotlin-probe.test.ts b/gitnexus/test/unit/build-tree-sitter-kotlin-probe.test.ts new file mode 100644 index 000000000..45e997f11 --- /dev/null +++ b/gitnexus/test/unit/build-tree-sitter-kotlin-probe.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** + * Behavioral coverage for the postinstall probe `scripts/build-tree-sitter-kotlin.cjs`. + * + * The probe's hard invariant is that it MUST NEVER exit non-zero — it runs in + * `gitnexus`'s postinstall, so a non-zero exit would break `npm install gitnexus` + * for every user. The static package.json assertion in `cli-commands.test.ts` + * only checks wiring (the script is referenced in `postinstall`); it never runs + * the probe, so a regression that turned an `exit(0)` into `exit(1)`/`throw` + * would ship undetected. This suite executes the real script bytes across its + * branches and asserts exit code 0 every time. + * + * To exercise the "package absent" branches without mutating the repo's real + * node_modules, the probe is copied into an isolated temp `scripts/` dir; its + * `__dirname`-relative `../node_modules/tree-sitter-kotlin` then resolves to a + * non-existent path — the exact state npm leaves behind after it prunes the + * failed optional dependency on a toolchain-less host (see #2107 / PR #2110). + */ + +const probeSource = readFileSync( + fileURLToPath(new URL('../../scripts/build-tree-sitter-kotlin.cjs', import.meta.url)), + 'utf8', +); + +const UNAVAILABLE = 'Kotlin (.kt/.kts) parsing will be unavailable'; + +let tmpRoot: string; +let scriptPath: string; + +beforeAll(() => { + tmpRoot = mkdtempSync(path.join(tmpdir(), 'gn-kotlin-probe-')); + const scriptsDir = path.join(tmpRoot, 'scripts'); + mkdirSync(scriptsDir, { recursive: true }); + scriptPath = path.join(scriptsDir, 'build-tree-sitter-kotlin.cjs'); + writeFileSync(scriptPath, probeSource); +}); + +afterAll(() => { + rmSync(tmpRoot, { recursive: true, force: true }); +}); + +function runProbe(overrides: Record) { + const env: Record = {}; + for (const [k, v] of Object.entries(process.env)) { + if (v !== undefined) env[k] = v; + } + // Normalize the two variables under test so the case is deterministic even + // when the test runner itself was launched under npm with these set. + delete env.GITNEXUS_SKIP_OPTIONAL_GRAMMARS; + delete env.npm_config_omit; + for (const [k, v] of Object.entries(overrides)) { + if (v === undefined) delete env[k]; + else env[k] = v; + } + return spawnSync(process.execPath, [scriptPath], { env, encoding: 'utf8', timeout: 10_000 }); +} + +describe('build-tree-sitter-kotlin.cjs install probe', () => { + it('exits 0 and reports skipping when GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1', () => { + const r = runProbe({ GITNEXUS_SKIP_OPTIONAL_GRAMMARS: '1' }); + expect(r.status).toBe(0); + expect(r.signal).toBeNull(); + expect(r.stderr).toContain('Skipping native-binding probe'); + expect(r.stderr).not.toContain(UNAVAILABLE); + }); + + it('warns (and exits 0) when the package is absent and optionals were not omitted', () => { + // Regression guard for #2107 / PR #2110: npm prunes the failed optional + // dependency on a toolchain-less host, so the probe must surface its guidance + // on the dir-absent branch rather than silently exiting. + const r = runProbe({}); + expect(r.status).toBe(0); + expect(r.signal).toBeNull(); + expect(r.stderr).toContain(UNAVAILABLE); + }); + + it('stays silent (and exits 0) when optionals were deliberately omitted (omit=optional)', () => { + const r = runProbe({ npm_config_omit: 'optional' }); + expect(r.status).toBe(0); + expect(r.stderr).not.toContain(UNAVAILABLE); + }); + + it('treats a comma-joined list (dev,optional) as an opt-out and stays silent', () => { + const r = runProbe({ npm_config_omit: 'dev,optional' }); + expect(r.status).toBe(0); + expect(r.stderr).not.toContain(UNAVAILABLE); + }); + + it('still warns when only non-optional groups are omitted (omit=dev)', () => { + const r = runProbe({ npm_config_omit: 'dev' }); + expect(r.status).toBe(0); + expect(r.stderr).toContain(UNAVAILABLE); + }); + + it('never exits non-zero across env permutations (postinstall hard invariant)', () => { + const permutations: Record[] = [ + { GITNEXUS_SKIP_OPTIONAL_GRAMMARS: '1' }, + {}, + { npm_config_omit: 'optional' }, + { npm_config_omit: 'dev,optional' }, + { npm_config_omit: 'dev' }, + ]; + for (const overrides of permutations) { + const r = runProbe(overrides); + expect(r.status).toBe(0); + expect(r.signal).toBeNull(); + } + }); +}); diff --git a/gitnexus/test/unit/cli-commands.test.ts b/gitnexus/test/unit/cli-commands.test.ts index fcae5d0bd..0d6b56995 100644 --- a/gitnexus/test/unit/cli-commands.test.ts +++ b/gitnexus/test/unit/cli-commands.test.ts @@ -99,6 +99,16 @@ describe('CLI commands', () => { expect(swiftPkg.default.dependencies).toBeUndefined(); expect(swiftPkg.default.peerDependencies['tree-sitter']).toContain('^0.21.1'); }); + + it('declares tree-sitter-kotlin as an optionalDependency probed at postinstall (#2107)', async () => { + const pkg = await import('../../package.json', { with: { type: 'json' } }); + const optional = pkg.default.optionalDependencies ?? {}; + // Kotlin is a third-party npm optionalDependency (not vendored), so npm + // skips it when its source-only native build soft-fails — the gitnexus + // install still succeeds. + expect(optional['tree-sitter-kotlin']).toBeDefined(); + expect(pkg.default.scripts.postinstall).toContain('build-tree-sitter-kotlin.cjs'); + }); }); describe('analyzeCommand', () => {