From 99ae83103c532eee6bdade491c142caac09bccdd Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 18 Jun 2026 05:21:15 +0000 Subject: [PATCH] fix(impact): harden PDG impact mode --- gitnexus/bench/impact-pdg/README.md | 22 + gitnexus/src/cli/ai-context.ts | 2 +- gitnexus/src/cli/eval-server.ts | 40 +- gitnexus/src/mcp/local/local-backend.ts | 1118 ++--------------- gitnexus/src/mcp/local/pdg-impact.ts | 1080 ++++++++++++++++ gitnexus/src/mcp/tools.ts | 12 +- .../impact-pdg-degradation.test.ts | 66 +- .../test/integration/impact-pdg-e2e.test.ts | 272 ++++ .../test/integration/impact-pdg-shape.test.ts | 44 +- .../integration/impact-pdg-traversal.test.ts | 33 +- gitnexus/test/unit/ai-context.test.ts | 2 + gitnexus/test/unit/calltool-dispatch.test.ts | 52 +- .../test/unit/cli-impact-pdg-format.test.ts | 52 +- gitnexus/test/unit/pdg-impact-engine.test.ts | 76 ++ gitnexus/test/unit/tools.test.ts | 7 +- 15 files changed, 1809 insertions(+), 1069 deletions(-) create mode 100644 gitnexus/src/mcp/local/pdg-impact.ts create mode 100644 gitnexus/test/integration/impact-pdg-e2e.test.ts create mode 100644 gitnexus/test/unit/pdg-impact-engine.test.ts diff --git a/gitnexus/bench/impact-pdg/README.md b/gitnexus/bench/impact-pdg/README.md index 142ab6c65..d50d94785 100644 --- a/gitnexus/bench/impact-pdg/README.md +++ b/gitnexus/bench/impact-pdg/README.md @@ -40,6 +40,28 @@ neither strictly dominates*. > measures; the earlier "PDG is empty / callgraph wins" verdict was an artifact of > the whole-symbol seed, now replaced. +## Runtime result contract + +`impact({mode:'pdg', line:N})` success results carry a target envelope +(`id`, `name`, `type`, `filePath`), `risk: 'UNKNOWN'`, `affectedStatements`, +`affectedStatementCount`, and the same empty-safe parity fields used by callgraph +consumers (`byDepth`, `byDepthCounts`, `summary`, `affected_processes`, +`affected_modules`). The risk stays UNKNOWN because a statement slice is +intra-procedural; it is precise for the function body but not a whole-program +safety verdict. + +Degraded PDG results are explicit, not empty successes. `no-layer`, +`sub-layer-missing`, and `unknown` responses keep `mode:'pdg'`, target metadata +when the target resolves, `risk:'UNKNOWN'`, a remediation note, and empty parity +fields. Truncation is also explicit: when both depth and per-step limit bounds +fire, `truncatedByReasons` reports both causes. + +Deferred architecture remains out of scope for this harness: explicit +`Function|Method -> BasicBlock` containment (`CONTAINS_BLOCK`), inter-procedural +summary edges / realizable call-return paths, mutation-derived AIS, and a hybrid +callgraph+PDG impact mode are follow-up features, not assumptions of the current +statement-level benchmark. + ## The corpus Each case is a tiny self-contained TypeScript source repo plus a diff --git a/gitnexus/src/cli/ai-context.ts b/gitnexus/src/cli/ai-context.ts index b1a66ffdf..3060fc381 100644 --- a/gitnexus/src/cli/ai-context.ts +++ b/gitnexus/src/cli/ai-context.ts @@ -201,7 +201,7 @@ This project is indexed by GitNexus as **${projectName}**${noStats ? '' : ` (${s - **MUST run impact analysis before editing any symbol.** Before modifying a function, class, or method, run \`impact({target: "symbolName", direction: "upstream"})\` and report the blast radius (direct callers, affected processes, risk level) to the user.${ hasPdg - ? ` For finer, intra-procedural precision within a function, add \`mode: "pdg"\` — it traces control/data dependence (CDG + REACHING_DEF) instead of call-graph reachability, but does NOT model cross-function impact (\`--pdg\` layer).` + ? ` For finer, intra-procedural precision within a function, add \`mode: "pdg"\` with \`line: \` — it returns statement-level \`affectedStatements\` over CDG + REACHING_DEF, but does NOT model cross-function impact; no-layer/degraded PDG results are UNKNOWN-risk notes (\`--pdg\` layer).` : '' } - **MUST run \`detect_changes()\` before committing** to verify your changes only affect expected symbols and execution flows. For regression review, compare against the default branch: \`detect_changes({scope: "compare", base_ref: ${JSON.stringify(markdownSafeBranch(defaultBranch))}})\`. diff --git a/gitnexus/src/cli/eval-server.ts b/gitnexus/src/cli/eval-server.ts index b4c755776..a722ce44e 100644 --- a/gitnexus/src/cli/eval-server.ts +++ b/gitnexus/src/cli/eval-server.ts @@ -178,6 +178,18 @@ export function formatContextResult(result: any): string { return lines.join('\n').trim(); } +function formatTruncationSuffix(result: { + truncatedBy?: unknown; + truncatedByReasons?: unknown; +}): string { + const label = Array.isArray(result.truncatedByReasons) + ? result.truncatedByReasons.join(', ') + : typeof result.truncatedBy === 'string' + ? result.truncatedBy + : ''; + return label ? ` (by ${label})` : ''; +} + export function formatImpactResult(result: any): string { if (result.error) { const suggestion = result.suggestion ? `\nSuggestion: ${result.suggestion}` : ''; @@ -194,6 +206,28 @@ export function formatImpactResult(result: any): string { // mirroring formatContextResult, so the real impact under whichever symbol the // caller meant is visible on the text surface, not just in the JSON. if (result.status === 'ambiguous') { + if (result.mode === 'pdg') { + const shown = result.candidates?.length ?? 0; + const totalCandidates = result.totalCandidates ?? shown; + const countPhrase = + totalCandidates > shown + ? `${totalCandidates} symbols (showing ${shown})` + : `${totalCandidates} symbols`; + const lines = [ + `${target?.name || '?'}: AMBIGUOUS — ${countPhrase} share this name. ` + + `PDG impact was not computed until the target is disambiguated. ` + + `Use --uid, file_path, or kind for one authoritative PDG result.`, + ]; + if (result.message) lines.push(String(result.message)); + for (const c of result.candidates || []) { + const score = typeof c.score === 'number' ? ` score ${c.score}` : ''; + lines.push( + ` ${c.kind} ${c.name} → ${c.filePath}:${c.line || '?'}${score} (uid: ${c.uid})`, + ); + } + return lines.join('\n'); + } + // #2129 review F11 — report the FULL match count (`totalCandidates`), not the // truncated `candidates[]` length; note when the candidate list is capped. const shown = result.candidates?.length ?? 0; @@ -307,7 +341,7 @@ export function formatImpactResult(result: any): string { // Truncation honesty — the slice may be a lower bound (depth or per-step // LIMIT bound). Surface it the same way the symbol render does. if (result.truncated) { - const by = result.truncatedBy ? ` (by ${result.truncatedBy})` : ''; + const by = formatTruncationSuffix(result); slLines.push( `⚠️ Truncated${by} — the dependence slice was bounded; deeper PDG-dependent statements may exist.`, ); @@ -383,11 +417,11 @@ export function formatImpactResult(result: any): string { if (result.unresolvedBlockCount > 0) { pdgLines.push( `⚠️ ${result.unresolvedBlockCount} dependence block(s) map to no owning ` + - `Function/Method (top-level statement / closure) — surfaced under their file.`, + `Function/Method/Constructor (top-level statement / closure) — surfaced under their file.`, ); } if (result.truncated) { - const by = result.truncatedBy ? ` (by ${result.truncatedBy})` : ''; + const by = formatTruncationSuffix(result); pdgLines.push( `⚠️ Truncated${by} — the dependence traversal was bounded; deeper PDG impacts may exist.`, ); diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 94598383b..0a52d5e79 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -65,6 +65,18 @@ import { import { findImportCycles } from '../../core/graph/import-cycles.js'; import { decodeTaintPath } from '../../core/ingestion/taint/path-codec.js'; import { EXTENSIONS } from '../../core/ingestion/import-resolvers/utils.js'; +import { + fnLineOf, + isPdgDegradedLayerStatus, + makePdgImpactErrorResult, + makePdgLayerDegradedResult, + pdgLayerStatus, + pdgStampForMode, + runImpactPDG, + validateImpactMode, + type ImpactMode, + type PdgImpactResult, +} from './pdg-impact.js'; /** Real source-file extensions (`.ts`, `.py`, …) from the resolver's list, * excluding the empty entry and the `/index.*` forms — used to decide whether @@ -97,617 +109,6 @@ function resolveAliasString(canonical: unknown, legacy: unknown): string | undef return undefined; } -/** - * Parse the `` segment out of a `BasicBlock` id (1-based function start - * line). The id template is - * `BasicBlock::::` - * and `` may itself contain `':'` (a Windows drive letter), so the - * segments are taken from the RIGHT: `` is last, `` second-last, - * `` third-last. Extracted from the `_pdgQueryImpl` closure (#2086) into - * a shared module-scope helper so the PDG impact traversal (U3/U4) reuses the - * exact same parse — the `pdg_query` read path is byte-identical to before. - */ -function fnLineOf(id: string): number { - const parts = id.split(':'); - return Number(parts[parts.length - 3]); -} - -/** - * Parse the `` segment out of a `BasicBlock` id, the COUNTERPART to - * `fnLineOf`. The id template is `BasicBlock::::`, - * so the file path is everything BETWEEN the `BasicBlock:` prefix and the last - * THREE colon-segments (`::`). `` may itself - * contain `':'` (a Windows drive letter), so we strip from both ends rather than - * split-and-pick. Returns `''` for an unparseable id (treated as unresolved). - */ -function fnFileOf(id: string): string { - const parts = id.split(':'); - // Need: `BasicBlock` + filePath(≥1) + fnLine + fnCol + blockIdx ⇒ ≥5 segments. - if (parts.length < 5 || parts[0] !== 'BasicBlock') return ''; - // Drop the leading `BasicBlock` token and the trailing fnLine/fnCol/blockIdx, - // rejoin the middle on ':' to restore a path that itself contained colons. - return parts.slice(1, parts.length - 3).join(':'); -} - -/** A reachable dependence block resolved to its source statement. */ -export interface PdgStatement { - /** 1-based source line where the statement's block starts. */ - line: number; - /** Repo-relative file path (parsed from the block id). */ - filePath: string; - /** The statement's source text (BasicBlock.text), trimmed. */ - text: string; -} - -/** - * Resolve a set of reachable BasicBlock ids to their source statements - * (line + text), deduped by `(filePath, line)` and sorted by line. This is the - * useful output of a statement-anchored PDG slice — the dependent statements the - * change reaches. A query error propagates (no `.catch` swallow) so a DB failure - * is never silently reported as "no affected statements". - */ -async function pdgStatementsForBlocks( - lbugPath: string, - blockIds: string[], - exec: typeof executeParameterized, -): Promise { - if (blockIds.length === 0) return []; - const rows = await exec( - lbugPath, - `MATCH (b:BasicBlock) WHERE b.id IN $ids - RETURN b.id AS id, b.startLine AS line, b.text AS text`, - { ids: blockIds }, - ); - const byKey = new Map(); - for (const r of rows as any[]) { - const id = String(r.id ?? r[0] ?? ''); - const line = Number(r.line ?? r[1] ?? 0); - if (!id || !Number.isFinite(line) || line <= 0) continue; - const filePath = fnFileOf(id); - const text = String(r.text ?? r[2] ?? '').trim(); - const key = `${filePath}:${line}`; - if (!byKey.has(key)) byKey.set(key, { line, filePath, text }); - } - return [...byKey.values()].sort((a, b) => - a.filePath === b.filePath ? a.line - b.line : a.filePath < b.filePath ? -1 : 1, - ); -} - -// ── Block → owning-symbol projection types (U4) ────────────────────────────── - -/** - * One owning-symbol candidate for a reachable BasicBlock, OR an explicit - * `unresolved` marker for a block that maps to no `Function`/`Method` symbol - * (top-level/free-statement block, or a nested lambda whose start line ≠ any - * symbol `startLine`). A null `id` is the shadow-path marker — the block is - * surfaced under its file, never silently dropped (R9: a silent drop is a hidden - * recall loss). - */ -export interface OwningSymbol { - /** Symbol UID, or `null` for the `unresolved` shadow-path entry. */ - id: string | null; - name: string; - /** `'Function' | 'Method' | …`, or `'unresolved'` for the shadow path. */ - type: string; - filePath: string; - /** Symbol `startLine` (0-based), present only for a resolved symbol. */ - startLine?: number; - /** - * True when this block's `(filePath, startLine)` query matched >1 symbol — - * same-line, different-name functions that the schema cannot disambiguate - * (no `startColumn` column; Feasibility Finding 1). ALL colliding symbols are - * reported (never a silent pick), each carrying this flag. - */ - ambiguous?: boolean; -} - -/** - * Net-new block → owning-symbol resolver (U4) — the REVERSE of - * `resolveBlockAnchor` (which goes symbol→blocks). No precedent exists: - * `_pdgQueryImpl` only ever extracts a raw `functionLine`, never an owning - * symbol. Built as a module-scope function taking injected deps (KTD2 - * extraction discipline) so it can move to a future `pdg-impact.ts` engine as a - * MOVE, not a rewrite. - * - * For each reachable block id `BasicBlock::::`: - * - `fnLineOf` → 1-based function start line; `fnFileOf` → file path. - * - Query `Function`/`Method` `WHERE filePath = $f AND startLine = (fnLine-1)` - * — block `fnLine` is 1-based, symbol `startLine` is 0-based, so subtract one - * (the `[symStart+1]` convention from `resolveBlockAnchor`, applied in - * reverse; NOT re-derived). - * - * Two non-happy paths, BOTH surfaced (never silent): - * - **>1 match** (same-line different-name functions): `fnCol` rides the block - * id but the schema has NO `startColumn` column and the symbol id encodes only - * the name, so a `(filePath, startLine)` join cannot disambiguate. Report ALL - * colliding symbols, each `ambiguous: true` (R4 / Feasibility Finding 1). - * - **0 matches** (top-level/free-statement block, or a lambda whose start line - * ≠ a symbol `startLine`): one `unresolved` entry (`id: null`) under the - * block's file (R9 shadow path). - * - * Distinct `(filePath, fnLine)` pairs are queried once each (a block and its - * siblings in the same function share a pair), so the cost is O(distinct - * functions), not O(blocks). - */ -async function projectBlocksToSymbols(deps: { - lbugPath: string; - blockIds: string[]; - executeParameterized: typeof executeParameterized; -}): Promise<{ symbols: OwningSymbol[]; unresolvedCount: number; ambiguousCount: number }> { - const { lbugPath, blockIds, executeParameterized: exec } = deps; - - // Group blocks by their (filePath, fnLine) owning-function key so each owning - // function is resolved with a single query regardless of block count. - const byFnKey = new Map(); - for (const id of blockIds) { - const filePath = fnFileOf(id); - const fnLine = fnLineOf(id); // 1-based - if (!filePath || !Number.isFinite(fnLine)) { - // Unparseable block id — record an unresolved key so it is reported, never - // dropped. Use the raw id as the key so duplicates collapse. - byFnKey.set(`#bad#${id}`, { filePath: filePath || id, symStart: NaN }); - continue; - } - const symStart = fnLine - 1; // 0-based symbol startLine (reverse [symStart+1]) - byFnKey.set(`${filePath}#${symStart}`, { filePath, symStart }); - } - - const resolved: OwningSymbol[] = []; - let unresolvedCount = 0; - let ambiguousCount = 0; - - await Promise.all( - Array.from(byFnKey.values()).map(async ({ filePath, symStart }) => { - if (!Number.isFinite(symStart)) { - // Unparseable id — shadow-path unresolved under (best-effort) file. - resolved.push({ id: null, name: '(unresolved)', type: 'unresolved', filePath }); - unresolvedCount += 1; - return; - } - // `Function`/`Method` carry name+filePath+startLine; the schema has NO - // `startColumn`, so the join is on (filePath, startLine) only. `filePath` - // and `symStart` are BOUND as params (KTD11 — never interpolated). A - // UNION ALL across the two explicit labels is used rather than a - // `(s:Function OR s:Method)` disjunction (unsupported in the LadybugDB - // Cypher subset — the established cross-label pattern, see - // `enrichCandidateLabels`). `LIMIT` is a small validated int literal. - // FIX 6: do NOT swallow a query failure as `[]`. A DB error (lock / - // corruption / missing path) must NOT masquerade as a genuine no-owning- - // symbol result — that would silently inflate `unresolvedCount` and hide - // the failure. Letting it reject propagates through `Promise.all` → - // `projectBlocksToSymbols` → `_runImpactPDG` → `_impactImpl` up to the - // `impact()` structured-error catch, where it surfaces as a real error - // with a recovery suggestion (rather than a clean-looking partial radius). - const rows = await exec( - lbugPath, - `MATCH (s:\`Function\`) - WHERE s.filePath = $filePath AND s.startLine = $symStart - RETURN s.id AS id, s.name AS name, 'Function' AS label, s.startLine AS startLine - UNION ALL - MATCH (s:\`Method\`) - WHERE s.filePath = $filePath AND s.startLine = $symStart - RETURN s.id AS id, s.name AS name, 'Method' AS label, s.startLine AS startLine - LIMIT 8`, - { filePath, symStart }, - ); - - if (rows.length === 0) { - // No owning symbol — top-level/free-statement block or a lambda whose - // start line ≠ a symbol startLine. Shadow path: report under its file. - resolved.push({ - id: null, - name: '(unresolved)', - type: 'unresolved', - filePath, - startLine: symStart, - }); - unresolvedCount += 1; - return; - } - - // >1 ⇒ ambiguous-projection (same-line, different-name functions). Report - // ALL colliding symbols, NEVER silently pick one (R4 / Feasibility 1). - const isAmbiguous = rows.length > 1; - for (const r of rows) { - resolved.push({ - id: String((r as any).id ?? (r as any)[0] ?? ''), - name: String((r as any).name ?? (r as any)[1] ?? ''), - type: String((r as any).label ?? (r as any)[2] ?? 'Function'), - filePath, - startLine: Number((r as any).startLine ?? (r as any)[3] ?? symStart), - ...(isAmbiguous ? { ambiguous: true as const } : {}), - }); - } - if (isAmbiguous) ambiguousCount += 1; - }), - ); - - // Deterministic order: by filePath, then startLine, then id (unresolved last - // within a file). Order-independence matters for the parity/fingerprint - // contract (KTD8 standing interchangeability) and for stable consumer output. - resolved.sort((a, b) => { - if (a.filePath !== b.filePath) return a.filePath < b.filePath ? -1 : 1; - const al = a.startLine ?? Number.MAX_SAFE_INTEGER; - const bl = b.startLine ?? Number.MAX_SAFE_INTEGER; - if (al !== bl) return al - bl; - const ai = a.id ?? '￿'; - const bi = b.id ?? '￿'; - return ai < bi ? -1 : ai > bi ? 1 : 0; - }); - - return { symbols: resolved, unresolvedCount, ambiguousCount }; -} - -/** - * The KTD8 parity fields a PDG impact result carries even when it short-circuits - * to an empty radius (degraded layer / no PDG body / no dependence reachability). - * - * A programmatic consumer iterating `byDepth`, reading `byDepthCounts[1]`, or - * coalescing `affected_processes`/`affected_modules` must find a well-formed - * (empty) shape on EVERY early return, not `undefined` (which would render as - * "isolated"/"no data" instead of "inconclusive"). The CLI branches on - * `pdgLayer` first so it is safe regardless, but the JSON contract must be - * uniform across all three early returns — this single source guarantees that. - */ -function emptyPdgParityFields(): { - byDepth: Record; - byDepthCounts: Record; - summary: { direct: number; processes_affected: number; modules_affected: number }; - affected_processes: unknown[]; - affected_modules: unknown[]; -} { - return { - byDepth: {}, - byDepthCounts: { 1: 0 }, - summary: { direct: 0, processes_affected: 0, modules_affected: 0 }, - affected_processes: [], - affected_modules: [], - }; -} - -/** - * Assemble the consumer-safe PDG impact result (U4 / KTD8 parity matrix). - * - * Takes the U3 traversal output (reachable block set + truncation signalling) - * plus the U4 block→symbol projection, and shapes a result STRUCTURALLY - * substitutable for the call-graph `_runImpactBFS` result so every consumer - * (CLI `formatImpactResult`, group `collectImpactSymbolUids`/`mergeRisk`, - * `impactByUid`) renders it without misrendering. This is a STANDING - * interchangeability contract, not a one-time check. - * - * Field-by-field vs the call-graph result (KTD8): - * - `target.id/name/type/filePath` — identical shape (`collectImpactSymbolUids` - * keys on `target.id`/`target.filePath`). - * - `byDepth` — same `{ [depth]: item[] }` map shape, but COLLAPSED to a single - * bucket (`1`): intra-procedural dependence has no meaningful inter-symbol hop - * count (block-hops are NOT call-hops). Items carry `{ id, name, type, - * filePath, … }` exactly like the call-graph items so `collectImpactSymbolUids` - * collects their UIDs. `unresolved` shadow-path entries keep `id: null` (they - * are surfaced, never dropped — but collect as no UID). - * - `byDepthCounts` — `{ 1: }`, same shape. - * - `affected_processes` / `affected_modules` — empty `[]` (no - * STEP_IN_PROCESS/module edges originate from BasicBlocks; consumers coalesce - * `[]` safely). - * - `epistemic` — a PDG-specific marker (`'pdg-intra-procedural'`), NOT the - * callgraph DI/dynamic-dispatch `'lower-bound'` copy. `note` carries the - * PDG framing so the CLI prints PDG text, not callgraph boundary text. - * - `risk` — the existing `'UNKNOWN'` sentinel (NOT a new label). `mergeRisk` - * already coalesces `'UNKNOWN'` correctly (never a confident `LOW`). - * - `impactedCount` — count of DISTINCT owning SYMBOLS (resolved UIDs), the - * meaningful unit for the impact question ("which symbols are affected"). - * `blockCount` is retained separately as the raw reachable-block count. - */ -function assemblePdgImpactResult(input: { - target: { id: string; name: string; type: string; filePath: string }; - direction: 'upstream' | 'downstream'; - reachableBlocks: string[]; - /** Reachable blocks resolved to source statements (the useful slice output). */ - affectedStatements?: PdgStatement[]; - /** The 1-based source line the slice was seeded on (statement mode only). */ - criterionLine?: number; - projection: { symbols: OwningSymbol[]; unresolvedCount: number; ambiguousCount: number }; - depthReached: number; - truncated: boolean; - truncatedBy?: 'depth' | 'limit'; -}): Record { - const { target, direction, reachableBlocks, projection } = input; - const { symbols, unresolvedCount, ambiguousCount } = projection; - const affectedStatements = input.affectedStatements ?? []; - const statementMode = typeof input.criterionLine === 'number'; - - // Items for the single collapsed bucket. Shaped like the call-graph byDepth - // items (`{ depth, id, name, type, filePath, processes }`) so consumers that - // iterate byDepth read the same fields. `unresolved` entries keep `id: null` - // (surfaced under their file; `collectImpactSymbolUids` skips a null id, which - // is correct — there is no symbol UID to attribute). - const items = symbols.map((s) => ({ - depth: 1, - id: s.id, - name: s.name, - type: s.type, - filePath: s.filePath, - ...(s.startLine !== undefined ? { startLine: s.startLine } : {}), - ...(s.ambiguous ? { ambiguous: true } : {}), - ...(s.id === null ? { unresolved: true } : {}), - processes: [] as unknown[], - })); - - // impactedCount = distinct owning SYMBOLS (resolved UIDs). Unresolved shadow - // entries are surfaced in byDepth but do NOT inflate the symbol count. - const resolvedUids = new Set(symbols.filter((s) => s.id !== null).map((s) => s.id as string)); - const impactedCount = resolvedUids.size; - - const byDepth: Record = items.length > 0 ? { 1: items } : {}; - const byDepthCounts: Record = { 1: items.length }; - - const noteParts: string[] = statementMode - ? [ - `mode:'pdg' — intra-procedural slice from line ${input.criterionLine} of ` + - `'${target.name}'. ${affectedStatements.length} ` + - `${affectedStatements.length === 1 ? 'statement is' : 'statements are'} ${direction}-` + - `dependent on it (over CDG + REACHING_DEF). Cross-function (inter-procedural) impact ` + - `is NOT modeled in this mode — use mode:'callgraph' for the call-graph blast radius.`, - ] - : [ - `mode:'pdg' — intra-procedural Program Dependence Graph. ${impactedCount} owning ` + - `${impactedCount === 1 ? 'symbol' : 'symbols'} reached via ${reachableBlocks.length} ` + - `dependence ${reachableBlocks.length === 1 ? 'block' : 'blocks'} ` + - `(${direction} over CDG + REACHING_DEF). Cross-function (inter-procedural) impact is ` + - `NOT modeled in this mode — use mode:'callgraph' for the call-graph blast radius.`, - ]; - if (ambiguousCount > 0) { - noteParts.push( - `${ambiguousCount} owning-symbol ${ambiguousCount === 1 ? 'projection is' : 'projections are'} ` + - `ambiguous: same-line functions cannot be disambiguated by start line alone (no startColumn ` + - `in the schema), so ALL colliding symbols are reported — none is silently picked.`, - ); - } - if (unresolvedCount > 0) { - noteParts.push( - `${unresolvedCount} reachable ${unresolvedCount === 1 ? 'block maps' : 'blocks map'} to no ` + - `owning Function/Method (top-level statement or a lambda whose start line is not a symbol ` + - `start) — surfaced under their file as 'unresolved', never dropped.`, - ); - } - - return { - mode: 'pdg', - target, - direction, - impactedCount, - // KTD8: reuse the existing UNKNOWN sentinel — never a confident LOW (which - // would read as "safe to refactor"; #2129/#1858 false-safe lineage). PDG - // mode is intra-procedural, so its count is a per-function lower bound on the - // true blast radius and risk is genuinely UNKNOWN at the program level. - risk: 'UNKNOWN', - // PDG-specific epistemic marker — NOT the callgraph 'lower-bound'/DI copy. - epistemic: 'pdg-intra-procedural', - note: noteParts.join(' '), - // Statement-level slice: the dependent source statements (line + text) the - // change reaches. This is the primary useful output of statement mode; the - // accuracy harness scores against these lines. - ...(statementMode ? { criterionLine: input.criterionLine } : {}), - affectedStatements, - affectedStatementCount: affectedStatements.length, - // Raw block-level detail retained alongside the symbol projection (U3 tests - // and the accuracy harness read these). - reachableBlocks, - blockCount: reachableBlocks.length, - depthReached: input.depthReached, - unresolvedBlockCount: unresolvedCount, - ambiguousProjectionCount: ambiguousCount, - ...(input.truncated ? { truncated: true } : {}), - ...(input.truncatedBy ? { truncatedBy: input.truncatedBy } : {}), - summary: { - direct: impactedCount, - processes_affected: 0, - modules_affected: 0, - }, - byDepthCounts, - affected_processes: [] as unknown[], - affected_modules: [] as unknown[], - byDepth, - }; -} - -/** 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)}.`, - }; -} - -/** The two independently-stamped PDG sub-layers (KTD7). */ -export type PdgSubLayer = 'CDG' | 'REACHING_DEF'; - -/** - * Four-state PDG-layer presence/degradation status (KTD7). - * - * - `'no-layer'` — `meta.pdg` is absent: this repo was never analyzed - * with `--pdg` (definitive; established with NO DB scan). - * - `'sub-layer-missing'`— exactly one of the two independently-stamped caps - * (`maxCdgEdgesPerFunction` / `maxReachingDefEdgesPerFunction`) - * is present. `impact`'s PDG mode needs BOTH, so a - * partial layer must not be reported as complete; the - * missing one is named in `missingSubLayer`. - * - `'ready'` — both caps present: the layer is fully stamped. - * - `'unknown'` — meta is unreadable (e.g. a seeded test DB with no - * `meta.json`). One bounded `LIMIT 1` probe distinguishes - * a genuinely edge-free index from a missing one; either - * way the conclusion is inconclusive (a missing layer is - * indistinguishable from an all-linear one — #2188). - */ -export interface PdgLayerStatus { - state: 'no-layer' | 'sub-layer-missing' | 'ready' | 'unknown'; - /** Set only for `'sub-layer-missing'` — the cap that was NOT stamped. */ - missingSubLayer?: PdgSubLayer; - /** Human-readable guidance for the degraded states (absent for `'ready'`). */ - note?: string; -} - -/** - * Per-cap presence read from `meta.pdg`, plus whether meta was readable at all. - * - * `metaReadable` is the seam between the `'unknown'` state (meta unreadable — - * fall through to a DB probe) and the meta-stamped states. When `metaReadable` - * is true but `meta.pdg` was absent, both `cdg`/`rd` are `false`. - */ -interface PdgMetaCaps { - metaReadable: boolean; - /** `maxCdgEdgesPerFunction !== undefined` (only meaningful when metaReadable). */ - cdg: boolean; - /** `maxReachingDefEdgesPerFunction !== undefined` (only meaningful when metaReadable). */ - rd: boolean; -} - -/** - * Read the two PDG sub-layer caps from the on-disk `meta.json` stamp — the - * single shared meta-probe both `_pdgQueryImpl` (one cap) and the PDG impact - * mode (both caps) key on. Never scans the DB. An unreadable / missing meta - * yields `metaReadable: false` (the `'unknown'` seam); a readable meta with no - * `pdg` stamp yields `metaReadable: true` with both caps `false` (no-layer). - */ -async function readPdgMetaCaps( - lbugPath: string, - loadMetaFn: typeof loadMeta, -): Promise { - try { - const meta = await loadMetaFn(path.dirname(lbugPath)); - if (!meta) return { metaReadable: false, cdg: false, rd: false }; - return { - metaReadable: true, - cdg: meta.pdg?.maxCdgEdgesPerFunction !== undefined, - rd: meta.pdg?.maxReachingDefEdgesPerFunction !== undefined, - }; - } catch { - // Meta unreadable — the caller decides from the DB (the `'unknown'` state). - return { metaReadable: false, cdg: false, rd: false }; - } -} - -/** - * Project the both-caps PDG meta read down to the single mode-relevant cap that - * `_pdgQueryImpl` keys on (`controls` → CDG, `flows` → REACHING_DEF), preserving - * its established tri-state `boolean | undefined` contract byte-for-byte - * (Feasibility Issue 4): - * - `false` — meta readable and the relevant cap absent → definitive - * no-layer (short-circuits before any DB scan). - * - `true` — meta readable and the relevant cap present → proceed. - * - `undefined` — meta unreadable → defer to the post-anchored-query probe. - * - * `_pdgQueryImpl` needs only ONE cap, so it collapses the both-caps read here - * rather than consuming `pdgLayerStatus` directly (whose `'unknown'` state does - * an upfront global probe — wrong timing/order for the anchored-query path). - */ -async function pdgStampForMode( - lbugPath: string, - mode: 'controls' | 'flows', - loadMetaFn: typeof loadMeta = loadMeta, -): Promise { - const caps = await readPdgMetaCaps(lbugPath, loadMetaFn); - if (!caps.metaReadable) return undefined; - return mode === 'controls' ? caps.cdg : caps.rd; -} - -/** - * PDG-layer presence/degradation check for the `impact` PDG mode (KTD7). - * - * Returns the four distinct states WITHOUT scanning the DB except for the single - * bounded `LIMIT 1` probe the `'unknown'` (meta-unreadable) case requires. The - * caller (`_impactImpl` PDG branch, and the accuracy harness) surfaces a - * distinct signal per state so a missing `--pdg` layer / partial layer is never - * silently misread as a confident empty blast radius. Impact needs BOTH the CDG - * and the REACHING_DEF sub-layer, so a partial stamp degrades, not proceeds. - * - * Deps are injected (KTD2 extraction discipline) so this can move to a future - * `pdg-impact.ts` engine as a move, not a rewrite. - */ -async function pdgLayerStatus(deps: { - lbugPath: string; - executeParameterized: typeof executeParameterized; - loadMetaFn?: typeof loadMeta; -}): Promise { - const loadMetaFn = deps.loadMetaFn ?? loadMeta; - const caps = await readPdgMetaCaps(deps.lbugPath, loadMetaFn); - - if (caps.metaReadable) { - // Meta is readable — the stamp is authoritative, no DB scan needed. - if (caps.cdg && caps.rd) return { state: 'ready' }; - if (caps.cdg !== caps.rd) { - // Exactly one sub-layer stamped (XOR) — partial layer; impact needs both. - const missingSubLayer: PdgSubLayer = caps.cdg ? 'REACHING_DEF' : 'CDG'; - return { - state: 'sub-layer-missing', - missingSubLayer, - note: - `PDG layer is incomplete — the ${missingSubLayer} sub-layer is missing ` + - `(impact's PDG mode needs both CDG and REACHING_DEF). ` + - `Re-run gitnexus analyze --pdg to record it.`, - }; - } - // Neither cap stamped (meta.pdg absent, or present with no caps) → the layer - // was never recorded. Definitive, no DB scan. - return { - state: 'no-layer', - note: 'no PDG layer — run gitnexus analyze --pdg to record CDG + REACHING_DEF edges for this repo', - }; - } - - // Meta unreadable (e.g. a seeded test DB): one bounded probe confirms the - // layer status is genuinely undeterminable from the DB. A missing layer is - // indistinguishable from an all-linear (edge-free) one (#2188), so whether the - // probe finds a row or not the state stays `'unknown'` (never the definitive - // no-layer wording). The probe is bounded (`LIMIT 1`) and anchored on the - // BasicBlock→BasicBlock partition (the `(:BasicBlock)…(:BasicBlock)` label pair - // restricts it to the sparse pdg-edge partition, never a global rel scan — the - // established `_explainImpl` anchoring pattern), and it is wrapped so a db-lock - // / missing-path throw degrades to the same `'unknown'` signal rather than - // propagating and losing it. - // - // The probe result is NOT discarded: a visible CDG/REACHING_DEF edge (with - // meta unreadable) is a weak-but-real "edges are present, but completeness is - // unprovable" signal, distinct from "no edges visible at all". Both stay - // `'unknown'` (inconclusive), but the note distinguishes them so the operator - // gets the more useful hint. - let edgesVisible = false; - try { - const rows = await deps.executeParameterized( - deps.lbugPath, - `MATCH (:BasicBlock)-[r:CodeRelation]->(:BasicBlock) WHERE r.type IN ['CDG', 'REACHING_DEF'] RETURN r.type AS type LIMIT 1`, - {}, - ); - edgesVisible = Array.isArray(rows) && rows.length > 0; - } catch { - // db-lock / missing-path / corrupt probe — fall through as not-visible, but - // keep the `'unknown'` signal (a probe failure must not lose it). - edgesVisible = false; - } - return { - state: 'unknown', - note: edgesVisible - ? 'PDG layer status unknown — CDG/REACHING_DEF edges ARE visible but meta is unreadable, so the layer cannot be confirmed complete (a partial layer looks the same); was this repo fully indexed with gitnexus analyze --pdg?' - : 'PDG layer status unknown — no CDG/REACHING_DEF edges visible and meta is unreadable; was this repo indexed with gitnexus analyze --pdg?', - }; -} // AI context generation is CLI-only (gitnexus analyze) // import { generateAIContextFiles } from '../../cli/ai-context.js'; @@ -3537,81 +2938,6 @@ export class LocalBackend { }; } - /** - * Build the SAME BasicBlock seed anchor (`anchorClause` + `queryParams`) as - * `resolveBlockAnchor`'s symbol branch, but from an ALREADY-RESOLVED symbol — - * WITHOUT re-running `resolveSymbolCandidates`. - * - * Why this exists (correctness keystone): `_impactImpl` already resolves the - * target to a confident single symbol honoring the caller's - * `target_uid`/`file_path`/`kind` hints. Re-resolving by the bare `sym.name` - * inside `_runImpactPDG` would (a) RE-AMBIGUATE a globally-ambiguous name the - * caller had disambiguated (returning the "ambiguous" early payload instead of - * the PDG result), or (b) anchor the seed on a DIFFERENT same-name symbol in - * another file → a wrong-symbol blast radius. Anchoring directly from the - * resolved `{ filePath, startLine, endLine }` preserves the disambiguation. - * - * The window is byte-identical to `resolveBlockAnchor`'s symbol branch: BOTH - * span bounds are shifted `+1` (1-based BasicBlock `startLine` vs the 0-based - * symbol span — the lower `+1` excludes a neighbor's block on the line above, - * the upper `+1` keeps a guard/def/use on the final line). A symbol with no - * usable span degrades to the same file-level id-prefix filter. This is the - * resolved-symbol counterpart, NOT a second window convention. - */ - private blockAnchorForResolvedSymbol(sym: { - filePath: string; - startLine?: number; - endLine?: number; - }): { anchorClause: string; queryParams: Record } { - const idPrefix = `BasicBlock:${sym.filePath}:`; - if ( - typeof sym.startLine === 'number' && - typeof sym.endLine === 'number' && - sym.endLine >= sym.startLine - ) { - return { - anchorClause: - 'a.id STARTS WITH $idPrefix AND a.startLine >= $symStart AND a.startLine <= $symEnd', - queryParams: { idPrefix, symStart: sym.startLine + 1, symEnd: sym.endLine + 1 }, - }; - } - return { anchorClause: 'a.id STARTS WITH $idPrefix', queryParams: { idPrefix } }; - } - - /** - * Build a STATEMENT seed anchor: the BasicBlock(s) starting at a specific - * 1-based source `line` WITHIN the resolved symbol. This is what makes - * `mode:'pdg'` useful — seeding the dependence slice on a single statement - * (the thing being changed) rather than the whole symbol. A whole-symbol seed - * captures every intra-procedural block, so the reachable-minus-seed set is - * empty (all intra reach is within the seed); a statement seed leaves the - * other dependent statements reachable. `BasicBlock.startLine` is 1-based and - * matches the source line, so no `+1` offset applies here (unlike the symbol - * span, where the 0-based symbol bounds are shifted). Bounded to the symbol's - * own span when known, so a line shared with a sibling symbol can't leak. - */ - private blockAnchorForStatement( - sym: { filePath: string; startLine?: number; endLine?: number }, - line: number, - ): { anchorClause: string; queryParams: Record } { - const idPrefix = `BasicBlock:${sym.filePath}:`; - if ( - typeof sym.startLine === 'number' && - typeof sym.endLine === 'number' && - sym.endLine >= sym.startLine - ) { - return { - anchorClause: - 'a.id STARTS WITH $idPrefix AND a.startLine = $line AND a.startLine >= $symStart AND a.startLine <= $symEnd', - queryParams: { idPrefix, line, symStart: sym.startLine + 1, symEnd: sym.endLine + 1 }, - }; - } - return { - anchorClause: 'a.id STARTS WITH $idPrefix AND a.startLine = $line', - queryParams: { idPrefix, line }, - }; - } - /** * Explain tool (#2083 M3 U6) — persisted taint-finding explanation. * WAL-aware wrapper mirroring `context`. @@ -4916,14 +4242,28 @@ export class LocalBackend { return await this._impactImpl(repo, params); } catch (err: any) { // Return structured error instead of crashing (#321) + const message = + (err instanceof Error ? err.message : String(err)) || 'Impact analysis failed'; + const suggestion = 'The graph query failed — try gitnexus context as a fallback'; + const recoverySuggestion = isWalCorruptionError(err) ? WAL_RECOVERY_SUGGESTION : undefined; + if (params.mode === 'pdg') { + return makePdgImpactErrorResult({ + mode: 'pdg', + error: message, + target: { name: params.target }, + direction: params.direction, + suggestion, + recoverySuggestion, + }); + } return { - error: (err instanceof Error ? err.message : String(err)) || 'Impact analysis failed', + error: message, target: { name: params.target }, direction: params.direction, impactedCount: 0, risk: 'UNKNOWN', - suggestion: 'The graph query failed — try gitnexus context as a fallback', - ...(isWalCorruptionError(err) ? { recoverySuggestion: WAL_RECOVERY_SUGGESTION } : {}), + suggestion, + ...(recoverySuggestion ? { recoverySuggestion } : {}), }; } } @@ -4935,7 +4275,7 @@ export class LocalBackend { // ── Dispatch order (KTD5) ────────────────────────────────────────── // (1) Validate `mode`. Absent/'callgraph' → unchanged path; 'pdg' → the - // intra-procedural PDG engine (stubbed in U1); anything else → hard error. + // intra-procedural PDG engine; 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); @@ -4966,13 +4306,20 @@ export class LocalBackend { params.line !== undefined && (!Number.isInteger(params.line) || (params.line as number) < 1) ) { - return { - error: `Parameter 'line' must be a positive integer (1-based source line), got ${JSON.stringify(params.line)}.`, - target: { name: params.target }, - direction: params.direction, - impactedCount: 0, - risk: 'UNKNOWN', - }; + return mode === 'pdg' + ? makePdgImpactErrorResult({ + mode: 'pdg', + error: `Parameter 'line' must be a positive integer (1-based source line), got ${JSON.stringify(params.line)}.`, + target: { name: params.target }, + direction: params.direction, + }) + : { + error: `Parameter 'line' must be a positive integer (1-based source line), got ${JSON.stringify(params.line)}.`, + target: { name: params.target }, + direction: params.direction, + impactedCount: 0, + risk: 'UNKNOWN', + }; } if (mode === 'pdg') { @@ -4990,49 +4337,14 @@ export class LocalBackend { if (params.crossDepth !== undefined) incompatible.push('crossDepth'); if (params.minConfidence !== undefined) incompatible.push('minConfidence'); if (incompatible.length > 0) { - return { + return makePdgImpactErrorResult({ + mode: 'pdg', 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 (U2, KTD7) — the four-state degradation - // contract, BEFORE resolveSymbolCandidates / any traversal. A repo never - // analyzed with `--pdg` (no-layer), one with only a partial layer - // (sub-layer-missing — impact needs BOTH CDG and REACHING_DEF), or one whose - // meta is unreadable (unknown) each returns a distinct guidance note here - // rather than a confusing empty blast radius. Only `ready` falls through to - // the traversal (the `_runImpactPDG` stub until U3/U4). This fires before the - // stub deliberately, so a degraded layer is reported as such, not as - // "pdg mode not yet implemented". - if (mode === 'pdg') { - const layer = await pdgLayerStatus({ - lbugPath: repo.lbugPath, - executeParameterized, - }); - if (layer.state !== 'ready') { - return { - mode, - pdgLayer: layer.state, - ...(layer.missingSubLayer ? { missingSubLayer: layer.missingSubLayer } : {}), - note: layer.note, - target: { name: target }, - direction, - // No confident zero: a degraded layer is inconclusive, not "safe to - // refactor". UNKNOWN (KTD8) — never LOW (#2129/#1858 false-safe). - impactedCount: 0, - risk: 'UNKNOWN', - // KTD8 parity: the no-body / no-dependence PDG returns carry these, so - // the degraded return must too — a programmatic consumer iterating - // byDepth / reading byDepthCounts must not get `undefined` here. - ...emptyPdgParityFields(), - }; + }); } } @@ -5088,13 +4400,20 @@ export class LocalBackend { if (outcome.kind === 'not_found') { const missing = params.target_uid ?? target; - return { - error: `Target '${missing}' not found`, - target: { name: target }, - direction, - impactedCount: 0, - risk: 'UNKNOWN', - }; + return mode === 'pdg' + ? makePdgImpactErrorResult({ + mode: 'pdg', + error: `Target '${missing}' not found`, + target: { name: target }, + direction, + }) + : { + error: `Target '${missing}' not found`, + target: { name: target }, + direction, + impactedCount: 0, + risk: 'UNKNOWN', + }; } if (outcome.kind === 'ambiguous') { @@ -5265,9 +4584,37 @@ export class LocalBackend { }; const symType = outcome.resolvedLabel || outcome.symbol.type || ''; + // (2) PDG-layer presence probe (U2, KTD7) — the four-state degradation + // contract after target resolution, before traversal. A repo never analyzed + // with `--pdg` (no-layer), one with only a partial layer (sub-layer-missing + // — impact needs BOTH CDG and REACHING_DEF), or one whose meta is unreadable + // (unknown) each returns a distinct guidance note here rather than a + // confusing empty blast radius. Resolve first so target-known degraded + // responses keep the same id/type/filePath envelope as successful PDG + // responses; only `ready` falls through to traversal. + if (mode === 'pdg') { + const layer = await pdgLayerStatus({ + lbugPath: repo.lbugPath, + executeParameterized, + }); + if (isPdgDegradedLayerStatus(layer)) { + return makePdgLayerDegradedResult({ + mode, + layer, + target: { + id: sym.id, + name: sym.name, + type: symType || 'Function', + filePath: sym.filePath, + }, + direction, + }); + } + } + // (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. + // The PDG engine does NOT touch `_runImpactBFS`, so a `pdg` call never runs + // callgraph. if (mode === 'pdg') { return this._runImpactPDG({ repo, @@ -5278,9 +4625,8 @@ export class LocalBackend { line: params.line, limit: Number.isFinite(params.limit) ? params.limit : 100, // 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. + // explicitly rather than `this.`-binding it. LocalBackend owns repo + // lifecycle; `pdg-impact.ts` owns traversal/projection. executeParameterized, }); } @@ -5304,39 +4650,12 @@ export class LocalBackend { } /** - * U3 — the PDG blast-radius TRAVERSAL (KTD2, KTD4, KTD6, KTD11). + * Delegates the PDG impact engine to `pdg-impact.ts`. * - * Resolve the target symbol to its seed BasicBlocks, then run a direction-aware - * bounded BFS over `CDG` + `REACHING_DEF` block edges, returning the reachable - * block set with truncation signalling. Block→owning-symbol projection and the - * final impact-shaped result are U4 — this returns a PROVISIONAL payload - * exposing the reachable blocks for U4 to reshape: - * { mode:'pdg', target, direction, reachableBlocks, blockCount, - * truncated, depthReached, note? } - * - * ── KTD4 direction × edge-type truth table (the correctness keystone) ─────── - * The combined CDG+RD frontier traverses the SAME sense for BOTH edge types - * under one `direction` label (mixing forward on one and reverse on the other - * is a silent correctness bug): - * downstream — FORWARD on both: from a frontier block `a`, follow edges - * `(a)-[CDG|REACHING_DEF]->(b)` and collect `b`. RD def→use - * (where the def's value flows); CDG controller→dependent (what - * this block controls). "What does changing this affect?" - * upstream — REVERSE on both: from a frontier block `b`, follow edges - * `(a)-[CDG|REACHING_DEF]->(b)` and collect `a`. RD: the defs - * reaching this block's uses; CDG: the blocks controlling it. - * "What does this depend on?" - * - * ── KTD11 LadybugDB constraints ──────────────────────────────────────────── - * Every step is ANCHORED on exact BasicBlock ids (`.id IN $frontier` - * — the tightest possible anchor, bound as a param, never interpolated), - * DEPTH-bounded (≤ `maxDepth` BFS rounds), and `LIMIT`-bounded via a validated - * integer interpolation (`LIMIT` cannot be parameterized in LadybugDB). The - * `(a:BasicBlock)-[..]->(b:BasicBlock)` label pair keeps every query on the - * sparse BasicBlock→BasicBlock partition, never a symbol-space scan. - * - * Deps are injected (KTD2 extraction discipline) so this can later move to a - * standalone `pdg-impact.ts` engine as a move, not a rewrite. + * The private method remains as the LocalBackend dispatch seam so existing + * tests can keep asserting that `mode:'pdg'` routes here and never falls back + * to `_runImpactBFS`. The traversal/projection/result assembly lives in the + * extracted helper module. */ private async _runImpactPDG(deps: { repo: RepoHandle; @@ -5345,238 +4664,10 @@ export class LocalBackend { direction: 'upstream' | 'downstream'; maxDepth: number; limit: number; - /** Statement anchor (1-based source line) — see ImpactParams.line. */ line?: number; executeParameterized: typeof executeParameterized; - }): Promise { - const { repo, sym, direction, maxDepth, line, executeParameterized: exec } = deps; - // `line` present ⇒ statement-anchored slice (the useful mode); absent ⇒ - // whole-symbol seed (intra-procedural reach collapses to empty for a - // function — kept for back-compat, with a note steering the caller to `line`). - const statementMode = typeof line === 'number' && Number.isInteger(line) && line >= 1; - // `target` carries the call-graph-compatible shape (id/name/type/filePath) so - // `collectImpactSymbolUids` keys on it identically to a callgraph result. - const target = { - id: sym.id, - name: sym.name, - type: deps.symType || 'Function', - filePath: sym.filePath, - }; - - // Validate the per-step LIMIT as a positive integer (KTD11 — interpolated, - // so it must be sanitised, never user-string-passed). A non-integer / out-of - // range value (NaN, 1.5, negative, huge) is CLAMPED to the bounded default - // rather than rejected: impact's `limit` is a soft page hint, and a clamp - // keeps the safety tool producing a (flagged-bounded) radius instead of a - // hard error. The clamp ceiling matches `pdg_query`'s validated max. - const rawLimit = deps.limit; - const stepLimit = - Number.isInteger(rawLimit) && rawLimit >= 1 && rawLimit <= PDG_QUERY_MAX_LIMIT - ? rawLimit - : PDG_QUERY_DEFAULT_LIMIT; - // Depth: clamp to a sane positive integer (the caller default is 3). - const depthBudget = Number.isInteger(maxDepth) && maxDepth >= 1 ? maxDepth : 3; - - // ── Seed: anchor the target's BasicBlocks from the ALREADY-RESOLVED symbol ─ - // `_impactImpl` already resolved `sym` to a confident single match honoring - // the caller's target_uid/file_path/kind hints. Re-resolving by the bare - // `sym.name` here would RE-AMBIGUATE a disambiguated name (returning the - // "ambiguous" early payload instead of the PDG result) or anchor the seed on - // a DIFFERENT same-name symbol in another file (wrong-symbol blast radius). - // So build the seed anchor DIRECTLY from the resolved symbol's - // [startLine+1, endLine+1] window — the same window `resolveBlockAnchor`'s - // symbol branch produces, without re-running `resolveSymbolCandidates`. - const { anchorClause, queryParams } = statementMode - ? this.blockAnchorForStatement(sym, line as number) - : this.blockAnchorForResolvedSymbol(sym); - - const seedRows = await exec( - repo.lbugPath, - `MATCH (a:BasicBlock) WHERE ${anchorClause} RETURN a.id AS id LIMIT ${stepLimit}`, - queryParams, - ); - const seedBlocks: string[] = seedRows - .map((r: any) => String(r.id ?? r[0] ?? '')) - .filter((id: string) => id.length > 0); - // FIX 7: the seed query is `LIMIT $stepLimit`-bounded like every BFS step. - // A function with more seed blocks than `stepLimit` would silently under-seed - // (and thus under-report) — flag it so the result carries the same truncation - // signal the BFS steps do, never a silent partial seed. - const seedTruncated = seedRows.length >= stepLimit; - - // ── KTD6 no-body contract: distinguish "no PDG body" from "no dependence" ── - // A symbol that resolves but produces ZERO anchored blocks has no CFG body - // (interface / type alias / abstract / ambient / one-line const). A bare - // impactedCount:0 / risk:'LOW' would read as "safe to refactor" — the exact - // false-safe `impact` exists to prevent (#2129/#1858). Surface an explicit - // note + a non-LOW epistemic marker, never a silent confident zero. - if (seedBlocks.length === 0) { - return { - mode: 'pdg', - target, - direction, - ...(statementMode ? { criterionLine: line } : {}), - reachableBlocks: [], - blockCount: 0, - affectedStatements: [], - affectedStatementCount: 0, - truncated: false, - depthReached: 0, - // statementMode: the requested line has no statement block inside the - // symbol (blank line, comment, outside the body, or a line the CFG did - // not materialise). Distinct from "no PDG body". - epistemic: statementMode ? 'pdg-no-block-at-line' : 'no-pdg-body', - note: statementMode - ? `No PDG statement block starts at line ${line} within '${sym.name}' ` + - `(${sym.filePath}). The line may be blank, a comment, a brace, or outside ` + - `the symbol's body. Pass a line that begins an executable statement.` - : `'${sym.name}' has no PDG body — no BasicBlocks / control- or data-dependence ` + - `edges exist for this symbol (e.g. an interface, type alias, abstract/ambient ` + - `member, or a one-line declaration with no CFG). This is NOT a confident ` + - `"no impact": the intra-procedural PDG mode cannot model this symbol kind. ` + - `Pass line: to slice from a statement, or use mode:'callgraph' for the ` + - `inter-procedural blast radius.`, - impactedCount: 0, - risk: 'UNKNOWN', - // KTD8 parity fields so a consumer iterating byDepth / reading the - // depth counts on a no-body result still finds a well-formed (empty) - // shape rather than `undefined` (which would render as "isolated"). - ...emptyPdgParityFields(), - unresolvedBlockCount: 0, - ambiguousProjectionCount: 0, - }; - } - - // ── Bounded direction-aware BFS over CDG + REACHING_DEF (KTD4, KTD11) ────── - // Seed blocks are NOT counted as reachable (they ARE the target); the - // reachable set is everything the BFS discovers from them. Visited tracks - // BOTH seeds and discovered blocks so a cycle never re-expands. - const visited = new Set(seedBlocks); - const reachable = new Set(); - let frontier = [...seedBlocks]; - let depthReached = 0; - // `truncatedByDepth`: the BFS still had a non-empty frontier when the depth - // budget ran out (more reachable blocks exist past `maxDepth`). - // `truncatedByLimit`: a single step's neighbour query hit the interpolated - // LIMIT, so that step's expansion is a lower bound. The SEED query is - // LIMIT-bounded too, so `seedTruncated` seeds this flag — a partial seed is - // a lower-bound expansion just like a partial step. Either flags `truncated`. - let truncatedByDepth = false; - let truncatedByLimit = seedTruncated; - - // The endpoint the frontier is matched on, and the endpoint collected, flip - // by direction — but the SAME sense applies to BOTH edge types (KTD4). - // downstream: frontier = source `a`, collect target `b` (forward) - // upstream: frontier = target `b`, collect source `a` (reverse) - const matchEndpoint = direction === 'downstream' ? 'a' : 'b'; - const collectEndpoint = direction === 'downstream' ? 'b' : 'a'; - - for (let depth = 0; depth < depthBudget; depth++) { - if (frontier.length === 0) break; - // Anchored on exact frontier ids (bound as a param — KTD11). The edge-type - // discriminator is a hardcoded literal list (never user input). `LIMIT` is - // the validated integer `stepLimit`. - const rows = await exec( - repo.lbugPath, - `MATCH (a:BasicBlock)-[r:CodeRelation]->(b:BasicBlock) - WHERE r.type IN ['CDG', 'REACHING_DEF'] AND ${matchEndpoint}.id IN $frontier - RETURN DISTINCT ${collectEndpoint}.id AS id - LIMIT ${stepLimit}`, - { frontier }, - ); - depthReached = depth + 1; - if (rows.length >= stepLimit) truncatedByLimit = true; - - const next: string[] = []; - for (const r of rows) { - const id = String((r as any).id ?? (r as any)[0] ?? ''); - if (!id || visited.has(id)) continue; - visited.add(id); - reachable.add(id); - next.push(id); - } - frontier = next; - } - // Frontier still non-empty after exhausting the depth budget ⇒ more blocks - // are reachable beyond `maxDepth` (depth truncation, distinct from natural - // completion where the frontier drains to empty inside the loop). - if (frontier.length > 0) truncatedByDepth = true; - - const reachableBlocks = [...reachable].sort(); - const truncated = truncatedByDepth || truncatedByLimit; - const truncatedBy: 'depth' | 'limit' | undefined = truncatedByDepth - ? 'depth' - : truncatedByLimit - ? 'limit' - : undefined; - - // ── Resolve the reachable blocks to source statements (line + text) ──────── - // This is the useful output of statement mode: the dependent statements the - // change at `line` reaches. Fetched once for the whole reachable set; sorted - // by line. Failure surfaces (no `.catch` swallow) rather than masquerading - // as "no affected statements". - const affectedStatements = await pdgStatementsForBlocks(repo.lbugPath, reachableBlocks, exec); - - // ── Has a PDG body but no intra-procedural dependence reachability ───────── - // Distinct from "no PDG body": the function exists and has blocks, but no - // CDG/REACHING_DEF edge leaves the target's blocks in this direction. For a - // WHOLE-SYMBOL seed this is the expected (and uninformative) result — every - // intra-procedural block is already a seed — so the note steers to `line`. - // Still not a confident zero — explicit note + UNKNOWN (KTD6/KTD8). - if (reachableBlocks.length === 0) { - return { - mode: 'pdg', - target, - direction, - ...(statementMode ? { criterionLine: line } : {}), - impactedCount: 0, - risk: 'UNKNOWN', - epistemic: 'pdg-intra-procedural', - note: statementMode - ? `No statement in '${sym.name}' is ${direction}-dependent on line ${line} ` + - `(no CDG/REACHING_DEF reachability from that statement). The line may have no ` + - `dependents in this direction.` - : `'${sym.name}' has a PDG body but a WHOLE-SYMBOL ${direction} slice is empty: ` + - `intra-procedural dependence stays inside the function, so every reachable block ` + - `is already part of the seed. Pass line: to slice from a specific statement ` + - `(what depends on the code at that line), or use mode:'callgraph' for the ` + - `inter-procedural blast radius.`, - reachableBlocks: [] as string[], - blockCount: 0, - affectedStatements: [], - affectedStatementCount: 0, - depthReached, - unresolvedBlockCount: 0, - ambiguousProjectionCount: 0, - ...(truncated ? { truncated: true } : {}), - ...(truncatedBy ? { truncatedBy } : {}), - ...emptyPdgParityFields(), - }; - } - - // ── U4: project reachable blocks → owning symbols, assemble parity result ── - const projection = await projectBlocksToSymbols({ - lbugPath: repo.lbugPath, - blockIds: reachableBlocks, - executeParameterized: exec, - }); - - return assemblePdgImpactResult({ - target: { - id: sym.id, - name: sym.name, - type: deps.symType || 'Function', - filePath: sym.filePath, - }, - direction, - reachableBlocks, - affectedStatements, - criterionLine: statementMode ? (line as number) : undefined, - projection, - depthReached, - truncated, - truncatedBy, - }); + }): Promise { + return runImpactPDG(deps); } /** @@ -6492,12 +5583,17 @@ export class LocalBackend { const groupModeResult = validateImpactMode(params.mode); if ('error' in groupModeResult) return { error: groupModeResult.error }; if (groupModeResult.mode === 'pdg') { - return { + return makePdgImpactErrorResult({ + mode: 'pdg', 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.', - }; + target: { name: String(params.target ?? '') }, + direction: (params.direction === 'downstream' ? 'downstream' : 'upstream') as + | 'upstream' + | 'downstream', + }); } const impactArgs: Record = { name: groupName, diff --git a/gitnexus/src/mcp/local/pdg-impact.ts b/gitnexus/src/mcp/local/pdg-impact.ts new file mode 100644 index 000000000..d4ac6fbea --- /dev/null +++ b/gitnexus/src/mcp/local/pdg-impact.ts @@ -0,0 +1,1080 @@ +/** + * PDG-backed impact helpers. + * + * Extracted from `local-backend.ts` so LocalBackend owns dispatch/repo lifecycle + * while this module owns the PDG layer probe, statement traversal, block + * projection, and result assembly contract. + */ + +import path from 'path'; +import type { executeParameterized } from '../../core/lbug/pool-adapter.js'; +import { loadMeta } from '../../storage/repo-manager.js'; +import { IMPACT_MAX_DEPTH, PDG_QUERY_DEFAULT_LIMIT, PDG_QUERY_MAX_LIMIT } from '../tools.js'; + +/** + * Parse the `` segment out of a `BasicBlock` id (1-based function start + * line). The id template is + * `BasicBlock::::` + * and `` may itself contain `':'` (a Windows drive letter), so the + * segments are taken from the RIGHT: `` is last, `` second-last, + * `` third-last. Extracted from the `_pdgQueryImpl` closure (#2086) into + * a shared module-scope helper so the PDG impact traversal (U3/U4) reuses the + * exact same parse — the `pdg_query` read path is byte-identical to before. + */ +export function fnLineOf(id: string): number { + const parts = id.split(':'); + return Number(parts[parts.length - 3]); +} + +/** + * Parse the `` segment out of a `BasicBlock` id, the COUNTERPART to + * `fnLineOf`. The id template is `BasicBlock::::`, + * so the file path is everything BETWEEN the `BasicBlock:` prefix and the last + * THREE colon-segments (`::`). `` may itself + * contain `':'` (a Windows drive letter), so we strip from both ends rather than + * split-and-pick. Returns `''` for an unparseable id (treated as unresolved). + */ +function fnFileOf(id: string): string { + const parts = id.split(':'); + // Need: `BasicBlock` + filePath(≥1) + fnLine + fnCol + blockIdx ⇒ ≥5 segments. + if (parts.length < 5 || parts[0] !== 'BasicBlock') return ''; + // Drop the leading `BasicBlock` token and the trailing fnLine/fnCol/blockIdx, + // rejoin the middle on ':' to restore a path that itself contained colons. + return parts.slice(1, parts.length - 3).join(':'); +} + +/** A reachable dependence block resolved to its source statement. */ +export interface PdgStatement { + /** 1-based source line where the statement's block starts. */ + line: number; + /** Repo-relative file path (parsed from the block id). */ + filePath: string; + /** The statement's source text (BasicBlock.text), trimmed. */ + text: string; +} + +/** + * Resolve a set of reachable BasicBlock ids to their source statements + * (line + text), deduped by BasicBlock id and sorted by line. This is the + * useful output of a statement-anchored PDG slice — the dependent statements the + * change reaches. A query error propagates (no `.catch` swallow) so a DB failure + * is never silently reported as "no affected statements". + */ +async function pdgStatementsForBlocks( + lbugPath: string, + blockIds: string[], + exec: typeof executeParameterized, +): Promise { + if (blockIds.length === 0) return []; + const rows = await exec( + lbugPath, + `MATCH (b:BasicBlock) WHERE b.id IN $ids + RETURN b.id AS id, b.startLine AS line, b.text AS text`, + { ids: blockIds }, + ); + const byKey = new Map(); + for (const r of rows as any[]) { + const id = String(r.id ?? r[0] ?? ''); + const line = Number(r.line ?? r[1] ?? 0); + if (!id || !Number.isFinite(line) || line <= 0) continue; + const filePath = fnFileOf(id); + const text = String(r.text ?? r[2] ?? '').trim(); + const key = `${filePath}:${line}:${id}`; + if (!byKey.has(key)) byKey.set(key, { line, filePath, text }); + } + return [...byKey.values()].sort((a, b) => { + if (a.filePath !== b.filePath) return a.filePath < b.filePath ? -1 : 1; + if (a.line !== b.line) return a.line - b.line; + return a.text < b.text ? -1 : a.text > b.text ? 1 : 0; + }); +} + +// ── Block → owning-symbol projection types (U4) ────────────────────────────── + +/** + * One owning-symbol candidate for a reachable BasicBlock, OR an explicit + * `unresolved` marker for a block that maps to no `Function`/`Method`/`Constructor` symbol + * (top-level/free-statement block, or a nested lambda whose start line ≠ any + * symbol `startLine`). A null `id` is the shadow-path marker — the block is + * surfaced under its file, never silently dropped (R9: a silent drop is a hidden + * recall loss). + */ +interface OwningSymbol { + /** Symbol UID, or `null` for the `unresolved` shadow-path entry. */ + id: string | null; + name: string; + /** `'Function' | 'Method' | …`, or `'unresolved'` for the shadow path. */ + type: string; + filePath: string; + /** Symbol `startLine` (0-based), present only for a resolved symbol. */ + startLine?: number; + /** + * True when this block's `(filePath, startLine)` query matched >1 symbol — + * same-line, different-name functions that the schema cannot disambiguate + * (no `startColumn` column; Feasibility Finding 1). ALL colliding symbols are + * reported (never a silent pick), each carrying this flag. + */ + ambiguous?: boolean; +} + +/** + * Net-new block → owning-symbol resolver (U4) — the REVERSE of + * `resolveBlockAnchor` (which goes symbol→blocks). No precedent exists: + * `_pdgQueryImpl` only ever extracts a raw `functionLine`, never an owning + * symbol. Lives in the extracted PDG impact engine and takes injected deps so + * LocalBackend keeps repo lifecycle/dispatch while this module owns projection. + * + * For each reachable block id `BasicBlock::::`: + * - `fnLineOf` → 1-based function start line; `fnFileOf` → file path. + * - Query `Function`/`Method`/`Constructor` `WHERE filePath = $f AND startLine = (fnLine-1)` + * — block `fnLine` is 1-based, symbol `startLine` is 0-based, so subtract one + * (the `[symStart+1]` convention from `resolveBlockAnchor`, applied in + * reverse; NOT re-derived). + * + * Two non-happy paths, BOTH surfaced (never silent): + * - **>1 match** (same-line different-name functions): `fnCol` rides the block + * id but the schema has NO `startColumn` column and the symbol id encodes only + * the name, so a `(filePath, startLine)` join cannot disambiguate. Report ALL + * colliding symbols, each `ambiguous: true` (R4 / Feasibility Finding 1). + * - **0 matches** (top-level/free-statement block, or a lambda whose start line + * ≠ a symbol `startLine`): one `unresolved` entry (`id: null`) under the + * block's file (R9 shadow path). + * + * Distinct `(filePath, fnLine)` pairs are queried once each (a block and its + * siblings in the same function share a pair), so the cost is O(distinct + * functions), not O(blocks). + */ +async function projectBlocksToSymbols(deps: { + lbugPath: string; + blockIds: string[]; + executeParameterized: typeof executeParameterized; +}): Promise<{ symbols: OwningSymbol[]; unresolvedCount: number; ambiguousCount: number }> { + const { lbugPath, blockIds, executeParameterized: exec } = deps; + + // Group blocks by their (filePath, fnLine) owning-function key so each owning + // function is resolved with a single query regardless of block count. + const byFnKey = new Map(); + for (const id of blockIds) { + const filePath = fnFileOf(id); + const fnLine = fnLineOf(id); // 1-based + if (!filePath || !Number.isFinite(fnLine)) { + // Unparseable block id — record an unresolved key so it is reported, never + // dropped. Use the raw id as the key so duplicates collapse. + byFnKey.set(`#bad#${id}`, { filePath: filePath || id, symStart: NaN }); + continue; + } + const symStart = fnLine - 1; // 0-based symbol startLine (reverse [symStart+1]) + byFnKey.set(`${filePath}#${symStart}`, { filePath, symStart }); + } + + const resolved: OwningSymbol[] = []; + let unresolvedCount = 0; + let ambiguousCount = 0; + + await Promise.all( + Array.from(byFnKey.values()).map(async ({ filePath, symStart }) => { + if (!Number.isFinite(symStart)) { + // Unparseable id — shadow-path unresolved under (best-effort) file. + resolved.push({ id: null, name: '(unresolved)', type: 'unresolved', filePath }); + unresolvedCount += 1; + return; + } + // `Function`/`Method`/`Constructor` carry name+filePath+startLine; the schema has NO + // `startColumn`, so the join is on (filePath, startLine) only. `filePath` + // and `symStart` are BOUND as params (KTD11 — never interpolated). A + // UNION ALL across explicit labels is used rather than a + // `(s:Function OR s:Method OR s:Constructor)` disjunction (unsupported in the LadybugDB + // Cypher subset — the established cross-label pattern, see + // `enrichCandidateLabels`). + // FIX 6: do NOT swallow a query failure as `[]`. A DB error (lock / + // corruption / missing path) must NOT masquerade as a genuine no-owning- + // symbol result — that would silently inflate `unresolvedCount` and hide + // the failure. Letting it reject propagates through `Promise.all` → + // `projectBlocksToSymbols` → `_runImpactPDG` → `_impactImpl` up to the + // `impact()` structured-error catch, where it surfaces as a real error + // with a recovery suggestion (rather than a clean-looking partial radius). + const rows = await exec( + lbugPath, + `MATCH (s:\`Function\`) + WHERE s.filePath = $filePath AND s.startLine = $symStart + RETURN s.id AS id, s.name AS name, 'Function' AS label, s.startLine AS startLine + UNION ALL + MATCH (s:\`Method\`) + WHERE s.filePath = $filePath AND s.startLine = $symStart + RETURN s.id AS id, s.name AS name, 'Method' AS label, s.startLine AS startLine + UNION ALL + MATCH (s:\`Constructor\`) + WHERE s.filePath = $filePath AND s.startLine = $symStart + RETURN s.id AS id, s.name AS name, 'Constructor' AS label, s.startLine AS startLine`, + { filePath, symStart }, + ); + + if (rows.length === 0) { + // No owning symbol — top-level/free-statement block or a lambda whose + // start line ≠ a symbol startLine. Shadow path: report under its file. + resolved.push({ + id: null, + name: '(unresolved)', + type: 'unresolved', + filePath, + startLine: symStart, + }); + unresolvedCount += 1; + return; + } + + // >1 ⇒ ambiguous-projection (same-line, different-name functions). Report + // ALL colliding symbols, NEVER silently pick one (R4 / Feasibility 1). + const isAmbiguous = rows.length > 1; + for (const r of rows) { + resolved.push({ + id: String((r as any).id ?? (r as any)[0] ?? ''), + name: String((r as any).name ?? (r as any)[1] ?? ''), + type: String((r as any).label ?? (r as any)[2] ?? 'Function'), + filePath, + startLine: Number((r as any).startLine ?? (r as any)[3] ?? symStart), + ...(isAmbiguous ? { ambiguous: true as const } : {}), + }); + } + if (isAmbiguous) ambiguousCount += 1; + }), + ); + + // Deterministic order: by filePath, then startLine, then id (unresolved last + // within a file). Order-independence matters for the parity/fingerprint + // contract (KTD8 standing interchangeability) and for stable consumer output. + resolved.sort((a, b) => { + if (a.filePath !== b.filePath) return a.filePath < b.filePath ? -1 : 1; + const al = a.startLine ?? Number.MAX_SAFE_INTEGER; + const bl = b.startLine ?? Number.MAX_SAFE_INTEGER; + if (al !== bl) return al - bl; + const ai = a.id ?? '￿'; + const bi = b.id ?? '￿'; + return ai < bi ? -1 : ai > bi ? 1 : 0; + }); + + return { symbols: resolved, unresolvedCount, ambiguousCount }; +} + +/** + * The KTD8 parity fields a PDG impact result carries even when it short-circuits + * to an empty radius (degraded layer / no PDG body / no dependence reachability). + * + * A programmatic consumer iterating `byDepth`, reading `byDepthCounts[1]`, or + * coalescing `affected_processes`/`affected_modules` must find a well-formed + * (empty) shape on EVERY early return, not `undefined` (which would render as + * "isolated"/"no data" instead of "inconclusive"). The CLI branches on + * `pdgLayer` first so it is safe regardless, but the JSON contract must be + * uniform across all three early returns — this single source guarantees that. + */ +function emptyPdgParityFields(): { + byDepth: Record; + byDepthCounts: Record; + summary: { direct: number; processes_affected: number; modules_affected: number }; + affected_processes: unknown[]; + affected_modules: unknown[]; +} { + return { + byDepth: {}, + byDepthCounts: { 1: 0 }, + summary: { direct: 0, processes_affected: 0, modules_affected: 0 }, + affected_processes: [], + affected_modules: [], + }; +} + +export interface PdgImpactTarget { + name: string; + id?: string; + type?: string; + filePath?: string; +} + +export interface PdgImpactParityFields { + byDepth: Record; + byDepthCounts: Record; + summary: { direct: number; processes_affected: number; modules_affected: number }; + affected_processes: unknown[]; + affected_modules: unknown[]; +} + +export interface PdgImpactBaseResult extends PdgImpactParityFields { + mode: 'pdg'; + target: PdgImpactTarget; + direction: 'upstream' | 'downstream'; + impactedCount: number; + risk: 'UNKNOWN'; + note?: string; +} + +export interface PdgImpactSuccessResult extends PdgImpactBaseResult { + target: Required; + epistemic: 'pdg-intra-procedural'; + reachableBlocks: string[]; + blockCount: number; + affectedStatements: PdgStatement[]; + affectedStatementCount: number; + depthReached: number; + unresolvedBlockCount: number; + ambiguousProjectionCount: number; + criterionLine?: number; + truncated?: boolean; + truncatedBy?: 'depth' | 'limit'; + truncatedByReasons?: readonly ('depth' | 'limit')[]; +} + +export interface PdgImpactEmptyResult extends PdgImpactBaseResult { + target: Required; + epistemic: 'no-pdg-body' | 'pdg-no-block-at-line' | 'pdg-intra-procedural'; + reachableBlocks: string[]; + blockCount: number; + affectedStatements: PdgStatement[]; + affectedStatementCount: number; + depthReached: number; + unresolvedBlockCount: number; + ambiguousProjectionCount: number; + criterionLine?: number; + truncated?: boolean; + truncatedBy?: 'depth' | 'limit'; + truncatedByReasons?: readonly ('depth' | 'limit')[]; +} + +export type PdgDegradedLayerState = Exclude; +export type PdgDegradedLayerStatus = PdgLayerStatus & { state: PdgDegradedLayerState }; + +export interface PdgImpactDegradedResult extends PdgImpactBaseResult { + pdgLayer: PdgDegradedLayerState; + missingSubLayer?: PdgSubLayer; +} + +export interface PdgImpactErrorResult { + mode?: 'pdg'; + error: string; + target: PdgImpactTarget; + direction: 'upstream' | 'downstream'; + impactedCount: 0; + risk: 'UNKNOWN'; + suggestion?: string; + recoverySuggestion?: string; +} + +export type PdgImpactResult = + | PdgImpactSuccessResult + | PdgImpactEmptyResult + | PdgImpactDegradedResult + | PdgImpactErrorResult; + +export function makePdgImpactErrorResult(input: { + error: string; + target: PdgImpactTarget; + direction: 'upstream' | 'downstream'; + mode?: 'pdg'; + suggestion?: string; + recoverySuggestion?: string; +}): PdgImpactErrorResult { + return { + ...(input.mode ? { mode: input.mode } : {}), + error: input.error, + target: input.target, + direction: input.direction, + impactedCount: 0, + risk: 'UNKNOWN', + ...(input.suggestion ? { suggestion: input.suggestion } : {}), + ...(input.recoverySuggestion ? { recoverySuggestion: input.recoverySuggestion } : {}), + }; +} + +export function isPdgDegradedLayerStatus(layer: PdgLayerStatus): layer is PdgDegradedLayerStatus { + return layer.state !== 'ready'; +} + +export function makePdgLayerDegradedResult(input: { + mode: 'pdg'; + target: PdgImpactTarget; + direction: 'upstream' | 'downstream'; + layer: PdgDegradedLayerStatus; +}): PdgImpactDegradedResult { + return { + mode: input.mode, + pdgLayer: input.layer.state, + ...(input.layer.missingSubLayer ? { missingSubLayer: input.layer.missingSubLayer } : {}), + note: input.layer.note, + target: input.target, + direction: input.direction, + impactedCount: 0, + risk: 'UNKNOWN', + ...emptyPdgParityFields(), + }; +} + +/** + * Assemble the consumer-safe PDG impact result (U4 / KTD8 parity matrix). + * + * Takes the U3 traversal output (reachable block set + truncation signalling) + * plus the U4 block→symbol projection, and shapes a result STRUCTURALLY + * substitutable for the call-graph `_runImpactBFS` result so every consumer + * (CLI `formatImpactResult`, group `collectImpactSymbolUids`/`mergeRisk`, + * `impactByUid`) renders it without misrendering. This is a STANDING + * interchangeability contract, not a one-time check. + * + * Field-by-field vs the call-graph result (KTD8): + * - `target.id/name/type/filePath` — identical shape (`collectImpactSymbolUids` + * keys on `target.id`/`target.filePath`). + * - `byDepth` — same `{ [depth]: item[] }` map shape, but COLLAPSED to a single + * bucket (`1`): intra-procedural dependence has no meaningful inter-symbol hop + * count (block-hops are NOT call-hops). Items carry `{ id, name, type, + * filePath, … }` exactly like the call-graph items so `collectImpactSymbolUids` + * collects their UIDs. `unresolved` shadow-path entries keep `id: null` (they + * are surfaced, never dropped — but collect as no UID). + * - `byDepthCounts` — `{ 1: }`, same shape. + * - `affected_processes` / `affected_modules` — empty `[]` (no + * STEP_IN_PROCESS/module edges originate from BasicBlocks; consumers coalesce + * `[]` safely). + * - `epistemic` — a PDG-specific marker (`'pdg-intra-procedural'`), NOT the + * callgraph DI/dynamic-dispatch `'lower-bound'` copy. `note` carries the + * PDG framing so the CLI prints PDG text, not callgraph boundary text. + * - `risk` — the existing `'UNKNOWN'` sentinel (NOT a new label). `mergeRisk` + * already coalesces `'UNKNOWN'` correctly (never a confident `LOW`). + * - `impactedCount` — count of DISTINCT owning SYMBOLS (resolved UIDs), the + * meaningful unit for the impact question ("which symbols are affected"). + * `blockCount` is retained separately as the raw reachable-block count. + */ +function assemblePdgImpactResult(input: { + target: { id: string; name: string; type: string; filePath: string }; + direction: 'upstream' | 'downstream'; + reachableBlocks: string[]; + /** Reachable blocks resolved to source statements (the useful slice output). */ + affectedStatements?: PdgStatement[]; + /** The 1-based source line the slice was seeded on (statement mode only). */ + criterionLine?: number; + projection: { symbols: OwningSymbol[]; unresolvedCount: number; ambiguousCount: number }; + depthReached: number; + truncated: boolean; + truncatedBy?: 'depth' | 'limit'; + truncatedByReasons?: readonly ('depth' | 'limit')[]; +}): PdgImpactSuccessResult { + const { target, direction, reachableBlocks, projection } = input; + const { symbols, unresolvedCount, ambiguousCount } = projection; + const affectedStatements = input.affectedStatements ?? []; + const statementMode = typeof input.criterionLine === 'number'; + + // Items for the single collapsed bucket. Shaped like the call-graph byDepth + // items (`{ depth, id, name, type, filePath, processes }`) so consumers that + // iterate byDepth read the same fields. `unresolved` entries keep `id: null` + // (surfaced under their file; `collectImpactSymbolUids` skips a null id, which + // is correct — there is no symbol UID to attribute). + const items = symbols.map((s) => ({ + depth: 1, + id: s.id, + name: s.name, + type: s.type, + filePath: s.filePath, + ...(s.startLine !== undefined ? { startLine: s.startLine } : {}), + ...(s.ambiguous ? { ambiguous: true } : {}), + ...(s.id === null ? { unresolved: true } : {}), + processes: [] as unknown[], + })); + + // impactedCount = distinct owning SYMBOLS (resolved UIDs). Unresolved shadow + // entries are surfaced in byDepth but do NOT inflate the symbol count. + const resolvedUids = new Set(symbols.filter((s) => s.id !== null).map((s) => s.id as string)); + const impactedCount = resolvedUids.size; + + const byDepth: Record = items.length > 0 ? { 1: items } : {}; + const byDepthCounts: Record = { 1: items.length }; + + const noteParts: string[] = statementMode + ? [ + `mode:'pdg' — intra-procedural slice from line ${input.criterionLine} of ` + + `'${target.name}'. ${affectedStatements.length} ` + + `${affectedStatements.length === 1 ? 'statement is' : 'statements are'} ${direction}-` + + `dependent on it (over CDG + REACHING_DEF). Cross-function (inter-procedural) impact ` + + `is NOT modeled in this mode — use mode:'callgraph' for the call-graph blast radius.`, + ] + : [ + `mode:'pdg' — intra-procedural Program Dependence Graph. ${impactedCount} owning ` + + `${impactedCount === 1 ? 'symbol' : 'symbols'} reached via ${reachableBlocks.length} ` + + `dependence ${reachableBlocks.length === 1 ? 'block' : 'blocks'} ` + + `(${direction} over CDG + REACHING_DEF). Cross-function (inter-procedural) impact is ` + + `NOT modeled in this mode — use mode:'callgraph' for the call-graph blast radius.`, + ]; + if (ambiguousCount > 0) { + noteParts.push( + `${ambiguousCount} owning-symbol ${ambiguousCount === 1 ? 'projection is' : 'projections are'} ` + + `ambiguous: same-line functions cannot be disambiguated by start line alone (no startColumn ` + + `in the schema), so ALL colliding symbols are reported — none is silently picked.`, + ); + } + if (unresolvedCount > 0) { + noteParts.push( + `${unresolvedCount} reachable ${unresolvedCount === 1 ? 'block maps' : 'blocks map'} to no ` + + `owning Function/Method/Constructor (top-level statement or a lambda whose start line is not a symbol ` + + `start) — surfaced under their file as 'unresolved', never dropped.`, + ); + } + + return { + mode: 'pdg', + target, + direction, + impactedCount, + // KTD8: reuse the existing UNKNOWN sentinel — never a confident LOW (which + // would read as "safe to refactor"; #2129/#1858 false-safe lineage). PDG + // mode is intra-procedural, so its count is a per-function lower bound on the + // true blast radius and risk is genuinely UNKNOWN at the program level. + risk: 'UNKNOWN', + // PDG-specific epistemic marker — NOT the callgraph 'lower-bound'/DI copy. + epistemic: 'pdg-intra-procedural', + note: noteParts.join(' '), + // Statement-level slice: the dependent source statements (line + text) the + // change reaches. This is the primary useful output of statement mode; the + // accuracy harness scores against these lines. + ...(statementMode ? { criterionLine: input.criterionLine } : {}), + affectedStatements, + affectedStatementCount: affectedStatements.length, + // Raw block-level detail retained alongside the symbol projection (U3 tests + // and the accuracy harness read these). + reachableBlocks, + blockCount: reachableBlocks.length, + depthReached: input.depthReached, + unresolvedBlockCount: unresolvedCount, + ambiguousProjectionCount: ambiguousCount, + ...(input.truncated ? { truncated: true } : {}), + ...(input.truncatedBy ? { truncatedBy: input.truncatedBy } : {}), + ...(input.truncatedByReasons ? { truncatedByReasons: input.truncatedByReasons } : {}), + summary: { + direct: impactedCount, + processes_affected: 0, + modules_affected: 0, + }, + byDepthCounts, + affected_processes: [] as unknown[], + affected_modules: [] as unknown[], + byDepth, + }; +} + +/** 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. + */ +export 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)}.`, + }; +} + +/** The two independently-stamped PDG sub-layers (KTD7). */ +export type PdgSubLayer = 'CDG' | 'REACHING_DEF'; + +/** + * Four-state PDG-layer presence/degradation status (KTD7). + * + * - `'no-layer'` — `meta.pdg` is absent: this repo was never analyzed + * with `--pdg` (definitive; established with NO DB scan). + * - `'sub-layer-missing'`— exactly one of the two independently-stamped caps + * (`maxCdgEdgesPerFunction` / `maxReachingDefEdgesPerFunction`) + * is present. `impact`'s PDG mode needs BOTH, so a + * partial layer must not be reported as complete; the + * missing one is named in `missingSubLayer`. + * - `'ready'` — both caps present: the layer is fully stamped. + * - `'unknown'` — meta is unreadable (e.g. a seeded test DB with no + * `meta.json`). One bounded `LIMIT 1` probe distinguishes + * a genuinely edge-free index from a missing one; either + * way the conclusion is inconclusive (a missing layer is + * indistinguishable from an all-linear one — #2188). + */ +export interface PdgLayerStatus { + state: 'no-layer' | 'sub-layer-missing' | 'ready' | 'unknown'; + /** Set only for `'sub-layer-missing'` — the cap that was NOT stamped. */ + missingSubLayer?: PdgSubLayer; + /** Human-readable guidance for the degraded states (absent for `'ready'`). */ + note?: string; +} + +/** + * Per-cap presence read from `meta.pdg`, plus whether meta was readable at all. + * + * `metaReadable` is the seam between the `'unknown'` state (meta unreadable — + * fall through to a DB probe) and the meta-stamped states. When `metaReadable` + * is true but `meta.pdg` was absent, both `cdg`/`rd` are `false`. + */ +interface PdgMetaCaps { + metaReadable: boolean; + /** `maxCdgEdgesPerFunction !== undefined` (only meaningful when metaReadable). */ + cdg: boolean; + /** `maxReachingDefEdgesPerFunction !== undefined` (only meaningful when metaReadable). */ + rd: boolean; +} + +/** + * Read the two PDG sub-layer caps from the on-disk `meta.json` stamp — the + * single shared meta-probe both `_pdgQueryImpl` (one cap) and the PDG impact + * mode (both caps) key on. Never scans the DB. An unreadable / missing meta + * yields `metaReadable: false` (the `'unknown'` seam); a readable meta with no + * `pdg` stamp yields `metaReadable: true` with both caps `false` (no-layer). + */ +async function readPdgMetaCaps( + lbugPath: string, + loadMetaFn: typeof loadMeta, +): Promise { + try { + const meta = await loadMetaFn(path.dirname(lbugPath)); + if (!meta) return { metaReadable: false, cdg: false, rd: false }; + return { + metaReadable: true, + cdg: meta.pdg?.maxCdgEdgesPerFunction !== undefined, + rd: meta.pdg?.maxReachingDefEdgesPerFunction !== undefined, + }; + } catch { + // Meta unreadable — the caller decides from the DB (the `'unknown'` state). + return { metaReadable: false, cdg: false, rd: false }; + } +} + +/** + * Project the both-caps PDG meta read down to the single mode-relevant cap that + * `_pdgQueryImpl` keys on (`controls` → CDG, `flows` → REACHING_DEF), preserving + * its established tri-state `boolean | undefined` contract byte-for-byte + * (Feasibility Issue 4): + * - `false` — meta readable and the relevant cap absent → definitive + * no-layer (short-circuits before any DB scan). + * - `true` — meta readable and the relevant cap present → proceed. + * - `undefined` — meta unreadable → defer to the post-anchored-query probe. + * + * `_pdgQueryImpl` needs only ONE cap, so it collapses the both-caps read here + * rather than consuming `pdgLayerStatus` directly (whose `'unknown'` state does + * an upfront global probe — wrong timing/order for the anchored-query path). + */ +export async function pdgStampForMode( + lbugPath: string, + mode: 'controls' | 'flows', + loadMetaFn: typeof loadMeta = loadMeta, +): Promise { + const caps = await readPdgMetaCaps(lbugPath, loadMetaFn); + if (!caps.metaReadable) return undefined; + return mode === 'controls' ? caps.cdg : caps.rd; +} + +/** + * PDG-layer presence/degradation check for the `impact` PDG mode (KTD7). + * + * Returns the four distinct states WITHOUT scanning the DB except for the single + * bounded `LIMIT 1` probe the `'unknown'` (meta-unreadable) case requires. The + * caller (`_impactImpl` PDG branch, and the accuracy harness) surfaces a + * distinct signal per state so a missing `--pdg` layer / partial layer is never + * silently misread as a confident empty blast radius. Impact needs BOTH the CDG + * and the REACHING_DEF sub-layer, so a partial stamp degrades, not proceeds. + */ +export async function pdgLayerStatus(deps: { + lbugPath: string; + executeParameterized: typeof executeParameterized; + loadMetaFn?: typeof loadMeta; +}): Promise { + const loadMetaFn = deps.loadMetaFn ?? loadMeta; + const caps = await readPdgMetaCaps(deps.lbugPath, loadMetaFn); + + if (caps.metaReadable) { + // Meta is readable — the stamp is authoritative, no DB scan needed. + if (caps.cdg && caps.rd) return { state: 'ready' }; + if (caps.cdg !== caps.rd) { + // Exactly one sub-layer stamped (XOR) — partial layer; impact needs both. + const missingSubLayer: PdgSubLayer = caps.cdg ? 'REACHING_DEF' : 'CDG'; + return { + state: 'sub-layer-missing', + missingSubLayer, + note: + `PDG layer is incomplete — the ${missingSubLayer} sub-layer is missing ` + + `(impact's PDG mode needs both CDG and REACHING_DEF). ` + + `Re-run gitnexus analyze --pdg to record it.`, + }; + } + // Neither cap stamped (meta.pdg absent, or present with no caps) → the layer + // was never recorded. Definitive, no DB scan. + return { + state: 'no-layer', + note: 'no PDG layer — run gitnexus analyze --pdg to record CDG + REACHING_DEF edges for this repo', + }; + } + + // Meta unreadable (e.g. a seeded test DB): one bounded probe confirms the + // layer status is genuinely undeterminable from the DB. A missing layer is + // indistinguishable from an all-linear (edge-free) one (#2188), so whether the + // probe finds a row or not the state stays `'unknown'` (never the definitive + // no-layer wording). The probe is bounded (`LIMIT 1`) and anchored on the + // BasicBlock→BasicBlock partition (the `(:BasicBlock)…(:BasicBlock)` label pair + // restricts it to the sparse pdg-edge partition, never a global rel scan — the + // established `_explainImpl` anchoring pattern), and it is wrapped so a db-lock + // / missing-path throw degrades to the same `'unknown'` signal rather than + // propagating and losing it. + // + // The probe result is NOT discarded: a visible CDG/REACHING_DEF edge (with + // meta unreadable) is a weak-but-real "edges are present, but completeness is + // unprovable" signal, distinct from "no edges visible at all". Both stay + // `'unknown'` (inconclusive), but the note distinguishes them so the operator + // gets the more useful hint. + let edgesVisible = false; + try { + const rows = await deps.executeParameterized( + deps.lbugPath, + `MATCH (:BasicBlock)-[r:CodeRelation]->(:BasicBlock) WHERE r.type IN ['CDG', 'REACHING_DEF'] RETURN r.type AS type LIMIT 1`, + {}, + ); + edgesVisible = Array.isArray(rows) && rows.length > 0; + } catch { + // db-lock / missing-path / corrupt probe — fall through as not-visible, but + // keep the `'unknown'` signal (a probe failure must not lose it). + edgesVisible = false; + } + return { + state: 'unknown', + note: edgesVisible + ? 'PDG layer status unknown — CDG/REACHING_DEF edges ARE visible but meta is unreadable, so the layer cannot be confirmed complete (a partial layer looks the same); was this repo fully indexed with gitnexus analyze --pdg?' + : 'PDG layer status unknown — no CDG/REACHING_DEF edges visible and meta is unreadable; was this repo indexed with gitnexus analyze --pdg?', + }; +} + +/** + * Build the SAME BasicBlock seed anchor (`anchorClause` + `queryParams`) as + * `resolveBlockAnchor`'s symbol branch, but from an ALREADY-RESOLVED symbol — + * WITHOUT re-running `resolveSymbolCandidates`. + * + * Why this exists (correctness keystone): `_impactImpl` already resolves the + * target to a confident single symbol honoring the caller's + * `target_uid`/`file_path`/`kind` hints. Re-resolving by the bare `sym.name` + * inside `_runImpactPDG` would (a) RE-AMBIGUATE a globally-ambiguous name the + * caller had disambiguated (returning the "ambiguous" early payload instead of + * the PDG result), or (b) anchor the seed on a DIFFERENT same-name symbol in + * another file → a wrong-symbol blast radius. Anchoring directly from the + * resolved `{ filePath, startLine, endLine }` preserves the disambiguation. + * + * The window is byte-identical to `resolveBlockAnchor`'s symbol branch: BOTH + * span bounds are shifted `+1` (1-based BasicBlock `startLine` vs the 0-based + * symbol span — the lower `+1` excludes a neighbor's block on the line above, + * the upper `+1` keeps a guard/def/use on the final line). A symbol with no + * usable span degrades to the same file-level id-prefix filter. This is the + * resolved-symbol counterpart, NOT a second window convention. + */ +function blockAnchorForResolvedSymbol(sym: { + filePath: string; + startLine?: number; + endLine?: number; +}): { anchorClause: string; queryParams: Record } { + const idPrefix = `BasicBlock:${sym.filePath}:`; + if ( + typeof sym.startLine === 'number' && + typeof sym.endLine === 'number' && + sym.endLine >= sym.startLine + ) { + return { + anchorClause: + 'a.id STARTS WITH $idPrefix AND a.startLine >= $symStart AND a.startLine <= $symEnd', + queryParams: { idPrefix, symStart: sym.startLine + 1, symEnd: sym.endLine + 1 }, + }; + } + return { anchorClause: 'a.id STARTS WITH $idPrefix', queryParams: { idPrefix } }; +} + +/** + * Build a STATEMENT seed anchor: the BasicBlock(s) starting at a specific + * 1-based source `line` WITHIN the resolved symbol. This is what makes + * `mode:'pdg'` useful — seeding the dependence slice on a single statement + * (the thing being changed) rather than the whole symbol. A whole-symbol seed + * captures every intra-procedural block, so the reachable-minus-seed set is + * empty (all intra reach is within the seed); a statement seed leaves the + * other dependent statements reachable. `BasicBlock.startLine` is 1-based and + * matches the source line, so no `+1` offset applies here (unlike the symbol + * span, where the 0-based symbol bounds are shifted). Bounded to the symbol's + * own span when known, so a line shared with a sibling symbol can't leak. + */ +function blockAnchorForStatement( + sym: { filePath: string; startLine?: number; endLine?: number }, + line: number, +): { anchorClause: string; queryParams: Record } { + const idPrefix = `BasicBlock:${sym.filePath}:`; + if ( + typeof sym.startLine === 'number' && + typeof sym.endLine === 'number' && + sym.endLine >= sym.startLine + ) { + return { + anchorClause: + 'a.id STARTS WITH $idPrefix AND a.startLine = $line AND a.startLine >= $symStart AND a.startLine <= $symEnd', + queryParams: { idPrefix, line, symStart: sym.startLine + 1, symEnd: sym.endLine + 1 }, + }; + } + return { + anchorClause: 'a.id STARTS WITH $idPrefix AND a.startLine = $line', + queryParams: { idPrefix, line }, + }; +} + +export interface RunPdgImpactDeps { + repo: { lbugPath: string }; + sym: { id: string; name: string; filePath: string; startLine?: number; endLine?: number }; + symType: string; + direction: 'upstream' | 'downstream'; + maxDepth: number; + limit: number; + /** Statement anchor (1-based source line). */ + line?: number; + executeParameterized: typeof executeParameterized; +} + +export async function runImpactPDG(deps: RunPdgImpactDeps): Promise { + const { repo, sym, direction, maxDepth, line, executeParameterized: exec } = deps; + // `line` present ⇒ statement-anchored slice (the useful mode); absent ⇒ + // whole-symbol seed (intra-procedural reach collapses to empty for a + // function — kept for back-compat, with a note steering the caller to `line`). + const statementMode = typeof line === 'number' && Number.isInteger(line) && line >= 1; + // `target` carries the call-graph-compatible shape (id/name/type/filePath) so + // `collectImpactSymbolUids` keys on it identically to a callgraph result. + const target = { + id: sym.id, + name: sym.name, + type: deps.symType || 'Function', + filePath: sym.filePath, + }; + + // Validate the per-step LIMIT as a positive integer (KTD11 — interpolated, + // so it must be sanitised, never user-string-passed). A non-integer / out-of + // range value (NaN, 1.5, negative, huge) is CLAMPED to the bounded default + // rather than rejected: impact's `limit` is a soft page hint, and a clamp + // keeps the safety tool producing a (flagged-bounded) radius instead of a + // hard error. The clamp ceiling matches `pdg_query`'s validated max. + const rawLimit = deps.limit; + const stepLimit = + Number.isInteger(rawLimit) && rawLimit >= 1 && rawLimit <= PDG_QUERY_MAX_LIMIT + ? rawLimit + : PDG_QUERY_DEFAULT_LIMIT; + // Depth: clamp to the documented impact server max. The BFS issues one DB + // query per depth level, so direct callTool callers must not bypass the + // schema's maxDepth cap. + const depthBudget = + Number.isInteger(maxDepth) && maxDepth >= 1 ? Math.min(maxDepth, IMPACT_MAX_DEPTH) : 3; + + // ── Seed: anchor the target's BasicBlocks from the ALREADY-RESOLVED symbol ─ + // `_impactImpl` already resolved `sym` to a confident single match honoring + // the caller's target_uid/file_path/kind hints. Re-resolving by the bare + // `sym.name` here would RE-AMBIGUATE a disambiguated name (returning the + // "ambiguous" early payload instead of the PDG result) or anchor the seed on + // a DIFFERENT same-name symbol in another file (wrong-symbol blast radius). + // So build the seed anchor DIRECTLY from the resolved symbol's + // [startLine+1, endLine+1] window — the same window `resolveBlockAnchor`'s + // symbol branch produces, without re-running `resolveSymbolCandidates`. + const { anchorClause, queryParams } = statementMode + ? blockAnchorForStatement(sym, line as number) + : blockAnchorForResolvedSymbol(sym); + + const probeLimit = stepLimit + 1; + const rawSeedRows = await exec( + repo.lbugPath, + `MATCH (a:BasicBlock) WHERE ${anchorClause} RETURN a.id AS id LIMIT ${probeLimit}`, + queryParams, + ); + const seedRows = rawSeedRows.slice(0, stepLimit); + const seedBlocks: string[] = seedRows + .map((r: any) => String(r.id ?? r[0] ?? '')) + .filter((id: string) => id.length > 0); + // FIX 7: the seed query probes one row past `stepLimit`, then processes at + // most `stepLimit` rows like every BFS step. A function with more seed blocks + // than `stepLimit` would silently under-seed (and thus under-report) — flag + // it so the result carries the same truncation + // signal the BFS steps do, never a silent partial seed. + const seedTruncated = rawSeedRows.length > stepLimit; + + // ── KTD6 no-body contract: distinguish "no PDG body" from "no dependence" ── + // A symbol that resolves but produces ZERO anchored blocks has no CFG body + // (interface / type alias / abstract / ambient / one-line const). A bare + // impactedCount:0 / risk:'LOW' would read as "safe to refactor" — the exact + // false-safe `impact` exists to prevent (#2129/#1858). Surface an explicit + // note + a non-LOW epistemic marker, never a silent confident zero. + if (seedBlocks.length === 0) { + return { + mode: 'pdg', + target, + direction, + ...(statementMode ? { criterionLine: line } : {}), + reachableBlocks: [], + blockCount: 0, + affectedStatements: [], + affectedStatementCount: 0, + truncated: false, + depthReached: 0, + // statementMode: the requested line has no statement block inside the + // symbol (blank line, comment, outside the body, or a line the CFG did + // not materialise). Distinct from "no PDG body". + epistemic: statementMode ? 'pdg-no-block-at-line' : 'no-pdg-body', + note: statementMode + ? `No PDG statement block starts at line ${line} within '${sym.name}' ` + + `(${sym.filePath}). The line may be blank, a comment, a brace, or outside ` + + `the symbol's body. Pass a line that begins an executable statement.` + : `'${sym.name}' has no PDG body — no BasicBlocks / control- or data-dependence ` + + `edges exist for this symbol (e.g. an interface, type alias, abstract/ambient ` + + `member, or a one-line declaration with no CFG). This is NOT a confident ` + + `"no impact": the intra-procedural PDG mode cannot model this symbol kind. ` + + `Pass line: to slice from a statement, or use mode:'callgraph' for the ` + + `inter-procedural blast radius.`, + impactedCount: 0, + risk: 'UNKNOWN', + // KTD8 parity fields so a consumer iterating byDepth / reading the + // depth counts on a no-body result still finds a well-formed (empty) + // shape rather than `undefined` (which would render as "isolated"). + ...emptyPdgParityFields(), + unresolvedBlockCount: 0, + ambiguousProjectionCount: 0, + }; + } + + // ── Bounded direction-aware BFS over CDG + REACHING_DEF (KTD4, KTD11) ────── + // Seed blocks are NOT counted as reachable (they ARE the target); the + // reachable set is everything the BFS discovers from them. Visited tracks + // BOTH seeds and discovered blocks so a cycle never re-expands. + const visited = new Set(seedBlocks); + const reachable = new Set(); + let frontier = [...seedBlocks]; + let depthReached = 0; + // `truncatedByDepth`: the BFS still had a non-empty frontier when the depth + // budget ran out (more reachable blocks exist past `maxDepth`). + // `truncatedByLimit`: a single step's neighbour query hit the one-past + // LIMIT probe, so that step's expansion is a lower bound. The SEED query is + // one-past-probed too, so `seedTruncated` seeds this flag — a partial seed is + // a lower-bound expansion just like a partial step. Either flags `truncated`. + let truncatedByDepth = false; + let truncatedByLimit = seedTruncated; + + // The endpoint the frontier is matched on, and the endpoint collected, flip + // by direction — but the SAME sense applies to BOTH edge types (KTD4). + // downstream: frontier = source `a`, collect target `b` (forward) + // upstream: frontier = target `b`, collect source `a` (reverse) + const matchEndpoint = direction === 'downstream' ? 'a' : 'b'; + const collectEndpoint = direction === 'downstream' ? 'b' : 'a'; + + for (let depth = 0; depth < depthBudget; depth++) { + if (frontier.length === 0) break; + // Anchored on exact frontier ids (bound as a param — KTD11). The edge-type + // discriminator is a hardcoded literal list (never user input). `LIMIT` is + // the validated integer `probeLimit` (one row past the processed page). + const rawRows = await exec( + repo.lbugPath, + `MATCH (a:BasicBlock)-[r:CodeRelation]->(b:BasicBlock) + WHERE r.type IN ['CDG', 'REACHING_DEF'] AND ${matchEndpoint}.id IN $frontier + RETURN DISTINCT ${collectEndpoint}.id AS id + LIMIT ${probeLimit}`, + { frontier }, + ); + const rows = rawRows.slice(0, stepLimit); + depthReached = depth + 1; + if (rawRows.length > stepLimit) truncatedByLimit = true; + + const next: string[] = []; + for (const r of rows) { + const id = String((r as any).id ?? (r as any)[0] ?? ''); + if (!id || visited.has(id)) continue; + visited.add(id); + reachable.add(id); + next.push(id); + } + frontier = next; + } + // Frontier still non-empty after exhausting the depth budget ⇒ more blocks + // are reachable beyond `maxDepth` (depth truncation, distinct from natural + // completion where the frontier drains to empty inside the loop). + if (frontier.length > 0) truncatedByDepth = true; + + const reachableBlocks = [...reachable].sort(); + const truncated = truncatedByDepth || truncatedByLimit; + const truncatedBy: 'depth' | 'limit' | undefined = truncatedByDepth + ? 'depth' + : truncatedByLimit + ? 'limit' + : undefined; + const truncatedByReasons: readonly ('depth' | 'limit')[] | undefined = + truncatedByDepth && truncatedByLimit ? (['depth', 'limit'] as const) : undefined; + + // ── Resolve the reachable blocks to source statements (line + text) ──────── + // This is the useful output of statement mode: the dependent statements the + // change at `line` reaches. Fetched once for the whole reachable set; sorted + // by line. Failure surfaces (no `.catch` swallow) rather than masquerading + // as "no affected statements". + const affectedStatements = await pdgStatementsForBlocks(repo.lbugPath, reachableBlocks, exec); + + // ── Has a PDG body but no intra-procedural dependence reachability ───────── + // Distinct from "no PDG body": the function exists and has blocks, but no + // CDG/REACHING_DEF edge leaves the target's blocks in this direction. For a + // WHOLE-SYMBOL seed this is the expected (and uninformative) result — every + // intra-procedural block is already a seed — so the note steers to `line`. + // Still not a confident zero — explicit note + UNKNOWN (KTD6/KTD8). + if (reachableBlocks.length === 0) { + return { + mode: 'pdg', + target, + direction, + ...(statementMode ? { criterionLine: line } : {}), + impactedCount: 0, + risk: 'UNKNOWN', + epistemic: 'pdg-intra-procedural', + note: statementMode + ? `No statement in '${sym.name}' is ${direction}-dependent on line ${line} ` + + `(no CDG/REACHING_DEF reachability from that statement). The line may have no ` + + `dependents in this direction.` + : `'${sym.name}' has a PDG body but a WHOLE-SYMBOL ${direction} slice is empty: ` + + `intra-procedural dependence stays inside the function, so every reachable block ` + + `is already part of the seed. Pass line: to slice from a specific statement ` + + `(what depends on the code at that line), or use mode:'callgraph' for the ` + + `inter-procedural blast radius.`, + reachableBlocks: [] as string[], + blockCount: 0, + affectedStatements: [], + affectedStatementCount: 0, + depthReached, + unresolvedBlockCount: 0, + ambiguousProjectionCount: 0, + ...(truncated ? { truncated: true } : {}), + ...(truncatedBy ? { truncatedBy } : {}), + ...(truncatedByReasons ? { truncatedByReasons } : {}), + ...emptyPdgParityFields(), + }; + } + + // ── U4: project reachable blocks → owning symbols, assemble parity result ── + const projection = await projectBlocksToSymbols({ + lbugPath: repo.lbugPath, + blockIds: reachableBlocks, + executeParameterized: exec, + }); + + return assemblePdgImpactResult({ + target: { + id: sym.id, + name: sym.name, + type: deps.symType || 'Function', + filePath: sym.filePath, + }, + direction, + reachableBlocks, + affectedStatements, + criterionLine: statementMode ? (line as number) : undefined, + projection, + depthReached, + truncated, + truncatedBy, + truncatedByReasons, + }); +} diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index bd7c621dd..7a79562bb 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -77,6 +77,10 @@ export const EXPLAIN_MAX_LIMIT = 200; export const PDG_QUERY_DEFAULT_LIMIT = 50; export const PDG_QUERY_MAX_LIMIT = 200; +// Shared impact traversal depth cap. The MCP schema advertises this bound; +// PDG direct backend callers also enforce it before running traversal. +export const IMPACT_MAX_DEPTH = 32; + export const GITNEXUS_TOOLS: ToolDefinition[] = [ { name: 'list_repos', @@ -413,11 +417,13 @@ MODE (opt-in): "callgraph" (default) walks symbol→symbol edges (CALLS/IMPORTS/ STATEMENT-ANCHORED PDG SLICE: with mode:'pdg', pass "line" (1-based source line within the target symbol) to seed the dependence slice on the statement at that line and return what depends on it — the dependent statements (line + text), not the whole-symbol set. Without "line", a whole-symbol pdg slice is structurally empty (intra-procedural reach stays inside the function), so "line" is what makes pdg mode useful. +PDG OUTPUT CONTRACT: successful PDG slices include mode:'pdg', a full target envelope (id/name/type/filePath), affectedStatements, affectedStatementCount, byDepth/byDepthCounts parity fields, risk:'UNKNOWN', and an intra-procedural note. Degraded PDG results (no-layer, sub-layer-missing, unknown) keep mode:'pdg', target metadata when the target resolves, risk:'UNKNOWN', note/remediation, and empty byDepth parity fields — never a false-safe zero. If depth and limit both bound the slice, truncatedByReasons reports both causes while truncatedBy remains scalar. + 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. Output includes: -- risk: LOW / MEDIUM / HIGH / CRITICAL +- risk: LOW / MEDIUM / HIGH / CRITICAL / UNKNOWN - summary: direct callers, processes affected, modules affected - affected_processes: which execution flows break and at which step - affected_modules: which functional areas are hit (direct vs indirect) @@ -459,7 +465,7 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep 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.", + "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`. PDG success returns affectedStatements, while degraded/no-layer results return a structured UNKNOWN-risk note with target metadata when resolved. The pdg mode is incompatible with relationTypes/crossDepth/minConfidence and with @group targets — each is rejected, not silently ignored.", }, line: { type: 'integer', @@ -481,7 +487,7 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep description: 'Max relationship depth (default: 3, server clamps to 1–32)', default: 3, minimum: 1, - maximum: 32, + maximum: IMPACT_MAX_DEPTH, }, crossDepth: { type: 'number', diff --git a/gitnexus/test/integration/impact-pdg-degradation.test.ts b/gitnexus/test/integration/impact-pdg-degradation.test.ts index 3d9f46d97..a3e211855 100644 --- a/gitnexus/test/integration/impact-pdg-degradation.test.ts +++ b/gitnexus/test/integration/impact-pdg-degradation.test.ts @@ -4,28 +4,25 @@ * End-to-end against a REAL LadybugDB, through the full `callTool('impact', …)` * dispatch. Exercises the four-state PDG-layer presence/degradation check * (`pdgLayerStatus`) wired into `_impactImpl`'s PDG branch — the check that - * fires BEFORE symbol resolution / traversal so a missing or partial `--pdg` - * layer returns a distinct guidance note instead of a confusing empty blast - * radius (or the U2-era `_runImpactPDG` "not yet implemented" stub error). + * fires after symbol resolution but before traversal so a missing or partial + * `--pdg` layer returns a distinct target-aware guidance note instead of a + * confusing empty blast radius. * * The four states (KTD7) are driven by what the (mocked) `loadMeta` returns — * matching the seeded-DB reality that there is no on-disk `meta.json`: * - no-layer : meta readable, no `pdg` stamp → run analyze --pdg * - sub-layer-missing : exactly one cap stamped (CDG xor RD) → names the missing one - * - ready : both caps stamped → falls through to the stub + * - ready : both caps stamped → falls through to traversal * - unknown : meta unreadable (null) → inconclusive, via 1 LIMIT 1 probe * - * The `_runImpactPDG` traversal is still a stub in U2, so the `ready` case - * asserts the layer check let it THROUGH (the stub's "not yet implemented" - * sentinel), proving the check is ordered before the stub for the degraded - * states and falls through only when the layer is complete. + * The `ready` case asserts the layer check lets the call THROUGH to the real + * traversal, while degraded states return before `_runImpactPDG`. */ import { describe, it, expect, beforeAll, beforeEach, vi } from 'vitest'; import type { RepoMeta } from '../../src/storage/repo-manager.js'; import { LocalBackend } from '../../src/mcp/local/local-backend.js'; import { listRegisteredRepos, loadMeta } from '../../src/storage/repo-manager.js'; import { withTestLbugDB } from '../helpers/test-indexed-db.js'; -import * as poolAdapter from '../../src/core/lbug/pool-adapter.js'; vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { const actual = await importOriginal(); @@ -54,6 +51,18 @@ const SEED_EDGE = `MATCH (a:BasicBlock {id: 'BasicBlock:src/hot.ts:1:0:0'}), (b: const META = (pdg?: RepoMeta['pdg']): RepoMeta => ({ pdg }) as unknown as RepoMeta; +function expectEmptyPdgParity(result: any): void { + expect(result.mode).toBe('pdg'); + expect(result.direction).toBe('downstream'); + expect(result.impactedCount).toBe(0); + expect(result.risk).toBe('UNKNOWN'); + expect(result.byDepth).toEqual({}); + expect(result.byDepthCounts).toEqual({ 1: 0 }); + expect(result.summary).toEqual({ direct: 0, processes_affected: 0, modules_affected: 0 }); + expect(result.affected_processes).toEqual([]); + expect(result.affected_modules).toEqual([]); +} + withTestLbugDB( 'impact-pdg-degradation', (handle) => { @@ -72,34 +81,30 @@ withTestLbugDB( }); describe('no-layer (meta readable, no pdg stamp)', () => { - it('returns the definitive "run analyze --pdg" note — and does NOT scan the DB', async () => { + it('returns the definitive target-aware "run analyze --pdg" note', async () => { // Readable meta with no `pdg` key ⇒ the layer was never recorded. vi.mocked(loadMeta).mockResolvedValueOnce(META(undefined)); - const spy = vi.spyOn(poolAdapter, 'executeParameterized'); - spy.mockClear(); - const result = await backend.callTool('impact', { target: 'hot', direction: 'downstream', mode: 'pdg', }); - // Definitive, meta-derived: no DB probe ran, so executeParameterized was - // never called between the spy clear and here (the no-layer branch - // returns before the probe AND before resolveSymbolCandidates). - expect(spy).not.toHaveBeenCalled(); - spy.mockRestore(); - expect(result.mode).toBe('pdg'); expect(result.pdgLayer).toBe('no-layer'); + expect(result.target).toEqual({ + id: 'func:hot', + name: 'hot', + type: 'Function', + filePath: 'src/hot.ts', + }); expect(result.note).toMatch(/no PDG layer/i); expect(result.note).toContain('--pdg'); - // Not the stub, not a status-unknown note, not a confident LOW. + // Not a status-unknown note, not a confident LOW. expect(result.error).toBeUndefined(); expect(result.note).not.toMatch(/status unknown/i); expect(result.note).not.toMatch(/not yet implemented/i); - expect(result.risk).toBe('UNKNOWN'); - expect(result.impactedCount).toBe(0); + expectEmptyPdgParity(result); }); }); @@ -112,11 +117,13 @@ withTestLbugDB( mode: 'pdg', }); expect(result.pdgLayer).toBe('sub-layer-missing'); + expect(result.target.filePath).toBe('src/hot.ts'); + expect(result.target.type).toBe('Function'); expect(result.missingSubLayer).toBe('REACHING_DEF'); expect(result.note).toMatch(/REACHING_DEF/); - // Partial layer must NOT be reported as complete (not the stub, no LOW). + // Partial layer must NOT be reported as complete (no LOW). expect(result.note).not.toMatch(/not yet implemented/i); - expect(result.risk).toBe('UNKNOWN'); + expectEmptyPdgParity(result); }); it('RD present, CDG absent → names CDG as missing', async () => { @@ -132,7 +139,7 @@ withTestLbugDB( expect(result.missingSubLayer).toBe('CDG'); expect(result.note).toMatch(/\bCDG\b/); expect(result.note).not.toMatch(/not yet implemented/i); - expect(result.risk).toBe('UNKNOWN'); + expectEmptyPdgParity(result); }); }); @@ -149,12 +156,12 @@ withTestLbugDB( // The layer is complete, so the check did NOT short-circuit: there is no // degradation note / pdgLayer marker — the call reached the traversal. expect(result.pdgLayer).toBeUndefined(); - // U3 landed: the stub "not yet implemented" error is gone. `hot` has a + // `hot` has a // PDG body (blocks B0→B1) but the only dependent (B1) is itself a seed // block of the symbol, so the intra-procedural downstream reachable set // is empty — and that is signalled as a real traversal result with the // distinct "has a body but no dependence" note, NOT the no-body / - // degradation path, NOT the old stub error. The load-bearing U2 fact — + // degradation path. The load-bearing U2 fact — // `ready` does NOT return a degradation note — still holds. expect(result.mode).toBe('pdg'); expect(result.error).toBeUndefined(); @@ -177,13 +184,14 @@ withTestLbugDB( mode: 'pdg', }); expect(result.pdgLayer).toBe('unknown'); + expect(result.target.filePath).toBe('src/hot.ts'); + expect(result.target.type).toBe('Function'); expect(result.note).toMatch(/status unknown/i); expect(result.note).toContain('--pdg'); // Inconclusive ≠ definitive no-layer wording. expect(result.note).not.toMatch(/no PDG layer/i); expect(result.note).not.toMatch(/not yet implemented/i); - expect(result.risk).toBe('UNKNOWN'); - expect(result.impactedCount).toBe(0); + expectEmptyPdgParity(result); }); }); diff --git a/gitnexus/test/integration/impact-pdg-e2e.test.ts b/gitnexus/test/integration/impact-pdg-e2e.test.ts new file mode 100644 index 000000000..abd6cd706 --- /dev/null +++ b/gitnexus/test/integration/impact-pdg-e2e.test.ts @@ -0,0 +1,272 @@ +/** + * Integration Tests: real emitter -> persisted PDG rows -> impact(mode:'pdg'). + * + * Seeded PDG traversal tests lock the graph algorithm. This suite locks the + * producer/consumer contract that seeded rows cannot cover: the real analysis + * pipeline must emit BasicBlock ids, source-line metadata, and REACHING_DEF/CDG + * rows in the exact shape consumed by LocalBackend's statement-anchored PDG + * impact traversal and affectedStatements projection. + */ +import { it, expect, beforeAll, vi } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import type { RepoMeta } from '../../src/storage/repo-manager.js'; +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; +import { executeParameterized } from '../../src/core/lbug/pool-adapter.js'; +import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; +import { withTestLbugDB, type IndexedDBHandle } from '../helpers/test-indexed-db.js'; + +const metaByStoragePath = vi.hoisted(() => new Map()); + +vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + listRegisteredRepos: vi.fn().mockResolvedValue([]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), + loadMeta: vi.fn().mockImplementation(async (storagePath: string) => { + return metaByStoragePath.get(storagePath) ?? null; + }), + }; +}); + +const FIXTURE = path.join(__dirname, 'cfg', 'fixtures', 'pdg-repo'); +const READY_PDG_META = { + pdg: { maxCdgEdgesPerFunction: 0, maxReachingDefEdgesPerFunction: 0 }, +} as unknown as RepoMeta; + +async function persistFixtureGraph( + pdg: boolean, +): Promise<{ pdgEdges: number; reachingDefEdges: number; cdgEdges: number }> { + const repoDir = fs.mkdtempSync( + path.join(os.tmpdir(), pdg ? 'gn-impact-pdg-' : 'gn-impact-nopdg-'), + ); + try { + fs.cpSync(FIXTURE, repoDir, { recursive: true }); + const pipelineResult = await runPipelineFromRepo(repoDir, () => {}, pdg ? { pdg: true } : {}); + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + const nodes: Array<{ label: 'BasicBlock' | 'Function'; props: Record }> = []; + pipelineResult.graph.forEachNode((n) => { + if (n.label === 'BasicBlock') { + nodes.push({ + label: 'BasicBlock', + props: { + id: n.id, + filePath: n.properties.filePath ?? '', + startLine: n.properties.startLine ?? 0, + endLine: n.properties.endLine ?? 0, + text: n.properties.text ?? '', + }, + }); + } else if (n.label === 'Function') { + nodes.push({ + label: 'Function', + props: { + id: n.id, + name: n.properties.name ?? '', + filePath: n.properties.filePath ?? '', + startLine: n.properties.startLine ?? 0, + endLine: n.properties.endLine ?? 0, + }, + }); + } + }); + + for (const node of nodes) { + const assignments = Object.keys(node.props) + .map((k) => `${k}: $${k}`) + .join(', '); + await adapter.executePrepared( + `CREATE (n:${node.label} {${assignments}})`, + node.props as Record, + ); + } + + let pdgEdges = 0; + let reachingDefEdges = 0; + let cdgEdges = 0; + for (const rel of pipelineResult.graph.iterRelationships()) { + if (rel.type !== 'CDG' && rel.type !== 'REACHING_DEF') continue; + await adapter.executePrepared( + `MATCH (a:BasicBlock {id: $src}), (b:BasicBlock {id: $dst}) + CREATE (a)-[:CodeRelation {type: '${rel.type}', confidence: $confidence, reason: $reason, step: 0}]->(b)`, + { + src: rel.sourceId, + dst: rel.targetId, + confidence: rel.confidence ?? 1.0, + reason: rel.reason ?? '', + }, + ); + pdgEdges++; + if (rel.type === 'REACHING_DEF') reachingDefEdges++; + if (rel.type === 'CDG') cdgEdges++; + } + + return { pdgEdges, reachingDefEdges, cdgEdges }; + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } +} + +function registerSingleRepo(handle: IndexedDBHandle, name: string, repoPath: string): void { + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name, + path: repoPath, + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'impact-pdg-e2e', + stats: { files: 4, nodes: 4, communities: 0, processes: 0 }, + }, + ]); +} + +withTestLbugDB( + 'impact-pdg-e2e', + (handle) => { + let backend: LocalBackend; + let counts: { pdgEdges: number; reachingDefEdges: number; cdgEdges: number }; + + beforeAll(() => { + const ext = handle as typeof handle & { + _backend?: LocalBackend; + _counts?: { pdgEdges: number; reachingDefEdges: number; cdgEdges: number }; + }; + if (!ext._backend || !ext._counts) throw new Error('PDG e2e setup did not finish'); + backend = ext._backend; + counts = ext._counts; + }); + + it('uses real emitted REACHING_DEF and CDG rows to return statement-level PDG impact', async () => { + expect(counts.pdgEdges).toBeGreaterThan(0); + expect(counts.reachingDefEdges).toBeGreaterThan(0); + expect(counts.cdgEdges).toBeGreaterThan(0); + + const result = await backend.callTool('impact', { + target: 'loopFlow', + direction: 'downstream', + mode: 'pdg', + line: 19, + maxDepth: 10, + limit: 50, + }); + + expect(result.error).toBeUndefined(); + expect(result.mode).toBe('pdg'); + expect(result.target.name).toBe('loopFlow'); + expect(result.target.filePath).toBe('guards.ts'); + expect(result.criterionLine).toBe(19); + + const persistedRd = await executeParameterized( + handle.dbPath, + `MATCH (:BasicBlock)-[r:CodeRelation]->(:BasicBlock) + WHERE r.type = 'REACHING_DEF' + RETURN r.type AS type + LIMIT 1`, + {}, + ); + expect(persistedRd.length).toBeGreaterThan(0); + const persistedCdg = await executeParameterized( + handle.dbPath, + `MATCH (:BasicBlock)-[r:CodeRelation]->(:BasicBlock) + WHERE r.type = 'CDG' + RETURN r.type AS type + LIMIT 1`, + {}, + ); + expect(persistedCdg.length).toBeGreaterThan(0); + expect(Array.isArray(result.affectedStatements)).toBe(true); + + const lines = (result.affectedStatements as any[]) + .map((statement) => statement.line) + .sort((a, b) => a - b); + expect(lines).toEqual(expect.arrayContaining([21, 23])); + expect(lines).not.toContain(19); + expect(result.affectedStatementCount).toBe(result.affectedStatements.length); + + const controlResult = await backend.callTool('impact', { + target: 'guarded', + direction: 'downstream', + mode: 'pdg', + line: 9, + maxDepth: 10, + limit: 50, + }); + expect(controlResult.error).toBeUndefined(); + expect(controlResult.mode).toBe('pdg'); + const controlLines = (controlResult.affectedStatements as any[]) + .map((statement) => statement.line) + .sort((a, b) => a - b); + expect(controlLines).toContain(10); + expect(controlLines).not.toContain(9); + }); + }, + { + poolAdapter: true, + timeout: 180_000, + afterSetup: async (handle) => { + metaByStoragePath.set(handle.tmpHandle.dbPath, READY_PDG_META); + const counts = await persistFixtureGraph(true); + if (counts.pdgEdges === 0 || counts.reachingDefEdges === 0 || counts.cdgEdges === 0) { + throw new Error('fixture produced no persisted PDG dependence edges'); + } + registerSingleRepo(handle, 'impact-pdg-e2e', '/impact/pdg/repo'); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + (handle as any)._counts = counts; + }, + }, +); + +withTestLbugDB( + 'impact-pdg-e2e-nopdg', + (handle) => { + let backend: LocalBackend; + + beforeAll(() => { + const ext = handle as typeof handle & { _backend?: LocalBackend }; + if (!ext._backend) throw new Error('no-PDG e2e setup did not finish'); + backend = ext._backend; + }); + + it('returns the no-layer envelope for the same fixture indexed without PDG', async () => { + const result = await backend.callTool('impact', { + target: 'loopFlow', + direction: 'downstream', + mode: 'pdg', + line: 19, + }); + + expect(result.error).toBeUndefined(); + expect(result.mode).toBe('pdg'); + expect(result.pdgLayer).toBe('no-layer'); + expect(result.note).toContain('--pdg'); + expect(result.target).toEqual({ + id: expect.any(String), + name: 'loopFlow', + type: 'Function', + filePath: 'guards.ts', + }); + expect(result.impactedCount).toBe(0); + expect(result.risk).toBe('UNKNOWN'); + expect(result.byDepthCounts).toEqual({ 1: 0 }); + }); + }, + { + poolAdapter: true, + timeout: 180_000, + afterSetup: async (handle) => { + metaByStoragePath.set(handle.tmpHandle.dbPath, {} as RepoMeta); + await persistFixtureGraph(false); + registerSingleRepo(handle, 'impact-pdg-e2e-nopdg', '/impact/no-pdg/repo'); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +); diff --git a/gitnexus/test/integration/impact-pdg-shape.test.ts b/gitnexus/test/integration/impact-pdg-shape.test.ts index dc925b944..8384be619 100644 --- a/gitnexus/test/integration/impact-pdg-shape.test.ts +++ b/gitnexus/test/integration/impact-pdg-shape.test.ts @@ -25,10 +25,11 @@ * - `ctl` fn at [29,31] ⇒ blocks K1@31, K2@32 (CDG dependents of S). * - `dupA` AND `dupB`, BOTH at 0-based [40,42] (SAME (filePath,startLine)) — * a CDG-reachable block T@41 maps to BOTH (ambiguous-projection). - * - a free/top-level block U@99 owned by NO Function (downstream of K2) — the + * - `FlowThing.constructor` at 0-based [55,55] owns CT@56 (constructor projection). + * - a free/top-level block U@99 owned by NO symbol (downstream of K2) — the * `unresolved` shadow path. * - * Downstream from S: RD → {D1,D2}; CDG → {K1,K2} → T(@41) → U(@99 top-level). + * Downstream from S: RD → {D1,D2,CT}; CDG → {K1,K2} → T(@41) → U(@99 top-level). * * `loadMeta` is mocked to stamp BOTH caps so `pdgLayerStatus` returns `ready`. */ @@ -63,6 +64,7 @@ const D2 = `BasicBlock:${F}:20:0:1`; // down@[19,21] const K1 = `BasicBlock:${F}:30:0:0`; // ctl@[29,31] const K2 = `BasicBlock:${F}:30:0:1`; // ctl@[29,31] const T = `BasicBlock:${F}:41:0:0`; // dupA AND dupB BOTH @[40,42] → ambiguous +const CT = `BasicBlock:${F}:56:0:0`; // Constructor FlowThing.constructor@[55,55] const U = `BasicBlock:${F}:99:0:0`; // top-level / no owning symbol → unresolved withTestLbugDB( @@ -103,10 +105,20 @@ withTestLbugDB( expect(down.filePath).toBe(F); }); + it('maps a reachable constructor block to its owning Constructor symbol', async () => { + const result = await downstream(); + const items = Object.values(result.byDepth as Record).flat(); + const ctor = items.find((i: any) => i.id === 'ctor:FlowThing'); + expect(ctor).toBeDefined(); + expect(ctor.name).toBe('FlowThing.constructor'); + expect(ctor.type).toBe('Constructor'); + expect(ctor.filePath).toBe(F); + }); + it('a reachable block owning NO symbol is reported as unresolved, never dropped (R9 shadow path)', async () => { const result = await downstream(); const items = Object.values(result.byDepth as Record).flat(); - // U@99 has no owning Function/Method → an explicit unresolved entry. + // U@99 has no owning Function/Method/Constructor → an explicit unresolved entry. const unresolved = items.filter((i: any) => i.id === null || i.type === 'unresolved'); expect(unresolved.length).toBeGreaterThanOrEqual(1); expect(unresolved[0].filePath).toBe(F); @@ -186,9 +198,9 @@ withTestLbugDB( it("risk is the existing 'UNKNOWN' sentinel, not a minted PDG label", async () => { const result = await downstream(); expect(result.risk).toBe('UNKNOWN'); - // impactedCount = distinct owning SYMBOLS (down, ctl, dupA, dupB) — the - // meaningful unit; unresolved blocks do not inflate it. - expect(result.impactedCount).toBe(4); + // impactedCount = distinct owning SYMBOLS (down, ctl, dupA, dupB, ctor) — + // the meaningful unit; unresolved blocks do not inflate it. + expect(result.impactedCount).toBe(5); // blockCount is the raw reachable-block count, retained separately. expect(result.blockCount).toBeGreaterThanOrEqual(result.impactedCount); }); @@ -257,6 +269,7 @@ withTestLbugDB( expect(uids).toContain('func:target'); // from target.id expect(uids).toContain('func:down'); expect(uids).toContain('func:ctl'); + expect(uids).toContain('ctor:FlowThing'); expect(uids).not.toContain('null'); expect(uids).not.toContain(''); expect(targetFilePath).toBe(F); @@ -425,12 +438,19 @@ withTestLbugDB( name: string, startLine: number, endLine: number, - type: 'Function' | 'Interface' = 'Function', - ) => - adapter.executePrepared( + type: 'Function' | 'Interface' | 'Constructor' = 'Function', + ) => { + if (type === 'Constructor') { + return adapter.executePrepared( + `CREATE (n:Constructor {id: $id, name: $name, filePath: $filePath, startLine: $startLine, endLine: $endLine, content: 'x', description: 'shape fixture'})`, + { id, name, filePath: F, startLine, endLine }, + ); + } + return adapter.executePrepared( `CREATE (n:${type} {id: $id, name: $name, filePath: $filePath, startLine: $startLine, endLine: $endLine, isExported: true, content: 'x', description: 'shape fixture'})`, { id, name, filePath: F, startLine, endLine }, ); + }; const block = (id: string, startLine: number, text: string) => adapter.executePrepared( `CREATE (b:BasicBlock {id: $id, filePath: $filePath, startLine: $startLine, endLine: $startLine, text: $text})`, @@ -453,6 +473,8 @@ withTestLbugDB( await fn('func:dupB', 'dupTarget', 40, 42); // No-body interface (no blocks). await fn('func:IShape', 'IShape', 50, 52, 'Interface'); + // Constructor owner projection. + await fn('ctor:FlowThing', 'FlowThing.constructor', 55, 55, 'Constructor'); // Blocks. await block(S, 11, 'const x = compute();'); @@ -462,12 +484,14 @@ withTestLbugDB( await block(K1, 31, 'doA();'); await block(K2, 32, 'doB();'); await block(T, 41, 'dispatch();'); // owned by BOTH dupA & dupB + await block(CT, 56, 'this.value = x;'); // owned by Constructor await block(U, 99, 'top-level-side-effect();'); // owned by NO symbol // RD chain (def→use): P → S → D1 → D2 await edge('REACHING_DEF', P, S, 'seed'); await edge('REACHING_DEF', S, D1, 'x'); await edge('REACHING_DEF', D1, D2, 'x'); + await edge('REACHING_DEF', S, CT, 'ctor'); // CDG chain: P(controller) → S → K1 → K2 → T(@dup line) → U(top-level) await edge('CDG', P, S, 'T'); await edge('CDG', S, K1, 'T'); @@ -572,7 +596,7 @@ withTestLbugDB( storagePath: handle.tmpHandle.dbPath, indexedAt: new Date().toISOString(), lastCommit: 'shape123', - stats: { files: 1, nodes: 16, communities: 0, processes: 0 }, + stats: { files: 1, nodes: 18, communities: 0, processes: 0 }, }, ]); const backend = new LocalBackend(); diff --git a/gitnexus/test/integration/impact-pdg-traversal.test.ts b/gitnexus/test/integration/impact-pdg-traversal.test.ts index 8d351b7f3..cec8a6744 100644 --- a/gitnexus/test/integration/impact-pdg-traversal.test.ts +++ b/gitnexus/test/integration/impact-pdg-traversal.test.ts @@ -6,9 +6,9 @@ * bounded BFS over CDG + REACHING_DEF block edges — the correctness keystone of * the feature (the KTD4 direction × edge-type truth table). * - * The intermediate U3 payload exposes the reachable BasicBlock set: - * { mode:'pdg', target, reachableBlocks:[...ids], truncated, depthReached, note? } - * (U4 reshapes this into the consumer-safe impact result; U3 is the traversal.) + * The result exposes the consumer-safe impact shape plus traversal details + * (`reachableBlocks`, `truncated`, `depthReached`) so this suite can pin the + * graph algorithm without bypassing the public `impact` tool contract. * * ── Fixture graph (hand-seeded, no parser; controlled line numbers) ────────── * One file `src/flow.ts`. The TARGET symbol `target` is a one-line function at @@ -274,6 +274,20 @@ withTestLbugDB( expect(result.truncated).toBeFalsy(); }); + it('an exact limit-sized seed/step is not flagged truncated without an extra row', async () => { + const result = await backend.callTool('impact', { + target: 'target', + direction: 'upstream', + mode: 'pdg', + maxDepth: 10, + limit: 1, + }); + expect(reachable(result)).toEqual([P]); + expect(result.truncated).toBeFalsy(); + expect(result.truncatedBy).toBeUndefined(); + expect(result.truncatedByReasons).toBeUndefined(); + }); + it('limit truncation bounds the reachable set and flags truncated', async () => { const result = await backend.callTool('impact', { target: 'target', @@ -288,6 +302,19 @@ withTestLbugDB( expect(result.truncated).toBe(true); }); + it('reports both depth and limit when both bounds truncate the slice', async () => { + const result = await backend.callTool('impact', { + target: 'target', + direction: 'downstream', + mode: 'pdg', + maxDepth: 1, + limit: 1, + }); + expect(result.truncated).toBe(true); + expect(result.truncatedBy).toBe('depth'); + expect(result.truncatedByReasons).toEqual(['depth', 'limit']); + }); + it('rejects or clamps a negative / huge / NaN limit (validated int interpolation)', async () => { for (const limit of [-1, NaN, 1.5]) { const result = await backend.callTool('impact', { diff --git a/gitnexus/test/unit/ai-context.test.ts b/gitnexus/test/unit/ai-context.test.ts index 30a9441c1..f6bdbfbb0 100644 --- a/gitnexus/test/unit/ai-context.test.ts +++ b/gitnexus/test/unit/ai-context.test.ts @@ -155,6 +155,8 @@ describe('generateAIContextFiles', () => { const withPdg = generateGitNexusContent('PdgProject', stats, { hasPdg: true }); expect(withPdg).toContain('pdg_query'); expect(withPdg).toContain('under what condition does X run'); + expect(withPdg).toContain('line: '); + expect(withPdg).toContain('affectedStatements'); // hasPdg omitted (default false) → no pdg_query line; a non-pdg index must // not advertise a tool that only returns a "no PDG layer" note. const withoutPdg = generateGitNexusContent('PlainProject', stats); diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 848bdb11c..9bb64724e 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -51,7 +51,7 @@ vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { // default (so branch-scope resolution, #2106, is unaffected). The // impact-mode block overrides it per-test to stamp a READY PDG layer, so the // U2 layer-presence probe falls THROUGH to the post-check surface (the - // `_runImpactPDG` stub / ambiguous fan-out) those tests assert. The + // `_runImpactPDG` delegate / ambiguous fan-out) those tests assert. The // four-state degradation contract itself is covered in // test/integration/impact-pdg-degradation.test.ts. loadMeta: vi.fn(actual.loadMeta), @@ -1402,15 +1402,15 @@ describe('LocalBackend.callTool', () => { // 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. +// default, pdg routes to the extracted traversal 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. + // dispatch (callgraph BFS or the PDG traversal). The callgraph BFS then issues + // executeQuery for its frontier; the PDG path delegates to runImpactPDG. function resolveSingleTarget() { (executeParameterized as any).mockResolvedValue([ { id: 'func:main', name: 'main', type: 'Function', filePath: 'src/index.ts' }, @@ -1423,7 +1423,7 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); // U2: stamp a READY PDG layer (both caps) so the layer-presence probe in // `_impactImpl` falls THROUGH to the mode-dispatch surface these tests pin - // (the `_runImpactPDG` stub / the ambiguous fan-out under `mode:'pdg'`). + // (the `_runImpactPDG` delegate / the ambiguous fan-out under `mode:'pdg'`). // Degraded-layer behavior is owned by the integration degradation suite. vi.mocked(loadMeta).mockResolvedValue({ pdg: { maxCdgEdgesPerFunction: 0, maxReachingDefEdgesPerFunction: 0 }, @@ -1437,7 +1437,7 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { 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. + // A clean callgraph result carries no mode error and runs the BFS. expect(result.error ?? '').not.toMatch(/Invalid "mode"/); expect(result.error ?? '').not.toMatch(/not yet implemented/); expect(result.target).toBeDefined(); @@ -1607,6 +1607,39 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { } }); + it("unknown target with mode:'pdg' returns the normalized PDG error envelope", async () => { + (executeParameterized as any).mockResolvedValue([]); + const result = await backend.callTool('impact', { + target: 'missingSymbol', + direction: 'upstream', + mode: 'pdg', + }); + expect(result.error).toMatch(/not found/); + expect(result.mode).toBe('pdg'); + expect(result.target).toEqual({ name: 'missingSymbol' }); + expect(result.direction).toBe('upstream'); + expect(result.impactedCount).toBe(0); + expect(result.risk).toBe('UNKNOWN'); + }); + + it("runtime failures with mode:'pdg' return the normalized PDG error envelope", async () => { + const failing = new Error('pdg query failed'); + const implSpy = vi.spyOn(backend as any, '_impactImpl').mockRejectedValueOnce(failing); + const result = await backend.callTool('impact', { + target: 'main', + direction: 'downstream', + mode: 'pdg', + }); + expect(result.error).toBe('pdg query failed'); + expect(result.mode).toBe('pdg'); + expect(result.target).toEqual({ name: 'main' }); + expect(result.direction).toBe('downstream'); + expect(result.impactedCount).toBe(0); + expect(result.risk).toBe('UNKNOWN'); + expect(result.suggestion).toMatch(/context/); + implSpy.mockRestore(); + }); + 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', { @@ -1616,6 +1649,11 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { repo: '@grp', }); expect(result.error).toMatch(/not supported for @group targets/); + expect(result.mode).toBe('pdg'); + expect(result.target).toEqual({ name: 'main' }); + expect(result.direction).toBe('upstream'); + expect(result.impactedCount).toBe(0); + expect(result.risk).toBe('UNKNOWN'); }); it("@group target with mode:'callgraph' still forwards to group impact (unchanged)", async () => { diff --git a/gitnexus/test/unit/cli-impact-pdg-format.test.ts b/gitnexus/test/unit/cli-impact-pdg-format.test.ts index 6104147db..15c7909b9 100644 --- a/gitnexus/test/unit/cli-impact-pdg-format.test.ts +++ b/gitnexus/test/unit/cli-impact-pdg-format.test.ts @@ -17,7 +17,7 @@ import { describe, expect, it } from 'vitest'; import { formatImpactResult } from '../../src/cli/eval-server.js'; // A representative PDG findings result, shaped exactly like -// `assemblePdgImpactResult` (local-backend.ts) emits. +// `assemblePdgImpactResult` (pdg-impact.ts) emits. function pdgFindings(overrides: Record = {}): Record { const items = [ { @@ -68,6 +68,44 @@ function pdgFindings(overrides: Record = {}): Record { + it('renders a PDG ambiguous target without fabricated zero blast-radius counts', () => { + const out = formatImpactResult({ + status: 'ambiguous', + mode: 'pdg', + message: + "Found 2 symbols matching 'login'. Disambiguate with target_uid for a single authoritative PDG result.", + target: { name: 'login' }, + direction: 'upstream', + totalCandidates: 2, + impactedCount: 0, + risk: 'UNKNOWN', + candidates: [ + { + uid: 'func:login:1', + name: 'login', + kind: 'Function', + filePath: 'src/auth.ts', + line: 5, + score: 1, + }, + { + uid: 'func:login:2', + name: 'login', + kind: 'Function', + filePath: 'src/admin/login.ts', + line: 8, + score: 0.91, + }, + ], + }); + + expect(out).toContain('login: AMBIGUOUS'); + expect(out).toContain('PDG impact was not computed'); + expect(out).toContain('func:login:1'); + expect(out).not.toContain('Max blast radius 0'); + expect(out).not.toContain('[0 upstream'); + }); + it('renders findings under PDG-dependent framing, not "depth N"', () => { const out = formatImpactResult(pdgFindings()); @@ -139,6 +177,18 @@ describe('formatImpactResult — PDG (mode:pdg) rendering', () => { expect(out).toContain('deeper PDG impacts may exist'); }); + it('renders multiple truncation causes honestly', () => { + const out = formatImpactResult( + pdgFindings({ + truncated: true, + truncatedBy: 'depth', + truncatedByReasons: ['depth', 'limit'], + }), + ); + expect(out).toContain('Truncated'); + expect(out).toContain('by depth, limit'); + }); + it('renders the degradation note as remediation, not a zero/empty blast radius', () => { // Shaped like the `_impactImpl` pdgLayer-degradation early return. const out = formatImpactResult({ diff --git a/gitnexus/test/unit/pdg-impact-engine.test.ts b/gitnexus/test/unit/pdg-impact-engine.test.ts new file mode 100644 index 000000000..f4e60ab2a --- /dev/null +++ b/gitnexus/test/unit/pdg-impact-engine.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest'; +import { IMPACT_MAX_DEPTH } from '../../src/mcp/tools.js'; +import { runImpactPDG } from '../../src/mcp/local/pdg-impact.js'; + +describe('runImpactPDG', () => { + it('clamps huge maxDepth values to the documented impact traversal cap', async () => { + let bfsQueries = 0; + const exec = async (_repo: string, query: string) => { + if (query.includes('MATCH (a:BasicBlock) WHERE')) { + return [{ id: 'BasicBlock:src/hot.ts:1:0:0' }]; + } + if (query.includes('MATCH (a:BasicBlock)-[r:CodeRelation]->(b:BasicBlock)')) { + bfsQueries += 1; + return [{ id: `BasicBlock:src/hot.ts:${bfsQueries + 1}:0:0` }]; + } + if (query.includes('MATCH (b:BasicBlock) WHERE b.id IN $ids')) return []; + if (query.includes('MATCH (s:`Function`)')) return []; + return []; + }; + + const result = await runImpactPDG({ + repo: { lbugPath: 'repo' }, + sym: { id: 'func:hot', name: 'hot', filePath: 'src/hot.ts', startLine: 0, endLine: 0 }, + symType: 'Function', + direction: 'downstream', + maxDepth: Number.MAX_SAFE_INTEGER, + limit: 50, + executeParameterized: exec as any, + }); + + expect(bfsQueries).toBe(IMPACT_MAX_DEPTH); + expect(result.truncated).toBe(true); + expect(result.truncatedBy).toBe('depth'); + }); + + it('keeps multiple reachable BasicBlocks on the same source line as separate statements', async () => { + let bfsQueries = 0; + const sameLineA = 'BasicBlock:src/hot.ts:1:0:1'; + const sameLineB = 'BasicBlock:src/hot.ts:1:0:2'; + const exec = async (_repo: string, query: string) => { + if (query.includes('MATCH (a:BasicBlock) WHERE')) { + return [{ id: 'BasicBlock:src/hot.ts:1:0:0' }]; + } + if (query.includes('MATCH (a:BasicBlock)-[r:CodeRelation]->(b:BasicBlock)')) { + bfsQueries += 1; + return bfsQueries === 1 ? [{ id: sameLineB }, { id: sameLineA }] : []; + } + if (query.includes('MATCH (b:BasicBlock) WHERE b.id IN $ids')) { + return [ + { id: sameLineB, line: 2, text: 'b();' }, + { id: sameLineA, line: 2, text: 'a();' }, + ]; + } + if (query.includes('MATCH (s:`Function`)')) { + return [{ id: 'func:hot', name: 'hot', label: 'Function', startLine: 0 }]; + } + return []; + }; + + const result = await runImpactPDG({ + repo: { lbugPath: 'repo' }, + sym: { id: 'func:hot', name: 'hot', filePath: 'src/hot.ts', startLine: 0, endLine: 3 }, + symType: 'Function', + direction: 'downstream', + maxDepth: 2, + limit: 50, + line: 1, + executeParameterized: exec as any, + }); + + expect(result.mode).toBe('pdg'); + expect((result as any).affectedStatementCount).toBe(2); + expect((result as any).affectedStatements.map((s: any) => s.line)).toEqual([2, 2]); + expect((result as any).affectedStatements.map((s: any) => s.text)).toEqual(['a();', 'b();']); + }); +}); diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 98f10e019..1d8906131 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -145,8 +145,11 @@ describe('GITNEXUS_TOOLS', () => { // The description names the mode:'pdg' statement-anchor semantics. expect(line.description).toMatch(/statement anchor/i); expect(line.description).toMatch(/pdg/i); - // The top-level description mentions the statement-anchored slice. + // The top-level description mentions the statement-anchored slice and result shape. expect(impactTool.description).toMatch(/statement-anchored|STATEMENT-ANCHORED/); + expect(impactTool.description).toContain('affectedStatements'); + expect(impactTool.description).toContain('target metadata'); + expect(impactTool.description).toContain('truncatedBy'); }); it('rename tool requires new_name', () => { @@ -304,6 +307,8 @@ describe('GITNEXUS_TOOLS', () => { expect(modeProp.description).toContain('pdg'); expect(modeProp.description).toContain('--pdg'); expect(modeProp.description.toLowerCase()).toContain('intra-procedural'); + expect(modeProp.description).toContain('affectedStatements'); + expect(modeProp.description).toContain('UNKNOWN-risk'); // The tool-level description must mention the mode so an LLM discovers it. expect(impactTool.description.toLowerCase()).toContain('mode'); expect(impactTool.description).toContain('pdg');