4 commits

Author	SHA1	Message	Date
Gergő Magyar	990d79ba8c	fix(mcp): make impact/context reproducible — deterministic ordering on every capped query (#2787) (#2796)	2026-08-02 17:03:15 +00:00
azizur100389	24584297d2	fix(trace): add file disambiguator alias (#2705)	2026-07-27 05:05:14 +01:00
Gergő Magyar	7f7255aef8	fix(analyze): load VECTOR before the incremental writeback touches embedding rows (#2623) (#2624) ... * feat(lbug): add ensureEmbeddingRowDmlSafe VECTOR gate for embedding-row DML LadybugDB refuses every mutation of a table carrying an HNSW index while the VECTOR extension is not loaded on that connection: DELETE and CREATE raise a Binder exception, DROP TABLE is refused while the index references it, and SET segfaults the process. Dropping the index is not an available recovery either — CALL DROP_VECTOR_INDEX is itself a VECTOR-extension function and is undefined in exactly that state. Add a single primitive that loads VECTOR under the analyze install policy and, only when that fails, reads CALL SHOW_INDEXES (which works without the extension) to decide whether an index actually exists to trip over. No call sites yet. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the #2623 VECTOR gate for embedding-row DML Three cases: no index + VECTOR unavailable stays safe (no needless escalation); index present + VECTOR unavailable is reported blocked AND the raw deleteNodesForFiles genuinely throws 'extension is not loaded' (proving the hazard is real, not theoretical); index present + VECTOR loadable is safe, the delete works, and the HNSW index survives — the invariant run-analyze relies on when it keeps the index across a surgical incremental run. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): load VECTOR before the incremental writeback touches embedding rows Incremental analyze died on every content change once a repo had built code_embedding_idx: Analysis failed: Binder exception: Trying to delete from an index on table CodeEmbedding but its extension is not loaded. The surgical writeback's first statement is deleteNodesForFiles' CodeEmbedding join-delete, but nothing on that path loaded VECTOR until Phase 4 — so the engine refused the delete. This is an ordering defect, not an environment one: it reproduces on machines where VECTOR loads fine. The dirty-flag recovery then forced a full rebuild on the next run, which is why it read as 'just slow'. Call ensureEmbeddingRowDmlSafe() once, before the escalation gate and before any row is touched — the same 'index lifecycle before row DML' seam dropSearchFTSIndexes occupies for FTS (#2589). Unconditional, because a DB carrying the index from an earlier --embeddings run hits the same wall on a plain incremental run. When VECTOR truly cannot load the table is immutable (the index cannot be dropped without the extension either), so the run falls through to the existing wipe-and-COPY escalation with a message naming cause, consequence and remedy. Fixes #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): pin the #2623 VECTOR-before-embedding-DML ordering end-to-end Sibling of the #2589 FTS drop-before-delete suite, same shape: drive the real runFullAnalysis incremental path over a real git repo and a real LadybugDB, seed real embedding rows, build the HNSW index, then assert the index state at the exact moment deleteNodesForFiles is invoked. Both cases were confirmed to discriminate — with the run-analyze change reverted they fail with the reported 'Trying to delete from an index on table CodeEmbedding but its extension is not loaded', and pass with it: - surgical path: the run completes, the index is still present AND extension_loaded at delete time, exactly one row per nodeId survives, and the untouched file's rows are preserved - blocked path: with GITNEXUS_LBUG_EXTENSION_INSTALL=never the run escalates to a full DB write and says so, instead of crashing Also applies prettier's reindent to the run-analyze log ternary. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(lbug): cite the pinned LadybugDB version in the #2623 probe note The probe matrix behind ensureEmbeddingRowDmlSafe was first recorded on 0.18.0, but gitnexus/package-lock.json pins 0.18.2 (#2587). Re-ran every case on 0.18.2: refused DELETE, refused CREATE, SIGSEGV on SET, DROP_VECTOR_INDEX undefined, DROP TABLE refused, SHOW_INDEXES readable with extension_loaded intact. Identical on both, so the design is unchanged — only the citation was wrong. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): preserve embeddings across the VECTOR-blocked rebuild, and check the catalog before loading Three follow-ups from reviewing the fix itself. 1. Data loss on the blocked path. Escalating wipes the DB files, and Phase 3.5 restores embedding rows from cachedEmbeddings — which deriveEmbeddingMode only populates when meta.stats.embeddings > 0. A DB holding embedding rows that its meta does not account for therefore had every vector destroyed silently by a rebuild it never asked for. Probe on a 3-file repo: 3 rows before, 0 after, no warning. Read the rows before escalating (a plain MATCH, no extension needed) so the existing restore has something to restore, and say so in the log. The blocked-path test now asserts the seeded rows survive exactly once, and that assertion fails without this rescue. 2. Catalog before extension. ensureEmbeddingRowDmlSafe loaded VECTOR first and only read SHOW_INDEXES on failure, so every incremental analyze on a machine without VECTOR paid a bounded out-of-process INSTALL attempt plus an 'extension unavailable' warning — including repos that never built an embedding index and can never hit this bug. One local catalog read settles that case first; the load is attempted only when an index actually gates DML, or when the catalog cannot be read. 3. Dead branch. targetConn is always the module singleton there, so the isSharedSingletonConn ternary could never take its second arm. Collapsed to withConnLock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(doctor): live-probe the VECTOR extension instead of printing the static platform capability Review finding on #2624 (MEDIUM), and exactly what #2623's reporter hit: doctor printed 'VECTOR index: available' — derived from a static platform check — while every incremental analyze on the same machine was dying on an unloaded VECTOR extension. The FTS line was switched to a live LOAD probe for the identical contradiction under #2374; VECTOR now gets the same treatment. probeVectorExtensionLoad shares the FTS probe's implementation (bounded, offline-safe, never runs the installer) and doctor's semantic-mode line now follows the probe, not the platform: without a loadable extension the vector index can be neither built nor queried, so search really is on exact scan. The load-error classifier's remedies are label-parameterized so the VECTOR row stops dispensing FTS-specific advice — 'run analyze --repair-fts' repairs FTS indexes only and was actively wrong for a missing vector extension. Default label stays 'FTS'; every existing caller and pinned remedy string is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lbug): remove the stale Windows VECTOR gate — the extension ships for win_amd64 The codebase categorically refused VECTOR on Windows (platform !== 'win32' in isVectorExtensionSupportedByPlatform, plus a hard early-return in loadVectorExtension) on the strength of an early-era report that in-process INSTALL VECTOR could SIGSEGV (#1365). That belief is stale, verified directly: - the extension server hosts win_amd64 VECTOR artifacts for every 0.18.x extension version — v0.18.0 and v0.18.1 both serve a real 14 MB PE32+ DLL (curl-probed; 'file' confirms PE32+ x86-64) - the pinned 0.18.2 core resolves its extension directory to 0.18.1 (strace-verified LOAD open()), so the pinned version's Windows artifact exists too - INSTALL now runs in a spawned child (installDuckDbExtensionOutOfProcess), so even a crashing installer kills only the child and degrades to unavailable — the original hazard cannot reach the parent process any more Windows now takes the same runtime path as every other OS: try LOAD, install out-of-process when policy allows, degrade to exact scan when it truly fails. The MCP semantic-search lane loses its static platform gate too — it always attempts the vector index and falls back to the exact scan on runtime failure, with a once-per-backend diagnostic naming the real error instead of a platform-policy message. isVectorExtensionSupportedByPlatform is deleted; getRuntimeCapabilities reports the platform capability as available everywhere and defers machine truth to the live probe. Windows CI is the enforcement: the vector suites skip visibly only when the extension genuinely cannot load, so green Windows lanes now actually exercise VECTOR instead of silently skipping by policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the catalog-read-failure fallback in ensureEmbeddingRowDmlSafe Review finding on #2624 (LOW): the one branch where the gate cannot cheaply prove safety — SHOW_INDEXES itself erroring — was exercised only by inference. Force it with a Connection.prototype.query spy over the real DB: the catalog read fails, and the gate must fall through to actually attempting the extension load (asserted via the recorded statement stream) rather than guessing, returning true here because the extension is loadable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): load VECTOR on the pool's shared Database so the semantic vector lane actually works Review finding on #2624 (MEDIUM): extension load scope is per-Database (probe-verified — LOAD on one connection enables QUERY_VECTOR_INDEX on every connection of the same Database), and the pool pre-warm loaded only FTS. So LocalBackend's vector lane has ALWAYS raised 'Catalog exception: function QUERY_VECTOR_INDEX is not defined' through the pool and silently fallen back to the exact scan — repos above the 10k exact-scan cap got empty semantic results. The serve path was unaffected (the embedding pipeline loads the extension itself). Mirror the FTS line at BOTH load sites — doInitLbug's pre-warm and initLbugWithDb's external-Database adoption — under the same load-only contract (the read pool never triggers a network install), tracked by a new SharedDB.vectorLoaded flag reset where ftsLoaded resets. The new pool test is discriminating and deliberately closes the writable core adapter before the pool opens: a shared/injected Database would inherit the VECTOR load from test seeding and pass either way, so the case forces the pool onto its OWN fresh read-only Database where only the pre-warm can make the lane legal. Verified: fails at the pre-fix tree with the exact Catalog exception, passes with the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run the #2623 ordering suite on Windows/macOS and pre-install VECTOR alongside FTS Two review findings on #2624, both landing in existing seams: - scripts/cross-platform-tests.ts gains incremental-vector-extension-ordering .test.ts: the win32 VECTOR gate is gone in this PR, so the #2623 drop-ordering + blocked-path escalation must be proven on the windows-latest native addon, not just Ubuntu. (The review's claim that lbug-delete-nodes-for-files.test.ts was also missing was wrong — it has been on the roster since #2409.) - scripts/ensure-fts.ts now pre-installs VECTOR under the same best-effort auto-policy contract, so every sharded CI process LOADs from ~/.lbdb instead of racing its own bounded out-of-process INSTALL; the workflow's extension cache already covers it (path is the whole extension dir — key kept for cache continuity). The cross-platform job sets GITNEXUS_REQUIRE_VECTOR=1 beside GITNEXUS_REQUIRE_FTS so a genuinely unavailable VECTOR is a loud failure, never a silent skip. Windows/macOS cannot be executed locally; the PR's CI lanes are the proof for this commit. Linux smoke: ensure-fts.ts reports both extensions ready; all 79 roster entries resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(pool): register loadVectorExtension in the pool unit-suite mocks The pool adapter's new loadVectorExtension import surfaced in four suites that mock lbug-adapter.js with explicit factories (vitest fails loudly on a missing mocked export). Register the export in each — resolving false where the suite's world assumes no vector, true where it mirrors FTS — and extend lbug-pool-fts-load.test.ts, the suite that owns pre-warm extension loading, with the vector pair: successful load cached per shared Database, failed load retried on the next open, both pinned to policy load-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): use POSIX literals for graph paths in the #2623 ordering suite First Windows CI run of this suite (it joined the cross-platform roster this PR) failed with 'Parser exception: Invalid input <MATCH (n:Function) WHERE n.filePath = '>' — path.join produces backslashes on Windows, and a backslash inside the seed helper's single-quoted Cypher literal breaks the parser. The graph stores repo-relative filePaths with forward slashes on every OS, so graph-side paths are POSIX literals now (the incremental-orchestration convention); path.join stays only for real filesystem access. The same Windows lane also proved the substance this suite exists for: lbug-vector-extension passed 7/7 on windows-latest — the extension installed, loaded, and built a real HNSW index there — and the pool vector-lane and DML gate suites passed too. This commit fixes the harness, not the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <abhigyan1.patwari@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 12:27:00 +01:00
Goutham Krishna Mandati	4c73b18387	feat(mcp): add trace tool for shortest call path between symbols (#2173) ... * feat(mcp): add trace tool for shortest call path between symbols (#1821) Implement the \ race\ MCP tool and \gitnexus trace\ CLI command that finds the shortest directed call path between two symbols using BFS over CALLS + HAS_METHOD edges. - MCP tool definition in tools.ts with READ_ONLY annotations - Directed BFS in local-backend.ts with parent-map path reconstruction - Symbol resolution via resolveSymbolCandidates (name/UID/file-hint) - Gap reporting with furthest reachable node and depth tracking - CLI wiring: gitnexus trace <from> <to> [--from-uid] [--to-uid] [--depth] - i18n keys in en.ts and zh-CN.ts + help-i18n.ts registration - ARCHITECTURE.md tools table entry - 16 unit tests (11 BFS core + 5 CLI wiring) * test(mcp): account for trace tool in tools.test.ts count The trace tool makes GITNEXUS_TOOLS length 15; update the hardcoded count, add 'trace' to the expected-names list, and refresh the stale "13 tools" comment and it() title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): sanitize trace maxDepth to reject 0/NaN/negative `Math.min(params.maxDepth ?? 10, 30)` had no lower bound and `??` does not recover 0 or NaN, so `--depth 0|-5|abc` made the BFS loop run zero iterations and return a false `no_path`. Clamp at the real boundary with a `Number.isInteger && > 0` guard (the MCP inputSchema minimum is advisory only), and reject a non-numeric `--depth` in the CLI up front rather than forwarding NaN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): check trace target before applying test-file filter The `isTestFilePath` filter ran before the target-equality check, but resolveSymbolCandidates does not exclude test-file symbols. A target (or a required hop) that lives in a test file was therefore skipped under the default includeTests=false and produced a false no_path with a misleading dynamic-dispatch suggestion. Match the explicitly-requested target first; non-target test-file nodes are still filtered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(mcp): bound trace BFS with per-level LIMIT and visited cap The per-level query had no LIMIT and the visited set was uncapped, so a high-fanout hub could materialize an unbounded frontier. Cap per-level rows (interpolated LIMIT — Kuzu does not bind LIMIT) and the total visited set; either cap sets a `truncated` flag so a resulting no_path reports that the search was cut short rather than implying the graph was exhausted. Note: the sibling impact BFS shares the same unbounded pattern; applying the cap there is deferred (out of scope for this PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): clarify trace traverses call + class-member edges trace was advertised as a "shortest call path" but also traverses HAS_METHOD (class→member) containment edges so a class-rooted trace can descend into its methods. Keep that capability (consistent with impact/ context) and make the docs honest: rename EDGE_TYPES→TRAVERSAL_EDGE_TYPES, state the call + class-member traversal in the MCP/CLI/i18n/ARCHITECTURE descriptions, and note each hop's edge type is reported in edges[]. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): set status:'error' on trace failure responses Every trace return path sets a `status` discriminator except the caught-error path, so a consumer switching on `result.status` saw undefined on failure. Add status:'error' to both the backend trace() catch and the CLI traceCommand catch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): return a friendly error for non-string trace from/to A non-string from/to reaching resolveSymbolCandidates surfaced a low-level "x.includes is not a function" via name.includes. Guard the four name/uid params at the top of _traceImpl and return a structured status:'error' with a clear message instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): single row-decode + drop dead field in trace BFS Decode each BFS row once into named locals instead of repeating `(row.x ?? row[N])` across the two parent.set calls and the furthest-tracking. Drop the `type` field from the parent map value (it was written but never read), and rename the internal `deepestInfo` to `lastReached` for accuracy (the output field `furthest` is unchanged). Pure refactor — no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): dedicated trace includeTests i18n key + guard coverage `trace|--include-tests` reused the impact help key, so rewording the impact option would silently change trace's help text. Add a dedicated help.option.trace.includeTests key in en + zh-CN and repoint it. Add CLI coverage for the (already symmetric) --from-uid/--to-uid flag-value guard and for --include-tests forwarding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): faithful BFS mock + expand trace coverage Fix makeResolveMock: concatenate neighbors across ALL frontier ids (it returned only the first node's, so a multi-node frontier was unmodelled) and key the UID branch on params.uid (the old query-text match never fired). Add coverage: shortest path through the second frontier node (proves the mock fix), confidence floor fallback, HAS_METHOD traversal with a mixed edge-type chain, no_path furthest:null, and from_file disambiguation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(trace): apply root prettier formatting to trace files The root `quality / format` gate (prettier --check, printWidth 100) runs on the full repo and flagged the trace sources/tests (the local config masks it). Reformat to root style — no behavior change; trace + tools suites and tsc stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): document the trace tool for AI agents Add `trace` to the GitNexus skill docs so agents reach for it instead of hand-chaining context/impact hops. The guide gains a Tools Reference row and a "shortest path between two symbols" subsection (params, result shape, status/furthest/truncated semantics); the debugging skill gains a "how does A reach B?" pattern row and a trace tool example. Mirrored to the .claude and claude-plugin copies (byte-identical) and the cursor copy (compact style). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): drop unused trace test fixtures (CodeQL js/unused-local-variable) CodeQL flagged two unused locals in the trace BFS tests: the top-level SYMBOL_C and a SYMBOL_D inside the maxDepth test (both defined, never referenced). Remove them. No behavior change — 58 trace/tools tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-14 10:43:01 +01:00