620 commits

Author	SHA1	Message	Date
Gergo Magyar	5869dde31d	fix(embeddings): make HTTP generation resumable (#2468)	2026-07-16 10:00:50 +00:00
Gergo Magyar	e814c5a10d	fix(embeddings): include File rows in the incremental delete sweeps ... The zero-symbol File fallback from #2455 writes File embedding rows, but the filePath-scoped delete sweeps joined through EMBEDDABLE_LABELS only. Docs repos accumulated duplicate rows on re-analyze and deleted files left orphans. Free for code repos: no File rows exist to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-16 10:00:26 +00:00
Gergo Magyar	de5321ed24	fix(embeddings): fall back to text-bearing file nodes (#2455)	2026-07-16 09:58:38 +00:00
Gergo Magyar	ee7161ef0d	fix(cache): degrade when the durable generation reset fails ... An fs failure while resetting a chunk generation now warns and continues like the neighboring durable-store paths instead of failing the analyze. Workers recreate the directory on write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-16 09:55:44 +00:00
Gergo Magyar	dc15de9dec	fix(cache): bound parsedfile generations (#2476)	2026-07-16 09:51:46 +00:00
Gergo Magyar	490e74a901	fix(scope): stabilize graph lookup collisions (#2480)	2026-07-16 08:55:23 +00:00
Gergo Magyar	193db519a5	fix(communities): canonicalize projection order (#2478)	2026-07-16 08:50:51 +00:00
Gergo Magyar	6b013e7d30	fix(scan): lock in Rust publish order and guard PHP suffix roots ... Adds the missing #2481 Rust regression test (importer before definer), fails closed when a PHP namespace suffix matches directories under different roots, and points the baselines note at #2481/#2482. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-16 08:42:07 +00:00
Eva	846b545680	fix(php): index import declaration directories	2026-07-16 09:58:33 +07:00
Eva	711a13c077	Merge remote-tracking branch 'upstream/main' into review/pr-2482	2026-07-16 09:54:37 +07:00
Eva	ff385a5472	Merge remote-tracking branch 'upstream/main' into upstream/community-order-2477	2026-07-16 09:51:57 +07:00
Eva	a6441b345c	Merge remote-tracking branch 'upstream/main' into upstream/parsedfile-generation-2475	2026-07-16 09:50:36 +07:00
Eva	1821b01dbe	fix(embeddings): bind resume checkpoints to provider	2026-07-16 09:48:03 +07:00
Eva	a117cf8a21	Merge remote-tracking branch 'upstream/main' into upstream/embedding-http-resilience	2026-07-16 09:44:17 +07:00
Eva	6eb82713f4	Merge remote-tracking branch 'upstream/main' into upstream/file-node-embedding-fallback	2026-07-16 09:40:29 +07:00
Eva	1f7036dbb0	fix(ingestion): canonicalize parse node insertion	2026-07-16 09:21:38 +07:00
Eva	ed3f58d292	Merge remote-tracking branch 'upstream/main' into review/pr-2480	2026-07-16 09:17:40 +07:00
Gergő Magyar	a05b501102	fix(cli): make Claude skills discoverable (#2434)	2026-07-15 20:40:20 +05:00
Eva	34955b57f6	fix(php): resolve symbol-named PSR-4 imports	2026-07-14 13:12:33 +07:00
Eva	5407747c67	fix(php): resolve function imports by declaring file	2026-07-14 12:46:59 +07:00
Eva	f0f316a7b2	fix(scan): canonicalize traversal without order-sensitive Rust binding	2026-07-14 12:21:40 +07:00
Eva	031a160090	fix(scope): stabilize graph lookup collisions	2026-07-14 04:02:22 +07:00
Eva	26d9bfe888	fix(communities): canonicalize projection order	2026-07-14 03:21:34 +07:00
Eva	3d6908ba4b	fix(cache): bound parsedfile generations	2026-07-14 03:17:42 +07:00
Eva	711ff8721d	fix(embeddings): make HTTP generation resumable	2026-07-14 02:15:57 +07:00
Eva	554c5181cf	fix(embeddings): fall back to text-bearing file nodes	2026-07-14 01:40:47 +07:00
Gergő Magyar	c6445096eb	fix: stop Napi::Error SIGABRT on analyze — index C++ type lookups, terminate workers only at JS-safe points (#2432) (#2436)	2026-07-11 18:07:08 +01:00
Gergő Magyar	accf61c672	fix(tree-sitter): recover declarations after embedded NUL bytes (#2430) ... * fix(tree-sitter): recover from embedded NUL bytes Normalize embedded NUL bytes only in parser input so tree-sitter keeps recovering through the full source shape. Pass the file label through the worker for diagnostics and cover both direct-string and callback parse paths with regression tests. * fix(review): align safe-parser contract count * test(tree-sitter): cover worker NUL diagnostics (#2430)	2026-07-11 08:33:50 +01:00
VL	117587d543	fix(cli): actionable diagnostics for non-4K page-size buffer manager failures (#2424)	2026-07-10 19:12:53 +01:00
Gergő Magyar	df1fc36094	fix: make large incremental writebacks commit reliably (#2409) (#2425)	2026-07-10 14:05:23 +01:00
Parafee41	dbc73adcf1	fix: surface incremental dirty state diagnostics (#2410) ... * fix: surface incremental dirty state diagnostics * address incremental dirty diagnostics review * stabilize windows analyze e2e timeout --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-07-09 13:28:26 +01:00
azizur100389	f236be05e0	feat: gate Icebug community engine prototype (#2376)	2026-07-09 05:43:49 +01:00
Gergő Magyar	5f4964b4e6	fix: resolve imported/composed FastAPI route path constants (#2391) (#2393) ... * feat(routes): add pure Python string-constant resolver (#2391 U1) * feat(routes): extract Python module constants from tree (#2391 U2) * feat(routes): capture non-literal FastAPI decorator args + per-file constants, bump parse-cache schema (#2391 U3) * feat(routes): resolve composed decorator route constants in parse-impl + skip floor (#2391 U4) * feat(routes): resolve composed FastAPI route constants in group HTTP-contract layer (#2391 U5) * test(routes): multi-hop, ingestion↔group parity, and warm-cache regression locks (#2391 U6) * docs(routes): mark the language-agnostic seam for cross-language const resolution (#2391) * refactor(routes): extract language-agnostic constant-fold core; Python becomes a binding (#2391) The fold, cycle guard, and depth cap now live in constant-resolver.ts and take a pluggable ImportResolver. python-const-resolver.ts supplies the Python import semantics + tree extractor and re-exports the same surface, so no call site changes. A Spring/Kotlin/C# binding can now reuse the core with its own resolver (proven by constant-resolver.test.ts driving it with a Java-style resolver). * fix(routes): treat the constant-fold cycle guard as a recursion stack (#2391) The `visited` set in `foldName` was added-to but never removed on unwind, so a constant referenced more than once in a single fold — `A + A`, a reused separator (`SLASH + PATH + SLASH`), or a diamond `X = P + Q` where P and Q share a base — tripped the cycle guard on its second occurrence and the whole route was silently dropped by the skip floor. Pop the guard in `finally` so it tracks the ACTIVE resolution stack, not every name ever seen: a true cycle (a name still on the stack) is still caught, but a name that already resolved and popped folds again. Re-computation stays bounded by MAX_RESOLVE_DEPTH, so no blowup is reintroduced. Locked in constant-resolver.test.ts (A+A, reused separator, shared-base diamond); the pre-existing real-cycle and depth-cap cases still return null. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): make module-constant binding writes mutually exclusive (#2391) `extractPythonModuleConstants` kept `literals`, `exprs`, and `imports` as three independent maps: `setName` cleared literals+exprs but never `imports`, and an import never cleared a prior literal/expr. Since `foldName` checks literals > exprs > imports regardless of source order, a name that was both imported and locally (re)assigned kept both bindings and the wrong one won — `from .c import ROUTE; ROUTE = os.getenv(...)` resolved the STALE import instead of dropping, a confidently wrong route path (the exact skip-floor invariant this feature is meant to uphold). Treat the three maps as one logical namespace: any write to one clears the other two for that name (via `imports.delete` in `setName` and a `bindImport` helper), so last-binding-in-source-order wins, matching Python. An import both imported and dynamically rebound now drops. Folding `+=`/`+` onto an imported base remains deferred (it drops safely, never a stale value). Locked in python-const-resolver.test.ts: dynamic-rebind drops, literal-shadows- import, import-shadows-literal, and `+=`-on-import drops. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): widen the group cost-gate to catch literal-leading concats (#2391) `NONLITERAL_ROUTE_DECORATOR_RE` required the first decorator argument to START with an identifier, so a string-literal-leading concat like `@router.get("/api" + SUFFIX)` never tripped `hasComposedRoute`. When such a route was the ONLY composed shape in a repo, the group layer left `constantsByFile` empty and dropped the route, while the ingestion side (which has no gate) resolved `/api/users` and emitted a Route node — an R4 provider/graph parity break. Widen the gate to also fire on a string-literal-leading `+`-concat, detected by a `+` before the closing paren on the decorator line. Gating on the `+` (not merely a leading quote) keeps a plain literal route `@router.get("/x")` OFF the gate, so a literal-only repo still pays no parse pass. Locked in fastapi-composed-provider.test.ts: a sole literal-leading concat now resolves (parseCalls>0 + provider emitted), plus previously-uncovered `@app.<verb>(CONST)` EXPR-branch resolution; the literal-only no-parse gate case still passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): correct package-init and over-deep relative import resolution (#2391) Two edges in `resolvePythonImport`: - `from . import X` (empty module after the dots) resolved to a sibling `<dir>.py` instead of the package `<dir>/__init__.py`. Resolve the bare-package case to `__init__.py`. - An over-deep relative import (more extra dots than the importing file has directory levels) silently clamped `dirOf('')` to `''` and could match an unrelated root-level `<name>.py` — a wrong file. Guard with `walk > depth → null` so an import that escapes above the repo root drops (skip floor). Both preserve the exact-match / ambiguity→null behavior for ordinary relative and absolute imports. Locked in python-const-resolver.test.ts: `from . import` → `__init__.py` (and null when absent), and an over-deep import returns null even when the clamped target file exists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): bound parseConstOperands recursion depth (#2391) `parseConstOperands` recursed on `binary_operator` children with no depth bound. A stack overflow is not currently reachable (tree-sitter caps expression nesting below the JS stack limit, so it throws on a deep `+`-chain before this runs), but add a depth guard (cap 64, mirroring the fold engine's MAX_RESOLVE_DEPTH) as defense-in-depth: a pathological chain now floors to null (skip) rather than relying on tree-sitter's limit. The `depth` parameter defaults to 0, so all existing callers are unaffected. Locked in python-const-resolver.test.ts: a 100-term `+` chain yields no binding (null) instead of throwing; ordinary short chains still fold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(routes): read each .py once in buildPythonRepoContext (#2391) The group repo-context builder read every `.py` file from disk twice: once in the `include_router` cross-file pre-pass and again in the #2391 constant cost-gate loop — an unconditional 2x read on every Python repo, on every group extraction. Hoist a single read pass that populates one `pyContents` map (and computes the composed-route cost gate); both the include_router pre-pass and the constant-map pass now consume the cached content. Behavior-preserving — a literal-only repo still does one read and zero parses. Covered by the existing group unit + integration suites (R4 parity and include_router prefix joins unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(routes): tidy constant-resolver docs and declaration order (#2391) Three no-behavior nits from the PR #2393 review: - Name `conditional_expression` (`x if c else y`) in the `parseConstOperands` jsdoc list of shapes that deferred to null. - Move `NONLITERAL_ROUTE_DECORATOR_RE` above `buildPythonRepoContext`, which references it — it read as a forward reference before (runtime-safe, but confusing). - Correct the integration-test comment that called `/v2/api/v1/widgets/get` "ingestion-only garnish": the group side emits it too (asserted separately); the four paths in that block are the shared-parity set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(routes): fold `X += "…"` onto an imported base constant (#2391) Previously `from .c import BASE; BASE += "/v1"` dropped (the extractor could not represent "the imported prior value" as an operand without self-referencing X and tripping the cycle guard). Preserve the imported prior under a synthetic `$imp$N` key — `$` can never appear in a Python identifier, so it cannot collide with a real name — and reference it, so the augmented assignment folds to `<imported BASE>/v1`. Extractor-only: no change to the `Operand` type, the fold core, or the cache shape, so no SCHEMA_BUMP. An imported base that is itself unresolvable still drops (skip floor preserved — never a wrong path). Locked in python-const-resolver.test.ts: single and chained `+=` fold onto an imported base; an unresolvable base still drops. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(routes): resolve bare decorator constants via the by-name entry (#2391) The group `resolveExprArg` hand-built `[{ kind: 'ref', name }]` and called `resolveOperands` for a bare-constant decorator argument — exactly what the language-agnostic core's `resolveConstant(file, name, repo)` seam does. Call it directly for the identifier case. This gives the previously test-only by-name entry point a real production caller (it is the documented reuse seam for future JVM/other bindings), drops the synthetic operand construction, and lets the now- unused `Operand` type import go. Behavior-identical — the `+`-concat path still parses to an operand list and folds via `resolveOperands`. Guarded by the existing group provider suite (bare-constant and concat cases). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(routes): parse each .py once in buildPythonRepoContext (#2391) The repo-context builder ran two parse loops — the include_router prefix pre-pass and the #2391 constant-map pass — so an include_router file in a composed repo was tree-sitter-parsed twice. Merge them into a single pass that parses each `.py` at most once and feeds both extractions from the same tree; a file that needs neither pass is still not parsed at all (cost gates unchanged). Complements the earlier single-read-pass change (this is the single-parse counterpart). Behavior-preserving (prefixes, R4 parity, and cost gates verified by the group + integration suites). Locked with a parseCalls assertion: a file needing both passes is parsed once, not twice. Note: a cross-run (cross-process) constant-map cache — the other deferred perf idea — remains out of scope; it needs disk persistence + invalidation and would add hashing/IO cost on the common path, so it fails the minimal-change bar this single-parse dedup meets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): bound constant-fold work and output to prevent OOM (#2391) The `finally`-popped cycle guard (recursion-stack semantics) correctly folds diamonds/repeated refs, but popping the guard removed the accidental work cap the old seen-ever set provided: a wide shared-descendant DAG re-folds each child once per reference, and a self-multiplying concat (`X = A + A; A = B + B; …`) builds a genuinely exponential string. Reviewers reproduced ~16.8M folds escalating to `RangeError: Invalid string length` and heap OOM — and neither fold call site is wrapped in try/catch, so it crashed the whole phase rather than dropping the route. Two complementary bounds, both flooring to null (skip), never a wrong value: - a never-popped `memo` in `foldName` caps recomputation at O(nodes) (successes only — a null may be transient on a cyclic branch); - a `MAX_FOLD_LENGTH` (8192) cap in `foldExpr` drops a fold whose output grows past any real route path, bounding the string size the depth cap does not. Corrects the prior "≤ 2^8 folds" comment (output grows multiplicatively, not additively). Locked with a 64^4-fanout construction that now drops in ~ms instead of OOMing; diamonds/cycles/depth-cap behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): snapshot assignment RHS refs at the assignment line (#2391) `ROUTE = BASE` was stored as a lazy `ref(BASE)`, resolved against BASE's FINAL binding. So `ROUTE = BASE; BASE += "/v1"` (or `ROUTE = API; API = "/other"`) resolved ROUTE to the MUTATED value — a confidently wrong path, since Python assigns by value at the `ROUTE =` line. This was latent for local constants at the base of this feature and the `+=`-on-import work extended it to imports. Snapshot each assignment/`+=` RHS reference to a bound name into that name's current frozen value at the assignment line (`freeze`/`snapshot`): a literal value, a copy of the current expr (whose refs are already frozen), or an import preserved under a `$imp$N` alias. Unbound refs (forward references) stay lazy. A later rebind of the aliased name can no longer change the earlier binding. `freeze` also unifies the previous `currentOps` + inline import-alias logic. Locked in python-const-resolver.test.ts: aliased-import-then-`+=`, aliased-local-then-`+=`, aliased-local-then-rebind all resolve to the pre-mutation value; normal reference chains still fold. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): fold group identifier args via resolveOperands for parity (#2391) Resolving a bare-constant decorator arg through `resolveConstant` entered `foldName` at depth 0, whereas the ingestion side folds `routePathOperands` through `resolveOperands([{ref}])`, entering at depth 1. At the MAX_RESOLVE_DEPTH boundary the group tolerated one more hop than ingestion, so a deep alias/re-export chain resolved in the group provider set but dropped from the graph Route nodes — an R4 parity break. Restore the operand-list path in the group so both subsystems share identical fold-entry depth. (`resolveConstant` reverts to the documented agnostic-core seam.) Locked in constant-resolver.test.ts: a 4-hop chain that `resolveOperands([ref])` drops but `resolveConstant` resolves, documenting why the group must use the operand-list entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): match multiline literal-leading concats in the cost gate (#2391) `NONLITERAL_ROUTE_DECORATOR_RE` used `[^)\n]*` so it only saw a literal-leading `+`-concat when the `+` was on the same line as the opening quote. A Black-formatted `@router.get(\n "/api"\n + SUFFIX\n)` therefore failed the gate, and when it was the only composed route in a repo the group dropped it while ingestion (which parses the tree, not the raw line) resolved it — an R4 parity break. Drop the `\n` exclusion: `[^)]*` spans the wrapped argument but stays bounded by the decorator's own closing paren, so a plain literal route still never trips the gate. Locked in fastapi-composed-provider.test.ts with a multiline concat fixture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): bump SCHEMA_BUMP for changed extractor output + E2E snapshot lock (#2391) `extractPythonModuleConstants` now emits DIFFERENT `moduleConstants` for the same source (binding mutual-exclusivity clears stale imports; RHS refs are snapshotted; `$imp$N` aliases). That output is cached verbatim in the parse cache, so a warm shard built at the pre-fix version would replay stale — in one case actively wrong — folded values, and the correctness fixes would silently no-op on upgrade. Bump SCHEMA_BUMP 11→12 to force re-extraction (same warm-cache-replay class the original 10→11 bump addressed for the field addition). Also adds the first end-to-end coverage for the new behavior through the real ingestion pipeline: app/snapshot.py aliases a constant (`SNAP = API_V1`) then mutates the source (`API_V1 += "/mutated"`), and the test asserts the Route node is `/api/v1`, never `/api/v1/mutated` — a case the pure-function unit tests covered but the pipeline did not. 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-07 13:23:05 +01:00
Gergő Magyar	b98f6e458f	fix(lbug): recognize Windows missing-shadow error so serve repo-switch recovers (#2382) (#2387)	2026-07-07 06:03:38 +01:00
Gergő Magyar	a7a5ea65a6	fix: report custom HTTP embedding endpoint failures instead of huggingface download errors (#2385) (#2386)	2026-07-06 22:06:08 +01:00
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	fbffa96554	fix(lbug/mcp): exact symbol content + 0-based line storage with 1-based MCP display (#2377, #2379) (#2380) ... * fix(lbug): store exact symbol content snippets * fix(ingestion): emit 0-based line numbers for COBOL/JCL/scope/markdown nodes COBOL/JCL processors, the scope-graph emitter, and the markdown Section emitter stored 1-based startLine/endLine, unlike every tree-sitter node (0-based). The exact-content slice (#2379) then dropped each symbol's declaration line for those languages. Convert to 0-based at the graph-node emission boundary via toZeroBasedLine — leaving parser-internal .line values, L${line} node/edge IDs, and containment checks untouched. Refs #2377, #2379 * refactor(lbug): single source of truth for symbol-content labels Extract SYMBOL_NODE_LABELS so the exact-content label set can't drift the way the inline copy did in #2379. csv-generator derives EXACT_SYMBOL_CONTENT_LABELS from it; manifest-extractor's near-identical allowlist is left behavior-unchanged (intentional subset, #2325-test-locked) with a documented cross-reference. Refs #2379 * test(ingestion): cover 0-based emitter output and pin exact-content slicing - csv-pipeline: replace the blank-buffer fixture (a +/-1 shift silently passed) with directly-adjacent neighbors; add one-line-symbol and Section (+/-2 fallback) cases. - cobol resolver: assert COBOL Module and JCL job/step emit 0-based startLine. - markdown CRLF: update Section startLine/endLine expectations to 0-based. Refs #2377, #2379 * feat(mcp): present 1-based line numbers in context/query/impact tools GraphNode startLine/endLine are stored 0-based (tree-sitter rows), which surprised users querying them (they don't line up with editors/sed). Add toDisplayLine and apply it at the context/query/impact response boundaries so line numbers are editor/sed-aligned. Raw cypher stays 0-based (documented in the schema resource); BasicBlock/PDG statement lines (already 1-based) and internal join params are left untouched. Refs #2377 * test(mcp): assert 1-based tool exposure with raw cypher staying 0-based context() reports startLine+1 (editor/sed aligned); a raw cypher RETURN of the same node keeps the stored 0-based value. Guards against double-conversion and leaking the display shift into raw results. Refs #2377 * fix(mcp): stop query() double-converting BM25 line numbers bm25Search applied toDisplayLine to its result rows, and query()'s aggregation loop applied it again, so BM25-matched symbols reported lines shifted +2 (stored 0-based 41 read as 43, not 42) while semantic-matched symbols were correct. bm25Search is called only from query(); return raw 0-based rows and let the single aggregation-loop conversion handle both retrievers. Adds a query() BM25 regression test asserting stored 41 -> 42 (would be 43 if double-converted), which the prior mcp-line-display test — covering only context()+cypher — never exercised. (#2380, #2377) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): use ?? not || so first-line symbols keep their line number `sym.startLine || sym[4]` treated a legitimate 0-based startLine of 0 as absent, so context()/query() dropped startLine/endLine for every symbol on line 1 of its file — every COBOL Module (toZeroBasedLine(1) = 0) and markdown h1. `??` only falls through to the positional fallback on null/undefined, preserving a real 0. This also repairs the rename definition-edit path, which consumes context()'s value. Adds a context() first-line (startLine:0 -> 1) assertion. (#2380, #2377) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): make group/cross-repo trace line numbers 1-based consistently A group/cross-repo trace presented 1-based endpoints (via resolveSymbolForGroup) but 0-based hops (tagHops copies port.trace output verbatim), so one response mixed bases. Wrap the trace port adapter (traceForGroup) to convert hop lines to 1-based too, matching the endpoints. Single-repo trace dispatches directly (not through this port) and stays 0-based — full single-repo parity is a tracked follow-up. core/group stays display-agnostic (no mcp import). Extends the cross-trace e2e test to assert hops share the endpoints' base (checkout 10 -> 11, getUsers 1 -> 2). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): present explain/pdg_query anchor line 1-based resolveBlockAnchor converted its ambiguous-candidate lines to 1-based but left the resolved-target anchor raw 0-based, so the same tool reported two bases depending on whether the target was ambiguous. Convert the display anchor to 1-based via toDisplayLine. The BasicBlock join param (symStart: sym.startLine + 1) is untouched — it targets the 1-based BasicBlock id space, not display. Asserts the resolved anchor is 1-based (targetFn stored 10 -> 11). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): bump schema + PDG result versions for the line-number change The 0-based storage flip for COBOL/JCL/markdown/scope (#2377/#2379) changed on-disk line semantics, and the PDG result startLine is now 1-based (#2380). Neither shipped a version bump, so an incremental re-analyze would preserve old 1-based rows (mixed-base index rendered one line too high) and PDG consumers got no signal. - INCREMENTAL_SCHEMA_VERSION 5 -> 6 (forces a one-time full re-analyze) - PDG_RESULT_VERSION 1 -> 2 (result-shape discriminator) Updates the version-pinning tests, the pdgResultVersion result type, and the tools.ts PDG output-contract doc. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): guard manifest label list against SYMBOL_NODE_LABELS drift manifest-extractor's CUSTOM_CONTRACT_RESOLVE_QUERY hand-lists the contract-resolvable labels as a deliberate subset of the shared SYMBOL_NODE_LABELS, guarded only by a comment — the same drift class (#2379) the shared-set refactor eliminated elsewhere. Derive the query's label set and assert it is a strict subset whose difference is exactly {Namespace, Variable, Module}, so adding a symbol label without a conscious manifest decision fails. Query string stays literal (#2325-test-locked). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(mcp): document which tools present 1-based vs 0-based line numbers The schema-resource note listed only context/query/impact as 1-based. After the trace/anchor fixes it now enumerates the full set — context, query, impact, group/cross-repo trace, and explain/pdg_query anchors are 1-based; raw Cypher and single-repo trace stay 0-based (full single-repo-trace parity is a tracked follow-up); BasicBlock/PDG statement lines are separately 1-based. (#2377, #2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): pin impact() line-value display (close the coverage gap) The prior mcp-line-display test only asserted context() + raw cypher, which is why the query() double-conversion (#2380) shipped green. Adds an impact() line-value assertion via the ambiguous-candidate path (the only impact response that surfaces a per-candidate line): two same-name symbols force ambiguity and the candidate at stored 0-based 41 must read 42. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): fix stale rename #2283 mock after 1-based context display rename resolves its symbol via context(), which now presents startLine 1-based (#2377), then subtracts 1 to recover the 0-based file index. The #2283 mock stored startLine:1 but put `oldName` on the file's line 0, so after the 1-based shift the definition edit no longer matched and the write-failure path never fired — the test read 'success' instead of 'partial'. Align the mock content to its stored line (oldName on 0-based line 1). Pre-existing failure surfaced once ubuntu/coverage completed on this branch. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): consolidate line-display tests into one shared DB block The query()/BM25 case had spun up a second full LadybugDB + FTS setup; fold it into the single existing block (adding FTS + the Zqxwvbm seed there) so the file builds one DB, not two. Trims per-file setup cost — relevant to the Windows platform-sensitive suite's under-load 15-minute timeout. Same five assertions, all green. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: kigland <shuaizhicheng336@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-06 16:16:45 +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	cdad478c96	fix: proxy-blocked installs survive onnxruntime-node postinstall and self-heal embeddings (#2370) (#2372)	2026-07-05 16:15:10 +01:00
Gergő Magyar	e46b87f291	feat: flat workspace index follows the checked-out branch (#2364) ... * feat: flat workspace index follows the checked-out branch (#2354) A plain `gitnexus analyze` now always targets the flat workspace slot, updating it incrementally across branch switches instead of auto-routing non-owner branches into `branches/<slug>/` sub-indexes (disk bloat) or nagging with the primary-inversion "run gitnexus clean" warning. No new CLI flag or config key: the smart behavior is the default. - Placement: only explicit `--branch` consults resolveBranchPlacement; plain runs resolve to the flat slot, `meta.branch` becomes an informational "last analyzed branch" label restamped each run. - Fast path: a same-commit clean-tree branch flip restamps the label and registry entry (adoptFlatBranchLabel, no-op for unregistered repos). - Shadow cleanup: when the flat slot adopts a label that has a pinned sub-index, the now-unreachable `branches/<slug>/` dir and its registry summary are removed together. - MCP: applyBranchScope always falls back to the on-disk flat meta before throwing "not indexed", so long-lived servers resolve a freshly restamped workspace branch. - status: no more "current branch not indexed" dead end — falls through to the workspace index with an informational line and the usual commit-based staleness verdict. - Deleted primaryInversionWarning; explicit `--branch` pinning, the checkout-mismatch guard, detached-HEAD/CI behavior, and `clean --branch` are unchanged. Supersedes the flag-based approaches in #2358/#2359. Closes #2354. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): check registry before deleting shadowed sub-index (#2364 review F2) adoptFlatBranchLabel ran the branches/<slug>/ rm before its own unregistered-repo no-op check, so a repo in the #2264 half-finalized state (up to date but unregistered) lost its pinned sub-index on a same-commit branch flip while the run still failed. The registry lookup now precedes the deletion, making the no-self-heal rule (#2264/#1169) cover disk as well as registry state. The 'never self-heals' unit test now materializes a sub-index dir and asserts it survives; the run-analyze #2354 fast-path test registers its repo under an isolated GITNEXUS_HOME (deletion is only legitimate for registered repos) with a new unregistered variant pinning dir survival. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): keep branch summary when sub-index rm fails (#2364 review F4) The shadow-cleanup fs.rm swallowed every error while the registry summary was dropped unconditionally. On Windows an lbug held open by a live MCP server fails the rm with EBUSY/EPERM, and once the summary is gone 'clean --branch' can never target the leftover dir (it resolves solely via the recorded summary) — stranding the exact un-cleanable disk bloat adoptFlatBranchLabel exists to prevent. The summary is now dropped only when the directory is verifiably gone (post-rm existence check); on failure the summary is retained, a warning names the path and errno, and the informational branch label still restamps. Later adopts retry the rm. New repo-manager-rm-failure.test.ts uses the delegating fs/promises mock idiom (vi.spyOn cannot intercept ESM namespace exports). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): restamp fast path adopt-first and tolerate read-only storage (#2364 review F3) The fast-path label sync stamped meta before adoptFlatBranchLabel, so a crash or adopt failure between the two flipped the retry guard (existingMeta.branch !== branchLabel) and locked in the partial state: every subsequent same-commit run skipped the cleanup and branch-scoped queries kept routing to the stale pinned sub-index. The block also sat outside any try/catch, so a same-commit branch flip on a read-only .gitnexus mount (the documented Docker :ro workflow, #1549) failed a byte-for-byte-current analyze over a purely informational label sync. Adopt now runs first and saveMeta last — any partial failure leaves the guard true and the next run self-heals — and the whole sync is best-effort: read-only errors warn citing #1549, anything else warns and retries next run. Safe because the block only fires on a same-commit clean tree, where the flat DB content is byte-valid for both labels. isReadOnlyFilesystemError is now exported. New run-analyze-adopt-failure.test.ts covers retry-after-partial- failure, adopt-before-stamp ordering, and EROFS/EACCES/EPERM (gaps 4 and 7); a detached-HEAD fast-path pin lands in run-analyze.test.ts (gap 6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): make flat meta authoritative in applyBranchScope (#2364 review F1) applyBranchScope trusted two pieces of cached state before its flat- meta disk fallback, and the handle cache only refreshes on a resolve miss — never on a hit. Post-#2354 that stale window is the routine case: (i) the handle.branch early-return served the flat handle under the OLD label after a workspace flip, silently returning the new branch's content as the old branch (the pool staleness reinit hot- swaps content without updating handle.branch); (ii) a stale cached branches[] summary routed to a branches/<slug>/ dir that adoptFlatBranchLabel had already deleted (raw 'LadybugDB not found' or POSIX ghost reads with staleness detection blinded). The on-disk flat meta is now read before any cached-state trust. A branches[] summary is served only when its sub-index lbug actually exists (the lbug is what the pool opens — serviceability truth); the cached label is trusted only when no readable flat meta contradicts it (#2106 R4 legacy shapes preserved). One refreshRepos() fires on detected staleness so subsequent calls see fresh handles. Safe against mid-analyze reads: dirty stamps spread the existing meta, preserving the old label until the end-of-run atomic write. Fixtures now materialize the pinned sub-index lbug; new regressions cover the stale-old-label error, adopted-summary fall-through to flat, and the dangling-summary partial-failure window (test gaps 1-2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): make end-of-run branch-label sync best-effort (#2364 review F5) The end-of-run adoptFlatBranchLabel sat inside the pipeline try whose catch rethrows, so a registry write failure (ENOSPC, ~/.gitnexus perms) after a successful multi-minute analyze failed the whole run — even though the index was complete and registered, the neighbouring parse-cache save is deliberately wrapped for exactly this reason, and adopt retries unconditionally on the next plain analyze. It now warns and continues, mirroring the parse-cache wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): correct branch-not-indexed guidance for workspace index (#2364 review F6) The error told users to 'Run: gitnexus analyze --branch <X>', but post-#2354 that command hard-errors unless X is checked out — and this message is now the common goodbye for a formerly-indexed branch whose sub-index the workspace slot adopted. The guidance now explains that the workspace index follows the checked-out branch and leads with the checkout; the '(primary only)' fallback becomes '(workspace only)'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: align primary/workspace vocabulary with the #2354 inversion (#2364 review F7) The review flagged pre-inversion 'primary/non-primary' wording that now misleads readers about the placement model: the isPrimaryBranch JSDoc (field name kept — public API surface), the two branches? JSDoc comments in local-backend, the base_ref gate comment in cli/analyze, and four branch-scope test names. Comment/JSDoc/test-name edits only; 'Registry-primary' and 'primary key' senses untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): clarify workspace index status wording (#2364 review F8) 'gitnexus analyze follows this branch' was ambiguous about WHICH branch analyze follows — the recorded one on the line or the current checkout. Both locales now say a re-run follows the current branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): re-read registry after the shadow rm in adoptFlatBranchLabel The F2 reorder moved the registry read to the top of the function, so the whole-file writeRegistry at the bottom persisted a snapshot taken BEFORE the recursive rm of an entire sub-index — widening the unlocked read-modify-write window from microseconds to the duration of a multi- hundred-MB delete. A concurrent registerRepo/removeBranchIndex writer in that window was silently clobbered (the #2106 R9 lost-update class; registerRepo re-reads before writing for exactly this reason). The top read is now a cheap membership gate only (the F2 no-op guarantee); the mutate re-reads its own fresh snapshot after the rm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: treat only provably-absent errno as gone in the new existence probes Both probes added by this series inverted the codebase's provably- absent polarity (listRegisteredRepos validate prunes only on ENOENT/ENOTDIR): adoptFlatBranchLabel's dirGone check read ANY fs.access failure — including a transient EACCES/EIO on a surviving dir — as 'verifiably gone' and dropped the summary, recreating exactly the stranded-bloat bug F4 fixed; applyBranchScope's sub-index check read the same transient errors on a healthy pinned lbug as 'adopted/ deleted', producing a false 'not indexed' error. A resolved force:true rm now proves absence without a probe; on failure the probe treats only ENOENT/ENOTDIR as gone, and a non-missing lbug serves the handle so the pool open surfaces the real error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): harden applyBranchScope stale-state coherence Four residual gaps in the new arm structure, found by post-fix review: - The stale-label error listed the just-contradicted cached label as indexed ('not indexed: main. Indexed branches: main'). The message now derives the flat label from the authoritative meta and excludes the requested branch from the hint list. - A branch pinned AFTER the server cached its handle never triggered a refresh (resolve hits skip the miss-refresh), erroring until restart. Every miss now fires exactly one best-effort refreshRepos() before the error, so the next call resolves; a refresh-once guard keeps doubly-stale resolutions to a single registry re-scan. - A registry entry claiming the branch both as flat label and pinned summary (the rm-failed adopt-degraded state) could serve the stale- vintage pin under a label the flat slot owns; the summary arm now requires handle.branch !== branch and the degraded state errors honestly. - The flat-meta match path returned the cached handle's pre-restamp branch/commit/stats; the meta that decided routing now also supplies the metadata. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): keep the real error visible in restamp warnings; correct the end-of-run retry claim The fast-path catch replaced the actual error with 'storage is read-only (#1549)' for any EACCES/EPERM — mislabeling ownership problems and transient Windows locks and discarding the only diagnostic signal. The warning now carries the real message with the #1549 hint appended. The end-of-run best-effort comment claimed adopt 'retries unconditionally on the next plain analyze'; same-commit runs take the fast path whose guard compares the already-stamped meta label, so the retry actually lands on the next content-changing run. The comment now states the true retry semantics and why the interim state is safe (flat meta stamped first; applyBranchScope trusts it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: unique tmpdir for the branch-scope fixture; drop redundant dynamic imports The branch-scope describe materialized its sub-index stub under a FIXED os.tmpdir()/gnx-2106-multi path — concurrent vitest runs on one host (the documented parallel-agents workflow) could rm each other's stub between beforeEach and the resolve under test, flaking the pinned-branch tests. The fixture root is now mkdtemp-unique per run with afterAll cleanup. run-analyze.test.ts dynamically imported repo-manager inside test bodies despite the module being statically imported at the top of the file (no vi.mock exists there to justify it); the three call sites now use the static import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-03 20:55:27 +01:00
Livio Gamassia	d546fa3cce	fix(storage): rename index metadata to gitnexus.json with dual-write compatibility (#2363)	2026-07-03 19:32:59 +01:00
ChunxueLi	1029a8ddd7	feat: add Spring DI resolver for @Autowired List<T> injection (#2200) ... * feat: add Spring DI resolver for @Autowired List<T> injection Addresses all P0/P1 findings from tri-review (#2200): - P0: Register INJECTS in RelationshipType union (compiles) - P0: Rewrite execute() to emit consumer→implementation edges from graph data only - P1: Register in VALID_RELATION_TYPES, single-pass O(N) indexes - P1: Java-only gate with early exit on non-Java repos - P1: Update FULL_ORDER golden test - 8 unit tests covering all edge cases * test: make VALID_RELATION_TYPES size assertion array-driven (no hardcoded count) The security test hardcoded toBe(16) for the relation type count, but PR #2200 added INJECTS, bumping it to 17. Replace the magic number with an EXPECTED_RELATION_TYPES array whose .length drives the size assertion, so future additions only need to append to the list. Fixes CI failure on PR #2200. * fix(ingestion): thread raw generic field types onto Property nodes so Spring DI matching works (review 4616076037 P0) Production declaredType is generics-stripped by design (extractSimpleTypeName: List<Shape> -> "List"), so the spring-di phase's anchored regexes could never match real extraction output — the phase was a silent no-op on every real Java repository, while its unit tests passed against hand-built node shapes. Add FieldInfo.rawDeclaredType captured verbatim from the field's type node (.text, generics and qualifiers preserved — same precedent as the JVM method extractor), thread it through both parse-worker Property sites, add it to the shared NodeProperties contract, and match on rawDeclaredType ONLY (no declaredType fallback: it can never match real data and would mask future plumbing regressions as quiet no-ops). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): gate Spring DI on real injection annotations, honest edge reason (review 4616076037 P1) Extract Java field annotations (shared extractAnnotations helper, moved verbatim from the method extractor) onto Property nodes and require @Autowired or @Inject before a collection field becomes an INJECTS candidate. Previously every edge's reason string fabricated "@Autowired" without any annotation ever being checked, and any plain collection field would have fanned out false edges once matching worked. @Resource is deliberately excluded: JSR-250 resolves by bean name first (defaulting to the field name), injecting a single named collection bean — the opposite of the collect-all-implementers fan-out INJECTS models. Pinned by a test. An annotated candidate missing rawDeclaredType now logs an isDev warning (plumbing-contract breach signal) instead of vanishing silently. SCHEMA_BUMP 9 -> 10: Property nodes gained rawDeclaredType + annotations; warm parse caches must invalidate or the DI phase silently no-ops on replayed pre-upgrade nodes (the #2038 trap). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(ingestion): framework-neutral di phase + language-scoped Spring matcher registry (review 4616076037 P1) spring-di was the only pipeline phase naming a language in shared core/ingestion code (DoD.md language rule; the maintainer's direction is a generic DI solution). Split it: - di-extractors/spring.ts: the Spring matcher (annotation gate, collection type parse, @Resource exclusion rationale, framework-specific reason payload) — language-scoped home, mirroring route-extractors/. - di-extractors/index.ts: DI_MATCHERS, a single-valued ReadonlyMap<SupportedLanguages, DiFieldMatcher> mirroring the SCOPE_RESOLVERS registry shape sanctioned by AGENTS.md. Constructor injection deliberately out of scope; widen to arrays only when a second same-language framework lands. - pipeline-phases/di.ts (renamed from spring-di.ts): framework-neutral — routes Property nodes to registered matchers by node language via a typed guard, then runs the unchanged reverse-index fan-out. Zero language or framework names remain (grep-verified). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): language- and qualified-name-scoped interface resolution for DI fan-out (review 4616076037 P2) The interface index was built from ALL Interface nodes regardless of language, keyed by bare simple name with last-writer-wins overwrite — a polyglot repo with a TS and a Java 'Shape' could fan Java INJECTS edges into TypeScript classes, and two same-named Java interfaces in different packages silently collapsed to whichever parsed last (documented GitNexus bug class: #2054, PR #1956). Resolution is now per-language with qualifiedName as the primary key (Interface nodes already carry package-qualified qualifiedName); dotted element types resolve via qualifiedName, bare names via a per-language simple-name index that records ambiguity and fails CLOSED. Ambiguity skips are observable: DIOutput.ambiguousSkipped + an aggregated isDev debug log, so 'no DI fields' is distinguishable from 'all candidates ambiguous'. Same-package tiebreaking is a pinned, documented follow-up. Order-independence pinned by running collision tests in both insertion orders. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): depth-aware Spring collection-type parser for idiomatic generics (review 4616076037 P3) The two anchored regexes silently skipped idiomatic Spring shapes: Map<Pair<A,B>, IFoo> (nested-generic key broke the [^,]+ split), List<? extends IFoo> / List<? super IFoo> (bounded wildcards), java.util.List<IFoo> (qualified wrapper), and whitespace/multi-line declarations. Replace them with a small scanner: whitespace normalization, wrapper matched by last dotted segment, depth-aware top-level-comma split, wildcard bound stripping, and a final plain-dotted-type-name gate so anything else (nested-generic elements, arrays, unbounded wildcards, embedded comments, unbalanced brackets) fails closed. Every accept and reject is documented in the module docstring and pinned by 27 table-driven cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(integration): prove Spring DI end-to-end through the real pipeline (review 4616076037 P1) Both no-op incarnations of this feature shipped with a green unit suite because every test hand-built the exact graph shape the phase expected — no test ever ran real Java source through the actual extraction pipeline. Add test/integration/spring-di-pipeline.test.ts: real .java fixtures via runPipelineFromRepo, pinning (a) the extraction contract on the annotated field's Property node (declaredType 'List', rawDeclaredType 'List<IFoo>', annotations ['@Autowired']), (b) set-equality on ALL INJECTS edges (exactly Consumer->FooA and Consumer->FooB; the non-annotated 'plain' field of the same type contributes nothing; no self-edges), and (c) a negative-control fixture with no injection annotations producing zero INJECTS edges. Either historical regression fails at least one of these. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(incremental): register INJECTS across product surfaces + delete-before-writeback (review 4616076037 P2) INJECTS was allowlisted in VALID_RELATION_TYPES but invisible or unhandled everywhere else. Register it deliberately: - REL_TYPES (gitnexus-shared schema-constants): web-side validRelType() otherwise silently rejects INJECTS filters (CLI/web single source of truth). - mcp/tools.ts cypher edge list (agent-facing schema discovery). - isGraphWideRelType: INJECTS validity is a whole-program property — a change to a THIRD file (the interface, or a new/removed implementer) creates/invalidates edges between two untouched files (the TAINT_PATH / #2084 M4 U6 class), so incremental extraction must always re-include the full fresh set. - deleteAllInjects (lbug-adapter): mirrors deleteAllInterprocTaintPaths — COUNT-then-DELETE under withConnLock, benign missing-table carve-out, re-throw otherwise (CodeRelation has no PK and there is no read-side dedup; a fail-soft delete + re-add would silently duplicate rows). - run-analyze.ts: the delete is UNCONDITIONAL, next to the Communities delete — deliberately NOT inside the options.pdg block: the di phase runs on every persisting analyze while the graph-wide re-include is unconditional, so a pdg-gated delete would append without deleting on every non-pdg incremental run (N runs = N copies). - local-backend.ts comment: opt-in traversal by design (not in default impact()/context() lists; no IMPACT_RELATION_CONFIDENCE entry per the WRAPS/FETCHES precedent — edges carry their own 0.8). - ARCHITECTURE.md: 14 -> 15 phases, DAG diagram, phase table, skip-list. Note: the tools.ts edge list also predates WRAPS/QUERIES/USES — that drift is pre-existing and left for a follow-up. Idempotency pinned end-to-end: two successive incremental runs (real runFullAnalysis + real LadybugDB, unrelated-file touches) leave the INJECTS row count stable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: describe INJECTS' actual precondition; drop stale fixed-at-16 comments (review 4616076037 P3) The shared-schema doc for INJECTS claimed an @Autowired precondition the code (pre-fix) never checked, and hardwired Spring semantics into what is now a framework-neutral edge type. Reword: precondition is an injection annotation recognized by a per-language matcher in di-extractors/; framework specifics live in the reason payload, not the type contract. security.test.ts comments still said the allow-list size 'stays fixed at 16' (it is 17 and the assertion derives from EXPECTED_RELATION_TYPES). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: simplify DI surfaces — narrow matcher contract, dedup delete-alls, derive tools edge list Post-implementation simplification pass (4 review angles): - DiFieldMatch/CandidateField carried collectionType + matchedAnnotation that no consumer read (the matcher bakes both into reason) — narrowed to {elementTypeName, reason}. - parseElementTypeName had two guard branches fully subsumed by the final plain-dotted-type-name gate — deleted, rationale folded into the regex comment. - The three byte-identical delete-all-by-rel-type functions in lbug-adapter (TAINT_PATH / CALL_SUMMARY / INJECTS) are now one parameterized helper + thin wrappers with identical names, signatures, and message text (character-diff verified) — the missing-table regex and abort policy now live in exactly one place. - The cypher tool's hand-maintained edge-type list (already missing WRAPS/QUERIES/USES) is now derived from the canonical REL_TYPES — the drift class is gone rather than patched. - di phase: interface indexes are built only for languages that actually have candidates; test builder gained a rawDeclaredType opt-out replacing a hand-rolled node. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: apply Tier-2 review findings — qualified-name fail-closed, honest cypher docs, pinned delete contract, hook isolation - byQualifiedName was last-writer-wins on duplicate qualified names (reproduced: order-dependent INJECTS edges with ambiguousSkipped 0 — same package+interface duplicated across monorepo modules/source roots; Java qualifiedName has no file-path component). Both indexes now share the AMBIGUOUS fail-closed sentinel; order-flip test added. - The REL_TYPES-derived cypher edge list advertised pdg-gated types with no caveat (LLM queries on them silently return zero rows on default indexes) — caveat appended, INJECTS example added, impact relationTypes description now names the DI fan-out opt-in. - The delete-all re-throw contract (only defense against duplicate CodeRelation rows) was untested — error classification extracted to a pure classifyDeleteAllError and pinned exhaustively. - extractRawType/extractAnnotations hooks lacked the per-hook try/catch the pipeline applies elsewhere (#2286 pattern): a throwing hook would silently drop every remaining file in the language group. Hardened, degradation tested. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-02 17:49:46 +01:00
Gergő Magyar	fa8ebf672e	fix: Java cast-wrapped and this.method() call edges (#2357) ... * fix: resolve Java cast-wrapped and this.method() call edges Two fixes for missing call edges in Java method resolution: 1. compound-receiver.ts — cast expression handling: - Strip (Type) cast wrappers from receiver text, tracking the outermost meaningful cast type - Resolve directly to the cast type class (not the field's declared type), since the cast narrows the receiver type - Add this.field chain walker for field-access receivers - Replace text → workingText throughout the function body 2. scope-resolver.ts: - Enable resolveThisViaEnclosingClass: true for Java (activates Case 0.5 in receiver-bound-calls.ts) Verified on a large-scale Java codebase with no regressions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(scope-resolution): format compound-receiver.ts with prettier (#2353 review F10) Mechanical prettier --write from repo root — 6 brace-expansion sites and one ternary re-join, zero logic changes. Clears the quality/format CI failure that was blocking CI Gate on PR #2353. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(scope-resolution): pin working Java cast-receiver shapes (#2353 review F3) Fixture-backs the cast resolutions PR #2353 gets right — simple cast, nested/CFR cast, cast over this.field, and the deliberate declared-type fallback for a resolvable-shape cast to an unindexed type — each with a same-named decoy method on the receiver's declared type so later refactors cannot silently regress them. No resolver changes; tests are green as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scope-resolution): resolve nothing for unparseable cast types (#2353 review F1) A receiver paren-group that is type-shaped but unparseable — generic (List<String>), array (Foo[]), fully-qualified (com.example.Foo) — is a cast whose type cannot be looked up. Stripping it and falling through resolved the pre-cast expression's own declared type, emitting a confident wrong CALLS edge. Classification is now three-way per peel: simple identifier → capture (outermost wins), type-shaped-unparseable → resolve nothing (pre-#2353 behavior; noise casts after a captured type still win), anything else → not a cast, text left untouched. Cast candidates require a non-empty trailing expression, so plain parenthesized receivers never capture a cast type. Red-first: all four shapes reproduced the wrong edge before the fix; golden digest byte-stable after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(scope-resolution): delete duplicate this.field walker, seed literal-this chain heads (#2353 review F4/F5/F7) A/B against the fixture corpus confirmed the generic per-segment walker (head resolved via the synthesized this typeBinding) already covers every method-body this.field chain — only initializer contexts (instance initializer block, field initializer) were walker-dependent, since no function scope exists there to carry a this binding. Deleting the duplicate walker removes the naive chainRest.split('.') (F5) and the widened fieldFallback use (F7) with it; the findEnclosingClassDef head seed is the deliberate residue covering initializer contexts — head-resolution only, the per-segment walk stays the single shared implementation. Post-seed edge set is byte-identical to pre-deletion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(scope-resolution): gate cast stripping behind opt-in stripReceiverCastExpressions (#2353 review F2) Cast handling in resolveCompoundReceiverClass now runs only for languages that opt in via the new ScopeResolver toggle (default off); Java is the sole opt-in. The peel loop is extracted into the pure, exported stripCastWrappers helper (placed with the file's other pure string helpers) so it can be unit-tested directly. Non-opting languages see receiver text untouched — pre-#2353 behavior by construction (golden digest unchanged, TS/C++/C# suites green, 796/796). Shared-code comments are language-neutral per AGENTS.md; the contract JSDoc carries the classifier grammar, the second-language escalation rule, and the Case 3b/Case 4 pass-through non-goal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scope-resolution): cap cast-peel iterations in stripCastWrappers (#2353 review F8) MAX_CAST_PEEL = 16 (each cast level costs at most two peels, so this covers 8-level nesting with headroom — real cast nesting, including decompiler output, is a handful of levels). Each peel rescans the working text for its matching close paren, so pathological nested-paren input was O(N²); the cap bounds it at O(N·16). Exceeding the cap bails all-or-nothing with the original text (not-a-cast outcome). Adds the helper's first unit tests: 14 scenarios covering capture, unparseable shapes, redundant-paren unwrap, captured-type precedence, rawName no-op, over/under-cap, and unbalanced-paren termination. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scope-resolution): revert Java resolveThisViaEnclosingClass, pin Case 4 bare-this dispatch (#2353 review F6/F9) Remove resolveThisViaEnclosingClass from the Java scope resolver: the toggle's own contract doc prescribes keeping it disabled where Case 4 (the synthesized this typeBinding) already handles this, and Case 0.5's C++-authored semantics (hiddenByName arity-hiding, method-before-field) provably bypass the interface-dispatch fan-out only Case 4 emits. A/B gate (new java-this-dispatch pinning fixtures): flag-off 7/7 green; flag-on 2/7 red (hiddenByName drops the this.greet overload site — masked by a free-call-fallback 'local-call' edge — and the interface-dispatch fan-out is missing). Corpus A/B over all 54 java-* fixtures: 2 fixtures differ — java-this-dispatch (reason 'local-call'→'global' on the bare-this overload site; +2 interface-dispatch fan-out edges flag-off) and java-this-field-chain (2 initializer-context bare-this ACCESSES reads emitted only by Case 0.5, which Case 4 cannot resolve — no synthesized this binding without a Function scope; the corresponding CALLS edges are unaffected via the F4 commit's literal-this head seed). Also (F9): insert Case 0.5 into the I4 case-order listings (contract + receiver-bound-calls header, now 8-case, marked gated) so the next flag flip is visible at review time; the two 'sole C++ language' comments are accurate again unedited. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scope-resolution): restrict literal-this head seed to initializer contexts (#2353 review follow-up) Final-review finding (two independent reviewer angles): the literal-this chain-head seed landed ungated in shared code, so any language's this-headed chain in a scope without a synthesized this typeBinding — including contexts where the language DELIBERATELY leaves this unbound (object-literal methods, nested plain functions) — would seed from the lexically enclosing class. isInitializerContext now permits the seed only when no Function scope sits between the site and its class, which is precisely the field-initializer / instance-initializer shape the seed exists for. Adds a TS guard fixture pinning that an object-literal method's this.field.method() chain emits no fabricated edge (mechanism did not empirically reproduce even ungated — the restriction is conservative hardening, and the pin keeps it that way). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(scope-resolution): attach stripCastWrappers JSDoc, fast-path non-paren receivers (#2353 review nits) Two final-review nits: a blank line detached the helper's 30-line classification-contract JSDoc from the declaration (IDE hover showed nothing at call sites); and the gate now skips the helper call plus result allocation for the majority of receivers that cannot be casts because they do not start with '(' — the helper's own check stays as the safety net. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * bench(scope-capture): rebaseline Java fingerprint for new #2357 fixtures The scope-capture correctness fingerprint hashes captures over the java-* fixture corpus; the three fixture dirs added by this PR (java-cast-receiver, java-this-field-chain, java-this-dispatch) extend that corpus, so the fingerprint moves. Verified purely additive: with the three new dirs parked, the fingerprint reproduces the prior baseline byte-identically — no emit/capture behavior changed. --check now passes for all 14 languages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: ww <ww@wwdeMacBook-Pro.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-02 17:19:33 +01:00
ACT900	5aada28da5	fix(embeddings): use system-matched onnxruntime-node CUDA build so CUDA 13 hosts use the GPU (#2341) ... * fix(embeddings): use system-matched onnxruntime-node CUDA build so CUDA 13 hosts use the GPU transformers.js exact-pins a CUDA-12 onnxruntime-node while gitnexus' own dep floats to a CUDA-13 build. npm/pnpm cannot dedupe an exact pin against a range, so npm i -g installs two copies and the gitnexus overrides block (root-only) is inert. On a CUDA-13-only host the nested CUDA-12 provider cannot load libcublasLt.so.12, the CUDA EP fails, and embeddings silently fall back to CPU (isCudaAvailable() also only probed .so.12). Add onnxruntime-node-resolver.ts (module.registerHooks redirect to the host-matching build, no-op elsewhere) mirroring onnxruntime-common-resolver.ts; probe libcublasLt .so.12 OR .so.13 against the copy that actually loads; unit test with 12 cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(embeddings): wire CUDA-13 build-match resolver into MCP query embedder The MCP query-time embedder (src/mcp/core/embedder.ts) has its own, separate initEmbedder() used for semantic search — it only called ensureOnnxRuntimeCommonResolvable() before importing transformers.js, so the CUDA-13 build-matching redirect added for the analyze/CLI embedder never applied here. A CUDA-13 host running MCP search with --embedding-device cuda still loaded the mismatched default onnxruntime-node build. Wire ensureOnnxRuntimeNodeMatchesSystem() into the same call site, mirroring the core embedder's ordering (registered after the common-resolver fallback, before the dynamic transformers import). * fix(embeddings): gate CUDA redirect decision on registerHooks availability decide() computed the CUDA-major redirect independent of whether Node's module.registerHooks API actually exists — only ensureOnnxRuntimeNodeMatchesSystem() checked that. On Node 22.0-22.14 (allowed by this package's engines floor; registerHooks needs >=22.15), isCudaAvailable() could therefore report a redirect target that ensureOnnxRuntimeNodeMatchesSystem() then silently failed to install, so transformers.js loaded the mismatched default onnxruntime-node build while the embedder still requested device:'cuda' against it — reintroducing the uncatchable native crash this probe exists to prevent. Move the registerHooks check to the top of decide() so the probe and the loader can never disagree, and skip CUDA-major probing entirely in that case (a redirect could never install anyway). Also fixes a related test-helper bug found while writing this unit's tests: loadResolver's destructuring default (`registerHooks = vi.fn()`) silently substituted a real mock function even when a test passed `registerHooks: undefined` to simulate Node < 22.15 — meaning the existing 'no-ops... when registerHooks is unavailable' test never actually exercised that path. Distinguish 'omitted' from 'explicitly undefined' via an 'in' check. * test(embeddings): drive decide() -> redirect:true and assert the resolve() closure The PR's actual shipped behavior — the installed registerHooks resolve() closure, and the full redirect-active decision path — had zero executed test coverage. All 4 prior ensureOnnxRuntimeNodeMatchesSystem tests avoided driving decide() into redirect:true because require.resolve/createRequire were never mocked, so the two-distinct-directory comparison decide() depends on always resolved against whatever's actually installed in this test's real node_modules (a single real copy, not the PR's two-copy scenario). Extend loadResolver()'s existing node:module mock to also fake createRequire, keyed by call origin, so resolveOurOrtNodeDir/resolveDefaultOrtNodeDir can be driven to two distinct fake directories with distinct CUDA majors — reaching redirect:true without adding any injection points to production code. Then capture the installed resolve() closure (mirroring the sibling onnxruntime-common-resolver.test.ts's captureResolve() pattern) and assert its three branches directly: onnxruntime-node redirect, onnxruntime-common redirect, and passthrough for any other specifier. * fix(embeddings): distinguish ldd detection-failure from no-CUDA-provider ortCudaMajor treated any execFileSync('ldd', ...) failure with no usable stdout (missing ldd binary, permission-denied .so, sandboxed exec) identically to 'CUDA provider genuinely absent'. The pre-PR detection (hasOrtCudaProvider) only used existsSync, never ldd, so this is a regression: a CUDA-12 host that worked fine before this PR can now silently fall back to CPU if ldd itself can't run, even though the provider .so and system CUDA libs are both genuinely present. readSoNeeded now reports whether ldd produced any usable output at all, distinct from 'ldd ran and just found no matching NEEDED entry' (the existing, already-handled '=> not found' case). When detection genuinely fails, log a warning so an operator can tell 'CPU fallback because detection itself failed' apart from 'CPU fallback because no CUDA build shipped' — the return value stays null either way (the type can't distinguish a third state), but the two cases are now observably different via the log. * fix(embeddings): check ourDir independently of whether defaultDir resolved decide()'s ourDir fallback lookup was nested inside 'if (systemMajor != null && defaultDir)', so a null defaultDir (transformers' own onnxruntime-node resolution failing outright, e.g. a partial/broken install) skipped checking ourDir entirely — getEffectiveOnnxRuntimeNodeDir() returned null even when gitnexus' own matching CUDA-13 copy would have resolved fine and worked. defaultDir resolving is not a precondition for the comparison: an unresolvable default already counts as 'the default doesn't match', so the ourDir check now runs whenever systemMajor is known, regardless of whether defaultDir resolved. * fix(embeddings): prefer CUDA 13 globally across the env-var directory scan detectSystemCudaMajor's CUDA_PATH/LD_LIBRARY_PATH scan returned on the first CUDA-major match within a single dir/sub pair, so a stale .so.12 found early (e.g. a leftover CUDA_PATH entry from a prior install) shadowed a genuine .so.13 found later in the search path, even though the scan's own ordering (checking 13 before 12 within each pair) was clearly intended to prefer 13 wherever possible. Keep scanning the full search space once a 12 is found, only returning early once a 13 is found (the best possible answer) or the space is exhausted. * fix(embeddings): have onnxruntime-common-resolver defer to the effective onnxruntime-node dir onnxruntime-common-resolver.ts independently re-derived transformers' default onnxruntime-node dir (its own copy of the 'resolve transformers' main entry, then onnxruntime-node' walk) to compute which onnxruntime-common to pair with — duplicating onnxruntime-node-resolver.ts's own walk, and capable of disagreeing with it: when the CUDA-major redirect is active, this hook would still pair onnxruntime-common with transformers' default (unredirected) onnxruntime-node, not the redirected copy the other hook just switched onnxruntime-node itself to. Have it call the already-exported getEffectiveOnnxRuntimeNodeDir() instead — the same decision the CUDA-major redirect hook uses — so both hooks always agree on which onnxruntime-node they're pairing onnxruntime-common against, and the duplicated resolve-walk is removed entirely rather than merely factored out. * fix(embeddings): cache the effective CUDA major to remove redundant subprocess spawns isCudaAvailable() in embedder.ts re-invoked ortCudaMajor/detectSystemCudaMajor directly even though decide() (via getEffectiveOnnxRuntimeNodeDir) had already computed both to make its redirect decision — a second, wasted ldconfig + up to 2 ldd spawns on every initEmbedder() call. Add effectiveMajor to the memoized Decision, computed once inside decide() alongside effectiveDir/systemMajor, and export a single isEffectiveCudaAvailable() that reads straight from the cached decision. embedder.ts's local isCudaAvailable() wrapper (and its now-unused getEffectiveOnnxRuntimeNodeDir/ortCudaMajor/detectSystemCudaMajor imports) is replaced by this one exported function. * fix(embeddings): surface CUDA redirect state at info level and in doctor A successful CUDA-build redirect logged only at logger.debug (filtered by the default 'info' level), and gitnexus doctor's embeddings section never mentioned the redirect at all — leaving no diagnostic path for 'why is my CUDA-13 host still on CPU' after this PR ships. Log the successful-redirect line at info (no-redirect/failure paths stay at debug, since those are the common, expected case). Add cudaRedirectDoctorStatus(), a pure summary of decide()'s already-computed decision mirroring doctor.ts's existing localEmbeddingDoctorStatus shape, and print it as a new literal (non-i18n) 'CUDA:' line in doctor's embeddings section alongside the existing 'Support:' line, matching that line's established convention. * test(embeddings): register onnxruntime-node-resolver.test.ts in the cross-platform subset The new test file guards on process.platform (linux/darwin cases) but was absent from cross-platform-tests.ts's PLATFORM_LOGIC list, which TESTING.md says platform-sensitive tests should be added to — so it never ran on the Windows/macOS CI matrix, only Ubuntu. Note: the sibling onnxruntime-common-resolver.test.ts has the identical, pre-existing gap (it predates this PR) — left as-is here, since fixing unrelated pre-existing test-registration debt is out of scope for this PR's own follow-up fixes. * test(embeddings): strengthen weak assertions, add garbled-output and CUDA_PATH coverage Three of the four ensureOnnxRuntimeNodeMatchesSystem tests only asserted 'doesn't throw' rather than a concrete outcome — including one literally named 'idempotent' that never asserted a call count on its own spy. Strengthen each to assert real outcomes (module stays functional after a no-op; spy call counts; return-value shape), while keeping the true install-once idempotency proof in the redirect-active test added earlier (this file's no-redirect scenario can't exercise it, since registerHooks is never called either way). Add the missing edge cases flagged in review: a CUDA_PATH-only fallback scan test (mirroring the existing LD_LIBRARY_PATH one), and garbled/ unrecognized ldconfig and ldd output cases for both detectSystemCudaMajor and ortCudaMajor, confirming neither falsely matches a CUDA major on unparseable input. Also parameterize the non-linux platform test across both darwin and win32 rather than darwin alone. Not changed: the process.env reassignment vs. Object.defineProperty 'inconsistency' flagged in review — process.env, unlike process.platform, has no getter-only restriction, so plain reassignment is already correct and switching it to Object.defineProperty would be unnecessary ceremony. * docs(embeddings): note the npm link/symlinked dev-checkout resolution caveat resolveOurOrtNodeDir/resolveDefaultOrtNodeDir anchor to this module's own real (post-symlink) location via import.meta.url, so a linked local dev checkout may resolve against its own node_modules rather than the consuming app's. Narrow, dev-only blast radius (regular npm/pnpm installs are unaffected) — document-only, no structural fix warranted. * fix(test): point the windowsHide spawn-family registry at the file that actually spawns hooks.test.ts's windowsHide regression check still listed gitnexus/src/core/embeddings/embedder.ts as a child_process-spawning file, but this PR itself already moved all execFileSync usage out of embedder.ts and into the new onnxruntime-node-resolver.ts — without updating this registry. The check was silently failing at the PR's own head commit (confirmed: 0 spawn-family calls found in embedder.ts, 'expected 0 to be greater than 0'), a pre-existing gap this fix-pass surfaced via a full-suite run rather than something introduced by any of the preceding follow-up commits. Swap the registry entry to onnxruntime-node-resolver.ts, which does import execFileSync (ldd + ldconfig, both already correctly passing windowsHide: true). * fix(test): make onnxruntime-node-resolver.test.ts path comparisons OS-agnostic Registering this file in cross-platform-tests.ts's PLATFORM_LOGIC (a prior commit in this series) means it now runs on the Windows CI matrix, not just Ubuntu — and several of the fakeDirs-based tests (redirect:true, ourDir-independent, subprocess-count, doctor-status) compared the resolver's real join()/dirname() output against hardcoded forward-slash fixture strings via exact-match or .startsWith(). Node's module is bound to path.win32 (or path.posix) based on the REAL host OS at process start — stubbing process.platform later, as these tests already do for the resolver's own platform branching, has no effect on it. So on a genuine Windows runner, join(effectiveDir, 'package.json') backslash-normalizes even under a faked platform:'linux', silently breaking every forward-slash comparison in this file: the createRequire dispatch would route to the wrong fake require, throw MODULE_NOT_FOUND, get swallowed by ensureOnnxRuntimeNodeMatchesSystem's outer try/catch, and registerHooks would never fire — the redirect-active tests would fail outright on Windows CI. Normalize every comparison point (the createRequire dispatcher, and the shared execFileSync/existsSync mocks) with a single toPosix() helper. Added a forceWin32Path test option (using path.win32's real join/dirname behavior) to prove this holds without needing an actual Windows runner — confirmed by temporarily reverting the fix and observing the new test fail with the exact predicted mismatch before restoring it. * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(embeddings): keep CUDA auto-detect working on Node < 22.15 when the default build already matches The registerHooks guard in decide() returned effectiveMajor: null unconditionally, so on Node 22.0-22.14 / 23.0-23.4 (engines floor is >=22.0.0) isEffectiveCudaAvailable() was always false and a CUDA-12 host whose default onnxruntime-node build already matched — which needs no hook at all to use the GPU — silently regressed from CUDA to CPU on the auto device path (pre-PR isCudaAvailable() behavior). Probe the system and the default copy regardless of registerHooks availability; only the ourDir redirect branch stays gated on it, so the probe still never reports a redirect target that cannot be installed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-07-02 07:53:32 +01:00
Parafee41	365de846d1	fix(lbug): retry single-writer transaction contention (#2342)	2026-07-02 06:17:12 +01:00
Gergő Magyar	35ebe37c42	fix(deps): pin Ladybug 0.18.0, validate the multi-writer deadlock fix (#2340) ... * chore(deps): bump @ladybugdb/core to 0.18.0 Pins the release containing LadybugDB/ladybug#605 (TransactionManager lock-order-inversion deadlock fix). Checked for known post-release regressions specific to 0.18.0 via the Ladybug issue tracker — none found. * fix(lbug): re-validate version-coupled comments and regexes for 0.18.0 Extends the LADYBUGDB-CONTRACT re-validation to two spots the marker convention doesn't catch (bridge-db.ts's LBUG_OPEN_RETRY_PATTERNS, conn-lock.ts's serialization rationale). Confirms via upstream source diff (v0.16.1..v0.18.0) that every matched error-text string is unchanged; conn-lock.ts's rationale is unaffected by #612/#623 since neither addresses concurrent queries on one connection. Adds a stemmer-sweep test proving the bundled 0.18.0 FTS extension accepts every entry in SUPPORTED_FTS_STEMMERS, not just the default porter. A live-trigger test for isMissingShadowSidecarError was attempted but abandoned after empirical probing showed it isn't reliably reproducible (even a SIGKILL-simulated crash didn't reproduce the error on reopen) — documented as inspection-verified instead of overclaiming test coverage that doesn't exist. * test(lbug): add concurrent multi-connection deadlock stress test (#2338) Directly validates LadybugDB/ladybug#605 — the TransactionManager lock-order-inversion deadlock between a commit()-triggered checkpoint and a concurrent beginAutoTransaction() — under a shape close to GitNexus's real concurrent-writer load, independent of conn-lock.ts's app-level serialization. Comparison run against 0.17.1 (pre-fix): 1 of 4 runs hung for the full 60s timeout, a direct reproduction of the deadlock. 9 consecutive runs against 0.18.0 (post-fix) all passed cleanly. Production is unchanged — conn-lock.ts still serializes every write; this test validates the engine-level fix without shipping multi-writer as a default. * fix(test): address code review findings in multiwriter deadlock test - Reuse lbug-config.ts's createLbugDatabase (via GITNEXUS_WAL_CHECKPOINT_THRESHOLD) instead of a hand-duplicated 9-arg raw constructor call whose stated justification (needing to bypass createLbugDatabase for the threshold override) was incorrect — the env var already provides it. - Close every QueryResult via the existing closeQueryResults helper (write loop, read loop, verify query, setup query) instead of leaking native cursors, matching lbug-adapter.ts's established pattern. - Move all cleanup (timers, connections, db close, env var restore) into the outer finally block so it runs on every exit path, not just the happy path — a timeout or a writer exhausting its retry budget no longer leaves dangling timers/connections/abandoned query loops. Verified: 8 consecutive runs after the refactor, all passing cleanly. Found via 8-angle parallel code review (medium effort); the two other findings (isDbBusyError not recognizing LadybugDB's 'Only one write transaction' message, and shadow-file poll timing sensitivity) are noted in the PR description as residual — the first is a production-code change beyond this validation test's scope, the second is inherent to observing a transient native sidecar file and not cleanly fixable without overengineering. * fix(test): apply ce-code-review autofix findings Fixes from an 8-persona parallel review round (correctness/testing/ maintainability/project-standards/reliability/adversarial/agent-native/ learnings): - Extract the duplicated skipUnlessFtsAvailable/FTS_UNAVAILABLE_NOTE helper (previously copy-pasted between lbug-core-adapter.test.ts and fts-stemmer-sweep.test.ts) into a shared test/helpers/fts-availability.ts. - Fix a native connection leak: verifyConn in the deadlock test's final verification block is now pushed into the readers array the outer finally already closes, so it's cleaned up even if the count query throws. - Fix a latent TypeScript type error (tsconfig.test.json catches it, tsconfig.json doesn't): conn.query() types as QueryResult | QueryResult[]; narrow to the single-result case before calling .getAll() rather than assuming the array branch never happens. - Replace repeated inline InstanceType<typeof import(...)> expressions with local LbugDatabase/LbugConnection type aliases. Verified: 12 consecutive runs of the deadlock test all pass, full lbug-db project (336 tests) green. Cross-reviewer-confirmed but left as residual (design judgment calls, not mechanical fixes) for the PR description: isDbBusyError doesn't recognize LadybugDB's 'Only one write transaction' message (pre-existing production gap, confirmed independently by 3 reviewers); the deadlock test's timeout path doesn't cancel in-flight writer/reader loops before closing connections; the reader loop has no bounded retry for transient errors during the race window; pinning @ladybugdb/core with a caret range trades automatic patch updates for less re-validation certainty. * docs: trim task-referencing JSDoc artifacts, add operator notes The U2 re-validation pass left verbose 'Re-validated on the 0.17.0->0.18.0 bump (#2338): ...' paragraphs stacked onto 5 production files' docstrings, alongside the already-updated version numbers. That narrative (SIGKILL-probe methodology, diff commands run, issue cross-references) belongs in the PR description, not in code comments that will accumulate a new paragraph on every future bump and confuse readers who just want the current fact. Trimmed each to state only the durable, current-state fact: - lbug-config.ts, sidecar-recovery.ts, lbug-adapter.ts, bridge-db.ts: dropped the bump-narrative paragraphs; kept only genuinely durable notes (e.g., which matchers are inspection-verified vs live-tested, what upstream wording changed). - conn-lock.ts: compressed a 12-line, 3-issue-number enumeration into 2 lines stating the current conclusion (no upstream 0.18.0 fix addresses the same-connection-concurrent-query risk this lock guards against). Also added operator-facing notes to GUARDRAILS.md and RUNBOOK.md's existing 'LadybugDB lock' sections: an isDbBusyError gap found during this validation (LadybugDB's 'Only one write transaction...' message isn't recognized by our busy/lock retry matcher) means that specific error can surface unretried. Documented so it's recognized as the same single-writer conflict, not a new failure mode. * refactor(test): use gitnexus-shared's withRetry in multiwriter deadlock test Replaces the hand-rolled writeWithRetry/sleep loop with the existing gitnexus-shared retry helper (already used by embeddings/hf-env.ts) instead of duplicating the pattern. * fix(test): guarantee non-zero retry delay in deadlock test's writer loop withRetry's isRetryable previously returned {retry: bool} with no afterMs, so computeBackoffMs's exponential-jitter formula gave a deterministic zero-delay on the first retry (floor(random()*1) is always 0 at attempt=0). This contradicted the file's own documented tuning, which specifically needs a non-zero 1-3ms delay to avoid tripping a different native guard. Return an explicit afterMs override on the retryable branch instead. * docs(test): remove dangling doc references from deadlock test JSDoc The JSDoc pointed to a local-session-only docs/plans/2026-07-01-001-... path (docs/ is repo-gitignored, so this never existed for anyone but the implementing session) and to "the PR description" as a source of truth that stops being current once the PR merges. Replace both with self-contained prose and durable references (issue/PR numbers, commit SHAs, GUARDRAILS.md/RUNBOOK.md) that stay resolvable after merge. * fix(search): harden SUPPORTED_FTS_STEMMERS against external mutation Type as ReadonlySet<string> to match this codebase's established convention for exported validation allowlists (EVAL_SERVER_TOOLS, STRUCTURAL_LABELS). Type-only change — no behavior change; both the internal .has() check and the sweep test's spread-iterate pattern continue to work unchanged. * docs(guardrails): fold Known-gap note into the LadybugDB Sign's Why label GUARDRAILS.md's own convention is strictly Trigger/Do/Why per Sign entry (stated in the file's header, followed by all 5 other entries). The new isDbBusyError gap note introduced a 4th label; fold it into Why instead, which is what it's actually explaining. * fix(test): run the multi-writer deadlock test on Windows too itLbugMultiwriter mirrored lbug-core-adapter.test.ts's win32 skip, but that pattern exists for a close-then-reopen-same-path lock lingering bug (kuzudb/kuzu#3872). This test never reopens the database — it holds connections open for the whole run — so the skip excluded the one test validating issue #2338's deadlock fix from the platform conn-lock.ts actually ships native bindings for. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>	2026-07-01 18:35:57 +01:00
Gergő Magyar	400cc6a440	feat(search): add opt-in CJK bigram segmentation for FTS search (#2339)	2026-07-01 16:41:41 +01:00
Parafee41	316aaed928	fix(indexing): keep full text file content searchable (#2323) ... * fix(indexing): keep full text file content searchable * fix(indexing): flush CSV chunks by byte size * fix(fts): flatten newlines/tabs in indexed content so multiline files are searchable (#2317) The end-to-end FTS test (review follow-up F2) exposed that removing the 10KB cap alone does NOT fix #2317: Ladybug's FTS tokenizer splits ONLY on the space character — \n, \r, and \t are not delimiters. So multiline file/symbol content indexes as a few giant cross-line tokens that no word query matches; full content is stored but stays unsearchable. (Verified: identical 8KB content is fully searchable when space-separated and entirely unsearchable when newline-separated.) The existing fts-description-search test never caught this because all its seed content is single-line. Collapse \r\n\t -> single space in the FTS-indexed text (extractContent's File and snippet content, plus the description column) via normalizeFtsText. This rewrites the stored column too, so File content returned via the graph API is space-flattened — an accepted trade for making file/symbol text searchable. Add the real end-to-end guard test/integration/fts-fullfile-search.test.ts: write a >16KB file, load it through the real streamAllCSVsToDisk -> COPY -> createSearchFTSIndexes path, and assert searchFTSFromLbug returns a needle past 10KB (plus a short-content no-regression and a stored-cell-not-truncated guard). It drives the COPY path a Cypher-seed test would bypass, reusing withTestLbugDB's FTS-availability gating via a new before-FTS load hook. * docs(lbug): note the deliberate File-unbounded / snippet-capped asymmetry The File branch returns full content (whitespace-normalized for FTS, bounded upstream by the walker cap) while the symbol snippet path 11 lines down stays MAX_SNIPPET-capped. Comment the intent so the uncapped File branch doesn't read as a forgotten guard. No behavior change. * test(lbug): update #2203 overlap round-trip for FTS whitespace normalization The newline/tab→space normalization (a170915a, #2317) flattens stored File content, so the #2203 overlap test's "File content == original multiline source" assertion no longer holds. The test's actual invariant — overlap path == serial path, byte-for-byte — is unchanged and still asserted; BasicBlock text (not FTS-indexed) still round-trips raw. Update only the File-content expectation to the whitespace-flattened form and document why. * fix(lbug): collapse CSV flush to a single byte threshold BufferedCSVWriter flushed on row-count (FLUSH_EVERY=500) OR byte-count (FLUSH_BYTES=8MB) — two independent triggers for one job. Byte count is the only one tied to the actual risk (an unbounded buffer.join('\n') string), so drop FLUSH_EVERY and make shouldFlushCSVBuffer single-arg. Rather than tune FLUSH_BYTES by guesswork or expose it as an env knob, derive its safety margin from constants the codebase already hard-enforces: a single row is capped at TREE_SITTER_MAX_BUFFER (32MB, clamped regardless of GITNEXUS_MAX_FILE_SIZE) and at most doubled by escapeCSVField's quote-escaping, so the worst-case joined chunk (FLUSH_BYTES + 2 * TREE_SITTER_MAX_BUFFER ≈ 72MB) sits >7x under Node's MAX_STRING_LENGTH (~512MB) — the ceiling that throws RangeError: Invalid string length. A new test pins that margin numerically so it can't erode unnoticed, which covers the "configurable" alternative better than a knob would: there's no evidence any deployment needs a different value, and an unbounded env var would let an operator silently walk the margin back into the danger zone. Also updates the two tests tied to the removed row-count path: the FLUSH_EVERY-boundary integration test now crosses FLUSH_BYTES with real oversized File content instead of relying on row count, and the shouldFlushCSVBuffer unit test drops to the new single-arg signature. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-07-01 08:27:09 +01:00
Gergő Magyar	905b7dfa21	feat(embeddings): compact, description-forward embedding text (#2333) (#2334)	2026-07-01 07:37:16 +01:00
Parafee41	f5a2e6a248	fix(search): make vector distance threshold configurable (#2330)	2026-07-01 05:37:46 +01:00