From 2f7a5edae0c64bf8a53289061fffb5c64a8a0c23 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 30 May 2026 06:16:03 +0000 Subject: [PATCH] feat(cli): add --uid/--file/--kind disambiguation flags to impact (#1907) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- gitnexus/src/cli/help-i18n.ts | 3 + gitnexus/src/cli/i18n/en.ts | 5 +- gitnexus/src/cli/i18n/zh-CN.ts | 4 +- gitnexus/src/cli/index.ts | 3 + gitnexus/src/cli/tool.ts | 6 ++ .../local-backend-calltool.test.ts | 85 +++++++++++++++++++ .../unit/cli-impact-disambiguation.test.ts | 67 +++++++++++++++ 7 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 gitnexus/test/unit/cli-impact-disambiguation.test.ts diff --git a/gitnexus/src/cli/help-i18n.ts b/gitnexus/src/cli/help-i18n.ts index 8d620fc6f..5fb42dbe1 100644 --- a/gitnexus/src/cli/help-i18n.ts +++ b/gitnexus/src/cli/help-i18n.ts @@ -101,6 +101,9 @@ const OPTION_DESCRIPTION_KEYS = { 'context|--content': 'help.option.content', 'impact|-d, --direction ': 'help.option.impact.direction', 'impact|-r, --repo ': 'help.option.repo.target', + 'impact|-u, --uid ': 'help.option.context.uid', + 'impact|-f, --file ': 'help.option.context.file', + 'impact|--kind ': 'help.option.impact.kind', 'impact|--depth ': 'help.option.impact.depth', 'impact|--include-tests': 'help.option.impact.includeTests', 'impact|--limit ': 'help.option.impact.limit', diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index 51cbc2816..dc65280ea 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -43,7 +43,8 @@ export const en = { 'tool.noIndexed': 'GitNexus: No indexed repositories found. Run: gitnexus analyze', 'tool.usage.query': 'Usage: gitnexus query ', 'tool.usage.context': 'Usage: gitnexus context [--uid ] [--file ]', - 'tool.usage.impact': 'Usage: gitnexus impact [--direction upstream|downstream]', + 'tool.usage.impact': + 'Usage: gitnexus impact [--uid ] [--file ] [--kind ] [--direction upstream|downstream]', 'tool.usage.cypher': 'Usage: gitnexus cypher ', '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', diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index 2a1374fc6..5b3992d72 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -47,7 +47,8 @@ export const zhCN = { 'tool.noIndexed': 'GitNexus：未找到已索引仓库。请运行：gitnexus analyze', 'tool.usage.query': '用法：gitnexus query <搜索词>', 'tool.usage.context': '用法：gitnexus context <符号名> [--uid ] [--file <路径>]', - 'tool.usage.impact': '用法：gitnexus impact <符号名> [--direction upstream|downstream]', + 'tool.usage.impact': + '用法：gitnexus impact <符号名> [--uid ] [--file <路径>] [--kind <类型>] [--direction upstream|downstream]', 'tool.usage.cypher': '用法：gitnexus 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': '在结果中包含测试文件', diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index 67b34387a..945e057ab 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -223,6 +223,9 @@ program .description('Blast radius analysis: what breaks if you change a symbol') .option('-d, --direction ', 'upstream (dependants) or downstream (dependencies)', 'upstream') .option('-r, --repo ', 'Target repository') + .option('-u, --uid ', 'Direct symbol UID (zero-ambiguity lookup)') + .option('-f, --file ', 'File path to disambiguate common names') + .option('--kind ', 'Kind filter to disambiguate common names (e.g. Function, Class, Method)') .option('--depth ', 'Max relationship depth (default: 3)') .option('--include-tests', 'Include test files in results') .option('--limit ', 'Max symbols per depth level (default: 100)') diff --git a/gitnexus/src/cli/tool.ts b/gitnexus/src/cli/tool.ts index 0e70c3970..a4bc80bea 100644 --- a/gitnexus/src/cli/tool.ts +++ b/gitnexus/src/cli/tool.ts @@ -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, diff --git a/gitnexus/test/integration/local-backend-calltool.test.ts b/gitnexus/test/integration/local-backend-calltool.test.ts index cd4b34d9c..3fd1b0890 100644 --- a/gitnexus/test/integration/local-backend-calltool.test.ts +++ b/gitnexus/test/integration/local-backend-calltool.test.ts @@ -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, diff --git a/gitnexus/test/unit/cli-impact-disambiguation.test.ts b/gitnexus/test/unit/cli-impact-disambiguation.test.ts new file mode 100644 index 000000000..7485153fa --- /dev/null +++ b/gitnexus/test/unit/cli-impact-disambiguation.test.ts @@ -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; + expect(params.target).toBe('AuthService'); + expect(params.target_uid).toBeUndefined(); + expect(params.file_path).toBeUndefined(); + expect(params.kind).toBeUndefined(); + }); +});