5 commits

Author	SHA1	Message	Date
Gergő Magyar	76a1c90b02	fix(fts): diagnose Windows FTS missing-dependency load failures (#2374, Phase 1) (#2383) ... * feat(lbug): classify FTS extension load errors with Windows missing-dependency guard (#2374) Add classifyExtensionLoadError() — a pure-string, lbug-free four-way classifier (missing_file / corrupt_file / missing_dependency / unknown). The Windows catch-all guard keys missing_dependency strictly on the error-126 signal, never LadybugDB's generic 'Failed to load library … needed by extension' wrapper, so 127/5/1114 and truncated (193) files route correctly. * feat(fts): surface classified missing-dependency remedy in doctor, repair-fts, and degrade warnings (#2374) Route the FTS load reason through classifyExtensionLoadError at all four surfaces (doctor, --repair-fts error, analyze degrade log, ftsDegradedWarning). For the Windows missing-dependency class, emit the runtime-install remedy (VC++ redist, then OpenSSL) instead of the wrong reinstall-over-network guidance; other classes keep their existing routing. Path redaction preserved on the client-facing warning. * test(fts): assert doctor surfaces the classified remedy end-to-end (#2374) Extend the broken-file e2e: doctor now prints the corrupt-file re-download remedy through the real CLI, and the Windows missing-dependency remedy (VC++/OpenSSL) must not misfire on a corrupt file — the catch-all guard, verified end-to-end. Also assert the repair path does not misfire. * style(fts): apply prettier formatting to #2374 diagnosis files * feat(fts): language-independent hedged fallback for Windows load failures (#2374) The Windows OS-error tail is localized, so matching only en/zh 126 text left other locales on the generic 'run doctor' remedy. lbug's 'Failed to load library' wrapper is English on every platform and present for all load failures, so use it as a fallback: when the localized tail matches no specific class, emit a hedged remedy that points the user at their own OS error and offers both branches (install runtime / --repair-fts) without prescribing the wrong single fix. Precise en/zh 126 keeps its definite remedy. * feat(fts): language-independent structural classifier via binary inspection (#2374) Add diagnoseExtensionLoad: pull the extension's file path out of lbug's own English wrapper and inspect the binary header (PE/ELF/Mach-O magic + arch) directly, so corrupt-vs-valid is decided by the file itself, not the localized OS-error tail. A valid binary that still failed to load ⇒ missing_dependency (runtime dep), decided in any OS display language and on all three platforms. Falls back to the string classifier (with its hedged fallback) when the file can't be read. Wire all four surfaces to it. Event Viewer / GetLastError-via-FFI were dead ends (lbug catches the failure — no crash event; no native FFI dep). * test(fts): exercise the structural classifier on real binaries (#2374) Add an integration suite that runs inspectExtensionBinary/diagnoseExtensionLoad against genuine binaries — the running node executable, the real lbugjs.node addon, and the installed FTS extension (valid); a truncated real binary and a real text file (corrupt). Registered in cross-platform-tests PLATFORM_LOGIC so it runs on the Windows + macOS matrix, proving the PE and Mach-O header parsing on real PE/Mach-O files (ubuntu covers ELF). * fix(fts): honor a corrupt_file verdict over a structurally-valid header (#2374) The structural probe in diagnoseExtensionLoad inspects only the first 4 KB, so a download truncated after its header reads 'valid' and was routed to the "install VC++, reinstalling will NOT help" remedy — the exact loop #2374 exists to kill, for the truncated-download case the module docstring claims it handles. Honor the loader's own corruption report ("file too short" / Windows error 193 "not a valid Win32 application") before defaulting to the dependency remedy; localized corrupt tails stay hedged missing_dependency, preserving language-independence. Addresses PR #2383 review finding F1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(fts): return indeterminate for a PE header beyond the read window (#2374) The structural probe reads only BINARY_HEADER_BYTES (4 KB). A valid PE with a large DOS stub whose e_lfanew points past that window was wrongly called 'corrupt', routing a fine DLL to "re-download". A garbage e_lfanew from a truly corrupt file is indistinguishable from here, so widen the header verdict with 'indeterminate' and return it in that case; the caller then defers to the loader's own report instead of asserting a false verdict. Fat Mach-O stays valid (LadybugDB ships thin per-arch binaries). Also covers the unmapped-arch and garbage-PE-signature branches. Addresses PR #2383 review finding F1-secondary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(fts): drop contradictory reinstall guidance from the analyze degrade log (#2374) For a missing runtime dependency the extension file is present, so appending FTS_UNAVAILABLE_MESSAGE (which tells the user to install it "with network access") to the remedy ("reinstalling will NOT help") produced self-contradictory guidance on the main analyze surface. Lead the missing_dependency degrade log with the class-neutral sentence (FTS_UNAVAILABLE_LEAD) and append only the classified remedy; other classes keep FTS_UNAVAILABLE_MESSAGE unchanged. Addresses PR #2383 review finding F2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(fts): cache the load diagnosis so the degraded warning does no per-request I/O (#2374) ftsDegradedWarning() runs on every degraded /api/search response and MCP query, and it was calling diagnoseExtensionLoad — a synchronous openSync/readSync of the extension file — on every call. Compute the diagnosis once at mark-unavailable time (the single load-failure sink, run per Database not per request), cache it on ExtensionCapability, and have the warning read the cached result (falling back to the pure, no-I/O string classifier if it is absent). Loader capability-shape assertions relax from toEqual to toMatchObject for the new optional field. Addresses PR #2383 review finding F3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(fts): cover the missing_dependency remedy on the --repair-fts path (#2374) The repair-fts error interpolates the classified remedy, but no test reached the missing_dependency branch — only the corrupt/invalid-ELF path. Add a Windows error-126 case asserting the thrown error carries the VC++ redistributable remedy and omits the old "retry the network install" tail, and that no index is dropped. Addresses PR #2383 review finding F6a (--repair-fts surface). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(fts): share the VC++ redistributable install hint (#2374) The Microsoft Visual C++ redistributable name and aka.ms URL were duplicated verbatim in WINDOWS_MISSING_DEPENDENCY_REMEDY and STRUCTURAL_MISSING_DEPENDENCY_REMEDY. Factor a single VC_REDIST_INSTALL_HINT constant so the pointer cannot drift between them; the composed remedy strings are byte-identical (existing exact-text assertions unchanged). Also adds a test covering the previously-unexercised structural remedy branch. Addresses PR #2383 review finding F5a. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(fts): guard FILE_CORRUPTION_SIGNATURES parity with the installer script (#2374) The corruption-signature list is deliberately duplicated between extension-load-error.ts and scripts/install-duckdb-extension.mjs (the .mjs cannot import the .ts), with nothing guarding against drift — a one-sided edit would desync the FORCE-INSTALL verb from remedy classification. Export the array from both and add a parity test that compares regex source + flags element-wise. Addresses PR #2383 review finding F5b. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(test): run extension-binary-real in the sequential lbug-db vitest project (#2374) extension-binary-real.test.ts imports @ladybugdb/core but ran in the parallel `default` project, contrary to TESTING.md's rule that native-LadybugDB tests live in the sequential `lbug-db` project. Add it to the lbug-db include list and the default exclude list; it now runs under lbug-db and no longer under default. Addresses PR #2383 review finding F6c. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(fts): fail loud, not silent-skip, on missing FTS artifacts under REQUIRE_FTS=1 (#2374) The real-binary structural tests gated on raw .skipIf(!lbugNative) / .skipIf(!installedFts), so under GITNEXUS_REQUIRE_FTS=1 a missing artifact would silently vanish from a green CI run (the #2299 trap). These tests inspect the extension file directly and need its path, not a loaded connection — so skipUnlessFtsAvailable (which needs an initialized LadybugDB) does not fit. Add requireFtsResourceOrSkip: skip gracefully offline, throw under REQUIRE_FTS=1. The always-on process.execPath assertion still runs everywhere. Addresses PR #2383 review finding F6d. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(fts): apply prettier formatting to the #2383 fix files (#2374) Line-wrapping only; the quality/format CI check flagged three files. No behavior change. 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-07-06 21:27:37 +01:00
Gergő Magyar	177bbc89c3	fix: surface real FTS extension LOAD errors and self-heal broken extension files (#2374) (#2375)	2026-07-06 06:41:05 +01:00
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
Gergő Magyar	3f0c74fea0	fix(deps): upgrade @ladybugdb/core to 0.16.0 to resolve native segfaults (#1235) ... * fix(deps): upgrade @ladybugdb/core to 0.16.0 to resolve native segfaults Resolves the SIGSEGV / access-violation (0xC0000005) / exit-139 crashes that have been reported widely since 1.6.3. The native crashes originate in @ladybugdb/core 0.15.x — primarily during FTS index creation, VECTOR extension load, and concurrent query teardown — and are reproducible on Linux, macOS and Windows. The maintainer-confirmed fix is to bump the runtime to 0.16.0, which ships nodejs async + memory-management fixes, extension ABI bump, and macOS Intel binaries. Adopting 0.16.0 cleanly required three supporting changes; without them the upgrade itself regresses other paths: 1. maxDBSize must be passed explicitly. 0.16.0 keeps the upstream JSDoc note that the default 0 is "introduced temporarily for now to get around with the default 8 TB mmap address space limit some environment". Constrained CI runners and laptops cannot reserve 8 TB and crash with "Buffer manager exception: Mmap for size 8796093022208 failed." A new gitnexus/src/core/lbug/lbug-config.ts centralises a 16 GiB default (overridable via GITNEXUS_LBUG_MAX_DB_SIZE) and every Database() construction site now passes it. 2. enableCompression default flipped from false to true in 0.16.0. Every Database() call site is updated to pass false explicitly so existing GitNexus indexes keep the same wire format. 3. Bridge DB sidecar files (.wal, .shadow). 0.16.0 enforces a database-id check on .wal / .shadow sidecars and rejects opens whose sidecars belong to a different base name. writeBridge now (a) cleans the full sidecar set when removing the tmp slot, (b) renames .wal / .shadow alongside the main file during the atomic .tmp -> .lbug swap, and (c) wraps openBridgeDbReadOnly in a bounded retry on transient Win32-Error-33 lock errors. Eager db.init() / conn.init() forces the lazy native handle to surface lock contention at the retry site. Known limitation (not a regression): on Windows the 0.16.0 native binary does not release the OS file lock until the process exits, so the close-then-reopen-same-process pattern raises Error 33 after the first close. Production paths (analyze / serve / mcp each open the DB exactly once per process) are unaffected, but eight tests that exercise the pattern are guarded with a process.platform === 'win32' skip; CI's Linux + macOS shards exercise them as before. Tracking upstream: kuzudb/kuzu#3872 / #3883 / #4730. Closes #1136 #1154 #1160 #1162 #1178 #1195 #1196 #1199 #1204 #1206 Refs #1209 (supersedes — Dependabot bump without the supporting fixes) Made-with: Cursor * fix(test): isolate LadybugDB native test state Use per-suite LadybugDB databases in integration helpers so test forks do not reopen a database created by Vitest global setup, and centralize Windows-tolerant native temp cleanup for bridge tests. * fix(lbug): avoid bridge existence reopen Reuse the built LadybugDB config in the extension installer and avoid native close/reopen cycles when checking bridge existence on Windows. Made-with: Cursor * chore(docs): exclude local lbug plan Keep the refactor planning note out of the PR while leaving the ignored local copy on disk. Made-with: Cursor * refactor(lbug): centralize database construction Route LadybugDB opens through shared helpers so native constructor defaults stay consistent across core, pool, bridge, and extension install paths. Made-with: Cursor --------- Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>	2026-04-30 17:40:39 +01:00
Gergő Magyar	ffa0510f9a	fix(lbug): prevent DuckDB extension install hangs (#1129) ... * fix(lbug): bound DuckDB extension install via ExtensionManager (closes #1128) `gitnexus analyze` could hang indefinitely (60% / 85% on Windows) when DuckDB's `INSTALL fts` or `INSTALL VECTOR` was unable to reach `extensions.duckdb.org`. The DuckDB driver's INSTALL is a synchronous network call, so any blocked egress would block the Node event loop forever. Replace the ad-hoc, in-process INSTALL/LOAD scattered across `lbug-adapter.ts` and `pool-adapter.ts` with a single `ExtensionManager` that owns the lifecycle of optional DuckDB extensions: * `LOAD` is always tried first — per-connection, idempotent, no network. * If `LOAD` fails and policy permits, INSTALL runs in a short-lived child Node process bounded by `GITNEXUS_LBUG_EXTENSION_INSTALL_TIMEOUT_MS` (default 15s). The parent loop keeps spinning; on timeout the child is killed with SIGKILL and the capability is flagged unavailable. * Capabilities and install attempts are cached per process, so a single bounded install per extension covers every subsequent call. Install policy is now an explicit, per-context decision: * `auto` (default for analyze) — try LOAD, fall back to bounded INSTALL. * `load-only` — used by `pool-adapter` (serve / MCP read paths) so user queries never block on a network install. * `never` — operator escape hatch for offline / airgapped environments. `createFTSIndex` and `createVectorIndex` now check the boolean return value before issuing the index DDL, so missing extensions degrade BM25 and semantic search gracefully without ever throwing during analyze. Tests: - New unit suite for `ExtensionManager` covering LOAD-first behavior, all three policies, install caching, observability, and warn dedup. - Existing vector-extension integration tests pass against the new boolean return type. - Existing embedding-pipeline mocks updated to return `true`. Docs: `gitnexus/README.md` documents `GITNEXUS_LBUG_EXTENSION_INSTALL` and `GITNEXUS_LBUG_EXTENSION_INSTALL_TIMEOUT_MS` with examples for offline and slow-network environments. Made-with: Cursor * fix(lbug): move DuckDB extension install child into script Keep the bounded out-of-process INSTALL behavior, but replace the inline child code with a stable packaged ESM script. This makes the child process directly runnable and gives debuggable stack traces without source-vs-dist branching or a runtime transpiler. Made-with: Cursor	2026-04-27 23:09:17 +01:00