diff --git a/gitnexus/src/core/ingestion/entry-point-scoring.ts b/gitnexus/src/core/ingestion/entry-point-scoring.ts index 58cf9389c..30a9c78c1 100644 --- a/gitnexus/src/core/ingestion/entry-point-scoring.ts +++ b/gitnexus/src/core/ingestion/entry-point-scoring.ts @@ -13,6 +13,7 @@ import { detectFrameworkFromPath } from './framework-detection.js'; import { SupportedLanguages } from 'gitnexus-shared'; import { providers } from './languages/index.js'; +import { isTestFilePath } from './utils/test-file-path.js'; // ============================================================================ // NAME PATTERNS @@ -164,54 +165,15 @@ export function calculateEntryPointScore( // ============================================================================ /** - * Check if a file path is a test file (should be excluded from entry points) - * Covers common test file patterns across all supported languages + * Check if a file path is a test file (should be excluded from entry points). + * + * Delegates to the shared predicate in `utils/test-file-path.ts`. This used to be + * a second, hand-maintained copy that had drifted from the one backing the MCP + * `includeTests` flag — see that module's header. Re-exported under this name so + * existing importers are unaffected. */ export function isTestFile(filePath: string): boolean { - const p = filePath.toLowerCase().replace(/\\/g, '/'); - - return ( - // JavaScript/TypeScript test patterns - p.includes('.test.') || - p.includes('.spec.') || - p.includes('__tests__/') || - p.includes('__mocks__/') || - // Generic test folders - p.includes('/test/') || - p.includes('/tests/') || - p.includes('/testing/') || - // Python test patterns - p.endsWith('_test.py') || - p.includes('/test_') || - // Go test patterns - p.endsWith('_test.go') || - // Java test patterns - p.includes('/src/test/') || - // Rust test patterns (inline tests are different, but test files) - p.includes('/tests/') || - // Swift/iOS test patterns - p.endsWith('tests.swift') || - p.endsWith('test.swift') || - p.includes('uitests/') || - // C# test patterns - p.endsWith('tests.cs') || - p.endsWith('test.cs') || - p.includes('.tests/') || - p.includes('.test/') || - p.includes('.integrationtests/') || - p.includes('.unittests/') || - p.includes('/testproject/') || - // PHP/Laravel test patterns - p.endsWith('test.php') || - p.endsWith('spec.php') || - p.includes('/tests/feature/') || - p.includes('/tests/unit/') || - // Ruby test patterns - p.endsWith('_spec.rb') || - p.endsWith('_test.rb') || - p.includes('/spec/') || - p.includes('/test/fixtures/') - ); + return isTestFilePath(filePath); } /** diff --git a/gitnexus/src/core/ingestion/utils/test-file-path.ts b/gitnexus/src/core/ingestion/utils/test-file-path.ts new file mode 100644 index 000000000..ceee59c56 --- /dev/null +++ b/gitnexus/src/core/ingestion/utils/test-file-path.ts @@ -0,0 +1,87 @@ +/** + * Test-file path classification — the single source of truth. + * + * WHY THIS MODULE EXISTS + * + * Two independent copies of this predicate existed and had drifted apart: + * + * - `core/ingestion/entry-point-scoring.ts` `isTestFile` — excludes test + * files from process entry-point detection. + * - `mcp/local/local-backend.ts` `isTestFilePath` — backs the + * `includeTests` flag on `impact` / `trace` / `context`. + * + * They answered "is this a test file?" differently, so the same path could be a + * test in one code path and not the other. The MCP copy recognized no C#, Java, + * or Swift test convention at all, meaning `includeTests: false` silently failed + * to filter them; the scoring copy missed `/conftest.` (it already matched + * `/test/`, so `/test/fixtures/` was never a scoring gap). + * + * The duplication was not gratuitous: `entry-point-scoring.ts` imports the + * language-provider registry, and #2802 deliberately cut that closure out of MCP + * server startup. Importing it back into `local-backend.ts` would reintroduce + * that cost. So the shared predicate lives here instead, with NO imports — pure + * string matching — and both callers delegate to it. + * + * Keep it dependency-free. Anything imported here lands in MCP startup. + */ + +/** + * Lowercase forward-slash path substrings. Directory needles include a leading + * slash so they match path components after the caller slash-prefixes relative + * paths. `/test/` already covers Maven `src/test` and `/test/fixtures/`; + * `/tests/` covers Laravel `tests/Feature` and `/tests/fixtures/`. + */ +const TEST_PATH_SUBSTRINGS: readonly string[] = [ + '.test.', + '.spec.', + '__tests__/', + '__mocks__/', + '/test/', + '/tests/', + '/testing/', + '/spec/', + '/test_', + '/conftest.', + '/uitests/', + '.tests/', + '.test/', + '.integrationtests/', + '.unittests/', + '/testproject/', +]; + +/** Case-insensitive suffixes that already include a delimiter (`_test.py`, not `test.py`). */ +const TEST_PATH_DELIMITED_SUFFIXES: readonly string[] = [ + '_test.py', + '_test.go', + '_spec.rb', + '_test.rb', +]; + +/** + * Case-sensitive `Test`/`Tests`/`Spec` suffixes. Lowercasing first would also + * match production names such as `Contest.swift` and `Latest.php`. + */ +const TEST_PATH_CASED_SUFFIXES: readonly string[] = [ + 'Tests.swift', + 'Test.swift', + 'Tests.cs', + 'Test.cs', + 'Test.php', + 'Spec.php', +]; + +/** Absent / empty paths are not test paths. */ +export function isTestFilePath(filePath: string | null | undefined): boolean { + if (!filePath) return false; + const slashed = filePath.replace(/\\/g, '/'); + const prefixed = slashed.startsWith('/') ? slashed : `/${slashed}`; + const lower = prefixed.toLowerCase(); + if (TEST_PATH_SUBSTRINGS.some((needle) => lower.includes(needle))) return true; + if (TEST_PATH_DELIMITED_SUFFIXES.some((suffix) => lower.endsWith(suffix))) return true; + // Xcode `{Product}UITests` targets. Slash-anchored `/uitests/` does not match + // `MyAppUITests`; an unanchored `uitests/` substring also matches `fruitests`. + if (prefixed.split('/').some((seg) => seg.endsWith('UITests'))) return true; + const basename = prefixed.slice(prefixed.lastIndexOf('/') + 1); + return TEST_PATH_CASED_SUFFIXES.some((suffix) => basename.endsWith(suffix)); +} diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index fa5170ae2..1480b0843 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -29,6 +29,7 @@ import { LBUG_ID_PROBE_BATCH_SIZE, LBUG_QUERY_BATCH_SIZE } from '../../core/lbug import { chunk, mapConcurrent } from '../../lib/utils.js'; import { pathSuffixOf } from './path-predicate.js'; import { toOneBasedLine } from '../../core/ingestion/utils/line-base.js'; +import { isTestFilePath } from '../../core/ingestion/utils/test-file-path.js'; import { isWalCorruptionError, WAL_RECOVERY_SUGGESTION } from '../../core/lbug/lbug-config.js'; // Embedding imports are lazy (dynamic import) to avoid loading onnxruntime-node // at MCP server startup — crashes on unsupported Node ABI versions (#89) @@ -313,31 +314,8 @@ function normalizeToolParams( // AI context generation is CLI-only (gitnexus analyze) // import { generateAIContextFiles } from '../../cli/ai-context.js'; -/** - * Quick test-file detection for filtering impact results. - * Matches common test file patterns across all supported languages. - */ -export function isTestFilePath(filePath: string | null | undefined): boolean { - if (!filePath) return false; - const p = filePath.toLowerCase().replace(/\\/g, '/'); - return ( - p.includes('.test.') || - p.includes('.spec.') || - p.includes('__tests__/') || - p.includes('__mocks__/') || - p.includes('/test/') || - p.includes('/tests/') || - p.includes('/testing/') || - p.includes('/fixtures/') || - p.endsWith('_test.go') || - p.endsWith('_test.py') || - p.endsWith('_spec.rb') || - p.endsWith('_test.rb') || - p.includes('/spec/') || - p.includes('/test_') || - p.includes('/conftest.') - ); -} +/** Shared predicate; re-exported so MCP importers keep the old public name. */ +export { isTestFilePath }; /** Valid LadybugDB node labels for safe Cypher query construction */ export const VALID_NODE_LABELS = new Set([ diff --git a/gitnexus/test/unit/test-file-path.test.ts b/gitnexus/test/unit/test-file-path.test.ts new file mode 100644 index 000000000..a4ffc71bd --- /dev/null +++ b/gitnexus/test/unit/test-file-path.test.ts @@ -0,0 +1,106 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { isTestFilePath } from '../../src/core/ingestion/utils/test-file-path.js'; +import { isTestFile } from '../../src/core/ingestion/entry-point-scoring.js'; +import { isTestFilePath as backendIsTestFilePath } from '../../src/mcp/local/local-backend.js'; + +describe('isTestFilePath — shared predicate', () => { + it('returns false for nullish input rather than throwing', () => { + expect(isTestFilePath(undefined)).toBe(false); + expect(isTestFilePath(null)).toBe(false); + expect(isTestFilePath('')).toBe(false); + }); + + it('normalizes Windows separators and casing', () => { + expect(isTestFilePath('SRC\\Test\\FooTests.cs')).toBe(true); + expect(isTestFilePath('pkg\\thing_test.go')).toBe(true); + expect(isTestFilePath('src\\Widgets.Tests\\WidgetTests.cs')).toBe(true); + }); + + // These were recognized by entry-point scoring but NOT by the MCP copy, so + // `includeTests: false` silently failed to filter them. + for (const p of [ + 'app/src/test/java/com/x/FooTest.java', + 'ios/MyAppTests/LoginTests.swift', + 'ios/MyAppUITests/FlowTest.swift', + 'ios/MyAppUITests/Flow.swift', + 'ios/MyAppUITests/README.swift', + 'src/Widgets.Tests/WidgetTests.cs', + 'src/Widgets.UnitTests/Thing.cs', + 'src/Widgets.IntegrationTests/Thing.cs', + 'tests/Feature/LoginTest.php', + 'tests/Unit/ThingSpec.php', + 'tests/Feature/Support/FakeGateway.php', + ]) { + it(`detects a test path the MCP copy used to miss: ${p}`, () => { + expect(isTestFilePath(p)).toBe(true); + }); + } + + // These were recognized by the MCP copy but NOT by entry-point scoring, so + // they could be selected as process entry points. + for (const p of ['tests/fixtures/sample.py', 'tests/conftest.py']) { + it(`detects a test path entry-point scoring used to miss: ${p}`, () => { + expect(isTestFilePath(p)).toBe(true); + }); + } + + for (const p of [ + 'src/app/widgets.ts', + 'src/core/ingestion/utils/test-file-path.ts', + 'pkg/service/handler.go', + 'app/models/user.rb', + 'Contest.swift', + 'Contest.cs', + 'Protest.cs', + 'Latest.php', + 'src/fixtures/schema.ts', + 'src/fruitests/helpers.swift', + ]) { + it(`does not classify production code as test: ${p}`, () => { + expect(isTestFilePath(p)).toBe(false); + }); + } +}); + +describe('test-file classification has exactly one implementation', () => { + // Regression guard: two hand-maintained copies drifted apart once already. + // Both public names must delegate to the same predicate. + const paths = [ + 'src/app/widgets.ts', + 'app/src/test/java/com/x/FooTest.java', + 'ios/MyAppTests/LoginTests.swift', + 'src/Widgets.Tests/WidgetTests.cs', + 'tests/conftest.py', + 'tests/fixtures/sample.py', + 'src/fixtures/schema.ts', + 'Contest.swift', + 'spec/models/user_spec.rb', + 'pkg/thing_test.go', + 'tests/Feature/LoginTest.php', + ]; + + it('entry-point-scoring isTestFile agrees with the shared predicate', () => { + for (const p of paths) expect(isTestFile(p)).toBe(isTestFilePath(p)); + }); + + it('local-backend isTestFilePath agrees with the shared predicate', () => { + for (const p of paths) expect(backendIsTestFilePath(p)).toBe(isTestFilePath(p)); + }); +}); + +describe('shared predicate stays dependency-free', () => { + // Poka-yoke for #2802: `local-backend.ts` imports this module, so anything + // imported here lands in MCP server startup. The duplication this module + // replaced existed precisely because `entry-point-scoring.ts` pulls in the + // language-provider registry. An import added here would silently reintroduce + // that startup cost. + it('declares no imports', () => { + const src = readFileSync( + new URL('../../src/core/ingestion/utils/test-file-path.ts', import.meta.url), + 'utf8', + ); + const imports = src.split('\n').filter((l) => /^\s*(import\b|export\s.*\sfrom\s)/.test(l)); + expect(imports).toEqual([]); + }); +});