GitNexus/gitnexus/scripts/bench/fts-rss-verdict.mjs
Gergő Magyar 288b96f3e5
fix: batch query enrichment, bake FTS extension into CLI image, add FTS memory repro (#2108)
* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

* 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) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-09 08:46:46 +01:00

105 lines
4.7 KiB
JavaScript
Raw Permalink Blame History

This file contains ambiguous Unicode characters

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

// 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, (peakbaseline)/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,
};
}