mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-03 02:21:44 +00:00
feat(query): exclude test files by default, matching impact precedent
Mirrors the include_tests opt-in convention already used by impact. Today query always includes tests, which can outrank production code when tests mention a concept more often than the implementation itself. Changes: - CLI: new --include-tests flag on gitnexus query (default false). - Backend: filter merged BM25 + semantic results via the existing isTestFilePath() helper before the per-symbol process lookup, so excluded items never incur the STEP_IN_PROCESS round-trip. - MCP schema: surface includeTests on the query tool input schema, same shape as impact. - Tests: two new dispatch cases in calltool-dispatch.test.ts cover default-exclude and includeTests=true round-trip. Behavioral note: default behavior change for query. Existing callers that relied on test files appearing in results need to pass --include-tests or includeTests: true going forward. Matches the precedent set when impact --include-tests shipped. No new dependencies, no schema migration, no infrastructure changes. Rebased onto current main after origin rev moved forward.
This commit is contained in:
parent
d9d6318b64
commit
a61dd2c3ab
5 changed files with 89 additions and 0 deletions
|
|
@ -207,6 +207,7 @@ program
|
|||
.option('-g, --goal <text>', 'What you want to find')
|
||||
.option('-l, --limit <n>', 'Max processes to return (default: 5)')
|
||||
.option('--content', 'Include full symbol source code')
|
||||
.option('--include-tests', 'Include test files in results (default: false, matching impact)')
|
||||
.action(createLbugLazyAction(() => import('./tool.js'), 'queryCommand'));
|
||||
|
||||
program
|
||||
|
|
|
|||
|
|
@ -66,6 +66,7 @@ export async function queryCommand(
|
|||
goal?: string;
|
||||
limit?: string;
|
||||
content?: boolean;
|
||||
includeTests?: boolean;
|
||||
},
|
||||
): Promise<void> {
|
||||
if (!queryText?.trim()) {
|
||||
|
|
@ -80,6 +81,7 @@ export async function queryCommand(
|
|||
goal: options?.goal,
|
||||
limit: options?.limit ? parseInt(options.limit) : undefined,
|
||||
include_content: options?.content ?? false,
|
||||
includeTests: options?.includeTests ?? false,
|
||||
repo: options?.repo,
|
||||
});
|
||||
output(result);
|
||||
|
|
|
|||
|
|
@ -924,6 +924,7 @@ export class LocalBackend {
|
|||
limit?: number;
|
||||
max_symbols?: number;
|
||||
include_content?: boolean;
|
||||
includeTests?: boolean;
|
||||
},
|
||||
): Promise<any> {
|
||||
if (!params.query?.trim()) {
|
||||
|
|
@ -935,6 +936,7 @@ export class LocalBackend {
|
|||
const processLimit = params.limit || 5;
|
||||
const maxSymbolsPerProcess = params.max_symbols || 10;
|
||||
const includeContent = params.include_content ?? false;
|
||||
const includeTests = params.includeTests ?? false;
|
||||
const searchQuery = params.query.trim();
|
||||
|
||||
// Per-phase timing instrumentation (#553). Records wall time for each
|
||||
|
|
@ -989,8 +991,13 @@ export class LocalBackend {
|
|||
}
|
||||
}
|
||||
|
||||
// Exclude test files by default so production paths outrank tests. Mirrors
|
||||
// the include_tests opt-in convention already used by impact. Filter at the
|
||||
// merge step so excluded items never incur the per-symbol STEP_IN_PROCESS
|
||||
// round-trip.
|
||||
const merged = Array.from(scoreMap.entries())
|
||||
.sort((a, b) => b[1].score - a[1].score)
|
||||
.filter(([, item]) => includeTests || !isTestFilePath(item.data.filePath || ''))
|
||||
.slice(0, searchLimit);
|
||||
timer.stop(); // merge
|
||||
|
||||
|
|
|
|||
|
|
@ -121,6 +121,11 @@ SERVICE: optional monorepo path prefix (POSIX-style, case-sensitive segments). W
|
|||
description: 'Include full symbol source code (default: false)',
|
||||
default: false,
|
||||
},
|
||||
includeTests: {
|
||||
type: 'boolean',
|
||||
description: 'Include test files in results (default: false, matching impact)',
|
||||
default: false,
|
||||
},
|
||||
repo: {
|
||||
type: 'string',
|
||||
description:
|
||||
|
|
|
|||
|
|
@ -357,6 +357,80 @@ describe('LocalBackend.callTool', () => {
|
|||
expect(result.error).toContain('query parameter is required');
|
||||
});
|
||||
|
||||
it('query excludes test-file results by default (matches impact precedent)', async () => {
|
||||
const { searchFTSFromLbug } = await import('../../src/core/search/bm25-index.js');
|
||||
(searchFTSFromLbug as any).mockResolvedValue({
|
||||
results: [
|
||||
{ filePath: 'src/auth.ts', score: 1.0 },
|
||||
{ filePath: 'src/auth.test.ts', score: 1.0 },
|
||||
{ filePath: 'src/__tests__/login.ts', score: 1.0 },
|
||||
],
|
||||
ftsAvailable: true,
|
||||
});
|
||||
(executeParameterized as any).mockImplementation(
|
||||
(_repoId: string, cypher: string, params: any) => {
|
||||
if (cypher.includes('n.filePath = $filePath')) {
|
||||
return Promise.resolve([
|
||||
{
|
||||
id: `${params.filePath}#sym`,
|
||||
name: 'foo',
|
||||
type: 'Function',
|
||||
filePath: params.filePath,
|
||||
startLine: 1,
|
||||
endLine: 10,
|
||||
},
|
||||
]);
|
||||
}
|
||||
return Promise.resolve([]);
|
||||
},
|
||||
);
|
||||
|
||||
const result = await backend.callTool('query', { query: 'auth' });
|
||||
const allPaths = [
|
||||
...(result.process_symbols ?? []).map((s: any) => s.filePath),
|
||||
...(result.definitions ?? []).map((d: any) => d.filePath),
|
||||
];
|
||||
expect(allPaths).toContain('src/auth.ts');
|
||||
expect(allPaths.some((p: string) => p.endsWith('.test.ts'))).toBe(false);
|
||||
expect(allPaths.some((p: string) => p.includes('__tests__'))).toBe(false);
|
||||
});
|
||||
|
||||
it('query includes test-file results when includeTests is true', async () => {
|
||||
const { searchFTSFromLbug } = await import('../../src/core/search/bm25-index.js');
|
||||
(searchFTSFromLbug as any).mockResolvedValue({
|
||||
results: [
|
||||
{ filePath: 'src/auth.ts', score: 1.0 },
|
||||
{ filePath: 'src/auth.test.ts', score: 1.0 },
|
||||
],
|
||||
ftsAvailable: true,
|
||||
});
|
||||
(executeParameterized as any).mockImplementation(
|
||||
(_repoId: string, cypher: string, params: any) => {
|
||||
if (cypher.includes('n.filePath = $filePath')) {
|
||||
return Promise.resolve([
|
||||
{
|
||||
id: `${params.filePath}#sym`,
|
||||
name: 'foo',
|
||||
type: 'Function',
|
||||
filePath: params.filePath,
|
||||
startLine: 1,
|
||||
endLine: 10,
|
||||
},
|
||||
]);
|
||||
}
|
||||
return Promise.resolve([]);
|
||||
},
|
||||
);
|
||||
|
||||
const result = await backend.callTool('query', { query: 'auth', includeTests: true });
|
||||
const allPaths = [
|
||||
...(result.process_symbols ?? []).map((s: any) => s.filePath),
|
||||
...(result.definitions ?? []).map((d: any) => d.filePath),
|
||||
];
|
||||
expect(allPaths).toContain('src/auth.ts');
|
||||
expect(allPaths.some((p: string) => p.endsWith('.test.ts'))).toBe(true);
|
||||
});
|
||||
|
||||
it('dispatches cypher tool and blocks write queries', async () => {
|
||||
(executeParameterized as any).mockRejectedValueOnce(new Error('read-only database'));
|
||||
const result = await backend.callTool('cypher', { query: 'CREATE (n:Test)' });
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue