mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-03 02:21:44 +00:00
feat(cli): add --uid/--file/--kind disambiguation flags to impact (#1907)
When `impact` reports an ambiguous target it tells the user to disambiguate, but the CLI had no way to do so — only the MCP impact tool accepted target_uid/file_path/kind (the CLI `context` command had --uid/--file, `impact` had neither). Register -u/--uid, -f/--file and --kind on the impact command and forward them to callTool('impact', ...) as target_uid/file_path/kind, matching the context CLI convention and the MCP impact surface. Help text and the usage hint are localized in en + zh-CN.
Tests: a unit test pins the CLI option -> tool-param mapping; integration tests cover the ambiguous report, target_uid/file_path resolution, and a cross-label (Function+Tool) collision resolving without a binder crash.
Note on the reported binder error ("Cannot find property id for n"): it is environmental — a stale on-disk catalog after an in-place upgrade without a full reindex — and not reproducible on a fresh index. Label-scoping the resolver's MATCH was investigated and is infeasible here (LadybugDB caps multi-label node patterns at 11 of 29 labels, and the startLine/endLine projection only exists on a subset of labels), so the unlabeled match, which is correct via lenient binding, is left unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bef3da59a7
commit
2f7a5edae0
7 changed files with 171 additions and 2 deletions
|
|
@ -101,6 +101,9 @@ const OPTION_DESCRIPTION_KEYS = {
|
|||
'context|--content': 'help.option.content',
|
||||
'impact|-d, --direction <dir>': 'help.option.impact.direction',
|
||||
'impact|-r, --repo <name>': 'help.option.repo.target',
|
||||
'impact|-u, --uid <uid>': 'help.option.context.uid',
|
||||
'impact|-f, --file <path>': 'help.option.context.file',
|
||||
'impact|--kind <kind>': 'help.option.impact.kind',
|
||||
'impact|--depth <n>': 'help.option.impact.depth',
|
||||
'impact|--include-tests': 'help.option.impact.includeTests',
|
||||
'impact|--limit <n>': 'help.option.impact.limit',
|
||||
|
|
|
|||
|
|
@ -43,7 +43,8 @@ export const en = {
|
|||
'tool.noIndexed': 'GitNexus: No indexed repositories found. Run: gitnexus analyze',
|
||||
'tool.usage.query': 'Usage: gitnexus query <search_query>',
|
||||
'tool.usage.context': 'Usage: gitnexus context <symbol_name> [--uid <uid>] [--file <path>]',
|
||||
'tool.usage.impact': 'Usage: gitnexus impact <symbol_name> [--direction upstream|downstream]',
|
||||
'tool.usage.impact':
|
||||
'Usage: gitnexus impact <symbol_name> [--uid <uid>] [--file <path>] [--kind <kind>] [--direction upstream|downstream]',
|
||||
'tool.usage.cypher': 'Usage: gitnexus cypher <cypher_query>',
|
||||
'tool.detectChanges.noChanges': 'No changes detected.',
|
||||
'tool.detectChanges.changesSummary': 'Changes: {{files}} files, {{symbols}} symbols',
|
||||
|
|
@ -213,6 +214,8 @@ export const en = {
|
|||
'help.option.repo.target': 'Target repository',
|
||||
'help.option.context.uid': 'Direct symbol UID (zero-ambiguity lookup)',
|
||||
'help.option.context.file': 'File path to disambiguate common names',
|
||||
'help.option.impact.kind':
|
||||
'Kind filter to disambiguate common names (e.g. Function, Class, Method)',
|
||||
'help.option.impact.direction': 'upstream (dependants) or downstream (dependencies)',
|
||||
'help.option.impact.depth': 'Max relationship depth (default: 3)',
|
||||
'help.option.impact.includeTests': 'Include test files in results',
|
||||
|
|
|
|||
|
|
@ -47,7 +47,8 @@ export const zhCN = {
|
|||
'tool.noIndexed': 'GitNexus:未找到已索引仓库。请运行:gitnexus analyze',
|
||||
'tool.usage.query': '用法:gitnexus query <搜索词>',
|
||||
'tool.usage.context': '用法:gitnexus context <符号名> [--uid <uid>] [--file <路径>]',
|
||||
'tool.usage.impact': '用法:gitnexus impact <符号名> [--direction upstream|downstream]',
|
||||
'tool.usage.impact':
|
||||
'用法:gitnexus impact <符号名> [--uid <uid>] [--file <路径>] [--kind <类型>] [--direction upstream|downstream]',
|
||||
'tool.usage.cypher': '用法:gitnexus cypher <Cypher 查询>',
|
||||
'tool.detectChanges.noChanges': '未检测到变更。',
|
||||
'tool.detectChanges.changesSummary': '变更:{{files}} 个文件,{{symbols}} 个符号',
|
||||
|
|
@ -199,6 +200,7 @@ export const zhCN = {
|
|||
'help.option.repo.target': '目标仓库',
|
||||
'help.option.context.uid': '直接符号 UID(零歧义查找)',
|
||||
'help.option.context.file': '用于消除常见名称歧义的文件路径',
|
||||
'help.option.impact.kind': '用于消除常见名称歧义的类型过滤(如 Function、Class、Method)',
|
||||
'help.option.impact.direction': 'upstream(依赖它的项)或 downstream(它依赖的项)',
|
||||
'help.option.impact.depth': '最大关系遍历深度(默认:3)',
|
||||
'help.option.impact.includeTests': '在结果中包含测试文件',
|
||||
|
|
|
|||
|
|
@ -223,6 +223,9 @@ program
|
|||
.description('Blast radius analysis: what breaks if you change a symbol')
|
||||
.option('-d, --direction <dir>', 'upstream (dependants) or downstream (dependencies)', 'upstream')
|
||||
.option('-r, --repo <name>', 'Target repository')
|
||||
.option('-u, --uid <uid>', 'Direct symbol UID (zero-ambiguity lookup)')
|
||||
.option('-f, --file <path>', 'File path to disambiguate common names')
|
||||
.option('--kind <kind>', 'Kind filter to disambiguate common names (e.g. Function, Class, Method)')
|
||||
.option('--depth <n>', 'Max relationship depth (default: 3)')
|
||||
.option('--include-tests', 'Include test files in results')
|
||||
.option('--limit <n>', 'Max symbols per depth level (default: 100)')
|
||||
|
|
|
|||
|
|
@ -115,6 +115,9 @@ export async function impactCommand(
|
|||
options?: {
|
||||
direction?: string;
|
||||
repo?: string;
|
||||
uid?: string;
|
||||
file?: string;
|
||||
kind?: string;
|
||||
depth?: string;
|
||||
includeTests?: boolean;
|
||||
limit?: string;
|
||||
|
|
@ -135,6 +138,9 @@ export async function impactCommand(
|
|||
const parsedOffset = Number.isFinite(rawOffset) ? rawOffset : undefined;
|
||||
const result = await backend.callTool('impact', {
|
||||
target,
|
||||
target_uid: options?.uid,
|
||||
file_path: options?.file,
|
||||
kind: options?.kind,
|
||||
direction: options?.direction || 'upstream',
|
||||
maxDepth: options?.depth ? parseInt(options.depth, 10) : undefined,
|
||||
includeTests: options?.includeTests ?? false,
|
||||
|
|
|
|||
|
|
@ -302,6 +302,91 @@ withTestLbugDB(
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── impact disambiguation + label-scoped resolution (#1907) ─────────
|
||||
// Covers the disambiguation surface the CLI --uid/--file/--kind flags
|
||||
// wire through to, and guards the label-scoped resolver against the
|
||||
// binder failure that motivated the fix.
|
||||
describe('impact disambiguation (#1907)', () => {
|
||||
let backend: LocalBackend;
|
||||
|
||||
beforeAll(async () => {
|
||||
const ext = handle as typeof handle & { _backend?: LocalBackend };
|
||||
if (!ext._backend) {
|
||||
throw new Error(
|
||||
'LocalBackend not initialized — afterSetup did not attach _backend to handle',
|
||||
);
|
||||
}
|
||||
backend = ext._backend;
|
||||
});
|
||||
|
||||
it('reports an ambiguous target with disambiguation guidance', async () => {
|
||||
// Two Methods named 'authenticate' (AuthService + BaseService).
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'authenticate',
|
||||
direction: 'upstream',
|
||||
});
|
||||
expect(result).not.toHaveProperty('error');
|
||||
expect(result.status).toBe('ambiguous');
|
||||
expect(result.message).toMatch(/disambiguate/i);
|
||||
const uids = (result.candidates ?? []).map((c: any) => c.uid);
|
||||
expect(uids).toContain('method:AuthService.authenticate');
|
||||
expect(uids).toContain('method:BaseService.authenticate');
|
||||
});
|
||||
|
||||
it('resolves the ambiguous target via target_uid (the --uid flag path)', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'authenticate',
|
||||
target_uid: 'method:BaseService.authenticate',
|
||||
direction: 'upstream',
|
||||
});
|
||||
expect(result).not.toHaveProperty('error');
|
||||
expect(result.status).not.toBe('ambiguous');
|
||||
// target_uid selects the exact symbol, bypassing the name ranker.
|
||||
expect(result.target?.id).toBe('method:BaseService.authenticate');
|
||||
expect(result.target?.filePath).toBe('src/base.ts');
|
||||
});
|
||||
|
||||
it('resolves the ambiguous target via file_path (the --file flag path)', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'authenticate',
|
||||
file_path: 'src/base.ts',
|
||||
direction: 'upstream',
|
||||
});
|
||||
expect(result).not.toHaveProperty('error');
|
||||
expect(result.status).not.toBe('ambiguous');
|
||||
expect(result.target?.id).toBe('method:BaseService.authenticate');
|
||||
});
|
||||
|
||||
it('does not crash when a name collides across symbol and non-symbol labels', async () => {
|
||||
// 'alpha' exists as both a Function and a Tool sharing src/tools.py.
|
||||
// The Tool node table has no startLine/endLine columns, so the
|
||||
// resolver's `RETURN n.startLine` projection only binds because the
|
||||
// candidate set also contains a label that *does* have those columns
|
||||
// (lenient multi-table binding). This guards that the disambiguation
|
||||
// path keeps tolerating non-symbol node types — and would catch a
|
||||
// future naive label-scoping that reintroduces the #1907 binder error
|
||||
// ("Cannot find property … for n") by matching property-poor tables
|
||||
// in isolation.
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'alpha',
|
||||
direction: 'upstream',
|
||||
});
|
||||
expect(result).not.toHaveProperty('error');
|
||||
expect(result.status).toBe('ambiguous');
|
||||
const uids = (result.candidates ?? []).map((c: any) => c.uid);
|
||||
expect(uids).toContain('func:alpha');
|
||||
expect(uids).toContain('Tool:alpha');
|
||||
});
|
||||
|
||||
it('context resolves the same cross-label collision without crashing', async () => {
|
||||
const result = await backend.callTool('context', { name: 'alpha' });
|
||||
expect(result).not.toHaveProperty('error');
|
||||
expect(result.status).toBe('ambiguous');
|
||||
const uids = (result.candidates ?? []).map((c: any) => c.uid);
|
||||
expect(uids).toContain('func:alpha');
|
||||
});
|
||||
});
|
||||
},
|
||||
{
|
||||
seed: LOCAL_BACKEND_SEED_DATA,
|
||||
|
|
|
|||
67
gitnexus/test/unit/cli-impact-disambiguation.test.ts
Normal file
67
gitnexus/test/unit/cli-impact-disambiguation.test.ts
Normal file
|
|
@ -0,0 +1,67 @@
|
|||
/**
|
||||
* Unit Tests: CLI `impact` disambiguation flag wiring (#1907)
|
||||
*
|
||||
* The CLI `impact` command gained --uid / --file / --kind so that, when impact
|
||||
* reports an `ambiguous` target, users can follow the "disambiguate" guidance
|
||||
* straight from the terminal (previously only the MCP tool accepted these).
|
||||
* These tests pin that impactCommand forwards the flags to
|
||||
* callTool('impact', …) under the backend's parameter names
|
||||
* (target_uid / file_path / kind) — the same names the MCP impact tool uses.
|
||||
*
|
||||
* The LocalBackend is fully mocked: this isolates the CLI option → tool param
|
||||
* mapping from any graph/DB behaviour.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { callTool, init } = vi.hoisted(() => ({
|
||||
callTool: vi.fn(),
|
||||
init: vi.fn().mockResolvedValue(true),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/mcp/local/local-backend.js', () => ({
|
||||
LocalBackend: class {
|
||||
init = init;
|
||||
callTool = callTool;
|
||||
},
|
||||
}));
|
||||
|
||||
import { impactCommand } from '../../src/cli/tool.js';
|
||||
|
||||
describe('CLI impact disambiguation flags (#1907)', () => {
|
||||
beforeEach(() => {
|
||||
callTool.mockReset();
|
||||
callTool.mockResolvedValue({ status: 'found', impactedCount: 0 });
|
||||
});
|
||||
|
||||
it('forwards --uid/--file/--kind as target_uid/file_path/kind', async () => {
|
||||
await impactCommand('get_embeddings', {
|
||||
direction: 'upstream',
|
||||
uid: 'Function:isma/scripts/ingest_md_file.py:get_embeddings',
|
||||
file: 'isma/scripts/ingest_md_file.py',
|
||||
kind: 'Function',
|
||||
});
|
||||
|
||||
expect(callTool).toHaveBeenCalledTimes(1);
|
||||
expect(callTool).toHaveBeenCalledWith(
|
||||
'impact',
|
||||
expect.objectContaining({
|
||||
target: 'get_embeddings',
|
||||
target_uid: 'Function:isma/scripts/ingest_md_file.py:get_embeddings',
|
||||
file_path: 'isma/scripts/ingest_md_file.py',
|
||||
kind: 'Function',
|
||||
direction: 'upstream',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves disambiguation params undefined when no flags are supplied', async () => {
|
||||
await impactCommand('AuthService', { direction: 'upstream' });
|
||||
|
||||
expect(callTool).toHaveBeenCalledTimes(1);
|
||||
const params = callTool.mock.calls[0][1] as Record<string, unknown>;
|
||||
expect(params.target).toBe('AuthService');
|
||||
expect(params.target_uid).toBeUndefined();
|
||||
expect(params.file_path).toBeUndefined();
|
||||
expect(params.kind).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue