feat(impact): add opt-in mode param with hard validation (U1)

Add mode:'callgraph'|'pdg' to the impact MCP tool. callgraph (default)
is byte-identical; pdg routes to a stub (U3/U4). Backend hard-validates
the enum (schema enum is advisory), rejects relationTypes/crossDepth/
minConfidence and @group targets under pdg, and forks the ambiguous
branch so no callgraph BFS runs under pdg. CLI --mode wired.

Refs U1 of docs/plans/2026-06-16-001-feat-pdg-impact-mode-and-accuracy-harness-plan.md
This commit is contained in:
Gergo Magyar 2026-06-16 07:36:01 +00:00
parent ff0124e067
commit 00dc899d34
6 changed files with 398 additions and 0 deletions

View file

@ -338,6 +338,11 @@ program
.command('impact [target]')
.description('Blast radius analysis: what breaks if you change a symbol')
.option('-d, --direction <dir>', 'upstream (dependants) or downstream (dependencies)', 'upstream')
.option(
'--mode <mode>',
'Engine: callgraph (default) or pdg (opt-in, intra-procedural; needs analyze --pdg)',
'callgraph',
)
.option('-r, --repo <name>', 'Target repository')
.option('--branch <name>', 'Scope to a specific branch index (multi-branch repos)')
.option('-u, --uid <uid>', 'Direct symbol UID (zero-ambiguity lookup)')

View file

@ -124,6 +124,7 @@ export async function impactCommand(
target?: string,
options?: {
direction?: string;
mode?: string;
repo?: string;
branch?: string;
uid?: string;
@ -168,6 +169,9 @@ export async function impactCommand(
file_path: options?.file,
kind: options?.kind,
direction: options?.direction || 'upstream',
// Forward the engine selector; backend validates the enum (callgraph/pdg)
// and treats the default 'callgraph' identically to an omitted mode.
mode: options?.mode,
maxDepth: options?.depth ? parseInt(options.depth, 10) : undefined,
includeTests: options?.includeTests ?? false,
repo: options?.repo,

View file

@ -96,6 +96,32 @@ function resolveAliasString(canonical: unknown, legacy: unknown): string | undef
}
return undefined;
}
/** The two impact engines (KTD1). `'callgraph'` is the default/established path. */
export type ImpactMode = 'callgraph' | 'pdg';
/**
* Validate the `impact` `mode` param (KTD5 — backend hard-gate).
*
* The MCP JSON-schema `enum` is advisory only (server.ts forwards args
* unvalidated and `callTool` is reachable directly), so this backend check is
* the real boundary — mirroring `_pdgQueryImpl`'s `mode` enum validation. A
* typo'd mode silently running callgraph is exactly the silent fallback this
* forbids (it would make the accuracy harness compare callgraph-vs-callgraph
* and report perfect parity).
*
* Absent / `undefined` / `'callgraph'` all resolve to `'callgraph'` (the
* unchanged default path). `'pdg'` is valid. Anything else — `'PDG'`, `'pgd'`,
* `''`, or a non-string (`0`, `null`, …) — returns a structured `{ error }`,
* never a callgraph result.
*/
function validateImpactMode(rawMode: unknown): { mode: ImpactMode } | { error: string } {
if (rawMode === undefined || rawMode === 'callgraph') return { mode: 'callgraph' };
if (rawMode === 'pdg') return { mode: 'pdg' };
return {
error: `Invalid "mode": expected "callgraph" or "pdg", got ${JSON.stringify(rawMode)}.`,
};
}
// AI context generation is CLI-only (gitnexus analyze)
// import { generateAIContextFiles } from '../../cli/ai-context.js';
@ -425,7 +451,15 @@ interface ImpactParams {
file_path?: string;
kind?: string;
direction: 'upstream' | 'downstream';
/**
* Blast-radius engine (KTD1/KTD5). Absent / `undefined` / `'callgraph'` →
* the unchanged inter-procedural symbol→symbol BFS. `'pdg'` → the opt-in,
* intra-procedural Program Dependence Graph traversal (`_runImpactPDG`).
* Validated in `_impactImpl`; any other value is a hard `{ error }`.
*/
mode?: ImpactMode;
maxDepth?: number;
crossDepth?: number;
relationTypes?: string[];
includeTests?: boolean;
minConfidence?: number;
@ -4238,6 +4272,56 @@ export class LocalBackend {
await this.ensureInitialized(repo);
const { target, direction } = params;
// ── Dispatch order (KTD5) ──────────────────────────────────────────
// (1) Validate `mode`. Absent/'callgraph' → unchanged path; 'pdg' → the
// intra-procedural PDG engine (stubbed in U1); anything else → hard error.
// This MUST come before resolveSymbolCandidates so the ambiguous branch can
// fork on the validated mode and never run the callgraph fan-out under pdg.
const modeResult = validateImpactMode(params.mode);
if ('error' in modeResult) {
return {
error: modeResult.error,
target: { name: target },
direction,
impactedCount: 0,
risk: 'UNKNOWN',
};
}
const mode = modeResult.mode;
if (mode === 'pdg') {
// KTD12 — param-compatibility hard rejections (decided as errors, NOT
// silent ignores and NOT an `ignoredParams` echo). Each names a symbol-
// graph / cross-repo concept the PDG engine cannot honor:
// relationTypes → names symbol edges (PDG walks BasicBlock edges).
// crossDepth → cross-repo hops (PDG is single-repo intra-procedural).
// minConfidence → CDG/RD edges may carry no confidence → would drop all.
// A loud failure beats a quietly-wrong result. (@group targets are
// rejected at the group-forward boundary in callToolAtGroupRepo before
// they ever reach here; see KTD12.)
const incompatible: string[] = [];
if (params.relationTypes !== undefined) incompatible.push('relationTypes');
if (params.crossDepth !== undefined) incompatible.push('crossDepth');
if (params.minConfidence !== undefined) incompatible.push('minConfidence');
if (incompatible.length > 0) {
return {
error:
`Parameter(s) ${incompatible.join(', ')} are not supported with mode:'pdg' ` +
`(intra-procedural, single-repo, dependence-edge based). Remove them or use mode:'callgraph'.`,
target: { name: target },
direction,
impactedCount: 0,
risk: 'UNKNOWN',
};
}
}
// (2) PDG-layer presence probe — STUB for U1; U2 fills in the four-state
// degradation contract (no-layer / partial / unknown / ready) here, before
// any DB scan, so a missing `--pdg` layer returns a guidance note rather
// than a confusing empty traversal. Intentionally a no-op for now.
const maxDepth = params.maxDepth || 3;
// Map legacy relation type names before filtering (backward compat for OVERRIDES → METHOD_OVERRIDES)
const mappedRelTypes = params.relationTypes?.flatMap((t: string) =>
@ -4300,6 +4384,44 @@ export class LocalBackend {
}
if (outcome.kind === 'ambiguous') {
// KTD5 ambiguous trap — under mode:'pdg' we MUST NOT fall into the
// callgraph fan-out below: it runs `_runImpactBFS` per candidate, which
// would silently execute the call-graph engine under a `pdg` call (the
// exact silent fallback KTD5 forbids). For U1 the pdg ambiguous path
// returns the candidate list WITHOUT any callgraph probe; the full pdg
// ambiguous handling (per-candidate PDG summaries / ranking) lands in U4.
if (mode === 'pdg') {
const AMBIGUOUS_MAX_CANDIDATES = 6;
const truncated = outcome.candidates.length > AMBIGUOUS_MAX_CANDIDATES;
const shown = outcome.candidates.slice(0, AMBIGUOUS_MAX_CANDIDATES);
return {
status: 'ambiguous',
mode,
message:
`Found ${outcome.candidates.length} symbols matching '${target}'` +
(truncated ? ` (showing ${shown.length} of ${outcome.candidates.length})` : '') +
`. Disambiguate with target_uid (or file_path/kind) for a single ` +
`authoritative PDG result.`,
target: { name: target },
direction,
totalCandidates: outcome.candidates.length,
// No single resolved symbol → impactedCount stays 0 / risk UNKNOWN
// (UNKNOWN must never read as "safe to refactor"). No callgraph
// fan-out runs, so there is no per-candidate blast radius here yet.
impactedCount: 0,
risk: 'UNKNOWN',
...(truncated && { candidatesTruncated: true }),
candidates: shown.map((c) => ({
uid: c.id,
name: c.name,
kind: c.type,
filePath: c.filePath,
line: c.startLine,
score: Number(c.score.toFixed(2)),
})),
};
}
// #2129 — a bare name that collides with several symbols must NOT report a
// bare `impactedCount: 0`. The real blast radius lives under whichever
// candidate the caller meant; a flat zero here is precisely the silent
@ -4425,6 +4547,27 @@ export class LocalBackend {
};
const symType = outcome.resolvedLabel || outcome.symbol.type || '';
// (4) single → route the resolved symbol to the engine selected by `mode`.
// The PDG engine is a stub in U1 (full traversal lands in U3/U4); crucially
// it does NOT touch `_runImpactBFS`, so a `pdg` call never runs callgraph.
if (mode === 'pdg') {
return this._runImpactPDG({
repo,
sym,
symType,
direction,
maxDepth,
limit: Number.isFinite(params.limit) ? params.limit : 100,
offset: Number.isFinite(params.offset) ? params.offset : 0,
summaryOnly: params.summaryOnly,
// KTD2 extraction-seam discipline: hand the engine its DB dependency
// explicitly rather than `this.`-binding it, so the traversal (U3/U4)
// can later move to a standalone `pdg-impact.ts` as a move, not a
// rewrite. The U3 block-anchor / projection resolvers join here.
executeParameterized,
});
}
const effectiveRelationTypes =
(symType === 'Class' || symType === 'Interface') &&
!hasExplicitRelationTypes &&
@ -4443,6 +4586,44 @@ export class LocalBackend {
});
}
/**
* PDG-backed blast radius (`mode:'pdg'`) — STUB (U1).
*
* The real engine (U3/U4) resolves the target symbol to its BasicBlocks,
* runs a direction-aware bounded BFS over the persisted `CDG` +
* `REACHING_DEF` edges (KTD4 truth table, KTD11 query constraints), then
* projects the reachable blocks back to owning symbols and assembles a
* consumer-safe result (KTD8 parity matrix). U1 ships only the param /
* validation surface, so this returns a structured "pending" payload.
*
* KTD2 extraction-seam discipline: written as a method taking its DB
* dependency as an explicit parameter (not reaching back through `this.` for
* the query path) so the traversal can later be lifted to a standalone
* `gitnexus/src/mcp/local/pdg-impact.ts` engine as a *move*, not a rewrite.
* The stub does not yet consume `executeParameterized` — the U3 anchor /
* BFS / projection code joins here.
*/
private async _runImpactPDG(deps: {
repo: RepoHandle;
sym: { id: string; name: string; filePath: string };
symType: string;
direction: 'upstream' | 'downstream';
maxDepth: number;
limit: number;
offset: number;
summaryOnly?: boolean;
executeParameterized: typeof executeParameterized;
}): Promise<any> {
return {
error: 'pdg mode not yet implemented (U3/U4)',
mode: 'pdg',
target: { name: deps.sym.name, id: deps.sym.id, filePath: deps.sym.filePath },
direction: deps.direction,
impactedCount: 0,
risk: 'UNKNOWN',
};
}
/**
* #1858 — epistemic lower-bound detection.
*
@ -5348,6 +5529,21 @@ export class LocalBackend {
const svc = this.getGroupService();
if (method === 'impact') {
// KTD5/KTD12 — validate `mode` at the group-forward boundary too (the
// JSON-schema enum is advisory). An invalid mode errors; `mode:'pdg'` is
// rejected for @group targets because PDG impact is single-repo and
// intra-procedural — there is no cross-repo dependence graph to walk.
// Rejecting here (before groupImpact) is the KTD12 @group hard error.
const groupModeResult = validateImpactMode(params.mode);
if ('error' in groupModeResult) return { error: groupModeResult.error };
if (groupModeResult.mode === 'pdg') {
return {
error:
"mode:'pdg' is not supported for @group targets — PDG impact is " +
'single-repo and intra-procedural. Run pdg impact against an ' +
'individual indexed repository instead.',
};
}
const impactArgs: Record<string, unknown> = {
name: groupName,
repo: resolved.repoPath,

View file

@ -409,6 +409,8 @@ Each edit is tagged with confidence:
description: `Analyze the blast radius of changing a code symbol.
Returns affected symbols grouped by depth, plus risk assessment, affected execution flows, and affected modules.
MODE (opt-in): "callgraph" (default) walks symbol→symbol edges (CALLS/IMPORTS/EXTENDS/IMPLEMENTS) — inter-procedural, the established behavior. "pdg" computes the blast radius from the persisted Program Dependence Graph (control + data dependence) — finer-grained WITHIN a function but intra-procedural, and requires an index built with \`gitnexus analyze --pdg\`. The two modes answer the same question with different engines; pdg is incompatible with relationTypes/crossDepth/minConfidence and with @group targets (each rejected).
WHEN TO USE: Before making code changes — especially refactoring, renaming, or modifying shared code. Shows what would break.
AFTER THIS: Review d=1 items (WILL BREAK). Use context() on high-risk symbols.
@ -450,6 +452,13 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep
type: 'string',
description: 'upstream (what depends on this) or downstream (what this depends on)',
},
mode: {
type: 'string',
enum: ['callgraph', 'pdg'],
default: 'callgraph',
description:
"Blast-radius engine. 'callgraph' (default) = inter-procedural symbol→symbol traversal (current behavior). 'pdg' = opt-in, intra-procedural Program Dependence Graph traversal (control + data dependence); requires an index built with `gitnexus analyze --pdg`. The pdg mode is incompatible with relationTypes/crossDepth/minConfidence and with @group targets — each is rejected, not silently ignored.",
},
file_path: {
type: 'string',
description: 'File path hint to disambiguate common names',

View file

@ -1385,6 +1385,172 @@ describe('LocalBackend.callTool', () => {
});
});
// ─── impact mode param (KTD1/KTD5/KTD12 — U1) ───────────────────────
//
// The MCP JSON-schema enum is advisory only (server forwards args
// unvalidated, callTool is reachable directly), so the backend `mode`
// validation is load-bearing. These tests pin: callgraph is the unchanged
// default, pdg routes to the stub and NEVER the callgraph BFS, invalid modes
// hard-error, and the KTD12 incompatible params / @group targets are rejected.
describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => {
let backend: LocalBackend;
// Resolve the target to a single Function so impact reaches the single-branch
// dispatch (callgraph BFS or the pdg stub). The callgraph BFS then issues
// executeQuery for its frontier; the pdg stub does not.
function resolveSingleTarget() {
(executeParameterized as any).mockResolvedValue([
{ id: 'func:main', name: 'main', type: 'Function', filePath: 'src/index.ts' },
]);
(executeQuery as any).mockResolvedValue([]);
}
beforeEach(async () => {
vi.clearAllMocks();
platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true);
backend = new LocalBackend();
setupSingleRepo();
await backend.init();
});
it('mode absent → callgraph result (target populated, no mode-error, BFS runs)', async () => {
resolveSingleTarget();
const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS');
const result = await backend.callTool('impact', { target: 'main', direction: 'upstream' });
// A clean callgraph result carries no mode/stub error and runs the BFS.
expect(result.error ?? '').not.toMatch(/Invalid "mode"/);
expect(result.error ?? '').not.toMatch(/not yet implemented/);
expect(result.target).toBeDefined();
expect(bfsSpy).toHaveBeenCalledTimes(1);
});
it("mode:'callgraph' and mode:undefined are byte-identical to absent (regression guard)", async () => {
resolveSingleTarget();
const absent = await backend.callTool('impact', { target: 'main', direction: 'upstream' });
const callgraph = await backend.callTool('impact', {
target: 'main',
direction: 'upstream',
mode: 'callgraph',
});
const undef = await backend.callTool('impact', {
target: 'main',
direction: 'upstream',
mode: undefined,
});
expect(callgraph).toEqual(absent);
expect(undef).toEqual(absent);
});
it("mode:'pdg' routes to the PDG stub and NEVER runs the callgraph BFS (KTD5)", async () => {
resolveSingleTarget();
const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS');
const result = await backend.callTool('impact', {
target: 'main',
direction: 'upstream',
mode: 'pdg',
});
// Stub payload — pending until U3/U4.
expect(result.error).toMatch(/not yet implemented/);
expect(result.mode).toBe('pdg');
// The callgraph engine must never be invoked under a pdg call.
expect(bfsSpy).not.toHaveBeenCalled();
});
it.each([['PDG'], ['pgd'], [''], [0], [null]])(
'invalid mode %j → structured {error}, never a callgraph result (KTD5 anti-silent-fallback)',
async (bad) => {
resolveSingleTarget();
const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS');
const result = await backend.callTool('impact', {
target: 'main',
direction: 'upstream',
mode: bad as any,
});
expect(result.error).toMatch(/Invalid "mode"/);
expect(result.risk).toBe('UNKNOWN');
// A typo'd mode must NEVER quietly run callgraph.
expect(bfsSpy).not.toHaveBeenCalled();
},
);
it.each([
['relationTypes', { relationTypes: ['CALLS'] }],
['crossDepth', { crossDepth: 2 }],
['minConfidence', { minConfidence: 0.5 }],
])("mode:'pdg' + %s → hard {error} (KTD12, not a silent ignore)", async (_label, extra) => {
resolveSingleTarget();
const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS');
const result = await backend.callTool('impact', {
target: 'main',
direction: 'upstream',
mode: 'pdg',
...extra,
});
expect(result.error).toMatch(/not supported with mode:'pdg'/);
expect(result.error).toContain(_label);
// Hard error — never silently ignored, never the callgraph fan-out.
expect(bfsSpy).not.toHaveBeenCalled();
});
it("ambiguous target under mode:'pdg' never invokes the callgraph fan-out (KTD5 ambiguous trap)", async () => {
// Two same-name Functions → resolver returns ambiguous.
(executeParameterized as any).mockResolvedValue([
{ id: 'func:login:1', name: 'login', type: 'Function', filePath: 'src/auth.ts', startLine: 5 },
{
id: 'func:login:2',
name: 'login',
type: 'Function',
filePath: 'src/admin/login.ts',
startLine: 8,
},
]);
const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS');
const result = await backend.callTool('impact', {
target: 'login',
direction: 'upstream',
mode: 'pdg',
});
expect(result.status).toBe('ambiguous');
expect(result.mode).toBe('pdg');
expect(result.candidates).toHaveLength(2);
expect(result.impactedCount).toBe(0);
expect(result.risk).toBe('UNKNOWN');
// The callgraph per-candidate probe fan-out MUST NOT run under pdg.
expect(bfsSpy).not.toHaveBeenCalled();
// No per-candidate blast radius is computed yet (U4), so the candidate
// entries carry no impactedCount field from a callgraph probe.
for (const c of result.candidates) {
expect(c.impactedCount).toBeUndefined();
}
});
it("@group target with mode:'pdg' is rejected (KTD12 — PDG is single-repo)", async () => {
resolveAtMemberMock.mockResolvedValue({ ok: true, repoPath: '/tmp/test-project' });
const result = await backend.callTool('impact', {
target: 'main',
direction: 'upstream',
mode: 'pdg',
repo: '@grp',
});
expect(result.error).toMatch(/not supported for @group targets/);
});
it("@group target with mode:'callgraph' still forwards to group impact (unchanged)", async () => {
resolveAtMemberMock.mockResolvedValue({ ok: true, repoPath: '/tmp/test-project' });
// groupImpact is reached only if the mode gate passes; we don't assert its
// payload (group infra is stubbed), only that no mode-error short-circuited.
const result = await backend.callTool('impact', {
target: 'main',
direction: 'upstream',
mode: 'callgraph',
repo: '@grp',
});
expect(result?.error ?? '').not.toMatch(/not supported for @group targets/);
expect(result?.error ?? '').not.toMatch(/Invalid "mode"/);
});
});
// ─── Repo resolution ────────────────────────────────────────────────
describe('LocalBackend.resolveRepo', () => {

View file

@ -276,6 +276,24 @@ describe('GITNEXUS_TOOLS', () => {
expect(relProp.items).toEqual({ type: 'string' });
});
it('impact advertises a mode param (callgraph default; pdg opt-in) — not a new tool (KTD1)', () => {
// KTD1: pdg impact ships as a PARAM on the existing tool, so the tool count
// must NOT change (asserted at 17 above) and `impact` must expose `mode`.
const impactTool = GITNEXUS_TOOLS.find((t) => t.name === 'impact')!;
const modeProp = impactTool.inputSchema.properties.mode;
expect(modeProp).toBeDefined();
expect(modeProp.type).toBe('string');
expect(modeProp.enum).toEqual(['callgraph', 'pdg']);
expect(modeProp.default).toBe('callgraph');
// The description must teach the opt-in / intra-procedural / --pdg contract.
expect(modeProp.description).toContain('pdg');
expect(modeProp.description).toContain('--pdg');
expect(modeProp.description.toLowerCase()).toContain('intra-procedural');
// The tool-level description must mention the mode so an LLM discovers it.
expect(impactTool.description.toLowerCase()).toContain('mode');
expect(impactTool.description).toContain('pdg');
});
it('route_map description defers to api_impact for pre-change analysis', () => {
const routeMapTool = GITNEXUS_TOOLS.find((t) => t.name === 'route_map')!;
expect(routeMapTool.description).toContain('api_impact');