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] 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'); + }); +});