From dae70a26ea1b71ad40d1ef58a4ff18db031a8992 Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Wed, 20 May 2026 12:06:51 +0100 Subject: [PATCH 1/5] feat(cpp): Add pointer nullptr ellipsis conversion ranks (#1708) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add C++ pointer null ellipsis ranks * test(cpp): Strengthen pointer overload assertions --------- Co-authored-by: Gergő Magyar --- .../languages/cpp/conversion-rank.ts | 73 ++++++++---- .../passes/free-call-fallback.ts | 12 +- .../passes/overload-narrowing.ts | 83 ++++++++++++- .../lib.cpp | 12 ++ .../cpp-overload-pointer-null-ellipsis/lib.h | 38 ++++++ .../test/integration/resolvers/cpp.test.ts | 58 +++++++++ .../test/integration/resolvers/helpers.ts | 6 + .../cpp/cpp-overload-ranking.test.ts | 111 ++++++++++++++++++ 8 files changed, 362 insertions(+), 31 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.cpp create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.h create mode 100644 gitnexus/test/unit/scope-resolution/cpp/cpp-overload-ranking.test.ts diff --git a/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts b/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts index 2a9e3bc01..bea3600a7 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts @@ -1,32 +1,30 @@ /** - * C++ conversion-rank scoring for overload resolution (#1578). + * C++ conversion-rank scoring for overload resolution (#1578, #1637). * - * Operates on **normalized** type strings (output of - * `normalizeCppParamType` in `arity-metadata.ts`). After normalization: - * - int/long/short/unsigned → 'int' - * - float/double → 'double' - * - char → 'char', bool → 'bool' - * - * Because the normalizer collapses promotion pairs (int↔long, - * float↔double) to the same string, those promotions are invisible at - * this layer — they appear as exact matches (rank 0). + * Operates on normalized type strings (output of `normalizeCppParamType` + * in `arity-metadata.ts`) plus optional shape sidecars from #1630. + * Normalization intentionally collapses cv/ref/pointer spelling for stable + * graph IDs, so pointer/nullptr rules must consult `ParameterTypeClass`. * * Post-normalization ranking: - * - rank 0 — exact (same normalized type) - * - rank 1 — integral promotion (char→int, bool→int) - * - rank 2 — standard arithmetic conversion (int↔double, char→double, - * bool→double) - * - Infinity — mismatch (string↔int, user types, pointers, etc.) + * - rank 0: exact (same normalized type) + * - rank 1: integral promotion (char -> int, bool -> int) + * - rank 2: standard conversion (arithmetic, nullptr -> T*, T* -> bool, + * T* -> void*) + * - rank 3: nullptr -> bool (kept worse than nullptr -> T*) + * - rank 4: ellipsis conversion (worst viable) + * - Infinity: mismatch (string -> int, user types, unsupported shapes) * - * This function is intentionally C++-specific (issue #1578 pitfall: - * keep conversion-rank tables out of shared overload-narrowing). Other - * languages may define their own `ConversionRankFn` in the future. + * This function is intentionally C++-specific. Other languages may define + * their own `ConversionRankFn` in the future. */ +import type { ParameterTypeClass } from 'gitnexus-shared'; + /** Set of normalized arithmetic types that support implicit conversion. */ const ARITHMETIC = new Set(['int', 'double', 'char', 'bool']); -/** Integral promotion targets: char→int and bool→int are rank 1. */ +/** Integral promotion targets: char -> int and bool -> int are rank 1. */ const INTEGRAL_PROMOTION = new Map([ ['char', 'int'], ['bool', 'int'], @@ -35,13 +33,40 @@ const INTEGRAL_PROMOTION = new Map([ /** * Return the conversion rank from `argType` to `paramType`. * - * @returns 0 for exact match, 1 for integral promotion (char/bool→int), - * 2 for standard arithmetic conversion, Infinity for mismatch. + * @returns 0 for exact match, 1 for integral promotion, 2 for standard + * conversion, 3 for nullptr -> bool, 4 for ellipsis, Infinity + * for mismatch. */ -export function cppConversionRank(argType: string, paramType: string): number { - if (argType === paramType) return 0; - // Integral promotions: char→int, bool→int (ISO C++ [conv.prom]) +export function cppConversionRank( + argType: string, + paramType: string, + argTypeClass?: ParameterTypeClass, + paramTypeClass?: ParameterTypeClass, +): number { + if (argType === paramType) { + return exactShapeCompatible(argTypeClass, paramTypeClass) ? 0 : Infinity; + } + if (paramType === '...') return 4; if (INTEGRAL_PROMOTION.get(argType) === paramType) return 1; if (ARITHMETIC.has(argType) && ARITHMETIC.has(paramType)) return 2; + if (argType === 'null' && isPointer(paramTypeClass)) return 2; + if (argType === 'null' && paramType === 'bool') return 3; + if (isPointer(argTypeClass) && paramType === 'bool') return 2; + if (isPointer(argTypeClass) && isPointer(paramTypeClass) && paramType === 'void') return 2; return Infinity; } + +function isPointer(typeClass: ParameterTypeClass | undefined): boolean { + return typeClass?.indirection === 'pointer' && typeClass.pointerDepth > 0; +} + +function exactShapeCompatible( + argTypeClass: ParameterTypeClass | undefined, + paramTypeClass: ParameterTypeClass | undefined, +): boolean { + if (argTypeClass === undefined || paramTypeClass === undefined) return true; + if (argTypeClass.indirection === 'unknown' || paramTypeClass.indirection === 'unknown') { + return true; + } + return isPointer(argTypeClass) === isPointer(paramTypeClass); +} diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts index f3010abf4..0d537db07 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts @@ -17,7 +17,13 @@ * generalization plan. */ -import type { ParsedFile, Reference, ScopeId, SymbolDefinition } from 'gitnexus-shared'; +import type { + ParameterTypeClass, + ParsedFile, + Reference, + ScopeId, + SymbolDefinition, +} from 'gitnexus-shared'; import type { KnowledgeGraph } from '../../../graph/types.js'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import type { SemanticModel } from '../../model/semantic-model.js'; @@ -277,6 +283,7 @@ export function emitFreeCallFallback( }) : undefined, site.argumentTypes, + site.argumentTypeClasses, options.conversionRankFn, ); } @@ -342,6 +349,7 @@ function pickUniqueGlobalCallable( callArity?: number, isCallerVisible?: (candidate: SymbolDefinition) => boolean, callArgTypes?: readonly string[], + callArgTypeClasses?: readonly ParameterTypeClass[], conversionRankFn?: ConversionRankFn, ): SymbolDefinition | undefined { const scopeDefs: SymbolDefinition[] = []; @@ -380,6 +388,7 @@ function pickUniqueGlobalCallable( // disambiguate (e.g., `f(int)` vs `f(double)` called with `f(2.5)`). if (scopeDefs.length > 1) { const narrowed = narrowOverloadCandidates(scopeDefs, callArity, callArgTypes, { + argumentTypeClasses: callArgTypeClasses, conversionRankFn, }); if (narrowed.length === 1) return narrowed[0]; @@ -420,6 +429,7 @@ function pickUniqueGlobalCallable( // Same argument-type + conversion-rank narrowing for the model pool. if (defs.length > 1) { const narrowed = narrowOverloadCandidates(defs, callArity, callArgTypes, { + argumentTypeClasses: callArgTypeClasses, conversionRankFn, }); if (narrowed.length === 1) return narrowed[0]; diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts index b9b452eb7..f28cd3185 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts @@ -38,7 +38,13 @@ * 5. Empty input returns empty output. */ -import type { ArityVerdict, Callsite, ConstraintContext, SymbolDefinition } from 'gitnexus-shared'; +import type { + ArityVerdict, + Callsite, + ConstraintContext, + ParameterTypeClass, + SymbolDefinition, +} from 'gitnexus-shared'; /** * Per-slot conversion-rank function. Returns a numeric cost for @@ -51,7 +57,12 @@ import type { ArityVerdict, Callsite, ConstraintContext, SymbolDefinition } from * Each language provides its own implementation. The function operates * on normalized type strings (output of the language's type normalizer). */ -export type ConversionRankFn = (argType: string, paramType: string) => number; +export type ConversionRankFn = ( + argType: string, + paramType: string, + argTypeClass?: ParameterTypeClass, + paramTypeClass?: ParameterTypeClass, +) => number; /** * Optional hook bundle for narrowing extension points. Threaded in @@ -130,7 +141,16 @@ export function narrowOverloadCandidates( if (params === undefined) return false; for (let i = 0; i < argTypes.length && i < params.length; i++) { if (argTypes[i] === '') continue; - if (argTypes[i] !== params[i]) return false; + if ( + !exactTypeSlotMatches( + argTypes[i], + params[i], + hookCtx?.argumentTypeClasses?.[i], + d.parameterTypeClasses?.[i], + ) + ) { + return false; + } } return true; }); @@ -144,7 +164,12 @@ export function narrowOverloadCandidates( // are returned; multiple survivors are genuinely ambiguous. When // ranking also yields empty, fall through to the arity-filtered // `candidates` set — matches pre-#1606 behavior. - const ranked = rankByConversion(candidates, argTypes, hookCtx.conversionRankFn); + const ranked = rankByConversion( + candidates, + argTypes, + hookCtx.conversionRankFn, + hookCtx.argumentTypeClasses, + ); if (ranked.length > 0) result = ranked; } } @@ -183,6 +208,27 @@ export function narrowOverloadCandidates( return result; } +function exactTypeSlotMatches( + argType: string, + paramType: string, + argTypeClass?: ParameterTypeClass, + paramTypeClass?: ParameterTypeClass, +): boolean { + if (argType !== paramType) return false; + // C++ normalizes away pointer markers (`int*` -> `int`). When both sides + // provide shape sidecars, do not let that collapse make `int` exactly match + // `int*`. Unknown sidecar evidence preserves the previous string-only path. + if (argTypeClass === undefined || paramTypeClass === undefined) return true; + if (argTypeClass.indirection === 'unknown' || paramTypeClass.indirection === 'unknown') { + return true; + } + return isPointerShape(argTypeClass) === isPointerShape(paramTypeClass); +} + +function isPointerShape(typeClass: ParameterTypeClass): boolean { + return typeClass.indirection === 'pointer' && typeClass.pointerDepth > 0; +} + /** * Pairwise dominance comparison (ISO C++ [over.ics.rank]). * @@ -199,6 +245,7 @@ function rankByConversion( candidates: readonly SymbolDefinition[], argTypes: readonly string[], rankFn: ConversionRankFn, + argTypeClasses?: readonly ParameterTypeClass[], ): readonly SymbolDefinition[] { // Step 1: compute per-slot ranks and exclude non-viable candidates. const viable: Array<{ def: SymbolDefinition; ranks: number[] }> = []; @@ -207,12 +254,22 @@ function rankByConversion( if (params === undefined) continue; const ranks: number[] = []; let ok = true; - for (let i = 0; i < argTypes.length && i < params.length; i++) { + for (let i = 0; i < argTypes.length; i++) { + const paramType = parameterTypeAt(params, i); + if (paramType === undefined) { + ok = false; + break; + } if (argTypes[i] === '') { ranks.push(0); // unknown arg → any-match (rank 0) continue; } - const r = rankFn(argTypes[i], params[i]); + const r = rankFn( + argTypes[i], + paramType, + argTypeClasses?.[i], + parameterTypeClassAt(d.parameterTypeClasses, i), + ); if (!isFinite(r)) { ok = false; break; @@ -239,6 +296,20 @@ function rankByConversion( return viable.filter((_, idx) => !dominated.has(idx)).map((v) => v.def); } +function parameterTypeAt(params: readonly string[], argIndex: number): string | undefined { + if (argIndex < params.length) return params[argIndex]; + return params[params.length - 1] === '...' ? '...' : undefined; +} + +function parameterTypeClassAt( + params: readonly ParameterTypeClass[] | undefined, + argIndex: number, +): ParameterTypeClass | undefined { + if (params === undefined) return undefined; + if (argIndex < params.length) return params[argIndex]; + return params[params.length - 1]?.base === '...' ? params[params.length - 1] : undefined; +} + /** * Compare two per-slot rank vectors. * Returns -1 if `a` dominates `b` (not worse everywhere, better somewhere), diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.cpp new file mode 100644 index 000000000..c6b0bb5a2 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.cpp @@ -0,0 +1,12 @@ +#include "lib.h" + +void Service::f(int* p) {} +void Service::f(bool flag) {} + +void Service::g(int a, int b) {} +void Service::g(int a, ...) {} + +void Service::h(int a, double b) {} +void Service::h(int a, ...) {} + +void Service::k(int a, ...) {} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.h new file mode 100644 index 000000000..c85f258a0 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.h @@ -0,0 +1,38 @@ +#pragma once + +class Service { +public: + void f(int* p); + void f(bool flag); + + void g(int a, int b); + void g(int a, ...); + + void h(int a, double b); + void h(int a, ...); + + void k(int a, ...); + + void runNullptr() { + f(nullptr); + } + + void runPointer() { + int* p = nullptr; + f(p); + } + + void runBoolConversion() { + f(42); + } + + void run() { + int* p = nullptr; + f(nullptr); + f(p); + f(42); + g(1, 2); + h(1, 'a'); + k(1, 2, 3); + } +}; diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index 1537c3884..e1376938e 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -1836,6 +1836,64 @@ describe('C++ overload resolution — conversion-rank disambiguation (#1578)', ( }); }); +// C++ overload resolution: pointer/nullptr/ellipsis conversion ranks (#1637) +describe('C++ overload resolution — pointer/nullptr/ellipsis ranks (#1637)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-overload-pointer-null-ellipsis'), + () => {}, + ); + }, 60000); + + it('f(nullptr) and f(p) resolve to f(int*) while f(42) resolves to f(bool)', () => { + const calls = getRelationships(result, 'CALLS'); + + const nullptrCall = calls.find((c) => c.source === 'runNullptr' && c.target === 'f'); + const pointerCall = calls.find((c) => c.source === 'runPointer' && c.target === 'f'); + const boolCall = calls.find((c) => c.source === 'runBoolConversion' && c.target === 'f'); + + expect( + result.graph.getNode(nullptrCall?.rel.targetId ?? '')?.properties.parameterTypes, + ).toEqual(['int']); + expect( + result.graph.getNode(pointerCall?.rel.targetId ?? '')?.properties.parameterTypes, + ).toEqual(['int']); + expect(result.graph.getNode(boolCall?.rel.targetId ?? '')?.properties.parameterTypes).toEqual([ + 'bool', + ]); + }); + + it('g(1, 2) resolves to fixed-arity g(int, int), not g(int, ...)', () => { + const calls = getRelationships(result, 'CALLS'); + const gCalls = calls.filter((c) => c.source === 'run' && c.target === 'g'); + + expect(gCalls.length).toBe(1); + const tgt = result.graph.getNode(gCalls[0].rel.targetId); + expect(tgt?.properties.parameterTypes).toEqual(['int', 'int']); + }); + + it("h(1, 'a') resolves to h(int, double), not h(int, ...)", () => { + const calls = getRelationships(result, 'CALLS'); + const hCalls = calls.filter((c) => c.source === 'run' && c.target === 'h'); + + expect(hCalls.length).toBe(1); + const tgt = result.graph.getNode(hCalls[0].rel.targetId); + expect(tgt?.properties.parameterTypes).toEqual(['int', 'double']); + }); + + it('k(1, 2, 3) keeps the ellipsis overload viable when it is the only match', () => { + const calls = getRelationships(result, 'CALLS'); + const kCalls = calls.filter((c) => c.source === 'run' && c.target === 'k'); + + expect(kCalls.length).toBe(1); + const tgt = result.graph.getNode(kCalls[0].rel.targetId); + expect(tgt?.properties.parameterCount).toBeUndefined(); + expect(tgt?.properties.parameterTypes).toEqual(['int']); + }); +}); + // --------------------------------------------------------------------------- // U3: anonymous-namespace symbols MUST NOT leak across translation units // (full-pipeline integration test; unit-level coverage exists separately) diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index bd5eaa474..296f3e458 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -196,6 +196,12 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly` overloads // guarded by mutually-exclusive `enable_if_t` predicates collapse diff --git a/gitnexus/test/unit/scope-resolution/cpp/cpp-overload-ranking.test.ts b/gitnexus/test/unit/scope-resolution/cpp/cpp-overload-ranking.test.ts new file mode 100644 index 000000000..5cb679a37 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/cpp/cpp-overload-ranking.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest'; +import type { ParameterTypeClass, SymbolDefinition } from 'gitnexus-shared'; +import { cppConversionRank } from '../../../../src/core/ingestion/languages/cpp/conversion-rank.js'; +import { narrowOverloadCandidates } from '../../../../src/core/ingestion/scope-resolution/passes/overload-narrowing.js'; + +const value = (base: string): ParameterTypeClass => ({ + base, + cv: 'none', + indirection: 'value', + pointerDepth: 0, +}); + +const pointer = (base: string): ParameterTypeClass => ({ + base, + cv: 'none', + indirection: 'pointer', + pointerDepth: 1, +}); + +const ellipsis = (): ParameterTypeClass => ({ + base: '...', + cv: 'unknown', + indirection: 'unknown', + pointerDepth: 0, +}); + +const mkDef = ( + nodeId: string, + parameterTypes: readonly string[], + parameterTypeClasses: readonly ParameterTypeClass[], +): SymbolDefinition => ({ + nodeId, + filePath: 'service.cpp', + type: 'Method', + parameterCount: parameterTypes.includes('...') ? undefined : parameterTypes.length, + requiredParameterCount: parameterTypes.includes('...') + ? parameterTypes.indexOf('...') + : parameterTypes.length, + parameterTypes: [...parameterTypes], + parameterTypeClasses: [...parameterTypeClasses], +}); + +describe('cppConversionRank pointer/nullptr/ellipsis ranks (#1637)', () => { + it('ranks nullptr -> T* ahead of nullptr -> bool', () => { + expect(cppConversionRank('null', 'int', value('null'), pointer('int'))).toBe(2); + expect(cppConversionRank('null', 'bool', value('null'), value('bool'))).toBe(3); + }); + + it('ranks pointer -> bool and pointer -> void* as standard conversions', () => { + expect(cppConversionRank('int', 'bool', pointer('int'), value('bool'))).toBe(2); + expect(cppConversionRank('int', 'void', pointer('int'), pointer('void'))).toBe(2); + }); + + it('keeps pointer exact matches shape-aware', () => { + expect(cppConversionRank('int', 'int', pointer('int'), pointer('int'))).toBe(0); + expect(cppConversionRank('int', 'int', value('int'), pointer('int'))).toBe(Infinity); + }); + + it('ranks ellipsis as the worst viable conversion', () => { + expect(cppConversionRank('int', '...', value('int'), ellipsis())).toBe(4); + }); +}); + +describe('narrowOverloadCandidates with C++ pointer-rank sidecars (#1637)', () => { + it('selects pointer overload for nullptr over bool overload', () => { + const byPointer = mkDef('f:intptr', ['int'], [pointer('int')]); + const byBool = mkDef('f:bool', ['bool'], [value('bool')]); + + const result = narrowOverloadCandidates([byPointer, byBool], 1, ['null'], { + argumentTypeClasses: [value('null')], + conversionRankFn: cppConversionRank, + }); + + expect(result.map((d) => d.nodeId)).toEqual(['f:intptr']); + }); + + it('does not treat normalized value and pointer types as exact matches', () => { + const byPointer = mkDef('f:intptr', ['int'], [pointer('int')]); + const byBool = mkDef('f:bool', ['bool'], [value('bool')]); + + const result = narrowOverloadCandidates([byPointer, byBool], 1, ['int'], { + argumentTypeClasses: [value('int')], + conversionRankFn: cppConversionRank, + }); + + expect(result.map((d) => d.nodeId)).toEqual(['f:bool']); + }); + + it('selects fixed-arity overload over ellipsis', () => { + const exact = mkDef('g:int-int', ['int', 'int'], [value('int'), value('int')]); + const variadic = mkDef('g:ellipsis', ['int', '...'], [value('int'), ellipsis()]); + + const result = narrowOverloadCandidates([exact, variadic], 2, ['int', 'int'], { + argumentTypeClasses: [value('int'), value('int')], + conversionRankFn: cppConversionRank, + }); + + expect(result.map((d) => d.nodeId)).toEqual(['g:int-int']); + }); + + it('keeps an ellipsis overload viable when it is the only match', () => { + const variadic = mkDef('log:ellipsis', ['int', '...'], [value('int'), ellipsis()]); + + const result = narrowOverloadCandidates([variadic], 3, ['int', 'int', 'double'], { + argumentTypeClasses: [value('int'), value('int'), value('double')], + conversionRankFn: cppConversionRank, + }); + + expect(result.map((d) => d.nodeId)).toEqual(['log:ellipsis']); + }); +}); From f350ae278aaa02362bea31b6a257b7779c5f2f1d Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 13:37:04 +0100 Subject: [PATCH 2/5] feat: Add `analyze --repair-fts`, enforce FTS verification, and harden repair safeguards (#1720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * feat(analyze): add --repair-fts and verify FTS index rebuilds Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/dccb3673-af86-43aa-aede-2e1449399775 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor(fts): tighten repair/verify messaging and option naming Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/dccb3673-af86-43aa-aede-2e1449399775 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * docs: highlight analyze --repair-fts vs --force in READMEs Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/61edc967-debc-419f-9f51-aebf2ef08d22 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(analyze): guard repair mode against missing graph store * fix(cli): reject --repair-fts with --force * test(analyze): document repair-store fixture intent * test(analyze): tidy repair failure fixtures and constants * test(analyze): clarify mock constants in repair tests * test(analyze): rename simulated missing-index constant * test(analyze): clarify mocked graph shape in full-verify test * refactor(analyze): finalize flag validation and test clarity * test(skip-git): avoid hard failing when FTS extension is unavailable * test(skip-git): log visible FTS-unavailable test skips * test(skip-git): tighten FTS-unavailable error detection * test(skip-git): simplify FTS-unavailable message checks * test(skip-git): avoid HOME pointing at parent repo in fixture env * fix(analyze): address Claude follow-up findings for repair guardrails Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d * fix(repair-fts): clarify invalid graph-store preflight errors Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d * test(analyze): strengthen assertions for conflict and missing-store errors Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d * fix(repair-fts): make invalid graph-store type errors explicit Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d * fix(repair-fts): improve graph-store type diagnostics Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergő Magyar --- README.md | 3 +- gitnexus/README.md | 3 +- gitnexus/src/cli/analyze.ts | 25 ++ gitnexus/src/cli/index.ts | 1 + gitnexus/src/core/lbug/lbug-adapter.ts | 5 +- gitnexus/src/core/run-analyze.ts | 96 ++++++- gitnexus/src/core/search/fts-indexes.ts | 39 ++- gitnexus/src/mcp/local/local-backend.ts | 2 +- gitnexus/src/server/api.ts | 2 +- .../test/unit/analyze-no-stats-bridge.test.ts | 41 ++- gitnexus/test/unit/bm25-search.test.ts | 25 ++ gitnexus/test/unit/calltool-dispatch.test.ts | 2 +- gitnexus/test/unit/cli-index-help.test.ts | 7 + .../test/unit/run-analyze-fts-repair.test.ts | 243 ++++++++++++++++++ gitnexus/test/unit/skip-git-cli.test.ts | 141 +++++++--- 15 files changed, 590 insertions(+), 45 deletions(-) create mode 100644 gitnexus/test/unit/run-analyze-fts-repair.test.ts diff --git a/README.md b/README.md index 714d2cbfd..6e6c7193d 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,8 @@ args = ["-y", "gitnexus@latest", "mcp"] ```bash gitnexus setup # Configure MCP for your editors (one-time) gitnexus analyze [path] # Index a repository (or update stale index) -gitnexus analyze --force # Force full re-index +gitnexus analyze --repair-fts # Fast path: rebuild/verify only FTS indexes on existing index data +gitnexus analyze --force # Full rebuild: re-parse + graph rebuild + FTS rebuild gitnexus analyze --skills # Generate repo-specific skill files from detected communities gitnexus analyze --skip-embeddings # Skip embedding generation (faster) gitnexus analyze --skip-agents-md # Preserve custom AGENTS.md/CLAUDE.md gitnexus section edits diff --git a/gitnexus/README.md b/gitnexus/README.md index 55cdc12f7..640053013 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -151,7 +151,8 @@ Your AI agent gets these tools automatically: ```bash gitnexus setup # Configure MCP for your editors (one-time) gitnexus analyze [path] # Index a repository (or update stale index) -gitnexus analyze --force # Force full re-index +gitnexus analyze --repair-fts # Fast path: rebuild/verify only FTS indexes on existing index data +gitnexus analyze --force # Full rebuild: re-parse + graph rebuild + FTS rebuild gitnexus analyze --embeddings # Enable embedding generation (slower, better search) gitnexus analyze --skip-agents-md # Preserve custom AGENTS.md/CLAUDE.md gitnexus section edits gitnexus analyze --verbose # Log skipped files when parsers are unavailable diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index e24b8c894..70183ecf9 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -166,6 +166,7 @@ function ensureHeap(): boolean { export interface AnalyzeOptions { force?: boolean; + repairFts?: boolean; /** * Embedding generation toggle. Commander parses `--embeddings [limit]` as: * - `undefined` when the flag is omitted @@ -343,6 +344,15 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption process.env.GITNEXUS_EMBEDDING_DEVICE = options.embeddingDevice; } + if (options?.repairFts && options?.force) { + cliError( + ' Cannot combine `--repair-fts` with `--force`. ' + + 'Use `--repair-fts` for fast FTS-only repair, or `--force` for a full rebuild.\n', + ); + process.exitCode = 1; + return; + } + console.log('\n GitNexus Analyzer\n'); // `--index-only` is the stronger contract — it suppresses every form of file @@ -521,9 +531,11 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption // needs a fresh pipelineResult. Has no bearing on the registry // collision guard (see allowDuplicateName below). force: options?.force || options?.skills, + repairFts: options?.repairFts, embeddings: embeddingsEnabled, embeddingsNodeLimit, dropEmbeddings: options?.dropEmbeddings, + verbose: options?.verbose, skipGit: options?.skipGit, skipAgentsMd, skipSkills, @@ -568,6 +580,19 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption return; } + if (result.ftsRepairedOnly) { + clearInterval(elapsedTimer); + process.removeListener('SIGINT', sigintHandler); + console.log = origLog; + // eslint-disable-next-line no-console -- restoring after intentional progress-bar routing + console.warn = origWarn; + // eslint-disable-next-line no-console -- restoring after intentional progress-bar routing + console.error = origError; + bar.stop(); + console.log(' FTS indexes repaired successfully\n'); + return; + } + // Post-finalize invariant (#1169): runFullAnalysis nominally writes // meta.json and registers the repo, but on Windows it has been // observed to return successfully with neither artifact present diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index 5b317e5c6..9450698d3 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -23,6 +23,7 @@ program .command('analyze [path]') .description('Index a repository (full analysis)') .option('-f, --force', 'Force full re-index even if up to date') + .option('--repair-fts', 'Repair/rebuild search FTS indexes without full re-analysis') .option( '--embeddings [limit]', 'Enable embedding generation for semantic search (off by default). ' + diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 8330614b2..77e9f65f1 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -1651,7 +1651,10 @@ export const createFTSIndex = async ( if (ensuredFTSIndexes.has(key)) return; if (!(await loadFTSExtension())) { - return; + throw new Error( + `FTS extension unavailable - cannot create FTS index ${tableName}.${indexName}. ` + + 'Run `gitnexus doctor` and ensure the LadybugDB FTS extension is installed and loadable on this machine.', + ); } const propList = properties.map((p) => `'${p}'`).join(', '); diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 425f18f9a..e11d0833b 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -25,7 +25,7 @@ import { deleteAllCommunitiesAndProcesses, queryImporters, } from './lbug/lbug-adapter.js'; -import { createSearchFTSIndexes } from './search/fts-indexes.js'; +import { createSearchFTSIndexes, verifySearchFTSIndexes } from './search/fts-indexes.js'; import { getStoragePaths, saveMeta, @@ -71,6 +71,10 @@ export interface AnalyzeOptions { * bypass. See `allowDuplicateName` below. */ force?: boolean; + /** Repair only search indexes without re-running full parsing/indexing. */ + repairFts?: boolean; + /** Emit per-index FTS create logs. */ + verbose?: boolean; embeddings?: boolean; /** * Override the auto-skip node-count cap for embedding generation. @@ -126,6 +130,8 @@ export interface AnalyzeResult { alreadyUpToDate?: boolean; /** The raw pipeline result — only populated when needed by callers (e.g. skill generation). */ pipelineResult?: any; + /** True when analyze only repaired FTS indexes and skipped pipeline re-analysis. */ + ftsRepairedOnly?: boolean; } // Re-export the pure flag-derivation helper so external callers (and tests) @@ -190,6 +196,78 @@ export async function runFullAnalysis( const currentCommit = repoHasGit ? getCurrentCommit(repoPath) : ''; const existingMeta = await loadMeta(storagePath); + // ── FTS-only repair path ──────────────────────────────────────────── + if (options.repairFts) { + if (!existingMeta) { + throw new Error( + 'Cannot repair FTS indexes because this repository has not been analyzed yet. ' + + 'Run `gitnexus analyze` first to create the initial index, then retry `--repair-fts`.', + ); + } + let lbugStat; + try { + lbugStat = await fs.lstat(lbugPath); + } catch { + throw new Error( + `Cannot repair FTS indexes: graph store at ${lbugPath} is missing. ` + + 'Run `gitnexus analyze` (full) to rebuild from scratch.', + ); + } + if (!lbugStat.isFile()) { + const foundType = lbugStat.isDirectory() + ? 'a directory' + : lbugStat.isSymbolicLink() + ? 'a symbolic link' + : lbugStat.isSocket() + ? 'a socket' + : lbugStat.isBlockDevice() + ? 'a block device' + : lbugStat.isCharacterDevice() + ? 'a character device' + : lbugStat.isFIFO() + ? 'a FIFO' + : 'not a regular file'; + throw new Error( + `Cannot repair FTS indexes: graph store at ${lbugPath} is ${foundType} (expected a file). ` + + 'Run `gitnexus analyze` (full) to rebuild from scratch.', + ); + } + try { + await initLbug(lbugPath); + progress('fts', 85, 'Repairing search indexes...'); + await createSearchFTSIndexes({ + onIndexStart: options.verbose + ? (table, indexName) => log(`FTS: creating ${table}.${indexName}`) + : undefined, + onIndexReady: options.verbose + ? (table, indexName) => log(`FTS: ready ${table}.${indexName}`) + : undefined, + }); + const missing = await verifySearchFTSIndexes(executeQuery); + if (missing.length > 0) { + throw new Error( + `FTS repair failed - missing indexes after rebuild: ${missing.join(', ')}. ` + + 'Run `gitnexus analyze --force` to perform a full graph+FTS rebuild; ' + + 'if that also fails, verify FTS extension availability via `gitnexus doctor`.', + ); + } + await ensureGitNexusIgnored(repoPath); + progress('fts', 90, 'Search indexes ready'); + progress('done', 100, 'Done'); + return { + repoName: + options.registryName ?? + getInferredRepoName(repoPath) ?? + path.basename(resolveRepoIdentityRoot(repoPath)), + repoPath, + stats: existingMeta.stats ?? {}, + ftsRepairedOnly: true, + }; + } finally { + await closeLbug().catch(() => {}); + } + } + // ── Crash recovery: dirty flag forces full rebuild ──────────────── // If the previous incremental run set incrementalInProgress and didn't // clear it, the on-disk index may be in a half-state. Cheapest path @@ -583,7 +661,21 @@ export async function runFullAnalysis( // ── Phase 3: FTS (85–90%) ───────────────────────────────────────── progress('fts', 85, 'Creating search indexes...'); - await createSearchFTSIndexes(); + await createSearchFTSIndexes({ + onIndexStart: options.verbose + ? (table, indexName) => log(`FTS: creating ${table}.${indexName}`) + : undefined, + onIndexReady: options.verbose + ? (table, indexName) => log(`FTS: ready ${table}.${indexName}`) + : undefined, + }); + const missingIndexNames = await verifySearchFTSIndexes(executeQuery); + if (missingIndexNames.length > 0) { + throw new Error( + `FTS verification failed - missing indexes after analyze: ${missingIndexNames.join(', ')}. ` + + 'Check FTS extension availability, then retry `gitnexus analyze --force` for a full rebuild.', + ); + } progress('fts', 90, 'Search indexes ready'); // ── Phase 3.5: Re-insert cached embeddings ──────────────────────── diff --git a/gitnexus/src/core/search/fts-indexes.ts b/gitnexus/src/core/search/fts-indexes.ts index 5dfbde647..01147beed 100644 --- a/gitnexus/src/core/search/fts-indexes.ts +++ b/gitnexus/src/core/search/fts-indexes.ts @@ -1,8 +1,45 @@ import { createFTSIndex } from '../lbug/lbug-adapter.js'; import { FTS_INDEXES } from './fts-schema.js'; -export async function createSearchFTSIndexes(): Promise { +export interface CreateSearchFTSIndexesOptions { + onIndexStart?: (table: string, indexName: string) => void; + onIndexReady?: (table: string, indexName: string) => void; +} + +export async function createSearchFTSIndexes( + options?: CreateSearchFTSIndexesOptions, +): Promise { for (const { table, indexName, properties } of FTS_INDEXES) { + options?.onIndexStart?.(table, indexName); await createFTSIndex(table, indexName, [...properties]); + options?.onIndexReady?.(table, indexName); } } + +export async function verifySearchFTSIndexes( + executeQuery: (cypher: string) => Promise, +): Promise { + const safeIdentifier = (value: string): string => { + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(value)) { + throw new Error(`Invalid FTS identifier: ${value}`); + } + return value; + }; + + const missing: string[] = []; + for (const { table, indexName } of FTS_INDEXES) { + const safeTable = safeIdentifier(table); + const safeIndex = safeIdentifier(indexName); + const probe = ` + CALL QUERY_FTS_INDEX('${safeTable}', '${safeIndex}', '__gitnexus_fts_probe__', conjunctive := false) + RETURN score + LIMIT 1 + `; + try { + await executeQuery(probe); + } catch { + missing.push(`${table}.${indexName}`); + } + } + return missing; +} diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 4cff47a28..d1297ea17 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -1066,7 +1066,7 @@ export class LocalBackend { timing, ...(!ftsUsed && { warning: - 'FTS indexes missing — keyword search degraded. Run: gitnexus analyze --force to rebuild indexes.', + 'FTS indexes missing — keyword search degraded. Run: gitnexus analyze --repair-fts (or gitnexus analyze --force) to rebuild indexes.', }), }; } diff --git a/gitnexus/src/server/api.ts b/gitnexus/src/server/api.ts index 7fc892574..796fa04e1 100644 --- a/gitnexus/src/server/api.ts +++ b/gitnexus/src/server/api.ts @@ -1233,7 +1233,7 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => const response: any = { results: results.searchResults ?? results }; if (results.ftsAvailable === false) { response.warning = - 'FTS indexes missing — keyword search degraded. Run: gitnexus analyze --force to rebuild indexes.'; + 'FTS indexes missing — keyword search degraded. Run: gitnexus analyze --repair-fts (or gitnexus analyze --force) to rebuild indexes.'; } res.json(response); } catch (err: any) { diff --git a/gitnexus/test/unit/analyze-no-stats-bridge.test.ts b/gitnexus/test/unit/analyze-no-stats-bridge.test.ts index f941141ef..b2d368c2f 100644 --- a/gitnexus/test/unit/analyze-no-stats-bridge.test.ts +++ b/gitnexus/test/unit/analyze-no-stats-bridge.test.ts @@ -1,16 +1,21 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { runFullAnalysisMock, generateAIContextFilesMock, generateSkillFilesMock } = vi.hoisted( - () => { +const { runFullAnalysisMock, generateAIContextFilesMock, generateSkillFilesMock, cliErrorMock } = + vi.hoisted(() => { const runFullAnalysisMock = vi.fn(); const generateAIContextFilesMock = vi.fn(async () => ({ files: [] as string[] })); const generateSkillFilesMock = vi.fn(async () => ({ skills: [{ name: 'c', label: 'Community', symbolCount: 1, fileCount: 1 }], outputPath: '/repo/.claude/skills/generated', })); - return { runFullAnalysisMock, generateAIContextFilesMock, generateSkillFilesMock }; - }, -); + const cliErrorMock = vi.fn(); + return { + runFullAnalysisMock, + generateAIContextFilesMock, + generateSkillFilesMock, + cliErrorMock, + }; + }); vi.mock('../../src/core/run-analyze.js', () => ({ runFullAnalysis: runFullAnalysisMock, @@ -24,6 +29,10 @@ vi.mock('../../src/cli/skill-gen.js', () => ({ generateSkillFiles: generateSkillFilesMock, })); +vi.mock('../../src/cli/cli-message.js', () => ({ + cliError: cliErrorMock, +})); + vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ closeLbug: vi.fn(async () => undefined), })); @@ -62,6 +71,7 @@ describe('analyzeCommand commander → runFullAnalysis noStats bridge (#1477)', skills: [{ name: 'c', label: 'Community', symbolCount: 1, fileCount: 1 }], outputPath: '/repo/.claude/skills/generated', }); + cliErrorMock.mockReset(); process.exitCode = undefined; process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS ?? ''} --max-old-space-size=8192`.trim(); }); @@ -104,6 +114,27 @@ describe('analyzeCommand commander → runFullAnalysis noStats bridge (#1477)', expect(opts.skipAgentsMd).toBe(true); }); + it('passes --repair-fts through to runFullAnalysis', async () => { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { repairFts: true }); + + const opts = runFullAnalysisMock.mock.calls[0][1]; + expect(opts.repairFts).toBe(true); + }); + + it('rejects combining --repair-fts with --force', async () => { + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { repairFts: true, force: true }); + + expect(process.exitCode).toBe(1); + expect(cliErrorMock).toHaveBeenCalledWith( + expect.stringMatching(/cannot combine `--repair-fts` with `--force`/i), + ); + expect(runFullAnalysisMock).not.toHaveBeenCalled(); + }); + it('passes stats:false as noStats to generateAIContextFiles on the --skills regeneration path (#1477)', async () => { runFullAnalysisMock.mockResolvedValueOnce({ repoName: 'repo', diff --git a/gitnexus/test/unit/bm25-search.test.ts b/gitnexus/test/unit/bm25-search.test.ts index 6b878ef1b..984e6364a 100644 --- a/gitnexus/test/unit/bm25-search.test.ts +++ b/gitnexus/test/unit/bm25-search.test.ts @@ -40,6 +40,31 @@ describe('BM25 search', () => { ['Interface', 'interface_fts', ['name', 'content']], ]); }); + + it('verifies all configured FTS indexes are queryable', async () => { + const executeQuery = vi.fn().mockResolvedValue([]); + const { verifySearchFTSIndexes } = await import('../../src/core/search/fts-indexes.js'); + + const missing = await verifySearchFTSIndexes(executeQuery); + + expect(missing).toEqual([]); + expect(executeQuery).toHaveBeenCalledTimes(5); + }); + + it('reports missing indexes when an FTS probe fails', async () => { + const executeQuery = vi + .fn() + .mockResolvedValueOnce([]) + .mockRejectedValueOnce(new Error('index does not exist')) + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([]); + const { verifySearchFTSIndexes } = await import('../../src/core/search/fts-indexes.js'); + + const missing = await verifySearchFTSIndexes(executeQuery); + + expect(missing).toEqual(['Function.function_fts']); + }); }); describe('searchFTSFromLbug', () => { diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 1ad46ba72..8c69a23d5 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -203,7 +203,7 @@ describe('LocalBackend.callTool', () => { const result = await backend.callTool('query', { query: 'ProcessActivity' }); expect(result).toHaveProperty('warning'); - expect((result as any).warning).toMatch(/gitnexus analyze --force/); + expect((result as any).warning).toMatch(/gitnexus analyze --repair-fts/); }); it('does not include warning when ftsAvailable is true with zero results', async () => { diff --git a/gitnexus/test/unit/cli-index-help.test.ts b/gitnexus/test/unit/cli-index-help.test.ts index 889a7473f..00f5c0572 100644 --- a/gitnexus/test/unit/cli-index-help.test.ts +++ b/gitnexus/test/unit/cli-index-help.test.ts @@ -76,4 +76,11 @@ describe('CLI help surface', () => { expect(result.stdout).toContain('understand-quickly'); expect(result.stdout).toContain('UNDERSTAND_QUICKLY_TOKEN'); }); + + it('analyze help includes the FTS repair option', () => { + const result = runHelp('analyze'); + + expect(result.status).toBe(0); + expect(result.stdout).toContain('--repair-fts'); + }); }); diff --git a/gitnexus/test/unit/run-analyze-fts-repair.test.ts b/gitnexus/test/unit/run-analyze-fts-repair.test.ts new file mode 100644 index 000000000..c35aae6e8 --- /dev/null +++ b/gitnexus/test/unit/run-analyze-fts-repair.test.ts @@ -0,0 +1,243 @@ +import fs from 'fs/promises'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { getStoragePaths, saveMeta } from '../../src/storage/repo-manager.js'; +import { createTempDir } from '../helpers/test-db.js'; + +const SIMULATED_MISSING_FTS_INDEX_NAME = 'File.file_fts'; +const PLACEHOLDER_GRAPH_STORE_CONTENT = 'fixture'; + +const createPlaceholderGraphStore = async (lbugPath: string): Promise => { + // Repair mode gates on existence before `initLbug` takes over open/validate. + // A placeholder file is enough to exercise this preflight branch. + await fs.writeFile(lbugPath, PLACEHOLDER_GRAPH_STORE_CONTENT); +}; + +const escapeForRegex = (value: string): string => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +describe('runFullAnalysis FTS repair and verification failure paths', () => { + afterEach(() => { + vi.doUnmock('../../src/core/lbug/lbug-adapter.js'); + vi.doUnmock('../../src/core/search/fts-indexes.js'); + vi.doUnmock('../../src/core/ingestion/pipeline.js'); + vi.resetModules(); + vi.clearAllMocks(); + }); + + it('fails repair mode when no base meta exists', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-repair-no-meta-'); + try { + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + await expect( + runFullAnalysis( + tmpRepo.dbPath, + { repairFts: true }, + { + onProgress: () => {}, + }, + ), + ).rejects.toThrow(/has not been analyzed yet/i); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('fails repair mode when graph store is missing', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-repair-missing-store-'); + try { + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, { + repoPath: tmpRepo.dbPath, + lastCommit: '', + indexedAt: new Date().toISOString(), + stats: {}, + }); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + await expect( + runFullAnalysis( + tmpRepo.dbPath, + { repairFts: true }, + { + onProgress: () => {}, + }, + ), + ).rejects.toThrow(new RegExp(`graph store at ${escapeForRegex(lbugPath)} is missing`)); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('fails repair mode when graph store path is not a file', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-repair-store-not-file-'); + try { + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, { + repoPath: tmpRepo.dbPath, + lastCommit: '', + indexedAt: new Date().toISOString(), + stats: {}, + }); + await fs.mkdir(lbugPath, { recursive: true }); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + await expect( + runFullAnalysis( + tmpRepo.dbPath, + { repairFts: true }, + { + onProgress: () => {}, + }, + ), + ).rejects.toThrow( + new RegExp( + `graph store at ${escapeForRegex(lbugPath)} is a directory \\(expected a file\\)`, + ), + ); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('fails repair mode when FTS verify still reports missing indexes', async () => { + const closeLbugMock = vi.fn(async () => undefined); + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ + initLbug: vi.fn(async () => undefined), + loadGraphToLbug: vi.fn(async () => undefined), + getLbugStats: vi.fn(async () => ({})), + executeQuery: vi.fn(async () => []), + executeWithReusedStatement: vi.fn(async () => []), + closeLbug: closeLbugMock, + loadCachedEmbeddings: vi.fn(async () => ({ embeddingNodeIds: new Set(), embeddings: [] })), + deleteNodesForFile: vi.fn(async () => undefined), + deleteAllCommunitiesAndProcesses: vi.fn(async () => undefined), + queryImporters: vi.fn(async () => []), + })); + vi.doMock('../../src/core/search/fts-indexes.js', () => ({ + createSearchFTSIndexes: vi.fn(async () => undefined), + verifySearchFTSIndexes: vi.fn(async () => [SIMULATED_MISSING_FTS_INDEX_NAME]), + })); + + const tmpRepo = await createTempDir('gitnexus-run-analyze-repair-verify-fail-'); + try { + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, { + repoPath: tmpRepo.dbPath, + lastCommit: '', + indexedAt: new Date().toISOString(), + stats: {}, + }); + await createPlaceholderGraphStore(lbugPath); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + await expect( + runFullAnalysis( + tmpRepo.dbPath, + { repairFts: true }, + { + onProgress: () => {}, + }, + ), + ).rejects.toThrow(/FTS repair failed - missing indexes after rebuild/i); + expect(closeLbugMock).toHaveBeenCalled(); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('surfaces extension-unavailable errors from FTS index creation in repair mode', async () => { + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ + initLbug: vi.fn(async () => undefined), + loadGraphToLbug: vi.fn(async () => undefined), + getLbugStats: vi.fn(async () => ({})), + executeQuery: vi.fn(async () => []), + executeWithReusedStatement: vi.fn(async () => []), + closeLbug: vi.fn(async () => undefined), + loadCachedEmbeddings: vi.fn(async () => ({ embeddingNodeIds: new Set(), embeddings: [] })), + deleteNodesForFile: vi.fn(async () => undefined), + deleteAllCommunitiesAndProcesses: vi.fn(async () => undefined), + queryImporters: vi.fn(async () => []), + })); + vi.doMock('../../src/core/search/fts-indexes.js', () => ({ + createSearchFTSIndexes: vi.fn(async () => { + throw new Error('FTS extension unavailable'); + }), + verifySearchFTSIndexes: vi.fn(async () => []), + })); + + const tmpRepo = await createTempDir('gitnexus-run-analyze-repair-extension-fail-'); + try { + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + await saveMeta(storagePath, { + repoPath: tmpRepo.dbPath, + lastCommit: '', + indexedAt: new Date().toISOString(), + stats: {}, + }); + await createPlaceholderGraphStore(lbugPath); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + await expect( + runFullAnalysis( + tmpRepo.dbPath, + { repairFts: true }, + { + onProgress: () => {}, + }, + ), + ).rejects.toThrow(/FTS extension unavailable/i); + } finally { + await tmpRepo.cleanup(); + } + }); + + it('fails full analyze when FTS verification reports missing indexes after creation', async () => { + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ + initLbug: vi.fn(async () => undefined), + loadGraphToLbug: vi.fn(async () => undefined), + getLbugStats: vi.fn(async () => ({ nodes: 0, edges: 0, communities: 0, processes: 0 })), + executeQuery: vi.fn(async () => []), + executeWithReusedStatement: vi.fn(async () => []), + closeLbug: vi.fn(async () => undefined), + loadCachedEmbeddings: vi.fn(async () => ({ embeddingNodeIds: new Set(), embeddings: [] })), + deleteNodesForFile: vi.fn(async () => undefined), + deleteAllCommunitiesAndProcesses: vi.fn(async () => undefined), + queryImporters: vi.fn(async () => []), + })); + vi.doMock('../../src/core/search/fts-indexes.js', () => ({ + createSearchFTSIndexes: vi.fn(async () => undefined), + verifySearchFTSIndexes: vi.fn(async () => ['Function.function_fts']), + })); + vi.doMock('../../src/core/ingestion/pipeline.js', () => ({ + runPipelineFromRepo: vi.fn(async (repoPath: string) => ({ + repoPath, + // Full-analyze path only needs `forEachNode` before the FTS verify guard. + graph: { forEachNode: () => undefined }, + })), + })); + + const tmpRepo = await createTempDir('gitnexus-run-analyze-full-verify-fail-'); + try { + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await expect( + runFullAnalysis( + tmpRepo.dbPath, + { force: true }, + { + onProgress: () => {}, + }, + ), + ).rejects.toThrow(/FTS verification failed - missing indexes after analyze/i); + } finally { + await tmpRepo.cleanup(); + } + }); +}); diff --git a/gitnexus/test/unit/skip-git-cli.test.ts b/gitnexus/test/unit/skip-git-cli.test.ts index 80c07ed17..3b9b7b7e8 100644 --- a/gitnexus/test/unit/skip-git-cli.test.ts +++ b/gitnexus/test/unit/skip-git-cli.test.ts @@ -6,6 +6,32 @@ import fs from 'fs'; describe('--skip-git CLI flag', () => { const cliPath = path.resolve(__dirname, '../../dist/cli/index.js'); + const ftsUnavailableMessage = 'FTS extension unavailable - cannot create FTS index'; + + interface ExecSyncLikeError { + message?: string; + stdout?: string | Buffer; + stderr?: string | Buffer; + } + + const isFtsUnavailableError = (err: unknown): boolean => { + if (!err || typeof err !== 'object') return false; + const e = err as ExecSyncLikeError; + return ( + e.message?.includes(ftsUnavailableMessage) || + e.stdout?.toString().includes(ftsUnavailableMessage) || + e.stderr?.toString().includes(ftsUnavailableMessage) + ); + }; + + const shouldSkipForFtsUnavailable = (err: unknown, testName: string): boolean => { + if (!isFtsUnavailableError(err)) return false; + + console.warn( + `[skip-git-cli.test] Skipping "${testName}" because FTS extension is unavailable.`, + ); + return true; + }; it('Commander maps --skip-git to options.skipGit (not --no-git inversion)', () => { // Verify the CLI defines --skip-git and --skip-agents-md in analyze help. @@ -37,14 +63,26 @@ describe('--skip-git CLI flag', () => { ...process.env, HOME: gitnexusHome, GITNEXUS_HOME: gitnexusHome, - GITNEXUS_LBUG_EXTENSION_INSTALL: 'never', }; try { - const output = execSync( - `node "${cliPath}" analyze "${tmpDir}" --index-only --skills --skip-agents-md`, - { encoding: 'utf8', timeout: 60000, env }, - ); + let output: string; + try { + output = execSync( + `node "${cliPath}" analyze "${tmpDir}" --index-only --skills --skip-agents-md`, + { + encoding: 'utf8', + timeout: 60000, + env, + }, + ); + } catch (err: unknown) { + if ( + shouldSkipForFtsUnavailable(err, 'warns when --index-only overrides --skills (PR 1485)') + ) + return; + throw err; + } expect(output).toContain('--index-only overrides --skills'); } finally { fs.rmSync(tmpDir, { recursive: true, force: true }); @@ -87,15 +125,25 @@ describe('--skip-git CLI flag', () => { ...process.env, HOME: gitnexusHome, GITNEXUS_HOME: gitnexusHome, - GITNEXUS_LBUG_EXTENSION_INSTALL: 'never', }; try { - execSync(`node "${cliPath}" analyze "${tmpDir}" --skip-git --skip-agents-md`, { - encoding: 'utf8', - timeout: 60000, - env, - }); + try { + execSync(`node "${cliPath}" analyze "${tmpDir}" --skip-git --skip-agents-md`, { + encoding: 'utf8', + timeout: 60000, + env, + }); + } catch (err: unknown) { + if ( + shouldSkipForFtsUnavailable( + err, + 'still respects .gitnexusignore when run with --skip-git', + ) + ) + return; + throw err; + } const keepContext = execSync( `node "${cliPath}" context keep --repo "${path.basename(tmpDir)}"`, @@ -130,9 +178,8 @@ describe('--skip-git CLI flag', () => { function testEnv() { return { ...process.env, - HOME: parentDir, + HOME: gitnexusHome, GITNEXUS_HOME: gitnexusHome, - GITNEXUS_LBUG_EXTENSION_INSTALL: 'never', }; } @@ -221,12 +268,24 @@ describe('--skip-git CLI flag', () => { createTestStructure(); try { // Run analyze from COOLIO with --skip-git - const output = execSync(`node "${cliPath}" analyze --skip-git --skip-agents-md`, { - cwd: path.join(parentDir, 'COOLIO'), - encoding: 'utf8', - timeout: 60000, - env: testEnv(), - }); + let output: string; + try { + output = execSync(`node "${cliPath}" analyze --skip-git --skip-agents-md`, { + cwd: path.join(parentDir, 'COOLIO'), + encoding: 'utf8', + timeout: 60000, + env: testEnv(), + }); + } catch (err: unknown) { + if ( + shouldSkipForFtsUnavailable( + err, + 'from subdir inside parent git repo, indexes subdir not parent', + ) + ) + return; + throw err; + } // Should mention COOLIO not the parent dir name expect(output).toContain('COOLIO'); @@ -255,12 +314,23 @@ describe('--skip-git CLI flag', () => { stdio: 'ignore', }); - execSync(`node "${cliPath}" analyze --skip-git --skip-agents-md`, { - cwd: path.join(parentDir, 'COOLIO'), - encoding: 'utf8', - timeout: 60000, - env: testEnv(), - }); + try { + execSync(`node "${cliPath}" analyze --skip-git --skip-agents-md`, { + cwd: path.join(parentDir, 'COOLIO'), + encoding: 'utf8', + timeout: 60000, + env: testEnv(), + }); + } catch (err: unknown) { + if ( + shouldSkipForFtsUnavailable( + err, + 'keeps parent git status clean for --skip-git subdir analyze (#1233)', + ) + ) + return; + throw err; + } expect( fs.readFileSync(path.join(parentDir, 'COOLIO', '.gitnexus', '.gitignore'), 'utf8'), @@ -278,12 +348,21 @@ describe('--skip-git CLI flag', () => { it('explicit input path with --skip-git indexes subdir', () => { createTestStructure(); try { - const output = execSync(`node "${cliPath}" analyze ./COOLIO --skip-git --skip-agents-md`, { - cwd: parentDir, - encoding: 'utf8', - timeout: 60000, - env: testEnv(), - }); + let output: string; + try { + output = execSync(`node "${cliPath}" analyze ./COOLIO --skip-git --skip-agents-md`, { + cwd: parentDir, + encoding: 'utf8', + timeout: 60000, + env: testEnv(), + }); + } catch (err: unknown) { + if ( + shouldSkipForFtsUnavailable(err, 'explicit input path with --skip-git indexes subdir') + ) + return; + throw err; + } expect(output).toContain('COOLIO'); expectCoolioRegistryEntry(); From df1882d36b45dc2cd9b83d1ab3ae648a92db9a01 Mon Sep 17 00:00:00 2001 From: jelsco <58397194+jelsco@users.noreply.github.com> Date: Wed, 20 May 2026 07:37:58 -0600 Subject: [PATCH 3/5] fix(ingestion): surface skipped large-file paths by default (#1659) (#1661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ingestion): surface skipped large-file paths by default (#1659) The 512 KB skip threshold in filesystem-walker is necessary, but the existing warning only said "Skipped N large files" with no paths unless GITNEXUS_VERBOSE=1 was set. In a repo with one or two oversized first- party source files (e.g. a 17K-line cron handler), every IMPORTS/CALLS edge from that file silently disappeared and the surface looked like a Python resolver bug. Issue #1659 was filed against the resolver for exactly that reason, but the resolver was fine; the file was being dropped before parse. Changes: * Always print up to 5 skipped paths after the count line. * If more than 5 were skipped, append "...and N more" with a hint to set GITNEXUS_VERBOSE=1 for the full list. * When running at the default threshold, emit a one-line hint about GITNEXUS_MAX_FILE_SIZE= so operators know how to widen it. * Cover the new behavior with three additional tests in the existing filesystem-walker integration suite, plus a new describe block for the >5 preview-cap case. Verified end-to-end on a 680-file Python repo that hit #1659: before the patch, "Skipped 3 large files (>512KB, ...)" was the only signal and impact upstream of a function called from cron.py returned 1 of 5 real callers; after the patch the cron file is listed by name with the hint, and running with GITNEXUS_MAX_FILE_SIZE=1024 brings the missing callers back (impactedCount 1 -> 9). * fix(ingestion): address #1661 adversarial review follow-ups (F1/F2/F3) Three non-blocking nits flagged by the adversarial review on #1661: F1 (output stability) — skippedLargePaths was populated by concurrent fs.stat callbacks in batches of 32, so push order within a batch was completion-order rather than input-order. The default preview's "first 5" could vary across runs on the same repo. Fix: sort the array before slicing. New test asserts the verbose output is in sorted order. F2 (boundary coverage) — the preview-cap describe block created 8 large files, so the SKIPPED_PREVIEW_CAP = 5 comparison was never exercised at the exact <= boundary. A future off-by-one (<= → <) would not fail the suite. Fix: add two tests, one with exactly 5 files (all listed, no truncation) and one with exactly 6 files (5 listed plus "...and 1 more"). F3 (hint accuracy) — isDefault compared effective bytes, so an operator who explicitly set GITNEXUS_MAX_FILE_SIZE=512 (the same KB as the default) would still see the "Set GITNEXUS_MAX_FILE_SIZE=..." hint. Fix: gate the hint on whether the env var is unset, not on the resulting byte value. New test pins the explicit-default-value case. All 34 filesystem-walker tests pass (was 30; +4 new). Prettier clean, typecheck clean for the changed files. --------- Co-authored-by: scotjelinski <58397194+scotjelinski@users.noreply.github.com> Co-authored-by: Gergő Magyar --- .../src/core/ingestion/filesystem-walker.ts | 27 ++- .../integration/filesystem-walker.test.ts | 167 ++++++++++++++++++ 2 files changed, 190 insertions(+), 4 deletions(-) diff --git a/gitnexus/src/core/ingestion/filesystem-walker.ts b/gitnexus/src/core/ingestion/filesystem-walker.ts index 4d6725e24..c30ea4321 100644 --- a/gitnexus/src/core/ingestion/filesystem-walker.ts +++ b/gitnexus/src/core/ingestion/filesystem-walker.ts @@ -74,12 +74,31 @@ export const walkRepositoryPaths = async ( if (skippedLarge > 0) { const isDefault = maxFileSizeBytes === DEFAULT_MAX_FILE_SIZE_BYTES; + const isOverrideUnset = !process.env.GITNEXUS_MAX_FILE_SIZE; const suffix = isDefault ? ', likely generated/vendored' : ''; logger.warn(` Skipped ${skippedLarge} large files (>${maxFileSizeBytes / 1024}KB${suffix})`); - if (isVerboseIngestionEnabled()) { - for (const p of skippedLargePaths) { - logger.warn(` - ${p}`); - } + + // Always show at least the first few paths so users can diagnose why + // edges are missing from a specific file (issue #1659). The full list is + // gated behind GITNEXUS_VERBOSE=1 to avoid flooding output on repos with + // many generated/vendored blobs. Sort before slicing so the preview is + // stable across runs (fs.stat callbacks race within each batch). + skippedLargePaths.sort(); + const SKIPPED_PREVIEW_CAP = 5; + const showAll = isVerboseIngestionEnabled() || skippedLargePaths.length <= SKIPPED_PREVIEW_CAP; + const preview = showAll ? skippedLargePaths : skippedLargePaths.slice(0, SKIPPED_PREVIEW_CAP); + for (const p of preview) { + logger.warn(` - ${p}`); + } + if (!showAll) { + const remaining = skippedLargePaths.length - SKIPPED_PREVIEW_CAP; + logger.warn(` ...and ${remaining} more (set GITNEXUS_VERBOSE=1 to list them all)`); + } + // Only hint about the env var when the user has not set it at all. An + // explicit GITNEXUS_MAX_FILE_SIZE=512 happens to resolve to the same + // bytes as the default but the operator clearly already knows the knob. + if (isDefault && isOverrideUnset) { + logger.warn(` Set GITNEXUS_MAX_FILE_SIZE= to include files above the default cap.`); } } diff --git a/gitnexus/test/integration/filesystem-walker.test.ts b/gitnexus/test/integration/filesystem-walker.test.ts index 2a1257ccc..1d743a898 100644 --- a/gitnexus/test/integration/filesystem-walker.test.ts +++ b/gitnexus/test/integration/filesystem-walker.test.ts @@ -398,5 +398,172 @@ describe('filesystem-walker', () => { expect(skipWarnings.length).toBeGreaterThan(0); expect(String(skipWarnings[0].msg ?? '')).toContain('generated/vendored'); }); + + // Regression: issue #1659. The skipped-paths list and the + // GITNEXUS_MAX_FILE_SIZE hint must appear by default, otherwise users + // see "Skipped N large files" with no actionable detail and misdiagnose + // missing IMPORTS/CALLS edges as a resolver bug. + it('lists the skipped path by default (not gated behind GITNEXUS_VERBOSE)', async () => { + await walkRepositoryPaths(sizeDir); + const pathWarnings = cap.records().filter((r) => String(r.msg ?? '').includes(BIG_FILE)); + expect(pathWarnings.length).toBeGreaterThan(0); + }); + + it('emits a GITNEXUS_MAX_FILE_SIZE hint when running with the default cap', async () => { + await walkRepositoryPaths(sizeDir); + const hint = cap + .records() + .filter((r) => String(r.msg ?? '').includes('GITNEXUS_MAX_FILE_SIZE=')); + expect(hint.length).toBe(1); + }); + + it('omits the GITNEXUS_MAX_FILE_SIZE hint when an override is active', async () => { + process.env.GITNEXUS_MAX_FILE_SIZE = '1'; + await walkRepositoryPaths(sizeDir); + const hint = cap + .records() + .filter((r) => String(r.msg ?? '').includes('GITNEXUS_MAX_FILE_SIZE=')); + expect(hint.length).toBe(0); + }); + + // Edge case from the #1661 adversarial review: setting GITNEXUS_MAX_FILE_SIZE + // to the same value as the default (512KB) used to still print the hint + // because the byte comparison resolved to equal. The hint should care + // about whether the operator set the env var, not what value they chose. + it('omits the GITNEXUS_MAX_FILE_SIZE hint when the override equals the default value', async () => { + process.env.GITNEXUS_MAX_FILE_SIZE = '512'; + await walkRepositoryPaths(sizeDir); + const hint = cap + .records() + .filter((r) => String(r.msg ?? '').includes('GITNEXUS_MAX_FILE_SIZE=')); + expect(hint.length).toBe(0); + }); + }); + + describe('large file skip preview cap (#1659)', () => { + let manyDir: string; + const ORIGINAL_ENV = process.env.GITNEXUS_MAX_FILE_SIZE; + const ORIGINAL_VERBOSE = process.env.GITNEXUS_VERBOSE; + let cap: ReturnType; + + beforeAll(async () => { + manyDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-walker-size-many-')); + await fs.mkdir(path.join(manyDir, 'src'), { recursive: true }); + // 8 files >512KB so the preview-cap path (5) is exercised. + for (let i = 0; i < 8; i++) { + await fs.writeFile(path.join(manyDir, 'src', `big${i}.ts`), 'x'.repeat(600 * 1024)); + } + }); + + afterAll(async () => { + await fs.rm(manyDir, { recursive: true, force: true }); + }); + + beforeEach(() => { + delete process.env.GITNEXUS_MAX_FILE_SIZE; + delete process.env.GITNEXUS_VERBOSE; + _resetMaxFileSizeWarnings(); + cap = _captureLogger(); + }); + + afterEach(() => { + if (ORIGINAL_ENV === undefined) { + delete process.env.GITNEXUS_MAX_FILE_SIZE; + } else { + process.env.GITNEXUS_MAX_FILE_SIZE = ORIGINAL_ENV; + } + if (ORIGINAL_VERBOSE === undefined) { + delete process.env.GITNEXUS_VERBOSE; + } else { + process.env.GITNEXUS_VERBOSE = ORIGINAL_VERBOSE; + } + cap.restore(); + }); + + it('truncates the path list to 5 and mentions GITNEXUS_VERBOSE when over the cap', async () => { + await walkRepositoryPaths(manyDir); + const pathLines = cap.records().filter((r) => /^\s*-\s/.test(String(r.msg ?? ''))); + expect(pathLines.length).toBe(5); + const more = cap + .records() + .filter((r) => String(r.msg ?? '').includes('and 3 more (set GITNEXUS_VERBOSE=1')); + expect(more.length).toBe(1); + }); + + // Boundary check from the #1661 adversarial review: the SKIPPED_PREVIEW_CAP + // comparison is `<=`, so 5 paths should list all five without a truncation + // line and 6 paths should list exactly five plus "...and 1 more". Tested + // explicitly so a future off-by-one refactor (`<=` → `<`) fails fast. + it('lists all paths and omits the truncation line at exactly 5 skipped files', async () => { + const fiveDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-walker-size-five-')); + try { + await fs.mkdir(path.join(fiveDir, 'src'), { recursive: true }); + for (let i = 0; i < 5; i++) { + await fs.writeFile(path.join(fiveDir, 'src', `big${i}.ts`), 'x'.repeat(600 * 1024)); + } + await walkRepositoryPaths(fiveDir); + const pathLines = cap.records().filter((r) => /^\s*-\s/.test(String(r.msg ?? ''))); + expect(pathLines.length).toBe(5); + const more = cap.records().filter((r) => String(r.msg ?? '').includes('...and ')); + expect(more.length).toBe(0); + } finally { + await fs.rm(fiveDir, { recursive: true, force: true }); + } + }); + + it('lists exactly 5 paths plus "...and 1 more" at exactly 6 skipped files', async () => { + const sixDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-walker-size-six-')); + try { + await fs.mkdir(path.join(sixDir, 'src'), { recursive: true }); + for (let i = 0; i < 6; i++) { + await fs.writeFile(path.join(sixDir, 'src', `big${i}.ts`), 'x'.repeat(600 * 1024)); + } + await walkRepositoryPaths(sixDir); + const pathLines = cap.records().filter((r) => /^\s*-\s/.test(String(r.msg ?? ''))); + expect(pathLines.length).toBe(5); + const more = cap + .records() + .filter((r) => String(r.msg ?? '').includes('and 1 more (set GITNEXUS_VERBOSE=1')); + expect(more.length).toBe(1); + } finally { + await fs.rm(sixDir, { recursive: true, force: true }); + } + }); + + it('lists every skipped path when GITNEXUS_VERBOSE=1', async () => { + process.env.GITNEXUS_VERBOSE = '1'; + await walkRepositoryPaths(manyDir); + const pathLines = cap.records().filter((r) => /^\s*-\s/.test(String(r.msg ?? ''))); + expect(pathLines.length).toBe(8); + const more = cap.records().filter((r) => String(r.msg ?? '').includes('and ')); + expect(more.length).toBe(0); + }); + + // Issue #1659 follow-up (PR #1661 review): paths were pushed in fs.stat + // completion order, so the default preview could vary between runs on + // the same repo. The implementation sorts skippedLargePaths before + // slicing, so the listed paths come out in sorted order, which is the + // stable contract operators can rely on. + it('lists skipped paths in sorted order (deterministic preview)', async () => { + process.env.GITNEXUS_VERBOSE = '1'; + await walkRepositoryPaths(manyDir); + const pathLines = cap + .records() + .map((r) => String(r.msg ?? '')) + .filter((m) => /^\s*-\s/.test(m)) + .map((m) => m.replace(/^\s*-\s*/, '')); + expect(pathLines).toEqual([...pathLines].sort()); + // sanity-check we actually saw all 8 of the manyDir fixture + expect(pathLines).toEqual([ + 'src/big0.ts', + 'src/big1.ts', + 'src/big2.ts', + 'src/big3.ts', + 'src/big4.ts', + 'src/big5.ts', + 'src/big6.ts', + 'src/big7.ts', + ]); + }); }); }); From 4d2ed0e52501104450a2c9200a543d1633690875 Mon Sep 17 00:00:00 2001 From: Shane Thurston Wijaya <129602553+sanguine59@users.noreply.github.com> Date: Wed, 20 May 2026 22:14:13 +0700 Subject: [PATCH 4/5] fix(eval-server): localhost now doesn't normalize into IPv4 instead lets OS decide which to bind (#1722) * fix(eval-server): localhost now doesn't normalize into IPv4 instead lets OS decide which to bind * fix(eval-server): EADDRNOTAVAIL now treats as potential IPv6 * test(eval-server): new integration test for --host localhost * docs(eval-server): updated eval/README.md based on latest update * fix(eval-server): clarify EADDRNOTAVAIL diagnostic, guard server.address(), and soften localhost docs --- eval/README.md | 2 + gitnexus/src/cli/eval-server.ts | 32 +++++-- gitnexus/test/integration/cli-e2e.test.ts | 97 ++++++++++++++++++++++ gitnexus/test/unit/eval-formatters.test.ts | 4 +- 4 files changed, 124 insertions(+), 11 deletions(-) diff --git a/eval/README.md b/eval/README.md index e3539cf20..1b01bce59 100644 --- a/eval/README.md +++ b/eval/README.md @@ -211,6 +211,8 @@ environment: Defaults are `port: 4848` and `host: 127.0.0.1` (loopback only). Use `0.0.0.0` only when the agent container needs to reach the eval-server from a separate network namespace. The health probe and tool scripts connect via the configured bind host (defaulting to `127.0.0.1`), which is reachable for both loopback and all-interface binds. +`"localhost"` is also a valid `eval_server_host` value. The OS resolves it at bind time — typically `127.0.0.1` on dual-stack or IPv4-only systems, and `::1` on IPv6-only systems. The exact result depends on your `/etc/hosts` and `gai.conf`. The READY signal will reflect the actual bound address (e.g. `GITNEXUS_EVAL_SERVER_READY:127.0.0.1:4848` or `GITNEXUS_EVAL_SERVER_READY:[::1]:4848`), not the literal string `localhost`. Use this when you want the server to bind to whichever loopback address the OS prefers rather than forcing IPv4. + **Running eval-server directly in Docker / Docker Compose:** ```bash diff --git a/gitnexus/src/cli/eval-server.ts b/gitnexus/src/cli/eval-server.ts index 8735abbef..88ad10592 100644 --- a/gitnexus/src/cli/eval-server.ts +++ b/gitnexus/src/cli/eval-server.ts @@ -44,10 +44,12 @@ export interface EvalServerOptions { /** * Validate the --host value. Accepts IPv4, IPv6, or "localhost". - * Returns the normalised host string, or null if invalid. + * Returns the host string unchanged, or null if invalid. + * "localhost" is passed through so the OS resolves it to the correct loopback + * address (127.0.0.1 or ::1) at bind time rather than forcing IPv4. */ export function validateHost(raw: string): string | null { - if (raw === 'localhost') return '127.0.0.1'; + if (raw === 'localhost') return raw; if (isIPv4(raw) || isIPv6(raw)) return raw; return null; } @@ -470,12 +472,14 @@ export async function evalServerCommand(options?: EvalServerOptions): Promise { }, 30000); }); }, 35000); + + it('emits READY signal with bound IP (not literal "localhost") when --host localhost is used', () => { + return new Promise((resolve, reject) => { + const child = spawn( + process.execPath, + [ + '--import', + tsxImportUrl, + cliEntry, + 'eval-server', + '--port', + '0', + '--host', + 'localhost', + '--idle-timeout', + '3', + ], + { + cwd: MINI_REPO, + stdio: ['ignore', 'pipe', 'pipe'], + env: cliEnv(), + }, + ); + + let stdoutBuffer = ''; + let settled = false; + + const settle = (fn: () => void) => { + if (settled) return; + settled = true; + clearTimeout(timer); + child.kill('SIGTERM'); + fn(); + }; + + child.stdout.on('data', async (chunk: Buffer) => { + stdoutBuffer += chunk.toString(); + const readyLine = stdoutBuffer + .split('\n') + .find((l) => l.startsWith('GITNEXUS_EVAL_SERVER_READY:')); + if (!readyLine || settled) return; + + // The signal must contain a real bound IP, not the literal input string + if (readyLine.includes(':localhost:')) { + settle(() => + reject( + new Error( + `READY signal contained literal "localhost" instead of a bound IP:\n${readyLine}`, + ), + ), + ); + return; + } + + // Parse host and port: everything after the prefix up to the last colon + const withoutPrefix = readyLine.slice('GITNEXUS_EVAL_SERVER_READY:'.length); + const lastColon = withoutPrefix.lastIndexOf(':'); + const signalHost = withoutPrefix.slice(0, lastColon); // "127.0.0.1" or "[::1]" + const boundPort = withoutPrefix.slice(lastColon + 1).trim(); + if (!boundPort || isNaN(Number(boundPort))) { + settle(() => reject(new Error(`Could not parse port from READY signal: ${readyLine}`))); + return; + } + + // Probe /health at the bound address to confirm the server is reachable + try { + const res = await fetch(`http://${signalHost}:${boundPort}/health`); + if (res.status === 200) { + settle(resolve); + } else { + settle(() => reject(new Error(`/health returned ${res.status}, expected 200`))); + } + } catch (err) { + settle(() => + reject( + new Error( + `eval-server bound to localhost but /health unreachable at ${signalHost}:${boundPort}: ${err}`, + ), + ), + ); + } + }); + + child.stderr.on('data', (chunk: Buffer) => { + const text = chunk.toString(); + if (text.includes('unknown option') || text.includes('error: unknown')) { + settle(() => reject(new Error(`eval-server rejected --host flag:\n${text}`))); + } + }); + + const timer = setTimeout(() => { + settle(() => + reject(new Error('eval-server --host localhost did not emit READY signal within 30s')), + ); + }, 30000); + }); + }, 35000); }); }); diff --git a/gitnexus/test/unit/eval-formatters.test.ts b/gitnexus/test/unit/eval-formatters.test.ts index 6cefbcdca..cfc09acd1 100644 --- a/gitnexus/test/unit/eval-formatters.test.ts +++ b/gitnexus/test/unit/eval-formatters.test.ts @@ -19,8 +19,8 @@ import { // ─── validateHost ──────────────────────────────────────────────────── describe('validateHost', () => { - it('normalizes "localhost" to "127.0.0.1"', () => { - expect(validateHost('localhost')).toBe('127.0.0.1'); + it('passes "localhost" through unchanged', () => { + expect(validateHost('localhost')).toBe('localhost'); }); it('accepts valid IPv4 addresses', () => { From aa8f4d6efe928e9fadd1f16208810fe3ea870d2d Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Wed, 20 May 2026 17:44:07 +0100 Subject: [PATCH 5/5] fix(group): Union HTTP graph and source contracts (#1709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Union HTTP graph and source contracts * test(group): Document HTTP source union follow-ups --------- Co-authored-by: Gergő Magyar --- .../group/extractors/http-route-extractor.ts | 46 ++++-- .../unit/group/http-route-extractor.test.ts | 148 ++++++++++++++++++ 2 files changed, 180 insertions(+), 14 deletions(-) diff --git a/gitnexus/src/core/group/extractors/http-route-extractor.ts b/gitnexus/src/core/group/extractors/http-route-extractor.ts index 898a22c38..54aeb9150 100644 --- a/gitnexus/src/core/group/extractors/http-route-extractor.ts +++ b/gitnexus/src/core/group/extractors/http-route-extractor.ts @@ -18,12 +18,14 @@ import { getPluginForFile, HTTP_SCAN_GLOB, type HttpDetection } from './http-pat * the preferred path because the graph has richer symbol metadata * (real uids, class/method structure, etc.). * - * 2. **Source-scan fallback (Strategy B)** — parse files directly with - * the per-language plugin registry in `./http-patterns/`. Used when - * the graph has no routes/fetches for this repo (e.g. a repo that - * hasn't been indexed yet, or whose indexer doesn't know the - * framework). Each plugin owns its tree-sitter grammar and query - * sources — this orchestrator imports NO grammars or query strings. + * 2. **Source-scan supplement (Strategy B)** — parse files directly with + * the per-language plugin registry in `./http-patterns/`. Used to + * fill gaps when graph extraction only covers part of a polyglot repo + * (e.g. Java graph routes plus Go source-scan routes). Graph entries + * remain authoritative for duplicate contract IDs because they carry + * richer symbol metadata. Each plugin owns its tree-sitter grammar + * and query sources — this orchestrator imports NO grammars or query + * strings. * * Adding a new language for Strategy B is a one-file edit in * `http-patterns/index.ts`: register a new `HttpLanguagePlugin` and @@ -194,17 +196,19 @@ export class HttpRouteExtractor implements ContractExtractor { const graphProviders = dbExecutor != null ? await this.extractProvidersGraph(dbExecutor, getDetections) : []; - const providers = - graphProviders.length > 0 - ? graphProviders - : this.extractProvidersSourceScan(await getScannedFiles(), getDetections); + // Source scan always runs to capture routes in languages/files not covered + // by graph edges; the glob and per-file parse results are cached above. + const providers = this.mergeGraphAndSourceContracts( + graphProviders, + this.extractProvidersSourceScan(await getScannedFiles(), getDetections), + ); const graphConsumers = dbExecutor != null ? await this.extractConsumersGraph(dbExecutor, getDetections) : []; - const consumers = - graphConsumers.length > 0 - ? graphConsumers - : this.extractConsumersSourceScan(await getScannedFiles(), getDetections); + const consumers = this.mergeGraphAndSourceContracts( + graphConsumers, + this.extractConsumersSourceScan(await getScannedFiles(), getDetections), + ); return [...providers, ...consumers]; } @@ -473,4 +477,18 @@ export class HttpRouteExtractor implements ContractExtractor { } return out; } + + private mergeGraphAndSourceContracts( + graphContracts: ExtractedContract[], + sourceContracts: ExtractedContract[], + ): ExtractedContract[] { + const seenContractIds = new Set(graphContracts.map((c) => c.contractId)); + const out = [...graphContracts]; + for (const contract of sourceContracts) { + if (seenContractIds.has(contract.contractId)) continue; + seenContractIds.add(contract.contractId); + out.push(contract); + } + return out; + } } diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index aa648a71d..d2c1b3fa4 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -92,6 +92,77 @@ public class UserController { expect(getRoute!.confidence).toBe(0.9); expect(getRoute!.symbolUid).not.toBe('file-uid-ctrl'); }); + + it('supplements graph providers with source-scan providers from other files', async () => { + const dir = path.join(tmpDir, 'graph-source-provider-union'); + fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true }); + fs.mkdirSync(path.join(dir, 'cmd'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/controller/UserController.java'), + ` +@RestController +@RequestMapping("/api/v2") +public class UserController { + @GetMapping("/users") + public List list() { return service.findAll(); } +} +`, + ); + fs.writeFileSync( + path.join(dir, 'cmd/server.go'), + ` +package main + +func healthHandler(w http.ResponseWriter, r *http.Request) {} + +func main() { + http.HandleFunc("/api/health", healthHandler) +} +`, + ); + + const mockDbExecutor = async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'file-uid-ctrl', + filePath: 'src/controller/UserController.java', + routePath: '/api/v2/users', + routeId: 'route-uid-users', + responseKeys: null, + routeSource: 'decorator-GetMapping', + }, + ]; + } + if (query.includes('FETCHES')) return []; + if (query.includes('CONTAINS')) { + return [ + { + uid: 'uid-ctrl-list', + name: 'list', + filePath: 'src/controller/UserController.java', + labels: ['Method'], + }, + ]; + } + return []; + }; + + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const providers = contracts.filter((c) => c.role === 'provider'); + + const graphRouteMatches = providers.filter( + (c) => c.contractId === 'http::GET::/api/v2/users', + ); + expect(graphRouteMatches).toHaveLength(1); + expect(graphRouteMatches[0].symbolUid).toBe('uid-ctrl-list'); + expect(graphRouteMatches[0].meta.extractionStrategy).toBe('graph_assisted'); + + const sourceRoute = providers.find((c) => c.contractId === 'http::GET::/api/health'); + expect(sourceRoute).toBeDefined(); + expect(sourceRoute?.symbolName).toBe('healthHandler'); + expect(sourceRoute?.meta.extractionStrategy).toBe('source_scan'); + }); }); describe('provider extraction — source-scan fallback (Strategy B)', () => { @@ -166,6 +237,30 @@ export default router; ).toBeDefined(); }); + it('dedupes source-only providers by contract id', async () => { + const dir = path.join(tmpDir, 'source-only-same-contract-id'); + fs.mkdirSync(path.join(dir, 'src/routes'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/routes/health-a.ts'), + ` +router.get('/api/health', healthA); +`, + ); + fs.writeFileSync( + path.join(dir, 'src/routes/health-b.ts'), + ` +router.get('/api/health', healthB); +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const providers = contracts.filter((c) => c.contractId === 'http::GET::/api/health'); + + expect(providers).toHaveLength(1); + expect(providers[0].role).toBe('provider'); + expect(providers[0].meta.extractionStrategy).toBe('source_scan'); + }); + it('extracts Go Gin and Echo route registrations', async () => { const dir = path.join(tmpDir, 'go-frameworks'); fs.mkdirSync(path.join(dir, 'cmd'), { recursive: true }); @@ -740,6 +835,59 @@ async def create_user(user: UserCreate): expect(consumers[0].confidence).toBe(0.9); expect(consumers[0].symbolName).toBe('fetchUsers'); }); + + it('supplements graph consumers with source-scan consumers from other files', async () => { + const dir = path.join(tmpDir, 'graph-source-consumer-union'); + fs.mkdirSync(path.join(dir, 'src/api'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'src/api/graph.ts'), 'export const api = {};'); + fs.writeFileSync( + path.join(dir, 'src/api/health.ts'), + ` +export async function fetchHealth() { + const res = await fetch('/api/health'); + return res.json(); +} +`, + ); + + const mockDbExecutor = async (query: string) => { + if (query.includes('HANDLES_ROUTE')) return []; + if (query.includes('FETCHES')) { + return [ + { + fileId: 'file-uid-api', + filePath: 'src/api/graph.ts', + routePath: '/api/users', + routeId: 'route-uid-users', + fetchReason: 'fetch-url-match', + }, + ]; + } + if (query.includes('CONTAINS')) { + return [ + { + uid: 'uid-fn-fetch', + name: 'fetchUsers', + filePath: 'src/api/graph.ts', + labels: ['Function'], + }, + ]; + } + return []; + }; + + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + const graphConsumer = consumers.find((c) => c.contractId === 'http::GET::/api/users'); + expect(graphConsumer).toBeDefined(); + expect(graphConsumer?.symbolUid).toBe('uid-fn-fetch'); + expect(graphConsumer?.meta.extractionStrategy).toBe('graph_assisted'); + + const sourceConsumer = consumers.find((c) => c.contractId === 'http::GET::/api/health'); + expect(sourceConsumer).toBeDefined(); + expect(sourceConsumer?.meta.extractionStrategy).toBe('source_scan'); + }); }); describe('edge cases', () => {